[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230110090650.2138edf0@bootlin.com>
Date: Tue, 10 Jan 2023 09:06:50 +0100
From: Herve Codina <herve.codina@...tlin.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: Li Yang <leoyang.li@....com>, Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Liam Girdwood <lgirdwood@...il.com>,
Mark Brown <broonie@...nel.org>,
Christophe Leroy <christophe.leroy@...roup.eu>,
Michael Ellerman <mpe@...erman.id.au>,
Nicholas Piggin <npiggin@...il.com>,
Qiang Zhao <qiang.zhao@....com>,
Jaroslav Kysela <perex@...ex.cz>,
Takashi Iwai <tiwai@...e.com>,
Shengjiu Wang <shengjiu.wang@...il.com>,
Xiubo Li <Xiubo.Lee@...il.com>,
Fabio Estevam <festevam@...il.com>,
Nicolin Chen <nicoleotsuka@...il.com>,
linuxppc-dev@...ts.ozlabs.org,
linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, alsa-devel@...a-project.org,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH v2 05/10] dt-bindings: soc: fsl: cpm_qe: Add QMC
controller
Hi Krzysztof,
On Sun, 8 Jan 2023 16:14:47 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org> wrote:
[...]
> > +
> > + interrupts:
> > + description: SCC interrupt line in the CPM interrupt controller
>
> Missing constraints.
'maxItems: 1' will be added in v3
>
> > +
> > + fsl,cpm-command:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: Cf. soc/fsl/cpm_qe/cpm.txt
>
> Missing description.
'fsl,cpm-command' will be removed in v3.
The value needed is determined based on other information.
This is not needed in the DT.
>
> > +
> > + tsa:
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + description: phandle to the TSA
>
> Missing vendor prefix. Does not look like a generic property.
Will be be changed to 'fsl,tsa'
and also 'tsa-cell-id' will be changed to 'fsl,tsa-cell-id'
>
[...]
> > +
> > +patternProperties:
> > + "^channel@([0-9]|[1-5][0-9]|6[0-3])$":
> > + description:
> > + A channel managed by this controller
> > + type: object
> > +
> > + properties:
> > + reg:
> > + minimum: 0
> > + maximum: 63
> > + description:
> > + The channel number
> > +
> > + fsl,mode:
> > + $ref: /schemas/types.yaml#/definitions/string
> > + enum: [transparent, hdlc]
> > + default: transparent
> > + description: Operational mode
>
> And what do they mean?
I will change with
description: |
The channel operational mode
- hdlc: The channel handles HDLC frames
- transparent: The channel handles raw data without any processing
>
> > +
> > + fsl,reverse-data:
> > + $ref: /schemas/types.yaml#/definitions/flag
> > + description:
> > + The bit order as seen on the channels is reversed,
> > + transmitting/receiving the MSB of each octet first.
> > + This flag is used only in 'transparent' mode.
> > +
> > + tx-ts-mask:
>
> Missing vendor prefix.
Will be added, also on rx-ts-mask.
>
> > + $ref: /schemas/types.yaml#/definitions/uint64
> > + description:
> > + Channel assigned Tx time-slots within the Tx time-slots routed
> > + by the TSA to this cell.
> > +
> > + rx-ts-mask:
> > + $ref: /schemas/types.yaml#/definitions/uint64
> > + description:
> > + Channel assigned Rx time-slots within the Rx time-slots routed
> > + by the TSA to this cell.
> > +
> > + required:
> > + - reg
> > + - tx-ts-mask
> > + - rx-ts-mask
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - reg-names
> > + - interrupts
> > + - tsa
> > + - tsa-cell-id
> > + - '#address-cells'
> > + - '#size-cells'
> > + - '#chan-cells'
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/soc/fsl-tsa.h>
> > +
> > + scc_qmc@a60 {
>
> No underscores in node names.
>
> Node names should be generic.
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
Will be changed to qmc@a60
>
> > + compatible = "fsl,mpc885-scc-qmc", "fsl,cpm1-scc-qmc";
> > + reg = <0xa60 0x20>,
> > + <0x3f00 0xc0>,
> > + <0x2000 0x1000>;
> > + reg-names = "scc_regs", "scc_pram", "dpram";
> > + interrupts = <27>;
> > + interrupt-parent = <&CPM_PIC>;
> > + fsl,cpm-command = <0xc0>;
> > +
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + #chan-cells = <1>;
> > +
> > + tsa = <&tsa>;
> > + tsa-cell-id = <FSL_CPM_TSA_SCC4>;
> > +
> > + channel@16 {
> > + /* Ch16 : First 4 even TS from all routed from TSA */
> > + reg = <16>;
> > + fsl,mode = "transparent";
> > + fsl,reverse-data;
> > + tx-ts-mask = <0x00000000 0x000000AA>;
> > + rx-ts-mask = <0x00000000 0x000000AA>;
>
> Keep case consistent. lower-case hex.
Will be fixed
>
> Best regards,
> Krzysztof
>
Thanks for the review,
Best regards,
Hervé
--
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Powered by blists - more mailing lists