Received-SPF: pass (google.com: domain of linux-kernel+bounces-166032-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <b3621642-485f-42d4-a9d2-0ccca63e219b@linaro.org>
Date: Thu, 2 May 2024 08:03:10 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] clk: samsung: gs101: mark some apm UASC and XIU clocks
 critical
To: =?UTF-8?Q?Andr=C3=A9_Draszik?= <andre.draszik@linaro.org>,
 Peter Griffin <peter.griffin@linaro.org>,
 Krzysztof Kozlowski <krzk@kernel.org>,
 Sylwester Nawrocki <s.nawrocki@samsung.com>,
 Chanwoo Choi <cw00.choi@samsung.com>, Alim Akhtar <alim.akhtar@samsung.com>,
 Michael Turquette <mturquette@baylibre.com>, Stephen Boyd <sboyd@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org,
 linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org,
 Will McVicker <willmcvicker@google.com>, kernel-team@android.com
References: <20240430-gs101-apm-clocks-v1-1-b2e2335e84f5@linaro.org>
Content-Language: en-US
From: Tudor Ambarus <tudor.ambarus@linaro.org>
In-Reply-To: <20240430-gs101-apm-clocks-v1-1-b2e2335e84f5@linaro.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit


On 4/30/24 10:54, André Draszik wrote:
> The system hangs when any of these clocks are turned off.
> 
> With the introduction of pinctrl clock support [1], the approach taken
> in this clock driver for the APM clocks to rely solely on the
> clk_ignore_unused kernel command line option does not work anymore and
> the system hangs during boot.
> 
> gout_apm_func is a parent clock to the clocks that are going to be
> handled by the pinctrl driver [2], namely
> gout_apm_apbif_gpio_alive_pclk and gout_apm_apbif_gpio_far_alive_pclk.
> It also is the parent to the clocks marked as critical in this commit
> here (and some others that aren't relevant for this commit)). This
> means that once the pinctrl driver decides to turn off clocks, the
> clock framework will subsequently turn off parent clocks of those
> pinctrl clocks if they have no (apparent) user. Since gout_apm_func is
> the parent, and since no drivers are hooked up to it or any of its
> other children, gout_apm_func will be turned off. This will cause the
> system to hang, as the clocks marked as critical in this commit stop
> having an input.
> 
> We might have to add a driver for these clocks, but in the meantime
> let's just ensure they stay on even if siblings are turned off.

It's sane, yes.

> 
> For the avoidance of doubt: This commit doesn't mean that we can boot
> with clk_ignore_unused.
> 

s/with/without. Because there are still other clocks that are not yet
handled/marked as critical where needed. There's still work to do.

With the typo addressed:
Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>