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
This series introduces a driver to address GPIO support, and a driver
to address power off support, alongside the corresponding dt-bindings.
Thanks,
Fab
Fabrizio Castro (5):
dt-bindings: gpio: Add RZ/V2M PWC GPIO driver bindings
dt-bindings: power: reset: Add RZ/V2M PWC Power OFF bindings
dt-bindings: mfd: Add RZ/V2M PWC global registers bindings
gpio: Add support for Renesas RZ/V2M PWC
power: reset: Add new driver for RZ/V2M PWC poweroff
.../bindings/gpio/renesas,rzv2m-pwc-gpio.yaml | 62 +++++++++
.../bindings/mfd/renesas,rzv2m-pwc.yaml | 70 ++++++++++
.../reset/renesas,rzv2m-pwc-poweroff.yaml | 48 +++++++
drivers/gpio/Kconfig | 8 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-rzv2m-pwc.c | 123 ++++++++++++++++++
drivers/power/reset/Kconfig | 10 ++
drivers/power/reset/Makefile | 1 +
drivers/power/reset/rzv2m-pwc-poweroff.c | 81 ++++++++++++
9 files changed, 404 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-gpio.yaml
create mode 100644 Documentation/devicetree/bindings/mfd/renesas,rzv2m-pwc.yaml
create mode 100644 Documentation/devicetree/bindings/power/reset/renesas,rzv2m-pwc-poweroff.yaml
create mode 100644 drivers/gpio/gpio-rzv2m-pwc.c
create mode 100644 drivers/power/reset/rzv2m-pwc-poweroff.c
--
2.34.1
The RZ/V2M PWC is a multi-function device, and its software
support relies on "syscon" and "simple-mfd".
Add the dt-bindings for the top level device tree node.
Signed-off-by: Fabrizio Castro <[email protected]>
---
.../bindings/mfd/renesas,rzv2m-pwc.yaml | 70 +++++++++++++++++++
1 file changed, 70 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..a7e180bfbd83
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/renesas,rzv2m-pwc.yaml
@@ -0,0 +1,70 @@
+# 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 <[email protected]>
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - renesas,r9a09g011-pwc # RZ/V2M
+ - renesas,r9a09g055-pwc # RZ/V2MA
+ - const: renesas,rzv2m-pwc
+ - const: syscon
+ - const: simple-mfd
+
+ reg:
+ maxItems: 1
+
+ gpio:
+ type: object
+ $ref: /schemas/gpio/renesas,rzv2m-pwc-gpio.yaml#
+ description: General-Purpose Output pins controller.
+
+ poweroff:
+ type: object
+ $ref: /schemas/power/reset/renesas,rzv2m-pwc-poweroff.yaml#
+ description: Power OFF controller.
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ pwc: pwc@a3700000 {
+ compatible = "renesas,r9a09g011-pwc", "renesas,rzv2m-pwc", "syscon",
+ "simple-mfd";
+ reg = <0xa3700000 0x800>;
+
+ gpio {
+ compatible = "renesas,r9a09g011-pwc-gpio",
+ "renesas,rzv2m-pwc-gpio";
+ regmap = <&pwc>;
+ offset = <0x80>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+
+ poweroff {
+ compatible = "renesas,r9a09g011-pwc-poweroff",
+ "renesas,rzv2m-pwc-poweroff";
+ regmap = <&pwc>;
+ };
+ };
--
2.34.1
The RZ/V2M PWC IP controls external power supplies and therefore
can turn the power supplies off when powering down the system.
PWC is essentially a Multi-Function Device (MFD), and this driver
relies on syscon and simple-mfd to integrate within the larger
scheme of things.
Signed-off-by: Fabrizio Castro <[email protected]>
---
drivers/power/reset/Kconfig | 10 +++
drivers/power/reset/Makefile | 1 +
drivers/power/reset/rzv2m-pwc-poweroff.c | 81 ++++++++++++++++++++++++
3 files changed, 92 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..9f7c9ed1a36e 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -303,4 +303,14 @@ 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"
+ depends on MFD_SYSCON
+ depends on ARCH_R9A09G011 || 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..e9bd16e65b6a
--- /dev/null
+++ b/drivers/power/reset/rzv2m-pwc-poweroff.c
@@ -0,0 +1,81 @@
+// 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/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reboot.h>
+#include <linux/regmap.h>
+
+#define PWC_PWCRST 0x00
+#define PWC_PWCCKEN 0x04
+#define PWC_PWCCTL 0x50
+
+#define PWC_PWCRST_RSTSOFTAX 0x1
+#define PWC_PWCCKEN_ENGCKMAIN 0x1
+#define PWC_PWCCTL_PWOFF 0x1
+
+struct rzv2m_pwc_poweroff_priv {
+ struct regmap *regmap;
+ 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;
+
+ regmap_write(priv->regmap, PWC_PWCRST, PWC_PWCRST_RSTSOFTAX);
+ regmap_write(priv->regmap, PWC_PWCCKEN, PWC_PWCCKEN_ENGCKMAIN);
+ regmap_write(priv->regmap, PWC_PWCCTL, PWC_PWCCTL_PWOFF);
+
+ 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_poweroff_priv *priv;
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+ "regmap");
+
+ if (IS_ERR(priv->regmap))
+ return dev_err_probe(&pdev->dev, PTR_ERR(priv->regmap),
+ "Can't find regmap property");
+
+ priv->dev = &pdev->dev;
+
+ return devm_register_power_off_handler(&pdev->dev, rzv2m_pwc_poweroff,
+ priv);
+}
+
+static const struct of_device_id rzv2m_pwc_poweroff_of_match[] = {
+ { .compatible = "renesas,rzv2m-pwc-poweroff" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, rzv2m_pwc_poweroff_of_match);
+
+static struct platform_driver rzv2m_pwc_poweroff_driver = {
+ .probe = rzv2m_pwc_poweroff_probe,
+ .driver = {
+ .name = "rzv2m_pwc_poweroff",
+ .of_match_table = of_match_ptr(rzv2m_pwc_poweroff_of_match),
+ },
+};
+module_platform_driver(rzv2m_pwc_poweroff_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Fabrizio Castro <[email protected]>");
+MODULE_DESCRIPTION("Renesas RZ/V2M PWC power OFF driver");
--
2.34.1
Add dt-bindings document for the RZ/V2M PWC GPIO driver.
Signed-off-by: Fabrizio Castro <[email protected]>
---
.../bindings/gpio/renesas,rzv2m-pwc-gpio.yaml | 62 +++++++++++++++++++
1 file changed, 62 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-gpio.yaml
diff --git a/Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-gpio.yaml b/Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-gpio.yaml
new file mode 100644
index 000000000000..ecc034d53259
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-gpio.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/renesas,rzv2m-pwc-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/V2M External Power Sequence Controller (PWC) GPIO
+
+description: |+
+ The PWC IP found in the RZ/V2M family of chips comes with General-Purpose
+ Output pins, alongside the below functions
+ - external power supply on/off sequence generation
+ - on/off signal generation for the LPDDR4 core power supply (LPVDD)
+ - key input signals processing
+ This node uses syscon to map the register used to control the GPIOs
+ (the register map is retrieved from the parent dt-node), and the node should
+ be represented as a sub node of a "syscon", "simple-mfd" node.
+
+maintainers:
+ - Fabrizio Castro <[email protected]>
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - renesas,r9a09g011-pwc-gpio # RZ/V2M
+ - renesas,r9a09g055-pwc-gpio # RZ/V2MA
+ - const: renesas,rzv2m-pwc-gpio
+
+ offset:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ Offset in the register map for controlling the GPIOs (in bytes).
+
+ regmap:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description: Phandle to the register map node.
+
+ gpio-controller: true
+
+ '#gpio-cells':
+ const: 2
+
+required:
+ - compatible
+ - regmap
+ - offset
+ - gpio-controller
+ - '#gpio-cells'
+
+additionalProperties: false
+
+examples:
+ - |
+ gpio {
+ compatible = "renesas,r9a09g011-pwc-gpio",
+ "renesas,rzv2m-pwc-gpio";
+ regmap = <®mapnode>;
+ offset = <0x80>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
--
2.34.1
On Tue, Dec 13, 2022 at 10:43:06PM +0000, Fabrizio Castro wrote:
> Add dt-bindings document for the RZ/V2M PWC GPIO driver.
Bindings are for h/w blocks/devices, not a specific driver.
>
> Signed-off-by: Fabrizio Castro <[email protected]>
> ---
> .../bindings/gpio/renesas,rzv2m-pwc-gpio.yaml | 62 +++++++++++++++++++
> 1 file changed, 62 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-gpio.yaml
>
> diff --git a/Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-gpio.yaml b/Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-gpio.yaml
> new file mode 100644
> index 000000000000..ecc034d53259
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-gpio.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/renesas,rzv2m-pwc-gpio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas RZ/V2M External Power Sequence Controller (PWC) GPIO
> +
> +description: |+
> + The PWC IP found in the RZ/V2M family of chips comes with General-Purpose
> + Output pins, alongside the below functions
> + - external power supply on/off sequence generation
> + - on/off signal generation for the LPDDR4 core power supply (LPVDD)
> + - key input signals processing
> + This node uses syscon to map the register used to control the GPIOs
> + (the register map is retrieved from the parent dt-node), and the node should
> + be represented as a sub node of a "syscon", "simple-mfd" node.
> +
> +maintainers:
> + - Fabrizio Castro <[email protected]>
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - renesas,r9a09g011-pwc-gpio # RZ/V2M
> + - renesas,r9a09g055-pwc-gpio # RZ/V2MA
> + - const: renesas,rzv2m-pwc-gpio
> +
> + offset:
Too generic of a name. We want any given property name (globally) to
have 1 type. With the below comment, this should be replaced with 'reg'
instead if you have child nodes.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + Offset in the register map for controlling the GPIOs (in bytes).
> +
> + regmap:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description: Phandle to the register map node.
Looks like GPIO is a sub-function of some other block. Define the
binding for that entire block. GPIO can be either either a function of
that node (just add GPIO provider properties) or you can have GPIO child
nodes. Depends on what the entire block looks like to decide. Do you
have multiple instances of the GPIO block would be one reason to have
child nodes.
> +
> + gpio-controller: true
> +
> + '#gpio-cells':
> + const: 2
> +
> +required:
> + - compatible
> + - regmap
> + - offset
> + - gpio-controller
> + - '#gpio-cells'
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + gpio {
> + compatible = "renesas,r9a09g011-pwc-gpio",
> + "renesas,rzv2m-pwc-gpio";
> + regmap = <®mapnode>;
> + offset = <0x80>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> --
> 2.34.1
>
>
On Tue, Dec 13, 2022 at 10:43:08PM +0000, Fabrizio Castro wrote:
> The RZ/V2M PWC is a multi-function device, and its software
> support relies on "syscon" and "simple-mfd".
> Add the dt-bindings for the top level device tree node.
>
> Signed-off-by: Fabrizio Castro <[email protected]>
> ---
> .../bindings/mfd/renesas,rzv2m-pwc.yaml | 70 +++++++++++++++++++
> 1 file changed, 70 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..a7e180bfbd83
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/renesas,rzv2m-pwc.yaml
> @@ -0,0 +1,70 @@
> +# 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 <[email protected]>
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - renesas,r9a09g011-pwc # RZ/V2M
> + - renesas,r9a09g055-pwc # RZ/V2MA
> + - const: renesas,rzv2m-pwc
> + - const: syscon
> + - const: simple-mfd
> +
> + reg:
> + maxItems: 1
> +
> + gpio:
> + type: object
> + $ref: /schemas/gpio/renesas,rzv2m-pwc-gpio.yaml#
> + description: General-Purpose Output pins controller.
> +
> + poweroff:
> + type: object
> + $ref: /schemas/power/reset/renesas,rzv2m-pwc-poweroff.yaml#
> + description: Power OFF controller.
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + pwc: pwc@a3700000 {
> + compatible = "renesas,r9a09g011-pwc", "renesas,rzv2m-pwc", "syscon",
> + "simple-mfd";
> + reg = <0xa3700000 0x800>;
> +
> + gpio {
> + compatible = "renesas,r9a09g011-pwc-gpio",
> + "renesas,rzv2m-pwc-gpio";
> + regmap = <&pwc>;
> + offset = <0x80>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> +
> + poweroff {
> + compatible = "renesas,r9a09g011-pwc-poweroff",
> + "renesas,rzv2m-pwc-poweroff";
> + regmap = <&pwc>;
Why does this need to be a child node? There aren't any resources for
it. 'regmap' is just the parent node.
Assuming this binding is complete, I don't think you need any child
nodes. A single node can have multiple providers.
Rob
Hi Rob,
Thanks for your feedback!
> From: Rob Herring <[email protected]>
> Sent: 14 December 2022 16:11
> To: Fabrizio Castro <[email protected]>
> Subject: Re: [PATCH 1/5] dt-bindings: gpio: Add RZ/V2M PWC GPIO driver
> bindings
>
> On Tue, Dec 13, 2022 at 10:43:06PM +0000, Fabrizio Castro wrote:
> > Add dt-bindings document for the RZ/V2M PWC GPIO driver.
>
> Bindings are for h/w blocks/devices, not a specific driver.
Apologies, I will reword the changelog in v2.
>
> >
> > Signed-off-by: Fabrizio Castro <[email protected]>
> > ---
> > .../bindings/gpio/renesas,rzv2m-pwc-gpio.yaml | 62 +++++++++++++++++++
> > 1 file changed, 62 insertions(+)
> > create mode 100644
> Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-gpio.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-
> gpio.yaml b/Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-
> gpio.yaml
> > new file mode 100644
> > index 000000000000..ecc034d53259
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-gpio.yaml
> > @@ -0,0 +1,62 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id:
> https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetre
> e.org%2Fschemas%2Fgpio%2Frenesas%2Crzv2m-pwc-
> gpio.yaml%23&data=05%7C01%7Cfabrizio.castro.jz%40renesas.com%7C603623c
> 766f4421b85bd08daddedcb8c%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C638
> 066310628408926%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMz
> IiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=o46ncDZK8YK5HYJ
> ZYDXuq3yfEA34vnaxEsIDBlcroc0%3D&reserved=0
> > +$schema:
> https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetre
> e.org%2Fmeta-
> schemas%2Fcore.yaml%23&data=05%7C01%7Cfabrizio.castro.jz%40renesas.com
> %7C603623c766f4421b85bd08daddedcb8c%7C53d82571da1947e49cb4625a166a4a2a%7C0
> %7C0%7C638066310628408926%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQ
> IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=VoWvV
> pW782DVH2zdTKIesyzqm6sjiFyacbl833%2BjRis%3D&reserved=0
> > +
> > +title: Renesas RZ/V2M External Power Sequence Controller (PWC) GPIO
> > +
> > +description: |+
> > + The PWC IP found in the RZ/V2M family of chips comes with General-
> Purpose
> > + Output pins, alongside the below functions
> > + - external power supply on/off sequence generation
> > + - on/off signal generation for the LPDDR4 core power supply (LPVDD)
> > + - key input signals processing
> > + This node uses syscon to map the register used to control the GPIOs
> > + (the register map is retrieved from the parent dt-node), and the node
> should
> > + be represented as a sub node of a "syscon", "simple-mfd" node.
> > +
> > +maintainers:
> > + - Fabrizio Castro <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - enum:
> > + - renesas,r9a09g011-pwc-gpio # RZ/V2M
> > + - renesas,r9a09g055-pwc-gpio # RZ/V2MA
> > + - const: renesas,rzv2m-pwc-gpio
> > +
> > + offset:
>
> Too generic of a name. We want any given property name (globally) to
> have 1 type. With the below comment, this should be replaced with 'reg'
> instead if you have child nodes.
My understanding is that syscon subnodes normally use this name for exactly
the same purpose, for example:
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/power/reset/syscon-poweroff.yaml#L27
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml#L30
What am I missing?
>
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: |
> > + Offset in the register map for controlling the GPIOs (in bytes).
> > +
> > + regmap:
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + description: Phandle to the register map node.
>
> Looks like GPIO is a sub-function of some other block. Define the
> binding for that entire block.
That's defined in patch 3 from this series.
I have sent it as patch 3 because that document references:
* /schemas/gpio/renesas,rzv2m-pwc-gpio.yaml
* /schemas/power/reset/renesas,rzv2m-pwc-poweroff.yaml
Which are defined in this patch and in patch 2 in the series.
Do you want me to move patch 3 to patch 1 in v2?
> GPIO can be either either a function of
> that node (just add GPIO provider properties) or you can have GPIO child
> nodes. Depends on what the entire block looks like to decide. Do you
> have multiple instances of the GPIO block would be one reason to have
> child nodes.
From a pure HW point of view, this GPIO block is contained inside the PWC block
(as PWC is basically a MFD device), and it only deals with 2 General-Purpose
Output pins, both controlled by 1 (and the same) register, therefore the GPIO
block is only 1 child.
If possible, I would like to keep the functionality offered by the sub-blocks of
PWC contained in separated drivers and DT nodes (either non-child nodes or child
nodes).
My understanding is that simple-mfd will automatically probe the children of the
simple-mfd node, and also hierarchically it makes sense to me to have the DT nodes
of the PWC sub-blocks as children of the "syscon", "simple-mfd" node. I have found
other instances of this same architecture in the kernel already (plenty from NXP/Freescale),
for example:
https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/freescale/imx8mm.dtsi#L585
https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/freescale/imx8mn.dtsi#L586
https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/freescale/imx8mp.dtsi#L451
https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/freescale/imx8mq.dtsi#L616
https://github.com/torvalds/linux/blob/master/arch/mips/boot/dts/mti/sead3.dts#L93
etc...
Something like the below could also work, but I don't think it would represent the
HW accurately:
pwc: pwc@a3700000 {
compatible = "renesas,r9a09g011-pwc", "renesas,rzv2m-pwc",
"syscon", "simple-mfd";
reg = <0 0xa3700000 0 0x800>;
};
pwc-gpio {
compatible = "renesas,r9a09g011-pwc-gpio",
"renesas,rzv2m-pwc-gpio";
regmap = <&pwc>;
gpio-controller;
#gpio-cells = <2>;
};
pwc-poweroff {
compatible = "renesas,r9a09g011-pwc-poweroff",
"renesas,rzv2m-pwc-poweroff";
regmap = <&pwc>;
};
I think the below describes things better:
pwc: pwc@a3700000 {
compatible = "renesas,r9a09g011-pwc", "renesas,rzv2m-pwc",
"syscon", "simple-mfd";
reg = <0 0xa3700000 0 0x800>;
gpio {
compatible = "renesas,r9a09g011-pwc-gpio",
"renesas,rzv2m-pwc-gpio";
regmap = <&pwc>;
offset = <0x80>;
gpio-controller;
#gpio-cells = <2>;
};
poweroff {
compatible = "renesas,r9a09g011-pwc-poweroff",
"renesas,rzv2m-pwc-poweroff";
regmap = <&pwc>;
};
};
Do you think the bindings I have sent out are causing confusion here?
What else can I improve?
Thanks,
Fab
>
> > +
> > + gpio-controller: true
> > +
> > + '#gpio-cells':
> > + const: 2
> > +
> > +required:
> > + - compatible
> > + - regmap
> > + - offset
> > + - gpio-controller
> > + - '#gpio-cells'
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + gpio {
> > + compatible = "renesas,r9a09g011-pwc-gpio",
> > + "renesas,rzv2m-pwc-gpio";
> > + regmap = <®mapnode>;
> > + offset = <0x80>;
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + };
> > --
> > 2.34.1
> >
> >
On 14/12/2022 19:26, Fabrizio Castro wrote:
> Hi Rob,
>
> Thanks for your feedback!
>
>> From: Rob Herring <[email protected]>
>> Sent: 14 December 2022 16:11
>> To: Fabrizio Castro <[email protected]>
>> Subject: Re: [PATCH 1/5] dt-bindings: gpio: Add RZ/V2M PWC GPIO driver
>> bindings
>>
>> On Tue, Dec 13, 2022 at 10:43:06PM +0000, Fabrizio Castro wrote:
>>> Add dt-bindings document for the RZ/V2M PWC GPIO driver.
>>
>> Bindings are for h/w blocks/devices, not a specific driver.
>
> Apologies, I will reword the changelog in v2.
>
>>
>>>
>>> Signed-off-by: Fabrizio Castro <[email protected]>
>>> ---
>>> .../bindings/gpio/renesas,rzv2m-pwc-gpio.yaml | 62 +++++++++++++++++++
>>> 1 file changed, 62 insertions(+)
>>> create mode 100644
>> Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-gpio.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-
>> gpio.yaml b/Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-
>> gpio.yaml
>>> new file mode 100644
>>> index 000000000000..ecc034d53259
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-gpio.yaml
>>> @@ -0,0 +1,62 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id:
>> https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetre
>> e.org%2Fschemas%2Fgpio%2Frenesas%2Crzv2m-pwc-
>> gpio.yaml%23&data=05%7C01%7Cfabrizio.castro.jz%40renesas.com%7C603623c
>> 766f4421b85bd08daddedcb8c%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C638
>> 066310628408926%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMz
>> IiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=o46ncDZK8YK5HYJ
>> ZYDXuq3yfEA34vnaxEsIDBlcroc0%3D&reserved=0
>>> +$schema:
>> https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetre
>> e.org%2Fmeta-
>> schemas%2Fcore.yaml%23&data=05%7C01%7Cfabrizio.castro.jz%40renesas.com
>> %7C603623c766f4421b85bd08daddedcb8c%7C53d82571da1947e49cb4625a166a4a2a%7C0
>> %7C0%7C638066310628408926%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQ
>> IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=VoWvV
>> pW782DVH2zdTKIesyzqm6sjiFyacbl833%2BjRis%3D&reserved=0
>>> +
>>> +title: Renesas RZ/V2M External Power Sequence Controller (PWC) GPIO
>>> +
>>> +description: |+
>>> + The PWC IP found in the RZ/V2M family of chips comes with General-
>> Purpose
>>> + Output pins, alongside the below functions
>>> + - external power supply on/off sequence generation
>>> + - on/off signal generation for the LPDDR4 core power supply (LPVDD)
>>> + - key input signals processing
>>> + This node uses syscon to map the register used to control the GPIOs
>>> + (the register map is retrieved from the parent dt-node), and the node
>> should
>>> + be represented as a sub node of a "syscon", "simple-mfd" node.
>>> +
>>> +maintainers:
>>> + - Fabrizio Castro <[email protected]>
>>> +
>>> +properties:
>>> + compatible:
>>> + items:
>>> + - enum:
>>> + - renesas,r9a09g011-pwc-gpio # RZ/V2M
>>> + - renesas,r9a09g055-pwc-gpio # RZ/V2MA
>>> + - const: renesas,rzv2m-pwc-gpio
>>> +
>>> + offset:
>>
>> Too generic of a name. We want any given property name (globally) to
>> have 1 type. With the below comment, this should be replaced with 'reg'
>> instead if you have child nodes.
>
> My understanding is that syscon subnodes normally use this name for exactly
> the same purpose, for example:
> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/power/reset/syscon-poweroff.yaml#L27
> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml#L30
>
> What am I missing?
These are generic drivers, so they need offset as they do not match any
specific programming model. You are not making a generic device. Address
offsets are not suitable in most cases for DTS. There are of course
exceptions so you can present reasons why this one is exception.
>
>>
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + description: |
>>> + Offset in the register map for controlling the GPIOs (in bytes).
>>> +
>>> + regmap:
>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>> + description: Phandle to the register map node.
>>
>> Looks like GPIO is a sub-function of some other block. Define the
>> binding for that entire block.
>
> That's defined in patch 3 from this series.
> I have sent it as patch 3 because that document references:
> * /schemas/gpio/renesas,rzv2m-pwc-gpio.yaml
> * /schemas/power/reset/renesas,rzv2m-pwc-poweroff.yaml
> Which are defined in this patch and in patch 2 in the series.
>
> Do you want me to move patch 3 to patch 1 in v2?
We do not want regmap, but proper definition of entire hardware.
>
>> GPIO can be either either a function of
>> that node (just add GPIO provider properties) or you can have GPIO child
>> nodes. Depends on what the entire block looks like to decide. Do you
>> have multiple instances of the GPIO block would be one reason to have
>> child nodes.
>
> From a pure HW point of view, this GPIO block is contained inside the PWC block
> (as PWC is basically a MFD device), and it only deals with 2 General-Purpose
> Output pins, both controlled by 1 (and the same) register, therefore the GPIO
> block is only 1 child.
>
> If possible, I would like to keep the functionality offered by the sub-blocks of
> PWC contained in separated drivers and DT nodes (either non-child nodes or child
> nodes).
Driver do not matter for bindings. We talk about regmap field which - as
you explained above - is not needed.
>
> My understanding is that simple-mfd will automatically probe the children of the
> simple-mfd node, and also hierarchically it makes sense to me to have the DT nodes
> of the PWC sub-blocks as children of the "syscon", "simple-mfd" node. I have found
> other instances of this same architecture in the kernel already (plenty from NXP/Freescale),
> for example:
I don't understand. You do not have here simple-mfd and it still does
not explain Rob's comment and regmap.
> https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/freescale/imx8mm.dtsi#L585
> https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/freescale/imx8mn.dtsi#L586
> https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/freescale/imx8mp.dtsi#L451
> https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/freescale/imx8mq.dtsi#L616
> https://github.com/torvalds/linux/blob/master/arch/mips/boot/dts/mti/sead3.dts#L93
> etc...
>
> Something like the below could also work, but I don't think it would represent the
> HW accurately:
> pwc: pwc@a3700000 {
> compatible = "renesas,r9a09g011-pwc", "renesas,rzv2m-pwc",
> "syscon", "simple-mfd";
> reg = <0 0xa3700000 0 0x800>;
> };
>
> pwc-gpio {
> compatible = "renesas,r9a09g011-pwc-gpio",
> "renesas,rzv2m-pwc-gpio";
> regmap = <&pwc>;
> gpio-controller;
> #gpio-cells = <2>;
> };
>
> pwc-poweroff {
> compatible = "renesas,r9a09g011-pwc-poweroff",
> "renesas,rzv2m-pwc-poweroff";
> regmap = <&pwc>;
> };
>
>
> I think the below describes things better:
> pwc: pwc@a3700000 {
> compatible = "renesas,r9a09g011-pwc", "renesas,rzv2m-pwc",
> "syscon", "simple-mfd";
> reg = <0 0xa3700000 0 0x800>;
>
> gpio {
> compatible = "renesas,r9a09g011-pwc-gpio",
> "renesas,rzv2m-pwc-gpio";
> regmap = <&pwc>;
You speak about two different things. So again - drop regmap. You do not
need it.
> offset = <0x80>;
> gpio-controller;
> #gpio-cells = <2>;
> };
>
> poweroff {
> compatible = "renesas,r9a09g011-pwc-poweroff",
> "renesas,rzv2m-pwc-poweroff";
> regmap = <&pwc>;
Drop regmap.
> };
> };
>
Best regards,
Krzysztof
Hi Rob,
Thanks for the feeback.
> From: Rob Herring <[email protected]>
> Sent: 14 December 2022 16:16
> To: Fabrizio Castro <[email protected]>
> Subject: Re: [PATCH 3/5] dt-bindings: mfd: Add RZ/V2M PWC global registers
> bindings
>
> On Tue, Dec 13, 2022 at 10:43:08PM +0000, Fabrizio Castro wrote:
> > The RZ/V2M PWC is a multi-function device, and its software
> > support relies on "syscon" and "simple-mfd".
> > Add the dt-bindings for the top level device tree node.
> >
> > Signed-off-by: Fabrizio Castro <[email protected]>
> > ---
> > .../bindings/mfd/renesas,rzv2m-pwc.yaml | 70 +++++++++++++++++++
> > 1 file changed, 70 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..a7e180bfbd83
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/renesas,rzv2m-pwc.yaml
> > @@ -0,0 +1,70 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +
> > +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 <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - enum:
> > + - renesas,r9a09g011-pwc # RZ/V2M
> > + - renesas,r9a09g055-pwc # RZ/V2MA
> > + - const: renesas,rzv2m-pwc
> > + - const: syscon
> > + - const: simple-mfd
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + gpio:
> > + type: object
> > + $ref: /schemas/gpio/renesas,rzv2m-pwc-gpio.yaml#
> > + description: General-Purpose Output pins controller.
> > +
> > + poweroff:
> > + type: object
> > + $ref: /schemas/power/reset/renesas,rzv2m-pwc-poweroff.yaml#
> > + description: Power OFF controller.
> > +
> > +required:
> > + - compatible
> > + - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + pwc: pwc@a3700000 {
> > + compatible = "renesas,r9a09g011-pwc", "renesas,rzv2m-pwc",
> "syscon",
> > + "simple-mfd";
> > + reg = <0xa3700000 0x800>;
> > +
> > + gpio {
> > + compatible = "renesas,r9a09g011-pwc-gpio",
> > + "renesas,rzv2m-pwc-gpio";
> > + regmap = <&pwc>;
> > + offset = <0x80>;
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + };
> > +
> > + poweroff {
> > + compatible = "renesas,r9a09g011-pwc-poweroff",
> > + "renesas,rzv2m-pwc-poweroff";
> > + regmap = <&pwc>;
>
> Why does this need to be a child node? There aren't any resources for
> it. 'regmap' is just the parent node.
>
> Assuming this binding is complete, I don't think you need any child
> nodes. A single node can have multiple providers.
Alright, then I'll just put everything the device needs into a single
node. I'll send v2 based on the below snippet:
pwc@a3700000 {
compatible = "renesas,r9a09g011-pwc", "renesas,rzv2m-pwc";
reg = <0xa3700000 0x800>;
gpio-controller;
#gpio-cells = <2>;
renesas,rzv2m-pwc-power;
};
Thanks,
Fab
>
> Rob
Hi Rob,
Thanks for the feedback.
> From: Rob Herring <[email protected]>
> Sent: 14 December 2022 16:11
> To: Fabrizio Castro <[email protected]>
> Subject: Re: [PATCH 1/5] dt-bindings: gpio: Add RZ/V2M PWC GPIO driver
> bindings
>
> On Tue, Dec 13, 2022 at 10:43:06PM +0000, Fabrizio Castro wrote:
> > Add dt-bindings document for the RZ/V2M PWC GPIO driver.
>
> Bindings are for h/w blocks/devices, not a specific driver.
Right, I'll be more careful next time.
>
> >
> > Signed-off-by: Fabrizio Castro <[email protected]>
> > ---
> > .../bindings/gpio/renesas,rzv2m-pwc-gpio.yaml | 62 +++++++++++++++++++
> > 1 file changed, 62 insertions(+)
> > create mode 100644
> Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-gpio.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-
> gpio.yaml b/Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-
> gpio.yaml
> > new file mode 100644
> > index 000000000000..ecc034d53259
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-gpio.yaml
> > @@ -0,0 +1,62 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +
> > +title: Renesas RZ/V2M External Power Sequence Controller (PWC) GPIO
> > +
> > +description: |+
> > + The PWC IP found in the RZ/V2M family of chips comes with General-
> Purpose
> > + Output pins, alongside the below functions
> > + - external power supply on/off sequence generation
> > + - on/off signal generation for the LPDDR4 core power supply (LPVDD)
> > + - key input signals processing
> > + This node uses syscon to map the register used to control the GPIOs
> > + (the register map is retrieved from the parent dt-node), and the node
> should
> > + be represented as a sub node of a "syscon", "simple-mfd" node.
> > +
> > +maintainers:
> > + - Fabrizio Castro <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - enum:
> > + - renesas,r9a09g011-pwc-gpio # RZ/V2M
> > + - renesas,r9a09g055-pwc-gpio # RZ/V2MA
> > + - const: renesas,rzv2m-pwc-gpio
> > +
> > + offset:
>
> Too generic of a name. We want any given property name (globally) to
> have 1 type. With the below comment, this should be replaced with 'reg'
> instead if you have child nodes.
I'll take it out
>
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: |
> > + Offset in the register map for controlling the GPIOs (in bytes).
> > +
> > + regmap:
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + description: Phandle to the register map node.
>
> Looks like GPIO is a sub-function of some other block. Define the
> binding for that entire block. GPIO can be either either a function of
> that node (just add GPIO provider properties) or you can have GPIO child
> nodes. Depends on what the entire block looks like to decide. Do you
> have multiple instances of the GPIO block would be one reason to have
> child nodes.
I'll take out this child node.
Thanks,
Fab
>
> > +
> > + gpio-controller: true
> > +
> > + '#gpio-cells':
> > + const: 2
> > +
> > +required:
> > + - compatible
> > + - regmap
> > + - offset
> > + - gpio-controller
> > + - '#gpio-cells'
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + gpio {
> > + compatible = "renesas,r9a09g011-pwc-gpio",
> > + "renesas,rzv2m-pwc-gpio";
> > + regmap = <®mapnode>;
> > + offset = <0x80>;
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + };
> > --
> > 2.34.1
> >
> >
Hi Krzysztof,
Thanks for your feedback.
> From: Krzysztof Kozlowski <[email protected]>
> Sent: 15 December 2022 09:49
> To: Fabrizio Castro <[email protected]>; Rob Herring
> <[email protected]>
> Subject: Re: [PATCH 1/5] dt-bindings: gpio: Add RZ/V2M PWC GPIO driver
> bindings
>
> On 14/12/2022 19:26, Fabrizio Castro wrote:
> > Hi Rob,
> >
> > Thanks for your feedback!
> >
> >> From: Rob Herring <[email protected]>
> >> Sent: 14 December 2022 16:11
> >> To: Fabrizio Castro <[email protected]>
> >> Subject: Re: [PATCH 1/5] dt-bindings: gpio: Add RZ/V2M PWC GPIO driver
> >> bindings
> >>
> >> On Tue, Dec 13, 2022 at 10:43:06PM +0000, Fabrizio Castro wrote:
> >>> Add dt-bindings document for the RZ/V2M PWC GPIO driver.
> >>
> >> Bindings are for h/w blocks/devices, not a specific driver.
> >
> > Apologies, I will reword the changelog in v2.
> >
> >>
> >>>
> >>> Signed-off-by: Fabrizio Castro <[email protected]>
> >>> ---
> >>> .../bindings/gpio/renesas,rzv2m-pwc-gpio.yaml | 62
> +++++++++++++++++++
> >>> 1 file changed, 62 insertions(+)
> >>> create mode 100644
> >> Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-gpio.yaml
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-
> >> gpio.yaml b/Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-
> >> gpio.yaml
> >>> new file mode 100644
> >>> index 000000000000..ecc034d53259
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/gpio/renesas,rzv2m-pwc-
> gpio.yaml
> >>> @@ -0,0 +1,62 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +
> >>> +title: Renesas RZ/V2M External Power Sequence Controller (PWC) GPIO
> >>> +
> >>> +description: |+
> >>> + The PWC IP found in the RZ/V2M family of chips comes with General-
> >> Purpose
> >>> + Output pins, alongside the below functions
> >>> + - external power supply on/off sequence generation
> >>> + - on/off signal generation for the LPDDR4 core power supply
> (LPVDD)
> >>> + - key input signals processing
> >>> + This node uses syscon to map the register used to control the GPIOs
> >>> + (the register map is retrieved from the parent dt-node), and the
> node
> >> should
> >>> + be represented as a sub node of a "syscon", "simple-mfd" node.
> >>> +
> >>> +maintainers:
> >>> + - Fabrizio Castro <[email protected]>
> >>> +
> >>> +properties:
> >>> + compatible:
> >>> + items:
> >>> + - enum:
> >>> + - renesas,r9a09g011-pwc-gpio # RZ/V2M
> >>> + - renesas,r9a09g055-pwc-gpio # RZ/V2MA
> >>> + - const: renesas,rzv2m-pwc-gpio
> >>> +
> >>> + offset:
> >>
> >> Too generic of a name. We want any given property name (globally) to
> >> have 1 type. With the below comment, this should be replaced with 'reg'
> >> instead if you have child nodes.
> >
> > My understanding is that syscon subnodes normally use this name for
> exactly
> > the same purpose, for example:
> >
> >
> > What am I missing?
>
> These are generic drivers, so they need offset as they do not match any
> specific programming model. You are not making a generic device. Address
> offsets are not suitable in most cases for DTS. There are of course
> exceptions so you can present reasons why this one is exception.
Thanks for the explanation
> >
> >>
> >>> + $ref: /schemas/types.yaml#/definitions/uint32
> >>> + description: |
> >>> + Offset in the register map for controlling the GPIOs (in
> bytes).
> >>> +
> >>> + regmap:
> >>> + $ref: /schemas/types.yaml#/definitions/phandle
> >>> + description: Phandle to the register map node.
> >>
> >> Looks like GPIO is a sub-function of some other block. Define the
> >> binding for that entire block.
> >
> > That's defined in patch 3 from this series.
> > I have sent it as patch 3 because that document references:
> > * /schemas/gpio/renesas,rzv2m-pwc-gpio.yaml
> > * /schemas/power/reset/renesas,rzv2m-pwc-poweroff.yaml
> > Which are defined in this patch and in patch 2 in the series.
> >
> > Do you want me to move patch 3 to patch 1 in v2?
>
> We do not want regmap, but proper definition of entire hardware.
Will do. I'll drop regmap.
>
> >
> >> GPIO can be either either a function of
> >> that node (just add GPIO provider properties) or you can have GPIO
> child
> >> nodes. Depends on what the entire block looks like to decide. Do you
> >> have multiple instances of the GPIO block would be one reason to have
> >> child nodes.
> >
> > From a pure HW point of view, this GPIO block is contained inside the
> PWC block
> > (as PWC is basically a MFD device), and it only deals with 2 General-
> Purpose
> > Output pins, both controlled by 1 (and the same) register, therefore the
> GPIO
> > block is only 1 child.
> >
> > If possible, I would like to keep the functionality offered by the sub-
> blocks of
> > PWC contained in separated drivers and DT nodes (either non-child nodes
> or child
> > nodes).
>
> Driver do not matter for bindings. We talk about regmap field which - as
> you explained above - is not needed.
Okay, I'll rework, and I'll send v2.
Thanks,
Fab
>
>
> >
> > My understanding is that simple-mfd will automatically probe the
> children of the
> > simple-mfd node, and also hierarchically it makes sense to me to have
> the DT nodes
> > of the PWC sub-blocks as children of the "syscon", "simple-mfd" node. I
> have found
> > other instances of this same architecture in the kernel already (plenty
> from NXP/Freescale),
> > for example:
>
> I don't understand. You do not have here simple-mfd and it still does
> not explain Rob's comment and regmap.
>
> >
> > etc...
> >
> > Something like the below could also work, but I don't think it would
> represent the
> > HW accurately:
> > pwc: pwc@a3700000 {
> > compatible = "renesas,r9a09g011-pwc", "renesas,rzv2m-pwc",
> > "syscon", "simple-mfd";
> > reg = <0 0xa3700000 0 0x800>;
> > };
> >
> > pwc-gpio {
> > compatible = "renesas,r9a09g011-pwc-gpio",
> > "renesas,rzv2m-pwc-gpio";
> > regmap = <&pwc>;
> > gpio-controller;
> > #gpio-cells = <2>;
> > };
> >
> > pwc-poweroff {
> > compatible = "renesas,r9a09g011-pwc-poweroff",
> > "renesas,rzv2m-pwc-poweroff";
> > regmap = <&pwc>;
> > };
> >
> >
> > I think the below describes things better:
> > pwc: pwc@a3700000 {
> > compatible = "renesas,r9a09g011-pwc", "renesas,rzv2m-pwc",
> > "syscon", "simple-mfd";
> > reg = <0 0xa3700000 0 0x800>;
> >
> > gpio {
> > compatible = "renesas,r9a09g011-pwc-gpio",
> > "renesas,rzv2m-pwc-gpio";
> > regmap = <&pwc>;
>
> You speak about two different things. So again - drop regmap. You do not
> need it.
>
> > offset = <0x80>;
> > gpio-controller;
> > #gpio-cells = <2>;
> > };
> >
> > poweroff {
> > compatible = "renesas,r9a09g011-pwc-poweroff",
> > "renesas,rzv2m-pwc-poweroff";
> > regmap = <&pwc>;
>
> Drop regmap.
>
> > };
> > };
> >
>
> Best regards,
> Krzysztof