linux-kernel - Re: [PATCH v5 2/2] iio: humidity: hdc100x: add manufacturer and device ID check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4ea235d1-46c1-87de-760f-dc4775007ae0@gmail.com>
Date:   Mon, 1 Aug 2022 09:50:28 +0800
From:   Potin Lai <potin.lai.pt@...il.com>
To:     Jonathan Cameron <jic23@...nel.org>
Cc:     Andy Shevchenko <andy.shevchenko@...il.com>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Patrick Williams <patrick@...cx.xyz>,
        Potin Lai <potin.lai@...ntatw.com>, linux-iio@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 2/2] iio: humidity: hdc100x: add manufacturer and
 device ID check


On 7/31/22 20:09, Jonathan Cameron wrote:
> On Thu, 28 Jul 2022 12:54:35 +0000
> Potin Lai <potin.lai.pt@...il.com> wrote:
>
>> Add manufacturer and device ID checking during probe, and skip the
>> checking if chip model not supported.
>>
>> Supported:
>> - HDC1000
>> - HDC1010
>> - HDC1050
>> - HDC1080
>>
>> Not supported:
>> - HDC1008
>>
>> Signed-off-by: Potin Lai <potin.lai.pt@...il.com>
> I need some more information on the 'why' of this patch.  There are a number
> of drivers that do a similar ID check but in recent times, that approach has
> been considered wrong because it breaks potential use of multiple compatible
> entries in device tree.   If a new device comes along and is backwards
> compatible with an existing one (maybe has new features, but using them is
> optional) then we want to have an entry that looks like
>
> compatible = "ti,hdc1099", "ti,hdc1080"
>
> If the new ID is not supported by the kernel that is being used, we still
> want the driver to 'work' using the fallback compatible.
>
> As such, we no generally do the following.
>
> 1) If we have a match to a device we know about but it's not the one the
>    firmware tells us to expect, print a warning but operate as if the firmware
>    had been correct - particularly if we know the parts aren't compatible
>    with each other. (this bit is optional as we should be able to assume firmware
>    doesn't do stupid things :)
> 2) If we don't match at all, print a warning about an unknown device but carry
>    on with assumption that the firmware is correct and this new device ID is
>    backwards compatible with the provided fallback compatible.
>
> So if this is just a bit of defensive programming (rather than necessary for some
> reason not yet explained) then change from returning an error on probe() to 
> printing an warning message but continuing anyway. (which is part (2) of the
> above)
Hi Jonathan,
In our hardware board, we have "ti,hdc1080" as main source, and "silabs,si7020"
for 2nd source. This two chip are locate at same bus and same slave address,
and we want to use multiple compatibles to support both chips with single device
node in device tree.
 
Ex:
compatible = "ti,hdc1099", "silabs,si7020";
 
In order to support this, I need to add ID checking mechanism into the current
hdc100x driver, so the si7020 chip will fail to probe with hdc100x driver
(because the ID checking is not failed), then success probe with si7020.
 
Base on you explanation, it looks multiple compatibles is not suitable in this
case? Would you mind advise us what would be the better approach for our case?

Thanks,
Potin