lkml.org 
[lkml]   [2019]   [Mar]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] irqchip: stm32: add a second level init to request hwspinlock
Date
Hi Marc,

Thank you for your feedback. Let me try to explain this patch, and the
reason of its unusual implementation choices.


Regarding the driver init mode:
As an important requirement, I want to keep this irq driver declared
with IRQCHIP_DECLARE(), so it is initialized early from
start_kernel()/init_IRQ().
Most of the other irq drivers are implemented this way and I imagine
that this ensures the availability of the irq drivers, before the other
platform drivers get probed.



Regarding the second init:
With the usage of the hwspinlock framework (used to protect against
coprocessor concurrent access to registers) we have a problem as the
hwspinlock driver is not available when the irq driver is being initialized.
In order to solve this, I added a second initialization where we get a
reference to hwspinlock.
You pointed that we are not supposed to use of_node_clear_flag (which
allows to get a second init call) :
I spent some time to find any information about it, but could not find
any reason to not use it.
Please, let me know if I missed something here.



Regarding the inits sequence and dependencies:
- The second init is guaranteed to be called after the first one, since
start_kernel()->init_IRQ() is called before platform drivers init.
- During the second init, the dependency with the hwspinlock driver is
implemented correctly : it makes use of defered probe when needed.



I understand that this patch is 'surprising' but I hope that my
explanations justify its implementation.
Waiting for your feedback

BR

Fabien

On 07/03/2019 5:44 PM, Marc Zyngier wrote:
> On 07/03/2019 16:23, Fabien Dessenne wrote:
>> Requesting hwspinlock, at the first time it is used, is not correct:
>> indeed, at that moment we are under raw_spin_lock_irqsave() context and
>> hwspin_lock_request_specific() may sleep ("BUG: sleeping function called
>> from invalid context").
>> Requesting hwspinlock during the init (stm32*_exti_of_init()) is also
>> not possible (the hwspinlock framework is not ready at that stage of the
>> kernel init).
>> As a consequence, add a second level init (probed with arch_initcall)
>> where we can safely request hwspinlock.
> No, this is fairly broken. You're playing with stuff you're not supposed
> to (OF_POPULATE? really?), and adding initcalls is completely unreliable
> (things depend on the link order and will randomly break).
>
> If you need dependencies, implement them correctly. Turn this driver
> into a real device driver (in the platform device sense), and return
> PROBE_DEFER when you can't find your dependency.
>
> Thanks,
>
> M.
\
 
 \ /
  Last update: 2019-03-08 15:05    [W:0.066 / U:1.464 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site