On 6/29/23 11:59, Dan Carpenter wrote:
On Thu, Jun 29, 2023 at 09:53:16AM +0300, Dan Carpenter wrote:
d2c6444389b625 Eddie James 2023-06-27 22 char out[8];
d2c6444389b625 Eddie James 2023-06-27 23 int rc;
d2c6444389b625 Eddie James 2023-06-27 24 int i;
d2c6444389b625 Eddie James 2023-06-27 25
d2c6444389b625 Eddie James 2023-06-27 26 rc = pmbus_lock_interruptible(client);
d2c6444389b625 Eddie James 2023-06-27 27 if (rc)
d2c6444389b625 Eddie James 2023-06-27 28 return rc;
d2c6444389b625 Eddie James 2023-06-27 29
d2c6444389b625 Eddie James 2023-06-27 30 rc = i2c_smbus_read_block_data(client, ACBEL_MFR_FW_REVISION, data);
d2c6444389b625 Eddie James 2023-06-27 31 pmbus_unlock(client);
d2c6444389b625 Eddie James 2023-06-27 32 if (rc < 0)
d2c6444389b625 Eddie James 2023-06-27 33 return rc;
d2c6444389b625 Eddie James 2023-06-27 34
d2c6444389b625 Eddie James 2023-06-27 35 for (i = 0; i < rc && i < 3; ++i)
d2c6444389b625 Eddie James 2023-06-27 @36 snprintf(&out[i * 2], 3, "%02X", data[i]);
If data[i] is negative this will print FFFFFFF1 etc. (This is an x86
config... Did we ever merge that patch to make char signed by default?)
I meant unsigned not signed. But actually we debated both ways...
Signed by default would annoy PowerPC devs since they try to really
lean into the fact that char is unsigned on that arch. :P
https://lwn.net/Articles/911914/
As if anything would be easy nowadays ;-). Anyway, in this case, the array
should be explicitly unsigned, so changing the type to u8 was the right
thing to do. Also, the driver should be usable on non-Intel systems,
which is another reason to make the type sign-specific (even more so in
the context of the above discussion).
Thanks,
Guenter