Re: [PATCH 3/6] pinctrl: single: Prepare for supporting SoC specific features
On Tue, Oct 8, 2013 at 7:55 PM, Linus Walleij linus.wall...@linaro.org wrote: On Mon, Oct 7, 2013 at 7:35 PM, Tony Lindgren t...@atomide.com wrote: Hi Linus W, Any comments on the pinctrl patches 3 - 5 in this series? I have no problems with this patch #3, as it is just changing syntax, not semantics. The problems start with patch #4. I am tormented with mixed feelings about this, because from one point of view I feel it is breaking the promise of pinctrl-single being a driver for platforms where a pin is controlled by a *single* register. If this was pinctrl-foo.c I would not have been so much bothered, but now it is something that was supposed to be self-contained and simple, pertaining to a single register, starting to look like something else. This is a bit like: oh yeah just one register controls the pins, but under some circumstances I also want to mess with this register over here, and then this register over there ... etc. I'd like Haojian to ACK this to proceed since he's also using this driver now. Then I feel better on continuing down this road ... Then I have a lesser comment on patch #4 since it makes it possible for this pin controller to support wake-up interrupt, as I don't see how this plays out with front-end GPIO controllers, but let's discuss that in the context of that patch. Yours, Linus Walleij I'm OK on both #3 #4. So Acked-by: Haojian Zhuang haojian.zhu...@gmail.com Regards Haojian -- 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 1/4] pinctrl: single: Prepare for supporting SoC specific features
On Sat, Jun 8, 2013 at 4:50 AM, Tony Lindgren t...@atomide.com wrote:
Let's replace is_pinconf with flags and add struct pcs_soc so we
can support also other features like pin wake-up events. Let's
export the probe so the SoC specific modules can pass their
SoC specific data to pinctrl-single if needed.
Done in collaboration with Roger Quadros rog...@ti.com.
Manjunathappa's pinctrl-single patch on enhancing bits is already merged.
This patch conflicts with his patch.
Could you rebase your patches?
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
---
drivers/pinctrl/pinctrl-single.c | 53
+++---
drivers/pinctrl/pinctrl-single.h | 15 +++
2 files changed, 52 insertions(+), 16 deletions(-)
create mode 100644 drivers/pinctrl/pinctrl-single.h
diff --git a/drivers/pinctrl/pinctrl-single.c
b/drivers/pinctrl/pinctrl-single.c
index b9fa046..0f178d1 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -1368,7 +1367,8 @@ static int pcs_probe(struct platform_device *pdev)
INIT_LIST_HEAD(pcs-pingroups);
INIT_LIST_HEAD(pcs-functions);
INIT_LIST_HEAD(pcs-gpiofuncs);
- pcs-is_pinconf = match-data;
+ pcs-flags = soc-flags;
+ pcs-soc = soc;
PCS_GET_PROP_U32(pinctrl-single,register-width, pcs-width,
register width not specified\n);
@@ -1437,7 +1437,7 @@ static int pcs_probe(struct platform_device *pdev)
pcs-desc.name = DRIVER_NAME;
pcs-desc.pctlops = pcs_pinctrl_ops;
pcs-desc.pmxops = pcs_pinmux_ops;
- if (pcs-is_pinconf)
+ if (pcs-flags PCS_HAS_PINCONF)
pcs-desc.confops = pcs_pinconf_ops;
pcs-desc.owner = THIS_MODULE;
@@ -1466,8 +1466,20 @@ free:
return ret;
}
+EXPORT_SYMBOL_GPL(pinctrl_single_probe);
+
+static int pcs_probe(struct platform_device *pdev)
+{
+ const struct of_device_id *match;
+
+ match = of_match_device(pcs_of_match, pdev-dev);
+ if (!match)
+ return -EINVAL;
+
+ return pinctrl_single_probe(pdev, (struct pcs_soc *)match-data);
+}
I think that you should declare pcs_probe() as EXPORT_SYMBOL_GPL.
Is it right?
-static int pcs_remove(struct platform_device *pdev)
+int pinctrl_single_remove(struct platform_device *pdev)
{
struct pcs_device *pcs = platform_get_drvdata(pdev);
@@ -1478,17 +1490,26 @@ static int pcs_remove(struct platform_device *pdev)
return 0;
}
+EXPORT_SYMBOL_GPL(pinctrl_single_remove);
Since you redefined pcs_probe(), you needn't change pcs_remove to
pinctrl_single_remove().
+
+static struct pcs_soc pinctrl_single = {
+ .flags = 0,
+};
+
+static struct pcs_soc pinconf_single = {
+ .flags = PCS_HAS_PINCONF,
+};
static struct of_device_id pcs_of_match[] = {
- { .compatible = pinctrl-single, .data = (void *)false },
- { .compatible = pinconf-single, .data = (void *)true },
+ { .compatible = pinctrl-single, .data = pinctrl_single },
+ { .compatible = pinconf-single, .data = pinconf_single },
{ },
};
MODULE_DEVICE_TABLE(of, pcs_of_match);
static struct platform_driver pcs_driver = {
.probe = pcs_probe,
- .remove = pcs_remove,
+ .remove = pinctrl_single_remove,
.driver = {
.owner = THIS_MODULE,
.name = DRIVER_NAME,
diff --git a/drivers/pinctrl/pinctrl-single.h
b/drivers/pinctrl/pinctrl-single.h
new file mode 100644
index 000..18f3205
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-single.h
@@ -0,0 +1,15 @@
Do you need append #ifndef __XX_H to protect head file
over loading?
+#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
+ */
+struct pcs_soc {
+ void *data;
+ unsigned flags;
+};
+
+extern int pinctrl_single_probe(struct platform_device *pdev,
+ const struct pcs_soc *soc);
+extern int pinctrl_single_remove(struct platform_device *pdev);
--
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 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
Re: [PATCH 1/4] pinctrl: single: Prepare for supporting SoC specific features
On Sat, Jun 8, 2013 at 11:27 PM, Tony Lindgren t...@atomide.com wrote: * Haojian Zhuang haojian.zhu...@gmail.com [130608 02:43]: Manjunathappa's pinctrl-single patch on enhancing bits is already merged. This patch conflicts with his patch. Could you rebase your patches? Sure. Looks like Linus W forgot to push out the branch as I don't see it yet in the pinctrl tree. Here's a version of this one against current Linux next + Manjunathappa's patches. Regards, Tony Acked-by: Haojian Zhuang haojian.zhu...@gmail.com -- 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 3/4] pinctrl: single: omap: Add SoC specific module for wake-up events
On Sat, Jun 8, 2013 at 4:50 AM, Tony Lindgren t...@atomide.com wrote: For wake-up events from deeper idle modes we need to check the configured padconf registers for the wake-up bit and then call the related interrupt handler. 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 --- drivers/pinctrl/Makefile |3 drivers/pinctrl/pinctrl-single-omap.c | 287 + include/linux/platform_data/pinctrl-single-omap.h |4 3 files changed, 293 insertions(+), 1 deletion(-) create mode 100644 drivers/pinctrl/pinctrl-single-omap.c create mode 100644 include/linux/platform_data/pinctrl-single-omap.h The hardware behavior likes PXA3xx. Acked-by: Haojian Zhuang haojian.zhu...@gmail.com Regards Haojian -- 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: [RESEND] [PATCH 3.6.0- 3/6] ARM/pxa: use module_platform_driver macro
On Tue, Oct 16, 2012 at 6:59 PM, Igor Grinberg grinb...@compulab.co.il wrote: On 10/12/12 09:11, Srinivas KANDAGATLA wrote: From: Srinivas Kandagatla srinivas.kandaga...@st.com This patch removes some code duplication by using module_platform_driver. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com Acked-by: Igor Grinberg grinb...@compulab.co.il Applied Thanks Haojian -- 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] ARM: remove header files included more than once
On Sat, Dec 10, 2011 at 12:25 AM, Omar Ramirez Luna omar.rami...@ti.com wrote: checkincludes.pl complains about these: linux/debugfs.h linux/dma-mapping.h linux/gpio.h linux/sched.h linux/slab.h plat/common.h plat/i2c.h Signed-off-by: Omar Ramirez Luna omar.rami...@ti.com --- arch/arm/mach-bcmring/dma.c | 1 - arch/arm/mach-ks8695/leds.c | 1 - arch/arm/mach-mmp/aspenite.c | 1 - arch/arm/mach-mmp/tavorevb.c | 1 - arch/arm/mach-msm/board-msm7x30.c | 1 - arch/arm/mach-msm/board-qsd8x50.c | 1 - arch/arm/mach-omap2/board-ldp.c | 1 - arch/arm/mach-omap2/io.c | 1 - arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 1 - arch/arm/mach-pxa/pxa25x.c | 1 - arch/arm/mach-pxa/pxa27x.c | 1 - arch/arm/mach-pxa/saarb.c | 1 - arch/arm/mach-shmobile/board-ag5evm.c | 1 - arch/arm/mach-ux500/board-mop500-u8500uib.c | 1 - arch/arm/plat-omap/clock.c | 1 - arch/arm/plat-samsung/dev-backlight.c | 1 - 16 files changed, 0 insertions(+), 16 deletions(-) Hi Omar, Thanks for your patch. Could you help to split your patches into small pieces? Since different people are handling for different files. For example, I can help to handle files in arch-pxa. But I can't handle other files. Best Regards Haojian -- 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: [RFC PATCH v2] Consolidate SRAM support
On Mon, Apr 18, 2011 at 4:52 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: This is the second revision of this patch. I've not moved it out of ARM yet as I haven't had a positive response from SH yet. It's now called pv_pool (for phys/virt pool) rather than sram_pool, and I've included MXC's iram support in this. Hopefully, if OMAP can remove the FB stuff from SRAM we can clean the OMAP bits up a little more. Neither have I sorted out the last reference to omap_sram_ceil. Some comments from OMAP people on what's going on there would be good. On Fri, Apr 15, 2011 at 02:06:07PM +0100, Russell King - ARM Linux wrote: This is work in progress. We have two SoCs using SRAM, both with their own allocation systems, and both with their own ways of copying functions into the SRAM. Let's unify this before we have additional SoCs re-implementing this obviously common functionality themselves. Unfortunately, we end up with code growth through doing this, but that will become a win when we have another SoC using this (which I know there's at least one in the pipeline). One of the considerations here is that we can easily convert sram-pool.c to hook into device tree stuff, which can tell the sram allocator: - physical address of sram - size of sram - allocation granularity and then we just need to ensure that it is appropriately mapped. This uses the physical address, and unlike Davinci's dma address usage, it always wants to have the physical address, and will always return the corresponding physical address when passed that pointer. OMAP could probably do with some more work to make the omapfb and other allocations use the sram allocator, rather than hooking in before the sram allocator is initialized - and then further cleanups so that we have an initialization function which just does sram_create(phys, size) virt = map sram(phys, size) create sram pool(virt, phys, size, min_alloc_order) Another question is whether we should allow multiple SRAM pools or not - this code does allow multiple pools, but so far we only have one pool per SoC. Overdesign? Maybe, but it prevents SoCs wanting to duplicate it if they want to partition the SRAM, or have peripheral-local SRAMs. Multiple SRAM pool does exist in Marvell MMP2 silicon. So it won't be overdesign. Lastly, uio_pruss should probably take the SRAM pool pointer via platform data so that it doesn't have to include Davinci specific includes. -- 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: [RFC PATCH] Consolidate SRAM support
On Fri, Apr 15, 2011 at 9:06 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: This is work in progress. We have two SoCs using SRAM, both with their own allocation systems, and both with their own ways of copying functions into the SRAM. Let's unify this before we have additional SoCs re-implementing this obviously common functionality themselves. Unfortunately, we end up with code growth through doing this, but that will become a win when we have another SoC using this (which I know there's at least one in the pipeline). One of the considerations here is that we can easily convert sram-pool.c to hook into device tree stuff, which can tell the sram allocator: - physical address of sram - size of sram - allocation granularity and then we just need to ensure that it is appropriately mapped. This uses the physical address, and unlike Davinci's dma address usage, it always wants to have the physical address, and will always return the corresponding physical address when passed that pointer. OMAP could probably do with some more work to make the omapfb and other allocations use the sram allocator, rather than hooking in before the sram allocator is initialized - and then further cleanups so that we have an initialization function which just does sram_create(phys, size) virt = map sram(phys, size) create sram pool(virt, phys, size, min_alloc_order) Another question is whether we should allow multiple SRAM pools or not - this code does allow multiple pools, but so far we only have one pool per SoC. Overdesign? Maybe, but it prevents SoCs wanting to duplicate it if they want to partition the SRAM, or have peripheral-local SRAMs. Lastly, uio_pruss should probably take the SRAM pool pointer via platform data so that it doesn't have to include Davinci specific includes. Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk This common sram driver is good for us. It can benefit us on DMA usage. I just have one question on SRAM for storing instruction. We still need to copy code into SRAM and flush cache TLB with this SRAM driver. TCM driver can allocate code into SRAM section in link stage. It needs to update link file and virtual memory layout. Is it worth to make SRAM driver support this behavior? The case of using SRAM as memory for instruction is switching frequency or entering/exiting low power mode in PXA silicons. Thanks Haojian -- 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
