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@gmail.com
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@li....
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