| Message ID | 1524019125-26287-4-git-send-email-yamada.masahiro@socionext.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Add Linux-compatible syscon_to_regmap API | expand |
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>
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
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?
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 --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, ¶ms->map); + ret = regmap_init_mem(dev_ofnode(dev), ¶ms->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, ¶ms->map); + ret = regmap_init_mem(dev_ofnode(dev), ¶ms->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, ¶ms->map); + ret = regmap_init_mem(dev_ofnode(dev), ¶ms->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
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(-)