| Message ID | 20170817182520.20102-3-prasannatsmkumar@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Herbert Xu |
| Headers | show |
Hi PrasannaKumar, On Thursday, 17 August 2017 11:25:16 PDT PrasannaKumar Muralidharan wrote: > Ingenic PRNG registers are a part of the same hardware block as clock > and power stuff. It is handled by CGU driver. Ingenic M200 SoC has power > related registers that are after the PRNG registers. So instead of > shortening the register range use syscon interface to expose regmap. > This regmap is used by the PRNG driver. > > Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com> > --- > arch/mips/boot/dts/ingenic/jz4740.dtsi | 14 +++++++---- > arch/mips/boot/dts/ingenic/jz4780.dtsi | 14 +++++++---- > drivers/clk/ingenic/cgu.c | 46 > +++++++++++++++++++++------------- drivers/clk/ingenic/cgu.h | > 9 +++++-- > drivers/clk/ingenic/jz4740-cgu.c | 30 +++++++++++----------- > drivers/clk/ingenic/jz4780-cgu.c | 10 ++++---- > 6 files changed, 73 insertions(+), 50 deletions(-) > > diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi > b/arch/mips/boot/dts/ingenic/jz4740.dtsi index 2ca7ce7..ada5e67 100644 > --- a/arch/mips/boot/dts/ingenic/jz4740.dtsi > +++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi > @@ -34,14 +34,18 @@ > clock-frequency = <32768>; > }; > > - cgu: jz4740-cgu@10000000 { > - compatible = "ingenic,jz4740-cgu"; > + cgu_registers { > + compatible = "simple-mfd", "syscon"; > reg = <0x10000000 0x100>; > > - clocks = <&ext>, <&rtc>; > - clock-names = "ext", "rtc"; > + cgu: jz4780-cgu@10000000 { > + compatible = "ingenic,jz4740-cgu"; > > - #clock-cells = <1>; > + clocks = <&ext>, <&rtc>; > + clock-names = "ext", "rtc"; > + > + #clock-cells = <1>; > + }; > }; > > rtc_dev: rtc@10003000 { > diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi > b/arch/mips/boot/dts/ingenic/jz4780.dtsi index 4853ef6..1301694 100644 > --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi > +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi > @@ -34,14 +34,18 @@ > clock-frequency = <32768>; > }; > > - cgu: jz4780-cgu@10000000 { > - compatible = "ingenic,jz4780-cgu"; > + cgu_registers { > + compatible = "simple-mfd", "syscon"; > reg = <0x10000000 0x100>; > > - clocks = <&ext>, <&rtc>; > - clock-names = "ext", "rtc"; > + cgu: jz4780-cgu@10000000 { > + compatible = "ingenic,jz4780-cgu"; > > - #clock-cells = <1>; > + clocks = <&ext>, <&rtc>; > + clock-names = "ext", "rtc"; > + > + #clock-cells = <1>; > + }; > }; > > pinctrl: pin-controller@10010000 { > diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c > index e8248f9..2f12c7c 100644 > --- a/drivers/clk/ingenic/cgu.c > +++ b/drivers/clk/ingenic/cgu.c > @@ -29,6 +29,18 @@ > > #define MHZ (1000 * 1000) > > +unsigned int cgu_readl(struct ingenic_cgu *cgu, unsigned int reg) > +{ > + unsigned int val = 0; > + regmap_read(cgu->regmap, reg, &val); > + return val; > +} > + > +int cgu_writel(struct ingenic_cgu *cgu, unsigned int val, unsigned int reg) > +{ > + return regmap_write(cgu->regmap, reg, val); > +} This is going to introduce a lot of overhead to the CGU driver - each regmap_read() or regmap_write() call will not only add overhead from the indirection but also acquire a lock which we really don't need. Could you instead perhaps: - Just add "syscon" as a second compatible string to the CGU node in the device tree, but otherwise leave it as-is without the extra cgu_registers node. - Have your RNG device node as a child of the CGU node, which should let it pick up the regmap via syscon_node_to_regmap() as it already does. - Leave the CGU driver as-is, so it can continue accessing memory directly rather than via regmap. Thanks, Paul > + > /** > * ingenic_cgu_gate_get() - get the value of clock gate register bit > * @cgu: reference to the CGU whose registers should be read > @@ -43,7 +55,7 @@ static inline bool > ingenic_cgu_gate_get(struct ingenic_cgu *cgu, > const struct ingenic_cgu_gate_info *info) > { > - return readl(cgu->base + info->reg) & BIT(info->bit); > + return cgu_readl(cgu, info->reg) & BIT(info->bit); > } > > /** > @@ -60,14 +72,14 @@ static inline void > ingenic_cgu_gate_set(struct ingenic_cgu *cgu, > const struct ingenic_cgu_gate_info *info, bool val) > { > - u32 clkgr = readl(cgu->base + info->reg); > + u32 clkgr = cgu_readl(cgu, info->reg); > > if (val) > clkgr |= BIT(info->bit); > else > clkgr &= ~BIT(info->bit); > > - writel(clkgr, cgu->base + info->reg); > + cgu_writel(cgu, clkgr, info->reg); > } > > /* > @@ -91,7 +103,7 @@ ingenic_pll_recalc_rate(struct clk_hw *hw, unsigned long > parent_rate) pll_info = &clk_info->pll; > > spin_lock_irqsave(&cgu->lock, flags); > - ctl = readl(cgu->base + pll_info->reg); > + ctl = cgu_readl(cgu, pll_info->reg); > spin_unlock_irqrestore(&cgu->lock, flags); > > m = (ctl >> pll_info->m_shift) & GENMASK(pll_info->m_bits - 1, 0); > @@ -190,7 +202,7 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned long > req_rate, clk_info->name, req_rate, rate); > > spin_lock_irqsave(&cgu->lock, flags); > - ctl = readl(cgu->base + pll_info->reg); > + ctl = cgu_readl(cgu, pll_info->reg); > > ctl &= ~(GENMASK(pll_info->m_bits - 1, 0) << pll_info->m_shift); > ctl |= (m - pll_info->m_offset) << pll_info->m_shift; > @@ -204,11 +216,11 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned long > req_rate, ctl &= ~BIT(pll_info->bypass_bit); > ctl |= BIT(pll_info->enable_bit); > > - writel(ctl, cgu->base + pll_info->reg); > + cgu_writel(cgu, ctl, pll_info->reg); > > /* wait for the PLL to stabilise */ > for (i = 0; i < timeout; i++) { > - ctl = readl(cgu->base + pll_info->reg); > + ctl = cgu_readl(cgu, pll_info->reg); > if (ctl & BIT(pll_info->stable_bit)) > break; > mdelay(1); > @@ -243,7 +255,7 @@ static u8 ingenic_clk_get_parent(struct clk_hw *hw) > clk_info = &cgu->clock_info[ingenic_clk->idx]; > > if (clk_info->type & CGU_CLK_MUX) { > - reg = readl(cgu->base + clk_info->mux.reg); > + reg = cgu_readl(cgu, clk_info->mux.reg); > hw_idx = (reg >> clk_info->mux.shift) & > GENMASK(clk_info->mux.bits - 1, 0); > > @@ -297,10 +309,10 @@ static int ingenic_clk_set_parent(struct clk_hw *hw, > u8 idx) spin_lock_irqsave(&cgu->lock, flags); > > /* write the register */ > - reg = readl(cgu->base + clk_info->mux.reg); > + reg = cgu_readl(cgu, clk_info->mux.reg); > reg &= ~mask; > reg |= hw_idx << clk_info->mux.shift; > - writel(reg, cgu->base + clk_info->mux.reg); > + cgu_writel(cgu, reg, clk_info->mux.reg); > > spin_unlock_irqrestore(&cgu->lock, flags); > return 0; > @@ -321,7 +333,7 @@ ingenic_clk_recalc_rate(struct clk_hw *hw, unsigned long > parent_rate) clk_info = &cgu->clock_info[ingenic_clk->idx]; > > if (clk_info->type & CGU_CLK_DIV) { > - div_reg = readl(cgu->base + clk_info->div.reg); > + div_reg = cgu_readl(cgu, clk_info->div.reg); > div = (div_reg >> clk_info->div.shift) & > GENMASK(clk_info->div.bits - 1, 0); > div += 1; > @@ -399,7 +411,7 @@ ingenic_clk_set_rate(struct clk_hw *hw, unsigned long > req_rate, return -EINVAL; > > spin_lock_irqsave(&cgu->lock, flags); > - reg = readl(cgu->base + clk_info->div.reg); > + reg = cgu_readl(cgu, clk_info->div.reg); > > /* update the divide */ > mask = GENMASK(clk_info->div.bits - 1, 0); > @@ -415,12 +427,12 @@ ingenic_clk_set_rate(struct clk_hw *hw, unsigned long > req_rate, reg |= BIT(clk_info->div.ce_bit); > > /* update the hardware */ > - writel(reg, cgu->base + clk_info->div.reg); > + cgu_writel(cgu, reg, clk_info->div.reg); > > /* wait for the change to take effect */ > if (clk_info->div.busy_bit != -1) { > for (i = 0; i < timeout; i++) { > - reg = readl(cgu->base + clk_info->div.reg); > + reg = cgu_readl(cgu, clk_info->div.reg); > if (!(reg & BIT(clk_info->div.busy_bit))) > break; > mdelay(1); > @@ -661,11 +673,9 @@ ingenic_cgu_new(const struct ingenic_cgu_clk_info > *clock_info, if (!cgu) > goto err_out; > > - cgu->base = of_iomap(np, 0); > - if (!cgu->base) { > - pr_err("%s: failed to map CGU registers\n", __func__); > + cgu->regmap = syscon_node_to_regmap(np->parent); > + if (cgu->regmap == NULL) > goto err_out_free; > - } > > cgu->np = np; > cgu->clock_info = clock_info; > diff --git a/drivers/clk/ingenic/cgu.h b/drivers/clk/ingenic/cgu.h > index 09700b2..86fd5b0 100644 > --- a/drivers/clk/ingenic/cgu.h > +++ b/drivers/clk/ingenic/cgu.h > @@ -19,7 +19,9 @@ > #define __DRIVERS_CLK_INGENIC_CGU_H__ > > #include <linux/bitops.h> > +#include <linux/mfd/syscon.h> > #include <linux/of.h> > +#include <linux/regmap.h> > #include <linux/spinlock.h> > > /** > @@ -171,14 +173,14 @@ struct ingenic_cgu_clk_info { > /** > * struct ingenic_cgu - data about the CGU > * @np: the device tree node that caused the CGU to be probed > - * @base: the ioremap'ed base address of the CGU registers > + * @regmap: the regmap object used to access the CGU registers > * @clock_info: an array containing information about implemented clocks > * @clocks: used to provide clocks to DT, allows lookup of struct clk* > * @lock: lock to be held whilst manipulating CGU registers > */ > struct ingenic_cgu { > struct device_node *np; > - void __iomem *base; > + struct regmap *regmap; > > const struct ingenic_cgu_clk_info *clock_info; > struct clk_onecell_data clocks; > @@ -186,6 +188,9 @@ struct ingenic_cgu { > spinlock_t lock; > }; > > +unsigned int cgu_readl(struct ingenic_cgu *cgu, unsigned int reg); > +int cgu_writel(struct ingenic_cgu *cgu, unsigned int val, unsigned int > reg); + > /** > * struct ingenic_clk - private data for a clock > * @hw: see Documentation/clk.txt > diff --git a/drivers/clk/ingenic/jz4740-cgu.c > b/drivers/clk/ingenic/jz4740-cgu.c index 510fe7e..3003afb 100644 > --- a/drivers/clk/ingenic/jz4740-cgu.c > +++ b/drivers/clk/ingenic/jz4740-cgu.c > @@ -232,7 +232,7 @@ CLK_OF_DECLARE(jz4740_cgu, "ingenic,jz4740-cgu", > jz4740_cgu_init); > > void jz4740_clock_set_wait_mode(enum jz4740_wait_mode mode) > { > - uint32_t lcr = readl(cgu->base + CGU_REG_LCR); > + uint32_t lcr = cgu_readl(cgu, CGU_REG_LCR); > > switch (mode) { > case JZ4740_WAIT_MODE_IDLE: > @@ -244,24 +244,24 @@ void jz4740_clock_set_wait_mode(enum jz4740_wait_mode > mode) break; > } > > - writel(lcr, cgu->base + CGU_REG_LCR); > + cgu_writel(cgu, lcr, CGU_REG_LCR); > } > > void jz4740_clock_udc_disable_auto_suspend(void) > { > - uint32_t clkgr = readl(cgu->base + CGU_REG_CLKGR); > + uint32_t clkgr = cgu_readl(cgu, CGU_REG_CLKGR); > > clkgr &= ~CLKGR_UDC; > - writel(clkgr, cgu->base + CGU_REG_CLKGR); > + cgu_writel(cgu, clkgr, CGU_REG_CLKGR); > } > EXPORT_SYMBOL_GPL(jz4740_clock_udc_disable_auto_suspend); > > void jz4740_clock_udc_enable_auto_suspend(void) > { > - uint32_t clkgr = readl(cgu->base + CGU_REG_CLKGR); > + uint32_t clkgr = cgu_readl(cgu, CGU_REG_CLKGR); > > clkgr |= CLKGR_UDC; > - writel(clkgr, cgu->base + CGU_REG_CLKGR); > + cgu_writel(cgu, clkgr, CGU_REG_CLKGR); > } > EXPORT_SYMBOL_GPL(jz4740_clock_udc_enable_auto_suspend); > > @@ -273,31 +273,31 @@ void jz4740_clock_suspend(void) > { > uint32_t clkgr, cppcr; > > - clkgr = readl(cgu->base + CGU_REG_CLKGR); > + clkgr = cgu_readl(cgu, CGU_REG_CLKGR); > clkgr |= JZ_CLOCK_GATE_TCU | JZ_CLOCK_GATE_DMAC | JZ_CLOCK_GATE_UART0; > - writel(clkgr, cgu->base + CGU_REG_CLKGR); > + cgu_writel(cgu, clkgr, CGU_REG_CLKGR); > > - cppcr = readl(cgu->base + CGU_REG_CPPCR); > + cppcr = cgu_readl(cgu, CGU_REG_CPPCR); > cppcr &= ~BIT(jz4740_cgu_clocks[JZ4740_CLK_PLL].pll.enable_bit); > - writel(cppcr, cgu->base + CGU_REG_CPPCR); > + cgu_writel(cgu, cppcr, CGU_REG_CPPCR); > } > > void jz4740_clock_resume(void) > { > uint32_t clkgr, cppcr, stable; > > - cppcr = readl(cgu->base + CGU_REG_CPPCR); > + cppcr = cgu_readl(cgu, CGU_REG_CPPCR); > cppcr |= BIT(jz4740_cgu_clocks[JZ4740_CLK_PLL].pll.enable_bit); > - writel(cppcr, cgu->base + CGU_REG_CPPCR); > + cgu_writel(cgu, cppcr, CGU_REG_CPPCR); > > stable = BIT(jz4740_cgu_clocks[JZ4740_CLK_PLL].pll.stable_bit); > do { > - cppcr = readl(cgu->base + CGU_REG_CPPCR); > + cppcr = cgu_readl(cgu, CGU_REG_CPPCR); > } while (!(cppcr & stable)); > > - clkgr = readl(cgu->base + CGU_REG_CLKGR); > + clkgr = cgu_readl(cgu, CGU_REG_CLKGR); > clkgr &= ~JZ_CLOCK_GATE_TCU; > clkgr &= ~JZ_CLOCK_GATE_DMAC; > clkgr &= ~JZ_CLOCK_GATE_UART0; > - writel(clkgr, cgu->base + CGU_REG_CLKGR); > + cgu_writel(cgu, clkgr, CGU_REG_CLKGR); > } > diff --git a/drivers/clk/ingenic/jz4780-cgu.c > b/drivers/clk/ingenic/jz4780-cgu.c index b35d6d9..8f83433 100644 > --- a/drivers/clk/ingenic/jz4780-cgu.c > +++ b/drivers/clk/ingenic/jz4780-cgu.c > @@ -113,11 +113,11 @@ static int jz4780_otg_phy_set_parent(struct clk_hw > *hw, u8 idx) > > spin_lock_irqsave(&cgu->lock, flags); > > - usbpcr1 = readl(cgu->base + CGU_REG_USBPCR1); > + usbpcr1 = cgu_readl(cgu, CGU_REG_USBPCR1); > usbpcr1 &= ~USBPCR1_REFCLKSEL_MASK; > /* we only use CLKCORE */ > usbpcr1 |= USBPCR1_REFCLKSEL_CORE; > - writel(usbpcr1, cgu->base + CGU_REG_USBPCR1); > + cgu_writel(cgu, usbpcr1, CGU_REG_USBPCR1); > > spin_unlock_irqrestore(&cgu->lock, flags); > return 0; > @@ -129,7 +129,7 @@ static unsigned long jz4780_otg_phy_recalc_rate(struct > clk_hw *hw, u32 usbpcr1; > unsigned refclk_div; > > - usbpcr1 = readl(cgu->base + CGU_REG_USBPCR1); > + usbpcr1 = cgu_readl(cgu, CGU_REG_USBPCR1); > refclk_div = usbpcr1 & USBPCR1_REFCLKDIV_MASK; > > switch (refclk_div) { > @@ -194,10 +194,10 @@ static int jz4780_otg_phy_set_rate(struct clk_hw *hw, > unsigned long req_rate, > > spin_lock_irqsave(&cgu->lock, flags); > > - usbpcr1 = readl(cgu->base + CGU_REG_USBPCR1); > + usbpcr1 = cgu_readl(cgu, CGU_REG_USBPCR1); > usbpcr1 &= ~USBPCR1_REFCLKDIV_MASK; > usbpcr1 |= div_bits; > - writel(usbpcr1, cgu->base + CGU_REG_USBPCR1); > + cgu_writel(cgu, usbpcr1, CGU_REG_USBPCR1); > > spin_unlock_irqrestore(&cgu->lock, flags); > return 0;
Hi Paul, Thanks for your review. On 19 August 2017 at 02:18, Paul Burton <paul.burton@imgtec.com> wrote: > Hi PrasannaKumar, > > On Thursday, 17 August 2017 11:25:16 PDT PrasannaKumar Muralidharan wrote: >> Ingenic PRNG registers are a part of the same hardware block as clock >> and power stuff. It is handled by CGU driver. Ingenic M200 SoC has power >> related registers that are after the PRNG registers. So instead of >> shortening the register range use syscon interface to expose regmap. >> This regmap is used by the PRNG driver. >> >> Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com> >> --- >> arch/mips/boot/dts/ingenic/jz4740.dtsi | 14 +++++++---- >> arch/mips/boot/dts/ingenic/jz4780.dtsi | 14 +++++++---- >> drivers/clk/ingenic/cgu.c | 46 >> +++++++++++++++++++++------------- drivers/clk/ingenic/cgu.h | >> 9 +++++-- >> drivers/clk/ingenic/jz4740-cgu.c | 30 +++++++++++----------- >> drivers/clk/ingenic/jz4780-cgu.c | 10 ++++---- >> 6 files changed, 73 insertions(+), 50 deletions(-) >> >> diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi >> b/arch/mips/boot/dts/ingenic/jz4740.dtsi index 2ca7ce7..ada5e67 100644 >> --- a/arch/mips/boot/dts/ingenic/jz4740.dtsi >> +++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi >> @@ -34,14 +34,18 @@ >> clock-frequency = <32768>; >> }; >> >> - cgu: jz4740-cgu@10000000 { >> - compatible = "ingenic,jz4740-cgu"; >> + cgu_registers { >> + compatible = "simple-mfd", "syscon"; >> reg = <0x10000000 0x100>; >> >> - clocks = <&ext>, <&rtc>; >> - clock-names = "ext", "rtc"; >> + cgu: jz4780-cgu@10000000 { >> + compatible = "ingenic,jz4740-cgu"; >> >> - #clock-cells = <1>; >> + clocks = <&ext>, <&rtc>; >> + clock-names = "ext", "rtc"; >> + >> + #clock-cells = <1>; >> + }; >> }; >> >> rtc_dev: rtc@10003000 { >> diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi >> b/arch/mips/boot/dts/ingenic/jz4780.dtsi index 4853ef6..1301694 100644 >> --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi >> +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi >> @@ -34,14 +34,18 @@ >> clock-frequency = <32768>; >> }; >> >> - cgu: jz4780-cgu@10000000 { >> - compatible = "ingenic,jz4780-cgu"; >> + cgu_registers { >> + compatible = "simple-mfd", "syscon"; >> reg = <0x10000000 0x100>; >> >> - clocks = <&ext>, <&rtc>; >> - clock-names = "ext", "rtc"; >> + cgu: jz4780-cgu@10000000 { >> + compatible = "ingenic,jz4780-cgu"; >> >> - #clock-cells = <1>; >> + clocks = <&ext>, <&rtc>; >> + clock-names = "ext", "rtc"; >> + >> + #clock-cells = <1>; >> + }; >> }; >> >> pinctrl: pin-controller@10010000 { >> diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c >> index e8248f9..2f12c7c 100644 >> --- a/drivers/clk/ingenic/cgu.c >> +++ b/drivers/clk/ingenic/cgu.c >> @@ -29,6 +29,18 @@ >> >> #define MHZ (1000 * 1000) >> >> +unsigned int cgu_readl(struct ingenic_cgu *cgu, unsigned int reg) >> +{ >> + unsigned int val = 0; >> + regmap_read(cgu->regmap, reg, &val); >> + return val; >> +} >> + >> +int cgu_writel(struct ingenic_cgu *cgu, unsigned int val, unsigned int reg) >> +{ >> + return regmap_write(cgu->regmap, reg, val); >> +} > > This is going to introduce a lot of overhead to the CGU driver - each > regmap_read() or regmap_write() call will not only add overhead from the > indirection but also acquire a lock which we really don't need. > Indeed. > Could you instead perhaps: > > - Just add "syscon" as a second compatible string to the CGU node in the > device tree, but otherwise leave it as-is without the extra cgu_registers > node. > > - Have your RNG device node as a child of the CGU node, which should let it > pick up the regmap via syscon_node_to_regmap() as it already does. > > - Leave the CGU driver as-is, so it can continue accessing memory directly > rather than via regmap. > As per my understanding, CGU driver and syscon will map the same register set. Is that fine? Thanks, PrasannaKumar
Hi PrasannaKumar, On Sunday, 20 August 2017 09:12:12 PDT PrasannaKumar Muralidharan wrote: > > Could you instead perhaps: > > - Just add "syscon" as a second compatible string to the CGU node in the > > > > device tree, but otherwise leave it as-is without the extra > > cgu_registers > > node. > > > > - Have your RNG device node as a child of the CGU node, which should let > > it > > > > pick up the regmap via syscon_node_to_regmap() as it already does. > > > > - Leave the CGU driver as-is, so it can continue accessing memory > > directly > > > > rather than via regmap. > > As per my understanding, CGU driver and syscon will map the same > register set. Is that fine? That should work fine. It's only risky if you have 2 drivers using the same registers in ways that might race with one another or clobber one another's data. In this case each of the 2 drivers is using a different subset of registers that just happen to be in the same address range, so there shouldn't be an issue. Thanks, Paul
diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi b/arch/mips/boot/dts/ingenic/jz4740.dtsi index 2ca7ce7..ada5e67 100644 --- a/arch/mips/boot/dts/ingenic/jz4740.dtsi +++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi @@ -34,14 +34,18 @@ clock-frequency = <32768>; }; - cgu: jz4740-cgu@10000000 { - compatible = "ingenic,jz4740-cgu"; + cgu_registers { + compatible = "simple-mfd", "syscon"; reg = <0x10000000 0x100>; - clocks = <&ext>, <&rtc>; - clock-names = "ext", "rtc"; + cgu: jz4780-cgu@10000000 { + compatible = "ingenic,jz4740-cgu"; - #clock-cells = <1>; + clocks = <&ext>, <&rtc>; + clock-names = "ext", "rtc"; + + #clock-cells = <1>; + }; }; rtc_dev: rtc@10003000 { diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi index 4853ef6..1301694 100644 --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi @@ -34,14 +34,18 @@ clock-frequency = <32768>; }; - cgu: jz4780-cgu@10000000 { - compatible = "ingenic,jz4780-cgu"; + cgu_registers { + compatible = "simple-mfd", "syscon"; reg = <0x10000000 0x100>; - clocks = <&ext>, <&rtc>; - clock-names = "ext", "rtc"; + cgu: jz4780-cgu@10000000 { + compatible = "ingenic,jz4780-cgu"; - #clock-cells = <1>; + clocks = <&ext>, <&rtc>; + clock-names = "ext", "rtc"; + + #clock-cells = <1>; + }; }; pinctrl: pin-controller@10010000 { diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c index e8248f9..2f12c7c 100644 --- a/drivers/clk/ingenic/cgu.c +++ b/drivers/clk/ingenic/cgu.c @@ -29,6 +29,18 @@ #define MHZ (1000 * 1000) +unsigned int cgu_readl(struct ingenic_cgu *cgu, unsigned int reg) +{ + unsigned int val = 0; + regmap_read(cgu->regmap, reg, &val); + return val; +} + +int cgu_writel(struct ingenic_cgu *cgu, unsigned int val, unsigned int reg) +{ + return regmap_write(cgu->regmap, reg, val); +} + /** * ingenic_cgu_gate_get() - get the value of clock gate register bit * @cgu: reference to the CGU whose registers should be read @@ -43,7 +55,7 @@ static inline bool ingenic_cgu_gate_get(struct ingenic_cgu *cgu, const struct ingenic_cgu_gate_info *info) { - return readl(cgu->base + info->reg) & BIT(info->bit); + return cgu_readl(cgu, info->reg) & BIT(info->bit); } /** @@ -60,14 +72,14 @@ static inline void ingenic_cgu_gate_set(struct ingenic_cgu *cgu, const struct ingenic_cgu_gate_info *info, bool val) { - u32 clkgr = readl(cgu->base + info->reg); + u32 clkgr = cgu_readl(cgu, info->reg); if (val) clkgr |= BIT(info->bit); else clkgr &= ~BIT(info->bit); - writel(clkgr, cgu->base + info->reg); + cgu_writel(cgu, clkgr, info->reg); } /* @@ -91,7 +103,7 @@ ingenic_pll_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) pll_info = &clk_info->pll; spin_lock_irqsave(&cgu->lock, flags); - ctl = readl(cgu->base + pll_info->reg); + ctl = cgu_readl(cgu, pll_info->reg); spin_unlock_irqrestore(&cgu->lock, flags); m = (ctl >> pll_info->m_shift) & GENMASK(pll_info->m_bits - 1, 0); @@ -190,7 +202,7 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned long req_rate, clk_info->name, req_rate, rate); spin_lock_irqsave(&cgu->lock, flags); - ctl = readl(cgu->base + pll_info->reg); + ctl = cgu_readl(cgu, pll_info->reg); ctl &= ~(GENMASK(pll_info->m_bits - 1, 0) << pll_info->m_shift); ctl |= (m - pll_info->m_offset) << pll_info->m_shift; @@ -204,11 +216,11 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned long req_rate, ctl &= ~BIT(pll_info->bypass_bit); ctl |= BIT(pll_info->enable_bit); - writel(ctl, cgu->base + pll_info->reg); + cgu_writel(cgu, ctl, pll_info->reg); /* wait for the PLL to stabilise */ for (i = 0; i < timeout; i++) { - ctl = readl(cgu->base + pll_info->reg); + ctl = cgu_readl(cgu, pll_info->reg); if (ctl & BIT(pll_info->stable_bit)) break; mdelay(1); @@ -243,7 +255,7 @@ static u8 ingenic_clk_get_parent(struct clk_hw *hw) clk_info = &cgu->clock_info[ingenic_clk->idx]; if (clk_info->type & CGU_CLK_MUX) { - reg = readl(cgu->base + clk_info->mux.reg); + reg = cgu_readl(cgu, clk_info->mux.reg); hw_idx = (reg >> clk_info->mux.shift) & GENMASK(clk_info->mux.bits - 1, 0); @@ -297,10 +309,10 @@ static int ingenic_clk_set_parent(struct clk_hw *hw, u8 idx) spin_lock_irqsave(&cgu->lock, flags); /* write the register */ - reg = readl(cgu->base + clk_info->mux.reg); + reg = cgu_readl(cgu, clk_info->mux.reg); reg &= ~mask; reg |= hw_idx << clk_info->mux.shift; - writel(reg, cgu->base + clk_info->mux.reg); + cgu_writel(cgu, reg, clk_info->mux.reg); spin_unlock_irqrestore(&cgu->lock, flags); return 0; @@ -321,7 +333,7 @@ ingenic_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) clk_info = &cgu->clock_info[ingenic_clk->idx]; if (clk_info->type & CGU_CLK_DIV) { - div_reg = readl(cgu->base + clk_info->div.reg); + div_reg = cgu_readl(cgu, clk_info->div.reg); div = (div_reg >> clk_info->div.shift) & GENMASK(clk_info->div.bits - 1, 0); div += 1; @@ -399,7 +411,7 @@ ingenic_clk_set_rate(struct clk_hw *hw, unsigned long req_rate, return -EINVAL; spin_lock_irqsave(&cgu->lock, flags); - reg = readl(cgu->base + clk_info->div.reg); + reg = cgu_readl(cgu, clk_info->div.reg); /* update the divide */ mask = GENMASK(clk_info->div.bits - 1, 0); @@ -415,12 +427,12 @@ ingenic_clk_set_rate(struct clk_hw *hw, unsigned long req_rate, reg |= BIT(clk_info->div.ce_bit); /* update the hardware */ - writel(reg, cgu->base + clk_info->div.reg); + cgu_writel(cgu, reg, clk_info->div.reg); /* wait for the change to take effect */ if (clk_info->div.busy_bit != -1) { for (i = 0; i < timeout; i++) { - reg = readl(cgu->base + clk_info->div.reg); + reg = cgu_readl(cgu, clk_info->div.reg); if (!(reg & BIT(clk_info->div.busy_bit))) break; mdelay(1); @@ -661,11 +673,9 @@ ingenic_cgu_new(const struct ingenic_cgu_clk_info *clock_info, if (!cgu) goto err_out; - cgu->base = of_iomap(np, 0); - if (!cgu->base) { - pr_err("%s: failed to map CGU registers\n", __func__); + cgu->regmap = syscon_node_to_regmap(np->parent); + if (cgu->regmap == NULL) goto err_out_free; - } cgu->np = np; cgu->clock_info = clock_info; diff --git a/drivers/clk/ingenic/cgu.h b/drivers/clk/ingenic/cgu.h index 09700b2..86fd5b0 100644 --- a/drivers/clk/ingenic/cgu.h +++ b/drivers/clk/ingenic/cgu.h @@ -19,7 +19,9 @@ #define __DRIVERS_CLK_INGENIC_CGU_H__ #include <linux/bitops.h> +#include <linux/mfd/syscon.h> #include <linux/of.h> +#include <linux/regmap.h> #include <linux/spinlock.h> /** @@ -171,14 +173,14 @@ struct ingenic_cgu_clk_info { /** * struct ingenic_cgu - data about the CGU * @np: the device tree node that caused the CGU to be probed - * @base: the ioremap'ed base address of the CGU registers + * @regmap: the regmap object used to access the CGU registers * @clock_info: an array containing information about implemented clocks * @clocks: used to provide clocks to DT, allows lookup of struct clk* * @lock: lock to be held whilst manipulating CGU registers */ struct ingenic_cgu { struct device_node *np; - void __iomem *base; + struct regmap *regmap; const struct ingenic_cgu_clk_info *clock_info; struct clk_onecell_data clocks; @@ -186,6 +188,9 @@ struct ingenic_cgu { spinlock_t lock; }; +unsigned int cgu_readl(struct ingenic_cgu *cgu, unsigned int reg); +int cgu_writel(struct ingenic_cgu *cgu, unsigned int val, unsigned int reg); + /** * struct ingenic_clk - private data for a clock * @hw: see Documentation/clk.txt diff --git a/drivers/clk/ingenic/jz4740-cgu.c b/drivers/clk/ingenic/jz4740-cgu.c index 510fe7e..3003afb 100644 --- a/drivers/clk/ingenic/jz4740-cgu.c +++ b/drivers/clk/ingenic/jz4740-cgu.c @@ -232,7 +232,7 @@ CLK_OF_DECLARE(jz4740_cgu, "ingenic,jz4740-cgu", jz4740_cgu_init); void jz4740_clock_set_wait_mode(enum jz4740_wait_mode mode) { - uint32_t lcr = readl(cgu->base + CGU_REG_LCR); + uint32_t lcr = cgu_readl(cgu, CGU_REG_LCR); switch (mode) { case JZ4740_WAIT_MODE_IDLE: @@ -244,24 +244,24 @@ void jz4740_clock_set_wait_mode(enum jz4740_wait_mode mode) break; } - writel(lcr, cgu->base + CGU_REG_LCR); + cgu_writel(cgu, lcr, CGU_REG_LCR); } void jz4740_clock_udc_disable_auto_suspend(void) { - uint32_t clkgr = readl(cgu->base + CGU_REG_CLKGR); + uint32_t clkgr = cgu_readl(cgu, CGU_REG_CLKGR); clkgr &= ~CLKGR_UDC; - writel(clkgr, cgu->base + CGU_REG_CLKGR); + cgu_writel(cgu, clkgr, CGU_REG_CLKGR); } EXPORT_SYMBOL_GPL(jz4740_clock_udc_disable_auto_suspend); void jz4740_clock_udc_enable_auto_suspend(void) { - uint32_t clkgr = readl(cgu->base + CGU_REG_CLKGR); + uint32_t clkgr = cgu_readl(cgu, CGU_REG_CLKGR); clkgr |= CLKGR_UDC; - writel(clkgr, cgu->base + CGU_REG_CLKGR); + cgu_writel(cgu, clkgr, CGU_REG_CLKGR); } EXPORT_SYMBOL_GPL(jz4740_clock_udc_enable_auto_suspend); @@ -273,31 +273,31 @@ void jz4740_clock_suspend(void) { uint32_t clkgr, cppcr; - clkgr = readl(cgu->base + CGU_REG_CLKGR); + clkgr = cgu_readl(cgu, CGU_REG_CLKGR); clkgr |= JZ_CLOCK_GATE_TCU | JZ_CLOCK_GATE_DMAC | JZ_CLOCK_GATE_UART0; - writel(clkgr, cgu->base + CGU_REG_CLKGR); + cgu_writel(cgu, clkgr, CGU_REG_CLKGR); - cppcr = readl(cgu->base + CGU_REG_CPPCR); + cppcr = cgu_readl(cgu, CGU_REG_CPPCR); cppcr &= ~BIT(jz4740_cgu_clocks[JZ4740_CLK_PLL].pll.enable_bit); - writel(cppcr, cgu->base + CGU_REG_CPPCR); + cgu_writel(cgu, cppcr, CGU_REG_CPPCR); } void jz4740_clock_resume(void) { uint32_t clkgr, cppcr, stable; - cppcr = readl(cgu->base + CGU_REG_CPPCR); + cppcr = cgu_readl(cgu, CGU_REG_CPPCR); cppcr |= BIT(jz4740_cgu_clocks[JZ4740_CLK_PLL].pll.enable_bit); - writel(cppcr, cgu->base + CGU_REG_CPPCR); + cgu_writel(cgu, cppcr, CGU_REG_CPPCR); stable = BIT(jz4740_cgu_clocks[JZ4740_CLK_PLL].pll.stable_bit); do { - cppcr = readl(cgu->base + CGU_REG_CPPCR); + cppcr = cgu_readl(cgu, CGU_REG_CPPCR); } while (!(cppcr & stable)); - clkgr = readl(cgu->base + CGU_REG_CLKGR); + clkgr = cgu_readl(cgu, CGU_REG_CLKGR); clkgr &= ~JZ_CLOCK_GATE_TCU; clkgr &= ~JZ_CLOCK_GATE_DMAC; clkgr &= ~JZ_CLOCK_GATE_UART0; - writel(clkgr, cgu->base + CGU_REG_CLKGR); + cgu_writel(cgu, clkgr, CGU_REG_CLKGR); } diff --git a/drivers/clk/ingenic/jz4780-cgu.c b/drivers/clk/ingenic/jz4780-cgu.c index b35d6d9..8f83433 100644 --- a/drivers/clk/ingenic/jz4780-cgu.c +++ b/drivers/clk/ingenic/jz4780-cgu.c @@ -113,11 +113,11 @@ static int jz4780_otg_phy_set_parent(struct clk_hw *hw, u8 idx) spin_lock_irqsave(&cgu->lock, flags); - usbpcr1 = readl(cgu->base + CGU_REG_USBPCR1); + usbpcr1 = cgu_readl(cgu, CGU_REG_USBPCR1); usbpcr1 &= ~USBPCR1_REFCLKSEL_MASK; /* we only use CLKCORE */ usbpcr1 |= USBPCR1_REFCLKSEL_CORE; - writel(usbpcr1, cgu->base + CGU_REG_USBPCR1); + cgu_writel(cgu, usbpcr1, CGU_REG_USBPCR1); spin_unlock_irqrestore(&cgu->lock, flags); return 0; @@ -129,7 +129,7 @@ static unsigned long jz4780_otg_phy_recalc_rate(struct clk_hw *hw, u32 usbpcr1; unsigned refclk_div; - usbpcr1 = readl(cgu->base + CGU_REG_USBPCR1); + usbpcr1 = cgu_readl(cgu, CGU_REG_USBPCR1); refclk_div = usbpcr1 & USBPCR1_REFCLKDIV_MASK; switch (refclk_div) { @@ -194,10 +194,10 @@ static int jz4780_otg_phy_set_rate(struct clk_hw *hw, unsigned long req_rate, spin_lock_irqsave(&cgu->lock, flags); - usbpcr1 = readl(cgu->base + CGU_REG_USBPCR1); + usbpcr1 = cgu_readl(cgu, CGU_REG_USBPCR1); usbpcr1 &= ~USBPCR1_REFCLKDIV_MASK; usbpcr1 |= div_bits; - writel(usbpcr1, cgu->base + CGU_REG_USBPCR1); + cgu_writel(cgu, usbpcr1, CGU_REG_USBPCR1); spin_unlock_irqrestore(&cgu->lock, flags); return 0;
Ingenic PRNG registers are a part of the same hardware block as clock and power stuff. It is handled by CGU driver. Ingenic M200 SoC has power related registers that are after the PRNG registers. So instead of shortening the register range use syscon interface to expose regmap. This regmap is used by the PRNG driver. Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com> --- arch/mips/boot/dts/ingenic/jz4740.dtsi | 14 +++++++---- arch/mips/boot/dts/ingenic/jz4780.dtsi | 14 +++++++---- drivers/clk/ingenic/cgu.c | 46 +++++++++++++++++++++------------- drivers/clk/ingenic/cgu.h | 9 +++++-- drivers/clk/ingenic/jz4740-cgu.c | 30 +++++++++++----------- drivers/clk/ingenic/jz4780-cgu.c | 10 ++++---- 6 files changed, 73 insertions(+), 50 deletions(-)