[PATCH v4] mfd: syscon: Add non-DT support

[PATCH v4] mfd: syscon: Add non-DT support

[Alexander Shiyan]

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@mail.ru>
---
 drivers/mfd/Kconfig        |  1 -
 drivers/mfd/syscon.c       | 64 +++++++++++++++++++++++++++++++++-------------
 include/linux/mfd/syscon.h |  1 +
 3 files changed, 47 insertions(+), 19 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 4a7ed5a..0773484 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -28,7 +28,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;
 
@@ -41,7 +41,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);
 
@@ -67,6 +67,33 @@ 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)
+		return !strcmp(id->name, (const char *)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(-ENODEV);
+
+	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)
 {
@@ -98,30 +125,25 @@ 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);
+			      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_resource(dev, res);
+	if (!syscon->base)
+		return -EBUSY;
 
-	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);
 	if (IS_ERR(syscon->regmap)) {
 		dev_err(dev, "regmap init failed\n");
 		return PTR_ERR(syscon->regmap);
@@ -130,11 +152,16 @@ 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);
+		 res->start, res->end);
 
 	return 0;
 }
 
+static const struct platform_device_id syscon_ids[] = {
+	{ "syscon", },
+	{ }
+};
+
 static struct platform_driver syscon_driver = {
 	.driver = {
 		.name = "syscon",
@@ -142,6 +169,7 @@ static struct platform_driver syscon_driver = {
 		.of_match_table = of_syscon_match,
 	},
 	.probe		= syscon_probe,
+	.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

Re: [PATCH v4] mfd: syscon: Add non-DT suppor

[Arnd Bergmann]

On Thursday 21 February 2013, 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@mail.ru>

Looks good to me now.


Acked-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH v4] mfd: syscon: Add non-DT support

[Dong Aisheng]

On Thu, Feb 21, 2013 at 07:29:02PM +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@mail.ru>

[...]

> +	syscon->base = devm_ioremap_resource(dev, res);
> +	if (!syscon->base)

Is this correct?

> +		return -EBUSY;
>

Otherwise, i'm also ok with this patch.
Acked-by: Dong Aisheng <dong.aisheng@linaro.org>

BTW, i did not see Samuel's tree having this new API.
So, who will pick this patch?

Regards
Dong Aisheng

Re[2]: [PATCH v4] mfd: syscon: Add non-DT support

[Alexander Shiyan]

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

> On Thu, Feb 21, 2013 at 07:29:02PM +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@mail.ru>
> 
> [...]
> 
> > +	syscon->base = devm_ioremap_resource(dev, res);
> > +	if (!syscon->base)
> 
> Is this correct?

Hmm, of course IS_ERR should be used here...
v5?

> 
> > +		return -EBUSY;
> >
> 
> Otherwise, i'm also ok with this patch.
> Acked-by: Dong Aisheng <dong.aisheng@linaro.org>
> 
> BTW, i did not see Samuel's tree having this new API.
> So, who will pick this patch?

I have same question.

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

Re: [PATCH v4] mfd: syscon: Add non-DT support

[Dong Aisheng]

On Fri, Feb 22, 2013 at 11:01:18AM +0400, Alexander Shiyan wrote:
> > On Thu, Feb 21, 2013 at 07:29:02PM +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@mail.ru>
> > 
> > [...]
> > 
> > > +	syscon->base = devm_ioremap_resource(dev, res);
> > > +	if (!syscon->base)
> > 
> > Is this correct?
> 
> Hmm, of course IS_ERR should be used here...
> v5?
> 

Yes.
>From here:
https://lkml.org/lkml/2013/1/21/140
It seems it is.

> > 
> > > +		return -EBUSY;

Both this line could also be changed.

> > >
> > 
> > Otherwise, i'm also ok with this patch.
> > Acked-by: Dong Aisheng <dong.aisheng@linaro.org>
> > 
> > BTW, i did not see Samuel's tree having this new API.
> > So, who will pick this patch?
> 
> I have same question.

I CCed Thierry and Greg who may know it.

Regards
Dong Aisheng

Re: [PATCH v4] mfd: syscon: Add non-DT support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1448 bytes --]

On Fri, Feb 22, 2013 at 03:13:12PM +0800, Dong Aisheng wrote:
> On Fri, Feb 22, 2013 at 11:01:18AM +0400, Alexander Shiyan wrote:
> > > On Thu, Feb 21, 2013 at 07:29:02PM +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@mail.ru>
> > > 
> > > [...]
> > > 
> > > > +	syscon->base = devm_ioremap_resource(dev, res);
> > > > +	if (!syscon->base)
> > > 
> > > Is this correct?
> > 
> > Hmm, of course IS_ERR should be used here...
> > v5?
> > 
> 
> Yes.
> >From here:
> https://lkml.org/lkml/2013/1/21/140
> It seems it is.
> 
> > > 
> > > > +		return -EBUSY;
> 
> Both this line could also be changed.
> 
> > > >
> > > 
> > > Otherwise, i'm also ok with this patch.
> > > Acked-by: Dong Aisheng <dong.aisheng@linaro.org>
> > > 
> > > BTW, i did not see Samuel's tree having this new API.
> > > So, who will pick this patch?
> > 
> > I have same question.
> 
> I CCed Thierry and Greg who may know it.

Yes, devm_ioremap_resource() never returns NULL. You always need to
check the returned pointer with IS_ERR(). The value that you return
should be extracted from the pointer with PTR_ERR().

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v4] mfd: syscon: Add non-DT support

[Dong Aisheng]

On Fri, Feb 22, 2013 at 08:27:19AM +0100, Thierry Reding wrote:
...
> > > > Otherwise, i'm also ok with this patch.
> > > > Acked-by: Dong Aisheng <dong.aisheng@linaro.org>
> > > > 
> > > > BTW, i did not see Samuel's tree having this new API.
> > > > So, who will pick this patch?
> > > 
> > > I have same question.
> > 
> > I CCed Thierry and Greg who may know it.
> 
> Yes, devm_ioremap_resource() never returns NULL. You always need to
> check the returned pointer with IS_ERR(). The value that you return
> should be extracted from the pointer with PTR_ERR().

Thanks Thierry.
Since Samuel's mdf tree does not have your patch introducing
the new API of devm_ioremap_resource,
do you know which tree this patch can go through, Greg's driver core tree?

Regards
Dong Aisheng

Re: [PATCH v4] mfd: syscon: Add non-DT support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1075 bytes --]

On Fri, Feb 22, 2013 at 04:29:55PM +0800, Dong Aisheng wrote:
> On Fri, Feb 22, 2013 at 08:27:19AM +0100, Thierry Reding wrote:
> ...
> > > > > Otherwise, i'm also ok with this patch.
> > > > > Acked-by: Dong Aisheng <dong.aisheng@linaro.org>
> > > > > 
> > > > > BTW, i did not see Samuel's tree having this new API.
> > > > > So, who will pick this patch?
> > > > 
> > > > I have same question.
> > > 
> > > I CCed Thierry and Greg who may know it.
> > 
> > Yes, devm_ioremap_resource() never returns NULL. You always need to
> > check the returned pointer with IS_ERR(). The value that you return
> > should be extracted from the pointer with PTR_ERR().
> 
> Thanks Thierry.
> Since Samuel's mdf tree does not have your patch introducing
> the new API of devm_ioremap_resource,
> do you know which tree this patch can go through, Greg's driver core tree?

I don't think it matters much at this point because Linus merged the
driver core tree yesterday, so anything that gets applied now should
automatically have the new API available.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v4] mfd: syscon: Add non-DT support

[Arnd Bergmann]

On Friday 22 February 2013, Thierry Reding wrote:
> On Fri, Feb 22, 2013 at 03:13:12PM +0800, Dong Aisheng wrote:
> > On Fri, Feb 22, 2013 at 11:01:18AM +0400, Alexander Shiyan wrote:
> > > > On Thu, Feb 21, 2013 at 07:29:02PM +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@mail.ru>
> > > > 
> > > > [...]
> > > > 
> > > > > +       syscon->base = devm_ioremap_resource(dev, res);
> > > > > +       if (!syscon->base)
> > > > 
> > > > Is this correct?
> > > 
> > > Hmm, of course IS_ERR should be used here...
> > > v5?
> > > 
> > 
> > Yes.
> > >From here:
> > https://lkml.org/lkml/2013/1/21/140
> > It seems it is.
> 
> Yes, devm_ioremap_resource() never returns NULL. You always need to
> check the returned pointer with IS_ERR(). The value that you return
> should be extracted from the pointer with PTR_ERR().
> 

Well, devm_ioremap_resource also tries to request the resource, which
as someone (Dong or Shawn?) pointed out, we should not do for the imx6q
case. I think this has to be reverted to platform_get_resource
and dev_ioremap.

	Arnd

Re: [PATCH v4] mfd: syscon: Add non-DT support

[Dong Aisheng]

On Fri, Feb 22, 2013 at 09:11:53AM +0000, Arnd Bergmann wrote:
> On Friday 22 February 2013, Thierry Reding wrote:
> > On Fri, Feb 22, 2013 at 03:13:12PM +0800, Dong Aisheng wrote:
> > > On Fri, Feb 22, 2013 at 11:01:18AM +0400, Alexander Shiyan wrote:
> > > > > On Thu, Feb 21, 2013 at 07:29:02PM +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@mail.ru>
> > > > > 
> > > > > [...]
> > > > > 
> > > > > > +       syscon->base = devm_ioremap_resource(dev, res);
> > > > > > +       if (!syscon->base)
> > > > > 
> > > > > Is this correct?
> > > > 
> > > > Hmm, of course IS_ERR should be used here...
> > > > v5?
> > > > 
> > > 
> > > Yes.
> > > >From here:
> > > https://lkml.org/lkml/2013/1/21/140
> > > It seems it is.
> > 
> > Yes, devm_ioremap_resource() never returns NULL. You always need to
> > check the returned pointer with IS_ERR(). The value that you return
> > should be extracted from the pointer with PTR_ERR().
> > 
> 
> Well, devm_ioremap_resource also tries to request the resource, which
> as someone (Dong or Shawn?) pointed out, we should not do for the imx6q
> case. I think this has to be reverted to platform_get_resource
> and dev_ioremap.
> 

Yes, i did not see detailed devm_ioremap_resource implementation,
but if it requests the resource then right, it may fail on imx6q due to
request overlapped resources.

Regards
Dong Aisheng

Re: [PATCH v4] mfd: syscon: Add non-DT support

[Dong Aisheng]

On Fri, Feb 22, 2013 at 09:52:12AM +0100, Thierry Reding wrote:
> On Fri, Feb 22, 2013 at 04:29:55PM +0800, Dong Aisheng wrote:
> > On Fri, Feb 22, 2013 at 08:27:19AM +0100, Thierry Reding wrote:
> > ...
> > > > > > Otherwise, i'm also ok with this patch.
> > > > > > Acked-by: Dong Aisheng <dong.aisheng@linaro.org>
> > > > > > 
> > > > > > BTW, i did not see Samuel's tree having this new API.
> > > > > > So, who will pick this patch?
> > > > > 
> > > > > I have same question.
> > > > 
> > > > I CCed Thierry and Greg who may know it.
> > > 
> > > Yes, devm_ioremap_resource() never returns NULL. You always need to
> > > check the returned pointer with IS_ERR(). The value that you return
> > > should be extracted from the pointer with PTR_ERR().
> > 
> > Thanks Thierry.
> > Since Samuel's mdf tree does not have your patch introducing
> > the new API of devm_ioremap_resource,
> > do you know which tree this patch can go through, Greg's driver core tree?
> 
> I don't think it matters much at this point because Linus merged the
> driver core tree yesterday, so anything that gets applied now should
> automatically have the new API available.
> 
I just tried update to latest linus tree, still did not find this API defined
in include/linux/io.h.
Would you mind point it out for me?

