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

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

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

I just noticed the above. A NULL regulators_node should actually work. From
drivers/regulator/of_regulator.c:regulator_of_get_init_node():

         if (desc->regulators_node) {
                 search = of_get_child_by_name(dev->of_node,
                                               desc->regulators_node);
         } else {
                 search = of_node_get(dev->of_node);

                 if (!strcmp(desc->of_match, search->name))
                         return search;
         }

So, yes, there has to be a name match, but that is on the node describing
the chip. That is how all single-regulator chips are supposed to work.

Guenter