| Message ID | 20180101204217.26165-2-martin.blumenstingl@googlemail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
Hi Martin, > Some Bluetooth modules (for example the ones found in Realtek RTL8723BS > and RTL8723DS) want to communicate with the host with even parity > enabled. > Add a new function and the corresponding internal callbacks so parity > can be configured. This supports enabling and disabling parity as well > as setting the type to odd or even. > > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > --- > drivers/tty/serdev/core.c | 12 ++++++++++++ > drivers/tty/serdev/serdev-ttyport.c | 21 +++++++++++++++++++++ > include/linux/serdev.h | 5 +++++ > 3 files changed, 38 insertions(+) > > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c > index 1bef39828ca7..d327b02980f5 100644 > --- a/drivers/tty/serdev/core.c > +++ b/drivers/tty/serdev/core.c > @@ -225,6 +225,18 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable) > } > EXPORT_SYMBOL_GPL(serdev_device_set_flow_control); > > +void serdev_device_set_parity(struct serdev_device *serdev, bool enable, > + bool odd) > +{ > + struct serdev_controller *ctrl = serdev->ctrl; > + > + if (!ctrl || !ctrl->ops->set_parity) > + return; > + > + ctrl->ops->set_parity(ctrl, enable, odd); > +} > +EXPORT_SYMBOL_GPL(serdev_device_set_parity); > + this really needs Rob’s ACK before I take the patch. Regards Marcel
Hi Marcel, Hi Rob, On Tue, Jan 2, 2018 at 12:16 PM, Marcel Holtmann <marcel@holtmann.org> wrote: > Hi Martin, > >> Some Bluetooth modules (for example the ones found in Realtek RTL8723BS >> and RTL8723DS) want to communicate with the host with even parity >> enabled. >> Add a new function and the corresponding internal callbacks so parity >> can be configured. This supports enabling and disabling parity as well >> as setting the type to odd or even. >> >> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> >> --- >> drivers/tty/serdev/core.c | 12 ++++++++++++ >> drivers/tty/serdev/serdev-ttyport.c | 21 +++++++++++++++++++++ >> include/linux/serdev.h | 5 +++++ >> 3 files changed, 38 insertions(+) >> >> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c >> index 1bef39828ca7..d327b02980f5 100644 >> --- a/drivers/tty/serdev/core.c >> +++ b/drivers/tty/serdev/core.c >> @@ -225,6 +225,18 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable) >> } >> EXPORT_SYMBOL_GPL(serdev_device_set_flow_control); >> >> +void serdev_device_set_parity(struct serdev_device *serdev, bool enable, >> + bool odd) >> +{ >> + struct serdev_controller *ctrl = serdev->ctrl; >> + >> + if (!ctrl || !ctrl->ops->set_parity) >> + return; >> + >> + ctrl->ops->set_parity(ctrl, enable, odd); >> +} >> +EXPORT_SYMBOL_GPL(serdev_device_set_parity); >> + > > this really needs Rob’s ACK before I take the patch. sure I could even live with a NACK in case these two bool parameters are considered to be ugly in that case I would propose an enum with three values: DISABLED, EVEN, ODD so the arguments would look like this: void serdev_device_set_parity(struct serdev_device *serdev, enum parity) Regards Martin
On Tue, Jan 2, 2018 at 10:16 PM, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > Hi Marcel, Hi Rob, > > On Tue, Jan 2, 2018 at 12:16 PM, Marcel Holtmann <marcel@holtmann.org> wrote: >> Hi Martin, >> >>> Some Bluetooth modules (for example the ones found in Realtek RTL8723BS >>> and RTL8723DS) want to communicate with the host with even parity >>> enabled. >>> Add a new function and the corresponding internal callbacks so parity >>> can be configured. This supports enabling and disabling parity as well >>> as setting the type to odd or even. >>> >>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> >>> --- >>> drivers/tty/serdev/core.c | 12 ++++++++++++ >>> drivers/tty/serdev/serdev-ttyport.c | 21 +++++++++++++++++++++ >>> include/linux/serdev.h | 5 +++++ >>> 3 files changed, 38 insertions(+) >>> >>> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c >>> index 1bef39828ca7..d327b02980f5 100644 >>> --- a/drivers/tty/serdev/core.c >>> +++ b/drivers/tty/serdev/core.c >>> @@ -225,6 +225,18 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable) >>> } >>> EXPORT_SYMBOL_GPL(serdev_device_set_flow_control); >>> >>> +void serdev_device_set_parity(struct serdev_device *serdev, bool enable, >>> + bool odd) >>> +{ >>> + struct serdev_controller *ctrl = serdev->ctrl; >>> + >>> + if (!ctrl || !ctrl->ops->set_parity) >>> + return; >>> + >>> + ctrl->ops->set_parity(ctrl, enable, odd); >>> +} >>> +EXPORT_SYMBOL_GPL(serdev_device_set_parity); >>> + >> >> this really needs Rob’s ACK before I take the patch. > sure > > I could even live with a NACK in case these two bool parameters are > considered to be ugly > in that case I would propose an enum with three values: DISABLED, > EVEN, ODD so the arguments would look like this: > void serdev_device_set_parity(struct serdev_device *serdev, enum parity) I just discovered: such a patch was already posted by Ulrich Hecht: [0] [0] https://patchwork.kernel.org/patch/9903787/
On Tue, Jan 02, 2018 at 10:34:04PM +0100, Martin Blumenstingl wrote: > On Tue, Jan 2, 2018 at 10:16 PM, Martin Blumenstingl > <martin.blumenstingl@googlemail.com> wrote: > > Hi Marcel, Hi Rob, > > > > On Tue, Jan 2, 2018 at 12:16 PM, Marcel Holtmann <marcel@holtmann.org> wrote: > >> Hi Martin, > >> > >>> Some Bluetooth modules (for example the ones found in Realtek RTL8723BS > >>> and RTL8723DS) want to communicate with the host with even parity > >>> enabled. > >>> Add a new function and the corresponding internal callbacks so parity > >>> can be configured. This supports enabling and disabling parity as well > >>> as setting the type to odd or even. > >>> > >>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > >>> --- > >>> drivers/tty/serdev/core.c | 12 ++++++++++++ > >>> drivers/tty/serdev/serdev-ttyport.c | 21 +++++++++++++++++++++ > >>> include/linux/serdev.h | 5 +++++ > >>> 3 files changed, 38 insertions(+) > >>> > >>> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c > >>> index 1bef39828ca7..d327b02980f5 100644 > >>> --- a/drivers/tty/serdev/core.c > >>> +++ b/drivers/tty/serdev/core.c > >>> @@ -225,6 +225,18 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable) > >>> } > >>> EXPORT_SYMBOL_GPL(serdev_device_set_flow_control); > >>> > >>> +void serdev_device_set_parity(struct serdev_device *serdev, bool enable, > >>> + bool odd) > >>> +{ > >>> + struct serdev_controller *ctrl = serdev->ctrl; > >>> + > >>> + if (!ctrl || !ctrl->ops->set_parity) > >>> + return; > >>> + > >>> + ctrl->ops->set_parity(ctrl, enable, odd); > >>> +} > >>> +EXPORT_SYMBOL_GPL(serdev_device_set_parity); > >>> + > >> > >> this really needs Rob’s ACK before I take the patch. > > sure > > > > I could even live with a NACK in case these two bool parameters are > > considered to be ugly > > in that case I would propose an enum with three values: DISABLED, > > EVEN, ODD so the arguments would look like this: > > void serdev_device_set_parity(struct serdev_device *serdev, enum parity) > I just discovered: such a patch was already posted by Ulrich Hecht: [0] > > > [0] https://patchwork.kernel.org/patch/9903787/ Yeah, that would be the preferred way of doing this as it's more readable and less error prone (and more easily extended if anyone ever needs mark and space parity). Also, I know serdev currently fails to check for errors from tty_set_termios, but we really need to start doing that. Not all serial drivers support (every) parity mode so you need to check the tty termios for the actual mode used after tty_set_termios() returns. Thanks, Johan
Hi Martin, >>>> Some Bluetooth modules (for example the ones found in Realtek RTL8723BS >>>> and RTL8723DS) want to communicate with the host with even parity >>>> enabled. >>>> Add a new function and the corresponding internal callbacks so parity >>>> can be configured. This supports enabling and disabling parity as well >>>> as setting the type to odd or even. >>>> >>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> >>>> --- >>>> drivers/tty/serdev/core.c | 12 ++++++++++++ >>>> drivers/tty/serdev/serdev-ttyport.c | 21 +++++++++++++++++++++ >>>> include/linux/serdev.h | 5 +++++ >>>> 3 files changed, 38 insertions(+) >>>> >>>> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c >>>> index 1bef39828ca7..d327b02980f5 100644 >>>> --- a/drivers/tty/serdev/core.c >>>> +++ b/drivers/tty/serdev/core.c >>>> @@ -225,6 +225,18 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable) >>>> } >>>> EXPORT_SYMBOL_GPL(serdev_device_set_flow_control); >>>> >>>> +void serdev_device_set_parity(struct serdev_device *serdev, bool enable, >>>> + bool odd) >>>> +{ >>>> + struct serdev_controller *ctrl = serdev->ctrl; >>>> + >>>> + if (!ctrl || !ctrl->ops->set_parity) >>>> + return; >>>> + >>>> + ctrl->ops->set_parity(ctrl, enable, odd); >>>> +} >>>> +EXPORT_SYMBOL_GPL(serdev_device_set_parity); >>>> + >>> >>> this really needs Rob’s ACK before I take the patch. >> sure >> >> I could even live with a NACK in case these two bool parameters are >> considered to be ugly >> in that case I would propose an enum with three values: DISABLED, >> EVEN, ODD so the arguments would look like this: >> void serdev_device_set_parity(struct serdev_device *serdev, enum parity) > I just discovered: such a patch was already posted by Ulrich Hecht: [0] > > > [0] https://patchwork.kernel.org/patch/9903787/ any idea what the status of this one is? It would be good if we get an ACK from Rob and you just include it in your patch series. I do not see it currently in Linus’ tree or net-next. If it goes via a different path to Linus, we will have a bit of a problem getting this all merged for the next kernel. Regards Marcel
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c index 1bef39828ca7..d327b02980f5 100644 --- a/drivers/tty/serdev/core.c +++ b/drivers/tty/serdev/core.c @@ -225,6 +225,18 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable) } EXPORT_SYMBOL_GPL(serdev_device_set_flow_control); +void serdev_device_set_parity(struct serdev_device *serdev, bool enable, + bool odd) +{ + struct serdev_controller *ctrl = serdev->ctrl; + + if (!ctrl || !ctrl->ops->set_parity) + return; + + ctrl->ops->set_parity(ctrl, enable, odd); +} +EXPORT_SYMBOL_GPL(serdev_device_set_parity); + void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout) { struct serdev_controller *ctrl = serdev->ctrl; diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c index 247788a16f0b..36bed86e5c96 100644 --- a/drivers/tty/serdev/serdev-ttyport.c +++ b/drivers/tty/serdev/serdev-ttyport.c @@ -190,6 +190,26 @@ static void ttyport_set_flow_control(struct serdev_controller *ctrl, bool enable tty_set_termios(tty, &ktermios); } +static void ttyport_set_parity(struct serdev_controller *ctrl, bool enable, + bool odd) +{ + struct serport *serport = serdev_controller_get_drvdata(ctrl); + struct tty_struct *tty = serport->tty; + struct ktermios ktermios = tty->termios; + + if (enable) + ktermios.c_cflag |= PARENB; + else + ktermios.c_cflag &= ~PARENB; + + if (odd) + ktermios.c_cflag |= PARODD; + else + ktermios.c_cflag &= ~PARODD; + + tty_set_termios(tty, &ktermios); +} + static void ttyport_wait_until_sent(struct serdev_controller *ctrl, long timeout) { struct serport *serport = serdev_controller_get_drvdata(ctrl); @@ -227,6 +247,7 @@ static const struct serdev_controller_ops ctrl_ops = { .open = ttyport_open, .close = ttyport_close, .set_flow_control = ttyport_set_flow_control, + .set_parity = ttyport_set_parity, .set_baudrate = ttyport_set_baudrate, .wait_until_sent = ttyport_wait_until_sent, .get_tiocm = ttyport_get_tiocm, diff --git a/include/linux/serdev.h b/include/linux/serdev.h index d609e6dc5bad..07f2d4d4e88c 100644 --- a/include/linux/serdev.h +++ b/include/linux/serdev.h @@ -86,6 +86,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_parity)(struct serdev_controller *, bool, bool); unsigned int (*set_baudrate)(struct serdev_controller *, unsigned int); void (*wait_until_sent)(struct serdev_controller *, long); int (*get_tiocm)(struct serdev_controller *); @@ -195,6 +196,7 @@ int serdev_device_open(struct serdev_device *); void serdev_device_close(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_parity(struct serdev_device *, bool, bool); int serdev_device_write_buf(struct serdev_device *, const unsigned char *, size_t); void serdev_device_wait_until_sent(struct serdev_device *, long); int serdev_device_get_tiocm(struct serdev_device *); @@ -237,6 +239,9 @@ static inline unsigned int serdev_device_set_baudrate(struct serdev_device *sdev return 0; } static inline void serdev_device_set_flow_control(struct serdev_device *sdev, bool enable) {} +static inline void serdev_device_set_parity(struct serdev_device *sdev, + bool enable, + bool odd) {} static inline int serdev_device_write_buf(struct serdev_device *serdev, const unsigned char *buf, size_t count)
Some Bluetooth modules (for example the ones found in Realtek RTL8723BS and RTL8723DS) want to communicate with the host with even parity enabled. Add a new function and the corresponding internal callbacks so parity can be configured. This supports enabling and disabling parity as well as setting the type to odd or even. Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> --- drivers/tty/serdev/core.c | 12 ++++++++++++ drivers/tty/serdev/serdev-ttyport.c | 21 +++++++++++++++++++++ include/linux/serdev.h | 5 +++++ 3 files changed, 38 insertions(+)