| Message ID | 20240509171736.2048414-5-christoph.fritz@hexdev.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | LIN Bus support for Linux | expand |
On Thu, 9 May 2024, Christoph Fritz wrote: > The recently introduced callback function receive_buf_fp() brings flags > buffer support. To allow signaling of TTY_BREAK flags, this patch > introduces serdev_device_set_break_detection() and an implementation for > ttyport. This enables serdev devices to configure their underlying tty > port to signal or ignore break conditions. > > Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de> > --- > drivers/tty/serdev/core.c | 11 +++++++++++ > drivers/tty/serdev/serdev-ttyport.c | 17 +++++++++++++++++ > include/linux/serdev.h | 2 ++ > 3 files changed, 30 insertions(+) > > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c > index 613cb356b918d..23a1e76cb553b 100644 > --- a/drivers/tty/serdev/core.c > +++ b/drivers/tty/serdev/core.c > @@ -339,6 +339,17 @@ unsigned int serdev_device_set_baudrate(struct serdev_device *serdev, unsigned i > } > EXPORT_SYMBOL_GPL(serdev_device_set_baudrate); > > +void serdev_device_set_break_detection(struct serdev_device *serdev, bool enable) > +{ > + struct serdev_controller *ctrl = serdev->ctrl; > + > + if (!ctrl || !ctrl->ops->set_break_detection) > + return; Why you need to test for !ctrl? > + ctrl->ops->set_break_detection(ctrl, enable); I'd use positive logic here: if (ctrl->ops->set_break_detection) ctrl->ops->set_break_detection(ctrl, enable); > +} > +EXPORT_SYMBOL_GPL(serdev_device_set_break_detection);
On Fri, 2024-05-10 at 17:21 +0300, Ilpo Järvinen wrote: > On Thu, 9 May 2024, Christoph Fritz wrote: ... > > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c > > index 613cb356b918d..23a1e76cb553b 100644 > > --- a/drivers/tty/serdev/core.c > > +++ b/drivers/tty/serdev/core.c > > @@ -339,6 +339,17 @@ unsigned int serdev_device_set_baudrate(struct serdev_device *serdev, unsigned i > > } > > EXPORT_SYMBOL_GPL(serdev_device_set_baudrate); > > > > +void serdev_device_set_break_detection(struct serdev_device *serdev, bool enable) > > +{ > > + struct serdev_controller *ctrl = serdev->ctrl; > > + > > + if (!ctrl || !ctrl->ops->set_break_detection) > > + return; > > Why you need to test for !ctrl? In our case we don't, it's an extra check like all the other functions here: https://elixir.bootlin.com/linux/v6.9-rc7/source/drivers/tty/serdev/core.c#L330 > > > + ctrl->ops->set_break_detection(ctrl, enable); > > I'd use positive logic here: > > if (ctrl->ops->set_break_detection) > ctrl->ops->set_break_detection(ctrl, enable);
On Sun, 12 May 2024, Christoph Fritz wrote: > On Fri, 2024-05-10 at 17:21 +0300, Ilpo Järvinen wrote: > > On Thu, 9 May 2024, Christoph Fritz wrote: > ... > > > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c > > > index 613cb356b918d..23a1e76cb553b 100644 > > > --- a/drivers/tty/serdev/core.c > > > +++ b/drivers/tty/serdev/core.c > > > @@ -339,6 +339,17 @@ unsigned int serdev_device_set_baudrate(struct serdev_device *serdev, unsigned i > > > } > > > EXPORT_SYMBOL_GPL(serdev_device_set_baudrate); > > > > > > +void serdev_device_set_break_detection(struct serdev_device *serdev, bool enable) > > > +{ > > > + struct serdev_controller *ctrl = serdev->ctrl; > > > + > > > + if (!ctrl || !ctrl->ops->set_break_detection) > > > + return; > > > > Why you need to test for !ctrl? > > In our case we don't, it's an extra check like all the other functions > here: > > https://elixir.bootlin.com/linux/v6.9-rc7/source/drivers/tty/serdev/core.c#L330 Okay. It might be I just don't understand all the structs in serdev well enough to understand when ctrl actually end ups being NULL so I don't object leaving it as is.
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c index 613cb356b918d..23a1e76cb553b 100644 --- a/drivers/tty/serdev/core.c +++ b/drivers/tty/serdev/core.c @@ -339,6 +339,17 @@ unsigned int serdev_device_set_baudrate(struct serdev_device *serdev, unsigned i } EXPORT_SYMBOL_GPL(serdev_device_set_baudrate); +void serdev_device_set_break_detection(struct serdev_device *serdev, bool enable) +{ + struct serdev_controller *ctrl = serdev->ctrl; + + if (!ctrl || !ctrl->ops->set_break_detection) + return; + + ctrl->ops->set_break_detection(ctrl, enable); +} +EXPORT_SYMBOL_GPL(serdev_device_set_break_detection); + void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable) { struct serdev_controller *ctrl = serdev->ctrl; diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c index bb47691afdb21..e928bf4175c6f 100644 --- a/drivers/tty/serdev/serdev-ttyport.c +++ b/drivers/tty/serdev/serdev-ttyport.c @@ -192,6 +192,22 @@ static void ttyport_set_flow_control(struct serdev_controller *ctrl, bool enable tty_set_termios(tty, &ktermios); } +static void ttyport_set_break_detection(struct serdev_controller *ctrl, bool enable) +{ + struct serport *serport = serdev_controller_get_drvdata(ctrl); + struct tty_struct *tty = serport->tty; + struct ktermios ktermios = tty->termios; + + ktermios.c_iflag &= ~(IGNBRK | BRKINT); + + if (enable) + ktermios.c_iflag |= BRKINT; + else + ktermios.c_iflag |= IGNBRK; + + tty_set_termios(tty, &ktermios); +} + static int ttyport_set_parity(struct serdev_controller *ctrl, enum serdev_parity parity) { @@ -263,6 +279,7 @@ static const struct serdev_controller_ops ctrl_ops = { .open = ttyport_open, .close = ttyport_close, .set_flow_control = ttyport_set_flow_control, + .set_break_detection = ttyport_set_break_detection, .set_parity = ttyport_set_parity, .set_baudrate = ttyport_set_baudrate, .wait_until_sent = ttyport_wait_until_sent, diff --git a/include/linux/serdev.h b/include/linux/serdev.h index 94fc81a1de933..84805762a67cc 100644 --- a/include/linux/serdev.h +++ b/include/linux/serdev.h @@ -91,6 +91,7 @@ struct serdev_controller_ops { int (*open)(struct serdev_controller *); void (*close)(struct serdev_controller *); void (*set_flow_control)(struct serdev_controller *, bool); + void (*set_break_detection)(struct serdev_controller *, bool); int (*set_parity)(struct serdev_controller *, enum serdev_parity); unsigned int (*set_baudrate)(struct serdev_controller *, unsigned int); void (*wait_until_sent)(struct serdev_controller *, long); @@ -208,6 +209,7 @@ void serdev_device_close(struct serdev_device *); int devm_serdev_device_open(struct device *, struct serdev_device *); unsigned int serdev_device_set_baudrate(struct serdev_device *, unsigned int); void serdev_device_set_flow_control(struct serdev_device *, bool); +void serdev_device_set_break_detection(struct serdev_device *, bool); int serdev_device_write_buf(struct serdev_device *, const u8 *, size_t); void serdev_device_wait_until_sent(struct serdev_device *, long); int serdev_device_get_tiocm(struct serdev_device *);
The recently introduced callback function receive_buf_fp() brings flags buffer support. To allow signaling of TTY_BREAK flags, this patch introduces serdev_device_set_break_detection() and an implementation for ttyport. This enables serdev devices to configure their underlying tty port to signal or ignore break conditions. Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de> --- drivers/tty/serdev/core.c | 11 +++++++++++ drivers/tty/serdev/serdev-ttyport.c | 17 +++++++++++++++++ include/linux/serdev.h | 2 ++ 3 files changed, 30 insertions(+)