diff mbox

gpio: Add driver for MEN 16Z127 GPIO controller

Message ID 1444068553-16899-1-git-send-email-andy@wernerandy.de
State New
Headers show

Commit Message

Andreas Werner Oct. 5, 2015, 6:09 p.m. UTC 
The 16Z127 is a GPIO controller on a MCB FPGA and has 32
configurable GPIOs.
The GPIOs can be configured as inputs and outputs

Signed-off-by: Andreas Werner <andy@wernerandy.de>
---
 drivers/gpio/Kconfig        |   6 ++
 drivers/gpio/Makefile       |   1 +
 drivers/gpio/gpio-menz127.c | 186 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 193 insertions(+)
 create mode 100644 drivers/gpio/gpio-menz127.c

Comments

Linus Walleij Oct. 6, 2015, 7:35 a.m. UTC | #1 
On Mon, Oct 5, 2015 at 8:09 PM, Andreas Werner <andy@wernerandy.de> wrote:

> The 16Z127 is a GPIO controller on a MCB FPGA and has 32
> configurable GPIOs.
> The GPIOs can be configured as inputs and outputs
>
> Signed-off-by: Andreas Werner <andy@wernerandy.de>

This driver looks like it can use the generic MMIO library.

select GPIO_GENERIC
#include <linux/basic_mmio_gpio.h>

Then look at example for how to do this, e.g.
drivers/gpio/gpio-74xx-mmio.c

> +config GPIO_MENZ127
> +       tristate "MEN 16Z127 GPIO support"
> +       depends on MCB

Is this "MCB" symbol already upstream? It seems a bit short.

> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/io.h>
> +#include <linux/mcb.h>
> +#include <linux/gpio.h>

Just
#include <linux/gpio/driver.h>

> +#define MEN_Z127_CTRL  0x00
> +#define MEN_Z127_PSR   0x04
> +#define MEN_Z127_IRQR  0x08
> +#define MEN_Z127_GPIODR        0x0c
> +#define MEN_Z127_IER1  0x10
> +#define MEN_Z127_IER2  0x14

It looks like it has interrupt support?

> +#define MEN_Z127_DBER  0x18

Debounce? In that case, maybe implement .set_debounce from day 1?

> +#define MEN_Z127_ODER  0x1C

And Open Drain? Maybe you should support that from day 1?

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
Andreas Werner Oct. 6, 2015, 5:54 p.m. UTC | #2 
On Tue, Oct 06, 2015 at 09:35:51AM +0200, Linus Walleij wrote:
> On Mon, Oct 5, 2015 at 8:09 PM, Andreas Werner <andy@wernerandy.de> wrote:
> 
> > The 16Z127 is a GPIO controller on a MCB FPGA and has 32
> > configurable GPIOs.
> > The GPIOs can be configured as inputs and outputs
> >
> > Signed-off-by: Andreas Werner <andy@wernerandy.de>
> 
> This driver looks like it can use the generic MMIO library.

Yes you are right, did not know about the gener MMIO lib but seems to be 
the right think for this driver. I will change that.

> 
> select GPIO_GENERIC
> #include <linux/basic_mmio_gpio.h>
> 
> Then look at example for how to do this, e.g.
> drivers/gpio/gpio-74xx-mmio.c
> 
> > +config GPIO_MENZ127
> > +       tristate "MEN 16Z127 GPIO support"
> > +       depends on MCB
> 
> Is this "MCB" symbol already upstream? It seems a bit short.
>

Yes MCB is already upstream and there is alreay a driver upstream named 
men_Z135_uart. MCB is called "MEN Chameleon Bus" which is out FPGA.
The FPGA implements a table named "Chameleon Table" which describes the IPs
in the FPGA. 
 
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/io.h>
> > +#include <linux/mcb.h>
> > +#include <linux/gpio.h>
> 
> Just
> #include <linux/gpio/driver.h>
> 
OK, will change that.

> > +#define MEN_Z127_CTRL  0x00
> > +#define MEN_Z127_PSR   0x04
> > +#define MEN_Z127_IRQR  0x08
> > +#define MEN_Z127_GPIODR        0x0c
> > +#define MEN_Z127_IER1  0x10
> > +#define MEN_Z127_IER2  0x14
> 
> It looks like it has interrupt support?
> 
Yes there is an Interrupt support but not completly implemented in HW.
My IC designer is currently in holiday and he is planned to do the IRQ work
in a few weeks (month).
The plan was to start without IRQ support and get the driver upstream.

> > +#define MEN_Z127_DBER  0x18
> 
> Debounce? In that case, maybe implement .set_debounce from day 1?

Yes did not want to implement this from day 1, but i think you are right
I should do that, its not a complicated thing :-)
> 
> > +#define MEN_Z127_ODER  0x1C
> 
> And Open Drain? Maybe you should support that from day 1?
> 
Need to check if this is current implementation is working, but should be no 
problem to implement this.

> Yours,
> Linus Walleij

Thanks for your comments. I will send a v2 today or tomorrow.

Regards
Andy
--
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
Andreas Werner Oct. 8, 2015, 4:55 p.m. UTC | #3 
Hi,
i have an additional question regarding the Open Drain setting.

The register is currently impelemented as a read/write register
which means the pin mode is configurable by software to
Push Pull or Open Drain.

There is also the possiblity (normal way) that the HW (FPGA) configures
each pin to the correct mode.

Is there actually a way to set an output mode from userland or by the gpio API?
I did not find anything about that.


If there is no way, i will implement it without software control. I just
read out the mode configuration and handle the pins as PP or Open Drain.

Regards
Andy
--
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
Andreas Werner Oct. 8, 2015, 7:49 p.m. UTC | #4 
Yet another thing.

if i will implement the open drain support, does it really makes sense to go with the
generic gpio lib?

I need to replace the direction_input and direction_outpu functions
because if the userland set an open drain pin to input i need to
set the pin to High-Z (driver "1") instead of setting the input bit
in the direction register.

So at the end there are just the "set" and "dat"
functions used to drive an output and to get an input state.
Alle other functions needs to be implemented in the driver.

Regards
Andy
--
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 Oct. 16, 2015, 7:51 p.m. UTC | #5 
On Thu, Oct 8, 2015 at 6:55 PM, Andreas Werner <andy@wernerandy.de> wrote:

> The register is currently impelemented as a read/write register
> which means the pin mode is configurable by software to
> Push Pull or Open Drain.

OK then you can set it in accordance to what clients request.

> There is also the possiblity (normal way) that the HW (FPGA) configures
> each pin to the correct mode.
>
> Is there actually a way to set an output mode from userland or by the gpio API?

As per <linux/gpio/machine.h> you can specify per-line if it should be
open drain using GPIO_OPEN_DRAIN, this will push through to the
gpio descriptor and those lines with this flag set will be handled specially.
We have merged device tree bindings for OD/OS but not code to
handle these, but I might have some code from Laurent Pinchart cooking
for the DT in-kernel backend too.

> I did not find anything about that.

git grep OPEN_DRAIN

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 Oct. 16, 2015, 7:54 p.m. UTC | #6 
On Thu, Oct 8, 2015 at 9:49 PM, Andreas Werner <andy@wernerandy.de> wrote:

> if i will implement the open drain support, does it really makes sense to go with the
> generic gpio lib?

Open Drain yes, for more esoterin pin config you may need to
also implement pin control, see Documentation/pinctrl.txt

> I need to replace the direction_input and direction_outpu functions
> because if the userland set an open drain pin to input i need to
> set the pin to High-Z (driver "1") instead of setting the input bit
> in the direction register.

Laurent has the same usecase I think, we need some fixes but
then we can detect (in the driver) that the gpio descriptor has
the open drain flag set in the .request() callback so you
can set it up there.

> So at the end there are just the "set" and "dat"
> functions used to drive an output and to get an input state.
> Alle other functions needs to be implemented in the driver.

