Re: [PATCH v2 0/2] OMAP2PLUS: DSS2: Clock Source Changes

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

On Tue, 2011-03-08 at 05:50 -0600, Taneja, Archit wrote:
> There are clocks on OMAP4 within DSS2 which can have multiple clock sources.
> This cleans up the existing way of selecting/getting the clock sources and add
> some of the new clock sources for OMAP4.
> 
> Applies over:
> http://gitorious.org/linux-omap-dss2/linux/commits/master
> 
> Archit Taneja (2):
>   OMAP2PLUS: DSS2: Cleanup clock source related code
>   OMAP4: DSS2: Clock source changes for OMAP4

The set looks good. I'll apply it tomorrow morning after some testing.

But something for future patches:

The naming of the functions could be made clearer. We have logic clock
for the LCD outputs but also for DISPC. So for example dispc_lclk_rate()
could be dispc_lcd_lclk_rate(), to make sure it's not called when the
caller needs to know the lclk of dispc. And it looks too similar to
dispc_fclk_rate() which returns clock for dispc core, easily giving the
impression that these two are linked.

And similarly dispc_pclk_rate() could be dispc_lcd_pclk_rate(), to make
clear that it's not for example venc pixel clock.

Perhaps there are also other functions whose name could be more specific
now that we have separate clocks for dispc core and the lcd paths.

 Tomi