On Thu, Dec 16, 2021 at 8:06 PM Biao Huang <biao.huang@xxxxxxxxxxxx> wrote: > > Dear Rob, > Thanks for your comments~ > > For mt8195, the eth device node will look like: > eth: ethernet@11021000 { > compatible = "mediatek,mt8195-gmac", "snps,dwmac-5.10a"; > ... > clock-names = "axi", > "apb", > "mac_cg", > "mac_main", > "ptp_ref", > "rmii_internal"; > clocks = <&pericfg_ao CLK_PERI_AO_ETHERNET>, > <&pericfg_ao CLK_PERI_AO_ETHERNET_BUS>, > <&pericfg_ao CLK_PERI_AO_ETHERNET_MAC>, > <&topckgen CLK_TOP_SNPS_ETH_250M>, > <&topckgen CLK_TOP_SNPS_ETH_62P4M_PTP>, > <&topckgen CLK_TOP_SNPS_ETH_50M_RMII>; > ... > } > > 1. "rmii_internal" is a special clock only required for > RMII phy interface, dwmac-mediatek.c will enable clocks > invoking clk_bulk_prepare_enable(xx, 6) for RMII, > and clk_bulk_prepare_enable(xx, 5) for other phy interfaces. > so, mt2712/mt8195 all put "rmii_internal" clock to the > end of clock list to simplify clock handling. > > If I put mac_cg as described above, a if condition is required > for clocks description in dt-binding, just like what I do in v7 send: > - if: > properties: > compatible: > contains: > enum: > - mediatek,mt2712-gmac > > then: > properties: > clocks: > minItems: 5 > items: > - description: AXI clock > - description: APB clock > - description: MAC Main clock > - description: PTP clock > - description: RMII reference clock provided by MAC > > clock-names: > minItems: 5 > items: > - const: axi > - const: apb > - const: mac_main > - const: ptp_ref > - const: rmii_internal > > - if: > properties: > compatible: > contains: > enum: > - mediatek,mt8195-gmac > > then: > properties: > clocks: > minItems: 6 > items: > - description: AXI clock > - description: APB clock > - description: MAC clock gate > - description: MAC Main clock > - description: PTP clock > - description: RMII reference clock provided by MAC > > This introduces some duplicated description. > > 2. If I put "mac_cg" to the end of clock list, > the dt-binding file can be simple just like > what we do in this v10 patch(need fix warnings reported by "make > DT_CHECKER_FLAGS=-m dt_binding_check"). > > But for mt8195: > the eth node in dts should be modified, I hope you are defining the binding before you use it... That's not good practice and not a valid argument. > and eth driver clock handling will be complex; How so? Rob