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

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... [2] https://lore.kernel.org/linux-clk/20210402192054.7934-1-dariobin@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 */