Re: [PATCH v3 05/10] dt-bindings: soc: fsl: cpm_qe: Add QMC controller

[Herve Codina <herve.codina@bootlin.com>]

On Tue, 24 Jan 2023 13:24:48 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 24/01/2023 12:23, Herve Codina wrote:
> > On Tue, 24 Jan 2023 11:02:52 +0100
> > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> >   
> >> On 24/01/2023 10:42, Herve Codina wrote:  
> >>> Hi Krzysztof,
> >>>
> >>> On Tue, 17 Jan 2023 12:31:09 +0100
> >>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> >>>     
> >>>> On 13/01/2023 11:37, Herve Codina wrote:    
> >>>>> Add support for the QMC (QUICC Multichannel Controller)
> >>>>> available in some PowerQUICC SoC such as MPC885 or MPC866.
> >>>>>
> >>>>> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> >>>>> ---
> >>>>>  .../bindings/soc/fsl/cpm_qe/fsl,qmc.yaml      | 164 ++++++++++++++++++
> >>>>>  1 file changed, 164 insertions(+)
> >>>>>  create mode 100644 Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qmc.yaml
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qmc.yaml b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qmc.yaml
> >>>>> new file mode 100644
> >>>>> index 000000000000..3ec52f1635c8
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,qmc.yaml
> >>>>> @@ -0,0 +1,164 @@
> >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>>>> +%YAML 1.2
> >>>>> +---
> >>>>> +$id: http://devicetree.org/schemas/soc/fsl/cpm_qe/fsl,qmc.yaml#
> >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>>> +
> >>>>> +title: PowerQUICC CPM QUICC Multichannel Controller (QMC)
> >>>>> +
> >>>>> +maintainers:
> >>>>> +  - Herve Codina <herve.codina@bootlin.com>
> >>>>> +
> >>>>> +description: |
> >>>>> +  The QMC (QUICC Multichannel Controller) emulates up to 64 channels within
> >>>>> +  one serial controller using the same TDM physical interface routed from
> >>>>> +  TSA.
> >>>>> +
> >>>>> +properties:
> >>>>> +  compatible:
> >>>>> +    items:
> >>>>> +      - enum:
> >>>>> +          - fsl,mpc885-scc-qmc
> >>>>> +          - fsl,mpc866-scc-qmc
> >>>>> +      - const: fsl,cpm1-scc-qmc
> >>>>> +
> >>>>> +  reg:
> >>>>> +    items:
> >>>>> +      - description: SCC (Serial communication controller) register base
> >>>>> +      - description: SCC parameter ram base
> >>>>> +      - description: Dual port ram base
> >>>>> +
> >>>>> +  reg-names:
> >>>>> +    items:
> >>>>> +      - const: scc_regs
> >>>>> +      - const: scc_pram
> >>>>> +      - const: dpram
> >>>>> +
> >>>>> +  interrupts:
> >>>>> +    maxItems: 1
> >>>>> +    description: SCC interrupt line in the CPM interrupt controller
> >>>>> +
> >>>>> +  fsl,tsa:
> >>>>> +    $ref: /schemas/types.yaml#/definitions/phandle
> >>>>> +    description: phandle to the TSA
> >>>>> +
> >>>>> +  fsl,tsa-cell-id:
> >>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>>>> +    enum: [1, 2, 3]
> >>>>> +    description: |
> >>>>> +      TSA cell ID (dt-bindings/soc/fsl,tsa.h defines these values)
> >>>>> +       - 1: SCC2
> >>>>> +       - 2: SCC3
> >>>>> +       - 3: SCC4      
> >>>>
> >>>> Is this used as argument to tsa? If so, this should be part of fsl,tsa
> >>>> property, just like we do for all syscon-like phandles.    
> >>>
> >>> Yes, indeed.
> >>> I will move 'fsl,tsa' to 'fsl,tsa-cell' with 'fsl,tsa-cell' a phandle/number
> >>> pair (the phandle to TSA node and the TSA cell id to use)    
> >>
> >> Move to fsl,tsa, not from.  
> > 
> > Well, I plan to remove both fsl,tsa and fsl,tsa-cell-id and use this:
> >   fsl,tsa-cell:
> >     $ref: /schemas/types.yaml#/definitions/phandle-array
> >     items:
> >       - items:
> >           - description: phandle to TSA node
> >           - enum: [1, 2, 3]
> >             description: |
> >               TSA cell ID (dt-bindings/soc/fsl,tsa.h defines these values)
> >                - 1: SCC2
> >                - 2: SCC3
> >                - 3: SCC4
> >     description:
> >       Should be a phandle/number pair. The phandle to TSA node and the TSA
> >       cell ID to use.
> > 
> > Is that what you were thinking about ?  
> 
> Yes, except again, so third time, why calling this "cell"? Move it to
> fsl,tsa.
> 

Why calling this "cell" ? Just because we reference a "cell" using the TSA
cell ID inside TSA and not the TSA itself.

Maybe the problem is the term "cell" as it is not the DT definition of
"cell" but the source/destination of the TSA routing.

TSA can route data from/to some "serial controller".
These serial controllers are :
- SCC (Serial Communication Controller)
- SMC (Serial Management Controller)
- UCC (Unified Communication Controller)

Only SCCs are handled here.

Maybe the term "serial" makes more sense which will lead to
  fsl,tsa-serial = <&tsa, SCC4>;

Best regards,
Hervé

-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com