Re: [PATCH] gpio: unmap gpio irqs properly
On Sat, Mar 29, 2014 at 5:27 PM, Alexandre Courbot gnu...@gmail.com wrote:
On Sat, Mar 29, 2014 at 4:44 AM, Linus Walleij linus.wall...@linaro.org
wrote:
+static void gpiochip_irq_unmap(struct irq_domain *d, unsigned int irq)
+{
+#ifdef CONFIG_ARM
+ set_irq_flags(irq, 0);
+#endif
Just curious here, why is a special case needed for ARM? I would not
expect architecture-specific code on gpiolib, but I'm sure you have
your reasons for that.
This is open-coded like this in all ARM-applicable drivers so I'm
just refactoring really.
I don't really know why but I was under the impression that this
ARM-specific function exists because the IRQ implementations in
the different archs have never really converged.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-gpio in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] GPIO: Add driver for Zynq GPIO controller
On Sat, Mar 29, 2014 at 5:44 AM, Harini Katakam
harinikatakamli...@gmail.com wrote:
On Sat, Mar 29, 2014 at 3:20 AM, Linus Walleij linus.wall...@linaro.org
wrote:
On Thu, Mar 27, 2014 at 4:25 PM, Harini Katakam hari...@xilinx.com wrote:
+/* Read/Write access to the GPIO PS registers */
+static inline u32 zynq_gpio_readreg(void __iomem *offset)
+{
+ return readl_relaxed(offset);
+}
+
+static inline void zynq_gpio_writereg(void __iomem *offset, u32 val)
+{
+ writel_relaxed(val, offset);
+}
I think this is unnecessary and confusing indirection.
Just use the readl_relaxed/writel_relaxed functions directly in
the code.
This is just to be flexible.
Define exactly what you mean with flexible in this context. I
only see unnecessary overhead and hard-to-read code.
This is also pretty convoluted. Are you sure you don't want to
implement one gpiochip per bank instead? I guess the final +1
means there is actually one IRQ per bank even?
There is only one IRQ for all four banks.
OK I get it. Then it makes sense to have all banks registered as
one device and the IRQ tied to this one device.
+static const struct dev_pm_ops zynq_gpio_dev_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(zynq_gpio_suspend, zynq_gpio_resume)
+ SET_RUNTIME_PM_OPS(zynq_gpio_runtime_suspend,
zynq_gpio_runtime_resume,
+ zynq_gpio_idle)
+};
Is this runtime PM implementation aligned with Ulf Hansson's recent
new helpers to simplify suspend+runtime PM coexistance?
I'm sorry i just looked at them -
pm_runtime_force_suspend/resume are not used here.
Sorry, I was more thinking of change
717e5d458e3bfca495a38dca61c64f274c049e46
PM / Runtime: Implement the pm_generic_runtime functions for CONFIG_PM
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-gpio in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] GPIO: Add driver for Zynq GPIO controller
On 27 March 2014 16:25, Harini Katakam hari...@xilinx.com wrote:
Add support for GPIO controller used by Xilinx Zynq
Signed-off-by: Harini Katakam hari...@xilinx.com
---
drivers/gpio/Kconfig |7 +
drivers/gpio/Makefile|1 +
drivers/gpio/gpio-zynq.c | 690
++
3 files changed, 698 insertions(+)
create mode 100644 drivers/gpio/gpio-zynq.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 903f24d..67a22a6 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -313,6 +313,13 @@ config GPIO_XTENSA
Say yes here to support the Xtensa internal GPIO32 IMPWIRE (input)
and EXPSTATE (output) ports
+config GPIO_ZYNQ
+ bool Xilinx ZYNQ GPIO support
+ depends on ARCH_ZYNQ
+ select GENERIC_IRQ_CHIP
+ help
+Say yes here to support Xilinx ZYNQ GPIO controller.
+
config GPIO_VR41XX
tristate NEC VR4100 series General-purpose I/O Uint support
depends on CPU_VR41XX
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 5d50179..439f23a 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -99,3 +99,4 @@ obj-$(CONFIG_GPIO_WM8350) += gpio-wm8350.o
obj-$(CONFIG_GPIO_WM8994) += gpio-wm8994.o
obj-$(CONFIG_GPIO_XILINX) += gpio-xilinx.o
obj-$(CONFIG_GPIO_XTENSA) += gpio-xtensa.o
+obj-$(CONFIG_GPIO_ZYNQ)+= gpio-zynq.o
diff --git a/drivers/gpio/gpio-zynq.c b/drivers/gpio/gpio-zynq.c
new file mode 100644
index 000..1f5fdfc
--- /dev/null
+++ b/drivers/gpio/gpio-zynq.c
@@ -0,0 +1,690 @@
+/*
+ * Xilinx Zynq GPIO device driver
+ *
+ * Copyright (C) 2009 - 2014 Xilinx, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
under
+ * the terms of the GNU General Public License as published by the Free
Software
+ * Foundation; either version 2 of the License, or (at your option) any later
+ * version.
+ */
+
+#include linux/clk.h
+#include linux/gpio.h
+#include linux/init.h
+#include linux/interrupt.h
+#include linux/io.h
+#include linux/irq.h
+#include linux/irqchip/chained_irq.h
+#include linux/irqdomain.h
+#include linux/module.h
+#include linux/platform_device.h
+#include linux/pm_runtime.h
+
+#define DRIVER_NAME zynq-gpio
+#define ZYNQ_GPIO_NR_GPIOS 118
+
+static struct irq_domain *irq_domain;
+
+/* Register offsets for the GPIO device */
+
+/* LSW Mask Data -WO */
+#define ZYNQ_GPIO_DATA_LSW_OFFSET(BANK)(0x000 + (8 * BANK))
+/* MSW Mask Data -WO */
+#define ZYNQ_GPIO_DATA_MSW_OFFSET(BANK)(0x004 + (8 * BANK))
+/* Data Register-RW */
+#define ZYNQ_GPIO_DATA_OFFSET(BANK)(0x040 + (4 * BANK))
+/* Direction mode reg-RW */
+#define ZYNQ_GPIO_DIRM_OFFSET(BANK)(0x204 + (0x40 * BANK))
+/* Output enable reg-RW */
+#define ZYNQ_GPIO_OUTEN_OFFSET(BANK) (0x208 + (0x40 * BANK))
+/* Interrupt mask reg-RO */
+#define ZYNQ_GPIO_INTMASK_OFFSET(BANK) (0x20C + (0x40 * BANK))
+/* Interrupt enable reg-WO */
+#define ZYNQ_GPIO_INTEN_OFFSET(BANK) (0x210 + (0x40 * BANK))
+/* Interrupt disable reg-WO */
+#define ZYNQ_GPIO_INTDIS_OFFSET(BANK) (0x214 + (0x40 * BANK))
+/* Interrupt status reg-RO */
+#define ZYNQ_GPIO_INTSTS_OFFSET(BANK) (0x218 + (0x40 * BANK))
+/* Interrupt type reg-RW */
+#define ZYNQ_GPIO_INTTYPE_OFFSET(BANK) (0x21C + (0x40 * BANK))
+/* Interrupt polarity reg-RW */
+#define ZYNQ_GPIO_INTPOL_OFFSET(BANK) (0x220 + (0x40 * BANK))
+/* Interrupt on any, reg-RW */
+#define ZYNQ_GPIO_INTANY_OFFSET(BANK) (0x224 + (0x40 * BANK))
+
+/* Read/Write access to the GPIO PS registers */
+static inline u32 zynq_gpio_readreg(void __iomem *offset)
+{
+ return readl_relaxed(offset);
+}
+
+static inline void zynq_gpio_writereg(void __iomem *offset, u32 val)
+{
+ writel_relaxed(val, offset);
+}
+
+static unsigned int zynq_gpio_pin_table[] = {
+ 31, /* 0 - 31 */
+ 53, /* 32 - 53 */
+ 85, /* 54 - 85 */
+ 117 /* 86 - 117 */
+};
+
+/* Maximum banks */
+#define ZYNQ_GPIO_MAX_BANK 4
+
+/* Disable all interrupts mask */
+#define ZYNQ_GPIO_IXR_DISABLE_ALL 0x
+
+/* GPIO pin high */
+#define ZYNQ_GPIO_PIN_HIGH 1
+
+/* Mid pin number of a bank */
+#define ZYNQ_GPIO_MID_PIN_NUM 16
+
+/* GPIO upper 16 bit mask */
+#define ZYNQ_GPIO_UPPER_MASK 0x
+
+/**
+ * struct zynq_gpio - gpio device private data structure
+ * @chip: instance of the gpio_chip
+ * @base_addr: base address of the GPIO device
+ * @irq: irq associated with the controller
+ * @irq_base: base of IRQ number for interrupt
+ * @clk: clock resource for this controller
+ */
+struct zynq_gpio {
+ struct gpio_chip chip;
+ void __iomem *base_addr;
+ unsigned int irq;
+ unsigned int irq_base;
+ struct clk *clk;
+};
+
+/**
+ * zynq_gpio_get_bank_pin - Get the bank number and pin number
Re: [PATCH 2/7] gpio: rcar: Add optional functional clock to bindings
Hi Laurent,
On Fri, Mar 28, 2014 at 5:53 PM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
--- a/Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt
+++ b/Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt
@@ -21,6 +21,10 @@ Required Properties:
GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW flags are supported.
- gpio-ranges: Range of pins managed by the GPIO controller.
+Optional properties:
+
+ - clocks: Must contain a reference to the functional clock.
+
I would make the property mandatory. Obviously the driver needs to consider it
as optional in order not to break the DT ABI, but the specification should
make it mandatory in order to ensure that all future implementations will
specify the clock.
I think it has to stay optional: unless I misinterpreted the datasheet, r8a7778
doesn't have MSTP bits for the GPIO modules. I guess it's the same for
other R-Car Gen1 like r8a7779. So it looks like the bits were added in
R-Car Gen2.
Hence I'll just add , if present, unless the Best Practice is to put such
properties under Required properties or Recommended properties?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line unsubscribe linux-gpio in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: gpio / ACPI: Add support for ACPI GPIO operation regions
On Mon, Mar 31, 2014 at 03:05:25PM +0300, Mika Westerberg wrote: On Fri, Mar 28, 2014 at 11:37:32AM +0300, Dan Carpenter wrote: Hello Mika Westerberg, The patch 473ed7be0da0: gpio / ACPI: Add support for ACPI GPIO operation regions from Mar 14, 2014, leads to the following static checker warning: drivers/gpio/gpiolib-acpi.c:454 acpi_gpio_adr_space_handler() warn: should 'gpiod_get_raw_value(desc) i' be a 64 bit type? Thanks for the report. However, I'm not able to reproduce this warning with sparse. How did you get this? It's not a Sparse warning. It's some unreleased stuff (too many false positives). I sort through the warnings manually and send the ones which seem valid. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-gpio in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: gpio / ACPI: Add support for ACPI GPIO operation regions
On Mon, Mar 31, 2014 at 03:11:33PM +0300, Dan Carpenter wrote: On Mon, Mar 31, 2014 at 03:05:25PM +0300, Mika Westerberg wrote: On Fri, Mar 28, 2014 at 11:37:32AM +0300, Dan Carpenter wrote: Hello Mika Westerberg, The patch 473ed7be0da0: gpio / ACPI: Add support for ACPI GPIO operation regions from Mar 14, 2014, leads to the following static checker warning: drivers/gpio/gpiolib-acpi.c:454 acpi_gpio_adr_space_handler() warn: should 'gpiod_get_raw_value(desc) i' be a 64 bit type? Thanks for the report. However, I'm not able to reproduce this warning with sparse. How did you get this? It's not a Sparse warning. It's some unreleased stuff (too many false positives). I sort through the warnings manually and send the ones which seem valid. I see. What do you think about the patch below? I have to admit that this kind of stuff is in my gray area of understanding. diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c index d5be56fe689e..401add28933f 100644 --- a/drivers/gpio/gpiolib-acpi.c +++ b/drivers/gpio/gpiolib-acpi.c @@ -451,7 +451,7 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address, if (function == ACPI_WRITE) gpiod_set_raw_value(desc, !!((1 i) *value)); else - *value |= gpiod_get_raw_value(desc) i; + *value |= (u64)gpiod_get_raw_value(desc) i; } out: -- To unsubscribe from this list: send the line unsubscribe linux-gpio in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: gpio / ACPI: Add support for ACPI GPIO operation regions
On Mon, Mar 31, 2014 at 04:45:57PM +0300, Mika Westerberg wrote: On Mon, Mar 31, 2014 at 03:11:33PM +0300, Dan Carpenter wrote: On Mon, Mar 31, 2014 at 03:05:25PM +0300, Mika Westerberg wrote: On Fri, Mar 28, 2014 at 11:37:32AM +0300, Dan Carpenter wrote: Hello Mika Westerberg, The patch 473ed7be0da0: gpio / ACPI: Add support for ACPI GPIO operation regions from Mar 14, 2014, leads to the following static checker warning: drivers/gpio/gpiolib-acpi.c:454 acpi_gpio_adr_space_handler() warn: should 'gpiod_get_raw_value(desc) i' be a 64 bit type? Thanks for the report. However, I'm not able to reproduce this warning with sparse. How did you get this? It's not a Sparse warning. It's some unreleased stuff (too many false positives). I sort through the warnings manually and send the ones which seem valid. I see. What do you think about the patch below? I have to admit that this kind of stuff is in my gray area of understanding. It looks good to me. The question, I guess is can pin_table_length ever be more than 31. gpiod_get_raw_value() returns and int of 0-1 so if i is 31 then *value is an unexpected number because the shift is undefined (it wraps around in GCC). If it can't go higher than 31 then the existing code is fine. (We never use the upper 32 bits of *value). But your patch silences a valid looking static checker warning. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-gpio in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: gpio / ACPI: Add support for ACPI GPIO operation regions
On Mon, Mar 31, 2014 at 04:51:24PM +0300, Dan Carpenter wrote: On Mon, Mar 31, 2014 at 04:45:57PM +0300, Mika Westerberg wrote: On Mon, Mar 31, 2014 at 03:11:33PM +0300, Dan Carpenter wrote: On Mon, Mar 31, 2014 at 03:05:25PM +0300, Mika Westerberg wrote: On Fri, Mar 28, 2014 at 11:37:32AM +0300, Dan Carpenter wrote: Hello Mika Westerberg, The patch 473ed7be0da0: gpio / ACPI: Add support for ACPI GPIO operation regions from Mar 14, 2014, leads to the following static checker warning: drivers/gpio/gpiolib-acpi.c:454 acpi_gpio_adr_space_handler() warn: should 'gpiod_get_raw_value(desc) i' be a 64 bit type? Thanks for the report. However, I'm not able to reproduce this warning with sparse. How did you get this? It's not a Sparse warning. It's some unreleased stuff (too many false positives). I sort through the warnings manually and send the ones which seem valid. I see. What do you think about the patch below? I have to admit that this kind of stuff is in my gray area of understanding. It looks good to me. The question, I guess is can pin_table_length ever be more than 31. gpiod_get_raw_value() returns and int of 0-1 so if i is 31 then *value is an unexpected number because the shift is undefined (it wraps around in GCC). It can be larger than 31 so I guess I'll submit this patch to LinusW. -- To unsubscribe from this list: send the line unsubscribe linux-gpio in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] gpio: dwapb: drop irq_setup_generic_chip()
On Tue, Mar 25, 2014 at 09:37:50PM +0100, Linus Walleij wrote: Aha Jamie not even on the original thread. Here. On Tue, Mar 25, 2014 at 9:37 PM, Linus Walleij linus.wall...@linaro.org wrote: On Thu, Mar 20, 2014 at 8:55 PM, Sebastian Andrzej Siewior bige...@linutronix.de wrote: This looks kinda wrong I didn't manage to fully test it. The driver calls irq_alloc_domain_generic_chips() which creates a gc and adds it to gc_list. The driver later then calls irq_setup_generic_chip() which also initializes the gc and adds it to the gc_list() and this corrupts the list. I can't find a single chip in tree which uses both functions so I think that irq_setup_generic_chip() can be dropped. Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de Jamie: comments? I don't have anything to add to either yours or Alan's comments. I'll be more than happy to review the versions with the generic helpers if I'm copied onto the review. Thanks, Jamie -- To unsubscribe from this list: send the line unsubscribe linux-gpio in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
clarets
ied, -- To unsubscribe from this list: send the line unsubscribe linux-gpio in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
