From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relmlor2.renesas.com ([210.160.252.172]:38055 "EHLO relmlie1.idc.renesas.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1756829AbcLOX72 (ORCPT ); Thu, 15 Dec 2016 18:59:28 -0500 Message-ID: <87fulou4xf.wl-kuninori.morimoto.gx@renesas.com> From: Kuninori Morimoto To: Chris Brandt CC: Geert Uytterhoeven , Michael Turquette , Stephen Boyd , Simon Horman , , Subject: Re: [PATCH v2] clk: renesas: mstp: Support 8-bit registers for r7s72100 In-Reply-To: <20161215170027.28411-1-chris.brandt@renesas.com> References: <20161215170027.28411-1-chris.brandt@renesas.com> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset="US-ASCII" Date: Thu, 15 Dec 2016 23:53:16 +0000 Sender: linux-renesas-soc-owner@vger.kernel.org List-ID: Hi Chris > The RZ/A1 is different than the other Renesas SOCs because the MSTP > registers are 8-bit instead of 32-bit and if you try writing values as > 32-bit nothing happens...meaning this driver never worked for r7s72100. > > Fixes: b6face404f38 ("ARM: shmobile: r7s72100: add essential clock nodes to dtsi") > Signed-off-by: Chris Brandt > --- Thanks. I looks nice for me Acked-by: Kuninori Morimoto > v2: > * move r7s72100 detection inside cpg_mstp_clocks_init() > * change width_8bit flag from global to inside group struct > --- > drivers/clk/renesas/clk-mstp.c | 27 ++++++++++++++++++++++----- > 1 file changed, 22 insertions(+), 5 deletions(-) > > diff --git a/drivers/clk/renesas/clk-mstp.c b/drivers/clk/renesas/clk-mstp.c > index 9375777..b533f99 100644 > --- a/drivers/clk/renesas/clk-mstp.c > +++ b/drivers/clk/renesas/clk-mstp.c > @@ -37,12 +37,14 @@ > * @smstpcr: module stop control register > * @mstpsr: module stop status register (optional) > * @lock: protects writes to SMSTPCR > + * @width_8bit: registers are 8-bit, not 32-bit > */ > struct mstp_clock_group { > struct clk_onecell_data data; > void __iomem *smstpcr; > void __iomem *mstpsr; > spinlock_t lock; > + bool width_8bit; > }; > > /** > @@ -59,6 +61,18 @@ struct mstp_clock { > > #define to_mstp_clock(_hw) container_of(_hw, struct mstp_clock, hw) > > +static inline u32 cpg_mstp_read(struct mstp_clock_group *group, > + u32 __iomem *reg) > +{ > + return group->width_8bit ? readb(reg) : clk_readl(reg); > +} > + > +static inline void cpg_mstp_write(struct mstp_clock_group *group, u32 val, > + u32 __iomem *reg) > +{ > + group->width_8bit ? writeb(val, reg) : clk_writel(val, reg); > +} > + > static int cpg_mstp_clock_endisable(struct clk_hw *hw, bool enable) > { > struct mstp_clock *clock = to_mstp_clock(hw); > @@ -70,12 +84,12 @@ static int cpg_mstp_clock_endisable(struct clk_hw *hw, bool enable) > > spin_lock_irqsave(&group->lock, flags); > > - value = clk_readl(group->smstpcr); > + value = cpg_mstp_read(group, group->smstpcr); > if (enable) > value &= ~bitmask; > else > value |= bitmask; > - clk_writel(value, group->smstpcr); > + cpg_mstp_write(group, value, group->smstpcr); > > spin_unlock_irqrestore(&group->lock, flags); > > @@ -83,7 +97,7 @@ static int cpg_mstp_clock_endisable(struct clk_hw *hw, bool enable) > return 0; > > for (i = 1000; i > 0; --i) { > - if (!(clk_readl(group->mstpsr) & bitmask)) > + if (!(cpg_mstp_read(group, group->mstpsr) & bitmask)) > break; > cpu_relax(); > } > @@ -114,9 +128,9 @@ static int cpg_mstp_clock_is_enabled(struct clk_hw *hw) > u32 value; > > if (group->mstpsr) > - value = clk_readl(group->mstpsr); > + value = cpg_mstp_read(group, group->mstpsr); > else > - value = clk_readl(group->smstpcr); > + value = cpg_mstp_read(group, group->smstpcr); > > return !(value & BIT(clock->bit_index)); > } > @@ -188,6 +202,9 @@ static void __init cpg_mstp_clocks_init(struct device_node *np) > return; > } > > + if (of_device_is_compatible(np, "renesas,r7s72100-mstp-clocks")) > + group->width_8bit = true; > + > for (i = 0; i < MSTP_MAX_CLOCKS; ++i) > clks[i] = ERR_PTR(-ENOENT); > > -- > 2.10.1 > >