[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@gmail.com/
> [2] https://lore.kernel.org/linux-clk/20210402192054.7934-1-dariobin@libero.it/T/
> 
> Signed-off-by: Dario Binacchi <dariobin at 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 */
>