From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E81F5C433EF for ; Mon, 14 Feb 2022 11:52:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1351722AbiBNLwb convert rfc822-to-8bit (ORCPT ); Mon, 14 Feb 2022 06:52:31 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:54504 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235729AbiBNLwZ (ORCPT ); Mon, 14 Feb 2022 06:52:25 -0500 Received: from aposti.net (aposti.net [89.234.176.197]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9BD44C34; Mon, 14 Feb 2022 03:52:17 -0800 (PST) Date: Mon, 14 Feb 2022 11:52:07 +0000 From: Paul Cercueil Subject: Re: [PATCH v2 2/2] clk: ingenic-tcu: Fix missing TCU clock for X1000 SoC To: Aidan MacDonald Cc: robh+dt@kernel.org, mturquette@baylibre.com, sboyd@kernel.org, linux-mips@vger.kernel.org, devicetree@vger.kernel.org, linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org Message-Id: In-Reply-To: <20220212150927.39513-2-aidanmacdonald.0x0@gmail.com> References: <20220212150927.39513-1-aidanmacdonald.0x0@gmail.com> <20220212150927.39513-2-aidanmacdonald.0x0@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1; format=flowed Content-Transfer-Encoding: 8BIT Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Aidan, Le sam., févr. 12 2022 at 15:09:28 +0000, Aidan MacDonald 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 > --- > 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 >