Re: [PATCH] irqchip: stm32: add a second level init to request hwspinlock

From: Marc Zyngier <marc.zyngier@arm.com>
To: Fabien Dessenne <fabien.dessenne@st.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Jason Cooper <jason@lakedaemon.net>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Alexandre Torgue <alexandre.torgue@st.com>,
	linux-kernel@vger.kernel.org,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org
Cc: Benjamin Gaignard <benjamin.gaignard@st.com>
Subject: Re: [PATCH] irqchip: stm32: add a second level init to request hwspinlock
Date: Thu, 7 Mar 2019 16:44:21 +0000	[thread overview]
Message-ID: <97040a1e-7a24-3d41-c3b7-43b551a70825@arm.com> (raw)
In-Reply-To: <1551975829-16350-1-git-send-email-fabien.dessenne@st.com>

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.
-- 
Jazz is not dead. It just smells funny...