[U-Boot] [PATCH 0/5] dm: regmap: Various fixes for regmap
Recently a feature was added to allow sandbox to do memory-mapped IO using readl(), writel(), etc. This adds a few fixes and improvements to this.
It also includes an updated version of Jean-Jacques patch to add more tests for regmap.
Jean-Jacques Hiblot (1): test: regmap: check the values read from the regmap
Simon Glass (4): sandbox: test: Show hex values on failure sandbox: Drop 'const' from sandbox_write() sandbox: test: Add a prototype for sandbox_set_enable_memio() dm: regmap: Fix mask in regmap_update_bits()
arch/sandbox/cpu/cpu.c | 3 +-- arch/sandbox/include/asm/io.h | 11 +++++------ arch/sandbox/include/asm/test.h | 11 +++++++++++ drivers/core/regmap.c | 2 +- include/regmap.h | 3 ++- include/test/ut.h | 3 ++- test/dm/regmap.c | 19 ++++++++++++++++--- 7 files changed, 38 insertions(+), 14 deletions(-)
Quite a few tests use addresses or hex values for comparisons. Add hex output for test failures, e.g.:
0x55ca22fa == reg: Expected 0x55ca22fa (1439310586), got 0x55ea22fb (1441407739)
Signed-off-by: Simon Glass sjg@chromium.org ---
include/test/ut.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/test/ut.h b/include/test/ut.h index 19bcb8c3748..fbfde10719f 100644 --- a/include/test/ut.h +++ b/include/test/ut.h @@ -61,7 +61,8 @@ void ut_failf(struct unit_test_state *uts, const char *fname, int line, if (val1 != val2) { \ ut_failf(uts, __FILE__, __LINE__, __func__, \ #expr1 " == " #expr2, \ - "Expected %d, got %d", val1, val2); \ + "Expected %#x (%d), got %#x (%d)", val1, val1, \ + val2, val2); \ return CMD_RET_FAILURE; \ } \ }
On Sat, Oct 12, 2019 at 6:28 AM Simon Glass sjg@chromium.org wrote:
Quite a few tests use addresses or hex values for comparisons. Add hex output for test failures, e.g.:
0x55ca22fa == reg: Expected 0x55ca22fa (1439310586), got 0x55ea22fb (1441407739)
Signed-off-by: Simon Glass sjg@chromium.org
include/test/ut.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
On Sat, Oct 12, 2019 at 6:28 AM Simon Glass sjg@chromium.org wrote:
Quite a few tests use addresses or hex values for comparisons. Add hex output for test failures, e.g.:
0x55ca22fa == reg: Expected 0x55ca22fa (1439310586), got 0x55ea22fb (1441407739)
Signed-off-by: Simon Glass sjg@chromium.org
include/test/ut.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Applied to u-boot-dm, thanks!
This function writes to its address so the address should not be declared as const. Fix it.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/cpu/cpu.c | 3 +-- arch/sandbox/include/asm/io.h | 11 +++++------ 2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c index 2046cb53c4a..f3af88d79e9 100644 --- a/arch/sandbox/cpu/cpu.c +++ b/arch/sandbox/cpu/cpu.c @@ -246,8 +246,7 @@ unsigned int sandbox_read(const void *addr, enum sandboxio_size_t size) return 0; }
-void sandbox_write(const void *addr, unsigned int val, - enum sandboxio_size_t size) +void sandbox_write(void *addr, unsigned int val, enum sandboxio_size_t size) { struct sandbox_state *state = state_get_current();
diff --git a/arch/sandbox/include/asm/io.h b/arch/sandbox/include/asm/io.h index 4a35c419723..ad6c29a4e26 100644 --- a/arch/sandbox/include/asm/io.h +++ b/arch/sandbox/include/asm/io.h @@ -46,8 +46,7 @@ static inline void unmap_sysmem(const void *vaddr) phys_addr_t map_to_sysmem(const void *ptr);
unsigned int sandbox_read(const void *addr, enum sandboxio_size_t size); -void sandbox_write(const void *addr, unsigned int val, - enum sandboxio_size_t size); +void sandbox_write(void *addr, unsigned int val, enum sandboxio_size_t size);
#define readb(addr) sandbox_read((const void *)addr, SB_SIZE_8) #define readw(addr) sandbox_read((const void *)addr, SB_SIZE_16) @@ -55,11 +54,11 @@ void sandbox_write(const void *addr, unsigned int val, #ifdef CONFIG_SANDBOX64 #define readq(addr) sandbox_read((const void *)addr, SB_SIZE_64) #endif -#define writeb(v, addr) sandbox_write((const void *)addr, v, SB_SIZE_8) -#define writew(v, addr) sandbox_write((const void *)addr, v, SB_SIZE_16) -#define writel(v, addr) sandbox_write((const void *)addr, v, SB_SIZE_32) +#define writeb(v, addr) sandbox_write((void *)addr, v, SB_SIZE_8) +#define writew(v, addr) sandbox_write((void *)addr, v, SB_SIZE_16) +#define writel(v, addr) sandbox_write((void *)addr, v, SB_SIZE_32) #ifdef CONFIG_SANDBOX64 -#define writeq(v, addr) sandbox_write((const void *)addr, v, SB_SIZE_64) +#define writeq(v, addr) sandbox_write((void *)addr, v, SB_SIZE_64) #endif
/*
Hi Simon,
On Sat, Oct 12, 2019 at 6:22 AM Simon Glass sjg@chromium.org wrote:
This function writes to its address so the address should not be declared as const. Fix it.
Signed-off-by: Simon Glass sjg@chromium.org
arch/sandbox/cpu/cpu.c | 3 +-- arch/sandbox/include/asm/io.h | 11 +++++------ 2 files changed, 6 insertions(+), 8 deletions(-)
It looks I did something wrong when testing the previous patch :) I remember adding const was to fix build warning somewhere else.
Anyway the changes look good to me. But I suspect we need give it a full build to see whether it generates warning somewhere.
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Hi Simon,
On Sat, Oct 12, 2019 at 6:22 AM Simon Glass sjg@chromium.org wrote:
This function writes to its address so the address should not be declared as const. Fix it.
Signed-off-by: Simon Glass sjg@chromium.org
arch/sandbox/cpu/cpu.c | 3 +-- arch/sandbox/include/asm/io.h | 11 +++++------ 2 files changed, 6 insertions(+), 8 deletions(-)
It looks I did something wrong when testing the previous patch :) I remember adding const was to fix build warning somewhere else.
Anyway the changes look good to me. But I suspect we need give it a full build to see whether it generates warning somewhere.
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Applied to u-boot-dm, thanks!
This function needs a prototype so that tests can use it. Add one.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/include/asm/test.h | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/arch/sandbox/include/asm/test.h b/arch/sandbox/include/asm/test.h index cd2b9e3155d..b885e1a14f1 100644 --- a/arch/sandbox/include/asm/test.h +++ b/arch/sandbox/include/asm/test.h @@ -213,4 +213,15 @@ int sandbox_get_pci_ep_irq_count(struct udevice *dev); */ uint sandbox_pci_read_bar(u32 barval, int type, uint size);
+/** + * sandbox_set_enable_memio() - Enable readl/writel() for sandbox + * + * Normally these I/O functions do nothing with sandbox. Certain tests need them + * to work as for other architectures, so this function can be used to enable + * them. + * + * @enable: true to enable, false to disable + */ +void sandbox_set_enable_memio(bool enable); + #endif
On Sat, Oct 12, 2019 at 6:22 AM Simon Glass sjg@chromium.org wrote:
This function needs a prototype so that tests can use it. Add one.
Signed-off-by: Simon Glass sjg@chromium.org
arch/sandbox/include/asm/test.h | 11 +++++++++++ 1 file changed, 11 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
On Sat, Oct 12, 2019 at 6:22 AM Simon Glass sjg@chromium.org wrote:
This function needs a prototype so that tests can use it. Add one.
Signed-off-by: Simon Glass sjg@chromium.org
arch/sandbox/include/asm/test.h | 11 +++++++++++ 1 file changed, 11 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Applied to u-boot-dm, thanks!
This function assumes that the 'val' parameter has no masked bits set. This is not defined by the function prototype though. Fix the function to mask the value and update the documentation.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/regmap.c | 2 +- include/regmap.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c index d1d12eef385..e9e55c9d165 100644 --- a/drivers/core/regmap.c +++ b/drivers/core/regmap.c @@ -462,5 +462,5 @@ int regmap_update_bits(struct regmap *map, uint offset, uint mask, uint val)
reg &= ~mask;
- return regmap_write(map, offset, reg | val); + return regmap_write(map, offset, reg | (val & mask)); } diff --git a/include/regmap.h b/include/regmap.h index 0854200a9c1..9ada1af5ef0 100644 --- a/include/regmap.h +++ b/include/regmap.h @@ -295,7 +295,8 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset, * @map: The map returned by regmap_init_mem*() * @offset: Offset of the memory * @mask: Mask to apply to the read value - * @val: Value to apply to the value to write + * @val: Value to OR with the read value after masking. Note that any + * bits set in @val which are not set in @mask are ignored * Return: 0 if OK, -ve on error */ int regmap_update_bits(struct regmap *map, uint offset, uint mask, uint val);
On Sat, Oct 12, 2019 at 6:25 AM Simon Glass sjg@chromium.org wrote:
This function assumes that the 'val' parameter has no masked bits set. This is not defined by the function prototype though. Fix the function to mask the value and update the documentation.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/core/regmap.c | 2 +- include/regmap.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
On 12/10/2019 00:16, Simon Glass wrote:
This function assumes that the 'val' parameter has no masked bits set. This is not defined by the function prototype though. Fix the function to mask the value and update the documentation.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/core/regmap.c | 2 +- include/regmap.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c index d1d12eef385..e9e55c9d165 100644 --- a/drivers/core/regmap.c +++ b/drivers/core/regmap.c @@ -462,5 +462,5 @@ int regmap_update_bits(struct regmap *map, uint offset, uint mask, uint val)
reg &= ~mask;
- return regmap_write(map, offset, reg | val);
- return regmap_write(map, offset, reg | (val & mask)); }
diff --git a/include/regmap.h b/include/regmap.h index 0854200a9c1..9ada1af5ef0 100644 --- a/include/regmap.h +++ b/include/regmap.h @@ -295,7 +295,8 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
- @map: The map returned by regmap_init_mem*()
- @offset: Offset of the memory
- @mask: Mask to apply to the read value
- @val: Value to apply to the value to write
- @val: Value to OR with the read value after masking. Note that any
*/ int regmap_update_bits(struct regmap *map, uint offset, uint mask, uint val);
- bits set in @val which are not set in @mask are ignored
- Return: 0 if OK, -ve on error
Reviewed-by: Jean-Jacques Hiblot jjhiblot@ti.com
On 12/10/2019 00:16, Simon Glass wrote:
This function assumes that the 'val' parameter has no masked bits set. This is not defined by the function prototype though. Fix the function to mask the value and update the documentation.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/core/regmap.c | 2 +- include/regmap.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
From: Jean-Jacques Hiblot jjhiblot@ti.com
The test did reads after writes but didn't check the value. It probably was because the sandbox didn't implement the writeX/readX functions.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com Updated to use sandbox_set_enable_memio(): Signed-off-by: Simon Glass sjg@chromium.org ---
test/dm/regmap.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/test/dm/regmap.c b/test/dm/regmap.c index 82de295cb8f..6fd1f20656b 100644 --- a/test/dm/regmap.c +++ b/test/dm/regmap.c @@ -99,18 +99,27 @@ static int dm_test_regmap_rw(struct unit_test_state *uts) struct regmap *map; uint reg;
+ sandbox_set_enable_memio(true); ut_assertok(uclass_get_device(UCLASS_SYSCON, 0, &dev)); map = syscon_get_regmap(dev); ut_assertok_ptr(map);
ut_assertok(regmap_write(map, 0, 0xcacafafa)); - ut_assertok(regmap_write(map, 3, 0x55aa2211)); + ut_assertok(regmap_write(map, 5, 0x55aa2211));
ut_assertok(regmap_read(map, 0, ®)); - ut_assertok(regmap_read(map, 3, ®)); + ut_asserteq(0xcacafafa, reg); + ut_assertok(regmap_read(map, 5, ®)); + ut_asserteq(0x55aa2211, reg);
+ ut_assertok(regmap_read(map, 0, ®)); + ut_asserteq(0xcacafafa, reg); ut_assertok(regmap_update_bits(map, 0, 0xff00ff00, 0x55aa2211)); - ut_assertok(regmap_update_bits(map, 3, 0x00ff00ff, 0xcacafada)); + ut_assertok(regmap_read(map, 0, ®)); + ut_asserteq(0x55ca22fa, reg); + ut_assertok(regmap_update_bits(map, 5, 0x00ff00ff, 0xcacafada)); + ut_assertok(regmap_read(map, 5, ®)); + ut_asserteq(0x55ca22da, reg);
return 0; } @@ -130,6 +139,7 @@ static int dm_test_regmap_getset(struct unit_test_state *uts) u32 val3; };
+ sandbox_set_enable_memio(true); ut_assertok(uclass_get_device(UCLASS_SYSCON, 0, &dev)); map = syscon_get_regmap(dev); ut_assertok_ptr(map); @@ -138,7 +148,9 @@ static int dm_test_regmap_getset(struct unit_test_state *uts) regmap_set(map, struct layout, val3, 0x55aa2211);
ut_assertok(regmap_get(map, struct layout, val0, ®)); + ut_asserteq(0xcacafafa, reg); ut_assertok(regmap_get(map, struct layout, val3, ®)); + ut_asserteq(0x55aa2211, reg);
return 0; } @@ -159,6 +171,7 @@ static int dm_test_regmap_poll(struct unit_test_state *uts)
start = get_timer(0);
+ ut_assertok(regmap_write(map, 0, 0x0)); ut_asserteq(-ETIMEDOUT, regmap_read_poll_timeout_test(map, 0, reg, (reg == 0xcacafafa),
On Sat, Oct 12, 2019 at 6:28 AM Simon Glass sjg@chromium.org wrote:
From: Jean-Jacques Hiblot jjhiblot@ti.com
The test did reads after writes but didn't check the value. It probably was because the sandbox didn't implement the writeX/readX functions.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com Updated to use sandbox_set_enable_memio(): Signed-off-by: Simon Glass sjg@chromium.org
test/dm/regmap.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com
On Sat, Oct 12, 2019 at 6:28 AM Simon Glass sjg@chromium.org wrote:
From: Jean-Jacques Hiblot jjhiblot@ti.com
The test did reads after writes but didn't check the value. It probably was because the sandbox didn't implement the writeX/readX functions.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com Updated to use sandbox_set_enable_memio(): Signed-off-by: Simon Glass sjg@chromium.org
test/dm/regmap.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com
Applied to u-boot-dm, thanks!
participants (4)
-
Bin Meng -
Jean-Jacques Hiblot -
Simon Glass -
sjg＠google.com