Hi Mauro, After many attempts ret was always 0. Please let me know if further testing is needed. Best regards, Derek On Mon, Nov 30, 2020 at 9:09 AM Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> wrote: > > Hi Derek, > > Em Mon, 30 Nov 2020 08:04:31 -0800 > VDRU VDRU <user.vdr@xxxxxxxxx> escreveu: > > > I have hardware that uses this driver and can conduct a test if it > > will help resolve any confusion/assumption. I'd also like to suggest > > that making changes to drivers with no means of testing those changes > > is bad. This has happened in the past and resulted in unnecessarily > > breaking drivers for those who use it. No patch should be merged > > without testing! > > It helps a lot if you could test it. > > The current situation is that, if the I2C read fails, the > driver will randomly power up only partially, which could > cause issues. > > The original proposed approach: > > https://lore.kernel.org/linux-media/20190627222020.45909-1-willemdebruijn.kernel@xxxxxxxxx/ > > Will just give up trying to powering it up, while the > patch I'm proposing will force the device to power up > all parts of it. So, it seems safer than the original > one. > > Please test with the enclosed patch. It is basically > the same as the one I proposed, although this one will > print a message at dlog, due to this: > > pr_info("ret returned %d\n", ret); > > This could happen when the device got plugged and/or if > you put the machine into suspend mode, when resuming it > while streaming[1] > > Regards, > Mauro > > [1] not sure if dvb-usb supports it. One of the rationales > behind dvb-usb-v2 were to be able of properly do > suspend/resumes. > > > > diff --git a/drivers/media/usb/dvb-usb/gp8psk.c b/drivers/media/usb/dvb-usb/gp8psk.c > index c07f46f5176e..be55496cc717 100644 > --- a/drivers/media/usb/dvb-usb/gp8psk.c > +++ b/drivers/media/usb/dvb-usb/gp8psk.c > @@ -182,11 +182,16 @@ static int gp8psk_load_bcm4500fw(struct dvb_usb_device *d) > > static int gp8psk_power_ctrl(struct dvb_usb_device *d, int onoff) > { > - u8 status, buf; > + u8 status = 0, buf; > + int ret; > int gp_product_id = le16_to_cpu(d->udev->descriptor.idProduct); > > if (onoff) { > - gp8psk_usb_in_op(d, GET_8PSK_CONFIG,0,0,&status,1); > + ret = gp8psk_usb_in_op(d, GET_8PSK_CONFIG,0,0,&status,1); > + // Just to check if the condition happens in practice > + if (ret < 0) > + pr_info("ret returned %d\n", ret); > + > if (! (status & bm8pskStarted)) { /* started */ > if(gp_product_id == USB_PID_GENPIX_SKYWALKER_CW3K) > gp8psk_usb_out_op(d, CW3K_INIT, 1, 0, NULL, 0); >
