On 23. 10. 23, 23:04, 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.
...
--- /dev/null +++ b/drivers/tty/serial/8250/8250_ni.c
...
+static int ni16550_probe(struct platform_device *pdev) +{ + const struct ni16550_device_info *info; + struct device *dev = &pdev->dev; + struct uart_8250_port uart = {}; + unsigned int prescaler = 0; + struct ni16550_data *data; + const char *portmode; + int txfifosz, rxfifosz;
Why not unsigned?
+ int rs232_property;
bool
+ int ret; + int irq; + + 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; + + /* + * Declaration of the base clock frequency can come from one of: + * - static declaration in this driver (for older ACPI IDs) + * - a "clock-frquency" ACPI or OF device property + * - an associated OF clock definition + */ + if (info->uartclk) + uart.port.uartclk = info->uartclk; + if (device_property_read_u32(dev, "clock-frequency", + &uart.port.uartclk)) { + data->clk = devm_clk_get_optional_enabled(dev, NULL); + if (data->clk)
!IS_ERR(data->clk)
+ uart.port.uartclk = clk_get_rate(data->clk); + } + + if (!uart.port.uartclk) { + dev_err(dev, "unable to determine clock frequency!\n"); + ret = -ENODEV; + goto err; + } + + 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, (u8)prescaler); + } + + /* + * The determination of whether or not this is an RS-485 or RS-232 port + * can come from a device property (if present), or it can come from + * the PMR (if present), and otherwise we're solely an RS-485 port. + * + * This is a device-specific property, and thus has a vendor-prefixed + * "ni,serial-port-mode" form as a devicetree binding. However, there + * are old devices in the field using "transceiver" as an ACPI device + * property, so we have to check for that as well. + */ + if (!device_property_read_string(dev, "ni,serial-port-mode", &portmode) || + !device_property_read_string(dev, "transceiver", &portmode)) { + rs232_property = strncmp(portmode, "RS-232", 6) == 0; + + dev_dbg(dev, "port is in %s mode (via device property)",
\n missing here and there.
+ 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) + goto err; + data->line = ret; + + platform_set_drvdata(pdev, data); + return 0; + +err: + clk_disable_unprepare(data->clk);
But you use devm_?
+ return ret; +} + +static int ni16550_remove(struct platform_device *pdev) +{ + struct ni16550_data *data = platform_get_drvdata(pdev); + + clk_disable_unprepare(data->clk); + serial8250_unregister_port(data->line);
This should be likely inverted. But I don't know if it matters here at all. Actually, devm_ again, so the first is unnecessary?
+ return 0; +}
...
+MODULE_AUTHOR("Jaeden Amero <jaeden.amero@xxxxxx>"); +MODULE_AUTHOR("Karthik Manamcheri <karthik.manamcheri@xxxxxx>");
Do the addresses work? Why not CC them? thanks, -- js suse labs