linux-kernel - Re: [PATCH 1/2] rtc: m48t59: Accommodate chips that lack a century bit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMuHMdWkcocokX7fgyt6baQdnr11F9a-apwNCAwTUf9Q2Ypauw@mail.gmail.com>
Date: Thu, 3 Oct 2024 09:46:52 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Finn Thain <fthain@...ux-m68k.org>
Cc: Alexandre Belloni <alexandre.belloni@...tlin.com>, Daniel Palmer <daniel@...f.com>, 
	Michael Pavone <pavone@...rodev.com>, linux-m68k@...ts.linux-m68k.org, 
	linux-rtc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] rtc: m48t59: Accommodate chips that lack a century bit

Hi Finn,

On Thu, Oct 3, 2024 at 5:27 AM Finn Thain <fthain@...ux-m68k.org> wrote:
> The m48t59 driver is needed by both SPARC and MVME systems. Linux on
> MVME uses 1970 as "year zero" rather than 1968 that's used on SPARC.
> Add support for the MVME convention. Otherwise, the RTC on non-SPARC
> systems can only read and write dates between 1900 and 1999.
>
> Tested-by: Daniel Palmer <daniel@...f.com>
> Signed-off-by: Finn Thain <fthain@...ux-m68k.org>

Thanks for your patch!

> --- a/drivers/rtc/rtc-m48t59.c
> +++ b/drivers/rtc/rtc-m48t59.c
> @@ -57,6 +57,17 @@ m48t59_mem_readb(struct device *dev, u32 ofs)
>         return readb(m48t59->ioaddr+ofs);
>  }
>
> +/*
> + * Sun SPARC machines count years since 1968. MVME machines running Linux
> + * count years since 1970.
> + */
> +
> +#ifdef CONFIG_SPARC
> +#define YEAR0 68
> +#else
 +#define YEAR0 70
> +#endif

This causes a change in behavior on other non-SPARC platforms,
if any out-of-tree platform exists that uses this driver.

So I'd rather use:

    #elif defined(CONFIG_VME)
    #define YEAR0 70
    #else
    #define YEAR0 0
    #endif

> +
>  /*
>   * NOTE: M48T59 only uses BCD mode
>   */
> @@ -82,10 +93,7 @@ static int m48t59_rtc_read_time(struct device *dev, struct rtc_time *tm)
>                 dev_dbg(dev, "Century bit is enabled\n");
>                 tm->tm_year += 100;     /* one century */
>         }
> -#ifdef CONFIG_SPARC
> -       /* Sun SPARC machines count years since 1968 */
> -       tm->tm_year += 68;
> -#endif
> +       tm->tm_year += YEAR0;

Upon closer look, the driver uses platform data, so a better solution
would be to add the year0 offset to struct m48t59_plat_data.

Another suggestion for improvement, not related to this patch,
would be to differentiate among M48T59, M48T02, and M48T08 by using
platform_driver.id_table and platform_device_id.driver_data, instead
of m48t59_plat_data.type.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds