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: > > > +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. _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev