* [PATCH v2] mfd: syscon: Decouple syscon interface from platform devices @ 2014-09-02 14:42 Pankaj Dubey 2014-09-02 14:50 ` Tomasz Figa ` (2 more replies) 0 siblings, 3 replies; 16+ messages in thread From: Pankaj Dubey @ 2014-09-02 14:42 UTC (permalink / raw) To: linux-arm-kernel, linux-samsung-soc, linux-kernel Cc: lee.jones, kgene.kim, linux, arnd, vikas.sajjan, joshi, naushad, thomas.ab, chow.kim, tomasz.figa, Pankaj Dubey, Tomasz Figa 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2] mfd: syscon: Decouple syscon interface from platform devices 2014-09-02 14:42 [PATCH v2] mfd: syscon: Decouple syscon interface from platform devices Pankaj Dubey @ 2014-09-02 14:50 ` Tomasz Figa 2014-09-02 15:20 ` Arnd Bergmann 2014-09-03 3:44 ` Vivek Gautam 2 siblings, 0 replies; 16+ messages in thread From: Tomasz Figa @ 2014-09-02 14:50 UTC (permalink / raw) To: Pankaj Dubey, linux-arm-kernel, linux-samsung-soc, linux-kernel Cc: lee.jones, kgene.kim, linux, arnd, vikas.sajjan, joshi, naushad, thomas.ab, chow.kim, 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] mfd: syscon: Decouple syscon interface from platform devices 2014-09-02 14:42 [PATCH v2] mfd: syscon: Decouple syscon interface from platform devices Pankaj Dubey 2014-09-02 14:50 ` Tomasz Figa @ 2014-09-02 15:20 ` Arnd Bergmann 2014-09-02 15:42 ` Alexander Shiyan ` (2 more replies) 2014-09-03 3:44 ` Vivek Gautam 2 siblings, 3 replies; 16+ messages in thread From: Arnd Bergmann @ 2014-09-02 15:20 UTC (permalink / raw) To: Pankaj Dubey Cc: linux-arm-kernel, linux-samsung-soc, linux-kernel, lee.jones, kgene.kim, linux, vikas.sajjan, joshi, naushad, thomas.ab, chow.kim, tomasz.figa, Tomasz Figa, Alexander Shiyan, Michal Simek 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] mfd: syscon: Decouple syscon interface from platform devices 2014-09-02 15:20 ` Arnd Bergmann @ 2014-09-02 15:42 ` Alexander Shiyan 2014-09-02 17:40 ` Arnd Bergmann 2014-09-03 13:16 ` Boris BREZILLON 2014-09-04 4:39 ` Pankaj Dubey 2 siblings, 1 reply; 16+ messages in thread From: Alexander Shiyan @ 2014-09-02 15:42 UTC (permalink / raw) To: Arnd Bergmann Cc: kgene.kim, linux, naushad, Tomasz Figa, linux-kernel, joshi, linux-samsung-soc, thomas.ab, tomasz.figa, vikas.sajjan, chow.kim, lee.jones, Michal Simek, linux-arm-kernel, Pankaj Dubey [-- 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¥ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] mfd: syscon: Decouple syscon interface from platform devices 2014-09-02 15:42 ` Alexander Shiyan @ 2014-09-02 17:40 ` Arnd Bergmann 0 siblings, 0 replies; 16+ messages in thread From: Arnd Bergmann @ 2014-09-02 17:40 UTC (permalink / raw) To: linux-arm-kernel, Alexander Shiyan Cc: kgene.kim, linux, naushad, lee.jones, Tomasz Figa, linux-kernel, joshi, linux-samsung-soc, thomas.ab, Pankaj Dubey, vikas.sajjan, chow.kim, tomasz.figa, Michal Simek 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] mfd: syscon: Decouple syscon interface from platform devices 2014-09-02 15:20 ` Arnd Bergmann 2014-09-02 15:42 ` Alexander Shiyan @ 2014-09-03 13:16 ` Boris BREZILLON 2014-09-03 13:49 ` Arnd Bergmann 2014-09-04 4:39 ` Pankaj Dubey 2 siblings, 1 reply; 16+ messages in thread From: Boris BREZILLON @ 2014-09-03 13:16 UTC (permalink / raw) To: Arnd Bergmann Cc: Pankaj Dubey, kgene.kim, linux, Alexander Shiyan, naushad, Tomasz Figa, linux-kernel, joshi, linux-samsung-soc, thomas.ab, tomasz.figa, vikas.sajjan, chow.kim, lee.jones, Michal Simek, linux-arm-kernel, Mark Brown 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] mfd: syscon: Decouple syscon interface from platform devices 2014-09-03 13:16 ` Boris BREZILLON @ 2014-09-03 13:49 ` Arnd Bergmann 2014-09-03 14:15 ` Boris BREZILLON 0 siblings, 1 reply; 16+ messages in thread From: Arnd Bergmann @ 2014-09-03 13:49 UTC (permalink / raw) To: Boris BREZILLON Cc: Pankaj Dubey, kgene.kim, linux, Alexander Shiyan, naushad, Tomasz Figa, linux-kernel, joshi, linux-samsung-soc, thomas.ab, tomasz.figa, vikas.sajjan, chow.kim, lee.jones, Michal Simek, linux-arm-kernel, Mark Brown 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] mfd: syscon: Decouple syscon interface from platform devices 2014-09-03 13:49 ` Arnd Bergmann @ 2014-09-03 14:15 ` Boris BREZILLON 2014-09-04 4:45 ` Pankaj Dubey 0 siblings, 1 reply; 16+ messages in thread From: Boris BREZILLON @ 2014-09-03 14:15 UTC (permalink / raw) To: Arnd Bergmann Cc: Pankaj Dubey, kgene.kim, linux, Alexander Shiyan, naushad, Tomasz Figa, linux-kernel, joshi, linux-samsung-soc, thomas.ab, tomasz.figa, vikas.sajjan, chow.kim, lee.jones, Michal Simek, linux-arm-kernel, Mark Brown 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v2] mfd: syscon: Decouple syscon interface from platform devices 2014-09-03 14:15 ` Boris BREZILLON @ 2014-09-04 4:45 ` Pankaj Dubey 2014-09-04 6:03 ` Boris BREZILLON 2014-09-05 8:14 ` Boris BREZILLON 0 siblings, 2 replies; 16+ messages in thread From: Pankaj Dubey @ 2014-09-04 4:45 UTC (permalink / raw) To: 'Boris BREZILLON', 'Arnd Bergmann' Cc: kgene.kim, linux, 'Alexander Shiyan', naushad, 'Tomasz Figa', linux-kernel, joshi, linux-samsung-soc, thomas.ab, tomasz.figa, vikas.sajjan, chow.kim, lee.jones, 'Michal Simek', linux-arm-kernel, 'Mark Brown' 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] mfd: syscon: Decouple syscon interface from platform devices 2014-09-04 4:45 ` Pankaj Dubey @ 2014-09-04 6:03 ` Boris BREZILLON 2014-09-05 8:14 ` Boris BREZILLON 1 sibling, 0 replies; 16+ messages in thread From: Boris BREZILLON @ 2014-09-04 6:03 UTC (permalink / raw) To: Pankaj Dubey Cc: 'Arnd Bergmann', kgene.kim, linux, 'Alexander Shiyan', naushad, 'Tomasz Figa', linux-kernel, joshi, linux-samsung-soc, thomas.ab, tomasz.figa, vikas.sajjan, chow.kim, lee.jones, 'Michal Simek', linux-arm-kernel, 'Mark Brown' 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] mfd: syscon: Decouple syscon interface from platform devices 2014-09-04 4:45 ` Pankaj Dubey 2014-09-04 6:03 ` Boris BREZILLON @ 2014-09-05 8:14 ` Boris BREZILLON 2014-09-16 11:53 ` Pankaj Dubey 1 sibling, 1 reply; 16+ messages in thread From: Boris BREZILLON @ 2014-09-05 8:14 UTC (permalink / raw) To: Pankaj Dubey Cc: 'Arnd Bergmann', kgene.kim, linux, 'Alexander Shiyan', naushad, 'Tomasz Figa', linux-kernel, joshi, linux-samsung-soc, thomas.ab, tomasz.figa, vikas.sajjan, chow.kim, lee.jones, 'Michal Simek', linux-arm-kernel, 'Mark Brown' 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, ^ permalink raw reply related [flat|nested] 16+ messages in thread
* RE: [PATCH v2] mfd: syscon: Decouple syscon interface from platform devices 2014-09-05 8:14 ` Boris BREZILLON @ 2014-09-16 11:53 ` Pankaj Dubey 2014-09-16 14:52 ` Arnd Bergmann 0 siblings, 1 reply; 16+ messages in thread From: Pankaj Dubey @ 2014-09-16 11:53 UTC (permalink / raw) To: 'Arnd Bergmann', lee.jones Cc: kgene.kim, linux, 'Alexander Shiyan', naushad, 'Tomasz Figa', linux-kernel, joshi, linux-samsung-soc, thomas.ab, tomasz.figa, vikas.sajjan, chow.kim, 'Michal Simek', linux-arm-kernel, 'Mark Brown', 'Boris BREZILLON', PRASHANTH GODREHAL 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] mfd: syscon: Decouple syscon interface from platform devices 2014-09-16 11:53 ` Pankaj Dubey @ 2014-09-16 14:52 ` Arnd Bergmann 0 siblings, 0 replies; 16+ messages in thread From: Arnd Bergmann @ 2014-09-16 14:52 UTC (permalink / raw) To: Pankaj Dubey Cc: lee.jones, kgene.kim, linux, 'Alexander Shiyan', naushad, 'Tomasz Figa', linux-kernel, joshi, linux-samsung-soc, thomas.ab, tomasz.figa, vikas.sajjan, chow.kim, 'Michal Simek', linux-arm-kernel, 'Mark Brown', 'Boris BREZILLON', PRASHANTH GODREHAL 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v2] mfd: syscon: Decouple syscon interface from platform devices 2014-09-02 15:20 ` Arnd Bergmann 2014-09-02 15:42 ` Alexander Shiyan 2014-09-03 13:16 ` Boris BREZILLON @ 2014-09-04 4:39 ` Pankaj Dubey 2014-09-04 4:52 ` Alexander Shiyan 2 siblings, 1 reply; 16+ messages in thread From: Pankaj Dubey @ 2014-09-04 4:39 UTC (permalink / raw) To: 'Arnd Bergmann' Cc: linux-arm-kernel, linux-samsung-soc, linux-kernel, lee.jones, kgene.kim, linux, vikas.sajjan, joshi, naushad, thomas.ab, chow.kim, tomasz.figa, 'Tomasz Figa', 'Alexander Shiyan', 'Michal Simek' 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] mfd: syscon: Decouple syscon interface from platform devices 2014-09-04 4:39 ` Pankaj Dubey @ 2014-09-04 4:52 ` Alexander Shiyan 0 siblings, 0 replies; 16+ messages in thread From: Alexander Shiyan @ 2014-09-04 4:52 UTC (permalink / raw) To: Pankaj Dubey Cc: kgene.kim, linux, naushad, 'Tomasz Figa', linux-kernel, joshi, linux-samsung-soc, thomas.ab, tomasz.figa, vikas.sajjan, chow.kim, lee.jones, 'Michal Simek', linux-arm-kernel, 'Arnd Bergmann' [-- 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¥ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] mfd: syscon: Decouple syscon interface from platform devices 2014-09-02 14:42 [PATCH v2] mfd: syscon: Decouple syscon interface from platform devices Pankaj Dubey 2014-09-02 14:50 ` Tomasz Figa 2014-09-02 15:20 ` Arnd Bergmann @ 2014-09-03 3:44 ` Vivek Gautam 2 siblings, 0 replies; 16+ messages in thread From: Vivek Gautam @ 2014-09-03 3:44 UTC (permalink / raw) To: Pankaj Dubey Cc: linux-arm-kernel, linux-samsung-soc, linux-kernel, lee.jones, Kukjin Kim, Russell King - ARM Linux, Arnd Bergmann, Vikas Sajjan, sunil joshi, naushad, Thomas Abraham, chow.kim, Tomasz Figa, Tomasz Figa 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2014-09-16 15:51 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-09-02 14:42 [PATCH v2] mfd: syscon: Decouple syscon interface from platform devices Pankaj Dubey 2014-09-02 14:50 ` Tomasz Figa 2014-09-02 15:20 ` Arnd Bergmann 2014-09-02 15:42 ` Alexander Shiyan 2014-09-02 17:40 ` Arnd Bergmann 2014-09-03 13:16 ` Boris BREZILLON 2014-09-03 13:49 ` Arnd Bergmann 2014-09-03 14:15 ` Boris BREZILLON 2014-09-04 4:45 ` Pankaj Dubey 2014-09-04 6:03 ` Boris BREZILLON 2014-09-05 8:14 ` Boris BREZILLON 2014-09-16 11:53 ` Pankaj Dubey 2014-09-16 14:52 ` Arnd Bergmann 2014-09-04 4:39 ` Pankaj Dubey 2014-09-04 4:52 ` Alexander Shiyan 2014-09-03 3:44 ` Vivek Gautam
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).