Hi Patrice,
On 31 July 2018 at 07:53, Patrice CHOTARD patrice.chotard@st.com wrote:
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 ;-)
OK good. Please change u8 there too :-)
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.
OK well I think what you have will work. We can separate out the parameters later if we see a need.
Also we need to call this from sandbox code for testing purposes,
Ok i will add a test for this.
Regards, Simon