Re: [PATCH v9 11/25] gpio/omap: cleanup omap_gpio_mod_init function

["DebBarma, Tarun Kanti" <tarun.kanti@ti.com>]

On Wed, Apr 25, 2012 at 7:15 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Apr 25, 2012 at 06:24:14PM +0530, DebBarma, Tarun Kanti wrote:
>> On Wed, Apr 25, 2012 at 12:09 PM, Shilimkar, Santosh
>> <santosh.shilimkar@ti.com> wrote:
>> > On Wed, Apr 25, 2012 at 10:04 AM, DebBarma, Tarun Kanti
>> > <tarun.kanti@ti.com> wrote:
>> >> On Tue, Apr 24, 2012 at 9:34 PM, Tony Lindgren <tony@atomide.com> wrote:
>> >>> * 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?
>> >> That's right, only bootup test was done on OMAP1710-SDP.
>> >>
>> >>>
>> >>>> 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.
>> >> Sure, I will do that!
>> >
>> > Looks like it is regression on 3.4 as well so CC stable when you
>> > post the patch.
>> Ok, I will do that.
>
> Correction.
>
> Don't email your patch in any way to the stable folk _before_ it has been
> taken into Linus' tree.  However, you _may_ add in the patch attributations
> a Cc: <stable@vger.kernel.org> tag if you want the stable folk to
> automatically pick up your patch when it _does_ end up in Linus' tree.
> But... make sure that git send-email or whatever doesn't automatically
> add that to the recipients for the emailed patch.
>
> If you send the stable people a patch before its in mainline, you'll get
> a whinge telling you to read Documentation/stable_kernel_rules.txt

Alright, I will add Cc: <stable@vger.kernel.org> tag in the patch.
Thanks.
--
Tarun