* Re: [PATCH] usb: cp210x: Added support for GPIO (CP2103/4/5)
From: Alan Cox @ 2012-04-28 21:08 UTC (permalink / raw)
To: Preston Fick
Cc: Preston Fick, gregkh@linuxfoundation.org,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-serial@vger.kernel.org
In-Reply-To: <00374F755A5C474884D69FBCA77349D105219FB334@EXCAUS002.silabs.com>
> I understand that this change is all over the place in this code, so I'm willing to change it back, and just use raw usb functions contained in the ones I modified. This should make it simpler and eliminate this problem.
I would suggest you split this into two patches then. The first patch
which changes the submission handling but nothing else, and a second
patch which adds the GPIO functions.
> Is there a better way to get this type of support for our devices? The reason I'm adding this here is because our customers need and use this functionality. The way we do this on Windows and Mac is through custom ioctl calls, so I assumed this would be the appropriate way to do this here as well.
I've Cc'd the linux-serial list as well to see what people think. I'd
like to avoid chip specific custom ioctls in favour of a standardised
way of doing it, but I'm not entirely sure how that should look.
That is really a minor detail but an important one so I'd like to see
what other feedback appears over the next few days (including some
weekdays).
> >>We could also the agree how that maps onto the extra gpio lines used with
> >>SIM card readers and the like so we can standardise that.
>
> I'm open to suggestions on how to properly get this implemented, so if there is some more feedback you can give to point me in the right direction I'd be glad to consider it and resubmit.
For what sort of things are the GPIO lines generally used by customers ?
Alan
^ permalink raw reply
* RE: [PATCH] usb: cp210x: Added support for GPIO (CP2103/4/5)
From: Preston Fick @ 2012-04-28 20:30 UTC (permalink / raw)
To: Alan Cox, Preston Fick
Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20120428190502.32eaf294-38n7/U1jhRXW96NNrWNlrekiAK3p4hvP@public.gmane.org>
Hi Alan -
Thanks for the feedback, as only my third (and most intricate) patch I'm still getting used to this process.
Please see responses inline:
Kind Regards -
Preston
________________________________________
From: Alan Cox [alan-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org]
Sent: Saturday, April 28, 2012 1:05 PM
To: Preston Fick
Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org; linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Preston Fick; linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH] usb: cp210x: Added support for GPIO (CP2103/4/5)
> */
> @@ -424,8 +430,8 @@ static int cp210x_open(struct tty_struct *tty, struct usb_serial_port *port)
>
> dbg("%s - port %d", __func__, port->number);
>
> - result = cp210x_set_config_single(port, CP210X_IFC_ENABLE,
> - UART_ENABLE);
> + result = cp210x_set_config(port, REQTYPE_HOST_TO_INTERFACE,
> + CP210X_IFC_ENABLE, UART_ENABLE, NULL, 0);
>>These kind of unrelated changes make review hard. Why are they there ?
I believe that this driver was originally developed by reverse engineering our device, and wasn't developed in a way that easily configures and calls the USB specific functions - everything is sent as a device request and certain calls actually need to be interface requests. I changed this so that it fills out the URB in a more complete way than it was before by allowing the call to contain more specific information.
I understand that this change is all over the place in this code, so I'm willing to change it back, and just use raw usb functions contained in the ones I modified. This should make it simpler and eliminate this problem.
> + case IOCTL_GPIOGET:
> + if ((port_priv->bPartNumber == CP2103_PARTNUM) ||
>>All the part specific stuff in ifs rapidly gets unmaintainable. Better to
>>set up a port_priv->get_gpio/set_gpio.
Can you elaborate on this a bit - I'm not sure I follow.
> /*
> * cp210x_get_termios
> * Reads the baud rate, data bits, parity, stop bits and flow control mode
> @@ -490,14 +565,16 @@ static void cp210x_get_termios_port(struct usb_serial_port *port,
>
> dbg("%s - port %d", __func__, port->number);
>
> - cp210x_get_config(port, CP210X_GET_BAUDRATE, &baud, 4);
> + cp210x_get_config(port, REQTYPE_INTERFACE_TO_HOST,
> + CP210X_GET_BAUDRATE, 0, &baud, 4);
>>And again it disturbs all the rest of the code for no apparent good
>>reason.
>>The large number of changes all over the code for a single localised
>>feature change ought to be flagging up that it's not being done in a
>>clean way.
Understood, I can plan to resubmit.
>>The other question is whether having some custom gpio poking interface is
>>actually a good idea. I suspect probably not. The kernel gpio layer can
>>help a bit but doesn't really solve the problem as there is no way to tie
>>a gpio to a port. Given how many devices seem to have gpios these days I
>>wonder if we need a gpio setting interface via termiox.
Is there a better way to get this type of support for our devices? The reason I'm adding this here is because our customers need and use this functionality. The way we do this on Windows and Mac is through custom ioctl calls, so I assumed this would be the appropriate way to do this here as well.
>>We could also the agree how that maps onto the extra gpio lines used with
>>SIM card readers and the like so we can standardise that.
I'm open to suggestions on how to properly get this implemented, so if there is some more feedback you can give to point me in the right direction I'd be glad to consider it and resubmit.
>>Alan
This message (including any attachments) is intended only for the use of the individual or entity to which it is addressed and may contain information that is non-public, proprietary, privileged, confidential, and exempt from disclosure under applicable law or may constitute as attorney work product. If you are not the intended recipient, you are hereby notified that any use, dissemination, distribution, or copying of this communication is strictly prohibited. If you have received this communication in error, notify us immediately by telephone and (i) destroy this message if a facsimile or (ii) delete this message immediately if this is an electronic communication.
Thank you.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] usb: cp210x: Added support for GPIO (CP2103/4/5)
From: Alan Cox @ 2012-04-28 19:33 UTC (permalink / raw)
To: Uwe Bonnes; +Cc: linux-usb, linux-kernel, linux-serial
In-Reply-To: <20380.13347.472373.965813@elektron.ikp.physik.tu-darmstadt.de>
On Sat, 28 Apr 2012 20:17:07 +0200
Uwe Bonnes <bon@elektron.ikp.physik.tu-darmstadt.de> wrote:
> >>>>> "Alan" == Alan Cox <alan@lxorguk.ukuu.org.uk> writes:
>
> ...
> Alan> The other question is whether having some custom gpio poking
> Alan> interface is actually a good idea. I suspect probably not. The
> Alan> kernel gpio layer can help a bit but doesn't really solve the
> Alan> problem as there is no way to tie a gpio to a port. Given how many
> Alan> devices seem to have gpios these days I wonder if we need a gpio
> Alan> setting interface via termiox.
>
> Is this really kernel stuff or better handled in libusb(x)
Tricky to do it that way when the kernel driver owns the interface. Also
it's looking increasingly like we'll need to support a variety of "serial
and a couple of extra magic lines" type interfaces for things like SIM
readers.
Alan
^ permalink raw reply
* Re: [PATCH] usb: cp210x: Added support for GPIO (CP2103/4/5)
From: Uwe Bonnes @ 2012-04-28 18:17 UTC (permalink / raw)
To: linux-usb, linux-kernel, linux-serial
In-Reply-To: <20120428190502.32eaf294@pyramind.ukuu.org.uk>
>>>>> "Alan" == Alan Cox <alan@lxorguk.ukuu.org.uk> writes:
...
Alan> The other question is whether having some custom gpio poking
Alan> interface is actually a good idea. I suspect probably not. The
Alan> kernel gpio layer can help a bit but doesn't really solve the
Alan> problem as there is no way to tie a gpio to a port. Given how many
Alan> devices seem to have gpios these days I wonder if we need a gpio
Alan> setting interface via termiox.
Is this really kernel stuff or better handled in libusb(x)?
Bye
--
Uwe Bonnes bon@elektron.ikp.physik.tu-darmstadt.de
Institut fuer Kernphysik Schlossgartenstrasse 9 64289 Darmstadt
--------- Tel. 06151 162516 -------- Fax. 06151 164321 ----------
^ permalink raw reply
* Re: [PATCH] usb: cp210x: Added support for GPIO (CP2103/4/5)
From: Alan Cox @ 2012-04-28 18:05 UTC (permalink / raw)
To: Preston Fick; +Cc: gregkh, linux-usb, linux-kernel, preston.fick, linux-serial
In-Reply-To: <1335632134-2488-1-git-send-email-preston.fick@silabs.com>
> */
> @@ -424,8 +430,8 @@ static int cp210x_open(struct tty_struct *tty, struct usb_serial_port *port)
>
> dbg("%s - port %d", __func__, port->number);
>
> - result = cp210x_set_config_single(port, CP210X_IFC_ENABLE,
> - UART_ENABLE);
> + result = cp210x_set_config(port, REQTYPE_HOST_TO_INTERFACE,
> + CP210X_IFC_ENABLE, UART_ENABLE, NULL, 0);
These kind of unrelated changes make review hard. Why are they there ?
> + case IOCTL_GPIOGET:
> + if ((port_priv->bPartNumber == CP2103_PARTNUM) ||
All the part specific stuff in ifs rapidly gets unmaintainable. Better to
set up a port_priv->get_gpio/set_gpio.
> /*
> * cp210x_get_termios
> * Reads the baud rate, data bits, parity, stop bits and flow control mode
> @@ -490,14 +565,16 @@ static void cp210x_get_termios_port(struct usb_serial_port *port,
>
> dbg("%s - port %d", __func__, port->number);
>
> - cp210x_get_config(port, CP210X_GET_BAUDRATE, &baud, 4);
> + cp210x_get_config(port, REQTYPE_INTERFACE_TO_HOST,
> + CP210X_GET_BAUDRATE, 0, &baud, 4);
And again it disturbs all the rest of the code for no apparent good
reason.
The large number of changes all over the code for a single localised
feature change ought to be flagging up that it's not being done in a
clean way.
The other question is whether having some custom gpio poking interface is
actually a good idea. I suspect probably not. The kernel gpio layer can
help a bit but doesn't really solve the problem as there is no way to tie
a gpio to a port. Given how many devices seem to have gpios these days I
wonder if we need a gpio setting interface via termiox.
We could also the agree how that maps onto the extra gpio lines used with
SIM card readers and the like so we can standardise that.
Alan
^ permalink raw reply
* Re: [PATCH 1/2] serial/imx: add DMA support
From: Huang Shijie @ 2012-04-28 8:54 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, alan, B20596, B20223, gregkh, r58066,
linux-serial, shawn.guo, s.hauer
In-Reply-To: <201204271724.45435.arnd@arndb.de>
Hi,
> I think we should not add any ad-hoc DMA bindings for device drivers
> until we have a proper generic binding for DMA.
I hope the generic binding for DMA becomes ready as soon as possilbe.
Best Regards
Huang Shijie
^ permalink raw reply
* Re: [PATCH 1/2] serial/imx: add DMA support
From: Huang Shijie @ 2012-04-28 8:53 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Huang Shijie, B20596, B20223, gregkh, r58066, linux-serial,
shawn.guo, s.hauer, linux-arm-kernel, alan
In-Reply-To: <20120427153031.GA27792@n2100.arm.linux.org.uk>
于 2012年04月27日 23:30, Russell King - ARM Linux 写道:
> On Fri, Apr 27, 2012 at 11:18:15AM -0400, Huang Shijie wrote:
>> On Fri, Apr 27, 2012 at 5:50 AM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>>> On Fri, Apr 27, 2012 at 05:46:22PM +0800, Huang Shijie wrote:
>>>>>>> 1. How do you deal with transmitting the high-priority XON/XOFF
>>>>>>> characters (port->x_char) which occur with s/w flow control and
>>>>>>> the tty buffers fill up?
>>>>>>> 2. How do you deal with flow control in general? IOW, what happens
>>>>>>> when the remote end deasserts your CTS with h/w flow control enabled.
>>>> If the remote end deasserts my CTS, it means the remote will not send
>>>> any data.
>>>>
>>>> My DMA for RX will expire in the following steps:
>>>> [1] the UART only waits for 32 bytes time long
>>>> [2] the UART triggers an IDLE Condition Detect DMA.
>>>> [3] the dma_rx_callback() will release the DMA for Rx.
>>> Err, hang on. I think you're totally confused about hardware flow
>>> control. Certainly you're not using the correct terms for what you're
>>> describing.
>>>
>>> The CTS input normally controls the transmitter. In many hardware
>>> assisted hardware flow control setups, the deassertion of CTS merely
>>> prevents the transmitter starting a new character.
>>>
>>> This shouldn't have any effect on the receiver of the same UART at all.
>>>
>>>>>>> How does your end deal with sending RTS according to flow control
>>>>>>> conditions?
>>>>>>>
>>>> If a CTS is received after we sent out a RTS, it will follow the steps:
>>>> imx_int() --> imx_rtsint() --> uart_handle_cts_change() -->start_tx()
>>>>
>>>> The start_tx() will create an TX DMA operation, and send out the data.
>>> The generation of RTS (connected to the remote ends CTS signal) is
>>> supposed to control whether the remote end sends you characters. RTS
>> http://en.wikipedia.org/wiki/Flow_control#Hardware_flow_control
>>
>> > From the wiki, the generation of RTS (assert by the "master end") is
>> used to send data
>> from the master to slave(the remote), not control the remote end sends
>> me characters.
> Well, there's a lot of confusion over RTS. Most implementations of it
> are as per this paragraph on the above page:
>
> A non-standard symmetric alternative, commonly called "RTS/CTS handshaking,"
> was developed by various equipment manufacturers. In this scheme, CTS is no
> longer a response to RTS; instead, CTS indicates permission from the DCE
> for the DTE to send data to the DCE, and RTS indicates permission from
> the DTE for the DCE to send data to the DTE. RTS and CTS are controlled
> by the DTE and DCE respectively, each independent of the other. This was
> eventually codified in version RS-232-E (actually TIA-232-E by that time)
> by defining a new signal, "RTR (Ready to Receive)," which is CCITT V.24
> circuit 133. TIA-232-E and the corresponding international standards were
> updated to show that circuit 133, when implemented, shares the same pin
> as RTS (Request to Send), and that when 133 is in use, RTS is assumed by
> the DCE to be ON at all times
>
> This is what is actually used by all NULL modem cables, serial consoles,
> and many modems can be (sensibly) configured to support it. Many
> modems can be configured via AT commands to respond as per the above
> paragraph too.
>
> And this is the hardware flow control scheme implemented by the Linux
> Kernel for CRTSCTS, plus the hardware assisted hardware flow control
> provided by industry standard UARTs such as the 1675x and later UARTs.
>
thanks a lot.
I will read the this explanation carefully, and fix my code.
Best Regards
Huang Shijie
--
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 1/2] serial/imx: add DMA support
From: Arnd Bergmann @ 2012-04-27 17:24 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Huang Shijie, alan, B20596, B20223, gregkh, r58066, linux-serial,
shawn.guo, s.hauer
In-Reply-To: <1335436632-29499-2-git-send-email-b32955@freescale.com>
On Thursday 26 April 2012, Huang Shijie wrote:
> diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt
> index a9c0406..f27489d 100644
> --- a/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt
> +++ b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt
> @@ -8,6 +8,10 @@ Required properties:
> Optional properties:
> - fsl,uart-has-rtscts : Indicate the uart has rts and cts
> - fsl,irda-mode : Indicate the uart supports irda mode
> +- fsl,enable-dma : Indicate the uart supports DMA
> +- fsl,uart-dma-events : contains the DMA events for RX and TX,
> + The first is the RX event, while the other is TX.
> +- fsl,enable-dte: Indicate the uart works in DTE mode
>
I think we should not add any ad-hoc DMA bindings for device drivers
until we have a proper generic binding for DMA.
Arnd
^ permalink raw reply
* 222012-4-28
From: 开减免税发票13928472421张 @ 2012-04-27 19:42 UTC (permalink / raw)
To: linux-serial
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="GB2312", Size: 18 bytes --]
–)îÆÇ«‰©oêäz¹Þ–Šà
[-- Attachment #2: Bfjnswi530.gif --]
[-- Type: image/gif, Size: 5 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] serial/imx: add DMA support
From: Russell King - ARM Linux @ 2012-04-27 15:30 UTC (permalink / raw)
To: Huang Shijie
Cc: Huang Shijie, B20596, B20223, gregkh, r58066, linux-serial,
shawn.guo, s.hauer, linux-arm-kernel, alan
In-Reply-To: <CAMiH66FoEYEqudAOx7Y8GpgjfT-gX2qOi0BjG0-X8xH8u24kSw@mail.gmail.com>
On Fri, Apr 27, 2012 at 11:18:15AM -0400, Huang Shijie wrote:
> On Fri, Apr 27, 2012 at 5:50 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Fri, Apr 27, 2012 at 05:46:22PM +0800, Huang Shijie wrote:
> >>>>> 1. How do you deal with transmitting the high-priority XON/XOFF
> >>>>> characters (port->x_char) which occur with s/w flow control and
> >>>>> the tty buffers fill up?
> >>>>> 2. How do you deal with flow control in general? IOW, what happens
> >>>>> when the remote end deasserts your CTS with h/w flow control enabled.
> >> If the remote end deasserts my CTS, it means the remote will not send
> >> any data.
> >>
> >> My DMA for RX will expire in the following steps:
> >> [1] the UART only waits for 32 bytes time long
> >> [2] the UART triggers an IDLE Condition Detect DMA.
> >> [3] the dma_rx_callback() will release the DMA for Rx.
> >
> > Err, hang on. I think you're totally confused about hardware flow
> > control. Certainly you're not using the correct terms for what you're
> > describing.
> >
> > The CTS input normally controls the transmitter. In many hardware
> > assisted hardware flow control setups, the deassertion of CTS merely
> > prevents the transmitter starting a new character.
> >
> > This shouldn't have any effect on the receiver of the same UART at all.
> >
> >>>>> How does your end deal with sending RTS according to flow control
> >>>>> conditions?
> >>>>>
> >> If a CTS is received after we sent out a RTS, it will follow the steps:
> >> imx_int() --> imx_rtsint() --> uart_handle_cts_change() -->start_tx()
> >>
> >> The start_tx() will create an TX DMA operation, and send out the data.
> >
> > The generation of RTS (connected to the remote ends CTS signal) is
> > supposed to control whether the remote end sends you characters. RTS
> http://en.wikipedia.org/wiki/Flow_control#Hardware_flow_control
>
> >From the wiki, the generation of RTS (assert by the "master end") is
> used to send data
> from the master to slave(the remote), not control the remote end sends
> me characters.
Well, there's a lot of confusion over RTS. Most implementations of it
are as per this paragraph on the above page:
A non-standard symmetric alternative, commonly called "RTS/CTS handshaking,"
was developed by various equipment manufacturers. In this scheme, CTS is no
longer a response to RTS; instead, CTS indicates permission from the DCE
for the DTE to send data to the DCE, and RTS indicates permission from
the DTE for the DCE to send data to the DTE. RTS and CTS are controlled
by the DTE and DCE respectively, each independent of the other. This was
eventually codified in version RS-232-E (actually TIA-232-E by that time)
by defining a new signal, "RTR (Ready to Receive)," which is CCITT V.24
circuit 133. TIA-232-E and the corresponding international standards were
updated to show that circuit 133, when implemented, shares the same pin
as RTS (Request to Send), and that when 133 is in use, RTS is assumed by
the DCE to be ON at all times
This is what is actually used by all NULL modem cables, serial consoles,
and many modems can be (sensibly) configured to support it. Many
modems can be configured via AT commands to respond as per the above
paragraph too.
And this is the hardware flow control scheme implemented by the Linux
Kernel for CRTSCTS, plus the hardware assisted hardware flow control
provided by industry standard UARTs such as the 1675x and later UARTs.
--
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 1/2] serial/imx: add DMA support
From: Huang Shijie @ 2012-04-27 15:18 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Huang Shijie, B20596, B20223, gregkh, r58066, linux-serial,
shawn.guo, s.hauer, linux-arm-kernel, alan
In-Reply-To: <20120427095003.GK24211@n2100.arm.linux.org.uk>
On Fri, Apr 27, 2012 at 5:50 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Apr 27, 2012 at 05:46:22PM +0800, Huang Shijie wrote:
>>>>> 1. How do you deal with transmitting the high-priority XON/XOFF
>>>>> characters (port->x_char) which occur with s/w flow control and
>>>>> the tty buffers fill up?
>>>>> 2. How do you deal with flow control in general? IOW, what happens
>>>>> when the remote end deasserts your CTS with h/w flow control enabled.
>> If the remote end deasserts my CTS, it means the remote will not send
>> any data.
>>
>> My DMA for RX will expire in the following steps:
>> [1] the UART only waits for 32 bytes time long
>> [2] the UART triggers an IDLE Condition Detect DMA.
>> [3] the dma_rx_callback() will release the DMA for Rx.
>
> Err, hang on. I think you're totally confused about hardware flow
> control. Certainly you're not using the correct terms for what you're
> describing.
>
> The CTS input normally controls the transmitter. In many hardware
> assisted hardware flow control setups, the deassertion of CTS merely
> prevents the transmitter starting a new character.
>
> This shouldn't have any effect on the receiver of the same UART at all.
>
>>>>> How does your end deal with sending RTS according to flow control
>>>>> conditions?
>>>>>
>> If a CTS is received after we sent out a RTS, it will follow the steps:
>> imx_int() --> imx_rtsint() --> uart_handle_cts_change() -->start_tx()
>>
>> The start_tx() will create an TX DMA operation, and send out the data.
>
> The generation of RTS (connected to the remote ends CTS signal) is
> supposed to control whether the remote end sends you characters. RTS
http://en.wikipedia.org/wiki/Flow_control#Hardware_flow_control
From the wiki, the generation of RTS (assert by the "master end") is
used to send data
from the master to slave(the remote), not control the remote end sends
me characters.
Best Regards
Huang Shijie
> gets deasserted either by software control when you're running out of
> space to store the received characters, or (in the case of hardware
> assisted hardware flow control) your receivers FIFO fills above a
> certain watermark.
>
>>>
--
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: [Query] serial: AT91: Add SIM Driver
From: Alan Cox @ 2012-04-27 14:51 UTC (permalink / raw)
To: Chen, Bo
Cc: alan@linux.intel.com, rick@efn.org, linux-serial@vger.kernel.org,
linux-arm-kernel@lists.arm.linux.org.uk,
linux-kernel@vger.kernel.org
In-Reply-To: <7AD597EA49E297498E79AFA326C986BB0D31A5CE@penmbx02>
On Fri, 27 Apr 2012 11:19:18 +0000
"Chen, Bo" <Bo.Chen@atmel.com> wrote:
> Hi Alan,
>
> > One way would be to make ISO7816 a line discipline (the layer which sits about the tty).
>
> >Your user space interface would then be
>
> > int ldisc = N_ISO7816;
> > ioctl(tty_fd, TIOCSETD, &ldisc);
>
> Make ISO7816 a line discipline would register it in driver(atmel_serial.c or a new named driver) via tty_register_ldisc(N_ISO7816, &tty_ldisc_ops)
Yes
> If I use ioctl(tty_fd, TIOCSETD, &ldisc), it will close current line discipline and open a new line discipline related to ISO7816 (tty_register_ldisc(N_ISO7816, &tty_ldisc_ops))?
Yes
> > at which point the read/write/ioctl etc calls on your tty will also be > routed via your ldisc so you can manage them and provide ISO7816 sensible > interfaces
>
> Does it mean when I call read/write/ioctl in user space, it will call tty_ldisc_ops functions (ISO7816 line discipline has been opened) such as gsmld_read/gsmld_write/gsmld_ioctl in N_gsm.c? What I need to do is to implement the interface in tty_ldisc_ops?
Yes. The read and write methods get called with the user buffer and size
requested being passed. The ioctl handler is called as well as part of the
standard tty handlers so that a line discipline can implement its own
ioctls.
> What is the reasonable way to add this line discipline, in atmel_serial.c or create a new driver (build as a module and dynamically load)? Would it be accepted by kernel mainline?
Probably as a separate driver. That makes it easier to test and to review.
Alan
^ permalink raw reply
* RE: [Query] serial: AT91: Add SIM Driver
From: Chen, Bo @ 2012-04-27 11:19 UTC (permalink / raw)
To: Alan Cox
Cc: alan@linux.intel.com, rick@efn.org, linux-serial@vger.kernel.org,
linux-arm-kernel@lists.arm.linux.org.uk,
linux-kernel@vger.kernel.org
In-Reply-To: <20120423115440.5bb1c7eb@pyramind.ukuu.org.uk>
Hi Alan,
> One way would be to make ISO7816 a line discipline (the layer which sits about the tty).
>Your user space interface would then be
> int ldisc = N_ISO7816;
> ioctl(tty_fd, TIOCSETD, &ldisc);
Make ISO7816 a line discipline would register it in driver(atmel_serial.c or a new named driver) via tty_register_ldisc(N_ISO7816, &tty_ldisc_ops)
If I use ioctl(tty_fd, TIOCSETD, &ldisc), it will close current line discipline and open a new line discipline related to ISO7816 (tty_register_ldisc(N_ISO7816, &tty_ldisc_ops))?
> at which point the read/write/ioctl etc calls on your tty will also be > routed via your ldisc so you can manage them and provide ISO7816 sensible > interfaces
Does it mean when I call read/write/ioctl in user space, it will call tty_ldisc_ops functions (ISO7816 line discipline has been opened) such as gsmld_read/gsmld_write/gsmld_ioctl in N_gsm.c? What I need to do is to implement the interface in tty_ldisc_ops?
> Take a look at the gsmld_ parts of drivers/tty/n_gsm.c for a fairly full > example of a line discipline. You can ignore most of the rest of the code > there, the rest is implementing GSM mux and virtual ttys.
What is the reasonable way to add this line discipline, in atmel_serial.c or create a new driver (build as a module and dynamically load)? Would it be accepted by kernel mainline?
Thanks,
Bo Chen
-----Original Message-----
From: Alan Cox [mailto:alan@lxorguk.ukuu.org.uk]
Sent: 2012年4月23日 18:55
To: Chen, Bo
Cc: alan@linux.intel.com; rick@efn.org; linux-serial@vger.kernel.org; linux-arm-kernel@lists.arm.linux.org.uk; linux-kernel@vger.kernel.org
Subject: Re: [Query] serial: AT91: Add SIM Driver
> I mean to create a char driver which contains ioctl interface to control GPIOs connected to SIM card VCC and RESET.
We have a GPIO layer and existing GPIO interface support so the gpio side
if they are kept independent ought to be easy. However I wonder if the
separate gpio control will make the userspace side harder ?
> Thirdly, I want to use usual tty function (read / write) to implement the ATR / send or receive command / PPS.
> That means tty usual driver read or write will be called by user space application such as openct or else to realize the ATR and PPS.
> Openct may also call above char driver to realize the power or reset function.
That may be difficult to get right because of the tty object lifetime
and the locking rules. One way would be to make ISO7816 a line discipline
(the layer which sits about the tty).
Your user space interface would then be
int ldisc = N_ISO7816;
ioctl(tty_fd, TIOCSETD, &ldisc);
at which point the read/write/ioctl etc calls on your tty will also be
routed via your ldisc so you can manage them and provide ISO7816 sensible
interfaces.
You'd still need a way for the ldisc to ask the tty device to manage the
GPIO lines but I don't think that is too tricky to add.
That way it would at least keep everything in one place and avoid adding
new strange paths calling into the tty layer code via non tty devices.
Take a look at the gsmld_ parts of drivers/tty/n_gsm.c for a fairly full
example of a line discipline. You can ignore most of the rest of the code
there, the rest is implementing GSM mux and virtual ttys.
The two thing s it doesn't use are
tty_set_termios(tty, &whatever)
which is a helper for the ldisc or similar to set the terminal state
itself.
and
tty->driver->ops->tiocmget/tiocmset/etc
for doing things like modem lines. One way to handle the gpios
transparently might be to map them to the nearest equivalent 'modem' line
when in ISO7816 mode.
The tty also sees the ldisc change via tty->ops->set_ldisc(). This is
used by some drivers already for things like automatically selecting IRDA
FIR mode when in IRDA modes.
In your case the driver would set/exit ISO7816 mode when it saw this.
Alan
^ permalink raw reply
* Re: [PATCH 1/2] serial/imx: add DMA support
From: Russell King - ARM Linux @ 2012-04-27 9:50 UTC (permalink / raw)
To: Huang Shijie
Cc: alan, B20596, B20223, gregkh, r58066, linux-serial, shawn.guo,
s.hauer, linux-arm-kernel
In-Reply-To: <4F9A6AEE.30005@freescale.com>
On Fri, Apr 27, 2012 at 05:46:22PM +0800, Huang Shijie wrote:
>>>> 1. How do you deal with transmitting the high-priority XON/XOFF
>>>> characters (port->x_char) which occur with s/w flow control and
>>>> the tty buffers fill up?
>>>> 2. How do you deal with flow control in general? IOW, what happens
>>>> when the remote end deasserts your CTS with h/w flow control enabled.
> If the remote end deasserts my CTS, it means the remote will not send
> any data.
>
> My DMA for RX will expire in the following steps:
> [1] the UART only waits for 32 bytes time long
> [2] the UART triggers an IDLE Condition Detect DMA.
> [3] the dma_rx_callback() will release the DMA for Rx.
Err, hang on. I think you're totally confused about hardware flow
control. Certainly you're not using the correct terms for what you're
describing.
The CTS input normally controls the transmitter. In many hardware
assisted hardware flow control setups, the deassertion of CTS merely
prevents the transmitter starting a new character.
This shouldn't have any effect on the receiver of the same UART at all.
>>>> How does your end deal with sending RTS according to flow control
>>>> conditions?
>>>>
> If a CTS is received after we sent out a RTS, it will follow the steps:
> imx_int() --> imx_rtsint() --> uart_handle_cts_change() -->start_tx()
>
> The start_tx() will create an TX DMA operation, and send out the data.
The generation of RTS (connected to the remote ends CTS signal) is
supposed to control whether the remote end sends you characters. RTS
gets deasserted either by software control when you're running out of
space to store the received characters, or (in the case of hardware
assisted hardware flow control) your receivers FIFO fills above a
certain watermark.
>> It should not. prep_slave_sg() must be callable from atomic contexts.
>>
> the Documentation/dmaengine.txt does not explicitly force all the dma
> engine driver
> to do so.
>
> Add more comments for it in Documentation/dmaengine.txt?
Nevertheless, it is common practice for drivers to call prep_slave_sg()
from atomic contexts. It sounds like your DMA driver is being fixed
anyway, so these tasklets can be killed.
^ permalink raw reply
* Re: [PATCH 1/2] serial/imx: add DMA support
From: Huang Shijie @ 2012-04-27 9:46 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: alan, B20596, B20223, gregkh, r58066, linux-serial, shawn.guo,
s.hauer, linux-arm-kernel
In-Reply-To: <20120427082245.GJ24211@n2100.arm.linux.org.uk>
于 2012年04月27日 16:22, Russell King - ARM Linux 写道:
> On Fri, Apr 27, 2012 at 03:00:39PM +0800, Huang Shijie wrote:
>> 于 2012年04月26日 19:11, Russell King - ARM Linux 写道:
>>> On Thu, Apr 26, 2012 at 06:37:11PM +0800, Huang Shijie wrote:
>>>> Add the DMA support for uart RX and TX.
>>>>
>>>> Signed-off-by: Huang Shijie<b32955@freescale.com>
>>>> ---
>>> Apart from the comments below,
>>>
>>> 1. How do you deal with transmitting the high-priority XON/XOFF
>>> characters (port->x_char) which occur with s/w flow control and
>>> the tty buffers fill up?
>>> 2. How do you deal with flow control in general? IOW, what happens
>>> when the remote end deasserts your CTS with h/w flow control enabled.
If the remote end deasserts my CTS, it means the remote will not send
any data.
My DMA for RX will expire in the following steps:
[1] the UART only waits for 32 bytes time long
[2] the UART triggers an IDLE Condition Detect DMA.
[3] the dma_rx_callback() will release the DMA for Rx.
>>> How does your end deal with sending RTS according to flow control
>>> conditions?
>>>
If a CTS is received after we sent out a RTS, it will follow the steps:
imx_int() --> imx_rtsint() --> uart_handle_cts_change() -->start_tx()
The start_tx() will create an TX DMA operation, and send out the data.
>>> i
>> The UART uses the DMA for RX/TX with the hardware flow control (RTS/CTS)
>> enabled all the time.
> Your answer is too vague. Please try again.
>
>> If we use the software flow control(XON/XOFF), we should not enable the DMA.
>> I think i should add more comments about this issue.
>>
>> For example:
>> The MX6Q arm2 has two uarts, one with the DMA disabled is used for debug,
>> the other one with the DMA/RTS.CTS enabled can be used for the Bluetooth.
>>>> .../bindings/tty/serial/fsl-imx-uart.txt | 7 +
>>>> drivers/tty/serial/imx.c | 386 +++++++++++++++++++-
>>>> 2 files changed, 389 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt
>>>> index a9c0406..f27489d 100644
>>>> --- a/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt
>>>> +++ b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt
>>>> @@ -8,6 +8,10 @@ Required properties:
>>>> Optional properties:
>>>> - fsl,uart-has-rtscts : Indicate the uart has rts and cts
>>>> - fsl,irda-mode : Indicate the uart supports irda mode
>>>> +- fsl,enable-dma : Indicate the uart supports DMA
>>>> +- fsl,uart-dma-events : contains the DMA events for RX and TX,
>>>> + The first is the RX event, while the other is TX.
>>>> +- fsl,enable-dte: Indicate the uart works in DTE mode
>>>>
>>>> Example:
>>>>
>>>> @@ -16,4 +20,7 @@ uart@73fbc000 {
>>>> reg =<0x73fbc000 0x4000>;
>>>> interrupts =<31>;
>>>> fsl,uart-has-rtscts;
>>>> + fsl,enable-dma;
>>>> + fsl,uart-dma-events =<xx xx>;
>>>> + fsl,enable-dte;
>>>> };
>>>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
>>>> index e7fecee..65ba24d 100644
>>>> --- a/drivers/tty/serial/imx.c
>>>> +++ b/drivers/tty/serial/imx.c
>>>> @@ -47,9 +47,11 @@
>>>> #include<linux/slab.h>
>>>> #include<linux/of.h>
>>>> #include<linux/of_device.h>
>>>> +#include<linux/dma-mapping.h>
>>>>
>>>> #include<asm/io.h>
>>>> #include<asm/irq.h>
>>>> +#include<mach/dma.h>
>>>> #include<mach/imx-uart.h>
>>>>
>>>> /* Register definitions */
>>>> @@ -82,6 +84,7 @@
>>>> #define UCR1_ADBR (1<<14) /* Auto detect baud rate */
>>>> #define UCR1_TRDYEN (1<<13) /* Transmitter ready interrupt enable */
>>>> #define UCR1_IDEN (1<<12) /* Idle condition interrupt */
>>>> +#define UCR1_ICD_REG(x) (((x)& 3)<< 10) /* idle condition detect */
>>>> #define UCR1_RRDYEN (1<<9) /* Recv ready interrupt enable */
>>>> #define UCR1_RDMAEN (1<<8) /* Recv ready DMA enable */
>>>> #define UCR1_IREN (1<<7) /* Infrared interface enable */
>>>> @@ -125,6 +128,7 @@
>>>> #define UCR4_ENIRI (1<<8) /* Serial infrared interrupt enable */
>>>> #define UCR4_WKEN (1<<7) /* Wake interrupt enable */
>>>> #define UCR4_REF16 (1<<6) /* Ref freq 16 MHz */
>>>> +#define UCR4_IDDMAEN (1<<6) /* DMA IDLE Condition Detected */
>>>> #define UCR4_IRSC (1<<5) /* IR special case */
>>>> #define UCR4_TCEN (1<<3) /* Transmit complete interrupt enable */
>>>> #define UCR4_BKEN (1<<2) /* Break condition interrupt enable */
>>>> @@ -134,6 +138,7 @@
>>>> #define UFCR_RFDIV (7<<7) /* Reference freq divider mask */
>>>> #define UFCR_RFDIV_REG(x) (((x)< 7 ? 6 - (x) : 6)<< 7)
>>>> #define UFCR_TXTL_SHF 10 /* Transmitter trigger level shift */
>>>> +#define UFCR_DCEDTE (1<<6)
>>>> #define USR1_PARITYERR (1<<15) /* Parity error interrupt flag */
>>>> #define USR1_RTSS (1<<14) /* RTS pin status */
>>>> #define USR1_TRDY (1<<13) /* Transmitter ready interrupt/dma flag */
>>>> @@ -200,12 +205,27 @@ struct imx_port {
>>>> unsigned int old_status;
>>>> int txirq,rxirq,rtsirq;
>>>> unsigned int have_rtscts:1;
>>>> + unsigned int enable_dte:1;
>>>> + unsigned int enable_dma:1;
>>>> unsigned int use_irda:1;
>>>> unsigned int irda_inv_rx:1;
>>>> unsigned int irda_inv_tx:1;
>>>> unsigned short trcv_delay; /* transceiver delay */
>>>> struct clk *clk;
>>>> struct imx_uart_data *devdata;
>>>> +
>>>> + /* DMA fields */
>>>> + unsigned int dma_req_rx;
>>>> + unsigned int dma_req_tx;
>>>> + struct imx_dma_data dma_data;
>>>> + struct dma_chan *dma_chan_rx, *dma_chan_tx;
>>>> + struct scatterlist rx_sgl, tx_sgl[2];
>>>> + void *rx_buf;
>>>> + unsigned int rx_bytes, tx_bytes;
>>>> + struct work_struct tsk_dma_rx, tsk_dma_tx;
>>> Why do you need a work struct to deal with DMA?
>>>
>> The uart uses the SDMA (drivers/dma/imx-sdma.c). And the SDMA may
>> schedule out in :
>> sdma_prep_slave_sg() --> sdma_load_contex() -->sdma_run_channel() .
> It should not. prep_slave_sg() must be callable from atomic contexts.
>
the Documentation/dmaengine.txt does not explicitly force all the dma
engine driver
to do so.
Add more comments for it in Documentation/dmaengine.txt?
>> This callback is called from IRQ context, with all the IRQ disabled. see:
>> sdma_int_handler() -->mxc_sdma_handle_channel() -->
>> mxc_sdma_handle_channel_normal()
>> --> .callback().
> That's a bug in your dmaengine driver.
>
OK, it's really a bug in the sdma driver.
thanks.
Best Regards
Huang Shijie
--
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: 8250.c: Generic support for RTS disabling once shiftreg and fifo is empty. (low latency RS485 etc).
From: Alan Cox @ 2012-04-27 9:02 UTC (permalink / raw)
To: Christian Melki; +Cc: linux-serial@vger.kernel.org
In-Reply-To: <1346E68D3A5B2043907CF4BF466383411A07E01021@ESESSCMS0353.eemea.ericsson.se>
On Fri, 27 Apr 2012 08:23:23 +0200
Christian Melki <christian.melki@ericsson.com> wrote:
> Hi.
>
> I have written a patch for 8250.c that introduces a new ioctl to tell the uart driver to shift disable RTS when uart has been completely emptied.
> This idea is to set the ioctl once the master is finished and wants to shift the bus.
> We needed this to be in the kernel since we have very little time to shift RTS according to the implemented protocol on top of RS485.
>
> Is there any interest in the community for such a behavior?
We have a set of RS485 ioctls that provide a standard interface for
assisting RS485 handling. The interface should probably use those if
possible.
> The patch is a little ugly right now. It busywaits a maximum amount of time for the shift register to become empty. I don't know how to do it without introducing a latency that is more than our max RTS time shift requirement (50us). I think it could be useful with a generic 8250.c support for this.
We've hit the same hardware limit in a couple of other places where you
end up having to spin on the last byte. I don't think it's a problem
providing it is only triggered when trying to do the RS485 bits with that
feature.
Alan
^ permalink raw reply
* Re: [PATCH 1/2] serial/imx: add DMA support
From: Richard Zhao @ 2012-04-27 8:38 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Huang Shijie, alan, B20596, B20223, gregkh, r58066, linux-serial,
shawn.guo, s.hauer, linux-arm-kernel
In-Reply-To: <20120427082245.GJ24211@n2100.arm.linux.org.uk>
On Fri, Apr 27, 2012 at 09:22:45AM +0100, Russell King - ARM Linux wrote:
> On Fri, Apr 27, 2012 at 03:00:39PM +0800, Huang Shijie wrote:
> > 于 2012年04月26日 19:11, Russell King - ARM Linux 写道:
> >> On Thu, Apr 26, 2012 at 06:37:11PM +0800, Huang Shijie wrote:
> >>> Add the DMA support for uart RX and TX.
> >>>
> >>> Signed-off-by: Huang Shijie<b32955@freescale.com>
> >>> ---
> >> Apart from the comments below,
> >>
> >> 1. How do you deal with transmitting the high-priority XON/XOFF
> >> characters (port->x_char) which occur with s/w flow control and
> >> the tty buffers fill up?
> >> 2. How do you deal with flow control in general? IOW, what happens
> >> when the remote end deasserts your CTS with h/w flow control enabled.
> >> How does your end deal with sending RTS according to flow control
> >> conditions?
> >>
> >> i
> > The UART uses the DMA for RX/TX with the hardware flow control (RTS/CTS)
> > enabled all the time.
>
> Your answer is too vague. Please try again.
>
> > If we use the software flow control(XON/XOFF), we should not enable the DMA.
> > I think i should add more comments about this issue.
> >
> > For example:
> > The MX6Q arm2 has two uarts, one with the DMA disabled is used for debug,
> > the other one with the DMA/RTS.CTS enabled can be used for the Bluetooth.
> >>> .../bindings/tty/serial/fsl-imx-uart.txt | 7 +
> >>> drivers/tty/serial/imx.c | 386 +++++++++++++++++++-
> >>> 2 files changed, 389 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt
> >>> index a9c0406..f27489d 100644
> >>> --- a/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt
> >>> +++ b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt
> >>> @@ -8,6 +8,10 @@ Required properties:
> >>> Optional properties:
> >>> - fsl,uart-has-rtscts : Indicate the uart has rts and cts
> >>> - fsl,irda-mode : Indicate the uart supports irda mode
> >>> +- fsl,enable-dma : Indicate the uart supports DMA
> >>> +- fsl,uart-dma-events : contains the DMA events for RX and TX,
> >>> + The first is the RX event, while the other is TX.
> >>> +- fsl,enable-dte: Indicate the uart works in DTE mode
> >>>
> >>> Example:
> >>>
> >>> @@ -16,4 +20,7 @@ uart@73fbc000 {
> >>> reg =<0x73fbc000 0x4000>;
> >>> interrupts =<31>;
> >>> fsl,uart-has-rtscts;
> >>> + fsl,enable-dma;
> >>> + fsl,uart-dma-events =<xx xx>;
> >>> + fsl,enable-dte;
> >>> };
> >>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> >>> index e7fecee..65ba24d 100644
> >>> --- a/drivers/tty/serial/imx.c
> >>> +++ b/drivers/tty/serial/imx.c
> >>> @@ -47,9 +47,11 @@
> >>> #include<linux/slab.h>
> >>> #include<linux/of.h>
> >>> #include<linux/of_device.h>
> >>> +#include<linux/dma-mapping.h>
> >>>
> >>> #include<asm/io.h>
> >>> #include<asm/irq.h>
> >>> +#include<mach/dma.h>
> >>> #include<mach/imx-uart.h>
> >>>
> >>> /* Register definitions */
> >>> @@ -82,6 +84,7 @@
> >>> #define UCR1_ADBR (1<<14) /* Auto detect baud rate */
> >>> #define UCR1_TRDYEN (1<<13) /* Transmitter ready interrupt enable */
> >>> #define UCR1_IDEN (1<<12) /* Idle condition interrupt */
> >>> +#define UCR1_ICD_REG(x) (((x)& 3)<< 10) /* idle condition detect */
> >>> #define UCR1_RRDYEN (1<<9) /* Recv ready interrupt enable */
> >>> #define UCR1_RDMAEN (1<<8) /* Recv ready DMA enable */
> >>> #define UCR1_IREN (1<<7) /* Infrared interface enable */
> >>> @@ -125,6 +128,7 @@
> >>> #define UCR4_ENIRI (1<<8) /* Serial infrared interrupt enable */
> >>> #define UCR4_WKEN (1<<7) /* Wake interrupt enable */
> >>> #define UCR4_REF16 (1<<6) /* Ref freq 16 MHz */
> >>> +#define UCR4_IDDMAEN (1<<6) /* DMA IDLE Condition Detected */
> >>> #define UCR4_IRSC (1<<5) /* IR special case */
> >>> #define UCR4_TCEN (1<<3) /* Transmit complete interrupt enable */
> >>> #define UCR4_BKEN (1<<2) /* Break condition interrupt enable */
> >>> @@ -134,6 +138,7 @@
> >>> #define UFCR_RFDIV (7<<7) /* Reference freq divider mask */
> >>> #define UFCR_RFDIV_REG(x) (((x)< 7 ? 6 - (x) : 6)<< 7)
> >>> #define UFCR_TXTL_SHF 10 /* Transmitter trigger level shift */
> >>> +#define UFCR_DCEDTE (1<<6)
> >>> #define USR1_PARITYERR (1<<15) /* Parity error interrupt flag */
> >>> #define USR1_RTSS (1<<14) /* RTS pin status */
> >>> #define USR1_TRDY (1<<13) /* Transmitter ready interrupt/dma flag */
> >>> @@ -200,12 +205,27 @@ struct imx_port {
> >>> unsigned int old_status;
> >>> int txirq,rxirq,rtsirq;
> >>> unsigned int have_rtscts:1;
> >>> + unsigned int enable_dte:1;
> >>> + unsigned int enable_dma:1;
> >>> unsigned int use_irda:1;
> >>> unsigned int irda_inv_rx:1;
> >>> unsigned int irda_inv_tx:1;
> >>> unsigned short trcv_delay; /* transceiver delay */
> >>> struct clk *clk;
> >>> struct imx_uart_data *devdata;
> >>> +
> >>> + /* DMA fields */
> >>> + unsigned int dma_req_rx;
> >>> + unsigned int dma_req_tx;
> >>> + struct imx_dma_data dma_data;
> >>> + struct dma_chan *dma_chan_rx, *dma_chan_tx;
> >>> + struct scatterlist rx_sgl, tx_sgl[2];
> >>> + void *rx_buf;
> >>> + unsigned int rx_bytes, tx_bytes;
> >>> + struct work_struct tsk_dma_rx, tsk_dma_tx;
> >> Why do you need a work struct to deal with DMA?
> >>
> > The uart uses the SDMA (drivers/dma/imx-sdma.c). And the SDMA may
> > schedule out in :
> > sdma_prep_slave_sg() --> sdma_load_contex() -->sdma_run_channel() .
>
> It should not. prep_slave_sg() must be callable from atomic contexts.
I just sent out a patch to fix it.
[PATCH 01/11] dma: imx-sdma: make channel0 operations atomic
Thanks
Richard
>
> > This callback is called from IRQ context, with all the IRQ disabled. see:
> > sdma_int_handler() -->mxc_sdma_handle_channel() -->
> > mxc_sdma_handle_channel_normal()
> > --> .callback().
>
> That's a bug in your dmaengine driver.
>
--
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 1/2] serial/imx: add DMA support
From: Russell King - ARM Linux @ 2012-04-27 8:22 UTC (permalink / raw)
To: Huang Shijie
Cc: alan, B20596, B20223, gregkh, r58066, linux-serial, shawn.guo,
s.hauer, linux-arm-kernel
In-Reply-To: <4F9A4417.7080107@freescale.com>
On Fri, Apr 27, 2012 at 03:00:39PM +0800, Huang Shijie wrote:
> 于 2012年04月26日 19:11, Russell King - ARM Linux 写道:
>> On Thu, Apr 26, 2012 at 06:37:11PM +0800, Huang Shijie wrote:
>>> Add the DMA support for uart RX and TX.
>>>
>>> Signed-off-by: Huang Shijie<b32955@freescale.com>
>>> ---
>> Apart from the comments below,
>>
>> 1. How do you deal with transmitting the high-priority XON/XOFF
>> characters (port->x_char) which occur with s/w flow control and
>> the tty buffers fill up?
>> 2. How do you deal with flow control in general? IOW, what happens
>> when the remote end deasserts your CTS with h/w flow control enabled.
>> How does your end deal with sending RTS according to flow control
>> conditions?
>>
>> i
> The UART uses the DMA for RX/TX with the hardware flow control (RTS/CTS)
> enabled all the time.
Your answer is too vague. Please try again.
> If we use the software flow control(XON/XOFF), we should not enable the DMA.
> I think i should add more comments about this issue.
>
> For example:
> The MX6Q arm2 has two uarts, one with the DMA disabled is used for debug,
> the other one with the DMA/RTS.CTS enabled can be used for the Bluetooth.
>>> .../bindings/tty/serial/fsl-imx-uart.txt | 7 +
>>> drivers/tty/serial/imx.c | 386 +++++++++++++++++++-
>>> 2 files changed, 389 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt
>>> index a9c0406..f27489d 100644
>>> --- a/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt
>>> +++ b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt
>>> @@ -8,6 +8,10 @@ Required properties:
>>> Optional properties:
>>> - fsl,uart-has-rtscts : Indicate the uart has rts and cts
>>> - fsl,irda-mode : Indicate the uart supports irda mode
>>> +- fsl,enable-dma : Indicate the uart supports DMA
>>> +- fsl,uart-dma-events : contains the DMA events for RX and TX,
>>> + The first is the RX event, while the other is TX.
>>> +- fsl,enable-dte: Indicate the uart works in DTE mode
>>>
>>> Example:
>>>
>>> @@ -16,4 +20,7 @@ uart@73fbc000 {
>>> reg =<0x73fbc000 0x4000>;
>>> interrupts =<31>;
>>> fsl,uart-has-rtscts;
>>> + fsl,enable-dma;
>>> + fsl,uart-dma-events =<xx xx>;
>>> + fsl,enable-dte;
>>> };
>>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
>>> index e7fecee..65ba24d 100644
>>> --- a/drivers/tty/serial/imx.c
>>> +++ b/drivers/tty/serial/imx.c
>>> @@ -47,9 +47,11 @@
>>> #include<linux/slab.h>
>>> #include<linux/of.h>
>>> #include<linux/of_device.h>
>>> +#include<linux/dma-mapping.h>
>>>
>>> #include<asm/io.h>
>>> #include<asm/irq.h>
>>> +#include<mach/dma.h>
>>> #include<mach/imx-uart.h>
>>>
>>> /* Register definitions */
>>> @@ -82,6 +84,7 @@
>>> #define UCR1_ADBR (1<<14) /* Auto detect baud rate */
>>> #define UCR1_TRDYEN (1<<13) /* Transmitter ready interrupt enable */
>>> #define UCR1_IDEN (1<<12) /* Idle condition interrupt */
>>> +#define UCR1_ICD_REG(x) (((x)& 3)<< 10) /* idle condition detect */
>>> #define UCR1_RRDYEN (1<<9) /* Recv ready interrupt enable */
>>> #define UCR1_RDMAEN (1<<8) /* Recv ready DMA enable */
>>> #define UCR1_IREN (1<<7) /* Infrared interface enable */
>>> @@ -125,6 +128,7 @@
>>> #define UCR4_ENIRI (1<<8) /* Serial infrared interrupt enable */
>>> #define UCR4_WKEN (1<<7) /* Wake interrupt enable */
>>> #define UCR4_REF16 (1<<6) /* Ref freq 16 MHz */
>>> +#define UCR4_IDDMAEN (1<<6) /* DMA IDLE Condition Detected */
>>> #define UCR4_IRSC (1<<5) /* IR special case */
>>> #define UCR4_TCEN (1<<3) /* Transmit complete interrupt enable */
>>> #define UCR4_BKEN (1<<2) /* Break condition interrupt enable */
>>> @@ -134,6 +138,7 @@
>>> #define UFCR_RFDIV (7<<7) /* Reference freq divider mask */
>>> #define UFCR_RFDIV_REG(x) (((x)< 7 ? 6 - (x) : 6)<< 7)
>>> #define UFCR_TXTL_SHF 10 /* Transmitter trigger level shift */
>>> +#define UFCR_DCEDTE (1<<6)
>>> #define USR1_PARITYERR (1<<15) /* Parity error interrupt flag */
>>> #define USR1_RTSS (1<<14) /* RTS pin status */
>>> #define USR1_TRDY (1<<13) /* Transmitter ready interrupt/dma flag */
>>> @@ -200,12 +205,27 @@ struct imx_port {
>>> unsigned int old_status;
>>> int txirq,rxirq,rtsirq;
>>> unsigned int have_rtscts:1;
>>> + unsigned int enable_dte:1;
>>> + unsigned int enable_dma:1;
>>> unsigned int use_irda:1;
>>> unsigned int irda_inv_rx:1;
>>> unsigned int irda_inv_tx:1;
>>> unsigned short trcv_delay; /* transceiver delay */
>>> struct clk *clk;
>>> struct imx_uart_data *devdata;
>>> +
>>> + /* DMA fields */
>>> + unsigned int dma_req_rx;
>>> + unsigned int dma_req_tx;
>>> + struct imx_dma_data dma_data;
>>> + struct dma_chan *dma_chan_rx, *dma_chan_tx;
>>> + struct scatterlist rx_sgl, tx_sgl[2];
>>> + void *rx_buf;
>>> + unsigned int rx_bytes, tx_bytes;
>>> + struct work_struct tsk_dma_rx, tsk_dma_tx;
>> Why do you need a work struct to deal with DMA?
>>
> The uart uses the SDMA (drivers/dma/imx-sdma.c). And the SDMA may
> schedule out in :
> sdma_prep_slave_sg() --> sdma_load_contex() -->sdma_run_channel() .
It should not. prep_slave_sg() must be callable from atomic contexts.
> This callback is called from IRQ context, with all the IRQ disabled. see:
> sdma_int_handler() -->mxc_sdma_handle_channel() -->
> mxc_sdma_handle_channel_normal()
> --> .callback().
That's a bug in your dmaengine driver.
--
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 1/2] serial/imx: add DMA support
From: Huang Shijie @ 2012-04-27 7:00 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: alan, B20596, B20223, gregkh, r58066, linux-serial, shawn.guo,
s.hauer, linux-arm-kernel
In-Reply-To: <20120426111116.GF24211@n2100.arm.linux.org.uk>
于 2012年04月26日 19:11, Russell King - ARM Linux 写道:
> On Thu, Apr 26, 2012 at 06:37:11PM +0800, Huang Shijie wrote:
>> Add the DMA support for uart RX and TX.
>>
>> Signed-off-by: Huang Shijie<b32955@freescale.com>
>> ---
> Apart from the comments below,
>
> 1. How do you deal with transmitting the high-priority XON/XOFF
> characters (port->x_char) which occur with s/w flow control and
> the tty buffers fill up?
> 2. How do you deal with flow control in general? IOW, what happens
> when the remote end deasserts your CTS with h/w flow control enabled.
> How does your end deal with sending RTS according to flow control
> conditions?
>
> i
The UART uses the DMA for RX/TX with the hardware flow control (RTS/CTS)
enabled all the time.
If we use the software flow control(XON/XOFF), we should not enable the DMA.
I think i should add more comments about this issue.
For example:
The MX6Q arm2 has two uarts, one with the DMA disabled is used for debug,
the other one with the DMA/RTS.CTS enabled can be used for the Bluetooth.
>> .../bindings/tty/serial/fsl-imx-uart.txt | 7 +
>> drivers/tty/serial/imx.c | 386 +++++++++++++++++++-
>> 2 files changed, 389 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt
>> index a9c0406..f27489d 100644
>> --- a/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt
>> +++ b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt
>> @@ -8,6 +8,10 @@ Required properties:
>> Optional properties:
>> - fsl,uart-has-rtscts : Indicate the uart has rts and cts
>> - fsl,irda-mode : Indicate the uart supports irda mode
>> +- fsl,enable-dma : Indicate the uart supports DMA
>> +- fsl,uart-dma-events : contains the DMA events for RX and TX,
>> + The first is the RX event, while the other is TX.
>> +- fsl,enable-dte: Indicate the uart works in DTE mode
>>
>> Example:
>>
>> @@ -16,4 +20,7 @@ uart@73fbc000 {
>> reg =<0x73fbc000 0x4000>;
>> interrupts =<31>;
>> fsl,uart-has-rtscts;
>> + fsl,enable-dma;
>> + fsl,uart-dma-events =<xx xx>;
>> + fsl,enable-dte;
>> };
>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
>> index e7fecee..65ba24d 100644
>> --- a/drivers/tty/serial/imx.c
>> +++ b/drivers/tty/serial/imx.c
>> @@ -47,9 +47,11 @@
>> #include<linux/slab.h>
>> #include<linux/of.h>
>> #include<linux/of_device.h>
>> +#include<linux/dma-mapping.h>
>>
>> #include<asm/io.h>
>> #include<asm/irq.h>
>> +#include<mach/dma.h>
>> #include<mach/imx-uart.h>
>>
>> /* Register definitions */
>> @@ -82,6 +84,7 @@
>> #define UCR1_ADBR (1<<14) /* Auto detect baud rate */
>> #define UCR1_TRDYEN (1<<13) /* Transmitter ready interrupt enable */
>> #define UCR1_IDEN (1<<12) /* Idle condition interrupt */
>> +#define UCR1_ICD_REG(x) (((x)& 3)<< 10) /* idle condition detect */
>> #define UCR1_RRDYEN (1<<9) /* Recv ready interrupt enable */
>> #define UCR1_RDMAEN (1<<8) /* Recv ready DMA enable */
>> #define UCR1_IREN (1<<7) /* Infrared interface enable */
>> @@ -125,6 +128,7 @@
>> #define UCR4_ENIRI (1<<8) /* Serial infrared interrupt enable */
>> #define UCR4_WKEN (1<<7) /* Wake interrupt enable */
>> #define UCR4_REF16 (1<<6) /* Ref freq 16 MHz */
>> +#define UCR4_IDDMAEN (1<<6) /* DMA IDLE Condition Detected */
>> #define UCR4_IRSC (1<<5) /* IR special case */
>> #define UCR4_TCEN (1<<3) /* Transmit complete interrupt enable */
>> #define UCR4_BKEN (1<<2) /* Break condition interrupt enable */
>> @@ -134,6 +138,7 @@
>> #define UFCR_RFDIV (7<<7) /* Reference freq divider mask */
>> #define UFCR_RFDIV_REG(x) (((x)< 7 ? 6 - (x) : 6)<< 7)
>> #define UFCR_TXTL_SHF 10 /* Transmitter trigger level shift */
>> +#define UFCR_DCEDTE (1<<6)
>> #define USR1_PARITYERR (1<<15) /* Parity error interrupt flag */
>> #define USR1_RTSS (1<<14) /* RTS pin status */
>> #define USR1_TRDY (1<<13) /* Transmitter ready interrupt/dma flag */
>> @@ -200,12 +205,27 @@ struct imx_port {
>> unsigned int old_status;
>> int txirq,rxirq,rtsirq;
>> unsigned int have_rtscts:1;
>> + unsigned int enable_dte:1;
>> + unsigned int enable_dma:1;
>> unsigned int use_irda:1;
>> unsigned int irda_inv_rx:1;
>> unsigned int irda_inv_tx:1;
>> unsigned short trcv_delay; /* transceiver delay */
>> struct clk *clk;
>> struct imx_uart_data *devdata;
>> +
>> + /* DMA fields */
>> + unsigned int dma_req_rx;
>> + unsigned int dma_req_tx;
>> + struct imx_dma_data dma_data;
>> + struct dma_chan *dma_chan_rx, *dma_chan_tx;
>> + struct scatterlist rx_sgl, tx_sgl[2];
>> + void *rx_buf;
>> + unsigned int rx_bytes, tx_bytes;
>> + struct work_struct tsk_dma_rx, tsk_dma_tx;
> Why do you need a work struct to deal with DMA?
>
The uart uses the SDMA (drivers/dma/imx-sdma.c). And the SDMA may
schedule out in :
sdma_prep_slave_sg() --> sdma_load_contex() -->sdma_run_channel() .
So i have to add a work struct to prepare and triger the DMA operations.
>> + unsigned int dma_tx_nents;
>> + bool dma_is_rxing;
>> + wait_queue_head_t dma_wait;
>> };
>>
>> struct imx_port_ucrs {
>> @@ -394,6 +414,13 @@ static void imx_stop_rx(struct uart_port *port)
>> struct imx_port *sport = (struct imx_port *)port;
>> unsigned long temp;
>>
>> + /*
>> + * We are maybe in the SMP now, so if the DMA RX thread is running,
>> + * we have to wait for it to finish.
>> + */
>> + if (sport->enable_dma&& sport->dma_is_rxing)
>> + return;
>> +
>> temp = readl(sport->port.membase + UCR2);
>> writel(temp&~ UCR2_RXEN, sport->port.membase + UCR2);
>> }
>> @@ -429,6 +456,80 @@ static inline void imx_transmit_buffer(struct imx_port *sport)
>> imx_stop_tx(&sport->port);
>> }
>>
>> +static void dma_tx_callback(void *data)
>> +{
>> + struct imx_port *sport = data;
>> + struct scatterlist *sgl =&sport->tx_sgl[0];
> struct scatterlist *sgl = sport->tx_sgl;
>
> is equivalent, and less typing.
>
>> + struct circ_buf *xmit =&sport->port.state->xmit;
>> +
>> + dma_unmap_sg(sport->port.dev, sgl, sport->dma_tx_nents, DMA_TO_DEVICE);
>> +
>> + /* update the stat */
>> + spin_lock(&sport->port.lock);
>> + xmit->tail = (xmit->tail + sport->tx_bytes)& (UART_XMIT_SIZE - 1);
>> + sport->port.icount.tx += sport->tx_bytes;
>> + spin_unlock(&sport->port.lock);
> Callbacks are called from tasklet context, and will have IRQs enabled.
> As the port lock is taken from IRQ context, this is waiting for a deadlock
> to happen. Have you run this with lockdep enabled?
>
This callback is called from IRQ context, with all the IRQ disabled. see:
sdma_int_handler() -->mxc_sdma_handle_channel() -->
mxc_sdma_handle_channel_normal()
--> .callback().
So spin_lock() is enough.
Best Regards
Huang Shijie
--
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
* 8250.c: Generic support for RTS disabling once shiftreg and fifo is empty. (low latency RS485 etc).
From: Christian Melki @ 2012-04-27 6:23 UTC (permalink / raw)
To: linux-serial@vger.kernel.org
Hi.
I have written a patch for 8250.c that introduces a new ioctl to tell the uart driver to shift disable RTS when uart has been completely emptied.
This idea is to set the ioctl once the master is finished and wants to shift the bus.
We needed this to be in the kernel since we have very little time to shift RTS according to the implemented protocol on top of RS485.
Is there any interest in the community for such a behavior?
The patch is a little ugly right now. It busywaits a maximum amount of time for the shift register to become empty. I don't know how to do it without introducing a latency that is more than our max RTS time shift requirement (50us). I think it could be useful with a generic 8250.c support for this.
Regards,
Christian Melki
^ permalink raw reply
* Re: [PATCH 1/2] serial/imx: add DMA support
From: Huang Shijie @ 2012-04-27 6:09 UTC (permalink / raw)
To: Sascha Hauer
Cc: alan, linux-serial, gregkh, shawn.guo, linux-arm-kernel, B20223,
r58066, B20596
In-Reply-To: <20120426120043.GH17184@pengutronix.de>
Hi,
>> +/* see the "i.MX61 SDMA Scripts User Manual.doc" for the parameters */
> I can't see how the manual helps here.
sorry, the comment is in the wrong place. it should placed at
configuring the `slave_config`.
> Please test this patch at least on one more SoC. There should be nothing
> i.MX6 specific in here, the fact that the i.MX6 is mentioned several
> times in the comments make me suspicious.
There are bugs in the UART DMA code in the firmware before the imx6q.
So we can not test the UART DMA except the imx6q arm2 board.
thanks
Huang Shijie
^ permalink raw reply
* How to detect serial buffer overruns
From: Corbin @ 2012-04-26 21:28 UTC (permalink / raw)
To: linux-serial
Hello,
Most of the documentation that I find to detect buffer overruns says
to use the TIOCGICOUNT ioctl along with serial_icounter_struct. It
appears, however, that the buf_overrun field in this struct is not
updated in the 8250 driver. I confirmed this by writing a lot of data
to my computer's built-in serial port at 115200 baud and then doing a
read, which resulted in only receiving 69376 bytes back. It appears
the tty flip buffers begin to overflow at 69376 bytes, but again, the
buf_overrun field in the serial_icounter_struct shows no change after
calling the TIOCGICOUNT ioctl. I am able to get other information from
the ioctl just fine, such as parity, break, and framing errors. I went
through several of the drivers in /drivers/tty/serial and
drivers/usb/serial and it looks like my problem stems from that fact
that there aren't really any serial drivers that make use of
buf_overrun, including the 8250 driver which my port is using.
So, my question is, what is the recommended way to detect buffer overruns?
Thanks,
Corbin Atkinson
--
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
* [patch 1/1] 8250_pci: fix pch uart matching
From: Arnaud Patard @ 2012-04-25 10:17 UTC (permalink / raw)
To: linux-serial; +Cc: Alan Cox, Greg Kroah-Hartman, stable
In-Reply-To: <20120425101723.820229746@hupstream.com>
[-- Attachment #1: pch_uart.patch --]
[-- Type: text/plain, Size: 2508 bytes --]
The rules used to make 8250_pci "ignore" the PCH uarts are lacking pci subids
entries, preventing it to match and thus is breaking serial port support for
theses systems.
This has been tested on a nanoETXexpress-TT, which has a specifici uart clock.
Tested-by: Erwan Velu <Erwan.Velu@zodiacaerospace.com>
[stable@: please apply to 3.0-stable, 3.2-stable and 3.3-stable]
Cc: stable@vger.kernel.org
Signed-off-by: Arnaud Patard <apatard@hupstream.com>
Index: arm-soc/drivers/tty/serial/8250/8250_pci.c
===================================================================
--- arm-soc.orig/drivers/tty/serial/8250/8250_pci.c 2012-04-19 20:40:44.336120603 +0200
+++ arm-soc/drivers/tty/serial/8250/8250_pci.c 2012-04-25 11:56:10.714697583 +0200
@@ -1609,54 +1609,72 @@ static struct pci_serial_quirk pci_seria
{
.vendor = PCI_VENDOR_ID_INTEL,
.device = 0x8811,
+ .subvendor = PCI_ANY_ID,
+ .subdevice = PCI_ANY_ID,
.init = pci_eg20t_init,
.setup = pci_default_setup,
},
{
.vendor = PCI_VENDOR_ID_INTEL,
.device = 0x8812,
+ .subvendor = PCI_ANY_ID,
+ .subdevice = PCI_ANY_ID,
.init = pci_eg20t_init,
.setup = pci_default_setup,
},
{
.vendor = PCI_VENDOR_ID_INTEL,
.device = 0x8813,
+ .subvendor = PCI_ANY_ID,
+ .subdevice = PCI_ANY_ID,
.init = pci_eg20t_init,
.setup = pci_default_setup,
},
{
.vendor = PCI_VENDOR_ID_INTEL,
.device = 0x8814,
+ .subvendor = PCI_ANY_ID,
+ .subdevice = PCI_ANY_ID,
.init = pci_eg20t_init,
.setup = pci_default_setup,
},
{
.vendor = 0x10DB,
.device = 0x8027,
+ .subvendor = PCI_ANY_ID,
+ .subdevice = PCI_ANY_ID,
.init = pci_eg20t_init,
.setup = pci_default_setup,
},
{
.vendor = 0x10DB,
.device = 0x8028,
+ .subvendor = PCI_ANY_ID,
+ .subdevice = PCI_ANY_ID,
.init = pci_eg20t_init,
.setup = pci_default_setup,
},
{
.vendor = 0x10DB,
.device = 0x8029,
+ .subvendor = PCI_ANY_ID,
+ .subdevice = PCI_ANY_ID,
.init = pci_eg20t_init,
.setup = pci_default_setup,
},
{
.vendor = 0x10DB,
.device = 0x800C,
+ .subvendor = PCI_ANY_ID,
+ .subdevice = PCI_ANY_ID,
.init = pci_eg20t_init,
.setup = pci_default_setup,
},
{
.vendor = 0x10DB,
.device = 0x800D,
+ .subvendor = PCI_ANY_ID,
+ .subdevice = PCI_ANY_ID,
.init = pci_eg20t_init,
.setup = pci_default_setup,
},
^ permalink raw reply
* Re: Deterministic behavior for TTY serial
From: Ivo Sieben @ 2012-04-26 14:27 UTC (permalink / raw)
To: Greg KH; +Cc: linux-serial, Alan Cox, RT
In-Reply-To: <20120419154609.GA9263@kroah.com>
Hi
>> 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..
>
> Do you have a pointer to the driver anywhere? Why isn't it submitted
> for inclusion in the main kernel tree?
This driver is work in progress and not yet mature enough for
inclusion in the main kernel tree.
> Have you done profiling to determine exactly what it taking "too long"
> for you? If so, what is the delay? If not, you should do this :)
>
I did some analyses using the ftrace 'function_graph' tracer to find
out what causes the TTY read to take longer than expected.
I use a test application, running on RT prioirty 99 that writes bursts
of 24 Bytes data to a my TTY device. A loop back connector is used, so
the application also reads back these 24 bytes. Non blocking reads &
writes are used.
This analyses is still ongoing...
But I found two issues that you might can help me to explain (but as
I said: I still need to "dive" further into the source)
1)
In some cases the tty_flush_to_ldisc() function (called by
drivers/tty/n_tty.c, line 1599) takes a rather long time than with
other TTY reads...
For trace see: http://pastebin.com/zXCYTLNj
If I understand it correctly, the TTY flip buffer uses a workqueue to
transfer receive read data from the TTY flip buffer into the buffer of
the line discipline. It seems like the N_TTY line discipline tries to
actively flush data from the TTY flip buffer into the line discipline
buffer. But my serial device drive initiates a tty_flip_buffer_push()
every time after a number of bytes were received (this is intiated
from threaded irq context by the way).
In the case of this trace: is the workqueue currently already busy
transferring data to the ldisc because of the UART receive interrupt
handling has queued that a new work item? So is the N_TTY read()
function actually waiting for that work item to be finished? I guess
in that case for non blocking reads you would like to return with an
EAGAIN, and try to read the data the next time you call the read
function... right?
2)
In some cases the tty_ldisc_deref() function (called by
drivers/tty/tty_io.c, line 977) takes a rather long time than with
other TTY reads
For trace see: http://pastebin.com/Nuh5cLGv
After a successful read from the N_TTY line discipline, the TTY
framework dereferences the line discipline.
Can the TTY read() block here because other processes are currently
try to get or release a reference to the N_TTY line discpline at the
same time?
So in that case: does the high priority TTY read has to wait for a
lower priority TTY read/write operation (e.g. from the terminal I/O).
Regards,
Ivo Sieben
--
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 1/2] serial/imx: add DMA support
From: Sascha Hauer @ 2012-04-26 12:00 UTC (permalink / raw)
To: Huang Shijie
Cc: alan, linux-serial, gregkh, shawn.guo, linux-arm-kernel, B20223,
r58066, B20596
In-Reply-To: <1335436632-29499-2-git-send-email-b32955@freescale.com>
On Thu, Apr 26, 2012 at 06:37:11PM +0800, Huang Shijie wrote:
> Add the DMA support for uart RX and TX.
>
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
> .../bindings/tty/serial/fsl-imx-uart.txt | 7 +
> drivers/tty/serial/imx.c | 386 +++++++++++++++++++-
> 2 files changed, 389 insertions(+), 4 deletions(-)
>
> + enum dma_status status;
> + unsigned long flags;
> + int ret;
> +
> + status = chan->device->device_tx_status(chan, (dma_cookie_t)NULL, NULL);
> + if (DMA_IN_PROGRESS == status)
> + return;
> +
> + spin_lock_irqsave(&sport->port.lock, flags);
> + sport->tx_bytes = uart_circ_chars_pending(xmit);
> + if (sport->tx_bytes > 0) {
Instead of putting nearly the whole body of this function inside a 'if'
you should return here.
> + if (xmit->tail > xmit->head) {
> + sport->dma_tx_nents = 2;
> + sg_init_table(sgl, 2);
> + sg_set_buf(sgl, xmit->buf + xmit->tail,
> + UART_XMIT_SIZE - xmit->tail);
> + sg_set_buf(&sgl[1], xmit->buf, xmit->head);
> + } else {
> + sport->dma_tx_nents = 1;
> + sg_init_one(sgl, xmit->buf + xmit->tail,
> + sport->tx_bytes);
> + }
> + spin_unlock_irqrestore(&sport->port.lock, flags);
> +
> + ret = dma_map_sg(sport->port.dev, sgl,
> + sport->dma_tx_nents, DMA_TO_DEVICE);
> + if (ret == 0) {
> + pr_err("DMA mapping error for TX.\n");
Use dev_* functions. Whoever reads the above in the logs won't have a
clue that it comes from the serial driver.
> + return;
> + }
> + desc = dmaengine_prep_slave_sg(chan, sgl,
> + sport->dma_tx_nents, DMA_MEM_TO_DEV, 0);
> + if (!desc) {
> + pr_err("We cannot prepare for the TX slave dma!\n");
> + return;
> + }
> + desc->callback = dma_tx_callback;
> + desc->callback_param = sport;
> +
> + /* fire it */
> + dmaengine_submit(desc);
> + dma_async_issue_pending(chan);
> + return;
> + }
> + spin_unlock_irqrestore(&sport->port.lock, flags);
> + return;
> +}
> +
> +/* see the "i.MX61 SDMA Scripts User Manual.doc" for the parameters */
I can't see how the manual helps here.
Please test this patch at least on one more SoC. There should be nothing
i.MX6 specific in here, the fact that the i.MX6 is mentioned several
times in the comments make me suspicious.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
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