On Tue, Dec 18, 2018 at 12:10:34PM +0100, Johan Hovold wrote: > On Thu, Nov 22, 2018 at 10:37:16PM +0530, Nishad Kamdar wrote: > > Convert the GPIO driver to use the GPIO irqchip library > > GPIOLIB_IRQCHIP instead of reimplementing the same. > > > > Signed-off-by: Nishad Kamdar <nishadkamdar@xxxxxxxxx> > > --- > > Changes in v2: > > - Retained irq.h and irqdomain.h headers. > > - Dropped function gb_gpio_irqchip_add() and > > called gpiochip_irqchip_add() from probe(). > > - Referred https://lkml.kernel.org/r/1476054589-28422-1-git-send-email-linus.walleij@xxxxxxxxxx. > > Thanks for the update, and sorry about the late review. This looks > mostly good now, except for a couple minor things pointed out below. > > You also included the conversion to gpiochip_get_data() (as Linus also > did in his patch) although that's really a separate change and should go > in its own patch. Please break that bit out in a follow-up patch. > > Also note that someone did a bunch random white space changes to this > file in the staging tree, so it will not apply cleanly any more. > > > --- > > drivers/staging/greybus/Kconfig | 1 + > > drivers/staging/greybus/gpio.c | 184 ++++---------------------------- > > 2 files changed, 24 insertions(+), 161 deletions(-) > > > > diff --git a/drivers/staging/greybus/Kconfig b/drivers/staging/greybus/Kconfig > > index ab096bcef98c..b571e4e8060b 100644 > > --- a/drivers/staging/greybus/Kconfig > > +++ b/drivers/staging/greybus/Kconfig > > @@ -148,6 +148,7 @@ if GREYBUS_BRIDGED_PHY > > config GREYBUS_GPIO > > tristate "Greybus GPIO Bridged PHY driver" > > depends on GPIOLIB > > + select GPIOLIB_IRQCHIP > > ---help--- > > Select this option if you have a device that follows the > > Greybus GPIO Bridged PHY Class specification. > > diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c > > index b1d4698019a1..2ec54744171d 100644 > > --- a/drivers/staging/greybus/gpio.c > > +++ b/drivers/staging/greybus/gpio.c > > @@ -9,9 +9,9 @@ > > #include <linux/kernel.h> > > #include <linux/module.h> > > #include <linux/slab.h> > > -#include <linux/gpio.h> > > #include <linux/irq.h> > > #include <linux/irqdomain.h> > > +#include <linux/gpio/driver.h> > > #include <linux/mutex.h> > > > > #include "greybus.h" > > @@ -39,15 +39,8 @@ struct gb_gpio_controller { > > > > struct gpio_chip chip; > > struct irq_chip irqc; > > Turns out struct gpio_chip will have an irqchip whenever > CONFIG_GPIOLIB_IRQCHIP is selected so you can drop this one too. > Ok, I'll do that. > > - struct irq_chip *irqchip; > > - struct irq_domain *irqdomain; > > - unsigned int irq_base; > > - irq_flow_handler_t irq_handler; > > - unsigned int irq_default_type; > > struct mutex irq_lock; > > }; > > -#define gpio_chip_to_gb_gpio_controller(chip) \ > > - container_of(chip, struct gb_gpio_controller, chip) > > #define irq_data_to_gpio_chip(d) (d->domain->host_data) > > > > static int gb_gpio_line_count_operation(struct gb_gpio_controller *ggc) > > @@ -276,7 +269,7 @@ static void _gb_gpio_irq_set_type(struct gb_gpio_controller *ggc, > > static void gb_gpio_irq_mask(struct irq_data *d) > > { > > struct gpio_chip *chip = irq_data_to_gpio_chip(d); > > - struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip); > > + struct gb_gpio_controller *ggc = gpiochip_get_data(chip); > > So please split these changes into a separate patch as they are not > related to the irqchip changes. > > Oh, and don't forget to update the TODO file now that the conversion is > done. :) > Sure, I'll do that. Thanks for the review. regards, Nishad > Thanks, > Johan _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
