All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rajendra Nayak <rnayak@ti.com>
To: "Cousson, Benoit" <b-cousson@ti.com>
Cc: linux-serial@vger.kernel.org, linux-omap@vger.kernel.org,
	devicetree-discuss@lists.ozlabs.org, tony@atomide.com,
	khilman@ti.com, govindraj.raja@ti.com,
	linux-arm-kernel@lists.infradead.org,
	linaro-dev@lists.linaro.org
Subject: Re: [PATCH 1/3] ARM: omap_device: handle first time activation of console device
Date: Thu, 17 Nov 2011 15:46:02 +0530	[thread overview]
Message-ID: <4EC4DEE2.10006@ti.com> (raw)
In-Reply-To: <4EC4D96C.9050804@ti.com>

[]...
>>>
>>> Why do we have to idle -> enable? The module is already enabled, isn't
>>> it?
>>> The comment is not super clear for me :-)
>>> Is the goal is to reset the IP?
>>
>> Yes, now that I read it, it does sound confusing. I should have at-least
>> read it once before I picked it from serial.c
>>
>> But anyway, here's what the problem is.
>>
>> I feel its partly to do with the inability of hwmod to handle some
>> modules which are left enabled post a setup, due to the
>> 'HWMOD_INIT_NO_IDLE' flag set.
>> Such modules end up with a hwmod state as '_HWMOD_STATE_ENABLED' post
>> a setup. Now when a driver for such devices/modules tries to enable the
>> module the first time, hwmod throws up a big WARN stating the hwmod is
>> already in an enabled state.
>
> OK, now, that makes sense :-)
> We have hwmod in ENABLE state whereas the omap_device is still in IDLE
> or even DISABLE.

Right.

>
>> [uart used as console is one such module, which cannot be idled post a
>> setup by hwmod]
>>
>> If hwmod could be made in some way intelligent enough to know that the
>> module is in enabled state because of the 'HWMOD_INIT_NO_IDLE' itself,
>> a lot of this hackery might not be needed at all.
>
> Fully agree, the fmwk should handle that.
>
>> Simplest way to do it could be to just add another intermediate state,
>> something like '_HWMOD_STATE_ENABLED_AT_INIT'.
>> I will post a patch for this, see if you like it being handled that way.
>
> That seems to be good. I'm just wondering if we need to introduce a new
> state for that or use a dedicated flag.
> My concern is just that we will have two flavors of HWMOD_STATE_ENABLED
> that we will have to check in various places in the hwmod core code.
>
> Maybe that's not such a big deal. Go ahead, and we will see how it looks
> like.

I initially thought I could do that using the existing flag itself, but
the key is that its needed only the first time a enable is done on the
hwmod. The rest of the state handling remains the same.
I just posted the patch, so that should make it more clear.

>
>> The other part of the problem is also with the inability to hook up
>> 'custom' omap_device_pm_latency for omap devices created from DT.
>> Maybe a lot of such cases which need custom activate/deactivate
>> functions might have to be handled in some way in the framework
>> itself like the one above.
>
> For the moment, it looks like only the serial is requiring such custom

yes.

> stuff, but anyway, we should have a mechanism to allow that...
>
> Thanks,
> Benoit
>


WARNING: multiple messages have this Message-ID (diff)
From: rnayak@ti.com (Rajendra Nayak)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/3] ARM: omap_device: handle first time activation of console device
Date: Thu, 17 Nov 2011 15:46:02 +0530	[thread overview]
Message-ID: <4EC4DEE2.10006@ti.com> (raw)
In-Reply-To: <4EC4D96C.9050804@ti.com>

[]...
>>>
>>> Why do we have to idle -> enable? The module is already enabled, isn't
>>> it?
>>> The comment is not super clear for me :-)
>>> Is the goal is to reset the IP?
>>
>> Yes, now that I read it, it does sound confusing. I should have at-least
>> read it once before I picked it from serial.c
>>
>> But anyway, here's what the problem is.
>>
>> I feel its partly to do with the inability of hwmod to handle some
>> modules which are left enabled post a setup, due to the
>> 'HWMOD_INIT_NO_IDLE' flag set.
>> Such modules end up with a hwmod state as '_HWMOD_STATE_ENABLED' post
>> a setup. Now when a driver for such devices/modules tries to enable the
>> module the first time, hwmod throws up a big WARN stating the hwmod is
>> already in an enabled state.
>
> OK, now, that makes sense :-)
> We have hwmod in ENABLE state whereas the omap_device is still in IDLE
> or even DISABLE.

