On Tue, 2012-08-07 at 13:58 +0530, Chandrabhanu Mahapatra wrote:
> The cpu_is checks have been removed from dss.c providing it a much generic and
> cleaner interface. The OMAP version and revision specific functions are
> initialized by dss_ops structure in dss features.
> 
> Signed-off-by: Chandrabhanu Mahapatra <cmahapa...@ti.com>
> ---
>  drivers/video/omap2/dss/dss.c          |  114 
> ++++++++++++++++++++++----------
>  drivers/video/omap2/dss/dss.h          |   23 ++++++-
>  drivers/video/omap2/dss/dss_features.c |   40 +++++++++++
>  drivers/video/omap2/dss/dss_features.h |    1 +
>  4 files changed, 141 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
> index 7b1c6ac..c263da7 100644
> --- a/drivers/video/omap2/dss/dss.c
> +++ b/drivers/video/omap2/dss/dss.c
> @@ -83,6 +83,8 @@ static struct {
>  
>       bool            ctx_valid;
>       u32             ctx[DSS_SZ_REGS / sizeof(u32)];
> +
> +     const struct dss_ops *ops;
>  } dss;
>  
>  static const char * const dss_generic_clk_source_names[] = {
> @@ -236,6 +238,15 @@ const char *dss_get_generic_clk_source_name(enum 
> omap_dss_clk_source clk_src)
>       return dss_generic_clk_source_names[clk_src];
>  }
>  
> +char *set_dump_clk_str_24_34(void)
> +{
> +     return "%s (%s) = %lu / %lu * 2  = %lu\n";
> +}
> +
> +char *set_dump_clk_str(void)
> +{
> +     return "%s (%s) = %lu / %lu  = %lu\n";
> +}

You don't need functions for these, or for the max fck div. Just add the
string or the number to the struct.

And I don't really like the format string being separately. It would
probably be better if you instead had just a flag for the x2 multiplier,
and the dump function would check the flag to see which format string it
uses.

> @@ -479,10 +511,7 @@ int dss_calc_clock_div(unsigned long req_pck, struct 
> dss_clock_info *dss_cinfo,
>  
>       max_dss_fck = dss_feat_get_param_max(FEAT_PARAM_DSS_FCK);
>  
> -     fck = clk_get_rate(dss.dss_clk);
> -     if (req_pck == dss.cache_req_pck &&
> -                     ((cpu_is_omap34xx() && prate == dss.cache_prate) ||
> -                      dss.cache_dss_cinfo.fck == fck)) {
> +     if (req_pck == dss.cache_req_pck && dss.ops->set_dss_cinfo()) {
>               DSSDBG("dispc clock info found from cache.\n");
>               *dss_cinfo = dss.cache_dss_cinfo;
>               *dispc_cinfo = dss.cache_dispc_cinfo;

This is quite confusing. "set" function should set something, normally
something that is given as a parameter. Your function seems to be
checking some values.

 Tomi

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to