Message ID | 1483518389-6200-2-git-send-email-j-keerthy@ti.com |
---|---|
State | New |
Headers | show |
On Wed, Jan 4, 2017 at 9:26 AM, Keerthy <j-keerthy@ti.com> wrote: > 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> > - regs = gpio2regs(base); > + regs = gpio_base + offset_array[i]; I understand this. > - 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; And this, if each instans has 32 GPIOs. > - g = gpio2regs(0); > + g = chips[0].regs; Also makes sense. > - g = gpio2regs(gpio); > + g = chips[bank / 2].regs; But what is this? I don't understand that /2 at all. Please insert a comment explaining it so I can figure it out. Are there two banks per instance? Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 11 January 2017 04:34 PM, Linus Walleij wrote: > On Wed, Jan 4, 2017 at 9:26 AM, Keerthy <j-keerthy@ti.com> wrote: > >> 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> > >> - regs = gpio2regs(base); >> + regs = gpio_base + offset_array[i]; > > I understand this. > >> - 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; > > And this, if each instans has 32 GPIOs. > >> - g = gpio2regs(0); >> + g = chips[0].regs; > > Also makes sense. > >> - g = gpio2regs(gpio); >> + g = chips[bank / 2].regs; > > But what is this? I don't understand that /2 at all. Please insert a > comment explaining it so I can figure it out. Are there two banks > per instance? Yes! There are register sets for 32 GPIOs. 2 banks of 16 GPIOs are covered by each set of registers hence /2. Shall i add a comment and send this patch alone separately? > > Yours, > Linus Walleij > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 11, 2017 at 1:40 PM, Keerthy <j-keerthy@ti.com> wrote: > On Wednesday 11 January 2017 04:34 PM, Linus Walleij wrote: >> On Wed, Jan 4, 2017 at 9:26 AM, Keerthy <j-keerthy@ti.com> wrote: >> >>> 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> >> >> >>> - regs = gpio2regs(base); >>> + regs = gpio_base + offset_array[i]; >> >> >> I understand this. >> >>> - 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; >> >> >> And this, if each instans has 32 GPIOs. >> >>> - g = gpio2regs(0); >>> + g = chips[0].regs; >> >> >> Also makes sense. >> >>> - g = gpio2regs(gpio); >>> + g = chips[bank / 2].regs; >> >> >> But what is this? I don't understand that /2 at all. Please insert a >> comment explaining it so I can figure it out. Are there two banks >> per instance? > > > Yes! There are register sets for 32 GPIOs. 2 banks of 16 GPIOs are covered > by each set of registers hence /2. Shall i add a comment and send this patch > alone separately? Yeah. I am currently confused by several patch sets for davinci in my inbox, can you rebase on my devel branch when I push it and resend *all* you have cooking for DaVinci? Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 11 January 2017 09:23 PM, Linus Walleij wrote: > On Wed, Jan 11, 2017 at 1:40 PM, Keerthy <j-keerthy@ti.com> wrote: >> On Wednesday 11 January 2017 04:34 PM, Linus Walleij wrote: >>> On Wed, Jan 4, 2017 at 9:26 AM, Keerthy <j-keerthy@ti.com> wrote: >>> >>>> 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> >>> >>> >>>> - regs = gpio2regs(base); >>>> + regs = gpio_base + offset_array[i]; >>> >>> >>> I understand this. >>> >>>> - 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; >>> >>> >>> And this, if each instans has 32 GPIOs. >>> >>>> - g = gpio2regs(0); >>>> + g = chips[0].regs; >>> >>> >>> Also makes sense. >>> >>>> - g = gpio2regs(gpio); >>>> + g = chips[bank / 2].regs; >>> >>> >>> But what is this? I don't understand that /2 at all. Please insert a >>> comment explaining it so I can figure it out. Are there two banks >>> per instance? >> >> >> Yes! There are register sets for 32 GPIOs. 2 banks of 16 GPIOs are covered >> by each set of registers hence /2. Shall i add a comment and send this patch >> alone separately? > > Yeah. > > I am currently confused by several patch sets for davinci in my inbox, > can you rebase on my devel branch when I push it and resend > *all* you have cooking for DaVinci? Sure Linus i will do that. Thanks, Keerthy > > Yours, > Linus Walleij > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 11 January 2017 09:23 PM, Linus Walleij wrote: > On Wed, Jan 11, 2017 at 1:40 PM, Keerthy <j-keerthy@ti.com> wrote: >> On Wednesday 11 January 2017 04:34 PM, Linus Walleij wrote: >>> On Wed, Jan 4, 2017 at 9:26 AM, Keerthy <j-keerthy@ti.com> wrote: >>> >>>> 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> >>> >>> >>>> - regs = gpio2regs(base); >>>> + regs = gpio_base + offset_array[i]; >>> >>> >>> I understand this. >>> >>>> - 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; >>> >>> >>> And this, if each instans has 32 GPIOs. >>> >>>> - g = gpio2regs(0); >>>> + g = chips[0].regs; >>> >>> >>> Also makes sense. >>> >>>> - g = gpio2regs(gpio); >>>> + g = chips[bank / 2].regs; >>> >>> >>> But what is this? I don't understand that /2 at all. Please insert a >>> comment explaining it so I can figure it out. Are there two banks >>> per instance? >> >> >> Yes! There are register sets for 32 GPIOs. 2 banks of 16 GPIOs are covered >> by each set of registers hence /2. Shall i add a comment and send this patch >> alone separately? > > Yeah. > > I am currently confused by several patch sets for davinci in my inbox, > can you rebase on my devel branch when I push it and resend > *all* you have cooking for DaVinci? I am assuming that Patch 1 from this is applied. I will club this one along with my latest series and send a new v2 series so that all are in one place. Thanks, Keerthy > > Yours, > Linus Walleij > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c index 163f81e..26b874a 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); @@ -574,7 +558,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);
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(-)