Re: [PATCH 2/2] silicon revision check for OMAP2/3

[Daniel Stone <daniel.stone@nokia.com>]

On Fri, Oct 26, 2007 at 08:42:03PM +0530, ext Girish wrote:
> +#define is_sil_rev_greater_than(rev) \
> +		(((((system_rev & 0xffff0000) >> 16) == \
> +		((rev & 0xffff0000) >> 16)) && 	\
> +		(system_rev & 0xf000) >> 12) > ((rev & 0xf000) >> 12))
> +#define is_sil_rev_less_than(rev) \
> +		(((((system_rev & 0xffff0000) >> 16) == \
> +		((rev & 0xffff0000) >> 16)) && 	\
> +		(system_rev & 0xf000) >> 12) < ((rev & 0xf000) >> 12))
> +#define is_sil_rev_equal_to(rev) \
> +		(((((system_rev & 0xffff0000) >> 16) ==	\
> +		((rev & 0xffff0000) >> 16)) && \
> +		(system_rev & 0xf000) >> 12) == ((rev & 0xf000) >> 12))
> +#define get_sil_rev() \
> +		((system_rev & 0xf000) >> 12)

Hi,
Just for code clarity, you could use (taking is_sil_rev_greater_than as
an example):
#define is_sil_rev_greater_than(rev) \
                ((system_rev & 0xffff0000) == (rev & 0xffff0000) && \
                 (system_rev & 0x0000f000) == (rev & 0x0000f000))

You don't need the extra shifts in the comparison.  But, even better,
you could use:
#define get_sil_generation(rev) ((rev & 0xffff0000) >> 16)
#define get_sil_revision(rev) ((rev & 0x00000f000) >> 12)
#define is_sil_rev_greater_than(rev) \
                ((get_sil_generation(system_rev) == \
                  get_sil_generation(rev)) && \
                 (get_sil_revision(system_rev) >
                  get_sil_revision(rev)))

and likewise for all the rest.

Cheers,
Daniel