Thread (43 messages) 43 messages, 5 authors, 2024-01-29

Re: [PATCH 9/9] clk: samsung: gs101: don't CLK_IGNORE_UNUSED peric1_sysreg clock

From: Krzysztof Kozlowski <hidden>
Date: 2024-01-29 14:08:57
Also in: linux-clk, linux-devicetree, linux-samsung-soc, lkml

On 29/01/2024 14:47, André Draszik wrote:
Hi Krzysztof,

On Mon, 2024-01-29 at 12:03 +0100, Krzysztof Kozlowski wrote:
quoted
On 27/01/2024 01:19, André Draszik wrote:
quoted
Now that we have hooked it up in the DTS, we can drop the
Your driver patch cannot depend on DTS. Not for a new platform. I am
repeating this all the time last days...
quoted
CLK_IGNORE_UNUSED from here.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/clk/samsung/clk-gs101.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/samsung/clk-gs101.c b/drivers/clk/samsung/clk-gs101.c
index 7f6c3b52d9ff..d55ed64d0e29 100644
--- a/drivers/clk/samsung/clk-gs101.c
+++ b/drivers/clk/samsung/clk-gs101.c
@@ -3393,7 +3393,7 @@ static const struct samsung_gate_clock peric1_gate_clks[] __initconst = {
 	GATE(CLK_GOUT_PERIC1_SYSREG_PERIC1_PCLK,
 	     "gout_peric1_sysreg_peric1_pclk", "mout_peric1_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_PERIC1_UID_SYSREG_PERIC1_IPCLKPORT_PCLK,
-	     21, CLK_IGNORE_UNUSED, 0),
I don't understand. You just added this clock in this patchset. This
means that your patch #3 is incorrect.
In patch #3 I'm hooking up all the clocks to Linux. If I don't CLK_IGNORE_UNUSED
for the 'sysreg' pclk in patch #3, then it'll hang on loading drivers that
require sysreg access (because Linux disabled the clock).
Then add clk_ignore_unused to cmdline. That's anyway recommended for
development platforms without full clock and pd description
(pd_ignore_unused). Not mentioning that we might default to
clk_ignore_unused at some point soon.
I can not change patch #8 to come between 2 and 3 either, because at that stage
neither the clock nor the DT node reference &cmu_peric1 actually exist, and the
clock and can't be claimed by sysreg.
At the point of me applying this patch, there will be no DTS node
either. This ordering fixes nothing.
Since we can not mix DT and driver changes in the same commit, I can not merge
patches #3 and #4 and #8 either.

I had to do it this way so that the platform always boots for every commit to keep
things bisectable.
But it is not bisectable - you did not fix anything. You can try by
yourself:
# git checkout drivers
# git am patch #1, #2, #3 and #9
# git checkout dt
# git am patch #4, #5, #6, #7, #8

and now try to bisect it. You will have the same problems you try to
avoid. So what is solved by this ordering? Nothing.

Alternatively, I could merge patches #4 and #8 (but that seems wrong to me), or
drop patches #7, #8 and #9 from this series and apply it later in the -rc phase?
Probably the mistake was done in the way how you upstream things: adding
sysreg syscon without its clocks.

Additionally:
1. Disabling unused clocks is current OS policy, so why the policy
should affect DTS and driver ordering?
2. This is platform did not receive a release kernel, so glitches are okay.

For this case #9 must be squashed with #3. #4 with #9.

Is there a better way that you have in mind that we're missing, that keeps things
atomic and bootable/bisectable? 

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help