回覆: [PATCH net-next v5 1/7] dt-bindings: net: ftgmac100: Make Aspeed compatibles standalone

[Jacky Chou]

Hi Krzysztof,

Thank you for your reply.

> > The aspeed,ast2400-mac, aspeed,ast2500-mac, and aspeed,ast2600-mac
> > compatibles are currently only valid when paired with the generic
> > "faraday,ftgmac100" fallback: if a node ever bound through the generic
> > entry alone, ftgmac100_probe() would set
> > rxdes0_edorr_mask/txdes0_edotr_mask to BIT(15) instead of the Aspeed
> > BIT(30), skip Aspeed clock setup, and leave the DMA mask too narrow.
> 
> And device would work or not? If device still works after all these steps above,
> then why isn't fallback applicable?
> 
> Be explicit, because I have no clue whether Aspeed clock setup is necessary for
> device to operate.
> 

It is necessary: ftgmac100_setup_clk() enables the MAC's clock, and
only runs when priv->is_aspeed is true - skipped if a node ever bound
through the generic entry. Without ftgmac100_setup_clk(), the MAC on
Aspeed platform would be left without a working clock, so it wouldn't
be able to configure register and TX/RX at all.

> >
> > a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> > b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> > index d14410018bcf..c870dd0bda7b 100644
> > --- a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> > +++ b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> > @@ -13,12 +13,10 @@ properties:
> >    compatible:
> >      oneOf:
> >        - const: faraday,ftgmac100
> 
> This const goes to your new enum
> 
> > -      - items:
> > -          - enum:
> > -              - aspeed,ast2400-mac
> > -              - aspeed,ast2500-mac
> > -              - aspeed,ast2600-mac
> > -          - const: faraday,ftgmac100
> > +      - enum:
> 
> here.
> 

Agreed, will collapse to a flat enum in next version:

  compatible:
    enum:
      - faraday,ftgmac100
      - aspeed,ast2400-mac
      - aspeed,ast2500-mac
      - aspeed,ast2600-mac

Thanks,
Jacky