linux-kernel - Re: [PATCH 3/3] net: stmmac: Use AVB mode by default

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c233aec8-c14c-52f6-8d76-59a4c3ad1eb8@synopsys.com>
Date:   Tue, 21 Mar 2017 16:52:42 +0000
From:   Joao Pinto <Joao.Pinto@...opsys.com>
To:     Thierry Reding <thierry.reding@...il.com>,
        Joao Pinto <Joao.Pinto@...opsys.com>
CC:     "David S . Miller" <davem@...emloft.net>,
        Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...com>,
        "LABBE Corentin" <clabbe.montjoie@...il.com>,
        <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/3] net: stmmac: Use AVB mode by default

À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@...dia.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@...dia.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@...opsys.com>
>>
>> Thierry
>>
>