* [PATCH 1/4] remoteproc: Remove unused function rproc_elf_sanity_check
2020-09-13 0:28 [PATCH 0/4] dm: Print device name in dev_xxx() like Linux Sean Anderson
@ 2020-09-13 0:28 ` Sean Anderson
2020-09-17 1:09 ` Simon Glass
2020-09-13 0:28 ` [PATCH 2/4] net: mdio: Supply appropriate devices for dev_dgb calls Sean Anderson
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Sean Anderson @ 2020-09-13 0:28 UTC (permalink / raw)
To: u-boot
This function is never used anywhere, and it also tries to log with a
nonexistant device.
Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
drivers/remoteproc/rproc-elf-loader.c | 16 ----------------
include/remoteproc.h | 13 -------------
2 files changed, 29 deletions(-)
diff --git a/drivers/remoteproc/rproc-elf-loader.c b/drivers/remoteproc/rproc-elf-loader.c
index c464ecebb7..b185a6cafb 100644
--- a/drivers/remoteproc/rproc-elf-loader.c
+++ b/drivers/remoteproc/rproc-elf-loader.c
@@ -158,22 +158,6 @@ int rproc_elf64_sanity_check(ulong addr, ulong size)
return 0;
}
-/* Basic function to verify ELF image format */
-int rproc_elf_sanity_check(ulong addr, ulong size)
-{
- Elf32_Ehdr *ehdr = (Elf32_Ehdr *)addr;
-
- if (!addr) {
- dev_err(dev, "Invalid firmware address\n");
- return -EFAULT;
- }
-
- if (ehdr->e_ident[EI_CLASS] == ELFCLASS64)
- return rproc_elf64_sanity_check(addr, size);
- else
- return rproc_elf32_sanity_check(addr, size);
-}
-
int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size)
{
Elf32_Ehdr *ehdr; /* Elf header structure pointer */
diff --git a/include/remoteproc.h b/include/remoteproc.h
index a903acb9b2..74d01723f6 100644
--- a/include/remoteproc.h
+++ b/include/remoteproc.h
@@ -226,19 +226,6 @@ int rproc_elf32_sanity_check(ulong addr, ulong size);
*/
int rproc_elf64_sanity_check(ulong addr, ulong size);
-/**
- * rproc_elf_sanity_check() - Verify if an image is a valid ELF one
- *
- * Check if a valid ELF image exists at the given memory location. Auto
- * detects ELF32/ELF64 and verifies basic ELF64/ELF32 format requirements
- * like magic number and sections size.
- *
- * @addr: address of the image to verify
- * @size: size of the image
- * @return 0 if the image looks good, else appropriate error value.
- */
-int rproc_elf_sanity_check(ulong addr, ulong size);
-
/**
* rproc_elf32_load_image() - load an ELF32 image
* @dev: device loading the ELF32 image
--
2.28.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/4] net: mdio: Supply appropriate devices for dev_dgb calls
2020-09-13 0:28 [PATCH 0/4] dm: Print device name in dev_xxx() like Linux Sean Anderson
2020-09-13 0:28 ` [PATCH 1/4] remoteproc: Remove unused function rproc_elf_sanity_check Sean Anderson
@ 2020-09-13 0:28 ` Sean Anderson
2020-09-17 1:09 ` Simon Glass
2020-09-13 0:28 ` [PATCH 3/4] dm: Use symbolic constants for log levels in dev_xxx() Sean Anderson
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Sean Anderson @ 2020-09-13 0:28 UTC (permalink / raw)
To: u-boot
The name of the device we are working on is `ethdev` and not just `dev`.
Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
net/mdio-uclass.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/mdio-uclass.c b/net/mdio-uclass.c
index 66ee2e1976..b5e8e46512 100644
--- a/net/mdio-uclass.c
+++ b/net/mdio-uclass.c
@@ -145,7 +145,7 @@ static struct phy_device *dm_eth_connect_phy_handle(struct udevice *ethdev,
break;
if (!ofnode_valid(phandle.node)) {
- dev_dbg(dev, "can't find PHY node\n");
+ dev_dbg(ethdev, "can't find PHY node\n");
return NULL;
}
@@ -161,7 +161,7 @@ static struct phy_device *dm_eth_connect_phy_handle(struct udevice *ethdev,
if (uclass_get_device_by_ofnode(UCLASS_MDIO,
ofnode_get_parent(phandle.node),
&mdiodev)) {
- dev_dbg(dev, "can't find MDIO bus for node %s\n",
+ dev_dbg(ethdev, "can't find MDIO bus for node %s\n",
ofnode_get_name(ofnode_get_parent(phandle.node)));
return NULL;
}
--
2.28.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/4] net: mdio: Supply appropriate devices for dev_dgb calls
2020-09-13 0:28 ` [PATCH 2/4] net: mdio: Supply appropriate devices for dev_dgb calls Sean Anderson
@ 2020-09-17 1:09 ` Simon Glass
2020-09-17 1:40 ` Sean Anderson
0 siblings, 1 reply; 11+ messages in thread
From: Simon Glass @ 2020-09-17 1:09 UTC (permalink / raw)
To: u-boot
On Sat, 12 Sep 2020 at 18:28, Sean Anderson <seanga2@gmail.com> wrote:
>
> The name of the device we are working on is `ethdev` and not just `dev`.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
> net/mdio-uclass.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
BTW I really like using 'dev' for the struct udevice if there is only
one, and other names for other things (e.g. edev for Ethernet, udev
for USB).
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/4] net: mdio: Supply appropriate devices for dev_dgb calls
2020-09-17 1:09 ` Simon Glass
@ 2020-09-17 1:40 ` Sean Anderson
2020-09-17 3:44 ` Simon Glass
0 siblings, 1 reply; 11+ messages in thread
From: Sean Anderson @ 2020-09-17 1:40 UTC (permalink / raw)
To: u-boot
On 9/16/20 9:09 PM, Simon Glass wrote:
> On Sat, 12 Sep 2020 at 18:28, Sean Anderson <seanga2@gmail.com> wrote:
>>
>> The name of the device we are working on is `ethdev` and not just `dev`.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> ---
>>
>> net/mdio-uclass.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> BTW I really like using 'dev' for the struct udevice if there is only
> one, and other names for other things (e.g. edev for Ethernet, udev
> for USB).
>
Yeah, I think it is only really necessary when disambiguating between
several different devices. However, I am trying to make these patches as
minimal as possible, so I would like to keep the variable names the same
where possible.
--Sean
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/4] net: mdio: Supply appropriate devices for dev_dgb calls
2020-09-17 1:40 ` Sean Anderson
@ 2020-09-17 3:44 ` Simon Glass
0 siblings, 0 replies; 11+ messages in thread
From: Simon Glass @ 2020-09-17 3:44 UTC (permalink / raw)
To: u-boot
Hi Sean,
On Wed, 16 Sep 2020 at 19:40, Sean Anderson <seanga2@gmail.com> wrote:
>
>
> On 9/16/20 9:09 PM, Simon Glass wrote:
> > On Sat, 12 Sep 2020 at 18:28, Sean Anderson <seanga2@gmail.com> wrote:
> >>
> >> The name of the device we are working on is `ethdev` and not just `dev`.
> >>
> >> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> >> ---
> >>
> >> net/mdio-uclass.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> >
> > BTW I really like using 'dev' for the struct udevice if there is only
> > one, and other names for other things (e.g. edev for Ethernet, udev
> > for USB).
> >
>
> Yeah, I think it is only really necessary when disambiguating between
> several different devices. However, I am trying to make these patches as
> minimal as possible, so I would like to keep the variable names the same
> where possible.
Yes definitely better the way you have the patches now.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/4] dm: Use symbolic constants for log levels in dev_xxx()
2020-09-13 0:28 [PATCH 0/4] dm: Print device name in dev_xxx() like Linux Sean Anderson
2020-09-13 0:28 ` [PATCH 1/4] remoteproc: Remove unused function rproc_elf_sanity_check Sean Anderson
2020-09-13 0:28 ` [PATCH 2/4] net: mdio: Supply appropriate devices for dev_dgb calls Sean Anderson
@ 2020-09-13 0:28 ` Sean Anderson
2020-09-17 1:09 ` Simon Glass
2020-09-13 0:28 ` [PATCH 4/4] dm: Print device name in dev_xxx() like Linux Sean Anderson
2020-09-13 1:43 ` [PATCH 0/4] " Sean Anderson
4 siblings, 1 reply; 11+ messages in thread
From: Sean Anderson @ 2020-09-13 0:28 UTC (permalink / raw)
To: u-boot
This substitutes literal log levels with their symbolic constants.
Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
include/dm/device_compat.h | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/include/dm/device_compat.h b/include/dm/device_compat.h
index 3d8cd09f4c..7c14aa464d 100644
--- a/include/dm/device_compat.h
+++ b/include/dm/device_compat.h
@@ -10,6 +10,7 @@
#ifndef _DM_DEVICE_COMPAT_H
#define _DM_DEVICE_COMPAT_H
+#include <log.h>
#include <linux/compat.h>
/*
@@ -48,28 +49,28 @@
})
#define dev_emerg(dev, fmt, ...) \
- __dev_printk(0, dev, fmt, ##__VA_ARGS__)
+ __dev_printk(LOGL_EMERG, dev, fmt, ##__VA_ARGS__)
#define dev_alert(dev, fmt, ...) \
- __dev_printk(1, dev, fmt, ##__VA_ARGS__)
+ __dev_printk(LOGL_ALERT, dev, fmt, ##__VA_ARGS__)
#define dev_crit(dev, fmt, ...) \
- __dev_printk(2, dev, fmt, ##__VA_ARGS__)
+ __dev_printk(LOGL_CRIT, dev, fmt, ##__VA_ARGS__)
#define dev_err(dev, fmt, ...) \
- __dev_printk(3, dev, fmt, ##__VA_ARGS__)
+ __dev_printk(LOGL_ERR, dev, fmt, ##__VA_ARGS__)
#define dev_warn(dev, fmt, ...) \
- __dev_printk(4, dev, fmt, ##__VA_ARGS__)
+ __dev_printk(LOGL_WARNING, dev, fmt, ##__VA_ARGS__)
#define dev_notice(dev, fmt, ...) \
- __dev_printk(5, dev, fmt, ##__VA_ARGS__)
+ __dev_printk(LOGL_NOTICE, dev, fmt, ##__VA_ARGS__)
#define dev_info(dev, fmt, ...) \
- __dev_printk(6, dev, fmt, ##__VA_ARGS__)
+ __dev_printk(LOGL_INFO, dev, fmt, ##__VA_ARGS__)
#ifdef DEBUG
#define dev_dbg(dev, fmt, ...) \
- __dev_printk(7, dev, fmt, ##__VA_ARGS__)
+ __dev_printk(LOGL_DEBUG, dev, fmt, ##__VA_ARGS__)
#else
#define dev_dbg(dev, fmt, ...) \
({ \
if (0) \
- __dev_printk(7, dev, fmt, ##__VA_ARGS__); \
+ __dev_printk(LOGL_DEBUG, dev, fmt, ##__VA_ARGS__); \
})
#endif
@@ -79,7 +80,7 @@
#define dev_vdbg(dev, fmt, ...) \
({ \
if (0) \
- __dev_printk(7, dev, fmt, ##__VA_ARGS__); \
+ __dev_printk(LOGL_DEBUG, dev, fmt, ##__VA_ARGS__); \
})
#endif
--
2.28.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 4/4] dm: Print device name in dev_xxx() like Linux
2020-09-13 0:28 [PATCH 0/4] dm: Print device name in dev_xxx() like Linux Sean Anderson
` (2 preceding siblings ...)
2020-09-13 0:28 ` [PATCH 3/4] dm: Use symbolic constants for log levels in dev_xxx() Sean Anderson
@ 2020-09-13 0:28 ` Sean Anderson
2020-09-13 1:43 ` [PATCH 0/4] " Sean Anderson
4 siblings, 0 replies; 11+ messages in thread
From: Sean Anderson @ 2020-09-13 0:28 UTC (permalink / raw)
To: u-boot
This adorns messages generated by dev_xxx() with the device and driver
names. It also redirects dev_xxx() to log() when it is available. The names
of these functions very roughly take inspiration from Linux, but there is
no deeper correlation.
Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
include/dm/device_compat.h | 58 ++++++++++++++++++--------------------
1 file changed, 28 insertions(+), 30 deletions(-)
diff --git a/include/dm/device_compat.h b/include/dm/device_compat.h
index 7c14aa464d..5d18eb68fa 100644
--- a/include/dm/device_compat.h
+++ b/include/dm/device_compat.h
@@ -1,5 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0+ */
/*
+ * Copyright (C) 2020 Sean Anderson <seanga2@gmail.com>
* Copyright (c) 2013 Google, Inc
*
* (C) Copyright 2012
@@ -33,19 +34,33 @@
#undef dev_warn
#endif
-/*
- * REVISIT:
- * print device name like Linux
- */
-#define dev_printk(dev, fmt, ...) \
-({ \
- printk(fmt, ##__VA_ARGS__); \
+#ifdef VERBOSE_DEBUG
+#define _VERBOSE_DEBUG 1
+#else
+#define _VERBOSE_DEBUG 0
+#endif
+
+#define dev_printk_emit(cat, level, fmt, ...) \
+({ \
+ if ((_DEBUG && level == LOGL_DEBUG) || \
+ (_VERBOSE_DEBUG && level == LOGL_DEBUG_CONTENT)) \
+ printf(fmt, ##__VA_ARGS__); \
+ else if (CONFIG_IS_ENABLED(LOG)) \
+ log(cat, level, fmt, ##__VA_ARGS__); \
+ else if (level < CONFIG_VAL(LOGLEVEL)) \
+ printf(fmt, ##__VA_ARGS__); \
})
-#define __dev_printk(level, dev, fmt, ...) \
-({ \
- if (level < CONFIG_VAL(LOGLEVEL)) \
- dev_printk(dev, fmt, ##__VA_ARGS__); \
+#define __dev_printk(level, dev, fmt, ...) \
+({ \
+ struct udevice *__dev = dev; \
+ if (__dev) \
+ dev_printk_emit(__dev->driver->id, level, "%s %s: " fmt, \
+ __dev->name, __dev->driver->name, \
+ ##__VA_ARGS__); \
+ else \
+ dev_printk_emit(LOG_CATEGORY, level, \
+ "(NULL udevice *): " fmt, ##__VA_ARGS__); \
})
#define dev_emerg(dev, fmt, ...) \
@@ -62,26 +77,9 @@
__dev_printk(LOGL_NOTICE, dev, fmt, ##__VA_ARGS__)
#define dev_info(dev, fmt, ...) \
__dev_printk(LOGL_INFO, dev, fmt, ##__VA_ARGS__)
-
-#ifdef DEBUG
#define dev_dbg(dev, fmt, ...) \
__dev_printk(LOGL_DEBUG, dev, fmt, ##__VA_ARGS__)
-#else
-#define dev_dbg(dev, fmt, ...) \
-({ \
- if (0) \
- __dev_printk(LOGL_DEBUG, dev, fmt, ##__VA_ARGS__); \
-})
-#endif
-
-#ifdef VERBOSE_DEBUG
-#define dev_vdbg dev_dbg
-#else
-#define dev_vdbg(dev, fmt, ...) \
-({ \
- if (0) \
- __dev_printk(LOGL_DEBUG, dev, fmt, ##__VA_ARGS__); \
-})
-#endif
+#define dev_vdbg(dev, fmt, ...) \
+ __dev_printk(LOGL_DEBUG_CONTENT, dev, fmt, ##__VA_ARGS__)
#endif
--
2.28.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 0/4] dm: Print device name in dev_xxx() like Linux
2020-09-13 0:28 [PATCH 0/4] dm: Print device name in dev_xxx() like Linux Sean Anderson
` (3 preceding siblings ...)
2020-09-13 0:28 ` [PATCH 4/4] dm: Print device name in dev_xxx() like Linux Sean Anderson
@ 2020-09-13 1:43 ` Sean Anderson
4 siblings, 0 replies; 11+ messages in thread
From: Sean Anderson @ 2020-09-13 1:43 UTC (permalink / raw)
To: u-boot
On 9/12/20 8:28 PM, Sean Anderson wrote:
> This series adds some additional information to dev_dbg. With this series, and
> the following options applied in addition to sandbox_defconfig
>
> CONFIG_LOGLEVEL=8
> CONFIG_LOG=n
> CONFIG_CMD_LOG=n
> CONFIG_CMD_TPM=n
>
> the output of U-Boot is
>
> U-Boot 2020.10-rc4-00006-g19b641dddd-dirty (Sep 12 2020 - 18:15:00 -0400)
>
> Model: sandbox
> DRAM: 128 MiB
> pinctrl-gpio sandbox_pinctrl_gpio: set_state_simple op missing
> leds gpio_led: set_state_simple op missing
> default_on gpio_led: set_state_simple op missing
> base-gpios sandbox_gpio: set_state_simple op missing
> default_off gpio_led: set_state_simple op missing
> wdt at 0 wdt_sandbox: set_state_simple op missing
> WDT: Started with servicing (60s timeout)
> MMC: mmc0 mmc_sandbox: set_state_simple op missing
> mmc1 mmc_sandbox: set_state_simple op missing
> mmc2 mmc_sandbox: set_state_simple op missing
> mmc2: 2 (SD), mmc1: 1 (SD), mmc0: 0 (SD)
> lcd sandbox_lcd_sdl: set_state_simple op missing
> In: serial
> Out: vidconsole
> Err: vidconsole
> Model: sandbox
> hog_input_active_low gpio_hog: set_state_simple op missing
> hog_input_active_high gpio_hog: set_state_simple op missing
> hog_output_low gpio_hog: set_state_simple op missing
> hog_output_high gpio_hog: set_state_simple op missing
> cros-ec google_cros_ec_sandbox: set_state_simple op missing
> SCSI:
> pci_ep pci_ep_sandbox: set_state_simple op missing
> Net: eth at 10002000 eth_sandbox: set_state_simple op missing
> eth0: eth at 10002000eth@10003000 eth_sandbox: set_state_simple op missing
> , eth5: eth at 10003000sbe5 eth_sandbox: set_state_simple op missing
> , eth3: sbe5eth at 10004000 eth_sandbox: set_state_simple op missing
> , eth6: eth at 10004000
> bootcount at 0 bootcount-rtc: set_state_simple op missing
> i2c at 0 i2c_sandbox: set_state_simple op missing
> rtc at 61 rtc-sandbox: set_state_simple op missing
> emul i2c_emul_parent_drv: set_state_simple op missing
> emull sandbox_i2c_rtc_emul: set_state_simple op missing
> Hit any key to stop autoboot:
>
> When using CONFIG_LOG the output of U-Boot is too verbose to fit in this email.
> Note that there is a soft dependency on [1] if you would like to test this patch
> with CONFIG_LOG and a higher LOG_LEVEL than LOGL_INFO.
>
> [1] https://patchwork.ozlabs.org/project/uboot/list/?series=201343
>
>
> Sean Anderson (4):
> remoteproc: Remove unused function rproc_elf_sanity_check
> net: mdio: Supply appropriate devices for dev_dgb calls
> dm: Use symbolic constants for log levels in dev_xxx()
> dm: Print device name in dev_xxx() like Linux
>
> drivers/remoteproc/rproc-elf-loader.c | 16 ------
> include/dm/device_compat.h | 75 +++++++++++++--------------
> include/remoteproc.h | 13 -----
> net/mdio-uclass.c | 4 +-
> 4 files changed, 39 insertions(+), 69 deletions(-)
>
In my haste I may have forgotten to run CI and make many more patches
like the first two to fix drivers calling a dev_ log function without an
actual device. So this series breaks a lot of things at the moment [1].
Consider it an RFC I guess for after I fix all those build failures.
--Sean
[1] https://dev.azure.com/seanga2/u-boot/_build/results?buildId=30&view=logs&j=7f2f73d6-a009-5260-5fdd-7738e0f76a89&t=97bcf3f1-e64d-5752-c0e9-95a663c62e27
^ permalink raw reply [flat|nested] 11+ messages in thread