lkml.org 
[lkml]   [2020]   [Jan]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3] clocksource: Add driver for the Ingenic JZ47xx OST


Le mer., janv. 15, 2020 at 20:54, Thomas Gleixner <tglx@linutronix.de>
a écrit :
> Paul Cercueil <paul@crapouillou.net> writes:
>> Le mer., janv. 15, 2020 at 18:48, Maarten ter Huurne
>> <maarten@treewalker.org> a écrit :
>>> On Wednesday, 15 January 2020 14:57:01 CET Paul Cercueil wrote:
>>>> Le mer., janv. 15, 2020 at 14:44, Daniel Lezcano
>>>> <daniel.lezcano@linaro.org> a écrit :
>>>> > Is the JZ47xx OST really a mfd needing a regmap? (Note
>>>> regmap_read
>>>> > will take a lock).
>>>>
>>>> Yes, the TCU_REG_OST_TCSR register is shared with the clocks
>>>> driver.
>>>
>>> The TCU_REG_OST_TCSR register is only used in the probe though.
>>>
>>> To get the counter value from TCU_REG_OST_CNTL/TCU_REG_OST_CNTH you
>>> could technically do it by reading the register directly, if
>>> performance
>>> concerns make it necessary to bypass the usual kernel
>>> infrastructure
>>> for
>>> dealing with shared registers.
>>
>> In theory yes, in practice there's no easy way to do that (the
>> underlying mmio pointer is not obtainable from the regmap), and
>> besides, the lock is just a spinlock and not a mutex.
>
> That lock still a massive contention point as clock readouts can be
> pretty
> frequent depending on workloads. Just think about tracing ...
>
> So I really would avoid both the lock and that ugly 64bit readout
> thing.

The 64bit readout thing is gone in V3.

The lock cannot go away unless we have a way to retrieve the underlying
mmio pointer from the regmap, which the regmap maintainers will never
accept. So I can't really change that now. Besides,
drivers/clocksource/ingenic-timer.c also registers a clocksource that's
read with the regmap, and nobody complained.

-Paul


\
 
 \ /
  Last update: 2020-01-15 22:00    [W:0.071 / U:5.888 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site