* [PATCH] clk: mediatek: mt8186: Change I2C 4/5/6 ap clocks parent to infra
@ 2023-10-19 12:49 AngeloGioacchino Del Regno
2023-10-20 5:06 ` Chen-Yu Tsai
0 siblings, 1 reply; 11+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-10-19 12:49 UTC (permalink / raw)
To: sboyd
Cc: mturquette, matthias.bgg, angelogioacchino.delregno, wenst,
u.kleine-koenig, chun-jie.chen, miles.chen, linux-clk,
linux-kernel, linux-arm-kernel, linux-mediatek
Fix the parenting of clocks imp_iic_wrap_ap_clock_i2c{4-6}, as those
are effectively parented to infra_ao_i2c{4-6} and not to the I2C_AP.
This permits the correct (and full) enablement and disablement of the
I2C4, I2C5 and I2C6 bus clocks, satisfying the whole clock tree of
those.
As an example, when requesting to enable imp_iic_wrap_ap_clock_i2c4:
Before: infra_ao_i2c_ap -> imp_iic_wrap_ap_clock_i2c4
After: infra_ao_i2c_ap -> infra_ao_i2c4 -> imp_iic_wrap_ap_clock_i2c4
Fixes: 66cd0b4b0ce5 ("clk: mediatek: Add MT8186 imp i2c wrapper clock support")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/clk/mediatek/clk-mt8186-imp_iic_wrap.c | 6 +++---
drivers/clk/mediatek/clk-mt8186-infra_ao.c | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/clk/mediatek/clk-mt8186-imp_iic_wrap.c b/drivers/clk/mediatek/clk-mt8186-imp_iic_wrap.c
index 640ccb553274..871b8ff4c287 100644
--- a/drivers/clk/mediatek/clk-mt8186-imp_iic_wrap.c
+++ b/drivers/clk/mediatek/clk-mt8186-imp_iic_wrap.c
@@ -29,11 +29,11 @@ static const struct mtk_gate imp_iic_wrap_clks[] = {
GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_AP_CLOCK_I2C3,
"imp_iic_wrap_ap_clock_i2c3", "infra_ao_i2c_ap", 3),
GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_AP_CLOCK_I2C4,
- "imp_iic_wrap_ap_clock_i2c4", "infra_ao_i2c_ap", 4),
+ "imp_iic_wrap_ap_clock_i2c4", "infra_ao_i2c4", 4),
GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_AP_CLOCK_I2C5,
- "imp_iic_wrap_ap_clock_i2c5", "infra_ao_i2c_ap", 5),
+ "imp_iic_wrap_ap_clock_i2c5", "infra_ao_i2c5", 5),
GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_AP_CLOCK_I2C6,
- "imp_iic_wrap_ap_clock_i2c6", "infra_ao_i2c_ap", 6),
+ "imp_iic_wrap_ap_clock_i2c6", "infra_ao_i2c6", 6),
GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_AP_CLOCK_I2C7,
"imp_iic_wrap_ap_clock_i2c7", "infra_ao_i2c_ap", 7),
GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_AP_CLOCK_I2C8,
diff --git a/drivers/clk/mediatek/clk-mt8186-infra_ao.c b/drivers/clk/mediatek/clk-mt8186-infra_ao.c
index 837304cd0ed7..c490f1a310f8 100644
--- a/drivers/clk/mediatek/clk-mt8186-infra_ao.c
+++ b/drivers/clk/mediatek/clk-mt8186-infra_ao.c
@@ -132,7 +132,7 @@ static const struct mtk_gate infra_ao_clks[] = {
GATE_INFRA_AO2(CLK_INFRA_AO_AUDIO_26M_BCLK, "infra_ao_audio26m", "clk26m", 4),
GATE_INFRA_AO2(CLK_INFRA_AO_SSUSB_TOP_P1_HCLK, "infra_ao_ssusb_p1_hclk", "top_axi", 5),
GATE_INFRA_AO2(CLK_INFRA_AO_SPI1, "infra_ao_spi1", "top_spi", 6),
- GATE_INFRA_AO2(CLK_INFRA_AO_I2C4, "infra_ao_i2c4", "top_i2c", 7),
+ GATE_INFRA_AO2(CLK_INFRA_AO_I2C4, "infra_ao_i2c4", "infra_ao_i2c_ap", 7),
GATE_INFRA_AO2(CLK_INFRA_AO_MODEM_TEMP_SHARE, "infra_ao_mdtemp", "clk26m", 8),
GATE_INFRA_AO2(CLK_INFRA_AO_SPI2, "infra_ao_spi2", "top_spi", 9),
GATE_INFRA_AO2(CLK_INFRA_AO_SPI3, "infra_ao_spi3", "top_spi", 10),
@@ -145,7 +145,7 @@ static const struct mtk_gate infra_ao_clks[] = {
GATE_INFRA_AO2_FLAGS(CLK_INFRA_AO_SSPM, "infra_ao_sspm", "top_sspm", 15, CLK_IS_CRITICAL),
GATE_INFRA_AO2(CLK_INFRA_AO_SSUSB_TOP_P1_SYS,
"infra_ao_ssusb_p1_sys", "top_ssusb_1p", 16),
- GATE_INFRA_AO2(CLK_INFRA_AO_I2C5, "infra_ao_i2c5", "top_i2c", 18),
+ GATE_INFRA_AO2(CLK_INFRA_AO_I2C5, "infra_ao_i2c5", "infra_ao_i2c_ap", 18),
GATE_INFRA_AO2(CLK_INFRA_AO_I2C5_ARBITER, "infra_ao_i2c5a", "top_i2c", 19),
GATE_INFRA_AO2(CLK_INFRA_AO_I2C5_IMM, "infra_ao_i2c5_imm", "top_i2c", 20),
GATE_INFRA_AO2(CLK_INFRA_AO_I2C1_ARBITER, "infra_ao_i2c1a", "top_i2c", 21),
@@ -167,7 +167,7 @@ static const struct mtk_gate infra_ao_clks[] = {
CLK_IS_CRITICAL),
GATE_INFRA_AO3_FLAGS(CLK_INFRA_AO_SSPM_32K_SELF, "infra_ao_sspm_32k", "clk32k", 4,
CLK_IS_CRITICAL),
- GATE_INFRA_AO3(CLK_INFRA_AO_I2C6, "infra_ao_i2c6", "top_i2c", 6),
+ GATE_INFRA_AO3(CLK_INFRA_AO_I2C6, "infra_ao_i2c6", "infra_ao_i2c_ap", 6),
GATE_INFRA_AO3(CLK_INFRA_AO_AP_MSDC0, "infra_ao_ap_msdc0", "top_axi", 7),
GATE_INFRA_AO3(CLK_INFRA_AO_MD_MSDC0, "infra_ao_md_msdc0", "top_axi", 8),
GATE_INFRA_AO3(CLK_INFRA_AO_MSDC0_SRC, "infra_ao_msdc0_clk", "top_msdc50_0", 9),
--
2.42.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] clk: mediatek: mt8186: Change I2C 4/5/6 ap clocks parent to infra
2023-10-19 12:49 [PATCH] clk: mediatek: mt8186: Change I2C 4/5/6 ap clocks parent to infra AngeloGioacchino Del Regno
@ 2023-10-20 5:06 ` Chen-Yu Tsai
2023-10-20 7:52 ` AngeloGioacchino Del Regno
2023-10-24 2:52 ` Stephen Boyd
0 siblings, 2 replies; 11+ messages in thread
From: Chen-Yu Tsai @ 2023-10-20 5:06 UTC (permalink / raw)
To: AngeloGioacchino Del Regno
Cc: sboyd, mturquette, matthias.bgg, u.kleine-koenig, chun-jie.chen,
miles.chen, linux-clk, linux-kernel, linux-arm-kernel,
linux-mediatek
On Thu, Oct 19, 2023 at 8:49 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Fix the parenting of clocks imp_iic_wrap_ap_clock_i2c{4-6}, as those
> are effectively parented to infra_ao_i2c{4-6} and not to the I2C_AP.
> This permits the correct (and full) enablement and disablement of the
> I2C4, I2C5 and I2C6 bus clocks, satisfying the whole clock tree of
> those.
>
> As an example, when requesting to enable imp_iic_wrap_ap_clock_i2c4:
>
> Before: infra_ao_i2c_ap -> imp_iic_wrap_ap_clock_i2c4
> After: infra_ao_i2c_ap -> infra_ao_i2c4 -> imp_iic_wrap_ap_clock_i2c4
>
> Fixes: 66cd0b4b0ce5 ("clk: mediatek: Add MT8186 imp i2c wrapper clock support")
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
I'm curious about what led to discovering this error?
ChenYu
> ---
> drivers/clk/mediatek/clk-mt8186-imp_iic_wrap.c | 6 +++---
> drivers/clk/mediatek/clk-mt8186-infra_ao.c | 6 +++---
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clk/mediatek/clk-mt8186-imp_iic_wrap.c b/drivers/clk/mediatek/clk-mt8186-imp_iic_wrap.c
> index 640ccb553274..871b8ff4c287 100644
> --- a/drivers/clk/mediatek/clk-mt8186-imp_iic_wrap.c
> +++ b/drivers/clk/mediatek/clk-mt8186-imp_iic_wrap.c
> @@ -29,11 +29,11 @@ static const struct mtk_gate imp_iic_wrap_clks[] = {
> GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_AP_CLOCK_I2C3,
> "imp_iic_wrap_ap_clock_i2c3", "infra_ao_i2c_ap", 3),
> GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_AP_CLOCK_I2C4,
> - "imp_iic_wrap_ap_clock_i2c4", "infra_ao_i2c_ap", 4),
> + "imp_iic_wrap_ap_clock_i2c4", "infra_ao_i2c4", 4),
> GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_AP_CLOCK_I2C5,
> - "imp_iic_wrap_ap_clock_i2c5", "infra_ao_i2c_ap", 5),
> + "imp_iic_wrap_ap_clock_i2c5", "infra_ao_i2c5", 5),
> GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_AP_CLOCK_I2C6,
> - "imp_iic_wrap_ap_clock_i2c6", "infra_ao_i2c_ap", 6),
> + "imp_iic_wrap_ap_clock_i2c6", "infra_ao_i2c6", 6),
> GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_AP_CLOCK_I2C7,
> "imp_iic_wrap_ap_clock_i2c7", "infra_ao_i2c_ap", 7),
> GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_AP_CLOCK_I2C8,
> diff --git a/drivers/clk/mediatek/clk-mt8186-infra_ao.c b/drivers/clk/mediatek/clk-mt8186-infra_ao.c
> index 837304cd0ed7..c490f1a310f8 100644
> --- a/drivers/clk/mediatek/clk-mt8186-infra_ao.c
> +++ b/drivers/clk/mediatek/clk-mt8186-infra_ao.c
> @@ -132,7 +132,7 @@ static const struct mtk_gate infra_ao_clks[] = {
> GATE_INFRA_AO2(CLK_INFRA_AO_AUDIO_26M_BCLK, "infra_ao_audio26m", "clk26m", 4),
> GATE_INFRA_AO2(CLK_INFRA_AO_SSUSB_TOP_P1_HCLK, "infra_ao_ssusb_p1_hclk", "top_axi", 5),
> GATE_INFRA_AO2(CLK_INFRA_AO_SPI1, "infra_ao_spi1", "top_spi", 6),
> - GATE_INFRA_AO2(CLK_INFRA_AO_I2C4, "infra_ao_i2c4", "top_i2c", 7),
> + GATE_INFRA_AO2(CLK_INFRA_AO_I2C4, "infra_ao_i2c4", "infra_ao_i2c_ap", 7),
> GATE_INFRA_AO2(CLK_INFRA_AO_MODEM_TEMP_SHARE, "infra_ao_mdtemp", "clk26m", 8),
> GATE_INFRA_AO2(CLK_INFRA_AO_SPI2, "infra_ao_spi2", "top_spi", 9),
> GATE_INFRA_AO2(CLK_INFRA_AO_SPI3, "infra_ao_spi3", "top_spi", 10),
> @@ -145,7 +145,7 @@ static const struct mtk_gate infra_ao_clks[] = {
> GATE_INFRA_AO2_FLAGS(CLK_INFRA_AO_SSPM, "infra_ao_sspm", "top_sspm", 15, CLK_IS_CRITICAL),
> GATE_INFRA_AO2(CLK_INFRA_AO_SSUSB_TOP_P1_SYS,
> "infra_ao_ssusb_p1_sys", "top_ssusb_1p", 16),
> - GATE_INFRA_AO2(CLK_INFRA_AO_I2C5, "infra_ao_i2c5", "top_i2c", 18),
> + GATE_INFRA_AO2(CLK_INFRA_AO_I2C5, "infra_ao_i2c5", "infra_ao_i2c_ap", 18),
> GATE_INFRA_AO2(CLK_INFRA_AO_I2C5_ARBITER, "infra_ao_i2c5a", "top_i2c", 19),
> GATE_INFRA_AO2(CLK_INFRA_AO_I2C5_IMM, "infra_ao_i2c5_imm", "top_i2c", 20),
> GATE_INFRA_AO2(CLK_INFRA_AO_I2C1_ARBITER, "infra_ao_i2c1a", "top_i2c", 21),
> @@ -167,7 +167,7 @@ static const struct mtk_gate infra_ao_clks[] = {
> CLK_IS_CRITICAL),
> GATE_INFRA_AO3_FLAGS(CLK_INFRA_AO_SSPM_32K_SELF, "infra_ao_sspm_32k", "clk32k", 4,
> CLK_IS_CRITICAL),
> - GATE_INFRA_AO3(CLK_INFRA_AO_I2C6, "infra_ao_i2c6", "top_i2c", 6),
> + GATE_INFRA_AO3(CLK_INFRA_AO_I2C6, "infra_ao_i2c6", "infra_ao_i2c_ap", 6),
> GATE_INFRA_AO3(CLK_INFRA_AO_AP_MSDC0, "infra_ao_ap_msdc0", "top_axi", 7),
> GATE_INFRA_AO3(CLK_INFRA_AO_MD_MSDC0, "infra_ao_md_msdc0", "top_axi", 8),
> GATE_INFRA_AO3(CLK_INFRA_AO_MSDC0_SRC, "infra_ao_msdc0_clk", "top_msdc50_0", 9),
> --
> 2.42.0
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] clk: mediatek: mt8186: Change I2C 4/5/6 ap clocks parent to infra
2023-10-20 5:06 ` Chen-Yu Tsai
@ 2023-10-20 7:52 ` AngeloGioacchino Del Regno
2023-10-24 2:52 ` Stephen Boyd
1 sibling, 0 replies; 11+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-10-20 7:52 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: sboyd, mturquette, matthias.bgg, u.kleine-koenig, chun-jie.chen,
miles.chen, linux-clk, linux-kernel, linux-arm-kernel,
linux-mediatek
Il 20/10/23 07:06, Chen-Yu Tsai ha scritto:
> On Thu, Oct 19, 2023 at 8:49 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Fix the parenting of clocks imp_iic_wrap_ap_clock_i2c{4-6}, as those
>> are effectively parented to infra_ao_i2c{4-6} and not to the I2C_AP.
>> This permits the correct (and full) enablement and disablement of the
>> I2C4, I2C5 and I2C6 bus clocks, satisfying the whole clock tree of
>> those.
>>
>> As an example, when requesting to enable imp_iic_wrap_ap_clock_i2c4:
>>
>> Before: infra_ao_i2c_ap -> imp_iic_wrap_ap_clock_i2c4
>> After: infra_ao_i2c_ap -> infra_ao_i2c4 -> imp_iic_wrap_ap_clock_i2c4
>>
>> Fixes: 66cd0b4b0ce5 ("clk: mediatek: Add MT8186 imp i2c wrapper clock support")
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>
> I'm curious about what led to discovering this error?
>
I had lockups during boot when probing I2C, so some research led me to
discover that the clock tree wasn't fully satisfied... :-)
Cheers,
Angelo
> ChenYu
>
>> ---
>> drivers/clk/mediatek/clk-mt8186-imp_iic_wrap.c | 6 +++---
>> drivers/clk/mediatek/clk-mt8186-infra_ao.c | 6 +++---
>> 2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/clk/mediatek/clk-mt8186-imp_iic_wrap.c b/drivers/clk/mediatek/clk-mt8186-imp_iic_wrap.c
>> index 640ccb553274..871b8ff4c287 100644
>> --- a/drivers/clk/mediatek/clk-mt8186-imp_iic_wrap.c
>> +++ b/drivers/clk/mediatek/clk-mt8186-imp_iic_wrap.c
>> @@ -29,11 +29,11 @@ static const struct mtk_gate imp_iic_wrap_clks[] = {
>> GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_AP_CLOCK_I2C3,
>> "imp_iic_wrap_ap_clock_i2c3", "infra_ao_i2c_ap", 3),
>> GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_AP_CLOCK_I2C4,
>> - "imp_iic_wrap_ap_clock_i2c4", "infra_ao_i2c_ap", 4),
>> + "imp_iic_wrap_ap_clock_i2c4", "infra_ao_i2c4", 4),
>> GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_AP_CLOCK_I2C5,
>> - "imp_iic_wrap_ap_clock_i2c5", "infra_ao_i2c_ap", 5),
>> + "imp_iic_wrap_ap_clock_i2c5", "infra_ao_i2c5", 5),
>> GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_AP_CLOCK_I2C6,
>> - "imp_iic_wrap_ap_clock_i2c6", "infra_ao_i2c_ap", 6),
>> + "imp_iic_wrap_ap_clock_i2c6", "infra_ao_i2c6", 6),
>> GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_AP_CLOCK_I2C7,
>> "imp_iic_wrap_ap_clock_i2c7", "infra_ao_i2c_ap", 7),
>> GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_AP_CLOCK_I2C8,
>> diff --git a/drivers/clk/mediatek/clk-mt8186-infra_ao.c b/drivers/clk/mediatek/clk-mt8186-infra_ao.c
>> index 837304cd0ed7..c490f1a310f8 100644
>> --- a/drivers/clk/mediatek/clk-mt8186-infra_ao.c
>> +++ b/drivers/clk/mediatek/clk-mt8186-infra_ao.c
>> @@ -132,7 +132,7 @@ static const struct mtk_gate infra_ao_clks[] = {
>> GATE_INFRA_AO2(CLK_INFRA_AO_AUDIO_26M_BCLK, "infra_ao_audio26m", "clk26m", 4),
>> GATE_INFRA_AO2(CLK_INFRA_AO_SSUSB_TOP_P1_HCLK, "infra_ao_ssusb_p1_hclk", "top_axi", 5),
>> GATE_INFRA_AO2(CLK_INFRA_AO_SPI1, "infra_ao_spi1", "top_spi", 6),
>> - GATE_INFRA_AO2(CLK_INFRA_AO_I2C4, "infra_ao_i2c4", "top_i2c", 7),
>> + GATE_INFRA_AO2(CLK_INFRA_AO_I2C4, "infra_ao_i2c4", "infra_ao_i2c_ap", 7),
>> GATE_INFRA_AO2(CLK_INFRA_AO_MODEM_TEMP_SHARE, "infra_ao_mdtemp", "clk26m", 8),
>> GATE_INFRA_AO2(CLK_INFRA_AO_SPI2, "infra_ao_spi2", "top_spi", 9),
>> GATE_INFRA_AO2(CLK_INFRA_AO_SPI3, "infra_ao_spi3", "top_spi", 10),
>> @@ -145,7 +145,7 @@ static const struct mtk_gate infra_ao_clks[] = {
>> GATE_INFRA_AO2_FLAGS(CLK_INFRA_AO_SSPM, "infra_ao_sspm", "top_sspm", 15, CLK_IS_CRITICAL),
>> GATE_INFRA_AO2(CLK_INFRA_AO_SSUSB_TOP_P1_SYS,
>> "infra_ao_ssusb_p1_sys", "top_ssusb_1p", 16),
>> - GATE_INFRA_AO2(CLK_INFRA_AO_I2C5, "infra_ao_i2c5", "top_i2c", 18),
>> + GATE_INFRA_AO2(CLK_INFRA_AO_I2C5, "infra_ao_i2c5", "infra_ao_i2c_ap", 18),
>> GATE_INFRA_AO2(CLK_INFRA_AO_I2C5_ARBITER, "infra_ao_i2c5a", "top_i2c", 19),
>> GATE_INFRA_AO2(CLK_INFRA_AO_I2C5_IMM, "infra_ao_i2c5_imm", "top_i2c", 20),
>> GATE_INFRA_AO2(CLK_INFRA_AO_I2C1_ARBITER, "infra_ao_i2c1a", "top_i2c", 21),
>> @@ -167,7 +167,7 @@ static const struct mtk_gate infra_ao_clks[] = {
>> CLK_IS_CRITICAL),
>> GATE_INFRA_AO3_FLAGS(CLK_INFRA_AO_SSPM_32K_SELF, "infra_ao_sspm_32k", "clk32k", 4,
>> CLK_IS_CRITICAL),
>> - GATE_INFRA_AO3(CLK_INFRA_AO_I2C6, "infra_ao_i2c6", "top_i2c", 6),
>> + GATE_INFRA_AO3(CLK_INFRA_AO_I2C6, "infra_ao_i2c6", "infra_ao_i2c_ap", 6),
>> GATE_INFRA_AO3(CLK_INFRA_AO_AP_MSDC0, "infra_ao_ap_msdc0", "top_axi", 7),
>> GATE_INFRA_AO3(CLK_INFRA_AO_MD_MSDC0, "infra_ao_md_msdc0", "top_axi", 8),
>> GATE_INFRA_AO3(CLK_INFRA_AO_MSDC0_SRC, "infra_ao_msdc0_clk", "top_msdc50_0", 9),
>> --
>> 2.42.0
>>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] clk: mediatek: mt8186: Change I2C 4/5/6 ap clocks parent to infra
2023-10-20 5:06 ` Chen-Yu Tsai
2023-10-20 7:52 ` AngeloGioacchino Del Regno
@ 2023-10-24 2:52 ` Stephen Boyd
2023-10-24 2:58 ` Chen-Yu Tsai
1 sibling, 1 reply; 11+ messages in thread
From: Stephen Boyd @ 2023-10-24 2:52 UTC (permalink / raw)
To: AngeloGioacchino Del Regno, Chen-Yu Tsai
Cc: mturquette, matthias.bgg, u.kleine-koenig, chun-jie.chen,
miles.chen, linux-clk, linux-kernel, linux-arm-kernel,
linux-mediatek
Quoting Chen-Yu Tsai (2023-10-19 22:06:35)
> On Thu, Oct 19, 2023 at 8:49 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
> >
> > Fix the parenting of clocks imp_iic_wrap_ap_clock_i2c{4-6}, as those
> > are effectively parented to infra_ao_i2c{4-6} and not to the I2C_AP.
> > This permits the correct (and full) enablement and disablement of the
> > I2C4, I2C5 and I2C6 bus clocks, satisfying the whole clock tree of
> > those.
> >
> > As an example, when requesting to enable imp_iic_wrap_ap_clock_i2c4:
> >
> > Before: infra_ao_i2c_ap -> imp_iic_wrap_ap_clock_i2c4
> > After: infra_ao_i2c_ap -> infra_ao_i2c4 -> imp_iic_wrap_ap_clock_i2c4
> >
> > Fixes: 66cd0b4b0ce5 ("clk: mediatek: Add MT8186 imp i2c wrapper clock support")
> > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>
> I'm curious about what led to discovering this error?
>
Is that an acked-by?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] clk: mediatek: mt8186: Change I2C 4/5/6 ap clocks parent to infra
2023-10-24 2:52 ` Stephen Boyd
@ 2023-10-24 2:58 ` Chen-Yu Tsai
2023-10-24 7:47 ` Yu-chang Lee (李禹璋)
0 siblings, 1 reply; 11+ messages in thread
From: Chen-Yu Tsai @ 2023-10-24 2:58 UTC (permalink / raw)
To: Stephen Boyd
Cc: AngeloGioacchino Del Regno, mturquette, matthias.bgg,
u.kleine-koenig, chun-jie.chen, miles.chen, linux-clk,
linux-kernel, linux-arm-kernel, linux-mediatek
On Tue, Oct 24, 2023 at 10:52 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Chen-Yu Tsai (2023-10-19 22:06:35)
> > On Thu, Oct 19, 2023 at 8:49 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@collabora.com> wrote:
> > >
> > > Fix the parenting of clocks imp_iic_wrap_ap_clock_i2c{4-6}, as those
> > > are effectively parented to infra_ao_i2c{4-6} and not to the I2C_AP.
> > > This permits the correct (and full) enablement and disablement of the
> > > I2C4, I2C5 and I2C6 bus clocks, satisfying the whole clock tree of
> > > those.
> > >
> > > As an example, when requesting to enable imp_iic_wrap_ap_clock_i2c4:
> > >
> > > Before: infra_ao_i2c_ap -> imp_iic_wrap_ap_clock_i2c4
> > > After: infra_ao_i2c_ap -> infra_ao_i2c4 -> imp_iic_wrap_ap_clock_i2c4
> > >
> > > Fixes: 66cd0b4b0ce5 ("clk: mediatek: Add MT8186 imp i2c wrapper clock support")
> > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> >
> > I'm curious about what led to discovering this error?
> >
>
> Is that an acked-by?
MediaTek engineers are saying the original code already matches the
documentation provided by their hardware engineers. I'm trying to get
them to respond on the mailing list.
ChenYu
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] clk: mediatek: mt8186: Change I2C 4/5/6 ap clocks parent to infra
2023-10-24 2:58 ` Chen-Yu Tsai
@ 2023-10-24 7:47 ` Yu-chang Lee (李禹璋)
2023-10-24 9:20 ` Chen-Yu Tsai
0 siblings, 1 reply; 11+ messages in thread
From: Yu-chang Lee (李禹璋) @ 2023-10-24 7:47 UTC (permalink / raw)
To: wenst@chromium.org, sboyd@kernel.org
Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
mturquette@baylibre.com, u.kleine-koenig@pengutronix.de,
Chun-Jie Chen (陳浚桀),
linux-arm-kernel@lists.infradead.org,
Miles Chen (陳民樺), linux-clk@vger.kernel.org,
matthias.bgg@gmail.com, angelogioacchino.delregno@collabora.com
On Tue, 2023-10-24 at 10:58 +0800, Chen-Yu Tsai wrote:
> On Tue, Oct 24, 2023 at 10:52 AM Stephen Boyd <sboyd@kernel.org>
> wrote:
> >
> > Quoting Chen-Yu Tsai (2023-10-19 22:06:35)
> > > On Thu, Oct 19, 2023 at 8:49 PM AngeloGioacchino Del Regno
> > > <angelogioacchino.delregno@collabora.com> wrote:
> > > >
> > > > Fix the parenting of clocks imp_iic_wrap_ap_clock_i2c{4-6}, as
> > > > those
> > > > are effectively parented to infra_ao_i2c{4-6} and not to the
> > > > I2C_AP.
> > > > This permits the correct (and full) enablement and disablement
> > > > of the
> > > > I2C4, I2C5 and I2C6 bus clocks, satisfying the whole clock tree
> > > > of
> > > > those.
> > > >
> > > > As an example, when requesting to enable
> > > > imp_iic_wrap_ap_clock_i2c4:
> > > >
> > > > Before: infra_ao_i2c_ap -> imp_iic_wrap_ap_clock_i2c4
> > > > After: infra_ao_i2c_ap -> infra_ao_i2c4 ->
> > > > imp_iic_wrap_ap_clock_i2c4
> > > >
> > > > Fixes: 66cd0b4b0ce5 ("clk: mediatek: Add MT8186 imp i2c wrapper
> > > > clock support")
> > > > Signed-off-by: AngeloGioacchino Del Regno <
> > > > angelogioacchino.delregno@collabora.com>
> > >
> > > I'm curious about what led to discovering this error?
> > >
> >
> > Is that an acked-by?
>
> MediaTek engineers are saying the original code already matches the
> documentation provided by their hardware engineers. I'm trying to get
> them to respond on the mailing list.
>
> ChenYu
>
After checking with I2C clock hardware designer there is no
infra_ao_i2c{4-6} clock gate in between. And the clock document at hand
aslo shows the same result. Generallly speaking, we would like to keep
sw setting align with the hardware design document. I would recommand
not to change this part of code, but enable infra_ao_i2c{4-6} prior to
the usage of imp_iic_wrap_ap_clock_i2c clock.
Best Regards,
YuChang
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] clk: mediatek: mt8186: Change I2C 4/5/6 ap clocks parent to infra
2023-10-24 7:47 ` Yu-chang Lee (李禹璋)
@ 2023-10-24 9:20 ` Chen-Yu Tsai
2023-10-25 9:50 ` Yu-chang Lee (李禹璋)
0 siblings, 1 reply; 11+ messages in thread
From: Chen-Yu Tsai @ 2023-10-24 9:20 UTC (permalink / raw)
To: Yu-chang Lee (李禹璋)
Cc: sboyd@kernel.org, linux-kernel@vger.kernel.org,
linux-mediatek@lists.infradead.org, mturquette@baylibre.com,
u.kleine-koenig@pengutronix.de,
Chun-Jie Chen (陳浚桀),
linux-arm-kernel@lists.infradead.org,
Miles Chen (陳民樺), linux-clk@vger.kernel.org,
matthias.bgg@gmail.com, angelogioacchino.delregno@collabora.com
On Tue, Oct 24, 2023 at 3:47 PM Yu-chang Lee (李禹璋)
<Yu-chang.Lee@mediatek.com> wrote:
>
> On Tue, 2023-10-24 at 10:58 +0800, Chen-Yu Tsai wrote:
> > On Tue, Oct 24, 2023 at 10:52 AM Stephen Boyd <sboyd@kernel.org>
> > wrote:
> > >
> > > Quoting Chen-Yu Tsai (2023-10-19 22:06:35)
> > > > On Thu, Oct 19, 2023 at 8:49 PM AngeloGioacchino Del Regno
> > > > <angelogioacchino.delregno@collabora.com> wrote:
> > > > >
> > > > > Fix the parenting of clocks imp_iic_wrap_ap_clock_i2c{4-6}, as
> > > > > those
> > > > > are effectively parented to infra_ao_i2c{4-6} and not to the
> > > > > I2C_AP.
> > > > > This permits the correct (and full) enablement and disablement
> > > > > of the
> > > > > I2C4, I2C5 and I2C6 bus clocks, satisfying the whole clock tree
> > > > > of
> > > > > those.
> > > > >
> > > > > As an example, when requesting to enable
> > > > > imp_iic_wrap_ap_clock_i2c4:
> > > > >
> > > > > Before: infra_ao_i2c_ap -> imp_iic_wrap_ap_clock_i2c4
> > > > > After: infra_ao_i2c_ap -> infra_ao_i2c4 ->
> > > > > imp_iic_wrap_ap_clock_i2c4
> > > > >
> > > > > Fixes: 66cd0b4b0ce5 ("clk: mediatek: Add MT8186 imp i2c wrapper
> > > > > clock support")
> > > > > Signed-off-by: AngeloGioacchino Del Regno <
> > > > > angelogioacchino.delregno@collabora.com>
> > > >
> > > > I'm curious about what led to discovering this error?
> > > >
> > >
> > > Is that an acked-by?
> >
> > MediaTek engineers are saying the original code already matches the
> > documentation provided by their hardware engineers. I'm trying to get
> > them to respond on the mailing list.
> >
> > ChenYu
> >
> After checking with I2C clock hardware designer there is no
> infra_ao_i2c{4-6} clock gate in between. And the clock document at hand
> aslo shows the same result. Generallly speaking, we would like to keep
> sw setting align with the hardware design document. I would recommand
> not to change this part of code, but enable infra_ao_i2c{4-6} prior to
> the usage of imp_iic_wrap_ap_clock_i2c clock.
Are infra_ao_i2c{4-6} actually used by the hardware? If so, for what purpose?
If it is actually needed by the hardware and it is not in the existing path,
then it needs to be described in the device tree and handled by the driver.
ChenYu
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] clk: mediatek: mt8186: Change I2C 4/5/6 ap clocks parent to infra
2023-10-24 9:20 ` Chen-Yu Tsai
@ 2023-10-25 9:50 ` Yu-chang Lee (李禹璋)
2023-10-25 11:29 ` AngeloGioacchino Del Regno
0 siblings, 1 reply; 11+ messages in thread
From: Yu-chang Lee (李禹璋) @ 2023-10-25 9:50 UTC (permalink / raw)
To: wenst@chromium.org
Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
mturquette@baylibre.com, u.kleine-koenig@pengutronix.de,
sboyd@kernel.org, linux-arm-kernel@lists.infradead.org,
Chun-Jie Chen (陳浚桀),
Miles Chen (陳民樺), linux-clk@vger.kernel.org,
matthias.bgg@gmail.com, angelogioacchino.delregno@collabora.com
On Tue, 2023-10-24 at 17:20 +0800, Chen-Yu Tsai wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> On Tue, Oct 24, 2023 at 3:47 PM Yu-chang Lee (李禹璋)
> <Yu-chang.Lee@mediatek.com> wrote:
> >
> > On Tue, 2023-10-24 at 10:58 +0800, Chen-Yu Tsai wrote:
> > > On Tue, Oct 24, 2023 at 10:52 AM Stephen Boyd <sboyd@kernel.org>
> > > wrote:
> > > >
> > > > Quoting Chen-Yu Tsai (2023-10-19 22:06:35)
> > > > > On Thu, Oct 19, 2023 at 8:49 PM AngeloGioacchino Del Regno
> > > > > <angelogioacchino.delregno@collabora.com> wrote:
> > > > > >
> > > > > > Fix the parenting of clocks imp_iic_wrap_ap_clock_i2c{4-6},
> as
> > > > > > those
> > > > > > are effectively parented to infra_ao_i2c{4-6} and not to
> the
> > > > > > I2C_AP.
> > > > > > This permits the correct (and full) enablement and
> disablement
> > > > > > of the
> > > > > > I2C4, I2C5 and I2C6 bus clocks, satisfying the whole clock
> tree
> > > > > > of
> > > > > > those.
> > > > > >
> > > > > > As an example, when requesting to enable
> > > > > > imp_iic_wrap_ap_clock_i2c4:
> > > > > >
> > > > > > Before: infra_ao_i2c_ap -> imp_iic_wrap_ap_clock_i2c4
> > > > > > After: infra_ao_i2c_ap -> infra_ao_i2c4 ->
> > > > > > imp_iic_wrap_ap_clock_i2c4
> > > > > >
> > > > > > Fixes: 66cd0b4b0ce5 ("clk: mediatek: Add MT8186 imp i2c
> wrapper
> > > > > > clock support")
> > > > > > Signed-off-by: AngeloGioacchino Del Regno <
> > > > > > angelogioacchino.delregno@collabora.com>
> > > > >
> > > > > I'm curious about what led to discovering this error?
> > > > >
> > > >
> > > > Is that an acked-by?
> > >
> > > MediaTek engineers are saying the original code already matches
> the
> > > documentation provided by their hardware engineers. I'm trying to
> get
> > > them to respond on the mailing list.
> > >
> > > ChenYu
> > >
> > After checking with I2C clock hardware designer there is no
> > infra_ao_i2c{4-6} clock gate in between. And the clock document at
> hand
> > aslo shows the same result. Generallly speaking, we would like to
> keep
> > sw setting align with the hardware design document. I would
> recommand
> > not to change this part of code, but enable infra_ao_i2c{4-6} prior
> to
> > the usage of imp_iic_wrap_ap_clock_i2c clock.
>
> Are infra_ao_i2c{4-6} actually used by the hardware? If so, for what
> purpose?
According to hardware designer it servers no purpose. Just a legacy of
previous design...
> If it is actually needed by the hardware and it is not in the
> existing path,
> then it needs to be described in the device tree and handled by the
> driver.
>
> ChenYu
After reviewing hardware design diagram, hardware designer concludes
that the clock tree is indeed
top_i2c -> infra_ao_i2c{4-6}
top_i2c -> infra_ao_i2c_ap -> imp_iic_wrap_ap_clock_i2c{4-6}
so I think we should keep this clock relation unchanged.
Thanks
YuChang
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] clk: mediatek: mt8186: Change I2C 4/5/6 ap clocks parent to infra
2023-10-25 9:50 ` Yu-chang Lee (李禹璋)
@ 2023-10-25 11:29 ` AngeloGioacchino Del Regno
2023-10-26 3:49 ` Yu-chang Lee (李禹璋)
0 siblings, 1 reply; 11+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-10-25 11:29 UTC (permalink / raw)
To: Yu-chang Lee (李禹璋), wenst@chromium.org
Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
mturquette@baylibre.com, u.kleine-koenig@pengutronix.de,
sboyd@kernel.org, linux-arm-kernel@lists.infradead.org,
Chun-Jie Chen (陳浚桀),
Miles Chen (陳民樺), linux-clk@vger.kernel.org,
matthias.bgg@gmail.com
Il 25/10/23 11:50, Yu-chang Lee (李禹璋) ha scritto:
> On Tue, 2023-10-24 at 17:20 +0800, Chen-Yu Tsai wrote:
>>
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>> On Tue, Oct 24, 2023 at 3:47 PM Yu-chang Lee (李禹璋)
>> <Yu-chang.Lee@mediatek.com> wrote:
>>>
>>> On Tue, 2023-10-24 at 10:58 +0800, Chen-Yu Tsai wrote:
>>>> On Tue, Oct 24, 2023 at 10:52 AM Stephen Boyd <sboyd@kernel.org>
>>>> wrote:
>>>>>
>>>>> Quoting Chen-Yu Tsai (2023-10-19 22:06:35)
>>>>>> On Thu, Oct 19, 2023 at 8:49 PM AngeloGioacchino Del Regno
>>>>>> <angelogioacchino.delregno@collabora.com> wrote:
>>>>>>>
>>>>>>> Fix the parenting of clocks imp_iic_wrap_ap_clock_i2c{4-6},
>> as
>>>>>>> those
>>>>>>> are effectively parented to infra_ao_i2c{4-6} and not to
>> the
>>>>>>> I2C_AP.
>>>>>>> This permits the correct (and full) enablement and
>> disablement
>>>>>>> of the
>>>>>>> I2C4, I2C5 and I2C6 bus clocks, satisfying the whole clock
>> tree
>>>>>>> of
>>>>>>> those.
>>>>>>>
>>>>>>> As an example, when requesting to enable
>>>>>>> imp_iic_wrap_ap_clock_i2c4:
>>>>>>>
>>>>>>> Before: infra_ao_i2c_ap -> imp_iic_wrap_ap_clock_i2c4
>>>>>>> After: infra_ao_i2c_ap -> infra_ao_i2c4 ->
>>>>>>> imp_iic_wrap_ap_clock_i2c4
>>>>>>>
>>>>>>> Fixes: 66cd0b4b0ce5 ("clk: mediatek: Add MT8186 imp i2c
>> wrapper
>>>>>>> clock support")
>>>>>>> Signed-off-by: AngeloGioacchino Del Regno <
>>>>>>> angelogioacchino.delregno@collabora.com>
>>>>>>
>>>>>> I'm curious about what led to discovering this error?
>>>>>>
>>>>>
>>>>> Is that an acked-by?
>>>>
>>>> MediaTek engineers are saying the original code already matches
>> the
>>>> documentation provided by their hardware engineers. I'm trying to
>> get
>>>> them to respond on the mailing list.
>>>>
>>>> ChenYu
>>>>
>>> After checking with I2C clock hardware designer there is no
>>> infra_ao_i2c{4-6} clock gate in between. And the clock document at
>> hand
>>> aslo shows the same result. Generallly speaking, we would like to
>> keep
>>> sw setting align with the hardware design document. I would
>> recommand
>>> not to change this part of code, but enable infra_ao_i2c{4-6} prior
>> to
>>> the usage of imp_iic_wrap_ap_clock_i2c clock.
>>
>> Are infra_ao_i2c{4-6} actually used by the hardware? If so, for what
>> purpose?
>
> According to hardware designer it servers no purpose. Just a legacy of
> previous design...
>
>> If it is actually needed by the hardware and it is not in the
>> existing path,
>> then it needs to be described in the device tree and handled by the
>> driver.
>>
>> ChenYu
>
> After reviewing hardware design diagram, hardware designer concludes
> that the clock tree is indeed
>
> top_i2c -> infra_ao_i2c{4-6}
> top_i2c -> infra_ao_i2c_ap -> imp_iic_wrap_ap_clock_i2c{4-6}
>
> so I think we should keep this clock relation unchanged.
>
> Thanks
> YuChang
>
Can you please also expand on CLK_INFRA_AO_I2C{1,2,5}_ARBITER clocks?
Is the I2C arbiter also legacy of previous designs?
Please check [1], as I've sent a commit adding those in the devicetree.
Thanks,
Angelo
[1]:
https://lore.kernel.org/all/20231020075540.15191-1-angelogioacchino.delregno@collabora.com/
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] clk: mediatek: mt8186: Change I2C 4/5/6 ap clocks parent to infra
2023-10-25 11:29 ` AngeloGioacchino Del Regno
@ 2023-10-26 3:49 ` Yu-chang Lee (李禹璋)
2023-10-26 8:24 ` AngeloGioacchino Del Regno
0 siblings, 1 reply; 11+ messages in thread
From: Yu-chang Lee (李禹璋) @ 2023-10-26 3:49 UTC (permalink / raw)
To: wenst@chromium.org, angelogioacchino.delregno@collabora.com
Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
mturquette@baylibre.com, u.kleine-koenig@pengutronix.de,
sboyd@kernel.org, linux-arm-kernel@lists.infradead.org,
Chun-Jie Chen (陳浚桀),
Miles Chen (陳民樺), linux-clk@vger.kernel.org,
matthias.bgg@gmail.com
On Wed, 2023-10-25 at 13:29 +0200, AngeloGioacchino Del Regno wrote:
> Il 25/10/23 11:50, Yu-chang Lee (李禹璋) ha scritto:
> > On Tue, 2023-10-24 at 17:20 +0800, Chen-Yu Tsai wrote:
> > >
> > > External email : Please do not click links or open attachments
> > > until
> > > you have verified the sender or the content.
> > > On Tue, Oct 24, 2023 at 3:47 PM Yu-chang Lee (李禹璋)
> > > <Yu-chang.Lee@mediatek.com> wrote:
> > > >
> > > > On Tue, 2023-10-24 at 10:58 +0800, Chen-Yu Tsai wrote:
> > > > > On Tue, Oct 24, 2023 at 10:52 AM Stephen Boyd <
> > > > > sboyd@kernel.org>
> > > > > wrote:
> > > > > >
> > > > > > Quoting Chen-Yu Tsai (2023-10-19 22:06:35)
> > > > > > > On Thu, Oct 19, 2023 at 8:49 PM AngeloGioacchino Del
> > > > > > > Regno
> > > > > > > <angelogioacchino.delregno@collabora.com> wrote:
> > > > > > > >
> > > > > > > > Fix the parenting of clocks
> > > > > > > > imp_iic_wrap_ap_clock_i2c{4-6},
> > >
> > > as
> > > > > > > > those
> > > > > > > > are effectively parented to infra_ao_i2c{4-6} and not
> > > > > > > > to
> > >
> > > the
> > > > > > > > I2C_AP.
> > > > > > > > This permits the correct (and full) enablement and
> > >
> > > disablement
> > > > > > > > of the
> > > > > > > > I2C4, I2C5 and I2C6 bus clocks, satisfying the whole
> > > > > > > > clock
> > >
> > > tree
> > > > > > > > of
> > > > > > > > those.
> > > > > > > >
> > > > > > > > As an example, when requesting to enable
> > > > > > > > imp_iic_wrap_ap_clock_i2c4:
> > > > > > > >
> > > > > > > > Before: infra_ao_i2c_ap -> imp_iic_wrap_ap_clock_i2c4
> > > > > > > > After: infra_ao_i2c_ap -> infra_ao_i2c4 ->
> > > > > > > > imp_iic_wrap_ap_clock_i2c4
> > > > > > > >
> > > > > > > > Fixes: 66cd0b4b0ce5 ("clk: mediatek: Add MT8186 imp i2c
> > >
> > > wrapper
> > > > > > > > clock support")
> > > > > > > > Signed-off-by: AngeloGioacchino Del Regno <
> > > > > > > > angelogioacchino.delregno@collabora.com>
> > > > > > >
> > > > > > > I'm curious about what led to discovering this error?
> > > > > > >
> > > > > >
> > > > > > Is that an acked-by?
> > > > >
> > > > > MediaTek engineers are saying the original code already
> > > > > matches
> > >
> > > the
> > > > > documentation provided by their hardware engineers. I'm
> > > > > trying to
> > >
> > > get
> > > > > them to respond on the mailing list.
> > > > >
> > > > > ChenYu
> > > > >
> > > >
> > > > After checking with I2C clock hardware designer there is no
> > > > infra_ao_i2c{4-6} clock gate in between. And the clock document
> > > > at
> > >
> > > hand
> > > > aslo shows the same result. Generallly speaking, we would like
> > > > to
> > >
> > > keep
> > > > sw setting align with the hardware design document. I would
> > >
> > > recommand
> > > > not to change this part of code, but enable infra_ao_i2c{4-6}
> > > > prior
> > >
> > > to
> > > > the usage of imp_iic_wrap_ap_clock_i2c clock.
> > >
> > > Are infra_ao_i2c{4-6} actually used by the hardware? If so, for
> > > what
> > > purpose?
> >
> > According to hardware designer it servers no purpose. Just a legacy
> > of
> > previous design...
> >
> > > If it is actually needed by the hardware and it is not in the
> > > existing path,
> > > then it needs to be described in the device tree and handled by
> > > the
> > > driver.
> > >
> > > ChenYu
> >
> > After reviewing hardware design diagram, hardware designer
> > concludes
> > that the clock tree is indeed
> >
> > top_i2c -> infra_ao_i2c{4-6}
> > top_i2c -> infra_ao_i2c_ap -> imp_iic_wrap_ap_clock_i2c{4-6}
> >
> > so I think we should keep this clock relation unchanged.
> >
> > Thanks
> > YuChang
> >
>
> Can you please also expand on CLK_INFRA_AO_I2C{1,2,5}_ARBITER clocks?
> Is the I2C arbiter also legacy of previous designs?
>
> Please check [1], as I've sent a commit adding those in the
> devicetree.
>
> Thanks,
> Angelo
>
> [1]:
>
https://lore.kernel.org/all/20231020075540.15191-1-angelogioacchino.delregno@collabora.com/
According to Hardware designer this arbiter clock is also lagecy of
previous design and serve no function. And they are conneted to top_i2c
as well.
top_i2c-> CLK_INFRA_AO_I2C{1,2,5}_ARBITER
Also may I know the experiment that lead to the conclusion that you
need the ARBITER clock, and the clock tree is incorrect? I will bring
it back to discuss with our I2C owner.
Thanks,
YuChang
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] clk: mediatek: mt8186: Change I2C 4/5/6 ap clocks parent to infra
2023-10-26 3:49 ` Yu-chang Lee (李禹璋)
@ 2023-10-26 8:24 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 11+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-10-26 8:24 UTC (permalink / raw)
To: Yu-chang Lee (李禹璋), wenst@chromium.org
Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
mturquette@baylibre.com, u.kleine-koenig@pengutronix.de,
sboyd@kernel.org, linux-arm-kernel@lists.infradead.org,
Chun-Jie Chen (陳浚桀),
Miles Chen (陳民樺), linux-clk@vger.kernel.org,
matthias.bgg@gmail.com
Il 26/10/23 05:49, Yu-chang Lee (李禹璋) ha scritto:
> On Wed, 2023-10-25 at 13:29 +0200, AngeloGioacchino Del Regno wrote:
>> Il 25/10/23 11:50, Yu-chang Lee (李禹璋) ha scritto:
>>> On Tue, 2023-10-24 at 17:20 +0800, Chen-Yu Tsai wrote:
>>>>
>>>> External email : Please do not click links or open attachments
>>>> until
>>>> you have verified the sender or the content.
>>>> On Tue, Oct 24, 2023 at 3:47 PM Yu-chang Lee (李禹璋)
>>>> <Yu-chang.Lee@mediatek.com> wrote:
>>>>>
>>>>> On Tue, 2023-10-24 at 10:58 +0800, Chen-Yu Tsai wrote:
>>>>>> On Tue, Oct 24, 2023 at 10:52 AM Stephen Boyd <
>>>>>> sboyd@kernel.org>
>>>>>> wrote:
>>>>>>>
>>>>>>> Quoting Chen-Yu Tsai (2023-10-19 22:06:35)
>>>>>>>> On Thu, Oct 19, 2023 at 8:49 PM AngeloGioacchino Del
>>>>>>>> Regno
>>>>>>>> <angelogioacchino.delregno@collabora.com> wrote:
>>>>>>>>>
>>>>>>>>> Fix the parenting of clocks
>>>>>>>>> imp_iic_wrap_ap_clock_i2c{4-6},
>>>>
>>>> as
>>>>>>>>> those
>>>>>>>>> are effectively parented to infra_ao_i2c{4-6} and not
>>>>>>>>> to
>>>>
>>>> the
>>>>>>>>> I2C_AP.
>>>>>>>>> This permits the correct (and full) enablement and
>>>>
>>>> disablement
>>>>>>>>> of the
>>>>>>>>> I2C4, I2C5 and I2C6 bus clocks, satisfying the whole
>>>>>>>>> clock
>>>>
>>>> tree
>>>>>>>>> of
>>>>>>>>> those.
>>>>>>>>>
>>>>>>>>> As an example, when requesting to enable
>>>>>>>>> imp_iic_wrap_ap_clock_i2c4:
>>>>>>>>>
>>>>>>>>> Before: infra_ao_i2c_ap -> imp_iic_wrap_ap_clock_i2c4
>>>>>>>>> After: infra_ao_i2c_ap -> infra_ao_i2c4 ->
>>>>>>>>> imp_iic_wrap_ap_clock_i2c4
>>>>>>>>>
>>>>>>>>> Fixes: 66cd0b4b0ce5 ("clk: mediatek: Add MT8186 imp i2c
>>>>
>>>> wrapper
>>>>>>>>> clock support")
>>>>>>>>> Signed-off-by: AngeloGioacchino Del Regno <
>>>>>>>>> angelogioacchino.delregno@collabora.com>
>>>>>>>>
>>>>>>>> I'm curious about what led to discovering this error?
>>>>>>>>
>>>>>>>
>>>>>>> Is that an acked-by?
>>>>>>
>>>>>> MediaTek engineers are saying the original code already
>>>>>> matches
>>>>
>>>> the
>>>>>> documentation provided by their hardware engineers. I'm
>>>>>> trying to
>>>>
>>>> get
>>>>>> them to respond on the mailing list.
>>>>>>
>>>>>> ChenYu
>>>>>>
>>>>>
>>>>> After checking with I2C clock hardware designer there is no
>>>>> infra_ao_i2c{4-6} clock gate in between. And the clock document
>>>>> at
>>>>
>>>> hand
>>>>> aslo shows the same result. Generallly speaking, we would like
>>>>> to
>>>>
>>>> keep
>>>>> sw setting align with the hardware design document. I would
>>>>
>>>> recommand
>>>>> not to change this part of code, but enable infra_ao_i2c{4-6}
>>>>> prior
>>>>
>>>> to
>>>>> the usage of imp_iic_wrap_ap_clock_i2c clock.
>>>>
>>>> Are infra_ao_i2c{4-6} actually used by the hardware? If so, for
>>>> what
>>>> purpose?
>>>
>>> According to hardware designer it servers no purpose. Just a legacy
>>> of
>>> previous design...
>>>
>>>> If it is actually needed by the hardware and it is not in the
>>>> existing path,
>>>> then it needs to be described in the device tree and handled by
>>>> the
>>>> driver.
>>>>
>>>> ChenYu
>>>
>>> After reviewing hardware design diagram, hardware designer
>>> concludes
>>> that the clock tree is indeed
>>>
>>> top_i2c -> infra_ao_i2c{4-6}
>>> top_i2c -> infra_ao_i2c_ap -> imp_iic_wrap_ap_clock_i2c{4-6}
>>>
>>> so I think we should keep this clock relation unchanged.
>>>
>>> Thanks
>>> YuChang
>>>
>>
>> Can you please also expand on CLK_INFRA_AO_I2C{1,2,5}_ARBITER clocks?
>> Is the I2C arbiter also legacy of previous designs?
>>
>> Please check [1], as I've sent a commit adding those in the
>> devicetree.
>>
>> Thanks,
>> Angelo
>>
>> [1]:
>>
> https://lore.kernel.org/all/20231020075540.15191-1-angelogioacchino.delregno@collabora.com/
>
> According to Hardware designer this arbiter clock is also lagecy of
> previous design and serve no function. And they are conneted to top_i2c
> as well.
>
> top_i2c-> CLK_INFRA_AO_I2C{1,2,5}_ARBITER
>
> Also may I know the experiment that lead to the conclusion that you
> need the ARBITER clock, and the clock tree is incorrect? I will bring
> it back to discuss with our I2C owner.
>
I had lockups during boot and PM suspend/resume, plus, I was getting issues with
losing trackpad functionality; adding the arbiter clocks fixed the issue.
Regards,
Angelo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-10-26 8:25 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-19 12:49 [PATCH] clk: mediatek: mt8186: Change I2C 4/5/6 ap clocks parent to infra AngeloGioacchino Del Regno
2023-10-20 5:06 ` Chen-Yu Tsai
2023-10-20 7:52 ` AngeloGioacchino Del Regno
2023-10-24 2:52 ` Stephen Boyd
2023-10-24 2:58 ` Chen-Yu Tsai
2023-10-24 7:47 ` Yu-chang Lee (李禹璋)
2023-10-24 9:20 ` Chen-Yu Tsai
2023-10-25 9:50 ` Yu-chang Lee (李禹璋)
2023-10-25 11:29 ` AngeloGioacchino Del Regno
2023-10-26 3:49 ` Yu-chang Lee (李禹璋)
2023-10-26 8:24 ` AngeloGioacchino Del Regno
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).