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

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

[Pankaj Dubey]

Currently a syscon entity can only be registered directly through a
platform device that binds to a dedicated syscon driver. However in
certain cases it is required to bind a device with it's dedicated
driver rather than binding with syscon driver.

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 platform driver, and
allows to create syscon objects first time when it is required by
calling of syscon_regmap_lookup_by APIs and keeps a list of such syscon
objects along with syscon provider device_nodes and regmap handles.

Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
V1 of this patchset [1] and related discussion can be found here [1].
 
Changes since v1:
 - Removed of_syscon_unregister function.
 - Modified of_syscon_register function and it will be used by syscon.c 
   to create syscon objects whenever required. 
 - Removed platform device support from syscon.
 - Removed syscon_regmap_lookup_by_pdevname API support.
 - As there are significant changes w.r.t patchset v1, I am taking over
   author for this patchset from Tomasz Figa.
 
Note: Current kernel has clps711x user of syscon_regmap_lookup_by_pdevname and 
will be broken after this patch. As per discussion over here [1], patches 
for making these drivers DT based are ready and once that is done they can use
syscon_regmap_lookup_by_phandle or syscon_regmap_lookup_by_compatible.

[1]: https://lkml.org/lkml/2014/8/22/81


 drivers/mfd/syscon.c       |  143 +++++++++++++-------------------------------
 include/linux/mfd/syscon.h |    1 +
 2 files changed, 44 insertions(+), 100 deletions(-)

diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
index ca15878..8247e93 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -14,41 +14,45 @@
 
 #include <linux/err.h>
 #include <linux/io.h>
-#include <linux/module.h>
+#include <linux/list.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
-#include <linux/of_platform.h>
-#include <linux/platform_data/syscon.h>
-#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;
-}
+static struct syscon *of_syscon_register(struct device_node *np);
 
 struct regmap *syscon_node_to_regmap(struct device_node *np)
 {
-	struct syscon *syscon;
-	struct device *dev;
+	struct syscon *entry, *syscon = NULL;
+
+	spin_lock(&syscon_list_slock);
 
-	dev = driver_find_device(&syscon_driver.driver, NULL, np,
-				 syscon_match_node);
-	if (!dev)
-		return ERR_PTR(-EPROBE_DEFER);
+	list_for_each_entry(entry, &syscon_list, list)
+		if (entry->np == np) {
+			syscon = entry;
+			break;
+		}
 
-	syscon = dev_get_drvdata(dev);
+	spin_unlock(&syscon_list_slock);
 
-	return syscon->regmap;
+	if (!syscon)
+		syscon = of_syscon_register(np);
+
+	if (!IS_ERR(syscon))
+		return syscon->regmap;
+	else
+		return ERR_CAST(syscon);
 }
 EXPORT_SYMBOL_GPL(syscon_node_to_regmap);
 
@@ -68,27 +72,6 @@ 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);
-
-	return syscon->regmap;
-}
-EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
-
 struct regmap *syscon_regmap_lookup_by_phandle(struct device_node *np,
 					const char *property)
 {
@@ -110,81 +93,41 @@ struct regmap *syscon_regmap_lookup_by_phandle(struct device_node *np,
 }
 EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_phandle);
 
-static const struct of_device_id of_syscon_match[] = {
-	{ .compatible = "syscon", },
-	{ },
-};
-
 static struct regmap_config syscon_regmap_config = {
 	.reg_bits = 32,
 	.val_bits = 32,
 	.reg_stride = 4,
 };
 
-static int syscon_probe(struct platform_device *pdev)
+static struct syscon *of_syscon_register(struct device_node *np)
 {
-	struct device *dev = &pdev->dev;
-	struct syscon_platform_data *pdata = dev_get_platdata(dev);
 	struct syscon *syscon;
-	struct resource *res;
+	struct regmap *regmap;
 	void __iomem *base;
 
-	syscon = devm_kzalloc(dev, sizeof(*syscon), GFP_KERNEL);
-	if (!syscon)
-		return -ENOMEM;
+	if (!of_device_is_compatible(np, "syscon"))
+		return ERR_PTR(-EINVAL);
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
-		return -ENOENT;
+	syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
+	if (!syscon)
+		return ERR_PTR(-ENOMEM);
 
-	base = devm_ioremap(dev, res->start, resource_size(res));
+	base = of_iomap(np, 0);
 	if (!base)
-		return -ENOMEM;
-
-	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,
-					&syscon_regmap_config);
-	if (IS_ERR(syscon->regmap)) {
-		dev_err(dev, "regmap init failed\n");
-		return PTR_ERR(syscon->regmap);
-	}
-
-	platform_set_drvdata(pdev, syscon);
+		return ERR_PTR(-ENOMEM);
 
-	dev_dbg(dev, "regmap %pR registered\n", res);
-
-	return 0;
-}
-
-static const struct platform_device_id syscon_ids[] = {
-	{ "syscon", },
-	{ }
-};
+	regmap = regmap_init_mmio(NULL, base, &syscon_regmap_config);
+	if (IS_ERR(regmap)) {
+		pr_err("regmap init failed\n");
+		return ERR_CAST(regmap);
+	}
 
-static struct platform_driver syscon_driver = {
-	.driver = {
-		.name = "syscon",
-		.owner = THIS_MODULE,
-		.of_match_table = of_syscon_match,
-	},
-	.probe		= syscon_probe,
-	.id_table	= syscon_ids,
-};
+	syscon->regmap = regmap;
+	syscon->np = np;
 
-static int __init syscon_init(void)
-{
-	return platform_driver_register(&syscon_driver);
-}
-postcore_initcall(syscon_init);
+	spin_lock(&syscon_list_slock);
+	list_add_tail(&syscon->list, &syscon_list);
+	spin_unlock(&syscon_list_slock);
 
-static void __exit syscon_exit(void)
-{
-	platform_driver_unregister(&syscon_driver);
+	return syscon;
 }
-module_exit(syscon_exit);
-
-MODULE_AUTHOR("Dong Aisheng <dong.aisheng@linaro.org>");
-MODULE_DESCRIPTION("System Control driver");
-MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
index 75e543b..a8e4c4b 100644
--- a/include/linux/mfd/syscon.h
+++ b/include/linux/mfd/syscon.h
@@ -18,6 +18,7 @@
 #include <linux/err.h>
 
 struct device_node;
+struct regmap;
 
 #ifdef CONFIG_MFD_SYSCON
 extern struct regmap *syscon_node_to_regmap(struct device_node *np);
-- 
1.7.9.5

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

[Pankaj Dubey]

Currently a syscon entity can only be registered directly through a
platform device that binds to a dedicated syscon driver. However in
certain cases it is required to bind a device with it's dedicated
driver rather than binding with syscon driver.

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 platform driver, and
allows to create syscon objects first time when it is required by
calling of syscon_regmap_lookup_by APIs and keeps a list of such syscon
objects along with syscon provider device_nodes and regmap handles.

Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
V1 of this patchset [1] and related discussion can be found here [1].
 
Changes since v1:
 - Removed of_syscon_unregister function.
 - Modified of_syscon_register function and it will be used by syscon.c 
   to create syscon objects whenever required. 
 - Removed platform device support from syscon.
 - Removed syscon_regmap_lookup_by_pdevname API support.
 - As there are significant changes w.r.t patchset v1, I am taking over
   author for this patchset from Tomasz Figa.
 
Note: Current kernel has clps711x user of syscon_regmap_lookup_by_pdevname and 
will be broken after this patch. As per discussion over here [1], patches 
for making these drivers DT based are ready and once that is done they can use
syscon_regmap_lookup_by_phandle or syscon_regmap_lookup_by_compatible.

[1]: https://lkml.org/lkml/2014/8/22/81


 drivers/mfd/syscon.c       |  143 +++++++++++++-------------------------------
 include/linux/mfd/syscon.h |    1 +
 2 files changed, 44 insertions(+), 100 deletions(-)

diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
index ca15878..8247e93 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -14,41 +14,45 @@
 
 #include <linux/err.h>
 #include <linux/io.h>
-#include <linux/module.h>
+#include <linux/list.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
-#include <linux/of_platform.h>
-#include <linux/platform_data/syscon.h>
-#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;
-}
+static struct syscon *of_syscon_register(struct device_node *np);
 
 struct regmap *syscon_node_to_regmap(struct device_node *np)
 {
-	struct syscon *syscon;
-	struct device *dev;
+	struct syscon *entry, *syscon = NULL;
+
+	spin_lock(&syscon_list_slock);
 
-	dev = driver_find_device(&syscon_driver.driver, NULL, np,
-				 syscon_match_node);
-	if (!dev)
-		return ERR_PTR(-EPROBE_DEFER);
+	list_for_each_entry(entry, &syscon_list, list)
+		if (entry->np == np) {
+			syscon = entry;
+			break;
+		}
 
-	syscon = dev_get_drvdata(dev);
+	spin_unlock(&syscon_list_slock);
 
-	return syscon->regmap;
+	if (!syscon)
+		syscon = of_syscon_register(np);
+
+	if (!IS_ERR(syscon))
+		return syscon->regmap;
+	else
+		return ERR_CAST(syscon);
 }
 EXPORT_SYMBOL_GPL(syscon_node_to_regmap);
 
@@ -68,27 +72,6 @@ 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);
-
-	return syscon->regmap;
-}
-EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
-
 struct regmap *syscon_regmap_lookup_by_phandle(struct device_node *np,
 					const char *property)
 {
@@ -110,81 +93,41 @@ struct regmap *syscon_regmap_lookup_by_phandle(struct device_node *np,
 }
 EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_phandle);
 
-static const struct of_device_id of_syscon_match[] = {
-	{ .compatible = "syscon", },
-	{ },
-};
-
 static struct regmap_config syscon_regmap_config = {
 	.reg_bits = 32,
 	.val_bits = 32,
 	.reg_stride = 4,
 };
 
-static int syscon_probe(struct platform_device *pdev)
+static struct syscon *of_syscon_register(struct device_node *np)
 {
-	struct device *dev = &pdev->dev;
-	struct syscon_platform_data *pdata = dev_get_platdata(dev);
 	struct syscon *syscon;
-	struct resource *res;
+	struct regmap *regmap;
 	void __iomem *base;
 
-	syscon = devm_kzalloc(dev, sizeof(*syscon), GFP_KERNEL);
-	if (!syscon)
-		return -ENOMEM;
+	if (!of_device_is_compatible(np, "syscon"))
+		return ERR_PTR(-EINVAL);
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
-		return -ENOENT;
+	syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
+	if (!syscon)
+		return ERR_PTR(-ENOMEM);
 
-	base = devm_ioremap(dev, res->start, resource_size(res));
+	base = of_iomap(np, 0);
 	if (!base)
-		return -ENOMEM;
-
-	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,
-					&syscon_regmap_config);
-	if (IS_ERR(syscon->regmap)) {
-		dev_err(dev, "regmap init failed\n");
-		return PTR_ERR(syscon->regmap);
-	}
-
-	platform_set_drvdata(pdev, syscon);
+		return ERR_PTR(-ENOMEM);
 
-	dev_dbg(dev, "regmap %pR registered\n", res);
-
-	return 0;
-}
-
-static const struct platform_device_id syscon_ids[] = {
-	{ "syscon", },
-	{ }
-};
+	regmap = regmap_init_mmio(NULL, base, &syscon_regmap_config);
+	if (IS_ERR(regmap)) {
+		pr_err("regmap init failed\n");
+		return ERR_CAST(regmap);
+	}
 
-static struct platform_driver syscon_driver = {
-	.driver = {
-		.name = "syscon",
-		.owner = THIS_MODULE,
-		.of_match_table = of_syscon_match,
-	},
-	.probe		= syscon_probe,
-	.id_table	= syscon_ids,
-};
+	syscon->regmap = regmap;
+	syscon->np = np;
 
-static int __init syscon_init(void)
-{
-	return platform_driver_register(&syscon_driver);
-}
-postcore_initcall(syscon_init);
+	spin_lock(&syscon_list_slock);
+	list_add_tail(&syscon->list, &syscon_list);
+	spin_unlock(&syscon_list_slock);
 
-static void __exit syscon_exit(void)
-{
-	platform_driver_unregister(&syscon_driver);
+	return syscon;
 }
-module_exit(syscon_exit);
-
-MODULE_AUTHOR("Dong Aisheng <dong.aisheng@linaro.org>");
-MODULE_DESCRIPTION("System Control driver");
-MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
index 75e543b..a8e4c4b 100644
--- a/include/linux/mfd/syscon.h
+++ b/include/linux/mfd/syscon.h
@@ -18,6 +18,7 @@
 #include <linux/err.h>
 
 struct device_node;
+struct regmap;
 
 #ifdef CONFIG_MFD_SYSCON
 extern struct regmap *syscon_node_to_regmap(struct device_node *np);
-- 
1.7.9.5

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

[Tomasz Figa]

On 02.09.2014 16:42, Pankaj Dubey wrote:
> Currently a syscon entity can only be registered directly through a
> platform device that binds to a dedicated syscon driver. However in
> certain cases it is required to bind a device with it's dedicated
> driver rather than binding with syscon driver.
> 
> 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 platform driver, and
> allows to create syscon objects first time when it is required by
> calling of syscon_regmap_lookup_by APIs and keeps a list of such syscon
> objects along with syscon provider device_nodes and regmap handles.
> 
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> ---
> V1 of this patchset [1] and related discussion can be found here [1].
>  
> Changes since v1:
>  - Removed of_syscon_unregister function.
>  - Modified of_syscon_register function and it will be used by syscon.c 
>    to create syscon objects whenever required. 
>  - Removed platform device support from syscon.
>  - Removed syscon_regmap_lookup_by_pdevname API support.
>  - As there are significant changes w.r.t patchset v1, I am taking over
>    author for this patchset from Tomasz Figa.

I guess you should also drop my Signed-off-by too. I think the best
thing would replacing it with my Suggested-by and adding Arnd's
Suggested-by too.

Best regards,
Tomasz

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

[Tomasz Figa]

On 02.09.2014 16:42, Pankaj Dubey wrote:
> Currently a syscon entity can only be registered directly through a
> platform device that binds to a dedicated syscon driver. However in
> certain cases it is required to bind a device with it's dedicated
> driver rather than binding with syscon driver.
> 
> 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 platform driver, and
> allows to create syscon objects first time when it is required by
> calling of syscon_regmap_lookup_by APIs and keeps a list of such syscon
> objects along with syscon provider device_nodes and regmap handles.
> 
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> ---
> V1 of this patchset [1] and related discussion can be found here [1].
>  
> Changes since v1:
>  - Removed of_syscon_unregister function.
>  - Modified of_syscon_register function and it will be used by syscon.c 
>    to create syscon objects whenever required. 
>  - Removed platform device support from syscon.
>  - Removed syscon_regmap_lookup_by_pdevname API support.
>  - As there are significant changes w.r.t patchset v1, I am taking over
>    author for this patchset from Tomasz Figa.

I guess you should also drop my Signed-off-by too. I think the best
thing would replacing it with my Suggested-by and adding Arnd's
Suggested-by too.

Best regards,
Tomasz

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

[Arnd Bergmann]

On Tuesday 02 September 2014 20:12:15 Pankaj Dubey wrote:
> Currently a syscon entity can only be registered directly through a
> platform device that binds to a dedicated syscon driver. However in
> certain cases it is required to bind a device with it's dedicated
> driver rather than binding with syscon driver.
> 
> 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 platform driver, and
> allows to create syscon objects first time when it is required by
> calling of syscon_regmap_lookup_by APIs and keeps a list of such syscon
> objects along with syscon provider device_nodes and regmap handles.
> 
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> ---
> V1 of this patchset [1] and related discussion can be found here [1].
>  
> Changes since v1:
>  - Removed of_syscon_unregister function.
>  - Modified of_syscon_register function and it will be used by syscon.c 
>    to create syscon objects whenever required. 
>  - Removed platform device support from syscon.
>  - Removed syscon_regmap_lookup_by_pdevname API support.
>  - As there are significant changes w.r.t patchset v1, I am taking over
>    author for this patchset from Tomasz Figa.

Note that you got the Signed-off-by: list wrong, you should never have
any people listed as Signed-off-by after your own name, and they should
be listed before your name only when you are forwarding their patches.

> Note: Current kernel has clps711x user of syscon_regmap_lookup_by_pdevname and 
> will be broken after this patch. As per discussion over here [1], patches 
> for making these drivers DT based are ready and once that is done they can use
> syscon_regmap_lookup_by_phandle or syscon_regmap_lookup_by_compatible.
> 
> [1]: https://lkml.org/lkml/2014/8/22/81

Adding Alexander Shiyan to Cc here, so we can make sure this is well
coordinated.

I'm also adding Michal Simek, since here was involved in earlier discussions
about doing this.

>  drivers/mfd/syscon.c       |  143 +++++++++++++-------------------------------
>  include/linux/mfd/syscon.h |    1 +
>  2 files changed, 44 insertions(+), 100 deletions(-)

I certainly like the diffstat ;-)

>  struct syscon {
> +	struct device_node *np;
>  	struct regmap *regmap;
> +	struct list_head list;
>  };

Right

> @@ -68,27 +72,6 @@ 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);
> -
> -	return syscon->regmap;
> -}
> -EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);

I think this can actually be left intact if that helps with clps71xx.
It could be done in a hacky way using bus_find_device_by_name()
to keep it simple, or in a somewhat nicer way by keeping the 
syscon platform_driver around for the non-DT case.


> +	regmap = regmap_init_mmio(NULL, base, &syscon_regmap_config);
> +	if (IS_ERR(regmap)) {
> +		pr_err("regmap init failed\n");
> +		return ERR_CAST(regmap);
> +	}

The last time I looked over this code, I think it was not safe to
call regmap_init_mmio() with a NULL device pointer, and we decided
that should be fixed. Have you checked whether it is ok now?

	Arnd

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

[Arnd Bergmann]

On Tuesday 02 September 2014 20:12:15 Pankaj Dubey wrote:
> Currently a syscon entity can only be registered directly through a
> platform device that binds to a dedicated syscon driver. However in
> certain cases it is required to bind a device with it's dedicated
> driver rather than binding with syscon driver.
> 
> 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 platform driver, and
> allows to create syscon objects first time when it is required by
> calling of syscon_regmap_lookup_by APIs and keeps a list of such syscon
> objects along with syscon provider device_nodes and regmap handles.
> 
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> ---
> V1 of this patchset [1] and related discussion can be found here [1].
>  
> Changes since v1:
>  - Removed of_syscon_unregister function.
>  - Modified of_syscon_register function and it will be used by syscon.c 
>    to create syscon objects whenever required. 
>  - Removed platform device support from syscon.
>  - Removed syscon_regmap_lookup_by_pdevname API support.
>  - As there are significant changes w.r.t patchset v1, I am taking over
>    author for this patchset from Tomasz Figa.

Note that you got the Signed-off-by: list wrong, you should never have
any people listed as Signed-off-by after your own name, and they should
be listed before your name only when you are forwarding their patches.

> Note: Current kernel has clps711x user of syscon_regmap_lookup_by_pdevname and 
> will be broken after this patch. As per discussion over here [1], patches 
> for making these drivers DT based are ready and once that is done they can use
> syscon_regmap_lookup_by_phandle or syscon_regmap_lookup_by_compatible.
> 
> [1]: https://lkml.org/lkml/2014/8/22/81

Adding Alexander Shiyan to Cc here, so we can make sure this is well
coordinated.

I'm also adding Michal Simek, since here was involved in earlier discussions
about doing this.

>  drivers/mfd/syscon.c       |  143 +++++++++++++-------------------------------
>  include/linux/mfd/syscon.h |    1 +
>  2 files changed, 44 insertions(+), 100 deletions(-)

I certainly like the diffstat ;-)

>  struct syscon {
> +	struct device_node *np;
>  	struct regmap *regmap;
> +	struct list_head list;
>  };

Right

> @@ -68,27 +72,6 @@ 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);
> -
> -	return syscon->regmap;
> -}
> -EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);

I think this can actually be left intact if that helps with clps71xx.
It could be done in a hacky way using bus_find_device_by_name()
to keep it simple, or in a somewhat nicer way by keeping the 
syscon platform_driver around for the non-DT case.


> +	regmap = regmap_init_mmio(NULL, base, &syscon_regmap_config);
> +	if (IS_ERR(regmap)) {
> +		pr_err("regmap init failed\n");
> +		return ERR_CAST(regmap);
> +	}

The last time I looked over this code, I think it was not safe to
call regmap_init_mmio() with a NULL device pointer, and we decided
that should be fixed. Have you checked whether it is ok now?

	Arnd

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

[Alexander Shiyan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 3409 bytes --]

Tue, 02 Sep 2014 17:20:03 +0200 от Arnd Bergmann <arnd@arndb.de>:
> On Tuesday 02 September 2014 20:12:15 Pankaj Dubey wrote:
> > Currently a syscon entity can only be registered directly through a
> > platform device that binds to a dedicated syscon driver. However in
> > certain cases it is required to bind a device with it's dedicated
> > driver rather than binding with syscon driver.
> > 
> > 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 platform driver, and
> > allows to create syscon objects first time when it is required by
> > calling of syscon_regmap_lookup_by APIs and keeps a list of such syscon
> > objects along with syscon provider device_nodes and regmap handles.
> > 
> > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> > Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> > ---
> > V1 of this patchset [1] and related discussion can be found here [1].
> >  
> > Changes since v1:
> >  - Removed of_syscon_unregister function.
> >  - Modified of_syscon_register function and it will be used by syscon.c 
> >    to create syscon objects whenever required. 
> >  - Removed platform device support from syscon.
> >  - Removed syscon_regmap_lookup_by_pdevname API support.
> >  - As there are significant changes w.r.t patchset v1, I am taking over
> >    author for this patchset from Tomasz Figa.
> 
> Note that you got the Signed-off-by: list wrong, you should never have
> any people listed as Signed-off-by after your own name, and they should
> be listed before your name only when you are forwarding their patches.
> 
> > Note: Current kernel has clps711x user of syscon_regmap_lookup_by_pdevname and 
> > will be broken after this patch. As per discussion over here [1], patches 
> > for making these drivers DT based are ready and once that is done they can use
> > syscon_regmap_lookup_by_phandle or syscon_regmap_lookup_by_compatible.
> > 
> > [1]: https://lkml.org/lkml/2014/8/22/81
...
> > -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);
> > -
> > -	return syscon->regmap;
> > -}
> > -EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
> 
> I think this can actually be left intact if that helps with clps71xx.
> It could be done in a hacky way using bus_find_device_by_name()
> to keep it simple, or in a somewhat nicer way by keeping the 
> syscon platform_driver around for the non-DT case.

It will not work anyway because the patch involves the use of
of_device_is_compatible(), of_iomap() etc...

---

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

[Alexander Shiyan]

Tue, 02 Sep 2014 17:20:03 +0200 от Arnd Bergmann <arnd@arndb.de>:
> On Tuesday 02 September 2014 20:12:15 Pankaj Dubey wrote:
> > Currently a syscon entity can only be registered directly through a
> > platform device that binds to a dedicated syscon driver. However in
> > certain cases it is required to bind a device with it's dedicated
> > driver rather than binding with syscon driver.
> > 
> > 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 platform driver, and
> > allows to create syscon objects first time when it is required by
> > calling of syscon_regmap_lookup_by APIs and keeps a list of such syscon
> > objects along with syscon provider device_nodes and regmap handles.
> > 
> > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> > Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> > ---
> > V1 of this patchset [1] and related discussion can be found here [1].
> >  
> > Changes since v1:
> >  - Removed of_syscon_unregister function.
> >  - Modified of_syscon_register function and it will be used by syscon.c 
> >    to create syscon objects whenever required. 
> >  - Removed platform device support from syscon.
> >  - Removed syscon_regmap_lookup_by_pdevname API support.
> >  - As there are significant changes w.r.t patchset v1, I am taking over
> >    author for this patchset from Tomasz Figa.
> 
> Note that you got the Signed-off-by: list wrong, you should never have
> any people listed as Signed-off-by after your own name, and they should
> be listed before your name only when you are forwarding their patches.
> 
> > Note: Current kernel has clps711x user of syscon_regmap_lookup_by_pdevname and 
> > will be broken after this patch. As per discussion over here [1], patches 
> > for making these drivers DT based are ready and once that is done they can use
> > syscon_regmap_lookup_by_phandle or syscon_regmap_lookup_by_compatible.
> > 
> > [1]: https://lkml.org/lkml/2014/8/22/81
...
> > -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);
> > -
> > -	return syscon->regmap;
> > -}
> > -EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
> 
> I think this can actually be left intact if that helps with clps71xx.
> It could be done in a hacky way using bus_find_device_by_name()
> to keep it simple, or in a somewhat nicer way by keeping the 
> syscon platform_driver around for the non-DT case.

It will not work anyway because the patch involves the use of
of_device_is_compatible(), of_iomap() etc...

---

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

[Alexander Shiyan]

Tue, 02 Sep 2014 17:20:03 +0200 от Arnd Bergmann <arnd@arndb.de>:
> On Tuesday 02 September 2014 20:12:15 Pankaj Dubey wrote:
> > Currently a syscon entity can only be registered directly through a
> > platform device that binds to a dedicated syscon driver. However in
> > certain cases it is required to bind a device with it's dedicated
> > driver rather than binding with syscon driver.
> > 
> > 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 platform driver, and
> > allows to create syscon objects first time when it is required by
> > calling of syscon_regmap_lookup_by APIs and keeps a list of such syscon
> > objects along with syscon provider device_nodes and regmap handles.
> > 
> > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> > Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> > ---
> > V1 of this patchset [1] and related discussion can be found here [1].
> >  
> > Changes since v1:
> >  - Removed of_syscon_unregister function.
> >  - Modified of_syscon_register function and it will be used by syscon.c 
> >    to create syscon objects whenever required. 
> >  - Removed platform device support from syscon.
> >  - Removed syscon_regmap_lookup_by_pdevname API support.
> >  - As there are significant changes w.r.t patchset v1, I am taking over
> >    author for this patchset from Tomasz Figa.
> 
> Note that you got the Signed-off-by: list wrong, you should never have
> any people listed as Signed-off-by after your own name, and they should
> be listed before your name only when you are forwarding their patches.
> 
> > Note: Current kernel has clps711x user of syscon_regmap_lookup_by_pdevname and 
> > will be broken after this patch. As per discussion over here [1], patches 
> > for making these drivers DT based are ready and once that is done they can use
> > syscon_regmap_lookup_by_phandle or syscon_regmap_lookup_by_compatible.
> > 
> > [1]: https://lkml.org/lkml/2014/8/22/81
...
> > -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);
> > -
> > -	return syscon->regmap;
> > -}
> > -EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
> 
> I think this can actually be left intact if that helps with clps71xx.
> It could be done in a hacky way using bus_find_device_by_name()
> to keep it simple, or in a somewhat nicer way by keeping the 
> syscon platform_driver around for the non-DT case.

It will not work anyway because the patch involves the use of
of_device_is_compatible(), of_iomap() etc...

---

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

[Alexander Shiyan]

Tue, 02 Sep 2014 17:20:03 +0200 ?? Arnd Bergmann <arnd@arndb.de>:
> On Tuesday 02 September 2014 20:12:15 Pankaj Dubey wrote:
> > Currently a syscon entity can only be registered directly through a
> > platform device that binds to a dedicated syscon driver. However in
> > certain cases it is required to bind a device with it's dedicated
> > driver rather than binding with syscon driver.
> > 
> > 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 platform driver, and
> > allows to create syscon objects first time when it is required by
> > calling of syscon_regmap_lookup_by APIs and keeps a list of such syscon
> > objects along with syscon provider device_nodes and regmap handles.
> > 
> > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> > Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> > ---
> > V1 of this patchset [1] and related discussion can be found here [1].
> >  
> > Changes since v1:
> >  - Removed of_syscon_unregister function.
> >  - Modified of_syscon_register function and it will be used by syscon.c 
> >    to create syscon objects whenever required. 
> >  - Removed platform device support from syscon.
> >  - Removed syscon_regmap_lookup_by_pdevname API support.
> >  - As there are significant changes w.r.t patchset v1, I am taking over
> >    author for this patchset from Tomasz Figa.
> 
> Note that you got the Signed-off-by: list wrong, you should never have
> any people listed as Signed-off-by after your own name, and they should
> be listed before your name only when you are forwarding their patches.
> 
> > Note: Current kernel has clps711x user of syscon_regmap_lookup_by_pdevname and 
> > will be broken after this patch. As per discussion over here [1], patches 
> > for making these drivers DT based are ready and once that is done they can use
> > syscon_regmap_lookup_by_phandle or syscon_regmap_lookup_by_compatible.
> > 
> > [1]: https://lkml.org/lkml/2014/8/22/81
...
> > -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);
> > -
> > -	return syscon->regmap;
> > -}
> > -EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
> 
> I think this can actually be left intact if that helps with clps71xx.
> It could be done in a hacky way using bus_find_device_by_name()
> to keep it simple, or in a somewhat nicer way by keeping the 
> syscon platform_driver around for the non-DT case.

It will not work anyway because the patch involves the use of
of_device_is_compatible(), of_iomap() etc...

---

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

[Arnd Bergmann]

On Tuesday 02 September 2014 19:42:52 Alexander Shiyan wrote:
> > > -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);
> > > -
> > > -   return syscon->regmap;
> > > -}
> > > -EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
> > 
> > I think this can actually be left intact if that helps with clps71xx.
> > It could be done in a hacky way using bus_find_device_by_name()
> > to keep it simple, or in a somewhat nicer way by keeping the 
> > syscon platform_driver around for the non-DT case.
> 
> It will not work anyway because the patch involves the use of
> of_device_is_compatible(), of_iomap() etc...

What I meant was to have the pdevname stuff keep working the way
it does today. At that point, you essentially have two completely
independent drivers, one that registers a platform driver and
provides syscon_regmap_lookup_by_pdevname, and one that provides
the other interfaces using DT lookup.

	Arnd

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

[Arnd Bergmann]

On Tuesday 02 September 2014 19:42:52 Alexander Shiyan wrote:
> > > -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);
> > > -
> > > -   return syscon->regmap;
> > > -}
> > > -EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
> > 
> > I think this can actually be left intact if that helps with clps71xx.
> > It could be done in a hacky way using bus_find_device_by_name()
> > to keep it simple, or in a somewhat nicer way by keeping the 
> > syscon platform_driver around for the non-DT case.
> 
> It will not work anyway because the patch involves the use of
> of_device_is_compatible(), of_iomap() etc...

What I meant was to have the pdevname stuff keep working the way
it does today. At that point, you essentially have two completely
independent drivers, one that registers a platform driver and
provides syscon_regmap_lookup_by_pdevname, and one that provides
the other interfaces using DT lookup.

	Arnd

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

[Vivek Gautam]

On Tue, Sep 2, 2014 at 8:12 PM, Pankaj Dubey <pankaj.dubey@samsung.com> wrote:
> Currently a syscon entity can only be registered directly through a
> platform device that binds to a dedicated syscon driver. However in
> certain cases it is required to bind a device with it's dedicated
> driver rather than binding with syscon driver.
>
> 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 platform driver, and
> allows to create syscon objects first time when it is required by
> calling of syscon_regmap_lookup_by APIs and keeps a list of such syscon
> objects along with syscon provider device_nodes and regmap handles.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> ---
> V1 of this patchset [1] and related discussion can be found here [1].
>
> Changes since v1:
>  - Removed of_syscon_unregister function.
>  - Modified of_syscon_register function and it will be used by syscon.c
>    to create syscon objects whenever required.
>  - Removed platform device support from syscon.
>  - Removed syscon_regmap_lookup_by_pdevname API support.
>  - As there are significant changes w.r.t patchset v1, I am taking over
>    author for this patchset from Tomasz Figa.
>
> Note: Current kernel has clps711x user of syscon_regmap_lookup_by_pdevname and
> will be broken after this patch. As per discussion over here [1], patches
> for making these drivers DT based are ready and once that is done they can use
> syscon_regmap_lookup_by_phandle or syscon_regmap_lookup_by_compatible.
>
> [1]: https://lkml.org/lkml/2014/8/22/81

[snip]

Tested on smdk5250 board with 3.17-rc3 for USB 2.0 and USB 3.0, since USB takes
syscon phandle for handling some of the pmu registers.
Things work fine with all the dt side changes in this patch.

Tested-by: Vivek Gautam <gautam.vivek@samsung.com>



-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

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

[Vivek Gautam]

On Tue, Sep 2, 2014 at 8:12 PM, Pankaj Dubey <pankaj.dubey@samsung.com> wrote:
> Currently a syscon entity can only be registered directly through a
> platform device that binds to a dedicated syscon driver. However in
> certain cases it is required to bind a device with it's dedicated
> driver rather than binding with syscon driver.
>
> 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 platform driver, and
> allows to create syscon objects first time when it is required by
> calling of syscon_regmap_lookup_by APIs and keeps a list of such syscon
> objects along with syscon provider device_nodes and regmap handles.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> ---
> V1 of this patchset [1] and related discussion can be found here [1].
>
> Changes since v1:
>  - Removed of_syscon_unregister function.
>  - Modified of_syscon_register function and it will be used by syscon.c
>    to create syscon objects whenever required.
>  - Removed platform device support from syscon.
>  - Removed syscon_regmap_lookup_by_pdevname API support.
>  - As there are significant changes w.r.t patchset v1, I am taking over
>    author for this patchset from Tomasz Figa.
>
> Note: Current kernel has clps711x user of syscon_regmap_lookup_by_pdevname and
> will be broken after this patch. As per discussion over here [1], patches
> for making these drivers DT based are ready and once that is done they can use
> syscon_regmap_lookup_by_phandle or syscon_regmap_lookup_by_compatible.
>
> [1]: https://lkml.org/lkml/2014/8/22/81

[snip]

Tested on smdk5250 board with 3.17-rc3 for USB 2.0 and USB 3.0, since USB takes
syscon phandle for handling some of the pmu registers.
Things work fine with all the dt side changes in this patch.

Tested-by: Vivek Gautam <gautam.vivek@samsung.com>



-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

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

[Vivek Gautam]

On Tue, Sep 2, 2014 at 8:12 PM, Pankaj Dubey <pankaj.dubey@samsung.com> wrote:
> Currently a syscon entity can only be registered directly through a
> platform device that binds to a dedicated syscon driver. However in
> certain cases it is required to bind a device with it's dedicated
> driver rather than binding with syscon driver.
>
> 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 platform driver, and
> allows to create syscon objects first time when it is required by
> calling of syscon_regmap_lookup_by APIs and keeps a list of such syscon
> objects along with syscon provider device_nodes and regmap handles.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> ---
> V1 of this patchset [1] and related discussion can be found here [1].
>
> Changes since v1:
>  - Removed of_syscon_unregister function.
>  - Modified of_syscon_register function and it will be used by syscon.c
>    to create syscon objects whenever required.
>  - Removed platform device support from syscon.
>  - Removed syscon_regmap_lookup_by_pdevname API support.
>  - As there are significant changes w.r.t patchset v1, I am taking over
>    author for this patchset from Tomasz Figa.
>
> Note: Current kernel has clps711x user of syscon_regmap_lookup_by_pdevname and
> will be broken after this patch. As per discussion over here [1], patches
> for making these drivers DT based are ready and once that is done they can use
> syscon_regmap_lookup_by_phandle or syscon_regmap_lookup_by_compatible.
>
> [1]: https://lkml.org/lkml/2014/8/22/81

[snip]

Tested on smdk5250 board with 3.17-rc3 for USB 2.0 and USB 3.0, since USB takes
syscon phandle for handling some of the pmu registers.
Things work fine with all the dt side changes in this patch.

Tested-by: Vivek Gautam <gautam.vivek@samsung.com>



-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

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

[Boris BREZILLON]

Hi,

I'm joining the thread cause I'm really interested in having a syscon
dev available during early init (I need it to share at91 PMC
registers among several subsystems, one of this subsystem being the CCF
which is called during early ARM boot process to register system
clocks).

On Tue, 02 Sep 2014 17:20:03 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Tuesday 02 September 2014 20:12:15 Pankaj Dubey wrote:
> > Currently a syscon entity can only be registered directly through a
> > platform device that binds to a dedicated syscon driver. However in
> > certain cases it is required to bind a device with it's dedicated
> > driver rather than binding with syscon driver.
> > 
> > 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 platform driver, and
> > allows to create syscon objects first time when it is required by
> > calling of syscon_regmap_lookup_by APIs and keeps a list of such syscon
> > objects along with syscon provider device_nodes and regmap handles.
> > 
> > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> > Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> > ---
> > V1 of this patchset [1] and related discussion can be found here [1].
> >  
> > Changes since v1:
> >  - Removed of_syscon_unregister function.
> >  - Modified of_syscon_register function and it will be used by syscon.c 
> >    to create syscon objects whenever required. 
> >  - Removed platform device support from syscon.
> >  - Removed syscon_regmap_lookup_by_pdevname API support.
> >  - As there are significant changes w.r.t patchset v1, I am taking over
> >    author for this patchset from Tomasz Figa.
> 
> Note that you got the Signed-off-by: list wrong, you should never have
> any people listed as Signed-off-by after your own name, and they should
> be listed before your name only when you are forwarding their patches.
> 
> > Note: Current kernel has clps711x user of syscon_regmap_lookup_by_pdevname and 
> > will be broken after this patch. As per discussion over here [1], patches 
> > for making these drivers DT based are ready and once that is done they can use
> > syscon_regmap_lookup_by_phandle or syscon_regmap_lookup_by_compatible.
> > 
> > [1]: https://lkml.org/lkml/2014/8/22/81
> 
> Adding Alexander Shiyan to Cc here, so we can make sure this is well
> coordinated.
> 
> I'm also adding Michal Simek, since here was involved in earlier discussions
> about doing this.
> 
> >  drivers/mfd/syscon.c       |  143 +++++++++++++-------------------------------
> >  include/linux/mfd/syscon.h |    1 +
> >  2 files changed, 44 insertions(+), 100 deletions(-)
> 
> I certainly like the diffstat ;-)
> 
> >  struct syscon {
> > +	struct device_node *np;
> >  	struct regmap *regmap;
> > +	struct list_head list;
> >  };
> 
> Right
> 
> > @@ -68,27 +72,6 @@ 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);
> > -
> > -	return syscon->regmap;
> > -}
> > -EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
> 
> I think this can actually be left intact if that helps with clps71xx.
> It could be done in a hacky way using bus_find_device_by_name()
> to keep it simple, or in a somewhat nicer way by keeping the 
> syscon platform_driver around for the non-DT case.
> 
> 
> > +	regmap = regmap_init_mmio(NULL, base, &syscon_regmap_config);
> > +	if (IS_ERR(regmap)) {
> > +		pr_err("regmap init failed\n");
> > +		return ERR_CAST(regmap);
> > +	}
> 
> The last time I looked over this code, I think it was not safe to
> call regmap_init_mmio() with a NULL device pointer, and we decided
> that should be fixed. Have you checked whether it is ok now?

I checked that part, and it appears most of the code is already there
(see usage of regmap_attach_dev function here [1]).

The only problem I see is that errors are still printed with dev_err,
which, AFAIK, will trigger a kernel panic if dev is NULL.
This is an issue in both the regmap_init function itself and the
regcache_init function (which is called by regmap_init).

BTW, maybe we should remove this line [2], as this is now part of the
regmap_attach_dev function [3] which is called at the end of
regmap_init if dev is not NULL [4].

Adding Mark in Cc.

Best Regards,

Boris

[1]http://lxr.free-electrons.com/source/drivers/base/regmap/regmap.c#L826
[2]http://lxr.free-electrons.com/source/drivers/base/regmap/regmap.c#L511
[3]http://lxr.free-electrons.com/source/drivers/base/regmap/regmap.c#L434
[4]http://lxr.free-electrons.com/source/drivers/base/regmap/regmap.c#L826


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

[Boris BREZILLON]

Hi,

I'm joining the thread cause I'm really interested in having a syscon
dev available during early init (I need it to share at91 PMC
registers among several subsystems, one of this subsystem being the CCF
which is called during early ARM boot process to register system
clocks).

On Tue, 02 Sep 2014 17:20:03 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Tuesday 02 September 2014 20:12:15 Pankaj Dubey wrote:
> > Currently a syscon entity can only be registered directly through a
> > platform device that binds to a dedicated syscon driver. However in
> > certain cases it is required to bind a device with it's dedicated
> > driver rather than binding with syscon driver.
> > 
> > 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 platform driver, and
> > allows to create syscon objects first time when it is required by
> > calling of syscon_regmap_lookup_by APIs and keeps a list of such syscon
> > objects along with syscon provider device_nodes and regmap handles.
> > 
> > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> > Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> > ---
> > V1 of this patchset [1] and related discussion can be found here [1].
> >  
> > Changes since v1:
> >  - Removed of_syscon_unregister function.
> >  - Modified of_syscon_register function and it will be used by syscon.c 
> >    to create syscon objects whenever required. 
> >  - Removed platform device support from syscon.
> >  - Removed syscon_regmap_lookup_by_pdevname API support.
> >  - As there are significant changes w.r.t patchset v1, I am taking over
> >    author for this patchset from Tomasz Figa.
> 
> Note that you got the Signed-off-by: list wrong, you should never have
> any people listed as Signed-off-by after your own name, and they should
> be listed before your name only when you are forwarding their patches.
> 
> > Note: Current kernel has clps711x user of syscon_regmap_lookup_by_pdevname and 
> > will be broken after this patch. As per discussion over here [1], patches 
> > for making these drivers DT based are ready and once that is done they can use
> > syscon_regmap_lookup_by_phandle or syscon_regmap_lookup_by_compatible.
> > 
> > [1]: https://lkml.org/lkml/2014/8/22/81
> 
> Adding Alexander Shiyan to Cc here, so we can make sure this is well
> coordinated.
> 
> I'm also adding Michal Simek, since here was involved in earlier discussions
> about doing this.
> 
> >  drivers/mfd/syscon.c       |  143 +++++++++++++-------------------------------
> >  include/linux/mfd/syscon.h |    1 +
> >  2 files changed, 44 insertions(+), 100 deletions(-)
> 
> I certainly like the diffstat ;-)
> 
> >  struct syscon {
> > +	struct device_node *np;
> >  	struct regmap *regmap;
> > +	struct list_head list;
> >  };
> 
> Right
> 
> > @@ -68,27 +72,6 @@ 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);
> > -
> > -	return syscon->regmap;
> > -}
> > -EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
> 
> I think this can actually be left intact if that helps with clps71xx.
> It could be done in a hacky way using bus_find_device_by_name()
> to keep it simple, or in a somewhat nicer way by keeping the 
> syscon platform_driver around for the non-DT case.
> 
> 
> > +	regmap = regmap_init_mmio(NULL, base, &syscon_regmap_config);
> > +	if (IS_ERR(regmap)) {
> > +		pr_err("regmap init failed\n");
> > +		return ERR_CAST(regmap);
> > +	}
> 
> The last time I looked over this code, I think it was not safe to
> call regmap_init_mmio() with a NULL device pointer, and we decided
> that should be fixed. Have you checked whether it is ok now?

I checked that part, and it appears most of the code is already there
(see usage of regmap_attach_dev function here [1]).

The only problem I see is that errors are still printed with dev_err,
which, AFAIK, will trigger a kernel panic if dev is NULL.
This is an issue in both the regmap_init function itself and the
regcache_init function (which is called by regmap_init).

BTW, maybe we should remove this line [2], as this is now part of the
regmap_attach_dev function [3] which is called at the end of
regmap_init if dev is not NULL [4].

Adding Mark in Cc.

Best Regards,

Boris

[1]http://lxr.free-electrons.com/source/drivers/base/regmap/regmap.c#L826
[2]http://lxr.free-electrons.com/source/drivers/base/regmap/regmap.c#L511
[3]http://lxr.free-electrons.com/source/drivers/base/regmap/regmap.c#L434
[4]http://lxr.free-electrons.com/source/drivers/base/regmap/regmap.c#L826


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

[Arnd Bergmann]

On Wednesday 03 September 2014 15:16:11 Boris BREZILLON wrote:
> I checked that part, and it appears most of the code is already there
> (see usage of regmap_attach_dev function here [1]).
> 
> The only problem I see is that errors are still printed with dev_err,
> which, AFAIK, will trigger a kernel panic if dev is NULL.

Actually not:

static int __dev_printk(const char *level, const struct device *dev,
                        struct va_format *vaf)
{
        if (!dev)
                return printk("%s(NULL device *): %pV", level, vaf);

        return dev_printk_emit(level[1] - '0', dev,
                               "%s %s: %pV",
                               dev_driver_string(dev), dev_name(dev), vaf);
}


	Arnd

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

[Arnd Bergmann]

On Wednesday 03 September 2014 15:16:11 Boris BREZILLON wrote:
> I checked that part, and it appears most of the code is already there
> (see usage of regmap_attach_dev function here [1]).
> 
> The only problem I see is that errors are still printed with dev_err,
> which, AFAIK, will trigger a kernel panic if dev is NULL.

Actually not:

static int __dev_printk(const char *level, const struct device *dev,
                        struct va_format *vaf)
{
        if (!dev)
                return printk("%s(NULL device *): %pV", level, vaf);

        return dev_printk_emit(level[1] - '0', dev,
                               "%s %s: %pV",
                               dev_driver_string(dev), dev_name(dev), vaf);
}


	Arnd

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

[Boris BREZILLON]

On Wed, 03 Sep 2014 15:49:04 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Wednesday 03 September 2014 15:16:11 Boris BREZILLON wrote:
> > I checked that part, and it appears most of the code is already there
> > (see usage of regmap_attach_dev function here [1]).
> > 
> > The only problem I see is that errors are still printed with dev_err,
> > which, AFAIK, will trigger a kernel panic if dev is NULL.
> 
> Actually not:
> 
> static int __dev_printk(const char *level, const struct device *dev,
>                         struct va_format *vaf)
> {
>         if (!dev)
>                 return printk("%s(NULL device *): %pV", level, vaf);
> 
>         return dev_printk_emit(level[1] - '0', dev,
>                                "%s %s: %pV",
>                                dev_driver_string(dev), dev_name(dev), vaf);
> }
> 

My bad then (I don't know where I looked at to think NULL dev was not
gracefully handled :-)). Thanks for pointing this out.
Given that, I think it should work fine even with a NULL dev.
I'll give it a try on at91 ;-).

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

[Boris BREZILLON]

On Wed, 03 Sep 2014 15:49:04 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Wednesday 03 September 2014 15:16:11 Boris BREZILLON wrote:
> > I checked that part, and it appears most of the code is already there
> > (see usage of regmap_attach_dev function here [1]).
> > 
> > The only problem I see is that errors are still printed with dev_err,
> > which, AFAIK, will trigger a kernel panic if dev is NULL.
> 
> Actually not:
> 
> static int __dev_printk(const char *level, const struct device *dev,
>                         struct va_format *vaf)
> {
>         if (!dev)
>                 return printk("%s(NULL device *): %pV", level, vaf);
> 
>         return dev_printk_emit(level[1] - '0', dev,
>                                "%s %s: %pV",
>                                dev_driver_string(dev), dev_name(dev), vaf);
> }
> 

My bad then (I don't know where I looked at to think NULL dev was not
gracefully handled :-)). Thanks for pointing this out.
Given that, I think it should work fine even with a NULL dev.
I'll give it a try on at91 ;-).

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

[Pankaj Dubey]

Hi Arnd,

On Tuesday, September 02, 2014 Arnd Bergmann wrote,
> To: Pankaj Dubey
> Cc: linux-arm-kernel@lists.infradead.org;
linux-samsung-soc@vger.kernel.org; linux-
> kernel@vger.kernel.org; lee.jones@linaro.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@gmail.com; Tomasz Figa; Alexander Shiyan; Michal Simek
> Subject: Re: [PATCH v2] mfd: syscon: Decouple syscon interface from
platform
> devices
> 
> On Tuesday 02 September 2014 20:12:15 Pankaj Dubey wrote:
> > Currently a syscon entity can only be registered directly through a
> > platform device that binds to a dedicated syscon driver. However in
> > certain cases it is required to bind a device with it's dedicated
> > driver rather than binding with syscon driver.
> >
> > 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 platform driver, and
> > allows to create syscon objects first time when it is required by
> > calling of syscon_regmap_lookup_by APIs and keeps a list of such
> > syscon objects along with syscon provider device_nodes and regmap
handles.
> >
> > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> > Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> > ---
> > V1 of this patchset [1] and related discussion can be found here [1].
> >
> > Changes since v1:
> >  - Removed of_syscon_unregister function.
> >  - Modified of_syscon_register function and it will be used by syscon.c
> >    to create syscon objects whenever required.
> >  - Removed platform device support from syscon.
> >  - Removed syscon_regmap_lookup_by_pdevname API support.
> >  - As there are significant changes w.r.t patchset v1, I am taking over
> >    author for this patchset from Tomasz Figa.
> 
> Note that you got the Signed-off-by: list wrong, you should never have any
people
> listed as Signed-off-by after your own name, and they should be listed
before your
> name only when you are forwarding their patches.
> 

Sorry, I was not aware of this. I will take care in future.

> > Note: Current kernel has clps711x user of
> > syscon_regmap_lookup_by_pdevname and will be broken after this patch.
> > As per discussion over here [1], patches for making these drivers DT
> > based are ready and once that is done they can use
> syscon_regmap_lookup_by_phandle or syscon_regmap_lookup_by_compatible.
> >
> > [1]: https://lkml.org/lkml/2014/8/22/81
> 
> Adding Alexander Shiyan to Cc here, so we can make sure this is well
coordinated.
> 
> I'm also adding Michal Simek, since here was involved in earlier
discussions about
> doing this.
> 

Thanks.

> >  drivers/mfd/syscon.c       |  143
+++++++++++++-------------------------------
> >  include/linux/mfd/syscon.h |    1 +
> >  2 files changed, 44 insertions(+), 100 deletions(-)
> 
> I certainly like the diffstat ;-)
> 
> >  struct syscon {
> > +	struct device_node *np;
> >  	struct regmap *regmap;
> > +	struct list_head list;
> >  };
> 
> Right
> 
> > @@ -68,27 +72,6 @@ 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);
> > -
> > -	return syscon->regmap;
> > -}
> > -EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
> 
> I think this can actually be left intact if that helps with clps71xx.
> It could be done in a hacky way using bus_find_device_by_name() to keep it
simple,
> or in a somewhat nicer way by keeping the syscon platform_driver around
for the
> non-DT case.
> 

Ok as per our last discussion you mentioned that clps71xx will be soon
migrating to DT.
So if that is not going to happen sooner, I would also prefer better keep
syscon_regmap_lookup_by_pdevname and syscon platform_driver for non-DT case,
so that this issue should not block this patch.

So please let's make final call to keep syscon platform_driver for non-DT
case which eventually
can be dropped once clps71xx driver migrates to DT based. So that I can
prepare next patchset
keeping syscon platform_driver support  and syscon_regmap_lookup_by_pdevname
API support
for non-DT case and send across for review. 


> 
> > +	regmap = regmap_init_mmio(NULL, base, &syscon_regmap_config);
> > +	if (IS_ERR(regmap)) {
> > +		pr_err("regmap init failed\n");
> > +		return ERR_CAST(regmap);
> > +	}
> 
> The last time I looked over this code, I think it was not safe to
> call regmap_init_mmio() with a NULL device pointer, and we decided
> that should be fixed. Have you checked whether it is ok now?
> 

At least we could not see any issues with this. We have tested it one
Exynos5250 board
where syscon object is getting created first time when other drivers (USB,
SATA Phy, Watchdog)
are calling helper function syscon_regmap_lookup_by_phandle and able to
access regmap handle
and able to use regmap_read/write APIs.

> 	Arnd

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

[Pankaj Dubey]

Hi Arnd,

On Tuesday, September 02, 2014 Arnd Bergmann wrote,
> To: Pankaj Dubey
> Cc: linux-arm-kernel at lists.infradead.org;
linux-samsung-soc at vger.kernel.org; linux-
> kernel at vger.kernel.org; lee.jones at linaro.org; kgene.kim at samsung.com;
> linux at arm.linux.org.uk; vikas.sajjan at samsung.com; joshi at samsung.com;
> naushad at samsung.com; thomas.ab at samsung.com; chow.kim at samsung.com;
> tomasz.figa at gmail.com; Tomasz Figa; Alexander Shiyan; Michal Simek
> Subject: Re: [PATCH v2] mfd: syscon: Decouple syscon interface from
platform
> devices
> 
> On Tuesday 02 September 2014 20:12:15 Pankaj Dubey wrote:
> > Currently a syscon entity can only be registered directly through a
> > platform device that binds to a dedicated syscon driver. However in
> > certain cases it is required to bind a device with it's dedicated
> > driver rather than binding with syscon driver.
> >
> > 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 platform driver, and
> > allows to create syscon objects first time when it is required by
> > calling of syscon_regmap_lookup_by APIs and keeps a list of such
> > syscon objects along with syscon provider device_nodes and regmap
handles.
> >
> > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> > Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> > ---
> > V1 of this patchset [1] and related discussion can be found here [1].
> >
> > Changes since v1:
> >  - Removed of_syscon_unregister function.
> >  - Modified of_syscon_register function and it will be used by syscon.c
> >    to create syscon objects whenever required.
> >  - Removed platform device support from syscon.
> >  - Removed syscon_regmap_lookup_by_pdevname API support.
> >  - As there are significant changes w.r.t patchset v1, I am taking over
> >    author for this patchset from Tomasz Figa.
> 
> Note that you got the Signed-off-by: list wrong, you should never have any
people
> listed as Signed-off-by after your own name, and they should be listed
before your
> name only when you are forwarding their patches.
> 

Sorry, I was not aware of this. I will take care in future.

> > Note: Current kernel has clps711x user of
> > syscon_regmap_lookup_by_pdevname and will be broken after this patch.
> > As per discussion over here [1], patches for making these drivers DT
> > based are ready and once that is done they can use
> syscon_regmap_lookup_by_phandle or syscon_regmap_lookup_by_compatible.
> >
> > [1]: https://lkml.org/lkml/2014/8/22/81
> 
> Adding Alexander Shiyan to Cc here, so we can make sure this is well
coordinated.
> 
> I'm also adding Michal Simek, since here was involved in earlier
discussions about
> doing this.
> 

Thanks.

> >  drivers/mfd/syscon.c       |  143
+++++++++++++-------------------------------
> >  include/linux/mfd/syscon.h |    1 +
> >  2 files changed, 44 insertions(+), 100 deletions(-)
> 
> I certainly like the diffstat ;-)
> 
> >  struct syscon {
> > +	struct device_node *np;
> >  	struct regmap *regmap;
> > +	struct list_head list;
> >  };
> 
> Right
> 
> > @@ -68,27 +72,6 @@ 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);
> > -
> > -	return syscon->regmap;
> > -}
> > -EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
> 
> I think this can actually be left intact if that helps with clps71xx.
> It could be done in a hacky way using bus_find_device_by_name() to keep it
simple,
> or in a somewhat nicer way by keeping the syscon platform_driver around
for the
> non-DT case.
> 

Ok as per our last discussion you mentioned that clps71xx will be soon
migrating to DT.
So if that is not going to happen sooner, I would also prefer better keep
syscon_regmap_lookup_by_pdevname and syscon platform_driver for non-DT case,
so that this issue should not block this patch.

So please let's make final call to keep syscon platform_driver for non-DT
case which eventually
can be dropped once clps71xx driver migrates to DT based. So that I can
prepare next patchset
keeping syscon platform_driver support  and syscon_regmap_lookup_by_pdevname
API support
for non-DT case and send across for review. 


> 
> > +	regmap = regmap_init_mmio(NULL, base, &syscon_regmap_config);
> > +	if (IS_ERR(regmap)) {
> > +		pr_err("regmap init failed\n");
> > +		return ERR_CAST(regmap);
> > +	}
> 
> The last time I looked over this code, I think it was not safe to
> call regmap_init_mmio() with a NULL device pointer, and we decided
> that should be fixed. Have you checked whether it is ok now?
> 

At least we could not see any issues with this. We have tested it one
Exynos5250 board
where syscon object is getting created first time when other drivers (USB,
SATA Phy, Watchdog)
are calling helper function syscon_regmap_lookup_by_phandle and able to
access regmap handle
and able to use regmap_read/write APIs.

> 	Arnd

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

[Pankaj Dubey]

Hi Boris,

On Wednesday, September 03, 2014 Boris BREZILLON wrote,
> To: Arnd Bergmann
> Cc: Pankaj Dubey; kgene.kim@samsung.com; linux@arm.linux.org.uk; Alexander
> Shiyan; naushad@samsung.com; Tomasz Figa; linux-kernel@vger.kernel.org;
> joshi@samsung.com; linux-samsung-soc@vger.kernel.org;
thomas.ab@samsung.com;
> tomasz.figa@gmail.com; vikas.sajjan@samsung.com; chow.kim@samsung.com;
> lee.jones@linaro.org; Michal Simek; linux-arm-kernel@lists.infradead.org;
Mark
> Brown
> Subject: Re: [PATCH v2] mfd: syscon: Decouple syscon interface from
platform
> devices
> 
> On Wed, 03 Sep 2014 15:49:04 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:
> 
> > On Wednesday 03 September 2014 15:16:11 Boris BREZILLON wrote:
> > > I checked that part, and it appears most of the code is already
> > > there (see usage of regmap_attach_dev function here [1]).
> > >
> > > The only problem I see is that errors are still printed with
> > > dev_err, which, AFAIK, will trigger a kernel panic if dev is NULL.
> >
> > Actually not:
> >
> > static int __dev_printk(const char *level, const struct device *dev,
> >                         struct va_format *vaf) {
> >         if (!dev)
> >                 return printk("%s(NULL device *): %pV", level, vaf);
> >
> >         return dev_printk_emit(level[1] - '0', dev,
> >                                "%s %s: %pV",
> >                                dev_driver_string(dev), dev_name(dev),
> > vaf); }
> >
> 
> My bad then (I don't know where I looked at to think NULL dev was not
gracefully
> handled :-)). Thanks for pointing this out.
> Given that, I think it should work fine even with a NULL dev.
> I'll give it a try on at91 ;-).
> 

We have tested this patch, on Exynos board and found working well.
In our use case DT based drivers such as USB Phy, SATA Phy, Watchdog are
calling 
syscon_regmap_lookup_by APIs to get regmap handle to Exynos PMU and it
worked 
well for these drivers. 

It would be great if after testing you share result here or give a
Tested-By.

Thanks,
Pankaj Dubey 
> Best Regards,
> 
> Boris
> 
> --
> Boris Brezillon, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

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

[Pankaj Dubey]

Hi Boris,

On Wednesday, September 03, 2014 Boris BREZILLON wrote,
> To: Arnd Bergmann
> Cc: Pankaj Dubey; kgene.kim at samsung.com; linux at arm.linux.org.uk; Alexander
> Shiyan; naushad at samsung.com; Tomasz Figa; linux-kernel at vger.kernel.org;
> joshi at samsung.com; linux-samsung-soc at vger.kernel.org;
thomas.ab at samsung.com;
> tomasz.figa at gmail.com; vikas.sajjan at samsung.com; chow.kim at samsung.com;
> lee.jones at linaro.org; Michal Simek; linux-arm-kernel at lists.infradead.org;
Mark
> Brown
> Subject: Re: [PATCH v2] mfd: syscon: Decouple syscon interface from
platform
> devices
> 
> On Wed, 03 Sep 2014 15:49:04 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:
> 
> > On Wednesday 03 September 2014 15:16:11 Boris BREZILLON wrote:
> > > I checked that part, and it appears most of the code is already
> > > there (see usage of regmap_attach_dev function here [1]).
> > >
> > > The only problem I see is that errors are still printed with
> > > dev_err, which, AFAIK, will trigger a kernel panic if dev is NULL.
> >
> > Actually not:
> >
> > static int __dev_printk(const char *level, const struct device *dev,
> >                         struct va_format *vaf) {
> >         if (!dev)
> >                 return printk("%s(NULL device *): %pV", level, vaf);
> >
> >         return dev_printk_emit(level[1] - '0', dev,
> >                                "%s %s: %pV",
> >                                dev_driver_string(dev), dev_name(dev),
> > vaf); }
> >
> 
> My bad then (I don't know where I looked at to think NULL dev was not
gracefully
> handled :-)). Thanks for pointing this out.
> Given that, I think it should work fine even with a NULL dev.
> I'll give it a try on at91 ;-).
> 

We have tested this patch, on Exynos board and found working well.
In our use case DT based drivers such as USB Phy, SATA Phy, Watchdog are
calling 
syscon_regmap_lookup_by APIs to get regmap handle to Exynos PMU and it
worked 
well for these drivers. 

It would be great if after testing you share result here or give a
Tested-By.

Thanks,
Pankaj Dubey 
> Best Regards,
> 
> Boris
> 
> --
> Boris Brezillon, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

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

[Alexander Shiyan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 2441 bytes --]

Thu, 04 Sep 2014 10:09:19 +0530 от Pankaj Dubey <pankaj.dubey@samsung.com>:
> Hi Arnd,
> 
> On Tuesday, September 02, 2014 Arnd Bergmann wrote,
> > To: Pankaj Dubey
> > Cc: linux-arm-kernel@lists.infradead.org;
> linux-samsung-soc@vger.kernel.org; linux-
> > kernel@vger.kernel.org; lee.jones@linaro.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@gmail.com; Tomasz Figa; Alexander Shiyan; Michal Simek
> > Subject: Re: [PATCH v2] mfd: syscon: Decouple syscon interface from
> platform
> > devices
...
> > > -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);
> > > -
> > > -	return syscon->regmap;
> > > -}
> > > -EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
> > 
> > I think this can actually be left intact if that helps with clps71xx.
> > It could be done in a hacky way using bus_find_device_by_name() to keep it
> simple,
> > or in a somewhat nicer way by keeping the syscon platform_driver around
> for the
> > non-DT case.
> > 
> 
> Ok as per our last discussion you mentioned that clps71xx will be soon
> migrating to DT.
> So if that is not going to happen sooner, I would also prefer better keep
> syscon_regmap_lookup_by_pdevname and syscon platform_driver for non-DT case,
> so that this issue should not block this patch.
> 
> So please let's make final call to keep syscon platform_driver for non-DT
> case which eventually
> can be dropped once clps71xx driver migrates to DT based. So that I can
> prepare next patchset
> keeping syscon platform_driver support  and syscon_regmap_lookup_by_pdevname
> API support
> for non-DT case and send across for review. 

Arnd, can you force the applying of the latest clps711x patches to accelerate the process?
I mean latest 3 arm-soc patches from Aug, 19.
After that I will need to make a patch for the SPI and TTY subsystems, then initial DT
support will be introduced.

---

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

[Alexander Shiyan]

Thu, 04 Sep 2014 10:09:19 +0530 от Pankaj Dubey <pankaj.dubey@samsung.com>:
> Hi Arnd,
> 
> On Tuesday, September 02, 2014 Arnd Bergmann wrote,
> > To: Pankaj Dubey
> > Cc: linux-arm-kernel@lists.infradead.org;
> linux-samsung-soc@vger.kernel.org; linux-
> > kernel@vger.kernel.org; lee.jones@linaro.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@gmail.com; Tomasz Figa; Alexander Shiyan; Michal Simek
> > Subject: Re: [PATCH v2] mfd: syscon: Decouple syscon interface from
> platform
> > devices
...
> > > -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);
> > > -
> > > -	return syscon->regmap;
> > > -}
> > > -EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
> > 
> > I think this can actually be left intact if that helps with clps71xx.
> > It could be done in a hacky way using bus_find_device_by_name() to keep it
> simple,
> > or in a somewhat nicer way by keeping the syscon platform_driver around
> for the
> > non-DT case.
> > 
> 
> Ok as per our last discussion you mentioned that clps71xx will be soon
> migrating to DT.
> So if that is not going to happen sooner, I would also prefer better keep
> syscon_regmap_lookup_by_pdevname and syscon platform_driver for non-DT case,
> so that this issue should not block this patch.
> 
> So please let's make final call to keep syscon platform_driver for non-DT
> case which eventually
> can be dropped once clps71xx driver migrates to DT based. So that I can
> prepare next patchset
> keeping syscon platform_driver support  and syscon_regmap_lookup_by_pdevname
> API support
> for non-DT case and send across for review. 

Arnd, can you force the applying of the latest clps711x patches to accelerate the process?
I mean latest 3 arm-soc patches from Aug, 19.
After that I will need to make a patch for the SPI and TTY subsystems, then initial DT
support will be introduced.

---

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

[Alexander Shiyan]

Thu, 04 Sep 2014 10:09:19 +0530 ?? Pankaj Dubey <pankaj.dubey@samsung.com>:
> Hi Arnd,
> 
> On Tuesday, September 02, 2014 Arnd Bergmann wrote,
> > To: Pankaj Dubey
> > Cc: linux-arm-kernel at lists.infradead.org;
> linux-samsung-soc at vger.kernel.org; linux-
> > kernel at vger.kernel.org; lee.jones at linaro.org; kgene.kim at samsung.com;
> > linux at arm.linux.org.uk; vikas.sajjan at samsung.com; joshi at samsung.com;
> > naushad at samsung.com; thomas.ab at samsung.com; chow.kim at samsung.com;
> > tomasz.figa at gmail.com; Tomasz Figa; Alexander Shiyan; Michal Simek
> > Subject: Re: [PATCH v2] mfd: syscon: Decouple syscon interface from
> platform
> > devices
...
> > > -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);
> > > -
> > > -	return syscon->regmap;
> > > -}
> > > -EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
> > 
> > I think this can actually be left intact if that helps with clps71xx.
> > It could be done in a hacky way using bus_find_device_by_name() to keep it
> simple,
> > or in a somewhat nicer way by keeping the syscon platform_driver around
> for the
> > non-DT case.
> > 
> 
> Ok as per our last discussion you mentioned that clps71xx will be soon
> migrating to DT.
> So if that is not going to happen sooner, I would also prefer better keep
> syscon_regmap_lookup_by_pdevname and syscon platform_driver for non-DT case,
> so that this issue should not block this patch.
> 
> So please let's make final call to keep syscon platform_driver for non-DT
> case which eventually
> can be dropped once clps71xx driver migrates to DT based. So that I can
> prepare next patchset
> keeping syscon platform_driver support  and syscon_regmap_lookup_by_pdevname
> API support
> for non-DT case and send across for review. 

Arnd, can you force the applying of the latest clps711x patches to accelerate the process?
I mean latest 3 arm-soc patches from Aug, 19.
After that I will need to make a patch for the SPI and TTY subsystems, then initial DT
support will be introduced.

---

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

[Boris BREZILLON]

Hi Pankaj

On Thu, 04 Sep 2014 10:15:27 +0530
Pankaj Dubey <pankaj.dubey@samsung.com> wrote:

> Hi Boris,
> 
> On Wednesday, September 03, 2014 Boris BREZILLON wrote,
> > To: Arnd Bergmann
> > Cc: Pankaj Dubey; kgene.kim@samsung.com; linux@arm.linux.org.uk; Alexander
> > Shiyan; naushad@samsung.com; Tomasz Figa; linux-kernel@vger.kernel.org;
> > joshi@samsung.com; linux-samsung-soc@vger.kernel.org;
> thomas.ab@samsung.com;
> > tomasz.figa@gmail.com; vikas.sajjan@samsung.com; chow.kim@samsung.com;
> > lee.jones@linaro.org; Michal Simek; linux-arm-kernel@lists.infradead.org;
> Mark
> > Brown
> > Subject: Re: [PATCH v2] mfd: syscon: Decouple syscon interface from
> platform
> > devices
> > 
> > On Wed, 03 Sep 2014 15:49:04 +0200
> > Arnd Bergmann <arnd@arndb.de> wrote:
> > 
> > > On Wednesday 03 September 2014 15:16:11 Boris BREZILLON wrote:
> > > > I checked that part, and it appears most of the code is already
> > > > there (see usage of regmap_attach_dev function here [1]).
> > > >
> > > > The only problem I see is that errors are still printed with
> > > > dev_err, which, AFAIK, will trigger a kernel panic if dev is NULL.
> > >
> > > Actually not:
> > >
> > > static int __dev_printk(const char *level, const struct device *dev,
> > >                         struct va_format *vaf) {
> > >         if (!dev)
> > >                 return printk("%s(NULL device *): %pV", level, vaf);
> > >
> > >         return dev_printk_emit(level[1] - '0', dev,
> > >                                "%s %s: %pV",
> > >                                dev_driver_string(dev), dev_name(dev),
> > > vaf); }
> > >
> > 
> > My bad then (I don't know where I looked at to think NULL dev was not
> gracefully
> > handled :-)). Thanks for pointing this out.
> > Given that, I think it should work fine even with a NULL dev.
> > I'll give it a try on at91 ;-).
> > 
> 
> We have tested this patch, on Exynos board and found working well.
> In our use case DT based drivers such as USB Phy, SATA Phy, Watchdog are
> calling 
> syscon_regmap_lookup_by APIs to get regmap handle to Exynos PMU and it
> worked 
> well for these drivers. 

Great!

> 
> It would be great if after testing you share result here or give a
> Tested-By.

Yep, that's the idea.
Let me some time to rework the PMC drivers (drivers/clk/at91/pmc.c) and
test it, and I'll add my Tested-by ;-).

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

[Boris BREZILLON]

Hi Pankaj

On Thu, 04 Sep 2014 10:15:27 +0530
Pankaj Dubey <pankaj.dubey@samsung.com> wrote:

> Hi Boris,
> 
> On Wednesday, September 03, 2014 Boris BREZILLON wrote,
> > To: Arnd Bergmann
> > Cc: Pankaj Dubey; kgene.kim at samsung.com; linux at arm.linux.org.uk; Alexander
> > Shiyan; naushad at samsung.com; Tomasz Figa; linux-kernel at vger.kernel.org;
> > joshi at samsung.com; linux-samsung-soc at vger.kernel.org;
> thomas.ab at samsung.com;
> > tomasz.figa at gmail.com; vikas.sajjan at samsung.com; chow.kim at samsung.com;
> > lee.jones at linaro.org; Michal Simek; linux-arm-kernel at lists.infradead.org;
> Mark
> > Brown
> > Subject: Re: [PATCH v2] mfd: syscon: Decouple syscon interface from
> platform
> > devices
> > 
> > On Wed, 03 Sep 2014 15:49:04 +0200
> > Arnd Bergmann <arnd@arndb.de> wrote:
> > 
> > > On Wednesday 03 September 2014 15:16:11 Boris BREZILLON wrote:
> > > > I checked that part, and it appears most of the code is already
> > > > there (see usage of regmap_attach_dev function here [1]).
> > > >
> > > > The only problem I see is that errors are still printed with
> > > > dev_err, which, AFAIK, will trigger a kernel panic if dev is NULL.
> > >
> > > Actually not:
> > >
> > > static int __dev_printk(const char *level, const struct device *dev,
> > >                         struct va_format *vaf) {
> > >         if (!dev)
> > >                 return printk("%s(NULL device *): %pV", level, vaf);
> > >
> > >         return dev_printk_emit(level[1] - '0', dev,
> > >                                "%s %s: %pV",
> > >                                dev_driver_string(dev), dev_name(dev),
> > > vaf); }
> > >
> > 
> > My bad then (I don't know where I looked at to think NULL dev was not
> gracefully
> > handled :-)). Thanks for pointing this out.
> > Given that, I think it should work fine even with a NULL dev.
> > I'll give it a try on at91 ;-).
> > 
> 
> We have tested this patch, on Exynos board and found working well.
> In our use case DT based drivers such as USB Phy, SATA Phy, Watchdog are
> calling 
> syscon_regmap_lookup_by APIs to get regmap handle to Exynos PMU and it
> worked 
> well for these drivers. 

Great!

> 
> It would be great if after testing you share result here or give a
> Tested-By.

Yep, that's the idea.
Let me some time to rework the PMC drivers (drivers/clk/at91/pmc.c) and
test it, and I'll add my Tested-by ;-).

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

[Boris BREZILLON]

On Thu, 04 Sep 2014 10:15:27 +0530
Pankaj Dubey <pankaj.dubey@samsung.com> wrote:

> Hi Boris,
> 
> On Wednesday, September 03, 2014 Boris BREZILLON wrote,
> > To: Arnd Bergmann
> > Cc: Pankaj Dubey; kgene.kim@samsung.com; linux@arm.linux.org.uk; Alexander
> > Shiyan; naushad@samsung.com; Tomasz Figa; linux-kernel@vger.kernel.org;
> > joshi@samsung.com; linux-samsung-soc@vger.kernel.org;
> thomas.ab@samsung.com;
> > tomasz.figa@gmail.com; vikas.sajjan@samsung.com; chow.kim@samsung.com;
> > lee.jones@linaro.org; Michal Simek; linux-arm-kernel@lists.infradead.org;
> Mark
> > Brown
> > Subject: Re: [PATCH v2] mfd: syscon: Decouple syscon interface from
> platform
> > devices
> > 
> > On Wed, 03 Sep 2014 15:49:04 +0200
> > Arnd Bergmann <arnd@arndb.de> wrote:
> > 
> > > On Wednesday 03 September 2014 15:16:11 Boris BREZILLON wrote:
> > > > I checked that part, and it appears most of the code is already
> > > > there (see usage of regmap_attach_dev function here [1]).
> > > >
> > > > The only problem I see is that errors are still printed with
> > > > dev_err, which, AFAIK, will trigger a kernel panic if dev is NULL.
> > >
> > > Actually not:
> > >
> > > static int __dev_printk(const char *level, const struct device *dev,
> > >                         struct va_format *vaf) {
> > >         if (!dev)
> > >                 return printk("%s(NULL device *): %pV", level, vaf);
> > >
> > >         return dev_printk_emit(level[1] - '0', dev,
> > >                                "%s %s: %pV",
> > >                                dev_driver_string(dev), dev_name(dev),
> > > vaf); }
> > >
> > 
> > My bad then (I don't know where I looked at to think NULL dev was not
> gracefully
> > handled :-)). Thanks for pointing this out.
> > Given that, I think it should work fine even with a NULL dev.
> > I'll give it a try on at91 ;-).
> > 
> 
> We have tested this patch, on Exynos board and found working well.
> In our use case DT based drivers such as USB Phy, SATA Phy, Watchdog are
> calling 
> syscon_regmap_lookup_by APIs to get regmap handle to Exynos PMU and it
> worked 
> well for these drivers. 
> 
> It would be great if after testing you share result here or give a
> Tested-By.
> 

I eventually tested it (just required minor modifications to my PMC
code: see below).
Still, this patch is not achieving my final goal which is to remove the
global at91_pmc_base variable and make use of the syscon regmap to
implement at91 cpu idle and platform suspend.
Moreover, I'd like to remove the lock in at91_pmc struct as regmap is
already taking care of locking the resources when accessing a
register, but this requires a lot more changes.

Anyway, here is my

Tested-by: Boris Brezillon <boris.brezillon@free-electrons.com>



diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig
index dd28e1f..7df2c9b 100644
--- a/arch/arm/mach-at91/Kconfig
+++ b/arch/arm/mach-at91/Kconfig
@@ -23,6 +23,7 @@ config COMMON_CLK_AT91
 	bool
 	default AT91_PMC_UNIT && USE_OF && !AT91_USE_OLD_CLK
 	select COMMON_CLK
+	select MFD_SYSCON
 
 config OLD_CLK_AT91
 	bool
diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
index 524196b..fb2c0af 100644
--- a/drivers/clk/at91/pmc.c
+++ b/drivers/clk/at91/pmc.c
@@ -19,6 +19,7 @@
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqdomain.h>
 #include <linux/of_irq.h>
+#include <linux/mfd/syscon.h>
 
 #include <asm/proc-fns.h>
 
@@ -190,6 +191,7 @@ static const struct at91_pmc_caps sama5d3_caps = {
 };
 
 static struct at91_pmc *__init at91_pmc_init(struct device_node *np,
+					     struct regmap *regmap,
 					     void __iomem *regbase,
int virq, const struct at91_pmc_caps *caps)
 {
@@ -205,7 +207,7 @@ static struct at91_pmc *__init at91_pmc_init(struct
device_node *np, return NULL;
 
 	spin_lock_init(&pmc->lock);
-	pmc->regbase = regbase;
+	pmc->regmap = regmap;
 	pmc->virq = virq;
 	pmc->caps = caps;
 
@@ -348,16 +350,46 @@ static void __init of_at91_pmc_setup(struct
device_node *np, void (*clk_setup)(struct device_node *, struct
at91_pmc *); const struct of_device_id *clk_id;
 	void __iomem *regbase = of_iomap(np, 0);
+	struct regmap *regmap;
 	int virq;
 
-	if (!regbase)
-		return;
+	/*
+	 * If the pmc compatible property does not contain the "syscon"
+	 * string, patch the property so that syscon_node_to_regmap can
+	 * consider it as a syscon device.
+	 */
+	if (!of_device_is_compatible(np, "syscon")) {
+		struct property *newprop, *oldprop;
+
+		oldprop = of_find_property(np, "compatible", NULL);
+		if (!oldprop)
+			panic("Could not find compatible property");
+
+		newprop = kzalloc(sizeof(*newprop), GFP_KERNEL);
+		if (!newprop)
+			panic("Could not allocate compatible
property"); +
+		newprop->name = "compatible";
+		newprop->length = oldprop->length + sizeof("syscon");
+		newprop->value = kmalloc(newprop->length, GFP_KERNEL);
+		if (!newprop->value) {
+			kfree(newprop->value);
+			panic("Could not allocate compatible string");
+		}
+		memcpy(newprop->value, oldprop->value,
oldprop->length);
+		memcpy(newprop->value + oldprop->length, "syscon",
sizeof("syscon"));
+		of_update_property(np, newprop);
+	}
+
+	regmap = syscon_node_to_regmap(np);
+	if (IS_ERR(regmap))
+		panic("Could not retrieve syscon regmap");
 
 	virq = irq_of_parse_and_map(np, 0);
 	if (!virq)
 		return;
 
-	pmc = at91_pmc_init(np, regbase, virq, caps);
+	pmc = at91_pmc_init(np, regmap, regbase, virq, caps);
 	if (!pmc)
 		return;
 	for_each_child_of_node(np, childnp) {
diff --git a/drivers/clk/at91/pmc.h b/drivers/clk/at91/pmc.h
index 6c76259..49589ea 100644
--- a/drivers/clk/at91/pmc.h
+++ b/drivers/clk/at91/pmc.h
@@ -14,6 +14,7 @@
 
 #include <linux/io.h>
 #include <linux/irqdomain.h>
+#include <linux/regmap.h>
 #include <linux/spinlock.h>
 
 struct clk_range {
@@ -28,7 +29,7 @@ struct at91_pmc_caps {
 };
 
 struct at91_pmc {
-	void __iomem *regbase;
+	struct regmap *regmap;
 	int virq;
 	spinlock_t lock;
 	const struct at91_pmc_caps *caps;
@@ -47,12 +48,16 @@ static inline void pmc_unlock(struct at91_pmc *pmc)
 
 static inline u32 pmc_read(struct at91_pmc *pmc, int offset)
 {
-	return readl(pmc->regbase + offset);
+	unsigned int ret = 0;
+
+	regmap_read(pmc->regmap, offset, &ret);
+
+	return ret;
 }
 
 static inline void pmc_write(struct at91_pmc *pmc, int offset, u32
value) {
-	writel(value, pmc->regbase + offset);
+	regmap_write(pmc->regmap, offset, value);
 }
 
 int of_at91_get_clk_range(struct device_node *np, const char *propname,

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

[Boris BREZILLON]

On Thu, 04 Sep 2014 10:15:27 +0530
Pankaj Dubey <pankaj.dubey@samsung.com> wrote:

> Hi Boris,
> 
> On Wednesday, September 03, 2014 Boris BREZILLON wrote,
> > To: Arnd Bergmann
> > Cc: Pankaj Dubey; kgene.kim at samsung.com; linux at arm.linux.org.uk; Alexander
> > Shiyan; naushad at samsung.com; Tomasz Figa; linux-kernel at vger.kernel.org;
> > joshi at samsung.com; linux-samsung-soc at vger.kernel.org;
> thomas.ab at samsung.com;
> > tomasz.figa at gmail.com; vikas.sajjan at samsung.com; chow.kim at samsung.com;
> > lee.jones at linaro.org; Michal Simek; linux-arm-kernel at lists.infradead.org;
> Mark
> > Brown
> > Subject: Re: [PATCH v2] mfd: syscon: Decouple syscon interface from
> platform
> > devices
> > 
> > On Wed, 03 Sep 2014 15:49:04 +0200
> > Arnd Bergmann <arnd@arndb.de> wrote:
> > 
> > > On Wednesday 03 September 2014 15:16:11 Boris BREZILLON wrote:
> > > > I checked that part, and it appears most of the code is already
> > > > there (see usage of regmap_attach_dev function here [1]).
> > > >
> > > > The only problem I see is that errors are still printed with
> > > > dev_err, which, AFAIK, will trigger a kernel panic if dev is NULL.
> > >
> > > Actually not:
> > >
> > > static int __dev_printk(const char *level, const struct device *dev,
> > >                         struct va_format *vaf) {
> > >         if (!dev)
> > >                 return printk("%s(NULL device *): %pV", level, vaf);
> > >
> > >         return dev_printk_emit(level[1] - '0', dev,
> > >                                "%s %s: %pV",
> > >                                dev_driver_string(dev), dev_name(dev),
> > > vaf); }
> > >
> > 
> > My bad then (I don't know where I looked at to think NULL dev was not
> gracefully
> > handled :-)). Thanks for pointing this out.
> > Given that, I think it should work fine even with a NULL dev.
> > I'll give it a try on at91 ;-).
> > 
> 
> We have tested this patch, on Exynos board and found working well.
> In our use case DT based drivers such as USB Phy, SATA Phy, Watchdog are
> calling 
> syscon_regmap_lookup_by APIs to get regmap handle to Exynos PMU and it
> worked 
> well for these drivers. 
> 
> It would be great if after testing you share result here or give a
> Tested-By.
> 

I eventually tested it (just required minor modifications to my PMC
code: see below).
Still, this patch is not achieving my final goal which is to remove the
global at91_pmc_base variable and make use of the syscon regmap to
implement at91 cpu idle and platform suspend.
Moreover, I'd like to remove the lock in at91_pmc struct as regmap is
already taking care of locking the resources when accessing a
register, but this requires a lot more changes.

Anyway, here is my

Tested-by: Boris Brezillon <boris.brezillon@free-electrons.com>



diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig
index dd28e1f..7df2c9b 100644
--- a/arch/arm/mach-at91/Kconfig
+++ b/arch/arm/mach-at91/Kconfig
@@ -23,6 +23,7 @@ config COMMON_CLK_AT91
 	bool
 	default AT91_PMC_UNIT && USE_OF && !AT91_USE_OLD_CLK
 	select COMMON_CLK
+	select MFD_SYSCON
 
 config OLD_CLK_AT91
 	bool
diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
index 524196b..fb2c0af 100644
--- a/drivers/clk/at91/pmc.c
+++ b/drivers/clk/at91/pmc.c
@@ -19,6 +19,7 @@
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqdomain.h>
 #include <linux/of_irq.h>
+#include <linux/mfd/syscon.h>
 
 #include <asm/proc-fns.h>
 
@@ -190,6 +191,7 @@ static const struct at91_pmc_caps sama5d3_caps = {
 };
 
 static struct at91_pmc *__init at91_pmc_init(struct device_node *np,
+					     struct regmap *regmap,
 					     void __iomem *regbase,
int virq, const struct at91_pmc_caps *caps)
 {
@@ -205,7 +207,7 @@ static struct at91_pmc *__init at91_pmc_init(struct
device_node *np, return NULL;
 
 	spin_lock_init(&pmc->lock);
-	pmc->regbase = regbase;
+	pmc->regmap = regmap;
 	pmc->virq = virq;
 	pmc->caps = caps;
 
@@ -348,16 +350,46 @@ static void __init of_at91_pmc_setup(struct
device_node *np, void (*clk_setup)(struct device_node *, struct
at91_pmc *); const struct of_device_id *clk_id;
 	void __iomem *regbase = of_iomap(np, 0);
+	struct regmap *regmap;
 	int virq;
 
-	if (!regbase)
-		return;
+	/*
+	 * If the pmc compatible property does not contain the "syscon"
+	 * string, patch the property so that syscon_node_to_regmap can
+	 * consider it as a syscon device.
+	 */
+	if (!of_device_is_compatible(np, "syscon")) {
+		struct property *newprop, *oldprop;
+
+		oldprop = of_find_property(np, "compatible", NULL);
+		if (!oldprop)
+			panic("Could not find compatible property");
+
+		newprop = kzalloc(sizeof(*newprop), GFP_KERNEL);
+		if (!newprop)
+			panic("Could not allocate compatible
property"); +
+		newprop->name = "compatible";
+		newprop->length = oldprop->length + sizeof("syscon");
+		newprop->value = kmalloc(newprop->length, GFP_KERNEL);
+		if (!newprop->value) {
+			kfree(newprop->value);
+			panic("Could not allocate compatible string");
+		}
+		memcpy(newprop->value, oldprop->value,
oldprop->length);
+		memcpy(newprop->value + oldprop->length, "syscon",
sizeof("syscon"));
+		of_update_property(np, newprop);
+	}
+
+	regmap = syscon_node_to_regmap(np);
+	if (IS_ERR(regmap))
+		panic("Could not retrieve syscon regmap");
 
 	virq = irq_of_parse_and_map(np, 0);
 	if (!virq)
 		return;
 
-	pmc = at91_pmc_init(np, regbase, virq, caps);
+	pmc = at91_pmc_init(np, regmap, regbase, virq, caps);
 	if (!pmc)
 		return;
 	for_each_child_of_node(np, childnp) {
diff --git a/drivers/clk/at91/pmc.h b/drivers/clk/at91/pmc.h
index 6c76259..49589ea 100644
--- a/drivers/clk/at91/pmc.h
+++ b/drivers/clk/at91/pmc.h
@@ -14,6 +14,7 @@
 
 #include <linux/io.h>
 #include <linux/irqdomain.h>
+#include <linux/regmap.h>
 #include <linux/spinlock.h>
 
 struct clk_range {
@@ -28,7 +29,7 @@ struct at91_pmc_caps {
 };
 
 struct at91_pmc {
-	void __iomem *regbase;
+	struct regmap *regmap;
 	int virq;
 	spinlock_t lock;
 	const struct at91_pmc_caps *caps;
@@ -47,12 +48,16 @@ static inline void pmc_unlock(struct at91_pmc *pmc)
 
 static inline u32 pmc_read(struct at91_pmc *pmc, int offset)
 {
-	return readl(pmc->regbase + offset);
+	unsigned int ret = 0;
+
+	regmap_read(pmc->regmap, offset, &ret);
+
+	return ret;
 }
 
 static inline void pmc_write(struct at91_pmc *pmc, int offset, u32
value) {
-	writel(value, pmc->regbase + offset);
+	regmap_write(pmc->regmap, offset, value);
 }
 
 int of_at91_get_clk_range(struct device_node *np, const char *propname,

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

[Pankaj Dubey]

Hi Arnd,  Lee Jones,


[snip]
> 
> On Thu, 04 Sep 2014 10:15:27 +0530
> Pankaj Dubey <pankaj.dubey@samsung.com> wrote:
> 
> > Hi Boris,
> >
> > On Wednesday, September 03, 2014 Boris BREZILLON wrote,
> > > To: Arnd Bergmann
> > > Cc: Pankaj Dubey; kgene.kim@samsung.com; linux@arm.linux.org.uk;
> > > Alexander Shiyan; naushad@samsung.com; Tomasz Figa;
> > > linux-kernel@vger.kernel.org; joshi@samsung.com;
> > > linux-samsung-soc@vger.kernel.org;
> > thomas.ab@samsung.com;
> > > tomasz.figa@gmail.com; vikas.sajjan@samsung.com;
> > > chow.kim@samsung.com; lee.jones@linaro.org; Michal Simek;
> > > linux-arm-kernel@lists.infradead.org;
> > Mark
> > > Brown
> > > Subject: Re: [PATCH v2] mfd: syscon: Decouple syscon interface from
> > platform
> > > devices
> > >
> > > On Wed, 03 Sep 2014 15:49:04 +0200
> > > Arnd Bergmann <arnd@arndb.de> wrote:
> > >
> > > > On Wednesday 03 September 2014 15:16:11 Boris BREZILLON wrote:
> > > > > I checked that part, and it appears most of the code is already
> > > > > there (see usage of regmap_attach_dev function here [1]).
> > > > >
> > > > > The only problem I see is that errors are still printed with
> > > > > dev_err, which, AFAIK, will trigger a kernel panic if dev is NULL.
> > > >
> > > > Actually not:
> > > >
> > > > static int __dev_printk(const char *level, const struct device *dev,
> > > >                         struct va_format *vaf) {
> > > >         if (!dev)
> > > >                 return printk("%s(NULL device *): %pV", level,
> > > > vaf);
> > > >
> > > >         return dev_printk_emit(level[1] - '0', dev,
> > > >                                "%s %s: %pV",
> > > >                                dev_driver_string(dev),
> > > > dev_name(dev), vaf); }
> > > >
> > >
> > > My bad then (I don't know where I looked at to think NULL dev was
> > > not
> > gracefully
> > > handled :-)). Thanks for pointing this out.
> > > Given that, I think it should work fine even with a NULL dev.
> > > I'll give it a try on at91 ;-).
> > >
> >
> > We have tested this patch, on Exynos board and found working well.
> > In our use case DT based drivers such as USB Phy, SATA Phy, Watchdog
> > are calling syscon_regmap_lookup_by APIs to get regmap handle to
> > Exynos PMU and it worked well for these drivers.
> >
> > It would be great if after testing you share result here or give a
> > Tested-By.
> >
> 
> I eventually tested it (just required minor modifications to my PMC
> code: see below).
> Still, this patch is not achieving my final goal which is to remove the
global
> at91_pmc_base variable and make use of the syscon regmap to implement at91
cpu
> idle and platform suspend.
> Moreover, I'd like to remove the lock in at91_pmc struct as regmap is
already taking
> care of locking the resources when accessing a register, but this requires
a lot more
> changes.
> 
> Anyway, here is my
> 
> Tested-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> 

Any update on this patch. As already it has been tested on two DT based
platforms.

If you think that we can go ahead and break clps711x till it gets migrated
over DT,
then please ack this patch, or else if you have opinion to keep support for
non-DT
based drivers (clps711x) then I can post another patch keeping platform
driver support
for non-DT based platform. 

I would prefer is to keep platform driver support for non-DT based platform
so that this patch set can go in this merge window, as lot of Exynos PMU
patches
(PMU patches of many Exynos SoCs [2,3,4] ) are critically dependent on this
change. 

As per discussion here [1], clps711x SPI and TTY driver patches still has to
be posted
which may take one more merge window, and eventually will drag this patch
also.

[1]:
https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg36291.html
[2]:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/275675.html
[3]:
https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg35701.html
[4]: http://www.spinics.net/lists/arm-kernel/msg358230.html


> 
> 
[snip]

Thanks,
Pankaj Dubey

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

[Pankaj Dubey]

Hi Arnd,  Lee Jones,


[snip]
> 
> On Thu, 04 Sep 2014 10:15:27 +0530
> Pankaj Dubey <pankaj.dubey@samsung.com> wrote:
> 
> > Hi Boris,
> >
> > On Wednesday, September 03, 2014 Boris BREZILLON wrote,
> > > To: Arnd Bergmann
> > > Cc: Pankaj Dubey; kgene.kim at samsung.com; linux at arm.linux.org.uk;
> > > Alexander Shiyan; naushad at samsung.com; Tomasz Figa;
> > > linux-kernel at vger.kernel.org; joshi at samsung.com;
> > > linux-samsung-soc at vger.kernel.org;
> > thomas.ab at samsung.com;
> > > tomasz.figa at gmail.com; vikas.sajjan at samsung.com;
> > > chow.kim at samsung.com; lee.jones at linaro.org; Michal Simek;
> > > linux-arm-kernel at lists.infradead.org;
> > Mark
> > > Brown
> > > Subject: Re: [PATCH v2] mfd: syscon: Decouple syscon interface from
> > platform
> > > devices
> > >
> > > On Wed, 03 Sep 2014 15:49:04 +0200
> > > Arnd Bergmann <arnd@arndb.de> wrote:
> > >
> > > > On Wednesday 03 September 2014 15:16:11 Boris BREZILLON wrote:
> > > > > I checked that part, and it appears most of the code is already
> > > > > there (see usage of regmap_attach_dev function here [1]).
> > > > >
> > > > > The only problem I see is that errors are still printed with
> > > > > dev_err, which, AFAIK, will trigger a kernel panic if dev is NULL.
> > > >
> > > > Actually not:
> > > >
> > > > static int __dev_printk(const char *level, const struct device *dev,
> > > >                         struct va_format *vaf) {
> > > >         if (!dev)
> > > >                 return printk("%s(NULL device *): %pV", level,
> > > > vaf);
> > > >
> > > >         return dev_printk_emit(level[1] - '0', dev,
> > > >                                "%s %s: %pV",
> > > >                                dev_driver_string(dev),
> > > > dev_name(dev), vaf); }
> > > >
> > >
> > > My bad then (I don't know where I looked at to think NULL dev was
> > > not
> > gracefully
> > > handled :-)). Thanks for pointing this out.
> > > Given that, I think it should work fine even with a NULL dev.
> > > I'll give it a try on at91 ;-).
> > >
> >
> > We have tested this patch, on Exynos board and found working well.
> > In our use case DT based drivers such as USB Phy, SATA Phy, Watchdog
> > are calling syscon_regmap_lookup_by APIs to get regmap handle to
> > Exynos PMU and it worked well for these drivers.
> >
> > It would be great if after testing you share result here or give a
> > Tested-By.
> >
> 
> I eventually tested it (just required minor modifications to my PMC
> code: see below).
> Still, this patch is not achieving my final goal which is to remove the
global
> at91_pmc_base variable and make use of the syscon regmap to implement at91
cpu
> idle and platform suspend.
> Moreover, I'd like to remove the lock in at91_pmc struct as regmap is
already taking
> care of locking the resources when accessing a register, but this requires
a lot more
> changes.
> 
> Anyway, here is my
> 
> Tested-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> 

Any update on this patch. As already it has been tested on two DT based
platforms.

If you think that we can go ahead and break clps711x till it gets migrated
over DT,
then please ack this patch, or else if you have opinion to keep support for
non-DT
based drivers (clps711x) then I can post another patch keeping platform
driver support
for non-DT based platform. 

I would prefer is to keep platform driver support for non-DT based platform
so that this patch set can go in this merge window, as lot of Exynos PMU
patches
(PMU patches of many Exynos SoCs [2,3,4] ) are critically dependent on this
change. 

As per discussion here [1], clps711x SPI and TTY driver patches still has to
be posted
which may take one more merge window, and eventually will drag this patch
also.

[1]:
https://www.mail-archive.com/linux-samsung-soc at vger.kernel.org/msg36291.html
[2]:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/275675.html
[3]:
https://www.mail-archive.com/linux-samsung-soc at vger.kernel.org/msg35701.html
[4]: http://www.spinics.net/lists/arm-kernel/msg358230.html


> 
> 
[snip]

Thanks,
Pankaj Dubey

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

[Arnd Bergmann]

On Tuesday 16 September 2014, Pankaj Dubey wrote:
> Hi Arnd,  Lee Jones,
> > On Thu, 04 Sep 2014 10:15:27 +0530
> > Pankaj Dubey <pankaj.dubey@samsung.com> wrote:
> 
> Any update on this patch. As already it has been tested on two DT based
> platforms.
> 
> If you think that we can go ahead and break clps711x till it gets migrated
> over DT,
> then please ack this patch, or else if you have opinion to keep support for
> non-DT
> based drivers (clps711x) then I can post another patch keeping platform
> driver support
> for non-DT based platform. 

The rule is that we don't intentionally break things, so please post a
patch that keeps the existing platform driver there. Ideally we get a DT-only
clps711x for the merge window as well, and if that happens we can add another
patch on top to remove that syscon-pdev support again but then we will
have a bisectable kernel that works for every point inbetween.


	Arnd

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

[Arnd Bergmann]

On Tuesday 16 September 2014, Pankaj Dubey wrote:
> Hi Arnd,  Lee Jones,
> > On Thu, 04 Sep 2014 10:15:27 +0530
> > Pankaj Dubey <pankaj.dubey@samsung.com> wrote:
> 
> Any update on this patch. As already it has been tested on two DT based
> platforms.
> 
> If you think that we can go ahead and break clps711x till it gets migrated
> over DT,
> then please ack this patch, or else if you have opinion to keep support for
> non-DT
> based drivers (clps711x) then I can post another patch keeping platform
> driver support
> for non-DT based platform. 

The rule is that we don't intentionally break things, so please post a
patch that keeps the existing platform driver there. Ideally we get a DT-only
clps711x for the merge window as well, and if that happens we can add another
patch on top to remove that syscon-pdev support again but then we will
have a bisectable kernel that works for every point inbetween.


	Arnd