Re: [PATCH v11] gpio: add a driver for the Synopsys DesignWare APB GPIO block
On Mon, Feb 24, 2014 at 6:20 AM, Linus Walleij wrote: > On Wed, Feb 12, 2014 at 5:17 PM, delicious quinoa > wrote: > >> I am wondering about the change in usage of >> irq_find_mapping/irq_create_mapping. It seems like all the GPIO >> drivers that use irq domains do it the way I was doing it > > It is a well known problems that the kernel contain many bad > examples. When in doubt ask for a good example to be provided. > > In gpio-omap.c you will find that this is done in the probe() > path. > >> (that's >> where I got the idea in the first place): irq_create_mapping is used >> in the to_irq() function. I guess this is a general direction all the >> other drivers will be encouraged to go in also? > > Yes. The reason is that it should be possible to reques an IRQ > from an irqchip no matter whether the gpio_to_irq() function > was called first, as the APIs are orthogonal. > > Yours, > Linus Walleij Hi Linus, Thanks, I made these changes and can see them working for me. The thing that is currently holding me up is that the DesignWare IP can be configured to have one irq exiting for the many gpios on port A or one gpio exiting for each gpio. I am trying to figure out how to support both cases with the simplest code. Currently support works for the one irq case. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v11] gpio: add a driver for the Synopsys DesignWare APB GPIO block
On Wed, Feb 12, 2014 at 5:17 PM, delicious quinoa wrote: > I am wondering about the change in usage of > irq_find_mapping/irq_create_mapping. It seems like all the GPIO > drivers that use irq domains do it the way I was doing it It is a well known problems that the kernel contain many bad examples. When in doubt ask for a good example to be provided. In gpio-omap.c you will find that this is done in the probe() path. > (that's > where I got the idea in the first place): irq_create_mapping is used > in the to_irq() function. I guess this is a general direction all the > other drivers will be encouraged to go in also? Yes. The reason is that it should be possible to reques an IRQ from an irqchip no matter whether the gpio_to_irq() function was called first, as the APIs are orthogonal. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v11] gpio: add a driver for the Synopsys DesignWare APB GPIO block
On Wed, Feb 12, 2014 at 5:17 PM, delicious quinoa delicious.qui...@gmail.com wrote: I am wondering about the change in usage of irq_find_mapping/irq_create_mapping. It seems like all the GPIO drivers that use irq domains do it the way I was doing it It is a well known problems that the kernel contain many bad examples. When in doubt ask for a good example to be provided. In gpio-omap.c you will find that this is done in the probe() path. (that's where I got the idea in the first place): irq_create_mapping is used in the to_irq() function. I guess this is a general direction all the other drivers will be encouraged to go in also? Yes. The reason is that it should be possible to reques an IRQ from an irqchip no matter whether the gpio_to_irq() function was called first, as the APIs are orthogonal. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v11] gpio: add a driver for the Synopsys DesignWare APB GPIO block
On Mon, Feb 24, 2014 at 6:20 AM, Linus Walleij linus.wall...@linaro.org wrote: On Wed, Feb 12, 2014 at 5:17 PM, delicious quinoa delicious.qui...@gmail.com wrote: I am wondering about the change in usage of irq_find_mapping/irq_create_mapping. It seems like all the GPIO drivers that use irq domains do it the way I was doing it It is a well known problems that the kernel contain many bad examples. When in doubt ask for a good example to be provided. In gpio-omap.c you will find that this is done in the probe() path. (that's where I got the idea in the first place): irq_create_mapping is used in the to_irq() function. I guess this is a general direction all the other drivers will be encouraged to go in also? Yes. The reason is that it should be possible to reques an IRQ from an irqchip no matter whether the gpio_to_irq() function was called first, as the APIs are orthogonal. Yours, Linus Walleij Hi Linus, Thanks, I made these changes and can see them working for me. The thing that is currently holding me up is that the DesignWare IP can be configured to have one irq exiting for the many gpios on port A or one gpio exiting for each gpio. I am trying to figure out how to support both cases with the simplest code. Currently support works for the one irq case. Alan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v11] gpio: add a driver for the Synopsys DesignWare APB GPIO block
On Mon, Feb 10, 2014 at 5:06 AM, Linus Walleij wrote:
> Hi Alan, this is starting to look good. I's like an ACK from a DT
> maintainer on the bindings but can't see anything really controversial
> about them.
>
> On Thu, Feb 6, 2014 at 11:06 PM, Alan Tull wrote:
>
>> +static int dwapb_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
>> +{
>> + struct bgpio_chip *bgc = to_bgpio_chip(gc);
>> + struct dwapb_gpio_port *port = container_of(bgc, struct
>> + dwapb_gpio_port, bgc);
>> + struct dwapb_gpio *gpio = port->gpio;
>> +
>> + return irq_create_mapping(gpio->domain, offset);
>> +}
>
> I think you want to call irq_find_mapping() here. irq_create_mapping()
> should be called for all valid IRQs on probe() instead.
>
>> + ct = irq_gc->chip_types;
>> + ct->chip.irq_ack = irq_gc_ack_set_bit;
>> + ct->chip.irq_mask = irq_gc_mask_set_bit;
>> + ct->chip.irq_unmask = irq_gc_mask_clr_bit;
>> + ct->chip.irq_set_type = dwapb_irq_set_type;
>> + ct->chip.irq_enable = dwapb_irq_enable;
>> + ct->chip.irq_disable = dwapb_irq_disable;
>> + ct->regs.ack = GPIO_PORTA_EOI;
>> + ct->regs.mask = GPIO_INTMASK;
>
> Please add .startup() and .shutdown() callbacks marking the
> respective lines as IRQs, compare to recent patches in the
> GPIO subsystem such as this:
> http://marc.info/?l=linux-gpio=138546100215235=2
>
> You probably want to call irq_create_mapping() for each
> valid IRQ after registering the chip in this function.
Hi Linus,
Thanks for the feedback. I am working on making these changes. The
startup/shutdown were easy to add.
I am wondering about the change in usage of
irq_find_mapping/irq_create_mapping. It seems like all the GPIO
drivers that use irq domains do it the way I was doing it (that's
where I got the idea in the first place): irq_create_mapping is used
in the to_irq() function. I guess this is a general direction all the
other drivers will be encouraged to go in also?
Regards,
Alan
>
> Yours,
> Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v11] gpio: add a driver for the Synopsys DesignWare APB GPIO block
On Mon, Feb 10, 2014 at 5:06 AM, Linus Walleij linus.wall...@linaro.org wrote:
Hi Alan, this is starting to look good. I's like an ACK from a DT
maintainer on the bindings but can't see anything really controversial
about them.
On Thu, Feb 6, 2014 at 11:06 PM, Alan Tull delicious.qui...@gmail.com wrote:
+static int dwapb_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
+{
+ struct bgpio_chip *bgc = to_bgpio_chip(gc);
+ struct dwapb_gpio_port *port = container_of(bgc, struct
+ dwapb_gpio_port, bgc);
+ struct dwapb_gpio *gpio = port-gpio;
+
+ return irq_create_mapping(gpio-domain, offset);
+}
I think you want to call irq_find_mapping() here. irq_create_mapping()
should be called for all valid IRQs on probe() instead.
+ ct = irq_gc-chip_types;
+ ct-chip.irq_ack = irq_gc_ack_set_bit;
+ ct-chip.irq_mask = irq_gc_mask_set_bit;
+ ct-chip.irq_unmask = irq_gc_mask_clr_bit;
+ ct-chip.irq_set_type = dwapb_irq_set_type;
+ ct-chip.irq_enable = dwapb_irq_enable;
+ ct-chip.irq_disable = dwapb_irq_disable;
+ ct-regs.ack = GPIO_PORTA_EOI;
+ ct-regs.mask = GPIO_INTMASK;
Please add .startup() and .shutdown() callbacks marking the
respective lines as IRQs, compare to recent patches in the
GPIO subsystem such as this:
http://marc.info/?l=linux-gpiom=138546100215235w=2
You probably want to call irq_create_mapping() for each
valid IRQ after registering the chip in this function.
Hi Linus,
Thanks for the feedback. I am working on making these changes. The
startup/shutdown were easy to add.
I am wondering about the change in usage of
irq_find_mapping/irq_create_mapping. It seems like all the GPIO
drivers that use irq domains do it the way I was doing it (that's
where I got the idea in the first place): irq_create_mapping is used
in the to_irq() function. I guess this is a general direction all the
other drivers will be encouraged to go in also?
Regards,
Alan
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v11] gpio: add a driver for the Synopsys DesignWare APB GPIO block
Hi Alan, this is starting to look good. I's like an ACK from a DT
maintainer on the bindings but can't see anything really controversial
about them.
On Thu, Feb 6, 2014 at 11:06 PM, Alan Tull wrote:
> +static int dwapb_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
> +{
> + struct bgpio_chip *bgc = to_bgpio_chip(gc);
> + struct dwapb_gpio_port *port = container_of(bgc, struct
> + dwapb_gpio_port, bgc);
> + struct dwapb_gpio *gpio = port->gpio;
> +
> + return irq_create_mapping(gpio->domain, offset);
> +}
I think you want to call irq_find_mapping() here. irq_create_mapping()
should be called for all valid IRQs on probe() instead.
> + ct = irq_gc->chip_types;
> + ct->chip.irq_ack = irq_gc_ack_set_bit;
> + ct->chip.irq_mask = irq_gc_mask_set_bit;
> + ct->chip.irq_unmask = irq_gc_mask_clr_bit;
> + ct->chip.irq_set_type = dwapb_irq_set_type;
> + ct->chip.irq_enable = dwapb_irq_enable;
> + ct->chip.irq_disable = dwapb_irq_disable;
> + ct->regs.ack = GPIO_PORTA_EOI;
> + ct->regs.mask = GPIO_INTMASK;
Please add .startup() and .shutdown() callbacks marking the
respective lines as IRQs, compare to recent patches in the
GPIO subsystem such as this:
http://marc.info/?l=linux-gpio=138546100215235=2
You probably want to call irq_create_mapping() for each
valid IRQ after registering the chip in this function.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v11] gpio: add a driver for the Synopsys DesignWare APB GPIO block
Hi Alan, this is starting to look good. I's like an ACK from a DT
maintainer on the bindings but can't see anything really controversial
about them.
On Thu, Feb 6, 2014 at 11:06 PM, Alan Tull delicious.qui...@gmail.com wrote:
+static int dwapb_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
+{
+ struct bgpio_chip *bgc = to_bgpio_chip(gc);
+ struct dwapb_gpio_port *port = container_of(bgc, struct
+ dwapb_gpio_port, bgc);
+ struct dwapb_gpio *gpio = port-gpio;
+
+ return irq_create_mapping(gpio-domain, offset);
+}
I think you want to call irq_find_mapping() here. irq_create_mapping()
should be called for all valid IRQs on probe() instead.
+ ct = irq_gc-chip_types;
+ ct-chip.irq_ack = irq_gc_ack_set_bit;
+ ct-chip.irq_mask = irq_gc_mask_set_bit;
+ ct-chip.irq_unmask = irq_gc_mask_clr_bit;
+ ct-chip.irq_set_type = dwapb_irq_set_type;
+ ct-chip.irq_enable = dwapb_irq_enable;
+ ct-chip.irq_disable = dwapb_irq_disable;
+ ct-regs.ack = GPIO_PORTA_EOI;
+ ct-regs.mask = GPIO_INTMASK;
Please add .startup() and .shutdown() callbacks marking the
respective lines as IRQs, compare to recent patches in the
GPIO subsystem such as this:
http://marc.info/?l=linux-gpiom=138546100215235w=2
You probably want to call irq_create_mapping() for each
valid IRQ after registering the chip in this function.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v11] gpio: add a driver for the Synopsys DesignWare APB GPIO block
From: Jamie Iles
The Synopsys DesignWare block is used in some ARM devices (picoxcell)
and can be configured to provide multiple banks of GPIO pins.
Signed-off-by: Jamie Iles
Signed-off-by: Alan Tull
Reviewed-by: Sebastian Hesselbarth
v11: - Use NULL when checking existence of 'interrupts' property
- Bindings descriptions cleanup
v10: - in documentation nr-gpio -> nr-gpios
v9: - cleanup in dt bindings doc
- use of_get_child_count()
v8: - remove socfpga.dtsi changes
- minor cleanup in devicetree documentation
v7: - use irq_generic_chip
- support one irq per gpio line or one irq for many
- s/bank/port/ and other cleanup
v6: - (atull) squash the set of patches
- use linear irq domain
- build fixes. Original driver was reviewed on v3.2.
- Fix setting irq edge type for 'rising' and 'both'.
- Support as a loadable module.
- Use bgpio_chip's spinlock during register access.
- Clean up register names to match spec
- s/bank/port/ because register names use the word 'port'
- s/nr-gpio/nr-gpios/
- don't get/put the of_node
- remove signoffs/acked-by's because of changes
- other cleanup
v5: - handle sparse bank population correctly
v3: - depend on rather than select IRQ_DOMAIN
- split IRQ support into a separate patch
v2: - use Rob Herring's irqdomain in generic irq chip patches
- use reg property to indicate bank index
- support irqs on both edges based on LinusW's u300 driver
---
.../devicetree/bindings/gpio/snps-dwapb-gpio.txt | 58 +++
drivers/gpio/Kconfig |9 +
drivers/gpio/Makefile |1 +
drivers/gpio/gpio-dwapb.c | 415
4 files changed, 483 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
create mode 100644 drivers/gpio/gpio-dwapb.c
diff --git a/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
new file mode 100644
index 000..38c2224
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
@@ -0,0 +1,58 @@
+* Synopsys DesignWare APB GPIO controller
+
+Required properties:
+- compatible : Should contain "snps,dw-apb-gpio"
+- reg : Address and length of the register set for the device.
+- #address-cells : should be 1 (for addressing port subnodes).
+- #size-cells : should be 0 (port subnodes).
+
+The GPIO controller has a configurable number of ports, each of which are
+represented as child nodes with the following properties:
+
+Required properties:
+- compatible : "snps,dw-apb-gpio-port"
+- gpio-controller : Marks the device node as a gpio controller.
+- #gpio-cells : Should be 1. It is the pin number.
+- reg : The integer port index of the port, a single cell.
+
+Optional properties:
+- interrupt-controller : The first port may be configured to be an interrupt
+controller.
+- #interrupt-cells : Specifies the number of cells needed to encode an
+ interrupt. Shall be set to 2. The first cell defines the interrupt number,
+ the second encodes the triger flags encoded as described in
+ Documentation/devicetree/bindings/interrupts.txt
+- interrupt-parent : The parent interrupt controller.
+- interrupts : The interrupts to the parent controller raised when GPIOs
+ generate the interrupts. The number of interrupts depends on the hardware
+ configuration of the DesignWare IP block.
+- snps,nr-gpios : The number of pins in the port, a single cell.
+
+Example:
+
+gpio: gpio@2 {
+ compatible = "snps,dw-apb-gpio";
+ reg = <0x2 0x1000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ porta: gpio-controller@0 {
+ compatible = "snps,dw-apb-gpio-port";
+ gpio-controller;
+ #gpio-cells = <2>;
+ snps,nr-gpios = <8>;
+ reg = <0>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ interrupt-parent = <>;
+ interrupts = <0>, <1>, <2>, <3>, <4>, <5>, <6>, <7>;
+ };
+
+ portb: gpio-controller@1 {
+ compatible = "snps,dw-apb-gpio-port";
+ gpio-controller;
+ #gpio-cells = <2>;
+ snps,nr-gpios = <8>;
+ reg = <1>;
+ };
+};
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 6973387..b72cd85 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -128,6 +128,15 @@ config GPIO_GENERIC_PLATFORM
help
Say yes here to support basic platform_device memory-mapped GPIO
controllers.
+config GPIO_DWAPB
+ tristate "Synopsys DesignWare APB GPIO driver"
+ select GPIO_GENERIC
+ select GENERIC_IRQ_CHIP
+ depends on OF_GPIO && IRQ_DOMAIN
+ help
+ Say Y or M here to build support for the Synopsys DesignWare APB
+ GPIO block.
+
config GPIO_IT8761E
[PATCH v11] gpio: add a driver for the Synopsys DesignWare APB GPIO block
From: Jamie Iles ja...@jamieiles.com
The Synopsys DesignWare block is used in some ARM devices (picoxcell)
and can be configured to provide multiple banks of GPIO pins.
Signed-off-by: Jamie Iles ja...@jamieiles.com
Signed-off-by: Alan Tull at...@altera.com
Reviewed-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
v11: - Use NULL when checking existence of 'interrupts' property
- Bindings descriptions cleanup
v10: - in documentation nr-gpio - nr-gpios
v9: - cleanup in dt bindings doc
- use of_get_child_count()
v8: - remove socfpga.dtsi changes
- minor cleanup in devicetree documentation
v7: - use irq_generic_chip
- support one irq per gpio line or one irq for many
- s/bank/port/ and other cleanup
v6: - (atull) squash the set of patches
- use linear irq domain
- build fixes. Original driver was reviewed on v3.2.
- Fix setting irq edge type for 'rising' and 'both'.
- Support as a loadable module.
- Use bgpio_chip's spinlock during register access.
- Clean up register names to match spec
- s/bank/port/ because register names use the word 'port'
- s/nr-gpio/nr-gpios/
- don't get/put the of_node
- remove signoffs/acked-by's because of changes
- other cleanup
v5: - handle sparse bank population correctly
v3: - depend on rather than select IRQ_DOMAIN
- split IRQ support into a separate patch
v2: - use Rob Herring's irqdomain in generic irq chip patches
- use reg property to indicate bank index
- support irqs on both edges based on LinusW's u300 driver
---
.../devicetree/bindings/gpio/snps-dwapb-gpio.txt | 58 +++
drivers/gpio/Kconfig |9 +
drivers/gpio/Makefile |1 +
drivers/gpio/gpio-dwapb.c | 415
4 files changed, 483 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
create mode 100644 drivers/gpio/gpio-dwapb.c
diff --git a/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
new file mode 100644
index 000..38c2224
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
@@ -0,0 +1,58 @@
+* Synopsys DesignWare APB GPIO controller
+
+Required properties:
+- compatible : Should contain snps,dw-apb-gpio
+- reg : Address and length of the register set for the device.
+- #address-cells : should be 1 (for addressing port subnodes).
+- #size-cells : should be 0 (port subnodes).
+
+The GPIO controller has a configurable number of ports, each of which are
+represented as child nodes with the following properties:
+
+Required properties:
+- compatible : snps,dw-apb-gpio-port
+- gpio-controller : Marks the device node as a gpio controller.
+- #gpio-cells : Should be 1. It is the pin number.
+- reg : The integer port index of the port, a single cell.
+
+Optional properties:
+- interrupt-controller : The first port may be configured to be an interrupt
+controller.
+- #interrupt-cells : Specifies the number of cells needed to encode an
+ interrupt. Shall be set to 2. The first cell defines the interrupt number,
+ the second encodes the triger flags encoded as described in
+ Documentation/devicetree/bindings/interrupts.txt
+- interrupt-parent : The parent interrupt controller.
+- interrupts : The interrupts to the parent controller raised when GPIOs
+ generate the interrupts. The number of interrupts depends on the hardware
+ configuration of the DesignWare IP block.
+- snps,nr-gpios : The number of pins in the port, a single cell.
+
+Example:
+
+gpio: gpio@2 {
+ compatible = snps,dw-apb-gpio;
+ reg = 0x2 0x1000;
+ #address-cells = 1;
+ #size-cells = 0;
+
+ porta: gpio-controller@0 {
+ compatible = snps,dw-apb-gpio-port;
+ gpio-controller;
+ #gpio-cells = 2;
+ snps,nr-gpios = 8;
+ reg = 0;
+ interrupt-controller;
+ #interrupt-cells = 2;
+ interrupt-parent = vic1;
+ interrupts = 0, 1, 2, 3, 4, 5, 6, 7;
+ };
+
+ portb: gpio-controller@1 {
+ compatible = snps,dw-apb-gpio-port;
+ gpio-controller;
+ #gpio-cells = 2;
+ snps,nr-gpios = 8;
+ reg = 1;
+ };
+};
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 6973387..b72cd85 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -128,6 +128,15 @@ config GPIO_GENERIC_PLATFORM
help
Say yes here to support basic platform_device memory-mapped GPIO
controllers.
+config GPIO_DWAPB
+ tristate Synopsys DesignWare APB GPIO driver
+ select GPIO_GENERIC
+ select GENERIC_IRQ_CHIP
+ depends on OF_GPIO IRQ_DOMAIN
+ help
+ Say Y or M here to build support for the Synopsys DesignWare APB
+
