Hi Andreas,

> I'd prefer returning -EINVAL for unsupported parameters.
>
> [snip]
> 
> I already had a patch for this, but forgot to submit it together with
> the frontend.h bits.

That seems reasonable. Do I need to do anything with this? I'm happy for
Mauro to scrub my drxd and mxl patches and use yours instead.

> Btw., "status = status;" looks odd.

Heh, yes it does. I wonder if that was put in to deal with an "unused
variable" compiler warning before the switch statement had a default
case? Otherwise, perhaps it's from the department of redundancy
department.

Regards,
-- 
Steve Kerrison MEng Hons.
http://www.stevekerrison.com/ 

On Mon, 2011-05-09 at 00:10 +0200, Andreas Oberritter wrote:
> On 05/08/2011 09:17 PM, Steve Kerrison wrote:
> > Additional bandwidth modes have been added in frontend.h
> > drxd_hard.c had no default case so the compiler was warning about
> > a non-exhausive switch statement.
> > 
> > This has been fixed by making the default behaviour the same as
> > BANDWIDTH_AUTO, with the addition of a printk to notify if this
> > ever happens.
> > 
> > Signed-off-by: Steve Kerrison <[email protected]>
> > ---
> >  drivers/media/dvb/frontends/drxd_hard.c |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/media/dvb/frontends/drxd_hard.c 
> > b/drivers/media/dvb/frontends/drxd_hard.c
> > index 30a78af..b3b0704 100644
> > --- a/drivers/media/dvb/frontends/drxd_hard.c
> > +++ b/drivers/media/dvb/frontends/drxd_hard.c
> > @@ -2325,6 +2325,10 @@ static int DRX_Start(struct drxd_state *state, s32 
> > off)
> >                InitEC and ResetEC
> >                functions */
> >             switch (p->bandwidth) {
> > +           default:
> > +                   printk(KERN_INFO "drxd: Unsupported bandwidth mode %u, 
> > reverting to default\n",
> > +                           p->bandwidth);
> > +                   /* Fall back to auto */
> 
> I'd prefer returning -EINVAL for unsupported parameters.
> 
> >             case BANDWIDTH_AUTO:
> >             case BANDWIDTH_8_MHZ:
> >                     /* (64/7)*(8/8)*1000000 */
> 
> I already had a patch for this, but forgot to submit it together with the 
> frontend.h bits.
> 
> From 73d630b57f584d7e35cac5e27149cbc564aedde2 Mon Sep 17 00:00:00 2001
> From: Andreas Oberritter <[email protected]>
> Date: Fri, 8 Apr 2011 16:39:20 +0000
> Subject: [PATCH 2/2] DVB: drxd_hard: handle new bandwidths by returning 
> -EINVAL
> 
> Signed-off-by: Andreas Oberritter <[email protected]>
> ---
>  drivers/media/dvb/frontends/drxd_hard.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/dvb/frontends/drxd_hard.c 
> b/drivers/media/dvb/frontends/drxd_hard.c
> index 30a78af..53319f4 100644
> --- a/drivers/media/dvb/frontends/drxd_hard.c
> +++ b/drivers/media/dvb/frontends/drxd_hard.c
> @@ -2348,6 +2348,9 @@ static int DRX_Start(struct drxd_state *state, s32 off)
>                       status = Write16(state,
>                                        FE_AG_REG_IND_DEL__A, 71, 0x0000);
>                       break;
> +             default:
> +                     status = -EINVAL;
> +                     break;
>               }
>               status = status;
>               if (status < 0)

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to