Re: [PATCH v2 2/2] hwmon: Add driver for TI INA232 Current and Power Monitor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/14/25 00:02, Leo Yang wrote:
Hi Guenter,

On Sat, Jan 11, 2025 at 12:22 AM Guenter Roeck <linux@xxxxxxxxxxxx> wrote:

+
+     /* If INA233 skips current/power, shunt-resistor and current-lsb aren't needed. */
+     /* read rshunt value (uOhm) */
+     if (of_property_read_u32(client->dev.of_node, "shunt-resistor", &rshunt) < 0)
+             rshunt = INA233_RSHUNT_DEFAULT;
+
+     /* read current_lsb value (uA) */
+     if (of_property_read_u16(client->dev.of_node, "ti,current-lsb", &current_lsb) < 0)
+             current_lsb = INA233_CURRENT_LSB_DEFAULT;
+
of_property_read_u16() returns -EOVERFLOW if the value provided was too large.
This should be checked to avoid situations where the value provided in devicetree
is too large.

Sorry I have a question, I can't get it to return -EOVERFLOW when I test it
I am using the following properties:
test16 = /bits/ 16 <0xfffd>;
of_property_read_u16 reads 0xfffd

test16o = /bits/ 16 <0xfffdd>;
of_property_read_u16 reads 0xffdd

test16o = <0xfffdd>;
of_property_read_u16 reads 0xf

test16array = /bits/ 16 <0xfffd 0xfffe>;
of_property_read_u16 reads 0xfffd

The same result for device_property_read_u16, it seems that a data
truncation occurs and none of them return -EOVERFLOW.

So maybe there is no need to check EOVERFLOW?
Or maybe we could use the minimum and maximum of the binding to
indicate the range.


of_property_read_u16() calls of_property_read_u16_array(). The API documentation
of of_property_read_u16_array() lists -ENODATA and -EOVERFLOW as possible error
return values, in addition to -EINVAL. Ignoring those errors is not a good idea,
even if it is not immediately obvious how to reproduce them.

Guenter





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux