Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753283AbbDHLJN (ORCPT ); Wed, 8 Apr 2015 07:09:13 -0400 Received: from lists.s-osg.org ([54.187.51.154]:52133 "EHLO lists.s-osg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751330AbbDHLJK (ORCPT ); Wed, 8 Apr 2015 07:09:10 -0400 Date: Wed, 8 Apr 2015 08:09:03 -0300 From: Mauro Carvalho Chehab To: luisbg Cc: linux-media@vger.kernel.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org, gregkh@linuxfoundation.org Subject: Re: [PATCH] media: cxd2099: move pre-init values out of init() Message-ID: <20150408080903.1fdc7c4e@recife.lan> In-Reply-To: <20150208205536.GA31543@turing> References: <20150208205536.GA31543@turing> Organization: Samsung X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.27; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1877 Lines: 60 Em Sun, 8 Feb 2015 20:55:36 +0000 luisbg escreveu: > Improve code readability by moving out all pre-init values from the init > function. > > Signed-off-by: Luis de Bethencourt > --- > drivers/staging/media/cxd2099/cxd2099.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/media/cxd2099/cxd2099.c b/drivers/staging/media/cxd2099/cxd2099.c > index 657ea48..bafe36f 100644 > --- a/drivers/staging/media/cxd2099/cxd2099.c > +++ b/drivers/staging/media/cxd2099/cxd2099.c > @@ -300,7 +300,6 @@ static int init(struct cxd *ci) > int status; > > mutex_lock(&ci->lock); > - ci->mode = -1; > do { > status = write_reg(ci, 0x00, 0x00); > if (status < 0) > @@ -420,7 +419,6 @@ static int init(struct cxd *ci) > status = write_regm(ci, 0x09, 0x08, 0x08); > if (status < 0) > break; > - ci->cammode = -1; > cam_mode(ci, 0); > } while (0); > mutex_unlock(&ci->lock); > @@ -711,6 +709,8 @@ struct dvb_ca_en50221 *cxd2099_attach(struct cxd2099_cfg *cfg, > > ci->en = en_templ; > ci->en.data = ci; > + ci->mode = -1; > + ci->cammode = -1; This actually changes the logic, as, cammode is == -1 only if the do {} while loop succeeds. Also, calling cam_mode(ci, 0) will change cammode to 0. Btw, for it to work, ci->mode should be initialized earlier. So, this patch looks very wrong on my eyes, except if you found a real bug on it. Have you tested it on a real device? What bug does it fix? Regards, Mauro > init(ci); > dev_info(&i2c->dev, "Attached CXD2099AR at %02x\n", ci->cfg.adr); > return &ci->en; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/