Re: [PATCH v2 1/2] dt-bindings: hwmon: pmbus: add lt3074

[Guenter Roeck <linux@roeck-us.net>]

On 3/21/25 10:09, Guenter Roeck wrote:
> On 3/21/25 09:53, Encarnacion, Cedric justine wrote:
>>> -----Original Message-----
>>> From: Encarnacion, Cedric justine
>>> Sent: Wednesday, March 19, 2025 12:10 PM
>>> To: Guenter Roeck <linux@roeck-us.net>; Krzysztof Kozlowski <krzk@kernel.org>
>>> Cc: Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>;
>>> Conor Dooley <conor+dt@kernel.org>; Jean Delvare <jdelvare@suse.com>;
>>> Jonathan Corbet <corbet@lwn.net>; Delphine CC Chiu
>>> <Delphine_CC_Chiu@wiwynn.com>; devicetree@vger.kernel.org; linux-
>>> kernel@vger.kernel.org; linux-hwmon@vger.kernel.org; linux-
>>> doc@vger.kernel.org; linux-i2c@vger.kernel.org
>>> Subject: RE: [PATCH v2 1/2] dt-bindings: hwmon: pmbus: add lt3074
>>>
>>>> -----Original Message-----
>>>> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
>>>> Sent: Tuesday, March 18, 2025 11:17 PM
>>>> To: Encarnacion, Cedric justine
>>>> <Cedricjustine.Encarnacion@analog.com>;
>>>> Krzysztof Kozlowski <krzk@kernel.org>
>>>> Cc: Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
>>>> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Jean Delvare
>>>> <jdelvare@suse.com>; Jonathan Corbet <corbet@lwn.net>; Delphine CC
>>>> Chiu <Delphine_CC_Chiu@wiwynn.com>; devicetree@vger.kernel.org; linux-
>>>> kernel@vger.kernel.org; linux-hwmon@vger.kernel.org; linux-
>>>> doc@vger.kernel.org; linux-i2c@vger.kernel.org
>>>> Subject: Re: [PATCH v2 1/2] dt-bindings: hwmon: pmbus: add lt3074
>>>>
>>>> [External]
>>>>
>>>> On 3/18/25 03:03, Encarnacion, Cedric justine wrote:
>>>>>> -----Original Message-----
>>>>>> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
>>>>>> Sent: Friday, February 28, 2025 12:33 AM
>>>>>> To: Krzysztof Kozlowski <krzk@kernel.org>
>>>>>> Cc: Rob Herring <robh@kernel.org>; Encarnacion, Cedric justine
>>>>>> <Cedricjustine.Encarnacion@analog.com>; Krzysztof Kozlowski
>>>>>> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Jean
>>>>>> Delvare <jdelvare@suse.com>; Jonathan Corbet <corbet@lwn.net>;
>>>>>> Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com>;
>>>>>> devicetree@vger.kernel.org; linux- kernel@vger.kernel.org;
>>>>>> linux-hwmon@vger.kernel.org; linux- doc@vger.kernel.org;
>>>>>> linux-i2c@vger.kernel.org
>>>>>> Subject: Re: [PATCH v2 1/2] dt-bindings: hwmon: pmbus: add lt3074
>>>>>>
>>>>>> diff --git a/drivers/hwmon/pmbus/pmbus.h
>>>>>> b/drivers/hwmon/pmbus/pmbus.h index ddb19c9726d6..289767e5d599
>>>>>> 100644
>>>>>> --- a/drivers/hwmon/pmbus/pmbus.h
>>>>>> +++ b/drivers/hwmon/pmbus/pmbus.h
>>>>>> @@ -512,7 +512,6 @@ int pmbus_regulator_init_cb(struct
>>>>>> regulator_dev
>>>> *rdev,
>>>>>>        {                            \
>>>>>>            .name = (_name),                \
>>>>>>            .of_match = of_match_ptr(_name),        \
>>>>>> -        .regulators_node = of_match_ptr("regulators"),    \
>>>>>>            .ops = &pmbus_regulator_ops,            \
>>>>>>            .type = REGULATOR_VOLTAGE,            \
>>>>>>            .owner = THIS_MODULE,                \
>>>>>>
>>>>>> Maybe someone can check if that works.
>>>>>>
>>>>>> Thanks,
>>>>>> Guenter
>>>>>
>>>>> I'd like to follow up on this one. As of this writing, my
>>>>> understanding is that the dt-binding should not expect regulators
>>>>> subnodes for simple devices like this. There is already a similar
>>>>> binding as mentioned in this thread particularly
>>>>> "dt-bindings/regulator/infineon,ir38060". I think a binding without
>>>>> the subnodes should still work with or without the change above.
>>>>
>>>> Interesting. I am not sure if it really works, though. I looked into
>>>> the regulator code, and I don't immediately see the code path it would
>>>> take.
>>>>
>>>>> With this, I'd like to know what the specific next steps are to
>>>>> continue this patch series.
>>>>
>>>> Can you try on hardware using a devicetree file which doesn't have the
>>>> regulators node ? If the current code works, just submit an updated
>>>> (simplified) .yaml file and we should be good. If not, I have an
>>>> untested patch series introducing another macro which doesn't set the
>>>> regulators node.
>>>
>>> Okay. I'll test this and get back to you.
>>
>> The "simplified" dt file (without the regulators node) does not work with
>> the current regulator_desc macro. I have also tried simply removing the
>> regulators_node setting from the regulator_desc macro, and it does not
>> work too. of_match looks for a certain regulator name in dt, and it seems
>> like it must handle NULL cases as well as suggested previously. I would
>> appreciate if this would be also verified on other ends. For now, I think I'll
>> wait for another macro to be introduced in pmbus to support this kind of
>> bindings.
>>
> 
> Figured. As it turns out, there is also a patch series pending which tries
> to fix the problem for ir38060 by changing its bindings.
> 
> I'll dig up my patch series to add a new macro and send it out as RFT.
> 

Question for DT maintainers:

Existing bindings, such as
	Documentation/devicetree/bindings/regulator/mps,mpq2286.yaml
expect a nested regulators node even though there is only a single
regulator. What is the correct approach: Keep the nesting requirement
for those regulators as is (even if there are no in-tree bindings
using them), or update the code and the bindings to drop the nesting ?

Thanks,
Guenter