Messages in this thread | | | Subject | Re: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs | From | Scott Wood <> | Date | Fri, 19 Sep 2014 15:41:55 -0500 |
| |
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@freescale.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
| |