[U-Boot] [PATCH 0/5] dm: regmap: Various fixes for regmap

[U-Boot] [PATCH 0/5] dm: regmap: Various fixes for regmap

[Simon Glass]

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(-)

-- 
2.23.0.700.g56cf767bdb-goog

[U-Boot] [PATCH 1/5] sandbox: test: Show hex values on failure

[Simon Glass]

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;					\
 	}								\
 }
-- 
2.23.0.700.g56cf767bdb-goog

[U-Boot] [PATCH 1/5] sandbox: test: Show hex values on failure

[Bin Meng]

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>

[U-Boot] [PATCH 1/5] sandbox: test: Show hex values on failure

[sjg at google.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!

[U-Boot] [PATCH 2/5] sandbox: Drop 'const' from sandbox_write()

[Simon Glass]

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
 
 /*
-- 
2.23.0.700.g56cf767bdb-goog

[U-Boot] [PATCH 2/5] sandbox: Drop 'const' from sandbox_write()

[Bin Meng]

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>

[U-Boot] [PATCH 2/5] sandbox: Drop 'const' from sandbox_write()

[sjg at google.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!

[U-Boot] [PATCH 3/5] sandbox: test: Add a prototype for sandbox_set_enable_memio()

[Simon Glass]

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
-- 
2.23.0.700.g56cf767bdb-goog

[U-Boot] [PATCH 3/5] sandbox: test: Add a prototype for sandbox_set_enable_memio()

[Bin Meng]

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>

[U-Boot] [PATCH 3/5] sandbox: test: Add a prototype for sandbox_set_enable_memio()

[sjg at google.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!

[U-Boot] [PATCH 4/5] dm: regmap: Fix mask in regmap_update_bits()

[Simon Glass]

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);
-- 
2.23.0.700.g56cf767bdb-goog

[U-Boot] [PATCH 4/5] dm: regmap: Fix mask in regmap_update_bits()

[Bin Meng]

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>

[U-Boot] [PATCH 4/5] dm: regmap: Fix mask in regmap_update_bits()

[Jean-Jacques Hiblot]

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
> + *	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);

Reviewed-by: Jean-Jacques Hiblot <jjhiblot@ti.com>

[U-Boot] [PATCH 4/5] dm: regmap: Fix mask in regmap_update_bits()

[sjg at google.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!

[U-Boot] [PATCH 5/5] test: regmap: check the values read from the regmap

[Simon Glass]

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, &reg));
-	ut_assertok(regmap_read(map, 3, &reg));
+	ut_asserteq(0xcacafafa, reg);
+	ut_assertok(regmap_read(map, 5, &reg));
+	ut_asserteq(0x55aa2211, reg);
 
+	ut_assertok(regmap_read(map, 0, &reg));
+	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, &reg));
+	ut_asserteq(0x55ca22fa, reg);
+	ut_assertok(regmap_update_bits(map, 5, 0x00ff00ff, 0xcacafada));
+	ut_assertok(regmap_read(map, 5, &reg));
+	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, &reg));
+	ut_asserteq(0xcacafafa, reg);
 	ut_assertok(regmap_get(map, struct layout, val3, &reg));
+	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),
-- 
2.23.0.700.g56cf767bdb-goog

[U-Boot] [PATCH 5/5] test: regmap: check the values read from the regmap

[Bin Meng]

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>

[U-Boot] [PATCH 5/5] test: regmap: check the values read from the regmap

[sjg at google.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!