[U-Boot] [PATCH 0/5] dm: serial: Fix up some serial API errors
New serial functions should use a device pointer as the first argument. The old functions are only there for backwards compatibilty, and can be adjusted soon.
For now, adjust the new functions to work correctly.
Simon Glass (5): serial: Move new functions to serial.h dm: serial: Adjust serial_getconfig() to use proper API dm: serial: Adjust serial_setconfig() to use proper API dm: serial: Adjust serial_getinfo() to use proper API dm: serial: Tidy up header file comments
arch/x86/lib/acpi_table.c | 11 +++++++---- drivers/serial/serial-uclass.c | 27 +++++++++---------------- include/common.h | 5 ----- include/serial.h | 36 +++++++++++++++++++++++++++++++--- test/dm/serial.c | 19 ++++++++++-------- 5 files changed, 60 insertions(+), 38 deletions(-)
We should not be adding new functions to common.h. Move these recently added functions to serial.h.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/common.h | 5 ----- include/serial.h | 4 ++++ 2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/include/common.h b/include/common.h index 20c99da1aa9..43d348b67c9 100644 --- a/include/common.h +++ b/include/common.h @@ -357,8 +357,6 @@ void smp_set_core_boot_addr(unsigned long addr, int corenr); void smp_kick_all_cpus(void);
/* $(CPU)/serial.c */ -struct serial_device_info; - int serial_init (void); void serial_setbrg (void); void serial_putc (const char); @@ -366,9 +364,6 @@ void serial_putc_raw(const char); void serial_puts (const char *); int serial_getc (void); int serial_tstc (void); -int serial_getconfig(uint *config); -int serial_setconfig(uint config); -int serial_getinfo(struct serial_device_info *info);
/* $(CPU)/speed.c */ int get_clocks (void); diff --git a/include/serial.h b/include/serial.h index c1a9fee250e..fa7e0130bd2 100644 --- a/include/serial.h +++ b/include/serial.h @@ -281,6 +281,10 @@ struct serial_dev_priv { /* Access the serial operations for a device */ #define serial_get_ops(dev) ((struct dm_serial_ops *)(dev)->driver->ops)
+int serial_getconfig(uint *config); +int serial_setconfig(uint config); +int serial_getinfo(struct serial_device_info *info); + void atmel_serial_initialize(void); void mcf_serial_initialize(void); void mpc85xx_serial_initialize(void);
All driver-model functions should have a device as the first parameter. Update this function accordingly.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/lib/acpi_table.c | 5 ++++- drivers/serial/serial-uclass.c | 9 +++------ include/serial.h | 2 +- test/dm/serial.c | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c index 79bc2000bda..bfcf2adbf12 100644 --- a/arch/x86/lib/acpi_table.c +++ b/arch/x86/lib/acpi_table.c @@ -342,6 +342,7 @@ static void acpi_create_spcr(struct acpi_spcr *spcr) struct acpi_table_header *header = &(spcr->header); struct serial_device_info serial_info = {0}; ulong serial_address, serial_offset; + struct udevice *dev; uint serial_config; uint serial_width; int access_size; @@ -431,7 +432,9 @@ static void acpi_create_spcr(struct acpi_spcr *spcr) break; }
- ret = serial_getconfig(&serial_config); + ret = uclass_first_device_err(UCLASS_SERIAL, &dev); + if (!ret) + ret = serial_getconfig(dev, &serial_config); if (ret) serial_config = SERIAL_DEFAULT_CONFIG;
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index ffcd6d15af2..81f1067f422 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -294,16 +294,13 @@ void serial_setbrg(void) ops->setbrg(gd->cur_serial_dev, gd->baudrate); }
-int serial_getconfig(uint *config) +int serial_getconfig(struct udevice *dev, uint *config) { struct dm_serial_ops *ops;
- if (!gd->cur_serial_dev) - return 0; - - ops = serial_get_ops(gd->cur_serial_dev); + ops = serial_get_ops(dev); if (ops->getconfig) - return ops->getconfig(gd->cur_serial_dev, config); + return ops->getconfig(dev, config);
return 0; } diff --git a/include/serial.h b/include/serial.h index fa7e0130bd2..5ba031ab534 100644 --- a/include/serial.h +++ b/include/serial.h @@ -281,7 +281,7 @@ struct serial_dev_priv { /* Access the serial operations for a device */ #define serial_get_ops(dev) ((struct dm_serial_ops *)(dev)->driver->ops)
-int serial_getconfig(uint *config); +int serial_getconfig(struct udevice *dev, uint *config); int serial_setconfig(uint config); int serial_getinfo(struct serial_device_info *info);
diff --git a/test/dm/serial.c b/test/dm/serial.c index 19a15d5d952..972755face3 100644 --- a/test/dm/serial.c +++ b/test/dm/serial.c @@ -24,7 +24,7 @@ static int dm_test_serial(struct unit_test_state *uts) * sandbox_serial driver */ ut_assertok(serial_setconfig(SERIAL_DEFAULT_CONFIG)); - ut_assertok(serial_getconfig(&value_serial)); + ut_assertok(serial_getconfig(dev_serial, &value_serial)); ut_assert(value_serial == SERIAL_DEFAULT_CONFIG); ut_assertok(serial_getinfo(&info_serial)); ut_assert(info_serial.type == SERIAL_CHIP_UNKNOWN); @@ -32,7 +32,7 @@ static int dm_test_serial(struct unit_test_state *uts) /* * test with a parameter which is NULL pointer */ - ut_asserteq(-EINVAL, serial_getconfig(NULL)); + ut_asserteq(-EINVAL, serial_getconfig(dev_serial, NULL)); ut_asserteq(-EINVAL, serial_getinfo(NULL)); /* * test with a serial config which is not supported by
All driver-model functions should have a device as the first parameter. Update this function accordingly.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/serial/serial-uclass.c | 9 +++------ include/serial.h | 2 +- test/dm/serial.c | 11 +++++++---- 3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index 81f1067f422..669c82f3793 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -305,16 +305,13 @@ int serial_getconfig(struct udevice *dev, uint *config) return 0; }
-int serial_setconfig(uint config) +int serial_setconfig(struct udevice *dev, uint config) { struct dm_serial_ops *ops;
- if (!gd->cur_serial_dev) - return 0; - - ops = serial_get_ops(gd->cur_serial_dev); + ops = serial_get_ops(dev); if (ops->setconfig) - return ops->setconfig(gd->cur_serial_dev, config); + return ops->setconfig(dev, config);
return 0; } diff --git a/include/serial.h b/include/serial.h index 5ba031ab534..8a05a090898 100644 --- a/include/serial.h +++ b/include/serial.h @@ -282,7 +282,7 @@ struct serial_dev_priv { #define serial_get_ops(dev) ((struct dm_serial_ops *)(dev)->driver->ops)
int serial_getconfig(struct udevice *dev, uint *config); -int serial_setconfig(uint config); +int serial_setconfig(struct udevice *dev, uint config); int serial_getinfo(struct serial_device_info *info);
void atmel_serial_initialize(void); diff --git a/test/dm/serial.c b/test/dm/serial.c index 972755face3..f82b4a19e8f 100644 --- a/test/dm/serial.c +++ b/test/dm/serial.c @@ -23,7 +23,7 @@ static int dm_test_serial(struct unit_test_state *uts) * test with default config which is the only one supported by * sandbox_serial driver */ - ut_assertok(serial_setconfig(SERIAL_DEFAULT_CONFIG)); + ut_assertok(serial_setconfig(dev_serial, SERIAL_DEFAULT_CONFIG)); ut_assertok(serial_getconfig(dev_serial, &value_serial)); ut_assert(value_serial == SERIAL_DEFAULT_CONFIG); ut_assertok(serial_getinfo(&info_serial)); @@ -39,7 +39,8 @@ static int dm_test_serial(struct unit_test_state *uts) * sandbox_serial driver: test with wrong parity */ ut_asserteq(-ENOTSUPP, - serial_setconfig(SERIAL_CONFIG(SERIAL_PAR_ODD, + serial_setconfig(dev_serial, + SERIAL_CONFIG(SERIAL_PAR_ODD, SERIAL_8_BITS, SERIAL_ONE_STOP))); /* @@ -47,7 +48,8 @@ static int dm_test_serial(struct unit_test_state *uts) * sandbox_serial driver: test with wrong bits number */ ut_asserteq(-ENOTSUPP, - serial_setconfig(SERIAL_CONFIG(SERIAL_PAR_NONE, + serial_setconfig(dev_serial, + SERIAL_CONFIG(SERIAL_PAR_NONE, SERIAL_6_BITS, SERIAL_ONE_STOP)));
@@ -56,7 +58,8 @@ static int dm_test_serial(struct unit_test_state *uts) * sandbox_serial driver: test with wrong stop bits number */ ut_asserteq(-ENOTSUPP, - serial_setconfig(SERIAL_CONFIG(SERIAL_PAR_NONE, + serial_setconfig(dev_serial, + SERIAL_CONFIG(SERIAL_PAR_NONE, SERIAL_8_BITS, SERIAL_TWO_STOP)));
All driver-model functions should have a device as the first parameter. Update this function accordingly.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/lib/acpi_table.c | 10 +++++----- drivers/serial/serial-uclass.c | 9 +++------ include/serial.h | 2 +- test/dm/serial.c | 4 ++-- 4 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c index bfcf2adbf12..cc7bbfa1436 100644 --- a/arch/x86/lib/acpi_table.c +++ b/arch/x86/lib/acpi_table.c @@ -354,7 +354,9 @@ static void acpi_create_spcr(struct acpi_spcr *spcr) header->length = sizeof(struct acpi_spcr); header->revision = 2;
- ret = serial_getinfo(&serial_info); + ret = uclass_first_device_err(UCLASS_SERIAL, &dev); + if (!ret) + ret = serial_getinfo(dev, &serial_info); if (ret) serial_info.type = SERIAL_CHIP_UNKNOWN;
@@ -432,11 +434,9 @@ static void acpi_create_spcr(struct acpi_spcr *spcr) break; }
- ret = uclass_first_device_err(UCLASS_SERIAL, &dev); - if (!ret) + serial_config = SERIAL_DEFAULT_CONFIG; + if (dev) ret = serial_getconfig(dev, &serial_config); - if (ret) - serial_config = SERIAL_DEFAULT_CONFIG;
spcr->parity = SERIAL_GET_PARITY(serial_config); spcr->stop_bits = SERIAL_GET_STOP(serial_config); diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index 669c82f3793..d4488a2cc28 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -316,21 +316,18 @@ int serial_setconfig(struct udevice *dev, uint config) return 0; }
-int serial_getinfo(struct serial_device_info *info) +int serial_getinfo(struct udevice *dev, struct serial_device_info *info) { struct dm_serial_ops *ops;
- if (!gd->cur_serial_dev) - return -ENODEV; - if (!info) return -EINVAL;
info->baudrate = gd->baudrate;
- ops = serial_get_ops(gd->cur_serial_dev); + ops = serial_get_ops(dev); if (ops->getinfo) - return ops->getinfo(gd->cur_serial_dev, info); + return ops->getinfo(dev, info);
return -EINVAL; } diff --git a/include/serial.h b/include/serial.h index 8a05a090898..8a790ccaaf4 100644 --- a/include/serial.h +++ b/include/serial.h @@ -283,7 +283,7 @@ struct serial_dev_priv {
int serial_getconfig(struct udevice *dev, uint *config); int serial_setconfig(struct udevice *dev, uint config); -int serial_getinfo(struct serial_device_info *info); +int serial_getinfo(struct udevice *dev, struct serial_device_info *info);
void atmel_serial_initialize(void); void mcf_serial_initialize(void); diff --git a/test/dm/serial.c b/test/dm/serial.c index f82b4a19e8f..3d741a8c363 100644 --- a/test/dm/serial.c +++ b/test/dm/serial.c @@ -26,14 +26,14 @@ static int dm_test_serial(struct unit_test_state *uts) ut_assertok(serial_setconfig(dev_serial, SERIAL_DEFAULT_CONFIG)); ut_assertok(serial_getconfig(dev_serial, &value_serial)); ut_assert(value_serial == SERIAL_DEFAULT_CONFIG); - ut_assertok(serial_getinfo(&info_serial)); + ut_assertok(serial_getinfo(dev_serial, &info_serial)); ut_assert(info_serial.type == SERIAL_CHIP_UNKNOWN); ut_assert(info_serial.addr == SERIAL_DEFAULT_ADDRESS); /* * test with a parameter which is NULL pointer */ ut_asserteq(-EINVAL, serial_getconfig(dev_serial, NULL)); - ut_asserteq(-EINVAL, serial_getinfo(NULL)); + ut_asserteq(-EINVAL, serial_getinfo(dev_serial, NULL)); /* * test with a serial config which is not supported by * sandbox_serial driver: test with wrong parity
The getconfig() comment is out of date. Fix this and add comments for recently added functions.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/serial.h | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/include/serial.h b/include/serial.h index 8a790ccaaf4..7acf781ebb8 100644 --- a/include/serial.h +++ b/include/serial.h @@ -235,9 +235,7 @@ struct dm_serial_ops { * Get a current config for this device. * * @dev: Device pointer - * @parity: parity to use - * @bits: bits number to use - * @stop: stop bits number to use + * @serial_config: Returns config information (see SERIAL_... above) * @return 0 if OK, -ve on error */ int (*getconfig)(struct udevice *dev, uint *serial_config); @@ -281,8 +279,36 @@ struct serial_dev_priv { /* Access the serial operations for a device */ #define serial_get_ops(dev) ((struct dm_serial_ops *)(dev)->driver->ops)
+/** + * serial_getconfig() - Get the uart configuration + * (parity, 5/6/7/8 bits word length, stop bits) + * + * Get a current config for this device. + * + * @dev: Device pointer + * @serial_config: Returns config information (see SERIAL_... above) + * @return 0 if OK, -ve on error + */ int serial_getconfig(struct udevice *dev, uint *config); + +/** + * serial_setconfig() - Set up the uart configuration + * (parity, 5/6/7/8 bits word length, stop bits) + * + * Set up a new config for this device. + * + * @dev: Device pointer + * @serial_config: number of bits, parity and number of stopbits to use + * @return 0 if OK, -ve on error + */ int serial_setconfig(struct udevice *dev, uint config); + +/** + * serial_getinfo() - Get serial device information + * + * @dev: Device pointer + * @info: struct serial_device_info to fill + */ int serial_getinfo(struct udevice *dev, struct serial_device_info *info);
void atmel_serial_initialize(void);
On Wed, Dec 5, 2018 at 9:54 PM Simon Glass sjg@chromium.org wrote:
New serial functions should use a device pointer as the first argument. The old functions are only there for backwards compatibilty, and can be adjusted soon.
For now, adjust the new functions to work correctly.
Thanks! I'm not sure I would be able to test it, but you may use my Reviewed-by: Andy Shevchenko andy.shevchenko@gmail.com
Couple of remarks: - I think it make sense to try for serial device only once in acpi_create_spcr() if I'm reading result correctly - the setconfig() / getconfig() have both some staled comments, it seems you dropped only for one.
Simon Glass (5): serial: Move new functions to serial.h dm: serial: Adjust serial_getconfig() to use proper API dm: serial: Adjust serial_setconfig() to use proper API dm: serial: Adjust serial_getinfo() to use proper API dm: serial: Tidy up header file comments
arch/x86/lib/acpi_table.c | 11 +++++++---- drivers/serial/serial-uclass.c | 27 +++++++++---------------- include/common.h | 5 ----- include/serial.h | 36 +++++++++++++++++++++++++++++++--- test/dm/serial.c | 19 ++++++++++-------- 5 files changed, 60 insertions(+), 38 deletions(-)
-- 2.20.0.rc1.387.gf8505762e3-goog
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Hi Andy,
On Wed, 5 Dec 2018 at 14:26, Andy Shevchenko andy.shevchenko@gmail.com wrote:
On Wed, Dec 5, 2018 at 9:54 PM Simon Glass sjg@chromium.org wrote:
New serial functions should use a device pointer as the first argument. The old functions are only there for backwards compatibilty, and can be adjusted soon.
For now, adjust the new functions to work correctly.
Thanks! I'm not sure I would be able to test it, but you may use my Reviewed-by: Andy Shevchenko andy.shevchenko@gmail.com
Couple of remarks:
- I think it make sense to try for serial device only once in
acpi_create_spcr() if I'm reading result correctly
I believe that is what I am doing. Can you please be more specific?
- the setconfig() / getconfig() have both some staled comments, it
seems you dropped only for one.
Again I am not sure where?
Regards, Simon
participants (2)
-
Andy Shevchenko -
Simon Glass