lkml.org 
[lkml]   [2020]   [Feb]   [8]   [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
Hi Daniel,

Le sam., févr. 8, 2020 at 08:09, Daniel Lezcano
<daniel.lezcano@linaro.org> a écrit :
> Hi Paul,
>
> On 15/01/2020 21:58, Paul Cercueil wrote:
>>
>>
>> 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.
>
> Is there any progress on this? Having a lock in this code path is very
> impacting.

I have a v4 ready, I'm just waiting for 5.6-rc1 to start to rebase on
top and send it.

-Paul


\
 
 \ /
  Last update: 2020-02-08 14:24    [W:0.069 / U:0.736 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site