Linux Serial subsystem development
 help / color / mirror / Atom feed
* Re: New serial card development
From: Matt Schulte @ 2012-10-15 19:08 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: linux-serial
In-Reply-To: <20121014093704.GA6207@thunk.org>

On Sun, Oct 14, 2012 at 4:37 AM, Theodore Ts'o <tytso@mit.edu> wrote:
>
> On Tue, Oct 09, 2012 at 01:43:10PM -0500, Matt Schulte wrote:
> > Hello, my name is Matt and I have recently developed a PCIe card based
> > on the Exar 17v35x series of PCIe multiport UART chips.
> >
> > We have written Linux drivers for our other products in the past but
> > they have never been what I would call the best practice for Linux
> > development.
> >
> > This time I would like to write a driver that uses the best practices
> > and possibly submit it to the kernel when I'm done.
> >
> > I am a little bit confused about what method would be best for this
> > device.   I have been examining the sample driver provided by Exar
> > and it seems that the only things that they have that are really
> > different from the generic 8250 serial driver are the interrupt
> > handler (optimized for their multiple ports), the transmit and
> > receive character functions (modified to use their 256 byte FIFOs)
> > and the function that calculates the baud.  Also I have two features
> > specific to my card that I would likely need to use an ioctl for.
>
> Hi Matt,
>
> It's been a while since I've done serial development (probably close
> to a decade ago), but way back when I was responsible for most of the
> code which is now in drivers/tty/serial/8250/serial.c, I had added
> support for the Exar 16C850 UART; since then someone else has added
> support for the Exar XR17D15x UART.
>
> The EXAR 16C850 had a 128 byte FIFO; it sounds like this EXAR variant
> is similar to the 16C850, although it's been further extended.
>
> > If these are the only differences would I be able to create a driver
> > like what is seen in the /drivers/tty/serial/8250 directory (8250_dw.c
> > for example)?  Or would I need to do something similar to what I find in
> > the /drviers/tty/serial directory?
>
> We support a very large number of 8250-like UART's already.  It
> doesn't make sense create a new driver which is essentially a clone of
> the 8250.c driver.  Instead, the 8250.c driver should be extended to
> support this new Exar UART variant.  The preferred way to do this is
> to detect the EXAR UART directly, via the method documented in the
> EXAR data sheet, instead of using the PCI id numbers to make this
> determination.  This way, we will be able to support that UART on
> other PCI serial cards, and not just yours.


That sounds reasonable but I am not sure what you mean my the "method
documented in the Exar data sheet."  I've been through this thing
quite a bit and I don't see anything about how one might detect the
ports without the PCI id numbers.  Could you explain what you mean by
this?
>
>
> If you have some features which are explicitly specific to your card,
> and which are not specific to the UART, then that may need to be
> separately triggered via the PCI vendor/device id numbers.  What
> feature is it, by the way?  If it has to do with controlling the clock
> crystal, there's infrastructure to deal with this already that you may
> be able to leverage.

* This new Exar UART does not use a crystal it uses a clock from the
PCI Express bus.  Also it has a fractional divisor that gets them
1/16th (0.0625) resolution on their divisors.  The normal DLL and DLM
registers contain the integer part of the divisor and the DLD register
provides the fractional component.  The divisor is able to be set to
anything between 1 and (2^16 - 0.0625) in increments of 0.0625.

* These UARTs have 256 byte FIFOs per port per direction.  I would
like to utilize these to their fullest potential.

* I need to toggle 16-bit general purpose IO signals to control two
things on my card: Receive Echo Cancel (useful for 2-wire 485) and the
selectable termination resistance on my receiver chips.

* I need to change two UART specific registers to change the sampling
mode (16x, 8x and 4x).

* I need to figure out if the new RS485 elements that have been added
to 8250 can be used on these Exar parts and if not I need something
that can enable RS485 for us.

* I need to be able to enable 9-bit mode, what they call Normal
Multi-drop as well as the Auto Address Detection Mode used in
conjunction with the Multi-drop mode.

* There is an interruptible hardware timer/counter that I might like
to utilize at some point.

With this laundry list of items that I would like to include, do you
still feel like this is something that should be included in the
kernel's 8250.c/8250_pci.c drivers?

Thanks for helping me think this through.

Matt

^ permalink raw reply

* [PATCH] serial/8250_hp300: Missing 8250 register interface conversion bits
From: Geert Uytterhoeven @ 2012-10-15 20:13 UTC (permalink / raw)
  To: Alan Cox, Greg Kroah-Hartman, Philip Blundell
  Cc: linux-serial, linux-kernel, Geert Uytterhoeven

commit 2655a2c76f80d91da34faa8f4e114d1793435ed3 ("8250: use the 8250
register interface not the legacy one") forgot to fully switch one
instance of struct uart_port to struct uart_8250_port, causing the
following compile failure:

drivers/tty/serial/8250/8250_hp300.c: In function ‘hpdca_init_one’:
drivers/tty/serial/8250/8250_hp300.c:174: error: ‘uart’ undeclared (first use in this function)
drivers/tty/serial/8250/8250_hp300.c:174: error: (Each undeclared identifier is reported only once
drivers/tty/serial/8250/8250_hp300.c:174: error: for each function it appears in.)

This went unnoticed in -next, as CONFIG_HPDCA is not set to y by
allmodconfig.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
Only compile-tested, due to lack of hardware

 drivers/tty/serial/8250/8250_hp300.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_hp300.c b/drivers/tty/serial/8250/8250_hp300.c
index 8f1dd2c..f3d0edf 100644
--- a/drivers/tty/serial/8250/8250_hp300.c
+++ b/drivers/tty/serial/8250/8250_hp300.c
@@ -162,7 +162,7 @@ int __init hp300_setup_serial_console(void)
 static int __devinit hpdca_init_one(struct dio_dev *d,
 				const struct dio_device_id *ent)
 {
-	struct uart_port port;
+	struct uart_8250_port uart;
 	int line;
 
 #ifdef CONFIG_SERIAL_8250_CONSOLE
@@ -174,19 +174,19 @@ static int __devinit hpdca_init_one(struct dio_dev *d,
 	memset(&uart, 0, sizeof(uart));
 
 	/* Memory mapped I/O */
-	port.iotype = UPIO_MEM;
-	port.flags = UPF_SKIP_TEST | UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF;
-	port.irq = d->ipl;
-	port.uartclk = HPDCA_BAUD_BASE * 16;
-	port.mapbase = (d->resource.start + UART_OFFSET);
-	port.membase = (char *)(port.mapbase + DIO_VIRADDRBASE);
-	port.regshift = 1;
-	port.dev = &d->dev;
+	uart.port.iotype = UPIO_MEM;
+	uart.port.flags = UPF_SKIP_TEST | UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF;
+	uart.port.irq = d->ipl;
+	uart.port.uartclk = HPDCA_BAUD_BASE * 16;
+	uart.port.mapbase = (d->resource.start + UART_OFFSET);
+	uart.port.membase = (char *)(uart.port.mapbase + DIO_VIRADDRBASE);
+	uart.port.regshift = 1;
+	uart.port.dev = &d->dev;
 	line = serial8250_register_8250_port(&uart);
 
 	if (line < 0) {
 		printk(KERN_NOTICE "8250_hp300: register_serial() DCA scode %d"
-		       " irq %d failed\n", d->scode, port.irq);
+		       " irq %d failed\n", d->scode, uart.port.irq);
 		return -ENOMEM;
 	}
 
-- 
1.7.0.4

^ permalink raw reply related

* Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
From: Kevin Hilman @ 2012-10-15 22:37 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Russell King - ARM Linux, Sourav, Paul Walmsley, Felipe Balbi,
	gregkh, linux-kernel, santosh.shilimkar, linux-serial, linux-omap,
	linux-arm-kernel, alan
In-Reply-To: <20121012215156.GL30339@atomide.com>

Tony Lindgren <tony@atomide.com> writes:

> * Kevin Hilman <khilman@deeprootsystems.com> [121012 13:34]:
>>
>> I'm not conviced (yet) that a mismatch is the root cause.  Yes, that's
>> what the author of $SUBJECT patch assumed and stated, but I'm not
>> pursuaded.  
>> 
>> If it's an improperly configured mux issue, then the UART will break
>> whenever the device is actually omap_device_enable'd, whether in the
>> driver or in the bus layer.
>
> I tried booting n800 here with CONFIG_OMAP_MUX disabled, and no
> change. Serial console output stops right when the console initializes.

OK, since it's not mux, and since those who actually maintain this
driver don't seem to be taking care of this, I did some digging today.

Russell is right.  It's a mismatch between assumed runtime PM state
(disabled) and actual HW state.

During init, all omap_devices are idled by default, so that they are
correctly in the state that the runtime PM framework will later expect
them to be.  That is, all devices *except* the console UART.  For that
one, we use the special hwmod flag to not idle/reset the UART since that
will cause problems during earlyprintk usage, and the switch between
the earlyprintk console and the real console driver.

Since the console uart was left enabled during init, it needs to be
fully enabled and the runtime PM status set accordingly.

The patch below does this, and works fine on 2420/n810, as well as on
3530/Overo, 3730/OveroSTORM, 3730/Beagle-XM and 4430/PandaES.

Will send patch with a proper changelog shortly,

Kevin

diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 0405c81..37b5dbe 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -327,6 +327,11 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
 	if ((console_uart_id == bdata->id) && no_console_suspend)
 		omap_device_disable_idle_on_suspend(pdev);
 
+	if (console_uart_id == bdata->id) {
+		omap_device_enable(pdev);
+		pm_runtime_set_active(&pdev->dev);
+	}
+
 	oh->mux = omap_hwmod_mux_init(bdata->pads, bdata->pads_cnt);
 
 	oh->dev_attr = uart;

^ permalink raw reply related

* Re: New serial card development
From: Alan Cox @ 2012-10-15 23:26 UTC (permalink / raw)
  To: Matt Schulte; +Cc: Theodore Ts'o, linux-serial
In-Reply-To: <CAJp1Oe6F+KiXoWdhb7JjbYmNBY4gxbtBzwNu0gWC7XzA_ZJxMg@mail.gmail.com>

> That sounds reasonable but I am not sure what you mean my the "method
> documented in the Exar data sheet."  I've been through this thing
> quite a bit and I don't see anything about how one might detect the
> ports without the PCI id numbers.  Could you explain what you mean by
> this?

I don't believe you can identify the new Exars except by PCI id - which
is fine as the type can be passed via the 8250_pci driver.

> * This new Exar UART does not use a crystal it uses a clock from the
> PCI Express bus.  Also it has a fractional divisor that gets them
> 1/16th (0.0625) resolution on their divisors.  The normal DLL and DLM
> registers contain the integer part of the divisor and the DLD register
> provides the fractional component.  The divisor is able to be set to
> anything between 1 and (2^16 - 0.0625) in increments of 0.0625.

That probably needs a bit of tweaking to getthe best results

> * These UARTs have 256 byte FIFOs per port per direction.  I would
> like to utilize these to their fullest potential.

We already handle other similar fifos so again in theory you just need to
set the size and it works.

> * I need to toggle 16-bit general purpose IO signals to control two
> things on my card: Receive Echo Cancel (useful for 2-wire 485) and the
> selectable termination resistance on my receiver chips.

Ok - we don't have a generalized way of doing that in the RS485 bits but
anything liek this should get added generically (in case other chips want
to do it)
 
> * I need to change two UART specific registers to change the sampling
> mode (16x, 8x and 4x).

What determines this.

> * I need to figure out if the new RS485 elements that have been added
> to 8250 can be used on these Exar parts and if not I need something
> that can enable RS485 for us.

They should provide the basics.

> * I need to be able to enable 9-bit mode, what they call Normal
> Multi-drop as well as the Auto Address Detection Mode used in
> conjunction with the Multi-drop mode.

That may actually be the most "fun" bit - the entire Linux tty layer is
devoutly 5-8bit, although people do some horrible hacks with  "8 plus
parity".

> With this laundry list of items that I would like to include, do you
> still feel like this is something that should be included in the
> kernel's 8250.c/8250_pci.c drivers?

I think so. It's also the path that lets you do basic enabling, get
it working upstream and then extend as is needed for the other features.

The one area 8250 really can't hack properly is DMA driven 8250 devices.

Alan

^ permalink raw reply

* Re: New serial card development
From: Theodore Ts'o @ 2012-10-16  2:32 UTC (permalink / raw)
  To: Alan Cox; +Cc: Matt Schulte, linux-serial
In-Reply-To: <20121016002608.64b33de5@pyramind.ukuu.org.uk>

On Tue, Oct 16, 2012 at 12:26:08AM +0100, Alan Cox wrote:
> > That sounds reasonable but I am not sure what you mean my the "method
> > documented in the Exar data sheet."  I've been through this thing
> > quite a bit and I don't see anything about how one might detect the
> > ports without the PCI id numbers.  Could you explain what you mean by
> > this?
> 
> I don't believe you can identify the new Exars except by PCI id - which
> is fine as the type can be passed via the 8250_pci driver.

Yeah, I just checked out the data sheet, and it looks like there isn't
a way to do this.  That's unfortunate, since board vendors can change
the PCI vendor and device ID.  So if they reprogram the EEPROM to use
their own id numbers, we won't be able to support that new board
without modifying the kernel source.  This is why I had tried to
implement ways where if the PCI class indicated a 16550 compatible
port, we would try to probe the UART registers to figure out exactly
what type of advanced features beyond those supported by the standard
16550A UART.  Sigh, it looks like we won't be able to do things that
way.

> > * I need to change two UART specific registers to change the sampling
> > mode (16x, 8x and 4x).
> 
> What determines this.

We have support for things like this already; see
serial8250_get_divisor and UPF_MAGIC_MULTIPLIER.  In general you only
want to change the sampling mode when you need it (i.e, for the high
speed baud rates).

> They should provide the basics.
> 
> > * I need to be able to enable 9-bit mode, what they call Normal
> > Multi-drop as well as the Auto Address Detection Mode used in
> > conjunction with the Multi-drop mode.
> 
> That may actually be the most "fun" bit - the entire Linux tty layer is
> devoutly 5-8bit, although people do some horrible hacks with  "8 plus
> parity".

That's what is actually recommended by the Exar application note;
since setting the 9th bit is only used for indicating that the lower 8
bits is the "address" bit for demultiplexing destination "drop", you
don't need to do it very often compared to the data bytes.  So
toggling the parity bit is the best way to deal with the fact you can
only use outb to send 8 bits.

						- Ted


^ permalink raw reply

* Re: [PATCH RESEND] serial/amba-pl011: use devm_* managed resources
From: Domenico Andreoli @ 2012-10-16  4:34 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-serial, Greg Kroah-Hartman, Anmar Oueja, Linus Walleij,
	linux-arm-kernel
In-Reply-To: <1350300961-7036-1-git-send-email-linus.walleij@stericsson.com>

On Mon, Oct 15, 2012 at 01:36:01PM +0200, Linus Walleij wrote:
> From: Linus Walleij <linus.walleij@linaro.org>
> 
> This switches a bunch of allocation and remapping to use the
> devm_* garbage collected methods and cleans up the error
> path and remove() paths consequently.
> 
> devm_ioremap() is only in <linux/io.h> so fix up the
> erroneous <asm/*> include as well.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/tty/serial/amba-pl011.c | 25 +++++++++----------------
>  1 file changed, 9 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index d7e1ede..7fca402 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -56,8 +56,7 @@
>  #include <linux/of_device.h>
>  #include <linux/pinctrl/consumer.h>
>  #include <linux/sizes.h>
> -
> -#include <asm/io.h>
> +#include <linux/io.h>
>  
>  #define UART_NR			14
>  
> @@ -1973,7 +1972,8 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
>  		goto out;
>  	}
>  
> -	uap = kzalloc(sizeof(struct uart_amba_port), GFP_KERNEL);
> +	uap = devm_kzalloc(&dev->dev, sizeof(struct uart_amba_port),
> +			   GFP_KERNEL);
>  	if (uap == NULL) {
>  		ret = -ENOMEM;
>  		goto out;
> @@ -1981,16 +1981,17 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
>  
>  	i = pl011_probe_dt_alias(i, &dev->dev);
>  
> -	base = ioremap(dev->res.start, resource_size(&dev->res));
> +	base = devm_ioremap(&dev->dev, dev->res.start,
> +			    resource_size(&dev->res));
>  	if (!base) {
>  		ret = -ENOMEM;
> -		goto free;
> +		goto out;
>  	}
>  
>  	uap->pinctrl = devm_pinctrl_get(&dev->dev);
>  	if (IS_ERR(uap->pinctrl)) {
>  		ret = PTR_ERR(uap->pinctrl);
> -		goto unmap;
> +		goto out;
>  	}
>  	uap->pins_default = pinctrl_lookup_state(uap->pinctrl,
>  						 PINCTRL_STATE_DEFAULT);
> @@ -2002,10 +2003,10 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
>  	if (IS_ERR(uap->pins_sleep))
>  		dev_dbg(&dev->dev, "could not get sleep pinstate\n");
>  
> -	uap->clk = clk_get(&dev->dev, NULL);
> +	uap->clk = devm_clk_get(&dev->dev, NULL);
>  	if (IS_ERR(uap->clk)) {
>  		ret = PTR_ERR(uap->clk);
> -		goto unmap;
> +		goto out;
>  	}
>  
>  	uap->vendor = vendor;
> @@ -2038,11 +2039,6 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
>  		amba_set_drvdata(dev, NULL);
>  		amba_ports[i] = NULL;
>  		pl011_dma_remove(uap);
> -		clk_put(uap->clk);
> - unmap:
> -		iounmap(base);
> - free:
> -		kfree(uap);
>  	}
>   out:
>  	return ret;
> @@ -2062,9 +2058,6 @@ static int pl011_remove(struct amba_device *dev)
>  			amba_ports[i] = NULL;
>  
>  	pl011_dma_remove(uap);
> -	iounmap(uap->port.membase);
> -	clk_put(uap->clk);
> -	kfree(uap);
>  	return 0;
>  }
>  

Tested-by: Domenico Andreoli <domenico.andreoli@linux.com>

Thanks,
Domenido

^ permalink raw reply

* [PATCH 0/3] serial: mxs-auart: add DMA support for auart in mx28
From: Huang Shijie @ 2012-10-16  6:03 UTC (permalink / raw)
  To: gregkh
  Cc: alan, linux-serial, linux-arm-kernel, shawn.guo, linux,
	lauri.hintsala, vinod.koul, Huang Shijie

This patch set adds the DMA support for auart in mx28.
patch 1:
	But in mx23, the DMA has a bug(see errata:2836). We can not add the
	DMA support in mx23, but we can add DMA support to auart in mx28.
	So in order to add the DMA support for the auart in mx28, we should add
	the platform_device_id to distinguish the distinguish SOCs.

patch 2: add the DMA support for mx28
	Only we meet the following conditions, we can enable the DMA support
	for auart:
        (1) We enable the DMA support in the dts file, such as
            arch/arm/boot/dts/imx28.dtsi.
        (2) We enable the hardware flow control.
        (3) We use the mx28, not the mx23. Due to hardware bug(see errata: 2836),
            we can not add the DMA support to mx23.

patch 3: enable the DMA support in dts for mx28 
	You can use the /ttyAPP0 to test this patch set. 
	I tested this patch in mx28-evk board.


Huang Shijie (3):
  serial: mxs-auart: distinguish the different SOCs
  serial: mxs-auart: add the DMA support for mx28
  ARM: dts: enable dma support for auart0 in mx28

 .../bindings/tty/serial/fsl-mxs-auart.txt          |    9 +-
 arch/arm/boot/dts/imx28.dtsi                       |   12 +-
 drivers/tty/serial/mxs-auart.c                     |  335 +++++++++++++++++++-
 3 files changed, 341 insertions(+), 15 deletions(-)



^ permalink raw reply

* [PATCH 1/3] serial: mxs-auart: distinguish the different SOCs
From: Huang Shijie @ 2012-10-16  6:03 UTC (permalink / raw)
  To: gregkh
  Cc: alan, linux-serial, linux-arm-kernel, shawn.guo, linux,
	lauri.hintsala, vinod.koul, Huang Shijie, Huang Shijie
In-Reply-To: <1350367386-7742-1-git-send-email-b32955@freescale.com>

From: Huang Shijie <shijie8@gmail.com>

The current mxs-auart driver is used for both mx23 and mx28.

But in mx23, the DMA has a bug(see errata:2836). We can not add the
DMA support in mx23, but we can add DMA support to auart in mx28.

So in order to add the DMA support for the auart in mx28, we should add
the platform_device_id to distinguish the distinguish SOCs.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 .../bindings/tty/serial/fsl-mxs-auart.txt          |    2 +-
 arch/arm/boot/dts/imx28.dtsi                       |   10 +++---
 drivers/tty/serial/mxs-auart.c                     |   28 +++++++++++++++----
 3 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt
index 2ee903f..a154bf1 100644
--- a/Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt
+++ b/Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt
@@ -8,7 +8,7 @@ Required properties:
 
 Example:
 auart0: serial@8006a000 {
-	compatible = "fsl,imx28-auart", "fsl,imx23-auart";
+	compatible = "fsl,imx28-auart";
 	reg = <0x8006a000 0x2000>;
 	interrupts = <112 70 71>;
 };
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index e16d631..6ed9215 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -795,7 +795,7 @@
 			};
 
 			auart0: serial@8006a000 {
-				compatible = "fsl,imx28-auart", "fsl,imx23-auart";
+				compatible = "fsl,imx28-auart";
 				reg = <0x8006a000 0x2000>;
 				interrupts = <112 70 71>;
 				clocks = <&clks 45>;
@@ -803,7 +803,7 @@
 			};
 
 			auart1: serial@8006c000 {
-				compatible = "fsl,imx28-auart", "fsl,imx23-auart";
+				compatible = "fsl,imx28-auart";
 				reg = <0x8006c000 0x2000>;
 				interrupts = <113 72 73>;
 				clocks = <&clks 45>;
@@ -811,7 +811,7 @@
 			};
 
 			auart2: serial@8006e000 {
-				compatible = "fsl,imx28-auart", "fsl,imx23-auart";
+				compatible = "fsl,imx28-auart";
 				reg = <0x8006e000 0x2000>;
 				interrupts = <114 74 75>;
 				clocks = <&clks 45>;
@@ -819,7 +819,7 @@
 			};
 
 			auart3: serial@80070000 {
-				compatible = "fsl,imx28-auart", "fsl,imx23-auart";
+				compatible = "fsl,imx28-auart";
 				reg = <0x80070000 0x2000>;
 				interrupts = <115 76 77>;
 				clocks = <&clks 45>;
@@ -827,7 +827,7 @@
 			};
 
 			auart4: serial@80072000 {
-				compatible = "fsl,imx28-auart", "fsl,imx23-auart";
+				compatible = "fsl,imx28-auart";
 				reg = <0x80072000 0x2000>;
 				interrupts = <116 78 79>;
 				clocks = <&clks 45>;
diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index 6db3baa..cd9ec1d 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -37,6 +37,11 @@
 
 #include <asm/cacheflush.h>
 
+/* Use the platform_id to distinguish different Archs. */
+#define IS_MX23			0x0
+#define IS_MX28			0x1
+#define AUART_IS_MX23(x)	((x)->pdev->id_entry->driver_data == IS_MX23)
+
 #define MXS_AUART_PORTS 5
 
 #define AUART_CTRL0			0x00000000
@@ -124,6 +129,7 @@ struct mxs_auart_port {
 
 	struct clk *clk;
 	struct device *dev;
+	struct platform_device *pdev;
 };
 
 static void mxs_auart_stop_tx(struct uart_port *u);
@@ -680,6 +686,19 @@ static struct uart_driver auart_driver = {
 #endif
 };
 
+static const struct platform_device_id auart_ids[] = {
+	{ .name = "imx23-auart", .driver_data = IS_MX23, },
+	{ .name = "imx28-auart", .driver_data = IS_MX28, },
+	{},
+};
+
+static struct of_device_id mxs_auart_dt_ids[] = {
+	{ .compatible = "fsl,imx23-auart", .data = (void *)&auart_ids[0] },
+	{ .compatible = "fsl,imx28-auart", .data = (void *)&auart_ids[1] },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mxs_auart_dt_ids);
+
 /*
  * This function returns 1 if pdev isn't a device instatiated by dt, 0 if it
  * could successfully get all information from dt or a negative errno.
@@ -701,6 +720,8 @@ static int serial_mxs_probe_dt(struct mxs_auart_port *s,
 	}
 	s->port.line = ret;
 
+	pdev->id_entry = of_match_device(mxs_auart_dt_ids, &pdev->dev)->data;
+
 	return 0;
 }
 
@@ -753,6 +774,7 @@ static int __devinit mxs_auart_probe(struct platform_device *pdev)
 
 	s->flags = 0;
 	s->ctrl = 0;
+	s->pdev = pdev;
 
 	s->irq = platform_get_irq(pdev, 0);
 	s->port.irq = s->irq;
@@ -805,12 +827,6 @@ static int __devexit mxs_auart_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static struct of_device_id mxs_auart_dt_ids[] = {
-	{ .compatible = "fsl,imx23-auart", },
-	{ /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, mxs_auart_dt_ids);
-
 static struct platform_driver mxs_auart_driver = {
 	.probe = mxs_auart_probe,
 	.remove = __devexit_p(mxs_auart_remove),
-- 
1.7.0.4



^ permalink raw reply related

* [PATCH 2/3] serial: mxs-auart: add the DMA support for mx28
From: Huang Shijie @ 2012-10-16  6:03 UTC (permalink / raw)
  To: gregkh
  Cc: alan, linux-serial, linux-arm-kernel, shawn.guo, linux,
	lauri.hintsala, vinod.koul, Huang Shijie
In-Reply-To: <1350367386-7742-1-git-send-email-b32955@freescale.com>

Only we meet the following conditions, we can enable the DMA support for
auart:

  (1) We enable the DMA support in the dts file, such as
      arch/arm/boot/dts/imx28.dtsi.

  (2) We enable the hardware flow control.

  (3) We use the mx28, not the mx23. Due to hardware bug(see errata: 2836),
      we can not add the DMA support to mx23.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 .../bindings/tty/serial/fsl-mxs-auart.txt          |    7 +
 drivers/tty/serial/mxs-auart.c                     |  307 +++++++++++++++++++-
 2 files changed, 311 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt
index a154bf1..67e54b4 100644
--- a/Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt
+++ b/Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt
@@ -6,11 +6,18 @@ Required properties:
 - reg : Address and length of the register set for the device
 - interrupts : Should contain the auart interrupt numbers
 
+Optional properties:
+- fsl,auart-dma-channel : The DMA channels, the first is for RX, the other
+			is for TX.
+- fsl,auart-enable-dma : Enable the DMA support for the auart.
+
 Example:
 auart0: serial@8006a000 {
 	compatible = "fsl,imx28-auart";
 	reg = <0x8006a000 0x2000>;
 	interrupts = <112 70 71>;
+	fsl,auart-dma-channel = <8 9>;
+	fsl,auart-enable-dma;
 };
 
 Note: Each auart port should have an alias correctly numbered in "aliases"
diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index cd9ec1d..2271330 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -34,6 +34,8 @@
 #include <linux/io.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/of_device.h>
+#include <linux/dma-mapping.h>
+#include <linux/fsl/mxs-dma.h>
 
 #include <asm/cacheflush.h>
 
@@ -76,7 +78,15 @@
 
 #define AUART_CTRL0_SFTRST			(1 << 31)
 #define AUART_CTRL0_CLKGATE			(1 << 30)
+#define AUART_CTRL0_RXTO_ENABLE			(1 << 27)
+#define AUART_CTRL0_RXTIMEOUT(v)		(((v) & 0x7ff) << 16)
+#define AUART_CTRL0_XFER_COUNT(v)		((v) & 0xffff)
 
+#define AUART_CTRL1_XFER_COUNT(v)		((v) & 0xffff)
+
+#define AUART_CTRL2_DMAONERR			(1 << 26)
+#define AUART_CTRL2_TXDMAE			(1 << 25)
+#define AUART_CTRL2_RXDMAE			(1 << 24)
 #define AUART_CTRL2_CTSEN			(1 << 15)
 #define AUART_CTRL2_RTSEN			(1 << 14)
 #define AUART_CTRL2_RTS				(1 << 11)
@@ -116,12 +126,15 @@
 #define AUART_STAT_BERR				(1 << 18)
 #define AUART_STAT_PERR				(1 << 17)
 #define AUART_STAT_FERR				(1 << 16)
+#define AUART_STAT_RXCOUNT_MASK			0xffff
 
 static struct uart_driver auart_driver;
 
 struct mxs_auart_port {
 	struct uart_port port;
 
+#define MXS_AUART_DMA_CONFIG	0x1
+#define MXS_AUART_DMA_ENABLED	0x2
 	unsigned int flags;
 	unsigned int ctrl;
 
@@ -130,16 +143,116 @@ struct mxs_auart_port {
 	struct clk *clk;
 	struct device *dev;
 	struct platform_device *pdev;
+
+	/* for DMA */
+	struct mxs_dma_data dma_data;
+	int dma_channel_rx, dma_channel_tx;
+	int dma_irq_rx, dma_irq_tx;
+	int dma_channel;
+
+	struct scatterlist tx_sgl;
+	struct dma_chan	*tx_dma_chan;
+	void *tx_dma_buf;
+
+	struct scatterlist rx_sgl;
+	struct dma_chan	*rx_dma_chan;
+	void *rx_dma_buf;
 };
 
+static inline bool auart_dma_enabled(struct mxs_auart_port *s)
+{
+	return s->flags & MXS_AUART_DMA_ENABLED;
+}
+
 static void mxs_auart_stop_tx(struct uart_port *u);
 
 #define to_auart_port(u) container_of(u, struct mxs_auart_port, port)
 
+static inline void mxs_auart_tx_chars(struct mxs_auart_port *s);
+
+static void dma_tx_callback(void *param)
+{
+	struct mxs_auart_port *s = param;
+	struct circ_buf *xmit = &s->port.state->xmit;
+
+	dma_unmap_sg(s->dev, &s->tx_sgl, 1, DMA_TO_DEVICE);
+
+	/* wake up the possible processes. */
+	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+		uart_write_wakeup(&s->port);
+
+	mxs_auart_tx_chars(s);
+}
+
+static int mxs_auart_dma_tx(struct mxs_auart_port *s, int size)
+{
+	struct dma_async_tx_descriptor *desc;
+	struct scatterlist *sgl = &s->tx_sgl;
+	struct dma_chan *channel = s->tx_dma_chan;
+	u32 pio[1];
+
+	/* [1] : send PIO. Note, the first pio word is CTRL1. */
+	pio[0] = AUART_CTRL1_XFER_COUNT(size);
+	desc = dmaengine_prep_slave_sg(channel, (struct scatterlist *)pio,
+					1, DMA_TRANS_NONE, 0);
+	if (!desc) {
+		dev_err(s->dev, "step 1 error\n");
+		return -EINVAL;
+	}
+
+	/* [2] : set DMA buffer. */
+	sg_init_one(sgl, s->tx_dma_buf, size);
+	dma_map_sg(s->dev, sgl, 1, DMA_TO_DEVICE);
+	desc = dmaengine_prep_slave_sg(channel, sgl,
+			1, DMA_MEM_TO_DEV, DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+	if (!desc) {
+		dev_err(s->dev, "step 2 error\n");
+		return -EINVAL;
+	}
+
+	/* [3] : submit the DMA */
+	desc->callback = dma_tx_callback;
+	desc->callback_param = s;
+	dmaengine_submit(desc);
+	dma_async_issue_pending(channel);
+	return 0;
+}
+
 static inline void mxs_auart_tx_chars(struct mxs_auart_port *s)
 {
 	struct circ_buf *xmit = &s->port.state->xmit;
 
+	if (auart_dma_enabled(s)) {
+		int i = 0;
+		int size;
+		void *buffer = s->tx_dma_buf;
+		enum dma_status status;
+
+		/* Check whether there is pending DMA operations. */
+		status = dmaengine_tx_status(s->tx_dma_chan, 0, NULL);
+		if (status != DMA_SUCCESS)
+			return;
+
+		while (!uart_circ_empty(xmit) && !uart_tx_stopped(&s->port)) {
+			size = min_t(u32, UART_XMIT_SIZE - i,
+				     CIRC_CNT_TO_END(xmit->head,
+						     xmit->tail,
+						     UART_XMIT_SIZE));
+			memcpy(buffer + i, xmit->buf + xmit->tail, size);
+			xmit->tail = (xmit->tail + size) & (UART_XMIT_SIZE - 1);
+
+			i += size;
+			if (i >= UART_XMIT_SIZE)
+				break;
+		}
+
+		if (uart_tx_stopped(&s->port))
+			mxs_auart_stop_tx(&s->port);
+		if (i)
+			mxs_auart_dma_tx(s, i);
+		return;
+	}
+
 	while (!(readl(s->port.membase + AUART_STAT) &
 		 AUART_STAT_TXFF)) {
 		if (s->port.x_char) {
@@ -293,10 +406,155 @@ static u32 mxs_auart_get_mctrl(struct uart_port *u)
 	return mctrl;
 }
 
+static bool mxs_auart_dma_filter(struct dma_chan *chan, void *param)
+{
+	struct mxs_auart_port *s = param;
+
+	if (!mxs_dma_is_apbx(chan))
+		return false;
+
+	if (s->dma_channel == chan->chan_id) {
+		chan->private = &s->dma_data;
+		return true;
+	}
+	return false;
+}
+
+static int mxs_auart_dma_prep_rx(struct mxs_auart_port *s);
+static void dma_rx_callback(void *arg)
+{
+	struct mxs_auart_port *s = (struct mxs_auart_port *) arg;
+	struct tty_struct *tty = s->port.state->port.tty;
+	int count;
+	u32 stat;
+
+	stat = readl(s->port.membase + AUART_STAT);
+	stat &= ~(AUART_STAT_OERR | AUART_STAT_BERR |
+			AUART_STAT_PERR | AUART_STAT_FERR);
+
+	count = stat & AUART_STAT_RXCOUNT_MASK;
+	tty_insert_flip_string(tty, s->rx_dma_buf, count);
+
+	writel(stat, s->port.membase + AUART_STAT);
+	tty_flip_buffer_push(tty);
+
+	/* start the next DMA for RX. */
+	mxs_auart_dma_prep_rx(s);
+}
+
+static int mxs_auart_dma_prep_rx(struct mxs_auart_port *s)
+{
+	struct dma_async_tx_descriptor *desc;
+	struct scatterlist *sgl = &s->rx_sgl;
+	struct dma_chan *channel = s->rx_dma_chan;
+	u32 pio[1];
+
+	/* [1] : send PIO */
+	pio[0] = AUART_CTRL0_RXTO_ENABLE
+		| AUART_CTRL0_RXTIMEOUT(0x80)
+		| AUART_CTRL0_XFER_COUNT(UART_XMIT_SIZE);
+	desc = dmaengine_prep_slave_sg(channel, (struct scatterlist *)pio,
+					1, DMA_TRANS_NONE, 0);
+	if (!desc) {
+		dev_err(s->dev, "step 1 error\n");
+		return -EINVAL;
+	}
+
+	/* [2] : send DMA request */
+	sg_init_one(sgl, s->rx_dma_buf, UART_XMIT_SIZE);
+	dma_map_sg(s->dev, sgl, 1, DMA_FROM_DEVICE);
+	desc = dmaengine_prep_slave_sg(channel, sgl, 1, DMA_DEV_TO_MEM,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+	if (!desc) {
+		dev_err(s->dev, "step 2 error\n");
+		return -1;
+	}
+
+	/* [3] : submit the DMA, but do not issue it. */
+	desc->callback = dma_rx_callback;
+	desc->callback_param = s;
+	dmaengine_submit(desc);
+	dma_async_issue_pending(channel);
+	return 0;
+}
+
+static void mxs_auart_dma_exit_channel(struct mxs_auart_port *s)
+{
+	if (s->tx_dma_chan) {
+		dma_release_channel(s->tx_dma_chan);
+		s->tx_dma_chan = NULL;
+	}
+	if (s->rx_dma_chan) {
+		dma_release_channel(s->rx_dma_chan);
+		s->rx_dma_chan = NULL;
+	}
+
+	kfree(s->tx_dma_buf);
+	kfree(s->rx_dma_buf);
+	s->tx_dma_buf = NULL;
+	s->rx_dma_buf = NULL;
+}
+
+static void mxs_auart_dma_exit(struct mxs_auart_port *s)
+{
+
+	writel(AUART_CTRL2_TXDMAE | AUART_CTRL2_RXDMAE | AUART_CTRL2_DMAONERR,
+		s->port.membase + AUART_CTRL2_CLR);
+
+	mxs_auart_dma_exit_channel(s);
+	s->flags &= ~MXS_AUART_DMA_ENABLED;
+}
+
+static int mxs_auart_dma_init(struct mxs_auart_port *s)
+{
+	dma_cap_mask_t mask;
+
+	if (auart_dma_enabled(s))
+		return 0;
+
+	/* We do not get the right DMA channels. */
+	if (s->dma_channel_rx == -1 || s->dma_channel_rx == -1)
+		return -EINVAL;
+
+	/* init for RX */
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+	s->dma_channel = s->dma_channel_rx;
+	s->dma_data.chan_irq = s->dma_irq_rx;
+	s->rx_dma_chan = dma_request_channel(mask, mxs_auart_dma_filter, s);
+	if (!s->rx_dma_chan)
+		goto err_out;
+	s->rx_dma_buf = kzalloc(UART_XMIT_SIZE, GFP_KERNEL | GFP_DMA);
+	if (!s->rx_dma_buf)
+		goto err_out;
+
+	/* init for TX */
+	s->dma_channel = s->dma_channel_tx;
+	s->dma_data.chan_irq = s->dma_irq_tx;
+	s->tx_dma_chan = dma_request_channel(mask, mxs_auart_dma_filter, s);
+	if (!s->tx_dma_chan)
+		goto err_out;
+	s->tx_dma_buf = kzalloc(UART_XMIT_SIZE, GFP_KERNEL | GFP_DMA);
+	if (!s->tx_dma_buf)
+		goto err_out;
+
+	/* set the flags */
+	s->flags |= MXS_AUART_DMA_ENABLED;
+	dev_dbg(s->dev, "enabled the DMA support.");
+
+	return 0;
+
+err_out:
+	mxs_auart_dma_exit_channel(s);
+	return -EINVAL;
+
+}
+
 static void mxs_auart_settermios(struct uart_port *u,
 				 struct ktermios *termios,
 				 struct ktermios *old)
 {
+	struct mxs_auart_port *s = to_auart_port(u);
 	u32 bm, ctrl, ctrl2, div;
 	unsigned int cflag, baud;
 
@@ -368,10 +626,23 @@ static void mxs_auart_settermios(struct uart_port *u,
 		ctrl |= AUART_LINECTRL_STP2;
 
 	/* figure out the hardware flow control settings */
-	if (cflag & CRTSCTS)
+	if (cflag & CRTSCTS) {
+		/*
+		 * The DMA has a bug(see errata:2836) in mx23.
+		 * So we can not implement the DMA for auart in mx23,
+		 * we can only implement the DMA support for auart
+		 * in mx28.
+		 */
+		if (!AUART_IS_MX23(s) && (s->flags & MXS_AUART_DMA_CONFIG)) {
+			if (!mxs_auart_dma_init(s))
+				/* enable DMA tranfer */
+				ctrl2 |= AUART_CTRL2_TXDMAE | AUART_CTRL2_RXDMAE
+				       | AUART_CTRL2_DMAONERR;
+		}
 		ctrl2 |= AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN;
-	else
+	} else {
 		ctrl2 &= ~(AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN);
+	}
 
 	/* set baud rate */
 	baud = uart_get_baud_rate(u, termios, old, 0, u->uartclk);
@@ -383,6 +654,17 @@ static void mxs_auart_settermios(struct uart_port *u,
 	writel(ctrl2, u->membase + AUART_CTRL2);
 
 	uart_update_timeout(u, termios->c_cflag, baud);
+
+	/* prepare for the DMA RX. */
+	if (auart_dma_enabled(s)) {
+		if (!mxs_auart_dma_prep_rx(s)) {
+			/* Disable the normal RX interrupt. */
+			writel(AUART_INTR_RXIEN, u->membase + AUART_INTR_CLR);
+		} else {
+			mxs_auart_dma_exit(s);
+			dev_err(s->dev, "We can not start up the DMA.\n");
+		}
+	}
 }
 
 static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
@@ -461,6 +743,9 @@ static void mxs_auart_shutdown(struct uart_port *u)
 {
 	struct mxs_auart_port *s = to_auart_port(u);
 
+	if (auart_dma_enabled(s))
+		mxs_auart_dma_exit(s);
+
 	writel(AUART_CTRL2_UARTEN, u->membase + AUART_CTRL2_CLR);
 
 	writel(AUART_INTR_RXIEN | AUART_INTR_RTIEN | AUART_INTR_CTSMIEN,
@@ -707,6 +992,7 @@ static int serial_mxs_probe_dt(struct mxs_auart_port *s,
 		struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
+	u32 dma_channel[2];
 	int ret;
 
 	if (!np)
@@ -722,6 +1008,22 @@ static int serial_mxs_probe_dt(struct mxs_auart_port *s,
 
 	pdev->id_entry = of_match_device(mxs_auart_dt_ids, &pdev->dev)->data;
 
+	ret = of_property_read_u32_array(np, "fsl,auart-dma-channel",
+					dma_channel, 2);
+	if (ret == 0) {
+		s->dma_channel_rx = dma_channel[0];
+		s->dma_channel_tx = dma_channel[1];
+	} else {
+		s->dma_channel_rx = -1;
+		s->dma_channel_tx = -1;
+	}
+
+	s->dma_irq_rx = platform_get_irq(pdev, 1);
+	s->dma_irq_tx = platform_get_irq(pdev, 2);
+
+	if (of_property_read_bool(np, "fsl,auart-enable-dma"))
+		s->flags |= MXS_AUART_DMA_CONFIG;
+
 	return 0;
 }
 
@@ -772,7 +1074,6 @@ static int __devinit mxs_auart_probe(struct platform_device *pdev)
 	s->port.type = PORT_IMX;
 	s->port.dev = s->dev = get_device(&pdev->dev);
 
-	s->flags = 0;
 	s->ctrl = 0;
 	s->pdev = pdev;
 
-- 
1.7.0.4



^ permalink raw reply related

* [PATCH 3/3] ARM: dts: enable dma support for auart0 in mx28
From: Huang Shijie @ 2012-10-16  6:03 UTC (permalink / raw)
  To: gregkh
  Cc: alan, linux-serial, linux-arm-kernel, shawn.guo, linux,
	lauri.hintsala, vinod.koul, Huang Shijie
In-Reply-To: <1350367386-7742-1-git-send-email-b32955@freescale.com>

enable the dma support for auart0 in mx28.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 arch/arm/boot/dts/imx28.dtsi |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index 6ed9215..738e023 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -798,6 +798,8 @@
 				compatible = "fsl,imx28-auart";
 				reg = <0x8006a000 0x2000>;
 				interrupts = <112 70 71>;
+				fsl,auart-dma-channel = <8 9>;
+				fsl,auart-enable-dma;
 				clocks = <&clks 45>;
 				status = "disabled";
 			};
-- 
1.7.0.4



^ permalink raw reply related

* Re: [PATCH-v2] tty: Use raw spin lock to protect RX flip buffer
From: Ivo Sieben @ 2012-10-16  6:55 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Steven Rostedt, linux-serial, linux-rt-users, Alan Cox, Greg KH
In-Reply-To: <alpine.LFD.2.02.1210091934010.10988@ionos>

Hi,

2012/10/9 Thomas Gleixner <tglx@linutronix.de>:
>
> __tty_buffer_flush() can call tty_buffer_free() which in turn can call
> kfree(), which is a nono on RT with preemption and interrupts
> disabled. Enabling CONFIG_DEBUG_ATOMIC_SLEEP and extensive testing
> should have told you that.
>
> I did not look at the other places where this lock is used, but I
> suspect there is more fallout lurking.
>
> Thanks,
>
>         tglx

OK, this is a big lesson for me to start using the Kernel Hacking
options more in the future, and especially before sending patches to
the mailing list... Sorry for bothering you with a patch that I could
find out myself was incorrect.

After your remarks, I had some more discussion & reviews with a
colleague of mine. And we finally came to the point where we figured
out that this patch actually tries to solve an issue that is not there
at all... All RX flip buffer handling is initiated from the threaded
IRQ handling: filling the buffer, pushing the buffer and in case of
the low_latency handling (always enabled for PREEMP_RT) also the
copying of the buffer to the line discipline. Therefore the spin lock
was only locked/unlocked by the RX irq thread during "normal" read
operation.

So I skrewed up: I thought I had seen measurements that this patch
improved performance, but after reverting this changeset and redoing
the measurements it proves it has no performance impact at all.

I'll abandon this patch
Thank you very much for reviewing

Regards,
Ivo Sieben

^ permalink raw reply

* Re: [RESEND PATCH 1/2] of serial port driver - add clk_get_rate() support
From: Murali Karicheri @ 2012-10-16 15:32 UTC (permalink / raw)
  To: Murali Karicheri; +Cc: alan, gregkh, linux-serial, linux-kernel, linux-keystone
In-Reply-To: <1350401051-8254-1-git-send-email-m-karicheri2@ti.com>

On 10/16/2012 11:24 AM, Murali Karicheri wrote:
> Currently this driver expects the clock-frequency attribute. This
> patch allows getting clock-frequency through clk driver API
> clk_get_rate() if clock-frequency attribute is not defined.
>
> So in the device bindings for serial device, one can add clocks
> phandle to refer to the clk device to get the rate.
>
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>
> diff --git a/drivers/tty/serial/of_serial.c b/drivers/tty/serial/of_serial.c
> index 34e7187..6f64f02 100644
> --- a/drivers/tty/serial/of_serial.c
> +++ b/drivers/tty/serial/of_serial.c
> @@ -21,8 +21,10 @@
>   #include <linux/of_serial.h>
>   #include <linux/of_platform.h>
>   #include <linux/nwpserial.h>
> +#include <linux/clk.h>
>   
>   struct of_serial_info {
> +	struct clk *clk;
>   	int type;
>   	int line;
>   };
> @@ -51,7 +53,8 @@ EXPORT_SYMBOL_GPL(tegra_serial_handle_break);
>    * Fill a struct uart_port for a given device node
>    */
>   static int __devinit of_platform_serial_setup(struct platform_device *ofdev,
> -					int type, struct uart_port *port)
> +			int type, struct uart_port *port,
> +			struct of_serial_info *info)
>   {
>   	struct resource resource;
>   	struct device_node *np = ofdev->dev.of_node;
> @@ -60,8 +63,17 @@ static int __devinit of_platform_serial_setup(struct platform_device *ofdev,
>   
>   	memset(port, 0, sizeof *port);
>   	if (of_property_read_u32(np, "clock-frequency", &clk)) {
> -		dev_warn(&ofdev->dev, "no clock-frequency property set\n");
> -		return -ENODEV;
> +
> +		/* Get clk rate through clk driver if present */
> +		info->clk = clk_get(&ofdev->dev, NULL);
> +		if (info->clk == NULL) {
> +			dev_warn(&ofdev->dev,
> +				"clk or clock-frequency not defined\n");
> +			return -ENODEV;
> +		}
> +
> +		clk_prepare_enable(info->clk);
> +		clk = clk_get_rate(info->clk);
>   	}
>   	/* If current-speed was set, then try not to change it. */
>   	if (of_property_read_u32(np, "current-speed", &spd) == 0)
> @@ -70,7 +82,7 @@ static int __devinit of_platform_serial_setup(struct platform_device *ofdev,
>   	ret = of_address_to_resource(np, 0, &resource);
>   	if (ret) {
>   		dev_warn(&ofdev->dev, "invalid address\n");
> -		return ret;
> +		goto out;
>   	}
>   
>   	spin_lock_init(&port->lock);
> @@ -97,7 +109,8 @@ static int __devinit of_platform_serial_setup(struct platform_device *ofdev,
>   		default:
>   			dev_warn(&ofdev->dev, "unsupported reg-io-width (%d)\n",
>   				 prop);
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto out;
>   		}
>   	}
>   
> @@ -111,6 +124,10 @@ static int __devinit of_platform_serial_setup(struct platform_device *ofdev,
>   		port->handle_break = tegra_serial_handle_break;
>   
>   	return 0;
> +out:
> +	if (info->clk)
> +		clk_disable_unprepare(info->clk);
> +	return ret;
>   }
>   
>   /*
> @@ -137,7 +154,7 @@ static int __devinit of_platform_serial_probe(struct platform_device *ofdev)
>   		return -ENOMEM;
>   
>   	port_type = (unsigned long)match->data;
> -	ret = of_platform_serial_setup(ofdev, port_type, &port);
> +	ret = of_platform_serial_setup(ofdev, port_type, &port, info);
>   	if (ret)
>   		goto out;
>   
> @@ -193,6 +210,9 @@ static int of_platform_serial_remove(struct platform_device *ofdev)
>   		/* need to add code for these */
>   		break;
>   	}
> +
> +	if (info->clk)
> +		clk_disable_unprepare(info->clk);
>   	kfree(info);
>   	return 0;
>   }
For some reason, I can't see the two patches in this series at the 
linux-serial server at http://marc.info/?l=linux-serial

Can someone tell me what could be wrong? My email seems to reach.. So trying
to reply and get this onto the mailing list.

Murali

^ permalink raw reply

* Re: [RESEND PATCH 2/2] Documentation: of-serial.txt - update for clocks phandle for clk
From: Murali Karicheri @ 2012-10-16 15:33 UTC (permalink / raw)
  To: Murali Karicheri; +Cc: alan, gregkh, linux-serial, linux-kernel, linux-keystone
In-Reply-To: <1350401051-8254-2-git-send-email-m-karicheri2@ti.com>

On 10/16/2012 11:24 AM, Murali Karicheri wrote:
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>
> diff --git a/Documentation/devicetree/bindings/tty/serial/of-serial.txt b/Documentation/devicetree/bindings/tty/serial/of-serial.txt
> index 0847fde..423b7ff 100644
> --- a/Documentation/devicetree/bindings/tty/serial/of-serial.txt
> +++ b/Documentation/devicetree/bindings/tty/serial/of-serial.txt
> @@ -14,7 +14,10 @@ Required properties:
>   	- "serial" if the port type is unknown.
>   - reg : offset and length of the register set for the device.
>   - interrupts : should contain uart interrupt.
> -- clock-frequency : the input clock frequency for the UART.
> +- clock-frequency : the input clock frequency for the UART
> +	 or
> +  clocks phandle to refer to the clk used as per Documentation/devicetree
> +  /bindings/clock/clock-bindings.txt
>   
>   Optional properties:
>   - current-speed : the current active speed of the UART.


^ permalink raw reply

* RE: 8250_early for big-endian
From: Noam Camus @ 2012-10-17  9:07 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-serial@vger.kernel.org
In-Reply-To: <20121014133216.686277e1@pyramind.ukuu.org.uk>

________________________________________
From: Alan Cox [alan@lxorguk.ukuu.org.uk]
Sent: Sunday, October 14, 2012 2:32 PM

>For 8250_early it may well be the right thing is to support similar
serial_in/serial_out methods.

Hello Alan,
Here is my patch:

>From e819fbbf9690b72620c54a62fa34545c7b035f91 Mon Sep 17 00:00:00 2001
From: Noam Camus <noamc@ezchip.com>
Date: Wed, 3 Oct 2012 09:40:36 +0200
Subject: [PATCH] tty/8250_early: Make serial_in/serial_out be over-ridden

Now one can replace default methods with similar fit to its needs.
e.g. choose other regshift, register offset...

Signed-off-by: Noam Camus <noamc@ezchip.com>
CC: Alan Cox <alan@lxorguk.ukuu.org.uk>
---
 drivers/tty/serial/8250/8250_early.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index eaafb98..ac31b0c 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -48,7 +48,7 @@ struct early_serial8250_device {
 
 static struct early_serial8250_device early_device;
 
-static unsigned int __init serial_in(struct uart_port *port, int offset)
+unsigned int __weak __init serial_in(struct uart_port *port, int offset)
 {
 	switch (port->iotype) {
 	case UPIO_MEM:
@@ -62,7 +62,7 @@ static unsigned int __init serial_in(struct uart_port *port, int offset)
 	}
 }
 
-static void __init serial_out(struct uart_port *port, int offset, int value)
+void __weak __init serial_out(struct uart_port *port, int offset, int value)
 {
 	switch (port->iotype) {
 	case UPIO_MEM:
-- 
1.7.1


Noam

^ permalink raw reply related

* [REPOST] tty: Use raw spin lock to protect TTY ldisc administration
From: Ivo Sieben @ 2012-10-17 12:03 UTC (permalink / raw)
  To: linux-serial, RT, Thomas Gleixner, Steven Rostedt
  Cc: Alan Cox, Greg KH, Ivo Sieben
In-Reply-To: <1348149928-4681-1-git-send-email-meltedpianoman@gmail.com>

The global "normal" spin lock that guards the line discipline
administration is replaced by a raw spin lock. On a PREEMPT_RT system this
prevents unwanted scheduling overhead around the line discipline administration.

On a 200 MHz AT91SAM9261 processor setup this fixes about 100us of scheduling
overhead on a TTY read or write call.

Signed-off-by: Ivo Sieben <meltedpianoman@gmail.com>
---

 Repost:

 @Thomas Gleixner & Steven Rostedt:
 I think this patch makes sense... a global spin lock causes additional
 scheduling overhead. Since the critical section is small a raw spinlock
 can be used here.

 I tested this on a 3.0.43-rt65 kernel, with the following Kernel Hacking
 options enabled:
 - CONFIG_DEBUG_RT_MUTEXES
 - CONFIG_DEBUG_SPINLOCK
 - CONFIG_DEBUG_MUTEXES
 - CONFIG_DEBUG_LOCK_ALLOC
 - CONFIG_PROVE_LOCKING
 - CONFIG_DEBUG_SPINLOCK_SLEEP

 drivers/tty/tty_ldisc.c |   32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 0f2a2c5..4eb24b1 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -26,7 +26,7 @@
  *	callers who will do ldisc lookups and cannot sleep.
  */
 
-static DEFINE_SPINLOCK(tty_ldisc_lock);
+static DEFINE_RAW_SPINLOCK(tty_ldisc_lock);
 static DECLARE_WAIT_QUEUE_HEAD(tty_ldisc_wait);
 /* Line disc dispatch table */
 static struct tty_ldisc_ops *tty_ldiscs[NR_LDISCS];
@@ -49,21 +49,21 @@ static void put_ldisc(struct tty_ldisc *ld)
 	 * If this is the last user, free the ldisc, and
 	 * release the ldisc ops.
 	 *
-	 * We really want an "atomic_dec_and_lock_irqsave()",
+	 * We really want an "atomic_dec_and_raw_lock_irqsave()",
 	 * but we don't have it, so this does it by hand.
 	 */
-	local_irq_save(flags);
-	if (atomic_dec_and_lock(&ld->users, &tty_ldisc_lock)) {
+	raw_spin_lock_irqsave(&tty_ldisc_lock, flags);
+	if (atomic_dec_and_test(&ld->users)) {
 		struct tty_ldisc_ops *ldo = ld->ops;
 
 		ldo->refcount--;
 		module_put(ldo->owner);
-		spin_unlock_irqrestore(&tty_ldisc_lock, flags);
+		raw_spin_unlock_irqrestore(&tty_ldisc_lock, flags);
 
 		kfree(ld);
 		return;
 	}
-	local_irq_restore(flags);
+	raw_spin_unlock_irqrestore(&tty_ldisc_lock, flags);
 	wake_up(&ld->wq_idle);
 }
 
@@ -88,11 +88,11 @@ int tty_register_ldisc(int disc, struct tty_ldisc_ops *new_ldisc)
 	if (disc < N_TTY || disc >= NR_LDISCS)
 		return -EINVAL;
 
-	spin_lock_irqsave(&tty_ldisc_lock, flags);
+	raw_spin_lock_irqsave(&tty_ldisc_lock, flags);
 	tty_ldiscs[disc] = new_ldisc;
 	new_ldisc->num = disc;
 	new_ldisc->refcount = 0;
-	spin_unlock_irqrestore(&tty_ldisc_lock, flags);
+	raw_spin_unlock_irqrestore(&tty_ldisc_lock, flags);
 
 	return ret;
 }
@@ -118,12 +118,12 @@ int tty_unregister_ldisc(int disc)
 	if (disc < N_TTY || disc >= NR_LDISCS)
 		return -EINVAL;
 
-	spin_lock_irqsave(&tty_ldisc_lock, flags);
+	raw_spin_lock_irqsave(&tty_ldisc_lock, flags);
 	if (tty_ldiscs[disc]->refcount)
 		ret = -EBUSY;
 	else
 		tty_ldiscs[disc] = NULL;
-	spin_unlock_irqrestore(&tty_ldisc_lock, flags);
+	raw_spin_unlock_irqrestore(&tty_ldisc_lock, flags);
 
 	return ret;
 }
@@ -134,7 +134,7 @@ static struct tty_ldisc_ops *get_ldops(int disc)
 	unsigned long flags;
 	struct tty_ldisc_ops *ldops, *ret;
 
-	spin_lock_irqsave(&tty_ldisc_lock, flags);
+	raw_spin_lock_irqsave(&tty_ldisc_lock, flags);
 	ret = ERR_PTR(-EINVAL);
 	ldops = tty_ldiscs[disc];
 	if (ldops) {
@@ -144,7 +144,7 @@ static struct tty_ldisc_ops *get_ldops(int disc)
 			ret = ldops;
 		}
 	}
-	spin_unlock_irqrestore(&tty_ldisc_lock, flags);
+	raw_spin_unlock_irqrestore(&tty_ldisc_lock, flags);
 	return ret;
 }
 
@@ -152,10 +152,10 @@ static void put_ldops(struct tty_ldisc_ops *ldops)
 {
 	unsigned long flags;
 
-	spin_lock_irqsave(&tty_ldisc_lock, flags);
+	raw_spin_lock_irqsave(&tty_ldisc_lock, flags);
 	ldops->refcount--;
 	module_put(ldops->owner);
-	spin_unlock_irqrestore(&tty_ldisc_lock, flags);
+	raw_spin_unlock_irqrestore(&tty_ldisc_lock, flags);
 }
 
 /**
@@ -287,11 +287,11 @@ static struct tty_ldisc *tty_ldisc_try(struct tty_struct *tty)
 	unsigned long flags;
 	struct tty_ldisc *ld;
 
-	spin_lock_irqsave(&tty_ldisc_lock, flags);
+	raw_spin_lock_irqsave(&tty_ldisc_lock, flags);
 	ld = NULL;
 	if (test_bit(TTY_LDISC, &tty->flags))
 		ld = get_ldisc(tty->ldisc);
-	spin_unlock_irqrestore(&tty_ldisc_lock, flags);
+	raw_spin_unlock_irqrestore(&tty_ldisc_lock, flags);
 	return ld;
 }
 
-- 
1.7.9.5



^ permalink raw reply related

* Re: New serial card development
From: Matt Schulte @ 2012-10-17 20:23 UTC (permalink / raw)
  To: linux-serial, Theodore Ts'o, Alan Cox
In-Reply-To: <CAJp1Oe6k7NWqdbYkJnd787JiT55-wSbG+tX1tP7Cy-oPShdVaA@mail.gmail.com>

Forgot to reply to all.
---------- Forwarded message ----------
From: Matt Schulte <matts@commtech-fastcom.com>
Date: Wed, Oct 17, 2012 at 2:10 PM
Subject: Re: New serial card development
To: Alan Cox <alan@lxorguk.ukuu.org.uk>


On Mon, Oct 15, 2012 at 6:26 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> That sounds reasonable but I am not sure what you mean my the "method
>> documented in the Exar data sheet."  I've been through this thing
>> quite a bit and I don't see anything about how one might detect the
>> ports without the PCI id numbers.  Could you explain what you mean by
>> this?
>
> I don't believe you can identify the new Exars except by PCI id - which
> is fine as the type can be passed via the 8250_pci driver.
>
>> * This new Exar UART does not use a crystal it uses a clock from the
>> PCI Express bus.  Also it has a fractional divisor that gets them
>> 1/16th (0.0625) resolution on their divisors.  The normal DLL and DLM
>> registers contain the integer part of the divisor and the DLD register
>> provides the fractional component.  The divisor is able to be set to
>> anything between 1 and (2^16 - 0.0625) in increments of 0.0625.
>
> That probably needs a bit of tweaking to getthe best results

Does this make you lean one way or the other in how I would best
implement this driver?

>> * These UARTs have 256 byte FIFOs per port per direction.  I would
>> like to utilize these to their fullest potential.
>
> We already handle other similar fifos so again in theory you just need to
> set the size and it works.

Good, I'm sure it will be fine ;)

>> * I need to toggle 16-bit general purpose IO signals to control two
>> things on my card: Receive Echo Cancel (useful for 2-wire 485) and the
>> selectable termination resistance on my receiver chips.
>
> Ok - we don't have a generalized way of doing that in the RS485 bits but
> anything liek this should get added generically (in case other chips want
> to do it)

I have two features, one that could be considered part of RS485 and
one that has nothing at all to do with RS485.

>> * I need to change two UART specific registers to change the sampling
>> mode (16x, 8x and 4x).
>
> What determines this.

User would have to decide to turn on non-standard 8x or 4x sampling mode.

>> * I need to figure out if the new RS485 elements that have been added
>> to 8250 can be used on these Exar parts and if not I need something
>> that can enable RS485 for us.
>
> They should provide the basics.

Where will I take the built in 485 flags and turn them into whatever
my board needs?

>> * I need to be able to enable 9-bit mode, what they call Normal
>> Multi-drop as well as the Auto Address Detection Mode used in
>> conjunction with the Multi-drop mode.
>
> That may actually be the most "fun" bit - the entire Linux tty layer is
> devoutly 5-8bit, although people do some horrible hacks with  "8 plus
> parity".

Transmitting is a little bit interesting but manageable.  In the
receive direction the chip can detect the 9th bit and recognize it as
an address bit and then check that byte against a preset station
address that has been specified by the user.

>> With this laundry list of items that I would like to include, do you
>> still feel like this is something that should be included in the
>> kernel's 8250.c/8250_pci.c drivers?
>
> I think so. It's also the path that lets you do basic enabling, get
> it working upstream and then extend as is needed for the other features.

So is there an example of a device already in the kernel that you
think I should try to emulate? Will I be best served with something
like 8250_dw.c or do you think I should try to get all of my things
directly into 8250 and 8250_pci?

Matt Schulte

^ permalink raw reply

* Re: New serial card development
From: Matt Schulte @ 2012-10-17 20:24 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: Alan Cox, linux-serial
In-Reply-To: <20121016023226.GA17446@thunk.org>

On Mon, Oct 15, 2012 at 9:32 PM, Theodore Ts'o <tytso@mit.edu> wrote:
> On Tue, Oct 16, 2012 at 12:26:08AM +0100, Alan Cox wrote:
>> > That sounds reasonable but I am not sure what you mean my the "method
>> > documented in the Exar data sheet."  I've been through this thing
>> > quite a bit and I don't see anything about how one might detect the
>> > ports without the PCI id numbers.  Could you explain what you mean by
>> > this?
>>
>> I don't believe you can identify the new Exars except by PCI id - which
>> is fine as the type can be passed via the 8250_pci driver.
>
> Yeah, I just checked out the data sheet, and it looks like there isn't
> a way to do this.  That's unfortunate, since board vendors can change
> the PCI vendor and device ID.  So if they reprogram the EEPROM to use
> their own id numbers, we won't be able to support that new board
> without modifying the kernel source.  This is why I had tried to
> implement ways where if the PCI class indicated a 16550 compatible
> port, we would try to probe the UART registers to figure out exactly
> what type of advanced features beyond those supported by the standard
> 16550A UART.  Sigh, it looks like we won't be able to do things that
> way.

Which is exactly what I do to the device id :)

>> > * I need to change two UART specific registers to change the sampling
>> > mode (16x, 8x and 4x).
>>
>> What determines this.
>
> We have support for things like this already; see
> serial8250_get_divisor and UPF_MAGIC_MULTIPLIER.  In general you only
> want to change the sampling mode when you need it (i.e, for the high
> speed baud rates).

I'm not sure the UPF_MAGIC_MULTIPLIER is the same as the sampling rate
that I'm looking for, though I might be able to come up with a way to
hijack the flag to do what I want.

>> They should provide the basics.
>>
>> > * I need to be able to enable 9-bit mode, what they call Normal
>> > Multi-drop as well as the Auto Address Detection Mode used in
>> > conjunction with the Multi-drop mode.
>>
>> That may actually be the most "fun" bit - the entire Linux tty layer is
>> devoutly 5-8bit, although people do some horrible hacks with  "8 plus
>> parity".
>
> That's what is actually recommended by the Exar application note;
> since setting the 9th bit is only used for indicating that the lower 8
> bits is the "address" bit for demultiplexing destination "drop", you
> don't need to do it very often compared to the data bytes.  So
> toggling the parity bit is the best way to deal with the fact you can
> only use outb to send 8 bits.

What I would need to have happen is for a flag to be set that says
"enable 9-bit mode transmit" or something.  When set, the first byte
of data of a given write will go out with mark parity, then the rest
of the bytes will have space parity.  The one byte with mark is the
address byte.

In the receive direction I would need to optionally enable a station
address which the chip will use to verify all incoming data and will
reject anything that is not of the appropriate address.

Matt Schulte

^ permalink raw reply

* Re: New serial card development
From: Alan Cox @ 2012-10-17 21:53 UTC (permalink / raw)
  To: Matt Schulte; +Cc: linux-serial, Theodore Ts'o
In-Reply-To: <CAJp1Oe7zyDh9w7Nc-xtbPQrbg7FEBr73QmYh_er3UX53KcGL8A@mail.gmail.com>

> > That probably needs a bit of tweaking to getthe best results
> 
> Does this make you lean one way or the other in how I would best
> implement this driver?

I would always start from what we have.

I wonder actually if there is a bit of a disconnect here from the line
of questions. I know in a lot of environments the model is

	concept design
	internal approval/verification
	consult with other stakeholders
	commit to model	
	build
	release

but in the Linux space the model is

	don't break anyone elses stuff
	get it working
	figure out as you go what is needed
	if it doesn't work out adjust
	polish as it becomes clear what can be done better
	repeat

So even if you submit initial patches to make it work with 8250.c and it
turns out that actually this was the wrong thing everyone will be happy
with the patches that then relocate it.

In addition what tends to occur is that the rules change anyway. You get
half way through submitting support for 256 byte fifos and someone comes
along with another different device that has the feature and so on.

So I would start from 8250.c and get trivial stuff like the fifo changes,
the PCI identifiers and so forth in as a small set of patches and then
work long the rest of it.

Even 'don't break other people's stuff' isn't a hard rule. The real rule
is 'if you break other people's stuff fix it back up' - because it
happens, and it's unavoidable.

Alan

^ permalink raw reply

* Re: [PATCH 1/3] serial: mxs-auart: distinguish the different SOCs
From: Shawn Guo @ 2012-10-18  2:44 UTC (permalink / raw)
  To: Huang Shijie
  Cc: gregkh, alan, linux-serial, linux-arm-kernel, linux,
	lauri.hintsala, vinod.koul, Huang Shijie
In-Reply-To: <1350367386-7742-2-git-send-email-b32955@freescale.com>

On Tue, Oct 16, 2012 at 02:03:04PM +0800, Huang Shijie wrote:
> From: Huang Shijie <shijie8@gmail.com>
> 
> The current mxs-auart driver is used for both mx23 and mx28.
> 
> But in mx23, the DMA has a bug(see errata:2836). We can not add the
> DMA support in mx23, but we can add DMA support to auart in mx28.
> 
> So in order to add the DMA support for the auart in mx28, we should add
> the platform_device_id to distinguish the distinguish SOCs.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  .../bindings/tty/serial/fsl-mxs-auart.txt          |    2 +-
>  arch/arm/boot/dts/imx28.dtsi                       |   10 +++---
>  drivers/tty/serial/mxs-auart.c                     |   28 +++++++++++++++----
>  3 files changed, 28 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt
> index 2ee903f..a154bf1 100644
> --- a/Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt
> +++ b/Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt
> @@ -8,7 +8,7 @@ Required properties:
>  
>  Example:
>  auart0: serial@8006a000 {
> -	compatible = "fsl,imx28-auart", "fsl,imx23-auart";
> +	compatible = "fsl,imx28-auart";
>  	reg = <0x8006a000 0x2000>;
>  	interrupts = <112 70 71>;
>  };
> diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> index e16d631..6ed9215 100644
> --- a/arch/arm/boot/dts/imx28.dtsi
> +++ b/arch/arm/boot/dts/imx28.dtsi
> @@ -795,7 +795,7 @@
>  			};
>  
>  			auart0: serial@8006a000 {
> -				compatible = "fsl,imx28-auart", "fsl,imx23-auart";
> +				compatible = "fsl,imx28-auart";
>  				reg = <0x8006a000 0x2000>;
>  				interrupts = <112 70 71>;
>  				clocks = <&clks 45>;
> @@ -803,7 +803,7 @@
>  			};
>  
>  			auart1: serial@8006c000 {
> -				compatible = "fsl,imx28-auart", "fsl,imx23-auart";
> +				compatible = "fsl,imx28-auart";
>  				reg = <0x8006c000 0x2000>;
>  				interrupts = <113 72 73>;
>  				clocks = <&clks 45>;
> @@ -811,7 +811,7 @@
>  			};
>  
>  			auart2: serial@8006e000 {
> -				compatible = "fsl,imx28-auart", "fsl,imx23-auart";
> +				compatible = "fsl,imx28-auart";
>  				reg = <0x8006e000 0x2000>;
>  				interrupts = <114 74 75>;
>  				clocks = <&clks 45>;
> @@ -819,7 +819,7 @@
>  			};
>  
>  			auart3: serial@80070000 {
> -				compatible = "fsl,imx28-auart", "fsl,imx23-auart";
> +				compatible = "fsl,imx28-auart";
>  				reg = <0x80070000 0x2000>;
>  				interrupts = <115 76 77>;
>  				clocks = <&clks 45>;
> @@ -827,7 +827,7 @@
>  			};
>  
>  			auart4: serial@80072000 {
> -				compatible = "fsl,imx28-auart", "fsl,imx23-auart";
> +				compatible = "fsl,imx28-auart";
>  				reg = <0x80072000 0x2000>;
>  				interrupts = <116 78 79>;
>  				clocks = <&clks 45>;

All changes above are unnecessary.  With "fsl,imx28-auart" added to
driver's compatible, driver will match it first.

> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index 6db3baa..cd9ec1d 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c
> @@ -37,6 +37,11 @@
>  
>  #include <asm/cacheflush.h>
>  
> +/* Use the platform_id to distinguish different Archs. */
> +#define IS_MX23			0x0
> +#define IS_MX28			0x1

I do not like the name.  We are distinguishing the IP not SoC, but just
happen to use SoC to identify the version.

You can look at drivers/mmc/host/sdhci-esdhc-imx.c for example.

> +#define AUART_IS_MX23(x)	((x)->pdev->id_entry->driver_data == IS_MX23)
> +

Use inline function.

>  #define MXS_AUART_PORTS 5
>  
>  #define AUART_CTRL0			0x00000000
> @@ -124,6 +129,7 @@ struct mxs_auart_port {
>  
>  	struct clk *clk;
>  	struct device *dev;
> +	struct platform_device *pdev;
>  };
>  
>  static void mxs_auart_stop_tx(struct uart_port *u);
> @@ -680,6 +686,19 @@ static struct uart_driver auart_driver = {
>  #endif
>  };
>  
> +static const struct platform_device_id auart_ids[] = {
> +	{ .name = "imx23-auart", .driver_data = IS_MX23, },
> +	{ .name = "imx28-auart", .driver_data = IS_MX28, },
> +	{},
> +};
> +

The driver is only used on mach-mxs.  Since mach-mxs becomes a DT only
platform, auart_ids is not really needed.  Look, you do not use it in
mxs_auart_driver for probing at all.

> +static struct of_device_id mxs_auart_dt_ids[] = {
> +	{ .compatible = "fsl,imx23-auart", .data = (void *)&auart_ids[0] },
> +	{ .compatible = "fsl,imx28-auart", .data = (void *)&auart_ids[1] },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mxs_auart_dt_ids);
> +
>  /*
>   * This function returns 1 if pdev isn't a device instatiated by dt, 0 if it
>   * could successfully get all information from dt or a negative errno.
> @@ -701,6 +720,8 @@ static int serial_mxs_probe_dt(struct mxs_auart_port *s,
>  	}
>  	s->port.line = ret;
>  
> +	pdev->id_entry = of_match_device(mxs_auart_dt_ids, &pdev->dev)->data;
> +
>  	return 0;
>  }
>  
> @@ -753,6 +774,7 @@ static int __devinit mxs_auart_probe(struct platform_device *pdev)
>  
>  	s->flags = 0;
>  	s->ctrl = 0;
> +	s->pdev = pdev;

What is this for?

Shawn

>  
>  	s->irq = platform_get_irq(pdev, 0);
>  	s->port.irq = s->irq;
> @@ -805,12 +827,6 @@ static int __devexit mxs_auart_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static struct of_device_id mxs_auart_dt_ids[] = {
> -	{ .compatible = "fsl,imx23-auart", },
> -	{ /* sentinel */ }
> -};
> -MODULE_DEVICE_TABLE(of, mxs_auart_dt_ids);
> -
>  static struct platform_driver mxs_auart_driver = {
>  	.probe = mxs_auart_probe,
>  	.remove = __devexit_p(mxs_auart_remove),
> -- 
> 1.7.0.4
> 
> 

^ permalink raw reply

* Re: [PATCH 2/3] serial: mxs-auart: add the DMA support for mx28
From: Shawn Guo @ 2012-10-18  3:20 UTC (permalink / raw)
  To: Huang Shijie
  Cc: gregkh, alan, linux-serial, linux-arm-kernel, linux,
	lauri.hintsala, vinod.koul
In-Reply-To: <1350367386-7742-3-git-send-email-b32955@freescale.com>

On Tue, Oct 16, 2012 at 02:03:05PM +0800, Huang Shijie wrote:
> Only we meet the following conditions, we can enable the DMA support for
> auart:
> 
>   (1) We enable the DMA support in the dts file, such as
>       arch/arm/boot/dts/imx28.dtsi.
> 
>   (2) We enable the hardware flow control.
> 
>   (3) We use the mx28, not the mx23. Due to hardware bug(see errata: 2836),
>       we can not add the DMA support to mx23.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  .../bindings/tty/serial/fsl-mxs-auart.txt          |    7 +
>  drivers/tty/serial/mxs-auart.c                     |  307 +++++++++++++++++++-
>  2 files changed, 311 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt
> index a154bf1..67e54b4 100644
> --- a/Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt
> +++ b/Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt
> @@ -6,11 +6,18 @@ Required properties:
>  - reg : Address and length of the register set for the device
>  - interrupts : Should contain the auart interrupt numbers
>  
> +Optional properties:
> +- fsl,auart-dma-channel : The DMA channels, the first is for RX, the other
> +			is for TX.
> +- fsl,auart-enable-dma : Enable the DMA support for the auart.
> +

If we want to have it decided by device tree, can we drop the property
and simply check if "fsl,auart-dma-channel" presents?

>  Example:
>  auart0: serial@8006a000 {
>  	compatible = "fsl,imx28-auart";
>  	reg = <0x8006a000 0x2000>;
>  	interrupts = <112 70 71>;
> +	fsl,auart-dma-channel = <8 9>;
> +	fsl,auart-enable-dma;
>  };
>  
>  Note: Each auart port should have an alias correctly numbered in "aliases"
> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index cd9ec1d..2271330 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c
> @@ -34,6 +34,8 @@
>  #include <linux/io.h>
>  #include <linux/pinctrl/consumer.h>
>  #include <linux/of_device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/fsl/mxs-dma.h>
>  
>  #include <asm/cacheflush.h>
>  
> @@ -76,7 +78,15 @@
>  
>  #define AUART_CTRL0_SFTRST			(1 << 31)
>  #define AUART_CTRL0_CLKGATE			(1 << 30)
> +#define AUART_CTRL0_RXTO_ENABLE			(1 << 27)
> +#define AUART_CTRL0_RXTIMEOUT(v)		(((v) & 0x7ff) << 16)
> +#define AUART_CTRL0_XFER_COUNT(v)		((v) & 0xffff)
>  
> +#define AUART_CTRL1_XFER_COUNT(v)		((v) & 0xffff)
> +
> +#define AUART_CTRL2_DMAONERR			(1 << 26)
> +#define AUART_CTRL2_TXDMAE			(1 << 25)
> +#define AUART_CTRL2_RXDMAE			(1 << 24)
>  #define AUART_CTRL2_CTSEN			(1 << 15)
>  #define AUART_CTRL2_RTSEN			(1 << 14)
>  #define AUART_CTRL2_RTS				(1 << 11)
> @@ -116,12 +126,15 @@
>  #define AUART_STAT_BERR				(1 << 18)
>  #define AUART_STAT_PERR				(1 << 17)
>  #define AUART_STAT_FERR				(1 << 16)
> +#define AUART_STAT_RXCOUNT_MASK			0xffff
>  
>  static struct uart_driver auart_driver;
>  
>  struct mxs_auart_port {
>  	struct uart_port port;
>  
> +#define MXS_AUART_DMA_CONFIG	0x1
> +#define MXS_AUART_DMA_ENABLED	0x2
>  	unsigned int flags;
>  	unsigned int ctrl;
>  
> @@ -130,16 +143,116 @@ struct mxs_auart_port {
>  	struct clk *clk;
>  	struct device *dev;
>  	struct platform_device *pdev;
> +
> +	/* for DMA */
> +	struct mxs_dma_data dma_data;
> +	int dma_channel_rx, dma_channel_tx;
> +	int dma_irq_rx, dma_irq_tx;
> +	int dma_channel;
> +
> +	struct scatterlist tx_sgl;
> +	struct dma_chan	*tx_dma_chan;
> +	void *tx_dma_buf;
> +
> +	struct scatterlist rx_sgl;
> +	struct dma_chan	*rx_dma_chan;
> +	void *rx_dma_buf;
>  };
>  
> +static inline bool auart_dma_enabled(struct mxs_auart_port *s)
> +{
> +	return s->flags & MXS_AUART_DMA_ENABLED;
> +}
> +
>  static void mxs_auart_stop_tx(struct uart_port *u);
>  
>  #define to_auart_port(u) container_of(u, struct mxs_auart_port, port)
>  
> +static inline void mxs_auart_tx_chars(struct mxs_auart_port *s);
> +
> +static void dma_tx_callback(void *param)
> +{
> +	struct mxs_auart_port *s = param;
> +	struct circ_buf *xmit = &s->port.state->xmit;
> +
> +	dma_unmap_sg(s->dev, &s->tx_sgl, 1, DMA_TO_DEVICE);
> +
> +	/* wake up the possible processes. */
> +	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
> +		uart_write_wakeup(&s->port);
> +
> +	mxs_auart_tx_chars(s);
> +}
> +
> +static int mxs_auart_dma_tx(struct mxs_auart_port *s, int size)
> +{
> +	struct dma_async_tx_descriptor *desc;
> +	struct scatterlist *sgl = &s->tx_sgl;
> +	struct dma_chan *channel = s->tx_dma_chan;
> +	u32 pio[1];

One element array looks strange to me.

> +
> +	/* [1] : send PIO. Note, the first pio word is CTRL1. */
> +	pio[0] = AUART_CTRL1_XFER_COUNT(size);
> +	desc = dmaengine_prep_slave_sg(channel, (struct scatterlist *)pio,
> +					1, DMA_TRANS_NONE, 0);
> +	if (!desc) {
> +		dev_err(s->dev, "step 1 error\n");
> +		return -EINVAL;
> +	}
> +
> +	/* [2] : set DMA buffer. */
> +	sg_init_one(sgl, s->tx_dma_buf, size);
> +	dma_map_sg(s->dev, sgl, 1, DMA_TO_DEVICE);
> +	desc = dmaengine_prep_slave_sg(channel, sgl,
> +			1, DMA_MEM_TO_DEV, DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> +	if (!desc) {
> +		dev_err(s->dev, "step 2 error\n");
> +		return -EINVAL;
> +	}
> +
> +	/* [3] : submit the DMA */
> +	desc->callback = dma_tx_callback;
> +	desc->callback_param = s;
> +	dmaengine_submit(desc);
> +	dma_async_issue_pending(channel);
> +	return 0;
> +}
> +
>  static inline void mxs_auart_tx_chars(struct mxs_auart_port *s)

I'm not sure why this function is inline from the beginning.  It
becomes even more inappropriate after you add more codes below.

Shawn

>  {
>  	struct circ_buf *xmit = &s->port.state->xmit;
>  
> +	if (auart_dma_enabled(s)) {
> +		int i = 0;
> +		int size;
> +		void *buffer = s->tx_dma_buf;
> +		enum dma_status status;
> +
> +		/* Check whether there is pending DMA operations. */
> +		status = dmaengine_tx_status(s->tx_dma_chan, 0, NULL);
> +		if (status != DMA_SUCCESS)
> +			return;
> +
> +		while (!uart_circ_empty(xmit) && !uart_tx_stopped(&s->port)) {
> +			size = min_t(u32, UART_XMIT_SIZE - i,
> +				     CIRC_CNT_TO_END(xmit->head,
> +						     xmit->tail,
> +						     UART_XMIT_SIZE));
> +			memcpy(buffer + i, xmit->buf + xmit->tail, size);
> +			xmit->tail = (xmit->tail + size) & (UART_XMIT_SIZE - 1);
> +
> +			i += size;
> +			if (i >= UART_XMIT_SIZE)
> +				break;
> +		}
> +
> +		if (uart_tx_stopped(&s->port))
> +			mxs_auart_stop_tx(&s->port);
> +		if (i)
> +			mxs_auart_dma_tx(s, i);
> +		return;
> +	}
> +
>  	while (!(readl(s->port.membase + AUART_STAT) &
>  		 AUART_STAT_TXFF)) {
>  		if (s->port.x_char) {
> @@ -293,10 +406,155 @@ static u32 mxs_auart_get_mctrl(struct uart_port *u)
>  	return mctrl;
>  }
>  
> +static bool mxs_auart_dma_filter(struct dma_chan *chan, void *param)
> +{
> +	struct mxs_auart_port *s = param;
> +
> +	if (!mxs_dma_is_apbx(chan))
> +		return false;
> +
> +	if (s->dma_channel == chan->chan_id) {
> +		chan->private = &s->dma_data;
> +		return true;
> +	}
> +	return false;
> +}
> +
> +static int mxs_auart_dma_prep_rx(struct mxs_auart_port *s);
> +static void dma_rx_callback(void *arg)
> +{
> +	struct mxs_auart_port *s = (struct mxs_auart_port *) arg;
> +	struct tty_struct *tty = s->port.state->port.tty;
> +	int count;
> +	u32 stat;
> +
> +	stat = readl(s->port.membase + AUART_STAT);
> +	stat &= ~(AUART_STAT_OERR | AUART_STAT_BERR |
> +			AUART_STAT_PERR | AUART_STAT_FERR);
> +
> +	count = stat & AUART_STAT_RXCOUNT_MASK;
> +	tty_insert_flip_string(tty, s->rx_dma_buf, count);
> +
> +	writel(stat, s->port.membase + AUART_STAT);
> +	tty_flip_buffer_push(tty);
> +
> +	/* start the next DMA for RX. */
> +	mxs_auart_dma_prep_rx(s);
> +}
> +
> +static int mxs_auart_dma_prep_rx(struct mxs_auart_port *s)
> +{
> +	struct dma_async_tx_descriptor *desc;
> +	struct scatterlist *sgl = &s->rx_sgl;
> +	struct dma_chan *channel = s->rx_dma_chan;
> +	u32 pio[1];
> +
> +	/* [1] : send PIO */
> +	pio[0] = AUART_CTRL0_RXTO_ENABLE
> +		| AUART_CTRL0_RXTIMEOUT(0x80)
> +		| AUART_CTRL0_XFER_COUNT(UART_XMIT_SIZE);
> +	desc = dmaengine_prep_slave_sg(channel, (struct scatterlist *)pio,
> +					1, DMA_TRANS_NONE, 0);
> +	if (!desc) {
> +		dev_err(s->dev, "step 1 error\n");
> +		return -EINVAL;
> +	}
> +
> +	/* [2] : send DMA request */
> +	sg_init_one(sgl, s->rx_dma_buf, UART_XMIT_SIZE);
> +	dma_map_sg(s->dev, sgl, 1, DMA_FROM_DEVICE);
> +	desc = dmaengine_prep_slave_sg(channel, sgl, 1, DMA_DEV_TO_MEM,
> +					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> +	if (!desc) {
> +		dev_err(s->dev, "step 2 error\n");
> +		return -1;
> +	}
> +
> +	/* [3] : submit the DMA, but do not issue it. */
> +	desc->callback = dma_rx_callback;
> +	desc->callback_param = s;
> +	dmaengine_submit(desc);
> +	dma_async_issue_pending(channel);
> +	return 0;
> +}
> +
> +static void mxs_auart_dma_exit_channel(struct mxs_auart_port *s)
> +{
> +	if (s->tx_dma_chan) {
> +		dma_release_channel(s->tx_dma_chan);
> +		s->tx_dma_chan = NULL;
> +	}
> +	if (s->rx_dma_chan) {
> +		dma_release_channel(s->rx_dma_chan);
> +		s->rx_dma_chan = NULL;
> +	}
> +
> +	kfree(s->tx_dma_buf);
> +	kfree(s->rx_dma_buf);
> +	s->tx_dma_buf = NULL;
> +	s->rx_dma_buf = NULL;
> +}
> +
> +static void mxs_auart_dma_exit(struct mxs_auart_port *s)
> +{
> +
> +	writel(AUART_CTRL2_TXDMAE | AUART_CTRL2_RXDMAE | AUART_CTRL2_DMAONERR,
> +		s->port.membase + AUART_CTRL2_CLR);
> +
> +	mxs_auart_dma_exit_channel(s);
> +	s->flags &= ~MXS_AUART_DMA_ENABLED;
> +}
> +
> +static int mxs_auart_dma_init(struct mxs_auart_port *s)
> +{
> +	dma_cap_mask_t mask;
> +
> +	if (auart_dma_enabled(s))
> +		return 0;
> +
> +	/* We do not get the right DMA channels. */
> +	if (s->dma_channel_rx == -1 || s->dma_channel_rx == -1)
> +		return -EINVAL;
> +
> +	/* init for RX */
> +	dma_cap_zero(mask);
> +	dma_cap_set(DMA_SLAVE, mask);
> +	s->dma_channel = s->dma_channel_rx;
> +	s->dma_data.chan_irq = s->dma_irq_rx;
> +	s->rx_dma_chan = dma_request_channel(mask, mxs_auart_dma_filter, s);
> +	if (!s->rx_dma_chan)
> +		goto err_out;
> +	s->rx_dma_buf = kzalloc(UART_XMIT_SIZE, GFP_KERNEL | GFP_DMA);
> +	if (!s->rx_dma_buf)
> +		goto err_out;
> +
> +	/* init for TX */
> +	s->dma_channel = s->dma_channel_tx;
> +	s->dma_data.chan_irq = s->dma_irq_tx;
> +	s->tx_dma_chan = dma_request_channel(mask, mxs_auart_dma_filter, s);
> +	if (!s->tx_dma_chan)
> +		goto err_out;
> +	s->tx_dma_buf = kzalloc(UART_XMIT_SIZE, GFP_KERNEL | GFP_DMA);
> +	if (!s->tx_dma_buf)
> +		goto err_out;
> +
> +	/* set the flags */
> +	s->flags |= MXS_AUART_DMA_ENABLED;
> +	dev_dbg(s->dev, "enabled the DMA support.");
> +
> +	return 0;
> +
> +err_out:
> +	mxs_auart_dma_exit_channel(s);
> +	return -EINVAL;
> +
> +}
> +
>  static void mxs_auart_settermios(struct uart_port *u,
>  				 struct ktermios *termios,
>  				 struct ktermios *old)
>  {
> +	struct mxs_auart_port *s = to_auart_port(u);
>  	u32 bm, ctrl, ctrl2, div;
>  	unsigned int cflag, baud;
>  
> @@ -368,10 +626,23 @@ static void mxs_auart_settermios(struct uart_port *u,
>  		ctrl |= AUART_LINECTRL_STP2;
>  
>  	/* figure out the hardware flow control settings */
> -	if (cflag & CRTSCTS)
> +	if (cflag & CRTSCTS) {
> +		/*
> +		 * The DMA has a bug(see errata:2836) in mx23.
> +		 * So we can not implement the DMA for auart in mx23,
> +		 * we can only implement the DMA support for auart
> +		 * in mx28.
> +		 */
> +		if (!AUART_IS_MX23(s) && (s->flags & MXS_AUART_DMA_CONFIG)) {
> +			if (!mxs_auart_dma_init(s))
> +				/* enable DMA tranfer */
> +				ctrl2 |= AUART_CTRL2_TXDMAE | AUART_CTRL2_RXDMAE
> +				       | AUART_CTRL2_DMAONERR;
> +		}
>  		ctrl2 |= AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN;
> -	else
> +	} else {
>  		ctrl2 &= ~(AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN);
> +	}
>  
>  	/* set baud rate */
>  	baud = uart_get_baud_rate(u, termios, old, 0, u->uartclk);
> @@ -383,6 +654,17 @@ static void mxs_auart_settermios(struct uart_port *u,
>  	writel(ctrl2, u->membase + AUART_CTRL2);
>  
>  	uart_update_timeout(u, termios->c_cflag, baud);
> +
> +	/* prepare for the DMA RX. */
> +	if (auart_dma_enabled(s)) {
> +		if (!mxs_auart_dma_prep_rx(s)) {
> +			/* Disable the normal RX interrupt. */
> +			writel(AUART_INTR_RXIEN, u->membase + AUART_INTR_CLR);
> +		} else {
> +			mxs_auart_dma_exit(s);
> +			dev_err(s->dev, "We can not start up the DMA.\n");
> +		}
> +	}
>  }
>  
>  static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
> @@ -461,6 +743,9 @@ static void mxs_auart_shutdown(struct uart_port *u)
>  {
>  	struct mxs_auart_port *s = to_auart_port(u);
>  
> +	if (auart_dma_enabled(s))
> +		mxs_auart_dma_exit(s);
> +
>  	writel(AUART_CTRL2_UARTEN, u->membase + AUART_CTRL2_CLR);
>  
>  	writel(AUART_INTR_RXIEN | AUART_INTR_RTIEN | AUART_INTR_CTSMIEN,
> @@ -707,6 +992,7 @@ static int serial_mxs_probe_dt(struct mxs_auart_port *s,
>  		struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node;
> +	u32 dma_channel[2];
>  	int ret;
>  
>  	if (!np)
> @@ -722,6 +1008,22 @@ static int serial_mxs_probe_dt(struct mxs_auart_port *s,
>  
>  	pdev->id_entry = of_match_device(mxs_auart_dt_ids, &pdev->dev)->data;
>  
> +	ret = of_property_read_u32_array(np, "fsl,auart-dma-channel",
> +					dma_channel, 2);
> +	if (ret == 0) {
> +		s->dma_channel_rx = dma_channel[0];
> +		s->dma_channel_tx = dma_channel[1];
> +	} else {
> +		s->dma_channel_rx = -1;
> +		s->dma_channel_tx = -1;
> +	}
> +
> +	s->dma_irq_rx = platform_get_irq(pdev, 1);
> +	s->dma_irq_tx = platform_get_irq(pdev, 2);
> +
> +	if (of_property_read_bool(np, "fsl,auart-enable-dma"))
> +		s->flags |= MXS_AUART_DMA_CONFIG;
> +
>  	return 0;
>  }
>  
> @@ -772,7 +1074,6 @@ static int __devinit mxs_auart_probe(struct platform_device *pdev)
>  	s->port.type = PORT_IMX;
>  	s->port.dev = s->dev = get_device(&pdev->dev);
>  
> -	s->flags = 0;
>  	s->ctrl = 0;
>  	s->pdev = pdev;
>  
> -- 
> 1.7.0.4
> 
> 

^ permalink raw reply

* Re: [PATCH 2/3] serial: mxs-auart: add the DMA support for mx28
From: Huang Shijie @ 2012-10-18  6:22 UTC (permalink / raw)
  To: Shawn Guo
  Cc: gregkh, alan, linux-serial, linux-arm-kernel, linux,
	lauri.hintsala, vinod.koul
In-Reply-To: <20121018032022.GB4378@S2101-09.ap.freescale.net>

于 2012年10月18日 11:20, Shawn Guo 写道:
> On Tue, Oct 16, 2012 at 02:03:05PM +0800, Huang Shijie wrote:
>> Only we meet the following conditions, we can enable the DMA support for
>> auart:
>>
>> @@ -6,11 +6,18 @@ Required properties:
>>   - reg : Address and length of the register set for the device
>>   - interrupts : Should contain the auart interrupt numbers
>>
>> +Optional properties:
>> +- fsl,auart-dma-channel : The DMA channels, the first is for RX, the other
>> +			is for TX.
>> +- fsl,auart-enable-dma : Enable the DMA support for the auart.
>> +
> If we want to have it decided by device tree, can we drop the property
> and simply check if "fsl,auart-dma-channel" presents?
It's ok to me. fix it in next version.
>>   Example:
>>   auart0: serial@8006a000 {
>>   	compatible = "fsl,imx28-auart";
>>   	reg =<0x8006a000 0x2000>;
>>   	interrupts =<112 70 71>;
>> +	fsl,auart-dma-channel =<8 9>;
>> +	fsl,auart-enable-dma;
>>   };
>>
>>   Note: Each auart port should have an alias correctly numbered in "aliases"
>> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
>> index cd9ec1d..2271330 100644
>> --- a/drivers/tty/serial/mxs-auart.c
>> +++ b/drivers/tty/serial/mxs-auart.c
>> @@ -34,6 +34,8 @@
>>   #include<linux/io.h>
>>   #include<linux/pinctrl/consumer.h>
>>   #include<linux/of_device.h>
>> +#include<linux/dma-mapping.h>
>> +#include<linux/fsl/mxs-dma.h>
>>
>>   #include<asm/cacheflush.h>
>>
>> @@ -76,7 +78,15 @@
>>
>>   #define AUART_CTRL0_SFTRST			(1<<  31)
>>   #define AUART_CTRL0_CLKGATE			(1<<  30)
>> +#define AUART_CTRL0_RXTO_ENABLE			(1<<  27)
>> +#define AUART_CTRL0_RXTIMEOUT(v)		(((v)&  0x7ff)<<  16)
>> +#define AUART_CTRL0_XFER_COUNT(v)		((v)&  0xffff)
>>
>> +#define AUART_CTRL1_XFER_COUNT(v)		((v)&  0xffff)
>> +
>> +#define AUART_CTRL2_DMAONERR			(1<<  26)
>> +#define AUART_CTRL2_TXDMAE			(1<<  25)
>> +#define AUART_CTRL2_RXDMAE			(1<<  24)
>>   #define AUART_CTRL2_CTSEN			(1<<  15)
>>   #define AUART_CTRL2_RTSEN			(1<<  14)
>>   #define AUART_CTRL2_RTS				(1<<  11)
>> @@ -116,12 +126,15 @@
>>   #define AUART_STAT_BERR				(1<<  18)
>>   #define AUART_STAT_PERR				(1<<  17)
>>   #define AUART_STAT_FERR				(1<<  16)
>> +#define AUART_STAT_RXCOUNT_MASK			0xffff
>>
>>   static struct uart_driver auart_driver;
>>
>>   struct mxs_auart_port {
>>   	struct uart_port port;
>>
>> +#define MXS_AUART_DMA_CONFIG	0x1
>> +#define MXS_AUART_DMA_ENABLED	0x2
>>   	unsigned int flags;
>>   	unsigned int ctrl;
>>
>> @@ -130,16 +143,116 @@ struct mxs_auart_port {
>>   	struct clk *clk;
>>   	struct device *dev;
>>   	struct platform_device *pdev;
>> +
>> +	/* for DMA */
>> +	struct mxs_dma_data dma_data;
>> +	int dma_channel_rx, dma_channel_tx;
>> +	int dma_irq_rx, dma_irq_tx;
>> +	int dma_channel;
>> +
>> +	struct scatterlist tx_sgl;
>> +	struct dma_chan	*tx_dma_chan;
>> +	void *tx_dma_buf;
>> +
>> +	struct scatterlist rx_sgl;
>> +	struct dma_chan	*rx_dma_chan;
>> +	void *rx_dma_buf;
>>   };
>>
>> +static inline bool auart_dma_enabled(struct mxs_auart_port *s)
>> +{
>> +	return s->flags&  MXS_AUART_DMA_ENABLED;
>> +}
>> +
>>   static void mxs_auart_stop_tx(struct uart_port *u);
>>
>>   #define to_auart_port(u) container_of(u, struct mxs_auart_port, port)
>>
>> +static inline void mxs_auart_tx_chars(struct mxs_auart_port *s);
>> +
>> +static void dma_tx_callback(void *param)
>> +{
>> +	struct mxs_auart_port *s = param;
>> +	struct circ_buf *xmit =&s->port.state->xmit;
>> +
>> +	dma_unmap_sg(s->dev,&s->tx_sgl, 1, DMA_TO_DEVICE);
>> +
>> +	/* wake up the possible processes. */
>> +	if (uart_circ_chars_pending(xmit)<  WAKEUP_CHARS)
>> +		uart_write_wakeup(&s->port);
>> +
>> +	mxs_auart_tx_chars(s);
>> +}
>> +
>> +static int mxs_auart_dma_tx(struct mxs_auart_port *s, int size)
>> +{
>> +	struct dma_async_tx_descriptor *desc;
>> +	struct scatterlist *sgl =&s->tx_sgl;
>> +	struct dma_chan *channel = s->tx_dma_chan;
>> +	u32 pio[1];
> One element array looks strange to me.
got it.
>> +
>> +	/* [1] : send PIO. Note, the first pio word is CTRL1. */
>> +	pio[0] = AUART_CTRL1_XFER_COUNT(size);
>> +	desc = dmaengine_prep_slave_sg(channel, (struct scatterlist *)pio,
>> +					1, DMA_TRANS_NONE, 0);
>> +	if (!desc) {
>> +		dev_err(s->dev, "step 1 error\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* [2] : set DMA buffer. */
>> +	sg_init_one(sgl, s->tx_dma_buf, size);
>> +	dma_map_sg(s->dev, sgl, 1, DMA_TO_DEVICE);
>> +	desc = dmaengine_prep_slave_sg(channel, sgl,
>> +			1, DMA_MEM_TO_DEV, DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
>> +	if (!desc) {
>> +		dev_err(s->dev, "step 2 error\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* [3] : submit the DMA */
>> +	desc->callback = dma_tx_callback;
>> +	desc->callback_param = s;
>> +	dmaengine_submit(desc);
>> +	dma_async_issue_pending(channel);
>> +	return 0;
>> +}
>> +
>>   static inline void mxs_auart_tx_chars(struct mxs_auart_port *s)
> I'm not sure why this function is inline from the beginning.  It
yes. The inline is not proper.

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

* [PATCH-v2] sched: Prevent wakeup to enter critical section needlessly
From: Ivo Sieben @ 2012-10-18  8:30 UTC (permalink / raw)
  To: linux-kernel, Andi Kleen, Oleg Nesterov, Peter Zijlstra,
	Ingo Molnar
  Cc: linux-serial, Alan Cox, Greg KH, Ivo Sieben
In-Reply-To: <20121010140249.GX16230@one.firstfloor.org>

Check the waitqueue task list to be non empty before entering the critical
section. This prevents locking the spin lock needlessly in case the queue
was empty, and therefor also prevent scheduling overhead on a PREEMPT_RT
system.

Signed-off-by: Ivo Sieben <meltedpianoman@gmail.com>
---

 v2:
 - We don't need the "careful" list empty, a normal list empty is sufficient:
   if you miss an update it was just as it happened a little later.
 - Because of memory ordering problems we can observe an unupdated list
   administration. This can cause an wait_event-like code to miss an event.
   Adding a memory barrier befor checking the list to be empty will guarantee we
   evaluate a 100% updated list adminsitration.

 kernel/sched/core.c |   19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 2d8927f..168a9b2 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3090,9 +3090,22 @@ void __wake_up(wait_queue_head_t *q, unsigned int mode,
 {
 	unsigned long flags;
 
-	spin_lock_irqsave(&q->lock, flags);
-	__wake_up_common(q, mode, nr_exclusive, 0, key);
-	spin_unlock_irqrestore(&q->lock, flags);
+	/*
+	 * We check for list emptiness outside the lock. This prevents the wake
+	 * up to enter the critical section needlessly when the task list is
+	 * empty.
+	 *
+	 * Placed a full memory barrier before checking list emptiness to make
+	 * 100% sure this function sees an up-to-date list administration.
+	 * Note that other code that manipulates the list uses a spin_lock and
+	 * therefore doesn't need additional memory barriers.
+	 */
+	smp_mb();
+	if (!list_empty(&q->task_list)) {
+		spin_lock_irqsave(&q->lock, flags);
+		__wake_up_common(q, mode, nr_exclusive, 0, key);
+		spin_unlock_irqrestore(&q->lock, flags);
+	}
 }
 EXPORT_SYMBOL(__wake_up);
 
-- 
1.7.9.5



^ permalink raw reply related

* [PATCH] serial: ifx6x60: del_timer_sync must not be called in interrupt context.
From: Jun Chen @ 2012-10-19 13:51 UTC (permalink / raw)
  To: alan; +Cc: linux-serial, russ.gorby, chuansheng.liu, chao.bi


This patch make use of del_timer instead of del_timer_sync in the
interrupt context.
The spi_timer function don't use any resources that may release after
running del_timer,
so using the del_timer is also safe and enough in this context.

Signed-off-by: Chen Jun <jun.d.chen@intel.com>
---
 drivers/tty/serial/ifx6x60.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/tty/serial/ifx6x60.c b/drivers/tty/serial/ifx6x60.c
index 5b9bc19..769396a 100644
--- a/drivers/tty/serial/ifx6x60.c
+++ b/drivers/tty/serial/ifx6x60.c
@@ -826,7 +826,7 @@ error_ret:
 static void ifx_spi_handle_srdy(struct ifx_spi_device *ifx_dev)
 {
 	if (test_bit(IFX_SPI_STATE_TIMER_PENDING, &ifx_dev->flags)) {
-		del_timer_sync(&ifx_dev->spi_timer);
+		del_timer(&ifx_dev->spi_timer);
 		clear_bit(IFX_SPI_STATE_TIMER_PENDING, &ifx_dev->flags);
 	}
 
-- 
1.7.4.1




^ permalink raw reply related

* RE: [PATCH] serial: ifx6x60: del_timer_sync must not be called in interrupt context.
From: Liu, Chuansheng @ 2012-10-19  6:47 UTC (permalink / raw)
  To: Chen, Jun D, alan@linux.intel.com
  Cc: linux-serial@vger.kernel.org, Gorby, Russ, Bi, Chao
In-Reply-To: <1350654690.4332.12.camel@chenjun-workstation>



> -----Original Message-----
> From: Chen, Jun D
> Sent: Friday, October 19, 2012 9:52 PM
> To: alan@linux.intel.com
> Cc: linux-serial@vger.kernel.org; Gorby, Russ; Liu, Chuansheng; Bi, Chao
> Subject: [PATCH] serial: ifx6x60: del_timer_sync must not be called in interrupt
> context.
> 
> 
> This patch make use of del_timer instead of del_timer_sync in the
> interrupt context.
> The spi_timer function don't use any resources that may release after
> running del_timer,
> so using the del_timer is also safe and enough in this context.
> 
> Signed-off-by: Chen Jun <jun.d.chen@intel.com>
It has been tested on our medfield platform.

^ permalink raw reply

* [PATCH tty-next] x86: ce4100: allow second UART usage
From: Florian Fainelli @ 2012-10-19  8:45 UTC (permalink / raw)
  To: alan; +Cc: linux-serial, gregkh, tglx, linux-kernel, mbizon,
	Florian Fainelli

From: Maxime Bizon <mbizon@freebox.fr>

The current CE4100 and 8250_pci code have both a limitation preventing the
registration and usage of CE4100's second UART. This patch changes the
platform code fixing up the UART port to work on a relative UART port
base address, as well as the 8250_pci code to make it register 2 UART ports
for CE4100 and pass the port index down to all consumers.

Signed-off-by: Florian Fainelli <ffainelli@freebox.fr>
---
 arch/x86/platform/ce4100/ce4100.c  |    3 +++
 drivers/tty/serial/8250/8250_pci.c |    6 +++---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/x86/platform/ce4100/ce4100.c b/arch/x86/platform/ce4100/ce4100.c
index 4c61b52..0dcc30e 100644
--- a/arch/x86/platform/ce4100/ce4100.c
+++ b/arch/x86/platform/ce4100/ce4100.c
@@ -92,8 +92,11 @@ static void ce4100_serial_fixup(int port, struct uart_port *up,
 		up->membase =
 			(void __iomem *)__fix_to_virt(FIX_EARLYCON_MEM_BASE);
 		up->membase += up->mapbase & ~PAGE_MASK;
+		up->mapbase += port * 0x100;
+		up->membase += port * 0x100;
 		up->iotype   = UPIO_MEM32;
 		up->regshift = 2;
+		up->irq = 4;
 	}
 #endif
 	up->iobase = 0;
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 17b7d26..cec8852 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1068,7 +1068,7 @@ ce4100_serial_setup(struct serial_private *priv,
 {
 	int ret;
 
-	ret = setup_port(priv, port, 0, 0, board->reg_shift);
+	ret = setup_port(priv, port, idx, 0, board->reg_shift);
 	port->port.iotype = UPIO_MEM32;
 	port->port.type = PORT_XSCALE;
 	port->port.flags = (port->port.flags | UPF_FIXED_PORT | UPF_FIXED_TYPE);
@@ -2658,8 +2658,8 @@ static struct pciserial_board pci_boards[] __devinitdata = {
 		.first_offset	= 0x1000,
 	},
 	[pbn_ce4100_1_115200] = {
-		.flags		= FL_BASE0,
-		.num_ports	= 1,
+		.flags		= FL_BASE_BARS,
+		.num_ports	= 2,
 		.base_baud	= 921600,
 		.reg_shift      = 2,
 	},
-- 
1.7.9.5


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox