[PATCH 0/3] hwmon: (pmbus) Use PMBUS_REGULATOR_ONE to declare regulators with single output

[PATCH 0/3] hwmon: (pmbus) Use PMBUS_REGULATOR_ONE to declare regulators with single output

[Guenter Roeck]

If a chip only provides a single regulator, it should be named 'vout'
and not 'vout0'. Declare regulators for chips with a single ouput using
PMBUS_REGULATOR_ONE() to make that happen.

----------------------------------------------------------------
Guenter Roeck (3):
      hwmon: (pmbus/tda38640) Use PMBUS_REGULATOR_ONE to declare regulator
      hwmon: (pmbus/lm25066) Use PMBUS_REGULATOR_ONE to declare regulator
      hwmon: (pmbus/ir38064) Use PMBUS_REGULATOR_ONE to declare regulator

 drivers/hwmon/pmbus/ir38064.c  | 2 +-
 drivers/hwmon/pmbus/lm25066.c  | 2 +-
 drivers/hwmon/pmbus/tda38640.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

[PATCH 1/3] hwmon: (pmbus/tda38640) Use PMBUS_REGULATOR_ONE to declare regulator

[Guenter Roeck]

If a chip only provides a single regulator, it should be named 'vout'
and not 'vout0'. Declare regulator using PMBUS_REGULATOR_ONE() to make
that happen.

Cc: Conor Dooley <conor@kernel.org>
Cc: Naresh Solanki <naresh.solanki@9elements.com>
Cc: Patrick Rudolph <patrick.rudolph@9elements.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/pmbus/tda38640.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/tda38640.c b/drivers/hwmon/pmbus/tda38640.c
index 09cd114b1736..c31889a036f0 100644
--- a/drivers/hwmon/pmbus/tda38640.c
+++ b/drivers/hwmon/pmbus/tda38640.c
@@ -15,7 +15,7 @@
 #include "pmbus.h"
 
 static const struct regulator_desc __maybe_unused tda38640_reg_desc[] = {
-	PMBUS_REGULATOR("vout", 0),
+	PMBUS_REGULATOR_ONE("vout"),
 };
 
 struct tda38640_data {
-- 
2.39.2

[PATCH 2/3] hwmon: (pmbus/lm25066) Use PMBUS_REGULATOR_ONE to declare regulator

[Guenter Roeck]

If a chip only provides a single regulator, it should be named 'vout'
and not 'vout0'. Declare regulator using PMBUS_REGULATOR_ONE() to make
that happen.

Cc: Conor Dooley <conor@kernel.org>
Cc: Naresh Solanki <naresh.solanki@9elements.com>
Cc: Zev Weiss <zev@bewilderbeest.net>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/pmbus/lm25066.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/lm25066.c b/drivers/hwmon/pmbus/lm25066.c
index 3a20df5a43ec..cfffa4cdc0df 100644
--- a/drivers/hwmon/pmbus/lm25066.c
+++ b/drivers/hwmon/pmbus/lm25066.c
@@ -437,7 +437,7 @@ static int lm25066_write_word_data(struct i2c_client *client, int page, int reg,
 
 #if IS_ENABLED(CONFIG_SENSORS_LM25066_REGULATOR)
 static const struct regulator_desc lm25066_reg_desc[] = {
-	PMBUS_REGULATOR("vout", 0),
+	PMBUS_REGULATOR_ONE("vout"),
 };
 #endif
 
-- 
2.39.2

[PATCH 3/3] hwmon: (pmbus/ir38064) Use PMBUS_REGULATOR_ONE to declare regulator

[Guenter Roeck]

If a chip only provides a single regulator, it should be named 'vout'
and not 'vout0'. Declare regulator using PMBUS_REGULATOR_ONE() to make
that happen.

Cc: Conor Dooley <conor@kernel.org>
Cc: Naresh Solanki <naresh.solanki@9elements.com>
Cc: Patrick Rudolph <patrick.rudolph@9elements.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/pmbus/ir38064.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/ir38064.c b/drivers/hwmon/pmbus/ir38064.c
index 04185be3fdb6..69e18cb468f6 100644
--- a/drivers/hwmon/pmbus/ir38064.c
+++ b/drivers/hwmon/pmbus/ir38064.c
@@ -22,7 +22,7 @@
 
 #if IS_ENABLED(CONFIG_SENSORS_IR38064_REGULATOR)
 static const struct regulator_desc ir38064_reg_desc[] = {
-	PMBUS_REGULATOR("vout", 0),
+	PMBUS_REGULATOR_ONE("vout"),
 };
 #endif /* CONFIG_SENSORS_IR38064_REGULATOR */
 
-- 
2.39.2

Re: [PATCH 2/3] hwmon: (pmbus/lm25066) Use PMBUS_REGULATOR_ONE to declare regulator

[Zev Weiss]

On Wed, Feb 14, 2024 at 11:43:41AM PST, Guenter Roeck wrote:
>If a chip only provides a single regulator, it should be named 'vout'
>and not 'vout0'. Declare regulator using PMBUS_REGULATOR_ONE() to make
>that happen.
>

Hi Guenter,

This will necessitate a DTS update on at least one platform to maintain 
compatibility (Delta ahe50dc BMC, [1]).  I'm not sure offhand if there 
are process/policy rules about mixing code changes and device-tree 
changes in the same commit, but changing either one without the other 
would break things.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/aspeed/aspeed-bmc-delta-ahe50dc.dts?id=8d3dea210042f54b952b481838c1e7dfc4ec751d#n21


Thanks,
Zev

Re: [PATCH 2/3] hwmon: (pmbus/lm25066) Use PMBUS_REGULATOR_ONE to declare regulator

[Guenter Roeck]

On 2/14/24 17:04, Zev Weiss wrote:
> On Wed, Feb 14, 2024 at 11:43:41AM PST, Guenter Roeck wrote:
>> If a chip only provides a single regulator, it should be named 'vout'
>> and not 'vout0'. Declare regulator using PMBUS_REGULATOR_ONE() to make
>> that happen.
>>
> 
> Hi Guenter,
> 
> This will necessitate a DTS update on at least one platform to maintain compatibility (Delta ahe50dc BMC, [1]).  I'm not sure offhand if there are process/policy rules about mixing code changes and device-tree changes in the same commit, but changing either one without the other would break things.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/aspeed/aspeed-bmc-delta-ahe50dc.dts?id=8d3dea210042f54b952b481838c1e7dfc4ec751d#n21
> 
> 

Sigh. Agreed, especially since changing the dts file in the kernel
won't change the dtb files on actual hardware.

I really have no good solution for this. We (Well, I) didn't realize that
there are regulator naming conventions/restrictions when we introduced
regulator support into PMBus drivers. My bad. Let's see what others say.

Guenter

Re: [PATCH 2/3] hwmon: (pmbus/lm25066) Use PMBUS_REGULATOR_ONE to declare regulator

[Zev Weiss]

On Wed, Feb 14, 2024 at 05:22:35PM PST, Guenter Roeck wrote:
>On 2/14/24 17:04, Zev Weiss wrote:
>>On Wed, Feb 14, 2024 at 11:43:41AM PST, Guenter Roeck wrote:
>>>If a chip only provides a single regulator, it should be named 'vout'
>>>and not 'vout0'. Declare regulator using PMBUS_REGULATOR_ONE() to make
>>>that happen.
>>>
>>
>>Hi Guenter,
>>
>>This will necessitate a DTS update on at least one platform to maintain compatibility (Delta ahe50dc BMC, [1]).  I'm not sure offhand if there are process/policy rules about mixing code changes and device-tree changes in the same commit, but changing either one without the other would break things.
>>
>>[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/aspeed/aspeed-bmc-delta-ahe50dc.dts?id=8d3dea210042f54b952b481838c1e7dfc4ec751d#n21
>>
>>
>
>Sigh. Agreed, especially since changing the dts file in the kernel
>won't change the dtb files on actual hardware.
>
>I really have no good solution for this. We (Well, I) didn't realize that
>there are regulator naming conventions/restrictions when we introduced
>regulator support into PMBus drivers. My bad. Let's see what others say.
>
>Guenter
>

Well, perhaps mitigating that slightly: I don't see any obvious cases of 
any other platforms' device-trees having any dependencies on the 
regulator naming that would be affected by this (judging by 'git grep 
vout0 arch/*/boot/dts' anyway), and at least with OpenBMC on the ahe50dc 
(the primary and AFAIK only user of that device-tree) the dtb would also 
be updated along with any kernel update.

So I wouldn't expect it to cause anyone any actual problems if we went 
ahead and changed it anyway; as long as the dts & driver do stay in sync 
with each other, maybe we could let it slide if it's otherwise a 
desirable change to make?


Zev

Re: [PATCH 2/3] hwmon: (pmbus/lm25066) Use PMBUS_REGULATOR_ONE to declare regulator

[Conor Dooley]

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

On Thu, Feb 15, 2024 at 02:14:37AM -0800, Zev Weiss wrote:
> On Wed, Feb 14, 2024 at 05:22:35PM PST, Guenter Roeck wrote:
> > On 2/14/24 17:04, Zev Weiss wrote:
> > > On Wed, Feb 14, 2024 at 11:43:41AM PST, Guenter Roeck wrote:
> > > > If a chip only provides a single regulator, it should be named 'vout'
> > > > and not 'vout0'. Declare regulator using PMBUS_REGULATOR_ONE() to make
> > > > that happen.
> > > > 
> > > 
> > > Hi Guenter,
> > > 
> > > This will necessitate a DTS update on at least one platform to maintain compatibility (Delta ahe50dc BMC, [1]).  I'm not sure offhand if there are process/policy rules about mixing code changes and device-tree changes in the same commit, but changing either one without the other would break things.
> > > 
> > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/aspeed/aspeed-bmc-delta-ahe50dc.dts?id=8d3dea210042f54b952b481838c1e7dfc4ec751d#n21
> > > 
> > > 
> > 
> > Sigh. Agreed, especially since changing the dts file in the kernel
> > won't change the dtb files on actual hardware.
> > 
> > I really have no good solution for this. We (Well, I) didn't realize that
> > there are regulator naming conventions/restrictions when we introduced
> > regulator support into PMBus drivers. My bad. Let's see what others say.
> > 
> > Guenter
> > 
> 
> Well, perhaps mitigating that slightly: I don't see any obvious cases of any
> other platforms' device-trees having any dependencies on the regulator
> naming that would be affected by this (judging by 'git grep vout0
> arch/*/boot/dts' anyway),

Which is a good thing, as none of these devices' bindings actually allow
regulator. Aspeed devicetrees are a mess of non-compliance with the
bindings, so I suppose that this is not surprising, but somewhere in the
wall of complaints there is a:
aspeed-bmc-delta-ahe50dc.dtb: /ahb/apb/bus@1e78a000/i2c-bus@40/pca9541@79/i2c-arb/efuse@12: failed to match any schema with compatible: ['lm25066']

I mentioned this on the other thread, but I guess the kernel is actually
using the i2c_device_ids to probe the driver and not the compatible,
since the documented compatible and the compatibles in the driver have a
vendor prefix?

> and at least with OpenBMC on the ahe50dc (the
> primary and AFAIK only user of that device-tree) the dtb would also be
> updated along with any kernel update.
> 
> So I wouldn't expect it to cause anyone any actual problems if we went ahead
> and changed it anyway; as long as the dts & driver do stay in sync with each
> other, maybe we could let it slide if it's otherwise a desirable change to
> make?

I think having "vout0" when there's nothing else looks a bit odd, but
I'm not gonna lose sleep over "vout0" being used if its used in device
documentation or is convention.

Whatever is done, please document the regulator child node in the
binding for this device and fix the dts to use a real compatible.

Cheers,
Conor.

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

Re: [PATCH 0/3] hwmon: (pmbus) Use PMBUS_REGULATOR_ONE to declare regulators with single output

[Conor Dooley]

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

On Wed, Feb 14, 2024 at 11:43:39AM -0800, Guenter Roeck wrote:
> If a chip only provides a single regulator, it should be named 'vout'
> and not 'vout0'. Declare regulators for chips with a single ouput using
> PMBUS_REGULATOR_ONE() to make that happen.

If we're gonna do this, we should go ahead and document each of these
regulators. If Naresh sends a v2 for the tda, I could do the others.

> 
> ----------------------------------------------------------------
> Guenter Roeck (3):
>       hwmon: (pmbus/tda38640) Use PMBUS_REGULATOR_ONE to declare regulator
>       hwmon: (pmbus/lm25066) Use PMBUS_REGULATOR_ONE to declare regulator
>       hwmon: (pmbus/ir38064) Use PMBUS_REGULATOR_ONE to declare regulator
> 
>  drivers/hwmon/pmbus/ir38064.c  | 2 +-
>  drivers/hwmon/pmbus/lm25066.c  | 2 +-
>  drivers/hwmon/pmbus/tda38640.c | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)

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

Re: [PATCH 0/3] hwmon: (pmbus) Use PMBUS_REGULATOR_ONE to declare regulators with single output

[Conor Dooley]

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

On Sat, Feb 17, 2024 at 10:00:30PM +0000, Conor Dooley wrote:
> On Wed, Feb 14, 2024 at 11:43:39AM -0800, Guenter Roeck wrote:
> > If a chip only provides a single regulator, it should be named 'vout'
> > and not 'vout0'. Declare regulators for chips with a single ouput using
> > PMBUS_REGULATOR_ONE() to make that happen.
> 
> If we're gonna do this, we should go ahead and document each of these
> regulators. If Naresh sends a v2 for the tda, I could do the others.

As it turns out, the infineon devices in question are not PMICs, but
regulators themselves. I think something like the following is what we
need. Perhaps Zev or the 9elements guys can comment.

Cheers,
Conor.

commit 9cf2671c0b98c0a0dd599fe9152985d1320bf320
Author: Conor Dooley <conor.dooley@microchip.com>
Date:   Mon Feb 19 14:24:48 2024 +0000

    dt-bindings: regulator: promote infineon buck converters to their own binding
    
    These devices may need to make use of the common regulator properties,
    but these are not permitted while only documented in
    trivial-devices.yaml
    
    Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

diff --git a/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml b/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
new file mode 100644
index 000000000000..bb0114f7e13f
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/infineon,ir38060.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Infineon Buck Regulators with PMBUS interfaces
+
+maintainers:
+  - Not Me.
+
+allOf:
+  - $ref: regulator.yaml#
+
+properties:
+  compatible:
+    enum:
+      - infineon,ir38060
+      - infineon,ir38064
+      - infineon,ir38164
+      - infineon,ir38263
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      regulator@34 {
+        compatible = "infineon,ir38060";
+        reg = <0x34>;
+
+        regulator-min-microvolt = <437500>;
+        regulator-max-microvolt = <1387500>;
+      };
+    };
+
diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index 9cdd5a534120..e07be7bf8395 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -130,14 +130,6 @@ properties:
           - infineon,dps310
             # Infineon IR36021 digital POL buck controller
           - infineon,ir36021
-            # Infineon IR38060 Voltage Regulator
-          - infineon,ir38060
-            # Infineon IR38064 Voltage Regulator
-          - infineon,ir38064
-            # Infineon IR38164 Voltage Regulator
-          - infineon,ir38164
-            # Infineon IR38263 Voltage Regulator
-          - infineon,ir38263
             # Infineon IRPS5401 Voltage Regulator (PMIC)
           - infineon,irps5401
             # Infineon TLV493D-A1B6 I2C 3D Magnetic Sensor

commit 597739840d2bc95ebb884d7f5fc4f3cc206d9af6
Author: Conor Dooley <conor.dooley@microchip.com>
Date:   Mon Feb 19 13:29:12 2024 +0000

    dt-bindings: hwmon/pmbus: ti,lm25066: document regulators
    
    All devices documented in the lm25066 binding are intended for use with
    a regulator, be that for purely monitoring purposes (lm25056) or, for
    the other devices, as the controller of that regulator. The binding does
    not currently allow regulator child nodes, so add one.
    
    Each of these devices interacts with only a single regulator and
    documentation refers to it as "Vout", hence the choice of child node
    name.
    
    Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml
index da8292bc32f5..4373e9c86e54 100644
--- a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml
+++ b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml
@@ -34,6 +34,18 @@ properties:
       Shunt (sense) resistor value in micro-Ohms
     default: 1000
 
+  regulators:
+    type: object
+
+    properties:
+      vout:
+        $ref: /schemas/regulator/regulator.yaml#
+        type: object
+
+        unevaluatedProperties: false
+
+    additionalProperties: false
+
 required:
   - compatible
   - reg


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

Re: [PATCH 0/3] hwmon: (pmbus) Use PMBUS_REGULATOR_ONE to declare regulators with single output

[Guenter Roeck]

On 2/19/24 06:48, Conor Dooley wrote:
> On Sat, Feb 17, 2024 at 10:00:30PM +0000, Conor Dooley wrote:
>> On Wed, Feb 14, 2024 at 11:43:39AM -0800, Guenter Roeck wrote:
>>> If a chip only provides a single regulator, it should be named 'vout'
>>> and not 'vout0'. Declare regulators for chips with a single ouput using
>>> PMBUS_REGULATOR_ONE() to make that happen.
>>
>> If we're gonna do this, we should go ahead and document each of these
>> regulators. If Naresh sends a v2 for the tda, I could do the others.
> 
> As it turns out, the infineon devices in question are not PMICs, but
> regulators themselves. I think something like the following is what we
> need. Perhaps Zev or the 9elements guys can comment.
> 
IMO The lm25066 patch should be separate.

Guenter

> Cheers,
> Conor.
> 
> commit 9cf2671c0b98c0a0dd599fe9152985d1320bf320
> Author: Conor Dooley <conor.dooley@microchip.com>
> Date:   Mon Feb 19 14:24:48 2024 +0000
> 
>      dt-bindings: regulator: promote infineon buck converters to their own binding
>      
>      These devices may need to make use of the common regulator properties,
>      but these are not permitted while only documented in
>      trivial-devices.yaml
>      
>      Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> 
> diff --git a/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml b/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
> new file mode 100644
> index 000000000000..bb0114f7e13f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/infineon,ir38060.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Infineon Buck Regulators with PMBUS interfaces
> +
> +maintainers:
> +  - Not Me.
> +
> +allOf:
> +  - $ref: regulator.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - infineon,ir38060
> +      - infineon,ir38064
> +      - infineon,ir38164
> +      - infineon,ir38263
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      regulator@34 {
> +        compatible = "infineon,ir38060";
> +        reg = <0x34>;
> +
> +        regulator-min-microvolt = <437500>;
> +        regulator-max-microvolt = <1387500>;
> +      };
> +    };
> +
> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> index 9cdd5a534120..e07be7bf8395 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> @@ -130,14 +130,6 @@ properties:
>             - infineon,dps310
>               # Infineon IR36021 digital POL buck controller
>             - infineon,ir36021
> -            # Infineon IR38060 Voltage Regulator
> -          - infineon,ir38060
> -            # Infineon IR38064 Voltage Regulator
> -          - infineon,ir38064
> -            # Infineon IR38164 Voltage Regulator
> -          - infineon,ir38164
> -            # Infineon IR38263 Voltage Regulator
> -          - infineon,ir38263
>               # Infineon IRPS5401 Voltage Regulator (PMIC)
>             - infineon,irps5401
>               # Infineon TLV493D-A1B6 I2C 3D Magnetic Sensor
> 
> commit 597739840d2bc95ebb884d7f5fc4f3cc206d9af6
> Author: Conor Dooley <conor.dooley@microchip.com>
> Date:   Mon Feb 19 13:29:12 2024 +0000
> 
>      dt-bindings: hwmon/pmbus: ti,lm25066: document regulators
>      
>      All devices documented in the lm25066 binding are intended for use with
>      a regulator, be that for purely monitoring purposes (lm25056) or, for
>      the other devices, as the controller of that regulator. The binding does
>      not currently allow regulator child nodes, so add one.
>      
>      Each of these devices interacts with only a single regulator and
>      documentation refers to it as "Vout", hence the choice of child node
>      name.
>      
>      Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml > index da8292bc32f5..4373e9c86e54 100644
> --- a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml
> @@ -34,6 +34,18 @@ properties:
>         Shunt (sense) resistor value in micro-Ohms
>       default: 1000
>   
> +  regulators:
> +    type: object
> +
> +    properties:
> +      vout:
> +        $ref: /schemas/regulator/regulator.yaml#
> +        type: object
> +
> +        unevaluatedProperties: false
> +
> +    additionalProperties: false
> +
>   required:
>     - compatible
>     - reg
> 

Re: [PATCH 0/3] hwmon: (pmbus) Use PMBUS_REGULATOR_ONE to declare regulators with single output

[Conor Dooley]

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

On Mon, Feb 19, 2024 at 07:13:33AM -0800, Guenter Roeck wrote:
> On 2/19/24 06:48, Conor Dooley wrote:
> > On Sat, Feb 17, 2024 at 10:00:30PM +0000, Conor Dooley wrote:
> > > On Wed, Feb 14, 2024 at 11:43:39AM -0800, Guenter Roeck wrote:
> > > > If a chip only provides a single regulator, it should be named 'vout'
> > > > and not 'vout0'. Declare regulators for chips with a single ouput using
> > > > PMBUS_REGULATOR_ONE() to make that happen.
> > > 
> > > If we're gonna do this, we should go ahead and document each of these
> > > regulators. If Naresh sends a v2 for the tda, I could do the others.
> > 
> > As it turns out, the infineon devices in question are not PMICs, but
> > regulators themselves. I think something like the following is what we
> > need. Perhaps Zev or the 9elements guys can comment.
> > 
> IMO The lm25066 patch should be separate.

It should already be a pair of changes - one for the infineon stuff and
another for the ti devices:

> > commit 9cf2671c0b98c0a0dd599fe9152985d1320bf320
> > Author: Conor Dooley <conor.dooley@microchip.com>
> > Date:   Mon Feb 19 14:24:48 2024 +0000
> > 
> >      dt-bindings: regulator: promote infineon buck converters to their own binding
> >      These devices may need to make use of the common regulator properties,
> >      but these are not permitted while only documented in
> >      trivial-devices.yaml
> >      Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

> > commit 597739840d2bc95ebb884d7f5fc4f3cc206d9af6
> > Author: Conor Dooley <conor.dooley@microchip.com>
> > Date:   Mon Feb 19 13:29:12 2024 +0000
> > 
> >      dt-bindings: hwmon/pmbus: ti,lm25066: document regulators
> >      All devices documented in the lm25066 binding are intended for use with
> >      a regulator, be that for purely monitoring purposes (lm25056) or, for
> >      the other devices, as the controller of that regulator. The binding does
> >      not currently allow regulator child nodes, so add one.
> >      Each of these devices interacts with only a single regulator and
> >      documentation refers to it as "Vout", hence the choice of child node
> >      name.
> >      Signed-off-by: Conor Dooley <conor.dooley@microchip.com>


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

Re: [PATCH 0/3] hwmon: (pmbus) Use PMBUS_REGULATOR_ONE to declare regulators with single output

[Zev Weiss]

On Wed, Feb 21, 2024 at 11:06:53AM PST, Conor Dooley wrote:
>On Mon, Feb 19, 2024 at 07:13:33AM -0800, Guenter Roeck wrote:
>> On 2/19/24 06:48, Conor Dooley wrote:
>> > On Sat, Feb 17, 2024 at 10:00:30PM +0000, Conor Dooley wrote:
>> > > On Wed, Feb 14, 2024 at 11:43:39AM -0800, Guenter Roeck wrote:
>> > > > If a chip only provides a single regulator, it should be named 'vout'
>> > > > and not 'vout0'. Declare regulators for chips with a single ouput using
>> > > > PMBUS_REGULATOR_ONE() to make that happen.
>> > >
>> > > If we're gonna do this, we should go ahead and document each of these
>> > > regulators. If Naresh sends a v2 for the tda, I could do the others.
>> >
>> > As it turns out, the infineon devices in question are not PMICs, but
>> > regulators themselves. I think something like the following is what we
>> > need. Perhaps Zev or the 9elements guys can comment.
>> >
>> IMO The lm25066 patch should be separate.
>
>It should already be a pair of changes - one for the infineon stuff and
>another for the ti devices:
>
>> > commit 9cf2671c0b98c0a0dd599fe9152985d1320bf320
>> > Author: Conor Dooley <conor.dooley@microchip.com>
>> > Date:   Mon Feb 19 14:24:48 2024 +0000
>> >
>> >      dt-bindings: regulator: promote infineon buck converters to their own binding
>> >      These devices may need to make use of the common regulator properties,
>> >      but these are not permitted while only documented in
>> >      trivial-devices.yaml
>> >      Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>
>> > commit 597739840d2bc95ebb884d7f5fc4f3cc206d9af6
>> > Author: Conor Dooley <conor.dooley@microchip.com>
>> > Date:   Mon Feb 19 13:29:12 2024 +0000
>> >
>> >      dt-bindings: hwmon/pmbus: ti,lm25066: document regulators
>> >      All devices documented in the lm25066 binding are intended for use with
>> >      a regulator, be that for purely monitoring purposes (lm25056) or, for
>> >      the other devices, as the controller of that regulator. The binding does
>> >      not currently allow regulator child nodes, so add one.
>> >      Each of these devices interacts with only a single regulator and
>> >      documentation refers to it as "Vout", hence the choice of child node
>> >      name.
>> >      Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>

This looks fine to me -- as far as the lm25066 patch goes,

Acked-by: Zev Weiss <zev@bewilderbeest.net>


Thanks,
Zev