Message ID | 4DC714EC.2060606@linuxtv.org (mailing list archive) |
---|---|
State | Accepted, archived |
Headers |
Return-path: <mchehab@gaivota> Envelope-to: mchehab@gaivota Delivery-date: Sun, 08 May 2011 23:26:59 -0300 Received: from mchehab by gaivota with local (Exim 4.73) (envelope-from <mchehab@gaivota>) id 1QJGBX-0001g7-Fn for mchehab@gaivota; Sun, 08 May 2011 23:26:59 -0300 Received: from casper.infradead.org [85.118.1.10] by gaivota with IMAP (fetchmail-6.3.19) for <mchehab@localhost> (single-drop); Sun, 08 May 2011 23:26:59 -0300 (BRT) Received: from vger.kernel.org ([209.132.180.67]) by casper.infradead.org with esmtp (Exim 4.72 #1 (Red Hat Linux)) id 1QJCBv-0007sv-EO; Sun, 08 May 2011 22:11:07 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754522Ab1EHWK4 (ORCPT <rfc822; kmpark@infradead.org> + 1 other); Sun, 8 May 2011 18:10:56 -0400 Received: from ffm.saftware.de ([83.141.3.46]:51431 "EHLO ffm.saftware.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751617Ab1EHWK4 (ORCPT <rfc822;linux-media@vger.kernel.org>); Sun, 8 May 2011 18:10:56 -0400 Received: from localhost (localhost [127.0.0.1]) by ffm.saftware.de (Postfix) with ESMTP id 5485D2C0A31; Mon, 9 May 2011 00:10:55 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at ffm.saftware.de Received: from ffm.saftware.de ([127.0.0.1]) by localhost (ffm.saftware.de [127.0.0.1]) (amavisd-new, port 10024) with LMTP id P59TnbTs8NEO; Mon, 9 May 2011 00:10:54 +0200 (CEST) Message-ID: <4DC714EC.2060606@linuxtv.org> Date: Mon, 09 May 2011 00:10:52 +0200 From: Andreas Oberritter <obi@linuxtv.org> User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.17) Gecko/20110424 Lightning/1.0b2 Thunderbird/3.1.10 MIME-Version: 1.0 To: Steve Kerrison <steve@stevekerrison.com> CC: Antti Palosaari <crope@iki.fi>, Mauro Carvalho Chehab <mchehab@redhat.com>, linux-media@vger.kernel.org Subject: Re: [PATCH v2 2/5] drxd: Fix warning caused by new entries in an enum References: <4DC6BF28.8070006@redhat.com> <1304882240-23044-3-git-send-email-steve@stevekerrison.com> In-Reply-To: <1304882240-23044-3-git-send-email-steve@stevekerrison.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org Sender: Mauro Carvalho Chehab <mchehab@gaivota> |
Commit Message
Andreas Oberritter
May 8, 2011, 10:10 p.m. UTC
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 <steve@stevekerrison.com> > --- > 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 <obi@linuxtv.org> 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 <obi@linuxtv.org> --- drivers/media/dvb/frontends/drxd_hard.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
Comments
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,
Em 09-05-2011 11:15, Steve Kerrison escreveu: > 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. Yes, there is. Linux defines a macro for it: uninitialized_var() (it basically will do status = status internally with newer gcc versions, but it helps to document what's happening there) it is sometimes better to initialize the var, as the warning may help to detect troubles after some changes. Cheers, Mauro. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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)