I don't understand this question.

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
diff mbox

Patch

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 8949b3f..329ebc4 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -269,6 +269,12 @@  config GPIO_MB86S7X
 	help
 	  Say yes here to support the GPIO controller in Fujitsu MB86S70 SoCs.
 
+config GPIO_MENZ127
+	tristate "MEN 16Z127 GPIO support"
+	depends on MCB
+	help
+	 Say yes here to support the MEN 16Z127 GPIO Controller.
+
 config GPIO_MM_LANTIQ
 	bool "Lantiq Memory mapped GPIOs"
 	depends on LANTIQ && SOC_XWAY
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index f79a7c4..2c48751 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -58,6 +58,7 @@  obj-$(CONFIG_GPIO_MB86S7X)	+= gpio-mb86s7x.o
 obj-$(CONFIG_GPIO_MC33880)	+= gpio-mc33880.o
 obj-$(CONFIG_GPIO_MC9S08DZ60)	+= gpio-mc9s08dz60.o
 obj-$(CONFIG_GPIO_MCP23S08)	+= gpio-mcp23s08.o
+obj-$(CONFIG_GPIO_MENZ127)	+= gpio-menz127.o
 obj-$(CONFIG_GPIO_ML_IOH)	+= gpio-ml-ioh.o
 obj-$(CONFIG_GPIO_MM_LANTIQ)	+= gpio-mm-lantiq.o
 obj-$(CONFIG_GPIO_MOXART)	+= gpio-moxart.o
diff --git a/drivers/gpio/gpio-menz127.c b/drivers/gpio/gpio-menz127.c
new file mode 100644
index 0000000..599e61a
--- /dev/null
+++ b/drivers/gpio/gpio-menz127.c
@@ -0,0 +1,186 @@ 
+/*
+ * MEN 16Z127 GPIO driver
+ *
+ * Copyright (C) 2015 MEN Mikroelektronik GmbH (www.men.de)
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; version 2 of the License.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/mcb.h>
+#include <linux/gpio.h>
+#include <linux/err.h>
+
+#define MEN_Z127_CTRL	0x00
+#define MEN_Z127_PSR	0x04
+#define MEN_Z127_IRQR	0x08
+#define MEN_Z127_GPIODR	0x0c
+#define MEN_Z127_IER1	0x10
+#define MEN_Z127_IER2	0x14
+#define MEN_Z127_DBER	0x18
+#define MEN_Z127_ODER	0x1C
+
+struct men_z127_gpio {
+	struct gpio_chip chip;
+	void __iomem *reg_base;
+	struct mcb_device *mdev;
+	struct resource *mem;
+	spinlock_t lock;
+};
+#define to_men_z127_gpio(gc) container_of(gc, struct men_z127_gpio, chip)
+
+static void men_z127_mod_reg(struct gpio_chip *chip, u32 reg, unsigned gpio,
+			     int val)
+{
+	struct men_z127_gpio *men_z127_gpio;
+	u32 reg_val;
+
+	men_z127_gpio = to_men_z127_gpio(chip);
+
+	spin_lock(&men_z127_gpio->lock);
+	reg_val = readl(men_z127_gpio->reg_base + reg);
+
+	if (val)
+		reg_val |= BIT(gpio);
+	else
+		reg_val &= ~BIT(gpio);
+
+	writel(reg_val, men_z127_gpio->reg_base + reg);
+	spin_unlock(&men_z127_gpio->lock);
+}
+
+static void men_z127_set_reg(struct gpio_chip *chip, u32 reg, unsigned gpio)
+{
+	men_z127_mod_reg(chip, reg, gpio, 1);
+}
+
+static void men_z127_clr_reg(struct gpio_chip *chip, u32 reg, unsigned gpio)
+{
+	men_z127_mod_reg(chip, reg, gpio, 0);
+}
+
+static int men_z127_dir_output(struct gpio_chip *chip, unsigned gpio,
+			       int value)
+{
+	men_z127_set_reg(chip, MEN_Z127_GPIODR, gpio);
+	return 0;
+}
+
+static int men_z127_dir_input(struct gpio_chip *chip, unsigned gpio)
+{
+
+	men_z127_clr_reg(chip, MEN_Z127_GPIODR, gpio);
+	return 0;
+}
+
+static void men_z127_set(struct gpio_chip *chip, unsigned gpio, int value)
+{
+	if (value)
+		men_z127_set_reg(chip, MEN_Z127_CTRL, gpio);
+	else
+		men_z127_clr_reg(chip, MEN_Z127_CTRL, gpio);
+}
+
+static int men_z127_get(struct gpio_chip *chip, unsigned gpio)
+{
+	struct men_z127_gpio *men_z127_gpio;
+	u32 val;
+
+	men_z127_gpio = to_men_z127_gpio(chip);
+	val = readl(men_z127_gpio->reg_base + MEN_Z127_PSR);
+
+	return !!(val & BIT(gpio));
+}
+
+static struct gpio_chip men_z127_chip = {
+	.label = "men-z127-gpio",
+	.owner = THIS_MODULE,
+	.direction_input = men_z127_dir_input,
+	.direction_output = men_z127_dir_output,
+	.get = men_z127_get,
+	.set = men_z127_set,
+	.ngpio = 32,
+	.base = 0,
+};
+
+static int men_z127_probe(struct mcb_device *mdev,
+			  const struct mcb_device_id *id)
+{
+	struct men_z127_gpio *men_z127_gpio;
+	struct device *dev = &mdev->dev;
+	struct resource *gpio_mem;
+	int ret;
+
+	men_z127_gpio = devm_kzalloc(dev, sizeof(struct men_z127_gpio),
+				     GFP_KERNEL);
+	if (!men_z127_gpio)
+		return -ENOMEM;
+
+	gpio_mem = mcb_request_mem(mdev, dev_name(dev));
+	if (IS_ERR(gpio_mem)) {
+		dev_err(dev, "failed to request device memory");
+		return PTR_ERR(gpio_mem);
+	}
+
+	men_z127_gpio->reg_base = ioremap(gpio_mem->start,
+					  resource_size(gpio_mem));
+	if (men_z127_gpio->reg_base == NULL) {
+		ret = -ENXIO;
+		goto err_out;
+	}
+
+	spin_lock_init(&men_z127_gpio->lock);
+
+	men_z127_chip.dev = &mdev->dev;
+	men_z127_gpio->mem = gpio_mem;
+	men_z127_gpio->chip = men_z127_chip;
+	mcb_set_drvdata(mdev, men_z127_gpio);
+
+	ret = gpiochip_add(&men_z127_gpio->chip);
+	if (ret) {
+		dev_err(dev, "failed to register MEN 16Z127 GPIO controller");
+		goto err_out;
+	}
+
+	dev_info(dev, "MEN 16Z127 GPIO driver registered");
+
+	return 0;
+
+err_out:
+	mcb_release_mem(gpio_mem);
+	return ret;
+}
+
+static void men_z127_remove(struct mcb_device *mdev)
+{
+	struct men_z127_gpio *men_z127_gpio = mcb_get_drvdata(mdev);
+
+	iounmap(men_z127_gpio->reg_base);
+	mcb_release_mem(men_z127_gpio->mem);
+}
+
+static const struct mcb_device_id men_z127_ids[] = {
+	{ .device = 0x7f },
+	{ }
+};
+MODULE_DEVICE_TABLE(mcb, men_z127_ids);
+
+static struct mcb_driver men_z127_driver = {
+	.driver = {
+		.name = "z127-gpio",
+		.owner = THIS_MODULE,
+	},
+	.probe = men_z127_probe,
+	.remove = men_z127_remove,
+	.id_table = men_z127_ids,
+};
+module_mcb_driver(men_z127_driver);
+
+MODULE_AUTHOR("Andreas Werner <andreas.werner@men.de>");
+MODULE_DESCRIPTION("MEN 16z127 GPIO Controller");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("mcb:16z127");