Messages in this thread |  | | | Date | Tue, 24 Apr 2012 09:04:48 -0700 | | From | Tony Lindgren <> | | Subject | Re: [PATCH v9 11/25] gpio/omap: cleanup omap_gpio_mod_init function |
| |
* DebBarma, Tarun Kanti <tarun.kanti@ti.com> [120424 08:40]: > Hi Janusz, > > On Tue, Apr 24, 2012 at 12:24 AM, DebBarma, Tarun Kanti > <tarun.kanti@ti.com> wrote: > > On Sat, Apr 21, 2012 at 7:33 PM, Janusz Krzysztofik > > <jkrzyszt@tis.icnet.pl> wrote: > >> On Thursday 02 of February 2012 23:00:37 Tarun Kanti DebBarma wrote: > >>> With register offsets now defined for respective OMAP versions we can get rid > >>> of cpu_class_* checks. This function now has common initialization code for > >>> all OMAP versions... > >>> > >>> Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com> > >>> Signed-off-by: Charulatha V <charu@ti.com> > >>> Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > >>> Acked-by: Tony Lindgren <tony@atomide.com> > >> > >> Sorry for being so late with my comment for chanes already present in > >> mainline, but this patch breaks GPIO on Amstrad Delta at least, and I > >> have neither enough spare time nor enough experience with non OMAP1 > >> machines to provide a solution myself. > > Yes, I looked at the omap_gpio_mod_init() and OMAP1 configurations are > > overwritten. > > Also looks like there is issue in making distinction between omap15xx > > and omap16xx. > > I will post a patch and you can help me testing it in OMAP1 platform. > > Thanks for pointing this out. ...
> Here is the patch, with attachment as well. I have just tested on > OMAP4 platform. > Testing on other OMAP2+ platforms is pending. In the meantime can you please > validate on OMAP1 platform and confirm? Thanks. > -- > Tarun > > From: Tarun Kanti DebBarma <tarun.kanti@ti.com> > Date: Tue, 24 Apr 2012 20:34:32 +0530 > Subject: [PATCH] gpio/omap: fix omap1 register overwrite in omap_gpio_mod_init > > Initialization of irqenable, irqstatus registers is the common > operation done in this function for all OMAP platforms, viz. > OMAP1, OMAP2+. The latter _gpio_rmw()'s to irqenable register > was overwriting the correctly programmed OMAP1 value at the > beginning. As a result, even though it worked on OMAP2+ > platforms it was breaking OMAP1 functionality.
Sounds like the original patch was never tested on omap1?
> On closer observation it is found that the first _gpio_rmw() > which is supposedly done to take care of OMAP1 platform is > generic enough and takes care of OMAP2+ platform as well. > Therefore remove the latter _gpio_rmw() to irqenable as they > are redundant. > > Also, changing the sequence and logic of initializing the > irqstatus.
Please mention also the breaking commit here and get this fix merged as a regression as soon as it's tested for the current -rc series.
Regards,
Tony
> Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com> > --- > drivers/gpio/gpio-omap.c | 5 +---- > 1 files changed, 1 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c > index 1adc2ec..b8f01c1 100644 > --- a/drivers/gpio/gpio-omap.c > +++ b/drivers/gpio/gpio-omap.c > @@ -964,11 +964,8 @@ static void omap_gpio_mod_init(struct gpio_bank *bank) > return; > } > > + _gpio_rmw(base, bank->regs->irqstatus, l, > bank->regs->irqenable_inv == 0 ); > _gpio_rmw(base, bank->regs->irqenable, l, bank->regs->irqenable_inv); > - _gpio_rmw(base, bank->regs->irqstatus, l, > - bank->regs->irqenable_inv == false); > - _gpio_rmw(base, bank->regs->irqenable, l, bank->regs->debounce_en != 0); > - _gpio_rmw(base, bank->regs->irqenable, l, bank->regs->ctrl != 0); > if (bank->regs->debounce_en) > _gpio_rmw(base, bank->regs->debounce_en, 0, 1); > > -- > 1.7.0.4
|  |