Re: [PATCH 2/2] net: dsa: mv88e6xxx: Add 88E6176 device tree support

[Andrew Lunn]

On Mon, Nov 28, 2016 at 04:44:47PM +0100, Uwe Kleine-König wrote:
> On 11/28/2016 02:17 PM, Andrew Lunn wrote:
> >> I still wonder (and didn't get an answer back when I asked about this)
> >> why a comment is preferred here. For other devices I know it's usual and
> >> requested by the maintainers to use:
> >>
> >> 	compatible = "exact name", "earlyer device to match driver";
> >>
> >> . This is more robust, documents the situation more formally and makes
> >> it better greppable. The price to pay is only a few bytes in the dtb
> >> which IMO is ok.
> > 
> > We did discuss this a while back. The information is useless and
> > should to be ignored if present.
> 
> Who is "we"?

Anybody on netdev a while back, but mostly Vivien and myself.

> I'd say fail to probe is the right thing to do. Of course that doesn't
> work for already supported models because it will break compatibility.

And that is the first rule of device tree, never break backwards
compatibility.

> Also it seems wrong to write "marvell,mv88e6085" (only) if I know the
> hardware is really a "marvell,mv88e6176".

Why? Remember, the property name is called "compatible", not "is".

> > Linus has said he does not like ARM devices because of all the busses
> > which are not enumerable. Here we have a device which with a little
> > bit of help we can enumerate. So we should. 
> 
> If you write
> 
> 	compatible = "marvell,mv88e6176", "marvell,mv88e6085";
> 
> you can still enumerate in the same way as before.

Sure it would. But if the driver decides to ignore it, it is likely to
be wrong. Developers are used to comments being wrong. We are
suspicious of comments, we don't trust them 100%. But if somebody sees
a property in a device tree, they put a higher 'trustability' value on
it. But it actually has less trustable than a comment.
 
> There are several more instances where the device tree specifies
> something that could be probed instead. Some examples:
> 
> 	compatible = "ethernet-phy-id0141.0DD1", "ethernet-phy-ieee802.3-c22";

There are examples of phys which don't implemented register 2 and
3. In that case, you do need to specify the ID, if you want the
correct driver to load.

> 	compatible = "spansion,s25fl164k", "jedec,spi-nor";

And there was a push recently to add "jedec,spi-nor" everywhere and
deprecate a specific compatible because it is also not needed.

> 	compatible = "fsl,imx25-flexcan", "fsl,p1010-flexcan";
> 	compatible = "arm,pl011", "arm,primecell";

I don't know these hardwares, so cannot comment.

  Andrew