[PATCH v2 0/2] Add driver for LTP8800-1A, LTP8800-2 and LTP8800-4A

[PATCH v2 0/2] Add driver for LTP8800-1A, LTP8800-2 and LTP8800-4A

[Cedric Encarnacion]

changes in v2:

ltp8800:
  * Added short commit description for LTP8800.
  * Removed scanned addresses.
  * Refactored documentation to unify all chips into a single prefix.
  * Removed unused headers.
  * Removed redundant i2c_check_functionality in probe.
  * Moved regulator configurations directly in ltp8800_info.
  * Used single compatible and device IDs instead of three.

Bindings:
  * Used "adi,ltp8800" instead of three entries.

Cedric Encarnacion (2):
  dt-bindings: trivial-devices: add ltp8800
  hwmon: pmbus: add driver for ltp8800-1a, ltp8800-4a, and ltp8800-2

 .../devicetree/bindings/trivial-devices.yaml  |  2 +
 Documentation/hwmon/index.rst                 |  1 +
 Documentation/hwmon/ltp8800.rst               | 91 +++++++++++++++++++
 MAINTAINERS                                   |  7 ++
 drivers/hwmon/pmbus/Kconfig                   | 18 ++++
 drivers/hwmon/pmbus/Makefile                  |  1 +
 drivers/hwmon/pmbus/ltp8800.c                 | 63 +++++++++++++
 7 files changed, 183 insertions(+)
 create mode 100644 Documentation/hwmon/ltp8800.rst
 create mode 100644 drivers/hwmon/pmbus/ltp8800.c


base-commit: 30a984c0c9d8c631cc135280f83c441d50096b6d
-- 
2.39.5

[PATCH v2 1/2] dt-bindings: trivial-devices: add ltp8800

[Cedric Encarnacion]

Add Analog Devices LTP8800-1A, LTP8800-2, and LTP8800-4A DC/DC μModule
regulator.

Signed-off-by: Cedric Encarnacion <cedricjustine.encarnacion@analog.com>
---
 Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
 MAINTAINERS                                            | 5 +++++
 2 files changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index 90a7c0a3dc48..72877d00b8dd 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -43,6 +43,8 @@ properties:
           - adi,adp5589
             # Analog Devices LT7182S Dual Channel 6A, 20V PolyPhase Step-Down Silent Switcher
           - adi,lt7182s
+            # Analog Devices LTP8800-1A/-2/-4A 150A/135A/200A, 54V DC/DC μModule regulator
+          - adi,ltp8800
             # AMS iAQ-Core VOC Sensor
           - ams,iaq-core
             # Temperature monitoring of Astera Labs PT5161L PCIe retimer
diff --git a/MAINTAINERS b/MAINTAINERS
index 7c357800519a..6ca691500fb7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13555,6 +13555,11 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/iio/light/liteon,ltr390.yaml
 F:	drivers/iio/light/ltr390.c
 
+LTP8800 HARDWARE MONITOR DRIVER
+M:	Cedric Encarnacion <cedricjustine.encarnacion@analog.com>
+L:	linux-hwmon@vger.kernel.org
+S:	Supported
+
 LYNX 28G SERDES PHY DRIVER
 M:	Ioana Ciornei <ioana.ciornei@nxp.com>
 L:	netdev@vger.kernel.org
-- 
2.39.5

Re: [PATCH v2 1/2] dt-bindings: trivial-devices: add ltp8800

[Guenter Roeck]

On 11/5/24 19:09, Cedric Encarnacion wrote:
> Add Analog Devices LTP8800-1A, LTP8800-2, and LTP8800-4A DC/DC μModule
> regulator.
> 
> Signed-off-by: Cedric Encarnacion <cedricjustine.encarnacion@analog.com>
> ---
>   Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
>   MAINTAINERS                                            | 5 +++++
>   2 files changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> index 90a7c0a3dc48..72877d00b8dd 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> @@ -43,6 +43,8 @@ properties:
>             - adi,adp5589
>               # Analog Devices LT7182S Dual Channel 6A, 20V PolyPhase Step-Down Silent Switcher
>             - adi,lt7182s
> +            # Analog Devices LTP8800-1A/-2/-4A 150A/135A/200A, 54V DC/DC μModule regulator
> +          - adi,ltp8800
>               # AMS iAQ-Core VOC Sensor
>             - ams,iaq-core
>               # Temperature monitoring of Astera Labs PT5161L PCIe retimer
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7c357800519a..6ca691500fb7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13555,6 +13555,11 @@ S:	Maintained
>   F:	Documentation/devicetree/bindings/iio/light/liteon,ltr390.yaml
>   F:	drivers/iio/light/ltr390.c
>   
> +LTP8800 HARDWARE MONITOR DRIVER
> +M:	Cedric Encarnacion <cedricjustine.encarnacion@analog.com>
> +L:	linux-hwmon@vger.kernel.org
> +S:	Supported
> +

This entry doesn't make sense in this patch.

Guenter

>   LYNX 28G SERDES PHY DRIVER
>   M:	Ioana Ciornei <ioana.ciornei@nxp.com>
>   L:	netdev@vger.kernel.org

Re: [PATCH v2 1/2] dt-bindings: trivial-devices: add ltp8800

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2021 bytes --]

On Tue, Nov 05, 2024 at 08:34:01PM -0800, Guenter Roeck wrote:
> On 11/5/24 19:09, Cedric Encarnacion wrote:
> > Add Analog Devices LTP8800-1A, LTP8800-2, and LTP8800-4A DC/DC μModule
> > regulator.

A single compatible for 3 devices is highly suspect. What is
different between these devices?

> > 
> > Signed-off-by: Cedric Encarnacion <cedricjustine.encarnacion@analog.com>
> > ---
> >   Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
> >   MAINTAINERS                                            | 5 +++++
> >   2 files changed, 7 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> > index 90a7c0a3dc48..72877d00b8dd 100644
> > --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> > +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> > @@ -43,6 +43,8 @@ properties:
> >             - adi,adp5589
> >               # Analog Devices LT7182S Dual Channel 6A, 20V PolyPhase Step-Down Silent Switcher
> >             - adi,lt7182s
> > +            # Analog Devices LTP8800-1A/-2/-4A 150A/135A/200A, 54V DC/DC μModule regulator
> > +          - adi,ltp8800
> >               # AMS iAQ-Core VOC Sensor
> >             - ams,iaq-core
> >               # Temperature monitoring of Astera Labs PT5161L PCIe retimer
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 7c357800519a..6ca691500fb7 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -13555,6 +13555,11 @@ S:	Maintained
> >   F:	Documentation/devicetree/bindings/iio/light/liteon,ltr390.yaml
> >   F:	drivers/iio/light/ltr390.c
> > +LTP8800 HARDWARE MONITOR DRIVER
> > +M:	Cedric Encarnacion <cedricjustine.encarnacion@analog.com>
> > +L:	linux-hwmon@vger.kernel.org
> > +S:	Supported
> > +
> 
> This entry doesn't make sense in this patch.
> 
> Guenter
> 
> >   LYNX 28G SERDES PHY DRIVER
> >   M:	Ioana Ciornei <ioana.ciornei@nxp.com>
> >   L:	netdev@vger.kernel.org
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/2] dt-bindings: trivial-devices: add ltp8800

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2430 bytes --]

On Wed, Nov 06, 2024 at 04:06:02PM +0000, Conor Dooley wrote:
> On Tue, Nov 05, 2024 at 08:34:01PM -0800, Guenter Roeck wrote:
> > On 11/5/24 19:09, Cedric Encarnacion wrote:
> > > Add Analog Devices LTP8800-1A, LTP8800-2, and LTP8800-4A DC/DC μModule
> > > regulator.
> 
> A single compatible for 3 devices is highly suspect. What is
> different between these devices?

Additionally, looking at one of the datasheets, this has several inputs
that could be controlled by a GPIO, a clock input and several supply
inputs. It also has a regulator output. I don't think it is suitable for
trivial-devices.yaml.

> 
> > > 
> > > Signed-off-by: Cedric Encarnacion <cedricjustine.encarnacion@analog.com>
> > > ---
> > >   Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
> > >   MAINTAINERS                                            | 5 +++++
> > >   2 files changed, 7 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> > > index 90a7c0a3dc48..72877d00b8dd 100644
> > > --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> > > +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> > > @@ -43,6 +43,8 @@ properties:
> > >             - adi,adp5589
> > >               # Analog Devices LT7182S Dual Channel 6A, 20V PolyPhase Step-Down Silent Switcher
> > >             - adi,lt7182s
> > > +            # Analog Devices LTP8800-1A/-2/-4A 150A/135A/200A, 54V DC/DC μModule regulator
> > > +          - adi,ltp8800
> > >               # AMS iAQ-Core VOC Sensor
> > >             - ams,iaq-core
> > >               # Temperature monitoring of Astera Labs PT5161L PCIe retimer
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 7c357800519a..6ca691500fb7 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -13555,6 +13555,11 @@ S:	Maintained
> > >   F:	Documentation/devicetree/bindings/iio/light/liteon,ltr390.yaml
> > >   F:	drivers/iio/light/ltr390.c
> > > +LTP8800 HARDWARE MONITOR DRIVER
> > > +M:	Cedric Encarnacion <cedricjustine.encarnacion@analog.com>
> > > +L:	linux-hwmon@vger.kernel.org
> > > +S:	Supported
> > > +
> > 
> > This entry doesn't make sense in this patch.
> > 
> > Guenter
> > 
> > >   LYNX 28G SERDES PHY DRIVER
> > >   M:	Ioana Ciornei <ioana.ciornei@nxp.com>
> > >   L:	netdev@vger.kernel.org
> > 



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/2] dt-bindings: trivial-devices: add ltp8800

[Guenter Roeck]

On 11/6/24 08:11, Conor Dooley wrote:
> On Wed, Nov 06, 2024 at 04:06:02PM +0000, Conor Dooley wrote:
>> On Tue, Nov 05, 2024 at 08:34:01PM -0800, Guenter Roeck wrote:
>>> On 11/5/24 19:09, Cedric Encarnacion wrote:
>>>> Add Analog Devices LTP8800-1A, LTP8800-2, and LTP8800-4A DC/DC μModule
>>>> regulator.
>>
>> A single compatible for 3 devices is highly suspect. What is
>> different between these devices?
> 
> Additionally, looking at one of the datasheets, this has several inputs
> that could be controlled by a GPIO, a clock input and several supply
> inputs. It also has a regulator output. I don't think it is suitable for
> trivial-devices.yaml.
> 

All PMBus devices are by definition regulators with input and output voltages.
After all, PMBus stands for "Power Management Bus". Some of them are listed
in trivial devices, some are not. Is that a general guidance, or in other
words should I (we) automatically reject patches adding PMBus devices
to the trivial devices file ?

Thanks,
Guenter

Re: [PATCH v2 1/2] dt-bindings: trivial-devices: add ltp8800

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1775 bytes --]

On Wed, Nov 06, 2024 at 08:43:54AM -0800, Guenter Roeck wrote:
> On 11/6/24 08:11, Conor Dooley wrote:
> > On Wed, Nov 06, 2024 at 04:06:02PM +0000, Conor Dooley wrote:
> > > On Tue, Nov 05, 2024 at 08:34:01PM -0800, Guenter Roeck wrote:
> > > > On 11/5/24 19:09, Cedric Encarnacion wrote:
> > > > > Add Analog Devices LTP8800-1A, LTP8800-2, and LTP8800-4A DC/DC μModule
> > > > > regulator.
> > > 
> > > A single compatible for 3 devices is highly suspect. What is
> > > different between these devices?
> > 
> > Additionally, looking at one of the datasheets, this has several inputs
> > that could be controlled by a GPIO, a clock input and several supply
> > inputs. It also has a regulator output. I don't think it is suitable for
> > trivial-devices.yaml.
> > 
> 
> All PMBus devices are by definition regulators with input and output voltages.
> After all, PMBus stands for "Power Management Bus". Some of them are listed
> in trivial devices, some are not. Is that a general guidance, or in other
> words should I (we) automatically reject patches adding PMBus devices
> to the trivial devices file ?

Personally I like what Jonathan does for iio devices, where he requires
input supplies to be documented, which in turns means they can't go into
trivial-devices.yaml. I wanted to add an input supply option to
trivial-devices.yaml but ?Rob? was not a fan.
In this case it would need a dedicated binding to document the regulator
child node and permit things like regulator-always-on or for any
consumers of the regulator to exist. I suppose that probably applies to
all pmbus bindings?
In this case, there seems to be an input "sync" clock that may need to
be enabled, which is another nail in the coffin for
trivial-devices.yaml.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/2] dt-bindings: trivial-devices: add ltp8800

[Guenter Roeck]

On 11/6/24 08:54, Conor Dooley wrote:
> On Wed, Nov 06, 2024 at 08:43:54AM -0800, Guenter Roeck wrote:
>> On 11/6/24 08:11, Conor Dooley wrote:
>>> On Wed, Nov 06, 2024 at 04:06:02PM +0000, Conor Dooley wrote:
>>>> On Tue, Nov 05, 2024 at 08:34:01PM -0800, Guenter Roeck wrote:
>>>>> On 11/5/24 19:09, Cedric Encarnacion wrote:
>>>>>> Add Analog Devices LTP8800-1A, LTP8800-2, and LTP8800-4A DC/DC μModule
>>>>>> regulator.
>>>>
>>>> A single compatible for 3 devices is highly suspect. What is
>>>> different between these devices?
>>>
>>> Additionally, looking at one of the datasheets, this has several inputs
>>> that could be controlled by a GPIO, a clock input and several supply
>>> inputs. It also has a regulator output. I don't think it is suitable for
>>> trivial-devices.yaml.
>>>
>>
>> All PMBus devices are by definition regulators with input and output voltages.
>> After all, PMBus stands for "Power Management Bus". Some of them are listed
>> in trivial devices, some are not. Is that a general guidance, or in other
>> words should I (we) automatically reject patches adding PMBus devices
>> to the trivial devices file ?
> 
> Personally I like what Jonathan does for iio devices, where he requires
> input supplies to be documented, which in turns means they can't go into
> trivial-devices.yaml. I wanted to add an input supply option to
> trivial-devices.yaml but ?Rob? was not a fan.

I may be missing something, but doesn't every chip have an input supply ?
granted, PMBus chips often have more than one, but still ...

> In this case it would need a dedicated binding to document the regulator
> child node and permit things like regulator-always-on or for any
> consumers of the regulator to exist. I suppose that probably applies to
> all pmbus bindings?

Yes. There may be a few exceptions, for example if a fan controller is
modeled as PMBus device, but that is rare. From a driver perspective,
exposing regulator nodes is optional, though.

> In this case, there seems to be an input "sync" clock that may need to
> be enabled, which is another nail in the coffin for
> trivial-devices.yaml.

I really don't know if it is a good idea to expose such data. That clock can
be connected to ground. It is only necessary in power-sharing configurations,
and requires all chips to use the same clock. I'd assume it to be a fixed clock
in pretty much all circumstances. The frequency needs to be configured into
the chip, but that needs to be done during board manufacturing because it
determines the switching frequency. Writing wrong data into the chip may
render the board unusable or even destroy it (I destroyed several PMBus chips
myself while playing with such parameters on evaluation boards). Maybe there
is some use case where changing the configuration is necessary, but I am not
in favor of exposing it due to the risk involved.

Thanks,
Guenter

Re: [PATCH v2 1/2] dt-bindings: trivial-devices: add ltp8800

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3576 bytes --]

On Wed, Nov 06, 2024 at 10:19:19AM -0800, Guenter Roeck wrote:
> On 11/6/24 08:54, Conor Dooley wrote:
> > On Wed, Nov 06, 2024 at 08:43:54AM -0800, Guenter Roeck wrote:
> > > On 11/6/24 08:11, Conor Dooley wrote:
> > > > On Wed, Nov 06, 2024 at 04:06:02PM +0000, Conor Dooley wrote:
> > > > > On Tue, Nov 05, 2024 at 08:34:01PM -0800, Guenter Roeck wrote:
> > > > > > On 11/5/24 19:09, Cedric Encarnacion wrote:
> > > > > > > Add Analog Devices LTP8800-1A, LTP8800-2, and LTP8800-4A DC/DC μModule
> > > > > > > regulator.
> > > > > 
> > > > > A single compatible for 3 devices is highly suspect. What is
> > > > > different between these devices?
> > > > 
> > > > Additionally, looking at one of the datasheets, this has several inputs
> > > > that could be controlled by a GPIO, a clock input and several supply
> > > > inputs. It also has a regulator output. I don't think it is suitable for
> > > > trivial-devices.yaml.
> > > > 
> > > 
> > > All PMBus devices are by definition regulators with input and output voltages.
> > > After all, PMBus stands for "Power Management Bus". Some of them are listed
> > > in trivial devices, some are not. Is that a general guidance, or in other
> > > words should I (we) automatically reject patches adding PMBus devices
> > > to the trivial devices file ?
> > 
> > Personally I like what Jonathan does for iio devices, where he requires
> > input supplies to be documented, which in turns means they can't go into
> > trivial-devices.yaml. I wanted to add an input supply option to
> > trivial-devices.yaml but ?Rob? was not a fan.
> 
> I may be missing something, but doesn't every chip have an input supply ?
> granted, PMBus chips often have more than one, but still ...

Yeah, that's why I wanted to permit a supply in trivial-devices, because
I bet 99% of devices in there have a supply. IIRC the problem was that
there wasn't a good "generic" name for one. I don't think it was a "you
cannot do this" but a "you need to come up with a name for that supply
that works generically" and I couldn't.

> > In this case it would need a dedicated binding to document the regulator
> > child node and permit things like regulator-always-on or for any
> > consumers of the regulator to exist. I suppose that probably applies to
> > all pmbus bindings?
> 
> Yes. There may be a few exceptions, for example if a fan controller is
> modeled as PMBus device, but that is rare. From a driver perspective,
> exposing regulator nodes is optional, though.
> 
> > In this case, there seems to be an input "sync" clock that may need to
> > be enabled, which is another nail in the coffin for
> > trivial-devices.yaml.
> 
> I really don't know if it is a good idea to expose such data. That clock can
> be connected to ground. It is only necessary in power-sharing configurations,
> and requires all chips to use the same clock. I'd assume it to be a fixed clock
> in pretty much all circumstances. The frequency needs to be configured into
> the chip, but that needs to be done during board manufacturing because it
> determines the switching frequency. Writing wrong data into the chip may
> render the board unusable or even destroy it (I destroyed several PMBus chips
> myself while playing with such parameters on evaluation boards). Maybe there
> is some use case where changing the configuration is necessary, but I am not
> in favor of exposing it due to the risk involved.

I figured it'd be fixed, but that doesn't mean it can't have an enable
(or a supply of its own).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/2] dt-bindings: trivial-devices: add ltp8800

[Guenter Roeck]

On 11/6/24 08:06, Conor Dooley wrote:
> On Tue, Nov 05, 2024 at 08:34:01PM -0800, Guenter Roeck wrote:
>> On 11/5/24 19:09, Cedric Encarnacion wrote:
>>> Add Analog Devices LTP8800-1A, LTP8800-2, and LTP8800-4A DC/DC μModule
>>> regulator.
> 
> A single compatible for 3 devices is highly suspect. What is
> different between these devices?
> 

The maximum supported current is different.

-2:  135A
-1A: 150A
-4A: 200A

Programming is exactly the same, which is why I had asked the submitter to use
a single compatible property. Sorry for that if it is inappropriate.

Is there some guidance explaining when to use a single vs. multiple compatible
properties for different chip variants ?

Note that there are also LTP8803-1A which supports 160A, and LTP8802A-1B
which supports 140A. Maybe there are more, but those are the ones I can find in
public. I don't know if there is a difference from programming perspective compared
to the LTP8800 chip variants; the datasheets are too vague to be sure. It would be
useful to know if those chips should get separate compatible entries if programming
is the same.

Thanks,
Guenter

Re: [PATCH v2 1/2] dt-bindings: trivial-devices: add ltp8800

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1697 bytes --]

On Wed, Nov 06, 2024 at 08:35:33AM -0800, Guenter Roeck wrote:
> On 11/6/24 08:06, Conor Dooley wrote:
> > On Tue, Nov 05, 2024 at 08:34:01PM -0800, Guenter Roeck wrote:
> > > On 11/5/24 19:09, Cedric Encarnacion wrote:
> > > > Add Analog Devices LTP8800-1A, LTP8800-2, and LTP8800-4A DC/DC μModule
> > > > regulator.
> > 
> > A single compatible for 3 devices is highly suspect. What is
> > different between these devices?
> > 
> 
> The maximum supported current is different.
> 
> -2:  135A
> -1A: 150A
> -4A: 200A
> 
> Programming is exactly the same, which is why I had asked the submitter to use
> a single compatible property. Sorry for that if it is inappropriate.
> 
> Is there some guidance explaining when to use a single vs. multiple compatible
> properties for different chip variants ?

TBH, I'm biased and a bit paranoid, so I'd probably give them all
compatibles and set one of them as a fallback. If the programming model
is actually identical, then it's probably fair to use a single
compatible (provided the commit message explains exactly why it's safe
to do) unless the different output conditions require using different
regulator output constraints that different compatibles would be
required to enforce.

> Note that there are also LTP8803-1A which supports 160A, and LTP8802A-1B
> which supports 140A. Maybe there are more, but those are the ones I can find in
> public. I don't know if there is a difference from programming perspective compared
> to the LTP8800 chip variants; the datasheets are too vague to be sure. It would be
> useful to know if those chips should get separate compatible entries if programming
> is the same.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/2] dt-bindings: trivial-devices: add ltp8800

[Guenter Roeck]

On 11/6/24 08:46, Conor Dooley wrote:
> On Wed, Nov 06, 2024 at 08:35:33AM -0800, Guenter Roeck wrote:
>> On 11/6/24 08:06, Conor Dooley wrote:
>>> On Tue, Nov 05, 2024 at 08:34:01PM -0800, Guenter Roeck wrote:
>>>> On 11/5/24 19:09, Cedric Encarnacion wrote:
>>>>> Add Analog Devices LTP8800-1A, LTP8800-2, and LTP8800-4A DC/DC μModule
>>>>> regulator.
>>>
>>> A single compatible for 3 devices is highly suspect. What is
>>> different between these devices?
>>>
>>
>> The maximum supported current is different.
>>
>> -2:  135A
>> -1A: 150A
>> -4A: 200A
>>
>> Programming is exactly the same, which is why I had asked the submitter to use
>> a single compatible property. Sorry for that if it is inappropriate.
>>
>> Is there some guidance explaining when to use a single vs. multiple compatible
>> properties for different chip variants ?
> 
> TBH, I'm biased and a bit paranoid, so I'd probably give them all
> compatibles and set one of them as a fallback. If the programming model

Sometimes compatibles have been rejected because a new chip variant is fully
compatible with an existing one. Now you are doing the opposite. A document
providing reliable guidance one way or the other would really be useful.

Thanks,
Guenter

[PATCH v2 2/2] hwmon: pmbus: add driver for ltp8800-1a, ltp8800-4a, and ltp8800-2

[Cedric Encarnacion]

LTP8800-1A 54V, 150A DC/DC µModule Regulator with PMBus Interface
LTP8800-4A 54V, 200A DC/DC µModule Regulator with PMBus Interface
LTP8800-2 54V, 135A DC/DC μModule Regulator with PMBus Interface

The LTP8800 is a family of step-down μModule regulators that provides
microprocessor core voltage from 54V power distribution architecture. It
features telemetry monitoring of input/output voltage, input current,
output power, and temperature over PMBus.

Signed-off-by: Cedric Encarnacion <cedricjustine.encarnacion@analog.com>
---
 Documentation/hwmon/index.rst   |  1 +
 Documentation/hwmon/ltp8800.rst | 91 +++++++++++++++++++++++++++++++++
 MAINTAINERS                     |  2 +
 drivers/hwmon/pmbus/Kconfig     | 18 +++++++
 drivers/hwmon/pmbus/Makefile    |  1 +
 drivers/hwmon/pmbus/ltp8800.c   | 63 +++++++++++++++++++++++
 6 files changed, 176 insertions(+)
 create mode 100644 Documentation/hwmon/ltp8800.rst
 create mode 100644 drivers/hwmon/pmbus/ltp8800.c

diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index 55f1111594b2..8e6799824859 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -136,6 +136,7 @@ Hardware Monitoring Kernel Drivers
    ltc4261
    ltc4282
    ltc4286
+   ltp8800
    max127
    max15301
    max16064
diff --git a/Documentation/hwmon/ltp8800.rst b/Documentation/hwmon/ltp8800.rst
new file mode 100644
index 000000000000..bbe1b4a7c827
--- /dev/null
+++ b/Documentation/hwmon/ltp8800.rst
@@ -0,0 +1,91 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+Kernel driver ltp8800
+=====================
+
+Supported chips:
+
+  * Analog Devices LTP8800-1A, LTP8800-2, LTP8800-4A
+
+    Prefix: 'ltp8800'
+
+    Addresses scanned: -
+
+    Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ltp8800-1a.pdf
+
+         https://www.analog.com/media/en/technical-documentation/data-sheets/ltp8800-2.pdf
+
+         https://www.analog.com/media/en/technical-documentation/data-sheets/ltp8800-4a.pdf
+
+Authors:
+    - Cedric Encarnacion <cedricjustine.encarnacion@analog.com>
+
+
+Description
+-----------
+
+The LTP8800 is a family of step-down μModule regulators that provides
+microprocessor core voltage from 54V power distribution architecture. LTP8800
+features telemetry monitoring of input/output voltage, input current, output
+power, and temperature over PMBus.
+
+The driver is a client driver to the core PMBus driver. Please see
+Documentation/hwmon/pmbus.rst for details on PMBus client drivers.
+
+Usage Notes
+-----------
+
+This driver does not auto-detect devices. You will have to instantiate the
+devices explicitly. Please see Documentation/i2c/instantiating-devices.rst for
+details.
+
+Platform data support
+---------------------
+
+The driver supports standard PMBus driver platform data. Please see
+Documentation/hwmon/pmbus.rst for details.
+
+Sysfs Attributes
+----------------
+
+======================= ===========================
+curr1_label		"iin"
+curr1_input		Measured input current
+curr1_crit		Critical maximum current
+curr1_crit_alarm	Current critical high alarm
+
+curr2_label		"iout1"
+curr2_input		Measured output current
+curr2_lcrit		Critical minimum current
+curr2_crit		Critical maximum current
+curr2_max		Maximum output current
+curr2_alarm		Current alarm
+
+in1_label		"vin"
+in1_input		Measured input voltage
+in1_lcrit		Critical minimum input voltage
+in1_lcrit_alarm		Input voltage critical low alarm
+in1_crit		Critical maximum input voltage
+in1_crit_alarm		Input voltage critical high alarm
+
+in2_label		"vout1"
+in2_input		Measured output voltage
+in2_lcrit		Critical minimum output voltage
+in2_lcrit_alarm		Output voltage critical low alarm
+in2_crit		Critical maximum output voltage
+in2_crit_alarm		Output voltage critical high alarm
+in2_max			Maximum output voltage
+in2_max_alarm		Output voltage high alarm
+in2_min			Minimum output voltage
+in2_min_alarm		Output voltage low alarm
+
+power1_label		"pout1"
+power1_input		Measured output power
+power1_crit		Critical maximum output power
+
+temp1_input		Measured temperature
+temp1_lcrit		Critical low temperature
+temp1_lcrit_alarm		Chip temperature critical low alarm
+temp1_crit		Critical high temperature
+temp1_crit_alarm		Chip temperature critical high alarm
+======================= ===========================
diff --git a/MAINTAINERS b/MAINTAINERS
index 6ca691500fb7..a5d1bd61fc2c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13559,6 +13559,8 @@ LTP8800 HARDWARE MONITOR DRIVER
 M:	Cedric Encarnacion <cedricjustine.encarnacion@analog.com>
 L:	linux-hwmon@vger.kernel.org
 S:	Supported
+F:	Documentation/hwmon/ltp8800.rst
+F:	drivers/hwmon/pmbus/ltp8800.c
 
 LYNX 28G SERDES PHY DRIVER
 M:	Ioana Ciornei <ioana.ciornei@nxp.com>
diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index f6d352841953..264c73275d86 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -247,6 +247,24 @@ config SENSORS_LTC4286
 	  If you say yes here you get hardware monitoring support for Analog
 	  Devices LTC4286.
 
+config SENSORS_LTP8800
+	tristate "Analog Devices LTP8800 and compatibles"
+	help
+	  If you say yes here you get hardware monitoring support for Analog
+	  Devices LTP8800-1A, LTP8800-4A, and LTP8800-2.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called ltp8800.
+
+config SENSORS_LTP8800_REGULATOR
+	bool "Regulator support for LTP8800 and compatibles"
+	depends on SENSORS_LTP8800 && REGULATOR
+	help
+	  If you say yes here you get regulator support for Analog Devices
+	  LTP8800-1A, LTP8800-4A, and LTP8800-2. LTP8800 is a family of DC/DC
+	  µModule regulators that can provide microprocessor power from 54V
+	  power distribution architecture.
+
 config SENSORS_MAX15301
 	tristate "Maxim MAX15301"
 	help
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index d00bcc758b97..aa5bbdb4a806 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_SENSORS_LT7182S)	+= lt7182s.o
 obj-$(CONFIG_SENSORS_LTC2978)	+= ltc2978.o
 obj-$(CONFIG_SENSORS_LTC3815)	+= ltc3815.o
 obj-$(CONFIG_SENSORS_LTC4286)	+= ltc4286.o
+obj-$(CONFIG_SENSORS_LTP8800)	+= ltp8800.o
 obj-$(CONFIG_SENSORS_MAX15301)	+= max15301.o
 obj-$(CONFIG_SENSORS_MAX16064)	+= max16064.o
 obj-$(CONFIG_SENSORS_MAX16601)	+= max16601.o
diff --git a/drivers/hwmon/pmbus/ltp8800.c b/drivers/hwmon/pmbus/ltp8800.c
new file mode 100644
index 000000000000..d57f8c058a89
--- /dev/null
+++ b/drivers/hwmon/pmbus/ltp8800.c
@@ -0,0 +1,63 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Hardware monitoring driver for Analog Devices LTP8800
+ *
+ * Copyright (C) 2024 Analog Devices, Inc.
+ */
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include "pmbus.h"
+
+static const struct regulator_desc ltp8800_reg_desc[] = {
+	PMBUS_REGULATOR("vout", 0),
+};
+
+static struct pmbus_driver_info ltp8800_info = {
+	.pages = 1,
+	.format[PSC_VOLTAGE_IN] = linear,
+	.format[PSC_VOLTAGE_OUT] = linear,
+	.format[PSC_CURRENT_IN] = linear,
+	.format[PSC_TEMPERATURE] = linear,
+	.func[0] = PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
+		   PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT |
+		   PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT |
+		   PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP |
+		   PMBUS_HAVE_POUT,
+#if IS_ENABLED(CONFIG_SENSORS_LTP8800_REGULATOR)
+	.num_regulators = 1,
+	.reg_desc = ltp8800_reg_desc,
+#endif
+};
+
+static int ltp8800_probe(struct i2c_client *client)
+{
+	return pmbus_do_probe(client, &ltp8800_info);
+}
+
+static const struct i2c_device_id ltp8800_id[] = {
+	{"ltp8800", 0},
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, ltp8800_id);
+
+static const struct of_device_id ltp8800_of_match[] = {
+	{ .compatible = "adi,ltp8800"},
+	{}
+};
+MODULE_DEVICE_TABLE(of, ltp8800_of_match);
+
+static struct i2c_driver ltp8800_driver = {
+	.driver = {
+		.name = "ltp8800",
+		.of_match_table = ltp8800_of_match,
+	},
+	.probe = ltp8800_probe,
+	.id_table = ltp8800_id,
+};
+module_i2c_driver(ltp8800_driver);
+
+MODULE_AUTHOR("Cedric Encarnacion <cedricjustine.encarnacion@analog.com>");
+MODULE_DESCRIPTION("Analog Devices LTP8800 HWMON PMBus Driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(PMBUS);
-- 
2.39.5

Re: [PATCH v2 2/2] hwmon: pmbus: add driver for ltp8800-1a, ltp8800-4a, and ltp8800-2

[Guenter Roeck]

On 11/5/24 19:09, Cedric Encarnacion wrote:
> LTP8800-1A 54V, 150A DC/DC µModule Regulator with PMBus Interface
> LTP8800-4A 54V, 200A DC/DC µModule Regulator with PMBus Interface
> LTP8800-2 54V, 135A DC/DC μModule Regulator with PMBus Interface
> 
> The LTP8800 is a family of step-down μModule regulators that provides
> microprocessor core voltage from 54V power distribution architecture. It
> features telemetry monitoring of input/output voltage, input current,
> output power, and temperature over PMBus.
> 
> Signed-off-by: Cedric Encarnacion <cedricjustine.encarnacion@analog.com>
> ---
>   Documentation/hwmon/index.rst   |  1 +
>   Documentation/hwmon/ltp8800.rst | 91 +++++++++++++++++++++++++++++++++
>   MAINTAINERS                     |  2 +
>   drivers/hwmon/pmbus/Kconfig     | 18 +++++++
>   drivers/hwmon/pmbus/Makefile    |  1 +
>   drivers/hwmon/pmbus/ltp8800.c   | 63 +++++++++++++++++++++++
>   6 files changed, 176 insertions(+)
>   create mode 100644 Documentation/hwmon/ltp8800.rst
>   create mode 100644 drivers/hwmon/pmbus/ltp8800.c
> 
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> index 55f1111594b2..8e6799824859 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -136,6 +136,7 @@ Hardware Monitoring Kernel Drivers
>      ltc4261
>      ltc4282
>      ltc4286
> +   ltp8800
>      max127
>      max15301
>      max16064
> diff --git a/Documentation/hwmon/ltp8800.rst b/Documentation/hwmon/ltp8800.rst
> new file mode 100644
> index 000000000000..bbe1b4a7c827
> --- /dev/null
> +++ b/Documentation/hwmon/ltp8800.rst
> @@ -0,0 +1,91 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +Kernel driver ltp8800
> +=====================
> +
> +Supported chips:
> +
> +  * Analog Devices LTP8800-1A, LTP8800-2, LTP8800-4A
> +
> +    Prefix: 'ltp8800'
> +
> +    Addresses scanned: -
> +
> +    Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ltp8800-1a.pdf
> +
> +         https://www.analog.com/media/en/technical-documentation/data-sheets/ltp8800-2.pdf
> +
> +         https://www.analog.com/media/en/technical-documentation/data-sheets/ltp8800-4a.pdf
> +
> +Authors:
> +    - Cedric Encarnacion <cedricjustine.encarnacion@analog.com>
> +
> +
> +Description
> +-----------
> +
> +The LTP8800 is a family of step-down μModule regulators that provides
> +microprocessor core voltage from 54V power distribution architecture. LTP8800
> +features telemetry monitoring of input/output voltage, input current, output
> +power, and temperature over PMBus.
> +
> +The driver is a client driver to the core PMBus driver. Please see
> +Documentation/hwmon/pmbus.rst for details on PMBus client drivers.
> +
> +Usage Notes
> +-----------
> +
> +This driver does not auto-detect devices. You will have to instantiate the
> +devices explicitly. Please see Documentation/i2c/instantiating-devices.rst for
> +details.
> +
> +Platform data support
> +---------------------
> +
> +The driver supports standard PMBus driver platform data. Please see
> +Documentation/hwmon/pmbus.rst for details.
> +
> +Sysfs Attributes
> +----------------
> +
> +======================= ===========================
> +curr1_label		"iin"
> +curr1_input		Measured input current
> +curr1_crit		Critical maximum current
> +curr1_crit_alarm	Current critical high alarm
> +
> +curr2_label		"iout1"
> +curr2_input		Measured output current
> +curr2_lcrit		Critical minimum current
> +curr2_crit		Critical maximum current
> +curr2_max		Maximum output current
> +curr2_alarm		Current alarm
> +
> +in1_label		"vin"
> +in1_input		Measured input voltage
> +in1_lcrit		Critical minimum input voltage
> +in1_lcrit_alarm		Input voltage critical low alarm
> +in1_crit		Critical maximum input voltage
> +in1_crit_alarm		Input voltage critical high alarm
> +
> +in2_label		"vout1"
> +in2_input		Measured output voltage
> +in2_lcrit		Critical minimum output voltage
> +in2_lcrit_alarm		Output voltage critical low alarm
> +in2_crit		Critical maximum output voltage
> +in2_crit_alarm		Output voltage critical high alarm
> +in2_max			Maximum output voltage
> +in2_max_alarm		Output voltage high alarm
> +in2_min			Minimum output voltage
> +in2_min_alarm		Output voltage low alarm
> +
> +power1_label		"pout1"
> +power1_input		Measured output power
> +power1_crit		Critical maximum output power
> +
> +temp1_input		Measured temperature
> +temp1_lcrit		Critical low temperature
> +temp1_lcrit_alarm		Chip temperature critical low alarm
> +temp1_crit		Critical high temperature
> +temp1_crit_alarm		Chip temperature critical high alarm
> +======================= ===========================
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6ca691500fb7..a5d1bd61fc2c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13559,6 +13559,8 @@ LTP8800 HARDWARE MONITOR DRIVER
>   M:	Cedric Encarnacion <cedricjustine.encarnacion@analog.com>
>   L:	linux-hwmon@vger.kernel.org
>   S:	Supported
> +F:	Documentation/hwmon/ltp8800.rst
> +F:	drivers/hwmon/pmbus/ltp8800.c
>   
>   LYNX 28G SERDES PHY DRIVER
>   M:	Ioana Ciornei <ioana.ciornei@nxp.com>
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index f6d352841953..264c73275d86 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -247,6 +247,24 @@ config SENSORS_LTC4286
>   	  If you say yes here you get hardware monitoring support for Analog
>   	  Devices LTC4286.
>   
> +config SENSORS_LTP8800
> +	tristate "Analog Devices LTP8800 and compatibles"
> +	help
> +	  If you say yes here you get hardware monitoring support for Analog
> +	  Devices LTP8800-1A, LTP8800-4A, and LTP8800-2.
> +
> +	  This driver can also be built as a module. If so, the module will
> +	  be called ltp8800.
> +
> +config SENSORS_LTP8800_REGULATOR
> +	bool "Regulator support for LTP8800 and compatibles"
> +	depends on SENSORS_LTP8800 && REGULATOR
> +	help
> +	  If you say yes here you get regulator support for Analog Devices
> +	  LTP8800-1A, LTP8800-4A, and LTP8800-2. LTP8800 is a family of DC/DC
> +	  µModule regulators that can provide microprocessor power from 54V
> +	  power distribution architecture.
> +
>   config SENSORS_MAX15301
>   	tristate "Maxim MAX15301"
>   	help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index d00bcc758b97..aa5bbdb4a806 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_SENSORS_LT7182S)	+= lt7182s.o
>   obj-$(CONFIG_SENSORS_LTC2978)	+= ltc2978.o
>   obj-$(CONFIG_SENSORS_LTC3815)	+= ltc3815.o
>   obj-$(CONFIG_SENSORS_LTC4286)	+= ltc4286.o
> +obj-$(CONFIG_SENSORS_LTP8800)	+= ltp8800.o
>   obj-$(CONFIG_SENSORS_MAX15301)	+= max15301.o
>   obj-$(CONFIG_SENSORS_MAX16064)	+= max16064.o
>   obj-$(CONFIG_SENSORS_MAX16601)	+= max16601.o
> diff --git a/drivers/hwmon/pmbus/ltp8800.c b/drivers/hwmon/pmbus/ltp8800.c
> new file mode 100644
> index 000000000000..d57f8c058a89
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/ltp8800.c
> @@ -0,0 +1,63 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Hardware monitoring driver for Analog Devices LTP8800
> + *
> + * Copyright (C) 2024 Analog Devices, Inc.
> + */
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include "pmbus.h"
> +
> +static const struct regulator_desc ltp8800_reg_desc[] = {
> +	PMBUS_REGULATOR("vout", 0),

Ah, sorry, I didn't realize this earlier. This needs to use "PMBUS_REGULATOR_ONE("vout")"
since there is only a single regulator (which needs to be named "vout" and not "vout0").

Thanks,
Guenter

> +};
> +
> +static struct pmbus_driver_info ltp8800_info = {
> +	.pages = 1,
> +	.format[PSC_VOLTAGE_IN] = linear,
> +	.format[PSC_VOLTAGE_OUT] = linear,
> +	.format[PSC_CURRENT_IN] = linear,
> +	.format[PSC_TEMPERATURE] = linear,
> +	.func[0] = PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
> +		   PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT |
> +		   PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT |
> +		   PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP |
> +		   PMBUS_HAVE_POUT,
> +#if IS_ENABLED(CONFIG_SENSORS_LTP8800_REGULATOR)
> +	.num_regulators = 1,
> +	.reg_desc = ltp8800_reg_desc,
> +#endif
> +};
> +
> +static int ltp8800_probe(struct i2c_client *client)
> +{
> +	return pmbus_do_probe(client, &ltp8800_info);
> +}
> +
> +static const struct i2c_device_id ltp8800_id[] = {
> +	{"ltp8800", 0},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, ltp8800_id);
> +
> +static const struct of_device_id ltp8800_of_match[] = {
> +	{ .compatible = "adi,ltp8800"},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, ltp8800_of_match);
> +
> +static struct i2c_driver ltp8800_driver = {
> +	.driver = {
> +		.name = "ltp8800",
> +		.of_match_table = ltp8800_of_match,
> +	},
> +	.probe = ltp8800_probe,
> +	.id_table = ltp8800_id,
> +};
> +module_i2c_driver(ltp8800_driver);
> +
> +MODULE_AUTHOR("Cedric Encarnacion <cedricjustine.encarnacion@analog.com>");
> +MODULE_DESCRIPTION("Analog Devices LTP8800 HWMON PMBus Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS(PMBUS);

Re: [PATCH v2 2/2] hwmon: pmbus: add driver for ltp8800-1a, ltp8800-4a, and ltp8800-2

[kernel test robot]

Hi Cedric,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 30a984c0c9d8c631cc135280f83c441d50096b6d]

url:    https://github.com/intel-lab-lkp/linux/commits/Cedric-Encarnacion/dt-bindings-trivial-devices-add-ltp8800/20241106-111734
base:   30a984c0c9d8c631cc135280f83c441d50096b6d
patch link:    https://lore.kernel.org/r/20241106030918.24849-3-cedricjustine.encarnacion%40analog.com
patch subject: [PATCH v2 2/2] hwmon: pmbus: add driver for ltp8800-1a, ltp8800-4a, and ltp8800-2
config: x86_64-buildonly-randconfig-004-20241107 (https://download.01.org/0day-ci/archive/20241107/202411070232.xrxV7tVx-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241107/202411070232.xrxV7tVx-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411070232.xrxV7tVx-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/hwmon/pmbus/ltp8800.c:7:
   In file included from include/linux/i2c.h:19:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:21:
   In file included from include/linux/mm.h:2213:
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
>> drivers/hwmon/pmbus/ltp8800.c:12:36: warning: unused variable 'ltp8800_reg_desc' [-Wunused-const-variable]
      12 | static const struct regulator_desc ltp8800_reg_desc[] = {
         |                                    ^~~~~~~~~~~~~~~~
   2 warnings generated.


vim +/ltp8800_reg_desc +12 drivers/hwmon/pmbus/ltp8800.c

    11	
  > 12	static const struct regulator_desc ltp8800_reg_desc[] = {
    13		PMBUS_REGULATOR("vout", 0),
    14	};
    15	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 2/2] hwmon: pmbus: add driver for ltp8800-1a, ltp8800-4a, and ltp8800-2

[kernel test robot]

Hi Cedric,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 30a984c0c9d8c631cc135280f83c441d50096b6d]

url:    https://github.com/intel-lab-lkp/linux/commits/Cedric-Encarnacion/dt-bindings-trivial-devices-add-ltp8800/20241106-111734
base:   30a984c0c9d8c631cc135280f83c441d50096b6d
patch link:    https://lore.kernel.org/r/20241106030918.24849-3-cedricjustine.encarnacion%40analog.com
patch subject: [PATCH v2 2/2] hwmon: pmbus: add driver for ltp8800-1a, ltp8800-4a, and ltp8800-2
config: x86_64-buildonly-randconfig-002-20241107 (https://download.01.org/0day-ci/archive/20241107/202411070253.p3aIhGGg-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241107/202411070253.p3aIhGGg-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411070253.p3aIhGGg-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/hwmon/pmbus/ltp8800.c:12:36: warning: 'ltp8800_reg_desc' defined but not used [-Wunused-const-variable=]
      12 | static const struct regulator_desc ltp8800_reg_desc[] = {
         |                                    ^~~~~~~~~~~~~~~~


vim +/ltp8800_reg_desc +12 drivers/hwmon/pmbus/ltp8800.c

    11	
  > 12	static const struct regulator_desc ltp8800_reg_desc[] = {
    13		PMBUS_REGULATOR("vout", 0),
    14	};
    15	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 2/2] hwmon: pmbus: add driver for ltp8800-1a, ltp8800-4a, and ltp8800-2

[Guenter Roeck]

On 11/5/24 19:09, Cedric Encarnacion wrote:
> LTP8800-1A 54V, 150A DC/DC µModule Regulator with PMBus Interface
> LTP8800-4A 54V, 200A DC/DC µModule Regulator with PMBus Interface
> LTP8800-2 54V, 135A DC/DC μModule Regulator with PMBus Interface
> 
> The LTP8800 is a family of step-down μModule regulators that provides
> microprocessor core voltage from 54V power distribution architecture. It
> features telemetry monitoring of input/output voltage, input current,
> output power, and temperature over PMBus.
> 
> Signed-off-by: Cedric Encarnacion <cedricjustine.encarnacion@analog.com>

Looking closer into the datasheets, I found that the PMBus commands are identical
to those of ADP1055, and an extension of the ADP1050 driver to support ADP1055
has been submitted.

With this in mind, please explain why this series warrants a new driver instead
of just extending the existing driver to support LTP8800.

Thanks,
Guenter

RE: [PATCH v2 2/2] hwmon: pmbus: add driver for ltp8800-1a, ltp8800-4a, and ltp8800-2

[Encarnacion, Cedric justine]

> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Thursday, November 7, 2024 9:45 AM
> To: Encarnacion, Cedric justine <Cedricjustine.Encarnacion@analog.com>;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> i2c@vger.kernel.org; linux-doc@vger.kernel.org; linux-
> hwmon@vger.kernel.org
> Cc: Jean Delvare <jdelvare@suse.com>; Jonathan Corbet <corbet@lwn.net>;
> Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>; Rob Herring
> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Peter Yin <peteryin.openbmc@gmail.com>; Noah
> Wang <noahwang.wang@outlook.com>; Marek Vasut <marex@denx.de>;
> Lukas Wunner <lukas@wunner.de>
> Subject: Re: [PATCH v2 2/2] hwmon: pmbus: add driver for ltp8800-1a,
> ltp8800-4a, and ltp8800-2
> 
> [External]
> 
> On 11/5/24 19:09, Cedric Encarnacion wrote:
> > LTP8800-1A 54V, 150A DC/DC µModule Regulator with PMBus Interface
> > LTP8800-4A 54V, 200A DC/DC µModule Regulator with PMBus Interface
> > LTP8800-2 54V, 135A DC/DC μModule Regulator with PMBus Interface
> >
> > The LTP8800 is a family of step-down μModule regulators that provides
> > microprocessor core voltage from 54V power distribution architecture. It
> > features telemetry monitoring of input/output voltage, input current,
> > output power, and temperature over PMBus.
> >
> > Signed-off-by: Cedric Encarnacion <cedricjustine.encarnacion@analog.com>
> 
> Looking closer into the datasheets, I found that the PMBus commands are
> identical
> to those of ADP1055, and an extension of the ADP1050 driver to support
> ADP1055
> has been submitted.
> 
> With this in mind, please explain why this series warrants a new driver instead
> of just extending the existing driver to support LTP8800.

It also appears that the LTP8800-1A regulator makes use of
factory-programmed ADP1055 but this is not explicitly stated for other
variants. Initially, I thought a new client driver would be reasonable
since this device is intended as a regulator while ADP1050/ADP1055 is a
more customizable digital controller, and both have their own multiple
variants. Indeed, it may be more reasonable to extend existing driver since
they are exposing the same feature. In this case, can this be done in
succeeding version/s of this series?

Thank you, 
Cedric

> 
> Thanks,
> Guenter

Re: [PATCH v2 2/2] hwmon: pmbus: add driver for ltp8800-1a, ltp8800-4a, and ltp8800-2

[Guenter Roeck]

On 11/7/24 23:44, Encarnacion, Cedric justine wrote:
>> -----Original Message-----
>> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
>> Sent: Thursday, November 7, 2024 9:45 AM
>> To: Encarnacion, Cedric justine <Cedricjustine.Encarnacion@analog.com>;
>> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
>> i2c@vger.kernel.org; linux-doc@vger.kernel.org; linux-
>> hwmon@vger.kernel.org
>> Cc: Jean Delvare <jdelvare@suse.com>; Jonathan Corbet <corbet@lwn.net>;
>> Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>; Rob Herring
>> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
>> <conor+dt@kernel.org>; Peter Yin <peteryin.openbmc@gmail.com>; Noah
>> Wang <noahwang.wang@outlook.com>; Marek Vasut <marex@denx.de>;
>> Lukas Wunner <lukas@wunner.de>
>> Subject: Re: [PATCH v2 2/2] hwmon: pmbus: add driver for ltp8800-1a,
>> ltp8800-4a, and ltp8800-2
>>
>> [External]
>>
>> On 11/5/24 19:09, Cedric Encarnacion wrote:
>>> LTP8800-1A 54V, 150A DC/DC µModule Regulator with PMBus Interface
>>> LTP8800-4A 54V, 200A DC/DC µModule Regulator with PMBus Interface
>>> LTP8800-2 54V, 135A DC/DC μModule Regulator with PMBus Interface
>>>
>>> The LTP8800 is a family of step-down μModule regulators that provides
>>> microprocessor core voltage from 54V power distribution architecture. It
>>> features telemetry monitoring of input/output voltage, input current,
>>> output power, and temperature over PMBus.
>>>
>>> Signed-off-by: Cedric Encarnacion <cedricjustine.encarnacion@analog.com>
>>
>> Looking closer into the datasheets, I found that the PMBus commands are
>> identical
>> to those of ADP1055, and an extension of the ADP1050 driver to support
>> ADP1055
>> has been submitted.
>>
>> With this in mind, please explain why this series warrants a new driver instead
>> of just extending the existing driver to support LTP8800.
> 
> It also appears that the LTP8800-1A regulator makes use of
> factory-programmed ADP1055 but this is not explicitly stated for other
> variants. Initially, I thought a new client driver would be reasonable
> since this device is intended as a regulator while ADP1050/ADP1055 is a
> more customizable digital controller, and both have their own multiple

We use the same driver for many variants of ltc2978. We should do the same here.

> variants. Indeed, it may be more reasonable to extend existing driver since
> they are exposing the same feature. In this case, can this be done in
> succeeding version/s of this series?
> 

Succeeding version, but please synchronize with the engineer adding support
for adp1055 to the adp1050 driver. Another option would be to make adding support
for adp1051/adp1055 and adding support for ltp880X chips a patch series. The
introductory patch could then reference the original patch series.

Thanks,
Guenter

RE: [PATCH v2 2/2] hwmon: pmbus: add driver for ltp8800-1a, ltp8800-4a, and ltp8800-2

[Encarnacion, Cedric justine]

> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Friday, November 8, 2024 10:31 PM
> To: Encarnacion, Cedric justine <Cedricjustine.Encarnacion@analog.com>;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> i2c@vger.kernel.org; linux-doc@vger.kernel.org; linux-hwmon@vger.kernel.org
> Cc: Jean Delvare <jdelvare@suse.com>; Jonathan Corbet <corbet@lwn.net>;
> Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>; Rob Herring
> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Peter Yin <peteryin.openbmc@gmail.com>; Noah
> Wang <noahwang.wang@outlook.com>; Marek Vasut <marex@denx.de>; Lukas
> Wunner <lukas@wunner.de>
> Subject: Re: [PATCH v2 2/2] hwmon: pmbus: add driver for ltp8800-1a, ltp8800-
> 4a, and ltp8800-2
> 
> [External]
> 
> On 11/7/24 23:44, Encarnacion, Cedric justine wrote:
> >> -----Original Message-----
> >> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> >> Sent: Thursday, November 7, 2024 9:45 AM
> >> To: Encarnacion, Cedric justine <Cedricjustine.Encarnacion@analog.com>;
> >> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> >> i2c@vger.kernel.org; linux-doc@vger.kernel.org; linux-
> >> hwmon@vger.kernel.org
> >> Cc: Jean Delvare <jdelvare@suse.com>; Jonathan Corbet <corbet@lwn.net>;
> >> Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>; Rob Herring
> >> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor
> Dooley
> >> <conor+dt@kernel.org>; Peter Yin <peteryin.openbmc@gmail.com>; Noah
> >> Wang <noahwang.wang@outlook.com>; Marek Vasut <marex@denx.de>;
> >> Lukas Wunner <lukas@wunner.de>
> >> Subject: Re: [PATCH v2 2/2] hwmon: pmbus: add driver for ltp8800-1a,
> >> ltp8800-4a, and ltp8800-2
> >>
> >> [External]
> >>
> >> On 11/5/24 19:09, Cedric Encarnacion wrote:
> >>> LTP8800-1A 54V, 150A DC/DC µModule Regulator with PMBus Interface
> >>> LTP8800-4A 54V, 200A DC/DC µModule Regulator with PMBus Interface
> >>> LTP8800-2 54V, 135A DC/DC μModule Regulator with PMBus Interface
> >>>
> >>> The LTP8800 is a family of step-down μModule regulators that provides
> >>> microprocessor core voltage from 54V power distribution architecture. It
> >>> features telemetry monitoring of input/output voltage, input current,
> >>> output power, and temperature over PMBus.
> >>>
> >>> Signed-off-by: Cedric Encarnacion
> <cedricjustine.encarnacion@analog.com>
> >>
> >> Looking closer into the datasheets, I found that the PMBus commands are
> >> identical
> >> to those of ADP1055, and an extension of the ADP1050 driver to support
> >> ADP1055
> >> has been submitted.
> >>
> >> With this in mind, please explain why this series warrants a new driver instead
> >> of just extending the existing driver to support LTP8800.
> >
> > It also appears that the LTP8800-1A regulator makes use of
> > factory-programmed ADP1055 but this is not explicitly stated for other
> > variants. Initially, I thought a new client driver would be reasonable
> > since this device is intended as a regulator while ADP1050/ADP1055 is a
> > more customizable digital controller, and both have their own multiple
> 
> We use the same driver for many variants of ltc2978. We should do the same
> here.
> 
> > variants. Indeed, it may be more reasonable to extend existing driver since
> > they are exposing the same feature. In this case, can this be done in
> > succeeding version/s of this series?
> >
> 
> Succeeding version, but please synchronize with the engineer adding support
> for adp1055 to the adp1050 driver. Another option would be to make adding
> support
> for adp1051/adp1055 and adding support for ltp880X chips a patch series. The
> introductory patch could then reference the original patch series.
> 
> Thanks,
> Guenter

I coordinated with the engineer adding support for ADP1051/ADP1055. We will
be submitting a new patch series adding support for ADP1051, ADP1055 and
LTP8800-XX with reference to the original patch series.

Regards,
Cedric