[RFC/RFT PATCH 0/2] SERIAL: OMAP: Remove idle handling from driver

[RFC/RFT PATCH 0/2] SERIAL: OMAP: Remove idle handling from driver

[Santosh Shilimkar]

OMAP UART IP needs manual idle modes based on state of the
IP. Currently this is handled by the driver with function pointers
implemented in platform code.

This however breaks in case of device tree because of missing
idle handling. 

The series tries to address the issue

Patches has been tested on OMAP4 and OMAP5 devices where the console
slugishness was observed without idle mode handling. CPUIDLE and
suspend tested ok on these devices.

Need help in testing on OMAP2, OMAP3 and AM3XXX devices. 

Santosh Shilimkar (2):
  ARM: OMAP2+: hwmod-data: UART IP needs software control of sidle
    modes
  SERIAL: OMAP: Remove the idle handling from the driver

 arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c |    3 ++
 arch/arm/mach-omap2/omap_hwmod_33xx_data.c         |    6 ++++
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c         |    4 +++
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c         |    6 +++-
 arch/arm/mach-omap2/serial.c                       |   31 --------------------
 drivers/tty/serial/omap-serial.c                   |   23 ---------------
 6 files changed, 18 insertions(+), 55 deletions(-)

-- 
1.7.9.5

[RFC/RFT PATCH 0/2] SERIAL: OMAP: Remove idle handling from driver

[a0131647]

Hi,
On Friday 15 February 2013 05:36 PM, Santosh Shilimkar wrote:
> OMAP UART IP needs manual idle modes based on state of the
> IP. Currently this is handled by the driver with function pointers
> implemented in platform code.
>
> This however breaks in case of device tree because of missing
> idle handling.
>
> The series tries to address the issue
>
> Patches has been tested on OMAP4 and OMAP5 devices where the console
> slugishness was observed without idle mode handling. CPUIDLE and
> suspend tested ok on these devices.
>
> Need help in testing on OMAP2, OMAP3 and AM3XXX devices.
>
Tested this series on omap3630 beagle board for:
1. Retention in idle and suspend.
2. Off mode in idle and suspend.

Tested-by: Sourav Poddar <sourav.poddar@ti.com>

> Santosh Shilimkar (2):
>    ARM: OMAP2+: hwmod-data: UART IP needs software control of sidle
>      modes
>    SERIAL: OMAP: Remove the idle handling from the driver
>
>   arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c |    3 ++
>   arch/arm/mach-omap2/omap_hwmod_33xx_data.c         |    6 ++++
>   arch/arm/mach-omap2/omap_hwmod_3xxx_data.c         |    4 +++
>   arch/arm/mach-omap2/omap_hwmod_44xx_data.c         |    6 +++-
>   arch/arm/mach-omap2/serial.c                       |   31 --------------------
>   drivers/tty/serial/omap-serial.c                   |   23 ---------------
>   6 files changed, 18 insertions(+), 55 deletions(-)
>

[RFC/RFT PATCH 0/2] SERIAL: OMAP: Remove idle handling from driver

[Felipe Balbi]

On Fri, Feb 15, 2013 at 06:33:58PM +0530, a0131647 wrote:
> Hi,
> On Friday 15 February 2013 05:36 PM, Santosh Shilimkar wrote:
> >OMAP UART IP needs manual idle modes based on state of the
> >IP. Currently this is handled by the driver with function pointers
> >implemented in platform code.
> >
> >This however breaks in case of device tree because of missing
> >idle handling.
> >
> >The series tries to address the issue
> >
> >Patches has been tested on OMAP4 and OMAP5 devices where the console
> >slugishness was observed without idle mode handling. CPUIDLE and
> >suspend tested ok on these devices.
> >
> >Need help in testing on OMAP2, OMAP3 and AM3XXX devices.
> >
> Tested this series on omap3630 beagle board for:
> 1. Retention in idle and suspend.
> 2. Off mode in idle and suspend.
> 
> Tested-by: Sourav Poddar <sourav.poddar@ti.com>

try setting autosuspend_delay to -1

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130215/a6093ebf/attachment.sig>

[RFC/RFT PATCH 0/2] SERIAL: OMAP: Remove idle handling from driver

[Felipe Balbi]

On Fri, Feb 15, 2013 at 03:07:45PM +0200, Felipe Balbi wrote:
> On Fri, Feb 15, 2013 at 06:33:58PM +0530, a0131647 wrote:
> > Hi,
> > On Friday 15 February 2013 05:36 PM, Santosh Shilimkar wrote:
> > >OMAP UART IP needs manual idle modes based on state of the
> > >IP. Currently this is handled by the driver with function pointers
> > >implemented in platform code.
> > >
> > >This however breaks in case of device tree because of missing
> > >idle handling.
> > >
> > >The series tries to address the issue
> > >
> > >Patches has been tested on OMAP4 and OMAP5 devices where the console
> > >slugishness was observed without idle mode handling. CPUIDLE and
> > >suspend tested ok on these devices.
> > >
> > >Need help in testing on OMAP2, OMAP3 and AM3XXX devices.
> > >
> > Tested this series on omap3630 beagle board for:
> > 1. Retention in idle and suspend.
> > 2. Off mode in idle and suspend.
> > 
> > Tested-by: Sourav Poddar <sourav.poddar@ti.com>
> 
> try setting autosuspend_delay to -1

Also try DT boot and try to wakeup from echo mem > /sys/power/state by
sending data through UART console.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130215/096d3ebe/attachment-0001.sig>

[RFC/RFT PATCH 0/2] SERIAL: OMAP: Remove idle handling from driver

[a0131647]

Hi,
On Friday 15 February 2013 06:38 PM, Felipe Balbi wrote:
> On Fri, Feb 15, 2013 at 03:07:45PM +0200, Felipe Balbi wrote:
>> On Fri, Feb 15, 2013 at 06:33:58PM +0530, a0131647 wrote:
>>> Hi,
>>> On Friday 15 February 2013 05:36 PM, Santosh Shilimkar wrote:
>>>> OMAP UART IP needs manual idle modes based on state of the
>>>> IP. Currently this is handled by the driver with function pointers
>>>> implemented in platform code.
>>>>
>>>> This however breaks in case of device tree because of missing
>>>> idle handling.
>>>>
>>>> The series tries to address the issue
>>>>
>>>> Patches has been tested on OMAP4 and OMAP5 devices where the console
>>>> slugishness was observed without idle mode handling. CPUIDLE and
>>>> suspend tested ok on these devices.
>>>>
>>>> Need help in testing on OMAP2, OMAP3 and AM3XXX devices.
>>>>
>>> Tested this series on omap3630 beagle board for:
>>> 1. Retention in idle and suspend.
>>> 2. Off mode in idle and suspend.
>>>
>>> Tested-by: Sourav Poddar<sourav.poddar@ti.com>
>> try setting autosuspend_delay to -1
> Also try DT boot and try to wakeup from echo mem>  /sys/power/state by
> sending data through UART console.
>
Suspend/resume with dt is not functional with the current mainline.

~Sourav

[RFC/RFT PATCH 0/2] SERIAL: OMAP: Remove idle handling from driver

[Santosh Shilimkar]

On Friday 15 February 2013 07:12 PM, a0131647 wrote:
> Hi,
> On Friday 15 February 2013 06:38 PM, Felipe Balbi wrote:
>> On Fri, Feb 15, 2013 at 03:07:45PM +0200, Felipe Balbi wrote:
>>> On Fri, Feb 15, 2013 at 06:33:58PM +0530, a0131647 wrote:
>>>> Hi,
>>>> On Friday 15 February 2013 05:36 PM, Santosh Shilimkar wrote:
>>>>> OMAP UART IP needs manual idle modes based on state of the
>>>>> IP. Currently this is handled by the driver with function pointers
>>>>> implemented in platform code.
>>>>>
>>>>> This however breaks in case of device tree because of missing
>>>>> idle handling.
>>>>>
>>>>> The series tries to address the issue
>>>>>
>>>>> Patches has been tested on OMAP4 and OMAP5 devices where the console
>>>>> slugishness was observed without idle mode handling. CPUIDLE and
>>>>> suspend tested ok on these devices.
>>>>>
>>>>> Need help in testing on OMAP2, OMAP3 and AM3XXX devices.
>>>>>
>>>> Tested this series on omap3630 beagle board for:
>>>> 1. Retention in idle and suspend.
>>>> 2. Off mode in idle and suspend.
>>>>
>>>> Tested-by: Sourav Poddar<sourav.poddar@ti.com>
>>> try setting autosuspend_delay to -1
>> Also try DT boot and try to wakeup from echo mem>  /sys/power/state by
>> sending data through UART console.
>>
> Suspend/resume with dt is not functional with the current mainline.
>
With DT, suspend driver itself are not getting registered.
The patch is in Paul's queue [1]

regards,
Santosh

[1] https://patchwork-mail2.kernel.org/patch/2116671/

[RFC/RFT PATCH 0/2] SERIAL: OMAP: Remove idle handling from driver

[Santosh Shilimkar]

On Friday 15 February 2013 05:36 PM, Santosh Shilimkar wrote:
> OMAP UART IP needs manual idle modes based on state of the
> IP. Currently this is handled by the driver with function pointers
> implemented in platform code.
>
> This however breaks in case of device tree because of missing
> idle handling.
>
> The series tries to address the issue
>
> Patches has been tested on OMAP4 and OMAP5 devices where the console
> slugishness was observed without idle mode handling. CPUIDLE and
> suspend tested ok on these devices.
>
> Need help in testing on OMAP2, OMAP3 and AM3XXX devices.
>
> Santosh Shilimkar (2):
>    ARM: OMAP2+: hwmod-data: UART IP needs software control of sidle
>      modes
>    SERIAL: OMAP: Remove the idle handling from the driver
>
HWMOD_SWSUP_SIDLE flag will is not what will help UART completely.
Also considering UART also needs async wakeup enabled as it implements
another such hook and attaches that through function pointer.

So some more work is needed to get that sorted out at least from
sysc point of view. That way we can deal with io_ring stuff using
pin control APIs.

Some patches will follow in attempt to address it. Stay tuned !!

Regards,
Santosh

[RFC/RFT PATCH 0/2] SERIAL: OMAP: Remove idle handling from driver

[Felipe Balbi]

Hi,

On Mon, Feb 18, 2013 at 03:34:56PM +0530, Santosh Shilimkar wrote:
> On Friday 15 February 2013 05:36 PM, Santosh Shilimkar wrote:
> >OMAP UART IP needs manual idle modes based on state of the
> >IP. Currently this is handled by the driver with function pointers
> >implemented in platform code.
> >
> >This however breaks in case of device tree because of missing
> >idle handling.
> >
> >The series tries to address the issue
> >
> >Patches has been tested on OMAP4 and OMAP5 devices where the console
> >slugishness was observed without idle mode handling. CPUIDLE and
> >suspend tested ok on these devices.
> >
> >Need help in testing on OMAP2, OMAP3 and AM3XXX devices.
> >
> >Santosh Shilimkar (2):
> >   ARM: OMAP2+: hwmod-data: UART IP needs software control of sidle
> >     modes
> >   SERIAL: OMAP: Remove the idle handling from the driver
> >
> HWMOD_SWSUP_SIDLE flag will is not what will help UART completely.
> Also considering UART also needs async wakeup enabled as it implements
> another such hook and attaches that through function pointer.

this is exactly what I said at [1], which I quote:

"Also, $SUBJECT isn't improving the situation regarding UART Wakeup,
there is still the regression of UART never being wakeup capable.

I wonder what are your ideas to sort that part out, I mean, how do you
plan to implement ->set_wake() for the tty port ?"

> So some more work is needed to get that sorted out at least from
> sysc point of view. That way we can deal with io_ring stuff using
> pin control APIs.
> 
> Some patches will follow in attempt to address it. Stay tuned !!

good

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130218/e4de2d46/attachment.sig>

[RFC/RFT PATCH 0/2] SERIAL: OMAP: Remove idle handling from driver

[Felipe Balbi]

On Mon, Feb 18, 2013 at 12:10:32PM +0200, Felipe Balbi wrote:
> > HWMOD_SWSUP_SIDLE flag will is not what will help UART completely.
> > Also considering UART also needs async wakeup enabled as it implements
> > another such hook and attaches that through function pointer.
> 
> this is exactly what I said at [1], which I quote:
> 
> "Also, $SUBJECT isn't improving the situation regarding UART Wakeup,
> there is still the regression of UART never being wakeup capable.
> 
> I wonder what are your ideas to sort that part out, I mean, how do you
> plan to implement ->set_wake() for the tty port ?"

[1] http://marc.info/?l=linux-omap&m=136093334914275&w=2

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130218/bb90f34d/attachment.sig>

[RFC/RFT PATCH 0/2] SERIAL: OMAP: Remove idle handling from driver

[Santosh Shilimkar]

On Monday 18 February 2013 03:41 PM, Felipe Balbi wrote:
> On Mon, Feb 18, 2013 at 12:10:32PM +0200, Felipe Balbi wrote:
>>> HWMOD_SWSUP_SIDLE flag will is not what will help UART completely.
>>> Also considering UART also needs async wakeup enabled as it implements
>>> another such hook and attaches that through function pointer.
>>
>> this is exactly what I said at [1], which I quote:
>>
>> "Also, $SUBJECT isn't improving the situation regarding UART Wakeup,
>> there is still the regression of UART never being wakeup capable.
>>
I also mentioned that am not touching the wakeup function pointer. The
reason an alternate is needed because wakeup function pointer back-end
comes in between. And the existing SW control flag isn't intended
completely for the purpose what we wanted to use for uart. And I didn't
expected the wakeup crap will come in between an implementation of just
slave idle bits.

>> I wonder what are your ideas to sort that part out, I mean, how do you
>> plan to implement ->set_wake() for the tty port ?"
>
> [1] http://marc.info/?l=linux-omap&m=136093334914275&w=2
>
The main need for uart wakeup is the io_ring() trigger and that needs
to happen via generic pin control API. SYSC wakeup bit isn't something
needs to be toggled so that can be decoupled. So again the idea is
to make SYSC handling transparent to UART drivers and let driver toggle
the io_ring() based on ->set_wake() as it is done today.

Regards,
Santosh

[RFC/RFT PATCH 0/2] SERIAL: OMAP: Remove idle handling from driver

[Felipe Balbi]

Hi,

On Mon, Feb 18, 2013 at 03:55:33PM +0530, Santosh Shilimkar wrote:
> >>I wonder what are your ideas to sort that part out, I mean, how do you
> >>plan to implement ->set_wake() for the tty port ?"
> >
> >[1] http://marc.info/?l=linux-omap&m=136093334914275&w=2
> >
> The main need for uart wakeup is the io_ring() trigger and that needs
> to happen via generic pin control API. SYSC wakeup bit isn't something
> needs to be toggled so that can be decoupled. So again the idea is
> to make SYSC handling transparent to UART drivers and let driver toggle
> the io_ring() based on ->set_wake() as it is done today.

We're either reading different code bases or not understanding each
other. Currently this is how ->set_wake() is implemented:

serial_omap_set_wake()
 serial_omap_enable_wakeup()
  omap_uart_enable_wakeup()
   omap_hwmod_enable_wakeup()
    if (SYSC_HAS_ENAWAKEUP) {	/* true for UART */
     _enable_wakeup()
     _write_sysconfig()
    }
    set_idle_ioring_wakeup()
     if (!oh->mux || !oh->mux->enabled)
      return;

If I read the code correctly there's nothing setting oh->mux during DT
boot. The only caller to omap_hwmod_mux_init() (which allocates and
returns omap_hwmod_mux_info pointer) is
arch/arm/mach-omap2/devices.c::omap4_keyboard_init(). This has nothing
to do with UART and, even more importantly, isn't even called during a
DT boot.

So, is there something else setting oh->mux to a valid pointer and
actually making set_idle_ioring_wakeup() do something useful ?

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130218/7a014e89/attachment-0001.sig>

[RFC/RFT PATCH 0/2] SERIAL: OMAP: Remove idle handling from driver

[Felipe Balbi]

On Mon, Feb 18, 2013 at 12:42:16PM +0200, Felipe Balbi wrote:
> Hi,
> 
> On Mon, Feb 18, 2013 at 03:55:33PM +0530, Santosh Shilimkar wrote:
> > >>I wonder what are your ideas to sort that part out, I mean, how do you
> > >>plan to implement ->set_wake() for the tty port ?"
> > >
> > >[1] http://marc.info/?l=linux-omap&m=136093334914275&w=2
> > >
> > The main need for uart wakeup is the io_ring() trigger and that needs
> > to happen via generic pin control API. SYSC wakeup bit isn't something
> > needs to be toggled so that can be decoupled. So again the idea is
> > to make SYSC handling transparent to UART drivers and let driver toggle
> > the io_ring() based on ->set_wake() as it is done today.
> 
> We're either reading different code bases or not understanding each
> other. Currently this is how ->set_wake() is implemented:
> 
> serial_omap_set_wake()
>  serial_omap_enable_wakeup()
>   omap_uart_enable_wakeup()
>    omap_hwmod_enable_wakeup()
>     if (SYSC_HAS_ENAWAKEUP) {	/* true for UART */
>      _enable_wakeup()
>      _write_sysconfig()
>     }
>     set_idle_ioring_wakeup()
>      if (!oh->mux || !oh->mux->enabled)
>       return;
> 
> If I read the code correctly there's nothing setting oh->mux during DT
> boot. The only caller to omap_hwmod_mux_init() (which allocates and
> returns omap_hwmod_mux_info pointer) is
> arch/arm/mach-omap2/devices.c::omap4_keyboard_init(). This has nothing

actually arch/arm/mach-omap2/serial.c::omap_serial_init_port() also
calls that, but it's also not used during DT boots.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130218/9f1b7bf9/attachment.sig>

[RFC/RFT PATCH 0/2] SERIAL: OMAP: Remove idle handling from driver

[Santosh Shilimkar]

On Monday 18 February 2013 04:15 PM, Felipe Balbi wrote:
> On Mon, Feb 18, 2013 at 12:42:16PM +0200, Felipe Balbi wrote:
>> Hi,
>>
>> On Mon, Feb 18, 2013 at 03:55:33PM +0530, Santosh Shilimkar wrote:
>>>>> I wonder what are your ideas to sort that part out, I mean, how do you
>>>>> plan to implement ->set_wake() for the tty port ?"
>>>>
>>>> [1] http://marc.info/?l=linux-omap&m=136093334914275&w=2
>>>>
>>> The main need for uart wakeup is the io_ring() trigger and that needs
>>> to happen via generic pin control API. SYSC wakeup bit isn't something
>>> needs to be toggled so that can be decoupled. So again the idea is
>>> to make SYSC handling transparent to UART drivers and let driver toggle
>>> the io_ring() based on ->set_wake() as it is done today.
>>
>> We're either reading different code bases or not understanding each
>> other. Currently this is how ->set_wake() is implemented:
>>
>> serial_omap_set_wake()
>>   serial_omap_enable_wakeup()
>>    omap_uart_enable_wakeup()
>>     omap_hwmod_enable_wakeup()
>>      if (SYSC_HAS_ENAWAKEUP) {	/* true for UART */
>>       _enable_wakeup()
There is no need to toggle this one. It can be set once and left as is.
>>       _write_sysconfig()
>>      }
>>      set_idle_ioring_wakeup()
So this is the part, we need to hookup with pads.

>>       if (!oh->mux || !oh->mux->enabled)
>>        return;
>>
>> If I read the code correctly there's nothing setting oh->mux during DT
>> boot. The only caller to omap_hwmod_mux_init() (which allocates and
>> returns omap_hwmod_mux_info pointer) is
>> arch/arm/mach-omap2/devices.c::omap4_keyboard_init(). This has nothing
>
> actually arch/arm/mach-omap2/serial.c::omap_serial_init_port() also
> calls that, but it's also not used during DT boots.
>
DT boot, all the function pointers are broken so wakeup as well as slave
idle-mode is broken. Nothing new here. Serial file does set the pads
as you noticed and this is what can be offloaded to generic pint control
which can triggered from driver.

Regards,
Santosh

[RFC/RFT PATCH 0/2] SERIAL: OMAP: Remove idle handling from driver

[Felipe Balbi]

Hi,

On Mon, Feb 18, 2013 at 04:25:58PM +0530, Santosh Shilimkar wrote:
> >>If I read the code correctly there's nothing setting oh->mux during DT
> >>boot. The only caller to omap_hwmod_mux_init() (which allocates and
> >>returns omap_hwmod_mux_info pointer) is
> >>arch/arm/mach-omap2/devices.c::omap4_keyboard_init(). This has nothing
> >
> >actually arch/arm/mach-omap2/serial.c::omap_serial_init_port() also
> >calls that, but it's also not used during DT boots.
> >
> DT boot, all the function pointers are broken so wakeup as well as slave
> idle-mode is broken. Nothing new here. Serial file does set the pads
> as you noticed and this is what can be offloaded to generic pint control
> which can triggered from driver.

fair enough, understood now.

cheers

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130218/10c134e7/attachment.sig>

[RFC/RFT PATCH 0/2] SERIAL: OMAP: Remove idle handling from driver

[Santosh Shilimkar]

On Monday 18 February 2013 04:43 PM, Felipe Balbi wrote:
> Hi,
>
> On Mon, Feb 18, 2013 at 04:25:58PM +0530, Santosh Shilimkar wrote:
>>>> If I read the code correctly there's nothing setting oh->mux during DT
>>>> boot. The only caller to omap_hwmod_mux_init() (which allocates and
>>>> returns omap_hwmod_mux_info pointer) is
>>>> arch/arm/mach-omap2/devices.c::omap4_keyboard_init(). This has nothing
>>>
>>> actually arch/arm/mach-omap2/serial.c::omap_serial_init_port() also
>>> calls that, but it's also not used during DT boots.
>>>
>> DT boot, all the function pointers are broken so wakeup as well as slave
>> idle-mode is broken. Nothing new here. Serial file does set the pads
>> as you noticed and this is what can be offloaded to generic pint control
>> which can triggered from driver.
>
> fair enough, understood now.
>
Just keep the thread posted, the slave idle handling with *wakeup*
bit is sorted out now. We are looking at the io_ring() and
pad wakeup stuff now. Once we have that working, we will repost
the series.

Regards,
Santosh