| Message ID | 201302272339.15402.sergei.shtylyov@cogentembedded.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Feb 27, 2013 at 11:39:14PM +0300, Sergei Shtylyov wrote: > From: Vladimir Barinov <vladimir.barinov@cogentembedded.com> > > Add SATA clock for r8a7779 SoC (for both device tree and usual cases). > Register SATA controller as a "late" platform device on r8a7779 SoC. Hi, If you have the a binding in the device tree (which you do through patch 1/3), then there's no reason to have a platform device for it. -Olof
On Thu, Feb 28, 2013 at 05:41:48PM -0800, Olof Johansson wrote: > On Wed, Feb 27, 2013 at 11:39:14PM +0300, Sergei Shtylyov wrote: > > From: Vladimir Barinov <vladimir.barinov@cogentembedded.com> > > > > Add SATA clock for r8a7779 SoC (for both device tree and usual cases). > > Register SATA controller as a "late" platform device on r8a7779 SoC. > > Hi, > > If you have the a binding in the device tree (which you do through patch 1/3), > then there's no reason to have a platform device for it. Hi Olof, the DT exists but currently the marzen board brings up all of its devices using platform devices. Which if nothing else is internally consistent. I suppose it would be possible to add a call to r8a7779_add_standard_devices_dt() and have the board bring up this device using DT and the rest using platform drivers (until the drivers are migrated to DT). Would that be your preferred option? Magnus, how do you feel about this idea?
On Fri, Mar 01, 2013 at 04:23:45PM +0900, Simon Horman wrote: > On Thu, Feb 28, 2013 at 05:41:48PM -0800, Olof Johansson wrote: > > On Wed, Feb 27, 2013 at 11:39:14PM +0300, Sergei Shtylyov wrote: > > > From: Vladimir Barinov <vladimir.barinov@cogentembedded.com> > > > > > > Add SATA clock for r8a7779 SoC (for both device tree and usual cases). > > > Register SATA controller as a "late" platform device on r8a7779 SoC. > > > > Hi, > > > > If you have the a binding in the device tree (which you do through patch 1/3), > > then there's no reason to have a platform device for it. > > Hi Olof, > > the DT exists but currently the marzen board brings up all > of its devices using platform devices. Which if nothing else is > internally consistent. > > I suppose it would be possible to add a call to > r8a7779_add_standard_devices_dt() and have the board bring > up this device using DT and the rest using platform drivers > (until the drivers are migrated to DT). > > Would that be your preferred option? > > Magnus, how do you feel about this idea? Ping.
On Mon, Mar 4, 2013 at 4:26 PM, Simon Horman <horms@verge.net.au> wrote: > On Fri, Mar 01, 2013 at 04:23:45PM +0900, Simon Horman wrote: >> On Thu, Feb 28, 2013 at 05:41:48PM -0800, Olof Johansson wrote: >> > On Wed, Feb 27, 2013 at 11:39:14PM +0300, Sergei Shtylyov wrote: >> > > From: Vladimir Barinov <vladimir.barinov@cogentembedded.com> >> > > >> > > Add SATA clock for r8a7779 SoC (for both device tree and usual cases). >> > > Register SATA controller as a "late" platform device on r8a7779 SoC. >> > >> > Hi, >> > >> > If you have the a binding in the device tree (which you do through patch 1/3), >> > then there's no reason to have a platform device for it. >> >> Hi Olof, >> >> the DT exists but currently the marzen board brings up all >> of its devices using platform devices. Which if nothing else is >> internally consistent. >> >> I suppose it would be possible to add a call to >> r8a7779_add_standard_devices_dt() and have the board bring >> up this device using DT and the rest using platform drivers >> (until the drivers are migrated to DT). >> >> Would that be your preferred option? >> >> Magnus, how do you feel about this idea? > > Ping. Sorry, I know the ping wasn't directed at me, but I didn't reply yet either. :) This is generally the right migration model for platforms that have some regular platform devices today; register what isn't yet DT enabled through the old ways, but also probe device tree for the devices that are described there. You might need AUXDATA tables to get clock names right (since device names are different) until you have clock bindings, that's what other platforms have done as well. Over time you can migrate away from it and switch to only DT. -Olof
Hi Simon On Fri, Mar 1, 2013 at 4:23 PM, Simon Horman <horms@verge.net.au> wrote: > On Thu, Feb 28, 2013 at 05:41:48PM -0800, Olof Johansson wrote: >> On Wed, Feb 27, 2013 at 11:39:14PM +0300, Sergei Shtylyov wrote: >> > From: Vladimir Barinov <vladimir.barinov@cogentembedded.com> >> > >> > Add SATA clock for r8a7779 SoC (for both device tree and usual cases). >> > Register SATA controller as a "late" platform device on r8a7779 SoC. >> >> Hi, >> >> If you have the a binding in the device tree (which you do through patch 1/3), >> then there's no reason to have a platform device for it. > > Hi Olof, > > the DT exists but currently the marzen board brings up all > of its devices using platform devices. Which if nothing else is > internally consistent. > > I suppose it would be possible to add a call to > r8a7779_add_standard_devices_dt() and have the board bring > up this device using DT and the rest using platform drivers > (until the drivers are migrated to DT). > > Would that be your preferred option? > > Magnus, how do you feel about this idea? I fail to see the upside I'm afraid. I believe the best way forward is for us to keep on following the same style as we have done so far. So for drivers they should begin by interfacing as regular platform devices drivers and then add DT support incrementally. This allows us to try out things in a non-ABI kind of way and it also allows us to add support for various hardware feature that do not yet support DT like in the case of DMA Engine. With platform devices we can also use the driver with the SH architecture. You can of course do some mixed DT and platform device support on a board-level, but I don't really see what would be good about it. Actually, I believe that will just make things overly complicated without giving us the possibility to try out certain experimental features with platform-device-only interface. Of course Simon, if you prefer to follow your proposal above then go ahead. However, please consider the case when this code will be back ported to LTSI-3.4. Thanks, / magnus
On Mon, Mar 04, 2013 at 05:05:35PM -0800, Olof Johansson wrote: > On Mon, Mar 4, 2013 at 4:26 PM, Simon Horman <horms@verge.net.au> wrote: > > On Fri, Mar 01, 2013 at 04:23:45PM +0900, Simon Horman wrote: > >> On Thu, Feb 28, 2013 at 05:41:48PM -0800, Olof Johansson wrote: > >> > On Wed, Feb 27, 2013 at 11:39:14PM +0300, Sergei Shtylyov wrote: > >> > > From: Vladimir Barinov <vladimir.barinov@cogentembedded.com> > >> > > > >> > > Add SATA clock for r8a7779 SoC (for both device tree and usual cases). > >> > > Register SATA controller as a "late" platform device on r8a7779 SoC. > >> > > >> > Hi, > >> > > >> > If you have the a binding in the device tree (which you do through patch 1/3), > >> > then there's no reason to have a platform device for it. > >> > >> Hi Olof, > >> > >> the DT exists but currently the marzen board brings up all > >> of its devices using platform devices. Which if nothing else is > >> internally consistent. > >> > >> I suppose it would be possible to add a call to > >> r8a7779_add_standard_devices_dt() and have the board bring > >> up this device using DT and the rest using platform drivers > >> (until the drivers are migrated to DT). > >> > >> Would that be your preferred option? > >> > >> Magnus, how do you feel about this idea? > > > > Ping. > > Sorry, I know the ping wasn't directed at me, but I didn't reply yet either. :) > > This is generally the right migration model for platforms that have > some regular platform devices today; register what isn't yet DT > enabled through the old ways, but also probe device tree for the > devices that are described there. > > You might need AUXDATA tables to get clock names right (since device > names are different) until you have clock bindings, that's what other > platforms have done as well. Over time you can migrate away from it > and switch to only DT. Thanks. Unless Magnus has an objection I will make it so.
On Tue, Mar 05, 2013 at 10:28:09AM +0900, Magnus Damm wrote: > Hi Simon > > On Fri, Mar 1, 2013 at 4:23 PM, Simon Horman <horms@verge.net.au> wrote: > > On Thu, Feb 28, 2013 at 05:41:48PM -0800, Olof Johansson wrote: > >> On Wed, Feb 27, 2013 at 11:39:14PM +0300, Sergei Shtylyov wrote: > >> > From: Vladimir Barinov <vladimir.barinov@cogentembedded.com> > >> > > >> > Add SATA clock for r8a7779 SoC (for both device tree and usual cases). > >> > Register SATA controller as a "late" platform device on r8a7779 SoC. > >> > >> Hi, > >> > >> If you have the a binding in the device tree (which you do through patch 1/3), > >> then there's no reason to have a platform device for it. > > > > Hi Olof, > > > > the DT exists but currently the marzen board brings up all > > of its devices using platform devices. Which if nothing else is > > internally consistent. > > > > I suppose it would be possible to add a call to > > r8a7779_add_standard_devices_dt() and have the board bring > > up this device using DT and the rest using platform drivers > > (until the drivers are migrated to DT). > > > > Would that be your preferred option? > > > > Magnus, how do you feel about this idea? > > I fail to see the upside I'm afraid. > > I believe the best way forward is for us to keep on following the same > style as we have done so far. So for drivers they should begin by > interfacing as regular platform devices drivers and then add DT > support incrementally. This allows us to try out things in a non-ABI > kind of way and it also allows us to add support for various hardware > feature that do not yet support DT like in the case of DMA Engine. > With platform devices we can also use the driver with the SH > architecture. Sure, but in this case the DT bindings are set (by virtue of having been merged) and it is my understanding that they offer the same features as the platform device. As it is a new device driver I lean towards Olof's suggestion. > You can of course do some mixed DT and platform device support on a > board-level, but I don't really see what would be good about it. > Actually, I believe that will just make things overly complicated > without giving us the possibility to try out certain experimental > features with platform-device-only interface. I think that what would be good about it is: * The DT code would be exercised by people using the default board file. * The platform device would not need to be added. > Of course Simon, if you prefer to follow your proposal above then go > ahead. However, please consider the case when this code will be back > ported to LTSI-3.4. With regards to backporting, the platform driver patch still exists (e.g. on the ML) so it can be used for backporting as needed.
On Mon, Mar 4, 2013 at 6:05 PM, Simon Horman <horms@verge.net.au> wrote: > On Tue, Mar 05, 2013 at 10:28:09AM +0900, Magnus Damm wrote: >> Hi Simon >> >> On Fri, Mar 1, 2013 at 4:23 PM, Simon Horman <horms@verge.net.au> wrote: >> > On Thu, Feb 28, 2013 at 05:41:48PM -0800, Olof Johansson wrote: >> >> On Wed, Feb 27, 2013 at 11:39:14PM +0300, Sergei Shtylyov wrote: >> >> > From: Vladimir Barinov <vladimir.barinov@cogentembedded.com> >> >> > >> >> > Add SATA clock for r8a7779 SoC (for both device tree and usual cases). >> >> > Register SATA controller as a "late" platform device on r8a7779 SoC. >> >> >> >> Hi, >> >> >> >> If you have the a binding in the device tree (which you do through patch 1/3), >> >> then there's no reason to have a platform device for it. >> > >> > Hi Olof, >> > >> > the DT exists but currently the marzen board brings up all >> > of its devices using platform devices. Which if nothing else is >> > internally consistent. >> > >> > I suppose it would be possible to add a call to >> > r8a7779_add_standard_devices_dt() and have the board bring >> > up this device using DT and the rest using platform drivers >> > (until the drivers are migrated to DT). >> > >> > Would that be your preferred option? >> > >> > Magnus, how do you feel about this idea? >> >> I fail to see the upside I'm afraid. >> >> I believe the best way forward is for us to keep on following the same >> style as we have done so far. So for drivers they should begin by >> interfacing as regular platform devices drivers and then add DT >> support incrementally. This allows us to try out things in a non-ABI >> kind of way and it also allows us to add support for various hardware >> feature that do not yet support DT like in the case of DMA Engine. >> With platform devices we can also use the driver with the SH >> architecture. > > Sure, but in this case the DT bindings are set (by virtue of having been > merged) and it is my understanding that they offer the same features as > the platform device. > > As it is a new device driver I lean towards Olof's suggestion. > >> You can of course do some mixed DT and platform device support on a >> board-level, but I don't really see what would be good about it. >> Actually, I believe that will just make things overly complicated >> without giving us the possibility to try out certain experimental >> features with platform-device-only interface. > > I think that what would be good about it is: > > * The DT code would be exercised by people using the default board file. > * The platform device would not need to be added. Right, and just to clarify: The driver is still a platform driver. For non-trivial devices with per-device data, there will be a function in the platform driver to fill in the platform data from the device tree contents. So besides that one function, the rest of the driver can be shared between shmobile and arch/sh, there just won't be a platform_device registered on the ARM side. -Olof
On Tue, Mar 5, 2013 at 11:05 AM, Simon Horman <horms@verge.net.au> wrote: > On Tue, Mar 05, 2013 at 10:28:09AM +0900, Magnus Damm wrote: >> Hi Simon >> >> On Fri, Mar 1, 2013 at 4:23 PM, Simon Horman <horms@verge.net.au> wrote: >> > On Thu, Feb 28, 2013 at 05:41:48PM -0800, Olof Johansson wrote: >> >> On Wed, Feb 27, 2013 at 11:39:14PM +0300, Sergei Shtylyov wrote: >> >> > From: Vladimir Barinov <vladimir.barinov@cogentembedded.com> >> >> > >> >> > Add SATA clock for r8a7779 SoC (for both device tree and usual cases). >> >> > Register SATA controller as a "late" platform device on r8a7779 SoC. >> >> >> >> Hi, >> >> >> >> If you have the a binding in the device tree (which you do through patch 1/3), >> >> then there's no reason to have a platform device for it. >> > >> > Hi Olof, >> > >> > the DT exists but currently the marzen board brings up all >> > of its devices using platform devices. Which if nothing else is >> > internally consistent. >> > >> > I suppose it would be possible to add a call to >> > r8a7779_add_standard_devices_dt() and have the board bring >> > up this device using DT and the rest using platform drivers >> > (until the drivers are migrated to DT). >> > >> > Would that be your preferred option? >> > >> > Magnus, how do you feel about this idea? >> >> I fail to see the upside I'm afraid. >> >> I believe the best way forward is for us to keep on following the same >> style as we have done so far. So for drivers they should begin by >> interfacing as regular platform devices drivers and then add DT >> support incrementally. This allows us to try out things in a non-ABI >> kind of way and it also allows us to add support for various hardware >> feature that do not yet support DT like in the case of DMA Engine. >> With platform devices we can also use the driver with the SH >> architecture. > > Sure, but in this case the DT bindings are set (by virtue of having been > merged) and it is my understanding that they offer the same features as > the platform device. And if we want to add an experimental feature, how do you suggest we proceed? > As it is a new device driver I lean towards Olof's suggestion. > >> You can of course do some mixed DT and platform device support on a >> board-level, but I don't really see what would be good about it. >> Actually, I believe that will just make things overly complicated >> without giving us the possibility to try out certain experimental >> features with platform-device-only interface. > > I think that what would be good about it is: > > * The DT code would be exercised by people using the default board file. > * The platform device would not need to be added. Sure. It is however just a couple of lines of data. Also, this bypasses the idea of the -reference boards, but hey. >> Of course Simon, if you prefer to follow your proposal above then go >> ahead. However, please consider the case when this code will be back >> ported to LTSI-3.4. > > With regards to backporting, the platform driver patch still exists > (e.g. on the ML) so it can be used for backporting as needed. And then there will be different code paths in the back port and in the upstream code. Well, well. Thanks, / magnus
On Tue, Mar 05, 2013 at 01:33:30PM +0900, Magnus Damm wrote: > On Tue, Mar 5, 2013 at 11:05 AM, Simon Horman <horms@verge.net.au> wrote: > > On Tue, Mar 05, 2013 at 10:28:09AM +0900, Magnus Damm wrote: > >> Hi Simon > >> > >> On Fri, Mar 1, 2013 at 4:23 PM, Simon Horman <horms@verge.net.au> wrote: > >> > On Thu, Feb 28, 2013 at 05:41:48PM -0800, Olof Johansson wrote: > >> >> On Wed, Feb 27, 2013 at 11:39:14PM +0300, Sergei Shtylyov wrote: > >> >> > From: Vladimir Barinov <vladimir.barinov@cogentembedded.com> > >> >> > > >> >> > Add SATA clock for r8a7779 SoC (for both device tree and usual cases). > >> >> > Register SATA controller as a "late" platform device on r8a7779 SoC. > >> >> > >> >> Hi, > >> >> > >> >> If you have the a binding in the device tree (which you do through patch 1/3), > >> >> then there's no reason to have a platform device for it. > >> > > >> > Hi Olof, > >> > > >> > the DT exists but currently the marzen board brings up all > >> > of its devices using platform devices. Which if nothing else is > >> > internally consistent. > >> > > >> > I suppose it would be possible to add a call to > >> > r8a7779_add_standard_devices_dt() and have the board bring > >> > up this device using DT and the rest using platform drivers > >> > (until the drivers are migrated to DT). > >> > > >> > Would that be your preferred option? > >> > > >> > Magnus, how do you feel about this idea? > >> > >> I fail to see the upside I'm afraid. > >> > >> I believe the best way forward is for us to keep on following the same > >> style as we have done so far. So for drivers they should begin by > >> interfacing as regular platform devices drivers and then add DT > >> support incrementally. This allows us to try out things in a non-ABI > >> kind of way and it also allows us to add support for various hardware > >> feature that do not yet support DT like in the case of DMA Engine. > >> With platform devices we can also use the driver with the SH > >> architecture. > > > > Sure, but in this case the DT bindings are set (by virtue of having been > > merged) and it is my understanding that they offer the same features as > > the platform device. > > And if we want to add an experimental feature, how do you suggest we proceed? I wonder if this is a general problem for all drivers that are initialised via DT. I wonder how they add experimental features without setting bindings in stone. > > As it is a new device driver I lean towards Olof's suggestion. > > > >> You can of course do some mixed DT and platform device support on a > >> board-level, but I don't really see what would be good about it. > >> Actually, I believe that will just make things overly complicated > >> without giving us the possibility to try out certain experimental > >> features with platform-device-only interface. > > > > I think that what would be good about it is: > > > > * The DT code would be exercised by people using the default board file. > > * The platform device would not need to be added. > > Sure. It is however just a couple of lines of data. Also, this > bypasses the idea of the -reference boards, but hey. > > >> Of course Simon, if you prefer to follow your proposal above then go > >> ahead. However, please consider the case when this code will be back > >> ported to LTSI-3.4. > > > > With regards to backporting, the platform driver patch still exists > > (e.g. on the ML) so it can be used for backporting as needed. > > And then there will be different code paths in the back port and in > the upstream code. As you say, its only a few lines of code. From my point of view I think it would be sane to use DT for initialisation where we can do so without losing functionality. However, with all that said. I do accept that this would be a departure from how shmobile has handled things up until now. And that it is probably something that needs to be well thought through. So with that in mind I now plan to merge this patch. The code can be removed it later if appropriate.
Index: renesas/arch/arm/mach-shmobile/clock-r8a7779.c =================================================================== --- renesas.orig/arch/arm/mach-shmobile/clock-r8a7779.c +++ renesas/arch/arm/mach-shmobile/clock-r8a7779.c @@ -87,6 +87,7 @@ static struct clk div4_clks[DIV4_NR] = { }; enum { MSTP323, MSTP322, MSTP321, MSTP320, + MSTP115, MSTP101, MSTP100, MSTP030, MSTP029, MSTP028, MSTP027, MSTP026, MSTP025, MSTP024, MSTP023, MSTP022, MSTP021, @@ -99,6 +100,7 @@ static struct clk mstp_clks[MSTP_NR] = { [MSTP322] = SH_CLK_MSTP32(&div4_clks[DIV4_P], MSTPCR3, 22, 0), /* SDHI1 */ [MSTP321] = SH_CLK_MSTP32(&div4_clks[DIV4_P], MSTPCR3, 21, 0), /* SDHI2 */ [MSTP320] = SH_CLK_MSTP32(&div4_clks[DIV4_P], MSTPCR3, 20, 0), /* SDHI3 */ + [MSTP115] = SH_CLK_MSTP32(&div4_clks[DIV4_P], MSTPCR1, 15, 0), /* SATA */ [MSTP101] = SH_CLK_MSTP32(&div4_clks[DIV4_P], MSTPCR1, 1, 0), /* USB2 */ [MSTP100] = SH_CLK_MSTP32(&div4_clks[DIV4_P], MSTPCR1, 0, 0), /* USB0/1 */ [MSTP030] = SH_CLK_MSTP32(&div4_clks[DIV4_P], MSTPCR0, 30, 0), /* I2C0 */ @@ -156,6 +158,8 @@ static struct clk_lookup lookups[] = { CLKDEV_CON_ID("peripheral_clk", &div4_clks[DIV4_P]), /* MSTP32 clocks */ + CLKDEV_DEV_ID("sata_rcar", &mstp_clks[MSTP115]), /* SATA */ + CLKDEV_DEV_ID("fc600000.sata", &mstp_clks[MSTP115]), /* SATA w/DT */ CLKDEV_DEV_ID("ehci-platform.1", &mstp_clks[MSTP101]), /* USB EHCI port2 */ CLKDEV_DEV_ID("ohci-platform.1", &mstp_clks[MSTP101]), /* USB OHCI port2 */ CLKDEV_DEV_ID("ehci-platform.0", &mstp_clks[MSTP100]), /* USB EHCI port0/1 */ Index: renesas/arch/arm/mach-shmobile/setup-r8a7779.c =================================================================== --- renesas.orig/arch/arm/mach-shmobile/setup-r8a7779.c +++ renesas/arch/arm/mach-shmobile/setup-r8a7779.c @@ -29,6 +29,7 @@ #include <linux/serial_sci.h> #include <linux/sh_intc.h> #include <linux/sh_timer.h> +#include <linux/dma-mapping.h> #include <mach/hardware.h> #include <mach/irqs.h> #include <mach/r8a7779.h> @@ -322,6 +323,30 @@ static struct platform_device i2c3_devic .num_resources = ARRAY_SIZE(rcar_i2c3_res), }; +static struct resource sata_resources[] = { + [0] = { + .name = "rcar-sata", + .start = 0xfc600000, + .end = 0xfc601fff, + .flags = IORESOURCE_MEM, + }, + [1] = { + .start = gic_spi(100), + .flags = IORESOURCE_IRQ, + }, +}; + +static struct platform_device sata_device = { + .name = "sata_rcar", + .id = -1, + .resource = sata_resources, + .num_resources = ARRAY_SIZE(sata_resources), + .dev = { + .dma_mask = &sata_device.dev.coherent_dma_mask, + .coherent_dma_mask = DMA_BIT_MASK(32), + }, +}; + static struct platform_device *r8a7779_devices_dt[] __initdata = { &scif0_device, &scif1_device, @@ -338,6 +363,7 @@ static struct platform_device *r8a7779_l &i2c1_device, &i2c2_device, &i2c3_device, + &sata_device, }; void __init r8a7779_add_standard_devices(void)