[PATCH 0/6] mfd: simple-mfd-i2c: add QIXIS CPLD support

[PATCH 0/6] mfd: simple-mfd-i2c: add QIXIS CPLD support

[Ioana Ciornei]

The MDIO mux on the LX2160AQDS, LX2162AQDS and LS1028AQDS boards never
worked in mainline. The DT files were submitted initially as-is, and
there is a downstream driver for the QIXIS CPLD device:
https://github.com/nxp-qoriq/linux/blob/lf-6.12.y/drivers/soc/fsl/qixis_ctrl.c

Since the HW works with the already existing
driver/mfd/similar-mfd-i2c.c driver, extend the list of compatible
strings to also cover these 3 new boards, instead of trying to upstream
a duplicate driver.

This patch set also adapts the DT nodes for each of the affected boards
so that we match on the new compatible strings.

The last patch describes the two on-board RGMII PHYs found on the
LX2160AQDS boards which make use of the MDIO bus found behind the CPLD
driven MDIO mux.

Ioana Ciornei (5):
  dt-bindings: mfd: add bindings for QIXIS CPLD
  mfd: simple-mfd-i2c: add compatible string for Layerscape QIXIS CPLD
  arm64: dts: lx2160a-qds: make the QIXIS CPLD use the simple-mfd-i2c.c
    driver
  arm64: dts: lx2162a-qds: make the QIXIS CPLD use the simple-mfd-i2c.c
    driver
  arm64: dts: lx2160a-qds: add the two on-board RGMII PHYs

Vladimir Oltean (1):
  arm64: dts: ls1028a-qds: make the QIXIS CPLD use the simple-mfd-i2c.c
    driver

 .../bindings/mfd/fsl,qixis-i2c.yaml           | 65 +++++++++++++++++++
 .../boot/dts/freescale/fsl-ls1028a-qds.dts    |  9 +--
 .../boot/dts/freescale/fsl-lx2160a-qds.dts    | 28 +++++++-
 .../boot/dts/freescale/fsl-lx2162a-qds.dts    |  8 ++-
 drivers/mfd/simple-mfd-i2c.c                  |  3 +
 5 files changed, 103 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml

-- 
2.25.1

[PATCH 1/6] dt-bindings: mfd: add bindings for QIXIS CPLD

[Ioana Ciornei]

This adds device tree bindings for the board management controller -
QIXIS CPLD - found on some Layerscape based boards such as LX2160A-RDB,
LX2160AQDS, LS1028AQDS etc.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 .../bindings/mfd/fsl,qixis-i2c.yaml           | 65 +++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml

diff --git a/Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml b/Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml
new file mode 100644
index 000000000000..562878050916
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/fsl,qixis-i2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP's QIXIS CPLD board management controller
+
+maintainers:
+  - Ioana Ciornei <ioana.ciornei@nxp.com>
+
+description: |
+  The board management controller found on some Layerscape boards contains
+  different IP blocks like GPIO controllers, interrupt controllers, reg-muxes
+  etc.
+
+properties:
+  compatible:
+    enum:
+      - fsl,lx2160a-qds-qixis-i2c
+      - fsl,lx2162a-qds-qixis-i2c
+      - fsl,ls1028a-qds-qixis-i2c
+
+  reg:
+    description:
+      I2C device address.
+    maxItems: 1
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  mux-controller:
+    $ref: /schemas/mux/reg-mux.yaml#
+
+required:
+  - "#address-cells"
+  - "#size-cells"
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        qixis@66 {
+            compatible = "fsl,lx2160a-qds-qixis-i2c";
+            reg = <0x66>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            mux: mux-controller {
+                compatible = "reg-mux";
+                #mux-control-cells = <1>;
+                mux-reg-masks = <0x54 0xf8>, /* 0: reg 0x54, bits 7:3 */
+                                <0x54 0x07>; /* 1: reg 0x54, bit 2:0 */
+            };
+        };
+    };
-- 
2.25.1

Re: [PATCH 1/6] dt-bindings: mfd: add bindings for QIXIS CPLD

[Krzysztof Kozlowski]

On Wed, Apr 30, 2025 at 06:36:29PM GMT, Ioana Ciornei wrote:
> This adds device tree bindings for the board management controller -
> QIXIS CPLD - found on some Layerscape based boards such as LX2160A-RDB,
> LX2160AQDS, LS1028AQDS etc.
> 
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
>  .../bindings/mfd/fsl,qixis-i2c.yaml           | 65 +++++++++++++++++++
>  1 file changed, 65 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml b/Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml
> new file mode 100644
> index 000000000000..562878050916
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml

Filename matching compatible.

> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/fsl,qixis-i2c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP's QIXIS CPLD board management controller
> +
> +maintainers:
> +  - Ioana Ciornei <ioana.ciornei@nxp.com>
> +
> +description: |
> +  The board management controller found on some Layerscape boards contains
> +  different IP blocks like GPIO controllers, interrupt controllers, reg-muxes
> +  etc.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - fsl,lx2160a-qds-qixis-i2c
> +      - fsl,lx2162a-qds-qixis-i2c
> +      - fsl,ls1028a-qds-qixis-i2c

Keep alphabetical order.

What is actual device name? I2C? Is this an I2C controller or device?

> +
> +  reg:
> +    description:
> +      I2C device address.

This says device, so i2c in compatible is wrong.

Anyway drop description, redundant.


> +    maxItems: 1
> +
> +  "#address-cells":
> +    const: 1

Why?

> +
> +  "#size-cells":
> +    const: 0

Why? Drop cells.

> +
> +  mux-controller:
> +    $ref: /schemas/mux/reg-mux.yaml#
> +
> +required:
> +  - "#address-cells"
> +  - "#size-cells"
> +  - compatible
> +  - reg

Keep same order as in properties

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        qixis@66 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

> +            compatible = "fsl,lx2160a-qds-qixis-i2c";
> +            reg = <0x66>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;

So were do you use address/size cells?

Best regards,
Krzysztof

Re: [PATCH 1/6] dt-bindings: mfd: add bindings for QIXIS CPLD

[Ioana Ciornei]

On Fri, May 02, 2025 at 09:01:59AM +0200, Krzysztof Kozlowski wrote:
> On Wed, Apr 30, 2025 at 06:36:29PM GMT, Ioana Ciornei wrote:
> > This adds device tree bindings for the board management controller -
> > QIXIS CPLD - found on some Layerscape based boards such as LX2160A-RDB,
> > LX2160AQDS, LS1028AQDS etc.
> > 
> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > ---
> >  .../bindings/mfd/fsl,qixis-i2c.yaml           | 65 +++++++++++++++++++
> >  1 file changed, 65 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml b/Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml
> > new file mode 100644
> > index 000000000000..562878050916
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml
> 
> Filename matching compatible.

How to choose one if there are multiple compatible strings?

> 
> > @@ -0,0 +1,65 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mfd/fsl,qixis-i2c.yaml
> > +$schema: http://devicetree.org/meta-schemas/core.yaml
> > +
> > +title: NXP's QIXIS CPLD board management controller
> > +
> > +maintainers:
> > +  - Ioana Ciornei <ioana.ciornei@nxp.com>
> > +
> > +description: |
> > +  The board management controller found on some Layerscape boards contains
> > +  different IP blocks like GPIO controllers, interrupt controllers, reg-muxes
> > +  etc.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - fsl,lx2160a-qds-qixis-i2c
> > +      - fsl,lx2162a-qds-qixis-i2c
> > +      - fsl,ls1028a-qds-qixis-i2c
> 
> Keep alphabetical order.
> 
> What is actual device name? I2C? Is this an I2C controller or device?
> 
> > +
> > +  reg:
> > +    description:
> > +      I2C device address.
> 
> This says device, so i2c in compatible is wrong.
> 
> Anyway drop description, redundant.

Ok, will drop.

> 
> 
> > +    maxItems: 1
> > +
> > +  "#address-cells":
> > +    const: 1
> 
> Why?
> 
> > +
> > +  "#size-cells":
> > +    const: 0
> 
> Why? Drop cells.
> 

See below.

> > +
> > +  mux-controller:
> > +    $ref: /schemas/mux/reg-mux.yaml#
> > +
> > +required:
> > +  - "#address-cells"
> > +  - "#size-cells"
> > +  - compatible
> > +  - reg
> 
> Keep same order as in properties

Ok.

> 
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        qixis@66 {
> 
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

In this case, an accepted node name is 'cpld'?

> 
> > +            compatible = "fsl,lx2160a-qds-qixis-i2c";
> > +            reg = <0x66>;
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> 
> So were do you use address/size cells?
> 

For example, fsl-ls1028a-qds.dts looks like this:

	fpga@66 {
		compatible = "fsl,ls1028a-qds-qixis-i2c";
		reg = <0x66>;
		#address-cells = <1>;
		#size-cells = <0>;

		mux: mux-controller@54 {
			compatible = "reg-mux";
			reg = <0x54>;
			#mux-control-cells = <1>;
			mux-reg-masks = <0x54 0xf0>; /* 0: reg 0x54, bits 7:4 */
		};
	};

Also, some boards have in their qixis CPLD gpio controllers and I am
planning to add them as the next step.

Ioana

Re: [PATCH 1/6] dt-bindings: mfd: add bindings for QIXIS CPLD

[Krzysztof Kozlowski]

On 06/05/2025 15:57, Ioana Ciornei wrote:
> On Fri, May 02, 2025 at 09:01:59AM +0200, Krzysztof Kozlowski wrote:
>> On Wed, Apr 30, 2025 at 06:36:29PM GMT, Ioana Ciornei wrote:
>>> This adds device tree bindings for the board management controller -
>>> QIXIS CPLD - found on some Layerscape based boards such as LX2160A-RDB,
>>> LX2160AQDS, LS1028AQDS etc.
>>>
>>> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
>>> ---
>>>  .../bindings/mfd/fsl,qixis-i2c.yaml           | 65 +++++++++++++++++++
>>>  1 file changed, 65 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml b/Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml
>>> new file mode 100644
>>> index 000000000000..562878050916
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml
>>
>> Filename matching compatible.
> 
> How to choose one if there are multiple compatible strings?

The fallback or the oldest or the lowest number or whichever you prefer
as a base.

> 
>>
>>> @@ -0,0 +1,65 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/mfd/fsl,qixis-i2c.yaml
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml
>>> +
>>> +title: NXP's QIXIS CPLD board management controller
>>> +
>>> +maintainers:
>>> +  - Ioana Ciornei <ioana.ciornei@nxp.com>
>>> +
>>> +description: |
>>> +  The board management controller found on some Layerscape boards contains
>>> +  different IP blocks like GPIO controllers, interrupt controllers, reg-muxes
>>> +  etc.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - fsl,lx2160a-qds-qixis-i2c
>>> +      - fsl,lx2162a-qds-qixis-i2c
>>> +      - fsl,ls1028a-qds-qixis-i2c
>>
>> Keep alphabetical order.
>>
>> What is actual device name? I2C? Is this an I2C controller or device?

I assume you will then drop the redundant part.

>>
>>> +
>>> +  reg:
>>> +    description:
>>> +      I2C device address.
>>
>> This says device, so i2c in compatible is wrong.
>>
>> Anyway drop description, redundant.
> 
> Ok, will drop.
> 
>>
>>
>>> +    maxItems: 1
>>> +
>>> +  "#address-cells":
>>> +    const: 1
>>
>> Why?
>>
>>> +
>>> +  "#size-cells":
>>> +    const: 0
>>
>> Why? Drop cells.
>>
> 
> See below.
> 
>>> +
>>> +  mux-controller:
>>> +    $ref: /schemas/mux/reg-mux.yaml#
>>> +
>>> +required:
>>> +  - "#address-cells"
>>> +  - "#size-cells"
>>> +  - compatible
>>> +  - reg
>>
>> Keep same order as in properties
> 
> Ok.
> 
>>
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    i2c {
>>> +        #address-cells = <1>;
>>> +        #size-cells = <0>;
>>> +
>>> +        qixis@66 {
>>
>> Node names should be generic. See also an explanation and list of
>> examples (not exhaustive) in DT specification:
>> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> 
> In this case, an accepted node name is 'cpld'?

If this is CPLD then yes.

> 
>>
>>> +            compatible = "fsl,lx2160a-qds-qixis-i2c";
>>> +            reg = <0x66>;
>>> +            #address-cells = <1>;
>>> +            #size-cells = <0>;
>>
>> So were do you use address/size cells?
>>
> 
> For example, fsl-ls1028a-qds.dts looks like this:
> 
> 	fpga@66 {
> 		compatible = "fsl,ls1028a-qds-qixis-i2c";
> 		reg = <0x66>;
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		mux: mux-controller@54 {
> 			compatible = "reg-mux";
> 			reg = <0x54>;
> 			#mux-control-cells = <1>;
> 			mux-reg-masks = <0x54 0xf0>; /* 0: reg 0x54, bits 7:4 */
> 		};
> 	};
> 
> Also, some boards have in their qixis CPLD gpio controllers and I am
> planning to add them as the next step.

And if you tested that DTS you would see that binding does not work
well... so my arguments stay valid - these properties in current binding
make no sense. However binding is just wrong, so maybe these properties
make sense after fixing the binding but then in both cases: current
stage is not correct.

> 
> Ioana


Best regards,
Krzysztof

Re: [PATCH 1/6] dt-bindings: mfd: add bindings for QIXIS CPLD

[Ioana Ciornei]

On Tue, May 06, 2025 at 04:05:46PM +0200, Krzysztof Kozlowski wrote:
> On 06/05/2025 15:57, Ioana Ciornei wrote:
> > On Fri, May 02, 2025 at 09:01:59AM +0200, Krzysztof Kozlowski wrote:
> >> On Wed, Apr 30, 2025 at 06:36:29PM GMT, Ioana Ciornei wrote:
> >>> This adds device tree bindings for the board management controller -
> >>> QIXIS CPLD - found on some Layerscape based boards such as LX2160A-RDB,
> >>> LX2160AQDS, LS1028AQDS etc.
> >>>
> >>> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> >>> ---
> >>>  .../bindings/mfd/fsl,qixis-i2c.yaml           | 65 +++++++++++++++++++
> >>>  1 file changed, 65 insertions(+)
> >>>  create mode 100644 Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml b/Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml
> >>> new file mode 100644
> >>> index 000000000000..562878050916
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml
> >>
> >> Filename matching compatible.
> > 
> > How to choose one if there are multiple compatible strings?
> 
> The fallback or the oldest or the lowest number or whichever you prefer
> as a base.
> 
> > 
> >>
> >>> @@ -0,0 +1,65 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/mfd/fsl,qixis-i2c.yaml
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml
> >>> +
> >>> +title: NXP's QIXIS CPLD board management controller
> >>> +
> >>> +maintainers:
> >>> +  - Ioana Ciornei <ioana.ciornei@nxp.com>
> >>> +
> >>> +description: |
> >>> +  The board management controller found on some Layerscape boards contains
> >>> +  different IP blocks like GPIO controllers, interrupt controllers, reg-muxes
> >>> +  etc.
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    enum:
> >>> +      - fsl,lx2160a-qds-qixis-i2c
> >>> +      - fsl,lx2162a-qds-qixis-i2c
> >>> +      - fsl,ls1028a-qds-qixis-i2c
> >>
> >> Keep alphabetical order.
> >>
> >> What is actual device name? I2C? Is this an I2C controller or device?
> 
> I assume you will then drop the redundant part.

Ok, I will drop the i2c part. Are you ok with the below compatible
strings?

	- fsl,lx2160a-qds-qixis-cpld
	- fsl,lx2162a-qds-qixis-cpld
	- fsl,ls1028a-qds-qixis-cpld

> 
> >>
> >>> +
> >>> +  reg:
> >>> +    description:
> >>> +      I2C device address.
> >>
> >> This says device, so i2c in compatible is wrong.
> >>
> >> Anyway drop description, redundant.
> > 
> > Ok, will drop.
> > 
> >>
> >>
> >>> +    maxItems: 1
> >>> +
> >>> +  "#address-cells":
> >>> +    const: 1
> >>
> >> Why?
> >>
> >>> +
> >>> +  "#size-cells":
> >>> +    const: 0
> >>
> >> Why? Drop cells.
> >>
> > 
> > See below.
> > 
> >>> +
> >>> +  mux-controller:
> >>> +    $ref: /schemas/mux/reg-mux.yaml#
> >>> +
> >>> +required:
> >>> +  - "#address-cells"
> >>> +  - "#size-cells"
> >>> +  - compatible
> >>> +  - reg
> >>
> >> Keep same order as in properties
> > 
> > Ok.
> > 
> >>
> >>> +
> >>> +additionalProperties: false
> >>> +
> >>> +examples:
> >>> +  - |
> >>> +    i2c {
> >>> +        #address-cells = <1>;
> >>> +        #size-cells = <0>;
> >>> +
> >>> +        qixis@66 {
> >>
> >> Node names should be generic. See also an explanation and list of
> >> examples (not exhaustive) in DT specification:
> >> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> > 
> > In this case, an accepted node name is 'cpld'?
> 
> If this is CPLD then yes.

Yes, it is. Thanks for the confirmation.

> 
> > 
> >>
> >>> +            compatible = "fsl,lx2160a-qds-qixis-i2c";
> >>> +            reg = <0x66>;
> >>> +            #address-cells = <1>;
> >>> +            #size-cells = <0>;
> >>
> >> So were do you use address/size cells?
> >>
> > 
> > For example, fsl-ls1028a-qds.dts looks like this:
> > 
> > 	fpga@66 {
> > 		compatible = "fsl,ls1028a-qds-qixis-i2c";
> > 		reg = <0x66>;
> > 		#address-cells = <1>;
> > 		#size-cells = <0>;
> > 
> > 		mux: mux-controller@54 {
> > 			compatible = "reg-mux";
> > 			reg = <0x54>;
> > 			#mux-control-cells = <1>;
> > 			mux-reg-masks = <0x54 0xf0>; /* 0: reg 0x54, bits 7:4 */
> > 		};
> > 	};
> > 
> > Also, some boards have in their qixis CPLD gpio controllers and I am
> > planning to add them as the next step.
> 
> And if you tested that DTS you would see that binding does not work
> well... so my arguments stay valid - these properties in current binding
> make no sense. However binding is just wrong, so maybe these properties
> make sense after fixing the binding but then in both cases: current
> stage is not correct.
> 

Yes, I agree that the current binding file is wrong. I even said this in
one of my ealier replies to the bot which found the new DTB warnings.
I will fix it in v2.

Ioana

Re: [PATCH 1/6] dt-bindings: mfd: add bindings for QIXIS CPLD

[Krzysztof Kozlowski]

On 06/05/2025 16:16, Ioana Ciornei wrote:
> On Tue, May 06, 2025 at 04:05:46PM +0200, Krzysztof Kozlowski wrote:
>> On 06/05/2025 15:57, Ioana Ciornei wrote:
>>> On Fri, May 02, 2025 at 09:01:59AM +0200, Krzysztof Kozlowski wrote:
>>>> On Wed, Apr 30, 2025 at 06:36:29PM GMT, Ioana Ciornei wrote:
>>>>> This adds device tree bindings for the board management controller -
>>>>> QIXIS CPLD - found on some Layerscape based boards such as LX2160A-RDB,
>>>>> LX2160AQDS, LS1028AQDS etc.
>>>>>
>>>>> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
>>>>> ---
>>>>>  .../bindings/mfd/fsl,qixis-i2c.yaml           | 65 +++++++++++++++++++
>>>>>  1 file changed, 65 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml b/Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..562878050916
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml
>>>>
>>>> Filename matching compatible.
>>>
>>> How to choose one if there are multiple compatible strings?
>>
>> The fallback or the oldest or the lowest number or whichever you prefer
>> as a base.
>>
>>>
>>>>
>>>>> @@ -0,0 +1,65 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/mfd/fsl,qixis-i2c.yaml
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml
>>>>> +
>>>>> +title: NXP's QIXIS CPLD board management controller
>>>>> +
>>>>> +maintainers:
>>>>> +  - Ioana Ciornei <ioana.ciornei@nxp.com>
>>>>> +
>>>>> +description: |
>>>>> +  The board management controller found on some Layerscape boards contains
>>>>> +  different IP blocks like GPIO controllers, interrupt controllers, reg-muxes
>>>>> +  etc.
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    enum:
>>>>> +      - fsl,lx2160a-qds-qixis-i2c
>>>>> +      - fsl,lx2162a-qds-qixis-i2c
>>>>> +      - fsl,ls1028a-qds-qixis-i2c
>>>>
>>>> Keep alphabetical order.
>>>>
>>>> What is actual device name? I2C? Is this an I2C controller or device?
>>
>> I assume you will then drop the redundant part.
> 
> Ok, I will drop the i2c part. Are you ok with the below compatible
> strings?
> 
> 	- fsl,lx2160a-qds-qixis-cpld
> 	- fsl,lx2162a-qds-qixis-cpld
> 	- fsl,ls1028a-qds-qixis-cpld


Not really, because you keep ignoring comments. What is the device name?
Can qixis be anything else than i2c or cpld? Use that name for the
compatible.


Best regards,
Krzysztof

[PATCH 2/6] mfd: simple-mfd-i2c: add compatible string for Layerscape QIXIS CPLD

[Ioana Ciornei]

The QIXIS CPLD found on Layerscape boards such as LX2160AQDS, LS1028AQDS
etc deals with power-on-reset timing, muxing etc. Use the simple-mfd-i2c
as its core driver by adding an individual compatible string for each
board.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/mfd/simple-mfd-i2c.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
index 22159913bea0..506b69858a84 100644
--- a/drivers/mfd/simple-mfd-i2c.c
+++ b/drivers/mfd/simple-mfd-i2c.c
@@ -99,6 +99,9 @@ static const struct of_device_id simple_mfd_i2c_of_match[] = {
 	{ .compatible = "maxim,max5970", .data = &maxim_max5970},
 	{ .compatible = "maxim,max5978", .data = &maxim_max5970},
 	{ .compatible = "maxim,max77705-battery", .data = &maxim_mon_max77705},
+	{ .compatible = "fsl,lx2160a-qds-qixis-i2c" },
+	{ .compatible = "fsl,lx2162a-qds-qixis-i2c" },
+	{ .compatible = "fsl,ls1028a-qds-qixis-i2c" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match);
-- 
2.25.1

[PATCH 3/6] arm64: dts: lx2160a-qds: make the QIXIS CPLD use the simple-mfd-i2c.c driver

[Ioana Ciornei]

The MDIO mux on the LX2160A-QDS never worked in mainline. There is a
downstream driver for the QIXIS FPGA which is very similar to the
already existing drivers/mfd/simple-mfd-i2c. Since the HW works with
simple-mfd-i2c.c there is no point in upstreaming the other one.

Adapt the compatible string and the child node of the FPGA node, so that
the simple-mfd-i2c.c driver accepts it.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
index 4d721197d837..d1db38d6a027 100644
--- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
@@ -240,12 +240,14 @@ &i2c0 {
 	status = "okay";
 
 	fpga@66 {
-		compatible = "fsl,lx2160aqds-fpga", "fsl,fpga-qixis-i2c",
-			     "simple-mfd";
+		compatible = "fsl,lx2160a-qds-qixis-i2c";
 		reg = <0x66>;
+		#address-cells = <1>;
+		#size-cells = <0>;
 
-		mux: mux-controller {
+		mux: mux-controller@54 {
 			compatible = "reg-mux";
+			reg = <0x54>;
 			#mux-control-cells = <1>;
 			mux-reg-masks = <0x54 0xf8>, /* 0: reg 0x54, bits 7:3 */
 					<0x54 0x07>; /* 1: reg 0x54, bit 2:0 */
-- 
2.25.1

[PATCH 4/6] arm64: dts: lx2162a-qds: make the QIXIS CPLD use the simple-mfd-i2c.c driver

[Ioana Ciornei]

The MDIO mux on the LX2162A-QDS never worked in mainline. There is a
downstream driver for the QIXIS FPGA which is very similar to the
already existing drivers/mfd/simple-mfd-i2c. Since the HW works with
simple-mfd-i2c.c there is no point in upstreaming the other one.

Adapt the compatible string and the child node of the FPGA node, so that
the simple-mfd-i2c.c driver accepts it.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 arch/arm64/boot/dts/freescale/fsl-lx2162a-qds.dts | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2162a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-lx2162a-qds.dts
index 9f5ff1ffe7d5..53a88e0b54ff 100644
--- a/arch/arm64/boot/dts/freescale/fsl-lx2162a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-lx2162a-qds.dts
@@ -267,12 +267,14 @@ &i2c0 {
 	status = "okay";
 
 	fpga@66 {
-		compatible = "fsl,lx2160aqds-fpga", "fsl,fpga-qixis-i2c",
-			     "simple-mfd";
+		compatible = "fsl,lx2162a-qds-qixis-i2c";
 		reg = <0x66>;
+		#address-cells = <1>;
+		#size-cells = <0>;
 
-		mux: mux-controller {
+		mux: mux-controller@54 {
 			compatible = "reg-mux";
+			reg = <0x54>;
 			#mux-control-cells = <1>;
 			mux-reg-masks = <0x54 0xf8>, /* 0: reg 0x54, bits 7:3 */
 					<0x54 0x07>; /* 1: reg 0x54, bit 2:0 */
-- 
2.25.1

[PATCH 5/6] arm64: dts: ls1028a-qds: make the QIXIS CPLD use the simple-mfd-i2c.c driver

[Ioana Ciornei]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

The MDIO mux on the LS1028A-QDS never worked in mainline. The device
tree was submitted as-is, and there is a downstream driver for the QIXIS
FPGA:

https://github.com/nxp-qoriq/linux/blob/lf-6.12.y/drivers/soc/fsl/qixis_ctrl.c

That driver is very similar to the already existing drivers/mfd/simple-mfd-i2c.c,
and the hardware works with the simple-mfd-i2c driver, so there isn't
any reason to upstream the other one.

Adapt the compatible string and child node format of the FPGA node, so
that the simple-mfd-i2c driver accepts it.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
index 0bb2f28a0441..58b54d521d75 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
@@ -338,17 +338,18 @@ sgtl5000: audio-codec@a {
 	};
 
 	fpga@66 {
-		compatible = "fsl,ls1028aqds-fpga", "fsl,fpga-qixis-i2c",
-			     "simple-mfd";
+		compatible = "fsl,ls1028a-qds-qixis-i2c";
 		reg = <0x66>;
+		#address-cells = <1>;
+		#size-cells = <0>;
 
-		mux: mux-controller {
+		mux: mux-controller@54 {
 			compatible = "reg-mux";
+			reg = <0x54>;
 			#mux-control-cells = <1>;
 			mux-reg-masks = <0x54 0xf0>; /* 0: reg 0x54, bits 7:4 */
 		};
 	};
-
 };
 
 &i2c1 {
-- 
2.25.1

Re: [PATCH 5/6] arm64: dts: ls1028a-qds: make the QIXIS CPLD use the simple-mfd-i2c.c driver

[Krzysztof Kozlowski]

On Wed, Apr 30, 2025 at 06:36:33PM GMT, Ioana Ciornei wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> The MDIO mux on the LS1028A-QDS never worked in mainline. The device
> tree was submitted as-is, and there is a downstream driver for the QIXIS
> FPGA:
> 
> https://github.com/nxp-qoriq/linux/blob/lf-6.12.y/drivers/soc/fsl/qixis_ctrl.c
> 
> That driver is very similar to the already existing drivers/mfd/simple-mfd-i2c.c,
> and the hardware works with the simple-mfd-i2c driver, so there isn't
> any reason to upstream the other one.
> 
> Adapt the compatible string and child node format of the FPGA node, so
> that the simple-mfd-i2c driver accepts it.

Why do you break the users based on some driver differences? Fix the
drivers, not the DTS.

> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
> index 0bb2f28a0441..58b54d521d75 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
> @@ -338,17 +338,18 @@ sgtl5000: audio-codec@a {
>  	};
>  
>  	fpga@66 {
> -		compatible = "fsl,ls1028aqds-fpga", "fsl,fpga-qixis-i2c",
> -			     "simple-mfd";
> +		compatible = "fsl,ls1028a-qds-qixis-i2c";

This breaks all the existing users. NAK.

>  		reg = <0x66>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
>  
> -		mux: mux-controller {
> +		mux: mux-controller@54 {

This was never tested. Your binding says something else.

>  			compatible = "reg-mux";
> +			reg = <0x54>;
>  			#mux-control-cells = <1>;
>  			mux-reg-masks = <0x54 0xf0>; /* 0: reg 0x54, bits 7:4 */
>  		};
>  	};
> -
>  };
>  
>  &i2c1 {
> -- 
> 2.25.1
> 

Re: [PATCH 5/6] arm64: dts: ls1028a-qds: make the QIXIS CPLD use the simple-mfd-i2c.c driver

[Ioana Ciornei]

On Fri, May 02, 2025 at 09:04:03AM +0200, Krzysztof Kozlowski wrote:
> On Wed, Apr 30, 2025 at 06:36:33PM GMT, Ioana Ciornei wrote:
> > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> > 
> > The MDIO mux on the LS1028A-QDS never worked in mainline. The device
> > tree was submitted as-is, and there is a downstream driver for the QIXIS
> > FPGA:
> > 
> > https://github.com/nxp-qoriq/linux/blob/lf-6.12.y/drivers/soc/fsl/qixis_ctrl.c
> > 
> > That driver is very similar to the already existing drivers/mfd/simple-mfd-i2c.c,
> > and the hardware works with the simple-mfd-i2c driver, so there isn't
> > any reason to upstream the other one.
> > 
> > Adapt the compatible string and child node format of the FPGA node, so
> > that the simple-mfd-i2c driver accepts it.
> 
> Why do you break the users based on some driver differences? Fix the
> drivers, not the DTS.
> 
> > 
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > ---
> >  arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
> > index 0bb2f28a0441..58b54d521d75 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
> > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
> > @@ -338,17 +338,18 @@ sgtl5000: audio-codec@a {
> >  	};
> >  
> >  	fpga@66 {
> > -		compatible = "fsl,ls1028aqds-fpga", "fsl,fpga-qixis-i2c",
> > -			     "simple-mfd";
> > +		compatible = "fsl,ls1028a-qds-qixis-i2c";
> 
> This breaks all the existing users. NAK.

Using a mainline kernel, this DT node was never used or probed by a
driver since that driver was never submitted. I am not breaking any user
of the mainline kernel.

> 
> >  		reg = <0x66>;
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> >  
> > -		mux: mux-controller {
> > +		mux: mux-controller@54 {
> 
> This was never tested. Your binding says something else.

Will fix the binding in v2.

Ioana

Re: [PATCH 5/6] arm64: dts: ls1028a-qds: make the QIXIS CPLD use the simple-mfd-i2c.c driver

[Krzysztof Kozlowski]

On 06/05/2025 16:21, Ioana Ciornei wrote:
> On Fri, May 02, 2025 at 09:04:03AM +0200, Krzysztof Kozlowski wrote:
>> On Wed, Apr 30, 2025 at 06:36:33PM GMT, Ioana Ciornei wrote:
>>> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>>>
>>> The MDIO mux on the LS1028A-QDS never worked in mainline. The device
>>> tree was submitted as-is, and there is a downstream driver for the QIXIS
>>> FPGA:
>>>
>>> https://github.com/nxp-qoriq/linux/blob/lf-6.12.y/drivers/soc/fsl/qixis_ctrl.c
>>>
>>> That driver is very similar to the already existing drivers/mfd/simple-mfd-i2c.c,
>>> and the hardware works with the simple-mfd-i2c driver, so there isn't
>>> any reason to upstream the other one.
>>>
>>> Adapt the compatible string and child node format of the FPGA node, so
>>> that the simple-mfd-i2c driver accepts it.
>>
>> Why do you break the users based on some driver differences? Fix the
>> drivers, not the DTS.
>>
>>>
>>> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>>> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
>>> ---
>>>  arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts | 9 +++++----
>>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
>>> index 0bb2f28a0441..58b54d521d75 100644
>>> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
>>> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
>>> @@ -338,17 +338,18 @@ sgtl5000: audio-codec@a {
>>>  	};
>>>  
>>>  	fpga@66 {
>>> -		compatible = "fsl,ls1028aqds-fpga", "fsl,fpga-qixis-i2c",
>>> -			     "simple-mfd";
>>> +		compatible = "fsl,ls1028a-qds-qixis-i2c";
>>
>> This breaks all the existing users. NAK.
> 
> Using a mainline kernel, this DT node was never used or probed by a
> driver since that driver was never submitted. I am not breaking any user
> of the mainline kernel.
1. Users of DTS is plural, so what about all other projects and out of
tree users?
2. Did you remove simple-mfd from kernel or what? How can you claim
there is no driver for simple-mfd?

Best regards,
Krzysztof

Re: [PATCH 5/6] arm64: dts: ls1028a-qds: make the QIXIS CPLD use the simple-mfd-i2c.c driver

[Ioana Ciornei]

On Wed, May 07, 2025 at 06:54:38AM +0200, Krzysztof Kozlowski wrote:
> On 06/05/2025 16:21, Ioana Ciornei wrote:
> > On Fri, May 02, 2025 at 09:04:03AM +0200, Krzysztof Kozlowski wrote:
> >> On Wed, Apr 30, 2025 at 06:36:33PM GMT, Ioana Ciornei wrote:
> >>> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> >>>
> >>> The MDIO mux on the LS1028A-QDS never worked in mainline. The device
> >>> tree was submitted as-is, and there is a downstream driver for the QIXIS
> >>> FPGA:
> >>>
> >>> https://github.com/nxp-qoriq/linux/blob/lf-6.12.y/drivers/soc/fsl/qixis_ctrl.c
> >>>
> >>> That driver is very similar to the already existing drivers/mfd/simple-mfd-i2c.c,
> >>> and the hardware works with the simple-mfd-i2c driver, so there isn't
> >>> any reason to upstream the other one.
> >>>
> >>> Adapt the compatible string and child node format of the FPGA node, so
> >>> that the simple-mfd-i2c driver accepts it.
> >>
> >> Why do you break the users based on some driver differences? Fix the
> >> drivers, not the DTS.
> >>
> >>>
> >>> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> >>> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> >>> ---
> >>>  arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts | 9 +++++----
> >>>  1 file changed, 5 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
> >>> index 0bb2f28a0441..58b54d521d75 100644
> >>> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
> >>> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
> >>> @@ -338,17 +338,18 @@ sgtl5000: audio-codec@a {
> >>>  	};
> >>>  
> >>>  	fpga@66 {
> >>> -		compatible = "fsl,ls1028aqds-fpga", "fsl,fpga-qixis-i2c",
> >>> -			     "simple-mfd";
> >>> +		compatible = "fsl,ls1028a-qds-qixis-i2c";
> >>
> >> This breaks all the existing users. NAK.
> > 
> > Using a mainline kernel, this DT node was never used or probed by a
> > driver since that driver was never submitted. I am not breaking any user
> > of the mainline kernel.
> 1. Users of DTS is plural, so what about all other projects and out of
> tree users?
> 2. Did you remove simple-mfd from kernel or what? How can you claim
> there is no driver for simple-mfd?

No, I did not remove simple-mfd from the kernel.

What I am saying is that using a clean linux-next tag the child devices
of this node are not probed.

For example, the LS1028AQDS's DT looks like this in linux-next:

        fpga@66 {
                compatible = "fsl,ls1028aqds-fpga", "fsl,fpga-qixis-i2c",
                             "simple-mfd";
                reg = <0x66>;

                mux: mux-controller {
                        compatible = "reg-mux";
                        #mux-control-cells = <1>;
                        mux-reg-masks = <0x54 0xf0>; /* 0: reg 0x54, bits 7:4 */
                };
        };

        (...)

        mdio-mux {
                compatible = "mdio-mux-multiplexer";
                mux-controls = <&mux 0>;
                mdio-parent-bus = <&enetc_mdio_pf3>;
                #address-cells = <1>;
                #size-cells = <0>;

                /* on-board RGMII PHY */
                mdio@0 {
                        #address-cells = <1>;
                        #size-cells = <0>;
                        reg = <0>;

                        qds_phy1: ethernet-phy@5 {
                                /* Atheros 8035 */
                                reg = <5>;
                        };
                };

                (...)
        };

        &enetc_port1 {
                phy-handle = <&qds_phy1>;
                phy-mode = "rgmii-id";
                status = "okay";
        };

If, as you say, this works just by having the simple-mfd, I should have
been able to see the enetc_port1 functional and the RGMII PHY be
accesible on the MDIO bus which is behind the reg-mux. But this is not
happening.

Instead, I get this:

	[   17.635216] platform mdio-mux: deferred probe pending: mdio-mux-multiplexer: Failed to get mux

	root@localhost:~# ip link set dev eno1 up
	[ 1155.190391] net eno1: could not attach to PHY
	root@localhost:~# uname -a
	Linux localhost 6.15.0-rc5-next-20250507 #112 SMP PREEMPT Wed May  7 15:21:14 EEST 2025 aarch64 aarch64 aarch64 GNU/Linux

From what I understand, even though the fpga@66 has the simple-mfd
compatible, no entity initializes an I2C regmap (since this is an I2C
device) for it so that it can be used by any child device.

Please let me know what I am missing.

Ioana

Re: [PATCH 5/6] arm64: dts: ls1028a-qds: make the QIXIS CPLD use the simple-mfd-i2c.c driver

[Krzysztof Kozlowski]

On 07/05/2025 14:28, Ioana Ciornei wrote:
> On Wed, May 07, 2025 at 06:54:38AM +0200, Krzysztof Kozlowski wrote:
>> On 06/05/2025 16:21, Ioana Ciornei wrote:
>>> On Fri, May 02, 2025 at 09:04:03AM +0200, Krzysztof Kozlowski wrote:
>>>> On Wed, Apr 30, 2025 at 06:36:33PM GMT, Ioana Ciornei wrote:
>>>>> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>>>>>
>>>>> The MDIO mux on the LS1028A-QDS never worked in mainline. The device
>>>>> tree was submitted as-is, and there is a downstream driver for the QIXIS
>>>>> FPGA:
>>>>>
>>>>> https://github.com/nxp-qoriq/linux/blob/lf-6.12.y/drivers/soc/fsl/qixis_ctrl.c
>>>>>
>>>>> That driver is very similar to the already existing drivers/mfd/simple-mfd-i2c.c,
>>>>> and the hardware works with the simple-mfd-i2c driver, so there isn't
>>>>> any reason to upstream the other one.
>>>>>
>>>>> Adapt the compatible string and child node format of the FPGA node, so
>>>>> that the simple-mfd-i2c driver accepts it.
>>>>
>>>> Why do you break the users based on some driver differences? Fix the
>>>> drivers, not the DTS.
>>>>
>>>>>
>>>>> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>>>>> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
>>>>> ---
>>>>>  arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts | 9 +++++----
>>>>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
>>>>> index 0bb2f28a0441..58b54d521d75 100644
>>>>> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
>>>>> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
>>>>> @@ -338,17 +338,18 @@ sgtl5000: audio-codec@a {
>>>>>  	};
>>>>>  
>>>>>  	fpga@66 {
>>>>> -		compatible = "fsl,ls1028aqds-fpga", "fsl,fpga-qixis-i2c",
>>>>> -			     "simple-mfd";
>>>>> +		compatible = "fsl,ls1028a-qds-qixis-i2c";
>>>>
>>>> This breaks all the existing users. NAK.
>>>
>>> Using a mainline kernel, this DT node was never used or probed by a
>>> driver since that driver was never submitted. I am not breaking any user
>>> of the mainline kernel.
>> 1. Users of DTS is plural, so what about all other projects and out of
>> tree users?

You still can have users in all possible projects. Dropping simple-mfd,
even though it is Linux thingy, is affecting users, potentially breaking
them. Exactly what we talked on last plumbers - don't do such changes.

I recall even warning from Rob for people adding simple-mfd: be careful
adding it, because dropping it creates compatibility issue.

This is not a fresh platform, where you do not care about users. It is
published to all users since ~2019.


>> 2. Did you remove simple-mfd from kernel or what? How can you claim
>> there is no driver for simple-mfd?
> 
> No, I did not remove simple-mfd from the kernel.
> 


...

> If, as you say, this works just by having the simple-mfd, I should have
> been able to see the enetc_port1 functional and the RGMII PHY be
> accesible on the MDIO bus which is behind the reg-mux. But this is not
> happening.
> 
> Instead, I get this:
> 
> 	[   17.635216] platform mdio-mux: deferred probe pending: mdio-mux-multiplexer: Failed to get mux
> 
> 	root@localhost:~# ip link set dev eno1 up
> 	[ 1155.190391] net eno1: could not attach to PHY
> 	root@localhost:~# uname -a
> 	Linux localhost 6.15.0-rc5-next-20250507 #112 SMP PREEMPT Wed May  7 15:21:14 EEST 2025 aarch64 aarch64 aarch64 GNU/Linux
> 
> From what I understand, even though the fpga@66 has the simple-mfd
> compatible, no entity initializes an I2C regmap (since this is an I2C
> device) for it so that it can be used by any child device.
> 
This sounds reasonable, thanks for providing context. Most of this (so a
summary) should be in the commit msg as the rationale for changing the
ABI, so please grow a bit the commit msg part:
"The MDIO mux on the LS1028A-QDS never worked in mainline because ...".

With all this I still do not get why do you need to affect the
compatibles. What is wrong with the actual compatibles?

Best regards,
Krzysztof

Re: [PATCH 5/6] arm64: dts: ls1028a-qds: make the QIXIS CPLD use the simple-mfd-i2c.c driver

[Vladimir Oltean]

Hi Krzysztof,

On Wed, May 07, 2025 at 03:56:20PM +0200, Krzysztof Kozlowski wrote:
> On 07/05/2025 14:28, Ioana Ciornei wrote:
> > On Wed, May 07, 2025 at 06:54:38AM +0200, Krzysztof Kozlowski wrote:
> >> On 06/05/2025 16:21, Ioana Ciornei wrote:
> >>> On Fri, May 02, 2025 at 09:04:03AM +0200, Krzysztof Kozlowski wrote:
> >>>> On Wed, Apr 30, 2025 at 06:36:33PM GMT, Ioana Ciornei wrote:
> >>>>> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> >>>>>
> >>>>> The MDIO mux on the LS1028A-QDS never worked in mainline. The device
> >>>>> tree was submitted as-is, and there is a downstream driver for the QIXIS
> >>>>> FPGA:
> >>>>>
> >>>>> https://github.com/nxp-qoriq/linux/blob/lf-6.12.y/drivers/soc/fsl/qixis_ctrl.c
> >>>>>
> >>>>> That driver is very similar to the already existing drivers/mfd/simple-mfd-i2c.c,
> >>>>> and the hardware works with the simple-mfd-i2c driver, so there isn't
> >>>>> any reason to upstream the other one.
> >>>>>
> >>>>> Adapt the compatible string and child node format of the FPGA node, so
> >>>>> that the simple-mfd-i2c driver accepts it.
> >>>>
> >>>> Why do you break the users based on some driver differences? Fix the
> >>>> drivers, not the DTS.
> >>>>
> >>>>>
> >>>>> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> >>>>> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> >>>>> ---
> >>>>>  arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts | 9 +++++----
> >>>>>  1 file changed, 5 insertions(+), 4 deletions(-)
> >>>>>
> >>>>> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
> >>>>> index 0bb2f28a0441..58b54d521d75 100644
> >>>>> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
> >>>>> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
> >>>>> @@ -338,17 +338,18 @@ sgtl5000: audio-codec@a {
> >>>>>  	};
> >>>>>  
> >>>>>  	fpga@66 {
> >>>>> -		compatible = "fsl,ls1028aqds-fpga", "fsl,fpga-qixis-i2c",
> >>>>> -			     "simple-mfd";
> >>>>> +		compatible = "fsl,ls1028a-qds-qixis-i2c";
> >>>>
> >>>> This breaks all the existing users. NAK.
> >>>
> >>> Using a mainline kernel, this DT node was never used or probed by a
> >>> driver since that driver was never submitted. I am not breaking any user
> >>> of the mainline kernel.
> >> 1. Users of DTS is plural, so what about all other projects and out of
> >> tree users?
> 
> You still can have users in all possible projects. Dropping simple-mfd,
> even though it is Linux thingy, is affecting users, potentially breaking
> them. Exactly what we talked on last plumbers - don't do such changes.
> 
> I recall even warning from Rob for people adding simple-mfd: be careful
> adding it, because dropping it creates compatibility issue.
> 
> This is not a fresh platform, where you do not care about users. It is
> published to all users since ~2019.
> 
> 
> >> 2. Did you remove simple-mfd from kernel or what? How can you claim
> >> there is no driver for simple-mfd?
> > 
> > No, I did not remove simple-mfd from the kernel.
> > 
> 
> 
> ...
> 
> > If, as you say, this works just by having the simple-mfd, I should have
> > been able to see the enetc_port1 functional and the RGMII PHY be
> > accesible on the MDIO bus which is behind the reg-mux. But this is not
> > happening.
> > 
> > Instead, I get this:
> > 
> > 	[   17.635216] platform mdio-mux: deferred probe pending: mdio-mux-multiplexer: Failed to get mux
> > 
> > 	root@localhost:~# ip link set dev eno1 up
> > 	[ 1155.190391] net eno1: could not attach to PHY
> > 	root@localhost:~# uname -a
> > 	Linux localhost 6.15.0-rc5-next-20250507 #112 SMP PREEMPT Wed May  7 15:21:14 EEST 2025 aarch64 aarch64 aarch64 GNU/Linux
> > 
> > From what I understand, even though the fpga@66 has the simple-mfd
> > compatible, no entity initializes an I2C regmap (since this is an I2C
> > device) for it so that it can be used by any child device.
> > 
> This sounds reasonable, thanks for providing context. Most of this (so a
> summary) should be in the commit msg as the rationale for changing the
> ABI, so please grow a bit the commit msg part:
> "The MDIO mux on the LS1028A-QDS never worked in mainline because ...".
> 
> With all this I still do not get why do you need to affect the
> compatibles. What is wrong with the actual compatibles?
> 
> Best regards,
> Krzysztof

I really care about not breaking compatibility with device trees too,
and that's why I occasionally report issues such as
https://lore.kernel.org/all/20250412001703.qbbfhtb6koofvner@skbuf/,
but in this case the compatibility breakage is not something to worry
about.

The QDS (QorIQ Development System) boards are not made to deploy any
production software on them, they are more fully-featured variants of
our RDB (Reference Design Boards) which sit in labs and are used
exclusively by engineers to test/prototype SoC features in order to
develop for other (production) platforms. Most if not all engineers use
TFTP to load the kernel and device tree at the same time. I think it's
going to be a case of a tree falling in a forest with no one to hear it.

In terms of other projects using the Linux device tree bindings - in
this case that would be U-Boot, which unfortunately has yet another
schema for the QIXIS CPLD:
https://github.com/u-boot/u-boot/blob/master/arch/arm/dts/fsl-ls1028a-qds.dtsi#L134
So not really a concern right now, but we will take it as an action item
to resync U-Boot with the upstream device trees for the LS1028A-QDS too,
like was done for the LS1028A-RDB.

In this case, why would changing the compatible string be preferable to
using the node as is? It would be nice for the QIXIS CPLD to have child
nodes with "reg". The current format lacks that, so we thought it would
be a cleaner break if we just introduced a new compatible string, to
make it easier to distinguish:
- "fsl,ls1028aqds-fpga", "fsl,fpga-qixis-i2c", "simple-mfd" doesn't
  expect children with "reg"
- "fsl,ls1028a-qds-qixis-i2c" does.

Re: [PATCH 5/6] arm64: dts: ls1028a-qds: make the QIXIS CPLD use the simple-mfd-i2c.c driver

[Krzysztof Kozlowski]

On 07/05/2025 17:38, Vladimir Oltean wrote:
>> This sounds reasonable, thanks for providing context. Most of this (so a
>> summary) should be in the commit msg as the rationale for changing the
>> ABI, so please grow a bit the commit msg part:
>> "The MDIO mux on the LS1028A-QDS never worked in mainline because ...".
>>
>> With all this I still do not get why do you need to affect the
>> compatibles. What is wrong with the actual compatibles?
>>
>> Best regards,
>> Krzysztof
> 
> I really care about not breaking compatibility with device trees too,
> and that's why I occasionally report issues such as
> https://lore.kernel.org/all/20250412001703.qbbfhtb6koofvner@skbuf/,
> but in this case the compatibility breakage is not something to worry
> about.
> 
> The QDS (QorIQ Development System) boards are not made to deploy any
> production software on them, they are more fully-featured variants of
> our RDB (Reference Design Boards) which sit in labs and are used
> exclusively by engineers to test/prototype SoC features in order to
> develop for other (production) platforms. Most if not all engineers use
> TFTP to load the kernel and device tree at the same time. I think it's
> going to be a case of a tree falling in a forest with no one to hear it.

OK, this has to be clearly documented so the platform maintainer can
understand the impact.

> 
> In terms of other projects using the Linux device tree bindings - in
> this case that would be U-Boot, which unfortunately has yet another
> schema for the QIXIS CPLD:
> https://github.com/u-boot/u-boot/blob/master/arch/arm/dts/fsl-ls1028a-qds.dtsi#L134
> So not really a concern right now, but we will take it as an action item
> to resync U-Boot with the upstream device trees for the LS1028A-QDS too,
> like was done for the LS1028A-RDB.
> 
> In this case, why would changing the compatible string be preferable to
> using the node as is? It would be nice for the QIXIS CPLD to have child
> nodes with "reg". The current format lacks that, so we thought it would
> be a cleaner break if we just introduced a new compatible string, to

So change the current form. I do not see any point in changing
compatibles. Whatever your drivers are doing is not really relevant
here, because you can change the implementation behind the ABI.

> make it easier to distinguish:
> - "fsl,ls1028aqds-fpga", "fsl,fpga-qixis-i2c", "simple-mfd" doesn't
>   expect children with "reg"
> - "fsl,ls1028a-qds-qixis-i2c" does.

Sorry but no, we are not going to have two different compatibles for the
same device.


Best regards,
Krzysztof

[PATCH 6/6] arm64: dts: lx2160a-qds: add the two on-board RGMII PHYs

[Ioana Ciornei]

Describe the two LX2160AQDS on-board RGMII PHYs on their respective MDIO
buses behind the MDIO multiplexer.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 .../boot/dts/freescale/fsl-lx2160a-qds.dts    | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
index d1db38d6a027..a40fdd949a8e 100644
--- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
@@ -43,12 +43,22 @@ mdio@0 { /* On-board PHY #1 RGMI1*/
 			reg = <0x00>;
 			#address-cells = <1>;
 			#size-cells = <0>;
+
+			rgmii_phy1: ethernet-phy@1 {
+				compatible = "ethernet-phy-id001c.c916";
+				reg = <0x1>;
+			};
 		};
 
 		mdio@8 { /* On-board PHY #2 RGMI2*/
 			reg = <0x8>;
 			#address-cells = <1>;
 			#size-cells = <0>;
+
+			rgmii_phy2: ethernet-phy@2 {
+				compatible = "ethernet-phy-id001c.c916";
+				reg = <0x2>;
+			};
 		};
 
 		mdio@18 { /* Slot #1 */
@@ -169,6 +179,16 @@ &crypto {
 	status = "okay";
 };
 
+&dpmac17 {
+	phy-handle = <&rgmii_phy1>;
+	phy-connection-type = "rgmii-id";
+};
+
+&dpmac18 {
+	phy-handle = <&rgmii_phy2>;
+	phy-connection-type = "rgmii-id";
+};
+
 &dspi0 {
 	status = "okay";
 
-- 
2.25.1

Re: [PATCH 0/6] mfd: simple-mfd-i2c: add QIXIS CPLD support

[Rob Herring (Arm)]

On Wed, 30 Apr 2025 18:36:28 +0300, Ioana Ciornei wrote:
> The MDIO mux on the LX2160AQDS, LX2162AQDS and LS1028AQDS boards never
> worked in mainline. The DT files were submitted initially as-is, and
> there is a downstream driver for the QIXIS CPLD device:
> https://github.com/nxp-qoriq/linux/blob/lf-6.12.y/drivers/soc/fsl/qixis_ctrl.c
> 
> Since the HW works with the already existing
> driver/mfd/similar-mfd-i2c.c driver, extend the list of compatible
> strings to also cover these 3 new boards, instead of trying to upstream
> a duplicate driver.
> 
> This patch set also adapts the DT nodes for each of the affected boards
> so that we match on the new compatible strings.
> 
> The last patch describes the two on-board RGMII PHYs found on the
> LX2160AQDS boards which make use of the MDIO bus found behind the CPLD
> driven MDIO mux.
> 
> Ioana Ciornei (5):
>   dt-bindings: mfd: add bindings for QIXIS CPLD
>   mfd: simple-mfd-i2c: add compatible string for Layerscape QIXIS CPLD
>   arm64: dts: lx2160a-qds: make the QIXIS CPLD use the simple-mfd-i2c.c
>     driver
>   arm64: dts: lx2162a-qds: make the QIXIS CPLD use the simple-mfd-i2c.c
>     driver
>   arm64: dts: lx2160a-qds: add the two on-board RGMII PHYs
> 
> Vladimir Oltean (1):
>   arm64: dts: ls1028a-qds: make the QIXIS CPLD use the simple-mfd-i2c.c
>     driver
> 
>  .../bindings/mfd/fsl,qixis-i2c.yaml           | 65 +++++++++++++++++++
>  .../boot/dts/freescale/fsl-ls1028a-qds.dts    |  9 +--
>  .../boot/dts/freescale/fsl-lx2160a-qds.dts    | 28 +++++++-
>  .../boot/dts/freescale/fsl-lx2162a-qds.dts    |  8 ++-
>  drivers/mfd/simple-mfd-i2c.c                  |  3 +
>  5 files changed, 103 insertions(+), 10 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml
> 
> --
> 2.25.1
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


This patch series was applied (using b4) to base:
 Base: attempting to guess base-commit...
 Base: tags/next-20250429 (exact match)

If this is not the correct base, please add 'base-commit' tag
(or use b4 which does this automatically)

New warnings running 'make CHECK_DTBS=y for arch/arm64/boot/dts/freescale/' for 20250430153634.2971736-1-ioana.ciornei@nxp.com:

arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dtb: fpga@66 (fsl,lx2160a-qds-qixis-i2c): 'mux-controller@54' does not match any of the regexes: '^pinctrl-[0-9]+$'
	from schema $id: http://devicetree.org/schemas/mfd/fsl,qixis-i2c.yaml#
arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dtb: fpga@66 (fsl,ls1028a-qds-qixis-i2c): 'mux-controller@54' does not match any of the regexes: '^pinctrl-[0-9]+$'
	from schema $id: http://devicetree.org/schemas/mfd/fsl,qixis-i2c.yaml#
arch/arm64/boot/dts/freescale/fsl-lx2162a-qds.dtb: fpga@66 (fsl,lx2162a-qds-qixis-i2c): 'mux-controller@54' does not match any of the regexes: '^pinctrl-[0-9]+$'
	from schema $id: http://devicetree.org/schemas/mfd/fsl,qixis-i2c.yaml#

Re: [PATCH 0/6] mfd: simple-mfd-i2c: add QIXIS CPLD support

[Ioana Ciornei]

On Wed, Apr 30, 2025 at 11:15:28PM -0500, Rob Herring (Arm) wrote:
> 
> On Wed, 30 Apr 2025 18:36:28 +0300, Ioana Ciornei wrote:
> > The MDIO mux on the LX2160AQDS, LX2162AQDS and LS1028AQDS boards never
> > worked in mainline. The DT files were submitted initially as-is, and
> > there is a downstream driver for the QIXIS CPLD device:
> > https://github.com/nxp-qoriq/linux/blob/lf-6.12.y/drivers/soc/fsl/qixis_ctrl.c
> > 
> > Since the HW works with the already existing
> > driver/mfd/similar-mfd-i2c.c driver, extend the list of compatible
> > strings to also cover these 3 new boards, instead of trying to upstream
> > a duplicate driver.
> > 
> > This patch set also adapts the DT nodes for each of the affected boards
> > so that we match on the new compatible strings.
> > 
> > The last patch describes the two on-board RGMII PHYs found on the
> > LX2160AQDS boards which make use of the MDIO bus found behind the CPLD
> > driven MDIO mux.
> > 
> > Ioana Ciornei (5):
> >   dt-bindings: mfd: add bindings for QIXIS CPLD
> >   mfd: simple-mfd-i2c: add compatible string for Layerscape QIXIS CPLD
> >   arm64: dts: lx2160a-qds: make the QIXIS CPLD use the simple-mfd-i2c.c
> >     driver
> >   arm64: dts: lx2162a-qds: make the QIXIS CPLD use the simple-mfd-i2c.c
> >     driver
> >   arm64: dts: lx2160a-qds: add the two on-board RGMII PHYs
> > 
> > Vladimir Oltean (1):
> >   arm64: dts: ls1028a-qds: make the QIXIS CPLD use the simple-mfd-i2c.c
> >     driver
> > 
> >  .../bindings/mfd/fsl,qixis-i2c.yaml           | 65 +++++++++++++++++++
> >  .../boot/dts/freescale/fsl-ls1028a-qds.dts    |  9 +--
> >  .../boot/dts/freescale/fsl-lx2160a-qds.dts    | 28 +++++++-
> >  .../boot/dts/freescale/fsl-lx2162a-qds.dts    |  8 ++-
> >  drivers/mfd/simple-mfd-i2c.c                  |  3 +
> >  5 files changed, 103 insertions(+), 10 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/fsl,qixis-i2c.yaml
> > 
> > --
> > 2.25.1
> > 
> > 
> > 
> 
> 
> My bot found new DTB warnings on the .dts files added or changed in this
> series.
> 
> Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
> are fixed by another series. Ultimately, it is up to the platform
> maintainer whether these warnings are acceptable or not. No need to reply
> unless the platform maintainer has comments.
> 
> If you already ran DT checks and didn't see these error(s), then
> make sure dt-schema is up to date:
> 
>   pip3 install dtschema --upgrade
> 
> 
> This patch series was applied (using b4) to base:
>  Base: attempting to guess base-commit...
>  Base: tags/next-20250429 (exact match)
> 
> If this is not the correct base, please add 'base-commit' tag
> (or use b4 which does this automatically)
> 
> New warnings running 'make CHECK_DTBS=y for arch/arm64/boot/dts/freescale/' for 20250430153634.2971736-1-ioana.ciornei@nxp.com:
> 
> arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dtb: fpga@66 (fsl,lx2160a-qds-qixis-i2c): 'mux-controller@54' does not match any of the regexes: '^pinctrl-[0-9]+$'
> 	from schema $id: http://devicetree.org/schemas/mfd/fsl,qixis-i2c.yaml
> arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dtb: fpga@66 (fsl,ls1028a-qds-qixis-i2c): 'mux-controller@54' does not match any of the regexes: '^pinctrl-[0-9]+$'
> 	from schema $id: http://devicetree.org/schemas/mfd/fsl,qixis-i2c.yaml
> arch/arm64/boot/dts/freescale/fsl-lx2162a-qds.dtb: fpga@66 (fsl,lx2162a-qds-qixis-i2c): 'mux-controller@54' does not match any of the regexes: '^pinctrl-[0-9]+$'
> 	from schema $id: http://devicetree.org/schemas/mfd/fsl,qixis-i2c.yaml

Sorry for this, my bad. I only run dt_binding_check and didn't see any
errors because the example was not the correct one.

I will fix this in v2.

Ioana