From: Linus Walleij <[email protected]>
This switches the BCMA GPIO driver to use GPIOLIB_IRQCHIP to
handle its interrupts instead of rolling its own copy of the
irqdomain handling etc.
Signed-off-by: Linus Walleij <[email protected]>
Signed-off-by: Hauke Mehrtens <[email protected]>
---
changes since:
v2:
* use bcma_bus_get_host_dev()
v1:
* I fixed some bugs in the original patch, some of them are compile
errors and others caused some runtime problems.
drivers/bcma/Kconfig | 2 +-
drivers/bcma/driver_gpio.c | 92 ++++++++++-------------------
include/linux/bcma/bcma_driver_chipcommon.h | 1 -
3 files changed, 31 insertions(+), 64 deletions(-)
diff --git a/drivers/bcma/Kconfig b/drivers/bcma/Kconfig
index be5fffb..023d448 100644
--- a/drivers/bcma/Kconfig
+++ b/drivers/bcma/Kconfig
@@ -92,7 +92,7 @@ config BCMA_DRIVER_GMAC_CMN
config BCMA_DRIVER_GPIO
bool "BCMA GPIO driver"
depends on BCMA && GPIOLIB
- select IRQ_DOMAIN if BCMA_HOST_SOC
+ select GPIOLIB_IRQCHIP if BCMA_HOST_SOC
help
Driver to provide access to the GPIO pins of the bcma bus.
diff --git a/drivers/bcma/driver_gpio.c b/drivers/bcma/driver_gpio.c
index 5f6018e..504899a7 100644
--- a/drivers/bcma/driver_gpio.c
+++ b/drivers/bcma/driver_gpio.c
@@ -8,10 +8,8 @@
* Licensed under the GNU/GPL. See COPYING for details.
*/
-#include <linux/gpio.h>
-#include <linux/irq.h>
+#include <linux/gpio/driver.h>
#include <linux/interrupt.h>
-#include <linux/irqdomain.h>
#include <linux/export.h>
#include <linux/bcma/bcma.h>
@@ -79,19 +77,11 @@ static void bcma_gpio_free(struct gpio_chip *chip, unsigned gpio)
}
#if IS_BUILTIN(CONFIG_BCM47XX) || IS_BUILTIN(CONFIG_ARCH_BCM_5301X)
-static int bcma_gpio_to_irq(struct gpio_chip *chip, unsigned gpio)
-{
- struct bcma_drv_cc *cc = bcma_gpio_get_cc(chip);
-
- if (cc->core->bus->hosttype == BCMA_HOSTTYPE_SOC)
- return irq_find_mapping(cc->irq_domain, gpio);
- else
- return -EINVAL;
-}
static void bcma_gpio_irq_unmask(struct irq_data *d)
{
- struct bcma_drv_cc *cc = irq_data_get_irq_chip_data(d);
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct bcma_drv_cc *cc = bcma_gpio_get_cc(gc);
int gpio = irqd_to_hwirq(d);
u32 val = bcma_chipco_gpio_in(cc, BIT(gpio));
@@ -101,7 +91,8 @@ static void bcma_gpio_irq_unmask(struct irq_data *d)
static void bcma_gpio_irq_mask(struct irq_data *d)
{
- struct bcma_drv_cc *cc = irq_data_get_irq_chip_data(d);
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct bcma_drv_cc *cc = bcma_gpio_get_cc(gc);
int gpio = irqd_to_hwirq(d);
bcma_chipco_gpio_intmask(cc, BIT(gpio), 0);
@@ -116,6 +107,7 @@ static struct irq_chip bcma_gpio_irq_chip = {
static irqreturn_t bcma_gpio_irq_handler(int irq, void *dev_id)
{
struct bcma_drv_cc *cc = dev_id;
+ struct gpio_chip *gc = &cc->gpio;
u32 val = bcma_cc_read32(cc, BCMA_CC_GPIOIN);
u32 mask = bcma_cc_read32(cc, BCMA_CC_GPIOIRQ);
u32 pol = bcma_cc_read32(cc, BCMA_CC_GPIOPOL);
@@ -125,81 +117,58 @@ static irqreturn_t bcma_gpio_irq_handler(int irq, void *dev_id)
if (!irqs)
return IRQ_NONE;
- for_each_set_bit(gpio, &irqs, cc->gpio.ngpio)
- generic_handle_irq(bcma_gpio_to_irq(&cc->gpio, gpio));
+ for_each_set_bit(gpio, &irqs, gc->ngpio)
+ generic_handle_irq(irq_find_mapping(gc->irqdomain, gpio));
bcma_chipco_gpio_polarity(cc, irqs, val & irqs);
return IRQ_HANDLED;
}
-static int bcma_gpio_irq_domain_init(struct bcma_drv_cc *cc)
+static int bcma_gpio_irq_init(struct bcma_drv_cc *cc)
{
struct gpio_chip *chip = &cc->gpio;
- int gpio, hwirq, err;
+ int hwirq, err;
if (cc->core->bus->hosttype != BCMA_HOSTTYPE_SOC)
return 0;
- cc->irq_domain = irq_domain_add_linear(NULL, chip->ngpio,
- &irq_domain_simple_ops, cc);
- if (!cc->irq_domain) {
- err = -ENODEV;
- goto err_irq_domain;
- }
- for (gpio = 0; gpio < chip->ngpio; gpio++) {
- int irq = irq_create_mapping(cc->irq_domain, gpio);
-
- irq_set_chip_data(irq, cc);
- irq_set_chip_and_handler(irq, &bcma_gpio_irq_chip,
- handle_simple_irq);
- }
-
hwirq = bcma_core_irq(cc->core, 0);
err = request_irq(hwirq, bcma_gpio_irq_handler, IRQF_SHARED, "gpio",
cc);
if (err)
- goto err_req_irq;
+ return err;
bcma_chipco_gpio_intmask(cc, ~0, 0);
bcma_cc_set32(cc, BCMA_CC_IRQMASK, BCMA_CC_IRQ_GPIO);
- return 0;
-
-err_req_irq:
- for (gpio = 0; gpio < chip->ngpio; gpio++) {
- int irq = irq_find_mapping(cc->irq_domain, gpio);
-
- irq_dispose_mapping(irq);
+ err = gpiochip_irqchip_add(chip,
+ &bcma_gpio_irq_chip,
+ 0,
+ handle_simple_irq,
+ IRQ_TYPE_NONE);
+ if (err) {
+ free_irq(hwirq, cc);
+ return err;
}
- irq_domain_remove(cc->irq_domain);
-err_irq_domain:
- return err;
+
+ return 0;
}
-static void bcma_gpio_irq_domain_exit(struct bcma_drv_cc *cc)
+static void bcma_gpio_irq_exit(struct bcma_drv_cc *cc)
{
- struct gpio_chip *chip = &cc->gpio;
- int gpio;
-
if (cc->core->bus->hosttype != BCMA_HOSTTYPE_SOC)
return;
bcma_cc_mask32(cc, BCMA_CC_IRQMASK, ~BCMA_CC_IRQ_GPIO);
free_irq(bcma_core_irq(cc->core, 0), cc);
- for (gpio = 0; gpio < chip->ngpio; gpio++) {
- int irq = irq_find_mapping(cc->irq_domain, gpio);
-
- irq_dispose_mapping(irq);
- }
- irq_domain_remove(cc->irq_domain);
}
#else
-static int bcma_gpio_irq_domain_init(struct bcma_drv_cc *cc)
+static int bcma_gpio_irq_init(struct bcma_drv_cc *cc)
{
return 0;
}
-static void bcma_gpio_irq_domain_exit(struct bcma_drv_cc *cc)
+static void bcma_gpio_irq_exit(struct bcma_drv_cc *cc)
{
}
#endif
@@ -218,9 +187,8 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
chip->set = bcma_gpio_set_value;
chip->direction_input = bcma_gpio_direction_input;
chip->direction_output = bcma_gpio_direction_output;
-#if IS_BUILTIN(CONFIG_BCM47XX) || IS_BUILTIN(CONFIG_ARCH_BCM_5301X)
- chip->to_irq = bcma_gpio_to_irq;
-#endif
+ chip->owner = THIS_MODULE;
+ chip->dev = bcma_bus_get_host_dev(bus);
#if IS_BUILTIN(CONFIG_OF)
if (cc->core->bus->hosttype == BCMA_HOSTTYPE_SOC)
chip->of_node = cc->core->dev.of_node;
@@ -248,13 +216,13 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
else
chip->base = -1;
- err = bcma_gpio_irq_domain_init(cc);
+ err = gpiochip_add(chip);
if (err)
return err;
- err = gpiochip_add(chip);
+ err = bcma_gpio_irq_init(cc);
if (err) {
- bcma_gpio_irq_domain_exit(cc);
+ gpiochip_remove(chip);
return err;
}
@@ -263,7 +231,7 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
int bcma_gpio_unregister(struct bcma_drv_cc *cc)
{
- bcma_gpio_irq_domain_exit(cc);
+ bcma_gpio_irq_exit(cc);
gpiochip_remove(&cc->gpio);
return 0;
}
diff --git a/include/linux/bcma/bcma_driver_chipcommon.h b/include/linux/bcma/bcma_driver_chipcommon.h
index 6cceedf..cf03843 100644
--- a/include/linux/bcma/bcma_driver_chipcommon.h
+++ b/include/linux/bcma/bcma_driver_chipcommon.h
@@ -640,7 +640,6 @@ struct bcma_drv_cc {
spinlock_t gpio_lock;
#ifdef CONFIG_BCMA_DRIVER_GPIO
struct gpio_chip gpio;
- struct irq_domain *irq_domain;
#endif
};
--
2.1.4
> From: Linus Walleij <[email protected]>
>
> This switches the BCMA GPIO driver to use GPIOLIB_IRQCHIP to
> handle its interrupts instead of rolling its own copy of the
> irqdomain handling etc.
>
> Signed-off-by: Linus Walleij <[email protected]>
> Signed-off-by: Hauke Mehrtens <[email protected]>
Thanks, applied to wireless-drivers-next.git.
Kalle Valo
On 12/22/2015 10:01 AM, Linus Walleij wrote:
> On Fri, Dec 18, 2015 at 7:01 PM, Hauke Mehrtens <[email protected]> wrote:
>> On 12/18/2015 07:27 AM, Rafał Miłecki wrote:
>>> I'm afraid it wasn't tested on BCM47XX (MIPS) :(
>>
>> Yes, you are probably right.
>>
>>> On 14 August 2015 at 00:21, Hauke Mehrtens <[email protected]> wrote:
>>>> @@ -218,9 +187,8 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
>>>> chip->set = bcma_gpio_set_value;
>>>> chip->direction_input = bcma_gpio_direction_input;
>>>> chip->direction_output = bcma_gpio_direction_output;
>>>> -#if IS_BUILTIN(CONFIG_BCM47XX) || IS_BUILTIN(CONFIG_ARCH_BCM_5301X)
>>>> - chip->to_irq = bcma_gpio_to_irq;
>>>> -#endif
>>>> + chip->owner = THIS_MODULE;
>>>> + chip->dev = bcma_bus_get_host_dev(bus);
>>>
>>> This assigns &bus->host_pdev->dev which is NULL.
>>
>> hmm, how do we fix this, as long as bcma does not have a device on mips
>> this is a problem. Should we create a new device for mips?
>
> Yes I think MIPS should just create a host device. That
> makes things simple and nice. Is it complex to do so?
>
> Yours,
> Linus Walleij
>
Currently the documentation says this for the parent member on struct
gpio_chip:
* @parent: optional parent device providing the GPIOs
I assume the optional is wrong here.
Hauke
On Tue, Dec 22, 2015 at 1:09 PM, Hauke Mehrtens <[email protected]> wrote:
> On 12/22/2015 10:01 AM, Linus Walleij wrote:
>> On Fri, Dec 18, 2015 at 7:01 PM, Hauke Mehrtens <[email protected]> wrote:
>>> On 12/18/2015 07:27 AM, Rafał Miłecki wrote:
>>>> I'm afraid it wasn't tested on BCM47XX (MIPS) :(
>>>
>>> Yes, you are probably right.
>>>
>>>> On 14 August 2015 at 00:21, Hauke Mehrtens <[email protected]> wrote:
>>>>> @@ -218,9 +187,8 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
>>>>> chip->set = bcma_gpio_set_value;
>>>>> chip->direction_input = bcma_gpio_direction_input;
>>>>> chip->direction_output = bcma_gpio_direction_output;
>>>>> -#if IS_BUILTIN(CONFIG_BCM47XX) || IS_BUILTIN(CONFIG_ARCH_BCM_5301X)
>>>>> - chip->to_irq = bcma_gpio_to_irq;
>>>>> -#endif
>>>>> + chip->owner = THIS_MODULE;
>>>>> + chip->dev = bcma_bus_get_host_dev(bus);
>>>>
>>>> This assigns &bus->host_pdev->dev which is NULL.
>>>
>>> hmm, how do we fix this, as long as bcma does not have a device on mips
>>> this is a problem. Should we create a new device for mips?
>>
>> Yes I think MIPS should just create a host device. That
>> makes things simple and nice. Is it complex to do so?
>>
>> Yours,
>> Linus Walleij
>>
> Currently the documentation says this for the parent member on struct
> gpio_chip:
>
> * @parent: optional parent device providing the GPIOs
>
> I assume the optional is wrong here.
For just gpiochip it is optional. But if you read on:
GPIO drivers providing IRQs
---------------------------
(...)
To use the helpers please keep the following in mind:
- Make sure to assign all relevant members of the struct gpio_chip so that
the irqchip can initialize. E.g. .dev and .can_sleep shall be set up
properly.
(...)
Well it's complex. But when adding the gpio character device I
am working with, I will introduce a struct device to the gpio_chip
(or similar sibling gpio_device) which we can then refer, so that
this actually will no longer be a requirement.
Yours,
Linus Walleij
On Fri, Dec 18, 2015 at 7:01 PM, Hauke Mehrtens <[email protected]> wrote:
> On 12/18/2015 07:27 AM, Rafał Miłecki wrote:
>> I'm afraid it wasn't tested on BCM47XX (MIPS) :(
>
> Yes, you are probably right.
>
>> On 14 August 2015 at 00:21, Hauke Mehrtens <[email protected]> wrote:
>>> @@ -218,9 +187,8 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
>>> chip->set = bcma_gpio_set_value;
>>> chip->direction_input = bcma_gpio_direction_input;
>>> chip->direction_output = bcma_gpio_direction_output;
>>> -#if IS_BUILTIN(CONFIG_BCM47XX) || IS_BUILTIN(CONFIG_ARCH_BCM_5301X)
>>> - chip->to_irq = bcma_gpio_to_irq;
>>> -#endif
>>> + chip->owner = THIS_MODULE;
>>> + chip->dev = bcma_bus_get_host_dev(bus);
>>
>> This assigns &bus->host_pdev->dev which is NULL.
>
> hmm, how do we fix this, as long as bcma does not have a device on mips
> this is a problem. Should we create a new device for mips?
Yes I think MIPS should just create a host device. That
makes things simple and nice. Is it complex to do so?
Yours,
Linus Walleij
On 12/18/2015 07:27 AM, Rafał Miłecki wrote:
> I'm afraid it wasn't tested on BCM47XX (MIPS) :(
Yes, you are probably right.
> On 14 August 2015 at 00:21, Hauke Mehrtens <[email protected]> wrote:
>> @@ -218,9 +187,8 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
>> chip->set = bcma_gpio_set_value;
>> chip->direction_input = bcma_gpio_direction_input;
>> chip->direction_output = bcma_gpio_direction_output;
>> -#if IS_BUILTIN(CONFIG_BCM47XX) || IS_BUILTIN(CONFIG_ARCH_BCM_5301X)
>> - chip->to_irq = bcma_gpio_to_irq;
>> -#endif
>> + chip->owner = THIS_MODULE;
>> + chip->dev = bcma_bus_get_host_dev(bus);
>
> This assigns &bus->host_pdev->dev which is NULL.
hmm, how do we fix this, as long as bcma does not have a device on mips
this is a problem. Should we create a new device for mips?
>> @@ -248,13 +216,13 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
>> else
>> chip->base = -1;
>>
>> - err = bcma_gpio_irq_domain_init(cc);
>> + err = gpiochip_add(chip);
>> if (err)
>> return err;
>>
>> - err = gpiochip_add(chip);
>> + err = bcma_gpio_irq_init(cc);
>
> This results in:
> [ 0.157054] missing gpiochip .dev parent pointer
> (coming from gpiochip_irqchip_add) and
> [ 0.157287] bcma: bus0: Error registering GPIO driver: -22
I'm afraid it wasn't tested on BCM47XX (MIPS) :(
On 14 August 2015 at 00:21, Hauke Mehrtens <[email protected]> wrote:
> @@ -218,9 +187,8 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
> chip->set = bcma_gpio_set_value;
> chip->direction_input = bcma_gpio_direction_input;
> chip->direction_output = bcma_gpio_direction_output;
> -#if IS_BUILTIN(CONFIG_BCM47XX) || IS_BUILTIN(CONFIG_ARCH_BCM_5301X)
> - chip->to_irq = bcma_gpio_to_irq;
> -#endif
> + chip->owner = THIS_MODULE;
> + chip->dev = bcma_bus_get_host_dev(bus);
This assigns &bus->host_pdev->dev which is NULL.
> @@ -248,13 +216,13 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
> else
> chip->base = -1;
>
> - err = bcma_gpio_irq_domain_init(cc);
> + err = gpiochip_add(chip);
> if (err)
> return err;
>
> - err = gpiochip_add(chip);
> + err = bcma_gpio_irq_init(cc);
This results in:
[ 0.157054] missing gpiochip .dev parent pointer
(coming from gpiochip_irqchip_add) and
[ 0.157287] bcma: bus0: Error registering GPIO driver: -22