[PATCH v2 0/4] Driver support for RZ/V2M PWC

[PATCH v2 0/4] Driver support for RZ/V2M PWC

[Fabrizio Castro]

The PWC IP found in the RZ/V2M family of chips fits the Multi-Function
Device (MFD) model quite well, and comes with the below capabilities:
* external power supply on/off sequence generation
* on/off signal generation for the LPDDR4 core power supply (LPVDD)
* key input signals processing
* general-purpose output pins

With this new version I have changed model for the DT/dt-bindings.
I have dropped syscon, simple-mfd, regmap, offset, and the child nodes.

Thanks,
Fab

Fabrizio Castro (4):
  dt-bindings: mfd: Add RZ/V2M PWC
  mfd: Add RZ/V2M PWC core driver
  gpio: Add support for the Renesas RZ/V2M PWC GPIOs
  power: reset: Add new driver for RZ/V2M PWC poweroff

 .../bindings/mfd/renesas,rzv2m-pwc.yaml       |  56 ++++++++++
 drivers/gpio/Kconfig                          |  10 ++
 drivers/gpio/Makefile                         |   1 +
 drivers/gpio/gpio-rzv2m-pwc.c                 | 105 ++++++++++++++++++
 drivers/mfd/Kconfig                           |  14 +++
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/rzv2m-pwc.c                       |  70 ++++++++++++
 drivers/mfd/rzv2m-pwc.h                       |  18 +++
 drivers/power/reset/Kconfig                   |   9 ++
 drivers/power/reset/Makefile                  |   1 +
 drivers/power/reset/rzv2m-pwc-poweroff.c      |  67 +++++++++++
 11 files changed, 352 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/renesas,rzv2m-pwc.yaml
 create mode 100644 drivers/gpio/gpio-rzv2m-pwc.c
 create mode 100644 drivers/mfd/rzv2m-pwc.c
 create mode 100644 drivers/mfd/rzv2m-pwc.h
 create mode 100644 drivers/power/reset/rzv2m-pwc-poweroff.c

-- 
2.34.1

[PATCH v2 1/4] dt-bindings: mfd: Add RZ/V2M PWC

[Fabrizio Castro]

The Renesas RZ/V2M External Power Sequence Controller (PWC)
IP is a multi-function device, and it's capable of:
* external power supply on/off sequence generation
* on/off signal generation for the LPDDR4 core power supply (LPVDD)
* key input signals processing
* general-purpose output pins

Add the corresponding dt-bindings.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---

v1->v2: I have dropped syscon, simple-mfd, regmap, offset, and the child nodes.

 .../bindings/mfd/renesas,rzv2m-pwc.yaml       | 56 +++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/renesas,rzv2m-pwc.yaml

diff --git a/Documentation/devicetree/bindings/mfd/renesas,rzv2m-pwc.yaml b/Documentation/devicetree/bindings/mfd/renesas,rzv2m-pwc.yaml
new file mode 100644
index 000000000000..e6794c5152d5
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/renesas,rzv2m-pwc.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/renesas,rzv2m-pwc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/V2M External Power Sequence Controller (PWC)
+
+description: |+
+  The PWC IP found in the RZ/V2M family of chips comes with the below
+  capabilities
+    - external power supply on/off sequence generation
+    - on/off signal generation for the LPDDR4 core power supply (LPVDD)
+    - key input signals processing
+    - general-purpose output pins
+
+maintainers:
+  - Fabrizio Castro <fabrizio.castro.jz@renesas.com>
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - renesas,r9a09g011-pwc # RZ/V2M
+          - renesas,r9a09g055-pwc # RZ/V2MA
+      - const: renesas,rzv2m-pwc
+
+  reg:
+    maxItems: 1
+
+  gpio-controller: true
+
+  '#gpio-cells':
+    const: 2
+
+  renesas,rzv2m-pwc-power:
+    description: The PWC is used to control the system power supplies.
+    type: boolean
+
+required:
+  - compatible
+  - reg
+  - gpio-controller
+  - '#gpio-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    pwc: pwc@a3700000 {
+      compatible = "renesas,r9a09g011-pwc", "renesas,rzv2m-pwc";
+      reg = <0xa3700000 0x800>;
+      gpio-controller;
+      #gpio-cells = <2>;
+      renesas,rzv2m-pwc-power;
+    };
-- 
2.34.1

[PATCH v2 2/4] mfd: Add RZ/V2M PWC core driver

[Fabrizio Castro]

The External Power Sequence Controller (PWC) IP (found in the
RZ/V2M SoC) is a controller for external power supplies (regulators
and power switches), and it supports the following features: it
generates a power on/off sequence for external power supplies,
it generates an on/off sequence for the LPDDR4 core power supply
(LPVDD), it comes with General-Purpose Outputs, and it processes
key input signals.

The PWC is basically a Multi-Function Device (MFD), its software
support comes with a core driver, and specialized drivers for
its specific features.

This patch adds the core driver for the RZ/V2M PWC IP.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---

v1->v2: This is a new driver, to match the relevant compatible string
	and instantiate the relevant mfd device drivers

 drivers/mfd/Kconfig     | 14 +++++++++
 drivers/mfd/Makefile    |  1 +
 drivers/mfd/rzv2m-pwc.c | 70 +++++++++++++++++++++++++++++++++++++++++
 drivers/mfd/rzv2m-pwc.h | 18 +++++++++++
 4 files changed, 103 insertions(+)
 create mode 100644 drivers/mfd/rzv2m-pwc.c
 create mode 100644 drivers/mfd/rzv2m-pwc.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 30db49f31866..ac4403e4f3cb 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2265,5 +2265,19 @@ config MFD_RSMU_SPI
 	  Additional drivers must be enabled in order to use the functionality
 	  of the device.
 
+config MFD_RZV2M_PWC_CORE
+	tristate "Renesas RZ/V2M PWC Core Driver"
+	select MFD_CORE
+	depends on ARCH_R9A09G011 || COMPILE_TEST
+	help
+	  Select this option to enable the RZ/V2M External Power Sequence
+	  Controller (PWC) core driver.
+
+	  The PWC is a controller for external power supplies (regulators and
+	  power switches), and it supports the following features: it generates
+	  a power on/off sequence for external power supplies, it generates an
+	  on/off sequence for the LPDDR4 core power supply (LPVDD), it comes
+	  with General-Purpose Outputs, and it processes key input signals.
+
 endmenu
 endif
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 457471478a93..e39252a2df23 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -278,3 +278,4 @@ rsmu-i2c-objs			:= rsmu_core.o rsmu_i2c.o
 rsmu-spi-objs			:= rsmu_core.o rsmu_spi.o
 obj-$(CONFIG_MFD_RSMU_I2C)	+= rsmu-i2c.o
 obj-$(CONFIG_MFD_RSMU_SPI)	+= rsmu-spi.o
