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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
