[PATCH v2 0/5] Revert "fdt: translate address if #size-cells = <0>"
As pointed by [1] and [2] the d64b9cdcd4 ("fdt: translate address if #size-cells = <0>") commit was wrong. The series reverts the patch and fixes the issue with platform code, adding custom routines to access the clocks registers. The solution has been inspired by the Linux Kernel code.
[1] https://patchwork.ozlabs.org/project/uboot/patch/1614324949-61314-1-git-send... [2] https://lore.kernel.org/linux-clk/20210402192054.7934-1-dariobin@libero.it/T...
Changes in v2: - Remove #if CONFIG_IS_ENABLED(AM33XX). It was counter intuitive. - Added Bin Meng Reviewed-by tag.
Dario Binacchi (5): clk: ti: add custom API for memory access clk: ti: change clk_ti_latch() signature clk: ti: gate: use custom API for memory access clk: ti: am3-dpll: use custom API for memory access Revert "fdt: translate address if #size-cells = <0>"
arch/sandbox/dts/test.dts | 21 ------- common/fdt_support.c | 6 +- drivers/clk/ti/clk-am3-dpll.c | 86 +++++++++++++++++----------- drivers/clk/ti/clk-divider.c | 20 ++++--- drivers/clk/ti/clk-gate.c | 23 ++++---- drivers/clk/ti/clk-mux.c | 20 +++---- drivers/clk/ti/clk.c | 95 +++++++++++++++++++++++++++++-- drivers/clk/ti/clk.h | 15 ++++- drivers/core/Kconfig | 12 ---- drivers/core/fdtaddr.c | 2 +- drivers/core/of_addr.c | 13 ++++- drivers/core/ofnode.c | 7 +-- drivers/core/root.c | 3 - include/asm-generic/global_data.h | 24 -------- test/dm/test-fdt.c | 68 ---------------------- 15 files changed, 206 insertions(+), 209 deletions(-)
As pointed by [1] and [2], commit d64b9cdcd4 ("fdt: translate address if #size-cells = <0>") is wrong: - It makes every 'reg' DT property translatable. It changes the address translation so that for an I2C 'reg' address you'll get back as reg the I2C controller address + reg value. - The quirk must be fixed with platform code.
The clk_ti_get_reg_addr() is the platform code able to make the correct address translation for the AM33xx clocks registers. Its implementation was inspired by the Linux Kernel code.
[1] https://patchwork.ozlabs.org/project/uboot/patch/1614324949-61314-1-git-send... [2] https://lore.kernel.org/linux-clk/20210402192054.7934-1-dariobin@libero.it/T...
Signed-off-by: Dario Binacchi dariobin@libero.it
---
Changes in v2: - Remove #if CONFIG_IS_ENABLED(AM33XX). It was counter intuitive.
drivers/clk/ti/clk.c | 85 ++++++++++++++++++++++++++++++++++++++++++++ drivers/clk/ti/clk.h | 13 +++++++ 2 files changed, 98 insertions(+)
diff --git a/drivers/clk/ti/clk.c b/drivers/clk/ti/clk.c index c999df213a..7a413d668d 100644 --- a/drivers/clk/ti/clk.c +++ b/drivers/clk/ti/clk.c @@ -6,10 +6,23 @@ */
#include <common.h> +#include <dm.h> #include <fdtdec.h> +#include <regmap.h> #include <asm/io.h> +#include <dm/device_compat.h> #include "clk.h"
+#define CLK_MAX_MEMMAPS 10 + +struct clk_iomap { + struct regmap *regmap; + ofnode node; +}; + +static unsigned int clk_memmaps_num; +static struct clk_iomap clk_memmaps[CLK_MAX_MEMMAPS]; + static void clk_ti_rmw(u32 val, u32 mask, fdt_addr_t reg) { u32 v; @@ -33,3 +46,75 @@ void clk_ti_latch(fdt_addr_t reg, s8 shift) clk_ti_rmw(0, latch, reg); readl(reg); /* OCP barrier */ } + +void clk_ti_writel(u32 val, struct clk_ti_reg *reg) +{ + struct clk_iomap *io = &clk_memmaps[reg->index]; + + regmap_write(io->regmap, reg->offset, val); +} + +u32 clk_ti_readl(struct clk_ti_reg *reg) +{ + struct clk_iomap *io = &clk_memmaps[reg->index]; + u32 val; + + regmap_read(io->regmap, reg->offset, &val); + return val; +} + +static ofnode clk_ti_get_regmap_node(struct udevice *dev) +{ + ofnode node = dev_ofnode(dev), parent; + + if (!ofnode_valid(node)) + return ofnode_null(); + + parent = ofnode_get_parent(node); + if (strcmp(ofnode_get_name(parent), "clocks")) + return ofnode_null(); + + return ofnode_get_parent(parent); +} + +int clk_ti_get_reg_addr(struct udevice *dev, int index, struct clk_ti_reg *reg) +{ + ofnode node; + int i, ret; + u32 val; + + ret = ofnode_read_u32_index(dev_ofnode(dev), "reg", index, &val); + if (ret) { + dev_err(dev, "%s must have reg[%d]\n", ofnode_get_name(node), + index); + return ret; + } + + /* parent = ofnode_get_parent(parent); */ + node = clk_ti_get_regmap_node(dev); + if (!ofnode_valid(node)) { + dev_err(dev, "failed to get regmap node\n"); + return -EFAULT; + } + + for (i = 0; i < clk_memmaps_num; i++) { + if (ofnode_equal(clk_memmaps[i].node, node)) + break; + } + + if (i == clk_memmaps_num) { + if (i == CLK_MAX_MEMMAPS) + return -ENOMEM; + + ret = regmap_init_mem(node, &clk_memmaps[i].regmap); + if (ret) + return ret; + + clk_memmaps[i].node = node; + clk_memmaps_num++; + } + + reg->index = i; + reg->offset = val; + return 0; +} diff --git a/drivers/clk/ti/clk.h b/drivers/clk/ti/clk.h index 601c3823f7..ea36d065ac 100644 --- a/drivers/clk/ti/clk.h +++ b/drivers/clk/ti/clk.h @@ -9,5 +9,18 @@ #define _CLK_TI_H
void clk_ti_latch(fdt_addr_t reg, s8 shift); +/** + * struct clk_ti_reg - TI register declaration + * @offset: offset from the master IP module base address + * @index: index of the master IP module + */ +struct clk_ti_reg { + u16 offset; + u8 index; +}; + +void clk_ti_writel(u32 val, struct clk_ti_reg *reg); +u32 clk_ti_readl(struct clk_ti_reg *reg); +int clk_ti_get_reg_addr(struct udevice *dev, int index, struct clk_ti_reg *reg);
#endif /* #ifndef _CLK_TI_H */
The clock access functions exported by the clk header use the struct clk_ti_reg parameter to get the address of the register. This must also apply to clk_ti_latch(). Changes to TI's clk-mux and clk-divider drivers prevented the patch from generating compile errors.
Signed-off-by: Dario Binacchi dariobin@libero.it ---
(no changes since v1)
drivers/clk/ti/clk-divider.c | 20 ++++++++++++-------- drivers/clk/ti/clk-mux.c | 20 ++++++++++---------- drivers/clk/ti/clk.c | 10 +++++----- drivers/clk/ti/clk.h | 2 +- 4 files changed, 28 insertions(+), 24 deletions(-)
diff --git a/drivers/clk/ti/clk-divider.c b/drivers/clk/ti/clk-divider.c index 270f2fbdf0..15941f1781 100644 --- a/drivers/clk/ti/clk-divider.c +++ b/drivers/clk/ti/clk-divider.c @@ -27,7 +27,7 @@
struct clk_ti_divider_priv { struct clk parent; - fdt_addr_t reg; + struct clk_ti_reg reg; const struct clk_div_table *table; u8 shift; u8 flags; @@ -200,11 +200,11 @@ static ulong clk_ti_divider_set_rate(struct clk *clk, ulong rate)
val = _get_val(priv->table, priv->div_flags, div);
- v = readl(priv->reg); + v = clk_ti_readl(&priv->reg); v &= ~(priv->mask << priv->shift); v |= val << priv->shift; - writel(v, priv->reg); - clk_ti_latch(priv->reg, priv->latch); + clk_ti_writel(v, &priv->reg); + clk_ti_latch(&priv->reg, priv->latch);
return clk_get_rate(clk); } @@ -220,7 +220,7 @@ static ulong clk_ti_divider_get_rate(struct clk *clk) if (IS_ERR_VALUE(parent_rate)) return parent_rate;
- v = readl(priv->reg) >> priv->shift; + v = clk_ti_readl(&priv->reg) >> priv->shift; v &= priv->mask;
div = _get_div(priv->table, priv->div_flags, v); @@ -287,10 +287,14 @@ static int clk_ti_divider_of_to_plat(struct udevice *dev) u32 min_div = 0; u32 max_val, max_div = 0; u16 mask; - int i, div_num; + int i, div_num, err; + + err = clk_ti_get_reg_addr(dev, 0, &priv->reg); + if (err) { + dev_err(dev, "failed to get register address\n"); + return err; + }
- priv->reg = dev_read_addr(dev); - dev_dbg(dev, "reg=0x%08lx\n", priv->reg); priv->shift = dev_read_u32_default(dev, "ti,bit-shift", 0); priv->latch = dev_read_s32_default(dev, "ti,latch-bit", -EINVAL); if (dev_read_bool(dev, "ti,index-starts-at-one")) diff --git a/drivers/clk/ti/clk-mux.c b/drivers/clk/ti/clk-mux.c index bb5e49e114..215241b161 100644 --- a/drivers/clk/ti/clk-mux.c +++ b/drivers/clk/ti/clk-mux.c @@ -17,7 +17,7 @@
struct clk_ti_mux_priv { struct clk_bulk parents; - fdt_addr_t reg; + struct clk_ti_reg reg; u32 flags; u32 mux_flags; u32 mask; @@ -58,7 +58,7 @@ static int clk_ti_mux_get_index(struct clk *clk) struct clk_ti_mux_priv *priv = dev_get_priv(clk->dev); u32 val;
- val = readl(priv->reg); + val = clk_ti_readl(&priv->reg); val >>= priv->shift; val &= priv->mask;
@@ -91,13 +91,13 @@ static int clk_ti_mux_set_parent(struct clk *clk, struct clk *parent) if (priv->flags & CLK_MUX_HIWORD_MASK) { val = priv->mask << (priv->shift + 16); } else { - val = readl(priv->reg); + val = clk_ti_readl(&priv->reg); val &= ~(priv->mask << priv->shift); }
val |= index << priv->shift; - writel(val, priv->reg); - clk_ti_latch(priv->reg, priv->latch); + clk_ti_writel(val, &priv->reg); + clk_ti_latch(&priv->reg, priv->latch); return 0; }
@@ -215,14 +215,14 @@ static int clk_ti_mux_probe(struct udevice *dev) static int clk_ti_mux_of_to_plat(struct udevice *dev) { struct clk_ti_mux_priv *priv = dev_get_priv(dev); + int err;
- priv->reg = dev_read_addr(dev); - if (priv->reg == FDT_ADDR_T_NONE) { - dev_err(dev, "failed to get register\n"); - return -EINVAL; + err = clk_ti_get_reg_addr(dev, 0, &priv->reg); + if (err) { + dev_err(dev, "failed to get register address\n"); + return err; }
- dev_dbg(dev, "reg=0x%08lx\n", priv->reg); priv->shift = dev_read_u32_default(dev, "ti,bit-shift", 0); priv->latch = dev_read_s32_default(dev, "ti,latch-bit", -EINVAL);
diff --git a/drivers/clk/ti/clk.c b/drivers/clk/ti/clk.c index 7a413d668d..6e5cc90f0f 100644 --- a/drivers/clk/ti/clk.c +++ b/drivers/clk/ti/clk.c @@ -23,17 +23,17 @@ struct clk_iomap { static unsigned int clk_memmaps_num; static struct clk_iomap clk_memmaps[CLK_MAX_MEMMAPS];
-static void clk_ti_rmw(u32 val, u32 mask, fdt_addr_t reg) +static void clk_ti_rmw(u32 val, u32 mask, struct clk_ti_reg *reg) { u32 v;
- v = readl(reg); + v = clk_ti_readl(reg); v &= ~mask; v |= val; - writel(v, reg); + clk_ti_writel(v, reg); }
-void clk_ti_latch(fdt_addr_t reg, s8 shift) +void clk_ti_latch(struct clk_ti_reg *reg, s8 shift) { u32 latch;
@@ -44,7 +44,7 @@ void clk_ti_latch(fdt_addr_t reg, s8 shift)
clk_ti_rmw(latch, latch, reg); clk_ti_rmw(0, latch, reg); - readl(reg); /* OCP barrier */ + clk_ti_readl(reg); /* OCP barrier */ }
void clk_ti_writel(u32 val, struct clk_ti_reg *reg) diff --git a/drivers/clk/ti/clk.h b/drivers/clk/ti/clk.h index ea36d065ac..96859f9dea 100644 --- a/drivers/clk/ti/clk.h +++ b/drivers/clk/ti/clk.h @@ -8,7 +8,6 @@ #ifndef _CLK_TI_H #define _CLK_TI_H
-void clk_ti_latch(fdt_addr_t reg, s8 shift); /** * struct clk_ti_reg - TI register declaration * @offset: offset from the master IP module base address @@ -19,6 +18,7 @@ struct clk_ti_reg { u8 index; };
+void clk_ti_latch(struct clk_ti_reg *reg, s8 shift); void clk_ti_writel(u32 val, struct clk_ti_reg *reg); u32 clk_ti_readl(struct clk_ti_reg *reg); int clk_ti_get_reg_addr(struct udevice *dev, int index, struct clk_ti_reg *reg);
Replaces the common memory access functions used by the driver with the ones exported from the TI clk module.
Signed-off-by: Dario Binacchi dariobin@libero.it ---
(no changes since v1)
drivers/clk/ti/clk-gate.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/clk/ti/clk-gate.c b/drivers/clk/ti/clk-gate.c index 0ca453c8de..eb15f6243f 100644 --- a/drivers/clk/ti/clk-gate.c +++ b/drivers/clk/ti/clk-gate.c @@ -13,9 +13,10 @@ #include <clk-uclass.h> #include <asm/io.h> #include <linux/clk-provider.h> +#include "clk.h"
struct clk_ti_gate_priv { - fdt_addr_t reg; + struct clk_ti_reg reg; u8 enable_bit; u32 flags; bool invert_enable; @@ -26,13 +27,13 @@ static int clk_ti_gate_disable(struct clk *clk) struct clk_ti_gate_priv *priv = dev_get_priv(clk->dev); u32 v;
- v = readl(priv->reg); + v = clk_ti_readl(&priv->reg); if (priv->invert_enable) v |= (1 << priv->enable_bit); else v &= ~(1 << priv->enable_bit);
- writel(v, priv->reg); + clk_ti_writel(v, &priv->reg); /* No OCP barrier needed here since it is a disable operation */ return 0; } @@ -42,29 +43,29 @@ static int clk_ti_gate_enable(struct clk *clk) struct clk_ti_gate_priv *priv = dev_get_priv(clk->dev); u32 v;
- v = readl(priv->reg); + v = clk_ti_readl(&priv->reg); if (priv->invert_enable) v &= ~(1 << priv->enable_bit); else v |= (1 << priv->enable_bit);
- writel(v, priv->reg); + clk_ti_writel(v, &priv->reg); /* OCP barrier */ - v = readl(priv->reg); + v = clk_ti_readl(&priv->reg); return 0; }
static int clk_ti_gate_of_to_plat(struct udevice *dev) { struct clk_ti_gate_priv *priv = dev_get_priv(dev); + int err;
- priv->reg = dev_read_addr(dev); - if (priv->reg == FDT_ADDR_T_NONE) { - dev_err(dev, "failed to get control register\n"); - return -EINVAL; + err = clk_ti_get_reg_addr(dev, 0, &priv->reg); + if (err) { + dev_err(dev, "failed to get control register address\n"); + return err; }
- dev_dbg(dev, "reg=0x%08lx\n", priv->reg); priv->enable_bit = dev_read_u32_default(dev, "ti,bit-shift", 0); if (dev_read_bool(dev, "ti,set-rate-parent")) priv->flags |= CLK_SET_RATE_PARENT;
Using the custom TI functions required not only replacing common memory access functions but also rewriting the routines used to set bypass and lock states. As for readl() and writel(), they also required the address of the register to be accessed, a parameter that is hidden by the TI clk module.
Signed-off-by: Dario Binacchi dariobin@libero.it ---
(no changes since v1)
drivers/clk/ti/clk-am3-dpll.c | 86 +++++++++++++++++++++-------------- 1 file changed, 53 insertions(+), 33 deletions(-)
diff --git a/drivers/clk/ti/clk-am3-dpll.c b/drivers/clk/ti/clk-am3-dpll.c index 90b4cc69ef..916d308034 100644 --- a/drivers/clk/ti/clk-am3-dpll.c +++ b/drivers/clk/ti/clk-am3-dpll.c @@ -17,15 +17,16 @@ #include <asm/arch/clock.h> #include <asm/arch/sys_proto.h> #include <asm/io.h> +#include "clk.h"
struct clk_ti_am3_dpll_drv_data { ulong max_rate; };
struct clk_ti_am3_dpll_priv { - fdt_addr_t clkmode_reg; - fdt_addr_t idlest_reg; - fdt_addr_t clksel_reg; + struct clk_ti_reg clkmode_reg; + struct clk_ti_reg idlest_reg; + struct clk_ti_reg clksel_reg; struct clk clk_bypass; struct clk clk_ref; u16 last_rounded_mult; @@ -75,6 +76,37 @@ static ulong clk_ti_am3_dpll_round_rate(struct clk *clk, ulong rate) return ret; }
+static void clk_ti_am3_dpll_clken(struct clk_ti_am3_dpll_priv *priv, + u8 clken_bits) +{ + u32 v; + + v = clk_ti_readl(&priv->clkmode_reg); + v &= ~CM_CLKMODE_DPLL_DPLL_EN_MASK; + v |= clken_bits << CM_CLKMODE_DPLL_EN_SHIFT; + clk_ti_writel(v, &priv->clkmode_reg); +} + +static int clk_ti_am3_dpll_state(struct clk *clk, u8 state) +{ + struct clk_ti_am3_dpll_priv *priv = dev_get_priv(clk->dev); + u32 i = 0, v; + + do { + v = clk_ti_readl(&priv->idlest_reg) & ST_DPLL_CLK_MASK; + if (v == state) { + dev_dbg(clk->dev, "transition to '%s' in %d loops\n", + state ? "locked" : "bypassed", i); + return 1; + } + + } while (++i < LDELAY); + + dev_err(clk->dev, "failed transition to '%s'\n", + state ? "locked" : "bypassed"); + return 0; +} + static ulong clk_ti_am3_dpll_set_rate(struct clk *clk, ulong rate) { struct clk_ti_am3_dpll_priv *priv = dev_get_priv(clk->dev); @@ -85,16 +117,13 @@ static ulong clk_ti_am3_dpll_set_rate(struct clk *clk, ulong rate) if (IS_ERR_VALUE(round_rate)) return round_rate;
- v = readl(priv->clksel_reg); + v = clk_ti_readl(&priv->clksel_reg);
/* enter bypass mode */ - clrsetbits_le32(priv->clkmode_reg, CM_CLKMODE_DPLL_DPLL_EN_MASK, - DPLL_EN_MN_BYPASS << CM_CLKMODE_DPLL_EN_SHIFT); + clk_ti_am3_dpll_clken(priv, DPLL_EN_MN_BYPASS);
/* wait for bypass mode */ - if (!wait_on_value(ST_DPLL_CLK_MASK, 0, - (void *)priv->idlest_reg, LDELAY)) - dev_err(clk->dev, "failed bypassing dpll\n"); + clk_ti_am3_dpll_state(clk, 0);
/* set M & N */ v &= ~CM_CLKSEL_DPLL_M_MASK; @@ -105,18 +134,14 @@ static ulong clk_ti_am3_dpll_set_rate(struct clk *clk, ulong rate) v |= ((priv->last_rounded_div - 1) << CM_CLKSEL_DPLL_N_SHIFT) & CM_CLKSEL_DPLL_N_MASK;
- writel(v, priv->clksel_reg); + clk_ti_writel(v, &priv->clksel_reg);
/* lock dpll */ - clrsetbits_le32(priv->clkmode_reg, CM_CLKMODE_DPLL_DPLL_EN_MASK, - DPLL_EN_LOCK << CM_CLKMODE_DPLL_EN_SHIFT); + clk_ti_am3_dpll_clken(priv, DPLL_EN_LOCK);
/* wait till the dpll locks */ - if (!wait_on_value(ST_DPLL_CLK_MASK, ST_DPLL_CLK_MASK, - (void *)priv->idlest_reg, LDELAY)) { - dev_err(clk->dev, "failed locking dpll\n"); + if (!clk_ti_am3_dpll_state(clk, ST_DPLL_CLK_MASK)) hang(); - }
return round_rate; } @@ -128,7 +153,7 @@ static ulong clk_ti_am3_dpll_get_rate(struct clk *clk) u32 m, n, v;
/* Return bypass rate if DPLL is bypassed */ - v = readl(priv->clkmode_reg); + v = clk_ti_readl(&priv->clkmode_reg); v &= CM_CLKMODE_DPLL_EN_MASK; v >>= CM_CLKMODE_DPLL_EN_SHIFT;
@@ -141,7 +166,7 @@ static ulong clk_ti_am3_dpll_get_rate(struct clk *clk) return rate; }
- v = readl(priv->clksel_reg); + v = clk_ti_readl(&priv->clksel_reg); m = v & CM_CLKSEL_DPLL_M_MASK; m >>= CM_CLKSEL_DPLL_M_SHIFT; n = v & CM_CLKSEL_DPLL_N_MASK; @@ -204,33 +229,28 @@ static int clk_ti_am3_dpll_of_to_plat(struct udevice *dev) struct clk_ti_am3_dpll_priv *priv = dev_get_priv(dev); struct clk_ti_am3_dpll_drv_data *data = (struct clk_ti_am3_dpll_drv_data *)dev_get_driver_data(dev); + int err;
priv->max_rate = data->max_rate;
- priv->clkmode_reg = dev_read_addr_index(dev, 0); - if (priv->clkmode_reg == FDT_ADDR_T_NONE) { - dev_err(dev, "failed to get clkmode register\n"); - return -EINVAL; + err = clk_ti_get_reg_addr(dev, 0, &priv->clkmode_reg); + if (err) { + dev_err(dev, "failed to get clkmode register address\n"); + return err; }
- dev_dbg(dev, "clkmode_reg=0x%08lx\n", priv->clkmode_reg); - - priv->idlest_reg = dev_read_addr_index(dev, 1); - if (priv->idlest_reg == FDT_ADDR_T_NONE) { + err = clk_ti_get_reg_addr(dev, 1, &priv->idlest_reg); + if (err) { dev_err(dev, "failed to get idlest register\n"); return -EINVAL; }
- dev_dbg(dev, "idlest_reg=0x%08lx\n", priv->idlest_reg); - - priv->clksel_reg = dev_read_addr_index(dev, 2); - if (priv->clksel_reg == FDT_ADDR_T_NONE) { + err = clk_ti_get_reg_addr(dev, 2, &priv->clksel_reg); + if (err) { dev_err(dev, "failed to get clksel register\n"); - return -EINVAL; + return err; }
- dev_dbg(dev, "clksel_reg=0x%08lx\n", priv->clksel_reg); - return 0; }
This reverts commit d64b9cdcd475eb7f07b49741ded87e24dae4a5fc.
As pointed by [1] and [2], the reverted patch made every DT 'reg' property translatable. What the patch was trying to fix was fixed in a different way from previously submitted patches which instead of correcting the generic address translation function fixed the issue with appropriate platform code.
[1] https://patchwork.ozlabs.org/project/uboot/patch/1614324949-61314-1-git-send... [2] https://lore.kernel.org/linux-clk/20210402192054.7934-1-dariobin@libero.it/T...
Signed-off-by: Dario Binacchi dariobin@libero.it Reviewed-by: Bin Meng bmeng.cn@gmail.com
---
Changes in v2: - Added Bin Meng Reviewed-by tag.
arch/sandbox/dts/test.dts | 21 ---------- common/fdt_support.c | 6 +-- drivers/core/Kconfig | 12 ------ drivers/core/fdtaddr.c | 2 +- drivers/core/of_addr.c | 13 ++++-- drivers/core/ofnode.c | 7 +--- drivers/core/root.c | 3 -- include/asm-generic/global_data.h | 24 ----------- test/dm/test-fdt.c | 68 ------------------------------- 9 files changed, 15 insertions(+), 141 deletions(-)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 48240aa26f..e23941bab0 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -45,7 +45,6 @@ fdt-dummy1 = "/translation-test@8000/dev@1,100"; fdt-dummy2 = "/translation-test@8000/dev@2,200"; fdt-dummy3 = "/translation-test@8000/noxlatebus@3,300/dev@42"; - fdt-dummy4 = "/translation-test@8000/xlatebus@4,400/devs/dev@19"; usb0 = &usb_0; usb1 = &usb_1; usb2 = &usb_2; @@ -1263,7 +1262,6 @@ 1 0x100 0x9000 0x1000 2 0x200 0xA000 0x1000 3 0x300 0xB000 0x1000 - 4 0x400 0xC000 0x1000 >;
dma-ranges = <0 0x000 0x10000000 0x1000 @@ -1300,25 +1298,6 @@ reg = <0x42>; }; }; - - xlatebus@4,400 { - compatible = "sandbox,zero-size-cells-bus"; - reg = <4 0x400 0x1000>; - #address-cells = <1>; - #size-cells = <1>; - ranges = <0 4 0x400 0x1000>; - - devs { - #address-cells = <1>; - #size-cells = <0>; - - dev@19 { - compatible = "denx,u-boot-fdt-dummy"; - reg = <0x19>; - }; - }; - }; - };
osd { diff --git a/common/fdt_support.c b/common/fdt_support.c index e624bbdf40..79b7f2423c 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -20,8 +20,6 @@ #include <exports.h> #include <fdtdec.h>
-DECLARE_GLOBAL_DATA_PTR; - /** * fdt_getprop_u32_default_node - Return a node's property or a default * @@ -991,8 +989,8 @@ void fdt_del_node_and_alias(void *blob, const char *alias) /* Max address size we deal with */ #define OF_MAX_ADDR_CELLS 4 #define OF_BAD_ADDR FDT_ADDR_T_NONE -#define OF_CHECK_COUNTS(na, ns) (((na) > 0 && (na) <= OF_MAX_ADDR_CELLS) && \ - ((ns) > 0 || gd_size_cells_0())) +#define OF_CHECK_COUNTS(na, ns) ((na) > 0 && (na) <= OF_MAX_ADDR_CELLS && \ + (ns) > 0)
/* Debug utility */ #ifdef DEBUG diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig index a7c3120860..d618e1637b 100644 --- a/drivers/core/Kconfig +++ b/drivers/core/Kconfig @@ -271,18 +271,6 @@ config OF_TRANSLATE used for the address translation. This function is faster and smaller in size than fdt_translate_address().
-config OF_TRANSLATE_ZERO_SIZE_CELLS - bool "Enable translation for zero size cells" - depends on OF_TRANSLATE - default n - help - The routine used to translate an FDT address into a physical CPU - address was developed by IBM. It considers that crossing any level - with #size-cells = <0> makes translation impossible, even if it is - not the way it was specified. - Enabling this option makes translation possible even in the case - of crossing levels with #size-cells = <0>. - config SPL_OF_TRANSLATE bool "Translate addresses using fdt_translate_address in SPL" depends on SPL_DM && SPL_OF_CONTROL diff --git a/drivers/core/fdtaddr.c b/drivers/core/fdtaddr.c index 83a50b6a3a..b9874c743d 100644 --- a/drivers/core/fdtaddr.c +++ b/drivers/core/fdtaddr.c @@ -50,7 +50,7 @@ fdt_addr_t devfdt_get_addr_index(const struct udevice *dev, int index)
reg += index * (na + ns);
- if (ns || gd_size_cells_0()) { + if (ns) { /* * Use the full-fledged translate function for complex * bus setups. diff --git a/drivers/core/of_addr.c b/drivers/core/of_addr.c index b3e384d2ee..9b77308182 100644 --- a/drivers/core/of_addr.c +++ b/drivers/core/of_addr.c @@ -18,8 +18,7 @@ /* Max address size we deal with */ #define OF_MAX_ADDR_CELLS 4 #define OF_CHECK_ADDR_COUNT(na) ((na) > 0 && (na) <= OF_MAX_ADDR_CELLS) -#define OF_CHECK_COUNTS(na, ns) (OF_CHECK_ADDR_COUNT(na) && \ - ((ns) > 0 || gd_size_cells_0())) +#define OF_CHECK_COUNTS(na, ns) (OF_CHECK_ADDR_COUNT(na) && (ns) > 0)
static struct of_bus *of_match_bus(struct device_node *np);
@@ -163,6 +162,11 @@ const __be32 *of_get_address(const struct device_node *dev, int index, } EXPORT_SYMBOL(of_get_address);
+static int of_empty_ranges_quirk(const struct device_node *np) +{ + return false; +} + static int of_translate_one(const struct device_node *parent, struct of_bus *bus, struct of_bus *pbus, __be32 *addr, int na, int ns, int pna, @@ -189,8 +193,11 @@ static int of_translate_one(const struct device_node *parent, * As far as we know, this damage only exists on Apple machines, so * This code is only enabled on powerpc. --gcl */ - ranges = of_get_property(parent, rprop, &rlen); + if (ranges == NULL && !of_empty_ranges_quirk(parent)) { + debug("no ranges; cannot translate\n"); + return 1; + } if (ranges == NULL || rlen == 0) { offset = of_read_number(addr, na); memset(addr, 0, pna * 4); diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c index fa0bd2a9c4..e4d457ecb4 100644 --- a/drivers/core/ofnode.c +++ b/drivers/core/ofnode.c @@ -317,8 +317,7 @@ fdt_addr_t ofnode_get_addr_size_index(ofnode node, int index, fdt_size_t *size)
ns = of_n_size_cells(ofnode_to_np(node));
- if (IS_ENABLED(CONFIG_OF_TRANSLATE) && - (ns > 0 || gd_size_cells_0())) { + if (IS_ENABLED(CONFIG_OF_TRANSLATE) && ns > 0) { return of_translate_address(ofnode_to_np(node), prop_val); } else { na = of_n_addr_cells(ofnode_to_np(node)); @@ -692,10 +691,8 @@ fdt_addr_t ofnode_get_addr_size(ofnode node, const char *property, ns = of_n_size_cells(np); *sizep = of_read_number(prop + na, ns);
- if (CONFIG_IS_ENABLED(OF_TRANSLATE) && - (ns > 0 || gd_size_cells_0())) { + if (CONFIG_IS_ENABLED(OF_TRANSLATE) && ns > 0) return of_translate_address(np, prop); - } else return of_read_number(prop, na); } else { diff --git a/drivers/core/root.c b/drivers/core/root.c index d9a19c5e6b..02eb352313 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -164,9 +164,6 @@ int dm_init(bool of_live) { int ret;
- if (IS_ENABLED(CONFIG_OF_TRANSLATE_ZERO_SIZE_CELLS)) - gd->dm_flags |= GD_DM_FLG_SIZE_CELLS_0; - if (gd->dm_root) { dm_warn("Virtual root driver already exists!\n"); return -EINVAL; diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index e1a5f4b1d1..47921d27b1 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -183,12 +183,6 @@ struct global_data { struct global_data *new_gd;
#ifdef CONFIG_DM - /** - * @dm_flags: additional flags for Driver Model - * - * See &enum gd_dm_flags - */ - unsigned long dm_flags; /** * @dm_root: root instance for Driver Model */ @@ -519,12 +513,6 @@ struct global_data { #define gd_acpi_ctx() NULL #endif
-#if CONFIG_IS_ENABLED(DM) -#define gd_size_cells_0() (gd->dm_flags & GD_DM_FLG_SIZE_CELLS_0) -#else -#define gd_size_cells_0() (0) -#endif - /** * enum gd_flags - global data flags * @@ -609,18 +597,6 @@ enum gd_flags { GD_FLG_SMP_READY = 0x40000, };
-/** - * enum gd_dm_flags - global data flags for Driver Model - * - * See field dm_flags of &struct global_data. - */ -enum gd_dm_flags { - /** - * @GD_DM_FLG_SIZE_CELLS_0: Enable #size-cells=<0> translation - */ - GD_DM_FLG_SIZE_CELLS_0 = 0x00001, -}; - #endif /* __ASSEMBLY__ */
#endif /* __ASM_GENERIC_GBL_DATA_H */ diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index 6552d09ba3..9b771fdf19 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -5,7 +5,6 @@
#include <common.h> #include <dm.h> -#include <dm/device_compat.h> #include <errno.h> #include <fdtdec.h> #include <log.h> @@ -551,64 +550,6 @@ U_BOOT_DRIVER(fdt_dummy_drv) = { .id = UCLASS_TEST_DUMMY, };
-static int zero_size_cells_bus_bind(struct udevice *dev) -{ - ofnode child; - int err; - - ofnode_for_each_subnode(child, dev_ofnode(dev)) { - if (ofnode_get_property(child, "compatible", NULL)) - continue; - - err = device_bind_driver_to_node(dev, - "zero_size_cells_bus_child_drv", - "zero_size_cells_bus_child", - child, NULL); - if (err) { - dev_err(dev, "%s: failed to bind %s\n", __func__, - ofnode_get_name(child)); - return err; - } - } - - return 0; -} - -static const struct udevice_id zero_size_cells_bus_ids[] = { - { .compatible = "sandbox,zero-size-cells-bus" }, - { } -}; - -U_BOOT_DRIVER(zero_size_cells_bus) = { - .name = "zero_size_cells_bus_drv", - .id = UCLASS_TEST_DUMMY, - .of_match = zero_size_cells_bus_ids, - .bind = zero_size_cells_bus_bind, -}; - -static int zero_size_cells_bus_child_bind(struct udevice *dev) -{ - ofnode child; - int err; - - ofnode_for_each_subnode(child, dev_ofnode(dev)) { - err = lists_bind_fdt(dev, child, NULL, false); - if (err) { - dev_err(dev, "%s: lists_bind_fdt, err=%d\n", - __func__, err); - return err; - } - } - - return 0; -} - -U_BOOT_DRIVER(zero_size_cells_bus_child_drv) = { - .name = "zero_size_cells_bus_child_drv", - .id = UCLASS_TEST_DUMMY, - .bind = zero_size_cells_bus_child_bind, -}; - static int dm_test_fdt_translation(struct unit_test_state *uts) { struct udevice *dev; @@ -630,17 +571,8 @@ static int dm_test_fdt_translation(struct unit_test_state *uts) /* No translation for busses with #size-cells == 0 */ ut_assertok(uclass_find_device_by_seq(UCLASS_TEST_DUMMY, 3, &dev)); ut_asserteq_str("dev@42", dev->name); - /* No translation for busses with #size-cells == 0 */ ut_asserteq(0x42, dev_read_addr(dev));
- /* Translation for busses with #size-cells == 0 */ - gd->dm_flags |= GD_DM_FLG_SIZE_CELLS_0; - ut_asserteq(0x8042, dev_read_addr(dev)); - ut_assertok(uclass_find_device_by_seq(UCLASS_TEST_DUMMY, 4, &dev)); - ut_asserteq_str("dev@19", dev->name); - ut_asserteq(0xc019, dev_read_addr(dev)); - gd->dm_flags &= ~GD_DM_FLG_SIZE_CELLS_0; - /* dma address translation */ ut_assertok(uclass_find_device_by_seq(UCLASS_TEST_DUMMY, 0, &dev)); dma_addr[0] = cpu_to_be32(0);
On 01/05/2021 18:05, Dario Binacchi wrote:
As pointed by [1] and [2] the d64b9cdcd4 ("fdt: translate address if #size-cells = <0>") commit was wrong. The series reverts the patch and fixes the issue with platform code, adding custom routines to access the clocks registers. The solution has been inspired by the Linux Kernel code.
[1] https://patchwork.ozlabs.org/project/uboot/patch/1614324949-61314-1-git-send... [2] https://lore.kernel.org/linux-clk/20210402192054.7934-1-dariobin@libero.it/T...
Changes in v2:
- Remove #if CONFIG_IS_ENABLED(AM33XX). It was counter intuitive.
- Added Bin Meng Reviewed-by tag.
For the whole series:
Reviewed-by: Tero Kristo kristo@kernel.org
Dario Binacchi (5): clk: ti: add custom API for memory access clk: ti: change clk_ti_latch() signature clk: ti: gate: use custom API for memory access clk: ti: am3-dpll: use custom API for memory access Revert "fdt: translate address if #size-cells = <0>"
arch/sandbox/dts/test.dts | 21 ------- common/fdt_support.c | 6 +- drivers/clk/ti/clk-am3-dpll.c | 86 +++++++++++++++++----------- drivers/clk/ti/clk-divider.c | 20 ++++--- drivers/clk/ti/clk-gate.c | 23 ++++---- drivers/clk/ti/clk-mux.c | 20 +++---- drivers/clk/ti/clk.c | 95 +++++++++++++++++++++++++++++-- drivers/clk/ti/clk.h | 15 ++++- drivers/core/Kconfig | 12 ---- drivers/core/fdtaddr.c | 2 +- drivers/core/of_addr.c | 13 ++++- drivers/core/ofnode.c | 7 +-- drivers/core/root.c | 3 - include/asm-generic/global_data.h | 24 -------- test/dm/test-fdt.c | 68 ---------------------- 15 files changed, 206 insertions(+), 209 deletions(-)
On 01/05/21 8:35 pm, Dario Binacchi wrote:
As pointed by [1] and [2] the d64b9cdcd4 ("fdt: translate address if #size-cells = <0>") commit was wrong. The series reverts the patch and fixes the issue with platform code, adding custom routines to access the clocks registers. The solution has been inspired by the Linux Kernel code.
[1] https://patchwork.ozlabs.org/project/uboot/patch/1614324949-61314-1-git-send... [2] https://lore.kernel.org/linux-clk/20210402192054.7934-1-dariobin@libero.it/T...
Changes in v2:
- Remove #if CONFIG_IS_ENABLED(AM33XX). It was counter intuitive.
- Added Bin Meng Reviewed-by tag.
Applied to u-boot-ti/for-rc
Thanks and regards, Lokesh
participants (3)
-
Dario Binacchi -
Lokesh Vutla -
Tero Kristo