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

[<Victor.Duicu@microchip.com>]

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@microchip.com kirjoitti:
> > From: Victor Duicu <victor.duicu@microchip.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