Re: [PATCH 2/4] ARM: mach-shmobile: r8a7779: add SATA support

[Simon Horman <horms@verge.net.au>]

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
>