[PATCH v2] mfd: syscon: allow to register syscon with a device

[PATCH v2] mfd: syscon: allow to register syscon with a device

[Philipp Zabel]

Commit bdb0066df96e ("mfd: syscon: Decouple syscon interface from platform
devices") added the possibility to register syscon devices without
associated platform device. This also removed regmap debugfs facilities,
which don't work without a device. Since there is no replacement, this
patch allows again to register syscon regions with an associated device
where that this device exists anyway.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
Changes since v1:
 - Rebased onto for-mfd-next
---
 drivers/mfd/syscon.c       | 10 ++++++++--
 include/linux/mfd/syscon.h | 10 ++++++++++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
index 2f2225e..fe67fb0 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -42,7 +42,7 @@ static const struct regmap_config syscon_regmap_config = {
 	.reg_stride = 4,
 };
 
-static struct syscon *of_syscon_register(struct device_node *np)
+struct syscon *syscon_register(struct device *dev, struct device_node *np)
 {
 	struct syscon *syscon;
 	struct regmap *regmap;
@@ -89,7 +89,7 @@ static struct syscon *of_syscon_register(struct device_node *np)
 	syscon_config.val_bits = reg_io_width * 8;
 	syscon_config.max_register = resource_size(&res) - reg_io_width;
 
-	regmap = regmap_init_mmio(NULL, base, &syscon_config);
+	regmap = regmap_init_mmio(dev, base, &syscon_config);
 	if (IS_ERR(regmap)) {
 		pr_err("regmap init failed\n");
 		ret = PTR_ERR(regmap);
@@ -111,6 +111,12 @@ err_map:
 	kfree(syscon);
 	return ERR_PTR(ret);
 }
+EXPORT_SYMBOL_GPL(syscon_register);
+
+static struct syscon *of_syscon_register(struct device_node *np)
+{
+	return syscon_register(NULL, np);
+}
 
 struct regmap *syscon_node_to_regmap(struct device_node *np)
 {
diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
index 1088149..e26037c 100644
--- a/include/linux/mfd/syscon.h
+++ b/include/linux/mfd/syscon.h
@@ -17,10 +17,14 @@
 
 #include <linux/err.h>
 
+struct device;
 struct device_node;
+struct syscon;
 
 #ifdef CONFIG_MFD_SYSCON
 extern struct regmap *syscon_node_to_regmap(struct device_node *np);
+extern struct syscon *syscon_register(struct device *dev,
+				      struct device_node *np);
 extern struct regmap *syscon_regmap_lookup_by_compatible(const char *s);
 extern struct regmap *syscon_regmap_lookup_by_pdevname(const char *s);
 extern struct regmap *syscon_regmap_lookup_by_phandle(
@@ -32,6 +36,12 @@ static inline struct regmap *syscon_node_to_regmap(struct device_node *np)
 	return ERR_PTR(-ENOTSUPP);
 }
 
+static struct syscon *syscon_register(struct device *dev,
+				      struct device_node *np)
+{
+	return ERR_PTR(-ENOTSUPP);
+}
+
 static inline struct regmap *syscon_regmap_lookup_by_compatible(const char *s)
 {
 	return ERR_PTR(-ENOTSUPP);
-- 
2.7.0

Re: [PATCH v2] mfd: syscon: allow to register syscon with a device

[Arnd Bergmann]

On Wednesday 24 February 2016 12:19:01 Philipp Zabel wrote:
> Commit bdb0066df96e ("mfd: syscon: Decouple syscon interface from platform
> devices") added the possibility to register syscon devices without
> associated platform device. This also removed regmap debugfs facilities,
> which don't work without a device. Since there is no replacement, this
> patch allows again to register syscon regions with an associated device
> where that this device exists anyway.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>

Can you elaborate how you want to use the new interface?

My immediate reaction would be that drivers calling syscon_register()
are probably doing something wrong. If we want to restore the
debugfs interfaces, there might be a better way to do this automatically.

	Arnd

Re: [PATCH v2] mfd: syscon: allow to register syscon with a device

[Philipp Zabel]

Hi Arnd,

Am Mittwoch, den 24.02.2016, 13:18 +0100 schrieb Arnd Bergmann:
> On Wednesday 24 February 2016 12:19:01 Philipp Zabel wrote:
> > Commit bdb0066df96e ("mfd: syscon: Decouple syscon interface from platform
> > devices") added the possibility to register syscon devices without
> > associated platform device. This also removed regmap debugfs facilities,
> > which don't work without a device. Since there is no replacement, this
> > patch allows again to register syscon regions with an associated device
> > where that this device exists anyway.
> > 
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> 
> Can you elaborate how you want to use the new interface?

I use this patch to attach the regmap to the IOMUXC device, which the
GPR (syscon) region on i.MX6 is a part of:

diff --git a/drivers/pinctrl/freescale/pinctrl-imx6q.c b/drivers/pinctrl/freescale/pinctrl-imx6q.c
index 4d1fcb8..74a68ec 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx6q.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx6q.c
@@ -15,6 +15,7 @@
 #include <linux/err.h>
 #include <linux/init.h>
 #include <linux/io.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -473,6 +474,12 @@ static const struct of_device_id imx6q_pinctrl_of_match[] = {
 
 static int imx6q_pinctrl_probe(struct platform_device *pdev)
 {
+       struct device_node *syscon_np;
+
+       syscon_np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-iomuxc-gpr");
+       if (syscon_np)
+               syscon_register(&pdev->dev, syscon_np);
+
        return imx_pinctrl_probe(pdev, &imx6q_pinctrl_info);
 }
 
The pinctrl driver is probed at arch_initcall time.

> My immediate reaction would be that drivers calling syscon_register()
> are probably doing something wrong. If we want to restore the
> debugfs interfaces, there might be a better way to do this automatically.

Do you have any suggestion?

regards
Philipp

Re: [PATCH v2] mfd: syscon: allow to register syscon with a device

[Arnd Bergmann]

On Wednesday 24 February 2016 13:44:24 Philipp Zabel wrote:
> Am Mittwoch, den 24.02.2016, 13:18 +0100 schrieb Arnd Bergmann:
> > On Wednesday 24 February 2016 12:19:01 Philipp Zabel wrote:
> > > Commit bdb0066df96e ("mfd: syscon: Decouple syscon interface from platform
> > > devices") added the possibility to register syscon devices without
> > > associated platform device. This also removed regmap debugfs facilities,
> > > which don't work without a device. Since there is no replacement, this
> > > patch allows again to register syscon regions with an associated device
> > > where that this device exists anyway.
> > > 
> > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > 
> > Can you elaborate how you want to use the new interface?
> 
> I use this patch to attach the regmap to the IOMUXC device, which the
> GPR (syscon) region on i.MX6 is a part of:
> 
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx6q.c b/drivers/pinctrl/freescale/pinctrl-imx6q.c
> index 4d1fcb8..74a68ec 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx6q.c
> +++ b/drivers/pinctrl/freescale/pinctrl-imx6q.c
> @@ -15,6 +15,7 @@
>  #include <linux/err.h>
>  #include <linux/init.h>
>  #include <linux/io.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> @@ -473,6 +474,12 @@ static const struct of_device_id imx6q_pinctrl_of_match[] = {
>  
>  static int imx6q_pinctrl_probe(struct platform_device *pdev)
>  {
> +       struct device_node *syscon_np;
> +
> +       syscon_np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-iomuxc-gpr");
> +       if (syscon_np)
> +               syscon_register(&pdev->dev, syscon_np);
> +
>         return imx_pinctrl_probe(pdev, &imx6q_pinctrl_info);
>  }
>  
> The pinctrl driver is probed at arch_initcall time.

I still don't see it where you are getting with this. Is this just for
the debugfs interface or is there something else this does on top of
that?

	Arnd

Re: [PATCH v2] mfd: syscon: allow to register syscon with a device

[Philipp Zabel]

Am Mittwoch, den 24.02.2016, 16:00 +0100 schrieb Arnd Bergmann:
> On Wednesday 24 February 2016 13:44:24 Philipp Zabel wrote:
> > Am Mittwoch, den 24.02.2016, 13:18 +0100 schrieb Arnd Bergmann:
> > > On Wednesday 24 February 2016 12:19:01 Philipp Zabel wrote:
> > > > Commit bdb0066df96e ("mfd: syscon: Decouple syscon interface from platform
> > > > devices") added the possibility to register syscon devices without
> > > > associated platform device. This also removed regmap debugfs facilities,
> > > > which don't work without a device. Since there is no replacement, this
> > > > patch allows again to register syscon regions with an associated device
> > > > where that this device exists anyway.
> > > > 
> > > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > > 
> > > Can you elaborate how you want to use the new interface?
> > 
> > I use this patch to attach the regmap to the IOMUXC device, which the
> > GPR (syscon) region on i.MX6 is a part of:
> > 
> > diff --git a/drivers/pinctrl/freescale/pinctrl-imx6q.c b/drivers/pinctrl/freescale/pinctrl-imx6q.c
> > index 4d1fcb8..74a68ec 100644
> > --- a/drivers/pinctrl/freescale/pinctrl-imx6q.c
> > +++ b/drivers/pinctrl/freescale/pinctrl-imx6q.c
> > @@ -15,6 +15,7 @@
> >  #include <linux/err.h>
> >  #include <linux/init.h>
> >  #include <linux/io.h>
> > +#include <linux/mfd/syscon.h>
> >  #include <linux/module.h>
> >  #include <linux/of.h>
> >  #include <linux/of_device.h>
> > @@ -473,6 +474,12 @@ static const struct of_device_id imx6q_pinctrl_of_match[] = {
> >  
> >  static int imx6q_pinctrl_probe(struct platform_device *pdev)
> >  {
> > +       struct device_node *syscon_np;
> > +
> > +       syscon_np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-iomuxc-gpr");
> > +       if (syscon_np)
> > +               syscon_register(&pdev->dev, syscon_np);
> > +
> >         return imx_pinctrl_probe(pdev, &imx6q_pinctrl_info);
> >  }
> >  
> > The pinctrl driver is probed at arch_initcall time.
> 
> I still don't see it where you are getting with this. Is this just for
> the debugfs interface or is there something else this does on top of
> that?

Sorry, yes, this is about the missing debugfs functionality.

regards
Philipp

Re: [PATCH v2] mfd: syscon: allow to register syscon with a device

[Arnd Bergmann]

On Wednesday 24 February 2016 16:02:28 Philipp Zabel wrote:
> > > @@ -473,6 +474,12 @@ static const struct of_device_id imx6q_pinctrl_of_match[] = {
> > >  
> > >  static int imx6q_pinctrl_probe(struct platform_device *pdev)
> > >  {
> > > +       struct device_node *syscon_np;
> > > +
> > > +       syscon_np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-iomuxc-gpr");
> > > +       if (syscon_np)
> > > +               syscon_register(&pdev->dev, syscon_np);
> > > +
> > >         return imx_pinctrl_probe(pdev, &imx6q_pinctrl_info);
> > >  }
> > >  
> > > The pinctrl driver is probed at arch_initcall time.
> > 
> > I still don't see it where you are getting with this. Is this just for
> > the debugfs interface or is there something else this does on top of
> > that?
> 
> Sorry, yes, this is about the missing debugfs functionality.

Ok, so maybe there is a different way of doing this: If I understood
it correctly, we now lost the debugfs handling on all syscon nodes?

Should we try to come up with a way to bring it back for all of them
then, rather than having to do it per device?

I can see two possible ways of doing that:

a) change the regmap code to provide a debugfs interface for all
   regmaps, including those without a device
b) change the syscon driver to explicitly register the debugfs
   interface at a later point in boot when the devices become
   available

	Arnd

Re: [PATCH v2] mfd: syscon: allow to register syscon with a device

[Philipp Zabel]

Hi Arnd,

Am Mittwoch, den 24.02.2016, 17:26 +0100 schrieb Arnd Bergmann:
> On Wednesday 24 February 2016 16:02:28 Philipp Zabel wrote:
> > > > @@ -473,6 +474,12 @@ static const struct of_device_id imx6q_pinctrl_of_match[] = {
> > > >  
> > > >  static int imx6q_pinctrl_probe(struct platform_device *pdev)
> > > >  {
> > > > +       struct device_node *syscon_np;
> > > > +
> > > > +       syscon_np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-iomuxc-gpr");
> > > > +       if (syscon_np)
> > > > +               syscon_register(&pdev->dev, syscon_np);
> > > > +
> > > >         return imx_pinctrl_probe(pdev, &imx6q_pinctrl_info);
> > > >  }
> > > >  
> > > > The pinctrl driver is probed at arch_initcall time.
> > > 
> > > I still don't see it where you are getting with this. Is this just for
> > > the debugfs interface or is there something else this does on top of
> > > that?
> > 
> > Sorry, yes, this is about the missing debugfs functionality.
> 
> Ok, so maybe there is a different way of doing this: If I understood
> it correctly, we now lost the debugfs handling on all syscon nodes?
> 
> Should we try to come up with a way to bring it back for all of them
> then, rather than having to do it per device?
> 
> I can see two possible ways of doing that:
> 
> a) change the regmap code to provide a debugfs interface for all
>    regmaps, including those without a device
> b) change the syscon driver to explicitly register the debugfs
>    interface at a later point in boot when the devices become
>    available

Thanks, b) seems to fit well for the i.MX IOMUXC. I have sent a patch to
use regmap_attach_dev for that.

regards
Philipp