[PATCH 0/2] Fix and clean MT8365 clock indexes

[PATCH 0/2] Fix and clean MT8365 clock indexes

[Alexandre Mergnat]

The first commit of this serie fix a regression which prevent the
initialization of the latest indexed clocks. The regression is
introduced with [1].

The second one cleans unused define since the probe mecanism change.[1]

[1]: Commit ffe91cb28f6a ("clk: mediatek: mt8365: Convert to
     mtk_clk_simple_{probe,remove}()")

Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>
---
Alexandre Mergnat (2):
      clk: mediatek: mt8365: fix the clock indexes
      clk: mediatek: mt8365: remove the max clock number defines

 include/dt-bindings/clock/mediatek,mt8365-clk.h | 375 +++++++++++-------------
 1 file changed, 175 insertions(+), 200 deletions(-)
---
base-commit: ac9a78681b921877518763ba0e89202254349d1b
change-id: 20230517-fix-clk-index-96043baf65be

Best regards,
-- 
Alexandre Mergnat <amergnat@baylibre.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] clk: mediatek: mt8365: fix the clock indexes

[Alexandre Mergnat]

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.

The simplest way to fix the regression is to remove from the header file
[2] the unused clocks.

[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@baylibre.com>
---
 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_DSI0_LNTC_DSICK		2
-#define CLK_TOP_VPLL_DPIX		3
-#define CLK_TOP_LVDSTX_CLKDIG_CTS	4
-#define CLK_TOP_MFGPLL			5
-#define CLK_TOP_SYSPLL_D2		6
-#define CLK_TOP_SYSPLL1_D2		7
-#define CLK_TOP_SYSPLL1_D4		8
-#define CLK_TOP_SYSPLL1_D8		9
-#define CLK_TOP_SYSPLL1_D16		10
-#define CLK_TOP_SYSPLL_D3		11
-#define CLK_TOP_SYSPLL2_D2		12
-#define CLK_TOP_SYSPLL2_D4		13
-#define CLK_TOP_SYSPLL2_D8		14
-#define CLK_TOP_SYSPLL_D5		15
-#define CLK_TOP_SYSPLL3_D2		16
-#define CLK_TOP_SYSPLL3_D4		17
-#define CLK_TOP_SYSPLL_D7		18
-#define CLK_TOP_SYSPLL4_D2		19
-#define CLK_TOP_SYSPLL4_D4		20
-#define CLK_TOP_UNIVPLL			21
-#define CLK_TOP_UNIVPLL_D2		22
-#define CLK_TOP_UNIVPLL1_D2		23
-#define CLK_TOP_UNIVPLL1_D4		24
-#define CLK_TOP_UNIVPLL_D3		25
-#define CLK_TOP_UNIVPLL2_D2		26
-#define CLK_TOP_UNIVPLL2_D4		27
-#define CLK_TOP_UNIVPLL2_D8		28
-#define CLK_TOP_UNIVPLL2_D32		29
-#define CLK_TOP_UNIVPLL_D5		30
-#define CLK_TOP_UNIVPLL3_D2		31
-#define CLK_TOP_UNIVPLL3_D4		32
-#define CLK_TOP_MMPLL			33
-#define CLK_TOP_MMPLL_D2		34
-#define CLK_TOP_LVDSPLL_D2		35
-#define CLK_TOP_LVDSPLL_D4		36
-#define CLK_TOP_LVDSPLL_D8		37
-#define CLK_TOP_LVDSPLL_D16		38
-#define CLK_TOP_USB20_192M		39
-#define CLK_TOP_USB20_192M_D4		40
-#define CLK_TOP_USB20_192M_D8		41
-#define CLK_TOP_USB20_192M_D16		42
-#define CLK_TOP_USB20_192M_D32		43
-#define CLK_TOP_APLL1			44
-#define CLK_TOP_APLL1_D2		45
-#define CLK_TOP_APLL1_D4		46
-#define CLK_TOP_APLL1_D8		47
-#define CLK_TOP_APLL2			48
-#define CLK_TOP_APLL2_D2		49
-#define CLK_TOP_APLL2_D4		50
-#define CLK_TOP_APLL2_D8		51
-#define CLK_TOP_SYS_26M_D2		52
-#define CLK_TOP_MSDCPLL			53
-#define CLK_TOP_MSDCPLL_D2		54
-#define CLK_TOP_DSPPLL			55
-#define CLK_TOP_DSPPLL_D2		56
-#define CLK_TOP_DSPPLL_D4		57
-#define CLK_TOP_DSPPLL_D8		58
-#define CLK_TOP_APUPLL			59
-#define CLK_TOP_CLK26M_D52		60
-#define CLK_TOP_AXI_SEL			61
-#define CLK_TOP_MEM_SEL			62
-#define CLK_TOP_MM_SEL			63
-#define CLK_TOP_SCP_SEL			64
-#define CLK_TOP_MFG_SEL			65
-#define CLK_TOP_ATB_SEL			66
-#define CLK_TOP_CAMTG_SEL		67
-#define CLK_TOP_CAMTG1_SEL		68
-#define CLK_TOP_UART_SEL		69
-#define CLK_TOP_SPI_SEL			70
-#define CLK_TOP_MSDC50_0_HC_SEL		71
-#define CLK_TOP_MSDC2_2_HC_SEL		72
-#define CLK_TOP_MSDC50_0_SEL		73
-#define CLK_TOP_MSDC50_2_SEL		74
-#define CLK_TOP_MSDC30_1_SEL		75
-#define CLK_TOP_AUDIO_SEL		76
-#define CLK_TOP_AUD_INTBUS_SEL		77
-#define CLK_TOP_AUD_1_SEL		78
-#define CLK_TOP_AUD_2_SEL		79
-#define CLK_TOP_AUD_ENGEN1_SEL		80
-#define CLK_TOP_AUD_ENGEN2_SEL		81
-#define CLK_TOP_AUD_SPDIF_SEL		82
-#define CLK_TOP_DISP_PWM_SEL		83
-#define CLK_TOP_DXCC_SEL		84
-#define CLK_TOP_SSUSB_SYS_SEL		85
-#define CLK_TOP_SSUSB_XHCI_SEL		86
-#define CLK_TOP_SPM_SEL			87
-#define CLK_TOP_I2C_SEL			88
-#define CLK_TOP_PWM_SEL			89
-#define CLK_TOP_SENIF_SEL		90
-#define CLK_TOP_AES_FDE_SEL		91
-#define CLK_TOP_CAMTM_SEL		92
-#define CLK_TOP_DPI0_SEL		93
-#define CLK_TOP_DPI1_SEL		94
-#define CLK_TOP_DSP_SEL			95
-#define CLK_TOP_NFI2X_SEL		96
-#define CLK_TOP_NFIECC_SEL		97
-#define CLK_TOP_ECC_SEL			98
-#define CLK_TOP_ETH_SEL			99
-#define CLK_TOP_GCPU_SEL		100
-#define CLK_TOP_GCPU_CPM_SEL		101
-#define CLK_TOP_APU_SEL			102
-#define CLK_TOP_APU_IF_SEL		103
-#define CLK_TOP_MBIST_DIAG_SEL		104
-#define CLK_TOP_APLL_I2S0_SEL		105
-#define CLK_TOP_APLL_I2S1_SEL		106
-#define CLK_TOP_APLL_I2S2_SEL		107
-#define CLK_TOP_APLL_I2S3_SEL		108
-#define CLK_TOP_APLL_TDMOUT_SEL		109
-#define CLK_TOP_APLL_TDMIN_SEL		110
-#define CLK_TOP_APLL_SPDIF_SEL		111
-#define CLK_TOP_APLL12_CK_DIV0		112
-#define CLK_TOP_APLL12_CK_DIV1		113
-#define CLK_TOP_APLL12_CK_DIV2		114
-#define CLK_TOP_APLL12_CK_DIV3		115
-#define CLK_TOP_APLL12_CK_DIV4		116
-#define CLK_TOP_APLL12_CK_DIV4B		117
-#define CLK_TOP_APLL12_CK_DIV5		118
-#define CLK_TOP_APLL12_CK_DIV5B		119
-#define CLK_TOP_APLL12_CK_DIV6		120
-#define CLK_TOP_AUD_I2S0_M		121
-#define CLK_TOP_AUD_I2S1_M		122
-#define CLK_TOP_AUD_I2S2_M		123
-#define CLK_TOP_AUD_I2S3_M		124
-#define CLK_TOP_AUD_TDMOUT_M		125
-#define CLK_TOP_AUD_TDMOUT_B		126
-#define CLK_TOP_AUD_TDMIN_M		127
-#define CLK_TOP_AUD_TDMIN_B		128
-#define CLK_TOP_AUD_SPDIF_M		129
-#define CLK_TOP_USB20_48M_EN		130
-#define CLK_TOP_UNIVPLL_48M_EN		131
-#define CLK_TOP_LVDSTX_CLKDIG_EN	132
-#define CLK_TOP_VPLL_DPIX_EN		133
-#define CLK_TOP_SSUSB_TOP_CK_EN		134
-#define CLK_TOP_SSUSB_PHY_CK_EN		135
-#define CLK_TOP_CONN_32K		136
-#define CLK_TOP_CONN_26M		137
-#define CLK_TOP_DSP_32K			138
-#define CLK_TOP_DSP_26M			139
-#define CLK_TOP_NR_CLK			140
+#define CLK_TOP_I2S0_BCK		0
+#define CLK_TOP_DSI0_LNTC_DSICK		1
+#define CLK_TOP_VPLL_DPIX		2
+#define CLK_TOP_LVDSTX_CLKDIG_CTS	3
+#define CLK_TOP_MFGPLL			4
+#define CLK_TOP_SYSPLL_D2		5
+#define CLK_TOP_SYSPLL1_D2		6
+#define CLK_TOP_SYSPLL1_D4		7
+#define CLK_TOP_SYSPLL1_D8		8
+#define CLK_TOP_SYSPLL1_D16		9
+#define CLK_TOP_SYSPLL_D3		10
+#define CLK_TOP_SYSPLL2_D2		11
+#define CLK_TOP_SYSPLL2_D4		12
+#define CLK_TOP_SYSPLL2_D8		13
+#define CLK_TOP_SYSPLL_D5		14
+#define CLK_TOP_SYSPLL3_D2		15
+#define CLK_TOP_SYSPLL3_D4		16
+#define CLK_TOP_SYSPLL_D7		17
+#define CLK_TOP_SYSPLL4_D2		18
+#define CLK_TOP_SYSPLL4_D4		19
+#define CLK_TOP_UNIVPLL			20
+#define CLK_TOP_UNIVPLL_D2		21
+#define CLK_TOP_UNIVPLL1_D2		22
+#define CLK_TOP_UNIVPLL1_D4		23
+#define CLK_TOP_UNIVPLL_D3		24
+#define CLK_TOP_UNIVPLL2_D2		25
+#define CLK_TOP_UNIVPLL2_D4		26
+#define CLK_TOP_UNIVPLL2_D8		27
+#define CLK_TOP_UNIVPLL2_D32		28
+#define CLK_TOP_UNIVPLL_D5		29
+#define CLK_TOP_UNIVPLL3_D2		30
+#define CLK_TOP_UNIVPLL3_D4		31
+#define CLK_TOP_MMPLL			32
+#define CLK_TOP_MMPLL_D2		33
+#define CLK_TOP_LVDSPLL_D2		34
+#define CLK_TOP_LVDSPLL_D4		35
+#define CLK_TOP_LVDSPLL_D8		36
+#define CLK_TOP_LVDSPLL_D16		37
+#define CLK_TOP_USB20_192M		38
+#define CLK_TOP_USB20_192M_D4		39
+#define CLK_TOP_USB20_192M_D8		40
+#define CLK_TOP_USB20_192M_D16		41
+#define CLK_TOP_USB20_192M_D32		42
+#define CLK_TOP_APLL1			43
+#define CLK_TOP_APLL1_D2		44
+#define CLK_TOP_APLL1_D4		45
+#define CLK_TOP_APLL1_D8		46
+#define CLK_TOP_APLL2			47
+#define CLK_TOP_APLL2_D2		48
+#define CLK_TOP_APLL2_D4		49
+#define CLK_TOP_APLL2_D8		50
+#define CLK_TOP_SYS_26M_D2		51
+#define CLK_TOP_MSDCPLL			52
+#define CLK_TOP_MSDCPLL_D2		53
+#define CLK_TOP_DSPPLL			54
+#define CLK_TOP_DSPPLL_D2		55
+#define CLK_TOP_DSPPLL_D4		56
+#define CLK_TOP_DSPPLL_D8		57
+#define CLK_TOP_APUPLL			58
+#define CLK_TOP_CLK26M_D52		59
+#define CLK_TOP_AXI_SEL			60
+#define CLK_TOP_MEM_SEL			61
+#define CLK_TOP_MM_SEL			62
+#define CLK_TOP_SCP_SEL			63
+#define CLK_TOP_MFG_SEL			64
+#define CLK_TOP_ATB_SEL			65
+#define CLK_TOP_CAMTG_SEL		66
+#define CLK_TOP_CAMTG1_SEL		67
+#define CLK_TOP_UART_SEL		68
+#define CLK_TOP_SPI_SEL			69
+#define CLK_TOP_MSDC50_0_HC_SEL		70
+#define CLK_TOP_MSDC2_2_HC_SEL		71
+#define CLK_TOP_MSDC50_0_SEL		72
+#define CLK_TOP_MSDC50_2_SEL		73
+#define CLK_TOP_MSDC30_1_SEL		74
+#define CLK_TOP_AUDIO_SEL		75
+#define CLK_TOP_AUD_INTBUS_SEL		76
+#define CLK_TOP_AUD_1_SEL		77
+#define CLK_TOP_AUD_2_SEL		78
+#define CLK_TOP_AUD_ENGEN1_SEL		79
+#define CLK_TOP_AUD_ENGEN2_SEL		80
+#define CLK_TOP_AUD_SPDIF_SEL		81
+#define CLK_TOP_DISP_PWM_SEL		82
+#define CLK_TOP_DXCC_SEL		83
+#define CLK_TOP_SSUSB_SYS_SEL		84
+#define CLK_TOP_SSUSB_XHCI_SEL		85
+#define CLK_TOP_SPM_SEL			86
+#define CLK_TOP_I2C_SEL			87
+#define CLK_TOP_PWM_SEL			88
+#define CLK_TOP_SENIF_SEL		89
+#define CLK_TOP_AES_FDE_SEL		90
+#define CLK_TOP_CAMTM_SEL		91
+#define CLK_TOP_DPI0_SEL		92
+#define CLK_TOP_DPI1_SEL		93
+#define CLK_TOP_DSP_SEL			94
+#define CLK_TOP_NFI2X_SEL		95
+#define CLK_TOP_NFIECC_SEL		96
+#define CLK_TOP_ECC_SEL			97
+#define CLK_TOP_ETH_SEL			98
+#define CLK_TOP_GCPU_SEL		99
+#define CLK_TOP_GCPU_CPM_SEL		100
+#define CLK_TOP_APU_SEL			101
+#define CLK_TOP_APU_IF_SEL		102
+#define CLK_TOP_MBIST_DIAG_SEL		103
+#define CLK_TOP_APLL_I2S0_SEL		104
+#define CLK_TOP_APLL_I2S1_SEL		105
+#define CLK_TOP_APLL_I2S2_SEL		106
+#define CLK_TOP_APLL_I2S3_SEL		107
+#define CLK_TOP_APLL_TDMOUT_SEL		108
+#define CLK_TOP_APLL_TDMIN_SEL		109
+#define CLK_TOP_APLL_SPDIF_SEL		110
+#define CLK_TOP_APLL12_CK_DIV0		111
+#define CLK_TOP_APLL12_CK_DIV1		112
+#define CLK_TOP_APLL12_CK_DIV2		113
+#define CLK_TOP_APLL12_CK_DIV3		114
+#define CLK_TOP_APLL12_CK_DIV6		115
+#define CLK_TOP_AUD_I2S0_M		116
+#define CLK_TOP_AUD_I2S1_M		117
+#define CLK_TOP_AUD_I2S2_M		118
+#define CLK_TOP_AUD_I2S3_M		119
+#define CLK_TOP_AUD_TDMOUT_M		120
+#define CLK_TOP_AUD_TDMOUT_B		121
+#define CLK_TOP_AUD_TDMIN_M		122
+#define CLK_TOP_AUD_TDMIN_B		123
+#define CLK_TOP_AUD_SPDIF_M		124
+#define CLK_TOP_USB20_48M_EN		125
+#define CLK_TOP_UNIVPLL_48M_EN		126
+#define CLK_TOP_LVDSTX_CLKDIG_EN	127
+#define CLK_TOP_VPLL_DPIX_EN		128
+#define CLK_TOP_SSUSB_TOP_CK_EN		129
+#define CLK_TOP_SSUSB_PHY_CK_EN		130
+#define CLK_TOP_CONN_32K		131
+#define CLK_TOP_CONN_26M		132
+#define CLK_TOP_DSP_32K			133
+#define CLK_TOP_DSP_26M			134
+#define CLK_TOP_NR_CLK			135
 
 /* INFRACFG */
 #define CLK_IFR_PMIC_TMR		0
@@ -179,49 +174,47 @@
 #define CLK_IFR_GCPU			26
 #define CLK_IFR_TRNG			27
 #define CLK_IFR_AUXADC			28
-#define CLK_IFR_CPUM			29
-#define CLK_IFR_AUXADC_MD		30
-#define CLK_IFR_AP_DMA			31
-#define CLK_IFR_DEBUGSYS		32
-#define CLK_IFR_AUDIO			33
-#define CLK_IFR_PWM_FBCLK6		34
-#define CLK_IFR_DISP_PWM		35
-#define CLK_IFR_AUD_26M_BK		36
-#define CLK_IFR_CQ_DMA			37
-#define CLK_IFR_MSDC0_SF		38
-#define CLK_IFR_MSDC1_SF		39
-#define CLK_IFR_MSDC2_SF		40
-#define CLK_IFR_AP_MSDC0		41
-#define CLK_IFR_MD_MSDC0		42
-#define CLK_IFR_MSDC0_SRC		43
-#define CLK_IFR_MSDC1_SRC		44
-#define CLK_IFR_MSDC2_SRC		45
-#define CLK_IFR_PWRAP_TMR		46
-#define CLK_IFR_PWRAP_SPI		47
-#define CLK_IFR_PWRAP_SYS		48
-#define CLK_IFR_MCU_PM_BK		49
-#define CLK_IFR_IRRX_26M		50
-#define CLK_IFR_IRRX_32K		51
-#define CLK_IFR_I2C0_AXI		52
-#define CLK_IFR_I2C1_AXI		53
-#define CLK_IFR_I2C2_AXI		54
-#define CLK_IFR_I2C3_AXI		55
-#define CLK_IFR_NIC_AXI			56
-#define CLK_IFR_NIC_SLV_AXI		57
-#define CLK_IFR_APU_AXI			58
-#define CLK_IFR_NFIECC			59
-#define CLK_IFR_NFIECC_BK		60
-#define CLK_IFR_NFI1X_BK		61
-#define CLK_IFR_NFI_BK			62
-#define CLK_IFR_MSDC2_AP_BK		63
-#define CLK_IFR_MSDC2_MD_BK		64
-#define CLK_IFR_MSDC2_BK		65
-#define CLK_IFR_SUSB_133_BK		66
-#define CLK_IFR_SUSB_66_BK		67
-#define CLK_IFR_SSUSB_SYS		68
-#define CLK_IFR_SSUSB_REF		69
-#define CLK_IFR_SSUSB_XHCI		70
-#define CLK_IFR_NR_CLK			71
+#define CLK_IFR_AUXADC_MD		29
+#define CLK_IFR_AP_DMA			30
+#define CLK_IFR_DEBUGSYS		31
+#define CLK_IFR_AUDIO			32
+#define CLK_IFR_PWM_FBCLK6		33
+#define CLK_IFR_DISP_PWM		34
+#define CLK_IFR_AUD_26M_BK		35
+#define CLK_IFR_CQ_DMA			36
+#define CLK_IFR_MSDC0_SF		37
+#define CLK_IFR_MSDC1_SF		38
+#define CLK_IFR_MSDC2_SF		39
+#define CLK_IFR_AP_MSDC0		40
+#define CLK_IFR_MD_MSDC0		41
+#define CLK_IFR_MSDC0_SRC		42
+#define CLK_IFR_MSDC1_SRC		43
+#define CLK_IFR_MSDC2_SRC		44
+#define CLK_IFR_PWRAP_TMR		45
+#define CLK_IFR_PWRAP_SPI		46
+#define CLK_IFR_PWRAP_SYS		47
+#define CLK_IFR_IRRX_26M		48
+#define CLK_IFR_IRRX_32K		49
+#define CLK_IFR_I2C0_AXI		50
+#define CLK_IFR_I2C1_AXI		51
+#define CLK_IFR_I2C2_AXI		52
+#define CLK_IFR_I2C3_AXI		53
+#define CLK_IFR_NIC_AXI			54
+#define CLK_IFR_NIC_SLV_AXI		55
+#define CLK_IFR_APU_AXI			56
+#define CLK_IFR_NFIECC			57
+#define CLK_IFR_NFIECC_BK		58
+#define CLK_IFR_NFI1X_BK		59
+#define CLK_IFR_NFI_BK			60
+#define CLK_IFR_MSDC2_AP_BK		61
+#define CLK_IFR_MSDC2_MD_BK		62
+#define CLK_IFR_MSDC2_BK		63
+#define CLK_IFR_SUSB_133_BK		64
+#define CLK_IFR_SUSB_66_BK		65
+#define CLK_IFR_SSUSB_SYS		66
+#define CLK_IFR_SSUSB_REF		67
+#define CLK_IFR_SSUSB_XHCI		68
+#define CLK_IFR_NR_CLK			69
 
 /* PERICFG */
 #define CLK_PERIAXI			0

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] clk: mediatek: mt8365: fix the clock indexes

[Krzysztof Kozlowski]

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.

This is huge ABI break and I don't understand why it is needed. Entire
description above did not explain me that.

> 
> 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@baylibre.com>
> ---
>  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?!?

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] clk: mediatek: mt8365: fix the clock indexes

[Alexandre Mergnat]

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@baylibre.com>
>> ---
>>   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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] clk: mediatek: mt8365: fix the clock indexes

[Krzysztof Kozlowski]

On 17/05/2023 11:34, Alexandre Mergnat wrote:
> 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.

That sounds like a driver, not bindings problem.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] clk: mediatek: mt8365: remove the max clock number defines

[Alexandre Mergnat]

Since we swap to the generic MTK probe function [1], the *_NR_CLK
defines used to know the number of defined clocks are no longer
necessary. This patch remove all *_NR_CLK.

[1]: Commit ffe91cb28f6a ("clk: mediatek: mt8365: Convert to
     mtk_clk_simple_{probe,remove}()")

Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>
---
 include/dt-bindings/clock/mediatek,mt8365-clk.h | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/include/dt-bindings/clock/mediatek,mt8365-clk.h b/include/dt-bindings/clock/mediatek,mt8365-clk.h
index fd59c8bdeb24..a1df91f73d73 100644
--- a/include/dt-bindings/clock/mediatek,mt8365-clk.h
+++ b/include/dt-bindings/clock/mediatek,mt8365-clk.h
@@ -142,7 +142,6 @@
 #define CLK_TOP_CONN_26M		132
 #define CLK_TOP_DSP_32K			133
 #define CLK_TOP_DSP_26M			134
-#define CLK_TOP_NR_CLK			135
 
 /* INFRACFG */
 #define CLK_IFR_PMIC_TMR		0
@@ -214,11 +213,9 @@
 #define CLK_IFR_SSUSB_SYS		66
 #define CLK_IFR_SSUSB_REF		67
 #define CLK_IFR_SSUSB_XHCI		68
-#define CLK_IFR_NR_CLK			69
 
 /* PERICFG */
 #define CLK_PERIAXI			0
-#define CLK_PERI_NR_CLK			1
 
 /* APMIXEDSYS */
 #define CLK_APMIXED_ARMPLL		0
@@ -238,7 +235,6 @@
 
 /* GCE */
 #define CLK_GCE_FAXI			0
-#define CLK_GCE_NR_CLK			1
 
 /* AUDIOTOP */
 #define CLK_AUD_AFE			0
@@ -259,40 +255,31 @@
 #define CLK_AUD_I2S2_BK			15
 #define CLK_AUD_I2S3_BK			16
 #define CLK_AUD_I2S4_BK			17
-#define CLK_AUD_NR_CLK			18
 
 /* MIPI_CSI0A */
 #define CLK_MIPI0A_CSR_CSI_EN_0A	0
-#define CLK_MIPI_RX_ANA_CSI0A_NR_CLK	1
 
 /* MIPI_CSI0B */
 #define CLK_MIPI0B_CSR_CSI_EN_0B	0
-#define CLK_MIPI_RX_ANA_CSI0B_NR_CLK	1
 
 /* MIPI_CSI1A */
 #define CLK_MIPI1A_CSR_CSI_EN_1A	0
-#define CLK_MIPI_RX_ANA_CSI1A_NR_CLK	1
 
 /* MIPI_CSI1B */
 #define CLK_MIPI1B_CSR_CSI_EN_1B	0
-#define CLK_MIPI_RX_ANA_CSI1B_NR_CLK	1
 
 /* MIPI_CSI2A */
 #define CLK_MIPI2A_CSR_CSI_EN_2A	0
-#define CLK_MIPI_RX_ANA_CSI2A_NR_CLK	1
 
 /* MIPI_CSI2B */
 #define CLK_MIPI2B_CSR_CSI_EN_2B	0
-#define CLK_MIPI_RX_ANA_CSI2B_NR_CLK	1
 
 /* MCUCFG */
 #define CLK_MCU_BUS_SEL			0
-#define CLK_MCU_NR_CLK			1
 
 /* MFGCFG */
 #define CLK_MFG_BG3D			0
 #define CLK_MFG_MBIST_DIAG		1
-#define CLK_MFG_NR_CLK			2
 
 /* MMSYS */
 #define CLK_MM_MM_MDP_RDMA0		0
@@ -331,7 +318,6 @@
 #define CLK_MM_MM_DPI0			33
 #define CLK_MM_LVDSTX_PXL		34
 #define CLK_MM_LVDSTX_CTS		35
-#define CLK_MM_NR_CLK			36
 
 /* IMGSYS */
 #define CLK_CAM_LARB2			0
@@ -342,17 +328,14 @@
 #define CLK_CAMSV1			5
 #define CLK_CAM_FDVT			6
 #define CLK_CAM_WPE			7
-#define CLK_CAM_NR_CLK			8
 
 /* VDECSYS */
 #define CLK_VDEC_VDEC			0
 #define CLK_VDEC_LARB1			1
-#define CLK_VDEC_NR_CLK			2
 
 /* VENCSYS */
 #define CLK_VENC			0
 #define CLK_VENC_JPGENC			1
-#define CLK_VENC_NR_CLK			2
 
 /* APUSYS */
 #define CLK_APU_IPU_CK			0
@@ -361,6 +344,5 @@
 #define CLK_APU_IF_CK			3
 #define CLK_APU_EDMA			4
 #define CLK_APU_AHB			5
-#define CLK_APU_NR_CLK			6
 
 #endif /* _DT_BINDINGS_CLK_MT8365_H */

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] clk: mediatek: mt8365: remove the max clock number defines

[Krzysztof Kozlowski]

On 17/05/2023 10:28, Alexandre Mergnat wrote:
> Since we swap to the generic MTK probe function [1], the *_NR_CLK
> defines used to know the number of defined clocks are no longer
> necessary. This patch remove all *_NR_CLK.

Please do not use "This commit/patch", but imperative mood. See longer
explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> 
> [1]: Commit ffe91cb28f6a ("clk: mediatek: mt8365: Convert to
>      mtk_clk_simple_{probe,remove}()")
> 
> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>


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.

And what about other users of bindings?

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel