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@xxxxxxxxxxxxx> >>>> --- >>>> [...] >>> >>> Please update the DTS example with all this. >> >> ok > > BTW, I also found: > > https://lore.kernel.org/linux-devicetree/20201214091616.13545-5-Sergey.Semin@xxxxxxxxxxxxxxxxxxxx/ >> >>> >>>> >>>> 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