Re: [RFC PATCH v2 00/15] DT/core: update cpu device of_node
On 17 July 2013 19:36, wrote: > 3. Added Acks from Viresh and Shawn Add it for the new cpufreq drivers included in this patchset too.. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH v2 11/15] cpufreq: kirkwood-cpufreq: remove device tree parsing for cpu nodes
On 17 July 2013 20:13, Andrew Lunn wrote: > Are we not going a bit backwards here? You are replacing two lines > with 10 lines. > > How about putting these 10 lines into some helper, > of_get_cpu_device()? It would be useful for spear, kirkwood and > imx6q, and maybe others. +1 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 6/7] ARM: spear: remove #gpio-ranges-cells property
On Fri, Jun 14, 2013 at 2:29 AM, Stephen Warren wrote: > From: Stephen Warren > > This property is no longer required by the GPIO binding. Remove it. > > Signed-off-by: Stephen Warren > --- > This should presumably be applied along with the previous changes > --- > arch/arm/boot/dts/spear1310.dtsi | 1 - > arch/arm/boot/dts/spear1340.dtsi | 1 - > arch/arm/boot/dts/spear310.dtsi | 1 - > arch/arm/boot/dts/spear320.dtsi | 2 -- > 4 files changed, 5 deletions(-) Acked-by: Viresh Kumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2] mtd: fsmc_nand: fix error return code in fsmc_nand_probe()
On 14 May 2013 08:00, Wei Yongjun wrote: > From: Wei Yongjun > > Fix to return -ENODEV in the dma channel request error handling > case instead of 0, as done elsewhere in this function. > > Signed-off-by: Wei Yongjun > --- > v1 -> v2: set ret for error cases only > --- > drivers/mtd/nand/fsmc_nand.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c > index 0e5a1d9..95e1768 100644 > --- a/drivers/mtd/nand/fsmc_nand.c > +++ b/drivers/mtd/nand/fsmc_nand.c > @@ -1035,12 +1035,14 @@ static int __init fsmc_nand_probe(struct > platform_device *pdev) > host->read_dma_chan = dma_request_channel(mask, filter, > pdata->read_dma_priv); > if (!host->read_dma_chan) { > + ret = -ENODEV; > dev_err(&pdev->dev, "Unable to get read dma > channel\n"); > goto err_req_read_chnl; > } > host->write_dma_chan = dma_request_channel(mask, filter, > pdata->write_dma_priv); > if (!host->write_dma_chan) { > + ret = -ENODEV; > dev_err(&pdev->dev, "Unable to get write dma > channel\n"); > goto err_req_write_chnl; > } > Acked-by: Viresh Kumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] mtd: fsmc_nand: fix error return code in fsmc_nand_probe()
On 13 May 2013 12:25, Wei Yongjun wrote: > From: Wei Yongjun > > Fix to return -ENODEV in the dma channel request error handling > case instead of 0, as done elsewhere in this function. > > Signed-off-by: Wei Yongjun > --- > drivers/mtd/nand/fsmc_nand.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c > index 911e243..6988fc8 100644 > --- a/drivers/mtd/nand/fsmc_nand.c > +++ b/drivers/mtd/nand/fsmc_nand.c > @@ -1032,6 +1032,7 @@ static int __init fsmc_nand_probe(struct > platform_device *pdev) > case USE_DMA_ACCESS: > dma_cap_zero(mask); > dma_cap_set(DMA_MEMCPY, mask); > + ret = -ENODEV; > host->read_dma_chan = dma_request_channel(mask, filter, > pdata->read_dma_priv); > if (!host->read_dma_chan) { > Better do it for error cases only.. like: > if (!host->read_dma_chan) { ret = -ENODEV; ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH v2 10/13] ARM: mach-spear: cpus/cpu nodes dts updates
On 22 April 2013 20:57, Lorenzo Pieralisi wrote: > This patch updates the in-kernel dts files according to the latest cpus > and cpu bindings updates for ARM. > > Signed-off-by: Lorenzo Pieralisi > --- > arch/arm/boot/dts/spear3xx.dtsi | 2 +- > arch/arm/boot/dts/spear600.dtsi | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) Acked-by: Viresh Kumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue
On 14 April 2013 01:43, Francesco Lavra wrote: >> + for_each_child_of_node(of_find_node_by_path("/cpus"), np) { > > If of_find_node_by_path() returns NULL, there will be a NULL pointer > dereference. > >> + if (count++ != cpu_dev->id) >> + continue; >> + if (!of_get_property(np, "operating-points", NULL)) >> + return -ENODATA; >> + >> + cpu_dev->of_node = np; >> + >> + ret = of_init_opp_table(cpu_dev); >> + if (ret) >> + return ret; >> + >> + return 0; > > of_node_put() should be called on np before returning. > Also, the reference count of the parent node should be decremented as well. > > These comments apply to the below function dt_get_transition_latency() too. Thanks Francesco. Below fixes this (I will send it separately to Rafael): Subject: [PATCH 1/2] cpufreq: ARM big LITTLE: put DT nodes after using them DT nodes should be put using of_node_put() to balance their usage counts. This is not done properly in ARM's big LITTLE driver. Fix it. Signed-off-by: Viresh Kumar --- drivers/cpufreq/arm_big_little_dt.c | 43 + 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/drivers/cpufreq/arm_big_little_dt.c b/drivers/cpufreq/arm_big_little_dt.c index 452ff46..44be311 100644 --- a/drivers/cpufreq/arm_big_little_dt.c +++ b/drivers/cpufreq/arm_big_little_dt.c @@ -31,22 +31,28 @@ static int dt_init_opp_table(struct device *cpu_dev) { - struct device_node *np = NULL; + struct device_node *np, *parent; int count = 0, ret; - for_each_child_of_node(of_find_node_by_path("/cpus"), np) { + parent = of_find_node_by_path("/cpus"); + if (!parent) { + pr_err("failed to find OF /cpus\n"); + return -ENOENT; + } + + for_each_child_of_node(parent, np) { if (count++ != cpu_dev->id) continue; - if (!of_get_property(np, "operating-points", NULL)) - return -ENODATA; - - cpu_dev->of_node = np; - - ret = of_init_opp_table(cpu_dev); - if (ret) - return ret; - - return 0; + if (!of_get_property(np, "operating-points", NULL)) { + ret = -ENODATA; + } else { + cpu_dev->of_node = np; + ret = of_init_opp_table(cpu_dev); + } + of_node_put(np); + of_node_put(parent); + + return ret; } return -ENODEV; @@ -54,15 +60,24 @@ static int dt_init_opp_table(struct device *cpu_dev) static int dt_get_transition_latency(struct device *cpu_dev) { - struct device_node *np = NULL; + struct device_node *np, *parent; u32 transition_latency = CPUFREQ_ETERNAL; int count = 0; - for_each_child_of_node(of_find_node_by_path("/cpus"), np) { + parent = of_find_node_by_path("/cpus"); + if (!parent) { + pr_err("failed to find OF /cpus\n"); + return -ENOENT; + } + + for_each_child_of_node(parent, np) { if (count++ != cpu_dev->id) continue; of_property_read_u32(np, "clock-latency", &transition_latency); + of_node_put(np); + of_node_put(parent); + return 0; } ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue
On 26 March 2013 18:47, Rob Herring wrote: > On 03/26/2013 04:51 AM, Viresh Kumar wrote: > I fail to see anything bL specific here. This is just multi-cluster, but > even for that I don't see anything new other than simply allowing per > cpu or per cluster opp's. The fact that we have one opp for a cluster is > simply an implementation decision in CortexA cores. For now it is a very simple driver and yes probably multi-cluster only. It will be updated with In Kernel Switcher support as soon as Linaro makes a decision to opensource that (likely to happen soon). Yes, there can be implementations, like krait, which can have per cpu control of opp, but we will wait for any such platform before making this driver complex. >> +NOTE: Cpus should boot in the order specified in DT and all cpus for a >> cluster >> +must be present contiguously. Generic DT driver will check only node 'x' for >> +cpu:x. > > The OS cannot necessarily control the order. The OS should be able to > boot on which ever core comes up first. This should be a FIXME instead (will update it). I am waiting for final DT bindings from ARM guys before this can be written on stone. For now this is the limitation and it should go away as soon as DT bindings are finalized. >> +Optional properties: >> +- clock-latency: Specify the possible maximum transition latency for clock, >> + in unit of nanoseconds. > > Don't we already have "transition-latency" defined? No, its clock-latency. $ git grep "clock-latency" Documentation/devicetree/bindings/ Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt:- clock-latency: Specify the possible maximum transition latency for clock, Documentation/devicetree/bindings/cpufreq/cpufreq-spear.txt:- clock-latency: Specify the possible maximum transition latency for clock, in Though transition-latency is mentioned by the example of cpufreq-cpu0. I will fix it. > Don't create > something new. Ideally, this should have had "-ns" appended, but the > binding is already defined. I had the same intention. >> diff --git a/drivers/cpufreq/arm_big_little.c >> b/drivers/cpufreq/arm_big_little.c > What is specific to bL? This looks like just a multi-cluster cpufreq > driver and should be generalized to that. Also, why can't the existing > cpufreq-cpu0 driver be extended to support this? With IKS code in, it will be a complete big LITTLE driver. >> +/* Currently we support only two clusters */ >> +#define MAX_CLUSTERS 2 > > Why? Because that is what the define is or there are other limitations? > Seems like this could and should be dynamically discovered with DT. To keep it simple, as for now as all early systems (atleast which are announced) have dual cluster systems only. We will update it once we have some real systems breaking this law. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH V2] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue
big LITTLE is ARM's new Architecture focussing power/performance needs of modern world. More information about big LITTLE can be found here: http://www.arm.com/products/processors/technologies/biglittleprocessing.php http://lwn.net/Articles/481055/ In order to keep cpufreq support for all big LITTLE platforms simple/generic, this patch tries to add a generic cpufreq driver layer for all big LITTLE platforms. The driver is divided into two parts: - Core driver: Generic and shared across all big LITTLE SoC's - Glue drivers: Per platform drivers providing ops to the core driver This patch adds in a generic glue driver which would extract information from Device Tree. Future SoC's can either reuse the DT glue or write their own depending on the need. Signed-off-by: Sudeep KarkadaNagesha Signed-off-by: Viresh Kumar --- V1->V2: - It was reviewed here earlier: https://lkml.org/lkml/2013/3/4/614 - It supports OPP library now and doesn't create a new binding for cpufreq table - It doesn't add any dependency on cluster node in DT, rather we work with existing cpu nodes, Documentation updated. - cpu_dev used because of OPP library and hence dev_err/dbg/info used at multiple places. - Interface with glue driver updated a bit - IS_ERR_OR_NULL replaced with IS_ERR for clk_get - clk_get_sys used instead of clk_get and name of clk is also updated - Few more minor cleanups done. It is pushed here: http://git.linaro.org/gitweb?p=people/vireshk/linux.git;a=shortlog;h=refs/heads/cpufreq-biglittle .../bindings/cpufreq/arm_big_little_dt.txt | 65 + MAINTAINERS| 11 + drivers/cpufreq/Kconfig.arm| 12 + drivers/cpufreq/Makefile | 4 + drivers/cpufreq/arm_big_little.c | 282 + drivers/cpufreq/arm_big_little.h | 40 +++ drivers/cpufreq/arm_big_little_dt.c| 92 +++ 7 files changed, 506 insertions(+) create mode 100644 Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt create mode 100644 drivers/cpufreq/arm_big_little.c create mode 100644 drivers/cpufreq/arm_big_little.h create mode 100644 drivers/cpufreq/arm_big_little_dt.c diff --git a/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt b/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt new file mode 100644 index 000..34a460d --- /dev/null +++ b/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt @@ -0,0 +1,65 @@ +Generic ARM big LITTLE cpufreq driver's DT glue +--- + +This is DT specific glue layer for generic cpufreq driver for big LITTLE +systems. + +Both required and optional properties listed below must be defined +under node /cpus/cpu@x. Where x is the first cpu inside a cluster. + +NOTE: Cpus should boot in the order specified in DT and all cpus for a cluster +must be present contiguously. Generic DT driver will check only node 'x' for +cpu:x. + +Required properties: +- operating-points: Refer to Documentation/devicetree/bindings/power/opp.txt + for details + +Optional properties: +- clock-latency: Specify the possible maximum transition latency for clock, + in unit of nanoseconds. + +Examples: + +cpus { + #address-cells = <1>; + #size-cells = <0>; + + cpu@0 { + compatible = "arm,cortex-a15"; + reg = <0>; + next-level-cache = <&L2>; + operating-points = < + /* kHzuV */ + 792000 110 + 396000 95 + 198000 85 + >; + clock-latency = <61036>; /* two CLK32 periods */ + }; + + cpu@1 { + compatible = "arm,cortex-a15"; + reg = <1>; + next-level-cache = <&L2>; + }; + + cpu@100 { + compatible = "arm,cortex-a7"; + reg = <100>; + next-level-cache = <&L2>; + operating-points = < + /* kHzuV */ + 792000 95 + 396000 75 + 198000 45 + >; + clock-latency = <61036>; /* two CLK32 periods */ + }; + + cpu@101 { + compatible = "arm,cortex-a7"; + reg = <101>; + next-level-cache = <&L2>; + }; +}; diff --git a/MAINTAINERS b/MAINTAINERS index 4cf5fd3..4071b71 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2206,6 +2206,17 @@ S: Maintained F: drivers/cpufreq/ F: include/linux/cpufreq.h +CPU FREQUENCY DRIVERS - ARM BIG LITTLE +M: Viresh Kumar +M: Sudeep KarkadaNagesha +L: c
Re: [PATCH V2] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue
On 22 March 2013 05:20, Rafael J. Wysocki wrote: > Please post a complete update patch if you want me to take it. I'd also would > like it to be ACKed by someone involved in the big-LITTLE work on the arch > side. Okay. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 8/8] cpufreq: omap: remove omap-cpufreq
On Fri, Mar 15, 2013 at 2:28 AM, Nishanth Menon wrote: > We can now use cpufreq-cpu0 driver using DT entries. > remove the redundant omap-cpufreq driver from the tree. > Remove MAINTAINERS file entry for the same as well. > > Cc: Kevin Hilman > Cc: "BenoƮt Cousson" > Cc: Santosh Shilimkar > Cc: Shawn Guo > Cc: Keerthy > Cc: linux-o...@vger.kernel.org > Cc: devicetree-discuss@lists.ozlabs.org > Cc: linux-arm-ker...@lists.infradead.org > Cc: cpuf...@vger.kernel.org > Cc: linux...@vger.kernel.org > > Signed-off-by: Nishanth Menon > --- > MAINTAINERS|1 - > drivers/cpufreq/Kconfig.arm|6 - > drivers/cpufreq/Makefile |1 - > drivers/cpufreq/omap-cpufreq.c | 291 > > 4 files changed, 299 deletions(-) > delete mode 100644 drivers/cpufreq/omap-cpufreq.c Acked-by: Viresh Kumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue
On 10 March 2013 23:58, Francesco Lavra wrote: > On 03/07/2013 06:14 PM, Viresh Kumar wrote: >> +void bL_cpufreq_unregister(struct cpufreq_arm_bL_ops *ops) >> +{ >> + if (arm_bL_ops != ops) { >> + pr_info("%s: Registered with: %s, can't unregister, exiting\n", >> + __func__, arm_bL_ops->name); >> + } > > The code is not doing what the info message says. Yes, following is the improvement: diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c index 8b8b07e..9d449cf 100644 --- a/drivers/cpufreq/arm_big_little.c +++ b/drivers/cpufreq/arm_big_little.c @@ -264,8 +264,9 @@ EXPORT_SYMBOL_GPL(bL_cpufreq_register); void bL_cpufreq_unregister(struct cpufreq_arm_bL_ops *ops) { if (arm_bL_ops != ops) { - pr_info("%s: Registered with: %s, can't unregister, exiting\n", + pr_err("%s: Registered with: %s, can't unregister, exiting\n", __func__, arm_bL_ops->name); + return; } ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue
On 8 March 2013 14:11, Guennadi Liakhovetski wrote: > Also in your driver you're doing > > cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE); > ... > cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE); > > So, theoretically you could install such notifiers to adjust CPU voltages > (using regulators too). But adding regulator calls directly to the driver > would make it consistent with cpufreq-cpu0.c. Yes > so, if this doesn't violate > any concepts, I think, it would be good to add those when suitable systems > appear. That's what i thought :) ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue
On 8 March 2013 05:56, Guennadi Liakhovetski wrote: > I like generic drivers :) Me too :) > cpufreq-cpu0 is yet another such generic > (cpufreq) driver. Now, comparing the functionality of the two: Great!! > we see, that this driver "only" switches CPU clock frequencies. Whereas > the cpufreq-cpu0 driver also manipulates a regulator (if available) > directly. I understand, power-saving is also an important consideration > for big.LITTLE systems. So, I presume, you plan to implement voltage > switching in cpufreq notifiers? So the platform on which we are currently testing these is ARM TC2 Soc and this switching is done by the firmware instead. And so didn't went for regulator hookups initially.. Obviously in future regulator hookups would find some space in this driver but not required for now. > Now, my question is: is this (notifier) > actually the preferred method and the cpufreq-cpu0 driver is doing it > "wrongly?" What notifiers are you talking about? I believe using the regulator framework is the right way of doing this. And that would be part of this code later on. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH V2] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue
big LITTLE is ARM's new Architecture focussing power/performance needs of modern world. More information about big LITTLE can be found here: http://www.arm.com/products/processors/technologies/biglittleprocessing.php http://lwn.net/Articles/481055/ In order to keep cpufreq support for all big LITTLE platforms simple/generic, this patch tries to add a generic cpufreq driver layer for all big LITTLE platforms. The driver is divided into two parts: - Core driver: Generic and shared across all big LITTLE SoC's - Glue drivers: Per platform drivers providing ops to the core driver This patch adds in a generic glue driver which would extract information from Device Tree. Future SoC's can either reuse the DT glue or write their own depending on the need. Signed-off-by: Sudeep KarkadaNagesha Signed-off-by: Viresh Kumar --- V1->V2: - s/IS_ERR_OR_NULL/IS_ERR for return value of clk_get - Added "depends on HAVE_CLK" in Kconfig - Removed "default n" from Kconfig - Renamed cluster clk to "cpu-cluster.%u" and added it as dev_id instead of con_id and using clk_get_sys() instead of clk_get() - Removed setting clk to freq_table to NULL on put_clk_and_freq_table call. .../bindings/cpufreq/arm_big_little_dt.txt | 29 +++ MAINTAINERS| 11 + drivers/cpufreq/Kconfig.arm| 12 + drivers/cpufreq/Makefile | 4 + drivers/cpufreq/arm_big_little.c | 276 + drivers/cpufreq/arm_big_little.h | 40 +++ drivers/cpufreq/arm_big_little_dt.c| 125 ++ 7 files changed, 497 insertions(+) create mode 100644 Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt create mode 100644 drivers/cpufreq/arm_big_little.c create mode 100644 drivers/cpufreq/arm_big_little.h create mode 100644 drivers/cpufreq/arm_big_little_dt.c diff --git a/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt b/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt new file mode 100644 index 000..6f534eb --- /dev/null +++ b/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt @@ -0,0 +1,29 @@ +Generic ARM big LITTLE cpufreq driver's DT glue +--- + +It is DT specific glue layer for generic cpufreq driver for big LITTLE systems. + +Both required and optional properties listed below must be defined under node +cluster*. * can be 0 or 1. + +Required properties: +- freqs: List of all supported frequencies. + +Optional properties: +- clock-latency: Specify the possible maximum transition latency for clock, in + unit of nanoseconds. + +Examples: + +cluster0: cluster@0 { +.. + + freqs = <5 6 7 8 9 10 11 12>; + clock-latency = <20>; + + .. + +cores { + .. +}; +}; diff --git a/MAINTAINERS b/MAINTAINERS index 554fd30..b14b749 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2206,6 +2206,17 @@ S: Maintained F: drivers/cpufreq/ F: include/linux/cpufreq.h +CPU FREQUENCY DRIVERS - ARM BIG LITTLE +M: Viresh Kumar +M: Sudeep KarkadaNagesha +L: cpuf...@vger.kernel.org +L: linux...@vger.kernel.org +W: http://www.arm.com/products/processors/technologies/biglittleprocessing.php +S: Maintained +F: drivers/cpufreq/arm_big_little.h +F: drivers/cpufreq/arm_big_little.c +F: drivers/cpufreq/arm_big_little_dt.c + CPUID/MSR DRIVER M: "H. Peter Anvin" S: Maintained diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index 030ddf6..87b7e48 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -2,6 +2,18 @@ # ARM CPU Frequency scaling drivers # +config ARM_BIG_LITTLE_CPUFREQ + tristate + depends on ARM_CPU_TOPOLOGY + +config ARM_DT_BL_CPUFREQ + tristate "Generic ARM big LITTLE CPUfreq driver probed via DT" + select ARM_BIG_LITTLE_CPUFREQ + depends on OF && HAVE_CLK + help + This enables the Generic CPUfreq driver for ARM big.LITTLE platform. + This gets frequency tables from DT. + config ARM_OMAP2PLUS_CPUFREQ bool "TI OMAP2+" depends on ARCH_OMAP2PLUS diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index 863fd18..d1b0832 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -44,6 +44,10 @@ obj-$(CONFIG_X86_INTEL_PSTATE) += intel_pstate.o ## # ARM SoC drivers +obj-$(CONFIG_ARM_BIG_LITTLE_CPUFREQ) += arm_big_little.o +# big LITTLE per platform glues. Keep DT_BL_CPUFREQ as the last entry in all big +# LITTLE drivers, so that it is probed last. +obj-$(CONFIG_ARM_DT_BL_
Re: [PATCH 0/4] dw_dmac: introduce generic DMA binding for DT
On 15 February 2013 23:51, Arnd Bergmann wrote: > As Andy pointed out today, we don't have a good solution for the > dw_dmac DT binding in linux-next yet. I have posted my series > once before and then got distracted after getting feedback from > Viresh, Andy and Russell. I have now updated my earlier patch > based on the feedback and rebased on your tree without any > of the arm-soc patches mixed in. Acked-by: Viresh Kumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/4] dmaengine: dw_dmac: move to generic DMA binding
On 15 February 2013 23:51, Arnd Bergmann wrote: > diff --git a/Documentation/devicetree/bindings/dma/snps-dma.txt > b/Documentation/devicetree/bindings/dma/snps-dma.txt > +- #dma-cells: must be <3> > +DMA clients connected to the Designware DMA controller must use the format > +described in the dma.txt file, using a five-cell specifier for each channel. s/five/four ? > +The four cells in order are: > + > +1. A phandle pointing to the DMA controller > +2. The DMA request line number > +3. Source master for transfers on allocated channel > +4. Destination master for transfers on allocated channel > diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c > +static bool dw_dma_generic_filter(struct dma_chan *chan, void *param) > { > + dws->cfg_hi = 0x; > + dws->cfg_lo = 0x; s/0x/-1 ? > + dws->src_master = fargs->src; > + dws->dst_master = fargs->dst; > + dwc->req= fargs->req; > > - return true; > - } > - } > + chan->private = dws; > + > + return true; > +} > + > +static struct dma_chan *dw_dma_xlate(struct of_phandle_args *dma_spec, > +struct of_dma *ofdma) > +{ > + struct dw_dma *dw = ofdma->of_dma_data; > + struct dw_dma_filter_args fargs = { > + .dw = dw, > + }; > + dma_cap_mask_t cap; > + > + if (dma_spec->args_count != 3) > + return NULL; > + > + fargs.req = be32_to_cpup(dma_spec->args+0); > + fargs.src = be32_to_cpup(dma_spec->args+1); > + fargs.dst = be32_to_cpup(dma_spec->args+2); > + > + if (WARN_ON(fargs.req >= 16 || fargs.src >= 2 || fargs.dst >= 2)) > + return NULL; > > - last_dw = dw; > - last_bus_id = param; > - return false; > + dma_cap_zero(cap); > + dma_cap_set(DMA_SLAVE, cap); > + > + /* TODO: there should be a simpler way to do this */ > + return dma_request_channel(cap, dw_dma_generic_filter, > &dma_spec->args[0]); > } > -EXPORT_SYMBOL(dw_dma_generic_filter); > > /* - Cyclic DMA API extensions */ > > @@ -1510,9 +1529,8 @@ static void dw_dma_off(struct dw_dma *dw) > static struct dw_dma_platform_data * > dw_dma_parse_dt(struct platform_device *pdev) > { > - struct device_node *sn, *cn, *np = pdev->dev.of_node; > + struct device_node *np = pdev->dev.of_node; > struct dw_dma_platform_data *pdata; > - struct dw_dma_slave *sd; > u32 tmp, arr[4]; > > if (!np) { > @@ -1524,7 +1542,7 @@ dw_dma_parse_dt(struct platform_device *pdev) > if (!pdata) > return NULL; > > - if (of_property_read_u32(np, "nr_channels", &pdata->nr_channels)) > + if (of_property_read_u32(np, "dma-channels", &pdata->nr_channels)) > return NULL; ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/5] dmaengine: dw_dmac: move to generic DMA binding
On 30 January 2013 15:38, Arnd Bergmann wrote: > I meant the spear13xx data sheet, which has to list the request lines > for its integrated components. There may be other SoCs using the > same dw_dmac, but this is the main one that is upstream now, and it's > probably as good as any other one. I just wouldn't want to establish > a binding that doesn't match any of the known implementations in the > way it expresses request lines. Your binding (https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/5] dmaengine: dw_dmac: move to generic DMA binding
On Wed, Jan 30, 2013 at 3:11 PM, Arnd Bergmann wrote: > On Wednesday 30 January 2013, Viresh Kumar wrote: >> I knew you will come to this :) >> So, the hardware is like: there are 16 request line slots per master, a >> platform can choose to connect same or separate devices to these. >> >> So, these are really 16 per master. > > Ok, I see. Do you know how these are numbered in the data sheet? > > If the convention is to have subsequent numbers for these in the > hardware description, we should probably just have that single > request number in the binding, too, and calculate the master number > from that. If it lists pairs of request/master number, we should > use pairs in the binding as well, in the same order. Actually what would be better to have is: - have this range from 0-15 only - together with the master we want to use for peripheral this should be enough. Datasheet of dw_dmac doesn't tell much about it.. just four bits for programming it and so values are from 0-15 :) >> > Ok. Would it be enough to have only one master and one request >> > field in the DT dma descriptor then, and have the code figure >> > whether to use it as source or destination, based on the >> > configuration? Which one should come first? Since you have >> > multiple masters per controller, and multiple requests per >> > master, it sounds like the cleanest descriptor form would >> > be >> > >> > ; >> > >> > Or possibly >> > >> > ; >> > >> > if the direction needs to be known at the time the channel >> > is requested. >> >> Its better to keep masters as is. So, that we can use appropriate >> masters for peripheral and memory to make the transfer fast. > > So you mean keep the format as > > ; > > ? Yes.. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/5] dmaengine: dw_dmac: move to generic DMA binding
On 29 January 2013 21:51, Arnd Bergmann wrote: > On Tuesday 29 January 2013, Viresh Kumar wrote: >> On 29 January 2013 19:01, Arnd Bergmann wrote: >> > Ah, good. So I guess the "dma-requests" property should actually >> > be "16" then. >> >> yes, even i was checking on that separately :) > > Actually, I just discovered something odd in the > arch/arm/mach-spear/spear13xx-dma.h file that gets removed > in the last patch: there, we define request numbers up to > 32, e.g. > > - SPEAR1310_DMA_REQ_UART2_RX = 14, > - SPEAR1310_DMA_REQ_UART2_TX = 15, > - SPEAR1310_DMA_REQ_UART5_RX = 16, > - SPEAR1310_DMA_REQ_UART5_TX = 17, > > What is the meaning of this, if the maximum request number is 15? I knew you will come to this :) So, the hardware is like: there are 16 request line slots per master, a platform can choose to connect same or separate devices to these. So, these are really 16 per master. > Ok. Would it be enough to have only one master and one request > field in the DT dma descriptor then, and have the code figure > whether to use it as source or destination, based on the > configuration? Which one should come first? Since you have > multiple masters per controller, and multiple requests per > master, it sounds like the cleanest descriptor form would > be > > ; > > Or possibly > > ; > > if the direction needs to be known at the time the channel > is requested. Its better to keep masters as is. So, that we can use appropriate masters for peripheral and memory to make the transfer fast. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/5] dmaengine: dw_dmac: move to generic DMA binding
On 29 January 2013 19:01, Arnd Bergmann wrote: > Ah, good. So I guess the "dma-requests" property should actually > be "16" then. yes, even i was checking on that separately :) > Does this mean that an implicit zero request line means memory? No. 0 is also request line for a peripheral and numbers are from 0 to 15. memcpy are identified by setting type of transfer as memcpy. > Could we have device-to-device DMAs with this controller, and if > we can, should we have both 'src' and 'dst' fields? Are the > two number ranges sharing the same address space, i.e. is > request '7' as the destination guaranteed to be the same device > as request '7' in the source? Request lines are per master... So, for a master single request line is independent of direction. Many DMA controllers have capability of doing dev-to-dev transfers but DMAENGINE doesn't have any support for it, even we don't have a usecase too :) > If we need two lines, we could interleave them with the bus > master numbers: not required. > In theory, we could use bit-stuffing to put them all into > a single 32 bit word I guess, but generally people don't > seem to like that for new bindings. :) -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/5] dmaengine: dw_dmac: move to generic DMA binding
On 29 January 2013 16:24, Andy Shevchenko wrote: > On Tue, 2013-01-29 at 16:19 +0530, Viresh Kumar wrote: >> if (DMA_TO_DEV) >>// dest is periph >>fargs.cfg_hi = be32_to_cpup(dma_spec->args+0) << 11; >> else if (DEV_TO_DMA) >>// src is periph >>fargs.cfg_hi = be32_to_cpup(dma_spec->args+0) << 7; > > We have macros for such shifts. > > drivers/dma/dw_dmac.c:187: cfghi = DWC_CFGH_DST_PER(... > drivers/dma/dw_dmac.c:189: cfghi = DWC_CFGH_SRC_PER(... I am getting older now, bad memory :) I grepped into drivers/dma/dw_dmac_regs.h and left include/linux/dw_dmac.h :( ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/5] dmaengine: dw_dmac: move to generic DMA binding
On 29 January 2013 16:05, Arnd Bergmann wrote: > On Tuesday 29 January 2013, Viresh Kumar wrote: >> Shouldn't this be 4? Would be better to mention what fields are these, >> right here. I have seen them below though. > > Correct. I changed these a couple of times while trying to understand > what the fields are, and I missed this instance. I'm still not sure > whether we actually need all four fields, or what the simplest format > for them would be. This just mirrors what you had in your binding. You can add request_line number and leave first two fields, cfghi and lo. >> > + /* FIXME: memory leak! could we put this into dw_dma_chan? */ >> > + sd = devm_kzalloc(dw->dma.dev, sizeof (*sd), GFP_KERNEL); >> >> Yes. > > Yes it can be in dw_dma_chan or yes it is a memory leak? Yes it can be in dw_dma_chan :) >> > + if (dma_spec->args_count != 4) >> >> args_count contains count of all params leaving the phandle? > > That was my interpretation from reading the code, but I have not tried it. Okay, it was just a question from my side :) >> > + /* FIXME: This binding is rather clumsy. Can't we use the >> > + request line numbers here instead? */ >> >> yes. > > Ok, Very good. What is the encoding of the registers then? You can still keep fargs as is and just fill them as: fargs.cfg_lo = 0; if (DMA_TO_DEV) // dest is periph fargs.cfg_hi = be32_to_cpup(dma_spec->args+0) << 11; else if (DEV_TO_DMA) // src is periph fargs.cfg_hi = be32_to_cpup(dma_spec->args+0) << 7; The field size is 4 bits. > Thanks a lot for the input. When I fix the above, are actually able > to test the changes, or have you lost access to the hardware when > leaving ST? I don't have any sort of access for testing these :( But, Vipul might try these at his end. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 4/5] ata: arasan: remove the need for platform_data
On 29 January 2013 03:28, Arnd Bergmann wrote: > This adds a complete DT binding for the arasan device driver. There is > currently only one user, which is the spear13xx platform, so we don't > actually have to parse all the properties until another user comes in, > but this does use the generic DMA binding to find the DMA channel. > > The patch is untested so far and is part of a series to convert > the spear platform over to use the generic DMA binding, so it > should stay with the rest of the series. Acked-by: Viresh Kumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: SPEAr13xx: Pass generic DW DMAC platform data from DT
You forgot spear-devel for this series. On 29 January 2013 03:28, Arnd Bergmann wrote: > This replaces an earlier patch from Viresh Kumar to move > the spear platform over to the generic DMA binding. This > version is now based on the merged multiplatform capable > spear platform, rather than the separate spear13xx/3xx/6xx > directories. > > Signed-off-by: Arnd Bergmann > Cc: Viresh Kumar > Cc: Vinod Koul > Cc: Andy Shevchenko > Cc: devicetree-discuss@lists.ozlabs.org > --- > arch/arm/boot/dts/spear1340.dtsi | 3 + > arch/arm/boot/dts/spear13xx.dtsi | 25 +- > arch/arm/mach-spear/generic.h| 6 -- > arch/arm/mach-spear/include/mach/spear.h | 2 - > arch/arm/mach-spear/spear1310.c | 30 +--- > arch/arm/mach-spear/spear1340.c | 32 +--- > arch/arm/mach-spear/spear13xx-dma.h | 128 > --- > arch/arm/mach-spear/spear13xx.c | 58 -- > 8 files changed, 29 insertions(+), 255 deletions(-) > delete mode 100644 arch/arm/mach-spear/spear13xx-dma.h > > diff --git a/arch/arm/boot/dts/spear1340.dtsi > b/arch/arm/boot/dts/spear1340.dtsi > index 34da11a..e1786a0 100644 > --- a/arch/arm/boot/dts/spear1340.dtsi > +++ b/arch/arm/boot/dts/spear1340.dtsi > @@ -113,6 +113,9 @@ > reg = <0xb410 0x1000>; > interrupts = <0 105 0x4>; > status = "disabled"; > + dmas = <&dwdma0 0x600 0 0 1>, /* 0xC << 11 */ > + <&dwdma0 0x680 0 1 0>; /* 0xD << 7 */ > + dma-names = "tx", "rx"; > }; > > thermal@e07008c4 { > diff --git a/arch/arm/boot/dts/spear13xx.dtsi > b/arch/arm/boot/dts/spear13xx.dtsi > index b4ca60f..45597fd 100644 > --- a/arch/arm/boot/dts/spear13xx.dtsi > +++ b/arch/arm/boot/dts/spear13xx.dtsi > @@ -98,13 +98,24 @@ > reg = <0xb280 0x1000>; > interrupts = <0 29 0x4>; > status = "disabled"; > + dmas = <&dwdma0 0 0 0 0>; > + dma-names = "data"; > }; > > - dma@ea80 { > + dwdma0: dma@ea80 { > compatible = "snps,dma-spear1340"; > reg = <0xea80 0x1000>; > interrupts = <0 19 0x4>; > status = "disabled"; > + > + dma-channels = <8>; > + #dma-cells = <3>; 4? ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/5] dmaengine: dw_dmac: move to generic DMA binding
On 29 January 2013 03:28, Arnd Bergmann wrote: > The original device tree binding for this driver, from Viresh Kumar > unfortunately conflicted with the generic DMA binding, and did not allow > to completely seperate slave device configuration from the controller. > > This is an attempt to replace it with an implementation of the generic > binding, but it is currently completely untested, because I do not have > any hardware with this particular controller. > > The patch applies on top of linux-next, which contains both the base > support for the generic DMA binding, as well as the earlier attempt from > Viresh. Both of these are currently not merged upstream however. This was really my work and i am feeling bad that i couldn't allocate any time for it. Thanks for starting it. > diff --git a/Documentation/devicetree/bindings/dma/snps-dma.txt > b/Documentation/devicetree/bindings/dma/snps-dma.txt > index 5bb3dfb..212d387 100644 > --- a/Documentation/devicetree/bindings/dma/snps-dma.txt > +++ b/Documentation/devicetree/bindings/dma/snps-dma.txt > @@ -3,59 +3,62 @@ > Required properties: > - compatible: "snps,dma-spear1340" > - reg: Address range of the DMAC registers > -- interrupt-parent: Should be the phandle for the interrupt controller > - that services interrupts for this device > - interrupt: Should contain the DMAC interrupt number > -- nr_channels: Number of channels supported by hardware > -- is_private: The device channels should be marked as private and not for by > the > - general purpose DMA channel allocator. False if not passed. > +- dma-channels: Number of channels supported by hardware > +- dma-requests: Number of DMA request lines supported > +- dma-masters: Number of AHB masters supported by the controller > +- #dma-cells: must be <3> Shouldn't this be 4? Would be better to mention what fields are these, right here. I have seen them below though. > +DMA clients connected to the Designware DMA controller must use the format > +described in the dma.txt file, using a five-cell specifier for each channel. > +The five cells in order are: > + > +1. A phandle pointing to the DMA controller > +2. The value for the cfg_hi register. > +3. The value for the cfg_lo register. > +4. Source master for transfers on allocated channel. > +5. Destination master for transfers on allocated channel. > diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c > index 8cfaaf8..88504c2 100644 > --- a/drivers/dma/dw_dmac.c > +++ b/drivers/dma/dw_dmac.c > @@ -18,6 +18,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -1179,49 +1180,69 @@ static void dwc_free_chan_resources(struct dma_chan > *chan) > dev_vdbg(chan2dev(chan), "%s: done\n", __func__); > } > > -bool dw_dma_generic_filter(struct dma_chan *chan, void *param) > +/* forward declaration used in filter */ > +static struct platform_driver dw_driver; extern? This is not a declaration but definition. > + > +struct dw_dma_filter_args { > + struct dw_dma *dw; > + u32 cfg_lo; > + u32 cfg_hi; > + unsigned src; Currently named as sms > + unsigned dst; dms > +}; > + > +static bool dw_dma_generic_filter(struct dma_chan *chan, void *param) > { > struct dw_dma *dw = to_dw_dma(chan->device); > - static struct dw_dma *last_dw; > - static char *last_bus_id; > - int i = -1; > + struct dw_dma_filter_args *fargs = param; > + struct dw_dma_slave *sd; > > - /* > -* dmaengine framework calls this routine for all channels of all dma > -* controller, until true is returned. If 'param' bus_id is not > -* registered with a dma controller (dw), then there is no need of > -* running below function for all channels of dw. > -* > -* This block of code does this by saving the parameters of last > -* failure. If dw and param are same, i.e. trying on same dw with > -* different channel, return false. > -*/ > - if ((last_dw == dw) && (last_bus_id == param)) > + /* both the driver and the device must match */ > +if (chan->device->dev->driver != &dw_driver.driver) > +return false; Can this ever happen? Isn't it the case that this routine would be called only for dw_dmac? > + if (dw != fargs->dw) > return false; > - /* > -* Return true: > -* - If dw_dma's platform data is not filled with slave info, then all > -* dma controllers are fine for transfer. > -* - Or if param is NULL > -
Re: [PATCH V3 2/3] dmaengine: dw_dmac: Enhance device tree support
On 12 December 2012 14:10, Andy Shevchenko wrote: > Will we survive if the patch is in mainline? I mean how big the impact > of it is? It doesn't fail to do fulfill its purpose and even ALL DT stuff would work well. Its just the matter of using the right API's, design and bindings :) ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V3 2/3] dmaengine: dw_dmac: Enhance device tree support
Sorry for replying late, was too busy with other work yesterday. On 11 December 2012 03:38, Arnd Bergmann wrote: > I'm deeply sorry for the very late complaint on this, but I only now Better late than never :) > noticed this patch as I was seeing build breakage in linux-next > because of it. Oops!! Here is a fix for that Author: Viresh Kumar Date: Wed Dec 12 08:28:07 2012 +0530 ARM: SPEAr1310: Fix CF DMA data We need to pass string with device-channel name to dma controller instead of dma controller specific dma struct. Fix CF dma data. Signed-off-by: Viresh Kumar --- arch/arm/mach-spear13xx/spear1310.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mach-spear13xx/spear1310.c b/arch/arm/mach-spear13xx/spear1310.c index f65ad5b..54f0e2e 100644 --- a/arch/arm/mach-spear13xx/spear1310.c +++ b/arch/arm/mach-spear13xx/spear1310.c @@ -36,7 +36,7 @@ static struct arasan_cf_pdata cf_pdata = { .cf_if_clk = CF_IF_CLK_166M, .quirk = CF_BROKEN_UDMA, - .dma_priv = &cf_dma_priv, + .dma_priv = "cf", }; > Viresh, there are multiple problems with your approach unfortunately: > > * It does not follow the binding from > Documentation/devicetree/bindings/dma/dma.txt When i patched it, this patch wasn't there in linux-next/master. Probably was getting reviewed somewhere :) > * It requires slave drivers to know that they are using the dw_dmac > driver and pass a pointer to dw_generic_filter, which is not > generic at all > > * It requires the dmac node to have information about all slaves > > There are also some minor issues, such as the naming of DT > properties, but the above need to be resolved first. I saw the binding document and it looks it can be applied to dw_dmac too, as there is nothing special for it. The question is how? We are already late for merge window and this one is queued. Supplying a new patch, getting it reviewed/tested and being pulled by Linus is not so easy :) Two ways: - Keep it as is, and i will fix it separately and quickly - Drop it :( -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH V7] mfd: stmpe: Update DT support for stmpe driver
From: Vipul Kumar Samar This patch extends existing DT support for stmpe devices. This updates: - missing header files in stmpe.c - stmpe_of_probe() with pwm, rotator and new bindings. - Bindings are updated in binding document. Acked-by: Lee Jones Acked-by: Linus Walleij Signed-off-by: Vipul Kumar Samar Signed-off-by: Viresh Kumar --- V6->V7: - Minor grammer correction in stmpe.txt - Removed comment over pdata->id = -1 Documentation/devicetree/bindings/mfd/stmpe.txt | 9 ++--- drivers/mfd/stmpe.c | 22 -- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/Documentation/devicetree/bindings/mfd/stmpe.txt b/Documentation/devicetree/bindings/mfd/stmpe.txt index 8f0aeda..9fc1715 100644 --- a/Documentation/devicetree/bindings/mfd/stmpe.txt +++ b/Documentation/devicetree/bindings/mfd/stmpe.txt @@ -1,8 +1,11 @@ -* STMPE Multi-Functional Device +* ST Microelectronics STMPE Multi-Functional Device + +STMPE is an MFD device which may expose the following inbuilt devices: gpio, +keypad, touchscreen, adc, pwm, rotator. Required properties: - - compatible : "st,stmpe[811|1601|2401|2403]" - - reg : I2C address of the device + - compatible : "st,stmpe[610|801|811|1601|2401|2403]" + - reg : I2C/SPI address of the device Optional properties: - interrupts : The interrupt outputs from the controller diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c index 34408b4..48c4f18 100644 --- a/drivers/mfd/stmpe.c +++ b/drivers/mfd/stmpe.c @@ -7,12 +7,15 @@ * Author: Rabin Vincent for ST-Ericsson */ +#include #include #include #include #include #include #include +#include +#include #include #include #include @@ -1019,6 +1022,9 @@ void __devinit stmpe_of_probe(struct stmpe_platform_data *pdata, { struct device_node *child; + pdata->id = -1; + pdata->irq_trigger = IRQF_TRIGGER_NONE; + of_property_read_u32(np, "st,autosleep-timeout", &pdata->autosleep_timeout); @@ -1027,15 +1033,16 @@ void __devinit stmpe_of_probe(struct stmpe_platform_data *pdata, for_each_child_of_node(np, child) { if (!strcmp(child->name, "stmpe_gpio")) { pdata->blocks |= STMPE_BLOCK_GPIO; - } - if (!strcmp(child->name, "stmpe_keypad")) { + } else if (!strcmp(child->name, "stmpe_keypad")) { pdata->blocks |= STMPE_BLOCK_KEYPAD; - } - if (!strcmp(child->name, "stmpe_touchscreen")) { + } else if (!strcmp(child->name, "stmpe_touchscreen")) { pdata->blocks |= STMPE_BLOCK_TOUCHSCREEN; - } - if (!strcmp(child->name, "stmpe_adc")) { + } else if (!strcmp(child->name, "stmpe_adc")) { pdata->blocks |= STMPE_BLOCK_ADC; + } else if (!strcmp(child->name, "stmpe_pwm")) { + pdata->blocks |= STMPE_BLOCK_PWM; + } else if (!strcmp(child->name, "stmpe_rotator")) { + pdata->blocks |= STMPE_BLOCK_ROTATOR; } } } @@ -1106,6 +1113,9 @@ int __devinit stmpe_probe(struct stmpe_client_info *ci, int partnum) return -ENODEV; } stmpe->variant = stmpe_noirq_variant_info[stmpe->partnum]; + } else if (pdata->irq_trigger == IRQF_TRIGGER_NONE) { + pdata->irq_trigger = + irqd_get_trigger_type(irq_get_irq_data(stmpe->irq)); } ret = stmpe_chip_init(stmpe); -- 1.7.12.rc2.18.g61b472e ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V6] mfd: stmpe: Update DT support for stmpe driver
On 6 December 2012 22:10, Lee Jones wrote: > On Thu, 06 Dec 2012, Viresh Kumar wrote: >> + pdata->id = -1; > > Perhaps PLATFORM_DEVID_AUTO would be better? Why do we want to allocate device id's for it? Multiple devices are already distinguished and so -1 would be better. >> + pdata->irq_trigger = IRQF_TRIGGER_NONE; >> + > > There's no need for this. It's guaranteed to be zero by: > > pdata = kzalloc(sizeof(*pdata), GFP_KERNEL); I kept it so that in future others don't think that we missed handling of irq_trigger for DT case. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH V6] mfd: stmpe: Update DT support for stmpe driver
From: Vipul Kumar Samar This patch extends existing DT support for stmpe devices. This updates: - missing header files in stmpe.c - stmpe_of_probe() with pwm, rotator and new bindings. - Bindings are updated in binding document. Acked-by: Lee Jones Signed-off-by: Vipul Kumar Samar Signed-off-by: Viresh Kumar --- Hi Samuel, I have added acked by Lee as he earlier Acked this patch, most of the part is still the same. And nothing new added. V5->V6: - Removed of_devide_id tables from stmpe-i2c[spi].c Documentation/devicetree/bindings/mfd/stmpe.txt | 9 ++--- drivers/mfd/stmpe.c | 27 +++-- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/Documentation/devicetree/bindings/mfd/stmpe.txt b/Documentation/devicetree/bindings/mfd/stmpe.txt index 8f0aeda..d2d88d9 100644 --- a/Documentation/devicetree/bindings/mfd/stmpe.txt +++ b/Documentation/devicetree/bindings/mfd/stmpe.txt @@ -1,8 +1,11 @@ -* STMPE Multi-Functional Device +* ST Microelectronics STMPE Multi-Functional Device + +STMPE is an MFD device which may expose following inbuilt devices: gpio, keypad, +touchscreen, adc, pwm, rotator. Required properties: - - compatible : "st,stmpe[811|1601|2401|2403]" - - reg : I2C address of the device + - compatible : "st,stmpe[610|801|811|1601|2401|2403]" + - reg : I2C/SPI address of the device Optional properties: - interrupts : The interrupt outputs from the controller diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c index 34408b4..bb2dfdb 100644 --- a/drivers/mfd/stmpe.c +++ b/drivers/mfd/stmpe.c @@ -7,12 +7,15 @@ * Author: Rabin Vincent for ST-Ericsson */ +#include #include #include #include #include #include #include +#include +#include #include #include #include @@ -1019,6 +1022,14 @@ void __devinit stmpe_of_probe(struct stmpe_platform_data *pdata, { struct device_node *child; + /* +* Distinct names of same cell-type within multiple instances of stmpe +* will be guaranteed by DT. +*/ + pdata->id = -1; + + pdata->irq_trigger = IRQF_TRIGGER_NONE; + of_property_read_u32(np, "st,autosleep-timeout", &pdata->autosleep_timeout); @@ -1027,15 +1038,16 @@ void __devinit stmpe_of_probe(struct stmpe_platform_data *pdata, for_each_child_of_node(np, child) { if (!strcmp(child->name, "stmpe_gpio")) { pdata->blocks |= STMPE_BLOCK_GPIO; - } - if (!strcmp(child->name, "stmpe_keypad")) { + } else if (!strcmp(child->name, "stmpe_keypad")) { pdata->blocks |= STMPE_BLOCK_KEYPAD; - } - if (!strcmp(child->name, "stmpe_touchscreen")) { + } else if (!strcmp(child->name, "stmpe_touchscreen")) { pdata->blocks |= STMPE_BLOCK_TOUCHSCREEN; - } - if (!strcmp(child->name, "stmpe_adc")) { + } else if (!strcmp(child->name, "stmpe_adc")) { pdata->blocks |= STMPE_BLOCK_ADC; + } else if (!strcmp(child->name, "stmpe_pwm")) { + pdata->blocks |= STMPE_BLOCK_PWM; + } else if (!strcmp(child->name, "stmpe_rotator")) { + pdata->blocks |= STMPE_BLOCK_ROTATOR; } } } @@ -1106,6 +1118,9 @@ int __devinit stmpe_probe(struct stmpe_client_info *ci, int partnum) return -ENODEV; } stmpe->variant = stmpe_noirq_variant_info[stmpe->partnum]; + } else if (pdata->irq_trigger == IRQF_TRIGGER_NONE) { + pdata->irq_trigger = + irqd_get_trigger_type(irq_get_irq_data(stmpe->irq)); } ret = stmpe_chip_init(stmpe); -- 1.7.12.rc2.18.g61b472e ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V5 2/2] mfd: stmpe: Update DT support in stmpe driver
On 6 December 2012 16:42, Lee Jones wrote: > I thought we'd be over this? The 'ID' will be represented by the > address of the chip i.e. stmpe1601@40, where '40' will be > distinguishing factor? I haven't tested it but i thought we are getting i2c device name from modalias() fn running on "st,stmpe1601" and so would give us "stmpe1601". Now, i went back to your earlier mail and checked device name: stmpe-i2c 0-0040: stmpe1601 detected, chip id: 0x212 It came from DT :) So, probably no more issues of id's. Will resend my patch, after getting rid of tables. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V5 2/2] mfd: stmpe: Update DT support in stmpe driver
On 6 December 2012 16:05, Lee Jones wrote: >> > Or you could not put unnecessary bindings into the Device Tree >> > by putting two and two together and realise that using the table >> > is the correct thing to do instead. This actually gives reason >> > to you previous patch, but should probably be fixed-up into it >> > so it has some proper meaning/purpose. ;) >> >> Couldn't understand this one :( > > Really? I tried again, but couldn't get it :( > Let's break it down - what do you need "stmpe,id" for? To distinguish two instances of of stmpe811 (for instance) on a board. Names of stmpe sub-modules would contain .0, .1, if we have an id passed to it. Passing -1 would create same names for both gpio blocks which belonged to different stmpe811's. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V5 2/2] mfd: stmpe: Update DT support in stmpe driver
On 6 December 2012 15:41, Lee Jones wrote: > So then I'm back to my original question, why? > > What is it used for? What difference does it make? > > I could understand if the .data attribute was used in the driver > to make vital decisions based on STMPE version, but it's not. So > why are we burdening the driver with unused code that's not > required? Other than to get your mainlined patch-count up of > course? This has an air of "placing header files in alphabetical > order" about it. ;) The count would still be the same as some part of this patch will go :) I said that because of Grant's comment: "An of_match_table is the robust way of identifying specific devices and allows for matching against any entry in the compatible list." So, thought its better we keep it. Now, the problem is, with this new table we will bind device and driver based on of_device_id table and probe it using device_id table. Ahh.. that's broken. Maybe yes, can remove it unless we have a real need of it. >> By chance >> our non-DT and DT tables had a difference of "st," only in the name >> of instances and so it worked without tables. Otherwise it couldn't >> have worked. >> >> Over that, i am looking to bring the "stmpe,id" binding back again (unless >> you have a better option), as device name is not coming from DT currently, >> which we discussed earlier. > > Or you could not put unnecessary bindings into the Device Tree > by putting two and two together and realise that using the table > is the correct thing to do instead. This actually gives reason > to you previous patch, but should probably be fixed-up into it > so it has some proper meaning/purpose. ;) Couldn't understand this one :( ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V5 2/2] mfd: stmpe: Update DT support in stmpe driver
On 6 December 2012 15:20, Lee Jones wrote: >> > But regardless, it is the responsiblity of the probe function to go and >> > look if of_driver_match_device() matches against anything if it cares >> > about the of_match_table entries (for instance, if there is extra data >> > attached). >> >> Ok, so filling .data field in of_device_id[] is not required for our case as >> we aren't doing anything special in our drivers. > > This is exactly my point, and the reason I bought it up in the > first place. Normally when you specify an ID table and populate > the .data attribute, you parse for it in the code and then cast > it back to some kind of useful data. However, you're not doing > that, which is precisely why I wondered if the table was > necessary at all. In all my testing, the DT portion worked and > the correct STMPE chip was identified without it. Probably Vipul (Author of this patch), copied it from existing i2c/spi clients, which have also added this blindly :) > So, are you adding the table for good reason, or because you > think it's the right thing to do? I would be keeping the table as that's the right thing to do. By chance our non-DT and DT tables had a difference of "st," only in the name of instances and so it worked without tables. Otherwise it couldn't have worked. Over that, i am looking to bring the "stmpe,id" binding back again (unless you have a better option), as device name is not coming from DT currently, which we discussed earlier. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V5 2/2] mfd: stmpe: Update DT support in stmpe driver
On 6 December 2012 04:12, Grant Likely wrote: > On Sat, 1 Dec 2012 00:33:46 +0530, Viresh Kumar > wrote: >> This first tries to match the table my patch added, _BUT_ the string will >> never match as we had "st,stmpe810" in table and "stmpe810" in dev. > > of_driver_match_device() matches against the compatible list in > dev->of_node, not against the device name. So, if the compatible > property has a string that is in the table, then it really should match > against it. Grant, but isn't it true that the final device's name would not be the DT way of names? It would simply be "stmpe810" for us and so if we have multiple instances of stmpe on a board, we need to distinguish them ourselves? One way for that was passing id for these instances, which would finally be used, when we create platform devices for sub-modules of stmpe (gpio, keypad, ts, etc). -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V5 2/2] mfd: stmpe: Update DT support in stmpe driver
First of all, thanks for explaining :) On 6 December 2012 04:12, Grant Likely wrote: > On Sat, 1 Dec 2012 00:33:46 +0530, Viresh Kumar > wrote: >> This first tries to match the table my patch added, _BUT_ the string will >> never match as we had "st,stmpe810" in table and "stmpe810" in dev. > > of_driver_match_device() matches against the compatible list in > dev->of_node, not against the device name. So, if the compatible > property has a string that is in the table, then it really should match > against it. How could i misread it? Yes you are correct. >> static int i2c_device_probe(struct device *dev) >> { >> . >> status = driver->probe(client, i2c_match_id(driver->id_table, client)); > > Here things are a bit wonky. Even if matched against the table, it is table means of_device_id table ? > possible that it also matches against i2c_match_id() and that data is > passed to the driver. It is a possibility or guarantee ? And so whatever device name we got from modalias routine, should match with the names in driver->id_table. > But regardless, it is the responsiblity of the probe function to go and > look if of_driver_match_device() matches against anything if it cares > about the of_match_table entries (for instance, if there is extra data > attached). Ok, so filling .data field in of_device_id[] is not required for our case as we aren't doing anything special in our drivers. >> Which will again match the legacy table to find correct struct i2c_device_id >> *id >> to pass to probe(). >> >> So, the final question: WTF is of_match_table for? > > A bit of history is valuable here. The whole of_modalias_node() thing is > really just a best-effort heuristic for figuring out which driver > *might* work against a device described in the device tree. It won't > work in all circumstances (and it was created at a time when there was > resistance to adding DT knowledge to drivers). An of_match_table is the > robust way of identifying specific devices and allows for matching > against any entry in the compatible list. Got it. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V5 2/2] mfd: stmpe: Update DT support in stmpe driver
On 5 December 2012 18:49, Lee Jones wrote: >> Ping!!! > > Documentation/development-process/2.Process: > > - Avoid top-posting (the practice of putting your answer above the quoted > text you are responding to). It makes your response harder to read and > makes a poor impression. Yes, i know that. Did that in hurry, as it was just an ping and wasn't answering any question. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V5 2/2] mfd: stmpe: Update DT support in stmpe driver
Ping!!! On 1 December 2012 00:33, Viresh Kumar wrote: > On 30 November 2012 21:15, Lee Jones wrote: >> But ... I don't see how the changes in the -i2c and -spi files >> are of benefit either. When I boot without the ID table I still >> get "stmpe-i2c 0-0040: stmpe1601 detected, chip id: 0x212". >> >> What is it that actually uses the IDs? >> >> Perhaps Viresh can shine some light on the matter? > > As you can see, i wasn't the author of this patch and when you asked > this question, i didn't had an answer to it. I went through code and > formed some theory/story :) . > > @Grant: i need your help to check if my theory is correct or not. Question > is about adding below code in any i2c client driver: > > +#ifdef CONFIG_OF > +static const struct of_device_id stmpe_dt_ids[] = { > + { .compatible = "st,stmpe610", .data = &stmpe_i2c_id[0], }, > + { .compatible = "st,stmpe801", .data = &stmpe_i2c_id[1], }, > + { .compatible = "st,stmpe811", .data = &stmpe_i2c_id[2], }, > + { .compatible = "st,stmpe1601", .data = &stmpe_i2c_id[3], }, > + { .compatible = "st,stmpe2401", .data = &stmpe_i2c_id[4], }, > + { .compatible = "st,stmpe2403", .data = &stmpe_i2c_id[5], }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, stmpe_dt_ids); > +#endif > + > static struct i2c_driver stmpe_i2c_driver = { > .driver = { > .name = "stmpe-i2c", > @@ -88,6 +102,7 @@ static struct i2c_driver stmpe_i2c_driver = { > #ifdef CONFIG_PM > .pm = &stmpe_dev_pm_ops, > #endif > + .of_match_table = of_match_ptr(stmpe_dt_ids), > > So, what is the use of this table when we already have i2c_driver.id_table > populated. > > This is my theory: > - > Adapter drivers supporting DT will call: > of_i2c_register_devices() > { > for_each_child_of_node(adap->dev.of_node, node) { > if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) > error condition > > ... > result = i2c_new_device(adap, &info); > > ... > } > > of_modalias_node(): expects compatible in child node, i.e. stmpe node in our > case. If it is not there, then that node is skipped. then it copies > string after ',' > to info.type. So, for us only "stmpe810" out of "st,stmpe810" is copied. > > Now this name, i.e. "stmpe810" is copied as client.name in i2c_new_device() > and device_register() is called, which will eventually call: > > i2c_device_match() > { > /* Attempt an OF style match */ > if (of_driver_match_device(dev, drv)) > return 1; > > driver = to_i2c_driver(drv); > /* match on an id table if there is one */ > if (driver->id_table) > return i2c_match_id(driver->id_table, client) != NULL; > } > > This first tries to match the table my patch added, _BUT_ the string will > never match as we had "st,stmpe810" in table and "stmpe810" in dev. > > So, we fall back to i2c_match_id(), which will match it against > i2c_driver.id_table present in driver, which has entry for "stmpe810" and > so strings matched. > > @Lee: This is what happened in your case. :) > > So, whether its DT or non DT, true is returned from here if something > matched. > > Later on, this will be called: > > static int i2c_device_probe(struct device *dev) > { > . > status = driver->probe(client, i2c_match_id(driver->id_table, > client)); > > } > > Which will again match the legacy table to find correct struct i2c_device_id > *id > to pass to probe(). > > So, the final question: WTF is of_match_table for? > > Then i thought maybe it is used when we don't have child nodes inside parent, > something related to the phandle way ? I grep'd i2c in arch/arm/boot/dts and > couldn't find anything of that sort, the way i2c clients are added is: > > in dtsi file: > > i2c0: i2c@address { > ... > } > > in dts file: > &i2c0 { > stmpe810 { > > } > } > > which i believe will be taken care by dtc and will fold client nodes > as child nodes > of i2c0. > > @Grant: Please throw some light here :) > > -- > viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2] gpio: stmpe: Add DT support for stmpe gpio
On 1 December 2012 22:17, Linus Walleij wrote: > If you can, try to avoid this: > Content-Type: text/plain; charset=WINDOWS-1252 > Content-Transfer-Encoding: quoted-printable > > That kind om message encoding makes my life a living hell > (but I fixed it, manually editing the whole patch) how did you > end up with this? Did you for example issue the command > git send-email from a Windows machine using some Windows > version of git? I don't use windows for any linux work. I went through the header to see what went wrong and realized, i sent this mail when i was in ARM's office and via arm id only. i.e. using arm id's msmtprc settings. I don't know why my linaro settings don't work from office, they work from outside with no changes. companies use microsoft exchange server and that played with this patch. Sorry for making your life miserable :( -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V5 2/2] mfd: stmpe: Update DT support in stmpe driver
On 30 November 2012 21:15, Lee Jones wrote: > But ... I don't see how the changes in the -i2c and -spi files > are of benefit either. When I boot without the ID table I still > get "stmpe-i2c 0-0040: stmpe1601 detected, chip id: 0x212". > > What is it that actually uses the IDs? > > Perhaps Viresh can shine some light on the matter? As you can see, i wasn't the author of this patch and when you asked this question, i didn't had an answer to it. I went through code and formed some theory/story :) . @Grant: i need your help to check if my theory is correct or not. Question is about adding below code in any i2c client driver: +#ifdef CONFIG_OF +static const struct of_device_id stmpe_dt_ids[] = { + { .compatible = "st,stmpe610", .data = &stmpe_i2c_id[0], }, + { .compatible = "st,stmpe801", .data = &stmpe_i2c_id[1], }, + { .compatible = "st,stmpe811", .data = &stmpe_i2c_id[2], }, + { .compatible = "st,stmpe1601", .data = &stmpe_i2c_id[3], }, + { .compatible = "st,stmpe2401", .data = &stmpe_i2c_id[4], }, + { .compatible = "st,stmpe2403", .data = &stmpe_i2c_id[5], }, + { } +}; +MODULE_DEVICE_TABLE(of, stmpe_dt_ids); +#endif + static struct i2c_driver stmpe_i2c_driver = { .driver = { .name = "stmpe-i2c", @@ -88,6 +102,7 @@ static struct i2c_driver stmpe_i2c_driver = { #ifdef CONFIG_PM .pm = &stmpe_dev_pm_ops, #endif + .of_match_table = of_match_ptr(stmpe_dt_ids), So, what is the use of this table when we already have i2c_driver.id_table populated. This is my theory: - Adapter drivers supporting DT will call: of_i2c_register_devices() { for_each_child_of_node(adap->dev.of_node, node) { if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) error condition ... result = i2c_new_device(adap, &info); ... } of_modalias_node(): expects compatible in child node, i.e. stmpe node in our case. If it is not there, then that node is skipped. then it copies string after ',' to info.type. So, for us only "stmpe810" out of "st,stmpe810" is copied. Now this name, i.e. "stmpe810" is copied as client.name in i2c_new_device() and device_register() is called, which will eventually call: i2c_device_match() { /* Attempt an OF style match */ if (of_driver_match_device(dev, drv)) return 1; driver = to_i2c_driver(drv); /* match on an id table if there is one */ if (driver->id_table) return i2c_match_id(driver->id_table, client) != NULL; } This first tries to match the table my patch added, _BUT_ the string will never match as we had "st,stmpe810" in table and "stmpe810" in dev. So, we fall back to i2c_match_id(), which will match it against i2c_driver.id_table present in driver, which has entry for "stmpe810" and so strings matched. @Lee: This is what happened in your case. :) So, whether its DT or non DT, true is returned from here if something matched. Later on, this will be called: static int i2c_device_probe(struct device *dev) { . status = driver->probe(client, i2c_match_id(driver->id_table, client)); } Which will again match the legacy table to find correct struct i2c_device_id *id to pass to probe(). So, the final question: WTF is of_match_table for? Then i thought maybe it is used when we don't have child nodes inside parent, something related to the phandle way ? I grep'd i2c in arch/arm/boot/dts and couldn't find anything of that sort, the way i2c clients are added is: in dtsi file: i2c0: i2c@address { ... } in dts file: &i2c0 { stmpe810 { } } which i believe will be taken care by dtc and will fold client nodes as child nodes of i2c0. @Grant: Please throw some light here :) -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V5 2/2] mfd: stmpe: Update DT support in stmpe driver
On Nov 30, 2012 6:50 PM, "Lee Jones" wrote: > > On Fri, 30 Nov 2012, Viresh Kumar wrote: > > > On 30 November 2012 18:15, Lee Jones wrote: > > > The patch doesn't apply for me - does it for you? > > > > > > Viresh, what's it based on? > > > > Because this was applied 2 days back by Samuel, and i didn't > > fetch it again yesterday: > > > > commit 20d5c7defc228cdaeff3ce3442f3a4e86af293c1 > > Author: Randy Dunlap > > Date: Mon Nov 12 09:20:49 2012 -0800 > > > > Its a simple conflict to fix, as this is the only place of conflict. > > Eh? You don't know what my tree is based on. ;) > > What does this patch apply on top of? Which -rc? Mfd/for-next > -- > Lee Jones > Linaro ST-Ericsson Landing Team Lead > Linaro.org ā Open source software for ARM SoCs > Follow Linaro: Facebook | Twitter | Blog ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V5 2/2] mfd: stmpe: Update DT support in stmpe driver
On 30 November 2012 18:15, Lee Jones wrote: > The patch doesn't apply for me - does it for you? > > Viresh, what's it based on? Because this was applied 2 days back by Samuel, and i didn't fetch it again yesterday: commit 20d5c7defc228cdaeff3ce3442f3a4e86af293c1 Author: Randy Dunlap Date: Mon Nov 12 09:20:49 2012 -0800 mfd: Fix stmpe.c build when OF is not enabled Fix build errors when CONFIG_OF is not enabled by including (needs to be added in any case). An alternative fix could be to make the driver depend on OF. drivers/mfd/stmpe.c:1025:2: error: implicit declaration of function 'of_property_read_u32' drivers/mfd/stmpe.c:1030:2: error: implicit declaration of function 'for_each_child_of_node' drivers/mfd/stmpe.c:1030:36: error: expected ';' before '{' token Signed-off-by: Randy Dunlap Acked-by: Linus Walleij Signed-off-by: Samuel Ortiz --- drivers/mfd/stmpe.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c index 0061d1b..f9f7de7 100644 --- a/drivers/mfd/stmpe.c +++ b/drivers/mfd/stmpe.c @@ -13,6 +13,7 @@ #include #include #include +#include Its a simple conflict to fix, as this is the only place of conflict. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH V5 2/2] mfd: stmpe: Update DT support in stmpe driver
From: Vipul Kumar Samar This patch extends existing DT support for stmpe devices. This updates: - DT support from stmpe SPI and I2C drivers - missing header files in stmpe.c - stmpe_of_probe() with pwm, rotator and new bindings. - Bindings are updated in binding document. Signed-off-by: Vipul Kumar Samar Signed-off-by: Viresh Kumar --- V4->V5: -- - 2/3 and 3/3 merged. - irq_trigger is kept same for non-DT booti. @Lee: I haven't added your Acked-by, because this differs from your Acked version. Documentation/devicetree/bindings/mfd/stmpe.txt | 9 ++--- drivers/mfd/stmpe-i2c.c | 15 ++ drivers/mfd/stmpe-spi.c | 15 ++ drivers/mfd/stmpe.c | 27 +++-- 4 files changed, 57 insertions(+), 9 deletions(-) diff --git a/Documentation/devicetree/bindings/mfd/stmpe.txt b/Documentation/devicetree/bindings/mfd/stmpe.txt index 8f0aeda..d2d88d9 100644 --- a/Documentation/devicetree/bindings/mfd/stmpe.txt +++ b/Documentation/devicetree/bindings/mfd/stmpe.txt @@ -1,8 +1,11 @@ -* STMPE Multi-Functional Device +* ST Microelectronics STMPE Multi-Functional Device + +STMPE is an MFD device which may expose following inbuilt devices: gpio, keypad, +touchscreen, adc, pwm, rotator. Required properties: - - compatible : "st,stmpe[811|1601|2401|2403]" - - reg : I2C address of the device + - compatible : "st,stmpe[610|801|811|1601|2401|2403]" + - reg : I2C/SPI address of the device Optional properties: - interrupts : The interrupt outputs from the controller diff --git a/drivers/mfd/stmpe-i2c.c b/drivers/mfd/stmpe-i2c.c index c734dc3..003c49b 100644 --- a/drivers/mfd/stmpe-i2c.c +++ b/drivers/mfd/stmpe-i2c.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include "stmpe.h" @@ -81,6 +82,19 @@ static const struct i2c_device_id stmpe_i2c_id[] = { }; MODULE_DEVICE_TABLE(i2c, stmpe_id); +#ifdef CONFIG_OF +static const struct of_device_id stmpe_dt_ids[] = { + { .compatible = "st,stmpe610", .data = &stmpe_i2c_id[0], }, + { .compatible = "st,stmpe801", .data = &stmpe_i2c_id[1], }, + { .compatible = "st,stmpe811", .data = &stmpe_i2c_id[2], }, + { .compatible = "st,stmpe1601", .data = &stmpe_i2c_id[3], }, + { .compatible = "st,stmpe2401", .data = &stmpe_i2c_id[4], }, + { .compatible = "st,stmpe2403", .data = &stmpe_i2c_id[5], }, + { } +}; +MODULE_DEVICE_TABLE(of, stmpe_dt_ids); +#endif + static struct i2c_driver stmpe_i2c_driver = { .driver = { .name = "stmpe-i2c", @@ -88,6 +102,7 @@ static struct i2c_driver stmpe_i2c_driver = { #ifdef CONFIG_PM .pm = &stmpe_dev_pm_ops, #endif + .of_match_table = of_match_ptr(stmpe_dt_ids), }, .probe = stmpe_i2c_probe, .remove = __devexit_p(stmpe_i2c_remove), diff --git a/drivers/mfd/stmpe-spi.c b/drivers/mfd/stmpe-spi.c index 9edfe86..1e2bff0 100644 --- a/drivers/mfd/stmpe-spi.c +++ b/drivers/mfd/stmpe-spi.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include "stmpe.h" @@ -119,6 +120,19 @@ static const struct spi_device_id stmpe_spi_id[] = { }; MODULE_DEVICE_TABLE(spi, stmpe_id); +#ifdef CONFIG_OF +static const struct of_device_id stmpe_dt_ids[] = { + { .compatible = "st,stmpe610", .data = (void *)STMPE610, }, + { .compatible = "st,stmpe801", .data = (void *)STMPE801, }, + { .compatible = "st,stmpe811", .data = (void *)STMPE811, }, + { .compatible = "st,stmpe1601", .data = (void *)STMPE1601, }, + { .compatible = "st,stmpe2401", .data = (void *)STMPE2401, }, + { .compatible = "st,stmpe2403", .data = (void *)STMPE2403, }, + { } +}; +MODULE_DEVICE_TABLE(of, stmpe_dt_ids); +#endif + static struct spi_driver stmpe_spi_driver = { .driver = { .name = "stmpe-spi", @@ -126,6 +140,7 @@ static struct spi_driver stmpe_spi_driver = { #ifdef CONFIG_PM .pm = &stmpe_dev_pm_ops, #endif + .of_match_table = of_match_ptr(stmpe_dt_ids), }, .probe = stmpe_spi_probe, .remove = __devexit_p(stmpe_spi_remove), diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c index 34408b4..bb2dfdb 100644 --- a/drivers/mfd/stmpe.c +++ b/drivers/mfd/stmpe.c @@ -7,12 +7,15 @@ * Author: Rabin Vincent for ST-Ericsson */ +#include #include #include #include #include #include #include +#include +#include #include #include #include @@ -1019,6 +1022,14 @@ void __devinit stmpe_of_probe(struct stmpe_plat
[PATCH V5 1/2] mfd: stmpe: Get rid of irq_invert_polarity
Since the very first patch, stmpe core driver is using irq_invert_polarity as part of platform data. But, nobody is actually using it in kernel till now. Also, this is not something part of hardware specs, but is included to cater some board mistakes or quirks. So, better get rid of it. This is earlier discussed here: https://lkml.org/lkml/2012/11/27/636 Acked-by: Lee Jones Signed-off-by: Viresh Kumar --- Nothing changed from last version. drivers/mfd/stmpe.c | 7 --- include/linux/mfd/stmpe.h | 2 -- 2 files changed, 9 deletions(-) diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c index 6175aa4..34408b4 100644 --- a/drivers/mfd/stmpe.c +++ b/drivers/mfd/stmpe.c @@ -960,13 +960,6 @@ static int __devinit stmpe_chip_init(struct stmpe *stmpe) else icr |= STMPE_ICR_LSB_HIGH; } - - if (stmpe->pdata->irq_invert_polarity) { - if (id == STMPE801_ID) - icr ^= STMPE801_REG_SYS_CTRL_INT_HI; - else - icr ^= STMPE_ICR_LSB_HIGH; - } } if (stmpe->pdata->autosleep) { diff --git a/include/linux/mfd/stmpe.h b/include/linux/mfd/stmpe.h index 15dac79..383ac15 100644 --- a/include/linux/mfd/stmpe.h +++ b/include/linux/mfd/stmpe.h @@ -190,7 +190,6 @@ struct stmpe_ts_platform_data { * @id: device id to distinguish between multiple STMPEs on the same board * @blocks: bitmask of blocks to enable (use STMPE_BLOCK_*) * @irq_trigger: IRQ trigger to use for the interrupt to the host - * @irq_invert_polarity: IRQ line is connected with reversed polarity * @autosleep: bool to enable/disable stmpe autosleep * @autosleep_timeout: inactivity timeout in milliseconds for autosleep * @irq_base: base IRQ number. %STMPE_NR_IRQS irqs will be used, or @@ -207,7 +206,6 @@ struct stmpe_platform_data { unsigned int blocks; int irq_base; unsigned int irq_trigger; - bool irq_invert_polarity; bool autosleep; bool irq_over_gpio; int irq_gpio; -- 1.7.12.rc2.18.g61b472e ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V4 Resend 2/3] mfd: stmpe: Remove irq_trigger from platform data
On 29 November 2012 15:04, Lee Jones wrote: > On Thu, 29 Nov 2012, Viresh Kumar wrote: > >> STMPE can confige > > configure? > >> the way the device emits interrupts and till now this > > until? Ahh... Will fix them. This happens when you send patches at midnight. :) >> diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c >> ret = devm_request_threaded_irq(ci->dev, stmpe->irq, NULL, >> - stmpe_irq, pdata->irq_trigger | IRQF_ONESHOT, >> - "stmpe", stmpe); >> + stmpe_irq, IRQF_ONESHOT, "stmpe", stmpe); > > Forgive my ignorance, but you're no longer passing irq_trigger. > > Is this intentional? If so, why was it needed before and not now? Yes, it was intentional. I thought it wasn't required at all. But my mind is changing a bit now. I feel it is not required for DT, as trigger prop is already passed in the interrupts cell. But for non-DT user, this is still required. As there is not other way by which IRQ controller will come to know what irq trigger type to enable for this irq line. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V4 Resend 3/3] mfd: stmpe: Update DT support in stmpe driver
On 29 November 2012 14:53, Lee Jones wrote: >> From: Vipul Kumar Samar >> + /* >> + * Distinct names of same cell-type within multiple instances of stmpe >> + * will be guaranteed by DT. >> + */ >> + pdata->id = -1; > > And what if we're not booting with DT? Then it is responsibility of code creating platform data for stmpe to fill it appropriately. i.e. If there is only one instance of stmpe on board, then better pass it as -1, else start counting. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH V4 Resend 3/3] mfd: stmpe: Update DT support in stmpe driver
From: Vipul Kumar Samar This patch extends existing DT support for stmpe devices. This updates: - DT support from stmpe SPI and I2C drivers - missing header files in stmpe.c - stmpe_of_probe() with pwm, rotator and new bindings. - Bindings are updated in binding document. Signed-off-by: Vipul Kumar Samar Signed-off-by: Viresh Kumar --- V3->V4: - Remove need to pass irq type and polarity from DT. - Remove need to pass id from platform data. Documentation/devicetree/bindings/mfd/stmpe.txt | 9 ++--- drivers/mfd/stmpe-i2c.c | 15 +++ drivers/mfd/stmpe-spi.c | 15 +++ drivers/mfd/stmpe.c | 22 -- 4 files changed, 52 insertions(+), 9 deletions(-) diff --git a/Documentation/devicetree/bindings/mfd/stmpe.txt b/Documentation/devicetree/bindings/mfd/stmpe.txt index 8f0aeda..d2d88d9 100644 --- a/Documentation/devicetree/bindings/mfd/stmpe.txt +++ b/Documentation/devicetree/bindings/mfd/stmpe.txt @@ -1,8 +1,11 @@ -* STMPE Multi-Functional Device +* ST Microelectronics STMPE Multi-Functional Device + +STMPE is an MFD device which may expose following inbuilt devices: gpio, keypad, +touchscreen, adc, pwm, rotator. Required properties: - - compatible : "st,stmpe[811|1601|2401|2403]" - - reg : I2C address of the device + - compatible : "st,stmpe[610|801|811|1601|2401|2403]" + - reg : I2C/SPI address of the device Optional properties: - interrupts : The interrupt outputs from the controller diff --git a/drivers/mfd/stmpe-i2c.c b/drivers/mfd/stmpe-i2c.c index c734dc3..003c49b 100644 --- a/drivers/mfd/stmpe-i2c.c +++ b/drivers/mfd/stmpe-i2c.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include "stmpe.h" @@ -81,6 +82,19 @@ static const struct i2c_device_id stmpe_i2c_id[] = { }; MODULE_DEVICE_TABLE(i2c, stmpe_id); +#ifdef CONFIG_OF +static const struct of_device_id stmpe_dt_ids[] = { + { .compatible = "st,stmpe610", .data = &stmpe_i2c_id[0], }, + { .compatible = "st,stmpe801", .data = &stmpe_i2c_id[1], }, + { .compatible = "st,stmpe811", .data = &stmpe_i2c_id[2], }, + { .compatible = "st,stmpe1601", .data = &stmpe_i2c_id[3], }, + { .compatible = "st,stmpe2401", .data = &stmpe_i2c_id[4], }, + { .compatible = "st,stmpe2403", .data = &stmpe_i2c_id[5], }, + { } +}; +MODULE_DEVICE_TABLE(of, stmpe_dt_ids); +#endif + static struct i2c_driver stmpe_i2c_driver = { .driver = { .name = "stmpe-i2c", @@ -88,6 +102,7 @@ static struct i2c_driver stmpe_i2c_driver = { #ifdef CONFIG_PM .pm = &stmpe_dev_pm_ops, #endif + .of_match_table = of_match_ptr(stmpe_dt_ids), }, .probe = stmpe_i2c_probe, .remove = __devexit_p(stmpe_i2c_remove), diff --git a/drivers/mfd/stmpe-spi.c b/drivers/mfd/stmpe-spi.c index 9edfe86..1e2bff0 100644 --- a/drivers/mfd/stmpe-spi.c +++ b/drivers/mfd/stmpe-spi.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include "stmpe.h" @@ -119,6 +120,19 @@ static const struct spi_device_id stmpe_spi_id[] = { }; MODULE_DEVICE_TABLE(spi, stmpe_id); +#ifdef CONFIG_OF +static const struct of_device_id stmpe_dt_ids[] = { + { .compatible = "st,stmpe610", .data = (void *)STMPE610, }, + { .compatible = "st,stmpe801", .data = (void *)STMPE801, }, + { .compatible = "st,stmpe811", .data = (void *)STMPE811, }, + { .compatible = "st,stmpe1601", .data = (void *)STMPE1601, }, + { .compatible = "st,stmpe2401", .data = (void *)STMPE2401, }, + { .compatible = "st,stmpe2403", .data = (void *)STMPE2403, }, + { } +}; +MODULE_DEVICE_TABLE(of, stmpe_dt_ids); +#endif + static struct spi_driver stmpe_spi_driver = { .driver = { .name = "stmpe-spi", @@ -126,6 +140,7 @@ static struct spi_driver stmpe_spi_driver = { #ifdef CONFIG_PM .pm = &stmpe_dev_pm_ops, #endif + .of_match_table = of_match_ptr(stmpe_dt_ids), }, .probe = stmpe_spi_probe, .remove = __devexit_p(stmpe_spi_remove), diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c index 10819e6..88cd8e5 100644 --- a/drivers/mfd/stmpe.c +++ b/drivers/mfd/stmpe.c @@ -7,12 +7,15 @@ * Author: Rabin Vincent for ST-Ericsson */ +#include #include #include #include #include #include #include +#include +#include #include #include #include @@ -1021,6 +1024,12 @@ void __devinit stmpe_of_probe(struct stmpe_platform_data *pdata, { struct device_node *child; +
[PATCH V4 Resend 2/3] mfd: stmpe: Remove irq_trigger from platform data
STMPE can confige the way the device emits interrupts and till now this information is passed as part of platform data. It would actually be good to ask the interrupt controller driver what kind of interrupt signal it expects for a given interrupt line. We can get the irq type by calling: irqd_get_trigger_type() routine. So, now we don't need to pass it via platform data. This is earlier discussed here: https://lkml.org/lkml/2012/11/26/711 Signed-off-by: Viresh Kumar --- @Linus/Shiraz: Can you please test this patch? You don't really need to test 1/3 and 3/3, but would be good if you do that too.. This is actually V1 of this patch. arch/arm/mach-ux500/board-mop500-stuib.c | 1 - drivers/mfd/stmpe.c | 7 --- include/linux/mfd/stmpe.h| 2 -- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/arch/arm/mach-ux500/board-mop500-stuib.c b/arch/arm/mach-ux500/board-mop500-stuib.c index 564f57d..0efcf97 100644 --- a/arch/arm/mach-ux500/board-mop500-stuib.c +++ b/arch/arm/mach-ux500/board-mop500-stuib.c @@ -57,7 +57,6 @@ static struct stmpe_keypad_platform_data stmpe1601_keypad_data = { static struct stmpe_platform_data stmpe1601_data = { .id = 1, .blocks = STMPE_BLOCK_KEYPAD, - .irq_trigger= IRQF_TRIGGER_FALLING, .irq_base = MOP500_STMPE1601_IRQ(0), .keypad = &stmpe1601_keypad_data, .autosleep = true, diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c index 34408b4..10819e6 100644 --- a/drivers/mfd/stmpe.c +++ b/drivers/mfd/stmpe.c @@ -914,7 +914,6 @@ static int __devinit stmpe_irq_init(struct stmpe *stmpe, static int __devinit stmpe_chip_init(struct stmpe *stmpe) { - unsigned int irq_trigger = stmpe->pdata->irq_trigger; int autosleep_timeout = stmpe->pdata->autosleep_timeout; struct stmpe_variant_info *variant = stmpe->variant; u8 icr = 0; @@ -941,6 +940,9 @@ static int __devinit stmpe_chip_init(struct stmpe *stmpe) return ret; if (stmpe->irq >= 0) { + unsigned int irq_trigger = + irqd_get_trigger_type(irq_get_irq_data(stmpe->irq)); + if (id == STMPE801_ID) icr = STMPE801_REG_SYS_CTRL_INT_EN; else @@ -1118,8 +1120,7 @@ int __devinit stmpe_probe(struct stmpe_client_info *ci, int partnum) return ret; ret = devm_request_threaded_irq(ci->dev, stmpe->irq, NULL, - stmpe_irq, pdata->irq_trigger | IRQF_ONESHOT, - "stmpe", stmpe); + stmpe_irq, IRQF_ONESHOT, "stmpe", stmpe); if (ret) { dev_err(stmpe->dev, "failed to request IRQ: %d\n", ret); diff --git a/include/linux/mfd/stmpe.h b/include/linux/mfd/stmpe.h index 383ac15..2519326 100644 --- a/include/linux/mfd/stmpe.h +++ b/include/linux/mfd/stmpe.h @@ -189,7 +189,6 @@ struct stmpe_ts_platform_data { * struct stmpe_platform_data - STMPE platform data * @id: device id to distinguish between multiple STMPEs on the same board * @blocks: bitmask of blocks to enable (use STMPE_BLOCK_*) - * @irq_trigger: IRQ trigger to use for the interrupt to the host * @autosleep: bool to enable/disable stmpe autosleep * @autosleep_timeout: inactivity timeout in milliseconds for autosleep * @irq_base: base IRQ number. %STMPE_NR_IRQS irqs will be used, or @@ -205,7 +204,6 @@ struct stmpe_platform_data { int id; unsigned int blocks; int irq_base; - unsigned int irq_trigger; bool autosleep; bool irq_over_gpio; int irq_gpio; -- 1.7.12.rc2.18.g61b472e ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH V4 Resend 1/3] mfd: stmpe: Get rid of irq_invert_polarity
Since the very first patch, stmpe core driver is using irq_invert_polarity as part of platform data. But, nobody is actually using it in kernel till now. Also, this is not something part of hardware specs, but is included to cater some board mistakes or quirks. So, better get rid of it. This is earlier discussed here: https://lkml.org/lkml/2012/11/27/636 Signed-off-by: Viresh Kumar --- This is actually V1 of this patch. drivers/mfd/stmpe.c | 7 --- include/linux/mfd/stmpe.h | 2 -- 2 files changed, 9 deletions(-) diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c index 6175aa4..34408b4 100644 --- a/drivers/mfd/stmpe.c +++ b/drivers/mfd/stmpe.c @@ -960,13 +960,6 @@ static int __devinit stmpe_chip_init(struct stmpe *stmpe) else icr |= STMPE_ICR_LSB_HIGH; } - - if (stmpe->pdata->irq_invert_polarity) { - if (id == STMPE801_ID) - icr ^= STMPE801_REG_SYS_CTRL_INT_HI; - else - icr ^= STMPE_ICR_LSB_HIGH; - } } if (stmpe->pdata->autosleep) { diff --git a/include/linux/mfd/stmpe.h b/include/linux/mfd/stmpe.h index 15dac79..383ac15 100644 --- a/include/linux/mfd/stmpe.h +++ b/include/linux/mfd/stmpe.h @@ -190,7 +190,6 @@ struct stmpe_ts_platform_data { * @id: device id to distinguish between multiple STMPEs on the same board * @blocks: bitmask of blocks to enable (use STMPE_BLOCK_*) * @irq_trigger: IRQ trigger to use for the interrupt to the host - * @irq_invert_polarity: IRQ line is connected with reversed polarity * @autosleep: bool to enable/disable stmpe autosleep * @autosleep_timeout: inactivity timeout in milliseconds for autosleep * @irq_base: base IRQ number. %STMPE_NR_IRQS irqs will be used, or @@ -207,7 +206,6 @@ struct stmpe_platform_data { unsigned int blocks; int irq_base; unsigned int irq_trigger; - bool irq_invert_polarity; bool autosleep; bool irq_over_gpio; int irq_gpio; -- 1.7.12.rc2.18.g61b472e ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V4 1/3] mfd: stmpe: Get rid of irq_invert_polarity
On 29 November 2012 00:05, Viresh Kumar wrote: > Since the very first patch, stmpe core driver is using irq_invert_polarity as > part of platform data. But, nobody is actually using it in kernel till now. > > Also, this is not something part of hardware specs, but is included to cater > some board mistakes or quirks. > > So, better get rid of it. This is earlier discussed here: > > https://lkml.org/lkml/2012/11/27/636 > > Signed-off-by: Viresh Kumar Please ignore this series.. Two issues: - rebased over an un accepted patch which rearranges header file - I can improve patch 2/3 a bit. I will resend it shortlly. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH V4 3/3] mfd: stmpe: Update DT support in stmpe driver
From: Vipul Kumar Samar This patch extends existing DT support for stmpe devices. This updates: - DT support from stmpe SPI and I2C drivers - missing header files in stmpe.c - stmpe_of_probe() with pwm, rotator and new bindings. - Bindings are updated in binding document. Signed-off-by: Vipul Kumar Samar Signed-off-by: Viresh Kumar --- V3->V4: - Remove need to pass irq type and polarity from DT. - Remove need to pass id from platform data. Documentation/devicetree/bindings/mfd/stmpe.txt | 9 ++--- drivers/mfd/stmpe-i2c.c | 15 +++ drivers/mfd/stmpe-spi.c | 15 +++ drivers/mfd/stmpe.c | 22 -- 4 files changed, 52 insertions(+), 9 deletions(-) diff --git a/Documentation/devicetree/bindings/mfd/stmpe.txt b/Documentation/devicetree/bindings/mfd/stmpe.txt index 8f0aeda..d2d88d9 100644 --- a/Documentation/devicetree/bindings/mfd/stmpe.txt +++ b/Documentation/devicetree/bindings/mfd/stmpe.txt @@ -1,8 +1,11 @@ -* STMPE Multi-Functional Device +* ST Microelectronics STMPE Multi-Functional Device + +STMPE is an MFD device which may expose following inbuilt devices: gpio, keypad, +touchscreen, adc, pwm, rotator. Required properties: - - compatible : "st,stmpe[811|1601|2401|2403]" - - reg : I2C address of the device + - compatible : "st,stmpe[610|801|811|1601|2401|2403]" + - reg : I2C/SPI address of the device Optional properties: - interrupts : The interrupt outputs from the controller diff --git a/drivers/mfd/stmpe-i2c.c b/drivers/mfd/stmpe-i2c.c index c734dc3..003c49b 100644 --- a/drivers/mfd/stmpe-i2c.c +++ b/drivers/mfd/stmpe-i2c.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include "stmpe.h" @@ -81,6 +82,19 @@ static const struct i2c_device_id stmpe_i2c_id[] = { }; MODULE_DEVICE_TABLE(i2c, stmpe_id); +#ifdef CONFIG_OF +static const struct of_device_id stmpe_dt_ids[] = { + { .compatible = "st,stmpe610", .data = &stmpe_i2c_id[0], }, + { .compatible = "st,stmpe801", .data = &stmpe_i2c_id[1], }, + { .compatible = "st,stmpe811", .data = &stmpe_i2c_id[2], }, + { .compatible = "st,stmpe1601", .data = &stmpe_i2c_id[3], }, + { .compatible = "st,stmpe2401", .data = &stmpe_i2c_id[4], }, + { .compatible = "st,stmpe2403", .data = &stmpe_i2c_id[5], }, + { } +}; +MODULE_DEVICE_TABLE(of, stmpe_dt_ids); +#endif + static struct i2c_driver stmpe_i2c_driver = { .driver = { .name = "stmpe-i2c", @@ -88,6 +102,7 @@ static struct i2c_driver stmpe_i2c_driver = { #ifdef CONFIG_PM .pm = &stmpe_dev_pm_ops, #endif + .of_match_table = of_match_ptr(stmpe_dt_ids), }, .probe = stmpe_i2c_probe, .remove = __devexit_p(stmpe_i2c_remove), diff --git a/drivers/mfd/stmpe-spi.c b/drivers/mfd/stmpe-spi.c index f86e3fc..e482f5f 100644 --- a/drivers/mfd/stmpe-spi.c +++ b/drivers/mfd/stmpe-spi.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include "stmpe.h" @@ -119,6 +120,19 @@ static const struct spi_device_id stmpe_spi_id[] = { }; MODULE_DEVICE_TABLE(spi, stmpe_id); +#ifdef CONFIG_OF +static const struct of_device_id stmpe_dt_ids[] = { + { .compatible = "st,stmpe610", .data = (void *)STMPE610, }, + { .compatible = "st,stmpe801", .data = (void *)STMPE801, }, + { .compatible = "st,stmpe811", .data = (void *)STMPE811, }, + { .compatible = "st,stmpe1601", .data = (void *)STMPE1601, }, + { .compatible = "st,stmpe2401", .data = (void *)STMPE2401, }, + { .compatible = "st,stmpe2403", .data = (void *)STMPE2403, }, + { } +}; +MODULE_DEVICE_TABLE(of, stmpe_dt_ids); +#endif + static struct spi_driver stmpe_spi_driver = { .driver = { .name = "stmpe-spi", @@ -126,6 +140,7 @@ static struct spi_driver stmpe_spi_driver = { #ifdef CONFIG_PM .pm = &stmpe_dev_pm_ops, #endif + .of_match_table = of_match_ptr(stmpe_dt_ids), }, .probe = stmpe_spi_probe, .remove = __devexit_p(stmpe_spi_remove), diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c index fa22ef4..d3771ba 100644 --- a/drivers/mfd/stmpe.c +++ b/drivers/mfd/stmpe.c @@ -7,6 +7,7 @@ * Author: Rabin Vincent for ST-Ericsson */ +#include #include #include #include @@ -14,6 +15,8 @@ #include #include #include +#include +#include #include #include #include "stmpe.h" @@ -1019,6 +1022,12 @@ void __devinit stmpe_of_probe(struct stmpe_platform_data *pdata, { struc
[PATCH V4 2/3] mfd: stmpe: Remove irq_trigger from platform data
STMPE can confige the way the device emits interrupts and till now this information is passed as part of platform data. It would actually be good to ask the interrupt controller driver what kind of interrupt signal it expects for a given interrupt line. We can get the irq type by calling: irqd_get_trigger_type() routine. So, now we don't need to pass it via platform data. This is earlier discussed here: https://lkml.org/lkml/2012/11/26/711 Signed-off-by: Viresh Kumar --- Linus/Shiraz: Can you please test this patch? You don't really need to test 1/3 and 3/3, but would be good if you do that too.. This is actually V1 of this patch. arch/arm/mach-ux500/board-mop500-stuib.c | 1 - drivers/mfd/stmpe.c | 8 +--- include/linux/mfd/stmpe.h| 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/arch/arm/mach-ux500/board-mop500-stuib.c b/arch/arm/mach-ux500/board-mop500-stuib.c index 564f57d..0efcf97 100644 --- a/arch/arm/mach-ux500/board-mop500-stuib.c +++ b/arch/arm/mach-ux500/board-mop500-stuib.c @@ -57,7 +57,6 @@ static struct stmpe_keypad_platform_data stmpe1601_keypad_data = { static struct stmpe_platform_data stmpe1601_data = { .id = 1, .blocks = STMPE_BLOCK_KEYPAD, - .irq_trigger= IRQF_TRIGGER_FALLING, .irq_base = MOP500_STMPE1601_IRQ(0), .keypad = &stmpe1601_keypad_data, .autosleep = true, diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c index fece28b..fa22ef4 100644 --- a/drivers/mfd/stmpe.c +++ b/drivers/mfd/stmpe.c @@ -914,7 +914,7 @@ static int __devinit stmpe_irq_init(struct stmpe *stmpe, static int __devinit stmpe_chip_init(struct stmpe *stmpe) { - unsigned int irq_trigger = stmpe->pdata->irq_trigger; + unsigned int irq_trigger = stmpe->irq_trigger; int autosleep_timeout = stmpe->pdata->autosleep_timeout; struct stmpe_variant_info *variant = stmpe->variant; u8 icr = 0; @@ -1106,6 +1106,9 @@ int __devinit stmpe_probe(struct stmpe_client_info *ci, int partnum) return -ENODEV; } stmpe->variant = stmpe_noirq_variant_info[stmpe->partnum]; + } else { + stmpe->irq_trigger = + irqd_get_trigger_type(irq_get_irq_data(stmpe->irq)); } ret = stmpe_chip_init(stmpe); @@ -1118,8 +1121,7 @@ int __devinit stmpe_probe(struct stmpe_client_info *ci, int partnum) return ret; ret = devm_request_threaded_irq(ci->dev, stmpe->irq, NULL, - stmpe_irq, pdata->irq_trigger | IRQF_ONESHOT, - "stmpe", stmpe); + stmpe_irq, IRQF_ONESHOT, "stmpe", stmpe); if (ret) { dev_err(stmpe->dev, "failed to request IRQ: %d\n", ret); diff --git a/include/linux/mfd/stmpe.h b/include/linux/mfd/stmpe.h index 383ac15..0f2ea36 100644 --- a/include/linux/mfd/stmpe.h +++ b/include/linux/mfd/stmpe.h @@ -71,6 +71,7 @@ struct stmpe_client_info; * different variants. Indexed by one of STMPE_IDX_*. * @irq: irq number for stmpe * @irq_base: starting IRQ number for internal IRQs + * @irq_trigger: IRQ trigger to use for the interrupt to the host * @num_gpios: number of gpios, differs for variants * @ier: cache of IER registers for bus_lock * @oldier: cache of IER registers for bus_lock @@ -89,6 +90,7 @@ struct stmpe { int irq; int irq_base; + unsigned int irq_trigger; int num_gpios; u8 ier[2]; u8 oldier[2]; @@ -189,7 +191,6 @@ struct stmpe_ts_platform_data { * struct stmpe_platform_data - STMPE platform data * @id: device id to distinguish between multiple STMPEs on the same board * @blocks: bitmask of blocks to enable (use STMPE_BLOCK_*) - * @irq_trigger: IRQ trigger to use for the interrupt to the host * @autosleep: bool to enable/disable stmpe autosleep * @autosleep_timeout: inactivity timeout in milliseconds for autosleep * @irq_base: base IRQ number. %STMPE_NR_IRQS irqs will be used, or @@ -205,7 +206,6 @@ struct stmpe_platform_data { int id; unsigned int blocks; int irq_base; - unsigned int irq_trigger; bool autosleep; bool irq_over_gpio; int irq_gpio; -- 1.7.12.rc2.18.g61b472e ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH V4 1/3] mfd: stmpe: Get rid of irq_invert_polarity
Since the very first patch, stmpe core driver is using irq_invert_polarity as part of platform data. But, nobody is actually using it in kernel till now. Also, this is not something part of hardware specs, but is included to cater some board mistakes or quirks. So, better get rid of it. This is earlier discussed here: https://lkml.org/lkml/2012/11/27/636 Signed-off-by: Viresh Kumar --- This is actually V1 of this patch. drivers/mfd/stmpe.c | 7 --- include/linux/mfd/stmpe.h | 2 -- 2 files changed, 9 deletions(-) diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c index 8b164f3..fece28b 100644 --- a/drivers/mfd/stmpe.c +++ b/drivers/mfd/stmpe.c @@ -960,13 +960,6 @@ static int __devinit stmpe_chip_init(struct stmpe *stmpe) else icr |= STMPE_ICR_LSB_HIGH; } - - if (stmpe->pdata->irq_invert_polarity) { - if (id == STMPE801_ID) - icr ^= STMPE801_REG_SYS_CTRL_INT_HI; - else - icr ^= STMPE_ICR_LSB_HIGH; - } } if (stmpe->pdata->autosleep) { diff --git a/include/linux/mfd/stmpe.h b/include/linux/mfd/stmpe.h index 15dac79..383ac15 100644 --- a/include/linux/mfd/stmpe.h +++ b/include/linux/mfd/stmpe.h @@ -190,7 +190,6 @@ struct stmpe_ts_platform_data { * @id: device id to distinguish between multiple STMPEs on the same board * @blocks: bitmask of blocks to enable (use STMPE_BLOCK_*) * @irq_trigger: IRQ trigger to use for the interrupt to the host - * @irq_invert_polarity: IRQ line is connected with reversed polarity * @autosleep: bool to enable/disable stmpe autosleep * @autosleep_timeout: inactivity timeout in milliseconds for autosleep * @irq_base: base IRQ number. %STMPE_NR_IRQS irqs will be used, or @@ -207,7 +206,6 @@ struct stmpe_platform_data { unsigned int blocks; int irq_base; unsigned int irq_trigger; - bool irq_invert_polarity; bool autosleep; bool irq_over_gpio; int irq_gpio; -- 1.7.12.rc2.18.g61b472e ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V3 3/3] mfd: stmpe: Update DT support in stmpe driver
On 28 November 2012 01:25, Rabin Vincent wrote: > 2012/11/27 Viresh Kumar : >> On 27 November 2012 14:10, Lee Jones wrote: >> I haven't seen this in any of SPEAr boards i have worked on. Maybe Rabin >> would have, that's why he added that part of code :) >> >> @Rabin/Linus: Do you remember why have you added this in stmpe driver: >> >> + if (stmpe->pdata->irq_invert_polarity) >> + icr ^= STMPE_ICR_LSB_HIGH; >> + >> >> Does somebody actually need it? > > It was (as irq_rev_pol) part of Luotao Fu's proposed STMPE811 patchset > (https://patchwork.kernel.org/patch/106173/) which I integrated into my > version of the STMPE driver, which didn't have it in its initial version > (https://patchwork.kernel.org/patch/103273/). > > It's not something _I_ ever used. I grep'd kernel and nobody is using it there, so lets get rid of it completely :) I will patch it. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH] fixup! cpufreq: SPEAr: Add CPUFreq driver
Signed-off-by: Viresh Kumar --- Hi Rafael, To make review easier and faster, i am sending incremental patch for SPEAr cpufreq driver. This patch was earlier discussed here: http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/135088.html .../devicetree/bindings/cpufreq/cpufreq-spear.txt | 33 +++--- drivers/cpufreq/spear-cpufreq.c| 4 +-- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-spear.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-spear.txt index 4cf2819..f3d44984 100644 --- a/Documentation/devicetree/bindings/cpufreq/cpufreq-spear.txt +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-spear.txt @@ -6,7 +6,6 @@ It supports both uniprocessor (UP) and symmetric multiprocessor (SMP) systems which share clock across all CPUs. Required properties: -- compatible: "st,cpufreq-spear" - cpufreq_tbl: Table of frequencies CPU could be transitioned into, in the increasing order. @@ -14,16 +13,30 @@ Optional properties: - clock-latency: Specify the possible maximum transition latency for clock, in unit of nanoseconds. +Both required and optional properties listed above must be defined under node +/cpus/cpu@0. + Examples: +cpus { + + <...> + + cpu@0 { + compatible = "arm,cortex-a9"; + reg = <0>; + + <...> + + cpufreq_tbl = < 166000 + 20 + 25 + 30 + 40 + 50 + 60 >; + }; + + <...> -cpufreq { - compatible = "st,cpufreq-spear"; - cpufreq_tbl = < 166000 - 20 - 25 - 30 - 40 - 50 - 60 >; }; diff --git a/drivers/cpufreq/spear-cpufreq.c b/drivers/cpufreq/spear-cpufreq.c index a7fe880..4575cfe 100644 --- a/drivers/cpufreq/spear-cpufreq.c +++ b/drivers/cpufreq/spear-cpufreq.c @@ -224,9 +224,9 @@ static int spear_cpufreq_driver_init(void) const __be32 *val; int cnt, i, ret; - np = of_find_compatible_node(NULL, NULL, "st,cpufreq-spear"); + np = of_find_node_by_path("/cpus/cpu@0"); if (!np) { - pr_err("No cpufreq node found"); + pr_err("No cpu node found"); return -ENODEV; } -- 1.7.12.rc2.18.g61b472e ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] cpufreq: SPEAr: Add CPUFreq driver
On 27 November 2012 17:08, Rafael J. Wysocki wrote: > On Tuesday, November 27, 2012 09:28:01 AM Viresh Kumar wrote: >> On 27 November 2012 09:31, Shawn Guo wrote: >> > On Sun, Nov 25, 2012 at 01:09:28AM +0530, Viresh Kumar wrote: >> >> +cpufreq { >> >> + compatible = "st,cpufreq-spear"; >> > >> > I do not think we need a "cpufreq" node, as we already have node for >> > cpu to contain these. >> >> Hmm.. Yes, can be done. > > OK, so changes are going to be made to the driver, right? > > I'll drop it then and please resubmit when ready (and tested) after the > changes. Yes, for now this would be the only change i have. I can resend it right now, if you can still pick it for 3.8. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] cpufreq: SPEAr: Add CPUFreq driver
On 27 November 2012 14:54, Sudeep K N wrote: > On Sat, Nov 24, 2012 at 7:39 PM, Viresh Kumar wrote: > [...] >> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-spear.txt >> b/Documentation/devicetree/bindings/cpufreq/cpufreq-spear.txt >> new file mode 100644 >> index 000..4cf2819 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-spear.txt >> @@ -0,0 +1,29 @@ >> +SPEAr cpufreq driver >> +--- >> + >> +SPEAr SoC cpufreq driver for CPU frequency scaling. >> +It supports both uniprocessor (UP) and symmetric multiprocessor (SMP) >> systems >> +which share clock across all CPUs. > > If this is the case, is there any particular reason why you cannot use > the generic > cpufreq-cpu0 driver ? Please go through the discussion already done with Shawn. There is lot of SPEAr specific code here. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V3 3/3] mfd: stmpe: Update DT support in stmpe driver
On 27 November 2012 14:10, Lee Jones wrote: > On Tue, 27 Nov 2012, Viresh Kumar wrote: >> Problem is with invert polarity, which the interrupt controller is not aware >> of. >> For example, suppose interrupt controller needs Rising edge interrupt, but >> the board has inverted the line between stmpe and IC. So, we will get >> Rising high from the routine you mentioned, but we need to generate >> opposite of that to make it rising high. > > Surely that would be a hardware design error/quirk? Yes. > Can you give an example where this has happened? I haven't seen this in any of SPEAr boards i have worked on. Maybe Rabin would have, that's why he added that part of code :) @Rabin/Linus: Do you remember why have you added this in stmpe driver: + if (stmpe->pdata->irq_invert_polarity) + icr ^= STMPE_ICR_LSB_HIGH; + Does somebody actually need it? -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] cpufreq: SPEAr: Add CPUFreq driver
On 27 November 2012 10:46, Shawn Guo wrote: > Shouldn't it also be handled by cpu clock provider? Not really sure. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] cpufreq: SPEAr: Add CPUFreq driver
Hi Shawn, Thanks for feedback. On 27 November 2012 09:31, Shawn Guo wrote: > On Sun, Nov 25, 2012 at 01:09:28AM +0530, Viresh Kumar wrote: >> +cpufreq { >> + compatible = "st,cpufreq-spear"; > > I do not think we need a "cpufreq" node, as we already have node for > cpu to contain these. Hmm.. Yes, can be done. >> + cpufreq_tbl = < 166000 >> + 20 >> + 25 >> + 30 >> + 40 >> + 50 >> + 60 >; > > It's a little bit unusual that cpu runs such a freq range > (166 ~ 600 MHz) at a fixed voltage. You really have no way > to scale voltage? AFAIK, there is no voltage scaling. @Shiraz/Deepak: Can you confirm? >> diff --git a/drivers/cpufreq/spear-cpufreq.c >> b/drivers/cpufreq/spear-cpufreq.c >> +static int spear_cpufreq_target(struct cpufreq_policy *policy, >> + unsigned int target_freq, unsigned int relation) >> +{ >> + struct cpufreq_freqs freqs; >> + unsigned long newfreq; >> + struct clk *srcclk; >> + int index, ret, mult = 1; >> + >> + if (cpufreq_frequency_table_target(policy, spear_cpufreq.freq_tbl, >> + target_freq, relation, &index)) >> + return -EINVAL; >> + >> + freqs.cpu = policy->cpu; >> + freqs.old = spear_cpufreq_get(0); >> + >> + newfreq = spear_cpufreq.freq_tbl[index].frequency * 1000; >> + if (of_machine_is_compatible("st,spear1340")) { >> + /* >> + * SPEAr1340 is special in the sense that due to the >> possibility >> + * of multiple clock sources for cpu clk's parent we can have >> + * different clock source for different frequency of cpu clk. >> + * Hence we need to choose one from amongst these possible >> clock >> + * sources. >> + */ >> + srcclk = spear1340_cpu_get_possible_parent(newfreq); > > From what I can see, if spear1340 clock driver can handle such special > setup of clk, the driver will be nothing SPEAr specific and can probably > be saved by just using cpufreq-cpu0. There are few more patches we have for this driver, which are depending on some mach stuff to be pushed first. We need to program system controller too for make cpu clock transitions more stable. They should come after some time. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V3 3/3] mfd: stmpe: Update DT support in stmpe driver
On 27 November 2012 08:10, Viresh Kumar wrote: > On 27 November 2012 00:10, Grant Likely wrote: >> It would actually be good to ask the interrupt controller driver what >> kind of interrupt signal it expects for a given interrupt line. That >> should also solve the problem and I think it would be more useful to >> other devices. Can you investigate whether or not >> irqd_get_trigger_type() returns the information you need? > > That's a pretty cool function to use. :) > > Will check it out :) I was thinking about this logic in my earlier mail, don't know what stopped me from thinking it is wrong. :( Problem is with invert polarity, which the interrupt controller is not aware of. For example, suppose interrupt controller needs Rising edge interrupt, but the board has inverted the line between stmpe and IC. So, we will get Rising high from the routine you mentioned, but we need to generate opposite of that to make it rising high. And so interrupt polarity field is still required. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V3 3/3] mfd: stmpe: Update DT support in stmpe driver
On 27 November 2012 00:10, Grant Likely wrote: > Ah, so it is configuring the way the device emits interrupts; not how > the interrupt controller processes them. Fair enough. Finally i am able to convince somebody that stmpe is different :) > It would actually be good to ask the interrupt controller driver what > kind of interrupt signal it expects for a given interrupt line. That > should also solve the problem and I think it would be more useful to > other devices. Can you investigate whether or not > irqd_get_trigger_type() returns the information you need? That's a pretty cool function to use. :) Will check it out :) -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V1 1/3] mfd: stmpe: Arrange #include in alphabetical order
On 26 November 2012 18:55, Lee Jones wrote: > Why do you need to sort them? Is there _really_ a need? Many people & maintainers like to have their header files ordered. The reason for that is: If they are not ordered, there is a possibility of adding an header file multiple times in a file. This might not do something serious when thinking about compilation time or Image size, but adding an header file multiple times is simply wrong. This can't be caught in patch reviews most of the time, as diff may not show the earlier inclusion. That's why i ordered them. And i don't see any harm in doing so. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH V2] gpio: stmpe: Add DT support for stmpe gpio
From: Vipul Kumar Samar This patch allows the STMPE GPIO driver to be successfully probed and initialised when Device Tree support is enabled. Bindings are mentioned in Documentation too. Acked-by: Lee Jones Signed-off-by: Vipul Kumar Samar Signed-off-by: Viresh Kumar --- V1->V2: - Fixed norequest-mask as st,norequest-mask in Binding Documentation - Added Acked-by from Lee Jones Documentation/devicetree/bindings/gpio/gpio-stmpe.txt | 18 ++ drivers/gpio/gpio-stmpe.c | 10 -- drivers/mfd/stmpe.c | 2 ++ 3 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-stmpe.txt diff --git a/Documentation/devicetree/bindings/gpio/gpio-stmpe.txt b/Documentation/devicetree/bindings/gpio/gpio-stmpe.txt new file mode 100644 index 000..a0e4cf8 --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-stmpe.txt @@ -0,0 +1,18 @@ +STMPE gpio +-- + +Required properties: + - compatible: "st,stmpe-gpio" + +Optional properties: + - st,norequest-mask: bitmask specifying which GPIOs should _not_ be requestable + due to different usage (e.g. touch, keypad) + +Node name must be stmpe_gpio and should be child node of stmpe node to which it +belongs. + +Example: + stmpe_gpio { + compatible = "st,stmpe-gpio"; + st,norequest-mask = <0x20>; //gpio 5 can't be used + }; diff --git a/drivers/gpio/gpio-stmpe.c b/drivers/gpio/gpio-stmpe.c index dce3472..522c90e 100644 --- a/drivers/gpio/gpio-stmpe.c +++ b/drivers/gpio/gpio-stmpe.c @@ -12,6 +12,7 @@ #include #include #include +#include #include /* @@ -304,6 +305,7 @@ static void stmpe_gpio_irq_remove(struct stmpe_gpio *stmpe_gpio) static int __devinit stmpe_gpio_probe(struct platform_device *pdev) { struct stmpe *stmpe = dev_get_drvdata(pdev->dev.parent); + struct device_node *np = pdev->dev.of_node; struct stmpe_gpio_platform_data *pdata; struct stmpe_gpio *stmpe_gpio; int ret; @@ -321,13 +323,17 @@ static int __devinit stmpe_gpio_probe(struct platform_device *pdev) stmpe_gpio->dev = &pdev->dev; stmpe_gpio->stmpe = stmpe; - stmpe_gpio->norequest_mask = pdata ? pdata->norequest_mask : 0; - stmpe_gpio->chip = template_chip; stmpe_gpio->chip.ngpio = stmpe->num_gpios; stmpe_gpio->chip.dev = &pdev->dev; stmpe_gpio->chip.base = pdata ? pdata->gpio_base : -1; + if (pdata) + stmpe_gpio->norequest_mask = pdata->norequest_mask; + else if (np) + of_property_read_u32(np, "st,norequest-mask", + &stmpe_gpio->norequest_mask); + if (irq >= 0) stmpe_gpio->irq_base = stmpe->irq_base + STMPE_INT_GPIO(0); else diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c index 0d5f7b7..8b164f3 100644 --- a/drivers/mfd/stmpe.c +++ b/drivers/mfd/stmpe.c @@ -295,12 +295,14 @@ static struct resource stmpe_gpio_resources[] = { static struct mfd_cell stmpe_gpio_cell = { .name = "stmpe-gpio", + .of_compatible = "st,stmpe-gpio", .resources = stmpe_gpio_resources, .num_resources = ARRAY_SIZE(stmpe_gpio_resources), }; static struct mfd_cell stmpe_gpio_cell_noirq = { .name = "stmpe-gpio", + .of_compatible = "st,stmpe-gpio", /* gpio cell resources consist of an irq only so no resources here */ }; -- 1.7.12.rc2.18.g61b472e ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[GIT PULL]: spear-for-3.8 (was earlier: Re: [PATCH 00/14] ARM: SPEAr: DT updates)
On 22 November 2012 01:54, Olof Johansson wrote: > We're also running out of time for 3.8 staging, but if you're able to > figure out the above I'll make an attempt to apply the patches from above. Hi Arnd/Olof, To make things easy for you, i have a PULL request for you now :) The following changes since commit 53d74fd79db19aae506de892273198b160c2ee95: Merge commit 'gpio-lw/devel' into spear-for-3.8 (2012-11-26 15:48:53 +0530) are available in the git repository at: git://git.linaro.org/people/vireshk/linux.git spear-for-3.8 for you to fetch changes up to df1590d9ae5e37e07e7cf91107e4c2c946ce8bf4: ARM: SPEAr3xx: Shirq: Move shirq controller out of plat/ (2012-11-26 16:55:33 +0530) Deepak Sikri (1): ARM: SPEAr: DT: Modify DT bindings for STMMAC Shiraz Hashim (6): ARM: SPEAr13xx: DT: Add spics gpio controller nodes ARM: SPEAr: DT: Update device nodes ARM: SPEAr13xx: Remove fields not required for ssp controller ARM: SPEAr3xx: shirq: simplify and move the shared irq multiplexor to DT ARM: SPEAr3xx: DT: add shirq node for interrupt multiplexor ARM: SPEAr320: DT: Add SPEAr 320 HMI board support Vipin Kumar (1): ARM: SPEAr: DT: Update partition info for MTD devices Vipul Kumar Samar (5): ARM: SPEAr: DT: Update pinctrl list ARM: SPEAr: DT: Fix existing DT support ARM: SPEAr: DT: add uart state to fix warning ARM: SPEAr1310: Move 1310 specific misc register into machine specific files ARM: SPEAr1310: Fix AUXDATA for compact flash controller Viresh Kumar (1): ARM: SPEAr3xx: Shirq: Move shirq controller out of plat/ .../devicetree/bindings/arm/spear/shirq.txt| 48 arch/arm/boot/dts/Makefile | 3 +- arch/arm/boot/dts/spear1310-evb.dts| 165 +-- arch/arm/boot/dts/spear1310.dtsi | 32 ++- arch/arm/boot/dts/spear1340-evb.dts| 253 +++-- arch/arm/boot/dts/spear1340.dtsi | 61 arch/arm/boot/dts/spear13xx.dtsi | 72 - arch/arm/boot/dts/spear300-evb.dts | 20 +- arch/arm/boot/dts/spear300.dtsi| 14 +- arch/arm/boot/dts/spear310-evb.dts | 30 +- arch/arm/boot/dts/spear310.dtsi| 18 ++ arch/arm/boot/dts/spear320-evb.dts | 35 ++- arch/arm/boot/dts/spear320-hmi.dts | 305 arch/arm/boot/dts/spear320.dtsi| 39 ++- arch/arm/boot/dts/spear3xx.dtsi| 5 +- arch/arm/boot/dts/spear600-evb.dts | 46 ++- arch/arm/boot/dts/spear600.dtsi| 16 ++ arch/arm/mach-spear13xx/include/mach/spear.h | 8 - arch/arm/mach-spear13xx/spear1310.c| 16 +- arch/arm/mach-spear13xx/spear13xx.c| 2 - arch/arm/mach-spear3xx/include/mach/irqs.h | 10 +- arch/arm/mach-spear3xx/spear300.c | 103 --- arch/arm/mach-spear3xx/spear310.c | 202 - arch/arm/mach-spear3xx/spear320.c | 205 + arch/arm/mach-spear3xx/spear3xx.c | 4 + arch/arm/plat-spear/Makefile | 2 +- arch/arm/plat-spear/shirq.c| 118 drivers/clk/spear/spear1310_clock.c| 1 + drivers/irqchip/Makefile | 3 +- drivers/irqchip/spear-shirq.c | 316 + .../shirq.h => include/linux/irqchip/spear-shirq.h | 49 ++-- 31 files changed, 1426 insertions(+), 775 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/spear/shirq.txt create mode 100644 arch/arm/boot/dts/spear320-hmi.dts delete mode 100644 arch/arm/plat-spear/shirq.c create mode 100644 drivers/irqchip/spear-shirq.c rename arch/arm/plat-spear/include/plat/shirq.h => include/linux/irqchip/spear-shirq.h (60%) ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH Resend] gpio: stmpe: Add DT support for stmpe gpio
On 26 November 2012 16:56, Lee Jones wrote: >> From: Vipul Kumar Samar >> +Optional properties: >> + - norequest-mask: bitmask specifying which GPIOs should _not_ be >> requestable > > You have 'norequest-mask' here and 'st,norequest-mask' in the example. > If 'st,norequest-mask' is truly required to be in the DT (I don't > really understand it well enough to comment), and you fix up the > inconsistency in the Documentation, you can apply my: > > Acked-by: Lee Jones Thanks for pointing out. Will fix it up. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 08/14] ARM: SPEAr: DT: Update device nodes
On 11 November 2012 10:09, Viresh Kumar wrote: > From: Shiraz Hashim > > This patch adds multiple device nodes for SPEAr machines and boards. > > Signed-off-by: Bhavna Yadav > Signed-off-by: Deepak Sikri > Signed-off-by: Rajeev Kumar > Signed-off-by: Shiraz Hashim > Signed-off-by: Vijay Kumar Mishra > Signed-off-by: Vipin Kumar > Signed-off-by: Vipul Kumar Samar > Signed-off-by: Viresh Kumar Arnd/Olof, As discussed (With Arnd) over IRC, i would be sending a PULL request for this patchset. There are few updates/fixes in the bindings for few nodes (as the drivers for them just got pushed by their maintainers after some updates) and here is a diff for that. I will merge this diff with current patch in my PULL request. From: Viresh Kumar Date: Mon, 26 Nov 2012 16:44:53 +0530 Subject: [PATCH] fixup! ARM: SPEAr: DT: Update device nodes --- arch/arm/boot/dts/spear1310-evb.dts | 14 -- arch/arm/boot/dts/spear1340-evb.dts | 25 - arch/arm/boot/dts/spear13xx.dtsi| 14 +++--- 3 files changed, 19 insertions(+), 34 deletions(-) diff --git a/arch/arm/boot/dts/spear1310-evb.dts b/arch/arm/boot/dts/spear1310-evb.dts index cd4e2f8..b56a801 100644 --- a/arch/arm/boot/dts/spear1310-evb.dts +++ b/arch/arm/boot/dts/spear1310-evb.dts @@ -348,7 +348,6 @@ cs-gpios = <&gpio1 7 0>, <&spics 0>, <&spics 1>; stmpe610@0 { - status = "okay"; compatible = "st,stmpe610"; reg = <0>; #address-cells = <1>; @@ -364,15 +363,12 @@ pl022,ctrl-len = <0x7>; pl022,wait-state = <0>; pl022,duplex = <0>; - id = <0>; - blocks = <4>; - irq_over_gpio; - irq-gpios = <&gpio1 6 0x4>; + interrupts = <6 0x4>; + interrupt-parent = <&gpio1>; irq-trigger = <0x2>; - stmpe610-ts { - compatible = "stmpe,ts"; - reg = <0>; + stmpe_touchscreen { + compatible = "st,stmpe-ts"; ts,sample-time = <4>; ts,mod-12b = <1>; ts,ref-sel = <0>; @@ -386,7 +382,6 @@ }; m25p80@1 { - status = "okay"; compatible = "st,m25p80"; reg = <1>; spi-max-frequency = <1200>; @@ -404,7 +399,6 @@ }; spidev@2 { - status = "okay"; compatible = "spidev"; reg = <2>; spi-max-frequency = <2500>; diff --git a/arch/arm/boot/dts/spear1340-evb.dts b/arch/arm/boot/dts/spear1340-evb.dts index c519fa1..d6c30ae 100644 --- a/arch/arm/boot/dts/spear1340-evb.dts +++ b/arch/arm/boot/dts/spear1340-evb.dts @@ -319,15 +319,12 @@ #address-cells = <1>; #size-cells = <0>; reg = <0x41>; - irq-over-gpio; - irq-gpios = <&gpio0 4 0x4>; - id = <0>; - blocks = <1>; + interrupts = <4 0x4>; + interrupt-parent = <&gpio0>; irq-trigger = <0x2>; - stmpegpio: stmpe-gpio { - compatible = "stmpe,gpio"; - reg = <0>; + stmpegpio: stmpe_gpio { +
Re: [PATCH V1 1/3] mfd: stmpe: Arrange #include in alphabetical order
On 26 November 2012 16:46, Samuel Ortiz wrote: > Hi Viresh, > > On Fri, Nov 23, 2012 at 12:26:18AM +0530, Viresh Kumar wrote: >> This helps managing them better and also reduces chances of adding an header >> file twice. The aim is to maintain the list of header files in alphabetical order. It helps in maintaining them.. I was adding some header files for my 3rd patch and was looking for the best place to add them and because the list wasn't sorted, i sorted it out in a separate patch. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH] cpufreq: SPEAr: Add CPUFreq driver
From: Deepak Sikri SPEAr is an ARM based family of SoCs. This patch adds in support of cpufreq driver for SPEAr SoCs. It is supported via DT only and so bindings are present in binding document. Signed-off-by: Deepak Sikri Signed-off-by: Viresh Kumar --- .../devicetree/bindings/cpufreq/cpufreq-spear.txt | 29 ++ arch/arm/Kconfig | 1 + drivers/cpufreq/Kconfig.arm| 7 + drivers/cpufreq/Makefile | 1 + drivers/cpufreq/spear-cpufreq.c| 291 + 5 files changed, 329 insertions(+) create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-spear.txt create mode 100644 drivers/cpufreq/spear-cpufreq.c diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-spear.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-spear.txt new file mode 100644 index 000..4cf2819 --- /dev/null +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-spear.txt @@ -0,0 +1,29 @@ +SPEAr cpufreq driver +--- + +SPEAr SoC cpufreq driver for CPU frequency scaling. +It supports both uniprocessor (UP) and symmetric multiprocessor (SMP) systems +which share clock across all CPUs. + +Required properties: +- compatible: "st,cpufreq-spear" +- cpufreq_tbl: Table of frequencies CPU could be transitioned into, in the + increasing order. + +Optional properties: +- clock-latency: Specify the possible maximum transition latency for clock, in + unit of nanoseconds. + +Examples: + + +cpufreq { + compatible = "st,cpufreq-spear"; + cpufreq_tbl = < 166000 + 20 + 25 + 30 + 40 + 50 + 60 >; +}; diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 14f8160..44bb5cf 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -902,6 +902,7 @@ config ARCH_NOMADIK config PLAT_SPEAR bool "ST SPEAr" + select ARCH_HAS_CPUFREQ select ARCH_REQUIRE_GPIOLIB select ARM_AMBA select CLKDEV_LOOKUP diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index b63c335..ee0ef07 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -76,3 +76,10 @@ config ARM_EXYNOS5250_CPUFREQ help This adds the CPUFreq driver for Samsung EXYNOS5250 SoC. + +config ARM_SPEAR_CPUFREQ + bool "SPEAr CPUFreq support" + depends on PLAT_SPEAR + default y + help + This adds the CPUFreq driver support for SPEAr SOCs. diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index 5b1413e..1f254ec0 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -50,6 +50,7 @@ obj-$(CONFIG_ARM_EXYNOS4210_CPUFREQ) += exynos4210-cpufreq.o obj-$(CONFIG_ARM_EXYNOS4X12_CPUFREQ) += exynos4x12-cpufreq.o obj-$(CONFIG_ARM_EXYNOS5250_CPUFREQ) += exynos5250-cpufreq.o obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ) += omap-cpufreq.o +obj-$(CONFIG_ARM_SPEAR_CPUFREQ)+= spear-cpufreq.o ## # PowerPC platform drivers diff --git a/drivers/cpufreq/spear-cpufreq.c b/drivers/cpufreq/spear-cpufreq.c new file mode 100644 index 000..a7fe880 --- /dev/null +++ b/drivers/cpufreq/spear-cpufreq.c @@ -0,0 +1,291 @@ +/* + * drivers/cpufreq/spear-cpufreq.c + * + * CPU Frequency Scaling for SPEAr platform + * + * Copyright (C) 2012 ST Microelectronics + * Deepak Sikri + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include +#include +#include +#include +#include +#include +#include +#include + +/* SPEAr CPUFreq driver data structure */ +static struct { + struct clk *clk; + unsigned int transition_latency; + struct cpufreq_frequency_table *freq_tbl; + u32 cnt; +} spear_cpufreq; + +int spear_cpufreq_verify(struct cpufreq_policy *policy) +{ + return cpufreq_frequency_table_verify(policy, spear_cpufreq.freq_tbl); +} + +static unsigned int spear_cpufreq_get(unsigned int cpu) +{ + return clk_get_rate(spear_cpufreq.clk) / 1000; +} + +static struct clk *spear1340_cpu_get_possible_parent(unsigned long newfreq) +{ + struct clk *sys_pclk; + int pclk; + /* +* In SPEAr1340, cpu clk's parent sys clk can take input from +* following sources +*/ + const char *sys_clk_src[] = { + "sys_syn_clk", + "pll1_clk", + "pll2_clk", + "pll3_clk", + }; + + /* +* As sys cl
Re: [PATCH V3 3/3] mfd: stmpe: Update DT support in stmpe driver
On 23 November 2012 15:11, Grant Likely wrote: > On Fri, 23 Nov 2012 00:26:20 +0530, Viresh Kumar > wrote: >> + - irq-trigger : IRQ trigger to use for the interrupt >> to the host >> + - irq-invert-polarity : bool, IRQ line is connected with >> reversed polarity > > This looks odd. Normally the interrupt polarity should be encoded in the irq > specifier flags field. Hi Grant and Lee Jones, This looks odd because stmpe is odd, i am taking the discussion held with Lee jones to this thread. So, how interrupt stuff works currently in DT.. We have a interrupt controller IC: ic: interrupt-controller@40008000 { compatible = "foo"; interrupt-controller; #interrupt-cells = <2>; ... }; And we have a user of this IC: foo-peripheral@40048000 { compatible = "foo-peripheral"; interrupt-parent = <&ic>; interrupts = <39 4>; }; Here first field of "interrupts" gives interrupt line number and the second one gives polarity, interrupt type etc.. All is good till now. Now, every interrupt controller supports the first field, but the second one depends on its capabilities. An interrupt controller might not have registers to configure interrupt polarity, type, etc of the interrupt it will service and so the second field wouldn't be available for them. For now just think stmpe is not a MFD and not a interrupt controller either. It is just a simple device, dev-foo. It will declare values of its interrupts field based on the type of interrupt controller that will service its interrupt and that can be anything like VIC/GIC/GPIO controller. Obviously nobody else than the parent IC driver can parse interrupts field of dev-foo, because only that driver understands the real meaning of these fields. Now, stmpe has a special property. It can decide the way its output interrupt line will work. i.e. its polarity and interrupt type - edle/level, etc.. This is not commonly seen in any peripheral. Now my original bindings and the real question here is about passing this information to stmpe driver. I can't pass it in interrupts field of stmpe node, as that field belongs to parent interrupt controller of stmpe. I can't pass that from child nodes of stmpe, as we are programming the interrupt coming out of stmpe and not the interrupt coming out of stmpe-gpio or stmpe-keypad. And that's why i added these bindings. Please suggest me if i am still missing something. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH Resend] gpio: stmpe: Add DT support for stmpe gpio
From: Vipul Kumar Samar This patch allows the STMPE GPIO driver to be successfully probed and initialised when Device Tree support is enabled. Bindings are mentioned in Documentation too. Signed-off-by: Vipul Kumar Samar Signed-off-by: Viresh Kumar --- V1->Resend: -- - Don't move asignment of stmpe_gpio->chip.base to if (pdata) block. Documentation/devicetree/bindings/gpio/gpio-stmpe.txt | 18 ++ drivers/gpio/gpio-stmpe.c | 10 -- drivers/mfd/stmpe.c | 2 ++ 3 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-stmpe.txt diff --git a/Documentation/devicetree/bindings/gpio/gpio-stmpe.txt b/Documentation/devicetree/bindings/gpio/gpio-stmpe.txt new file mode 100644 index 000..7f010e0 --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-stmpe.txt @@ -0,0 +1,18 @@ +STMPE gpio +-- + +Required properties: + - compatible: "st,stmpe-gpio" + +Optional properties: + - norequest-mask: bitmask specifying which GPIOs should _not_ be requestable + due to different usage (e.g. touch, keypad) + +Node name must be stmpe_gpio and should be child node of stmpe node to which it +belongs. + +Example: + stmpe_gpio { + compatible = "st,stmpe-gpio"; + st,norequest-mask = <0x20>; //gpio 5 can't be used + }; diff --git a/drivers/gpio/gpio-stmpe.c b/drivers/gpio/gpio-stmpe.c index dce3472..522c90e 100644 --- a/drivers/gpio/gpio-stmpe.c +++ b/drivers/gpio/gpio-stmpe.c @@ -12,6 +12,7 @@ #include #include #include +#include #include /* @@ -304,6 +305,7 @@ static void stmpe_gpio_irq_remove(struct stmpe_gpio *stmpe_gpio) static int __devinit stmpe_gpio_probe(struct platform_device *pdev) { struct stmpe *stmpe = dev_get_drvdata(pdev->dev.parent); + struct device_node *np = pdev->dev.of_node; struct stmpe_gpio_platform_data *pdata; struct stmpe_gpio *stmpe_gpio; int ret; @@ -321,13 +323,17 @@ static int __devinit stmpe_gpio_probe(struct platform_device *pdev) stmpe_gpio->dev = &pdev->dev; stmpe_gpio->stmpe = stmpe; - stmpe_gpio->norequest_mask = pdata ? pdata->norequest_mask : 0; - stmpe_gpio->chip = template_chip; stmpe_gpio->chip.ngpio = stmpe->num_gpios; stmpe_gpio->chip.dev = &pdev->dev; stmpe_gpio->chip.base = pdata ? pdata->gpio_base : -1; + if (pdata) + stmpe_gpio->norequest_mask = pdata->norequest_mask; + else if (np) + of_property_read_u32(np, "st,norequest-mask", + &stmpe_gpio->norequest_mask); + if (irq >= 0) stmpe_gpio->irq_base = stmpe->irq_base + STMPE_INT_GPIO(0); else diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c index fd64efc..3e3ca9c 100644 --- a/drivers/mfd/stmpe.c +++ b/drivers/mfd/stmpe.c @@ -298,12 +298,14 @@ static struct resource stmpe_gpio_resources[] = { static struct mfd_cell stmpe_gpio_cell = { .name = "stmpe-gpio", + .of_compatible = "st,stmpe-gpio", .resources = stmpe_gpio_resources, .num_resources = ARRAY_SIZE(stmpe_gpio_resources), }; static struct mfd_cell stmpe_gpio_cell_noirq = { .name = "stmpe-gpio", + .of_compatible = "st,stmpe-gpio", /* gpio cell resources consist of an irq only so no resources here */ }; -- 1.7.12.rc2.18.g61b472e ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2 2/2] mfd: stmpe: Extend DT support in stmpe driver
On 23 November 2012 21:13, Lee Jones wrote: > No, in DT devices named as part of the hiearchy, so you'd have: > > soc-u9500/i2c@80004000/stmpe1601@40/stmpe_keypad > soc-u9500/i2c@80004000/stmpe1601@41/stmpe_keypad > ... etc Obviously. How could i miss this naming :( Okay, so i need to pass -1 and that would be enough. Right? -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2 2/2] mfd: stmpe: Extend DT support in stmpe driver
On 23 November 2012 15:06, Lee Jones wrote: > On Fri, 23 Nov 2012, Viresh Kumar wrote: >> pdev = platform_device_alloc(cell->name, id + cell->id); >> >> This is required when we have multiple instances of MFD device present >> on board. How do you want me to handle this ? > > There are lots of examples of this already. I have to leave something > to the imagination, or I'll be requesting a cut of your salary. :D My manager already reduced my salary by 20% after reading this mail :( Ok, this is what my understanding of whole this is. Platform devices are named like: - pdev-name: if id passed in pdev.id is -1 - pdev-name.0[1|2|...]: if id passed is 0[1|2|...] - pdev-name.: if id passed is -2 Now, we don't declare cell->id fields and they are currently zero and so value is passed from pdata->id field. So, for example with multiple instances of stmpe on a board, we have: - stmpe-0: //Name just for reference... - stmpe-gpio.0 - stmpe-ts.0 - stmpe-1: - stmpe-gpio.1 - stmpe-ts.1 - stmpe-2: - stmpe-gpio.2 - stmpe-ts.2 I main idea is to distinguish various instances of sub modules, like stmpe-gpio. And this works well with non-DT support we have currently. With DT, i am not sure how should we pass id field to mfd_add_devices(). If we pass it -1, then multiple instances will have same name: "stmpe-gpio" Sorry, for my lack of knowledge. Don't send another mail with salary cut suggestion as that will make it -40% in total ;) Thanks for reviewing :) -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] gpio: stmpe: Add DT support for stmpe gpio
On 23 November 2012 17:44, Lee Jones wrote: > I'm saying, just leave it where it is. So you are suggesting this code: stmpe_gpio->chip.base = pdata ? pdata->gpio_base : -1; if (pdata) stmpe_gpio->norequest_mask = pdata->norequest_mask; else if (np) of_property_read_u32(np, "st,norequest-mask", &pdata->norequest_mask); Right? Then yes i can do it. >> >> + if (np) >> >> + of_property_read_u32(np, "st,norequest-mask", >> >> + &pdata->norequest_mask); >> > >> > Can you explain to me what this does? >> >> You mean pdata->norequest_mask? It marks few gpios as unusable. >> Because these pads might be used by other blocks of stmpe. > > I'm not sure if that should be set with DT or not. > > Second opinion anyone? Why i kept it in DT is because it is board dependent and there is no better way of communicating this from board to driver. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] input: stmpe-ts: Add DT support for stmpe touchscreen
On 23 November 2012 16:16, Lee Jones wrote: >> +++ b/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt >> @@ -0,0 +1,43 @@ >> +STMPE Touchscreen >> + >> + >> +Required properties: >> + - compatible: "st,stmpe-ts" > > You shouldn't be specifying a compatible string in the DT. > >> +Example: >> + >> + stmpe_touchscreen { >> + compatible = "st,stmpe-ts"; > > This needs to be removed. Copying my earlier reply from stmpe-gpio patch for others to know what i have to say on this :) I believe these are required by the code you wrote in mfd-core.c if (parent->of_node && cell->of_compatible) { for_each_child_of_node(parent->of_node, np) { if (of_device_is_compatible(np, cell->of_compatible)) { pdev->dev.of_node = np; break; } } } This matches compatible of child node with compatible of cell. And that's why you have added that in your keypad mappings as well. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] gpio: stmpe: Add DT support for stmpe gpio
On 23 November 2012 16:11, Lee Jones wrote: >> > +++ b/Documentation/devicetree/bindings/gpio/gpio-stmpe.txt >> > @@ -0,0 +1,18 @@ >> > +STMPE gpio >> > +-- >> > + >> > +Required properties: >> > + - compatible: "st,stmpe-gpio" > > ... but this is wrong. > >> > +Example: >> > + stmpe_gpio { >> > + compatible = "st,stmpe-gpio"; >> > + st,norequest-mask = <0x20>; //gpio 5 can't be used >> > + }; > > As is the example. > > So will be the the DT - if you've already written it. Again, I believe these are required by the code you wrote in mfd-core.c if (parent->of_node && cell->of_compatible) { for_each_child_of_node(parent->of_node, np) { if (of_device_is_compatible(np, cell->of_compatible)) { pdev->dev.of_node = np; break; } } } This matches compatible of child node with compatible of cell. And that's why you have added that in your keypad mappings as well. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] gpio: stmpe: Add DT support for stmpe gpio
On 23 November 2012 16:04, Lee Jones wrote: > On Fri, 23 Nov 2012, Viresh Kumar wrote: >> diff --git a/drivers/gpio/gpio-stmpe.c b/drivers/gpio/gpio-stmpe.c >> static int __devinit stmpe_gpio_probe(struct platform_device *pdev) >> { >> - stmpe_gpio->chip.base = pdata ? pdata->gpio_base : -1; > > Why have you deleted this? > >> + >> + if (pdata) { >> + stmpe_gpio->norequest_mask = pdata->norequest_mask; >> + stmpe_gpio->chip.base = pdata->gpio_base; > > Then added this? > >> + } else { >> + stmpe_gpio->chip.base = -1; > > And this? To group all non-DT assignments in a single if block, instead of two. > Just leave the top line in and it saves you lots of complecations. Sorry, Couldn't get this one. >> + if (np) >> + of_property_read_u32(np, "st,norequest-mask", >> + &pdata->norequest_mask); > > Can you explain to me what this does? You mean pdata->norequest_mask? It marks few gpios as unusable. Because these pads might be used by other blocks of stmpe. >> diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c >> static struct mfd_cell stmpe_gpio_cell = { >> .name = "stmpe-gpio", >> + .of_compatible = "st,stmpe-gpio", > > There's no need for any of the STMPE to have their own compatible > string, as they are MFD devices. They are registered as platform > devices from the MFD subsystem. This is required by mfd-core.c, mfd_add_device() isn't it? if (parent->of_node && cell->of_compatible) { for_each_child_of_node(parent->of_node, np) { if (of_device_is_compatible(np, cell->of_compatible)) { pdev->dev.of_node = np; break; } } } -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2 1/2] mfd: stmpe: Use devm_*() routines
On 23 November 2012 15:10, Lee Jones wrote: > That's not how it works. > > Apply my Acked-by yourself and re-send the patch-set as a whole. I don't thinks so. This patch is not doing anything with DT stuff and so can be applied separately. Related patches should be posted together though. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH] input: stmpe-ts: Add DT support for stmpe touchscreen
From: Vipul Kumar Samar This patch allows the STMPE Touchscreen driver to be successfully probed and initialised when Device Tree support is enabled. Bindings are mentioned in Documentation too. Signed-off-by: Vipul Kumar Samar Signed-off-by: Viresh Kumar --- .../bindings/input/touchscreen/stmpe.txt | 43 ++ drivers/input/touchscreen/stmpe-ts.c | 65 -- drivers/mfd/stmpe.c| 1 + 3 files changed, 91 insertions(+), 18 deletions(-) create mode 100644 Documentation/devicetree/bindings/input/touchscreen/stmpe.txt diff --git a/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt b/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt new file mode 100644 index 000..127baa3 --- /dev/null +++ b/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt @@ -0,0 +1,43 @@ +STMPE Touchscreen + + +Required properties: + - compatible: "st,stmpe-ts" + +Optional properties: +- st,sample-time: ADC converstion time in number of clock. (0 -> 36 clocks, 1 -> + 44 clocks, 2 -> 56 clocks, 3 -> 64 clocks, 4 -> 80 clocks, 5 -> 96 clocks, 6 + -> 144 clocks), recommended is 4. +- st,mod-12b: ADC Bit mode (0 -> 10bit ADC, 1 -> 12bit ADC) +- st,ref-sel: ADC reference source (0 -> internal reference, 1 -> external + reference) +- st,adc-freq: ADC Clock speed (0 -> 1.625 MHz, 1 -> 3.25 MHz, 2 || 3 -> 6.5 MHz) +- st,ave-ctrl: Sample average control (0 -> 1 sample, 1 -> 2 samples, 2 -> 4 + samples, 3 -> 8 samples) +- st,touch-det-delay: Touch detect interrupt delay (0 -> 10 us, 1 -> 50 us, 2 -> + 100 us, 3 -> 500 us, 4-> 1 ms, 5 -> 5 ms, 6 -> 10 ms, 7 -> 50 ms) recommended + is 3 +- st,settling: Panel driver settling time (0 -> 10 us, 1 -> 100 us, 2 -> 500 us, 3 + -> 1 ms, 4 -> 5 ms, 5 -> 10 ms, 6 for 50 ms, 7 -> 100 ms) recommended is 2 +- st,fraction-z: Length of the fractional part in z (fraction-z ([0..7]) = Count of + the fractional part) recommended is 7 +- st,i-drive: current limit value of the touchscreen drivers (0 -> 20 mA typical 35 + mA max, 1 -> 50 mA typical 80 mA max) + +Node name must be stmpe_touchscreen and should be child node of stmpe node to +which it belongs. + +Example: + + stmpe_touchscreen { + compatible = "st,stmpe-ts"; + st,sample-time = <4>; + st,mod-12b = <1>; + st,ref-sel = <0>; + st,adc-freq = <1>; + st,ave-ctrl = <1>; + st,touch-det-delay = <2>; + st,settling = <2>; + st,fraction-z = <7>; + st,i-drive = <1>; + }; diff --git a/drivers/input/touchscreen/stmpe-ts.c b/drivers/input/touchscreen/stmpe-ts.c index b3f7503..8f4985a 100644 --- a/drivers/input/touchscreen/stmpe-ts.c +++ b/drivers/input/touchscreen/stmpe-ts.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -262,11 +263,53 @@ static void stmpe_ts_close(struct input_dev *dev) STMPE_TSC_CTRL_TSC_EN, 0); } -static int __devinit stmpe_input_probe(struct platform_device *pdev) +static void stmpe_ts_get_info(struct platform_device *pdev, + struct stmpe_touch *ts) { struct stmpe *stmpe = dev_get_drvdata(pdev->dev.parent); - const struct stmpe_platform_data *pdata = stmpe->pdata; - const struct stmpe_ts_platform_data *ts_pdata = NULL; + struct device_node *np = pdev->dev.of_node; + struct stmpe_ts_platform_data *ts_pdata = NULL; + + ts->stmpe = stmpe; + + if (stmpe->pdata && stmpe->pdata->ts) { + ts_pdata = stmpe->pdata->ts; + + ts->sample_time = ts_pdata->sample_time; + ts->mod_12b = ts_pdata->mod_12b; + ts->ref_sel = ts_pdata->ref_sel; + ts->adc_freq = ts_pdata->adc_freq; + ts->ave_ctrl = ts_pdata->ave_ctrl; + ts->touch_det_delay = ts_pdata->touch_det_delay; + ts->settling = ts_pdata->settling; + ts->fraction_z = ts_pdata->fraction_z; + ts->i_drive = ts_pdata->i_drive; + } else if (np) { + u32 val; + + if (!of_property_read_u32(np, "st,sample-time", &val)) + ts->sample_time = val; + if (!of_property_read_u32(np, "st,mod-12b", &val)) + ts->mod_12b = val; + if (!of_property_read_u32(np, "st,ref-sel", &val)) + ts->ref_sel = val; + if (!of_property_read_u32(np, "st,adc-freq", &val)) +
[PATCH] gpio: stmpe: Add DT support for stmpe gpio
From: Vipul Kumar Samar This patch allows the STMPE GPIO driver to be successfully probed and initialised when Device Tree support is enabled. Bindings are mentioned in Documentation too. Signed-off-by: Vipul Kumar Samar Signed-off-by: Viresh Kumar --- Documentation/devicetree/bindings/gpio/gpio-stmpe.txt | 18 ++ drivers/gpio/gpio-stmpe.c | 15 --- drivers/mfd/stmpe.c | 2 ++ 3 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-stmpe.txt diff --git a/Documentation/devicetree/bindings/gpio/gpio-stmpe.txt b/Documentation/devicetree/bindings/gpio/gpio-stmpe.txt new file mode 100644 index 000..7f010e0 --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-stmpe.txt @@ -0,0 +1,18 @@ +STMPE gpio +-- + +Required properties: + - compatible: "st,stmpe-gpio" + +Optional properties: + - norequest-mask: bitmask specifying which GPIOs should _not_ be requestable + due to different usage (e.g. touch, keypad) + +Node name must be stmpe_gpio and should be child node of stmpe node to which it +belongs. + +Example: + stmpe_gpio { + compatible = "st,stmpe-gpio"; + st,norequest-mask = <0x20>; //gpio 5 can't be used + }; diff --git a/drivers/gpio/gpio-stmpe.c b/drivers/gpio/gpio-stmpe.c index dce3472..91455d4 100644 --- a/drivers/gpio/gpio-stmpe.c +++ b/drivers/gpio/gpio-stmpe.c @@ -12,6 +12,7 @@ #include #include #include +#include #include /* @@ -304,6 +305,7 @@ static void stmpe_gpio_irq_remove(struct stmpe_gpio *stmpe_gpio) static int __devinit stmpe_gpio_probe(struct platform_device *pdev) { struct stmpe *stmpe = dev_get_drvdata(pdev->dev.parent); + struct device_node *np = pdev->dev.of_node; struct stmpe_gpio_platform_data *pdata; struct stmpe_gpio *stmpe_gpio; int ret; @@ -321,12 +323,19 @@ static int __devinit stmpe_gpio_probe(struct platform_device *pdev) stmpe_gpio->dev = &pdev->dev; stmpe_gpio->stmpe = stmpe; - stmpe_gpio->norequest_mask = pdata ? pdata->norequest_mask : 0; - stmpe_gpio->chip = template_chip; stmpe_gpio->chip.ngpio = stmpe->num_gpios; stmpe_gpio->chip.dev = &pdev->dev; - stmpe_gpio->chip.base = pdata ? pdata->gpio_base : -1; + + if (pdata) { + stmpe_gpio->norequest_mask = pdata->norequest_mask; + stmpe_gpio->chip.base = pdata->gpio_base; + } else { + stmpe_gpio->chip.base = -1; + if (np) + of_property_read_u32(np, "st,norequest-mask", + &pdata->norequest_mask); + } if (irq >= 0) stmpe_gpio->irq_base = stmpe->irq_base + STMPE_INT_GPIO(0); diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c index e2c0dda..c757ac3 100644 --- a/drivers/mfd/stmpe.c +++ b/drivers/mfd/stmpe.c @@ -298,12 +298,14 @@ static struct resource stmpe_gpio_resources[] = { static struct mfd_cell stmpe_gpio_cell = { .name = "stmpe-gpio", + .of_compatible = "st,stmpe-gpio", .resources = stmpe_gpio_resources, .num_resources = ARRAY_SIZE(stmpe_gpio_resources), }; static struct mfd_cell stmpe_gpio_cell_noirq = { .name = "stmpe-gpio", + .of_compatible = "st,stmpe-gpio", /* gpio cell resources consist of an irq only so no resources here */ }; -- 1.7.12.rc2.18.g61b472e ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2 2/2] mfd: stmpe: Extend DT support in stmpe driver
On 22 November 2012 16:54, Lee Jones wrote: >> diff --git a/Documentation/devicetree/bindings/mfd/stmpe.txt >> b/Documentation/devicetree/bindings/mfd/stmpe.txt >> stmpe1601: stmpe1601@40 { >> + id = <0>; > > Don't do this. Device IDs are Linux specific. Hi Lee, This is id of the mfd device that we need to pass to mfd_add_device() and is used in following: pdev = platform_device_alloc(cell->name, id + cell->id); This is required when we have multiple instances of MFD device present on board. How do you want me to handle this ? -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH V3 3/3] mfd: stmpe: Update DT support in stmpe driver
From: Vipul Kumar Samar This patch extends existing DT support for stmpe devices. This updates: - DT support from stmpe SPI and I2C drivers - missing header files in stmpe.c - stmpe_of_probe() with pwm, rotator and new bindings. - Bindings are updated in binding document. Signed-off-by: Vipul Kumar Samar Signed-off-by: Viresh Kumar --- V2->V3: -- - Removed sub-modules DT bindings from this patch - Retain original work done by Lee Jones Documentation/devicetree/bindings/mfd/stmpe.txt | 12 drivers/mfd/stmpe-i2c.c | 15 +++ drivers/mfd/stmpe-spi.c | 15 +++ drivers/mfd/stmpe.c | 21 +++-- 4 files changed, 53 insertions(+), 10 deletions(-) diff --git a/Documentation/devicetree/bindings/mfd/stmpe.txt b/Documentation/devicetree/bindings/mfd/stmpe.txt index 8f0aeda..8f65c8d 100644 --- a/Documentation/devicetree/bindings/mfd/stmpe.txt +++ b/Documentation/devicetree/bindings/mfd/stmpe.txt @@ -1,13 +1,17 @@ -* STMPE Multi-Functional Device +* ST Microelectronics STMPE Multi-Functional Device + +STMPE is an MFD device which may expose following inbuilt devices: gpio, keypad, +touchscreen, adc, pwm, rotator. Required properties: - - compatible : "st,stmpe[811|1601|2401|2403]" - - reg : I2C address of the device + - compatible : "st,stmpe[610|801|811|1601|2401|2403]" + - reg : I2C/SPI address of the device Optional properties: - interrupts : The interrupt outputs from the controller - - interrupt-controller : Marks the device node as an interrupt controller - interrupt-parent : Specifies which IRQ controller we're connected to + - irq-trigger : IRQ trigger to use for the interrupt to the host + - irq-invert-polarity : bool, IRQ line is connected with reversed polarity - i2c-client-wake : Marks the input device as wakable - st,autosleep-timeout : Valid entries (ms); 4, 16, 32, 64, 128, 256, 512 and 1024 diff --git a/drivers/mfd/stmpe-i2c.c b/drivers/mfd/stmpe-i2c.c index c734dc3..003c49b 100644 --- a/drivers/mfd/stmpe-i2c.c +++ b/drivers/mfd/stmpe-i2c.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include "stmpe.h" @@ -81,6 +82,19 @@ static const struct i2c_device_id stmpe_i2c_id[] = { }; MODULE_DEVICE_TABLE(i2c, stmpe_id); +#ifdef CONFIG_OF +static const struct of_device_id stmpe_dt_ids[] = { + { .compatible = "st,stmpe610", .data = &stmpe_i2c_id[0], }, + { .compatible = "st,stmpe801", .data = &stmpe_i2c_id[1], }, + { .compatible = "st,stmpe811", .data = &stmpe_i2c_id[2], }, + { .compatible = "st,stmpe1601", .data = &stmpe_i2c_id[3], }, + { .compatible = "st,stmpe2401", .data = &stmpe_i2c_id[4], }, + { .compatible = "st,stmpe2403", .data = &stmpe_i2c_id[5], }, + { } +}; +MODULE_DEVICE_TABLE(of, stmpe_dt_ids); +#endif + static struct i2c_driver stmpe_i2c_driver = { .driver = { .name = "stmpe-i2c", @@ -88,6 +102,7 @@ static struct i2c_driver stmpe_i2c_driver = { #ifdef CONFIG_PM .pm = &stmpe_dev_pm_ops, #endif + .of_match_table = of_match_ptr(stmpe_dt_ids), }, .probe = stmpe_i2c_probe, .remove = __devexit_p(stmpe_i2c_remove), diff --git a/drivers/mfd/stmpe-spi.c b/drivers/mfd/stmpe-spi.c index f86e3fc..e482f5f 100644 --- a/drivers/mfd/stmpe-spi.c +++ b/drivers/mfd/stmpe-spi.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include "stmpe.h" @@ -119,6 +120,19 @@ static const struct spi_device_id stmpe_spi_id[] = { }; MODULE_DEVICE_TABLE(spi, stmpe_id); +#ifdef CONFIG_OF +static const struct of_device_id stmpe_dt_ids[] = { + { .compatible = "st,stmpe610", .data = (void *)STMPE610, }, + { .compatible = "st,stmpe801", .data = (void *)STMPE801, }, + { .compatible = "st,stmpe811", .data = (void *)STMPE811, }, + { .compatible = "st,stmpe1601", .data = (void *)STMPE1601, }, + { .compatible = "st,stmpe2401", .data = (void *)STMPE2401, }, + { .compatible = "st,stmpe2403", .data = (void *)STMPE2403, }, + { } +}; +MODULE_DEVICE_TABLE(of, stmpe_dt_ids); +#endif + static struct spi_driver stmpe_spi_driver = { .driver = { .name = "stmpe-spi", @@ -126,6 +140,7 @@ static struct spi_driver stmpe_spi_driver = { #ifdef CONFIG_PM .pm = &stmpe_dev_pm_ops, #endif + .of_match_table = of_match_ptr(stmpe_dt_ids), }, .probe = stmpe_spi_probe, .remove
[PATCH V1 2/3] mfd: stmpe-i2c: Move .driver structure fields inside {} in stmpe_i2c_driver
Currently, few fields in stmpe_i2c_driver are initialized as: .driver.owner = THIS_MODULE, Group them under {}, like: .driver = { .owner = THIS_MODULE, ... }, Signed-off-by: Viresh Kumar --- drivers/mfd/stmpe-i2c.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/mfd/stmpe-i2c.c b/drivers/mfd/stmpe-i2c.c index 947a06a..c734dc3 100644 --- a/drivers/mfd/stmpe-i2c.c +++ b/drivers/mfd/stmpe-i2c.c @@ -82,11 +82,13 @@ static const struct i2c_device_id stmpe_i2c_id[] = { MODULE_DEVICE_TABLE(i2c, stmpe_id); static struct i2c_driver stmpe_i2c_driver = { - .driver.name= "stmpe-i2c", - .driver.owner = THIS_MODULE, + .driver = { + .name = "stmpe-i2c", + .owner = THIS_MODULE, #ifdef CONFIG_PM - .driver.pm = &stmpe_dev_pm_ops, + .pm = &stmpe_dev_pm_ops, #endif + }, .probe = stmpe_i2c_probe, .remove = __devexit_p(stmpe_i2c_remove), .id_table = stmpe_i2c_id, -- 1.7.12.rc2.18.g61b472e ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH V1 1/3] mfd: stmpe: Arrange #include in alphabetical order
This helps managing them better and also reduces chances of adding an header file twice. Signed-off-by: Viresh Kumar --- drivers/mfd/stmpe-spi.c | 2 +- drivers/mfd/stmpe.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/mfd/stmpe-spi.c b/drivers/mfd/stmpe-spi.c index 9edfe86..f86e3fc 100644 --- a/drivers/mfd/stmpe-spi.c +++ b/drivers/mfd/stmpe-spi.c @@ -7,10 +7,10 @@ * Author: Viresh Kumar for ST Microelectronics */ -#include #include #include #include +#include #include #include "stmpe.h" diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c index 8a4247b..6086481 100644 --- a/drivers/mfd/stmpe.c +++ b/drivers/mfd/stmpe.c @@ -7,15 +7,15 @@ * Author: Rabin Vincent for ST-Ericsson */ -#include #include -#include +#include #include #include #include +#include +#include #include #include -#include #include "stmpe.h" static int __stmpe_enable(struct stmpe *stmpe, unsigned int blocks) -- 1.7.12.rc2.18.g61b472e ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2 2/2] mfd: stmpe: Extend DT support in stmpe driver
On 22 November 2012 16:54, Lee Jones wrote: >> diff --git a/Documentation/devicetree/bindings/mfd/stmpe.txt >> b/Documentation/devicetree/bindings/mfd/stmpe.txt >> +Optional properties: >> +- irq-trigger: IRQ trigger to use for the interrupt to the host >> +- irq-invert-polarity: bool, IRQ line is connected with reversed polarity > > Are these new? > > When adding new bindings, ask yourself: I was trying to get information of these two bindings from other sources, but couldn't find. The only other place can be the interrupts field, right? Because interrupts field depends on the interrupt controller, which can be a wide variety of controllers in our case, we can't use that. Also, the cells of interrupts field will have something that is required to be programmed in the interrupt controller and not in the user of IC. But here we are talking about programming stmpe and so these bindings are very much required in stmpe only. Comments?? -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2 1/2] mfd: stmpe: Use devm_*() routines
On 22 November 2012 15:57, Lee Jones wrote: > On Thu, 22 Nov 2012, Viresh Kumar wrote: > >> This patch frees stmpe driver from tension of freeing resources :) >> devm_* derivatives of multiple routines are used while allocating resources, >> which would be freed automatically by kernel. >> >> Signed-off-by: Viresh Kumar > Acked-by: Lee Jones Hi Sameo, Please apply this patch only from this series. I will not resend it in V3 of my set. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2 2/2] mfd: stmpe: Extend DT support in stmpe driver
On 22 November 2012 21:16, Lee Jones wrote: >> >> diff --git a/Documentation/devicetree/bindings/mfd/stmpe.txt >> >> b/Documentation/devicetree/bindings/mfd/stmpe.txt >> >> +- irq-over-gpio: bool, true if gpio is used to get irq >> >> +- irq-gpios: gpio number over which irq will be requested (significant >> >> only if >> >> + irq-over-gpio is true) >> > >> > You don't need these. Use gpio_to_irq() instead. >> >> I am passing gpio numbers here and am doing gpio_to_irq() in driver. >> Didn't get this one :( > > For a start you have 'irq-over-gpio' in the binding document and Device Tree > and 'irq_over_gpio' in the code. Has it even been tested? > > GPIOs are used as IRQ lines in many other previously DT:ed drivers. Take a > look to see how they are handled without adding unnecessary DT bindings. I already knew it, should have picked that up. :( >> stmpe is an interrupt controller for the IP's which are present inside >> it: gpio, adc. >> But interrupt lines for them are managed by stmpe driver internally. So >> should >> we really add interrupt-controller for it? > > You can't manage IRQ lines internally, you have to go through > the IRQ subsystem. When you request an IRQ via device tree you > will do so like this: By that i meant, there is no external node which would have stmpe as interrupt controller. Because all of them would be its child node. This is guaranteed because stmpe is an external device is present on board. So, it will have its entry in board dts file, and so wouldn't be scattered in different files. > The STMPE GPIO controller can't be used by Device Tree yet in any case, > because it doesn't have an IRQ domain. This is compulsory, or it won't > work. Have you tried to test this functionality yet? I don't have SPEAr board to test it anymore. I have moved out of ST now and working in linaro as ARM asignee. Just pushing these as an part time activity. Though ST guys would have tested stmpe, but stmpe-gpio, i am not sure about. > Okay, I've just had a look at my tested bindings. > > We already have these: > > debounce-interval /* This is a generic binding */ > st,scan-count /* These are vendor specific */ > st,no-autorepeat/*" */ > > Vendor specific bindings should be ",", rather than > "," Will take care of these. >> >> + reg = <0>; >> > >> > You have reg twice here. Also reg should never be '0'. >> >> For SPI, there are chip selects and there is no reg offset. > > I understand the addressing issues, but you have 'reg' twice. > > Which one should be used? I haven't replied on that, because it was accepted. Only one definition should be there for reg. > I didn't go through them, but are you sure that: > > 1. Can I do without them? >1.1 Can I derive the configuration from other things? >2.2 Are they _really_ required, or am I just blindly copying platform data? > 2. Does a similar binding already exist? > 3. Can other drivers make use of them? >3.1 If so, create a generic binding >3.2 If not, prepend the binding with "," I will go through them again. >> >> diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c >> Because i wanted to keep all DT stuff together. Obviously i have seen keypad >> driver earlier :) > > If you had, you'd realise that these bindings already exist. ;) Ok. I had seen it during my V1 and missed these bindings. will fix them now. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2 2/2] mfd: stmpe: Extend DT support in stmpe driver
On 22 November 2012 16:54, Lee Jones wrote: > Big fat NACK. > > You've just overwritten the current implementation with your own. > Please take time to understand the mechanisms in place before > you submit any changes or additions to it. :) My fault. Comments on all overwritten stuff accepted >> diff --git a/Documentation/devicetree/bindings/mfd/stmpe.txt >> b/Documentation/devicetree/bindings/mfd/stmpe.txt >> +- irq-over-gpio: bool, true if gpio is used to get irq >> +- irq-gpios: gpio number over which irq will be requested (significant only >> if >> + irq-over-gpio is true) > > You don't need these. Use gpio_to_irq() instead. I am passing gpio numbers here and am doing gpio_to_irq() in driver. Didn't get this one :( >> Optional properties: >> - - interrupts : The interrupt outputs from the controller >> - - interrupt-controller : Marks the device node as an interrupt >> controller >> - - interrupt-parent : Specifies which IRQ controller we're >> connected to >> - - i2c-client-wake : Marks the input device as wakable >> - - st,autosleep-timeout : Valid entries (ms); 4, 16, 32, 64, 128, >> 256, 512 and 1024 > > And you've removed these why? No. They are readjusted... One thing removed is interrupt-controller. I had a doubt on this. stmpe, by itself doesn't give any interrupt lines to SoC to freely use them. Instead gpio controller driver part of it does. And so adding interrupt-controller for that is the right option. stmpe is an interrupt controller for the IP's which are present inside it: gpio, adc. But interrupt lines for them are managed by stmpe driver internally. So should we really add interrupt-controller for it? >> +- keypad,scan-count: number of key scanning cycles to confirm key data. >> Maximum >> + is STMPE_KEYPAD_MAX_SCAN_COUNT. >> +- keypad,debounce-ms: debounce interval, in ms. Maximum is >> + STMPE_KEYPAD_MAX_DEBOUNCE. >> +- keypad,no-autorepeat: bool, disable key autorepeat > > See "When adding new bindings, ask yourself" above. Yes, these are required. This is part of platform data it expects. >> +stmpe-ts: >> +--- > See "When adding new bindings, ask yourself" above. Same. Can you explicitly point out, which bindings you didn't like. >> +spi@e010 { > > This shouldn't be a child of the SPI device becuase it uses SPI. > > Drivers use clocks, regulators, IRQ controller too, but they're > not children of those devices. Yes. >> + reg = <0>; > > You have reg twice here. Also reg should never be '0'. For SPI, there are chip selects and there is no reg offset. >> + stmpe610-ts { >> + compatible = "stmpe,ts"; >> + ts,sample-time = <4>; >> + ts,mod-12b = <1>; >> + ts,ref-sel = <0>; >> + ts,adc-freq = <1>; >> + ts,ave-ctrl = <1>; >> + ts,touch-det-delay = <2>; >> + ts,settling = <2>; >> + ts,fraction-z = <7>; >> + ts,i-drive = <1>; > > Wow! See "When adding new bindings, ask yourself" above. :) They are required. I didn't get your point, sorry. >> diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c >> +static struct stmpe_keypad_platform_data * >> +get_keyboard_pdata_dt(struct device *dev, struct device_node *np) >> +{ >> + struct stmpe_keypad_platform_data *pdata; >> + u32 val; >> + >> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); >> + if (!pdata) { >> + dev_warn(dev, "stmpe keypad kzalloc fail\n"); >> + return NULL; >> + } >> + >> + if (!of_property_read_u32(np, "keypad,scan-count", &val)) >> + pdata->scan_count = val; >> + if (!of_property_read_u32(np, "keypad,debounce-ms", &val)) >> + pdata->debounce_ms = val; >> + if (of_property_read_bool(np, "keypad,no-autorepeat")) >> + pdata->no_autorepeat = true; > > Why are you (re)adding these here? Have you even looked in the driver? Because i wanted to keep all DT stuff together. Obviously i have seen keypad driver earlier :) I am not setting pdata of stmpe here, but pdata of keypad. >> +static struct stmpe_gpio_platform_data *get_gpio_pdata_dt(struct device >> *dev, >> + struct device_node *np) >> +{ >> + struct stmpe_gpio_platform_data *pdata; >> + u32 val; >> + >> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); >> + if (!pdata) { >> + dev_warn(dev, "stmpe gpio kzalloc fail\n"); >> + return NULL; >> + } >> + >> + if (!of_property_read_u32(np, "gpio,norequest-mask", &val)) >> + pdata->norequest_mask = val; >> + >> + /* assign gpio numbers dynamically */ >> + pdata->gpio_base = -1; >> + >> + return pdata; >> +} > > Is this function really required? Even if is is, should it live here > or in the STMPE driver? As said earlier, ei
Re: [PATCH V2 1/2] mfd: stmpe: Use devm_*() routines
On 22 November 2012 15:57, Lee Jones wrote: > I'm assuming you've reversed the semantics here for >80 chars reasons? Not for 80 chars reason :) I did it to decrease nesting level of if/else statements :) -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH V2 2/2] mfd: stmpe: Extend DT support in stmpe driver
From: Vipul Kumar Samar This patch extends existing DT support for stmpe devices. Bindings are mentioned in binding document. Signed-off-by: Vipul Kumar Samar Signed-off-by: Viresh Kumar --- V1->V2: -- - Partial DT support was already there, which i missed earlier. - Now, this patch is an enhancement of existing DT support. Documentation/devicetree/bindings/mfd/stmpe.txt | 127 ++-- drivers/mfd/stmpe-i2c.c | 23 ++- drivers/mfd/stmpe-spi.c | 15 ++ drivers/mfd/stmpe.c | 264 +--- 4 files changed, 384 insertions(+), 45 deletions(-) diff --git a/Documentation/devicetree/bindings/mfd/stmpe.txt b/Documentation/devicetree/bindings/mfd/stmpe.txt index 8f0aeda..44aebf3 100644 --- a/Documentation/devicetree/bindings/mfd/stmpe.txt +++ b/Documentation/devicetree/bindings/mfd/stmpe.txt @@ -1,25 +1,124 @@ -* STMPE Multi-Functional Device +ST Microelectronics STMPE Multi-Functional Device +- +STMPE is an MFD device which may expose following inbuilt devices: gpio, keypad, +touchscreen, adc, pwm, rotator. + +stmpe: +- Required properties: - - compatible : "st,stmpe[811|1601|2401|2403]" - - reg : I2C address of the device +- compatible: Must be one of: "st,stmpe610", "st,stmpe801", "st,stmpe811", + "st,stmpe1601", "st,stmpe2401", "st,stmpe2403", +- id: device id to distinguish between multiple STMPEs on the same board +- reg: I2C/SPI address of the device + +Optional properties: +- irq-trigger: IRQ trigger to use for the interrupt to the host +- irq-invert-polarity: bool, IRQ line is connected with reversed polarity +- autosleep-timeout: inactivity timeout in milliseconds for autosleep. Valid + entries (ms); 4, 16, 32, 64, 128, 256, 512 and 1024 +- interrupts: interrupt number of the device, if interrupt is not via gpio +- interrupt-parent: Specifies which IRQ controller we're connected to +- irq-over-gpio: bool, true if gpio is used to get irq +- irq-gpios: gpio number over which irq will be requested (significant only if + irq-over-gpio is true) +- i2c-client-wake: Marks the input device as wakable + +stmpe-keypad: +-- +Required properties in addition to those specified by the shared matrix-keyboard +bindings: +- compatible: Must be "stmpe,keypad" Optional properties: - - interrupts : The interrupt outputs from the controller - - interrupt-controller : Marks the device node as an interrupt controller - - interrupt-parent : Specifies which IRQ controller we're connected to - - i2c-client-wake : Marks the input device as wakable - - st,autosleep-timeout : Valid entries (ms); 4, 16, 32, 64, 128, 256, 512 and 1024 +- keypad,scan-count: number of key scanning cycles to confirm key data. Maximum + is STMPE_KEYPAD_MAX_SCAN_COUNT. +- keypad,debounce-ms: debounce interval, in ms. Maximum is + STMPE_KEYPAD_MAX_DEBOUNCE. +- keypad,no-autorepeat: bool, disable key autorepeat + +stmpe-gpio: +--- +Required properties: +- compatible: Must be "stmpe,gpio" + +Optional properties: +- norequest-mask: bitmask specifying which GPIOs should _not_ be requestable due + to different usage (e.g. touch, keypad) STMPE_GPIO_NOREQ_* macros can be used + here. + +stmpe-ts: +--- +Required properties: +- compatible: Must be "stmpe,ts" + +Optional properties: +- sample-time: ADC converstion time in number of clock. (0 -> 36 clocks, 1 -> + 44 clocks, 2 -> 56 clocks, 3 -> 64 clocks, 4 -> 80 clocks, 5 -> 96 clocks, 6 + -> 144 clocks), recommended is 4. +- mod-12b: ADC Bit mode (0 -> 10bit ADC, 1 -> 12bit ADC) +- ref-sel: ADC reference source (0 -> internal reference, 1 -> external + reference) +- adc-freq: ADC Clock speed (0 -> 1.625 MHz, 1 -> 3.25 MHz, 2 || 3 -> 6.5 MHz) +- ave-ctrl: Sample average control (0 -> 1 sample, 1 -> 2 samples, 2 -> 4 + samples, 3 -> 8 samples) +- touch-det-delay: Touch detect interrupt delay (0 -> 10 us, 1 -> 50 us, 2 -> + 100 us, 3 -> 500 us, 4-> 1 ms, 5 -> 5 ms, 6 -> 10 ms, 7 -> 50 ms) recommended + is 3 +- settling: Panel driver settling time (0 -> 10 us, 1 -> 100 us, 2 -> 500 us, 3 + -> 1 ms, 4 -> 5 ms, 5 -> 10 ms, 6 for 50 ms, 7 -> 100 ms) recommended is 2 +- fraction-z: Length of the fractional part in z (fraction-z ([0..7]) = Count of + the fractional part) recommended is 7 +- i-drive: current limit value of the touchscreen drivers (0 -> 20 mA typical 35 + mA max, 1 -> 50 mA typical 80 mA max) + +stmpe-adc: +--- +Required properties: +- compatible: Must be "stmpe,adc" + +stmpe-pwm: +--- +Required properties: +- compatible: Must be "stmpe,pwm"
[PATCH V2 1/2] mfd: stmpe: Use devm_*() routines
This patch frees stmpe driver from tension of freeing resources :) devm_* derivatives of multiple routines are used while allocating resources, which would be freed automatically by kernel. Signed-off-by: Viresh Kumar --- V1->V2: -- - Rebased over latest for-next from Samuel - updated additional kzalloc with devm_kzalloc(), first one seen below. drivers/mfd/stmpe.c | 60 +++-- 1 file changed, 21 insertions(+), 39 deletions(-) diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c index ba157d4..c0df4b9 100644 --- a/drivers/mfd/stmpe.c +++ b/drivers/mfd/stmpe.c @@ -1052,17 +1052,17 @@ int __devinit stmpe_probe(struct stmpe_client_info *ci, int partnum) int ret; if (!pdata) { - if (np) { - pdata = kzalloc(sizeof(*pdata), GFP_KERNEL); - if (!pdata) - return -ENOMEM; - - stmpe_of_probe(pdata, np); - } else + if (!np) return -EINVAL; + + pdata = devm_kzalloc(ci->dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return -ENOMEM; + + stmpe_of_probe(pdata, np); } - stmpe = kzalloc(sizeof(struct stmpe), GFP_KERNEL); + stmpe = devm_kzalloc(ci->dev, sizeof(struct stmpe), GFP_KERNEL); if (!stmpe) return -ENOMEM; @@ -1084,11 +1084,12 @@ int __devinit stmpe_probe(struct stmpe_client_info *ci, int partnum) ci->init(stmpe); if (pdata->irq_over_gpio) { - ret = gpio_request_one(pdata->irq_gpio, GPIOF_DIR_IN, "stmpe"); + ret = devm_gpio_request_one(ci->dev, pdata->irq_gpio, + GPIOF_DIR_IN, "stmpe"); if (ret) { dev_err(stmpe->dev, "failed to request IRQ GPIO: %d\n", ret); - goto out_free; + return ret; } stmpe->irq = gpio_to_irq(pdata->irq_gpio); @@ -1105,48 +1106,37 @@ int __devinit stmpe_probe(struct stmpe_client_info *ci, int partnum) dev_err(stmpe->dev, "%s does not support no-irq mode!\n", stmpe->variant->name); - ret = -ENODEV; - goto free_gpio; + return -ENODEV; } stmpe->variant = stmpe_noirq_variant_info[stmpe->partnum]; } ret = stmpe_chip_init(stmpe); if (ret) - goto free_gpio; + return ret; if (stmpe->irq >= 0) { ret = stmpe_irq_init(stmpe, np); if (ret) - goto free_gpio; + return ret; - ret = request_threaded_irq(stmpe->irq, NULL, stmpe_irq, - pdata->irq_trigger | IRQF_ONESHOT, + ret = devm_request_threaded_irq(ci->dev, stmpe->irq, NULL, + stmpe_irq, pdata->irq_trigger | IRQF_ONESHOT, "stmpe", stmpe); if (ret) { dev_err(stmpe->dev, "failed to request IRQ: %d\n", ret); - goto free_gpio; + return ret; } } ret = stmpe_devices_init(stmpe); - if (ret) { - dev_err(stmpe->dev, "failed to add children\n"); - goto out_removedevs; - } - - return 0; + if (!ret) + return 0; -out_removedevs: + dev_err(stmpe->dev, "failed to add children\n"); mfd_remove_devices(stmpe->dev); - if (stmpe->irq >= 0) - free_irq(stmpe->irq, stmpe); -free_gpio: - if (pdata->irq_over_gpio) - gpio_free(pdata->irq_gpio); -out_free: - kfree(stmpe); + return ret; } @@ -1154,14 +1144,6 @@ int stmpe_remove(struct stmpe *stmpe) { mfd_remove_devices(stmpe->dev); - if (stmpe->irq >= 0) - free_irq(stmpe->irq, stmpe); - - if (stmpe->pdata->irq_over_gpio) - gpio_free(stmpe->pdata->irq_gpio); - - kfree(stmpe); - return 0; } -- 1.7.12.rc2.18.g61b472e ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 00/14] ARM: SPEAr: DT updates
On 22 November 2012 01:54, Olof Johansson wrote: > for-next is LinusW's unstable branch. If the patches have moved over into > his /devel branch we can normally pull them in as a dependency though. Do > you happen to know which patches we need for this in particular, i.e. at > what point we need to pull in the branch? I don't want to have to search > for it if I can avoid and we try to pull in minimal dependencies if > we can, instead of all his staged code. > > We're also running out of time for 3.8 staging, but if you're able to > figure out the above I'll make an attempt to apply the patches from above. Hi Olof, Last week i had chat with Arnd and Linus over Hangout, and following is the status: - Dependency patch of pinctrl is: commit 4ddb1c295752252f61670e35c791bf16e58bbce6 Author: Viresh Kumar Date: Sat Oct 27 15:21:39 2012 +0530 ARM: SPEAr: Add plgpio node in device tree dtsi files And is already in pinctrl/devel branch. And this branch is stable as per Linus. - First patch of this set is applied by Linus in his gpio tree. So, you need to get that branch too. And is present in devel branch as: commit b53bc2819a71099ecfc3d61ba0796b3dcc6be321 Author: Shiraz Hashim Date: Fri Nov 16 10:45:25 2012 +0530 gpio: SPEAr: add spi chipselect control driver -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] dt: add helper function to read u8 & u16 variables & arrays
On 20 November 2012 13:51, Shevchenko, Andriy wrote: > You could at least create macro to do a precheck if you want to. > > Like > #define CHECK_PROP(prop, sz, out) > > { > if (!prop) > return -EINVAL; > if (!prop->value) > return -ENODATA; > if ((sz * sizeof(*out)) > prop->length) > return -EOVERFLOW; > } Hi Andriy, Initially i started with a macro for the complete routine, but later as types started to play important role in it, i have to split it to routines. I thought of this idea to do prop check in a macro, but then i thought it might cover a bigger range of files and so thought of doing that separately. For, simplicity left it this time. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/2] mfd: stmpe: Use devm_*() routines
On 9 November 2012 21:01, Viresh Kumar wrote: > This patch frees stmpe driver from tension of freeing resources :) > devm_* derivatives of multiple routines are used while allocating resources, > which would be freed automatically by kernel. > > Signed-off-by: Viresh Kumar Ping!! ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH] dt: add helper function to read u8 & u16 variables & arrays
This adds following helper routines: - of_property_read_u8_array() - of_property_read_u16_array() - of_property_read_u8() - of_property_read_u16() This expects arrays from DT to be passed as: - u8 array: property = /bits/ 8 <0x50 0x60 0x70>; - u16 array: property = /bits/ 16 <0x5000 0x6000 0x7000>; Signed-off-by: Viresh Kumar --- V2->V3: - Expect u8 & u16 arrays to be passed using: /bits/ 8 or 16 - remove common macro, as not much common now :( - Tested on ARM platform. drivers/of/base.c | 77 ++ include/linux/of.h | 30 + 2 files changed, 107 insertions(+) diff --git a/drivers/of/base.c b/drivers/of/base.c index af3b22a..f564e31 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -671,12 +671,89 @@ struct device_node *of_find_node_by_phandle(phandle handle) EXPORT_SYMBOL(of_find_node_by_phandle); /** + * of_property_read_u8_array - Find and read an array of u8 from a property. + * + * @np:device node from which the property value is to be read. + * @propname: name of the property to be searched. + * @out_value: pointer to return value, modified only if return value is 0. + * @sz:number of array elements to read + * + * Search for a property in a device node and read 8-bit value(s) from + * it. Returns 0 on success, -EINVAL if the property does not exist, + * -ENODATA if property does not have a value, and -EOVERFLOW if the + * property data isn't large enough. + * + * dts entry of array should be like: + * property = /bits/ 8 <0x50 0x60 0x70>; + * + * The out_value is modified only if a valid u8 value can be decoded. + */ +int of_property_read_u8_array(const struct device_node *np, + const char *propname, u8 *out_values, size_t sz) +{ + struct property *prop = of_find_property(np, propname, NULL); + const u8 *val; + + if (!prop) + return -EINVAL; + if (!prop->value) + return -ENODATA; + if ((sz * sizeof(*out_values)) > prop->length) + return -EOVERFLOW; + + val = prop->value; + while (sz--) + *out_values++ = *val++; + return 0; +} +EXPORT_SYMBOL_GPL(of_property_read_u8_array); + +/** + * of_property_read_u16_array - Find and read an array of u16 from a property. + * + * @np:device node from which the property value is to be read. + * @propname: name of the property to be searched. + * @out_value: pointer to return value, modified only if return value is 0. + * @sz:number of array elements to read + * + * Search for a property in a device node and read 16-bit value(s) from + * it. Returns 0 on success, -EINVAL if the property does not exist, + * -ENODATA if property does not have a value, and -EOVERFLOW if the + * property data isn't large enough. + * + * dts entry of array should be like: + * property = /bits/ 16 <0x5000 0x6000 0x7000>; + * + * The out_value is modified only if a valid u16 value can be decoded. + */ +int of_property_read_u16_array(const struct device_node *np, + const char *propname, u16 *out_values, size_t sz) +{ + struct property *prop = of_find_property(np, propname, NULL); + const __be16 *val; + + if (!prop) + return -EINVAL; + if (!prop->value) + return -ENODATA; + if ((sz * sizeof(*out_values)) > prop->length) + return -EOVERFLOW; + + val = prop->value; + while (sz--) + *out_values++ = be16_to_cpup(val++); + return 0; +} +EXPORT_SYMBOL_GPL(of_property_read_u16_array); + +/** * of_property_read_u32_array - Find and read an array of 32 bit integers * from a property. * * @np:device node from which the property value is to be read. * @propname: name of the property to be searched. * @out_value: pointer to return value, modified only if return value is 0. + * @sz:number of array elements to read * * Search for a property in a device node and read 32-bit value(s) from * it. Returns 0 on success, -EINVAL if the property does not exist, diff --git a/include/linux/of.h b/include/linux/of.h index b4e50d5..bfdc130 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -223,6 +223,10 @@ extern struct device_node *of_find_node_with_property( extern struct property *of_find_property(const struct device_node *np, const char *name, int *lenp); +extern int of_property_read_u8_array(const struct device_node *np, + const char *propname, u8 *out_values, size_t sz); +extern int of_property_read_u16_array(const struct device_node *np, + const char *propname, u16 *out_values, size_t sz); extern int of_property_read_u32_array(const st
Re: [PATCH Resend V2] dt: add helper function to read u8 & u16 variables & arrays
On 19 November 2012 21:58, Stephen Warren wrote: > Support for byte- and word- properties is relatively recent I believe > (or at least, the /bits/ syntax is). Which dtc version are you using? Ok, i was on a older version. I just saw this patch now: commit cd296721a9645f9f28800a072490fa15458d1fb7 Author: Stephen Warren Date: Fri Sep 28 21:25:59 2012 + dtc: import latest upstream dtc This updates scripts/dtc to commit 317a5d9 "dtc: zero out new label objects" from git://git.jdl.com/software/dtc.git. This adds features such as: * /bits/ syntax for cell data. * Math expressions within cell data. * The ability to delete properties or nodes. * Support for #line directives in the input file, which allows the use of cpp on *.dts. * -i command-line option (/include/ path) * -W/-E command-line options for error/warning control. * Removal of spew to STDOUT containing the filename being compiled. * Many additions to the libfdt API. Signed-off-by: Stephen Warren Acked-by: Jon Loeliger Signed-off-by: Rob Herring Will try it tomorrow -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH Resend V2] dt: add helper function to read u8 & u16 variables & arrays
On 19 November 2012 12:05, Rajanikanth HV wrote: > On 19 November 2012 12:00, Viresh Kumar wrote: >> Firstly you tried square braces [ ], I am not sure if that is allowed. >> Can you point me to the specification? > http://www.devicetree.org/Device_Tree_Usage > " > a-byte-data-property = [0x01 0x23 0x34 0x56]; > " Ok, but what about 16 bit then {} :) ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH Resend V2] dt: add helper function to read u8 & u16 variables & arrays
On 19 November 2012 11:54, Rajanikanth HV wrote: >> data1 = /bits/ 8 <0x50 0x60 0x70>; > as per spec, format for data byte defines will be: > data1 = [ 0x50 0x60 0x70 ]; > however, i see a parse error from device tree compiler when i tried. Firstly you tried square braces [ ], I am not sure if that is allowed. Can you point me to the specification? And simply passing 0x50, 0x60 etc.. will make them u32 instead of u8. i.e. they will be stored as 0x0050, etc. -- viresh ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH Resend V2] dt: add helper function to read u8 & u16 variables & arrays
On 12 November 2012 09:03, Viresh Kumar wrote: > On 12 November 2012 01:12, Rob Herring wrote: >> I don't think the size is stored in the dtb. It is only in the dts. You >> need to define the size in the binding definitions and use '/bits/' >> annotation. With this the data is packed. Then the array function used >> should match what the binding defines. Hi Rob, Can you please help me in getting the correct format to do this from dts. I tried: data1 = /bits/ 8 <0x50 0x60 0x70>; as written in your earlier mail... but it didn't worked. Tried to search too, but couldn't find it. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss