All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] gpio: davinci: Add k2g support
@ 2016-10-19  5:33 Keerthy
  2016-10-19  5:33 ` [PATCH 1/5] gpio: davinci: Remove gpio2regs function to accommodate multi instances Keerthy
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Keerthy @ 2016-10-19  5:33 UTC (permalink / raw)
  To: linus.walleij-QSEj5FYQhm4dnm+yROfE0A
  Cc: gnurou-Re5JQEeQqe8AvxtiuMwx3w, lokeshvutla-l0cyMroinI0,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA, rogerq-l0cyMroinI0,
	grygorii.strashko-l0cyMroinI0, j-keerthy-l0cyMroinI0

k2g has two instances of gpio controller. The gpio controller
is bit identical to the one used in da-vinci. The da-vinci gpio
driver was written keeping in mind about only one instance of
gpio controller. The patch series enhances the driver to support
multiple instances.

k2g has 2 instances of gpio controller with 144 pins each.
Totally 9 banks with 16 pins each totalling to 144 pins per
controller instance. GPIO1 instance has GPIO:68 to GPIO:143
marked as reserved hence we have a total of 144(bank0) and 68
(bank1) GPIOs. 

Grygorii Strashko (1):
  gpio: davinci: irq support for multiple gpio controllers

Keerthy (3):
  gpio: davinci: Remove gpio2regs function to accommodate      multi
    instances
  gpio: davinci: Store both irqs into the controller
  gpio: davinci: Add a separate compatible for k2g

Lokesh Vutla (1):
  drivers: gpio: Add support for multiple IPs

 .../devicetree/bindings/gpio/gpio-davinci.txt      |   2 +-
 drivers/gpio/gpio-davinci.c                        | 115 ++++++++++++++-------
 include/linux/platform_data/gpio-davinci.h         |   3 +
 3 files changed, 84 insertions(+), 36 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/5] gpio: davinci: Remove gpio2regs function to accommodate  multi instances
  2016-10-19  5:33 [PATCH 0/5] gpio: davinci: Add k2g support Keerthy
@ 2016-10-19  5:33 ` Keerthy
  2016-10-19  5:33 ` [PATCH 4/5] gpio: davinci: Store both irqs into the controller Keerthy
       [not found] ` <1476855239-32730-1-git-send-email-j-keerthy-l0cyMroinI0@public.gmane.org>
  2 siblings, 0 replies; 17+ messages in thread
From: Keerthy @ 2016-10-19  5:33 UTC (permalink / raw)
  To: linus.walleij
  Cc: gnurou, lokeshvutla, robh+dt, linux-gpio, devicetree, linux-omap,
	rogerq, grygorii.strashko, j-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>
---
 drivers/gpio/gpio-davinci.c | 30 +++++++-----------------------
 1 file changed, 7 insertions(+), 23 deletions(-)

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index dd262f0..419cfaf 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -42,25 +42,7 @@ struct davinci_gpio_regs {
 #define BINTEN	0x8 /* GPIO Interrupt Per-Bank Enable Register */
 
 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 offset_array[5] = {0x10, 0x38, 0x60, 0x88, 0xb0};
 
 static inline struct davinci_gpio_regs __iomem *irq2regs(struct irq_data *d)
 {
@@ -257,7 +239,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;
@@ -415,7 +397,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");
@@ -552,7 +536,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);
 
@@ -572,7 +556,7 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 	 */
 	for (gpio = 0, bank = 0; gpio < ngpio; bank++, bank_irq++, gpio += 16) {
 		/* disabled by default, enabled only as needed */
-		g = gpio2regs(gpio);
+		g = chips[bank / 2].regs;
 		writel_relaxed(~0, &g->clr_falling);
 		writel_relaxed(~0, &g->clr_rising);
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/5] drivers: gpio: Add support for multiple IPs
       [not found] ` <1476855239-32730-1-git-send-email-j-keerthy-l0cyMroinI0@public.gmane.org>
@ 2016-10-19  5:33   ` Keerthy
       [not found]     ` <1476855239-32730-3-git-send-email-j-keerthy-l0cyMroinI0@public.gmane.org>
  2016-10-19  5:33   ` [PATCH 3/5] gpio: davinci: irq support for multiple gpio controllers Keerthy
  2016-10-19  5:33   ` [PATCH 5/5] gpio: davinci: Add a separate compatible for k2g Keerthy
  2 siblings, 1 reply; 17+ messages in thread
From: Keerthy @ 2016-10-19  5:33 UTC (permalink / raw)
  To: linus.walleij-QSEj5FYQhm4dnm+yROfE0A
  Cc: gnurou-Re5JQEeQqe8AvxtiuMwx3w, lokeshvutla-l0cyMroinI0,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA, rogerq-l0cyMroinI0,
	grygorii.strashko-l0cyMroinI0, j-keerthy-l0cyMroinI0

From: Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org>

Update GPIO driver to support Multiple GPIO IPs.

Signed-off-by: Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org>
Signed-off-by: Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org>
---
 drivers/gpio/gpio-davinci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 419cfaf..2654dae 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -183,6 +183,7 @@ static int davinci_gpio_probe(struct platform_device *pdev)
 	struct davinci_gpio_regs __iomem *regs;
 	struct device *dev = &pdev->dev;
 	struct resource *res;
+	static int bank_base;
 
 	pdata = davinci_gpio_get_pdata(pdev);
 	if (!pdata) {
@@ -226,7 +227,8 @@ static int davinci_gpio_probe(struct platform_device *pdev)
 		chips[i].chip.direction_output = davinci_direction_out;
 		chips[i].chip.set = davinci_gpio_set;
 
-		chips[i].chip.base = base;
+		chips[i].chip.base = bank_base;
+		bank_base += 32;
 		chips[i].chip.ngpio = ngpio - base;
 		if (chips[i].chip.ngpio > 32)
 			chips[i].chip.ngpio = 32;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/5] gpio: davinci: irq support for multiple gpio controllers
       [not found] ` <1476855239-32730-1-git-send-email-j-keerthy-l0cyMroinI0@public.gmane.org>
  2016-10-19  5:33   ` [PATCH 2/5] drivers: gpio: Add support for multiple IPs Keerthy
@ 2016-10-19  5:33   ` Keerthy
  2016-10-19  5:33   ` [PATCH 5/5] gpio: davinci: Add a separate compatible for k2g Keerthy
  2 siblings, 0 replies; 17+ messages in thread
From: Keerthy @ 2016-10-19  5:33 UTC (permalink / raw)
  To: linus.walleij-QSEj5FYQhm4dnm+yROfE0A
  Cc: gnurou-Re5JQEeQqe8AvxtiuMwx3w, lokeshvutla-l0cyMroinI0,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA, rogerq-l0cyMroinI0,
	grygorii.strashko-l0cyMroinI0, j-keerthy-l0cyMroinI0

From: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>

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

The gpio2hwirq conversation is performing in the following way:
hwirq = gpio0_chipX.base + gpio_offsetN

where gpio_offsetN is gpio pin number inside gpio0_chipX
and gpio0_chipX.base can have values 0..128 with step 32.

Above will work properly only when one Davinci GPIO controller is
present, but if second one (68 gpios) is added IRQs will not work for it,
because gpio1_chipX.base will have values 144..208 with step 32 and
above formula will not work any more.

Hence, update Davinci GPIO driver to handle this situation properly:
- add new field in struct davinci_gpio_controller.ctrl_base and
  save Linux GPIO number of the first GPIO assigned to the Davinci GPIO
  controller in .probe() - value of static variable "bank_base";
- correct gpio2hwirq conversation formula as below
  hwirq = (gpio0_chipX.base - gpio_controller.ctrl_base) + gpio_offsetN

Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
[j-keerthy-l0cyMroinI0@public.gmane.org added the ctrl_base to keep track of gpios per controller]
Signed-off-by: Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org>
---
 drivers/gpio/gpio-davinci.c                | 28 ++++++++++++++++++++++------
 include/linux/platform_data/gpio-davinci.h |  1 +
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 2654dae..2bc308a 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -176,7 +176,7 @@ static int davinci_gpio_of_xlate(struct gpio_chip *gc,
 
 static int davinci_gpio_probe(struct platform_device *pdev)
 {
-	int i, base;
+	int i, base, temp_ctrl_base;
 	unsigned ngpio, nbank;
 	struct davinci_gpio_controller *chips;
 	struct davinci_gpio_platform_data *pdata;
@@ -219,6 +219,8 @@ static int davinci_gpio_probe(struct platform_device *pdev)
 	if (IS_ERR(gpio_base))
 		return PTR_ERR(gpio_base);
 
+	temp_ctrl_base = bank_base;
+
 	for (i = 0, base = 0; base < ngpio; i++, base += 32) {
 		chips[i].chip.label = "DaVinci";
 
@@ -228,10 +230,13 @@ static int davinci_gpio_probe(struct platform_device *pdev)
 		chips[i].chip.set = davinci_gpio_set;
 
 		chips[i].chip.base = bank_base;
+		chips[i].ctrl_base = temp_ctrl_base;
 		bank_base += 32;
 		chips[i].chip.ngpio = ngpio - base;
 		if (chips[i].chip.ngpio > 32)
 			chips[i].chip.ngpio = 32;
+		else
+			bank_base = ngpio;
 
 #ifdef CONFIG_OF_GPIO
 		chips[i].chip.of_gpio_n_cells = 2;
@@ -329,6 +334,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 +347,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 = (d->chip.base - d->ctrl_base) + 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);
@@ -353,11 +363,17 @@ static void gpio_irq_handler(struct irq_desc *desc)
 static int gpio_to_irq_banked(struct gpio_chip *chip, unsigned offset)
 {
 	struct davinci_gpio_controller *d = gpiochip_get_data(chip);
+	irq_hw_number_t hw_irq;
 
-	if (d->irq_domain)
-		return irq_create_mapping(d->irq_domain, d->chip.base + offset);
-	else
+	if (d->irq_domain) {
+		/* Max number of gpios per controller is 144 so
+		 * hw_irq will be in [0..143]
+		 */
+		hw_irq = (d->chip.base - d->ctrl_base) + offset;
+		return irq_create_mapping(d->irq_domain, hw_irq);
+	} else {
 		return -ENXIO;
+	}
 }
 
 static int gpio_to_irq_unbanked(struct gpio_chip *chip, unsigned offset)
diff --git a/include/linux/platform_data/gpio-davinci.h b/include/linux/platform_data/gpio-davinci.h
index 6ace3fd..0a0cdd7 100644
--- a/include/linux/platform_data/gpio-davinci.h
+++ b/include/linux/platform_data/gpio-davinci.h
@@ -38,6 +38,7 @@ struct davinci_gpio_controller {
 	void __iomem		*in_data;
 	int			gpio_unbanked;
 	unsigned		gpio_irq;
+	unsigned int		ctrl_base;
 };
 
 /*
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/5] gpio: davinci: Store both irqs into the controller
  2016-10-19  5:33 [PATCH 0/5] gpio: davinci: Add k2g support Keerthy
  2016-10-19  5:33 ` [PATCH 1/5] gpio: davinci: Remove gpio2regs function to accommodate multi instances Keerthy
@ 2016-10-19  5:33 ` Keerthy
       [not found] ` <1476855239-32730-1-git-send-email-j-keerthy-l0cyMroinI0@public.gmane.org>
  2 siblings, 0 replies; 17+ messages in thread
From: Keerthy @ 2016-10-19  5:33 UTC (permalink / raw)
  To: linus.walleij
  Cc: gnurou, lokeshvutla, robh+dt, linux-gpio, devicetree, linux-omap,
	rogerq, grygorii.strashko, j-keerthy

There are 32 GPIOs per controller which means 2 banks with 16 gpios
and 2 separate irqs. Hence store the both the irq numbers for the
controller which makes it easier to distnguish the bank in irq handler.

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

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 2bc308a..f7c506b 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -326,7 +326,7 @@ static void gpio_irq_handler(struct irq_desc *desc)
 	g = (struct davinci_gpio_regs __iomem *)d->regs;
 
 	/* we only care about one bank */
-	if (irq & 1)
+	if (irq == d->birq2)
 		mask <<= 16;
 
 	/* temporarily mask (level sensitive) parent IRQ */
@@ -578,6 +578,12 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 		writel_relaxed(~0, &g->clr_falling);
 		writel_relaxed(~0, &g->clr_rising);
 
+		bank_irq = platform_get_irq(pdev, bank);
+		if (bank % 2)
+			chips[bank / 2].birq2 = bank_irq;
+		else
+			chips[bank / 2].birq1 = bank_irq;
+
 		/*
 		 * Each chip handles 32 gpios, and each irq bank consists of 16
 		 * gpio irqs. Pass the irq bank's corresponding controller to
diff --git a/include/linux/platform_data/gpio-davinci.h b/include/linux/platform_data/gpio-davinci.h
index 0a0cdd7..6439b81 100644
--- a/include/linux/platform_data/gpio-davinci.h
+++ b/include/linux/platform_data/gpio-davinci.h
@@ -39,6 +39,8 @@ struct davinci_gpio_controller {
 	int			gpio_unbanked;
 	unsigned		gpio_irq;
 	unsigned		ctrl_base;
+	unsigned int		birq1;
+	unsigned int		birq2;
 };
 
 /*
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 5/5] gpio: davinci: Add a separate compatible for k2g
       [not found] ` <1476855239-32730-1-git-send-email-j-keerthy-l0cyMroinI0@public.gmane.org>
  2016-10-19  5:33   ` [PATCH 2/5] drivers: gpio: Add support for multiple IPs Keerthy
  2016-10-19  5:33   ` [PATCH 3/5] gpio: davinci: irq support for multiple gpio controllers Keerthy
@ 2016-10-19  5:33   ` Keerthy
       [not found]     ` <1476855239-32730-6-git-send-email-j-keerthy-l0cyMroinI0@public.gmane.org>
  2 siblings, 1 reply; 17+ messages in thread
From: Keerthy @ 2016-10-19  5:33 UTC (permalink / raw)
  To: linus.walleij-QSEj5FYQhm4dnm+yROfE0A
  Cc: gnurou-Re5JQEeQqe8AvxtiuMwx3w, lokeshvutla-l0cyMroinI0,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA, rogerq-l0cyMroinI0,
	grygorii.strashko-l0cyMroinI0, j-keerthy-l0cyMroinI0

In the case of k2g the clocks are handled differently as when compared
with other keystones. Hence adding a separate compatible and match tables
accordingly.

Signed-off-by: Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org>
---
 .../devicetree/bindings/gpio/gpio-davinci.txt      |  2 +-
 drivers/gpio/gpio-davinci.c                        | 45 ++++++++++++++++++++--
 2 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
index 5079ba7..a76abd2 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
@@ -1,7 +1,7 @@
 Davinci/Keystone GPIO controller bindings
 
 Required Properties:
-- compatible: should be "ti,dm6441-gpio", "ti,keystone-gpio"
+- compatible: should be "ti,dm6441-gpio", "ti,keystone-gpio", "ti,k2g-gpio"
 
 - reg: Physical base address of the controller and the size of memory mapped
        registers.
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index f7c506b..d91a9a8 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -452,6 +452,26 @@ static struct irq_chip *keystone_gpio_get_irq_chip(unsigned int irq)
 
 static const struct of_device_id davinci_gpio_ids[];
 
+struct gpio_driver_data {
+	gpio_get_irq_chip_cb_t gpio_get_irq_chip;
+	bool clk_optional;
+};
+
+static struct gpio_driver_data davinci_data = {
+	.gpio_get_irq_chip = davinci_gpio_get_irq_chip,
+	.clk_optional = false,
+};
+
+static struct gpio_driver_data keystone_data = {
+	.gpio_get_irq_chip = keystone_gpio_get_irq_chip,
+	.clk_optional = false,
+};
+
+static struct gpio_driver_data k2g_data = {
+	.gpio_get_irq_chip = keystone_gpio_get_irq_chip,
+	.clk_optional = true,
+};
+
 /*
  * NOTE:  for suspend/resume, probably best to make a platform_device with
  * suspend_late/resume_resume calls hooking into results of the set_wake()
@@ -475,6 +495,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 gpio_driver_data *driver_data = NULL;
 	gpio_get_irq_chip_cb_t gpio_get_irq_chip;
 
 	/*
@@ -483,8 +504,10 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 	gpio_get_irq_chip = davinci_gpio_get_irq_chip;
 	match = of_match_device(of_match_ptr(davinci_gpio_ids),
 				dev);
-	if (match)
-		gpio_get_irq_chip = (gpio_get_irq_chip_cb_t)match->data;
+	if (match) {
+		driver_data = (struct gpio_driver_data *)match->data;
+		gpio_get_irq_chip = driver_data->gpio_get_irq_chip;
+	}
 
 	ngpio = pdata->ngpio;
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -500,6 +523,9 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	if (driver_data && driver_data->clk_optional)
+		goto skip_clk_handling;
+
 	clk = devm_clk_get(dev, "gpio");
 	if (IS_ERR(clk)) {
 		printk(KERN_ERR "Error %ld getting gpio clock?\n",
@@ -508,6 +534,7 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 	}
 	clk_prepare_enable(clk);
 
+skip_clk_handling:
 	if (!pdata->gpio_unbanked) {
 		irq = irq_alloc_descs(-1, 0, ngpio, 0);
 		if (irq < 0) {
@@ -607,8 +634,18 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 
 #if IS_ENABLED(CONFIG_OF)
 static const struct of_device_id davinci_gpio_ids[] = {
-	{ .compatible = "ti,keystone-gpio", keystone_gpio_get_irq_chip},
-	{ .compatible = "ti,dm6441-gpio", davinci_gpio_get_irq_chip},
+	{
+		.compatible = "ti,keystone-gpio",
+		.data = &keystone_data,
+	},
+	{
+		.compatible = "ti,dm6441-gpio",
+		.data = &davinci_data,
+	},
+	{
+		.compatible = "ti,k2g-gpio",
+		.data = &k2g_data,
+	},
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, davinci_gpio_ids);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/5] drivers: gpio: Add support for multiple IPs
       [not found]     ` <1476855239-32730-3-git-send-email-j-keerthy-l0cyMroinI0@public.gmane.org>
@ 2016-10-23 10:32       ` Linus Walleij
       [not found]         ` <CACRpkdbsgM-yiWpiA5G35jQR8v3FO=Me+4rhe6TzE0qsWOJ8Ew-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Linus Walleij @ 2016-10-23 10:32 UTC (permalink / raw)
  To: Keerthy
  Cc: Alexandre Courbot, Lokesh Vutla, Rob Herring,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux-OMAP, Roger Quadros,
	Grygorii Strashko

On Wed, Oct 19, 2016 at 7:33 AM, Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org> wrote:

> From: Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org>
>
> Update GPIO driver to support Multiple GPIO IPs.
>
> Signed-off-by: Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org>

This commit message is not at all describing what the patch is doing.

What it does is bumping the GPIO pin offset in the Linux global
GPIO number space with 32 for each new controller.

> +       static int bank_base;
>
>         pdata = davinci_gpio_get_pdata(pdev);
>         if (!pdata) {
> @@ -226,7 +227,8 @@ static int davinci_gpio_probe(struct platform_device *pdev)
>                 chips[i].chip.direction_output = davinci_direction_out;
>                 chips[i].chip.set = davinci_gpio_set;
>
> -               chips[i].chip.base = base;
> +               chips[i].chip.base = bank_base;
> +               bank_base += 32;

Why can you not rewrite the driver to pass -1 as base and
get a dynamic allocation of GPIO numbers instead? Then
you won't have this hairy problem.

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/5] gpio: davinci: Add a separate compatible for k2g
       [not found]     ` <1476855239-32730-6-git-send-email-j-keerthy-l0cyMroinI0@public.gmane.org>
@ 2016-10-26 21:08       ` Rob Herring
  2016-10-27  3:28         ` Keerthy
  0 siblings, 1 reply; 17+ messages in thread
From: Rob Herring @ 2016-10-26 21:08 UTC (permalink / raw)
  To: Keerthy
  Cc: linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w, lokeshvutla-l0cyMroinI0,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA, rogerq-l0cyMroinI0,
	grygorii.strashko-l0cyMroinI0

On Wed, Oct 19, 2016 at 11:03:59AM +0530, Keerthy wrote:
> In the case of k2g the clocks are handled differently as when compared
> with other keystones. Hence adding a separate compatible and match tables
> accordingly.
> 
> Signed-off-by: Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org>
> ---
>  .../devicetree/bindings/gpio/gpio-davinci.txt      |  2 +-
>  drivers/gpio/gpio-davinci.c                        | 45 ++++++++++++++++++++--
>  2 files changed, 42 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
> index 5079ba7..a76abd2 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
> @@ -1,7 +1,7 @@
>  Davinci/Keystone GPIO controller bindings
>  
>  Required Properties:
> -- compatible: should be "ti,dm6441-gpio", "ti,keystone-gpio"
> +- compatible: should be "ti,dm6441-gpio", "ti,keystone-gpio", "ti,k2g-gpio"

Is this "one of" or all? Seems line the former in which case please 
reformat to one per line.

>  
>  - reg: Physical base address of the controller and the size of memory mapped
>         registers.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/5] gpio: davinci: Add a separate compatible for k2g
  2016-10-26 21:08       ` Rob Herring
@ 2016-10-27  3:28         ` Keerthy
  0 siblings, 0 replies; 17+ messages in thread
From: Keerthy @ 2016-10-27  3:28 UTC (permalink / raw)
  To: Rob Herring
  Cc: linus.walleij, gnurou, lokeshvutla, linux-gpio, devicetree,
	linux-omap, rogerq, grygorii.strashko



On Thursday 27 October 2016 02:38 AM, Rob Herring wrote:
> On Wed, Oct 19, 2016 at 11:03:59AM +0530, Keerthy wrote:
>> In the case of k2g the clocks are handled differently as when compared
>> with other keystones. Hence adding a separate compatible and match tables
>> accordingly.
>>
>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>> ---
>>  .../devicetree/bindings/gpio/gpio-davinci.txt      |  2 +-
>>  drivers/gpio/gpio-davinci.c                        | 45 ++++++++++++++++++++--
>>  2 files changed, 42 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>> index 5079ba7..a76abd2 100644
>> --- a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>> @@ -1,7 +1,7 @@
>>  Davinci/Keystone GPIO controller bindings
>>
>>  Required Properties:
>> -- compatible: should be "ti,dm6441-gpio", "ti,keystone-gpio"
>> +- compatible: should be "ti,dm6441-gpio", "ti,keystone-gpio", "ti,k2g-gpio"
>
> Is this "one of" or all? Seems line the former in which case please
> reformat to one per line.

It is One of. I will reformat.

>
>>
>>  - reg: Physical base address of the controller and the size of memory mapped
>>         registers.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/5] drivers: gpio: Add support for multiple IPs
       [not found]         ` <CACRpkdbsgM-yiWpiA5G35jQR8v3FO=Me+4rhe6TzE0qsWOJ8Ew-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2016-10-27  3:42           ` Keerthy
  2016-10-27  7:53             ` Roger Quadros
  0 siblings, 1 reply; 17+ messages in thread
From: Keerthy @ 2016-10-27  3:42 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Alexandre Courbot, Lokesh Vutla, Rob Herring,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux-OMAP, Roger Quadros,
	Grygorii Strashko



On Sunday 23 October 2016 04:02 PM, Linus Walleij wrote:
> On Wed, Oct 19, 2016 at 7:33 AM, Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org> wrote:
>
>> From: Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org>
>>
>> Update GPIO driver to support Multiple GPIO IPs.
>>
>> Signed-off-by: Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org>
>> Signed-off-by: Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org>
>
> This commit message is not at all describing what the patch is doing.
>
> What it does is bumping the GPIO pin offset in the Linux global
> GPIO number space with 32 for each new controller.
>
>> +       static int bank_base;
>>
>>         pdata = davinci_gpio_get_pdata(pdev);
>>         if (!pdata) {
>> @@ -226,7 +227,8 @@ static int davinci_gpio_probe(struct platform_device *pdev)
>>                 chips[i].chip.direction_output = davinci_direction_out;
>>                 chips[i].chip.set = davinci_gpio_set;
>>
>> -               chips[i].chip.base = base;
>> +               chips[i].chip.base = bank_base;
>> +               bank_base += 32;
>
> Why can you not rewrite the driver to pass -1 as base and
> get a dynamic allocation of GPIO numbers instead? Then
> you won't have this hairy problem.

Ok i will try that.

In case of k2g. There are 2 big GPIO modules GPIO0 and GPIO1.
GPIO0 comprises of 144 GPIOs
and GPIO1 has about 68 GPIOs. Wanted feedback from you on how this is 
being modeled.

I am creating a controller for every 32 GPIOs under the big module each 
containing a gpio_chip. Each 32 GPIOs chip has 2 banks of 16 GPIOs each.
Each 16 GPIO bank has an interrupt.

Is this modeling fine or do you think creating one chip with 144 pins 
and another with 68 pins is a better way?


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/5] drivers: gpio: Add support for multiple IPs
  2016-10-27  3:42           ` Keerthy
@ 2016-10-27  7:53             ` Roger Quadros
  2016-10-27  8:07               ` Keerthy
  0 siblings, 1 reply; 17+ messages in thread
From: Roger Quadros @ 2016-10-27  7:53 UTC (permalink / raw)
  To: Keerthy, Linus Walleij
  Cc: Alexandre Courbot, Lokesh Vutla, Rob Herring, linux-gpio,
	devicetree, Linux-OMAP, Grygorii Strashko

Keerthy,

On 27/10/16 06:42, Keerthy wrote:
> 
> 
> On Sunday 23 October 2016 04:02 PM, Linus Walleij wrote:
>> On Wed, Oct 19, 2016 at 7:33 AM, Keerthy <j-keerthy@ti.com> wrote:
>>
>>> From: Lokesh Vutla <lokeshvutla@ti.com>
>>>
>>> Update GPIO driver to support Multiple GPIO IPs.
>>>
>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>>
>> This commit message is not at all describing what the patch is doing.
>>
>> What it does is bumping the GPIO pin offset in the Linux global
>> GPIO number space with 32 for each new controller.
>>
>>> +       static int bank_base;
>>>
>>>         pdata = davinci_gpio_get_pdata(pdev);
>>>         if (!pdata) {
>>> @@ -226,7 +227,8 @@ static int davinci_gpio_probe(struct platform_device *pdev)
>>>                 chips[i].chip.direction_output = davinci_direction_out;
>>>                 chips[i].chip.set = davinci_gpio_set;
>>>
>>> -               chips[i].chip.base = base;
>>> +               chips[i].chip.base = bank_base;
>>> +               bank_base += 32;
>>
>> Why can you not rewrite the driver to pass -1 as base and
>> get a dynamic allocation of GPIO numbers instead? Then
>> you won't have this hairy problem.
> 
> Ok i will try that.
> 
> In case of k2g. There are 2 big GPIO modules GPIO0 and GPIO1.
> GPIO0 comprises of 144 GPIOs
> and GPIO1 has about 68 GPIOs. Wanted feedback from you on how this is being modeled.
> 
> I am creating a controller for every 32 GPIOs under the big module each containing a gpio_chip. Each 32 GPIOs chip has 2 banks of 16 GPIOs each.
> Each 16 GPIO bank has an interrupt.
> 
> Is this modeling fine or do you think creating one chip with 144 pins and another with 68 pins is a better way?

If GPIO0 has 144 GPIOs, why don't we model it as a gpiochip with 144 GPIOs?
What is the benefit of partitioning it into gpiochips of 32 GPIOs each?

cheers,
-roger

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/5] drivers: gpio: Add support for multiple IPs
  2016-10-27  7:53             ` Roger Quadros
@ 2016-10-27  8:07               ` Keerthy
       [not found]                 ` <c047fb27-a8bb-c10d-2b16-a3e6bf45d88f-l0cyMroinI0@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Keerthy @ 2016-10-27  8:07 UTC (permalink / raw)
  To: Roger Quadros, Linus Walleij, Grygorii Strashko
  Cc: Alexandre Courbot, Lokesh Vutla, Rob Herring, linux-gpio,
	devicetree, Linux-OMAP



On Thursday 27 October 2016 01:23 PM, Roger Quadros wrote:
> Keerthy,
>
> On 27/10/16 06:42, Keerthy wrote:
>>
>>
>> On Sunday 23 October 2016 04:02 PM, Linus Walleij wrote:
>>> On Wed, Oct 19, 2016 at 7:33 AM, Keerthy <j-keerthy@ti.com> wrote:
>>>
>>>> From: Lokesh Vutla <lokeshvutla@ti.com>
>>>>
>>>> Update GPIO driver to support Multiple GPIO IPs.
>>>>
>>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>>>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>>>
>>> This commit message is not at all describing what the patch is doing.
>>>
>>> What it does is bumping the GPIO pin offset in the Linux global
>>> GPIO number space with 32 for each new controller.
>>>
>>>> +       static int bank_base;
>>>>
>>>>         pdata = davinci_gpio_get_pdata(pdev);
>>>>         if (!pdata) {
>>>> @@ -226,7 +227,8 @@ static int davinci_gpio_probe(struct platform_device *pdev)
>>>>                 chips[i].chip.direction_output = davinci_direction_out;
>>>>                 chips[i].chip.set = davinci_gpio_set;
>>>>
>>>> -               chips[i].chip.base = base;
>>>> +               chips[i].chip.base = bank_base;
>>>> +               bank_base += 32;
>>>
>>> Why can you not rewrite the driver to pass -1 as base and
>>> get a dynamic allocation of GPIO numbers instead? Then
>>> you won't have this hairy problem.
>>
>> Ok i will try that.
>>
>> In case of k2g. There are 2 big GPIO modules GPIO0 and GPIO1.
>> GPIO0 comprises of 144 GPIOs
>> and GPIO1 has about 68 GPIOs. Wanted feedback from you on how this is being modeled.
>>
>> I am creating a controller for every 32 GPIOs under the big module each containing a gpio_chip. Each 32 GPIOs chip has 2 banks of 16 GPIOs each.
>> Each 16 GPIO bank has an interrupt.
>>
>> Is this modeling fine or do you think creating one chip with 144 pins and another with 68 pins is a better way?
>
> If GPIO0 has 144 GPIOs, why don't we model it as a gpiochip with 144 GPIOs?
> What is the benefit of partitioning it into gpiochips of 32 GPIOs each?

144 GPIOs where in 16 GPIOs form a bank. So about 9 banks with one 
interrupt each. So split it into gpiochips with 32 GPIOs each handling 2 
Interrupts.

Grygorii,

Any strong reason that you recollect of so as to why this modeling was 
chosen?

>
> cheers,
> -roger
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/5] drivers: gpio: Add support for multiple IPs
       [not found]                 ` <c047fb27-a8bb-c10d-2b16-a3e6bf45d88f-l0cyMroinI0@public.gmane.org>
@ 2016-10-31 14:58                   ` Grygorii Strashko
  2016-11-04 14:28                   ` Linus Walleij
  1 sibling, 0 replies; 17+ messages in thread
From: Grygorii Strashko @ 2016-10-31 14:58 UTC (permalink / raw)
  To: Keerthy, Roger Quadros, Linus Walleij
  Cc: Alexandre Courbot, Lokesh Vutla, Rob Herring,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux-OMAP



On 10/27/2016 03:07 AM, Keerthy wrote:
> 
> 
> On Thursday 27 October 2016 01:23 PM, Roger Quadros wrote:
>> Keerthy,
>>
>> On 27/10/16 06:42, Keerthy wrote:
>>>
>>>
>>> On Sunday 23 October 2016 04:02 PM, Linus Walleij wrote:
>>>> On Wed, Oct 19, 2016 at 7:33 AM, Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org> wrote:
>>>>
>>>>> From: Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org>
>>>>>
>>>>> Update GPIO driver to support Multiple GPIO IPs.
>>>>>
>>>>> Signed-off-by: Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org>
>>>>> Signed-off-by: Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org>
>>>>
>>>> This commit message is not at all describing what the patch is doing.
>>>>
>>>> What it does is bumping the GPIO pin offset in the Linux global
>>>> GPIO number space with 32 for each new controller.
>>>>
>>>>> +       static int bank_base;
>>>>>
>>>>>         pdata = davinci_gpio_get_pdata(pdev);
>>>>>         if (!pdata) {
>>>>> @@ -226,7 +227,8 @@ static int davinci_gpio_probe(struct
>>>>> platform_device *pdev)
>>>>>                 chips[i].chip.direction_output =
>>>>> davinci_direction_out;
>>>>>                 chips[i].chip.set = davinci_gpio_set;
>>>>>
>>>>> -               chips[i].chip.base = base;
>>>>> +               chips[i].chip.base = bank_base;
>>>>> +               bank_base += 32;
>>>>
>>>> Why can you not rewrite the driver to pass -1 as base and
>>>> get a dynamic allocation of GPIO numbers instead? Then
>>>> you won't have this hairy problem.
>>>
>>> Ok i will try that.
>>>
>>> In case of k2g. There are 2 big GPIO modules GPIO0 and GPIO1.
>>> GPIO0 comprises of 144 GPIOs
>>> and GPIO1 has about 68 GPIOs. Wanted feedback from you on how this is
>>> being modeled.
>>>
>>> I am creating a controller for every 32 GPIOs under the big module
>>> each containing a gpio_chip. Each 32 GPIOs chip has 2 banks of 16
>>> GPIOs each.
>>> Each 16 GPIO bank has an interrupt.
>>>
>>> Is this modeling fine or do you think creating one chip with 144 pins
>>> and another with 68 pins is a better way?
>>
>> If GPIO0 has 144 GPIOs, why don't we model it as a gpiochip with 144
>> GPIOs?
>> What is the benefit of partitioning it into gpiochips of 32 GPIOs each?
> 
> 144 GPIOs where in 16 GPIOs form a bank. So about 9 banks with one
> interrupt each. So split it into gpiochips with 32 GPIOs each handling 2
> Interrupts.
> 
> Grygorii,
> 
> Any strong reason that you recollect of so as to why this modeling was
> chosen?
> 

I think, there was a restriction on max number of GPIOs supported by one gpiochip
(32) at time when this driver was introduced an updated for Keystone.
But, seems, this might work now since GPIO core was transformed to use gpio descriptors.

regards,
-grygorii

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/5] drivers: gpio: Add support for multiple IPs
       [not found]                 ` <c047fb27-a8bb-c10d-2b16-a3e6bf45d88f-l0cyMroinI0@public.gmane.org>
  2016-10-31 14:58                   ` Grygorii Strashko
@ 2016-11-04 14:28                   ` Linus Walleij
       [not found]                     ` <CACRpkdZmuc__QL6b7Jvb8xL5Bm+PD2X60NtiugrnH7iAO=-tLQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  1 sibling, 1 reply; 17+ messages in thread
From: Linus Walleij @ 2016-11-04 14:28 UTC (permalink / raw)
  To: Keerthy
  Cc: Roger Quadros, Grygorii Strashko, Alexandre Courbot,
	Lokesh Vutla, Rob Herring, linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux-OMAP

On Thu, Oct 27, 2016 at 10:07 AM, Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org> wrote:
> On Thursday 27 October 2016 01:23 PM, Roger Quadros wrote:
>> On 27/10/16 06:42, Keerthy wrote:
>>> On Sunday 23 October 2016 04:02 PM, Linus Walleij wrote:
>>>> On Wed, Oct 19, 2016 at 7:33 AM, Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org> wrote:
>>>>> From: Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org>

>>> In case of k2g. There are 2 big GPIO modules GPIO0 and GPIO1.
>>> GPIO0 comprises of 144 GPIOs
>>> and GPIO1 has about 68 GPIOs. Wanted feedback from you on how this is
>>> being modeled.
>>>
>>> I am creating a controller for every 32 GPIOs under the big module each
>>> containing a gpio_chip. Each 32 GPIOs chip has 2 banks of 16 GPIOs each.
>>> Each 16 GPIO bank has an interrupt.
>>>
>>> Is this modeling fine or do you think creating one chip with 144 pins and
>>> another with 68 pins is a better way?
>>
>>
>> If GPIO0 has 144 GPIOs, why don't we model it as a gpiochip with 144
>> GPIOs?
>> What is the benefit of partitioning it into gpiochips of 32 GPIOs each?
>
> 144 GPIOs where in 16 GPIOs form a bank. So about 9 banks with one interrupt
> each. So split it into gpiochips with 32 GPIOs each handling 2 Interrupts.

I'm a bit confused.

This sounds like you should either have one gpio_chip per interrupt
(if that fits with how the device tree looks) or one big gpio_chip for
all the lines.

The DT model sort of mandates how the interrupts should be mapped
at this point, and as far as I can tell from the binding the example looks
like so:

gpio: gpio@1e26000 {
        compatible = "ti,dm6441-gpio";
        gpio-controller;
        #gpio-cells = <2>;
        reg = <0x226000 0x1000>;
        interrupt-parent = <&intc>;
        interrupts = <42 IRQ_TYPE_EDGE_BOTH 43 IRQ_TYPE_EDGE_BOTH
                44 IRQ_TYPE_EDGE_BOTH 45 IRQ_TYPE_EDGE_BOTH
                46 IRQ_TYPE_EDGE_BOTH 47 IRQ_TYPE_EDGE_BOTH
                48 IRQ_TYPE_EDGE_BOTH 49 IRQ_TYPE_EDGE_BOTH
                50 IRQ_TYPE_EDGE_BOTH>;
        ti,ngpio = <144>;
        ti,davinci-gpio-unbanked = <0>;
        interrupt-controller;
        #interrupt-cells = <2>;
};

So I don't see any reason to split this up in subchips internally in
Linux?

It looks like the irqdomain will be a bit tricksy though.

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/5] drivers: gpio: Add support for multiple IPs
       [not found]                     ` <CACRpkdZmuc__QL6b7Jvb8xL5Bm+PD2X60NtiugrnH7iAO=-tLQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2016-11-04 19:59                       ` Grygorii Strashko
       [not found]                         ` <b92f729b-5897-c29a-31ec-97510776d85d-l0cyMroinI0@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Grygorii Strashko @ 2016-11-04 19:59 UTC (permalink / raw)
  To: Linus Walleij, Keerthy
  Cc: Roger Quadros, Alexandre Courbot, Lokesh Vutla, Rob Herring,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux-OMAP



On 11/04/2016 09:28 AM, Linus Walleij wrote:
> On Thu, Oct 27, 2016 at 10:07 AM, Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org> wrote:
>> On Thursday 27 October 2016 01:23 PM, Roger Quadros wrote:
>>> On 27/10/16 06:42, Keerthy wrote:
>>>> On Sunday 23 October 2016 04:02 PM, Linus Walleij wrote:
>>>>> On Wed, Oct 19, 2016 at 7:33 AM, Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org> wrote:
>>>>>> From: Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org>
> 
>>>> In case of k2g. There are 2 big GPIO modules GPIO0 and GPIO1.
>>>> GPIO0 comprises of 144 GPIOs
>>>> and GPIO1 has about 68 GPIOs. Wanted feedback from you on how this is
>>>> being modeled.
>>>>
>>>> I am creating a controller for every 32 GPIOs under the big module each
>>>> containing a gpio_chip. Each 32 GPIOs chip has 2 banks of 16 GPIOs each.
>>>> Each 16 GPIO bank has an interrupt.
>>>>
>>>> Is this modeling fine or do you think creating one chip with 144 pins and
>>>> another with 68 pins is a better way?
>>>
>>>
>>> If GPIO0 has 144 GPIOs, why don't we model it as a gpiochip with 144
>>> GPIOs?
>>> What is the benefit of partitioning it into gpiochips of 32 GPIOs each?
>>
>> 144 GPIOs where in 16 GPIOs form a bank. So about 9 banks with one interrupt
>> each. So split it into gpiochips with 32 GPIOs each handling 2 Interrupts.
> 
> I'm a bit confused.
> 
> This sounds like you should either have one gpio_chip per interrupt
> (if that fits with how the device tree looks) or one big gpio_chip for
> all the lines.

yep. This HW confuses a bit :(, So, there are few links on TRMs/DM and
my brief overview:
Keystone k2g (66AK2G02) http://www.ti.com/lit/ug/spruhy8d/spruhy8d.pdf
Keystone k2hk/e/l http://www.ti.com/lit/ug/sprugv1/sprugv1.pdf
omap-l138 http://www.ti.com/lit/ug/spruh77c/spruh77c.pdf

Each GPIO IP is implemented as GPIO controller which has some set of
common registers (minimum BINTEN) and up to 16 (?) gpio banks - 16 pins per bank.
SoC may contain more than one GPIO controllers.  

GPIO banks groped by two in 32 bit registers, so overall registers structure:

common		0h GPIO_PID Peripheral Identification Register
common		8h GPIO_BINTEN Interrupt Per-Bank Enable Register
		
banks0&1	10h GPIO_DIR01 Direction 0 and 1 Register
		14h GPIO_OUT_DATA01 Output Data 0 and 1 Register
		18h GPIO_SET_DATA01 Set Data 0 and 1 Register
		1Ch GPIO_CLR_DATA01 Clear Data 0 and 1 Register
		
		20h GPIO_IN_DATA01 Input Data 0 and 1
		24h GPIO_SET_RIS_TRIG01 Set Rising Edge Interrupt 0 and 1
		28h GPIO_CLR_RIS_TRIG01 Clear Rising Edge Interrupt 0 and 1
		2Ch GPIO_SET_FAL_TRIG01 Set Falling Edge Interrupt 0 and 1
		30h GPIO_CLR_FAL_TRIG01 Clear Falling Edge Interrupt 0 and 1
		34h GPIO_INTSTAT01 GPIO Interrupt status 0 and 1 Register
		
banks2&3	38h DIR23 GPIO Banks 2 and 3 Direction Register
...

IRQ handling can be done in two ways - depending on SoC - which can be combined
on some SoCs (not supported by current driver)
1) Direct IRQs - each GPIO pin has separate IRQ line assigned in Main IRQ controller (example k2hk/e/l)
2) banked IRQs - each bank (16 pins) has one assigned IRQ. So, two IRQs per banksX&Y register set.

> 
> The DT model sort of mandates how the interrupts should be mapped
> at this point, and as far as I can tell from the binding the example looks
> like so:
> 
> gpio: gpio@1e26000 {
>         compatible = "ti,dm6441-gpio";
>         gpio-controller;
>         #gpio-cells = <2>;
>         reg = <0x226000 0x1000>;
>         interrupt-parent = <&intc>;
>         interrupts = <42 IRQ_TYPE_EDGE_BOTH 43 IRQ_TYPE_EDGE_BOTH
>                 44 IRQ_TYPE_EDGE_BOTH 45 IRQ_TYPE_EDGE_BOTH
>                 46 IRQ_TYPE_EDGE_BOTH 47 IRQ_TYPE_EDGE_BOTH
>                 48 IRQ_TYPE_EDGE_BOTH 49 IRQ_TYPE_EDGE_BOTH
>                 50 IRQ_TYPE_EDGE_BOTH>;
>         ti,ngpio = <144>;
>         ti,davinci-gpio-unbanked = <0>;
>         interrupt-controller;
>         #interrupt-cells = <2>;
> };

Above, DT bindings models Davinci GPIO IP as monolithic GPIO controller
with N gpio pins, but internally separate GPIO chips are created for each
banksX&Y register set (32 pins, 2 banked irq -or- 32 direct irqs).

Translation from linear GPIO numbering to the proper internal GPIO chip is done
using chip.of_xlate().

> 
> So I don't see any reason to split this up in subchips internally in
> Linux?
> 



-- 
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/5] drivers: gpio: Add support for multiple IPs
       [not found]                         ` <b92f729b-5897-c29a-31ec-97510776d85d-l0cyMroinI0@public.gmane.org>
@ 2016-11-05  8:23                           ` Linus Walleij
       [not found]                             ` <CACRpkdac_UgM=xeXkSZ+rOvZvutt40USjCoEt5-SoFkqspuExw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Linus Walleij @ 2016-11-05  8:23 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: Keerthy, Roger Quadros, Alexandre Courbot, Lokesh Vutla,
	Rob Herring, linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux-OMAP

On Fri, Nov 4, 2016 at 8:59 PM, Grygorii Strashko
<grygorii.strashko-l0cyMroinI0@public.gmane.org> wrote:
> On 11/04/2016 09:28 AM, Linus Walleij wrote:

>> The DT model sort of mandates how the interrupts should be mapped
>> at this point, and as far as I can tell from the binding the example looks
>> like so:
>>
>> gpio: gpio@1e26000 {
>>         compatible = "ti,dm6441-gpio";
>>         gpio-controller;
>>         #gpio-cells = <2>;
>>         reg = <0x226000 0x1000>;
>>         interrupt-parent = <&intc>;
>>         interrupts = <42 IRQ_TYPE_EDGE_BOTH 43 IRQ_TYPE_EDGE_BOTH
>>                 44 IRQ_TYPE_EDGE_BOTH 45 IRQ_TYPE_EDGE_BOTH
>>                 46 IRQ_TYPE_EDGE_BOTH 47 IRQ_TYPE_EDGE_BOTH
>>                 48 IRQ_TYPE_EDGE_BOTH 49 IRQ_TYPE_EDGE_BOTH
>>                 50 IRQ_TYPE_EDGE_BOTH>;
>>         ti,ngpio = <144>;
>>         ti,davinci-gpio-unbanked = <0>;
>>         interrupt-controller;
>>         #interrupt-cells = <2>;
>> };
>
> Above, DT bindings models Davinci GPIO IP as monolithic GPIO controller
> with N gpio pins, but internally separate GPIO chips are created for each
> banksX&Y register set (32 pins, 2 banked irq -or- 32 direct irqs).

Hm it would be good to get away from that and just have one big gpio
chip.

> Translation from linear GPIO numbering to the proper internal GPIO chip is done
> using chip.of_xlate().

Yeah :/ this could be made simpler with a single chip just spanning all
the banks and the common registers I think.

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/5] drivers: gpio: Add support for multiple IPs
       [not found]                             ` <CACRpkdac_UgM=xeXkSZ+rOvZvutt40USjCoEt5-SoFkqspuExw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2016-11-07  3:59                               ` Keerthy
  0 siblings, 0 replies; 17+ messages in thread
From: Keerthy @ 2016-11-07  3:59 UTC (permalink / raw)
  To: Linus Walleij, Grygorii Strashko
  Cc: Roger Quadros, Alexandre Courbot, Lokesh Vutla, Rob Herring,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux-OMAP



On Saturday 05 November 2016 01:53 PM, Linus Walleij wrote:
> On Fri, Nov 4, 2016 at 8:59 PM, Grygorii Strashko
> <grygorii.strashko-l0cyMroinI0@public.gmane.org> wrote:
>> On 11/04/2016 09:28 AM, Linus Walleij wrote:
>
>>> The DT model sort of mandates how the interrupts should be mapped
>>> at this point, and as far as I can tell from the binding the example looks
>>> like so:
>>>
>>> gpio: gpio@1e26000 {
>>>         compatible = "ti,dm6441-gpio";
>>>         gpio-controller;
>>>         #gpio-cells = <2>;
>>>         reg = <0x226000 0x1000>;
>>>         interrupt-parent = <&intc>;
>>>         interrupts = <42 IRQ_TYPE_EDGE_BOTH 43 IRQ_TYPE_EDGE_BOTH
>>>                 44 IRQ_TYPE_EDGE_BOTH 45 IRQ_TYPE_EDGE_BOTH
>>>                 46 IRQ_TYPE_EDGE_BOTH 47 IRQ_TYPE_EDGE_BOTH
>>>                 48 IRQ_TYPE_EDGE_BOTH 49 IRQ_TYPE_EDGE_BOTH
>>>                 50 IRQ_TYPE_EDGE_BOTH>;
>>>         ti,ngpio = <144>;
>>>         ti,davinci-gpio-unbanked = <0>;
>>>         interrupt-controller;
>>>         #interrupt-cells = <2>;
>>> };
>>
>> Above, DT bindings models Davinci GPIO IP as monolithic GPIO controller
>> with N gpio pins, but internally separate GPIO chips are created for each
>> banksX&Y register set (32 pins, 2 banked irq -or- 32 direct irqs).
>
> Hm it would be good to get away from that and just have one big gpio
> chip.
>
>> Translation from linear GPIO numbering to the proper internal GPIO chip is done
>> using chip.of_xlate().
>
> Yeah :/ this could be made simpler with a single chip just spanning all
> the banks and the common registers I think.

Okay Linus. Thanks for the direction.

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2016-11-07  3:59 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-19  5:33 [PATCH 0/5] gpio: davinci: Add k2g support Keerthy
2016-10-19  5:33 ` [PATCH 1/5] gpio: davinci: Remove gpio2regs function to accommodate multi instances Keerthy
2016-10-19  5:33 ` [PATCH 4/5] gpio: davinci: Store both irqs into the controller Keerthy
     [not found] ` <1476855239-32730-1-git-send-email-j-keerthy-l0cyMroinI0@public.gmane.org>
2016-10-19  5:33   ` [PATCH 2/5] drivers: gpio: Add support for multiple IPs Keerthy
     [not found]     ` <1476855239-32730-3-git-send-email-j-keerthy-l0cyMroinI0@public.gmane.org>
2016-10-23 10:32       ` Linus Walleij
     [not found]         ` <CACRpkdbsgM-yiWpiA5G35jQR8v3FO=Me+4rhe6TzE0qsWOJ8Ew-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-10-27  3:42           ` Keerthy
2016-10-27  7:53             ` Roger Quadros
2016-10-27  8:07               ` Keerthy
     [not found]                 ` <c047fb27-a8bb-c10d-2b16-a3e6bf45d88f-l0cyMroinI0@public.gmane.org>
2016-10-31 14:58                   ` Grygorii Strashko
2016-11-04 14:28                   ` Linus Walleij
     [not found]                     ` <CACRpkdZmuc__QL6b7Jvb8xL5Bm+PD2X60NtiugrnH7iAO=-tLQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-11-04 19:59                       ` Grygorii Strashko
     [not found]                         ` <b92f729b-5897-c29a-31ec-97510776d85d-l0cyMroinI0@public.gmane.org>
2016-11-05  8:23                           ` Linus Walleij
     [not found]                             ` <CACRpkdac_UgM=xeXkSZ+rOvZvutt40USjCoEt5-SoFkqspuExw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-11-07  3:59                               ` Keerthy
2016-10-19  5:33   ` [PATCH 3/5] gpio: davinci: irq support for multiple gpio controllers Keerthy
2016-10-19  5:33   ` [PATCH 5/5] gpio: davinci: Add a separate compatible for k2g Keerthy
     [not found]     ` <1476855239-32730-6-git-send-email-j-keerthy-l0cyMroinI0@public.gmane.org>
2016-10-26 21:08       ` Rob Herring
2016-10-27  3:28         ` Keerthy

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.