[PATCH v1 0/3] BeagleV Starlight reset support

[PATCH v1 0/3] BeagleV Starlight reset support

[Emil Renner Berthing]

This series adds reset support to the BeagleV Starlight board[1], and
converts the TPS65086 PMIC device tree binding to YAML while at it.

Support for the StarFive JH7100 RISC-V SoC on there is not yet
upstreamed, but early feedback on this part would be very welcome.

[1] https://github.com/beagleboard/beaglev-starlight

Emil Renner Berthing (3):
  dt-bindings: mfd: convert tps65086.txt to YAML
  mfd: tps65086: Make interrupt line optional
  power: reset: Add TPS65086 restart driver

 .../devicetree/bindings/mfd/ti,tps65086.yaml  | 124 ++++++++++++++++++
 .../devicetree/bindings/mfd/tps65086.txt      |  54 --------
 drivers/mfd/tps65086.c                        |  22 ++--
 drivers/power/reset/Kconfig                   |   6 +
 drivers/power/reset/Makefile                  |   1 +
 drivers/power/reset/tps65086-restart.c        |  99 ++++++++++++++
 6 files changed, 242 insertions(+), 64 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
 delete mode 100644 Documentation/devicetree/bindings/mfd/tps65086.txt
 create mode 100644 drivers/power/reset/tps65086-restart.c

-- 
2.32.0

[PATCH v1 1/3] dt-bindings: mfd: convert tps65086.txt to YAML

[Emil Renner Berthing]

This converts the tps65086.txt binding description to YAML schema so
dts files can be verified automatically.

Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
---
@Andrew: The dt-bindings meta schema requires a maintainer entry. I hope
it's ok that I've added you, otherwise I don't know who should be
listed.

 .../devicetree/bindings/mfd/ti,tps65086.yaml  | 127 ++++++++++++++++++
 .../devicetree/bindings/mfd/tps65086.txt      |  54 --------
 2 files changed, 127 insertions(+), 54 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
 delete mode 100644 Documentation/devicetree/bindings/mfd/tps65086.txt

diff --git a/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml b/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
new file mode 100644
index 000000000000..ba638bd10a58
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
@@ -0,0 +1,127 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/ti,tps65086.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TPS65086 Power Management Integrated Circuit (PMIC)
+
+maintainers:
+  - Andrew F. Davis <afd@ti.com>
+
+properties:
+  compatible:
+    const: ti,tps65086
+
+  reg:
+    const: 0x5e
+    description: I2C slave address
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  '#interrupt-cells':
+    const: 2
+    description: |
+      The first cell is the IRQ number. The second cell is the flags,
+      encoded as trigger masks from ../interrupt-controller/interrupts.txt.
+
+  gpio-controller: true
+
+  '#gpio-cells':
+    const: 2
+    description: |
+      The first cell is the pin number and the second cell is used to specify
+      flags.  See ../gpio/gpio.txt for more information.
+
+  regulators:
+    type: object
+    description: |
+      List of child nodes that specify the regulator initialization data.
+      Child nodes must be named after their hardware counterparts:
+      buck[1-6], ldoa[1-3], swa1, swb[1-2], and vtt.
+      Each child node is defined using the standard binding for regulators and
+      the optional regulator properties defined below.
+
+    patternProperties:
+      "^buck[1-6]$":
+        type: object
+        $ref: ../regulator/regulator.yaml
+
+        properties:
+          regulator-name: true
+          regulator-boot-on: true
+          regulator-always-on: true
+          regulator-min-microvolt: true
+          regulator-max-microvolt: true
+          ti,regulator-step-size-25mv:
+            type: boolean
+            description: |
+              Set this if the regulator is factory set with a 25mv step voltage
+              mapping.
+          ti,regulator-decay:
+            type: boolean
+            description: |
+              Set this if the output needs to decay, default is for the output
+              to slew down.
+
+        additionalProperties: false
+
+      "^(ldoa[1-3]|swa1|swb[1-2]|vtt)$":
+        type: object
+        $ref: ../regulator/regulator.yaml
+
+        properties:
+          regulator-name: true
+          regulator-boot-on: true
+          regulator-always-on: true
+          regulator-min-microvolt: true
+          regulator-max-microvolt: true
+
+        additionalProperties: false
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-controller
+  - '#interrupt-cells'
+  - gpio-controller
+  - '#gpio-cells'
+  - regulators
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pmic: tps65086@5e {
+            compatible = "ti,tps65086";
+            reg = <0x5e>;
+            interrupt-parent = <&gpio1>;
+            interrupts = <28 IRQ_TYPE_LEVEL_LOW>;
+            interrupt-controller;
+            #interrupt-cells = <2>;
+            gpio-controller;
+            #gpio-cells = <2>;
+
+            regulators {
+                buck1 {
+                    regulator-name = "vcc1";
+                    regulator-min-microvolt = <1600000>;
+                    regulator-max-microvolt = <1600000>;
+                    regulator-boot-on;
+                    ti,regulator-decay;
+                    ti,regulator-step-size-25mv;
+                };
+            };
+        };
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/mfd/tps65086.txt b/Documentation/devicetree/bindings/mfd/tps65086.txt
deleted file mode 100644
index 67eac0ed32df..000000000000
--- a/Documentation/devicetree/bindings/mfd/tps65086.txt
+++ /dev/null
@@ -1,54 +0,0 @@
-* TPS65086 Power Management Integrated Circuit (PMIC) bindings
-
-Required properties:
- - compatible		: Should be "ti,tps65086".
- - reg			: I2C slave address.
- - interrupts		: The interrupt line the device is connected to.
- - interrupt-controller	: Marks the device node as an interrupt controller.
- - #interrupt-cells	: The number of cells to describe an IRQ, should be 2.
-			    The first cell is the IRQ number.
-			    The second cell is the flags, encoded as trigger
-			    masks from ../interrupt-controller/interrupts.txt.
- - gpio-controller      : Marks the device node as a GPIO Controller.
- - #gpio-cells          : Should be two.  The first cell is the pin number and
-                            the second cell is used to specify flags.
-                            See ../gpio/gpio.txt for more information.
- - regulators:          : List of child nodes that specify the regulator
-                            initialization data. Child nodes must be named
-                            after their hardware counterparts: buck[1-6],
-                            ldoa[1-3], swa1, swb[1-2], and vtt. Each child
-                            node is defined using the standard binding for
-                            regulators and the optional regulator properties
-                            defined below.
-
-Optional regulator properties:
- - ti,regulator-step-size-25mv	: This is applicable for buck[1-6], set this
-				    if the regulator is factory set with a 25mv
-				    step voltage mapping.
- - ti,regulator-decay		: This is applicable for buck[1-6], set this if
-				    the output needs to decay, default is for
-				    the output to slew down.
-
-Example:
-
-	pmic: tps65086@5e {
-		compatible = "ti,tps65086";
-		reg = <0x5e>;
-		interrupt-parent = <&gpio1>;
-		interrupts = <28 IRQ_TYPE_LEVEL_LOW>;
-		interrupt-controller;
-		#interrupt-cells = <2>;
-		gpio-controller;
-		#gpio-cells = <2>;
-
-		regulators {
-			buck1 {
-				regulator-name = "vcc1";
-				regulator-min-microvolt = <1600000>;
-				regulator-max-microvolt = <1600000>;
-				regulator-boot-on;
-				ti,regulator-decay;
-				ti,regulator-step-size-25mv;
-			};
-		};
-	};
-- 
2.32.0

