[PATCH] iio: humidity: hdc100x: Add ACPI HID table

[PATCH] iio: humidity: hdc100x: Add ACPI HID table

[Kai-Heng Feng]

x86 boards may use ACPI HID "HDC1010" to for hdc100x device.

So add an ACPI match table for that accordingly.

Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 drivers/iio/humidity/hdc100x.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
index 9e0fce917ce4c..ad1dfac543c52 100644
--- a/drivers/iio/humidity/hdc100x.c
+++ b/drivers/iio/humidity/hdc100x.c
@@ -417,10 +417,18 @@ static const struct of_device_id hdc100x_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, hdc100x_dt_ids);
 
+static const struct acpi_device_id hdc100x_acpi_match[] = {
+	{"HDC1010"},
+	{ },
+};
+
+MODULE_DEVICE_TABLE(acpi, hdc100x_acpi_match);
+
 static struct i2c_driver hdc100x_driver = {
 	.driver = {
 		.name	= "hdc100x",
 		.of_match_table = hdc100x_dt_ids,
+		.acpi_match_table = ACPI_PTR(hdc100x_acpi_match),
 	},
 	.probe = hdc100x_probe,
 	.id_table = hdc100x_id,
-- 
2.33.1

Re: [PATCH] iio: humidity: hdc100x: Add ACPI HID table

[Matt Ranostay]

On Fri, Dec 24, 2021 at 6:59 AM Kai-Heng Feng
<kai.heng.feng@canonical.com> wrote:
>
> x86 boards may use ACPI HID "HDC1010" to for hdc100x device.
>
> So add an ACPI match table for that accordingly.
>

Acked-by: Matt Ranostay <matt.ranostay@konsulko.com>

> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
>  drivers/iio/humidity/hdc100x.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
> index 9e0fce917ce4c..ad1dfac543c52 100644
> --- a/drivers/iio/humidity/hdc100x.c
> +++ b/drivers/iio/humidity/hdc100x.c
> @@ -417,10 +417,18 @@ static const struct of_device_id hdc100x_dt_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(of, hdc100x_dt_ids);
>
> +static const struct acpi_device_id hdc100x_acpi_match[] = {
> +       {"HDC1010"},
> +       { },
> +};
> +
> +MODULE_DEVICE_TABLE(acpi, hdc100x_acpi_match);
> +
>  static struct i2c_driver hdc100x_driver = {
>         .driver = {
>                 .name   = "hdc100x",
>                 .of_match_table = hdc100x_dt_ids,
> +               .acpi_match_table = ACPI_PTR(hdc100x_acpi_match),
>         },
>         .probe = hdc100x_probe,
>         .id_table = hdc100x_id,
> --
> 2.33.1
>

Re: [PATCH] iio: humidity: hdc100x: Add ACPI HID table

[Kai-Heng Feng]

On Sat, Dec 25, 2021 at 7:32 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
>
>
> On Friday, December 24, 2021, Kai-Heng Feng <kai.heng.feng@canonical.com> wrote:
>>
>> x86 boards may use ACPI HID "HDC1010" to for hdc100x device.
>>
>> So add an ACPI match table for that accordingly.
>
>
> No. We do not add abusing IDs blindly.
> Why this is in use? Is the creative creator of that informed that is an ACPI spec abuse?

Can you please elaborate more on this topic? How is this an ACPI spec abuse?
I did suggest them to use PRP0001, but I also don't think this is an abuse.

> What devices are those? Is it available on the market? Where is the link to DSDT?

It's not on the market yet. Do you need the full DSDT? Or just the
part of the ACPI device?
I'll need approve from customer to disclose these info.

>
> Does TI has an ID for that?

I was told by customer this is approved by TI.

>
> NAK until all above is answered in the commit message and all parties are informed.

OK, I hope this can be resolved.

Kai-Heng

>
>
>>
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> ---
>>  drivers/iio/humidity/hdc100x.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
>> index 9e0fce917ce4c..ad1dfac543c52 100644
>> --- a/drivers/iio/humidity/hdc100x.c
>> +++ b/drivers/iio/humidity/hdc100x.c
>> @@ -417,10 +417,18 @@ static const struct of_device_id hdc100x_dt_ids[] = {
>>  };
>>  MODULE_DEVICE_TABLE(of, hdc100x_dt_ids);
>>
>> +static const struct acpi_device_id hdc100x_acpi_match[] = {
>> +       {"HDC1010"},
>> +       { },
>
>
> No comma.
>
>>
>> +};
>> +
>> +MODULE_DEVICE_TABLE(acpi, hdc100x_acpi_match);
>> +
>>  static struct i2c_driver hdc100x_driver = {
>>         .driver = {
>>                 .name   = "hdc100x",
>>                 .of_match_table = hdc100x_dt_ids,
>> +               .acpi_match_table = ACPI_PTR(hdc100x_acpi_match),
>
>
> Use of ACPI_PTR() is wrong here.
>
>>
>>         },
>>         .probe = hdc100x_probe,
>>         .id_table = hdc100x_id,
>> --
>> 2.33.1
>>
>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH] iio: humidity: hdc100x: Add ACPI HID table

[Andy Shevchenko]

On Sat, Dec 25, 2021 at 3:05 PM Kai-Heng Feng
<kai.heng.feng@canonical.com> wrote:
> On Sat, Dec 25, 2021 at 7:32 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Friday, December 24, 2021, Kai-Heng Feng <kai.heng.feng@canonical.com> wrote:
> >>
> >> x86 boards may use ACPI HID "HDC1010" to for hdc100x device.
> >>
> >> So add an ACPI match table for that accordingly.
> >
> > No. We do not add abusing IDs blindly.
> > Why this is in use? Is the creative creator of that informed that is an ACPI spec abuse?
>
> Can you please elaborate more on this topic? How is this an ACPI spec abuse?

Yes, https://uefi.org/PNP_ACPI_Registry has links to PNP and ACPI ID registries.
Note, the main differences between them are:
- PNP uses 3-letter vendor ID, ACPI uses 4-letter
- PNP is in maintenance mode and shouldn't be expanded

So, Since above should formally satisfy the PNP ID, the HDC is not a
TI vendor ID in the PNP ID registry.

The section in the spec that refers to it:
https://uefi.org/specs/ACPI/6.4/06_Device_Configuration/Device_Configuration.html#hid-hardware-id

> I did suggest them to use PRP0001, but I also don't think this is an abuse.

The PRP0001 is not an abuse per se, but rather it is highly _not_
recommended for a production use.

> > What devices are those? Is it available on the market? Where is the link to DSDT?
>
> It's not on the market yet.

Thank God!

> Do you need the full DSDT? Or just the
> part of the ACPI device?
> I'll need approve from customer to disclose these info.

This is a requirement only for the devices on the market when nobody
prevented the disaster from becoming... Since it's not yet, no need
for it.

> > Does TI has an ID for that?
>
> I was told by customer this is approved by TI.

How can TI approve that for "HARDCOM ELEKTRONIK & DATATEKNIK"? Is
there evidence that this company has been bought by TI or TI got full
rights on their IPs?

ACPI: TEXAS INSTRUMENTS TXNW
PNP: TEXAS INSTURMENTS TXN
(seems a typo in the company's name, but it should be notified to the
appropriate channels)

So, the ID should start with one of the above, and not HDC as far as I
understood.

PNP: TEXAS MICROSYSTEM TMI
This one probably is not related, but I'm not informed.

I would like you (and your customer) to point to my blog post on the
topic and it explains how the things should be done with the ACPI IDs:
https://andy-shev.dreamwidth.org/151340.html

Hope this will help and be resolved soon.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: humidity: hdc100x: Add ACPI HID table

[kernel test robot]

Hi Kai-Heng,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on v5.16-rc7 next-20211224]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Kai-Heng-Feng/iio-humidity-hdc100x-Add-ACPI-HID-table/20211224-230046
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: mips-randconfig-c004-20211227 (https://download.01.org/0day-ci/archive/20211227/202112271213.Z6jyi5YK-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 511726c64d3b6cca66f7c54d457d586aa3129f67)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install mips cross compiling tool for clang build
        # apt-get install binutils-mips-linux-gnu
        # https://github.com/0day-ci/linux/commit/59648fe59d706ab2b9e719418c490d505ee5cc93
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Kai-Heng-Feng/iio-humidity-hdc100x-Add-ACPI-HID-table/20211224-230046
        git checkout 59648fe59d706ab2b9e719418c490d505ee5cc93
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/iio/humidity/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/iio/humidity/hdc100x.c:420:36: warning: unused variable 'hdc100x_acpi_match' [-Wunused-const-variable]
   static const struct acpi_device_id hdc100x_acpi_match[] = {
                                      ^
   1 warning generated.


vim +/hdc100x_acpi_match +420 drivers/iio/humidity/hdc100x.c

   419	
 > 420	static const struct acpi_device_id hdc100x_acpi_match[] = {
   421		{"HDC1010"},
   422		{ },
   423	};
   424	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org