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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 28 Nov 2016 09:09:40 +0100
From:   Uwe Kleine-König <uwe@...ine-koenig.org>
To:     Andrew Lunn <andrew@...n.ch>, Rob Herring <robh+dt@...nel.org>,
        Frank Rowand <frowand.list@...il.com>
Cc:     Andreas Färber <afaerber@...e.de>,
        netdev@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        Michal Hrusecki <Michal.Hrusecky@....cz>,
        Tomas Hlavacek <tomas.hlavacek@....cz>,
        Bed??icha Ko??atu <bedrich.kosata@....cz>,
        Vivien Didelot <vivien.didelot@...oirfairelinux.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH 2/2] net: dsa: mv88e6xxx: Add 88E6176 device tree support

Hello Andrew,

On Mon, Nov 28, 2016 at 12:10:09AM +0100, Andrew Lunn wrote:
> > Try to see it from my perspective: I see that some vf610 device I don't
> > have (found via `git grep marvell,mv88e6` or so) uses
> > "marvell,mv88e6085". I then assume it has that device on board. How
> > would I know it doesn't? Same for the other boards you mention.
> > 
> > Unfortunately some of your replies are slightly cryptic. Had you simply
> > replied 'please just use "marvell,mv88e6085" instead', it would've been
> > much more clear what you want. (Same for extending the subject instead
> > of just pointing to some FAQ.)
> 
> By reading the FAQ you have learnt more than me saying put the correct
> tree in the subject line. By asking you to explain why you need a
> compatible string, i'm trying to make you think, look at the code and
> understand it. In the future, you might think and understand the code
> before posting a patch, and then we all save time.

I agree to Andreas though, that it makes an school teacher impression.
Something like:

	Please fix the subject. Check the FAQ for the details, which btw
	is worth a read completely.

is IMHO better in this regard and once you found the problem there you
don't need to ask back if it's that what was meant.

> > So are you okay with patch 1/2 documenting the compatible? Then we could
> > drop 2/2 and use "marvell,mv88e6176", "marvell,mv88e6085" instead of
> > just the latter. Or would you rather drop both and keep the actual chip
> > a comment?
> 
> A comment only please.

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.

Best regards
Uwe

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)