LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] crypto:nx - fix nx-aes-gcm verification
From: jmlatten @ 2013-08-14 22:17 UTC (permalink / raw)
  To: herbert; +Cc: linuxppc-dev, linux-crypto

This patch fixes a bug in the nx-aes-gcm implementation.
Corrected the code so that the authtag is always verified after
decrypting and not just when there is associated data included.
Also, corrected the code to retrieve the input authtag from src
instead of dst. 

Reviewed-by: Fionnuala Gunter <fin@linux.vnet.ibm.com>
Reviewed-by: Marcelo Cerri <mhcerri@linux.vnet.ibm.com>
Signed-off-by: Joy Latten <jmlatten@linux.vnet.ibm.com>

diff --git a/drivers/crypto/nx/nx-aes-gcm.c b/drivers/crypto/nx/nx-aes-gcm.c
index 6cca6c3..eb851bb 100644
--- a/drivers/crypto/nx/nx-aes-gcm.c
+++ b/drivers/crypto/nx/nx-aes-gcm.c
@@ -243,11 +243,11 @@ static int gcm_aes_nx_crypt(struct aead_request *req, int enc)
 				 req->dst, nbytes,
 				 crypto_aead_authsize(crypto_aead_reqtfm(req)),
 				 SCATTERWALK_TO_SG);
-	} else if (req->assoclen) {
+	} else {
 		u8 *itag = nx_ctx->priv.gcm.iauth_tag;
 		u8 *otag = csbcpb->cpb.aes_gcm.out_pat_or_mac;
 
-		scatterwalk_map_and_copy(itag, req->dst, nbytes,
+		scatterwalk_map_and_copy(itag, req->src, nbytes,
 				 crypto_aead_authsize(crypto_aead_reqtfm(req)),
 				 SCATTERWALK_FROM_SG);
 		rc = memcmp(itag, otag,

^ permalink raw reply related

* Re: Pull request: scottwood/linux.git next
From: Benjamin Herrenschmidt @ 2013-08-14 21:01 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <1376499759.31636.15.camel@snotra.buserror.net>

On Wed, 2013-08-14 at 12:02 -0500, Scott Wood wrote:
> On Wed, 2013-08-14 at 14:18 +1000, Benjamin Herrenschmidt wrote:
> > On Thu, 2013-08-08 at 17:45 -0500, Scott Wood wrote:
> > >       powerpc/e500: Update compilation flags with core specific
> > > options
> > 
> > This breaks the build for my FSL test configs. For some reason gcc 4.7.3
> > doesn't know about -mcpu=e5500
> 
> Ugh.  I guess that's what I get for using toolchains provided internally
> rather than building them myself

:-)

I recommend you use one of tony's on kernel.org, that way you have
something that matches what other people use :-)

>  -- though it doesn't help that the GCC
> people love finding new ways to break building GCC without libc (why
> doesn't --without-headers automatically disable any component that
> requires libc headers?)

Hehe, yeah.

> , and usually respond to bug reports with "go run
> crosstool and leave us alone".  It looks like e5500 support is in 4.8.1,
> but I can't actually get it to build (libdecnumber wants to
> #include_next <stdlib.h> and can't be disabled -- arm64 toolchain built
> fine with the similar configure options).  I don't know about earlier
> versions.

In any case, there are mechanisms in Kbuild to check if the option
exist, so that should be used here. Look at the other ones.

> > Additionally, on 64-bit, that means one can no longer make a kernel that
> > does both A2 and e5500...
> 
> Other than the toolchain issue, I'm not sure how this is worse than it
> was before, when such a kernel would have had -Wa,-me500 forced.

Probably similarly bad though it did work ... but if you are touching
it, may as well do it right...

> What -mcpu value should be used in such a combined kernel?

Good question. We lack a generic booke option. What about powerpc64 ?

A default like that is fine as long as tricky asm uses the macros for
that and the *optional* -mcpu=<xxx> option is available (and you can put
it in defconfig).

It might be worth asking gcc to add something like -march=<arch version>
or something like that though.

> > I'm reverting that crap patch, please make such optimizations CONFIG_*
> > options like power5...7
> 
> Speaking of crap patches, those config options don't limit themselves to
> book3s and thus we're now getting CONFIG_GENERIC_CPU (and thus
> -mtune=power7) on e5500 builds.

Indeed, another crap patch ... I didn't spot it because it didn't break
the build :-)

Feel free to submit a fix !

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH RESEND 0/8] use platform_{get,set}_drvdata()
From: Sergei Shtylyov @ 2013-08-14 18:56 UTC (permalink / raw)
  To: Libo Chen
  Cc: Greg KH, netdev, jg1.han, LKML, Li Zefan, vbordug, linuxppc-dev,
	David Miller
In-Reply-To: <520AFB3F.2090006@huawei.com>

On 08/14/2013 07:36 AM, Libo Chen wrote:

> We can use the wrapper functions platform_{get,set}_drvdata() instead of
> dev_{get,set}_drvdata() with &pdev->dev, it is convenient for user.
>
> Also, unnecessary dev_set_drvdata() is removed, because the driver core
> clears the driver data to NULL after device_release or on probe failure.

    Saying "also" in the changelog is usually a good sign that the patch 
should be split.

WBR, Sergei

^ permalink raw reply

* Re: Pull request: scottwood/linux.git next
From: Scott Wood @ 2013-08-14 17:02 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1376453881.4255.57.camel@pasglop>

On Wed, 2013-08-14 at 14:18 +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2013-08-08 at 17:45 -0500, Scott Wood wrote:
> >       powerpc/e500: Update compilation flags with core specific
> > options
> 
> This breaks the build for my FSL test configs. For some reason gcc 4.7.3
> doesn't know about -mcpu=e5500

Ugh.  I guess that's what I get for using toolchains provided internally
rather than building them myself -- though it doesn't help that the GCC
people love finding new ways to break building GCC without libc (why
doesn't --without-headers automatically disable any component that
requires libc headers?), and usually respond to bug reports with "go run
crosstool and leave us alone".  It looks like e5500 support is in 4.8.1,
but I can't actually get it to build (libdecnumber wants to
#include_next <stdlib.h> and can't be disabled -- arm64 toolchain built
fine with the similar configure options).  I don't know about earlier
versions.

> Additionally, on 64-bit, that means one can no longer make a kernel that
> does both A2 and e5500...

Other than the toolchain issue, I'm not sure how this is worse than it
was before, when such a kernel would have had -Wa,-me500 forced.

What -mcpu value should be used in such a combined kernel?

> I'm reverting that crap patch, please make such optimizations CONFIG_*
> options like power5...7

Speaking of crap patches, those config options don't limit themselves to
book3s and thus we're now getting CONFIG_GENERIC_CPU (and thus
-mtune=power7) on e5500 builds.

-Scott

^ permalink raw reply

* Re: [PATCH] Revert "cxgb3: Check and handle the dma mapping errors"
From: Divy Le ray @ 2013-08-14 15:57 UTC (permalink / raw)
  To: Alexey Kardashevskiy
  Cc: Santosh Rastapur, Jay Fenlason, netdev@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	Linus Torvalds, David S. Miller
In-Reply-To: <1376471941-2914-1-git-send-email-aik@ozlabs.ru>

On 08/14/2013 02:19 AM, Alexey Kardashevskiy wrote:
> This reverts commit f83331bab149e29fa2c49cf102c0cd8c3f1ce9f9.
>
> As the tests PPC64 (powernv platform) show, IOMMU pages are leaking
> when transferring big amount of small packets (<=64 bytes),
> "ping -f" and waiting for 15 seconds is the simplest way to confirm the bug.
>
> Cc: Linus Torvalds<torvalds@linux-foundation.org>
> Cc: Santosh Rastapur<santosh@chelsio.com>
> Cc: Jay Fenlason<fenlason@redhat.com>
> Cc: David S. Miller<davem@davemloft.net>
> Cc: Divy Le ray<divy@chelsio.com>
> Signed-off-by: Alexey Kardashevskiy<aik@ozlabs.ru>

Acked-by: Divy Le Ray <divy@chelsio.com>

We are revisiting this patch in the light of the leak, and will repost 
once fixed.

Cheers,
Divy

