[U-Boot] [PATCH v1 1/2] dm: serial: Replace setparity by setconfig
Simon Glass
sjg at chromium.org
Tue Jul 31 11:52:47 UTC 2018
Hi Patrice,
On 30 July 2018 at 09:23, Patrice Chotard <patrice.chotard at st.com> wrote:
> From: Patrick Delaunay <patrick.delaunay at st.com>
>
> Replace setparity by more generic setconfig ops
> to allow uart parity, bits word length and stop bits
> number change.
>
> Adds SERIAL_GET_PARITY/BITS/STOP macros.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
> Signed-off-by: Patrice Chotard <patrice.chotard at st.com>
> ---
>
> drivers/serial/serial-uclass.c | 14 ++++++++++++++
> include/serial.h | 42 +++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 53 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
> index 321d23ee93bf..9f523751ce17 100644
> --- a/drivers/serial/serial-uclass.c
> +++ b/drivers/serial/serial-uclass.c
> @@ -287,6 +287,18 @@ void serial_setbrg(void)
> ops->setbrg(gd->cur_serial_dev, gd->baudrate);
> }
>
> +void serial_setconfig(u8 config)
> +{
> + struct dm_serial_ops *ops;
> +
> + if (!gd->cur_serial_dev)
> + return;
> +
> + ops = serial_get_ops(gd->cur_serial_dev);
> + if (ops->setconfig)
> + ops->setconfig(gd->cur_serial_dev, config);
> +}
> +
> void serial_stdio_init(void)
> {
> }
> @@ -398,6 +410,8 @@ static int serial_post_probe(struct udevice *dev)
> ops->pending += gd->reloc_off;
> if (ops->clear)
> ops->clear += gd->reloc_off;
> + if (ops->setconfig)
> + ops->setconfig += gd->reloc_off;
> #if CONFIG_POST & CONFIG_SYS_POST_UART
> if (ops->loop)
> ops->loop += gd->reloc_off
> diff --git a/include/serial.h b/include/serial.h
> index b9ef6d91c9c5..61c1362e9e32 100644
> --- a/include/serial.h
> +++ b/include/serial.h
> @@ -73,6 +73,39 @@ enum serial_par {
> SERIAL_PAR_EVEN
> };
>
> +#define SERIAL_PAR_MASK 0x03
> +#define SERIAL_PAR_SHIFT 0
> +#define SERIAL_GET_PARITY(config) \
> + ((config & SERIAL_PAR_MASK) >> SERIAL_PAR_SHIFT)
> +
> +enum serial_bits {
> + SERIAL_5_BITS,
> + SERIAL_6_BITS,
> + SERIAL_7_BITS,
> + SERIAL_8_BITS
> +};
> +
> +#define SERIAL_BITS_MASK 0x0C
> +#define SERIAL_BITS_SHIFT 2
For consistency:
#define SERIAL_BITS_SHIFT 2
#define SERIAL_BITS_MASK (0x3 << SERIAL_BITS_SHIFT)
> +#define SERIAL_GET_BITS(config) \
> + ((config & SERIAL_BITS_MASK) >> SERIAL_BITS_SHIFT)
> +
> +enum serial_stop {
> + SERIAL_HALF_STOP, /* 0.5 stop bit */
> + SERIAL_ONE_STOP, /* 1 stop bit */
> + SERIAL_ONE_HALF_STOP, /* 1.5 stop bit */
> + SERIAL_TWO_STOP /* 2 stop bit */
> +};
> +
> +#define SERIAL_STOP_MASK 0x30
> +#define SERIAL_STOP_SHIFT 4
same here
> +#define SERIAL_GET_STOP(config) \
> + ((config & SERIAL_STOP_MASK) >> SERIAL_STOP_SHIFT)
> +
> +#define SERIAL_DEFAULT_CONFIG SERIAL_PAR_NONE << SERIAL_PAR_SHIFT | \
> + SERIAL_8_BITS << SERIAL_BITS_SHIFT | \
> + SERIAL_ONE_STOP << SERIAL_STOP_SHIFT
> +
> /**
> * struct struct dm_serial_ops - Driver model serial operations
> *
> @@ -150,15 +183,18 @@ struct dm_serial_ops {
> int (*loop)(struct udevice *dev, int on);
> #endif
> /**
> - * setparity() - Set up the parity
> + * setconfig() - Set up the uart configuration
> + * (parity, 5/6/7/8 bits word length, stop bits)
> *
> - * Set up a new parity for this device.
> + * Set up a new config for this device.
> *
> * @dev: Device pointer
> * @parity: parity to use
> + * @bits: bits number to use
> + * @stop: stop bits number to use
> * @return 0 if OK, -ve on error
> */
> - int (*setparity)(struct udevice *dev, enum serial_par parity);
> + int (*setconfig)(struct udevice *dev, u8 serial_config);
Please make this uint instead of u8. There is no point in using u8
since the compiler will use a register anyway. It can only make code
size worse, if the compile add masking, etc.
> };
>
> /**
> --
> 1.9.1
>
Also you need a serial_setconfig() function call in the uclass so
people can call it.
Perhaps that could have separate parameters for each setting, so that
the caller does not have to make up a mask? I'm not sure if that is
better or not.
Also we need to call this from sandbox code for testing purposes,
Regards,
Simon
More information about the U-Boot
mailing list