linux-kernel - Re: [linux-next, 1/1] clk: imx: imx8mm-anatop: probe only on i.MX8MM platforms

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <c929b7d8-6348-4978-824e-6902d0364a00@linaro.org>
Date: Mon, 19 May 2025 16:13:14 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Peng Fan <peng.fan@....com>,
 Dario Binacchi <dario.binacchi@...rulasolutions.com>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Cc: "linux-amarula@...rulasolutions.com"
 <linux-amarula@...rulasolutions.com>, Mark Brown <broonie@...nel.org>,
 Abel Vesa <abelvesa@...nel.org>, Fabio Estevam <festevam@...il.com>,
 Michael Turquette <mturquette@...libre.com>,
 Pengutronix Kernel Team <kernel@...gutronix.de>,
 Sascha Hauer <s.hauer@...gutronix.de>, Shawn Guo <shawnguo@...nel.org>,
 Stephen Boyd <sboyd@...nel.org>, "imx@...ts.linux.dev"
 <imx@...ts.linux.dev>,
 "linux-arm-kernel@...ts.infradead.org"
 <linux-arm-kernel@...ts.infradead.org>,
 "linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>
Subject: Re: [linux-next, 1/1] clk: imx: imx8mm-anatop: probe only on i.MX8MM
 platforms

On 19/05/2025 14:58, Peng Fan wrote:
>> Subject: [linux-next, 1/1] clk: imx: imx8mm-anatop: probe only on
>> i.MX8MM platforms
> 
> DT Maintainer Krzysztof NACKed [1] because of ABI break.
> 
> Are we continuing breaking the ABI? 
> 
> [1] https://lore.kernel.org/imx/6a28f9bb-05fa-45ff-8c0b-790c0caf3252@kernel.org/T/#u
> 
> Regards,
> Peng.
> 
>>
>> Commit 9c1e388af87c ("clk: imx: add support for i.MX8MM anatop
>> clock
>> driver") breaks boot on i.MX8M{P,N} platforms.
>>
>> Here's the log for a board based on the i.MX8MP platform:
>>
>> [    1.439320] i.MX clk 1: register failed with -2
>> [    1.441014] i.MX clk 2: register failed with -2
>> [    1.445610] imx8mm-anatop 30360000.clock-controller: NXP
>> i.MX8MM anatop clock driver probed
>> [    1.455068] Unable to handle kernel paging request at virtual address
>> fffffffffffffffe
>>
>> ...
>>
>> [    1.634650] Call trace:
>> [    1.637102]  __clk_get_hw+0x4/0x18 (P)
>> [    1.640862]  imx8mp_clocks_probe+0xdc/0x2f50
>> [    1.645152]  platform_probe+0x68/0xc4
>> [    1.648827]  really_probe+0xbc/0x298
>> [    1.652413]  __driver_probe_device+0x78/0x12c
>>
>> In the imx8mp.dtsi device tree, the anatop compatible string is:
>>
>> compatible = "fsl,imx8mp-anatop", "fsl,imx8mm-anatop";
>>
>> So, in configurations like arm64 defconfig, where
>> CONFIG_CLK_IMX8MP and CONFIG_CLK_IMX8MM as well as
>> CONFIG_CLK_IMX8MN are enabled, the driver for the i.MX8MM
>> anatop is incorrectly loaded.
>>
>> The patch fixes the regression by ensuring that the i.MX8MM anatop
>> driver only probes on i.MX8MM platforms.
>>
>> Fixes: 9c1e388af87c ("clk: imx: add support for i.MX8MM anatop clock
>> driver")
>> Signed-off-by: Dario Binacchi <dario.binacchi@...rulasolutions.com>
>>
>> ---
>>
>>  drivers/clk/imx/clk-imx8mm-anatop.c | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/clk/imx/clk-imx8mm-anatop.c b/drivers/clk/imx/clk-
>> imx8mm-anatop.c
>> index 4ac870df6370..90ff11a93fe5 100644
>> --- a/drivers/clk/imx/clk-imx8mm-anatop.c
>> +++ b/drivers/clk/imx/clk-imx8mm-anatop.c
>> @@ -37,6 +37,19 @@ static const char * const clkout_sels[] =
>> {"audio_pll1_out", "audio_pll2_out", "
>>  static struct clk_hw_onecell_data *clk_hw_data;  static struct clk_hw
>> **hws;
>>
>> +static int is_really_imx8mm(struct device_node *np) {
>> +	const char *compat;
>> +	struct property *p;
>> +
>> +	of_property_for_each_string(np, "compatible", p, compat) {
>> +		if (strcmp(compat, "fsl,imx8mm-anatop"))
>> +			return -EFAULT;

EFAULT does not seem like proper error code for ignoring probe. I am
pretty sure it leaves you with dmesg regression.

Probably you wanted to fix driver matching. Otherwise your compatibles
are just wrong.

You claim with this:

	compatible = "fsl,imx8mp-anatop", "fsl,imx8mm-anatop";

That 8mp is fully compatible with 8mm, yet here you claim that 8mm is
not handled. So it is both compatible and not compatible.

There is some gross misunderstanding what the compatibles mean. Please
look at DT spec. Shortly explaining: fallback (8mm) means new device
will work somehow correctly, but maybe with limited features, with the
driver when bound by the fallback.

Other meanings are most likely wrong.

I consider that "Support spread spectrum clocking for i.MX8M PLLs"
patchset broken (to which I was pointing on early discussions) and
should be dropped. Don't fix broken stuff with more broken code.

Best regards,
Krzysztof