+obj-$(CONFIG_MFD_RZV2M_PWC_CORE) += rzv2m-pwc.o
diff --git a/drivers/mfd/rzv2m-pwc.c b/drivers/mfd/rzv2m-pwc.c
new file mode 100644
index 000000000000..f9055fcafda2
--- /dev/null
+++ b/drivers/mfd/rzv2m-pwc.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 Renesas Electronics Corporation
+ *
+ * Core driver for the Renesas RZ/V2M External Power Sequence Controller (PWC)
+ */
+
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/core.h>
+#include "rzv2m-pwc.h"
+
+static const struct mfd_cell rzv2m_pwc_gpio_devs[] = {
+	{ .name = "gpio_rzv2m_pwc", },
+};
+
+static const struct mfd_cell rzv2m_pwc_poweroff_devs[] = {
+	{ .name = "rzv2m_pwc_poweroff", },
+};
+
+static int rzv2m_pwc_probe(struct platform_device *pdev)
+{
+	struct rzv2m_pwc_priv *priv;
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	platform_set_drvdata(pdev, priv);
+
+	ret = devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_AUTO,
+				   rzv2m_pwc_gpio_devs,
+				   ARRAY_SIZE(rzv2m_pwc_gpio_devs), NULL, 0,
+				   NULL);
+	if (ret)
+		return ret;
+
+	if (of_property_read_bool(pdev->dev.of_node, "renesas,rzv2m-pwc-power"))
+		ret = devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_AUTO,
+					   rzv2m_pwc_poweroff_devs,
+					   ARRAY_SIZE(rzv2m_pwc_poweroff_devs),
+					   NULL, 0, NULL);
+
+	return ret;
+}
+
+static const struct of_device_id rzv2m_pwc_of_match[] = {
+	{ .compatible = "renesas,rzv2m-pwc" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, rzv2m_pwc_of_match);
+
+static struct platform_driver rzv2m_pwc_driver = {
+	.probe = rzv2m_pwc_probe,
+	.driver = {
+		.name = "rzv2m_pwc",
+		.of_match_table = of_match_ptr(rzv2m_pwc_of_match),
+	},
+};
+module_platform_driver(rzv2m_pwc_driver);
+
+MODULE_SOFTDEP("post: gpio_rzv2m_pwc rzv2m_pwc_poweroff");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Fabrizio Castro <castro.fabrizio.jz@renesas.com>");
+MODULE_DESCRIPTION("Renesas RZ/V2M PWC core driver");
diff --git a/drivers/mfd/rzv2m-pwc.h b/drivers/mfd/rzv2m-pwc.h
new file mode 100644
index 000000000000..8f3d777557c9
--- /dev/null
+++ b/drivers/mfd/rzv2m-pwc.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022 Renesas Electronics Corporation
+ */
+
+#ifndef __LINUX_RZV2M_PWC_H__
+#define __LINUX_RZV2M_PWC_H__
+
+#define PWC_PWCRST			0x00
+#define PWC_PWCCKEN			0x04
+#define PWC_PWCCTL			0x50
+#define PWC_GPIO			0x80
+
+struct rzv2m_pwc_priv {
+	void __iomem *base;
+};
+
+#endif /* __LINUX_RZV2M_PWC_H__ */
-- 
2.34.1

[PATCH v2 3/4] gpio: Add support for the Renesas RZ/V2M PWC GPIOs

[Fabrizio Castro]

The RZ/V2M SoC contains an External Power Sequence Controller
(PWC) module. The PWC module provides an external power supply
on/off sequence, on/off signal for the LPDDR4 core power supply,
General-Purpose Outputs, and key input signals.

Add a driver for controlling the General-Purpose Outputs.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---

v1->v2: Dropped OF match table and syscon as a result of the change in
        DT model

 drivers/gpio/Kconfig          |  10 ++++
 drivers/gpio/Makefile         |   1 +
 drivers/gpio/gpio-rzv2m-pwc.c | 105 ++++++++++++++++++++++++++++++++++
 3 files changed, 116 insertions(+)
 create mode 100644 drivers/gpio/gpio-rzv2m-pwc.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index ec7cfd4f52b1..4c77fb6966e0 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -553,6 +553,16 @@ config GPIO_ROCKCHIP
 	help
 	  Say yes here to support GPIO on Rockchip SoCs.
 
+config GPIO_RZV2M_PWC
+	tristate "Renesas RZ/V2M PWC GPIO support"
+	depends on MFD_RZV2M_PWC_CORE || COMPILE_TEST
+	help
+	  Say yes here to support the External Power Sequence Controller (PWC)
+	  GPIO controller driver for RZ/V2M devices.
+
+	  The PWSDxSEL pins can be used as General-Purpose Ouputs.
+	  Their output is low by default.
+
 config GPIO_SAMA5D2_PIOBU
 	tristate "SAMA5D2 PIOBU GPIO support"
 	depends on MFD_SYSCON
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 010587025fc8..a5c159ae9db5 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -132,6 +132,7 @@ obj-$(CONFIG_GPIO_RDC321X)		+= gpio-rdc321x.o
 obj-$(CONFIG_GPIO_REALTEK_OTTO)		+= gpio-realtek-otto.o
 obj-$(CONFIG_GPIO_REG)			+= gpio-reg.o
 obj-$(CONFIG_GPIO_ROCKCHIP)	+= gpio-rockchip.o
+obj-$(CONFIG_GPIO_RZV2M_PWC)		+= gpio-rzv2m-pwc.o
 obj-$(CONFIG_ARCH_SA1100)		+= gpio-sa1100.o
 obj-$(CONFIG_GPIO_SAMA5D2_PIOBU)	+= gpio-sama5d2-piobu.o
 obj-$(CONFIG_GPIO_SCH311X)		+= gpio-sch311x.o
diff --git a/drivers/gpio/gpio-rzv2m-pwc.c b/drivers/gpio/gpio-rzv2m-pwc.c
new file mode 100644
index 000000000000..19bdb949b3d3
--- /dev/null
+++ b/drivers/gpio/gpio-rzv2m-pwc.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 Renesas Electronics Corporation
+ *
+ * GPIO driver for Renesas RZ/V2M External Power Sequence Controller (PWC)
+ */
+
+#include <linux/gpio/driver.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include "../mfd/rzv2m-pwc.h"
+
+struct rzv2m_pwc_gpio_priv {
+	void __iomem *base;
+	struct gpio_chip gp;
+	DECLARE_BITMAP(ch_en_bits, 2);
+};
+
+static void rzv2m_pwc_gpio_set(struct gpio_chip *chip, unsigned int offset,
+			       int value)
+{
+	struct rzv2m_pwc_gpio_priv *priv = gpiochip_get_data(chip);
+	u32 reg;
+
+	/* BIT 16 enables write to BIT 0, and BIT 17 enables write to BIT 1 */
+	reg = BIT(offset + 16);
+	if (value)
+		reg |= BIT(offset);
+
+	writel(reg, priv->base + PWC_GPIO);
+
+	if (value)
+		set_bit(offset, priv->ch_en_bits);
+	else
+		clear_bit(offset, priv->ch_en_bits);
+}
+
+static int rzv2m_pwc_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct rzv2m_pwc_gpio_priv *priv = gpiochip_get_data(chip);
+
+	return test_bit(offset, priv->ch_en_bits);
+}
+
+static int rzv2m_pwc_gpio_direction_output(struct gpio_chip *gc,
+					   unsigned int nr, int value)
+{
+	if (nr > 1)
+		return -EINVAL;
+
+	rzv2m_pwc_gpio_set(gc, nr, value);
+
+	return 0;
+}
+
+static const struct gpio_chip rzv2m_pwc_gc = {
+	.label = "gpio_rzv2m_pwc",
+	.owner = THIS_MODULE,
+	.get = rzv2m_pwc_gpio_get,
+	.set = rzv2m_pwc_gpio_set,
+	.direction_output = rzv2m_pwc_gpio_direction_output,
+	.can_sleep = false,
+	.ngpio = 2,
+	.base = -1,
+};
+
+static int rzv2m_pwc_gpio_probe(struct platform_device *pdev)
+{
+	struct rzv2m_pwc_priv *pdata = dev_get_drvdata(pdev->dev.parent);
+	struct rzv2m_pwc_gpio_priv *priv;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->base = pdata->base;
+	/*
+	 * The register used by this driver cannot be read, therefore set the
+	 * outputs to their default values and initialize priv->ch_en_bits
+	 * accordingly. BIT 16 enables write to BIT 0, BIT 17 enables write to
+	 * BIT 1, and the default value of both BIT 0 and BIT 1 is 0.
+	 */
+	writel(BIT(17) | BIT(16), priv->base + PWC_GPIO);
+	bitmap_zero(priv->ch_en_bits, 2);
+
+	priv->gp = rzv2m_pwc_gc;
+	priv->gp.parent = pdev->dev.parent;
+	priv->gp.fwnode = dev_fwnode(pdev->dev.parent);
+
+	return devm_gpiochip_add_data(&pdev->dev, &priv->gp, priv);
+}
+
+static struct platform_driver rzv2m_pwc_gpio_driver = {
+	.probe = rzv2m_pwc_gpio_probe,
+	.driver = {
+		.name = "gpio_rzv2m_pwc",
+	},
+};
+module_platform_driver(rzv2m_pwc_gpio_driver);
+
+MODULE_ALIAS("platform:gpio_rzv2m_pwc");
+MODULE_SOFTDEP("pre: rzv2m_pwc");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Fabrizio Castro <castro.fabrizio.jz@renesas.com>");
+MODULE_DESCRIPTION("Renesas RZ/V2M PWC GPIO");
-- 
2.34.1

[PATCH v2 4/4] power: reset: Add new driver for RZ/V2M PWC poweroff

[Fabrizio Castro]

The RZ/V2M PWC IP controls external power supplies and therefore
can turn the power supplies off when powering down the system.

Add driver to poweroff the system.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---

v1->v2: Dropped OF match table and syscon as a result of the change in
        DT model

 drivers/power/reset/Kconfig              |  9 ++++
 drivers/power/reset/Makefile             |  1 +
 drivers/power/reset/rzv2m-pwc-poweroff.c | 67 ++++++++++++++++++++++++
 3 files changed, 77 insertions(+)
 create mode 100644 drivers/power/reset/rzv2m-pwc-poweroff.c

diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index a8c46ba5878f..1fcf691ae68e 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -303,4 +303,13 @@ config POWER_MLXBF
 	help
 	  This driver supports reset or low power mode handling for Mellanox BlueField.
 
+config POWER_RESET_RZV2M_PWC
+	tristate "Renesas RZ/V2M PWC Power OFF support"
+	depends on MFD_RZV2M_PWC_CORE || COMPILE_TEST
+	help
+	  The RZ/V2M PWC IP controls external power supplies and therefore can
+	  turn the power supplies off when powering down the system.
+	  Enable this driver when PWC is in control of the system power supplies
+	  and it's the preferred way to shutdown the system.
+
 endif
diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
index 0a39424fc558..f05a8abff2eb 100644
--- a/drivers/power/reset/Makefile
+++ b/drivers/power/reset/Makefile
@@ -36,3 +36,4 @@ obj-$(CONFIG_SYSCON_REBOOT_MODE) += syscon-reboot-mode.o
 obj-$(CONFIG_POWER_RESET_SC27XX) += sc27xx-poweroff.o
 obj-$(CONFIG_NVMEM_REBOOT_MODE) += nvmem-reboot-mode.o
 obj-$(CONFIG_POWER_MLXBF) += pwr-mlxbf.o
+obj-$(CONFIG_POWER_RESET_RZV2M_PWC) += rzv2m-pwc-poweroff.o
diff --git a/drivers/power/reset/rzv2m-pwc-poweroff.c b/drivers/power/reset/rzv2m-pwc-poweroff.c
new file mode 100644
index 000000000000..f5bc383c22e1
--- /dev/null
+++ b/drivers/power/reset/rzv2m-pwc-poweroff.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 Renesas Electronics Corporation
+ *
+ * Reset driver for Renesas RZ/V2M External Power Sequence Controller (PWC)
+ */
+
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/reboot.h>
+#include "../../mfd/rzv2m-pwc.h"
+
+#define PWC_PWCRST_RSTSOFTAX		0x1
+#define PWC_PWCCKEN_ENGCKMAIN		0x1
+#define PWC_PWCCTL_PWOFF		0x1
+
+struct rzv2m_pwc_poweroff_priv {
+	void __iomem *base;
+	struct device *dev;
+};
+
+static int rzv2m_pwc_poweroff(struct sys_off_data *data)
+{
+	struct rzv2m_pwc_poweroff_priv *priv =
+		(struct rzv2m_pwc_poweroff_priv *)data->cb_data;
+
+	writel(PWC_PWCRST_RSTSOFTAX, priv->base + PWC_PWCRST);
+	writel(PWC_PWCCKEN_ENGCKMAIN, priv->base + PWC_PWCCKEN);
+	writel(PWC_PWCCTL_PWOFF, priv->base + PWC_PWCCTL);
+
+	mdelay(150);
+
+	dev_err(priv->dev, "Failed to power off the system");
+
+	return NOTIFY_DONE;
+}
+
+static int rzv2m_pwc_poweroff_probe(struct platform_device *pdev)
+{
+	struct rzv2m_pwc_priv *pdata = dev_get_drvdata(pdev->dev.parent);
+	struct rzv2m_pwc_poweroff_priv *priv;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->base = pdata->base;
+	priv->dev = &pdev->dev;
+
+	return devm_register_power_off_handler(&pdev->dev, rzv2m_pwc_poweroff,
+					       priv);
+}
+
+static struct platform_driver rzv2m_pwc_poweroff_driver = {
+	.probe = rzv2m_pwc_poweroff_probe,
+	.driver = {
+		.name = "rzv2m_pwc_poweroff",
+	},
+};
+module_platform_driver(rzv2m_pwc_poweroff_driver);
+
+MODULE_ALIAS("platform:rzv2m_pwc_poweroff");
+MODULE_SOFTDEP("pre: rzv2m_pwc");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Fabrizio Castro <castro.fabrizio.jz@renesas.com>");
+MODULE_DESCRIPTION("Renesas RZ/V2M PWC power OFF driver");
-- 
2.34.1

Re: [PATCH v2 1/4] dt-bindings: mfd: Add RZ/V2M PWC

[Rob Herring]

On Wed, 21 Dec 2022 21:09:14 +0000, Fabrizio Castro wrote:
> The Renesas RZ/V2M External Power Sequence Controller (PWC)
> IP is a multi-function device, and it's capable of:
> * external power supply on/off sequence generation
> * on/off signal generation for the LPDDR4 core power supply (LPVDD)
> * key input signals processing
> * general-purpose output pins
> 
> Add the corresponding dt-bindings.
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> ---
> 
> v1->v2: I have dropped syscon, simple-mfd, regmap, offset, and the child nodes.
> 
>  .../bindings/mfd/renesas,rzv2m-pwc.yaml       | 56 +++++++++++++++++++
>  1 file changed, 56 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/renesas,rzv2m-pwc.yaml
> 

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

Re: [PATCH v2 3/4] gpio: Add support for the Renesas RZ/V2M PWC GPIOs

[Linus Walleij]

On Wed, Dec 21, 2022 at 10:09 PM Fabrizio Castro
<fabrizio.castro.jz@renesas.com> wrote:

> The RZ/V2M SoC contains an External Power Sequence Controller
> (PWC) module. The PWC module provides an external power supply
> on/off sequence, on/off signal for the LPDDR4 core power supply,
> General-Purpose Outputs, and key input signals.
>
> Add a driver for controlling the General-Purpose Outputs.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>

This is a nice driver.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v2 4/4] power: reset: Add new driver for RZ/V2M PWC poweroff

[Sebastian Reichel]

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

Hi,

On Wed, Dec 21, 2022 at 09:09:17PM +0000, Fabrizio Castro wrote:
> The RZ/V2M PWC IP controls external power supplies and therefore
> can turn the power supplies off when powering down the system.
> 
> Add driver to poweroff the system.
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> ---

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

-- Sebastian

> 
> v1->v2: Dropped OF match table and syscon as a result of the change in
>         DT model
> 
>  drivers/power/reset/Kconfig              |  9 ++++
>  drivers/power/reset/Makefile             |  1 +
>  drivers/power/reset/rzv2m-pwc-poweroff.c | 67 ++++++++++++++++++++++++
>  3 files changed, 77 insertions(+)
>  create mode 100644 drivers/power/reset/rzv2m-pwc-poweroff.c
> 
> diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
> index a8c46ba5878f..1fcf691ae68e 100644
> --- a/drivers/power/reset/Kconfig
> +++ b/drivers/power/reset/Kconfig
> @@ -303,4 +303,13 @@ config POWER_MLXBF
>  	help
>  	  This driver supports reset or low power mode handling for Mellanox BlueField.
>  
> +config POWER_RESET_RZV2M_PWC
> +	tristate "Renesas RZ/V2M PWC Power OFF support"
> +	depends on MFD_RZV2M_PWC_CORE || COMPILE_TEST
> +	help
> +	  The RZ/V2M PWC IP controls external power supplies and therefore can
> +	  turn the power supplies off when powering down the system.
> +	  Enable this driver when PWC is in control of the system power supplies
> +	  and it's the preferred way to shutdown the system.
> +
>  endif
> diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
> index 0a39424fc558..f05a8abff2eb 100644
> --- a/drivers/power/reset/Makefile
> +++ b/drivers/power/reset/Makefile
> @@ -36,3 +36,4 @@ obj-$(CONFIG_SYSCON_REBOOT_MODE) += syscon-reboot-mode.o
>  obj-$(CONFIG_POWER_RESET_SC27XX) += sc27xx-poweroff.o
>  obj-$(CONFIG_NVMEM_REBOOT_MODE) += nvmem-reboot-mode.o
>  obj-$(CONFIG_POWER_MLXBF) += pwr-mlxbf.o
> +obj-$(CONFIG_POWER_RESET_RZV2M_PWC) += rzv2m-pwc-poweroff.o
> diff --git a/drivers/power/reset/rzv2m-pwc-poweroff.c b/drivers/power/reset/rzv2m-pwc-poweroff.c
> new file mode 100644
> index 000000000000..f5bc383c22e1
> --- /dev/null
> +++ b/drivers/power/reset/rzv2m-pwc-poweroff.c
> @@ -0,0 +1,67 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2022 Renesas Electronics Corporation
> + *
> + * Reset driver for Renesas RZ/V2M External Power Sequence Controller (PWC)
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <linux/reboot.h>
> +#include "../../mfd/rzv2m-pwc.h"
> +
> +#define PWC_PWCRST_RSTSOFTAX		0x1
> +#define PWC_PWCCKEN_ENGCKMAIN		0x1
> +#define PWC_PWCCTL_PWOFF		0x1
> +
> +struct rzv2m_pwc_poweroff_priv {
> +	void __iomem *base;
> +	struct device *dev;
> +};
> +
> +static int rzv2m_pwc_poweroff(struct sys_off_data *data)
> +{
> +	struct rzv2m_pwc_poweroff_priv *priv =
> +		(struct rzv2m_pwc_poweroff_priv *)data->cb_data;
> +
> +	writel(PWC_PWCRST_RSTSOFTAX, priv->base + PWC_PWCRST);
> +	writel(PWC_PWCCKEN_ENGCKMAIN, priv->base + PWC_PWCCKEN);
> +	writel(PWC_PWCCTL_PWOFF, priv->base + PWC_PWCCTL);
> +
> +	mdelay(150);
> +
> +	dev_err(priv->dev, "Failed to power off the system");
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static int rzv2m_pwc_poweroff_probe(struct platform_device *pdev)
> +{
> +	struct rzv2m_pwc_priv *pdata = dev_get_drvdata(pdev->dev.parent);
> +	struct rzv2m_pwc_poweroff_priv *priv;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->base = pdata->base;
> +	priv->dev = &pdev->dev;
> +
> +	return devm_register_power_off_handler(&pdev->dev, rzv2m_pwc_poweroff,
> +					       priv);
> +}
> +
> +static struct platform_driver rzv2m_pwc_poweroff_driver = {
> +	.probe = rzv2m_pwc_poweroff_probe,
> +	.driver = {
> +		.name = "rzv2m_pwc_poweroff",
> +	},
> +};
> +module_platform_driver(rzv2m_pwc_poweroff_driver);
> +
> +MODULE_ALIAS("platform:rzv2m_pwc_poweroff");
> +MODULE_SOFTDEP("pre: rzv2m_pwc");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Fabrizio Castro <castro.fabrizio.jz@renesas.com>");
> +MODULE_DESCRIPTION("Renesas RZ/V2M PWC power OFF driver");
> -- 
> 2.34.1
> 

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

Re: [PATCH v2 4/4] power: reset: Add new driver for RZ/V2M PWC poweroff

[Geert Uytterhoeven]

Hi Fabrizio,

On Wed, Dec 21, 2022 at 10:09 PM Fabrizio Castro
<fabrizio.castro.jz@renesas.com> wrote:
> The RZ/V2M PWC IP controls external power supplies and therefore
> can turn the power supplies off when powering down the system.
>
> Add driver to poweroff the system.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> ---
>
> v1->v2: Dropped OF match table and syscon as a result of the change in
>         DT model

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/power/reset/rzv2m-pwc-poweroff.c
> @@ -0,0 +1,67 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2022 Renesas Electronics Corporation
> + *
> + * Reset driver for Renesas RZ/V2M External Power Sequence Controller (PWC)
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <linux/reboot.h>
> +#include "../../mfd/rzv2m-pwc.h"
> +
> +#define PWC_PWCRST_RSTSOFTAX           0x1
> +#define PWC_PWCCKEN_ENGCKMAIN          0x1
> +#define PWC_PWCCTL_PWOFF               0x1
> +
> +struct rzv2m_pwc_poweroff_priv {
> +       void __iomem *base;
> +       struct device *dev;
> +};
> +
> +static int rzv2m_pwc_poweroff(struct sys_off_data *data)
> +{
> +       struct rzv2m_pwc_poweroff_priv *priv =
> +               (struct rzv2m_pwc_poweroff_priv *)data->cb_data;

No need for this cast.

> +
> +       writel(PWC_PWCRST_RSTSOFTAX, priv->base + PWC_PWCRST);
> +       writel(PWC_PWCCKEN_ENGCKMAIN, priv->base + PWC_PWCCKEN);
> +       writel(PWC_PWCCTL_PWOFF, priv->base + PWC_PWCCTL);
> +
> +       mdelay(150);
> +
> +       dev_err(priv->dev, "Failed to power off the system");
> +
> +       return NOTIFY_DONE;
> +}

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 1/4] dt-bindings: mfd: Add RZ/V2M PWC

[Geert Uytterhoeven]

Hi Fabrizio,

On Wed, Dec 21, 2022 at 10:09 PM Fabrizio Castro
<fabrizio.castro.jz@renesas.com> wrote:
> The Renesas RZ/V2M External Power Sequence Controller (PWC)
> IP is a multi-function device, and it's capable of:
> * external power supply on/off sequence generation
> * on/off signal generation for the LPDDR4 core power supply (LPVDD)
> * key input signals processing
> * general-purpose output pins
>
> Add the corresponding dt-bindings.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> ---
>
> v1->v2: I have dropped syscon, simple-mfd, regmap, offset, and the child nodes.

Thanks for the update!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/renesas,rzv2m-pwc.yaml
> @@ -0,0 +1,56 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/renesas,rzv2m-pwc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas RZ/V2M External Power Sequence Controller (PWC)
> +
> +description: |+
> +  The PWC IP found in the RZ/V2M family of chips comes with the below
> +  capabilities
> +    - external power supply on/off sequence generation
> +    - on/off signal generation for the LPDDR4 core power supply (LPVDD)
> +    - key input signals processing
> +    - general-purpose output pins
> +
> +maintainers:
> +  - Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - renesas,r9a09g011-pwc # RZ/V2M
> +          - renesas,r9a09g055-pwc # RZ/V2MA
> +      - const: renesas,rzv2m-pwc
> +
> +  reg:
> +    maxItems: 1
> +
> +  gpio-controller: true
> +
> +  '#gpio-cells':
> +    const: 2
> +
> +  renesas,rzv2m-pwc-power:
> +    description: The PWC is used to control the system power supplies.
> +    type: boolean

I'm wondering if there is some other way to represent this, e.g.
using DT topology? Some regulator relation?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 2/4] mfd: Add RZ/V2M PWC core driver

[Geert Uytterhoeven]

Hi Fabrizio,

On Wed, Dec 21, 2022 at 10:09 PM Fabrizio Castro
<fabrizio.castro.jz@renesas.com> wrote:
> The External Power Sequence Controller (PWC) IP (found in the
> RZ/V2M SoC) is a controller for external power supplies (regulators
> and power switches), and it supports the following features: it
> generates a power on/off sequence for external power supplies,
> it generates an on/off sequence for the LPDDR4 core power supply
> (LPVDD), it comes with General-Purpose Outputs, and it processes
> key input signals.

Thanks for your patch!

> The PWC is basically a Multi-Function Device (MFD), its software
> support comes with a core driver, and specialized drivers for
> its specific features.

I have to admit I'm not such a big fan of MFD.  In this driver,
you are not even sharing resources in the MFD cells, just the mapped
register base.  So I think you can easily save +100 LoC and reduce
maintenance synchronization overhead across subsystems by just having
a single non-MFD driver instead.

Did you pick MFD because the PWC poweroff feature depends on board
wiring, and thus is optional?

Are there any other MFD cells planned (e.g. regulators) to be added
later? The public datasheet does not list the actual registers of the
block, only a high-level overview with (rather detailed) behavioral
information.

Thanks!

> --- /dev/null
> +++ b/drivers/mfd/rzv2m-pwc.h

> +struct rzv2m_pwc_priv {
> +       void __iomem *base;
> +};
> +
> +#endif /* __LINUX_RZV2M_PWC_H__ */

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH v2 2/4] mfd: Add RZ/V2M PWC core driver

[Fabrizio Castro]

Hi Geert,

Thanks for your feedback!

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 03 January 2023 08:37
> To: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>; Bartosz Golaszewski
> <brgl@bgdev.pl>; Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt@linaro.org>; Sebastian Reichel <sre@kernel.org>;
> Geert Uytterhoeven <geert+renesas@glider.be>; Lee Jones <lee@kernel.org>;
> linux-gpio@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-pm@vger.kernel.org; Chris Paterson
> <Chris.Paterson2@renesas.com>; Biju Das <biju.das@bp.renesas.com>; linux-
> renesas-soc@vger.kernel.org; Laurent Pinchart
> <laurent.pinchart@ideasonboard.com>; Jacopo Mondi <jacopo@jmondi.org>
> Subject: Re: [PATCH v2 2/4] mfd: Add RZ/V2M PWC core driver
> 
> Hi Fabrizio,
> 
> On Wed, Dec 21, 2022 at 10:09 PM Fabrizio Castro
> <fabrizio.castro.jz@renesas.com> wrote:
> > The External Power Sequence Controller (PWC) IP (found in the
> > RZ/V2M SoC) is a controller for external power supplies (regulators
> > and power switches), and it supports the following features: it
> > generates a power on/off sequence for external power supplies,
> > it generates an on/off sequence for the LPDDR4 core power supply
> > (LPVDD), it comes with General-Purpose Outputs, and it processes
> > key input signals.
> 
> Thanks for your patch!
> 
> > The PWC is basically a Multi-Function Device (MFD), its software
> > support comes with a core driver, and specialized drivers for
> > its specific features.
> 
> I have to admit I'm not such a big fan of MFD.  In this driver,
> you are not even sharing resources in the MFD cells, just the mapped
> register base.  So I think you can easily save +100 LoC and reduce
> maintenance synchronization overhead across subsystems by just having
> a single non-MFD driver instead.
> 
> Did you pick MFD because the PWC poweroff feature depends on board
> wiring, and thus is optional?

I am not a big fan of MFD, either.
I picked MFD because we were not 100% sure of what the IP could do
when we started working on it.
I have received more information regarding the IP now (which I don't
have the liberty to discuss), I am still not 100% sure that's all
of it, but basically its support may require expansion later on.

I liked the solution based on syscon and simple-mfd for several reasons,
but having dropped syscon and simple-mfd due to issues with the dt-bindings
I have moved on with a core driver to instantiate the required SW support.
We could of course move to a unified driver if that makes more sense?
If we were to move to unified driver, under which directory would you
suggest we put it?

> 
> Are there any other MFD cells planned (e.g. regulators) to be added
> later? The public datasheet does not list the actual registers of the
> block, only a high-level overview with (rather detailed) behavioral
> information.

No MFD cells are planned for now, but I can't exclude we won't
have any in the future.

Thanks,
Fab

> 
> Thanks!
> 
> > --- /dev/null
> > +++ b/drivers/mfd/rzv2m-pwc.h
> 
> > +struct rzv2m_pwc_priv {
> > +       void __iomem *base;
> > +};
> > +
> > +#endif /* __LINUX_RZV2M_PWC_H__ */
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker.
> But
> when I'm talking to journalists I just say "programmer" or something like
> that.
>                                 -- Linus Torvalds

Re: [PATCH v2 2/4] mfd: Add RZ/V2M PWC core driver

[Geert Uytterhoeven]

Hi Fabrizio,

On Tue, Jan 3, 2023 at 1:05 PM Fabrizio Castro
<fabrizio.castro.jz@renesas.com> wrote:
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > On Wed, Dec 21, 2022 at 10:09 PM Fabrizio Castro
> > <fabrizio.castro.jz@renesas.com> wrote:
> > > The External Power Sequence Controller (PWC) IP (found in the
> > > RZ/V2M SoC) is a controller for external power supplies (regulators
> > > and power switches), and it supports the following features: it
> > > generates a power on/off sequence for external power supplies,
> > > it generates an on/off sequence for the LPDDR4 core power supply
> > > (LPVDD), it comes with General-Purpose Outputs, and it processes
> > > key input signals.
> >
> > Thanks for your patch!
> >
> > > The PWC is basically a Multi-Function Device (MFD), its software
> > > support comes with a core driver, and specialized drivers for
> > > its specific features.
> >
> > I have to admit I'm not such a big fan of MFD.  In this driver,
> > you are not even sharing resources in the MFD cells, just the mapped
> > register base.  So I think you can easily save +100 LoC and reduce
> > maintenance synchronization overhead across subsystems by just having
> > a single non-MFD driver instead.
> >
> > Did you pick MFD because the PWC poweroff feature depends on board
> > wiring, and thus is optional?
>
> I am not a big fan of MFD, either.
> I picked MFD because we were not 100% sure of what the IP could do
> when we started working on it.
> I have received more information regarding the IP now (which I don't
> have the liberty to discuss), I am still not 100% sure that's all
> of it, but basically its support may require expansion later on.
>
> I liked the solution based on syscon and simple-mfd for several reasons,
> but having dropped syscon and simple-mfd due to issues with the dt-bindings
> I have moved on with a core driver to instantiate the required SW support.
> We could of course move to a unified driver if that makes more sense?
> If we were to move to unified driver, under which directory would you
> suggest we put it?

As the GPIO part is larger than the poweroff part, I would put it under
drivers/gpio/.  Although drivers/soc/renesas/ could be an option, too.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 2/4] mfd: Add RZ/V2M PWC core driver

[Lee Jones]

On Tue, 03 Jan 2023, Fabrizio Castro wrote:

> Hi Geert,
> 
> Thanks for your feedback!
> 
> > -----Original Message-----
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > Sent: 03 January 2023 08:37
> > To: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> > Cc: Linus Walleij <linus.walleij@linaro.org>; Bartosz Golaszewski
> > <brgl@bgdev.pl>; Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski
> > <krzysztof.kozlowski+dt@linaro.org>; Sebastian Reichel <sre@kernel.org>;
> > Geert Uytterhoeven <geert+renesas@glider.be>; Lee Jones <lee@kernel.org>;
> > linux-gpio@vger.kernel.org; devicetree@vger.kernel.org; linux-
> > kernel@vger.kernel.org; linux-pm@vger.kernel.org; Chris Paterson
> > <Chris.Paterson2@renesas.com>; Biju Das <biju.das@bp.renesas.com>; linux-
> > renesas-soc@vger.kernel.org; Laurent Pinchart
> > <laurent.pinchart@ideasonboard.com>; Jacopo Mondi <jacopo@jmondi.org>
> > Subject: Re: [PATCH v2 2/4] mfd: Add RZ/V2M PWC core driver
> > 
> > Hi Fabrizio,
> > 
> > On Wed, Dec 21, 2022 at 10:09 PM Fabrizio Castro
> > <fabrizio.castro.jz@renesas.com> wrote:
> > > The External Power Sequence Controller (PWC) IP (found in the
> > > RZ/V2M SoC) is a controller for external power supplies (regulators
> > > and power switches), and it supports the following features: it
> > > generates a power on/off sequence for external power supplies,
> > > it generates an on/off sequence for the LPDDR4 core power supply
> > > (LPVDD), it comes with General-Purpose Outputs, and it processes
> > > key input signals.
> > 
> > Thanks for your patch!
> > 
> > > The PWC is basically a Multi-Function Device (MFD), its software
> > > support comes with a core driver, and specialized drivers for
> > > its specific features.
> > 
> > I have to admit I'm not such a big fan of MFD.  In this driver,
> > you are not even sharing resources in the MFD cells, just the mapped
> > register base.  So I think you can easily save +100 LoC and reduce
> > maintenance synchronization overhead across subsystems by just having
> > a single non-MFD driver instead.
> > 
> > Did you pick MFD because the PWC poweroff feature depends on board
> > wiring, and thus is optional?
> 
> I am not a big fan of MFD, either.

Interesting.

Could you both elaborate further please?

> I picked MFD because we were not 100% sure of what the IP could do
> when we started working on it.
> I have received more information regarding the IP now (which I don't
> have the liberty to discuss), I am still not 100% sure that's all
> of it, but basically its support may require expansion later on.
> 
> I liked the solution based on syscon and simple-mfd for several reasons,
> but having dropped syscon and simple-mfd due to issues with the dt-bindings
> I have moved on with a core driver to instantiate the required SW support.
> We could of course move to a unified driver if that makes more sense?
> If we were to move to unified driver, under which directory would you
> suggest we put it?

If you do not have any resources to share, you can simply register each
of the devices via Device Tree.  I do not see a valid reason to force a
parent / child relationship for your use-case.

Many people attempt to use MFD as a dumping ground / workaround for a
bunch of reasons.  Some valid, others not so much.

-- 
Lee Jones [李琼斯]

RE: [PATCH v2 2/4] mfd: Add RZ/V2M PWC core driver

[Fabrizio Castro]

Hi Geert,

Thank you for your feedback!

> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 03 January 2023 12:10
> Subject: Re: [PATCH v2 2/4] mfd: Add RZ/V2M PWC core driver
> 
> Hi Fabrizio,
> 
> On Tue, Jan 3, 2023 at 1:05 PM Fabrizio Castro
> <fabrizio.castro.jz@renesas.com> wrote:
> > > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > > On Wed, Dec 21, 2022 at 10:09 PM Fabrizio Castro
> > > <fabrizio.castro.jz@renesas.com> wrote:
> > > > The External Power Sequence Controller (PWC) IP (found in the
> > > > RZ/V2M SoC) is a controller for external power supplies (regulators
> > > > and power switches), and it supports the following features: it
> > > > generates a power on/off sequence for external power supplies,
> > > > it generates an on/off sequence for the LPDDR4 core power supply
> > > > (LPVDD), it comes with General-Purpose Outputs, and it processes
> > > > key input signals.
> > >
> > > Thanks for your patch!
> > >
> > > > The PWC is basically a Multi-Function Device (MFD), its software
> > > > support comes with a core driver, and specialized drivers for
> > > > its specific features.
> > >
> > > I have to admit I'm not such a big fan of MFD.  In this driver,
> > > you are not even sharing resources in the MFD cells, just the mapped
> > > register base.  So I think you can easily save +100 LoC and reduce
> > > maintenance synchronization overhead across subsystems by just having
> > > a single non-MFD driver instead.
> > >
> > > Did you pick MFD because the PWC poweroff feature depends on board
> > > wiring, and thus is optional?
> >
> > I am not a big fan of MFD, either.
> > I picked MFD because we were not 100% sure of what the IP could do
> > when we started working on it.
> > I have received more information regarding the IP now (which I don't
> > have the liberty to discuss), I am still not 100% sure that's all
> > of it, but basically its support may require expansion later on.
> >
> > I liked the solution based on syscon and simple-mfd for several reasons,
> > but having dropped syscon and simple-mfd due to issues with the dt-
> bindings
> > I have moved on with a core driver to instantiate the required SW
> support.
> > We could of course move to a unified driver if that makes more sense?
> > If we were to move to unified driver, under which directory would you
> > suggest we put it?
> 
> As the GPIO part is larger than the poweroff part, I would put it under
> drivers/gpio/.  Although drivers/soc/renesas/ could be an option, too.

I'll try the unified approach then, under drivers/soc/renesas .

Thanks,
Fab 

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker.
> But
> when I'm talking to journalists I just say "programmer" or something like
> that.
>                                 -- Linus Torvalds

RE: [PATCH v2 2/4] mfd: Add RZ/V2M PWC core driver

[Fabrizio Castro]

Hi Lees,

Thanks for your feedback!

> From: Lee Jones <lee@kernel.org>
> Sent: 03 January 2023 12:52
> Subject: Re: [PATCH v2 2/4] mfd: Add RZ/V2M PWC core driver
> 
> On Tue, 03 Jan 2023, Fabrizio Castro wrote:
> 
> > Hi Geert,
> >
> > Thanks for your feedback!
> >
> > > -----Original Message-----
> > > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > > Sent: 03 January 2023 08:37
> > > To: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> > > Cc: Linus Walleij <linus.walleij@linaro.org>; Bartosz Golaszewski
> > > <brgl@bgdev.pl>; Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski
> > > <krzysztof.kozlowski+dt@linaro.org>; Sebastian Reichel
> <sre@kernel.org>;
> > > Geert Uytterhoeven <geert+renesas@glider.be>; Lee Jones
> <lee@kernel.org>;
> > > linux-gpio@vger.kernel.org; devicetree@vger.kernel.org; linux-
> > > kernel@vger.kernel.org; linux-pm@vger.kernel.org; Chris Paterson
> > > <Chris.Paterson2@renesas.com>; Biju Das <biju.das@bp.renesas.com>;
> linux-
> > > renesas-soc@vger.kernel.org; Laurent Pinchart
> > > <laurent.pinchart@ideasonboard.com>; Jacopo Mondi <jacopo@jmondi.org>
> > > Subject: Re: [PATCH v2 2/4] mfd: Add RZ/V2M PWC core driver
> > >
> > > Hi Fabrizio,
> > >
> > > On Wed, Dec 21, 2022 at 10:09 PM Fabrizio Castro
> > > <fabrizio.castro.jz@renesas.com> wrote:
> > > > The External Power Sequence Controller (PWC) IP (found in the
> > > > RZ/V2M SoC) is a controller for external power supplies (regulators
> > > > and power switches), and it supports the following features: it
> > > > generates a power on/off sequence for external power supplies,
> > > > it generates an on/off sequence for the LPDDR4 core power supply
> > > > (LPVDD), it comes with General-Purpose Outputs, and it processes
> > > > key input signals.
> > >
> > > Thanks for your patch!
> > >
> > > > The PWC is basically a Multi-Function Device (MFD), its software
> > > > support comes with a core driver, and specialized drivers for
> > > > its specific features.
> > >
> > > I have to admit I'm not such a big fan of MFD.  In this driver,
> > > you are not even sharing resources in the MFD cells, just the mapped
> > > register base.  So I think you can easily save +100 LoC and reduce
> > > maintenance synchronization overhead across subsystems by just having
> > > a single non-MFD driver instead.
> > >
> > > Did you pick MFD because the PWC poweroff feature depends on board
> > > wiring, and thus is optional?
> >
> > I am not a big fan of MFD, either.
> 
> Interesting.
> 
> Could you both elaborate further please?

I have nothing against MFD, it's just that, as I am finding out, it looks
like there is always a reason to not go down that road.
I have tried simple-mfd (which I think is a brilliant solution, especially
when combined with syscon), and it didn't fly. With this version I have tried
another approach based on MFD, and it's not flying.
I'll end up with a single driver supporting the various features of this
MFD device, which is fine, yet the software will have nothing to do with
MFD.

> 
> > I picked MFD because we were not 100% sure of what the IP could do
> > when we started working on it.
> > I have received more information regarding the IP now (which I don't
> > have the liberty to discuss), I am still not 100% sure that's all
> > of it, but basically its support may require expansion later on.
> >
> > I liked the solution based on syscon and simple-mfd for several reasons,
> > but having dropped syscon and simple-mfd due to issues with the dt-
> bindings
> > I have moved on with a core driver to instantiate the required SW
> support.
> > We could of course move to a unified driver if that makes more sense?
> > If we were to move to unified driver, under which directory would you
> > suggest we put it?
> 
> If you do not have any resources to share, you can simply register each
> of the devices via Device Tree.  I do not see a valid reason to force a
> parent / child relationship for your use-case.

There would probably be overlapping on the same memory region, which would
lead to ioremapping the same region multiple times, which is something
I would prefer to avoid if possible.

> 
> Many people attempt to use MFD as a dumping ground / workaround for a
> bunch of reasons.  Some valid, others not so much.

As it turns out, it looks like I don't have valid reasons to use MFD,
therefore I'll switch to a single, non MFD, driver.

Thank you for taking the time to look into this though! Really
appreciated.

Fab

> 
> --
> Lee Jones [李琼斯]

RE: [PATCH v2 1/4] dt-bindings: mfd: Add RZ/V2M PWC

[Fabrizio Castro]

Hi Geert,

Thanks for your feedback!

> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 03 January 2023 08:29
> Subject: Re: [PATCH v2 1/4] dt-bindings: mfd: Add RZ/V2M PWC
> 
> Hi Fabrizio,
> 
> On Wed, Dec 21, 2022 at 10:09 PM Fabrizio Castro
> <fabrizio.castro.jz@renesas.com> wrote:
> > The Renesas RZ/V2M External Power Sequence Controller (PWC)
> > IP is a multi-function device, and it's capable of:
> > * external power supply on/off sequence generation
> > * on/off signal generation for the LPDDR4 core power supply (LPVDD)
> > * key input signals processing
> > * general-purpose output pins
> >
> > Add the corresponding dt-bindings.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> > ---
> >
> > v1->v2: I have dropped syscon, simple-mfd, regmap, offset, and the child
> nodes.
> 
> Thanks for the update!
> 
> > +  renesas,rzv2m-pwc-power:
> > +    description: The PWC is used to control the system power supplies.
> > +    type: boolean
> 
> I'm wondering if there is some other way to represent this, e.g.
> using DT topology? Some regulator relation?

Not that I can think of. With respect to power, this IP only generates
control (enable) signals for external regulators, it does not supply
power.

Thanks,
Fab

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker.
> But
> when I'm talking to journalists I just say "programmer" or something like
> that.
>                                 -- Linus Torvalds

Re: [PATCH v2 2/4] mfd: Add RZ/V2M PWC core driver

[Lee Jones]

On Tue, 03 Jan 2023, Fabrizio Castro wrote:

> Hi Lees,
> 
> Thanks for your feedback!
> 
> > From: Lee Jones <lee@kernel.org>
> > Sent: 03 January 2023 12:52
> > Subject: Re: [PATCH v2 2/4] mfd: Add RZ/V2M PWC core driver
> > 
> > On Tue, 03 Jan 2023, Fabrizio Castro wrote:
> > 
> > > Hi Geert,
> > >
> > > Thanks for your feedback!
> > >
> > > > -----Original Message-----
> > > > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > > > Sent: 03 January 2023 08:37
> > > > To: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> > > > Cc: Linus Walleij <linus.walleij@linaro.org>; Bartosz Golaszewski
> > > > <brgl@bgdev.pl>; Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski
> > > > <krzysztof.kozlowski+dt@linaro.org>; Sebastian Reichel
> > <sre@kernel.org>;
> > > > Geert Uytterhoeven <geert+renesas@glider.be>; Lee Jones
> > <lee@kernel.org>;
> > > > linux-gpio@vger.kernel.org; devicetree@vger.kernel.org; linux-
> > > > kernel@vger.kernel.org; linux-pm@vger.kernel.org; Chris Paterson
> > > > <Chris.Paterson2@renesas.com>; Biju Das <biju.das@bp.renesas.com>;
> > linux-
> > > > renesas-soc@vger.kernel.org; Laurent Pinchart
> > > > <laurent.pinchart@ideasonboard.com>; Jacopo Mondi <jacopo@jmondi.org>
> > > > Subject: Re: [PATCH v2 2/4] mfd: Add RZ/V2M PWC core driver

Could you please tell your mailer to remove mail headers from the body
please.

> > > > > The External Power Sequence Controller (PWC) IP (found in the
> > > > > RZ/V2M SoC) is a controller for external power supplies (regulators
> > > > > and power switches), and it supports the following features: it
> > > > > generates a power on/off sequence for external power supplies,
> > > > > it generates an on/off sequence for the LPDDR4 core power supply
> > > > > (LPVDD), it comes with General-Purpose Outputs, and it processes
> > > > > key input signals.
> > > >
> > > > Thanks for your patch!
> > > >
> > > > > The PWC is basically a Multi-Function Device (MFD), its software
> > > > > support comes with a core driver, and specialized drivers for
> > > > > its specific features.
> > > >
> > > > I have to admit I'm not such a big fan of MFD.  In this driver,
> > > > you are not even sharing resources in the MFD cells, just the mapped
> > > > register base.  So I think you can easily save +100 LoC and reduce
> > > > maintenance synchronization overhead across subsystems by just having
> > > > a single non-MFD driver instead.
> > > >
> > > > Did you pick MFD because the PWC poweroff feature depends on board
> > > > wiring, and thus is optional?
> > >
> > > I am not a big fan of MFD, either.
> > 
> > Interesting.
> > 
> > Could you both elaborate further please?
> 
> I have nothing against MFD

Okay, just checking.  I'll withdraw my next command then. :)

$ rm -rf drivers/mfd

> > If you do not have any resources to share, you can simply register each
> > of the devices via Device Tree.  I do not see a valid reason to force a
> > parent / child relationship for your use-case.
> 
> There would probably be overlapping on the same memory region, which would
> lead to ioremapping the same region multiple times, which is something
> I would prefer to avoid if possible.

Okay, so you *do* have shared resources.

In which case, why is simple-mfd not working for you?

> > Many people attempt to use MFD as a dumping ground / workaround for a
> > bunch of reasons.  Some valid, others not so much.
> 
> As it turns out, it looks like I don't have valid reasons to use MFD,
> therefore I'll switch to a single, non MFD, driver.
> 
> Thank you for taking the time to look into this though! Really
> appreciated.

Although it is considered okay to have a multi-purpose driver in any one
of the subsystems, it's sometimes nicer to split the various
functionality to be looked after (maintained) by their respective
subject matter experts.  You have to do what's right in any given
situation.

Ultimately it's a call you need to make with the maintainer(s).

-- 
Lee Jones [李琼斯]

RE: [PATCH v2 2/4] mfd: Add RZ/V2M PWC core driver

[Fabrizio Castro]

Hi Lee,

Thanks for your reply.

> 
> On Tue, 03 Jan 2023, Fabrizio Castro wrote:
> 
> > Hi Lees,
> >
> > Thanks for your feedback!
> >
> Could you please tell your mailer to remove mail headers from the body
> please.

I wish my mailer could that automatically. I need to remember to manually
remove it, and sometimes I forget, sadly.

> > >
> > > Could you both elaborate further please?
> >
> > I have nothing against MFD
> 
> Okay, just checking.  I'll withdraw my next command then. :)
> 
> $ rm -rf drivers/mfd

:)

> 
> > > If you do not have any resources to share, you can simply register
> each
> > > of the devices via Device Tree.  I do not see a valid reason to force
> a
> > > parent / child relationship for your use-case.
> >
> > There would probably be overlapping on the same memory region, which
> would
> > lead to ioremapping the same region multiple times, which is something
> > I would prefer to avoid if possible.
> 
> Okay, so you *do* have shared resources.
> 
> In which case, why is simple-mfd not working for you?

The corresponding dt-bindings got rejected, unfortunately. I had to drop
simple-mfd as a result of dropping the children of my simple-mfd DT node.

> 
> > > Many people attempt to use MFD as a dumping ground / workaround for a
> > > bunch of reasons.  Some valid, others not so much.
> >
> > As it turns out, it looks like I don't have valid reasons to use MFD,
> > therefore I'll switch to a single, non MFD, driver.
> >
> > Thank you for taking the time to look into this though! Really
> > appreciated.
> 
> Although it is considered okay to have a multi-purpose driver in any one
> of the subsystems, it's sometimes nicer to split the various
> functionality to be looked after (maintained) by their respective
> subject matter experts.

I am 100% with you. That would be my preference, too.

> You have to do what's right in any given
> situation.
> 
> Ultimately it's a call you need to make with the maintainer(s).

Yeah, that's why the change of direction.

Thanks,
Fab

> 
> --
> Lee Jones [李琼斯]

Re: [PATCH v2 2/4] mfd: Add RZ/V2M PWC core driver

[Lee Jones]

> > > > If you do not have any resources to share, you can simply register
> > each
> > > > of the devices via Device Tree.  I do not see a valid reason to force
> > a
> > > > parent / child relationship for your use-case.
> > >
> > > There would probably be overlapping on the same memory region, which
> > would
> > > lead to ioremapping the same region multiple times, which is something
> > > I would prefer to avoid if possible.
> > 
> > Okay, so you *do* have shared resources.
> > 
> > In which case, why is simple-mfd not working for you?
> 
> The corresponding dt-bindings got rejected, unfortunately. I had to drop
> simple-mfd as a result of dropping the children of my simple-mfd DT node.

You have to write DT bindings to be OS agnostic.

They *must* match the H/W.  Little else matters.

How we interpret those in Linux is flexible however.

-- 
Lee Jones [李琼斯]

RE: [PATCH v2 4/4] power: reset: Add new driver for RZ/V2M PWC poweroff

[Fabrizio Castro]

Hi Geert,

Thanks for your feedback!

> 
> Hi Fabrizio,
> 
> On Wed, Dec 21, 2022 at 10:09 PM Fabrizio Castro
> <fabrizio.castro.jz@renesas.com> wrote:
> > The RZ/V2M PWC IP controls external power supplies and therefore
> > can turn the power supplies off when powering down the system.
> >
> > Add driver to poweroff the system.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> > ---
> >
> > v1->v2: Dropped OF match table and syscon as a result of the change in
> >         DT model
> 
> Thanks for your patch!
> 
> > --- /dev/null
> > +++ b/drivers/power/reset/rzv2m-pwc-poweroff.c
> > @@ -0,0 +1,67 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2022 Renesas Electronics Corporation
> > + *
> > + * Reset driver for Renesas RZ/V2M External Power Sequence Controller
> (PWC)
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/io.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/reboot.h>
> > +#include "../../mfd/rzv2m-pwc.h"
> > +
> > +#define PWC_PWCRST_RSTSOFTAX           0x1
> > +#define PWC_PWCCKEN_ENGCKMAIN          0x1
> > +#define PWC_PWCCTL_PWOFF               0x1
> > +
> > +struct rzv2m_pwc_poweroff_priv {
> > +       void __iomem *base;
> > +       struct device *dev;
> > +};
> > +
> > +static int rzv2m_pwc_poweroff(struct sys_off_data *data)
> > +{
> > +       struct rzv2m_pwc_poweroff_priv *priv =
> > +               (struct rzv2m_pwc_poweroff_priv *)data->cb_data;
> 
> No need for this cast.

Thanks for pointing this out. I'll fix that in v3.

Thanks,
Fab

> 
> > +
> > +       writel(PWC_PWCRST_RSTSOFTAX, priv->base + PWC_PWCRST);
> > +       writel(PWC_PWCCKEN_ENGCKMAIN, priv->base + PWC_PWCCKEN);
> > +       writel(PWC_PWCCTL_PWOFF, priv->base + PWC_PWCCTL);
> > +
> > +       mdelay(150);
> > +
> > +       dev_err(priv->dev, "Failed to power off the system");
> > +
> > +       return NOTIFY_DONE;
> > +}
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker.
> But
> when I'm talking to journalists I just say "programmer" or something like
> that.
>                                 -- Linus Torvalds