On Sat, 2 Nov 2024 15:45:09 +0000 Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > On Thu, 31 Oct 2024 08:52:05 +0200 > <victor.duicu@xxxxxxxxxxxxx> wrote: > > > From: Victor Duicu <victor.duicu@xxxxxxxxxxxxx> > > > > This patch implements ACPI support to Microchip pac1921. > > The driver can read shunt resistor value and label from ACPI table. > > > > Signed-off-by: Victor Duicu <victor.duicu@xxxxxxxxxxxxx> > Hi Victor. > > I made a few minor tweaks whilst applying (see inline) > > Applied to the togreg branch of iio.git and pushed out as testing for now > so that the bots can take a look and see if we missed anything. Backed out again so you can resolve Andy's comments. Jonathan > > Jonathan > > > +/* > > + * documentation related to the ACPI device definition > > + * https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ApplicationNotes/ApplicationNotes/PAC193X-Integration-Notes-for-Microsoft-Windows-10-and-Windows-11-Driver-Support-DS00002534.pdf > > + */ > > +static int pac1921_match_acpi_device(struct iio_dev *indio_dev) > > +{ > > + acpi_handle handle; > > + union acpi_object *rez; > > + guid_t guid; > > + char *label; > > + struct pac1921_priv *priv = iio_priv(indio_dev); > > + struct device *dev = &priv->client->dev; > > + > > + guid_parse(PAC1921_DSM_UUID, &guid); > > + handle = ACPI_HANDLE(dev); > > + > > + rez = acpi_evaluate_dsm(handle, &guid, 1, PAC1921_ACPI_GET_UOHMS_VALS, NULL); > > + if (!rez) > > + return dev_err_probe(dev, -EINVAL, > > + "Could not read shunt from ACPI table\n"); > > + > > + priv->rshunt_uohm = rez->package.elements[0].integer.value; > > + ACPI_FREE(rez); > > + > > + rez = acpi_evaluate_dsm(handle, &guid, 1, PAC1921_ACPI_GET_LABEL, NULL); > > + if (!rez) > > + return dev_err_probe(dev, -EINVAL, > > + "Could not read label from ACPI table\n"); > > + > > + label = devm_kstrdup(dev, rez->package.elements->string.pointer, GFP_KERNEL); > > + if (!label) > > + return dev_err_probe(dev, -EINVAL, "Label is NULL\n"); > ENOMEM appropriate I think. > > Also, a package is an array of elements, and this is the first one so > maybe res->package.elements[0].string.pointer is more appropriate? > (similar to above). > > > > + > > + indio_dev->label = label; > > + ACPI_FREE(rez); > > + > > + return 0; > > +} > >