This patchset implements IRQ support for the Hollywood GPIO controller
(Hollywood is one of the chips in the Nintendo Wii game console).
Together with two device tree patches[1], I can use the buttons (via the
gpio-keys driver) and the photoelectric sensor in the disc slot (via
gpio-event-mon).
[1]: https://www.spinics.net/lists/kernel/msg3010459.html
Jonathan Neuschäfer (2):
gpio: hlwd: Add basic IRQ support
gpio: hlwd: Implement edge trigger emulation
drivers/gpio/Kconfig | 1 +
drivers/gpio/gpio-hlwd.c | 193 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 193 insertions(+), 1 deletion(-)
--
2.20.1
Like the Spreadtrum EIC driver[1], this driver needs to emulate edge
triggered interrupts to support the generic gpio-keys driver.
[1]: https://www.spinics.net/lists/kernel/msg2764576.html
Signed-off-by: Jonathan Neuschäfer <[email protected]>
---
drivers/gpio/gpio-hlwd.c | 57 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/drivers/gpio/gpio-hlwd.c b/drivers/gpio/gpio-hlwd.c
index 9a93546f3d11..3d85e6a71b35 100644
--- a/drivers/gpio/gpio-hlwd.c
+++ b/drivers/gpio/gpio-hlwd.c
@@ -51,6 +51,8 @@ struct hlwd_gpio {
struct irq_chip irqc;
void __iomem *regs;
int irq;
+ u32 edge_emulation;
+ u32 rising_edge, falling_edge;
};
static void hlwd_gpio_irqhandler(struct irq_desc *desc)
@@ -61,10 +63,37 @@ static void hlwd_gpio_irqhandler(struct irq_desc *desc)
unsigned long flags;
unsigned long pending;
int hwirq;
+ u32 emulated_pending;
spin_lock_irqsave(&hlwd->gpioc.bgpio_lock, flags);
pending = hlwd->gpioc.read_reg(hlwd->regs + HW_GPIOB_INTFLAG);
pending &= hlwd->gpioc.read_reg(hlwd->regs + HW_GPIOB_INTMASK);
+
+ /* Treat interrupts due to edge trigger emulation separately */
+ emulated_pending = hlwd->edge_emulation & pending;
+ pending &= ~emulated_pending;
+ if (emulated_pending) {
+ u32 level, rising, falling;
+
+ level = hlwd->gpioc.read_reg(hlwd->regs + HW_GPIOB_INTLVL);
+ rising = level & emulated_pending;
+ falling = ~level & emulated_pending;
+
+ /* Invert the levels */
+ hlwd->gpioc.write_reg(hlwd->regs + HW_GPIOB_INTLVL,
+ level ^ emulated_pending);
+
+ /* Ack all emulated-edge interrupts */
+ hlwd->gpioc.write_reg(hlwd->regs + HW_GPIOB_INTFLAG,
+ emulated_pending);
+
+ /* Signal interrupts only on the correct edge */
+ rising &= hlwd->rising_edge;
+ falling &= hlwd->falling_edge;
+
+ /* Mark emulated interrupts as pending */
+ pending |= rising | falling;
+ }
spin_unlock_irqrestore(&hlwd->gpioc.bgpio_lock, flags);
chained_irq_enter(chip, desc);
@@ -120,6 +149,27 @@ static void hlwd_gpio_irq_enable(struct irq_data *data)
hlwd_gpio_irq_unmask(data);
}
+static void hlwd_gpio_irq_setup_emulation(struct hlwd_gpio *hlwd, int hwirq,
+ unsigned int flow_type)
+{
+ u32 level, state;
+
+ /* Set the trigger level to the inactive level */
+ level = hlwd->gpioc.read_reg(hlwd->regs + HW_GPIOB_INTLVL);
+ state = hlwd->gpioc.read_reg(hlwd->regs + HW_GPIOB_IN) & BIT(hwirq);
+ level &= ~BIT(hwirq);
+ level |= state ^ BIT(hwirq);
+ hlwd->gpioc.write_reg(hlwd->regs + HW_GPIOB_INTLVL, level);
+
+ hlwd->edge_emulation |= BIT(hwirq);
+ hlwd->rising_edge &= ~BIT(hwirq);
+ hlwd->falling_edge &= ~BIT(hwirq);
+ if (flow_type & IRQ_TYPE_EDGE_RISING)
+ hlwd->rising_edge |= BIT(hwirq);
+ if (flow_type & IRQ_TYPE_EDGE_FALLING)
+ hlwd->falling_edge |= BIT(hwirq);
+}
+
static int hlwd_gpio_irq_set_type(struct irq_data *data, unsigned int flow_type)
{
struct hlwd_gpio *hlwd =
@@ -129,6 +179,8 @@ static int hlwd_gpio_irq_set_type(struct irq_data *data, unsigned int flow_type)
spin_lock_irqsave(&hlwd->gpioc.bgpio_lock, flags);
+ hlwd->edge_emulation &= ~BIT(data->hwirq);
+
switch (flow_type) {
case IRQ_TYPE_LEVEL_HIGH:
level = hlwd->gpioc.read_reg(hlwd->regs + HW_GPIOB_INTLVL);
@@ -140,6 +192,11 @@ static int hlwd_gpio_irq_set_type(struct irq_data *data, unsigned int flow_type)
level &= ~BIT(data->hwirq);
hlwd->gpioc.write_reg(hlwd->regs + HW_GPIOB_INTLVL, level);
break;
+ case IRQ_TYPE_EDGE_RISING:
+ case IRQ_TYPE_EDGE_FALLING:
+ case IRQ_TYPE_EDGE_BOTH:
+ hlwd_gpio_irq_setup_emulation(hlwd, data->hwirq, flow_type);
+ break;
default:
spin_unlock_irqrestore(&hlwd->gpioc.bgpio_lock, flags);
return -EINVAL;
--
2.20.1
This patch implements level-triggered IRQs in the Hollywood GPIO driver.
Edge triggered interrupts are not supported in this GPIO controller, so
I moved their emulation into a separate patch.
Signed-off-by: Jonathan Neuschäfer <[email protected]>
---
I'm not entirely sure about the locking, but lockdep doesn't complain.
---
drivers/gpio/Kconfig | 1 +
drivers/gpio/gpio-hlwd.c | 136 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 136 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index b5a2845347ec..f77180dd87bd 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -258,6 +258,7 @@ config GPIO_HLWD
tristate "Nintendo Wii (Hollywood) GPIO"
depends on OF_GPIO
select GPIO_GENERIC
+ select GPIOLIB_IRQCHIP
help
Select this to support the GPIO controller of the Nintendo Wii.
diff --git a/drivers/gpio/gpio-hlwd.c b/drivers/gpio/gpio-hlwd.c
index a63136a68ba3..9a93546f3d11 100644
--- a/drivers/gpio/gpio-hlwd.c
+++ b/drivers/gpio/gpio-hlwd.c
@@ -48,9 +48,107 @@
struct hlwd_gpio {
struct gpio_chip gpioc;
+ struct irq_chip irqc;
void __iomem *regs;
+ int irq;
};
+static void hlwd_gpio_irqhandler(struct irq_desc *desc)
+{
+ struct hlwd_gpio *hlwd =
+ gpiochip_get_data(irq_desc_get_handler_data(desc));
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ unsigned long flags;
+ unsigned long pending;
+ int hwirq;
+
+ spin_lock_irqsave(&hlwd->gpioc.bgpio_lock, flags);
+ pending = hlwd->gpioc.read_reg(hlwd->regs + HW_GPIOB_INTFLAG);
+ pending &= hlwd->gpioc.read_reg(hlwd->regs + HW_GPIOB_INTMASK);
+ spin_unlock_irqrestore(&hlwd->gpioc.bgpio_lock, flags);
+
+ chained_irq_enter(chip, desc);
+
+ for_each_set_bit(hwirq, &pending, 32) {
+ int irq = irq_find_mapping(hlwd->gpioc.irq.domain, hwirq);
+
+ generic_handle_irq(irq);
+ }
+
+ chained_irq_exit(chip, desc);
+}
+
+static void hlwd_gpio_irq_ack(struct irq_data *data)
+{
+ struct hlwd_gpio *hlwd =
+ gpiochip_get_data(irq_data_get_irq_chip_data(data));
+
+ hlwd->gpioc.write_reg(hlwd->regs + HW_GPIOB_INTFLAG, BIT(data->hwirq));
+}
+
+static void hlwd_gpio_irq_mask(struct irq_data *data)
+{
+ struct hlwd_gpio *hlwd =
+ gpiochip_get_data(irq_data_get_irq_chip_data(data));
+ unsigned long flags;
+ u32 mask;
+
+ spin_lock_irqsave(&hlwd->gpioc.bgpio_lock, flags);
+ mask = hlwd->gpioc.read_reg(hlwd->regs + HW_GPIOB_INTMASK);
+ mask &= ~BIT(data->hwirq);
+ hlwd->gpioc.write_reg(hlwd->regs + HW_GPIOB_INTMASK, mask);
+ spin_unlock_irqrestore(&hlwd->gpioc.bgpio_lock, flags);
+}
+
+static void hlwd_gpio_irq_unmask(struct irq_data *data)
+{
+ struct hlwd_gpio *hlwd =
+ gpiochip_get_data(irq_data_get_irq_chip_data(data));
+ unsigned long flags;
+ u32 mask;
+
+ spin_lock_irqsave(&hlwd->gpioc.bgpio_lock, flags);
+ mask = hlwd->gpioc.read_reg(hlwd->regs + HW_GPIOB_INTMASK);
+ mask |= BIT(data->hwirq);
+ hlwd->gpioc.write_reg(hlwd->regs + HW_GPIOB_INTMASK, mask);
+ spin_unlock_irqrestore(&hlwd->gpioc.bgpio_lock, flags);
+}
+
+static void hlwd_gpio_irq_enable(struct irq_data *data)
+{
+ hlwd_gpio_irq_ack(data);
+ hlwd_gpio_irq_unmask(data);
+}
+
+static int hlwd_gpio_irq_set_type(struct irq_data *data, unsigned int flow_type)
+{
+ struct hlwd_gpio *hlwd =
+ gpiochip_get_data(irq_data_get_irq_chip_data(data));
+ unsigned long flags;
+ u32 level;
+
+ spin_lock_irqsave(&hlwd->gpioc.bgpio_lock, flags);
+
+ switch (flow_type) {
+ case IRQ_TYPE_LEVEL_HIGH:
+ level = hlwd->gpioc.read_reg(hlwd->regs + HW_GPIOB_INTLVL);
+ level |= BIT(data->hwirq);
+ hlwd->gpioc.write_reg(hlwd->regs + HW_GPIOB_INTLVL, level);
+ break;
+ case IRQ_TYPE_LEVEL_LOW:
+ level = hlwd->gpioc.read_reg(hlwd->regs + HW_GPIOB_INTLVL);
+ level &= ~BIT(data->hwirq);
+ hlwd->gpioc.write_reg(hlwd->regs + HW_GPIOB_INTLVL, level);
+ break;
+ default:
+ spin_unlock_irqrestore(&hlwd->gpioc.bgpio_lock, flags);
+ return -EINVAL;
+ }
+
+ spin_unlock_irqrestore(&hlwd->gpioc.bgpio_lock, flags);
+ return 0;
+}
+
static int hlwd_gpio_probe(struct platform_device *pdev)
{
struct hlwd_gpio *hlwd;
@@ -92,7 +190,43 @@ static int hlwd_gpio_probe(struct platform_device *pdev)
ngpios = 32;
hlwd->gpioc.ngpio = ngpios;
- return devm_gpiochip_add_data(&pdev->dev, &hlwd->gpioc, hlwd);
+ res = devm_gpiochip_add_data(&pdev->dev, &hlwd->gpioc, hlwd);
+ if (res)
+ return res;
+
+ /* Mask and ack all interrupts */
+ iowrite32be(0, hlwd->regs + HW_GPIOB_INTMASK);
+ iowrite32be(0xffffffff, hlwd->regs + HW_GPIOB_INTFLAG);
+
+ /*
+ * If this GPIO controller is not marked as an interrupt controller in
+ * the DT, return.
+ */
+ if (!of_property_read_bool(pdev->dev.of_node, "interrupt-controller"))
+ return 0;
+
+ hlwd->irq = platform_get_irq(pdev, 0);
+ if (hlwd->irq < 0) {
+ dev_info(&pdev->dev, "platform_get_irq returned %d\n",
+ hlwd->irq);
+ return hlwd->irq;
+ }
+
+ hlwd->irqc.name = "GPIO";
+ hlwd->irqc.irq_mask = hlwd_gpio_irq_mask;
+ hlwd->irqc.irq_unmask = hlwd_gpio_irq_unmask;
+ hlwd->irqc.irq_enable = hlwd_gpio_irq_enable;
+ hlwd->irqc.irq_set_type = hlwd_gpio_irq_set_type;
+
+ res = gpiochip_irqchip_add(&hlwd->gpioc, &hlwd->irqc, 0,
+ handle_level_irq, IRQ_TYPE_NONE);
+ if (res)
+ return res;
+
+ gpiochip_set_chained_irqchip(&hlwd->gpioc, &hlwd->irqc,
+ hlwd->irq, hlwd_gpio_irqhandler);
+
+ return 0;
}
static const struct of_device_id hlwd_gpio_match[] = {
--
2.20.1
Hi Jonathan,
thanks for the patch! It is looking very good.
Some minor comments:
On Sun, Jan 13, 2019 at 3:00 PM Jonathan Neuschäfer
<[email protected]> wrote:
> select GPIO_GENERIC
> + select GPIOLIB_IRQCHIP
Nice!
> +static void hlwd_gpio_irqhandler(struct irq_desc *desc)
> +{
> + struct hlwd_gpio *hlwd =
> + gpiochip_get_data(irq_desc_get_handler_data(desc));
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> + unsigned long flags;
> + unsigned long pending;
> + int hwirq;
> +
> + spin_lock_irqsave(&hlwd->gpioc.bgpio_lock, flags);
> + pending = hlwd->gpioc.read_reg(hlwd->regs + HW_GPIOB_INTFLAG);
> + pending &= hlwd->gpioc.read_reg(hlwd->regs + HW_GPIOB_INTMASK);
Please just access IO directly instead through the helper.
ioread32be()/iowrite32be() I suppose?
> +static void hlwd_gpio_irq_ack(struct irq_data *data)
> +{
> + struct hlwd_gpio *hlwd =
> + gpiochip_get_data(irq_data_get_irq_chip_data(data));
> +
> + hlwd->gpioc.write_reg(hlwd->regs + HW_GPIOB_INTFLAG, BIT(data->hwirq));
Dito.
> + spin_lock_irqsave(&hlwd->gpioc.bgpio_lock, flags);
> + mask = hlwd->gpioc.read_reg(hlwd->regs + HW_GPIOB_INTMASK);
> + mask &= ~BIT(data->hwirq);
> + hlwd->gpioc.write_reg(hlwd->regs + HW_GPIOB_INTMASK, mask);
Dito.
> + spin_lock_irqsave(&hlwd->gpioc.bgpio_lock, flags);
> + mask = hlwd->gpioc.read_reg(hlwd->regs + HW_GPIOB_INTMASK);
> + mask |= BIT(data->hwirq);
> + hlwd->gpioc.write_reg(hlwd->regs + HW_GPIOB_INTMASK, mask);
Dito.
> + switch (flow_type) {
> + case IRQ_TYPE_LEVEL_HIGH:
> + level = hlwd->gpioc.read_reg(hlwd->regs + HW_GPIOB_INTLVL);
> + level |= BIT(data->hwirq);
> + hlwd->gpioc.write_reg(hlwd->regs + HW_GPIOB_INTLVL, level);
> + break;
> + case IRQ_TYPE_LEVEL_LOW:
> + level = hlwd->gpioc.read_reg(hlwd->regs + HW_GPIOB_INTLVL);
> + level &= ~BIT(data->hwirq);
> + hlwd->gpioc.write_reg(hlwd->regs + HW_GPIOB_INTLVL, level);
> + break;
Dito.
> + hlwd->irqc.name = "GPIO";
What about using something device-unique?
hlwd->irqc.name = dev_name(&pdev->dev);
for example?
otherwise it looks fine!
Yours,
Linus Walleij
Hi,
On Mon, Jan 14, 2019 at 11:20:49AM +0100, Linus Walleij wrote:
> Hi Jonathan,
>
> thanks for the patch! It is looking very good.
> Some minor comments:
>
> On Sun, Jan 13, 2019 at 3:00 PM Jonathan Neuschäfer
[...]
> > + spin_lock_irqsave(&hlwd->gpioc.bgpio_lock, flags);
> > + pending = hlwd->gpioc.read_reg(hlwd->regs + HW_GPIOB_INTFLAG);
> > + pending &= hlwd->gpioc.read_reg(hlwd->regs + HW_GPIOB_INTMASK);
>
> Please just access IO directly instead through the helper.
> ioread32be()/iowrite32be() I suppose?
Ok. Yes, 32-bit big endian is correct.
[...]
> > + hlwd->irqc.name = "GPIO";
>
> What about using something device-unique?
>
> hlwd->irqc.name = dev_name(&pdev->dev);
>
> for example?
Good idea, especially considering that the Wii U (the successor of the
Wii) has two of these GPIO controllers.
> otherwise it looks fine!
Cool, I'll send a v2 with these two points addressed.
Thanks,
Jonathan Neuschäfer