Regards
Dong Aisheng

Re: [PATCH v4] mfd: syscon: Add non-DT support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1608 bytes --]

On Fri, Feb 22, 2013 at 05:20:56PM +0800, Dong Aisheng wrote:
> On Fri, Feb 22, 2013 at 09:52:12AM +0100, Thierry Reding wrote:
> > On Fri, Feb 22, 2013 at 04:29:55PM +0800, Dong Aisheng wrote:
> > > On Fri, Feb 22, 2013 at 08:27:19AM +0100, Thierry Reding wrote:
> > > ...
> > > > > > > Otherwise, i'm also ok with this patch.
> > > > > > > Acked-by: Dong Aisheng <dong.aisheng@linaro.org>
> > > > > > > 
> > > > > > > BTW, i did not see Samuel's tree having this new API.
> > > > > > > So, who will pick this patch?
> > > > > > 
> > > > > > I have same question.
> > > > > 
> > > > > I CCed Thierry and Greg who may know it.
> > > > 
> > > > Yes, devm_ioremap_resource() never returns NULL. You always need to
> > > > check the returned pointer with IS_ERR(). The value that you return
> > > > should be extracted from the pointer with PTR_ERR().
> > > 
> > > Thanks Thierry.
> > > Since Samuel's mdf tree does not have your patch introducing
> > > the new API of devm_ioremap_resource,
> > > do you know which tree this patch can go through, Greg's driver core tree?
> > 
> > I don't think it matters much at this point because Linus merged the
> > driver core tree yesterday, so anything that gets applied now should
> > automatically have the new API available.
> > 
> I just tried update to latest linus tree, still did not find this API defined
> in include/linux/io.h.
> Would you mind point it out for me?

Commit 75096579c3ac39ddc2f8b0d9a8924eba31f4d920 introduced it. The
prototype is in include/linux/device.h and the implementation in
lib/devres.c.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v4] mfd: syscon: Add non-DT support

[Dong Aisheng]

On 22 February 2013 17:24, Thierry Reding
<thierry.reding@avionic-design.de> wrote:
...
>> I just tried update to latest linus tree, still did not find this API defined
>> in include/linux/io.h.
>> Would you mind point it out for me?
>
> Commit 75096579c3ac39ddc2f8b0d9a8924eba31f4d920 introduced it. The
> prototype is in include/linux/device.h and the implementation in
> lib/devres.c.
>
Got it.
Thanks.

Regards
Dong Aisheng

Re[2]: [PATCH v4] mfd: syscon: Add non-DT support

[Alexander Shiyan]

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

> On Fri, Feb 22, 2013 at 04:29:55PM +0800, Dong Aisheng wrote:
> > On Fri, Feb 22, 2013 at 08:27:19AM +0100, Thierry Reding wrote:
> > ...
> > > > > > Otherwise, i'm also ok with this patch.
> > > > > > Acked-by: Dong Aisheng <dong.aisheng@linaro.org>
> > > > > > 
> > > > > > BTW, i did not see Samuel's tree having this new API.
> > > > > > So, who will pick this patch?
> > > > > 
> > > > > I have same question.
> > > > 
> > > > I CCed Thierry and Greg who may know it.
> > > 
> > > Yes, devm_ioremap_resource() never returns NULL. You always need to
> > > check the returned pointer with IS_ERR(). The value that you return
> > > should be extracted from the pointer with PTR_ERR().
> > 
> > Thanks Thierry.
> > Since Samuel's mdf tree does not have your patch introducing
> > the new API of devm_ioremap_resource,
> > do you know which tree this patch can go through, Greg's driver core tree?
> 
> I don't think it matters much at this point because Linus merged the
> driver core tree yesterday, so anything that gets applied now should
> automatically have the new API available.

So, i will send all the three parts of the patch as v5 and keep all current CCs.
Thanks.

---
ÿôèº{.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¥