| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 11 Mar 2013 17:40:06 +0800
From: Dong Aisheng <b29396@...escale.com>
To: Alexander Shiyan <shc_work@...l.ru>
CC: <linux-kernel@...r.kernel.org>, Arnd Bergmann <arnd@...db.de>,
Dong Aisheng <dong.aisheng@...aro.org>,
Samuel Ortiz <sameo@...ux.intel.com>,
Mark Brown <broonie@...nsource.wolfsonmicro.com>,
Thierry Reding <thierry.reding@...onic-design.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Dmitry Torokhov <dmitry.torokhov@...il.com>,
Stephen Warren <swarren@...dotorg.org>
Subject: Re: [PATCH v6 2/2] mfd: syscon: Add non-DT support
On Thu, Feb 28, 2013 at 06:57:14PM +0400, Alexander Shiyan wrote:
> This patch allow using syscon driver from the platform data, i.e.
> possibility using driver on systems without oftree support.
> For search syscon device from the client drivers,
> "syscon_regmap_lookup_by_pdevname" function was added.
>
> Signed-off-by: Alexander Shiyan <shc_work@...l.ru>
> ---
> drivers/mfd/Kconfig | 1 -
> drivers/mfd/syscon.c | 77 +++++++++++++++++++++++++++-------------------
> include/linux/mfd/syscon.h | 1 +
> 3 files changed, 47 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 671f5b1..8fdd87e 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1070,7 +1070,6 @@ config MFD_STA2X11
>
> config MFD_SYSCON
> bool "System Controller Register R/W Based on Regmap"
> - depends on OF
> select REGMAP_MMIO
> help
> Select this option to enable accessing system control registers
> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
> index 674af14..e5b6be1 100644
> --- a/drivers/mfd/syscon.c
> +++ b/drivers/mfd/syscon.c
> @@ -29,7 +29,7 @@ struct syscon {
> struct regmap *regmap;
> };
>
> -static int syscon_match(struct device *dev, void *data)
> +static int syscon_match_node(struct device *dev, void *data)
> {
> struct device_node *dn = data;
>
> @@ -42,7 +42,7 @@ struct regmap *syscon_node_to_regmap(struct device_node *np)
> struct device *dev;
>
> dev = driver_find_device(&syscon_driver.driver, NULL, np,
> - syscon_match);
> + syscon_match_node);
> if (!dev)
> return ERR_PTR(-EPROBE_DEFER);
>
> @@ -68,6 +68,34 @@ 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)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + const struct platform_device_id *id = platform_get_device_id(pdev);
> +
> + if (id)
> + if (!strcmp(id->name, (const char *)data))
> + return 1;
> +
> + 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(-ENODEV);
Should it be 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)
> {
> @@ -99,30 +127,24 @@ static struct regmap_config syscon_regmap_config = {
> static int syscon_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> - struct device_node *np = dev->of_node;
> struct syscon *syscon;
> - struct resource res;
> - int ret;
> -
> - if (!np)
> - return -ENOENT;
> + struct resource *res;
>
> - syscon = devm_kzalloc(dev, sizeof(struct syscon),
> - GFP_KERNEL);
> + syscon = devm_kzalloc(dev, sizeof(struct syscon), GFP_KERNEL);
> if (!syscon)
> return -ENOMEM;
>
> - syscon->base = of_iomap(np, 0);
> - if (!syscon->base)
> - return -EADDRNOTAVAIL;
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -ENOENT;
>
> - ret = of_address_to_resource(np, 0, &res);
> - if (ret)
> - return ret;
> + syscon->base = devm_ioremap(dev, res->start, resource_size(res));
> + if (!syscon->base)
> + return -ENOMEM;
>
> - syscon_regmap_config.max_register = res.end - res.start - 3;
> + syscon_regmap_config.max_register = res->end - res->start - 3;
> syscon->regmap = devm_regmap_init_mmio(dev, syscon->base,
> - &syscon_regmap_config);
> + &syscon_regmap_config);
Unneeded change to me
> if (IS_ERR(syscon->regmap)) {
> dev_err(dev, "regmap init failed\n");
> return PTR_ERR(syscon->regmap);
> @@ -130,22 +152,15 @@ static int syscon_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, syscon);
>
> - dev_info(dev, "syscon regmap start 0x%x end 0x%x registered\n",
> - res.start, res.end);
> + dev_info(dev, "regmap 0x%x-0x%x registered\n", res->start, res->end);
Doesn't with a 'syscon' prefix look better?
>
> return 0;
> }
>
> -static int syscon_remove(struct platform_device *pdev)
> -{
> - struct syscon *syscon;
> -
> - syscon = platform_get_drvdata(pdev);
> - iounmap(syscon->base);
> - platform_set_drvdata(pdev, NULL);
> -
> - return 0;
> -}
> +static const struct platform_device_id syscon_ids[] = {
> + { "syscon", },
> + { }
> +};
>
> static struct platform_driver syscon_driver = {
> .driver = {
> @@ -154,7 +169,7 @@ static struct platform_driver syscon_driver = {
> .of_match_table = of_syscon_match,
> },
> .probe = syscon_probe,
> - .remove = syscon_remove,
So you reply on the managed resource to address the resource leak issue
raised by Dmitry on doing manually unbind here, right?
this looks ok to me.
So, besides above minor comments,
Acked-by: Dong Aisheng <dong.aisheng@...aro.org>
Regards
Dong Aisheng
> + .id_table = syscon_ids,
> };
>
> static int __init syscon_init(void)
> diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
> index 6aeb6b8..5c9ee6e 100644
> --- a/include/linux/mfd/syscon.h
> +++ b/include/linux/mfd/syscon.h
> @@ -17,6 +17,7 @@
>
> extern struct regmap *syscon_node_to_regmap(struct device_node *np);
> extern struct regmap *syscon_regmap_lookup_by_compatible(const char *s);
> +extern struct regmap *syscon_regmap_lookup_by_pdevname(const char *s);
> extern struct regmap *syscon_regmap_lookup_by_phandle(
> struct device_node *np,
> const char *property);
> --
> 1.7.12.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists