Re: [PATCH 2/4] pinctrl: single: Add hardware specific hooks for IRQ and GPIO wake-up events
* Linus Walleij linus.wall...@linaro.org [130722 14:50]: On Mon, Jun 10, 2013 at 5:36 PM, Tony Lindgren t...@atomide.com wrote: At least on omaps, each board typically has at least one device configured as wake-up capable from deeper idle modes. In the deeper idle modes the normal interrupt wake-up path won't work as the logic is powered off and separate wake-up hardware is available either via IO ring or GPIO hardware. What I do not understand is why the irq_set_wake() should not fall through to that IO ring / GPIO hardware. That certainly makes sense. For example: a composite GPIO+irqchip driver should surely set the wake up for a certain line for irq_set_wake()? Yes we might be able to make it all transparent to consumer drivers. Although irq_set_wake() is for suspend/resume only AFAIK, but ideally we would not have to configure anything from the consumer drivers for runtime PM. For the GPIO wake-up events, GPIO + irqchip driver is not enough as we also need the mapping between pinctrl registers and GPIO numbers. And to stay out of the database business, that mapping should ideally come from device tree as it's only needed for the pins used, not for all hundreds of pins on a SoC. The wake-up event can be device specific, or may need to be dynamically remuxed to GPIO input for wake-up events. When the wake-up event happens, it's IRQ need to be called so the device won't lose interrupts. I recognize this hardware type. The name I use for these things are latent interrupts. OK What I think is that they should maybe be modeled as irqchip from end to end, so that we don't orthogonally use any pinctrl states to set this up. OK I'll take a look. Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] pinctrl: single: Add hardware specific hooks for IRQ and GPIO wake-up events
On Mon, Jun 10, 2013 at 5:36 PM, Tony Lindgren t...@atomide.com wrote:
At least on omaps, each board typically has at least one device
configured as wake-up capable from deeper idle modes. In the
deeper idle modes the normal interrupt wake-up path won't work
as the logic is powered off and separate wake-up hardware is
available either via IO ring or GPIO hardware.
What I do not understand is why the irq_set_wake() should
not fall through to that IO ring / GPIO hardware.
For example: a composite GPIO+irqchip driver should surely
set the wake up for a certain line for irq_set_wake()?
The wake-up
event can be device specific, or may need to be dynamically
remuxed to GPIO input for wake-up events. When the wake-up
event happens, it's IRQ need to be called so the device won't
lose interrupts.
I recognize this hardware type. The name I use for these
things are latent interrupts.
What I think is that they should maybe be modeled as
irqchip from end to end, so that we don't orthogonally use
any pinctrl states to set this up.
Allow supporting IRQ and GPIO wake-up events if a hardware
spefific module is registered for the enable and disable
s/spefific/specific
calls.
Done in collaboration with Roger Quadros rog...@ti.com.
Cc: Haojian Zhuang haojian.zhu...@gmail.com
Cc: Peter Ujfalusi peter.ujfal...@ti.com
Cc: devicetree-disc...@lists.ozlabs.org
Signed-off-by: Roger Quadros rog...@ti.com
Signed-off-by: Tony Lindgren t...@atomide.com
(...)
+- interrrupts : the interrupt that a function may have for a wake-up event
interrupts
+
+- gpios: the gpio that a function may have for a wake-up event
Is this a GPIO property or a pin property?
wake up is not supported by the GPIO subsystem is it?
Not in linux/gpio.h atleast.
This smells like shoehorning pin config stuff into the GPIO
subsystem again which is a no-no :-)
diff --git a/drivers/pinctrl/pinctrl-single.c
b/drivers/pinctrl/pinctrl-single.c
@@ -1183,6 +1241,24 @@ static int pcs_parse_one_pinctrl_entry(struct
pcs_device *pcs,
} else {
*num_maps = 1;
}
+
+ if (pcs-flags PCS_HAS_FUNCTION_IRQ)
+ function-irq = irq_of_parse_and_map(np, 0);
+
+ if (pcs-flags PCS_HAS_FUNCTION_GPIO) {
+ function-gpio = of_get_gpio(np, 0);
+ if (function-gpio 0 !function-irq) {
+ if (gpio_is_valid(function-gpio))
+ function-irq = gpio_to_irq(function-gpio);
+ }
+ }
+
+ if (function-irq 0 pcs-soc pcs-soc-reg_init) {
+ res = pcs_parse_wakeup(pcs, np, function);
+ if (res)
+ goto free_pingroups;
+ }
So this thing here. Instead of introducing a cross reference to the
IRQ and GPIO here and
+ * @irq: optional irq specified for wake-up for example
+ * @gpio: optional gpio specified for wake-up for example
+ * @node: optional list
+ */
+struct pcs_reg {
+ unsigned (*read)(void __iomem *reg);
+ void (*write)(unsigned val, void __iomem *reg);
+ void __iomem *reg;
+ unsigned val;
+ int irq;
+ int gpio;
+ struct list_head node;
+};
+
+#define PCS_HAS_FUNCTION_GPIO (1 2)
+#define PCS_HAS_FUNCTION_IRQ(1 1)
#define PCS_HAS_PINCONF (1 0)
/**
* struct pcs_soc - SoC specific interface to pinctrl-single
* @data: SoC specific data pointer
* @flags: mask of PCS_HAS_xxx values
+ * @reg_init: SoC specific register init function
+ * @enable:SoC specific enable function
+ * @disable: SoC specific disable function
*/
struct pcs_soc {
void *data;
unsigned flags;
+ int (*reg_init)(const struct pcs_soc *soc, struct pcs_reg *r);
+ int (*enable)(const struct pcs_soc *soc, struct pcs_reg *r);
+ void (*disable)(const struct pcs_soc *soc, struct pcs_reg *r);
Then these quirks and sub-modules ...
Why can't the irqchip/GPIO driver call down to this driver
instead?
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] pinctrl: single: Add hardware specific hooks for IRQ and GPIO wake-up events
* Haojian Zhuang haojian.zhu...@gmail.com [130608 21:51]:
I assume that this patch is used in both v1 v2 version. Since Manjunathappa
changed the logic of distinguishing bits and pins in blew.
if (pcs-bits_per_mux)
mask = vals-mask;
else
mask = pcs-fmask
Would you like to sync with his style?
Thanks for catching that, yes that's how it should be now. Updated
patch below.
Regards,
Tony
From: Tony Lindgren t...@atomide.com
Date: Sat, 8 Jun 2013 08:40:35 -0700
Subject: [PATCH] pinctrl: single: Add hardware specific hooks for IRQ and GPIO
wake-up events
At least on omaps, each board typically has at least one device
configured as wake-up capable from deeper idle modes. In the
deeper idle modes the normal interrupt wake-up path won't work
as the logic is powered off and separate wake-up hardware is
available either via IO ring or GPIO hardware. The wake-up
event can be device specific, or may need to be dynamically
remuxed to GPIO input for wake-up events. When the wake-up
event happens, it's IRQ need to be called so the device won't
lose interrupts.
Allow supporting IRQ and GPIO wake-up events if a hardware
spefific module is registered for the enable and disable
calls.
Done in collaboration with Roger Quadros rog...@ti.com.
Cc: Haojian Zhuang haojian.zhu...@gmail.com
Cc: Peter Ujfalusi peter.ujfal...@ti.com
Cc: devicetree-disc...@lists.ozlabs.org
Signed-off-by: Roger Quadros rog...@ti.com
Signed-off-by: Tony Lindgren t...@atomide.com
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
index 5a02e30..b95fa6c 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
@@ -69,6 +69,10 @@ Optional properties:
The number of parameters is depend on #pinctrl-single,gpio-range-cells
property.
+- interrrupts : the interrupt that a function may have for a wake-up event
+
+- gpios: the gpio that a function may have for a wake-up event
+
/* pin base, nr pins gpio function */
pinctrl-single,gpio-range = range 0 3 0 range 3 9 1;
@@ -205,6 +209,7 @@ pmx_gpio: pinmux@d401e000 {
0xdc 0x118
0xde 0
;
+ interrupts = 74;
};
};
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index e3b1f76..72efc8e 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -19,6 +19,8 @@
#include linux/of.h
#include linux/of_device.h
#include linux/of_address.h
+#include linux/of_gpio.h
+#include linux/of_irq.h
#include linux/pinctrl/pinctrl.h
#include linux/pinctrl/pinmux.h
@@ -95,6 +97,8 @@ struct pcs_conf_type {
* @nvals: number of entries in vals array
* @pgnames: array of pingroup names the function uses
* @npgnames: number of pingroup names the function uses
+ * @irq: optional irq associated with the function
+ * @gpio: optional gpio associated with the function
* @node: list node
*/
struct pcs_function {
@@ -105,6 +109,8 @@ struct pcs_function {
int npgnames;
struct pcs_conf_vals *conf;
int nconfs;
+ int irq;
+ int gpio;
struct list_head node;
};
@@ -411,6 +417,18 @@ static int pcs_get_function(struct pinctrl_dev *pctldev,
unsigned pin,
return 0;
}
+static void pcs_reg_init(struct pcs_reg *p, struct pcs_device *pcs,
+struct pcs_function *func,
+void __iomem *reg, unsigned val)
+{
+ p-read = pcs-read;
+ p-write = pcs-write;
+ p-irq = func-irq;
+ p-gpio = func-gpio;
+ p-reg = reg;
+ p-val = val;
+}
+
static int pcs_enable(struct pinctrl_dev *pctldev, unsigned fselector,
unsigned group)
{
@@ -444,6 +462,12 @@ static int pcs_enable(struct pinctrl_dev *pctldev,
unsigned fselector,
val = ~mask;
val |= (vals-val mask);
pcs-write(val, vals-reg);
+ if ((func-irq || func-gpio) pcs-soc pcs-soc-enable) {
+ struct pcs_reg pcsr;
+
+ pcs_reg_init(pcsr, pcs, func, vals-reg, val);
+ pcs-soc-enable(pcs-soc, pcsr);
+ }
}
return 0;
@@ -468,18 +492,6 @@ static void pcs_disable(struct pinctrl_dev *pctldev,
unsigned fselector,
return;
}
- /*
-* Ignore disable if function-off is not specified. Some hardware
-* does not have clearly defined disable function. For pin specific
-* off modes, you can use alternate named states as described in
-* pinctrl-bindings.txt.
-*/
- if (pcs-foff == PCS_OFF_DISABLED) {
- dev_dbg(pcs-dev, ignoring disable for %s function%i\n,
- func-name, fselector);
- return;
-
Re: [PATCH 2/4] pinctrl: single: Add hardware specific hooks for IRQ and GPIO wake-up events
On Sat, Jun 8, 2013 at 4:50 AM, Tony Lindgren t...@atomide.com wrote:
At least on omaps, each board typically has at least one device
configured as wake-up capable from deeper idle modes. In the
deeper idle modes the normal interrupt wake-up path won't work
as the logic is powered off and separate wake-up hardware is
available either via IO ring or GPIO hardware. The wake-up
event can be device specific, or may need to be dynamically
remuxed to GPIO input for wake-up events. When the wake-up
event happens, it's IRQ need to be called so the device won't
lose interrupts.
Allow supporting IRQ and GPIO wake-up events if a hardware
spefific module is registered for the enable and disable
calls.
Done in collaboration with Roger Quadros rog...@ti.com.
Cc: Haojian Zhuang haojian.zhu...@gmail.com
Cc: Peter Ujfalusi peter.ujfal...@ti.com
Cc: devicetree-disc...@lists.ozlabs.org
Signed-off-by: Roger Quadros rog...@ti.com
Signed-off-by: Tony Lindgren t...@atomide.com
---
.../devicetree/bindings/pinctrl/pinctrl-single.txt |5 +
drivers/pinctrl/pinctrl-single.c | 104
+---
drivers/pinctrl/pinctrl-single.h | 28 +
3 files changed, 123 insertions(+), 14 deletions(-)
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
index 08f0c3d..5dfd74b 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
@@ -68,6 +68,10 @@ Optional properties:
The number of parameters is depend on #pinctrl-single,gpio-range-cells
property.
+- interrrupts : the interrupt that a function may have for a wake-up event
+
+- gpios: the gpio that a function may have for a wake-up event
+
/* pin base, nr pins gpio function */
pinctrl-single,gpio-range = range 0 3 0 range 3 9 1;
@@ -204,6 +208,7 @@ pmx_gpio: pinmux@d401e000 {
0xdc 0x118
0xde 0
;
+ interrupts = 74;
};
};
diff --git a/drivers/pinctrl/pinctrl-single.c
b/drivers/pinctrl/pinctrl-single.c
index 0f178d1..7cb7940 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -19,6 +19,8 @@
#include linux/of.h
#include linux/of_device.h
#include linux/of_address.h
+#include linux/of_gpio.h
+#include linux/of_irq.h
#include linux/pinctrl/pinctrl.h
#include linux/pinctrl/pinmux.h
@@ -95,6 +97,8 @@ struct pcs_conf_type {
* @nvals: number of entries in vals array
* @pgnames: array of pingroup names the function uses
* @npgnames: number of pingroup names the function uses
+ * @irq: optional irq associated with the function
+ * @gpio: optional gpio associated with the function
* @node: list node
*/
struct pcs_function {
@@ -105,6 +109,8 @@ struct pcs_function {
int npgnames;
struct pcs_conf_vals *conf;
int nconfs;
+ int irq;
+ int gpio;
struct list_head node;
};
@@ -410,6 +416,18 @@ static int pcs_get_function(struct pinctrl_dev *pctldev,
unsigned pin,
return 0;
}
+static void pcs_reg_init(struct pcs_reg *p, struct pcs_device *pcs,
+struct pcs_function *func,
+void __iomem *reg, unsigned val)
+{
+ p-read = pcs-read;
+ p-write = pcs-write;
+ p-irq = func-irq;
+ p-gpio = func-gpio;
+ p-reg = reg;
+ p-val = val;
+}
+
static int pcs_enable(struct pinctrl_dev *pctldev, unsigned fselector,
unsigned group)
{
@@ -442,6 +460,12 @@ static int pcs_enable(struct pinctrl_dev *pctldev,
unsigned fselector,
val = ~mask;
val |= (vals-val mask);
pcs-write(val, vals-reg);
+ if ((func-irq || func-gpio) pcs-soc
pcs-soc-enable) {
+ struct pcs_reg pcsr;
+
+ pcs_reg_init(pcsr, pcs, func, vals-reg, val);
+ pcs-soc-enable(pcs-soc, pcsr);
+ }
}
return 0;
@@ -466,18 +490,6 @@ static void pcs_disable(struct pinctrl_dev *pctldev,
unsigned fselector,
return;
}
- /*
-* Ignore disable if function-off is not specified. Some hardware
-* does not have clearly defined disable function. For pin specific
-* off modes, you can use alternate named states as described in
-* pinctrl-bindings.txt.
-*/
- if (pcs-foff == PCS_OFF_DISABLED) {
- dev_dbg(pcs-dev, ignoring disable for %s function%i\n,
- func-name, fselector);
- return;
- }
-
dev_dbg(pcs-dev, disabling function%i %s\n,
fselector, func-name);
@@ -488,8 +500,28 @@ static void
[PATCH 2/4] pinctrl: single: Add hardware specific hooks for IRQ and GPIO wake-up events
At least on omaps, each board typically has at least one device
configured as wake-up capable from deeper idle modes. In the
deeper idle modes the normal interrupt wake-up path won't work
as the logic is powered off and separate wake-up hardware is
available either via IO ring or GPIO hardware. The wake-up
event can be device specific, or may need to be dynamically
remuxed to GPIO input for wake-up events. When the wake-up
event happens, it's IRQ need to be called so the device won't
lose interrupts.
Allow supporting IRQ and GPIO wake-up events if a hardware
spefific module is registered for the enable and disable
calls.
Done in collaboration with Roger Quadros rog...@ti.com.
Cc: Haojian Zhuang haojian.zhu...@gmail.com
Cc: Peter Ujfalusi peter.ujfal...@ti.com
Cc: devicetree-disc...@lists.ozlabs.org
Signed-off-by: Roger Quadros rog...@ti.com
Signed-off-by: Tony Lindgren t...@atomide.com
---
.../devicetree/bindings/pinctrl/pinctrl-single.txt |5 +
drivers/pinctrl/pinctrl-single.c | 104 +---
drivers/pinctrl/pinctrl-single.h | 28 +
3 files changed, 123 insertions(+), 14 deletions(-)
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
index 08f0c3d..5dfd74b 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
@@ -68,6 +68,10 @@ Optional properties:
The number of parameters is depend on #pinctrl-single,gpio-range-cells
property.
+- interrrupts : the interrupt that a function may have for a wake-up event
+
+- gpios: the gpio that a function may have for a wake-up event
+
/* pin base, nr pins gpio function */
pinctrl-single,gpio-range = range 0 3 0 range 3 9 1;
@@ -204,6 +208,7 @@ pmx_gpio: pinmux@d401e000 {
0xdc 0x118
0xde 0
;
+ interrupts = 74;
};
};
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 0f178d1..7cb7940 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -19,6 +19,8 @@
#include linux/of.h
#include linux/of_device.h
#include linux/of_address.h
+#include linux/of_gpio.h
+#include linux/of_irq.h
#include linux/pinctrl/pinctrl.h
#include linux/pinctrl/pinmux.h
@@ -95,6 +97,8 @@ struct pcs_conf_type {
* @nvals: number of entries in vals array
* @pgnames: array of pingroup names the function uses
* @npgnames: number of pingroup names the function uses
+ * @irq: optional irq associated with the function
+ * @gpio: optional gpio associated with the function
* @node: list node
*/
struct pcs_function {
@@ -105,6 +109,8 @@ struct pcs_function {
int npgnames;
struct pcs_conf_vals *conf;
int nconfs;
+ int irq;
+ int gpio;
struct list_head node;
};
@@ -410,6 +416,18 @@ static int pcs_get_function(struct pinctrl_dev *pctldev,
unsigned pin,
return 0;
}
+static void pcs_reg_init(struct pcs_reg *p, struct pcs_device *pcs,
+struct pcs_function *func,
+void __iomem *reg, unsigned val)
+{
+ p-read = pcs-read;
+ p-write = pcs-write;
+ p-irq = func-irq;
+ p-gpio = func-gpio;
+ p-reg = reg;
+ p-val = val;
+}
+
static int pcs_enable(struct pinctrl_dev *pctldev, unsigned fselector,
unsigned group)
{
@@ -442,6 +460,12 @@ static int pcs_enable(struct pinctrl_dev *pctldev,
unsigned fselector,
val = ~mask;
val |= (vals-val mask);
pcs-write(val, vals-reg);
+ if ((func-irq || func-gpio) pcs-soc pcs-soc-enable) {
+ struct pcs_reg pcsr;
+
+ pcs_reg_init(pcsr, pcs, func, vals-reg, val);
+ pcs-soc-enable(pcs-soc, pcsr);
+ }
}
return 0;
@@ -466,18 +490,6 @@ static void pcs_disable(struct pinctrl_dev *pctldev,
unsigned fselector,
return;
}
- /*
-* Ignore disable if function-off is not specified. Some hardware
-* does not have clearly defined disable function. For pin specific
-* off modes, you can use alternate named states as described in
-* pinctrl-bindings.txt.
-*/
- if (pcs-foff == PCS_OFF_DISABLED) {
- dev_dbg(pcs-dev, ignoring disable for %s function%i\n,
- func-name, fselector);
- return;
- }
-
dev_dbg(pcs-dev, disabling function%i %s\n,
fselector, func-name);
@@ -488,8 +500,28 @@ static void pcs_disable(struct pinctrl_dev *pctldev,
unsigned fselector,
vals = func-vals[i];
val = pcs-read(vals-reg);
val = ~pcs-fmask;
-
