From: sourav.poddar@ti•com (a0131647)
To: linux-arm-kernel@lists•infradead.org
Subject: [RFC/RFT PATCH 2/2] SERIAL: OMAP: Remove the idle handling from the driver
Date: Fri, 15 Feb 2013 19:15:02 +0530 [thread overview]
Message-ID: <511E3BDE.9040105@ti.com> (raw)
In-Reply-To: <511E3B8E.20806@ti.com>
Hi,
On Friday 15 February 2013 07:13 PM, Santosh Shilimkar wrote:
> On Friday 15 February 2013 07:04 PM, Felipe Balbi wrote:
>> Hi,
>>
>> On Fri, Feb 15, 2013 at 06:49:20PM +0530, Santosh Shilimkar wrote:
>>>>> @@ -279,8 +259,6 @@ static void serial_omap_stop_tx(struct
>>>>> uart_port *port)
>>>>> serial_out(up, UART_IER, up->ier);
>>>>> }
>>>>>
>>>>> - serial_omap_set_forceidle(up);
>>>>> -
>>>>> pm_runtime_mark_last_busy(up->dev);
>>>>> pm_runtime_put_autosuspend(up->dev);
>>>>> }
>>>>> @@ -348,7 +326,6 @@ static void serial_omap_start_tx(struct
>>>>> uart_port *port)
>>>>>
>>>>> pm_runtime_get_sync(up->dev);
>>>>> serial_omap_enable_ier_thri(up);
>>>>> - serial_omap_set_noidle(up);
>>>>
>>>> this patch is changing behavior. Currently driver has:
>>>>
>>>> start_tx()
>>>> get_sync()
>>>> set_noidle()
>>>> put_autosuspend()
>>>>
>>>> ....
>>>>
>>>> stop_tx()
>>>> get_sync()
>>>> set_force_idle()
>>>> put_autosuspend()
>>>>
>>>> with this change, you will have:
>>>>
>>>> start_tx()
>>>> get_sync()
>>>> set_noidle()
>>>> put_autosuspend()
>>>> set_force_idle()
>>>>
>>>> this in itself might be enough to go back to corrupted serial if
>>>> put_autosuspend is so quick that set_force_idle() is called before all
>>>> data has been shifted out of FIFO and through the UART lines.
>>>>
>>> Really. Infact the old sequence was buggy because you are setting
>>> force_idle even before suspending the device. And that force idle
>>
>> then that bug has to be fixed first and patch needs to be sent to stable
>> before we change that part of the code, wouldn't you agree ?
>>
> Agree. Will be good to get that fixed for stable. Probably Sourabh
> can fix that one.
>
Yes, will send a patch fix for this.
>>> isn't really force idle but setting ip to smart idle. Just look at
>>> what serial.c
>>
>> indeed.
>>
>>>> Before doing this, you should at least test that removing
>>>> pm_runtime_mark_last_busy() and pm_runtime_put_autosuspend() from
>>>> start_tx() and removing pm_runtime_get_sync() from stop_tx() works
>>>> fine.
>>>>
>>> Seems to work for me with above changes as well. Can you please
>>> try out and see if you see any issue. I doubt you will...
>>
>> what I'm saying is that current code, as you put it yourself, is buggy,
>> so we ought to fix the bugs first before changing behavior. If not for
>> anything else, at least to have a clean tree which we can bisect.
>>
>>>> Also, $SUBJECT isn't improving the situation regarding UART Wakeup,
>>>> there is still the regression of UART never being wakeup capable.
>>>>
>>> You are mixing the stuff here. The subject is correct.
>>
>> ->enable_wakeup() sets the IP to smart_idle_wakeup, which is done on
>> SYSC register. If $SUBJECT wants to fix SYSC usage, it ought to fix them
>> all.
>>
> But SYSC is already in smart_idle_wakeup() mode. I get your point
> though. The main purpose of that wakeup hook is to trigger io_ring
> and pad wakeup.
>
> BTW, Rajendra is looking at how to get rid of wakeup function pointer
> as well since that also comes in way for DT.
>
> Regards
> Santosh
>
>
~Sourav
next prev parent reply other threads:[~2013-02-15 13:45 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-15 12:08 [RFC/RFT PATCH 2/2] SERIAL: OMAP: Remove the idle handling from the driver Santosh Shilimkar
2013-02-15 12:57 ` Felipe Balbi
2013-02-15 13:19 ` Santosh Shilimkar
2013-02-15 13:34 ` Felipe Balbi
2013-02-15 13:43 ` Santosh Shilimkar
2013-02-15 13:45 ` a0131647 [this message]
2013-02-18 6:28 ` Bedia, Vaibhav
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=511E3BDE.9040105@ti.com \
--to=sourav.poddar@ti$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox