Hi Krzysztof,
On Sun, 8 Jan 2023 16:14:47 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> 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
