Re: [PATCH v3 2/4] gpio-tz1090: add TZ1090 gpio driver

2013-06-24 Thread Grant Likely 
On Thu, 20 Jun 2013 10:26:28 +0100, James Hogan james.ho...@imgtec.com wrote:
 Add a GPIO driver for the main GPIOs found in the TZ1090 (Comet) SoC.
 This doesn't include low-power GPIOs as they're controlled separately
 via the Powerdown Controller (PDC) registers.
 
 The driver is instantiated by device tree and supports interrupts for
 all GPIOs.
 
 Signed-off-by: James Hogan james.ho...@imgtec.com
 Cc: Grant Likely grant.lik...@linaro.org
 Cc: Rob Herring rob.herr...@calxeda.com
 Cc: Rob Landley r...@landley.net
 Cc: Linus Walleij linus.wall...@linaro.org
 Cc: linux-...@vger.kernel.org
 Cc: devicetree-discuss@lists.ozlabs.org
 ---
 Changes in v3:
  - separated from irq-imgpdc and removed arch/metag changes to allow
these patches to go upstream separately via the pinctrl[/gpio] trees
(particularly the pinctrl drivers depend on the new pinconf DT
bindings).
  - some s/unsigned/unsigned int/.
  - some s/unsigned int/bool/ and use of BIT().
  - gpio-tz1090*: refer to dt-bindings/gpio/gpio.h and
dt-bindings/interrupt-controller/irq.h flags in bindings.
  - gpio-tz1090*: move initcall from postcore to subsys.
  - gpio-tz1090: add REG_ prefix to some constants for consistency.
  - gpio-tz1090: add comment to explain tz1090_gpio_irq_next_edge
cunningness.
 
 Changes in v2:
  - gpio-tz1090: remove references to Linux flags in dt bindings
  - gpio-tz1090: make use of BIT() from linux/bitops.h
  - gpio-tz1090: make register accessors inline to match pinctrl
  - gpio-tz1090: update gpio-ranges to use 3 cells after recent ABI
breakage
 
  .../devicetree/bindings/gpio/gpio-tz1090.txt   |  87 +++
  drivers/gpio/Kconfig   |   7 +
  drivers/gpio/Makefile  |   1 +
  drivers/gpio/gpio-tz1090.c | 633 
 +
  4 files changed, 728 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-tz1090.txt
  create mode 100644 drivers/gpio/gpio-tz1090.c
 
 diff --git a/Documentation/devicetree/bindings/gpio/gpio-tz1090.txt 
 b/Documentation/devicetree/bindings/gpio/gpio-tz1090.txt
 new file mode 100644
 index 000..e017d4b
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/gpio/gpio-tz1090.txt
 @@ -0,0 +1,87 @@
 +ImgTec TZ1090 GPIO Controller
 +
 +Required properties:
 +- compatible: Compatible property value should be img,tz1090-gpio.

typo at end of line

 +
 +- reg: Physical base address of the controller and length of memory mapped
 +  region.
 +
 +- #address-cells: Should be 1 (for bank subnodes)
 +
 +- #size-cells: Should be 0 (for bank subnodes)
 +
 +- Each bank of GPIOs should have a subnode to represent it.
 +
 +  Bank subnode required properties:
 +  - reg: Index of bank in the range 0 to 2.
 +
 +  - gpio-controller: Specifies that the node is a gpio controller.
 +
 +  - #gpio-cells: Should be 2. The syntax of the gpio specifier used by client
 +nodes should have the following values.
 +   [phandle of the gpio controller node]
 +[gpio number within the gpio bank]
 +[gpio flags]
 +
 +Values for gpio specifier:
 +- GPIO number: a value in the range 0 to 29.
 +- GPIO flags: bit field of flags, as defined in 
 dt-bindings/gpio/gpio.h.
 +  Only the following flags are supported:
 +GPIO_ACTIVE_HIGH
 +GPIO_ACTIVE_LOW
 +
 +  Bank subnode optional properties:
 +  - gpio-ranges: Mapping to pin controller pins

