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

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 31 Oct 2024 08:52:05 +0200
<victor.duicu@microchip.com> wrote:

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

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;
> +}