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

[Jacek Anaszewski <jacek.anaszewski@gmail.com>]

Hi Cedric,

On 08/24/2017 01:30 PM, Cédric Le Goater wrote:
> On 08/12/2017 10:42 AM, Pavel Machek wrote:
>> Hi!
>>
>>>>>>> +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.

-- 
Best regards,
Jacek Anaszewski