[v6,08/30] dt-bindings: soc: fsl: cpm_qe: cpm1-scc-qmc: Add support for QMC HDLC

  The QMC (QUICC mutichannel controller) is a controller present in some
PowerQUICC SoC such as MPC885.
The QMC HDLC uses the QMC controller to transfer HDLC data.

Additionally, a framer can be connected to the QMC HDLC.
If present, this framer is the interface between the TDM bus used by the
QMC HDLC and the E1/T1 line.
The QMC HDLC can use this framer to get information about the E1/T1 line
and configure the E1/T1 line.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 .../soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml      | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)
  

On Fri, 22 Sep 2023 09:58:43 +0200, Herve Codina wrote:
> The QMC (QUICC mutichannel controller) is a controller present in some
> PowerQUICC SoC such as MPC885.
> The QMC HDLC uses the QMC controller to transfer HDLC data.
> 
> Additionally, a framer can be connected to the QMC HDLC.
> If present, this framer is the interface between the TDM bus used by the
> QMC HDLC and the E1/T1 line.
> The QMC HDLC can use this framer to get information about the E1/T1 line
> and configure the E1/T1 line.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  .../soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml      | 24 +++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>
  

On 22/09/2023 09:58, Herve Codina wrote:
> The QMC (QUICC mutichannel controller) is a controller present in some
> PowerQUICC SoC such as MPC885.
> The QMC HDLC uses the QMC controller to transfer HDLC data.
> 
> Additionally, a framer can be connected to the QMC HDLC.
> If present, this framer is the interface between the TDM bus used by the
> QMC HDLC and the E1/T1 line.
> The QMC HDLC can use this framer to get information about the E1/T1 line
> and configure the E1/T1 line.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  .../soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml      | 24 +++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml
> index 82d9beb48e00..61dfd5ef7407 100644
> --- a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml
> +++ b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml
> @@ -101,6 +101,27 @@ patternProperties:
>            Channel assigned Rx time-slots within the Rx time-slots routed by the
>            TSA to this cell.
>  
> +      compatible:
> +        const: fsl,qmc-hdlc

Why this is not a device/SoC specific compatible?

> +
> +      fsl,framer:
> +        $ref: /schemas/types.yaml#/definitions/phandle
> +        description:
> +          phandle to the framer node. The framer is in charge of an E1/T1 line
> +          interface connected to the TDM bus. It can be used to get the E1/T1 line
> +          status such as link up/down.
> +
> +    allOf:
> +      - if:
> +          properties:
> +            compatible:
> +              not:
> +                contains:
> +                  const: fsl,qmc-hdlc
> +        then:
> +          properties:
> +            fsl,framer: false
> +
>      required:
>        - reg
>        - fsl,tx-ts-mask
> @@ -159,5 +180,8 @@ examples:
>              fsl,operational-mode = "hdlc";
>              fsl,tx-ts-mask = <0x00000000 0x0000ff00>;
>              fsl,rx-ts-mask = <0x00000000 0x0000ff00>;
> +
> +            compatible = "fsl,qmc-hdlc";

compatible is always the first property.

> +            fsl,framer = <&framer>;
>          };
>      };

Best regards,
Krzysztof
  

Hi Krzysztof,

On Sat, 23 Sep 2023 19:39:49 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 22/09/2023 09:58, Herve Codina wrote:
> > The QMC (QUICC mutichannel controller) is a controller present in some
> > PowerQUICC SoC such as MPC885.
> > The QMC HDLC uses the QMC controller to transfer HDLC data.
> > 
> > Additionally, a framer can be connected to the QMC HDLC.
> > If present, this framer is the interface between the TDM bus used by the
> > QMC HDLC and the E1/T1 line.
> > The QMC HDLC can use this framer to get information about the E1/T1 line
> > and configure the E1/T1 line.
> > 
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > ---
> >  .../soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml      | 24 +++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml
> > index 82d9beb48e00..61dfd5ef7407 100644
> > --- a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml
> > +++ b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml
> > @@ -101,6 +101,27 @@ patternProperties:
> >            Channel assigned Rx time-slots within the Rx time-slots routed by the
> >            TSA to this cell.
> >  
> > +      compatible:
> > +        const: fsl,qmc-hdlc  
> 
> Why this is not a device/SoC specific compatible?

This compatible is present in a QMC channel.
The parent node (the QMC itself) contains a compatible with device/SoC:
--- 8< ---
  compatible:
    items:
      - enum:
          - fsl,mpc885-scc-qmc
          - fsl,mpc866-scc-qmc
      - const: fsl,cpm1-scc-qmc
--- 8< ---

At the child level (ie QMC channel), I am not sure that adding device/SoC
makes sense. This compatible indicates that the QMC channel is handled by
the QMC HDLC driver.
At this level, whatever the device/SoC, we have to be QMC compliant.

With these details, do you still think I need to change the child (channel)
compatible ?

> 
> > +
> > +      fsl,framer:
> > +        $ref: /schemas/types.yaml#/definitions/phandle
> > +        description:
> > +          phandle to the framer node. The framer is in charge of an E1/T1 line
> > +          interface connected to the TDM bus. It can be used to get the E1/T1 line
> > +          status such as link up/down.
> > +
> > +    allOf:
> > +      - if:
> > +          properties:
> > +            compatible:
> > +              not:
> > +                contains:
> > +                  const: fsl,qmc-hdlc
> > +        then:
> > +          properties:
> > +            fsl,framer: false
> > +
> >      required:
> >        - reg
> >        - fsl,tx-ts-mask
> > @@ -159,5 +180,8 @@ examples:
> >              fsl,operational-mode = "hdlc";
> >              fsl,tx-ts-mask = <0x00000000 0x0000ff00>;
> >              fsl,rx-ts-mask = <0x00000000 0x0000ff00>;
> > +
> > +            compatible = "fsl,qmc-hdlc";  
> 
> compatible is always the first property.

Will be moved to the first property in the next iteration.

Best regards,
Hervé

> 
> > +            fsl,framer = <&framer>;
> >          };
> >      };  
> 
> Best regards,
> Krzysztof
>
  

On 25/09/2023 10:17, Herve Codina wrote:
> Hi Krzysztof,
> 
> On Sat, 23 Sep 2023 19:39:49 +0200
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
>> On 22/09/2023 09:58, Herve Codina wrote:
>>> The QMC (QUICC mutichannel controller) is a controller present in some
>>> PowerQUICC SoC such as MPC885.
>>> The QMC HDLC uses the QMC controller to transfer HDLC data.
>>>
>>> Additionally, a framer can be connected to the QMC HDLC.
>>> If present, this framer is the interface between the TDM bus used by the
>>> QMC HDLC and the E1/T1 line.
>>> The QMC HDLC can use this framer to get information about the E1/T1 line
>>> and configure the E1/T1 line.
>>>
>>> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
>>> ---
>>>  .../soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml      | 24 +++++++++++++++++++
>>>  1 file changed, 24 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml
>>> index 82d9beb48e00..61dfd5ef7407 100644
>>> --- a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml
>>> +++ b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml
>>> @@ -101,6 +101,27 @@ patternProperties:
>>>            Channel assigned Rx time-slots within the Rx time-slots routed by the
>>>            TSA to this cell.
>>>  
>>> +      compatible:
>>> +        const: fsl,qmc-hdlc  
>>
>> Why this is not a device/SoC specific compatible?
> 
> This compatible is present in a QMC channel.
> The parent node (the QMC itself) contains a compatible with device/SoC:
> --- 8< ---
>   compatible:
>     items:
>       - enum:
>           - fsl,mpc885-scc-qmc
>           - fsl,mpc866-scc-qmc
>       - const: fsl,cpm1-scc-qmc
> --- 8< ---
> 
> At the child level (ie QMC channel), I am not sure that adding device/SoC
> makes sense. This compatible indicates that the QMC channel is handled by
> the QMC HDLC driver.
> At this level, whatever the device/SoC, we have to be QMC compliant.
> 
> With these details, do you still think I need to change the child (channel)
> compatible ?

From OS point of view, you have a driver binding to this child-level
compatible. How do you enforce Linux driver binding based on parent
compatible? I looked at your next patch and I did not see it.

Best regards,
Krzysztof
  

On Mon, 25 Sep 2023 10:21:15 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 25/09/2023 10:17, Herve Codina wrote:
> > Hi Krzysztof,
> > 
> > On Sat, 23 Sep 2023 19:39:49 +0200
> > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> >   
> >> On 22/09/2023 09:58, Herve Codina wrote:  
> >>> The QMC (QUICC mutichannel controller) is a controller present in some
> >>> PowerQUICC SoC such as MPC885.
> >>> The QMC HDLC uses the QMC controller to transfer HDLC data.
> >>>
> >>> Additionally, a framer can be connected to the QMC HDLC.
> >>> If present, this framer is the interface between the TDM bus used by the
> >>> QMC HDLC and the E1/T1 line.
> >>> The QMC HDLC can use this framer to get information about the E1/T1 line
> >>> and configure the E1/T1 line.
> >>>
> >>> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> >>> ---
> >>>  .../soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml      | 24 +++++++++++++++++++
> >>>  1 file changed, 24 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml
> >>> index 82d9beb48e00..61dfd5ef7407 100644
> >>> --- a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml
> >>> +++ b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml
> >>> @@ -101,6 +101,27 @@ patternProperties:
> >>>            Channel assigned Rx time-slots within the Rx time-slots routed by the
> >>>            TSA to this cell.
> >>>  
> >>> +      compatible:
> >>> +        const: fsl,qmc-hdlc    
> >>
> >> Why this is not a device/SoC specific compatible?  
> > 
> > This compatible is present in a QMC channel.
> > The parent node (the QMC itself) contains a compatible with device/SoC:
> > --- 8< ---
> >   compatible:
> >     items:
> >       - enum:
> >           - fsl,mpc885-scc-qmc
> >           - fsl,mpc866-scc-qmc
> >       - const: fsl,cpm1-scc-qmc
> > --- 8< ---
> > 
> > At the child level (ie QMC channel), I am not sure that adding device/SoC
> > makes sense. This compatible indicates that the QMC channel is handled by
> > the QMC HDLC driver.
> > At this level, whatever the device/SoC, we have to be QMC compliant.
> > 
> > With these details, do you still think I need to change the child (channel)
> > compatible ?  
> 
> From OS point of view, you have a driver binding to this child-level
> compatible. How do you enforce Linux driver binding based on parent
> compatible? I looked at your next patch and I did not see it.

We do not need to have the child driver binding based on parent.
We have to ensure that the child handles a QMC channel and the parent provides
a QMC channel.

A QMC controller (parent) has to implement the QMC API (include/soc/fsl/qe/qmc.h)
and a QMC channel driver (child) has to use the QMC API.

Best regards,
Hervé

> 
> Best regards,
> Krzysztof
>
  

On 25/09/2023 12:27, Herve Codina wrote:
> On Mon, 25 Sep 2023 10:21:15 +0200
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
>> On 25/09/2023 10:17, Herve Codina wrote:
>>> Hi Krzysztof,
>>>
>>> On Sat, 23 Sep 2023 19:39:49 +0200
>>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>>>   
>>>> On 22/09/2023 09:58, Herve Codina wrote:  
>>>>> The QMC (QUICC mutichannel controller) is a controller present in some
>>>>> PowerQUICC SoC such as MPC885.
>>>>> The QMC HDLC uses the QMC controller to transfer HDLC data.
>>>>>
>>>>> Additionally, a framer can be connected to the QMC HDLC.
>>>>> If present, this framer is the interface between the TDM bus used by the
>>>>> QMC HDLC and the E1/T1 line.
>>>>> The QMC HDLC can use this framer to get information about the E1/T1 line
>>>>> and configure the E1/T1 line.
>>>>>
>>>>> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
>>>>> ---
>>>>>  .../soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml      | 24 +++++++++++++++++++
>>>>>  1 file changed, 24 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml
>>>>> index 82d9beb48e00..61dfd5ef7407 100644
>>>>> --- a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml
>>>>> +++ b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml
>>>>> @@ -101,6 +101,27 @@ patternProperties:
>>>>>            Channel assigned Rx time-slots within the Rx time-slots routed by the
>>>>>            TSA to this cell.
>>>>>  
>>>>> +      compatible:
>>>>> +        const: fsl,qmc-hdlc    
>>>>
>>>> Why this is not a device/SoC specific compatible?  
>>>
>>> This compatible is present in a QMC channel.
>>> The parent node (the QMC itself) contains a compatible with device/SoC:
>>> --- 8< ---
>>>   compatible:
>>>     items:
>>>       - enum:
>>>           - fsl,mpc885-scc-qmc
>>>           - fsl,mpc866-scc-qmc
>>>       - const: fsl,cpm1-scc-qmc
>>> --- 8< ---
>>>
>>> At the child level (ie QMC channel), I am not sure that adding device/SoC
>>> makes sense. This compatible indicates that the QMC channel is handled by
>>> the QMC HDLC driver.
>>> At this level, whatever the device/SoC, we have to be QMC compliant.
>>>
>>> With these details, do you still think I need to change the child (channel)
>>> compatible ?  
>>
>> From OS point of view, you have a driver binding to this child-level
>> compatible. How do you enforce Linux driver binding based on parent
>> compatible? I looked at your next patch and I did not see it.
> 
> We do not need to have the child driver binding based on parent.

Exactly, that's what I said.

> We have to ensure that the child handles a QMC channel and the parent provides
> a QMC channel.
> 
> A QMC controller (parent) has to implement the QMC API (include/soc/fsl/qe/qmc.h)
> and a QMC channel driver (child) has to use the QMC API.

How does this solve my concerns? Sorry, I do not understand. Your driver
is a platform driver and binds to the generic compatible. How do you
solve regular compatibility issues (need for quirks) if parent
compatible is not used?

How does being QMC compliant affects driver binding and
compatibility/quirks?

We are back to my original question and I don't think you answered to
any of the concerns.

Best regards,
Krzysztof
  

On Mon, 25 Sep 2023 12:44:35 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 25/09/2023 12:27, Herve Codina wrote:
> > On Mon, 25 Sep 2023 10:21:15 +0200
> > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> >   
> >> On 25/09/2023 10:17, Herve Codina wrote:  
> >>> Hi Krzysztof,
> >>>
> >>> On Sat, 23 Sep 2023 19:39:49 +0200
> >>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> >>>     
> >>>> On 22/09/2023 09:58, Herve Codina wrote:    
> >>>>> The QMC (QUICC mutichannel controller) is a controller present in some
> >>>>> PowerQUICC SoC such as MPC885.
> >>>>> The QMC HDLC uses the QMC controller to transfer HDLC data.
> >>>>>
> >>>>> Additionally, a framer can be connected to the QMC HDLC.
> >>>>> If present, this framer is the interface between the TDM bus used by the
> >>>>> QMC HDLC and the E1/T1 line.
> >>>>> The QMC HDLC can use this framer to get information about the E1/T1 line
> >>>>> and configure the E1/T1 line.
> >>>>>
> >>>>> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> >>>>> ---
> >>>>>  .../soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml      | 24 +++++++++++++++++++
> >>>>>  1 file changed, 24 insertions(+)
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml
> >>>>> index 82d9beb48e00..61dfd5ef7407 100644
> >>>>> --- a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml
> >>>>> +++ b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml
> >>>>> @@ -101,6 +101,27 @@ patternProperties:
> >>>>>            Channel assigned Rx time-slots within the Rx time-slots routed by the
> >>>>>            TSA to this cell.
> >>>>>  
> >>>>> +      compatible:
> >>>>> +        const: fsl,qmc-hdlc      
> >>>>
> >>>> Why this is not a device/SoC specific compatible?    
> >>>
> >>> This compatible is present in a QMC channel.
> >>> The parent node (the QMC itself) contains a compatible with device/SoC:
> >>> --- 8< ---
> >>>   compatible:
> >>>     items:
> >>>       - enum:
> >>>           - fsl,mpc885-scc-qmc
> >>>           - fsl,mpc866-scc-qmc
> >>>       - const: fsl,cpm1-scc-qmc
> >>> --- 8< ---
> >>>
> >>> At the child level (ie QMC channel), I am not sure that adding device/SoC
> >>> makes sense. This compatible indicates that the QMC channel is handled by
> >>> the QMC HDLC driver.
> >>> At this level, whatever the device/SoC, we have to be QMC compliant.
> >>>
> >>> With these details, do you still think I need to change the child (channel)
> >>> compatible ?    
> >>
> >> From OS point of view, you have a driver binding to this child-level
> >> compatible. How do you enforce Linux driver binding based on parent
> >> compatible? I looked at your next patch and I did not see it.  
> > 
> > We do not need to have the child driver binding based on parent.  
> 
> Exactly, that's what I said.
> 
> > We have to ensure that the child handles a QMC channel and the parent provides
> > a QMC channel.
> > 
> > A QMC controller (parent) has to implement the QMC API (include/soc/fsl/qe/qmc.h)
> > and a QMC channel driver (child) has to use the QMC API.  
> 
> How does this solve my concerns? Sorry, I do not understand. Your driver
> is a platform driver and binds to the generic compatible. How do you
> solve regular compatibility issues (need for quirks) if parent
> compatible is not used?
> 
> How does being QMC compliant affects driver binding and
> compatibility/quirks?
> 
> We are back to my original question and I don't think you answered to
> any of the concerns.

Well, to be sure that I understand correctly, do you mean that I should
provide a compatible for the child (HDLC) with something like this:
--- 8< ---
  compatible:
    items:
      - enum:
          - fsl,mpc885-qmc-hdlc
          - fsl,mpc866-qmc-hdlc
      - const: fsl,cpm1-qmc-hdlc
      - const: fsl,qmc-hdlc
--- 8< ---

If so, I didn't do that because a QMC channel consumer (driver matching
fsl,qmc-hdlc) doesn't contains any SoC specific part.
It uses the channel as a communication channel to send/receive HDLC frames
to/from this communication channel.
All the specific SoC part is handled by the QMC controller (parent) itself and
not by any consumer (child).

Best regards,
Hervé
  

On 25/09/2023 15:50, Herve Codina wrote:
>>>>> With these details, do you still think I need to change the child (channel)
>>>>> compatible ?    
>>>>
>>>> From OS point of view, you have a driver binding to this child-level
>>>> compatible. How do you enforce Linux driver binding based on parent
>>>> compatible? I looked at your next patch and I did not see it.  
>>>
>>> We do not need to have the child driver binding based on parent.  
>>
>> Exactly, that's what I said.
>>
>>> We have to ensure that the child handles a QMC channel and the parent provides
>>> a QMC channel.
>>>
>>> A QMC controller (parent) has to implement the QMC API (include/soc/fsl/qe/qmc.h)
>>> and a QMC channel driver (child) has to use the QMC API.  
>>
>> How does this solve my concerns? Sorry, I do not understand. Your driver
>> is a platform driver and binds to the generic compatible. How do you
>> solve regular compatibility issues (need for quirks) if parent
>> compatible is not used?
>>
>> How does being QMC compliant affects driver binding and
>> compatibility/quirks?
>>
>> We are back to my original question and I don't think you answered to
>> any of the concerns.
> 
> Well, to be sure that I understand correctly, do you mean that I should
> provide a compatible for the child (HDLC) with something like this:
> --- 8< ---
>   compatible:
>     items:
>       - enum:
>           - fsl,mpc885-qmc-hdlc
>           - fsl,mpc866-qmc-hdlc
>       - const: fsl,cpm1-qmc-hdlc
>       - const: fsl,qmc-hdlc
> --- 8< ---

Yes, more or less, depending on actual compatibility and SoC-family.
Maybe "fsl,cpm1-qmc-hdlc" item in the middle is not needed.

> 
> If so, I didn't do that because a QMC channel consumer (driver matching
> fsl,qmc-hdlc) doesn't contains any SoC specific part.

Just like hundreds of other drivers. :)

There is a paragraph about specific compatibles here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/writing-schema.html


> It uses the channel as a communication channel to send/receive HDLC frames
> to/from this communication channel.
> All the specific SoC part is handled by the QMC controller (parent) itself and
> not by any consumer (child).

OK, so you guarantee in 100% for this hardware and all future (including
designs unknown currently), that they will be 100% compatible with
existing QMC channel consumer (child, matching fsl,qmc-hdlc) driver,
thus there will be no need for any quirk. Specifically, there will be no
chances that it would be reasonable to re-use the same driver for child
(currently fsl,qmc-hdlc) in different parent.

P.S. If you received this email twice, apologies, I have here troubles
with internet.

Best regards,
Krzysztof
  

Hi Krzysztof,

On Tue, 26 Sep 2023 22:59:14 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 25/09/2023 15:50, Herve Codina wrote:
> >>>>> With these details, do you still think I need to change the child (channel)
> >>>>> compatible ?      
> >>>>
> >>>> From OS point of view, you have a driver binding to this child-level
> >>>> compatible. How do you enforce Linux driver binding based on parent
> >>>> compatible? I looked at your next patch and I did not see it.    
> >>>
> >>> We do not need to have the child driver binding based on parent.    
> >>
> >> Exactly, that's what I said.
> >>  
> >>> We have to ensure that the child handles a QMC channel and the parent provides
> >>> a QMC channel.
> >>>
> >>> A QMC controller (parent) has to implement the QMC API (include/soc/fsl/qe/qmc.h)
> >>> and a QMC channel driver (child) has to use the QMC API.    
> >>
> >> How does this solve my concerns? Sorry, I do not understand. Your driver
> >> is a platform driver and binds to the generic compatible. How do you
> >> solve regular compatibility issues (need for quirks) if parent
> >> compatible is not used?
> >>
> >> How does being QMC compliant affects driver binding and
> >> compatibility/quirks?
> >>
> >> We are back to my original question and I don't think you answered to
> >> any of the concerns.  
> > 
> > Well, to be sure that I understand correctly, do you mean that I should
> > provide a compatible for the child (HDLC) with something like this:
> > --- 8< ---
> >   compatible:
> >     items:
> >       - enum:
> >           - fsl,mpc885-qmc-hdlc
> >           - fsl,mpc866-qmc-hdlc
> >       - const: fsl,cpm1-qmc-hdlc
> >       - const: fsl,qmc-hdlc
> > --- 8< ---  
> 
> Yes, more or less, depending on actual compatibility and SoC-family.
> Maybe "fsl,cpm1-qmc-hdlc" item in the middle is not needed.

Ok,
I will keep "fsl,cpm1-qmc-hdlc". The CPM1 is the co-processor present in these
SoCs and it handles the QMC controller. So, it makes sense to have it in this
binding.

I plan to add support for other SoCs in the future and for these SoCs, the
co-processor is not the CPM1. So, it makes sense to keep "fsl,cpm1-qmc-hdlc"
to identify the co-processor.

> 
> > 
> > If so, I didn't do that because a QMC channel consumer (driver matching
> > fsl,qmc-hdlc) doesn't contains any SoC specific part.  
> 
> Just like hundreds of other drivers. :)
> 
> There is a paragraph about specific compatibles here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/writing-schema.html
> 
> 
> > It uses the channel as a communication channel to send/receive HDLC frames
> > to/from this communication channel.
> > All the specific SoC part is handled by the QMC controller (parent) itself and
> > not by any consumer (child).  
> 
> OK, so you guarantee in 100% for this hardware and all future (including
> designs unknown currently), that they will be 100% compatible with
> existing QMC channel consumer (child, matching fsl,qmc-hdlc) driver,
> thus there will be no need for any quirk. Specifically, there will be no
> chances that it would be reasonable to re-use the same driver for child
> (currently fsl,qmc-hdlc) in different parent.

Right,
compatible strings with SoC and co-processor will be added in the next iteration.

Thanks for your feedback.

Best regards,
Hervé

> 
> P.S. If you received this email twice, apologies, I have here troubles
> with internet.
> 
> Best regards,
> Krzysztof
>