On Tue, 22 Mar 2022 11:41:11 +0100 Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: > On 22/03/2022 11:19, Jonathan Cameron wrote: > >>> Obviously it wouldn't work anyway with an old kernel, but > >>> without the fallback compatible at least there would be no error message > >>> saying that the device is not the icm20608 we expected to see. > >> I'm not sure if that's really an issue? > >> The old kernel is clearly not handling the compatible "correctly", > >> since the compatible says that the interface is a superset of > >> the icm20608 interface, and that using the icm20608 > >> interface will work. > >> If the driver makes the incorrect assumption that > >> the WHOAMI being different means the interface cannot > >> be icm20608 compatible, then that seems like an issue > >> with the driver? > >> And I believe the single reason for why catering to > >> a broken driver would ever be considered is if not doing > >> so would result in breaking the devicetree ABI promise, > >> which doesn't seem to happen here. > > > > I'll be honest I no longer care that much either way. > > > > If someone would point me to clear documentation of that > > DT ABI promise > > Documentation/devicetree/bindings/ABI.rst Hi Krzyztof, I am not willfully ignoring your reference. I simply disagree that it says what you understand it to. This may be an aspect of the intended meaning but it is sufficiently vague that I read that reference several times and did not come to the same conclusion as you have. If you want to take your description and propose it as additional documentation I would be happy to review it. I'm not keen to continue this discussion because we are talking cross purposes and going over the same ground with no fundamental change of opinions, so it is not productive use of time. Jonathan > > > and how it describes things as being compatible > > that would be great and provide me with a clear statement > > to point others to in the future. > > It's very concise, so let me decipher the first paragraph. > It is safe to > add new compatibles to the chain (so exactly like here "icm20608d, > icm20608") because the system can bind: > 1. against new compatible bringing all new features, > 2. old compatible, working with "old" or limited set of features. > > What I was explaining you in mails before, which you responded to with: > "Whilst I don't personally agree with the definition of "compatible" > and think you are making false distinctions between hardware and > software..." > we do not talk here about software, as in device driver. We talk about > bindings which describe the hardware, therefore the compatible should be > rather understood in the hardware model, not driver model. > > The compatible field does not mean that one driver is compatible with > this or other hardware. It means that devices have a compatible > programming model or interface. > > Now driver should be implemented in that way. If driver handles > "icm20608" compatible, it should nicely handle only icm20608 features, > regardless whether device is icm20608 or icm20608d. > > Now let's imagine, that icm20608d is slightly different than icm20608 in > the basic feature set. Than it's not compatible and should deserve > another separate binding entry, regardless how driver handles it. > > Keep in mind what Rob said - driver implementation can changed, but > device compatibility in bindings should stay the same. Specially that > bindings are used in other operating systems (*BSD) and software pieces > (u-boot). > > > Perhaps I've just been missing that documentation or it > > needs writing. > > > > I think that having to ignore a WHOAMI value that > > is unknown to the driver because there might be a future part > > which is compatible is a very bad way to support > > devices in a reliable fashion and going to lead to annoyed > > users and bug reports. > > I see your point. It's a safer choice than just accepting any device. > However it's a designer/programmers fault to provide a DTB with a > matching compatible for a non-compatible device. Not driver programmer > fault. Usually you do not have to protect the driver from it. > > > This is different to electing to > > using a shared compatible when two parts are introduced at > > the same time and we are doing detection in the driver of > > which variant we have. > > > > I mentioned earlier that we have this type of defensive coding > > precisely because we have had false assessments about > > compatibility in the past. This manufacturer does not in > > general document compatibility across parts. I have no idea if > > they do for this particular part as there doesn't seem to be > > a public datasheet. > > Kind of continuing my previous thought also here - it's not a problem of > driver developer, but DTB developer. If the devices are not compatible > (thus driver will not work correctly), the person using that compatible > in DTB made mistake. Bug reports should be sent to that person, not to > driver developer, not to you. > > > It didn't work before, now it won't work and will complain about it > > which may lead to some bug reports that won't be resolved but > > I'll adopt the majority opinion which seems to be that we > > don't care about that. > > Yes, we don't care but the DTB/DTS person should. :) > > > I'd also be happy to see us reduce > > the problem scope here by having a 'fix' for that rejection > > of unknown IDs that we can push back to stable kernels. > > Relaxing it to a warning should be sufficient, though we probably > > want to screen out whatever comes back from the bus if there > > is no device present at all as the WHOAMI check is also > > providing that protection. > > A dev_warn() with a disclaimer might be actually better approach. Unless > it might be a safety-critical device. > > Best regards, > Krzysztof
