On Tue, 2022-01-11 at 17:36 -0600, Rob Herring wrote:

On Thu, Dec 16, 2021 at 8:06 PM Biao Huang [off-list ref]
wrote:

quoted

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.

quoted

     and eth driver clock handling will be complex;

How so?

Rob

OK, I'll add a driver patch to make clock setting more reasonable,
and modify this patch as previous comments.

Thanks for your comments~