[PATCH 0/5] Revert "fdt: translate address if #size-cells = <0>"

[PATCH 0/5] Revert "fdt: translate address if #size-cells = <0>"

[Dario Binacchi]

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-email-bmeng.cn at gmail.com/
[2] https://lore.kernel.org/linux-clk/20210402192054.7934-1-dariobin at libero.it/T/


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              | 102 ++++++++++++++++++++++++++++--
 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, 213 insertions(+), 209 deletions(-)

-- 
2.17.1

[PATCH 1/5] clk: ti: add custom API for memory access

[Dario Binacchi]

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-email-bmeng.cn at gmail.com/
[2] https://lore.kernel.org/linux-clk/20210402192054.7934-1-dariobin at libero.it/T/

Signed-off-by: Dario Binacchi <dariobin@libero.it>
---

 drivers/clk/ti/clk.c | 92 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/ti/clk.h | 13 +++++++
 2 files changed, 105 insertions(+)

diff --git a/drivers/clk/ti/clk.c b/drivers/clk/ti/clk.c
index c999df213a..68abe053cb 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,82 @@ 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;
+}
+
+#if CONFIG_IS_ENABLED(AM33XX)
+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);
+}
+#else
+static ofnode clk_ti_get_regmap_node(struct udevice *dev)
+{
+	return ofnode_null();
+}
+#endif
+
+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 */
-- 
2.17.1

[PATCH 2/5] clk: ti: change clk_ti_latch() signature

[Dario Binacchi]

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

 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 68abe053cb..8babadcb8a 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);
-- 
2.17.1

[PATCH 3/5] clk: ti: gate: use custom API for memory access

[Dario Binacchi]

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

 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;
-- 
2.17.1

[PATCH 4/5] clk: ti: am3-dpll: use custom API for memory access

[Dario Binacchi]

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

 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;
 }
 
-- 
2.17.1

[PATCH 5/5] Revert "fdt: translate address if #size-cells = <0>"

[Dario Binacchi]

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-email-bmeng.cn at gmail.com/
[2] https://lore.kernel.org/linux-clk/20210402192054.7934-1-dariobin at libero.it/T/

Signed-off-by: Dario Binacchi <dariobin@libero.it>

---

 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 at 8000/dev at 1,100";
 		fdt-dummy2 = "/translation-test at 8000/dev at 2,200";
 		fdt-dummy3 = "/translation-test at 8000/noxlatebus at 3,300/dev at 42";
-		fdt-dummy4 = "/translation-test at 8000/xlatebus at 4,400/devs/dev at 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 at 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 at 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 at 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 at 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);
-- 
2.17.1

[PATCH 5/5] Revert "fdt: translate address if #size-cells = <0>"

[Bin Meng]

On Sun, Apr 25, 2021 at 10:17 PM Dario Binacchi <dariobin@libero.it> wrote:
>
> 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-email-bmeng.cn at gmail.com/
> [2] https://lore.kernel.org/linux-clk/20210402192054.7934-1-dariobin at libero.it/T/
>
> Signed-off-by: Dario Binacchi <dariobin@libero.it>
>
> ---
>
>  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(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH 1/5] clk: ti: add custom API for memory access

[Tero Kristo]

Hi Dario,

One question below.

On 25/04/2021 17:17, Dario Binacchi wrote:
> 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-email-bmeng.cn at gmail.com/
> [2] https://lore.kernel.org/linux-clk/20210402192054.7934-1-dariobin at libero.it/T/
> 
> Signed-off-by: Dario Binacchi <dariobin@libero.it>
> ---
> 
>   drivers/clk/ti/clk.c | 92 ++++++++++++++++++++++++++++++++++++++++++++
>   drivers/clk/ti/clk.h | 13 +++++++
>   2 files changed, 105 insertions(+)
> 
> diff --git a/drivers/clk/ti/clk.c b/drivers/clk/ti/clk.c
> index c999df213a..68abe053cb 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,82 @@ 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;
> +}
> +
> +#if CONFIG_IS_ENABLED(AM33XX)

Why do you have this ifdef here? These drivers are not planned to be 
used by anything but am33xx, or they don't work on any other device?

-Tero

> +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);
> +}
> +#else
> +static ofnode clk_ti_get_regmap_node(struct udevice *dev)
> +{
> +	return ofnode_null();
> +}
> +#endif
> +
> +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 */
> 

[PATCH 1/5] clk: ti: add custom API for memory access

[Dario Binacchi]

Hi Tero,

> Il 27/04/2021 09:01 Tero Kristo <kristo@kernel.org> ha scritto:
> 
>  
> Hi Dario,
> 
> One question below.
> 
> On 25/04/2021 17:17, Dario Binacchi wrote:
> > 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-email-bmeng.cn at gmail.com/
> > [2] https://lore.kernel.org/linux-clk/20210402192054.7934-1-dariobin at libero.it/T/
> > 
> > Signed-off-by: Dario Binacchi <dariobin@libero.it>
> > ---
> > 
> >   drivers/clk/ti/clk.c | 92 ++++++++++++++++++++++++++++++++++++++++++++
> >   drivers/clk/ti/clk.h | 13 +++++++
> >   2 files changed, 105 insertions(+)
> > 
> > diff --git a/drivers/clk/ti/clk.c b/drivers/clk/ti/clk.c
> > index c999df213a..68abe053cb 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,82 @@ 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;
> > +}
> > +
> > +#if CONFIG_IS_ENABLED(AM33XX)
> 
> Why do you have this ifdef here? These drivers are not planned to be 
> used by anything but am33xx, or they don't work on any other device?
> 

The patch was developed and tested for the AM33XX SOC (beaglebone black). 
The drivers in the clk/ti folder were added by my patches but can also be
used by boards based on different device trees. In those cases, if required, 
platform versions of clk_ti_get_regmap_node() could be implemented.

Thanks and regards,
Dario

> -Tero
> 
> > +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);
> > +}
> > +#else
> > +static ofnode clk_ti_get_regmap_node(struct udevice *dev)
> > +{
> > +	return ofnode_null();
> > +}
> > +#endif
> > +
> > +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 */
> >

[PATCH 1/5] clk: ti: add custom API for memory access

[Tero Kristo]

On 28/04/2021 20:31, Dario Binacchi wrote:
> Hi Tero,
> 
>> Il 27/04/2021 09:01 Tero Kristo <kristo@kernel.org> ha scritto:
>>
>>   
>> Hi Dario,
>>
>> One question below.
>>
>> On 25/04/2021 17:17, Dario Binacchi wrote:
>>> 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-email-bmeng.cn at gmail.com/
>>> [2] https://lore.kernel.org/linux-clk/20210402192054.7934-1-dariobin at libero.it/T/
>>>
>>> Signed-off-by: Dario Binacchi <dariobin@libero.it>
>>> ---
>>>
>>>    drivers/clk/ti/clk.c | 92 ++++++++++++++++++++++++++++++++++++++++++++
>>>    drivers/clk/ti/clk.h | 13 +++++++
>>>    2 files changed, 105 insertions(+)
>>>
>>> diff --git a/drivers/clk/ti/clk.c b/drivers/clk/ti/clk.c
>>> index c999df213a..68abe053cb 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,82 @@ 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;
>>> +}
>>> +
>>> +#if CONFIG_IS_ENABLED(AM33XX)
>>
>> Why do you have this ifdef here? These drivers are not planned to be
>> used by anything but am33xx, or they don't work on any other device?
>>
> 
> The patch was developed and tested for the AM33XX SOC (beaglebone black).
> The drivers in the clk/ti folder were added by my patches but can also be
> used by boards based on different device trees. In those cases, if required,
> platform versions of clk_ti_get_regmap_node() could be implemented.

Ok, but this could be handled simply via the defconfigs? None of these 
drivers are enabled explicitly for any platform afaics. If someone wants 
to try this out, it might work out of box for, say am43xx also. Having 
to deal with this ifdef seems counter intuitive.

-Tero

> 
> Thanks and regards,
> Dario
> 
>> -Tero
>>
>>> +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);
>>> +}
>>> +#else
>>> +static ofnode clk_ti_get_regmap_node(struct udevice *dev)
>>> +{
>>> +	return ofnode_null();
>>> +}
>>> +#endif
>>> +
>>> +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 */
>>>

[PATCH 0/5] Revert "fdt: translate address if #size-cells = <0>"

[Simon Glass]

On Sun, 25 Apr 2021 at 07:17, Dario Binacchi <dariobin@libero.it> 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-email-bmeng.cn at gmail.com/
> [2] https://lore.kernel.org/linux-clk/20210402192054.7934-1-dariobin at libero.it/T/
>
>
> 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              | 102 ++++++++++++++++++++++++++++--
>  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, 213 insertions(+), 209 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>