Re: [GIT PULL] for testing: OMAP hwmod driver conversions: watchdog, UART, i2c

[Kevin Hilman <khilman@deeprootsystems.com>]

"Nayak, Rajendra" <rnayak@ti.com> writes:

>> -----Original Message-----
>> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>> Sent: Wednesday, October 06, 2010 2:05 AM
>> To: Nayak, Rajendra
>> Cc: Paul Walmsley; Cousson, Benoit; Tony Lindgren; linux-omap@vger.kernel.org; Varadarajan, Charulatha; Raja,
>> Govindraj
>> Subject: Re: [GIT PULL] for testing: OMAP hwmod driver conversions: watchdog, UART, i2c
>> 
>> "Nayak, Rajendra" <rnayak@ti.com> writes:
>> 
>> >> -----Original Message-----
>> >> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>> >> Sent: Wednesday, October 06, 2010 1:19 AM
>> >> To: Nayak, Rajendra
>> >> Cc: Paul Walmsley; Cousson, Benoit; Tony Lindgren; linux-omap@vger.kernel.org; Varadarajan, Charulatha; Raja,
>> >> Govindraj
>> >> Subject: Re: [GIT PULL] for testing: OMAP hwmod driver conversions: watchdog, UART, i2c
>> >>
>> >> "Nayak, Rajendra" <rnayak@ti.com> writes:
>> >>
>> >> >> -----Original Message-----
>> >> >> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>> >> >> Sent: Tuesday, October 05, 2010 10:34 PM
>> >> >> To: Nayak, Rajendra
>> >> >> Cc: Paul Walmsley; Cousson, Benoit; Tony Lindgren; linux-omap@vger.kernel.org; Varadarajan, Charulatha;
>> Raja,
>> >> >> Govindraj
>> >> >> Subject: Re: [GIT PULL] for testing: OMAP hwmod driver conversions: watchdog, UART, i2c
>> >> >>
>> >> >> "Nayak, Rajendra" <rnayak@ti.com> writes:
>> >> >>
>> >> >> > <snip>..
>> >> >> >
>> >> >> >> >
>> >> >> >> > Below is an untested patch to provide some mechanism to deal with this --
>> >> >> >> > I'd appreciate everyone's comments on this, particularly the comments in
>> >> >> >> > the patch code on how to deal with this problem.
>> >> >> >>
>> >> >> >> Hi Paul,
>> >> >> >>
>> >> >> >> I'll test this patch to see if it solves the issue seen with i2c on n800.
>> >> >> >
>> >> >> > Hi Paul, Benoit, Kevin,
>> >> >> >
>> >> >> > So looks like the issue with i2c on n800 isn't really because of additional
>> >> >> > delay needed after a clock enable, but related to the omap_readl/writel's
>> >> >> > done on the 16bit i2c registers as I was suspecting earlier.
>> >> >> >
>> >> >> > The below patch fixes the n800 issue for me. My mailer might mess up the alignment
>> >> >> > hence attaching it too.
>> >> >> >
>> >> >> > From af9733f2e63a16b446635d52015e05d631f0788f Mon Sep 17 00:00:00 2001
>> >> >> > From: Rajendra Nayak <rnayak@ti.com>
>> >> >> > Date: Tue, 5 Oct 2010 16:36:30 +0530
>> >> >> > Subject: [PATCH] OMAP: hwmod: Handle modules with 16bit registers
>> >> >> >
>> >> >> > Some modules which have 16bit registers can cause imprecise
>> >> >> > aborts if a __raw_readl/writel is used to read/write 32 bits.
>> >> >> >
>> >> >> > Add an additional flag to identify modules which have such
>> >> >> > hard requirement, and handle it in the hwmod framework.
>> >> >> >
>> >> >> > Signed-off-by: Rajendra Nayak <rnayak@ti.com>
>> >> >>
>> >> >> Can you also send a corresponding patch to the i2c hwmod data.
>> >> > Hi Kevin,
>> >> >
>> >> > Ok, I'll repost the original patch which adds the omap2 hwmod data with
>> >> > these flags added for omap2420. Does that sound fine?
>> >>
>> >> Since I've alrady done some manual merge conflicts here, you can just
>> >> post an additional patch and I will fold it in.
>> >
>> > Ok, so here's the patch
>> > ------
>> > From 06b4bb7a0ce96776d6d9b344b4786b48fceeffe1 Mon Sep 17 00:00:00 2001
>> > From: Rajendra Nayak <rnayak@ti.com>
>> > Date: Wed, 6 Oct 2010 01:49:32 +0530
>> > Subject: [PATCH] OMAP2: hmwod: Fix impresice aborts seen on n800/n810
>> >
>> > I2C module on OMAP2420 has 16bit registers and causes imprecise
>> > aborts if 32bits are read/wriitten into.
>> > Use the HWMOD_16BIT_REG flag to notify the hmwod framework of this
>> > hard requirement so that __raw_writew/readw is used to read /write
>> > the mdoule registers.
>> >
>> > Signed-off-by: Rajendra Nayak <rnayak@ti.com>
>> > ---
>> >  arch/arm/mach-omap2/omap_hwmod_2420_data.c |    2 ++
>> >  1 files changed, 2 insertions(+), 0 deletions(-)
>> 
>> Thanks.
>> 
>> Shouldn't we do the same for 2430?  I guess the imprecise aborts aren't
>> happening on 2430, but I think we should still use the 16-bit accesses
>> there too.
>> 
>> What do you think?
>
> Yeah, actually i2c modules on all OMAPs including OMAP4 have had 16 bit registers.
> But somehow only 2420 seemed to cause issues.

OK, then let's keep this as a targetted fix for this merge window, and
we can discuss whether this needs to be done for all OMAPs later.

I'll use you current patch,

Thanks,

Kevin