Re: [PATCH] clk: mediatek: mt8186: Change I2C 4/5/6 ap clocks parent to infra

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 (李禹璋)
[off-list ref] wrote:

quoted

On Tue, 2023-10-24 at 10:58 +0800, Chen-Yu Tsai wrote:

quoted

On Tue, Oct 24, 2023 at 10:52 AM Stephen Boyd [off-list ref]
wrote:

quoted

Quoting Chen-Yu Tsai (2023-10-19 22:06:35)

quoted

On Thu, Oct 19, 2023 at 8:49 PM AngeloGioacchino Del Regno
[off-list ref] wrote:

quoted

Fix the parenting of clocks imp_iic_wrap_ap_clock_i2c{4-6}, 

as

quoted

quoted

quoted

quoted

quoted

those
are effectively parented to infra_ao_i2c{4-6} and not to

the

quoted

quoted

quoted

quoted

quoted

I2C_AP.
This permits the correct (and full) enablement and

disablement

quoted

quoted

quoted

quoted

quoted

of the
I2C4, I2C5 and I2C6 bus clocks, satisfying the whole clock

tree

quoted

quoted

quoted

quoted

quoted

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

quoted

quoted

quoted

quoted

quoted

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

quoted

quoted

documentation provided by their hardware engineers. I'm trying to

get

quoted

quoted

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

quoted

aslo shows the same result. Generallly speaking, we would like to

keep

quoted

sw setting align with the hardware design document. I would

recommand

quoted

not to change this part of code, but enable infra_ao_i2c{4-6} prior

to

quoted

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