gpio: lpc32xx: Use priv data instead of platdata

PATCH New Axel Lin 2 participants 4 messages Apr 11, 2015 – Apr 14, 2015

Subsystems (2)

GPIO Arm/LPC32XX SoC support

lore

Share & tools

lore

Axel Lin

author

2015-04-11 02:20 UTC #0 lore raw

PATCH 1/1 gpio: lpc32xx: Use priv data instead of platdata

Initially I found this driver has set priv_auto_alloc_size but it actually never use dev->priv. The U_BOOT_DEVICE(lpc32xx_gpios) does not provide the platdata and all fields in struct lpc32xx_gpio_platdata are set in probe. It looks like the struct lpc32xx_gpio_platdata actually should be a priv data. Thus this patch renames lpc32xx_gpio_platdata to lpc32xx_gpio_priv and converts all dev_get_platdata() to dev_get_priv().

Signed-off-by Axel Lin <[email protected]>

---
Hi Albert,
I don't have this h/w for testing, so only compile test. I'd appreciate if you can review and test this patch. Thanks,
Axel

 drivers/gpio/lpc32xx_gpio.c | 39 +++++++++++++++++++--------------------
 1 file changed, 19 insertions(+), 20 deletions(-)

drivers/gpio/lpc32xx_gpio.c +19 ~~-20~~

index 96b3125..8a9826e 100644
@@ -37,7 +37,7 @@
 
 #define LPC32XX_GPIOS 128
 
-struct lpc32xx_gpio_platdata {
+struct lpc32xx_gpio_priv {
 	struct gpio_regs *regs;
 	/* GPIO FUNCTION: SEE WARNING #2 */
 	signed char function[LPC32XX_GPIOS];

@@ -60,8 +60,8 @@ struct lpc32xx_gpio_platdata {
 static int lpc32xx_gpio_direction_input(struct udevice *dev, unsigned offset)
 {
 	int port, mask;
-	struct lpc32xx_gpio_platdata *gpio_platdata = dev_get_platdata(dev);
-	struct gpio_regs *regs = gpio_platdata->regs;
+	struct lpc32xx_gpio_priv *gpio_priv = dev_get_priv(dev);
+	struct gpio_regs *regs = gpio_priv->regs;
 
 	port = GPIO_TO_PORT(offset);
 	mask = GPIO_TO_MASK(offset);

@@ -83,7 +83,7 @@ static int lpc32xx_gpio_direction_input(struct udevice *dev, unsigned offset)
 	}
 
 	/* GPIO FUNCTION: SEE WARNING #2 */
-	gpio_platdata->function[offset] = GPIOF_INPUT;
+	gpio_priv->function[offset] = GPIOF_INPUT;
 
 	return 0;
 }

@@ -95,8 +95,8 @@ static int lpc32xx_gpio_direction_input(struct udevice *dev, unsigned offset)
 static int lpc32xx_gpio_get_value(struct udevice *dev, unsigned offset)
 {
 	int port, rank, mask, value;
-	struct lpc32xx_gpio_platdata *gpio_platdata = dev_get_platdata(dev);
-	struct gpio_regs *regs = gpio_platdata->regs;
+	struct lpc32xx_gpio_priv *gpio_priv = dev_get_priv(dev);
+	struct gpio_regs *regs = gpio_priv->regs;
 
 	port = GPIO_TO_PORT(offset);

@@ -130,8 +130,8 @@ static int lpc32xx_gpio_get_value(struct udevice *dev, unsigned offset)
 static int gpio_set(struct udevice *dev, unsigned gpio)
 {
 	int port, mask;
-	struct lpc32xx_gpio_platdata *gpio_platdata = dev_get_platdata(dev);
-	struct gpio_regs *regs = gpio_platdata->regs;
+	struct lpc32xx_gpio_priv *gpio_priv = dev_get_priv(dev);
+	struct gpio_regs *regs = gpio_priv->regs;
 
 	port = GPIO_TO_PORT(gpio);
 	mask = GPIO_TO_MASK(gpio);

@@ -162,8 +162,8 @@ static int gpio_set(struct udevice *dev, unsigned gpio)
 static int gpio_clr(struct udevice *dev, unsigned gpio)
 {
 	int port, mask;
-	struct lpc32xx_gpio_platdata *gpio_platdata = dev_get_platdata(dev);
-	struct gpio_regs *regs = gpio_platdata->regs;
+	struct lpc32xx_gpio_priv *gpio_priv = dev_get_priv(dev);
+	struct gpio_regs *regs = gpio_priv->regs;
 
 	port = GPIO_TO_PORT(gpio);
 	mask = GPIO_TO_MASK(gpio);

@@ -208,8 +208,8 @@ static int lpc32xx_gpio_direction_output(struct udevice *dev, unsigned offset,
 				       int value)
 {
 	int port, mask;
-	struct lpc32xx_gpio_platdata *gpio_platdata = dev_get_platdata(dev);
-	struct gpio_regs *regs = gpio_platdata->regs;
+	struct lpc32xx_gpio_priv *gpio_priv = dev_get_priv(dev);
+	struct gpio_regs *regs = gpio_priv->regs;
 
 	port = GPIO_TO_PORT(offset);
 	mask = GPIO_TO_MASK(offset);

@@ -231,7 +231,7 @@ static int lpc32xx_gpio_direction_output(struct udevice *dev, unsigned offset,
 	}
 
 	/* GPIO FUNCTION: SEE WARNING #2 */
-	gpio_platdata->function[offset] = GPIOF_OUTPUT;
+	gpio_priv->function[offset] = GPIOF_OUTPUT;
 
 	return lpc32xx_gpio_set_value(dev, offset, value);
 }

@@ -251,8 +251,8 @@ static int lpc32xx_gpio_direction_output(struct udevice *dev, unsigned offset,
 
 static int lpc32xx_gpio_get_function(struct udevice *dev, unsigned offset)
 {
-	struct lpc32xx_gpio_platdata *gpio_platdata = dev_get_platdata(dev);
-	return gpio_platdata->function[offset];
+	struct lpc32xx_gpio_priv *gpio_priv = dev_get_priv(dev);
+	return gpio_priv->function[offset];
 }
 
 static const struct dm_gpio_ops gpio_lpc32xx_ops = {

@@ -265,7 +265,7 @@ static const struct dm_gpio_ops gpio_lpc32xx_ops = {
 
 static int lpc32xx_gpio_probe(struct udevice *dev)
 {
-	struct lpc32xx_gpio_platdata *gpio_platdata = dev_get_platdata(dev);
+	struct lpc32xx_gpio_priv *gpio_priv = dev_get_priv(dev);
 	struct gpio_dev_priv *uc_priv = dev->uclass_priv;
 
 	if (dev->of_offset == -1) {

@@ -274,12 +274,11 @@ static int lpc32xx_gpio_probe(struct udevice *dev)
 	}
 
 	/* set base address for GPIO registers */
-	gpio_platdata->regs = (struct gpio_regs *)GPIO_BASE;
+	gpio_priv->regs = (struct gpio_regs *)GPIO_BASE;
 
 	/* all GPIO functions are unknown until requested */
 	/* GPIO FUNCTION: SEE WARNING #2 */
-	memset(gpio_platdata->function, GPIOF_UNKNOWN,
-	       sizeof(gpio_platdata->function));
+	memset(gpio_priv->function, GPIOF_UNKNOWN, sizeof(gpio_priv->function));
 
 	return 0;
 }

@@ -289,5 +288,5 @@ U_BOOT_DRIVER(gpio_lpc32xx) = {
 	.id	= UCLASS_GPIO,
 	.ops	= &gpio_lpc32xx_ops,
 	.probe	= lpc32xx_gpio_probe,
-	.priv_auto_alloc_size = sizeof(struct lpc32xx_gpio_platdata),
+	.priv_auto_alloc_size = sizeof(struct lpc32xx_gpio_priv),
 };

Signature · 1 line

1.9.1

Albert ARIBAUD

2015-04-13 08:41 UTC #1 lore raw

PATCH 1/1 gpio: lpc32xx: Use priv data instead of platdata

Hi Axel,

Le Sat, 11 Apr 2015 10:20:08 +0800, Axel Lin <[email protected]> a ?crit :

Quoted 7 lines

Initially I found this driver has set priv_auto_alloc_size but it actually never use dev->priv. The U_BOOT_DEVICE(lpc32xx_gpios) does not provide the platdata and all fields in struct lpc32xx_gpio_platdata are set in probe. It looks like the struct lpc32xx_gpio_platdata actually should be a priv data. Thus this patch renames lpc32xx_gpio_platdata to lpc32xx_gpio_priv and converts all dev_get_platdata() to dev_get_priv().

Quoted 7 lines

Signed-off-by Axel Lin <[email protected]>
---
Hi Albert,
I don't have this h/w for testing, so only compile test. I'd appreciate if you can review and test this patch. Thanks,
Axel

Indeed the driver allocates priv and does not use it. However, I think that the allocation should be removed as useless, rather than plat data be converted to priv. IIUC, priv is a way to apply a single driver to several similar devices, and LPC32XX only has one GPIO device.

Cordialement,
Albert ARIBAUD
3ADEV

Axel Lin

author

2015-04-14 00:30 UTC #2 lore raw

PATCH 1/1 gpio: lpc32xx: Use priv data instead of platdata

2015-04-13 16:41 GMT+08:00 Albert ARIBAUD <[email protected]>:

Quoted 20 lines

Hi Axel,
Le Sat, 11 Apr 2015 10:20:08 +0800, Axel Lin <[email protected]> a ?crit :
Quoted 7 lines
Initially I found this driver has set priv_auto_alloc_size but it actually never use dev->priv. The U_BOOT_DEVICE(lpc32xx_gpios) does not provide the platdata and all fields in struct lpc32xx_gpio_platdata are set in probe. It looks like the struct lpc32xx_gpio_platdata actually should be a priv data. Thus this patch renames lpc32xx_gpio_platdata to lpc32xx_gpio_priv and converts all dev_get_platdata() to dev_get_priv().
Quoted 7 lines
Signed-off-by Axel Lin <[email protected]>
---
Hi Albert,
I don't have this h/w for testing, so only compile test. I'd appreciate if you can review and test this patch. Thanks,
Axel

Quoted 4 lines

Indeed the driver allocates priv and does not use it. However, I think that the allocation should be removed as useless, rather than plat data be converted to priv. IIUC, priv is a way to apply a single driver to several similar devices, and LPC32XX only has one GPIO device.

Hi Albert,

I think it's fine to use privdata here even though LPC32XX only has one GPIO device.
gpio_platdata->function stores the runtime state which should be stored in privdata.
I don't see any good reason to use platdata here, that is why I think it's a misue of platdata and thus convert it to use privdata.

Regards,
Axel

Albert ARIBAUD

2015-04-14 05:45 UTC #3 lore raw

PATCH 1/1 gpio: lpc32xx: Use priv data instead of platdata

Hi Axel,

Le Tue, 14 Apr 2015 08:30:03 +0800, Axel Lin <[email protected]> a ?crit :

Quoted 26 lines

2015-04-13 16:41 GMT+08:00 Albert ARIBAUD <[email protected]>:
Quoted 20 lines
Hi Axel,
Le Sat, 11 Apr 2015 10:20:08 +0800, Axel Lin <[email protected]> a ?crit :
Quoted 7 lines
Initially I found this driver has set priv_auto_alloc_size but it actually never use dev->priv. The U_BOOT_DEVICE(lpc32xx_gpios) does not provide the platdata and all fields in struct lpc32xx_gpio_platdata are set in probe. It looks like the struct lpc32xx_gpio_platdata actually should be a priv data. Thus this patch renames lpc32xx_gpio_platdata to lpc32xx_gpio_priv and converts all dev_get_platdata() to dev_get_priv().
Quoted 7 lines
Signed-off-by Axel Lin <[email protected]>
---
Hi Albert,
I don't have this h/w for testing, so only compile test. I'd appreciate if you can review and test this patch. Thanks,
Axel
Quoted 4 lines
Indeed the driver allocates priv and does not use it. However, I think that the allocation should be removed as useless, rather than plat data be converted to priv. IIUC, priv is a way to apply a single driver to several similar devices, and LPC32XX only has one GPIO device.

Quoted 8 lines

Hi Albert,
I think it's fine to use privdata here even though LPC32XX only has one GPIO device.
gpio_platdata->function stores the runtime state which should be stored in privdata.
I don't see any good reason to use platdata here, that is why I think it's a misue of platdata and thus convert it to use privdata.

That's a good point, sorry to have missed it.

Can you please reword the commit message so that it describes what you just wrote? Something along the lines of "The LPC32XX GPIO driver platdata currently contains GPIO state information, which should go into priv_data"?

Thanks in advance.

Quoted 2 lines

Regards,
Axel

Cordialement,
Albert ARIBAUD
3ADEV