Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1605704 > unrolled thread

[PATCH 3/3] net: stmmac: Use AVB mode by default

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 3/3] net: stmmac: Use AVB mode by default Thierry Reding <thierry.reding@gmail.com> - 2017-03-21 16:20 +0100
    Re: [PATCH 3/3] net: stmmac: Use AVB mode by default Joao Pinto <Joao.Pinto@synopsys.com> - 2017-03-21 16:30 +0100
      Re: [PATCH 3/3] net: stmmac: Use AVB mode by default Thierry Reding <thierry.reding@gmail.com> - 2017-03-21 17:50 +0100
        Re: [PATCH 3/3] net: stmmac: Use AVB mode by default Joao Pinto <Joao.Pinto@synopsys.com> - 2017-03-21 18:00 +0100
        Re: [PATCH 3/3] net: stmmac: Use AVB mode by default Joao Pinto <Joao.Pinto@synopsys.com> - 2017-03-21 18:00 +0100
    Re: [PATCH 3/3] net: stmmac: Use AVB mode by default David Miller <davem@davemloft.net> - 2017-03-22 20:20 +0100

#1605704 — [PATCH 3/3] net: stmmac: Use AVB mode by default

From: Thierry Reding <treding@nvidia.com>

Prior to the recent multi-queue changes the driver would configure the
queues to use the AVB mode, but the mode then got switched to DCB. The
hardware still works fine in DCB mode, but my testing capabilities are
limited, so it's safer to revert to the prior setting anyway.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 include/linux/stmmac.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index be47b859e954..8349a5c1537b 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -56,8 +56,8 @@
 #define MTL_RX_ALGORITHM_WSP	0x5
 
 /* RX/TX Queue Mode */
-#define MTL_QUEUE_DCB		0x0
-#define MTL_QUEUE_AVB		0x1
+#define MTL_QUEUE_AVB		0x0
+#define MTL_QUEUE_DCB		0x1
 
 /* The MDC clock could be set higher than the IEEE 802.3
  * specified frequency limit 0f 2.5 MHz, by programming a clock divider
-- 
2.12.0

[toc] | [next] | [standalone]

#1605715

Às 3:12 PM de 3/21/2017, Thierry Reding escreveu:
> From: Thierry Reding <treding@nvidia.com>
> 
> Prior to the recent multi-queue changes the driver would configure the
> queues to use the AVB mode, but the mode then got switched to DCB. The
> hardware still works fine in DCB mode, but my testing capabilities are
> limited, so it's safer to revert to the prior setting anyway.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  include/linux/stmmac.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index be47b859e954..8349a5c1537b 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -56,8 +56,8 @@
>  #define MTL_RX_ALGORITHM_WSP	0x5
>  
>  /* RX/TX Queue Mode */
> -#define MTL_QUEUE_DCB		0x0
> -#define MTL_QUEUE_AVB		0x1
> +#define MTL_QUEUE_AVB		0x0
> +#define MTL_QUEUE_DCB		0x1
>  
>  /* The MDC clock could be set higher than the IEEE 802.3
>   * specified frequency limit 0f 2.5 MHz, by programming a clock divider
> 

Thierry, I don't understand this patch. It will have 0 impact.

In stmmac_platform configuration, 0 impact:

		if (of_property_read_bool(q_node, "snps,dcb-algorithm"))
			plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;
		else if (of_property_read_bool(q_node, "snps,avb-algorithm"))
			plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_AVB;
		else
**			plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;

In dwmac4_core, 0 impact:

	value &= GMAC_RX_QUEUE_CLEAR(queue);
	if (mode == MTL_QUEUE_AVB)
		value |= GMAC_RX_AV_QUEUE_ENABLE(queue);
	else if (mode == MTL_QUEUE_DCB)
		value |= GMAC_RX_DCB_QUEUE_ENABLE(queue);

I think you should set the default mode in (**).

Thanks.

[toc] | [prev] | [next] | [standalone]

#1605809

[Multipart message — attachments visible in raw view] — view raw

On Tue, Mar 21, 2017 at 03:23:00PM +0000, Joao Pinto wrote:
> Às 3:12 PM de 3/21/2017, Thierry Reding escreveu:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Prior to the recent multi-queue changes the driver would configure the
> > queues to use the AVB mode, but the mode then got switched to DCB. The
> > hardware still works fine in DCB mode, but my testing capabilities are
> > limited, so it's safer to revert to the prior setting anyway.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  include/linux/stmmac.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> > index be47b859e954..8349a5c1537b 100644
> > --- a/include/linux/stmmac.h
> > +++ b/include/linux/stmmac.h
> > @@ -56,8 +56,8 @@
> >  #define MTL_RX_ALGORITHM_WSP	0x5
> >  
> >  /* RX/TX Queue Mode */
> > -#define MTL_QUEUE_DCB		0x0
> > -#define MTL_QUEUE_AVB		0x1
> > +#define MTL_QUEUE_AVB		0x0
> > +#define MTL_QUEUE_DCB		0x1
> >  
> >  /* The MDC clock could be set higher than the IEEE 802.3
> >   * specified frequency limit 0f 2.5 MHz, by programming a clock divider
> > 
> 
> Thierry, I don't understand this patch. It will have 0 impact.
> 
> In stmmac_platform configuration, 0 impact:
> 
> 		if (of_property_read_bool(q_node, "snps,dcb-algorithm"))
> 			plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;
> 		else if (of_property_read_bool(q_node, "snps,avb-algorithm"))
> 			plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_AVB;
> 		else
> **			plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;
> 
> In dwmac4_core, 0 impact:
> 
> 	value &= GMAC_RX_QUEUE_CLEAR(queue);
> 	if (mode == MTL_QUEUE_AVB)
> 		value |= GMAC_RX_AV_QUEUE_ENABLE(queue);
> 	else if (mode == MTL_QUEUE_DCB)
> 		value |= GMAC_RX_DCB_QUEUE_ENABLE(queue);
> 
> I think you should set the default mode in (**).

That was my initial attempt, but then I realized that for old DTBs,
stmmac_mtl_setup() will already exit prematurely because of the missing
snps,mtl-{rx,tx}-config properties. It's pretty much for the same reason
as the separate assignment of the default {rx,tx}_queues_to_use. In this
case it's somewhat more obfuscated, though. Changing AVB to be mode 0
means that plat->rx_queues_cfg[].mode_to_use will contain AVB as default
because plat is devm_kzalloc()'ed.

Effectively this change makes all queues use AVB by default unless they
are configured using the new device tree bindings.

Thierry

[toc] | [prev] | [next] | [standalone]

#1605814

Às 4:50 PM de 3/21/2017, Joao Pinto escreveu:
> Às 4:42 PM de 3/21/2017, Thierry Reding escreveu:
>> On Tue, Mar 21, 2017 at 03:23:00PM +0000, Joao Pinto wrote:
>>> Às 3:12 PM de 3/21/2017, Thierry Reding escreveu:
>>>> From: Thierry Reding <treding@nvidia.com>
>>>>
>>>> Prior to the recent multi-queue changes the driver would configure the
>>>> queues to use the AVB mode, but the mode then got switched to DCB. The
>>>> hardware still works fine in DCB mode, but my testing capabilities are
>>>> limited, so it's safer to revert to the prior setting anyway.
>>>>
>>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>>> ---
>>>>  include/linux/stmmac.h | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
>>>> index be47b859e954..8349a5c1537b 100644
>>>> --- a/include/linux/stmmac.h
>>>> +++ b/include/linux/stmmac.h
>>>> @@ -56,8 +56,8 @@
>>>>  #define MTL_RX_ALGORITHM_WSP	0x5
>>>>  
>>>>  /* RX/TX Queue Mode */
>>>> -#define MTL_QUEUE_DCB		0x0
>>>> -#define MTL_QUEUE_AVB		0x1
>>>> +#define MTL_QUEUE_AVB		0x0
>>>> +#define MTL_QUEUE_DCB		0x1
>>>>  
>>>>  /* The MDC clock could be set higher than the IEEE 802.3
>>>>   * specified frequency limit 0f 2.5 MHz, by programming a clock divider
>>>>
>>>
>>> Thierry, I don't understand this patch. It will have 0 impact.
>>>
>>> In stmmac_platform configuration, 0 impact:
>>>
>>> 		if (of_property_read_bool(q_node, "snps,dcb-algorithm"))
>>> 			plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;
>>> 		else if (of_property_read_bool(q_node, "snps,avb-algorithm"))
>>> 			plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_AVB;
>>> 		else
>>> **			plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;
>>>
>>> In dwmac4_core, 0 impact:
>>>
>>> 	value &= GMAC_RX_QUEUE_CLEAR(queue);
>>> 	if (mode == MTL_QUEUE_AVB)
>>> 		value |= GMAC_RX_AV_QUEUE_ENABLE(queue);
>>> 	else if (mode == MTL_QUEUE_DCB)
>>> 		value |= GMAC_RX_DCB_QUEUE_ENABLE(queue);
>>>
>>> I think you should set the default mode in (**).
>>
>> That was my initial attempt, but then I realized that for old DTBs,
>> stmmac_mtl_setup() will already exit prematurely because of the missing
>> snps,mtl-{rx,tx}-config properties. It's pretty much for the same reason
>> as the separate assignment of the default {rx,tx}_queues_to_use. In this
>> case it's somewhat more obfuscated, though. Changing AVB to be mode 0
>> means that plat->rx_queues_cfg[].mode_to_use will contain AVB as default
>> because plat is devm_kzalloc()'ed.
>>
>> Effectively this change makes all queues use AVB by default unless they
>> are configured using the new device tree bindings.
> 
> Yes I keep forgeting that :), but you are assuming that
> plat->rx_queues_cfg[queue].mode_to_use is 0 by default, which might not be the
> case, but I agree with you that this is the simpler approach. Let's see what
> others have to say.

Forget what I said, yes devm_kzalloc() in plat guarantees this. I need a cup of
coffee :).

Acked-By: Joao Pinto <jpinto@synopsys.com>
>>
>> Thierry
>>
> 

[toc] | [prev] | [next] | [standalone]

#1605819

Às 4:42 PM de 3/21/2017, Thierry Reding escreveu:
> On Tue, Mar 21, 2017 at 03:23:00PM +0000, Joao Pinto wrote:
>> Às 3:12 PM de 3/21/2017, Thierry Reding escreveu:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> Prior to the recent multi-queue changes the driver would configure the
>>> queues to use the AVB mode, but the mode then got switched to DCB. The
>>> hardware still works fine in DCB mode, but my testing capabilities are
>>> limited, so it's safer to revert to the prior setting anyway.
>>>
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>> ---
>>>  include/linux/stmmac.h | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
>>> index be47b859e954..8349a5c1537b 100644
>>> --- a/include/linux/stmmac.h
>>> +++ b/include/linux/stmmac.h
>>> @@ -56,8 +56,8 @@
>>>  #define MTL_RX_ALGORITHM_WSP	0x5
>>>  
>>>  /* RX/TX Queue Mode */
>>> -#define MTL_QUEUE_DCB		0x0
>>> -#define MTL_QUEUE_AVB		0x1
>>> +#define MTL_QUEUE_AVB		0x0
>>> +#define MTL_QUEUE_DCB		0x1
>>>  
>>>  /* The MDC clock could be set higher than the IEEE 802.3
>>>   * specified frequency limit 0f 2.5 MHz, by programming a clock divider
>>>
>>
>> Thierry, I don't understand this patch. It will have 0 impact.
>>
>> In stmmac_platform configuration, 0 impact:
>>
>> 		if (of_property_read_bool(q_node, "snps,dcb-algorithm"))
>> 			plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;
>> 		else if (of_property_read_bool(q_node, "snps,avb-algorithm"))
>> 			plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_AVB;
>> 		else
>> **			plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;
>>
>> In dwmac4_core, 0 impact:
>>
>> 	value &= GMAC_RX_QUEUE_CLEAR(queue);
>> 	if (mode == MTL_QUEUE_AVB)
>> 		value |= GMAC_RX_AV_QUEUE_ENABLE(queue);
>> 	else if (mode == MTL_QUEUE_DCB)
>> 		value |= GMAC_RX_DCB_QUEUE_ENABLE(queue);
>>
>> I think you should set the default mode in (**).
> 
> That was my initial attempt, but then I realized that for old DTBs,
> stmmac_mtl_setup() will already exit prematurely because of the missing
> snps,mtl-{rx,tx}-config properties. It's pretty much for the same reason
> as the separate assignment of the default {rx,tx}_queues_to_use. In this
> case it's somewhat more obfuscated, though. Changing AVB to be mode 0
> means that plat->rx_queues_cfg[].mode_to_use will contain AVB as default
> because plat is devm_kzalloc()'ed.
> 
> Effectively this change makes all queues use AVB by default unless they
> are configured using the new device tree bindings.

Yes I keep forgeting that :), but you are assuming that
plat->rx_queues_cfg[queue].mode_to_use is 0 by default, which might not be the
case, but I agree with you that this is the simpler approach. Let's see what
others have to say.
> 
> Thierry
> 

[toc] | [prev] | [next] | [standalone]

#1606931

From: Thierry Reding <thierry.reding@gmail.com>
Date: Tue, 21 Mar 2017 16:12:11 +0100

> From: Thierry Reding <treding@nvidia.com>
> 
> Prior to the recent multi-queue changes the driver would configure the
> queues to use the AVB mode, but the mode then got switched to DCB. The
> hardware still works fine in DCB mode, but my testing capabilities are
> limited, so it's safer to revert to the prior setting anyway.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Applied to net-next

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web