Date:	Fri, 19 Sep 2014 15:41:55 -0500
From:	Scott Wood <scottwood@...escale.com>
To:	Alexander Graf <agraf@...e.de>
CC:	Kim Phillips <kim.phillips@...escale.com>,
	Yoder Stuart-B08248 <stuart.yoder@...escale.com>,
	Rivera Jose-B46482 <German.Rivera@...escale.com>,
	"<gregkh@...uxfoundation.org>" <gregkh@...uxfoundation.org>,
	"<arnd@...db.de>" <arnd@...db.de>,
	"<linux-kernel@...r.kernel.org>" <linux-kernel@...r.kernel.org>,
	"<linuxppc-release@...ux.freescale.net>" 
	<linuxppc-release@...ux.freescale.net>
Subject: Re: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs

On Fri, 2014-09-19 at 22:32 +0200, Alexander Graf wrote:
> 
> On 19.09.14 22:24, Kim Phillips wrote:
> > On Fri, 19 Sep 2014 14:06:32 -0500
> > Yoder Stuart-B08248 <stuart.yoder@...escale.com> wrote:
> > 
> >>>>>>>>> +/**
> >>>>>>>>> + * @brief    Management Complex firmware version information
> >>>>>>>>> + */
> >>>>>>>>> +#define MC_VER_MAJOR 2
> >>>>>>>>> +#define MC_VER_MINOR 0
> >>>>>>>>
> >>>>>>>> code should be adjusted to run on all *compatible* versions of h/w,
> >>>>>>>> not strictly the one set in these defines.
> >>>>>>> This comment is not precise enough be actionable.
> >>>>>>> What exactly you want to be changed  here?
> >>>>>>
> >>>>>> I think the easy thing to do is to convert the exact version check into a ranged version check: have
> >>> minimum and maximum versions you support. Or a list of exact versions you support. Or not check for the
> >>> version at all - or only for the major version and guarantee that the major version indicates backwards
> >>> compatibility.
> >>>>>
> >>>>> yes, this was my point: elsewhere I noticed the code denies to run
> >>>>> iff those defines are not matched exactly: that code should change
> >>>>> to run as Alex describes.
> >>>>>
> >>>> As I mentioned in the reply to Alex, I will remove the minor version check.
> >>>
> >>> the code should be able to run on all subsequent versions of the
> >>> h/w, even in the major version case.
> >>
> >> You're right, in the future if there are future major versions we would want this
> >> same driver to function on multiple versions of the hardware.  But at this
> >> point in time we don't know what future evolutions there will be and we 
> >> need the check to error out for now.
> > 
> > why?  We have to make the standard assumption that newer versions
> > will be backward compatible, in which case the driver should be left
> > to run.
> 
> How much is the interface set in stone? Can we indicate to the MC that
> we want version x of the protocol? Then the MC can tell us whether it's
> compatible or not.

I don't trust that new versions will be 100% backwards compatible
(though I hope they will be), but do we normally bother making a driver
refuse to run on newer versions?  Sure, if we need to explicitly match a
comptible string or PCI ID, the match will be rejected if the driver
doesn't know about it, but if it's a version in a register we usually
only check for known issues with certain versions.
 
> >>   The driver will have to be changed
> >> in the future to dynamically deal with different versions.
> >>
> >> We could add a TODO in the driver to note that.
> > 
> > "TODO: add support for new h/w versions" is almost universally true
> > for all drivers, we don't need to write that down.
> > 
> > Support for new h/w versions with new features should be
> > incrementally added once they're known.
> 
> The "version id" is basically the equivalent of the pci device id. We
> don't add wildcards there either for unknown pieces of hardware, so
> limiting to driver to "known good" devices is sane IMHO.

How would you go about adding a wildcard to a PCI ID even if you wanted
to?  Version information on PCI is not separate from device
identification.

-Scott


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists