[PATCH V2 0/6] gpio: davinci: Redesign driver to accommodate ngpios in one gpio chip

[PATCH V2 0/6] gpio: davinci: Redesign driver to accommodate ngpios in one gpio chip

[Keerthy]

The Davinci GPIO driver is implemented to work with one monolithic
Davinci GPIO platform device which may have up to Y(144) gpios.
The Davinci GPIO driver instantiates number of GPIO chips with
max 32 gpio pins per each during initialization and one IRQ domain.
So, the current GPIO's  opjects structure is:

<platform device> Davinci GPIO controller
 |- <gpio0_chip0> ------|
 ...                    |--- irq_domain (hwirq [0..143])
 |- <gpio0_chipN> ------|

Current driver creates one chip for every 32 GPIOs in a controller.
This was a limitation earlier now there is no need for that. Hence
redesigning the driver to create one gpio chip for all the ngpio
in the controller.

|- <gpio0_chip0> ------|--- irq_domain (hwirq [0..143]).

The previous discussion on this can be found here:
https://www.spinics.net/lists/linux-omap/msg132869.html

The series is posted on top of: 

https://lkml.org/lkml/2017/1/4/94

Changes in v2:

  * Optimized the re-design patch.
  * Added couple of code clean ups after the re-design.
  * Included v2 of https://patchwork.ozlabs.org/patch/710855/ 

Keerthy (6):
  gpio: davinci: Remove gpio2regs function to accommodate multi
    instances
  gpio: davinci: Remove unwanted blank line
  gpio: davinci: Redesign driver to accommodate ngpios in one gpio chip
  gpio: davinci: Add support for multiple GPIO controllers
  gpio: davinci: Remove custom .xlate
  gpio: davinci: Remove redundant macros

 drivers/gpio/gpio-davinci.c                | 174 +++++++++++++----------------
 include/linux/platform_data/gpio-davinci.h |  20 ++--
 2 files changed, 90 insertions(+), 104 deletions(-)

-- 
1.9.1

[PATCH V2 1/6] gpio: davinci: Remove gpio2regs function to accommodate multi instances

[Keerthy]

gpio2regs is written making an assumption that driver supports only
one instance of gpio controller. Removing this and adding a generic
array so as to support multiple instances of gpio controllers.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---

Changes in v2:

  * Added a comment to explain divide by 2 logic for register sets.

 drivers/gpio/gpio-davinci.c | 35 +++++++++++------------------------
 1 file changed, 11 insertions(+), 24 deletions(-)

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 163f81e..bb47de3 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -43,25 +43,7 @@ struct davinci_gpio_regs {
 #define MAX_LABEL_SIZE 20
 
 static void __iomem *gpio_base;
-
-static struct davinci_gpio_regs __iomem *gpio2regs(unsigned gpio)
-{
-	void __iomem *ptr;
-
-	if (gpio < 32 * 1)
-		ptr = gpio_base + 0x10;
-	else if (gpio < 32 * 2)
-		ptr = gpio_base + 0x38;
-	else if (gpio < 32 * 3)
-		ptr = gpio_base + 0x60;
-	else if (gpio < 32 * 4)
-		ptr = gpio_base + 0x88;
-	else if (gpio < 32 * 5)
-		ptr = gpio_base + 0xb0;
-	else
-		ptr = NULL;
-	return ptr;
-}
+static unsigned int offset_array[5] = {0x10, 0x38, 0x60, 0x88, 0xb0};
 
 static inline struct davinci_gpio_regs __iomem *irq2regs(struct irq_data *d)
 {
@@ -262,7 +244,7 @@ static int davinci_gpio_probe(struct platform_device *pdev)
 #endif
 		spin_lock_init(&chips[i].lock);
 
-		regs = gpio2regs(base);
+		regs = gpio_base + offset_array[i];
 		if (!regs)
 			return -ENXIO;
 		chips[i].regs = regs;
@@ -417,7 +399,9 @@ static int gpio_irq_type_unbanked(struct irq_data *data, unsigned trigger)
 davinci_gpio_irq_map(struct irq_domain *d, unsigned int irq,
 		     irq_hw_number_t hw)
 {
-	struct davinci_gpio_regs __iomem *g = gpio2regs(hw);
+	struct davinci_gpio_controller *chips =
+				(struct davinci_gpio_controller *)d->host_data;
+	struct davinci_gpio_regs __iomem *g = chips[hw / 32].regs;
 
 	irq_set_chip_and_handler_name(irq, &gpio_irqchip, handle_simple_irq,
 				"davinci_gpio");
@@ -554,7 +538,7 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 		irq_chip->irq_set_type = gpio_irq_type_unbanked;
 
 		/* default trigger: both edges */
-		g = gpio2regs(0);
+		g = chips[0].regs;
 		writel_relaxed(~0, &g->set_falling);
 		writel_relaxed(~0, &g->set_rising);
 
@@ -573,8 +557,11 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 	 * then chain through our own handler.
 	 */
 	for (gpio = 0, bank = 0; gpio < ngpio; bank++, bank_irq++, gpio += 16) {
-		/* disabled by default, enabled only as needed */
-		g = gpio2regs(gpio);
+		/* disabled by default, enabled only as needed
+		 * There are register sets for 32 GPIOs. 2 banks of 16
+		 * GPIOs are covered by each set of registers hence divide by 2
+		 */
+		g = chips[bank / 2].regs;
 		writel_relaxed(~0, &g->clr_falling);
 		writel_relaxed(~0, &g->clr_rising);
 
-- 
1.9.1

[PATCH V2 2/6] gpio: davinci: Remove unwanted blank line

[Keerthy]

Remove redundant blank line.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 include/linux/platform_data/gpio-davinci.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/linux/platform_data/gpio-davinci.h b/include/linux/platform_data/gpio-davinci.h
index 44ca530..18127c4 100644
--- a/include/linux/platform_data/gpio-davinci.h
+++ b/include/linux/platform_data/gpio-davinci.h
@@ -26,7 +26,6 @@ struct davinci_gpio_platform_data {
 	u32	gpio_unbanked;
 };
 
-
 struct davinci_gpio_controller {
 	struct gpio_chip	chip;
 	struct irq_domain	*irq_domain;
-- 
1.9.1

[PATCH V2 3/6] gpio: davinci: Redesign driver to accommodate ngpios in one gpio chip

[Keerthy]

The Davinci GPIO driver is implemented to work with one monolithic
Davinci GPIO platform device which may have up to Y(144) gpios.
The Davinci GPIO driver instantiates number of GPIO chips with
max 32 gpio pins per each during initialization and one IRQ domain.
So, the current GPIO's  opjects structure is:

<platform device> Davinci GPIO controller
 |- <gpio0_chip0> ------|
 ...                    |--- irq_domain (hwirq [0..143])
 |- <gpio0_chipN> ------|

Current driver creates one chip for every 32 GPIOs in a controller.
This was a limitation earlier now there is no need for that. Hence
redesigning the driver to create one gpio chip for all the ngpio
in the controller.

|- <gpio0_chip0> ------|--- irq_domain (hwirq [0..143]).

The previous discussion on this can be found here:
https://www.spinics.net/lists/linux-omap/msg132869.html

Signed-off-by: Keerthy <j-keerthy@ti.com>
---

Changes in v2:

  * Optimized irqdata allocation logic to use a single pointer
    instead of array of pointers.
  * Removed a redundant Macro.
  * Used __gpio_mask instead of hardcoding every single time.
  * MAX_BANKS changed to MAX_REGS_BANKS

 drivers/gpio/gpio-davinci.c                | 127 +++++++++++++++++------------
 include/linux/platform_data/gpio-davinci.h |  12 ++-
 2 files changed, 83 insertions(+), 56 deletions(-)

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index bb47de3..a527e88 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -63,11 +63,13 @@ static inline int __davinci_direction(struct gpio_chip *chip,
 			unsigned offset, bool out, int value)
 {
 	struct davinci_gpio_controller *d = gpiochip_get_data(chip);
-	struct davinci_gpio_regs __iomem *g = d->regs;
+	struct davinci_gpio_regs __iomem *g;
 	unsigned long flags;
 	u32 temp;
-	u32 mask = 1 << offset;
+	int bank = offset / 32;
+	u32 mask = __gpio_mask(offset);
 
+	g = d->regs[bank];
 	spin_lock_irqsave(&d->lock, flags);
 	temp = readl_relaxed(&g->dir);
 	if (out) {
@@ -103,9 +105,12 @@ static int davinci_direction_in(struct gpio_chip *chip, unsigned offset)
 static int davinci_gpio_get(struct gpio_chip *chip, unsigned offset)
 {
 	struct davinci_gpio_controller *d = gpiochip_get_data(chip);
-	struct davinci_gpio_regs __iomem *g = d->regs;
+	struct davinci_gpio_regs __iomem *g;
+	int bank = offset / 32;
 
-	return !!((1 << offset) & readl_relaxed(&g->in_data));
+	g = d->regs[bank];
+
+	return !!(__gpio_mask(offset) & readl_relaxed(&g->in_data));
 }
 
 /*
@@ -115,9 +120,13 @@ static int davinci_gpio_get(struct gpio_chip *chip, unsigned offset)
 davinci_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 {
 	struct davinci_gpio_controller *d = gpiochip_get_data(chip);
-	struct davinci_gpio_regs __iomem *g = d->regs;
+	struct davinci_gpio_regs __iomem *g;
+	int bank = offset / 32;
 
-	writel_relaxed((1 << offset), value ? &g->set_data : &g->clr_data);
+	g = d->regs[bank];
+
+	writel_relaxed(__gpio_mask(offset),
+		       value ? &g->set_data : &g->clr_data);
 }
 
 static struct davinci_gpio_platform_data *
@@ -165,7 +174,7 @@ static int davinci_gpio_of_xlate(struct gpio_chip *gc,
 	if (gpiospec->args[0] > pdata->ngpio)
 		return -EINVAL;
 
-	if (gc != &chips[gpiospec->args[0] / 32].chip)
+	if (gc != &chips->chip)
 		return -EINVAL;
 
 	if (flags)
@@ -177,11 +186,11 @@ static int davinci_gpio_of_xlate(struct gpio_chip *gc,
 
 static int davinci_gpio_probe(struct platform_device *pdev)
 {
-	int i, base;
+	static int ctrl_num;
+	int gpio, bank;
 	unsigned ngpio, nbank;
 	struct davinci_gpio_controller *chips;
 	struct davinci_gpio_platform_data *pdata;
-	struct davinci_gpio_regs __iomem *regs;
 	struct device *dev = &pdev->dev;
 	struct resource *res;
 	char label[MAX_LABEL_SIZE];
@@ -220,38 +229,30 @@ static int davinci_gpio_probe(struct platform_device *pdev)
 	if (IS_ERR(gpio_base))
 		return PTR_ERR(gpio_base);
 
-	for (i = 0, base = 0; base < ngpio; i++, base += 32) {
-		snprintf(label, MAX_LABEL_SIZE, "davinci_gpio.%d", i);
-		chips[i].chip.label = devm_kstrdup(dev, label, GFP_KERNEL);
-		if (!chips[i].chip.label)
+	snprintf(label, MAX_LABEL_SIZE, "davinci_gpio.%d", ctrl_num++);
+	chips->chip.label = devm_kstrdup(dev, label, GFP_KERNEL);
+		if (!chips->chip.label)
 			return -ENOMEM;
 
-		chips[i].chip.direction_input = davinci_direction_in;
-		chips[i].chip.get = davinci_gpio_get;
-		chips[i].chip.direction_output = davinci_direction_out;
-		chips[i].chip.set = davinci_gpio_set;
+	chips->chip.direction_input = davinci_direction_in;
+	chips->chip.get = davinci_gpio_get;
+	chips->chip.direction_output = davinci_direction_out;
+	chips->chip.set = davinci_gpio_set;
 
-		chips[i].chip.base = base;
-		chips[i].chip.ngpio = ngpio - base;
-		if (chips[i].chip.ngpio > 32)
-			chips[i].chip.ngpio = 32;
+	chips->chip.ngpio = ngpio;
 
 #ifdef CONFIG_OF_GPIO
-		chips[i].chip.of_gpio_n_cells = 2;
-		chips[i].chip.of_xlate = davinci_gpio_of_xlate;
-		chips[i].chip.parent = dev;
-		chips[i].chip.of_node = dev->of_node;
+	chips->chip.of_gpio_n_cells = 2;
+	chips->chip.of_xlate = davinci_gpio_of_xlate;
+	chips->chip.parent = dev;
+	chips->chip.of_node = dev->of_node;
 #endif
-		spin_lock_init(&chips[i].lock);
-
-		regs = gpio_base + offset_array[i];
-		if (!regs)
-			return -ENXIO;
-		chips[i].regs = regs;
+	spin_lock_init(&chips->lock);
 
-		gpiochip_add_data(&chips[i].chip, &chips[i]);
-	}
+	for (gpio = 0, bank = 0; gpio < ngpio; gpio += 32, bank++)
+		chips->regs[bank] = gpio_base + offset_array[bank];
 
+	gpiochip_add_data(&chips->chip, chips);
 	platform_set_drvdata(pdev, chips);
 	davinci_gpio_irq_setup(pdev);
 	return 0;
@@ -312,16 +313,19 @@ static int gpio_irq_type(struct irq_data *d, unsigned trigger)
 
 static void gpio_irq_handler(struct irq_desc *desc)
 {
-	unsigned int irq = irq_desc_get_irq(desc);
 	struct davinci_gpio_regs __iomem *g;
 	u32 mask = 0xffff;
+	int bank_num;
 	struct davinci_gpio_controller *d;
+	struct davinci_gpio_irq_data *irqdata;
 
-	d = (struct davinci_gpio_controller *)irq_desc_get_handler_data(desc);
-	g = (struct davinci_gpio_regs __iomem *)d->regs;
+	irqdata = (struct davinci_gpio_irq_data *)irq_desc_get_handler_data(desc);
+	bank_num = irqdata->bank_num;
+	g = irqdata->regs;
+	d = irqdata->chip;
 
 	/* we only care about one bank */
-	if (irq & 1)
+	if ((bank_num % 2) == 1)
 		mask <<= 16;
 
 	/* temporarily mask (level sensitive) parent IRQ */
@@ -329,6 +333,7 @@ static void gpio_irq_handler(struct irq_desc *desc)
 	while (1) {
 		u32		status;
 		int		bit;
+		irq_hw_number_t hw_irq;
 
 		/* ack any irqs */
 		status = readl_relaxed(&g->intstat) & mask;
@@ -341,9 +346,13 @@ static void gpio_irq_handler(struct irq_desc *desc)
 		while (status) {
 			bit = __ffs(status);
 			status &= ~BIT(bit);
+			/* Max number of gpios per controller is 144 so
+			 * hw_irq will be in [0..143]
+			 */
+			hw_irq = (bank_num / 2) * 32 + bit;
+
 			generic_handle_irq(
-				irq_find_mapping(d->irq_domain,
-						 d->chip.base + bit));
+				irq_find_mapping(d->irq_domain, hw_irq));
 		}
 	}
 	chained_irq_exit(irq_desc_get_chip(desc), desc);
@@ -355,7 +364,7 @@ static int gpio_to_irq_banked(struct gpio_chip *chip, unsigned offset)
 	struct davinci_gpio_controller *d = gpiochip_get_data(chip);
 
 	if (d->irq_domain)
-		return irq_create_mapping(d->irq_domain, d->chip.base + offset);
+		return irq_create_mapping(d->irq_domain, offset);
 	else
 		return -ENXIO;
 }
@@ -369,7 +378,7 @@ static int gpio_to_irq_unbanked(struct gpio_chip *chip, unsigned offset)
 	 * can provide direct-mapped IRQs to AINTC (up to 32 GPIOs).
 	 */
 	if (offset < d->gpio_unbanked)
-		return d->gpio_irq + offset;
+		return d->base_irq + offset;
 	else
 		return -ENODEV;
 }
@@ -382,7 +391,7 @@ static int gpio_irq_type_unbanked(struct irq_data *data, unsigned trigger)
 
 	d = (struct davinci_gpio_controller *)irq_data_get_irq_handler_data(data);
 	g = (struct davinci_gpio_regs __iomem *)d->regs;
-	mask = __gpio_mask(data->irq - d->gpio_irq);
+	mask = __gpio_mask(data->irq - d->base_irq);
 
 	if (trigger & ~(IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING))
 		return -EINVAL;
@@ -401,7 +410,7 @@ static int gpio_irq_type_unbanked(struct irq_data *data, unsigned trigger)
 {
 	struct davinci_gpio_controller *chips =
 				(struct davinci_gpio_controller *)d->host_data;
-	struct davinci_gpio_regs __iomem *g = chips[hw / 32].regs;
+	struct davinci_gpio_regs __iomem *g = chips->regs[hw / 32];
 
 	irq_set_chip_and_handler_name(irq, &gpio_irqchip, handle_simple_irq,
 				"davinci_gpio");
@@ -459,6 +468,7 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 	struct irq_domain	*irq_domain = NULL;
 	const struct of_device_id *match;
 	struct irq_chip *irq_chip;
+	struct davinci_gpio_irq_data *irqdata;
 	gpio_get_irq_chip_cb_t gpio_get_irq_chip;
 
 	/*
@@ -514,10 +524,8 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 	 * IRQs, while the others use banked IRQs, would need some setup
 	 * tweaks to recognize hardware which can do that.
 	 */
-	for (gpio = 0, bank = 0; gpio < ngpio; bank++, gpio += 32) {
-		chips[bank].chip.to_irq = gpio_to_irq_banked;
-		chips[bank].irq_domain = irq_domain;
-	}
+	chips->chip.to_irq = gpio_to_irq_banked;
+	chips->irq_domain = irq_domain;
 
 	/*
 	 * AINTC can handle direct/unbanked IRQs for GPIOs, with the GPIO
@@ -526,9 +534,9 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 	 */
 	if (pdata->gpio_unbanked) {
 		/* pass "bank 0" GPIO IRQs to AINTC */
-		chips[0].chip.to_irq = gpio_to_irq_unbanked;
-		chips[0].gpio_irq = bank_irq;
-		chips[0].gpio_unbanked = pdata->gpio_unbanked;
+		chips->chip.to_irq = gpio_to_irq_unbanked;
+		chips->base_irq = bank_irq;
+		chips->gpio_unbanked = pdata->gpio_unbanked;
 		binten = GENMASK(pdata->gpio_unbanked / 16, 0);
 
 		/* AINTC handles mask/unmask; GPIO handles triggering */
@@ -538,14 +546,14 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 		irq_chip->irq_set_type = gpio_irq_type_unbanked;
 
 		/* default trigger: both edges */
-		g = chips[0].regs;
+		g = chips->regs[0];
 		writel_relaxed(~0, &g->set_falling);
 		writel_relaxed(~0, &g->set_rising);
 
 		/* set the direct IRQs up to use that irqchip */
 		for (gpio = 0; gpio < pdata->gpio_unbanked; gpio++, irq++) {
 			irq_set_chip(irq, irq_chip);
-			irq_set_handler_data(irq, &chips[gpio / 32]);
+			irq_set_handler_data(irq, chips);
 			irq_set_status_flags(irq, IRQ_TYPE_EDGE_BOTH);
 		}
 
@@ -561,7 +569,7 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 		 * There are register sets for 32 GPIOs. 2 banks of 16
 		 * GPIOs are covered by each set of registers hence divide by 2
 		 */
-		g = chips[bank / 2].regs;
+		g = chips->regs[bank / 2];
 		writel_relaxed(~0, &g->clr_falling);
 		writel_relaxed(~0, &g->clr_rising);
 
@@ -570,8 +578,19 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 		 * gpio irqs. Pass the irq bank's corresponding controller to
 		 * the chained irq handler.
 		 */
+		irqdata = devm_kzalloc(&pdev->dev,
+				       sizeof(struct
+					      davinci_gpio_irq_data),
+					      GFP_KERNEL);
+		if (!irqdata)
+			return -ENOMEM;
+
+		irqdata->regs = g;
+		irqdata->bank_num = bank;
+		irqdata->chip = chips;
+
 		irq_set_chained_handler_and_data(bank_irq, gpio_irq_handler,
-						 &chips[gpio / 32]);
+						 irqdata);
 
 		binten |= BIT(bank);
 	}
diff --git a/include/linux/platform_data/gpio-davinci.h b/include/linux/platform_data/gpio-davinci.h
index 18127c4..c62a943 100644
--- a/include/linux/platform_data/gpio-davinci.h
+++ b/include/linux/platform_data/gpio-davinci.h
@@ -21,19 +21,27 @@
 
 #include <asm-generic/gpio.h>
 
+#define MAX_REGS_BANKS		5
+
 struct davinci_gpio_platform_data {
 	u32	ngpio;
 	u32	gpio_unbanked;
 };
 
+struct davinci_gpio_irq_data {
+	void __iomem			*regs;
+	struct davinci_gpio_controller	*chip;
+	int				bank_num;
+};
+
 struct davinci_gpio_controller {
 	struct gpio_chip	chip;
 	struct irq_domain	*irq_domain;
 	/* Serialize access to GPIO registers */
 	spinlock_t		lock;
-	void __iomem		*regs;
+	void __iomem		*regs[MAX_REGS_BANKS];
 	int			gpio_unbanked;
-	unsigned		gpio_irq;
+	unsigned int		base_irq;
 };
 
 /*
-- 
1.9.1

Re: [PATCH V2 3/6] gpio: davinci: Redesign driver to accommodate ngpios in one gpio chip

[Linus Walleij]

On Fri, Jan 13, 2017 at 5:20 AM, Keerthy <j-keerthy@ti.com> wrote:

> The Davinci GPIO driver is implemented to work with one monolithic
> Davinci GPIO platform device which may have up to Y(144) gpios.
> The Davinci GPIO driver instantiates number of GPIO chips with
> max 32 gpio pins per each during initialization and one IRQ domain.
> So, the current GPIO's  opjects structure is:
>
> <platform device> Davinci GPIO controller
>  |- <gpio0_chip0> ------|
>  ...                    |--- irq_domain (hwirq [0..143])
>  |- <gpio0_chipN> ------|
>
> Current driver creates one chip for every 32 GPIOs in a controller.
> This was a limitation earlier now there is no need for that. Hence
> redesigning the driver to create one gpio chip for all the ngpio
> in the controller.
>
> |- <gpio0_chip0> ------|--- irq_domain (hwirq [0..143]).
>
> The previous discussion on this can be found here:
> https://www.spinics.net/lists/linux-omap/msg132869.html
>
> Signed-off-by: Keerthy <j-keerthy@ti.com>

Patch applied.

Yours,
Linus Walleij

[PATCH V2 4/6] gpio: davinci: Add support for multiple GPIO controllers

[Keerthy]

Update GPIO driver to support Multiple GPIO controllers by updating
the base of subsequent GPIO chips with total of previous chips
gpio count so that gpio_add_chip gets unique numbers.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 drivers/gpio/gpio-davinci.c                | 4 +++-
 include/linux/platform_data/gpio-davinci.h | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index a527e88..2a5ae04 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -186,7 +186,7 @@ static int davinci_gpio_of_xlate(struct gpio_chip *gc,
 
 static int davinci_gpio_probe(struct platform_device *pdev)
 {
-	static int ctrl_num;
+	static int ctrl_num, bank_base;
 	int gpio, bank;
 	unsigned ngpio, nbank;
 	struct davinci_gpio_controller *chips;
@@ -240,6 +240,7 @@ static int davinci_gpio_probe(struct platform_device *pdev)
 	chips->chip.set = davinci_gpio_set;
 
 	chips->chip.ngpio = ngpio;
+	chips->chip.base = bank_base;
 
 #ifdef CONFIG_OF_GPIO
 	chips->chip.of_gpio_n_cells = 2;
@@ -248,6 +249,7 @@ static int davinci_gpio_probe(struct platform_device *pdev)
 	chips->chip.of_node = dev->of_node;
 #endif
 	spin_lock_init(&chips->lock);
+	bank_base += ngpio;
 
 	for (gpio = 0, bank = 0; gpio < ngpio; gpio += 32, bank++)
 		chips->regs[bank] = gpio_base + offset_array[bank];
diff --git a/include/linux/platform_data/gpio-davinci.h b/include/linux/platform_data/gpio-davinci.h
index c62a943..90ae19c 100644
--- a/include/linux/platform_data/gpio-davinci.h
+++ b/include/linux/platform_data/gpio-davinci.h
@@ -42,6 +42,7 @@ struct davinci_gpio_controller {
 	void __iomem		*regs[MAX_REGS_BANKS];
 	int			gpio_unbanked;
 	unsigned int		base_irq;
+	unsigned int		base;
 };
 
 /*
-- 
1.9.1

[PATCH V2 5/6] gpio: davinci: Remove custom .xlate

[Keerthy]

With the current redesign of driver it's not necessary to have
custom .xlate() as the gpiolib will assign default of_gpio_simple_xlate().

Suggested-by: Grygorii Strashko <grygorii.strashko@ti.com>
Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 drivers/gpio/gpio-davinci.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 2a5ae04..86b4950 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -163,27 +163,6 @@ static int davinci_gpio_get(struct gpio_chip *chip, unsigned offset)
 	return NULL;
 }
 
-#ifdef CONFIG_OF_GPIO
-static int davinci_gpio_of_xlate(struct gpio_chip *gc,
-			     const struct of_phandle_args *gpiospec,
-			     u32 *flags)
-{
-	struct davinci_gpio_controller *chips = dev_get_drvdata(gc->parent);
-	struct davinci_gpio_platform_data *pdata = dev_get_platdata(gc->parent);
-
-	if (gpiospec->args[0] > pdata->ngpio)
-		return -EINVAL;
-
-	if (gc != &chips->chip)
-		return -EINVAL;
-
-	if (flags)
-		*flags = gpiospec->args[1];
-
-	return gpiospec->args[0] % 32;
-}
-#endif
-
 static int davinci_gpio_probe(struct platform_device *pdev)
 {
 	static int ctrl_num, bank_base;
@@ -244,7 +223,6 @@ static int davinci_gpio_probe(struct platform_device *pdev)
 
 #ifdef CONFIG_OF_GPIO
 	chips->chip.of_gpio_n_cells = 2;
-	chips->chip.of_xlate = davinci_gpio_of_xlate;
 	chips->chip.parent = dev;
 	chips->chip.of_node = dev->of_node;
 #endif
-- 
1.9.1

[PATCH V2 6/6] gpio: davinci: Remove redundant macros

[Keerthy]

Some of the macros were needed as per old driver design.
With the current implementation they are unwanted. Hence remove
them.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 include/linux/platform_data/gpio-davinci.h | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/include/linux/platform_data/gpio-davinci.h b/include/linux/platform_data/gpio-davinci.h
index 90ae19c..f922601 100644
--- a/include/linux/platform_data/gpio-davinci.h
+++ b/include/linux/platform_data/gpio-davinci.h
@@ -45,14 +45,6 @@ struct davinci_gpio_controller {
 	unsigned int		base;
 };
 
-/*
- * basic gpio routines
- */
-#define	GPIO(X)		(X)	/* 0 <= X <= (DAVINCI_N_GPIO - 1) */
-
-/* Convert GPIO signal to GPIO pin number */
-#define GPIO_TO_PIN(bank, gpio)	(16 * (bank) + (gpio))
-
 static inline u32 __gpio_mask(unsigned gpio)
 {
 	return 1 << (gpio % 32);
-- 
1.9.1

Re: [PATCH V2 6/6] gpio: davinci: Remove redundant macros

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 5741 bytes --]

Hi Keerthy,

[auto build test ERROR on gpio/for-next]
[also build test ERROR on next-20170113]
[cannot apply to v4.10-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Keerthy/gpio-davinci-Redesign-driver-to-accommodate-ngpios-in-one-gpio-chip/20170114-204048
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: arm-davinci_all_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All error/warnings (new ones prefixed by >>):

>> arch/arm/mach-davinci/board-neuros-osd2.c:131:34: error: implicit declaration of function 'GPIO' [-Werror=implicit-function-declaration]
     { .name = "led1_green", .gpio = GPIO(10), },
                                     ^~~~
>> arch/arm/mach-davinci/board-neuros-osd2.c:131:34: error: initializer element is not constant
   arch/arm/mach-davinci/board-neuros-osd2.c:131:34: note: (near initialization for 'ntosd2_leds[0].gpio')
   arch/arm/mach-davinci/board-neuros-osd2.c:132:34: error: initializer element is not constant
     { .name = "led1_red",   .gpio = GPIO(11), },
                                     ^~~~
   arch/arm/mach-davinci/board-neuros-osd2.c:132:34: note: (near initialization for 'ntosd2_leds[1].gpio')
   arch/arm/mach-davinci/board-neuros-osd2.c:133:34: error: initializer element is not constant
     { .name = "led2_green", .gpio = GPIO(12), },
                                     ^~~~
   arch/arm/mach-davinci/board-neuros-osd2.c:133:34: note: (near initialization for 'ntosd2_leds[2].gpio')
   arch/arm/mach-davinci/board-neuros-osd2.c:134:34: error: initializer element is not constant
     { .name = "led2_red",   .gpio = GPIO(13), },
                                     ^~~~
   arch/arm/mach-davinci/board-neuros-osd2.c:134:34: note: (near initialization for 'ntosd2_leds[3].gpio')
   cc1: some warnings being treated as errors
--
   arch/arm/mach-davinci/board-da830-evm.c: In function 'da830_evm_usb_set_power':
>> arch/arm/mach-davinci/board-da830-evm.c:45:23: error: implicit declaration of function 'GPIO_TO_PIN' [-Werror=implicit-function-declaration]
    #define ON_BD_USB_DRV GPIO_TO_PIN(1, 15)
                          ^
>> arch/arm/mach-davinci/board-da830-evm.c:57:17: note: in expansion of macro 'ON_BD_USB_DRV'
     gpio_set_value(ON_BD_USB_DRV, on);
                    ^~~~~~~~~~~~~
   cc1: some warnings being treated as errors
--
   arch/arm/mach-davinci/board-da850-evm.c: In function 'da850_panel_power_ctrl':
>> arch/arm/mach-davinci/board-da850-evm.c:58:27: error: implicit declaration of function 'GPIO_TO_PIN' [-Werror=implicit-function-declaration]
    #define DA850_LCD_BL_PIN  GPIO_TO_PIN(2, 15)
                              ^
>> arch/arm/mach-davinci/board-da850-evm.c:790:17: note: in expansion of macro 'DA850_LCD_BL_PIN'
     gpio_set_value(DA850_LCD_BL_PIN, val);
                    ^~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors
--
   arch/arm/mach-davinci/board-omapl138-hawk.c: In function 'hawk_usb_set_power':
>> arch/arm/mach-davinci/board-omapl138-hawk.c:30:30: error: implicit declaration of function 'GPIO_TO_PIN' [-Werror=implicit-function-declaration]
    #define DA850_USB1_VBUS_PIN  GPIO_TO_PIN(2, 4)
                                 ^
>> arch/arm/mach-davinci/board-omapl138-hawk.c:174:17: note: in expansion of macro 'DA850_USB1_VBUS_PIN'
     gpio_set_value(DA850_USB1_VBUS_PIN, on);
                    ^~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/GPIO_TO_PIN +45 arch/arm/mach-davinci/board-da830-evm.c

8593790d Mark A. Greer     2009-06-03  39  #include <mach/da8xx.h>
8593790d Mark A. Greer     2009-06-03  40  
782f2d78 Cyril Chemparathy 2010-09-15  41  #define DA830_EVM_PHY_ID		""
0e9a3ddc Sergei Shtylyov   2009-09-25  42  /*
0e9a3ddc Sergei Shtylyov   2009-09-25  43   * USB1 VBUS is controlled by GPIO1[15], over-current is reported on GPIO2[4].
0e9a3ddc Sergei Shtylyov   2009-09-25  44   */
0e9a3ddc Sergei Shtylyov   2009-09-25 @45  #define ON_BD_USB_DRV	GPIO_TO_PIN(1, 15)
0e9a3ddc Sergei Shtylyov   2009-09-25  46  #define ON_BD_USB_OVC	GPIO_TO_PIN(2, 4)
0e9a3ddc Sergei Shtylyov   2009-09-25  47  
0e9a3ddc Sergei Shtylyov   2009-09-25  48  static const short da830_evm_usb11_pins[] = {
0e9a3ddc Sergei Shtylyov   2009-09-25  49  	DA830_GPIO1_15, DA830_GPIO2_4,
0e9a3ddc Sergei Shtylyov   2009-09-25  50  	-1
0e9a3ddc Sergei Shtylyov   2009-09-25  51  };
0e9a3ddc Sergei Shtylyov   2009-09-25  52  
0e9a3ddc Sergei Shtylyov   2009-09-25  53  static da8xx_ocic_handler_t da830_evm_usb_ocic_handler;
0e9a3ddc Sergei Shtylyov   2009-09-25  54  
0e9a3ddc Sergei Shtylyov   2009-09-25  55  static int da830_evm_usb_set_power(unsigned port, int on)
0e9a3ddc Sergei Shtylyov   2009-09-25  56  {
0e9a3ddc Sergei Shtylyov   2009-09-25 @57  	gpio_set_value(ON_BD_USB_DRV, on);
0e9a3ddc Sergei Shtylyov   2009-09-25  58  	return 0;
0e9a3ddc Sergei Shtylyov   2009-09-25  59  }
0e9a3ddc Sergei Shtylyov   2009-09-25  60  

:::::: The code at line 45 was first introduced by commit
:::::: 0e9a3ddc91882a19e255dceb18b712f57e3bb731 davinci: DA830 EVM: OHCI platform code

:::::: TO: Sergei Shtylyov <sshtylyov@ru.mvista.com>
:::::: CC: Kevin Hilman <khilman@deeprootsystems.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23085 bytes --]

Re: [PATCH V2 6/6] gpio: davinci: Remove redundant macros

[Keerthy]

Linus,

On Friday 13 January 2017 09:50 AM, Keerthy wrote:
> Some of the macros were needed as per old driver design.
> With the current implementation they are unwanted. Hence remove
> them.

Seems like Macros are being used in:
arch/arm/mach-davinci/board-neuros-osd2.c

So this patch can be dropped from the series.

>
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
>  include/linux/platform_data/gpio-davinci.h | 8 --------
>  1 file changed, 8 deletions(-)
>
> diff --git a/include/linux/platform_data/gpio-davinci.h b/include/linux/platform_data/gpio-davinci.h
> index 90ae19c..f922601 100644
> --- a/include/linux/platform_data/gpio-davinci.h
> +++ b/include/linux/platform_data/gpio-davinci.h
> @@ -45,14 +45,6 @@ struct davinci_gpio_controller {
>  	unsigned int		base;
>  };
>
> -/*
> - * basic gpio routines
> - */
> -#define	GPIO(X)		(X)	/* 0 <= X <= (DAVINCI_N_GPIO - 1) */
> -
> -/* Convert GPIO signal to GPIO pin number */
> -#define GPIO_TO_PIN(bank, gpio)	(16 * (bank) + (gpio))
> -
>  static inline u32 __gpio_mask(unsigned gpio)
>  {
>  	return 1 << (gpio % 32);
>

Re: [PATCH V2 0/6] gpio: davinci: Redesign driver to accommodate ngpios in one gpio chip

[Grygorii Strashko]

On 01/12/2017 10:20 PM, Keerthy wrote:
> The Davinci GPIO driver is implemented to work with one monolithic
> Davinci GPIO platform device which may have up to Y(144) gpios.
> The Davinci GPIO driver instantiates number of GPIO chips with
> max 32 gpio pins per each during initialization and one IRQ domain.
> So, the current GPIO's  opjects structure is:
>
> <platform device> Davinci GPIO controller
>  |- <gpio0_chip0> ------|
>  ...                    |--- irq_domain (hwirq [0..143])
>  |- <gpio0_chipN> ------|
>
> Current driver creates one chip for every 32 GPIOs in a controller.
> This was a limitation earlier now there is no need for that. Hence
> redesigning the driver to create one gpio chip for all the ngpio
> in the controller.
>
> |- <gpio0_chip0> ------|--- irq_domain (hwirq [0..143]).
>
> The previous discussion on this can be found here:
> https://www.spinics.net/lists/linux-omap/msg132869.html
>
> The series is posted on top of:
>
> https://lkml.org/lkml/2017/1/4/94
>
> Changes in v2:
>
>   * Optimized the re-design patch.
>   * Added couple of code clean ups after the re-design.
>   * Included v2 of https://patchwork.ozlabs.org/patch/710855/
>
> Keerthy (6):
>   gpio: davinci: Remove gpio2regs function to accommodate multi
>     instances
>   gpio: davinci: Remove unwanted blank line
>   gpio: davinci: Redesign driver to accommodate ngpios in one gpio chip
>   gpio: davinci: Add support for multiple GPIO controllers
>   gpio: davinci: Remove custom .xlate
>   gpio: davinci: Remove redundant macros
>
>  drivers/gpio/gpio-davinci.c                | 174 +++++++++++++----------------
>  include/linux/platform_data/gpio-davinci.h |  20 ++--
>  2 files changed, 90 insertions(+), 104 deletions(-)
>

Reviewed-by: Grygorii Strashko <grygorii.strashko@ti.com>

-- 
regards,
-grygorii