On Sat, Nov 09, 2013 at 01:51:04PM +0100, Laurent Pinchart wrote:
> Hi Dave,
> 
> Could you please pick this patch up ?
> 
> On Monday 28 October 2013 23:49:22 Laurent Pinchart wrote:
> > Turn clk_enable() and clk_disable() calls into clk_prepare_enable() and
> > clk_disable_unprepare() to get ready for the migration to the common
> > clock framework.
> > 
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: dri-devel@lists.freedesktop.org
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >  drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
> > b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c index 54bad98..562f9a4 100644
> > --- a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
> > +++ b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
> > @@ -40,7 +40,7 @@
> >  static void shmob_drm_clk_on(struct shmob_drm_device *sdev)
> >  {
> >  	if (sdev->clock)
> > -		clk_enable(sdev->clock);
> > +		clk_prepare_enable(sdev->clock);

Sorry for jumping in so late, but shouldn't this be split into two
separate calls, clk_prepare() in .probe() and clk_enable() here?

Also note that both clk_prepare() and clk_enable() (and therefore
clk_prepare_enable() as well) can fail, so you should really check
the return values here.

> >  #if 0
> >  	if (sdev->meram_dev && sdev->meram_dev->pdev)
> >  		pm_runtime_get_sync(&sdev->meram_dev->pdev->dev);
> > @@ -54,7 +54,7 @@ static void shmob_drm_clk_off(struct shmob_drm_device
> > *sdev) pm_runtime_put_sync(&sdev->meram_dev->pdev->dev);
> >  #endif
> >  	if (sdev->clock)
> > -		clk_disable(sdev->clock);
> > +		clk_disable_unprepare(sdev->clock);

Similarily I'd expect this to be clk_disable() only, with the
clk_unprepare() in .remove(). Or perhaps there's a very good reason to
do both here?

Thierry