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 <[email protected]>
---
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 <[email protected]>");
+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
Hi Fabrizio,
On Wed, Dec 21, 2022 at 10:09 PM Fabrizio Castro
<[email protected]> 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 -- [email protected]
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
Hi Fabrizio,
On Tue, Jan 3, 2023 at 1:05 PM Fabrizio Castro
<[email protected]> wrote:
> > From: Geert Uytterhoeven <[email protected]>
> > On Wed, Dec 21, 2022 at 10:09 PM Fabrizio Castro
> > <[email protected]> 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 -- [email protected]
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
Hi Geert,
Thanks for your feedback!
> -----Original Message-----
> From: Geert Uytterhoeven <[email protected]>
> Sent: 03 January 2023 08:37
> To: Fabrizio Castro <[email protected]>
> Cc: Linus Walleij <[email protected]>; Bartosz Golaszewski
> <[email protected]>; Rob Herring <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; Sebastian Reichel <[email protected]>;
> Geert Uytterhoeven <[email protected]>; Lee Jones <[email protected]>;
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; Chris Paterson
> <[email protected]>; Biju Das <[email protected]>; linux-
> [email protected]; Laurent Pinchart
> <[email protected]>; Jacopo Mondi <[email protected]>
> 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
> <[email protected]> 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
On Tue, 03 Jan 2023, Fabrizio Castro wrote:
> Hi Geert,
>
> Thanks for your feedback!
>
> > -----Original Message-----
> > From: Geert Uytterhoeven <[email protected]>
> > Sent: 03 January 2023 08:37
> > To: Fabrizio Castro <[email protected]>
> > Cc: Linus Walleij <[email protected]>; Bartosz Golaszewski
> > <[email protected]>; Rob Herring <[email protected]>; Krzysztof Kozlowski
> > <[email protected]>; Sebastian Reichel <[email protected]>;
> > Geert Uytterhoeven <[email protected]>; Lee Jones <[email protected]>;
> > [email protected]; [email protected]; linux-
> > [email protected]; [email protected]; Chris Paterson
> > <[email protected]>; Biju Das <[email protected]>; linux-
> > [email protected]; Laurent Pinchart
> > <[email protected]>; Jacopo Mondi <[email protected]>
> > 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
> > <[email protected]> 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 [李琼斯]
Hi Geert,
Thank you for your feedback!
> From: Geert Uytterhoeven <[email protected]>
> 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
> <[email protected]> wrote:
> > > From: Geert Uytterhoeven <[email protected]>
> > > On Wed, Dec 21, 2022 at 10:09 PM Fabrizio Castro
> > > <[email protected]> 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
Hi Lees,
Thanks for your feedback!
> From: Lee Jones <[email protected]>
> 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 <[email protected]>
> > > Sent: 03 January 2023 08:37
> > > To: Fabrizio Castro <[email protected]>
> > > Cc: Linus Walleij <[email protected]>; Bartosz Golaszewski
> > > <[email protected]>; Rob Herring <[email protected]>; Krzysztof Kozlowski
> > > <[email protected]>; Sebastian Reichel
> <[email protected]>;
> > > Geert Uytterhoeven <[email protected]>; Lee Jones
> <[email protected]>;
> > > [email protected]; [email protected]; linux-
> > > [email protected]; [email protected]; Chris Paterson
> > > <[email protected]>; Biju Das <[email protected]>;
> linux-
> > > [email protected]; Laurent Pinchart
> > > <[email protected]>; Jacopo Mondi <[email protected]>
> > > 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
> > > <[email protected]> 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 [李琼斯]
On Tue, 03 Jan 2023, Fabrizio Castro wrote:
> Hi Lees,
>
> Thanks for your feedback!
>
> > From: Lee Jones <[email protected]>
> > 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 <[email protected]>
> > > > Sent: 03 January 2023 08:37
> > > > To: Fabrizio Castro <[email protected]>
> > > > Cc: Linus Walleij <[email protected]>; Bartosz Golaszewski
> > > > <[email protected]>; Rob Herring <[email protected]>; Krzysztof Kozlowski
> > > > <[email protected]>; Sebastian Reichel
> > <[email protected]>;
> > > > Geert Uytterhoeven <[email protected]>; Lee Jones
> > <[email protected]>;
> > > > [email protected]; [email protected]; linux-
> > > > [email protected]; [email protected]; Chris Paterson
> > > > <[email protected]>; Biju Das <[email protected]>;
> > linux-
> > > > [email protected]; Laurent Pinchart
> > > > <[email protected]>; Jacopo Mondi <[email protected]>
> > > > 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 [李琼斯]
> > > > 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 [李琼斯]
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 [李琼斯]