From mboxrd@z Thu Jan 1 00:00:00 1970 From: Javier Martinez Canillas Subject: Re: [PATCH 3/5] gpio/omap: Add DT support to GPIO driver Date: Mon, 15 Apr 2013 13:25:38 +0200 Message-ID: References: <1329321854-24490-1-git-send-email-b-cousson@ti.com> <4F47AD08.4030504@ti.com> <512D39DA.7020306@ti.com> <512D3AB1.1080202@wwwdotorg.org> <512D3EC2.6050408@ti.com> <20130302200524.D230F3E1571@localhost> <51391F41.5000303@ti.com> <514C79E1.4090106@wwwdotorg.org> <514CE0AB.6060207@ti.com> <515319D5.20105@wwwdotorg.org> <5155C902.7080207@wwwdotorg.org> <5165CB9D.1090202@wwwdotorg.org> <51671D7B.5060303@wwwdotorg.org> <51673D70.3010503@wwwdotorg.org> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: In-Reply-To: Sender: linux-omap-owner@vger.kernel.org To: Linus Walleij Cc: Stephen Warren , Jon Hunter , Grant Likely , Alexandre Courbot , Stephen Warren , Kevin Hilman , "devicetree-discuss@lists.ozlabs.org" , "linux-omap@vger.kernel.org" , Tarun Kanti DebBarma , "linux-arm-kernel@lists.infradead.org" List-Id: devicetree@vger.kernel.org On Sun, Apr 14, 2013 at 10:53 PM, Linus Walleij wrote: > On Sun, Apr 14, 2013 at 3:35 AM, Javier Martinez Canillas > wrote: > >> Is the following inlined patch [1] what you were thinking that would >> be the right approach? Hi Linus, thanks a lot for your feedback. > > This looks sort of OK, but I'm still struggling with the question of > what we could do to help other implementations facing the same issue. > Yes, I don't know how we can make it easier to other implementations (besides adding the irq_request hook to irq_chip) since the logic is GPIO controller specific. For example I took a look to drivers/gpio/gpio-tegra.c and if I understood correctly, the implementation for this driver should be something like this: diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c index 414ad91..a2d5c3d 100644 --- a/drivers/gpio/gpio-tegra.c +++ b/drivers/gpio/gpio-tegra.c @@ -346,6 +346,11 @@ static int tegra_gpio_wake_enable(struct irq_data *d, unsigned int enable) } #endif +static int tegra_gpio_irq_request(struct irq_data *d) +{ + tegra_gpio_request(NULL, d->hwirq); +} + static struct irq_chip tegra_gpio_irq_chip = { .name = "GPIO", .irq_ack = tegra_gpio_irq_ack, @@ -355,6 +360,7 @@ static struct irq_chip tegra_gpio_irq_chip = { #ifdef CONFIG_PM_SLEEP .irq_set_wake = tegra_gpio_wake_enable, #endif + .irq_request = tegra_gpio_irq_request, }; static const struct dev_pm_ops tegra_gpio_pm_ops = { This is definitely quite similar to the omap-gpio.c change but not the same. > This is a pretty hard design pattern to properly replicate in every such > driver is it not? > > Hmmmm > Is indeed a hard design pattern but I couldn't figure out a more generic way to handle this. Maybe we could use [1] for now to solve the issue that we have with OMAP GPIO and later when the same issue is found on other GPIO controllers and a similar change is made on these drivers we will see some form of pattern that emerge allowing us to make a later refactor to reduce the code duplication. Or do you have a better idea on how to solve this? > Yours, > Linus Walleij Best regards, Javier