dm: rockchip: Move rockchip boards to to use CONFIG_BLK

- Rename 'w' to 'width' to make it more obvious what it is used for
- Use bool and int types instead of char to avoid register-masking on 32-bit machines

Signed-off-by Simon Glass <[email protected]>

Quoted 5 lines · Simon Glass

- Rename 'w' to 'width' to make it more obvious what it is used for
- Use bool and int types instead of char to avoid register-masking on 32-bit machines

Quoted 1 line

Signed-off-by Simon Glass <[email protected]>

Reviewed-by Stefan Roese <[email protected]>

Thanks,
Stefan

Add a simple version of this function for SPL. It does not check the buffer size as this would add to the code size.

Signed-off-by Simon Glass <[email protected]>

Quoted 5 lines · Simon Glass

Add a simple version of this function for SPL. It does not check the buffer
size as this would add to the code size.

Quoted 1 line

Signed-off-by Simon Glass <[email protected]>

Reviewed-by Stefan Roese <[email protected]>

Thanks,
Stefan

This adds to code size and is not needed, since hang() will print a message.

Signed-off-by Simon Glass <[email protected]>

---

 drivers/misc/reset-uclass.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

This function is no longer used.

Signed-off-by Simon Glass <[email protected]>

---

 drivers/mmc/mmc.c | 9 ---------
 include/mmc.h     | 1 -
 2 files changed, 10 deletions(-)

All boards that use MMC define CONFIG_GENERIC_MMC now, so we can drop this old code.

Signed-off-by Simon Glass <[email protected]>

---

 cmd/mmc.c     | 62 -----------------------------------------------------------
 include/mmc.h |  4 ----
 2 files changed, 66 deletions(-)

We don't need an int since no value is over 80. This saves a small amount of SPL space (about 44 bytes).

Signed-off-by Simon Glass <[email protected]>

---

 drivers/mmc/mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Move these to debug() like the one in check_cache range(), to save SPL space.

Signed-off-by Simon Glass <[email protected]>

---

 arch/arm/cpu/armv7/cache_v7.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

arch/arm/cpu/armv7/cache_v7.c +4 -4

index dc309da..68cf62e 100644
@@ -66,8 +66,8 @@ static void v7_dcache_inval_range(u32 start, u32 stop, u32 line_len)
 	 * invalidate the first cache-line
 	 */
 	if (start & (line_len - 1)) {
-		printf("ERROR: %s - start address is not aligned - 0x%08x\n",
-			__func__, start);
+		debug("ERROR: %s - start address is not aligned - 0x%08x\n",
+		      __func__, start);
 		/* move to next cache line */
 		start = (start + line_len - 1) & ~(line_len - 1);
 	}

@@ -77,8 +77,8 @@ static void v7_dcache_inval_range(u32 start, u32 stop, u32 line_len)
 	 * invalidate the last cache-line
 	 */
 	if (stop & (line_len - 1)) {
-		printf("ERROR: %s - stop address is not aligned - 0x%08x\n",
-			__func__, stop);
+		debug("ERROR: %s - stop address is not aligned - 0x%08x\n",
+		      __func__, stop);
 		/* align to the beginning of this cache line */
 		stop &= ~(line_len - 1);
 	}

Quoted 1 line · Simon Glass

Move these to debug() like the one in check_cache range(), to save SPL space.

This hides cache problems, which were visibly reported so far. I am opposed to this patch.

Wouldn't it make more sense to completely disable printf() and co. in SPL if you're after saving space?

Quoted 31 lines

Signed-off-by Simon Glass <[email protected]>

---

 arch/arm/cpu/armv7/cache_v7.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

arch/arm/cpu/armv7/cache_v7.c +4 -4

index dc309da..68cf62e 100644
@@ -66,8 +66,8 @@ static void v7_dcache_inval_range(u32 start, u32 stop, u32 line_len)
       * invalidate the first cache-line
       */
      if (start & (line_len - 1)) {
-             printf("ERROR: %s - start address is not aligned - 0x%08x\n",
-                     __func__, start);
+             debug("ERROR: %s - start address is not aligned - 0x%08x\n",
+                   __func__, start);
              /* move to next cache line */
              start = (start + line_len - 1) & ~(line_len - 1);
      }

@@ -77,8 +77,8 @@ static void v7_dcache_inval_range(u32 start, u32 stop, u32 line_len)
       * invalidate the last cache-line
       */
      if (stop & (line_len - 1)) {
-             printf("ERROR: %s - stop address is not aligned - 0x%08x\n",
-                     __func__, stop);
+             debug("ERROR: %s - stop address is not aligned - 0x%08x\n",
+                   __func__, stop);
              /* align to the beginning of this cache line */

arch/arm/cpu/armv7/cache_v7.c static void v7_dcache_inval_range(u32 start, u32 stop, u32 line_len) · line 84

Quoted 3 lines

arch/arm/cpu/armv7/cache_v7.c +0 -0

              stop &= ~(line_len - 1);
      }

--
Best regards,
Marek Vasut

Hi Marek,

Quoted 5 lines · Simon Glass

Quoted 1 line

Move these to debug() like the one in check_cache range(), to save SPL space.

This hides cache problems, which were visibly reported so far. I am opposed to this patch.

Sure, but see check_cache_range(). It uses debug(). In fact I found the at91 cache problem only after trying #define DEBUG in the code there.

Quoted 3 lines

Wouldn't it make more sense to completely disable printf() and co. in SPL if you're after saving space?

Or maybe we need something that prints a message in U-Boot proper, but not SPL? I'll take a look.

Quoted 31 lines

Quoted 31 lines

Signed-off-by Simon Glass <[email protected]>

---

 arch/arm/cpu/armv7/cache_v7.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

arch/arm/cpu/armv7/cache_v7.c +4 -4

index dc309da..68cf62e 100644
@@ -66,8 +66,8 @@ static void v7_dcache_inval_range(u32 start, u32 stop, u32 line_len)
       * invalidate the first cache-line
       */
      if (start & (line_len - 1)) {
-             printf("ERROR: %s - start address is not aligned - 0x%08x\n",
-                     __func__, start);
+             debug("ERROR: %s - start address is not aligned - 0x%08x\n",
+                   __func__, start);
              /* move to next cache line */
              start = (start + line_len - 1) & ~(line_len - 1);
      }

@@ -77,8 +77,8 @@ static void v7_dcache_inval_range(u32 start, u32 stop, u32 line_len)
       * invalidate the last cache-line
       */
      if (stop & (line_len - 1)) {
-             printf("ERROR: %s - stop address is not aligned - 0x%08x\n",
-                     __func__, stop);
+             debug("ERROR: %s - stop address is not aligned - 0x%08x\n",
+                   __func__, stop);
              /* align to the beginning of this cache line */

Quoted 8 lines

arch/arm/cpu/armv7/cache_v7.c static void v7_dcache_inval_range(u32 start, u32 stop, u32 line_len) · line 84

Quoted 3 lines

arch/arm/cpu/armv7/cache_v7.c +0 -0

              stop &= ~(line_len - 1);
      }

--
Best regards,
Marek Vasut

Regards,
Simon

Quoted 1 line · Simon Glass

Hi Marek,

Hi!

Quoted 10 lines · Marek Vasut

Quoted 5 lines · Simon Glass

Quoted 1 line

Move these to debug() like the one in check_cache range(), to save SPL space.

This hides cache problems, which were visibly reported so far. I am opposed to this patch.

Sure, but see check_cache_range(). It uses debug(). In fact I found the at91 cache problem only after trying #define DEBUG in the code there.

Which is the reason we should really be vocal about such cache misuse. I had a few of such cache problems bite me too, which is why I would like to avoid silencing this warning with debug() by default.

I think check_cache_range() should also be fixed and should use printf() by default.

Quoted 1 line

Quoted 1 line

Quoted 5 lines

Quoted 2 lines

Wouldn't it make more sense to completely disable printf() and co. in SPL if you're after saving space?

Or maybe we need something that prints a message in U-Boot proper, but not SPL? I'll take a look.

But what if you trigger the issue only in SPL ?

Quoted 26 lines

Quoted 26 lines

Quoted 26 lines

Signed-off-by Simon Glass <[email protected]>

---

 arch/arm/cpu/armv7/cache_v7.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

arch/arm/cpu/armv7/cache_v7.c +2 -2

index dc309da..68cf62e 100644
@@ -66,8 +66,8 @@ static void v7_dcache_inval_range(u32 start, u32 stop, u32 line_len)
       * invalidate the first cache-line
       */
      if (start & (line_len - 1)) {
-             printf("ERROR: %s - start address is not aligned - 0x%08x\n",
-                     __func__, start);
+             debug("ERROR: %s - start address is not aligned - 0x%08x\n",
+                   __func__, start);
              /* move to next cache line */
              start = (start + line_len - 1) & ~(line_len - 1);
      }

@@ -77,8 +77,8 @@ static void v7_dcache_inval_range(u32 start, u32 stop, u32 line_len)
       * invalidate the last cache-line
       */

Quoted 8 lines

Quoted 8 lines

arch/arm/cpu/armv7/cache_v7.c static void v7_dcache_inval_range(u32 start, u32 stop, u32 line_len) · line 84

Quoted 8 lines

arch/arm/cpu/armv7/cache_v7.c +2 -2

      if (stop & (line_len - 1)) {
-             printf("ERROR: %s - stop address is not aligned - 0x%08x\n",
-                     __func__, stop);
+             debug("ERROR: %s - stop address is not aligned - 0x%08x\n",
+                   __func__, stop);
              /* align to the beginning of this cache line */
              stop &= ~(line_len - 1);
      }

Hi Marek,

Quoted 27 lines · Marek Vasut

Quoted 1 line · Simon Glass

Hi Marek,

Hi!

Quoted 10 lines · Marek Vasut

Quoted 5 lines · Simon Glass

Quoted 1 line

Move these to debug() like the one in check_cache range(), to save SPL space.

This hides cache problems, which were visibly reported so far. I am opposed to this patch.

Sure, but see check_cache_range(). It uses debug(). In fact I found the at91 cache problem only after trying #define DEBUG in the code there.

Which is the reason we should really be vocal about such cache misuse. I had a few of such cache problems bite me too, which is why I would like to avoid silencing this warning with debug() by default.

I think check_cache_range() should also be fixed and should use printf() by default.

Quoted 4 lines

Quoted 3 lines

Wouldn't it make more sense to completely disable printf() and co. in SPL if you're after saving space?

Quoted 4 lines

Quoted 2 lines

Or maybe we need something that prints a message in U-Boot proper, but not SPL? I'll take a look.

But what if you trigger the issue only in SPL ?

Yes, but is that likely? So far I don't think the cache is enabled in SPL...

Quoted 26 lines

Quoted 26 lines

Quoted 26 lines

Quoted 26 lines

Signed-off-by Simon Glass <[email protected]>

---

 arch/arm/cpu/armv7/cache_v7.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

arch/arm/cpu/armv7/cache_v7.c +2 -2

index dc309da..68cf62e 100644
@@ -66,8 +66,8 @@ static void v7_dcache_inval_range(u32 start, u32 stop, u32 line_len)
       * invalidate the first cache-line
       */
      if (start & (line_len - 1)) {
-             printf("ERROR: %s - start address is not aligned - 0x%08x\n",
-                     __func__, start);
+             debug("ERROR: %s - start address is not aligned - 0x%08x\n",
+                   __func__, start);
              /* move to next cache line */
              start = (start + line_len - 1) & ~(line_len - 1);
      }

