Messages in this thread | | | Date | Fri, 14 Jan 2022 12:03:41 +0100 | From | Laurent Vivier <> | Subject | Re: [PATCH v6 3/4] clocksource/drivers: Add a goldfish-timer clocksource |
| |
Le 14/01/2022 à 11:46, Arnd Bergmann a écrit : > On Thu, Jan 13, 2022 at 9:19 PM Laurent Vivier <laurent@vivier.eu> wrote: >> >> Add a clocksource based on the goldfish-rtc device. >> >> Signed-off-by: Laurent Vivier <laurent@vivier.eu> >> --- >> drivers/clocksource/Kconfig | 7 ++ >> drivers/clocksource/Makefile | 1 + >> drivers/clocksource/timer-goldfish.c | 130 +++++++++++++++++++++++++++ >> include/clocksource/timer-goldfish.h | 12 +++ >> 4 files changed, 150 insertions(+) >> create mode 100644 drivers/clocksource/timer-goldfish.c >> create mode 100644 include/clocksource/timer-goldfish.h >> >> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig >> index f65e31bab9ae..6ca9bb78407d 100644 >> --- a/drivers/clocksource/Kconfig >> +++ b/drivers/clocksource/Kconfig >> @@ -711,4 +711,11 @@ config MICROCHIP_PIT64B >> modes and high resolution. It is used as a clocksource >> and a clockevent. >> >> +config GOLDFISH_TIMER >> + bool "Clocksource using goldfish-rtc" >> + select RTC_CLASS >> + select RTC_DRV_GOLDFISH > > This should probably be > > depends on M68K || COMPILE_TEST > depends on RTC_DRV_GOLDFISH > > A driver should never 'select' another user-selectable subsystem
ok
> >> +static int goldfish_timer_set_oneshot(struct clock_event_device *evt) >> +{ >> + struct goldfish_timer *timerdrv = ced_to_gf(evt); >> + void __iomem *base = timerdrv->base; >> + >> + __raw_writel(0, base + TIMER_ALARM_HIGH); >> + __raw_writel(0, base + TIMER_ALARM_LOW); >> + __raw_writel(1, base + TIMER_IRQ_ENABLED); > > As mentioned elsewhere, the __raw_* accessors are not portable, please > use readl()/writel() here, or possibly ioread32_be()/iowrite32_be() for > the big-endian variant.
We can't use readl()/writel() here because it's supposed to read from a little endian device, and goldfish endianness depends on the endianness of the machine.
For goldfish, readl()/writel() works fine on little-endian machine but not on big-endian machine.
On m68k, you have:
#define readl(addr) in_le32(addr) #define writel(val,addr) out_le32((addr),(val))
and with goldfish it's wrong as the device is not little-endian, it is big-endian like the machine.
same comment with ioread32_be()/iowrite32_be(): it will work on big-endian machine not on little-endian.
We need an accessor that doesn't byteswap the value, that accesses it natively, and in all other parts of the kernel __raw_writel() and __raw_readl() are used.
Thanks, Laurent
| |