Skip to contentSkip to conversation

sa11x0 PCMCIA updates

PATCH New Russell King - ARM Linux 3 participants 12 messages Mar 26, 2015 – Mar 28, 2015

Subsystems (2)
PXA2xx/PXA3xx support Pcmcia
lore
Share & tools
lore

Russell King - ARM Linux

author
#0 lore raw

cover sa11x0 PCMCIA updates

Here's a collection of patches which cleans up and updates the SA11x0 and PXA2xx socket drivers, particularly converting them to the devm_* APIs to ensure that resources are properly cleaned up.

I've stopped short of totally converting sa1111_generic as there are some corner cases where we want to initialise only socket 1, but not socket 0, which would require further testing to ensure that these changes are correct.

I will be testing the sa11x0 changes later today, but as I gave most of my PXA2xx platforms to Robert, I need Robert to test this on the Lubbock.

 drivers/pcmcia/pxa2xx_base.c    | 17 +++++------------
 drivers/pcmcia/sa1100_generic.c |  2 --
 drivers/pcmcia/sa1111_generic.c | 14 +++++++-------
 drivers/pcmcia/sa11xx_base.c    | 17 ++---------------
 drivers/pcmcia/soc_common.h     |  1 -
 5 files changed, 14 insertions(+), 37 deletions(-)
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
#1 lore raw

PATCH 1/8 pcmcia: sa11x0: fix missing clk_put() in sa11x0 socket drivers

Fix the lack of clk_put() in sa11xx_base.c's error cleanup paths by converting the driver to the devm_* API.

Fixes 86d88bfca475 ("ARM: 8247/2: pcmcia: sa1100: make use of device clock")
Signed-off-by Russell King <[email protected]>
---
 drivers/pcmcia/sa1100_generic.c | 1 -
 drivers/pcmcia/sa11xx_base.c    | 3 +--
 2 files changed, 1 insertion(+), 3 deletions(-)
drivers/pcmcia/sa1100_generic.c +0 -1
index 803945259da8..42861cc70158 100644
@@ -93,7 +93,6 @@ static int sa11x0_drv_pcmcia_remove(struct platform_device *dev)
 	for (i = 0; i < sinfo->nskt; i++)
 		soc_pcmcia_remove_one(&sinfo->skt[i]);
 
-	clk_put(sinfo->clk);
 	kfree(sinfo);
 	return 0;
 }
drivers/pcmcia/sa11xx_base.c +1 -2
index cf6de2c2b329..553d70a67f80 100644
@@ -222,7 +222,7 @@ int sa11xx_drv_pcmcia_probe(struct device *dev, struct pcmcia_low_level *ops,
 	int i, ret = 0;
 	struct clk *clk;
 
-	clk = clk_get(dev, NULL);
+	clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(clk))
 		return PTR_ERR(clk);
 

@@ -251,7 +251,6 @@ int sa11xx_drv_pcmcia_probe(struct device *dev, struct pcmcia_low_level *ops,
 	if (ret) {
 		while (--i >= 0)
 			soc_pcmcia_remove_one(&sinfo->skt[i]);
-		clk_put(clk);
 		kfree(sinfo);
 	} else {
 		dev_set_drvdata(dev, sinfo);
Signature · 1 line
1.8.3.1
#2 lore raw

PATCH 2/8 pcmcia: sa11x0: convert memory allocation to devm_* API

In reply to Russell King - ARM Linux

Convert the sa11x0 socket driver memory allocation to use devm_kzalloc() to simplify the cleanup path.

Signed-off-by Russell King <[email protected]>
---
 drivers/pcmcia/sa1100_generic.c | 1 -
 drivers/pcmcia/sa11xx_base.c    | 3 +--
 2 files changed, 1 insertion(+), 3 deletions(-)
drivers/pcmcia/sa1100_generic.c +0 -1
index 42861cc70158..66acdc85727c 100644
@@ -93,7 +93,6 @@ static int sa11x0_drv_pcmcia_remove(struct platform_device *dev)
 	for (i = 0; i < sinfo->nskt; i++)
 		soc_pcmcia_remove_one(&sinfo->skt[i]);
 
-	kfree(sinfo);
 	return 0;
 }
drivers/pcmcia/sa11xx_base.c +1 -2
index 553d70a67f80..6e6336d47d4a 100644
@@ -228,7 +228,7 @@ int sa11xx_drv_pcmcia_probe(struct device *dev, struct pcmcia_low_level *ops,
 
 	sa11xx_drv_pcmcia_ops(ops);
 
-	sinfo = kzalloc(SKT_DEV_INFO_SIZE(nr), GFP_KERNEL);
+	sinfo = devm_kzalloc(dev, SKT_DEV_INFO_SIZE(nr), GFP_KERNEL);
 	if (!sinfo)
 		return -ENOMEM;
 

@@ -251,7 +251,6 @@ int sa11xx_drv_pcmcia_probe(struct device *dev, struct pcmcia_low_level *ops,
 	if (ret) {
 		while (--i >= 0)
 			soc_pcmcia_remove_one(&sinfo->skt[i]);
-		kfree(sinfo);
 	} else {
 		dev_set_drvdata(dev, sinfo);
 	}
Signature · 1 line
1.8.3.1
#3 lore raw

PATCH 3/8 pcmcia: pxa2xx: update socket driver to use devm_clk_get() API

In reply to Russell King - ARM Linux

Update the pxa2xx socket driver to use the devm_clk_get() API so that the cleanup paths are simplified.

Signed-off-by Russell King <[email protected]>
---
 drivers/pcmcia/pxa2xx_base.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)
drivers/pcmcia/pxa2xx_base.c +2 -6
index 984a8ff559d8..197c77a64ce0 100644
@@ -296,17 +296,15 @@ static int pxa2xx_drv_pcmcia_probe(struct platform_device *dev)
 		goto err0;
 	}
 
-	clk = clk_get(&dev->dev, NULL);
+	clk = devm_clk_get(&dev->dev, NULL);
 	if (IS_ERR(clk))
 		return -ENODEV;
 
 	pxa2xx_drv_pcmcia_ops(ops);
 
 	sinfo = kzalloc(SKT_DEV_INFO_SIZE(ops->nr), GFP_KERNEL);
-	if (!sinfo) {
-		clk_put(clk);
+	if (!sinfo)
 		return -ENOMEM;
-	}
 
 	sinfo->nskt = ops->nr;
 	sinfo->clk = clk;

@@ -332,7 +330,6 @@ static int pxa2xx_drv_pcmcia_probe(struct platform_device *dev)
 err1:
 	while (--i >= 0)
 		soc_pcmcia_remove_one(&sinfo->skt[i]);
-	clk_put(clk);
 	kfree(sinfo);
 err0:
 	return ret;

@@ -348,7 +345,6 @@ static int pxa2xx_drv_pcmcia_remove(struct platform_device *dev)
 	for (i = 0; i < sinfo->nskt; i++)
 		soc_pcmcia_remove_one(&sinfo->skt[i]);
 
-	clk_put(sinfo->clk);
 	kfree(sinfo);
 	return 0;
 }
Signature · 1 line
1.8.3.1
#4 lore raw

PATCH 4/8 pcmcia: pxa2xx: convert memory allocation to devm_* API

In reply to Russell King - ARM Linux

Convert the pxa2xx socket driver memory allocation to use devm_kzalloc() to simplify the cleanup path.

Signed-off-by Russell King <[email protected]>
---
 drivers/pcmcia/pxa2xx_base.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
drivers/pcmcia/pxa2xx_base.c +3 -5
index 197c77a64ce0..dfcf07828605 100644
@@ -302,7 +302,8 @@ static int pxa2xx_drv_pcmcia_probe(struct platform_device *dev)
 
 	pxa2xx_drv_pcmcia_ops(ops);
 
-	sinfo = kzalloc(SKT_DEV_INFO_SIZE(ops->nr), GFP_KERNEL);
+	sinfo = devm_kzalloc(&pdev->dev, SKT_DEV_INFO_SIZE(ops->nr),
+			     GFP_KERNEL);
 	if (!sinfo)
 		return -ENOMEM;
 

@@ -330,7 +331,7 @@ static int pxa2xx_drv_pcmcia_probe(struct platform_device *dev)
 err1:
 	while (--i >= 0)
 		soc_pcmcia_remove_one(&sinfo->skt[i]);
-	kfree(sinfo);
+
 err0:
 	return ret;
 }

@@ -340,12 +341,9 @@ static int pxa2xx_drv_pcmcia_remove(struct platform_device *dev)
 	struct skt_dev_info *sinfo = platform_get_drvdata(dev);
 	int i;
 
-	platform_set_drvdata(dev, NULL);
-
 	for (i = 0; i < sinfo->nskt; i++)
 		soc_pcmcia_remove_one(&sinfo->skt[i]);
 
-	kfree(sinfo);
 	return 0;
 }
Signature · 1 line
1.8.3.1
maintainer
#5 lore raw

PATCH 4/8 pcmcia: pxa2xx: convert memory allocation to devm_* API

Quoted 13 lines · Russell King

Convert the pxa2xx socket driver memory allocation to use devm_kzalloc() to simplify the cleanup path.

Signed-off-by Russell King <[email protected]>
---
 drivers/pcmcia/pxa2xx_base.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
drivers/pcmcia/pxa2xx_base.c +0 -0
index 197c77a64ce0..dfcf07828605 100644

drivers/pcmcia/pxa2xx_base.c static int pxa2xx_drv_pcmcia_probe(struct platform_device *dev) · line 305

Quoted 5 lines
drivers/pcmcia/pxa2xx_base.c +1 -1
@@ -302,7 +302,8 @@ static int pxa2xx_drv_pcmcia_probe(struct platform_device *dev)
 
 	pxa2xx_drv_pcmcia_ops(ops);
 
-	sinfo = kzalloc(SKT_DEV_INFO_SIZE(ops->nr), GFP_KERNEL);
+	sinfo = devm_kzalloc(&pdev->dev, SKT_DEV_INFO_SIZE(ops->nr),

There is a typo here : &pdev->dev should be &dev->dev.

Now given that fix, you can add my :

Tested-by Robert Jarzmik <[email protected]>

This test of mine is rather cheap :
- it relies on 4.0-rc4
- I don't have any PCMCIA card to insert in the lubbock, hence the "cheap"
- I judge that pcmcia is working because :
- cat /proc/iomem gives:

     10000000-10001fff : sa1111
       10000400-100005ff : 0400
       10000800-100009ff : 0800
       10000a00-10000bff : 0a00
         10000a00-10000bff : sa1111-ps2
       10000c00-10000dff : 0c00
         10000c00-10000dff : sa1111-ps2
       10001600-100017ff : irq
       10001800-100019ff : 1800
         10001800-100019ff : sa1111-pcmcia
    - I don't see any error message in kernel log

As a side note mainly to myself, the pcmcia is broken on pxa2xx architectures and panics the kernel at boot time because :
- in drivers/pcmcia/sa1111_lubbock.c I see :
pcmcia_lubbock_init()

     pxa2xx_configure_sockets(&sadev->dev);
       here it is expected that platform_data contains the pcmcia ops, but it's
       NULL, and the dereferencing in pxa2xx_configure_sockets() of ops->first
       panics.

Cheers.

Signature · 1 line
Robert
author
#6 lore raw

PATCH 4/8 pcmcia: pxa2xx: convert memory allocation to devm_* API

Quoted 42 lines · Robert Jarzmik
Quoted 13 lines · Russell King

Convert the pxa2xx socket driver memory allocation to use devm_kzalloc() to simplify the cleanup path.

Signed-off-by Russell King <[email protected]>
---
 drivers/pcmcia/pxa2xx_base.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
drivers/pcmcia/pxa2xx_base.c +0 -0
index 197c77a64ce0..dfcf07828605 100644

drivers/pcmcia/pxa2xx_base.c static int pxa2xx_drv_pcmcia_probe(struct platform_device *dev) · line 305

Quoted 5 lines
drivers/pcmcia/pxa2xx_base.c +1 -1
@@ -302,7 +302,8 @@ static int pxa2xx_drv_pcmcia_probe(struct platform_device *dev)
 
 	pxa2xx_drv_pcmcia_ops(ops);
 
-	sinfo = kzalloc(SKT_DEV_INFO_SIZE(ops->nr), GFP_KERNEL);
+	sinfo = devm_kzalloc(&pdev->dev, SKT_DEV_INFO_SIZE(ops->nr),

There is a typo here : &pdev->dev should be &dev->dev.

Now given that fix, you can add my :

Tested-by Robert Jarzmik <[email protected]>

This test of mine is rather cheap :
- it relies on 4.0-rc4
- I don't have any PCMCIA card to insert in the lubbock, hence the "cheap"
- I judge that pcmcia is working because :

   - cat /proc/iomem gives:
     10000000-10001fff : sa1111
       10000400-100005ff : 0400
       10000800-100009ff : 0800
       10000a00-10000bff : 0a00
         10000a00-10000bff : sa1111-ps2
       10000c00-10000dff : 0c00
         10000c00-10000dff : sa1111-ps2
       10001600-100017ff : irq
       10001800-100019ff : 1800
         10001800-100019ff : sa1111-pcmcia
    - I don't see any error message in kernel log
Quoted 8 lines

As a side note mainly to myself, the pcmcia is broken on pxa2xx architectures and panics the kernel at boot time because :
- in drivers/pcmcia/sa1111_lubbock.c I see :
pcmcia_lubbock_init()

     pxa2xx_configure_sockets(&sadev->dev);
       here it is expected that platform_data contains the pcmcia ops, but it's
       NULL, and the dereferencing in pxa2xx_configure_sockets() of ops->first
       panics.

I guess it's broken as a result of 5b703683b6cc3cb97bbe6b1b14898b273eb59279 which added that call without really understanding the call path. I don't see any solution other than partially reverting that commit. As can be seen from the commit message, this was done blindly with no testing - so it's no surprise that it broke.

Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
#7 lore raw

PATCH 5/8 pcmcia: sa1111: update socket driver to use devm_clk_get() API

In reply to Russell King - ARM Linux

Update the pxa2xx socket driver to use the devm_clk_get() API so that the cleanup paths are simplified.

Signed-off-by Russell King <[email protected]>
---
 drivers/pcmcia/sa1111_generic.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/pcmcia/sa1111_generic.c +1 -2
index 80b8e9d05275..fb902af8a5b7 100644
@@ -145,7 +145,7 @@ int sa1111_pcmcia_add(struct sa1111_dev *dev, struct pcmcia_low_level *ops,
 			return -ENOMEM;
 
 		s->soc.nr = ops->first + i;
-		s->soc.clk = clk_get(&dev->dev, NULL);
+		s->soc.clk = devm_clk_get(&dev->dev, NULL);
 		if (IS_ERR(s->soc.clk)) {
 			ret = PTR_ERR(s->soc.clk);
 			kfree(s);

@@ -226,7 +226,6 @@ static int pcmcia_remove(struct sa1111_dev *dev)
 	for (; s; s = next) {
 		next = s->next;
 		soc_pcmcia_remove_one(&s->soc);
-		clk_put(s->soc.clk);
 		kfree(s);
 	}
Signature · 1 line
1.8.3.1
#8 lore raw

PATCH 6/8 pcmcia: sa1111: simplify clk handing in sa1111_pcmcia_add()

In reply to Russell King - ARM Linux

clk_get(dev, NULL) will always refer to the same clock, so it's pointless calling this multiple times for the same device. As we no longer have to worry about the cleanup (via use of devm_clk_get()) we can simplify sa1111_pcmcia_add() too.

Signed-off-by Russell King <[email protected]>
---
 drivers/pcmcia/sa1111_generic.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
drivers/pcmcia/sa1111_generic.c +7 -6
index fb902af8a5b7..a1531feb8460 100644
@@ -135,8 +135,13 @@ int sa1111_pcmcia_add(struct sa1111_dev *dev, struct pcmcia_low_level *ops,
 	int (*add)(struct soc_pcmcia_socket *))
 {
 	struct sa1111_pcmcia_socket *s;
+	struct clk *clk;
 	int i, ret = 0;
 
+	clk = devm_clk_get(&dev->dev, NULL);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
 	ops->socket_state = sa1111_pcmcia_socket_state;
 
 	for (i = 0; i < ops->nr; i++) {

@@ -145,12 +150,8 @@ int sa1111_pcmcia_add(struct sa1111_dev *dev, struct pcmcia_low_level *ops,
 			return -ENOMEM;
 
 		s->soc.nr = ops->first + i;
-		s->soc.clk = devm_clk_get(&dev->dev, NULL);
-		if (IS_ERR(s->soc.clk)) {
-			ret = PTR_ERR(s->soc.clk);
-			kfree(s);
-			return ret;
-		}
+		s->soc.clk = clk;
+
 		soc_pcmcia_init_one(&s->soc, ops, &dev->dev);
 		s->dev = dev;
 		if (s->soc.nr) {
Signature · 1 line
1.8.3.1
#9 lore raw

PATCH 7/8 pcmcia: sa11xx_base.c: remove useless init/exit functions

In reply to Russell King - ARM Linux

A library module is not required to have module init/exit functions. Get rid of these unnecessary functions.

Signed-off-by Russell King <[email protected]>
---
 drivers/pcmcia/sa11xx_base.c | 10 ----------
 1 file changed, 10 deletions(-)
drivers/pcmcia/sa11xx_base.c +0 -10
index 6e6336d47d4a..815b6950975c 100644
@@ -259,16 +259,6 @@ int sa11xx_drv_pcmcia_probe(struct device *dev, struct pcmcia_low_level *ops,
 }
 EXPORT_SYMBOL(sa11xx_drv_pcmcia_probe);
 
-static int __init sa11xx_pcmcia_init(void)
-{
-	return 0;
-}
-fs_initcall(sa11xx_pcmcia_init);
-
-static void __exit sa11xx_pcmcia_exit(void) {}
-
-module_exit(sa11xx_pcmcia_exit);
-
 MODULE_AUTHOR("John Dorsey <[email protected]>");
 MODULE_DESCRIPTION("Linux PCMCIA Card Services: SA-11xx core socket driver");
 MODULE_LICENSE("Dual MPL/GPL");
Signature · 1 line
1.8.3.1
#10 lore raw

PATCH 8/8 pcmcia: soc_common: remove skt_dev_info's clk pointer

In reply to Russell King - ARM Linux

We no longer need to store the clk pointer in struct skt_dev_info as we no longer need to remember the clk pointer for the cleanup paths.

Signed-off-by Russell King <[email protected]>
---
 drivers/pcmcia/pxa2xx_base.c | 1 -
 drivers/pcmcia/sa11xx_base.c | 1 -
 drivers/pcmcia/soc_common.h  | 1 -
 3 files changed, 3 deletions(-)
drivers/pcmcia/pxa2xx_base.c +0 -1
index dfcf07828605..1b2c58dd43cf 100644
@@ -308,7 +308,6 @@ static int pxa2xx_drv_pcmcia_probe(struct platform_device *dev)
 		return -ENOMEM;
 
 	sinfo->nskt = ops->nr;
-	sinfo->clk = clk;
 
 	/* Initialize processor specific parameters */
 	for (i = 0; i < ops->nr; i++) {
drivers/pcmcia/sa11xx_base.c +0 -1
index 815b6950975c..9f6ec87b9f9e 100644
@@ -233,7 +233,6 @@ int sa11xx_drv_pcmcia_probe(struct device *dev, struct pcmcia_low_level *ops,
 		return -ENOMEM;
 
 	sinfo->nskt = nr;
-	sinfo->clk = clk;
 
 	/* Initialize processor specific parameters */
 	for (i = 0; i < nr; i++) {
drivers/pcmcia/soc_common.h +0 -1
index e6fcbea5b682..94762a54d731 100644
@@ -68,7 +68,6 @@ struct soc_pcmcia_socket {
 
 struct skt_dev_info {
 	int nskt;
-	struct clk *clk;
 	struct soc_pcmcia_socket skt[0];
 };
Signature · 1 line
1.8.3.1
maintainer
#11 lore raw

cover sa11x0 PCMCIA updates

In reply to Russell King - ARM Linux
Quoted 3 lines · Russell King - ARM Linux

I will be testing the sa11x0 changes later today, but as I gave most of my PXA2xx platforms to Robert, I need Robert to test this on the Lubbock.

I will be done this weekend Russell.

Cheers.

Signature · 1 line
Robert