Re: [PATCH 1/2] dt-bindings: iio: imu: mpu6050: Document invensense,icm20608d

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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.

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




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux