| 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
| ||
|
Message-ID: <515ECCE1.5070003@ti.com>
Date: Fri, 5 Apr 2013 18:38:49 +0530
From: Sourav Poddar <sourav.poddar@...com>
To: Santosh Shilimkar <santosh.shilimkar@...com>
CC: Kevin Hilman <khilman@...aro.org>, Felipe Balbi <balbi@...com>,
Rajendra nayak <rnayak@...com>, <tony@...mide.com>,
<rmk+kernel@....linux.org.uk>, <linux-serial@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>, <linux-omap@...r.kernel.org>
Subject: Re: [PATCH/Resend 2/2] arm: mach-omap2: prevent UART console idle
on suspend while using "no_console_suspend"
Hi Santosh,
On Friday 05 April 2013 12:45 PM, Santosh Shilimkar wrote:
> On Friday 05 April 2013 12:38 PM, Sourav Poddar wrote:
>> Hi Kevin,
>> On Wednesday 03 April 2013 11:18 PM, Kevin Hilman wrote:
>>> Sourav Poddar<sourav.poddar@...com> writes:
>>>
>>>> Hi Kevin,
>>>> On Wednesday 20 March 2013 05:36 PM, Sourav Poddar wrote:
>>>>> Realised the list to whom the patch was send got dropped. Ccing
>>>>> them all..
>>>>> On Wednesday 20 March 2013 05:18 PM, Sourav Poddar wrote:
>>>>>> Hi Kevin,
>>>>>> On Tuesday 19 March 2013 12:24 AM, Kevin Hilman wrote:
>>>>>>> Sourav Poddar<sourav.poddar@...com> writes:
>>>>>>>
>>>>>>>> With dt boot, uart wakeup after suspend is non functional on
>>>>>>>> omap4/5 while using
>>>>>>>> "no_console_suspend" in the bootargs. With "no_console_suspend"
>>>>>>>> used, od->flags
>>>>>>>> should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not
>>>>>>>> allowing the console
>>>>>>>> to idle in the suspend path. For non-dt case, this was taken care
>>>>>>>> by platform data.
>>>>>>>>
>>>>>>>> Tested on omap5430evm, omap4430sdp.
>>>>>>>>
>>>>>>>> Cc: Santosh Shilimkar<santosh.shilimkar@...com>
>>>>>>>> Cc: Felipe Balbi<balbi@...com>
>>>>>>>> Cc: Rajendra nayak<rnayak@...com>
>>>>>>>> Signed-off-by: Sourav Poddar<sourav.poddar@...com>
>>>>>>> This patch creates a dependency between omap_device (generic,
>>>>>>> device-independent code) and a specific driver (UART.)
>>>>>>>
>>>>>>> If you need to do something like this that's DT boot specific, then
>>>>>>> we probably need some late initcall in serial.c to handle this.
>>>>>>> It does
>>>>>>> not belong in omap_device.
>>>>>>>
>>>>>> The following function "omap_device_disable_idle_on_suspend(pdev)"
>>>>>> should only
>>>>>> be called once the omap device has been build, which in the case of
>>>>>> device tree is
>>>>>> done in omap_device.c file. Moreover, the above call should be
>>>>>> executed conditionally
>>>>>> and should depend on the following two parameter.
>>>>>>
>>>>>> [1] a. Whether "no_console_suspend" is set and
>>>>>> b. the device build is a console uart.
>>>>>>
>>>>>> When I look closely into the serial.c file, I realised that
>>>>>> "core_initcall(omap_serial_early_init)" gets called irrespective
>>>>>> of dt/non dt boot and will take care of most of the stuff(checking
>>>>>> whether
>>>>>> "no_console_suspend" is used and which uart is used as a console
>>>>>> uart) which the
>>>>>> $subject patch is proposing.
>>>>>>
>>>>>> But the problem is that we need to exchange the parsed information
>>>>>> from serial.c to the omap_device file for the condtional execution of
>>>>>> "omap_device_disable_idle_on_suspend"
>>>>>>
>>>>>> In this case,
>>>>>> from "serial.c" we need
>>>>>> 1. no_console_suspend = true
>>>>>> 2. strcpy(console_name, oh_name), where oh_name corresponds to
>>>>>> the console uart.
>>>>>>
>>>>>> then in "omap_device.c" do
>>>>>> if (no_console_suspend&& !strcmp(oh->name, console_name))
>>>>>> omap_device_disable_idle_on_suspend(pdev);
>>>>>>
>>>>>> Please correct if I am understanding it incorrectly.
>>>>>>
>>>>>> If the above understanding looks good to you, is there a way we can
>>>>>> make this
>>>>>> exchange of information happen between serial.c and omap_device.c file?
>>>> Any input on this?
>>>> As I explained earlier, that there is a need to parse information in
>>>> serial.c and use that in
>>>> omap_device.c only after the device is build.
>>> As I explained earlier, any device specific hacks inside omap_device
>>> should be a red flag that something has gone wrong.
>>>
>>> How about fixing the UART driver/core to not runtime suspend if
>>> no_console_suspend is given?
>>>
>>> Then we can get rid of this no_idle_on_suspend hack all together since
>>> UART is the only remaining user.
>>>
>> Yes, that can be done.
>>
>> I cooked up an experimental patch, based on your above suggestion. The patch is
>> inlined along with the mail. I tested the patch and it resolves
>> the issue.
>>
> Would be good to start a new thread.
>
I will post a new version based on your comments below.
>> Depending on whether no_console_suspend is used or not, we can set a variable
>> which can be used as a condition in runtime api's in serial driver.
>>
>> The information whether "no_console_suspend" is used or not is parsed in
>> serial.c file, and this need to be communicated to the driver layer.
>>
> WHY ?
> Serial core already knows about 'no_console_suspend' and thats used
> to manage console lock.
>
True.
>> In my experimental patch below, I have used an extern variable
>> "force_console_suspend_disable".
>>
>> Do you see a better way of handling this without using extern?
>>
>> ---------
>> From: Sourav Poddar<sourav.poddar@...com>
>> Date: Wed, 13 Mar 2013 20:32:36 +0530
>> Subject: [PATCH] arm: mach-omap2: prevent UART console idle on suspend while using "no_console_suspend"
>>
>> With dt boot, uart wakeup after suspend is non functional while using
>> "no_console_suspend" in the bootargs. With "no_console_suspend" used, od->flags
>> should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not allowing the console
>> to idle in the suspend path.
>>
>> Tested on omap5430evm, omap4430sdp.
>>
>> Signed-off-by: Sourav Poddar<sourav.poddar@...com>
>> ---
>> arch/arm/mach-omap2/serial.c | 6 ++++--
>> drivers/tty/serial/omap-serial.c | 6 +++++-
>> 2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
>> index f660156..427c407 100644
>> --- a/arch/arm/mach-omap2/serial.c
>> +++ b/arch/arm/mach-omap2/serial.c
>> @@ -51,7 +51,7 @@
>> #define DEFAULT_AUTOSUSPEND_DELAY -1
>>
>> #define MAX_UART_HWMOD_NAME_LEN 16
>> -
>> +int force_console_suspend_disable;
>> struct omap_uart_state {
>> int num;
>>
>> @@ -207,8 +207,10 @@ static int __init omap_serial_early_init(void)
>> uart_name, uart->num);
>> }
>>
>> - if (cmdline_find_option("no_console_suspend"))
>> + if (cmdline_find_option("no_console_suspend")) {
>> + force_console_suspend_disable = 1;
>> no_console_suspend = true;
>> + }
>>
>> /*
>> * omap-uart can be used for earlyprintk logs
>> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
>> index 08332f3..395ade8 100644
>> --- a/drivers/tty/serial/omap-serial.c
>> +++ b/drivers/tty/serial/omap-serial.c
>> @@ -43,6 +43,7 @@
>> #include<linux/pinctrl/consumer.h>
>> #include<linux/platform_data/serial-omap.h>
>>
>> +extern int force_console_suspend_disable;
> This is no good. You need to use the information from
> core serial layer
>
Ok. Will change.
>> #define OMAP_MAX_HSUART_PORTS 6
>>
>> #define UART_BUILD_REVISION(x, y) (((x)<< 8) | (y))
>> @@ -1582,7 +1583,7 @@ static int serial_omap_runtime_suspend(struct device *dev)
>> struct uart_omap_port *up = dev_get_drvdata(dev);
>> struct omap_uart_port_info *pdata = dev->platform_data;
>>
>> - if (!up)
>> + if (!up || force_console_suspend_disable)
>> return -EINVAL;
>>
>> if (!pdata)
>> @@ -1614,6 +1615,9 @@ static int serial_omap_runtime_resume(struct device *dev)
>>
>> int loss_cnt = serial_omap_get_context_loss_count(up);
>>
>> + if (force_console_suspend_disable)
>> + return -EINVAL;
>> +
>> if (loss_cnt< 0) {
>> dev_err(dev, "serial_omap_get_context_loss_count failed : %d\n",
>> loss_cnt);
--
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/
Powered by blists - more mailing lists