diff mbox series

[3/6] dt-bindings: hwmon: ibm,cffps: move to trivial devices

Message ID 20210921102832.143352-3-krzysztof.kozlowski@canonical.com
State Not Applicable, archived
Headers show
Series [1/6] dt-bindings: hwmon: ti,tmp108: convert to dtschema | expand

Checks

Context Check Description
robh/checkpatch warning total: 0 errors, 2 warnings, 19 lines checked

Commit Message

Krzysztof Kozlowski Sept. 21, 2021, 10:28 a.m. UTC 
The IBM Common Form Factor Power Supply Versions 1 and 2 bindings are
trivial, so they can be integrated into trivial devices bindings.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
 .../devicetree/bindings/hwmon/ibm,cffps1.txt  | 26 -------------------
 .../devicetree/bindings/trivial-devices.yaml  |  6 +++++
 MAINTAINERS                                   |  1 -
 3 files changed, 6 insertions(+), 27 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/hwmon/ibm,cffps1.txt

Comments

Guenter Roeck Sept. 21, 2021, 12:30 p.m. UTC | #1 
On Tue, Sep 21, 2021 at 12:28:29PM +0200, Krzysztof Kozlowski wrote:
> The IBM Common Form Factor Power Supply Versions 1 and 2 bindings are
> trivial, so they can be integrated into trivial devices bindings.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>

I won't accept any of those "move to trivial devices" patches. In many cases
the bindings are simply incomplete. I can not and will not make that call,
and I always did and will leave it up to driver authors to decide if they
want to add a device to trivial devices or provide explicit bindings.

Please stop sending those patches.

Guenter

> ---
>  .../devicetree/bindings/hwmon/ibm,cffps1.txt  | 26 -------------------
>  .../devicetree/bindings/trivial-devices.yaml  |  6 +++++
>  MAINTAINERS                                   |  1 -
>  3 files changed, 6 insertions(+), 27 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/hwmon/ibm,cffps1.txt
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/ibm,cffps1.txt b/Documentation/devicetree/bindings/hwmon/ibm,cffps1.txt
> deleted file mode 100644
> index d9a2719f9243..000000000000
> --- a/Documentation/devicetree/bindings/hwmon/ibm,cffps1.txt
> +++ /dev/null
> @@ -1,26 +0,0 @@
> -Device-tree bindings for IBM Common Form Factor Power Supply Versions 1 and 2
> ------------------------------------------------------------------------------
> -
> -Required properties:
> - - compatible				: Must be one of the following:
> -						"ibm,cffps1"
> -						"ibm,cffps2"
> -						or "ibm,cffps" if the system
> -						must support any version of the
> -						power supply
> - - reg = < I2C bus address >;		: Address of the power supply on the
> -					  I2C bus.
> -
> -Example:
> -
> -    i2c-bus@100 {
> -        #address-cells = <1>;
> -        #size-cells = <0>;
> -        #interrupt-cells = <1>;
> -        < more properties >
> -
> -        power-supply@68 {
> -            compatible = "ibm,cffps1";
> -            reg = <0x68>;
> -        };
> -    };
> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> index 6ad0628741cf..791079021f1b 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> @@ -113,6 +113,12 @@ properties:
>            - mps,mp2975
>              # Honeywell Humidicon HIH-6130 humidity/temperature sensor
>            - honeywell,hi6130
> +            # IBM Common Form Factor Power Supply Versions (all versions)
> +          - ibm,cffps
> +            # IBM Common Form Factor Power Supply Versions 1
> +          - ibm,cffps1
> +            # IBM Common Form Factor Power Supply Versions 2
> +          - ibm,cffps2
>              # Infineon IR36021 digital POL buck controller
>            - infineon,ir36021
>              # Infineon IR38064 Voltage Regulator
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 15c4d3c809e8..cc5df54bdc51 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14956,7 +14956,6 @@ S:	Maintained
>  W:	http://hwmon.wiki.kernel.org/
>  W:	http://www.roeck-us.net/linux/drivers/
>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git
> -F:	Documentation/devicetree/bindings/hwmon/ibm,cffps1.txt
>  F:	Documentation/devicetree/bindings/hwmon/ltc2978.txt
>  F:	Documentation/devicetree/bindings/hwmon/max31785.txt
>  F:	Documentation/hwmon/adm1275.rst
> -- 
> 2.30.2
>
Krzysztof Kozlowski Sept. 21, 2021, 12:45 p.m. UTC | #2 
On 21/09/2021 14:30, Guenter Roeck wrote:
> On Tue, Sep 21, 2021 at 12:28:29PM +0200, Krzysztof Kozlowski wrote:
>> The IBM Common Form Factor Power Supply Versions 1 and 2 bindings are
>> trivial, so they can be integrated into trivial devices bindings.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> 
> I won't accept any of those "move to trivial devices" patches. In many cases
> the bindings are simply incomplete. I can not and will not make that call,
> and I always did and will leave it up to driver authors to decide if they
> want to add a device to trivial devices or provide explicit bindings.
> 
> Please stop sending those patches.
> 

Back in the older times, there were no trivial-devices and checkpatch
plus maintainers required documenting compatibles, so some of such
simple bindings were created.

I understand however your point, fair enough. I'll stop sending such
patches.

Best regards,
Krzysztof
Guenter Roeck Sept. 21, 2021, 1:18 p.m. UTC | #3 
On Tue, Sep 21, 2021 at 02:45:42PM +0200, Krzysztof Kozlowski wrote:
> On 21/09/2021 14:30, Guenter Roeck wrote:
> > On Tue, Sep 21, 2021 at 12:28:29PM +0200, Krzysztof Kozlowski wrote:
> >> The IBM Common Form Factor Power Supply Versions 1 and 2 bindings are
> >> trivial, so they can be integrated into trivial devices bindings.
> >>
> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> > 
> > I won't accept any of those "move to trivial devices" patches. In many cases
> > the bindings are simply incomplete. I can not and will not make that call,
> > and I always did and will leave it up to driver authors to decide if they
> > want to add a device to trivial devices or provide explicit bindings.
> > 
> > Please stop sending those patches.
> > 
> 
> Back in the older times, there were no trivial-devices and checkpatch
> plus maintainers required documenting compatibles, so some of such
> simple bindings were created.
> 

At the same time, as I said, the bindings for many chips are incomplete.
For this driver, we can not make that call because the datasheet is not
public. The same is true for dps650ab. For others, the datasheet is
available, and a reasonable decision can be made if the chip may ever
need more complete bindings.

So, let's qualify my statement: I'll accept such patches if you can show,
from the datasheet, that it is unlikely that explicit bindings will ever
be needed. That would be the case for LM70, for example. That would need
more than a statement that "bindings are trivial", though. It also require
a statement along the line that you have confirmed, from to the datasheet,
that there is nothing to configure for this chip that would ever require
explicit bindings.

Thanks,
Guenter

> I understand however your point, fair enough. I'll stop sending such
> patches.
> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski Sept. 21, 2021, 2:40 p.m. UTC | #4 
On 21/09/2021 15:18, Guenter Roeck wrote:
> On Tue, Sep 21, 2021 at 02:45:42PM +0200, Krzysztof Kozlowski wrote:
>> On 21/09/2021 14:30, Guenter Roeck wrote:
>>> On Tue, Sep 21, 2021 at 12:28:29PM +0200, Krzysztof Kozlowski wrote:
>>>> The IBM Common Form Factor Power Supply Versions 1 and 2 bindings are
>>>> trivial, so they can be integrated into trivial devices bindings.
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>>>
>>> I won't accept any of those "move to trivial devices" patches. In many cases
>>> the bindings are simply incomplete. I can not and will not make that call,
>>> and I always did and will leave it up to driver authors to decide if they
>>> want to add a device to trivial devices or provide explicit bindings.
>>>
>>> Please stop sending those patches.
>>>
>>
>> Back in the older times, there were no trivial-devices and checkpatch
>> plus maintainers required documenting compatibles, so some of such
>> simple bindings were created.
>>
> 
> At the same time, as I said, the bindings for many chips are incomplete.
> For this driver, we can not make that call because the datasheet is not
> public. The same is true for dps650ab. For others, the datasheet is
> available, and a reasonable decision can be made if the chip may ever
> need more complete bindings.
> 
> So, let's qualify my statement: I'll accept such patches if you can show,
> from the datasheet, that it is unlikely that explicit bindings will ever
> be needed. That would be the case for LM70, for example. That would need
> more than a statement that "bindings are trivial", though. It also require
> a statement along the line that you have confirmed, from to the datasheet,
> that there is nothing to configure for this chip that would ever require
> explicit bindings.

Thanks for longer explanation. I agree with your reasoning here. I'll
re-check my move-to-trivial patches and, if applicable, send a new
version with better explanation.

I hope this won't stop from reviewing the few other patches in the set
adding separate bindings.


Best regards,
Krzysztof
Guenter Roeck Sept. 22, 2021, 12:28 p.m. UTC | #5 
On Tue, Sep 21, 2021 at 04:40:32PM +0200, Krzysztof Kozlowski wrote:
> 
> I hope this won't stop from reviewing the few other patches in the set
> adding separate bindings.
> 
I don't actively review dt patches, but rely on Rob to do that.
Reason for that is that I almost always get something wrong.
So let's wait for Rob's feedback.

Thanks,
Guenter
Rob Herring Sept. 23, 2021, 10:24 p.m. UTC | #6 
On Tue, Sep 21, 2021 at 02:45:42PM +0200, Krzysztof Kozlowski wrote:
> On 21/09/2021 14:30, Guenter Roeck wrote:
> > On Tue, Sep 21, 2021 at 12:28:29PM +0200, Krzysztof Kozlowski wrote:
> >> The IBM Common Form Factor Power Supply Versions 1 and 2 bindings are
> >> trivial, so they can be integrated into trivial devices bindings.
> >>
> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> > 
> > I won't accept any of those "move to trivial devices" patches. In many cases
> > the bindings are simply incomplete. I can not and will not make that call,
> > and I always did and will leave it up to driver authors to decide if they
> > want to add a device to trivial devices or provide explicit bindings.
> > 
> > Please stop sending those patches.
> > 
> 
> Back in the older times, there were no trivial-devices and checkpatch
> plus maintainers required documenting compatibles, so some of such
> simple bindings were created.

We've had trivial-devices since at least 2011, but it was under i2c 
until 2017. Many existed before we had clock, regulator, etc. bindings, 
so they may have been complete at the time.

> 
> I understand however your point, fair enough. I'll stop sending such
> patches.

Just about every device has a power supply which would not fall in the 
current definition of trivial-devices, though maybe that could be 
extended. I do sometimes wonder if we should just get rid of 
trivial-devices.yaml.

On the flip side, I'd much rather have a schema for these than wait on 
someone to decide to convert them. That could mean a device.txt -> 
trivial-devices.yaml -> device.yaml trip, but that doesn't seem that 
terrible to me. Then we at least are running schema checks on the 
devices and know if actual users have more undocumented properties. 
We still have ~2000 free form binding docs to convert. I'm looking for 
whatever ways we can speed that up and this looks like one of them to 
me (both Krzysztof's great job doing all these conversions and utilizing 
trivial-devices.yaml).

Rob
Rob Herring Sept. 27, 2021, 4:28 p.m. UTC | #7 
On Tue, 21 Sep 2021 12:28:29 +0200, Krzysztof Kozlowski wrote:
> The IBM Common Form Factor Power Supply Versions 1 and 2 bindings are
> trivial, so they can be integrated into trivial devices bindings.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> ---
>  .../devicetree/bindings/hwmon/ibm,cffps1.txt  | 26 -------------------
>  .../devicetree/bindings/trivial-devices.yaml  |  6 +++++
>  MAINTAINERS                                   |  1 -
>  3 files changed, 6 insertions(+), 27 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/hwmon/ibm,cffps1.txt
> 

Reviewed-by: Rob Herring <robh@kernel.org>
Guenter Roeck Oct. 8, 2021, 1:03 p.m. UTC | #8 
On Tue, Sep 21, 2021 at 12:28:29PM +0200, Krzysztof Kozlowski wrote:
> The IBM Common Form Factor Power Supply Versions 1 and 2 bindings are
> trivial, so they can be integrated into trivial devices bindings.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Following Rob's suggestion:

"... I'd much rather have a schema for these than wait on 
someone to decide to convert them. That could mean a device.txt -> 
trivial-devices.yaml -> device.yaml trip, but that doesn't seem that 
terrible to me. Then we at least are running schema checks on the 
devices and know if actual users have more undocumented properties."

Applied.

Thanks,
Guenter

