Re: [PATCH v3 3/4] leds: pca955x: add GPIO support

On 08/24/2017 10:03 PM, Jacek Anaszewski wrote:

Hi Cedric,

On 08/24/2017 01:30 PM, Cédric Le Goater wrote:

quoted

On 08/12/2017 10:42 AM, Pavel Machek wrote:

quoted

Hi!

quoted

quoted

quoted

quoted

quoted

+static u8 pca955x_read_input(struct i2c_client *client, int n)
+{
+	return (u8)i2c_smbus_read_byte_data(client, n);
+}

Is the cast needed? Should you attempt to handle errors?

ah. this is a left over. I can fix in a resend.

No need to resend just for this.

Should you WARN_ON() if the read_byte_data returns < 0 or something?

https://linuxtv.org/downloads/v4l-dvb-internals/device-drivers/API-i2c-smbus-read-byte-data.html

I don't think so as this is just to read the gpio value. 

I suppose that the i2c layer will output some errors before 
if it catches some invalid state.

Normally its driver's job to output errors. (Same for write).

ok. I can respin the patchset with an extra patch adding checks
for I2C errors (see below). In which order would you want that, 
before the gpio support or after ? 

I'd prefer an incremental patch. The patch set has received 10
days of testing and the merge window is imminent. Not the best
moment to respin the for-next branch.

OK. The patch applies on top of the patchset. I will give it some 
more testing before sending.

Thanks,

C.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html