drivers/iio/adc/pac1921.c | 109 +++++++++++++++++++++++++++++++++----- 1 file changed, 96 insertions(+), 13 deletions(-)
From: Victor Duicu <victor.duicu@microchip.com>
This driver 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>
---
The patch was tested on minnowboard and sama5.
Differences related to previous versions:
v5:
- set maximum acceptable value of shunt resistor to 2KOHM in devicetree,
ACPI table and user input. The chosen value is lesser than INT_MAX,
which is about 2.1KOHM.
- in pac1921_match_acpi_device and pac1921_parse_of_fw change to only
read 32b values for resistor shunt.
v4:
- change name of pac1921_shunt_is_valid to pac1921_shunt_is_invalid.
- fix coding style.
- in pac1921_parse_of_fw change back to device_property_read_u32.
v3:
- simplify and make inline function pac1921_shunt_is_valid. Make argument u64.
- fix link to DSM documentation.
- in pac1921_match_acpi_device and pac1921_parse_of_fw, the shunt value is
read as u64.
- in pac1921_parse_of_fw remove code for reading label value from
devicetree.
- in pac1921_write_shunt_resistor cast the multiply result to u64 in order
to fix overflow.
v2:
- remove name variable from priv. Driver reads label attribute with
sysfs.
- define pac1921_shunt_is_valid function.
- move default assignments in pac1921_probe to original position.
- roll back coding style changes.
- add documentation for DSM(the linked document was used as reference).
- remove acpi_match_device in pac1921_match_acpi_device.
- remove unnecessary null assignment and comment.
- change name of function pac1921_match_of_device to
pac1921_parse_of_fw.
v1:
- initial version for review.
drivers/iio/adc/pac1921.c | 109 +++++++++++++++++++++++++++++++++-----
1 file changed, 96 insertions(+), 13 deletions(-)
diff --git a/drivers/iio/adc/pac1921.c b/drivers/iio/adc/pac1921.c
index a96fae546bc1..9622b0da6196 100644
--- a/drivers/iio/adc/pac1921.c
+++ b/drivers/iio/adc/pac1921.c
@@ -67,6 +67,13 @@ enum pac1921_mxsl {
#define PAC1921_DEFAULT_DI_GAIN 0 /* 2^(value): 1x gain (HW default) */
#define PAC1921_DEFAULT_NUM_SAMPLES 0 /* 2^(value): 1 sample (HW default) */
+#define PAC1921_ACPI_GET_UOHMS_VALS 0
+#define PAC1921_ACPI_GET_LABEL 1
+#define PAC1921_DSM_UUID "f7bb9932-86ee-4516-a236-7a7a742e55cb"
+/*The maximum accepted value of shunt_resistor is 2Kohms */
+#define PAC1921_MAX_SHUNT_VALUE_OHMS 2000
+#define PAC1921_MAX_SHUNT_VALUE_UOHMS 2000000000
+
/*
* Pre-computed scale factors for BUS voltage
* format: IIO_VAL_INT_PLUS_NANO
@@ -204,6 +211,11 @@ struct pac1921_priv {
} scan;
};
+static inline bool pac1921_shunt_is_invalid(u32 shunt_val)
+{
+ return (shunt_val == 0 || shunt_val > PAC1921_MAX_SHUNT_VALUE_UOHMS);
+}
+
/*
* Check if first integration after configuration update has completed.
*
@@ -781,7 +793,7 @@ static ssize_t pac1921_write_shunt_resistor(struct iio_dev *indio_dev,
const char *buf, size_t len)
{
struct pac1921_priv *priv = iio_priv(indio_dev);
- u64 rshunt_uohm;
+ u32 rshunt_uohm;
int val, val_fract;
int ret;
@@ -791,9 +803,13 @@ static ssize_t pac1921_write_shunt_resistor(struct iio_dev *indio_dev,
ret = iio_str_to_fixpoint(buf, 100000, &val, &val_fract);
if (ret)
return ret;
+
+ /* This check is to ensure val*MICRO won't overflow*/
+ if (val < 0 || val > PAC1921_MAX_SHUNT_VALUE_OHMS)
+ return -EINVAL;
rshunt_uohm = val * MICRO + val_fract;
- if (rshunt_uohm == 0 || rshunt_uohm > INT_MAX)
+ if (pac1921_shunt_is_invalid(rshunt_uohm))
return -EINVAL;
guard(mutex)(&priv->lock);
@@ -1150,6 +1166,71 @@ static void pac1921_regulator_disable(void *data)
regulator_disable(regulator);
}
+/*
+ * 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 i2c_client *client, struct pac1921_priv *priv,
+ struct iio_dev *indio_dev)
+{
+ acpi_handle handle;
+ union acpi_object *rez;
+ guid_t guid;
+ char *label;
+ u32 temp;
+
+ guid_parse(PAC1921_DSM_UUID, &guid);
+ handle = ACPI_HANDLE(&client->dev);
+
+ rez = acpi_evaluate_dsm(handle, &guid, 1, PAC1921_ACPI_GET_UOHMS_VALS, NULL);
+ if (!rez)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Could not read shunt from ACPI table\n");
+
+ temp = rez->package.elements[0].integer.value;
+ ACPI_FREE(rez);
+
+ if (pac1921_shunt_is_invalid(temp))
+ return dev_err_probe(&client->dev, -EINVAL, "Invalid shunt resistor\n");
+
+ priv->rshunt_uohm = temp;
+ pac1921_calc_current_scales(priv);
+
+ rez = acpi_evaluate_dsm(handle, &guid, 1, PAC1921_ACPI_GET_LABEL, NULL);
+ if (!rez)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Could not read label from ACPI table\n");
+
+ label = devm_kmemdup(&client->dev, rez->package.elements->string.pointer,
+ (size_t)rez->package.elements->string.length + 1,
+ GFP_KERNEL);
+ label[rez->package.elements->string.length] = '\0';
+ indio_dev->label = label;
+ ACPI_FREE(rez);
+
+ return 0;
+}
+
+static int pac1921_parse_of_fw(struct i2c_client *client, struct pac1921_priv *priv)
+{
+ int ret;
+ struct device *dev = &client->dev;
+
+ ret = device_property_read_u32(dev, "shunt-resistor-micro-ohms",
+ &priv->rshunt_uohm);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Cannot read shunt resistor property\n");
+
+ if (pac1921_shunt_is_invalid(priv->rshunt_uohm))
+ return dev_err_probe(dev, -EINVAL, "Invalid shunt resistor: %u\n",
+ priv->rshunt_uohm);
+
+ pac1921_calc_current_scales(priv);
+
+ return 0;
+}
+
static int pac1921_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
@@ -1176,17 +1257,13 @@ static int pac1921_probe(struct i2c_client *client)
priv->di_gain = PAC1921_DEFAULT_DI_GAIN;
priv->n_samples = PAC1921_DEFAULT_NUM_SAMPLES;
- ret = device_property_read_u32(dev, "shunt-resistor-micro-ohms",
- &priv->rshunt_uohm);
- if (ret)
- return dev_err_probe(dev, ret,
- "Cannot read shunt resistor property\n");
- if (priv->rshunt_uohm == 0 || priv->rshunt_uohm > INT_MAX)
- return dev_err_probe(dev, -EINVAL,
- "Invalid shunt resistor: %u\n",
- priv->rshunt_uohm);
-
- pac1921_calc_current_scales(priv);
+ if (ACPI_HANDLE(&client->dev))
+ ret = pac1921_match_acpi_device(client, priv, indio_dev);
+ else
+ ret = pac1921_parse_of_fw(client, priv);
+ if (ret < 0)
+ return dev_err_probe(&client->dev, ret,
+ "parameter parsing error\n");
priv->vdd = devm_regulator_get(dev, "vdd");
if (IS_ERR(priv->vdd))
@@ -1243,11 +1320,17 @@ static const struct of_device_id pac1921_of_match[] = {
};
MODULE_DEVICE_TABLE(of, pac1921_of_match);
+static const struct acpi_device_id pac1921_acpi_match[] = {
+ { "MCHP1921" },
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, pac1921_acpi_match);
static struct i2c_driver pac1921_driver = {
.driver = {
.name = "pac1921",
.pm = pm_sleep_ptr(&pac1921_pm_ops),
.of_match_table = pac1921_of_match,
+ .acpi_match_table = pac1921_acpi_match
},
.probe = pac1921_probe,
.id_table = pac1921_id,
base-commit: 57573ace0c1b142433dfe3d63ebf375269c80fc1
--
2.43.0Quoting victor.duicu@microchip.com (2024-10-18 14:06:24)
> From: Victor Duicu <victor.duicu@microchip.com>
>
> This driver 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>
> ---
>
> The patch was tested on minnowboard and sama5.
>
> Differences related to previous versions:
> v5:
I think v4 was cleaner. Are the following changes necessary?
> - set maximum acceptable value of shunt resistor to 2KOHM in devicetree,
> ACPI table and user input. The chosen value is lesser than INT_MAX,
> which is about 2.1KOHM.
I would personally keep INT_MAX since the limitation is only given by the types
used in the current conversions (see pac1921_calc_scale()) rather than being a
specific PAC1921 hardware limitation. Otherwise I would extend the comment on
top of those two definitions explaining why that's the maximum.
> - in pac1921_match_acpi_device and pac1921_parse_of_fw change to only
> read 32b values for resistor shunt.
>
I think that in pac1921_match_acpi_device(), the u64 temp var was introduced to
address the possible overflow coming from the u64 rez->package.elements[0].integer.value
and to safely perform a sanity check. I don't think we can trust blindly that
the ACPI table always has a valid 32bit value in that field.
pac1921_parse_of_fw() doesn't look changed compared to v4, you already switched
back to device_property_read_u32, if it's what you are referring to.
>
> v4:
> - change name of pac1921_shunt_is_valid to pac1921_shunt_is_invalid.
> - fix coding style.
> - in pac1921_parse_of_fw change back to device_property_read_u32.
>
> v3:
> - simplify and make inline function pac1921_shunt_is_valid. Make argument u64.
> - fix link to DSM documentation.
> - in pac1921_match_acpi_device and pac1921_parse_of_fw, the shunt value is
> read as u64.
> - in pac1921_parse_of_fw remove code for reading label value from
> devicetree.
> - in pac1921_write_shunt_resistor cast the multiply result to u64 in order
> to fix overflow.
>
> v2:
> - remove name variable from priv. Driver reads label attribute with
> sysfs.
> - define pac1921_shunt_is_valid function.
> - move default assignments in pac1921_probe to original position.
> - roll back coding style changes.
> - add documentation for DSM(the linked document was used as reference).
> - remove acpi_match_device in pac1921_match_acpi_device.
> - remove unnecessary null assignment and comment.
> - change name of function pac1921_match_of_device to
> pac1921_parse_of_fw.
>
> v1:
> - initial version for review.
>
> drivers/iio/adc/pac1921.c | 109 +++++++++++++++++++++++++++++++++-----
> 1 file changed, 96 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/iio/adc/pac1921.c b/drivers/iio/adc/pac1921.c
> index a96fae546bc1..9622b0da6196 100644
> --- a/drivers/iio/adc/pac1921.c
> +++ b/drivers/iio/adc/pac1921.c
> @@ -67,6 +67,13 @@ enum pac1921_mxsl {
> #define PAC1921_DEFAULT_DI_GAIN 0 /* 2^(value): 1x gain (HW default) */
> #define PAC1921_DEFAULT_NUM_SAMPLES 0 /* 2^(value): 1 sample (HW default) */
>
> +#define PAC1921_ACPI_GET_UOHMS_VALS 0
> +#define PAC1921_ACPI_GET_LABEL 1
> +#define PAC1921_DSM_UUID "f7bb9932-86ee-4516-a236-7a7a742e55cb"
> +/*The maximum accepted value of shunt_resistor is 2Kohms */
> +#define PAC1921_MAX_SHUNT_VALUE_OHMS 2000
> +#define PAC1921_MAX_SHUNT_VALUE_UOHMS 2000000000
> +
> /*
> * Pre-computed scale factors for BUS voltage
> * format: IIO_VAL_INT_PLUS_NANO
> @@ -204,6 +211,11 @@ struct pac1921_priv {
> } scan;
> };
>
> +static inline bool pac1921_shunt_is_invalid(u32 shunt_val)
> +{
> + return (shunt_val == 0 || shunt_val > PAC1921_MAX_SHUNT_VALUE_UOHMS);
> +}
> +
> /*
> * Check if first integration after configuration update has completed.
> *
> @@ -781,7 +793,7 @@ static ssize_t pac1921_write_shunt_resistor(struct iio_dev *indio_dev,
> const char *buf, size_t len)
> {
> struct pac1921_priv *priv = iio_priv(indio_dev);
> - u64 rshunt_uohm;
> + u32 rshunt_uohm;
> int val, val_fract;
> int ret;
>
> @@ -791,9 +803,13 @@ static ssize_t pac1921_write_shunt_resistor(struct iio_dev *indio_dev,
> ret = iio_str_to_fixpoint(buf, 100000, &val, &val_fract);
> if (ret)
> return ret;
> +
> + /* This check is to ensure val*MICRO won't overflow*/
> + if (val < 0 || val > PAC1921_MAX_SHUNT_VALUE_OHMS)
> + return -EINVAL;
>
> rshunt_uohm = val * MICRO + val_fract;
> - if (rshunt_uohm == 0 || rshunt_uohm > INT_MAX)
> + if (pac1921_shunt_is_invalid(rshunt_uohm))
> return -EINVAL;
>
In v3 you added the (u64)val cast to solve the multiplication overflow. Wasn't
that enough? However, if this is for better clarity I am fine with it.
> guard(mutex)(&priv->lock);
> @@ -1150,6 +1166,71 @@ static void pac1921_regulator_disable(void *data)
> regulator_disable(regulator);
> }
>
> +/*
> + * 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 i2c_client *client, struct pac1921_priv *priv,
> + struct iio_dev *indio_dev)
> +{
> + acpi_handle handle;
> + union acpi_object *rez;
> + guid_t guid;
> + char *label;
> + u32 temp;
> +
> + guid_parse(PAC1921_DSM_UUID, &guid);
> + handle = ACPI_HANDLE(&client->dev);
> +
> + rez = acpi_evaluate_dsm(handle, &guid, 1, PAC1921_ACPI_GET_UOHMS_VALS, NULL);
> + if (!rez)
> + return dev_err_probe(&client->dev, -EINVAL,
> + "Could not read shunt from ACPI table\n");
> +
> + temp = rez->package.elements[0].integer.value;
I don't think we can trust rez->package.elements[0].integer.value to always be
in 32bit boundaries. But if that's the case then the temp var looks unnecessary.
> + ACPI_FREE(rez);
> +
> + if (pac1921_shunt_is_invalid(temp))
> + return dev_err_probe(&client->dev, -EINVAL, "Invalid shunt resistor\n");
> +
> + priv->rshunt_uohm = temp;
> + pac1921_calc_current_scales(priv);
> +
> + rez = acpi_evaluate_dsm(handle, &guid, 1, PAC1921_ACPI_GET_LABEL, NULL);
> + if (!rez)
> + return dev_err_probe(&client->dev, -EINVAL,
> + "Could not read label from ACPI table\n");
> +
> + label = devm_kmemdup(&client->dev, rez->package.elements->string.pointer,
> + (size_t)rez->package.elements->string.length + 1,
> + GFP_KERNEL);
> + label[rez->package.elements->string.length] = '\0';
> + indio_dev->label = label;
> + ACPI_FREE(rez);
> +
> + return 0;
> +}
> +
> +static int pac1921_parse_of_fw(struct i2c_client *client, struct pac1921_priv *priv)
> +{
> + int ret;
> + struct device *dev = &client->dev;
> +
> + ret = device_property_read_u32(dev, "shunt-resistor-micro-ohms",
> + &priv->rshunt_uohm);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Cannot read shunt resistor property\n");
> +
> + if (pac1921_shunt_is_invalid(priv->rshunt_uohm))
> + return dev_err_probe(dev, -EINVAL, "Invalid shunt resistor: %u\n",
> + priv->rshunt_uohm);
> +
> + pac1921_calc_current_scales(priv);
> +
> + return 0;
> +}
> +
> static int pac1921_probe(struct i2c_client *client)
> {
> struct device *dev = &client->dev;
> @@ -1176,17 +1257,13 @@ static int pac1921_probe(struct i2c_client *client)
> priv->di_gain = PAC1921_DEFAULT_DI_GAIN;
> priv->n_samples = PAC1921_DEFAULT_NUM_SAMPLES;
>
> - ret = device_property_read_u32(dev, "shunt-resistor-micro-ohms",
> - &priv->rshunt_uohm);
> - if (ret)
> - return dev_err_probe(dev, ret,
> - "Cannot read shunt resistor property\n");
> - if (priv->rshunt_uohm == 0 || priv->rshunt_uohm > INT_MAX)
> - return dev_err_probe(dev, -EINVAL,
> - "Invalid shunt resistor: %u\n",
> - priv->rshunt_uohm);
> -
> - pac1921_calc_current_scales(priv);
> + if (ACPI_HANDLE(&client->dev))
> + ret = pac1921_match_acpi_device(client, priv, indio_dev);
> + else
> + ret = pac1921_parse_of_fw(client, priv);
> + if (ret < 0)
> + return dev_err_probe(&client->dev, ret,
> + "parameter parsing error\n");
>
> priv->vdd = devm_regulator_get(dev, "vdd");
> if (IS_ERR(priv->vdd))
> @@ -1243,11 +1320,17 @@ static const struct of_device_id pac1921_of_match[] = {
> };
> MODULE_DEVICE_TABLE(of, pac1921_of_match);
>
> +static const struct acpi_device_id pac1921_acpi_match[] = {
> + { "MCHP1921" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(acpi, pac1921_acpi_match);
> static struct i2c_driver pac1921_driver = {
> .driver = {
> .name = "pac1921",
> .pm = pm_sleep_ptr(&pac1921_pm_ops),
> .of_match_table = pac1921_of_match,
> + .acpi_match_table = pac1921_acpi_match
> },
> .probe = pac1921_probe,
> .id_table = pac1921_id,
>
> base-commit: 57573ace0c1b142433dfe3d63ebf375269c80fc1
> --
> 2.43.0
>
Best regards,
Matteo Martelli
On Fri, 2024-10-18 at 17:47 +0200, Matteo Martelli wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> Quoting victor.duicu@microchip.com (2024-10-18 14:06:24)
> > From: Victor Duicu <victor.duicu@microchip.com>
> >
> > This driver 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>
> > ---
> >
> > The patch was tested on minnowboard and sama5.
> >
> > Differences related to previous versions:
> > v5:
>
> I think v4 was cleaner. Are the following changes necessary?
>
> > - set maximum acceptable value of shunt resistor to 2KOHM in
> > devicetree,
> > ACPI table and user input. The chosen value is lesser than INT_MAX,
> > which is about 2.1KOHM.
>
> I would personally keep INT_MAX since the limitation is only given by
> the types
> used in the current conversions (see pac1921_calc_scale()) rather
> than being a
> specific PAC1921 hardware limitation. Otherwise I would extend the
> comment on
> top of those two definitions explaining why that's the maximum.
Will set maximum acceptable value to INT_MAX.
> > - in pac1921_match_acpi_device and pac1921_parse_of_fw change to
> > only
> > read 32b values for resistor shunt.
> >
>
> I think that in pac1921_match_acpi_device(), the u64 temp var was
> introduced to
> address the possible overflow coming from the u64 rez-
> >package.elements[0].integer.value
> and to safely perform a sanity check. I don't think we can trust
> blindly that
> the ACPI table always has a valid 32bit value in that field.
If the ACPI table has revision set to 1, all values higher than 32b
will be truncated to 32b.
> pac1921_parse_of_fw() doesn't look changed compared to v4, you
> already switched
> back to device_property_read_u32, if it's what you are referring to.
Yes, in v4 I changed pac1921_parse_of_fw and in v5 I changed
pac1921_match_acpi_device. I changed both back to 32b in order
to be consistent.
> >
> > v4:
> > - change name of pac1921_shunt_is_valid to
> > pac1921_shunt_is_invalid.
> > - fix coding style.
> > - in pac1921_parse_of_fw change back to device_property_read_u32.
> >
> >
> > drivers/iio/adc/pac1921.c | 109 +++++++++++++++++++++++++++++++++-
> > ----
> > 1 file changed, 96 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/iio/adc/pac1921.c b/drivers/iio/adc/pac1921.c
> > index a96fae546bc1..9622b0da6196 100644
> > --- a/drivers/iio/adc/pac1921.c
> > +++ b/drivers/iio/adc/pac1921.c
> > @@ -67,6 +67,13 @@ enum pac1921_mxsl {
> > #define PAC1921_DEFAULT_DI_GAIN 0 /* 2^(value): 1x
> > gain (HW default) */
> > #define PAC1921_DEFAULT_NUM_SAMPLES 0 /* 2^(value): 1 sample
> > (HW default) */
> >
> >
> >
> > @@ -791,9 +803,13 @@ static ssize_t
> > pac1921_write_shunt_resistor(struct iio_dev *indio_dev,
> > ret = iio_str_to_fixpoint(buf, 100000, &val, &val_fract);
> > if (ret)
> > return ret;
> > +
> > + /* This check is to ensure val*MICRO won't overflow*/
> > + if (val < 0 || val > PAC1921_MAX_SHUNT_VALUE_OHMS)
> > + return -EINVAL;
> >
> > rshunt_uohm = val * MICRO + val_fract;
> > - if (rshunt_uohm == 0 || rshunt_uohm > INT_MAX)
> > + if (pac1921_shunt_is_invalid(rshunt_uohm))
> > return -EINVAL;
> >
>
> In v3 you added the (u64)val cast to solve the multiplication
> overflow. Wasn't
> that enough? However, if this is for better clarity I am fine with
> it.
The cast to 64b was removed in order to be consistent
with dt and ACPI that will read only 32b values.
> > guard(mutex)(&priv->lock);
> > @@ -1150,6 +1166,71 @@ static void pac1921_regulator_disable(void
> > *data)
> > regulator_disable(regulator);
> > }
> >
> > +/*
> > + * 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 i2c_client *client,
> > struct pac1921_priv *priv,
> > + struct iio_dev *indio_dev)
> > +{
> > + acpi_handle handle;
> > + union acpi_object *rez;
> > + guid_t guid;
> > + char *label;
> > + u32 temp;
> > +
> > + guid_parse(PAC1921_DSM_UUID, &guid);
> > + handle = ACPI_HANDLE(&client->dev);
> > +
> > + rez = acpi_evaluate_dsm(handle, &guid, 1,
> > PAC1921_ACPI_GET_UOHMS_VALS, NULL);
> > + if (!rez)
> > + return dev_err_probe(&client->dev, -EINVAL,
> > + "Could not read shunt from
> > ACPI table\n");
> > +
> > + temp = rez->package.elements[0].integer.value;
>
> I don't think we can trust rez->package.elements[0].integer.value to
> always be
> in 32bit boundaries. But if that's the case then the temp var looks
> unnecessary.
Will remove the temp var in the next version.
> > + ACPI_FREE(rez);
> > +
> > + if (pac1921_shunt_is_invalid(temp))
> > + return dev_err_probe(&client->dev, -EINVAL,
> > "Invalid shunt resistor\n");
> > +
> > + priv->rshunt_uohm = temp;
> > + pac1921_calc_current_scales(priv);
> > +
> > + rez = acpi_evaluate_dsm(handle, &guid, 1,
> > PAC1921_ACPI_GET_LABEL, NULL);
> > + if (!rez)
> > + return dev_err_probe(&client->dev, -EINVAL,
> > + "Could not read label from
> > ACPI table\n");
> > +
> > + label = devm_kmemdup(&client->dev, rez->package.elements-
> > >string.pointer,
> > + (size_t)rez->package.elements-
> > >string.length + 1,
> > + GFP_KERNEL);
> > + label[rez->package.elements->string.length] = '\0';
> > + indio_dev->label = label;
> > + ACPI_FREE(rez);
> > +
> > + return 0;
> > +}
> >
> > --
> > 2.43.0
> >
>
> Best regards,
> Matteo Martelli
© 2016 - 2026 Red Hat, Inc.