On Fri, 11 Oct 2024 16:44:54 +0300
<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.
>
> The patch was tested on a minnowboard(64b) and sama5(32b).
> In order to avoid overflow when reading 64b values from ACPi table or
> devicetree it is necessary:
> - the revision of .dsl file must be 2 or greater to enable 64b arithmetic.
> - the shunt resistor variable in devicetree must have the prefix "/bits/ 64".
>
Change log should be below the --- as it doesn't want to end up in the git history.
> Differences related to previous versions:
> 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.
>
> Signed-off-by: Victor Duicu <victor.duicu@xxxxxxxxxxxxx>
A few comments in line.
J
int pac1921_parse_of_fw(struct i2c_client *client, struct pac1921_priv *priv,
> + struct iio_dev *indio_dev)
> +{
> + int ret;
> + struct device *dev = &client->dev;
> + u64 temp;
> +
> + ret = device_property_read_u64(dev, "shunt-resistor-micro-ohms", &temp);
> +
No line break here. Want to associate that error check with the line above
as clearly as possible.
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Cannot read shunt resistor property\n");
> +
> + if (pac1921_shunt_is_valid(temp))
> + return dev_err_probe(dev, -EINVAL, "Invalid shunt resistor: %u\n",
> + priv->rshunt_uohm);
> +
> + priv->rshunt_uohm = (u32)temp;
> + pac1921_calc_current_scales(priv);
> +
> + return 0;
> +}
