On Mon, 21 Mar 2022 19:07:26 +0100 Michael Srba <Michael.Srba@xxxxxxxxx> wrote: > On 21. 03. 22 18:42, Jonathan Cameron wrote: > > On Mon, 21 Mar 2022 16:22:38 +0100 > > Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: > > > >> On 21/03/2022 16:04, Jonathan Cameron wrote: > >>> On Mon, 21 Mar 2022 09:04:11 +0100 > >>> Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: > >>> > >>>> On 20/03/2022 16:12, Jonathan Cameron wrote: > >>>>> On Thu, 10 Mar 2022 22:24:03 +0100 > >>>>> Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxxxxx> wrote: > >>>>> > >>>>>> On 10/03/2022 19:56, Michael Srba wrote: > >>>>>>> Hi, > >>>>>>> the thing is, the only reason the different compatible is needed at all > >>>>>>> is that the chip presents a different WHOAMI, and the invensense,icm20608 > >>>>>>> compatible seems to imply the non-D WHOAMI value. > >>>>>> But this is a driver implementation issue, not related to bindings. > >>>>>> Bindings describe the hardware. > >>>>> Indeed, but the key thing here is the WHOAMI register is hardware. > >>>>> > >>>>>> > >>>>>>> I'm not sure how the driver would react to both compatibles being present, > >>>>>>> and looking at the driver code, it seems that icm20608d is not the only > >>>>>>> fully icm20608-compatible (to the extent of features supported by > >>>>>>> the driver, and excluding the WHOAMI value) invensense IC, yet none > >>>>>>> of these other ICs add the invensense,icm20608 compatible, so I guess I > >>>>>>> don't see a good reason to do something different. > >>>>>> Probably my question should be asked earlier, when these other > >>>>>> compatibles were added in such way. > >>>>>> > >>>>>> Skipping the DMP core, the new device is fully backwards compatible with > >>>>>> icm20608. > >>>>> No. It is 'nearly' compatible... The different WHOAMI value (used > >>>>> to check the chip is the one we expect) makes it incompatible. Now we > >>>>> could change the driver to allow for that bit of incompatibility and > >>>>> some other drivers do (often warning when the whoami is wrong but continuing > >>>>> anyway). > >>>> Different value of HW register within the same programming model does > >>>> not make him incompatible. Quite contrary - it is compatible and to > >>>> differentiate variants you do not need specific compatibles. > >>> Whilst I don't personally agree with the definition of "compatible" > >>> and think you are making false distinctions between hardware and software... > >>> > >>> I'll accept Rob's statement of best practice. However we can't just > >>> add a compatible that won't work if someone uses it on a new board > >>> that happens to run an old kernel. > >>> > >> The please explain me how this patch (the compatible set I proposed) > >> fails to work in such case? How a new board with icm20608 (not > >> icm20608d!) fails to work? > > I'm confused. An actual icm20608 would work. > > I guess you mean an icm20608d via compatible "invensense,icm20608"? > > > >> To remind, the compatible has a format of: > >> comaptible = "new", "old" > >> e.g.: "invensense,icm20608d", "invensense,icm20608" > > Old kernel fails to match invensense,icm20608d, matches on invensense,icm20608. > > Checks the WHOAMI value and reports a missmatched value and fails the probe > > as it has no idea what the part was so no idea how to support it. > > > > 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 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. 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. 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. 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. 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. Jonathan > > btw, when this is resolved, I will be sending a v3 with > fixed dt-schema errors now that I managed to reproduce > those errors locally. > > Regards, > Michael. > > Jonathan > > > >> Best regards, > >> Krzysztof >
