mbox series

[v5,00/31] Add support for QMC HDLC, framer infrastructure and PEF2256 framer

Message ID 20230912081527.208499-1-herve.codina@bootlin.com
Headers show
Series Add support for QMC HDLC, framer infrastructure and PEF2256 framer | expand

Message

Herve Codina Sept. 12, 2023, 8:14 a.m. UTC
Hi,

I have a system where I need to handle an HDLC interface and some audio
data.

The HDLC data are transferred using a TDM bus on which a PEF2256
(E1/T1 framer) is present. The PEF2256 transfers data from/to the TDM
bus to/from the E1 line. This PEF2256 is connected to a PowerQUICC SoC
for the control path and the TDM is connected to the SoC (QMC component)
for the data path.

Comments

Linus Walleij Sept. 12, 2023, 11:05 a.m. UTC | #1
On Tue, Sep 12, 2023 at 12:15 PM Herve Codina <herve.codina@bootlin.com> wrote:

> After contributing the driver, add myself as the maintainer for the
> Lantiq PEF2256 driver.
>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
>  MAINTAINERS | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8b987f2c8633..dbc5867016bc 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11876,6 +11876,15 @@ S:     Maintained
>  F:     arch/mips/lantiq
>  F:     drivers/soc/lantiq
>
> +LANTIQ PEF2256 DRIVER
> +M:     Herve Codina <herve.codina@bootlin.com>
> +S:     Maintained
> +F:     Documentation/devicetree/bindings/net/lantiq,pef2256.yaml
> +F:     drivers/net/wan/framer/pef2256/
> +F:     drivers/pinctrl/pinctrl-pef2256-regs.h
> +F:     drivers/pinctrl/pinctrl-pef2256.c

Just use a glob expression:
F:     drivers/pinctrl/pinctrl-pef2256-*

Yours,
Linus Walleij
Mark Brown Sept. 12, 2023, 2:31 p.m. UTC | #2
On Tue, Sep 12, 2023 at 01:04:56PM +0200, Linus Walleij wrote:
> On Tue, Sep 12, 2023 at 12:15 PM Herve Codina <herve.codina@bootlin.com> wrote:

> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*

> I think SPDX mandates that you start the tag with C99 comments

> // SPDX-License-Identifier: GPL-2.0-only

Not for headers, they should use C style since they might be included in
contexts where C++ isn't supported.
Conor Dooley Sept. 12, 2023, 4:36 p.m. UTC | #3
On Tue, Sep 12, 2023 at 10:14:57AM +0200, Herve Codina wrote:
> The given example mentions the 'fsl,mode' property whereas the
> correct property name, the one described, is 'fsl,operational-mode'.
> 
> Fix the example to use the correct property name.
> 
> Fixes: a9b121327c93 ("dt-bindings: soc: fsl: cpm_qe: Add QMC controller")
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

> ---
>  .../bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml           | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> 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 ec888f48cac8..450a0354cb1d 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
> @@ -137,7 +137,7 @@ examples:
>          channel@16 {
>              /* Ch16 : First 4 even TS from all routed from TSA */
>              reg = <16>;
> -            fsl,mode = "transparent";
> +            fsl,operational-mode = "transparent";
>              fsl,reverse-data;
>              fsl,tx-ts-mask = <0x00000000 0x000000aa>;
>              fsl,rx-ts-mask = <0x00000000 0x000000aa>;
> @@ -146,7 +146,7 @@ examples:
>          channel@17 {
>              /* Ch17 : First 4 odd TS from all routed from TSA */
>              reg = <17>;
> -            fsl,mode = "transparent";
> +            fsl,operational-mode = "transparent";
>              fsl,reverse-data;
>              fsl,tx-ts-mask = <0x00000000 0x00000055>;
>              fsl,rx-ts-mask = <0x00000000 0x00000055>;
> @@ -155,7 +155,7 @@ examples:
>          channel@19 {
>              /* Ch19 : 8 TS (TS 8..15) from all routed from TSA */
>              reg = <19>;
> -            fsl,mode = "hdlc";
> +            fsl,operational-mode = "hdlc";
>              fsl,tx-ts-mask = <0x00000000 0x0000ff00>;
>              fsl,rx-ts-mask = <0x00000000 0x0000ff00>;
>          };
> -- 
> 2.41.0
>
Conor Dooley Sept. 12, 2023, 5:21 p.m. UTC | #4
On Tue, Sep 12, 2023 at 12:10:18PM +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>
> ---
>  .../bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml   | 13 +++++++++++++
>  1 file changed, 13 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..b5073531f3f1 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,16 @@ patternProperties:
>            Channel assigned Rx time-slots within the Rx time-slots routed by the
>            TSA to this cell.
>  
> +      compatible:
> +        const: fsl,qmc-hdlc
> +
> +      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.

Sounds like this fsl,framer property should depend on the compatible
being present, no?

Thanks,
Conor.

> +
>      required:
>        - reg
>        - fsl,tx-ts-mask
> @@ -159,5 +169,8 @@ examples:
>              fsl,operational-mode = "hdlc";
>              fsl,tx-ts-mask = <0x00000000 0x0000ff00>;
>              fsl,rx-ts-mask = <0x00000000 0x0000ff00>;
> +
> +            compatible = "fsl,qmc-hdlc";
> +            fsl,framer = <&framer>;
>          };
>      };
> -- 
> 2.41.0
>
Linus Walleij Sept. 12, 2023, 9:04 p.m. UTC | #5
On Tue, Sep 12, 2023 at 4:31 PM Mark Brown <broonie@kernel.org> wrote:
> On Tue, Sep 12, 2023 at 01:04:56PM +0200, Linus Walleij wrote:
> > On Tue, Sep 12, 2023 at 12:15 PM Herve Codina <herve.codina@bootlin.com> wrote:
>
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +/*
>
> > I think SPDX mandates that you start the tag with C99 comments
>
> > // SPDX-License-Identifier: GPL-2.0-only
>
> Not for headers, they should use C style since they might be included in
> contexts where C++ isn't supported.

Oh right. Thanks Mark!

Yours,
Linus Walleij
Herve Codina Sept. 13, 2023, 7:26 a.m. UTC | #6
Hi Conor,

On Tue, 12 Sep 2023 18:21:58 +0100
Conor Dooley <conor@kernel.org> wrote:

> On Tue, Sep 12, 2023 at 12:10:18PM +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>
> > ---
> >  .../bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml   | 13 +++++++++++++
> >  1 file changed, 13 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..b5073531f3f1 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,16 @@ patternProperties:
> >            Channel assigned Rx time-slots within the Rx time-slots routed by the
> >            TSA to this cell.
> >  
> > +      compatible:
> > +        const: fsl,qmc-hdlc
> > +
> > +      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.  
> 
> Sounds like this fsl,framer property should depend on the compatible
> being present, no?

Well from the implementation point of view, only the QMC HDLC driver uses this
property.

From the hardware description point of view, this property means that the time slots
handled by this channel are connected to the framer. So I think it makes sense for
any channel no matter the compatible (even if compatible is not present).

Should I change and constraint the fsl,framer property to the compatible presence ?
If so, is the following correct for this contraint ?
   --- 8< ---
   dependencies:
     - fsl,framer: [ compatible ];
   --- 8< ---

Regards,
Hervé

> 
> Thanks,
> Conor.
> 
> > +
> >      required:
> >        - reg
> >        - fsl,tx-ts-mask
> > @@ -159,5 +169,8 @@ examples:
> >              fsl,operational-mode = "hdlc";
> >              fsl,tx-ts-mask = <0x00000000 0x0000ff00>;
> >              fsl,rx-ts-mask = <0x00000000 0x0000ff00>;
> > +
> > +            compatible = "fsl,qmc-hdlc";
> > +            fsl,framer = <&framer>;
> >          };
> >      };
> > -- 
> > 2.41.0
> >
Conor Dooley Sept. 13, 2023, 2:42 p.m. UTC | #7
On Wed, Sep 13, 2023 at 09:26:40AM +0200, Herve Codina wrote:
> Hi Conor,
> 
> On Tue, 12 Sep 2023 18:21:58 +0100
> Conor Dooley <conor@kernel.org> wrote:
> 
> > On Tue, Sep 12, 2023 at 12:10:18PM +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>
> > > ---
> > >  .../bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml   | 13 +++++++++++++
> > >  1 file changed, 13 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..b5073531f3f1 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,16 @@ patternProperties:
> > >            Channel assigned Rx time-slots within the Rx time-slots routed by the
> > >            TSA to this cell.
> > >  
> > > +      compatible:
> > > +        const: fsl,qmc-hdlc
> > > +
> > > +      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.  
> > 
> > Sounds like this fsl,framer property should depend on the compatible
> > being present, no?
> 
> Well from the implementation point of view, only the QMC HDLC driver uses this
> property.
> 
> From the hardware description point of view, this property means that the time slots
> handled by this channel are connected to the framer. So I think it makes sense for
> any channel no matter the compatible (even if compatible is not present).
> 
> Should I change and constraint the fsl,framer property to the compatible presence ?
> If so, is the following correct for this contraint ?
>    --- 8< ---
>    dependencies:
>      - fsl,framer: [ compatible ];
>    --- 8< ---

The regular sort of
if:
	compatible:
		contains:
			const: foo
then:
	required:
		- fsl,framer
would fit the bill, no?
Herve Codina Sept. 13, 2023, 2:52 p.m. UTC | #8
On Wed, 13 Sep 2023 15:42:45 +0100
Conor Dooley <conor@kernel.org> wrote:

> On Wed, Sep 13, 2023 at 09:26:40AM +0200, Herve Codina wrote:
> > Hi Conor,
> > 
> > On Tue, 12 Sep 2023 18:21:58 +0100
> > Conor Dooley <conor@kernel.org> wrote:
> >   
> > > On Tue, Sep 12, 2023 at 12:10:18PM +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>
> > > > ---
> > > >  .../bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml   | 13 +++++++++++++
> > > >  1 file changed, 13 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..b5073531f3f1 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,16 @@ patternProperties:
> > > >            Channel assigned Rx time-slots within the Rx time-slots routed by the
> > > >            TSA to this cell.
> > > >  
> > > > +      compatible:
> > > > +        const: fsl,qmc-hdlc
> > > > +
> > > > +      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.    
> > > 
> > > Sounds like this fsl,framer property should depend on the compatible
> > > being present, no?  
> > 
> > Well from the implementation point of view, only the QMC HDLC driver uses this
> > property.
> > 
> > From the hardware description point of view, this property means that the time slots
> > handled by this channel are connected to the framer. So I think it makes sense for
> > any channel no matter the compatible (even if compatible is not present).
> > 
> > Should I change and constraint the fsl,framer property to the compatible presence ?
> > If so, is the following correct for this contraint ?
> >    --- 8< ---
> >    dependencies:
> >      - fsl,framer: [ compatible ];
> >    --- 8< ---  
> 
> The regular sort of
> if:
> 	compatible:
> 		contains:
> 			const: foo
> then:
> 	required:
> 		- fsl,framer
> would fit the bill, no?

Not sure.
"fsl,framer" is an optional property (depending on the hardware we can have
a framer or not).

Hervé
Conor Dooley Sept. 13, 2023, 2:56 p.m. UTC | #9
On Wed, Sep 13, 2023 at 04:52:50PM +0200, Herve Codina wrote:
> On Wed, 13 Sep 2023 15:42:45 +0100
> Conor Dooley <conor@kernel.org> wrote:
> 
> > On Wed, Sep 13, 2023 at 09:26:40AM +0200, Herve Codina wrote:
> > > Hi Conor,
> > > 
> > > On Tue, 12 Sep 2023 18:21:58 +0100
> > > Conor Dooley <conor@kernel.org> wrote:
> > >   
> > > > On Tue, Sep 12, 2023 at 12:10:18PM +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>
> > > > > ---
> > > > >  .../bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml   | 13 +++++++++++++
> > > > >  1 file changed, 13 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..b5073531f3f1 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,16 @@ patternProperties:
> > > > >            Channel assigned Rx time-slots within the Rx time-slots routed by the
> > > > >            TSA to this cell.
> > > > >  
> > > > > +      compatible:
> > > > > +        const: fsl,qmc-hdlc
> > > > > +
> > > > > +      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.    
> > > > 
> > > > Sounds like this fsl,framer property should depend on the compatible
> > > > being present, no?  
> > > 
> > > Well from the implementation point of view, only the QMC HDLC driver uses this
> > > property.
> > > 
> > > From the hardware description point of view, this property means that the time slots
> > > handled by this channel are connected to the framer. So I think it makes sense for
> > > any channel no matter the compatible (even if compatible is not present).
> > > 
> > > Should I change and constraint the fsl,framer property to the compatible presence ?
> > > If so, is the following correct for this contraint ?
> > >    --- 8< ---
> > >    dependencies:
> > >      - fsl,framer: [ compatible ];
> > >    --- 8< ---  
> > 
> > The regular sort of
> > if:
> > 	compatible:
> > 		contains:
> > 			const: foo
> > then:
> > 	required:
> > 		- fsl,framer
> > would fit the bill, no?
> 
> Not sure.
> "fsl,framer" is an optional property (depending on the hardware we can have
> a framer or not).

Ah apologies, I had it backwards! Your suggestion seems fair in that
case.

Thanks,
Conor.
Conor Dooley Sept. 13, 2023, 2:59 p.m. UTC | #10
On Wed, Sep 13, 2023 at 03:56:16PM +0100, Conor Dooley wrote:
> On Wed, Sep 13, 2023 at 04:52:50PM +0200, Herve Codina wrote:
> > On Wed, 13 Sep 2023 15:42:45 +0100
> > Conor Dooley <conor@kernel.org> wrote:
> > 
> > > On Wed, Sep 13, 2023 at 09:26:40AM +0200, Herve Codina wrote:
> > > > Hi Conor,
> > > > 
> > > > On Tue, 12 Sep 2023 18:21:58 +0100
> > > > Conor Dooley <conor@kernel.org> wrote:
> > > >   
> > > > > On Tue, Sep 12, 2023 at 12:10:18PM +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>
> > > > > > ---
> > > > > >  .../bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml   | 13 +++++++++++++
> > > > > >  1 file changed, 13 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..b5073531f3f1 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,16 @@ patternProperties:
> > > > > >            Channel assigned Rx time-slots within the Rx time-slots routed by the
> > > > > >            TSA to this cell.
> > > > > >  
> > > > > > +      compatible:
> > > > > > +        const: fsl,qmc-hdlc
> > > > > > +
> > > > > > +      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.    
> > > > > 
> > > > > Sounds like this fsl,framer property should depend on the compatible
> > > > > being present, no?  
> > > > 
> > > > Well from the implementation point of view, only the QMC HDLC driver uses this
> > > > property.
> > > > 
> > > > From the hardware description point of view, this property means that the time slots
> > > > handled by this channel are connected to the framer. So I think it makes sense for
> > > > any channel no matter the compatible (even if compatible is not present).
> > > > 
> > > > Should I change and constraint the fsl,framer property to the compatible presence ?
> > > > If so, is the following correct for this contraint ?
> > > >    --- 8< ---
> > > >    dependencies:
> > > >      - fsl,framer: [ compatible ];
> > > >    --- 8< ---  
> > > 
> > > The regular sort of
> > > if:
> > > 	compatible:
> > > 		contains:
> > > 			const: foo
> > > then:
> > > 	required:
> > > 		- fsl,framer
> > > would fit the bill, no?
> > 
> > Not sure.
> > "fsl,framer" is an optional property (depending on the hardware we can have
> > a framer or not).
> 
> Ah apologies, I had it backwards! Your suggestion seems fair in that
> case.

Or actually,
if:
	compatible:
		not:
	 		contains:
	 			const: foo
 then:
 	properties:
 		fsl,framer: false
? That should do the trick in a more conventional way.
Mark Brown Sept. 13, 2023, 4:59 p.m. UTC | #11
On Tue, Sep 12, 2023 at 12:14:36PM +0200, Herve Codina wrote:
> A framer is a component in charge of an E1/T1 line interface.
> Connected usually to a TDM bus, it converts TDM frames to/from E1/T1
> frames. It also provides information related to the E1/T1 line.
> 
> The framer framework provides a set of APIs for the framer drivers
> (framer provider) to create/destroy a framer and APIs for the framer
> users (framer consumer) to obtain a reference to the framer, and
> use the framer.

If people are fine with this could we perhaps get it applied on a branch
with a tag?  That way we could cut down the size of the series a little
and I could apply the generic ASoC bit too, neither of the two patches
have any dependency on the actual hardware.
Herve Codina Sept. 18, 2023, 7:42 a.m. UTC | #12
Hi Conor,

On Wed, 13 Sep 2023 15:59:41 +0100
Conor Dooley <conor@kernel.org> wrote:

> On Wed, Sep 13, 2023 at 03:56:16PM +0100, Conor Dooley wrote:
> > On Wed, Sep 13, 2023 at 04:52:50PM +0200, Herve Codina wrote:  
> > > On Wed, 13 Sep 2023 15:42:45 +0100
> > > Conor Dooley <conor@kernel.org> wrote:
> > >   
> > > > On Wed, Sep 13, 2023 at 09:26:40AM +0200, Herve Codina wrote:  
> > > > > Hi Conor,
> > > > > 
> > > > > On Tue, 12 Sep 2023 18:21:58 +0100
> > > > > Conor Dooley <conor@kernel.org> wrote:
> > > > >     
> > > > > > On Tue, Sep 12, 2023 at 12:10:18PM +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>
> > > > > > > ---
> > > > > > >  .../bindings/soc/fsl/cpm_qe/fsl,cpm1-scc-qmc.yaml   | 13 +++++++++++++
> > > > > > >  1 file changed, 13 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..b5073531f3f1 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,16 @@ patternProperties:
> > > > > > >            Channel assigned Rx time-slots within the Rx time-slots routed by the
> > > > > > >            TSA to this cell.
> > > > > > >  
> > > > > > > +      compatible:
> > > > > > > +        const: fsl,qmc-hdlc
> > > > > > > +
> > > > > > > +      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.      
> > > > > > 
> > > > > > Sounds like this fsl,framer property should depend on the compatible
> > > > > > being present, no?    
> > > > > 
> > > > > Well from the implementation point of view, only the QMC HDLC driver uses this
> > > > > property.
> > > > > 
> > > > > From the hardware description point of view, this property means that the time slots
> > > > > handled by this channel are connected to the framer. So I think it makes sense for
> > > > > any channel no matter the compatible (even if compatible is not present).
> > > > > 
> > > > > Should I change and constraint the fsl,framer property to the compatible presence ?
> > > > > If so, is the following correct for this contraint ?
> > > > >    --- 8< ---
> > > > >    dependencies:
> > > > >      - fsl,framer: [ compatible ];
> > > > >    --- 8< ---    
> > > > 
> > > > The regular sort of
> > > > if:
> > > > 	compatible:
> > > > 		contains:
> > > > 			const: foo
> > > > then:
> > > > 	required:
> > > > 		- fsl,framer
> > > > would fit the bill, no?  
> > > 
> > > Not sure.
> > > "fsl,framer" is an optional property (depending on the hardware we can have
> > > a framer or not).  
> > 
> > Ah apologies, I had it backwards! Your suggestion seems fair in that
> > case.  
> 
> Or actually,
> if:
> 	compatible:
> 		not:
> 	 		contains:
> 	 			const: foo
>  then:
>  	properties:
>  		fsl,framer: false
> ? That should do the trick in a more conventional way.

Thanks for this proposal.
I will use it in the next iteration.

Regards,
Hervé