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