Messages in this thread | | | From | Yu-chang Lee (李禹璋) <> | Subject | Re: [PATCH] clk: mediatek: mt8186: Change I2C 4/5/6 ap clocks parent to infra | Date | Wed, 25 Oct 2023 09:50:11 +0000 |
| |
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
| |