Re: [PATCH 1/4] [media] tw5864, fc0011: better handle WARN_ON()

2017-05-19 Thread Anton Sviridenko
ok, I'll check this patch on tw5864 hardware bit later  and report results here

On Thu, May 18, 2017 at 10:48 PM, Andrey Utkin
 wrote:
> On Thu, May 18, 2017 at 11:06:43AM -0300, Mauro Carvalho Chehab wrote:
>> As such macro will check if the expression is true, it may fall through, as
>> warned:
>
>> On both cases, it means an error, so, let's return an error
>> code, to make gcc happy.
>>
>> Signed-off-by: Mauro Carvalho Chehab 
>> ---
>>  drivers/media/pci/tw5864/tw5864-video.c | 1 +
>>  drivers/media/tuners/fc0011.c   | 1 +
>>  2 files changed, 2 insertions(+)
>>
>> diff --git a/drivers/media/pci/tw5864/tw5864-video.c 
>> b/drivers/media/pci/tw5864/tw5864-video.c
>> index 2a044be729da..e7bd2b8484e3 100644
>> --- a/drivers/media/pci/tw5864/tw5864-video.c
>> +++ b/drivers/media/pci/tw5864/tw5864-video.c
>> @@ -545,6 +545,7 @@ static int tw5864_fmt_vid_cap(struct file *file, void 
>> *priv,
>>   switch (input->std) {
>>   default:
>>   WARN_ON_ONCE(1);
>> + return -EINVAL;
>>   case STD_NTSC:
>>   f->fmt.pix.height = 480;
>>   break;
>
> Hi Mauro,
>
> Thanks for the patch.
>
> I actually meant it to fall through, but I agree this is not how it
> should be.
> I'm fine with this patch. Unfortunately I don't possess tw5864 hardware
> now. CCing Anton Sviridenko whom I've handed it (I guess he's on
> Bluecherry Maintainers groupmail as well).
>
> Anton, just in case, could you please ensure the driver with this patch
> works well in runtime?


Re: [PATCH 1/4] [media] tw5864, fc0011: better handle WARN_ON()

2017-05-18 Thread Michael Büsch
On Thu, 18 May 2017 11:06:43 -0300
Mauro Carvalho Chehab  wrote:

> diff --git a/drivers/media/tuners/fc0011.c b/drivers/media/tuners/fc0011.c
> index 192b1c7740df..145407dee3db 100644
> --- a/drivers/media/tuners/fc0011.c
> +++ b/drivers/media/tuners/fc0011.c
> @@ -342,6 +342,7 @@ static int fc0011_set_params(struct dvb_frontend *fe)
>   switch (vco_sel) {
>   default:
>   WARN_ON(1);
> + return -EINVAL;
>   case 0:
>   if (vco_cal < 8) {
>   regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | 
> FC11_VCOSEL_2);

This fall through is intentional, but I guess returning an error is OK,
too. This should not happen anyway.
I cannot test this, though.

Acked-by: Michael Büsch 

-- 
Michael


pgpfAl6Qz6id8.pgp
Description: OpenPGP digital signature


Re: [PATCH 1/4] [media] tw5864, fc0011: better handle WARN_ON()

2017-05-18 Thread Andrey Utkin
On Thu, May 18, 2017 at 11:06:43AM -0300, Mauro Carvalho Chehab wrote:
> As such macro will check if the expression is true, it may fall through, as
> warned:

> On both cases, it means an error, so, let's return an error
> code, to make gcc happy.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/pci/tw5864/tw5864-video.c | 1 +
>  drivers/media/tuners/fc0011.c   | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/media/pci/tw5864/tw5864-video.c 
> b/drivers/media/pci/tw5864/tw5864-video.c
> index 2a044be729da..e7bd2b8484e3 100644
> --- a/drivers/media/pci/tw5864/tw5864-video.c
> +++ b/drivers/media/pci/tw5864/tw5864-video.c
> @@ -545,6 +545,7 @@ static int tw5864_fmt_vid_cap(struct file *file, void 
> *priv,
>   switch (input->std) {
>   default:
>   WARN_ON_ONCE(1);
> + return -EINVAL;
>   case STD_NTSC:
>   f->fmt.pix.height = 480;
>   break;

Hi Mauro,

Thanks for the patch.

I actually meant it to fall through, but I agree this is not how it
should be.
I'm fine with this patch. Unfortunately I don't possess tw5864 hardware
now. CCing Anton Sviridenko whom I've handed it (I guess he's on
Bluecherry Maintainers groupmail as well).

Anton, just in case, could you please ensure the driver with this patch
works well in runtime?