Linux-RTC Archive on lore.kernel.org
 help / Atom feed
* [RFC PATCH v2 07/10] gpio: Initial support for ROHM bd70528 GPIO block
@ 2019-01-25 11:05 Matti Vaittinen
  2019-01-28 12:55 ` Linus Walleij
  0 siblings, 1 reply; 3+ messages in thread
From: Matti Vaittinen @ 2019-01-25 11:05 UTC (permalink / raw)
  To: mazziesaccount, matti.vaittinen
  Cc: heikki.haikola, mikko.mutanen, lee.jones, robh+dt, mark.rutland,
	broonie, gregkh, rafael, mturquette, sboyd, linus.walleij,
	bgolaszewski, sre, lgirdwood, a.zummo, alexandre.belloni, wim,
	linux, devicetree, linux-kernel, linux-clk, linux-gpio, linux-pm,
	linux-rtc, linux-watchdog

ROHM BD70528 PMIC has 4 GPIO pins. Allow them to be
controlled by GPIO framework.

IRQs are handled by regmap-irq and GPIO driver is not
aware of the irq usage.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 drivers/gpio/Kconfig        |  11 +++
 drivers/gpio/Makefile       |   1 +
 drivers/gpio/gpio-bd70528.c | 192 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 204 insertions(+)
 create mode 100644 drivers/gpio/gpio-bd70528.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index b5a2845347ec..c82187648630 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -962,6 +962,17 @@ config GPIO_ARIZONA
 	help
 	  Support for GPIOs on Wolfson Arizona class devices.
 
+config GPIO_BD70528
+	tristate "ROHM BD70528 GPIO support"
+	depends on MFD_ROHM_BD70528
+	help
+	  Support for GPIOs on ROHM BD70528 PMIC. There are four GPIOs
+	  available on the ROHM PMIC in total. The GPIOs can also
+	  generate interrupts.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called gpio-bd70528.
+
 config GPIO_BD9571MWV
 	tristate "ROHM BD9571 GPIO support"
 	depends on MFD_BD9571MWV
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 37628f8dbf70..6f12c83598fc 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_GPIO_ATH79)	+= gpio-ath79.o
 obj-$(CONFIG_GPIO_ASPEED)	+= gpio-aspeed.o
 obj-$(CONFIG_GPIO_RASPBERRYPI_EXP)	+= gpio-raspberrypi-exp.o
 obj-$(CONFIG_GPIO_BCM_KONA)	+= gpio-bcm-kona.o
+obj-$(CONFIG_GPIO_BD70528) 	+= gpio-bd70528.o
 obj-$(CONFIG_GPIO_BD9571MWV)	+= gpio-bd9571mwv.o
 obj-$(CONFIG_GPIO_BRCMSTB)	+= gpio-brcmstb.o
 obj-$(CONFIG_GPIO_BT8XX)	+= gpio-bt8xx.o
diff --git a/drivers/gpio/gpio-bd70528.c b/drivers/gpio/gpio-bd70528.c
new file mode 100644
index 000000000000..684d6f744409
--- /dev/null
+++ b/drivers/gpio/gpio-bd70528.c
@@ -0,0 +1,192 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 ROHM Semiconductors
+// gpio-bd70528.c ROHM BD70528MWV gpio driver
+
+#include <linux/gpio/driver.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/rohm-bd70528.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define GPIO_IN_REG(offset) (BD70528_REG_GPIO1_IN + offset*2)
+#define GPIO_OUT_REG(offset) (BD70528_REG_GPIO1_OUT + offset*2)
+
+struct bd70528_gpio {
+	struct rohm_regmap_dev chip;
+	struct gpio_chip gpio;
+};
+
+static int bd70528_set_debounce(struct bd70528_gpio *bdgpio,
+				unsigned int offset, unsigned int debounce)
+{
+	u8 val;
+
+	switch (debounce) {
+	case 0:
+		val = BD70528_DEBOUNCE_DISABLE;
+		break;
+	case 1 ... 15:
+		val = BD70528_DEBOUNCE_15MS;
+		break;
+	case 16 ... 30:
+		val = BD70528_DEBOUNCE_30MS;
+		break;
+	case 31 ... 50:
+		val = BD70528_DEBOUNCE_50MS;
+		break;
+	default:
+		dev_err(bdgpio->chip.dev,
+			"Invalid debouce value %u\n", debounce);
+		return -EINVAL;
+	}
+	return regmap_update_bits(bdgpio->chip.regmap, GPIO_IN_REG(offset),
+				 BD70528_DEBOUNCE_MASK, val);
+}
+
+static int bd70528_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+	struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
+	int val, ret;
+
+	/* Do we need to do something to IRQs here? */
+	ret = regmap_read(bdgpio->chip.regmap, GPIO_OUT_REG(offset), &val);
+	if (ret) {
+		dev_err(bdgpio->chip.dev, "Could not read gpio direction\n");
+		return ret;
+	}
+
+	return !(val & BD70528_GPIO_OUT_EN_MASK);
+}
+
+static int bd70528_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
+				   unsigned long config)
+{
+	struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
+
+	switch (pinconf_to_config_param(config)) {
+	case PIN_CONFIG_DRIVE_OPEN_DRAIN:
+		return regmap_update_bits(bdgpio->chip.regmap,
+					  GPIO_OUT_REG(offset),
+					  BD70528_GPIO_DRIVE_MASK,
+					  BD70528_GPIO_OPEN_DRAIN);
+		break;
+	case PIN_CONFIG_DRIVE_PUSH_PULL:
+		return regmap_update_bits(bdgpio->chip.regmap,
+					  GPIO_OUT_REG(offset),
+					  BD70528_GPIO_DRIVE_MASK,
+					  BD70528_GPIO_PUSH_PULL);
+		break;
+	case PIN_CONFIG_INPUT_DEBOUNCE:
+		return bd70528_set_debounce(bdgpio, offset,
+					    pinconf_to_config_argument(config));
+		break;
+	default:
+		break;
+	}
+	return -ENOTSUPP;
+}
+
+static int bd70528_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+	struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
+
+	/* Do we need to do something to IRQs here? */
+	return regmap_update_bits(bdgpio->chip.regmap, GPIO_OUT_REG(offset),
+				 BD70528_GPIO_OUT_EN_MASK,
+				 BD70528_GPIO_OUT_DISABLE);
+}
+static void bd70528_gpio_set(struct gpio_chip *chip, unsigned int offset,
+			     int value)
+{
+	int ret;
+	struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
+	u8 val = (value) ? BD70528_GPIO_OUT_HI : BD70528_GPIO_OUT_LO;
+
+	ret = regmap_update_bits(bdgpio->chip.regmap, GPIO_OUT_REG(offset),
+				  BD70528_GPIO_OUT_MASK, val);
+	if (ret)
+		dev_err(bdgpio->chip.dev, "Could not set gpio to %d\n", value);
+}
+
+static int bd70528_direction_output(struct gpio_chip *chip, unsigned int offset,
+				    int value)
+{
+	struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
+
+	bd70528_gpio_set(chip, offset, value);
+	return regmap_update_bits(bdgpio->chip.regmap, GPIO_OUT_REG(offset),
+				 BD70528_GPIO_OUT_EN_MASK,
+				 BD70528_GPIO_OUT_ENABLE);
+}
+
+#define GPIO_STATE_MASK(offset) (BD70528_GPIO_IN_STATE_BASE << offset)
+
+static int bd70528_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	unsigned int val;
+	int ret = -EINVAL;
+	struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
+
+	ret = regmap_read(bdgpio->chip.regmap, BD70528_REG_GPIO_STATE, &val);
+
+	if (!ret)
+		ret = !(val & GPIO_STATE_MASK(offset));
+
+	return ret;
+}
+
+static int  bd70528_probe(struct platform_device *pdev)
+{
+	struct bd70528_gpio *bdgpio;
+	struct bd70528 *bd70528;
+	int ret;
+
+	bd70528 = dev_get_drvdata(pdev->dev.parent);
+	if (!bd70528) {
+		dev_err(&pdev->dev, "No MFD driver data\n");
+		return -EINVAL;
+	}
+
+	bdgpio = devm_kzalloc(&pdev->dev, sizeof(*bdgpio),
+				    GFP_KERNEL);
+	if (!bdgpio)
+		return -ENOMEM;
+	bdgpio->chip.dev = &pdev->dev;
+	bdgpio->gpio.parent = pdev->dev.parent;
+	bdgpio->gpio.label = "bd70528-gpio";
+	bdgpio->gpio.owner = THIS_MODULE;
+	bdgpio->gpio.get_direction = &bd70528_get_direction;
+	bdgpio->gpio.direction_input = &bd70528_direction_input;
+	bdgpio->gpio.direction_output = &bd70528_direction_output;
+	bdgpio->gpio.set_config = &bd70528_gpio_set_config;
+	bdgpio->gpio.can_sleep = true;
+	bdgpio->gpio.get = &bd70528_gpio_get;
+	bdgpio->gpio.set = &bd70528_gpio_set;
+	bdgpio->gpio.ngpio = 4;
+	bdgpio->gpio.base = -1;
+#ifdef CONFIG_OF_GPIO
+	bdgpio->gpio.of_node = pdev->dev.parent->of_node;
+#endif
+	bdgpio->chip.regmap = bd70528->chip.regmap;
+
+	ret = devm_gpiochip_add_data(&pdev->dev, &bdgpio->gpio,
+				     bdgpio);
+	if (ret)
+		dev_err(&pdev->dev, "gpio_init: Failed to add bd70528-gpio\n");
+
+	return ret;
+}
+
+static struct platform_driver bd70528_gpio = {
+	.driver = {
+		.name = "bd70528-gpio"
+	},
+	.probe = bd70528_probe,
+};
+
+module_platform_driver(bd70528_gpio);
+
+MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
+MODULE_DESCRIPTION("BD70528 voltage regulator driver");
+MODULE_LICENSE("GPL");
-- 
2.14.3


-- 
Matti Vaittinen
ROHM Semiconductors

~~~ "I don't think so," said Rene Descartes.  Just then, he vanished ~~~

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH v2 07/10] gpio: Initial support for ROHM bd70528 GPIO block
  2019-01-25 11:05 [RFC PATCH v2 07/10] gpio: Initial support for ROHM bd70528 GPIO block Matti Vaittinen
@ 2019-01-28 12:55 ` Linus Walleij
  2019-01-28 14:04   ` Matti Vaittinen
  0 siblings, 1 reply; 3+ messages in thread
From: Linus Walleij @ 2019-01-28 12:55 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Matti Vaittinen, heikki.haikola, mikko.mutanen, Lee Jones,
	Rob Herring, Mark Rutland, Mark Brown, Greg KH,
	Rafael J. Wysocki, Michael Turquette, Stephen Boyd,
	Bartosz Golaszewski, Sebastian Reichel, Liam Girdwood,
	Alessandro Zummo, Alexandre Belloni, Wim Van Sebroeck,
	Guenter Roeck,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel, linux-clk, open list:GPIO SUBSYSTEM, Linux PM list,
	linux-rtc, LINUXWATCHDOG

Hi Matti!

Thanks for your patch!

On Fri, Jan 25, 2019 at 12:05 PM Matti Vaittinen
<matti.vaittinen@fi.rohmeurope.com> wrote:

> ROHM BD70528 PMIC has 4 GPIO pins. Allow them to be
> controlled by GPIO framework.
>
> IRQs are handled by regmap-irq and GPIO driver is not
> aware of the irq usage.
>
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>

This is overall a very nicely written driver.

Just small comments:

> +#include <linux/gpio/driver.h>
> +#include <linux/interrupt.h>

Why interrupt? You do not use it.

> +static int bd70528_get_direction(struct gpio_chip *chip, unsigned int offset)
> +{
> +       struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
> +       int val, ret;
> +
> +       /* Do we need to do something to IRQs here? */

Well you don't support IRQs yet so no problem as long as they're masked?

> +       ret = regmap_read(bdgpio->chip.regmap, GPIO_OUT_REG(offset), &val);
> +       if (ret) {
> +               dev_err(bdgpio->chip.dev, "Could not read gpio direction\n");
> +               return ret;
> +       }
> +
> +       return !(val & BD70528_GPIO_OUT_EN_MASK);
> +}
> +
> +static int bd70528_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
> +                                  unsigned long config)

This is very nice. With Thomas Petazzoni's ongoing work you will be
able to also support pull up/down if you need it.

> +static int bd70528_direction_input(struct gpio_chip *chip, unsigned int offset)
> +{
> +       struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
> +
> +       /* Do we need to do something to IRQs here? */

Hmmm?

Apart from that it looks good. Feel free to add:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
on the next iteration.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH v2 07/10] gpio: Initial support for ROHM bd70528 GPIO block
  2019-01-28 12:55 ` Linus Walleij
@ 2019-01-28 14:04   ` Matti Vaittinen
  0 siblings, 0 replies; 3+ messages in thread
From: Matti Vaittinen @ 2019-01-28 14:04 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Matti Vaittinen, heikki.haikola, mikko.mutanen, Lee Jones,
	Rob Herring, Mark Rutland, Mark Brown, Greg KH,
	Rafael J. Wysocki, Michael Turquette, Stephen Boyd,
	Bartosz Golaszewski, Sebastian Reichel, Liam Girdwood,
	Alessandro Zummo, Alexandre Belloni, Wim Van Sebroeck,
	Guenter Roeck,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel, linux-clk, open list:GPIO SUBSYSTEM, Linux PM list,
	linux-rtc, LINUXWATCHDOG

On Mon, Jan 28, 2019 at 01:55:48PM +0100, Linus Walleij wrote:
> Hi Matti!
> 
> Thanks for your patch!

No, thank you for the review =)

> On Fri, Jan 25, 2019 at 12:05 PM Matti Vaittinen
> <matti.vaittinen@fi.rohmeurope.com> wrote:
> 
> > ROHM BD70528 PMIC has 4 GPIO pins. Allow them to be
> > controlled by GPIO framework.
> >
> > IRQs are handled by regmap-irq and GPIO driver is not
> > aware of the irq usage.
> >
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> 
> This is overall a very nicely written driver.
> 
> Just small comments:
> 
> > +#include <linux/gpio/driver.h>
> > +#include <linux/interrupt.h>
> 
> Why interrupt? You do not use it.

Right, I'd better to drop it.

> 
> > +static int bd70528_get_direction(struct gpio_chip *chip, unsigned int offset)
> > +{
> > +       struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
> > +       int val, ret;
> > +
> > +       /* Do we need to do something to IRQs here? */
> 
> Well you don't support IRQs yet so no problem as long as they're masked?

I may have not worded this correctly. These pins can be used for IRQs.
HW supports it and I added the irq registers in regmap-irq controller
(please see the MFD part). It's just that this GPIO driver is currently not
aware of that. This comment here is a reminder for me to figure out if
we should look whether the pin is used for I/O or for irqs here - and
what should we return if pin is configured to be used for IRQs.

> > +       ret = regmap_read(bdgpio->chip.regmap, GPIO_OUT_REG(offset), &val);
> > +       if (ret) {
> > +               dev_err(bdgpio->chip.dev, "Could not read gpio direction\n");
> > +               return ret;
> > +       }
> > +
> > +       return !(val & BD70528_GPIO_OUT_EN_MASK);
> > +}
> > +
> > +static int bd70528_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
> > +                                  unsigned long config)
> 
> This is very nice. With Thomas Petazzoni's ongoing work you will be
> able to also support pull up/down if you need it.
> 
> > +static int bd70528_direction_input(struct gpio_chip *chip, unsigned int offset)
> > +{
> > +       struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
> > +
> > +       /* Do we need to do something to IRQs here? */
> 
> Hmmm?

Same thing as with the get_direction. Should we allow setting the
direction if pin is already configured as an irq source? Should we
somehow try to warn user if it seems the pin is tried to be used for
both irqs and I/O?

> 
> Apart from that it looks good. Feel free to add:
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> on the next iteration.

I will need to do some further testing on this before submitting next
version. I will gladly maintain your Reviewed-by if I don't do any
bigger changes to the driver - but if I do I'll drop your tag and ask
for re-review :)

Br,
	Matti Vaittinen

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-25 11:05 [RFC PATCH v2 07/10] gpio: Initial support for ROHM bd70528 GPIO block Matti Vaittinen
2019-01-28 12:55 ` Linus Walleij
2019-01-28 14:04   ` Matti Vaittinen

Linux-RTC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-rtc/0 linux-rtc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-rtc linux-rtc/ https://lore.kernel.org/linux-rtc \
		linux-rtc@vger.kernel.org linux-rtc@archiver.kernel.org
	public-inbox-index linux-rtc


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-rtc


AGPL code for this site: git clone https://public-inbox.org/ public-inbox