Re: [2/4] OMAP: DSS2: Renaming register macro DISPC_DIVISOR(ch)

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

Hi,

On Thu, 2011-02-03 at 14:09 +0000, Raghuveer Murthy wrote:
> Renamed DISPC_DIVISOR(ch) to DISPC_DIVISORo(ch), to facilitate introduction
> of DISPC_DIVISOR register, which is specific for OMAP4. OMAP4 has 3 registers
> DISPC_DIVISOR, DISPC_DIVISOR1 and DISPC_DIVISOR2.
> 
> Also updated, all the usages of DISPC_DIVISOR(ch) to DISPC_DIVISORo(ch).
> 
> OMAP4 TRM uses DISPC_DIVISORo generically to refer to DISPC_DIVISOR1 and
> DISPC_DIVISOR2
> 
> Signed-off-by: Raghuveer Murthy <raghuveer.murthy@ti.com>
> 
> ---
> drivers/video/omap2/dss/dispc.c |   31 ++++++++++++++++++-------------
>  1 files changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index cc58208..e52a413 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -72,7 +72,12 @@ struct dispc_reg { u16 idx; };
>  #define DISPC_TIMING_H(ch)		DISPC_REG(ch != 2 ? 0x0064 : 0x0400)
>  #define DISPC_TIMING_V(ch)		DISPC_REG(ch != 2 ? 0x0068 : 0x0404)
>  #define DISPC_POL_FREQ(ch)		DISPC_REG(ch != 2 ? 0x006C : 0x0408)
> -#define DISPC_DIVISOR(ch)		DISPC_REG(ch != 2 ? 0x0070 : 0x040C)
> +/*
> + * Use DISPC_DIVISORo(ch) when DISPC_DIVISOR1 or DISPC_DIVISOR2 has to be
> + * configured. OMAP4 TRM uses DISPC_DIVISORo generically to refer DISPC_DIVISOR1
> + * and DISPC_DIVISOR2
> + */

While comments are good, I think this is extra. The purpose of the
register is clear from TRM. I like to keep the code clean of
trivialities =). The same goes for the register comment in the next
patch also.

You could mention this in the patch description though, so that patch
reviewing is easier.

And about DISPC_DIVISORo, I think the TRM writer has typoed it, he meant
to write DISPC_DIVISORi =). But let's stick with the naming from TRM.

 Tomi