Date: Tue, 4 Aug 2015 21:30:57 -0500
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Michael Ellerman <mpe@ellerman.id.au>
Cc: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>,
        Paul Mackerras <paulus@samba.org>, linux-kernel@vger.kernel.org,
        linuxppc-dev@lists.ozlabs.org, mikey@neuling.org
Subject: Re: powerpc: Add an inline function to update HID0
Message-ID: <20150805023057.GT11083@gate.crashing.org>
References: <1438677058-12883-1-git-send-email-ego@linux.vnet.ibm.com> <20150804100858.1F272140306@ozlabs.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20150804100858.1F272140306@ozlabs.org>
User-Agent: Mutt/1.4.2.3i
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1318
Lines: 37

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