[RFC v2 1/9] serdev: implement parity configuration

[johan@kernel.org (Johan Hovold)]

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