* [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding
@ 2025-02-07 13:28 Naresh Solanki
2025-02-07 13:28 ` [PATCH 2/2] ARM: dts: aspeed: sbp1: Align regulator node with Infineon ir38060 Naresh Solanki
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: Naresh Solanki @ 2025-02-07 13:28 UTC (permalink / raw)
To: Guenter Roeck, broonie, conor, Jean Delvare, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Patrick Rudolph,
Naresh Solanki, Andrew Jeffery
Cc: linux-hwmon, Naresh Solanki, devicetree, linux-kernel
Move dt binding under hwmon/pmbus & align accordingly.
Previously the DT binding was invalid & wouldn't work with pmbus driver.
Pmbus driver expects a regulator node & hence added the same.
Fixes: 1d333cd641fb ("ARM: dts: aspeed: sbp1: IBM sbp1 BMC board")
Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com>
---
Changes in V2:
1. Update commit message
2. Add Fixes tags
---
.../hwmon/pmbus/infineon,ir38060.yaml | 61 +++++++++++++++++++
.../bindings/regulator/infineon,ir38060.yaml | 45 --------------
2 files changed, 61 insertions(+), 45 deletions(-)
create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
delete mode 100644 Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
new file mode 100644
index 000000000000..e1f683846a54
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/pmbus/infineon,ir38060.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Infineon Buck Regulators with PMBUS interfaces
+
+maintainers:
+ - Not Me.
+
+properties:
+ compatible:
+ enum:
+ - infineon,ir38060
+ - infineon,ir38064
+ - infineon,ir38164
+ - infineon,ir38263
+
+ reg:
+ maxItems: 1
+
+ regulators:
+ type: object
+ description:
+ list of regulators provided by this controller.
+
+ properties:
+ vout:
+ $ref: /schemas/regulator/regulator.yaml#
+ type: object
+
+ unevaluatedProperties: false
+
+ additionalProperties: false
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ regulator@34 {
+ compatible = "infineon,ir38060";
+ reg = <0x34>;
+
+ regulators {
+ vout {
+ regulator-name = "p5v_aux";
+ regulator-min-microvolt = <437500>;
+ regulator-max-microvolt = <1387500>;
+ };
+ };
+ };
+ };
diff --git a/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml b/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
deleted file mode 100644
index e6ffbc2a2298..000000000000
--- a/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
+++ /dev/null
@@ -1,45 +0,0 @@
-# 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>;
- };
- };
base-commit: bfbb730c4255e1965d202f48e7aa71baa9a7c65b
--
2.42.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 2/2] ARM: dts: aspeed: sbp1: Align regulator node with Infineon ir38060
2025-02-07 13:28 [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding Naresh Solanki
@ 2025-02-07 13:28 ` Naresh Solanki
2025-02-07 15:21 ` [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding Rob Herring (Arm)
2025-02-12 17:57 ` Rob Herring
2 siblings, 0 replies; 19+ messages in thread
From: Naresh Solanki @ 2025-02-07 13:28 UTC (permalink / raw)
To: Guenter Roeck, broonie, conor, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery
Cc: linux-hwmon, Naresh Solanki, devicetree, linux-arm-kernel,
linux-aspeed, linux-kernel
The PMBus driver expects a regulator node, which was missing in the
board's device tree. This was corrected in the latest device binding
update for ir38060.yaml.
Update the board's DT binding accordingly to align with the fixed
device binding and ensure proper regulator support.
Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com>
---
Changes in V2:
1. Update commit message
---
.../boot/dts/aspeed/aspeed-bmc-ibm-sbp1.dts | 124 +++++++++++-------
1 file changed, 80 insertions(+), 44 deletions(-)
diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-sbp1.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-sbp1.dts
index 8d98be3d5f2e..34f3d773a775 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-sbp1.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-sbp1.dts
@@ -1838,13 +1838,17 @@ i2c@2 {
#address-cells = <1>;
#size-cells = <0>;
- pvcore_nic2: ir38263-pvcore-nic2@40 {
+ ir38263_pvcore_nic2: ir38263-pvcore-nic2@40 {
compatible = "infineon,ir38263";
reg = <0x40>;
- regulator-name = "pvcore_nic2";
- regulator-enable-ramp-delay = <2000>;
- vin-supply = <&p12v>;
+ regulators {
+ pvcore_nic2: vout {
+ regulator-name = "pvcore_nic2";
+ regulator-enable-ramp-delay = <2000>;
+ vin-supply = <&p12v>;
+ };
+ };
};
};
@@ -1853,13 +1857,17 @@ i2c@3 {
#address-cells = <1>;
#size-cells = <0>;
- pvcore_nic1: ir38263-pvcore-nic1@40 {
+ ir38263_pvcore_nic1: ir38263-pvcore-nic1@40 {
compatible = "infineon,ir38263";
reg = <0x40>;
- regulator-name = "pvcore_nic1";
- regulator-enable-ramp-delay = <2000>;
- vin-supply = <&p12v>;
+ regulators {
+ pvcore_nic1: vout {
+ regulator-name = "pvcore_nic1";
+ regulator-enable-ramp-delay = <2000>;
+ vin-supply = <&p12v>;
+ };
+ };
};
};
@@ -1874,13 +1882,17 @@ i2c@5 {
#address-cells = <1>;
#size-cells = <0>;
- p3v3_nic: ir38263-p3v3-nic@40 {
+ ir38263_p3v3_nic: ir38263-p3v3-nic@40 {
compatible = "infineon,ir38263";
reg = <0x40>;
- regulator-name = "p3v3_nic";
- regulator-enable-ramp-delay = <2000>;
- vin-supply = <&p12v>;
+ regulators {
+ p3v3_nic: vout {
+ regulator-name = "p3v3_nic";
+ regulator-enable-ramp-delay = <2000>;
+ vin-supply = <&p12v>;
+ };
+ };
};
};
@@ -1889,13 +1901,17 @@ i2c@6 {
#address-cells = <1>;
#size-cells = <0>;
- p1v2_nic: ir38263-p1v2-nic@40 {
+ ir38263_p1v2_nic: ir38263-p1v2-nic@40 {
compatible = "infineon,ir38263";
reg = <0x40>;
- regulator-name = "p1v2_nic";
- regulator-enable-ramp-delay = <2000>;
- vin-supply = <&p12v>;
+ regulators {
+ p1v2_nic: vout {
+ regulator-name = "p1v2_nic";
+ regulator-enable-ramp-delay = <2000>;
+ vin-supply = <&p12v>;
+ };
+ };
};
};
@@ -1904,13 +1920,17 @@ i2c@7 {
#address-cells = <1>;
#size-cells = <0>;
- p1v8_nic: ir38263-p1v8-nic@40 {
+ ir38263_p1v8_nic: ir38263-p1v8-nic@40 {
compatible = "infineon,ir38263";
reg = <0x40>;
- regulator-name = "p1v8_nic";
- regulator-enable-ramp-delay = <2000>;
- vin-supply = <&p12v>;
+ regulators {
+ p1v8_nic: vout {
+ regulator-name = "p1v8_nic";
+ regulator-enable-ramp-delay = <2000>;
+ vin-supply = <&p12v>;
+ };
+ };
};
};
};
@@ -2070,13 +2090,17 @@ i2c@1 {
#address-cells = <1>;
#size-cells = <0>;
- p1v05_pch_aux: ir38263-p1v05-pch-aux@40 {
+ ir38263_p1v05_pch_aux: ir38263-p1v05-pch-aux@40 {
compatible = "infineon,ir38263";
reg = <0x40>;
- regulator-name = "p1v05_pch_aux";
- regulator-enable-ramp-delay = <2000>;
- vin-supply = <&p12v>;
+ regulators {
+ p1v05_pch_aux: vout {
+ regulator-name = "p1v05_pch_aux";
+ regulator-enable-ramp-delay = <2000>;
+ vin-supply = <&p12v>;
+ };
+ };
};
};
@@ -2085,13 +2109,17 @@ i2c@2 {
#address-cells = <1>;
#size-cells = <0>;
- p1v8_pch_aux: ir38060-p1v8-pch-aux@40 {
+ ir38060_p1v8_pch_aux: ir38060-p1v8-pch-aux@40 {
compatible = "infineon,ir38060";
reg = <0x40>;
- regulator-name = "p1v8_pch_aux";
- regulator-enable-ramp-delay = <2000>;
- vin-supply = <&p12v>;
+ regulators {
+ p1v8_pch_aux: vout {
+ regulator-name = "p1v8_pch_aux";
+ regulator-enable-ramp-delay = <2000>;
+ vin-supply = <&p12v>;
+ };
+ };
};
};
@@ -3596,34 +3624,42 @@ i2c@1 {
reg = <1>;
#address-cells = <1>;
#size-cells = <0>;
- p5v_aux: ir38263-p5v-aux@40 {
+ ir38263_p5v_aux: ir38263-p5v-aux@40 {
compatible = "infineon,ir38263";
reg = <0x40>;
- regulator-name = "p5v_aux";
- regulator-enable-ramp-delay = <2000>;
- vin-supply = <&p12v>;
- vbus-supply = <&p3v3_bmc_aux>;
- regulator-always-on;
- regulator-boot-on;
+ regulators {
+ p5v_aux: vout {
+ regulator-name = "p5v_aux";
+ regulator-enable-ramp-delay = <2000>;
+ vin-supply = <&p12v>;
+ vbus-supply = <&p3v3_bmc_aux>;
+ regulator-always-on;
+ regulator-boot-on;
+ };
+ };
};
};
i2c@2 {
reg = <2>;
#address-cells = <1>;
#size-cells = <0>;
- p3v3_aux: ir38263-p3v3-aux@40 {
+ ir38263_p3v3_aux: ir38263-p3v3-aux@40 {
compatible = "infineon,ir38263";
reg = <0x40>;
- vin-supply = <&p12v>;
- regulator-name = "p3v3_aux";
- /*
- * 2msec for regulator + 18msec for board capacitance
- * Note: Every IC has a PTC which slowly charges the bypass
- * cap.
- */
- regulator-enable-ramp-delay = <200000>;
+ regulators {
+ p3v3_aux: vout {
+ regulator-name = "p3v3_aux";
+ /*
+ * 2msec for regulator + 18msec for board capacitance
+ * Note: Every IC has a PTC which slowly charges the bypass
+ * cap.
+ */
+ vin-supply = <&p12v>;
+ regulator-enable-ramp-delay = <200000>;
+ };
+ };
};
};
i2c@3 {
--
2.42.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding
2025-02-07 13:28 [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding Naresh Solanki
2025-02-07 13:28 ` [PATCH 2/2] ARM: dts: aspeed: sbp1: Align regulator node with Infineon ir38060 Naresh Solanki
@ 2025-02-07 15:21 ` Rob Herring (Arm)
2025-02-12 17:57 ` Rob Herring
2 siblings, 0 replies; 19+ messages in thread
From: Rob Herring (Arm) @ 2025-02-07 15:21 UTC (permalink / raw)
To: Naresh Solanki
Cc: Krzysztof Kozlowski, Jean Delvare, Andrew Jeffery, devicetree,
Patrick Rudolph, linux-hwmon, linux-kernel, conor, broonie,
Liam Girdwood, Naresh Solanki, Conor Dooley, Guenter Roeck
On Fri, 07 Feb 2025 18:58:03 +0530, Naresh Solanki wrote:
> Move dt binding under hwmon/pmbus & align accordingly.
>
> Previously the DT binding was invalid & wouldn't work with pmbus driver.
> Pmbus driver expects a regulator node & hence added the same.
>
> Fixes: 1d333cd641fb ("ARM: dts: aspeed: sbp1: IBM sbp1 BMC board")
> Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com>
> ---
> Changes in V2:
> 1. Update commit message
> 2. Add Fixes tags
> ---
> .../hwmon/pmbus/infineon,ir38060.yaml | 61 +++++++++++++++++++
> .../bindings/regulator/infineon,ir38060.yaml | 45 --------------
> 2 files changed, 61 insertions(+), 45 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> delete mode 100644 Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
>
My bot found new DTB warnings on the .dts files added or changed in this
series.
Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.
If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:
pip3 install dtschema --upgrade
New warnings running 'make CHECK_DTBS=y for arch/arm/boot/dts/aspeed/' for 20250207132806.3113268-1-naresh.solanki@9elements.com:
arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-bletchley.dtb: ethernet-phy@0: 'reg' is a required property
from schema $id: http://devicetree.org/schemas/net/ethernet-phy.yaml#
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding
2025-02-07 13:28 [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding Naresh Solanki
2025-02-07 13:28 ` [PATCH 2/2] ARM: dts: aspeed: sbp1: Align regulator node with Infineon ir38060 Naresh Solanki
2025-02-07 15:21 ` [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding Rob Herring (Arm)
@ 2025-02-12 17:57 ` Rob Herring
2025-02-12 18:33 ` Conor Dooley
2 siblings, 1 reply; 19+ messages in thread
From: Rob Herring @ 2025-02-12 17:57 UTC (permalink / raw)
To: Naresh Solanki
Cc: Guenter Roeck, broonie, conor, Jean Delvare, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Patrick Rudolph, Andrew Jeffery,
linux-hwmon, devicetree, linux-kernel
On Fri, Feb 07, 2025 at 06:58:03PM +0530, Naresh Solanki wrote:
> Move dt binding under hwmon/pmbus & align accordingly.
>
> Previously the DT binding was invalid & wouldn't work with pmbus driver.
> Pmbus driver expects a regulator node & hence added the same.
2 out of 3 (schema, dts, driver) agree. Fix the driver.
>
> Fixes: 1d333cd641fb ("ARM: dts: aspeed: sbp1: IBM sbp1 BMC board")
> Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com>
> ---
> Changes in V2:
> 1. Update commit message
> 2. Add Fixes tags
> ---
> .../hwmon/pmbus/infineon,ir38060.yaml | 61 +++++++++++++++++++
> .../bindings/regulator/infineon,ir38060.yaml | 45 --------------
> 2 files changed, 61 insertions(+), 45 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> delete mode 100644 Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
>
> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> new file mode 100644
> index 000000000000..e1f683846a54
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/pmbus/infineon,ir38060.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Infineon Buck Regulators with PMBUS interfaces
> +
> +maintainers:
> + - Not Me.
Nothing new here, but WTF. Expect a meta-schema change to warn on this.
Rob
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding
2025-02-12 17:57 ` Rob Herring
@ 2025-02-12 18:33 ` Conor Dooley
0 siblings, 0 replies; 19+ messages in thread
From: Conor Dooley @ 2025-02-12 18:33 UTC (permalink / raw)
To: Rob Herring
Cc: Naresh Solanki, Guenter Roeck, broonie, Jean Delvare,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Patrick Rudolph,
Andrew Jeffery, linux-hwmon, devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1868 bytes --]
On Wed, Feb 12, 2025 at 11:57:13AM -0600, Rob Herring wrote:
> On Fri, Feb 07, 2025 at 06:58:03PM +0530, Naresh Solanki wrote:
> > Move dt binding under hwmon/pmbus & align accordingly.
> >
> > Previously the DT binding was invalid & wouldn't work with pmbus driver.
> > Pmbus driver expects a regulator node & hence added the same.
>
> 2 out of 3 (schema, dts, driver) agree. Fix the driver.
>
> >
> > Fixes: 1d333cd641fb ("ARM: dts: aspeed: sbp1: IBM sbp1 BMC board")
> > Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com>
> > ---
> > Changes in V2:
> > 1. Update commit message
> > 2. Add Fixes tags
> > ---
> > .../hwmon/pmbus/infineon,ir38060.yaml | 61 +++++++++++++++++++
> > .../bindings/regulator/infineon,ir38060.yaml | 45 --------------
> > 2 files changed, 61 insertions(+), 45 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> > delete mode 100644 Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> > new file mode 100644
> > index 000000000000..e1f683846a54
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> > @@ -0,0 +1,61 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/hwmon/pmbus/infineon,ir38060.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Infineon Buck Regulators with PMBUS interfaces
> > +
> > +maintainers:
> > + - Not Me.
>
> Nothing new here, but WTF. Expect a meta-schema change to warn on this.
And it was fucking me that did it, that's the best/worst part about it.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding
@ 2025-02-04 18:03 Naresh Solanki
2025-02-04 19:22 ` Conor Dooley
2025-02-04 23:34 ` Rob Herring (Arm)
0 siblings, 2 replies; 19+ messages in thread
From: Naresh Solanki @ 2025-02-04 18:03 UTC (permalink / raw)
To: Guenter Roeck, broonie, Jean Delvare, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood
Cc: linux-hwmon, Naresh Solanki, devicetree, linux-kernel
Move dt binding under hwmon/pmbus & align accordingly.
Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com>
---
.../hwmon/pmbus/infineon,ir38060.yaml | 61 +++++++++++++++++++
.../bindings/regulator/infineon,ir38060.yaml | 45 --------------
2 files changed, 61 insertions(+), 45 deletions(-)
create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
delete mode 100644 Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
new file mode 100644
index 000000000000..e1f683846a54
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/pmbus/infineon,ir38060.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Infineon Buck Regulators with PMBUS interfaces
+
+maintainers:
+ - Not Me.
+
+properties:
+ compatible:
+ enum:
+ - infineon,ir38060
+ - infineon,ir38064
+ - infineon,ir38164
+ - infineon,ir38263
+
+ reg:
+ maxItems: 1
+
+ regulators:
+ type: object
+ description:
+ list of regulators provided by this controller.
+
+ properties:
+ vout:
+ $ref: /schemas/regulator/regulator.yaml#
+ type: object
+
+ unevaluatedProperties: false
+
+ additionalProperties: false
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ regulator@34 {
+ compatible = "infineon,ir38060";
+ reg = <0x34>;
+
+ regulators {
+ vout {
+ regulator-name = "p5v_aux";
+ regulator-min-microvolt = <437500>;
+ regulator-max-microvolt = <1387500>;
+ };
+ };
+ };
+ };
diff --git a/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml b/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
deleted file mode 100644
index e6ffbc2a2298..000000000000
--- a/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
+++ /dev/null
@@ -1,45 +0,0 @@
-# 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>;
- };
- };
base-commit: bfbb730c4255e1965d202f48e7aa71baa9a7c65b
--
2.42.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding
2025-02-04 18:03 Naresh Solanki
@ 2025-02-04 19:22 ` Conor Dooley
2025-02-05 10:21 ` Naresh Solanki
2025-02-04 23:34 ` Rob Herring (Arm)
1 sibling, 1 reply; 19+ messages in thread
From: Conor Dooley @ 2025-02-04 19:22 UTC (permalink / raw)
To: Naresh Solanki
Cc: Guenter Roeck, broonie, Jean Delvare, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, linux-hwmon,
devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3952 bytes --]
On Tue, Feb 04, 2025 at 11:33:03PM +0530, Naresh Solanki wrote:
> Move dt binding under hwmon/pmbus & align accordingly.
>
> Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com>
> ---
> .../hwmon/pmbus/infineon,ir38060.yaml | 61 +++++++++++++++++++
> .../bindings/regulator/infineon,ir38060.yaml | 45 --------------
> 2 files changed, 61 insertions(+), 45 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> delete mode 100644 Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
>
> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> new file mode 100644
> index 000000000000..e1f683846a54
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/pmbus/infineon,ir38060.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Infineon Buck Regulators with PMBUS interfaces
> +
> +maintainers:
> + - Not Me.
How the hell did this get merged!
> +
> +properties:
> + compatible:
> + enum:
> + - infineon,ir38060
> + - infineon,ir38064
> + - infineon,ir38164
> + - infineon,ir38263
> +
> + reg:
> + maxItems: 1
> +
> + regulators:
> + type: object
> + description:
> + list of regulators provided by this controller.
Can you explain why this change is justified? Your commit message is
explaining what you're doing but not why it's okay to do.
Cheers,
Conor.
> +
> + properties:
> + vout:
> + $ref: /schemas/regulator/regulator.yaml#
> + type: object
> +
> + unevaluatedProperties: false
> +
> + additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + regulator@34 {
> + compatible = "infineon,ir38060";
> + reg = <0x34>;
> +
> + regulators {
> + vout {
> + regulator-name = "p5v_aux";
> + regulator-min-microvolt = <437500>;
> + regulator-max-microvolt = <1387500>;
> + };
> + };
> + };
> + };
> diff --git a/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml b/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
> deleted file mode 100644
> index e6ffbc2a2298..000000000000
> --- a/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
> +++ /dev/null
> @@ -1,45 +0,0 @@
> -# 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>;
> - };
> - };
>
> base-commit: bfbb730c4255e1965d202f48e7aa71baa9a7c65b
> --
> 2.42.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding
2025-02-04 19:22 ` Conor Dooley
@ 2025-02-05 10:21 ` Naresh Solanki
2025-02-05 20:13 ` Conor Dooley
0 siblings, 1 reply; 19+ messages in thread
From: Naresh Solanki @ 2025-02-05 10:21 UTC (permalink / raw)
To: Conor Dooley
Cc: Guenter Roeck, broonie, Jean Delvare, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, linux-hwmon,
devicetree, linux-kernel
Hi Conor,
On Wed, 5 Feb 2025 at 00:52, Conor Dooley <conor@kernel.org> wrote:
>
> On Tue, Feb 04, 2025 at 11:33:03PM +0530, Naresh Solanki wrote:
> > Move dt binding under hwmon/pmbus & align accordingly.
> >
> > Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com>
> > ---
> > .../hwmon/pmbus/infineon,ir38060.yaml | 61 +++++++++++++++++++
> > .../bindings/regulator/infineon,ir38060.yaml | 45 --------------
> > 2 files changed, 61 insertions(+), 45 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> > delete mode 100644 Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> > new file mode 100644
> > index 000000000000..e1f683846a54
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> > @@ -0,0 +1,61 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/hwmon/pmbus/infineon,ir38060.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Infineon Buck Regulators with PMBUS interfaces
> > +
> > +maintainers:
> > + - Not Me.
>
> How the hell did this get merged!
>
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - infineon,ir38060
> > + - infineon,ir38064
> > + - infineon,ir38164
> > + - infineon,ir38263
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + regulators:
> > + type: object
> > + description:
> > + list of regulators provided by this controller.
>
> Can you explain why this change is justified? Your commit message is
> explaining what you're doing but not why it's okay to do.
This is based on other similar dt-bindings under hwmon/pmbus.
Regards,
Naresh
>
> Cheers,
> Conor.
>
> > +
> > + properties:
> > + vout:
> > + $ref: /schemas/regulator/regulator.yaml#
> > + type: object
> > +
> > + unevaluatedProperties: false
> > +
> > + additionalProperties: false
> > +
> > +required:
> > + - compatible
> > + - reg
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > + - |
> > + i2c {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + regulator@34 {
> > + compatible = "infineon,ir38060";
> > + reg = <0x34>;
> > +
> > + regulators {
> > + vout {
> > + regulator-name = "p5v_aux";
> > + regulator-min-microvolt = <437500>;
> > + regulator-max-microvolt = <1387500>;
> > + };
> > + };
> > + };
> > + };
> > diff --git a/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml b/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
> > deleted file mode 100644
> > index e6ffbc2a2298..000000000000
> > --- a/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
> > +++ /dev/null
> > @@ -1,45 +0,0 @@
> > -# 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>;
> > - };
> > - };
> >
> > base-commit: bfbb730c4255e1965d202f48e7aa71baa9a7c65b
> > --
> > 2.42.0
> >
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding
2025-02-05 10:21 ` Naresh Solanki
@ 2025-02-05 20:13 ` Conor Dooley
2025-02-06 15:53 ` Naresh Solanki
0 siblings, 1 reply; 19+ messages in thread
From: Conor Dooley @ 2025-02-05 20:13 UTC (permalink / raw)
To: Naresh Solanki
Cc: Guenter Roeck, broonie, Jean Delvare, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, linux-hwmon,
devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 5502 bytes --]
On Wed, Feb 05, 2025 at 03:51:25PM +0530, Naresh Solanki wrote:
> On Wed, 5 Feb 2025 at 00:52, Conor Dooley <conor@kernel.org> wrote:
> > On Tue, Feb 04, 2025 at 11:33:03PM +0530, Naresh Solanki wrote:
> > > Move dt binding under hwmon/pmbus & align accordingly.
> > >
> > > Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com>
> > > ---
> > > .../hwmon/pmbus/infineon,ir38060.yaml | 61 +++++++++++++++++++
> > > .../bindings/regulator/infineon,ir38060.yaml | 45 --------------
> > > 2 files changed, 61 insertions(+), 45 deletions(-)
> > > create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> > > delete mode 100644 Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> > > new file mode 100644
> > > index 000000000000..e1f683846a54
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> > > @@ -0,0 +1,61 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/hwmon/pmbus/infineon,ir38060.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Infineon Buck Regulators with PMBUS interfaces
> > > +
> > > +maintainers:
> > > + - Not Me.
> >
> > How the hell did this get merged!
> >
> > > +
> > > +properties:
> > > + compatible:
> > > + enum:
> > > + - infineon,ir38060
> > > + - infineon,ir38064
> > > + - infineon,ir38164
> > > + - infineon,ir38263
> > > +
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + regulators:
> > > + type: object
> > > + description:
> > > + list of regulators provided by this controller.
> >
> > Can you explain why this change is justified? Your commit message is
> > explaining what you're doing but not why it's okay to do.
> This is based on other similar dt-bindings under hwmon/pmbus.
Okay, but what I am looking for is an explanation of why it is okay to
change the node from
| regulator@34 {
| compatible = "infineon,ir38060";
| reg = <0x34>;
|
| regulator-min-microvolt = <437500>;
| regulator-max-microvolt = <1387500>;
| };
to
| regulator@34 {
| compatible = "infineon,ir38060";
| reg = <0x34>;
|
| regulators {
| vout {
| regulator-name = "p5v_aux";
| regulator-min-microvolt = <437500>;
| regulator-max-microvolt = <1387500>;
| };
| };
?
Will the driver handle both of these identically? Is backwards
compatibility with the old format maintained? Was the original format
wrong and does not work? Why is a list of regulators needed when the
device only provides one?
Cheers,
Conor.
> > > + properties:
> > > + vout:
> > > + $ref: /schemas/regulator/regulator.yaml#
> > > + type: object
> > > +
> > > + unevaluatedProperties: false
> > > +
> > > + additionalProperties: false
> > > +
> > > +required:
> > > + - compatible
> > > + - reg
> > > +
> > > +unevaluatedProperties: false
> > > +
> > > +examples:
> > > + - |
> > > + i2c {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + regulator@34 {
> > > + compatible = "infineon,ir38060";
> > > + reg = <0x34>;
> > > +
> > > + regulators {
> > > + vout {
> > > + regulator-name = "p5v_aux";
> > > + regulator-min-microvolt = <437500>;
> > > + regulator-max-microvolt = <1387500>;
> > > + };
> > > + };
> > > + };
> > > + };
> > > diff --git a/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml b/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
> > > deleted file mode 100644
> > > index e6ffbc2a2298..000000000000
> > > --- a/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
> > > +++ /dev/null
> > > @@ -1,45 +0,0 @@
> > > -# 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>;
> > > - };
> > > - };
> > >
> > > base-commit: bfbb730c4255e1965d202f48e7aa71baa9a7c65b
> > > --
> > > 2.42.0
> > >
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding
2025-02-05 20:13 ` Conor Dooley
@ 2025-02-06 15:53 ` Naresh Solanki
2025-02-06 18:09 ` Conor Dooley
0 siblings, 1 reply; 19+ messages in thread
From: Naresh Solanki @ 2025-02-06 15:53 UTC (permalink / raw)
To: Conor Dooley
Cc: Guenter Roeck, broonie, Jean Delvare, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, linux-hwmon,
devicetree, linux-kernel
Hi Conor,
On Thu, 6 Feb 2025 at 01:43, Conor Dooley <conor@kernel.org> wrote:
>
> On Wed, Feb 05, 2025 at 03:51:25PM +0530, Naresh Solanki wrote:
> > On Wed, 5 Feb 2025 at 00:52, Conor Dooley <conor@kernel.org> wrote:
> > > On Tue, Feb 04, 2025 at 11:33:03PM +0530, Naresh Solanki wrote:
> > > > Move dt binding under hwmon/pmbus & align accordingly.
> > > >
> > > > Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com>
> > > > ---
> > > > .../hwmon/pmbus/infineon,ir38060.yaml | 61 +++++++++++++++++++
> > > > .../bindings/regulator/infineon,ir38060.yaml | 45 --------------
> > > > 2 files changed, 61 insertions(+), 45 deletions(-)
> > > > create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> > > > delete mode 100644 Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> > > > new file mode 100644
> > > > index 000000000000..e1f683846a54
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> > > > @@ -0,0 +1,61 @@
> > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/hwmon/pmbus/infineon,ir38060.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Infineon Buck Regulators with PMBUS interfaces
> > > > +
> > > > +maintainers:
> > > > + - Not Me.
> > >
> > > How the hell did this get merged!
> > >
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + enum:
> > > > + - infineon,ir38060
> > > > + - infineon,ir38064
> > > > + - infineon,ir38164
> > > > + - infineon,ir38263
> > > > +
> > > > + reg:
> > > > + maxItems: 1
> > > > +
> > > > + regulators:
> > > > + type: object
> > > > + description:
> > > > + list of regulators provided by this controller.
> > >
> > > Can you explain why this change is justified? Your commit message is
> > > explaining what you're doing but not why it's okay to do.
>
> > This is based on other similar dt-bindings under hwmon/pmbus.
>
> Okay, but what I am looking for is an explanation of why it is okay to
> change the node from
>
> | regulator@34 {
> | compatible = "infineon,ir38060";
> | reg = <0x34>;
> |
> | regulator-min-microvolt = <437500>;
> | regulator-max-microvolt = <1387500>;
> | };
As I have understood the driver, this isn't supported.
>
> to
>
> | regulator@34 {
> | compatible = "infineon,ir38060";
> | reg = <0x34>;
> |
> | regulators {
> | vout {
> | regulator-name = "p5v_aux";
> | regulator-min-microvolt = <437500>;
> | regulator-max-microvolt = <1387500>;
> | };
> | };
Above is the typical approach in other pmbus dt bindings.
Even pmbus driver expects this approach.
>
> ?
>
> Will the driver handle both of these identically? Is backwards
> compatibility with the old format maintained? Was the original format
> wrong and does not work? Why is a list of regulators needed when the
> device only provides one?
Driver doesn't support both.
Based on the pmbus driver original format was wrong.
pmbus driver looks for a regulator node to start with.
Reference:
https://github.com/torvalds/linux/blob/master/drivers/hwmon/pmbus/pmbus.h#L515
Regards,
Naresh
>
> Cheers,
> Conor.
>
> > > > + properties:
> > > > + vout:
> > > > + $ref: /schemas/regulator/regulator.yaml#
> > > > + type: object
> > > > +
> > > > + unevaluatedProperties: false
> > > > +
> > > > + additionalProperties: false
> > > > +
> > > > +required:
> > > > + - compatible
> > > > + - reg
> > > > +
> > > > +unevaluatedProperties: false
> > > > +
> > > > +examples:
> > > > + - |
> > > > + i2c {
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > +
> > > > + regulator@34 {
> > > > + compatible = "infineon,ir38060";
> > > > + reg = <0x34>;
> > > > +
> > > > + regulators {
> > > > + vout {
> > > > + regulator-name = "p5v_aux";
> > > > + regulator-min-microvolt = <437500>;
> > > > + regulator-max-microvolt = <1387500>;
> > > > + };
> > > > + };
> > > > + };
> > > > + };
> > > > diff --git a/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml b/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
> > > > deleted file mode 100644
> > > > index e6ffbc2a2298..000000000000
> > > > --- a/Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
> > > > +++ /dev/null
> > > > @@ -1,45 +0,0 @@
> > > > -# 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>;
> > > > - };
> > > > - };
> > > >
> > > > base-commit: bfbb730c4255e1965d202f48e7aa71baa9a7c65b
> > > > --
> > > > 2.42.0
> > > >
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding
2025-02-06 15:53 ` Naresh Solanki
@ 2025-02-06 18:09 ` Conor Dooley
2025-02-06 19:10 ` Naresh Solanki
2025-02-12 10:43 ` Andrew Jeffery
0 siblings, 2 replies; 19+ messages in thread
From: Conor Dooley @ 2025-02-06 18:09 UTC (permalink / raw)
To: Naresh Solanki
Cc: Guenter Roeck, broonie, Jean Delvare, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, linux-hwmon,
devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 4050 bytes --]
On Thu, Feb 06, 2025 at 09:23:03PM +0530, Naresh Solanki wrote:
> On Thu, 6 Feb 2025 at 01:43, Conor Dooley <conor@kernel.org> wrote:
> > On Wed, Feb 05, 2025 at 03:51:25PM +0530, Naresh Solanki wrote:
> > > On Wed, 5 Feb 2025 at 00:52, Conor Dooley <conor@kernel.org> wrote:
> > > > On Tue, Feb 04, 2025 at 11:33:03PM +0530, Naresh Solanki wrote:
> > > > > Move dt binding under hwmon/pmbus & align accordingly.
> > > > >
> > > > > Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com>
> > > > > ---
> > > > > .../hwmon/pmbus/infineon,ir38060.yaml | 61 +++++++++++++++++++
> > > > > .../bindings/regulator/infineon,ir38060.yaml | 45 --------------
> > > > > 2 files changed, 61 insertions(+), 45 deletions(-)
> > > > > create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> > > > > delete mode 100644 Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..e1f683846a54
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> > > > > @@ -0,0 +1,61 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/hwmon/pmbus/infineon,ir38060.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: Infineon Buck Regulators with PMBUS interfaces
> > > > > +
> > > > > +maintainers:
> > > > > + - Not Me.
> > > >
> > > > How the hell did this get merged!
> > > >
> > > > > +
> > > > > +properties:
> > > > > + compatible:
> > > > > + enum:
> > > > > + - infineon,ir38060
> > > > > + - infineon,ir38064
> > > > > + - infineon,ir38164
> > > > > + - infineon,ir38263
> > > > > +
> > > > > + reg:
> > > > > + maxItems: 1
> > > > > +
> > > > > + regulators:
> > > > > + type: object
> > > > > + description:
> > > > > + list of regulators provided by this controller.
> > > >
> > > > Can you explain why this change is justified? Your commit message is
> > > > explaining what you're doing but not why it's okay to do.
> >
> > > This is based on other similar dt-bindings under hwmon/pmbus.
> >
> > Okay, but what I am looking for is an explanation of why it is okay to
> > change the node from
> >
> > | regulator@34 {
> > | compatible = "infineon,ir38060";
> > | reg = <0x34>;
> > |
> > | regulator-min-microvolt = <437500>;
> > | regulator-max-microvolt = <1387500>;
> > | };
> As I have understood the driver, this isn't supported.
> >
> > to
> >
> > | regulator@34 {
> > | compatible = "infineon,ir38060";
> > | reg = <0x34>;
> > |
> > | regulators {
> > | vout {
> > | regulator-name = "p5v_aux";
> > | regulator-min-microvolt = <437500>;
> > | regulator-max-microvolt = <1387500>;
> > | };
> > | };
> Above is the typical approach in other pmbus dt bindings.
> Even pmbus driver expects this approach.
> >
> > ?
> >
> > Will the driver handle both of these identically? Is backwards
> > compatibility with the old format maintained? Was the original format
> > wrong and does not work? Why is a list of regulators needed when the
> > device only provides one?
> Driver doesn't support both.
> Based on the pmbus driver original format was wrong.
> pmbus driver looks for a regulator node to start with.
>
> Reference:
> https://github.com/torvalds/linux/blob/master/drivers/hwmon/pmbus/pmbus.h#L515
Then all of the in-tree users are all just broken? They're in aspeed
bmcs, so I would not be surprised at all if that were the case.
Can you send a new version with a fixes tag and an explanation that what
was there was wrong?
Cheers,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding
2025-02-06 18:09 ` Conor Dooley
@ 2025-02-06 19:10 ` Naresh Solanki
2025-02-07 0:05 ` Conor Dooley
2025-02-12 10:43 ` Andrew Jeffery
1 sibling, 1 reply; 19+ messages in thread
From: Naresh Solanki @ 2025-02-06 19:10 UTC (permalink / raw)
To: Conor Dooley
Cc: Guenter Roeck, broonie, Jean Delvare, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, linux-hwmon,
devicetree, linux-kernel
Hi Conor,
On Thu, 6 Feb 2025 at 23:39, Conor Dooley <conor@kernel.org> wrote:
>
> On Thu, Feb 06, 2025 at 09:23:03PM +0530, Naresh Solanki wrote:
> > On Thu, 6 Feb 2025 at 01:43, Conor Dooley <conor@kernel.org> wrote:
> > > On Wed, Feb 05, 2025 at 03:51:25PM +0530, Naresh Solanki wrote:
> > > > On Wed, 5 Feb 2025 at 00:52, Conor Dooley <conor@kernel.org> wrote:
> > > > > On Tue, Feb 04, 2025 at 11:33:03PM +0530, Naresh Solanki wrote:
> > > > > > Move dt binding under hwmon/pmbus & align accordingly.
> > > > > >
> > > > > > Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com>
> > > > > > ---
> > > > > > .../hwmon/pmbus/infineon,ir38060.yaml | 61 +++++++++++++++++++
> > > > > > .../bindings/regulator/infineon,ir38060.yaml | 45 --------------
> > > > > > 2 files changed, 61 insertions(+), 45 deletions(-)
> > > > > > create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> > > > > > delete mode 100644 Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> > > > > > new file mode 100644
> > > > > > index 000000000000..e1f683846a54
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> > > > > > @@ -0,0 +1,61 @@
> > > > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > > > > +%YAML 1.2
> > > > > > +---
> > > > > > +$id: http://devicetree.org/schemas/hwmon/pmbus/infineon,ir38060.yaml#
> > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > +
> > > > > > +title: Infineon Buck Regulators with PMBUS interfaces
> > > > > > +
> > > > > > +maintainers:
> > > > > > + - Not Me.
> > > > >
> > > > > How the hell did this get merged!
> > > > >
> > > > > > +
> > > > > > +properties:
> > > > > > + compatible:
> > > > > > + enum:
> > > > > > + - infineon,ir38060
> > > > > > + - infineon,ir38064
> > > > > > + - infineon,ir38164
> > > > > > + - infineon,ir38263
> > > > > > +
> > > > > > + reg:
> > > > > > + maxItems: 1
> > > > > > +
> > > > > > + regulators:
> > > > > > + type: object
> > > > > > + description:
> > > > > > + list of regulators provided by this controller.
> > > > >
> > > > > Can you explain why this change is justified? Your commit message is
> > > > > explaining what you're doing but not why it's okay to do.
> > >
> > > > This is based on other similar dt-bindings under hwmon/pmbus.
> > >
> > > Okay, but what I am looking for is an explanation of why it is okay to
> > > change the node from
> > >
> > > | regulator@34 {
> > > | compatible = "infineon,ir38060";
> > > | reg = <0x34>;
> > > |
> > > | regulator-min-microvolt = <437500>;
> > > | regulator-max-microvolt = <1387500>;
> > > | };
> > As I have understood the driver, this isn't supported.
> > >
> > > to
> > >
> > > | regulator@34 {
> > > | compatible = "infineon,ir38060";
> > > | reg = <0x34>;
> > > |
> > > | regulators {
> > > | vout {
> > > | regulator-name = "p5v_aux";
> > > | regulator-min-microvolt = <437500>;
> > > | regulator-max-microvolt = <1387500>;
> > > | };
> > > | };
> > Above is the typical approach in other pmbus dt bindings.
> > Even pmbus driver expects this approach.
> > >
> > > ?
> > >
> > > Will the driver handle both of these identically? Is backwards
> > > compatibility with the old format maintained? Was the original format
> > > wrong and does not work? Why is a list of regulators needed when the
> > > device only provides one?
> > Driver doesn't support both.
> > Based on the pmbus driver original format was wrong.
> > pmbus driver looks for a regulator node to start with.
> >
> > Reference:
> > https://github.com/torvalds/linux/blob/master/drivers/hwmon/pmbus/pmbus.h#L515
>
> Then all of the in-tree users are all just broken? They're in aspeed
> bmcs, so I would not be surprised at all if that were the case.
> Can you send a new version with a fixes tag and an explanation that what
> was there was wrong?
Sure. I will add an explanation in the commit message.
I'm not sure what you meant by 'fixes tag'
Regards,
Naresh
>
> Cheers,
> Conor.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding
2025-02-06 19:10 ` Naresh Solanki
@ 2025-02-07 0:05 ` Conor Dooley
0 siblings, 0 replies; 19+ messages in thread
From: Conor Dooley @ 2025-02-07 0:05 UTC (permalink / raw)
To: Naresh Solanki
Cc: Guenter Roeck, broonie, Jean Delvare, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, linux-hwmon,
devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 382 bytes --]
On Fri, Feb 07, 2025 at 12:40:38AM +0530, Naresh Solanki wrote:
> I'm not sure what you meant by 'fixes tag'
I am surprised you don't know what a fixes tag is, feel like you've been
around long enough to encounter one! They look like Fixes: <hash> ("<shortlog>")
and there should be documentation on them at https://docs.kernel.org/process/submitting-patches.html#describe-changes
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding
2025-02-06 18:09 ` Conor Dooley
2025-02-06 19:10 ` Naresh Solanki
@ 2025-02-12 10:43 ` Andrew Jeffery
2025-02-12 14:46 ` Guenter Roeck
2025-02-12 18:56 ` Conor Dooley
1 sibling, 2 replies; 19+ messages in thread
From: Andrew Jeffery @ 2025-02-12 10:43 UTC (permalink / raw)
To: Conor Dooley, Naresh Solanki
Cc: Guenter Roeck, broonie, Jean Delvare, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, linux-hwmon,
devicetree, linux-kernel
On Thu, 2025-02-06 at 18:09 +0000, Conor Dooley wrote:
> On Thu, Feb 06, 2025 at 09:23:03PM +0530, Naresh Solanki wrote:
> > On Thu, 6 Feb 2025 at 01:43, Conor Dooley <conor@kernel.org> wrote:
> > > On Wed, Feb 05, 2025 at 03:51:25PM +0530, Naresh Solanki wrote:
> > > > On Wed, 5 Feb 2025 at 00:52, Conor Dooley <conor@kernel.org>
> > > > wrote:
> > > > > On Tue, Feb 04, 2025 at 11:33:03PM +0530, Naresh Solanki
> > > > > wrote:
> > > > > > Move dt binding under hwmon/pmbus & align accordingly.
> > > > > >
> > > > > > Signed-off-by: Naresh Solanki
> > > > > > <naresh.solanki@9elements.com>
> > > > > > ---
> > > > > > .../hwmon/pmbus/infineon,ir38060.yaml | 61
> > > > > > +++++++++++++++++++
> > > > > > .../bindings/regulator/infineon,ir38060.yaml | 45 -------
> > > > > > -------
> > > > > > 2 files changed, 61 insertions(+), 45 deletions(-)
> > > > > > create mode 100644
> > > > > > Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38
> > > > > > 060.yaml
> > > > > > delete mode 100644
> > > > > > Documentation/devicetree/bindings/regulator/infineon,ir3806
> > > > > > 0.yaml
> > > > > >
> > > > > > diff --git
> > > > > > a/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir
> > > > > > 38060.yaml
> > > > > > b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir
> > > > > > 38060.yaml
> > > > > > new file mode 100644
> > > > > > index 000000000000..e1f683846a54
> > > > > > --- /dev/null
> > > > > > +++
> > > > > > b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir
> > > > > > 38060.yaml
> > > > > > @@ -0,0 +1,61 @@
> > > > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > > > > +%YAML 1.2
> > > > > > +---
> > > > > > +$id:
> > > > > > http://devicetree.org/schemas/hwmon/pmbus/infineon,ir38060.yaml#
> > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > +
> > > > > > +title: Infineon Buck Regulators with PMBUS interfaces
> > > > > > +
> > > > > > +maintainers:
> > > > > > + - Not Me.
> > > > >
> > > > > How the hell did this get merged!
> > > > >
> > > > > > +
> > > > > > +properties:
> > > > > > + compatible:
> > > > > > + enum:
> > > > > > + - infineon,ir38060
> > > > > > + - infineon,ir38064
> > > > > > + - infineon,ir38164
> > > > > > + - infineon,ir38263
> > > > > > +
> > > > > > + reg:
> > > > > > + maxItems: 1
> > > > > > +
> > > > > > + regulators:
> > > > > > + type: object
> > > > > > + description:
> > > > > > + list of regulators provided by this controller.
> > > > >
> > > > > Can you explain why this change is justified? Your commit
> > > > > message is
> > > > > explaining what you're doing but not why it's okay to do.
> > >
> > > > This is based on other similar dt-bindings under hwmon/pmbus.
> > >
> > > Okay, but what I am looking for is an explanation of why it is
> > > okay to
> > > change the node from
> > >
> > > > regulator@34 {
> > > > compatible = "infineon,ir38060";
> > > > reg = <0x34>;
> > > >
> > > > regulator-min-microvolt = <437500>;
> > > > regulator-max-microvolt = <1387500>;
> > > > };
> > As I have understood the driver, this isn't supported.
> > >
> > > to
> > >
> > > > regulator@34 {
> > > > compatible = "infineon,ir38060";
> > > > reg = <0x34>;
> > > >
> > > > regulators {
> > > > vout {
> > > > regulator-name = "p5v_aux";
> > > > regulator-min-microvolt = <437500>;
> > > > regulator-max-microvolt = <1387500>;
> > > > };
> > > > };
> > Above is the typical approach in other pmbus dt bindings.
> > Even pmbus driver expects this approach.
> > >
> > > ?
> > >
> > > Will the driver handle both of these identically? Is backwards
> > > compatibility with the old format maintained? Was the original
> > > format
> > > wrong and does not work? Why is a list of regulators needed when
> > > the
> > > device only provides one?
> > Driver doesn't support both.
> > Based on the pmbus driver original format was wrong.
> > pmbus driver looks for a regulator node to start with.
> >
> > Reference:
> > https://github.com/torvalds/linux/blob/master/drivers/hwmon/pmbus/pmbus.h#L515
>
> Then all of the in-tree users are all just broken? They're in aspeed
> bmcs, so I would not be surprised at all if that were the case.
Can you unpack the intent of this remark for me a little?
The history of the problem from what I can see looks like:
1. pmbus regulator support exploiting "regulators" as an OF child
node was merged for 3.19[1]
2. The infineon driver support was merged with trivial bindings[2]
and released in v5.17
3. A patch was proposed that extracted the Infineon regulator
compatibles and provided a dedicated binding[3], however it
lacked the "regulators" object property
4. The patch in 3 was merged as [4] with relevant tags, and was
released in v6.9
5. The system1 devicetree was merged and released in v6.10, and sbp1
is merged in v6.14-rc1 for release in v6.14. Both devicetrees, as
far as I'm aware, conform to the binding as written.
In addition to keeping an eye out for Rob's bot, I check all Aspeed-
related devicetree patches against the bindings using the usual tooling
while applying them. I would like to avoid diving into driver
implementations as a blocker to applying devicetree patches where
possible - the formalised bindings and tooling should exist to separate
us from having to do that.
If the complaint is that people submitting Aspeed devicetree patches
are regularly not testing them to make sure they behave correctly on
hardware, then sure, that's something to complain about. Otherwise, I'm
well aware of the (Aspeed) bindings and warnings situation; we've
spoken about it previously. If there's something I should change in my
process (beyond eventually addressing all the warnings) please let me
know, but I don't see that there is in this specific instance.
Andrew
[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ddbb4db4ced1ba784fcd3500179a7291b6c5d7b7
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ca003af3aa1574646b784abee861626a52d345ea
[3]: https://lore.kernel.org/all/20240223-blabber-obnoxious-353e519541a6@spud/
[4]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bad582f9879812bcf74adb520e005051eb021cfb
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding
2025-02-12 10:43 ` Andrew Jeffery
@ 2025-02-12 14:46 ` Guenter Roeck
2025-02-13 0:25 ` Andrew Jeffery
2025-02-12 18:56 ` Conor Dooley
1 sibling, 1 reply; 19+ messages in thread
From: Guenter Roeck @ 2025-02-12 14:46 UTC (permalink / raw)
To: Andrew Jeffery, Conor Dooley, Naresh Solanki
Cc: broonie, Jean Delvare, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, linux-hwmon, devicetree,
linux-kernel
Hi Andrew,
On 2/12/25 02:43, Andrew Jeffery wrote:
[ ... ]
> The history of the problem from what I can see looks like:
>
> 1. pmbus regulator support exploiting "regulators" as an OF child
> node was merged for 3.19[1]
I hope you mean "make full use of and derive benefit from (a resource)"
and not "use (a situation or person) in an unfair or selfish way".
If you think it is not appropriate for PMBus devices to register as
regulators, please let me know, and feel free to suggest a better solution.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding
2025-02-12 14:46 ` Guenter Roeck
@ 2025-02-13 0:25 ` Andrew Jeffery
0 siblings, 0 replies; 19+ messages in thread
From: Andrew Jeffery @ 2025-02-13 0:25 UTC (permalink / raw)
To: Guenter Roeck, Conor Dooley, Naresh Solanki
Cc: broonie, Jean Delvare, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, linux-hwmon, devicetree,
linux-kernel
Hi Guenter,
On Wed, 2025-02-12 at 06:46 -0800, Guenter Roeck wrote:
> Hi Andrew,
>
> On 2/12/25 02:43, Andrew Jeffery wrote:
> [ ... ]
> > The history of the problem from what I can see looks like:
> >
> > 1. pmbus regulator support exploiting "regulators" as an OF
> > child
> > node was merged for 3.19[1]
>
> I hope you mean "make full use of and derive benefit from (a
> resource)"
> and not "use (a situation or person) in an unfair or selfish way".
Certainly "make full use of and derive benefit from (a
resource)" and nothing further.
> If you think it is not appropriate for PMBus devices to register as
> regulators, please let me know, and feel free to suggest a better
> solution.
My only intent was to observe the history of the problem highlighted by
Naresh's patch. I don't have any concerns about PMBus devices
registering as regulators.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding
2025-02-12 10:43 ` Andrew Jeffery
2025-02-12 14:46 ` Guenter Roeck
@ 2025-02-12 18:56 ` Conor Dooley
2025-02-13 0:33 ` Andrew Jeffery
1 sibling, 1 reply; 19+ messages in thread
From: Conor Dooley @ 2025-02-12 18:56 UTC (permalink / raw)
To: Andrew Jeffery
Cc: Naresh Solanki, Guenter Roeck, broonie, Jean Delvare, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, linux-hwmon,
devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 5115 bytes --]
On Wed, Feb 12, 2025 at 09:13:11PM +1030, Andrew Jeffery wrote:
> On Thu, 2025-02-06 at 18:09 +0000, Conor Dooley wrote:
> > On Thu, Feb 06, 2025 at 09:23:03PM +0530, Naresh Solanki wrote:
> > > On Thu, 6 Feb 2025 at 01:43, Conor Dooley <conor@kernel.org> wrote:
> > > > On Wed, Feb 05, 2025 at 03:51:25PM +0530, Naresh Solanki wrote:
> > > > > On Wed, 5 Feb 2025 at 00:52, Conor Dooley <conor@kernel.org>
> > > > > wrote:
> > > > > > On Tue, Feb 04, 2025 at 11:33:03PM +0530, Naresh Solanki
> > > > > > wrote:
> > > > > > > + regulators:
> > > > > > > + type: object
> > > > > > > + description:
> > > > > > > + list of regulators provided by this controller.
> > > > > >
> > > > > > Can you explain why this change is justified? Your commit
> > > > > > message is
> > > > > > explaining what you're doing but not why it's okay to do.
> > > >
> > > > > This is based on other similar dt-bindings under hwmon/pmbus.
> > > >
> > > > Okay, but what I am looking for is an explanation of why it is
> > > > okay to
> > > > change the node from
> > > >
> > > > > regulator@34 {
> > > > > compatible = "infineon,ir38060";
> > > > > reg = <0x34>;
> > > > >
> > > > > regulator-min-microvolt = <437500>;
> > > > > regulator-max-microvolt = <1387500>;
> > > > > };
> > > As I have understood the driver, this isn't supported.
> > > >
> > > > to
> > > >
> > > > > regulator@34 {
> > > > > compatible = "infineon,ir38060";
> > > > > reg = <0x34>;
> > > > >
> > > > > regulators {
> > > > > vout {
> > > > > regulator-name = "p5v_aux";
> > > > > regulator-min-microvolt = <437500>;
> > > > > regulator-max-microvolt = <1387500>;
> > > > > };
> > > > > };
> > > Above is the typical approach in other pmbus dt bindings.
> > > Even pmbus driver expects this approach.
> > > >
> > > > ?
> > > >
> > > > Will the driver handle both of these identically? Is backwards
> > > > compatibility with the old format maintained? Was the original
> > > > format
> > > > wrong and does not work? Why is a list of regulators needed when
> > > > the
> > > > device only provides one?
> > > Driver doesn't support both.
> > > Based on the pmbus driver original format was wrong.
> > > pmbus driver looks for a regulator node to start with.
> > >
> > > Reference:
> > > https://github.com/torvalds/linux/blob/master/drivers/hwmon/pmbus/pmbus.h#L515
> >
> > Then all of the in-tree users are all just broken? They're in aspeed
> > bmcs, so I would not be surprised at all if that were the case.
>
> Can you unpack the intent of this remark for me a little?
>
> The history of the problem from what I can see looks like:
>
> 1. pmbus regulator support exploiting "regulators" as an OF child
> node was merged for 3.19[1]
> 2. The infineon driver support was merged with trivial bindings[2]
> and released in v5.17
> 3. A patch was proposed that extracted the Infineon regulator
> compatibles and provided a dedicated binding[3], however it
> lacked the "regulators" object property
> 4. The patch in 3 was merged as [4] with relevant tags, and was
> released in v6.9
> 5. The system1 devicetree was merged and released in v6.10, and sbp1
> is merged in v6.14-rc1 for release in v6.14. Both devicetrees, as
> far as I'm aware, conform to the binding as written.
>
> In addition to keeping an eye out for Rob's bot, I check all Aspeed-
> related devicetree patches against the bindings using the usual tooling
> while applying them. I would like to avoid diving into driver
> implementations as a blocker to applying devicetree patches where
> possible - the formalised bindings and tooling should exist to separate
> us from having to do that.
>
> If the complaint is that people submitting Aspeed devicetree patches
> are regularly not testing them to make sure they behave correctly on
> hardware, then sure, that's something to complain about. Otherwise, I'm
> well aware of the (Aspeed) bindings and warnings situation; we've
> spoken about it previously. If there's something I should change in my
> process (beyond eventually addressing all the warnings) please let me
> know, but I don't see that there is in this specific instance.
Ye, it's not a jab at aspeed maintainership, it's about the bmc stuff in
particular. I saw far too many warnings from Rob's bot on series with a
version number where the submitter should know better, so the idea that
it had not been tested in other ways wasn't exactly a stretch.
I made a mistake how I pulled these devices out of trivial-devices.yaml,
given the existing driver didn't work with that binding, but I don't
really see why there's a requirement for a regulator child here in the
first place. I get it for something like the lm25066 that is a monitor
IC that you connect a regulator to, as the regulator is a distinct
device - but the ir38060 is a regulator that has a pmbus interface so
both describe the same device.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding
2025-02-12 18:56 ` Conor Dooley
@ 2025-02-13 0:33 ` Andrew Jeffery
0 siblings, 0 replies; 19+ messages in thread
From: Andrew Jeffery @ 2025-02-13 0:33 UTC (permalink / raw)
To: Conor Dooley
Cc: Naresh Solanki, Guenter Roeck, broonie, Jean Delvare, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, linux-hwmon,
devicetree, linux-kernel
On Wed, 2025-02-12 at 18:56 +0000, Conor Dooley wrote:
> On Wed, Feb 12, 2025 at 09:13:11PM +1030, Andrew Jeffery wrote:
> > On Thu, 2025-02-06 at 18:09 +0000, Conor Dooley wrote:
> > > On Thu, Feb 06, 2025 at 09:23:03PM +0530, Naresh Solanki wrote:
> > > > On Thu, 6 Feb 2025 at 01:43, Conor Dooley <conor@kernel.org> wrote:
> > > > > On Wed, Feb 05, 2025 at 03:51:25PM +0530, Naresh Solanki wrote:
> > > > > > On Wed, 5 Feb 2025 at 00:52, Conor Dooley <conor@kernel.org>
> > > > > > wrote:
> > > > > > > On Tue, Feb 04, 2025 at 11:33:03PM +0530, Naresh Solanki
> > > > > > > wrote:
> > > > > > > > + regulators:
> > > > > > > > + type: object
> > > > > > > > + description:
> > > > > > > > + list of regulators provided by this controller.
> > > > > > >
> > > > > > > Can you explain why this change is justified? Your commit
> > > > > > > message is
> > > > > > > explaining what you're doing but not why it's okay to do.
> > > > >
> > > > > > This is based on other similar dt-bindings under hwmon/pmbus.
> > > > >
> > > > > Okay, but what I am looking for is an explanation of why it is
> > > > > okay to
> > > > > change the node from
> > > > >
> > > > > > regulator@34 {
> > > > > > compatible = "infineon,ir38060";
> > > > > > reg = <0x34>;
> > > > > >
> > > > > > regulator-min-microvolt = <437500>;
> > > > > > regulator-max-microvolt = <1387500>;
> > > > > > };
> > > > As I have understood the driver, this isn't supported.
> > > > >
> > > > > to
> > > > >
> > > > > > regulator@34 {
> > > > > > compatible = "infineon,ir38060";
> > > > > > reg = <0x34>;
> > > > > >
> > > > > > regulators {
> > > > > > vout {
> > > > > > regulator-name = "p5v_aux";
> > > > > > regulator-min-microvolt = <437500>;
> > > > > > regulator-max-microvolt = <1387500>;
> > > > > > };
> > > > > > };
> > > > Above is the typical approach in other pmbus dt bindings.
> > > > Even pmbus driver expects this approach.
> > > > >
> > > > > ?
> > > > >
> > > > > Will the driver handle both of these identically? Is backwards
> > > > > compatibility with the old format maintained? Was the original
> > > > > format
> > > > > wrong and does not work? Why is a list of regulators needed when
> > > > > the
> > > > > device only provides one?
> > > > Driver doesn't support both.
> > > > Based on the pmbus driver original format was wrong.
> > > > pmbus driver looks for a regulator node to start with.
> > > >
> > > > Reference:
> > > > https://github.com/torvalds/linux/blob/master/drivers/hwmon/pmbus/pmbus.h#L515
> > >
> > > Then all of the in-tree users are all just broken? They're in aspeed
> > > bmcs, so I would not be surprised at all if that were the case.
> >
> > Can you unpack the intent of this remark for me a little?
> >
> > The history of the problem from what I can see looks like:
> >
> > 1. pmbus regulator support exploiting "regulators" as an OF child
> > node was merged for 3.19[1]
> > 2. The infineon driver support was merged with trivial bindings[2]
> > and released in v5.17
> > 3. A patch was proposed that extracted the Infineon regulator
> > compatibles and provided a dedicated binding[3], however it
> > lacked the "regulators" object property
> > 4. The patch in 3 was merged as [4] with relevant tags, and was
> > released in v6.9
> > 5. The system1 devicetree was merged and released in v6.10, and sbp1
> > is merged in v6.14-rc1 for release in v6.14. Both devicetrees, as
> > far as I'm aware, conform to the binding as written.
> >
> > In addition to keeping an eye out for Rob's bot, I check all Aspeed-
> > related devicetree patches against the bindings using the usual tooling
> > while applying them. I would like to avoid diving into driver
> > implementations as a blocker to applying devicetree patches where
> > possible - the formalised bindings and tooling should exist to separate
> > us from having to do that.
> >
> > If the complaint is that people submitting Aspeed devicetree patches
> > are regularly not testing them to make sure they behave correctly on
> > hardware, then sure, that's something to complain about. Otherwise, I'm
> > well aware of the (Aspeed) bindings and warnings situation; we've
> > spoken about it previously. If there's something I should change in my
> > process (beyond eventually addressing all the warnings) please let me
> > know, but I don't see that there is in this specific instance.
>
> Ye, it's not a jab at aspeed maintainership, it's about the bmc stuff in
> particular. I saw far too many warnings from Rob's bot on series with a
> version number where the submitter should know better, so the idea that
> it had not been tested in other ways wasn't exactly a stretch.
Thanks for elaborating :)
>
> I made a mistake how I pulled these devices out of trivial-devices.yaml,
> given the existing driver didn't work with that binding, but I don't
> really see why there's a requirement for a regulator child here in the
> first place. I get it for something like the lm25066 that is a monitor
> IC that you connect a regulator to, as the regulator is a distinct
> device - but the ir38060 is a regulator that has a pmbus interface so
> both describe the same device.
Makes sense. Maybe it's best to support the existing description in
pmbus core as Rob's already suggested in another part of the thread.
Andrew
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding
2025-02-04 18:03 Naresh Solanki
2025-02-04 19:22 ` Conor Dooley
@ 2025-02-04 23:34 ` Rob Herring (Arm)
1 sibling, 0 replies; 19+ messages in thread
From: Rob Herring (Arm) @ 2025-02-04 23:34 UTC (permalink / raw)
To: Naresh Solanki
Cc: Guenter Roeck, Liam Girdwood, Krzysztof Kozlowski, devicetree,
broonie, linux-hwmon, linux-kernel, Conor Dooley, Jean Delvare
On Tue, 04 Feb 2025 23:33:03 +0530, Naresh Solanki wrote:
> Move dt binding under hwmon/pmbus & align accordingly.
>
> Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com>
> ---
> .../hwmon/pmbus/infineon,ir38060.yaml | 61 +++++++++++++++++++
> .../bindings/regulator/infineon,ir38060.yaml | 45 --------------
> 2 files changed, 61 insertions(+), 45 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/infineon,ir38060.yaml
> delete mode 100644 Documentation/devicetree/bindings/regulator/infineon,ir38060.yaml
>
My bot found new DTB warnings on the .dts files added or changed in this
series.
Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.
If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:
pip3 install dtschema --upgrade
New warnings running 'make CHECK_DTBS=y for arch/arm/boot/dts/aspeed/' for 20250204180306.2755444-1-naresh.solanki@9elements.com:
arch/arm/boot/dts/aspeed/aspeed-bmc-vegman-n110.dtb: /ahb/apb/display@1e6e6000: failed to match any schema with compatible: ['aspeed,ast2500-gfx', 'syscon']
arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-rainier-1s4u.dtb: /ahb/apb/fsi@1e79b000/cfam@0,0/hub@3400/cfam@2,0/i2c@1800: failed to match any schema with compatible: ['ibm,fsi-i2c-master']
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2025-02-13 0:33 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-07 13:28 [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding Naresh Solanki
2025-02-07 13:28 ` [PATCH 2/2] ARM: dts: aspeed: sbp1: Align regulator node with Infineon ir38060 Naresh Solanki
2025-02-07 15:21 ` [PATCH 1/2] dt-bindings: hwmon: ir38060: Move & update dt binding Rob Herring (Arm)
2025-02-12 17:57 ` Rob Herring
2025-02-12 18:33 ` Conor Dooley
-- strict thread matches above, loose matches on Subject: below --
2025-02-04 18:03 Naresh Solanki
2025-02-04 19:22 ` Conor Dooley
2025-02-05 10:21 ` Naresh Solanki
2025-02-05 20:13 ` Conor Dooley
2025-02-06 15:53 ` Naresh Solanki
2025-02-06 18:09 ` Conor Dooley
2025-02-06 19:10 ` Naresh Solanki
2025-02-07 0:05 ` Conor Dooley
2025-02-12 10:43 ` Andrew Jeffery
2025-02-12 14:46 ` Guenter Roeck
2025-02-13 0:25 ` Andrew Jeffery
2025-02-12 18:56 ` Conor Dooley
2025-02-13 0:33 ` Andrew Jeffery
2025-02-04 23:34 ` Rob Herring (Arm)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox