Re: [PATCH 08/21] dt-bindings: mfd: x-powers: add AC200
James Hilliard <[email protected]> Mon, 3 Aug 2026 01:54:16 -0600
| Newsgroups | dev.linux.lists.linux-sunxi,dev.linux.lists.mfd,org.infradead.lists.linux-arm-kernel,org.infradead.lists.linux-rockchip,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| Message-ID | <CADvTj4q7HnOYJZE-M070-1npEdne__Ct5D9FeUNVVXa8Bsm7qQ@mail.gmail.com> |
On Mon, Aug 3, 2026 at 1:07=E2=80=AFAM Krzysztof Kozlowski <[email protected]= > 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 initializa= tion > > 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 =3D <1>; > > + #size-cells =3D <0>; > > + > > + mixed-signal@10 { > > ... > > > +... > > diff --git a/include/dt-bindings/mfd/x-powers,ac200.h b/include/dt-bind= ings/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