From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Thu, 28 May 2020 08:32:17 -0600 Subject: [PATCH 1/5] dm: core: Fix devfdt_get_addr_ptr return value In-Reply-To: <20200527064140.21391-1-ovidiu.panait@windriver.com> References: <20200527064140.21391-1-ovidiu.panait@windriver.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Wed, 27 May 2020 at 00:45, Ovidiu Panait wrote: > > According to the description of devfdt_get_addr_ptr, this function should > return NULL on failure, but currently it returns (void *)FDT_ADDR_T_NONE. > > This is also a problem because there are two definitions for > dev_read_addr_ptr, depending on CONFIG_DM_DEV_READ_INLINE: > > 1. one returning NULL on failure (drivers/core/read.c): > void *dev_read_addr_ptr(const struct udevice *dev) > { > fdt_addr_t addr = dev_read_addr(dev); > > return (addr == FDT_ADDR_T_NONE) ? NULL : map_sysmem(addr, 0); > } > > 2. another one, which is a wrapper over devfdt_get_addr_ptr, returning > (void *)FDT_ADDR_T_NONE (include/dm/read.h) > > static inline void *dev_read_addr_ptr(const struct udevice *dev) > { > return devfdt_get_addr_ptr(dev); > } > > Currently, some drivers which make use of devfdt_get_addr_ptr check the > return value for NULL: > drivers/i2c/mvtwsi.c > drivers/i2c/designware_i2c.c > drivers/usb/host/ehci-zynq.c > > while others check the return value for (void *)FDT_ADDR_T_NONE: > drivers/pinctrl/mvebu/pinctrl-mvebu.c > drivers/timer/ast_timer.c > drivers/watchdog/ast_wdt.c > > Fix this by making devfdt_get_addr_ptr return NULL on failure, as > described in the function comments. Also, update the drivers currently > checking (void *)FDT_ADDR_T_NONE to check for NULL. > > Signed-off-by: Ovidiu Panait > --- > > drivers/clk/aspeed/clk_ast2500.c | 4 ++-- > drivers/core/fdtaddr.c | 5 ++++- > drivers/i2c/ast_i2c.c | 4 ++-- > drivers/pinctrl/mvebu/pinctrl-mvebu.c | 2 +- > drivers/timer/ast_timer.c | 4 ++-- > drivers/watchdog/ast_wdt.c | 4 ++-- > 6 files changed, 13 insertions(+), 10 deletions(-) > +Sean Anderson I did already review another patch on this topic[1]. I'm not sure when that is going to land though. Perhaps this one should be rebased on that? Or Matthias can check with the other custodian to see what is happening with that series. I've sent a patch for a test which detects the inconsistency[2]. It is very easy to write a new test so I encourage people to do that whenever there is a fix like this. So with that: Tested on sandbox Tested-by: Simon Glass [1] http://patchwork.ozlabs.org/project/uboot/patch/20200521161503.384823-10-seanga2 at gmail.com/ [2] http://patchwork.ozlabs.org/project/uboot/patch/20200528082045.1.Ibd999a0382164a37d1e59c00c8c7a9ff92b8f53a at changeid/