* [PATCH 0/5] arm: dts: lpc32xx: fixes and updates to lpc32xx.dtsi
@ 2015-10-12 23:54 Vladimir Zapolskiy
2015-10-12 23:54 ` [PATCH 1/5] arm: dts: lpc32xx: change include syntax to be C preprocessor friendly Vladimir Zapolskiy
` (4 more replies)
0 siblings, 5 replies; 21+ messages in thread
From: Vladimir Zapolskiy @ 2015-10-12 23:54 UTC (permalink / raw)
To: linux-arm-kernel
The changeset fixes two problems in lpc32xx.dtsi:
- misusage of ranges property,
- improper description of two separate PWM controllers under one
device node
and adds a number of lesser clean-ups and improvements.
Vladimir Zapolskiy (5):
arm: dts: lpc32xx: change include syntax to be C preprocessor friendly
arm: dts: lpc32xx: fix improper usage of ranges property
arm: dts: lpc32xx: add labels to all defined peripheral nodes
arm: dts: lpc32xx: remove unneeded cell settings from cpus
arm: dts: lpc32xx: add device node for the second pwm controller
arch/arm/boot/dts/ea3250.dts | 2 +-
arch/arm/boot/dts/lpc32xx.dtsi | 41 ++++++++++++++++++++++-------------------
arch/arm/boot/dts/phy3250.dts | 2 +-
3 files changed, 24 insertions(+), 21 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH 1/5] arm: dts: lpc32xx: change include syntax to be C preprocessor friendly 2015-10-12 23:54 [PATCH 0/5] arm: dts: lpc32xx: fixes and updates to lpc32xx.dtsi Vladimir Zapolskiy @ 2015-10-12 23:54 ` Vladimir Zapolskiy 2015-10-12 23:54 ` [PATCH 2/5] arm: dts: lpc32xx: fix improper usage of ranges property Vladimir Zapolskiy ` (3 subsequent siblings) 4 siblings, 0 replies; 21+ messages in thread From: Vladimir Zapolskiy @ 2015-10-12 23:54 UTC (permalink / raw) To: linux-arm-kernel The change replaces /include/ to #include in lpc32xx.dtsi and derivatives, it is required, if C preprocessor is intended to be used over dtsi/dts files, otherwise errors like one below are generated: Error: ea3250.dts:15.1-9 syntax error FATAL ERROR: Unable to parse input tree Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> --- arch/arm/boot/dts/ea3250.dts | 2 +- arch/arm/boot/dts/lpc32xx.dtsi | 2 +- arch/arm/boot/dts/phy3250.dts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm/boot/dts/ea3250.dts b/arch/arm/boot/dts/ea3250.dts index a4ba31b..121d032 100644 --- a/arch/arm/boot/dts/ea3250.dts +++ b/arch/arm/boot/dts/ea3250.dts @@ -12,7 +12,7 @@ */ /dts-v1/; -/include/ "lpc32xx.dtsi" +#include "lpc32xx.dtsi" / { model = "Embedded Artists LPC3250 board based on NXP LPC3250"; diff --git a/arch/arm/boot/dts/lpc32xx.dtsi b/arch/arm/boot/dts/lpc32xx.dtsi index 3abebb7..f35e982 100644 --- a/arch/arm/boot/dts/lpc32xx.dtsi +++ b/arch/arm/boot/dts/lpc32xx.dtsi @@ -11,7 +11,7 @@ * http://www.gnu.org/copyleft/gpl.html */ -/include/ "skeleton.dtsi" +#include "skeleton.dtsi" / { compatible = "nxp,lpc3220"; diff --git a/arch/arm/boot/dts/phy3250.dts b/arch/arm/boot/dts/phy3250.dts index 90fdbd7..2a2d2cf 100644 --- a/arch/arm/boot/dts/phy3250.dts +++ b/arch/arm/boot/dts/phy3250.dts @@ -12,7 +12,7 @@ */ /dts-v1/; -/include/ "lpc32xx.dtsi" +#include "lpc32xx.dtsi" / { model = "PHYTEC phyCORE-LPC3250 board based on NXP LPC3250"; -- 2.1.4 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/5] arm: dts: lpc32xx: fix improper usage of ranges property 2015-10-12 23:54 [PATCH 0/5] arm: dts: lpc32xx: fixes and updates to lpc32xx.dtsi Vladimir Zapolskiy 2015-10-12 23:54 ` [PATCH 1/5] arm: dts: lpc32xx: change include syntax to be C preprocessor friendly Vladimir Zapolskiy @ 2015-10-12 23:54 ` Vladimir Zapolskiy 2015-10-13 12:44 ` Arnd Bergmann 2015-10-12 23:54 ` [PATCH 3/5] arm: dts: lpc32xx: add labels to all defined peripheral nodes Vladimir Zapolskiy ` (2 subsequent siblings) 4 siblings, 1 reply; 21+ messages in thread From: Vladimir Zapolskiy @ 2015-10-12 23:54 UTC (permalink / raw) To: linux-arm-kernel The change corrects invalid custom translations to 1:1 translations, otherwise during initialization there are too many failed attempts to translate addresses from subnodes, which anyway result in fallback 1:1 translations, also it is found that due to this problem proper usage of ranges property in subnodes (e.g. for defining flash partitions) can not be done. Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> --- arch/arm/boot/dts/lpc32xx.dtsi | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm/boot/dts/lpc32xx.dtsi b/arch/arm/boot/dts/lpc32xx.dtsi index f35e982..3ef804c 100644 --- a/arch/arm/boot/dts/lpc32xx.dtsi +++ b/arch/arm/boot/dts/lpc32xx.dtsi @@ -31,7 +31,7 @@ #address-cells = <1>; #size-cells = <1>; compatible = "simple-bus"; - ranges = <0x20000000 0x20000000 0x30000000>; + ranges; /* * Enable either SLC or MLC @@ -89,7 +89,7 @@ #address-cells = <1>; #size-cells = <1>; compatible = "simple-bus"; - ranges = <0x20000000 0x20000000 0x30000000>; + ranges; ssp0: ssp at 20084000 { compatible = "arm,pl022", "arm,primecell"; @@ -207,7 +207,7 @@ #address-cells = <1>; #size-cells = <1>; compatible = "simple-bus"; - ranges = <0x20000000 0x20000000 0x30000000>; + ranges; /* * MIC Interrupt controller includes: -- 2.1.4 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/5] arm: dts: lpc32xx: fix improper usage of ranges property 2015-10-12 23:54 ` [PATCH 2/5] arm: dts: lpc32xx: fix improper usage of ranges property Vladimir Zapolskiy @ 2015-10-13 12:44 ` Arnd Bergmann 2015-10-13 15:51 ` Vladimir Zapolskiy 0 siblings, 1 reply; 21+ messages in thread From: Arnd Bergmann @ 2015-10-13 12:44 UTC (permalink / raw) To: linux-arm-kernel On Tuesday 13 October 2015 02:54:02 Vladimir Zapolskiy wrote: > The change corrects invalid custom translations to 1:1 translations, > otherwise during initialization there are too many failed attempts to > translate addresses from subnodes, which anyway result in fallback 1:1 > translations, also it is found that due to this problem proper usage > of ranges property in subnodes (e.g. for defining flash partitions) > can not be done. > > Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> > I don't get it. What kind of errors do you see? The existing version looks cleaner than the new one, as it only translates the MMIO areas that are actually used. Arnd ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/5] arm: dts: lpc32xx: fix improper usage of ranges property 2015-10-13 12:44 ` Arnd Bergmann @ 2015-10-13 15:51 ` Vladimir Zapolskiy 2015-10-13 19:36 ` Arnd Bergmann 0 siblings, 1 reply; 21+ messages in thread From: Vladimir Zapolskiy @ 2015-10-13 15:51 UTC (permalink / raw) To: linux-arm-kernel Arnd, On 13.10.2015 15:44, Arnd Bergmann wrote: > On Tuesday 13 October 2015 02:54:02 Vladimir Zapolskiy wrote: >> The change corrects invalid custom translations to 1:1 translations, >> otherwise during initialization there are too many failed attempts to >> translate addresses from subnodes, which anyway result in fallback 1:1 >> translations, also it is found that due to this problem proper usage >> of ranges property in subnodes (e.g. for defining flash partitions) >> can not be done. >> >> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> >> > adding Joachim to Cc list as the author of PL172 memory controller driver and related DT bindings, may be he has something to add. > I don't get it. What kind of errors do you see? The existing > version looks cleaner than the new one, as it only translates > the MMIO areas that are actually used. > The problem is found when I add PL175 device node to lpc32xx.dtsi and expand the node in my board file. The external memory controller manages up to 4 memory devices, which by means of the controller are mapped into physical memory starting from address 0xe0000000. Below are my declarations, similar to one found in lpc18xx.dtsi. lpc32xx.dtsi change common for all LPC32xx boards: emc: emc at 31080000 { compatible = "arm,pl175", "arm,primecell"; reg = <0x31080000 0x1000>; clocks = <&scf LPC32XX_CLK_DDRAM>; clock-names = "mpmcclk"; #address-cells = <1>; #size-cells = <1>; ranges = <0 0xe0000000 0x01000000>, <1 0xe1000000 0x01000000>, <2 0xe2000000 0x01000000>, <3 0xe3000000 0x01000000>; status = "disabled"; }; myboard.dts declaration: &emc { status = "okay"; cs at 0 { #address-cells = <1>; #size-cells = <1>; ranges; mpmc,cs = <0>; mpmc,memory-width = <16>; mpmc,byte-lane-low; mpmc,write-enable-delay = <0>; mpmc,output-enable-delay = <0>; mpmc,read-access-delay = <55>; mpmc,page-mode-read-delay = <55>; mpmc,write-access-delay = <22>; mpmc,turn-round-delay = <8>; nor at 0 { #address-cells = <1>; #size-cells = <1>; compatible = "spansion,s29gl032a", "cfi-flash"; reg = <0 0x00400000>; bank-width = <2>; }; }; }; The problem with non-empty ranges is that NOR device is not mapped to 0xe0000000, because da > (cp + s): OF: ** translation for device /ahb/emc at 31080000/cs at 0/nor at 0 ** OF: bus is default (na=1, ns=1) on /ahb/emc at 31080000/cs at 0 OF: translating address: 00000000 OF: parent bus is default (na=1, ns=1) on /ahb/emc at 31080000 OF: empty ranges; 1:1 translation OF: parent translation for: 00000000 OF: with offset: 0 OF: one level translation: 00000000 OF: parent bus is default (na=1, ns=1) on /ahb OF: walking ranges... OF: default map, cp=0, s=1000000, da=0 OF: parent translation for: e0000000 OF: with offset: 0 OF: one level translation: e0000000 OF: parent bus is default (na=1, ns=1) on / OF: walking ranges... OF: default map, cp=20000000, s=30000000, da=e0000000 OF: not found ! This proposed 2/5 change fixes the problem for me: OF: ** translation for device /ahb/emc at 31080000/cs at 0/nor at 0 ** OF: bus is default (na=1, ns=1) on /ahb/emc at 31080000/cs at 0 OF: translating address: 00000000 OF: parent bus is default (na=1, ns=1) on /ahb/emc at 31080000 OF: empty ranges; 1:1 translation OF: parent translation for: 00000000 OF: with offset: 0 OF: one level translation: 00000000 OF: parent bus is default (na=1, ns=1) on /ahb OF: walking ranges... OF: default map, cp=0, s=1000000, da=0 OF: parent translation for: e0000000 OF: with offset: 0 OF: one level translation: e0000000 OF: parent bus is default (na=1, ns=1) on / OF: empty ranges; 1:1 translation OF: parent translation for: 00000000 OF: with offset: e0000000 OF: one level translation: e0000000 OF: reached root node e0000000.nor: Found 1 x16 devices at 0x0 in 16-bit bank. Manufacturer ID 0x000001 Chip ID 0x001a00 Also note that the change does not break any other translations, moreover boot time delay is slightly faster. Original address translation for a device: [ 0.259969] OF: ** translation for device /ahb/fab/watchdog at 4003C000 ** [ 0.260050] OF: bus is default (na=1, ns=1) on /ahb/fab [ 0.260084] OF: translating address: 4003c000 [ 0.260156] OF: parent bus is default (na=1, ns=1) on /ahb [ 0.260196] OF: walking ranges... [ 0.260248] OF: default map, cp=20000000, s=30000000, da=4003c000 [ 0.260282] OF: parent translation for: 20000000 [ 0.260333] OF: with offset: 2003c000 [ 0.260364] OF: one level translation: 4003c000 [ 0.260431] OF: parent bus is default (na=1, ns=1) on / [ 0.260468] OF: walking ranges... [ 0.260518] OF: default map, cp=20000000, s=30000000, da=4003c000 [ 0.260550] OF: parent translation for: 20000000 [ 0.260600] OF: with offset: 2003c000 [ 0.260630] OF: one level translation: 4003c000 [ 0.260676] OF: reached root node With this proposed change applied address translation looks simpler: [ 0.253194] OF: ** translation for device /ahb/fab/watchdog at 4003C000 ** [ 0.253281] OF: bus is default (na=1, ns=1) on /ahb/fab [ 0.253314] OF: translating address: 4003c000 [ 0.253386] OF: parent bus is default (na=1, ns=1) on /ahb [ 0.253426] OF: empty ranges; 1:1 translation [ 0.253457] OF: parent translation for: 00000000 [ 0.253506] OF: with offset: 4003c000 [ 0.253537] OF: one level translation: 4003c000 [ 0.253601] OF: parent bus is default (na=1, ns=1) on / [ 0.253639] OF: empty ranges; 1:1 translation [ 0.253669] OF: parent translation for: 00000000 [ 0.253717] OF: with offset: 4003c000 [ 0.253747] OF: one level translation: 4003c000 [ 0.253793] OF: reached root node Support of EMC device node will be added to lpc32xx.dtsi, when this kind of problem is fixed. -- With best wishes, Vladimir ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/5] arm: dts: lpc32xx: fix improper usage of ranges property 2015-10-13 15:51 ` Vladimir Zapolskiy @ 2015-10-13 19:36 ` Arnd Bergmann 2015-10-13 23:13 ` Vladimir Zapolskiy 0 siblings, 1 reply; 21+ messages in thread From: Arnd Bergmann @ 2015-10-13 19:36 UTC (permalink / raw) To: linux-arm-kernel On Tuesday 13 October 2015 18:51:24 Vladimir Zapolskiy wrote: > On 13.10.2015 15:44, Arnd Bergmann wrote: > > I don't get it. What kind of errors do you see? The existing > > version looks cleaner than the new one, as it only translates > > the MMIO areas that are actually used. > > > > The problem is found when I add PL175 device node to lpc32xx.dtsi and > expand the node in my board file. The external memory controller manages > up to 4 memory devices, which by means of the controller are mapped into > physical memory starting from address 0xe0000000. > > Below are my declarations, similar to one found in lpc18xx.dtsi. > > lpc32xx.dtsi change common for all LPC32xx boards: > > emc: emc at 31080000 { > compatible = "arm,pl175", "arm,primecell"; > reg = <0x31080000 0x1000>; > clocks = <&scf LPC32XX_CLK_DDRAM>; > clock-names = "mpmcclk"; > > #address-cells = <1>; > #size-cells = <1>; > ranges = <0 0xe0000000 0x01000000>, > <1 0xe1000000 0x01000000>, > <2 0xe2000000 0x01000000>, > <3 0xe3000000 0x01000000>; > status = "disabled"; > }; > Ok, got it. > Support of EMC device node will be added to lpc32xx.dtsi, when this kind > of problem is fixed. As I see it, the problem is that you now have ranges that are not for devices inside of the device but that are external. As the memory-controller node needs both its own registers and the translation for the external ones, we unfortunately can't just put it in the root node, which would avoid the issue. Instead, I would suggest adding a range for the 0xe0000000 area. Arnd ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/5] arm: dts: lpc32xx: fix improper usage of ranges property 2015-10-13 19:36 ` Arnd Bergmann @ 2015-10-13 23:13 ` Vladimir Zapolskiy 2015-10-14 13:52 ` Arnd Bergmann 0 siblings, 1 reply; 21+ messages in thread From: Vladimir Zapolskiy @ 2015-10-13 23:13 UTC (permalink / raw) To: linux-arm-kernel Arnd, On 13.10.2015 22:36, Arnd Bergmann wrote: > On Tuesday 13 October 2015 18:51:24 Vladimir Zapolskiy wrote: >> On 13.10.2015 15:44, Arnd Bergmann wrote: >>> I don't get it. What kind of errors do you see? The existing >>> version looks cleaner than the new one, as it only translates >>> the MMIO areas that are actually used. >>> >> >> The problem is found when I add PL175 device node to lpc32xx.dtsi and >> expand the node in my board file. The external memory controller manages >> up to 4 memory devices, which by means of the controller are mapped into >> physical memory starting from address 0xe0000000. >> >> Below are my declarations, similar to one found in lpc18xx.dtsi. >> >> lpc32xx.dtsi change common for all LPC32xx boards: >> >> emc: emc at 31080000 { >> compatible = "arm,pl175", "arm,primecell"; >> reg = <0x31080000 0x1000>; >> clocks = <&scf LPC32XX_CLK_DDRAM>; >> clock-names = "mpmcclk"; >> >> #address-cells = <1>; >> #size-cells = <1>; >> ranges = <0 0xe0000000 0x01000000>, >> <1 0xe1000000 0x01000000>, >> <2 0xe2000000 0x01000000>, >> <3 0xe3000000 0x01000000>; >> status = "disabled"; >> }; >> > > Ok, got it. > >> Support of EMC device node will be added to lpc32xx.dtsi, when this kind >> of problem is fixed. > > As I see it, the problem is that you now have ranges that are not > for devices inside of the device but that are external. As the > memory-controller node needs both its own registers and the translation > for the external ones, we unfortunately can't just put it in the root > node, which would avoid the issue. > > Instead, I would suggest adding a range for the 0xe0000000 area. Ok, practically it should work for my purposes, but the change must be done along with added EMC device node description. I'm not so confident that it is correct to add description of static memory banks to ahb node though, please give me a short confirmation, then I'll send a change for inclusion of EMC description -- the one above excluding clocks and clock-names properties, work on CCF is in progress. -- With best wishes, Vladimir ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/5] arm: dts: lpc32xx: fix improper usage of ranges property 2015-10-13 23:13 ` Vladimir Zapolskiy @ 2015-10-14 13:52 ` Arnd Bergmann 2015-10-14 14:07 ` Vladimir Zapolskiy 2015-10-14 17:23 ` Joachim Eastwood 0 siblings, 2 replies; 21+ messages in thread From: Arnd Bergmann @ 2015-10-14 13:52 UTC (permalink / raw) To: linux-arm-kernel On Wednesday 14 October 2015 02:13:49 Vladimir Zapolskiy wrote: > Ok, practically it should work for my purposes, but the change must be > done along with added EMC device node description. > > I'm not so confident that it is correct to add description of static > memory banks to ahb node though, please give me a short confirmation, The DT should reflect whichever memory ranges are accessible on the bus. I've looked up the memory map in the data sheet, and I think a reasonable representation would be /ahb { ranges = <0x20000000 0x20000000 0x10000000>, /* AHB port 5 */ <0x30000000 0x30000000 0x10000000>, /* AHB port 6 */ <0x40000000 0x40000000 0x10000000>, /* AHB port 7 */ <0x80000000 0x80000000 0x40000000>, /* EMC DYCS0/1 */ <0xE0000000 0xE0000000 0x04000000>; /* EMC CS0-3 */ apb { ranges = <0x20080000 0x20080000 0x00020000>; }; }; alternatively, each AHB port could be a separate node, with a more direct translation like /ahb5 { ranges = <0 0x20000000 0x10000000>; apb { ranges = <0 0x80000 0x20000>; }; }; /ahb6 { ranges = <0 0x30000000 0x10000000>, /* AHB registers */ <0x80000000 0x80000000 0x40000000>, /* EMC DYCS0/1 */ <0xE0000000 0xE0000000 0x04000000>; /* EMC CS0-3 */ memory-controller at 1080000 { reg = <0x1080000 0x10000>; }; }; ... Does this make sense? > then I'll send a change for inclusion of EMC description -- the one > above excluding clocks and clock-names properties, work on CCF is in > progress. Ah, very nice! That should get us very close to multiplatform support! I've just tried building lpc32xx without the headers to check for other issues, and ended up with the patch below to get it building. It's clearly wrong, but it highlights a number of issues. Arnd arch/arm/Kconfig | 2 ++ arch/arm/mach-lpc32xx/Makefile | 2 +- arch/arm/mach-lpc32xx/{include/mach => }/board.h | 0 arch/arm/mach-lpc32xx/clock.c | 4 +-- arch/arm/mach-lpc32xx/common.c | 4 +-- arch/arm/mach-lpc32xx/common.h | 2 +- .../gpio-lpc32xx.c => arch/arm/mach-lpc32xx/gpio.c | 6 ++-- .../arm/mach-lpc32xx/{include/mach => }/hardware.h | 0 arch/arm/mach-lpc32xx/include/mach/entry-macro.S | 37 ---------------------- arch/arm/mach-lpc32xx/include/mach/uncompress.h | 4 +-- arch/arm/mach-lpc32xx/irq.c | 24 +++++++++++--- arch/arm/mach-lpc32xx/{include/mach => }/irqs.h | 2 +- arch/arm/mach-lpc32xx/phy3250.c | 8 +++-- .../arm/mach-lpc32xx/{include/mach => }/platform.h | 0 arch/arm/mach-lpc32xx/pm.c | 4 +-- arch/arm/mach-lpc32xx/serial.c | 4 +-- arch/arm/mach-lpc32xx/suspend.S | 4 +-- arch/arm/mach-lpc32xx/timer.c | 5 +-- drivers/gpio/Makefile | 1 - drivers/net/ethernet/nxp/lpc_eth.c | 14 ++++++-- drivers/tty/serial/lpc32xx_hs.c | 8 +++-- drivers/usb/gadget/udc/lpc32xx_udc.c | 16 +++++++--- drivers/usb/host/ohci-nxp.c | 18 ++++++++--- drivers/watchdog/pnx4008_wdt.c | 1 - 24 files changed, 88 insertions(+), 82 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index fedbe63e4380..e5fa01ae2bf6 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -499,6 +499,8 @@ config ARCH_LPC32XX select CPU_ARM926T select GENERIC_CLOCKEVENTS select HAVE_IDE + select MULTI_IRQ_HANDLER + select SPARSE_IRQ select USE_OF help Support for the NXP LPC32XX family of processors diff --git a/arch/arm/mach-lpc32xx/Makefile b/arch/arm/mach-lpc32xx/Makefile index f5db805ab958..bd86fd4c3e5b 100644 --- a/arch/arm/mach-lpc32xx/Makefile +++ b/arch/arm/mach-lpc32xx/Makefile @@ -2,7 +2,7 @@ # Makefile for the linux kernel. # -obj-y := timer.o irq.o common.o serial.o clock.o +obj-y := timer.o irq.o common.o serial.o clock.o gpio.o obj-y += pm.o suspend.o obj-y += phy3250.o diff --git a/arch/arm/mach-lpc32xx/include/mach/board.h b/arch/arm/mach-lpc32xx/board.h similarity index 100% rename from arch/arm/mach-lpc32xx/include/mach/board.h rename to arch/arm/mach-lpc32xx/board.h diff --git a/arch/arm/mach-lpc32xx/clock.c b/arch/arm/mach-lpc32xx/clock.c index 661c8f4b2310..595980cafdce 100644 --- a/arch/arm/mach-lpc32xx/clock.c +++ b/arch/arm/mach-lpc32xx/clock.c @@ -94,8 +94,8 @@ #include <linux/amba/clcd.h> #include <linux/clkdev.h> -#include <mach/hardware.h> -#include <mach/platform.h> +#include "hardware.h" +#include "platform.h" #include "clock.h" #include "common.h" diff --git a/arch/arm/mach-lpc32xx/common.c b/arch/arm/mach-lpc32xx/common.c index 716e83eb1db8..e75cfed33d5e 100644 --- a/arch/arm/mach-lpc32xx/common.c +++ b/arch/arm/mach-lpc32xx/common.c @@ -28,8 +28,8 @@ #include <asm/mach/map.h> #include <asm/system_info.h> -#include <mach/hardware.h> -#include <mach/platform.h> +#include "hardware.h" +#include "platform.h" #include "common.h" /* diff --git a/arch/arm/mach-lpc32xx/common.h b/arch/arm/mach-lpc32xx/common.h index 1cd8853b2f9b..21a56653c5f6 100644 --- a/arch/arm/mach-lpc32xx/common.h +++ b/arch/arm/mach-lpc32xx/common.h @@ -19,7 +19,7 @@ #ifndef __LPC32XX_COMMON_H #define __LPC32XX_COMMON_H -#include <mach/board.h> +#include "board.h" #include <linux/platform_device.h> #include <linux/reboot.h> diff --git a/drivers/gpio/gpio-lpc32xx.c b/arch/arm/mach-lpc32xx/gpio.c similarity index 99% rename from drivers/gpio/gpio-lpc32xx.c rename to arch/arm/mach-lpc32xx/gpio.c index 47e2dde63734..8987b1af0044 100644 --- a/drivers/gpio/gpio-lpc32xx.c +++ b/arch/arm/mach-lpc32xx/gpio.c @@ -27,9 +27,9 @@ #include <linux/module.h> #include <linux/platform_data/gpio-lpc32xx.h> -#include <mach/hardware.h> -#include <mach/platform.h> -#include <mach/irqs.h> +#include "hardware.h" +#include "platform.h" +#include "irqs.h" #define LPC32XX_GPIO_P3_INP_STATE _GPREG(0x000) #define LPC32XX_GPIO_P3_OUTP_SET _GPREG(0x004) diff --git a/arch/arm/mach-lpc32xx/include/mach/hardware.h b/arch/arm/mach-lpc32xx/hardware.h similarity index 100% rename from arch/arm/mach-lpc32xx/include/mach/hardware.h rename to arch/arm/mach-lpc32xx/hardware.h diff --git a/arch/arm/mach-lpc32xx/include/mach/entry-macro.S b/arch/arm/mach-lpc32xx/include/mach/entry-macro.S deleted file mode 100644 index 24ca11b377c8..000000000000 --- a/arch/arm/mach-lpc32xx/include/mach/entry-macro.S +++ /dev/null @@ -1,37 +0,0 @@ -/* - * arch/arm/mach-lpc32xx/include/mach/entry-macro.S - * - * Author: Kevin Wells <kevin.wells@nxp.com> - * - * Copyright (C) 2010 NXP Semiconductors - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -#include <mach/hardware.h> -#include <mach/platform.h> - -#define LPC32XX_INTC_MASKED_STATUS_OFS 0x8 - - .macro get_irqnr_preamble, base, tmp - ldr \base, =IO_ADDRESS(LPC32XX_MIC_BASE) - .endm - -/* - * Return IRQ number in irqnr. Also return processor Z flag status in CPSR - * as set if an interrupt is pending. - */ - .macro get_irqnr_and_base, irqnr, irqstat, base, tmp - ldr \irqstat, [\base, #LPC32XX_INTC_MASKED_STATUS_OFS] - clz \irqnr, \irqstat - rsb \irqnr, \irqnr, #31 - teq \irqstat, #0 - .endm diff --git a/arch/arm/mach-lpc32xx/include/mach/uncompress.h b/arch/arm/mach-lpc32xx/include/mach/uncompress.h index 1198a89183cd..6e4d072c6f66 100644 --- a/arch/arm/mach-lpc32xx/include/mach/uncompress.h +++ b/arch/arm/mach-lpc32xx/include/mach/uncompress.h @@ -21,8 +21,8 @@ #include <linux/io.h> -#include <mach/hardware.h> -#include <mach/platform.h> +#define LPC32XX_UART5_BASE 0x40090000 + /* * Uncompress output is hardcoded to standard UART 5 diff --git a/arch/arm/mach-lpc32xx/irq.c b/arch/arm/mach-lpc32xx/irq.c index 2ae431e8bc1b..4c35196c7784 100644 --- a/arch/arm/mach-lpc32xx/irq.c +++ b/arch/arm/mach-lpc32xx/irq.c @@ -28,9 +28,11 @@ #include <linux/irqdomain.h> #include <linux/module.h> -#include <mach/irqs.h> -#include <mach/hardware.h> -#include <mach/platform.h> +#include <asm/exception.h> + +#include "irqs.h" +#include "hardware.h" +#include "platform.h" #include "common.h" /* @@ -81,7 +83,7 @@ struct lpc32xx_event_info { /* * Maps an IRQ number to and event mask and register */ -static const struct lpc32xx_event_info lpc32xx_events[NR_IRQS] = { +static const struct lpc32xx_event_info lpc32xx_events[LPC32XX_LEGACY_IRQS] = { [IRQ_LPC32XX_GPI_08] = { .event_group = &lpc32xx_event_pin_regs, .mask = LPC32XX_CLKPWR_EXTSRC_GPI_08_BIT, @@ -370,6 +372,19 @@ static struct irq_chip lpc32xx_irq_chip = { .irq_set_wake = lpc32xx_irq_wake }; +static void __exception_irq_entry lpc32xx_mic_handler(struct pt_regs *regs) +{ + unsigned long ints = __raw_readl(LPC32XX_INTC_STAT(LPC32XX_MIC_BASE)); + + while (ints) { + int irqno = fls(ints) - 1; + + ints &= ~(1 << irqno); + + handle_IRQ(irqno, regs); + } +} + static void lpc32xx_sic1_handler(struct irq_desc *desc) { unsigned long ints = __raw_readl(LPC32XX_INTC_STAT(LPC32XX_SIC1_BASE)); @@ -400,6 +415,7 @@ static int __init __lpc32xx_mic_of_init(struct device_node *node, struct device_node *parent) { lpc32xx_mic_np = node; + set_handle_irq(lpc32xx_mic_handler); return 0; } diff --git a/arch/arm/mach-lpc32xx/include/mach/irqs.h b/arch/arm/mach-lpc32xx/irqs.h similarity index 99% rename from arch/arm/mach-lpc32xx/include/mach/irqs.h rename to arch/arm/mach-lpc32xx/irqs.h index 9e3b90df32e1..006bd71dcbb3 100644 --- a/arch/arm/mach-lpc32xx/include/mach/irqs.h +++ b/arch/arm/mach-lpc32xx/irqs.h @@ -112,6 +112,6 @@ #define IRQ_LPC32XX_GPI_06 LPC32XX_SIC2_IRQ(28) #define IRQ_LPC32XX_SYSCLK LPC32XX_SIC2_IRQ(31) -#define NR_IRQS 96 +#define LPC32XX_LEGACY_IRQS 96 #endif diff --git a/arch/arm/mach-lpc32xx/phy3250.c b/arch/arm/mach-lpc32xx/phy3250.c index 77d6b1bab278..25b5548a6136 100644 --- a/arch/arm/mach-lpc32xx/phy3250.c +++ b/arch/arm/mach-lpc32xx/phy3250.c @@ -42,10 +42,11 @@ #include <asm/mach-types.h> #include <asm/mach/arch.h> -#include <mach/hardware.h> -#include <mach/platform.h> -#include <mach/board.h> +#include "hardware.h" +#include "platform.h" +#include "board.h" #include "common.h" +#include "irqs.h" /* * Mapped GPIOLIB GPIOs @@ -258,6 +259,7 @@ static const char *const lpc32xx_dt_compat[] __initconst = { DT_MACHINE_START(LPC32XX_DT, "LPC32XX SoC (Flattened Device Tree)") .atag_offset = 0x100, + .nr_irqs = LPC32XX_LEGACY_IRQS, .map_io = lpc32xx_map_io, .init_irq = lpc32xx_init_irq, .init_time = lpc32xx_timer_init, diff --git a/arch/arm/mach-lpc32xx/include/mach/platform.h b/arch/arm/mach-lpc32xx/platform.h similarity index 100% rename from arch/arm/mach-lpc32xx/include/mach/platform.h rename to arch/arm/mach-lpc32xx/platform.h diff --git a/arch/arm/mach-lpc32xx/pm.c b/arch/arm/mach-lpc32xx/pm.c index 207e81275ff0..b50faab808bc 100644 --- a/arch/arm/mach-lpc32xx/pm.c +++ b/arch/arm/mach-lpc32xx/pm.c @@ -70,8 +70,8 @@ #include <asm/cacheflush.h> -#include <mach/hardware.h> -#include <mach/platform.h> +#include "hardware.h" +#include "platform.h" #include "common.h" #include "clock.h" diff --git a/arch/arm/mach-lpc32xx/serial.c b/arch/arm/mach-lpc32xx/serial.c index 05621a29fba2..065d6a82b5a2 100644 --- a/arch/arm/mach-lpc32xx/serial.c +++ b/arch/arm/mach-lpc32xx/serial.c @@ -25,8 +25,8 @@ #include <linux/clk.h> #include <linux/io.h> -#include <mach/hardware.h> -#include <mach/platform.h> +#include "hardware.h" +#include "platform.h" #include "common.h" #define LPC32XX_SUART_FIFO_SIZE 64 diff --git a/arch/arm/mach-lpc32xx/suspend.S b/arch/arm/mach-lpc32xx/suspend.S index 374f9f07fe48..d7f8800d0087 100644 --- a/arch/arm/mach-lpc32xx/suspend.S +++ b/arch/arm/mach-lpc32xx/suspend.S @@ -11,8 +11,8 @@ */ #include <linux/linkage.h> #include <asm/assembler.h> -#include <mach/platform.h> -#include <mach/hardware.h> +#include "platform.h" +#include "hardware.h" /* Using named register defines makes the code easier to follow */ #define WORK1_REG r0 diff --git a/arch/arm/mach-lpc32xx/timer.c b/arch/arm/mach-lpc32xx/timer.c index ff3499d1fb1a..e6a27af37122 100644 --- a/arch/arm/mach-lpc32xx/timer.c +++ b/arch/arm/mach-lpc32xx/timer.c @@ -27,9 +27,10 @@ #include <asm/mach/time.h> -#include <mach/hardware.h> -#include <mach/platform.h> +#include "hardware.h" +#include "platform.h" #include "common.h" +#include "irqs.h" static int lpc32xx_clkevt_next_event(unsigned long delta, struct clock_event_device *dev) diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index 11fcf2cc4e92..f31e982a578f 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -48,7 +48,6 @@ obj-$(CONFIG_GPIO_INTEL_MID) += gpio-intel-mid.o obj-$(CONFIG_GPIO_LOONGSON) += gpio-loongson.o obj-$(CONFIG_GPIO_LP3943) += gpio-lp3943.o obj-$(CONFIG_GPIO_LPC18XX) += gpio-lpc18xx.o -obj-$(CONFIG_ARCH_LPC32XX) += gpio-lpc32xx.o obj-$(CONFIG_GPIO_LYNXPOINT) += gpio-lynxpoint.o obj-$(CONFIG_GPIO_MAX730X) += gpio-max730x.o obj-$(CONFIG_GPIO_MAX7300) += gpio-max7300.o diff --git a/drivers/net/ethernet/nxp/lpc_eth.c b/drivers/net/ethernet/nxp/lpc_eth.c index b159ef8303cc..a157826600d3 100644 --- a/drivers/net/ethernet/nxp/lpc_eth.c +++ b/drivers/net/ethernet/nxp/lpc_eth.c @@ -44,9 +44,6 @@ #include <linux/types.h> #include <linux/io.h> -#include <mach/board.h> -#include <mach/platform.h> -#include <mach/hardware.h> #define MODNAME "lpc-eth" #define DRV_VERSION "1.00" @@ -1312,9 +1309,12 @@ static int lpc_eth_drv_probe(struct platform_device *pdev) struct phy_device *phydev; dma_addr_t dma_handle; int irq, ret; +#if 0 u32 tmp; /* Setup network interface for RMII or MII mode */ + FIXME: use pinctrl + tmp = __raw_readl(LPC32XX_CLKPWR_MACCLK_CTRL); tmp &= ~LPC32XX_CLKPWR_MACCTRL_PINS_MSK; if (lpc_phy_interface_mode(&pdev->dev) == PHY_INTERFACE_MODE_MII) @@ -1322,6 +1322,7 @@ static int lpc_eth_drv_probe(struct platform_device *pdev) else tmp |= LPC32XX_CLKPWR_MACCTRL_USE_RMII_PINS; __raw_writel(tmp, LPC32XX_CLKPWR_MACCLK_CTRL); +#endif /* Get platform resources */ res = platform_get_resource(pdev, IORESOURCE_MEM, 0); @@ -1387,6 +1388,8 @@ static int lpc_eth_drv_probe(struct platform_device *pdev) pldat->dma_buff_base_v = 0; if (use_iram_for_net(&pldat->pdev->dev)) { +#if 0 + /* FIXME: get iram from DT */ dma_handle = LPC32XX_IRAM_BASE; if (pldat->dma_buff_size <= lpc32xx_return_iram_size()) pldat->dma_buff_base_v = @@ -1394,6 +1397,7 @@ static int lpc_eth_drv_probe(struct platform_device *pdev) else netdev_err(ndev, "IRAM not big enough for net buffers, using SDRAM instead.\n"); +#endif } if (pldat->dma_buff_base_v == 0) { @@ -1483,11 +1487,13 @@ static int lpc_eth_drv_probe(struct platform_device *pdev) err_out_unregister_netdev: unregister_netdev(ndev); err_out_dma_unmap: +#if 0 if (!use_iram_for_net(&pldat->pdev->dev) || pldat->dma_buff_size > lpc32xx_return_iram_size()) dma_free_coherent(&pldat->pdev->dev, pldat->dma_buff_size, pldat->dma_buff_base_v, pldat->dma_buff_base_p); +#endif err_out_free_irq: free_irq(ndev->irq, ndev); err_out_iounmap: @@ -1509,11 +1515,13 @@ static int lpc_eth_drv_remove(struct platform_device *pdev) unregister_netdev(ndev); +#if 0 if (!use_iram_for_net(&pldat->pdev->dev) || pldat->dma_buff_size > lpc32xx_return_iram_size()) dma_free_coherent(&pldat->pdev->dev, pldat->dma_buff_size, pldat->dma_buff_base_v, pldat->dma_buff_base_p); +#endif free_irq(ndev->irq, ndev); iounmap(pldat->net_base); mdiobus_unregister(pldat->mii_bus); diff --git a/drivers/tty/serial/lpc32xx_hs.c b/drivers/tty/serial/lpc32xx_hs.c index 7eb04ae71cc8..c0b609a5c78d 100644 --- a/drivers/tty/serial/lpc32xx_hs.c +++ b/drivers/tty/serial/lpc32xx_hs.c @@ -34,8 +34,6 @@ #include <linux/irq.h> #include <linux/gpio.h> #include <linux/of.h> -#include <mach/platform.h> -#include <mach/hardware.h> /* * High Speed UART register offsets @@ -447,9 +445,9 @@ static void serial_lpc32xx_break_ctl(struct uart_port *port, /* LPC3250 Errata HSUART.1: Hang workaround via loopback mode on inactivity */ static void lpc32xx_loopback_set(resource_size_t mapbase, int state) { +#if 0 int bit; u32 tmp; - switch (mapbase) { case LPC32XX_HS_UART1_BASE: bit = 0; @@ -471,6 +469,7 @@ static void lpc32xx_loopback_set(resource_size_t mapbase, int state) else tmp &= ~(1 << bit); writel(tmp, LPC32XX_UARTCTL_CLOOP); +#endif } /* port->lock is not held. */ @@ -700,7 +699,10 @@ static int serial_hs_lpc32xx_probe(struct platform_device *pdev) p->port.irq = ret; p->port.iotype = UPIO_MEM32; +#if 0 + /* FIXME: use clk_get_rate() */ p->port.uartclk = LPC32XX_MAIN_OSC_FREQ; +#endif p->port.regshift = 2; p->port.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT | UPF_IOREMAP; p->port.dev = &pdev->dev; diff --git a/drivers/usb/gadget/udc/lpc32xx_udc.c b/drivers/usb/gadget/udc/lpc32xx_udc.c index 00b5006baf15..e3686bf02f5a 100644 --- a/drivers/usb/gadget/udc/lpc32xx_udc.c +++ b/drivers/usb/gadget/udc/lpc32xx_udc.c @@ -52,13 +52,9 @@ #include <linux/usb/isp1301.h> #include <asm/byteorder.h> -#include <mach/hardware.h> #include <linux/io.h> #include <asm/irq.h> -#include <mach/platform.h> -#include <mach/irqs.h> -#include <mach/board.h> #ifdef CONFIG_USB_GADGET_DEBUG_FILES #include <linux/debugfs.h> #include <linux/seq_file.h> @@ -652,8 +648,11 @@ static void isp1301_udc_configure(struct lpc32xx_udc *udc) i2c_smbus_write_byte_data(udc->isp1301_i2c_client, ISP1301_I2C_INTERRUPT_RISING, INT_VBUS_VLD); +#if 0 + /* FIXME: use clock interface */ /* Enable usb_need_clk clock after transceiver is initialized */ writel((readl(USB_CTRL) | USB_DEV_NEED_CLK_EN), USB_CTRL); +#endif dev_info(udc->dev, "ISP1301 Vendor ID : 0x%04x\n", i2c_smbus_read_word_data(udc->isp1301_i2c_client, 0x00)); @@ -997,9 +996,11 @@ static void udc_clk_set(struct lpc32xx_udc *udc, int enable) /* 48MHz PLL up */ clk_enable(udc->usb_pll_clk); +#if 0 /* Enable the USB device clock */ writel(readl(USB_CTRL) | USB_DEV_NEED_CLK_EN, USB_CTRL); +#endif clk_enable(udc->usb_otg_clk); } else { @@ -1013,10 +1014,11 @@ static void udc_clk_set(struct lpc32xx_udc *udc, int enable) /* 48MHz PLL dpwn */ clk_disable(udc->usb_pll_clk); +#if 0 /* Disable the USB device clock */ writel(readl(USB_CTRL) & ~USB_DEV_NEED_CLK_EN, USB_CTRL); - +#endif clk_disable(udc->usb_otg_clk); } } @@ -3138,8 +3140,10 @@ static int lpc32xx_udc_probe(struct platform_device *pdev) goto io_map_fail; } +#if 0 /* Enable AHB slave USB clock, needed for further USB clock control */ writel(USB_SLAVE_HCLK_EN | (1 << 19), USB_CTRL); +#endif /* Get required clocks */ udc->usb_pll_clk = clk_get(&pdev->dev, "ck_pll5"); @@ -3174,7 +3178,9 @@ static int lpc32xx_udc_probe(struct platform_device *pdev) goto pll_set_fail; } +#if 0 writel(readl(USB_CTRL) | USB_DEV_NEED_CLK_EN, USB_CTRL); +#endif /* Enable USB device clock */ retval = clk_enable(udc->usb_slv_clk); diff --git a/drivers/usb/host/ohci-nxp.c b/drivers/usb/host/ohci-nxp.c index d9f0481d7258..8f01d04b8687 100644 --- a/drivers/usb/host/ohci-nxp.c +++ b/drivers/usb/host/ohci-nxp.c @@ -33,13 +33,10 @@ #include "ohci.h" -#include <mach/hardware.h> #include <asm/mach-types.h> #include <asm/io.h> -#include <mach/platform.h> -#include <mach/irqs.h> - +#if 0 #define USB_CONFIG_BASE 0x31020000 #define PWRMAN_BASE 0x40004000 @@ -52,6 +49,7 @@ #define PAD_CONTROL_LAST_DRIVEN (1 << 19) #define USB_OTG_STAT_CONTROL IO_ADDRESS(USB_CONFIG_BASE + 0x110) +#endif /* USB_OTG_STAT_CONTROL bit defines */ #define TRANSPARENT_I2C_EN (1 << 7) @@ -117,8 +115,10 @@ static void isp1301_configure_lpc32xx(void) i2c_smbus_write_byte_data(isp1301_i2c_client, ISP1301_I2C_INTERRUPT_RISING | ISP1301_I2C_REG_CLEAR_ADDR, ~0); +#if 0 /* Enable usb_need_clk clock after transceiver is initialized */ __raw_writel(__raw_readl(USB_CTRL) | USB_HOST_NEED_CLK_EN, USB_CTRL); +#endif printk(KERN_INFO "ISP1301 Vendor ID : 0x%04x\n", i2c_smbus_read_word_data(isp1301_i2c_client, 0x00)); @@ -148,17 +148,21 @@ static inline void isp1301_vbus_off(void) static void ohci_nxp_start_hc(void) { +#if 0 unsigned long tmp = __raw_readl(USB_OTG_STAT_CONTROL) | HOST_EN; __raw_writel(tmp, USB_OTG_STAT_CONTROL); +#endif isp1301_vbus_on(); } static void ohci_nxp_stop_hc(void) { - unsigned long tmp; isp1301_vbus_off(); +#if 0 + unsigned long tmp; tmp = __raw_readl(USB_OTG_STAT_CONTROL) & ~HOST_EN; __raw_writel(tmp, USB_OTG_STAT_CONTROL); +#endif } static int ohci_hcd_nxp_probe(struct platform_device *pdev) @@ -192,8 +196,10 @@ static int ohci_hcd_nxp_probe(struct platform_device *pdev) goto fail_disable; } +#if 0 /* Enable AHB slave USB clock, needed for further USB clock control */ __raw_writel(USB_SLAVE_HCLK_EN | PAD_CONTROL_LAST_DRIVEN, USB_CTRL); +#endif /* Enable USB PLL */ usb_pll_clk = devm_clk_get(&pdev->dev, "ck_pll5"); @@ -237,7 +243,9 @@ static int ohci_hcd_nxp_probe(struct platform_device *pdev) goto fail_otg; } +#if 0 __raw_writel(__raw_readl(USB_CTRL) | USB_HOST_NEED_CLK_EN, USB_CTRL); +#endif ret = clk_enable(usb_otg_clk); if (ret < 0) { diff --git a/drivers/watchdog/pnx4008_wdt.c b/drivers/watchdog/pnx4008_wdt.c index 4224b3ec83a5..1ad107ac78e9 100644 --- a/drivers/watchdog/pnx4008_wdt.c +++ b/drivers/watchdog/pnx4008_wdt.c @@ -31,7 +31,6 @@ #include <linux/slab.h> #include <linux/err.h> #include <linux/of.h> -#include <mach/hardware.h> /* WatchDog Timer - Chapter 23 Page 207 */ ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/5] arm: dts: lpc32xx: fix improper usage of ranges property 2015-10-14 13:52 ` Arnd Bergmann @ 2015-10-14 14:07 ` Vladimir Zapolskiy 2015-10-14 14:13 ` Arnd Bergmann 2015-10-14 17:23 ` Joachim Eastwood 1 sibling, 1 reply; 21+ messages in thread From: Vladimir Zapolskiy @ 2015-10-14 14:07 UTC (permalink / raw) To: linux-arm-kernel On 14.10.2015 16:52, Arnd Bergmann wrote: > On Wednesday 14 October 2015 02:13:49 Vladimir Zapolskiy wrote: >> Ok, practically it should work for my purposes, but the change must be >> done along with added EMC device node description. >> >> I'm not so confident that it is correct to add description of static >> memory banks to ahb node though, please give me a short confirmation, > > The DT should reflect whichever memory ranges are accessible > on the bus. I've looked up the memory map in the data sheet, and > I think a reasonable representation would be > > /ahb { > ranges = <0x20000000 0x20000000 0x10000000>, /* AHB port 5 */ > <0x30000000 0x30000000 0x10000000>, /* AHB port 6 */ > <0x40000000 0x40000000 0x10000000>, /* AHB port 7 */ > <0x80000000 0x80000000 0x40000000>, /* EMC DYCS0/1 */ > <0xE0000000 0xE0000000 0x04000000>; /* EMC CS0-3 */ > > apb { > ranges = <0x20080000 0x20080000 0x00020000>; > }; > > }; A simpler version of this change in /ahb I successfully tested with EMC yesterday: - ranges = <0x20000000 0x20000000 0x30000000>; + ranges = <0x20000000 0x20000000 0x30000000 + 0xe0000000 0xe0000000 0x04000000>; > alternatively, each AHB port could be a separate node, with a > more direct translation like > > /ahb5 { > ranges = <0 0x20000000 0x10000000>; > > apb { > ranges = <0 0x80000 0x20000>; > }; > }; > /ahb6 { > ranges = <0 0x30000000 0x10000000>, /* AHB registers */ > <0x80000000 0x80000000 0x40000000>, /* EMC DYCS0/1 */ > <0xE0000000 0xE0000000 0x04000000>; /* EMC CS0-3 */ > > memory-controller at 1080000 { > reg = <0x1080000 0x10000>; > }; > }; > ... > > Does this make sense? Yes, I personally would prefer to see the first version as the simplest working one, if needed it should be converted to the second one later on. >> then I'll send a change for inclusion of EMC description -- the one >> above excluding clocks and clock-names properties, work on CCF is in >> progress. > > Ah, very nice! That should get us very close to multiplatform support! > I've just tried building lpc32xx without the headers to check for > other issues, and ended up with the patch below to get it building. > It's clearly wrong, but it highlights a number of issues. Sure, I'll review the highlighted problems, thanks. -- With best wishes, Vladimir ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/5] arm: dts: lpc32xx: fix improper usage of ranges property 2015-10-14 14:07 ` Vladimir Zapolskiy @ 2015-10-14 14:13 ` Arnd Bergmann 0 siblings, 0 replies; 21+ messages in thread From: Arnd Bergmann @ 2015-10-14 14:13 UTC (permalink / raw) To: linux-arm-kernel On Wednesday 14 October 2015 17:07:26 Vladimir Zapolskiy wrote: > On 14.10.2015 16:52, Arnd Bergmann wrote: > > On Wednesday 14 October 2015 02:13:49 Vladimir Zapolskiy wrote: > >> Ok, practically it should work for my purposes, but the change must be > >> done along with added EMC device node description. > >> > >> I'm not so confident that it is correct to add description of static > >> memory banks to ahb node though, please give me a short confirmation, > > > > The DT should reflect whichever memory ranges are accessible > > on the bus. I've looked up the memory map in the data sheet, and > > I think a reasonable representation would be > > > > /ahb { > > ranges = <0x20000000 0x20000000 0x10000000>, /* AHB port 5 */ > > <0x30000000 0x30000000 0x10000000>, /* AHB port 6 */ > > <0x40000000 0x40000000 0x10000000>, /* AHB port 7 */ > > <0x80000000 0x80000000 0x40000000>, /* EMC DYCS0/1 */ > > <0xE0000000 0xE0000000 0x04000000>; /* EMC CS0-3 */ > > > > apb { > > ranges = <0x20080000 0x20080000 0x00020000>; > > }; > > > > }; > > A simpler version of this change in /ahb I successfully tested with EMC > yesterday: > > - ranges = <0x20000000 0x20000000 0x30000000>; > + ranges = <0x20000000 0x20000000 0x30000000 > + 0xe0000000 0xe0000000 0x04000000>; Ok, but please write this as ranges = <0x20000000 0x20000000 0x30000000>, <0xe0000000 0xe0000000 0x04000000>; The binary is identical, it's just more structured. > >> then I'll send a change for inclusion of EMC description -- the one > >> above excluding clocks and clock-names properties, work on CCF is in > >> progress. > > > > Ah, very nice! That should get us very close to multiplatform support! > > I've just tried building lpc32xx without the headers to check for > > other issues, and ended up with the patch below to get it building. > > It's clearly wrong, but it highlights a number of issues. > > Sure, I'll review the highlighted problems, thanks. Thanks! Arnd ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/5] arm: dts: lpc32xx: fix improper usage of ranges property 2015-10-14 13:52 ` Arnd Bergmann 2015-10-14 14:07 ` Vladimir Zapolskiy @ 2015-10-14 17:23 ` Joachim Eastwood 2015-10-14 20:07 ` Arnd Bergmann 1 sibling, 1 reply; 21+ messages in thread From: Joachim Eastwood @ 2015-10-14 17:23 UTC (permalink / raw) To: linux-arm-kernel Hi Arnd, On 14 October 2015 at 15:52, Arnd Bergmann <arnd@arndb.de> wrote: > On Wednesday 14 October 2015 02:13:49 Vladimir Zapolskiy wrote: >> Ok, practically it should work for my purposes, but the change must be >> done along with added EMC device node description. >> >> I'm not so confident that it is correct to add description of static >> memory banks to ahb node though, please give me a short confirmation, > > The DT should reflect whichever memory ranges are accessible > on the bus. I've looked up the memory map in the data sheet, and > I think a reasonable representation would be > > /ahb { > ranges = <0x20000000 0x20000000 0x10000000>, /* AHB port 5 */ > <0x30000000 0x30000000 0x10000000>, /* AHB port 6 */ > <0x40000000 0x40000000 0x10000000>, /* AHB port 7 */ > <0x80000000 0x80000000 0x40000000>, /* EMC DYCS0/1 */ > <0xE0000000 0xE0000000 0x04000000>; /* EMC CS0-3 */ > > apb { > ranges = <0x20080000 0x20080000 0x00020000>; > }; > > }; > > alternatively, each AHB port could be a separate node, with a > more direct translation like > > /ahb5 { > ranges = <0 0x20000000 0x10000000>; > > apb { > ranges = <0 0x80000 0x20000>; > }; > }; > /ahb6 { > ranges = <0 0x30000000 0x10000000>, /* AHB registers */ > <0x80000000 0x80000000 0x40000000>, /* EMC DYCS0/1 */ > <0xE0000000 0xE0000000 0x04000000>; /* EMC CS0-3 */ > > memory-controller at 1080000 { > reg = <0x1080000 0x10000>; > }; > }; Sorry for hijacking the thread, but I have related question to this. What is the advantage of using a hierarchical bus structure in dt? I thought the recommendation, at least for new device trees, was to keep it flat under a "soc"-node. If doesn't offer any advantages why not remove instead of adding the ranges property which seems to grow a bit complex now? Of course removing it would create a lot of churn because of the re-indentation but at least the end result would be simpler. regards, Joachim Eastwood ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/5] arm: dts: lpc32xx: fix improper usage of ranges property 2015-10-14 17:23 ` Joachim Eastwood @ 2015-10-14 20:07 ` Arnd Bergmann 2015-10-14 21:15 ` Joachim Eastwood 0 siblings, 1 reply; 21+ messages in thread From: Arnd Bergmann @ 2015-10-14 20:07 UTC (permalink / raw) To: linux-arm-kernel On Wednesday 14 October 2015 19:23:09 Joachim Eastwood wrote: > > > > /ahb5 { > > ranges = <0 0x20000000 0x10000000>; > > > > apb { > > ranges = <0 0x80000 0x20000>; > > }; > > }; > > /ahb6 { > > ranges = <0 0x30000000 0x10000000>, /* AHB registers */ > > <0x80000000 0x80000000 0x40000000>, /* EMC DYCS0/1 */ > > <0xE0000000 0xE0000000 0x04000000>; /* EMC CS0-3 */ > > > > memory-controller at 1080000 { > > reg = <0x1080000 0x10000>; > > }; > > }; > > Sorry for hijacking the thread, but I have related question to this. > > What is the advantage of using a hierarchical bus structure in dt? > I thought the recommendation, at least for new device trees, was to > keep it flat under a "soc"-node. > > If doesn't offer any advantages why not remove instead of adding the > ranges property which seems to grow a bit complex now? > Of course removing it would create a lot of churn because of the > re-indentation but at least the end result would be simpler. The general recommendation is to have the DT structure resemble the hardware as closely as possible. If the chip has a clear hierarchy and you know it, then it's best to describe it that way. The reason for having a single 'soc' node in a lot of the modern dtsi files is that either it's not documented, or they use AXI 'buses' that are not really hierarchical but are point-to-point connections. Arnd ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/5] arm: dts: lpc32xx: fix improper usage of ranges property 2015-10-14 20:07 ` Arnd Bergmann @ 2015-10-14 21:15 ` Joachim Eastwood 0 siblings, 0 replies; 21+ messages in thread From: Joachim Eastwood @ 2015-10-14 21:15 UTC (permalink / raw) To: linux-arm-kernel On 14 October 2015 at 22:07, Arnd Bergmann <arnd@arndb.de> wrote: > On Wednesday 14 October 2015 19:23:09 Joachim Eastwood wrote: >> > >> > /ahb5 { >> > ranges = <0 0x20000000 0x10000000>; >> > >> > apb { >> > ranges = <0 0x80000 0x20000>; >> > }; >> > }; >> > /ahb6 { >> > ranges = <0 0x30000000 0x10000000>, /* AHB registers */ >> > <0x80000000 0x80000000 0x40000000>, /* EMC DYCS0/1 */ >> > <0xE0000000 0xE0000000 0x04000000>; /* EMC CS0-3 */ >> > >> > memory-controller at 1080000 { >> > reg = <0x1080000 0x10000>; >> > }; >> > }; >> >> Sorry for hijacking the thread, but I have related question to this. >> >> What is the advantage of using a hierarchical bus structure in dt? >> I thought the recommendation, at least for new device trees, was to >> keep it flat under a "soc"-node. >> >> If doesn't offer any advantages why not remove instead of adding the >> ranges property which seems to grow a bit complex now? >> Of course removing it would create a lot of churn because of the >> re-indentation but at least the end result would be simpler. > > The general recommendation is to have the DT structure resemble > the hardware as closely as possible. If the chip has a clear > hierarchy and you know it, then it's best to describe it that way. > > The reason for having a single 'soc' node in a lot of the modern > dtsi files is that either it's not documented, or they use AXI > 'buses' that are not really hierarchical but are point-to-point > connections. Thanks for explaining, Arnd! In that case lpc18xx.dtsi should have had a hierarchical structure as well. NXP LPC user manuals usually documents the bus structure. regards, Joachim Eastwood ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/5] arm: dts: lpc32xx: add labels to all defined peripheral nodes 2015-10-12 23:54 [PATCH 0/5] arm: dts: lpc32xx: fixes and updates to lpc32xx.dtsi Vladimir Zapolskiy 2015-10-12 23:54 ` [PATCH 1/5] arm: dts: lpc32xx: change include syntax to be C preprocessor friendly Vladimir Zapolskiy 2015-10-12 23:54 ` [PATCH 2/5] arm: dts: lpc32xx: fix improper usage of ranges property Vladimir Zapolskiy @ 2015-10-12 23:54 ` Vladimir Zapolskiy 2015-10-12 23:54 ` [PATCH 4/5] arm: dts: lpc32xx: remove unneeded cell settings from cpus Vladimir Zapolskiy 2015-10-12 23:54 ` [PATCH 5/5] arm: dts: lpc32xx: add device node for the second pwm controller Vladimir Zapolskiy 4 siblings, 0 replies; 21+ messages in thread From: Vladimir Zapolskiy @ 2015-10-12 23:54 UTC (permalink / raw) To: linux-arm-kernel To simplify writing of dts files for all lpc32xx.dtsi users who adjust device node properties, add labels to all defined peripheral device nodes in lpc32xx.dtsi. Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> --- arch/arm/boot/dts/lpc32xx.dtsi | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/arch/arm/boot/dts/lpc32xx.dtsi b/arch/arm/boot/dts/lpc32xx.dtsi index 3ef804c..dcb52cb 100644 --- a/arch/arm/boot/dts/lpc32xx.dtsi +++ b/arch/arm/boot/dts/lpc32xx.dtsi @@ -49,7 +49,7 @@ status = "disabled"; }; - dma at 31000000 { + dma: dma at 31000000 { compatible = "arm,pl080", "arm,primecell"; reg = <0x31000000 0x1000>; interrupts = <0x1c 0>; @@ -58,21 +58,21 @@ /* * Enable either ohci or usbd (gadget)! */ - ohci at 31020000 { + ohci: ohci at 31020000 { compatible = "nxp,ohci-nxp", "usb-ohci"; reg = <0x31020000 0x300>; interrupts = <0x3b 0>; status = "disabled"; }; - usbd at 31020000 { + usbd: usbd at 31020000 { compatible = "nxp,lpc3220-udc"; reg = <0x31020000 0x300>; interrupts = <0x3d 0>, <0x3e 0>, <0x3c 0>, <0x3a 0>; status = "disabled"; }; - clcd at 31040000 { + clcd: clcd at 31040000 { compatible = "arm,pl110", "arm,primecell"; reg = <0x31040000 0x1000>; interrupts = <0x0e 0>; @@ -118,7 +118,7 @@ reg = <0x20094000 0x1000>; }; - sd at 20098000 { + sd: sd at 20098000 { compatible = "arm,pl18x", "arm,primecell"; reg = <0x20098000 0x1000>; interrupts = <0x0f 0>, <0x0d 0>; @@ -243,7 +243,7 @@ status = "disabled"; }; - rtc at 40024000 { + rtc: rtc at 40024000 { compatible = "nxp,lpc3220-rtc"; reg = <0x40024000 0x1000>; interrupts = <0x34 0>; @@ -256,7 +256,7 @@ #gpio-cells = <3>; /* bank, pin, flags */ }; - watchdog at 4003C000 { + watchdog: watchdog at 4003C000 { compatible = "nxp,pnx4008-wdt"; reg = <0x4003C000 0x1000>; }; @@ -268,21 +268,21 @@ * them */ - adc at 40048000 { + adc: adc at 40048000 { compatible = "nxp,lpc3220-adc"; reg = <0x40048000 0x1000>; interrupts = <0x27 0>; status = "disabled"; }; - tsc at 40048000 { + tsc: tsc at 40048000 { compatible = "nxp,lpc3220-tsc"; reg = <0x40048000 0x1000>; interrupts = <0x27 0>; status = "disabled"; }; - key at 40050000 { + key: key at 40050000 { compatible = "nxp,lpc3220-key"; reg = <0x40050000 0x1000>; interrupts = <54 0>; -- 2.1.4 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 4/5] arm: dts: lpc32xx: remove unneeded cell settings from cpus 2015-10-12 23:54 [PATCH 0/5] arm: dts: lpc32xx: fixes and updates to lpc32xx.dtsi Vladimir Zapolskiy ` (2 preceding siblings ...) 2015-10-12 23:54 ` [PATCH 3/5] arm: dts: lpc32xx: add labels to all defined peripheral nodes Vladimir Zapolskiy @ 2015-10-12 23:54 ` Vladimir Zapolskiy 2015-10-13 8:18 ` Joachim Eastwood 2015-10-13 16:20 ` [PATCH v2 4/5] arm: dts: lpc32xx: add reg property to cpu device node Vladimir Zapolskiy 2015-10-12 23:54 ` [PATCH 5/5] arm: dts: lpc32xx: add device node for the second pwm controller Vladimir Zapolskiy 4 siblings, 2 replies; 21+ messages in thread From: Vladimir Zapolskiy @ 2015-10-12 23:54 UTC (permalink / raw) To: linux-arm-kernel There is no addressable devices under cpus device node, remove noisy address and size cells properties. Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> --- arch/arm/boot/dts/lpc32xx.dtsi | 3 --- 1 file changed, 3 deletions(-) diff --git a/arch/arm/boot/dts/lpc32xx.dtsi b/arch/arm/boot/dts/lpc32xx.dtsi index dcb52cb..929458d 100644 --- a/arch/arm/boot/dts/lpc32xx.dtsi +++ b/arch/arm/boot/dts/lpc32xx.dtsi @@ -18,9 +18,6 @@ interrupt-parent = <&mic>; cpus { - #address-cells = <0>; - #size-cells = <0>; - cpu { compatible = "arm,arm926ej-s"; device_type = "cpu"; -- 2.1.4 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 4/5] arm: dts: lpc32xx: remove unneeded cell settings from cpus 2015-10-12 23:54 ` [PATCH 4/5] arm: dts: lpc32xx: remove unneeded cell settings from cpus Vladimir Zapolskiy @ 2015-10-13 8:18 ` Joachim Eastwood 2015-10-13 10:03 ` Vladimir Zapolskiy 2015-10-13 16:20 ` [PATCH v2 4/5] arm: dts: lpc32xx: add reg property to cpu device node Vladimir Zapolskiy 1 sibling, 1 reply; 21+ messages in thread From: Joachim Eastwood @ 2015-10-13 8:18 UTC (permalink / raw) To: linux-arm-kernel Hi Vladimir, On 13 October 2015 at 01:54, Vladimir Zapolskiy <vz@mleia.com> wrote: > There is no addressable devices under cpus device node, remove noisy > address and size cells properties. > > Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> > --- > arch/arm/boot/dts/lpc32xx.dtsi | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/arch/arm/boot/dts/lpc32xx.dtsi b/arch/arm/boot/dts/lpc32xx.dtsi > index dcb52cb..929458d 100644 > --- a/arch/arm/boot/dts/lpc32xx.dtsi > +++ b/arch/arm/boot/dts/lpc32xx.dtsi > @@ -18,9 +18,6 @@ > interrupt-parent = <&mic>; > > cpus { > - #address-cells = <0>; > - #size-cells = <0>; > - According to Documentation/devicetree/bindings/arm/cpus.txt these properties are required. Take a look at Example 3 in the doc for it should look like on a ARM 926EJ-S uniprocessor 32-bit system. regards, Joachim Eastwood ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 4/5] arm: dts: lpc32xx: remove unneeded cell settings from cpus 2015-10-13 8:18 ` Joachim Eastwood @ 2015-10-13 10:03 ` Vladimir Zapolskiy 0 siblings, 0 replies; 21+ messages in thread From: Vladimir Zapolskiy @ 2015-10-13 10:03 UTC (permalink / raw) To: linux-arm-kernel Hi Joachim, On 13.10.2015 11:18, Joachim Eastwood wrote: > Hi Vladimir, > > On 13 October 2015 at 01:54, Vladimir Zapolskiy <vz@mleia.com> wrote: >> There is no addressable devices under cpus device node, remove noisy >> address and size cells properties. >> >> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> >> --- >> arch/arm/boot/dts/lpc32xx.dtsi | 3 --- >> 1 file changed, 3 deletions(-) >> >> diff --git a/arch/arm/boot/dts/lpc32xx.dtsi b/arch/arm/boot/dts/lpc32xx.dtsi >> index dcb52cb..929458d 100644 >> --- a/arch/arm/boot/dts/lpc32xx.dtsi >> +++ b/arch/arm/boot/dts/lpc32xx.dtsi >> @@ -18,9 +18,6 @@ >> interrupt-parent = <&mic>; >> >> cpus { >> - #address-cells = <0>; >> - #size-cells = <0>; >> - > > According to Documentation/devicetree/bindings/arm/cpus.txt these > properties are required. > > Take a look at Example 3 in the doc for it should look like on a ARM > 926EJ-S uniprocessor 32-bit system. thank you for review and pointing the fact out, then according to documentation #address-cells must be set to 1 plus reg property in cpu node is missing, I'll send a fix as patch v2 4/5. -- With best wishes, Vladimir ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 4/5] arm: dts: lpc32xx: add reg property to cpu device node 2015-10-12 23:54 ` [PATCH 4/5] arm: dts: lpc32xx: remove unneeded cell settings from cpus Vladimir Zapolskiy 2015-10-13 8:18 ` Joachim Eastwood @ 2015-10-13 16:20 ` Vladimir Zapolskiy 1 sibling, 0 replies; 21+ messages in thread From: Vladimir Zapolskiy @ 2015-10-13 16:20 UTC (permalink / raw) To: linux-arm-kernel According to device tree bindings for ARM cpus cpu node must contain a reg property for enumeration scheme. The change adds reg = <0x0> indicating that the processor does not have CPU identification register and updates cell settings. Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> --- Changes from v1 to v2: - instead of removing address/size cells properties update them in accordance to ARM CPU device tree bindings, thanks to Joachim for review The change replaces 4/5 "arm: dts: lpc32xx: remove unneeded cell settings from cpus" arch/arm/boot/dts/lpc32xx.dtsi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/lpc32xx.dtsi b/arch/arm/boot/dts/lpc32xx.dtsi index dcb52cb..343a94f 100644 --- a/arch/arm/boot/dts/lpc32xx.dtsi +++ b/arch/arm/boot/dts/lpc32xx.dtsi @@ -18,12 +18,13 @@ interrupt-parent = <&mic>; cpus { - #address-cells = <0>; + #address-cells = <1>; #size-cells = <0>; - cpu { + cpu at 0 { compatible = "arm,arm926ej-s"; device_type = "cpu"; + reg = <0x0>; }; }; -- 2.1.4 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 5/5] arm: dts: lpc32xx: add device node for the second pwm controller 2015-10-12 23:54 [PATCH 0/5] arm: dts: lpc32xx: fixes and updates to lpc32xx.dtsi Vladimir Zapolskiy ` (3 preceding siblings ...) 2015-10-12 23:54 ` [PATCH 4/5] arm: dts: lpc32xx: remove unneeded cell settings from cpus Vladimir Zapolskiy @ 2015-10-12 23:54 ` Vladimir Zapolskiy 2015-10-14 18:04 ` Joachim Eastwood 4 siblings, 1 reply; 21+ messages in thread From: Vladimir Zapolskiy @ 2015-10-12 23:54 UTC (permalink / raw) To: linux-arm-kernel LPC32xx SoCs have two independent PWM controllers, they have different clock parents, clock gates and even slightly different controls, each of these two PWM controllers has one output channel. Due to almost similar controls arranged in a row it is incorrectly assumed that there is one PWM controller with two channels, fix this problem in lpc32xx.dtsi, which at the moment prevents separate configuration of different clock parents and gates for both PWM controllers. Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> --- arch/arm/boot/dts/lpc32xx.dtsi | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/lpc32xx.dtsi b/arch/arm/boot/dts/lpc32xx.dtsi index 929458d..f4d2a0e 100644 --- a/arch/arm/boot/dts/lpc32xx.dtsi +++ b/arch/arm/boot/dts/lpc32xx.dtsi @@ -286,9 +286,15 @@ status = "disabled"; }; - pwm: pwm at 4005C000 { + pwm1: pwm at 4005C000 { compatible = "nxp,lpc3220-pwm"; - reg = <0x4005C000 0x8>; + reg = <0x4005C000 0x4>; + status = "disabled"; + }; + + pwm2: pwm at 4005C004 { + compatible = "nxp,lpc3220-pwm"; + reg = <0x4005C004 0x4>; status = "disabled"; }; }; -- 2.1.4 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 5/5] arm: dts: lpc32xx: add device node for the second pwm controller 2015-10-12 23:54 ` [PATCH 5/5] arm: dts: lpc32xx: add device node for the second pwm controller Vladimir Zapolskiy @ 2015-10-14 18:04 ` Joachim Eastwood 2015-10-15 10:25 ` Vladimir Zapolskiy 0 siblings, 1 reply; 21+ messages in thread From: Joachim Eastwood @ 2015-10-14 18:04 UTC (permalink / raw) To: linux-arm-kernel Hi Vladimir, On 13 October 2015 at 01:54, Vladimir Zapolskiy <vz@mleia.com> wrote: > LPC32xx SoCs have two independent PWM controllers, they have different > clock parents, clock gates and even slightly different controls, > each of these two PWM controllers has one output channel. Due to > almost similar controls arranged in a row it is incorrectly assumed > that there is one PWM controller with two channels, fix this problem > in lpc32xx.dtsi, which at the moment prevents separate configuration > of different clock parents and gates for both PWM controllers. > > Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> > --- > arch/arm/boot/dts/lpc32xx.dtsi | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/boot/dts/lpc32xx.dtsi b/arch/arm/boot/dts/lpc32xx.dtsi > index 929458d..f4d2a0e 100644 > --- a/arch/arm/boot/dts/lpc32xx.dtsi > +++ b/arch/arm/boot/dts/lpc32xx.dtsi > @@ -286,9 +286,15 @@ > status = "disabled"; > }; > > - pwm: pwm at 4005C000 { > + pwm1: pwm at 4005C000 { > compatible = "nxp,lpc3220-pwm"; > - reg = <0x4005C000 0x8>; > + reg = <0x4005C000 0x4>; > + status = "disabled"; > + }; > + > + pwm2: pwm at 4005C004 { > + compatible = "nxp,lpc3220-pwm"; > + reg = <0x4005C004 0x4>; > status = "disabled"; > }; > }; I am not really against your change, but... What's wrong with a binding like the one below? pwm: pwm at 0x4005c000 { compatible = "nxp,lpc3220-pwm"; reg = <0x4005C000 0x8>; clocks =<&clk CLK_PWM1, &clk CLK_PWM2>; clock-names = "pwm1", "pwm2"; #pwm-cells = <3>; }; With two clocks and where the first pwm-cell would select either PWM1 or PWM2. Seems like the driver only handle one clock, but should be fairly easy to fix. Note: with your DT change you would also need to change the driver since it currently sets npwm to 2. regards, Joachim Eastwood ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 5/5] arm: dts: lpc32xx: add device node for the second pwm controller 2015-10-14 18:04 ` Joachim Eastwood @ 2015-10-15 10:25 ` Vladimir Zapolskiy 0 siblings, 0 replies; 21+ messages in thread From: Vladimir Zapolskiy @ 2015-10-15 10:25 UTC (permalink / raw) To: linux-arm-kernel Hi Joachim, On 14.10.2015 21:04, Joachim Eastwood wrote: > Hi Vladimir, > > On 13 October 2015 at 01:54, Vladimir Zapolskiy <vz@mleia.com> wrote: >> LPC32xx SoCs have two independent PWM controllers, they have different >> clock parents, clock gates and even slightly different controls, >> each of these two PWM controllers has one output channel. Due to >> almost similar controls arranged in a row it is incorrectly assumed >> that there is one PWM controller with two channels, fix this problem >> in lpc32xx.dtsi, which at the moment prevents separate configuration >> of different clock parents and gates for both PWM controllers. >> >> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> >> --- >> arch/arm/boot/dts/lpc32xx.dtsi | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm/boot/dts/lpc32xx.dtsi b/arch/arm/boot/dts/lpc32xx.dtsi >> index 929458d..f4d2a0e 100644 >> --- a/arch/arm/boot/dts/lpc32xx.dtsi >> +++ b/arch/arm/boot/dts/lpc32xx.dtsi >> @@ -286,9 +286,15 @@ >> status = "disabled"; >> }; >> >> - pwm: pwm at 4005C000 { >> + pwm1: pwm at 4005C000 { >> compatible = "nxp,lpc3220-pwm"; >> - reg = <0x4005C000 0x8>; >> + reg = <0x4005C000 0x4>; >> + status = "disabled"; >> + }; >> + >> + pwm2: pwm at 4005C004 { >> + compatible = "nxp,lpc3220-pwm"; >> + reg = <0x4005C004 0x4>; >> status = "disabled"; >> }; >> }; > > I am not really against your change, but... > > What's wrong with a binding like the one below? > pwm: pwm at 0x4005c000 { > compatible = "nxp,lpc3220-pwm"; > reg = <0x4005C000 0x8>; > clocks =<&clk CLK_PWM1, &clk CLK_PWM2>; > clock-names = "pwm1", "pwm2"; > #pwm-cells = <3>; > }; > > With two clocks and where the first pwm-cell would select either PWM1 or PWM2. > > Seems like the driver only handle one clock, but should be fairly easy to fix. I thought about it and IMHO it is a more complicated change in DTS (and no doubts in the driver), which hides the structure of hardware. There is no one PWM with two channels. There are two independent PWMs, even control registers are different, PWM2 can be programmed to output the internal interrupt status, and it means that possibly in future I may want to describe one of the PWMs with a different "compatible" property. > Note: with your DT change you would also need to change the driver > since it currently sets npwm to 2. > It is done -- http://permalink.gmane.org/gmane.linux.pwm/2831 Thanks for review. -- With best wishes, Vladimir ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2015-10-15 10:25 UTC | newest] Thread overview: 21+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-10-12 23:54 [PATCH 0/5] arm: dts: lpc32xx: fixes and updates to lpc32xx.dtsi Vladimir Zapolskiy 2015-10-12 23:54 ` [PATCH 1/5] arm: dts: lpc32xx: change include syntax to be C preprocessor friendly Vladimir Zapolskiy 2015-10-12 23:54 ` [PATCH 2/5] arm: dts: lpc32xx: fix improper usage of ranges property Vladimir Zapolskiy 2015-10-13 12:44 ` Arnd Bergmann 2015-10-13 15:51 ` Vladimir Zapolskiy 2015-10-13 19:36 ` Arnd Bergmann 2015-10-13 23:13 ` Vladimir Zapolskiy 2015-10-14 13:52 ` Arnd Bergmann 2015-10-14 14:07 ` Vladimir Zapolskiy 2015-10-14 14:13 ` Arnd Bergmann 2015-10-14 17:23 ` Joachim Eastwood 2015-10-14 20:07 ` Arnd Bergmann 2015-10-14 21:15 ` Joachim Eastwood 2015-10-12 23:54 ` [PATCH 3/5] arm: dts: lpc32xx: add labels to all defined peripheral nodes Vladimir Zapolskiy 2015-10-12 23:54 ` [PATCH 4/5] arm: dts: lpc32xx: remove unneeded cell settings from cpus Vladimir Zapolskiy 2015-10-13 8:18 ` Joachim Eastwood 2015-10-13 10:03 ` Vladimir Zapolskiy 2015-10-13 16:20 ` [PATCH v2 4/5] arm: dts: lpc32xx: add reg property to cpu device node Vladimir Zapolskiy 2015-10-12 23:54 ` [PATCH 5/5] arm: dts: lpc32xx: add device node for the second pwm controller Vladimir Zapolskiy 2015-10-14 18:04 ` Joachim Eastwood 2015-10-15 10:25 ` Vladimir Zapolskiy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).