| Message ID | 201302170143.36052.sergei.shtylyov@cogentembedded.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Sergei > From: Vladimir Barinov <vladimir.barinov@cogentembedded.com> > > Add SATA clock and platform device resources on r8a7779 SoC. > Add entry to r8a7779_auxdata_lookup[], so that devm_clk_get() in the driver > still works when we're using the device tree. > > Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com> > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> (snip) > /* MSTP32 clocks */ > + CLKDEV_DEV_ID("sata_rcar", &mstp_clks[MSTP115]), /* SATA */ > 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 */ (snip) > static const struct of_dev_auxdata r8a7779_auxdata_lookup[] __initconst = { > + OF_DEV_AUXDATA("renesas,rcar-sata", 0xfc600000, "sata_rcar", NULL), ?? Is this settings really required for DT ?? I guess you can remove it, and add + CLKDEV_DEV_ID("sata_rcar", &mstp_clks[MSTP115]), // for platform + CLKDEV_DEV_ID("fc600000.sata_rcar", &mstp_clks[MSTP115]), // for DT And... > [1/4] ARM: mach-shmobile: r8a7779: SATA DT configuration > [2/4] ARM: mach-shmobile: r8a7779: add SATA support > [3/4] libata: add R-Car SATA driver > [4/4] ARM: mach-shmobile: marzen: add SATA support I believe [3/4] patch should be base patch ? Best regards --- Kuninori Morimoto
Hello. On 18-02-2013 5:23, Kuninori Morimoto wrote: >> From: Vladimir Barinov <vladimir.barinov@cogentembedded.com> >> Add SATA clock and platform device resources on r8a7779 SoC. >> Add entry to r8a7779_auxdata_lookup[], so that devm_clk_get() in the driver >> still works when we're using the device tree. >> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com> >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > (snip) >> /* MSTP32 clocks */ >> + CLKDEV_DEV_ID("sata_rcar", &mstp_clks[MSTP115]), /* SATA */ >> 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 */ > (snip) >> static const struct of_dev_auxdata r8a7779_auxdata_lookup[] __initconst = { >> + OF_DEV_AUXDATA("renesas,rcar-sata", 0xfc600000, "sata_rcar", NULL), > ?? > Is this settings really required for DT ?? Yes, TTBOMK, it's the last resort measure used in exctly this case. > I guess you can remove it, and add > + CLKDEV_DEV_ID("sata_rcar", &mstp_clks[MSTP115]), // for platform > + CLKDEV_DEV_ID("fc600000.sata_rcar", &mstp_clks[MSTP115]), // for DT IMO, this neither looks nor scales well. > And... >> [1/4] ARM: mach-shmobile: r8a7779: SATA DT configuration >> [2/4] ARM: mach-shmobile: r8a7779: add SATA support >> [3/4] libata: add R-Car SATA driver >> [4/4] ARM: mach-shmobile: marzen: add SATA support > I believe [3/4] patch should be base patch ? You're probably right, I'll reorder the patches when posting V2. > Best regards > --- > Kuninori Morimoto WBR, Sergei
Hi Sergei, Thanks for your efforts with this SATA driver. On Mon, Feb 18, 2013 at 11:07 PM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > On 18-02-2013 5:23, Kuninori Morimoto wrote: >>> From: Vladimir Barinov <vladimir.barinov@cogentembedded.com> >>> Add SATA clock and platform device resources on r8a7779 SoC. >>> Add entry to r8a7779_auxdata_lookup[], so that devm_clk_get() in the >>> driver >>> still works when we're using the device tree. > > >>> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com> >>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >> >> (snip) > > >>> /* MSTP32 clocks */ >>> + CLKDEV_DEV_ID("sata_rcar", &mstp_clks[MSTP115]), /* SATA */ >>> 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 */ >> >> (snip) >>> >>> static const struct of_dev_auxdata r8a7779_auxdata_lookup[] __initconst >>> = { >>> + OF_DEV_AUXDATA("renesas,rcar-sata", 0xfc600000, "sata_rcar", >>> NULL), > > >> ?? >> Is this settings really required for DT ?? > > > Yes, TTBOMK, it's the last resort measure used in exctly this case. Well, I have to agree with Morimoto-san here. Other vendors may chose to use AUXDATA to map clocks, and I believe it makes sense in the case of adding platform data as a workaround during transition to full DT support. But for simply mapping clocks please follow the same style as we have done so far, which is what Morimoto-san pointed out: CLKDEV_DEV_ID("fc600000.sata_rcar", &mstp_clks[MSTP115]), Also, I don't think SATA is needed as an early device so it should be enough to register it late as a regular platform device. =) Thanks, / magnus
Hi Sergei, Magnus, Simon > >>> static const struct of_dev_auxdata r8a7779_auxdata_lookup[] __initconst > >>> = { > >>> + OF_DEV_AUXDATA("renesas,rcar-sata", 0xfc600000, "sata_rcar", > >>> NULL), > > > > > >> ?? > >> Is this settings really required for DT ?? > > > > > > Yes, TTBOMK, it's the last resort measure used in exctly this case. > > Well, I have to agree with Morimoto-san here. Other vendors may chose > to use AUXDATA to map clocks, and I believe it makes sense in the case > of adding platform data as a workaround during transition to full DT > support. But for simply mapping clocks please follow the same style as > we have done so far, which is what Morimoto-san pointed out: > > CLKDEV_DEV_ID("fc600000.sata_rcar", &mstp_clks[MSTP115]), > > Also, I don't think SATA is needed as an early device so it should be > enough to register it late as a regular platform device. =) BTW, Sergei, which branch are you using for R-Car H1 DT booting ? I'm not sure detail, but, I guess R-Car H1 DT detection needs simon's latest branch. If you are using old branch, DT detection will be failed ? Simon, is my comment correct ? Best regards --- Kuninori Morimoto
On Mon, Feb 18, 2013 at 11:21:19PM +0900, Magnus Damm wrote: > Hi Sergei, > > Thanks for your efforts with this SATA driver. > > On Mon, Feb 18, 2013 at 11:07 PM, Sergei Shtylyov > <sergei.shtylyov@cogentembedded.com> wrote: > > On 18-02-2013 5:23, Kuninori Morimoto wrote: > >>> From: Vladimir Barinov <vladimir.barinov@cogentembedded.com> > >>> Add SATA clock and platform device resources on r8a7779 SoC. > >>> Add entry to r8a7779_auxdata_lookup[], so that devm_clk_get() in the > >>> driver > >>> still works when we're using the device tree. > > > > > >>> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com> > >>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > >> > >> (snip) > > > > > >>> /* MSTP32 clocks */ > >>> + CLKDEV_DEV_ID("sata_rcar", &mstp_clks[MSTP115]), /* SATA */ > >>> 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 */ > >> > >> (snip) > >>> > >>> static const struct of_dev_auxdata r8a7779_auxdata_lookup[] __initconst > >>> = { > >>> + OF_DEV_AUXDATA("renesas,rcar-sata", 0xfc600000, "sata_rcar", > >>> NULL), > > > > > >> ?? > >> Is this settings really required for DT ?? > > > > > > Yes, TTBOMK, it's the last resort measure used in exctly this case. > > Well, I have to agree with Morimoto-san here. Other vendors may chose > to use AUXDATA to map clocks, and I believe it makes sense in the case > of adding platform data as a workaround during transition to full DT > support. But for simply mapping clocks please follow the same style as > we have done so far, which is what Morimoto-san pointed out: > > CLKDEV_DEV_ID("fc600000.sata_rcar", &mstp_clks[MSTP115]), FWIW, this CLKDEV_DEV_ID() is consistent with how shmobile has handled other cases so far. (Mostly because of the argument Magnus makes above). > Also, I don't think SATA is needed as an early device so it should be > enough to register it late as a regular platform device. =) > > Thanks, > > / magnus > -- > To unsubscribe from this list: send the line "unsubscribe linux-sh" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On Mon, Feb 18, 2013 at 04:40:16PM -0800, Kuninori Morimoto wrote: > > Hi Sergei, Magnus, Simon > > > >>> static const struct of_dev_auxdata r8a7779_auxdata_lookup[] __initconst > > >>> = { > > >>> + OF_DEV_AUXDATA("renesas,rcar-sata", 0xfc600000, "sata_rcar", > > >>> NULL), > > > > > > > > >> ?? > > >> Is this settings really required for DT ?? > > > > > > > > > Yes, TTBOMK, it's the last resort measure used in exctly this case. > > > > Well, I have to agree with Morimoto-san here. Other vendors may chose > > to use AUXDATA to map clocks, and I believe it makes sense in the case > > of adding platform data as a workaround during transition to full DT > > support. But for simply mapping clocks please follow the same style as > > we have done so far, which is what Morimoto-san pointed out: > > > > CLKDEV_DEV_ID("fc600000.sata_rcar", &mstp_clks[MSTP115]), > > > > Also, I don't think SATA is needed as an early device so it should be > > enough to register it late as a regular platform device. =) > > BTW, Sergei, which branch are you using for R-Car H1 DT booting ? > I'm not sure detail, but, I guess R-Car H1 DT detection needs > simon's latest branch. > If you are using old branch, DT detection will be failed ? > > Simon, is my comment correct ? I guess so too.
Hello. On 19-02-2013 4:40, Kuninori Morimoto wrote: >>>>> static const struct of_dev_auxdata r8a7779_auxdata_lookup[] __initconst >>>>> = { >>>>> + OF_DEV_AUXDATA("renesas,rcar-sata", 0xfc600000, "sata_rcar", >>>>> NULL), >>>> ?? >>>> Is this settings really required for DT ?? >>> Yes, TTBOMK, it's the last resort measure used in exctly this case. >> Well, I have to agree with Morimoto-san here. Other vendors may chose >> to use AUXDATA to map clocks, and I believe it makes sense in the case >> of adding platform data as a workaround during transition to full DT >> support. But for simply mapping clocks please follow the same style as >> we have done so far, which is what Morimoto-san pointed out: >> CLKDEV_DEV_ID("fc600000.sata_rcar", &mstp_clks[MSTP115]), >> Also, I don't think SATA is needed as an early device so it should be >> enough to register it late as a regular platform device. =) > BTW, Sergei, which branch are you using for R-Car H1 DT booting ? > I'm not sure detail, but, I guess R-Car H1 DT detection needs > simon's latest branch. I noted in the cover letter that the patches are against the 'next' branch. > If you are using old branch, DT detection will be failed ? Those old/new branches sound too vague for me. WBR, Sergei
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,7 @@ 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("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 @@ -322,6 +322,26 @@ 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), +}; + static struct platform_device *r8a7779_early_devices_dt[] __initdata = { &scif0_device, &scif1_device, @@ -338,6 +358,7 @@ static struct platform_device *r8a7779_e &i2c1_device, &i2c2_device, &i2c3_device, + &sata_device, }; void __init r8a7779_add_standard_devices(void) @@ -404,6 +425,7 @@ void __init r8a7779_add_early_devices_dt } static const struct of_dev_auxdata r8a7779_auxdata_lookup[] __initconst = { + OF_DEV_AUXDATA("renesas,rcar-sata", 0xfc600000, "sata_rcar", NULL), {}, };