Hi Michal,
On 04/09/2014 12:00 AM, Michal Simek wrote:
> Some platforms need to get system controller
> ready as soon as possible.
> The patch provides early_syscon_initialization
> which create early mapping for all syscon compatible
> devices in early_syscon_probe.
> Regmap is get via syscon_early_regmap_lookup_by_phandle()
>
> Regular device probes attach device to regmap
> via regmap_attach_dev().
>
> For early syscon initialization is necessary to extend
> struct syscon and provide remove function
> which unmap all early init structures.
>
> Signed-off-by: Michal Simek <[email protected]>
> ---
>
> Changes in v3:
> - Keep backward compatibility for platform drivers and test it
> - Use only one probe method which is early_syscon_probe
> suggested by Lee Jones. Do not force anybody to call early_syscon_init
> - Add kernel-doc description
>
> Changes in v2:
> - Fix bad logic in early_syscon_probe
> - Fix compilation failure for x86_64 reported by zero day testing system
> - Regmap change available here
> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git tags/nodev
>
> drivers/mfd/syscon.c | 159 ++++++++++++++++++++++++++++++++++++++++-----
> include/linux/mfd/syscon.h | 11 ++++
> 2 files changed, 153 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
> index 71841f9..8e2ff88 100644
> --- a/drivers/mfd/syscon.c
> +++ b/drivers/mfd/syscon.c
> @@ -20,12 +20,15 @@
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
> +#include <linux/slab.h>
> #include <linux/mfd/syscon.h>
>
> static struct platform_driver syscon_driver;
>
> struct syscon {
> + void __iomem *base;
> struct regmap *regmap;
> + struct resource res;
> };
>
> static int syscon_match_node(struct device *dev, void *data)
> @@ -95,6 +98,30 @@ struct regmap *syscon_regmap_lookup_by_pdevname(const char *s)
> }
> EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
>
> +/**
> + * syscon_early_regmap_lookup_by_phandle - Early phandle lookup function
> + * @np: device_node pointer
> + * @property: property name which handle system controller phandle
> + * Return: regmap pointer, an error pointer otherwise
> + */
> +struct regmap *syscon_early_regmap_lookup_by_phandle(struct device_node *np,
> + const char *property)
> +{
> + struct device_node *syscon_np;
> + struct syscon *syscon;
> +
> + syscon_np = of_parse_phandle(np, property, 0);
> + if (!syscon_np)
> + return ERR_PTR(-ENODEV);
> +
> + syscon = syscon_np->data;
> +
> + of_node_put(syscon_np);
> +
> + return syscon->regmap;
> +}
> +EXPORT_SYMBOL_GPL(syscon_early_regmap_lookup_by_phandle);
> +
> struct regmap *syscon_regmap_lookup_by_phandle(struct device_node *np,
> const char *property)
> {
> @@ -123,36 +150,118 @@ static struct regmap_config syscon_regmap_config = {
> .reg_stride = 4,
> };
>
> -static int syscon_probe(struct platform_device *pdev)
> +/**
> + * early_syscon_probe - Early system controller probe method
> + * @np: device_node pointer
> + * @syscon_p: syscon pointer
> + * @res: device IO resource
> + * Return: 0 if successful, a negative error code otherwise
> + */
> +static int early_syscon_probe(struct device_node *np, struct syscon **syscon_p,
> + struct resource *res)
> {
> - struct device *dev = &pdev->dev;
> struct syscon *syscon;
> - struct resource *res;
> - void __iomem *base;
> + int ret;
> +
> + if (np && np->data) {
> + pr_debug("Early syscon was called\n");
> + *syscon_p = (struct syscon *)&np->data;
> + return 0;
> + }
>
> - syscon = devm_kzalloc(dev, sizeof(*syscon), GFP_KERNEL);
> + syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
> if (!syscon)
> return -ENOMEM;
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (!res)
> - return -ENOENT;
> + *syscon_p = (struct syscon *)&syscon;
>
> - base = devm_ioremap(dev, res->start, resource_size(res));
> - if (!base)
> - return -ENOMEM;
> + if (!res && np) {
> + if (of_address_to_resource(np, 0, &syscon->res)) {
> + ret = -EINVAL;
> + goto alloc;
> + }
> +
> + np->data = syscon;
> + of_node_put(np);
> + } else {
> + syscon->res = *res;
> + }
>
> - syscon_regmap_config.max_register = res->end - res->start - 3;
> - syscon->regmap = devm_regmap_init_mmio(dev, base,
> - &syscon_regmap_config);
> + syscon->base = ioremap(syscon->res.start, resource_size(&syscon->res));
> + if (!syscon->base) {
> + pr_err("%s: Unable to map I/O memory\n", __func__);
> + ret = PTR_ERR(syscon->base);
> + goto alloc;
> + }
> +
> + syscon_regmap_config.max_register = syscon->res.end -
> + syscon->res.start - 3;
> + syscon->regmap = regmap_init_mmio(NULL, syscon->base,
> + &syscon_regmap_config);
> if (IS_ERR(syscon->regmap)) {
> - dev_err(dev, "regmap init failed\n");
> - return PTR_ERR(syscon->regmap);
> + pr_err("regmap init failed\n");
> + ret = PTR_ERR(syscon->regmap);
> + goto iomap;
> }
> + if (np)
> + pr_info("syscon: %s regmap %pR registered\n", np->name,
> + &syscon->res);
> +
> + return 0;
> +
> +iomap:
> + iounmap(syscon->base);
> +alloc:
> + kfree(syscon);
> +
> + return ret;
> +}
> +
> +/**
> + * early_syscon_init - Early system controller initialization
> + */
> +void __init early_syscon_init(void)
> +{
> + struct device_node *np;
> + struct syscon *syscon = NULL;
> +
> + for_each_matching_node_and_match(np, of_syscon_match, NULL) {
> + if (early_syscon_probe(np, &syscon, NULL))
> + BUG();
> + }
> +}
> +
> +/**
> + * syscon_probe - System controller probe method
> + * @pdev: Platform device
> + * Return: 0 if successful, a negative error code otherwise
> + */
> +static int syscon_probe(struct platform_device *pdev)
> +{
> + struct syscon *syscon, *syscon_p;
> + struct resource *res = NULL;
> + struct device *dev = &pdev->dev;
> + struct device_node *np = pdev->dev.of_node;
> + int ret;
> +
> + if (!np) {
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -ENOENT;
> + }
> + ret = early_syscon_probe(np, &syscon_p, res);
> + if (ret) {
> + dev_err(dev, "Syscon probe failed\n");
> + return ret;
> + }
> +
> + syscon = *(struct syscon **)syscon_p;
> +
> + regmap_attach_dev(dev, syscon->regmap, &syscon_regmap_config);
>
> platform_set_drvdata(pdev, syscon);
>
> - dev_info(dev, "regmap %pR registered\n", res);
> + dev_info(dev, "regmap attach device to %pR\n", &syscon->res);
>
> return 0;
> }
> @@ -162,6 +271,21 @@ static const struct platform_device_id syscon_ids[] = {
> { }
> };
>
> +/**
> + * syscon_remove - System controller cleanup function
> + * @pdev: Platform device
> + * Return: 0 always
> + */
> +static int syscon_remove(struct platform_device *pdev)
> +{
> + struct syscon *syscon = platform_get_drvdata(pdev);
> +
> + iounmap(syscon->base);
> + kfree(syscon);
> +
> + return 0;
> +}
> +
> static struct platform_driver syscon_driver = {
> .driver = {
> .name = "syscon",
> @@ -169,6 +293,7 @@ static struct platform_driver syscon_driver = {
> .of_match_table = of_syscon_match,
> },
> .probe = syscon_probe,
> + .remove = syscon_remove,
> .id_table = syscon_ids,
> };
>
> diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
> index 8789fa3..465c092 100644
> --- a/include/linux/mfd/syscon.h
> +++ b/include/linux/mfd/syscon.h
> @@ -24,6 +24,10 @@ extern struct regmap *syscon_regmap_lookup_by_pdevname(const char *s);
> extern struct regmap *syscon_regmap_lookup_by_phandle(
> struct device_node *np,
> const char *property);
> +extern struct regmap *syscon_early_regmap_lookup_by_phandle(
> + struct device_node *np,
> + const char *property);
> +extern void early_syscon_init(void);
> #else
> static inline struct regmap *syscon_node_to_regmap(struct device_node *np)
> {
> @@ -46,6 +50,13 @@ static inline struct regmap *syscon_regmap_lookup_by_phandle(
> {
> return ERR_PTR(-ENOSYS);
> }
> +
> +static struct regmap *syscon_early_regmap_lookup_by_phandle(
> + struct device_node *np,
> + const char *property)
> +{
> + return ERR_PTR(-ENOSYS);
> +}
> #endif
>
> #endif /* __LINUX_MFD_SYSCON_H__ */
> --
> 1.8.2.3
>
Thanks for CC'ing me.
I have tested this patch along with Exynos PMU related changes posted here
https://lkml.org/lkml/2014/4/2/53 and modified it for using Syscon, and it's
working for me.
I have observed one issue during this testing:
Even though we are saying early initialization I could not use
"early_syscon_init" from machine's
"map_io" or "init_early". I observed that "early_syscon_init" failed to called
"early_syscon_probe",
because it can not get any matching node, when I debug further I found that as
"early_syscon_init"
is calling "for_each_matching_node_and_match" and it can't work before
unflatten'ing device tree,
which happens in "setup_arch" a bit late, after "map_io" and "init_early" calls
of machine.
So if I have to use "early_syscon_init" I MUST call it after device tree is
unflattened. It worked for me
when I called it from "init_machine" (from exynos_dt_machine_init), But if
someone needs it at very
early stage then it might not work. So how about using "of_scan_flat_dt" in
"early_syscon_init"?, so that
we can use this functionality at very early stage if required.
--
Best Regards,
Pankaj Dubey
On 04/10/2014 08:20 AM, Pankaj Dubey wrote:
> Hi Michal,
>
> On 04/09/2014 12:00 AM, Michal Simek wrote:
>> Some platforms need to get system controller
>> ready as soon as possible.
>> The patch provides early_syscon_initialization
>> which create early mapping for all syscon compatible
>> devices in early_syscon_probe.
>> Regmap is get via syscon_early_regmap_lookup_by_phandle()
>>
>> Regular device probes attach device to regmap
>> via regmap_attach_dev().
>>
>> For early syscon initialization is necessary to extend
>> struct syscon and provide remove function
>> which unmap all early init structures.
>>
>> Signed-off-by: Michal Simek <[email protected]>
>> ---
>>
>> Changes in v3:
>> - Keep backward compatibility for platform drivers and test it
>> - Use only one probe method which is early_syscon_probe
>> suggested by Lee Jones. Do not force anybody to call early_syscon_init
>> - Add kernel-doc description
>>
>> Changes in v2:
>> - Fix bad logic in early_syscon_probe
>> - Fix compilation failure for x86_64 reported by zero day testing system
>> - Regmap change available here
>> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git tags/nodev
>>
>> drivers/mfd/syscon.c | 159 ++++++++++++++++++++++++++++++++++++++++-----
>> include/linux/mfd/syscon.h | 11 ++++
>> 2 files changed, 153 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
>> index 71841f9..8e2ff88 100644
>> --- a/drivers/mfd/syscon.c
>> +++ b/drivers/mfd/syscon.c
>> @@ -20,12 +20,15 @@
>> #include <linux/of_platform.h>
>> #include <linux/platform_device.h>
>> #include <linux/regmap.h>
>> +#include <linux/slab.h>
>> #include <linux/mfd/syscon.h>
>>
>> static struct platform_driver syscon_driver;
>>
>> struct syscon {
>> + void __iomem *base;
>> struct regmap *regmap;
>> + struct resource res;
>> };
>>
>> static int syscon_match_node(struct device *dev, void *data)
>> @@ -95,6 +98,30 @@ struct regmap *syscon_regmap_lookup_by_pdevname(const char *s)
>> }
>> EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
>>
>> +/**
>> + * syscon_early_regmap_lookup_by_phandle - Early phandle lookup function
>> + * @np: device_node pointer
>> + * @property: property name which handle system controller phandle
>> + * Return: regmap pointer, an error pointer otherwise
>> + */
>> +struct regmap *syscon_early_regmap_lookup_by_phandle(struct device_node *np,
>> + const char *property)
>> +{
>> + struct device_node *syscon_np;
>> + struct syscon *syscon;
>> +
>> + syscon_np = of_parse_phandle(np, property, 0);
>> + if (!syscon_np)
>> + return ERR_PTR(-ENODEV);
>> +
>> + syscon = syscon_np->data;
>> +
>> + of_node_put(syscon_np);
>> +
>> + return syscon->regmap;
>> +}
>> +EXPORT_SYMBOL_GPL(syscon_early_regmap_lookup_by_phandle);
>> +
>> struct regmap *syscon_regmap_lookup_by_phandle(struct device_node *np,
>> const char *property)
>> {
>> @@ -123,36 +150,118 @@ static struct regmap_config syscon_regmap_config = {
>> .reg_stride = 4,
>> };
>>
>> -static int syscon_probe(struct platform_device *pdev)
>> +/**
>> + * early_syscon_probe - Early system controller probe method
>> + * @np: device_node pointer
>> + * @syscon_p: syscon pointer
>> + * @res: device IO resource
>> + * Return: 0 if successful, a negative error code otherwise
>> + */
>> +static int early_syscon_probe(struct device_node *np, struct syscon **syscon_p,
>> + struct resource *res)
>> {
>> - struct device *dev = &pdev->dev;
>> struct syscon *syscon;
>> - struct resource *res;
>> - void __iomem *base;
>> + int ret;
>> +
>> + if (np && np->data) {
>> + pr_debug("Early syscon was called\n");
>> + *syscon_p = (struct syscon *)&np->data;
>> + return 0;
>> + }
>>
>> - syscon = devm_kzalloc(dev, sizeof(*syscon), GFP_KERNEL);
>> + syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
>> if (!syscon)
>> return -ENOMEM;
>>
>> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> - if (!res)
>> - return -ENOENT;
>> + *syscon_p = (struct syscon *)&syscon;
>>
>> - base = devm_ioremap(dev, res->start, resource_size(res));
>> - if (!base)
>> - return -ENOMEM;
>> + if (!res && np) {
>> + if (of_address_to_resource(np, 0, &syscon->res)) {
>> + ret = -EINVAL;
>> + goto alloc;
>> + }
>> +
>> + np->data = syscon;
>> + of_node_put(np);
>> + } else {
>> + syscon->res = *res;
>> + }
>>
>> - syscon_regmap_config.max_register = res->end - res->start - 3;
>> - syscon->regmap = devm_regmap_init_mmio(dev, base,
>> - &syscon_regmap_config);
>> + syscon->base = ioremap(syscon->res.start, resource_size(&syscon->res));
>> + if (!syscon->base) {
>> + pr_err("%s: Unable to map I/O memory\n", __func__);
>> + ret = PTR_ERR(syscon->base);
>> + goto alloc;
>> + }
>> +
>> + syscon_regmap_config.max_register = syscon->res.end -
>> + syscon->res.start - 3;
>> + syscon->regmap = regmap_init_mmio(NULL, syscon->base,
>> + &syscon_regmap_config);
>> if (IS_ERR(syscon->regmap)) {
>> - dev_err(dev, "regmap init failed\n");
>> - return PTR_ERR(syscon->regmap);
>> + pr_err("regmap init failed\n");
>> + ret = PTR_ERR(syscon->regmap);
>> + goto iomap;
>> }
>> + if (np)
>> + pr_info("syscon: %s regmap %pR registered\n", np->name,
>> + &syscon->res);
>> +
>> + return 0;
>> +
>> +iomap:
>> + iounmap(syscon->base);
>> +alloc:
>> + kfree(syscon);
>> +
>> + return ret;
>> +}
>> +
>> +/**
>> + * early_syscon_init - Early system controller initialization
>> + */
>> +void __init early_syscon_init(void)
>> +{
>> + struct device_node *np;
>> + struct syscon *syscon = NULL;
>> +
>> + for_each_matching_node_and_match(np, of_syscon_match, NULL) {
>> + if (early_syscon_probe(np, &syscon, NULL))
>> + BUG();
>> + }
>> +}
>> +
>> +/**
>> + * syscon_probe - System controller probe method
>> + * @pdev: Platform device
>> + * Return: 0 if successful, a negative error code otherwise
>> + */
>> +static int syscon_probe(struct platform_device *pdev)
>> +{
>> + struct syscon *syscon, *syscon_p;
>> + struct resource *res = NULL;
>> + struct device *dev = &pdev->dev;
>> + struct device_node *np = pdev->dev.of_node;
>> + int ret;
>> +
>> + if (!np) {
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + if (!res)
>> + return -ENOENT;
>> + }
>> + ret = early_syscon_probe(np, &syscon_p, res);
>> + if (ret) {
>> + dev_err(dev, "Syscon probe failed\n");
>> + return ret;
>> + }
>> +
>> + syscon = *(struct syscon **)syscon_p;
>> +
>> + regmap_attach_dev(dev, syscon->regmap, &syscon_regmap_config);
>>
>> platform_set_drvdata(pdev, syscon);
>>
>> - dev_info(dev, "regmap %pR registered\n", res);
>> + dev_info(dev, "regmap attach device to %pR\n", &syscon->res);
>>
>> return 0;
>> }
>> @@ -162,6 +271,21 @@ static const struct platform_device_id syscon_ids[] = {
>> { }
>> };
>>
>> +/**
>> + * syscon_remove - System controller cleanup function
>> + * @pdev: Platform device
>> + * Return: 0 always
>> + */
>> +static int syscon_remove(struct platform_device *pdev)
>> +{
>> + struct syscon *syscon = platform_get_drvdata(pdev);
>> +
>> + iounmap(syscon->base);
>> + kfree(syscon);
>> +
>> + return 0;
>> +}
>> +
>> static struct platform_driver syscon_driver = {
>> .driver = {
>> .name = "syscon",
>> @@ -169,6 +293,7 @@ static struct platform_driver syscon_driver = {
>> .of_match_table = of_syscon_match,
>> },
>> .probe = syscon_probe,
>> + .remove = syscon_remove,
>> .id_table = syscon_ids,
>> };
>>
>> diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
>> index 8789fa3..465c092 100644
>> --- a/include/linux/mfd/syscon.h
>> +++ b/include/linux/mfd/syscon.h
>> @@ -24,6 +24,10 @@ extern struct regmap *syscon_regmap_lookup_by_pdevname(const char *s);
>> extern struct regmap *syscon_regmap_lookup_by_phandle(
>> struct device_node *np,
>> const char *property);
>> +extern struct regmap *syscon_early_regmap_lookup_by_phandle(
>> + struct device_node *np,
>> + const char *property);
>> +extern void early_syscon_init(void);
>> #else
>> static inline struct regmap *syscon_node_to_regmap(struct device_node *np)
>> {
>> @@ -46,6 +50,13 @@ static inline struct regmap *syscon_regmap_lookup_by_phandle(
>> {
>> return ERR_PTR(-ENOSYS);
>> }
>> +
>> +static struct regmap *syscon_early_regmap_lookup_by_phandle(
>> + struct device_node *np,
>> + const char *property)
>> +{
>> + return ERR_PTR(-ENOSYS);
>> +}
>> #endif
>>
>> #endif /* __LINUX_MFD_SYSCON_H__ */
>> --
>> 1.8.2.3
>>
> Thanks for CC'ing me.
no problem.
>
> I have tested this patch along with Exynos PMU related changes posted here
> https://lkml.org/lkml/2014/4/2/53 and modified it for using Syscon, and it's working for me.
great.
> I have observed one issue during this testing:
>
> Even though we are saying early initialization I could not use "early_syscon_init" from machine's
> "map_io" or "init_early". I observed that "early_syscon_init" failed to called "early_syscon_probe",
> because it can not get any matching node, when I debug further I found that as "early_syscon_init"
> is calling "for_each_matching_node_and_match" and it can't work before unflatten'ing device tree,
> which happens in "setup_arch" a bit late, after "map_io" and "init_early" calls of machine.
>
> So if I have to use "early_syscon_init" I MUST call it after device tree is unflattened. It worked for me
> when I called it from "init_machine" (from exynos_dt_machine_init), But if someone needs it at very
> early stage then it might not work. So how about using "of_scan_flat_dt" in "early_syscon_init"?, so that
> we can use this functionality at very early stage if required.
Yes you are right. I have discussed this with Arnd and for Zynq there is no need to call it so early
that's why using this function is fine. Arnd wasn't sure if there is any need to call it before unflattening
that's why I didn't try to solve this case.
Can you send me that patch? I will test it on Zynq and if it is working, let's include it to v4.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
On 04/10/2014 08:17 AM, Michal Simek wrote:
> On 04/10/2014 08:20 AM, Pankaj Dubey wrote:
>> Hi Michal,
>>
>> On 04/09/2014 12:00 AM, Michal Simek wrote:
>>> Some platforms need to get system controller
>>> ready as soon as possible.
>>> The patch provides early_syscon_initialization
>>> which create early mapping for all syscon compatible
>>> devices in early_syscon_probe.
>>> Regmap is get via syscon_early_regmap_lookup_by_phandle()
>>>
>>> Regular device probes attach device to regmap
>>> via regmap_attach_dev().
>>>
>>> For early syscon initialization is necessary to extend
>>> struct syscon and provide remove function
>>> which unmap all early init structures.
>>>
>>> Signed-off-by: Michal Simek <[email protected]>
>>> ---
>>>
>>> Changes in v3:
>>> - Keep backward compatibility for platform drivers and test it
>>> - Use only one probe method which is early_syscon_probe
>>> suggested by Lee Jones. Do not force anybody to call early_syscon_init
>>> - Add kernel-doc description
>>>
>>> Changes in v2:
>>> - Fix bad logic in early_syscon_probe
>>> - Fix compilation failure for x86_64 reported by zero day testing system
>>> - Regmap change available here
>>> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git tags/nodev
>>>
>>> drivers/mfd/syscon.c | 159 ++++++++++++++++++++++++++++++++++++++++-----
>>> include/linux/mfd/syscon.h | 11 ++++
>>> 2 files changed, 153 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
>>> index 71841f9..8e2ff88 100644
>>> --- a/drivers/mfd/syscon.c
>>> +++ b/drivers/mfd/syscon.c
>>> @@ -20,12 +20,15 @@
>>> #include <linux/of_platform.h>
>>> #include <linux/platform_device.h>
>>> #include <linux/regmap.h>
>>> +#include <linux/slab.h>
>>> #include <linux/mfd/syscon.h>
>>>
>>> static struct platform_driver syscon_driver;
>>>
>>> struct syscon {
>>> + void __iomem *base;
>>> struct regmap *regmap;
>>> + struct resource res;
>>> };
>>>
>>> static int syscon_match_node(struct device *dev, void *data)
>>> @@ -95,6 +98,30 @@ struct regmap *syscon_regmap_lookup_by_pdevname(const char *s)
>>> }
>>> EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
>>>
>>> +/**
>>> + * syscon_early_regmap_lookup_by_phandle - Early phandle lookup function
>>> + * @np: device_node pointer
>>> + * @property: property name which handle system controller phandle
>>> + * Return: regmap pointer, an error pointer otherwise
>>> + */
>>> +struct regmap *syscon_early_regmap_lookup_by_phandle(struct device_node *np,
>>> + const char *property)
>>> +{
>>> + struct device_node *syscon_np;
>>> + struct syscon *syscon;
>>> +
>>> + syscon_np = of_parse_phandle(np, property, 0);
>>> + if (!syscon_np)
>>> + return ERR_PTR(-ENODEV);
>>> +
>>> + syscon = syscon_np->data;
>>> +
>>> + of_node_put(syscon_np);
>>> +
>>> + return syscon->regmap;
>>> +}
>>> +EXPORT_SYMBOL_GPL(syscon_early_regmap_lookup_by_phandle);
>>> +
>>> struct regmap *syscon_regmap_lookup_by_phandle(struct device_node *np,
>>> const char *property)
>>> {
>>> @@ -123,36 +150,118 @@ static struct regmap_config syscon_regmap_config = {
>>> .reg_stride = 4,
>>> };
>>>
>>> -static int syscon_probe(struct platform_device *pdev)
>>> +/**
>>> + * early_syscon_probe - Early system controller probe method
>>> + * @np: device_node pointer
>>> + * @syscon_p: syscon pointer
>>> + * @res: device IO resource
>>> + * Return: 0 if successful, a negative error code otherwise
>>> + */
>>> +static int early_syscon_probe(struct device_node *np, struct syscon **syscon_p,
>>> + struct resource *res)
>>> {
>>> - struct device *dev = &pdev->dev;
>>> struct syscon *syscon;
>>> - struct resource *res;
>>> - void __iomem *base;
>>> + int ret;
>>> +
>>> + if (np && np->data) {
>>> + pr_debug("Early syscon was called\n");
>>> + *syscon_p = (struct syscon *)&np->data;
>>> + return 0;
>>> + }
>>>
>>> - syscon = devm_kzalloc(dev, sizeof(*syscon), GFP_KERNEL);
>>> + syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
>>> if (!syscon)
>>> return -ENOMEM;
>>>
>>> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> - if (!res)
>>> - return -ENOENT;
>>> + *syscon_p = (struct syscon *)&syscon;
>>>
>>> - base = devm_ioremap(dev, res->start, resource_size(res));
>>> - if (!base)
>>> - return -ENOMEM;
>>> + if (!res && np) {
>>> + if (of_address_to_resource(np, 0, &syscon->res)) {
>>> + ret = -EINVAL;
>>> + goto alloc;
>>> + }
>>> +
>>> + np->data = syscon;
>>> + of_node_put(np);
>>> + } else {
>>> + syscon->res = *res;
>>> + }
>>>
>>> - syscon_regmap_config.max_register = res->end - res->start - 3;
>>> - syscon->regmap = devm_regmap_init_mmio(dev, base,
>>> - &syscon_regmap_config);
>>> + syscon->base = ioremap(syscon->res.start, resource_size(&syscon->res));
>>> + if (!syscon->base) {
>>> + pr_err("%s: Unable to map I/O memory\n", __func__);
>>> + ret = PTR_ERR(syscon->base);
>>> + goto alloc;
>>> + }
>>> +
>>> + syscon_regmap_config.max_register = syscon->res.end -
>>> + syscon->res.start - 3;
>>> + syscon->regmap = regmap_init_mmio(NULL, syscon->base,
>>> + &syscon_regmap_config);
>>> if (IS_ERR(syscon->regmap)) {
>>> - dev_err(dev, "regmap init failed\n");
>>> - return PTR_ERR(syscon->regmap);
>>> + pr_err("regmap init failed\n");
>>> + ret = PTR_ERR(syscon->regmap);
>>> + goto iomap;
>>> }
>>> + if (np)
>>> + pr_info("syscon: %s regmap %pR registered\n", np->name,
>>> + &syscon->res);
>>> +
>>> + return 0;
>>> +
>>> +iomap:
>>> + iounmap(syscon->base);
>>> +alloc:
>>> + kfree(syscon);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +/**
>>> + * early_syscon_init - Early system controller initialization
>>> + */
>>> +void __init early_syscon_init(void)
>>> +{
>>> + struct device_node *np;
>>> + struct syscon *syscon = NULL;
>>> +
>>> + for_each_matching_node_and_match(np, of_syscon_match, NULL) {
>>> + if (early_syscon_probe(np, &syscon, NULL))
>>> + BUG();
>>> + }
>>> +}
>>> +
>>> +/**
>>> + * syscon_probe - System controller probe method
>>> + * @pdev: Platform device
>>> + * Return: 0 if successful, a negative error code otherwise
>>> + */
>>> +static int syscon_probe(struct platform_device *pdev)
>>> +{
>>> + struct syscon *syscon, *syscon_p;
>>> + struct resource *res = NULL;
>>> + struct device *dev = &pdev->dev;
>>> + struct device_node *np = pdev->dev.of_node;
>>> + int ret;
>>> +
>>> + if (!np) {
>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> + if (!res)
>>> + return -ENOENT;
>>> + }
>>> + ret = early_syscon_probe(np, &syscon_p, res);
>>> + if (ret) {
>>> + dev_err(dev, "Syscon probe failed\n");
>>> + return ret;
>>> + }
>>> +
>>> + syscon = *(struct syscon **)syscon_p;
>>> +
>>> + regmap_attach_dev(dev, syscon->regmap, &syscon_regmap_config);
>>>
>>> platform_set_drvdata(pdev, syscon);
>>>
>>> - dev_info(dev, "regmap %pR registered\n", res);
>>> + dev_info(dev, "regmap attach device to %pR\n", &syscon->res);
>>>
>>> return 0;
>>> }
>>> @@ -162,6 +271,21 @@ static const struct platform_device_id syscon_ids[] = {
>>> { }
>>> };
>>>
>>> +/**
>>> + * syscon_remove - System controller cleanup function
>>> + * @pdev: Platform device
>>> + * Return: 0 always
>>> + */
>>> +static int syscon_remove(struct platform_device *pdev)
>>> +{
>>> + struct syscon *syscon = platform_get_drvdata(pdev);
>>> +
>>> + iounmap(syscon->base);
>>> + kfree(syscon);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static struct platform_driver syscon_driver = {
>>> .driver = {
>>> .name = "syscon",
>>> @@ -169,6 +293,7 @@ static struct platform_driver syscon_driver = {
>>> .of_match_table = of_syscon_match,
>>> },
>>> .probe = syscon_probe,
>>> + .remove = syscon_remove,
>>> .id_table = syscon_ids,
>>> };
>>>
>>> diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
>>> index 8789fa3..465c092 100644
>>> --- a/include/linux/mfd/syscon.h
>>> +++ b/include/linux/mfd/syscon.h
>>> @@ -24,6 +24,10 @@ extern struct regmap *syscon_regmap_lookup_by_pdevname(const char *s);
>>> extern struct regmap *syscon_regmap_lookup_by_phandle(
>>> struct device_node *np,
>>> const char *property);
>>> +extern struct regmap *syscon_early_regmap_lookup_by_phandle(
>>> + struct device_node *np,
>>> + const char *property);
>>> +extern void early_syscon_init(void);
>>> #else
>>> static inline struct regmap *syscon_node_to_regmap(struct device_node *np)
>>> {
>>> @@ -46,6 +50,13 @@ static inline struct regmap *syscon_regmap_lookup_by_phandle(
>>> {
>>> return ERR_PTR(-ENOSYS);
>>> }
>>> +
>>> +static struct regmap *syscon_early_regmap_lookup_by_phandle(
>>> + struct device_node *np,
>>> + const char *property)
>>> +{
>>> + return ERR_PTR(-ENOSYS);
>>> +}
>>> #endif
>>>
>>> #endif /* __LINUX_MFD_SYSCON_H__ */
>>> --
>>> 1.8.2.3
>>>
>> Thanks for CC'ing me.
>
> no problem.
>
>>
>> I have tested this patch along with Exynos PMU related changes posted here
>> https://lkml.org/lkml/2014/4/2/53 and modified it for using Syscon, and it's working for me.
>
> great.
>
>
>> I have observed one issue during this testing:
>>
>> Even though we are saying early initialization I could not use "early_syscon_init" from machine's
>> "map_io" or "init_early". I observed that "early_syscon_init" failed to called "early_syscon_probe",
>> because it can not get any matching node, when I debug further I found that as "early_syscon_init"
>> is calling "for_each_matching_node_and_match" and it can't work before unflatten'ing device tree,
>> which happens in "setup_arch" a bit late, after "map_io" and "init_early" calls of machine.
>>
>> So if I have to use "early_syscon_init" I MUST call it after device tree is unflattened. It worked for me
>> when I called it from "init_machine" (from exynos_dt_machine_init), But if someone needs it at very
>> early stage then it might not work. So how about using "of_scan_flat_dt" in "early_syscon_init"?, so that
>> we can use this functionality at very early stage if required.
>
> Yes you are right. I have discussed this with Arnd and for Zynq there is no need to call it so early
> that's why using this function is fine. Arnd wasn't sure if there is any need to call it before unflattening
> that's why I didn't try to solve this case.
>
> Can you send me that patch? I will test it on Zynq and if it is working, let's include it to v4.
Pankaj: Any update on this? I think that your patch can be applied on the top of this one.
Lee: Can you please look at these changes? It is around for a while and I would like to close it.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
On 04/23/2014 04:27 PM, Michal Simek wrote:
> On 04/10/2014 08:17 AM, Michal Simek wrote:
>> On 04/10/2014 08:20 AM, Pankaj Dubey wrote:
>>> Hi Michal,
>>>
>>> On 04/09/2014 12:00 AM, Michal Simek wrote:
>>>> Some platforms need to get system controller
>>>> ready as soon as possible.
>>>> The patch provides early_syscon_initialization
>>>> which create early mapping for all syscon compatible
>>>> devices in early_syscon_probe.
>>>> Regmap is get via syscon_early_regmap_lookup_by_phandle()
>>>>
>>>> Regular device probes attach device to regmap
>>>> via regmap_attach_dev().
>>>>
>>>> For early syscon initialization is necessary to extend
>>>> struct syscon and provide remove function
>>>> which unmap all early init structures.
>>>>
>>>> Signed-off-by: Michal Simek <[email protected]>
>>>> ---
>>>>
>>>> Changes in v3:
>>>> - Keep backward compatibility for platform drivers and test it
>>>> - Use only one probe method which is early_syscon_probe
>>>> suggested by Lee Jones. Do not force anybody to call early_syscon_init
>>>> - Add kernel-doc description
>>>>
>>>> Changes in v2:
>>>> - Fix bad logic in early_syscon_probe
>>>> - Fix compilation failure for x86_64 reported by zero day testing system
>>>> - Regmap change available here
>>>> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git tags/nodev
>>>>
>>>> drivers/mfd/syscon.c | 159 ++++++++++++++++++++++++++++++++++++++++-----
>>>> include/linux/mfd/syscon.h | 11 ++++
>>>> 2 files changed, 153 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
>>>> index 71841f9..8e2ff88 100644
>>>> --- a/drivers/mfd/syscon.c
>>>> +++ b/drivers/mfd/syscon.c
>>>> @@ -20,12 +20,15 @@
>>>> #include <linux/of_platform.h>
>>>> #include <linux/platform_device.h>
>>>> #include <linux/regmap.h>
>>>> +#include <linux/slab.h>
>>>> #include <linux/mfd/syscon.h>
>>>>
>>>> static struct platform_driver syscon_driver;
>>>>
>>>> struct syscon {
>>>> + void __iomem *base;
>>>> struct regmap *regmap;
>>>> + struct resource res;
>>>> };
>>>>
>>>> static int syscon_match_node(struct device *dev, void *data)
>>>> @@ -95,6 +98,30 @@ struct regmap *syscon_regmap_lookup_by_pdevname(const char *s)
>>>> }
>>>> EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
>>>>
>>>> +/**
>>>> + * syscon_early_regmap_lookup_by_phandle - Early phandle lookup function
>>>> + * @np: device_node pointer
>>>> + * @property: property name which handle system controller phandle
>>>> + * Return: regmap pointer, an error pointer otherwise
>>>> + */
>>>> +struct regmap *syscon_early_regmap_lookup_by_phandle(struct device_node *np,
>>>> + const char *property)
>>>> +{
>>>> + struct device_node *syscon_np;
>>>> + struct syscon *syscon;
>>>> +
>>>> + syscon_np = of_parse_phandle(np, property, 0);
>>>> + if (!syscon_np)
>>>> + return ERR_PTR(-ENODEV);
>>>> +
>>>> + syscon = syscon_np->data;
>>>> +
>>>> + of_node_put(syscon_np);
>>>> +
>>>> + return syscon->regmap;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(syscon_early_regmap_lookup_by_phandle);
>>>> +
>>>> struct regmap *syscon_regmap_lookup_by_phandle(struct device_node *np,
>>>> const char *property)
>>>> {
>>>> @@ -123,36 +150,118 @@ static struct regmap_config syscon_regmap_config = {
>>>> .reg_stride = 4,
>>>> };
>>>>
>>>> -static int syscon_probe(struct platform_device *pdev)
>>>> +/**
>>>> + * early_syscon_probe - Early system controller probe method
>>>> + * @np: device_node pointer
>>>> + * @syscon_p: syscon pointer
>>>> + * @res: device IO resource
>>>> + * Return: 0 if successful, a negative error code otherwise
>>>> + */
>>>> +static int early_syscon_probe(struct device_node *np, struct syscon **syscon_p,
>>>> + struct resource *res)
>>>> {
>>>> - struct device *dev = &pdev->dev;
>>>> struct syscon *syscon;
>>>> - struct resource *res;
>>>> - void __iomem *base;
>>>> + int ret;
>>>> +
>>>> + if (np && np->data) {
>>>> + pr_debug("Early syscon was called\n");
>>>> + *syscon_p = (struct syscon *)&np->data;
>>>> + return 0;
>>>> + }
>>>>
>>>> - syscon = devm_kzalloc(dev, sizeof(*syscon), GFP_KERNEL);
>>>> + syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
>>>> if (!syscon)
>>>> return -ENOMEM;
>>>>
>>>> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> - if (!res)
>>>> - return -ENOENT;
>>>> + *syscon_p = (struct syscon *)&syscon;
>>>>
>>>> - base = devm_ioremap(dev, res->start, resource_size(res));
>>>> - if (!base)
>>>> - return -ENOMEM;
>>>> + if (!res && np) {
>>>> + if (of_address_to_resource(np, 0, &syscon->res)) {
>>>> + ret = -EINVAL;
>>>> + goto alloc;
>>>> + }
>>>> +
>>>> + np->data = syscon;
>>>> + of_node_put(np);
>>>> + } else {
>>>> + syscon->res = *res;
>>>> + }
>>>>
>>>> - syscon_regmap_config.max_register = res->end - res->start - 3;
>>>> - syscon->regmap = devm_regmap_init_mmio(dev, base,
>>>> - &syscon_regmap_config);
>>>> + syscon->base = ioremap(syscon->res.start, resource_size(&syscon->res));
>>>> + if (!syscon->base) {
>>>> + pr_err("%s: Unable to map I/O memory\n", __func__);
>>>> + ret = PTR_ERR(syscon->base);
>>>> + goto alloc;
>>>> + }
>>>> +
>>>> + syscon_regmap_config.max_register = syscon->res.end -
>>>> + syscon->res.start - 3;
>>>> + syscon->regmap = regmap_init_mmio(NULL, syscon->base,
>>>> + &syscon_regmap_config);
>>>> if (IS_ERR(syscon->regmap)) {
>>>> - dev_err(dev, "regmap init failed\n");
>>>> - return PTR_ERR(syscon->regmap);
>>>> + pr_err("regmap init failed\n");
>>>> + ret = PTR_ERR(syscon->regmap);
>>>> + goto iomap;
>>>> }
>>>> + if (np)
>>>> + pr_info("syscon: %s regmap %pR registered\n", np->name,
>>>> + &syscon->res);
>>>> +
>>>> + return 0;
>>>> +
>>>> +iomap:
>>>> + iounmap(syscon->base);
>>>> +alloc:
>>>> + kfree(syscon);
>>>> +
>>>> + return ret;
>>>> +}
>>>> +
>>>> +/**
>>>> + * early_syscon_init - Early system controller initialization
>>>> + */
>>>> +void __init early_syscon_init(void)
>>>> +{
>>>> + struct device_node *np;
>>>> + struct syscon *syscon = NULL;
>>>> +
>>>> + for_each_matching_node_and_match(np, of_syscon_match, NULL) {
>>>> + if (early_syscon_probe(np, &syscon, NULL))
>>>> + BUG();
>>>> + }
>>>> +}
>>>> +
>>>> +/**
>>>> + * syscon_probe - System controller probe method
>>>> + * @pdev: Platform device
>>>> + * Return: 0 if successful, a negative error code otherwise
>>>> + */
>>>> +static int syscon_probe(struct platform_device *pdev)
>>>> +{
>>>> + struct syscon *syscon, *syscon_p;
>>>> + struct resource *res = NULL;
>>>> + struct device *dev = &pdev->dev;
>>>> + struct device_node *np = pdev->dev.of_node;
>>>> + int ret;
>>>> +
>>>> + if (!np) {
>>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> + if (!res)
>>>> + return -ENOENT;
>>>> + }
>>>> + ret = early_syscon_probe(np, &syscon_p, res);
>>>> + if (ret) {
>>>> + dev_err(dev, "Syscon probe failed\n");
>>>> + return ret;
>>>> + }
>>>> +
>>>> + syscon = *(struct syscon **)syscon_p;
>>>> +
>>>> + regmap_attach_dev(dev, syscon->regmap, &syscon_regmap_config);
>>>>
>>>> platform_set_drvdata(pdev, syscon);
>>>>
>>>> - dev_info(dev, "regmap %pR registered\n", res);
>>>> + dev_info(dev, "regmap attach device to %pR\n", &syscon->res);
>>>>
>>>> return 0;
>>>> }
>>>> @@ -162,6 +271,21 @@ static const struct platform_device_id syscon_ids[] = {
>>>> { }
>>>> };
>>>>
>>>> +/**
>>>> + * syscon_remove - System controller cleanup function
>>>> + * @pdev: Platform device
>>>> + * Return: 0 always
>>>> + */
>>>> +static int syscon_remove(struct platform_device *pdev)
>>>> +{
>>>> + struct syscon *syscon = platform_get_drvdata(pdev);
>>>> +
>>>> + iounmap(syscon->base);
>>>> + kfree(syscon);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> static struct platform_driver syscon_driver = {
>>>> .driver = {
>>>> .name = "syscon",
>>>> @@ -169,6 +293,7 @@ static struct platform_driver syscon_driver = {
>>>> .of_match_table = of_syscon_match,
>>>> },
>>>> .probe = syscon_probe,
>>>> + .remove = syscon_remove,
>>>> .id_table = syscon_ids,
>>>> };
>>>>
>>>> diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
>>>> index 8789fa3..465c092 100644
>>>> --- a/include/linux/mfd/syscon.h
>>>> +++ b/include/linux/mfd/syscon.h
>>>> @@ -24,6 +24,10 @@ extern struct regmap *syscon_regmap_lookup_by_pdevname(const char *s);
>>>> extern struct regmap *syscon_regmap_lookup_by_phandle(
>>>> struct device_node *np,
>>>> const char *property);
>>>> +extern struct regmap *syscon_early_regmap_lookup_by_phandle(
>>>> + struct device_node *np,
>>>> + const char *property);
>>>> +extern void early_syscon_init(void);
>>>> #else
>>>> static inline struct regmap *syscon_node_to_regmap(struct device_node *np)
>>>> {
>>>> @@ -46,6 +50,13 @@ static inline struct regmap *syscon_regmap_lookup_by_phandle(
>>>> {
>>>> return ERR_PTR(-ENOSYS);
>>>> }
>>>> +
>>>> +static struct regmap *syscon_early_regmap_lookup_by_phandle(
>>>> + struct device_node *np,
>>>> + const char *property)
>>>> +{
>>>> + return ERR_PTR(-ENOSYS);
>>>> +}
>>>> #endif
>>>>
>>>> #endif /* __LINUX_MFD_SYSCON_H__ */
>>>> --
>>>> 1.8.2.3
>>>>
>>> Thanks for CC'ing me.
>> no problem.
>>
>>> I have tested this patch along with Exynos PMU related changes posted here
>>> https://lkml.org/lkml/2014/4/2/53 and modified it for using Syscon, and it's working for me.
>> great.
>>
>>
>>> I have observed one issue during this testing:
>>>
>>> Even though we are saying early initialization I could not use "early_syscon_init" from machine's
>>> "map_io" or "init_early". I observed that "early_syscon_init" failed to called "early_syscon_probe",
>>> because it can not get any matching node, when I debug further I found that as "early_syscon_init"
>>> is calling "for_each_matching_node_and_match" and it can't work before unflatten'ing device tree,
>>> which happens in "setup_arch" a bit late, after "map_io" and "init_early" calls of machine.
>>>
>>> So if I have to use "early_syscon_init" I MUST call it after device tree is unflattened. It worked for me
>>> when I called it from "init_machine" (from exynos_dt_machine_init), But if someone needs it at very
>>> early stage then it might not work. So how about using "of_scan_flat_dt" in "early_syscon_init"?, so that
>>> we can use this functionality at very early stage if required.
>> Yes you are right. I have discussed this with Arnd and for Zynq there is no need to call it so early
>> that's why using this function is fine. Arnd wasn't sure if there is any need to call it before unflattening
>> that's why I didn't try to solve this case.
>>
>> Can you send me that patch? I will test it on Zynq and if it is working, let's include it to v4.
> Pankaj: Any update on this? I think that your patch can be applied on the top of this one.
Hi Michal,
Sorry for late reply, I was a bit busy with some official assignments.
Usage of "of_scan_flat_dt" in "early_syscon_init" was just a suggestion from my
side, and I have
not worked on it till now. As I mentioned your patches worked for me if I call
them a bit late in "init_machine".
I do not see as such any issues, until some one needs it before DT gets unflattened.
>
> Lee: Can you please look at these changes? It is around for a while and I would like to close it.
>
> Thanks,
> Michal
>
>
>
--
Best Regards,
Pankaj Dubey
On 04/23/2014 10:22 AM, Pankaj Dubey wrote:
> On 04/23/2014 04:27 PM, Michal Simek wrote:
>> On 04/10/2014 08:17 AM, Michal Simek wrote:
>>> On 04/10/2014 08:20 AM, Pankaj Dubey wrote:
>>>> Hi Michal,
>>>>
>>>> On 04/09/2014 12:00 AM, Michal Simek wrote:
>>>>> Some platforms need to get system controller
>>>>> ready as soon as possible.
>>>>> The patch provides early_syscon_initialization
>>>>> which create early mapping for all syscon compatible
>>>>> devices in early_syscon_probe.
>>>>> Regmap is get via syscon_early_regmap_lookup_by_phandle()
>>>>>
>>>>> Regular device probes attach device to regmap
>>>>> via regmap_attach_dev().
>>>>>
>>>>> For early syscon initialization is necessary to extend
>>>>> struct syscon and provide remove function
>>>>> which unmap all early init structures.
>>>>>
>>>>> Signed-off-by: Michal Simek <[email protected]>
>>>>> ---
>>>>>
>>>>> Changes in v3:
>>>>> - Keep backward compatibility for platform drivers and test it
>>>>> - Use only one probe method which is early_syscon_probe
>>>>> suggested by Lee Jones. Do not force anybody to call early_syscon_init
>>>>> - Add kernel-doc description
>>>>>
>>>>> Changes in v2:
>>>>> - Fix bad logic in early_syscon_probe
>>>>> - Fix compilation failure for x86_64 reported by zero day testing system
>>>>> - Regmap change available here
>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git tags/nodev
>>>>>
>>>>> drivers/mfd/syscon.c | 159 ++++++++++++++++++++++++++++++++++++++++-----
>>>>> include/linux/mfd/syscon.h | 11 ++++
>>>>> 2 files changed, 153 insertions(+), 17 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
>>>>> index 71841f9..8e2ff88 100644
>>>>> --- a/drivers/mfd/syscon.c
>>>>> +++ b/drivers/mfd/syscon.c
>>>>> @@ -20,12 +20,15 @@
>>>>> #include <linux/of_platform.h>
>>>>> #include <linux/platform_device.h>
>>>>> #include <linux/regmap.h>
>>>>> +#include <linux/slab.h>
>>>>> #include <linux/mfd/syscon.h>
>>>>>
>>>>> static struct platform_driver syscon_driver;
>>>>>
>>>>> struct syscon {
>>>>> + void __iomem *base;
>>>>> struct regmap *regmap;
>>>>> + struct resource res;
>>>>> };
>>>>>
>>>>> static int syscon_match_node(struct device *dev, void *data)
>>>>> @@ -95,6 +98,30 @@ struct regmap *syscon_regmap_lookup_by_pdevname(const char *s)
>>>>> }
>>>>> EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
>>>>>
>>>>> +/**
>>>>> + * syscon_early_regmap_lookup_by_phandle - Early phandle lookup function
>>>>> + * @np: device_node pointer
>>>>> + * @property: property name which handle system controller phandle
>>>>> + * Return: regmap pointer, an error pointer otherwise
>>>>> + */
>>>>> +struct regmap *syscon_early_regmap_lookup_by_phandle(struct device_node *np,
>>>>> + const char *property)
>>>>> +{
>>>>> + struct device_node *syscon_np;
>>>>> + struct syscon *syscon;
>>>>> +
>>>>> + syscon_np = of_parse_phandle(np, property, 0);
>>>>> + if (!syscon_np)
>>>>> + return ERR_PTR(-ENODEV);
>>>>> +
>>>>> + syscon = syscon_np->data;
>>>>> +
>>>>> + of_node_put(syscon_np);
>>>>> +
>>>>> + return syscon->regmap;
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(syscon_early_regmap_lookup_by_phandle);
>>>>> +
>>>>> struct regmap *syscon_regmap_lookup_by_phandle(struct device_node *np,
>>>>> const char *property)
>>>>> {
>>>>> @@ -123,36 +150,118 @@ static struct regmap_config syscon_regmap_config = {
>>>>> .reg_stride = 4,
>>>>> };
>>>>>
>>>>> -static int syscon_probe(struct platform_device *pdev)
>>>>> +/**
>>>>> + * early_syscon_probe - Early system controller probe method
>>>>> + * @np: device_node pointer
>>>>> + * @syscon_p: syscon pointer
>>>>> + * @res: device IO resource
>>>>> + * Return: 0 if successful, a negative error code otherwise
>>>>> + */
>>>>> +static int early_syscon_probe(struct device_node *np, struct syscon **syscon_p,
>>>>> + struct resource *res)
>>>>> {
>>>>> - struct device *dev = &pdev->dev;
>>>>> struct syscon *syscon;
>>>>> - struct resource *res;
>>>>> - void __iomem *base;
>>>>> + int ret;
>>>>> +
>>>>> + if (np && np->data) {
>>>>> + pr_debug("Early syscon was called\n");
>>>>> + *syscon_p = (struct syscon *)&np->data;
>>>>> + return 0;
>>>>> + }
>>>>>
>>>>> - syscon = devm_kzalloc(dev, sizeof(*syscon), GFP_KERNEL);
>>>>> + syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
>>>>> if (!syscon)
>>>>> return -ENOMEM;
>>>>>
>>>>> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>> - if (!res)
>>>>> - return -ENOENT;
>>>>> + *syscon_p = (struct syscon *)&syscon;
>>>>>
>>>>> - base = devm_ioremap(dev, res->start, resource_size(res));
>>>>> - if (!base)
>>>>> - return -ENOMEM;
>>>>> + if (!res && np) {
>>>>> + if (of_address_to_resource(np, 0, &syscon->res)) {
>>>>> + ret = -EINVAL;
>>>>> + goto alloc;
>>>>> + }
>>>>> +
>>>>> + np->data = syscon;
>>>>> + of_node_put(np);
>>>>> + } else {
>>>>> + syscon->res = *res;
>>>>> + }
>>>>>
>>>>> - syscon_regmap_config.max_register = res->end - res->start - 3;
>>>>> - syscon->regmap = devm_regmap_init_mmio(dev, base,
>>>>> - &syscon_regmap_config);
>>>>> + syscon->base = ioremap(syscon->res.start, resource_size(&syscon->res));
>>>>> + if (!syscon->base) {
>>>>> + pr_err("%s: Unable to map I/O memory\n", __func__);
>>>>> + ret = PTR_ERR(syscon->base);
>>>>> + goto alloc;
>>>>> + }
>>>>> +
>>>>> + syscon_regmap_config.max_register = syscon->res.end -
>>>>> + syscon->res.start - 3;
>>>>> + syscon->regmap = regmap_init_mmio(NULL, syscon->base,
>>>>> + &syscon_regmap_config);
>>>>> if (IS_ERR(syscon->regmap)) {
>>>>> - dev_err(dev, "regmap init failed\n");
>>>>> - return PTR_ERR(syscon->regmap);
>>>>> + pr_err("regmap init failed\n");
>>>>> + ret = PTR_ERR(syscon->regmap);
>>>>> + goto iomap;
>>>>> }
>>>>> + if (np)
>>>>> + pr_info("syscon: %s regmap %pR registered\n", np->name,
>>>>> + &syscon->res);
>>>>> +
>>>>> + return 0;
>>>>> +
>>>>> +iomap:
>>>>> + iounmap(syscon->base);
>>>>> +alloc:
>>>>> + kfree(syscon);
>>>>> +
>>>>> + return ret;
>>>>> +}
>>>>> +
>>>>> +/**
>>>>> + * early_syscon_init - Early system controller initialization
>>>>> + */
>>>>> +void __init early_syscon_init(void)
>>>>> +{
>>>>> + struct device_node *np;
>>>>> + struct syscon *syscon = NULL;
>>>>> +
>>>>> + for_each_matching_node_and_match(np, of_syscon_match, NULL) {
>>>>> + if (early_syscon_probe(np, &syscon, NULL))
>>>>> + BUG();
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +/**
>>>>> + * syscon_probe - System controller probe method
>>>>> + * @pdev: Platform device
>>>>> + * Return: 0 if successful, a negative error code otherwise
>>>>> + */
>>>>> +static int syscon_probe(struct platform_device *pdev)
>>>>> +{
>>>>> + struct syscon *syscon, *syscon_p;
>>>>> + struct resource *res = NULL;
>>>>> + struct device *dev = &pdev->dev;
>>>>> + struct device_node *np = pdev->dev.of_node;
>>>>> + int ret;
>>>>> +
>>>>> + if (!np) {
>>>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>> + if (!res)
>>>>> + return -ENOENT;
>>>>> + }
>>>>> + ret = early_syscon_probe(np, &syscon_p, res);
>>>>> + if (ret) {
>>>>> + dev_err(dev, "Syscon probe failed\n");
>>>>> + return ret;
>>>>> + }
>>>>> +
>>>>> + syscon = *(struct syscon **)syscon_p;
>>>>> +
>>>>> + regmap_attach_dev(dev, syscon->regmap, &syscon_regmap_config);
>>>>>
>>>>> platform_set_drvdata(pdev, syscon);
>>>>>
>>>>> - dev_info(dev, "regmap %pR registered\n", res);
>>>>> + dev_info(dev, "regmap attach device to %pR\n", &syscon->res);
>>>>>
>>>>> return 0;
>>>>> }
>>>>> @@ -162,6 +271,21 @@ static const struct platform_device_id syscon_ids[] = {
>>>>> { }
>>>>> };
>>>>>
>>>>> +/**
>>>>> + * syscon_remove - System controller cleanup function
>>>>> + * @pdev: Platform device
>>>>> + * Return: 0 always
>>>>> + */
>>>>> +static int syscon_remove(struct platform_device *pdev)
>>>>> +{
>>>>> + struct syscon *syscon = platform_get_drvdata(pdev);
>>>>> +
>>>>> + iounmap(syscon->base);
>>>>> + kfree(syscon);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> static struct platform_driver syscon_driver = {
>>>>> .driver = {
>>>>> .name = "syscon",
>>>>> @@ -169,6 +293,7 @@ static struct platform_driver syscon_driver = {
>>>>> .of_match_table = of_syscon_match,
>>>>> },
>>>>> .probe = syscon_probe,
>>>>> + .remove = syscon_remove,
>>>>> .id_table = syscon_ids,
>>>>> };
>>>>>
>>>>> diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
>>>>> index 8789fa3..465c092 100644
>>>>> --- a/include/linux/mfd/syscon.h
>>>>> +++ b/include/linux/mfd/syscon.h
>>>>> @@ -24,6 +24,10 @@ extern struct regmap *syscon_regmap_lookup_by_pdevname(const char *s);
>>>>> extern struct regmap *syscon_regmap_lookup_by_phandle(
>>>>> struct device_node *np,
>>>>> const char *property);
>>>>> +extern struct regmap *syscon_early_regmap_lookup_by_phandle(
>>>>> + struct device_node *np,
>>>>> + const char *property);
>>>>> +extern void early_syscon_init(void);
>>>>> #else
>>>>> static inline struct regmap *syscon_node_to_regmap(struct device_node *np)
>>>>> {
>>>>> @@ -46,6 +50,13 @@ static inline struct regmap *syscon_regmap_lookup_by_phandle(
>>>>> {
>>>>> return ERR_PTR(-ENOSYS);
>>>>> }
>>>>> +
>>>>> +static struct regmap *syscon_early_regmap_lookup_by_phandle(
>>>>> + struct device_node *np,
>>>>> + const char *property)
>>>>> +{
>>>>> + return ERR_PTR(-ENOSYS);
>>>>> +}
>>>>> #endif
>>>>>
>>>>> #endif /* __LINUX_MFD_SYSCON_H__ */
>>>>> --
>>>>> 1.8.2.3
>>>>>
>>>> Thanks for CC'ing me.
>>> no problem.
>>>
>>>> I have tested this patch along with Exynos PMU related changes posted here
>>>> https://lkml.org/lkml/2014/4/2/53 and modified it for using Syscon, and it's working for me.
>>> great.
>>>
>>>
>>>> I have observed one issue during this testing:
>>>>
>>>> Even though we are saying early initialization I could not use "early_syscon_init" from machine's
>>>> "map_io" or "init_early". I observed that "early_syscon_init" failed to called "early_syscon_probe",
>>>> because it can not get any matching node, when I debug further I found that as "early_syscon_init"
>>>> is calling "for_each_matching_node_and_match" and it can't work before unflatten'ing device tree,
>>>> which happens in "setup_arch" a bit late, after "map_io" and "init_early" calls of machine.
>>>>
>>>> So if I have to use "early_syscon_init" I MUST call it after device tree is unflattened. It worked for me
>>>> when I called it from "init_machine" (from exynos_dt_machine_init), But if someone needs it at very
>>>> early stage then it might not work. So how about using "of_scan_flat_dt" in "early_syscon_init"?, so that
>>>> we can use this functionality at very early stage if required.
>>> Yes you are right. I have discussed this with Arnd and for Zynq there is no need to call it so early
>>> that's why using this function is fine. Arnd wasn't sure if there is any need to call it before unflattening
>>> that's why I didn't try to solve this case.
>>>
>>> Can you send me that patch? I will test it on Zynq and if it is working, let's include it to v4.
>> Pankaj: Any update on this? I think that your patch can be applied on the top of this one.
>
> Hi Michal,
> Sorry for late reply, I was a bit busy with some official assignments.
> Usage of "of_scan_flat_dt" in "early_syscon_init" was just a suggestion from my side, and I have
> not worked on it till now. As I mentioned your patches worked for me if I call them a bit late in "init_machine".
> I do not see as such any issues, until some one needs it before DT gets unflattened.
ok great. Can you give me more formal response?
Acked-by, Reviewed-by or Tested-by if you have time to test it?
Thanks,
Michal
> >>> Some platforms need to get system controller
> >>> ready as soon as possible.
> >>> The patch provides early_syscon_initialization
> >>> which create early mapping for all syscon compatible
> >>> devices in early_syscon_probe.
> >>> Regmap is get via syscon_early_regmap_lookup_by_phandle()
> >>>
> >>> Regular device probes attach device to regmap
> >>> via regmap_attach_dev().
> >>>
> >>> For early syscon initialization is necessary to extend
> >>> struct syscon and provide remove function
> >>> which unmap all early init structures.
> >>>
> >>> Signed-off-by: Michal Simek <[email protected]>
> >>> ---
> >>>
> >>> Changes in v3:
> >>> - Keep backward compatibility for platform drivers and test it
> >>> - Use only one probe method which is early_syscon_probe
> >>> suggested by Lee Jones. Do not force anybody to call early_syscon_init
> >>> - Add kernel-doc description
> >>>
> >>> Changes in v2:
> >>> - Fix bad logic in early_syscon_probe
> >>> - Fix compilation failure for x86_64 reported by zero day testing system
> >>> - Regmap change available here
> >>> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git tags/nodev
> >>>
> >>> drivers/mfd/syscon.c | 159 ++++++++++++++++++++++++++++++++++++++++-----
> >>> include/linux/mfd/syscon.h | 11 ++++
> >>> 2 files changed, 153 insertions(+), 17 deletions(-)
> Lee: Can you please look at these changes? It is around for a while
> and I would like to close it.
Actually I'm _still_ waiting on Arnd and Mark's opinion on this, as my
reservations from v1 still apply.
https://lkml.org/lkml/2014/2/12/149
I'll poke them now.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
I think leaving Arnd and Mark off of CC was a mistake.
Correcting now.
> Some platforms need to get system controller
> ready as soon as possible.
> The patch provides early_syscon_initialization
> which create early mapping for all syscon compatible
> devices in early_syscon_probe.
> Regmap is get via syscon_early_regmap_lookup_by_phandle()
>
> Regular device probes attach device to regmap
> via regmap_attach_dev().
>
> For early syscon initialization is necessary to extend
> struct syscon and provide remove function
> which unmap all early init structures.
>
> Signed-off-by: Michal Simek <[email protected]>
> ---
>
> Changes in v3:
> - Keep backward compatibility for platform drivers and test it
> - Use only one probe method which is early_syscon_probe
> suggested by Lee Jones. Do not force anybody to call early_syscon_init
> - Add kernel-doc description
>
> Changes in v2:
> - Fix bad logic in early_syscon_probe
> - Fix compilation failure for x86_64 reported by zero day testing system
> - Regmap change available here
> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git tags/nodev
>
> drivers/mfd/syscon.c | 159 ++++++++++++++++++++++++++++++++++++++++-----
> include/linux/mfd/syscon.h | 11 ++++
> 2 files changed, 153 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
> index 71841f9..8e2ff88 100644
> --- a/drivers/mfd/syscon.c
> +++ b/drivers/mfd/syscon.c
> @@ -20,12 +20,15 @@
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
> +#include <linux/slab.h>
> #include <linux/mfd/syscon.h>
>
> static struct platform_driver syscon_driver;
>
> struct syscon {
> + void __iomem *base;
> struct regmap *regmap;
> + struct resource res;
> };
>
> static int syscon_match_node(struct device *dev, void *data)
> @@ -95,6 +98,30 @@ struct regmap *syscon_regmap_lookup_by_pdevname(const char *s)
> }
> EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
>
> +/**
> + * syscon_early_regmap_lookup_by_phandle - Early phandle lookup function
> + * @np: device_node pointer
> + * @property: property name which handle system controller phandle
> + * Return: regmap pointer, an error pointer otherwise
> + */
> +struct regmap *syscon_early_regmap_lookup_by_phandle(struct device_node *np,
> + const char *property)
> +{
> + struct device_node *syscon_np;
> + struct syscon *syscon;
> +
> + syscon_np = of_parse_phandle(np, property, 0);
> + if (!syscon_np)
> + return ERR_PTR(-ENODEV);
> +
> + syscon = syscon_np->data;
> +
> + of_node_put(syscon_np);
> +
> + return syscon->regmap;
> +}
> +EXPORT_SYMBOL_GPL(syscon_early_regmap_lookup_by_phandle);
> +
> struct regmap *syscon_regmap_lookup_by_phandle(struct device_node *np,
> const char *property)
> {
> @@ -123,36 +150,118 @@ static struct regmap_config syscon_regmap_config = {
> .reg_stride = 4,
> };
>
> -static int syscon_probe(struct platform_device *pdev)
> +/**
> + * early_syscon_probe - Early system controller probe method
> + * @np: device_node pointer
> + * @syscon_p: syscon pointer
> + * @res: device IO resource
> + * Return: 0 if successful, a negative error code otherwise
> + */
> +static int early_syscon_probe(struct device_node *np, struct syscon **syscon_p,
> + struct resource *res)
> {
> - struct device *dev = &pdev->dev;
> struct syscon *syscon;
> - struct resource *res;
> - void __iomem *base;
> + int ret;
> +
> + if (np && np->data) {
> + pr_debug("Early syscon was called\n");
> + *syscon_p = (struct syscon *)&np->data;
> + return 0;
> + }
>
> - syscon = devm_kzalloc(dev, sizeof(*syscon), GFP_KERNEL);
> + syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
> if (!syscon)
> return -ENOMEM;
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (!res)
> - return -ENOENT;
> + *syscon_p = (struct syscon *)&syscon;
>
> - base = devm_ioremap(dev, res->start, resource_size(res));
> - if (!base)
> - return -ENOMEM;
> + if (!res && np) {
> + if (of_address_to_resource(np, 0, &syscon->res)) {
> + ret = -EINVAL;
> + goto alloc;
> + }
> +
> + np->data = syscon;
> + of_node_put(np);
> + } else {
> + syscon->res = *res;
> + }
>
> - syscon_regmap_config.max_register = res->end - res->start - 3;
> - syscon->regmap = devm_regmap_init_mmio(dev, base,
> - &syscon_regmap_config);
> + syscon->base = ioremap(syscon->res.start, resource_size(&syscon->res));
> + if (!syscon->base) {
> + pr_err("%s: Unable to map I/O memory\n", __func__);
> + ret = PTR_ERR(syscon->base);
> + goto alloc;
> + }
> +
> + syscon_regmap_config.max_register = syscon->res.end -
> + syscon->res.start - 3;
> + syscon->regmap = regmap_init_mmio(NULL, syscon->base,
> + &syscon_regmap_config);
> if (IS_ERR(syscon->regmap)) {
> - dev_err(dev, "regmap init failed\n");
> - return PTR_ERR(syscon->regmap);
> + pr_err("regmap init failed\n");
> + ret = PTR_ERR(syscon->regmap);
> + goto iomap;
> }
> + if (np)
> + pr_info("syscon: %s regmap %pR registered\n", np->name,
> + &syscon->res);
> +
> + return 0;
> +
> +iomap:
> + iounmap(syscon->base);
> +alloc:
> + kfree(syscon);
> +
> + return ret;
> +}
> +
> +/**
> + * early_syscon_init - Early system controller initialization
> + */
> +void __init early_syscon_init(void)
> +{
> + struct device_node *np;
> + struct syscon *syscon = NULL;
> +
> + for_each_matching_node_and_match(np, of_syscon_match, NULL) {
> + if (early_syscon_probe(np, &syscon, NULL))
> + BUG();
> + }
> +}
> +
> +/**
> + * syscon_probe - System controller probe method
> + * @pdev: Platform device
> + * Return: 0 if successful, a negative error code otherwise
> + */
> +static int syscon_probe(struct platform_device *pdev)
> +{
> + struct syscon *syscon, *syscon_p;
> + struct resource *res = NULL;
> + struct device *dev = &pdev->dev;
> + struct device_node *np = pdev->dev.of_node;
> + int ret;
> +
> + if (!np) {
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -ENOENT;
> + }
> + ret = early_syscon_probe(np, &syscon_p, res);
> + if (ret) {
> + dev_err(dev, "Syscon probe failed\n");
> + return ret;
> + }
> +
> + syscon = *(struct syscon **)syscon_p;
> +
> + regmap_attach_dev(dev, syscon->regmap, &syscon_regmap_config);
>
> platform_set_drvdata(pdev, syscon);
>
> - dev_info(dev, "regmap %pR registered\n", res);
> + dev_info(dev, "regmap attach device to %pR\n", &syscon->res);
>
> return 0;
> }
> @@ -162,6 +271,21 @@ static const struct platform_device_id syscon_ids[] = {
> { }
> };
>
> +/**
> + * syscon_remove - System controller cleanup function
> + * @pdev: Platform device
> + * Return: 0 always
> + */
> +static int syscon_remove(struct platform_device *pdev)
> +{
> + struct syscon *syscon = platform_get_drvdata(pdev);
> +
> + iounmap(syscon->base);
> + kfree(syscon);
> +
> + return 0;
> +}
> +
> static struct platform_driver syscon_driver = {
> .driver = {
> .name = "syscon",
> @@ -169,6 +293,7 @@ static struct platform_driver syscon_driver = {
> .of_match_table = of_syscon_match,
> },
> .probe = syscon_probe,
> + .remove = syscon_remove,
> .id_table = syscon_ids,
> };
>
> diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
> index 8789fa3..465c092 100644
> --- a/include/linux/mfd/syscon.h
> +++ b/include/linux/mfd/syscon.h
> @@ -24,6 +24,10 @@ extern struct regmap *syscon_regmap_lookup_by_pdevname(const char *s);
> extern struct regmap *syscon_regmap_lookup_by_phandle(
> struct device_node *np,
> const char *property);
> +extern struct regmap *syscon_early_regmap_lookup_by_phandle(
> + struct device_node *np,
> + const char *property);
> +extern void early_syscon_init(void);
> #else
> static inline struct regmap *syscon_node_to_regmap(struct device_node *np)
> {
> @@ -46,6 +50,13 @@ static inline struct regmap *syscon_regmap_lookup_by_phandle(
> {
> return ERR_PTR(-ENOSYS);
> }
> +
> +static struct regmap *syscon_early_regmap_lookup_by_phandle(
> + struct device_node *np,
> + const char *property)
> +{
> + return ERR_PTR(-ENOSYS);
> +}
> #endif
>
> #endif /* __LINUX_MFD_SYSCON_H__ */
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Wed, Apr 23, 2014 at 11:05:30AM +0100, Lee Jones wrote:
> > +struct regmap *syscon_early_regmap_lookup_by_phandle(struct device_node *np,
> > + const char *property)
> > +{
> > + struct device_node *syscon_np;
> > + struct syscon *syscon;
> > +
> > + syscon_np = of_parse_phandle(np, property, 0);
> > + if (!syscon_np)
> > + return ERR_PTR(-ENODEV);
> > +
> > + syscon = syscon_np->data;
> > +
> > + of_node_put(syscon_np);
> > +
> > + return syscon->regmap;
> > +}
> > +EXPORT_SYMBOL_GPL(syscon_early_regmap_lookup_by_phandle);
I don't know what this is doing but it looks dodgy, we're returning
something stored in the DT node after dropping our reference to the DT
node. For FDT systems this probably makes no difference since we don't
actually free the node but someone might decide to do something like
clear data that's associated with a node (however that happened) when
the node goes unreferenced.
> > - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > - if (!res)
> > - return -ENOENT;
> > + *syscon_p = (struct syscon *)&syscon;
What's this cast for?
On 04/23/2014 01:42 PM, Mark Brown wrote:
> On Wed, Apr 23, 2014 at 11:05:30AM +0100, Lee Jones wrote:
>
>>> +struct regmap *syscon_early_regmap_lookup_by_phandle(struct device_node *np,
>>> + const char *property)
>>> +{
>>> + struct device_node *syscon_np;
>>> + struct syscon *syscon;
>>> +
>>> + syscon_np = of_parse_phandle(np, property, 0);
>>> + if (!syscon_np)
>>> + return ERR_PTR(-ENODEV);
>>> +
>>> + syscon = syscon_np->data;
>>> +
>>> + of_node_put(syscon_np);
>>> +
>>> + return syscon->regmap;
>>> +}
>>> +EXPORT_SYMBOL_GPL(syscon_early_regmap_lookup_by_phandle);
>
> I don't know what this is doing but it looks dodgy, we're returning
> something stored in the DT node after dropping our reference to the DT
> node. For FDT systems this probably makes no difference since we don't
> actually free the node but someone might decide to do something like
> clear data that's associated with a node (however that happened) when
> the node goes unreferenced.
In early_syscon_probe np->data contains pointer to struct syscon which
stores base, regmap and res - driver private data for system controller.
(init in early_syscon_probe)
This is the way I am aware of how to share driver private data without pdev.
Maybe there is better way how to do it that's why please let me know
if you are aware about it.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
On Wed, Apr 23, 2014 at 03:04:28PM +0200, Michal Simek wrote:
> In early_syscon_probe np->data contains pointer to struct syscon which
> stores base, regmap and res - driver private data for system controller.
> (init in early_syscon_probe)
> This is the way I am aware of how to share driver private data without pdev.
> Maybe there is better way how to do it that's why please let me know
> if you are aware about it.
The main issue I'm seeing is that the fact that the pointer in the node
is being used without a reference being held; I'd expect something to
explicitly own the reference. Either hold the reference somewhere and
hand it off between the various stages or free the syscon object and
allocate a new one each time it's used prior to the device being
instantiated.
On 04/23/2014 05:27 PM, Michal Simek wrote:
> On 04/23/2014 10:22 AM, Pankaj Dubey wrote:
>> On 04/23/2014 04:27 PM, Michal Simek wrote:
>>> On 04/10/2014 08:17 AM, Michal Simek wrote:
>>>> On 04/10/2014 08:20 AM, Pankaj Dubey wrote:
>>>>> Hi Michal,
>>>>>
>>>>> On 04/09/2014 12:00 AM, Michal Simek wrote:
>>>>>> Some platforms need to get system controller
>>>>>> ready as soon as possible.
>>>>>> The patch provides early_syscon_initialization
>>>>>> which create early mapping for all syscon compatible
>>>>>> devices in early_syscon_probe.
>>>>>> Regmap is get via syscon_early_regmap_lookup_by_phandle()
>>>>>>
>>>>>> Regular device probes attach device to regmap
>>>>>> via regmap_attach_dev().
>>>>>>
>>>>>> For early syscon initialization is necessary to extend
>>>>>> struct syscon and provide remove function
>>>>>> which unmap all early init structures.
>>>>>>
>>>>>> Signed-off-by: Michal Simek <[email protected]>
>>>>>> ---
>>>>>>
>>>>>> Changes in v3:
>>>>>> - Keep backward compatibility for platform drivers and test it
>>>>>> - Use only one probe method which is early_syscon_probe
>>>>>> suggested by Lee Jones. Do not force anybody to call early_syscon_init
>>>>>> - Add kernel-doc description
>>>>>>
>>>>>> Changes in v2:
>>>>>> - Fix bad logic in early_syscon_probe
>>>>>> - Fix compilation failure for x86_64 reported by zero day testing system
>>>>>> - Regmap change available here
>>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git tags/nodev
>>>>>>
>>>>>> drivers/mfd/syscon.c | 159 ++++++++++++++++++++++++++++++++++++++++-----
>>>>>> include/linux/mfd/syscon.h | 11 ++++
>>>>>> 2 files changed, 153 insertions(+), 17 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
>>>>>> index 71841f9..8e2ff88 100644
>>>>>> --- a/drivers/mfd/syscon.c
>>>>>> +++ b/drivers/mfd/syscon.c
>>>>>> @@ -20,12 +20,15 @@
>>>>>> #include <linux/of_platform.h>
>>>>>> #include <linux/platform_device.h>
>>>>>> #include <linux/regmap.h>
>>>>>> +#include <linux/slab.h>
>>>>>> #include <linux/mfd/syscon.h>
>>>>>>
>>>>>> static struct platform_driver syscon_driver;
>>>>>>
>>>>>> struct syscon {
>>>>>> + void __iomem *base;
>>>>>> struct regmap *regmap;
>>>>>> + struct resource res;
>>>>>> };
>>>>>>
>>>>>> static int syscon_match_node(struct device *dev, void *data)
>>>>>> @@ -95,6 +98,30 @@ struct regmap *syscon_regmap_lookup_by_pdevname(const char *s)
>>>>>> }
>>>>>> EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
>>>>>>
>>>>>> +/**
>>>>>> + * syscon_early_regmap_lookup_by_phandle - Early phandle lookup function
>>>>>> + * @np: device_node pointer
>>>>>> + * @property: property name which handle system controller phandle
>>>>>> + * Return: regmap pointer, an error pointer otherwise
>>>>>> + */
>>>>>> +struct regmap *syscon_early_regmap_lookup_by_phandle(struct device_node *np,
>>>>>> + const char *property)
>>>>>> +{
>>>>>> + struct device_node *syscon_np;
>>>>>> + struct syscon *syscon;
>>>>>> +
>>>>>> + syscon_np = of_parse_phandle(np, property, 0);
>>>>>> + if (!syscon_np)
>>>>>> + return ERR_PTR(-ENODEV);
>>>>>> +
>>>>>> + syscon = syscon_np->data;
>>>>>> +
>>>>>> + of_node_put(syscon_np);
>>>>>> +
>>>>>> + return syscon->regmap;
>>>>>> +}
>>>>>> +EXPORT_SYMBOL_GPL(syscon_early_regmap_lookup_by_phandle);
>>>>>> +
>>>>>> struct regmap *syscon_regmap_lookup_by_phandle(struct device_node *np,
>>>>>> const char *property)
>>>>>> {
>>>>>> @@ -123,36 +150,118 @@ static struct regmap_config syscon_regmap_config = {
>>>>>> .reg_stride = 4,
>>>>>> };
>>>>>>
>>>>>> -static int syscon_probe(struct platform_device *pdev)
>>>>>> +/**
>>>>>> + * early_syscon_probe - Early system controller probe method
>>>>>> + * @np: device_node pointer
>>>>>> + * @syscon_p: syscon pointer
>>>>>> + * @res: device IO resource
>>>>>> + * Return: 0 if successful, a negative error code otherwise
>>>>>> + */
>>>>>> +static int early_syscon_probe(struct device_node *np, struct syscon **syscon_p,
>>>>>> + struct resource *res)
>>>>>> {
>>>>>> - struct device *dev = &pdev->dev;
>>>>>> struct syscon *syscon;
>>>>>> - struct resource *res;
>>>>>> - void __iomem *base;
>>>>>> + int ret;
>>>>>> +
>>>>>> + if (np && np->data) {
>>>>>> + pr_debug("Early syscon was called\n");
>>>>>> + *syscon_p = (struct syscon *)&np->data;
>>>>>> + return 0;
>>>>>> + }
>>>>>>
>>>>>> - syscon = devm_kzalloc(dev, sizeof(*syscon), GFP_KERNEL);
>>>>>> + syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
>>>>>> if (!syscon)
>>>>>> return -ENOMEM;
>>>>>>
>>>>>> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>>> - if (!res)
>>>>>> - return -ENOENT;
>>>>>> + *syscon_p = (struct syscon *)&syscon;
>>>>>>
>>>>>> - base = devm_ioremap(dev, res->start, resource_size(res));
>>>>>> - if (!base)
>>>>>> - return -ENOMEM;
>>>>>> + if (!res && np) {
>>>>>> + if (of_address_to_resource(np, 0, &syscon->res)) {
>>>>>> + ret = -EINVAL;
>>>>>> + goto alloc;
>>>>>> + }
>>>>>> +
>>>>>> + np->data = syscon;
>>>>>> + of_node_put(np);
>>>>>> + } else {
>>>>>> + syscon->res = *res;
>>>>>> + }
>>>>>>
>>>>>> - syscon_regmap_config.max_register = res->end - res->start - 3;
>>>>>> - syscon->regmap = devm_regmap_init_mmio(dev, base,
>>>>>> - &syscon_regmap_config);
>>>>>> + syscon->base = ioremap(syscon->res.start, resource_size(&syscon->res));
>>>>>> + if (!syscon->base) {
>>>>>> + pr_err("%s: Unable to map I/O memory\n", __func__);
>>>>>> + ret = PTR_ERR(syscon->base);
>>>>>> + goto alloc;
>>>>>> + }
>>>>>> +
>>>>>> + syscon_regmap_config.max_register = syscon->res.end -
>>>>>> + syscon->res.start - 3;
>>>>>> + syscon->regmap = regmap_init_mmio(NULL, syscon->base,
>>>>>> + &syscon_regmap_config);
>>>>>> if (IS_ERR(syscon->regmap)) {
>>>>>> - dev_err(dev, "regmap init failed\n");
>>>>>> - return PTR_ERR(syscon->regmap);
>>>>>> + pr_err("regmap init failed\n");
>>>>>> + ret = PTR_ERR(syscon->regmap);
>>>>>> + goto iomap;
>>>>>> }
>>>>>> + if (np)
>>>>>> + pr_info("syscon: %s regmap %pR registered\n", np->name,
>>>>>> + &syscon->res);
>>>>>> +
>>>>>> + return 0;
>>>>>> +
>>>>>> +iomap:
>>>>>> + iounmap(syscon->base);
>>>>>> +alloc:
>>>>>> + kfree(syscon);
>>>>>> +
>>>>>> + return ret;
>>>>>> +}
>>>>>> +
>>>>>> +/**
>>>>>> + * early_syscon_init - Early system controller initialization
>>>>>> + */
>>>>>> +void __init early_syscon_init(void)
>>>>>> +{
>>>>>> + struct device_node *np;
>>>>>> + struct syscon *syscon = NULL;
>>>>>> +
>>>>>> + for_each_matching_node_and_match(np, of_syscon_match, NULL) {
>>>>>> + if (early_syscon_probe(np, &syscon, NULL))
>>>>>> + BUG();
>>>>>> + }
>>>>>> +}
>>>>>> +
>>>>>> +/**
>>>>>> + * syscon_probe - System controller probe method
>>>>>> + * @pdev: Platform device
>>>>>> + * Return: 0 if successful, a negative error code otherwise
>>>>>> + */
>>>>>> +static int syscon_probe(struct platform_device *pdev)
>>>>>> +{
>>>>>> + struct syscon *syscon, *syscon_p;
>>>>>> + struct resource *res = NULL;
>>>>>> + struct device *dev = &pdev->dev;
>>>>>> + struct device_node *np = pdev->dev.of_node;
>>>>>> + int ret;
>>>>>> +
>>>>>> + if (!np) {
>>>>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>>> + if (!res)
>>>>>> + return -ENOENT;
>>>>>> + }
>>>>>> + ret = early_syscon_probe(np, &syscon_p, res);
>>>>>> + if (ret) {
>>>>>> + dev_err(dev, "Syscon probe failed\n");
>>>>>> + return ret;
>>>>>> + }
>>>>>> +
>>>>>> + syscon = *(struct syscon **)syscon_p;
>>>>>> +
>>>>>> + regmap_attach_dev(dev, syscon->regmap, &syscon_regmap_config);
>>>>>>
>>>>>> platform_set_drvdata(pdev, syscon);
>>>>>>
>>>>>> - dev_info(dev, "regmap %pR registered\n", res);
>>>>>> + dev_info(dev, "regmap attach device to %pR\n", &syscon->res);
>>>>>>
>>>>>> return 0;
>>>>>> }
>>>>>> @@ -162,6 +271,21 @@ static const struct platform_device_id syscon_ids[] = {
>>>>>> { }
>>>>>> };
>>>>>>
>>>>>> +/**
>>>>>> + * syscon_remove - System controller cleanup function
>>>>>> + * @pdev: Platform device
>>>>>> + * Return: 0 always
>>>>>> + */
>>>>>> +static int syscon_remove(struct platform_device *pdev)
>>>>>> +{
>>>>>> + struct syscon *syscon = platform_get_drvdata(pdev);
>>>>>> +
>>>>>> + iounmap(syscon->base);
>>>>>> + kfree(syscon);
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> static struct platform_driver syscon_driver = {
>>>>>> .driver = {
>>>>>> .name = "syscon",
>>>>>> @@ -169,6 +293,7 @@ static struct platform_driver syscon_driver = {
>>>>>> .of_match_table = of_syscon_match,
>>>>>> },
>>>>>> .probe = syscon_probe,
>>>>>> + .remove = syscon_remove,
>>>>>> .id_table = syscon_ids,
>>>>>> };
>>>>>>
>>>>>> diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
>>>>>> index 8789fa3..465c092 100644
>>>>>> --- a/include/linux/mfd/syscon.h
>>>>>> +++ b/include/linux/mfd/syscon.h
>>>>>> @@ -24,6 +24,10 @@ extern struct regmap *syscon_regmap_lookup_by_pdevname(const char *s);
>>>>>> extern struct regmap *syscon_regmap_lookup_by_phandle(
>>>>>> struct device_node *np,
>>>>>> const char *property);
>>>>>> +extern struct regmap *syscon_early_regmap_lookup_by_phandle(
>>>>>> + struct device_node *np,
>>>>>> + const char *property);
>>>>>> +extern void early_syscon_init(void);
>>>>>> #else
>>>>>> static inline struct regmap *syscon_node_to_regmap(struct device_node *np)
>>>>>> {
>>>>>> @@ -46,6 +50,13 @@ static inline struct regmap *syscon_regmap_lookup_by_phandle(
>>>>>> {
>>>>>> return ERR_PTR(-ENOSYS);
>>>>>> }
>>>>>> +
>>>>>> +static struct regmap *syscon_early_regmap_lookup_by_phandle(
>>>>>> + struct device_node *np,
>>>>>> + const char *property)
>>>>>> +{
>>>>>> + return ERR_PTR(-ENOSYS);
>>>>>> +}
>>>>>> #endif
>>>>>>
>>>>>> #endif /* __LINUX_MFD_SYSCON_H__ */
>>>>>> --
>>>>>> 1.8.2.3
>>>>>>
>>>>> Thanks for CC'ing me.
>>>> no problem.
>>>>
>>>>> I have tested this patch along with Exynos PMU related changes posted here
>>>>> https://lkml.org/lkml/2014/4/2/53 and modified it for using Syscon, and it's working for me.
>>>> great.
>>>>
>>>>
>>>>> I have observed one issue during this testing:
>>>>>
>>>>> Even though we are saying early initialization I could not use "early_syscon_init" from machine's
>>>>> "map_io" or "init_early". I observed that "early_syscon_init" failed to called "early_syscon_probe",
>>>>> because it can not get any matching node, when I debug further I found that as "early_syscon_init"
>>>>> is calling "for_each_matching_node_and_match" and it can't work before unflatten'ing device tree,
>>>>> which happens in "setup_arch" a bit late, after "map_io" and "init_early" calls of machine.
>>>>>
>>>>> So if I have to use "early_syscon_init" I MUST call it after device tree is unflattened. It worked for me
>>>>> when I called it from "init_machine" (from exynos_dt_machine_init), But if someone needs it at very
>>>>> early stage then it might not work. So how about using "of_scan_flat_dt" in "early_syscon_init"?, so that
>>>>> we can use this functionality at very early stage if required.
>>>> Yes you are right. I have discussed this with Arnd and for Zynq there is no need to call it so early
>>>> that's why using this function is fine. Arnd wasn't sure if there is any need to call it before unflattening
>>>> that's why I didn't try to solve this case.
>>>>
>>>> Can you send me that patch? I will test it on Zynq and if it is working, let's include it to v4.
>>> Pankaj: Any update on this? I think that your patch can be applied on the top of this one.
>> Hi Michal,
>> Sorry for late reply, I was a bit busy with some official assignments.
>> Usage of "of_scan_flat_dt" in "early_syscon_init" was just a suggestion from my side, and I have
>> not worked on it till now. As I mentioned your patches worked for me if I call them a bit late in "init_machine".
>> I do not see as such any issues, until some one needs it before DT gets unflattened.
> ok great. Can you give me more formal response?
> Acked-by, Reviewed-by or Tested-by if you have time to test it?
Michal, I have tested your patch on Exynos5250, and it' working well.
But still I would like to point out there is one limitation as I mentioned
already we are not able to use
this very early during boot.
I have posted my patches based on this patch you can have a look here:
https://lkml.org/lkml/2014/4/25/252
You can see even though I have used early syscon API, but in one case where I
needed it before secondary
core boot I could not use it.
With this limitation if you would like to you can add my
Tested-by: Pankaj Dubey <[email protected]>
>
> Thanks,
> Michal
>
>
>
--
Best Regards,
Pankaj Dubey
Hi Michal,
First of all, even though not touching DT bindings, this change is DT
related and so devicetree mailing list should be on Cc, as well as some
DT people, especially Grant. Adding them.
Otherwise, please see my comments inline.
On 08.04.2014 17:00, Michal Simek wrote:
> Some platforms need to get system controller
> ready as soon as possible.
> The patch provides early_syscon_initialization
> which create early mapping for all syscon compatible
> devices in early_syscon_probe.
> Regmap is get via syscon_early_regmap_lookup_by_phandle()
>
> Regular device probes attach device to regmap
> via regmap_attach_dev().
>
> For early syscon initialization is necessary to extend
> struct syscon and provide remove function
> which unmap all early init structures.
>
> Signed-off-by: Michal Simek <[email protected]>
> ---
>
> Changes in v3:
> - Keep backward compatibility for platform drivers and test it
> - Use only one probe method which is early_syscon_probe
> suggested by Lee Jones. Do not force anybody to call early_syscon_init
> - Add kernel-doc description
>
> Changes in v2:
> - Fix bad logic in early_syscon_probe
> - Fix compilation failure for x86_64 reported by zero day testing system
> - Regmap change available here
> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git tags/nodev
>
> drivers/mfd/syscon.c | 159 ++++++++++++++++++++++++++++++++++++++++-----
> include/linux/mfd/syscon.h | 11 ++++
> 2 files changed, 153 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
> index 71841f9..8e2ff88 100644
> --- a/drivers/mfd/syscon.c
> +++ b/drivers/mfd/syscon.c
> @@ -20,12 +20,15 @@
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
> +#include <linux/slab.h>
> #include <linux/mfd/syscon.h>
>
> static struct platform_driver syscon_driver;
>
> struct syscon {
> + void __iomem *base;
> struct regmap *regmap;
> + struct resource res;
> };
>
> static int syscon_match_node(struct device *dev, void *data)
> @@ -95,6 +98,30 @@ struct regmap *syscon_regmap_lookup_by_pdevname(const char *s)
> }
> EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
>
> +/**
> + * syscon_early_regmap_lookup_by_phandle - Early phandle lookup function
> + * @np: device_node pointer
> + * @property: property name which handle system controller phandle
> + * Return: regmap pointer, an error pointer otherwise
> + */
> +struct regmap *syscon_early_regmap_lookup_by_phandle(struct device_node *np,
> + const char *property)
> +{
> + struct device_node *syscon_np;
> + struct syscon *syscon;
> +
> + syscon_np = of_parse_phandle(np, property, 0);
> + if (!syscon_np)
> + return ERR_PTR(-ENODEV);
> +
> + syscon = syscon_np->data;
> +
> + of_node_put(syscon_np);
As it was mentioned in other review comments, this is rather fishy.
Instead, couldn't you create a local list of system controllers
available in the system and then use that to perform look-up? This could
also let you eliminate the need to have separate functions for early and
normal look-up as the list could be simply used for both cases. Of
course you would have to add of_node and list_head fields to struct
syscon, but I don't think that matters.
As a side effect, the look-up would not need to iterate over all devices
in the system anymore, just over the list of registered system controllers.
Best regards,
Tomasz