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