linux-kernel - Re: [PATCH v7] iio: adc: pac1921: Add ACPI support to Microchip pac1921

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] [day] [month] [year] [list]

Message-ID: <11fc5bd7fcf5a4bb674ed167fbc9e2ce8f6cd565.camel@microchip.com>
Date: Tue, 5 Nov 2024 08:31:53 +0000
From: <Victor.Duicu@...rochip.com>
To: <andy.shevchenko@...il.com>
CC: <Marius.Cristea@...rochip.com>, <jic23@...nel.org>,
	<matteomartelli3@...il.com>, <lars@...afoo.de>, <linux-iio@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v7] iio: adc: pac1921: Add ACPI support to Microchip
 pac1921

On Sat, 2024-11-02 at 21:48 +0200, Andy Shevchenko wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> Thu, Oct 31, 2024 at 08:52:05AM +0200,
> victor.duicu@...rochip.com kirjoitti:
> > From: Victor Duicu <victor.duicu@...rochip.com>

Hi Andy,

> > 
> > This patch implements ACPI support to Microchip pac1921.
> > The driver can read shunt resistor value and label from ACPI table.
> 
> This ID might be okay, but can we please have:
> 1) the list of the models (or a model) of the device on the market
> that has this;
> 2) ACPI DSDT excerpt of the respective Device object?
> 
> (I mean a laptop, tablet, phone or other device that has this sensor
> described
> in the ACPI)

We do not have a list of end-use devices for pac1921.

> 
> ...
> 
> > +/*
> > + * documentation related to the ACPI device definition
> 
> Documentation
> 
> > + *
> > https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ApplicationNotes/ApplicationNotes/PAC193X-Integration-Notes-for-Microsoft-Windows-10-and-Windows-11-Driver-Support-DS00002534.pdf
> > + */
> 
> ...
> 
> > +     if (ACPI_HANDLE(dev))
> 
> Hmm... Want this be really needed? You can try to call DSM. and if it
> fails try
> DT (or actually other way around as we usually do).
> 
> > +             ret = pac1921_match_acpi_device(indio_dev);
> > +     else
> > +             ret = pac1921_parse_of_fw(indio_dev);

My approach is to cleanly separate the code for dt and ACPI and to
allow flexibility for future additions.

> 
> ...
> 
> > +static const struct acpi_device_id pac1921_acpi_match[] = {
> > +     { "MCHP1921" },
> > +     { }
> > +};
> > +MODULE_DEVICE_TABLE(acpi, pac1921_acpi_match);
> 
> Missing blank line here.
> 
> ...
> 
> >       .driver  = {
> >               .name = "pac1921",
> >               .pm = pm_sleep_ptr(&pac1921_pm_ops),
> >               .of_match_table = pac1921_of_match,
> > +             .acpi_match_table = pac1921_acpi_match
> 
> 
> Missing trailing comma here.
> 
> >       },
> 
> --
> With Best Regards,
> Andy Shevchenko
> 
> 

Best Regards,
Victor Duicu