Am 19. Oktober 2022 10:28:59 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>: >Il 17/10/22 12:41, Frank Wunderlich ha scritto: >If this SoC has a different clock tree... then you should add bindings for this >kind of clock tree. > >CLK_INFRA_IPCIER_CK is *not* a peri clock: "peri" means PERICFG, which does not >seem to be present in this SoC... so no, you can't assign it to "peri_26m", nor >you can assign it to tl_32k, as that's not a 32KHz clock. > >CLK_INFRA_PCIEB_CK can be a "top_133m" clock... as it is gating "sysaxi_sel", >which is a topckgen clock. > >CLK_INFRA_IPCIE_CK is your "tl_(something)" clock, as that's effectively gating >"pextp_tl_ck_sel" (which is the PCIe Transaction Layer clock mux). > >CLK_INFRA_IPCIE_PIPE_CK seems to be parented to "top_xtal", frequency = 40MHz, >so I don't see how can this be a pl_250m? Looks like being a 40m clock and I >wish we didn't have clock frequencies specified in the names, as "pl" would fit, >but "pl_250m" does not. >I wonder if we can change the clock names and reflect the changes to the mt8192 >devicetree (mt8195 does not have any pcie node yet), and if that would be a good >idea right now. > >...and I've left the first for last, because... > >CLK_INFRA_PCIE_SEL: I have no datasheet for this SoC, but if you're sure that >this clock is selecting the source clock to CLK_INFRA_IPCIE_CK, then the clock >driver is wrong... > >Right now, I see the following: > >static const char *const infra_pcie_parents[] __initconst = { > "top_rtc_32p7k", "csw_f26m_sel", "top_xtal", "pextp_tl_ck_sel" >}; > >GATE_INFRA2(CLK_INFRA_IPCIE_CK, "infra_ipcie", "pextp_tl_ck_sel", 12), > >MUX_GATE_CLR_SET_UPD(CLK_INFRA_PCIE_SEL, "infra_pcie_sel", > infra_pcie_parents, 0x0028, 0x0020, 0x0024, 0, 2, > -1, -1, -1), > >....so if you're right, we should instead have: > >GATE_INFRA2(CLK_INFRA_IPCIE_CK, "infra_ipcie", "infra_pcie_sel", 12), > >....with this meaning that adding CLK_INFRA_PCIE_SEL in devicetree is useless. > >This clock tree looks a bit unclear (because again, there's no datasheet around), >but that's what I understand with a rather fast look in the clock drivers and >with some experience on other MTK SoCs. > >Then again, if this tree is effectively incompatible with the one from MT8192 and >MT8195, you should have different clock names... and just as a fast idea: > >clock-names = "axi", "tl", "pl", "top"; > >with clocks, in order: >CLK_INFRA_PCIEB_CK, CLK_INFRA_IPCIE_CK, >CLK_INFRA_IPCIE_PIPE_CK, CLK_INFRA_IPCIER_CK. > >...but feel free to reiterate that :-) >Hope that was helpful. > >Cheers, >Angelo Hi, thanks for digging into the clock-driver. Currently i have mapped it like this (see comment to part7) clocks = <&infracfg CLK_INFRA_IPCIE_PIPE_CK>, <&infracfg CLK_INFRA_IPCIE_CK>, <&clk40m>, <&clk40m>, <&infracfg CLK_INFRA_IPCIER_CK>, <&infracfg CLK_INFRA_IPCIEB_CK>; clock-names = "pl_250m", "tl_26m", "tl_96m", "tl_32k", "peri_26m", "top_133m"; Mtk says it has same IP block and except missing clocks it is compatible with mt8xxx gen3 pcie driver/binding. Pcie driver only enables the clocks in bulk without names,but binding requires the names property so mapping needs to be correct. regards Frank