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, 11 Oct 2021 11:47:18 +0530
From:   Manivannan Sadhasivam <mani@kernel.org>
To:     Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Cc:     Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>, linuxarm@huawei.com,
        mauro.chehab@huawei.com, linux-clk@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/2] clk: fix the need of booking clk_ignore_unused=true
 on embedded devs
Message-ID: <20211011061718.GB1834@thinkpad>
References: <cover.1633607765.git.mchehab+huawei@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <cover.1633607765.git.mchehab+huawei@kernel.org>
Precedence: bulk

Hi Mauro,

On Thu, Oct 07, 2021 at 02:06:53PM +0200, Mauro Carvalho Chehab wrote:
> Currently, the only way to boot a Kernel with drivers built as modules on embedded 
> devices like HiKey 970 is to pass clk_ignore_unused=true as a modprobe parameter.
> 
> There are two separate issues:
> 
> 1. the clk's core calls clk_disable_unused() too early. By the time this
>    function is called, only the builtin drivers were already probed/initialized.
>    Drivers built as modules will only be probed afterwards.
> 
>    This cause a race condition and boot instability, as the clk core will try
>    to disable clocks while the drivers built as modules are still being
>    probed and initialized.

So you are mentioning a "race" condition here but it is not mentioned in the
actual patch. If the issue you are seeing is because the clocks used by the
modules are disabled before they are probed, why can't they just enable the
clocks during the probe time?

Am I missing something?

Thanks,
Mani

> 
>    I suspect that the same problem used to happen at the regulator's core,
>    as there's a code that waits for 30 seconds before disabling unused
>    regulators;
> 
> 2. there are some gate clocks defined at HiKey 970 that should always be on,
>    as otherwise the system will hang, or the filesystem I/O will stop.
> 
> Ps.: 
>   I submitted already 3 or 4 versions of patches for HiKey 970 clock, but
>   they're all unreliable, due to the race conditions at the clk core due to (1).
>    
> Patch 1 solves the issue with the clk core.
> Patch 2 solves the HiKey 970 specific issues.
> 
> Mauro Carvalho Chehab (2):
>   clk: wait for extra time before disabling unused clocks
>   clk: clk-hi3670: mark some clocks as CLK_IS_CRITICAL
> 
>  drivers/clk/clk.c                  | 51 +++++++++++++++++++-----------
>  drivers/clk/hisilicon/clk-hi3670.c | 24 +++++++-------
>  2 files changed, 44 insertions(+), 31 deletions(-)
> 
> -- 
> 2.31.1
> 
>