[PATCH v4 0/5] hwmon: (adt7475) attenuator bypass and pwm invert

[PATCH v4 0/5] hwmon: (adt7475) attenuator bypass and pwm invert

[Chris Packham]

I've picked up Logan's changes[1] and am combining them with an old series of
mine[2] to hopefully get them both over the line.

[1] - https://lore.kernel.org/linux-hwmon/20191219033213.30364-1-logan.shaw@alliedtelesis.co.nz/
[2] - https://lore.kernel.org/linux-hwmon/20181107040010.27436-1-chris.packham@alliedtelesis.co.nz/

I know I caused some confusion with my last series which should have said "v3".
This version (v4) updates the new dt properies based on Rob's feedback.

Chris Packham (2):
  dt-bindings: hwmon: Document adt7475 invert-pwm property
  hwmon: (adt7475) Add support for inverting pwm output

Logan Shaw (3):
  dt-bindings: hwmon: Document adt7475 binding
  dt-bindings: hwmon: Document adt7475 bypass-attenuator property
  hwmon: (adt7475) Add attenuator bypass support

 .../devicetree/bindings/hwmon/adt7475.yaml    |  82 ++++++++++++++
 .../devicetree/bindings/trivial-devices.yaml  |   8 --
 drivers/hwmon/adt7475.c                       | 100 +++++++++++++++++-
 3 files changed, 179 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/hwmon/adt7475.yaml

-- 
2.25.0

[PATCH v4 1/5] dt-bindings: hwmon: Document adt7475 binding

[Chris Packham]

From: Logan Shaw <logan.shaw@alliedtelesis.co.nz>

Add binding for adi,adt7475 and variants.
Remove from trivial-devices.

Signed-off-by: Logan Shaw <logan.shaw@alliedtelesis.co.nz>
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v3:
    - split conversion to yaml from addition of new properties

 .../devicetree/bindings/hwmon/adt7475.yaml    | 57 +++++++++++++++++++
 .../devicetree/bindings/trivial-devices.yaml  |  8 ---
 2 files changed, 57 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/hwmon/adt7475.yaml

diff --git a/Documentation/devicetree/bindings/hwmon/adt7475.yaml b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
new file mode 100644
index 000000000000..2252499ea201
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/adt7475.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ADT7475 hwmon sensor
+
+maintainers:
+  - Jean Delvare <jdelvare@suse.com>
+
+description: |
+  The ADT7473, ADT7475, ADT7476, and ADT7490 are thermal monitors and multiple
+  PWN fan controllers.
+
+  They support monitoring and controlling up to four fans (the ADT7490 can only
+  control up to three). They support reading a single on chip temperature
+  sensor and two off chip temperature sensors (the ADT7490 additionally
+  supports measuring up to three current external temperature sensors with
+  series resistance cancellation (SRC)).
+
+  Datasheets:
+  https://www.onsemi.com/pub/Collateral/ADT7473-D.PDF
+  https://www.onsemi.com/pub/Collateral/ADT7475-D.PDF
+  https://www.onsemi.com/pub/Collateral/ADT7476-D.PDF
+  https://www.onsemi.com/pub/Collateral/ADT7490-D.PDF
+
+  Description taken from onsemiconductors specification sheets, with minor
+  rephrasing.
+
+properties:
+  compatible:
+    enum:
+      - adi,adt7473
+      - adi,adt7475
+      - adi,adt7476
+      - adi,adt7490
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      hwmon@2e {
+        compatible = "adi,adt7476";
+        reg = <0x2e>;
+      };
+    };
+
diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index 978de7d37c66..57173ec41c30 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -34,14 +34,6 @@ properties:
           - adi,adt7461
             # +/-1C TDM Extended Temp Range I.C
           - adt7461
-            # +/-1C TDM Extended Temp Range I.C
-          - adi,adt7473
-            # +/-1C TDM Extended Temp Range I.C
-          - adi,adt7475
-            # +/-1C TDM Extended Temp Range I.C
-          - adi,adt7476
-            # +/-1C TDM Extended Temp Range I.C
-          - adi,adt7490
             # Three-Axis Digital Accelerometer
           - adi,adxl345
             # Three-Axis Digital Accelerometer (backward-compatibility value "adi,adxl345" must be listed too)
