linux-kernel - Re: [PATCH v2 3/3] mfd: syscon: Allow syscon nodes without a "syscon" compatible

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b2413460-ec8b-4c77-99b8-4c32b262439a@ti.com>
Date: Wed, 22 Jan 2025 15:13:48 +0530
From: Vaishnav Achath <vaishnav.a@...com>
To: "Rob Herring (Arm)" <robh@...nel.org>, Lee Jones <lee@...nel.org>,
        Arnd
 Bergmann <arnd@...db.de>,
        Pankaj Dubey <pankaj.dubey@...sung.com>,
        Heiko
 Stuebner <heiko@...ech.de>, Liviu Dudau <liviu.dudau@....com>,
        Sudeep Holla
	<sudeep.holla@....com>,
        Lorenzo Pieralisi <lpieralisi@...nel.org>
CC: Peter Griffin <peter.griffin@...aro.org>,
        Will McVicker
	<willmcvicker@...gle.com>,
        John Madieu <john.madieu.xa@...renesas.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        <linux-kernel@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
        Andrew Davis <afd@...com>, Nishanth Menon <nm@...com>,
        Vignesh Raghavendra
	<vigneshr@...com>,
        "Kumar, Udit" <u-kumar1@...com>
Subject: Re: [PATCH v2 3/3] mfd: syscon: Allow syscon nodes without a "syscon"
 compatible

Hi Rob,

On 17/12/24 23:41, Rob Herring (Arm) wrote:
> of_syscon_register_regmap() was added for nodes which need a custom
> regmap setup. It's not really correct for those nodes to claim they are
> compatible with "syscon" as the default handling likely doesn't work in
> those cases. If device_node_get_regmap() happens to be called first,
> then of_syscon_register() will be called and an incorrect regmap will be
> created (barring some other error). That may lead to unknown results in
> the worst case. In the best case, of_syscon_register_regmap() will fail
> with -EEXIST. This problem remains unless these cases drop "syscon" (an
> ABI issue) or we exclude them using their specific compatible. ATM,
> there is only one user: "google,gs101-pmu"
> 
> There are also cases of adding "syscon" compatible to existing nodes
> after the fact in order to register the syscon. That presents a
> potential DT ABI problem. Instead, if there's a kernel change needing a
> syscon for a node, then it should be possible to allow the kernel to
> register a syscon without a DT change. That's only possible by using
> of_syscon_register_regmap() currently, but in the future we may want to
> support a match list for cases which don't need a custom regmap.
> 
> With this change, the lookup functions will succeed for any node
> registered by of_syscon_register_regmap() regardless of whether the node
> compatible contains "syscon".
> 
> Signed-off-by: Rob Herring (Arm) <robh@...nel.org>
> ---
> v2:
>   - Fix logic when a syscon is found in list to not return an error
> ---
>   drivers/mfd/syscon.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
> index bfb1f69fcff1d3cd35cf04ccd4c449e7d0395c79..226915ca3c93dcaf47bdd46b58e00e10e155f952 100644
> --- a/drivers/mfd/syscon.c
> +++ b/drivers/mfd/syscon.c
> @@ -171,9 +171,12 @@ static struct regmap *device_node_get_regmap(struct device_node *np,
>   			break;
>   		}
>   
> -	if (!syscon)
> -		syscon = of_syscon_register(np, check_res);
> -
> +	if (!syscon) {
> +		if (of_device_is_compatible(np, "syscon"))
> +			syscon = of_syscon_register(np, check_res);
> +		else
> +			syscon = ERR_PTR(-EINVAL);
> +	}

Earlier the above check was only in syscon_node_to_regmap() , but now 
the users of device_node_to_regmap() also undergoes this check and there 
are few drivers in tree which uses device_node_to_regmap() without 
having "syscon" in compatible, likely those drivers need to be fixed, 
but this patch breaks all those drivers now, atleast the below drivers 
are affected and all TI platforms fail to boot due to this:

drivers/soc/ti/k3-socinfo.c
drivers/mux/mmio.c ("reg-mux" users)
drivers/clk/keystone/syscon-clk.c

All the above drivers fail due to the above check for syscon compatible.

What is the recommended solution to fix this?

Thanks and Regards,
Vaishnav

>   	mutex_unlock(&syscon_list_lock);
>   
>   	if (IS_ERR(syscon))
> @@ -238,9 +241,6 @@ EXPORT_SYMBOL_GPL(device_node_to_regmap);
>   
>   struct regmap *syscon_node_to_regmap(struct device_node *np)
>   {
> -	if (!of_device_is_compatible(np, "syscon"))
> -		return ERR_PTR(-EINVAL);
> -
>   	return device_node_get_regmap(np, true);
>   }
>   EXPORT_SYMBOL_GPL(syscon_node_to_regmap);
>