RE: [PATCH 3/3] omap: add hwspinlock device

[Kamoolkar, Mugdha]

>  wrote:
> >> Let's take the i2c-omap for example.
> >>
> >> It sounds like it must have a predefined hwspinlock, but what if:
> >>
> >> 1. It will use omap_hwspinlock_request() to dynamically allocate
> a hwspinlock
> >> 2. Obviously, the hwspinlock id number must be communicated to
> the M3
> >> BIOS, so the i2c-omap will publish that id using a small shared
> memory
> >> entry that will be allocated for this sole purpose
> >> 3. we will make sure that 1+2 completes before the remote
> processor is
> >> taken out of reset
> >>
> >> This does not require any smart IPC and it will allow us to get
> rid of
> >> the omap_hwspinlock_request_specific() API and its early-callers
> >> requirement.
> >
> > Yes, that would indeed simplify things.
> 
> Balaji, Nishant, are you OK with this ?
> 
The problem with this approach is that the i2c driver would have to 
sync up on the shared memory location that it uses to share the 
information of the spinlock ID. What location would this be? How would 
the i2c driver on the m3 know about this location? Does this mean 
hard-coding of the shared memory address? If yes, then there would be 
an impact to users if they wanted to change their memory map. Any kind 
of hard-coding of this sort can be painful in such cases, especially 
if it happens in multiple drivers. On the other hand, hard-coding 
(reserving) spinlock IDs is a much safer option and does not impact 
anything else.

> It means that the I2C driver will dynamically get a hwspinlock and
> then it would need to announce the id of that hwspinlock before the
> M3
> is taken out of reset.
> 
> It will help us get rid of static allocations that will never scale
> nicely and static vs. dynamic allocation races.
> 
> >
> >> All we will be left to take care of is the order of the ->probe()
> >> execution (assuming we want both the i2c and the hwspinlock
> drivers to
> >> be device_initcall)
> >
> > I understand the dependency between i2c and hwspinlock for some
> > platforms with a shared i2c bus.  Besides that being a broken
> hardware
> > design, I can see the need for the i2c driver to take a hwspinlock
> for
> > i2c xfers, but why does the i2c driver need to take the hwspinlock
> at
> > probe time?
> 
> It doesn't; it just needs to reserve a hwspinlock, and for that, we
> need the hwspinlock driver in place.
> 
> >  Presumably, this is before the remote cores are executing
> > code.
> >
> >>>
> >>> This kind of thing needs to be done by platform_data function
> pointers,
> >>> as is done for every other driver that needs platform-specific
> driver
> >>> customization.
> >>
> >> Why would we need platform-specific function pointers here ? I'm
> not
> >> sure I'm following this one.
> >
> > So that board code (built-in) does not call the hwspinlock driver
> > (potentially a module.)
> >
> > The way to solve this is to have platform_data with function
> pointers,
> > so that when the driver's ->probe() is done, it can call cany
> custom
> > hooks registered by the board code.
> 
> Sorry, still not following.
> 
> Do you refer to the i2c driver calling the hwspinlcok_request
> function
> at probe time via platform_data function pointers ?
> 
> With the previous _request_specific() allocation API, the important
> issue to follow was the timing:  it had to be called before any
> driver
> gets the chance to call the dynamic _request() API.
> That's why we had to have early board code calling it. Obviously the
> hwspinlock instance would then had to be delivered to the driver via
> platform data.
> 
> But now, if we get rid of that static allocation method entirely,
> i2c_omap can just call hwspinlock_request() on probe(), but again,
> no
> pdata function pointers are involved because this API is
> EXPORT_SYMBOL'ed.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-
> omap" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Regards,
Mugdha
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3] OMAP: DSS2: don't power off a panel twice

[Grazvydas Ignotas]

Tomi can you queue this? This has been tested by multiple people now,
also see here:
http://marc.info/?l=linux-omap&m=128759415821646&w=2

On Fri, Sep 3, 2010 at 6:03 AM, Stanley.Miao  wrote:
> If we blank the panel by
> echo 1 > /sys/devices/platform/omapfb/graphics/fb0/blank
>
> Then, we reboot the sytem, the kernel will crash at
> drivers/video/omap2/dss/core.c:323
>
> This is because the panel is closed twice. Now check the state of a dssdev
> to forbid a panel is power on or power off twice.
>
> Signed-off-by: Stanley.Miao 
> ---
>  drivers/video/omap2/displays/panel-acx565akm.c     |    6 ++
>  drivers/video/omap2/displays/panel-generic.c       |    6 ++
>  .../video/omap2/displays/panel-sharp-lq043t1dg01.c |    6 ++
>  .../video/omap2/displays/panel-sharp-ls037v7dw01.c |    6 ++
>  .../video/omap2/displays/panel-toppoly-tdo35s.c    |    6 ++
>  .../video/omap2/displays/panel-tpo-td043mtea1.c    |    6 ++
>  6 files changed, 36 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/video/omap2/displays/panel-acx565akm.c 
> b/drivers/video/omap2/displays/panel-acx565akm.c
> index 07fbb8a..e773106 100644
> --- a/drivers/video/omap2/displays/panel-acx565akm.c
> +++ b/drivers/video/omap2/displays/panel-acx565akm.c
> @@ -587,6 +587,9 @@ static int acx_panel_power_on(struct omap_dss_device 
> *dssdev)
>
>        dev_dbg(&dssdev->dev, "%s\n", __func__);
>
> +       if (dssdev->state == OMAP_DSS_DISPLAY_ACTIVE)
> +               return 0;
> +
>        mutex_lock(&md->mutex);
>
>        r = omapdss_sdi_display_enable(dssdev);
> @@ -644,6 +647,9 @@ static void acx_panel_power_off(struct omap_dss_device 
> *dssdev)
>
>        dev_dbg(&dssdev->dev, "%s\n", __func__);
>
> +       if (dssdev->state != OMAP_DSS_DISPLAY_ACTIVE)
> +               return;
> +
>        mutex_lock(&md->mutex);
>
>        if (!md->enabled) {
> diff --git a/drivers/video/omap2/displays/panel-generic.c 
> b/drivers/video/omap2/displays/panel-generic.c
> index 300eff5..395a68d 100644
> --- a/drivers/video/omap2/displays/panel-generic.c
> +++ b/drivers/video/omap2/displays/panel-generic.c
> @@ -39,6 +39,9 @@ static int generic_panel_power_on(struct omap_dss_device 
> *dssdev)
>  {
>        int r;
>
> +       if (dssdev->state == OMAP_DSS_DISPLAY_ACTIVE)
> +               return 0;
> +
>        r = omapdss_dpi_display_enable(dssdev);
>        if (r)
>                goto err0;
> @@ -58,6 +61,9 @@ err0:
>
>  static void generic_panel_power_off(struct omap_dss_device *dssdev)
>  {
> +       if (dssdev->state != OMAP_DSS_DISPLAY_ACTIVE)
> +               return;
> +
>        if (dssdev->platform_disable)
>                dssdev->platform_disable(dssdev);
>
> diff --git a/drivers/video/omap2/displays/panel-sharp-lq043t1dg01.c 
> b/drivers/video/omap2/displays/panel-sharp-lq043t1dg01.c
> index 1026746..0c6896c 100644
> --- a/drivers/video/omap2/displays/panel-sharp-lq043t1dg01.c
> +++ b/drivers/video/omap2/displays/panel-sharp-lq043t1dg01.c
> @@ -43,6 +43,9 @@ static int sharp_lq_panel_power_on(struct omap_dss_device 
> *dssdev)
>  {
>        int r;
>
> +       if (dssdev->state == OMAP_DSS_DISPLAY_ACTIVE)
> +               return 0;
> +
>        r = omapdss_dpi_display_enable(dssdev);
>        if (r)
>                goto err0;
> @@ -65,6 +68,9 @@ err0:
>
>  static void sharp_lq_panel_power_off(struct omap_dss_device *dssdev)
>  {
> +       if (dssdev->state != OMAP_DSS_DISPLAY_ACTIVE)
> +               return;
> +
>        if (dssdev->platform_disable)
>                dssdev->platform_disable(dssdev);
>
> diff --git a/drivers/video/omap2/displays/panel-sharp-ls037v7dw01.c 
> b/drivers/video/omap2/displays/panel-sharp-ls037v7dw01.c
> index 7d9eb2b..9a138f6 100644
> --- a/drivers/video/omap2/displays/panel-sharp-ls037v7dw01.c
> +++ b/drivers/video/omap2/displays/panel-sharp-ls037v7dw01.c
> @@ -135,6 +135,9 @@ static int sharp_ls_power_on(struct omap_dss_device 
> *dssdev)
>  {
>        int r = 0;
>
> +       if (dssdev->state == OMAP_DSS_DISPLAY_ACTIVE)
> +               return 0;
> +
>        r = omapdss_dpi_display_enable(dssdev);
>        if (r)
>                goto err0;
> @@ -157,6 +160,9 @@ err0:
>
>  static void sharp_ls_power_off(struct omap_dss_device *dssdev)
>  {
> +       if (dssdev->state != OMAP_DSS_DISPLAY_ACTIVE)
> +               return;
> +
>        if (dssdev->platform_disable)
>                dssdev->platform_disable(dssdev);
>
> diff --git a/drivers/video/omap2/displays/panel-toppoly-tdo35s.c 
> b/drivers/video/omap2/displays/panel-toppoly-tdo35s.c
> index e320e67..526e906 100644
> --- a/drivers/video/omap2/displays/panel-toppoly-tdo35s.c
> +++ b/drivers/video/omap2/displays/panel-toppoly-tdo35s.c
> @@ -46,6 +46,9 @@ static int toppoly_tdo_panel_power_on(struct 
> omap_dss_device *dssdev)
>  {
>        int r;
>
> +       if (dssdev->state == OMAP_DSS_DISPLAY_ACTIVE)
> +               return 0;
> +
>        r = omapdss_dpi_display_enable(dssdev);
>        if (r)
>                goto err0;
> @@ -65,6 

Re: [PATCH 1/3] drivers: misc: add omap_hwspinlock driver

[Arnd Bergmann]

On Thursday 21 October 2010, Ohad Ben-Cohen wrote:
> This sounds like adding a set of API that resembles spin_{unlock,lock}_irq.
> 
> My gut feeling here is that while this may be useful and simple at
> certain places, it is somewhat error prone; a driver which would
> erroneously use this at the wrong place will end up enabling
> interrupts where it really shouldn't.
> 
> Don't you feel that proving a simple spin_lock_irqsave-like API is
> actually safer and less error prone ?
> 
> I guess that is one of the reasons why spin_lock_irqsave is much more
> popular than spin_lock_irq - people just know it will never screw up.

People can screw that up in different ways, e.g.

spin_lock_irqsave(&lock_a, flags);
spin_lock_irqsave(&lock_b, flags); /* overwrites flags */

spin_lock_irqrestore(&lock_b, flags);
spin_lock_irqrestore(&lock_a, flags); /* still disabled! */

I use the presence of spin_lock_irqsave in driver code as an indication
of whether the author had any clue about locking. Most experienced
coders use the right version intuitively, while beginners often just
use _irqsave because they didn't understand the API and they were told
that using this is safe.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Contreras]

On Wed, Oct 20, 2010 at 8:54 PM, Felipe Balbi  wrote:
> On Wed, Oct 20, 2010 at 11:22:17AM -0500, Felipe Contreras wrote:
>>
>> Users would not need to care... if they enable USB and USB_GADGET, it
>> will be selected automatically for OMAP3, which is what users would
>> expect.
>
> wouldn't a user expect USB to be enabled if the board _has_ a USB
> connector ?

Not if it's not going to be used.

>> I was talking more about developers.
>>
>> How should the help be fixed?
>> "Select y if you are using an OMAP3 system that has TWL4030"
>
> If you have the TWL4030 family of PMIC chips and want to use USB, you
> probably need this. If unsure, say Y.

If you see TWL4030_USB, it's because you have enabled USB_SUPPORT, so
you want to use USB. And that help still doesn't say anything to most
people.

>> 1) Do you think all the OMAP3 boards should add that?
>
> why not ?

You prefer a patch that adds one line per board rather than a patch
that adds one line in total... That is exactly the opposite of Linus's
complaint regarding big ARM changesets.

>> 2) What if you don't want to use USB?
>
> what if you don't want to use USB and we use your choice of:
>
> default y if ARCH_OMAP3 ??
>
> what's the big difference ? The default will be "USB enabled" and if you
> don't want, just drop it. The thing is that in most cases we want to
> support what is wired on the board.

For "most cases" there is 'default'.

> RX51 has a USB micro-B connector, so
> by default we want USB peripheral enabled and host disabled. If a user
> (or developer for that matter) wants to play with the unsupported host
> mode on RX51, he will enable that part by himself and pray for it to
> work. If, on the other hand, a camera developer doesn't need USB at all
> and decides to save a few bytes on the binary, he will make sure to
> disable it.

Your patch would not allow such camera developer to disable USB
through the kernel configuration.

> The thing is that there's no way to make everybody happy, so we stick to
> what the board is wired to do.

I don't understand, currently TWL4030_USB is not selected in any way,
you are saying it should always be enabled (which I disagree with),
but that's not currently happening. If anything, my patch is changing
things to go into the direction you want, not the opposite.

My patch shuffles _one_ line that:
1) Allows clueless OMAP3 users to have TWL4030_USB enabled by default
_if_ they enable USB and USB_GADGET regardless of which board is being
used
2) Still allow people to build kernels without USB, nor TWL4030_USB
3) Simplify defconfgs by removing one line
4) Don't need extra changes in the board configurations
5) Doesn't increase the total lines in Kconfigs

Compared to the current situation 1) would actually change things into
the direction you advocated to: TWL4030_USB is enabled by default.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] omap: add hwspinlock device

[Ohad Ben-Cohen]

On Thu, Oct 21, 2010 at 10:36 AM, Kamoolkar, Mugdha  wrote:
>>  wrote:
>> > Yes, that would indeed simplify things.
>>
>> Balaji, Nishant, are you OK with this ?
>>
> The problem with this approach is that the i2c driver would have to
> sync up on the shared memory location that it uses to share the
> information of the spinlock ID.

I agree.

But we seem to have this sort of problem anyway:

Since we are forbidden to take a hwspinlock over a lengthy period of
time, i2c-omap can't really use it directly to achieve mutual
exclusion over the entire i2c transfer (which is long and involved
sleeping).

It was suggested that i2c-omap would only use the hwspinlock to
protect a shared memory entry which would indicate the owner of the
i2c bus. Either that, or use something like Peterson's mutual
exclusion algorithm which is entirely implemented in software.

Both of the latter approaches involves sync'ing up on a shared memory
location..  so it seems like i2c-omap anyway has to deal with this
kind of pain, and having a predefined hwspinlock id number doesn't
relieve it.

What do you think ?

Thanks,
Ohad.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH V3] OMAP: DSS2: don't power off a panel twice

[Guruswamy, Senthilvadivu]

> -Original Message-
> From: linux-fbdev-ow...@vger.kernel.org [mailto:linux-fbdev-
> ow...@vger.kernel.org] On Behalf Of Grazvydas Ignotas
> Sent: Thursday, October 21, 2010 2:32 PM
> To: tomi.valkei...@nokia.com
> Cc: Stanley.Miao; linux-omap@vger.kernel.org; bryan...@canonical.com;
> mathieu.poir...@canonical.com; linux-fb...@vger.kernel.org
> Subject: Re: [PATCH V3] OMAP: DSS2: don't power off a panel twice
> 
> Tomi can you queue this? This has been tested by multiple people now,
> also see here:
> http://marc.info/?l=linux-omap&m=128759415821646&w=2
> 
> On Fri, Sep 3, 2010 at 6:03 AM, Stanley.Miao 
> wrote:
> > If we blank the panel by
> > echo 1 > /sys/devices/platform/omapfb/graphics/fb0/blank
> >
> > Then, we reboot the sytem, the kernel will crash at
> > drivers/video/omap2/dss/core.c:323
> >
> > This is because the panel is closed twice. Now check the state of a
> dssdev
> > to forbid a panel is power on or power off twice.
> >
> > Signed-off-by: Stanley.Miao 
> > ---
> >  drivers/video/omap2/displays/panel-acx565akm.c     |    6 ++
> >  drivers/video/omap2/displays/panel-generic.c       |    6 ++
> >  .../video/omap2/displays/panel-sharp-lq043t1dg01.c |    6 ++
> >  .../video/omap2/displays/panel-sharp-ls037v7dw01.c |    6 ++
> >  .../video/omap2/displays/panel-toppoly-tdo35s.c    |    6 ++
> >  .../video/omap2/displays/panel-tpo-td043mtea1.c    |    6 ++
> > 
[Senthil]  Why is this change not applicable for panel-taal.c ?
> >
> > diff --git a/drivers/video/omap2/displays/panel-acx565akm.c
> b/drivers/video/omap2/displays/panel-acx565akm.c
> > index 07fbb8a..e773106 100644
> > --- a/drivers/video/omap2/displays/panel-acx565akm.c
> > +++ b/drivers/video/omap2/displays/panel-acx565akm.c
> > @@ -587,6 +587,9 @@ static int acx_panel_power_on(struct omap_dss_device
> *dssdev)
> >
> >        dev_dbg(&dssdev->dev, "%s\n", __func__);
> >
> > +       if (dssdev->state == OMAP_DSS_DISPLAY_ACTIVE)
> > +               return 0;
> > +
> >        mutex_lock(&md->mutex);
> >
> >        r = omapdss_sdi_display_enable(dssdev);
> > @@ -644,6 +647,9 @@ static void acx_panel_power_off(struct
> omap_dss_device *dssdev)
> >
> >        dev_dbg(&dssdev->dev, "%s\n", __func__);
> >
> > +       if (dssdev->state != OMAP_DSS_DISPLAY_ACTIVE)
> > +               return;
> > +
> >        mutex_lock(&md->mutex);
> >
> >        if (!md->enabled) {

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] omap: am3517: fix build break

[Anand Gadiyar]

Patch usb-am35x-add-musb-support.patch in greg's USB queue
adds backan include of . This file was renamed
by another commit in the omap tree and the file was already
fixed to reflect this.

Remove this include to fix up a build break.

Signed-off-by: Anand Gadiyar 
Cc: Tony Lindgren 
Cc: Ajay Kumar Gupta 
Cc: Felipe Balbi 
Cc: Greg Kroah-Hartman 
---
Build break observed with linux-next as of 20101021.

Greg, okay with me to fold this into the original patch
if the others in copy agree. It's a trivial enough change.

The patch that renamed plat/control.h is this one:
http://git.kernel.org/?p=linux/kernel/git/sfr/linux-next.git;a=commitdiff;h=4814ced5116e3b73dc4f63eec84999739fc8ed11

 arch/arm/mach-omap2/board-am3517evm.c |1 -
 1 file changed, 1 deletion(-)

Index: mainline/arch/arm/mach-omap2/board-am3517evm.c
===
--- mainline.orig/arch/arm/mach-omap2/board-am3517evm.c
+++ mainline/arch/arm/mach-omap2/board-am3517evm.c
@@ -35,7 +35,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include "mux.h"
 #include "control.h"
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 0/3] OMAP2/3: DMA: FIFO drain errata fixes

[Peter Ujfalusi]

Sorry, I did missed this mail...

On Saturday 09 October 2010 01:17:46 ext Tony Lindgren wrote:

> Guys, as we're so late into getting 2.6.36 tagged, I'm thinking about just
> queueing these for 2.6.37 for the following reasons:
> 
> - The 24xx patch is a fix for commit c12abc0 that's was merged over a
>   year ago. We've lived with it for over a year now. What difference does
>   extra few more months make if we have it in 2.6.37 instead of 2.6.36?
> 
> - The second 34xx fix seems to happen only when disabling DMA on the fly.
>   Again, we've had support for 34xx in the mainline kernel for a few
>   years, and we're not seeing unrecoverable issues with MMC or USB or
>   any other code calling omap_dma_stop().
> 
> Sure these fix major issues with the DMA, but are these really critical
> for 2.6.36?
> 
> So if you really want me to argue for merging these into 2.6.36, please let
> me know some oops causing cases or data corruption that happens without
> these patches.

Well 2.6.36 is released.
I still would like to see this to be part of the 2.6.36.1 release. We can 
reproduce the DMA error in 2.6.36 with audio.

Tony: can you make sure it is going to part of 2.6.36.1?

Thanks,
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] OMAP: hmwod: Update the sysc_cache in case module context is lost

[Nayak, Rajendra]

> -Original Message-
> From: Kevin Hilman [mailto:khil...@deeprootsystems.com]
> Sent: Friday, October 15, 2010 9:10 PM
> To: Shilimkar, Santosh
> Cc: Nayak, Rajendra; linux-omap@vger.kernel.org; Paul Walmsley; Cousson, 
> Benoit
> Subject: Re: [PATCH] OMAP: hmwod: Update the sysc_cache in case module 
> context is lost
> 
> "Shilimkar, Santosh"  writes:
> 
> >> -Original Message-
> >> From: Kevin Hilman [mailto:khil...@deeprootsystems.com]
> >> Sent: Friday, October 15, 2010 3:44 AM
> >> To: Nayak, Rajendra
> >> Cc: linux-omap@vger.kernel.org; Paul Walmsley; Cousson, Benoit; Shilimkar,
> >> Santosh
> >> Subject: Re: [PATCH] OMAP: hmwod: Update the sysc_cache in case module
> >> context is lost
> >>
> >> Rajendra Nayak  writes:
> >>
> >> > Do not skip the sysc programming in the hmwod framework based
> >> > on the cached value alone, since at times the module might have lost
> >> > context (due to the Powerdomain in which the module belongs
> >> > transitions to either Open Switch RET or OFF).
> >>
> >> Shouldn't the driver for each IP be responsible for restoring it's
> >> register contents after context loss, including it's SYSC?
> >>
> >> Seems to me that if SYSC is lost, it means the driver's save/restore
> >> is buggy.
> >
> > I am glad you asked this question. I had a same argument with Benoit
> > that driver anyway does context save restore for other registers and
> > it can do SYSC as well.
> >
> > But Benoit's point was that "sysconfig is a part of the PRCM located
> > in the IP, but this is purely TI implementation specific. The same
> > IP in another platform will not have this sysconfig entry. That's why
> > its important to hide them from the driver "
> 
> OK, but this patch still doesn't address the real problem.  Namely, that
> *somebody* needs to save/restore the SYSC reg for the IP.

Hi Kevin,

What the patch does is reprogram's the sysc value (from the cache)
whenever its lost. So its infact saved in the cache and restored when 
needed. 

> 
> Otherwise, all this patch does is refresh the _sysc_cache with
> completely unknown contents.  It also somewhat defeats the purpose of
> having a cache.  If you're going to read SYSC in order to determine
> whether or not you can avoid a write, you might as well just blindly
> write.

I thought of this and dismissed it thinking I would end up with a read/or/write
and instead a read always to avoid write is better.
But now looking back again, it does make sense to still keep the cache to avoid
a read (since a read has significantly more latency than write) and do a blind 
write
always. Does that make sense?

Regards,
Rajendra

> 
> One option to fix the save/restore would be that TI specific context
> save/restore could be done in the device layer by adding some additional
> save/restore do the functions calling omap_device_[idle|enable].
> 
> IOW, most devices just call omap_device directly by doing something like:
> 
> struct omap_device_pm_latency omap_wdt_latency[] = {
>   [0] = {
>   .deactivate_func = omap_device_idle_hwmods,
>   .activate_func   = omap_device_enable_hwmods,
>   .flags   = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
>   },
> };
> 
> 
> But these [de]activate functions can be anything, and could be made into
> functions that do some additional save/restore and then call
> omap_device_*
> 
> Kevin

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] drivers: misc: add omap_hwspinlock driver

[Ohad Ben-Cohen]

On Thu, Oct 21, 2010 at 11:04 AM, Arnd Bergmann  wrote:
> On Thursday 21 October 2010, Ohad Ben-Cohen wrote:
>> This sounds like adding a set of API that resembles spin_{unlock,lock}_irq.
>>
>> My gut feeling here is that while this may be useful and simple at
>> certain places, it is somewhat error prone; a driver which would
>> erroneously use this at the wrong place will end up enabling
>> interrupts where it really shouldn't.
>>
>> Don't you feel that proving a simple spin_lock_irqsave-like API is
>> actually safer and less error prone ?
>>
>> I guess that is one of the reasons why spin_lock_irqsave is much more
>> popular than spin_lock_irq - people just know it will never screw up.
>
> People can screw that up in different ways, e.g.

Sure...

> I use the presence of spin_lock_irqsave in driver code as an indication
> of whether the author had any clue about locking. Most experienced
> coders use the right version intuitively, while beginners often just
> use _irqsave because they didn't understand the API and they were told
> that using this is safe.

Agree.

I'll add the _irq pair of APIs, this will make the users' code
cleaner. This is valuable especially since all of our current users
have their interrupts enabled anyway.

The change is also pretty trivial:

* move the internal locking implementation to raw_ methods
* the raw_ methods would save the current interrupt state only if
given a placeholder
* wrap those raw_ methods with the desired API (but only support the
_irq and _irqsave flavors)

Thanks,
Ohad.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] smsc95xx: generate random MAC address once, not every ifup

[David Miller]

From: Bernard Blackham 
Date: Tue, 19 Oct 2010 10:16:39 +1100

> The smsc95xx driver currently generates a new random MAC address
> every time the interface is brought up. This makes it impossible to
> override using the standard `ifconfig hw ether` approach.
> 
> Past patches tried to make the MAC address a module parameter or
> base it off the die ID, but it seems to me much simpler (and
> hopefully less controversial) to stick with the current random
> generation scheme, but allow the user to change the address.
> 
> This patch does exactly that - it moves the random address
> generation from smsc95xx_reset() into smsc95xx_bind(), so that it is
> done once on module load, not on every ifup. The user can then
> override this using the standard mechanisms.
> 
> Applies against 2.6.35 and linux-2.6 head.
> 
> Signed-off-by: Bernard Blackham 

Applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Balbi]

hi,

On Thu, Oct 21, 2010 at 04:04:27AM -0500, Felipe Contreras wrote:

wouldn't a user expect USB to be enabled if the board _has_ a USB
connector ?


Not if it's not going to be used.


and how would you know before hand if it's going to be used or not ?


1) Do you think all the OMAP3 boards should add that?


why not ?


You prefer a patch that adds one line per board rather than a patch
that adds one line in total... That is exactly the opposite of Linus's
complaint regarding big ARM changesets.


The thing is that what you are proposing will not scale when we have too
many boards with different transceivers. Then we will end up with things
like:

config USB_ARCH_HAS_OHCI
boolean
# ARM:
default y if SA
default y if ARCH_OMAP
default y if ARCH_LH7A404
default y if ARCH_S3C2410
default y if PXA27x
default y if PXA3xx
default y if ARCH_EP93XX
default y if ARCH_AT91
default y if ARCH_PNX4008 && I2C
default y if MFD_TC6393XB
default y if ARCH_W90X900
default y if ARCH_DAVINCI_DA8XX
# PPC:
default y if STB03xxx
default y if PPC_MPC52xx
# MIPS:
default y if MIPS_ALCHEMY
default y if MACH_JZ4740
# SH:
default y if CPU_SUBTYPE_SH7720
default y if CPU_SUBTYPE_SH7721
default y if CPU_SUBTYPE_SH7763
default y if CPU_SUBTYPE_SH7786
# more:
default PCI

which to me would be much better that e.g. SA does a select
USB_ARCH_HAS_OHCI like it's done for HAVE_CLK

--
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] omap: am3517: fix build break

[Felipe Balbi]

On Thu, Oct 21, 2010 at 04:20:06AM -0500, Gadiyar, Anand wrote:

Patch usb-am35x-add-musb-support.patch in greg's USB queue
adds backan include of . This file was renamed
by another commit in the omap tree and the file was already
fixed to reflect this.

Remove this include to fix up a build break.

Signed-off-by: Anand Gadiyar 
Cc: Tony Lindgren 
Cc: Ajay Kumar Gupta 
Cc: Felipe Balbi 


Acked-by: Felipe Balbi 

--
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Contreras]

Hi,

On Thu, Oct 21, 2010 at 1:24 PM, Felipe Balbi  wrote:
> On Thu, Oct 21, 2010 at 04:04:27AM -0500, Felipe Contreras wrote:
>>>
>>> wouldn't a user expect USB to be enabled if the board _has_ a USB
>>> connector ?
>>
>> Not if it's not going to be used.
>
> and how would you know before hand if it's going to be used or not ?

You don't, that's what Kconfig is for: so the user can dynamically
select what he wants or not on the kernel. And in order for it to be
useful, dependencies have to be correct.

 1) Do you think all the OMAP3 boards should add that?
>>>
>>> why not ?
>>
>> You prefer a patch that adds one line per board rather than a patch
>> that adds one line in total... That is exactly the opposite of Linus's
>> complaint regarding big ARM changesets.
>
> The thing is that what you are proposing will not scale when we have too
> many boards with different transceivers. Then we will end up with things
> like:
>
> config USB_ARCH_HAS_OHCI

[...]

We are not talking about USB_ARCH_HAS_OHCI, we are talking about
TWL4030_USB, there's nothing to scale there.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] drivers: misc: add omap_hwspinlock driver

[Arnd Bergmann]

On Thursday 21 October 2010, Ohad Ben-Cohen wrote:
> The change is also pretty trivial:
> 
> * move the internal locking implementation to raw_ methods
> * the raw_ methods would save the current interrupt state only if
> given a placeholder
> * wrap those raw_ methods with the desired API (but only support the
> _irq and _irqsave flavors)
> 

Sounds good!

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 3/3] omap: add hwspinlock device

[Kanigeri, Hari]

Mugdha,

> > >>
> > >> This does not require any smart IPC and it will allow us to get
> > rid of
> > >> the omap_hwspinlock_request_specific() API and its early-callers
> > >> requirement.
> > >
> > > Yes, that would indeed simplify things.
> >
> > Balaji, Nishant, are you OK with this ?
> >
> The problem with this approach is that the i2c driver would have to
> sync up on the shared memory location that it uses to share the
> information of the spinlock ID. What location would this be? How would
> the i2c driver on the m3 know about this location? Does this mean
> hard-coding of the shared memory address? If yes, then there would be
> an impact to users if they wanted to change their memory map. Any kind
> of hard-coding of this sort can be painful in such cases, especially
> if it happens in multiple drivers. On the other hand, hard-coding
> (reserving) spinlock IDs is a much safer option and does not impact
> anything else.
> 

We can avoid the hard-coding of shared memory location if I2C Driver maps using 
iommu some dynamic memory for shared memory location to M3 virtual address and 
shares this information to I2c driver counter-part on M3 using the IPC call. 
But this might not be trivial and this would be against what Ohad mentioned 
about not requiring any smart IPC :).
I prefer hard-coding of spinlock id to keep things simple.

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] OMAP: DSS2: Add NEC NL8048HL11-01B display panel

[Samreen]

From: Erik Gilling 

NEC WVGA LCD NL8048HL11-01B panel support has been added.
This panel is being used in zoom2/zoom3/3630 sdp boards.

Signed-off-by: Mukund Mittal 
Signed-off-by: Rajkumar N 
Signed-off-by: Samreen 
CC: Subbu Venkatesh 
CC: Erik Gilling 
---
 Version2:
- the brightness updation is removed from the nec_8048_panel_enable()
to assure the user set brightness value is not over written
- error checking added where ever missing
- cosmetic changes are also included

 This panel driver has orginated from panel-zoom2 from the L25.x 
 releases.Refer Commit:
 
http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=75fc121865125d9e3ca3b36f1f19f86b394e1f6b

 drivers/video/omap2/displays/Kconfig   |7 +
 drivers/video/omap2/displays/Makefile  |1 +
 .../omap2/displays/panel-nec-nl8048hl11-01b.c  |  379 
 3 files changed, 387 insertions(+), 0 deletions(-)
 create mode 100644 drivers/video/omap2/displays/panel-nec-nl8048hl11-01b.c

diff --git a/drivers/video/omap2/displays/Kconfig 
b/drivers/video/omap2/displays/Kconfig
index 12327bb..f8152cf 100644
--- a/drivers/video/omap2/displays/Kconfig
+++ b/drivers/video/omap2/displays/Kconfig
@@ -20,6 +20,13 @@ config PANEL_SHARP_LQ043T1DG01
 help
   LCD Panel used in TI's OMAP3517 EVM boards
 
+config PANEL_NEC_NL8048HL11_01B
+   tristate "NEC NL8048HL11-01B Panel"
+   depends on OMAP2_DSS
+   help
+   This NEC NL8048HL11-01B panel is TFT LCD
+   used in the Zoom2/3/3630 sdp boards.
+
 config PANEL_TAAL
 tristate "Taal DSI Panel"
 depends on OMAP2_DSS_DSI
diff --git a/drivers/video/omap2/displays/Makefile 
b/drivers/video/omap2/displays/Makefile
index aa38609..8ece91c 100644
--- a/drivers/video/omap2/displays/Makefile
+++ b/drivers/video/omap2/displays/Makefile
@@ -1,6 +1,7 @@
 obj-$(CONFIG_PANEL_GENERIC) += panel-generic.o
 obj-$(CONFIG_PANEL_SHARP_LS037V7DW01) += panel-sharp-ls037v7dw01.o
 obj-$(CONFIG_PANEL_SHARP_LQ043T1DG01) += panel-sharp-lq043t1dg01.o
+obj-$(CONFIG_PANEL_NEC_NL8048HL11_01B) += panel-nec-nl8048hl11-01b.o
 
 obj-$(CONFIG_PANEL_TAAL) += panel-taal.o
 obj-$(CONFIG_PANEL_TOPPOLY_TDO35S) += panel-toppoly-tdo35s.o
diff --git a/drivers/video/omap2/displays/panel-nec-nl8048hl11-01b.c 
b/drivers/video/omap2/displays/panel-nec-nl8048hl11-01b.c
new file mode 100644
index 000..0327731
--- /dev/null
+++ b/drivers/video/omap2/displays/panel-nec-nl8048hl11-01b.c
@@ -0,0 +1,379 @@
+/*
+ * Support for NEC-nl8048hl11-01b panel driver
+ *
+ * Copyright (C) 2010 Texas Instruments Inc.
+ * Author: Erik Gilling 
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#define LCD_XRES   800
+#define LCD_YRES   480
+/*
+ * NEC PIX Clock Ratings
+ * MIN:21.8MHz TYP:23.8MHz MAX:25.7MHz
+ */
+#define LCD_PIXEL_CLOCK23800
+
+struct nec_8048_data {
+   struct backlight_device *bl;
+};
+
+/*
+ * NEC NL8048HL11-01B  Manual
+ * defines HFB, HSW, HBP, VFP, VSW, VBP as shown below
+ */
+
+static struct omap_video_timings nec_8048_panel_timings = {
+   /* 800 x 480 @ 60 Hz  Reduced blanking VESA CVT 0.31M3-R */
+   .x_res  = LCD_XRES,
+   .y_res  = LCD_YRES,
+   .pixel_clock= LCD_PIXEL_CLOCK,
+   .hfp= 6,
+   .hsw= 1,
+   .hbp= 4,
+   .vfp= 3,
+   .vsw= 1,
+   .vbp= 4,
+};
+
+static int nec_8048_bl_update_status(struct backlight_device *bl)
+{
+   struct omap_dss_device *dssdev = dev_get_drvdata(&bl->dev);
+   int level;
+
+   if (!dssdev->set_backlight)
+   return -EINVAL;
+
+   if (bl->props.fb_blank == FB_BLANK_UNBLANK &&
+   bl->props.power == FB_BLANK_UNBLANK)
+   level = bl->props.brightness;
+   else
+   level = 0;
+
+   return dssdev->set_backlight(dssdev, level);
+}
+
+static int nec_8048_bl_get_brightness(struct backlight_device *bl)
+{
+   if (bl->props.fb_blank == FB_BLANK_UNBLANK &&
+   bl->props.power == FB_BLANK_UNBLANK)
+   return bl->props.brightness;
+
+   return 0;
+}
+
+static const struct backlight_ops nec_8048_bl_ops = {
+   .get_brightness = nec_8048_bl_get_brightness,
+   .update_status  = nec_8048_bl_update_stat

Re: [PATCH V2] OMAP: DSS2: don't power off a panel twice

[Tomi Valkeinen]

Hi,

On Fri, 2010-09-03 at 04:52 +0200, ext stanley.miao wrote:
> Hi, Tomi,
> 
> Tomi Valkeinen wrote:
> > 
> >
> > Otherwise this looks fine, except that panel-taal.c does not need
> > modifications, as it already handles this case.
> >   
> 
> I will send a V3 to remove the panel-taal.c part.
> 
> > Also, at some point I (or somebody else =) should think how to do proper
> > locking for the panel drivers. Currently it's rather broken, and, for
> > example, enabling and disabling a panel at the same time will cause
> > problems. Except for panel-taal, which uses its own lock.
> >   
> 
> First, I don't think there is any occasion where you need to enable and 
> disable a panel at the same time.

No, but that doesn't mean that enable and disable won't be called at the
same time.

> If so, what kind of result do you want ? enabled or disabled ?

Enabled or disabled, but not crashing or bugging display.

> Second, now the dssdev->state can do the lock job properly. It can 
> ensure only one function can run if both disable()
> and enable are called at the same time.

It doesn't do any kind of locking, it just checks the variable. enable
and disable can still be run at the same time.

 Tomi


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3] OMAP: DSS2: don't power off a panel twice

[Tomi Valkeinen]

On Thu, 2010-10-21 at 11:02 +0200, ext Grazvydas Ignotas wrote:
> Tomi can you queue this? This has been tested by multiple people now,
> also see here:
> http://marc.info/?l=linux-omap&m=128759415821646&w=2

Yes, I seem to have missed/forgotten this. I'll add it to my tree.

 Tomi


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] OMAP: DSS2: Add NEC NL8048HL11-01B display panel

[Grazvydas Ignotas]

On Thu, Oct 21, 2010 at 3:25 PM, Samreen  wrote:
> From: Erik Gilling 
>
> NEC WVGA LCD NL8048HL11-01B panel support has been added.
> This panel is being used in zoom2/zoom3/3630 sdp boards.
>



> diff --git a/drivers/video/omap2/displays/panel-nec-nl8048hl11-01b.c 
> b/drivers/video/omap2/displays/panel-nec-nl8048hl11-01b.c
> new file mode 100644
> index 000..0327731
> --- /dev/null
> +++ b/drivers/video/omap2/displays/panel-nec-nl8048hl11-01b.c



> +static int spi_send(struct spi_device *spi, unsigned char reg_addr,
> +                       unsigned char reg_data)

you could be more consistent and name it nec_8048_spi_send.

> +{
> +       int ret = 0;
> +       unsigned int cmd = 0, data = 0;
> +
> +       cmd = 0x | reg_addr; /* register address write */
> +       data = 0x0100 | reg_data ; /* register data write */
> +       data = (cmd << 16) | data;
> +
> +       ret = spi_write(spi, (unsigned char *)&data, 4);
> +       if (ret)
> +               pr_err("error in spi_write %x\n", data);
> +
> +       return ret;
> +}
> +
> +

one blank line is enough

> +static int init_nec_8048_wvga_lcd(struct spi_device *spi)
> +{
> +       /* Initialization Sequence */
> +       /* spi_send(spi, REG, VAL) */
> +       spi_send(spi, 3, 0x01);
> +       spi_send(spi, 0, 0x00);
> +       spi_send(spi, 1, 0x01);    /* R1 = 0x01 (normal), 0x03 (reversed) */
> +       spi_send(spi, 4, 0x00);
> +       spi_send(spi, 5, 0x14);
> +       spi_send(spi, 6, 0x24);

This code wastes quite a bit of space (at least 3 mov and one bl
instruction for every such write on ARM), you could better do
something like:

static const struct {
  u8 address;
  u8 value;
} nec_8048_init_seq[] = {
  { 3, 0x01 },
  { 0, 0x00 },
  { 1, 0x01 },
  ...
};

static int init_nec_8048_wvga_lcd(struct spi_device *spi)
{
  int i, ret;
  for (i = 0; i < ARRAY_SIZE(nec_8048_init_seq); i++) {
ret = nec_8048_spi_send(spi, nec_8048_init_seq[i].address,
nec_8048_init_seq[i].value);
if (ret < 0)
  break;
  }
  udelay(20);
  nec_8048_spi_send(spi, 2, 0x00);
}


> +MODULE_DESCRIPTION("Nec Driver");

could use more complete description here.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Roger Quadros]

Hi,

On 10/20/2010 05:38 PM, ext Felipe Contreras wrote:

On Wed, Oct 20, 2010 at 3:33 PM, Roger Quadros  wrote:

On 10/20/2010 12:23 PM, ext Felipe Contreras wrote:

On Wed, Oct 20, 2010 at 12:14 PM, Roger Quadros
  wrote:

USB_G_NOKIA just needs a USB gadget controller to work. The gadget
controller used for the board should come from the board's Kconfig which
will ideally be supplied by the board's vendor.


Ok, but USB_OMAP is not supposed to work on the N900? (I tried and it
didn't)


No it won't work. USB_OMAP was for older OMAP's. MUSB is on OMAP3 and later.
But MUSB is not limited to OMAP SoC.


config USB_GADGET_OMAP
   depends ARCH_OMAP

So how about s/ARCH_OMAP/ARCH_OMAP1/

Then USB_GADGET_MUSB_HDRC would be selected by default on OMAP2+.



But I'm not sure that omap_udc is used only for OMAP1 architecture.
Dave/Tony should confirm this.


--
regards,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] OMAP: hmwod: Update the sysc_cache in case module context is lost

[Cousson, Benoit]

On 10/21/2010 12:13 PM, Nayak, Rajendra wrote:




Hi Kevin,

What the patch does is reprogram's the sysc value (from the cache)
whenever its lost. So its infact saved in the cache and restored when
needed.



Otherwise, all this patch does is refresh the _sysc_cache with
completely unknown contents.  It also somewhat defeats the purpose of
having a cache.  If you're going to read SYSC in order to determine
whether or not you can avoid a write, you might as well just blindly
write.


I thought of this and dismissed it thinking I would end up with a read/or/write
and instead a read always to avoid write is better.
But now looking back again, it does make sense to still keep the cache to avoid
a read (since a read has significantly more latency than write) and do a blind 
write
always. Does that make sense?


That seems indeed better. The point is that cache is already a location 
for the "save" part.

So writing blindly will do the restore with always the good value.
It is anyway faster than trying to check if we lost context or not 
through PRM registers.


Benoit
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Felipe Balbi]

Hi,

On Thu, Oct 21, 2010 at 09:07:59AM -0500, Roger Quadros wrote:

But I'm not sure that omap_udc is used only for OMAP1 architecture.
Dave/Tony should confirm this.


Maybe the old omap2420_h4 board still uses it, but from 2430 I think the
ip isn't even there anymore.

--
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/3] drivers: cleanup Kconfig stuff

[Tony Lindgren]

* Felipe Balbi  [101021 08:32]:
> Hi,
> 
> On Thu, Oct 21, 2010 at 09:07:59AM -0500, Roger Quadros wrote:
> >But I'm not sure that omap_udc is used only for OMAP1 architecture.
> >Dave/Tony should confirm this.
> 
> Maybe the old omap2420_h4 board still uses it, but from 2430 I think the
> ip isn't even there anymore.

I don't think it's currently working on 2420. But still, the drivers
should be platform independent. All the drivers that might be possible
should at least build and not mess up things for other platforms.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: linux-next - multi-omap image fails to boot on omap3/4

[Tony Lindgren]

* Gadiyar, Anand  [101019 23:21]:
> On Wed, Oct 20, 2010 at 5:16 AM, Gadiyar, Anand  wrote:
> > On Wed, Oct 20, 2010 at 5:02 AM, Tony Lindgren  wrote:
> >> * Tony Lindgren  [101019 15:48]:
> >>> * Gadiyar, Anand  [101019 11:26]:
> >>> > On Tue, Oct 19, 2010 at 11:51 PM, Tony Lindgren  
> >>> > wrote:
> >>> > > * Anand Gadiyar  [101019 07:41]:
> >>> > >> Hi all,
> >>> > >>
> >>> > >> linux-next, as of 20101019, built with the omap2plus_defconfig fails
> >>> > >> to boot on omap3 and omap4. (I've disabled CONFIG_ARCH_OMAP2 or
> >>> > >> CONFIG_SWP_EMULATE to get the image to build). Building with only
> >>> > >> ARCH_OMAP3 allows the resultant image to boot up on OMAP3. Likewise,
> >>> > >> an image built with only ARCH_OMAP4 boots up on OMAP4 boards.
> >>> > >>
> >>> > >> earlyprintk does not provide any additional prints after
> >>> > >> "Uncompressing Linux... done, booting the kernel."
> >>> > >>
> >>> > >> Any ideas where to look?
> >>> > >
> >>> > > Hmm I did a quick test merge of linux-omap master and rmk/devel
> >>> > > branches and that boots just fine.
> >>> > >
> >>> > > So it's probably something that already got fixed in rmk/devel
> >>> > > but is not yet in next, or something that came from elsewhere,
> >>> > > or something that we have in omap-testing branch that's not in
> >>> > > for-next for some reason.
> >>> >
> >>> > I tried bisecting linux-next between v2.6.36-rc8 and HEAD, but
> >>> > there's a commit in between that breaks the build. I'll take another
> >>> > stab at this in a while.
> >>>
> >>> Looks like current next at 80f8f1f8b33750d954beb386c0c8142d0c01c25c
> >>> boots again except on 2430sdp it produces a NULL pointer at ubi_io_write.
> >>
> >> Oops, sorry it's still broken, wrong tree.
> >>
> >
> > I've started bisecting again - I needed to pick commit 5bac0926121e
> > (driver core: platform_bus: allow runtime override of dev_pm_ops) to
> > solve the build break.
> >
> > Will report back if I find something.
> >
> 
> I tried disabling CONFIG_SMP and CONFIG_SMP_ON_UP, and the resulting
> image booted up just fine.

Looks like it's caused by this patch:

"ARM: hotplug cpu: Keep processor information, startup code & 
__lookup_processor_type"

It's probably trashing registers it shouldn't. Will take a look and post
something to linux-arm-kernel.

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] OMAP: hmwod: Update the sysc_cache in case module context is lost

[Kevin Hilman]

"Nayak, Rajendra"  writes:

>> -Original Message-
>> From: Kevin Hilman [mailto:khil...@deeprootsystems.com]
>> Sent: Friday, October 15, 2010 9:10 PM
>> To: Shilimkar, Santosh
>> Cc: Nayak, Rajendra; linux-omap@vger.kernel.org; Paul Walmsley; Cousson, 
>> Benoit
>> Subject: Re: [PATCH] OMAP: hmwod: Update the sysc_cache in case module 
>> context is lost
>> 
>> "Shilimkar, Santosh"  writes:
>> 
>> >> -Original Message-
>> >> From: Kevin Hilman [mailto:khil...@deeprootsystems.com]
>> >> Sent: Friday, October 15, 2010 3:44 AM
>> >> To: Nayak, Rajendra
>> >> Cc: linux-omap@vger.kernel.org; Paul Walmsley; Cousson, Benoit; Shilimkar,
>> >> Santosh
>> >> Subject: Re: [PATCH] OMAP: hmwod: Update the sysc_cache in case module
>> >> context is lost
>> >>
>> >> Rajendra Nayak  writes:
>> >>
>> >> > Do not skip the sysc programming in the hmwod framework based
>> >> > on the cached value alone, since at times the module might have lost
>> >> > context (due to the Powerdomain in which the module belongs
>> >> > transitions to either Open Switch RET or OFF).
>> >>
>> >> Shouldn't the driver for each IP be responsible for restoring it's
>> >> register contents after context loss, including it's SYSC?
>> >>
>> >> Seems to me that if SYSC is lost, it means the driver's save/restore
>> >> is buggy.
>> >
>> > I am glad you asked this question. I had a same argument with Benoit
>> > that driver anyway does context save restore for other registers and
>> > it can do SYSC as well.
>> >
>> > But Benoit's point was that "sysconfig is a part of the PRCM located
>> > in the IP, but this is purely TI implementation specific. The same
>> > IP in another platform will not have this sysconfig entry. That's why
>> > its important to hide them from the driver "
>> 
>> OK, but this patch still doesn't address the real problem.  Namely, that
>> *somebody* needs to save/restore the SYSC reg for the IP.
>
> Hi Kevin,
>
> What the patch does is reprogram's the sysc value (from the cache)
> whenever its lost. So its infact saved in the cache and restored when 
> needed. 
>
>> 
>> Otherwise, all this patch does is refresh the _sysc_cache with
>> completely unknown contents.  It also somewhat defeats the purpose of
>> having a cache.  If you're going to read SYSC in order to determine
>> whether or not you can avoid a write, you might as well just blindly
>> write.
>
> I thought of this and dismissed it thinking I would end up with a 
> read/or/write
> and instead a read always to avoid write is better.
> But now looking back again, it does make sense to still keep the cache to 
> avoid
> a read (since a read has significantly more latency than write) and do a 
> blind write
> always. Does that make sense?

Makes sense to me.

Kevin

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 2/2] OMAP3: CPUidle: trigger early idle notification call chain

[Kevin Hilman]

"Sripathy, Vishwanath"  writes:

>> 
>> During the early part of the CPUidle path (state selection by the
>> governer, checking for device activity, etc.) there is no (good)
>> reason to have interrupts disabled.
>> 
>> Therefore, enable interrupts early in the CPUidle path and then
>> trigger the "early" idle notifier call chain after device activity
>> checks and the next power states have been programmed.  Interrupts are
>> then (re)disabled after the final state is selected.
>> 
>> Signed-off-by: Kevin Hilman 
>> ---
>>  arch/arm/mach-omap2/cpuidle34xx.c |   27 --
>> -
>>  1 files changed, 24 insertions(+), 3 deletions(-)
>> 
>>
>> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-
>> omap2/cpuidle34xx.c
>> index 0d50b45..8c57360 100644
>> --- a/arch/arm/mach-omap2/cpuidle34xx.c
>> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
>> @@ -30,6 +30,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>> 
>>  #include "pm.h"
>>  #include "control.h"
>> @@ -128,12 +129,14 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
>>  /* Used to keep track of the total time in idle */
>>  getnstimeofday(&ts_preidle);
>> 
>> -local_irq_disable();
>> -local_fiq_disable();
>> -
>>  pwrdm_set_next_pwrst(mpu_pd, mpu_state);
>>  pwrdm_set_next_pwrst(core_pd, core_state);
>> 
>> +omap_early_idle_notifier_start();
>> +
>> +local_irq_disable();
>> +local_fiq_disable();
>> +
>>  if (omap_irq_pending() || need_resched())
>>  goto return_sleep_time;
>> 
>> @@ -157,6 +160,8 @@ return_sleep_time:
>>  local_irq_enable();
>>  local_fiq_enable();
>> 
>> +omap_early_idle_notifier_end();

> It appears that idle notifiers (for restore path) are called after
> getnstimeofday is called which means time spent in idle callbacks are
> not accounted in cpuidle time.  Will it not impact the cpuidle c state
> prediction and latency computation?

You're right.

The current patch includes the pre-idle notifiers, but not the post-idle
notifiers.  We should either include both or exclude both in the timing
but not do one or the other.

I will post and updated version which includes both in the timing
measurement.

But you've hit on something that I'm not too crazy about, and that
Rajendra raised earlier as well, and also why this series is still RFC.

Not only does the CPUidle timing measurement now measure the idle
callbacks, because interrupts are enabled, it will also measure any
other interrupt processing and/or scheduling that might happend because
of the notifiers.

At LPC this year, we'll hopefully be discussing better ways to decouple
CPU idle states and device idle states, and hopefully come up with some
better options here.  Until then, I think this approach is a good
compromise.

Kevin




--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 2/2] OMAP3: CPUidle: trigger early idle notification call chain

[Kevin Hilman]

[updated version of the patch, fixing issues raised by Vishwa]

>From 1661b0f7ad614d221f90eed590040f2cca01c265 Mon Sep 17 00:00:00 2001
From: Kevin Hilman 
Date: Thu, 23 Sep 2010 09:54:41 -0700
Subject: [PATCH] OMAP3: CPUidle: trigger early idle notification call chain

During the early part of the CPUidle path (state selection by the
governer, checking for device activity, etc.) there is no (good)
reason to have interrupts disabled.

Therefore, enable interrupts early in the CPUidle path and then
trigger the "early" idle notifier call chain after device activity
checks and the next power states have been programmed.  Interrupts are
then (re)disabled after the final state is selected.

Also adjust the timing measurements reported back to the CPUidle core
to ensure that time spent in the idle notifiers is included as part of
the accounted time.

Signed-off-by: Kevin Hilman 
---
 arch/arm/mach-omap2/cpuidle34xx.c |   33 +++--
 1 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c 
b/arch/arm/mach-omap2/cpuidle34xx.c
index 0d50b45..6c806a8 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "pm.h"
 #include "control.h"
@@ -128,12 +129,14 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
/* Used to keep track of the total time in idle */
getnstimeofday(&ts_preidle);
 
-   local_irq_disable();
-   local_fiq_disable();
-
pwrdm_set_next_pwrst(mpu_pd, mpu_state);
pwrdm_set_next_pwrst(core_pd, core_state);
 
+   omap_early_idle_notifier_start();
+
+   local_irq_disable();
+   local_fiq_disable();
+
if (omap_irq_pending() || need_resched())
goto return_sleep_time;
 
@@ -151,12 +154,14 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
}
 
 return_sleep_time:
-   getnstimeofday(&ts_postidle);
-   ts_idle = timespec_sub(ts_postidle, ts_preidle);
-
local_irq_enable();
local_fiq_enable();
 
+   omap_early_idle_notifier_end();
+
+   getnstimeofday(&ts_postidle);
+   ts_idle = timespec_sub(ts_postidle, ts_preidle);
+
return ts_idle.tv_nsec / NSEC_PER_USEC + ts_idle.tv_sec * USEC_PER_SEC;
 }
 
@@ -459,6 +464,21 @@ struct cpuidle_driver omap3_idle_driver = {
.owner =THIS_MODULE,
 };
 
+static int omap3_idle_prepare(struct cpuidle_device *dev)
+{
+   /*
+* Enable interrupts during the early part of the CPUidle path.
+* They are later (re)disabled when the final state is selected.
+*
+* The primary reason for this is to enable non-atomic idle
+* notifications to drivers that want to coordinate device
+* idle transitions with CPU idle transitions.
+*/
+   local_irq_enable();
+
+   return 0;
+}
+
 /**
  * omap3_idle_init - Init routine for OMAP3 idle
  *
@@ -482,6 +502,7 @@ int __init omap3_idle_init(void)
 
dev = &per_cpu(omap3_idle_dev, smp_processor_id());
 
+   dev->prepare = omap3_idle_prepare;
for (i = OMAP3_STATE_C1; i < OMAP3_MAX_STATES; i++) {
cx = &omap3_power_states[i];
state = &dev->states[count];
-- 
1.7.2.1

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] arm: omap: counter-32k: convert to pm_runtime API

[Kevin Hilman]

Felipe Balbi  writes:

> Trivial patch removing clock framework and adding
> pm_runtime API.
>
> Signed-off-by: Felipe Balbi 

Minor nit: the runtime PM 'put' calls do not need to be the _sync
versions.  You can easily get by using the "normal" (async) versions
here.

Kevin


> ---
>  arch/arm/plat-omap/counter-32k.c |   48 ++---
>  1 files changed, 13 insertions(+), 35 deletions(-)
>
> diff --git a/arch/arm/plat-omap/counter-32k.c 
> b/arch/arm/plat-omap/counter-32k.c
> index f3fcb38..7bfd67a 100644
> --- a/arch/arm/plat-omap/counter-32k.c
> +++ b/arch/arm/plat-omap/counter-32k.c
> @@ -27,7 +27,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  #include 
>  
>  struct omap_counter_32k_device {
> @@ -37,7 +37,6 @@ struct omap_counter_32k_device {
>   cycles_tlast_cycles;
>  
>   struct device   *dev;
> - struct clk  *ick;
>   void __iomem*base;
>  
>   /*
> @@ -117,7 +116,6 @@ static int __init omap_counter_32k_probe(struct 
> platform_device *pdev)
>  {
>   struct omap_counter_32k_device  *omap;
>   struct resource *res;
> - struct clk  *ick;
>  
>   int ret;
>  
> @@ -130,11 +128,18 @@ static int __init omap_counter_32k_probe(struct 
> platform_device *pdev)
>   goto err0;
>   }
>  
> + pm_runtime_enable(&pdev->dev);
> + ret = pm_runtime_get_sync(&pdev->dev);
> + if (ret) {
> + dev_dbg(&pdev->dev, "unable to enable runtime pm\n");
> + goto err1;
> + }
> +
>   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   if (!res) {
>   dev_dbg(&pdev->dev, "couldn't get resource\n");
>   ret = -ENODEV;
> - goto err1;
> + goto err2;
>   }
>  
>   base = ioremap(res->start, resource_size(res));
> @@ -144,22 +149,8 @@ static int __init omap_counter_32k_probe(struct 
> platform_device *pdev)
>   goto err2;
>   }
>  
> - ick = clk_get(&pdev->dev, "ick");
> - if (IS_ERR(ick)) {
> - dev_dbg(&pdev->dev, "couldn't get clock\n");
> - ret = PTR_ERR(ick);
> - goto err3;
> - }
> -
> - ret = clk_enable(ick);
> - if (ret) {
> - dev_dbg(&pdev->dev, "couldn't enable clock\n");
> - goto err4;
> - }
> -
>   omap->base  = base;
>   omap->dev   = &pdev->dev;
> - omap->ick   = ick;
>  
>   omap->cs.name   = "counter-32k";
>   omap->cs.rating = 250;
> @@ -174,7 +165,7 @@ static int __init omap_counter_32k_probe(struct 
> platform_device *pdev)
>   ret = clocksource_register(&omap->cs);
>   if (ret) {
>   dev_dbg(&pdev->dev, "failed to register clocksource\n");
> - goto err5;
> + goto err3;
>   }
>  
>   /* initialize our offset */
> @@ -190,16 +181,12 @@ static int __init omap_counter_32k_probe(struct 
> platform_device *pdev)
>  
>   return 0;
>  
> -err5:
> - clk_disable(ick);
> -
> -err4:
> - clk_put(ick);
> -
>  err3:
>   iounmap(base);
>  
>  err2:
> + pm_runtime_put_sync(&pdev->dev);
> +
>  err1:
>   kfree(omap);
>  
> @@ -211,9 +198,8 @@ static int __exit omap_counter_32k_remove(struct 
> platform_device *pdev)
>  {
>   struct omap_counter_32k_device  *omap = platform_get_drvdata(pdev);
>  
> + pm_runtime_put_sync(&pdev->dev);
>   clocksource_unregister(&omap->cs);
> - clk_disable(omap->ick);
> - clk_put(omap->ick);
>   iounmap(omap->base);
>   kfree(omap);
>   platform_set_drvdata(pdev, NULL);
> @@ -221,16 +207,8 @@ static int __exit omap_counter_32k_remove(struct 
> platform_device *pdev)
>   return 0;
>  }
>  
> -static void omap_counter_32k_shutdown(struct platform_device *pdev)
> -{
> - struct omap_counter_32k_device  *omap = platform_get_drvdata(pdev);
> -
> - clk_disable(omap->ick);
> -}
> -
>  static struct platform_driver omap_counter_32k_driver = {
>   .remove = __exit_p(omap_counter_32k_remove),
> - .shutdown   = omap_counter_32k_shutdown,
>   .driver = {
>   .name   = "omap-counter-32k",
>   },
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] arm: omap: introduce 32k timer hwmod for omap2/3/4

[Kevin Hilman]

Felipe Balbi  writes:

> Add 32k timer hwmod to the database.
>
> Signed-off-by: Felipe Balbi 

Not sure how this is working correctly on OMAP2 and OMAP3.  All the
hwmods are mising the oh->prcm.omap2.module_offs field.

Without this, _wait_target_ready *should* fail, and the hwmod should not
actually be enabled.

Since this was tested to work, I guess what's happening, is because
module_offs == 0 (OCP_MOD), it's reading from the IDLEST register offset
in OCP_MOD, which is an undefined register.  On 34xx, we get lucky that
that bit is zero so omap2_cm_wait_module_ready succeeds.  On 24xx, the
polarity of the idlest bits is inversed, so this would likely fail on
OMAP2.

Either way, the right fix for this is to ensure that OMAP2/3 hwmods have
.module_offs populated correctly.

Kevin


> ---
>  arch/arm/mach-omap2/omap_hwmod_2420_data.c |   52 +++
>  arch/arm/mach-omap2/omap_hwmod_2430_data.c |   52 +++
>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   51 +++
>  arch/arm/mach-omap2/omap_hwmod_44xx_data.c |   61 
> 
>  4 files changed, 216 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod_2420_data.c 
> b/arch/arm/mach-omap2/omap_hwmod_2420_data.c
> index a1a3dd6..05b9d2a 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_2420_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_2420_data.c
> @@ -557,6 +557,57 @@ static struct omap_hwmod omap2420_i2c2_hwmod = {
>   .flags  = HWMOD_16BIT_REG,
>  };
>  
> +/*
> + * 'counter' class
> + * 32-bit ordinary counter, clocked by the falling edge of the 32 khz clock
> + */
> +
> +static struct omap_hwmod_class omap2420_counter_hwmod_class = {
> + .name = "counter",
> +};
> +
> +/* counter_32k */
> +static struct omap_hwmod omap2420_counter_32k_hwmod;
> +static struct omap_hwmod_addr_space omap2420_counter_32k_addrs[] = {
> + {
> + .pa_start   = 0x48004000,
> + .pa_end = 0x48000fff,
> + .flags  = ADDR_TYPE_RT
> + },
> +};
> +
> +/* l4_wkup -> counter_32k */
> +static struct omap_hwmod_ocp_if omap2420_l4_wkup__counter_32k = {
> + .master = &omap2420_l4_wkup_hwmod,
> + .slave  = &omap2420_counter_32k_hwmod,
> + .clk= "l4_ck",
> + .addr   = omap2420_counter_32k_addrs,
> + .addr_cnt   = ARRAY_SIZE(omap2420_counter_32k_addrs),
> + .user   = OCP_USER_MPU | OCP_USER_SDMA,
> +};
> +
> +/* counter_32k slave ports */
> +static struct omap_hwmod_ocp_if *omap2420_counter_32k_slaves[] = {
> + &omap2420_l4_wkup__counter_32k,
> +};
> +
> +static struct omap_hwmod omap2420_counter_32k_hwmod = {
> + .name   = "counter_32k",
> + .class  = &omap2420_counter_hwmod_class,
> + .main_clk   = "sync_32k_ick",
> + .prcm = {
> + .omap2 = {
> + .prcm_reg_id = 1,
> + .module_bit = OMAP24XX_EN_GPT1_SHIFT,
> + .idlest_reg_id = 1,
> + .idlest_idle_bit = OMAP24XX_ST_GPT1_SHIFT,
> + },
> + },
> + .slaves = omap2420_counter_32k_slaves,
> + .slaves_cnt = ARRAY_SIZE(omap2420_counter_32k_slaves),
> + .omap_chip  = OMAP_CHIP_INIT(CHIP_IS_OMAP2420),
> +};
> +
>  static __initdata struct omap_hwmod *omap2420_hwmods[] = {
>   &omap2420_l3_main_hwmod,
>   &omap2420_l4_core_hwmod,
> @@ -569,6 +620,7 @@ static __initdata struct omap_hwmod *omap2420_hwmods[] = {
>   &omap2420_uart3_hwmod,
>   &omap2420_i2c1_hwmod,
>   &omap2420_i2c2_hwmod,
> + &omap2420_counter_32k_hwmod,
>   NULL,
>  };
>  
> diff --git a/arch/arm/mach-omap2/omap_hwmod_2430_data.c 
> b/arch/arm/mach-omap2/omap_hwmod_2430_data.c
> index 7cf0d3a..96e9b12 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_2430_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_2430_data.c
> @@ -569,6 +569,57 @@ static struct omap_hwmod omap2430_i2c2_hwmod = {
>   .omap_chip  = OMAP_CHIP_INIT(CHIP_IS_OMAP2430),
>  };
>  
> +/*
> + * 'counter' class
> + * 32-bit ordinary counter, clocked by the falling edge of the 32 khz clock
> + */
> +
> +static struct omap_hwmod_class omap2430_counter_hwmod_class = {
> + .name = "counter",
> +};
> +
> +/* counter_32k */
> +static struct omap_hwmod omap2430_counter_32k_hwmod;
> +static struct omap_hwmod_addr_space omap2430_counter_32k_addrs[] = {
> + {
> + .pa_start   = 0x48004000,
> + .pa_end = 0x48000fff,
> + .flags  = ADDR_TYPE_RT
> + },
> +};
> +
> +/* l4_wkup -> counter_32k */
> +static struct omap_hwmod_ocp_if omap2430_l4_wkup__counter_32k = {
> + .master = &omap2430_l4_wkup_hwmod,
> + .slave  = &omap2430_counter_32k_hwmod,
> + .clk= "l4_ck",
> + .addr   = omap2430_counter_32k_addrs,
> + .addr_cnt   = ARRAY_SIZE(omap2430_counter_32k_addrs),
> + .user

Re: [PATCH 0/5] 32k sync timer meets hwmod

[Kevin Hilman]

Felipe Balbi  writes:

> Converted 32k-sync timer to platform_driver
> and now using pm_runtime and hwmod.
>
> Tested on 3430 by me and 4430 by Tarun
>
> If someone could test on 2430 and 2420, I would
> be really glad.

Hey, don't you have a 2420 device.  There were some cool 2420-based
tablets a few years ago made by some company in Finland that you may
have heard of.  ;)

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] omap: am3517: fix build break

[Greg KH]

On Thu, Oct 21, 2010 at 02:50:06PM +0530, Anand Gadiyar wrote:
> Patch usb-am35x-add-musb-support.patch in greg's USB queue
> adds backan include of . This file was renamed
> by another commit in the omap tree and the file was already
> fixed to reflect this.
> 
> Remove this include to fix up a build break.
> 
> Signed-off-by: Anand Gadiyar 
> Cc: Tony Lindgren 
> Cc: Ajay Kumar Gupta 
> Cc: Felipe Balbi 
> Cc: Greg Kroah-Hartman 
> ---
> Build break observed with linux-next as of 20101021.
> 
> Greg, okay with me to fold this into the original patch
> if the others in copy agree. It's a trivial enough change.

I've folded it in with the original patch.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 4/7] omap:mailbox-send message in process context

[Sapiens, Rene]

Hi Hari,

On Thursday, October 14, 2010 9:13 PM Kanigeri, Hari wrote:
> Schedule the Tasklet to send only when mailbox fifo is full, else
> send the message in the Process context. This would avoid
> needless scheduling of Tasklet for every message transfer
> 
> Signed-off-by: Hari Kanigeri 
> ---
>  arch/arm/plat-omap/mailbox.c |9 +++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
> index ed960c1..a4170c7 100644
> --- a/arch/arm/plat-omap/mailbox.c
> +++ b/arch/arm/plat-omap/mailbox.c
> @@ -92,20 +92,25 @@ int omap_mbox_msg_send(struct omap_mbox *mbox,
>   mbox_msg_t msg) struct omap_mbox_queue *mq = mbox->txq;
>   int ret = 0, len;
> 
> - spin_lock(&mq->lock);
> + spin_lock_bh(&mq->lock);
> 

Please check if this scenario looks valid to you, as discussed and depicted
with Fernando:

Supposing that at this point the hw fifo is full and there are messages in
the kfifo.

>   if (kfifo_avail(&mq->fifo) < sizeof(msg)) {
>   ret = -ENOMEM;
>   goto out;
>   }
> 

We reach this point.

In this scenario, the DSP or other Core reads a message from the mbox hw fifo.
The next happens:
1.- The not full interrupt is triggered to the MPU.
2.- The mbox's ISR schedules the tasklet to write the message.
3.- The ISR is left and returns to here (since we still have the bh lock the
tasklet won't run).

> + if (!__mbox_poll_for_space(mbox)) {\

4.- We check for mbox_fifo_full and we have space. so the message is written.

> + mbox_fifo_write(mbox, msg);

At this point it looks that the FIFO order is lost. We write this message
before the ones in the kfifo.

A solution for this could be checking if there are messages in the kfifo
before trying to write directly to the hw fifo, if so, just continue
scheduling the tasklet.

> + goto out;
> + }
> +
>   len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
>   WARN_ON(len != sizeof(msg));
> 
>   tasklet_schedule(&mbox->txq->tasklet);
> 
>  out:
> - spin_unlock(&mq->lock);
> + spin_unlock_bh(&mq->lock);
>   return ret;
>  }
>  EXPORT_SYMBOL(omap_mbox_msg_send);
> --
> 1.7.0--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] arm: omap: counter-32k: convert to pm_runtime API

[Felipe Balbi]

On Thu, Oct 21, 2010 at 12:46:11PM -0500, Kevin Hilman wrote:

Felipe Balbi  writes:


Trivial patch removing clock framework and adding
pm_runtime API.

Signed-off-by: Felipe Balbi 


Minor nit: the runtime PM 'put' calls do not need to be the _sync
versions.  You can easily get by using the "normal" (async) versions
here.


Will change for next version, was just following what other drivers were
doing.

--
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] ARM: OMAP: Modified omap_mux_init_signal() to take in const char *

[Tim Nordell]

If one does the following line twice with the old code:

omap_mux_init_signal("uart3_rts_sd.gpio_164", OMAP_PIN_INPUT);
omap_mux_init_signal("uart3_rts_sd.gpio_164", OMAP_PIN_INPUT);

the compiler optimizes the two const strings into one string
internally in the compiler.  The old code would modify the string
by inserting a NULL effectively making the second call become:

omap_mux_init_signal("uart3_rts_sd", OMAP_PIN_INPUT);

Since the code changes _all_ uart3_rts_sd signals over to this,
it'll cause unknown behavior, as well as making it more
difficult to track down the reason since the string
"uart3_rts_sd" by itself may not actually exist in your
normal mux initialization sequence.

Signed-off-by: Tim Nordell 
---
 arch/arm/mach-omap2/mux.c |   13 -
 arch/arm/mach-omap2/mux.h |4 ++--
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-omap2/mux.c b/arch/arm/mach-omap2/mux.c
index ab403b2..b685540 100644
--- a/arch/arm/mach-omap2/mux.c
+++ b/arch/arm/mach-omap2/mux.c
@@ -127,19 +127,21 @@ int __init omap_mux_init_gpio(int gpio, int val)
return 0;
 }
 
-int __init omap_mux_init_signal(char *muxname, int val)
+int __init omap_mux_init_signal(const char *muxname, int val)
 {
struct omap_mux_entry *e;
-   char *m0_name = NULL, *mode_name = NULL;
+   const char *m0_name = NULL, *mode_name = NULL;
int found = 0;
+   int m0_name_len;
 
mode_name = strchr(muxname, '.');
if (mode_name) {
-   *mode_name = '\0';
mode_name++;
m0_name = muxname;
+   m0_name_len = mode_name - muxname - 1;
} else {
mode_name = muxname;
+   m0_name_len = strlen(muxname);
}
 
list_for_each_entry(e, &muxmodes, node) {
@@ -147,7 +149,8 @@ int __init omap_mux_init_signal(char *muxname, int val)
char *m0_entry = m->muxnames[0];
int i;
 
-   if (m0_name && strcmp(m0_name, m0_entry))
+   if (m0_name && (strncmp(m0_name, m0_entry, m0_name_len)
+   || strlen(m0_entry) != m0_name_len))
continue;
 
for (i = 0; i < OMAP_MUX_NR_MODES; i++) {
@@ -164,7 +167,7 @@ int __init omap_mux_init_signal(char *muxname, int val)
mux_mode = val | i;
printk(KERN_DEBUG "mux: Setting signal "
"%s.%s 0x%04x -> 0x%04x\n",
-   m0_entry, muxname, old_mode, mux_mode);
+   m0_entry, mode_cur, old_mode, mux_mode);
omap_mux_write(mux_mode, m->reg_offset);
found++;
}
diff --git a/arch/arm/mach-omap2/mux.h b/arch/arm/mach-omap2/mux.h
index a8e040c..55e3a21 100644
--- a/arch/arm/mach-omap2/mux.h
+++ b/arch/arm/mach-omap2/mux.h
@@ -120,7 +120,7 @@ int omap_mux_init_gpio(int gpio, int val);
  * @muxname:   Mux name in mode0_name.signal_name format
  * @val:   Options for the mux register value
  */
-int omap_mux_init_signal(char *muxname, int val);
+int omap_mux_init_signal(const char *muxname, int val);
 
 #else
 
@@ -128,7 +128,7 @@ static inline int omap_mux_init_gpio(int gpio, int val)
 {
return 0;
 }
-static inline int omap_mux_init_signal(char *muxname, int val)
+static inline int omap_mux_init_signal(const char *muxname, int val)
 {
return 0;
 }
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] arm: omap: introduce 32k timer hwmod for omap2/3/4

[Felipe Balbi]

On Thu, Oct 21, 2010 at 12:57:41PM -0500, Kevin Hilman wrote:

Felipe Balbi  writes:


Add 32k timer hwmod to the database.

Signed-off-by: Felipe Balbi 


Not sure how this is working correctly on OMAP2 and OMAP3.  All the
hwmods are mising the oh->prcm.omap2.module_offs field.

Without this, _wait_target_ready *should* fail, and the hwmod should not
actually be enabled.

Since this was tested to work, I guess what's happening, is because
module_offs == 0 (OCP_MOD), it's reading from the IDLEST register offset
in OCP_MOD, which is an undefined register.  On 34xx, we get lucky that
that bit is zero so omap2_cm_wait_module_ready succeeds.  On 24xx, the
polarity of the idlest bits is inversed, so this would likely fail on
OMAP2.

Either way, the right fix for this is to ensure that OMAP2/3 hwmods have
.module_offs populated correctly.


I'll look again but when I was reading omap3 TRM I couldn't find IDLEST
for this module, maybe I missed something.

--
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] 32k sync timer meets hwmod

[Felipe Balbi]

On Thu, Oct 21, 2010 at 01:00:19PM -0500, Kevin Hilman wrote:

Felipe Balbi  writes:


Converted 32k-sync timer to platform_driver
and now using pm_runtime and hwmod.

Tested on 3430 by me and 4430 by Tarun

If someone could test on 2430 and 2420, I would
be really glad.


Hey, don't you have a 2420 device.  There were some cool 2420-based
tablets a few years ago made by some company in Finland that you may
have heard of.  ;)


I have an n810 which doesn't work :-p sucks to be me :-p

--
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: OMAP: Modified omap_mux_init_signal() to take in const char *

[Tim Nordell]

 On 10/21/10 13:50, Tim Nordell wrote:
> If one does the following line twice with the old code:
>
> omap_mux_init_signal("uart3_rts_sd.gpio_164", OMAP_PIN_INPUT);
> omap_mux_init_signal("uart3_rts_sd.gpio_164", OMAP_PIN_INPUT);
>
> the compiler optimizes the two const strings into one string
> internally in the compiler.  The old code would modify the string
> by inserting a NULL effectively making the second call become:
>
> omap_mux_init_signal("uart3_rts_sd", OMAP_PIN_INPUT);
>
> Since the code changes _all_ uart3_rts_sd signals over to this,
> it'll cause unknown behavior, as well as making it more
> difficult to track down the reason since the string
> "uart3_rts_sd" by itself may not actually exist in your
> normal mux initialization sequence.
>

Gah.  I just realized there was a patch in the linux-omap tree to do the
same thing.

http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=commitdiff;h=5a3b2f7a5a79082dd3a5f2294cbd85fc3b173d98;hp=7ad0e386d46e9edff64705ab25337ad9130baf63

It looks like by inspection that there could be a couple of things wrong
with that patch however.  Namely, on the comparison of muxname to
m0_entry, if they have the same string up to mode0_len, such as
"dss_data1" and "dss_data12" it'd match there prematurely as it'd stop
comparing.  Also, a minor display issue on the printk of the new muxname.

- Tim
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] arm: omap: counter-32k: convert to pm_runtime API

[Kevin Hilman]

Felipe Balbi  writes:

> On Thu, Oct 21, 2010 at 12:46:11PM -0500, Kevin Hilman wrote:
>>Felipe Balbi  writes:
>>
>>> Trivial patch removing clock framework and adding
>>> pm_runtime API.
>>>
>>> Signed-off-by: Felipe Balbi 
>>
>>Minor nit: the runtime PM 'put' calls do not need to be the _sync
>>versions.  You can easily get by using the "normal" (async) versions
>>here.
>
> Will change for next version, was just following what other drivers were
> doing.

You're right, I wasn't as picky about this before, but will be going
forward.  You're the lucky first victim. :)

Replacing clk_disable() with put_sync() is the correct functional
replacement, since clk_disable is immediate, ans so is put_sync.
However, since we have the option of a delayed disable now with runtime
PM, I think we should use it.

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] arm: omap: introduce 32k timer hwmod for omap2/3/4

[Kevin Hilman]

Felipe Balbi  writes:

> On Thu, Oct 21, 2010 at 12:57:41PM -0500, Kevin Hilman wrote:
>>Felipe Balbi  writes:
>>
>>> Add 32k timer hwmod to the database.
>>>
>>> Signed-off-by: Felipe Balbi 
>>
>>Not sure how this is working correctly on OMAP2 and OMAP3.  All the
>>hwmods are mising the oh->prcm.omap2.module_offs field.
>>
>>Without this, _wait_target_ready *should* fail, and the hwmod should not
>>actually be enabled.
>>
>>Since this was tested to work, I guess what's happening, is because
>>module_offs == 0 (OCP_MOD), it's reading from the IDLEST register offset
>>in OCP_MOD, which is an undefined register.  On 34xx, we get lucky that
>>that bit is zero so omap2_cm_wait_module_ready succeeds.  On 24xx, the
>>polarity of the idlest bits is inversed, so this would likely fail on
>>OMAP2.
>>
>>Either way, the right fix for this is to ensure that OMAP2/3 hwmods have
>>.module_offs populated correctly.
>
> I'll look again but when I was reading omap3 TRM I couldn't find IDLEST
> for this module, maybe I missed something.

It's there, see bit 2 of CM_IDLEST_WKUP.

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] arm: omap: introduce 32k timer hwmod for omap2/3/4

[Kevin Hilman]

Felipe Balbi  writes:

> Add 32k timer hwmod to the database.
>
> Signed-off-by: Felipe Balbi 
> ---

Looking closer at this patch, there are some other problems... 

[...]

> +static struct omap_hwmod omap2420_counter_32k_hwmod = {
> + .name   = "counter_32k",
> + .class  = &omap2420_counter_hwmod_class,
> + .main_clk   = "sync_32k_ick",
> + .prcm = {
> + .omap2 = {
> + .prcm_reg_id = 1,
> + .module_bit = OMAP24XX_EN_GPT1_SHIFT,
> + .idlest_reg_id = 1,
> + .idlest_idle_bit = OMAP24XX_ST_GPT1_SHIFT,
> + },
> + },

You're using the PRCM bits for GPT1, which is not the same as the 32k
counter.

So if this worked, it would be toggling GPT1, which is the kernel timer,
and that would be a rather bad thing.

[...]

> +static struct omap_hwmod omap2430_counter_32k_hwmod = {
> + .name   = "counter_32k",
> + .class  = &omap2430_counter_hwmod_class,
> + .main_clk   = "sync_32k_ick",
> + .prcm = {
> + .omap2 = {
> + .prcm_reg_id = 1,
> + .module_bit = OMAP24XX_EN_GPT1_SHIFT,
> + .idlest_reg_id = 1,
> + .idlest_idle_bit = OMAP24XX_ST_GPT1_SHIFT,
> + },
> + },

GPT1 again

[...]

> +static struct omap_hwmod omap3xxx_counter_32k_hwmod = {
> + .name   = "counter_32k",
> + .class  = &omap3xxx_counter_hwmod_class,
> + .main_clk   = "omap_32ksync_ick",
> + .prcm = {
> + .omap2 = {
> + .prcm_reg_id = 1,
> + .module_bit = OMAP3430_EN_GPT1_SHIFT,
> + .idlest_reg_id = 1,
> + .idlest_idle_bit = OMAP3430_ST_GPT1_SHIFT,
> + },
> + },

and again

> + .slaves = omap3xxx_counter_32k_slaves,
> + .slaves_cnt = ARRAY_SIZE(omap3xxx_counter_32k_slaves),
> + .omap_chip  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
> +};
> +

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] omap:mailbox-send message in process context

[Hari Kanigeri]

Rene,

Thanks for your comment.


>> @@ -92,20 +92,25 @@ int omap_mbox_msg_send(struct omap_mbox *mbox,
>>       mbox_msg_t msg) struct omap_mbox_queue *mq = mbox->txq;
>>       int ret = 0, len;
>>
>> -     spin_lock(&mq->lock);
>> +     spin_lock_bh(&mq->lock);
>>
>
> Please check if this scenario looks valid to you, as discussed and depicted
> with Fernando:
>
> Supposing that at this point the hw fifo is full and there are messages in
> the kfifo.
>
>>       if (kfifo_avail(&mq->fifo) < sizeof(msg)) {
>>               ret = -ENOMEM;
>>               goto out;
>>       }
>>
>
> We reach this point.
>
> In this scenario, the DSP or other Core reads a message from the mbox hw fifo.
> The next happens:
> 1.- The not full interrupt is triggered to the MPU.
> 2.- The mbox's ISR schedules the tasklet to write the message.
> 3.- The ISR is left and returns to here (since we still have the bh lock the
>    tasklet won't run).
>
>> +     if (!__mbox_poll_for_space(mbox)) {\
>
> 4.- We check for mbox_fifo_full and we have space. so the message is written.
>
>> +             mbox_fifo_write(mbox, msg);
>
> At this point it looks that the FIFO order is lost. We write this message
> before the ones in the kfifo.
>

Good point. I agree.

> A solution for this could be checking if there are messages in the kfifo
> before trying to write directly to the hw fifo, if so, just continue
> scheduling the tasklet.

A change something like this ?

-   if (!__mbox_poll_for_space(mbox)) {
+   if (kfifo_is_empty() && !__mbox_poll_for_space(mbox)) {
mbox_fifo_write(mbox, msg);

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 4/7] omap:mailbox-send message in process context

[Sapiens, Rene]

Hari,

On Thursday, October 21, 2010 3:49 PM Hari Kanigeri wrote:
> Rene,
> 
> Thanks for your comment.
> 
> 
>>> @@ -92,20 +92,25 @@ int omap_mbox_msg_send(struct omap_mbox *mbox,
>>>       mbox_msg_t msg) struct omap_mbox_queue *mq = mbox->txq;
>>>       int ret = 0, len;
>>> 
>>> -     spin_lock(&mq->lock);
>>> +     spin_lock_bh(&mq->lock);
>>> 
>> 
>> Please check if this scenario looks valid to you, as discussed and depicted
>> with Fernando:
>> 
>> Supposing that at this point the hw fifo is full and there are messages in
>> the kfifo.
>> 
>>>       if (kfifo_avail(&mq->fifo) < sizeof(msg)) {
>>>               ret = -ENOMEM;
>>>               goto out;
>>>       }
>>> 
>> 
>> We reach this point.
>> 
>> In this scenario, the DSP or other Core reads a message from the mbox hw
>> fifo. The next happens:
>> 1.- The not full interrupt is triggered to the MPU.
>> 2.- The mbox's ISR schedules the tasklet to write the message.
>> 3.- The ISR is left and returns to here (since we still have the bh lock
>> the    tasklet won't run).
>> 
>>> +     if (!__mbox_poll_for_space(mbox)) {\
>> 
>> 4.- We check for mbox_fifo_full and we have space. so the message is
>> written. 
>> 
>>> +             mbox_fifo_write(mbox, msg);
>> 
>> At this point it looks that the FIFO order is lost. We write this message
>> before the ones in the kfifo.
>> 
> 
> Good point. I agree.
> 
>> A solution for this could be checking if there are messages in the kfifo
>> before trying to write directly to the hw fifo, if so, just continue
>> scheduling the tasklet.
> 
> A change something like this ?
> 
> -   if (!__mbox_poll_for_space(mbox)) {
> +   if (kfifo_is_empty() && !__mbox_poll_for_space(mbox)) {

Yes, this looks good to me.

> mbox_fifo_write(mbox, msg);

Regards,
Rene--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2] OMAP: DSS2: Add NEC NL8048HL11-01B display panel

[Nilofer, Samreen]

Grazvydas Ignotas wrote:
> On Thu, Oct 21, 2010 at 3:25 PM, Samreen  wrote:
>> From: Erik Gilling 
>> 
>> NEC WVGA LCD NL8048HL11-01B panel support has been added.
>> This panel is being used in zoom2/zoom3/3630 sdp boards.
>> 
> 
> 
> 
>> diff --git
> a/drivers/video/omap2/displays/panel-nec-nl8048hl11-01b.c
>> b/drivers/video/omap2/displays/panel-nec-nl8048hl11-01b.c
>> new file mode 100644
>> index 000..0327731
>> --- /dev/null
>> +++ b/drivers/video/omap2/displays/panel-nec-nl8048hl11-01b.c
> 
> 
> 
>> +static int spi_send(struct spi_device *spi, unsigned char reg_addr,
>> +                       unsigned char reg_data)
> 
> you could be more consistent and name it nec_8048_spi_send.
[Samreen]
 Will take care of this.
> 
>> +{
>> +       int ret = 0;
>> +       unsigned int cmd = 0, data = 0;
>> +
>> +       cmd = 0x | reg_addr; /* register address write */
>> +       data = 0x0100 | reg_data ; /* register data write */
>> +       data = (cmd << 16) | data;
>> +
>> +       ret = spi_write(spi, (unsigned char *)&data, 4); +       if (ret)
>> +               pr_err("error in spi_write %x\n", data); +
>> +       return ret;
>> +}
>> +
>> +
> 
> one blank line is enough
[Samreen]
  Will fix this
> 
>> +static int init_nec_8048_wvga_lcd(struct spi_device *spi) {
>> +       /* Initialization Sequence */
>> +       /* spi_send(spi, REG, VAL) */
>> +       spi_send(spi, 3, 0x01);
>> +       spi_send(spi, 0, 0x00);
>> +       spi_send(spi, 1, 0x01);    /* R1 = 0x01 (normal), 0x03 +(reversed) */
>> +       spi_send(spi, 4, 0x00);
>> +       spi_send(spi, 5, 0x14);
>> +       spi_send(spi, 6, 0x24);
> 
> This code wastes quite a bit of space (at least 3 mov and one
> bl instruction for every such write on ARM), you could better do something
> like: 
> 
> static const struct {
>   u8 address;
>   u8 value;
> } nec_8048_init_seq[] = {
>   { 3, 0x01 },
>   { 0, 0x00 },
>   { 1, 0x01 },
>   ...
> };
[Samreen]
  Agree, I'll surely take this more optimized approach
> 
> static int init_nec_8048_wvga_lcd(struct spi_device *spi) {   int i, ret;
>   for (i = 0; i < ARRAY_SIZE(nec_8048_init_seq); i++) {
> ret = nec_8048_spi_send(spi,
> nec_8048_init_seq[i].address, nec_8048_init_seq[i].value); if (ret < 0)
>   break;
>   }
>   udelay(20);
>   nec_8048_spi_send(spi, 2, 0x00);
> }
> 
> 
>> +MODULE_DESCRIPTION("Nec Driver");
> 
> could use more complete description here.
[Samreen]
  Sure..--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] arm: omap: counter-32k: convert to pm_runtime API

[Felipe Balbi]

On Thu, Oct 21, 2010 at 03:17:47PM -0500, Kevin Hilman wrote:

You're right, I wasn't as picky about this before, but will be going
forward.  You're the lucky first victim. :)


someone has to be stoned first. Hmm, that does sound ambiguous.


Replacing clk_disable() with put_sync() is the correct functional
replacement, since clk_disable is immediate, ans so is put_sync.
However, since we have the option of a delayed disable now with runtime
PM, I think we should use it.


makes sense. I'll work on it during next week.

--
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html