From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jarod Wilson Subject: Re: [PATCH 3/5] IR: ene_ir: few bugfixes Date: Fri, 15 Oct 2010 17:00:02 -0400 Message-ID: <20101015210002.GP9658@redhat.com> References: <1287158799-21486-1-git-send-email-maximlevitsky@gmail.com> <1287158799-21486-4-git-send-email-maximlevitsky@gmail.com> <20101015200212.GK9658@redhat.com> <1287174295.1867.1.camel@MAIN> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <1287174295.1867.1.camel@MAIN> Sender: linux-media-owner@vger.kernel.org To: Maxim Levitsky Cc: lirc-list@lists.sourceforge.net, Jarod Wilson , David =?iso-8859-1?Q?H=E4rdeman?= , mchehab@infradead.org, linux-input@vger.kernel.org, linux-media@vger.kernel.org List-Id: linux-input@vger.kernel.org On Fri, Oct 15, 2010 at 10:24:55PM +0200, Maxim Levitsky wrote: > On Fri, 2010-10-15 at 16:02 -0400, Jarod Wilson wrote: > > On Fri, Oct 15, 2010 at 06:06:37PM +0200, Maxim Levitsky wrote: > > > This is a result of last round of debug with > > > Sami R . > > > > > > Thank you Sami very much! > > > > > > The biggest bug I fixed is that, > > > I was clobbering the CIRCFG register after it is setup > > > That wasn't a good idea really > > > > > > And some small refactoring, etc. > > > > > > Signed-off-by: Maxim Levitsky > > > --- > > > drivers/media/IR/ene_ir.c | 43 ++++++++++++++++++++----------------------- > > > 1 files changed, 20 insertions(+), 23 deletions(-) > > > > > > diff --git a/drivers/media/IR/ene_ir.c b/drivers/media/IR/ene_ir.c > > > index dc32509..8639621 100644 > > > --- a/drivers/media/IR/ene_ir.c > > > +++ b/drivers/media/IR/ene_ir.c > > ... > > > @@ -282,6 +287,7 @@ static void ene_rx_setup(struct ene_device *dev) > > > ene_set_reg_mask(dev, ENE_CIRCFG, ENE_CIRCFG_CARR_DEMOD); > > > > > > /* Enable carrier detection */ > > > + ene_write_reg(dev, ENE_CIRCAR_PULS, 0x63); > > > > Looks sane, though I'd prefer to see symbolic bit names or some such thing > > here instead of 0x63. Not something to hold up the patch though. > > > > Acked-by: Jarod Wilson > > > 63 isn't a bitfileld, but rather two numbers. > 3 is number of carrier pulses to skip, and 6 is number of carrier pulses > to average. > > I have a note about that in header. Ah, so you do. Apologies then. Personally, I'd probably document that next to the 0x63 as well, just so readers who neglect to go find the register define and discover the comment there don't think the same as me. ;) -- Jarod Wilson jarod@redhat.com