On Mon, Jan 08, 2024 at 07:18:31AM +0100, Jiri Slaby wrote: > On 06. 01. 24, 17:02, Christoph Winklhofer via B4 Relay wrote: > > From: Christoph Winklhofer <cj.winklhofer@xxxxxxxxx> > > > > 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. > ... > > --- /dev/null > > +++ b/drivers/w1/masters/w1-uart.c > > @@ -0,0 +1,398 @@ > > +// 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: requests baud-rate 9600 > > + * > > + * - touch_bit: requests baud-rate 115200 > > + * > > + * Author: Christoph Winklhofer <cj.winklhofer@xxxxxxxxx> > > + */ > > + > > +#include <linux/completion.h> > > +#include <linux/delay.h> > > +#include <linux/jiffies.h> > > +#include <linux/module.h> > > +#include <linux/mutex.h> > > +#include <linux/of.h> > > +#include <linux/serdev.h> > > +#include <linux/w1.h> > > + > > +#define W1_UART_TIMEOUT msecs_to_jiffies(500) > > + > > +/* > > + * struct w1_uart_config - configuration for 1-Wire operation > > + * > > + * @baudrate: baud-rate returned from serdev > > + * @delay_us: delay to complete a 1-Wire cycle (in us) > > + * @tx_byte: byte to generate 1-Wire timing pattern > > + */ > > +struct w1_uart_config { > > + unsigned int baudrate; > > + unsigned int delay_us; > > + unsigned char tx_byte; > > If it is a "byte", it should be u8. > will change this and all others to u8. ... > > + > > +static inline unsigned int baud_to_bit_ns(unsigned int baud) > > +{ > > + return 1000000000 / baud; > > NSEC_PER_SEC > > > +} > > + > > +static inline unsigned int to_ns(unsigned int us) > > +{ > > + return us * 1000; > > NSEC_PER_USEC > and use the correct constants. ... > > +} > > + > > +/* > > + * Set baud-rate, delay and tx-byte to create a 1-Wire pulse and adapt > > + * the tx-byte according to the actual baud-rate. > > + * > > + * Reject when: > > + * - time for a bit outside min/max range > > + * - a 1-Wire response is not detectable for sent byte > > + */ > > +static int w1_uart_set_config(struct serdev_device *serdev, > > + const struct w1_uart_limits *limits, > > + struct w1_uart_config *w1cfg) > > +{ ... > > + /* 1-Wire response detectable for sent byte */ > > + if (limits->sample_us > 0 && > > + bit_ns * 8 < low_ns + to_ns(limits->sample_us)) > > BITS_PER_BYTE > ok, change it (it is the time for the UART data-frame). > > + return -EINVAL; > > + > > + /* delay to complete 1-Wire cycle, include start and stop-bit */ > > + w1cfg->delay_us = 0; > > + if (bit_ns * 10 < to_ns(limits->cycle_us)) > > What is this 10? Dub it. > > > + w1cfg->delay_us = > > + (to_ns(limits->cycle_us) - bit_ns * 10) / 1000; > > And this 10? > > The end: / NSEC_PER_USEC > will be more explicit (it is the time for the UART packet: BITS_PER_BYTE + 2 (start and stop-bit). ... > > +static int w1_uart_serdev_receive_buf(struct serdev_device *serdev, > > + const unsigned char *buf, size_t count) > > serdev already uses u8 * here. You are basing on the top of some old tree. Yes, this patch is based on the w1-next branch of git://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux-w1.git was not sure from where to start. I guess that this change is probably in the w1-tree after the next stable release. > > regards, > -- > js > suse labs > Thanks Jiri for the review! Kind regards, Christoph
