* [PATCH V2 2/2] MIPS: BCM47XX: Prepare support for GPIO buttons
2013-11-29 16:09 [PATCH V2 1/2] bcma: gpio: add own IRQ domain Rafał Miłecki
@ 2013-11-29 16:09 ` Rafał Miłecki
2013-12-09 13:33 ` Hauke Mehrtens
2013-12-10 15:24 ` [PATCH V3 " Rafał Miłecki
2013-11-29 16:31 ` [PATCH V2 1/2] bcma: gpio: add own IRQ domain Hauke Mehrtens
2013-11-29 17:48 ` [PATCH V3 " Rafał Miłecki
2 siblings, 2 replies; 20+ messages in thread
From: Rafał Miłecki @ 2013-11-29 16:09 UTC (permalink / raw)
To: linux-mips, Ralf Baechle; +Cc: Hauke Mehrtens, Rafał Miłecki
So far this adds support for one Netgear model only, but it's designed
and ready to add many more device. We could hopefully import database
from OpenWrt.
Support for SSB is currently disabled, because SSB doesn't implement IRQ
domain yet.
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
arch/mips/bcm47xx/Makefile | 2 +-
arch/mips/bcm47xx/bcm47xx_private.h | 3 ++
arch/mips/bcm47xx/buttons.c | 81 +++++++++++++++++++++++++++++++++++
arch/mips/bcm47xx/setup.c | 1 +
4 files changed, 86 insertions(+), 1 deletion(-)
create mode 100644 arch/mips/bcm47xx/buttons.c
diff --git a/arch/mips/bcm47xx/Makefile b/arch/mips/bcm47xx/Makefile
index 84e9aed..006a05e 100644
--- a/arch/mips/bcm47xx/Makefile
+++ b/arch/mips/bcm47xx/Makefile
@@ -4,5 +4,5 @@
#
obj-y += irq.o nvram.o prom.o serial.o setup.o time.o sprom.o
-obj-y += board.o leds.o
+obj-y += board.o buttons.o leds.o
obj-$(CONFIG_BCM47XX_SSB) += wgt634u.o
diff --git a/arch/mips/bcm47xx/bcm47xx_private.h b/arch/mips/bcm47xx/bcm47xx_private.h
index 1a1e600..5c94ace 100644
--- a/arch/mips/bcm47xx/bcm47xx_private.h
+++ b/arch/mips/bcm47xx/bcm47xx_private.h
@@ -3,6 +3,9 @@
#include <linux/kernel.h>
+/* buttons.c */
+int __init bcm47xx_buttons_register(void);
+
/* leds.c */
void __init bcm47xx_leds_register(void);
diff --git a/arch/mips/bcm47xx/buttons.c b/arch/mips/bcm47xx/buttons.c
new file mode 100644
index 0000000..ecd6b1b
--- /dev/null
+++ b/arch/mips/bcm47xx/buttons.c
@@ -0,0 +1,81 @@
+#include "bcm47xx_private.h"
+
+#include <linux/input.h>
+#include <linux/gpio_keys.h>
+#include <linux/interrupt.h>
+#include <linux/ssb/ssb_embedded.h>
+#include <bcm47xx_board.h>
+#include <bcm47xx.h>
+
+struct input_dev *input;
+
+/**************************************************
+ * Database
+ **************************************************/
+
+static struct gpio_keys_button
+bcm47xx_buttons_netgear_wndr4500_v1[] = {
+ {
+ .code = KEY_WPS_BUTTON,
+ .gpio = 4,
+ .active_low = 1,
+ },
+ {
+ .code = KEY_RFKILL,
+ .gpio = 5,
+ .active_low = 1,
+ },
+ {
+ .code = KEY_RESTART,
+ .gpio = 6,
+ .active_low = 1,
+ },
+};
+
+/**************************************************
+ * Init
+ **************************************************/
+
+static struct gpio_keys_platform_data bcm47xx_button_pdata;
+
+static struct platform_device bcm47xx_buttons_gpio_keys = {
+ .name = "gpio-keys",
+ .dev = {
+ .platform_data = &bcm47xx_button_pdata,
+ }
+};
+
+#define bcm47xx_set_bdata(dev_buttons) do { \
+ bcm47xx_button_pdata.buttons = dev_buttons; \
+ bcm47xx_button_pdata.nbuttons = ARRAY_SIZE(dev_buttons); \
+} while (0)
+
+int __init bcm47xx_buttons_register(void)
+{
+ enum bcm47xx_board board = bcm47xx_board_get();
+ int err;
+
+#ifdef CONFIG_BCM47XX_SSB
+ if (bcm47xx_bus_type == BCM47XX_BUS_TYPE_SSB) {
+ pr_debug("Buttons on SSB are not supported yet.\n");
+ return -ENOTSUPP;
+ }
+#endif
+
+ switch (board) {
+ case BCM47XX_BOARD_NETGEAR_WNDR4500V1:
+ bcm47xx_set_bdata(bcm47xx_buttons_netgear_wndr4500_v1);
+ break;
+ default:
+ pr_debug("No buttons configuration found for this device\n");
+ return -ENOTSUPP;
+ }
+
+ err = platform_device_register(&bcm47xx_buttons_gpio_keys);
+ if (err) {
+ pr_err("Failed to register platform device: %d\n", err);
+ return err;
+ }
+
+ return 0;
+}
diff --git a/arch/mips/bcm47xx/setup.c b/arch/mips/bcm47xx/setup.c
index 7e61c0b..a791124 100644
--- a/arch/mips/bcm47xx/setup.c
+++ b/arch/mips/bcm47xx/setup.c
@@ -242,6 +242,7 @@ static int __init bcm47xx_register_bus_complete(void)
#endif
}
+ bcm47xx_buttons_register();
bcm47xx_leds_register();
return 0;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V2 2/2] MIPS: BCM47XX: Prepare support for GPIO buttons
2013-11-29 16:09 ` [PATCH V2 2/2] MIPS: BCM47XX: Prepare support for GPIO buttons Rafał Miłecki
@ 2013-12-09 13:33 ` Hauke Mehrtens
2013-12-10 15:24 ` [PATCH V3 " Rafał Miłecki
1 sibling, 0 replies; 20+ messages in thread
From: Hauke Mehrtens @ 2013-12-09 13:33 UTC (permalink / raw)
To: Rafał Miłecki; +Cc: linux-mips, Ralf Baechle
On 11/29/2013 05:09 PM, Rafał Miłecki wrote:
> So far this adds support for one Netgear model only, but it's designed
> and ready to add many more device. We could hopefully import database
> from OpenWrt.
> Support for SSB is currently disabled, because SSB doesn't implement IRQ
> domain yet.
>
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
> arch/mips/bcm47xx/Makefile | 2 +-
> arch/mips/bcm47xx/bcm47xx_private.h | 3 ++
> arch/mips/bcm47xx/buttons.c | 81 +++++++++++++++++++++++++++++++++++
> arch/mips/bcm47xx/setup.c | 1 +
> 4 files changed, 86 insertions(+), 1 deletion(-)
> create mode 100644 arch/mips/bcm47xx/buttons.c
>
> diff --git a/arch/mips/bcm47xx/Makefile b/arch/mips/bcm47xx/Makefile
> index 84e9aed..006a05e 100644
> --- a/arch/mips/bcm47xx/Makefile
> +++ b/arch/mips/bcm47xx/Makefile
> @@ -4,5 +4,5 @@
> #
>
> obj-y += irq.o nvram.o prom.o serial.o setup.o time.o sprom.o
> -obj-y += board.o leds.o
> +obj-y += board.o buttons.o leds.o
> obj-$(CONFIG_BCM47XX_SSB) += wgt634u.o
> diff --git a/arch/mips/bcm47xx/bcm47xx_private.h b/arch/mips/bcm47xx/bcm47xx_private.h
> index 1a1e600..5c94ace 100644
> --- a/arch/mips/bcm47xx/bcm47xx_private.h
> +++ b/arch/mips/bcm47xx/bcm47xx_private.h
> @@ -3,6 +3,9 @@
>
> #include <linux/kernel.h>
>
> +/* buttons.c */
> +int __init bcm47xx_buttons_register(void);
> +
> /* leds.c */
> void __init bcm47xx_leds_register(void);
>
> diff --git a/arch/mips/bcm47xx/buttons.c b/arch/mips/bcm47xx/buttons.c
> new file mode 100644
> index 0000000..ecd6b1b
> --- /dev/null
> +++ b/arch/mips/bcm47xx/buttons.c
> @@ -0,0 +1,81 @@
> +#include "bcm47xx_private.h"
> +
> +#include <linux/input.h>
> +#include <linux/gpio_keys.h>
> +#include <linux/interrupt.h>
> +#include <linux/ssb/ssb_embedded.h>
> +#include <bcm47xx_board.h>
> +#include <bcm47xx.h>
> +
> +struct input_dev *input;
This is unused please remove it.
> +
> +/**************************************************
> + * Database
> + **************************************************/
> +
> +static struct gpio_keys_button
> +bcm47xx_buttons_netgear_wndr4500_v1[] = {
Could you make this __initconst so it will be freed after the kernel
booted. Then you have to make sure it gets copied into a different non
init memory region if it will be used after the kernel booted. This
should scale to ~100 devices so this memory gets significant.
> + {
> + .code = KEY_WPS_BUTTON,
> + .gpio = 4,
> + .active_low = 1,
> + },
> + {
> + .code = KEY_RFKILL,
> + .gpio = 5,
> + .active_low = 1,
> + },
> + {
> + .code = KEY_RESTART,
> + .gpio = 6,
> + .active_low = 1,
> + },
> +};
> +
> +/**************************************************
> + * Init
> + **************************************************/
> +
> +static struct gpio_keys_platform_data bcm47xx_button_pdata;
> +
> +static struct platform_device bcm47xx_buttons_gpio_keys = {
> + .name = "gpio-keys",
> + .dev = {
> + .platform_data = &bcm47xx_button_pdata,
> + }
> +};
> +
> +#define bcm47xx_set_bdata(dev_buttons) do { \
> + bcm47xx_button_pdata.buttons = dev_buttons; \
> + bcm47xx_button_pdata.nbuttons = ARRAY_SIZE(dev_buttons); \
> +} while (0)
> +
> +int __init bcm47xx_buttons_register(void)
> +{
> + enum bcm47xx_board board = bcm47xx_board_get();
> + int err;
> +
> +#ifdef CONFIG_BCM47XX_SSB
> + if (bcm47xx_bus_type == BCM47XX_BUS_TYPE_SSB) {
> + pr_debug("Buttons on SSB are not supported yet.\n");
> + return -ENOTSUPP;
> + }
> +#endif
> +
> + switch (board) {
> + case BCM47XX_BOARD_NETGEAR_WNDR4500V1:
> + bcm47xx_set_bdata(bcm47xx_buttons_netgear_wndr4500_v1);
> + break;
> + default:
> + pr_debug("No buttons configuration found for this device\n");
> + return -ENOTSUPP;
> + }
> +
> + err = platform_device_register(&bcm47xx_buttons_gpio_keys);
> + if (err) {
> + pr_err("Failed to register platform device: %d\n", err);
> + return err;
> + }
> +
> + return 0;
> +}
> diff --git a/arch/mips/bcm47xx/setup.c b/arch/mips/bcm47xx/setup.c
> index 7e61c0b..a791124 100644
> --- a/arch/mips/bcm47xx/setup.c
> +++ b/arch/mips/bcm47xx/setup.c
> @@ -242,6 +242,7 @@ static int __init bcm47xx_register_bus_complete(void)
> #endif
> }
>
> + bcm47xx_buttons_register();
> bcm47xx_leds_register();
>
> return 0;
>
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH V3 2/2] MIPS: BCM47XX: Prepare support for GPIO buttons
2013-11-29 16:09 ` [PATCH V2 2/2] MIPS: BCM47XX: Prepare support for GPIO buttons Rafał Miłecki
2013-12-09 13:33 ` Hauke Mehrtens
@ 2013-12-10 15:24 ` Rafał Miłecki
2013-12-11 21:56 ` Hauke Mehrtens
2014-01-02 12:31 ` [PATCH V4] " Rafał Miłecki
1 sibling, 2 replies; 20+ messages in thread
From: Rafał Miłecki @ 2013-12-10 15:24 UTC (permalink / raw)
To: linux-mips, Ralf Baechle; +Cc: Hauke Mehrtens, Rafał Miłecki
So far this adds support for one Netgear model only, but it's designed
and ready to add many more device. We could hopefully import database
from OpenWrt.
Support for SSB is currently disabled, because SSB doesn't implement IRQ
domain yet.
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
V3: Use __initconst and copy data for detected device. This will allow
us to free some memory after init.
---
arch/mips/bcm47xx/Makefile | 2 +-
arch/mips/bcm47xx/bcm47xx_private.h | 3 ++
arch/mips/bcm47xx/buttons.c | 95 +++++++++++++++++++++++++++++++++++
arch/mips/bcm47xx/setup.c | 1 +
4 files changed, 100 insertions(+), 1 deletion(-)
create mode 100644 arch/mips/bcm47xx/buttons.c
diff --git a/arch/mips/bcm47xx/Makefile b/arch/mips/bcm47xx/Makefile
index 84e9aed..006a05e 100644
--- a/arch/mips/bcm47xx/Makefile
+++ b/arch/mips/bcm47xx/Makefile
@@ -4,5 +4,5 @@
#
obj-y += irq.o nvram.o prom.o serial.o setup.o time.o sprom.o
-obj-y += board.o leds.o
+obj-y += board.o buttons.o leds.o
obj-$(CONFIG_BCM47XX_SSB) += wgt634u.o
diff --git a/arch/mips/bcm47xx/bcm47xx_private.h b/arch/mips/bcm47xx/bcm47xx_private.h
index 1a1e600..5c94ace 100644
--- a/arch/mips/bcm47xx/bcm47xx_private.h
+++ b/arch/mips/bcm47xx/bcm47xx_private.h
@@ -3,6 +3,9 @@
#include <linux/kernel.h>
+/* buttons.c */
+int __init bcm47xx_buttons_register(void);
+
/* leds.c */
void __init bcm47xx_leds_register(void);
diff --git a/arch/mips/bcm47xx/buttons.c b/arch/mips/bcm47xx/buttons.c
new file mode 100644
index 0000000..3138f03
--- /dev/null
+++ b/arch/mips/bcm47xx/buttons.c
@@ -0,0 +1,95 @@
+#include "bcm47xx_private.h"
+
+#include <linux/input.h>
+#include <linux/gpio_keys.h>
+#include <linux/interrupt.h>
+#include <linux/ssb/ssb_embedded.h>
+#include <bcm47xx_board.h>
+#include <bcm47xx.h>
+
+/**************************************************
+ * Database
+ **************************************************/
+
+static struct gpio_keys_button
+bcm47xx_buttons_netgear_wndr4500_v1[] __initconst = {
+ {
+ .code = KEY_WPS_BUTTON,
+ .gpio = 4,
+ .active_low = 1,
+ },
+ {
+ .code = KEY_RFKILL,
+ .gpio = 5,
+ .active_low = 1,
+ },
+ {
+ .code = KEY_RESTART,
+ .gpio = 6,
+ .active_low = 1,
+ },
+};
+
+/**************************************************
+ * Init
+ **************************************************/
+
+static struct gpio_keys_platform_data bcm47xx_button_pdata;
+
+static struct platform_device bcm47xx_buttons_gpio_keys = {
+ .name = "gpio-keys",
+ .dev = {
+ .platform_data = &bcm47xx_button_pdata,
+ }
+};
+
+/* Copy data from __initconst */
+static int __init bcm47xx_buttons_copy(struct gpio_keys_button *buttons,
+ size_t nbuttons)
+{
+ size_t size = nbuttons * sizeof(*buttons);
+
+ bcm47xx_button_pdata.buttons = kmalloc(size, GFP_KERNEL);
+ if (!bcm47xx_button_pdata.buttons)
+ return -ENOMEM;
+ memcpy(bcm47xx_button_pdata.buttons, buttons, size);
+ bcm47xx_button_pdata.nbuttons = nbuttons;
+
+ return 0;
+}
+
+#define bcm47xx_copy_bdata(dev_buttons) \
+ bcm47xx_buttons_copy(dev_buttons, ARRAY_SIZE(dev_buttons));
+
+int __init bcm47xx_buttons_register(void)
+{
+ enum bcm47xx_board board = bcm47xx_board_get();
+ int err;
+
+#ifdef CONFIG_BCM47XX_SSB
+ if (bcm47xx_bus_type == BCM47XX_BUS_TYPE_SSB) {
+ pr_debug("Buttons on SSB are not supported yet.\n");
+ return -ENOTSUPP;
+ }
+#endif
+
+ switch (board) {
+ case BCM47XX_BOARD_NETGEAR_WNDR4500V1:
+ err = bcm47xx_copy_bdata(bcm47xx_buttons_netgear_wndr4500_v1);
+ break;
+ default:
+ pr_debug("No buttons configuration found for this device\n");
+ return -ENOTSUPP;
+ }
+
+ if (err)
+ return -ENOMEM;
+
+ err = platform_device_register(&bcm47xx_buttons_gpio_keys);
+ if (err) {
+ pr_err("Failed to register platform device: %d\n", err);
+ return err;
+ }
+
+ return 0;
+}
diff --git a/arch/mips/bcm47xx/setup.c b/arch/mips/bcm47xx/setup.c
index 7e61c0b..a791124 100644
--- a/arch/mips/bcm47xx/setup.c
+++ b/arch/mips/bcm47xx/setup.c
@@ -242,6 +242,7 @@ static int __init bcm47xx_register_bus_complete(void)
#endif
}
+ bcm47xx_buttons_register();
bcm47xx_leds_register();
return 0;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V3 2/2] MIPS: BCM47XX: Prepare support for GPIO buttons
2013-12-10 15:24 ` [PATCH V3 " Rafał Miłecki
@ 2013-12-11 21:56 ` Hauke Mehrtens
2014-01-02 12:31 ` [PATCH V4] " Rafał Miłecki
1 sibling, 0 replies; 20+ messages in thread
From: Hauke Mehrtens @ 2013-12-11 21:56 UTC (permalink / raw)
To: Rafał Miłecki, Ralf Baechle; +Cc: linux-mips
On 12/10/2013 04:24 PM, Rafał Miłecki wrote:
> So far this adds support for one Netgear model only, but it's designed
> and ready to add many more device. We could hopefully import database
> from OpenWrt.
> Support for SSB is currently disabled, because SSB doesn't implement IRQ
> domain yet.
>
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
Acked-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
> V3: Use __initconst and copy data for detected device. This will allow
> us to free some memory after init.
> ---
> arch/mips/bcm47xx/Makefile | 2 +-
> arch/mips/bcm47xx/bcm47xx_private.h | 3 ++
> arch/mips/bcm47xx/buttons.c | 95 +++++++++++++++++++++++++++++++++++
> arch/mips/bcm47xx/setup.c | 1 +
> 4 files changed, 100 insertions(+), 1 deletion(-)
> create mode 100644 arch/mips/bcm47xx/buttons.c
>
> diff --git a/arch/mips/bcm47xx/Makefile b/arch/mips/bcm47xx/Makefile
> index 84e9aed..006a05e 100644
> --- a/arch/mips/bcm47xx/Makefile
> +++ b/arch/mips/bcm47xx/Makefile
> @@ -4,5 +4,5 @@
> #
>
> obj-y += irq.o nvram.o prom.o serial.o setup.o time.o sprom.o
> -obj-y += board.o leds.o
> +obj-y += board.o buttons.o leds.o
> obj-$(CONFIG_BCM47XX_SSB) += wgt634u.o
> diff --git a/arch/mips/bcm47xx/bcm47xx_private.h b/arch/mips/bcm47xx/bcm47xx_private.h
> index 1a1e600..5c94ace 100644
> --- a/arch/mips/bcm47xx/bcm47xx_private.h
> +++ b/arch/mips/bcm47xx/bcm47xx_private.h
> @@ -3,6 +3,9 @@
>
> #include <linux/kernel.h>
>
> +/* buttons.c */
> +int __init bcm47xx_buttons_register(void);
> +
> /* leds.c */
> void __init bcm47xx_leds_register(void);
>
> diff --git a/arch/mips/bcm47xx/buttons.c b/arch/mips/bcm47xx/buttons.c
> new file mode 100644
> index 0000000..3138f03
> --- /dev/null
> +++ b/arch/mips/bcm47xx/buttons.c
> @@ -0,0 +1,95 @@
> +#include "bcm47xx_private.h"
> +
> +#include <linux/input.h>
> +#include <linux/gpio_keys.h>
> +#include <linux/interrupt.h>
> +#include <linux/ssb/ssb_embedded.h>
> +#include <bcm47xx_board.h>
> +#include <bcm47xx.h>
> +
> +/**************************************************
> + * Database
> + **************************************************/
> +
> +static struct gpio_keys_button
> +bcm47xx_buttons_netgear_wndr4500_v1[] __initconst = {
> + {
> + .code = KEY_WPS_BUTTON,
> + .gpio = 4,
> + .active_low = 1,
> + },
> + {
> + .code = KEY_RFKILL,
> + .gpio = 5,
> + .active_low = 1,
> + },
> + {
> + .code = KEY_RESTART,
> + .gpio = 6,
> + .active_low = 1,
> + },
> +};
> +
> +/**************************************************
> + * Init
> + **************************************************/
> +
> +static struct gpio_keys_platform_data bcm47xx_button_pdata;
> +
> +static struct platform_device bcm47xx_buttons_gpio_keys = {
> + .name = "gpio-keys",
> + .dev = {
> + .platform_data = &bcm47xx_button_pdata,
> + }
> +};
> +
> +/* Copy data from __initconst */
> +static int __init bcm47xx_buttons_copy(struct gpio_keys_button *buttons,
> + size_t nbuttons)
> +{
> + size_t size = nbuttons * sizeof(*buttons);
> +
> + bcm47xx_button_pdata.buttons = kmalloc(size, GFP_KERNEL);
> + if (!bcm47xx_button_pdata.buttons)
> + return -ENOMEM;
> + memcpy(bcm47xx_button_pdata.buttons, buttons, size);
> + bcm47xx_button_pdata.nbuttons = nbuttons;
> +
> + return 0;
> +}
> +
> +#define bcm47xx_copy_bdata(dev_buttons) \
> + bcm47xx_buttons_copy(dev_buttons, ARRAY_SIZE(dev_buttons));
> +
> +int __init bcm47xx_buttons_register(void)
> +{
> + enum bcm47xx_board board = bcm47xx_board_get();
> + int err;
> +
> +#ifdef CONFIG_BCM47XX_SSB
> + if (bcm47xx_bus_type == BCM47XX_BUS_TYPE_SSB) {
> + pr_debug("Buttons on SSB are not supported yet.\n");
> + return -ENOTSUPP;
> + }
> +#endif
> +
> + switch (board) {
> + case BCM47XX_BOARD_NETGEAR_WNDR4500V1:
> + err = bcm47xx_copy_bdata(bcm47xx_buttons_netgear_wndr4500_v1);
> + break;
> + default:
> + pr_debug("No buttons configuration found for this device\n");
> + return -ENOTSUPP;
> + }
> +
> + if (err)
> + return -ENOMEM;
> +
> + err = platform_device_register(&bcm47xx_buttons_gpio_keys);
> + if (err) {
> + pr_err("Failed to register platform device: %d\n", err);
> + return err;
> + }
> +
> + return 0;
> +}
> diff --git a/arch/mips/bcm47xx/setup.c b/arch/mips/bcm47xx/setup.c
> index 7e61c0b..a791124 100644
> --- a/arch/mips/bcm47xx/setup.c
> +++ b/arch/mips/bcm47xx/setup.c
> @@ -242,6 +242,7 @@ static int __init bcm47xx_register_bus_complete(void)
> #endif
> }
>
> + bcm47xx_buttons_register();
> bcm47xx_leds_register();
>
> return 0;
>
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH V4] MIPS: BCM47XX: Prepare support for GPIO buttons
2013-12-10 15:24 ` [PATCH V3 " Rafał Miłecki
2013-12-11 21:56 ` Hauke Mehrtens
@ 2014-01-02 12:31 ` Rafał Miłecki
1 sibling, 0 replies; 20+ messages in thread
From: Rafał Miłecki @ 2014-01-02 12:31 UTC (permalink / raw)
To: linux-mips, Ralf Baechle; +Cc: Hauke Mehrtens, Rafał Miłecki
So far this adds support for one Netgear model only, but it's designed
and ready to add many more device. We could hopefully import database
from OpenWrt.
Support for SSB is currently disabled, because SSB doesn't implement IRQ
domain yet.
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
Acked-by: Hauke Mehrtens <hauke@hauke-m.de>
---
V3: Use __initconst and copy data for detected device. This will allow
us to free some memory after init.
V4: Use const for bcm47xx_buttons_netgear_wndr4500_v1
Rebase on top of linux-john.git mips-next-3.14
---
arch/mips/bcm47xx/Makefile | 2 +-
arch/mips/bcm47xx/bcm47xx_private.h | 3 ++
arch/mips/bcm47xx/buttons.c | 95 +++++++++++++++++++++++++++++++++++
arch/mips/bcm47xx/setup.c | 1 +
4 files changed, 100 insertions(+), 1 deletion(-)
create mode 100644 arch/mips/bcm47xx/buttons.c
diff --git a/arch/mips/bcm47xx/Makefile b/arch/mips/bcm47xx/Makefile
index 84e9aed..006a05e 100644
--- a/arch/mips/bcm47xx/Makefile
+++ b/arch/mips/bcm47xx/Makefile
@@ -4,5 +4,5 @@
#
obj-y += irq.o nvram.o prom.o serial.o setup.o time.o sprom.o
-obj-y += board.o leds.o
+obj-y += board.o buttons.o leds.o
obj-$(CONFIG_BCM47XX_SSB) += wgt634u.o
diff --git a/arch/mips/bcm47xx/bcm47xx_private.h b/arch/mips/bcm47xx/bcm47xx_private.h
index 1a1e600..5c94ace 100644
--- a/arch/mips/bcm47xx/bcm47xx_private.h
+++ b/arch/mips/bcm47xx/bcm47xx_private.h
@@ -3,6 +3,9 @@
#include <linux/kernel.h>
+/* buttons.c */
+int __init bcm47xx_buttons_register(void);
+
/* leds.c */
void __init bcm47xx_leds_register(void);
diff --git a/arch/mips/bcm47xx/buttons.c b/arch/mips/bcm47xx/buttons.c
new file mode 100644
index 0000000..d93711b
--- /dev/null
+++ b/arch/mips/bcm47xx/buttons.c
@@ -0,0 +1,95 @@
+#include "bcm47xx_private.h"
+
+#include <linux/input.h>
+#include <linux/gpio_keys.h>
+#include <linux/interrupt.h>
+#include <linux/ssb/ssb_embedded.h>
+#include <bcm47xx_board.h>
+#include <bcm47xx.h>
+
+/**************************************************
+ * Database
+ **************************************************/
+
+static const struct gpio_keys_button
+bcm47xx_buttons_netgear_wndr4500_v1[] __initconst = {
+ {
+ .code = KEY_WPS_BUTTON,
+ .gpio = 4,
+ .active_low = 1,
+ },
+ {
+ .code = KEY_RFKILL,
+ .gpio = 5,
+ .active_low = 1,
+ },
+ {
+ .code = KEY_RESTART,
+ .gpio = 6,
+ .active_low = 1,
+ },
+};
+
+/**************************************************
+ * Init
+ **************************************************/
+
+static struct gpio_keys_platform_data bcm47xx_button_pdata;
+
+static struct platform_device bcm47xx_buttons_gpio_keys = {
+ .name = "gpio-keys",
+ .dev = {
+ .platform_data = &bcm47xx_button_pdata,
+ }
+};
+
+/* Copy data from __initconst */
+static int __init bcm47xx_buttons_copy(const struct gpio_keys_button *buttons,
+ size_t nbuttons)
+{
+ size_t size = nbuttons * sizeof(*buttons);
+
+ bcm47xx_button_pdata.buttons = kmalloc(size, GFP_KERNEL);
+ if (!bcm47xx_button_pdata.buttons)
+ return -ENOMEM;
+ memcpy(bcm47xx_button_pdata.buttons, buttons, size);
+ bcm47xx_button_pdata.nbuttons = nbuttons;
+
+ return 0;
+}
+
+#define bcm47xx_copy_bdata(dev_buttons) \
+ bcm47xx_buttons_copy(dev_buttons, ARRAY_SIZE(dev_buttons));
+
+int __init bcm47xx_buttons_register(void)
+{
+ enum bcm47xx_board board = bcm47xx_board_get();
+ int err;
+
+#ifdef CONFIG_BCM47XX_SSB
+ if (bcm47xx_bus_type == BCM47XX_BUS_TYPE_SSB) {
+ pr_debug("Buttons on SSB are not supported yet.\n");
+ return -ENOTSUPP;
+ }
+#endif
+
+ switch (board) {
+ case BCM47XX_BOARD_NETGEAR_WNDR4500V1:
+ err = bcm47xx_copy_bdata(bcm47xx_buttons_netgear_wndr4500_v1);
+ break;
+ default:
+ pr_debug("No buttons configuration found for this device\n");
+ return -ENOTSUPP;
+ }
+
+ if (err)
+ return -ENOMEM;
+
+ err = platform_device_register(&bcm47xx_buttons_gpio_keys);
+ if (err) {
+ pr_err("Failed to register platform device: %d\n", err);
+ return err;
+ }
+
+ return 0;
+}
diff --git a/arch/mips/bcm47xx/setup.c b/arch/mips/bcm47xx/setup.c
index bd84473..0bd4702 100644
--- a/arch/mips/bcm47xx/setup.c
+++ b/arch/mips/bcm47xx/setup.c
@@ -265,6 +265,7 @@ static int __init bcm47xx_register_bus_complete(void)
#endif
}
+ bcm47xx_buttons_register();
bcm47xx_leds_register();
fixed_phy_add(PHY_POLL, 0, &bcm47xx_fixed_phy_status);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH V2 1/2] bcma: gpio: add own IRQ domain
2013-11-29 16:09 [PATCH V2 1/2] bcma: gpio: add own IRQ domain Rafał Miłecki
2013-11-29 16:09 ` [PATCH V2 2/2] MIPS: BCM47XX: Prepare support for GPIO buttons Rafał Miłecki
@ 2013-11-29 16:31 ` Hauke Mehrtens
2013-11-29 16:55 ` Rafał Miłecki
2013-11-29 17:48 ` [PATCH V3 " Rafał Miłecki
2 siblings, 1 reply; 20+ messages in thread
From: Hauke Mehrtens @ 2013-11-29 16:31 UTC (permalink / raw)
To: Rafał Miłecki; +Cc: linux-mips, Ralf Baechle
On 11/29/2013 05:09 PM, Rafał Miłecki wrote:
> Input GPIO changes can generate interrupts, but we need kind of ACK for
> them by changing IRQ polarity. This is required to stop hardware from
> keep generating interrupts and generate another one on the next GPIO
> state change.
> This code allows using GPIOs with standard interrupts and add for
> example GPIO buttons support.
As far as I know IRQs for GPIOs are only support on SoCs and not on PCIe
wifi cards like the BCM43224. I think the dependency to IRQ_DOMAIN
should only be added when BCMA_HOST_SOC is set.
Hauke
>
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
> drivers/bcma/Kconfig | 1 +
> drivers/bcma/driver_gpio.c | 92 ++++++++++++++++++++++++++-
> include/linux/bcma/bcma_driver_chipcommon.h | 1 +
> 3 files changed, 92 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bcma/Kconfig b/drivers/bcma/Kconfig
> index 7c081b3..649568e 100644
> --- a/drivers/bcma/Kconfig
> +++ b/drivers/bcma/Kconfig
> @@ -75,6 +75,7 @@ config BCMA_DRIVER_GMAC_CMN
> config BCMA_DRIVER_GPIO
> bool "BCMA GPIO driver"
> depends on BCMA && GPIOLIB
> + select IRQ_DOMAIN
> 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 45f0996..0abf09a 100644
> --- a/drivers/bcma/driver_gpio.c
> +++ b/drivers/bcma/driver_gpio.c
> @@ -9,6 +9,9 @@
> */
>
> #include <linux/gpio.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
> +#include <linux/irqdomain.h>
> #include <linux/export.h>
> #include <linux/bcma/bcma.h>
>
> @@ -78,15 +81,60 @@ 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 bcma_core_irq(cc->core);
> + 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);
> + int gpio = irqd_to_hwirq(d);
> +
> + bcma_chipco_gpio_intmask(cc, BIT(gpio), BIT(gpio));
> +}
> +
> +static void bcma_gpio_irq_mask(struct irq_data *d)
> +{
> + struct bcma_drv_cc *cc = irq_data_get_irq_chip_data(d);
> + int gpio = irqd_to_hwirq(d);
> +
> + bcma_chipco_gpio_intmask(cc, BIT(gpio), 0);
> +}
> +
> +static struct irq_chip bcma_gpio_irq_chip = {
> + .name = "BCMA-GPIO",
> + .irq_mask = bcma_gpio_irq_mask,
> + .irq_unmask = bcma_gpio_irq_unmask,
> +};
> +
> +static irqreturn_t bcma_gpio_irq_handler(int irq, void *dev_id)
> +{
> + struct bcma_drv_cc *cc = dev_id;
> + 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);
> + u32 irqs = (val ^ pol) & mask;
> + int gpio;
> +
> + for_each_set_bit(gpio, (unsigned long *)&irqs, cc->gpio.ngpio) {
> + generic_handle_irq(bcma_gpio_to_irq(&cc->gpio, gpio));
> + bcma_chipco_gpio_polarity(cc, BIT(gpio),
> + (val & BIT(gpio)) ? BIT(gpio) : 0);
What about setting this once for all irqs at the end of this function?
bcma_chipco_gpio_polarity() takes an lock.
> + }
> +
> + return irqs ? IRQ_HANDLED : IRQ_NONE;
> +}
> +
> int bcma_gpio_init(struct bcma_drv_cc *cc)
> {
> struct gpio_chip *chip = &cc->gpio;
> + unsigned int hwirq, gpio;
> + int err;
>
> + /*
> + * GPIO chip
> + */
> chip->label = "bcma_gpio";
> chip->owner = THIS_MODULE;
> chip->request = bcma_gpio_request;
> @@ -104,8 +152,48 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
> chip->base = 0;
> else
> chip->base = -1;
> + err = gpiochip_add(chip);
> + if (err)
> + goto err_gpiochip_add;
Shouldn't the gpio chip be registered after we set up the irq handling
so no one uses it before?
> +
> + /*
> + * IRQ domain
> + */
> + 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);
> + }
Is there some method needed to free or unregister these functions? This
could be build as an module when it is not used on a SoC.
> +
> + /*
> + * IRQ handler
> + */
> + hwirq = bcma_core_irq(cc->core);
> + err = request_irq(hwirq, bcma_gpio_irq_handler, IRQF_SHARED, "gpio",
> + cc);
> + if (err)
> + goto err_req_irq;
> +
> + bcma_cc_set32(cc, BCMA_CC_IRQMASK, BCMA_CC_IRQ_GPIO);
> +
> + return 0;
>
> - return gpiochip_add(chip);
> +err_req_irq:
> + irq_domain_remove(cc->irq_domain);
> +err_irq_domain:
> + err = gpiochip_remove(&cc->gpio);
> + if (err)
> + pr_err("Failed to remove GPIO chip: %d\n", err);
> +err_gpiochip_add:
> + return err;
> }
>
> int bcma_gpio_unregister(struct bcma_drv_cc *cc)
> diff --git a/include/linux/bcma/bcma_driver_chipcommon.h b/include/linux/bcma/bcma_driver_chipcommon.h
> index c49e1a1..63d105c 100644
> --- a/include/linux/bcma/bcma_driver_chipcommon.h
> +++ b/include/linux/bcma/bcma_driver_chipcommon.h
> @@ -640,6 +640,7 @@ struct bcma_drv_cc {
> spinlock_t gpio_lock;
> #ifdef CONFIG_BCMA_DRIVER_GPIO
> struct gpio_chip gpio;
> + struct irq_domain *irq_domain;
> #endif
> };
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH V2 1/2] bcma: gpio: add own IRQ domain
2013-11-29 16:31 ` [PATCH V2 1/2] bcma: gpio: add own IRQ domain Hauke Mehrtens
@ 2013-11-29 16:55 ` Rafał Miłecki
0 siblings, 0 replies; 20+ messages in thread
From: Rafał Miłecki @ 2013-11-29 16:55 UTC (permalink / raw)
To: Hauke Mehrtens; +Cc: linux-mips@linux-mips.org, Ralf Baechle
2013/11/29 Hauke Mehrtens <hauke@hauke-m.de>:
> On 11/29/2013 05:09 PM, Rafał Miłecki wrote:
>> Input GPIO changes can generate interrupts, but we need kind of ACK for
>> them by changing IRQ polarity. This is required to stop hardware from
>> keep generating interrupts and generate another one on the next GPIO
>> state change.
>> This code allows using GPIOs with standard interrupts and add for
>> example GPIO buttons support.
>
> As far as I know IRQs for GPIOs are only support on SoCs and not on PCIe
> wifi cards like the BCM43224. I think the dependency to IRQ_DOMAIN
> should only be added when BCMA_HOST_SOC is set.
Sounds sane.
>> + for_each_set_bit(gpio, (unsigned long *)&irqs, cc->gpio.ngpio) {
>> + generic_handle_irq(bcma_gpio_to_irq(&cc->gpio, gpio));
>> + bcma_chipco_gpio_polarity(cc, BIT(gpio),
>> + (val & BIT(gpio)) ? BIT(gpio) : 0);
>
> What about setting this once for all irqs at the end of this function?
> bcma_chipco_gpio_polarity() takes an lock.
OK
>> int bcma_gpio_init(struct bcma_drv_cc *cc)
>> {
>> struct gpio_chip *chip = &cc->gpio;
>> + unsigned int hwirq, gpio;
>> + int err;
>>
>> + /*
>> + * GPIO chip
>> + */
>> chip->label = "bcma_gpio";
>> chip->owner = THIS_MODULE;
>> chip->request = bcma_gpio_request;
>> @@ -104,8 +152,48 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
>> chip->base = 0;
>> else
>> chip->base = -1;
>> + err = gpiochip_add(chip);
>> + if (err)
>> + goto err_gpiochip_add;
>
> Shouldn't the gpio chip be registered after we set up the irq handling
> so no one uses it before?
I don't know.
gpio-tegra.c calls functions in this order:
1) gpiochip_add
2) irq_create_mapping3
3) irq_set_chip_and_handler
4) irq_set_chained_handler
gpio-msm-v2:
1) gpiochip_add
2) irq_domain_add_linear
3) irq_set_chained_handler
gpio-mpc8xxx.c: like above
gpio-adnp.c on the other hand:
1) irq_domain_add_linear
2) request_threaded_irq
3) gpiochip_add
gpio-grgpio.c:
1) irq_domain_add_linear
2) gpiochip_add
So that differs pretty much between the drivers.
But I think calling "gpiochip_add" at the end makes some sense. I
guess I was looking at gpio-tegra.c too much.
>> + 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);
>> + }
>
> Is there some method needed to free or unregister these functions? This
> could be build as an module when it is not used on a SoC.
Yes: irq_dispose_mapping (it calls irq_set_chip_and_handler with NULL
and NULL for us). I'll use it.
--
Rafał
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH V3 1/2] bcma: gpio: add own IRQ domain
2013-11-29 16:09 [PATCH V2 1/2] bcma: gpio: add own IRQ domain Rafał Miłecki
2013-11-29 16:09 ` [PATCH V2 2/2] MIPS: BCM47XX: Prepare support for GPIO buttons Rafał Miłecki
2013-11-29 16:31 ` [PATCH V2 1/2] bcma: gpio: add own IRQ domain Hauke Mehrtens
@ 2013-11-29 17:48 ` Rafał Miłecki
2013-11-29 18:37 ` Hauke Mehrtens
2013-11-29 19:12 ` [PATCH V4 " Rafał Miłecki
2 siblings, 2 replies; 20+ messages in thread
From: Rafał Miłecki @ 2013-11-29 17:48 UTC (permalink / raw)
To: linux-mips, Ralf Baechle; +Cc: Hauke Mehrtens, Rafał Miłecki
Input GPIO changes can generate interrupts, but we need kind of ACK for
them by changing IRQ polarity. This is required to stop hardware from
keep generating interrupts and generate another one on the next GPIO
state change.
This code allows using GPIOs with standard interrupts and add for
example GPIO buttons support.
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
V3: Apply Hauke's comments.
1) Use IRQ domain for CONFIG_BCMA_HOST_SOC only
2) Optimize bcma_gpio_irq_handler
3) Register GPIO chip after doing everything else
4) Improve cleaning paths
---
drivers/bcma/Kconfig | 1 +
drivers/bcma/driver_gpio.c | 126 ++++++++++++++++++++++++++-
include/linux/bcma/bcma_driver_chipcommon.h | 1 +
3 files changed, 126 insertions(+), 2 deletions(-)
diff --git a/drivers/bcma/Kconfig b/drivers/bcma/Kconfig
index 7c081b3..0ee48be 100644
--- a/drivers/bcma/Kconfig
+++ b/drivers/bcma/Kconfig
@@ -75,6 +75,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
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 45f0996..fe9b1bc 100644
--- a/drivers/bcma/driver_gpio.c
+++ b/drivers/bcma/driver_gpio.c
@@ -9,6 +9,9 @@
*/
#include <linux/gpio.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/irqdomain.h>
#include <linux/export.h>
#include <linux/bcma/bcma.h>
@@ -73,19 +76,117 @@ static void bcma_gpio_free(struct gpio_chip *chip, unsigned gpio)
bcma_chipco_gpio_pullup(cc, 1 << gpio, 0);
}
+#ifdef CONFIG_BCMA_HOST_SOC
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 bcma_core_irq(cc->core);
+ 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);
+ int gpio = irqd_to_hwirq(d);
+
+ bcma_chipco_gpio_intmask(cc, BIT(gpio), BIT(gpio));
+}
+
+static void bcma_gpio_irq_mask(struct irq_data *d)
+{
+ struct bcma_drv_cc *cc = irq_data_get_irq_chip_data(d);
+ int gpio = irqd_to_hwirq(d);
+
+ bcma_chipco_gpio_intmask(cc, BIT(gpio), 0);
+}
+
+static struct irq_chip bcma_gpio_irq_chip = {
+ .name = "BCMA-GPIO",
+ .irq_mask = bcma_gpio_irq_mask,
+ .irq_unmask = bcma_gpio_irq_unmask,
+};
+
+static irqreturn_t bcma_gpio_irq_handler(int irq, void *dev_id)
+{
+ struct bcma_drv_cc *cc = dev_id;
+ 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);
+ u32 irqs = (val ^ pol) & mask;
+ u32 pol_mask = 0, pol_val = 0;
+ int gpio;
+
+ for_each_set_bit(gpio, (unsigned long *)&irqs, cc->gpio.ngpio) {
+ generic_handle_irq(bcma_gpio_to_irq(&cc->gpio, gpio));
+ pol_mask |= BIT(gpio);
+ if (val & BIT(gpio))
+ pol_val |= BIT(gpio);
+ }
+ bcma_chipco_gpio_polarity(cc, pol_mask, pol_val);
+
+ return irqs ? IRQ_HANDLED : IRQ_NONE;
+}
+
+static int bcma_gpio_irq_domain_init(struct bcma_drv_cc *cc)
+{
+ struct gpio_chip *chip = &cc->gpio;
+ int gpio, hwirq, err;
+
+ 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);
+ err = request_irq(hwirq, bcma_gpio_irq_handler, IRQF_SHARED, "gpio",
+ cc);
+ if (err)
+ goto err_req_irq;
+
+ 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);
+ }
+ irq_domain_remove(cc->irq_domain);
+err_irq_domain:
+ return err;
+}
+
+static void bcma_gpio_irq_domain_exit(struct bcma_drv_cc *cc)
+{
+ struct gpio_chip *chip = &cc->gpio;
+ int gpio;
+
+ free_irq(bcma_core_irq(cc->core), 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);
+}
+#endif
+
int bcma_gpio_init(struct bcma_drv_cc *cc)
{
struct gpio_chip *chip = &cc->gpio;
+ int err;
chip->label = "bcma_gpio";
chip->owner = THIS_MODULE;
@@ -95,7 +196,9 @@ 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;
+#ifdef CONFIG_BCMA_HOST_SOC
chip->to_irq = bcma_gpio_to_irq;
+#endif
chip->ngpio = 16;
/* There is just one SoC in one device and its GPIO addresses should be
* deterministic to address them more easily. The other buses could get
@@ -105,10 +208,29 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
else
chip->base = -1;
- return gpiochip_add(chip);
+#ifdef CONFIG_BCMA_HOST_SOC
+ err = bcma_gpio_irq_domain_init(cc);
+ if (err)
+ return err;
+#endif
+
+ err = gpiochip_add(chip);
+ if (err) {
+#ifdef CONFIG_BCMA_HOST_SOC
+ bcma_gpio_irq_domain_exit(cc);
+#endif
+ return err;
+ }
+
+ bcma_cc_set32(cc, BCMA_CC_IRQMASK, BCMA_CC_IRQ_GPIO);
+
+ return 0;
}
int bcma_gpio_unregister(struct bcma_drv_cc *cc)
{
+#ifdef CONFIG_BCMA_HOST_SOC
+ bcma_gpio_irq_domain_exit(cc);
+#endif
return gpiochip_remove(&cc->gpio);
}
diff --git a/include/linux/bcma/bcma_driver_chipcommon.h b/include/linux/bcma/bcma_driver_chipcommon.h
index c49e1a1..63d105c 100644
--- a/include/linux/bcma/bcma_driver_chipcommon.h
+++ b/include/linux/bcma/bcma_driver_chipcommon.h
@@ -640,6 +640,7 @@ struct bcma_drv_cc {
spinlock_t gpio_lock;
#ifdef CONFIG_BCMA_DRIVER_GPIO
struct gpio_chip gpio;
+ struct irq_domain *irq_domain;
#endif
};
--
1.7.10.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V3 1/2] bcma: gpio: add own IRQ domain
2013-11-29 17:48 ` [PATCH V3 " Rafał Miłecki
@ 2013-11-29 18:37 ` Hauke Mehrtens
2013-11-29 19:12 ` [PATCH V4 " Rafał Miłecki
1 sibling, 0 replies; 20+ messages in thread
From: Hauke Mehrtens @ 2013-11-29 18:37 UTC (permalink / raw)
To: Rafał Miłecki; +Cc: linux-mips, Ralf Baechle
On 11/29/2013 06:48 PM, Rafał Miłecki wrote:
> Input GPIO changes can generate interrupts, but we need kind of ACK for
> them by changing IRQ polarity. This is required to stop hardware from
> keep generating interrupts and generate another one on the next GPIO
> state change.
> This code allows using GPIOs with standard interrupts and add for
> example GPIO buttons support.
>
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
> V3: Apply Hauke's comments.
> 1) Use IRQ domain for CONFIG_BCMA_HOST_SOC only
> 2) Optimize bcma_gpio_irq_handler
> 3) Register GPIO chip after doing everything else
> 4) Improve cleaning paths
> ---
> drivers/bcma/Kconfig | 1 +
> drivers/bcma/driver_gpio.c | 126 ++++++++++++++++++++++++++-
> include/linux/bcma/bcma_driver_chipcommon.h | 1 +
> 3 files changed, 126 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bcma/Kconfig b/drivers/bcma/Kconfig
> index 7c081b3..0ee48be 100644
> --- a/drivers/bcma/Kconfig
> +++ b/drivers/bcma/Kconfig
> @@ -75,6 +75,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
> 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 45f0996..fe9b1bc 100644
> --- a/drivers/bcma/driver_gpio.c
> +++ b/drivers/bcma/driver_gpio.c
> @@ -9,6 +9,9 @@
> */
>
> #include <linux/gpio.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
> +#include <linux/irqdomain.h>
> #include <linux/export.h>
> #include <linux/bcma/bcma.h>
>
> @@ -73,19 +76,117 @@ static void bcma_gpio_free(struct gpio_chip *chip, unsigned gpio)
> bcma_chipco_gpio_pullup(cc, 1 << gpio, 0);
> }
>
> +#ifdef CONFIG_BCMA_HOST_SOC
> 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 bcma_core_irq(cc->core);
> + 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);
> + int gpio = irqd_to_hwirq(d);
> +
> + bcma_chipco_gpio_intmask(cc, BIT(gpio), BIT(gpio));
> +}
> +
> +static void bcma_gpio_irq_mask(struct irq_data *d)
> +{
> + struct bcma_drv_cc *cc = irq_data_get_irq_chip_data(d);
> + int gpio = irqd_to_hwirq(d);
> +
> + bcma_chipco_gpio_intmask(cc, BIT(gpio), 0);
> +}
> +
> +static struct irq_chip bcma_gpio_irq_chip = {
> + .name = "BCMA-GPIO",
> + .irq_mask = bcma_gpio_irq_mask,
> + .irq_unmask = bcma_gpio_irq_unmask,
> +};
> +
> +static irqreturn_t bcma_gpio_irq_handler(int irq, void *dev_id)
> +{
> + struct bcma_drv_cc *cc = dev_id;
> + 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);
> + u32 irqs = (val ^ pol) & mask;
> + u32 pol_mask = 0, pol_val = 0;
> + int gpio;
> +
> + for_each_set_bit(gpio, (unsigned long *)&irqs, cc->gpio.ngpio) {
> + generic_handle_irq(bcma_gpio_to_irq(&cc->gpio, gpio));
> + pol_mask |= BIT(gpio);
> + if (val & BIT(gpio))
> + pol_val |= BIT(gpio);
> + }
Instead of calculation pol_mask and pol_val bit for bit manually, you
can pretty easy calculate valid value like this:
pol_mask = irqs;
pol_val = val & irqs;
> + bcma_chipco_gpio_polarity(cc, pol_mask, pol_val);
> +
> + return irqs ? IRQ_HANDLED : IRQ_NONE;
> +}
> +
> +static int bcma_gpio_irq_domain_init(struct bcma_drv_cc *cc)
> +{
> + struct gpio_chip *chip = &cc->gpio;
> + int gpio, hwirq, err;
> +
> + 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);
> + err = request_irq(hwirq, bcma_gpio_irq_handler, IRQF_SHARED, "gpio",
> + cc);
> + if (err)
> + goto err_req_irq;
> +
> + 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);
> + }
> + irq_domain_remove(cc->irq_domain);
> +err_irq_domain:
> + return err;
> +}
> +
> +static void bcma_gpio_irq_domain_exit(struct bcma_drv_cc *cc)
> +{
> + struct gpio_chip *chip = &cc->gpio;
> + int gpio;
> +
> + free_irq(bcma_core_irq(cc->core), 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);
> +}
> +#endif
I do not like all these ifdefs, why not add this and remove some of them
from the code in bcma_gpio_init():
#else /* CONFIG_BCMA_HOST_SOC */
static int bcma_gpio_irq_domain_init(struct bcma_drv_cc *cc)
{
return 0;
}
static void bcma_gpio_irq_domain_exit(struct bcma_drv_cc *cc)
{
/* Nothing */
}
#endif /* CONFIG_BCMA_HOST_SOC */
> +
> int bcma_gpio_init(struct bcma_drv_cc *cc)
> {
> struct gpio_chip *chip = &cc->gpio;
> + int err;
>
> chip->label = "bcma_gpio";
> chip->owner = THIS_MODULE;
> @@ -95,7 +196,9 @@ 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;
> +#ifdef CONFIG_BCMA_HOST_SOC
> chip->to_irq = bcma_gpio_to_irq;
> +#endif
> chip->ngpio = 16;
> /* There is just one SoC in one device and its GPIO addresses should be
> * deterministic to address them more easily. The other buses could get
> @@ -105,10 +208,29 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
> else
> chip->base = -1;
>
> - return gpiochip_add(chip);
> +#ifdef CONFIG_BCMA_HOST_SOC
> + err = bcma_gpio_irq_domain_init(cc);
> + if (err)
> + return err;
> +#endif
> +
> + err = gpiochip_add(chip);
> + if (err) {
> +#ifdef CONFIG_BCMA_HOST_SOC
> + bcma_gpio_irq_domain_exit(cc);
> +#endif
> + return err;
> + }
> +
> + bcma_cc_set32(cc, BCMA_CC_IRQMASK, BCMA_CC_IRQ_GPIO);
This should be moved into bcma_gpio_irq_domain_ini(), I do not know what
happens when we activate them on a PCIe device. IRQs should get
deactivated in bcma_gpio_irq_domain_exit().
> +
> + return 0;
> }
>
> int bcma_gpio_unregister(struct bcma_drv_cc *cc)
> {
> +#ifdef CONFIG_BCMA_HOST_SOC
> + bcma_gpio_irq_domain_exit(cc);
> +#endif
> return gpiochip_remove(&cc->gpio);
> }
> diff --git a/include/linux/bcma/bcma_driver_chipcommon.h b/include/linux/bcma/bcma_driver_chipcommon.h
> index c49e1a1..63d105c 100644
> --- a/include/linux/bcma/bcma_driver_chipcommon.h
> +++ b/include/linux/bcma/bcma_driver_chipcommon.h
> @@ -640,6 +640,7 @@ struct bcma_drv_cc {
> spinlock_t gpio_lock;
> #ifdef CONFIG_BCMA_DRIVER_GPIO
> struct gpio_chip gpio;
> + struct irq_domain *irq_domain;
> #endif
> };
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH V4 1/2] bcma: gpio: add own IRQ domain
2013-11-29 17:48 ` [PATCH V3 " Rafał Miłecki
2013-11-29 18:37 ` Hauke Mehrtens
@ 2013-11-29 19:12 ` Rafał Miłecki
2013-11-29 20:37 ` John Crispin
2013-12-10 11:56 ` [PATCH V5 " Rafał Miłecki
1 sibling, 2 replies; 20+ messages in thread
From: Rafał Miłecki @ 2013-11-29 19:12 UTC (permalink / raw)
To: linux-mips, Ralf Baechle; +Cc: Hauke Mehrtens, Rafał Miłecki
Input GPIO changes can generate interrupts, but we need kind of ACK for
them by changing IRQ polarity. This is required to stop hardware from
keep generating interrupts and generate another one on the next GPIO
state change.
This code allows using GPIOs with standard interrupts and add for
example GPIO buttons support.
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
V3: Apply Hauke's comments.
1) Use IRQ domain for CONFIG_BCMA_HOST_SOC only
2) Optimize bcma_gpio_irq_handler
3) Register GPIO chip after doing everything else
4) Improve cleaning paths
V4: More fixes from Hauke's comments:
1) Less #ifdef CONFIG_BCMA_HOST_SOC
2) Move BCMA_CC_IRQMASK op (and unset on exit)
3) Optimize bcma_gpio_irq_handler
---
drivers/bcma/Kconfig | 1 +
drivers/bcma/driver_gpio.c | 128 ++++++++++++++++++++++++++-
include/linux/bcma/bcma_driver_chipcommon.h | 1 +
3 files changed, 128 insertions(+), 2 deletions(-)
diff --git a/drivers/bcma/Kconfig b/drivers/bcma/Kconfig
index 7c081b3..0ee48be 100644
--- a/drivers/bcma/Kconfig
+++ b/drivers/bcma/Kconfig
@@ -75,6 +75,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
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 45f0996..ae88d5d 100644
--- a/drivers/bcma/driver_gpio.c
+++ b/drivers/bcma/driver_gpio.c
@@ -9,6 +9,9 @@
*/
#include <linux/gpio.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/irqdomain.h>
#include <linux/export.h>
#include <linux/bcma/bcma.h>
@@ -73,19 +76,127 @@ static void bcma_gpio_free(struct gpio_chip *chip, unsigned gpio)
bcma_chipco_gpio_pullup(cc, 1 << gpio, 0);
}
+#ifdef CONFIG_BCMA_HOST_SOC
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 bcma_core_irq(cc->core);
+ 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);
+ int gpio = irqd_to_hwirq(d);
+
+ bcma_chipco_gpio_intmask(cc, BIT(gpio), BIT(gpio));
+}
+
+static void bcma_gpio_irq_mask(struct irq_data *d)
+{
+ struct bcma_drv_cc *cc = irq_data_get_irq_chip_data(d);
+ int gpio = irqd_to_hwirq(d);
+
+ bcma_chipco_gpio_intmask(cc, BIT(gpio), 0);
+}
+
+static struct irq_chip bcma_gpio_irq_chip = {
+ .name = "BCMA-GPIO",
+ .irq_mask = bcma_gpio_irq_mask,
+ .irq_unmask = bcma_gpio_irq_unmask,
+};
+
+static irqreturn_t bcma_gpio_irq_handler(int irq, void *dev_id)
+{
+ struct bcma_drv_cc *cc = dev_id;
+ 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);
+ u32 irqs = (val ^ pol) & mask;
+ int gpio;
+
+ if (!irqs)
+ return IRQ_NONE;
+
+ for_each_set_bit(gpio, (unsigned long *)&irqs, cc->gpio.ngpio)
+ generic_handle_irq(bcma_gpio_to_irq(&cc->gpio, gpio));
+ bcma_chipco_gpio_polarity(cc, irqs, val & irqs);
+
+ return IRQ_HANDLED;
+}
+
+static int bcma_gpio_irq_domain_init(struct bcma_drv_cc *cc)
+{
+ struct gpio_chip *chip = &cc->gpio;
+ int gpio, hwirq, err;
+
+ 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);
+ err = request_irq(hwirq, bcma_gpio_irq_handler, IRQF_SHARED, "gpio",
+ cc);
+ if (err)
+ goto err_req_irq;
+
+ 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);
+ }
+ irq_domain_remove(cc->irq_domain);
+err_irq_domain:
+ return err;
+}
+
+static void bcma_gpio_irq_domain_exit(struct bcma_drv_cc *cc)
+{
+ struct gpio_chip *chip = &cc->gpio;
+ int gpio;
+
+ bcma_cc_mask32(cc, BCMA_CC_IRQMASK, ~BCMA_CC_IRQ_GPIO);
+ free_irq(bcma_core_irq(cc->core), 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)
+{
+ return 0;
+}
+
+static void bcma_gpio_irq_domain_exit(struct bcma_drv_cc *cc)
+{
+}
+#endif
+
int bcma_gpio_init(struct bcma_drv_cc *cc)
{
struct gpio_chip *chip = &cc->gpio;
+ int err;
chip->label = "bcma_gpio";
chip->owner = THIS_MODULE;
@@ -95,7 +206,9 @@ 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;
+#ifdef CONFIG_BCMA_HOST_SOC
chip->to_irq = bcma_gpio_to_irq;
+#endif
chip->ngpio = 16;
/* There is just one SoC in one device and its GPIO addresses should be
* deterministic to address them more easily. The other buses could get
@@ -105,10 +218,21 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
else
chip->base = -1;
- return gpiochip_add(chip);
+ err = bcma_gpio_irq_domain_init(cc);
+ if (err)
+ return err;
+
+ err = gpiochip_add(chip);
+ if (err) {
+ bcma_gpio_irq_domain_exit(cc);
+ return err;
+ }
+
+ return 0;
}
int bcma_gpio_unregister(struct bcma_drv_cc *cc)
{
+ bcma_gpio_irq_domain_exit(cc);
return gpiochip_remove(&cc->gpio);
}
diff --git a/include/linux/bcma/bcma_driver_chipcommon.h b/include/linux/bcma/bcma_driver_chipcommon.h
index c49e1a1..63d105c 100644
--- a/include/linux/bcma/bcma_driver_chipcommon.h
+++ b/include/linux/bcma/bcma_driver_chipcommon.h
@@ -640,6 +640,7 @@ struct bcma_drv_cc {
spinlock_t gpio_lock;
#ifdef CONFIG_BCMA_DRIVER_GPIO
struct gpio_chip gpio;
+ struct irq_domain *irq_domain;
#endif
};
--
1.7.10.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V4 1/2] bcma: gpio: add own IRQ domain
2013-11-29 19:12 ` [PATCH V4 " Rafał Miłecki
@ 2013-11-29 20:37 ` John Crispin
2013-11-29 20:53 ` Rafał Miłecki
2013-12-10 11:56 ` [PATCH V5 " Rafał Miłecki
1 sibling, 1 reply; 20+ messages in thread
From: John Crispin @ 2013-11-29 20:37 UTC (permalink / raw)
To: Rafał Miłecki; +Cc: linux-mips, Ralf Baechle, Hauke Mehrtens
On 29/11/13 20:12, Rafał Miłecki wrote:
> +#ifdef CONFIG_BCMA_HOST_SOC
> chip->to_irq = bcma_gpio_to_irq;
> +#endif
> chip->ngpio = 16;
Hi,
Should this not be
if (IS_ENABLED(CONFIG_BCMA_HOST_SOC))
chip->to_irq = bcma_gpio_to_irq;
John
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V4 1/2] bcma: gpio: add own IRQ domain
2013-11-29 20:37 ` John Crispin
@ 2013-11-29 20:53 ` Rafał Miłecki
2013-11-29 21:16 ` John Crispin
0 siblings, 1 reply; 20+ messages in thread
From: Rafał Miłecki @ 2013-11-29 20:53 UTC (permalink / raw)
To: John Crispin; +Cc: linux-mips@linux-mips.org, Ralf Baechle, Hauke Mehrtens
2013/11/29 John Crispin <john@phrozen.org>:
> On 29/11/13 20:12, Rafał Miłecki wrote:
>>
>> +#ifdef CONFIG_BCMA_HOST_SOC
>> chip->to_irq = bcma_gpio_to_irq;
>> +#endif
>> chip->ngpio = 16;
>
>
>
> Hi,
>
> Should this not be
>
> if (IS_ENABLED(CONFIG_BCMA_HOST_SOC))
> chip->to_irq = bcma_gpio_to_irq;
I can't find a proper documentation about that. It's definitely nicer to use
#if IS_ENABLED(FOO)
instead of
#if defined(FOO) || defined(FOO_MODULE)
But are we supposed to use it also for a simple
#if defined(FOO)
?
I tried to Google about this but found only some minor flame-wars ;)
Is that documented anywhere?
--
Rafał
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V4 1/2] bcma: gpio: add own IRQ domain
2013-11-29 20:53 ` Rafał Miłecki
@ 2013-11-29 21:16 ` John Crispin
2013-12-08 18:10 ` Hauke Mehrtens
0 siblings, 1 reply; 20+ messages in thread
From: John Crispin @ 2013-11-29 21:16 UTC (permalink / raw)
To: linux-mips
On 29/11/13 21:53, Rafał Miłecki wrote:
> 2013/11/29 John Crispin<john@phrozen.org>:
>> On 29/11/13 20:12, Rafał Miłecki wrote:
>>>
>>> +#ifdef CONFIG_BCMA_HOST_SOC
>>> chip->to_irq = bcma_gpio_to_irq;
>>> +#endif
>>> chip->ngpio = 16;
>>
>>
>>
>> Hi,
>>
>> Should this not be
>>
>> if (IS_ENABLED(CONFIG_BCMA_HOST_SOC))
>> chip->to_irq = bcma_gpio_to_irq;
>
> I can't find a proper documentation about that. It's definitely nicer to use
> #if IS_ENABLED(FOO)
> instead of
> #if defined(FOO) || defined(FOO_MODULE)
>
> But are we supposed to use it also for a simple
> #if defined(FOO)
> ?
>
> I tried to Google about this but found only some minor flame-wars ;)
>
> Is that documented anywhere?
>
The commit message has the relevant info ...
http://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?id=2a11c8ea20bf850b3a2c60db8c2e7497d28aba99
John
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V4 1/2] bcma: gpio: add own IRQ domain
2013-11-29 21:16 ` John Crispin
@ 2013-12-08 18:10 ` Hauke Mehrtens
2013-12-08 19:24 ` Sergei Shtylyov
0 siblings, 1 reply; 20+ messages in thread
From: Hauke Mehrtens @ 2013-12-08 18:10 UTC (permalink / raw)
To: John Crispin, Rafał Miłecki; +Cc: linux-mips
On 11/29/2013 10:16 PM, John Crispin wrote:
> On 29/11/13 21:53, Rafał Miłecki wrote:
>> 2013/11/29 John Crispin<john@phrozen.org>:
>>> On 29/11/13 20:12, Rafał Miłecki wrote:
>>>>
>>>> +#ifdef CONFIG_BCMA_HOST_SOC
>>>> chip->to_irq = bcma_gpio_to_irq;
>>>> +#endif
>>>> chip->ngpio = 16;
>>>
>>>
>>>
>>> Hi,
>>>
>>> Should this not be
>>>
>>> if (IS_ENABLED(CONFIG_BCMA_HOST_SOC))
>>> chip->to_irq = bcma_gpio_to_irq;
>>
>> I can't find a proper documentation about that. It's definitely nicer
>> to use
>> #if IS_ENABLED(FOO)
>> instead of
>> #if defined(FOO) || defined(FOO_MODULE)
>>
>> But are we supposed to use it also for a simple
>> #if defined(FOO)
>> ?
>>
>> I tried to Google about this but found only some minor flame-wars ;)
>>
>> Is that documented anywhere?
>>
>
>
> The commit message has the relevant info ...
>
> http://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?id=2a11c8ea20bf850b3a2c60db8c2e7497d28aba99
I read this and as far as I understand that when CONFIG_BCMA_HOST_SOC is
bool and not tristate "#ifdef CONFIG_BCMA_HOST_SOC" and "#if
IS_ENABLED(CONFIG_BCMA_HOST_SOC)" will have the same effect?
@Rafał
Acked-by: Hauke Mehrtens <hauke@hauke-m.de>
for the "bcma: gpio: add own IRQ domain" patch.
Hauke
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V4 1/2] bcma: gpio: add own IRQ domain
2013-12-08 18:10 ` Hauke Mehrtens
@ 2013-12-08 19:24 ` Sergei Shtylyov
0 siblings, 0 replies; 20+ messages in thread
From: Sergei Shtylyov @ 2013-12-08 19:24 UTC (permalink / raw)
To: Hauke Mehrtens, John Crispin, Rafał Miłecki; +Cc: linux-mips
Hello.
On 12/08/2013 09:10 PM, Hauke Mehrtens wrote:
>>>>> +#ifdef CONFIG_BCMA_HOST_SOC
>>>>> chip->to_irq = bcma_gpio_to_irq;
>>>>> +#endif
>>>>> chip->ngpio = 16;
>>>> Hi,
>>>> Should this not be
>>>> if (IS_ENABLED(CONFIG_BCMA_HOST_SOC))
>>>> chip->to_irq = bcma_gpio_to_irq;
>>> I can't find a proper documentation about that. It's definitely nicer
>>> to use
>>> #if IS_ENABLED(FOO)
>>> instead of
>>> #if defined(FOO) || defined(FOO_MODULE)
>>> But are we supposed to use it also for a simple
>>> #if defined(FOO)
>>> ?
>>> I tried to Google about this but found only some minor flame-wars ;)
>>> Is that documented anywhere?
>> The commit message has the relevant info ...
>> http://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?id=2a11c8ea20bf850b3a2c60db8c2e7497d28aba99
> I read this and as far as I understand that when CONFIG_BCMA_HOST_SOC is
> bool and not tristate "#ifdef CONFIG_BCMA_HOST_SOC" and "#if
> IS_ENABLED(CONFIG_BCMA_HOST_SOC)" will have the same effect?
You can also use IS_BUILTIN(CONFIG_BCMA_HOST_SOC) to avoid unneeded test
for the modular case.
> Hauke
WBR, Sergei
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH V5 1/2] bcma: gpio: add own IRQ domain
2013-11-29 19:12 ` [PATCH V4 " Rafał Miłecki
2013-11-29 20:37 ` John Crispin
@ 2013-12-10 11:56 ` Rafał Miłecki
2013-12-11 21:53 ` Hauke Mehrtens
2013-12-12 12:42 ` [PATCH V6 " Rafał Miłecki
1 sibling, 2 replies; 20+ messages in thread
From: Rafał Miłecki @ 2013-12-10 11:56 UTC (permalink / raw)
To: linux-mips, Ralf Baechle; +Cc: Hauke Mehrtens, Rafał Miłecki
Input GPIO changes can generate interrupts, but we need kind of ACK for
them by changing IRQ polarity. This is required to stop hardware from
keep generating interrupts and generate another one on the next GPIO
state change.
This code allows using GPIOs with standard interrupts and add for
example GPIO buttons support.
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
Acked-by: Hauke Mehrtens <hauke@hauke-m.de>
---
V3: Apply Hauke's comments.
1) Use IRQ domain for CONFIG_BCMA_HOST_SOC only
2) Optimize bcma_gpio_irq_handler
3) Register GPIO chip after doing everything else
4) Improve cleaning paths
V4: More fixes from Hauke's comments:
1) Less #ifdef CONFIG_BCMA_HOST_SOC
2) Move BCMA_CC_IRQMASK op (and unset on exit)
3) Optimize bcma_gpio_irq_handler
V5: Use IS_BUILTIN
---
drivers/bcma/Kconfig | 1 +
drivers/bcma/driver_gpio.c | 129 ++++++++++++++++++++++++++-
include/linux/bcma/bcma_driver_chipcommon.h | 1 +
3 files changed, 128 insertions(+), 3 deletions(-)
diff --git a/drivers/bcma/Kconfig b/drivers/bcma/Kconfig
index 7c081b3..0ee48be 100644
--- a/drivers/bcma/Kconfig
+++ b/drivers/bcma/Kconfig
@@ -75,6 +75,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
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 45f0996..d5b02d2 100644
--- a/drivers/bcma/driver_gpio.c
+++ b/drivers/bcma/driver_gpio.c
@@ -9,6 +9,9 @@
*/
#include <linux/gpio.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/irqdomain.h>
#include <linux/export.h>
#include <linux/bcma/bcma.h>
@@ -73,19 +76,127 @@ static void bcma_gpio_free(struct gpio_chip *chip, unsigned gpio)
bcma_chipco_gpio_pullup(cc, 1 << gpio, 0);
}
+#if IS_BUILTIN(CONFIG_BCMA_HOST_SOC)
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 bcma_core_irq(cc->core);
+ 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);
+ int gpio = irqd_to_hwirq(d);
+
+ bcma_chipco_gpio_intmask(cc, BIT(gpio), BIT(gpio));
+}
+
+static void bcma_gpio_irq_mask(struct irq_data *d)
+{
+ struct bcma_drv_cc *cc = irq_data_get_irq_chip_data(d);
+ int gpio = irqd_to_hwirq(d);
+
+ bcma_chipco_gpio_intmask(cc, BIT(gpio), 0);
+}
+
+static struct irq_chip bcma_gpio_irq_chip = {
+ .name = "BCMA-GPIO",
+ .irq_mask = bcma_gpio_irq_mask,
+ .irq_unmask = bcma_gpio_irq_unmask,
+};
+
+static irqreturn_t bcma_gpio_irq_handler(int irq, void *dev_id)
+{
+ struct bcma_drv_cc *cc = dev_id;
+ 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);
+ u32 irqs = (val ^ pol) & mask;
+ int gpio;
+
+ if (!irqs)
+ return IRQ_NONE;
+
+ for_each_set_bit(gpio, (unsigned long *)&irqs, cc->gpio.ngpio)
+ generic_handle_irq(bcma_gpio_to_irq(&cc->gpio, gpio));
+ bcma_chipco_gpio_polarity(cc, irqs, val & irqs);
+
+ return IRQ_HANDLED;
+}
+
+static int bcma_gpio_irq_domain_init(struct bcma_drv_cc *cc)
+{
+ struct gpio_chip *chip = &cc->gpio;
+ int gpio, hwirq, err;
+
+ 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);
+ err = request_irq(hwirq, bcma_gpio_irq_handler, IRQF_SHARED, "gpio",
+ cc);
+ if (err)
+ goto err_req_irq;
+
+ 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);
+ }
+ irq_domain_remove(cc->irq_domain);
+err_irq_domain:
+ return err;
+}
+
+static void bcma_gpio_irq_domain_exit(struct bcma_drv_cc *cc)
+{
+ struct gpio_chip *chip = &cc->gpio;
+ int gpio;
+
+ bcma_cc_mask32(cc, BCMA_CC_IRQMASK, ~BCMA_CC_IRQ_GPIO);
+ free_irq(bcma_core_irq(cc->core), 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)
+{
+ return 0;
+}
+
+static void bcma_gpio_irq_domain_exit(struct bcma_drv_cc *cc)
+{
+}
+#endif
+
int bcma_gpio_init(struct bcma_drv_cc *cc)
{
struct gpio_chip *chip = &cc->gpio;
+ int err;
chip->label = "bcma_gpio";
chip->owner = THIS_MODULE;
@@ -95,7 +206,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;
- chip->to_irq = bcma_gpio_to_irq;
+ if (IS_BUILTIN(CONFIG_BCMA_HOST_SOC))
+ chip->to_irq = bcma_gpio_to_irq;
chip->ngpio = 16;
/* There is just one SoC in one device and its GPIO addresses should be
* deterministic to address them more easily. The other buses could get
@@ -105,10 +217,21 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
else
chip->base = -1;
- return gpiochip_add(chip);
+ err = bcma_gpio_irq_domain_init(cc);
+ if (err)
+ return err;
+
+ err = gpiochip_add(chip);
+ if (err) {
+ bcma_gpio_irq_domain_exit(cc);
+ return err;
+ }
+
+ return 0;
}
int bcma_gpio_unregister(struct bcma_drv_cc *cc)
{
+ bcma_gpio_irq_domain_exit(cc);
return gpiochip_remove(&cc->gpio);
}
diff --git a/include/linux/bcma/bcma_driver_chipcommon.h b/include/linux/bcma/bcma_driver_chipcommon.h
index c49e1a1..63d105c 100644
--- a/include/linux/bcma/bcma_driver_chipcommon.h
+++ b/include/linux/bcma/bcma_driver_chipcommon.h
@@ -640,6 +640,7 @@ struct bcma_drv_cc {
spinlock_t gpio_lock;
#ifdef CONFIG_BCMA_DRIVER_GPIO
struct gpio_chip gpio;
+ struct irq_domain *irq_domain;
#endif
};
--
1.7.10.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V5 1/2] bcma: gpio: add own IRQ domain
2013-12-10 11:56 ` [PATCH V5 " Rafał Miłecki
@ 2013-12-11 21:53 ` Hauke Mehrtens
2013-12-12 12:42 ` [PATCH V6 " Rafał Miłecki
1 sibling, 0 replies; 20+ messages in thread
From: Hauke Mehrtens @ 2013-12-11 21:53 UTC (permalink / raw)
To: Rafał Miłecki; +Cc: linux-mips, Ralf Baechle
On 12/10/2013 12:56 PM, Rafał Miłecki wrote:
> Input GPIO changes can generate interrupts, but we need kind of ACK for
> them by changing IRQ polarity. This is required to stop hardware from
> keep generating interrupts and generate another one on the next GPIO
> state change.
> This code allows using GPIOs with standard interrupts and add for
> example GPIO buttons support.
>
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> Acked-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
> V3: Apply Hauke's comments.
> 1) Use IRQ domain for CONFIG_BCMA_HOST_SOC only
> 2) Optimize bcma_gpio_irq_handler
> 3) Register GPIO chip after doing everything else
> 4) Improve cleaning paths
>
> V4: More fixes from Hauke's comments:
> 1) Less #ifdef CONFIG_BCMA_HOST_SOC
> 2) Move BCMA_CC_IRQMASK op (and unset on exit)
> 3) Optimize bcma_gpio_irq_handler
>
> V5: Use IS_BUILTIN
> ---
> drivers/bcma/Kconfig | 1 +
> drivers/bcma/driver_gpio.c | 129 ++++++++++++++++++++++++++-
> include/linux/bcma/bcma_driver_chipcommon.h | 1 +
> 3 files changed, 128 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bcma/Kconfig b/drivers/bcma/Kconfig
> index 7c081b3..0ee48be 100644
> --- a/drivers/bcma/Kconfig
> +++ b/drivers/bcma/Kconfig
> @@ -75,6 +75,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
> 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 45f0996..d5b02d2 100644
> --- a/drivers/bcma/driver_gpio.c
> +++ b/drivers/bcma/driver_gpio.c
> @@ -9,6 +9,9 @@
> */
>
> #include <linux/gpio.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
> +#include <linux/irqdomain.h>
> #include <linux/export.h>
> #include <linux/bcma/bcma.h>
>
> @@ -73,19 +76,127 @@ static void bcma_gpio_free(struct gpio_chip *chip, unsigned gpio)
> bcma_chipco_gpio_pullup(cc, 1 << gpio, 0);
> }
>
> +#if IS_BUILTIN(CONFIG_BCMA_HOST_SOC)
> 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 bcma_core_irq(cc->core);
> + 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);
> + int gpio = irqd_to_hwirq(d);
> +
> + bcma_chipco_gpio_intmask(cc, BIT(gpio), BIT(gpio));
> +}
> +
> +static void bcma_gpio_irq_mask(struct irq_data *d)
> +{
> + struct bcma_drv_cc *cc = irq_data_get_irq_chip_data(d);
> + int gpio = irqd_to_hwirq(d);
> +
> + bcma_chipco_gpio_intmask(cc, BIT(gpio), 0);
> +}
> +
> +static struct irq_chip bcma_gpio_irq_chip = {
> + .name = "BCMA-GPIO",
> + .irq_mask = bcma_gpio_irq_mask,
> + .irq_unmask = bcma_gpio_irq_unmask,
> +};
> +
> +static irqreturn_t bcma_gpio_irq_handler(int irq, void *dev_id)
> +{
> + struct bcma_drv_cc *cc = dev_id;
> + 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);
> + u32 irqs = (val ^ pol) & mask;
> + int gpio;
> +
> + if (!irqs)
> + return IRQ_NONE;
> +
> + for_each_set_bit(gpio, (unsigned long *)&irqs, cc->gpio.ngpio)
> + generic_handle_irq(bcma_gpio_to_irq(&cc->gpio, gpio));
> + bcma_chipco_gpio_polarity(cc, irqs, val & irqs);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int bcma_gpio_irq_domain_init(struct bcma_drv_cc *cc)
> +{
> + struct gpio_chip *chip = &cc->gpio;
> + int gpio, hwirq, err;
> +
> + 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);
> + err = request_irq(hwirq, bcma_gpio_irq_handler, IRQF_SHARED, "gpio",
> + cc);
> + if (err)
> + goto err_req_irq;
> +
> + 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);
> + }
> + irq_domain_remove(cc->irq_domain);
> +err_irq_domain:
> + return err;
> +}
> +
> +static void bcma_gpio_irq_domain_exit(struct bcma_drv_cc *cc)
> +{
> + struct gpio_chip *chip = &cc->gpio;
> + int gpio;
> +
> + bcma_cc_mask32(cc, BCMA_CC_IRQMASK, ~BCMA_CC_IRQ_GPIO);
> + free_irq(bcma_core_irq(cc->core), 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)
> +{
> + return 0;
> +}
> +
> +static void bcma_gpio_irq_domain_exit(struct bcma_drv_cc *cc)
> +{
> +}
> +#endif
> +
> int bcma_gpio_init(struct bcma_drv_cc *cc)
> {
> struct gpio_chip *chip = &cc->gpio;
> + int err;
>
> chip->label = "bcma_gpio";
> chip->owner = THIS_MODULE;
> @@ -95,7 +206,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;
> - chip->to_irq = bcma_gpio_to_irq;
> + if (IS_BUILTIN(CONFIG_BCMA_HOST_SOC))
> + chip->to_irq = bcma_gpio_to_irq;
> chip->ngpio = 16;
> /* There is just one SoC in one device and its GPIO addresses should be
> * deterministic to address them more easily. The other buses could get
> @@ -105,10 +217,21 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
> else
> chip->base = -1;
>
> - return gpiochip_add(chip);
> + err = bcma_gpio_irq_domain_init(cc);
> + if (err)
> + return err;
bcma_gpio_irq_domain_init() should only be called if
(cc->core->bus->hosttype == BCMA_HOSTTYPE_SOC) because
bcma_gpio_to_irq() will return -EINVAL for any other type anyway and we
will get the error you posted me in jabber.
I do not know how an IRQ in chipcommon on a PCIe card would work and I
do not think GPIO irqs will not be used there.
In bcma_core_irq() in drivers/bcma/driver_mips.c we should check if
drv_mips.core is not null to prevent any such problems in an additional
patch.
> +
> + err = gpiochip_add(chip);
> + if (err) {
> + bcma_gpio_irq_domain_exit(cc);
> + return err;
> + }
> +
> + return 0;
> }
>
> int bcma_gpio_unregister(struct bcma_drv_cc *cc)
> {
> + bcma_gpio_irq_domain_exit(cc);
> return gpiochip_remove(&cc->gpio);
> }
> diff --git a/include/linux/bcma/bcma_driver_chipcommon.h b/include/linux/bcma/bcma_driver_chipcommon.h
> index c49e1a1..63d105c 100644
> --- a/include/linux/bcma/bcma_driver_chipcommon.h
> +++ b/include/linux/bcma/bcma_driver_chipcommon.h
> @@ -640,6 +640,7 @@ struct bcma_drv_cc {
> spinlock_t gpio_lock;
> #ifdef CONFIG_BCMA_DRIVER_GPIO
> struct gpio_chip gpio;
> + struct irq_domain *irq_domain;
> #endif
> };
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH V6 1/2] bcma: gpio: add own IRQ domain
2013-12-10 11:56 ` [PATCH V5 " Rafał Miłecki
2013-12-11 21:53 ` Hauke Mehrtens
@ 2013-12-12 12:42 ` Rafał Miłecki
2013-12-12 12:46 ` [PATCH V7 " Rafał Miłecki
1 sibling, 1 reply; 20+ messages in thread
From: Rafał Miłecki @ 2013-12-12 12:42 UTC (permalink / raw)
To: linux-mips, Ralf Baechle; +Cc: Hauke Mehrtens, Rafał Miłecki
Input GPIO changes can generate interrupts, but we need kind of ACK for
them by changing IRQ polarity. This is required to stop hardware from
keep generating interrupts and generate another one on the next GPIO
state change.
This code allows using GPIOs with standard interrupts and add for
example GPIO buttons support.
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
Acked-by: Hauke Mehrtens <hauke@hauke-m.de>
---
V3: Apply Hauke's comments.
1) Use IRQ domain for CONFIG_BCMA_HOST_SOC only
2) Optimize bcma_gpio_irq_handler
3) Register GPIO chip after doing everything else
4) Improve cleaning paths
V4: More fixes from Hauke's comments:
1) Less #ifdef CONFIG_BCMA_HOST_SOC
2) Move BCMA_CC_IRQMASK op (and unset on exit)
3) Optimize bcma_gpio_irq_handler
V5: Use IS_BUILTIN
V6: Add IRQ domain on SOC only
Hauke: does it look OK to you?
---
drivers/bcma/Kconfig | 1 +
drivers/bcma/driver_gpio.c | 135 ++++++++++++++++++++++++++-
include/linux/bcma/bcma_driver_chipcommon.h | 1 +
3 files changed, 134 insertions(+), 3 deletions(-)
diff --git a/drivers/bcma/Kconfig b/drivers/bcma/Kconfig
index 7c081b3..0ee48be 100644
--- a/drivers/bcma/Kconfig
+++ b/drivers/bcma/Kconfig
@@ -75,6 +75,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
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 45f0996..fd8ce7d 100644
--- a/drivers/bcma/driver_gpio.c
+++ b/drivers/bcma/driver_gpio.c
@@ -9,6 +9,9 @@
*/
#include <linux/gpio.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/irqdomain.h>
#include <linux/export.h>
#include <linux/bcma/bcma.h>
@@ -73,19 +76,133 @@ static void bcma_gpio_free(struct gpio_chip *chip, unsigned gpio)
bcma_chipco_gpio_pullup(cc, 1 << gpio, 0);
}
+#if IS_BUILTIN(CONFIG_BCMA_HOST_SOC)
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 bcma_core_irq(cc->core);
+ 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);
+ int gpio = irqd_to_hwirq(d);
+
+ bcma_chipco_gpio_intmask(cc, BIT(gpio), BIT(gpio));
+}
+
+static void bcma_gpio_irq_mask(struct irq_data *d)
+{
+ struct bcma_drv_cc *cc = irq_data_get_irq_chip_data(d);
+ int gpio = irqd_to_hwirq(d);
+
+ bcma_chipco_gpio_intmask(cc, BIT(gpio), 0);
+}
+
+static struct irq_chip bcma_gpio_irq_chip = {
+ .name = "BCMA-GPIO",
+ .irq_mask = bcma_gpio_irq_mask,
+ .irq_unmask = bcma_gpio_irq_unmask,
+};
+
+static irqreturn_t bcma_gpio_irq_handler(int irq, void *dev_id)
+{
+ struct bcma_drv_cc *cc = dev_id;
+ 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);
+ u32 irqs = (val ^ pol) & mask;
+ int gpio;
+
+ if (!irqs)
+ return IRQ_NONE;
+
+ for_each_set_bit(gpio, (unsigned long *)&irqs, cc->gpio.ngpio)
+ generic_handle_irq(bcma_gpio_to_irq(&cc->gpio, gpio));
+ bcma_chipco_gpio_polarity(cc, irqs, val & irqs);
+
+ return IRQ_HANDLED;
+}
+
+static int bcma_gpio_irq_domain_init(struct bcma_drv_cc *cc)
+{
+ struct gpio_chip *chip = &cc->gpio;
+ int gpio, 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);
+ err = request_irq(hwirq, bcma_gpio_irq_handler, IRQF_SHARED, "gpio",
+ cc);
+ if (err)
+ goto err_req_irq;
+
+ 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);
+ }
+ irq_domain_remove(cc->irq_domain);
+err_irq_domain:
+ return err;
+}
+
+static void bcma_gpio_irq_domain_exit(struct bcma_drv_cc *cc)
+{
+ struct gpio_chip *chip = &cc->gpio;
+ int gpio;
+
+ if (cc->core->bus->hosttype != BCMA_HOSTTYPE_SOC)
+ return 0;
+
+ bcma_cc_mask32(cc, BCMA_CC_IRQMASK, ~BCMA_CC_IRQ_GPIO);
+ free_irq(bcma_core_irq(cc->core), 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)
+{
+ return 0;
+}
+
+static void bcma_gpio_irq_domain_exit(struct bcma_drv_cc *cc)
+{
+}
+#endif
+
int bcma_gpio_init(struct bcma_drv_cc *cc)
{
struct gpio_chip *chip = &cc->gpio;
+ int err;
chip->label = "bcma_gpio";
chip->owner = THIS_MODULE;
@@ -95,7 +212,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;
- chip->to_irq = bcma_gpio_to_irq;
+ if (IS_BUILTIN(CONFIG_BCMA_HOST_SOC))
+ chip->to_irq = bcma_gpio_to_irq;
chip->ngpio = 16;
/* There is just one SoC in one device and its GPIO addresses should be
* deterministic to address them more easily. The other buses could get
@@ -105,10 +223,21 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
else
chip->base = -1;
- return gpiochip_add(chip);
+ err = bcma_gpio_irq_domain_init(cc);
+ if (err)
+ return err;
+
+ err = gpiochip_add(chip);
+ if (err) {
+ bcma_gpio_irq_domain_exit(cc);
+ return err;
+ }
+
+ return 0;
}
int bcma_gpio_unregister(struct bcma_drv_cc *cc)
{
+ bcma_gpio_irq_domain_exit(cc);
return gpiochip_remove(&cc->gpio);
}
diff --git a/include/linux/bcma/bcma_driver_chipcommon.h b/include/linux/bcma/bcma_driver_chipcommon.h
index c49e1a1..63d105c 100644
--- a/include/linux/bcma/bcma_driver_chipcommon.h
+++ b/include/linux/bcma/bcma_driver_chipcommon.h
@@ -640,6 +640,7 @@ struct bcma_drv_cc {
spinlock_t gpio_lock;
#ifdef CONFIG_BCMA_DRIVER_GPIO
struct gpio_chip gpio;
+ struct irq_domain *irq_domain;
#endif
};
--
1.7.10.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH V7 1/2] bcma: gpio: add own IRQ domain
2013-12-12 12:42 ` [PATCH V6 " Rafał Miłecki
@ 2013-12-12 12:46 ` Rafał Miłecki
0 siblings, 0 replies; 20+ messages in thread
From: Rafał Miłecki @ 2013-12-12 12:46 UTC (permalink / raw)
To: linux-mips, Ralf Baechle; +Cc: Hauke Mehrtens, Rafał Miłecki
Input GPIO changes can generate interrupts, but we need kind of ACK for
them by changing IRQ polarity. This is required to stop hardware from
keep generating interrupts and generate another one on the next GPIO
state change.
This code allows using GPIOs with standard interrupts and add for
example GPIO buttons support.
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
Acked-by: Hauke Mehrtens <hauke@hauke-m.de>
---
V3: Apply Hauke's comments.
1) Use IRQ domain for CONFIG_BCMA_HOST_SOC only
2) Optimize bcma_gpio_irq_handler
3) Register GPIO chip after doing everything else
4) Improve cleaning paths
V4: More fixes from Hauke's comments:
1) Less #ifdef CONFIG_BCMA_HOST_SOC
2) Move BCMA_CC_IRQMASK op (and unset on exit)
3) Optimize bcma_gpio_irq_handler
V5: Use IS_BUILTIN
V6: Add IRQ domain on SOC only
V7: Change "return 0" to "return"
Hauke: does it look OK to you?
---
drivers/bcma/Kconfig | 1 +
drivers/bcma/driver_gpio.c | 135 ++++++++++++++++++++++++++-
include/linux/bcma/bcma_driver_chipcommon.h | 1 +
3 files changed, 134 insertions(+), 3 deletions(-)
diff --git a/drivers/bcma/Kconfig b/drivers/bcma/Kconfig
index 7c081b3..0ee48be 100644
--- a/drivers/bcma/Kconfig
+++ b/drivers/bcma/Kconfig
@@ -75,6 +75,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
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 45f0996..ec422a9 100644
--- a/drivers/bcma/driver_gpio.c
+++ b/drivers/bcma/driver_gpio.c
@@ -9,6 +9,9 @@
*/
#include <linux/gpio.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/irqdomain.h>
#include <linux/export.h>
#include <linux/bcma/bcma.h>
@@ -73,19 +76,133 @@ static void bcma_gpio_free(struct gpio_chip *chip, unsigned gpio)
bcma_chipco_gpio_pullup(cc, 1 << gpio, 0);
}
+#if IS_BUILTIN(CONFIG_BCMA_HOST_SOC)
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 bcma_core_irq(cc->core);
+ 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);
+ int gpio = irqd_to_hwirq(d);
+
+ bcma_chipco_gpio_intmask(cc, BIT(gpio), BIT(gpio));
+}
+
+static void bcma_gpio_irq_mask(struct irq_data *d)
+{
+ struct bcma_drv_cc *cc = irq_data_get_irq_chip_data(d);
+ int gpio = irqd_to_hwirq(d);
+
+ bcma_chipco_gpio_intmask(cc, BIT(gpio), 0);
+}
+
+static struct irq_chip bcma_gpio_irq_chip = {
+ .name = "BCMA-GPIO",
+ .irq_mask = bcma_gpio_irq_mask,
+ .irq_unmask = bcma_gpio_irq_unmask,
+};
+
+static irqreturn_t bcma_gpio_irq_handler(int irq, void *dev_id)
+{
+ struct bcma_drv_cc *cc = dev_id;
+ 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);
+ u32 irqs = (val ^ pol) & mask;
+ int gpio;
+
+ if (!irqs)
+ return IRQ_NONE;
+
+ for_each_set_bit(gpio, (unsigned long *)&irqs, cc->gpio.ngpio)
+ generic_handle_irq(bcma_gpio_to_irq(&cc->gpio, gpio));
+ bcma_chipco_gpio_polarity(cc, irqs, val & irqs);
+
+ return IRQ_HANDLED;
+}
+
+static int bcma_gpio_irq_domain_init(struct bcma_drv_cc *cc)
+{
+ struct gpio_chip *chip = &cc->gpio;
+ int gpio, 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);
+ err = request_irq(hwirq, bcma_gpio_irq_handler, IRQF_SHARED, "gpio",
+ cc);
+ if (err)
+ goto err_req_irq;
+
+ 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);
+ }
+ irq_domain_remove(cc->irq_domain);
+err_irq_domain:
+ return err;
+}
+
+static void bcma_gpio_irq_domain_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), 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)
+{
+ return 0;
+}
+
+static void bcma_gpio_irq_domain_exit(struct bcma_drv_cc *cc)
+{
+}
+#endif
+
int bcma_gpio_init(struct bcma_drv_cc *cc)
{
struct gpio_chip *chip = &cc->gpio;
+ int err;
chip->label = "bcma_gpio";
chip->owner = THIS_MODULE;
@@ -95,7 +212,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;
- chip->to_irq = bcma_gpio_to_irq;
+ if (IS_BUILTIN(CONFIG_BCMA_HOST_SOC))
+ chip->to_irq = bcma_gpio_to_irq;
chip->ngpio = 16;
/* There is just one SoC in one device and its GPIO addresses should be
* deterministic to address them more easily. The other buses could get
@@ -105,10 +223,21 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
else
chip->base = -1;
- return gpiochip_add(chip);
+ err = bcma_gpio_irq_domain_init(cc);
+ if (err)
+ return err;
+
+ err = gpiochip_add(chip);
+ if (err) {
+ bcma_gpio_irq_domain_exit(cc);
+ return err;
+ }
+
+ return 0;
}
int bcma_gpio_unregister(struct bcma_drv_cc *cc)
{
+ bcma_gpio_irq_domain_exit(cc);
return gpiochip_remove(&cc->gpio);
}
diff --git a/include/linux/bcma/bcma_driver_chipcommon.h b/include/linux/bcma/bcma_driver_chipcommon.h
index c49e1a1..63d105c 100644
--- a/include/linux/bcma/bcma_driver_chipcommon.h
+++ b/include/linux/bcma/bcma_driver_chipcommon.h
@@ -640,6 +640,7 @@ struct bcma_drv_cc {
spinlock_t gpio_lock;
#ifdef CONFIG_BCMA_DRIVER_GPIO
struct gpio_chip gpio;
+ struct irq_domain *irq_domain;
#endif
};
--
1.7.10.4
^ permalink raw reply related [flat|nested] 20+ messages in thread