Right.

>
>> [uart used as console is one such module, which cannot be idled post a
>> setup by hwmod]
>>
>> If hwmod could be made in some way intelligent enough to know that the
>> module is in enabled state because of the 'HWMOD_INIT_NO_IDLE' itself,
>> a lot of this hackery might not be needed at all.
>
> Fully agree, the fmwk should handle that.
>
>> Simplest way to do it could be to just add another intermediate state,
>> something like '_HWMOD_STATE_ENABLED_AT_INIT'.
>> I will post a patch for this, see if you like it being handled that way.
>
> That seems to be good. I'm just wondering if we need to introduce a new
> state for that or use a dedicated flag.
> My concern is just that we will have two flavors of HWMOD_STATE_ENABLED
> that we will have to check in various places in the hwmod core code.
>
> Maybe that's not such a big deal. Go ahead, and we will see how it looks
> like.

I initially thought I could do that using the existing flag itself, but
the key is that its needed only the first time a enable is done on the
hwmod. The rest of the state handling remains the same.
I just posted the patch, so that should make it more clear.

>
>> The other part of the problem is also with the inability to hook up
>> 'custom' omap_device_pm_latency for omap devices created from DT.
>> Maybe a lot of such cases which need custom activate/deactivate
>> functions might have to be handled in some way in the framework
>> itself like the one above.
>
> For the moment, it looks like only the serial is requiring such custom

yes.

> stuff, but anyway, we should have a mechanism to allow that...
>
> Thanks,
> Benoit
>

  reply	other threads:[~2011-11-17 10:16 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-16 11:02 [PATCH 0/3] OMAP serial device tree support Rajendra Nayak
2011-11-16 11:02 ` Rajendra Nayak
2011-11-16 11:02 ` [PATCH 1/3] ARM: omap_device: handle first time activation of console device Rajendra Nayak
2011-11-16 11:02   ` Rajendra Nayak
2011-11-16 14:50   ` Rob Herring
2011-11-16 14:50     ` Rob Herring
2011-11-16 15:14     ` Cousson, Benoit
2011-11-16 15:14       ` Cousson, Benoit
2011-11-16 15:41       ` Rob Herring
2011-11-16 15:41         ` Rob Herring
2011-11-16 18:18         ` Cousson, Benoit
2011-11-16 18:18           ` Cousson, Benoit
2011-11-17  7:31     ` Rajendra Nayak
2011-11-17  7:31       ` Rajendra Nayak
2011-11-16 15:01   ` Cousson, Benoit
2011-11-16 15:01     ` Cousson, Benoit
2011-11-17  7:19     ` Rajendra Nayak
2011-11-17  7:19       ` Rajendra Nayak
2011-11-17  9:52       ` Cousson, Benoit
2011-11-17  9:52         ` Cousson, Benoit
2011-11-17 10:16         ` Rajendra Nayak [this message]
2011-11-17 10:16           ` Rajendra Nayak
2011-11-16 11:02 ` [PATCH 2/3] omap-serial: Add minimal device tree support Rajendra Nayak
2011-11-16 11:02   ` Rajendra Nayak
2011-11-16 14:59   ` Rob Herring
2011-11-16 14:59     ` Rob Herring
2011-11-17  8:39     ` Rajendra Nayak
2011-11-17  8:39       ` Rajendra Nayak
2011-11-16 11:02 ` [PATCH 3/3] ARM: omap: pass minimal SoC/board data for UART from dt Rajendra Nayak
2011-11-16 11:02   ` Rajendra Nayak
2011-11-17  1:04   ` Rob Herring
2011-11-17  1:04     ` Rob Herring
2011-11-17  8:42     ` Rajendra Nayak
2011-11-17  8:42       ` Rajendra Nayak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4EC4DEE2.10006@ti.com \
    --to=rnayak@ti.com \
    --cc=b-cousson@ti.com \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=govindraj.raja@ti.com \
    --cc=khilman@ti.com \
    --cc=linaro-dev@lists.linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.