Messages in this thread |  | | Subject | Re: [PATCH v3] clocksource: Add driver for the Ingenic JZ47xx OST | From | Daniel Lezcano <> | Date | Sat, 8 Feb 2020 08:09:45 +0100 |
| |
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.
-- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
|  |