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@linaro.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-devicetre...

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é