Re: [PATCH v7 1/3] staging: greybus: gpio: switch GPIO portions to use GPIOLIB_IRQCHIP

2019-01-15 Thread Greg Kroah-Hartman
On Tue, Jan 15, 2019 at 04:13:17PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Jan 14, 2019 at 08:26:16PM +0530, Nishad Kamdar wrote:
> > Convert the GPIO driver to use the GPIO irqchip library
> > GPIOLIB_IRQCHIP instead of reimplementing the same.
> > 
> > Reviewed-by: Johan Hovold 
> > Signed-off-by: Nishad Kamdar 
> 
> Did you test build this patch?
> 
> It fails horribly for me:
> 
> drivers/staging/greybus/gpio.c: In function ‘gb_gpio_request_handler’:
> drivers/staging/greybus/gpio.c:389:34: error: ‘struct gpio_chip’ has no 
> member named ‘irq’
>   irq = irq_find_mapping(ggc->chip.irq.domain, event->which);
>   ^
>   CC [M]  drivers/staging/rtl8712/usb_intf.o
> drivers/staging/greybus/gpio.c: In function ‘gb_gpio_probe’:
> drivers/staging/greybus/gpio.c:577:8: error: implicit declaration of function 
> ‘gpiochip_irqchip_add’; did you mean ‘gpiochip_add’? 
> [-Werror=implicit-function-declaration]
>   ret = gpiochip_irqchip_add(gpio, irqc, 0, handle_level_irq,
> ^~~~
> gpiochip_add
> 
> 
> Please be more careful...

Hm, this seems to be a problem on my side.  You selected the new config
symbol, but my quick build system didn't catch that and it didnt
regenerate the kernel configuration, which causes this problem.

So this is fine, sorry for the noise, my fault.

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v7 1/3] staging: greybus: gpio: switch GPIO portions to use GPIOLIB_IRQCHIP

2019-01-15 Thread Greg Kroah-Hartman
On Mon, Jan 14, 2019 at 08:26:16PM +0530, Nishad Kamdar wrote:
> Convert the GPIO driver to use the GPIO irqchip library
> GPIOLIB_IRQCHIP instead of reimplementing the same.
> 
> Reviewed-by: Johan Hovold 
> Signed-off-by: Nishad Kamdar 

Did you test build this patch?

It fails horribly for me:

drivers/staging/greybus/gpio.c: In function ‘gb_gpio_request_handler’:
drivers/staging/greybus/gpio.c:389:34: error: ‘struct gpio_chip’ has no member 
named ‘irq’
  irq = irq_find_mapping(ggc->chip.irq.domain, event->which);
  ^
  CC [M]  drivers/staging/rtl8712/usb_intf.o
drivers/staging/greybus/gpio.c: In function ‘gb_gpio_probe’:
drivers/staging/greybus/gpio.c:577:8: error: implicit declaration of function 
‘gpiochip_irqchip_add’; did you mean ‘gpiochip_add’? 
[-Werror=implicit-function-declaration]
  ret = gpiochip_irqchip_add(gpio, irqc, 0, handle_level_irq,
^~~~
gpiochip_add


Please be more careful...

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v7 1/3] staging: greybus: gpio: switch GPIO portions to use GPIOLIB_IRQCHIP

2019-01-14 Thread Nishad Kamdar
Convert the GPIO driver to use the GPIO irqchip library
GPIOLIB_IRQCHIP instead of reimplementing the same.

Reviewed-by: Johan Hovold 
Signed-off-by: Nishad Kamdar 
---
Changes in v7:
 - No change.
Changes in v5:
 - Restore "struct irq_chip irqc" in "struct gb_gpio_controller"
   This is because we cannot use the gpio-chip irqchip, as that
   will register the irqchip automatically and possibly in an
   incompatible way.
Changes in v4:
 - Remove changes related to conversion to gpiochip_get_data() to
   include it as a new patch.
 - Remove the 'struct irq_chip' field from 'struct gb_gpio_controller'
   as struct gpio_chip will have an irqchip whenever
   CONFIG_GPIOLIB_IRQCHIP is selected.
 - Update the TODO file as per the changes.
Changes in v3:
 - Combine patches as into a patch series.
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.wall...@linaro.org.
---
 drivers/staging/greybus/Kconfig |   1 +
 drivers/staging/greybus/TODO|   2 -
 drivers/staging/greybus/gpio.c  | 156 ++--
 3 files changed, 11 insertions(+), 148 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/TODO b/drivers/staging/greybus/TODO
index 3b90a5711998..31f1f2cb401c 100644
--- a/drivers/staging/greybus/TODO
+++ b/drivers/staging/greybus/TODO
@@ -1,5 +1,3 @@
 * Convert all uses of the old GPIO API from  to the
   GPIO descriptor API in  and look up GPIO
   lines from device tree or ACPI.
-* Convert the GPIO driver to use the GPIO irqchip library
-  GPIOLIB_IRQCHIP instead of reimplementing the same.
diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c
index e110681e6f86..3151004d26fb 100644
--- a/drivers/staging/greybus/gpio.c
+++ b/drivers/staging/greybus/gpio.c
@@ -9,9 +9,9 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
+#include 
 #include 
 
 #include "greybus.h"
@@ -39,11 +39,6 @@ struct gb_gpio_controller {
 
struct gpio_chipchip;
struct irq_chip irqc;
-   struct irq_chip *irqchip;
-   struct irq_domain   *irqdomain;
-   unsigned intirq_base;
-   irq_flow_handler_t  irq_handler;
-   unsigned intirq_default_type;
struct mutexirq_lock;
 };
 #define gpio_chip_to_gb_gpio_controller(chip) \
@@ -391,7 +386,7 @@ static int gb_gpio_request_handler(struct gb_operation *op)
return -EINVAL;
}
 
-   irq = irq_find_mapping(ggc->irqdomain, event->which);
+   irq = irq_find_mapping(ggc->chip.irq.domain, event->which);
if (!irq) {
dev_err(dev, "failed to find IRQ\n");
return -EINVAL;
@@ -506,135 +501,6 @@ static int gb_gpio_controller_setup(struct 
gb_gpio_controller *ggc)
return ret;
 }
 
-/**
- * gb_gpio_irq_map() - maps an IRQ into a GB gpio irqchip
- * @d: the irqdomain used by this irqchip
- * @irq: the global irq number used by this GB gpio irqchip irq
- * @hwirq: the local IRQ/GPIO line offset on this GB gpio
- *
- * This function will set up the mapping for a certain IRQ line on a
- * GB gpio by assigning the GB gpio as chip data, and using the irqchip
- * stored inside the GB gpio.
- */
-static int gb_gpio_irq_map(struct irq_domain *domain, unsigned int irq,
-  irq_hw_number_t hwirq)
-{
-   struct gpio_chip *chip = domain->host_data;
-   struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
-
-   irq_set_chip_data(irq, ggc);
-   irq_set_chip_and_handler(irq, ggc->irqchip, ggc->irq_handler);
-   irq_set_noprobe(irq);
-   /*
-* No set-up of the hardware will happen if IRQ_TYPE_NONE
-* is passed as default type.
-*/
-   if (ggc->irq_default_type != IRQ_TYPE_NONE)
-   irq_set_irq_type(irq, ggc->irq_default_type);
-
-   return 0;
-}
-
-static void gb_gpio_irq_unmap(struct irq_domain *d, unsigned int irq)
-{
-   irq_set_chip_and_handler(irq, NULL, NULL);
-   irq_set_chip_data(irq, NULL);
-}
-
-static const struct irq_domain_ops gb_gpio_domain_ops = {
-   .map= gb_gpio_irq_map,
-   .unmap  = gb_gpio_irq_unmap,
-};
-
-/**
- * gb_gpio_irqchip_remove() - removes an irqchip added to a gb_gpio_controller
- * @ggc: the gb_gpio_controller to remove the irqchip from
- *
- * This i