diff mbox series

[3/4] regmap: change regmap_init_mem() to take ofnode instead udevice

Message ID 1524019125-26287-4-git-send-email-yamada.masahiro@socionext.com
State Superseded
Headers show
Series Add Linux-compatible syscon_to_regmap API | expand

Commit Message

Masahiro Yamada April 18, 2018, 2:38 a.m. UTC
Currently, regmap_init_mem() takes udevice. This requires the node
has already been associated with a device. It prevents syscon/regmap
from behaving like those in Linux.

Change the first argumenet to take the device node.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 arch/arm/mach-aspeed/ast2500/sdram_ast2500.c |  2 +-
 drivers/core/regmap.c                        | 11 +++++------
 drivers/core/syscon-uclass.c                 |  2 +-
 drivers/phy/meson-gxl-usb2.c                 |  2 +-
 drivers/phy/meson-gxl-usb3.c                 |  2 +-
 drivers/ram/rockchip/dmc-rk3368.c            |  2 +-
 drivers/ram/rockchip/sdram_rk3188.c          |  2 +-
 drivers/ram/rockchip/sdram_rk322x.c          |  2 +-
 drivers/ram/rockchip/sdram_rk3288.c          |  2 +-
 drivers/ram/rockchip/sdram_rk3399.c          |  2 +-
 drivers/ram/stm32mp1/stm32mp1_ram.c          |  2 +-
 drivers/reset/reset-meson.c                  |  2 +-
 include/regmap.h                             |  4 ++--
 13 files changed, 18 insertions(+), 19 deletions(-)

Comments

Neil Armstrong April 18, 2018, 3:34 p.m. UTC | #1
On 18/04/2018 04:38, Masahiro Yamada wrote:
> Currently, regmap_init_mem() takes udevice. This requires the node
> has already been associated with a device. It prevents syscon/regmap
> from behaving like those in Linux.
> 
> Change the first argumenet to take the device node.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  arch/arm/mach-aspeed/ast2500/sdram_ast2500.c |  2 +-
>  drivers/core/regmap.c                        | 11 +++++------
>  drivers/core/syscon-uclass.c                 |  2 +-
>  drivers/phy/meson-gxl-usb2.c                 |  2 +-
>  drivers/phy/meson-gxl-usb3.c                 |  2 +-
>  drivers/ram/rockchip/dmc-rk3368.c            |  2 +-
>  drivers/ram/rockchip/sdram_rk3188.c          |  2 +-
>  drivers/ram/rockchip/sdram_rk322x.c          |  2 +-
>  drivers/ram/rockchip/sdram_rk3288.c          |  2 +-
>  drivers/ram/rockchip/sdram_rk3399.c          |  2 +-
>  drivers/ram/stm32mp1/stm32mp1_ram.c          |  2 +-
>  drivers/reset/reset-meson.c                  |  2 +-
>  include/regmap.h                             |  4 ++--
>  13 files changed, 18 insertions(+), 19 deletions(-)
> 
[..]
> diff --git a/drivers/phy/meson-gxl-usb2.c b/drivers/phy/meson-gxl-usb2.c
> index 15c9c89..7242bf6 100644
> --- a/drivers/phy/meson-gxl-usb2.c
> +++ b/drivers/phy/meson-gxl-usb2.c
> @@ -195,7 +195,7 @@ int meson_gxl_usb2_phy_probe(struct udevice *dev)
>  	struct phy_meson_gxl_usb2_priv *priv = dev_get_priv(dev);
>  	int ret;
>  
> -	ret = regmap_init_mem(dev, &priv->regmap);
> +	ret = regmap_init_mem(dev_ofnode(dev), &priv->regmap);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/phy/meson-gxl-usb3.c b/drivers/phy/meson-gxl-usb3.c
> index a385fbd..47a41fd 100644
> --- a/drivers/phy/meson-gxl-usb3.c
> +++ b/drivers/phy/meson-gxl-usb3.c
> @@ -166,7 +166,7 @@ int meson_gxl_usb3_phy_probe(struct udevice *dev)
>  	struct phy_meson_gxl_usb3_priv *priv = dev_get_priv(dev);
>  	int ret;
>  
> -	ret = regmap_init_mem(dev, &priv->regmap);
> +	ret = regmap_init_mem(dev_ofnode(dev), &priv->regmap);
>  	if (ret)
>  		return ret;
>  	
[..]
> diff --git a/drivers/reset/reset-meson.c b/drivers/reset/reset-meson.c
> index 5324f86..c41d176 100644
> --- a/drivers/reset/reset-meson.c
> +++ b/drivers/reset/reset-meson.c
> @@ -77,7 +77,7 @@ static int meson_reset_probe(struct udevice *dev)
>  {
>  	struct meson_reset_priv *priv = dev_get_priv(dev);
>  	
> -	return regmap_init_mem(dev, &priv->regmap);
> +	return regmap_init_mem(dev_ofnode(dev), &priv->regmap);
>  }
>  
>  U_BOOT_DRIVER(meson_reset) = {

For reset-meson, meson-gxl-usb*

Acked-by: Neil Armstrong <narmstrong@baylibre.com>
Simon Glass April 22, 2018, 8:10 p.m. UTC | #2
Hi Masahiro,

On 17 April 2018 at 20:38, Masahiro Yamada <yamada.masahiro@socionext.com>
wrote:
> Currently, regmap_init_mem() takes udevice. This requires the node
> has already been associated with a device. It prevents syscon/regmap
> from behaving like those in Linux.
>
> Change the first argumenet to take the device node.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  arch/arm/mach-aspeed/ast2500/sdram_ast2500.c |  2 +-
>  drivers/core/regmap.c                        | 11 +++++------
>  drivers/core/syscon-uclass.c                 |  2 +-
>  drivers/phy/meson-gxl-usb2.c                 |  2 +-
>  drivers/phy/meson-gxl-usb3.c                 |  2 +-
>  drivers/ram/rockchip/dmc-rk3368.c            |  2 +-
>  drivers/ram/rockchip/sdram_rk3188.c          |  2 +-
>  drivers/ram/rockchip/sdram_rk322x.c          |  2 +-
>  drivers/ram/rockchip/sdram_rk3288.c          |  2 +-
>  drivers/ram/rockchip/sdram_rk3399.c          |  2 +-
>  drivers/ram/stm32mp1/stm32mp1_ram.c          |  2 +-
>  drivers/reset/reset-meson.c                  |  2 +-
>  include/regmap.h                             |  4 ++--
>  13 files changed, 18 insertions(+), 19 deletions(-)

Can you please add a simple test for regmap on sandbox?

Regards,
Simon
Masahiro Yamada April 23, 2018, 4:56 a.m. UTC | #3
Hi Simon,


2018-04-23 5:10 GMT+09:00 Simon Glass <sjg@chromium.org>:
> Hi Masahiro,
>
> On 17 April 2018 at 20:38, Masahiro Yamada <yamada.masahiro@socionext.com>
> wrote:
>> Currently, regmap_init_mem() takes udevice. This requires the node
>> has already been associated with a device. It prevents syscon/regmap
>> from behaving like those in Linux.
>>
>> Change the first argumenet to take the device node.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>>  arch/arm/mach-aspeed/ast2500/sdram_ast2500.c |  2 +-
>>  drivers/core/regmap.c                        | 11 +++++------
>>  drivers/core/syscon-uclass.c                 |  2 +-
>>  drivers/phy/meson-gxl-usb2.c                 |  2 +-
>>  drivers/phy/meson-gxl-usb3.c                 |  2 +-
>>  drivers/ram/rockchip/dmc-rk3368.c            |  2 +-
>>  drivers/ram/rockchip/sdram_rk3188.c          |  2 +-
>>  drivers/ram/rockchip/sdram_rk322x.c          |  2 +-
>>  drivers/ram/rockchip/sdram_rk3288.c          |  2 +-
>>  drivers/ram/rockchip/sdram_rk3399.c          |  2 +-
>>  drivers/ram/stm32mp1/stm32mp1_ram.c          |  2 +-
>>  drivers/reset/reset-meson.c                  |  2 +-
>>  include/regmap.h                             |  4 ++--
>>  13 files changed, 18 insertions(+), 19 deletions(-)
>
> Can you please add a simple test for regmap on sandbox?
>

No.

Please stop this boiler-plate response.

Simple tests for regmap already exist in test/dm/regmap.c

regmap_init_mem() is not a new function, and it is tested
through other function tests.

You block patches several times before
by making the contributors say "Sorry, I am busy".


I am really busy, but I need to fix the misimplementation
of syscon for Socionext drivers.

Why should I be annoyed by additional work
to fix the problem?
Simon Glass April 25, 2018, 5:01 a.m. UTC | #4
Hi Masahiro,

On 22 April 2018 at 22:56, Masahiro Yamada <yamada.masahiro@socionext.com>
wrote:
> Hi Simon,
>
>
> 2018-04-23 5:10 GMT+09:00 Simon Glass <sjg@chromium.org>:
>> Hi Masahiro,
>>
>> On 17 April 2018 at 20:38, Masahiro Yamada <yamada.masahiro@socionext.com
>
>> wrote:
>>> Currently, regmap_init_mem() takes udevice. This requires the node
>>> has already been associated with a device. It prevents syscon/regmap
>>> from behaving like those in Linux.
>>>
>>> Change the first argumenet to take the device node.
>>>
>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>> ---
>>>
>>>  arch/arm/mach-aspeed/ast2500/sdram_ast2500.c |  2 +-
>>>  drivers/core/regmap.c                        | 11 +++++------
>>>  drivers/core/syscon-uclass.c                 |  2 +-
>>>  drivers/phy/meson-gxl-usb2.c                 |  2 +-
>>>  drivers/phy/meson-gxl-usb3.c                 |  2 +-
>>>  drivers/ram/rockchip/dmc-rk3368.c            |  2 +-
>>>  drivers/ram/rockchip/sdram_rk3188.c          |  2 +-
>>>  drivers/ram/rockchip/sdram_rk322x.c          |  2 +-
>>>  drivers/ram/rockchip/sdram_rk3288.c          |  2 +-
>>>  drivers/ram/rockchip/sdram_rk3399.c          |  2 +-
>>>  drivers/ram/stm32mp1/stm32mp1_ram.c          |  2 +-
>>>  drivers/reset/reset-meson.c                  |  2 +-
>>>  include/regmap.h                             |  4 ++--
>>>  13 files changed, 18 insertions(+), 19 deletions(-)
>>
>> Can you please add a simple test for regmap on sandbox?
>>
>
> No.
>
> Please stop this boiler-plate response.
>
> Simple tests for regmap already exist in test/dm/regmap.c
>
> regmap_init_mem() is not a new function, and it is tested
> through other function tests.

Can you please point to that? I don't see anything for sandbox.

>
> You block patches several times before
> by making the contributors say "Sorry, I am busy".
>
>
> I am really busy, but I need to fix the misimplementation
> of syscon for Socionext drivers.
>
> Why should I be annoyed by additional work
> to fix the problem?

Because otherwise I have no idea if the code works, no one will be able to
change it later without potentially breaking it, etc. If people get into
the habit of writing a little test at the same time, it takes very little
extra effort.

I have pour an ENORMOUS amount of time into making testing in U-Boot
better, as has Stephen Warren. We need to continue the effort. I'm sorry
that this adds a little more time to the patch submission process, but we
gain a lot of benefits. Look at all the times we have tried to fix address
translation in U-Boot, or FIT behaviour, when we don't have tests or even a
clear definition of the correct behaviour.

So please try to understand my point of view here. This is not just about
your patch, it is about the future of U-Boot for future users and
contributors.

Regards,
Simon
diff mbox series

Patch

diff --git a/arch/arm/mach-aspeed/ast2500/sdram_ast2500.c b/arch/arm/mach-aspeed/ast2500/sdram_ast2500.c
index 6383f72..f267c58 100644
--- a/arch/arm/mach-aspeed/ast2500/sdram_ast2500.c
+++ b/arch/arm/mach-aspeed/ast2500/sdram_ast2500.c
@@ -392,7 +392,7 @@  static int ast2500_sdrammc_ofdata_to_platdata(struct udevice *dev)
 	struct regmap *map;
 	int ret;
 
-	ret = regmap_init_mem(dev, &map);
+	ret = regmap_init_mem(dev_ofnode(dev), &map);
 	if (ret)
 		return ret;
 
diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c
index 6c3dbe9..1185a44 100644
--- a/drivers/core/regmap.c
+++ b/drivers/core/regmap.c
@@ -51,7 +51,7 @@  int regmap_init_mem_platdata(struct udevice *dev, fdt_val_t *reg, int count,
 	return 0;
 }
 #else
-int regmap_init_mem(struct udevice *dev, struct regmap **mapp)
+int regmap_init_mem(ofnode node, struct regmap **mapp)
 {
 	struct regmap_range *range;
 	struct regmap *map;
@@ -59,14 +59,13 @@  int regmap_init_mem(struct udevice *dev, struct regmap **mapp)
 	int addr_len, size_len, both_len;
 	int len;
 	int index;
-	ofnode node = dev_ofnode(dev);
 	struct resource r;
 
-	addr_len = dev_read_simple_addr_cells(dev->parent);
-	size_len = dev_read_simple_size_cells(dev->parent);
+	addr_len = ofnode_read_simple_addr_cells(ofnode_get_parent(node));
+	size_len = ofnode_read_simple_size_cells(ofnode_get_parent(node));
 	both_len = addr_len + size_len;
 
-	len = dev_read_size(dev, "reg");
+	len = ofnode_read_size(node, "reg");
 	if (len < 0)
 		return len;
 	len /= sizeof(fdt32_t);
@@ -87,7 +86,7 @@  int regmap_init_mem(struct udevice *dev, struct regmap **mapp)
 			range->size = r.end - r.start + 1;
 		} else {
 			range->start = fdtdec_get_addr_size_fixed(gd->fdt_blob,
-					dev_of_offset(dev), "reg", index,
+					ofnode_to_offset(node), "reg", index,
 					addr_len, size_len, &sz, true);
 			range->size = sz;
 		}
diff --git a/drivers/core/syscon-uclass.c b/drivers/core/syscon-uclass.c
index a69937e..c99409b 100644
--- a/drivers/core/syscon-uclass.c
+++ b/drivers/core/syscon-uclass.c
@@ -41,7 +41,7 @@  static int syscon_pre_probe(struct udevice *dev)
 	return regmap_init_mem_platdata(dev, plat->reg, ARRAY_SIZE(plat->reg),
 					&priv->regmap);
 #else
-	return regmap_init_mem(dev, &priv->regmap);
+	return regmap_init_mem(dev_ofnode(dev), &priv->regmap);
 #endif
 }
 
diff --git a/drivers/phy/meson-gxl-usb2.c b/drivers/phy/meson-gxl-usb2.c
index 15c9c89..7242bf6 100644
--- a/drivers/phy/meson-gxl-usb2.c
+++ b/drivers/phy/meson-gxl-usb2.c
@@ -195,7 +195,7 @@  int meson_gxl_usb2_phy_probe(struct udevice *dev)
 	struct phy_meson_gxl_usb2_priv *priv = dev_get_priv(dev);
 	int ret;
 
-	ret = regmap_init_mem(dev, &priv->regmap);
+	ret = regmap_init_mem(dev_ofnode(dev), &priv->regmap);
 	if (ret)
 		return ret;
 
diff --git a/drivers/phy/meson-gxl-usb3.c b/drivers/phy/meson-gxl-usb3.c
index a385fbd..47a41fd 100644
--- a/drivers/phy/meson-gxl-usb3.c
+++ b/drivers/phy/meson-gxl-usb3.c
@@ -166,7 +166,7 @@  int meson_gxl_usb3_phy_probe(struct udevice *dev)
 	struct phy_meson_gxl_usb3_priv *priv = dev_get_priv(dev);
 	int ret;
 
-	ret = regmap_init_mem(dev, &priv->regmap);
+	ret = regmap_init_mem(dev_ofnode(dev), &priv->regmap);
 	if (ret)
 		return ret;
 	
diff --git a/drivers/ram/rockchip/dmc-rk3368.c b/drivers/ram/rockchip/dmc-rk3368.c
index bfcb1dd..9bf64bf 100644
--- a/drivers/ram/rockchip/dmc-rk3368.c
+++ b/drivers/ram/rockchip/dmc-rk3368.c
@@ -880,7 +880,7 @@  static int rk3368_dmc_ofdata_to_platdata(struct udevice *dev)
 #if !CONFIG_IS_ENABLED(OF_PLATDATA)
 	struct rk3368_sdram_params *plat = dev_get_platdata(dev);
 
-	ret = regmap_init_mem(dev, &plat->map);
+	ret = regmap_init_mem(dev_ofnode(dev), &plat->map);
 	if (ret)
 		return ret;
 #endif
diff --git a/drivers/ram/rockchip/sdram_rk3188.c b/drivers/ram/rockchip/sdram_rk3188.c
index 365d00e..c0a2618 100644
--- a/drivers/ram/rockchip/sdram_rk3188.c
+++ b/drivers/ram/rockchip/sdram_rk3188.c
@@ -842,7 +842,7 @@  static int rk3188_dmc_ofdata_to_platdata(struct udevice *dev)
 		printf("%s: Cannot read rockchip,sdram-params\n", __func__);
 		return -EINVAL;
 	}
-	ret = regmap_init_mem(dev, &params->map);
+	ret = regmap_init_mem(dev_ofnode(dev), &params->map);
 	if (ret)
 		return ret;
 #endif
diff --git a/drivers/ram/rockchip/sdram_rk322x.c b/drivers/ram/rockchip/sdram_rk322x.c
index cc3138b..dca07db 100644
--- a/drivers/ram/rockchip/sdram_rk322x.c
+++ b/drivers/ram/rockchip/sdram_rk322x.c
@@ -744,7 +744,7 @@  static int rk322x_dmc_ofdata_to_platdata(struct udevice *dev)
 		printf("%s: Cannot read rockchip,sdram-params\n", __func__);
 		return -EINVAL;
 	}
-	ret = regmap_init_mem(dev, &params->map);
+	ret = regmap_init_mem(dev_ofnode(dev), &params->map);
 	if (ret)
 		return ret;
 #endif
diff --git a/drivers/ram/rockchip/sdram_rk3288.c b/drivers/ram/rockchip/sdram_rk3288.c
index 95efb11..ffb663c 100644
--- a/drivers/ram/rockchip/sdram_rk3288.c
+++ b/drivers/ram/rockchip/sdram_rk3288.c
@@ -1003,7 +1003,7 @@  static int rk3288_dmc_ofdata_to_platdata(struct udevice *dev)
 
 	priv->is_veyron = !fdt_node_check_compatible(blob, 0, "google,veyron");
 #endif
-	ret = regmap_init_mem(dev, &params->map);
+	ret = regmap_init_mem(dev_ofnode(dev), &params->map);
 	if (ret)
 		return ret;
 #endif
diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
index 5cb470c..6e2a39e 100644
--- a/drivers/ram/rockchip/sdram_rk3399.c
+++ b/drivers/ram/rockchip/sdram_rk3399.c
@@ -1100,7 +1100,7 @@  static int rk3399_dmc_ofdata_to_platdata(struct udevice *dev)
 		       __func__, ret);
 		return ret;
 	}
-	ret = regmap_init_mem(dev, &plat->map);
+	ret = regmap_init_mem(dev_ofnode(dev), &plat->map);
 	if (ret)
 		printf("%s: regmap failed %d\n", __func__, ret);
 
diff --git a/drivers/ram/stm32mp1/stm32mp1_ram.c b/drivers/ram/stm32mp1/stm32mp1_ram.c
index 9599444..a4d5906 100644
--- a/drivers/ram/stm32mp1/stm32mp1_ram.c
+++ b/drivers/ram/stm32mp1/stm32mp1_ram.c
@@ -149,7 +149,7 @@  static int stm32mp1_ddr_probe(struct udevice *dev)
 	debug("STM32MP1 DDR probe\n");
 	priv->dev = dev;
 
-	ret = regmap_init_mem(dev, &map);
+	ret = regmap_init_mem(dev_ofnode(dev), &map);
 	if (ret)
 		return ret;
 
diff --git a/drivers/reset/reset-meson.c b/drivers/reset/reset-meson.c
index 5324f86..c41d176 100644
--- a/drivers/reset/reset-meson.c
+++ b/drivers/reset/reset-meson.c
@@ -77,7 +77,7 @@  static int meson_reset_probe(struct udevice *dev)
 {
 	struct meson_reset_priv *priv = dev_get_priv(dev);
 	
-	return regmap_init_mem(dev, &priv->regmap);
+	return regmap_init_mem(dev_ofnode(dev), &priv->regmap);
 }
 
 U_BOOT_DRIVER(meson_reset) = {
diff --git a/include/regmap.h b/include/regmap.h
index 858aa7e..c8a1df0 100644
--- a/include/regmap.h
+++ b/include/regmap.h
@@ -48,10 +48,10 @@  int regmap_read(struct regmap *map, uint offset, uint *valp);
  *
  * Use regmap_uninit() to free it.
  *
- * @dev:	Device that uses this map
+ * @node:	Device node that uses this map
  * @mapp:	Returns allocated map
  */
-int regmap_init_mem(struct udevice *dev, struct regmap **mapp);
+int regmap_init_mem(ofnode node, struct regmap **mapp);
 
 /**
  * regmap_init_mem_platdata() - Set up a new memory register map for of-platdata