| Message ID | 20250225-upstream-lt3074-v2-0-18ad10ba542e@analog.com |
|---|---|
| Headers | show |
| Series | Add support for LT3074 low voltage linear regulator | expand |
On Tue, Feb 25, 2025 at 09:01:13PM +0800, Cedric Encarnacion wrote: > Add Analog Devices LT3074 Ultralow Noise, High PSRR Dropout Linear > Regulator. > > Signed-off-by: Cedric Encarnacion <cedricjustine.encarnacion@analog.com> > --- > .../bindings/hwmon/pmbus/adi,lt3074.yaml | 64 ++++++++++++++++++++++ > MAINTAINERS | 7 +++ > 2 files changed, 71 insertions(+) > > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,lt3074.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,lt3074.yaml > new file mode 100644 > index 0000000000000000000000000000000000000000..714426fd655a8daa96e15e1f789743f36001ac7a > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/adi,lt3074.yaml > @@ -0,0 +1,64 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/hwmon/pmbus/adi,lt3074.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices LT3074 voltage regulator > + > +maintainers: > + - Cedric Encarnacion <cedricjustine.encarnacion@analog.com> > + > +description: | > + The LT3074 is a low voltage, ultra-low noise and ultra-fast transient > + response linear regulator. It allows telemetry for input/output voltage, > + output current and temperature through the PMBus serial interface. > + > + Datasheet: > + https://www.analog.com/en/products/lt3074.html > + > +properties: > + compatible: > + enum: > + - adi,lt3074 > + > + reg: > + maxItems: 1 > + > + regulators: > + type: object > + description: | > + list of regulators provided by this controller. You have only one regulator, so drop the "regulators". vout could be here, but since you do not have any other resources, I doubt it stands on its own either. This is even visible in your DTS - you named the device as regulator, so logically this is the regulator. Regulator does not have regulators (otherwise they could also have regulators... so triple regulator). hwmon code might need some changes, but that's not really relevant for proper hardware description. Best regards, Krzysztof
On Wed, Feb 26, 2025 at 09:20:40AM +0100, Krzysztof Kozlowski wrote: > On Tue, Feb 25, 2025 at 09:01:13PM +0800, Cedric Encarnacion wrote: > > Add Analog Devices LT3074 Ultralow Noise, High PSRR Dropout Linear > > Regulator. > > > > Signed-off-by: Cedric Encarnacion <cedricjustine.encarnacion@analog.com> > > --- > > .../bindings/hwmon/pmbus/adi,lt3074.yaml | 64 ++++++++++++++++++++++ > > MAINTAINERS | 7 +++ > > 2 files changed, 71 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,lt3074.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,lt3074.yaml > > new file mode 100644 > > index 0000000000000000000000000000000000000000..714426fd655a8daa96e15e1f789743f36001ac7a > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/adi,lt3074.yaml > > @@ -0,0 +1,64 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/hwmon/pmbus/adi,lt3074.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Analog Devices LT3074 voltage regulator > > + > > +maintainers: > > + - Cedric Encarnacion <cedricjustine.encarnacion@analog.com> > > + > > +description: | > > + The LT3074 is a low voltage, ultra-low noise and ultra-fast transient > > + response linear regulator. It allows telemetry for input/output voltage, > > + output current and temperature through the PMBus serial interface. > > + > > + Datasheet: > > + https://www.analog.com/en/products/lt3074.html > > + > > +properties: > > + compatible: > > + enum: > > + - adi,lt3074 > > + > > + reg: > > + maxItems: 1 > > + > > + regulators: > > + type: object > > + description: | > > + list of regulators provided by this controller. > > You have only one regulator, so drop the "regulators". vout could be > here, but since you do not have any other resources, I doubt it stands > on its own either. This is even visible in your DTS - you named the > device as regulator, so logically this is the regulator. Regulator does > not have regulators (otherwise they could also have regulators... so > triple regulator). > > hwmon code might need some changes, but that's not really relevant for > proper hardware description. Normally, I would agree, but it seems generic pmbus code expects this structure. This just came up with changing another binding maintained by 'Not Me' to follow this structure. We're stuck with the existing way, so I don't know that it is worth supporting 2 ways forever. OTOH, is it guaranteed that these devices will only ever be pmbus devices or that other regulator devices which are not handled as pmbus devices currently will be in the future. If so, more flexibility in the bindings will be needed. Rob
On 2/26/25 06:59, Rob Herring wrote: > On Wed, Feb 26, 2025 at 09:20:40AM +0100, Krzysztof Kozlowski wrote: >> On Tue, Feb 25, 2025 at 09:01:13PM +0800, Cedric Encarnacion wrote: >>> Add Analog Devices LT3074 Ultralow Noise, High PSRR Dropout Linear >>> Regulator. >>> >>> Signed-off-by: Cedric Encarnacion <cedricjustine.encarnacion@analog.com> >>> --- >>> .../bindings/hwmon/pmbus/adi,lt3074.yaml | 64 ++++++++++++++++++++++ >>> MAINTAINERS | 7 +++ >>> 2 files changed, 71 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,lt3074.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,lt3074.yaml >>> new file mode 100644 >>> index 0000000000000000000000000000000000000000..714426fd655a8daa96e15e1f789743f36001ac7a >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/adi,lt3074.yaml >>> @@ -0,0 +1,64 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/hwmon/pmbus/adi,lt3074.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Analog Devices LT3074 voltage regulator >>> + >>> +maintainers: >>> + - Cedric Encarnacion <cedricjustine.encarnacion@analog.com> >>> + >>> +description: | >>> + The LT3074 is a low voltage, ultra-low noise and ultra-fast transient >>> + response linear regulator. It allows telemetry for input/output voltage, >>> + output current and temperature through the PMBus serial interface. >>> + >>> + Datasheet: >>> + https://www.analog.com/en/products/lt3074.html >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - adi,lt3074 >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + regulators: >>> + type: object >>> + description: | >>> + list of regulators provided by this controller. >> >> You have only one regulator, so drop the "regulators". vout could be >> here, but since you do not have any other resources, I doubt it stands >> on its own either. This is even visible in your DTS - you named the >> device as regulator, so logically this is the regulator. Regulator does >> not have regulators (otherwise they could also have regulators... so >> triple regulator). >> >> hwmon code might need some changes, but that's not really relevant for >> proper hardware description. > > Normally, I would agree, but it seems generic pmbus code expects this > structure. This just came up with changing another binding maintained by > 'Not Me' to follow this structure. We're stuck with the existing way, so > I don't know that it is worth supporting 2 ways forever. OTOH, is it > guaranteed that these devices will only ever be pmbus devices or that > other regulator devices which are not handled as pmbus devices currently > will be in the future. If so, more flexibility in the bindings will be > needed. > I would appreciate if someone would explain to me what the problems with the current PMBus code actually are. I have seen several comments claiming that the code should be changed, but I have no idea what the expected changes actually are or, in other words, what the PMBus code should be doing differently. Thanks, Guenter
On Wed, Feb 26, 2025 at 11:17:48AM -0800, Guenter Roeck wrote: > On 2/26/25 06:59, Rob Herring wrote: > > On Wed, Feb 26, 2025 at 09:20:40AM +0100, Krzysztof Kozlowski wrote: > > > On Tue, Feb 25, 2025 at 09:01:13PM +0800, Cedric Encarnacion wrote: > > > > Add Analog Devices LT3074 Ultralow Noise, High PSRR Dropout Linear > > > > Regulator. > > > > > > > > Signed-off-by: Cedric Encarnacion <cedricjustine.encarnacion@analog.com> > > > > --- > > > > .../bindings/hwmon/pmbus/adi,lt3074.yaml | 64 ++++++++++++++++++++++ > > > > MAINTAINERS | 7 +++ > > > > 2 files changed, 71 insertions(+) > > > > > > > > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,lt3074.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,lt3074.yaml > > > > new file mode 100644 > > > > index 0000000000000000000000000000000000000000..714426fd655a8daa96e15e1f789743f36001ac7a > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/adi,lt3074.yaml > > > > @@ -0,0 +1,64 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/hwmon/pmbus/adi,lt3074.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: Analog Devices LT3074 voltage regulator > > > > + > > > > +maintainers: > > > > + - Cedric Encarnacion <cedricjustine.encarnacion@analog.com> > > > > + > > > > +description: | > > > > + The LT3074 is a low voltage, ultra-low noise and ultra-fast transient > > > > + response linear regulator. It allows telemetry for input/output voltage, > > > > + output current and temperature through the PMBus serial interface. > > > > + > > > > + Datasheet: > > > > + https://www.analog.com/en/products/lt3074.html > > > > + > > > > +properties: > > > > + compatible: > > > > + enum: > > > > + - adi,lt3074 > > > > + > > > > + reg: > > > > + maxItems: 1 > > > > + > > > > + regulators: > > > > + type: object > > > > + description: | > > > > + list of regulators provided by this controller. > > > > > > You have only one regulator, so drop the "regulators". vout could be > > > here, but since you do not have any other resources, I doubt it stands > > > on its own either. This is even visible in your DTS - you named the > > > device as regulator, so logically this is the regulator. Regulator does > > > not have regulators (otherwise they could also have regulators... so > > > triple regulator). > > > > > > hwmon code might need some changes, but that's not really relevant for > > > proper hardware description. > > > > Normally, I would agree, but it seems generic pmbus code expects this > > structure. This just came up with changing another binding maintained by > > 'Not Me' to follow this structure. We're stuck with the existing way, so > > I don't know that it is worth supporting 2 ways forever. OTOH, is it > > guaranteed that these devices will only ever be pmbus devices or that > > other regulator devices which are not handled as pmbus devices currently > > will be in the future. If so, more flexibility in the bindings will be > > needed. > > > > I would appreciate if someone would explain to me what the problems with > the current PMBus code actually are. I have seen several comments claiming Not exactly a problem but missing feature. pmbus code (at least one of macros I looked at) expects regulator node and some sort of child of it (vout), while such simple devices should be: regulator { compatible = "adi,lt3074"; regulator-name = "vout"; regulator-min-microvolt = "100000"; regulator-max-microvolt = "100000"; }; so without any of regulators and regulators/vout subnodes. > that the code should be changed, but I have no idea what the expected changes > actually are or, in other words, what the PMBus code should be doing > differently. I did not investigate much into pmbus code, but this might be as simple as accepting arguments for .of_match and .regulators_node and then accepting NULLs as them as well. Or a new macro which assigns NULLs there. Regulator core handles .regulators_node=NULL already. Best regards, Krzysztof
On Thu, Feb 27, 2025 at 09:50:23AM +0100, Krzysztof Kozlowski wrote: > > > > > > > > hwmon code might need some changes, but that's not really relevant for > > > > proper hardware description. > > > > > > Normally, I would agree, but it seems generic pmbus code expects this > > > structure. This just came up with changing another binding maintained by > > > 'Not Me' to follow this structure. We're stuck with the existing way, so > > > I don't know that it is worth supporting 2 ways forever. OTOH, is it > > > guaranteed that these devices will only ever be pmbus devices or that > > > other regulator devices which are not handled as pmbus devices currently > > > will be in the future. If so, more flexibility in the bindings will be > > > needed. > > > > > > > I would appreciate if someone would explain to me what the problems with > > the current PMBus code actually are. I have seen several comments claiming > > Not exactly a problem but missing feature. pmbus code (at least one of > macros I looked at) expects regulator node and some sort of child of it > (vout), while such simple devices should be: > > regulator { > compatible = "adi,lt3074"; > regulator-name = "vout"; > regulator-min-microvolt = "100000"; > regulator-max-microvolt = "100000"; > }; > > so without any of regulators and regulators/vout subnodes. > > > that the code should be changed, but I have no idea what the expected changes > > actually are or, in other words, what the PMBus code should be doing > > differently. > > I did not investigate much into pmbus code, but this might be as simple > as accepting arguments for .of_match and .regulators_node and then > accepting NULLs as them as well. Or a new macro which assigns NULLs > there. > Unless I am missing something, the following should do the trick. 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
> -----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 > > [External] > > On Thu, Feb 27, 2025 at 09:50:23AM +0100, Krzysztof Kozlowski wrote: > > > > > > > > > > hwmon code might need some changes, but that's not really > > > > > relevant for proper hardware description. > > > > > > > > Normally, I would agree, but it seems generic pmbus code expects > > > > this structure. This just came up with changing another binding > > > > maintained by 'Not Me' to follow this structure. We're stuck with > > > > the existing way, so I don't know that it is worth supporting 2 > > > > ways forever. OTOH, is it guaranteed that these devices will only > > > > ever be pmbus devices or that other regulator devices which are > > > > not handled as pmbus devices currently will be in the future. If > > > > so, more flexibility in the bindings will be needed. > > > > > > > > > > I would appreciate if someone would explain to me what the problems > > > with the current PMBus code actually are. I have seen several > > > comments claiming > > > > Not exactly a problem but missing feature. pmbus code (at least one of > > macros I looked at) expects regulator node and some sort of child of > > it (vout), while such simple devices should be: > > > > regulator { > > compatible = "adi,lt3074"; > > regulator-name = "vout"; > > regulator-min-microvolt = "100000"; > > regulator-max-microvolt = "100000"; > > }; > > > > so without any of regulators and regulators/vout subnodes. > > > > > that the code should be changed, but I have no idea what the > > > expected changes actually are or, in other words, what the PMBus > > > code should be doing differently. > > > > I did not investigate much into pmbus code, but this might be as > > simple as accepting arguments for .of_match and .regulators_node and > > then accepting NULLs as them as well. Or a new macro which assigns > > NULLs there. > > > > Unless I am missing something, the following should do the trick. > > 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. With this, I'd like to know what the specific next steps are to continue this patch series. Thanks, Cedric
> -----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. Best Regards, Cedric
> -----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. Best regards, Cedric
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. Thanks, Guenter
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
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
On 21/03/2025 18:24, Guenter Roeck wrote: >> >> 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 ? > I would recommend keep the nesting, so don't touch it. There might be external users, other projects relying on this. You can however deprecate old node (nesting), if the driver can support both. Not sure if it is worth the effort. Best regards, Krzysztof
On 3/24/25 00:16, Krzysztof Kozlowski wrote: > On 21/03/2025 18:24, Guenter Roeck wrote: >>> >>> 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 ? >> > I would recommend keep the nesting, so don't touch it. There might be > external users, other projects relying on this. You can however > deprecate old node (nesting), if the driver can support both. Not sure > if it is worth the effort. > That is not in driver control: If regulators_node is set by the driver, the regulator subsystem mandates the sub-node. Thanks, Guenter
Introduce hardware monitoring and regulator support for LT3074. The component is an ultrafast, ultralow noise 3A, 5.5V dropout linear regulator with a PMBus serial interface that allows telemetry for input/output voltage, output current, and die temperature. It has a single channel and requires a bias voltage which can be monitored via manufacturer-specific registers. Signed-off-by: Cedric Encarnacion <cedricjustine.encarnacion@analog.com> --- Changes in v2: * Separated dt-binding for LT3074. * Added __maybe_unused attribute to of_device_id. This addresses kernel test robot warning. * Added entry to MAINTAINERS. - Link to v1: https://lore.kernel.org/r/20250124-upstream-lt3074-v1-0-7603f346433e@analog.com --- Cedric Encarnacion (2): dt-bindings: hwmon: pmbus: add lt3074 hwmon: (pmbus/lt3074): add support for lt3074 .../bindings/hwmon/pmbus/adi,lt3074.yaml | 64 +++++++++++ Documentation/hwmon/index.rst | 1 + Documentation/hwmon/lt3074.rst | 72 ++++++++++++ MAINTAINERS | 9 ++ drivers/hwmon/pmbus/Kconfig | 18 +++ drivers/hwmon/pmbus/Makefile | 1 + drivers/hwmon/pmbus/lt3074.c | 122 +++++++++++++++++++++ 7 files changed, 287 insertions(+) --- base-commit: 8df0f002827e18632dcd986f7546c1abf1953a6f change-id: 20250124-upstream-lt3074-123384246e0b Best regards,