| Message ID | 20230329154235.615349-3-brenda.streiff@ni.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | serial: Add driver for National Instruments UARTs | expand |
On Wed, Mar 29, 2023 at 10:42:35AM -0500, Brenda Streiff wrote: > The National Instruments (NI) 16550 is a 16550-like UART with larger > FIFOs and embedded RS-232/RS-485 transceiver control circuitry. This > patch adds a driver that can operate this UART, which is used for > onboard serial ports in several NI embedded controller designs. People are still making new 8250-like variants with different ways of controlling them these days? That's the design pattern that will not die, or at least, it keeps getting a "value-add" :( > > Portions of this driver were originally written by Jaeden Amero and > Karthik Manamcheri, with extensive cleanups and refactors since. > > Signed-off-by: Brenda Streiff <brenda.streiff@ni.com> > Cc: Gratian Crisan <gratian.crisan@ni.com> > Cc: Jason Smith <jason.smith@ni.com> > --- > MAINTAINERS | 7 + > drivers/tty/serial/8250/8250_ni.c | 447 ++++++++++++++++++++++++++++++ > drivers/tty/serial/8250/Kconfig | 9 + > drivers/tty/serial/8250/Makefile | 1 + > 4 files changed, 464 insertions(+) > create mode 100644 drivers/tty/serial/8250/8250_ni.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index d8ebab595b2a..c5283a7385fa 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -14322,6 +14322,13 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next > F: drivers/mtd/nand/ > F: include/linux/mtd/*nand*.h > > +NATIONAL INSTRUMENTS SERIAL DRIVER > +M: Brenda Streiff <brenda.streiff@ni.com> > +L: linux-serial@vger.kernel.org > +S: Maintained > +F: Documentation/devicetree/bindings/serial/ni,ni16550.yaml > +F: drivers/tty/serial/8250/8250_ni.c > + > NATIVE INSTRUMENTS USB SOUND INTERFACE DRIVER > M: Daniel Mack <zonque@gmail.com> > L: alsa-devel@alsa-project.org (moderated for non-subscribers) > diff --git a/drivers/tty/serial/8250/8250_ni.c b/drivers/tty/serial/8250/8250_ni.c > new file mode 100644 > index 000000000000..8bd9768576a7 > --- /dev/null > +++ b/drivers/tty/serial/8250/8250_ni.c > @@ -0,0 +1,447 @@ > +// SPDX-License-Identifier: GPL-2.0+ Do you really mean "+"? Sorry, I have to ask. > +/* > + * NI 16550 UART Driver > + * > + * The National Instruments (NI) 16550 is a UART that is compatible with the > + * TL16C550C and OX16C950B register interfaces, but has additional functions > + * for RS-485 transceiver control. This driver implements support for the > + * additional functionality on top of the standard serial8250 core. > + * > + * Copyright 2012-2023 National Instruments Corporation Um, 2012 and every year since then? You all have had an out-of-tree driver for 11+ years that has been constantly updated every year? > + */ > + > +#include <linux/acpi.h> > +#include <linux/device.h> > +#include <linux/io.h> > +#include <linux/init.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/property.h> > + > +#include "8250.h" > + > +/* Extra bits in UART_ACR */ > +#define NI16550_ACR_AUTO_DTR_EN BIT(4) > + > +/* TFS - TX FIFO Size */ > +#define NI16550_TFS_OFFSET 0x0C > +/* RFS - RX FIFO Size */ > +#define NI16550_RFS_OFFSET 0x0D > + > +/* PMR - Port Mode Register */ > +#define NI16550_PMR_OFFSET 0x0E > +/* PMR[1:0] - Port Capabilities */ > +#define NI16550_PMR_CAP_MASK 0x03 > +#define NI16550_PMR_NOT_IMPL 0x00 /* not implemented */ > +#define NI16550_PMR_CAP_RS232 0x01 /* RS-232 capable */ > +#define NI16550_PMR_CAP_RS485 0x02 /* RS-485 capable */ > +#define NI16550_PMR_CAP_DUAL 0x03 /* dual-port */ > +/* PMR[4] - Interface Mode */ > +#define NI16550_PMR_MODE_MASK 0x10 > +#define NI16550_PMR_MODE_RS232 0x00 /* currently 232 */ > +#define NI16550_PMR_MODE_RS485 0x10 /* currently 485 */ > + > +/* PCR - Port Control Register */ > +#define NI16550_PCR_OFFSET 0x0F > +#define NI16550_PCR_RS422 0x00 > +#define NI16550_PCR_ECHO_RS485 0x01 > +#define NI16550_PCR_DTR_RS485 0x02 > +#define NI16550_PCR_AUTO_RS485 0x03 > +#define NI16550_PCR_WIRE_MODE_MASK 0x03 > +#define NI16550_PCR_TXVR_ENABLE_BIT BIT(3) > +#define NI16550_PCR_RS485_TERMINATION_BIT BIT(6) > + > +/* flags for ni16550_device_info */ > +#define NI_HAS_PMR BIT(0) > + > +struct ni16550_device_info { > + unsigned int uartclk; > + uint8_t prescaler; Please use proper kernel types, u8. > + unsigned int flags; And that's a horrible packing, do you mean to have those offsets? And why "unsigned int", don't you mean "u64" or "u32"? > +}; > + > +struct ni16550_data { > + int line; > +}; > + > +static int ni16550_enable_transceivers(struct uart_port *port) > +{ > + uint8_t pcr; > + > + pcr = port->serial_in(port, NI16550_PCR_OFFSET); > + pcr |= NI16550_PCR_TXVR_ENABLE_BIT; > + dev_dbg(port->dev, "enable transceivers: write pcr: 0x%02x\n", pcr); > + port->serial_out(port, NI16550_PCR_OFFSET, pcr); > + > + return 0; > +} > + > +static int ni16550_disable_transceivers(struct uart_port *port) > +{ > + uint8_t pcr; > + > + pcr = port->serial_in(port, NI16550_PCR_OFFSET); > + pcr &= ~NI16550_PCR_TXVR_ENABLE_BIT; > + dev_dbg(port->dev, "disable transceivers: write pcr: 0x%02x\n", pcr); > + port->serial_out(port, NI16550_PCR_OFFSET, pcr); > + > + return 0; > +} > + > +static int ni16550_rs485_config(struct uart_port *port, > + struct ktermios *termios, > + struct serial_rs485 *rs485) > +{ > + uint8_t pcr; > + struct uart_8250_port *up = container_of(port, struct uart_8250_port, > + port); > + > + /* "rs485" should be given to us non-NULL. */ > + if (WARN_ON(rs485 == NULL)) Can this ever happen? If not, don't check for it, otherwise you just rebooted a machine that has panic-on-warn enabled :( > + return -EINVAL; Or better yet, handle the case and return the error, why the WARN_ON()? > + > + pcr = serial_in(up, NI16550_PCR_OFFSET); > + pcr &= ~NI16550_PCR_WIRE_MODE_MASK; > + > + if (rs485->flags & SER_RS485_ENABLED) { > + /* RS-485 */ > + dev_vdbg(port->dev, "2-wire Auto\n"); Why "dev_vdbg()"? Why not just "dev_dbg()"? When are you going to rebuild this code to enable the "dev_vdbg()" lines? > + pcr |= NI16550_PCR_AUTO_RS485; > + up->acr |= NI16550_ACR_AUTO_DTR_EN; > + } else { > + /* RS-422 */ > + dev_vdbg(port->dev, "4-wire\n"); > + pcr |= NI16550_PCR_RS422; > + up->acr &= ~NI16550_ACR_AUTO_DTR_EN; > + } > + > + dev_dbg(port->dev, "config rs485: write pcr: 0x%02x, acr: %02x\n", pcr, up->acr); > + serial_out(up, NI16550_PCR_OFFSET, pcr); > + serial_icr_write(up, UART_ACR, up->acr); > + > + return 0; > +} > + > +static bool is_pmr_rs232_mode(struct uart_8250_port *up) > +{ > + uint8_t pmr = serial_in(up, NI16550_PMR_OFFSET); Again, proper kernel types everywhere please "u8". > + > + /* > + * If the PMR is not implemented, then by default NI UARTs are > + * connected to RS-485 transceivers > + */ > + if ((pmr & NI16550_PMR_CAP_MASK) == NI16550_PMR_NOT_IMPL) > + return false; > + > + if ((pmr & NI16550_PMR_CAP_MASK) == NI16550_PMR_CAP_DUAL) > + /* > + * If the port is dual-mode capable, then read the mode bit > + * to know the current mode > + */ > + return ((pmr & NI16550_PMR_MODE_MASK) > + == NI16550_PMR_MODE_RS232); > + else > + /* > + * If it is not dual-mode capable, then decide based on the > + * capability > + */ > + return ((pmr & NI16550_PMR_CAP_MASK) == NI16550_PMR_CAP_RS232); > +} > + > +static void ni16550_config_prescaler(struct uart_8250_port *up, > + uint8_t prescaler) > +{ > + /* > + * Page in the Enhanced Mode Registers > + * Sets EFR[4] for Enhanced Mode. > + */ > + uint8_t lcr_value; > + uint8_t efr_value; > + > + lcr_value = serial_in(up, UART_LCR); > + serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); > + > + efr_value = serial_in(up, UART_EFR); > + efr_value |= UART_EFR_ECB; > + > + serial_out(up, UART_EFR, efr_value); > + > + /* Page out the Enhanced Mode Registers */ > + serial_out(up, UART_LCR, lcr_value); > + > + /* Set prescaler to CPR register. */ > + serial_out(up, UART_SCR, UART_CPR); > + serial_out(up, UART_ICR, prescaler); > +} > + > +static const struct serial_rs485 ni16550_rs485_supported = { > + .flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND, > +}; > + > +static void ni16550_rs485_setup(struct uart_port *port) > +{ > + port->rs485_config = ni16550_rs485_config; > + port->rs485_supported = ni16550_rs485_supported; > + /* > + * The hardware comes up by default in 2-wire auto mode and we > + * set the flags to represent that > + */ > + port->rs485.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND; > +} > + > +static int ni16550_port_startup(struct uart_port *port) > +{ > + int ret; > + > + ret = serial8250_do_startup(port); > + if (ret) > + return ret; > + > + return ni16550_enable_transceivers(port); > +} > + > +static void ni16550_port_shutdown(struct uart_port *port) > +{ > + ni16550_disable_transceivers(port); > + > + serial8250_do_shutdown(port); > +} > + > +static int ni16550_get_regs(struct platform_device *pdev, > + struct uart_port *port) > +{ > + struct resource *regs; > + > + regs = platform_get_resource(pdev, IORESOURCE_IO, 0); > + if (regs) { > + port->iotype = UPIO_PORT; > + port->iobase = regs->start; > + > + return 0; > + } > + > + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (regs) { > + port->iotype = UPIO_MEM; > + port->mapbase = regs->start; > + port->mapsize = resource_size(regs); > + port->flags |= UPF_IOREMAP; > + > + port->membase = devm_ioremap(&pdev->dev, port->mapbase, > + port->mapsize); > + if (!port->membase) > + return -ENOMEM; > + > + return 0; > + } > + > + dev_err(&pdev->dev, "no registers defined\n"); > + return -EINVAL; > +} > + > +static int ni16550_read_fifo_size(struct uart_8250_port *uart, int reg) Why not return "u8"? > +{ > + /* > + * Very old implementations don't have the TFS or RFS registers > + * defined, so we may read all-0s or all-1s. For such devices, > + * assume a FIFO size of 128. > + */ > + int value = serial_in(uart, reg); Again, u8? > + > + if (value == 0x00 || value == 0xFF) > + return 128; > + > + return value; > +} > + > +static void ni16550_set_mctrl(struct uart_port *port, unsigned int mctrl) > +{ > + mctrl |= UART_MCR_CLKSEL; > + > + serial8250_do_set_mctrl(port, mctrl); > +} > + > +static int ni16550_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct uart_8250_port uart = {}; > + struct ni16550_data *data; > + const struct ni16550_device_info *info; > + int ret = 0; > + int irq; > + int rs232_property = 0; > + unsigned int prescaler; > + const char *transceiver; > + int txfifosz, rxfifosz; > + > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + spin_lock_init(&uart.port.lock); > + > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) > + return irq; > + > + ret = ni16550_get_regs(pdev, &uart.port); > + if (ret < 0) > + return ret; > + > + /* early setup so that serial_in()/serial_out() work */ > + serial8250_set_defaults(&uart); > + > + info = device_get_match_data(dev); > + > + uart.port.dev = dev; > + uart.port.irq = irq; > + uart.port.irqflags = IRQF_SHARED; > + uart.port.flags |= UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF > + | UPF_FIXED_PORT | UPF_FIXED_TYPE; > + uart.port.startup = ni16550_port_startup; > + uart.port.shutdown = ni16550_port_shutdown; > + > + /* > + * Hardware instantiation of FIFO sizes are held in registers. > + */ > + txfifosz = ni16550_read_fifo_size(&uart, NI16550_TFS_OFFSET); > + rxfifosz = ni16550_read_fifo_size(&uart, NI16550_RFS_OFFSET); > + > + dev_dbg(dev, "NI 16550 has TX FIFO size %d, RX FIFO size %d\n", > + txfifosz, rxfifosz); > + > + uart.port.type = PORT_16550A; > + uart.port.fifosize = txfifosz; > + uart.tx_loadsz = txfifosz; > + uart.fcr = UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10; > + uart.capabilities = UART_CAP_FIFO | UART_CAP_AFE | UART_CAP_EFR; > + > + /* > + * OF device-tree and NIC7A69 ACPI can declare clock-frequency, > + * but may be missing for other instantiations, so this is optional. > + * If present, override what we've defined in this driver. > + */ > + if (info->uartclk) > + uart.port.uartclk = info->uartclk; > + device_property_read_u32(dev, "clock-frequency", &uart.port.uartclk); > + if (!uart.port.uartclk) { > + dev_err(dev, "unable to determine clock frequency!\n"); > + return -ENODEV; > + } > + > + if (info->prescaler) > + prescaler = info->prescaler; > + device_property_read_u32(dev, "clock-prescaler", &prescaler); > + > + if (prescaler != 0) { > + uart.port.set_mctrl = ni16550_set_mctrl; > + ni16550_config_prescaler(&uart, (uint8_t)prescaler); > + } > + > + /* > + * The determination of whether or not this is an RS-485 or RS-232 port > + * can come from the "transceiver" device property (if present), or it > + * can come from the PMR (if it is present), and otherwise we're solely > + * an RS-485 port. > + */ > + if (!device_property_read_string(dev, "transceiver", &transceiver)) { > + rs232_property = strncmp(transceiver, "RS-232", 6) == 0; > + > + dev_dbg(dev, "port is in %s mode (via device property)", > + (rs232_property ? "RS-232" : "RS-485")); > + } else if (info->flags & NI_HAS_PMR) { > + rs232_property = is_pmr_rs232_mode(&uart); > + > + dev_dbg(dev, "port is in %s mode (via PMR)", > + (rs232_property ? "RS-232" : "RS-485")); > + } else { > + rs232_property = 0; > + > + dev_dbg(dev, "port is fixed as RS-485"); > + } > + > + if (!rs232_property) { > + /* > + * Neither the 'transceiver' property nor the PMR indicate > + * that this is an RS-232 port, so it must be an RS-485 one. > + */ > + ni16550_rs485_setup(&uart.port); > + } > + > + ret = serial8250_register_8250_port(&uart); > + if (ret < 0) > + return ret; > + > + data->line = ret; > + > + platform_set_drvdata(pdev, data); > + > + return 0; > +} > + > +static int ni16550_remove(struct platform_device *pdev) > +{ > + struct ni16550_data *data = platform_get_drvdata(pdev); > + > + serial8250_unregister_port(data->line); > + return 0; > +} > + > +static const struct ni16550_device_info ni16550_default = { }; > + > +static const struct of_device_id ni16550_of_match[] = { > + { .compatible = "ni,ni16550", .data = &ni16550_default }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, ni16550_of_match); > + > +/* NI 16550 RS-485 Interface */ > +static const struct ni16550_device_info nic7750 = { > + .uartclk = 33333333, > +}; > + > +/* NI CVS-145x RS-485 Interface */ > +static const struct ni16550_device_info nic7772 = { > + .uartclk = 1843200, > + .flags = NI_HAS_PMR, > +}; > + > +/* NI cRIO-904x RS-485 Interface */ > +static const struct ni16550_device_info nic792b = { > + /* Sets UART clock rate to 22.222 MHz with 1.125 prescale */ > + .uartclk = 25000000, > + .prescaler = 0x09, > +}; > + > +/* NI sbRIO 96x8 RS-232/485 Interfaces */ > +static const struct ni16550_device_info nic7a69 = { > + /* Set UART clock rate to 29.629 MHz with 1.125 prescale */ > + .uartclk = 29629629, > + .prescaler = 0x09, > +}; > + > +static const struct acpi_device_id ni16550_acpi_match[] = { > + { "NIC7750", (kernel_ulong_t)&nic7750 }, > + { "NIC7772", (kernel_ulong_t)&nic7772 }, > + { "NIC792B", (kernel_ulong_t)&nic792b }, > + { "NIC7A69", (kernel_ulong_t)&nic7a69 }, > + { }, > +}; > +MODULE_DEVICE_TABLE(acpi, ni16550_acpi_match); > + > +static struct platform_driver ni16550_driver = { > + .driver = { > + .name = "ni16550", > + .of_match_table = ni16550_of_match, > + .acpi_match_table = ACPI_PTR(ni16550_acpi_match), > + }, > + .probe = ni16550_probe, > + .remove = ni16550_remove, > +}; > + > +module_platform_driver(ni16550_driver); > + > +MODULE_AUTHOR("Jaeden Amero <jaeden.amero@ni.com>"); > +MODULE_AUTHOR("Karthik Manamcheri <karthik.manamcheri@ni.com>"); > +MODULE_DESCRIPTION("NI 16550 Driver"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig > index 5313aa31930f..c0f3aec51d13 100644 > --- a/drivers/tty/serial/8250/Kconfig > +++ b/drivers/tty/serial/8250/Kconfig > @@ -565,6 +565,15 @@ config SERIAL_8250_BCM7271 > including DMA support and high accuracy BAUD rates, say > Y to this option. If unsure, say N. > > +config SERIAL_8250_NI > + tristate "NI 16550 based serial port" > + depends on SERIAL_8250 > + help > + This driver supports the integrated serial ports on National > + Instruments (NI) controller hardware. This is required for all NI > + controller models with onboard RS-485 or dual-mode RS-485/RS-232 > + ports. Module name if built as a module? thanks, greg k-h
Hi Brenda,
I love your patch! Perhaps something to improve:
[auto build test WARNING on tty/tty-testing]
[also build test WARNING on tty/tty-next tty/tty-linus linus/master v6.3-rc4 next-20230330]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Brenda-Streiff/serial-8250-add-driver-for-NI-UARTs/20230330-010726
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
patch link: https://lore.kernel.org/r/20230329154235.615349-3-brenda.streiff%40ni.com
patch subject: [PATCH tty-next 2/2] serial: 8250: add driver for NI UARTs
config: powerpc-allyesconfig (https://download.01.org/0day-ci/archive/20230330/202303301456.qfo36qC0-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install powerpc cross compiling tool for clang build
# apt-get install binutils-powerpc-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/6c50983e6760b2db288e3f8b7254cea537c0f052
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Brenda-Streiff/serial-8250-add-driver-for-NI-UARTs/20230330-010726
git checkout 6c50983e6760b2db288e3f8b7254cea537c0f052
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash drivers/tty/serial/8250/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303301456.qfo36qC0-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/tty/serial/8250/8250_ni.c:423:36: warning: unused variable 'ni16550_acpi_match' [-Wunused-const-variable]
static const struct acpi_device_id ni16550_acpi_match[] = {
^
1 warning generated.
vim +/ni16550_acpi_match +423 drivers/tty/serial/8250/8250_ni.c
422
> 423 static const struct acpi_device_id ni16550_acpi_match[] = {
424 { "NIC7750", (kernel_ulong_t)&nic7750 },
425 { "NIC7772", (kernel_ulong_t)&nic7772 },
426 { "NIC792B", (kernel_ulong_t)&nic792b },
427 { "NIC7A69", (kernel_ulong_t)&nic7a69 },
428 { },
429 };
430 MODULE_DEVICE_TABLE(acpi, ni16550_acpi_match);
431
On 29/03/2023 17:42, Brenda Streiff wrote: > The National Instruments (NI) 16550 is a 16550-like UART with larger > FIFOs and embedded RS-232/RS-485 transceiver control circuitry. This > patch adds a driver that can operate this UART, which is used for > onboard serial ports in several NI embedded controller designs. > + > +static const struct acpi_device_id ni16550_acpi_match[] = { > + { "NIC7750", (kernel_ulong_t)&nic7750 }, > + { "NIC7772", (kernel_ulong_t)&nic7772 }, > + { "NIC792B", (kernel_ulong_t)&nic792b }, > + { "NIC7A69", (kernel_ulong_t)&nic7a69 }, > + { }, > +}; > +MODULE_DEVICE_TABLE(acpi, ni16550_acpi_match); Looks like should spark warnings on allyesconfig with !ACPI. Best regards, Krzysztof
On Wed, 29 Mar 2023, Brenda Streiff wrote: > The National Instruments (NI) 16550 is a 16550-like UART with larger > FIFOs and embedded RS-232/RS-485 transceiver control circuitry. This > patch adds a driver that can operate this UART, which is used for > onboard serial ports in several NI embedded controller designs. > > Portions of this driver were originally written by Jaeden Amero and > Karthik Manamcheri, with extensive cleanups and refactors since. > > Signed-off-by: Brenda Streiff <brenda.streiff@ni.com> > Cc: Gratian Crisan <gratian.crisan@ni.com> > Cc: Jason Smith <jason.smith@ni.com> > --- > MAINTAINERS | 7 + > drivers/tty/serial/8250/8250_ni.c | 447 ++++++++++++++++++++++++++++++ > drivers/tty/serial/8250/Kconfig | 9 + > drivers/tty/serial/8250/Makefile | 1 + > 4 files changed, 464 insertions(+) > create mode 100644 drivers/tty/serial/8250/8250_ni.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index d8ebab595b2a..c5283a7385fa 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -14322,6 +14322,13 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next > F: drivers/mtd/nand/ > F: include/linux/mtd/*nand*.h > > +NATIONAL INSTRUMENTS SERIAL DRIVER > +M: Brenda Streiff <brenda.streiff@ni.com> > +L: linux-serial@vger.kernel.org > +S: Maintained > +F: Documentation/devicetree/bindings/serial/ni,ni16550.yaml > +F: drivers/tty/serial/8250/8250_ni.c > + > NATIVE INSTRUMENTS USB SOUND INTERFACE DRIVER > M: Daniel Mack <zonque@gmail.com> > L: alsa-devel@alsa-project.org (moderated for non-subscribers) > diff --git a/drivers/tty/serial/8250/8250_ni.c b/drivers/tty/serial/8250/8250_ni.c > new file mode 100644 > index 000000000000..8bd9768576a7 > --- /dev/null > +++ b/drivers/tty/serial/8250/8250_ni.c > @@ -0,0 +1,447 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * NI 16550 UART Driver > + * > + * The National Instruments (NI) 16550 is a UART that is compatible with the > + * TL16C550C and OX16C950B register interfaces, but has additional functions > + * for RS-485 transceiver control. This driver implements support for the > + * additional functionality on top of the standard serial8250 core. > + * > + * Copyright 2012-2023 National Instruments Corporation > + */ > + > +#include <linux/acpi.h> > +#include <linux/device.h> > +#include <linux/io.h> > +#include <linux/init.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/property.h> > + > +#include "8250.h" > + > +/* Extra bits in UART_ACR */ > +#define NI16550_ACR_AUTO_DTR_EN BIT(4) > + > +/* TFS - TX FIFO Size */ > +#define NI16550_TFS_OFFSET 0x0C > +/* RFS - RX FIFO Size */ > +#define NI16550_RFS_OFFSET 0x0D > + > +/* PMR - Port Mode Register */ > +#define NI16550_PMR_OFFSET 0x0E > +/* PMR[1:0] - Port Capabilities */ > +#define NI16550_PMR_CAP_MASK 0x03 > +#define NI16550_PMR_NOT_IMPL 0x00 /* not implemented */ > +#define NI16550_PMR_CAP_RS232 0x01 /* RS-232 capable */ > +#define NI16550_PMR_CAP_RS485 0x02 /* RS-485 capable */ > +#define NI16550_PMR_CAP_DUAL 0x03 /* dual-port */ > +/* PMR[4] - Interface Mode */ > +#define NI16550_PMR_MODE_MASK 0x10 > +#define NI16550_PMR_MODE_RS232 0x00 /* currently 232 */ > +#define NI16550_PMR_MODE_RS485 0x10 /* currently 485 */ > + > +/* PCR - Port Control Register */ > +#define NI16550_PCR_OFFSET 0x0F > +#define NI16550_PCR_RS422 0x00 > +#define NI16550_PCR_ECHO_RS485 0x01 > +#define NI16550_PCR_DTR_RS485 0x02 > +#define NI16550_PCR_AUTO_RS485 0x03 > +#define NI16550_PCR_WIRE_MODE_MASK 0x03 > +#define NI16550_PCR_TXVR_ENABLE_BIT BIT(3) > +#define NI16550_PCR_RS485_TERMINATION_BIT BIT(6) > + > +/* flags for ni16550_device_info */ > +#define NI_HAS_PMR BIT(0) > + > +struct ni16550_device_info { > + unsigned int uartclk; > + uint8_t prescaler; > + unsigned int flags; > +}; > + > +struct ni16550_data { > + int line; > +}; > + > +static int ni16550_enable_transceivers(struct uart_port *port) > +{ > + uint8_t pcr; > + > + pcr = port->serial_in(port, NI16550_PCR_OFFSET); > + pcr |= NI16550_PCR_TXVR_ENABLE_BIT; > + dev_dbg(port->dev, "enable transceivers: write pcr: 0x%02x\n", pcr); > + port->serial_out(port, NI16550_PCR_OFFSET, pcr); > + > + return 0; > +} > + > +static int ni16550_disable_transceivers(struct uart_port *port) > +{ > + uint8_t pcr; > + > + pcr = port->serial_in(port, NI16550_PCR_OFFSET); > + pcr &= ~NI16550_PCR_TXVR_ENABLE_BIT; > + dev_dbg(port->dev, "disable transceivers: write pcr: 0x%02x\n", pcr); > + port->serial_out(port, NI16550_PCR_OFFSET, pcr); > + > + return 0; > +} > + > +static int ni16550_rs485_config(struct uart_port *port, > + struct ktermios *termios, > + struct serial_rs485 *rs485) > +{ > + uint8_t pcr; > + struct uart_8250_port *up = container_of(port, struct uart_8250_port, > + port); Reverse declaration order. > + > + /* "rs485" should be given to us non-NULL. */ > + if (WARN_ON(rs485 == NULL)) > + return -EINVAL; > + > + pcr = serial_in(up, NI16550_PCR_OFFSET); > + pcr &= ~NI16550_PCR_WIRE_MODE_MASK; > + > + if (rs485->flags & SER_RS485_ENABLED) { > + /* RS-485 */ > + dev_vdbg(port->dev, "2-wire Auto\n"); > + pcr |= NI16550_PCR_AUTO_RS485; > + up->acr |= NI16550_ACR_AUTO_DTR_EN; > + } else { > + /* RS-422 */ > + dev_vdbg(port->dev, "4-wire\n"); > + pcr |= NI16550_PCR_RS422; > + up->acr &= ~NI16550_ACR_AUTO_DTR_EN; > + } > + > + dev_dbg(port->dev, "config rs485: write pcr: 0x%02x, acr: %02x\n", pcr, up->acr); > + serial_out(up, NI16550_PCR_OFFSET, pcr); > + serial_icr_write(up, UART_ACR, up->acr); > + > + return 0; > +} > + > +static bool is_pmr_rs232_mode(struct uart_8250_port *up) > +{ > + uint8_t pmr = serial_in(up, NI16550_PMR_OFFSET); > + > + /* > + * If the PMR is not implemented, then by default NI UARTs are > + * connected to RS-485 transceivers > + */ > + if ((pmr & NI16550_PMR_CAP_MASK) == NI16550_PMR_NOT_IMPL) > + return false; > + > + if ((pmr & NI16550_PMR_CAP_MASK) == NI16550_PMR_CAP_DUAL) > + /* > + * If the port is dual-mode capable, then read the mode bit > + * to know the current mode > + */ > + return ((pmr & NI16550_PMR_MODE_MASK) > + == NI16550_PMR_MODE_RS232); Extra parenthesis. > + else > + /* > + * If it is not dual-mode capable, then decide based on the > + * capability > + */ > + return ((pmr & NI16550_PMR_CAP_MASK) == NI16550_PMR_CAP_RS232); Extra parenthesis. Wouldn't it be easier to add do the anding only once into the local variable rather than 4 times? > +} > + > +static void ni16550_config_prescaler(struct uart_8250_port *up, > + uint8_t prescaler) > +{ > + /* > + * Page in the Enhanced Mode Registers > + * Sets EFR[4] for Enhanced Mode. > + */ > + uint8_t lcr_value; > + uint8_t efr_value; > + > + lcr_value = serial_in(up, UART_LCR); > + serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); > + > + efr_value = serial_in(up, UART_EFR); > + efr_value |= UART_EFR_ECB; > + > + serial_out(up, UART_EFR, efr_value); > + > + /* Page out the Enhanced Mode Registers */ > + serial_out(up, UART_LCR, lcr_value); > + > + /* Set prescaler to CPR register. */ > + serial_out(up, UART_SCR, UART_CPR); > + serial_out(up, UART_ICR, prescaler); > +} > + > +static const struct serial_rs485 ni16550_rs485_supported = { > + .flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND, > +}; > + > +static void ni16550_rs485_setup(struct uart_port *port) > +{ > + port->rs485_config = ni16550_rs485_config; > + port->rs485_supported = ni16550_rs485_supported; > + /* > + * The hardware comes up by default in 2-wire auto mode and we > + * set the flags to represent that > + */ > + port->rs485.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND; > +} > + > +static int ni16550_port_startup(struct uart_port *port) > +{ > + int ret; > + > + ret = serial8250_do_startup(port); > + if (ret) > + return ret; > + > + return ni16550_enable_transceivers(port); > +} > + > +static void ni16550_port_shutdown(struct uart_port *port) > +{ > + ni16550_disable_transceivers(port); > + > + serial8250_do_shutdown(port); > +} > + > +static int ni16550_get_regs(struct platform_device *pdev, > + struct uart_port *port) > +{ > + struct resource *regs; > + > + regs = platform_get_resource(pdev, IORESOURCE_IO, 0); > + if (regs) { > + port->iotype = UPIO_PORT; > + port->iobase = regs->start; > + > + return 0; > + } Do you need the port io? > + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (regs) { > + port->iotype = UPIO_MEM; > + port->mapbase = regs->start; > + port->mapsize = resource_size(regs); > + port->flags |= UPF_IOREMAP; > + > + port->membase = devm_ioremap(&pdev->dev, port->mapbase, > + port->mapsize); > + if (!port->membase) > + return -ENOMEM; > + > + return 0; > + } > + > + dev_err(&pdev->dev, "no registers defined\n"); > + return -EINVAL; > +} > + > +static int ni16550_read_fifo_size(struct uart_8250_port *uart, int reg) > +{ > + /* > + * Very old implementations don't have the TFS or RFS registers > + * defined, so we may read all-0s or all-1s. For such devices, > + * assume a FIFO size of 128. > + */ > + int value = serial_in(uart, reg); > + > + if (value == 0x00 || value == 0xFF) > + return 128; > + > + return value; > +} > + > +static void ni16550_set_mctrl(struct uart_port *port, unsigned int mctrl) > +{ > + mctrl |= UART_MCR_CLKSEL; > + > + serial8250_do_set_mctrl(port, mctrl); > +} > + > +static int ni16550_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct uart_8250_port uart = {}; > + struct ni16550_data *data; > + const struct ni16550_device_info *info; > + int ret = 0; Unecessary = 0. > + int irq; > + int rs232_property = 0; > + unsigned int prescaler; > + const char *transceiver; > + int txfifosz, rxfifosz; Try to follow reverse xmas-tree order. > + > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + spin_lock_init(&uart.port.lock); > + > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) > + return irq; > + > + ret = ni16550_get_regs(pdev, &uart.port); > + if (ret < 0) > + return ret; > + > + /* early setup so that serial_in()/serial_out() work */ > + serial8250_set_defaults(&uart); > + > + info = device_get_match_data(dev); > + > + uart.port.dev = dev; > + uart.port.irq = irq; > + uart.port.irqflags = IRQF_SHARED; > + uart.port.flags |= UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF > + | UPF_FIXED_PORT | UPF_FIXED_TYPE; Why |= ?
On 3/29/23 11:30, Greg Kroah-Hartman wrote: Hi Greg, thanks for looking at this so quickly. > On Wed, Mar 29, 2023 at 10:42:35AM -0500, Brenda Streiff wrote: >> The National Instruments (NI) 16550 is a 16550-like UART with larger >> FIFOs and embedded RS-232/RS-485 transceiver control circuitry. This >> patch adds a driver that can operate this UART, which is used for >> onboard serial ports in several NI embedded controller designs. > > People are still making new 8250-like variants with different ways of > controlling them these days? That's the design pattern that will not > die, or at least, it keeps getting a "value-add" :( > This design has been on NI devices in some form since at least around 2009, so I hesitate to call it "new". But yes, it does seem like if you let a hardware engineer be bored for too long you'll end up with a new 8250, a new SPI controller, or a new I2C controller. Sometimes all three! >> +++ b/drivers/tty/serial/8250/8250_ni.c >> @@ -0,0 +1,447 @@ >> +// SPDX-License-Identifier: GPL-2.0+ > > Do you really mean "+"? Sorry, I have to ask. It sits atop 8250_core.c which is marked as GPL-2.0+, and has been marked as 'any later version' since it had been added to our tree [1] in the pre-SPDX times. [1] https://github.com/ni/linux/commit/6bf16de92cc9 >> +/* >> + * NI 16550 UART Driver >> + * >> + * The National Instruments (NI) 16550 is a UART that is compatible with the >> + * TL16C550C and OX16C950B register interfaces, but has additional functions >> + * for RS-485 transceiver control. This driver implements support for the >> + * additional functionality on top of the standard serial8250 core. >> + * >> + * Copyright 2012-2023 National Instruments Corporation > > Um, 2012 and every year since then? You all have had an out-of-tree > driver for 11+ years that has been constantly updated every year? It's been in _a_ tree-- NI's-- in some form for that long. But... yes, this is correct. I can't defend having it out of mainline for so long as having been a good or wise choice, but that is the state of things. >> +/* flags for ni16550_device_info */ >> +#define NI_HAS_PMR BIT(0) >> + >> +struct ni16550_device_info { >> + unsigned int uartclk; >> + uint8_t prescaler; > > Please use proper kernel types, u8. Okay, did a scrub to remove C99 types. > >> + unsigned int flags; > > And that's a horrible packing, do you mean to have those offsets? I wasn't thinking about struct packing here, but yes these can be reordered. > > And why "unsigned int", don't you mean "u64" or "u32"? For "uartclk" it was because struct uart_port::uartclk is an "unsigned int" in include/linux/serial_8250.h. For "flags" it was because there are loads of other places under drivers/tty/serial/8250/* that use "flags" as an "unsigned int" or an "unsigned long". Using a width-named types would be clearer (and I don't mind making the change), but I tried to adhere to the convention in nearby code. >> +static int ni16550_rs485_config(struct uart_port *port, >> + struct ktermios *termios, >> + struct serial_rs485 *rs485) >> +{ >> + uint8_t pcr; >> + struct uart_8250_port *up = container_of(port, struct uart_8250_port, >> + port); >> + >> + /* "rs485" should be given to us non-NULL. */ >> + if (WARN_ON(rs485 == NULL)) > > Can this ever happen? If not, don't check for it, otherwise you just > rebooted a machine that has panic-on-warn enabled :( > >> + return -EINVAL; > > Or better yet, handle the case and return the error, why the WARN_ON()? I'm not sure if this was ever possible, but it doesn't look like any of the other drivers supporting rs485_config do this check today. Into the dustbin it goes.
On 3/31/23 06:46, Ilpo Järvinen wrote: > On Wed, 29 Mar 2023, Brenda Streiff wrote: > >> +static int ni16550_get_regs(struct platform_device *pdev, >> + struct uart_port *port) >> +{ >> + struct resource *regs; >> + >> + regs = platform_get_resource(pdev, IORESOURCE_IO, 0); >> + if (regs) { >> + port->iotype = UPIO_PORT; >> + port->iobase = regs->start; >> + >> + return 0; >> + } > > Do you need the port io? Yes; on our x86_64 hardware this UART IP is in logic connected to LPC, and the registers lie in I/O port space. >> + int irq; >> + int rs232_property = 0; >> + unsigned int prescaler; >> + const char *transceiver; >> + int txfifosz, rxfifosz; > > Try to follow reverse xmas-tree order. Is reverse xmas tree also the rule in the tty subsystem? I was aware of it for netdev but I thought that was a netdev-specific rule (since it only shows up in maintainer-netdev.rst and not more broadly)
On Fri, 31 Mar 2023, Brenda Streiff wrote: > On 3/31/23 06:46, Ilpo Järvinen wrote: > > On Wed, 29 Mar 2023, Brenda Streiff wrote: > > > > + int irq; > > > + int rs232_property = 0; > > > + unsigned int prescaler; > > > + const char *transceiver; > > > + int txfifosz, rxfifosz; > > > > Try to follow reverse xmas-tree order. > > Is reverse xmas tree also the rule in the tty subsystem? I was aware of > it for netdev but I thought that was a netdev-specific rule (since it > only shows up in maintainer-netdev.rst and not more broadly) I'd say that not as strictly as e.g. netdev does so if e.g. due to initialization order it cannot be fully achieved no special trickery is required (in contrast to what you'd get from e.g. netdev telling to put them out of line). It seems generally useful for declarations, especially when they're as many as in the one I picked up above (I think might be due to less eye movement required when looking for a particular variable by its name).
diff --git a/MAINTAINERS b/MAINTAINERS index d8ebab595b2a..c5283a7385fa 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14322,6 +14322,13 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next F: drivers/mtd/nand/ F: include/linux/mtd/*nand*.h +NATIONAL INSTRUMENTS SERIAL DRIVER +M: Brenda Streiff <brenda.streiff@ni.com> +L: linux-serial@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/serial/ni,ni16550.yaml +F: drivers/tty/serial/8250/8250_ni.c + NATIVE INSTRUMENTS USB SOUND INTERFACE DRIVER M: Daniel Mack <zonque@gmail.com> L: alsa-devel@alsa-project.org (moderated for non-subscribers) diff --git a/drivers/tty/serial/8250/8250_ni.c b/drivers/tty/serial/8250/8250_ni.c new file mode 100644 index 000000000000..8bd9768576a7 --- /dev/null +++ b/drivers/tty/serial/8250/8250_ni.c @@ -0,0 +1,447 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * NI 16550 UART Driver + * + * The National Instruments (NI) 16550 is a UART that is compatible with the + * TL16C550C and OX16C950B register interfaces, but has additional functions + * for RS-485 transceiver control. This driver implements support for the + * additional functionality on top of the standard serial8250 core. + * + * Copyright 2012-2023 National Instruments Corporation + */ + +#include <linux/acpi.h> +#include <linux/device.h> +#include <linux/io.h> +#include <linux/init.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/property.h> + +#include "8250.h" + +/* Extra bits in UART_ACR */ +#define NI16550_ACR_AUTO_DTR_EN BIT(4) + +/* TFS - TX FIFO Size */ +#define NI16550_TFS_OFFSET 0x0C +/* RFS - RX FIFO Size */ +#define NI16550_RFS_OFFSET 0x0D + +/* PMR - Port Mode Register */ +#define NI16550_PMR_OFFSET 0x0E +/* PMR[1:0] - Port Capabilities */ +#define NI16550_PMR_CAP_MASK 0x03 +#define NI16550_PMR_NOT_IMPL 0x00 /* not implemented */ +#define NI16550_PMR_CAP_RS232 0x01 /* RS-232 capable */ +#define NI16550_PMR_CAP_RS485 0x02 /* RS-485 capable */ +#define NI16550_PMR_CAP_DUAL 0x03 /* dual-port */ +/* PMR[4] - Interface Mode */ +#define NI16550_PMR_MODE_MASK 0x10 +#define NI16550_PMR_MODE_RS232 0x00 /* currently 232 */ +#define NI16550_PMR_MODE_RS485 0x10 /* currently 485 */ + +/* PCR - Port Control Register */ +#define NI16550_PCR_OFFSET 0x0F +#define NI16550_PCR_RS422 0x00 +#define NI16550_PCR_ECHO_RS485 0x01 +#define NI16550_PCR_DTR_RS485 0x02 +#define NI16550_PCR_AUTO_RS485 0x03 +#define NI16550_PCR_WIRE_MODE_MASK 0x03 +#define NI16550_PCR_TXVR_ENABLE_BIT BIT(3) +#define NI16550_PCR_RS485_TERMINATION_BIT BIT(6) + +/* flags for ni16550_device_info */ +#define NI_HAS_PMR BIT(0) + +struct ni16550_device_info { + unsigned int uartclk; + uint8_t prescaler; + unsigned int flags; +}; + +struct ni16550_data { + int line; +}; + +static int ni16550_enable_transceivers(struct uart_port *port) +{ + uint8_t pcr; + + pcr = port->serial_in(port, NI16550_PCR_OFFSET); + pcr |= NI16550_PCR_TXVR_ENABLE_BIT; + dev_dbg(port->dev, "enable transceivers: write pcr: 0x%02x\n", pcr); + port->serial_out(port, NI16550_PCR_OFFSET, pcr); + + return 0; +} + +static int ni16550_disable_transceivers(struct uart_port *port) +{ + uint8_t pcr; + + pcr = port->serial_in(port, NI16550_PCR_OFFSET); + pcr &= ~NI16550_PCR_TXVR_ENABLE_BIT; + dev_dbg(port->dev, "disable transceivers: write pcr: 0x%02x\n", pcr); + port->serial_out(port, NI16550_PCR_OFFSET, pcr); + + return 0; +} + +static int ni16550_rs485_config(struct uart_port *port, + struct ktermios *termios, + struct serial_rs485 *rs485) +{ + uint8_t pcr; + struct uart_8250_port *up = container_of(port, struct uart_8250_port, + port); + + /* "rs485" should be given to us non-NULL. */ + if (WARN_ON(rs485 == NULL)) + return -EINVAL; + + pcr = serial_in(up, NI16550_PCR_OFFSET); + pcr &= ~NI16550_PCR_WIRE_MODE_MASK; + + if (rs485->flags & SER_RS485_ENABLED) { + /* RS-485 */ + dev_vdbg(port->dev, "2-wire Auto\n"); + pcr |= NI16550_PCR_AUTO_RS485; + up->acr |= NI16550_ACR_AUTO_DTR_EN; + } else { + /* RS-422 */ + dev_vdbg(port->dev, "4-wire\n"); + pcr |= NI16550_PCR_RS422; + up->acr &= ~NI16550_ACR_AUTO_DTR_EN; + } + + dev_dbg(port->dev, "config rs485: write pcr: 0x%02x, acr: %02x\n", pcr, up->acr); + serial_out(up, NI16550_PCR_OFFSET, pcr); + serial_icr_write(up, UART_ACR, up->acr); + + return 0; +} + +static bool is_pmr_rs232_mode(struct uart_8250_port *up) +{ + uint8_t pmr = serial_in(up, NI16550_PMR_OFFSET); + + /* + * If the PMR is not implemented, then by default NI UARTs are + * connected to RS-485 transceivers + */ + if ((pmr & NI16550_PMR_CAP_MASK) == NI16550_PMR_NOT_IMPL) + return false; + + if ((pmr & NI16550_PMR_CAP_MASK) == NI16550_PMR_CAP_DUAL) + /* + * If the port is dual-mode capable, then read the mode bit + * to know the current mode + */ + return ((pmr & NI16550_PMR_MODE_MASK) + == NI16550_PMR_MODE_RS232); + else + /* + * If it is not dual-mode capable, then decide based on the + * capability + */ + return ((pmr & NI16550_PMR_CAP_MASK) == NI16550_PMR_CAP_RS232); +} + +static void ni16550_config_prescaler(struct uart_8250_port *up, + uint8_t prescaler) +{ + /* + * Page in the Enhanced Mode Registers + * Sets EFR[4] for Enhanced Mode. + */ + uint8_t lcr_value; + uint8_t efr_value; + + lcr_value = serial_in(up, UART_LCR); + serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); + + efr_value = serial_in(up, UART_EFR); + efr_value |= UART_EFR_ECB; + + serial_out(up, UART_EFR, efr_value); + + /* Page out the Enhanced Mode Registers */ + serial_out(up, UART_LCR, lcr_value); + + /* Set prescaler to CPR register. */ + serial_out(up, UART_SCR, UART_CPR); + serial_out(up, UART_ICR, prescaler); +} + +static const struct serial_rs485 ni16550_rs485_supported = { + .flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND, +}; + +static void ni16550_rs485_setup(struct uart_port *port) +{ + port->rs485_config = ni16550_rs485_config; + port->rs485_supported = ni16550_rs485_supported; + /* + * The hardware comes up by default in 2-wire auto mode and we + * set the flags to represent that + */ + port->rs485.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND; +} + +static int ni16550_port_startup(struct uart_port *port) +{ + int ret; + + ret = serial8250_do_startup(port); + if (ret) + return ret; + + return ni16550_enable_transceivers(port); +} + +static void ni16550_port_shutdown(struct uart_port *port) +{ + ni16550_disable_transceivers(port); + + serial8250_do_shutdown(port); +} + +static int ni16550_get_regs(struct platform_device *pdev, + struct uart_port *port) +{ + struct resource *regs; + + regs = platform_get_resource(pdev, IORESOURCE_IO, 0); + if (regs) { + port->iotype = UPIO_PORT; + port->iobase = regs->start; + + return 0; + } + + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (regs) { + port->iotype = UPIO_MEM; + port->mapbase = regs->start; + port->mapsize = resource_size(regs); + port->flags |= UPF_IOREMAP; + + port->membase = devm_ioremap(&pdev->dev, port->mapbase, + port->mapsize); + if (!port->membase) + return -ENOMEM; + + return 0; + } + + dev_err(&pdev->dev, "no registers defined\n"); + return -EINVAL; +} + +static int ni16550_read_fifo_size(struct uart_8250_port *uart, int reg) +{ + /* + * Very old implementations don't have the TFS or RFS registers + * defined, so we may read all-0s or all-1s. For such devices, + * assume a FIFO size of 128. + */ + int value = serial_in(uart, reg); + + if (value == 0x00 || value == 0xFF) + return 128; + + return value; +} + +static void ni16550_set_mctrl(struct uart_port *port, unsigned int mctrl) +{ + mctrl |= UART_MCR_CLKSEL; + + serial8250_do_set_mctrl(port, mctrl); +} + +static int ni16550_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct uart_8250_port uart = {}; + struct ni16550_data *data; + const struct ni16550_device_info *info; + int ret = 0; + int irq; + int rs232_property = 0; + unsigned int prescaler; + const char *transceiver; + int txfifosz, rxfifosz; + + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + spin_lock_init(&uart.port.lock); + + irq = platform_get_irq(pdev, 0); + if (irq < 0) + return irq; + + ret = ni16550_get_regs(pdev, &uart.port); + if (ret < 0) + return ret; + + /* early setup so that serial_in()/serial_out() work */ + serial8250_set_defaults(&uart); + + info = device_get_match_data(dev); + + uart.port.dev = dev; + uart.port.irq = irq; + uart.port.irqflags = IRQF_SHARED; + uart.port.flags |= UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF + | UPF_FIXED_PORT | UPF_FIXED_TYPE; + uart.port.startup = ni16550_port_startup; + uart.port.shutdown = ni16550_port_shutdown; + + /* + * Hardware instantiation of FIFO sizes are held in registers. + */ + txfifosz = ni16550_read_fifo_size(&uart, NI16550_TFS_OFFSET); + rxfifosz = ni16550_read_fifo_size(&uart, NI16550_RFS_OFFSET); + + dev_dbg(dev, "NI 16550 has TX FIFO size %d, RX FIFO size %d\n", + txfifosz, rxfifosz); + + uart.port.type = PORT_16550A; + uart.port.fifosize = txfifosz; + uart.tx_loadsz = txfifosz; + uart.fcr = UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10; + uart.capabilities = UART_CAP_FIFO | UART_CAP_AFE | UART_CAP_EFR; + + /* + * OF device-tree and NIC7A69 ACPI can declare clock-frequency, + * but may be missing for other instantiations, so this is optional. + * If present, override what we've defined in this driver. + */ + if (info->uartclk) + uart.port.uartclk = info->uartclk; + device_property_read_u32(dev, "clock-frequency", &uart.port.uartclk); + if (!uart.port.uartclk) { + dev_err(dev, "unable to determine clock frequency!\n"); + return -ENODEV; + } + + if (info->prescaler) + prescaler = info->prescaler; + device_property_read_u32(dev, "clock-prescaler", &prescaler); + + if (prescaler != 0) { + uart.port.set_mctrl = ni16550_set_mctrl; + ni16550_config_prescaler(&uart, (uint8_t)prescaler); + } + + /* + * The determination of whether or not this is an RS-485 or RS-232 port + * can come from the "transceiver" device property (if present), or it + * can come from the PMR (if it is present), and otherwise we're solely + * an RS-485 port. + */ + if (!device_property_read_string(dev, "transceiver", &transceiver)) { + rs232_property = strncmp(transceiver, "RS-232", 6) == 0; + + dev_dbg(dev, "port is in %s mode (via device property)", + (rs232_property ? "RS-232" : "RS-485")); + } else if (info->flags & NI_HAS_PMR) { + rs232_property = is_pmr_rs232_mode(&uart); + + dev_dbg(dev, "port is in %s mode (via PMR)", + (rs232_property ? "RS-232" : "RS-485")); + } else { + rs232_property = 0; + + dev_dbg(dev, "port is fixed as RS-485"); + } + + if (!rs232_property) { + /* + * Neither the 'transceiver' property nor the PMR indicate + * that this is an RS-232 port, so it must be an RS-485 one. + */ + ni16550_rs485_setup(&uart.port); + } + + ret = serial8250_register_8250_port(&uart); + if (ret < 0) + return ret; + + data->line = ret; + + platform_set_drvdata(pdev, data); + + return 0; +} + +static int ni16550_remove(struct platform_device *pdev) +{ + struct ni16550_data *data = platform_get_drvdata(pdev); + + serial8250_unregister_port(data->line); + return 0; +} + +static const struct ni16550_device_info ni16550_default = { }; + +static const struct of_device_id ni16550_of_match[] = { + { .compatible = "ni,ni16550", .data = &ni16550_default }, + { }, +}; +MODULE_DEVICE_TABLE(of, ni16550_of_match); + +/* NI 16550 RS-485 Interface */ +static const struct ni16550_device_info nic7750 = { + .uartclk = 33333333, +}; + +/* NI CVS-145x RS-485 Interface */ +static const struct ni16550_device_info nic7772 = { + .uartclk = 1843200, + .flags = NI_HAS_PMR, +}; + +/* NI cRIO-904x RS-485 Interface */ +static const struct ni16550_device_info nic792b = { + /* Sets UART clock rate to 22.222 MHz with 1.125 prescale */ + .uartclk = 25000000, + .prescaler = 0x09, +}; + +/* NI sbRIO 96x8 RS-232/485 Interfaces */ +static const struct ni16550_device_info nic7a69 = { + /* Set UART clock rate to 29.629 MHz with 1.125 prescale */ + .uartclk = 29629629, + .prescaler = 0x09, +}; + +static const struct acpi_device_id ni16550_acpi_match[] = { + { "NIC7750", (kernel_ulong_t)&nic7750 }, + { "NIC7772", (kernel_ulong_t)&nic7772 }, + { "NIC792B", (kernel_ulong_t)&nic792b }, + { "NIC7A69", (kernel_ulong_t)&nic7a69 }, + { }, +}; +MODULE_DEVICE_TABLE(acpi, ni16550_acpi_match); + +static struct platform_driver ni16550_driver = { + .driver = { + .name = "ni16550", + .of_match_table = ni16550_of_match, + .acpi_match_table = ACPI_PTR(ni16550_acpi_match), + }, + .probe = ni16550_probe, + .remove = ni16550_remove, +}; + +module_platform_driver(ni16550_driver); + +MODULE_AUTHOR("Jaeden Amero <jaeden.amero@ni.com>"); +MODULE_AUTHOR("Karthik Manamcheri <karthik.manamcheri@ni.com>"); +MODULE_DESCRIPTION("NI 16550 Driver"); +MODULE_LICENSE("GPL"); diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig index 5313aa31930f..c0f3aec51d13 100644 --- a/drivers/tty/serial/8250/Kconfig +++ b/drivers/tty/serial/8250/Kconfig @@ -565,6 +565,15 @@ config SERIAL_8250_BCM7271 including DMA support and high accuracy BAUD rates, say Y to this option. If unsure, say N. +config SERIAL_8250_NI + tristate "NI 16550 based serial port" + depends on SERIAL_8250 + help + This driver supports the integrated serial ports on National + Instruments (NI) controller hardware. This is required for all NI + controller models with onboard RS-485 or dual-mode RS-485/RS-232 + ports. + config SERIAL_OF_PLATFORM tristate "Devicetree based probing for 8250 ports" depends on SERIAL_8250 && OF diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile index 4fc2fc1f41b6..58dc1b5ff054 100644 --- a/drivers/tty/serial/8250/Makefile +++ b/drivers/tty/serial/8250/Makefile @@ -45,6 +45,7 @@ obj-$(CONFIG_SERIAL_8250_PERICOM) += 8250_pericom.o obj-$(CONFIG_SERIAL_8250_PXA) += 8250_pxa.o obj-$(CONFIG_SERIAL_8250_TEGRA) += 8250_tegra.o obj-$(CONFIG_SERIAL_8250_BCM7271) += 8250_bcm7271.o +obj-$(CONFIG_SERIAL_8250_NI) += 8250_ni.o obj-$(CONFIG_SERIAL_OF_PLATFORM) += 8250_of.o CFLAGS_8250_ingenic.o += -I$(srctree)/scripts/dtc/libfdt
The National Instruments (NI) 16550 is a 16550-like UART with larger FIFOs and embedded RS-232/RS-485 transceiver control circuitry. This patch adds a driver that can operate this UART, which is used for onboard serial ports in several NI embedded controller designs. Portions of this driver were originally written by Jaeden Amero and Karthik Manamcheri, with extensive cleanups and refactors since. Signed-off-by: Brenda Streiff <brenda.streiff@ni.com> Cc: Gratian Crisan <gratian.crisan@ni.com> Cc: Jason Smith <jason.smith@ni.com> --- MAINTAINERS | 7 + drivers/tty/serial/8250/8250_ni.c | 447 ++++++++++++++++++++++++++++++ drivers/tty/serial/8250/Kconfig | 9 + drivers/tty/serial/8250/Makefile | 1 + 4 files changed, 464 insertions(+) create mode 100644 drivers/tty/serial/8250/8250_ni.c