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

[Jacky Chou]

Hi Rob,

> > 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.
> >
> > Make each of them a standalone compatible instead, so none of the
> > Aspeed compatibles require the generic fallback alongside them.
> >
> > Signed-off-by: Jacky Chou <jacky_chou@xxxxxxxxxxxxxx>
> >
> > ---
> > ftgmac100_of_match[] has had dedicated entries for AST2400, AST2500,
> > and AST2600 since commit 03a2aba50b2c ("net: ftgmac100: List all
> > compatibles"), so there is no known kernel + devicetree combination
> > where a node has actually bound through the generic entry and gotten
> > the wrong EDORR bit. This part is a binding correctness fix, not a fix
> > for an observed failure.
> >
> > Note that arch/arm/boot/dts/aspeed/aspeed-g4.dtsi, aspeed-g5.dtsi, and
> > aspeed-g6.dtsi still declare their MAC nodes with the two-string
> > "aspeed,astXXXX-mac", "faraday,ftgmac100" compatible, so this will
> > make dtbs_check warn against those files until the redundant fallback
> > compatible is dropped there as well. That .dtsi change will be sent as
> > a separate series through the Aspeed/arm-soc tree, since dt-bindings
> > changes don't go through that tree.
> 
> No. This just creates unnecessary warnings even if only temporary.
> 
> What a driver uses doesn't matter. That can evolve over time, but that doesn't
> mean we change the DT schema.
> 

Thanks for the quick look.

Just so I don't end up re-spinning this back and forth: this patch
was written in response to Conor's comment on the previous round,
where he suggested folding the AST2400/2500/2600 fix into this
series rather than deferring it:

  "I'd fix the binding here, since you're gonna generate conflicts
   with yourself otherwise. The dts changes need to be sent
   separately anyway (or at least applied separately) since binding
   changes don't go via soc maintainers."

My original plan was actually closer to what you're asking for: only
touch aspeed,ast2700-mac (zero migration cost, no existing dts uses
it yet) and leave AST2400/2500/2600 paired with the faraday,ftgmac100
fallback as-is, specifically to avoid the kind of transitional
dtbs_check warning you're pointing out here. Conor asked me to widen
the scope to all four compatibles in this same series instead.

Since the two of you seem to want different things here, could you
sync up on which approach you'd prefer? I'm happy to go either way:

 (a) revert to my original scope - keep AST2400/2500/2600 paired
     with the fallback, and only make aspeed,ast2700-mac a standalone
     compatible, since it's the one case with no existing dts users
     to break, or

 (b) keep the standalone-compatible change for all four as in this
     patch, and coordinate with the Aspeed/arm-soc maintainers so the
     aspeed-g4/g5/g6.dtsi fallback removal lands in the same cycle
     (e.g. via an immutable branch), so dtbs_check never shows a
     warning in an actual tagged release.

Let me know which you'd like and I'll send the next version
accordingly.

Thanks,
Jacky