diff mbox series

[v7,1/2] dt-bindings: mfd: x-powers: Add AC200

Message ID 20260811-submit-ac200-mfd-v7-1-8b06f552a4d7@gmail.com (mailing list archive)
State New
Headers
Series mfd: add X-Powers AC200 support |

Commit Message

James Hilliard Aug. 11, 2026, 11:11 p.m. UTC 
The AC200 is an I2C-controlled mixed-signal companion IC containing
audio, video, RTC and Fast Ethernet PHY functions.

Describe the parent device and its shared input clock. The Ethernet PHY
is represented by a PHY package on its MDIO bus, with a phandle back to
this device for access to the package control registers, so it does not
require an MFD child node. Function-specific supplies belong to their
consumer bindings rather than the shared parent.

The input clock must run at 24 or 27 MHz when the Ethernet PHY is used,
matching the rates encoded by its documented clock selector.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 .../devicetree/bindings/mfd/x-powers,ac200.yaml    | 49 ++++++++++++++++++++++
 1 file changed, 49 insertions(+)

Comments

sashiko-bot@kernel.org Aug. 11, 2026, 11:22 p.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Potential incomplete Device Tree Binding Schema for AC200 hardware functions
--

--- Patch [1]: [PATCH v7 1/2] dt-bindings: mfd: x-powers: Add AC200 ---
commit edf03c82fb970a30013a24a5a03b1e0de64b8288
Author: James Hilliard <james.hilliard1@gmail.com>

dt-bindings: mfd: x-powers: Add AC200

This commit introduces the device tree binding schema for the X-Powers AC200
mixed-signal IC. The device provides audio, video, RTC, and Fast Ethernet PHY
functions controlled over I2C.

> diff --git a/Documentation/devicetree/bindings/mfd/x-powers,ac200.yaml b/Documentation/devicetree/bindings/mfd/x-powers,ac200.yaml
> new file mode 100644
> index 0000000000000..609831b15c806
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/x-powers,ac200.yaml

[ ... ]

> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +
> +additionalProperties: false

[Severity: Medium]
Since the AC200 provides audio, video, and RTC functions as mentioned in the
commit message, will these require child nodes in the device tree?

With additionalProperties: false, any attempt to add child nodes for these
sub-devices, or a shared interrupts property for the parent, will fail schema
validation. If these functions are meant to be modeled as MFD child nodes,
should they be documented here to avoid validation failures when device trees
start describing the IC's full capabilities?

> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        mixed-signal@10 {
> +            compatible = "x-powers,ac200";
> +            reg = <0x10>;
> +            clocks = <&pwm 5>;
> +        };
> +    };
> +...
Krzysztof Kozlowski Aug. 13, 2026, 6:49 a.m. UTC | #2 
On Tue, Aug 11, 2026 at 05:11:30PM -0600, James Hilliard wrote:
> The AC200 is an I2C-controlled mixed-signal companion IC containing
> audio, video, RTC and Fast Ethernet PHY functions.
> 
> Describe the parent device and its shared input clock. The Ethernet PHY
> is represented by a PHY package on its MDIO bus, with a phandle back to
> this device for access to the package control registers, so it does not
> require an MFD child node. Function-specific supplies belong to their
> consumer bindings rather than the shared parent.

Other way to represent it is to put ethernet phy here - either as a
child or folded into MFD device node - and provide phandle to MDIO bus.
We do like this already for several devices for I2C case with "i2c-bus"
property. Here it would be "mdio-bus" for example. Such solution feels
more vendor-agnostic, easier to re-use, however I cannot find any actual
arguments against your approach.

Maybe Rob will have his preference, but this looks okay for me then:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof
James Hilliard Aug. 13, 2026, 6:58 a.m. UTC | #3 
On Thu, Aug 13, 2026 at 12:49 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Tue, Aug 11, 2026 at 05:11:30PM -0600, James Hilliard wrote:
> > The AC200 is an I2C-controlled mixed-signal companion IC containing
> > audio, video, RTC and Fast Ethernet PHY functions.
> >
> > Describe the parent device and its shared input clock. The Ethernet PHY
> > is represented by a PHY package on its MDIO bus, with a phandle back to
> > this device for access to the package control registers, so it does not
> > require an MFD child node. Function-specific supplies belong to their
> > consumer bindings rather than the shared parent.
>
> Other way to represent it is to put ethernet phy here - either as a
> child or folded into MFD device node - and provide phandle to MDIO bus.
> We do like this already for several devices for I2C case with "i2c-bus"
> property. Here it would be "mdio-bus" for example. Such solution feels
> more vendor-agnostic, easier to re-use, however I cannot find any actual
> arguments against your approach.

The PHY package remains on the MDIO bus because the link PHY is addressed
and accessed through MDIO on both AC200 and AC300. Only the AC200
package-control registers require the additional I2C path; AC300 exposes
its package controls through MDIO as well. Keeping the package under MDIO
therefore gives both variants the same representation, with the AC200
phandle describing only its secondary control path.

>
> Maybe Rob will have his preference, but this looks okay for me then:
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
>
> Best regards,
> Krzysztof
>
Andrew Lunn Aug. 13, 2026, 1:31 p.m. UTC | #4 
On Thu, Aug 13, 2026 at 08:49:33AM +0200, Krzysztof Kozlowski wrote:
> On Tue, Aug 11, 2026 at 05:11:30PM -0600, James Hilliard wrote:
> > The AC200 is an I2C-controlled mixed-signal companion IC containing
> > audio, video, RTC and Fast Ethernet PHY functions.
> > 
> > Describe the parent device and its shared input clock. The Ethernet PHY
> > is represented by a PHY package on its MDIO bus, with a phandle back to
> > this device for access to the package control registers, so it does not
> > require an MFD child node. Function-specific supplies belong to their
> > consumer bindings rather than the shared parent.
> 
> Other way to represent it is to put ethernet phy here - either as a
> child or folded into MFD device node - and provide phandle to MDIO bus.

That would be odd. IEEE 802.3 specified that the PHY should be on an
MDIO bus. And this PHY is on an MDIO bus, that is its primary
management interface. I2C is just for ancillary configuration.

The only kind of sort of an exception we have in the current MDIO
subsystem is for SFP modules. They only have an I2C bus, not
MDIO. However, SFP vendors have various protocols for MDIO over
I2C. So the SFP does appear in the I2C tree, but we then instantiate
an MDIO bus as an I2C client, and then the PHY is then just a normal
PHY on an emulated MDIO bus.

	Andrew
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mfd/x-powers,ac200.yaml b/Documentation/devicetree/bindings/mfd/x-powers,ac200.yaml
new file mode 100644
index 000000000000..609831b15c80
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/x-powers,ac200.yaml
@@ -0,0 +1,49 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/x-powers,ac200.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: X-Powers AC200 mixed-signal IC
+
+maintainers:
+  - James Hilliard <james.hilliard1@gmail.com>
+
+description:
+  The AC200 is a mixed-signal companion IC containing audio, video, RTC and
+  Fast Ethernet PHY functions. Its control registers are accessed over I2C.
+
+properties:
+  compatible:
+    const: x-powers,ac200
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+    description:
+      AC200 input clock. When using the Ethernet PHY, its configured rate must
+      be 24 or 27 MHz, matching the rates encoded by the documented EPHY clock
+      selector.
+
+required:
+  - compatible
+  - reg
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        mixed-signal@10 {
+            compatible = "x-powers,ac200";
+            reg = <0x10>;
+            clocks = <&pwm 5>;
+        };
+    };
+...