@@ -77,8 +77,8 @@ static void v7_dcache_inval_range(u32 start, u32 stop, u32 line_len)
       * invalidate the last cache-line
       */

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

arch/arm/cpu/armv7/cache_v7.c static void v7_dcache_inval_range(u32 start, u32 stop, u32 line_len) · line 84

Quoted 8 lines

arch/arm/cpu/armv7/cache_v7.c +2 -2

      if (stop & (line_len - 1)) {
-             printf("ERROR: %s - stop address is not aligned - 0x%08x\n",
-                     __func__, stop);
+             debug("ERROR: %s - stop address is not aligned - 0x%08x\n",
+                   __func__, stop);
              /* align to the beginning of this cache line */
              stop &= ~(line_len - 1);
      }

Regards,
Simon

Quoted 33 lines · Simon Glass

Hi Marek,

Quoted 30 lines · Marek Vasut

Quoted 1 line · Simon Glass

Hi Marek,

Hi!

Quoted 10 lines · Marek Vasut

Quoted 5 lines · Simon Glass

Quoted 1 line

Move these to debug() like the one in check_cache range(), to save SPL space.

This hides cache problems, which were visibly reported so far. I am opposed to this patch.

Sure, but see check_cache_range(). It uses debug(). In fact I found the at91 cache problem only after trying #define DEBUG in the code there.

Which is the reason we should really be vocal about such cache misuse. I had a few of such cache problems bite me too, which is why I would like to avoid silencing this warning with debug() by default.

I think check_cache_range() should also be fixed and should use printf() by default.

Quoted 6 lines

Quoted 3 lines

Wouldn't it make more sense to completely disable printf() and co. in SPL if you're after saving space?

Or maybe we need something that prints a message in U-Boot proper, but not SPL? I'll take a look.

Quoted 3 lines

Quoted 1 line

But what if you trigger the issue only in SPL ?

Yes, but is that likely? So far I don't think the cache is enabled in SPL...

Yeah, it's probably unlikely.

btw have you tried patching away all console IO support in SPL? Does it save space?

We need a correct name (rk3288, rk3036) so check this to avoid a crash later.

Signed-off-by Simon Glass <[email protected]>

---

 tools/rkimage.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

While we consider whether to drop use of DT in SPL, remove some unwanted properties. This reduces SPL size by about 250 bytes.

Signed-off-by Simon Glass <[email protected]>

---

 configs/firefly-rk3288_defconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Enable this option to correct display artifacts when a write-back cache is in use.

Signed-off-by Simon Glass <[email protected]>

---

 drivers/video/rockchip/rk_vop.c | 1 +
 1 file changed, 1 insertion(+)

This is not currently used and saves a little over 1KB of SPL image size.

Signed-off-by Simon Glass <[email protected]>

---

 include/configs/rk3288_common.h | 1 -
 1 file changed, 1 deletion(-)

Update the MMC environment code so that it works with driver-model enabled for block devices.

Signed-off-by Simon Glass <[email protected]>

---

 common/env_mmc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Update sdhci.c so that it works with driver model enabled for block devices.

Signed-off-by Simon Glass <[email protected]>

---

 drivers/mmc/sdhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

This code does not currently build with driver model enabled for block devices. Update it to correct this.

Signed-off-by Simon Glass <[email protected]>

Quoted 3 lines · Simon Glass

This code does not currently build with driver model enabled for block devices. Update it to correct this.

Quoted 1 line

Signed-off-by Simon Glass <[email protected]>

Reviewed-by Alexander Graf <[email protected]>

Alex

Allow driver model to be used for block devices in SPL.

Signed-off-by Simon Glass <[email protected]>

---

 common/spl/spl_mmc.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Add support for using driver model for block devices in this driver.

Signed-off-by Simon Glass <[email protected]>

---

 drivers/mmc/dw_mmc.c | 42 ++++++++++++++++++++++++++++--------------
 include/dwmmc.h      |  7 ++++++-
 2 files changed, 34 insertions(+), 15 deletions(-)

Allow driver model to be used for block devices in the rockchip mmc driver.

Signed-off-by Simon Glass <[email protected]>

---

 drivers/mmc/rockchip_dw_mmc.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

When these functions are not compiled in, we still need to declare the correct function signature to avoid a build warnings in SPL. Fix this.

Signed-off-by Simon Glass <[email protected]>

---

 drivers/mmc/mmc_private.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

This small change tidies up the code slightly.

Signed-off-by Simon Glass <[email protected]>

---

 drivers/mmc/mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Enable CONFIG_BLK to move to using driver model for block devices. This affects MMC booting in SPL, as well as MMC access in U-Boot proper.

Signed-off-by Simon Glass <[email protected]>

---

 arch/arm/mach-rockchip/Kconfig | 3 +++
 1 file changed, 3 insertions(+)

This series expands the CONFIG_BLK support to SPL, fixes up the EFI boot code and adds a few other tweaks so that all rockchip boards can move to using driver model for block devices.

It also introduces snprintf() in the tiny-printf code and tidies up the mmc code a little.

Overall the code size for firefly-rk3288 drops by 1KB. This is mostly due to removing GPIO support and cutting down on unnecessary strings. The move to CONFIG_BLK unfortunately adds 0.5KB, or the improvement would be greater.

Simon Glass (20):
tiny-printf: Tidy up a few nits
tiny-printf: Support snprintf()
reset: Drop the reset failure message
mmc: Drop mmc_register()
mmc: Drop dead mmc code for non-generic MMC
mmc: Use byte array for multipliers
arm: Avoid error messages in cache_v7
rockchip: Check image name for the rksd image
rockchip: Drop unnecessary SPL properties
rockchip: video: Flush the cache when the display is updated
rockchip: Drop SPL GPIO support for rk3288
dm: env: mmc: Convert env_mmc to support CONFIG_BLK
dm: mmc: Convert sdhci to support CONFIG_BLK
dm: efi: Update for CONFIG_BLK
dm: mmc: spl: Add support for CONFIG_BLK
dm: mmc: dwmmc: Support CONFIG_BLK
dm: rockchip: mmc: Allow use of CONFIG_BLK
dm: mmc: Fix up mmc_bread/bwrite() prototypes for SPL
dm: mmc: Use cfg directly in mmc_bind()
dm: rockchip: Enable CONFIG_BLK

 arch/arm/cpu/armv7/cache_v7.c    |  8 +++---
 arch/arm/mach-rockchip/Kconfig   |  3 ++
 cmd/mmc.c                        | 62 ----------------------------------------
 common/env_mmc.c                 |  8 +++---
 common/spl/spl_mmc.c             |  9 +++---
 configs/firefly-rk3288_defconfig |  2 +-
 drivers/misc/reset-uclass.c      |  2 +-
 drivers/mmc/dw_mmc.c             | 42 ++++++++++++++++++---------
 drivers/mmc/mmc.c                | 13 ++-------
 drivers/mmc/mmc_private.h        | 14 +++++++++
 drivers/mmc/rockchip_dw_mmc.c    | 31 ++++++++++++++++++++
 drivers/mmc/sdhci.c              |  2 +-
 drivers/video/rockchip/rk_vop.c  |  1 +

 include/configs/rk3288_common.h  |  1 -
 include/dwmmc.h                  |  7 ++++-
 include/efi_loader.h             |  2 +-
 include/mmc.h                    |  5 ----
 lib/efi_loader/efi_disk.c        | 61 +++++++++++++++++++++++++++++----------
 lib/tiny-printf.c                | 43 +++++++++++++++++++++-------
 tools/rkimage.c                  |  7 +----
 20 files changed, 181 insertions(+), 142 deletions(-)