On Tue, Jan 05, 2021 at 10:29:25AM +0000, József Horváth wrote: > This is a serial port driver for > Silicon Labs Si4455 Sub-GHz transciver. > > The goal of this driver is to removing wires > between central(linux) device and remote serial devices/sensors, > but keeping the original user software. > It represents regular serial interface for the user space. > > Datasheet: https://www.silabs.com/documents/public/data-sheets/Si4455.pdf > > Guide: https://github.com/dministro/linux-serial-si4455 > > Signed-off-by: Jozsef Horvath <info@xxxxxxxxxxx> > --- > > changes v1: > - fixed: drivers: serial: si4455: coding style > - fixed: drivers: serial: si4455: error checking and order > - fixed: drivers: serial: si4455: remove unnecessary compatibility > strings from si4455_dt_ids > > changes v2: > - fixed: drivers: serial: si4455: coding style > > changes v3: > - fixed: drivers: serial: si4455: coding style > - fixed: drivers: serial: si4455: replace device configuration procedure > (SI4455_IOC_SEZC ioctl call) with request_firmware(...). > The firmware name comes from dt (silabs,ez-config) > - fixed: drivers: serial: si4455: replace transmit/receive channel > select (SI4455_IOC_STXC/SI4455_IOC_SRXC ioctl calls) > with sysfs entries (tx_channel, rx_channel). > Initial values comes from dt (silabs,tx-channel and silabs,rx-channel) > - fixed: drivers: serial: si4455: replace package size setting > (SI4455_IOC_SSIZ ioctl call) with sysfs entry (package_size). > Initial value comes from dt (silabs,package-size) > - fixed: drivers: serial: si4455: replace getting last rssi > (SI4455_IOC_GRSSI ioctl call) with sysfs entry (current_rssi) > - fixed: drivers: serial: si4455: remove si4455_api.h > and custom ioctl definitions > > changes v5: > - fixed: drivers: serial: si4455: coding style > - fixed: drivers: serial: si4455: remove struct si4455_one, > members moved to struct si4455_port > - fixed: drivers: serial: si4455: fix line endings in dev_err and > dev_dbg messages > - fixed: drivers: serial: si4455: remove unnecessary else { ... } > - fixed: drivers: serial: si4455: refactor si4455_do_work(...), > xmit circular buffer handling and start tx moved to > si4455_start_tx_xmit(...) > - fixed: drivers: serial: si4455: refactor si4455_configure > - fixed: drivers: serial: si4455: refactor interrupt handling, > remove unnecessary wrapper > - fixed: drivers: serial: si4455: modem line(si4455_get_mctrl) > and tx buffer status(si4455_tx_empty) conditions and signaling > - fixed: drivers: serial: si4455: remove unsafe int to pointer conversion > > changes v6: > - fixed: drivers: serial: si4455: illegal characters in > MODULE_AUTHOR("...") Reported-by: kernel test robot <lkp@xxxxxxxxx> > > changes v7: > - added: drivers: serial: si4455: variable packet length support > - added: drivers: serial: si4455: extended error handling > - added: dt: bindings: silabs,si4455: silabs,tx-timeout property definition > --- > MAINTAINERS | 6 + > drivers/tty/serial/Kconfig | 8 + > drivers/tty/serial/Makefile | 1 + > drivers/tty/serial/si4455.c | 1721 +++++++++++++++++++++++++++++++++++ > 4 files changed, 1736 insertions(+) > create mode 100644 drivers/tty/serial/si4455.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index a008b70f3c16..ca6e32693a74 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -15952,6 +15952,12 @@ M: Jérôme Pouiller <jerome.pouiller@xxxxxxxxxx> > S: Supported > F: drivers/staging/wfx/ > > +SILICON LABS SI4455 SERIAL DRIVER > +M: Jozsef Horvath <info@xxxxxxxxxxx> > +S: Maintained > +F: Documentation/devicetree/bindings/serial/silabs,si4455.yaml > +F: drivers/tty/serial/si4455.c > + > SILICON MOTION SM712 FRAME BUFFER DRIVER > M: Sudip Mukherjee <sudipm.mukherjee@xxxxxxxxx> > M: Teddy Wang <teddy.wang@xxxxxxxxxxxxxxxxx> > diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig > index 28f22e58639c..560aa311cd03 100644 > --- a/drivers/tty/serial/Kconfig > +++ b/drivers/tty/serial/Kconfig > @@ -1583,6 +1583,14 @@ config SERIAL_MILBEAUT_USIO_CONSOLE > receives all kernel messages and warnings and which allows logins in > single user mode). > > +config SERIAL_SI4455 > + tristate "Si4455 support" > + depends on SPI > + select SERIAL_CORE > + help > + This driver is for Silicon Labs's Si4455 Sub-GHz transciver. > + Say 'Y' here if you wish to use it as serial port. > + No module name? > endmenu > > config SERIAL_MCTRL_GPIO > diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile > index caf167f0c10a..f01ff43db1d6 100644 > --- a/drivers/tty/serial/Makefile > +++ b/drivers/tty/serial/Makefile > @@ -89,6 +89,7 @@ obj-$(CONFIG_SERIAL_MPS2_UART) += mps2-uart.o > obj-$(CONFIG_SERIAL_OWL) += owl-uart.o > obj-$(CONFIG_SERIAL_RDA) += rda-uart.o > obj-$(CONFIG_SERIAL_MILBEAUT_USIO) += milbeaut_usio.o > +obj-$(CONFIG_SERIAL_SI4455) += si4455.o > obj-$(CONFIG_SERIAL_SIFIVE) += sifive.o > > # GPIOLIB helpers for modem control lines > diff --git a/drivers/tty/serial/si4455.c b/drivers/tty/serial/si4455.c > new file mode 100644 > index 000000000000..615c783b7aa2 > --- /dev/null > +++ b/drivers/tty/serial/si4455.c > @@ -0,0 +1,1721 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2020 József Horváth <info@xxxxxxxxxxx> > + * > + */ > +#include <linux/bitops.h> > +#include <linux/clk.h> > +#include <linux/delay.h> > +#include <linux/device.h> > +#include <linux/gpio/driver.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/of_gpio.h> > +#include <linux/regmap.h> > +#include <linux/serial_core.h> > +#include <linux/serial.h> > +#include <linux/tty.h> > +#include <linux/tty_flip.h> > +#include <linux/spi/spi.h> > +#include <linux/uaccess.h> > +#include <linux/string.h> > +#include <linux/firmware.h> > +#include <linux/timer.h> > +#ifdef CONFIG_DEBUG_FS > +#include <linux/debugfs.h> > +#endif No need for #ifdef for .h files. > + > +#define PORT_SI4455 1096 > +#define SI4455_NAME "Si4455" > +#define SI4455_MAJOR 432 > +#define SI4455_MINOR 567 Where are these major/minor numbers being used and where did they come from? Why do you need them? > +struct si4455_port { > + struct uart_port port; > +#ifdef CONFIG_DEBUG_FS > + struct dentry *dbgfs_dir; > +#endif Do not put #ifdefs in .c code, you never need to check for this type of thing. > +static struct uart_driver si4455_uart = { > + .owner = THIS_MODULE, > + .driver_name = SI4455_NAME, > +#ifdef CONFIG_DEVFS_FS > + .dev_name = "ttySI%d", Looks like you are porting this from a _VERY_ old kernel. That config option went away 15+ years ago. Are you sure this works? > +#else > + .dev_name = "ttySI", Where did you get that name from? > +static int si4455_begin_tx(struct uart_port *port, u32 channel, int length, > + u8 *data) > +{ > + int ret = 0; > + struct si4455_int_status int_status = { 0 }; > + struct si4455_fifo_info fifo_info = { 0 }; > + > + dev_dbg(port->dev, "%s(%u, %u)\n", __func__, channel, length); No need for these types of debugging lines, just use ftrace. Please remove them, you have them in a few places (same for the end of functions.) > +static void si4455_null_void(struct uart_port *port) > +{ > + /* Do nothing */ Why do you need this??? > +#ifdef CONFIG_DEBUG_FS Again, no #ifdef needed. > +static int si4455_debugfs_init(struct device *dev) > +{ > + struct si4455_port *s = dev_get_drvdata(dev); > + struct dentry *dbgfs_si_dir; > + struct dentry *dbgfs_partinfo_dir; > + struct dentry *dbgfs_entry; > + > + s->dbgfs_dir = debugfs_create_dir(dev_name(dev), NULL); > + if (IS_ERR(s->dbgfs_dir)) > + return PTR_ERR(s->dbgfs_dir); No need to check any debugfs return value, just use it and move on. > + > + dbgfs_si_dir = debugfs_create_dir("si4455", s->dbgfs_dir); > + if (IS_ERR(dbgfs_si_dir)) > + return PTR_ERR(dbgfs_si_dir); > + > + dbgfs_entry = debugfs_create_u32("cts_error_count", 0444, > + dbgfs_si_dir, &s->cts_error_count); > + if (IS_ERR(dbgfs_entry)) > + return PTR_ERR(dbgfs_entry); Same for all of these, no need to check anything. > + > + dbgfs_entry = debugfs_create_u32("tx_error_count", 0444, > + dbgfs_si_dir, &s->tx_error_count); > + if (IS_ERR(dbgfs_entry)) > + return PTR_ERR(dbgfs_entry); > + > + dbgfs_partinfo_dir = debugfs_create_dir("partinfo", dbgfs_si_dir); > + if (IS_ERR(dbgfs_partinfo_dir)) > + return PTR_ERR(dbgfs_partinfo_dir); > + > + dbgfs_entry = debugfs_create_u8("chip_rev", 0444, > + dbgfs_partinfo_dir, > + &s->part_info.chip_rev); Wait, did you even build this code? Does it work? It shouldn't, these debugfs calls have changed... I'm stopping reviewing here. thanks, greg k-h