> ---
>   drivers/net/ethernet/chelsio/cxgb3/sge.c | 107 +++++++------------------------
>   1 file changed, 24 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/net/ethernet/chelsio/cxgb3/sge.c b/drivers/net/ethernet/chelsio/cxgb3/sge.c
> index 687ec4a..9c89dc8 100644
> --- a/drivers/net/ethernet/chelsio/cxgb3/sge.c
> +++ b/drivers/net/ethernet/chelsio/cxgb3/sge.c
> @@ -455,11 +455,6 @@ static int alloc_pg_chunk(struct adapter *adapter, struct sge_fl *q,
>   		q->pg_chunk.offset = 0;
>   		mapping = pci_map_page(adapter->pdev, q->pg_chunk.page,
>   				       0, q->alloc_size, PCI_DMA_FROMDEVICE);
> -		if (unlikely(pci_dma_mapping_error(adapter->pdev, mapping))) {
> -			__free_pages(q->pg_chunk.page, order);
> -			q->pg_chunk.page = NULL;
> -			return -EIO;
> -		}
>   		q->pg_chunk.mapping = mapping;
>   	}
>   	sd->pg_chunk = q->pg_chunk;
> @@ -954,75 +949,40 @@ static inline unsigned int calc_tx_descs(const struct sk_buff *skb)
>   	return flits_to_desc(flits);
>   }
>   
> -
> -/*	map_skb - map a packet main body and its page fragments
> - *	@pdev: the PCI device
> - *	@skb: the packet
> - *	@addr: placeholder to save the mapped addresses
> - *
> - *	map the main body of an sk_buff and its page fragments, if any.
> - */
> -static int map_skb(struct pci_dev *pdev, const struct sk_buff *skb,
> -		   dma_addr_t *addr)
> -{
> -	const skb_frag_t *fp, *end;
> -	const struct skb_shared_info *si;
> -
> -	*addr = pci_map_single(pdev, skb->data, skb_headlen(skb),
> -			       PCI_DMA_TODEVICE);
> -	if (pci_dma_mapping_error(pdev, *addr))
> -		goto out_err;
> -
> -	si = skb_shinfo(skb);
> -	end = &si->frags[si->nr_frags];
> -
> -	for (fp = si->frags; fp < end; fp++) {
> -		*++addr = skb_frag_dma_map(&pdev->dev, fp, 0, skb_frag_size(fp),
> -					   DMA_TO_DEVICE);
> -		if (pci_dma_mapping_error(pdev, *addr))
> -			goto unwind;
> -	}
> -	return 0;
> -
> -unwind:
> -	while (fp-- > si->frags)
> -		dma_unmap_page(&pdev->dev, *--addr, skb_frag_size(fp),
> -			       DMA_TO_DEVICE);
> -
> -	pci_unmap_single(pdev, addr[-1], skb_headlen(skb), PCI_DMA_TODEVICE);
> -out_err:
> -	return -ENOMEM;
> -}
> -
>   /**
> - *	write_sgl - populate a scatter/gather list for a packet
> + *	make_sgl - populate a scatter/gather list for a packet
>    *	@skb: the packet
>    *	@sgp: the SGL to populate
>    *	@start: start address of skb main body data to include in the SGL
>    *	@len: length of skb main body data to include in the SGL
> - *	@addr: the list of the mapped addresses
> + *	@pdev: the PCI device
>    *
> - *	Copies the scatter/gather list for the buffers that make up a packet
> + *	Generates a scatter/gather list for the buffers that make up a packet
>    *	and returns the SGL size in 8-byte words.  The caller must size the SGL
>    *	appropriately.
>    */
> -static inline unsigned int write_sgl(const struct sk_buff *skb,
> +static inline unsigned int make_sgl(const struct sk_buff *skb,
>   				    struct sg_ent *sgp, unsigned char *start,
> -				    unsigned int len, const dma_addr_t *addr)
> +				    unsigned int len, struct pci_dev *pdev)
>   {
> -	unsigned int i, j = 0, k = 0, nfrags;
> +	dma_addr_t mapping;
> +	unsigned int i, j = 0, nfrags;
>   
>   	if (len) {
> +		mapping = pci_map_single(pdev, start, len, PCI_DMA_TODEVICE);
>   		sgp->len[0] = cpu_to_be32(len);
> -		sgp->addr[j++] = cpu_to_be64(addr[k++]);
> +		sgp->addr[0] = cpu_to_be64(mapping);
> +		j = 1;
>   	}
>   
>   	nfrags = skb_shinfo(skb)->nr_frags;
>   	for (i = 0; i < nfrags; i++) {
>   		const skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
>   
> +		mapping = skb_frag_dma_map(&pdev->dev, frag, 0, skb_frag_size(frag),
> +					   DMA_TO_DEVICE);
>   		sgp->len[j] = cpu_to_be32(skb_frag_size(frag));
> -		sgp->addr[j] = cpu_to_be64(addr[k++]);
> +		sgp->addr[j] = cpu_to_be64(mapping);
>   		j ^= 1;
>   		if (j == 0)
>   			++sgp;
> @@ -1178,7 +1138,7 @@ static void write_tx_pkt_wr(struct adapter *adap, struct sk_buff *skb,
>   			    const struct port_info *pi,
>   			    unsigned int pidx, unsigned int gen,
>   			    struct sge_txq *q, unsigned int ndesc,
> -			    unsigned int compl, const dma_addr_t *addr)
> +			    unsigned int compl)
>   {
>   	unsigned int flits, sgl_flits, cntrl, tso_info;
>   	struct sg_ent *sgp, sgl[MAX_SKB_FRAGS / 2 + 1];
> @@ -1236,7 +1196,7 @@ static void write_tx_pkt_wr(struct adapter *adap, struct sk_buff *skb,
>   	}
>   
>   	sgp = ndesc == 1 ? (struct sg_ent *)&d->flit[flits] : sgl;
> -	sgl_flits = write_sgl(skb, sgp, skb->data, skb_headlen(skb), addr);
> +	sgl_flits = make_sgl(skb, sgp, skb->data, skb_headlen(skb), adap->pdev);
>   
>   	write_wr_hdr_sgl(ndesc, skb, d, pidx, q, sgl, flits, sgl_flits, gen,
>   			 htonl(V_WR_OP(FW_WROPCODE_TUNNEL_TX_PKT) | compl),
> @@ -1267,7 +1227,6 @@ netdev_tx_t t3_eth_xmit(struct sk_buff *skb, struct net_device *dev)
>   	struct netdev_queue *txq;
>   	struct sge_qset *qs;
>   	struct sge_txq *q;
> -	dma_addr_t addr[MAX_SKB_FRAGS + 1];
>   
>   	/*
>   	 * The chip min packet length is 9 octets but play safe and reject
> @@ -1296,11 +1255,6 @@ netdev_tx_t t3_eth_xmit(struct sk_buff *skb, struct net_device *dev)
>   		return NETDEV_TX_BUSY;
>   	}
>   
> -	if (unlikely(map_skb(adap->pdev, skb, addr) < 0)) {
> -		dev_kfree_skb(skb);
> -		return NETDEV_TX_OK;
> -	}
> -
>   	q->in_use += ndesc;
>   	if (unlikely(credits - ndesc < q->stop_thres)) {
>   		t3_stop_tx_queue(txq, qs, q);
> @@ -1358,7 +1312,7 @@ netdev_tx_t t3_eth_xmit(struct sk_buff *skb, struct net_device *dev)
>   	if (likely(!skb_shared(skb)))
>   		skb_orphan(skb);
>   
> -	write_tx_pkt_wr(adap, skb, pi, pidx, gen, q, ndesc, compl, addr);
> +	write_tx_pkt_wr(adap, skb, pi, pidx, gen, q, ndesc, compl);
>   	check_ring_tx_db(adap, q);
>   	return NETDEV_TX_OK;
>   }
> @@ -1623,8 +1577,7 @@ static void setup_deferred_unmapping(struct sk_buff *skb, struct pci_dev *pdev,
>    */
>   static void write_ofld_wr(struct adapter *adap, struct sk_buff *skb,
>   			  struct sge_txq *q, unsigned int pidx,
> -			  unsigned int gen, unsigned int ndesc,
> -			  const dma_addr_t *addr)
> +			  unsigned int gen, unsigned int ndesc)
>   {
>   	unsigned int sgl_flits, flits;
>   	struct work_request_hdr *from;
> @@ -1645,9 +1598,9 @@ static void write_ofld_wr(struct adapter *adap, struct sk_buff *skb,
>   
>   	flits = skb_transport_offset(skb) / 8;
>   	sgp = ndesc == 1 ? (struct sg_ent *)&d->flit[flits] : sgl;
> -	sgl_flits = write_sgl(skb, sgp, skb_transport_header(skb),
> -			     skb_tail_pointer(skb) -
> -			     skb_transport_header(skb), addr);
> +	sgl_flits = make_sgl(skb, sgp, skb_transport_header(skb),
> +			     skb->tail - skb->transport_header,
> +			     adap->pdev);
>   	if (need_skb_unmap()) {
>   		setup_deferred_unmapping(skb, adap->pdev, sgp, sgl_flits);
>   		skb->destructor = deferred_unmap_destructor;
> @@ -1705,11 +1658,6 @@ again:	reclaim_completed_tx(adap, q, TX_RECLAIM_CHUNK);
>   		goto again;
>   	}
>   
> -	if (map_skb(adap->pdev, skb, (dma_addr_t *)skb->head)) {
> -		spin_unlock(&q->lock);
> -		return NET_XMIT_SUCCESS;
> -	}
> -
>   	gen = q->gen;
>   	q->in_use += ndesc;
>   	pidx = q->pidx;
> @@ -1720,7 +1668,7 @@ again:	reclaim_completed_tx(adap, q, TX_RECLAIM_CHUNK);
>   	}
>   	spin_unlock(&q->lock);
>   
> -	write_ofld_wr(adap, skb, q, pidx, gen, ndesc, (dma_addr_t *)skb->head);
> +	write_ofld_wr(adap, skb, q, pidx, gen, ndesc);
>   	check_ring_tx_db(adap, q);
>   	return NET_XMIT_SUCCESS;
>   }
> @@ -1738,7 +1686,6 @@ static void restart_offloadq(unsigned long data)
>   	struct sge_txq *q = &qs->txq[TXQ_OFLD];
>   	const struct port_info *pi = netdev_priv(qs->netdev);
>   	struct adapter *adap = pi->adapter;
> -	unsigned int written = 0;
>   
>   	spin_lock(&q->lock);
>   again:	reclaim_completed_tx(adap, q, TX_RECLAIM_CHUNK);
> @@ -1758,14 +1705,10 @@ again:	reclaim_completed_tx(adap, q, TX_RECLAIM_CHUNK);
>   			break;
>   		}
>   
> -		if (map_skb(adap->pdev, skb, (dma_addr_t *)skb->head))
> -			break;
> -
>   		gen = q->gen;
>   		q->in_use += ndesc;
>   		pidx = q->pidx;
>   		q->pidx += ndesc;
> -		written += ndesc;
>   		if (q->pidx >= q->size) {
>   			q->pidx -= q->size;
>   			q->gen ^= 1;
> @@ -1773,8 +1716,7 @@ again:	reclaim_completed_tx(adap, q, TX_RECLAIM_CHUNK);
>   		__skb_unlink(skb, &q->sendq);
>   		spin_unlock(&q->lock);
>   
> -		write_ofld_wr(adap, skb, q, pidx, gen, ndesc,
> -			     (dma_addr_t *)skb->head);
> +		write_ofld_wr(adap, skb, q, pidx, gen, ndesc);
>   		spin_lock(&q->lock);
>   	}
>   	spin_unlock(&q->lock);
> @@ -1784,9 +1726,8 @@ again:	reclaim_completed_tx(adap, q, TX_RECLAIM_CHUNK);
>   	set_bit(TXQ_LAST_PKT_DB, &q->flags);
>   #endif
>   	wmb();
> -	if (likely(written))
> -		t3_write_reg(adap, A_SG_KDOORBELL,
> -			     F_SELEGRCNTX | V_EGRCNTX(q->cntxt_id));
> +	t3_write_reg(adap, A_SG_KDOORBELL,
> +		     F_SELEGRCNTX | V_EGRCNTX(q->cntxt_id));
>   }
>   
>   /**

^ permalink raw reply

* RE: [PATCH 1/3] powerpc: Add iommu domain pointer to device archdata
From: Sethi Varun-B16395 @ 2013-08-14 16:44 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Wood Scott-B07421, iommu@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, Yoder Stuart-B08248,
	alex.williamson@redhat.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20130814161522.GP28811@8bytes.org>

Thanks Joerg.

> -----Original Message-----
> From: Joerg Roedel [mailto:joro@8bytes.org]
> Sent: Wednesday, August 14, 2013 9:45 PM
> To: Sethi Varun-B16395
> Cc: iommu@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org;
> linux-kernel@vger.kernel.org; benh@kernel.crashing.org;
> galak@kernel.crashing.org; alex.williamson@redhat.com; Yoder Stuart-
> B08248; Wood Scott-B07421
> Subject: Re: [PATCH 1/3] powerpc: Add iommu domain pointer to device
> archdata
>=20
> On Wed, Aug 14, 2013 at 09:56:11AM +0000, Sethi Varun-B16395 wrote:
> > Please find the .config file attached with this mail.
>=20
> Fantastic, thanks. The build works fine, I'll include the driver into my
> next-branch. I also have two minor clean-up patches on-top, just if you
> where wondering.
>=20
>=20
> 	Joerg
>=20
>=20

^ permalink raw reply

* Re: [PATCH 1/3] powerpc: Add iommu domain pointer to device archdata
From: Joerg Roedel @ 2013-08-14 16:15 UTC (permalink / raw)
  To: Sethi Varun-B16395
  Cc: Wood Scott-B07421, iommu@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, Yoder Stuart-B08248,
	alex.williamson@redhat.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <C5ECD7A89D1DC44195F34B25E172658D0A42CDB5@039-SN2MPN1-013.039d.mgd.msft.net>

On Wed, Aug 14, 2013 at 09:56:11AM +0000, Sethi Varun-B16395 wrote:
> Please find the .config file attached with this mail.

Fantastic, thanks. The build works fine, I'll include the driver into my
next-branch. I also have two minor clean-up patches on-top, just if you
where wondering.


	Joerg

^ permalink raw reply

* Re: [alsa-devel] [PATCH v4 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
From: Stephen Warren @ 2013-08-14 15:47 UTC (permalink / raw)
  To: Shawn Guo
  Cc: devicetree, alsa-devel, lars, linuxppc-dev, Nicolin Chen, timur,
	rob.herring, broonie, p.zabel, Fabio Estevam
In-Reply-To: <20130814081445.GC31141@S2101-09.ap.freescale.net>

On 08/14/2013 02:14 AM, Shawn Guo wrote:
> On Wed, Aug 14, 2013 at 02:34:45PM +0800, Nicolin Chen wrote:
>> On Wed, Aug 14, 2013 at 02:39:33PM +0800, Shawn Guo wrote:
>>> We only need to maintain those versions that require different
>>> programming model in the list.  For example, if S/PDIF on Vybrid
>>> is completely compatible with imx6q one and uses the exactly same
>>> programming model, we do not need to maintain a compatible string
>>> for Vybrid S/PDIF at all.  Instead, we only need to have something
>>> like below in Vybrid dts file, and S/PDIF driver will just work for it.
>>>
>>> 	compatible = "fsl,vf600-spdif", "fsl,imx6q-spdif";
>>>
>>> Shawn
>>
>> Clear. Thank you for the explain.
>>
>> Then I think I can merely remain "fsl,imx6q-spdif" here,
>> because all other cases should be completely compatible
>> with this one. They are only different in the clock source
>> names list, which's already being specified in dts file.
>>
>> Please correct me if you think this still isn't proper.

If the clock source name list is different, then it needs a different
compatible value, so that each compatible value can specify which clock
names are required.

Also, the compatible value itself should always include the exact HW
that's present (most specific HW version), as well as any other HW it's
compatible with.

> And we generally prefer to use the soc that firstly integrates the IP to
> name the compatible.  For IMX series, I think imx35 is the one, so we
> would name it "fsl,imx35-spdif".

^ permalink raw reply

* Embedded System Question
From: Tobias Stillger @ 2013-08-14 14:17 UTC (permalink / raw)
  To: linuxppc-dev

Hi,

I have to struggle with a broadcast video decoder with an unknown PPC =
Linux. I would like to compile
a new module, but I need a build environment. Here some infos about the =
system:=20

Linux version 2.6.14-gemppc-iolite

processor	: 0
cpu		: 405GPr
clock		: 399MHz
revision	: 9.81 (pvr 5091 0951)
bogomips	: 398.33
machine		: GemPPC Iolite
plb bus clock	: 133MHz
pci bus clock	: 33MHz


GNU/Linux 2.6.14 port for the GemPPC Iolite platform

Is there a chance to work with this semi closed system.=20

Thanks in advance for all input.=20


=20=

^ permalink raw reply

* Re: [GIT PULL] DT/core: cpu_ofnode updates for v3.12
From: Sudeep KarkadaNagesha @ 2013-08-14 13:27 UTC (permalink / raw)
  To: Rob Herring, Benjamin Herrenschmidt
  Cc: devicetree@vger.kernel.org, linux-pm@vger.kernel.org,
	Sudeep KarkadaNagesha, Viresh Kumar, linux-kernel@vger.kernel.org,
	Rafael J. Wysocki, Olof Johansson, linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <520B7DAE.3060501@gmail.com>

On 14/08/13 13:53, Rob Herring wrote:
> On 08/14/2013 05:01 AM, Sudeep KarkadaNagesha wrote:
>> On 13/08/13 22:07, Benjamin Herrenschmidt wrote:
>>> On Tue, 2013-08-13 at 19:29 +0100, Sudeep KarkadaNagesha wrote:
>>>> I don't understand completely the use of ibm,ppc-interrupt-server#s an=
d
>>>> its implications on generic of_get_cpu_node implementation.
>>>> I see the PPC specific definition of of_get_cpu_node uses thread id on=
ly
>>>> in 2 instances. Based on that, I have tried to move all the other
>>>> instances to use generic definition.
>>>>
>>>> Let me know if the idea is correct.
>>>
>>> No. The device-tree historically only represents cores, not HW threads,=
 so
>>> it makes sense to retrieve also the thread number corresponding to the =
CPU.
>>>
>> Ok
>>
>>> However, the mechanism to represent HW threads in the device-tree is cu=
rrently
>>> somewhat platform specific (the ibm,ppc-interrupt-server#s).
>> I see most of the callers pass NULL to thread id argument except 2
>> instances in entire tree. If that's the case why can't we move to use
>> generic of_get_cpu_node in most of those cases and have PPC specific
>> implementation for the ones using thread id.
>>
>>>
>>> So what you could do for now is:
>>>
>>>  - Have a generic version that always returns 0 as the thread, which is=
 weak
>> I would prefer to move to generic of_get_cpu_node where ever possible
>> and rename the function that takes thread id rather than making generic
>> one weak.
>>
>>>
>>>  - powerpc keeps its own implementation
>> How about only in cases where it needs thread_id.
>>
>>>
>>>  - Start a discussion on the bindings (if not already there) to define =
threads
>>> in a better way at which point the generic function can be updated.
>>>
>> I am not sure if we need to define any new bindings. Excerpts from ePAPR
>> (v1.1):
>> "3.7.1 General Properties of CPU nodes
>> The value of "reg" is a <prop-encoded-array> that defines a unique
>> CPU/thread id for the CPU/threads represented by the CPU node.
>> If a CPU supports more than one thread (i.e. multiple streams of
>> execution) the reg property is an array with 1 element per thread. The
>> #address-cells on the /cpus node specifies how many cells each element
>> of the array takes. Software can determine the number of threads by
>> dividing the size of reg by the parent node's #address-cells."
>>
>> And this is exactly in agreement to what's implemented in the generic
>> of_get_cpu_node:
>>
>> for_each_child_of_node(cpus, cpun) {
>>         if (of_node_cmp(cpun->type, "cpu"))
>>                 continue;
>>         cell =3D of_get_property(cpun, "reg", &prop_len);
>>         if (!cell) {
>>                 pr_warn("%s: missing reg property\n", cpun->full_name);
>>                 continue;
>>         }
>>         prop_len /=3D sizeof(*cell);
>>         while (prop_len) {
>>                 hwid =3D of_read_number(cell, ac);
>>                 prop_len -=3D ac;
>>                 if (arch_match_cpu_phys_id(cpu, hwid))
>>                         return cpun;
>>         }
>> }
>=20
> How about something like this:
>=20
> for_each_child_of_node(cpus, cpun) {
>         if (of_node_cmp(cpun->type, "cpu"))
>                 continue;
>=20
> =09if (arch_of_get_cpu_node(cpun, thread))
> =09=09return cpun;
>=20
>         cell =3D of_get_property(cpun, "reg", &prop_len);
>         if (!cell) {
>                 pr_warn("%s: missing reg property\n", cpun->full_name);
>                 continue;
>         }
>         prop_len /=3D sizeof(*cell);
>         while (prop_len) {
>                 hwid =3D of_read_number(cell, ac);
>                 prop_len -=3D ac;
>                 if (arch_match_cpu_phys_id(cpu, hwid))
>        =09                return cpun;
>         }
> }
>=20
> For PPC:
>=20
> arch_of_get_cpu_node()
> {
>         const u32 *intserv;
>         unsigned int plen, t;
>=20
>         /* Check for ibm,ppc-interrupt-server#s. */
>         intserv =3D of_get_property(np, "ibm,ppc-interrupt-server#s",
>                                 &plen);
>         if (!intserv)
> =09=09return false;
>=20
> =09hardid =3D get_hard_smp_processor_id(cpu);
>=20
>         plen /=3D sizeof(u32);
>         for (t =3D 0; t < plen; t++) {
>                  if (hardid =3D=3D intserv[t]) {
>                          if (thread)
>                                   *thread =3D t;
>                          return true;
>                  }
>         }
> =09return false;
> }
>=20

Sorry responded to earlier mail before seeing this. This approach looks
good, but we still need to have thread id as argument which should be fine.

But as per my understanding on how logical cpu<->hard proccessor id is
setup, the thread_id is implicit in the logical cpu id making it
unnecessary to depend on DT each time.

Regards,
Sudeep

>>
>> Yes this doesn't cover the historical "ibm,ppc-interrupt-server#s", for
>> which we can have PPC specific wrapper above the generic one i.e. get
>> the cpu node and then parse for thread id under custom property.
>>
>> Let me know your thoughts.
>>
>> Regards,
>> Sudeep
>>
>>
>>
>=20
>=20

^ permalink raw reply

* Re: [GIT PULL] DT/core: cpu_ofnode updates for v3.12
From: Sudeep KarkadaNagesha @ 2013-08-14 13:21 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: devicetree@vger.kernel.org, linux-pm@vger.kernel.org,
	Sudeep KarkadaNagesha, Viresh Kumar, linux-kernel@vger.kernel.org,
	rob.herring@calxeda.com, Rafael J. Wysocki, Olof Johansson,
	linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <1376480227.4255.66.camel@pasglop>

On 14/08/13 12:37, Benjamin Herrenschmidt wrote:
> On Wed, 2013-08-14 at 11:01 +0100, Sudeep KarkadaNagesha wrote:
>> Yes this doesn't cover the historical "ibm,ppc-interrupt-server#s",
>> for
>> which we can have PPC specific wrapper above the generic one i.e. get
>> the cpu node and then parse for thread id under custom property.
>=20
> A wrapper is wrong. I don't want to have to have all ppc callers to use
> a different function.
Ok.

On the side note the main intention of this patch series[1] is to avoid
calling of_get_cpu_node function once CPU devices are registered. It
initialises the of_node in all the cpu devices using this function when
registering the CPU device. It can be retrieved from cpu_dev->of_node.
So direct users of of_get_cpu_node can be reduced to avoid unnecessary
parsing of DT to find cpu node.

>=20
> As I said, just make a generic one that returns a thread ID, ie, same
> signature as the powerpc one. Make it weak, we can override it in
> powerpc-land,
IMO, making generic definition which adhere to the ePAPR specification
as weak is not so clean.

> or we can move the ibm,ppc-interrupt-server#s handling
> into the generic one, it won't hurt, but leave the thread_id return
> there, it doesn't hurt it will come in handy in a few cases without
> causing code duplication.
>=20

IMO moving of handling ibm,ppc-interrupt-server#s to generic code under
#ifdef CONFIG_PPC seems to be cleaner approach than weak definitation.

As per my understanding each thread is a different logical cpu.
Each logical cpu is mapped to unique physical id(either present in reg
field or legacy ibm,ppc-interrupt-server#s field). So given a logical
cpu id we can get the cpu node corresponding to it.
Looking @ smp_setup_cpu_maps in arch/powerpc/kernel/setup-common.c
and the comment in the same file: "This implementation only supports
power of 2 number of threads.." the thread id id is implicit in the
logical cpu id. Do we need to fetch that from DT ?

Regards,
Sudeep

[1] https://lkml.org/lkml/2013/7/22/219

^ permalink raw reply

* Re: [GIT PULL] DT/core: cpu_ofnode updates for v3.12
From: Rob Herring @ 2013-08-14 12:53 UTC (permalink / raw)
  To: Sudeep KarkadaNagesha, Benjamin Herrenschmidt
  Cc: devicetree@vger.kernel.org, linux-pm@vger.kernel.org,
	Viresh Kumar, linux-kernel@vger.kernel.org, Rafael J. Wysocki,
	Olof Johansson, linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <520B5584.7030608@arm.com>

On 08/14/2013 05:01 AM, Sudeep KarkadaNagesha wrote:
> On 13/08/13 22:07, Benjamin Herrenschmidt wrote:
>> On Tue, 2013-08-13 at 19:29 +0100, Sudeep KarkadaNagesha wrote:
>>> I don't understand completely the use of ibm,ppc-interrupt-server#s and
>>> its implications on generic of_get_cpu_node implementation.
>>> I see the PPC specific definition of of_get_cpu_node uses thread id only
>>> in 2 instances. Based on that, I have tried to move all the other
>>> instances to use generic definition.
>>>
>>> Let me know if the idea is correct.
>>
>> No. The device-tree historically only represents cores, not HW threads, so
>> it makes sense to retrieve also the thread number corresponding to the CPU.
>>
> Ok
> 
>> However, the mechanism to represent HW threads in the device-tree is currently
>> somewhat platform specific (the ibm,ppc-interrupt-server#s).
> I see most of the callers pass NULL to thread id argument except 2
> instances in entire tree. If that's the case why can't we move to use
> generic of_get_cpu_node in most of those cases and have PPC specific
> implementation for the ones using thread id.
> 
>>
>> So what you could do for now is:
>>
>>  - Have a generic version that always returns 0 as the thread, which is weak
> I would prefer to move to generic of_get_cpu_node where ever possible
> and rename the function that takes thread id rather than making generic
> one weak.
> 
>>
>>  - powerpc keeps its own implementation
> How about only in cases where it needs thread_id.
> 
>>
>>  - Start a discussion on the bindings (if not already there) to define threads
>> in a better way at which point the generic function can be updated.
>>
> I am not sure if we need to define any new bindings. Excerpts from ePAPR
> (v1.1):
> "3.7.1 General Properties of CPU nodes
> The value of "reg" is a <prop-encoded-array> that defines a unique
> CPU/thread id for the CPU/threads represented by the CPU node.
> If a CPU supports more than one thread (i.e. multiple streams of
> execution) the reg property is an array with 1 element per thread. The
> #address-cells on the /cpus node specifies how many cells each element
> of the array takes. Software can determine the number of threads by
> dividing the size of reg by the parent node's #address-cells."
> 
> And this is exactly in agreement to what's implemented in the generic
> of_get_cpu_node:
> 
> for_each_child_of_node(cpus, cpun) {
>         if (of_node_cmp(cpun->type, "cpu"))
>                 continue;
>         cell = of_get_property(cpun, "reg", &prop_len);
>         if (!cell) {
>                 pr_warn("%s: missing reg property\n", cpun->full_name);
>                 continue;
>         }
>         prop_len /= sizeof(*cell);
>         while (prop_len) {
>                 hwid = of_read_number(cell, ac);
>                 prop_len -= ac;
>                 if (arch_match_cpu_phys_id(cpu, hwid))
>                         return cpun;
>         }
> }

How about something like this:

for_each_child_of_node(cpus, cpun) {
        if (of_node_cmp(cpun->type, "cpu"))
                continue;

	if (arch_of_get_cpu_node(cpun, thread))
		return cpun;

        cell = of_get_property(cpun, "reg", &prop_len);
        if (!cell) {
                pr_warn("%s: missing reg property\n", cpun->full_name);
                continue;
        }
        prop_len /= sizeof(*cell);
        while (prop_len) {
                hwid = of_read_number(cell, ac);
                prop_len -= ac;
                if (arch_match_cpu_phys_id(cpu, hwid))
       	                return cpun;
        }
}

For PPC:

arch_of_get_cpu_node()
{
        const u32 *intserv;
        unsigned int plen, t;

        /* Check for ibm,ppc-interrupt-server#s. */
        intserv = of_get_property(np, "ibm,ppc-interrupt-server#s",
                                &plen);
        if (!intserv)
		return false;

	hardid = get_hard_smp_processor_id(cpu);

        plen /= sizeof(u32);
        for (t = 0; t < plen; t++) {
                 if (hardid == intserv[t]) {
                         if (thread)
                                  *thread = t;
                         return true;
                 }
        }
	return false;
}

> 
> Yes this doesn't cover the historical "ibm,ppc-interrupt-server#s", for
> which we can have PPC specific wrapper above the generic one i.e. get
> the cpu node and then parse for thread id under custom property.
> 
> Let me know your thoughts.
> 
> Regards,
> Sudeep
> 
> 
> 

^ permalink raw reply

* Re: [alsa-devel] [PATCH v4 resent 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
From: Sascha Hauer @ 2013-08-14 12:19 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: devicetree, alsa-devel, lars, timur, rob.herring, broonie,
	p.zabel, shawn.guo, linuxppc-dev
In-Reply-To: <20130814102346.GK31651@MrMyself>

On Wed, Aug 14, 2013 at 06:23:46PM +0800, Nicolin Chen wrote:
> Hi,
> 
> > > Surely, if I misunderstand your point, please correct me. And
> > > if you have any sage idea, please guide me.
> > 
> > Something like this:
> > 
> > clocks = <&clks 197>, <&clks 3>, <&clks 197>, <&clks 107>, <&clks SPDIF_EXT>,
> >          <&clks 118>, <&clks 62>, <&clks 139>, <&clks MLB_PHY>
> > clock-names = "core", "rxtx0", "rxtx1", "rxtx2", "rxtx3", "rxtx4", "rxtx5", "rxtx6", "rxtx7"
> > 
> > This describes the different input clocks to the spdif core and also
> > gives a hint to the array index (rxtx_n_) to use.
> 
> Thank you for the idea, and..hmm..I'm a bit confused.. Is this really
> a nicer way?

Yes, since the clk names are not an API. Exposing them to the devicetree
is not an option. The fact that the names are defined in
arch/arm/mach-imx/clk-imx6q.c and are used in the spdif driver makes
this really clear.

> 
> Actually the rx clock list and tx clock list are totally different.
> So doing this I have to list, in the maximum case, 24 (8 + 16) clock
> phandles for these two lists. And plussing another 6 I've listed in
> this binding doc -- thus there are totally 30 clock phanldes. But
> the 24 of 30 are only used to get two indexes.

The spdif core has 8 input clocks which have to be described in the
devicetree. Nobody says the mapping which clock name corresponds to
which bit combination has to be in the devicetree.

Look at the possible clocks:

0000 if (DPLL Locked) SPDIF_RxClk else extal
0001 if (DPLL Locked) SPDIF_RxClk else spdif_clk
0010 if (DPLL Locked) SPDIF_RxClk else asrc_clk
0011 if (DPLL Locked) SPDIF_RxClk else spdif_extclk
0100 if (DPLL Locked) SPDIF_Rxclk else esai_hckt
0101 extal_clk
0110 spdif_clk
0111 asrc_clk
1000 spdif_extclk
1001 esai_hckt
1010 if (DPLL Locked) SPDIF_RxClk else mlb_clk
1011 if (DPLL Locked) SPDIF_RxClk else mlb_phy_clk
1100 mkb_clk
1101 mlb_phy_clk

Only half of them actually are clocks. "if (DPLL Locked) SPDIF_RxClk
else ..." is not a clock. Every sane hardware developer would have
introduced a mux with 8 entries and an additional "Use DPLL if possible"
bit. Now this is not the case here so we have to live with it and
maintain the above table in the driver. And another one for the i.MX35
and still another one for i.MX53.

> 
> I think I need a little help here to understand why this is better.

As said, the clock names are not an API. Nobody changing clk-imx6q.c
expects to break devicetree bindings.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCH v4 resent 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
From: Philipp Zabel @ 2013-08-14 12:06 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: devicetree, alsa-devel, lars, Sascha Hauer, timur, rob.herring,
	broonie, linuxppc-dev
In-Reply-To: <20130814084801.GH31651@MrMyself>

Am Mittwoch, den 14.08.2013, 16:48 +0800 schrieb Nicolin Chen:
> Hi Sascha,
> 
> On Wed, Aug 14, 2013 at 09:50:17AM +0200, Sascha Hauer wrote:
> > > +  - tx-clksrc-names : The names for all available clock sources for tx, which
> > > +  is also being listed in SoC reference manual, ClkSrc_Sel bit of SPDIF_SRPC.
> > > +  And the name list would be different between different SoC. Use 'null' for
> > > +  those unlisted names, and the max number of tx-clksrc-names should be 8.
> > > +
> > > +  - rx-clksrc-names : The names for all available clock sources for rx, which
> > > +  is also being listed in SoC reference manual, TxClk_Source bit of SPDIF_STC.
> > > +  And the name list would be different between different SoC. Use 'null' for
> > > +  those unlisted names, and the max number of rx-clksrc-names should be 16.
> > > +
> > > +Optional properties:
> > > +
> > > +  - rx-clksrc-lock: This is a boolean property. If present, ClkSrc_Sel bit
> > > +  of SPDIF_SRPC would be set a clock source that cares DPLL locked condition.
> > > +
> > > +Example1:
> > > +
> > > +spdif: spdif@02004000 {
> > > +	clocks = <&clks 197>;
> > > +	clock-names = "core";
> > > +	rx-clksrc-lock;
> > > +	rx-clksrc-names =
> > > +		"lock.ext", "lock.spdif", "lock.asrc",
> > > +		"lock.spdif_ext", "lock.esai", "ext",
> > > +		"spdif", "asrc", "spdif_ext", "esai",
> > > +		"lock.mlb", "lock.mlb_phy", "mlb",
> > > +		"mlb_phy";
> > > +	tx-clksrc-names =
> > > +		"xtal", "spdif", "asrc", "spdif_ext",
> > > +		"esai", "ipg", "mlb", "mlb_phy";
> > 
> > I had a hard time understanding what you are doing here.
> > 
> > With this the clk names in arch/arm/mach-imx/clk-imx6q.c become an API
> > between the Kernel and the devicetree. Don't do that.
> > 
> > There is a standardized devicetree binding for clocks. Use it.
>  
> I think I should first explain to you what this part is doing:
> The driver needs to set Clk_source bit for TX/RX to select the 
> clock from a clock mux. The names listed above are those of the 
> clocks connecting to the mux, while they are not only internal 
> clocks which're included in clk-imx6q.c but also external ones,
> an on-board external osc for example.
> 
> The driver does get the clock by using the standard DT binding,
> see the 'clocks = <&clks 197>' above, and then compare this
> obtained clock->name with the name list to decide which value
> should be set to the Clk_source bit.
> 
> ==================================================================
> ClkSrc_Sel from i.MX6Q reference manual:
> 
> Clock source selection, all other settings not shown are reserved:
> 0000 if (DPLL Locked) SPDIF_RxClk else extal
> 0001 if (DPLL Locked) SPDIF_RxClk else spdif_clk
> 0010 if (DPLL Locked) SPDIF_RxClk else asrc_clk
> 0011 if (DPLL Locked) SPDIF_RxClk else spdif_extclk
> 0100 if (DPLL Locked) SPDIF_Rxclk else esai_hckt
> 0101 extal_clk
> 0110 spdif_clk
> 0111 asrc_clk
> 1000 spdif_extclk
> 1001 esai_hckt
> 1010 if (DPLL Locked) SPDIF_RxClk else mlb_clk
> ==================================================================

==================================================================
>From i.MX53 reference manual:

0000 if (DPLL Locked) SPDIF_RxClk else extal
0001 if (DPLL Locked) SPDIF_RxClk else spdif_clk
0011 if (DPLL Locked) SPDIF_RxClk else asrc_clk
0100 if (DPLL Locked) SPDIF_Rxclk else esai_hckt
0101 extal_clk
0110 spdif_clk
1000 asrc_clk
1001 esai_hckt
1010 if (DPLL Locked) SPDIF_RxClk else mlb_clk
1011 if (DPLL Locked) SPDIF_RxClk else camp_clk
1100 mkb_clk
1101 camp_clk
==================================================================

To me this looks like the device tree should just contain the list of
unique clock inputs using phandles.
	/* for i.MX6Q: */
	clocks = <&...>;
	clock-names = "xtal", "spdif", "asrc",
	              "spdif_ext", "esai", "mlb";

	/* for i.MX53: */
	clocks = <&...>;
	clock-names = "xtal", "spdif", "asrc",
	              "esai", "mlb", "camp";

The driver could contain this list of named inputs to the multiplexer
and the DPLL locking information for each SoC version. The per-clock
DPLL locking bit shouldn't be in the device tree at all.

> So the name list here basically is not being used to obtain a
> clock like what standardized DT binding does but to provide the
> driver a full list to look up which value should be exactly used
> according to the obtained clock.
> 
> I think I should revise the binding doc for these two lists. It 
> might be hard to explain within that kinda short paragraph. 
> 
> Surely, if I misunderstand your point, please correct me. And
> if you have any sage idea, please guide me.

regards
Philipp

^ permalink raw reply

* [RFC V2 PATCH 6/6] cpuidle/ppc : Queue a hrtimer on bc_cpu, explicitly to do broadcast handling
From: Preeti U Murthy @ 2013-08-14 11:56 UTC (permalink / raw)
  To: fweisbec, paul.gortmaker, paulus, shangw, galak, deepthi, benh,
	paulmck, arnd, linux-pm, rostedt, rjw, john.stultz, tglx,
	chenhui.zhao, michael, r58472, geoff, linux-kernel, srivatsa.bhat,
	schwidefsky, svaidy, linuxppc-dev
In-Reply-To: <20130814115311.5193.32212.stgit@preeti.in.ibm.com>

In the current design we were depending on the timer interrupt on the
bc_cpu to trigger broadcast handling. In tickless idle, timer interrupts
could be many ticks away which could result in missed wakeups on CPUs in deep
idle states. Disabling tickless idle on the bc_cpu is not good for
powersavings.

Therefore queue a hrtimer specifically for broadcast handling. When the broadcast
CPU is chosen, it schedules this hrtimer to fire after a jiffy.
This is meant to initiate broadcast handling. For each expiration of
this hrtimer thereon, it is reprogrammed to fire at the time the next broadcast
handling has to be done. But if there is no pending broadcast handling to be
done in the future, the broadcast cpu is invalidated and the hrtimer is
cancelled. The above cycle repeats when the next CPU attempts to enter sleep state.

Of course the time at which the hrtimer fires initially can be scheduled for
time=target_residency of deep idle state instead of a jiffy, since CPUs going
into deep idle states will not have their next wakeup event before this
target_residency time of the the deep idle state.
  But this patchset is based on longnap which is now used to mimick sleep
but is actually nap with decrementer interrupts disabled. Therefore its
target_residency is the same as nap. The CPUs going into longnap, will
probably need to be woken up sooner than they would have been,had they gone into
sleep. Hence the initial scheduling of the hrtimer is held at a jiffy as of now.

There is one other significant point. On CPU hotplug, the hrtimer on the
broadcast CPU is cancelled, the bc_cpu entry is invalidated, a new
broadcast cpu is chosen as before, and an IPI is sent to it. However instead
of using a broadcast IPI to wake it up, use smp_call_function_single(),
because apart from just wakeup, the new broadcast CPU has to restart
the hrtimer on itself so as to continue broadcast handling.

Signed-off-by: Preeti U Murthy<preeti@linux.vnet.ibm.com>
---

 arch/powerpc/include/asm/time.h                 |    5 ++
 arch/powerpc/kernel/time.c                      |   47 ++++++++++++++++++++---
 arch/powerpc/platforms/powernv/processor_idle.c |   38 ++++++++++++++-----
 3 files changed, 73 insertions(+), 17 deletions(-)

diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index 92260c9..b9a60eb 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -16,6 +16,7 @@
 #ifdef __KERNEL__
 #include <linux/types.h>
 #include <linux/percpu.h>
+#include <linux/ktime.h>
 
 #include <asm/processor.h>
 
@@ -26,6 +27,7 @@ extern unsigned long tb_ticks_per_sec;
 extern struct clock_event_device decrementer_clockevent;
 extern struct clock_event_device broadcast_clockevent;
 extern struct clock_event_device bc_timer;
+extern struct hrtimer *bc_hrtimer;
 
 struct rtc_time;
 extern void to_tm(int tim, struct rtc_time * tm);
@@ -35,7 +37,10 @@ extern void decrementer_timer_interrupt(void);
 extern void generic_calibrate_decr(void);
 
 extern void set_dec_cpu6(unsigned int val);
+extern ktime_t get_next_bc_tick(void);
+extern enum hrtimer_restart handle_broadcast(struct hrtimer *hrtimer);
 extern int bc_cpu;
+extern int bc_hrtimer_initialized;
 
 /* Some sane defaults: 125 MHz timebase, 1GHz processor */
 extern unsigned long ppc_proc_freq;
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index a19c8ca..1a64d58 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -43,6 +43,8 @@
 #include <linux/kernel_stat.h>
 #include <linux/time.h>
 #include <linux/timer.h>
+#include <linux/hrtimer.h>
+#include <linux/ktime.h>
 #include <linux/init.h>
 #include <linux/profile.h>
 #include <linux/cpu.h>
@@ -128,6 +130,8 @@ EXPORT_SYMBOL(broadcast_clockevent);
 DEFINE_PER_CPU(u64, decrementers_next_tb);
 static DEFINE_PER_CPU(struct clock_event_device, decrementers);
 struct clock_event_device bc_timer;
+struct hrtimer *bc_hrtimer;
+int bc_hrtimer_initialized = 0;
 
 int bc_cpu = -1;
 #define XSEC_PER_SEC (1024*1024)
@@ -504,8 +508,6 @@ void timer_interrupt(struct pt_regs * regs)
 	struct pt_regs *old_regs;
 	u64 *next_tb = &__get_cpu_var(decrementers_next_tb);
 	struct clock_event_device *evt = &__get_cpu_var(decrementers);
-	struct clock_event_device *bc_evt = &bc_timer;
-	int cpu = smp_processor_id();
 	u64 now;
 
 	/* Ensure a positive value is written to the decrementer, or else
@@ -551,10 +553,6 @@ void timer_interrupt(struct pt_regs * regs)
 		*next_tb = ~(u64)0;
 		if (evt->event_handler)
 			evt->event_handler(evt);
-		if (cpu == bc_cpu && bc_evt->event_handler) {
-			bc_evt->event_handler(bc_evt);
-		}
-
 	} else {
 		now = *next_tb - now;
 		if (now <= DECREMENTER_MAX)
@@ -864,6 +862,42 @@ static void decrementer_timer_broadcast(const struct cpumask *mask)
 	arch_send_tick_broadcast(mask);
 }
 
+ktime_t get_next_bc_tick(void)
+{
+	u64 next_bc_ns;
+
+	next_bc_ns = (tb_ticks_per_jiffy / tb_ticks_per_usec) * 1000;
+	return ns_to_ktime(next_bc_ns);
+}
+
+enum hrtimer_restart handle_broadcast(struct hrtimer *hrtimer)
+{
+	struct clock_event_device *bc_evt = &bc_timer;
+	int cpu = smp_processor_id();
+	ktime_t interval;
+
+	u64 now = get_tb_or_rtc();
+	ktime_t now_ktime = ns_to_ktime((now / tb_ticks_per_usec) * 1000);
+
+	bc_evt->event_handler(bc_evt);
+
+	/* FIXME: There could be a race condition between the time we do this
+	 * check and invalidate the bc_cpu and CPUs check for the existence of
+	 * bc_cpu and enter longnap_loop.This region could be protected by
+	 * the longnap_idle_lock as well. But is there a better way to handle such
+	 * race conditions, like relying on the existing tick_broadcast_lock
+	 * and remove explicit locking under such circumstances as below?
+	 */
+	if (bc_evt->next_event.tv64 == KTIME_MAX) {
+		bc_cpu = -1;
+		return HRTIMER_NORESTART;
+	}
+
+	interval.tv64 = bc_evt->next_event.tv64 - now_ktime.tv64;
+	hrtimer_forward_now(hrtimer, interval);
+	return HRTIMER_RESTART;
+}
+
 static void register_decrementer_clockevent(int cpu)
 {
 	struct clock_event_device *dec = &per_cpu(decrementers, cpu);
@@ -888,7 +922,6 @@ static void register_broadcast_clockevent(int cpu)
 		    bc_evt->name, bc_evt->mult, bc_evt->shift, cpu);
 
 	clockevents_register_device(bc_evt);
-	bc_cpu = cpu;
 }
 
 static void __init init_decrementer_clockevent(void)
diff --git a/arch/powerpc/platforms/powernv/processor_idle.c b/arch/powerpc/platforms/powernv/processor_idle.c
index 9554da6..8386ef6 100644
--- a/arch/powerpc/platforms/powernv/processor_idle.c
+++ b/arch/powerpc/platforms/powernv/processor_idle.c
@@ -12,6 +12,7 @@
 #include <linux/clockchips.h>
 #include <linux/tick.h>
 #include <linux/spinlock.h>
+#include <linux/slab.h>
 
 #include <asm/machdep.h>
 #include <asm/runlatch.h>
@@ -98,8 +99,6 @@ static int longnap_loop(struct cpuidle_device *dev,
 			spin_unlock_irqrestore(&longnap_idle_lock, flags);
 		}
 		else {
-			if (cpumask_empty(tick_get_broadcast_oneshot_mask()))
-				bc_cpu = -1;
 			/* Wakeup on a decrementer interrupt, Do a nap */
 			lpcr |= LPCR_PECE1;
 			mtspr(SPRN_LPCR, lpcr);
@@ -107,9 +106,21 @@ static int longnap_loop(struct cpuidle_device *dev,
 			power7_nap();
 		}
 	} else {
-		/* First CPU to go to longnap, hence become the bc_cpu. Then
-		 * exit idle and re-enter to disable tickless idle on this cpu
-		 */
+		/* First CPU to go to longnap, hence become the bc_cpu.
+ 		 */
+		if (!bc_hrtimer_initialized) {
+			bc_hrtimer = kmalloc(sizeof(*bc_hrtimer), GFP_KERNEL);
+			if (!bc_hrtimer)
+				return index;
+			hrtimer_init(bc_hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_PINNED);
+			bc_hrtimer->function = handle_broadcast;
+			hrtimer_start(bc_hrtimer, get_next_bc_tick(),
+				HRTIMER_MODE_REL_PINNED);
+			bc_hrtimer_initialized = 1;
+		}
+		else
+			hrtimer_start(bc_hrtimer, get_next_bc_tick(), HRTIMER_MODE_REL_PINNED);
+
 		bc_cpu = cpu;
 		spin_unlock_irqrestore(&longnap_idle_lock, flags);
 	}
@@ -117,6 +128,11 @@ static int longnap_loop(struct cpuidle_device *dev,
 	return index;
 }
 
+static void start_hrtimer(void *data)
+{
+	hrtimer_start(bc_hrtimer, get_next_bc_tick(), HRTIMER_MODE_REL_PINNED);
+}
+
 /*
  * States for dedicated partition case.
  */
@@ -165,12 +181,14 @@ static int powernv_cpuidle_add_cpu_notifier(struct notifier_block *n,
 		case CPU_DYING:
 		case CPU_DYING_FROZEN:
 			spin_lock_irqsave(&longnap_idle_lock, flags);
-			if (hotcpu == bc_cpu)
+			if (hotcpu == bc_cpu) {
 				bc_cpu = -1;
-			if (!cpumask_empty(tick_get_broadcast_oneshot_mask())) {
-				cpu = cpumask_first(tick_get_broadcast_oneshot_mask());
-				bc_cpu = cpu;
-				arch_send_tick_broadcast(cpumask_of(cpu));
+				hrtimer_cancel(bc_hrtimer);
+				if (!cpumask_empty(tick_get_broadcast_oneshot_mask())) {
+					cpu = cpumask_first(tick_get_broadcast_oneshot_mask());
+					bc_cpu = cpu;
+					smp_call_function_single(cpu, start_hrtimer, NULL, 0);
+				}
 			}
 			spin_unlock_irqrestore(&longnap_idle_lock, flags);
 			break;

^ permalink raw reply related

* [RFC V2 PATCH 5/6] cpuidle/ppc: Enable dynamic movement of the broadcast functionality across CPUs
From: Preeti U Murthy @ 2013-08-14 11:56 UTC (permalink / raw)
  To: fweisbec, paul.gortmaker, paulus, shangw, galak, deepthi, benh,
	paulmck, arnd, linux-pm, rostedt, rjw, john.stultz, tglx,
	chenhui.zhao, michael, r58472, geoff, linux-kernel, srivatsa.bhat,
	schwidefsky, svaidy, linuxppc-dev
In-Reply-To: <20130814115311.5193.32212.stgit@preeti.in.ibm.com>

In the current design of the timer offload framework for powerpc, there is a
dedicated broadcast CPU, which is the boot CPU. But this is not good because:

a.It disallows this CPU from being hotplugged out.

b.Overburdening this CPU with the broadcast duty can take
a toll on the performance, which could worsen if this CPU
is already too busy.

c.This could lead to thermal or power imbalance within the chip.

To overcome the above constraints, float around the duty of doing a broadcast
around the CPUs. The current design proposes to choose the first CPU that
attempts to go to deep idle state to be the broadcast CPU/bc_cpu.
It is disallowed from entering deep idle state.

Let the broadcast CPU become invalidated when there are no more CPUs in
the broadcast mask. Until this point the rest of the CPUs attempting to enter
deep idle will be allowed to do so, to be woken up by the broadcast CPU.
Hence the set and unset of the bc_cpu variable is done only by the broadcast
CPU.

Protect the region of all the above activity with a lock in order to avoid
race conditions between readers and writers of the bc_cpu
entry and the broadcast cpus mask. One typical scenario could be:

CPUA				CPUB

Read bc_cpu exists		Is the bc_cpu, finds the broadcast mask
				empty,and invalidates the bc_cpu.

Enter itself into the
the broadcast mask.

Thus, CPUA  would go into deep idle when broadcast handling is inactive.

The broadcast clockevent device is now one single pseudo device capable of working for
all possible cpus (instead of being per-cpu like it was before, there is no
point in having so), due to the dynamic movement of the broadcast CPU. This
is a pseudo device and the dynamic movement of bc_cpu will therefore not affect
its functioning. The broadcast clockevent device's event handler will be called
by the bc_cpu in each of its timer interrupt.

This patchset adds hotplug notifiers to change the bc_cpu, in case it goes
offline. In this case choose the first cpu in the broadcast mask to be the
next bc_cpu and send an IPI, to wake it up to begin to handle broadcast events
thereon. This IPI is the same as the broadcast IPI.
   The idea being the intention of both these scenarios(hotplug and actual
broadcast wakeup) is to wake up a CPU in the broadcast mask, except that
they are for different reasons.

Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
---

 arch/powerpc/include/asm/time.h                 |    1 
 arch/powerpc/kernel/time.c                      |   10 ++--
 arch/powerpc/platforms/powernv/processor_idle.c |   56 +++++++++++++++++++----
 3 files changed, 53 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index 936be0d..92260c9 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -25,6 +25,7 @@ extern unsigned long tb_ticks_per_usec;
 extern unsigned long tb_ticks_per_sec;
 extern struct clock_event_device decrementer_clockevent;
 extern struct clock_event_device broadcast_clockevent;
+extern struct clock_event_device bc_timer;
 
 struct rtc_time;
 extern void to_tm(int tim, struct rtc_time * tm);
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 7e858e1..a19c8ca 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -127,9 +127,9 @@ EXPORT_SYMBOL(broadcast_clockevent);
 
 DEFINE_PER_CPU(u64, decrementers_next_tb);
 static DEFINE_PER_CPU(struct clock_event_device, decrementers);
-static DEFINE_PER_CPU(struct clock_event_device, bc_timer);
+struct clock_event_device bc_timer;
 
-int bc_cpu;
+int bc_cpu = -1;
 #define XSEC_PER_SEC (1024*1024)
 
 #ifdef CONFIG_PPC64
@@ -504,7 +504,7 @@ void timer_interrupt(struct pt_regs * regs)
 	struct pt_regs *old_regs;
 	u64 *next_tb = &__get_cpu_var(decrementers_next_tb);
 	struct clock_event_device *evt = &__get_cpu_var(decrementers);
-	struct clock_event_device *bc_evt = &__get_cpu_var(bc_timer);
+	struct clock_event_device *bc_evt = &bc_timer;
 	int cpu = smp_processor_id();
 	u64 now;
 
@@ -879,10 +879,10 @@ static void register_decrementer_clockevent(int cpu)
 
 static void register_broadcast_clockevent(int cpu)
 {
-	struct clock_event_device *bc_evt = &per_cpu(bc_timer, cpu);
+	struct clock_event_device *bc_evt = &bc_timer;
 
 	*bc_evt = broadcast_clockevent;
-	bc_evt->cpumask = cpumask_of(cpu);
+	bc_evt->cpumask = cpu_possible_mask;
 
 	printk_once(KERN_DEBUG "clockevent: %s mult[%x] shift[%d] cpu[%d]\n",
 		    bc_evt->name, bc_evt->mult, bc_evt->shift, cpu);
diff --git a/arch/powerpc/platforms/powernv/processor_idle.c b/arch/powerpc/platforms/powernv/processor_idle.c
index 9aca502..9554da6 100644
--- a/arch/powerpc/platforms/powernv/processor_idle.c
+++ b/arch/powerpc/platforms/powernv/processor_idle.c
@@ -10,6 +10,8 @@
 #include <linux/cpu.h>
 #include <linux/notifier.h>
 #include <linux/clockchips.h>
+#include <linux/tick.h>
+#include <linux/spinlock.h>
 
 #include <asm/machdep.h>
 #include <asm/runlatch.h>
@@ -26,6 +28,8 @@ static int max_idle_state = MAX_IDLE_STATE_COUNT - 1;
 static struct cpuidle_device __percpu *powernv_cpuidle_devices;
 static struct cpuidle_state *cpuidle_state_table;
 
+static DEFINE_SPINLOCK(longnap_idle_lock);
+
 static int snooze_loop(struct cpuidle_device *dev,
 			struct cpuidle_driver *drv,
 			int index)
@@ -70,6 +74,7 @@ static int longnap_loop(struct cpuidle_device *dev,
 	int cpu = dev->cpu;
 
 	unsigned long lpcr = mfspr(SPRN_LPCR);
+	unsigned long flags;
 
 	lpcr &= ~(LPCR_MER | LPCR_PECE); /* lpcr[mer] must be 0 */
 
@@ -78,16 +83,35 @@ static int longnap_loop(struct cpuidle_device *dev,
 	 */
 	lpcr |= LPCR_PECE0;
 
-	if (cpu != bc_cpu) {
-		mtspr(SPRN_LPCR, lpcr);
-		clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu);
-		power7_nap();
-		clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu);
+	spin_lock_irqsave(&longnap_idle_lock, flags);
+
+	if (bc_cpu != -1) {
+		if (cpu != bc_cpu) {
+			mtspr(SPRN_LPCR, lpcr);
+			clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu);
+			spin_unlock_irqrestore(&longnap_idle_lock, flags);
+
+			power7_nap();
+
+			spin_lock_irqsave(&longnap_idle_lock, flags);
+			clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu);
+			spin_unlock_irqrestore(&longnap_idle_lock, flags);
+		}
+		else {
+			if (cpumask_empty(tick_get_broadcast_oneshot_mask()))
+				bc_cpu = -1;
+			/* Wakeup on a decrementer interrupt, Do a nap */
+			lpcr |= LPCR_PECE1;
+			mtspr(SPRN_LPCR, lpcr);
+			spin_unlock_irqrestore(&longnap_idle_lock, flags);
+			power7_nap();
+		}
 	} else {
-		/* Wakeup on a decrementer interrupt, Do a nap */
-		lpcr |= LPCR_PECE1;
-		mtspr(SPRN_LPCR, lpcr);
-		power7_nap();
+		/* First CPU to go to longnap, hence become the bc_cpu. Then
+		 * exit idle and re-enter to disable tickless idle on this cpu
+		 */
+		bc_cpu = cpu;
+		spin_unlock_irqrestore(&longnap_idle_lock, flags);
 	}
 
 	return index;
@@ -124,6 +148,8 @@ static int powernv_cpuidle_add_cpu_notifier(struct notifier_block *n,
 			unsigned long action, void *hcpu)
 {
 	int hotcpu = (unsigned long)hcpu;
+	int cpu;
+	unsigned long flags;
 	struct cpuidle_device *dev =
 			per_cpu_ptr(powernv_cpuidle_devices, hotcpu);
 
@@ -136,6 +162,18 @@ static int powernv_cpuidle_add_cpu_notifier(struct notifier_block *n,
 			cpuidle_resume_and_unlock();
 			break;
 
+		case CPU_DYING:
+		case CPU_DYING_FROZEN:
+			spin_lock_irqsave(&longnap_idle_lock, flags);
+			if (hotcpu == bc_cpu)
+				bc_cpu = -1;
+			if (!cpumask_empty(tick_get_broadcast_oneshot_mask())) {
+				cpu = cpumask_first(tick_get_broadcast_oneshot_mask());
+				bc_cpu = cpu;
+				arch_send_tick_broadcast(cpumask_of(cpu));
+			}
+			spin_unlock_irqrestore(&longnap_idle_lock, flags);
+			break;
 		case CPU_DEAD:
 		case CPU_DEAD_FROZEN:
 			cpuidle_pause_and_lock();

^ permalink raw reply related

* [RFC V2 PATCH 4/6] cpuidle/ppc: Add longnap state to the idle states on powernv
From: Preeti U Murthy @ 2013-08-14 11:56 UTC (permalink / raw)
  To: fweisbec, paul.gortmaker, paulus, shangw, galak, deepthi, benh,
	paulmck, arnd, linux-pm, rostedt, rjw, john.stultz, tglx,
	chenhui.zhao, michael, r58472, geoff, linux-kernel, srivatsa.bhat,
	schwidefsky, svaidy, linuxppc-dev
In-Reply-To: <20130814115311.5193.32212.stgit@preeti.in.ibm.com>

This patch hooks into the existing broadcast framework along with the support
that this patchset introduces for ppc, and the cpuidle driver backend
for powernv(posted out by Deepthi Dharwar:https://lkml.org/lkml/2013/7/23/128)
to add sleep state as one of the deep idle states, in which the decrementer
is switched off.

However in this patch, we only emulate sleep by going into a state which does
a nap with the decrementer interrupts disabled, termed as longnap. This enables
focus on the timer broadcast framework for ppc in this series of patches ,
which is required as a first step to enable sleep on ppc.

Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
---

 arch/powerpc/platforms/powernv/processor_idle.c |   48 +++++++++++++++++++++++
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/processor_idle.c b/arch/powerpc/platforms/powernv/processor_idle.c
index f43ad91a..9aca502 100644
--- a/arch/powerpc/platforms/powernv/processor_idle.c
+++ b/arch/powerpc/platforms/powernv/processor_idle.c
@@ -9,16 +9,18 @@
 #include <linux/cpuidle.h>
 #include <linux/cpu.h>
 #include <linux/notifier.h>
+#include <linux/clockchips.h>
 
 #include <asm/machdep.h>
 #include <asm/runlatch.h>
+#include <asm/time.h>
 
 struct cpuidle_driver powernv_idle_driver = {
 	.name =		"powernv_idle",
 	.owner =	THIS_MODULE,
 };
 
-#define MAX_IDLE_STATE_COUNT	2
+#define MAX_IDLE_STATE_COUNT	3
 
 static int max_idle_state = MAX_IDLE_STATE_COUNT - 1;
 static struct cpuidle_device __percpu *powernv_cpuidle_devices;
@@ -54,6 +56,43 @@ static int nap_loop(struct cpuidle_device *dev,
 	return index;
 }
 
+/* Emulate sleep, with long nap.
+ * During sleep, the core does not receive decrementer interrupts.
+ * Emulate sleep using long nap with decrementers interrupts disabled.
+ * This is an initial prototype to test the timer offload framework for ppc.
+ * We will eventually introduce the sleep state once the timer offload framework
+ * for ppc is stable.
+ */
+static int longnap_loop(struct cpuidle_device *dev,
+				struct cpuidle_driver *drv,
+				int index)
+{
+	int cpu = dev->cpu;
+
+	unsigned long lpcr = mfspr(SPRN_LPCR);
+
+	lpcr &= ~(LPCR_MER | LPCR_PECE); /* lpcr[mer] must be 0 */
+
+	/* exit powersave upon external interrupt, but not decrementer
+	 * interrupt, Emulate sleep.
+	 */
+	lpcr |= LPCR_PECE0;
+
+	if (cpu != bc_cpu) {
+		mtspr(SPRN_LPCR, lpcr);
+		clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu);
+		power7_nap();
+		clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu);
+	} else {
+		/* Wakeup on a decrementer interrupt, Do a nap */
+		lpcr |= LPCR_PECE1;
+		mtspr(SPRN_LPCR, lpcr);
+		power7_nap();
+	}
+
+	return index;
+}
+
 /*
  * States for dedicated partition case.
  */
@@ -72,6 +111,13 @@ static struct cpuidle_state powernv_states[MAX_IDLE_STATE_COUNT] = {
 		.exit_latency = 10,
 		.target_residency = 100,
 		.enter = &nap_loop },
+	 { /* LongNap */
+		.name = "LongNap",
+		.desc = "LongNap",
+		.flags = CPUIDLE_FLAG_TIME_VALID,
+		.exit_latency = 10,
+		.target_residency = 100,
+		.enter = &longnap_loop },
 };
 
 static int powernv_cpuidle_add_cpu_notifier(struct notifier_block *n,

^ permalink raw reply related

* [RFC V2 PATCH 3/6] cpuidle/ppc: Add timer offload framework to support deep idle states
From: Preeti U Murthy @ 2013-08-14 11:56 UTC (permalink / raw)
  To: fweisbec, paul.gortmaker, paulus, shangw, galak, deepthi, benh,
	paulmck, arnd, linux-pm, rostedt, rjw, john.stultz, tglx,
	chenhui.zhao, michael, r58472, geoff, linux-kernel, srivatsa.bhat,
	schwidefsky, svaidy, linuxppc-dev
In-Reply-To: <20130814115311.5193.32212.stgit@preeti.in.ibm.com>

On ppc, in deep idle states, the local clock event device of CPUs gets
switched off. On PowerPC, the local clock event device is called the
decrementer. Make use of the broadcast framework to issue interrupts to
cpus in deep idle states on their timer events, except that on ppc, we
do not have an external device such as HPET, but we use the decrementer
of one of the CPUs itself as the broadcast device.

Instantiate two different clock event devices, one representing the
decrementer and another representing the broadcast device for each cpu.
The cpu which registers its broadcast device will be responsible for
performing the function of issuing timer interrupts to CPUs in deep idle
states, and is referred to as the broadcast cpu/bc_cpu in the changelogs of this
patchset for convenience. Such a CPU is not allowed to enter deep idle
states, where the decrementer is switched off.

For now, only the boot cpu's broadcast device gets registered as a clock event
device along with the decrementer. Hence this is the broadcast cpu.

On the broadcast cpu, on each timer interrupt, apart from the regular local
timer event handler the broadcast handler is also called. We avoid the overhead
of programming the decrementer specifically for a broadcast event. The reason is for
performance and scalability reasons. Say cpuX goes to deep idle state. It
has to ask the broadcast CPU to reprogram its(broadcast CPU's) decrementer for
the next local timer event of cpuX. cpuX can do so only by sending an IPI to the
broadcast CPU. With many more cpus going to deep idle, this model of sending
IPIs each time will result in performance bottleneck and may not scale well.

Apart from this there is no change in the way broadcast is handled today. On
a broadcast ipi the event handler for a timer interrupt is called on the cpu
in deep idle state to handle the local events.

The current design and implementation of the timer offload framework supports
the ONESHOT tick mode but not the PERIODIC mode.

Signed-off-by: Preeti U. Murthy <preeti@linux.vnet.ibm.com>
---

 arch/powerpc/include/asm/time.h        |    3 +
 arch/powerpc/kernel/smp.c              |    4 +-
 arch/powerpc/kernel/time.c             |   81 ++++++++++++++++++++++++++++++++
 arch/powerpc/platforms/powernv/Kconfig |    1 
 4 files changed, 86 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index c1f2676..936be0d 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -24,14 +24,17 @@ extern unsigned long tb_ticks_per_jiffy;
 extern unsigned long tb_ticks_per_usec;
 extern unsigned long tb_ticks_per_sec;
 extern struct clock_event_device decrementer_clockevent;
+extern struct clock_event_device broadcast_clockevent;
 
 struct rtc_time;
 extern void to_tm(int tim, struct rtc_time * tm);
 extern void GregorianDay(struct rtc_time *tm);
+extern void decrementer_timer_interrupt(void);
 
 extern void generic_calibrate_decr(void);
 
 extern void set_dec_cpu6(unsigned int val);
+extern int bc_cpu;
 
 /* Some sane defaults: 125 MHz timebase, 1GHz processor */
 extern unsigned long ppc_proc_freq;
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 6a68ca4..d3b7014 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -114,7 +114,7 @@ int smp_generic_kick_cpu(int nr)
 
 static irqreturn_t timer_action(int irq, void *data)
 {
-	timer_interrupt();
+	decrementer_timer_interrupt();
 	return IRQ_HANDLED;
 }
 
@@ -223,7 +223,7 @@ irqreturn_t smp_ipi_demux(void)
 
 #ifdef __BIG_ENDIAN
 		if (all & (1 << (24 - 8 * PPC_MSG_TIMER)))
-			timer_interrupt();
+			decrementer_timer_interrupt();
 		if (all & (1 << (24 - 8 * PPC_MSG_RESCHEDULE)))
 			scheduler_ipi();
 		if (all & (1 << (24 - 8 * PPC_MSG_CALL_FUNC_SINGLE)))
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 65ab9e9..7e858e1 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -42,6 +42,7 @@
 #include <linux/timex.h>
 #include <linux/kernel_stat.h>
 #include <linux/time.h>
+#include <linux/timer.h>
 #include <linux/init.h>
 #include <linux/profile.h>
 #include <linux/cpu.h>
@@ -97,8 +98,11 @@ static struct clocksource clocksource_timebase = {
 
 static int decrementer_set_next_event(unsigned long evt,
 				      struct clock_event_device *dev);
+static int broadcast_set_next_event(unsigned long evt,
+				      struct clock_event_device *dev);
 static void decrementer_set_mode(enum clock_event_mode mode,
 				 struct clock_event_device *dev);
+static void decrementer_timer_broadcast(const struct cpumask *mask);
 
 struct clock_event_device decrementer_clockevent = {
 	.name           = "decrementer",
@@ -106,13 +110,26 @@ struct clock_event_device decrementer_clockevent = {
 	.irq            = 0,
 	.set_next_event = decrementer_set_next_event,
 	.set_mode       = decrementer_set_mode,
-	.features       = CLOCK_EVT_FEAT_ONESHOT,
+	.broadcast	= decrementer_timer_broadcast,
+	.features       = CLOCK_EVT_FEAT_C3STOP | CLOCK_EVT_FEAT_ONESHOT,
 };
 EXPORT_SYMBOL(decrementer_clockevent);
 
+struct clock_event_device broadcast_clockevent = {
+	.name           = "broadcast",
+	.rating         = 200,
+	.irq            = 0,
+	.set_next_event = broadcast_set_next_event,
+	.set_mode       = decrementer_set_mode,
+	.features       = CLOCK_EVT_FEAT_ONESHOT,
+};
+EXPORT_SYMBOL(broadcast_clockevent);
+
 DEFINE_PER_CPU(u64, decrementers_next_tb);
 static DEFINE_PER_CPU(struct clock_event_device, decrementers);
+static DEFINE_PER_CPU(struct clock_event_device, bc_timer);
 
+int bc_cpu;
 #define XSEC_PER_SEC (1024*1024)
 
 #ifdef CONFIG_PPC64
@@ -487,6 +504,8 @@ void timer_interrupt(struct pt_regs * regs)
 	struct pt_regs *old_regs;
 	u64 *next_tb = &__get_cpu_var(decrementers_next_tb);
 	struct clock_event_device *evt = &__get_cpu_var(decrementers);
+	struct clock_event_device *bc_evt = &__get_cpu_var(bc_timer);
+	int cpu = smp_processor_id();
 	u64 now;
 
 	/* Ensure a positive value is written to the decrementer, or else
@@ -532,6 +551,10 @@ void timer_interrupt(struct pt_regs * regs)
 		*next_tb = ~(u64)0;
 		if (evt->event_handler)
 			evt->event_handler(evt);
+		if (cpu == bc_cpu && bc_evt->event_handler) {
+			bc_evt->event_handler(bc_evt);
+		}
+
 	} else {
 		now = *next_tb - now;
 		if (now <= DECREMENTER_MAX)
@@ -806,6 +829,20 @@ static int decrementer_set_next_event(unsigned long evt,
 	return 0;
 }
 
+/*
+ * We cannot program the decrementer of a remote CPU. Hence CPUs going into
+ * deep idle states need to send IPIs to the broadcast CPU to program its
+ * decrementer for their next local event so as to receive a broadcast IPI
+ * for the same. In order to avoid the overhead of multiple CPUs from sending
+ * IPIs, this function is a nop. Instead the broadcast CPU will handle the
+ * wakeup of CPUs in deep idle states in each of its local timer interrupts.
+ */
+static int broadcast_set_next_event(unsigned long evt,
+					struct clock_event_device *dev)
+{
+	return 0;
+}
+
 static void decrementer_set_mode(enum clock_event_mode mode,
 				 struct clock_event_device *dev)
 {
@@ -813,6 +850,20 @@ static void decrementer_set_mode(enum clock_event_mode mode,
 		decrementer_set_next_event(DECREMENTER_MAX, dev);
 }
 
+void decrementer_timer_interrupt(void)
+{
+	struct clock_event_device *evt;
+	evt = &per_cpu(decrementers, smp_processor_id());
+
+	if (evt->event_handler)
+		evt->event_handler(evt);
+}
+
+static void decrementer_timer_broadcast(const struct cpumask *mask)
+{
+	arch_send_tick_broadcast(mask);
+}
+
 static void register_decrementer_clockevent(int cpu)
 {
 	struct clock_event_device *dec = &per_cpu(decrementers, cpu);
@@ -826,6 +877,20 @@ static void register_decrementer_clockevent(int cpu)
 	clockevents_register_device(dec);
 }
 
+static void register_broadcast_clockevent(int cpu)
+{
+	struct clock_event_device *bc_evt = &per_cpu(bc_timer, cpu);
+
+	*bc_evt = broadcast_clockevent;
+	bc_evt->cpumask = cpumask_of(cpu);
+
+	printk_once(KERN_DEBUG "clockevent: %s mult[%x] shift[%d] cpu[%d]\n",
+		    bc_evt->name, bc_evt->mult, bc_evt->shift, cpu);
+
+	clockevents_register_device(bc_evt);
+	bc_cpu = cpu;
+}
+
 static void __init init_decrementer_clockevent(void)
 {
 	int cpu = smp_processor_id();
@@ -840,6 +905,19 @@ static void __init init_decrementer_clockevent(void)
 	register_decrementer_clockevent(cpu);
 }
 
+static void __init init_broadcast_clockevent(void)
+{
+	int cpu = smp_processor_id();
+
+	clockevents_calc_mult_shift(&broadcast_clockevent, ppc_tb_freq, 4);
+
+	broadcast_clockevent.max_delta_ns =
+		clockevent_delta2ns(DECREMENTER_MAX, &broadcast_clockevent);
+	broadcast_clockevent.min_delta_ns =
+		clockevent_delta2ns(2, &broadcast_clockevent);
+	register_broadcast_clockevent(cpu);
+}
+
 void secondary_cpu_time_init(void)
 {
 	/* Start the decrementer on CPUs that have manual control
@@ -916,6 +994,7 @@ void __init time_init(void)
 	clocksource_init();
 
 	init_decrementer_clockevent();
+	init_broadcast_clockevent();
 }
 
 
diff --git a/arch/powerpc/platforms/powernv/Kconfig b/arch/powerpc/platforms/powernv/Kconfig
index ace2d22..e1a96eb 100644
--- a/arch/powerpc/platforms/powernv/Kconfig
+++ b/arch/powerpc/platforms/powernv/Kconfig
@@ -6,6 +6,7 @@ config PPC_POWERNV
 	select PPC_ICP_NATIVE
 	select PPC_P7_NAP
 	select PPC_PCI_CHOICE if EMBEDDED
+	select GENERIC_CLOCKEVENTS_BROADCAST
 	select EPAPR_BOOT
 	default y
 

^ permalink raw reply related

* [RFC V2 PATCH 2/6] powerpc: Implement broadcast timer interrupt as an IPI message
From: Preeti U Murthy @ 2013-08-14 11:56 UTC (permalink / raw)
  To: fweisbec, paul.gortmaker, paulus, shangw, galak, deepthi, benh,
	paulmck, arnd, linux-pm, rostedt, rjw, john.stultz, tglx,
	chenhui.zhao, michael, r58472, geoff, linux-kernel, srivatsa.bhat,
	schwidefsky, svaidy, linuxppc-dev
In-Reply-To: <20130814115311.5193.32212.stgit@preeti.in.ibm.com>

From: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>

For scalability and performance reasons, we want the broadcast timer
interrupts to be handled as efficiently as possible. Fixed IPI messages
are one of the most efficient mechanisms available - they are faster
than the smp_call_function mechanism because the IPI handlers are fixed
and hence they don't involve costly operations such as adding IPI handlers
to the target CPU's function queue, acquiring locks for synchronization etc.

Luckily we have an unused IPI message slot, so use that to implement
broadcast timer interrupts efficiently.

Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
---

 arch/powerpc/include/asm/smp.h          |    3 ++-
 arch/powerpc/kernel/smp.c               |   19 +++++++++++++++----
 arch/powerpc/platforms/cell/interrupt.c |    2 +-
 arch/powerpc/platforms/ps3/smp.c        |    2 +-
 4 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
index 51bf017..d877b69 100644
--- a/arch/powerpc/include/asm/smp.h
+++ b/arch/powerpc/include/asm/smp.h
@@ -117,7 +117,7 @@ extern int cpu_to_core_id(int cpu);
  *
  * Make sure this matches openpic_request_IPIs in open_pic.c, or what shows up
  * in /proc/interrupts will be wrong!!! --Troy */
-#define PPC_MSG_UNUSED		0
+#define PPC_MSG_TIMER		0
 #define PPC_MSG_RESCHEDULE      1
 #define PPC_MSG_CALL_FUNC_SINGLE	2
 #define PPC_MSG_DEBUGGER_BREAK  3
@@ -190,6 +190,7 @@ extern struct smp_ops_t *smp_ops;
 
 extern void arch_send_call_function_single_ipi(int cpu);
 extern void arch_send_call_function_ipi_mask(const struct cpumask *mask);
+extern void arch_send_tick_broadcast(const struct cpumask *mask);
 
 /* Definitions relative to the secondary CPU spin loop
  * and entry point. Not all of them exist on both 32 and
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index bc41e9f..6a68ca4 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -35,6 +35,7 @@
 #include <asm/ptrace.h>
 #include <linux/atomic.h>
 #include <asm/irq.h>
+#include <asm/hw_irq.h>
 #include <asm/page.h>
 #include <asm/pgtable.h>
 #include <asm/prom.h>
@@ -111,9 +112,9 @@ int smp_generic_kick_cpu(int nr)
 }
 #endif /* CONFIG_PPC64 */
 
-static irqreturn_t unused_action(int irq, void *data)
+static irqreturn_t timer_action(int irq, void *data)
 {
-	/* This slot is unused and hence available for use, if needed */
+	timer_interrupt();
 	return IRQ_HANDLED;
 }
 
@@ -144,14 +145,14 @@ static irqreturn_t debug_ipi_action(int irq, void *data)
 }
 
 static irq_handler_t smp_ipi_action[] = {
-	[PPC_MSG_UNUSED] =  unused_action, /* Slot available for future use */
+	[PPC_MSG_TIMER] =  timer_action,
 	[PPC_MSG_RESCHEDULE] = reschedule_action,
 	[PPC_MSG_CALL_FUNC_SINGLE] = call_function_single_action,
 	[PPC_MSG_DEBUGGER_BREAK] = debug_ipi_action,
 };
 
 const char *smp_ipi_name[] = {
-	[PPC_MSG_UNUSED] =  "ipi unused",
+	[PPC_MSG_TIMER] =  "ipi timer",
 	[PPC_MSG_RESCHEDULE] = "ipi reschedule",
 	[PPC_MSG_CALL_FUNC_SINGLE] = "ipi call function single",
 	[PPC_MSG_DEBUGGER_BREAK] = "ipi debugger",
@@ -221,6 +222,8 @@ irqreturn_t smp_ipi_demux(void)
 		all = xchg(&info->messages, 0);
 
 #ifdef __BIG_ENDIAN
+		if (all & (1 << (24 - 8 * PPC_MSG_TIMER)))
+			timer_interrupt();
 		if (all & (1 << (24 - 8 * PPC_MSG_RESCHEDULE)))
 			scheduler_ipi();
 		if (all & (1 << (24 - 8 * PPC_MSG_CALL_FUNC_SINGLE)))
@@ -266,6 +269,14 @@ void arch_send_call_function_ipi_mask(const struct cpumask *mask)
 		do_message_pass(cpu, PPC_MSG_CALL_FUNC_SINGLE);
 }
 
+void arch_send_tick_broadcast(const struct cpumask *mask)
+{
+	unsigned int cpu;
+
+	for_each_cpu(cpu, mask)
+		do_message_pass(cpu, PPC_MSG_TIMER);
+}
+
 #if defined(CONFIG_DEBUGGER) || defined(CONFIG_KEXEC)
 void smp_send_debugger_break(void)
 {
diff --git a/arch/powerpc/platforms/cell/interrupt.c b/arch/powerpc/platforms/cell/interrupt.c
index 28166e4..1359113 100644
--- a/arch/powerpc/platforms/cell/interrupt.c
+++ b/arch/powerpc/platforms/cell/interrupt.c
@@ -213,7 +213,7 @@ static void iic_request_ipi(int msg)
 
 void iic_request_IPIs(void)
 {
-	iic_request_ipi(PPC_MSG_UNUSED);
+	iic_request_ipi(PPC_MSG_TIMER);
 	iic_request_ipi(PPC_MSG_RESCHEDULE);
 	iic_request_ipi(PPC_MSG_CALL_FUNC_SINGLE);
 	iic_request_ipi(PPC_MSG_DEBUGGER_BREAK);
diff --git a/arch/powerpc/platforms/ps3/smp.c b/arch/powerpc/platforms/ps3/smp.c
index 488f069..5cb742a 100644
--- a/arch/powerpc/platforms/ps3/smp.c
+++ b/arch/powerpc/platforms/ps3/smp.c
@@ -74,7 +74,7 @@ static int __init ps3_smp_probe(void)
 		* to index needs to be setup.
 		*/
 
-		BUILD_BUG_ON(PPC_MSG_UNUSED	      != 0);
+		BUILD_BUG_ON(PPC_MSG_TIMER	      != 0);
 		BUILD_BUG_ON(PPC_MSG_RESCHEDULE       != 1);
 		BUILD_BUG_ON(PPC_MSG_CALL_FUNC_SINGLE != 2);
 		BUILD_BUG_ON(PPC_MSG_DEBUGGER_BREAK   != 3);

^ permalink raw reply related

* [RFC V2 PATCH 1/6] powerpc: Free up the IPI message slot of ipi call function (PPC_MSG_CALL_FUNC)
From: Preeti U Murthy @ 2013-08-14 11:55 UTC (permalink / raw)
  To: fweisbec, paul.gortmaker, paulus, shangw, galak, deepthi, benh,
	paulmck, arnd, linux-pm, rostedt, rjw, john.stultz, tglx,
	chenhui.zhao, michael, r58472, geoff, linux-kernel, srivatsa.bhat,
	schwidefsky, svaidy, linuxppc-dev
In-Reply-To: <20130814115311.5193.32212.stgit@preeti.in.ibm.com>

From: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>

The IPI handlers for both PPC_MSG_CALL_FUNC and PPC_MSG_CALL_FUNC_SINGLE
map to a common implementation - generic_smp_call_function_single_interrupt().
So, we can consolidate them and save one of the IPI message slots, (which are
precious, since only 4 of those slots are available).

So, implement the functionality of PPC_MSG_CALL_FUNC using
PPC_MSG_CALL_FUNC_SINGLE itself and release its IPI message slot, so that it
can be used for something else in the future, if desired.

Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
---

 arch/powerpc/include/asm/smp.h          |    2 +-
 arch/powerpc/kernel/smp.c               |   12 +++++-------
 arch/powerpc/platforms/cell/interrupt.c |    2 +-
 arch/powerpc/platforms/ps3/smp.c        |    2 +-
 4 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
index ffbaabe..51bf017 100644
--- a/arch/powerpc/include/asm/smp.h
+++ b/arch/powerpc/include/asm/smp.h
@@ -117,7 +117,7 @@ extern int cpu_to_core_id(int cpu);
  *
  * Make sure this matches openpic_request_IPIs in open_pic.c, or what shows up
  * in /proc/interrupts will be wrong!!! --Troy */
-#define PPC_MSG_CALL_FUNCTION   0
+#define PPC_MSG_UNUSED		0
 #define PPC_MSG_RESCHEDULE      1
 #define PPC_MSG_CALL_FUNC_SINGLE	2
 #define PPC_MSG_DEBUGGER_BREAK  3
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 38b0ba6..bc41e9f 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -111,9 +111,9 @@ int smp_generic_kick_cpu(int nr)
 }
 #endif /* CONFIG_PPC64 */
 
-static irqreturn_t call_function_action(int irq, void *data)
+static irqreturn_t unused_action(int irq, void *data)
 {
-	generic_smp_call_function_interrupt();
+	/* This slot is unused and hence available for use, if needed */
 	return IRQ_HANDLED;
 }
 
@@ -144,14 +144,14 @@ static irqreturn_t debug_ipi_action(int irq, void *data)
 }
 
 static irq_handler_t smp_ipi_action[] = {
-	[PPC_MSG_CALL_FUNCTION] =  call_function_action,
+	[PPC_MSG_UNUSED] =  unused_action, /* Slot available for future use */
 	[PPC_MSG_RESCHEDULE] = reschedule_action,
 	[PPC_MSG_CALL_FUNC_SINGLE] = call_function_single_action,
 	[PPC_MSG_DEBUGGER_BREAK] = debug_ipi_action,
 };
 
 const char *smp_ipi_name[] = {
-	[PPC_MSG_CALL_FUNCTION] =  "ipi call function",
+	[PPC_MSG_UNUSED] =  "ipi unused",
 	[PPC_MSG_RESCHEDULE] = "ipi reschedule",
 	[PPC_MSG_CALL_FUNC_SINGLE] = "ipi call function single",
 	[PPC_MSG_DEBUGGER_BREAK] = "ipi debugger",
@@ -221,8 +221,6 @@ irqreturn_t smp_ipi_demux(void)
 		all = xchg(&info->messages, 0);
 
 #ifdef __BIG_ENDIAN
-		if (all & (1 << (24 - 8 * PPC_MSG_CALL_FUNCTION)))
-			generic_smp_call_function_interrupt();
 		if (all & (1 << (24 - 8 * PPC_MSG_RESCHEDULE)))
 			scheduler_ipi();
 		if (all & (1 << (24 - 8 * PPC_MSG_CALL_FUNC_SINGLE)))
@@ -265,7 +263,7 @@ void arch_send_call_function_ipi_mask(const struct cpumask *mask)
 	unsigned int cpu;
 
 	for_each_cpu(cpu, mask)
-		do_message_pass(cpu, PPC_MSG_CALL_FUNCTION);
+		do_message_pass(cpu, PPC_MSG_CALL_FUNC_SINGLE);
 }
 
 #if defined(CONFIG_DEBUGGER) || defined(CONFIG_KEXEC)
diff --git a/arch/powerpc/platforms/cell/interrupt.c b/arch/powerpc/platforms/cell/interrupt.c
index 2d42f3b..28166e4 100644
--- a/arch/powerpc/platforms/cell/interrupt.c
+++ b/arch/powerpc/platforms/cell/interrupt.c
@@ -213,7 +213,7 @@ static void iic_request_ipi(int msg)
 
 void iic_request_IPIs(void)
 {
-	iic_request_ipi(PPC_MSG_CALL_FUNCTION);
+	iic_request_ipi(PPC_MSG_UNUSED);
 	iic_request_ipi(PPC_MSG_RESCHEDULE);
 	iic_request_ipi(PPC_MSG_CALL_FUNC_SINGLE);
 	iic_request_ipi(PPC_MSG_DEBUGGER_BREAK);
diff --git a/arch/powerpc/platforms/ps3/smp.c b/arch/powerpc/platforms/ps3/smp.c
index 4b35166..488f069 100644
--- a/arch/powerpc/platforms/ps3/smp.c
+++ b/arch/powerpc/platforms/ps3/smp.c
@@ -74,7 +74,7 @@ static int __init ps3_smp_probe(void)
 		* to index needs to be setup.
 		*/
 
-		BUILD_BUG_ON(PPC_MSG_CALL_FUNCTION    != 0);
+		BUILD_BUG_ON(PPC_MSG_UNUSED	      != 0);
 		BUILD_BUG_ON(PPC_MSG_RESCHEDULE       != 1);
 		BUILD_BUG_ON(PPC_MSG_CALL_FUNC_SINGLE != 2);
 		BUILD_BUG_ON(PPC_MSG_DEBUGGER_BREAK   != 3);

^ permalink raw reply related

* [RFC V2 PATCH 0/6] cpuidle/ppc: Timer offload framework to support deep idle states
From: Preeti U Murthy @ 2013-08-14 11:55 UTC (permalink / raw)
  To: fweisbec, paul.gortmaker, paulus, shangw, galak, deepthi, benh,
	paulmck, arnd, linux-pm, rostedt, rjw, john.stultz, tglx,
	chenhui.zhao, michael, r58472, geoff, linux-kernel, srivatsa.bhat,
	schwidefsky, svaidy, linuxppc-dev

On PowerPC, when CPUs enter deep idle states, their local timers are
switched off. The responsibility of waking them up at their next timer event,
needs to be handed over to an external device. On PowerPC, we do not have an
external device equivalent to HPET, which is currently done on architectures
like x86. Instead we assign the local timer of one of the CPUs to do this job.

This patchset is an attempt to make use of the existing timer broadcast
framework in the kernel to meet the above requirement, except that the tick
broadcast device is the local timer of the boot CPU.

This patch series is ported ontop of 3.11-rc1 + the cpuidle driver backend
for powernv posted by Deepthi Dharwar recently. NOHZ_FULL is disabled for all
testing purposes.
  The current design and implementation supports the ONESHOT tick mode.
It does not yet support the PERIODIC tick mode.

The discussion around V1 of this patchset can be found here:
https://lkml.org/lkml/2013/7/25/740.

Changes since V1:
1. Dynamically pick a broadcast CPU, instead of having a dedicated one.
2. Remove the constraint of having to disable tickless idle on the broadcast CPU.

Thanks to Ben H, Frederic Weisbecker, Li Yang and Vaidyanathan Srinivasan for
all their comments and suggestions on the V1 of this patchset.

Patch[1/6], Patch[2/6]: optimize the broadcast mechanism on ppc.
Patch[3/6]: Introduces the core of the timer offload framework on powerpc.
Patch[4/6]: Add a deep idle state to the cpuidle state table on powernv
Patch[5/6]: Dynamically pick a broadcast CPU
Patch[6/6]: Remove the constraint of having to disable tickless idle on the
broadcast cpu, by queueing a hrtimer exclusively to do broadcast handling.

---

Preeti U Murthy (4):
      cpuidle/ppc: Add timer offload framework to support deep idle states
      cpuidle/ppc: Add longnap state to the idle states on powernv
      cpuidle/ppc: Enable dynamic movement of the broadcast functionality across CPUs
      cpuidle/ppc : Queue a hrtimer on bc_cpu, explicitly to do broadcast handling

Srivatsa S. Bhat (2):
      powerpc: Free up the IPI message slot of ipi call function (PPC_MSG_CALL_FUNC)
      powerpc: Implement broadcast timer interrupt as an IPI message


 arch/powerpc/include/asm/smp.h                  |    3 -
 arch/powerpc/include/asm/time.h                 |    9 ++
 arch/powerpc/kernel/smp.c                       |   23 +++--
 arch/powerpc/kernel/time.c                      |  114 +++++++++++++++++++++++
 arch/powerpc/platforms/cell/interrupt.c         |    2 
 arch/powerpc/platforms/powernv/Kconfig          |    1 
 arch/powerpc/platforms/powernv/processor_idle.c |  104 +++++++++++++++++++++
 arch/powerpc/platforms/ps3/smp.c                |    2 
 8 files changed, 246 insertions(+), 12 deletions(-)

^ permalink raw reply

* Re: [GIT PULL] DT/core: cpu_ofnode updates for v3.12
From: Benjamin Herrenschmidt @ 2013-08-14 11:37 UTC (permalink / raw)
  To: Sudeep KarkadaNagesha
  Cc: devicetree@vger.kernel.org, linux-pm@vger.kernel.org,
	Viresh Kumar, linux-kernel@vger.kernel.org,
	rob.herring@calxeda.com, Rafael J. Wysocki, Olof Johansson,
	linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <520B5584.7030608@arm.com>

On Wed, 2013-08-14 at 11:01 +0100, Sudeep KarkadaNagesha wrote:
> Yes this doesn't cover the historical "ibm,ppc-interrupt-server#s",
> for
> which we can have PPC specific wrapper above the generic one i.e. get
> the cpu node and then parse for thread id under custom property.

A wrapper is wrong. I don't want to have to have all ppc callers to use
a different function.

As I said, just make a generic one that returns a thread ID, ie, same
signature as the powerpc one. Make it weak, we can override it in
powerpc-land, or we can move the ibm,ppc-interrupt-server#s handling
into the generic one, it won't hurt, but leave the thread_id return
there, it doesn't hurt it will come in handy in a few cases without
causing code duplication.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH] crypto: nx - fix concurrency issue
From: Herbert Xu @ 2013-08-14 10:49 UTC (permalink / raw)
  To: Marcelo Cerri; +Cc: linuxppc-dev, linux-kernel, linux-crypto
In-Reply-To: <1376344177-7205-1-git-send-email-mhcerri@linux.vnet.ibm.com>

On Mon, Aug 12, 2013 at 06:49:37PM -0300, Marcelo Cerri wrote:
> The NX driver uses the transformation context to store several fields
> containing data related to the state of the operations in progress.
> Since a single tfm can be used by different kernel threads at the same
> time, we need to protect the data stored into the context.
> 
> This patch makes use of spin locks to protect the data where a race
> condition can happen.
> 
> Reviewed-by: Fionnuala Gunter <fin@linux.vnet.ibm.com>
> Reviewed-by: Joy Latten <jmlatten@linux.vnet.ibm.com>
> Signed-off-by: Marcelo Cerri <mhcerri@linux.vnet.ibm.com>

Patch applied.  Thanks a lot!
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [alsa-devel] [PATCH v4 resent 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
From: Nicolin Chen @ 2013-08-14 10:23 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: devicetree, alsa-devel, lars, timur, rob.herring, broonie,
	p.zabel, shawn.guo, linuxppc-dev
In-Reply-To: <20130814095652.GX26614@pengutronix.de>

Hi,

On Wed, Aug 14, 2013 at 11:56:52AM +0200, Sascha Hauer wrote:
> > I think I should first explain to you what this part is doing:
> > The driver needs to set Clk_source bit for TX/RX to select the 
> > clock from a clock mux. The names listed above are those of the 
> > clocks connecting to the mux, while they are not only internal 
> > clocks which're included in clk-imx6q.c but also external ones,
> > an on-board external osc for example.
> > 
> > The driver does get the clock by using the standard DT binding,
> > see the 'clocks = <&clks 197>' above, and then compare this
> > obtained clock->name with the name list to decide which value
> > should be set to the Clk_source bit.
> > 
> > ==================================================================
> > ClkSrc_Sel from i.MX6Q reference manual:
> > 
> > Clock source selection, all other settings not shown are reserved:
> > 0000 if (DPLL Locked) SPDIF_RxClk else extal
> > 0001 if (DPLL Locked) SPDIF_RxClk else spdif_clk
> > 0010 if (DPLL Locked) SPDIF_RxClk else asrc_clk
> > 0011 if (DPLL Locked) SPDIF_RxClk else spdif_extclk
> > 0100 if (DPLL Locked) SPDIF_Rxclk else esai_hckt
> > 0101 extal_clk
> > 0110 spdif_clk
> > 0111 asrc_clk
> > 1000 spdif_extclk
> > 1001 esai_hckt
> > 1010 if (DPLL Locked) SPDIF_RxClk else mlb_clk
> > ==================================================================
> > 
> > So the name list here basically is not being used to obtain a
> > clock like what standardized DT binding does but to provide the
> > driver a full list to look up which value should be exactly used
> > according to the obtained clock.
> > 
> > I think I should revise the binding doc for these two lists. It 
> > might be hard to explain within that kinda short paragraph. 
> > 
> > Surely, if I misunderstand your point, please correct me. And
> > if you have any sage idea, please guide me.
> 
> Something like this:
> 
> clocks = <&clks 197>, <&clks 3>, <&clks 197>, <&clks 107>, <&clks SPDIF_EXT>,
>          <&clks 118>, <&clks 62>, <&clks 139>, <&clks MLB_PHY>
> clock-names = "core", "rxtx0", "rxtx1", "rxtx2", "rxtx3", "rxtx4", "rxtx5", "rxtx6", "rxtx7"
> 
> This describes the different input clocks to the spdif core and also
> gives a hint to the array index (rxtx_n_) to use.

Thank you for the idea, and..hmm..I'm a bit confused.. Is this really
a nicer way?

Actually the rx clock list and tx clock list are totally different.
So doing this I have to list, in the maximum case, 24 (8 + 16) clock
phandles for these two lists. And plussing another 6 I've listed in
this binding doc -- thus there are totally 30 clock phanldes. But
the 24 of 30 are only used to get two indexes.

I think I need a little help here to understand why this is better.
It looks more complicated to me.

Creating the two name lists just because I can describe them in the 
dtsi of SoC, since they are totally fixed and identical to the SoC
reference manual, while the clocks area can be remained for users
to select the actual clocks (core/rx/tx/tx-32000/tx-44100/tx-48000),
so they can configure these clocks in dts file of a specific board,
and they don't need to touch the name lists any more.

Thank you,
Nicolin Chen

^ permalink raw reply

* Re: [GIT PULL] DT/core: cpu_ofnode updates for v3.12
From: Sudeep KarkadaNagesha @ 2013-08-14 10:01 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: devicetree@vger.kernel.org, linux-pm@vger.kernel.org,
	Sudeep KarkadaNagesha, Viresh Kumar, linux-kernel@vger.kernel.org,
	rob.herring@calxeda.com, Rafael J. Wysocki, Olof Johansson,
	linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <1376428024.4255.14.camel@pasglop>

On 13/08/13 22:07, Benjamin Herrenschmidt wrote:
> On Tue, 2013-08-13 at 19:29 +0100, Sudeep KarkadaNagesha wrote:
>> I don't understand completely the use of ibm,ppc-interrupt-server#s and
>> its implications on generic of_get_cpu_node implementation.
>> I see the PPC specific definition of of_get_cpu_node uses thread id only
>> in 2 instances. Based on that, I have tried to move all the other
>> instances to use generic definition.
>>
>> Let me know if the idea is correct.
>=20
> No. The device-tree historically only represents cores, not HW threads, s=
o
> it makes sense to retrieve also the thread number corresponding to the CP=
U.
>=20
Ok

> However, the mechanism to represent HW threads in the device-tree is curr=
ently
> somewhat platform specific (the ibm,ppc-interrupt-server#s).
I see most of the callers pass NULL to thread id argument except 2
instances in entire tree. If that's the case why can't we move to use
generic of_get_cpu_node in most of those cases and have PPC specific
implementation for the ones using thread id.

>=20
> So what you could do for now is:
>=20
>  - Have a generic version that always returns 0 as the thread, which is w=
eak
I would prefer to move to generic of_get_cpu_node where ever possible
and rename the function that takes thread id rather than making generic
one weak.

>=20
>  - powerpc keeps its own implementation
How about only in cases where it needs thread_id.

>=20
>  - Start a discussion on the bindings (if not already there) to define th=
reads
> in a better way at which point the generic function can be updated.
>=20
I am not sure if we need to define any new bindings. Excerpts from ePAPR
(v1.1):
"3.7.1 General Properties of CPU nodes
The value of "reg" is a <prop-encoded-array> that defines a unique
CPU/thread id for the CPU/threads represented by the CPU node.
If a CPU supports more than one thread (i.e. multiple streams of
execution) the reg property is an array with 1 element per thread. The
#address-cells on the /cpus node specifies how many cells each element
of the array takes. Software can determine the number of threads by
dividing the size of reg by the parent node's #address-cells."

And this is exactly in agreement to what's implemented in the generic
of_get_cpu_node:

for_each_child_of_node(cpus, cpun) {
        if (of_node_cmp(cpun->type, "cpu"))
                continue;
        cell =3D of_get_property(cpun, "reg", &prop_len);
        if (!cell) {
                pr_warn("%s: missing reg property\n", cpun->full_name);
                continue;
        }
        prop_len /=3D sizeof(*cell);
        while (prop_len) {
                hwid =3D of_read_number(cell, ac);
                prop_len -=3D ac;
                if (arch_match_cpu_phys_id(cpu, hwid))
                        return cpun;
        }
}

Yes this doesn't cover the historical "ibm,ppc-interrupt-server#s", for
which we can have PPC specific wrapper above the generic one i.e. get
the cpu node and then parse for thread id under custom property.

Let me know your thoughts.

Regards,
Sudeep

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox