回覆: [PATCH net-next v2 1/6] dt-bindings: net: ftgmac100: Add AST2700 compatible

[Jacky Chou]

Hi Conor,

Thank you for your reply.

> > dt-bindings: net: ftgmac100: Add AST2700 compatible
> >
> > This adds aspeed,ast2700-mac to the FTGMAC100 compatible list, which
> > requires the faraday,ftgmac100 fallback. It also updates the RMII RCLK
> > clock description to include AST2700, and allows the resets property
> > for
> > AST2700 in the same way as for AST2600.
> >
> > > diff --git
> > > a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> > > b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> > > index d14410018bcf6..bee9570a6f63f 100644
> > > --- a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> > > +++ b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> > > @@ -18,6 +18,7 @@ properties:
> > >                - aspeed,ast2400-mac
> > >                - aspeed,ast2500-mac
> > >                - aspeed,ast2600-mac
> > > +              - aspeed,ast2700-mac
> > >            - const: faraday,ftgmac100
> >
> > [Severity: Low]
> > With this patch, "aspeed,ast2700-mac", "faraday,ftgmac100" becomes a
> > valid compatible pair. However, at this commit, ftgmac100_of_match[]
> > in drivers/net/ethernet/faraday/ftgmac100.c has no aspeed,ast2700-mac
> entry.
> > Such a node could only bind through the generic faraday,ftgmac100
> > entry (FTGMAC100_FARADAY).
> >
> > On that path, ftgmac100_probe() would do the following:
> >
> >   - program the ring-end bits at BIT(15) instead of the Aspeed BIT(30)
> >   - leave is_aspeed false, so ftgmac100_setup_clk() never claims
> >     MACCLK/RCLK
> >   - register the embedded MDIO bus through ftgmac100_setup_mdio()
> >   - never set a DMA mask wider than 32 bits
> >
> > Would an AST2700 node work correctly if it fell back to the Faraday
> > match?
> 
> Sounds like no, can the submitter please clarify if the fallback is valid?
> 

Agreed. I think the 'aspeed,ast2700-mac' should not be based on the 'faraday,ftgmac100'.
Maybe the dt-bindings put ast2700-mac as a const like:
```
...
properties:
  compatible:
    oneOf:
      - const: faraday,ftgmac100
      - const: aspeed,ast2700-mac             // as a const
      - items:
          - enum:
              - aspeed,ast2400-mac
              - aspeed,ast2500-mac
              - aspeed,ast2600-mac
          - const: faraday,ftgmac100
...
```

Thanks,
Jacky