Re: [PATCH][next] rtc: msc313: Fix unintentional sign extension issue on left shift of a u16

[Colin Ian King <colin.king@canonical.com>]

On 28/09/2021 14:31, Daniel Palmer wrote:
> Hi Colin,
> 
> On Tue, 28 Sept 2021 at 21:39, Colin King <colin.king@canonical.com> wrote:
>> Shifting the u16 value returned by readw by 16 bits to the left
>> will be promoted to a 32 bit signed int and then sign-extended
>> to an unsigned long. If the top bit of the readw is set then
>> the shifted value will be sign extended and the top 32 bits of
>> the result will be set.
> 
> Ah,.. C is fun in all the wrong places. :)
> These chips are full of 32bit registers that are split into two 16
> registers 4 bytes apart when seen from the ARM CPU so we probably have
> this same mistake in a few other places.
> 
> A similar pattern is used a bit later on in the same file to read the counter:
> 
> seconds = readw(priv->rtc_base + REG_RTC_CNT_VAL_L)
> | (readw(priv->rtc_base + REG_RTC_CNT_VAL_H) << 16);

Ah, I missed that one! I'll send a V2.

> 
> I guess it works at the moment because the top bit won't be set until 2038.

I hope to be retired by then, but I guess fixing it up before 2038 is a
good idea ;-)

> 
> Thanks,
> 
> Daniel
>