Re: [PATCH] ARM: OMAP2+: hwmod code/data: fix 32K sync timer

["Cousson, Benoit" <b-cousson@ti.com>]

Salut Paul,

On 6/6/2012 12:55 AM, Paul Walmsley wrote:
> Hello Benoît
>
> On Tue, 29 May 2012, Cousson, Benoit wrote:
>
>> On 5/25/2012 11:56 PM, Paul Walmsley wrote:
>>
>>> This patch is effectively a workaround for a hardware oversight.  A
>>> better hardware approach would have been to implement a smart-idle
>>> target idle mode for this IP block.  The smart-idle mode in this case
>>> would behave identically to the force-idle mode.
>>
>> I'm not sure to follow you here :-)
>
> I should rewrite that paragraph.  It is not as clear as it could be.
>
>> force-idle is almost equivalent to smart-idle, so it is the right
>> workaround when smart-idle is not implemented.
>>
>> In force-idle idleack = idlereq. Whereas in smart-idle idleack = idlereq
>> if internal /OCP activity is completed.
>
> It would be nice if the hardware people just implemented smart-idle on
> all IP blocks.  Section 3.1.1.1.2 "Module-Level Clock Management" of The
> OMAP4430 TRM Rev. vAA states:
>
> "Smart-idle mode is the preferred mode of operation, while forced-idle and
> no-idle modes are intended for debugging purposes."
>
> Then no flags or software workarounds would be needed, except for
> debugging.

I know... I'm dreaming as well of HW that stick to the spec :-)

>> So in fact, I'm wondering if a new flag is needed. We can potentially apply that if
>> idlemodes == (SIDLE_FORCE | SIDLE_NO).
>>
>> We need to check which IP will have that to ensure that does not add any side-effects.
>
> I guess that means me :-)

Hehe, not really, I did it for OMAP4/5. counter_32k is the only IP with 
that broken mode. GPU and IVA does have some missing modes as well but 
at least smart_idle is there.

Regards,
Benoit

>> BTW, please note that the current idlemodes flags are wrong for the
>> counter 32k. I've just figured out that fixing the STANDBY flags for
>> some OMAP5 IPs. I'll add that fix in my WIP OMAP4 hwmod data cleanup for
>> 3.6.
>>
>> Something like that:
>
> Okay will queue up a fix for 3.5-rc.
>
>>>    			.context_offs = OMAP4_RM_WKUP_SYNCTIMER_CONTEXT_OFFSET,
>>>    		},
>>>    	},
>>> +	.flags		= HWMOD_ALWAYS_FORCE_SIDLE,
>>
>> I'm confused by the location and the value, both linus/master and
>> lo/master branches does already have a flag for the counter_32k:
>>
>> /* counter_32k */
>> static struct omap_hwmod omap44xx_counter_32k_hwmod = {
>>          .name           = "counter_32k",
>>          .class          =&omap44xx_counter_hwmod_class,
>>          .clkdm_name     = "l4_wkup_clkdm",
>>          .flags          = HWMOD_SWSUP_SIDLE,
>>          .main_clk       = "sys_32k_ck",
>>          .prcm = {
>>                  .omap4 = {
>>                          .clkctrl_offs = OMAP4_CM_WKUP_SYNCTIMER_CLKCTRL_OFFSET,
>>                          .context_offs = OMAP4_RM_WKUP_SYNCTIMER_CONTEXT_OFFSET,
>>                  },
>>          },
>> };
>>
>> So the patch should be slightly different, I guess ?
>>
>>   	.class          =&omap44xx_counter_hwmod_class,
>>   	.clkdm_name     = "l4_wkup_clkdm",
>> -	.flags          = HWMOD_SWSUP_SIDLE,
>> +	.flags		= HWMOD_ALWAYS_FORCE_SIDLE,
>>   	.main_clk       = "sys_32k_ck",
>>   	.prcm = {
>>
>>
>> This is the same issue for OMAP3 data.
>>
>> What base branch are you using?
>
> The wrong one, evidently.  Will ensure this is fixed.
>
>
> - Paul

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