[v2,0/3] mach-omap2: handle autoidle denial

Message

Andreas Kemnade Nov. 10, 2018, 8:31 p.m. UTC

On the gta04 with a dm3730 omap_hdq does not work properly when the
device enters lower power states. Idling uart1 and 2 is enough
to show up that problem, if there are no other things enabled.
Further research reveals that hdq iclk must not be turned off during
transfers, also according to the TRM. That fact is also correctly described
in the flags but the code to handle that is incomplete.

To handle multiple users of a single ick, autoidle is disabled
when a user of that ick requires that (has the OCPIF_SWSUP_IDLE))

Changes since v1:
- uses spinlocks instead of mutexes
- invert counter logic
- check whether clock type is basic

Andreas Kemnade (3):
  clk: ti: add a usecount for autoidle
  clk: ti: check clock type before doing autoidle ops
  arm: omap_hwmod disable ick autoidling when a hwmod requires that

 arch/arm/mach-omap2/omap_hwmod.c | 16 ++++++++++----
 drivers/clk/ti/autoidle.c        | 48 +++++++++++++++++++++++++++++++---------
 include/linux/clk/ti.h           |  1 +
 3 files changed, 51 insertions(+), 14 deletions(-)

Comments

Stephen Boyd Nov. 30, 2018, 12:26 a.m. UTC | #1

Quoting Andreas Kemnade (2018-11-10 12:31:12)
> On the gta04 with a dm3730 omap_hdq does not work properly when the
> device enters lower power states. Idling uart1 and 2 is enough
> to show up that problem, if there are no other things enabled.
> Further research reveals that hdq iclk must not be turned off during
> transfers, also according to the TRM. That fact is also correctly described
> in the flags but the code to handle that is incomplete.
> 
> To handle multiple users of a single ick, autoidle is disabled
> when a user of that ick requires that (has the OCPIF_SWSUP_IDLE))
> 
> Changes since v1:
> - uses spinlocks instead of mutexes
> - invert counter logic
> - check whether clock type is basic
> 

I'm expecting someone like Tero or Tony to review this.

Tero Kristo Nov. 30, 2018, 7:37 a.m. UTC | #2

On 30/11/2018 02:26, Stephen Boyd wrote:
> Quoting Andreas Kemnade (2018-11-10 12:31:12)
>> On the gta04 with a dm3730 omap_hdq does not work properly when the
>> device enters lower power states. Idling uart1 and 2 is enough
>> to show up that problem, if there are no other things enabled.
>> Further research reveals that hdq iclk must not be turned off during
>> transfers, also according to the TRM. That fact is also correctly described
>> in the flags but the code to handle that is incomplete.
>>
>> To handle multiple users of a single ick, autoidle is disabled
>> when a user of that ick requires that (has the OCPIF_SWSUP_IDLE))
>>
>> Changes since v1:
>> - uses spinlocks instead of mutexes
>> - invert counter logic
>> - check whether clock type is basic
>>
> 
> I'm expecting someone like Tero or Tony to review this.
> 

Rest of it looks fine to me, except for the discussion under the 
CLK_IS_BASIC flag, which might trigger a bigger rework of the code.

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Stephen Boyd Nov. 30, 2018, 7:57 a.m. UTC | #3

Quoting Tero Kristo (2018-11-29 23:37:35)
> On 30/11/2018 02:26, Stephen Boyd wrote:
> > Quoting Andreas Kemnade (2018-11-10 12:31:12)
> >> On the gta04 with a dm3730 omap_hdq does not work properly when the
> >> device enters lower power states. Idling uart1 and 2 is enough
> >> to show up that problem, if there are no other things enabled.
> >> Further research reveals that hdq iclk must not be turned off during
> >> transfers, also according to the TRM. That fact is also correctly described
> >> in the flags but the code to handle that is incomplete.
> >>
> >> To handle multiple users of a single ick, autoidle is disabled
> >> when a user of that ick requires that (has the OCPIF_SWSUP_IDLE))
> >>
> >> Changes since v1:
> >> - uses spinlocks instead of mutexes
> >> - invert counter logic
> >> - check whether clock type is basic
> >>
> > 
> > I'm expecting someone like Tero or Tony to review this.
> > 
> 
> Rest of it looks fine to me, except for the discussion under the 
> CLK_IS_BASIC flag, which might trigger a bigger rework of the code.
> 

Is that a Reviewed-by tag?

Tero Kristo Nov. 30, 2018, 9:21 a.m. UTC | #4

On 30/11/2018 09:57, Stephen Boyd wrote:
> Quoting Tero Kristo (2018-11-29 23:37:35)
>> On 30/11/2018 02:26, Stephen Boyd wrote:
>>> Quoting Andreas Kemnade (2018-11-10 12:31:12)
>>>> On the gta04 with a dm3730 omap_hdq does not work properly when the
>>>> device enters lower power states. Idling uart1 and 2 is enough
>>>> to show up that problem, if there are no other things enabled.
>>>> Further research reveals that hdq iclk must not be turned off during
>>>> transfers, also according to the TRM. That fact is also correctly described
>>>> in the flags but the code to handle that is incomplete.
>>>>
>>>> To handle multiple users of a single ick, autoidle is disabled
>>>> when a user of that ick requires that (has the OCPIF_SWSUP_IDLE))
>>>>
>>>> Changes since v1:
>>>> - uses spinlocks instead of mutexes
>>>> - invert counter logic
>>>> - check whether clock type is basic
>>>>
>>>
>>> I'm expecting someone like Tero or Tony to review this.
>>>
>>
>> Rest of it looks fine to me, except for the discussion under the
>> CLK_IS_BASIC flag, which might trigger a bigger rework of the code.
>>
> 
> Is that a Reviewed-by tag?
> 

Not yet, lets see where discussion ends up with patch #2. :)

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki