On 17/12/2023 13:20, Christoph Winklhofer wrote: > Add a UART 1-wire bus driver. The driver utilizes the UART interface via > the Serial Device Bus to create the 1-wire timing patterns. The driver > was tested on a "Raspberry Pi 3B" with a DS18B20 and on a "Variscite > DART-6UL" with a DS18S20 temperature sensor. > > The 1-wire timing pattern and the corresponding UART baud-rate with the > interpretation of the transferred bytes are described in the document: > > Link: https://www.analog.com/en/technical-articles/using-a-uart-to-implement-a-1wire-bus-master.html > > In short, the UART peripheral must support full-duplex and operate in > open-drain mode. The timing patterns are generated by a specific > combination of baud-rate and transmitted byte, which corresponds to a > 1-Wire read bit, write bit or reset. > > Signed-off-by: Christoph Winklhofer <cj.winklhofer@xxxxxxxxx> > --- > Documentation/w1/masters/index.rst | 1 + > Documentation/w1/masters/w1-uart.rst | 48 +++++++ > drivers/w1/masters/Kconfig | 10 ++ > drivers/w1/masters/Makefile | 1 + > drivers/w1/masters/w1-uart.c | 189 +++++++++++++++++++++++++++ > 5 files changed, 249 insertions(+) > create mode 100644 Documentation/w1/masters/w1-uart.rst > create mode 100644 drivers/w1/masters/w1-uart.c > > diff --git a/Documentation/w1/masters/index.rst b/Documentation/w1/masters/index.rst > index 4442a98850ad..cc40189909fd 100644 > --- a/Documentation/w1/masters/index.rst > +++ b/Documentation/w1/masters/index.rst > @@ -12,3 +12,4 @@ > mxc-w1 > omap-hdq > w1-gpio > + w1-uart > diff --git a/Documentation/w1/masters/w1-uart.rst b/Documentation/w1/masters/w1-uart.rst > new file mode 100644 > index 000000000000..0f546cf77f13 > --- /dev/null > +++ b/Documentation/w1/masters/w1-uart.rst > @@ -0,0 +1,48 @@ > +.. SPDX-License-Identifier: GPL-2.0-or-later > + > +===================== > +Kernel driver w1-uart > +===================== > + > +Author: Christoph Winklhofer <cj.winklhofer@xxxxxxxxx> > + > + > +Description > +----------- > + > +UART 1-wire bus driver. The driver utilizes the UART interface via the > +Serial Device Bus to create the 1-Wire timing patterns as described in > +the document `"Using a UART to Implement a 1-Wire Bus Master"`_. > + > +.. _"Using a UART to Implement a 1-Wire Bus Master": https://www.analog.com/en/technical-articles/using-a-uart-to-implement-a-1wire-bus-master.html > + > +In short, the UART peripheral must support full-duplex and operate in > +open-drain mode. The timing patterns are generated by a specific > +combination of baud-rate and transmitted byte, which corresponds to a > +1-Wire read bit, write bit or reset pulse. > + > +For instance the timing pattern for a 1-Wire reset and presence detect uses > +the baud-rate 9600, i.e. 104.2 us per bit. The transmitted byte 0xf0 over > +UART (least significant bit first, start-bit low) sets the reset low time > +for 1-Wire to 521 us. A present 1-Wire device changes the received byte by > +pulling the line low, which is used by the driver to evaluate the result of > +the 1-Wire operation. > + > +Similar for a 1-Wire read bit or write bit, which uses the baud-rate > +115200, i.e. 8.7 us per bit. The transmitted byte 0x00 is used for a > +Write-0 operation and the byte 0xff for Read-0, Read-1 and Write-1. > + > + > +Usage > +----- > + > +Specify the UART 1-wire bus in the device tree by adding the single child > +onewire to the serial node (e.g. uart0). For example: > +:: > + > + @uart0 { > + ... > + onewire { > + compatible = "w1-uart"; > + }; > + }; > diff --git a/drivers/w1/masters/Kconfig b/drivers/w1/masters/Kconfig > index 513c0b114337..e6049a75b35b 100644 > --- a/drivers/w1/masters/Kconfig > +++ b/drivers/w1/masters/Kconfig > @@ -78,5 +78,15 @@ config W1_MASTER_SGI > This support is also available as a module. If so, the module > will be called sgi_w1. > > +config W1_MASTER_UART > + tristate "UART 1-wire driver" > + depends on SERIAL_DEV_BUS > + help > + Say Y here if you want to communicate with your 1-wire devices using > + UART interface. > + > + This support is also available as a module. If so, the module > + will be called w1-uart. > + > endmenu > > diff --git a/drivers/w1/masters/Makefile b/drivers/w1/masters/Makefile > index 6c5a21f9b88c..227f80987e69 100644 > --- a/drivers/w1/masters/Makefile > +++ b/drivers/w1/masters/Makefile > @@ -12,3 +12,4 @@ obj-$(CONFIG_W1_MASTER_MXC) += mxc_w1.o > obj-$(CONFIG_W1_MASTER_GPIO) += w1-gpio.o > obj-$(CONFIG_HDQ_MASTER_OMAP) += omap_hdq.o > obj-$(CONFIG_W1_MASTER_SGI) += sgi_w1.o > +obj-$(CONFIG_W1_MASTER_UART) += w1-uart.o > diff --git a/drivers/w1/masters/w1-uart.c b/drivers/w1/masters/w1-uart.c > new file mode 100644 > index 000000000000..e0da8ebfef58 > --- /dev/null > +++ b/drivers/w1/masters/w1-uart.c > @@ -0,0 +1,189 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * w1-uart - UART 1-Wire bus driver > + * > + * Uses the UART interface (via Serial Device Bus) to create the 1-Wire > + * timing patterns. Implements the following 1-Wire master interface: > + * > + * - reset_bus: baud-rate 9600 (bit time 104.2 us) and tx-byte 0xf0 > + * > + * - touch_bit: baud-rate 115200 (bit time 8.7 us) and tx-byte 0x00 > + * (Write-0) or 0xff (Read-0, Read-1, Write-1) > + * > + * Author: Christoph Winklhofer <cj.winklhofer@xxxxxxxxx> > + */ > + > +#include <linux/atomic.h> > +#include <linux/completion.h> > +#include <linux/jiffies.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/serdev.h> > +#include <linux/w1.h> > + > +#define W1_UART_TIMEOUT msecs_to_jiffies(500) > + > +struct w1_uart_device { > + struct serdev_device *serdev; > + struct w1_bus_master bus; > + > + struct completion buf_received; > + atomic_t buf; > + > + unsigned int baudrate; > +}; > + > +/* > + * Send one byte (tx_byte) and read one byte (rx_byte) via serdev. > + */ > +static int w1_uart_serdev_tx_rx(struct w1_uart_device *w1dev, > + unsigned int baudrate, unsigned char tx_byte, > + unsigned char *rx_byte) > +{ > + struct serdev_device *serdev = w1dev->serdev; > + struct device *dev = &serdev->dev; > + int ret; > + > + serdev_device_write_flush(serdev); > + if (w1dev->baudrate != baudrate) { > + w1dev->baudrate = serdev_device_set_baudrate(serdev, baudrate); > + if (w1dev->baudrate != baudrate) { > + dev_err(dev, "set baudrate failed\n"); > + return -1; Use errno values when applicable (e.g. for internal parts). > + } > + } > + > + reinit_completion(&w1dev->buf_received); > + ret = serdev_device_write_buf(serdev, &tx_byte, 1); > + if (ret != 1) { > + dev_err(dev, "write failed\n"); This is going to spam the logs. Check existing usages - nothing prints errors on every write or read. > + return -1; > + } > + ret = wait_for_completion_interruptible_timeout(&w1dev->buf_received, > + W1_UART_TIMEOUT); > + if (ret <= 0) { > + dev_err(dev, "receive failed\n"); Also problem of possible flood. > + return -1; > + } > + > + *rx_byte = (unsigned char)atomic_read(&w1dev->buf); > + return 0; > +} > + > +static int w1_uart_serdev_receive_buf(struct serdev_device *serdev, > + const unsigned char *buf, size_t count) > +{ > + struct w1_uart_device *w1dev = serdev_device_get_drvdata(serdev); > + > + if (count > 0) { > + atomic_set(&w1dev->buf, buf[count - 1]); I fail to understand why you use atomic here. It does not solve any locking problems - it's still there. Load and store tearing? But is it ever possible for one byte stores and loads? Doesn't C standard guarantee that byte writes are atomic? I also do not understand why you store only last byte, ignoring all the rest. > + complete(&w1dev->buf_received); > + } > + > + return count; > +} > + > +static const struct serdev_device_ops w1_uart_serdev_ops = { > + .receive_buf = w1_uart_serdev_receive_buf, > + .write_wakeup = serdev_device_write_wakeup, > +}; > + > +/* > + * 1-wire reset and presence detect: > + * The baud-rate 9600 (104.2 us per bit) and the byte 0xf0 generates a > + * reset low time of 521 us (start-bit low). A present slave will > + * manipulate the received byte by pulling the 1-wire low. > + */ > +static u8 w1_uart_reset_bus(void *data) > +{ > + struct w1_uart_device *w1dev = data; > + unsigned char buf; This is not buffer, just a value. > + int ret; > + > + ret = w1_uart_serdev_tx_rx(w1dev, 9600, 0xf0, &buf); Why the rate is fixed? What if all devices support different baud rates? > + if (ret < 0) > + return -1; > + > + /* Device present (0) or no device (1) */ > + return buf != 0xf0 ? 0 : 1; > +} > + > +/* > + * 1-Wire read and write cycle: > + * The baud-rate 115200 (8.7 us per bit) and the byte 0xff generates the > + * timing pattern for Read-0, Read-1 and Write-1 operation, with a low > + * time of 8.7 us (start-bit low). Only the Read-0 manipulates the > + * received byte, all others left the line untouched. > + */ > +static u8 w1_uart_touch_bit(void *data, u8 bit) > +{ > + struct w1_uart_device *w1dev = data; > + unsigned char buf; One character is not a buffer. > + int ret; > + > + ret = w1_uart_serdev_tx_rx(w1dev, 115200, bit ? 0xff : 0x00, &buf); Why the rate is fixed? What if all devices support different baud rates? > + /* return inactive bus state on error */ > + if (ret < 0) > + return 1; > + > + return buf == 0xff ? 1 : 0; > +} > + > +static int w1_uart_probe(struct serdev_device *serdev) > +{ > + struct device *dev = &serdev->dev; > + struct w1_uart_device *w1dev; > + int ret; > + > + w1dev = devm_kzalloc(dev, sizeof(struct w1_uart_device), GFP_KERNEL); sizeof(*) > + if (!w1dev) > + return -ENOMEM; > + w1dev->bus.data = w1dev; > + w1dev->bus.reset_bus = w1_uart_reset_bus; > + w1dev->bus.touch_bit = w1_uart_touch_bit; > + w1dev->serdev = serdev; > + > + serdev_device_set_drvdata(serdev, w1dev); > + serdev_device_set_client_ops(serdev, &w1_uart_serdev_ops); > + > + init_completion(&w1dev->buf_received); > + > + ret = devm_serdev_device_open(dev, serdev); > + if (ret < 0) > + return ret; > + serdev_device_set_parity(serdev, SERDEV_PARITY_NONE); > + serdev_device_set_flow_control(serdev, false); > + > + return w1_add_master_device(&w1dev->bus); > +} > + > +static void w1_uart_remove(struct serdev_device *serdev) > +{ > + struct w1_uart_device *w1dev = serdev_device_get_drvdata(serdev); > + > + complete(&w1dev->buf_received); This is odd - w1_uart_serdev_tx_rx() will read data on driver removal. You should make sure that on removal any reads do not follow usual path. > + w1_remove_master_device(&w1dev->bus); > +} > + > +#ifdef CONFIG_OF Drop > +static const struct of_device_id w1_uart_of_match[] = { > + { .compatible = "w1-uart" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, w1_uart_of_match); > +#endif > + > +static struct serdev_device_driver w1_uart_driver = { > + .driver = { > + .name = "w1-uart", > + .of_match_table = of_match_ptr(w1_uart_of_match), Drop of_match_ptr > + }, > + .probe = w1_uart_probe, > + .remove = w1_uart_remove, > +}; Best regards, Krzysztof