> ---
>  .../devicetree/bindings/hwmon/ibm,cffps1.txt  | 26 -------------------
>  .../devicetree/bindings/trivial-devices.yaml  |  6 +++++
>  MAINTAINERS                                   |  1 -
>  3 files changed, 6 insertions(+), 27 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/hwmon/ibm,cffps1.txt
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/ibm,cffps1.txt b/Documentation/devicetree/bindings/hwmon/ibm,cffps1.txt
> deleted file mode 100644
> index d9a2719f9243..000000000000
> --- a/Documentation/devicetree/bindings/hwmon/ibm,cffps1.txt
> +++ /dev/null
> @@ -1,26 +0,0 @@
> -Device-tree bindings for IBM Common Form Factor Power Supply Versions 1 and 2
> ------------------------------------------------------------------------------
> -
> -Required properties:
> - - compatible				: Must be one of the following:
> -						"ibm,cffps1"
> -						"ibm,cffps2"
> -						or "ibm,cffps" if the system
> -						must support any version of the
> -						power supply
> - - reg = < I2C bus address >;		: Address of the power supply on the
> -					  I2C bus.
> -
> -Example:
> -
> -    i2c-bus@100 {
> -        #address-cells = <1>;
> -        #size-cells = <0>;
> -        #interrupt-cells = <1>;
> -        < more properties >
> -
> -        power-supply@68 {
> -            compatible = "ibm,cffps1";
> -            reg = <0x68>;
> -        };
> -    };
> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> index 6ad0628741cf..791079021f1b 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> @@ -113,6 +113,12 @@ properties:
>            - mps,mp2975
>              # Honeywell Humidicon HIH-6130 humidity/temperature sensor
>            - honeywell,hi6130
> +            # IBM Common Form Factor Power Supply Versions (all versions)
> +          - ibm,cffps
> +            # IBM Common Form Factor Power Supply Versions 1
> +          - ibm,cffps1
> +            # IBM Common Form Factor Power Supply Versions 2
> +          - ibm,cffps2
>              # Infineon IR36021 digital POL buck controller
>            - infineon,ir36021
>              # Infineon IR38064 Voltage Regulator
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 15c4d3c809e8..cc5df54bdc51 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14956,7 +14956,6 @@ S:	Maintained
>  W:	http://hwmon.wiki.kernel.org/
>  W:	http://www.roeck-us.net/linux/drivers/
>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git
> -F:	Documentation/devicetree/bindings/hwmon/ibm,cffps1.txt
>  F:	Documentation/devicetree/bindings/hwmon/ltc2978.txt
>  F:	Documentation/devicetree/bindings/hwmon/max31785.txt
>  F:	Documentation/hwmon/adm1275.rst
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/hwmon/ibm,cffps1.txt b/Documentation/devicetree/bindings/hwmon/ibm,cffps1.txt
deleted file mode 100644
index d9a2719f9243..000000000000
--- a/Documentation/devicetree/bindings/hwmon/ibm,cffps1.txt
+++ /dev/null
@@ -1,26 +0,0 @@ 
-Device-tree bindings for IBM Common Form Factor Power Supply Versions 1 and 2
------------------------------------------------------------------------------
-
-Required properties:
- - compatible				: Must be one of the following:
-						"ibm,cffps1"
-						"ibm,cffps2"
-						or "ibm,cffps" if the system
-						must support any version of the
-						power supply
- - reg = < I2C bus address >;		: Address of the power supply on the
-					  I2C bus.
-
-Example:
-
-    i2c-bus@100 {
-        #address-cells = <1>;
-        #size-cells = <0>;
-        #interrupt-cells = <1>;
-        < more properties >
-
-        power-supply@68 {
-            compatible = "ibm,cffps1";
-            reg = <0x68>;
-        };
-    };
diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index 6ad0628741cf..791079021f1b 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -113,6 +113,12 @@  properties:
           - mps,mp2975
             # Honeywell Humidicon HIH-6130 humidity/temperature sensor
           - honeywell,hi6130
+            # IBM Common Form Factor Power Supply Versions (all versions)
+          - ibm,cffps
+            # IBM Common Form Factor Power Supply Versions 1
+          - ibm,cffps1
+            # IBM Common Form Factor Power Supply Versions 2
+          - ibm,cffps2
             # Infineon IR36021 digital POL buck controller
           - infineon,ir36021
             # Infineon IR38064 Voltage Regulator
diff --git a/MAINTAINERS b/MAINTAINERS
index 15c4d3c809e8..cc5df54bdc51 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14956,7 +14956,6 @@  S:	Maintained
 W:	http://hwmon.wiki.kernel.org/
 W:	http://www.roeck-us.net/linux/drivers/
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git
-F:	Documentation/devicetree/bindings/hwmon/ibm,cffps1.txt
 F:	Documentation/devicetree/bindings/hwmon/ltc2978.txt
 F:	Documentation/devicetree/bindings/hwmon/max31785.txt
 F:	Documentation/hwmon/adm1275.rst