* Re: Deterministic behavior for TTY serial
From: Ivo Sieben @ 2012-04-19 15:42 UTC (permalink / raw)
To: Alan Cox; +Cc: linux-serial, RT, Greg KH
In-Reply-To: <20120419121938.1215b8c1@bob.linux.org.uk>
Hi Alan,
Op 19 april 2012 13:19 heeft Alan Cox <alan@linux.intel.com> het
volgende geschreven:
>> We are wondering if it is possible to give the TTY device in more
>> deterministic behavior (as in "less locks & no sleeping")
>> So in case of non blocking read/write behavior:
>> - We want directly write data to the serial_core transmit buffer and
>> return immediately.
>
> If you have the tty in raw mode then that is basically what the ldisc
> code does (plus any flow control you may have selected).
>
>> - Incoming data should be buffered, on a read data is read directly
>> from that buffer and when no data available return immediately
>
> Ditto in raw mode, and you can use O_NDELAY or the VMIN/VTIME fields to
> optimise block transfer behaviour. We do actually do an additional
> memcpy but memory copies of cached memory are so cheap it should be
> irrelevant unless trying to do megabit speeds on low end embedded
> processors.
>
I assume we use raw mode:
- We call the cfmakeraw() function.
- We also set the O_NDELAY flag, VMIN = 1, VTIME = 0
>> We have the idea that the default N_TTY line discipline introduces too
>
> Based upon what analysis ?
>
I think you are right: I did not yet do "real" analysis on the N_TTY
line discipline apart from source code analysis.
I will do some extra ftrace tests, to find out what causes the delays
in the execution time.
I'll come back to that...
>> much overhead & locking behavior what makes it less suitable for
>> deterministic serial communication on a PREEMT_RT platform.
>
> Are you using USB ports ?
>
No, it is a UART peripheral in a FPGA on our target board.
We use a self written serial_core base driver
> Alan
Thanks,
Ivo Sieben
^ permalink raw reply
* Re: Deterministic behavior for TTY serial
From: Ivo Sieben @ 2012-04-19 15:37 UTC (permalink / raw)
To: Greg KH; +Cc: linux-serial, Alan Cox, RT
In-Reply-To: <20120419001407.GA24710@kroah.com>
Hi Greg,
Op 19 april 2012 02:14 heeft Greg KH <gregkh@linuxfoundation.org> het
volgende geschreven:
> On Tue, Apr 17, 2012 at 04:38:30PM +0200, Ivo Sieben wrote:
>> Hello,
>>
>> We are currently using the TTY framework for serial communication.
>>
>> We are wondering if it is possible to give the TTY device in more
>> deterministic behavior (as in "less locks & no sleeping")
>
> What specifically are you looking for?
>
We run an application with a real-time thread, running on a high priority.
This thread does serial communication, using non blocking read/write
file I/O on a tty device, with small amounts of data (= 24 bytes).
This application runs on a AT2AM9261 processor, 200 MHz
The maximum execution time of both the read & write go up to 200 us
>> So in case of non blocking read/write behavior:
>> - We want directly write data to the serial_core transmit buffer and
>> return immediately.
>
> What is "immediately"?
>
We use non blocking read & write functions
We would like the read/write functions to always execute less than 100us
>> - Incoming data should be buffered, on a read data is read directly
>> from that buffer and when no data available return immediately
>
> That doesn't happen today?
>
> What type of latencies are you seeing today that is bothering you? What
> hardware are you expecting to work in this manner? What exact UART are
> you using? What happens when the UART buffers data?
>
> thanks,
>
> greg k-h
We use a self written serial_core device uart driver that implements a
driver for a UART peripheral in a FPGA on our target board..
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] tty: omap-serial: Keep the wakeup mechanism enabled by default
From: Raja, Govindraj @ 2012-04-19 14:30 UTC (permalink / raw)
To: Alan Cox
Cc: Kevin Hilman, linux-omap, Paul Walmsley, linux-arm-kernel,
linux-serial
In-Reply-To: <20120418200833.24ef25ff@pyramind.ukuu.org.uk>
On Thu, Apr 19, 2012 at 12:38 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> The point is that wakeups should be enabled whenever driver is in use,
>> and disabled when the driver is not in use.
>
>
> Which is the tty_port methods for initialisation and shutdown, which are
> mutex protected against each other and hang up.
>
> Not sure how the uart layer glue exposes that.
Is it okay to read the port flags to identify the port state in
driver whether the serial port shutdown ops is called from
port_suspend or due to port closure.
Since port_shutdown gets called even from uart_suspend_port
Is things like
test_bit(ASYNCB_SUSPENDED, &port->flags);
or
test_bit(ASYNCB_CLOSING, &port->flags);
allowed in low level driver ?
--
Thanks,
Govindraj.R
^ permalink raw reply
* Re: Deterministic behavior for TTY serial
From: Alan Cox @ 2012-04-19 11:19 UTC (permalink / raw)
To: Ivo Sieben; +Cc: linux-serial, RT
In-Reply-To: <CAMSQXEENByqSmK2bSxMngY2WuV86AAcX_LxTbA5JSOjT3ta_FA@mail.gmail.com>
> We are wondering if it is possible to give the TTY device in more
> deterministic behavior (as in "less locks & no sleeping")
> So in case of non blocking read/write behavior:
> - We want directly write data to the serial_core transmit buffer and
> return immediately.
If you have the tty in raw mode then that is basically what the ldisc
code does (plus any flow control you may have selected).
> - Incoming data should be buffered, on a read data is read directly
> from that buffer and when no data available return immediately
Ditto in raw mode, and you can use O_NDELAY or the VMIN/VTIME fields to
optimise block transfer behaviour. We do actually do an additional
memcpy but memory copies of cached memory are so cheap it should be
irrelevant unless trying to do megabit speeds on low end embedded
processors.
> We have the idea that the default N_TTY line discipline introduces too
Based upon what analysis ?
> much overhead & locking behavior what makes it less suitable for
> deterministic serial communication on a PREEMT_RT platform.
Are you using USB ports ?
Alan
^ permalink raw reply
* Re: Deterministic behavior for TTY serial
From: Greg KH @ 2012-04-19 0:14 UTC (permalink / raw)
To: Ivo Sieben; +Cc: linux-serial, Alan Cox, RT
In-Reply-To: <CAMSQXEENByqSmK2bSxMngY2WuV86AAcX_LxTbA5JSOjT3ta_FA@mail.gmail.com>
On Tue, Apr 17, 2012 at 04:38:30PM +0200, Ivo Sieben wrote:
> Hello,
>
> We are currently using the TTY framework for serial communication.
>
> We are wondering if it is possible to give the TTY device in more
> deterministic behavior (as in "less locks & no sleeping")
What specifically are you looking for?
> So in case of non blocking read/write behavior:
> - We want directly write data to the serial_core transmit buffer and
> return immediately.
What is "immediately"?
> - Incoming data should be buffered, on a read data is read directly
> from that buffer and when no data available return immediately
That doesn't happen today?
What type of latencies are you seeing today that is bothering you? What
hardware are you expecting to work in this manner? What exact UART are
you using? What happens when the UART buffers data?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v4 2/3] serial/8250_pci: Clear FIFOs for Intel ME Serial Over Lan device on BI
From: Greg KH @ 2012-04-18 23:00 UTC (permalink / raw)
To: Dan Williams
Cc: Sudhakar Mamillapalli, Nhan H Mai, linux-kernel, linux-serial,
Alan Cox
In-Reply-To: <CABE8wwvw1R_ObcjGLfPXhGLq1g1H54QTRZf2Y-E3XHYg+v=APg@mail.gmail.com>
On Wed, Apr 18, 2012 at 03:36:40PM -0700, Dan Williams wrote:
> On Wed, Apr 18, 2012 at 3:12 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Tue, Apr 10, 2012 at 02:10:58PM -0700, Dan Williams wrote:
> >> From: Sudhakar Mamillapalli <sudhakar@fb.com>
> >>
> >> When using Serial Over Lan (SOL) over the virtual serial port in a Intel
> >> management engine (ME) device, on device reset the serial FIFOs need to
> >> be cleared to keep the FIFO indexes in-sync between the host and the
> >> engine.
> >>
> >> On a reset the serial device assertes BI, so using that as a cue FIFOs
> >> are cleared. So for this purpose a new handle_break callback has been
> >> added. One other problem is that the serial registers might temporarily
> >> go to 0 on reset of this device. So instead of using the IER register
> >> read, if 0 returned use the ier value in uart_8250_port. This is hidden
> >> under a custom serial_in.
> >>
> >> Cc: Nhan H Mai <nhan.h.mai@intel.com>
> >> Signed-off-by: Sudhakar Mamillapalli <sudhakar@fb.com>
> >> Acked-by: Alan Cox <alan@linux.intel.com>
> >> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> >
> > What tree did you make this against?
> >
> > It fails with:
> > patching file drivers/tty/serial/8250/8250.c
> > patching file drivers/tty/serial/8250/8250.h
> > patching file drivers/tty/serial/8250/8250_pci.c
> > Hunk #2 FAILED at 1093.
> > 1 out of 2 hunks FAILED -- saving rejects to file drivers/tty/serial/8250/8250_pci.c.rej
> >
> > While that file hasn't been changed since it was moved to that location.
> >
> > Any ideas?
>
> Hmm, I think this is because this patch depends on the reverts went
> into 3.4 via tty-linus.
Ah, ok, that makes more sense, let me go merge 3.4-rc3 into tty-next and
see if that solves the problem...
Yup, it did, thanks, I'll go apply these two patches now, sorry for the
noise.
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 2/3] serial/8250_pci: Clear FIFOs for Intel ME Serial Over Lan device on BI
From: Dan Williams @ 2012-04-18 22:36 UTC (permalink / raw)
To: Greg KH
Cc: Sudhakar Mamillapalli, Nhan H Mai, linux-kernel, linux-serial,
Alan Cox
In-Reply-To: <20120418221207.GA4489@kroah.com>
On Wed, Apr 18, 2012 at 3:12 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Apr 10, 2012 at 02:10:58PM -0700, Dan Williams wrote:
>> From: Sudhakar Mamillapalli <sudhakar@fb.com>
>>
>> When using Serial Over Lan (SOL) over the virtual serial port in a Intel
>> management engine (ME) device, on device reset the serial FIFOs need to
>> be cleared to keep the FIFO indexes in-sync between the host and the
>> engine.
>>
>> On a reset the serial device assertes BI, so using that as a cue FIFOs
>> are cleared. So for this purpose a new handle_break callback has been
>> added. One other problem is that the serial registers might temporarily
>> go to 0 on reset of this device. So instead of using the IER register
>> read, if 0 returned use the ier value in uart_8250_port. This is hidden
>> under a custom serial_in.
>>
>> Cc: Nhan H Mai <nhan.h.mai@intel.com>
>> Signed-off-by: Sudhakar Mamillapalli <sudhakar@fb.com>
>> Acked-by: Alan Cox <alan@linux.intel.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>
> What tree did you make this against?
>
> It fails with:
> patching file drivers/tty/serial/8250/8250.c
> patching file drivers/tty/serial/8250/8250.h
> patching file drivers/tty/serial/8250/8250_pci.c
> Hunk #2 FAILED at 1093.
> 1 out of 2 hunks FAILED -- saving rejects to file drivers/tty/serial/8250/8250_pci.c.rej
>
> While that file hasn't been changed since it was moved to that location.
>
> Any ideas?
Hmm, I think this is because this patch depends on the reverts went
into 3.4 via tty-linus.
On my internal tree I have the following against 3.4-rc2
48013c6 Revert "serial/8250_pci: init-quirk msi support for kt serial
controller"
0f4149f Revert "serial/8250_pci: setup-quirk workaround for the kt
serial controller"
5580b22 serial/8250_pci: add a "force background timer" flag and use
it for the "kt" serial port
d8a8351 tegra, serial8250: add ->handle_break() uart_port op
1d9aafe serial/8250_pci: Clear FIFOs for Intel ME Serial Over Lan device on BI
d4cb218 serial/8250_pci: fix suspend/resume vs init/exit quirks
> Because of this, I've only applied your first patch in this series, not
> the last two.
Not sure how these things are handled in tty.git, backmerge?
--
Dan
^ permalink raw reply
* Re: [PATCH v4 2/3] serial/8250_pci: Clear FIFOs for Intel ME Serial Over Lan device on BI
From: Greg KH @ 2012-04-18 22:12 UTC (permalink / raw)
To: Dan Williams
Cc: Sudhakar Mamillapalli, Nhan H Mai, linux-kernel, linux-serial,
Alan Cox
In-Reply-To: <20120410211058.13805.75833.stgit@dwillia2-linux.jf.intel.com>
On Tue, Apr 10, 2012 at 02:10:58PM -0700, Dan Williams wrote:
> From: Sudhakar Mamillapalli <sudhakar@fb.com>
>
> When using Serial Over Lan (SOL) over the virtual serial port in a Intel
> management engine (ME) device, on device reset the serial FIFOs need to
> be cleared to keep the FIFO indexes in-sync between the host and the
> engine.
>
> On a reset the serial device assertes BI, so using that as a cue FIFOs
> are cleared. So for this purpose a new handle_break callback has been
> added. One other problem is that the serial registers might temporarily
> go to 0 on reset of this device. So instead of using the IER register
> read, if 0 returned use the ier value in uart_8250_port. This is hidden
> under a custom serial_in.
>
> Cc: Nhan H Mai <nhan.h.mai@intel.com>
> Signed-off-by: Sudhakar Mamillapalli <sudhakar@fb.com>
> Acked-by: Alan Cox <alan@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
What tree did you make this against?
It fails with:
patching file drivers/tty/serial/8250/8250.c
patching file drivers/tty/serial/8250/8250.h
patching file drivers/tty/serial/8250/8250_pci.c
Hunk #2 FAILED at 1093.
1 out of 2 hunks FAILED -- saving rejects to file drivers/tty/serial/8250/8250_pci.c.rej
While that file hasn't been changed since it was moved to that location.
Any ideas?
Because of this, I've only applied your first patch in this series, not
the last two.
thanks,
greg k-h
^ permalink raw reply
* THE G00GLE 13TH ANNIVERSARY GIVEAWAYS
From: Online Giveaway Team @ 2012-04-18 16:00 UTC (permalink / raw)
--
The owner of this email acct. has w o n £850,000 UK Pounds ( with C.G.P.N
:AS/MY/0010/011| E-T!CKET#: 899-633/UK-11-001).
As indicated in your coupon, your C.G.P.N and E-TICKET # falls under our
Asia Zone Disbursement booklet /Payout Center.
To get full info from your payout center, Please verify this email acct. is
Still in Use.
Inquiry #: +44 7031988569
_____________________________________________________________________
Florianonet Ltda.
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] serial: 8250: Add cpufreq support
From: Alan Cox @ 2012-04-18 19:47 UTC (permalink / raw)
To: Shankarmurthy, Akshay
Cc: linux-serial@vger.kernel.org, alan@linux.intel.com,
gregkh@linuxfoundation.org, paul.gortmaker@windriver.com,
jamie@jamieiles.com, swarren@nvidia.com, dianders@chromium.org,
davinci-linux-open-source@linux.davincidsp.com,
linux-kernel@vger.kernel.org
In-Reply-To: <6F6B80BD2E18844D97BB4B51FE3705C83E92A183@DBDE01.ent.ti.com>
> As per my understanding, tty count keeps the count of tty_open and tty_release, that is
> how many open and close happened on the tty device file(/dev/ttyS*).
> The requirement here is a tty count check, if it is greater than zero then only it should
> proceed for further execution.
The physical port has a different lifetime to the struct tty_struct
(which may change many times within the lifetime of the port). You can't
simply go from the port to a tty struct and assume this is safe and will
not change.
> Kindly elaborate more on the same if it's not correct.
Look at how tty_port_tty_get is used
^ permalink raw reply
* Re: [PATCH] tty: omap-serial: Keep the wakeup mechanism enabled by default
From: Alan Cox @ 2012-04-18 19:08 UTC (permalink / raw)
To: Kevin Hilman
Cc: Raja, Govindraj, linux-omap, Paul Walmsley, linux-arm-kernel,
linux-serial
In-Reply-To: <87mx69nms2.fsf@ti.com>
> The point is that wakeups should be enabled whenever driver is in use,
> and disabled when the driver is not in use.
Which is the tty_port methods for initialisation and shutdown, which are
mutex protected against each other and hang up.
Not sure how the uart layer glue exposes that.
Alan
^ permalink raw reply
* Re: [PATCH 2/7] UART: OMAP: Cut the clock in the error cases
From: Shubhrajyoti @ 2012-04-18 15:13 UTC (permalink / raw)
To: Kevin Hilman
Cc: linux-serial, linux-omap, linux-arm-kernel, stable, Govindraj.R
In-Reply-To: <87bompp2bk.fsf@ti.com>
On Wednesday 18 April 2012 07:30 PM, Kevin Hilman wrote:
>>>> >>> if (ret < 0) {
>>>> >>> serial_omap_enable_ier_thri(up);
>>>> >>> + pm_runtime_mark_last_busy(&up->pdev->dev);
>>>> >>> + pm_runtime_put_autosuspend(&up->pdev->dev);
>>> >> Why the autosuspend version here?
>>> >>
>>> >> Kevin
>>> >>
>>> >>
>> > In case the request_dma fails we enable the thri( effectively like intr
>> > mode) so
>> > I thought of using the autosuspend version here .
>> >
>> > Do you prefer put version instead ?
> Not necessarily, it should just be well described in the changelog.
>
> Kevin
Yes agree completely. Will describe that in the changelog.
^ permalink raw reply
* Re: [PATCH] tty: omap-serial: Keep the wakeup mechanism enabled by default
From: Russell King - ARM Linux @ 2012-04-18 15:02 UTC (permalink / raw)
To: Kevin Hilman
Cc: Raja, Govindraj, Paul Walmsley, linux-omap, linux-serial,
linux-arm-kernel
In-Reply-To: <87mx69nms2.fsf@ti.com>
On Wed, Apr 18, 2012 at 07:21:33AM -0700, Kevin Hilman wrote:
> "Raja, Govindraj" <govindraj.raja@ti.com> writes:
>
> > On Wed, Apr 18, 2012 at 4:55 AM, Kevin Hilman <khilman@ti.com> wrote:
> >> "Govindraj.R" <govindraj.raja@ti.com> writes:
> >>
> >>> From: "Govindraj.R" <govindraj.raja@ti.com>
> >>>
> >>> The wakeups can be left enabled by default and should be disabled
> >>> only when disabled from sysfs and while entering suspend.
> >>
> >> Left enabled? That assumes something else has initizlied them, but we
> >> can't make that assumption.
> >>
> >> First, wakeups should be disabled when ->probe has finished. Then,
> >> they should be enabled whenever driver is in use, and disabled when
> >> the driver is not in use.
> >>
> >> I'm not familiar enough with uart_ops, but it looks like they should
> >> probably be enabled in uart_ops->startup and disabled in
> >> uart_ops->shutdown.
> >
> > uart_ops->shutdown gets called in suspend path also
> > serial_omap_suspend => uart_suspend_port = > ops->shutdown(uport);
> >
> > This will leave uart wakeup disabled in suspend path.
>
> As I said, I'm not familiar enough with uart_ops to know which are the
> right ones.
>
> Maybe ->request_port and ->release_port are the right ones?
No, the clue's there in the name - these are supposed to be for resource
management. They only get called when the port is first acquired by the
driver, and released when the port is no longer required (iow, they
cover the span in time where the driver is capable of using the device.)
They don't tell you about the lifetime that the user has the port open.
> The point is that wakeups should be enabled whenever driver is in use,
> and disabled when the driver is not in use.
Well, we don't actually tell low level drivers that kind of detail (we
assume they're rather dumb - which maybe we shouldn't.)
It looks to me like there is the possibility of checking the TTY port
flags to see whether ASYNC_INITIALIZED is set - this will be clear in
->shutdown() method if the user is closing the port, otherwise it will
be set. Same goes for the ->startup() method.
I'm not sure whether Alan would allow that kind of knowledge in low
level serial drivers though...
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] tty: omap-serial: Keep the wakeup mechanism enabled by default
From: Kevin Hilman @ 2012-04-18 14:21 UTC (permalink / raw)
To: Raja, Govindraj; +Cc: linux-omap, Paul Walmsley, linux-arm-kernel, linux-serial
In-Reply-To: <CAMrsUdL1S5GiyG0egS=CHg2fdNkyZVF-wuCNGDg547kfTUODng@mail.gmail.com>
"Raja, Govindraj" <govindraj.raja@ti.com> writes:
> On Wed, Apr 18, 2012 at 4:55 AM, Kevin Hilman <khilman@ti.com> wrote:
>> "Govindraj.R" <govindraj.raja@ti.com> writes:
>>
>>> From: "Govindraj.R" <govindraj.raja@ti.com>
>>>
>>> The wakeups can be left enabled by default and should be disabled
>>> only when disabled from sysfs and while entering suspend.
>>
>> Left enabled? That assumes something else has initizlied them, but we
>> can't make that assumption.
>>
>> First, wakeups should be disabled when ->probe has finished. Then,
>> they should be enabled whenever driver is in use, and disabled when
>> the driver is not in use.
>>
>> I'm not familiar enough with uart_ops, but it looks like they should
>> probably be enabled in uart_ops->startup and disabled in
>> uart_ops->shutdown.
>
> uart_ops->shutdown gets called in suspend path also
> serial_omap_suspend => uart_suspend_port = > ops->shutdown(uport);
>
> This will leave uart wakeup disabled in suspend path.
As I said, I'm not familiar enough with uart_ops to know which are the
right ones.
Maybe ->request_port and ->release_port are the right ones?
The point is that wakeups should be enabled whenever driver is in use,
and disabled when the driver is not in use.
Kevin
--
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
^ permalink raw reply
* Re: [PATCH 2/7] UART: OMAP: Cut the clock in the error cases
From: Kevin Hilman @ 2012-04-18 14:00 UTC (permalink / raw)
To: Shubhrajyoti
Cc: linux-serial, linux-omap, linux-arm-kernel, stable, Govindraj.R
In-Reply-To: <4F8E62D2.9080206@ti.com>
Shubhrajyoti <shubhrajyoti@ti.com> writes:
> Hi Kevin,
> Thanks for the review.
>
> On Wednesday 18 April 2012 05:36 AM, Kevin Hilman wrote:
>> Shubhrajyoti D <shubhrajyoti@ti.com> writes:
>>
>>> In the error cases the clock cut is missed. This patch intends to fix the
>>> same.
>> Please change the references to 'cut clocks' in subject/changelog here
>> (and in other patches) to use runtime suspend instead. First, runtime PM
>> calls do more than cut clocks, but they only do so when
>> usecounting/autosuspend timeouts permit.
> Yes thanks will fix it.
>>
>>
>>> Cc: stable@vger.kernel.org
>> Please hold off on Cc'ing stable until your patches are reviewed and accepted.
> OK
>>> Cc: Govindraj.R <govindraj.raja@ti.com>
>>> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
>>> ---
>>> drivers/tty/serial/omap-serial.c | 6 +++++-
>>> 1 files changed, 5 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
>>> index fe099bb..10e80bb 100644
>>> --- a/drivers/tty/serial/omap-serial.c
>>> +++ b/drivers/tty/serial/omap-serial.c
>>> @@ -319,6 +319,8 @@ static void serial_omap_start_tx(struct uart_port *port)
>>>
>>> if (ret < 0) {
>>> serial_omap_enable_ier_thri(up);
>>> + pm_runtime_mark_last_busy(&up->pdev->dev);
>>> + pm_runtime_put_autosuspend(&up->pdev->dev);
>> Why the autosuspend version here?
>>
>> Kevin
>>
>>
> In case the request_dma fails we enable the thri( effectively like intr
> mode) so
> I thought of using the autosuspend version here .
>
> Do you prefer put version instead ?
Not necessarily, it should just be well described in the changelog.
Kevin
^ permalink raw reply
* Re: [PATCH] tty: omap-serial: Keep the wakeup mechanism enabled by default
From: Raja, Govindraj @ 2012-04-18 13:16 UTC (permalink / raw)
To: Kevin Hilman; +Cc: linux-omap, Paul Walmsley, linux-arm-kernel, linux-serial
In-Reply-To: <87vckyq6ti.fsf@ti.com>
On Wed, Apr 18, 2012 at 4:55 AM, Kevin Hilman <khilman@ti.com> wrote:
> "Govindraj.R" <govindraj.raja@ti.com> writes:
>
>> From: "Govindraj.R" <govindraj.raja@ti.com>
>>
>> The wakeups can be left enabled by default and should be disabled
>> only when disabled from sysfs and while entering suspend.
>
> Left enabled? That assumes something else has initizlied them, but we
> can't make that assumption.
>
> First, wakeups should be disabled when ->probe has finished. Then,
> they should be enabled whenever driver is in use, and disabled when
> the driver is not in use.
>
> I'm not familiar enough with uart_ops, but it looks like they should
> probably be enabled in uart_ops->startup and disabled in
> uart_ops->shutdown.
uart_ops->shutdown gets called in suspend path also
serial_omap_suspend => uart_suspend_port = > ops->shutdown(uport);
This will leave uart wakeup disabled in suspend path.
--
Thanks,
Govindraj.R
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/7] UART: OMAP: Cut the clock in the error cases
From: Shubhrajyoti @ 2012-04-18 6:44 UTC (permalink / raw)
To: Kevin Hilman
Cc: linux-serial, linux-omap, linux-arm-kernel, stable, Govindraj.R
In-Reply-To: <87bomprjic.fsf@ti.com>
Hi Kevin,
Thanks for the review.
On Wednesday 18 April 2012 05:36 AM, Kevin Hilman wrote:
> Shubhrajyoti D <shubhrajyoti@ti.com> writes:
>
>> In the error cases the clock cut is missed. This patch intends to fix the
>> same.
> Please change the references to 'cut clocks' in subject/changelog here
> (and in other patches) to use runtime suspend instead. First, runtime PM
> calls do more than cut clocks, but they only do so when
> usecounting/autosuspend timeouts permit.
Yes thanks will fix it.
>
>
>> Cc: stable@vger.kernel.org
> Please hold off on Cc'ing stable until your patches are reviewed and accepted.
OK
>> Cc: Govindraj.R <govindraj.raja@ti.com>
>> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
>> ---
>> drivers/tty/serial/omap-serial.c | 6 +++++-
>> 1 files changed, 5 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
>> index fe099bb..10e80bb 100644
>> --- a/drivers/tty/serial/omap-serial.c
>> +++ b/drivers/tty/serial/omap-serial.c
>> @@ -319,6 +319,8 @@ static void serial_omap_start_tx(struct uart_port *port)
>>
>> if (ret < 0) {
>> serial_omap_enable_ier_thri(up);
>> + pm_runtime_mark_last_busy(&up->pdev->dev);
>> + pm_runtime_put_autosuspend(&up->pdev->dev);
> Why the autosuspend version here?
>
> Kevin
>
>
In case the request_dma fails we enable the thri( effectively like intr
mode) so
I thought of using the autosuspend version here .
Do you prefer put version instead ?
With Regards,
Shubhro
^ permalink raw reply
* Re: [PATCH 2/7] UART: OMAP: Cut the clock in the error cases
From: Kevin Hilman @ 2012-04-18 0:06 UTC (permalink / raw)
To: Shubhrajyoti D
Cc: linux-serial, linux-omap, linux-arm-kernel, stable, Govindraj.R
In-Reply-To: <1334588821-5224-3-git-send-email-shubhrajyoti@ti.com>
Shubhrajyoti D <shubhrajyoti@ti.com> writes:
> In the error cases the clock cut is missed. This patch intends to fix the
> same.
Please change the references to 'cut clocks' in subject/changelog here
(and in other patches) to use runtime suspend instead. First, runtime PM
calls do more than cut clocks, but they only do so when
usecounting/autosuspend timeouts permit.
> Cc: stable@vger.kernel.org
Please hold off on Cc'ing stable until your patches are reviewed and accepted.
> Cc: Govindraj.R <govindraj.raja@ti.com>
> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> ---
> drivers/tty/serial/omap-serial.c | 6 +++++-
> 1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index fe099bb..10e80bb 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -319,6 +319,8 @@ static void serial_omap_start_tx(struct uart_port *port)
>
> if (ret < 0) {
> serial_omap_enable_ier_thri(up);
> + pm_runtime_mark_last_busy(&up->pdev->dev);
> + pm_runtime_put_autosuspend(&up->pdev->dev);
Why the autosuspend version here?
Kevin
> return;
> }
> }
> @@ -1029,8 +1031,10 @@ static int serial_omap_poll_get_char(struct uart_port *port)
>
> pm_runtime_get_sync(&up->pdev->dev);
> status = serial_in(up, UART_LSR);
> - if (!(status & UART_LSR_DR))
> + if (!(status & UART_LSR_DR)) {
> + pm_runtime_put(&up->pdev->dev);
> return NO_POLL_CHAR;
> + }
>
> status = serial_in(up, UART_RX);
> pm_runtime_put(&up->pdev->dev);
^ permalink raw reply
* Re: [PATCH] tty: omap-serial: Keep the wakeup mechanism enabled by default
From: Kevin Hilman @ 2012-04-17 23:25 UTC (permalink / raw)
To: Govindraj.R; +Cc: linux-omap, Paul Walmsley, linux-arm-kernel, linux-serial
In-Reply-To: <1334579407-16970-1-git-send-email-govindraj.raja@ti.com>
"Govindraj.R" <govindraj.raja@ti.com> writes:
> From: "Govindraj.R" <govindraj.raja@ti.com>
>
> The wakeups can be left enabled by default and should be disabled
> only when disabled from sysfs and while entering suspend.
Left enabled? That assumes something else has initizlied them, but we
can't make that assumption.
First, wakeups should be disabled when ->probe has finished. Then,
they should be enabled whenever driver is in use, and disabled when
the driver is not in use.
I'm not familiar enough with uart_ops, but it looks like they should
probably be enabled in uart_ops->startup and disabled in
uart_ops->shutdown.
I've hacked up a test patch[1] which was required for module-wakeups to
work for me. I tested module-level wakeups by disabling all C-states
except C1 using the new sysfs control for disabling C-states[2].
More comments below...
> Thanks to Kevin Hilman <khilman@ti.com> for suggesting this.
> Discussion References:
> http://www.spinics.net/lists/linux-omap/msg67764.html
> http://www.spinics.net/lists/linux-omap/msg67838.html
>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Kevin Hilman <khilman@ti.com>
> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
> ---
> drivers/tty/serial/omap-serial.c | 30 +++++++++++-------------------
> 1 files changed, 11 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index d00b38e..4a92447 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -930,13 +930,6 @@ serial_omap_pm(struct uart_port *port, unsigned int state,
> serial_out(up, UART_EFR, efr);
> serial_out(up, UART_LCR, 0);
>
> - if (!device_may_wakeup(&up->pdev->dev)) {
> - if (!state)
> - pm_runtime_forbid(&up->pdev->dev);
> - else
> - pm_runtime_allow(&up->pdev->dev);
> - }
> -
> pm_runtime_put(&up->pdev->dev);
> }
>
> @@ -1184,10 +1177,16 @@ static struct uart_driver serial_omap_reg = {
> static int serial_omap_suspend(struct device *dev)
> {
> struct uart_omap_port *up = dev_get_drvdata(dev);
> + struct omap_uart_port_info *pdata = dev->platform_data;
>
> if (up) {
> uart_suspend_port(&serial_omap_reg, &up->port);
> flush_work_sync(&up->qos_work);
> +
> + if (!device_may_wakeup(dev)) {
> + pdata->enable_wakeup(up->pdev, false);
Should check for the presence of pdata->enable_wakeup() before calling.
Same thing below.
> + up->wakeups_enabled = false;
> + }
> }
>
> return 0;
> @@ -1582,18 +1581,6 @@ static int serial_omap_runtime_suspend(struct device *dev)
> if (pdata->get_context_loss_count)
> up->context_loss_cnt = pdata->get_context_loss_count(dev);
>
> - if (device_may_wakeup(dev)) {
> - if (!up->wakeups_enabled) {
> - pdata->enable_wakeup(up->pdev, true);
> - up->wakeups_enabled = true;
> - }
> - } else {
> - if (up->wakeups_enabled) {
> - pdata->enable_wakeup(up->pdev, false);
> - up->wakeups_enabled = false;
> - }
> - }
> -
> /* Errata i291 */
> if (up->use_dma && pdata->set_forceidle &&
> (up->errata & UART_ERRATA_i291_DMA_FORCEIDLE))
> @@ -1618,6 +1605,11 @@ static int serial_omap_runtime_resume(struct device *dev)
> serial_omap_restore_context(up);
> }
>
> + if (!up->wakeups_enabled) {
> + pdata->enable_wakeup(up->pdev, true);
> + up->wakeups_enabled = true;
> + }
You put the disable in ->suspend, but the enable in ->runtime_resume.
Doesn't this belong in ->resume ?
Kevin
> /* Errata i291 */
> if (up->use_dma && pdata->set_noidle &&
> (up->errata & UART_ERRATA_i291_DMA_FORCEIDLE))
[1] Feel free to fold this into your original patch.
>From 8a4a24998aaf35240f0010b172537da64351a7d6 Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@ti.com>
Date: Tue, 17 Apr 2012 16:24:05 -0700
Subject: [PATCH] omap-serial: fix default wakeup settings
---
drivers/tty/serial/omap-serial.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 4a92447..6458ec9 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -511,6 +511,7 @@ static void serial_omap_break_ctl(struct uart_port *port, int break_state)
static int serial_omap_startup(struct uart_port *port)
{
struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
unsigned long flags = 0;
int retval;
@@ -525,6 +526,9 @@ static int serial_omap_startup(struct uart_port *port)
dev_dbg(up->port.dev, "serial_omap_startup+%d\n", up->port.line);
pm_runtime_get_sync(&up->pdev->dev);
+ if (pdata->enable_wakeup)
+ pdata->enable_wakeup(up->pdev, true);
+
/*
* Clear the FIFO buffers and disable them.
* (they will be reenabled in set_termios())
@@ -589,6 +593,7 @@ static int serial_omap_startup(struct uart_port *port)
static void serial_omap_shutdown(struct uart_port *port)
{
struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
unsigned long flags = 0;
dev_dbg(up->port.dev, "serial_omap_shutdown+%d\n", up->port.line);
@@ -628,6 +633,8 @@ static void serial_omap_shutdown(struct uart_port *port)
up->uart_dma.rx_buf = NULL;
}
+ if (pdata->enable_wakeup)
+ pdata->enable_wakeup(up->pdev, false);
pm_runtime_put(&up->pdev->dev);
free_irq(up->port.irq, up);
}
@@ -1475,6 +1482,9 @@ static int serial_omap_probe(struct platform_device *pdev)
if (ret != 0)
goto err_add_port;
+ if (omap_up_info->enable_wakeup)
+ omap_up_info->enable_wakeup(pdev, false);
+
pm_runtime_put(&pdev->dev);
platform_set_drvdata(pdev, up);
return 0;
--
1.7.9.2
[1] shell snippit for disabling C-states
# CPUidle: disable everything but C1
cd /sys/devices/system/cpu/cpu0/cpuidle
for state in state[1-6]*; do
if [ -e ${state} ]; then
echo 1 > cat ${state}/disable
fi
done
^ permalink raw reply related
* Deterministic behavior for TTY serial
From: Ivo Sieben @ 2012-04-17 14:38 UTC (permalink / raw)
To: linux-serial; +Cc: Alan Cox, RT
Hello,
We are currently using the TTY framework for serial communication.
We are wondering if it is possible to give the TTY device in more
deterministic behavior (as in "less locks & no sleeping")
So in case of non blocking read/write behavior:
- We want directly write data to the serial_core transmit buffer and
return immediately.
- Incoming data should be buffered, on a read data is read directly
from that buffer and when no data available return immediately
We have the idea that the default N_TTY line discipline introduces too
much overhead & locking behavior what makes it less suitable for
deterministic serial communication on a PREEMT_RT platform.
Our first thought was that we need to use some kind of "raw" line
discipline, that directly writes data to the serial_core transmit
buffer, and buffers incoming data.
But isn't such a line discipline already available?
Or are we missing a option/flag for the N_TTY line discipline, that
will achieve the same behavior?
Regards,
Ivo Sieben
^ permalink raw reply
* RE: [PATCH] serial: 8250: Add cpufreq support
From: Shankarmurthy, Akshay @ 2012-04-17 9:43 UTC (permalink / raw)
To: Alan Cox
Cc: linux-serial@vger.kernel.org, alan@linux.intel.com,
gregkh@linuxfoundation.org, paul.gortmaker@windriver.com,
jamie@jamieiles.com, swarren@nvidia.com, dianders@chromium.org,
davinci-linux-open-source@linux.davincidsp.com,
linux-kernel@vger.kernel.org
In-Reply-To: <20120412104758.3fc8085d@pyramind.ukuu.org.uk>
On Thu, Apr 12, 2012 at 15:17:58, Alan Cox wrote:
> > > > I will have a look at this and add the lock if necessary.
> > >
> > > At the very least you need reference counts held on the tty struct and to allow for the tty having vanished under you.
> > >
> >
> > I am planning to add "tty->termios_mutex" lock. Any comments ?
>
> Explain how this will help. You need a reference count to the tty itself.
>
I have gone through the serial driver, still I am not able to figure out the requirement
of tty reference count here. Correct me if I am wrong.
As per my understanding, tty count keeps the count of tty_open and tty_release, that is
how many open and close happened on the tty device file(/dev/ttyS*).
The requirement here is a tty count check, if it is greater than zero then only it should
proceed for further execution.
Kindly elaborate more on the same if it's not correct.
>
> Alan
>
^ permalink raw reply
* [PATCH 2/7] UART: OMAP: Cut the clock in the error cases
From: Shubhrajyoti D @ 2012-04-16 15:06 UTC (permalink / raw)
To: linux-serial
Cc: linux-omap, linux-arm-kernel, Shubhrajyoti D, stable, Govindraj.R
In-Reply-To: <1334588821-5224-1-git-send-email-shubhrajyoti@ti.com>
In the error cases the clock cut is missed. This patch intends to fix the
same.
Cc: stable@vger.kernel.org
Cc: Govindraj.R <govindraj.raja@ti.com>
Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
drivers/tty/serial/omap-serial.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index fe099bb..10e80bb 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -319,6 +319,8 @@ static void serial_omap_start_tx(struct uart_port *port)
if (ret < 0) {
serial_omap_enable_ier_thri(up);
+ pm_runtime_mark_last_busy(&up->pdev->dev);
+ pm_runtime_put_autosuspend(&up->pdev->dev);
return;
}
}
@@ -1029,8 +1031,10 @@ static int serial_omap_poll_get_char(struct uart_port *port)
pm_runtime_get_sync(&up->pdev->dev);
status = serial_in(up, UART_LSR);
- if (!(status & UART_LSR_DR))
+ if (!(status & UART_LSR_DR)) {
+ pm_runtime_put(&up->pdev->dev);
return NO_POLL_CHAR;
+ }
status = serial_in(up, UART_RX);
pm_runtime_put(&up->pdev->dev);
--
1.7.4.1
^ permalink raw reply related
* [PATCH 5/7] UART: OMAP: Prevent cutting of clocks if put_sync immediately follows
From: Shubhrajyoti D @ 2012-04-16 15:06 UTC (permalink / raw)
To: linux-serial; +Cc: linux-omap, linux-arm-kernel, Shubhrajyoti D, Govindraj.R
In-Reply-To: <1334588821-5224-1-git-send-email-shubhrajyoti@ti.com>
In serial_omap_stop_tx in the dma case. We do a autosuspend and
immediately a put_sync. Prevent the call of auto suspend. The else
case is left as is.
Cc: Govindraj.R <govindraj.raja@ti.com>
Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
drivers/tty/serial/omap-serial.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 7c69b21..1593986 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -149,11 +149,10 @@ static void serial_omap_stop_tx(struct uart_port *port)
omap_stop_dma(up->uart_dma.tx_dma_channel);
omap_free_dma(up->uart_dma.tx_dma_channel);
up->uart_dma.tx_dma_channel = OMAP_UART_DMA_CH_FREE;
- pm_runtime_mark_last_busy(&up->pdev->dev);
- pm_runtime_put_autosuspend(&up->pdev->dev);
+ } else {
+ pm_runtime_get_sync(&up->pdev->dev);
}
- pm_runtime_get_sync(&up->pdev->dev);
if (up->ier & UART_IER_THRI) {
up->ier &= ~UART_IER_THRI;
serial_out(up, UART_IER, up->ier);
--
1.7.4.1
^ permalink raw reply related
* [PATCH 0/7] UART: OMAP: Updates
From: Shubhrajyoti D @ 2012-04-16 15:06 UTC (permalink / raw)
To: linux-serial; +Cc: linux-omap, linux-arm-kernel, Shubhrajyoti D
The patch series collects few previously posted patches
and adds some clock fixes to the series.
The patch series does following.
- Makes the context_loss_cnt signed to make error handling possible.
- The clock cutting is missed in the error cases, fix it.
- Make the serial_omap_console_ports depend on the macro
OMAP_MAX_HSUART_PORTS.
- use devinit and devexit for the probe and remove functions.
- Fixes to the software flow control.
The following changes since commit e816b57a337ea3b755de72bec38c10c864f23015:
Linux 3.4-rc3 (2012-04-15 18:28:29 -0700)
are available in the git repository at:
git://gitorious.org/linus-tree/linus-tree.git uart-omap-next
Shubhrajyoti D (6):
ARM: OMAP: UART: Make context_loss_cnt signed
UART: OMAP: Cut the clock in the error cases
UART: OMAP: Remove the default setting of special character
UART: OMAP: Prevent cutting of clocks if put_sync immediately follows
UART: OMAP: Remove the hardcode serial_omap_console_ports array.
UART: OMAP: Trivial optimisation of the probe and remove
Vikram Pandita (1):
UART: OMAP: fix software flow control
arch/arm/plat-omap/include/plat/omap-serial.h | 6 ++--
drivers/tty/serial/omap-serial.c | 47 ++++++++++++++-----------
2 files changed, 29 insertions(+), 24 deletions(-)
^ permalink raw reply
* [PATCH 7/7] UART: OMAP: Trivial optimisation of the probe and remove
From: Shubhrajyoti D @ 2012-04-16 15:07 UTC (permalink / raw)
To: linux-serial; +Cc: linux-omap, linux-arm-kernel, Shubhrajyoti D, Govindraj.R
In-Reply-To: <1334588821-5224-1-git-send-email-shubhrajyoti@ti.com>
The probe and remove may not be needed after initialisation and
remove the code could be be put in the devint and devexit section.
Cc: Govindraj.R <govindraj.raja@ti.com>
Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
drivers/tty/serial/omap-serial.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index cbc9d64..42ad500 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1358,7 +1358,7 @@ static struct omap_uart_port_info *of_get_uart_port_info(struct device *dev)
return omap_up_info;
}
-static int serial_omap_probe(struct platform_device *pdev)
+static int __devinit serial_omap_probe(struct platform_device *pdev)
{
struct uart_omap_port *up;
struct resource *mem, *irq, *dma_tx, *dma_rx;
@@ -1489,7 +1489,7 @@ err_port_line:
return ret;
}
-static int serial_omap_remove(struct platform_device *dev)
+static int __devexit serial_omap_remove(struct platform_device *dev)
{
struct uart_omap_port *up = platform_get_drvdata(dev);
@@ -1654,7 +1654,7 @@ MODULE_DEVICE_TABLE(of, omap_serial_of_match);
static struct platform_driver serial_omap_driver = {
.probe = serial_omap_probe,
- .remove = serial_omap_remove,
+ .remove = __devexit_p(serial_omap_remove),
.driver = {
.name = DRIVER_NAME,
.pm = &serial_omap_dev_pm_ops,
--
1.7.4.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox