Skip to contentSkip to conversation

gpio: Add support for Intel Crystal Cove PMIC

PATCH In review Zhu, Lejun 2 participants 3 messages May 29, 2014 – May 30, 2014

Subsystems (2)
lore
Share & tools
lore

Zhu, Lejun

author
#0 lore raw

PATCH 1/1 gpio: Add support for Intel Crystal Cove PMIC

Devices based on Intel SoC products such as Baytrail have a Power Management IC. In the PMIC there are subsystems for voltage regulation, A/D conversion, GPIO and PWMs. The PMIC in Baytrail-T platform is called Crystal Cove.

This patch adds support for the GPIO function in Crystal Cove.

---
v2:
- Use IRQ chip helper to provide irqdomain.
- Implement .remove and can now build as a module.
- Various fix for unreadable or ugly code pieces.
v3:
- More fix in irq_handler and probe.
v4:
- Minor fix of one return statement.
v5:
- intel_soc_pmic_readb/writeb/setb/clearb are removed from the MFD driver, so use regmap_* API here instead.
- Check for unsupported IRQ type and return -EINVAL.
- Create to_cg() and to_reg() inline functions to make the code readable.

---
 drivers/gpio/Kconfig            |  13 ++
 drivers/gpio/Makefile           |   1 +
 drivers/gpio/gpio-crystalcove.c | 380 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 394 insertions(+)
 create mode 100644 drivers/gpio/gpio-crystalcove.c
drivers/gpio/Kconfig +13 -0
index a86c49a..fed08d9d 100644
@@ -440,6 +440,19 @@ config GPIO_ARIZONA
 	help
 	  Support for GPIOs on Wolfson Arizona class devices.
 
+config GPIO_CRYSTAL_COVE
+	tristate "GPIO support for Crystal Cove PMIC"
+	depends on INTEL_SOC_PMIC
+	select GPIOLIB_IRQCHIP
+	help
+	  Support for GPIO pins on Crystal Cove PMIC.
+
+	  Say Yes if you have a Intel SoC based tablet with Crystal Cove PMIC
+	  inside.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called gpio-crystalcove.
+
 config GPIO_LP3943
 	tristate "TI/National Semiconductor LP3943 GPIO expander"
 	depends on MFD_LP3943
drivers/gpio/Makefile +1 -0
index 6309aff..e6cd935 100644
@@ -20,6 +20,7 @@ obj-$(CONFIG_GPIO_BCM_KONA)	+= gpio-bcm-kona.o
 obj-$(CONFIG_GPIO_BT8XX)	+= gpio-bt8xx.o
 obj-$(CONFIG_GPIO_CLPS711X)	+= gpio-clps711x.o
 obj-$(CONFIG_GPIO_CS5535)	+= gpio-cs5535.o
+obj-$(CONFIG_GPIO_CRYSTAL_COVE)	+= gpio-crystalcove.o
 obj-$(CONFIG_GPIO_DA9052)	+= gpio-da9052.o
 obj-$(CONFIG_GPIO_DA9055)	+= gpio-da9055.o
 obj-$(CONFIG_GPIO_DAVINCI)	+= gpio-davinci.o
drivers/gpio/gpio-crystalcove.c new file +380 -0

380 lines not on this page · raw message

Signature · 1 line
1.8.3.2
#1 lore raw
Quoted 9 lines · Lejun

Devices based on Intel SoC products such as Baytrail have a Power Management IC. In the PMIC there are subsystems for voltage regulation, A/D conversion, GPIO and PWMs. The PMIC in Baytrail-T platform is called Crystal Cove.

This patch adds support for the GPIO function in Crystal Cove.

You have gotten several Reviewed-by and Acked-by tags for this driver have you not? Please collect them here so I don't have to do it.

Quoted 7 lines

---
v2:
- Use IRQ chip helper to provide irqdomain.
- Implement .remove and can now build as a module.
- Various fix for unreadable or ugly code pieces. v3:
- More fix in irq_handler and probe.

Quoted 8 lines

v4:
- Minor fix of one return statement.
v5:
- intel_soc_pmic_readb/writeb/setb/clearb are removed from the MFD driver, so use regmap_* API here instead.
- Check for unsupported IRQ type and return -EINVAL.
- Create to_cg() and to_reg() inline functions to make the code readable.

Pretty cool eh? :-)

Quoted 1 line

+#include <linux/mfd/intel_soc_pmic.h>

As you realize I cannot merge this patch until that header file exists, so the MFD core patch needs to go in first.

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define NUM_GPIO               16

Too generic name, say CRYSTAL_NUM_GPIO or something.

Quoted 6 lines
drivers/gpio/gpio-crystalcove.c new file +6 -0
+       gpiochip_irqchip_add(&cg->chip, &crystalcove_irqchip, 0,
+                            handle_simple_irq, IRQ_TYPE_NONE);
+
+       retval = request_threaded_irq(irq, NULL, crystalcove_gpio_irq_handler,
+                                     IRQF_ONESHOT, KBUILD_MODNAME, cg);
+

drivers/gpio/gpio-crystalcove.c line 343

Quoted 10 lines
drivers/gpio/gpio-crystalcove.c new file +10 -0
+       if (retval) {
+               dev_warn(&pdev->dev, "request irq failed: %d\n", retval);
+               goto out;
+       }
+
+       retval = gpiochip_add(&cg->chip);
+       if (retval) {
+               dev_warn(&pdev->dev, "add gpio chip error: %d\n", retval);
+               goto out_free_irq;
+       }

As mentioned, swap these around.

But the driver is looking real good now, if you just fix the above you have my Reviewed-by so you can merge it along with the MFD patch in the MFD tree if you like.

Yours,
Linus Walleij

author
#2 lore raw
Quoted 7 lines · Linus Walleij
Quoted 5 lines · Lejun

Devices based on Intel SoC products such as Baytrail have a Power Management IC. In the PMIC there are subsystems for voltage regulation, A/D conversion, GPIO and PWMs. The PMIC in Baytrail-T platform is called Crystal Cove.

Quoted 8 lines
Quoted 4 lines

This patch adds support for the GPIO function in Crystal Cove.

You have gotten several Reviewed-by and Acked-by tags for this driver have you not? Please collect them here so I don't have to do it.

I'll do it.

Quoted 48 lines
Quoted 7 lines

---
v2:
- Use IRQ chip helper to provide irqdomain.
- Implement .remove and can now build as a module.
- Various fix for unreadable or ugly code pieces. v3:
- More fix in irq_handler and probe.

Quoted 8 lines

v4:
- Minor fix of one return statement.
v5:
- intel_soc_pmic_readb/writeb/setb/clearb are removed from the MFD driver, so use regmap_* API here instead.
- Check for unsupported IRQ type and return -EINVAL.
- Create to_cg() and to_reg() inline functions to make the code readable.

Pretty cool eh? :-)

Quoted 1 line

+#include <linux/mfd/intel_soc_pmic.h>

As you realize I cannot merge this patch until that header file exists, so the MFD core patch needs to go in first.

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define NUM_GPIO               16

Too generic name, say CRYSTAL_NUM_GPIO or something.

Quoted 6 lines
drivers/gpio/gpio-crystalcove.c new file +6 -0
+       gpiochip_irqchip_add(&cg->chip, &crystalcove_irqchip, 0,
+                            handle_simple_irq, IRQ_TYPE_NONE);
+
+       retval = request_threaded_irq(irq, NULL, crystalcove_gpio_irq_handler,
+                                     IRQF_ONESHOT, KBUILD_MODNAME, cg);
+

drivers/gpio/gpio-crystalcove.c line 343

Quoted 10 lines
drivers/gpio/gpio-crystalcove.c new file +10 -0
+       if (retval) {
+               dev_warn(&pdev->dev, "request irq failed: %d\n", retval);
+               goto out;
+       }
+
+       retval = gpiochip_add(&cg->chip);
+       if (retval) {
+               dev_warn(&pdev->dev, "add gpio chip error: %d\n", retval);
+               goto out_free_irq;
+       }

As mentioned, swap these around.

Quoted 3 lines

But the driver is looking real good now, if you just fix the above you have my Reviewed-by so you can merge it along with the MFD patch in the MFD tree if you like.

Thank you. I will ask Lee Jones if he will do that.

Best Regards
Lejun