[PATCH] mfd: syscon: Decouple syscon interface from syscon devices

[PATCH] mfd: syscon: Decouple syscon interface from syscon devices

[Pankaj Dubey]

From: Tomasz Figa <t.figa@samsung.com>

Currently a syscon entity can be only registered directly through a
platform device that binds to a dedicated driver. However in certain use
cases it is desirable to make a device used with another driver a syscon
interface provider. For example, certain SoCs (e.g. Exynos) contain
system controller blocks which perform various functions such as power
domain control, CPU power management, low power mode control, but in
addition contain certain IP integration glue, such as various signal
masks, coprocessor power control, etc. In such case, there is a need to
have a dedicated driver for such system controller but also share
registers with other drivers. The latter is where the syscon interface
is helpful.

This patch decouples syscon object from syscon driver, so that it can be
registered from any driver in addition to the original "syscon" platform
driver.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---

RFC patch [1] was posted by Tomasz Figa. This patch addresses some of
comments given by Arnd to RFC patch, and further decouples syscon from
device model. It also gives flexibility of registering with syscon at
early stage using device_node object.

[1]: https://lkml.org/lkml/2014/6/17/331

 drivers/mfd/syscon.c       |  112 ++++++++++++++++++++++++++++----------------
 include/linux/mfd/syscon.h |   14 ++++++
 2 files changed, 86 insertions(+), 40 deletions(-)

diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
index ca15878..a91db30 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -14,6 +14,7 @@
 
 #include <linux/err.h>
 #include <linux/io.h>
+#include <linux/list.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
@@ -22,33 +23,32 @@
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/mfd/syscon.h>
+#include <linux/slab.h>
 
-static struct platform_driver syscon_driver;
+static DEFINE_SPINLOCK(syscon_list_slock);
+static LIST_HEAD(syscon_list);
 
 struct syscon {
+	struct device_node *np;
 	struct regmap *regmap;
+	struct list_head list;
 };
 
-static int syscon_match_node(struct device *dev, void *data)
-{
-	struct device_node *dn = data;
-
-	return (dev->of_node == dn) ? 1 : 0;
-}
-
 struct regmap *syscon_node_to_regmap(struct device_node *np)
 {
-	struct syscon *syscon;
-	struct device *dev;
+	struct syscon *entry, *syscon = NULL;
 
-	dev = driver_find_device(&syscon_driver.driver, NULL, np,
-				 syscon_match_node);
-	if (!dev)
-		return ERR_PTR(-EPROBE_DEFER);
+	spin_lock(&syscon_list_slock);
 
-	syscon = dev_get_drvdata(dev);
+	list_for_each_entry(entry, &syscon_list, list)
+		if (entry->np == np) {
+			syscon = entry;
+			break;
+		}
 
-	return syscon->regmap;
+	spin_unlock(&syscon_list_slock);
+
+	return syscon ? syscon->regmap : ERR_PTR(-EPROBE_DEFER);
 }
 EXPORT_SYMBOL_GPL(syscon_node_to_regmap);
 
@@ -68,24 +68,22 @@ struct regmap *syscon_regmap_lookup_by_compatible(const char *s)
 }
 EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_compatible);
 
-static int syscon_match_pdevname(struct device *dev, void *data)
-{
-	return !strcmp(dev_name(dev), (const char *)data);
-}
-
 struct regmap *syscon_regmap_lookup_by_pdevname(const char *s)
 {
-	struct device *dev;
-	struct syscon *syscon;
-
-	dev = driver_find_device(&syscon_driver.driver, NULL, (void *)s,
-				 syscon_match_pdevname);
-	if (!dev)
-		return ERR_PTR(-EPROBE_DEFER);
-
-	syscon = dev_get_drvdata(dev);
+	struct syscon *entry, *syscon = NULL;
+	struct platform_device *pdev = NULL;
+
+	spin_lock(&syscon_list_slock);
+	list_for_each_entry(entry, &syscon_list, list) {
+		 pdev = of_find_device_by_node(entry->np);
+		if (pdev && !strcmp(dev_name(&pdev->dev), s)) {
+			syscon = entry;
+			break;
+		}
+	}
+	spin_unlock(&syscon_list_slock);
 
-	return syscon->regmap;
+	return syscon ? syscon->regmap : ERR_PTR(-EPROBE_DEFER);
 }
 EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
 
@@ -121,17 +119,49 @@ static struct regmap_config syscon_regmap_config = {
 	.reg_stride = 4,
 };
 
+void of_syscon_unregister(struct device_node *np)
+{
+	struct syscon *entry;
+
+	spin_lock(&syscon_list_slock);
+
+	list_for_each_entry(entry, &syscon_list, list)
+		if (entry->np == np) {
+			list_del(&entry->list);
+			break;
+		}
+
+	spin_unlock(&syscon_list_slock);
+}
+EXPORT_SYMBOL_GPL(of_syscon_unregister);
+
+int of_syscon_register(struct device_node *np, struct regmap *regmap)
+{
+	struct syscon *syscon;
+
+	syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
+	if (!syscon)
+		return -ENOMEM;
+
+	syscon->regmap = regmap;
+	syscon->np = np;
+
+	spin_lock(&syscon_list_slock);
+	list_add_tail(&syscon->list, &syscon_list);
+	spin_unlock(&syscon_list_slock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_syscon_register);
+
 static int syscon_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct syscon_platform_data *pdata = dev_get_platdata(dev);
-	struct syscon *syscon;
+	struct regmap *regmap;
 	struct resource *res;
 	void __iomem *base;
-
-	syscon = devm_kzalloc(dev, sizeof(*syscon), GFP_KERNEL);
-	if (!syscon)
-		return -ENOMEM;
+	int ret;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res)
@@ -144,14 +174,16 @@ static int syscon_probe(struct platform_device *pdev)
 	syscon_regmap_config.max_register = res->end - res->start - 3;
 	if (pdata)
 		syscon_regmap_config.name = pdata->label;
-	syscon->regmap = devm_regmap_init_mmio(dev, base,
+	regmap = devm_regmap_init_mmio(dev, base,
 					&syscon_regmap_config);
-	if (IS_ERR(syscon->regmap)) {
+	if (IS_ERR(regmap)) {
 		dev_err(dev, "regmap init failed\n");
-		return PTR_ERR(syscon->regmap);
+		return PTR_ERR(regmap);
 	}
 
-	platform_set_drvdata(pdev, syscon);
+	ret = of_syscon_register(dev->of_node, regmap);
+	if (ret)
+		return ret;
 
 	dev_dbg(dev, "regmap %pR registered\n", res);
 
diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
index 75e543b..dc2807b 100644
--- a/include/linux/mfd/syscon.h
+++ b/include/linux/mfd/syscon.h
@@ -18,8 +18,12 @@
 #include <linux/err.h>
 
 struct device_node;
+struct regmap;
 
 #ifdef CONFIG_MFD_SYSCON
+extern int of_syscon_register(struct device_node *np, struct regmap *regmap);
+extern void of_syscon_unregister(struct device_node *np);
+
 extern struct regmap *syscon_node_to_regmap(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);
@@ -27,6 +31,16 @@ extern struct regmap *syscon_regmap_lookup_by_phandle(
 					struct device_node *np,
 					const char *property);
 #else
+static inline int of_syscon_register(struct device_node *np,
+				       struct regmap *regmap)
+{
+	return -ENOSYS;
+}
+
+static inline void of_syscon_unregister(struct device_node *np)
+{
+}
+
 static inline struct regmap *syscon_node_to_regmap(struct device_node *np)
 {
 	return ERR_PTR(-ENOSYS);
-- 
1.7.9.5

RE: [PATCH] mfd: syscon: Decouple syscon interface from syscon devices

[Pankaj Dubey]

Sorry I forgot to add maintainer into CC. 

+Lee Jones

Any comments on this patch. 
As a lot of Exynos PMU patch sets are dependent on this patch.

Thanks,
Pankaj Dubey
	

> -----Original Message-----
> From: Pankaj Dubey [mailto:pankaj.dubey@samsung.com]
> Sent: Friday, August 22, 2014 1:40 PM
> To: linux-arm-kernel@lists.infradead.org;
linux-samsung-soc@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Cc: kgene.kim@samsung.com; linux@arm.linux.org.uk; arnd@arndb.de;
> vikas.sajjan@samsung.com; joshi@samsung.com; naushad@samsung.com;
> thomas.ab@samsung.com; chow.kim@samsung.com; Tomasz Figa; Pankaj Dubey
> Subject: [PATCH] mfd: syscon: Decouple syscon interface from syscon
devices
> 
> From: Tomasz Figa <t.figa@samsung.com>
> 
> Currently a syscon entity can be only registered directly through a
platform device
> that binds to a dedicated driver. However in certain use cases it is
desirable to make a
> device used with another driver a syscon interface provider. For example,
certain
> SoCs (e.g. Exynos) contain system controller blocks which perform various
functions
> such as power domain control, CPU power management, low power mode
control,
> but in addition contain certain IP integration glue, such as various
signal masks,
> coprocessor power control, etc. In such case, there is a need to have a
dedicated
> driver for such system controller but also share registers with other
drivers. The latter
> is where the syscon interface is helpful.
> 
> This patch decouples syscon object from syscon driver, so that it can be
registered
> from any driver in addition to the original "syscon" platform driver.
> 
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
> 
> RFC patch [1] was posted by Tomasz Figa. This patch addresses some of
comments
> given by Arnd to RFC patch, and further decouples syscon from device
model. It also
> gives flexibility of registering with syscon at early stage using
device_node object.
> 
> [1]: https://lkml.org/lkml/2014/6/17/331
> 
>  drivers/mfd/syscon.c       |  112 ++++++++++++++++++++++++++++----
> ------------
>  include/linux/mfd/syscon.h |   14 ++++++
>  2 files changed, 86 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c index
ca15878..a91db30
> 100644
> --- a/drivers/mfd/syscon.c
> +++ b/drivers/mfd/syscon.c
> @@ -14,6 +14,7 @@
> 
>  #include <linux/err.h>
>  #include <linux/io.h>
> +#include <linux/list.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
> @@ -22,33 +23,32 @@
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
>  #include <linux/mfd/syscon.h>
> +#include <linux/slab.h>
> 
> -static struct platform_driver syscon_driver;
> +static DEFINE_SPINLOCK(syscon_list_slock);
> +static LIST_HEAD(syscon_list);
> 
>  struct syscon {
> +	struct device_node *np;
>  	struct regmap *regmap;
> +	struct list_head list;
>  };
> 
> -static int syscon_match_node(struct device *dev, void *data) -{
> -	struct device_node *dn = data;
> -
> -	return (dev->of_node == dn) ? 1 : 0;
> -}
> -
>  struct regmap *syscon_node_to_regmap(struct device_node *np)  {
> -	struct syscon *syscon;
> -	struct device *dev;
> +	struct syscon *entry, *syscon = NULL;
> 
> -	dev = driver_find_device(&syscon_driver.driver, NULL, np,
> -				 syscon_match_node);
> -	if (!dev)
> -		return ERR_PTR(-EPROBE_DEFER);
> +	spin_lock(&syscon_list_slock);
> 
> -	syscon = dev_get_drvdata(dev);
> +	list_for_each_entry(entry, &syscon_list, list)
> +		if (entry->np == np) {
> +			syscon = entry;
> +			break;
> +		}
> 
> -	return syscon->regmap;
> +	spin_unlock(&syscon_list_slock);
> +
> +	return syscon ? syscon->regmap : ERR_PTR(-EPROBE_DEFER);
>  }
>  EXPORT_SYMBOL_GPL(syscon_node_to_regmap);
> 
> @@ -68,24 +68,22 @@ struct regmap
> *syscon_regmap_lookup_by_compatible(const char *s)  }
> EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_compatible);
> 
> -static int syscon_match_pdevname(struct device *dev, void *data) -{
> -	return !strcmp(dev_name(dev), (const char *)data);
> -}
> -
>  struct regmap *syscon_regmap_lookup_by_pdevname(const char *s)  {
> -	struct device *dev;
> -	struct syscon *syscon;
> -
> -	dev = driver_find_device(&syscon_driver.driver, NULL, (void *)s,
> -				 syscon_match_pdevname);
> -	if (!dev)
> -		return ERR_PTR(-EPROBE_DEFER);
> -
> -	syscon = dev_get_drvdata(dev);
> +	struct syscon *entry, *syscon = NULL;
> +	struct platform_device *pdev = NULL;
> +
> +	spin_lock(&syscon_list_slock);
> +	list_for_each_entry(entry, &syscon_list, list) {
> +		 pdev = of_find_device_by_node(entry->np);
> +		if (pdev && !strcmp(dev_name(&pdev->dev), s)) {
> +			syscon = entry;
> +			break;
> +		}
> +	}
> +	spin_unlock(&syscon_list_slock);
> 
> -	return syscon->regmap;
> +	return syscon ? syscon->regmap : ERR_PTR(-EPROBE_DEFER);
>  }
>  EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
> 
> @@ -121,17 +119,49 @@ static struct regmap_config syscon_regmap_config = {
>  	.reg_stride = 4,
>  };
> 
> +void of_syscon_unregister(struct device_node *np) {
> +	struct syscon *entry;
> +
> +	spin_lock(&syscon_list_slock);
> +
> +	list_for_each_entry(entry, &syscon_list, list)
> +		if (entry->np == np) {
> +			list_del(&entry->list);
> +			break;
> +		}
> +
> +	spin_unlock(&syscon_list_slock);
> +}
> +EXPORT_SYMBOL_GPL(of_syscon_unregister);
> +
> +int of_syscon_register(struct device_node *np, struct regmap *regmap) {
> +	struct syscon *syscon;
> +
> +	syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
> +	if (!syscon)
> +		return -ENOMEM;
> +
> +	syscon->regmap = regmap;
> +	syscon->np = np;
> +
> +	spin_lock(&syscon_list_slock);
> +	list_add_tail(&syscon->list, &syscon_list);
> +	spin_unlock(&syscon_list_slock);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_syscon_register);
> +
>  static int syscon_probe(struct platform_device *pdev)  {
>  	struct device *dev = &pdev->dev;
>  	struct syscon_platform_data *pdata = dev_get_platdata(dev);
> -	struct syscon *syscon;
> +	struct regmap *regmap;
>  	struct resource *res;
>  	void __iomem *base;
> -
> -	syscon = devm_kzalloc(dev, sizeof(*syscon), GFP_KERNEL);
> -	if (!syscon)
> -		return -ENOMEM;
> +	int ret;
> 
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!res)
> @@ -144,14 +174,16 @@ static int syscon_probe(struct platform_device
*pdev)
>  	syscon_regmap_config.max_register = res->end - res->start - 3;
>  	if (pdata)
>  		syscon_regmap_config.name = pdata->label;
> -	syscon->regmap = devm_regmap_init_mmio(dev, base,
> +	regmap = devm_regmap_init_mmio(dev, base,
>  					&syscon_regmap_config);
> -	if (IS_ERR(syscon->regmap)) {
> +	if (IS_ERR(regmap)) {
>  		dev_err(dev, "regmap init failed\n");
> -		return PTR_ERR(syscon->regmap);
> +		return PTR_ERR(regmap);
>  	}
> 
> -	platform_set_drvdata(pdev, syscon);
> +	ret = of_syscon_register(dev->of_node, regmap);
> +	if (ret)
> +		return ret;
> 
>  	dev_dbg(dev, "regmap %pR registered\n", res);
> 
> diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h index
> 75e543b..dc2807b 100644
> --- a/include/linux/mfd/syscon.h
> +++ b/include/linux/mfd/syscon.h
> @@ -18,8 +18,12 @@
>  #include <linux/err.h>
> 
>  struct device_node;
> +struct regmap;
> 
>  #ifdef CONFIG_MFD_SYSCON
> +extern int of_syscon_register(struct device_node *np, struct regmap
> +*regmap); extern void of_syscon_unregister(struct device_node *np);
> +
>  extern struct regmap *syscon_node_to_regmap(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); @@ -27,6 +31,16
> @@ extern struct regmap *syscon_regmap_lookup_by_phandle(
>  					struct device_node *np,
>  					const char *property);
>  #else
> +static inline int of_syscon_register(struct device_node *np,
> +				       struct regmap *regmap)
> +{
> +	return -ENOSYS;
> +}
> +
> +static inline void of_syscon_unregister(struct device_node *np) { }
> +
>  static inline struct regmap *syscon_node_to_regmap(struct device_node
*np)  {
>  	return ERR_PTR(-ENOSYS);
> --
> 1.7.9.5

Re: [PATCH] mfd: syscon: Decouple syscon interface from syscon devices

[Lee Jones]

On Fri, 22 Aug 2014, Pankaj Dubey wrote:

> From: Tomasz Figa <t.figa@samsung.com>
> 
> Currently a syscon entity can be only registered directly through a
> platform device that binds to a dedicated driver. However in certain use
> cases it is desirable to make a device used with another driver a syscon
> interface provider. For example, certain SoCs (e.g. Exynos) contain
> system controller blocks which perform various functions such as power
> domain control, CPU power management, low power mode control, but in
> addition contain certain IP integration glue, such as various signal
> masks, coprocessor power control, etc. In such case, there is a need to
> have a dedicated driver for such system controller but also share
> registers with other drivers. The latter is where the syscon interface
> is helpful.
> 
> This patch decouples syscon object from syscon driver, so that it can be
> registered from any driver in addition to the original "syscon" platform
> driver.
> 
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
> 
> RFC patch [1] was posted by Tomasz Figa. This patch addresses some of
> comments given by Arnd to RFC patch, and further decouples syscon from
> device model. It also gives flexibility of registering with syscon at
> early stage using device_node object.

It would be helpful if Arnd gave this revision his blessing (Ack).

> [1]: https://lkml.org/lkml/2014/6/17/331
> 
>  drivers/mfd/syscon.c       |  112 ++++++++++++++++++++++++++++----------------
>  include/linux/mfd/syscon.h |   14 ++++++
>  2 files changed, 86 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
> index ca15878..a91db30 100644
> --- a/drivers/mfd/syscon.c
> +++ b/drivers/mfd/syscon.c
> @@ -14,6 +14,7 @@

[...]

>  struct regmap *syscon_regmap_lookup_by_pdevname(const char *s)
>  {
> -	struct device *dev;
> -	struct syscon *syscon;
> -
> -	dev = driver_find_device(&syscon_driver.driver, NULL, (void *)s,
> -				 syscon_match_pdevname);
> -	if (!dev)
> -		return ERR_PTR(-EPROBE_DEFER);
> -
> -	syscon = dev_get_drvdata(dev);
> +	struct syscon *entry, *syscon = NULL;
> +	struct platform_device *pdev = NULL;
> +
> +	spin_lock(&syscon_list_slock);
> +	list_for_each_entry(entry, &syscon_list, list) {
> +		 pdev = of_find_device_by_node(entry->np);

White space error.

Did you run this through checkpatch.pl?

> +		if (pdev && !strcmp(dev_name(&pdev->dev), s)) {
> +			syscon = entry;
> +			break;
> +		}
> +	}
> +	spin_unlock(&syscon_list_slock);
>  
> -	return syscon->regmap;
> +	return syscon ? syscon->regmap : ERR_PTR(-EPROBE_DEFER);
>  }
>  EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);

[...]

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] mfd: syscon: Decouple syscon interface from syscon devices

[Arnd Bergmann]

On Monday 01 September 2014 08:49:18 Lee Jones wrote:
> On Fri, 22 Aug 2014, Pankaj Dubey wrote:
> 
> > From: Tomasz Figa <t.figa@samsung.com>
> > 
> > Currently a syscon entity can be only registered directly through a
> > platform device that binds to a dedicated driver. However in certain use
> > cases it is desirable to make a device used with another driver a syscon
> > interface provider. For example, certain SoCs (e.g. Exynos) contain
> > system controller blocks which perform various functions such as power
> > domain control, CPU power management, low power mode control, but in
> > addition contain certain IP integration glue, such as various signal
> > masks, coprocessor power control, etc. In such case, there is a need to
> > have a dedicated driver for such system controller but also share
> > registers with other drivers. The latter is where the syscon interface
> > is helpful.
> > 
> > This patch decouples syscon object from syscon driver, so that it can be
> > registered from any driver in addition to the original "syscon" platform
> > driver.
> > 
> > Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> > ---
> > 
> > RFC patch [1] was posted by Tomasz Figa. This patch addresses some of
> > comments given by Arnd to RFC patch, and further decouples syscon from
> > device model. It also gives flexibility of registering with syscon at
> > early stage using device_node object.
> 
> It would be helpful if Arnd gave this revision his blessing (Ack).

I never saw a reason why we don't take this all the way as discussed
a few times: Completely remove the dependency of syscon on having
a platform driver for it, and make it possible to just call
syscon_regmap_lookup_by_phandle() without having to register
it first.

	Arnd

Re: [PATCH] mfd: syscon: Decouple syscon interface from syscon devices

[Lee Jones]

On Mon, 01 Sep 2014, Arnd Bergmann wrote:
> On Monday 01 September 2014 08:49:18 Lee Jones wrote:
> > On Fri, 22 Aug 2014, Pankaj Dubey wrote:
> > 
> > > From: Tomasz Figa <t.figa@samsung.com>
> > > 
> > > Currently a syscon entity can be only registered directly through a
> > > platform device that binds to a dedicated driver. However in certain use
> > > cases it is desirable to make a device used with another driver a syscon
> > > interface provider. For example, certain SoCs (e.g. Exynos) contain
> > > system controller blocks which perform various functions such as power
> > > domain control, CPU power management, low power mode control, but in
> > > addition contain certain IP integration glue, such as various signal
> > > masks, coprocessor power control, etc. In such case, there is a need to
> > > have a dedicated driver for such system controller but also share
> > > registers with other drivers. The latter is where the syscon interface
> > > is helpful.
> > > 
> > > This patch decouples syscon object from syscon driver, so that it can be
> > > registered from any driver in addition to the original "syscon" platform
> > > driver.
> > > 
> > > Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> > > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> > > ---
> > > 
> > > RFC patch [1] was posted by Tomasz Figa. This patch addresses some of
> > > comments given by Arnd to RFC patch, and further decouples syscon from
> > > device model. It also gives flexibility of registering with syscon at
> > > early stage using device_node object.
> > 
> > It would be helpful if Arnd gave this revision his blessing (Ack).
> 
> I never saw a reason why we don't take this all the way as discussed
> a few times: Completely remove the dependency of syscon on having
> a platform driver for it, and make it possible to just call
> syscon_regmap_lookup_by_phandle() without having to register
> it first.

I think this sounds like a good end-state.  Migrating over by
supporting both methods in this way does sound like the correct thing
to do though.  Doing so is likely to dramatically reduce the effect on
current users.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

RE: [PATCH] mfd: syscon: Decouple syscon interface from syscon devices

[Pankaj Dubey]

Hi Arnd,

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Monday, September 01, 2014 4:07 PM
> To: Lee Jones
> Cc: Pankaj Dubey; linux-arm-kernel@lists.infradead.org; linux-samsung-
> soc@vger.kernel.org; linux-kernel@vger.kernel.org; kgene.kim@samsung.com;
> linux@arm.linux.org.uk; vikas.sajjan@samsung.com; joshi@samsung.com;
> naushad@samsung.com; thomas.ab@samsung.com; chow.kim@samsung.com;
> Tomasz Figa; broonie@kernel.org
> Subject: Re: [PATCH] mfd: syscon: Decouple syscon interface from syscon
devices
> 
> On Monday 01 September 2014 08:49:18 Lee Jones wrote:
> > On Fri, 22 Aug 2014, Pankaj Dubey wrote:
> >
> > > From: Tomasz Figa <t.figa@samsung.com>
> > >
> > > Currently a syscon entity can be only registered directly through a
> > > platform device that binds to a dedicated driver. However in certain
> > > use cases it is desirable to make a device used with another driver
> > > a syscon interface provider. For example, certain SoCs (e.g. Exynos)
> > > contain system controller blocks which perform various functions
> > > such as power domain control, CPU power management, low power mode
> > > control, but in addition contain certain IP integration glue, such
> > > as various signal masks, coprocessor power control, etc. In such
> > > case, there is a need to have a dedicated driver for such system
> > > controller but also share registers with other drivers. The latter
> > > is where the syscon interface is helpful.
> > >
> > > This patch decouples syscon object from syscon driver, so that it
> > > can be registered from any driver in addition to the original
> > > "syscon" platform driver.
> > >
> > > Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> > > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> > > ---
> > >
> > > RFC patch [1] was posted by Tomasz Figa. This patch addresses some
> > > of comments given by Arnd to RFC patch, and further decouples syscon
> > > from device model. It also gives flexibility of registering with
> > > syscon at early stage using device_node object.
> >
> > It would be helpful if Arnd gave this revision his blessing (Ack).
> 
> I never saw a reason why we don't take this all the way as discussed a few
times:
> Completely remove the dependency of syscon on having a platform driver for
it, and
> make it possible to just call
> syscon_regmap_lookup_by_phandle() without having to register it first.
> 

Please have a look at this thread [1], where we discussed the need of syscon
as platform device.
Looks like still there are users of syscon_regmap_lookup_by_pdevname
(clps711x.c).
So we can't make it completely independent of platform_device. 

[1]
https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg35708.html


> 	Arnd

Re: [PATCH] mfd: syscon: Decouple syscon interface from syscon devices

[Arnd Bergmann]

On Monday 01 September 2014 17:05:33 Pankaj Dubey wrote:
> 
> Please have a look at this thread [1], where we discussed the need of syscon
> as platform device.
> Looks like still there are users of syscon_regmap_lookup_by_pdevname
> (clps711x.c).
> So we can't make it completely independent of platform_device. 
> 
> [1]
> https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg35708.html

I think the clps711x platform will soon stop using it, we already
have patches for migrating to DT.

	Arnd

Re: [PATCH] mfd: syscon: Decouple syscon interface from syscon devices

[Arnd Bergmann]

On Monday 01 September 2014 12:25:49 Lee Jones wrote:
> On Mon, 01 Sep 2014, Arnd Bergmann wrote:
> > On Monday 01 September 2014 08:49:18 Lee Jones wrote:
> > > On Fri, 22 Aug 2014, Pankaj Dubey wrote:
> > > 
> > > > From: Tomasz Figa <t.figa@samsung.com>
> > > > 
> > > > Currently a syscon entity can be only registered directly through a
> > > > platform device that binds to a dedicated driver. However in certain use
> > > > cases it is desirable to make a device used with another driver a syscon
> > > > interface provider. For example, certain SoCs (e.g. Exynos) contain
> > > > system controller blocks which perform various functions such as power
> > > > domain control, CPU power management, low power mode control, but in
> > > > addition contain certain IP integration glue, such as various signal
> > > > masks, coprocessor power control, etc. In such case, there is a need to
> > > > have a dedicated driver for such system controller but also share
> > > > registers with other drivers. The latter is where the syscon interface
> > > > is helpful.
> > > > 
> > > > This patch decouples syscon object from syscon driver, so that it can be
> > > > registered from any driver in addition to the original "syscon" platform
> > > > driver.
> > > > 
> > > > Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> > > > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> > > > ---
> > > > 
> > > > RFC patch [1] was posted by Tomasz Figa. This patch addresses some of
> > > > comments given by Arnd to RFC patch, and further decouples syscon from
> > > > device model. It also gives flexibility of registering with syscon at
> > > > early stage using device_node object.
> > > 
> > > It would be helpful if Arnd gave this revision his blessing (Ack).
> > 
> > I never saw a reason why we don't take this all the way as discussed
> > a few times: Completely remove the dependency of syscon on having
> > a platform driver for it, and make it possible to just call
> > syscon_regmap_lookup_by_phandle() without having to register
> > it first.
> 
> I think this sounds like a good end-state.  Migrating over by
> supporting both methods in this way does sound like the correct thing
> to do though.  Doing so is likely to dramatically reduce the effect on
> current users.

Maybe I'm misreading the patch, but I don't see how it creates a
migration path. What I want to end up with is infrastructure that
lets anybody call syscon_regmap_lookup_by_pdevname or
syscon_regmap_lookup_by_compatible (if they really need to)
without needing the platform_driver for syscon. That should not
require any form of compatibility layer because to the driver
using it there is no API change.

In contrast, this patch introduces a new of_syscon_{un,}register()
interface that would get removed after the the above has
been implemented, causing extra churn for any driver that also
wants to provide a regmap-like interface.

	Arnd

Re: [PATCH] mfd: syscon: Decouple syscon interface from syscon devices

[Lee Jones]

On Mon, 01 Sep 2014, Arnd Bergmann wrote:

> On Monday 01 September 2014 12:25:49 Lee Jones wrote:
> > On Mon, 01 Sep 2014, Arnd Bergmann wrote:
> > > On Monday 01 September 2014 08:49:18 Lee Jones wrote:
> > > > On Fri, 22 Aug 2014, Pankaj Dubey wrote:
> > > > 
> > > > > From: Tomasz Figa <t.figa@samsung.com>
> > > > > 
> > > > > Currently a syscon entity can be only registered directly through a
> > > > > platform device that binds to a dedicated driver. However in certain use
> > > > > cases it is desirable to make a device used with another driver a syscon
> > > > > interface provider. For example, certain SoCs (e.g. Exynos) contain
> > > > > system controller blocks which perform various functions such as power
> > > > > domain control, CPU power management, low power mode control, but in
> > > > > addition contain certain IP integration glue, such as various signal
> > > > > masks, coprocessor power control, etc. In such case, there is a need to
> > > > > have a dedicated driver for such system controller but also share
> > > > > registers with other drivers. The latter is where the syscon interface
> > > > > is helpful.
> > > > > 
> > > > > This patch decouples syscon object from syscon driver, so that it can be
> > > > > registered from any driver in addition to the original "syscon" platform
> > > > > driver.
> > > > > 
> > > > > Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> > > > > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> > > > > ---
> > > > > 
> > > > > RFC patch [1] was posted by Tomasz Figa. This patch addresses some of
> > > > > comments given by Arnd to RFC patch, and further decouples syscon from
> > > > > device model. It also gives flexibility of registering with syscon at
> > > > > early stage using device_node object.
> > > > 
> > > > It would be helpful if Arnd gave this revision his blessing (Ack).
> > > 
> > > I never saw a reason why we don't take this all the way as discussed
> > > a few times: Completely remove the dependency of syscon on having
> > > a platform driver for it, and make it possible to just call
> > > syscon_regmap_lookup_by_phandle() without having to register
> > > it first.
> > 
> > I think this sounds like a good end-state.  Migrating over by
> > supporting both methods in this way does sound like the correct thing
> > to do though.  Doing so is likely to dramatically reduce the effect on
> > current users.
> 
> Maybe I'm misreading the patch, but I don't see how it creates a
> migration path. What I want to end up with is infrastructure that
> lets anybody call syscon_regmap_lookup_by_pdevname or
> syscon_regmap_lookup_by_compatible (if they really need to)
> without needing the platform_driver for syscon. That should not
> require any form of compatibility layer because to the driver
> using it there is no API change.

Somehow I think the likelyhood is that I am misreading the patch.

I thought that before this patch drivers we had to register a syscon
device to bind to this driver, which was fine for the first use-cases
of syscon as it wasn't required too early during boot.  However, now
there are use-cases where systems require access to syscon registers
eariler in boot we require a means to obtain access prior to device
probing.  I thought this patch not only provides that possibilty, but
also leaves in the ability to register direct from DT.

> In contrast, this patch introduces a new of_syscon_{un,}register()
> interface that would get removed after the the above has
> been implemented, causing extra churn for any driver that also
> wants to provide a regmap-like interface.

When will we ever not have to register syscon?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] mfd: syscon: Decouple syscon interface from syscon devices

[Arnd Bergmann]

On Monday 01 September 2014 17:04:26 Lee Jones wrote:
> On Mon, 01 Sep 2014, Arnd Bergmann wrote:
> > Maybe I'm misreading the patch, but I don't see how it creates a
> > migration path. What I want to end up with is infrastructure that
> > lets anybody call syscon_regmap_lookup_by_pdevname or
> > syscon_regmap_lookup_by_compatible (if they really need to)
> > without needing the platform_driver for syscon. That should not
> > require any form of compatibility layer because to the driver
> > using it there is no API change.
> 
> Somehow I think the likelyhood is that I am misreading the patch.
> 
> I thought that before this patch drivers we had to register a syscon
> device to bind to this driver, which was fine for the first use-cases
> of syscon as it wasn't required too early during boot.  However, now
> there are use-cases where systems require access to syscon registers
> eariler in boot we require a means to obtain access prior to device
> probing.  I thought this patch not only provides that possibilty, but
> also leaves in the ability to register direct from DT.

Right, it does provide the ability to have syscon before devices
are registered, I missed that part.

> > In contrast, this patch introduces a new of_syscon_{un,}register()
> > interface that would get removed after the the above has
> > been implemented, causing extra churn for any driver that also
> > wants to provide a regmap-like interface.
> 
> When will we ever not have to register syscon?

The idea is that we implicitly register the syscon block when someone
calls syscon_regmap_lookup_by_compatible or syscon_regmap_lookup_by_phandle
and then return a reference to that new syscon. When another driver
looks up the same device node, we just pass a reference to the existing
syscon.

	Arnd

Re: [PATCH] mfd: syscon: Decouple syscon interface from syscon devices

[Lee Jones]

On Mon, 01 Sep 2014, Arnd Bergmann wrote:

> On Monday 01 September 2014 17:04:26 Lee Jones wrote:
> > On Mon, 01 Sep 2014, Arnd Bergmann wrote:
> > > Maybe I'm misreading the patch, but I don't see how it creates a
> > > migration path. What I want to end up with is infrastructure that
> > > lets anybody call syscon_regmap_lookup_by_pdevname or
> > > syscon_regmap_lookup_by_compatible (if they really need to)
> > > without needing the platform_driver for syscon. That should not
> > > require any form of compatibility layer because to the driver
> > > using it there is no API change.
> > 
> > Somehow I think the likelyhood is that I am misreading the patch.
> > 
> > I thought that before this patch drivers we had to register a syscon
> > device to bind to this driver, which was fine for the first use-cases
> > of syscon as it wasn't required too early during boot.  However, now
> > there are use-cases where systems require access to syscon registers
> > eariler in boot we require a means to obtain access prior to device
> > probing.  I thought this patch not only provides that possibilty, but
> > also leaves in the ability to register direct from DT.
> 
> Right, it does provide the ability to have syscon before devices
> are registered, I missed that part.
> 
> > > In contrast, this patch introduces a new of_syscon_{un,}register()
> > > interface that would get removed after the the above has
> > > been implemented, causing extra churn for any driver that also
> > > wants to provide a regmap-like interface.
> > 
> > When will we ever not have to register syscon?
> 
> The idea is that we implicitly register the syscon block when someone
> calls syscon_regmap_lookup_by_compatible or syscon_regmap_lookup_by_phandle
> and then return a reference to that new syscon. When another driver
> looks up the same device node, we just pass a reference to the existing
> syscon.

Doesn't sound too unreasonable.  So how about instead of exporting
these new of_syscon_{un,}register() calls, we make them static and
call them from syscon_regmap_lookup_by_{phandle,compatible}?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] mfd: syscon: Decouple syscon interface from syscon devices

[Arnd Bergmann]

On Tuesday 02 September 2014 09:05:16 Lee Jones wrote:
> On Mon, 01 Sep 2014, Arnd Bergmann wrote:
> 
> > On Monday 01 September 2014 17:04:26 Lee Jones wrote:
> > > On Mon, 01 Sep 2014, Arnd Bergmann wrote:
> > > > Maybe I'm misreading the patch, but I don't see how it creates a
> > > > migration path. What I want to end up with is infrastructure that
> > > > lets anybody call syscon_regmap_lookup_by_pdevname or
> > > > syscon_regmap_lookup_by_compatible (if they really need to)
> > > > without needing the platform_driver for syscon. That should not
> > > > require any form of compatibility layer because to the driver
> > > > using it there is no API change.
> > > 
> > > Somehow I think the likelyhood is that I am misreading the patch.
> > > 
> > > I thought that before this patch drivers we had to register a syscon
> > > device to bind to this driver, which was fine for the first use-cases
> > > of syscon as it wasn't required too early during boot.  However, now
> > > there are use-cases where systems require access to syscon registers
> > > eariler in boot we require a means to obtain access prior to device
> > > probing.  I thought this patch not only provides that possibilty, but
> > > also leaves in the ability to register direct from DT.
> > 
> > Right, it does provide the ability to have syscon before devices
> > are registered, I missed that part.
> > 
> > > > In contrast, this patch introduces a new of_syscon_{un,}register()
> > > > interface that would get removed after the the above has
> > > > been implemented, causing extra churn for any driver that also
> > > > wants to provide a regmap-like interface.
> > > 
> > > When will we ever not have to register syscon?
> > 
> > The idea is that we implicitly register the syscon block when someone
> > calls syscon_regmap_lookup_by_compatible or syscon_regmap_lookup_by_phandle
> > and then return a reference to that new syscon. When another driver
> > looks up the same device node, we just pass a reference to the existing
> > syscon.
> 
> Doesn't sound too unreasonable.  So how about instead of exporting
> these new of_syscon_{un,}register() calls, we make them static and
> call them from syscon_regmap_lookup_by_{phandle,compatible}?

Yes, that would be a good start. We should think about whether we want
to remove the existing DT probing at the same time, since it becomes
unused, and we might want to move the code to drivers/base/regmap_*.c
at some point.

	Arnd

RE: [PATCH] mfd: syscon: Decouple syscon interface from syscon devices

[Pankaj Dubey]

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Tuesday, September 02, 2014 1:45 PM
> To: linux-arm-kernel@lists.infradead.org
> Cc: Lee Jones; kgene.kim@samsung.com; linux@arm.linux.org.uk;
> naushad@samsung.com; Pankaj Dubey; Tomasz Figa;
linux-kernel@vger.kernel.org;
> joshi@samsung.com; vikas.sajjan@samsung.com; linux-samsung-
> soc@vger.kernel.org; broonie@kernel.org; thomas.ab@samsung.com;
> chow.kim@samsung.com
> Subject: Re: [PATCH] mfd: syscon: Decouple syscon interface from syscon
devices
> 
> On Tuesday 02 September 2014 09:05:16 Lee Jones wrote:
> > On Mon, 01 Sep 2014, Arnd Bergmann wrote:
> >
> > > On Monday 01 September 2014 17:04:26 Lee Jones wrote:
> > > > On Mon, 01 Sep 2014, Arnd Bergmann wrote:
> > > > > Maybe I'm misreading the patch, but I don't see how it creates a
> > > > > migration path. What I want to end up with is infrastructure
> > > > > that lets anybody call syscon_regmap_lookup_by_pdevname or
> > > > > syscon_regmap_lookup_by_compatible (if they really need to)
> > > > > without needing the platform_driver for syscon. That should not
> > > > > require any form of compatibility layer because to the driver
> > > > > using it there is no API change.
> > > >
> > > > Somehow I think the likelyhood is that I am misreading the patch.
> > > >
> > > > I thought that before this patch drivers we had to register a
> > > > syscon device to bind to this driver, which was fine for the first
> > > > use-cases of syscon as it wasn't required too early during boot.
> > > > However, now there are use-cases where systems require access to
> > > > syscon registers eariler in boot we require a means to obtain
> > > > access prior to device probing.  I thought this patch not only
> > > > provides that possibilty, but also leaves in the ability to register
direct from
> DT.
> > >
> > > Right, it does provide the ability to have syscon before devices are
> > > registered, I missed that part.
> > >
> > > > > In contrast, this patch introduces a new
> > > > > of_syscon_{un,}register() interface that would get removed after
> > > > > the the above has been implemented, causing extra churn for any
> > > > > driver that also wants to provide a regmap-like interface.
> > > >
> > > > When will we ever not have to register syscon?
> > >
> > > The idea is that we implicitly register the syscon block when
> > > someone calls syscon_regmap_lookup_by_compatible or
> > > syscon_regmap_lookup_by_phandle and then return a reference to that
> > > new syscon. When another driver looks up the same device node, we
> > > just pass a reference to the existing syscon.
> >
> > Doesn't sound too unreasonable.  So how about instead of exporting
> > these new of_syscon_{un,}register() calls, we make them static and
> > call them from syscon_regmap_lookup_by_{phandle,compatible}?
> 
> Yes, that would be a good start. We should think about whether we want to
remove
> the existing DT probing at the same time, since it becomes unused, and we
might
> want to move the code to drivers/base/regmap_*.c at some point.
> 


OK, I am working on these two parts. Once code is in proper form and tested
on
our setup I will post here.

Thanks,
Pankaj

> 	Arnd

Re: [PATCH] mfd: syscon: Decouple syscon interface from syscon devices

[Lee Jones]

On Tue, 02 Sep 2014, Arnd Bergmann wrote:

> On Tuesday 02 September 2014 09:05:16 Lee Jones wrote:
> > On Mon, 01 Sep 2014, Arnd Bergmann wrote:
> > 
> > > On Monday 01 September 2014 17:04:26 Lee Jones wrote:
> > > > On Mon, 01 Sep 2014, Arnd Bergmann wrote:
> > > > > Maybe I'm misreading the patch, but I don't see how it creates a
> > > > > migration path. What I want to end up with is infrastructure that
> > > > > lets anybody call syscon_regmap_lookup_by_pdevname or
> > > > > syscon_regmap_lookup_by_compatible (if they really need to)
> > > > > without needing the platform_driver for syscon. That should not
> > > > > require any form of compatibility layer because to the driver
> > > > > using it there is no API change.
> > > > 
> > > > Somehow I think the likelyhood is that I am misreading the patch.
> > > > 
> > > > I thought that before this patch drivers we had to register a syscon
> > > > device to bind to this driver, which was fine for the first use-cases
> > > > of syscon as it wasn't required too early during boot.  However, now
> > > > there are use-cases where systems require access to syscon registers
> > > > eariler in boot we require a means to obtain access prior to device
> > > > probing.  I thought this patch not only provides that possibilty, but
> > > > also leaves in the ability to register direct from DT.
> > > 
> > > Right, it does provide the ability to have syscon before devices
> > > are registered, I missed that part.
> > > 
> > > > > In contrast, this patch introduces a new of_syscon_{un,}register()
> > > > > interface that would get removed after the the above has
> > > > > been implemented, causing extra churn for any driver that also
> > > > > wants to provide a regmap-like interface.
> > > > 
> > > > When will we ever not have to register syscon?
> > > 
> > > The idea is that we implicitly register the syscon block when someone
> > > calls syscon_regmap_lookup_by_compatible or syscon_regmap_lookup_by_phandle
> > > and then return a reference to that new syscon. When another driver
> > > looks up the same device node, we just pass a reference to the existing
> > > syscon.
> > 
> > Doesn't sound too unreasonable.  So how about instead of exporting
> > these new of_syscon_{un,}register() calls, we make them static and
> > call them from syscon_regmap_lookup_by_{phandle,compatible}?
> 
> Yes, that would be a good start. We should think about whether we want
> to remove the existing DT probing at the same time, since it becomes
> unused, and we might want to move the code to drivers/base/regmap_*.c
> at some point

I'd support that move - and can even draft the patch if required.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog