Re: [PATCH] USB: serial: cp210x: Fix GPIO in autosuspend

[Karoly Pados]

Uhm, yes, sorry, it kind of went out of my head. I am doing a lot of travelling lately (in the
past 48 hours I've been on 3 airplanes), and I had travels on earlier weeks too, mixed with
some project releases and family celebrations. So my head is just somewhere else.
Realistically, I'll submit something by the weekend when I'm home again.

Karoly

February 4, 2019 5:09 PM, "Johan Hovold" <johan@xxxxxxxxxx> wrote:

> On Tue, Jan 15, 2019 at 11:29:42AM +0100, Johan Hovold wrote:
> 
>> On Tue, Jan 15, 2019 at 10:26:07AM +0100, Johan Hovold wrote:
>> On Tue, Jan 15, 2019 at 09:17:58AM +0000, Karoly Pados wrote:
>>>> I think it's better to add the autopm call to gpio210x_gpio_get/set
>>>> only. This will allow for a simpler patch, and keeps the autopm handling
>>>> confined to the gpio paths.
>>> 
>>> I'll submit a v2.
>>> 
>>>>> @@ -1383,6 +1397,7 @@ static void cp210x_gpio_set(struct gpio_chip *gc, unsigned int gpio, int
>>>>> value)
>>>>> } else {
>>>>> u16 wIndex = buf.state << 8 | buf.mask;
>>>>> 
>>>>> + usb_autopm_get_interface(serial->interface);
>>>> 
>>>> Also make sure to always check for errors from autopm_get().
>>> 
>>> I checked everywhere else, the reason I didn't check here is on
>>> purpose based on your previous feedback. The caller function here
>>> doesn't have a return value, so the only way to return errors is to
>>> log, but in my last patch to ftdi_sio you made clear that errors from
>>> autopm_get shouldn't get logged. Trying to call usb_control_msg() even
>>> though the device could not wake does not cause issues, and the return
>>> value from usb_control_msg() clearly identifies the reason for failure
>>> (failure due to autosuspend), so error information is not lost either.
>>> So I thought not checking here has no real disadvantage and I still
>>> stay conformant to your previous guidance.
>> 
>> Ok, I understand your reasoning, but please do check for errors and bail
>> out early if autopm_get() fails. No need to log errors.
>> 
>> Actually, we should probably add the missing error handling to the
>> callers and have gpio_set() propagate errors too. If you want to take a
>> stab at that, that could be a follow-on patch.
> 
> Karoly, did you plan on sending a v2 of this one?
> 
> Thanks,
> Johan