On Tue, 23 Jan 2024, Frank Li wrote: > In typical embedded Linux systems, UART consoles require at least two pins, > TX and RX. In scenarios where I2C/I3C devices like sensors or PMICs are > present, we can save these two pins by using this driver. Pins is crucial > resources, especially in small chip packages. > > This introduces support for using the I3C bus to transfer console tty data, > effectively replacing the need for dedicated UART pins. This not only > conserves valuable pin resources but also facilitates testing of I3C's > advanced features, including early termination, in-band interrupt (IBI) > support, and the creation of more complex data patterns. Additionally, > it aids in identifying and addressing issues within the I3C controller > driver. > > Signed-off-by: Frank Li <Frank.Li@xxxxxxx> > --- > > Notes: > Notes: > Version number use i3c target patches. > Change from v3 to v4 > - add static at i3c_remove() > Change v2 > - using system_unbound_wq working queue > - fixed accoring to Jiri Slaby's comments > > Change before send with i3c target support > > Change from v4 to v5 > - send in i3c improvememtn patches. > > Change from v2 to v4 > - none > > Change from v1 to v2 > - update commit message. > - using goto for err handle > - using one working queue for all tty-i3c device > - fixed typo found by js > - update kconfig help > - using kfifo > > Still below items not be fixed (according to Jiri Slaby's comments) > - rxwork thread: need trigger from two position. > - common thread queue: need some suggestion > > drivers/tty/Kconfig | 13 ++ > drivers/tty/Makefile | 1 + > drivers/tty/i3c_tty.c | 426 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 440 insertions(+) > create mode 100644 drivers/tty/i3c_tty.c > > diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig > index 5646dc6242cd9..9ab4cd480e9f8 100644 > --- a/drivers/tty/Kconfig > +++ b/drivers/tty/Kconfig > @@ -412,6 +412,19 @@ config RPMSG_TTY > To compile this driver as a module, choose M here: the module will be > called rpmsg_tty. > > +config I3C_TTY > + tristate "TTY over I3C" > + depends on I3C > + help > + Select this option to use TTY over I3C master controller. > + > + This makes it possible for user-space programs to send and receive > + data as a standard tty protocol. I3C provide relatively higher data > + transfer rate and less pin numbers, SDA/SCL are shared with other > + devices. > + > + If unsure, say N > + > endif # TTY > > source "drivers/tty/serdev/Kconfig" > diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile > index 07aca5184a55d..f329f9c7d308a 100644 > --- a/drivers/tty/Makefile > +++ b/drivers/tty/Makefile > @@ -27,5 +27,6 @@ obj-$(CONFIG_GOLDFISH_TTY) += goldfish.o > obj-$(CONFIG_MIPS_EJTAG_FDC_TTY) += mips_ejtag_fdc.o > obj-$(CONFIG_VCC) += vcc.o > obj-$(CONFIG_RPMSG_TTY) += rpmsg_tty.o > +obj-$(CONFIG_I3C_TTY) += i3c_tty.o > > obj-y += ipwireless/ > diff --git a/drivers/tty/i3c_tty.c b/drivers/tty/i3c_tty.c > new file mode 100644 > index 0000000000000..8f4e87dfa01cd > --- /dev/null > +++ b/drivers/tty/i3c_tty.c > @@ -0,0 +1,426 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright 2023 NXP. > + * > + * Author: Frank Li <Frank.Li@xxxxxxx> > + */ > + > +#include <linux/delay.h> > +#include <linux/kernel.h> > +#include <linux/mod_devicetable.h> > +#include <linux/module.h> > +#include <linux/i3c/device.h> > +#include <linux/i3c/master.h> > +#include <linux/slab.h> > +#include <linux/console.h> > +#include <linux/serial_core.h> > +#include <linux/interrupt.h> Do you need this header. > +#include <linux/workqueue.h> > +#include <linux/tty_flip.h> > + > +static DEFINE_IDR(i3c_tty_minors); > +static DEFINE_MUTEX(i3c_tty_minors_lock); > + > +static struct tty_driver *i3c_tty_driver; > + > +#define I3C_TTY_MINORS 8 > +#define I3C_TTY_TRANS_SIZE 16 > +#define I3C_TTY_RX_STOP 0 > +#define I3C_TTY_RETRY 20 > +#define I3C_TTY_YIELD_US 100 > + > +struct ttyi3c_port { > + struct tty_port port; Missing #include > + int minor; > + spinlock_t xlock; /* protect xmit */ Missing #include > + u8 tx_buff[I3C_TTY_TRANS_SIZE]; > + u8 rx_buff[I3C_TTY_TRANS_SIZE]; > + struct i3c_device *i3cdev; > + struct work_struct txwork; > + struct work_struct rxwork; > + struct completion txcomplete; Missing #include > + unsigned long status; > + u32 buf_overrun; > +}; > +static void i3c_port_shutdown(struct tty_port *port) > +{ > + struct ttyi3c_port *sport = > + container_of(port, struct ttyi3c_port, port); On one line. > + > + i3c_device_disable_ibi(sport->i3cdev); > + tty_port_free_xmit_buf(port); > +} > + > +static void i3c_port_destruct(struct tty_port *port) > +{ > + struct ttyi3c_port *sport = > + container_of(port, struct ttyi3c_port, port); Ditto. > +static void tty_i3c_rxwork(struct work_struct *work) > +{ > + struct ttyi3c_port *sport = container_of(work, struct ttyi3c_port, rxwork); > + struct i3c_priv_xfer xfers; > + u32 retry = I3C_TTY_RETRY; > + u16 status = BIT(0); Unnecessary initialization. > + int ret; > + > + memset(&xfers, 0, sizeof(xfers)); > + xfers.data.in = sport->rx_buff; > + xfers.len = I3C_TTY_TRANS_SIZE; > + xfers.rnw = 1; > + > + do { > + if (test_bit(I3C_TTY_RX_STOP, &sport->status)) > + break; > + > + i3c_device_do_priv_xfers(sport->i3cdev, &xfers, 1); > + > + if (xfers.actual_len) { > + ret = tty_insert_flip_string(&sport->port, sport->rx_buff, > + xfers.actual_len); > + if (ret < xfers.actual_len) > + sport->buf_overrun++; > + > + retry = I3C_TTY_RETRY; > + continue; > + } > + > + status = BIT(0); Can this BIT(0) be named with a #define? > + i3c_device_getstatus_format1(sport->i3cdev, &status); > + /* > + * Target side needs some time to fill data into fifo. Target side may not > + * have hardware update status in real time. Software update status always > + * needs some delays. > + * > + * Generally, target side have circular buffer in memory, it will be moved > + * into FIFO by CPU or DMA. 'status' just show if circular buffer empty. But > + * there are gap, especially CPU have not response irq to fill FIFO in time. > + * So xfers.actual will be zero, wait for little time to avoid flood > + * transfer in i3c bus. > + */ > + usleep_range(I3C_TTY_YIELD_US, 10 * I3C_TTY_YIELD_US); > + retry--; > + > + } while (retry && (status & BIT(0))); Name with define? -- i.