Re: [PATCH V5 1/6] OMAPDSS: DISPC: cleanup cpu_is_xxxx checks

[Tomi Valkeinen]

On Mon, 2012-08-20 at 18:52 +0530, Chandrabhanu Mahapatra wrote:
> All the cpu_is checks have been moved to dispc_init_features function 
> providing
> a much more generic and cleaner interface. The OMAP version and revision
> specific functions and data are initialized by dispc_features structure which 
> is
> local to dispc.c.
> 
> Signed-off-by: Chandrabhanu Mahapatra <cmahapa...@ti.com>
> ---
>  drivers/video/omap2/dss/dispc.c |  433 
> +++++++++++++++++++++++++--------------
>  1 file changed, 278 insertions(+), 155 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index ff52702..3fad33a 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -81,6 +81,23 @@ struct dispc_irq_stats {
>       unsigned irqs[32];
>  };
>  
> +struct dispc_features {
> +     int hp_max;
> +     int vp_max;
> +     int sw_max;
> +     int sw_start;
> +     int fp_start;
> +     int bp_start;

Here you could use a bit smaller datatype. u16 should probably be more
than enough.

> +static int __init dispc_init_features(struct device *dev)
> +{
> +     struct dispc_features *feat = devm_kzalloc(dev, sizeof(*feat),
> +                                                             GFP_KERNEL);
> +     if (!feat) {
> +             dev_err(dev, "Failed to allocate DISPC Features\n");
> +             return -ENOMEM;
> +     }
> +
> +     if (cpu_is_omap24xx()) {
> +             memcpy(feat, &omap24xx_dispc_feats, sizeof(*feat));
> +     } else if (cpu_is_omap34xx()) {
> +             if (omap_rev() < OMAP3430_REV_ES3_0)
> +                     memcpy(feat, &omap34xx_rev1_0_dispc_feats,
> +                                                     sizeof(*feat));
> +             else
> +                     memcpy(feat, &omap34xx_rev3_0_dispc_feats,
> +                                                     sizeof(*feat));
> +     } else if (cpu_is_omap44xx()) {
> +             memcpy(feat, &omap44xx_dispc_feats, sizeof(*feat));
> +     } else {
> +             return -ENODEV;
> +     }
> +
> +     dispc.feat = feat;
> +
> +     return 0;
> +}

This becomes much cleaner with something like the following (same could
be used in dss.c also):

        const struct dispc_features *src;
        struct dispc_features *dst;

        dst = devm_kzalloc(dev, sizeof(*dst), GFP_KERNEL);
        if (!dsst) {
                dev_err(dev, "Failed to allocate DISPC Features\n");
                return -ENOMEM;
        }

        if (cpu_is_omap24xx()) {
                src = &omap24xx_dispc_feats;
        } else if (cpu_is_omap34xx()) {
                if (omap_rev() < OMAP3430_REV_ES3_0)
                        src = &omap34xx_rev1_0_dispc_feats;
                else
                        src = &omap34xx_rev3_0_dispc_feats;
        } else if (cpu_is_omap44xx()) {
                src = &omap44xx_dispc_feats;
        } else {
                return -ENODEV;
        }

        memcpy(dst, src, sizeof(*dst));

        dispc.feat = dst;

 Tomi

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