linux-kernel - Re: [PATCHv3] driver: serial: prevent UART console idle on suspend while using "no_console

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 10 Apr 2013 15:13:44 +0530
From:	Sourav Poddar <sourav.poddar@...com>
To:	"Bedia, Vaibhav" <vaibhav.bedia@...com>
CC:	Kevin Hilman <khilman@...aro.org>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"tony@...mide.com" <tony@...mide.com>,
	"rmk+kernel@....linux.org.uk" <rmk+kernel@....linux.org.uk>,
	"linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
	"Shilimkar, Santosh" <santosh.shilimkar@...com>,
	"Balbi, Felipe" <balbi@...com>, "Nayak, Rajendra" <rnayak@...com>
Subject: Re: [PATCHv3] driver: serial: prevent UART console idle on suspend
 while using "no_console_suspend"

Hi Vaibhav,
On Wednesday 10 April 2013 11:49 AM, Bedia, Vaibhav wrote:
> Hi Sourav, Kevin,
>
> On Wed, Apr 10, 2013 at 11:37:28, Poddar, Sourav wrote:
>> Hi,
>> On Wednesday 10 April 2013 12:37 AM, Kevin Hilman wrote:
>>> Sourav Poddar<sourav.poddar@...com>   writes:
>>>
>>>> Hi Kevin,
>>>> On Friday 05 April 2013 11:10 PM, Kevin Hilman wrote:
>>>>> Sourav Poddar<sourav.poddar@...com>    writes:
>>>>>
>>>>>> With dt boot, uart wakeup after suspend is non functional while using
>>>>>> "no_console_suspend" in the bootargs. With "no_console_suspend" used, we
>>>>>> should prevent the runtime suspend of the uart port which is getting used
>>>>>> as an console.
>>>>>>
>>>>>> Cc: Santosh Shilimkar<santosh.shilimkar@...com>
>>>>>> Cc: Felipe Balbi<balbi@...com>
>>>>>> Cc: Rajendra nayak<rnayak@...com>
>>>>>> Tested on omap5430evm, omap4430sdp.
>>>>>>
>>>>>> Signed-off-by: Sourav Poddar<sourav.poddar@...com>
>>>>> Rather than make these special checks inside the driver's runtime PM
>>>>> callbacks, you should just disable runtime PM (pm_runtime_disable())
>>>>>
>>>>> Then, this should be broken into 2 patches.
>>>>>
>>>>> 1) serial core: add the '->is_console' flag.  (nit on naming: don't call
>>>>>       it port_is_console, since the struct is already a uart_port)
>>>>>
>>>>> 2) In the OMAP UART driver's ->prepare callback, check the is_console flag
>>>>>       and pm_runtime_disable() accordingly  (then pm_runtime_enable() in
>>>>>       the drivers's ->complete callback.
>>>>>
>>>>> Kevin
>>>> I was working on your above suggestions, but realised there is not
>>>> only console
>>>> uart which has the requirement of keeping the clocks enabled while going on
>>>> suspend.
>>>>
>>>> If you see arch/arm/boot/dts/am33xx.dtsi, there is a ocmcram which has
>>>> "no_idle_on_suspend" property used.
>>> Can you please ask the AM33xx folks how (and why) this is being used?
>>>
>>> I don't see/find a driver for this device in mainline, so without a
>>> driver this flag will not be used.
>>>
>> Looping in Vaibhav Bedia for ocmcram..
>>
>> [Vaibhav]:
>>     There is a discussion going on about a cleaner way of handling
>>     ti, no_idle_on_suspend" part (as this is a sort of hack). We got a way
>>     around for UART ($subject) by making serial core/driver handle this
>> for us.
>>     But with this, we will delete codes around "no_idle_on_suspend" flag in
>>     omap_device file.
>>
>>     But, we realised that its not only UART which requires the clocks to
>> be active
>>     whie going for suspend. There is a dts entry for ocmcram also.
>>
>>     As Kevin also pointed out, we don't see a driver for this device in
>> mainline, It would be
>>     great if you can explain how its getting used?
>>
>>     You can find the complete discussion on v3 here:
>>        https://lkml.org/lkml/2013/4/5/239
>>
> The flag in question is used to ensure that the clock to OCMC RAM is not
> disabled by the PM code.
>
>  From the changelog which added this flag:
>
> "Note: OCMC RAM is part of the PER power domain and supports
>   retention. The assembly code for low power entry/exit will
>   run from OCMC RAM. To ensure that the OMAP PM code does not
>   attempt to disable the clock to OCMC RAM as part of the
>   suspend process add the no_idle_on_suspend flag."
>
> We had discussed about the usage of this flag in the RFC version
> of the patch [1].
>
Yes, had a look at that and found your situation similar to UART.

But how exactly this gets used, I mean I don't  see any drivers/ in mainline
making use of this compatible string  "ti,am3352-ocmcram".  ?
> Regards,
> Vaibhav
>
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/129510.html

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/