[08/21] dt-bindings: mfd: x-powers: add AC200

The AC200 is an I2C-controlled mixed-signal companion IC containing
audio, video, RTC and Fast Ethernet PHY functions.

Describe the parent device, its input clock, required function supplies,
the optional SID bandgap calibration cell used by the vendor initialization
sequence, and its optional Ethernet PHY control child. Document the 24 and
27 MHz rates encoded by the public EPHY clock selector.

Also describe the optional open-drain, level-triggered INTB output and the
nested interrupt controller which exposes the TV encoder, Ethernet PHY and
RTC sources. Define the interrupt source numbers for child consumers.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 .../devicetree/bindings/mfd/x-powers,ac200.yaml    | 118 +++++++++++++++++++++
 include/dt-bindings/mfd/x-powers,ac200.h           |  13 +++
 2 files changed, 131 insertions(+)
  

On 03/08/2026 07:14, James Hilliard wrote:
> The AC200 is an I2C-controlled mixed-signal companion IC containing
> audio, video, RTC and Fast Ethernet PHY functions.

This fails when applied, because you did not explain the
dependencies/merging of this patchset.

This is THE MOST important information of cover letter. The first thing
to explain.

> 
> Describe the parent device, its input clock, required function supplies,
> the optional SID bandgap calibration cell used by the vendor initialization
> sequence, and its optional Ethernet PHY control child. Document the 24 and
> 27 MHz rates encoded by the public EPHY clock selector.
> 

...

> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - ac-ldoin-supply
> +  - ephy-vcc-supply
> +  - rtc-vcc-supply
> +  - tv-vcc-supply
> +
> +dependencies:
> +  interrupts: [ interrupt-controller ]
> +  interrupt-controller: [ '#interrupt-cells', interrupts ]
> +  '#interrupt-cells': [ interrupt-controller ]
> +  nvmem-cells: [ nvmem-cell-names ]
> +  nvmem-cell-names: [ nvmem-cells ]

Why do you need all these dependencies? What are you trying to express?

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        mixed-signal@10 {

...

> +...
> diff --git a/include/dt-bindings/mfd/x-powers,ac200.h b/include/dt-bindings/mfd/x-powers,ac200.h
> new file mode 100644
> index 000000000000..cc59e2ab4912
> --- /dev/null
> +++ b/include/dt-bindings/mfd/x-powers,ac200.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Interrupt numbers of the X-Powers AC200 interrupt controller.
> + */
> +
> +#ifndef _DT_BINDINGS_MFD_X_POWERS_AC200_H
> +#define _DT_BINDINGS_MFD_X_POWERS_AC200_H
> +
> +#define AC200_IRQ_TVE			0
> +#define AC200_IRQ_EPHY			1
> +#define AC200_IRQ_RTC			2

Hardware constants are not really bindings, even though you use them in
the driver.

> +
> +#endif /* _DT_BINDINGS_MFD_X_POWERS_AC200_H */
> 


Best regards,
Krzysztof
  

On Mon, Aug 3, 2026 at 1:07 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 03/08/2026 07:14, James Hilliard wrote:
> > The AC200 is an I2C-controlled mixed-signal companion IC containing
> > audio, video, RTC and Fast Ethernet PHY functions.
>
> This fails when applied, because you did not explain the
> dependencies/merging of this patchset.
>
> This is THE MOST important information of cover letter. The first thing
> to explain.

I did mention in the cover letter that the pwm series is a dependency:
https://lore.kernel.org/all/20260703152215.192859-1-richard.genoud@bootlin.com/

With the pwm series first this should apply on top of master. Should
I just mention that it applies on master or should I reference a specific
commit hash or something?

> >
> > Describe the parent device, its input clock, required function supplies,
> > the optional SID bandgap calibration cell used by the vendor initialization
> > sequence, and its optional Ethernet PHY control child. Document the 24 and
> > 27 MHz rates encoded by the public EPHY clock selector.
> >
>
> ...
>
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - ac-ldoin-supply
> > +  - ephy-vcc-supply
> > +  - rtc-vcc-supply
> > +  - tv-vcc-supply
> > +
> > +dependencies:
> > +  interrupts: [ interrupt-controller ]
> > +  interrupt-controller: [ '#interrupt-cells', interrupts ]
> > +  '#interrupt-cells': [ interrupt-controller ]
> > +  nvmem-cells: [ nvmem-cell-names ]
> > +  nvmem-cell-names: [ nvmem-cells ]
>
> Why do you need all these dependencies? What are you trying to express?

Looks like we probably can get rid of all except these:
interrupt-controller: [ interrupts ]
nvmem-cells: [ nvmem-cell-names ]

I was just trying to express the MFD controller dependencies.

> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        mixed-signal@10 {
>
> ...
>
> > +...
> > diff --git a/include/dt-bindings/mfd/x-powers,ac200.h b/include/dt-bindings/mfd/x-powers,ac200.h
> > new file mode 100644
> > index 000000000000..cc59e2ab4912
> > --- /dev/null
> > +++ b/include/dt-bindings/mfd/x-powers,ac200.h
> > @@ -0,0 +1,13 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> > +/*
> > + * Interrupt numbers of the X-Powers AC200 interrupt controller.
> > + */
> > +
> > +#ifndef _DT_BINDINGS_MFD_X_POWERS_AC200_H
> > +#define _DT_BINDINGS_MFD_X_POWERS_AC200_H
> > +
> > +#define AC200_IRQ_TVE                        0
> > +#define AC200_IRQ_EPHY                       1
> > +#define AC200_IRQ_RTC                        2
>
> Hardware constants are not really bindings, even though you use them in
> the driver.

Should I do something different for this?

>
> > +
> > +#endif /* _DT_BINDINGS_MFD_X_POWERS_AC200_H */
> >
>
>
> Best regards,
> Krzysztof
  

On 03/08/2026 09:54, James Hilliard wrote:
> On Mon, Aug 3, 2026 at 1:07 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 03/08/2026 07:14, James Hilliard wrote:
>>> The AC200 is an I2C-controlled mixed-signal companion IC containing
>>> audio, video, RTC and Fast Ethernet PHY functions.
>>
>> This fails when applied, because you did not explain the
>> dependencies/merging of this patchset.
>>
>> This is THE MOST important information of cover letter. The first thing
>> to explain.
> 
> I did mention in the cover letter that the pwm series is a dependency:
> https://lore.kernel.org/all/20260703152215.192859-1-richard.genoud@bootlin.com/

Apply this patch and test.

> 
> With the pwm series first this should apply on top of master. Should
> I just mention that it applies on master or should I reference a specific
> commit hash or something?
> 
>>>
>>> Describe the parent device, its input clock, required function supplies,
>>> the optional SID bandgap calibration cell used by the vendor initialization
>>> sequence, and its optional Ethernet PHY control child. Document the 24 and
>>> 27 MHz rates encoded by the public EPHY clock selector.
>>>
>>
>> ...
>>
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - clocks
>>> +  - ac-ldoin-supply
>>> +  - ephy-vcc-supply
>>> +  - rtc-vcc-supply
>>> +  - tv-vcc-supply
>>> +
>>> +dependencies:
>>> +  interrupts: [ interrupt-controller ]
>>> +  interrupt-controller: [ '#interrupt-cells', interrupts ]
>>> +  '#interrupt-cells': [ interrupt-controller ]
>>> +  nvmem-cells: [ nvmem-cell-names ]
>>> +  nvmem-cell-names: [ nvmem-cells ]
>>
>> Why do you need all these dependencies? What are you trying to express?
> 
> Looks like we probably can get rid of all except these:
> interrupt-controller: [ interrupts ]
> nvmem-cells: [ nvmem-cell-names ]
> 
> I was just trying to express the MFD controller dependencies.
> 
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/interrupt-controller/irq.h>
>>> +
>>> +    i2c {
>>> +        #address-cells = <1>;
>>> +        #size-cells = <0>;
>>> +
>>> +        mixed-signal@10 {
>>
>> ...
>>
>>> +...
>>> diff --git a/include/dt-bindings/mfd/x-powers,ac200.h b/include/dt-bindings/mfd/x-powers,ac200.h
>>> new file mode 100644
>>> index 000000000000..cc59e2ab4912
>>> --- /dev/null
>>> +++ b/include/dt-bindings/mfd/x-powers,ac200.h
>>> @@ -0,0 +1,13 @@
>>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>>> +/*
>>> + * Interrupt numbers of the X-Powers AC200 interrupt controller.
>>> + */
>>> +
>>> +#ifndef _DT_BINDINGS_MFD_X_POWERS_AC200_H
>>> +#define _DT_BINDINGS_MFD_X_POWERS_AC200_H
>>> +
>>> +#define AC200_IRQ_TVE                        0
>>> +#define AC200_IRQ_EPHY                       1
>>> +#define AC200_IRQ_RTC                        2
>>
>> Hardware constants are not really bindings, even though you use them in
>> the driver.
> 
> Should I do something different for this?

I would just drop the defines and the header, because these are fixed
hardware numbers.


Best regards,
Krzysztof
  

On 03/08/2026 10:20, Krzysztof Kozlowski wrote:
>>>> --- /dev/null
>>>> +++ b/include/dt-bindings/mfd/x-powers,ac200.h
>>>> @@ -0,0 +1,13 @@
>>>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>>>> +/*
>>>> + * Interrupt numbers of the X-Powers AC200 interrupt controller.
>>>> + */
>>>> +
>>>> +#ifndef _DT_BINDINGS_MFD_X_POWERS_AC200_H
>>>> +#define _DT_BINDINGS_MFD_X_POWERS_AC200_H
>>>> +
>>>> +#define AC200_IRQ_TVE                        0
>>>> +#define AC200_IRQ_EPHY                       1
>>>> +#define AC200_IRQ_RTC                        2
>>>
>>> Hardware constants are not really bindings, even though you use them in
>>> the driver.
>>
>> Should I do something different for this?
> 
> I would just drop the defines and the header, because these are fixed
> hardware numbers.
> 

Hm, unless they are not and you added abstract ID numbers for both DTS
and drivers? Then this would be fine.

Best regards,
Krzysztof
  

> +            ethernet-phy-control {
> +                compatible = "x-powers,ac200-ephy-ctl";
> +                nvmem-cells = <&ephy_calibration>;
> +                nvmem-cell-names = "calibration";
> +                phy-mode = "rmii";
> +            };

What do you mean by an ethernet PHY control?

I assume this is not an actual Ethernet PHY, but some control logic
around it? Where is the ethernet PHY itself?

       Andrew
  

On Mon, Aug 3, 2026 at 2:21 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 03/08/2026 10:20, Krzysztof Kozlowski wrote:
> >>>> --- /dev/null
> >>>> +++ b/include/dt-bindings/mfd/x-powers,ac200.h
> >>>> @@ -0,0 +1,13 @@
> >>>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> >>>> +/*
> >>>> + * Interrupt numbers of the X-Powers AC200 interrupt controller.
> >>>> + */
> >>>> +
> >>>> +#ifndef _DT_BINDINGS_MFD_X_POWERS_AC200_H
> >>>> +#define _DT_BINDINGS_MFD_X_POWERS_AC200_H
> >>>> +
> >>>> +#define AC200_IRQ_TVE                        0
> >>>> +#define AC200_IRQ_EPHY                       1
> >>>> +#define AC200_IRQ_RTC                        2
> >>>
> >>> Hardware constants are not really bindings, even though you use them in
> >>> the driver.
> >>
> >> Should I do something different for this?
> >
> > I would just drop the defines and the header, because these are fixed
> > hardware numbers.
> >
>
> Hm, unless they are not and you added abstract ID numbers for both DTS
> and drivers? Then this would be fine.

Yeah, these are more abstract ID numbers, they aren't really hardware
constants as the driver translates them.

>
> Best regards,
> Krzysztof
  

On Mon, Aug 3, 2026 at 7:18 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > +            ethernet-phy-control {
> > +                compatible = "x-powers,ac200-ephy-ctl";
> > +                nvmem-cells = <&ephy_calibration>;
> > +                nvmem-cell-names = "calibration";
> > +                phy-mode = "rmii";
> > +            };
>
> What do you mean by an ethernet PHY control?
>
> I assume this is not an actual Ethernet PHY, but some control logic
> around it? Where is the ethernet PHY itself?

Correct, this is not the Ethernet PHY itself. It is the package-specific
sideband control block that must be configured before the normal Clause 22
PHY registers become usable. For AC200, this control block is accessed
through the parent AC200 I2C regmap. For AC300, the equivalent control block
is accessed through a separate non-PHY Clause 22 address.

The actual Ethernet PHY is a separate device on the MAC's MDIO bus. The
shared PHY driver for that device is added in patch 16:

https://lore.kernel.org/linux-sunxi/20260802-submit-acx00-of-dynamic-v1-v1-16-0a53cd9e21cc@gmail.com/

The later board DTS patches instantiate it as an ethernet-phy node beneath
the MAC's MDIO bus and reference the appropriate sideband control provider.

>
>        Andrew
  

On Tue, Aug 4, 2026 at 12:45 AM James Hilliard
<james.hilliard1@gmail.com> wrote:
>
> On Mon, Aug 3, 2026 at 7:18 AM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > +            ethernet-phy-control {
> > > +                compatible = "x-powers,ac200-ephy-ctl";
> > > +                nvmem-cells = <&ephy_calibration>;
> > > +                nvmem-cell-names = "calibration";
> > > +                phy-mode = "rmii";
> > > +            };
> >
> > What do you mean by an ethernet PHY control?
> >
> > I assume this is not an actual Ethernet PHY, but some control logic
> > around it? Where is the ethernet PHY itself?
>
> Correct, this is not the Ethernet PHY itself. It is the package-specific
> sideband control block that must be configured before the normal Clause 22
> PHY registers become usable. For AC200, this control block is accessed
> through the parent AC200 I2C regmap. For AC300, the equivalent control block
> is accessed through a separate non-PHY Clause 22 address.

Basically, all the PHY behavior that is normally configured using strapping
pins on a discrete PHY is done over this I2C channel.

> The actual Ethernet PHY is a separate device on the MAC's MDIO bus. The
> shared PHY driver for that device is added in patch 16:
>
> https://lore.kernel.org/linux-sunxi/20260802-submit-acx00-of-dynamic-v1-v1-16-0a53cd9e21cc@gmail.com/
>
> The later board DTS patches instantiate it as an ethernet-phy node beneath
> the MAC's MDIO bus and reference the appropriate sideband control provider.
>
> >
> >        Andrew
  

On Mon, Aug 3, 2026 at 2:20 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 03/08/2026 09:54, James Hilliard wrote:
> > On Mon, Aug 3, 2026 at 1:07 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On 03/08/2026 07:14, James Hilliard wrote:
> >>> The AC200 is an I2C-controlled mixed-signal companion IC containing
> >>> audio, video, RTC and Fast Ethernet PHY functions.
> >>
> >> This fails when applied, because you did not explain the
> >> dependencies/merging of this patchset.
> >>
> >> This is THE MOST important information of cover letter. The first thing
> >> to explain.
> >
> > I did mention in the cover letter that the pwm series is a dependency:
> > https://lore.kernel.org/all/20260703152215.192859-1-richard.genoud@bootlin.com/
>
> Apply this patch and test.
>

How were you applying the series?

I noticed sashiko was also failing to apply the series:
https://sashiko.dev/#/patchset/20260802-submit-acx00-of-dynamic-v1-v1-0-0a53cd9e21cc%40gmail.com

But that appeared to be due to sashiko missing dependency resolution,
I went ahead and created a PR that should hopefully fix that:
https://github.com/sashiko-dev/sashiko/pull/389
  

> How were you applying the series?

All the networking patches will be applied to net-next.

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

You are going to need to split them out.

    Andrew
  

On Tue, Aug 04, 2026 at 12:51:57AM +0800, Chen-Yu Tsai wrote:
> On Tue, Aug 4, 2026 at 12:45 AM James Hilliard
> <james.hilliard1@gmail.com> wrote:
> >
> > On Mon, Aug 3, 2026 at 7:18 AM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > > +            ethernet-phy-control {
> > > > +                compatible = "x-powers,ac200-ephy-ctl";
> > > > +                nvmem-cells = <&ephy_calibration>;
> > > > +                nvmem-cell-names = "calibration";
> > > > +                phy-mode = "rmii";
> > > > +            };
> > >
> > > What do you mean by an ethernet PHY control?
> > >
> > > I assume this is not an actual Ethernet PHY, but some control logic
> > > around it? Where is the ethernet PHY itself?
> >
> > Correct, this is not the Ethernet PHY itself. It is the package-specific
> > sideband control block that must be configured before the normal Clause 22
> > PHY registers become usable. For AC200, this control block is accessed
> > through the parent AC200 I2C regmap. For AC300, the equivalent control block
> > is accessed through a separate non-PHY Clause 22 address.
> 
> Basically, all the PHY behavior that is normally configured using strapping
> pins on a discrete PHY is done over this I2C channel.

To me this should still be part of the PHY driver, not a separate
driver. The driver drives the hardware, it should drive all of it, all
in one place.

	Andrew
  

On 03/08/2026 23:34, James Hilliard wrote:
> On Mon, Aug 3, 2026 at 2:20 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 03/08/2026 09:54, James Hilliard wrote:
>>> On Mon, Aug 3, 2026 at 1:07 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>
>>>> On 03/08/2026 07:14, James Hilliard wrote:
>>>>> The AC200 is an I2C-controlled mixed-signal companion IC containing
>>>>> audio, video, RTC and Fast Ethernet PHY functions.
>>>>
>>>> This fails when applied, because you did not explain the
>>>> dependencies/merging of this patchset.
>>>>
>>>> This is THE MOST important information of cover letter. The first thing
>>>> to explain.
>>>
>>> I did mention in the cover letter that the pwm series is a dependency:
>>> https://lore.kernel.org/all/20260703152215.192859-1-richard.genoud@bootlin.com/
>>
>> Apply this patch and test.
>>
> 
> How were you applying the series?

b4. But it does not matter, your patchset simply fails for maintainer.

AGAIN: apply this one and test.


Best regards,
Krzysztof