On 31 October 2017 at 19:37, Mark Brown <broo...@kernel.org> wrote: > On Tue, Oct 31, 2017 at 07:31:57PM +0800, Baolin Wang wrote: >> On 31 October 2017 at 18:38, Mark Brown <broo...@kernel.org> wrote: > >> > Given that we have no error handling path on the locks should we be >> > supporting timeout mode at all? Otherwise we should probably add a >> > set of error handling paths whenever we take the lock... > >> It will be more helpful to use the timeout to try more times to get >> the hwlock, and we usually do not use hwspin_trylock_xxx(), so we can >> remove hwspin_trylock_xxx() support and set timeout as MAX value as >> default to avoid adding 'hwlock_timeout' config, >> is this OK for you? > > I *think* so - but let's see the code. It might make sense to do two > patches, one with the base hwspinlock support then another adding the > timeout functionality. That way if there's any problem we can still > merge the non-timeout code and there's less to review next time.
What I mean is we only introduce the timeout functions, since we do not want to get the hwlocks failed to avoid error handling path: static void regmap_lock_hwlock(void *__map) { struct regmap *map = __map; hwspin_lock_timeout(map->hwlock, ~0U); } static void regmap_lock_hwlock_irq(void *__map) { struct regmap *map = __map; hwspin_lock_timeout_irq(map->hwlock, ~0U); } static void regmap_lock_hwlock_irqsave(void *__map) { struct regmap *map = __map; hwspin_lock_timeout_irqsave(map->hwlock, ~0U, &map->spinlock_flags); } -- Baolin.wang Best Regards