[PATCH v1 2/3] mfd: tps65086: Make interrupt line optional

[Emil Renner Berthing]

The BeagleV Starlight v0.9 board[1] doesn't have the IRQB line routed to
the SoC, but it is still useful to be able to reach the PMIC over I2C
for the other functionality it provides.

[1] https://github.com/beagleboard/beaglev-starlight

Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
---
 .../devicetree/bindings/mfd/ti,tps65086.yaml  |  3 ---
 drivers/mfd/tps65086.c                        | 21 ++++++++++---------
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml b/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
index ba638bd10a58..4b629fcc0df9 100644
--- a/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
+++ b/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
@@ -87,9 +87,6 @@ additionalProperties: false
 required:
   - compatible
   - reg
-  - interrupts
-  - interrupt-controller
-  - '#interrupt-cells'
   - gpio-controller
   - '#gpio-cells'
   - regulators
diff --git a/drivers/mfd/tps65086.c b/drivers/mfd/tps65086.c
index 341466ef20cc..cc3478ee9a64 100644
--- a/drivers/mfd/tps65086.c
+++ b/drivers/mfd/tps65086.c
@@ -100,29 +100,30 @@ static int tps65086_probe(struct i2c_client *client,
 		 (char)((version & TPS65086_DEVICEID_OTP_MASK) >> 4) + 'A',
 		 (version & TPS65086_DEVICEID_REV_MASK) >> 6);
 
-	ret = regmap_add_irq_chip(tps->regmap, tps->irq, IRQF_ONESHOT, 0,
-				  &tps65086_irq_chip, &tps->irq_data);
-	if (ret) {
-		dev_err(tps->dev, "Failed to register IRQ chip\n");
-		return ret;
+	if (tps->irq > 0) {
+		ret = regmap_add_irq_chip(tps->regmap, tps->irq, IRQF_ONESHOT, 0,
+					  &tps65086_irq_chip, &tps->irq_data);
+		if (ret) {
+			dev_err(tps->dev, "Failed to register IRQ chip\n");
+			return ret;
+		}
 	}
 
 	ret = mfd_add_devices(tps->dev, PLATFORM_DEVID_AUTO, tps65086_cells,
 			      ARRAY_SIZE(tps65086_cells), NULL, 0,
 			      regmap_irq_get_domain(tps->irq_data));
-	if (ret) {
+	if (ret && tps->irq > 0)
 		regmap_del_irq_chip(tps->irq, tps->irq_data);
-		return ret;
-	}
 
-	return 0;
+	return ret;
 }
 
 static int tps65086_remove(struct i2c_client *client)
 {
 	struct tps65086 *tps = i2c_get_clientdata(client);
 
-	regmap_del_irq_chip(tps->irq, tps->irq_data);
+	if (tps->irq > 0)
+		regmap_del_irq_chip(tps->irq, tps->irq_data);
 
 	return 0;
 }
-- 
2.32.0

[PATCH v1 3/3] power: reset: Add TPS65086 restart driver

[Emil Renner Berthing]

The only way to reset the BeagleV Starlight v0.9 board[1] properly is to
tell the PMIC to reset itself which will then assert the external reset
lines of the SoC, USB hub and ethernet phy.

This adds a driver to register a reset handler to do just that.

[1] https://github.com/beagleboard/beaglev-starlight

Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
---
 drivers/mfd/tps65086.c                 |  1 +
 drivers/power/reset/Kconfig            |  6 ++
 drivers/power/reset/Makefile           |  1 +
 drivers/power/reset/tps65086-restart.c | 99 ++++++++++++++++++++++++++
 4 files changed, 107 insertions(+)
 create mode 100644 drivers/power/reset/tps65086-restart.c

diff --git a/drivers/mfd/tps65086.c b/drivers/mfd/tps65086.c
index cc3478ee9a64..1bfba0758fcc 100644
--- a/drivers/mfd/tps65086.c
+++ b/drivers/mfd/tps65086.c
@@ -24,6 +24,7 @@
 static const struct mfd_cell tps65086_cells[] = {
 	{ .name = "tps65086-regulator", },
 	{ .name = "tps65086-gpio", },
+	{ .name = "tps65086-restart", },
 };
 
 static const struct regmap_range tps65086_yes_ranges[] = {
diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index 4d1192062508..4b563db3ab3e 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -204,6 +204,12 @@ config POWER_RESET_ST
 	help
 	  Reset support for STMicroelectronics boards.
 
+config POWER_RESET_TPS65086
+	bool "TPS65086 restart driver"
+	depends on MFD_TPS65086
+	help
+	  This driver adds support for resetting the TPS65086 PMIC on restart.
+
 config POWER_RESET_VERSATILE
 	bool "ARM Versatile family reboot driver"
 	depends on ARM
diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
index cf3f4d02d8a5..f606a2f60539 100644
--- a/drivers/power/reset/Makefile
+++ b/drivers/power/reset/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_POWER_RESET_QNAP) += qnap-poweroff.o
 obj-$(CONFIG_POWER_RESET_REGULATOR) += regulator-poweroff.o
 obj-$(CONFIG_POWER_RESET_RESTART) += restart-poweroff.o
 obj-$(CONFIG_POWER_RESET_ST) += st-poweroff.o
+obj-$(CONFIG_POWER_RESET_TPS65086) += tps65086-restart.o
 obj-$(CONFIG_POWER_RESET_VERSATILE) += arm-versatile-reboot.o
 obj-$(CONFIG_POWER_RESET_VEXPRESS) += vexpress-poweroff.o
 obj-$(CONFIG_POWER_RESET_XGENE) += xgene-reboot.o
diff --git a/drivers/power/reset/tps65086-restart.c b/drivers/power/reset/tps65086-restart.c
new file mode 100644
index 000000000000..ad9f2c5a84ac
--- /dev/null
+++ b/drivers/power/reset/tps65086-restart.c
@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Emil Renner Berthing
+ */
+
+#include <linux/mfd/tps65086.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/reboot.h>
+
+struct tps65086_restart {
+	struct notifier_block handler;
+	struct tps65086 *tps;
+	struct device *dev;
+};
+
+static int tps65086_restart_notify(struct notifier_block *this,
+				   unsigned long mode, void *cmd)
+{
+	struct tps65086_restart *tps65086_restart =
+		container_of(this, struct tps65086_restart, handler);
+	int ret;
+
+	ret = regmap_write(tps65086_restart->tps->regmap, TPS65086_FORCESHUTDN, 1);
+	if (ret) {
+		dev_err(tps65086_restart->dev, "%s: error writing to tps65086 pmic: %d\n",
+			__func__, ret);
+		return NOTIFY_DONE;
+	}
+
+	/* give it a little time */
+	mdelay(200);
+
+	WARN_ON(1);
+
+	return NOTIFY_DONE;
+}
+
+static int tps65086_restart_probe(struct platform_device *pdev)
+{
+	struct tps65086_restart *tps65086_restart;
+	int ret;
+
+	tps65086_restart = devm_kzalloc(&pdev->dev, sizeof(*tps65086_restart), GFP_KERNEL);
+	if (!tps65086_restart)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, tps65086_restart);
+
+	tps65086_restart->handler.notifier_call = tps65086_restart_notify;
+	tps65086_restart->handler.priority = 192;
+	tps65086_restart->tps = dev_get_drvdata(pdev->dev.parent);
+	tps65086_restart->dev = &pdev->dev;
+
+	ret = register_restart_handler(&tps65086_restart->handler);
+	if (ret) {
+		dev_err(&pdev->dev, "%s: cannot register restart handler: %d\n",
+			__func__, ret);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static int tps65086_restart_remove(struct platform_device *pdev)
+{
+	struct tps65086_restart *tps65086_restart = platform_get_drvdata(pdev);
+	int ret;
+
+	ret = unregister_restart_handler(&tps65086_restart->handler);
+	if (ret) {
+		dev_err(&pdev->dev, "%s: cannot unregister restart handler: %d\n",
+			__func__, ret);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static const struct platform_device_id tps65086_restart_id_table[] = {
+	{ "tps65086-restart", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, tps65086_restart_id_table);
+
+static struct platform_driver tps65086_restart_driver = {
+	.driver = {
+		.name = "tps65086-restart",
+	},
+	.probe = tps65086_restart_probe,
+	.remove = tps65086_restart_remove,
+	.id_table = tps65086_restart_id_table,
+};
+module_platform_driver(tps65086_restart_driver);
+
+MODULE_AUTHOR("Emil Renner Berthing <kernel@esmil.dk>");
+MODULE_DESCRIPTION("TPS65086 restart driver");
+MODULE_LICENSE("GPL v2");
-- 
2.32.0

Re: [PATCH v1 2/3] mfd: tps65086: Make interrupt line optional

[Lee Jones]

On Sat, 26 Jun 2021, Emil Renner Berthing wrote:

> The BeagleV Starlight v0.9 board[1] doesn't have the IRQB line routed to
> the SoC, but it is still useful to be able to reach the PMIC over I2C

What is still useful?

The GPIO and Regulator drivers?

> for the other functionality it provides.
> 
> [1] https://github.com/beagleboard/beaglev-starlight
> 
> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> ---
>  .../devicetree/bindings/mfd/ti,tps65086.yaml  |  3 ---
>  drivers/mfd/tps65086.c                        | 21 ++++++++++---------
>  2 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml b/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> index ba638bd10a58..4b629fcc0df9 100644
> --- a/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> +++ b/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> @@ -87,9 +87,6 @@ additionalProperties: false
>  required:
>    - compatible
>    - reg
> -  - interrupts
> -  - interrupt-controller
> -  - '#interrupt-cells'

I can't say that I've been keeping up with the latest DT binding
changes, but shouldn't these lines be relocated into some kind of
optional listing?

Or are optional properties omitted from documentation?

>    - gpio-controller
>    - '#gpio-cells'
>    - regulators
> diff --git a/drivers/mfd/tps65086.c b/drivers/mfd/tps65086.c
> index 341466ef20cc..cc3478ee9a64 100644
> --- a/drivers/mfd/tps65086.c
> +++ b/drivers/mfd/tps65086.c
> @@ -100,29 +100,30 @@ static int tps65086_probe(struct i2c_client *client,
>  		 (char)((version & TPS65086_DEVICEID_OTP_MASK) >> 4) + 'A',
>  		 (version & TPS65086_DEVICEID_REV_MASK) >> 6);
>  
> -	ret = regmap_add_irq_chip(tps->regmap, tps->irq, IRQF_ONESHOT, 0,
> -				  &tps65086_irq_chip, &tps->irq_data);
> -	if (ret) {
> -		dev_err(tps->dev, "Failed to register IRQ chip\n");
> -		return ret;
> +	if (tps->irq > 0) {

Are you sure that the 0th line is not a valid IRQ?

> +		ret = regmap_add_irq_chip(tps->regmap, tps->irq, IRQF_ONESHOT, 0,
> +					  &tps65086_irq_chip, &tps->irq_data);
> +		if (ret) {
> +			dev_err(tps->dev, "Failed to register IRQ chip\n");
> +			return ret;
> +		}
>  	}
>  
>  	ret = mfd_add_devices(tps->dev, PLATFORM_DEVID_AUTO, tps65086_cells,
>  			      ARRAY_SIZE(tps65086_cells), NULL, 0,
>  			      regmap_irq_get_domain(tps->irq_data));
> -	if (ret) {
> +	if (ret && tps->irq > 0)
>  		regmap_del_irq_chip(tps->irq, tps->irq_data);
> -		return ret;
> -	}
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static int tps65086_remove(struct i2c_client *client)
>  {
>  	struct tps65086 *tps = i2c_get_clientdata(client);
>  
> -	regmap_del_irq_chip(tps->irq, tps->irq_data);
> +	if (tps->irq > 0)
> +		regmap_del_irq_chip(tps->irq, tps->irq_data);
>  
>  	return 0;
>  }

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 2/3] mfd: tps65086: Make interrupt line optional

[Rob Herring]

On Tue, Jul 13, 2021 at 10:18:01AM +0100, Lee Jones wrote:
> On Sat, 26 Jun 2021, Emil Renner Berthing wrote:
> 
> > The BeagleV Starlight v0.9 board[1] doesn't have the IRQB line routed to
> > the SoC, but it is still useful to be able to reach the PMIC over I2C
> 
> What is still useful?
> 
> The GPIO and Regulator drivers?
> 
> > for the other functionality it provides.
> > 
> > [1] https://github.com/beagleboard/beaglev-starlight
> > 
> > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> > ---
> >  .../devicetree/bindings/mfd/ti,tps65086.yaml  |  3 ---
> >  drivers/mfd/tps65086.c                        | 21 ++++++++++---------
> >  2 files changed, 11 insertions(+), 13 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml b/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> > index ba638bd10a58..4b629fcc0df9 100644
> > --- a/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> > @@ -87,9 +87,6 @@ additionalProperties: false
> >  required:
> >    - compatible
> >    - reg
> > -  - interrupts
> > -  - interrupt-controller
> > -  - '#interrupt-cells'
> 
> I can't say that I've been keeping up with the latest DT binding
> changes, but shouldn't these lines be relocated into some kind of
> optional listing?
> 
> Or are optional properties omitted from documentation?

Optional properties are the ones not listed in the 'required' list.

Rob

Re: [PATCH v1 1/3] dt-bindings: mfd: convert tps65086.txt to YAML

[Rob Herring]

On Sat, Jun 26, 2021 at 12:47:42AM +0200, Emil Renner Berthing wrote:
> This converts the tps65086.txt binding description to YAML schema so
> dts files can be verified automatically.
> 
> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> ---
> @Andrew: The dt-bindings meta schema requires a maintainer entry. I hope
> it's ok that I've added you, otherwise I don't know who should be
> listed.

Anyone that knows the h/w and/or cares if the binding is changed or 
deleted. If you can't find anyone, then good, one less binding. :)

> 
>  .../devicetree/bindings/mfd/ti,tps65086.yaml  | 127 ++++++++++++++++++
>  .../devicetree/bindings/mfd/tps65086.txt      |  54 --------
>  2 files changed, 127 insertions(+), 54 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
>  delete mode 100644 Documentation/devicetree/bindings/mfd/tps65086.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml b/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> new file mode 100644
> index 000000000000..ba638bd10a58
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> @@ -0,0 +1,127 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/ti,tps65086.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TPS65086 Power Management Integrated Circuit (PMIC)
> +
> +maintainers:
> +  - Andrew F. Davis <afd@ti.com>

Not a valid email any more. 

> +
> +properties:
> +  compatible:
> +    const: ti,tps65086
> +
> +  reg:
> +    const: 0x5e
> +    description: I2C slave address
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  interrupt-controller: true
> +
> +  '#interrupt-cells':
> +    const: 2
> +    description: |
> +      The first cell is the IRQ number. The second cell is the flags,
> +      encoded as trigger masks from ../interrupt-controller/interrupts.txt.
> +
> +  gpio-controller: true
> +
> +  '#gpio-cells':
> +    const: 2
> +    description: |
> +      The first cell is the pin number and the second cell is used to specify
> +      flags.  See ../gpio/gpio.txt for more information.
> +
> +  regulators:
> +    type: object
> +    description: |
> +      List of child nodes that specify the regulator initialization data.
> +      Child nodes must be named after their hardware counterparts:
> +      buck[1-6], ldoa[1-3], swa1, swb[1-2], and vtt.
> +      Each child node is defined using the standard binding for regulators and
> +      the optional regulator properties defined below.
> +
> +    patternProperties:
> +      "^buck[1-6]$":
> +        type: object
> +        $ref: ../regulator/regulator.yaml
> +
> +        properties:
> +          regulator-name: true
> +          regulator-boot-on: true
> +          regulator-always-on: true
> +          regulator-min-microvolt: true
> +          regulator-max-microvolt: true
> +          ti,regulator-step-size-25mv:
> +            type: boolean
> +            description: |
> +              Set this if the regulator is factory set with a 25mv step voltage
> +              mapping.
> +          ti,regulator-decay:
> +            type: boolean
> +            description: |
> +              Set this if the output needs to decay, default is for the output
> +              to slew down.
> +
> +        additionalProperties: false
> +
> +      "^(ldoa[1-3]|swa1|swb[1-2]|vtt)$":
> +        type: object
> +        $ref: ../regulator/regulator.yaml
> +
> +        properties:
> +          regulator-name: true
> +          regulator-boot-on: true
> +          regulator-always-on: true
> +          regulator-min-microvolt: true
> +          regulator-max-microvolt: true
> +
> +        additionalProperties: false
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - interrupt-controller
> +  - '#interrupt-cells'
> +  - gpio-controller
> +  - '#gpio-cells'
> +  - regulators
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c0 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        pmic: tps65086@5e {

pmic@5e

> +            compatible = "ti,tps65086";
> +            reg = <0x5e>;
> +            interrupt-parent = <&gpio1>;
> +            interrupts = <28 IRQ_TYPE_LEVEL_LOW>;
> +            interrupt-controller;
> +            #interrupt-cells = <2>;
> +            gpio-controller;
> +            #gpio-cells = <2>;
> +
> +            regulators {
> +                buck1 {
> +                    regulator-name = "vcc1";
> +                    regulator-min-microvolt = <1600000>;
> +                    regulator-max-microvolt = <1600000>;
> +                    regulator-boot-on;
> +                    ti,regulator-decay;
> +                    ti,regulator-step-size-25mv;
> +                };
> +            };
> +        };
> +    };
> +
> +...
> diff --git a/Documentation/devicetree/bindings/mfd/tps65086.txt b/Documentation/devicetree/bindings/mfd/tps65086.txt
> deleted file mode 100644
> index 67eac0ed32df..000000000000
> --- a/Documentation/devicetree/bindings/mfd/tps65086.txt
> +++ /dev/null
> @@ -1,54 +0,0 @@
> -* TPS65086 Power Management Integrated Circuit (PMIC) bindings
> -
> -Required properties:
> - - compatible		: Should be "ti,tps65086".
> - - reg			: I2C slave address.
> - - interrupts		: The interrupt line the device is connected to.
> - - interrupt-controller	: Marks the device node as an interrupt controller.
> - - #interrupt-cells	: The number of cells to describe an IRQ, should be 2.
> -			    The first cell is the IRQ number.
> -			    The second cell is the flags, encoded as trigger
> -			    masks from ../interrupt-controller/interrupts.txt.
> - - gpio-controller      : Marks the device node as a GPIO Controller.
> - - #gpio-cells          : Should be two.  The first cell is the pin number and
> -                            the second cell is used to specify flags.
> -                            See ../gpio/gpio.txt for more information.
> - - regulators:          : List of child nodes that specify the regulator
> -                            initialization data. Child nodes must be named
> -                            after their hardware counterparts: buck[1-6],
> -                            ldoa[1-3], swa1, swb[1-2], and vtt. Each child
> -                            node is defined using the standard binding for
> -                            regulators and the optional regulator properties
> -                            defined below.
> -
> -Optional regulator properties:
> - - ti,regulator-step-size-25mv	: This is applicable for buck[1-6], set this
> -				    if the regulator is factory set with a 25mv
> -				    step voltage mapping.
> - - ti,regulator-decay		: This is applicable for buck[1-6], set this if
> -				    the output needs to decay, default is for
> -				    the output to slew down.
> -
> -Example:
> -
> -	pmic: tps65086@5e {
> -		compatible = "ti,tps65086";
> -		reg = <0x5e>;
> -		interrupt-parent = <&gpio1>;
> -		interrupts = <28 IRQ_TYPE_LEVEL_LOW>;
> -		interrupt-controller;
> -		#interrupt-cells = <2>;
> -		gpio-controller;
> -		#gpio-cells = <2>;
> -
> -		regulators {
> -			buck1 {
> -				regulator-name = "vcc1";
> -				regulator-min-microvolt = <1600000>;
> -				regulator-max-microvolt = <1600000>;
> -				regulator-boot-on;
> -				ti,regulator-decay;
> -				ti,regulator-step-size-25mv;
> -			};
> -		};
> -	};
> -- 
> 2.32.0
> 
> 

Re: [PATCH v1 2/3] mfd: tps65086: Make interrupt line optional

[Rob Herring]

On Sat, 26 Jun 2021 00:47:43 +0200, Emil Renner Berthing wrote:
> The BeagleV Starlight v0.9 board[1] doesn't have the IRQB line routed to
> the SoC, but it is still useful to be able to reach the PMIC over I2C
> for the other functionality it provides.
> 
> [1] https://github.com/beagleboard/beaglev-starlight
> 
> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> ---
>  .../devicetree/bindings/mfd/ti,tps65086.yaml  |  3 ---
>  drivers/mfd/tps65086.c                        | 21 ++++++++++---------
>  2 files changed, 11 insertions(+), 13 deletions(-)
> 

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

Re: [PATCH v1 1/3] dt-bindings: mfd: convert tps65086.txt to YAML

[Emil Renner Berthing]

On Wed, 14 Jul 2021 at 22:44, Rob Herring <robh@kernel.org> wrote:
> On Sat, Jun 26, 2021 at 12:47:42AM +0200, Emil Renner Berthing wrote:
> > This converts the tps65086.txt binding description to YAML schema so
> > dts files can be verified automatically.
> >
> > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> > ---
> > @Andrew: The dt-bindings meta schema requires a maintainer entry. I hope
> > it's ok that I've added you, otherwise I don't know who should be
> > listed.
>
> Anyone that knows the h/w and/or cares if the binding is changed or
> deleted. If you can't find anyone, then good, one less binding. :)

Hmm.. unless someone else responds I guess I'll just put myself then
since I at least care that it isn't deleted.

> >
> >  .../devicetree/bindings/mfd/ti,tps65086.yaml  | 127 ++++++++++++++++++
> >  .../devicetree/bindings/mfd/tps65086.txt      |  54 --------
> >  2 files changed, 127 insertions(+), 54 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> >  delete mode 100644 Documentation/devicetree/bindings/mfd/tps65086.txt
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml b/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> > new file mode 100644
> > index 000000000000..ba638bd10a58
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> > @@ -0,0 +1,127 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mfd/ti,tps65086.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: TPS65086 Power Management Integrated Circuit (PMIC)
> > +
> > +maintainers:
> > +  - Andrew F. Davis <afd@ti.com>
>
> Not a valid email any more.
>
> > +
> > +properties:
> > +  compatible:
> > +    const: ti,tps65086
> > +
> > +  reg:
> > +    const: 0x5e
> > +    description: I2C slave address
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  interrupt-controller: true
> > +
> > +  '#interrupt-cells':
> > +    const: 2
> > +    description: |
> > +      The first cell is the IRQ number. The second cell is the flags,
> > +      encoded as trigger masks from ../interrupt-controller/interrupts.txt.
> > +
> > +  gpio-controller: true
> > +
> > +  '#gpio-cells':
> > +    const: 2
> > +    description: |
> > +      The first cell is the pin number and the second cell is used to specify
> > +      flags.  See ../gpio/gpio.txt for more information.
> > +
> > +  regulators:
> > +    type: object
> > +    description: |
> > +      List of child nodes that specify the regulator initialization data.
> > +      Child nodes must be named after their hardware counterparts:
> > +      buck[1-6], ldoa[1-3], swa1, swb[1-2], and vtt.
> > +      Each child node is defined using the standard binding for regulators and
> > +      the optional regulator properties defined below.
> > +
> > +    patternProperties:
> > +      "^buck[1-6]$":
> > +        type: object
> > +        $ref: ../regulator/regulator.yaml
> > +
> > +        properties:
> > +          regulator-name: true
> > +          regulator-boot-on: true
> > +          regulator-always-on: true
> > +          regulator-min-microvolt: true
> > +          regulator-max-microvolt: true
> > +          ti,regulator-step-size-25mv:
> > +            type: boolean
> > +            description: |
> > +              Set this if the regulator is factory set with a 25mv step voltage
> > +              mapping.
> > +          ti,regulator-decay:
> > +            type: boolean
> > +            description: |
> > +              Set this if the output needs to decay, default is for the output
> > +              to slew down.
> > +
> > +        additionalProperties: false
> > +
> > +      "^(ldoa[1-3]|swa1|swb[1-2]|vtt)$":
> > +        type: object
> > +        $ref: ../regulator/regulator.yaml
> > +
> > +        properties:
> > +          regulator-name: true
> > +          regulator-boot-on: true
> > +          regulator-always-on: true
> > +          regulator-min-microvolt: true
> > +          regulator-max-microvolt: true
> > +
> > +        additionalProperties: false
> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - interrupt-controller
> > +  - '#interrupt-cells'
> > +  - gpio-controller
> > +  - '#gpio-cells'
> > +  - regulators
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    i2c0 {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        pmic: tps65086@5e {
>
> pmic@5e

Will fix, thanks!

> > +            compatible = "ti,tps65086";
> > +            reg = <0x5e>;
> > +            interrupt-parent = <&gpio1>;
> > +            interrupts = <28 IRQ_TYPE_LEVEL_LOW>;
> > +            interrupt-controller;
> > +            #interrupt-cells = <2>;
> > +            gpio-controller;
> > +            #gpio-cells = <2>;
> > +
> > +            regulators {
> > +                buck1 {
> > +                    regulator-name = "vcc1";
> > +                    regulator-min-microvolt = <1600000>;
> > +                    regulator-max-microvolt = <1600000>;
> > +                    regulator-boot-on;
> > +                    ti,regulator-decay;
> > +                    ti,regulator-step-size-25mv;
> > +                };
> > +            };
> > +        };
> > +    };
> > +
> > +...
> > diff --git a/Documentation/devicetree/bindings/mfd/tps65086.txt b/Documentation/devicetree/bindings/mfd/tps65086.txt
> > deleted file mode 100644
> > index 67eac0ed32df..000000000000
> > --- a/Documentation/devicetree/bindings/mfd/tps65086.txt
> > +++ /dev/null
> > @@ -1,54 +0,0 @@
> > -* TPS65086 Power Management Integrated Circuit (PMIC) bindings
> > -
> > -Required properties:
> > - - compatible                : Should be "ti,tps65086".
> > - - reg                       : I2C slave address.
> > - - interrupts                : The interrupt line the device is connected to.
> > - - interrupt-controller      : Marks the device node as an interrupt controller.
> > - - #interrupt-cells  : The number of cells to describe an IRQ, should be 2.
> > -                         The first cell is the IRQ number.
> > -                         The second cell is the flags, encoded as trigger
> > -                         masks from ../interrupt-controller/interrupts.txt.
> > - - gpio-controller      : Marks the device node as a GPIO Controller.
> > - - #gpio-cells          : Should be two.  The first cell is the pin number and
> > -                            the second cell is used to specify flags.
> > -                            See ../gpio/gpio.txt for more information.
> > - - regulators:          : List of child nodes that specify the regulator
> > -                            initialization data. Child nodes must be named
> > -                            after their hardware counterparts: buck[1-6],
> > -                            ldoa[1-3], swa1, swb[1-2], and vtt. Each child
> > -                            node is defined using the standard binding for
> > -                            regulators and the optional regulator properties
> > -                            defined below.
> > -
> > -Optional regulator properties:
> > - - ti,regulator-step-size-25mv       : This is applicable for buck[1-6], set this
> > -                                 if the regulator is factory set with a 25mv
> > -                                 step voltage mapping.
> > - - ti,regulator-decay                : This is applicable for buck[1-6], set this if
> > -                                 the output needs to decay, default is for
> > -                                 the output to slew down.
> > -
> > -Example:
> > -
> > -     pmic: tps65086@5e {
> > -             compatible = "ti,tps65086";
> > -             reg = <0x5e>;
> > -             interrupt-parent = <&gpio1>;
> > -             interrupts = <28 IRQ_TYPE_LEVEL_LOW>;
> > -             interrupt-controller;
> > -             #interrupt-cells = <2>;
> > -             gpio-controller;
> > -             #gpio-cells = <2>;
> > -
> > -             regulators {
> > -                     buck1 {
> > -                             regulator-name = "vcc1";
> > -                             regulator-min-microvolt = <1600000>;
> > -                             regulator-max-microvolt = <1600000>;
> > -                             regulator-boot-on;
> > -                             ti,regulator-decay;
> > -                             ti,regulator-step-size-25mv;
> > -                     };
> > -             };
> > -     };
> > --
> > 2.32.0
> >
> >

Re: [PATCH v1 2/3] mfd: tps65086: Make interrupt line optional

[Lee Jones]

On Wed, 14 Jul 2021, Rob Herring wrote:

> On Tue, Jul 13, 2021 at 10:18:01AM +0100, Lee Jones wrote:
> > On Sat, 26 Jun 2021, Emil Renner Berthing wrote:
> > 
> > > The BeagleV Starlight v0.9 board[1] doesn't have the IRQB line routed to
> > > the SoC, but it is still useful to be able to reach the PMIC over I2C
> > 
> > What is still useful?
> > 
> > The GPIO and Regulator drivers?
> > 
> > > for the other functionality it provides.
> > > 
> > > [1] https://github.com/beagleboard/beaglev-starlight
> > > 
> > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> > > ---
> > >  .../devicetree/bindings/mfd/ti,tps65086.yaml  |  3 ---
> > >  drivers/mfd/tps65086.c                        | 21 ++++++++++---------
> > >  2 files changed, 11 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml b/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> > > index ba638bd10a58..4b629fcc0df9 100644
> > > --- a/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> > > +++ b/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> > > @@ -87,9 +87,6 @@ additionalProperties: false
> > >  required:
> > >    - compatible
> > >    - reg
> > > -  - interrupts
> > > -  - interrupt-controller
> > > -  - '#interrupt-cells'
> > 
> > I can't say that I've been keeping up with the latest DT binding
> > changes, but shouldn't these lines be relocated into some kind of
> > optional listing?
> > 
> > Or are optional properties omitted from documentation?
> 
> Optional properties are the ones not listed in the 'required' list.

Ah, so they are already documented somewhere else in the file?

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 2/3] mfd: tps65086: Make interrupt line optional

[Emil Renner Berthing]

On Thu, 15 Jul 2021 at 13:13, Lee Jones <lee.jones@linaro.org> wrote:
> On Wed, 14 Jul 2021, Rob Herring wrote:
> > On Tue, Jul 13, 2021 at 10:18:01AM +0100, Lee Jones wrote:
> > > On Sat, 26 Jun 2021, Emil Renner Berthing wrote:
> > >
> > > > The BeagleV Starlight v0.9 board[1] doesn't have the IRQB line routed to
> > > > the SoC, but it is still useful to be able to reach the PMIC over I2C
> > >
> > > What is still useful?
> > >
> > > The GPIO and Regulator drivers?
> > >
> > > > for the other functionality it provides.
> > > >
> > > > [1] https://github.com/beagleboard/beaglev-starlight
> > > >
> > > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> > > > ---
> > > >  .../devicetree/bindings/mfd/ti,tps65086.yaml  |  3 ---
> > > >  drivers/mfd/tps65086.c                        | 21 ++++++++++---------
> > > >  2 files changed, 11 insertions(+), 13 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml b/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> > > > index ba638bd10a58..4b629fcc0df9 100644
> > > > --- a/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> > > > +++ b/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> > > > @@ -87,9 +87,6 @@ additionalProperties: false
> > > >  required:
> > > >    - compatible
> > > >    - reg
> > > > -  - interrupts
> > > > -  - interrupt-controller
> > > > -  - '#interrupt-cells'
> > >
> > > I can't say that I've been keeping up with the latest DT binding
> > > changes, but shouldn't these lines be relocated into some kind of
> > > optional listing?
> > >
> > > Or are optional properties omitted from documentation?
> >
> > Optional properties are the ones not listed in the 'required' list.
>
> Ah, so they are already documented somewhere else in the file?

Yes, just a few lines above.

Re: [PATCH v1 2/3] mfd: tps65086: Make interrupt line optional

[Lee Jones]

On Sat, 26 Jun 2021, Emil Renner Berthing wrote:

> The BeagleV Starlight v0.9 board[1] doesn't have the IRQB line routed to
> the SoC, but it is still useful to be able to reach the PMIC over I2C
> for the other functionality it provides.
> 
> [1] https://github.com/beagleboard/beaglev-starlight
> 
> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> ---
>  .../devicetree/bindings/mfd/ti,tps65086.yaml  |  3 ---

This is not present in my current tree.

Looks like it's still *.txt.

Am I missing a patch?

>  drivers/mfd/tps65086.c                        | 21 ++++++++++---------
>  2 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml b/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> index ba638bd10a58..4b629fcc0df9 100644
> --- a/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> +++ b/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> @@ -87,9 +87,6 @@ additionalProperties: false
>  required:
>    - compatible
>    - reg
> -  - interrupts
> -  - interrupt-controller
> -  - '#interrupt-cells'
>    - gpio-controller
>    - '#gpio-cells'
>    - regulators
> diff --git a/drivers/mfd/tps65086.c b/drivers/mfd/tps65086.c
> index 341466ef20cc..cc3478ee9a64 100644
> --- a/drivers/mfd/tps65086.c
> +++ b/drivers/mfd/tps65086.c
> @@ -100,29 +100,30 @@ static int tps65086_probe(struct i2c_client *client,
>  		 (char)((version & TPS65086_DEVICEID_OTP_MASK) >> 4) + 'A',
>  		 (version & TPS65086_DEVICEID_REV_MASK) >> 6);
>  
> -	ret = regmap_add_irq_chip(tps->regmap, tps->irq, IRQF_ONESHOT, 0,
> -				  &tps65086_irq_chip, &tps->irq_data);
> -	if (ret) {
> -		dev_err(tps->dev, "Failed to register IRQ chip\n");
> -		return ret;
> +	if (tps->irq > 0) {
> +		ret = regmap_add_irq_chip(tps->regmap, tps->irq, IRQF_ONESHOT, 0,
> +					  &tps65086_irq_chip, &tps->irq_data);
> +		if (ret) {
> +			dev_err(tps->dev, "Failed to register IRQ chip\n");
> +			return ret;
> +		}
>  	}
>  
>  	ret = mfd_add_devices(tps->dev, PLATFORM_DEVID_AUTO, tps65086_cells,
>  			      ARRAY_SIZE(tps65086_cells), NULL, 0,
>  			      regmap_irq_get_domain(tps->irq_data));
> -	if (ret) {
> +	if (ret && tps->irq > 0)
>  		regmap_del_irq_chip(tps->irq, tps->irq_data);
> -		return ret;
> -	}
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static int tps65086_remove(struct i2c_client *client)
>  {
>  	struct tps65086 *tps = i2c_get_clientdata(client);
>  
> -	regmap_del_irq_chip(tps->irq, tps->irq_data);
> +	if (tps->irq > 0)
> +		regmap_del_irq_chip(tps->irq, tps->irq_data);
>  
>  	return 0;
>  }

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 2/3] mfd: tps65086: Make interrupt line optional

[Emil Renner Berthing]

On Tue, 20 Jul 2021 at 17:07, Lee Jones <lee.jones@linaro.org> wrote:
> On Sat, 26 Jun 2021, Emil Renner Berthing wrote:
> > The BeagleV Starlight v0.9 board[1] doesn't have the IRQB line routed to
> > the SoC, but it is still useful to be able to reach the PMIC over I2C
> > for the other functionality it provides.
> >
> > [1] https://github.com/beagleboard/beaglev-starlight
> >
> > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> > ---
> >  .../devicetree/bindings/mfd/ti,tps65086.yaml  |  3 ---
>
> This is not present in my current tree.
>
> Looks like it's still *.txt.
>
> Am I missing a patch?

Yes, the first patch in the series converts that to yaml. I'm quite
sure I had the same list of recipients on all 4 mails in the series,
so don't know why that should be missing.

Re: [PATCH v1 2/3] mfd: tps65086: Make interrupt line optional

[Lee Jones]

On Tue, 20 Jul 2021, Emil Renner Berthing wrote:

> On Tue, 20 Jul 2021 at 17:07, Lee Jones <lee.jones@linaro.org> wrote:
> > On Sat, 26 Jun 2021, Emil Renner Berthing wrote:
> > > The BeagleV Starlight v0.9 board[1] doesn't have the IRQB line routed to
> > > the SoC, but it is still useful to be able to reach the PMIC over I2C
> > > for the other functionality it provides.
> > >
> > > [1] https://github.com/beagleboard/beaglev-starlight
> > >
> > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> > > ---
> > >  .../devicetree/bindings/mfd/ti,tps65086.yaml  |  3 ---
> >
> > This is not present in my current tree.
> >
> > Looks like it's still *.txt.
> >
> > Am I missing a patch?
> 
> Yes, the first patch in the series converts that to yaml. I'm quite
> sure I had the same list of recipients on all 4 mails in the series,
> so don't know why that should be missing.

Oh, it's not marked as 'important' because it has open review comments
on it.

Just have this for now then:

For my own reference (apply this as-is to your sign-off block):

  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 2/3] mfd: tps65086: Make interrupt line optional

[Emil Renner Berthing]

On Tue, 20 Jul 2021 at 17:32, Lee Jones <lee.jones@linaro.org> wrote:
> On Tue, 20 Jul 2021, Emil Renner Berthing wrote:
> > On Tue, 20 Jul 2021 at 17:07, Lee Jones <lee.jones@linaro.org> wrote:
> > > On Sat, 26 Jun 2021, Emil Renner Berthing wrote:
> > > > The BeagleV Starlight v0.9 board[1] doesn't have the IRQB line routed to
> > > > the SoC, but it is still useful to be able to reach the PMIC over I2C
> > > > for the other functionality it provides.
> > > >
> > > > [1] https://github.com/beagleboard/beaglev-starlight
> > > >
> > > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> > > > ---
> > > >  .../devicetree/bindings/mfd/ti,tps65086.yaml  |  3 ---
> > >
> > > This is not present in my current tree.
> > >
> > > Looks like it's still *.txt.
> > >
> > > Am I missing a patch?
> >
> > Yes, the first patch in the series converts that to yaml. I'm quite
> > sure I had the same list of recipients on all 4 mails in the series,
> > so don't know why that should be missing.
>
> Oh, it's not marked as 'important' because it has open review comments
> on it.
>
> Just have this for now then:
>
> For my own reference (apply this as-is to your sign-off block):
>
>   Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

Thanks! Do you want to have a look at 3/3 or should I just send a v2
to fix the yaml conversion now?

/Emil

Re: [PATCH v1 2/3] mfd: tps65086: Make interrupt line optional

[Lee Jones]

On Tue, 20 Jul 2021, Emil Renner Berthing wrote:

> On Tue, 20 Jul 2021 at 17:32, Lee Jones <lee.jones@linaro.org> wrote:
> > On Tue, 20 Jul 2021, Emil Renner Berthing wrote:
> > > On Tue, 20 Jul 2021 at 17:07, Lee Jones <lee.jones@linaro.org> wrote:
> > > > On Sat, 26 Jun 2021, Emil Renner Berthing wrote:
> > > > > The BeagleV Starlight v0.9 board[1] doesn't have the IRQB line routed to
> > > > > the SoC, but it is still useful to be able to reach the PMIC over I2C
> > > > > for the other functionality it provides.
> > > > >
> > > > > [1] https://github.com/beagleboard/beaglev-starlight
> > > > >
> > > > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> > > > > ---
> > > > >  .../devicetree/bindings/mfd/ti,tps65086.yaml  |  3 ---
> > > >
> > > > This is not present in my current tree.
> > > >
> > > > Looks like it's still *.txt.
> > > >
> > > > Am I missing a patch?
> > >
> > > Yes, the first patch in the series converts that to yaml. I'm quite
> > > sure I had the same list of recipients on all 4 mails in the series,
> > > so don't know why that should be missing.
> >
> > Oh, it's not marked as 'important' because it has open review comments
> > on it.
> >
> > Just have this for now then:
> >
> > For my own reference (apply this as-is to your sign-off block):
> >
> >   Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> 
> Thanks! Do you want to have a look at 3/3 or should I just send a v2
> to fix the yaml conversion now?

Patch 3 should be split.

I think it should also s/restart/reset/.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 3/3] power: reset: Add TPS65086 restart driver

[Sebastian Reichel]

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

Hi,

On Sat, Jun 26, 2021 at 12:47:44AM +0200, Emil Renner Berthing wrote:
> The only way to reset the BeagleV Starlight v0.9 board[1] properly is to
> tell the PMIC to reset itself which will then assert the external reset
> lines of the SoC, USB hub and ethernet phy.
> 
> This adds a driver to register a reset handler to do just that.
> 
> [1] https://github.com/beagleboard/beaglev-starlight
> 
> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> ---

Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

>  drivers/mfd/tps65086.c                 |  1 +
>  drivers/power/reset/Kconfig            |  6 ++
>  drivers/power/reset/Makefile           |  1 +
>  drivers/power/reset/tps65086-restart.c | 99 ++++++++++++++++++++++++++
>  4 files changed, 107 insertions(+)
>  create mode 100644 drivers/power/reset/tps65086-restart.c
> 
> diff --git a/drivers/mfd/tps65086.c b/drivers/mfd/tps65086.c
> index cc3478ee9a64..1bfba0758fcc 100644
> --- a/drivers/mfd/tps65086.c
> +++ b/drivers/mfd/tps65086.c
> @@ -24,6 +24,7 @@
>  static const struct mfd_cell tps65086_cells[] = {
>  	{ .name = "tps65086-regulator", },
>  	{ .name = "tps65086-gpio", },
> +	{ .name = "tps65086-restart", },
>  };
>  
>  static const struct regmap_range tps65086_yes_ranges[] = {
> diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
> index 4d1192062508..4b563db3ab3e 100644
> --- a/drivers/power/reset/Kconfig
> +++ b/drivers/power/reset/Kconfig
> @@ -204,6 +204,12 @@ config POWER_RESET_ST
>  	help
>  	  Reset support for STMicroelectronics boards.
>  
> +config POWER_RESET_TPS65086
> +	bool "TPS65086 restart driver"
> +	depends on MFD_TPS65086
> +	help
> +	  This driver adds support for resetting the TPS65086 PMIC on restart.
> +
>  config POWER_RESET_VERSATILE
>  	bool "ARM Versatile family reboot driver"
>  	depends on ARM
> diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
> index cf3f4d02d8a5..f606a2f60539 100644
> --- a/drivers/power/reset/Makefile
> +++ b/drivers/power/reset/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_POWER_RESET_QNAP) += qnap-poweroff.o
>  obj-$(CONFIG_POWER_RESET_REGULATOR) += regulator-poweroff.o
>  obj-$(CONFIG_POWER_RESET_RESTART) += restart-poweroff.o
>  obj-$(CONFIG_POWER_RESET_ST) += st-poweroff.o
> +obj-$(CONFIG_POWER_RESET_TPS65086) += tps65086-restart.o
>  obj-$(CONFIG_POWER_RESET_VERSATILE) += arm-versatile-reboot.o
>  obj-$(CONFIG_POWER_RESET_VEXPRESS) += vexpress-poweroff.o
>  obj-$(CONFIG_POWER_RESET_XGENE) += xgene-reboot.o
> diff --git a/drivers/power/reset/tps65086-restart.c b/drivers/power/reset/tps65086-restart.c
> new file mode 100644
> index 000000000000..ad9f2c5a84ac
> --- /dev/null
> +++ b/drivers/power/reset/tps65086-restart.c
> @@ -0,0 +1,99 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2021 Emil Renner Berthing
> + */
> +
> +#include <linux/mfd/tps65086.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/reboot.h>
> +
> +struct tps65086_restart {
> +	struct notifier_block handler;
> +	struct tps65086 *tps;
> +	struct device *dev;
> +};
> +
> +static int tps65086_restart_notify(struct notifier_block *this,
> +				   unsigned long mode, void *cmd)
> +{
> +	struct tps65086_restart *tps65086_restart =
> +		container_of(this, struct tps65086_restart, handler);
> +	int ret;
> +
> +	ret = regmap_write(tps65086_restart->tps->regmap, TPS65086_FORCESHUTDN, 1);
> +	if (ret) {
> +		dev_err(tps65086_restart->dev, "%s: error writing to tps65086 pmic: %d\n",
> +			__func__, ret);
> +		return NOTIFY_DONE;
> +	}
> +
> +	/* give it a little time */
> +	mdelay(200);
> +
> +	WARN_ON(1);
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static int tps65086_restart_probe(struct platform_device *pdev)
> +{
> +	struct tps65086_restart *tps65086_restart;
> +	int ret;
> +
> +	tps65086_restart = devm_kzalloc(&pdev->dev, sizeof(*tps65086_restart), GFP_KERNEL);
> +	if (!tps65086_restart)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, tps65086_restart);
> +
> +	tps65086_restart->handler.notifier_call = tps65086_restart_notify;
> +	tps65086_restart->handler.priority = 192;
> +	tps65086_restart->tps = dev_get_drvdata(pdev->dev.parent);
> +	tps65086_restart->dev = &pdev->dev;
> +
> +	ret = register_restart_handler(&tps65086_restart->handler);
> +	if (ret) {
> +		dev_err(&pdev->dev, "%s: cannot register restart handler: %d\n",
> +			__func__, ret);
> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}
> +
> +static int tps65086_restart_remove(struct platform_device *pdev)
> +{
> +	struct tps65086_restart *tps65086_restart = platform_get_drvdata(pdev);
> +	int ret;
> +
> +	ret = unregister_restart_handler(&tps65086_restart->handler);
> +	if (ret) {
> +		dev_err(&pdev->dev, "%s: cannot unregister restart handler: %d\n",
> +			__func__, ret);
> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct platform_device_id tps65086_restart_id_table[] = {
> +	{ "tps65086-restart", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(platform, tps65086_restart_id_table);
> +
> +static struct platform_driver tps65086_restart_driver = {
> +	.driver = {
> +		.name = "tps65086-restart",
> +	},
> +	.probe = tps65086_restart_probe,
> +	.remove = tps65086_restart_remove,
> +	.id_table = tps65086_restart_id_table,
> +};
> +module_platform_driver(tps65086_restart_driver);
> +
> +MODULE_AUTHOR("Emil Renner Berthing <kernel@esmil.dk>");
> +MODULE_DESCRIPTION("TPS65086 restart driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.32.0
> 

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

Re: [PATCH v1 2/3] mfd: tps65086: Make interrupt line optional

[Emil Renner Berthing]

On Tue, 20 Jul 2021 at 18:07, Lee Jones <lee.jones@linaro.org> wrote:
> On Tue, 20 Jul 2021, Emil Renner Berthing wrote:
> > On Tue, 20 Jul 2021 at 17:32, Lee Jones <lee.jones@linaro.org> wrote:
> > > On Tue, 20 Jul 2021, Emil Renner Berthing wrote:
> > > > On Tue, 20 Jul 2021 at 17:07, Lee Jones <lee.jones@linaro.org> wrote:
> > > > > On Sat, 26 Jun 2021, Emil Renner Berthing wrote:
> > > > > > The BeagleV Starlight v0.9 board[1] doesn't have the IRQB line routed to
> > > > > > the SoC, but it is still useful to be able to reach the PMIC over I2C
> > > > > > for the other functionality it provides.
> > > > > >
> > > > > > [1] https://github.com/beagleboard/beaglev-starlight
> > > > > >
> > > > > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> > > > > > ---
> > > > > >  .../devicetree/bindings/mfd/ti,tps65086.yaml  |  3 ---
> > > > >
> > > > > This is not present in my current tree.
> > > > >
> > > > > Looks like it's still *.txt.
> > > > >
> > > > > Am I missing a patch?
> > > >
> > > > Yes, the first patch in the series converts that to yaml. I'm quite
> > > > sure I had the same list of recipients on all 4 mails in the series,
> > > > so don't know why that should be missing.
> > >
> > > Oh, it's not marked as 'important' because it has open review comments
> > > on it.
> > >
> > > Just have this for now then:
> > >
> > > For my own reference (apply this as-is to your sign-off block):
> > >
> > >   Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> >
> > Thanks! Do you want to have a look at 3/3 or should I just send a v2
> > to fix the yaml conversion now?
>
> Patch 3 should be split.

Oh, how split? Split off the series or split into adding the "driver"
and then add the cell to the parent?

> I think it should also s/restart/reset/.

Oh right yes. You mean the cell name needs to be .name =
"tps65086-reset", right?
I'll fix that anyway.

> --
> Lee Jones [李琼斯]
> Senior Technical Lead - Developer Services
> Linaro.org │ Open source software for Arm SoCs
> Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 2/3] mfd: tps65086: Make interrupt line optional

[Lee Jones]

On Tue, 20 Jul 2021, Emil Renner Berthing wrote:

> On Tue, 20 Jul 2021 at 18:07, Lee Jones <lee.jones@linaro.org> wrote:
> > On Tue, 20 Jul 2021, Emil Renner Berthing wrote:
> > > On Tue, 20 Jul 2021 at 17:32, Lee Jones <lee.jones@linaro.org> wrote:
> > > > On Tue, 20 Jul 2021, Emil Renner Berthing wrote:
> > > > > On Tue, 20 Jul 2021 at 17:07, Lee Jones <lee.jones@linaro.org> wrote:
> > > > > > On Sat, 26 Jun 2021, Emil Renner Berthing wrote:
> > > > > > > The BeagleV Starlight v0.9 board[1] doesn't have the IRQB line routed to
> > > > > > > the SoC, but it is still useful to be able to reach the PMIC over I2C
> > > > > > > for the other functionality it provides.
> > > > > > >
> > > > > > > [1] https://github.com/beagleboard/beaglev-starlight
> > > > > > >
> > > > > > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> > > > > > > ---
> > > > > > >  .../devicetree/bindings/mfd/ti,tps65086.yaml  |  3 ---
> > > > > >
> > > > > > This is not present in my current tree.
> > > > > >
> > > > > > Looks like it's still *.txt.
> > > > > >
> > > > > > Am I missing a patch?
> > > > >
> > > > > Yes, the first patch in the series converts that to yaml. I'm quite
> > > > > sure I had the same list of recipients on all 4 mails in the series,
> > > > > so don't know why that should be missing.
> > > >
> > > > Oh, it's not marked as 'important' because it has open review comments
> > > > on it.
> > > >
> > > > Just have this for now then:
> > > >
> > > > For my own reference (apply this as-is to your sign-off block):
> > > >
> > > >   Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> > >
> > > Thanks! Do you want to have a look at 3/3 or should I just send a v2
> > > to fix the yaml conversion now?
> >
> > Patch 3 should be split.
> 
> Oh, how split? Split off the series or split into adding the "driver"
> and then add the cell to the parent?

I'm not overly concerned about the ordering.

Just split out the patch per-subsystem.

> > I think it should also s/restart/reset/.
> 
> Oh right yes. You mean the cell name needs to be .name =
> "tps65086-reset", right?

Right.

> I'll fix that anyway.

Ta.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog