| Message ID | 1397668667-27328-6-git-send-email-ynvich@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> + baud = uart_get_baud_rate(port, termios, old, > + port->uartclk / 16 / 0xffff, > + port->uartclk / 16); > + switch (baud) { > + case 2400: > + len |= 1; > + break; > + case 4800: > + len |= 2; > + break; > + case 19200: > + len |= 4; > + break; > + case 38400: > + len |= 5; > + break; > + case 57600: > + len |= 6; > + break; > + case 115200: > + len |= 7; > + break; > + case 9600: > + default: > + len |= 3; > + break; > + }; Some explanation of this would be useful - eg why is it set to 7 for 115200 baud and 3 for 115201 baud ?
One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> wrote: >> + baud = uart_get_baud_rate(port, termios, old, >> + port->uartclk / 16 / 0xffff, >> + port->uartclk / 16); >> + switch (baud) { >> + case 2400: >> + len |= 1; >> + break; >> + case 4800: >> + len |= 2; >> + break; >> + case 19200: >> + len |= 4; >> + break; >> + case 38400: >> + len |= 5; >> + break; >> + case 57600: >> + len |= 6; >> + break; >> + case 115200: >> + len |= 7; >> + break; >> + case 9600: >> + default: >> + len |= 3; >> + break; >> + }; > >Some explanation of this would be useful - eg why is it set to 7 for >115200 baud and 3 for 115201 baud ? I am not related to the device vendor in any way, so please take my answers for what they are worth. It seems that there is not enough pins to properly connect the chips to the memory bus and just you the standard 8250 UART driver. Instead, clock divisor is set using this register. So, if you know you're asking for (115200) you get it. If you don't or guess (115201), you get the default 9600. This is a policy, it may not be the right way to write a driver, but it is cheap and it works.
On Wed, 16 Apr 2014 23:01:47 +0400 Sergei Ianovich <ynvich@gmail.com> wrote: > One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> wrote: > >> + baud = uart_get_baud_rate(port, termios, old, > >> + port->uartclk / 16 / 0xffff, > >> + port->uartclk / 16); > >> + switch (baud) { > >> + case 2400: > >> + len |= 1; > >> + break; > >> + case 4800: > >> + len |= 2; > >> + break; > >> + case 19200: > >> + len |= 4; > >> + break; > >> + case 38400: > >> + len |= 5; > >> + break; > >> + case 57600: > >> + len |= 6; > >> + break; > >> + case 115200: > >> + len |= 7; > >> + break; > >> + case 9600: > >> + default: > >> + len |= 3; > >> + break; > >> + }; > > > >Some explanation of this would be useful - eg why is it set to 7 for > >115200 baud and 3 for 115201 baud ? > > I am not related to the device vendor in any way, so please take my answers for what they are worth. > > It seems that there is not enough pins to properly connect the chips to the memory bus and just you the standard 8250 UART driver. Instead, clock divisor is set using this register. > > So, if you know you're asking for (115200) you get it. If you don't or guess (115201), you get the default 9600. Thats not quite what the code actually implements though. > This is a policy, it may not be the right way to write a driver, but it is cheap and it works. If thats how the hardware works and only supports a few fixed rates then fine, but for default: you need to actually update he baudrate in the passed termios struct so that the userspace knows its request was not matched. Take a look how the standard 8250 termios does it. Also looking at this and some of the other serial bits I see no dependencies on the problematic DMA driver, so does that mean you've got a set of pieces that can be submitted anyway while the DMA driver discussion continues ? Alan
One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> wrote: >On Wed, 16 Apr 2014 23:01:47 +0400 >Sergei Ianovich <ynvich@gmail.com> wrote: > >> One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> wrote: >> >> + baud = uart_get_baud_rate(port, termios, old, >> >> + port->uartclk / 16 / 0xffff, >> >> + port->uartclk / 16); >> >> + switch (baud) { >> >> + case 2400: >> >> + len |= 1; >> >> + break; >> >> + case 4800: >> >> + len |= 2; >> >> + break; >> >> + case 19200: >> >> + len |= 4; >> >> + break; >> >> + case 38400: >> >> + len |= 5; >> >> + break; >> >> + case 57600: >> >> + len |= 6; >> >> + break; >> >> + case 115200: >> >> + len |= 7; >> >> + break; >> >> + case 9600: >> >> + default: >> >> + len |= 3; >> >> + break; >> >> + }; >> > >> >Some explanation of this would be useful - eg why is it set to 7 for >> >115200 baud and 3 for 115201 baud ? >> >> I am not related to the device vendor in any way, so please take my >answers for what they are worth. >> >> It seems that there is not enough pins to properly connect the chips >to the memory bus and just you the standard 8250 UART driver. Instead, >clock divisor is set using this register. >> >> So, if you know you're asking for (115200) you get it. If you don't >or guess (115201), you get the default 9600. > >Thats not quite what the code actually implements though. >> This is a policy, it may not be the right way to write a driver, but >it is cheap and it works. >If thats how the hardware works and only supports a few fixed rates >then >fine, but for default: you need to actually update he baudrate in the >passed termios struct so that the userspace knows its request was not >matched. > >Take a look how the standard 8250 termios does it. I will. I've tested the driver by setting termios from userspace. There was no chance to ask for arbitrary rate, so I didn't think much about this case. >Also looking at this and some of the other serial bits I see no >dependencies on the problematic DMA driver, so does that mean you've >got >a set of pieces that can be submitted anyway while the DMA driver >discussion continues ? Yes, getting each peace in will reduce the support burden. The degree of interdependence varies from feature to feature. rtc, mtd, irq and serial could be taken independently. The main patch is #8. It requires #1-7. The is problem with DMA is one of policy, not technical one. It is explained in #6-7 in details. Everything in the misc depends on bus. But the bus is very simple. There is even no interrupts.
diff --git a/Documentation/devicetree/bindings/serial/lp8x4x-serial.txt b/Documentation/devicetree/bindings/serial/lp8x4x-serial.txt new file mode 100644 index 0000000..5f9a4c1 --- /dev/null +++ b/Documentation/devicetree/bindings/serial/lp8x4x-serial.txt @@ -0,0 +1,35 @@ +UART ports on ICP DAS LP-8x4x + +ICP DAS LP-8x4x contains three additional serial ports interfaced via +Analog Devices ADM213EA chips in addition to 3 serial ports on PXA CPU. + +Required properties: +- compatible : should be "icpdas,uart-lp8x4x" + +- reg : should provide 16 byte man IO memory region and 1 byte region for + termios + +- interrupts : should provide interrupt + +- interrupt-parent : should provide a link to interrupt controller either + explicitly or implicitly from a parent node + +Examples (from pxa27x-lp8x4x.dts): + + uart@9050 { + compatible = "icpdas,uart-lp8x4x"; + reg = <0x9050 0x10 + 0x9030 0x02>; + interrupt-parent = <&fpgairg>; + interrupts = <13>; + status = "okay"; + }; + + uart@9060 { + compatible = "icpdas,uart-lp8x4x"; + reg = <0x9060 0x10 + 0x9032 0x02>; + interrupt-parent = <&fpgairg>; + interrupts = <14>; + status = "okay"; + }; diff --git a/arch/arm/configs/lp8x4x_defconfig b/arch/arm/configs/lp8x4x_defconfig index 17a4e6f..9116ce1 100644 --- a/arch/arm/configs/lp8x4x_defconfig +++ b/arch/arm/configs/lp8x4x_defconfig @@ -112,6 +112,7 @@ CONFIG_SERIAL_8250_EXTENDED=y CONFIG_SERIAL_8250_MANY_PORTS=y CONFIG_SERIAL_8250_SHARE_IRQ=y CONFIG_SERIAL_PXA=y +CONFIG_SERIAL_8250_LP8X4X=m CONFIG_HW_RANDOM=y CONFIG_I2C=m # CONFIG_I2C_COMPAT is not set diff --git a/drivers/tty/serial/8250/8250_lp8x4x.c b/drivers/tty/serial/8250/8250_lp8x4x.c new file mode 100644 index 0000000..93e0f59 --- /dev/null +++ b/drivers/tty/serial/8250/8250_lp8x4x.c @@ -0,0 +1,169 @@ +/* linux/drivers/tty/serial/8250/8250_lp8x4x.c + * + * Support for 16550A serial ports on ICP DAS LP-8x4x + * + * Copyright (C) 2013 Sergei Ianovich <ynvich@gmail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ +#include <linux/init.h> +#include <linux/io.h> +#include <linux/irq.h> +#include <linux/module.h> +#include <linux/serial_8250.h> +#include <linux/of.h> +#include <linux/platform_device.h> + +struct lp8x4x_serial_data { + int line; + void *ios_mem; +}; + +static void lp8x4x_serial_set_termios(struct uart_port *port, + struct ktermios *termios, struct ktermios *old) +{ + unsigned int len; + unsigned int baud; + struct lp8x4x_serial_data *data = port->private_data; + + switch (termios->c_cflag & CSIZE) { + case CS5: + len = 7; + break; + case CS6: + len = 8; + break; + case CS7: + len = 9; + break; + default: + case CS8: + len = 10; + break; + } + + if (termios->c_cflag & CSTOPB) + len++; + if (termios->c_cflag & PARENB) + len++; + if (!(termios->c_cflag & PARODD)) + len++; +#ifdef CMSPAR + if (termios->c_cflag & CMSPAR) + len++; +#endif + + len -= 9; + len &= 3; + len <<= 3; + /* + * Ask the core to calculate the divisor for us. + */ + baud = uart_get_baud_rate(port, termios, old, + port->uartclk / 16 / 0xffff, + port->uartclk / 16); + switch (baud) { + case 2400: + len |= 1; + break; + case 4800: + len |= 2; + break; + case 19200: + len |= 4; + break; + case 38400: + len |= 5; + break; + case 57600: + len |= 6; + break; + case 115200: + len |= 7; + break; + case 9600: + default: + len |= 3; + break; + }; + iowrite8(len, data->ios_mem); + + serial8250_do_set_termios(port, termios, old); +} + +static struct of_device_id lp8x4x_serial_dt_ids[] = { + { .compatible = "icpdas,uart-lp8x4x", }, + {} +}; +MODULE_DEVICE_TABLE(of, lp8x4x_serial_dt_ids); + +static int lp8x4x_serial_probe(struct platform_device *pdev) +{ + struct uart_8250_port uart = {}; + struct lp8x4x_serial_data *data; + struct resource *mmres, *mires, *irqres; + int ret; + + mmres = platform_get_resource(pdev, IORESOURCE_MEM, 0); + mires = platform_get_resource(pdev, IORESOURCE_MEM, 1); + irqres = platform_get_resource(pdev, IORESOURCE_IRQ, 0); + if (!mmres || !mires || !irqres) + return -ENODEV; + + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + data->ios_mem = devm_ioremap_resource(&pdev->dev, mires); + if (!data->ios_mem) + return -EFAULT; + + uart.port.iotype = UPIO_MEM; + uart.port.mapbase = mmres->start; + uart.port.iobase = mmres->start; + uart.port.regshift = 1; + uart.port.irq = irqres->start; + uart.port.flags = UPF_IOREMAP; + uart.port.dev = &pdev->dev; + uart.port.uartclk = 14745600; + uart.port.set_termios = lp8x4x_serial_set_termios; + uart.port.private_data = data; + + ret = serial8250_register_8250_port(&uart); + if (ret < 0) + return ret; + + data->line = ret; + + platform_set_drvdata(pdev, data); + + return 0; +} + +static int lp8x4x_serial_remove(struct platform_device *pdev) +{ + struct lp8x4x_serial_data *data = platform_get_drvdata(pdev); + + serial8250_unregister_port(data->line); + + return 0; +} + +static struct platform_driver lp8x4x_serial_driver = { + .probe = lp8x4x_serial_probe, + .remove = lp8x4x_serial_remove, + + .driver = { + .name = "uart-lp8x4x", + .owner = THIS_MODULE, + .of_match_table = lp8x4x_serial_dt_ids, + }, +}; + +module_platform_driver(lp8x4x_serial_driver); + +MODULE_AUTHOR("Sergei Ianovich"); +MODULE_DESCRIPTION("8250 serial port module for LP-8x4x"); +MODULE_LICENSE("GPL"); diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig index 81bd7c9..9fb0fbb 100644 --- a/drivers/tty/serial/8250/Kconfig +++ b/drivers/tty/serial/8250/Kconfig @@ -311,3 +311,15 @@ config SERIAL_PXA can enable its onboard serial ports by enabling this option. If you choose M here, the module name will be 8250_pxa. + +config SERIAL_8250_LP8X4X + tristate "Support 16550A ports on ICP DAS LP-8x4x" + depends on OF && SERIAL_8250 && SERIAL_8250_MANY_PORTS && ARCH_PXA + select LP8X4X_IRQ + help + In addition to serial ports on PXA270 SoC, LP-8x4x has 1 dual + RS232/RS485 port, 1 RS485 port and 1 RS232 port. + + Say N here, unless you plan to run this kernel on a LP-8x4x system. + + If you choose M here, the module name will be 8250_lp8x4x. diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile index b7d1b61..7370bfb 100644 --- a/drivers/tty/serial/8250/Makefile +++ b/drivers/tty/serial/8250/Makefile @@ -17,6 +17,7 @@ obj-$(CONFIG_SERIAL_8250_ACCENT) += 8250_accent.o obj-$(CONFIG_SERIAL_8250_BOCA) += 8250_boca.o obj-$(CONFIG_SERIAL_8250_EXAR_ST16C554) += 8250_exar_st16c554.o obj-$(CONFIG_SERIAL_8250_HUB6) += 8250_hub6.o +obj-$(CONFIG_SERIAL_8250_LP8X4X) += 8250_lp8x4x.o obj-$(CONFIG_SERIAL_8250_FSL) += 8250_fsl.o obj-$(CONFIG_SERIAL_8250_DW) += 8250_dw.o obj-$(CONFIG_SERIAL_8250_EM) += 8250_em.o