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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241103110127.6453b537@jic23-huawei>
Date: Sun, 3 Nov 2024 11:01:27 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: <victor.duicu@...rochip.com>
Cc: <matteomartelli3@...il.com>, <lars@...afoo.de>,
 <marius.cristea@...rochip.com>, <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, 2 Nov 2024 15:45:09 +0000
Jonathan Cameron <jic23@...nel.org> wrote:

> On Thu, 31 Oct 2024 08:52:05 +0200
> <victor.duicu@...rochip.com> wrote:
> 
> > From: Victor Duicu <victor.duicu@...rochip.com>
> > 
> > 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@...rochip.com>  
> 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;
> > +}  
> 
>