linux-kernel - Re: [PATCH RFC] clk: mxs: Fix invalid 32-bit access to frac registers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <201412142019.50047.marex@denx.de>
Date:	Sun, 14 Dec 2014 20:19:49 +0100
From:	Marek Vasut <marex@...x.de>
To:	Stefan Wahren <stefan.wahren@...e.com>
Cc:	linux-kernel@...r.kernel.org, festevam@...il.com,
	shawn.guo@...aro.org, mturquette@...aro.org,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH RFC] clk: mxs: Fix invalid 32-bit access to frac registers

On Sunday, December 14, 2014 at 06:16:17 PM, Stefan Wahren wrote:
> Hi Marek,
> 
> > Marek Vasut <marex@...x.de> hat am 14. Dezember 2014 um 17:12 geschrieben:
> > > static void __iomem *digctrl;
> > > #define DIGCTRL digctrl
> > > @@ -118,11 +119,12 @@ static void __init clk_misc_init(void)
> > > /*
> > > * 480 MHz seems too high to be ssp clock source directly,
> > > * so set frac0 to get a 288 MHz ref_io0 and ref_io1.
> > > + * According to reference manual we must access frac0 bytewise.
> > > */
> > > - val = readl_relaxed(FRAC0);
> > > - val &= ~((0x3f << BP_FRAC0_IO0FRAC) | (0x3f << BP_FRAC0_IO1FRAC));
> > > - val |= (30 << BP_FRAC0_IO0FRAC) | (30 << BP_FRAC0_IO1FRAC);
> > > - writel_relaxed(val, FRAC0);
> > > + writeb_relaxed(0x3f, FRAC0 + FRAC0_IO0 + CLR);
> > > + writeb_relaxed(30, FRAC0 + FRAC0_IO0 + SET);
> > > + writeb_relaxed(0x3f, FRAC0 + FRAC0_IO1 + CLR);
> > > + writeb_relaxed(30, FRAC0 + FRAC0_IO1 + SET);
> > 
> > This used to be a R-M-W sequence, but now it's changed to multiple
> > writes. This
> > changes the behavior and seeing you use the CLR register, I am worried
> > this might be prone to clock glitches. What do you think please ?
> 
> you are right. I adapt the imx23 init to the imx28 to make code simple. But
> it would be better to avoid glitches.
> I hope it's okay for this bugfix to introduce a R-M-W sequence for the
> imx23 init. So it's consequent.

It should be OK. Make sure to document it in the commit message.

> > [...]
> > 
> > Also, it might be a good idea to zap the 0x3f mask and use HEX and DEC
> > numbers consistently, but this is an idea for another patch.
> 
> Yes.
> 
> Btw i hope this patch also fixes a SPI communication issue with our
> hardware which forces us to bypass ref_io1 for ssp2.
> But i will have access to that hardware tomorrow.

Which issue would that be please ? What are the symptoms ?

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/