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

[Krzysztof Kozlowski <krzk@kernel.org>]

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

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip