All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] dm: core: Fix devfdt_get_addr_ptr return value
@ 2020-05-27  6:41 Ovidiu Panait
  2020-05-27  6:41 ` [PATCH 2/5] pinctrl: bcm283x: DM_FLAG_PRE_RELOC: Remove OF_CONTROL check Ovidiu Panait
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Ovidiu Panait @ 2020-05-27  6:41 UTC (permalink / raw)
  To: u-boot

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 <ovidiu.panait@windriver.com>
---

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

diff --git a/drivers/clk/aspeed/clk_ast2500.c b/drivers/clk/aspeed/clk_ast2500.c
index ccfeded30c..284f5f58d6 100644
--- a/drivers/clk/aspeed/clk_ast2500.c
+++ b/drivers/clk/aspeed/clk_ast2500.c
@@ -498,8 +498,8 @@ static int ast2500_clk_ofdata_to_platdata(struct udevice *dev)
 	struct ast2500_clk_priv *priv = dev_get_priv(dev);
 
 	priv->scu = devfdt_get_addr_ptr(dev);
-	if (IS_ERR(priv->scu))
-		return PTR_ERR(priv->scu);
+	if (!priv->scu)
+		return -EINVAL;
 
 	return 0;
 }
diff --git a/drivers/core/fdtaddr.c b/drivers/core/fdtaddr.c
index dfcb868f65..044d5871b0 100644
--- a/drivers/core/fdtaddr.c
+++ b/drivers/core/fdtaddr.c
@@ -14,6 +14,7 @@
 #include <log.h>
 #include <asm/io.h>
 #include <dm/device-internal.h>
+#include <mapmem.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -154,7 +155,9 @@ fdt_addr_t devfdt_get_addr(const struct udevice *dev)
 
 void *devfdt_get_addr_ptr(const struct udevice *dev)
 {
-	return (void *)(uintptr_t)devfdt_get_addr_index(dev, 0);
+	fdt_addr_t addr = devfdt_get_addr_index(dev, 0);
+
+	return (addr == FDT_ADDR_T_NONE) ? NULL : map_sysmem(addr, 0);
 }
 
 void *devfdt_remap_addr_index(const struct udevice *dev, int index)
diff --git a/drivers/i2c/ast_i2c.c b/drivers/i2c/ast_i2c.c
index 214362d04b..253e653666 100644
--- a/drivers/i2c/ast_i2c.c
+++ b/drivers/i2c/ast_i2c.c
@@ -93,8 +93,8 @@ static int ast_i2c_ofdata_to_platdata(struct udevice *dev)
 	int ret;
 
 	priv->regs = devfdt_get_addr_ptr(dev);
-	if (IS_ERR(priv->regs))
-		return PTR_ERR(priv->regs);
+	if (!priv->regs)
+		return -EINVAL;
 
 	ret = clk_get_by_index(dev, 0, &priv->clk);
 	if (ret < 0) {
diff --git a/drivers/pinctrl/mvebu/pinctrl-mvebu.c b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
index 2206e958ec..ac0377e796 100644
--- a/drivers/pinctrl/mvebu/pinctrl-mvebu.c
+++ b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
@@ -194,7 +194,7 @@ int mvebu_pinctl_probe(struct udevice *dev)
 	}
 
 	priv->base_reg = devfdt_get_addr_ptr(dev);
-	if (priv->base_reg == (void *)FDT_ADDR_T_NONE) {
+	if (!priv->base_reg) {
 		debug("%s: Failed to get base address\n", __func__);
 		return -EINVAL;
 	}
diff --git a/drivers/timer/ast_timer.c b/drivers/timer/ast_timer.c
index 3838601f54..9f28cbfcf9 100644
--- a/drivers/timer/ast_timer.c
+++ b/drivers/timer/ast_timer.c
@@ -65,8 +65,8 @@ static int ast_timer_ofdata_to_platdata(struct udevice *dev)
 	struct ast_timer_priv *priv = dev_get_priv(dev);
 
 	priv->regs = devfdt_get_addr_ptr(dev);
-	if (IS_ERR(priv->regs))
-		return PTR_ERR(priv->regs);
+	if (!priv->regs)
+		return -EINVAL;
 
 	priv->tmc = ast_get_timer_counter(priv->regs, AST_TICK_TIMER);
 
diff --git a/drivers/watchdog/ast_wdt.c b/drivers/watchdog/ast_wdt.c
index 7e11465a57..a21f9a4d14 100644
--- a/drivers/watchdog/ast_wdt.c
+++ b/drivers/watchdog/ast_wdt.c
@@ -91,8 +91,8 @@ static int ast_wdt_ofdata_to_platdata(struct udevice *dev)
 	struct ast_wdt_priv *priv = dev_get_priv(dev);
 
 	priv->regs = devfdt_get_addr_ptr(dev);
-	if (IS_ERR(priv->regs))
-		return PTR_ERR(priv->regs);
+	if (!priv->regs)
+		return -EINVAL;
 
 	return 0;
 }
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/5] pinctrl: bcm283x: DM_FLAG_PRE_RELOC: Remove OF_CONTROL check
  2020-05-27  6:41 [PATCH 1/5] dm: core: Fix devfdt_get_addr_ptr return value Ovidiu Panait
@ 2020-05-27  6:41 ` Ovidiu Panait
  2020-05-27  6:41 ` [PATCH 3/5] pinctrl: bcm283x: Read address from DT in ofdata_to_platdata Ovidiu Panait
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Ovidiu Panait @ 2020-05-27  6:41 UTC (permalink / raw)
  To: u-boot

Remove CONFIG_IS_ENABLED(OF_CONTROL) check from DM_FLAG_PRE_RELOC, since
this driver only supports OF_CONTROL.

drivers/pinctrl/broadcom/Kconfig:
config PINCTRL_BCM283X
    depends on ARCH_BCM283X && PINCTRL_FULL && OF_CONTROL

Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---

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

diff --git a/drivers/pinctrl/broadcom/pinctrl-bcm283x.c b/drivers/pinctrl/broadcom/pinctrl-bcm283x.c
index f44af6cf9a..8bf7916627 100644
--- a/drivers/pinctrl/broadcom/pinctrl-bcm283x.c
+++ b/drivers/pinctrl/broadcom/pinctrl-bcm283x.c
@@ -150,7 +150,7 @@ U_BOOT_DRIVER(pinctrl_bcm283x) = {
 	.priv_auto_alloc_size = sizeof(struct bcm283x_pinctrl_priv),
 	.ops		= &bcm283x_pinctrl_ops,
 	.probe		= bcm283x_pinctl_probe,
-#if !CONFIG_IS_ENABLED(OF_CONTROL) || CONFIG_IS_ENABLED(OF_BOARD)
+#if CONFIG_IS_ENABLED(OF_BOARD)
 	.flags		= DM_FLAG_PRE_RELOC,
 #endif
 };
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/5] pinctrl: bcm283x: Read address from DT in ofdata_to_platdata
  2020-05-27  6:41 [PATCH 1/5] dm: core: Fix devfdt_get_addr_ptr return value Ovidiu Panait
  2020-05-27  6:41 ` [PATCH 2/5] pinctrl: bcm283x: DM_FLAG_PRE_RELOC: Remove OF_CONTROL check Ovidiu Panait
@ 2020-05-27  6:41 ` Ovidiu Panait
  2020-05-27  6:41 ` [PATCH 4/5] pinctrl: bcm283x: Fix return value check for dev_read_addr_ptr Ovidiu Panait
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Ovidiu Panait @ 2020-05-27  6:41 UTC (permalink / raw)
  To: u-boot

Factor out reading IP base address to ofdata_to_platdata function, which
is designed for this purpose. Also, drop the dev->priv NULL check, since
this is already done by the dm core when allocating space using
priv_auto_alloc_size feature. (in drivers/core/device.c ->
device_ofdata_to_platdata).

Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---

 drivers/pinctrl/broadcom/pinctrl-bcm283x.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/pinctrl/broadcom/pinctrl-bcm283x.c b/drivers/pinctrl/broadcom/pinctrl-bcm283x.c
index 8bf7916627..9ab0baee33 100644
--- a/drivers/pinctrl/broadcom/pinctrl-bcm283x.c
+++ b/drivers/pinctrl/broadcom/pinctrl-bcm283x.c
@@ -104,17 +104,11 @@ static const struct udevice_id bcm2835_pinctrl_id[] = {
 	{}
 };
 
-int bcm283x_pinctl_probe(struct udevice *dev)
+int bcm283x_pinctl_ofdata_to_platdata(struct udevice *dev)
 {
 	struct bcm283x_pinctrl_priv *priv;
-	int ret;
-	struct udevice *pdev;
 
 	priv = dev_get_priv(dev);
-	if (!priv) {
-		debug("%s: Failed to get private\n", __func__);
-		return -EINVAL;
-	}
 
 	priv->base_reg = dev_read_addr_ptr(dev);
 	if (priv->base_reg == (void *)FDT_ADDR_T_NONE) {
@@ -122,6 +116,14 @@ int bcm283x_pinctl_probe(struct udevice *dev)
 		return -EINVAL;
 	}
 
+	return 0;
+}
+
+int bcm283x_pinctl_probe(struct udevice *dev)
+{
+	int ret;
+	struct udevice *pdev;
+
 	/* Create GPIO device as well */
 	ret = device_bind(dev, lists_driver_lookup_name("gpio_bcm2835"),
 			  "gpio_bcm2835", NULL, dev_of_offset(dev), &pdev);
@@ -147,6 +149,7 @@ U_BOOT_DRIVER(pinctrl_bcm283x) = {
 	.name		= "bcm283x_pinctrl",
 	.id		= UCLASS_PINCTRL,
 	.of_match	= of_match_ptr(bcm2835_pinctrl_id),
+	.ofdata_to_platdata = bcm283x_pinctl_ofdata_to_platdata,
 	.priv_auto_alloc_size = sizeof(struct bcm283x_pinctrl_priv),
 	.ops		= &bcm283x_pinctrl_ops,
 	.probe		= bcm283x_pinctl_probe,
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/5] pinctrl: bcm283x: Fix return value check for dev_read_addr_ptr
  2020-05-27  6:41 [PATCH 1/5] dm: core: Fix devfdt_get_addr_ptr return value Ovidiu Panait
  2020-05-27  6:41 ` [PATCH 2/5] pinctrl: bcm283x: DM_FLAG_PRE_RELOC: Remove OF_CONTROL check Ovidiu Panait
  2020-05-27  6:41 ` [PATCH 3/5] pinctrl: bcm283x: Read address from DT in ofdata_to_platdata Ovidiu Panait
@ 2020-05-27  6:41 ` Ovidiu Panait
  2020-05-27  6:41 ` [PATCH 5/5] pinctrl: bcm283x: Store the return value of dev_read_u32_default to int Ovidiu Panait
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Ovidiu Panait @ 2020-05-27  6:41 UTC (permalink / raw)
  To: u-boot

dev_read_addr_ptr returns NULL on failure, so add the proper check.

Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---

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

diff --git a/drivers/pinctrl/broadcom/pinctrl-bcm283x.c b/drivers/pinctrl/broadcom/pinctrl-bcm283x.c
index 9ab0baee33..49791c5962 100644
--- a/drivers/pinctrl/broadcom/pinctrl-bcm283x.c
+++ b/drivers/pinctrl/broadcom/pinctrl-bcm283x.c
@@ -111,7 +111,7 @@ int bcm283x_pinctl_ofdata_to_platdata(struct udevice *dev)
 	priv = dev_get_priv(dev);
 
 	priv->base_reg = dev_read_addr_ptr(dev);
-	if (priv->base_reg == (void *)FDT_ADDR_T_NONE) {
+	if (!priv->base_reg) {
 		debug("%s: Failed to get base address\n", __func__);
 		return -EINVAL;
 	}
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/5] pinctrl: bcm283x: Store the return value of dev_read_u32_default to int
  2020-05-27  6:41 [PATCH 1/5] dm: core: Fix devfdt_get_addr_ptr return value Ovidiu Panait
                   ` (2 preceding siblings ...)
  2020-05-27  6:41 ` [PATCH 4/5] pinctrl: bcm283x: Fix return value check for dev_read_addr_ptr Ovidiu Panait
@ 2020-05-27  6:41 ` Ovidiu Panait
  2020-05-27 12:05 ` [PATCH 1/5] dm: core: Fix devfdt_get_addr_ptr return value Matthias Brugger
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Ovidiu Panait @ 2020-05-27  6:41 UTC (permalink / raw)
  To: u-boot

Currently, the return value of dev_read_u32_default is stored in an u32,
causing the subsequent "if (function < 0)" to always be false:

u32 function
...
function = dev_read_u32_default(config, "brcm,function", -1);
if (function < 0) {
        debug("Failed reading function for pinconfig %s (%d)\n",
                      config->name, function);
        return -EINVAL;
}

Make "function" variable an int to fix this.

Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---

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

diff --git a/drivers/pinctrl/broadcom/pinctrl-bcm283x.c b/drivers/pinctrl/broadcom/pinctrl-bcm283x.c
index 49791c5962..41da814123 100644
--- a/drivers/pinctrl/broadcom/pinctrl-bcm283x.c
+++ b/drivers/pinctrl/broadcom/pinctrl-bcm283x.c
@@ -63,7 +63,7 @@ static int bcm2835_gpio_get_func_id(struct udevice *dev, unsigned int gpio)
 int bcm283x_pinctrl_set_state(struct udevice *dev, struct udevice *config)
 {
 	u32 pin_arr[MAX_PINS_PER_BANK];
-	u32 function;
+	int function;
 	int i, len, pin_count = 0;
 
 	if (!dev_read_prop(config, "brcm,pins", &len) || !len ||
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 1/5] dm: core: Fix devfdt_get_addr_ptr return value
  2020-05-27  6:41 [PATCH 1/5] dm: core: Fix devfdt_get_addr_ptr return value Ovidiu Panait
                   ` (3 preceding siblings ...)
  2020-05-27  6:41 ` [PATCH 5/5] pinctrl: bcm283x: Store the return value of dev_read_u32_default to int Ovidiu Panait
@ 2020-05-27 12:05 ` Matthias Brugger
  2020-06-11  9:07   ` Ovidiu Panait
  2020-05-28  9:22 ` Cédric Le Goater
  2020-05-28 14:32 ` Simon Glass
  6 siblings, 1 reply; 11+ messages in thread
From: Matthias Brugger @ 2020-05-27 12:05 UTC (permalink / raw)
  To: u-boot



On 27/05/2020 08:41, 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 <ovidiu.panait@windriver.com>

Reviewed-by: Matthias Brugger <mbrugger@suse.com>

Simon can you have a look. If it's OK for you I can queue the series through my
tree.

Regards,
Matthias

> ---
> 
>  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(-)
> 
> diff --git a/drivers/clk/aspeed/clk_ast2500.c b/drivers/clk/aspeed/clk_ast2500.c
> index ccfeded30c..284f5f58d6 100644
> --- a/drivers/clk/aspeed/clk_ast2500.c
> +++ b/drivers/clk/aspeed/clk_ast2500.c
> @@ -498,8 +498,8 @@ static int ast2500_clk_ofdata_to_platdata(struct udevice *dev)
>  	struct ast2500_clk_priv *priv = dev_get_priv(dev);
>  
>  	priv->scu = devfdt_get_addr_ptr(dev);
> -	if (IS_ERR(priv->scu))
> -		return PTR_ERR(priv->scu);
> +	if (!priv->scu)
> +		return -EINVAL;
>  
>  	return 0;
>  }
> diff --git a/drivers/core/fdtaddr.c b/drivers/core/fdtaddr.c
> index dfcb868f65..044d5871b0 100644
> --- a/drivers/core/fdtaddr.c
> +++ b/drivers/core/fdtaddr.c
> @@ -14,6 +14,7 @@
>  #include <log.h>
>  #include <asm/io.h>
>  #include <dm/device-internal.h>
> +#include <mapmem.h>
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> @@ -154,7 +155,9 @@ fdt_addr_t devfdt_get_addr(const struct udevice *dev)
>  
>  void *devfdt_get_addr_ptr(const struct udevice *dev)
>  {
> -	return (void *)(uintptr_t)devfdt_get_addr_index(dev, 0);
> +	fdt_addr_t addr = devfdt_get_addr_index(dev, 0);
> +
> +	return (addr == FDT_ADDR_T_NONE) ? NULL : map_sysmem(addr, 0);
>  }
>  
>  void *devfdt_remap_addr_index(const struct udevice *dev, int index)
> diff --git a/drivers/i2c/ast_i2c.c b/drivers/i2c/ast_i2c.c
> index 214362d04b..253e653666 100644
> --- a/drivers/i2c/ast_i2c.c
> +++ b/drivers/i2c/ast_i2c.c
> @@ -93,8 +93,8 @@ static int ast_i2c_ofdata_to_platdata(struct udevice *dev)
>  	int ret;
>  
>  	priv->regs = devfdt_get_addr_ptr(dev);
> -	if (IS_ERR(priv->regs))
> -		return PTR_ERR(priv->regs);
> +	if (!priv->regs)
> +		return -EINVAL;
>  
>  	ret = clk_get_by_index(dev, 0, &priv->clk);
>  	if (ret < 0) {
> diff --git a/drivers/pinctrl/mvebu/pinctrl-mvebu.c b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
> index 2206e958ec..ac0377e796 100644
> --- a/drivers/pinctrl/mvebu/pinctrl-mvebu.c
> +++ b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
> @@ -194,7 +194,7 @@ int mvebu_pinctl_probe(struct udevice *dev)
>  	}
>  
>  	priv->base_reg = devfdt_get_addr_ptr(dev);
> -	if (priv->base_reg == (void *)FDT_ADDR_T_NONE) {
> +	if (!priv->base_reg) {
>  		debug("%s: Failed to get base address\n", __func__);
>  		return -EINVAL;
>  	}
> diff --git a/drivers/timer/ast_timer.c b/drivers/timer/ast_timer.c
> index 3838601f54..9f28cbfcf9 100644
> --- a/drivers/timer/ast_timer.c
> +++ b/drivers/timer/ast_timer.c
> @@ -65,8 +65,8 @@ static int ast_timer_ofdata_to_platdata(struct udevice *dev)
>  	struct ast_timer_priv *priv = dev_get_priv(dev);
>  
>  	priv->regs = devfdt_get_addr_ptr(dev);
> -	if (IS_ERR(priv->regs))
> -		return PTR_ERR(priv->regs);
> +	if (!priv->regs)
> +		return -EINVAL;
>  
>  	priv->tmc = ast_get_timer_counter(priv->regs, AST_TICK_TIMER);
>  
> diff --git a/drivers/watchdog/ast_wdt.c b/drivers/watchdog/ast_wdt.c
> index 7e11465a57..a21f9a4d14 100644
> --- a/drivers/watchdog/ast_wdt.c
> +++ b/drivers/watchdog/ast_wdt.c
> @@ -91,8 +91,8 @@ static int ast_wdt_ofdata_to_platdata(struct udevice *dev)
>  	struct ast_wdt_priv *priv = dev_get_priv(dev);
>  
>  	priv->regs = devfdt_get_addr_ptr(dev);
> -	if (IS_ERR(priv->regs))
> -		return PTR_ERR(priv->regs);
> +	if (!priv->regs)
> +		return -EINVAL;
>  
>  	return 0;
>  }
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/5] dm: core: Fix devfdt_get_addr_ptr return value
  2020-05-27  6:41 [PATCH 1/5] dm: core: Fix devfdt_get_addr_ptr return value Ovidiu Panait
                   ` (4 preceding siblings ...)
  2020-05-27 12:05 ` [PATCH 1/5] dm: core: Fix devfdt_get_addr_ptr return value Matthias Brugger
@ 2020-05-28  9:22 ` Cédric Le Goater
  2020-05-28 14:32 ` Simon Glass
  6 siblings, 0 replies; 11+ messages in thread
From: Cédric Le Goater @ 2020-05-28  9:22 UTC (permalink / raw)
  To: u-boot

On 5/27/20 8:41 AM, 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 <ovidiu.panait@windriver.com> ---

Reviewed-by: C?dric Le Goater <clg@kaod.org>


> 
> 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(-)
> 
> diff --git a/drivers/clk/aspeed/clk_ast2500.c
> b/drivers/clk/aspeed/clk_ast2500.c index ccfeded30c..284f5f58d6
> 100644 --- a/drivers/clk/aspeed/clk_ast2500.c +++
> b/drivers/clk/aspeed/clk_ast2500.c @@ -498,8 +498,8 @@ static int
> ast2500_clk_ofdata_to_platdata(struct udevice *dev) struct
> ast2500_clk_priv *priv = dev_get_priv(dev);
> 
> priv->scu = devfdt_get_addr_ptr(dev); -	if (IS_ERR(priv->scu)) -
> return PTR_ERR(priv->scu); +	if (!priv->scu) +		return -EINVAL;
> 
> return 0; } diff --git a/drivers/core/fdtaddr.c
> b/drivers/core/fdtaddr.c index dfcb868f65..044d5871b0 100644 ---
> a/drivers/core/fdtaddr.c +++ b/drivers/core/fdtaddr.c @@ -14,6 +14,7
> @@ #include <log.h> #include <asm/io.h> #include
> <dm/device-internal.h> +#include <mapmem.h>
> 
> DECLARE_GLOBAL_DATA_PTR;
> 
> @@ -154,7 +155,9 @@ fdt_addr_t devfdt_get_addr(const struct udevice
> *dev)
> 
> void *devfdt_get_addr_ptr(const struct udevice *dev) { -	return (void
> *)(uintptr_t)devfdt_get_addr_index(dev, 0); +	fdt_addr_t addr =
> devfdt_get_addr_index(dev, 0); + +	return (addr == FDT_ADDR_T_NONE) ?
> NULL : map_sysmem(addr, 0); }
> 
> void *devfdt_remap_addr_index(const struct udevice *dev, int index) 
> diff --git a/drivers/i2c/ast_i2c.c b/drivers/i2c/ast_i2c.c index
> 214362d04b..253e653666 100644 --- a/drivers/i2c/ast_i2c.c +++
> b/drivers/i2c/ast_i2c.c @@ -93,8 +93,8 @@ static int
> ast_i2c_ofdata_to_platdata(struct udevice *dev) int ret;
> 
> priv->regs = devfdt_get_addr_ptr(dev); -	if (IS_ERR(priv->regs)) -
> return PTR_ERR(priv->regs); +	if (!priv->regs) +		return -EINVAL;
> 
> ret = clk_get_by_index(dev, 0, &priv->clk); if (ret < 0) { diff --git
> a/drivers/pinctrl/mvebu/pinctrl-mvebu.c
> b/drivers/pinctrl/mvebu/pinctrl-mvebu.c index 2206e958ec..ac0377e796
> 100644 --- a/drivers/pinctrl/mvebu/pinctrl-mvebu.c +++
> b/drivers/pinctrl/mvebu/pinctrl-mvebu.c @@ -194,7 +194,7 @@ int
> mvebu_pinctl_probe(struct udevice *dev) }
> 
> priv->base_reg = devfdt_get_addr_ptr(dev); -	if (priv->base_reg ==
> (void *)FDT_ADDR_T_NONE) { +	if (!priv->base_reg) { debug("%s: Failed
> to get base address\n", __func__); return -EINVAL; } diff --git
> a/drivers/timer/ast_timer.c b/drivers/timer/ast_timer.c index
> 3838601f54..9f28cbfcf9 100644 --- a/drivers/timer/ast_timer.c +++
> b/drivers/timer/ast_timer.c @@ -65,8 +65,8 @@ static int
> ast_timer_ofdata_to_platdata(struct udevice *dev) struct
> ast_timer_priv *priv = dev_get_priv(dev);
> 
> priv->regs = devfdt_get_addr_ptr(dev); -	if (IS_ERR(priv->regs)) -
> return PTR_ERR(priv->regs); +	if (!priv->regs) +		return -EINVAL;
> 
> priv->tmc = ast_get_timer_counter(priv->regs, AST_TICK_TIMER);
> 
> diff --git a/drivers/watchdog/ast_wdt.c b/drivers/watchdog/ast_wdt.c 
> index 7e11465a57..a21f9a4d14 100644 --- a/drivers/watchdog/ast_wdt.c 
> +++ b/drivers/watchdog/ast_wdt.c @@ -91,8 +91,8 @@ static int
> ast_wdt_ofdata_to_platdata(struct udevice *dev) struct ast_wdt_priv
> *priv = dev_get_priv(dev);
> 
> priv->regs = devfdt_get_addr_ptr(dev); -	if (IS_ERR(priv->regs)) -
> return PTR_ERR(priv->regs); +	if (!priv->regs) +		return -EINVAL;
> 
> return 0; }
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/5] dm: core: Fix devfdt_get_addr_ptr return value
  2020-05-27  6:41 [PATCH 1/5] dm: core: Fix devfdt_get_addr_ptr return value Ovidiu Panait
                   ` (5 preceding siblings ...)
  2020-05-28  9:22 ` Cédric Le Goater
@ 2020-05-28 14:32 ` Simon Glass
  2020-05-28 19:29   ` Sean Anderson
  6 siblings, 1 reply; 11+ messages in thread
From: Simon Glass @ 2020-05-28 14:32 UTC (permalink / raw)
  To: u-boot

On Wed, 27 May 2020 at 00:45, Ovidiu Panait <ovidiu.panait@windriver.com> 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 <ovidiu.panait@windriver.com>
> ---
>
>  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 <sjg@chromium.org>

[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/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/5] dm: core: Fix devfdt_get_addr_ptr return value
  2020-05-28 14:32 ` Simon Glass
@ 2020-05-28 19:29   ` Sean Anderson
  2020-05-29  0:28     ` Simon Glass
  0 siblings, 1 reply; 11+ messages in thread
From: Sean Anderson @ 2020-05-28 19:29 UTC (permalink / raw)
  To: u-boot

On 5/28/20 10:32 AM, Simon Glass wrote:
> On Wed, 27 May 2020 at 00:45, Ovidiu Panait <ovidiu.panait@windriver.com> 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 <ovidiu.panait@windriver.com>
>> ---
>>
>>  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

It looks like I never actually got CC'd

> 
> 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.

Hopefully soon; I believe the last holdup was passing CI.

> 
> 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:

I think that's a good idea. However, from what I can see, that patch
tests dev_read_addr_ptr, not devfdt_get_addr_ptr. You may also want to
check the error case, to make sure FDT_ADDR_T_NONE gets returned.

> 
> Tested on sandbox
> Tested-by: Simon Glass <sjg@chromium.org>
> 
> [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/
> 

--Sean

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/5] dm: core: Fix devfdt_get_addr_ptr return value
  2020-05-28 19:29   ` Sean Anderson
@ 2020-05-29  0:28     ` Simon Glass
  0 siblings, 0 replies; 11+ messages in thread
From: Simon Glass @ 2020-05-29  0:28 UTC (permalink / raw)
  To: u-boot

Hi Sean,


On Thu, 28 May 2020 at 13:29, Sean Anderson <seanga2@gmail.com> wrote:
>
> On 5/28/20 10:32 AM, Simon Glass wrote:
> > On Wed, 27 May 2020 at 00:45, Ovidiu Panait <ovidiu.panait@windriver.com> 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 <ovidiu.panait@windriver.com>
> >> ---
> >>
> >>  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
>
> It looks like I never actually got CC'd


Oops sorry.

>
>
> >
> > 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.
>
> Hopefully soon; I believe the last holdup was passing CI.
>
> >
> > 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:
>
> I think that's a good idea. However, from what I can see, that patch
> tests dev_read_addr_ptr, not devfdt_get_addr_ptr. You may also want to
> check the error case, to make sure FDT_ADDR_T_NONE gets returned.

Yes that's right...it is a test for dev_read_addr_ptr(), although in
one case it calls the latter.

>
>
> >
> > Tested on sandbox
> > Tested-by: Simon Glass <sjg@chromium.org>
> >
> > [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/
> >
>
> --Sean
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/5] dm: core: Fix devfdt_get_addr_ptr return value
  2020-05-27 12:05 ` [PATCH 1/5] dm: core: Fix devfdt_get_addr_ptr return value Matthias Brugger
@ 2020-06-11  9:07   ` Ovidiu Panait
  0 siblings, 0 replies; 11+ messages in thread
From: Ovidiu Panait @ 2020-06-11  9:07 UTC (permalink / raw)
  To: u-boot

Hi,

On 27.05.2020 15:05, Matthias Brugger wrote:
>
> On 27/05/2020 08:41, 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 <ovidiu.panait@windriver.com>
> Reviewed-by: Matthias Brugger <mbrugger@suse.com>
>
> Simon can you have a look. If it's OK for you I can queue the series through my
> tree.

I will resend this patch after 
http://patchwork.ozlabs.org/project/uboot/patch/20200521161503.384823-10-seanga2 at gmail.com/ 
series gets merged.

In the meantime, I resent the rest of "pinctrl: bcm283x" patches from 
this series separately so maybe they could be picked up in the meantime:

https://patchwork.ozlabs.org/project/uboot/list/?series=182703


Thanks,

Ovidiu

> Regards,
> Matthias
>
>> ---
>>
>>   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(-)
>>
>> diff --git a/drivers/clk/aspeed/clk_ast2500.c b/drivers/clk/aspeed/clk_ast2500.c
>> index ccfeded30c..284f5f58d6 100644
>> --- a/drivers/clk/aspeed/clk_ast2500.c
>> +++ b/drivers/clk/aspeed/clk_ast2500.c
>> @@ -498,8 +498,8 @@ static int ast2500_clk_ofdata_to_platdata(struct udevice *dev)
>>   	struct ast2500_clk_priv *priv = dev_get_priv(dev);
>>   
>>   	priv->scu = devfdt_get_addr_ptr(dev);
>> -	if (IS_ERR(priv->scu))
>> -		return PTR_ERR(priv->scu);
>> +	if (!priv->scu)
>> +		return -EINVAL;
>>   
>>   	return 0;
>>   }
>> diff --git a/drivers/core/fdtaddr.c b/drivers/core/fdtaddr.c
>> index dfcb868f65..044d5871b0 100644
>> --- a/drivers/core/fdtaddr.c
>> +++ b/drivers/core/fdtaddr.c
>> @@ -14,6 +14,7 @@
>>   #include <log.h>
>>   #include <asm/io.h>
>>   #include <dm/device-internal.h>
>> +#include <mapmem.h>
>>   
>>   DECLARE_GLOBAL_DATA_PTR;
>>   
>> @@ -154,7 +155,9 @@ fdt_addr_t devfdt_get_addr(const struct udevice *dev)
>>   
>>   void *devfdt_get_addr_ptr(const struct udevice *dev)
>>   {
>> -	return (void *)(uintptr_t)devfdt_get_addr_index(dev, 0);
>> +	fdt_addr_t addr = devfdt_get_addr_index(dev, 0);
>> +
>> +	return (addr == FDT_ADDR_T_NONE) ? NULL : map_sysmem(addr, 0);
>>   }
>>   
>>   void *devfdt_remap_addr_index(const struct udevice *dev, int index)
>> diff --git a/drivers/i2c/ast_i2c.c b/drivers/i2c/ast_i2c.c
>> index 214362d04b..253e653666 100644
>> --- a/drivers/i2c/ast_i2c.c
>> +++ b/drivers/i2c/ast_i2c.c
>> @@ -93,8 +93,8 @@ static int ast_i2c_ofdata_to_platdata(struct udevice *dev)
>>   	int ret;
>>   
>>   	priv->regs = devfdt_get_addr_ptr(dev);
>> -	if (IS_ERR(priv->regs))
>> -		return PTR_ERR(priv->regs);
>> +	if (!priv->regs)
>> +		return -EINVAL;
>>   
>>   	ret = clk_get_by_index(dev, 0, &priv->clk);
>>   	if (ret < 0) {
>> diff --git a/drivers/pinctrl/mvebu/pinctrl-mvebu.c b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
>> index 2206e958ec..ac0377e796 100644
>> --- a/drivers/pinctrl/mvebu/pinctrl-mvebu.c
>> +++ b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
>> @@ -194,7 +194,7 @@ int mvebu_pinctl_probe(struct udevice *dev)
>>   	}
>>   
>>   	priv->base_reg = devfdt_get_addr_ptr(dev);
>> -	if (priv->base_reg == (void *)FDT_ADDR_T_NONE) {
>> +	if (!priv->base_reg) {
>>   		debug("%s: Failed to get base address\n", __func__);
>>   		return -EINVAL;
>>   	}
>> diff --git a/drivers/timer/ast_timer.c b/drivers/timer/ast_timer.c
>> index 3838601f54..9f28cbfcf9 100644
>> --- a/drivers/timer/ast_timer.c
>> +++ b/drivers/timer/ast_timer.c
>> @@ -65,8 +65,8 @@ static int ast_timer_ofdata_to_platdata(struct udevice *dev)
>>   	struct ast_timer_priv *priv = dev_get_priv(dev);
>>   
>>   	priv->regs = devfdt_get_addr_ptr(dev);
>> -	if (IS_ERR(priv->regs))
>> -		return PTR_ERR(priv->regs);
>> +	if (!priv->regs)
>> +		return -EINVAL;
>>   
>>   	priv->tmc = ast_get_timer_counter(priv->regs, AST_TICK_TIMER);
>>   
>> diff --git a/drivers/watchdog/ast_wdt.c b/drivers/watchdog/ast_wdt.c
>> index 7e11465a57..a21f9a4d14 100644
>> --- a/drivers/watchdog/ast_wdt.c
>> +++ b/drivers/watchdog/ast_wdt.c
>> @@ -91,8 +91,8 @@ static int ast_wdt_ofdata_to_platdata(struct udevice *dev)
>>   	struct ast_wdt_priv *priv = dev_get_priv(dev);
>>   
>>   	priv->regs = devfdt_get_addr_ptr(dev);
>> -	if (IS_ERR(priv->regs))
>> -		return PTR_ERR(priv->regs);
>> +	if (!priv->regs)
>> +		return -EINVAL;
>>   
>>   	return 0;
>>   }
>>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-06-11  9:07 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-27  6:41 [PATCH 1/5] dm: core: Fix devfdt_get_addr_ptr return value Ovidiu Panait
2020-05-27  6:41 ` [PATCH 2/5] pinctrl: bcm283x: DM_FLAG_PRE_RELOC: Remove OF_CONTROL check Ovidiu Panait
2020-05-27  6:41 ` [PATCH 3/5] pinctrl: bcm283x: Read address from DT in ofdata_to_platdata Ovidiu Panait
2020-05-27  6:41 ` [PATCH 4/5] pinctrl: bcm283x: Fix return value check for dev_read_addr_ptr Ovidiu Panait
2020-05-27  6:41 ` [PATCH 5/5] pinctrl: bcm283x: Store the return value of dev_read_u32_default to int Ovidiu Panait
2020-05-27 12:05 ` [PATCH 1/5] dm: core: Fix devfdt_get_addr_ptr return value Matthias Brugger
2020-06-11  9:07   ` Ovidiu Panait
2020-05-28  9:22 ` Cédric Le Goater
2020-05-28 14:32 ` Simon Glass
2020-05-28 19:29   ` Sean Anderson
2020-05-29  0:28     ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.