Re: [PATCH 4/4] ARM: shmobile: marzen: Use disabled variant of clock workaround for scif devices

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

On Wed, Apr 30, 2014 at 02:47:30AM +0200, Laurent Pinchart wrote:
> Hi Simon,
> 
> On Wednesday 30 April 2014 08:51:11 Simon Horman wrote:
> > On Tue, Apr 29, 2014 at 01:40:08PM +0000, Magnus Damm wrote:
> > > On Tue, Apr 29, 2014 at 7:43 AM, Simon Horman wrote:
> > > > Now that SCIF devices are initialised using DT it should
> > > > be sufficient to use the disabled variant of the clock workaround.
> > > > 
> > > > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > > > ---
> > > > 
> > > >  arch/arm/mach-shmobile/board-marzen-reference.c | 12 ++++++------
> > > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/arch/arm/mach-shmobile/board-marzen-reference.c
> > > > b/arch/arm/mach-shmobile/board-marzen-reference.c index
> > > > 6d34baf..1f14d80 100644
> > > > --- a/arch/arm/mach-shmobile/board-marzen-reference.c
> > > > +++ b/arch/arm/mach-shmobile/board-marzen-reference.c
> > > > @@ -40,12 +40,6 @@ static void __init marzen_init_timer(void)
> > > > 
> > > >   * devices until they get moved to DT.
> > > >   */
> > > >  
> > > >  static const struct clk_name clk_names[] __initconst = {
> > > > 
> > > > -       { "scif0", NULL, "sh-sci.0" },
> > > > -       { "scif1", NULL, "sh-sci.1" },
> > > > -       { "scif2", NULL, "sh-sci.2" },
> > > > -       { "scif3", NULL, "sh-sci.3" },
> > > > -       { "scif4", NULL, "sh-sci.4" },
> > > > -       { "scif5", NULL, "sh-sci.5" },
> > > > 
> > > >         { "tmu0", NULL, "sh_tmu.0" },
> > > >         { "tmu1", NULL, "sh_tmu.1" },
> > > >         { "tmu2", NULL, "sh_tmu.2" },
> > > > 
> > > > @@ -55,6 +49,12 @@ static const struct clk_name clk_names[] __initconst
> > > > = {
> > > > 
> > > >   * This is a really crude hack to work around core platform clock
> > > >   issues
> > > >   */
> > > >  
> > > >  static const struct clk_name clk_enables[] __initconst = {
> > > > 
> > > > +       { "scif0", NULL, "sh-sci.0" },
> > > > +       { "scif1", NULL, "sh-sci.1" },
> > > > +       { "scif2", NULL, "sh-sci.2" },
> > > > +       { "scif3", NULL, "sh-sci.3" },
> > > > +       { "scif4", NULL, "sh-sci.4" },
> > > > +       { "scif5", NULL, "sh-sci.5" },
> > > > 
> > > >         { "sdhi0", NULL, "ffe4c000.sd" },
> > > >         { "thermal", NULL, "ffc48000.thermal" },
> > > >  
> > > >  };
> > > 
> > > Hi Simon,
> > > 
> > > Thanks for your SCIF DT patches for r8a7779. They look good and clean
> > > in general I think.
> > > 
> > > Regarding this specific patch, I'm not sure why you need to perform
> > > this kind of change. Is it following the same style as other SoCs?
> > > 
> > > In my mind the SCIF driver at least used to rely on both Runtime PM
> > > and the clock framework for clock control so only relying on clock
> > > framework should be enough for now. The driver may have been updated
> > > though, but if so we should make sure we follow the same pattern on
> > > other SoCs as well.
> > 
> > Thanks, I agree this is not needed.
> 
> Shouldn't we remove the scif clock entries from the clk_names array completely 
> ?

To be honest I am somewhat confused.

My understanding was that clk_enables were used for devices
that are initialised using DT but whose clocks need to be forcibly
enabled because the interaction with runtime PM isn't handled properly
yet.

And my reading of Magnus's email was that SCI devices do not need
to be enabled as the driver interacts with runtime PM correctly.

So I tend to agree with you that we ought to be able to simply remove
the SCI devices from clk_names now they are initialised via DT
(unlike TMU which is still initialised as a platform device).