RE: [PATCH 6/7] [RFC] OMAP: hwmod: SYSCONFIG register modification for MCBSP
> -Original Message-
> From: Kevin Hilman [mailto:khil...@deeprootsystems.com]
> Sent: Thursday, December 02, 2010 4:24 PM
> To: Cousson, Benoit
> Cc: Basak, Partha; ABRAHAM, KISHON VIJAY; Paul Walmsley; linux-
> o...@vger.kernel.org; Kamat, Nishant; Varadarajan, Charulatha; Datta,
> Shubhrajyoti; ext-eero.nurkk...@nokia.com; eduardo.valen...@nokia.com;
> Raja, Govindraj
> Subject: Re: [PATCH 6/7] [RFC] OMAP: hwmod: SYSCONFIG register
> modification for MCBSP
>
> "Cousson, Benoit" writes:
>
> [...]
>
> >
> > The issue is not really for the mcbsp but for the serial that need to
> > handle the 3 different states.
> >
> > if (enable) {
> > /**
> > * Errata 2.15: [UART]:Cannot Acknowledge Idle Requests
> > * in Smartidle Mode When Configured for DMA Operations.
> > */
> > if (uart->dma_enabled)
> > idlemode = HWMOD_IDLEMODE_FORCE;
> > else
> > idlemode = HWMOD_IDLEMODE_SMART;
> > } else {
> > idlemode = HWMOD_IDLEMODE_NO;
> > }
> >
> > You do need to explicitly set the 3 modes, hence the following 3
> APIs:
> > omap_device_force_idle
> > omap_device_no_idle
> > omap_device_smart_idle
I am OK with Benoit's 3 + 1 API approach, 3 individual set functions and
one common function to restore back to the value as dictated by the flag.
What do you say guys?
> >
> > You can potentially add another API to restore the default idle mode:
> >
> > omap_device_default_idle
> >
> > That seems much simpler than 3 pairs of APIs.
>
> My $0.02... based on the fact that we already have some IPs needing to
> conrol all 3 modes, I think having the 3 functions above is better than
> having 3 pairs of request/release functions.
>
> 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 6/7] [RFC] OMAP: hwmod: SYSCONFIG register modification for MCBSP
"Cousson, Benoit" writes:
[...]
>
> The issue is not really for the mcbsp but for the serial that need to
> handle the 3 different states.
>
> if (enable) {
> /**
>* Errata 2.15: [UART]:Cannot Acknowledge Idle Requests
>* in Smartidle Mode When Configured for DMA Operations.
>*/
> if (uart->dma_enabled)
> idlemode = HWMOD_IDLEMODE_FORCE;
> else
> idlemode = HWMOD_IDLEMODE_SMART;
> } else {
> idlemode = HWMOD_IDLEMODE_NO;
> }
>
> You do need to explicitly set the 3 modes, hence the following 3 APIs:
> omap_device_force_idle
> omap_device_no_idle
> omap_device_smart_idle
>
> You can potentially add another API to restore the default idle mode:
>
> omap_device_default_idle
>
> That seems much simpler than 3 pairs of APIs.
My $0.02... based on the fact that we already have some IPs needing to
conrol all 3 modes, I think having the 3 functions above is better than
having 3 pairs of request/release functions.
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 6/7] [RFC] OMAP: hwmod: SYSCONFIG register modification for MCBSP
On Wed, Dec 1, 2010 at 4:45 PM, Cousson, Benoit wrote:
> + Govindraj
>
> Hi Partha,
>
> On 12/1/2010 8:14 AM, Basak, Partha wrote:
>>
>>> From: Cousson, Benoit
>>> Sent: Tuesday, November 30, 2010 9:33 PM
>>>
>>> Hi Kishon,
>>>
>>> Sorry, for the delay.
>>>
>>> On 11/22/2010 4:59 PM, ABRAHAM, KISHON VIJAY wrote:
Resending the mail in plain text format..
On Mon, Nov 22, 2010 at 9:20 PM, ABRAHAM, KISHON VIJAY
>>>
>>> wrote:
>
> On Mon, Oct 11, 2010 at 11:48 AM, ABRAHAM, KISHON
>>>
>>> VIJAY wrote:
>>
>> On Friday 08 October 2010 01:12 PM, Cousson, Benoit wrote:
>>>
>>> Hi Kishon,
>>>
>>> On 10/5/2010 6:37 PM, ABRAHAM, KISHON VIJAY wrote:
MCBSP 2 and 3 in OMAP3 has sidetone feature which requires
autoidle to be disabled before starting the sidetone. Also
>>>
>>> SYSCONFIG
register has to be set with smart idle or no idle depending on
>>>
>>> the
dma op mode (threshold or element sync). For doing these
>>>
>>> operations
dynamically at runtime, hwmod API'S are used to modify SYSCONFIG
>>
>> register
directly.
>>>
>>> OK, it looks like we don't have the choice... But for the
>>> implementation, please discussed with Manju on that, He is doing
>>>
>>> the
>>>
>>> similar thing for the smartstandby issue on SDMA.
>>
>> OK.
>
>
> Looks like we have a problem when we write an API similar to
>>>
>>> the one written
>
> by Manju (omap_hwmod_set_master_standbymode()) [1]. In the
>>>
>>> case of McBSP,
>
> I have to modify omap_hwmod_set_slave_idlemode() (which is
>>>
>>> already existing),
>
> to include something like
>
> + if (sf& SYSC_HAS_SIDLEMODE) {
> + if (idlemode)
> + idlemode = HWMOD_IDLEMODE_NO;
> + else
> + idlemode = (oh->flags&
>>>
>>> HWMOD_SWSUP_SIDLE) ?
>
> + HWMOD_IDLEMODE_NO :
>>>
>>> HWMOD_IDLEMODE_SMART;
>
> + }
>>>
>>> How to you enable HWMOD_IDLEMODE_FORCE mode in that case?
>>>
> Then an API like omap_device_require_no_idle() and
>>>
>>> omap_device_release_no_idle()
>
> would be written similar to omap_device_require_no_mstandby
>>>
>>> and
>
> omap_device_release_no_mstandby() (see [1]) that calls
> omap_hwmod_set_slave_idlemode() with true/false. Passing true
>>>
>>> to
>
> omap_hwmod_set_slave_idlemode() will write SIDLE bits with
> HWMOD_IDLEMODE_NO and false to it will write
> HWMOD_IDLEMODE_NO/HWMOD_IDLEMODE_SMART depending on
> HWMOD_SWSUP_SIDLE to SIDLE bits.
>
> This works fine for McBSP since only two modes come to play
>>>
>>> here
>
> (HWMOD_IDLEMODE_NO/HWMOD_IDLEMODE_SMART).
>
> But omap_hwmod_set_slave_idlemode() API is used by UART
> (serial.c Please refer [2]) which writes SIDLE bits to
> HWMOD_IDLEMODE_NO/HWMOD_IDLEMODE_SMART/ and
> HWMOD_IDLEMODE_FORCE.
>>>
>>> That code should not be there. It was a temporary hacks that should be
>>> replaced eventually with a clean omap_device API like the one you are
>>> currently implementing.
>>> There are a bunch of direct access to omap_hwmod struct that need to be
>>> removed as well.
>>>
> Modifying omap_hwmod_set_slave_idlemode() will require
> 1) serial.c to be modified to call functions like
>>>
>>> 'omap_device_require_no_idle(),
>
> omap_device_release_no_idle()' and
>>>
>>> 'omap_device_require_force_idle() and
>
> omap_device_release_force_idle()' instead of
> omap_hwmod_set_slave_idlemode() which is currently
>>>
>>> present.
>>>
>>> Yep, you're right.
>>>
>
> There are 2 problems associated with it
> 1) The first problem is having a single API like
>>>
>>> omap_hwmod_set_slave_idlemode() to
>
> set two different values like HWMOD_IDLEMODE_NO or
> HWMOD_IDLEMODE_FORCE (intended to be called by
>>>
>>> omap_device_require_no_idle()
>
> and omap_device_require_force_idle() respectively).
>>>
>>> According to the new design
>
> omap_hwmod_set_slave_idlemode() is intended to take only
>>>
>>> true/false as
>
> arguments.
>
> 2) The second problem is having the release API's
>>>
>>> (omap_device_release_no_idle() and
>
> omap_device_release_force_idle()) to restore the
>>>
>>> SYSCONFIG to the previous state.
>
> Remember, this was not problem for McBSP since only two
>>>
>>> mo
Re: [PATCH 6/7] [RFC] OMAP: hwmod: SYSCONFIG register modification for MCBSP
+ Govindraj
Hi Partha,
On 12/1/2010 8:14 AM, Basak, Partha wrote:
From: Cousson, Benoit
Sent: Tuesday, November 30, 2010 9:33 PM
Hi Kishon,
Sorry, for the delay.
On 11/22/2010 4:59 PM, ABRAHAM, KISHON VIJAY wrote:
Resending the mail in plain text format..
On Mon, Nov 22, 2010 at 9:20 PM, ABRAHAM, KISHON VIJAY
wrote:
On Mon, Oct 11, 2010 at 11:48 AM, ABRAHAM, KISHON
VIJAY wrote:
On Friday 08 October 2010 01:12 PM, Cousson, Benoit wrote:
Hi Kishon,
On 10/5/2010 6:37 PM, ABRAHAM, KISHON VIJAY wrote:
MCBSP 2 and 3 in OMAP3 has sidetone feature which requires
autoidle to be disabled before starting the sidetone. Also
SYSCONFIG
register has to be set with smart idle or no idle depending on
the
dma op mode (threshold or element sync). For doing these
operations
dynamically at runtime, hwmod API'S are used to modify SYSCONFIG
register
directly.
OK, it looks like we don't have the choice... But for the
implementation, please discussed with Manju on that, He is doing
the
similar thing for the smartstandby issue on SDMA.
OK.
Looks like we have a problem when we write an API similar to
the one written
by Manju (omap_hwmod_set_master_standbymode()) [1]. In the
case of McBSP,
I have to modify omap_hwmod_set_slave_idlemode() (which is
already existing),
to include something like
+if (sf& SYSC_HAS_SIDLEMODE) {
+if (idlemode)
+idlemode = HWMOD_IDLEMODE_NO;
+else
+idlemode = (oh->flags&
HWMOD_SWSUP_SIDLE) ?
+HWMOD_IDLEMODE_NO :
HWMOD_IDLEMODE_SMART;
+}
How to you enable HWMOD_IDLEMODE_FORCE mode in that case?
Then an API like omap_device_require_no_idle() and
omap_device_release_no_idle()
would be written similar to omap_device_require_no_mstandby
and
omap_device_release_no_mstandby() (see [1]) that calls
omap_hwmod_set_slave_idlemode() with true/false. Passing true
to
omap_hwmod_set_slave_idlemode() will write SIDLE bits with
HWMOD_IDLEMODE_NO and false to it will write
HWMOD_IDLEMODE_NO/HWMOD_IDLEMODE_SMART depending on
HWMOD_SWSUP_SIDLE to SIDLE bits.
This works fine for McBSP since only two modes come to play
here
(HWMOD_IDLEMODE_NO/HWMOD_IDLEMODE_SMART).
But omap_hwmod_set_slave_idlemode() API is used by UART
(serial.c Please refer [2]) which writes SIDLE bits to
HWMOD_IDLEMODE_NO/HWMOD_IDLEMODE_SMART/ and
HWMOD_IDLEMODE_FORCE.
That code should not be there. It was a temporary hacks that should be
replaced eventually with a clean omap_device API like the one you are
currently implementing.
There are a bunch of direct access to omap_hwmod struct that need to be
removed as well.
Modifying omap_hwmod_set_slave_idlemode() will require
1) serial.c to be modified to call functions like
'omap_device_require_no_idle(),
omap_device_release_no_idle()' and
'omap_device_require_force_idle() and
omap_device_release_force_idle()' instead of
omap_hwmod_set_slave_idlemode() which is currently
present.
Yep, you're right.
There are 2 problems associated with it
1) The first problem is having a single API like
omap_hwmod_set_slave_idlemode() to
set two different values like HWMOD_IDLEMODE_NO or
HWMOD_IDLEMODE_FORCE (intended to be called by
omap_device_require_no_idle()
and omap_device_require_force_idle() respectively).
According to the new design
omap_hwmod_set_slave_idlemode() is intended to take only
true/false as
arguments.
2) The second problem is having the release API's
(omap_device_release_no_idle() and
omap_device_release_force_idle()) to restore the
SYSCONFIG to the previous state.
Remember, this was not problem for McBSP since only two
modes were needed.
And what about 3 APIs?
omap_device_force_idle
omap_device_no_idle
omap_device_smart_idle
Want to reiterate Paul Walmsley's comments on Manju's DMA email dated 11/11/2010
2. Rather than just using a single omap_device_mstandby() function call,
I'd rather see something like omap_device_require_no_standby() and
omap_device_release_no_standby(). omap_device_require_no_standby() would
force the initiator port into no-standby.
omap_device_release_no_standby() would return the initiator port
MSTANDBYMODE to whatever the state should be (this depends on whether
HWMOD_SWSUP_MSTDBY is set). I'd rather not have the drivers manage the
contents of the MSTANDBYMODE bits directly. Then you should be able to
drop _get_master_standbymode() and omap_hwmod_get_master_idlemode() also.
Can you please make these changes?
The key point
RE: [PATCH 6/7] [RFC] OMAP: hwmod: SYSCONFIG register modification for MCBSP
> -Original Message-
> From: Cousson, Benoit
> Sent: Tuesday, November 30, 2010 9:33 PM
> To: ABRAHAM, KISHON VIJAY; Paul Walmsley
> Cc: Kevin Hilman; Basak, Partha; linux-omap@vger.kernel.org; Kamat,
> Nishant; Varadarajan, Charulatha; Datta, Shubhrajyoti; ext-
> eero.nurkk...@nokia.com; eduardo.valen...@nokia.com
> Subject: Re: [PATCH 6/7] [RFC] OMAP: hwmod: SYSCONFIG register
> modification for MCBSP
>
> Hi Kishon,
>
> Sorry, for the delay.
>
> On 11/22/2010 4:59 PM, ABRAHAM, KISHON VIJAY wrote:
> > Resending the mail in plain text format..
> >
> > On Mon, Nov 22, 2010 at 9:20 PM, ABRAHAM, KISHON VIJAY
> wrote:
> >>
> >> On Mon, Oct 11, 2010 at 11:48 AM, ABRAHAM, KISHON
> VIJAY wrote:
> >>>
> >>> On Friday 08 October 2010 01:12 PM, Cousson, Benoit wrote:
> >>>> Hi Kishon,
> >>>>
> >>>> On 10/5/2010 6:37 PM, ABRAHAM, KISHON VIJAY wrote:
> >>>>> MCBSP 2 and 3 in OMAP3 has sidetone feature which requires
> >>>>> autoidle to be disabled before starting the sidetone. Also
> SYSCONFIG
> >>>>> register has to be set with smart idle or no idle depending on
> the
> >>>>> dma op mode (threshold or element sync). For doing these
> operations
> >>>>> dynamically at runtime, hwmod API'S are used to modify SYSCONFIG
> >>> register
> >>>>> directly.
> >>>>
> >>>> OK, it looks like we don't have the choice... But for the
> >>>> implementation, please discussed with Manju on that, He is doing
> the
> >>>> similar thing for the smartstandby issue on SDMA.
> >>>
> >>>OK.
> >>
> >>
> >>Looks like we have a problem when we write an API similar to
> the one written
> >>by Manju (omap_hwmod_set_master_standbymode()) [1]. In the
> case of McBSP,
> >>I have to modify omap_hwmod_set_slave_idlemode() (which is
> already existing),
> >>to include something like
> >>
> >> +if (sf& SYSC_HAS_SIDLEMODE) {
> >> +if (idlemode)
> >> +idlemode = HWMOD_IDLEMODE_NO;
> >> +else
> >> +idlemode = (oh->flags&
> HWMOD_SWSUP_SIDLE) ?
> >> +HWMOD_IDLEMODE_NO :
> HWMOD_IDLEMODE_SMART;
> >> +}
>
> How to you enable HWMOD_IDLEMODE_FORCE mode in that case?
>
> >>Then an API like omap_device_require_no_idle() and
> omap_device_release_no_idle()
> >>would be written similar to omap_device_require_no_mstandby
> and
> >>omap_device_release_no_mstandby() (see [1]) that calls
> >>omap_hwmod_set_slave_idlemode() with true/false. Passing true
> to
> >>omap_hwmod_set_slave_idlemode() will write SIDLE bits with
> >>HWMOD_IDLEMODE_NO and false to it will write
> >>HWMOD_IDLEMODE_NO/HWMOD_IDLEMODE_SMART depending on
> >>HWMOD_SWSUP_SIDLE to SIDLE bits.
> >>
> >>This works fine for McBSP since only two modes come to play
> here
> >>(HWMOD_IDLEMODE_NO/HWMOD_IDLEMODE_SMART).
> >>
> >>But omap_hwmod_set_slave_idlemode() API is used by UART
> >>(serial.c Please refer [2]) which writes SIDLE bits to
> >>HWMOD_IDLEMODE_NO/HWMOD_IDLEMODE_SMART/ and
> >>HWMOD_IDLEMODE_FORCE.
>
> That code should not be there. It was a temporary hacks that should be
> replaced eventually with a clean omap_device API like the one you are
> currently implementing.
> There are a bunch of direct access to omap_hwmod struct that need to be
> removed as well.
>
> >>Modifying omap_hwmod_set_slave_idlemode() will require
> >>1) serial.c to be modified to call functions like
> 'omap_device_require_no_idle(),
> >>omap_device_release_no_idle()' and
> 'omap_device_require_force_idle() and
> >>omap_device_release_force_idle()' instead of
> >>omap_hwmod_set_slave_idlemode() which is currently
> present.
>
> Yep, you're right.
>
> >>
> >>There are 2 problems associated with it
> >>1) The first problem is having a single API like
> omap_hwmod_set_slave_idlemode() to
> >>set two
Re: [PATCH 6/7] [RFC] OMAP: hwmod: SYSCONFIG register modification for MCBSP
Hi Kishon,
Sorry, for the delay.
On 11/22/2010 4:59 PM, ABRAHAM, KISHON VIJAY wrote:
Resending the mail in plain text format..
On Mon, Nov 22, 2010 at 9:20 PM, ABRAHAM, KISHON VIJAY wrote:
On Mon, Oct 11, 2010 at 11:48 AM, ABRAHAM, KISHON VIJAY wrote:
On Friday 08 October 2010 01:12 PM, Cousson, Benoit wrote:
Hi Kishon,
On 10/5/2010 6:37 PM, ABRAHAM, KISHON VIJAY wrote:
MCBSP 2 and 3 in OMAP3 has sidetone feature which requires
autoidle to be disabled before starting the sidetone. Also SYSCONFIG
register has to be set with smart idle or no idle depending on the
dma op mode (threshold or element sync). For doing these operations
dynamically at runtime, hwmod API'S are used to modify SYSCONFIG
register
directly.
OK, it looks like we don't have the choice... But for the
implementation, please discussed with Manju on that, He is doing the
similar thing for the smartstandby issue on SDMA.
OK.
Looks like we have a problem when we write an API similar to the one
written
by Manju (omap_hwmod_set_master_standbymode()) [1]. In the case of McBSP,
I have to modify omap_hwmod_set_slave_idlemode() (which is already
existing),
to include something like
+if (sf& SYSC_HAS_SIDLEMODE) {
+if (idlemode)
+idlemode = HWMOD_IDLEMODE_NO;
+else
+idlemode = (oh->flags&
HWMOD_SWSUP_SIDLE) ?
+HWMOD_IDLEMODE_NO :
HWMOD_IDLEMODE_SMART;
+}
How to you enable HWMOD_IDLEMODE_FORCE mode in that case?
Then an API like omap_device_require_no_idle() and
omap_device_release_no_idle()
would be written similar to omap_device_require_no_mstandby and
omap_device_release_no_mstandby() (see [1]) that calls
omap_hwmod_set_slave_idlemode() with true/false. Passing true to
omap_hwmod_set_slave_idlemode() will write SIDLE bits with
HWMOD_IDLEMODE_NO and false to it will write
HWMOD_IDLEMODE_NO/HWMOD_IDLEMODE_SMART depending on
HWMOD_SWSUP_SIDLE to SIDLE bits.
This works fine for McBSP since only two modes come to play here
(HWMOD_IDLEMODE_NO/HWMOD_IDLEMODE_SMART).
But omap_hwmod_set_slave_idlemode() API is used by UART
(serial.c Please refer [2]) which writes SIDLE bits to
HWMOD_IDLEMODE_NO/HWMOD_IDLEMODE_SMART/ and
HWMOD_IDLEMODE_FORCE.
That code should not be there. It was a temporary hacks that should be
replaced eventually with a clean omap_device API like the one you are
currently implementing.
There are a bunch of direct access to omap_hwmod struct that need to be
removed as well.
Modifying omap_hwmod_set_slave_idlemode() will require
1) serial.c to be modified to call functions like
'omap_device_require_no_idle(),
omap_device_release_no_idle()' and
'omap_device_require_force_idle() and
omap_device_release_force_idle()' instead of
omap_hwmod_set_slave_idlemode() which is currently present.
Yep, you're right.
There are 2 problems associated with it
1) The first problem is having a single API like
omap_hwmod_set_slave_idlemode() to
set two different values like HWMOD_IDLEMODE_NO or
HWMOD_IDLEMODE_FORCE (intended to be called by
omap_device_require_no_idle()
and omap_device_require_force_idle() respectively). According to the
new design
omap_hwmod_set_slave_idlemode() is intended to take only true/false
as
arguments.
2) The second problem is having the release API's
(omap_device_release_no_idle() and
omap_device_release_force_idle()) to restore the SYSCONFIG to the
previous state.
Remember, this was not problem for McBSP since only two modes were
needed.
And what about 3 APIs?
omap_device_force_idle
omap_device_no_idle
omap_device_smart_idle
It is probably much more appropriate than a require / release in that case?
Regards,
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 6/7] [RFC] OMAP: hwmod: SYSCONFIG register modification for MCBSP
Resending the mail in plain text format..
On Mon, Nov 22, 2010 at 9:20 PM, ABRAHAM, KISHON VIJAY wrote:
>
> On Mon, Oct 11, 2010 at 11:48 AM, ABRAHAM, KISHON VIJAY wrote:
>>
>> On Friday 08 October 2010 01:12 PM, Cousson, Benoit wrote:
>> > Hi Kishon,
>> >
>> > On 10/5/2010 6:37 PM, ABRAHAM, KISHON VIJAY wrote:
>> >> MCBSP 2 and 3 in OMAP3 has sidetone feature which requires
>> >> autoidle to be disabled before starting the sidetone. Also SYSCONFIG
>> >> register has to be set with smart idle or no idle depending on the
>> >> dma op mode (threshold or element sync). For doing these operations
>> >> dynamically at runtime, hwmod API'S are used to modify SYSCONFIG
>> register
>> >> directly.
>> >
>> > OK, it looks like we don't have the choice... But for the
>> > implementation, please discussed with Manju on that, He is doing the
>> > similar thing for the smartstandby issue on SDMA.
>>
>> OK.
>
>
> Looks like we have a problem when we write an API similar to the one
> written
> by Manju (omap_hwmod_set_master_standbymode()) [1]. In the case of
> McBSP,
> I have to modify omap_hwmod_set_slave_idlemode() (which is already
> existing),
> to include something like
>
> + if (sf & SYSC_HAS_SIDLEMODE) {
> + if (idlemode)
> + idlemode = HWMOD_IDLEMODE_NO;
> + else
> + idlemode = (oh->flags &
> HWMOD_SWSUP_SIDLE) ?
> + HWMOD_IDLEMODE_NO :
> HWMOD_IDLEMODE_SMART;
> + }
>
> Then an API like omap_device_require_no_idle() and
> omap_device_release_no_idle()
> would be written similar to omap_device_require_no_mstandby and
> omap_device_release_no_mstandby() (see [1]) that calls
> omap_hwmod_set_slave_idlemode() with true/false. Passing true to
> omap_hwmod_set_slave_idlemode() will write SIDLE bits with
> HWMOD_IDLEMODE_NO and false to it will write
> HWMOD_IDLEMODE_NO/HWMOD_IDLEMODE_SMART depending on
> HWMOD_SWSUP_SIDLE to SIDLE bits.
>
> This works fine for McBSP since only two modes come to play here
> (HWMOD_IDLEMODE_NO/HWMOD_IDLEMODE_SMART).
>
> But omap_hwmod_set_slave_idlemode() API is used by UART
> (serial.c Please refer [2]) which writes SIDLE bits to
> HWMOD_IDLEMODE_NO/HWMOD_IDLEMODE_SMART/ and
> HWMOD_IDLEMODE_FORCE.
>
> Modifying omap_hwmod_set_slave_idlemode() will require
> 1) serial.c to be modified to call functions like
> 'omap_device_require_no_idle(),
> omap_device_release_no_idle()' and
> 'omap_device_require_force_idle() and
> omap_device_release_force_idle()' instead of
> omap_hwmod_set_slave_idlemode() which is currently present.
>
> There are 2 problems associated with it
> 1) The first problem is having a single API like
> omap_hwmod_set_slave_idlemode() to
> set two different values like HWMOD_IDLEMODE_NO or
> HWMOD_IDLEMODE_FORCE (intended to be called by
> omap_device_require_no_idle()
> and omap_device_require_force_idle() respectively). According to
> the new design
> omap_hwmod_set_slave_idlemode() is intended to take only true/false
> as
> arguments.
>
> 2) The second problem is having the release API's
> (omap_device_release_no_idle() and
> omap_device_release_force_idle()) to restore the SYSCONFIG to the
> previous state.
> Remember, this was not problem for McBSP since only two modes were
> needed.
>
> Please provide your comment on this.
>
> -Kishon
>
> [1] https://patchwork.kernel.org/patch/335591/
> [2]
> http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=blob;f=arch/arm/mach-omap2/serial.c;h=edd7c99de38dde5bf877788fb4e48055c0d9fbfa;hb=HEAD
>>
>> >
>> >>
>> >> Signed-off-by: Kishon Vijay Abraham I
>> >> Signed-off-by: Charulatha V
>> >> Signed-off-by: Shubhrajyoti D
>> >> Cc: Partha Basak
>> >> ---
>> >> arch/arm/plat-omap/mcbsp.c | 69
>> ++--
>> >> 1 files changed, 41 insertions(+), 28 deletions(-)
>> >>
>> >> diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
>> >> index c7c6a83..6b705e1 100644
>> >> --- a/arch/arm/plat-omap/mcbsp.c
>> >> +++ b/arch/arm/plat-omap/mcbsp.c
>> >> @@ -228,10 +228,21 @@ void omap_mcbsp_config(unsigned int id, const
>> struct omap_mcbsp_reg_cfg *config)
>> >> EXPORT_SYMBOL(omap_mcbsp_config);
>> >>
>> >> #ifdef CONFIG_ARCH_OMAP3
>> >> +static struct omap_hwmod **find_hwmods_by_dev(struct device *dev)
>> >> +{
>> >> + struct platform_device *pdev;
>> >> + struct omap_device *od;
>> >> + pdev = container_of(dev, struct platform_device, dev);
>> >> + od = container_of(pdev, struct omap_device, pdev);
>> >> + return od->hwmods;
>> >
>> > R
Re: [PATCH 6/7] [RFC] OMAP: hwmod: SYSCONFIG register modification for MCBSP
On Friday 08 October 2010 01:12 PM, Cousson, Benoit wrote:
Hi Kishon,
On 10/5/2010 6:37 PM, ABRAHAM, KISHON VIJAY wrote:
MCBSP 2 and 3 in OMAP3 has sidetone feature which requires
autoidle to be disabled before starting the sidetone. Also SYSCONFIG
register has to be set with smart idle or no idle depending on the
dma op mode (threshold or element sync). For doing these operations
dynamically at runtime, hwmod API'S are used to modify SYSCONFIG register
directly.
OK, it looks like we don't have the choice... But for the
implementation, please discussed with Manju on that, He is doing the
similar thing for the smartstandby issue on SDMA.
OK.
Signed-off-by: Kishon Vijay Abraham I
Signed-off-by: Charulatha V
Signed-off-by: Shubhrajyoti D
Cc: Partha Basak
---
arch/arm/plat-omap/mcbsp.c | 69
++--
1 files changed, 41 insertions(+), 28 deletions(-)
diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
index c7c6a83..6b705e1 100644
--- a/arch/arm/plat-omap/mcbsp.c
+++ b/arch/arm/plat-omap/mcbsp.c
@@ -228,10 +228,21 @@ void omap_mcbsp_config(unsigned int id, const struct
omap_mcbsp_reg_cfg *config)
EXPORT_SYMBOL(omap_mcbsp_config);
#ifdef CONFIG_ARCH_OMAP3
+static struct omap_hwmod **find_hwmods_by_dev(struct device *dev)
+{
+ struct platform_device *pdev;
+ struct omap_device *od;
+ pdev = container_of(dev, struct platform_device, dev);
+ od = container_of(pdev, struct omap_device, pdev);
+ return od->hwmods;
Rule #1, don't touch oh in your code. The device should be the only
interface.
If such API is needed, it should be in omap_device. But in your case, I
don't thing it is needed.
OK
+}
+
static void omap_st_on(struct omap_mcbsp *mcbsp)
{
unsigned int w;
+ struct omap_hwmod **oh;
+ oh = find_hwmods_by_dev(mcbsp->dev);
/*
* Sidetone uses McBSP ICLK - which must not idle when sidetones
* are enabled or sidetones start sounding ugly.
@@ -244,8 +255,7 @@ static void omap_st_on(struct omap_mcbsp *mcbsp)
w = MCBSP_READ(mcbsp, SSELCR);
MCBSP_WRITE(mcbsp, SSELCR, w | SIDETONEEN);
- w = MCBSP_ST_READ(mcbsp, SYSCONFIG);
- MCBSP_ST_WRITE(mcbsp, SYSCONFIG, w& ~(ST_AUTOIDLE));
+ omap_hwmod_set_module_autoidle(oh[1], 0);
Don't use internal hwmod API. You have to create a similar omap_device API.
You don't have to select only one hwmod in this case, just change the
sysconfig setting for all hwmod that belong to the omap_device, and it
will be fine. It will avoid you to access one hwmod in particular.
This is the implementation that Manju is doing.
OK
/* Enable Sidetone from Sidetone Core */
w = MCBSP_ST_READ(mcbsp, SSELCR);
@@ -255,12 +265,14 @@ static void omap_st_on(struct omap_mcbsp *mcbsp)
static void omap_st_off(struct omap_mcbsp *mcbsp)
{
unsigned int w;
+ struct omap_hwmod **oh;
+
+ oh = find_hwmods_by_dev(mcbsp->dev);
w = MCBSP_ST_READ(mcbsp, SSELCR);
MCBSP_ST_WRITE(mcbsp, SSELCR, w& ~(ST_SIDETONEEN));
- w = MCBSP_ST_READ(mcbsp, SYSCONFIG);
- MCBSP_ST_WRITE(mcbsp, SYSCONFIG, w | ST_AUTOIDLE);
+ omap_hwmod_set_module_autoidle(oh[1], 1);
w = MCBSP_READ(mcbsp, SSELCR);
MCBSP_WRITE(mcbsp, SSELCR, w& ~(SIDETONEEN));
@@ -273,9 +285,11 @@ static void omap_st_off(struct omap_mcbsp *mcbsp)
static void omap_st_fir_write(struct omap_mcbsp *mcbsp, s16 *fir)
{
u16 val, i;
+ struct omap_hwmod **oh;
- val = MCBSP_ST_READ(mcbsp, SYSCONFIG);
- MCBSP_ST_WRITE(mcbsp, SYSCONFIG, val& ~(ST_AUTOIDLE));
+ oh = find_hwmods_by_dev(mcbsp->dev);
+
+ omap_hwmod_set_module_autoidle(oh[1], 0);
val = MCBSP_ST_READ(mcbsp, SSELCR);
@@ -303,9 +317,11 @@ static void omap_st_chgain(struct omap_mcbsp *mcbsp)
{
u16 w;
struct omap_mcbsp_st_data *st_data = mcbsp->st_data;
+ struct omap_hwmod **oh;
+
+ oh = find_hwmods_by_dev(mcbsp->dev);
- w = MCBSP_ST_READ(mcbsp, SYSCONFIG);
- MCBSP_ST_WRITE(mcbsp, SYSCONFIG, w& ~(ST_AUTOIDLE));
+ omap_hwmod_set_module_autoidle(oh[1], 0);
w = MCBSP_ST_READ(mcbsp, SSELCR);
@@ -648,49 +664,46 @@ EXPORT_SYMBOL(omap_mcbsp_get_dma_op_mode);
static inline void omap34xx_mcbsp_request(struct omap_mcbsp *mcbsp)
{
+ struct omap_hwmod **oh;
+
+ oh = find_hwmods_by_dev(mcbsp->dev);
/*
* Enable wakup behavior, smart idle and all wakeups
* REVISIT: some wakeups may be unnecessary
*/
if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
- u16 syscon;
-
- syscon = MCBSP_READ(mcbsp, SYSCON);
- syscon&= ~(ENAWAKEUP | SIDLEMODE(0x03) | CLOCKACTIVITY(0x03));
if (mcbsp->dma_op_mode == MCBSP_DMA_MODE_THRESHOLD) {
- syscon |= (ENAWAKEUP | SIDLEMODE(0x02) |
-
Re: [PATCH 6/7] [RFC] OMAP: hwmod: SYSCONFIG register modification for MCBSP
Hi Kishon,
On 10/5/2010 6:37 PM, ABRAHAM, KISHON VIJAY wrote:
MCBSP 2 and 3 in OMAP3 has sidetone feature which requires
autoidle to be disabled before starting the sidetone. Also SYSCONFIG
register has to be set with smart idle or no idle depending on the
dma op mode (threshold or element sync). For doing these operations
dynamically at runtime, hwmod API'S are used to modify SYSCONFIG register
directly.
OK, it looks like we don't have the choice... But for the
implementation, please discussed with Manju on that, He is doing the
similar thing for the smartstandby issue on SDMA.
Signed-off-by: Kishon Vijay Abraham I
Signed-off-by: Charulatha V
Signed-off-by: Shubhrajyoti D
Cc: Partha Basak
---
arch/arm/plat-omap/mcbsp.c | 69 ++--
1 files changed, 41 insertions(+), 28 deletions(-)
diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
index c7c6a83..6b705e1 100644
--- a/arch/arm/plat-omap/mcbsp.c
+++ b/arch/arm/plat-omap/mcbsp.c
@@ -228,10 +228,21 @@ void omap_mcbsp_config(unsigned int id, const struct
omap_mcbsp_reg_cfg *config)
EXPORT_SYMBOL(omap_mcbsp_config);
#ifdef CONFIG_ARCH_OMAP3
+static struct omap_hwmod **find_hwmods_by_dev(struct device *dev)
+{
+ struct platform_device *pdev;
+ struct omap_device *od;
+ pdev = container_of(dev, struct platform_device, dev);
+ od = container_of(pdev, struct omap_device, pdev);
+ return od->hwmods;
Rule #1, don't touch oh in your code. The device should be the only
interface.
If such API is needed, it should be in omap_device. But in your case, I
don't thing it is needed.
+}
+
static void omap_st_on(struct omap_mcbsp *mcbsp)
{
unsigned int w;
+ struct omap_hwmod **oh;
+ oh = find_hwmods_by_dev(mcbsp->dev);
/*
* Sidetone uses McBSP ICLK - which must not idle when sidetones
* are enabled or sidetones start sounding ugly.
@@ -244,8 +255,7 @@ static void omap_st_on(struct omap_mcbsp *mcbsp)
w = MCBSP_READ(mcbsp, SSELCR);
MCBSP_WRITE(mcbsp, SSELCR, w | SIDETONEEN);
- w = MCBSP_ST_READ(mcbsp, SYSCONFIG);
- MCBSP_ST_WRITE(mcbsp, SYSCONFIG, w& ~(ST_AUTOIDLE));
+ omap_hwmod_set_module_autoidle(oh[1], 0);
Don't use internal hwmod API. You have to create a similar omap_device API.
You don't have to select only one hwmod in this case, just change the
sysconfig setting for all hwmod that belong to the omap_device, and it
will be fine. It will avoid you to access one hwmod in particular.
This is the implementation that Manju is doing.
/* Enable Sidetone from Sidetone Core */
w = MCBSP_ST_READ(mcbsp, SSELCR);
@@ -255,12 +265,14 @@ static void omap_st_on(struct omap_mcbsp *mcbsp)
static void omap_st_off(struct omap_mcbsp *mcbsp)
{
unsigned int w;
+ struct omap_hwmod **oh;
+
+ oh = find_hwmods_by_dev(mcbsp->dev);
w = MCBSP_ST_READ(mcbsp, SSELCR);
MCBSP_ST_WRITE(mcbsp, SSELCR, w& ~(ST_SIDETONEEN));
- w = MCBSP_ST_READ(mcbsp, SYSCONFIG);
- MCBSP_ST_WRITE(mcbsp, SYSCONFIG, w | ST_AUTOIDLE);
+ omap_hwmod_set_module_autoidle(oh[1], 1);
w = MCBSP_READ(mcbsp, SSELCR);
MCBSP_WRITE(mcbsp, SSELCR, w& ~(SIDETONEEN));
@@ -273,9 +285,11 @@ static void omap_st_off(struct omap_mcbsp *mcbsp)
static void omap_st_fir_write(struct omap_mcbsp *mcbsp, s16 *fir)
{
u16 val, i;
+ struct omap_hwmod **oh;
- val = MCBSP_ST_READ(mcbsp, SYSCONFIG);
- MCBSP_ST_WRITE(mcbsp, SYSCONFIG, val& ~(ST_AUTOIDLE));
+ oh = find_hwmods_by_dev(mcbsp->dev);
+
+ omap_hwmod_set_module_autoidle(oh[1], 0);
val = MCBSP_ST_READ(mcbsp, SSELCR);
@@ -303,9 +317,11 @@ static void omap_st_chgain(struct omap_mcbsp *mcbsp)
{
u16 w;
struct omap_mcbsp_st_data *st_data = mcbsp->st_data;
+ struct omap_hwmod **oh;
+
+ oh = find_hwmods_by_dev(mcbsp->dev);
- w = MCBSP_ST_READ(mcbsp, SYSCONFIG);
- MCBSP_ST_WRITE(mcbsp, SYSCONFIG, w& ~(ST_AUTOIDLE));
+ omap_hwmod_set_module_autoidle(oh[1], 0);
w = MCBSP_ST_READ(mcbsp, SSELCR);
@@ -648,49 +664,46 @@ EXPORT_SYMBOL(omap_mcbsp_get_dma_op_mode);
static inline void omap34xx_mcbsp_request(struct omap_mcbsp *mcbsp)
{
+ struct omap_hwmod **oh;
+
+ oh = find_hwmods_by_dev(mcbsp->dev);
/*
* Enable wakup behavior, smart idle and all wakeups
* REVISIT: some wakeups may be unnecessary
*/
if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
- u16 syscon;
-
- syscon = MCBSP_READ(mcbsp, SYSCON);
- syscon&= ~(ENAWAKEUP | SIDLEMODE(0x03) | CLOCKACTIVITY(0x03));
if (mcbsp->dma_op_mode == MCBSP_DMA_MODE_THRESHOLD) {
- syscon |= (ENAWAKEUP | SIDLEMODE(0x02) |
- CLOCKACTIVITY(0x02));
- MCBSP_WRITE(mcbsp, WAKEUPEN, XR
[PATCH 6/7] [RFC] OMAP: hwmod: SYSCONFIG register modification for MCBSP
MCBSP 2 and 3 in OMAP3 has sidetone feature which requires
autoidle to be disabled before starting the sidetone. Also SYSCONFIG
register has to be set with smart idle or no idle depending on the
dma op mode (threshold or element sync). For doing these operations
dynamically at runtime, hwmod API'S are used to modify SYSCONFIG register
directly.
Signed-off-by: Kishon Vijay Abraham I
Signed-off-by: Charulatha V
Signed-off-by: Shubhrajyoti D
Cc: Partha Basak
---
arch/arm/plat-omap/mcbsp.c | 69 ++--
1 files changed, 41 insertions(+), 28 deletions(-)
diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
index c7c6a83..6b705e1 100644
--- a/arch/arm/plat-omap/mcbsp.c
+++ b/arch/arm/plat-omap/mcbsp.c
@@ -228,10 +228,21 @@ void omap_mcbsp_config(unsigned int id, const struct
omap_mcbsp_reg_cfg *config)
EXPORT_SYMBOL(omap_mcbsp_config);
#ifdef CONFIG_ARCH_OMAP3
+static struct omap_hwmod **find_hwmods_by_dev(struct device *dev)
+{
+ struct platform_device *pdev;
+ struct omap_device *od;
+ pdev = container_of(dev, struct platform_device, dev);
+ od = container_of(pdev, struct omap_device, pdev);
+ return od->hwmods;
+}
+
static void omap_st_on(struct omap_mcbsp *mcbsp)
{
unsigned int w;
+ struct omap_hwmod **oh;
+ oh = find_hwmods_by_dev(mcbsp->dev);
/*
* Sidetone uses McBSP ICLK - which must not idle when sidetones
* are enabled or sidetones start sounding ugly.
@@ -244,8 +255,7 @@ static void omap_st_on(struct omap_mcbsp *mcbsp)
w = MCBSP_READ(mcbsp, SSELCR);
MCBSP_WRITE(mcbsp, SSELCR, w | SIDETONEEN);
- w = MCBSP_ST_READ(mcbsp, SYSCONFIG);
- MCBSP_ST_WRITE(mcbsp, SYSCONFIG, w & ~(ST_AUTOIDLE));
+ omap_hwmod_set_module_autoidle(oh[1], 0);
/* Enable Sidetone from Sidetone Core */
w = MCBSP_ST_READ(mcbsp, SSELCR);
@@ -255,12 +265,14 @@ static void omap_st_on(struct omap_mcbsp *mcbsp)
static void omap_st_off(struct omap_mcbsp *mcbsp)
{
unsigned int w;
+ struct omap_hwmod **oh;
+
+ oh = find_hwmods_by_dev(mcbsp->dev);
w = MCBSP_ST_READ(mcbsp, SSELCR);
MCBSP_ST_WRITE(mcbsp, SSELCR, w & ~(ST_SIDETONEEN));
- w = MCBSP_ST_READ(mcbsp, SYSCONFIG);
- MCBSP_ST_WRITE(mcbsp, SYSCONFIG, w | ST_AUTOIDLE);
+ omap_hwmod_set_module_autoidle(oh[1], 1);
w = MCBSP_READ(mcbsp, SSELCR);
MCBSP_WRITE(mcbsp, SSELCR, w & ~(SIDETONEEN));
@@ -273,9 +285,11 @@ static void omap_st_off(struct omap_mcbsp *mcbsp)
static void omap_st_fir_write(struct omap_mcbsp *mcbsp, s16 *fir)
{
u16 val, i;
+ struct omap_hwmod **oh;
- val = MCBSP_ST_READ(mcbsp, SYSCONFIG);
- MCBSP_ST_WRITE(mcbsp, SYSCONFIG, val & ~(ST_AUTOIDLE));
+ oh = find_hwmods_by_dev(mcbsp->dev);
+
+ omap_hwmod_set_module_autoidle(oh[1], 0);
val = MCBSP_ST_READ(mcbsp, SSELCR);
@@ -303,9 +317,11 @@ static void omap_st_chgain(struct omap_mcbsp *mcbsp)
{
u16 w;
struct omap_mcbsp_st_data *st_data = mcbsp->st_data;
+ struct omap_hwmod **oh;
+
+ oh = find_hwmods_by_dev(mcbsp->dev);
- w = MCBSP_ST_READ(mcbsp, SYSCONFIG);
- MCBSP_ST_WRITE(mcbsp, SYSCONFIG, w & ~(ST_AUTOIDLE));
+ omap_hwmod_set_module_autoidle(oh[1], 0);
w = MCBSP_ST_READ(mcbsp, SSELCR);
@@ -648,49 +664,46 @@ EXPORT_SYMBOL(omap_mcbsp_get_dma_op_mode);
static inline void omap34xx_mcbsp_request(struct omap_mcbsp *mcbsp)
{
+ struct omap_hwmod **oh;
+
+ oh = find_hwmods_by_dev(mcbsp->dev);
/*
* Enable wakup behavior, smart idle and all wakeups
* REVISIT: some wakeups may be unnecessary
*/
if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
- u16 syscon;
-
- syscon = MCBSP_READ(mcbsp, SYSCON);
- syscon &= ~(ENAWAKEUP | SIDLEMODE(0x03) | CLOCKACTIVITY(0x03));
if (mcbsp->dma_op_mode == MCBSP_DMA_MODE_THRESHOLD) {
- syscon |= (ENAWAKEUP | SIDLEMODE(0x02) |
- CLOCKACTIVITY(0x02));
- MCBSP_WRITE(mcbsp, WAKEUPEN, XRDYEN | RRDYEN);
+ omap_hwmod_enable_wakeup(oh[0]);
+ omap_hwmod_set_slave_idlemode(oh[0],
+ HWMOD_IDLEMODE_SMART);
} else {
- syscon |= SIDLEMODE(0x01);
+ omap_hwmod_disable_wakeup(oh[0]);
+ omap_hwmod_set_slave_idlemode(oh[0],
+ HWMOD_IDLEMODE_NO);
}
-
- MCBSP_WRITE(mcbsp, SYSCON, syscon);
}
}
static inline void omap34xx_mcbsp_free(struct omap_mcbsp *mcbsp)
{
+ struct omap_hwmod **oh;
+
+ oh = find_hwmods_by_dev(mcbsp->dev);
/*
* Disable wakup behavior, smart idle and a
