* [PATCH 0/2] Adding SATA support for Armada 370/XP @ 2012-10-24 13:49 Gregory CLEMENT 2012-10-24 13:49 ` [PATCH 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP Gregory CLEMENT 2012-10-24 13:49 ` [PATCH 2/2] arm: mvebu: adding SATA support: dt binding and config update Gregory CLEMENT 0 siblings, 2 replies; 21+ messages in thread From: Gregory CLEMENT @ 2012-10-24 13:49 UTC (permalink / raw) To: Jason Cooper, Andrew Lunn, Gregory Clement Cc: linux-arm-kernel, Arnd Bergmann, Olof Johansson, Ben Dooks, Ian Molton, Nicolas Pitre, Lior Amsalem, Maen Suleiman, Tawfik Bayouk, Shadi Ammouri, Eran Ben-Avi, Yehuda Yitschak, Nadav Haklai, Ike Pan, Jani Monoses, Chris Van Hoof, Dan Frazier, Thomas Petazzoni, Leif Lindholm, Jon Masters, David Marlin, Sebastian Hesselbarth, linux-kernel Hello, this small patch set adds the SATA support for Armada 370 and Armada XP. The evaluation boards for Armada 370 and Armada XP come with 2 SATA ports, and when both are enable the coherent pool for DMA mapping was too short. It was exactly the same issue that was fixed for Kirkwood two months ago. So I used the same fix in the first patch. Later when Kirkwood will be part of mach-mvebu, then this fix will be shared between the 2 SoCs families. This patch set is based on 3.7-rc2 and depends one the framework clock support (the last version was posted last week: http://thread.gmane.org/gmane.linux.kernel/1375701). The git branch called mvebu-SATA-for-3.8 is also available at https://github.com/MISL-EBU-System-SW/mainline-public.git. Regards, Gregory Gregory CLEMENT (1): arm: mvebu: increase atomic coherent pool size for armada 370/XP Lior Amsalem (1): arm: mvebu: adding SATA support: dt binding and config update arch/arm/boot/dts/armada-370-db.dts | 3 +++ arch/arm/boot/dts/armada-370-xp.dtsi | 10 ++++++++++ arch/arm/boot/dts/armada-xp-db.dts | 3 +++ arch/arm/configs/mvebu_defconfig | 7 +++++++ arch/arm/mach-mvebu/armada-370-xp.c | 12 ++++++++++++ 5 files changed, 35 insertions(+) -- 1.7.9.5 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP 2012-10-24 13:49 [PATCH 0/2] Adding SATA support for Armada 370/XP Gregory CLEMENT @ 2012-10-24 13:49 ` Gregory CLEMENT 2012-10-25 5:29 ` Marek Szyprowski 2012-10-25 11:27 ` Arnd Bergmann 2012-10-24 13:49 ` [PATCH 2/2] arm: mvebu: adding SATA support: dt binding and config update Gregory CLEMENT 1 sibling, 2 replies; 21+ messages in thread From: Gregory CLEMENT @ 2012-10-24 13:49 UTC (permalink / raw) To: Jason Cooper, Andrew Lunn, Gregory Clement Cc: linux-arm-kernel, Arnd Bergmann, Olof Johansson, Ben Dooks, Ian Molton, Nicolas Pitre, Lior Amsalem, Maen Suleiman, Tawfik Bayouk, Shadi Ammouri, Eran Ben-Avi, Yehuda Yitschak, Nadav Haklai, Ike Pan, Jani Monoses, Chris Van Hoof, Dan Frazier, Thomas Petazzoni, Leif Lindholm, Jon Masters, David Marlin, Sebastian Hesselbarth, linux-kernel, Marek Szyprowski For Armada 370/XP we have the same problem that for the commit cb01b63, so we applied the same solution: "The default 256 KiB coherent pool may be too small for some of the Kirkwood devices, so increase it to make sure that devices will be able to allocate their buffers with GFP_ATOMIC flag" Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> Cc: Marek Szyprowski <m.szyprowski@samsung.com> --- arch/arm/mach-mvebu/armada-370-xp.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/arch/arm/mach-mvebu/armada-370-xp.c b/arch/arm/mach-mvebu/armada-370-xp.c index 2af6ce5..cbad821 100644 --- a/arch/arm/mach-mvebu/armada-370-xp.c +++ b/arch/arm/mach-mvebu/armada-370-xp.c @@ -17,6 +17,7 @@ #include <linux/of_platform.h> #include <linux/io.h> #include <linux/time-armada-370-xp.h> +#include <linux/dma-mapping.h> #include <asm/mach/arch.h> #include <asm/mach/map.h> #include <asm/mach/time.h> @@ -43,6 +44,16 @@ void __init armada_370_xp_timer_and_clk_init(void) armada_370_xp_timer_init(); } +void __init armada_370_xp_init_early(void) +{ + /* + * Some Armada 370/XP devices allocate their coherent buffers + * from atomic context. Increase size of atomic coherent pool + * to make sure such the allocations won't fail. + */ + init_dma_coherent_pool_size(SZ_1M); +} + struct sys_timer armada_370_xp_timer = { .init = armada_370_xp_timer_and_clk_init, }; @@ -61,6 +72,7 @@ static const char * const armada_370_xp_dt_board_dt_compat[] = { DT_MACHINE_START(ARMADA_XP_DT, "Marvell Aramada 370/XP (Device Tree)") .init_machine = armada_370_xp_dt_init, .map_io = armada_370_xp_map_io, + .init_early = armada_370_xp_init_early, .init_irq = armada_370_xp_init_irq, .handle_irq = armada_370_xp_handle_irq, .timer = &armada_370_xp_timer, -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP 2012-10-24 13:49 ` [PATCH 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP Gregory CLEMENT @ 2012-10-25 5:29 ` Marek Szyprowski 2012-10-25 11:27 ` Arnd Bergmann 1 sibling, 0 replies; 21+ messages in thread From: Marek Szyprowski @ 2012-10-25 5:29 UTC (permalink / raw) To: Gregory CLEMENT Cc: Jason Cooper, Andrew Lunn, linux-arm-kernel, Arnd Bergmann, Olof Johansson, Ben Dooks, Ian Molton, Nicolas Pitre, Lior Amsalem, Maen Suleiman, Tawfik Bayouk, Shadi Ammouri, Eran Ben-Avi, Yehuda Yitschak, Nadav Haklai, Ike Pan, Jani Monoses, Chris Van Hoof, Dan Frazier, Thomas Petazzoni, Leif Lindholm, Jon Masters, David Marlin, Sebastian Hesselbarth, linux-kernel Hello, On 10/24/2012 3:49 PM, Gregory CLEMENT wrote: > For Armada 370/XP we have the same problem that for the commit > cb01b63, so we applied the same solution: "The default 256 KiB > coherent pool may be too small for some of the Kirkwood devices, so > increase it to make sure that devices will be able to allocate their > buffers with GFP_ATOMIC flag" > > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > > Cc: Marek Szyprowski <m.szyprowski@samsung.com> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > arch/arm/mach-mvebu/armada-370-xp.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/arch/arm/mach-mvebu/armada-370-xp.c b/arch/arm/mach-mvebu/armada-370-xp.c > index 2af6ce5..cbad821 100644 > --- a/arch/arm/mach-mvebu/armada-370-xp.c > +++ b/arch/arm/mach-mvebu/armada-370-xp.c > @@ -17,6 +17,7 @@ > #include <linux/of_platform.h> > #include <linux/io.h> > #include <linux/time-armada-370-xp.h> > +#include <linux/dma-mapping.h> > #include <asm/mach/arch.h> > #include <asm/mach/map.h> > #include <asm/mach/time.h> > @@ -43,6 +44,16 @@ void __init armada_370_xp_timer_and_clk_init(void) > armada_370_xp_timer_init(); > } > > +void __init armada_370_xp_init_early(void) > +{ > + /* > + * Some Armada 370/XP devices allocate their coherent buffers > + * from atomic context. Increase size of atomic coherent pool > + * to make sure such the allocations won't fail. > + */ > + init_dma_coherent_pool_size(SZ_1M); > +} > + > struct sys_timer armada_370_xp_timer = { > .init = armada_370_xp_timer_and_clk_init, > }; > @@ -61,6 +72,7 @@ static const char * const armada_370_xp_dt_board_dt_compat[] = { > DT_MACHINE_START(ARMADA_XP_DT, "Marvell Aramada 370/XP (Device Tree)") > .init_machine = armada_370_xp_dt_init, > .map_io = armada_370_xp_map_io, > + .init_early = armada_370_xp_init_early, > .init_irq = armada_370_xp_init_irq, > .handle_irq = armada_370_xp_handle_irq, > .timer = &armada_370_xp_timer, > Best regards -- Marek Szyprowski Samsung Poland R&D Center ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP 2012-10-24 13:49 ` [PATCH 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP Gregory CLEMENT 2012-10-25 5:29 ` Marek Szyprowski @ 2012-10-25 11:27 ` Arnd Bergmann 2012-10-25 11:43 ` Thomas Petazzoni 1 sibling, 1 reply; 21+ messages in thread From: Arnd Bergmann @ 2012-10-25 11:27 UTC (permalink / raw) To: Gregory CLEMENT Cc: Jason Cooper, Andrew Lunn, linux-arm-kernel, Olof Johansson, Ben Dooks, Ian Molton, Nicolas Pitre, Lior Amsalem, Maen Suleiman, Tawfik Bayouk, Shadi Ammouri, Eran Ben-Avi, Yehuda Yitschak, Nadav Haklai, Ike Pan, Jani Monoses, Chris Van Hoof, Dan Frazier, Thomas Petazzoni, Leif Lindholm, Jon Masters, David Marlin, Sebastian Hesselbarth, linux-kernel, Marek Szyprowski On Wednesday 24 October 2012, Gregory CLEMENT wrote: > For Armada 370/XP we have the same problem that for the commit > cb01b63, so we applied the same solution: "The default 256 KiB > coherent pool may be too small for some of the Kirkwood devices, so > increase it to make sure that devices will be able to allocate their > buffers with GFP_ATOMIC flag" > > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> Do you know why the ATA driver needs this? I find it surprising that it's necessary, so I'd like to make sure we're not just working around a device driver bug here. Arnd ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP 2012-10-25 11:27 ` Arnd Bergmann @ 2012-10-25 11:43 ` Thomas Petazzoni 2012-10-25 13:46 ` Arnd Bergmann 0 siblings, 1 reply; 21+ messages in thread From: Thomas Petazzoni @ 2012-10-25 11:43 UTC (permalink / raw) To: Arnd Bergmann Cc: Gregory CLEMENT, Jason Cooper, Andrew Lunn, linux-arm-kernel, Olof Johansson, Ben Dooks, Ian Molton, Nicolas Pitre, Lior Amsalem, Maen Suleiman, Tawfik Bayouk, Shadi Ammouri, Eran Ben-Avi, Yehuda Yitschak, Nadav Haklai, Ike Pan, Jani Monoses, Chris Van Hoof, Dan Frazier, Leif Lindholm, Jon Masters, David Marlin, Sebastian Hesselbarth, linux-kernel, Marek Szyprowski Arnd, On Thu, 25 Oct 2012 11:27:36 +0000, Arnd Bergmann wrote: > On Wednesday 24 October 2012, Gregory CLEMENT wrote: > > For Armada 370/XP we have the same problem that for the commit > > cb01b63, so we applied the same solution: "The default 256 KiB > > coherent pool may be too small for some of the Kirkwood devices, so > > increase it to make sure that devices will be able to allocate their > > buffers with GFP_ATOMIC flag" > > > > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > > Do you know why the ATA driver needs this? I find it surprising that > it's necessary, so I'd like to make sure we're not just working around > a device driver bug here. The sata_mv driver create dma_pool and allocate objects from them, and all the memory allocated for dma_pools is allocated using dma_alloc_coherent(), and I guess the driver is using too much of them. Seems like the driver is too lazy and allocates everything coherent to avoid the hassle of doing dma_map/dma_unmap operations when needed, but I haven't looked in details at the driver yet to see if it would be possible to switch those DMA coherent allocations into non-coherent allocations + appropriate calls to the DMA operations. That said, that's for sure a larger task than just enabling SATA on Armada 370/XP, so I would advocate to handle this problem separately. Best regards, Thomas -- Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP 2012-10-25 11:43 ` Thomas Petazzoni @ 2012-10-25 13:46 ` Arnd Bergmann 2012-10-25 14:09 ` Thomas Petazzoni 0 siblings, 1 reply; 21+ messages in thread From: Arnd Bergmann @ 2012-10-25 13:46 UTC (permalink / raw) To: Thomas Petazzoni Cc: Gregory CLEMENT, Jason Cooper, Andrew Lunn, linux-arm-kernel, Olof Johansson, Ben Dooks, Ian Molton, Nicolas Pitre, Lior Amsalem, Maen Suleiman, Tawfik Bayouk, Shadi Ammouri, Eran Ben-Avi, Yehuda Yitschak, Nadav Haklai, Ike Pan, Jani Monoses, Chris Van Hoof, Dan Frazier, Leif Lindholm, Jon Masters, David Marlin, Sebastian Hesselbarth, linux-kernel, Marek Szyprowski On Thursday 25 October 2012, Thomas Petazzoni wrote: > On Thu, 25 Oct 2012 11:27:36 +0000, Arnd Bergmann wrote: > > On Wednesday 24 October 2012, Gregory CLEMENT wrote: > > > For Armada 370/XP we have the same problem that for the commit > > > cb01b63, so we applied the same solution: "The default 256 KiB > > > coherent pool may be too small for some of the Kirkwood devices, so > > > increase it to make sure that devices will be able to allocate their > > > buffers with GFP_ATOMIC flag" > > > > > > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > > > > Do you know why the ATA driver needs this? I find it surprising that > > it's necessary, so I'd like to make sure we're not just working around > > a device driver bug here. > > The sata_mv driver create dma_pool and allocate objects from them, and > all the memory allocated for dma_pools is allocated using > dma_alloc_coherent(), and I guess the driver is using too much of them. > > Seems like the driver is too lazy and allocates everything coherent to > avoid the hassle of doing dma_map/dma_unmap operations when needed, but > I haven't looked in details at the driver yet to see if it would be > possible to switch those DMA coherent allocations into non-coherent > allocations + appropriate calls to the DMA operations. Using coherent allocations is fine, I was wondering whether they need to be atomic or not. > That said, that's for sure a larger task than just enabling SATA on > Armada 370/XP, so I would advocate to handle this problem separately. Agreed. Arnd ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP 2012-10-25 13:46 ` Arnd Bergmann @ 2012-10-25 14:09 ` Thomas Petazzoni 0 siblings, 0 replies; 21+ messages in thread From: Thomas Petazzoni @ 2012-10-25 14:09 UTC (permalink / raw) To: Arnd Bergmann Cc: Gregory CLEMENT, Jason Cooper, Andrew Lunn, linux-arm-kernel, Olof Johansson, Ben Dooks, Ian Molton, Nicolas Pitre, Lior Amsalem, Maen Suleiman, Tawfik Bayouk, Shadi Ammouri, Eran Ben-Avi, Yehuda Yitschak, Nadav Haklai, Ike Pan, Jani Monoses, Chris Van Hoof, Dan Frazier, Leif Lindholm, Jon Masters, David Marlin, Sebastian Hesselbarth, linux-kernel, Marek Szyprowski On Thu, 25 Oct 2012 13:46:41 +0000, Arnd Bergmann wrote: > > Seems like the driver is too lazy and allocates everything coherent > > to avoid the hassle of doing dma_map/dma_unmap operations when > > needed, but I haven't looked in details at the driver yet to see if > > it would be possible to switch those DMA coherent allocations into > > non-coherent allocations + appropriate calls to the DMA operations. > > Using coherent allocations is fine, I was wondering whether they need > to be atomic or not. You're raising a good point here: all the dma_pool_alloc() allocations done by sata_mv are GFP_KERNEL. So why are we having problems with the /atomic/ coherent pool size? Is it the libata core that's doing GFP_ATOMIC DMA coherent allocations. It doesn't seem so. Something's odd. Thomas -- Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/2] arm: mvebu: adding SATA support: dt binding and config update 2012-10-24 13:49 [PATCH 0/2] Adding SATA support for Armada 370/XP Gregory CLEMENT 2012-10-24 13:49 ` [PATCH 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP Gregory CLEMENT @ 2012-10-24 13:49 ` Gregory CLEMENT 2012-10-24 14:01 ` Thomas Petazzoni 2012-10-24 14:08 ` Andrew Lunn 1 sibling, 2 replies; 21+ messages in thread From: Gregory CLEMENT @ 2012-10-24 13:49 UTC (permalink / raw) To: Jason Cooper, Andrew Lunn, Gregory Clement Cc: linux-arm-kernel, Arnd Bergmann, Olof Johansson, Ben Dooks, Ian Molton, Nicolas Pitre, Lior Amsalem, Maen Suleiman, Tawfik Bayouk, Shadi Ammouri, Eran Ben-Avi, Yehuda Yitschak, Nadav Haklai, Ike Pan, Jani Monoses, Chris Van Hoof, Dan Frazier, Thomas Petazzoni, Leif Lindholm, Jon Masters, David Marlin, Sebastian Hesselbarth, linux-kernel From: Lior Amsalem <alior@marvell.com> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> Signed-off-by: Lior Amsalem <alior@marvell.com> --- arch/arm/boot/dts/armada-370-db.dts | 3 +++ arch/arm/boot/dts/armada-370-xp.dtsi | 10 ++++++++++ arch/arm/boot/dts/armada-xp-db.dts | 3 +++ arch/arm/configs/mvebu_defconfig | 7 +++++++ 4 files changed, 23 insertions(+) diff --git a/arch/arm/boot/dts/armada-370-db.dts b/arch/arm/boot/dts/armada-370-db.dts index 4a31b03..2a2aa75 100644 --- a/arch/arm/boot/dts/armada-370-db.dts +++ b/arch/arm/boot/dts/armada-370-db.dts @@ -34,5 +34,8 @@ clock-frequency = <200000000>; status = "okay"; }; + sata@d00a0000 { + status = "okay"; + }; }; }; diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi index 94b4b9e..3f08233 100644 --- a/arch/arm/boot/dts/armada-370-xp.dtsi +++ b/arch/arm/boot/dts/armada-370-xp.dtsi @@ -69,6 +69,16 @@ compatible = "marvell,armada-addr-decoding-controller"; reg = <0xd0020000 0x258>; }; + + sata@d00a0000 { + compatible = "marvell,orion-sata"; + reg = <0xd00a0000 0x2400>; + interrupts = <55>; + nr-ports = <2>; + clocks = <&coreclk 0>;//, <&coreclk 0>; + status = "disabled"; + }; + }; }; diff --git a/arch/arm/boot/dts/armada-xp-db.dts b/arch/arm/boot/dts/armada-xp-db.dts index b1fc728..b0db9a3 100644 --- a/arch/arm/boot/dts/armada-xp-db.dts +++ b/arch/arm/boot/dts/armada-xp-db.dts @@ -46,5 +46,8 @@ clock-frequency = <250000000>; status = "okay"; }; + sata@d00a0000 { + status = "okay"; + }; }; }; diff --git a/arch/arm/configs/mvebu_defconfig b/arch/arm/configs/mvebu_defconfig index 7bcf850..8ac5f3c 100644 --- a/arch/arm/configs/mvebu_defconfig +++ b/arch/arm/configs/mvebu_defconfig @@ -18,6 +18,13 @@ CONFIG_ZBOOT_ROM_BSS=0x0 CONFIG_ARM_APPENDED_DTB=y CONFIG_VFP=y CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug" +CONFIG_BLK_DEV_SD=y +CONFIG_ATA=y +CONFIG_SATA_MV=y +CONFIG_MD=y +CONFIG_BLK_DEV_MD=y +# CONFIG_MD_AUTODETECT is not set +CONFIG_MD_RAID456=y CONFIG_SERIAL_8250=y CONFIG_SERIAL_8250_CONSOLE=y CONFIG_SERIAL_OF_PLATFORM=y -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] arm: mvebu: adding SATA support: dt binding and config update 2012-10-24 13:49 ` [PATCH 2/2] arm: mvebu: adding SATA support: dt binding and config update Gregory CLEMENT @ 2012-10-24 14:01 ` Thomas Petazzoni 2012-10-24 14:05 ` Gregory CLEMENT 2012-10-24 14:08 ` Andrew Lunn 1 sibling, 1 reply; 21+ messages in thread From: Thomas Petazzoni @ 2012-10-24 14:01 UTC (permalink / raw) To: Gregory CLEMENT Cc: Jason Cooper, Andrew Lunn, linux-arm-kernel, Arnd Bergmann, Olof Johansson, Ben Dooks, Ian Molton, Nicolas Pitre, Lior Amsalem, Maen Suleiman, Tawfik Bayouk, Shadi Ammouri, Eran Ben-Avi, Yehuda Yitschak, Nadav Haklai, Ike Pan, Jani Monoses, Chris Van Hoof, Dan Frazier, Leif Lindholm, Jon Masters, David Marlin, Sebastian Hesselbarth, linux-kernel Hello, Shouldn't you split into one commit adding the SATA definition in the .dtsi + doing the defconfig change (the "SoC" level modifications), and then another commit for the .dts change? I don't really care personally, it's really up to Jason/Andrew on this. Another comment below, though. On Wed, 24 Oct 2012 15:49:21 +0200, Gregory CLEMENT wrote: > diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi > index 94b4b9e..3f08233 100644 > --- a/arch/arm/boot/dts/armada-370-xp.dtsi > +++ b/arch/arm/boot/dts/armada-370-xp.dtsi > @@ -69,6 +69,16 @@ > compatible = "marvell,armada-addr-decoding-controller"; > reg = <0xd0020000 0x258>; > }; > + > + sata@d00a0000 { > + compatible = "marvell,orion-sata"; > + reg = <0xd00a0000 0x2400>; > + interrupts = <55>; > + nr-ports = <2>; > + clocks = <&coreclk 0>;//, <&coreclk 0>; Alignment problem + remainings of tests or something like that. Thomas -- Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] arm: mvebu: adding SATA support: dt binding and config update 2012-10-24 14:01 ` Thomas Petazzoni @ 2012-10-24 14:05 ` Gregory CLEMENT 2012-10-25 13:18 ` Jason Cooper 0 siblings, 1 reply; 21+ messages in thread From: Gregory CLEMENT @ 2012-10-24 14:05 UTC (permalink / raw) To: Thomas Petazzoni Cc: Jason Cooper, Andrew Lunn, linux-arm-kernel, Arnd Bergmann, Olof Johansson, Ben Dooks, Ian Molton, Nicolas Pitre, Lior Amsalem, Maen Suleiman, Tawfik Bayouk, Shadi Ammouri, Eran Ben-Avi, Yehuda Yitschak, Nadav Haklai, Ike Pan, Jani Monoses, Chris Van Hoof, Dan Frazier, Leif Lindholm, Jon Masters, David Marlin, Sebastian Hesselbarth, linux-kernel On 10/24/2012 04:01 PM, Thomas Petazzoni wrote: > Hello, > > Shouldn't you split into one commit adding the SATA definition in > the .dtsi + doing the defconfig change (the "SoC" level modifications), > and then another commit for the .dts change? I don't really care > personally, it's really up to Jason/Andrew on this. > > Another comment below, though. > > On Wed, 24 Oct 2012 15:49:21 +0200, Gregory CLEMENT wrote: > >> diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi >> index 94b4b9e..3f08233 100644 >> --- a/arch/arm/boot/dts/armada-370-xp.dtsi >> +++ b/arch/arm/boot/dts/armada-370-xp.dtsi >> @@ -69,6 +69,16 @@ >> compatible = "marvell,armada-addr-decoding-controller"; >> reg = <0xd0020000 0x258>; >> }; >> + >> + sata@d00a0000 { >> + compatible = "marvell,orion-sata"; >> + reg = <0xd00a0000 0x2400>; >> + interrupts = <55>; >> + nr-ports = <2>; >> + clocks = <&coreclk 0>;//, <&coreclk 0>; > > Alignment problem + remainings of tests or something like that. True I missed this one. Jason, Andrew, do you want I split this patch as suggested by Thomas or are you fine with having one single patch? I will wait your answer before sending the V2. Thanks, Gregory ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] arm: mvebu: adding SATA support: dt binding and config update 2012-10-24 14:05 ` Gregory CLEMENT @ 2012-10-25 13:18 ` Jason Cooper 2012-10-25 13:21 ` Thomas Petazzoni 2012-10-25 13:53 ` Rob Herring 0 siblings, 2 replies; 21+ messages in thread From: Jason Cooper @ 2012-10-25 13:18 UTC (permalink / raw) To: Gregory CLEMENT Cc: Thomas Petazzoni, Andrew Lunn, linux-arm-kernel, Arnd Bergmann, Olof Johansson, Ben Dooks, Ian Molton, Nicolas Pitre, Lior Amsalem, Maen Suleiman, Tawfik Bayouk, Shadi Ammouri, Eran Ben-Avi, Yehuda Yitschak, Nadav Haklai, Ike Pan, Jani Monoses, Chris Van Hoof, Dan Frazier, Leif Lindholm, Jon Masters, David Marlin, Sebastian Hesselbarth, linux-kernel On Wed, Oct 24, 2012 at 04:05:45PM +0200, Gregory CLEMENT wrote: > On 10/24/2012 04:01 PM, Thomas Petazzoni wrote: > > Hello, > > > > Shouldn't you split into one commit adding the SATA definition in > > the .dtsi + doing the defconfig change (the "SoC" level modifications), > > and then another commit for the .dts change? I don't really care > > personally, it's really up to Jason/Andrew on this. > > > > Another comment below, though. > > > > On Wed, 24 Oct 2012 15:49:21 +0200, Gregory CLEMENT wrote: > > > >> diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi > >> index 94b4b9e..3f08233 100644 > >> --- a/arch/arm/boot/dts/armada-370-xp.dtsi > >> +++ b/arch/arm/boot/dts/armada-370-xp.dtsi > >> @@ -69,6 +69,16 @@ > >> compatible = "marvell,armada-addr-decoding-controller"; > >> reg = <0xd0020000 0x258>; > >> }; > >> + > >> + sata@d00a0000 { > >> + compatible = "marvell,orion-sata"; > >> + reg = <0xd00a0000 0x2400>; > >> + interrupts = <55>; > >> + nr-ports = <2>; > >> + clocks = <&coreclk 0>;//, <&coreclk 0>; > > > > Alignment problem + remainings of tests or something like that. > > True I missed this one. > > Jason, Andrew, do you want I split this patch as suggested by Thomas or > are you fine with having one single patch? Yes, please make the defconfig changes a separate patch. Also, please make sure only the minimum is enabled (eq RAID... isn't needed). thx, Jason. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] arm: mvebu: adding SATA support: dt binding and config update 2012-10-25 13:18 ` Jason Cooper @ 2012-10-25 13:21 ` Thomas Petazzoni 2012-10-25 13:34 ` Gregory CLEMENT 2012-10-25 13:35 ` Jason Cooper 2012-10-25 13:53 ` Rob Herring 1 sibling, 2 replies; 21+ messages in thread From: Thomas Petazzoni @ 2012-10-25 13:21 UTC (permalink / raw) To: Jason Cooper Cc: Gregory CLEMENT, Andrew Lunn, linux-arm-kernel, Arnd Bergmann, Olof Johansson, Ben Dooks, Ian Molton, Nicolas Pitre, Lior Amsalem, Maen Suleiman, Tawfik Bayouk, Shadi Ammouri, Eran Ben-Avi, Yehuda Yitschak, Nadav Haklai, Ike Pan, Jani Monoses, Chris Van Hoof, Dan Frazier, Leif Lindholm, Jon Masters, David Marlin, Sebastian Hesselbarth, linux-kernel Jason, On Thu, 25 Oct 2012 09:18:18 -0400, Jason Cooper wrote: > > Jason, Andrew, do you want I split this patch as suggested by > > Thomas or are you fine with having one single patch? > > Yes, please make the defconfig changes a separate patch. Also, please > make sure only the minimum is enabled (eq RAID... isn't needed). I haven't looked in details at the driver, but is nr-ports = <foo> the right way of doing things? We may have platforms were port 0 is not used, but port 1 is used, and just a number of ports doesn't allow to express this. Shouldn't the DT property be ports = <0>, <1> ports = <1> ports = <1>, <3> In order to allow to more precisely enabled SATA ports? Or maybe the SATA ports cannot be enabled/disabled on a per-port basis, in which case I'm obviously wrong here. Best regards, Thomas -- Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] arm: mvebu: adding SATA support: dt binding and config update 2012-10-25 13:21 ` Thomas Petazzoni @ 2012-10-25 13:34 ` Gregory CLEMENT 2012-10-25 13:57 ` Rob Herring 2012-10-25 13:35 ` Jason Cooper 1 sibling, 1 reply; 21+ messages in thread From: Gregory CLEMENT @ 2012-10-25 13:34 UTC (permalink / raw) To: Thomas Petazzoni Cc: Jason Cooper, Andrew Lunn, linux-arm-kernel, Arnd Bergmann, Olof Johansson, Ben Dooks, Ian Molton, Nicolas Pitre, Lior Amsalem, Maen Suleiman, Tawfik Bayouk, Shadi Ammouri, Eran Ben-Avi, Yehuda Yitschak, Nadav Haklai, Ike Pan, Jani Monoses, Chris Van Hoof, Dan Frazier, Leif Lindholm, Jon Masters, David Marlin, Sebastian Hesselbarth, linux-kernel On 10/25/2012 03:21 PM, Thomas Petazzoni wrote: > Jason, > > On Thu, 25 Oct 2012 09:18:18 -0400, Jason Cooper wrote: > >>> Jason, Andrew, do you want I split this patch as suggested by >>> Thomas or are you fine with having one single patch? >> >> Yes, please make the defconfig changes a separate patch. Also, please >> make sure only the minimum is enabled (eq RAID... isn't needed). > > I haven't looked in details at the driver, but is nr-ports = <foo> the > right way of doing things? We may have platforms were port 0 is not > used, but port 1 is used, and just a number of ports doesn't allow to > express this. > > Shouldn't the DT property be > > ports = <0>, <1> > ports = <1> > ports = <1>, <3> > > In order to allow to more precisely enabled SATA ports? Or maybe the > SATA ports cannot be enabled/disabled on a per-port basis, in which > case I'm obviously wrong here. The actual implementation of mv_sata.c doesn't work like this. You can only pass the number of ports supported not the list of the port you want to support. I've checked in the device tree binding documentation _and_ also in the code. > > Best regards, > > Thomas > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] arm: mvebu: adding SATA support: dt binding and config update 2012-10-25 13:34 ` Gregory CLEMENT @ 2012-10-25 13:57 ` Rob Herring 2012-10-25 16:00 ` Gregory CLEMENT 0 siblings, 1 reply; 21+ messages in thread From: Rob Herring @ 2012-10-25 13:57 UTC (permalink / raw) To: Gregory CLEMENT Cc: Thomas Petazzoni, Lior Amsalem, Andrew Lunn, Ike Pan, Nadav Haklai, Ian Molton, David Marlin, Yehuda Yitschak, Jani Monoses, Tawfik Bayouk, Dan Frazier, Eran Ben-Avi, Leif Lindholm, Sebastian Hesselbarth, Jason Cooper, Arnd Bergmann, Jon Masters, Ben Dooks, linux-arm-kernel, Chris Van Hoof, Nicolas Pitre, linux-kernel, Maen Suleiman, Shadi Ammouri, Olof Johansson On 10/25/2012 08:34 AM, Gregory CLEMENT wrote: > On 10/25/2012 03:21 PM, Thomas Petazzoni wrote: >> Jason, >> >> On Thu, 25 Oct 2012 09:18:18 -0400, Jason Cooper wrote: >> >>>> Jason, Andrew, do you want I split this patch as suggested by >>>> Thomas or are you fine with having one single patch? >>> >>> Yes, please make the defconfig changes a separate patch. Also, please >>> make sure only the minimum is enabled (eq RAID... isn't needed). >> >> I haven't looked in details at the driver, but is nr-ports = <foo> the >> right way of doing things? We may have platforms were port 0 is not >> used, but port 1 is used, and just a number of ports doesn't allow to >> express this. >> >> Shouldn't the DT property be >> >> ports = <0>, <1> >> ports = <1> >> ports = <1>, <3> >> >> In order to allow to more precisely enabled SATA ports? Or maybe the >> SATA ports cannot be enabled/disabled on a per-port basis, in which >> case I'm obviously wrong here. > > The actual implementation of mv_sata.c doesn't work like this. You can > only pass the number of ports supported not the list of the port you > want to support. I've checked in the device tree binding documentation > _and_ also in the code. Is that a statement about the driver or the h/w? It does not matter what the driver does. If the h/w can support skipping a port, then the dts should allow that. A bitmask would be most appropriate here (and matches how AHCI does the equivalent). Rob ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] arm: mvebu: adding SATA support: dt binding and config update 2012-10-25 13:57 ` Rob Herring @ 2012-10-25 16:00 ` Gregory CLEMENT 0 siblings, 0 replies; 21+ messages in thread From: Gregory CLEMENT @ 2012-10-25 16:00 UTC (permalink / raw) To: Rob Herring Cc: Lior Amsalem, Andrew Lunn, Ike Pan, Nadav Haklai, Ian Molton, David Marlin, Yehuda Yitschak, Jani Monoses, Tawfik Bayouk, Dan Frazier, Eran Ben-Avi, Leif Lindholm, Sebastian Hesselbarth, Jason Cooper, Arnd Bergmann, Jon Masters, Ben Dooks, linux-arm-kernel, Thomas Petazzoni, Chris Van Hoof, Nicolas Pitre, linux-kernel, Maen Suleiman, Shadi Ammouri, Olof Johansson On 10/25/2012 03:57 PM, Rob Herring wrote: > On 10/25/2012 08:34 AM, Gregory CLEMENT wrote: >> On 10/25/2012 03:21 PM, Thomas Petazzoni wrote: >>> Jason, >>> >>> On Thu, 25 Oct 2012 09:18:18 -0400, Jason Cooper wrote: >>> >>>>> Jason, Andrew, do you want I split this patch as suggested by >>>>> Thomas or are you fine with having one single patch? >>>> >>>> Yes, please make the defconfig changes a separate patch. Also, please >>>> make sure only the minimum is enabled (eq RAID... isn't needed). >>> >>> I haven't looked in details at the driver, but is nr-ports = <foo> the >>> right way of doing things? We may have platforms were port 0 is not >>> used, but port 1 is used, and just a number of ports doesn't allow to >>> express this. >>> >>> Shouldn't the DT property be >>> >>> ports = <0>, <1> >>> ports = <1> >>> ports = <1>, <3> >>> >>> In order to allow to more precisely enabled SATA ports? Or maybe the >>> SATA ports cannot be enabled/disabled on a per-port basis, in which >>> case I'm obviously wrong here. >> >> The actual implementation of mv_sata.c doesn't work like this. You can >> only pass the number of ports supported not the list of the port you >> want to support. I've checked in the device tree binding documentation >> _and_ also in the code. > > Is that a statement about the driver or the h/w? It does not matter what > the driver does. If the h/w can support skipping a port, then the dts > should allow that. Indeed I didn't see anything in the datasheet which would prevent to use only port 2. I agree to add this feature if needed, but currently there is no Marvell based board whith this kind of configuration. Can we keep this series as a simple series to enable SATA on Armada 370/XP, and prepare an other series for this new feature? > > A bitmask would be most appropriate here (and matches how AHCI does the > equivalent). I didn't see any bitmask for AHCI, just an optional list of phandle on combophy. > > Rob > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] arm: mvebu: adding SATA support: dt binding and config update 2012-10-25 13:21 ` Thomas Petazzoni 2012-10-25 13:34 ` Gregory CLEMENT @ 2012-10-25 13:35 ` Jason Cooper 1 sibling, 0 replies; 21+ messages in thread From: Jason Cooper @ 2012-10-25 13:35 UTC (permalink / raw) To: Thomas Petazzoni Cc: Gregory CLEMENT, Andrew Lunn, linux-arm-kernel, Arnd Bergmann, Olof Johansson, Ben Dooks, Ian Molton, Nicolas Pitre, Lior Amsalem, Maen Suleiman, Tawfik Bayouk, Shadi Ammouri, Eran Ben-Avi, Yehuda Yitschak, Nadav Haklai, Ike Pan, Jani Monoses, Chris Van Hoof, Dan Frazier, Leif Lindholm, Jon Masters, David Marlin, Sebastian Hesselbarth, linux-kernel On Thu, Oct 25, 2012 at 03:21:14PM +0200, Thomas Petazzoni wrote: > Jason, > > On Thu, 25 Oct 2012 09:18:18 -0400, Jason Cooper wrote: > > > > Jason, Andrew, do you want I split this patch as suggested by > > > Thomas or are you fine with having one single patch? > > > > Yes, please make the defconfig changes a separate patch. Also, please > > make sure only the minimum is enabled (eq RAID... isn't needed). > > I haven't looked in details at the driver, but is nr-ports = <foo> the > right way of doing things? We may have platforms were port 0 is not > used, but port 1 is used, and just a number of ports doesn't allow to > express this. Do you have an example of this? thx, Jason. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] arm: mvebu: adding SATA support: dt binding and config update 2012-10-25 13:18 ` Jason Cooper 2012-10-25 13:21 ` Thomas Petazzoni @ 2012-10-25 13:53 ` Rob Herring 2012-10-25 14:11 ` Gregory CLEMENT 1 sibling, 1 reply; 21+ messages in thread From: Rob Herring @ 2012-10-25 13:53 UTC (permalink / raw) To: Jason Cooper Cc: Gregory CLEMENT, Lior Amsalem, Andrew Lunn, Ike Pan, Nadav Haklai, Ian Molton, David Marlin, Yehuda Yitschak, Jani Monoses, Tawfik Bayouk, Dan Frazier, Eran Ben-Avi, Leif Lindholm, Sebastian Hesselbarth, Arnd Bergmann, Jon Masters, Ben Dooks, linux-arm-kernel, Thomas Petazzoni, Chris Van Hoof, Nicolas Pitre, linux-kernel, Maen Suleiman, Shadi Ammouri, Olof Johansson On 10/25/2012 08:18 AM, Jason Cooper wrote: > On Wed, Oct 24, 2012 at 04:05:45PM +0200, Gregory CLEMENT wrote: >> On 10/24/2012 04:01 PM, Thomas Petazzoni wrote: >>> Hello, >>> >>> Shouldn't you split into one commit adding the SATA definition in >>> the .dtsi + doing the defconfig change (the "SoC" level modifications), >>> and then another commit for the .dts change? I don't really care >>> personally, it's really up to Jason/Andrew on this. >>> >>> Another comment below, though. >>> >>> On Wed, 24 Oct 2012 15:49:21 +0200, Gregory CLEMENT wrote: >>> >>>> diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi >>>> index 94b4b9e..3f08233 100644 >>>> --- a/arch/arm/boot/dts/armada-370-xp.dtsi >>>> +++ b/arch/arm/boot/dts/armada-370-xp.dtsi >>>> @@ -69,6 +69,16 @@ >>>> compatible = "marvell,armada-addr-decoding-controller"; >>>> reg = <0xd0020000 0x258>; >>>> }; >>>> + >>>> + sata@d00a0000 { >>>> + compatible = "marvell,orion-sata"; >>>> + reg = <0xd00a0000 0x2400>; >>>> + interrupts = <55>; >>>> + nr-ports = <2>; >>>> + clocks = <&coreclk 0>;//, <&coreclk 0>; >>> >>> Alignment problem + remainings of tests or something like that. >> >> True I missed this one. >> >> Jason, Andrew, do you want I split this patch as suggested by Thomas or >> are you fine with having one single patch? > > Yes, please make the defconfig changes a separate patch. Also, please > make sure only the minimum is enabled (eq RAID... isn't needed). What about updating multi_v7_defconfig instead? Rob ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] arm: mvebu: adding SATA support: dt binding and config update 2012-10-25 13:53 ` Rob Herring @ 2012-10-25 14:11 ` Gregory CLEMENT 2012-10-25 14:27 ` Rob Herring 0 siblings, 1 reply; 21+ messages in thread From: Gregory CLEMENT @ 2012-10-25 14:11 UTC (permalink / raw) To: Rob Herring Cc: Jason Cooper, Lior Amsalem, Andrew Lunn, Ike Pan, Nadav Haklai, Ian Molton, David Marlin, Yehuda Yitschak, Jani Monoses, Tawfik Bayouk, Dan Frazier, Eran Ben-Avi, Leif Lindholm, Sebastian Hesselbarth, Arnd Bergmann, Jon Masters, Ben Dooks, linux-arm-kernel, Thomas Petazzoni, Chris Van Hoof, Nicolas Pitre, linux-kernel, Maen Suleiman, Shadi Ammouri, Olof Johansson On 10/25/2012 03:53 PM, Rob Herring wrote: > On 10/25/2012 08:18 AM, Jason Cooper wrote: >> On Wed, Oct 24, 2012 at 04:05:45PM +0200, Gregory CLEMENT wrote: >>> On 10/24/2012 04:01 PM, Thomas Petazzoni wrote: >>>> Hello, >>>> >>>> Shouldn't you split into one commit adding the SATA definition in >>>> the .dtsi + doing the defconfig change (the "SoC" level modifications), >>>> and then another commit for the .dts change? I don't really care >>>> personally, it's really up to Jason/Andrew on this. >>>> >>>> Another comment below, though. >>>> >>>> On Wed, 24 Oct 2012 15:49:21 +0200, Gregory CLEMENT wrote: >>>> >>>>> diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi >>>>> index 94b4b9e..3f08233 100644 >>>>> --- a/arch/arm/boot/dts/armada-370-xp.dtsi >>>>> +++ b/arch/arm/boot/dts/armada-370-xp.dtsi >>>>> @@ -69,6 +69,16 @@ >>>>> compatible = "marvell,armada-addr-decoding-controller"; >>>>> reg = <0xd0020000 0x258>; >>>>> }; >>>>> + >>>>> + sata@d00a0000 { >>>>> + compatible = "marvell,orion-sata"; >>>>> + reg = <0xd00a0000 0x2400>; >>>>> + interrupts = <55>; >>>>> + nr-ports = <2>; >>>>> + clocks = <&coreclk 0>;//, <&coreclk 0>; >>>> >>>> Alignment problem + remainings of tests or something like that. >>> >>> True I missed this one. >>> >>> Jason, Andrew, do you want I split this patch as suggested by Thomas or >>> are you fine with having one single patch? >> >> Yes, please make the defconfig changes a separate patch. Also, please >> make sure only the minimum is enabled (eq RAID... isn't needed). > > What about updating multi_v7_defconfig instead? About the _instead_, when I proposed to removed mvebu_defconfig Thomas argued that it was more convenient to build a mvebu-only kernel when doing kernel development, and Andrew also pointed that this defconfig should be useful for kisskb. And about updated both mvebu_defconfig and multi_v7_defconfig, I'm fine with it. Gregory ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] arm: mvebu: adding SATA support: dt binding and config update 2012-10-25 14:11 ` Gregory CLEMENT @ 2012-10-25 14:27 ` Rob Herring 0 siblings, 0 replies; 21+ messages in thread From: Rob Herring @ 2012-10-25 14:27 UTC (permalink / raw) To: Gregory CLEMENT Cc: Jason Cooper, Lior Amsalem, Andrew Lunn, Ike Pan, Nadav Haklai, Ian Molton, David Marlin, Yehuda Yitschak, Jani Monoses, Tawfik Bayouk, Dan Frazier, Eran Ben-Avi, Leif Lindholm, Sebastian Hesselbarth, Arnd Bergmann, Jon Masters, Ben Dooks, linux-arm-kernel, Thomas Petazzoni, Chris Van Hoof, Nicolas Pitre, linux-kernel, Maen Suleiman, Shadi Ammouri, Olof Johansson On 10/25/2012 09:11 AM, Gregory CLEMENT wrote: > On 10/25/2012 03:53 PM, Rob Herring wrote: >> On 10/25/2012 08:18 AM, Jason Cooper wrote: >>> On Wed, Oct 24, 2012 at 04:05:45PM +0200, Gregory CLEMENT wrote: >>>> On 10/24/2012 04:01 PM, Thomas Petazzoni wrote: >>>>> Hello, >>>>> >>>>> Shouldn't you split into one commit adding the SATA definition in >>>>> the .dtsi + doing the defconfig change (the "SoC" level modifications), >>>>> and then another commit for the .dts change? I don't really care >>>>> personally, it's really up to Jason/Andrew on this. >>>>> >>>>> Another comment below, though. >>>>> >>>>> On Wed, 24 Oct 2012 15:49:21 +0200, Gregory CLEMENT wrote: >>>>> >>>>>> diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi >>>>>> index 94b4b9e..3f08233 100644 >>>>>> --- a/arch/arm/boot/dts/armada-370-xp.dtsi >>>>>> +++ b/arch/arm/boot/dts/armada-370-xp.dtsi >>>>>> @@ -69,6 +69,16 @@ >>>>>> compatible = "marvell,armada-addr-decoding-controller"; >>>>>> reg = <0xd0020000 0x258>; >>>>>> }; >>>>>> + >>>>>> + sata@d00a0000 { >>>>>> + compatible = "marvell,orion-sata"; >>>>>> + reg = <0xd00a0000 0x2400>; >>>>>> + interrupts = <55>; >>>>>> + nr-ports = <2>; >>>>>> + clocks = <&coreclk 0>;//, <&coreclk 0>; >>>>> >>>>> Alignment problem + remainings of tests or something like that. >>>> >>>> True I missed this one. >>>> >>>> Jason, Andrew, do you want I split this patch as suggested by Thomas or >>>> are you fine with having one single patch? >>> >>> Yes, please make the defconfig changes a separate patch. Also, please >>> make sure only the minimum is enabled (eq RAID... isn't needed). >> >> What about updating multi_v7_defconfig instead? > > About the _instead_, when I proposed to removed mvebu_defconfig Thomas > argued that it was more convenient to build a mvebu-only kernel when > doing kernel development, and Andrew also pointed that this defconfig > should be useful for kisskb. Okay, missed that discussion. > And about updated both mvebu_defconfig and multi_v7_defconfig, I'm > fine with it. Yes, then please update both. Rob ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] arm: mvebu: adding SATA support: dt binding and config update 2012-10-24 13:49 ` [PATCH 2/2] arm: mvebu: adding SATA support: dt binding and config update Gregory CLEMENT 2012-10-24 14:01 ` Thomas Petazzoni @ 2012-10-24 14:08 ` Andrew Lunn 2012-10-24 14:45 ` Gregory CLEMENT 1 sibling, 1 reply; 21+ messages in thread From: Andrew Lunn @ 2012-10-24 14:08 UTC (permalink / raw) To: Gregory CLEMENT Cc: Jason Cooper, Andrew Lunn, linux-arm-kernel, Arnd Bergmann, Olof Johansson, Ben Dooks, Ian Molton, Nicolas Pitre, Lior Amsalem, Maen Suleiman, Tawfik Bayouk, Shadi Ammouri, Eran Ben-Avi, Yehuda Yitschak, Nadav Haklai, Ike Pan, Jani Monoses, Chris Van Hoof, Dan Frazier, Thomas Petazzoni, Leif Lindholm, Jon Masters, David Marlin, Sebastian Hesselbarth, linux-kernel On Wed, Oct 24, 2012 at 03:49:21PM +0200, Gregory CLEMENT wrote: > From: Lior Amsalem <alior@marvell.com> > > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > Signed-off-by: Lior Amsalem <alior@marvell.com> > --- > arch/arm/boot/dts/armada-370-db.dts | 3 +++ > arch/arm/boot/dts/armada-370-xp.dtsi | 10 ++++++++++ > arch/arm/boot/dts/armada-xp-db.dts | 3 +++ > arch/arm/configs/mvebu_defconfig | 7 +++++++ > 4 files changed, 23 insertions(+) > > diff --git a/arch/arm/boot/dts/armada-370-db.dts b/arch/arm/boot/dts/armada-370-db.dts > index 4a31b03..2a2aa75 100644 > --- a/arch/arm/boot/dts/armada-370-db.dts > +++ b/arch/arm/boot/dts/armada-370-db.dts > @@ -34,5 +34,8 @@ > clock-frequency = <200000000>; > status = "okay"; > }; > + sata@d00a0000 { > + status = "okay"; > + }; > }; > }; > diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi > index 94b4b9e..3f08233 100644 > --- a/arch/arm/boot/dts/armada-370-xp.dtsi > +++ b/arch/arm/boot/dts/armada-370-xp.dtsi > @@ -69,6 +69,16 @@ > compatible = "marvell,armada-addr-decoding-controller"; > reg = <0xd0020000 0x258>; > }; > + > + sata@d00a0000 { > + compatible = "marvell,orion-sata"; > + reg = <0xd00a0000 0x2400>; > + interrupts = <55>; > + nr-ports = <2>; > + clocks = <&coreclk 0>;//, <&coreclk 0>; > + status = "disabled"; > + }; > + > }; > }; > > diff --git a/arch/arm/boot/dts/armada-xp-db.dts b/arch/arm/boot/dts/armada-xp-db.dts > index b1fc728..b0db9a3 100644 > --- a/arch/arm/boot/dts/armada-xp-db.dts > +++ b/arch/arm/boot/dts/armada-xp-db.dts > @@ -46,5 +46,8 @@ > clock-frequency = <250000000>; > status = "okay"; > }; > + sata@d00a0000 { > + status = "okay"; > + }; > }; > }; Hi Gregory Should there be some pinctrl setup somewhere, to ensure the pins used for SATA are really setup up for SATA? Also, for kirkwood, the number of SATA ports varies. So we don't define it in the base kirkwood.dtsi and leave each board to set it. Do we want to be consistent between kirkwood and 370/xp? Thanks Andrew ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] arm: mvebu: adding SATA support: dt binding and config update 2012-10-24 14:08 ` Andrew Lunn @ 2012-10-24 14:45 ` Gregory CLEMENT 0 siblings, 0 replies; 21+ messages in thread From: Gregory CLEMENT @ 2012-10-24 14:45 UTC (permalink / raw) To: Andrew Lunn Cc: Lior Amsalem, Ike Pan, Nadav Haklai, Ian Molton, David Marlin, Yehuda Yitschak, Jani Monoses, Tawfik Bayouk, Dan Frazier, Eran Ben-Avi, Leif Lindholm, Sebastian Hesselbarth, Jason Cooper, Arnd Bergmann, Jon Masters, Ben Dooks, linux-arm-kernel, Thomas Petazzoni, Chris Van Hoof, Nicolas Pitre, linux-kernel, Maen Suleiman, Shadi Ammouri, Olof Johansson On 10/24/2012 04:08 PM, Andrew Lunn wrote: > On Wed, Oct 24, 2012 at 03:49:21PM +0200, Gregory CLEMENT wrote: >> From: Lior Amsalem <alior@marvell.com> >> >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> >> Signed-off-by: Lior Amsalem <alior@marvell.com> >> --- >> arch/arm/boot/dts/armada-370-db.dts | 3 +++ >> arch/arm/boot/dts/armada-370-xp.dtsi | 10 ++++++++++ >> arch/arm/boot/dts/armada-xp-db.dts | 3 +++ >> arch/arm/configs/mvebu_defconfig | 7 +++++++ >> 4 files changed, 23 insertions(+) >> >> diff --git a/arch/arm/boot/dts/armada-370-db.dts b/arch/arm/boot/dts/armada-370-db.dts >> index 4a31b03..2a2aa75 100644 >> --- a/arch/arm/boot/dts/armada-370-db.dts >> +++ b/arch/arm/boot/dts/armada-370-db.dts >> @@ -34,5 +34,8 @@ >> clock-frequency = <200000000>; >> status = "okay"; >> }; >> + sata@d00a0000 { >> + status = "okay"; >> + }; >> }; >> }; >> diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi >> index 94b4b9e..3f08233 100644 >> --- a/arch/arm/boot/dts/armada-370-xp.dtsi >> +++ b/arch/arm/boot/dts/armada-370-xp.dtsi >> @@ -69,6 +69,16 @@ >> compatible = "marvell,armada-addr-decoding-controller"; >> reg = <0xd0020000 0x258>; >> }; >> + >> + sata@d00a0000 { >> + compatible = "marvell,orion-sata"; >> + reg = <0xd00a0000 0x2400>; >> + interrupts = <55>; >> + nr-ports = <2>; >> + clocks = <&coreclk 0>;//, <&coreclk 0>; >> + status = "disabled"; >> + }; >> + >> }; >> }; >> >> diff --git a/arch/arm/boot/dts/armada-xp-db.dts b/arch/arm/boot/dts/armada-xp-db.dts >> index b1fc728..b0db9a3 100644 >> --- a/arch/arm/boot/dts/armada-xp-db.dts >> +++ b/arch/arm/boot/dts/armada-xp-db.dts >> @@ -46,5 +46,8 @@ >> clock-frequency = <250000000>; >> status = "okay"; >> }; >> + sata@d00a0000 { >> + status = "okay"; >> + }; >> }; >> }; > > Hi Gregory > > Should there be some pinctrl setup somewhere, to ensure the pins used > for SATA are really setup up for SATA? Yes you're right we should not depend of the bootloader configuration. > > Also, for kirkwood, the number of SATA ports varies. So we don't > define it in the base kirkwood.dtsi and leave each board to set it. > Do we want to be consistent between kirkwood and 370/xp? Yes sure. I will move it from dtsi to dts. > > Thanks > Andrew > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2012-10-25 16:01 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-10-24 13:49 [PATCH 0/2] Adding SATA support for Armada 370/XP Gregory CLEMENT 2012-10-24 13:49 ` [PATCH 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP Gregory CLEMENT 2012-10-25 5:29 ` Marek Szyprowski 2012-10-25 11:27 ` Arnd Bergmann 2012-10-25 11:43 ` Thomas Petazzoni 2012-10-25 13:46 ` Arnd Bergmann 2012-10-25 14:09 ` Thomas Petazzoni 2012-10-24 13:49 ` [PATCH 2/2] arm: mvebu: adding SATA support: dt binding and config update Gregory CLEMENT 2012-10-24 14:01 ` Thomas Petazzoni 2012-10-24 14:05 ` Gregory CLEMENT 2012-10-25 13:18 ` Jason Cooper 2012-10-25 13:21 ` Thomas Petazzoni 2012-10-25 13:34 ` Gregory CLEMENT 2012-10-25 13:57 ` Rob Herring 2012-10-25 16:00 ` Gregory CLEMENT 2012-10-25 13:35 ` Jason Cooper 2012-10-25 13:53 ` Rob Herring 2012-10-25 14:11 ` Gregory CLEMENT 2012-10-25 14:27 ` Rob Herring 2012-10-24 14:08 ` Andrew Lunn 2012-10-24 14:45 ` Gregory CLEMENT
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).