Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 6 Sep 2021 10:18:04 +0100
From:   Lee Jones <lee.jones@linaro.org>
To:     Vladimir Oltean <vladimir.oltean@nxp.com>
Cc:     Mark Brown <broonie@kernel.org>,
        Rasmus Villemoes <linux@rasmusvillemoes.dk>,
        Arnd Bergmann <arnd@arndb.de>,
        Thomas Gleixner <tglx@linutronix.de>,
        Marc Zyngier <maz@kernel.org>,
        Hou Zhiqiang <Zhiqiang.Hou@nxp.com>,
        Biwen Li <biwen.li@nxp.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] mfd: syscon: request a regmap with raw spinlocks for
 some devices
Message-ID: <YTXczJbHYCgkpVGu@google.com>
References: <20210825205041.927788-1-vladimir.oltean@nxp.com>
 <20210825205041.927788-3-vladimir.oltean@nxp.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20210825205041.927788-3-vladimir.oltean@nxp.com>
Precedence: bulk

On Wed, 25 Aug 2021, Vladimir Oltean wrote:

> This patch solves a ls-extirq irqchip driver bug in a perhaps
> non-intuitive (at least non-localized) way.
> 
> The issue is that ls-extirq uses regmap, and due to the fact that it is
> being called by the IRQ core under raw spinlock context, it needs to use
> raw spinlocks itself. So it needs to request raw spinlocks from the
> regmap config.
> 
> All is fine so far, except the ls-extirq driver does not manage its own
> regmap, instead it uses syscon_node_to_regmap() to get it from the
> parent syscon (this driver).
> 
> Because the syscon regmap is initialized before any of the consumer
> drivers (ls-extirq) probe, we need to know beforehand whether to request
> raw spinlocks or not.
> 
> The solution seems to be to check some compatible string. The ls-extirq
> driver probes on quite a few NXP Layerscape SoCs, all with different
> compatible strings. This is potentially fragile and subject to bit rot
> (since the fix is not localized to the ls-extirq driver, adding new
> compatible strings there but not here seems plausible). Anyway, it is
> probably the best we can do without major rework.
> 
> Suggested-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  drivers/mfd/syscon.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
> index 765c0210cb52..70da4e87b072 100644
> --- a/drivers/mfd/syscon.c
> +++ b/drivers/mfd/syscon.c
> @@ -83,6 +83,22 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_clk)
>  	if (ret)
>  		reg_io_width = 4;
>  
> +	/*
> +	 * We might be providing a regmap to e.g. an irqchip driver, and in
> +	 * that case, normal spinlocks won't do: the IRQ core holds raw
> +	 * spinlocks, so it needs to be raw spinlocks all the way down.
> +	 * Detect those drivers here (currently "ls-extirq") and request raw
> +	 * spinlocks in the regmap config for them.
> +	 */
> +	if (of_device_is_compatible(np, "fsl,lx2160a-isc") ||
> +	    of_device_is_compatible(np, "fsl,ls2080a-isc") ||
> +	    of_device_is_compatible(np, "fsl,ls2080a-isc") ||
> +	    of_device_is_compatible(np, "fsl,ls1088a-isc") ||
> +	    of_device_is_compatible(np, "fsl,ls1043a-scfg") ||
> +	    of_device_is_compatible(np, "fsl,ls1046a-scfg") ||
> +	    of_device_is_compatible(np, "fsl,ls1021a-scfg"))
> +		syscon_config.use_raw_spinlock = true;
> +

Since syscon is meant to be a generic solution, I'd like to avoid
spraying platform specific hacks throughout.  So, *IF* this is the
chosen solution, I'd prefer to solve this with a generic DT property,
rather than matching on a bunch of compatible strings.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog