Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <d0ac1a26ed5943127cb0156148735f5f52a07075.1606459576.git.mchehab+huawei@kernel.org>
 <CA+FuTSenOoVxM6W9viwXQmPHo_MEoQzQ=GPxJi72fYGHHmqmwA@mail.gmail.com>
 <20201130104420.321531ec@coco.lan> <CAA7C2qiOAZR+QwY5Bs-UHQzBEfA15gMG-GjriqNo3Q5biY4+ZQ@mail.gmail.com>
 <20201130180834.07a3116f@coco.lan>
In-Reply-To: <20201130180834.07a3116f@coco.lan>
From:   VDRU VDRU <user.vdr@gmail.com>
Date:   Mon, 30 Nov 2020 18:07:37 -0800
Message-ID: <CAA7C2qgAC8vnxu4xVhdi2CsMXyx85J44yUxGk00JhoQ6j5U3tw@mail.gmail.com>
Subject: Re: [PATCH] media: gp8psk: initialize stats at power control logic
To:     Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Cc:     Willem de Bruijn <willemdebruijn.kernel@gmail.com>,
        Linux Media Mailing List <linux-media@vger.kernel.org>,
        linuxarm@huawei.com, mauro.chehab@huawei.com,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        syzbot <syzkaller@googlegroups.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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@kernel.org> wrote:
>
> Hi Derek,
>
> Em Mon, 30 Nov 2020 08:04:31 -0800
> VDRU VDRU <user.vdr@gmail.com> 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@gmail.com/
>
> 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);
>