* [PATCH 0/2] Technologic I2C-FPGA gpio support
@ 2016-08-05 14:41 Lucile Quirion
2016-08-05 14:41 ` [PATCH 1/2] gpio: add bindings for Technologic I2C-FPGA gpio controller Lucile Quirion
2016-08-05 14:41 ` [PATCH 2/2] gpio: add Technologic I2C-FPGA gpio support Lucile Quirion
0 siblings, 2 replies; 6+ messages in thread
From: Lucile Quirion @ 2016-08-05 14:41 UTC (permalink / raw)
To: linux-kernel, linux-gpio
Cc: Linus Walleij, Alexandre Courbot, kernel, Lucile Quirion
This driver aims to support all Technologic Systems's boards embedding FPGA
GPIOs with an I2C interface.
Lucile Quirion (2):
gpio: add bindings for Technologic I2C-FPGA gpio controller
gpio: add Technologic I2C-FPGA gpio support
.../devicetree/bindings/gpio/gpio-ts4900.txt | 29 +++
drivers/gpio/Kconfig | 6 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-ts4900.c | 238 +++++++++++++++++++++
4 files changed, 274 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-ts4900.txt
create mode 100644 drivers/gpio/gpio-ts4900.c
--
2.5.5
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] gpio: add bindings for Technologic I2C-FPGA gpio controller
2016-08-05 14:41 [PATCH 0/2] Technologic I2C-FPGA gpio support Lucile Quirion
@ 2016-08-05 14:41 ` Lucile Quirion
2016-08-11 13:14 ` Linus Walleij
2016-08-05 14:41 ` [PATCH 2/2] gpio: add Technologic I2C-FPGA gpio support Lucile Quirion
1 sibling, 1 reply; 6+ messages in thread
From: Lucile Quirion @ 2016-08-05 14:41 UTC (permalink / raw)
To: linux-kernel, linux-gpio
Cc: Linus Walleij, Alexandre Courbot, kernel, Lucile Quirion
Device tree binding documentation for Technologic's I2C-FPGA GPIO
controller.
Signed-off-by: Lucile Quirion <lucile.quirion@savoirfairelinux.com>
---
.../devicetree/bindings/gpio/gpio-ts4900.txt | 29 ++++++++++++++++++++++
1 file changed, 29 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-ts4900.txt
diff --git a/Documentation/devicetree/bindings/gpio/gpio-ts4900.txt b/Documentation/devicetree/bindings/gpio/gpio-ts4900.txt
new file mode 100644
index 0000000..e114f6f
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-ts4900.txt
@@ -0,0 +1,29 @@
+* Technologic Systems I2C-FPGA's GPIO controller bindings
+
+This bindings describes the GPIO controller for Technologic's FPGA core.
+TS-4900's FPGA encodes the GPIO state on 3 bits, whereas the TS-7970's FPGA
+uses 2 bits: it doesn't use a dedicated input bit.
+
+Required properties:
+- compatible: Should be one of the following
+ "technologic,ts4900-gpio"
+ "technologic,ts7970-gpio"
+- reg: Physical base address of the controller and length
+ of memory mapped region.
+- #gpio-cells: Should be two. The first cell is the pin number.
+- gpio-controller: Marks the device node as a gpio controller.
+
+Optional property:
+- ngpios: see "gpio.txt".
+
+Example:
+
+&i2c2 {
+ gpio8: ts4900-gpio@28 {
+ compatible = "technologic,ts4900-gpio";
+ reg = <0x28>;
+ #gpio-cells = <2>;
+ gpio-controller;
+ ngpios = <32>;
+ };
+};
--
2.5.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] gpio: add Technologic I2C-FPGA gpio support
2016-08-05 14:41 [PATCH 0/2] Technologic I2C-FPGA gpio support Lucile Quirion
2016-08-05 14:41 ` [PATCH 1/2] gpio: add bindings for Technologic I2C-FPGA gpio controller Lucile Quirion
@ 2016-08-05 14:41 ` Lucile Quirion
2016-08-05 22:45 ` Paul Gortmaker
2016-08-11 13:27 ` Linus Walleij
1 sibling, 2 replies; 6+ messages in thread
From: Lucile Quirion @ 2016-08-05 14:41 UTC (permalink / raw)
To: linux-kernel, linux-gpio
Cc: Linus Walleij, Alexandre Courbot, kernel, Lucile Quirion
This driver is generic and aims to support all Technologic Systems's
boards embedding FPGA GPIOs with an I2C interface.
This driver supports TS-4900, TS-7970, TS-7990 and TS-4100 series.
Signed-off-by: Lucile Quirion <lucile.quirion@savoirfairelinux.com>
---
drivers/gpio/Kconfig | 6 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-ts4900.c | 238 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 245 insertions(+)
create mode 100644 drivers/gpio/gpio-ts4900.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 98dd47a..459fb71 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -779,6 +779,12 @@ config GPIO_TPIC2810
To compile this driver as a module, choose M here: the module will
be called gpio-tpic2810.
+config GPIO_TS4900
+ bool "Technologic Systems FPGA I2C GPIO"
+ help
+ Say yes here to enabled the GPIO driver for Technologic's FPGA core.
+ Series supported include TS-4100, TS-4900, TS-7970 and TS-7990.
+
endmenu
menu "MFD GPIO expanders"
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 2a035ed..4dcc21f 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -110,6 +110,7 @@ obj-$(CONFIG_GPIO_TPS6586X) += gpio-tps6586x.o
obj-$(CONFIG_GPIO_TPS65910) += gpio-tps65910.o
obj-$(CONFIG_GPIO_TPS65912) += gpio-tps65912.o
obj-$(CONFIG_GPIO_TS4800) += gpio-ts4800.o
+obj-$(CONFIG_GPIO_TS4900) += gpio-ts4900.o
obj-$(CONFIG_GPIO_TS5500) += gpio-ts5500.o
obj-$(CONFIG_GPIO_TWL4030) += gpio-twl4030.o
obj-$(CONFIG_GPIO_TWL6040) += gpio-twl6040.o
diff --git a/drivers/gpio/gpio-ts4900.c b/drivers/gpio/gpio-ts4900.c
new file mode 100644
index 0000000..104d1e0
--- /dev/null
+++ b/drivers/gpio/gpio-ts4900.c
@@ -0,0 +1,238 @@
+/*
+ * Digital I/O driver for Technologic Systems I2C FPGA Core
+ *
+ * Copyright (C) 2015 Technologic Systems
+ * Copyright (C) 2016 Savoir-Faire Linux
+ *
+ * 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.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether expressed or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License version 2 for more details.
+ */
+
+#include <linux/gpio/driver.h>
+#include <linux/i2c.h>
+#include <linux/of_device.h>
+#include <linux/module.h>
+
+#define DEFAULT_PIN_NUMBER 32
+/*
+ * Register bits used by the GPIO device
+ * Some boards, such as TS-7970 do not have a separate input bit
+ */
+#define TS4900_GPIO_OE 0x01
+#define TS4900_GPIO_OD 0x02
+#define TS4900_GPIO_ID 0x04
+#define TS7970_GPIO_ID 0x02
+
+struct ts4900_gpio_priv {
+ struct i2c_client *client;
+ struct gpio_chip gpio_chip;
+ unsigned int input_bit;
+};
+
+static int ts4900_gpio_write(struct i2c_client *client, u16 addr, u8 data)
+{
+ struct i2c_msg msg;
+ u8 buf[3];
+ int ret;
+
+ buf[0] = addr >> 8;
+ buf[1] = addr & 0xFF;
+ buf[2] = data;
+
+ msg.addr = client->addr;
+ msg.flags = 0;
+ msg.len = 3;
+ msg.buf = buf;
+
+ ret = i2c_transfer(client->adapter, &msg, 1);
+ if (ret != 1) {
+ dev_err(&client->dev, "%s: write error, ret=%d\n",
+ __func__, ret);
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int ts4900_gpio_read(struct i2c_client *client, u16 addr)
+{
+ struct i2c_msg msgs[2];
+ u8 buf[2];
+ int ret;
+
+ buf[0] = addr >> 8;
+ buf[1] = addr & 0xFF;
+
+ msgs[0].addr = client->addr;
+ msgs[0].flags = 0;
+ msgs[0].len = 2;
+ msgs[0].buf = buf;
+
+ msgs[1].addr = client->addr;
+ msgs[1].flags = I2C_M_RD;
+ msgs[1].len = 1;
+ msgs[1].buf = buf;
+
+ ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+ if (ret != ARRAY_SIZE(msgs)) {
+ dev_err(&client->dev, "%s: read error, ret=%d\n",
+ __func__, ret);
+ return -EIO;
+ }
+
+ return buf[0];
+}
+
+static int __ts4900_gpio_direction_output(struct i2c_client *client, int gpio,
+ int value)
+{
+ u8 reg = 0;
+
+ if (value)
+ reg = TS4900_GPIO_OD | TS4900_GPIO_OE;
+ else
+ reg = TS4900_GPIO_OE;
+
+ return ts4900_gpio_write(client, gpio, reg);
+}
+
+static int ts4900_gpio_direction_input(struct gpio_chip *chip,
+ unsigned int offset)
+{
+ struct ts4900_gpio_priv *priv = gpiochip_get_data(chip);
+
+ /*
+ * This will clear the output enable bit, the other bits are
+ * dontcare when this is cleared
+ */
+ return ts4900_gpio_write(priv->client, offset, 0);
+}
+
+static int ts4900_gpio_direction_output(struct gpio_chip *chip,
+ unsigned int offset, int value)
+{
+ struct ts4900_gpio_priv *priv = gpiochip_get_data(chip);
+
+ return __ts4900_gpio_direction_output(priv->client, offset, value);
+}
+
+static int ts4900_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+ struct ts4900_gpio_priv *priv = gpiochip_get_data(chip);
+ int ret;
+ u8 reg;
+
+ reg = ts4900_gpio_read(priv->client, offset);
+
+ ret = (reg & priv->input_bit) ? 1 : 0;
+
+ return ret;
+}
+
+static void ts4900_gpio_set(struct gpio_chip *chip, unsigned int offset,
+ int value)
+{
+ struct ts4900_gpio_priv *priv = gpiochip_get_data(chip);
+
+ __ts4900_gpio_direction_output(priv->client, offset, value);
+}
+
+static struct gpio_chip template_chip = {
+ .label = "ts4900-gpio",
+ .owner = THIS_MODULE,
+ .direction_input = ts4900_gpio_direction_input,
+ .direction_output = ts4900_gpio_direction_output,
+ .get = ts4900_gpio_get,
+ .set = ts4900_gpio_set,
+ .base = -1,
+ .can_sleep = true,
+};
+
+static const struct of_device_id ts4900_gpio_of_match_table[] = {
+ {
+ .compatible = "technologic,ts4900-gpio",
+ .data = (void *)TS4900_GPIO_ID,
+ }, {
+ .compatible = "technologic,ts7970-gpio",
+ .data = (void *)TS7970_GPIO_ID,
+ },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, ts4900_gpio_of_match_table);
+
+static int ts4900_gpio_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ const struct of_device_id *match;
+ struct ts4900_gpio_priv *priv;
+ u32 ngpio;
+ int ret;
+
+ if (!i2c_check_functionality(client->adapter,
+ I2C_FUNC_SMBUS_BYTE_DATA))
+ return -EIO;
+
+ match = of_match_device(ts4900_gpio_of_match_table, &client->dev);
+ if (!match)
+ return -EINVAL;
+
+ if (of_property_read_u32(client->dev.of_node, "ngpios", &ngpio))
+ ngpio = DEFAULT_PIN_NUMBER;
+
+ priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ i2c_set_clientdata(client, priv);
+
+ priv->client = client;
+ priv->gpio_chip = template_chip;
+ priv->gpio_chip.label = "ts4900-gpio";
+ priv->gpio_chip.ngpio = ngpio;
+ priv->gpio_chip.parent = &client->dev;
+ priv->input_bit = (uintptr_t)match->data;
+
+ ret = gpiochip_add_data(&priv->gpio_chip, priv);
+ if (ret < 0) {
+ dev_err(&client->dev, "Unable to register gpiochip\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ts4900_gpio_remove(struct i2c_client *client)
+{
+ struct ts4900_gpio_priv *priv = i2c_get_clientdata(client);
+
+ gpiochip_remove(&priv->gpio_chip);
+
+ return 0;
+}
+
+static const struct i2c_device_id ts4900_gpio_id_table[] = {
+ { "ts4900-gpio", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(i2c, ts4900_gpio_id_table);
+
+static struct i2c_driver ts4900_gpio_driver = {
+ .driver = {
+ .name = "ts4900-gpio",
+ .of_match_table = ts4900_gpio_of_match_table,
+ },
+ .probe = ts4900_gpio_probe,
+ .remove = ts4900_gpio_remove,
+ .id_table = ts4900_gpio_id_table,
+};
+module_i2c_driver(ts4900_gpio_driver);
+
+MODULE_AUTHOR("Technologic Systems");
+MODULE_DESCRIPTION("GPIO interface for Technologic Systems I2C-FPGA core");
+MODULE_LICENSE("GPL");
--
2.5.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] gpio: add Technologic I2C-FPGA gpio support
2016-08-05 14:41 ` [PATCH 2/2] gpio: add Technologic I2C-FPGA gpio support Lucile Quirion
@ 2016-08-05 22:45 ` Paul Gortmaker
2016-08-11 13:27 ` Linus Walleij
1 sibling, 0 replies; 6+ messages in thread
From: Paul Gortmaker @ 2016-08-05 22:45 UTC (permalink / raw)
To: Lucile Quirion; +Cc: LKML, linux-gpio, Linus Walleij, Alexandre Courbot, kernel
On Fri, Aug 5, 2016 at 10:41 AM, Lucile Quirion
<lucile.quirion@savoirfairelinux.com> wrote:
> This driver is generic and aims to support all Technologic Systems's
> boards embedding FPGA GPIOs with an I2C interface.
>
> This driver supports TS-4900, TS-7970, TS-7990 and TS-4100 series.
>
> Signed-off-by: Lucile Quirion <lucile.quirion@savoirfairelinux.com>
> ---
> drivers/gpio/Kconfig | 6 ++
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-ts4900.c | 238 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 245 insertions(+)
> create mode 100644 drivers/gpio/gpio-ts4900.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 98dd47a..459fb71 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -779,6 +779,12 @@ config GPIO_TPIC2810
> To compile this driver as a module, choose M here: the module will
> be called gpio-tpic2810.
>
> +config GPIO_TS4900
> + bool "Technologic Systems FPGA I2C GPIO"
Please don't use module.h and MODULE_<xyz> macros in drivers that are bool.
Either delete all the module related stuff and use a builtin registration fcn,
or use a tristate Kconfig if there is a genuine use case for a modular driver.
Thanks,
Paul.
--
> + help
> + Say yes here to enabled the GPIO driver for Technologic's FPGA core.
> + Series supported include TS-4100, TS-4900, TS-7970 and TS-7990.
> +
> endmenu
>
> menu "MFD GPIO expanders"
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 2a035ed..4dcc21f 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -110,6 +110,7 @@ obj-$(CONFIG_GPIO_TPS6586X) += gpio-tps6586x.o
> obj-$(CONFIG_GPIO_TPS65910) += gpio-tps65910.o
> obj-$(CONFIG_GPIO_TPS65912) += gpio-tps65912.o
> obj-$(CONFIG_GPIO_TS4800) += gpio-ts4800.o
> +obj-$(CONFIG_GPIO_TS4900) += gpio-ts4900.o
> obj-$(CONFIG_GPIO_TS5500) += gpio-ts5500.o
> obj-$(CONFIG_GPIO_TWL4030) += gpio-twl4030.o
> obj-$(CONFIG_GPIO_TWL6040) += gpio-twl6040.o
> diff --git a/drivers/gpio/gpio-ts4900.c b/drivers/gpio/gpio-ts4900.c
> new file mode 100644
> index 0000000..104d1e0
> --- /dev/null
> +++ b/drivers/gpio/gpio-ts4900.c
> @@ -0,0 +1,238 @@
> +/*
> + * Digital I/O driver for Technologic Systems I2C FPGA Core
> + *
> + * Copyright (C) 2015 Technologic Systems
> + * Copyright (C) 2016 Savoir-Faire Linux
> + *
> + * 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.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether expressed or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License version 2 for more details.
> + */
> +
> +#include <linux/gpio/driver.h>
> +#include <linux/i2c.h>
> +#include <linux/of_device.h>
> +#include <linux/module.h>
> +
[...]
> +static const struct i2c_device_id ts4900_gpio_id_table[] = {
> + { "ts4900-gpio", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(i2c, ts4900_gpio_id_table);
> +
> +static struct i2c_driver ts4900_gpio_driver = {
> + .driver = {
> + .name = "ts4900-gpio",
> + .of_match_table = ts4900_gpio_of_match_table,
> + },
> + .probe = ts4900_gpio_probe,
> + .remove = ts4900_gpio_remove,
> + .id_table = ts4900_gpio_id_table,
> +};
> +module_i2c_driver(ts4900_gpio_driver);
> +
> +MODULE_AUTHOR("Technologic Systems");
> +MODULE_DESCRIPTION("GPIO interface for Technologic Systems I2C-FPGA core");
> +MODULE_LICENSE("GPL");
> --
> 2.5.5
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] gpio: add bindings for Technologic I2C-FPGA gpio controller
2016-08-05 14:41 ` [PATCH 1/2] gpio: add bindings for Technologic I2C-FPGA gpio controller Lucile Quirion
@ 2016-08-11 13:14 ` Linus Walleij
0 siblings, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2016-08-11 13:14 UTC (permalink / raw)
To: Lucile Quirion; +Cc: linux-kernel, linux-gpio, Alexandre Courbot, kernel
On Fri, Aug 5, 2016 at 4:41 PM, Lucile Quirion
<lucile.quirion@savoirfairelinux.com> wrote:
> Device tree binding documentation for Technologic's I2C-FPGA GPIO
> controller.
>
> Signed-off-by: Lucile Quirion <lucile.quirion@savoirfairelinux.com>
(...)
> +Optional property:
> +- ngpios: see "gpio.txt".
gpio.txt gives examples of what this is used for. Detail why it is used
for this specific hardware.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] gpio: add Technologic I2C-FPGA gpio support
2016-08-05 14:41 ` [PATCH 2/2] gpio: add Technologic I2C-FPGA gpio support Lucile Quirion
2016-08-05 22:45 ` Paul Gortmaker
@ 2016-08-11 13:27 ` Linus Walleij
1 sibling, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2016-08-11 13:27 UTC (permalink / raw)
To: Lucile Quirion; +Cc: linux-kernel, linux-gpio, Alexandre Courbot, kernel
On Fri, Aug 5, 2016 at 4:41 PM, Lucile Quirion
<lucile.quirion@savoirfairelinux.com> wrote:
> This driver is generic and aims to support all Technologic Systems's
> boards embedding FPGA GPIOs with an I2C interface.
>
> This driver supports TS-4900, TS-7970, TS-7990 and TS-4100 series.
>
> Signed-off-by: Lucile Quirion <lucile.quirion@savoirfairelinux.com>
(...)
> +static int ts4900_gpio_write(struct i2c_client *client, u16 addr, u8 data)
> +{
> + struct i2c_msg msg;
> + u8 buf[3];
> + int ret;
> +
> + buf[0] = addr >> 8;
> + buf[1] = addr & 0xFF;
> + buf[2] = data;
> +
> + msg.addr = client->addr;
> + msg.flags = 0;
> + msg.len = 3;
> + msg.buf = buf;
> +
> + ret = i2c_transfer(client->adapter, &msg, 1);
> + if (ret != 1) {
> + dev_err(&client->dev, "%s: write error, ret=%d\n",
> + __func__, ret);
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static int ts4900_gpio_read(struct i2c_client *client, u16 addr)
> +{
> + struct i2c_msg msgs[2];
> + u8 buf[2];
> + int ret;
> +
> + buf[0] = addr >> 8;
> + buf[1] = addr & 0xFF;
> +
> + msgs[0].addr = client->addr;
> + msgs[0].flags = 0;
> + msgs[0].len = 2;
> + msgs[0].buf = buf;
> +
> + msgs[1].addr = client->addr;
> + msgs[1].flags = I2C_M_RD;
> + msgs[1].len = 1;
> + msgs[1].buf = buf;
> +
> + ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> + if (ret != ARRAY_SIZE(msgs)) {
> + dev_err(&client->dev, "%s: read error, ret=%d\n",
> + __func__, ret);
> + return -EIO;
> + }
> +
> + return buf[0];
> +}
It appears that you can very clearly replace this stuff with an
I2C regmap. If you look in drivers/base/regmap/regmap-i2c.c
you can see that you're just reimplementing regmaps marshalling.
Look at other drivers using I2C regmap for inspiration, e.g.
drivers/mfd/stw481x.c.
select REGMAP_I2C #include <linux/regmap.h> and start by implementing
devm_regmap_init_i2c() etc.
> +static int __ts4900_gpio_direction_output(struct i2c_client *client, int gpio,
> + int value)
I don't like __prefixed functions. Come up with a better name.
> +{
> + u8 reg = 0;
> +
> + if (value)
> + reg = TS4900_GPIO_OD | TS4900_GPIO_OE;
> + else
> + reg = TS4900_GPIO_OE;
> +
> + return ts4900_gpio_write(client, gpio, reg);
> +}
TS4900_GPIO_OD sounds like you can switch the output
between push-pull and open drain and here you are just open
coding all outputs to be open drain. That is confusing.
For controlling open drain implement
.set_single_ended() in the GPIO chip and let consumers
explicitly request that they want their line open drain.
If "OD" does not mean open drain it is extremely confusing
but OK then I am wrong...
> +static int ts4900_gpio_direction_input(struct gpio_chip *chip,
> + unsigned int offset)
> +static int ts4900_gpio_direction_output(struct gpio_chip *chip,
> + unsigned int offset, int value)
Also implement ts4900_gpio_get_direction()
> +static int ts4900_gpio_get(struct gpio_chip *chip, unsigned int offset)
> +{
> + struct ts4900_gpio_priv *priv = gpiochip_get_data(chip);
> + int ret;
> + u8 reg;
> +
> + reg = ts4900_gpio_read(priv->client, offset);
> +
> + ret = (reg & priv->input_bit) ? 1 : 0;
> +
> + return ret;
Just replace the two last lines with:
return !!(reg & priv->input_bit);
> +static void ts4900_gpio_set(struct gpio_chip *chip, unsigned int offset,
> + int value)
> +{
> + struct ts4900_gpio_priv *priv = gpiochip_get_data(chip);
> +
> + __ts4900_gpio_direction_output(priv->client, offset, value);
> +}
Why should you always set the direction to output when setting
an output value? It doesn't make sense. Make the functions
do one thing. .direction_output() sets an output value at the
same time, but .set() should not force the line as output.
(...)
> + ret = gpiochip_add_data(&priv->gpio_chip, priv);
Use devm_gpiochip_add_data()
> +static int ts4900_gpio_remove(struct i2c_client *client)
> +{
> + struct ts4900_gpio_priv *priv = i2c_get_clientdata(client);
> +
> + gpiochip_remove(&priv->gpio_chip);
> +
> + return 0;
> +}
Then this is not needed at all (handled by devm*)
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-08-11 13:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-05 14:41 [PATCH 0/2] Technologic I2C-FPGA gpio support Lucile Quirion
2016-08-05 14:41 ` [PATCH 1/2] gpio: add bindings for Technologic I2C-FPGA gpio controller Lucile Quirion
2016-08-11 13:14 ` Linus Walleij
2016-08-05 14:41 ` [PATCH 2/2] gpio: add Technologic I2C-FPGA gpio support Lucile Quirion
2016-08-05 22:45 ` Paul Gortmaker
2016-08-11 13:27 ` Linus Walleij
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.