-- 
2.25.0

[PATCH v4 2/5] dt-bindings: hwmon: Document adt7475 bypass-attenuator property

[Chris Packham]

From: Logan Shaw <logan.shaw@alliedtelesis.co.nz>

Add documentation for the bypass-attenuator-in[0-4] property.

Signed-off-by: Logan Shaw <logan.shaw@alliedtelesis.co.nz>
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v4:
    - use $ref uint32 and enum
    - add 'adi' vendor prefix
    
    Changes in v3:
    - separated addition of new properties from conversion to yaml

 .../devicetree/bindings/hwmon/adt7475.yaml          | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/adt7475.yaml b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
index 2252499ea201..e40612ee075f 100644
--- a/Documentation/devicetree/bindings/hwmon/adt7475.yaml
+++ b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
@@ -39,6 +39,17 @@ properties:
   reg:
     maxItems: 1
 
+patternProperties:
+  "^adi,bypass-attenuator-in[0-4]$":
+    description: |
+      Configures bypassing the individual voltage input attenuator. If
+      set to 1 the attenuator is bypassed if set to 0 the attenuator is
+      not bypassed. If the property is absent then the attenuator
+      retains it's configuration from the bios/bootloader.
+    allOf:
+     - $ref: /schemas/types.yaml#/definitions/uint32
+     - enum: [0, 1]
+
 required:
   - compatible
   - reg
@@ -52,6 +63,8 @@ examples:
       hwmon@2e {
         compatible = "adi,adt7476";
         reg = <0x2e>;
+        adi,bypass-attenuator-in0 = <1>;
+        adi,bypass-attenuator-in1 = <0>;
       };
     };
 
-- 
2.25.0

[PATCH v4 3/5] dt-bindings: hwmon: Document adt7475 invert-pwm property

[Chris Packham]

Add binding information for the invert-pwm property.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v4:
    - use $ref uint32 and enum
    - add adi vendor prefix
    
    Cahnges in v3:
    - new

 Documentation/devicetree/bindings/hwmon/adt7475.yaml | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/adt7475.yaml b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
index e40612ee075f..6a358b30586c 100644
--- a/Documentation/devicetree/bindings/hwmon/adt7475.yaml
+++ b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
@@ -50,6 +50,17 @@ patternProperties:
      - $ref: /schemas/types.yaml#/definitions/uint32
      - enum: [0, 1]
 
+  "^adi,invert-pwm[1-3]$":
+    description: |
+      Configures the pwm output to use inverted logic. If set to 1
+      the pwm uses a logic low output for 100% duty cycle. If set
+      to 0 the pwm uses a logic high output for 100% duty cycle.
+      If the property is absent the pwm retains it's configuration
+      from the bios/bootloader.
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+      - enum: [0, 1]
+
 required:
   - compatible
   - reg
@@ -65,6 +76,7 @@ examples:
         reg = <0x2e>;
         adi,bypass-attenuator-in0 = <1>;
         adi,bypass-attenuator-in1 = <0>;
+        adi,invert-pwm1 = <1>;
       };
     };
 
-- 
2.25.0

[PATCH v4 4/5] hwmon: (adt7475) Add attenuator bypass support

[Chris Packham]

From: Logan Shaw <logan.shaw@alliedtelesis.co.nz>

Added support for reading DTS properties to set attenuators on
device probe for the ADT7473, ADT7475, ADT7476, and ADT7490.

Signed-off-by: Logan Shaw <logan.shaw@alliedtelesis.co.nz>
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v4:
    - use vendor prefix for new property
    
    Changes in v3:
    - move config2 to struct adt7475_data
    - set_property_bit() new helper function to set/clear bit based on dt
      property value.
    - rename to use load_attenuators()

 drivers/hwmon/adt7475.c | 61 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 58 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index 01c2eeb02aa9..3649b18359dc 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -19,6 +19,7 @@
 #include <linux/hwmon-vid.h>
 #include <linux/err.h>
 #include <linux/jiffies.h>
+#include <linux/of.h>
 #include <linux/util_macros.h>
 
 /* Indexes for the sysfs hooks */
@@ -193,6 +194,7 @@ struct adt7475_data {
 	unsigned long measure_updated;
 	bool valid;
 
+	u8 config2;
 	u8 config4;
 	u8 config5;
 	u8 has_voltage;
@@ -1458,6 +1460,55 @@ static int adt7475_update_limits(struct i2c_client *client)
 	return 0;
 }
 
+static int set_property_bit(const struct i2c_client *client, char *property,
+			  u8 *config, u8 bit_index)
+{
+	u32 prop_value = 0;
+	int ret = of_property_read_u32(client->dev.of_node, property,
+					&prop_value);
+
+	if (!ret) {
+		if (prop_value)
+			*config |= (1 << bit_index);
+		else
+			*config &= ~(1 << bit_index);
+	}
+
+	return ret;
+}
+
+static int load_attenuators(const struct i2c_client *client, int chip,
+			    struct adt7475_data *data)
+{
+	int ret;
+
+	if (chip == adt7476 || chip == adt7490) {
+		set_property_bit(client, "adi,bypass-attenuator-in0",
+				 &data->config4, 4);
+		set_property_bit(client, "adi,bypass-attenuator-in1",
+				 &data->config4, 5);
+		set_property_bit(client, "adi,bypass-attenuator-in3",
+				 &data->config4, 6);
+		set_property_bit(client, "adi,bypass-attenuator-in4",
+				 &data->config4, 7);
+
+		ret = i2c_smbus_write_byte_data(client, REG_CONFIG4,
+						data->config4);
+		if (ret < 0)
+			return ret;
+	} else if (chip == adt7473 || chip == adt7475) {
+		set_property_bit(client, "adi,bypass-attenuator-in1",
+				 &data->config2, 5);
+
+		ret = i2c_smbus_write_byte_data(client, REG_CONFIG2,
+						data->config2);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int adt7475_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
@@ -1472,7 +1523,7 @@ static int adt7475_probe(struct i2c_client *client,
 	struct adt7475_data *data;
 	struct device *hwmon_dev;
 	int i, ret = 0, revision, group_num = 0;
-	u8 config2, config3;
+	u8 config3;
 
 	data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
 	if (data == NULL)
@@ -1546,8 +1597,12 @@ static int adt7475_probe(struct i2c_client *client,
 	}
 
 	/* Voltage attenuators can be bypassed, globally or individually */
-	config2 = adt7475_read(REG_CONFIG2);
-	if (config2 & CONFIG2_ATTN) {
+	data->config2 = adt7475_read(REG_CONFIG2);
+	ret = load_attenuators(client, chip, data);
+	if (ret)
+		dev_err(&client->dev, "Error configuring attenuator bypass\n");
+
+	if (data->config2 & CONFIG2_ATTN) {
 		data->bypass_attn = (0x3 << 3) | 0x3;
 	} else {
 		data->bypass_attn = ((data->config4 & CONFIG4_ATTN_IN10) >> 4) |
-- 
2.25.0

[PATCH v4 5/5] hwmon: (adt7475) Add support for inverting pwm output

[Chris Packham]

Add a "invert-pwm" device-tree property to allow hardware designs to use
inverted logic on the PWM output. Preserve the invert PWM output bit if
the property is not found to allow for bootloaders/bios which may have
configured this earlier.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v4:
    - use vendor prefix for new property
    
    Changes in v3:
    - New

 drivers/hwmon/adt7475.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index 3649b18359dc..13267181dde6 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -1509,6 +1509,41 @@ static int load_attenuators(const struct i2c_client *client, int chip,
 	return 0;
 }
 
+static int adt7475_set_pwm_polarity(struct i2c_client *client)
+{
+	char *prop;
+	int ret, i;
+	u8 val;
+
+	for (i = 0; i < ADT7475_PWM_COUNT; i++) {
+		ret = adt7475_read(PWM_CONFIG_REG(i));
+		if (ret < 0)
+			return ret;
+		val = ret;
+		switch (i) {
+		case 0:
+			prop = "adi,invert-pwm1";
+			break;
+		case 1:
+			prop = "adi,invert-pwm2";
+			break;
+		case 2:
+			prop = "adi,invert-pwm3";
+			break;
+		}
+
+		ret = set_property_bit(client, prop, &val, 4);
+		if (ret)
+			continue;
+
+		ret = i2c_smbus_write_byte_data(client, PWM_CONFIG_REG(i), val);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int adt7475_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
@@ -1617,6 +1652,10 @@ static int adt7475_probe(struct i2c_client *client,
 	for (i = 0; i < ADT7475_PWM_COUNT; i++)
 		adt7475_read_pwm(client, i);
 
+	ret = adt7475_set_pwm_polarity(client);
+	if (ret)
+		dev_err(&client->dev, "Error configuring pwm polarity");
+
 	/* Start monitoring */
 	switch (chip) {
 	case adt7475:
-- 
2.25.0

Re: [PATCH v4 3/5] dt-bindings: hwmon: Document adt7475 invert-pwm property

[Rob Herring]

On Thu, Feb 20, 2020 at 10:16 PM Chris Packham
<chris.packham@alliedtelesis.co.nz> wrote:
>
> Add binding information for the invert-pwm property.
>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>
> Notes:
>     Changes in v4:
>     - use $ref uint32 and enum
>     - add adi vendor prefix
>
>     Cahnges in v3:
>     - new
>
>  Documentation/devicetree/bindings/hwmon/adt7475.yaml | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/adt7475.yaml b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
> index e40612ee075f..6a358b30586c 100644
> --- a/Documentation/devicetree/bindings/hwmon/adt7475.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
> @@ -50,6 +50,17 @@ patternProperties:
>       - $ref: /schemas/types.yaml#/definitions/uint32
>       - enum: [0, 1]
>
> +  "^adi,invert-pwm[1-3]$":
> +    description: |
> +      Configures the pwm output to use inverted logic. If set to 1
> +      the pwm uses a logic low output for 100% duty cycle. If set
> +      to 0 the pwm uses a logic high output for 100% duty cycle.
> +      If the property is absent the pwm retains it's configuration
> +      from the bios/bootloader.

I believe we already have an inverted flag for consumers. That doesn't
work if you don't have a consumer described in DT, but then the
question is should you? Or is this something the user will want to
configure from userspace.

The problem with 'invert' properties is they assume you know what the
not inverted state is. I would also make this an array:

adi,pwm-active-state = <1 0 0 1>; // PWM1 and PWM2 active low

And not present means <1 1 1 1>.

Rob

Re: [PATCH v4 3/5] dt-bindings: hwmon: Document adt7475 invert-pwm property

[Guenter Roeck]

On Fri, Feb 21, 2020 at 09:40:00AM -0600, Rob Herring wrote:
> On Thu, Feb 20, 2020 at 10:16 PM Chris Packham
> <chris.packham@alliedtelesis.co.nz> wrote:
> >
> > Add binding information for the invert-pwm property.
> >
> > Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> > ---
> >
> > Notes:
> >     Changes in v4:
> >     - use $ref uint32 and enum
> >     - add adi vendor prefix
> >
> >     Cahnges in v3:
> >     - new
> >
> >  Documentation/devicetree/bindings/hwmon/adt7475.yaml | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/adt7475.yaml b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
> > index e40612ee075f..6a358b30586c 100644
> > --- a/Documentation/devicetree/bindings/hwmon/adt7475.yaml
> > +++ b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
> > @@ -50,6 +50,17 @@ patternProperties:
> >       - $ref: /schemas/types.yaml#/definitions/uint32
> >       - enum: [0, 1]
> >
> > +  "^adi,invert-pwm[1-3]$":
> > +    description: |
> > +      Configures the pwm output to use inverted logic. If set to 1
> > +      the pwm uses a logic low output for 100% duty cycle. If set
> > +      to 0 the pwm uses a logic high output for 100% duty cycle.
> > +      If the property is absent the pwm retains it's configuration
> > +      from the bios/bootloader.
> 
> I believe we already have an inverted flag for consumers. That doesn't
> work if you don't have a consumer described in DT, but then the
> question is should you? Or is this something the user will want to
> configure from userspace.
> 

Normally that is a system property. It is difficult to imagine
that it would ever have to be configured from userspace at runtime.
Most of the time users won't have any idea, and the board datasheet
(if avaible) won't list such information.

Guenter

Re: [PATCH v4 3/5] dt-bindings: hwmon: Document adt7475 invert-pwm property

[Rob Herring]

On Fri, Feb 21, 2020 at 9:53 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Fri, Feb 21, 2020 at 09:40:00AM -0600, Rob Herring wrote:
> > On Thu, Feb 20, 2020 at 10:16 PM Chris Packham
> > <chris.packham@alliedtelesis.co.nz> wrote:
> > >
> > > Add binding information for the invert-pwm property.
> > >
> > > Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> > > ---
> > >
> > > Notes:
> > >     Changes in v4:
> > >     - use $ref uint32 and enum
> > >     - add adi vendor prefix
> > >
> > >     Cahnges in v3:
> > >     - new
> > >
> > >  Documentation/devicetree/bindings/hwmon/adt7475.yaml | 12 ++++++++++++
> > >  1 file changed, 12 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/hwmon/adt7475.yaml b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
> > > index e40612ee075f..6a358b30586c 100644
> > > --- a/Documentation/devicetree/bindings/hwmon/adt7475.yaml
> > > +++ b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
> > > @@ -50,6 +50,17 @@ patternProperties:
> > >       - $ref: /schemas/types.yaml#/definitions/uint32
> > >       - enum: [0, 1]
> > >
> > > +  "^adi,invert-pwm[1-3]$":
> > > +    description: |
> > > +      Configures the pwm output to use inverted logic. If set to 1
> > > +      the pwm uses a logic low output for 100% duty cycle. If set
> > > +      to 0 the pwm uses a logic high output for 100% duty cycle.
> > > +      If the property is absent the pwm retains it's configuration
> > > +      from the bios/bootloader.
> >
> > I believe we already have an inverted flag for consumers. That doesn't
> > work if you don't have a consumer described in DT, but then the
> > question is should you? Or is this something the user will want to
> > configure from userspace.
> >
>
> Normally that is a system property. It is difficult to imagine
> that it would ever have to be configured from userspace at runtime.
> Most of the time users won't have any idea, and the board datasheet
> (if avaible) won't list such information.

Yes, I agree and for those cases I'd expect the consumer is described
in DT too. I read and refreshed my memory on this binding after
sending this. I believe this is for a fan which I'd expect to be in DT
as fans need power and there's different types.

The userspace case I was thinking of was more the hobbyist boards with
PWM to a connector.

Rob

Re: [PATCH v4 1/5] dt-bindings: hwmon: Document adt7475 binding

[Rob Herring]

On Fri, 21 Feb 2020 17:16:27 +1300, Chris Packham wrote:
> 
> From: Logan Shaw <logan.shaw@alliedtelesis.co.nz>
> 
> Add binding for adi,adt7475 and variants.
> Remove from trivial-devices.
> 
> Signed-off-by: Logan Shaw <logan.shaw@alliedtelesis.co.nz>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> 
> Notes:
>     Changes in v3:
>     - split conversion to yaml from addition of new properties
> 
>  .../devicetree/bindings/hwmon/adt7475.yaml    | 57 +++++++++++++++++++
>  .../devicetree/bindings/trivial-devices.yaml  |  8 ---
>  2 files changed, 57 insertions(+), 8 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/adt7475.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v4 2/5] dt-bindings: hwmon: Document adt7475 bypass-attenuator property

[Rob Herring]

On Fri, 21 Feb 2020 17:16:28 +1300, Chris Packham wrote:
> 
> From: Logan Shaw <logan.shaw@alliedtelesis.co.nz>
> 
> Add documentation for the bypass-attenuator-in[0-4] property.
> 
> Signed-off-by: Logan Shaw <logan.shaw@alliedtelesis.co.nz>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> 
> Notes:
>     Changes in v4:
>     - use $ref uint32 and enum
>     - add 'adi' vendor prefix
>     
>     Changes in v3:
>     - separated addition of new properties from conversion to yaml
> 
>  .../devicetree/bindings/hwmon/adt7475.yaml          | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>