Re: [Qemu-devel] [PATCH 1/4] hw/ds1338.c: Fix handling of HOURS register.

[Peter Maydell]

On 4 December 2012 20:57, Antoine Mathys <address@hidden> wrote:
> On 12/04/2012 06:42 PM, Peter Maydell wrote:
>> This looks good as far as the logic goes, but I think we could use some
>> symbolic constants for the 12-hour and PM bits rather than all the literal
>> 0x20 0x40 0x60. thanks -- PMM
>
> I refrained from using symbolic constants for three reasons:
> 1. You need to refer to the datasheet anyway to understand what is going on.
> 2. The code is short and it is easy to keep track of a few constants.
> 3. Symbolic names, which can be quite long, tend to make the code harder
> rather than easier to read

...I had the datasheet open and I still had to bounce back and forth
several times to work out which bits you were working with, so I
think we definitely disagree on this point...

> It is largely a matter of preference though and I'm not opposed to the idea.
> I would appreciate naming suggestions though.
>
> I propose not to have symbolic constants for the registers and to use CH,
> OSF, MODE12, PM for the various bits, provided such short names are
> acceptable. I'd really hate something like REG_HOURS_MODE12.

Your suggestions are definitely way too short; I'd go for something
in the middle like CTRL_OSF and HOURS_12HR, HOURS_PM.

-- PMM