Re: powerpc: Add an inline function to update HID0

Hi Segher,

Thanks for the suggestions. I will rename the function to
update_power8_hid0() and use asm volatile.


On Tue, Aug 04, 2015 at 09:30:57PM -0500, Segher Boessenkool wrote:

On Tue, Aug 04, 2015 at 08:08:58PM +1000, Michael Ellerman wrote:

quoted

quoted

+static inline void update_hid0(unsigned long hid0)
+{
+	/*
+	 *  The HID0 update should at the very least be preceded by a
+	 *  a SYNC instruction followed by an ISYNC instruction
+	 */
+	mb();
+	mtspr(SPRN_HID0, hid0);
+	isync();

That's going to turn into three separate inline asm blocks, which is maybe a
bit unfortunate. Have you checked the generated code is what we want, ie. just
sync, mtspr, isync ?

The "mb()" is not such a great name anyway: you don't want a memory
barrier, you want an actual sync instruction ("sync 0", "hwsync",
whatever the currently preferred spelling is).

The function name should also say this is for POWER8 (the required
sequences are different for some other processors; and some others
might not even _have_ a HID0, or not at 1008).  power8_write_hid0
or such?

For writing it as one asm, why not just

  asm volatile("sync ; mtspr %0,%1 ; isync" : : "i"(SPRN_HID0), "r"(hid0));

instead of the stringify stuff?


Segher

--
Thanks and Regards
gautham.