From: Conor Dooley <[email protected]>
A v2 of Guenter's series that changed the name of regulators registered
by PBMUS. The documenation (offical, not bindings) doesn't call the
output Vout0 and there were no bindings for these devices that allowed
regulator child nodes.
Document the regular child nodes for the TI devices and allow regulator
properties for the infineon ones.
Naresh is already working on the tda38640, so there's nothing for that
here DT wise.
Cheers,
Conor.
CC: Jean Delvare <[email protected]>
CC: Guenter Roeck <[email protected]>
CC: Rob Herring <[email protected]>
CC: Krzysztof Kozlowski <[email protected]>
CC: Conor Dooley <[email protected]>
CC: Liam Girdwood <[email protected]>
CC: Mark Brown <[email protected]>
CC: Zev Weiss <[email protected]>
CC: Patrick Rudolph <[email protected]>
CC: Peter Yin <[email protected]>
CC: Alexander Stein <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
Conor Dooley (2):
dt-bindings: hwmon/pmbus: ti,lm25066: document regulators
regulator: dt-bindings: promote infineon buck converters to their own
binding
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
.../bindings/hwmon/pmbus/ti,lm25066.yaml | 12 +++++
.../bindings/regulator/infineon,ir38060.yaml | 46 +++++++++++++++++++
.../devicetree/bindings/trivial-devices.yaml | 8 ----
drivers/hwmon/pmbus/ir38064.c | 2 +-
drivers/hwmon/pmbus/lm25066.c | 2 +-
drivers/hwmon/pmbus/tda38640.c | 2 +-
6 files changed, 61 insertions(+), 11 deletions(-)
create mode 100644 Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
--
2.43.0
From: Conor Dooley <[email protected]>
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.
Acked-by: Zev Weiss <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
---
.../devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml | 12 ++++++++++++
1 file changed, 12 insertions(+)
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
--
2.43.0
From: Conor Dooley <[email protected]>
These devices are regulators 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 <[email protected]>
---
.../bindings/regulator/infineon,ir38060.yaml | 46 +++++++++++++++++++
.../devicetree/bindings/trivial-devices.yaml | 8 ----
2 files changed, 46 insertions(+), 8 deletions(-)
create mode 100644 Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
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
--
2.43.0
From: Guenter Roeck <[email protected]>
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 <[email protected]>
Cc: Naresh Solanki <[email protected]>
Cc: Patrick Rudolph <[email protected]>
Signed-off-by: Guenter Roeck <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
---
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.43.0
From: Guenter Roeck <[email protected]>
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 <[email protected]>
Cc: Naresh Solanki <[email protected]>
Cc: Patrick Rudolph <[email protected]>
Signed-off-by: Guenter Roeck <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
---
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.43.0
From: Guenter Roeck <[email protected]>
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 <[email protected]>
Cc: Naresh Solanki <[email protected]>
Cc: Zev Weiss <[email protected]>
Signed-off-by: Guenter Roeck <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
---
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.43.0
On Fri, Feb 23, 2024 at 08:21:08AM PST, Conor Dooley wrote:
>From: Guenter Roeck <[email protected]>
>
>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.
>
As mentioned on Guenter's v1, this change necessitates a corresponding
update to arch/arm/boot/dts/aspeed/aspeed-bmc-delta-ahe50dc.dts, which
has a dependency on the name of the regulator. Given (AFAICT) the lack
of any combined dts & driver patches anywhere in the kernel git history
I guess maybe doing both atomically in a single commit might not be
considered kosher, but could it at least be included in the same patch
series?
Thanks,
Zev
On Fri, Feb 23, 2024 at 03:16:28PM PST, Zev Weiss wrote:
>On Fri, Feb 23, 2024 at 08:21:08AM PST, Conor Dooley wrote:
>>From: Guenter Roeck <[email protected]>
>>
>>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.
>>
>
>Given (AFAICT) the >lack of any combined dts & driver patches anywhere
>in the kernel git history I guess maybe doing both atomically in a
>single commit might not be considered kosher, but could it at least be
>included in the same patch series?
>
Ah, except I realize now I neglected to pass '--full-diff' to 'git log'
when checking that, and after fixing that I see there is in fact some
precedent for commits changing device-trees and driver code together, so
ideally that would be my preference here too.
Thanks,
Zev
On 2/23/24 15:26, Zev Weiss wrote:
> On Fri, Feb 23, 2024 at 03:16:28PM PST, Zev Weiss wrote:
>> On Fri, Feb 23, 2024 at 08:21:08AM PST, Conor Dooley wrote:
>>> From: Guenter Roeck <[email protected]>
>>>
>>> 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.
>>>
>
>
>>
>> Given (AFAICT) the >lack of any combined dts & driver patches anywhere in the kernel git history I guess maybe doing both atomically in a single commit might not be considered kosher, but could it at least be included in the same patch series?
>>
>
> Ah, except I realize now I neglected to pass '--full-diff' to 'git log' when checking that, and after fixing that I see there is in fact some precedent for commits changing device-trees and driver code together, so ideally that would be my preference here too.
>
That is not going to happen in the hwmon subsystem unless something slips by.
In a large project like the Linux kernel you'll find precedents for everything,
so citing one is not a valid argument.
As a general rule, I don't apply patches in .dts[i] files through the hwmon
branch at all, not even as part of a patch series. Architecture maintainers
tend to strongly oppose that idea, for the simple reason that it creates
the risk of merge conflicts and thus of bugs during commit windows.
Guenter
On 23/02/2024 17:21, Conor Dooley wrote:
> From: Conor Dooley <[email protected]>
>
> 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.
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On 23/02/2024 17:21, Conor Dooley wrote:
> From: Conor Dooley <[email protected]>
>
> These devices are regulators 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 <[email protected]>
> ---
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On Fri, Feb 23, 2024 at 04:21:04PM +0000, Conor Dooley wrote:
> From: Conor Dooley <[email protected]>
>
> A v2 of Guenter's series that changed the name of regulators registered
> by PBMUS. The documenation (offical, not bindings) doesn't call the
> output Vout0 and there were no bindings for these devices that allowed
> regulator child nodes.
>
> Document the regular child nodes for the TI devices and allow regulator
> properties for the infineon ones.
>
> Naresh is already working on the tda38640, so there's nothing for that
> here DT wise.
>
Series applied to hwmon-next.
Thanks,
Guenter
On Fri, Feb 23, 2024 at 04:21:06PM +0000, Conor Dooley wrote:
> From: Conor Dooley <[email protected]>
>
> These devices are regulators may need to make use of the common
> regulator properties, but these are not permitted while only documented
> in trivial-devices.yaml
Reviewed-by: Mark Brown <[email protected]>
On Fri, Feb 23, 2024 at 03:16:26PM -0800, Zev Weiss wrote:
> On Fri, Feb 23, 2024 at 08:21:08AM PST, Conor Dooley wrote:
> > From: Guenter Roeck <[email protected]>
> >
> > 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.
> >
>
> As mentioned on Guenter's v1, this change necessitates a corresponding
> update to arch/arm/boot/dts/aspeed/aspeed-bmc-delta-ahe50dc.dts, which has a
> dependency on the name of the regulator. Given (AFAICT) the lack of any
> combined dts & driver patches anywhere in the kernel git history I guess
> maybe doing both atomically in a single commit might not be considered
> kosher, but could it at least be included in the same patch series?
I was gonna do it in isolation if this went ahead, but I see you went
and did it yourself - thanks. As Guenter pointed out later in the
thread, any patch that combines dts changes with drivers or bindings
is something that likely snuck in or was some sort of extenuating
circumstance.
Cheers,
Conor.