[U-Boot] [PATCH 1/3] vsprintf: Support phys_addr_t specifier unconditionally

[U-Boot] [PATCH 1/3] vsprintf: Support phys_addr_t specifier unconditionally

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

When phys_addr_t printf specifier support was first introduced in commit
1eebd14b7902 ("vsprintf: Add modifier for phys_addr_t"), it was enabled
only if CONFIG_CMD_NET was selected. Since physical addresses are not
unique to networking support it doesn't make sense to conditionally add
it in those cases only. Move support for it outside of the CMD_NET guard
so that the specifier is always supported.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 lib/vsprintf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 1b6c154d8d72..2403825dc98d 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -457,7 +457,6 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 		return device_path_string(buf, end, ptr, field_width,
 					  precision, flags);
 #endif
-#ifdef CONFIG_CMD_NET
 	case 'a':
 		flags |= SPECIAL | ZEROPAD;
 
@@ -469,6 +468,7 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 			break;
 		}
 		break;
+#ifdef CONFIG_CMD_NET
 	case 'm':
 		flags |= SPECIAL;
 		/* Fallthrough */
-- 
2.20.1

[U-Boot] [PATCH 2/3] sandbox: Use correct phys_{addr, size}_t for PHYS_64BIT=y

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

If 64-bit physical addresses support is enabled, make sure the sandox
defines the correct types for phys_addr_t and phys_size_t.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 arch/sandbox/include/asm/types.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/sandbox/include/asm/types.h b/arch/sandbox/include/asm/types.h
index 7cd56b41474f..c1a5d2af8285 100644
--- a/arch/sandbox/include/asm/types.h
+++ b/arch/sandbox/include/asm/types.h
@@ -18,21 +18,21 @@ typedef unsigned short umode_t;
 /*
  * Number of bits in a C 'long' on this architecture.
  */
-#ifdef	CONFIG_PHYS64
+#ifdef	CONFIG_PHYS_64BIT
 #define BITS_PER_LONG 64
-#else	/* CONFIG_PHYS64 */
+#else	/* CONFIG_PHYS_64BIT */
 #define BITS_PER_LONG 32
-#endif	/* CONFIG_PHYS64 */
+#endif	/* CONFIG_PHYS_64BIT */
 
-#ifdef	CONFIG_PHYS64
+#ifdef	CONFIG_PHYS_64BIT
 typedef unsigned long long dma_addr_t;
 typedef u64 phys_addr_t;
 typedef u64 phys_size_t;
-#else	/* CONFIG_PHYS64 */
+#else	/* CONFIG_PHYS_64BIT */
 typedef unsigned long dma_addr_t;
 typedef u32 phys_addr_t;
 typedef u32 phys_size_t;
-#endif	/* CONFIG_PHYS64 */
+#endif	/* CONFIG_PHYS_64BIT */
 
 #endif /* __KERNEL__ */
 
-- 
2.20.1

[U-Boot] [PATCH 3/3] sandbox: Properly print physical addresses

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Use the %pap printf specifier to print physical addresses. The physical
address is passed by reference and hence avoids the need to play tricks
with the preprocessor to use the correct specifier.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 arch/sandbox/lib/pci_io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/sandbox/lib/pci_io.c b/arch/sandbox/lib/pci_io.c
index 5039973cd7af..01822c606956 100644
--- a/arch/sandbox/lib/pci_io.c
+++ b/arch/sandbox/lib/pci_io.c
@@ -34,7 +34,7 @@ int pci_map_physmem(phys_addr_t paddr, unsigned long *lenp,
 		return 0;
 	}
 
-	debug("%s: failed: addr=%x\n", __func__, paddr);
+	debug("%s: failed: addr=%pap\n", __func__, &paddr);
 	return -ENOSYS;
 }
 
-- 
2.20.1

[U-Boot] [PATCH 1/3] vsprintf: Support phys_addr_t specifier unconditionally

[Simon Glass]

On Tue, 12 Mar 2019 at 04:38, Thierry Reding <thierry.reding@gmail.com> wrote:
>
> From: Thierry Reding <treding@nvidia.com>
>
> When phys_addr_t printf specifier support was first introduced in commit
> 1eebd14b7902 ("vsprintf: Add modifier for phys_addr_t"), it was enabled
> only if CONFIG_CMD_NET was selected. Since physical addresses are not
> unique to networking support it doesn't make sense to conditionally add
> it in those cases only. Move support for it outside of the CMD_NET guard
> so that the specifier is always supported.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  lib/vsprintf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Is there a code-size impact here? Perhaps instead it should be a new
Kconfig option, implied by CMD_NET?

Reviewed-by: Simon Glass <sjg@chromium.org>

Regards,
Simon

[U-Boot] [PATCH 1/3] vsprintf: Support phys_addr_t specifier unconditionally

[Thierry Reding]

On Sat, Mar 30, 2019 at 03:19:27PM -0600, Simon Glass wrote:
> On Tue, 12 Mar 2019 at 04:38, Thierry Reding <thierry.reding@gmail.com> wrote:
> >
> > From: Thierry Reding <treding@nvidia.com>
> >
> > When phys_addr_t printf specifier support was first introduced in commit
> > 1eebd14b7902 ("vsprintf: Add modifier for phys_addr_t"), it was enabled
> > only if CONFIG_CMD_NET was selected. Since physical addresses are not
> > unique to networking support it doesn't make sense to conditionally add
> > it in those cases only. Move support for it outside of the CMD_NET guard
> > so that the specifier is always supported.
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  lib/vsprintf.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Is there a code-size impact here? Perhaps instead it should be a new
> Kconfig option, implied by CMD_NET?

There's a tiny code-size impact needed for the extra "case" statement,
etc. But it shouldn't be more than maybe a handful of instructions. I
don't think we can reasonable guard this by a Kconfig option because
this specifier is used to print physical addresses, which can happen
pretty much anywhere. A quick grep on a recent master shows that this
is used in a number of different drivers:

    $ git grep -n '".*%pa.*"'
    arch/arm/lib/cache-cp15.c:66:   debug("%s: start=%pa, size=%zu, option=%llx\n", __func__, &start, size,
    arch/arm/lib/cache-cp15.c:69:   debug("%s: start=%pa, size=%zu, option=0x%x\n", __func__, &start, size,
    arch/arm/lib/cache.c:84:        debug("mapping memory %pa-%pa non-cached\n", &start, &end);
    arch/arm/lib/cache.c:102:       debug("allocated %zu bytes of uncached memory @%pa\n", size, &next);
    drivers/pinctrl/pinctrl-single.c:52:                    dev_dbg(dev, "  invalid register offset 0x%pa\n", &reg);
    drivers/pinctrl/pinctrl-single.c:69:            dev_dbg(dev, "  reg/val 0x%pa/0x%08x\n", &reg, val);
    drivers/spi/fsl_dspi.c:653:     debug("DSPI: regs=%pa, max-frequency=%d, endianess=%s, num-cs=%d\n",
    drivers/video/tegra.c:330:      debug("LCD frame buffer at %pa, size %x\n", &priv->frame_buffer,

Guarding the specifier with a Kconfig symbol would require extending the
list of dependencies everytime someone else starts using it, which means
that people (i.e. reviewers) actually have to know about this, remember
it and very closely look at the printf format string to detect whether a
%pa is used or not.

Note also that while the above seems like only a small number of users,
there are potentially a lot more that actually print out a physical
address but use some other specifier to do so. You only really need the
%pa if you need to support code across platforms where phys_addr_t has a
different size.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190401/7c7e9d52/attachment.sig>

[U-Boot] [PATCH 2/3] sandbox: Use correct phys_{addr, size}_t for PHYS_64BIT=y

[Simon Glass]

On Tue, 12 Mar 2019 at 04:38, Thierry Reding <thierry.reding@gmail.com> wrote:
>
> From: Thierry Reding <treding@nvidia.com>
>
> If 64-bit physical addresses support is enabled, make sure the sandox
> defines the correct types for phys_addr_t and phys_size_t.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  arch/sandbox/include/asm/types.h | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>

Applied to u-boot-dm, thanks!