Hi Tero,
> Il 27/04/2021 09:01 Tero Kristo <kri...@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...@gmail.com/
> > [2]
> > https://lore.kernel.org/linux-clk/20210402192054.7934-1-dario...@libero.it/T/
> >
> > Signed-off-by: Dario Binacchi <dario...@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 */
> >
