[v2,12/18] gpio: madera: Support Cirrus Logic Madera class codecs
diff mbox

Message ID 1493050124-5970-13-git-send-email-rf@opensource.wolfsonmicro.com
State New
Headers show

Commit Message

Richard Fitzgerald April 24, 2017, 4:08 p.m. UTC
This adds support for the GPIOs on Cirrus Logic Madera class codecs.
Any pins not used for special functions (see the pinctrl driver) can be
used as general single-bit input or output lines. The number of available
GPIOs varies between codecs.

Signed-off-by: Nariman Poushin <nariman@opensource.wolfsonmicro.com>
Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
Changes from V1:
- dt bindings moved to a separate patch
- dependent on pinctrl driver instead of parent MFD
- added get_direction function
- added .request / .free / .set_config to work with pinctrl driver
- register range with pinctrl driver

 MAINTAINERS                |   2 +
 drivers/gpio/Kconfig       |   6 ++
 drivers/gpio/Makefile      |   1 +
 drivers/gpio/gpio-madera.c | 196 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 205 insertions(+)
 create mode 100644 drivers/gpio/gpio-madera.c

Comments

Linus Walleij April 25, 2017, 2:13 p.m. UTC | #1
On Mon, Apr 24, 2017 at 6:08 PM, Richard Fitzgerald
<rf@opensource.wolfsonmicro.com> wrote:

> This adds support for the GPIOs on Cirrus Logic Madera class codecs.
> Any pins not used for special functions (see the pinctrl driver) can be
> used as general single-bit input or output lines. The number of available
> GPIOs varies between codecs.
>
> Signed-off-by: Nariman Poushin <nariman@opensource.wolfsonmicro.com>
> Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
> Changes from V1:
> - dt bindings moved to a separate patch
> - dependent on pinctrl driver instead of parent MFD
> - added get_direction function
> - added .request / .free / .set_config to work with pinctrl driver
> - register range with pinctrl driver

Nice, but...

> +       ret = gpiochip_add_pin_range(&madera_gpio->gpio_chip, "madera-pinctrl",
> +                                    0, 0, madera_gpio->gpio_chip.ngpio);
> +       if (ret) {
> +               dev_warn(&pdev->dev, "Failed to add pin range (%d)\n", ret);
> +               return ret;
> +       }

This is all fine, but we have generic code for adding ranges from
the device tree, see
Documentation/devicetree/bindings/gpio/gpio.txt

With that this range should not even be needed.
Apart from that it looks pretty solid.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Richard Fitzgerald April 25, 2017, 2:44 p.m. UTC | #2
On Tue, 2017-04-25 at 16:13 +0200, Linus Walleij wrote:
> On Mon, Apr 24, 2017 at 6:08 PM, Richard Fitzgerald
> <rf@opensource.wolfsonmicro.com> wrote:
> 
> > This adds support for the GPIOs on Cirrus Logic Madera class codecs.
> > Any pins not used for special functions (see the pinctrl driver) can be
> > used as general single-bit input or output lines. The number of available
> > GPIOs varies between codecs.
> >
> > Signed-off-by: Nariman Poushin <nariman@opensource.wolfsonmicro.com>
> > Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
> > Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> > ---
> > Changes from V1:
> > - dt bindings moved to a separate patch
> > - dependent on pinctrl driver instead of parent MFD
> > - added get_direction function
> > - added .request / .free / .set_config to work with pinctrl driver
> > - register range with pinctrl driver
> 
> Nice, but...
> 
> > +       ret = gpiochip_add_pin_range(&madera_gpio->gpio_chip, "madera-pinctrl",
> > +                                    0, 0, madera_gpio->gpio_chip.ngpio);
> > +       if (ret) {
> > +               dev_warn(&pdev->dev, "Failed to add pin range (%d)\n", ret);
> > +               return ret;
> > +       }
> 
> This is all fine, but we have generic code for adding ranges from
> the device tree, see
> Documentation/devicetree/bindings/gpio/gpio.txt
> 

The range of gpio pins is a fixed property of the chip, and so is the
combination of gpio+pinctrl drivers.

I think the general principle of the DT maintainers is that DT should be
used for things that the drivers don't already know and can't figure
out.

> With that this range should not even be needed.
> Apart from that it looks pretty solid.
> 
> Yours,
> Linus Walleij


--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij April 28, 2017, 7:44 a.m. UTC | #3
On Mon, Apr 24, 2017 at 6:08 PM, Richard Fitzgerald
<rf@opensource.wolfsonmicro.com> wrote:

> This adds support for the GPIOs on Cirrus Logic Madera class codecs.
> Any pins not used for special functions (see the pinctrl driver) can be
> used as general single-bit input or output lines. The number of available
> GPIOs varies between codecs.
>
> Signed-off-by: Nariman Poushin <nariman@opensource.wolfsonmicro.com>
> Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
> Changes from V1:
> - dt bindings moved to a separate patch
> - dependent on pinctrl driver instead of parent MFD
> - added get_direction function
> - added .request / .free / .set_config to work with pinctrl driver
> - register range with pinctrl driver

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij April 28, 2017, 7:46 a.m. UTC | #4
On Tue, Apr 25, 2017 at 4:44 PM, Richard Fitzgerald
<rf@opensource.wolfsonmicro.com> wrote:
> On Tue, 2017-04-25 at 16:13 +0200, Linus Walleij wrote:
>> On Mon, Apr 24, 2017 at 6:08 PM, Richard Fitzgerald
>> <rf@opensource.wolfsonmicro.com> wrote:
>>
>> > This adds support for the GPIOs on Cirrus Logic Madera class codecs.
>> > Any pins not used for special functions (see the pinctrl driver) can be
>> > used as general single-bit input or output lines. The number of available
>> > GPIOs varies between codecs.
>> >
>> > Signed-off-by: Nariman Poushin <nariman@opensource.wolfsonmicro.com>
>> > Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
>> > Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
>> > ---
>> > Changes from V1:
>> > - dt bindings moved to a separate patch
>> > - dependent on pinctrl driver instead of parent MFD
>> > - added get_direction function
>> > - added .request / .free / .set_config to work with pinctrl driver
>> > - register range with pinctrl driver
>>
>> Nice, but...
>>
>> > +       ret = gpiochip_add_pin_range(&madera_gpio->gpio_chip, "madera-pinctrl",
>> > +                                    0, 0, madera_gpio->gpio_chip.ngpio);
>> > +       if (ret) {
>> > +               dev_warn(&pdev->dev, "Failed to add pin range (%d)\n", ret);
>> > +               return ret;
>> > +       }
>>
>> This is all fine, but we have generic code for adding ranges from
>> the device tree, see
>> Documentation/devicetree/bindings/gpio/gpio.txt
>>
>
> The range of gpio pins is a fixed property of the chip, and so is the
> combination of gpio+pinctrl drivers.

Well so is the IRQ number, but we still put that in the device tree.

> I think the general principle of the DT maintainers is that DT should be
> used for things that the drivers don't already know and can't figure
> out.

It's a grayzone. People use ranges in the device tree for example when
there are several instances of the same GPIO block with different
ranges into a single pin controller and then it makes sense.

But in this case you have your separate chip with one instance so
it doesn't make sense, keep it like this.

Also it is necessary to proble from boardfiles as you explained in the
pinctrl patch.

Good job, it's a nice driver. Also the pinctrl driver is very nice.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/MAINTAINERS b/MAINTAINERS
index 3e42a03..66c5263 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3304,10 +3304,12 @@  L:	patches@opensource.wolfsonmicro.com
 T:	git https://github.com/CirrusLogic/linux-drivers.git
 W:	https://github.com/CirrusLogic/linux-drivers/wiki
 S:	Supported
+F:	Documentation/devicetree/bindings/gpio/gpio-madera.txt
 F:	Documentation/devicetree/bindings/mfd/madera.txt
 F:	Documentation/devicetree/bindings/pinctrl/cirrus,madera-pinctrl.txt
 F:	include/linux/irqchip/irq-madera*
 F:	include/linux/mfd/madera/*
+F:	drivers/gpio/gpio-madera*
 F:	drivers/irqchip/irq-madera*
 F:	drivers/mfd/madera*
 F:	drivers/mfd/cs47l*
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 05e8fbf..9488abe 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -941,6 +941,12 @@  config GPIO_LP873X
 	  This driver can also be built as a module. If so, the module will be
           called gpio-lp873x.
 
+config GPIO_MADERA
+	bool "Cirrus Logic Madera class codecs"
+	depends on PINCTRL_MADERA
+	help
+	  Support for GPIOs on Cirrus Logic Madera class codecs.
+
 config GPIO_MAX77620
 	tristate "GPIO support for PMIC MAX77620 and MAX20024"
 	depends on MFD_MAX77620
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 095598e..d4b6c30 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -66,6 +66,7 @@  obj-$(CONFIG_GPIO_LPC18XX)	+= gpio-lpc18xx.o
 obj-$(CONFIG_ARCH_LPC32XX)	+= gpio-lpc32xx.o
 obj-$(CONFIG_GPIO_LP873X)	+= gpio-lp873x.o
 obj-$(CONFIG_GPIO_LYNXPOINT)	+= gpio-lynxpoint.o
+obj-$(CONFIG_GPIO_MADERA)	+= gpio-madera.o
 obj-$(CONFIG_GPIO_MAX730X)	+= gpio-max730x.o
 obj-$(CONFIG_GPIO_MAX7300)	+= gpio-max7300.o
 obj-$(CONFIG_GPIO_MAX7301)	+= gpio-max7301.o
diff --git a/drivers/gpio/gpio-madera.c b/drivers/gpio/gpio-madera.c
new file mode 100644
index 0000000..37ac706
--- /dev/null
+++ b/drivers/gpio/gpio-madera.c
@@ -0,0 +1,196 @@ 
+/*
+ * GPIO support for Cirrus Logic Madera codecs
+ *
+ * Copyright 2015-2017 Cirrus Logic
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/device.h>
+#include <linux/gpio.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include <linux/mfd/madera/core.h>
+#include <linux/mfd/madera/pdata.h>
+#include <linux/mfd/madera/registers.h>
+
+struct madera_gpio {
+	struct madera *madera;
+	struct gpio_chip gpio_chip;
+};
+
+static int madera_gpio_get_direction(struct gpio_chip *chip,
+				     unsigned int offset)
+{
+	struct madera_gpio *madera_gpio = gpiochip_get_data(chip);
+	struct madera *madera = madera_gpio->madera;
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(madera->regmap,
+			  MADERA_GPIO1_CTRL_2 + (2 * offset), &val);
+	if (ret < 0)
+		return ret;
+
+	return (val & MADERA_GP1_DIR_MASK) >> MADERA_GP1_DIR_SHIFT;
+}
+
+static int madera_gpio_direction_in(struct gpio_chip *chip, unsigned int offset)
+{
+	struct madera_gpio *madera_gpio = gpiochip_get_data(chip);
+	struct madera *madera = madera_gpio->madera;
+
+	return regmap_update_bits(madera->regmap,
+				  MADERA_GPIO1_CTRL_2 + (2 * offset),
+				  MADERA_GP1_DIR_MASK, MADERA_GP1_DIR);
+}
+
+static int madera_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct madera_gpio *madera_gpio = gpiochip_get_data(chip);
+	struct madera *madera = madera_gpio->madera;
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(madera->regmap,
+			  MADERA_GPIO1_CTRL_1 + (2 * offset), &val);
+	if (ret < 0)
+		return ret;
+
+	return !!(val & MADERA_GP1_LVL_MASK);
+}
+
+static int madera_gpio_direction_out(struct gpio_chip *chip,
+				     unsigned int offset, int value)
+{
+	struct madera_gpio *madera_gpio = gpiochip_get_data(chip);
+	struct madera *madera = madera_gpio->madera;
+	unsigned int regval;
+	int ret;
+
+	if (value)
+		regval = MADERA_GP1_LVL;
+	else
+		regval = 0;
+
+	ret = regmap_update_bits(madera->regmap,
+				 MADERA_GPIO1_CTRL_2 + (2 * offset),
+				 MADERA_GP1_DIR_MASK, 0);
+	if (ret < 0)
+		return ret;
+
+	return regmap_update_bits(madera->regmap,
+				  MADERA_GPIO1_CTRL_1 + (2 * offset),
+				  MADERA_GP1_LVL_MASK, regval);
+}
+
+static void madera_gpio_set(struct gpio_chip *chip, unsigned int offset,
+			    int value)
+{
+	struct madera_gpio *madera_gpio = gpiochip_get_data(chip);
+	struct madera *madera = madera_gpio->madera;
+	unsigned int regval;
+	int ret;
+
+	if (value)
+		regval = MADERA_GP1_LVL;
+	else
+		regval = 0;
+
+	ret = regmap_update_bits(madera->regmap,
+				 MADERA_GPIO1_CTRL_1 + (2 * offset),
+				 MADERA_GP1_LVL_MASK, regval);
+	if (ret)
+		dev_warn(madera->dev, "Failed to write to 0x%x (%d)\n",
+			 MADERA_GPIO1_CTRL_1 + (2 * offset), ret);
+}
+
+static struct gpio_chip template_chip = {
+	.label			= "madera",
+	.owner			= THIS_MODULE,
+	.request		= gpiochip_generic_request,
+	.free			= gpiochip_generic_free,
+	.get_direction		= madera_gpio_get_direction,
+	.direction_input	= madera_gpio_direction_in,
+	.get			= madera_gpio_get,
+	.direction_output	= madera_gpio_direction_out,
+	.set			= madera_gpio_set,
+	.set_config		= gpiochip_generic_config,
+	.can_sleep		= true,
+};
+
+static int madera_gpio_probe(struct platform_device *pdev)
+{
+	struct madera *madera = dev_get_drvdata(pdev->dev.parent);
+	struct madera_pdata *pdata = dev_get_platdata(madera->dev);
+	struct madera_gpio *madera_gpio;
+	int ret;
+
+	madera_gpio = devm_kzalloc(&pdev->dev, sizeof(*madera_gpio),
+				   GFP_KERNEL);
+	if (!madera_gpio)
+		return -ENOMEM;
+
+	madera_gpio->madera = madera;
+	madera_gpio->gpio_chip = template_chip;
+	madera_gpio->gpio_chip.parent = &pdev->dev;
+
+	if (IS_ENABLED(CONFIG_OF_GPIO))
+		madera_gpio->gpio_chip.of_node = pdev->dev.of_node;
+
+	switch (madera->type) {
+	case CS47L35:
+		madera_gpio->gpio_chip.ngpio = CS47L35_NUM_GPIOS;
+		break;
+	case CS47L85:
+	case WM1840:
+		madera_gpio->gpio_chip.ngpio = CS47L85_NUM_GPIOS;
+		break;
+	case CS47L90:
+	case CS47L91:
+		madera_gpio->gpio_chip.ngpio = CS47L90_NUM_GPIOS;
+		break;
+	default:
+		dev_err(&pdev->dev, "Unknown chip variant %d\n", madera->type);
+		return -EINVAL;
+	}
+
+	if (pdata && pdata->gpio_base)
+		madera_gpio->gpio_chip.base = pdata->gpio_base;
+	else
+		madera_gpio->gpio_chip.base = -1;
+
+	ret = devm_gpiochip_add_data(&pdev->dev, &madera_gpio->gpio_chip,
+				     madera_gpio);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Could not register gpiochip, %d\n", ret);
+		return ret;
+	}
+
+	ret = gpiochip_add_pin_range(&madera_gpio->gpio_chip, "madera-pinctrl",
+				     0, 0, madera_gpio->gpio_chip.ngpio);
+	if (ret) {
+		dev_warn(&pdev->dev, "Failed to add pin range (%d)\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static struct platform_driver madera_gpio_driver = {
+	.driver.name	= "madera-gpio",
+	.driver.owner	= THIS_MODULE,
+	.probe		= madera_gpio_probe,
+};
+
+module_platform_driver(madera_gpio_driver);
+
+MODULE_DESCRIPTION("GPIO interface for Madera codecs");
+MODULE_AUTHOR("Nariman Poushin <nariman@opensource.wolfsonmicro.com>");
+MODULE_AUTHOR("Richard Fitzgerald <rf@opensource.wolfsonmicro.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:madera-gpio");