lkml.org 
[lkml]   [2022]   [Jan]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2022-01-14 12:04    [W:0.085 / U:0.592 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site