[U-Boot] [PATCH v1 3/5] dm: serial: Add setparity

[Simon Glass]

Hi Patrick,

On 17 May 2018 at 06:50, Patrice Chotard <patrice.chotard at st.com> wrote:
> From: Patrick Delaunay <patrick.delaunay at st.com>
>
> Implements serial setparity ops to allow uart parity change.
> It allows to select ODD, EVEN or NONE parity.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
> Signed-off-by: Patrice Chotard <patrice.chotard at st.com>
> ---
>
>  include/serial.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/include/serial.h b/include/serial.h
> index 384df94ed0b3..b9ef6d91c9c5 100644
> --- a/include/serial.h
> +++ b/include/serial.h
> @@ -67,6 +67,12 @@ extern int usbtty_tstc(void);
>
>  struct udevice;
>
> +enum serial_par {
> +       SERIAL_PAR_NONE,
> +       SERIAL_PAR_ODD,
> +       SERIAL_PAR_EVEN
> +};
> +
>  /**
>   * struct struct dm_serial_ops - Driver model serial operations
>   *
> @@ -143,6 +149,16 @@ struct dm_serial_ops {
>          */
>         int (*loop)(struct udevice *dev, int on);
>  #endif
> +       /**
> +        * setparity() - Set up the parity
> +        *
> +        * Set up a new parity for this device.
> +        *
> +        * @dev: Device pointer
> +        * @parity: parity to use
> +        * @return 0 if OK, -ve on error
> +        */
> +       int (*setparity)(struct udevice *dev, enum serial_par parity);

To me it seems that changing parity while in the middle of operation
might be tricky. I suppose this follows along with setbrg() so fair
enough.

But I worry about adding more operations here. The next thing to come
is presumably the length (7 bits, 8 bits, ...). Perhaps we should have
a more generic setconfig() which change change speed, format and
parity all at once? For format and parity, we could have a 'default'
parameter value.

Also, there should be a corresponding function in serial-uclass.c and
ideal a call from some sandbox test (although I see at present we
don't have test/dm/serial.c)

Regards,
Simon