DA9055 GPIO driver

PATCH In review Ashish Jangam 4 participants 6 messages Sep 14, 2012 – Oct 12, 2012

Subsystems (3)

HID GPIO Dialog semiconductor drivers

lore

Share & tools

lore

Ashish Jangam

author

2012-09-14 13:20 UTC #0 lore raw

PATCH 4/7 DA9055 GPIO driver

This is the GPIO patch for the DA9055 PMIC. This patch has got dependency on the DA9055 MFD core.

This patch is functionally tested on SMDK6410 board.

Signed-off-by David Dajun Chen <[email protected]>

Signed-off-by Ashish Jangam <[email protected]>

---
 drivers/gpio/Kconfig       |   11 +++
 drivers/gpio/Makefile      |    1 +
 drivers/gpio/gpio-da9055.c |  204 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 216 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/gpio-da9055.c

drivers/gpio/Kconfig +11 -0

index b16c8a7..0b0d884 100644
@@ -86,6 +86,17 @@ config GPIO_DA9052
 	help
 	  Say yes here to enable the GPIO driver for the DA9052 chip.
 
+config GPIO_DA9055
+	tristate "Dialog Semiconductor DA9055 GPIO"
+	depends on MFD_DA9055
+	help
+	  Say yes here to enable the GPIO driver for the DA9055 chip.
+
+	  The Dialog DA9055 PMIC chip has 3 GPIO pins that can be
+	  be controller by this driver.
+
+	  If driver is built as a module it will be called gpio-da9055.
+
 config GPIO_MAX730X
 	tristate

drivers/gpio/Makefile +1 -0

index 153cace..23d9d13 100644
@@ -17,6 +17,7 @@ obj-$(CONFIG_GPIO_ARIZONA)	+= gpio-arizona.o
 obj-$(CONFIG_GPIO_BT8XX)	+= gpio-bt8xx.o
 obj-$(CONFIG_GPIO_CS5535)	+= gpio-cs5535.o
 obj-$(CONFIG_GPIO_DA9052)	+= gpio-da9052.o
+obj-$(CONFIG_GPIO_DA9055)	+= gpio-da9055.o
 obj-$(CONFIG_ARCH_DAVINCI)	+= gpio-davinci.o
 obj-$(CONFIG_GPIO_EM)		+= gpio-em.o
 obj-$(CONFIG_GPIO_EP93XX)	+= gpio-ep93xx.o

drivers/gpio/gpio-da9055.c new file +204 -0

204 lines not on this page · raw message

Signature · 1 line

1.7.0.4

Laxman Dewangan

2012-09-14 13:33 UTC #1 lore raw

drivers/gpio/gpio-da9055.c line 35

Quoted 13 lines · Ashish Jangam

This is the GPIO patch for the DA9055 PMIC. This patch has got dependency on the DA9055 MFD core.
This patch is functionally tested on SMDK6410 board.
Signed-off-by David Dajun Chen<[email protected]>
Signed-off-by Ashish Jangam<[email protected]>
---
drivers/gpio/gpio-da9055.c new file +3 -0
+struct da9055_gpio {
+	struct da9055 *da9055;
+	struct gpio_chip gp;

I suggest to make gp as first member so that offset of member will be 0 in container_of macro and avoid one ops.

Quoted 1 line

+static struct gpio_chip reference_gp __devinitdata = {

This is device int data and you used after device initailisation. I think this can create issue.

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Linus Walleij

Acked-by on v1 4/7

2012-09-14 14:13 UTC #2 lore raw

In reply to Ashish Jangam

Quoted 7 lines · Ashish Jangam

This is the GPIO patch for the DA9055 PMIC. This patch has got dependency on the DA9055 MFD core.
This patch is functionally tested on SMDK6410 board.
Signed-off-by David Dajun Chen <[email protected]>
Signed-off-by Ashish Jangam <[email protected]>

This looks OK

Acked-by Linus Walleij <[email protected]>

But I want Marks comment on this, for example:

drivers/gpio/gpio-da9055.c line 52

Quoted 10 lines

drivers/gpio/gpio-da9055.c new file +10 -0

+static int da9055_gpio_get(struct gpio_chip *gc, unsigned offset)
+{
+       struct da9055_gpio *gpio = to_da9055_gpio(gc);
+       int gpio_direction = 0;
+       int ret;
+
+       /* Get GPIO direction */
+       ret = da9055_reg_read(gpio->da9055, (offset >> 1) + DA9055_REG_GPIO0_1);
+       if (ret < 0)
+               return ret;

So unique functions to read/write registers (as I'm used to).

But the parent driver is using regmap, sand you seem not to use the struct da9055
for anything else than passing reads/writes, so isn't it simpler to just pass the struct regmap * and use update_bits etc directly and remove a layer of indirection?

I'm very uncertain but Mark will know the proper design pattern.

Yours,
Linus Walleij

Mark Brown

2012-09-14 16:38 UTC #3 lore raw

Quoted 7 lines · Linus Walleij

But the parent driver is using regmap, sand you seem not to use the struct da9055
for anything else than passing reads/writes, so isn't it simpler to just pass the struct regmap * and use update_bits etc directly and remove a layer of indirection?
I'm very uncertain but Mark will know the proper design pattern.

Either way is fine - usually the device specific functions end up being static inlines in the header so the code is the same. It really depends if it's useful to have the full device pointer around for other things.

Ashish Jangam

author

2012-10-12 10:00 UTC #4 lore raw

In reply to Linus Walleij

Quoted 38 lines

-----Original Message-----
From: Linus Walleij [mailto:[email protected]]
Sent: Friday, September 14, 2012 7:44 PM
To: Ashish Jangam; Mark Brown
Cc: Grant Likely; Samuel Ortiz; [email protected]; David Dajun Chen
Subject: Re: [Patch v1 4/7] DA9055 GPIO driver
Quoted 7 lines · Ashish Jangam
This is the GPIO patch for the DA9055 PMIC. This patch has got dependency on the DA9055 MFD core.
This patch is functionally tested on SMDK6410 board.
Signed-off-by David Dajun Chen <[email protected]>
Signed-off-by Ashish Jangam <[email protected]>
This looks OK
Acked-by Linus Walleij <[email protected]>
But I want Marks comment on this, for example:
drivers/gpio/gpio-da9055.c
Quoted 8 lines
drivers/gpio/gpio-da9055.c new file +8 -0
+static int da9055_gpio_get(struct gpio_chip *gc, unsigned offset)
+{
+       struct da9055_gpio *gpio = to_da9055_gpio(gc);
+       int gpio_direction = 0;
+       int ret;
+
+       /* Get GPIO direction */
+       ret = da9055_reg_read(gpio->da9055, (offset >> 1) +
DA9055_REG_GPIO0_1);
Review
Quoted 2 lines
(quoted diff) +2 -0
+       if (ret < 0)
+               return ret;
So unique functions to read/write registers (as I'm used to).

Quoted 9 lines

But the parent driver is using regmap, sand you seem not to use the struct da9055
for anything else than passing reads/writes, so isn't it simpler to just pass
the struct regmap * and use update_bits etc directly and remove a layer of indirection?
I'm very uncertain but Mark will know the proper design pattern.

As Mark was fine with this then will it be possible to upstream this patch?

Quoted 2 lines

Yours,
Linus Walleij

Linus Walleij

2012-10-12 21:17 UTC #5 lore raw

In reply to Ashish Jangam

Quoted 7 lines · Ashish Jangam

This is the GPIO patch for the DA9055 PMIC. This patch has got dependency on the DA9055 MFD core.
This patch is functionally tested on SMDK6410 board.
Signed-off-by David Dajun Chen <[email protected]>
Signed-off-by Ashish Jangam <[email protected]>

Patch applied!

Thanks,
Linus Walleij