[PATCH 1/3] gpio: sx150x: move to irqdomain framework for sx150x driver

[PATCH 1/3] gpio: sx150x: move to irqdomain framework for sx150x driver

[Barry Song]

From: Wei Chen <Wei.Chen@csr.com>

The sx150x gpio driver used a loop to set liner irq map for gpio pins.
Now we use the irq domain to rebuild this irq mappig and make sure the
codes are still compatible to old users.

this patch also adds IRQF_ONESHOT flag to fix the IRQ flooding issues.

Signed-off-by: Wei Chen <Wei.Chen@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/gpio/gpio-sx150x.c | 72 ++++++++++++----------------------------------
 1 file changed, 18 insertions(+), 54 deletions(-)

diff --git a/drivers/gpio/gpio-sx150x.c b/drivers/gpio/gpio-sx150x.c
index bce6c61..0bc61c2 100644
--- a/drivers/gpio/gpio-sx150x.c
+++ b/drivers/gpio/gpio-sx150x.c
@@ -293,27 +293,11 @@ static int sx150x_gpio_direction_output(struct gpio_chip *gc,
 	return status;
 }
 
-static int sx150x_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
-{
-	struct sx150x_chip *chip;
-
-	chip = container_of(gc, struct sx150x_chip, gpio_chip);
-
-	if (offset >= chip->dev_cfg->ngpios)
-		return -EINVAL;
-
-	if (chip->irq_base < 0)
-		return -EINVAL;
-
-	return chip->irq_base + offset;
-}
-
 static void sx150x_irq_mask(struct irq_data *d)
 {
 	struct sx150x_chip *chip = irq_data_get_irq_chip_data(d);
-	unsigned n;
+	unsigned n = d->hwirq;
 
-	n = d->irq - chip->irq_base;
 	chip->irq_masked |= (1 << n);
 	chip->irq_update = n;
 }
@@ -321,9 +305,8 @@ static void sx150x_irq_mask(struct irq_data *d)
 static void sx150x_irq_unmask(struct irq_data *d)
 {
 	struct sx150x_chip *chip = irq_data_get_irq_chip_data(d);
-	unsigned n;
+	unsigned n = d->hwirq;
 
-	n = d->irq - chip->irq_base;
 	chip->irq_masked &= ~(1 << n);
 	chip->irq_update = n;
 }
@@ -336,7 +319,7 @@ static int sx150x_irq_set_type(struct irq_data *d, unsigned int flow_type)
 	if (flow_type & (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW))
 		return -EINVAL;
 
-	n = d->irq - chip->irq_base;
+	n = d->hwirq;
 
 	if (flow_type & IRQ_TYPE_EDGE_RISING)
 		val |= 0x1;
@@ -371,7 +354,9 @@ static irqreturn_t sx150x_irq_thread_fn(int irq, void *dev_id)
 				val);
 		for (n = 0; n < 8; ++n) {
 			if (val & (1 << n)) {
-				sub_irq = chip->irq_base + (i * 8) + n;
+				sub_irq = irq_find_mapping(
+					chip->gpio_chip.irqdomain,
+					(i * 8) + n);
 				handle_nested_irq(sub_irq);
 				++nhandled;
 			}
@@ -428,12 +413,12 @@ static void sx150x_init_chip(struct sx150x_chip *chip,
 
 	chip->client                     = client;
 	chip->dev_cfg                    = &sx150x_devices[driver_data];
+	chip->gpio_chip.dev              = &client->dev;
 	chip->gpio_chip.label            = client->name;
 	chip->gpio_chip.direction_input  = sx150x_gpio_direction_input;
 	chip->gpio_chip.direction_output = sx150x_gpio_direction_output;
 	chip->gpio_chip.get              = sx150x_gpio_get;
 	chip->gpio_chip.set              = sx150x_gpio_set;
-	chip->gpio_chip.to_irq           = sx150x_gpio_to_irq;
 	chip->gpio_chip.base             = pdata->gpio_base;
 	chip->gpio_chip.can_sleep        = true;
 	chip->gpio_chip.ngpio            = chip->dev_cfg->ngpios;
@@ -529,31 +514,24 @@ static int sx150x_install_irq_chip(struct sx150x_chip *chip,
 				int irq_base)
 {
 	int err;
-	unsigned n;
-	unsigned irq;
 
 	chip->irq_summary = irq_summary;
 	chip->irq_base    = irq_base;
 
-	for (n = 0; n < chip->dev_cfg->ngpios; ++n) {
-		irq = irq_base + n;
-		irq_set_chip_data(irq, chip);
-		irq_set_chip_and_handler(irq, &chip->irq_chip, handle_edge_irq);
-		irq_set_nested_thread(irq, 1);
-#ifdef CONFIG_ARM
-		set_irq_flags(irq, IRQF_VALID);
-#else
-		irq_set_noprobe(irq);
-#endif
+	/* Add gpio chip to irq subsystem */
+	err = gpiochip_irqchip_add(&chip->gpio_chip,
+		&chip->irq_chip, chip->irq_base,
+		handle_edge_irq, IRQ_TYPE_EDGE_BOTH);
+	if (err) {
+		dev_err(&chip->client->dev,
+			"could not connect irqchip to gpiochip\n");
+		return  err;
 	}
 
 	err = devm_request_threaded_irq(&chip->client->dev,
-				irq_summary,
-				NULL,
-				sx150x_irq_thread_fn,
-				IRQF_SHARED | IRQF_TRIGGER_FALLING,
-				chip->irq_chip.name,
-				chip);
+			irq_summary, NULL, sx150x_irq_thread_fn,
+			IRQF_ONESHOT | IRQF_SHARED | IRQF_TRIGGER_FALLING,
+			chip->irq_chip.name, chip);
 	if (err < 0) {
 		chip->irq_summary = -1;
 		chip->irq_base    = -1;
@@ -562,17 +540,6 @@ static int sx150x_install_irq_chip(struct sx150x_chip *chip,
 	return err;
 }
 
-static void sx150x_remove_irq_chip(struct sx150x_chip *chip)
-{
-	unsigned n;
-	unsigned irq;
-
-	for (n = 0; n < chip->dev_cfg->ngpios; ++n) {
-		irq = chip->irq_base + n;
-		irq_set_chip_and_handler(irq, NULL, NULL);
-	}
-}
-
 static int sx150x_probe(struct i2c_client *client,
 				const struct i2c_device_id *id)
 {
@@ -626,9 +593,6 @@ static int sx150x_remove(struct i2c_client *client)
 	chip = i2c_get_clientdata(client);
 	gpiochip_remove(&chip->gpio_chip);
 
-	if (chip->irq_summary >= 0)
-		sx150x_remove_irq_chip(chip);
-
 	return 0;
 }
 
-- 
2.2.0

[PATCH 2/3] gpio: sx150x: add support for sx1506 gpio expander device

[Barry Song]

From: Wei Chen <Wei.Chen@csr.com>

semtech has two series of sx150x gpio expanders: sx150x-456 and
sx150x-789.

The current gpio-150x driver in linux only support sx1508 and
sx1509.

We added sx1506 support code into this driver.

Signed-off-by: Wei Chen <Wei.Chen@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/gpio/gpio-sx150x.c | 157 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 112 insertions(+), 45 deletions(-)

diff --git a/drivers/gpio/gpio-sx150x.c b/drivers/gpio/gpio-sx150x.c
index 0bc61c2..b32fb38 100644
--- a/drivers/gpio/gpio-sx150x.c
+++ b/drivers/gpio/gpio-sx150x.c
@@ -26,20 +26,43 @@
 
 #define NO_UPDATE_PENDING	-1
 
+/* The chip models of sx150x */
+#define SX150X_456 0
+#define SX150X_789 1
+
+struct sx150x_456_pri {
+	u8 reg_pld_mode;
+	u8 reg_pld_table0;
+	u8 reg_pld_table1;
+	u8 reg_pld_table2;
+	u8 reg_pld_table3;
+	u8 reg_pld_table4;
+	u8 reg_advance;
+};
+
+struct sx150x_789_pri {
+	u8 reg_drain;
+	u8 reg_polarity;
+	u8 reg_clock;
+	u8 reg_misc;
+	u8 reg_reset;
+	u8 ngpios;
+};
+
 struct sx150x_device_data {
+	u8 model;
 	u8 reg_pullup;
 	u8 reg_pulldn;
-	u8 reg_drain;
-	u8 reg_polarity;
 	u8 reg_dir;
 	u8 reg_data;
 	u8 reg_irq_mask;
 	u8 reg_irq_src;
 	u8 reg_sense;
-	u8 reg_clock;
-	u8 reg_misc;
-	u8 reg_reset;
 	u8 ngpios;
+	union {
+		struct sx150x_456_pri x456;
+		struct sx150x_789_pri x789;
+	} pri;
 };
 
 struct sx150x_chip {
@@ -59,40 +82,67 @@ struct sx150x_chip {
 
 static const struct sx150x_device_data sx150x_devices[] = {
 	[0] = { /* sx1508q */
-		.reg_pullup   = 0x03,
-		.reg_pulldn   = 0x04,
-		.reg_drain    = 0x05,
-		.reg_polarity = 0x06,
-		.reg_dir      = 0x07,
-		.reg_data     = 0x08,
-		.reg_irq_mask = 0x09,
-		.reg_irq_src  = 0x0c,
-		.reg_sense    = 0x0b,
-		.reg_clock    = 0x0f,
-		.reg_misc     = 0x10,
-		.reg_reset    = 0x7d,
-		.ngpios       = 8
+		.model = SX150X_789,
+		.reg_pullup	= 0x03,
+		.reg_pulldn	= 0x04,
+		.reg_dir	= 0x07,
+		.reg_data	= 0x08,
+		.reg_irq_mask	= 0x09,
+		.reg_irq_src	= 0x0c,
+		.reg_sense	= 0x0b,
+		.pri.x789 = {
+			.reg_drain	= 0x05,
+			.reg_polarity	= 0x06,
+			.reg_clock	= 0x0f,
+			.reg_misc	= 0x10,
+			.reg_reset	= 0x7d,
+		},
+		.ngpios = 8,
 	},
 	[1] = { /* sx1509q */
-		.reg_pullup   = 0x07,
-		.reg_pulldn   = 0x09,
-		.reg_drain    = 0x0b,
-		.reg_polarity = 0x0d,
-		.reg_dir      = 0x0f,
-		.reg_data     = 0x11,
-		.reg_irq_mask = 0x13,
-		.reg_irq_src  = 0x19,
-		.reg_sense    = 0x17,
-		.reg_clock    = 0x1e,
-		.reg_misc     = 0x1f,
-		.reg_reset    = 0x7d,
-		.ngpios       = 16
+		.model = SX150X_789,
+		.reg_pullup	= 0x07,
+		.reg_pulldn	= 0x09,
+		.reg_dir	= 0x0f,
+		.reg_data	= 0x11,
+		.reg_irq_mask	= 0x13,
+		.reg_irq_src	= 0x19,
+		.reg_sense	= 0x17,
+		.pri.x789 = {
+			.reg_drain	= 0x0b,
+			.reg_polarity	= 0x0d,
+			.reg_clock	= 0x1e,
+			.reg_misc	= 0x1f,
+			.reg_reset	= 0x7d,
+		},
+		.ngpios	= 16
+	},
+	[2] = { /* sx1506q */
+		.model = SX150X_456,
+		.reg_pullup	= 0x05,
+		.reg_pulldn	= 0x07,
+		.reg_dir	= 0x03,
+		.reg_data	= 0x01,
+		.reg_irq_mask	= 0x09,
+		.reg_irq_src	= 0x0f,
+		.reg_sense	= 0x0d,
+		.pri.x456 = {
+			.reg_pld_mode	= 0x21,
+			.reg_pld_table0	= 0x23,
+			.reg_pld_table1	= 0x25,
+			.reg_pld_table2	= 0x27,
+			.reg_pld_table3	= 0x29,
+			.reg_pld_table4	= 0x2b,
+			.reg_advance	= 0xad,
+		},
+		.ngpios	= 16
 	},
 };
 
 static const struct i2c_device_id sx150x_id[] = {
 	{"sx1508q", 0},
 	{"sx1509q", 1},
+	{"sx1506q", 2},
 	{}
 };
 MODULE_DEVICE_TABLE(i2c, sx150x_id);
@@ -191,7 +241,7 @@ static int sx150x_get_io(struct sx150x_chip *chip, unsigned offset)
 static void sx150x_set_oscio(struct sx150x_chip *chip, int val)
 {
 	sx150x_i2c_write(chip->client,
-			chip->dev_cfg->reg_clock,
+			chip->dev_cfg->pri.x789.reg_clock,
 			(val ? 0x1f : 0x10));
 }
 
@@ -455,13 +505,13 @@ static int sx150x_reset(struct sx150x_chip *chip)
 	int err;
 
 	err = i2c_smbus_write_byte_data(chip->client,
-					chip->dev_cfg->reg_reset,
+					chip->dev_cfg->pri.x789.reg_reset,
 					0x12);
 	if (err < 0)
 		return err;
 
 	err = i2c_smbus_write_byte_data(chip->client,
-					chip->dev_cfg->reg_reset,
+					chip->dev_cfg->pri.x789.reg_reset,
 					0x34);
 	return err;
 }
@@ -477,9 +527,14 @@ static int sx150x_init_hw(struct sx150x_chip *chip,
 			return err;
 	}
 
-	err = sx150x_i2c_write(chip->client,
-			chip->dev_cfg->reg_misc,
-			0x01);
+	if (chip->dev_cfg->model == SX150X_789)
+		err = sx150x_i2c_write(chip->client,
+				chip->dev_cfg->pri.x789.reg_misc,
+				0x01);
+	else
+		err = sx150x_i2c_write(chip->client,
+				chip->dev_cfg->pri.x456.reg_advance,
+				0x04);
 	if (err < 0)
 		return err;
 
@@ -493,15 +548,27 @@ static int sx150x_init_hw(struct sx150x_chip *chip,
 	if (err < 0)
 		return err;
 
-	err = sx150x_init_io(chip, chip->dev_cfg->reg_drain,
-			pdata->io_open_drain_ena);
-	if (err < 0)
-		return err;
+	if (chip->dev_cfg->model == SX150X_789) {
+		err = sx150x_init_io(chip,
+				chip->dev_cfg->pri.x789.reg_drain,
+				pdata->io_open_drain_ena);
+		if (err < 0)
+			return err;
+
+		err = sx150x_init_io(chip,
+				chip->dev_cfg->pri.x789.reg_polarity,
+				pdata->io_polarity);
+		if (err < 0)
+			return err;
+	} else {
+		/* Set all pins to work in normal mode */
+		err = sx150x_init_io(chip,
+				chip->dev_cfg->pri.x456.reg_pld_mode,
+				0);
+		if (err < 0)
+			return err;
+	}
 
-	err = sx150x_init_io(chip, chip->dev_cfg->reg_polarity,
-			pdata->io_polarity);
-	if (err < 0)
-		return err;
 
 	if (pdata->oscio_is_gpo)
 		sx150x_set_oscio(chip, 0);
-- 
2.2.0

[PATCH 2/3] gpio: sx150x: add support for sx1506 gpio expander device

[Linus Walleij]

On Thu, Dec 4, 2014 at 1:12 PM, Barry Song <21cnbao@gmail.com> wrote:

> From: Wei Chen <Wei.Chen@csr.com>
>
> semtech has two series of sx150x gpio expanders: sx150x-456 and
> sx150x-789.
>
> The current gpio-150x driver in linux only support sx1508 and
> sx1509.
>
> We added sx1506 support code into this driver.
>
> Signed-off-by: Wei Chen <Wei.Chen@csr.com>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>

Patch applied.

Yours,
Linus Walleij

[PATCH 3/3] gpio: sx150x: add dts support for sx150x driver

[Barry Song]

From: Wei Chen <Wei.Chen@csr.com>

Current sx150x gpio expander driver doesn't support DT. Now we added DT support
for this driver.

Signed-off-by: Wei Chen <Wei.Chen@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 .../devicetree/bindings/gpio/gpio-sx150x.txt       | 69 ++++++++++++++++++
 .../devicetree/bindings/vendor-prefixes.txt        |  1 +
 drivers/gpio/gpio-sx150x.c                         | 83 +++++++++++++++++++++-
 3 files changed, 151 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-sx150x.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-sx150x.txt b/Documentation/devicetree/bindings/gpio/gpio-sx150x.txt
new file mode 100644
index 0000000..7f8a61b
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-sx150x.txt
@@ -0,0 +1,69 @@
+SEMTECH SX150x GPIO expander bindings
+
+
+Required properties:
+
+- compatible: should be "semtech,sx1506q",
+			"semtech,sx1508q",
+			"semtech,sx1509q".
+
+- reg: The I2C slave address for this device.
+
+- interrupt-parent: phandle of the parent interrupt controller.
+
+- interrupts: Interrupt specifier for the controllers interrupt.
+
+- #gpio-cells: Should be 2. The first cell is the GPIO number and the
+		second cell is used to specify optional parameters:
+		bit 0: polarity (0: normal, 1: inverted)
+
+- gpio-controller: Marks the device as a GPIO controller.
+
+- interrupt-controller: Marks the device as a interrupt controller.
+
+Optional properties:
+- oscio_is_gpo: Boolean.
+	Whether the HW considers OSCIO as a GPO instead of as an
+	oscillator.
+
+- reset_during_probe: Boolean.
+	Whether the HW supports full reset of the chip at the beginning
+	of the probe.
+
+-pullup_ena:A bit-mask which enables or disables the pull-up resistor
+	for each IO line in the expander. = <0x0>;
+
+-pulldn_ena:A bit-mask which enables-or disables the pull-down resistor
+	for each IO line in the expander.
+
+-open_drain_ena:A bit-mask which enables-or disables open-drain
+	operation for each IO line in the expander.
+
+-polarity: A bit-mask which enables polarity inversion for each IO line
+	in the expander.
+
+The GPIO expander can optionally be used as an interrupt controller, in
+which case it uses the default two cell specifier as described in
+Documentation/devicetree/bindings/interrupt-controller/interrupts.txt.
+
+Example:
+
+	i2c_gpio_expander at 20{
+		#gpio-cells = <2>;
+		#interrupt-cells = <2>;
+		compatible = "semtech,sx1506q";
+		reg = <0x20>;
+		interrupt-parent = <&gpio_1>;
+		interrupts = <16 0>;
+
+		gpio-controller;
+		interrupt-controller;
+
+		oscio_is_gpo;
+		reset_during_probe;
+
+		pullup_ena = <0x01>;
+		pulldn_ena = <0x10>;
+		open_drain_ena = <0x40>;
+		polarity = <0x02>;
+	};
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index c177cd7..6968148 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -140,6 +140,7 @@ sandisk	Sandisk Corporation
 sbs	Smart Battery System
 schindler	Schindler
 seagate	Seagate Technology PLC
+semtech	Semtech Corporation
 sil	Silicon Image
 silabs	Silicon Laboratories
 simtek
diff --git a/drivers/gpio/gpio-sx150x.c b/drivers/gpio/gpio-sx150x.c
index b32fb38..28d2f03 100644
--- a/drivers/gpio/gpio-sx150x.c
+++ b/drivers/gpio/gpio-sx150x.c
@@ -23,6 +23,11 @@
 #include <linux/mutex.h>
 #include <linux/slab.h>
 #include <linux/i2c/sx150x.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_gpio.h>
+#include <linux/of_device.h>
 
 #define NO_UPDATE_PENDING	-1
 
@@ -147,6 +152,13 @@ static const struct i2c_device_id sx150x_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, sx150x_id);
 
+static const struct of_device_id sx150x_dt_id[] = {
+	{ .compatible = "semtech,sx1508q", .data = &sx150x_devices[0]},
+	{ .compatible = "semtech,sx1509q", .data = &sx150x_devices[1]},
+	{ .compatible = "semtech,sx1506q", .data = &sx150x_devices[2]},
+	{},
+};
+
 static s32 sx150x_i2c_write(struct i2c_client *client, u8 reg, u8 val)
 {
 	s32 err = i2c_smbus_write_byte_data(client, reg, val);
@@ -472,6 +484,8 @@ static void sx150x_init_chip(struct sx150x_chip *chip,
 	chip->gpio_chip.base             = pdata->gpio_base;
 	chip->gpio_chip.can_sleep        = true;
 	chip->gpio_chip.ngpio            = chip->dev_cfg->ngpios;
+	chip->gpio_chip.of_node          = client->dev.of_node;
+	chip->gpio_chip.of_gpio_n_cells  = 2;
 	if (pdata->oscio_is_gpo)
 		++chip->gpio_chip.ngpio;
 
@@ -607,6 +621,66 @@ static int sx150x_install_irq_chip(struct sx150x_chip *chip,
 	return err;
 }
 
+static struct sx150x_platform_data *of_sx150x_get_platdata(
+					struct i2c_client *client)
+{
+	int rc, gpio;
+	struct sx150x_platform_data *pdata;
+	struct device_node *np;
+
+	if (!client->dev.of_node)
+		return NULL;
+
+	np = client->dev.of_node;
+
+	pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	gpio = of_get_named_gpio(np, "int-gpios", 0);
+	if (gpio_is_valid(gpio)) {
+		rc = devm_gpio_request_one(&client->dev, gpio,
+			GPIOF_DIR_IN, "sx150x_interrupt");
+		if (rc)
+			return ERR_PTR(rc);
+	}
+
+	pdata->irq_summary = irq_of_parse_and_map(np, 0);
+	if (!pdata->irq_summary)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	pdata->oscio_is_gpo = of_property_read_bool(np, "oscio_is_gpo");
+	pdata->reset_during_probe =
+			of_property_read_bool(np, "reset_during_probe");
+
+	rc = of_property_read_u16(np, "pullup_ena",
+				&pdata->io_pullup_ena);
+	if (rc)
+		pdata->io_pullup_ena = 0;
+
+	rc = of_property_read_u16(np, "pulldn_ena",
+				&pdata->io_pulldn_ena);
+	if (rc)
+		pdata->io_pulldn_ena = 0;
+
+	rc = of_property_read_u16(np, "open_drain_ena",
+				&pdata->io_open_drain_ena);
+	if (rc)
+		pdata->io_open_drain_ena = 0;
+
+	rc = of_property_read_u16(np, "polarity",
+				&pdata->io_polarity);
+	if (rc)
+		pdata->io_polarity = 0;
+
+	/* Let OF gpiochip_add to detect dynamical gpio_base & irq_base */
+	pdata->gpio_base = -1;
+	/* We should use the dynamical irq_base */
+	pdata->irq_base = 0;
+
+	return pdata;
+}
+
 static int sx150x_probe(struct i2c_client *client,
 				const struct i2c_device_id *id)
 {
@@ -617,8 +691,13 @@ static int sx150x_probe(struct i2c_client *client,
 	int rc;
 
 	pdata = dev_get_platdata(&client->dev);
-	if (!pdata)
-		return -EINVAL;
+	if (!pdata) {
+		pdata = of_sx150x_get_platdata(client);
+		if (!pdata)
+			return -EINVAL;
+		else if (IS_ERR(pdata))
+			return PTR_ERR(pdata);
+	}
 
 	if (!i2c_check_functionality(client->adapter, i2c_funcs))
 		return -ENOSYS;
-- 
2.2.0

[PATCH 3/3] gpio: sx150x: add dts support for sx150x driver

[Linus Walleij]

On Thu, Dec 4, 2014 at 1:12 PM, Barry Song <21cnbao@gmail.com> wrote:

> From: Wei Chen <Wei.Chen@csr.com>
>
> Current sx150x gpio expander driver doesn't support DT. Now we added DT support
> for this driver.
>
> Signed-off-by: Wei Chen <Wei.Chen@csr.com>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
(...)
> +-pullup_ena:A bit-mask which enables or disables the pull-up resistor
> +       for each IO line in the expander. = <0x0>;
> +
> +-pulldn_ena:A bit-mask which enables-or disables the pull-down resistor
> +       for each IO line in the expander.
> +
> +-open_drain_ena:A bit-mask which enables-or disables open-drain
> +       operation for each IO line in the expander.
> +
> +-polarity: A bit-mask which enables polarity inversion for each IO line
> +       in the expander.

I don't particularly like these properties.

This is basically pin control stuff.

I know that the driver was merged way before the pin control subsystem
existed and before I was maintainer of GPIO. So it has some tradition
behind it.

Still I would *really* prefer that it was at least described the way
pin controllers usually are, so we have the option of converting it
to pin control later.

The major difference if you check the pin control standard bindings,
is that instead of hammering down the config of different lines in
the controller, the config is tied to the consumers, as these are
the devices actually requiring that config.

I.e. it's not a property of the *controller* that a line is pulled down,
that is a property of whatever is connected to that line.

Maybe I'm overzealous so that this would require implenenting
a whole lot of pin control stuff and move the driver to drivers/pinctrl,
but I won't let it go without a discussion first.

Yours,
Linus Walleij

[PATCH 3/3] gpio: sx150x: add dts support for sx150x driver

[Barry Song]

2015-01-09 18:02 GMT+08:00 Linus Walleij <linus.walleij@linaro.org>:
> On Thu, Dec 4, 2014 at 1:12 PM, Barry Song <21cnbao@gmail.com> wrote:
>
>> From: Wei Chen <Wei.Chen@csr.com>
>>
>> Current sx150x gpio expander driver doesn't support DT. Now we added DT support
>> for this driver.
>>
>> Signed-off-by: Wei Chen <Wei.Chen@csr.com>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> (...)
>> +-pullup_ena:A bit-mask which enables or disables the pull-up resistor
>> +       for each IO line in the expander. = <0x0>;
>> +
>> +-pulldn_ena:A bit-mask which enables-or disables the pull-down resistor
>> +       for each IO line in the expander.
>> +
>> +-open_drain_ena:A bit-mask which enables-or disables open-drain
>> +       operation for each IO line in the expander.
>> +
>> +-polarity: A bit-mask which enables polarity inversion for each IO line
>> +       in the expander.
>
> I don't particularly like these properties.
>
> This is basically pin control stuff.
>
> I know that the driver was merged way before the pin control subsystem
> existed and before I was maintainer of GPIO. So it has some tradition
> behind it.
>
> Still I would *really* prefer that it was at least described the way
> pin controllers usually are, so we have the option of converting it
> to pin control later.
>
> The major difference if you check the pin control standard bindings,
> is that instead of hammering down the config of different lines in
> the controller, the config is tied to the consumers, as these are
> the devices actually requiring that config.
>
> I.e. it's not a property of the *controller* that a line is pulled down,
> that is a property of whatever is connected to that line.
>
> Maybe I'm overzealous so that this would require implenenting
> a whole lot of pin control stuff and move the driver to drivers/pinctrl,
> but I won't let it go without a discussion first.

my suggestion is we split this patch into two, the first one adds the
basic gpio dt support, then we consider the pin configuration issue.

>
> Yours,
> Linus Walleij
-barry

[PATCH 3/3] gpio: sx150x: add dts support for sx150x driver

[Linus Walleij]

On Fri, Jan 9, 2015 at 7:02 PM, Barry Song <21cnbao@gmail.com> wrote:
> 2015-01-09 18:02 GMT+08:00 Linus Walleij <linus.walleij@linaro.org>:

>> Maybe I'm overzealous so that this would require implenenting
>> a whole lot of pin control stuff and move the driver to drivers/pinctrl,
>> but I won't let it go without a discussion first.
>
> my suggestion is we split this patch into two, the first one adds the
> basic gpio dt support, then we consider the pin configuration issue.

OK.

Yours,
Linus Walleij

[PATCH 1/3] gpio: sx150x: move to irqdomain framework for sx150x driver

[Linus Walleij]

On Thu, Dec 4, 2014 at 1:12 PM, Barry Song <21cnbao@gmail.com> wrote:

> From: Wei Chen <Wei.Chen@csr.com>
>
> The sx150x gpio driver used a loop to set liner irq map for gpio pins.
> Now we use the irq domain to rebuild this irq mappig and make sure the
> codes are still compatible to old users.
>
> this patch also adds IRQF_ONESHOT flag to fix the IRQ flooding issues.
>
> Signed-off-by: Wei Chen <Wei.Chen@csr.com>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>

Excellent, patch applied.

I added select GPIOLIB_IRQCHIP to the Kconfig though, to
make sure we have the proper library functions.

Yours,
Linus Walleij