| Message ID | 20240430-gs101-apm-clocks-v1-1-b2e2335e84f5@linaro.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | clk: samsung: gs101: mark some apm UASC and XIU clocks critical | expand |
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>
On 02/05/2024 09:03, Tudor Ambarus wrote: >> 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: Thanks, applied with fix above. Best regards, Krzysztof
On Tue, 30 Apr 2024 10:54:59 +0100, 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. > > [...] Applied, thanks! [1/1] clk: samsung: gs101: mark some apm UASC and XIU clocks critical https://git.kernel.org/krzk/linux/c/6b32d7474e9b833dc7fadc1b4d4f08af9bd87fde Best regards,
diff --git a/drivers/clk/samsung/clk-gs101.c b/drivers/clk/samsung/clk-gs101.c index 05129c3b2f68..e2a6a1992505 100644 --- a/drivers/clk/samsung/clk-gs101.c +++ b/drivers/clk/samsung/clk-gs101.c @@ -1896,16 +1896,16 @@ static const struct samsung_gate_clock apm_gate_clks[] __initconst = { CLK_CON_GAT_GOUT_BLK_APM_UID_UASC_G_SWD_IPCLKPORT_PCLK, 21, 0, 0), GATE(CLK_GOUT_APM_UASC_P_APM_ACLK, "gout_apm_uasc_p_apm_aclk", "gout_apm_func", - CLK_CON_GAT_GOUT_BLK_APM_UID_UASC_P_APM_IPCLKPORT_ACLK, 21, 0, 0), + CLK_CON_GAT_GOUT_BLK_APM_UID_UASC_P_APM_IPCLKPORT_ACLK, 21, CLK_IS_CRITICAL, 0), GATE(CLK_GOUT_APM_UASC_P_APM_PCLK, "gout_apm_uasc_p_apm_pclk", "gout_apm_func", - CLK_CON_GAT_GOUT_BLK_APM_UID_UASC_P_APM_IPCLKPORT_PCLK, 21, 0, 0), + CLK_CON_GAT_GOUT_BLK_APM_UID_UASC_P_APM_IPCLKPORT_PCLK, 21, CLK_IS_CRITICAL, 0), GATE(CLK_GOUT_APM_WDT_APM_PCLK, "gout_apm_wdt_apm_pclk", "gout_apm_func", CLK_CON_GAT_GOUT_BLK_APM_UID_WDT_APM_IPCLKPORT_PCLK, 21, 0, 0), GATE(CLK_GOUT_APM_XIU_DP_APM_ACLK, "gout_apm_xiu_dp_apm_aclk", "gout_apm_func", - CLK_CON_GAT_GOUT_BLK_APM_UID_XIU_DP_APM_IPCLKPORT_ACLK, 21, 0, 0), + CLK_CON_GAT_GOUT_BLK_APM_UID_XIU_DP_APM_IPCLKPORT_ACLK, 21, CLK_IS_CRITICAL, 0), }; static const struct samsung_cmu_info apm_cmu_info __initconst = {
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. For the avoidance of doubt: This commit doesn't mean that we can boot with clk_ignore_unused. Link: https://lore.kernel.org/r/20240426-samsung-pinctrl-busclock-v3-0-adb8664b8a7e@linaro.org [1] Link: https://lore.kernel.org/r/20240429-samsung-pinctrl-busclock-dts-v1-0-5e935179f3ca@linaro.org [2] Signed-off-by: André Draszik <andre.draszik@linaro.org> --- drivers/clk/samsung/clk-gs101.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) --- base-commit: d04466706db5e241ee026f17b5f920e50dee26b5 change-id: 20240430-gs101-apm-clocks-fb0918ceaecb Best regards,