[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3aa78c08-6d19-8480-cf31-2de41642b100@linaro.org>
Date: Mon, 24 Oct 2022 19:29:09 -0400
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Sebastian Reichel <sebastian.reichel@...labora.com>
Cc: Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>,
Giuseppe Cavallaro <peppe.cavallaro@...com>,
Jose Abreu <joabreu@...opsys.com>, netdev@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
kernel@...labora.com
Subject: Re: [PATCH 1/1] dt-bindings: net: snps,dwmac: Document queue config
subnodes
On 24/10/2022 19:28, Krzysztof Kozlowski wrote:
> On 24/10/2022 18:28, Sebastian Reichel wrote:
>> Hi,
>>
>> On Sat, Oct 22, 2022 at 12:05:15PM -0400, Krzysztof Kozlowski wrote:
>>> On 21/10/2022 13:10, Sebastian Reichel wrote:
>>>> The queue configuration is referenced by snps,mtl-rx-config and
>>>> snps,mtl-tx-config. Most in-tree DTs put the referenced object
>>>> as child node of the dwmac node.
>>>>
>>>> This adds proper description for this setup, which has the
>>>> advantage of properly making sure only known properties are
>>>> used.
>>>>
>>>> Signed-off-by: Sebastian Reichel <sebastian.reichel@...labora.com>
>>>> ---
>>>> [...]
>>>
>>> Please update the DTS example with all this.
>>
>> ok
>
> BTW, I also found:
>
> https://lore.kernel.org/linux-devicetree/20201214091616.13545-5-Sergey.Semin@baikalelectronics.ru/
>>
>>>
>>>>
>>>> snps,mtl-tx-config:
>>>> $ref: /schemas/types.yaml#/definitions/phandle
>>>> description:
>>>> - Multiple TX Queues parameters. Phandle to a node that can
>>>> - contain the following properties
>>>> - * snps,tx-queues-to-use, number of TX queues to be used in the
>>>> - driver
>>>> - * Choose one of these TX scheduling algorithms
>>>> - * snps,tx-sched-wrr, Weighted Round Robin
>>>> - * snps,tx-sched-wfq, Weighted Fair Queuing
>>>> - * snps,tx-sched-dwrr, Deficit Weighted Round Robin
>>>> - * snps,tx-sched-sp, Strict priority
>>>> - * For each TX queue
>>>> - * snps,weight, TX queue weight (if using a DCB weight
>>>> - algorithm)
>>>> - * Choose one of these modes
>>>> - * snps,dcb-algorithm, TX queue will be working in DCB
>>>> - * snps,avb-algorithm, TX queue will be working in AVB
>>>> - [Attention] Queue 0 is reserved for legacy traffic
>>>> - and so no AVB is available in this queue.
>>>> - * Configure Credit Base Shaper (if AVB Mode selected)
>>>> - * snps,send_slope, enable Low Power Interface
>>>> - * snps,idle_slope, unlock on WoL
>>>> - * snps,high_credit, max write outstanding req. limit
>>>> - * snps,low_credit, max read outstanding req. limit
>>>> - * snps,priority, bitmask of the priorities assigned to the queue.
>>>> - When a PFC frame is received with priorities matching the bitmask,
>>>> - the queue is blocked from transmitting for the pause time specified
>>>> - in the PFC frame.
>>>> + Multiple TX Queues parameters. Phandle to a node that
>>>> + implements the 'tx-queues-config' object described in
>>>> + this binding.
>>>> +
>>>> + tx-queues-config:
>>>> + type: object
>>>> + properties:
>>>> + snps,tx-queues-to-use:
>>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>>> + description: number of TX queues to be used in the driver
>>>> + snps,tx-sched-wrr:
>>>> + type: boolean
>>>> + description: Weighted Round Robin
>>>> + snps,tx-sched-wfq:
>>>> + type: boolean
>>>> + description: Weighted Fair Queuing
>>>> + snps,tx-sched-dwrr:
>>>> + type: boolean
>>>> + description: Deficit Weighted Round Robin
>>>> + snps,tx-sched-sp:
>>>> + type: boolean
>>>> + description: Strict priority
>>>> + patternProperties:
>>>> + "^queue[0-9]$":
>>>> + description: Each subnode represents a queue.
>>>> + type: object
>>>> + properties:
>>>> + snps,weight:
>>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>>> + description: TX queue weight (if using a DCB weight algorithm)
>>>> + snps,dcb-algorithm:
>>>> + type: boolean
>>>> + description: TX queue will be working in DCB
>>>> + snps,avb-algorithm:
>>>
>>> Is DCB and AVB compatible with each other? If not, then this should be
>>> rather enum (with a string for algorithm name).
>>>
>>> This applies also to other fields which are mutually exclusive.
>>
>> Yes and I agree it is ugly. But this is not a new binding, but just
>> properly describing the existing binding. It's not my fault :)
>
> I understand (and did not think it's your fault), but you are
> redesigning them. Existing DTS will have to be updated. If this is
> already implemented by some other DTS, then well... they did not follow
> bindings, so it's their fault. :)
>
> What I want to say, why refactoring it if the new format is still poor?
>>
>>>> + type: boolean
>>>> + description:
>>>> + TX queue will be working in AVB.
>>>> + Queue 0 is reserved for legacy traffic and so no AVB is
>>>> + available in this queue.
>>>> + snps,send_slope:
>>>
>>> Use hyphens, no underscores.
>>> (This is already an incompatible change in bindings, so we can fix up
>>> the naming)
>>
>> No, this is not an incompatible change in the bindings. It's 100%
>> compatible. What this patch does is removing the text description
>> for 'snps,mtl-tx-config' and instead documenting the node in YAML
>> syntax. 'snps,mtl-tx-config' does not specify where this node should
>> be, so many DTS files do this:
>
> Old binding did not document "tx-queues-config". Old binding had
> "snps,mtl-tx-config" which was a phandle, so this is an ABI break of
> bindings.
Bah, not ABI break, just change in bindings, of course :)
Best regards,
Krzysztof
Powered by blists - more mailing lists