[U-Boot] [PATCH v3 3/4] dm: video: use constants to refer to colors

[Heinrich Schuchardt]

On 02/07/2018 05:38 AM, Simon Glass wrote:
> Hi Heinrich,
> 
> On 4 February 2018 at 15:55, Heinrich Schuchardt <xypron.glpk at gmx.de 
> <mailto:xypron.glpk at gmx.de>> wrote:
>  > On 02/04/2018 02:40 PM, Simon Glass wrote:
>  >> Hi Heinrich,
>  >>
>  >> On 29 January 2018 at 00:19, Heinrich Schuchardt <xypron.glpk at gmx.de 
> <mailto:xypron.glpk at gmx.de>> wrote:
>  >>> Use constants to refer to colors.
>  >>> Adjust initialization of foreground and background color to avoid
>  >>> setting reserved bits.
>  >>> Consistently u32 instead of unsigned for color bit mask.
>  >>>
>  >>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de 
> <mailto:xypron.glpk at gmx.de>>
>  >>> ---
>  >>> v3
>  >>>         Use color constants for initalizing the console.
>  >>> v2
>  >>>         no change
>  >>> ---
>  >>>  drivers/video/vidconsole-uclass.c | 55 
> +++++++++++++++++++++++----------------
>  >>>  drivers/video/video-uclass.c      | 19 +++++++++-----
>  >>>  include/video.h                   | 11 ++++++--
>  >>>  include/video_console.h           | 31 ++++++++++++++++++++++
>  >>>  4 files changed, 85 insertions(+), 31 deletions(-)
>  >>
>  >> Reviewed-by: Simon Glass <sjg at chromium.org <mailto:sjg at chromium.org>>
>  >>
>  >> Regarding my point about using u32 in function return values and args,
>  >> I don't think I explained it very well.
>  >>
>  >> IMO is makes no sense to structure all the intermediate code which
>  >> generates pixel values to use u32, when an unsigned int is enough on
>  >> all machines that U-Boot supports. The packing / unpacking into a
>  >> 32-bit word in memory is something that is done once when the pixel is
>  >> accessed. Thereafter I don't see a need to push things around in a
>  >> particular format.
>  >
>  > The definition I found was
>  > typedef unsigned int u32;
>  >
>  > What makes you believe there is any packing and unpacking or masking
>  > overhead avoided by using unsigned int or any other 32bit integer type?
> 
> A machine with a 32-bit register has to mask its argument on entry to 
> the function to ensure that the caller does not pass an invalid value.

If this were true it would also hold true for unsigned int. See above 
typedef.

C does not check if a value is negative when copying int to unsigned int.

> 
>  >
>  > For struct vid_rgb other integer types could be used. I not sure if
>  > accessing a char is any faster in this context than using 32bit integers.
> 
> In general the natural word size is best for arguments and return values 
> I think (int or unsigned int).

So I leave it as it is.

By the way: natural word size is size_t and not int.

> 
>  >
>  > Regards
>  >
>  > Heinrich
>  >
>  >>
>  >> I have an aversion to code which forces the compiler to mask every
>  >> variable access just to pass the data around.
>  >>
>  >> So I would prefer to use u32 only when accessing the hardware, or for
>  >> pointers which do that.
>  >>
>  >> Reards,
>  >> Simon
>  >>
>  >
> 
> Regards,
> Simon