Linux IIO development
 help / color / mirror / Atom feed
From: Potin Lai <potin.lai.pt@gmail.com>
To: Jonathan Cameron <jic23@kernel.org>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Patrick Williams <patrick@stwcx.xyz>,
	Potin Lai <potin.lai@quantatw.com>,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 2/2] iio: humidity: hdc100x: add manufacturer and device ID check
Date: Mon, 1 Aug 2022 09:50:28 +0800	[thread overview]
Message-ID: <4ea235d1-46c1-87de-760f-dc4775007ae0@gmail.com> (raw)
In-Reply-To: <20220731130959.50826fc4@jic23-huawei>


On 7/31/22 20:09, Jonathan Cameron wrote:
> On Thu, 28 Jul 2022 12:54:35 +0000
> Potin Lai <potin.lai.pt@gmail.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@gmail.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


  reply	other threads:[~2022-08-01  1:52 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-28 12:54 [PATCH v5 0/2] iio: humidity: hdc100x: add manufacturer and device ID check Potin Lai
2022-07-28 12:54 ` [PATCH v5 1/2] iio: humidity: hdc100x: switch to probe_new callback Potin Lai
2022-07-28 20:41   ` Andy Shevchenko
2022-07-29  0:43     ` Potin Lai
2022-07-29 18:00       ` Andy Shevchenko
2022-07-28 12:54 ` [PATCH v5 2/2] iio: humidity: hdc100x: add manufacturer and device ID check Potin Lai
2022-07-28 20:42   ` Andy Shevchenko
2022-07-31 12:09   ` Jonathan Cameron
2022-08-01  1:50     ` Potin Lai [this message]
2022-08-01  8:22       ` Andy Shevchenko
2022-08-01 16:12         ` Patrick Williams
2022-08-01 16:26           ` Andy Shevchenko
2022-08-01 16:30             ` Andy Shevchenko
2022-08-06 17:12               ` Jonathan Cameron
2022-08-08  9:40                 ` Krzysztof Kozlowski
2022-08-08  9:49                   ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4ea235d1-46c1-87de-760f-dc4775007ae0@gmail.com \
    --to=potin.lai.pt@gmail.com \
    --cc=andy.shevchenko@gmail.com \
    --cc=jic23@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patrick@stwcx.xyz \
    --cc=potin.lai@quantatw.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox