linux-kernel - Re: [PATCH v2 2/2] clk: ingenic-tcu: Fix missing TCU clock for X1000 SoC

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: <VAMA7R.PG2P5IFR07772@crapouillou.net>
Date:   Mon, 14 Feb 2022 11:52:07 +0000
From:   Paul Cercueil <paul@...pouillou.net>
To:     Aidan MacDonald <aidanmacdonald.0x0@...il.com>
Cc:     robh+dt@...nel.org, mturquette@...libre.com, sboyd@...nel.org,
        linux-mips@...r.kernel.org, devicetree@...r.kernel.org,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] clk: ingenic-tcu: Fix missing TCU clock for X1000
 SoC

Hi Aidan,

Le sam., févr. 12 2022 at 15:09:28 +0000, Aidan MacDonald 
<aidanmacdonald.0x0@...il.com> a écrit :
> The X1000 does have a TCU clock gate, so pass it to the driver.
> Without this the TCU can be gated automatically, which prevents
> timers from running.
> 
> Fixes: dc6a81c3382f74fe ("clk: Ingenic: Add support for TCU of 
> X1000.")
> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@...il.com>
> ---
> I've just realized, maybe this is an ABI break. Now that the TCU 
> clock is
> required, the driver probe will fail if given an old device tree 
> which is
> missing that clock. Is it necessary to add a hack of some sort to 
> support
> the old device tree?

Yes, that's a valid concern. The driver should then support the TCU 
clock being missing (but only for the x1000), with a comment that 
explain why the workaround exists.

You can use of_clk_get_by_name(), and if you get -EINVAL and the 
workaround flag is set, allow the driver to continue. Also change the 
checks for (tcu->soc_info->has_tcu_clk) in the function's cleanup to 
checks for (tcu->clk) so that the clk_disable_unprepare/clk_put are 
only done on a valid pointer.

Note that the x1830 also has a TCU clock that's not specified in the 
device tree; so you could add a patch similar to your current [1/2] 
that adds it to x1830.dtsi as well. It uses the "ingenic,x1000-tcu" 
string as fallback, so the driver wouldn't have to be modified further.

Cheers,
-Paul

> 
>  drivers/clk/ingenic/tcu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/ingenic/tcu.c b/drivers/clk/ingenic/tcu.c
> index 77acfbeb4830..9c86043f673a 100644
> --- a/drivers/clk/ingenic/tcu.c
> +++ b/drivers/clk/ingenic/tcu.c
> @@ -320,7 +320,7 @@ static const struct ingenic_soc_info 
> jz4770_soc_info = {
>  static const struct ingenic_soc_info x1000_soc_info = {
>  	.num_channels = 8,
>  	.has_ost = false, /* X1000 has OST, but it not belong TCU */
> -	.has_tcu_clk = false,
> +	.has_tcu_clk = true,
>  };
> 
>  static const struct of_device_id __maybe_unused 
> ingenic_tcu_of_match[] __initconst = {
> --
> 2.34.1
>