netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
To: Vivien Didelot
	<vivien.didelot-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/@public.gmane.org>
Cc: "Uwe Kleine-König"
	<uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org>,
	"Rob Herring" <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	"Frank Rowand"
	<frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	"Andreas Färber" <afaerber-l3A5Bk7waGM@public.gmane.org>,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	"Michal Hrusecki" <Michal.Hrusecky-x+rMaJPWets@public.gmane.org>,
	"Tomas Hlavacek" <tomas.hlavacek-x+rMaJPWets@public.gmane.org>,
	"Bed??icha Ko??atu" <bedrich.kosata-x+rMaJPWets@public.gmane.org>,
	"Florian Fainelli"
	<f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH 2/2] net: dsa: mv88e6xxx: Add 88E6176 device tree support
Date: Tue, 29 Nov 2016 19:20:02 +0100	[thread overview]
Message-ID: <20161129182002.GB21645@lunn.ch> (raw)
In-Reply-To: <87oa0yb29b.fsf-BOtmAI95c/+pXNIQCVAXCG0Lkn3mC4nZ0tOlhedn3YvkypF1WZHjJXhe7Zk3YmMvjmZSf7Nhrd8@public.gmane.org>

On Tue, Nov 29, 2016 at 12:54:24PM -0500, Vivien Didelot wrote:
> Hi,
> 
> Uwe Kleine-König <uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org> writes:
> 
> > Also it seems wrong to write "marvell,mv88e6085" (only) if I know the
> > hardware is really a "marvell,mv88e6176".
> 
> I agree. It might be complex for a user to dig into the driver in order
> to figure out how the switch ID is read and which compatible to choose.
> 
> I've sent a patch to change this https://lkml.org/lkml/2016/6/8/1198 but
> Andrew had a stronger opinion on compatible strings, which makes sense.
> 
> >> 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.
> 
> So we don't break the existing DTS files, I like this.
> 
> The driver already prints info about the detected switch. Instead of
> failing at probe, which seems against the notion of compatible and
> breaks the existing behavior, it could report the eventual mismatch?

I'm still against it.

Say we let the driver probe and warn when the compatible string is
wrong. Is this likely to get fixed? Probably not, it is just a
warning, people ignore them.

A few years later, we have accumulated a number of broken device
trees. And suddenly we really do have a Marvell device with a broken
ID in its port register, or more likely, the revision number was not
incremented but there was incompatible register changes. We suddenly
do have to look at the compatible string. But we know many are actually
wrong. How do we know which to trust? We basically have to say, if the
compatible is "marvell,mv88e6042" we trust it, all the others we don't
trust and ignore.

Isn't that madness?

What we have today is verified correct. If this hypothetical
"marvell,mv88e6042" does happen, then no problems, we add a compatible
string for it, and it works.

We should probably add a comment to the mv88e6xxx_of_match array,
explaining how it currently works.

      Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

      parent reply	other threads:[~2016-11-29 18:20 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-27 20:57 [PATCH 1/2] Documentation: net: dsa: marvell: Add 88E6176 Andreas Färber
2016-11-27 20:57 ` [PATCH 2/2] net: dsa: mv88e6xxx: Add 88E6176 device tree support Andreas Färber
2016-11-27 21:27   ` Andrew Lunn
2016-11-27 21:50     ` Andreas Färber
2016-11-27 22:08       ` Andrew Lunn
2016-11-27 22:42         ` Andreas Färber
2016-11-27 23:10           ` Andrew Lunn
2016-11-28  8:09             ` Uwe Kleine-König
     [not found]               ` <20161128080939.ippqlytvojitefkp-jgopVnDzZD+b0XQX99//ntPVjbGH4+40kFgPdswSElo@public.gmane.org>
2016-11-28 13:17                 ` Andrew Lunn
     [not found]                   ` <20161128131735.GA4379-g2DYL2Zd6BY@public.gmane.org>
2016-11-28 15:44                     ` Uwe Kleine-König
     [not found]                       ` <2c59cc79-b6dc-9920-1725-a7785ff3b6bf-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org>
2016-11-28 16:14                         ` Andrew Lunn
2016-11-29 17:54                       ` Vivien Didelot
     [not found]                         ` <87oa0yb29b.fsf-BOtmAI95c/+pXNIQCVAXCG0Lkn3mC4nZ0tOlhedn3YvkypF1WZHjJXhe7Zk3YmMvjmZSf7Nhrd8@public.gmane.org>
2016-11-29 18:20                           ` Andrew Lunn [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161129182002.GB21645@lunn.ch \
    --to=andrew-g2dyl2zd6by@public.gmane.org \
    --cc=Michal.Hrusecky-x+rMaJPWets@public.gmane.org \
    --cc=afaerber-l3A5Bk7waGM@public.gmane.org \
    --cc=bedrich.kosata-x+rMaJPWets@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=tomas.hlavacek-x+rMaJPWets@public.gmane.org \
    --cc=uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org \
    --cc=vivien.didelot-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).