On Friday, January 27, 2017, Chris Brandt wrote:
> When there is no status bit, it is possible for the clock enable/disable
> operation to have not completed by the time the driver code resumes
> execution. This is due to the fact that write operations are sometimes
> queued and delayed internally. Doing a read ensures the write operations
> has completed.
>
> Signed-off-by: Chris Brandt <chris.brandt@xxxxxxxxxxx>
> ---
> drivers/clk/renesas/clk-mstp.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/clk/renesas/clk-mstp.c b/drivers/clk/renesas/clk-
> mstp.c
> index 3ce819c..69cfdb9 100644
> --- a/drivers/clk/renesas/clk-mstp.c
> +++ b/drivers/clk/renesas/clk-mstp.c
> @@ -91,6 +91,12 @@ static int cpg_mstp_clock_endisable(struct clk_hw *hw,
> bool enable)
> value |= bitmask;
> cpg_mstp_write(group, value, group->smstpcr);
>
> + if (!group->mstpsr) {
> + /* dummy read to ensure write has completed */
> + clk_readl(group->smstpcr);
> + barrier_data(group->smstpcr);
> + }
> +
> spin_unlock_irqrestore(&group->lock, flags);
>
> if (!enable || !group->mstpsr)
Just to show why I think this helps, here's my testing.
I first discovered this with the RSPI driver where it would stop working. The reason is that after the clock is turned back on, it would do a read/modify/write on a register. But, sometimes the register value would come back as 0x00...to it would go from Master mode to Slave mode.
So to demonstration this, I ran the following code below on boot up.
WITHOUT this patch I'm proposing, you will get the following message printed out many times (every time SPCR is read as 0x00 instead of its correct value of 0x40)
=====SPCR=00 SPSR=60 SPCMD=078B
=====SPCR=00 SPSR=60 SPCMD=078B
=====SPCR=00 SPSR=60 SPCMD=078B
=====SPCR=00 SPSR=60 SPCMD=078B
=====SPCR=00 SPSR=60 SPCMD=078B
etc...
However, WITH this patch applied, you will never get this message (meaning this patch fixes this issue).
Unfortunately, I think some peripherals like RIIC (I2C) still need more time to become fully working...but that's a different topic.
For reference, here was the test code I was running to show this effect.
/* MSTP testing */
{
struct clk *rspi4_clk;
void *rspi4_base = ioremap_nocache(0xE800E800, 0x30);
u8 spcr = 0xFF;
u8 spsr = 0xFF;
u16 spcmd = 0xFFFF;
int i;
/* set SPCR to 0x40 (reset: SPCR = 0x00) */
rspi4_clk = clk_get_sys(NULL,"spi4");
clk_prepare_enable(rspi4_clk);
udelay(1000);
*(u8 *)rspi4_base = 0x40;
udelay(1000);
clk_disable_unprepare(rspi4_clk);
msleep(100);
for (i=0;i<1000;i++)
{
/* Enable Clock */
clk_prepare_enable(rspi4_clk);
/* read registers */
spcr = *(u8 *)(rspi4_base); /* 0x40 = OK */
spsr = *(u8 *)(rspi4_base + 3); /* 0x60 = OK */
spcmd = *(u16 *)(rspi4_base + 0x10);/* 0x070D = OK */
/* value=0 is BAD data */
if (!spcr || !spsr || !spcmd)
printk("=====SPCR=%02X SPSR=%02X SPCMD=%04X\n", spcr,spsr,spcmd);
/* Disable Clock */
clk_disable_unprepare(rspi4_clk);
}
}
Chris
