linux-kernel - Re: [PATCH v9 11/25] gpio/omap: cleanup omap_gpio_mod

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120424160447.GI3739@atomide.com>
Date:	Tue, 24 Apr 2012 09:04:48 -0700
From:	Tony Lindgren <tony@...mide.com>
To:	"DebBarma, Tarun Kanti" <tarun.kanti@...com>
Cc:	Janusz Krzysztofik <jkrzyszt@....icnet.pl>,
	linux-omap@...r.kernel.org, grant.likely@...retlab.ca,
	khilman@...com, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, Charulatha V <charu@...com>
Subject: Re: [PATCH v9 11/25] gpio/omap: cleanup omap_gpio_mod_init function

* DebBarma, Tarun Kanti <tarun.kanti@...com> [120424 08:40]:
> Hi Janusz,
> 
> On Tue, Apr 24, 2012 at 12:24 AM, DebBarma, Tarun Kanti
> <tarun.kanti@...com> wrote:
> > On Sat, Apr 21, 2012 at 7:33 PM, Janusz Krzysztofik
> > <jkrzyszt@....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@...com>
> >>> Signed-off-by: Charulatha V <charu@...com>
> >>> Reviewed-by: Santosh Shilimkar <santosh.shilimkar@...com>
> >>> Acked-by: Tony Lindgren <tony@...mide.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@...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@...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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/