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

From: Andreas FÃrber
Date: Sun Nov 27 2016 - 17:43:13 EST


Andrew,

Am 27.11.2016 um 23:08 schrieb Andrew Lunn:
>>> This driver already supports nearly 30 different Marvell switch
>>> models. Please document why the marvell,mv88e6176 is special and why
>>> it needs its own compatible string when the others don't.
>>
>> I don't understand.
>
> Think about what i said. Why does the 6176 need its own compatible
> string, when the two 6352s and the 6165 on the zii-devel-b don't have
> one? And the DIR 665 has a 6171, which does not have a compatible
> string of its own. The clearfog actually has a 6176, and it seems to
> work fine without a compatible string.
>
>> You as driver author should know that the .data pointer is vital to your
>> driver
>
> Exactly, so if i ask why is it needed, maybe you should stop and think
> for a while.
>
>> you even recently accepted another model that conflicted with
>> my patch.
>
> And think about that also, and you will find the 6390 family, who's
> first device is 6190, is not compatible with the 6085, and so needs a
> different compatible string.

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.)

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?

Regards,
Andreas

--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)