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

[Tomi Valkeinen <tomi.valkeinen@xxxxxx>]

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 <cmahapatra@xxxxxx>
> ---
>  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

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