On 17/05/2023 10:40, Krzysztof Kozlowski wrote:
On 17/05/2023 10:28, Alexandre Mergnat wrote:
Before the patch [1], the clock probe was done directly in the
clk-mt8365 driver. In this probe function, the array which stores the
data clocks is sized using the higher defined numbers (*_NR_CLOCK) in
the clock lists [2]. Currently, with the patch [1], the specific
clk-mt8365 probe function is replaced by the mtk generic one [3], which
size the clock data array by adding all the clock descriptor array size
provided by the clk-mt8365 driver.
Actually, all clock indexes come from the header file [2], that mean, if
there are more clock (then more index) in the header file [2] than the
number of clock declared in the clock descriptor arrays (which is the
case currently), the clock data array will be undersized and then the
generic probe function will overflow when it will try to write in
"clk_data[CLK_INDEX]". Actually, instead of crashing at boot, the probe
function returns an error in the log which looks like:
"of_clk_hw_onecell_get: invalid index 135", then this clock isn't
enabled.
Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching.
I will.
This is huge ABI break and I don't understand why it is needed. Entire
description above did not explain me that.
Briefly, clocks with the higher index than the data clock array can't be
used. I've this issue:
[ 0.427054] of_clk_hw_onecell_get: invalid index 135
[ 0.429525] of_clk_hw_onecell_get: invalid index 69
[ 0.442998] of_clk_hw_onecell_get: invalid index 70
That means CLK_TOP_SSUSB_PHY_CK_EN, CLK_IFR_SSUSB_REF and
CLK_IFR_SSUSB_XHCI aren't working when I need them. So my USB doesn't work.
The simplest way to fix the regression is to remove from the header file
[2] the unused clocks.
??? The simples is to revert the patch, so you won't break the ABI.
[1]: Commit ffe91cb28f6a ("clk: mediatek: mt8365: Convert to
mtk_clk_simple_{probe,remove}()")
[2]: include/dt-bindings/clock/mediatek,mt8365-clk.h
[3]: drivers/clk/mediatek/clk-mtk.c
Fixes: ffe91cb28f6a ("clk: mediatek: mt8365: Convert to mtk_clk_simple_{probe,remove}()")
Signed-off-by: Alexandre Mergnat <amergnat@xxxxxxxxxxxx>
---
include/dt-bindings/clock/mediatek,mt8365-clk.h | 361 ++++++++++++------------
1 file changed, 177 insertions(+), 184 deletions(-)
diff --git a/include/dt-bindings/clock/mediatek,mt8365-clk.h b/include/dt-bindings/clock/mediatek,mt8365-clk.h
index f9aff1775810..fd59c8bdeb24 100644
--- a/include/dt-bindings/clock/mediatek,mt8365-clk.h
+++ b/include/dt-bindings/clock/mediatek,mt8365-clk.h
@@ -7,147 +7,142 @@
#define _DT_BINDINGS_CLK_MT8365_H
/* TOPCKGEN */
-#define CLK_TOP_CLK_NULL 0> -#define CLK_TOP_I2S0_BCK 1
...
+#define CLK_TOP_I2S0_BCK 0
Why? This is really broken. You can remove the defines, but re-shuffling
everything?!?
I've under-estimated the impact of modifying the defines, I will try to
find another way to fix the regression in the drivers directly.
Thanks for your review.
Best regards,
Krzysztof
--
Regards,
Alexandre
