Hi Simon
On 07/31/2018 01:52 PM, Simon Glass wrote:
Hi Patrice,
On 30 July 2018 at 09:23, Patrice Chotard patrice.chotard@st.com wrote:
From: Patrick Delaunay patrick.delaunay@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@st.com Signed-off-by: Patrice Chotard patrice.chotard@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;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)
Ok
+#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
ok
+#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.
ok
};
/**
1.9.1
Also you need a serial_setconfig() function call in the uclass so people can call it.
I already add serial_setconfig() function call in serial-uclass at the beginning of this patch ;-)
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.
Don't know what is better, currently only STM32 platforms will use it, internally we already use this API.
Also we need to call this from sandbox code for testing purposes,
Ok i will add a test for this.
Thanks
Patrice
Regards, Simon