This is specific to this binding. To avoid namespace colisions, add a
img, prefix to the property name.

 +
 +  - interrupts: Interrupt for the entire bank
 +
 +  - interrupt-controller: Specifies that the node is an interrupt controller
 +
 +  - #interrupt-cells: Should be 2. The syntax of the interrupt specifier 
 used by
 +client nodes should have the following values.
 +   [phandle of the interurupt controller]
 +[gpio number within the gpio bank]
 +[irq flags]
 +
 +Values for irq specifier:
 +- GPIO number: a value in the range 0 to 29
 +- IRQ flags: value to describe edge and level triggering, as defined in
 +  dt-bindings/interrupt-controller/irq.h. Only the following flags are
 +  supported:
 +IRQ_TYPE_EDGE_RISING
 +IRQ_TYPE_EDGE_FALLING
 +IRQ_TYPE_EDGE_BOTH
 +IRQ_TYPE_LEVEL_HIGH
 +IRQ_TYPE_LEVEL_LOW
 +
 +
 +
 +Example:
 +
 + gpios: gpio-controller@02005800 {
 + #address-cells = 1;
 + #size-cells = 0;
 + compatible = img,tz1090-gpio;
 + reg = 0x02005800 0x90;
 +
 + /* bank 0 with an interrupt */
 + gpios0: bank@0 {
 + #gpio-cells = 2;
 + #interrupt-cells = 2;
 + reg = 0;
 + interrupts = 13 IRQ_TYPE_LEVEL_HIGH;
 + gpio-controller;
 + gpio-ranges = pinctrl 0 30;
 + interrupt-controller;
 + };
 +
 +

Re: [PATCH v3 2/4] gpio-tz1090: add TZ1090 gpio driver

2013-06-24 Thread James Hogan 
On 24/06/13 14:34, Grant Likely wrote:
 On Thu, 20 Jun 2013 10:26:28 +0100, James Hogan james.ho...@imgtec.com 
 wrote:
 diff --git a/Documentation/devicetree/bindings/gpio/gpio-tz1090.txt 
 b/Documentation/devicetree/bindings/gpio/gpio-tz1090.txt
 new file mode 100644
 index 000..e017d4b
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/gpio/gpio-tz1090.txt
 @@ -0,0 +1,87 @@
 +ImgTec TZ1090 GPIO Controller
 +
 +Required properties:
 +- compatible: Compatible property value should be img,tz1090-gpio.
 
 typo at end of line

Yes, I'll fix in gpio-tz1090-pdc driver bindings too

 +  Bank subnode optional properties:
 +  - gpio-ranges: Mapping to pin controller pins
 
 This is specific to this binding. To avoid namespace colisions, add a
 img, prefix to the property name.

This property is described in
Documentation/devicetree/bindings/gpio/gpio.txt... (and my examples are
out of date from when the gpio offset cell was added in v3.10). I'll add
a reference to that Document.

 +/* GPIO chip callbacks */
 +
 +static int tz1090_gpio_direction_input(struct gpio_chip *chip,
 +   unsigned int offset)
 +{
 +struct tz1090_gpio_bank *bank = to_bank(chip);
 +tz1090_gpio_set_bit(bank, REG_GPIO_DIR, offset);
 +
 +return 0;
 +}
 +
 +static int tz1090_gpio_direction_output(struct gpio_chip *chip,
 +unsigned int offset, int output_value)
 +{
 +struct tz1090_gpio_bank *bank = to_bank(chip);
 +int lstat;
 +
 +__global_lock2(lstat);
 +_tz1090_gpio_mod_bit(bank, REG_GPIO_DOUT, offset, output_value);
 +_tz1090_gpio_clear_bit(bank, REG_GPIO_DIR, offset);
 +__global_unlock2(lstat);
 +
 +return 0;
 +}
 +
 +/*
 + * Return GPIO level
 + */
 +static int tz1090_gpio_get(struct gpio_chip *chip, unsigned int offset)
 +{
 +struct tz1090_gpio_bank *bank = to_bank(chip);
 +
 +return tz1090_gpio_read_bit(bank, REG_GPIO_DIN, offset);
 +}
 +
 +/*
 + * Set output GPIO level
 + */
 +static void tz1090_gpio_set(struct gpio_chip *chip, unsigned int offset,
 +int output_value)
 +{
 +struct tz1090_gpio_bank *bank = to_bank(chip);
 +
 +tz1090_gpio_mod_bit(bank, REG_GPIO_DOUT, offset, output_value);
 +}
 +
 +static int tz1090_gpio_request(struct gpio_chip *chip, unsigned int offset)
 +{
 +struct tz1090_gpio_bank *bank = to_bank(chip);
 +int ret;
 +
 +ret = pinctrl_request_gpio(chip-base + offset);
 +if (ret)
 +return ret;
 +
 +tz1090_gpio_set_bit(bank, REG_GPIO_DIR, offset);
 +tz1090_gpio_set_bit(bank, REG_GPIO_BIT_EN, offset);
 +
 +return 0;
 +}
 
 Is it possible to use the gpio-generic.c hooks for manipulating the
 gpio bits?

Due to the unfortunate necessity to use the __global_lock2 functions
(for atomic accesses between different non-linux threads/cores) I don't
think this is possible.

 +/* IRQ chip handlers */
 +
 +/* Get TZ1090 GPIO chip from irq data provided to generic IRQ callbacks */
 +static inline struct tz1090_gpio_bank *irqd_to_gpio_bank(struct irq_data 
 *data)
 +{
 +return (struct tz1090_gpio_bank *)data-domain-host_data;
 +}
 +
 +static void tz1090_gpio_irq_clear(struct tz1090_gpio_bank *bank,
 +  unsigned int offset)
 +{
 +tz1090_gpio_clear_bit(bank, REG_GPIO_IRQ_STS, offset);
 +}
 +
 +static void tz1090_gpio_irq_enable(struct tz1090_gpio_bank *bank,
 +   unsigned int offset, bool enable)
 +{
 +tz1090_gpio_mod_bit(bank, REG_GPIO_IRQ_EN, offset, enable);
 +}
 +
 +static void tz1090_gpio_irq_polarity(struct tz1090_gpio_bank *bank,
 + unsigned int offset, unsigned int polarity)
 +{
 +tz1090_gpio_mod_bit(bank, REG_GPIO_IRQ_PLRT, offset, polarity);
 +}
 +
 +static int tz1090_gpio_valid_handler(struct irq_desc *desc)
 +{
 +return desc-handle_irq == handle_level_irq ||
 +desc-handle_irq == handle_edge_irq;
 +}
 +
 +static void tz1090_gpio_irq_type(struct tz1090_gpio_bank *bank,
 + unsigned int offset, unsigned int type)
 +{
 +tz1090_gpio_mod_bit(bank, REG_GPIO_IRQ_TYPE, offset, type);
 +}
 +
 +/* set polarity to trigger on next edge, whether rising or falling */
 +static void tz1090_gpio_irq_next_edge(struct tz1090_gpio_bank *bank,
 +  unsigned int offset)
 +{
 +unsigned int value_p, value_i;
 +int lstat;
 +
 +/*
 + * Set the GPIO's interrupt polarity to the opposite of the current
 + * input value so that the next edge triggers an interrupt.
 + */
 +__global_lock2(lstat);
 +value_i = ~tz1090_gpio_read(bank, REG_GPIO_DIN);
 +value_p = tz1090_gpio_read(bank, REG_GPIO_IRQ_PLRT);
 +value_p = ~BIT(offset);
 +value_p |= value_i  BIT(offset);
 +tz1090_gpio_write(bank, REG_GPIO_IRQ_PLRT, value_p);
 +__global_unlock2(lstat);
 +}
 +
 +static void gpio_ack_irq(struct irq_data *data)
 +{
 +struct tz1090_gpio_bank *bank

Re: [PATCH v3 2/4] gpio-tz1090: add TZ1090 gpio driver

2013-06-24 Thread James Hogan 
On 24/06/13 15:48, James Hogan wrote:
 On 24/06/13 14:34, Grant Likely wrote:
 Similarly, can this driver use the generic irq chip to eliminate the
 above hooks?
 
 hmm, I could probably get away with it for irq callbacks since a bank's
 IRQ cannot be shared with non-Linux threads/cores.

I just remembered, the commits that make the generic irqchip work with
linear irq domains are in tip/irq/core so this won't work on
pinctrl/devel or gpio/next branches.

How would you like that handled? I'm happy to write a patch to convert
to generic irqchip ready to be applied later (e.g. for v3.12).

Cheers
James

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

[PATCH v3 2/4] gpio-tz1090: add TZ1090 gpio driver

2013-06-20 Thread James Hogan 
Add a GPIO driver for the main GPIOs found in the TZ1090 (Comet) SoC.
This doesn't include low-power GPIOs as they're controlled separately
via the Powerdown Controller (PDC) registers.

The driver is instantiated by device tree and supports interrupts for
all GPIOs.

Signed-off-by: James Hogan james.ho...@imgtec.com
Cc: Grant Likely grant.lik...@linaro.org
Cc: Rob Herring rob.herr...@calxeda.com
Cc: Rob Landley r...@landley.net
Cc: Linus Walleij linus.wall...@linaro.org
Cc: linux-...@vger.kernel.org
Cc: devicetree-discuss@lists.ozlabs.org
---
Changes in v3:
 - separated from irq-imgpdc and removed arch/metag changes to allow
   these patches to go upstream separately via the pinctrl[/gpio] trees
   (particularly the pinctrl drivers depend on the new pinconf DT
   bindings).
 - some s/unsigned/unsigned int/.
 - some s/unsigned int/bool/ and use of BIT().
 - gpio-tz1090*: refer to dt-bindings/gpio/gpio.h and
   dt-bindings/interrupt-controller/irq.h flags in bindings.
 - gpio-tz1090*: move initcall from postcore to subsys.
 - gpio-tz1090: add REG_ prefix to some constants for consistency.
 - gpio-tz1090: add comment to explain tz1090_gpio_irq_next_edge
   cunningness.

Changes in v2:
 - gpio-tz1090: remove references to Linux flags in dt bindings
 - gpio-tz1090: make use of BIT() from linux/bitops.h
 - gpio-tz1090: make register accessors inline to match pinctrl
 - gpio-tz1090: update gpio-ranges to use 3 cells after recent ABI
   breakage

 .../devicetree/bindings/gpio/gpio-tz1090.txt   |  87 +++
 drivers/gpio/Kconfig   |   7 +
 drivers/gpio/Makefile  |   1 +
 drivers/gpio/gpio-tz1090.c | 633 +
 4 files changed, 728 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-tz1090.txt
 create mode 100644 drivers/gpio/gpio-tz1090.c

diff --git a/Documentation/devicetree/bindings/gpio/gpio-tz1090.txt 
b/Documentation/devicetree/bindings/gpio/gpio-tz1090.txt
new file mode 100644
index 000..e017d4b
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-tz1090.txt
@@ -0,0 +1,87 @@
+ImgTec TZ1090 GPIO Controller
+
+Required properties:
+- compatible: Compatible property value should be img,tz1090-gpio.
+
+- reg: Physical base address of the controller and length of memory mapped
+  region.
+
+- #address-cells: Should be 1 (for bank subnodes)
+
+- #size-cells: Should be 0 (for bank subnodes)
+
+- Each bank of GPIOs should have a subnode to represent it.
+
+  Bank subnode required properties:
+  - reg: Index of bank in the range 0 to 2.
+
+  - gpio-controller: Specifies that the node is a gpio controller.
+
+  - #gpio-cells: Should be 2. The syntax of the gpio specifier used by client
+nodes should have the following values.
+   [phandle of the gpio controller node]
+[gpio number within the gpio bank]
+[gpio flags]
+
+Values for gpio specifier:
+- GPIO number: a value in the range 0 to 29.
+- GPIO flags: bit field of flags, as defined in dt-bindings/gpio/gpio.h.
+  Only the following flags are supported:
+GPIO_ACTIVE_HIGH
+GPIO_ACTIVE_LOW
+
+  Bank subnode optional properties:
+  - gpio-ranges: Mapping to pin controller pins
+
+  - interrupts: Interrupt for the entire bank
+
+  - interrupt-controller: Specifies that the node is an interrupt controller
+
+  - #interrupt-cells: Should be 2. The syntax of the interrupt specifier used 
by
+client nodes should have the following values.
+   [phandle of the interurupt controller]
+[gpio number within the gpio bank]
+[irq flags]
+
+Values for irq specifier:
+- GPIO number: a value in the range 0 to 29
+- IRQ flags: value to describe edge and level triggering, as defined in
+  dt-bindings/interrupt-controller/irq.h. Only the following flags are
+  supported:
+IRQ_TYPE_EDGE_RISING
+IRQ_TYPE_EDGE_FALLING
+IRQ_TYPE_EDGE_BOTH
+IRQ_TYPE_LEVEL_HIGH
+IRQ_TYPE_LEVEL_LOW
+
+
+
+Example:
+
+   gpios: gpio-controller@02005800 {
+   #address-cells = 1;
+   #size-cells = 0;
+   compatible = img,tz1090-gpio;
+   reg = 0x02005800 0x90;
+
+   /* bank 0 with an interrupt */
+   gpios0: bank@0 {
+   #gpio-cells = 2;
+   #interrupt-cells = 2;
+   reg = 0;
+   interrupts = 13 IRQ_TYPE_LEVEL_HIGH;
+   gpio-controller;
+   gpio-ranges = pinctrl 0 30;
+   interrupt-controller;
+   };
+
+   /* bank 2 without interrupt */
+   gpios2: bank@2 {
+   #gpio-cells = 2;
+   reg = 2;
+   gpio-controller;
+   gpio-ranges = pinctrl 60 30;
+   };
+   };
+
+
diff --git a/drivers/gpio/Kconfig