* RE: [PATCH 5/9] dmaengine: provide a common function for completing a dma descriptor
From: Boojin Kim @ 2012-04-23 11:06 UTC (permalink / raw)
To: 'Vinod Koul', 'Russell King - ARM Linux'
Cc: 'Stephen Warren', 'Linus Walleij',
'Srinidhi Kasagar', 'Dan Williams', linuxppc-dev,
linux-arm-kernel
In-Reply-To: <1335175278.31825.108.camel@vkoul-udesk3>
Vinod Koul wrote:
> Sent: Monday, April 23, 2012 7:01 PM
> To: Russell King - ARM Linux
> Cc: 'Stephen Warren'; 'Linus Walleij'; 'Srinidhi Kasagar'; Boojin Kim; 'Dan Williams'; 'Li Yang';
> linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH 5/9] dmaengine: provide a common function for completing a dma descriptor
>
> On Mon, 2012-04-23 at 10:50 +0100, Russell King - ARM Linux wrote:
> > On Mon, Apr 23, 2012 at 06:40:06PM +0900, Boojin Kim wrote:
> > > I met a problem on DMA cyclic mode (DMA_CYCLIC) for sound playback.
> > > Kernel BUG occurs during DMA transfer with DMA cyclic mode.
> > > This patch makes the cookies into zero. But, cookies should be kept
> > > during cyclic mode because cyclic mode re-uses the cookies.
> >
> > The protection is there to prevent cookies being accidentally re-used.
> > If you're running a cyclic transfer, even then you shouldn't be completing
> > the same cookie time and time again - I think Vinod also concurs with this.
> Right :)
> I recently committed patch for imx-dma which doesn't mark the cyclic
> descriptor as complete. Descriptor represents a transaction and makes no
> sense to complete t if the transaction is still continuing.
Dear Vinod,
you already fixed it. :) thanks.
And I have other question. (Actually, It doesn't relate to this patch.)
I met the DMA probing fail problem on Linux 3.4.
It's because the return value on regulator_get() is changed
from ENODEV to EPROBE_DEFER in case not to supply a vcore regulator.
So, I try to change the check value about the return value of regulator_get()
in amba_get_enable_vcore()from ENODEV to EPROBE_DEFER.
How about it ? Do you already fix it too?
Thanks,
Boojin
> >
> > I think our preference is for cyclic transfers to entire remain uncompleted,
> > or to get a new cookie each time they allegedly "complete".
> No it is not complete. Cyclic never completes, it aborts when user
> wants. The "notification" interrupt is for updating the
> counters/notifying (timestamp/periods elapsed in sound), and shouldn't
> be used for anything else
>
> --
> ~Vinod
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 5/9] dmaengine: provide a common function for completing a dma descriptor
From: Vinod Koul @ 2012-04-23 10:01 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: 'Stephen Warren', 'Linus Walleij',
'Srinidhi Kasagar', Boojin Kim, 'Dan Williams',
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20120423095013.GL24211@n2100.arm.linux.org.uk>
On Mon, 2012-04-23 at 10:50 +0100, Russell King - ARM Linux wrote:
> On Mon, Apr 23, 2012 at 06:40:06PM +0900, Boojin Kim wrote:
> > I met a problem on DMA cyclic mode (DMA_CYCLIC) for sound playback.
> > Kernel BUG occurs during DMA transfer with DMA cyclic mode.
> > This patch makes the cookies into zero. But, cookies should be kept
> > during cyclic mode because cyclic mode re-uses the cookies.
>
> The protection is there to prevent cookies being accidentally re-used.
> If you're running a cyclic transfer, even then you shouldn't be completing
> the same cookie time and time again - I think Vinod also concurs with this.
Right :)
I recently committed patch for imx-dma which doesn't mark the cyclic
descriptor as complete. Descriptor represents a transaction and makes no
sense to complete t if the transaction is still continuing.
>
> I think our preference is for cyclic transfers to entire remain uncompleted,
> or to get a new cookie each time they allegedly "complete".
No it is not complete. Cyclic never completes, it aborts when user
wants. The "notification" interrupt is for updating the
counters/notifying (timestamp/periods elapsed in sound), and shouldn't
be used for anything else
--
~Vinod
^ permalink raw reply
* Re: PowerPC radeon KMS - is it possible?
From: Michel Dänzer @ 2012-04-23 9:56 UTC (permalink / raw)
To: Gerhard Pircher; +Cc: linuxppc-dev
In-Reply-To: <20120420161407.190300@gmx.net>
On Fre, 2012-04-20 at 18:14 +0200, Gerhard Pircher wrote:=20
> > Von: "Michel D=C3=A4nzer" <michel@daenzer.net>
> > On Fre, 2012-04-20 at 13:15 +0200, Gerhard Pircher wrote:=20
> > > > Von: "Michel D=C3=A4nzer" <michel@daenzer.net>
> > > > On Don, 2012-04-19 at 13:48 +0200, Gerhard Pircher wrote:=20
> > > > >=20
> > > > > The "former case" is an explanation, why I see data corruption
> > > > > with my< AGPGART driver (more or less a copy of the uninorth
> > > > > driver) on my non-coherent platform. There are no cache flushes
> > > > > done for writes to already mapped pages.
> > > >=20
> > > > As I said, the radeon driver always maps AGP memory uncacheable for
> > > > the CPU, so no such CPU cache flushes should be necessary.
> > > I know. We also discussed this topic over two years ago. :-)
> > >=20
> > > What I didn't understand yet is how this uncacheable memory is
> > > allocated (well, I never took the time to look at this again). The
> > > functions in ttm_page_alloc.c seem to allocate normal cacheable
> > > memory and try to set the page flags with set_pages_array_uc(),
> > > which is more or less a no-op on powerpc. ttm_page_alloc_dma.c on
> > > the other side is only used with SWIOTLB!?
> > [...]=20
> > > Could it be that the memory is finally mapped uncacheable by
> > radeon_bo_kmap()/
> > > ttm_bo_kmap()/..some other TTM functions../vmap()?
> >=20
> > Yeah, AFAICT, basically ttm_io_prot() defines the mapping attributes,
> > and vmap() is used to enforce them for kernel mappings.
> Okay, that sounds like the approach used by arch/powerpc/mm/dma-
> noncoherent.c in my ("green") ears. What about the PCIGART mode?
> Is the driver free to use cached memory in this mode?
Yes, it assumes PCI(e) GART to be CPU cache coherent.
> > > [ 5.878809] [drm:radeon_test_moves] *ERROR* Incorrect VRAM->GTT co=
py 0: Got 0xf1416ec0, expected 0xf1570ec0 (VRAM map 0xf1480000-0xf1580000)
> > [...]
> > > The VRAM->GTT copy totally puzzles me, as it returns a wrong page
> > > address, but the offset is fine!?
> >=20
> > Maybe it's still the values from the GTT->VRAM test, i.e. either the GP=
U
> > writes didn't make it to the memory mapped into the AGP GART (some AGP
> > bridges are known to have issues with that) or the CPU doesn't see it.
> What is the workaround for such an AGP bridge? If there is one at all...
The only workaround short of not using AGP would be not doing GPU->AGP
transfers, but that's not implemented or even planned at all.
> > BTW, does your driver set cant_use_aperture, or is the linear aperture
> > accessible by the CPU?
> The driver sets cant_use_aperture. I couldn't get it working at all
> without it.
Does the hardware really not allow the CPU to access the linear aperture
though? Because if it does, I wonder if that might be more reliable.
--=20
Earthling Michel D=C3=A4nzer | http://www.amd.c=
om
Libre software enthusiast | Debian, X and DRI developer
^ permalink raw reply
* RE: [PATCH 5/9] dmaengine: provide a common function for completing a dma descriptor
From: Boojin Kim @ 2012-04-23 9:40 UTC (permalink / raw)
To: 'Russell King - ARM Linux', 'Dan Williams',
'Vinod Koul'
Cc: 'Stephen Warren', 'Linus Walleij',
'Srinidhi Kasagar', linuxppc-dev, linux-arm-kernel
In-Reply-To: <E1S52yJ-0002Mx-2e@rmk-PC.arm.linux.org.uk>
Russell King - ARM Linux wrote:
> Sent: Wednesday, March 07, 2012 7:35 AM
> To: Dan Williams; Vinod Koul
> Cc: Stephen Warren; Linus Walleij; Srinidhi Kasagar; Li Yang; linuxppc-dev@lists.ozlabs.org;
> linux-arm-kernel@lists.infradead.org
> Subject: [PATCH 5/9] dmaengine: provide a common function for completing a dma descriptor
>
> Provide a common function to do the cookie mechanics for completing
> a DMA descriptor.
Dear Russell,
I met a problem on DMA cyclic mode (DMA_CYCLIC) for sound playback.
Kernel BUG occurs during DMA transfer with DMA cyclic mode.
This patch makes the cookies into zero. But, cookies should be kept during cyclic mode
because cyclic mode re-uses the cookies.
So, Error occurs on "BUG_ON(tx->cookie < DMA_MIN_COOKIE)" lines on dma_cookie_complete().
Please see following error.
------------[ cut here ]------------
kernel BUG at drivers/dma/dmaengine.h:53!
Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 0 Tainted: G W (3.4.0-rc2-00596-gc7d7a63 #5)
PC is at pl330_tasklet+0x58c/0x59c
LR is at _raw_spin_lock_irqsave+0x10/0x14
pc : [<c0238a84>] lr : [<c052a3f4>] psr: 68000193
sp : c06efde0 ip : 00000000 fp : c06efe4c
r10: 00000000 r9 : 00000000 r8 : c06efe18
r7 : d881b414 r6 : d881b410 r5 : d881b450 r4 : 00000003
r3 : d8814bcc r2 : d8814b60 r1 : 00000000 r0 : d8814b60
Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 10c5387d Table: 57eb806a DAC: 00000015
I think the completing a dma descriptor without setting zero to cookies is required for cyclic mode.
Do I make new macro or modify dma_cookie_complete() for it?
Thanks
Boojin.
>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
> drivers/dma/amba-pl08x.c | 2 +-
> drivers/dma/at_hdmac.c | 2 +-
> drivers/dma/coh901318.c | 2 +-
> drivers/dma/dmaengine.h | 18 ++++++++++++++++++
> drivers/dma/dw_dmac.c | 2 +-
> drivers/dma/ep93xx_dma.c | 2 +-
> drivers/dma/fsldma.c | 2 +-
> drivers/dma/imx-dma.c | 2 +-
> drivers/dma/imx-sdma.c | 2 +-
> drivers/dma/intel_mid_dma.c | 2 +-
> drivers/dma/ioat/dma.c | 3 +--
> drivers/dma/ioat/dma_v2.c | 3 +--
> drivers/dma/ioat/dma_v3.c | 3 +--
> drivers/dma/ipu/ipu_idmac.c | 2 +-
> drivers/dma/mxs-dma.c | 2 +-
> drivers/dma/pl330.c | 2 +-
> drivers/dma/ste_dma40.c | 2 +-
> drivers/dma/timb_dma.c | 2 +-
> drivers/dma/txx9dmac.c | 2 +-
> 19 files changed, 36 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
> index 5996386..f0888c1 100644
> --- a/drivers/dma/amba-pl08x.c
> +++ b/drivers/dma/amba-pl08x.c
> @@ -1540,7 +1540,7 @@ static void pl08x_tasklet(unsigned long data)
>
> if (txd) {
> /* Update last completed */
> - plchan->chan.completed_cookie = txd->tx.cookie;
> + dma_cookie_complete(&txd->tx);
> }
>
> /* If a new descriptor is queued, set it up plchan->at is NULL here */
> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> index df47e7d..b282630 100644
> --- a/drivers/dma/at_hdmac.c
> +++ b/drivers/dma/at_hdmac.c
> @@ -249,7 +249,7 @@ atc_chain_complete(struct at_dma_chan *atchan, struct at_desc *desc)
> dev_vdbg(chan2dev(&atchan->chan_common),
> "descriptor %u complete\n", txd->cookie);
>
> - atchan->chan_common.completed_cookie = txd->cookie;
> + dma_cookie_complete(txd);
>
> /* move children to free_list */
> list_splice_init(&desc->tx_list, &atchan->free_list);
> diff --git a/drivers/dma/coh901318.c b/drivers/dma/coh901318.c
> index 843a1a3..24837d7 100644
> --- a/drivers/dma/coh901318.c
> +++ b/drivers/dma/coh901318.c
> @@ -691,7 +691,7 @@ static void dma_tasklet(unsigned long data)
> callback_param = cohd_fin->desc.callback_param;
>
> /* sign this job as completed on the channel */
> - cohc->chan.completed_cookie = cohd_fin->desc.cookie;
> + dma_cookie_complete(&cohd_fin->desc);
>
> /* release the lli allocation and remove the descriptor */
> coh901318_lli_free(&cohc->base->pool, &cohd_fin->lli);
> diff --git a/drivers/dma/dmaengine.h b/drivers/dma/dmaengine.h
> index 7692c86..47e0997 100644
> --- a/drivers/dma/dmaengine.h
> +++ b/drivers/dma/dmaengine.h
> @@ -5,6 +5,7 @@
> #ifndef DMAENGINE_H
> #define DMAENGINE_H
>
> +#include <linux/bug.h>
> #include <linux/dmaengine.h>
>
> /**
> @@ -27,4 +28,21 @@ static inline dma_cookie_t dma_cookie_assign(struct dma_async_tx_descriptor
> *tx)
> return cookie;
> }
>
> +/**
> + * dma_cookie_complete - complete a descriptor
> + * @tx: descriptor to complete
> + *
> + * Mark this descriptor complete by updating the channels completed
> + * cookie marker. Zero the descriptors cookie to prevent accidental
> + * repeated completions.
> + *
> + * Note: caller is expected to hold a lock to prevent concurrency.
> + */
> +static inline void dma_cookie_complete(struct dma_async_tx_descriptor *tx)
> +{
> + BUG_ON(tx->cookie < DMA_MIN_COOKIE);
> + tx->chan->completed_cookie = tx->cookie;
> + tx->cookie = 0;
> +}
> +
> #endif
> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> index 5ee9498..a190c88 100644
> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c
> @@ -231,7 +231,7 @@ dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc,
> dev_vdbg(chan2dev(&dwc->chan), "descriptor %u complete\n", txd->cookie);
>
> spin_lock_irqsave(&dwc->lock, flags);
> - dwc->chan.completed_cookie = txd->cookie;
> + dma_cookie_complete(txd);
> if (callback_required) {
> callback = txd->callback;
> param = txd->callback_param;
> diff --git a/drivers/dma/ep93xx_dma.c b/drivers/dma/ep93xx_dma.c
> index e5aaae8..1c56f75 100644
> --- a/drivers/dma/ep93xx_dma.c
> +++ b/drivers/dma/ep93xx_dma.c
> @@ -703,7 +703,7 @@ static void ep93xx_dma_tasklet(unsigned long data)
> desc = ep93xx_dma_get_active(edmac);
> if (desc) {
> if (desc->complete) {
> - edmac->chan.completed_cookie = desc->txd.cookie;
> + dma_cookie_complete(&desc->txd);
> list_splice_init(&edmac->active, &list);
> }
> callback = desc->txd.callback;
> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 04b4347..f36e8b1 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -1081,8 +1081,8 @@ static void dma_do_tasklet(unsigned long data)
>
> desc = to_fsl_desc(chan->ld_running.prev);
> cookie = desc->async_tx.cookie;
> + dma_cookie_complete(&desc->async_tx);
>
> - chan->common.completed_cookie = cookie;
> chan_dbg(chan, "completed_cookie=%d\n", cookie);
> }
>
> diff --git a/drivers/dma/imx-dma.c b/drivers/dma/imx-dma.c
> index 42154b6..f1226ad 100644
> --- a/drivers/dma/imx-dma.c
> +++ b/drivers/dma/imx-dma.c
> @@ -66,7 +66,7 @@ static void imxdma_handle(struct imxdma_channel *imxdmac)
> {
> if (imxdmac->desc.callback)
> imxdmac->desc.callback(imxdmac->desc.callback_param);
> - imxdmac->chan.completed_cookie = imxdmac->desc.cookie;
> + dma_cookie_complete(&imxdmac->desc);
> }
>
> static void imxdma_irq_handler(int channel, void *data)
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 4e4f40e..4406be2 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -524,7 +524,7 @@ static void mxc_sdma_handle_channel_normal(struct sdma_channel *sdmac)
> else
> sdmac->status = DMA_SUCCESS;
>
> - sdmac->chan.completed_cookie = sdmac->desc.cookie;
> + dma_cookie_complete(&sdmac->desc);
> if (sdmac->desc.callback)
> sdmac->desc.callback(sdmac->desc.callback_param);
> }
> diff --git a/drivers/dma/intel_mid_dma.c b/drivers/dma/intel_mid_dma.c
> index dfe4396..340509e 100644
> --- a/drivers/dma/intel_mid_dma.c
> +++ b/drivers/dma/intel_mid_dma.c
> @@ -290,7 +290,7 @@ static void midc_descriptor_complete(struct intel_mid_dma_chan *midc,
> struct intel_mid_dma_lli *llitem;
> void *param_txd = NULL;
>
> - midc->chan.completed_cookie = txd->cookie;
> + dma_cookie_complete(txd);
> callback_txd = txd->callback;
> param_txd = txd->callback_param;
>
> diff --git a/drivers/dma/ioat/dma.c b/drivers/dma/ioat/dma.c
> index 5c06117..b0517c8 100644
> --- a/drivers/dma/ioat/dma.c
> +++ b/drivers/dma/ioat/dma.c
> @@ -600,8 +600,7 @@ static void __cleanup(struct ioat_dma_chan *ioat, unsigned long phys_complete)
> */
> dump_desc_dbg(ioat, desc);
> if (tx->cookie) {
> - chan->common.completed_cookie = tx->cookie;
> - tx->cookie = 0;
> + dma_cookie_complete(tx);
> ioat_dma_unmap(chan, tx->flags, desc->len, desc->hw);
> ioat->active -= desc->hw->tx_cnt;
> if (tx->callback) {
> diff --git a/drivers/dma/ioat/dma_v2.c b/drivers/dma/ioat/dma_v2.c
> index 17ecacb..e8e110f 100644
> --- a/drivers/dma/ioat/dma_v2.c
> +++ b/drivers/dma/ioat/dma_v2.c
> @@ -149,8 +149,7 @@ static void __cleanup(struct ioat2_dma_chan *ioat, unsigned long
> phys_complete)
> dump_desc_dbg(ioat, desc);
> if (tx->cookie) {
> ioat_dma_unmap(chan, tx->flags, desc->len, desc->hw);
> - chan->common.completed_cookie = tx->cookie;
> - tx->cookie = 0;
> + dma_cookie_complete(tx);
> if (tx->callback) {
> tx->callback(tx->callback_param);
> tx->callback = NULL;
> diff --git a/drivers/dma/ioat/dma_v3.c b/drivers/dma/ioat/dma_v3.c
> index d4afac7..1bda46c 100644
> --- a/drivers/dma/ioat/dma_v3.c
> +++ b/drivers/dma/ioat/dma_v3.c
> @@ -277,9 +277,8 @@ static void __cleanup(struct ioat2_dma_chan *ioat, unsigned long
> phys_complete)
> dump_desc_dbg(ioat, desc);
> tx = &desc->txd;
> if (tx->cookie) {
> - chan->common.completed_cookie = tx->cookie;
> + dma_cookie_complete(tx);
> ioat3_dma_unmap(ioat, desc, idx + i);
> - tx->cookie = 0;
> if (tx->callback) {
> tx->callback(tx->callback_param);
> tx->callback = NULL;
> diff --git a/drivers/dma/ipu/ipu_idmac.c b/drivers/dma/ipu/ipu_idmac.c
> index d4620c5..bff9250 100644
> --- a/drivers/dma/ipu/ipu_idmac.c
> +++ b/drivers/dma/ipu/ipu_idmac.c
> @@ -1289,7 +1289,7 @@ static irqreturn_t idmac_interrupt(int irq, void *dev_id)
> /* Flip the active buffer - even if update above failed */
> ichan->active_buffer = !ichan->active_buffer;
> if (done)
> - ichan->dma_chan.completed_cookie = desc->txd.cookie;
> + dma_cookie_complete(&desc->txd);
>
> callback = desc->txd.callback;
> callback_param = desc->txd.callback_param;
> diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> index 4d3b6ff..5f3492e 100644
> --- a/drivers/dma/mxs-dma.c
> +++ b/drivers/dma/mxs-dma.c
> @@ -262,7 +262,7 @@ static irqreturn_t mxs_dma_int_handler(int irq, void *dev_id)
> stat1 &= ~(1 << channel);
>
> if (mxs_chan->status == DMA_SUCCESS)
> - mxs_chan->chan.completed_cookie = mxs_chan->desc.cookie;
> + dma_cookie_complete(&mxs_chan->desc);
>
> /* schedule tasklet on this channel */
> tasklet_schedule(&mxs_chan->tasklet);
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index ec9638b..76871b8 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -233,7 +233,7 @@ static void pl330_tasklet(unsigned long data)
> /* Pick up ripe tomatoes */
> list_for_each_entry_safe(desc, _dt, &pch->work_list, node)
> if (desc->status == DONE) {
> - pch->chan.completed_cookie = desc->txd.cookie;
> + dma_cookie_complete(&desc->txd);
> list_move_tail(&desc->node, &list);
> }
>
> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index 23e2edc..c246375 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -1347,7 +1347,7 @@ static void dma_tasklet(unsigned long data)
> goto err;
>
> if (!d40d->cyclic)
> - d40c->chan.completed_cookie = d40d->txd.cookie;
> + dma_cookie_complete(&d40d->txd);
>
> /*
> * If terminating a channel pending_tx is set to zero.
> diff --git a/drivers/dma/timb_dma.c b/drivers/dma/timb_dma.c
> index b6e83fc..1845ac9 100644
> --- a/drivers/dma/timb_dma.c
> +++ b/drivers/dma/timb_dma.c
> @@ -285,7 +285,7 @@ static void __td_finish(struct timb_dma_chan *td_chan)
> else
> iowrite32(0, td_chan->membase + TIMBDMA_OFFS_TX_DLAR);
> */
> - td_chan->chan.completed_cookie = txd->cookie;
> + dma_cookie_complete(txd);
> td_chan->ongoing = false;
>
> callback = txd->callback;
> diff --git a/drivers/dma/txx9dmac.c b/drivers/dma/txx9dmac.c
> index 66f8fca..8a5225b 100644
> --- a/drivers/dma/txx9dmac.c
> +++ b/drivers/dma/txx9dmac.c
> @@ -411,7 +411,7 @@ txx9dmac_descriptor_complete(struct txx9dmac_chan *dc,
> dev_vdbg(chan2dev(&dc->chan), "descriptor %u %p complete\n",
> txd->cookie, desc);
>
> - dc->chan.completed_cookie = txd->cookie;
> + dma_cookie_complete(txd);
> callback = txd->callback;
> param = txd->callback_param;
>
> --
> 1.7.4.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 5/9] dmaengine: provide a common function for completing a dma descriptor
From: Russell King - ARM Linux @ 2012-04-23 9:50 UTC (permalink / raw)
To: Boojin Kim
Cc: 'Stephen Warren', 'Linus Walleij',
'Srinidhi Kasagar', 'Vinod Koul',
'Dan Williams', linuxppc-dev, linux-arm-kernel
In-Reply-To: <056201cd2135$10ea5b70$32bf1250$%kim@samsung.com>
On Mon, Apr 23, 2012 at 06:40:06PM +0900, Boojin Kim wrote:
> I met a problem on DMA cyclic mode (DMA_CYCLIC) for sound playback.
> Kernel BUG occurs during DMA transfer with DMA cyclic mode.
> This patch makes the cookies into zero. But, cookies should be kept
> during cyclic mode because cyclic mode re-uses the cookies.
The protection is there to prevent cookies being accidentally re-used.
If you're running a cyclic transfer, even then you shouldn't be completing
the same cookie time and time again - I think Vinod also concurs with this.
I think our preference is for cyclic transfers to entire remain uncompleted,
or to get a new cookie each time they allegedly "complete".
^ permalink raw reply
* RE: [PATCH][2/3][RFC] TDM Framework
From: Aggrwal Poonam-B10812 @ 2012-04-23 8:00 UTC (permalink / raw)
To: Aggrwal Poonam-B10812, linuxppc-dev@lists.ozlabs.org; +Cc: Singh Sandeep-B37400
In-Reply-To: <1331384221-29661-1-git-send-email-poonam.aggrwal@freescale.com>
> -----Original Message-----
> From: Aggrwal Poonam-B10812
> Sent: Saturday, March 10, 2012 6:27 PM
> To: linuxppc-dev@lists.ozlabs.org
> Cc: Singh Sandeep-B37400; Aggrwal Poonam-B10812
> Subject: [PATCH][2/3][RFC] TDM Framework
>=20
Any feedback on this patchset?
> From: Sandeep Singh <Sandeep@freescale.com>
>=20
> TDM Framework is an attempt to provide a platform independent layer which
> can offer a standard interface for TDM access to different client
> modules.
> Beneath, the framework layer can house different types of TDM drivers to
> handle various TDM devices, the hardware intricacies of the devices being
> completely taken care by TDM drivers.
>=20
> This framework layer will allow any type of TDM device to hook with it.
> For example Freescale controller as on MPC8315, UCC based TDM controller,
> or any other controller.
>=20
> The main functions of this Framework are:
> - provides interface to TDM clients to access TDM functionalities.
> - provides standard interface for TDM drivers to hook with the framework.
> - handles various data handling stuff and buffer management.
>=20
> In future this Framework will be extended to provide Interface for Line
> control devices also. For example SLIC, E1/T1 Framers etc.
>=20
> Limitations/Future Work
> ---------------------------
> 1. Presently the framework supports only Single Port channelised mode.
> 2. Also the configurability options are limited which will be extended
> later on.
> 3. Only kernel mode TDM clients are supported currently. Support for User
> mode clients will be added later.
>=20
> Signed-off-by: Sandeep Singh <Sandeep@freescale.com>
> Signed-off-by: Poonam Aggrwal <poonam.aggrwal@freescale.com>
> ---
> A couple of todos' are left in the patch, we are working on it and will
> be addressed in the updated patch set.
> drivers/Kconfig | 1 +
> drivers/Makefile | 1 +
> drivers/tdm/Kconfig | 25 +
> drivers/tdm/tdm-core.c | 1146
> +++++++++++++++++++++++++++++++++++++++
> include/linux/mod_devicetable.h | 11 +
> include/linux/tdm.h | 347 ++++++++++++
> 6 files changed, 1531 insertions(+), 0 deletions(-) create mode 100644
> drivers/tdm/Kconfig create mode 100644 drivers/tdm/tdm-core.c create
> mode 100644 include/linux/tdm.h
>=20
> diff --git a/drivers/Kconfig b/drivers/Kconfig index ad6c1eb..25f7f5b
> 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -130,4 +130,5 @@ source "drivers/virt/Kconfig"
>=20
> source "drivers/net/dpa/NetCommSw/Kconfig"
>=20
> +source "drivers/tdm/Kconfig"
> endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile index cd546eb..362b5ed
> 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -102,6 +102,7 @@ obj-$(CONFIG_INFINIBAND) +=3D infiniband/
> obj-$(CONFIG_SGI_SN) +=3D sn/
> obj-y +=3D firmware/
> obj-$(CONFIG_CRYPTO) +=3D crypto/
> +obj-$(CONFIG_TDM) +=3D tdm/
> obj-$(CONFIG_SUPERH) +=3D sh/
> obj-$(CONFIG_ARCH_SHMOBILE) +=3D sh/
> ifndef CONFIG_ARCH_USES_GETTIMEOFFSET
> diff --git a/drivers/tdm/Kconfig b/drivers/tdm/Kconfig new file mode
> 100644 index 0000000..8db2b05
> --- /dev/null
> +++ b/drivers/tdm/Kconfig
> @@ -0,0 +1,25 @@
> +#
> +# TDM subsystem configuration
> +#
> +
> +menuconfig TDM
> + tristate "TDM support"
> + ---help---
> + More information is contained in the directory
> <file:Documentation/tdm/>,
> + especially in the file called "summary" there.
> + If you want TDM support, you should say Y here and also to the
> + specific driver for your bus adapter(s) below.
> +
> + This TDM support can also be built as a module. If so, the
> module
> + will be called tdm-core.
> +
> +if TDM
> +
> +config TDM_DEBUG_CORE
> + bool "TDM Core debugging messages"
> + help
> + Say Y here if you want the TDM core to produce a bunch of debug
> + messages to the system log. Select this if you are having a
> + problem with TDM support and want to see more of what is going
> on.
> +
> +endif # TDM
> diff --git a/drivers/tdm/tdm-core.c b/drivers/tdm/tdm-core.c new file
> mode 100644 index 0000000..cdda260
> --- /dev/null
> +++ b/drivers/tdm/tdm-core.c
> @@ -0,0 +1,1146 @@
> +/* driver/tdm/tdm-core.c
> + *
> + * Copyright (C) 2012 Freescale Semiconductor, Inc, All rights reserved.
> + *
> + * TDM core is the interface between TDM clients and TDM devices.
> + * It is also intended to serve as an interface for line controld
> + * devices later on.
> + *
> + * Author:Hemant Agrawal <hemant@freescale.com>
> + * Rajesh Gumasta <rajesh.gumasta@freescale.com>
> + *
> + * Modified by Sandeep Kr Singh <sandeep@freescale.com>
> + * Poonam Aggarwal <poonam.aggarwal@freescale.com>
> + * 1. Added framework based initialization of device.
> + * 2. All the init/run time configuration is now done by framework.
> + * 3. Added channel level operations.
> + *
> + * This program is free software; you can redistribute it and/or
> +modify it
> + * under the terms of the GNU General Public License as published by
> +the
> + * Free Software Foundation; either version 2 of the License, or (at
> +your
> + * option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> +along
> + * with this program; if not, write to the Free Software Foundation,
> +Inc.,
> + * 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +/* if read write debug required */
> +#undef TDM_CORE_DEBUG
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/errno.h>
> +#include <linux/slab.h>
> +#include <linux/tdm.h>
> +#include <linux/init.h>
> +#include <linux/idr.h>
> +#include <linux/mutex.h>
> +#include <linux/completion.h>
> +#include <linux/hardirq.h>
> +#include <linux/irqflags.h>
> +#include <linux/list.h>
> +#include <linux/uaccess.h>
> +#include <linux/io.h>
> +#include "device/tdm_fsl.h"
> +
> +
> +static DEFINE_MUTEX(tdm_core_lock);
> +static DEFINE_IDR(tdm_adapter_idr);
> +/* List of TDM adapters registered with TDM framework */
> +LIST_HEAD(adapter_list);
> +
> +/* List of TDM clients registered with TDM framework */
> +LIST_HEAD(driver_list);
> +
> +/* In case the previous data is not fetched by the client driver, the
> + * de-interleaving function will discard the old data and rewrite the
> + * new data */
> +static int use_latest_tdm_data =3D 1;
> +
> +/* this tasklet is created for each adapter instance */ static void
> +tdm_data_tasklet_fn(unsigned long);
> +
> +/* tries to match client driver with the adapter */ static int
> +tdm_device_match(struct tdm_driver *driver, struct tdm_adapter *adap) {
> + /* match on an id table if there is one */
> + if (driver->id_table && driver->id_table->name[0]) {
> + if (!(strcmp(driver->id_table->name, adap->name)))
> + return (int)driver->id_table;
> + }
> + return TDM_E_OK;
> +}
> +
> +static int tdm_attach_driver_adap(struct tdm_driver *driver,
> + struct tdm_adapter *adap)
> +{
> + /* if driver is already attached to any other adapter, return*/
> + if (driver->adapter && (driver->adapter !=3D adap))
> + return TDM_E_OK;
> +
> + driver->adapter =3D adap;
> +
> + if (driver->attach_adapter) {
> + if (driver->attach_adapter(adap) < 0)
> + /* We ignore the return code; if it fails, too bad */
> + pr_err("attach_adapter failed for driver [%s]\n",
> + driver->name);
> + }
> + adap->drv_count++;
> +
> + if (!adap->tasklet_conf) {
> + tasklet_init(&adap->tdm_data_tasklet, tdm_data_tasklet_fn,
> + (unsigned long)adap);
> + adap->tasklet_conf =3D 1;
> + }
> +
> + return TDM_E_OK;
> +}
> +
> +/* Detach client driver and adapter */
> +static int tdm_detach_driver_adap(struct tdm_driver *driver,
> + struct tdm_adapter *adap)
> +{
> + int res =3D TDM_E_OK;
> +
> + if (!driver->adapter || (driver->adapter !=3D adap))
> + return TDM_E_OK;
> +
> + if (!driver->detach_adapter)
> + return TDM_E_OK;
> +
> + adap->drv_count--;
> +
> + /* If no more driver is registed with the adapter*/
> + if (!adap->drv_count && adap->tasklet_conf) {
> + tasklet_disable(&adap->tdm_data_tasklet);
> + tasklet_kill(&adap->tdm_data_tasklet);
> + adap->tasklet_conf =3D 0;
> + }
> +
> + if (driver->detach_adapter) {
> + if (driver->detach_adapter(adap))
> + pr_err("detach_adapter failed for driver [%s]\n",
> + driver->name);
> + }
> +
> + driver->adapter =3D NULL;
> + return res;
> +}
> +
> +/* TDM adapter Registration/De-registration with TDM framework */
> +
> +static int tdm_register_adapter(struct tdm_adapter *adap) {
> + int res =3D TDM_E_OK;
> + struct tdm_driver *driver, *next;
> +
> + if (!adap) {
> + pr_err("%s:Invalid handle\n", __func__);
> + return -EINVAL;
> + }
> +
> + mutex_init(&adap->adap_lock);
> + INIT_LIST_HEAD(&adap->myports);
> + spin_lock_init(&adap->portlist_lock);
> +
> + adap->drv_count =3D 0;
> + adap->tasklet_conf =3D 0;
> +
> + list_add_tail(&adap->list, &adapter_list);
> +
> + /* initialization of driver by framework in default configuration
> */
> + init_config_adapter(adap);
> +
> + /* Notify drivers */
> + pr_info("adapter [%s] registered\n", adap->name);
> + mutex_lock(&tdm_core_lock);
> + list_for_each_entry_safe(driver, next, &driver_list, list) {
> + if (tdm_device_match(driver, adap)) {
> + res =3D tdm_attach_driver_adap(driver, adap);
> + pr_info(
> + "Driver(ID=3D%d) is attached with Adapter %s(ID =3D %d)\n",
> + driver->id, adap->name, adap->id);
> + }
> + }
> + mutex_unlock(&tdm_core_lock);
> +
> + return res;
> +}
> +
> +/*
> + * tdm_add_adapter - declare tdm adapter, use dynamic device number
> + * @adapter: the adapter to add
> + * Context: can sleep
> + *
> + * This routine is used to declare a TDM adapter
> + * When this returns zero, a new device number will be allocated and
> +stored
> + * in adap->id, and the specified adapter became available for the
> clients.
> + * Otherwise, a negative errno value is returned.
> + */
> +int tdm_add_adapter(struct tdm_adapter *adapter) {
> + int id, res =3D TDM_E_OK;
> + if (!adapter) {
> + pr_err("%s:Invalid handle\n", __func__);
> + return -EINVAL;
> + }
> +
> +retry:
> + if (idr_pre_get(&tdm_adapter_idr, GFP_KERNEL) =3D=3D 0)
> + return -ENOMEM;
> +
> + mutex_lock(&tdm_core_lock);
> + res =3D idr_get_new(&tdm_adapter_idr, adapter, &id);
> + mutex_unlock(&tdm_core_lock);
> +
> + if (res < 0) {
> + if (res =3D=3D -EAGAIN)
> + goto retry;
> + return res;
> + }
> +
> + adapter->id =3D id;
> + return tdm_register_adapter(adapter);
> +}
> +EXPORT_SYMBOL(tdm_add_adapter);
> +
> +
> +/**
> + * tdm_del_adapter - unregister TDM adapter
> + * @adap: the adapter being unregistered
> + *
> + * This unregisters an TDM adapter which was previously registered
> + * by @tdm_add_adapter.
> + */
> +int tdm_del_adapter(struct tdm_adapter *adap) {
> + int res =3D TDM_E_OK;
> + struct tdm_adapter *found;
> + struct tdm_driver *driver, *next;
> +
> + if (!adap) {
> + pr_err("%s:Invalid handle\n", __func__);
> + return -EINVAL;
> + }
> +
> + /* First make sure that this adapter was ever added */
> + mutex_lock(&tdm_core_lock);
> + found =3D idr_find(&tdm_adapter_idr, adap->id);
> + mutex_unlock(&tdm_core_lock);
> + if (found !=3D adap) {
> + pr_err("tdm-core: attempting to delete unregistered "
> + "adapter [%s]\n", adap->name);
> + return -EINVAL;
> + }
> +
> + /*disable and kill the data processing tasklet */
> + if (adap->tasklet_conf) {
> + tasklet_disable(&adap->tdm_data_tasklet);
> + tasklet_kill(&adap->tdm_data_tasklet);
> + adap->tasklet_conf =3D 0;
> + }
> +
> + /* Detach any active ports. This can't fail, thus we do not
> + checking the returned value. */
> + mutex_lock(&tdm_core_lock);
> + list_for_each_entry_safe(driver, next, &driver_list, list) {
> + if (tdm_device_match(driver, adap)) {
> + tdm_detach_driver_adap(driver, adap);
> + pr_info(
> + "Driver(ID=3D%d) is detached from Adapter %s(ID =3D %d)\n",
> + driver->id, adap->name, adap->id);
> + }
> + }
> + mutex_unlock(&tdm_core_lock);
> +
> + mutex_lock(&tdm_core_lock);
> + idr_remove(&tdm_adapter_idr, adap->id);
> + mutex_unlock(&tdm_core_lock);
> +
> + pr_debug("adapter [%s] unregistered\n", adap->name);
> +
> + list_del(&adap->list);
> + /* Clear the device structure in case this adapter is ever going to
> be
> + added again */
> + adap->parent =3D NULL;
> +
> + return res;
> +}
> +EXPORT_SYMBOL(tdm_del_adapter);
> +
> +/* TDM Client Drivers Registration/De-registration Functions */ int
> +tdm_register_driver(struct tdm_driver *driver) {
> + int res =3D TDM_E_OK;
> + struct tdm_adapter *adap, *next;
> +
> + list_add_tail(&driver->list, &driver_list);
> +
> + mutex_lock(&tdm_core_lock);
> + /* Walk the adapters that are already present */
> + list_for_each_entry_safe(adap, next, &adapter_list, list) {
> + if (tdm_device_match(driver, adap)) {
> + res =3D tdm_attach_driver_adap(driver, adap);
> + pr_info("TDM Driver(ID=3D%d)is attached with Adapter"
> + "%s(ID =3D %d) drv_count=3D%d", driver->id,
> + adap->name, adap->id, adap->drv_count);
> + break;
> + }
> + }
> + mutex_unlock(&tdm_core_lock);
> +
> + return res;
> +}
> +EXPORT_SYMBOL(tdm_register_driver);
> +
> +/*
> + * tdm_unregister_driver - unregister TDM client driver from TDM
> +framework
> + * @driver: the driver being unregistered */ void
> +tdm_unregister_driver(struct tdm_driver *driver) {
> + if (!driver) {
> + pr_err("%s:Invalid handle\n", __func__);
> + return;
> + }
> + /* A driver can register to only one adapter,
> + * so no need to browse the list */
> + mutex_lock(&tdm_core_lock);
> + tdm_detach_driver_adap(driver, driver->adapter);
> + mutex_unlock(&tdm_core_lock);
> +
> + list_del(&driver->list);
> +
> + pr_debug("tdm-core: driver [%s] unregistered\n", driver->name); }
> +EXPORT_SYMBOL(tdm_unregister_driver);
> +
> +/* TDM Framework init and exit */
> +static int __init tdm_init(void)
> +{
> + pr_info("%s\n", __func__);
> + return TDM_E_OK;
> +}
> +
> +static void __exit tdm_exit(void)
> +{
> + pr_info("%s\n", __func__);
> + return;
> +}
> +
> +/* We must initialize early, because some subsystems register tdm
> +drivers
> + * in subsys_initcall() code, but are linked (and initialized) before
> tdm.
> + */
> +postcore_initcall(tdm_init);
> +module_exit(tdm_exit);
> +
> +
> +/* Interface to the tdm device/adapter */
> +
> +/* tdm_adap_send - issue a TDM write
> + * @adap: Handle to TDM device
> + * @buf: Data that will be written to the TDM device
> + * @count: How many bytes to write
> + *
> + * Returns negative errno, or else the number of bytes written.
> + */
> +int tdm_adap_send(struct tdm_adapter *adap, void **buf, int count) {
> + int res;
> +
> + if ((adap =3D=3D NULL) || (buf =3D=3D NULL)) { /* invalid handle*/
> + pr_err("%s: Invalid Handle\n", __func__);
> + return -ENXIO;
> + }
> +
> + if (adap->algo->tdm_write)
> + res =3D adap->algo->tdm_write(adap, buf, count);
> + else {
> + pr_err("TDM level write not supported\n");
> + return -EOPNOTSUPP;
> + }
> +
> + /* If everything went ok (i.e. frame transmitted), return #bytes
> + transmitted, else error code. */
> + return (res =3D=3D 1) ? count : res;
> +}
> +EXPORT_SYMBOL(tdm_adap_send);
> +
> +/**
> + * tdm_adap_recv - issue a TDM read
> + * @adap: Handle to TDM device
> + * @buf: Where to store data read from TDM device
> + *
> + * Returns negative errno, or else the number of bytes read.
> + */
> +int tdm_adap_recv(struct tdm_adapter *adap, void **buf) {
> + int res;
> +
> + if (adap->algo->tdm_read)
> + res =3D adap->algo->tdm_read(adap, (u16 **)buf);
> + else {
> + pr_err("TDM level read not supported\n");
> + return -EOPNOTSUPP;
> + }
> + /* If everything went ok (i.e. frame received), return #bytes
> + transmitted, else error code. */
> + return res;
> +}
> +
> +/**
> + * tdm_adap_get_write_buf - get next write TDM device buffer
> + * @adap: Handle to TDM device
> + * @buf: pointer to TDM device buffer
> + *
> + * Returns negative errno, or else size of the write buffer.
> + */
> +int tdm_adap_get_write_buf(struct tdm_adapter *adap, void **buf) {
> + int res;
> +
> + if (adap->algo->tdm_get_write_buf) {
> + res =3D adap->algo->tdm_get_write_buf(adap, (u16 **)buf);
> + } else {
> + pr_err("TDM level write buf get not supported\n");
> + return -EOPNOTSUPP;
> + }
> + /* If everything went ok (i.e. 1 msg received), return #bytes
> + transmitted, else error code. */
> + return res;
> +}
> +EXPORT_SYMBOL(tdm_adap_get_write_buf);
> +
> +int tdm_adap_enable(struct tdm_driver *drv) {
> + int res;
> + struct tdm_adapter *adap;
> + if (drv =3D=3D NULL) { /* invalid handle*/
> + pr_err("%s: Invalid Handle\n", __func__);
> + return -ENXIO;
> + }
> + adap =3D drv->adapter;
> +
> + if (adap->algo->tdm_enable) {
> + res =3D adap->algo->tdm_enable(adap);
> + } else {
> + pr_err("TDM level enable not supported\n");
> + return -EOPNOTSUPP;
> + }
> + return res;
> +}
> +EXPORT_SYMBOL(tdm_adap_enable);
> +
> +int tdm_adap_disable(struct tdm_driver *drv) {
> + int res;
> + struct tdm_adapter *adap;
> + if (drv =3D=3D NULL) { /* invalid handle*/
> + pr_err("%s: Invalid Handle\n", __func__);
> + return -ENXIO;
> + }
> + adap =3D drv->adapter;
> +
> + if (adap->algo->tdm_disable) {
> + res =3D adap->algo->tdm_disable(adap);
> + } else {
> + pr_err("TDM level enable not supported\n");
> + return -EOPNOTSUPP;
> + }
> + return res;
> +}
> +EXPORT_SYMBOL(tdm_adap_disable);
> +
> +struct tdm_adapter *tdm_get_adapter(int id) {
> + struct tdm_adapter *adapter;
> +
> + mutex_lock(&tdm_core_lock);
> + adapter =3D idr_find(&tdm_adapter_idr, id);
> + if (adapter && !try_module_get(adapter->owner))
> + adapter =3D NULL;
> +
> + mutex_unlock(&tdm_core_lock);
> +
> + return adapter;
> +}
> +EXPORT_SYMBOL(tdm_get_adapter);
> +
> +void tdm_put_adapter(struct tdm_adapter *adap) {
> + module_put(adap->owner);
> +}
> +EXPORT_SYMBOL(tdm_put_adapter);
> +
> +
> +/* Port Level APIs of TDM Framework */
> +unsigned int tdm_port_open(struct tdm_driver *driver, void **h_port) {
> + struct tdm_port *port;
> + struct tdm_adapter *adap;
> + unsigned long flags;
> + int res =3D TDM_E_OK;
> +
> + if (driver =3D=3D NULL) {
> + pr_err("driver NULL\n");
> + return -ENODEV;
> + }
> + if (driver->adapter =3D=3D NULL) {
> + pr_err("adapter NULL\n");
> + return -ENODEV;
> + }
> +
> + adap =3D tdm_get_adapter(driver->adapter->id);
> + if (!adap)
> + return -ENODEV;
> +
> + /* This creates an anonymous tdm_port, which may later be
> + * pointed to some slot.
> + *
> + */
> + port =3D kzalloc(sizeof(*port), GFP_KERNEL);
> + if (!port) {
> + res =3D -ENOMEM;
> + goto out;
> + }
> +
> + init_waitqueue_head(&port->ch_wait_queue);
> +
> +
> + port->rx_max_frames =3D NUM_SAMPLES_PER_FRAME;
> + port->port_cfg.port_mode =3D e_TDM_PORT_CHANNELIZED;
> +
> + port->in_use =3D 1;
> +
> + snprintf(driver->name, TDM_NAME_SIZE, "tdm-dev");
> + port->driver =3D driver;
> + port->adapter =3D adap;
> +
> + spin_lock_irqsave(&adap->portlist_lock, flags);
> + list_add_tail(&port->list, &adap->myports);
> + spin_unlock_irqrestore(&adap->portlist_lock, flags);
> +
> + INIT_LIST_HEAD(&port->mychannels);
> +
> + *h_port =3D port;
> +
> +out:
> + return res;
> +}
> +EXPORT_SYMBOL(tdm_port_open);
> +
> +unsigned int tdm_port_close(void *h_port) {
> + struct tdm_adapter *adap;
> + struct tdm_driver *driver;
> + struct tdm_port *port;
> + struct tdm_channel *temp, *channel;
> + unsigned long flags;
> + int res =3D TDM_E_OK;
> + port =3D (struct tdm_port *)h_port;
> +
> + if (port =3D=3D NULL) { /* invalid handle*/
> + pr_err("Invalid Handle");
> + return -ENXIO;
> + }
> +
> + driver =3D port->driver;
> +
> + if (driver =3D=3D NULL) {
> + pr_err("driver NULL\n");
> + res =3D -ENODEV;
> + goto out;
> + }
> + if (driver->adapter =3D=3D NULL) {
> + pr_err("adapter NULL\n");
> + res =3D -ENODEV;
> + goto out;
> + }
> +
> + list_for_each_entry_safe(channel, temp, &port->mychannels, list) {
> + if (channel)
> + if (channel->in_use) {
> + pr_err("%s: Cannot close port. Channel in use\n",
> + __func__);
> + res =3D -ENXIO;
> + goto out;
> + }
> + }
> + adap =3D driver->adapter;
> +
> + spin_lock_irqsave(&adap->portlist_lock, flags);
> + list_del(&port->list);
> + spin_unlock_irqrestore(&adap->portlist_lock, flags);
> +
> + if (port->p_port_data !=3D NULL) {
> + int i;
> + struct tdm_bd *ch_bd;
> +
> + /* If the tdm is in channelised mode,
> + de-allocate the channelised buffer */
> + ch_bd =3D &(port->p_port_data->rx_data_fifo[0]);
> + for (i =3D 0; ch_bd && i < TDM_CH_RX_BD_RING_SIZE; i++) {
> + ch_bd->flag =3D 0;
> + ch_bd++;
> + }
> + ch_bd =3D &(port->p_port_data->tx_data_fifo[0]);
> + for (i =3D 0; ch_bd && i < TDM_CH_TX_BD_RING_SIZE; i++) {
> + ch_bd->flag =3D 0;
> + ch_bd++;
> + }
> + kfree(port->p_port_data);
> + }
> + kfree(port);
> + return res;
> +out:
> + if (port)
> + kfree(port->p_port_data);
> + kfree(port);
> + return res;
> +}
> +EXPORT_SYMBOL(tdm_port_close);
> +
> +unsigned int tdm_channel_read(void *h_port, void *h_channel,
> + void *p_data, u16 *size)
> +{
> + struct tdm_port *port;
> + struct tdm_channel *channel;
> + struct tdm_bd *rx_bd;
> + unsigned long flags;
> + int i, res =3D TDM_E_OK;
> + unsigned short *buf, *buf1;
> + port =3D (struct tdm_port *)h_port;
> + channel =3D (struct tdm_channel *)h_channel;
> +
> + if ((port && channel) =3D=3D 0) { /* invalid handle*/
> + pr_err("%s:Invalid Handle\n", __func__);
> + return -ENXIO;
> + }
> +
> + if (!port->in_use)
> + return -EIO;
> + if (!channel->p_ch_data || !channel->in_use)
> + return -EIO;
> +
> + spin_lock_irqsave(&channel->p_ch_data->rx_channel_lock, flags);
> + rx_bd =3D channel->p_ch_data->rx_out_data;
> +
> + if (rx_bd->flag) {
> + *size =3D rx_bd->length;
> + buf =3D (u16 *) p_data;
> + buf1 =3D (u16 *)rx_bd->p_data;
> + for (i =3D 0; i < NUM_SAMPLES_PER_FRAME; i++)
> + buf[i] =3D buf1[i];
> + rx_bd->flag =3D 0;
> + rx_bd->offset =3D 0;
> + channel->p_ch_data->rx_out_data =3D (rx_bd->wrap) ?
> + channel->p_ch_data->rx_data_fifo : rx_bd + 1;
> +
> + } else {
> + spin_unlock_irqrestore(&channel->p_ch_data->rx_channel_lock,
> + flags);
> + pr_info("No Data Available");
> + return -EAGAIN;
> + }
> + spin_unlock_irqrestore(&channel->p_ch_data->rx_channel_lock,
> flags);
> +
> + return res;
> +}
> +EXPORT_SYMBOL(tdm_channel_read);
> +
> +
> +unsigned int tdm_channel_write(void *h_port, void *h_channel,
> + void *p_data, u16 size)
> +{
> + struct tdm_port *port;
> + struct tdm_channel *channel;
> + struct tdm_bd *tx_bd;
> + unsigned long flags;
> + int err =3D TDM_E_OK;
> + port =3D (struct tdm_port *)h_port;
> + channel =3D (struct tdm_channel *)h_channel; #ifdef TDM_CORE_DEBUG
> + bool data_flag =3D 0;
> +#endif
> +
> + if ((port && channel) =3D=3D 0) { /* invalid handle*/
> + pr_err("Invalid Handle");
> + return -ENXIO;
> + }
> +
> + if (p_data =3D=3D NULL) { /* invalid data*/
> + pr_err("Invalid Data");
> + return -EFAULT;
> + }
> +
> + if (!port->in_use)
> + return -EIO;
> + if (!channel->p_ch_data || !channel->in_use)
> + return -EIO;
> +
> + spin_lock_irqsave(&channel->p_ch_data->tx_channel_lock, flags);
> + tx_bd =3D channel->p_ch_data->tx_in_data;
> +
> + if (!tx_bd->flag) {
> + tx_bd->length =3D size;
> + memcpy(tx_bd->p_data, p_data,
> + size * port->adapter->adapt_cfg.slot_width);
> + tx_bd->flag =3D 1;
> + tx_bd->offset =3D 0;
> + channel->p_ch_data->tx_in_data =3D (tx_bd->wrap) ?
> + channel->p_ch_data->tx_data_fifo : tx_bd+1;
> + port->port_stat.tx_pkt_count++;
> +#ifdef TDM_CORE_DEBUG
> + data_flag =3D 1;
> +#endif
> + } else {
> + spin_unlock_irqrestore(&channel->p_ch_data->tx_channel_lock,
> + flags);
> + port->port_stat.tx_pkt_drop_count++;
> + pr_err("E_NO_MEMORY -Failed Transmit");
> + return -ENOMEM;
> + }
> + spin_unlock_irqrestore(&channel->p_ch_data->tx_channel_lock,
> flags);
> +
> +#ifdef TDM_CORE_DEBUG
> + if (data_flag) {
> + int k;
> + pr_info("\nTX port:%d - Write - Port TX-%d\n",
> + port->port_id, size);
> + for (k =3D 0; k < size; k++)
> + pr_info("%x", p_data[k]);
> + pr_info("\n");
> + }
> +#endif
> + return err;
> +}
> +EXPORT_SYMBOL(tdm_channel_write);
> +
> +wait_queue_head_t *tdm_port_get_wait_queue(void *h_port) {
> + struct tdm_port *port;
> + port =3D (struct tdm_port *)h_port;
> +
> + if (port =3D=3D NULL) { /* invalid handle*/
> + pr_err("Invalid Handle");
> + return NULL;
> + }
> +
> + return &port->ch_wait_queue;
> +
> +}
> +EXPORT_SYMBOL(tdm_port_get_wait_queue);
> +
> +/* Driver Function for select and poll. Based on Port no, it sleeps on
> + * waitqueue */
> +unsigned int tdm_port_poll(void *h_port, unsigned int wait_time) {
> + struct tdm_port *port;
> + unsigned long timeout =3D msecs_to_jiffies(wait_time);
> + port =3D (struct tdm_port *)h_port;
> +
> + if (port =3D=3D NULL) { /* invalid handle*/
> + pr_err("%s: Invalid Handle\n", __func__);
> + return -ENXIO;
> + }
> + if (!port->p_port_data || !port->in_use)
> + return -EIO;
> +
> + if (port->p_port_data->rx_out_data->flag) {
> + pr_debug("Data Available");
> + return TDM_E_OK;
> + }
> + if (timeout) {
> + wait_event_interruptible_timeout(port->ch_wait_queue,
> + port->p_port_data->rx_out_data->flag,
> + timeout);
> +
> + if (port->p_port_data->rx_out_data->flag) {
> + pr_debug("Data Available");
> + return TDM_E_OK;
> + }
> + }
> + return -EAGAIN;
> +}
> +EXPORT_SYMBOL(tdm_port_poll);
> +
> +unsigned int tdm_port_get_stats(void *h_port, struct tdm_port_stats
> +*portStat) {
> + struct tdm_port *port;
> + int port_num;
> + port =3D (struct tdm_port *)h_port;
> +
> + if (port =3D=3D NULL || portStat =3D=3D NULL) { /* invalid handle*/
> + pr_err("Invalid Handle");
> + return -ENXIO;
> + }
> + port_num =3D port->port_id;
> +
> + memcpy(portStat, &port->port_stat, sizeof(struct tdm_port_stats));
> +
> + pr_info("TDM Port %d Get Stats", port_num);
> +
> + return TDM_E_OK;
> +}
> +EXPORT_SYMBOL(tdm_port_get_stats);
> +
> +/* Data handling functions */
> +
> +static int tdm_data_rx_deinterleave(struct tdm_adapter *adap) {
> + struct tdm_port *port, *next;
> + struct tdm_channel *channel, *temp;
> + struct tdm_bd *ch_bd;
> +
> + int i, buf_size, ch_data_len;
> + u16 *input_tdm_buffer;
> + u16 *pcm_buffer;
> + int slot_width;
> + int frame_ch_data_size;
> + bool ch_data;
> + int bytes_in_fifo_per_frame;
> + int bytes_slot_offset;
> +
> + ch_data_len =3D NUM_SAMPLES_PER_FRAME;
> + frame_ch_data_size =3D NUM_SAMPLES_PER_FRAME;
> + ch_data =3D 0;
> +
> + if (!adap) { /* invalid handle*/
> + pr_err("%s: Invalid Handle\n", __func__);
> + return -ENXIO;
> + }
> +
> + slot_width =3D adap->adapt_cfg.slot_width;
> + buf_size =3D tdm_adap_recv(adap, (void **)&input_tdm_buffer);
> + if (buf_size <=3D 0 || !input_tdm_buffer)
> + return -EINVAL;
> +
> + bytes_in_fifo_per_frame =3D buf_size/frame_ch_data_size;
> + bytes_slot_offset =3D bytes_in_fifo_per_frame/slot_width;
> +
> + /* de-interleaving for all ports*/
> + list_for_each_entry_safe(port, next, &adap->myports, list) {
> +
> + /* if the port is not open */
> + if (!port->in_use)
> + continue;
> +
> + list_for_each_entry_safe(channel, temp, &port->mychannels,
> + list) {
> + /* if the channel is not open */
> + if (!channel->in_use || !channel->p_ch_data)
> + continue;
> + ch_bd =3D channel->p_ch_data->rx_in_data;
> + spin_lock(&channel->p_ch_data->rx_channel_lock);
> + /*if old data is to be discarded */
> + if (use_latest_tdm_data)
> + if (ch_bd->flag) {
> + ch_bd->flag =3D 0;
> + ch_bd->offset =3D 0;
> + if (ch_bd =3D=3D channel->p_ch_data->rx_out_data)
> + channel->p_ch_data->rx_out_data =3D
> + ch_bd->wrap ?
> + channel->p_ch_data->rx_data_fifo
> + : ch_bd+1;
> + port->port_stat.rx_pkt_drop_count++;
> + }
> + /* if the bd is empty */
> + if (!ch_bd->flag) {
> + if (ch_bd->offset =3D=3D 0)
> + ch_bd->length =3D port->rx_max_frames;
> +
> + pcm_buffer =3D ch_bd->p_data + ch_bd->offset;
> + /* De-interleaving the data */
> + for (i =3D 0; i < ch_data_len; i++) {
> + pcm_buffer[i]
> + =3D input_tdm_buffer[i*bytes_slot_offset +
> + channel->ch_id];
> + }
> + ch_bd->offset +=3D ch_data_len * slot_width;
> +
> + if (ch_bd->offset >=3D
> + (ch_bd->length - frame_ch_data_size)*
> + (adap->adapt_cfg.slot_width)) {
> + ch_bd->flag =3D 1;
> + ch_bd->offset =3D 0;
> + channel->p_ch_data->rx_in_data =3D
> + ch_bd->wrap ?
> + channel->p_ch_data->rx_data_fifo
> + : ch_bd+1;
> + ch_data =3D 1;
> + }
> + } else {
> + port->port_stat.rx_pkt_drop_count++;
> + }
> + spin_unlock(&channel->p_ch_data->rx_channel_lock);
> + }
> +
> + if (ch_data) {
> + /* Wake up the Port Data Poll event */
> + wake_up_interruptible(&port->ch_wait_queue);
> +#ifdef TDM_CORE_DEBUG
> + pr_info("Port RX-%d-%d\n", channel->ch_id,
> ch_data_len);
> + for (i =3D 0; i < ch_data_len; i++)
> + pr_info("%x", pcm_buffer[i]);
> + pr_info("\n");
> +#endif
> + port->port_stat.rx_pkt_count++;
> + ch_data =3D 0;
> + }
> + }
> + return TDM_E_OK;
> +}
> +
> +static int tdm_data_tx_interleave(struct tdm_adapter *adap) {
> + struct tdm_port *port, *next;
> + struct tdm_channel *channel, *temp;
> + struct tdm_bd *ch_bd;
> + int i, buf_size, ch_data_len =3D NUM_SAMPLES_PER_FRAME;
> + bool last_data =3D 0;
> + u16 *output_tdm_buffer;
> + u16 *pcm_buffer;
> + int frame_ch_data_size =3D NUM_SAMPLES_PER_FRAME;
> + int bytes_in_fifo_per_frame;
> + int bytes_slot_offset;
> +
> +#ifdef TDM_CORE_DEBUG
> + u8 data_flag =3D 0;
> +#endif
> +
> + if (adap =3D=3D NULL) { /* invalid handle*/
> + pr_err("%s: Invalid Handle\n", __func__);
> + return -ENXIO;
> + }
> +
> + buf_size =3D tdm_adap_get_write_buf(adap, (void
> **)&output_tdm_buffer);
> + if (buf_size <=3D 0 || !output_tdm_buffer)
> + return -EINVAL;
> +
> + bytes_in_fifo_per_frame =3D buf_size/frame_ch_data_size;
> + bytes_slot_offset =3D
> +bytes_in_fifo_per_frame/adap->adapt_cfg.slot_width;
> +
> +
> + memset(output_tdm_buffer, 0, sizeof(buf_size));
> +
> + list_for_each_entry_safe(port, next, &adap->myports, list) {
> +
> + /* check if the port is open */
> + if (!port->in_use)
> + continue;
> +
> + list_for_each_entry_safe(channel, temp, &port->mychannels,
> + list) {
> + pr_debug("TX-Tdm %d (slots-)", channel->ch_id);
> +
> +
> + /* if the channel is open */
> + if (!channel->in_use || !channel->p_ch_data)
> + continue;
> +
> + spin_lock(&channel->p_ch_data->tx_channel_lock);
> + if (!channel->in_use || !channel->p_ch_data)
> + continue;
> + ch_bd =3D channel->p_ch_data->tx_out_data;
> + if (ch_bd->flag) {
> + pcm_buffer =3D (u16 *)((uint8_t *)ch_bd->p_data +
> + ch_bd->offset);
> + /*if the buffer has less frames than required */
> + if (frame_ch_data_size >=3D
> + ((ch_bd->length) - (ch_bd->offset/
> + adap->adapt_cfg.slot_width))) {
> + ch_data_len =3D
> + (ch_bd->length) - (ch_bd->offset/
> + adap->adapt_cfg.slot_width);
> + last_data =3D 1;
> + } else {
> + ch_data_len =3D frame_ch_data_size;
> + }
> + /* Interleaving the data */
> + for (i =3D 0; i < ch_data_len; i++) {
> + /* TODO- need to be generic for any size
> + assignment*/
> + output_tdm_buffer[channel->ch_id +
> + bytes_slot_offset * i] =3D
> + pcm_buffer[i];
> + }
> + /* If all the data of this buffer is
> + transmitted */
> + if (last_data) {
> + ch_bd->flag =3D 0;
> + ch_bd->offset =3D 0;
> + channel->p_ch_data->tx_out_data =3D
> + ch_bd->wrap ?
> + channel->p_ch_data->tx_data_fifo
> + : ch_bd+1;
> + port->port_stat.tx_pkt_conf_count++;
> + } else {
> + ch_bd->offset +=3D ch_data_len *
> + (adap->adapt_cfg.slot_width);
> + }
> +#ifdef TDM_CORE_DEBUG
> + data_flag =3D 1;
> +#endif
> + }
> + spin_unlock(&channel->p_ch_data->tx_channel_lock);
> + }
> + }
> +
> +#ifdef TDM_CORE_DEBUG
> + if (data_flag) {
> + pr_info("TX-TDM Interleaved Data-\n");
> + for (i =3D 0; i < 64; i++)
> + pr_info("%x", output_tdm_buffer[i]);
> + pr_info("\n");
> + }
> +#endif
> + return TDM_E_OK;
> +}
> +
> +/* Channel Level APIs of TDM Framework */ int tdm_channel_open(u16
> +chanid, u16 ch_width, struct tdm_port *port,
> + void **h_channel)
> +{
> + struct tdm_channel *channel, *temp;
> + unsigned long flags;
> + struct tdm_ch_data *p_ch_data;
> + int res =3D TDM_E_OK;
> +
> + if (!(port && h_channel)) {
> + pr_err("%s: Invalid handle\n", __func__);
> + return -EINVAL;
> + }
> +
> + if (ch_width !=3D 1) {
> + pr_err("%s: Mode not supported\n", __func__);
> + return -EINVAL;
> + }
> +
> + list_for_each_entry_safe(channel, temp, &port->mychannels, list) {
> + if (channel->ch_id =3D=3D chanid) {
> + pr_err("%s: Channel %d already open\n",
> + __func__, chanid);
> + return -EINVAL;
> + }
> + }
> +
> + channel =3D kzalloc(sizeof(*channel), GFP_KERNEL);
> + if (!channel) {
> + res =3D -ENOMEM;
> + goto out;
> + }
> +
> + p_ch_data =3D kzalloc(sizeof(struct tdm_port_data), GFP_KERNEL);
> + if (!p_ch_data) {
> + res =3D -ENOMEM;
> + goto outdata;
> + }
> +
> + p_ch_data->rx_data_fifo[TDM_CH_RX_BD_RING_SIZE-1].wrap =3D 1;
> + p_ch_data->tx_data_fifo[TDM_CH_TX_BD_RING_SIZE-1].wrap =3D 1;
> +
> + p_ch_data->rx_in_data =3D p_ch_data->rx_data_fifo;
> + p_ch_data->rx_out_data =3D p_ch_data->rx_data_fifo;
> + p_ch_data->tx_in_data =3D p_ch_data->tx_data_fifo;
> + p_ch_data->tx_out_data =3D p_ch_data->tx_data_fifo;
> + spin_lock_init(&p_ch_data->rx_channel_lock);
> + spin_lock_init(&p_ch_data->tx_channel_lock);
> +
> + channel->p_ch_data =3D p_ch_data;
> +
> + channel->ch_id =3D chanid;
> + channel->ch_cfg.first_slot =3D chanid;
> + channel->ch_cfg.num_slots =3D 1; /* This is 1 for channelized
> mode and
> + configurable for other modes */
> + channel->port =3D port;
> + channel->in_use =3D 1;
> +
> + spin_lock_irqsave(&port->ch_list_lock, flags);
> + list_add_tail(&channel->list, &port->mychannels);
> + spin_unlock_irqrestore(&port->ch_list_lock, flags);
> +
> + *h_channel =3D channel;
> +
> + return res;
> +
> +outdata:
> + kfree(channel);
> +out:
> + return res;
> +}
> +EXPORT_SYMBOL(tdm_channel_open);
> +
> +int tdm_channel_close(u16 chanid, u16 ch_width, struct tdm_port *port,
> + struct tdm_channel *h_channel)
> +{
> + struct tdm_channel *channel;
> + unsigned long flags;
> + int res =3D TDM_E_OK;
> + channel =3D h_channel;
> +
> + if (!(port && channel)) {
> + pr_err("%s: Invalid handle\n", __func__);
> + res =3D -EINVAL;
> + goto out;
> + }
> +
> + if (ch_width !=3D 1) {
> + pr_err("%s: Mode not supported\n", __func__);
> + res =3D -EINVAL;
> + goto out;
> + }
> +
> + spin_lock_irqsave(&port->ch_list_lock, flags);
> + list_del(&channel->list);
> + spin_unlock_irqrestore(&port->ch_list_lock, flags);
> +
> +out:
> + if (channel)
> + kfree(channel->p_ch_data);
> + kfree(channel);
> + return res;
> +}
> +EXPORT_SYMBOL(tdm_channel_close);
> +
> +void init_config_adapter(struct tdm_adapter *adap) {
> + struct fsl_tdm_adapt_cfg default_adapt_cfg =3D {
> + .loopback =3D e_TDM_PROCESS_NORMAL,
> + .num_ch =3D NUM_CHANNELS,
> + .ch_size_type =3D CHANNEL_16BIT_LIN,
> + .frame_len =3D NUM_SAMPLES_PER_FRAME,
> + .num_frames =3D NUM_SAMPLES_PER_FRAME,
> + .adap_mode =3D e_TDM_ADAPTER_MODE_NONE
> + };
> +
> + default_adapt_cfg.slot_width =3D default_adapt_cfg.ch_size_type/3 +
> 1;
> +
> + memcpy(&adap->adapt_cfg, &default_adapt_cfg,
> + sizeof(struct fsl_tdm_adapt_cfg));
> +
> + return;
> +}
> +EXPORT_SYMBOL(init_config_adapter);
> +
> +static void tdm_data_tasklet_fn(unsigned long data) {
> + struct tdm_adapter *adapter;
> + adapter =3D (struct tdm_adapter *)data;
> + if (adapter !=3D NULL) {
> + tdm_data_tx_interleave(adapter);
> + tdm_data_rx_deinterleave(adapter);
> + }
> +}
> +
> +
> +MODULE_AUTHOR("Hemant Agrawal <hemant@freescale.com> and "
> + "Rajesh Gumasta <rajesh.gumasta@freescale.com>");
> +MODULE_DESCRIPTION("TDM Driver Framework Core"); MODULE_LICENSE("GPL");
> diff --git a/include/linux/mod_devicetable.h
> b/include/linux/mod_devicetable.h index ae28e93..dc1a655 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -416,6 +416,17 @@ struct i2c_device_id {
> __attribute__((aligned(sizeof(kernel_ulong_t))));
> };
>=20
> +/* tdm */
> +
> +#define TDM_NAME_SIZE 20
> +#define TDM_MODULE_PREFIX "tdm:"
> +
> +struct tdm_device_id {
> + char name[TDM_NAME_SIZE];
> + kernel_ulong_t driver_data /* Data private to the driver */
> + __attribute__((aligned(sizeof(kernel_ulong_t))));
> +};
> +
> /* spi */
>=20
> #define SPI_NAME_SIZE 32
> diff --git a/include/linux/tdm.h b/include/linux/tdm.h new file mode
> 100644 index 0000000..8cf4ef5
> --- /dev/null
> +++ b/include/linux/tdm.h
> @@ -0,0 +1,347 @@
> +/* include/linux/tdm.h
> + *
> + * Copyright (C) 2012 Freescale Semiconductor, Inc, All rights reserved.
> + *
> + * tdm.h - definitions for the tdm-device framework interface
> + *
> + * Author:Hemant Agrawal <hemant@freescale.com>
> + * Rajesh Gumasta <rajesh.gumasta@freescale.com>
> + *
> + * This program is free software; you can redistribute it and/or
> +modify it
> + * under the terms of the GNU General Public License as published by
> +the
> + * Free Software Foundation; either version 2 of the License, or (at
> +your
> + * option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> +along
> + * with this program; if not, write to the Free Software Foundation,
> +Inc.,
> + * 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +
> +#ifndef _LINUX_TDM_H
> +#define _LINUX_TDM_H
> +
> +#ifdef __KERNEL__
> +#include <linux/types.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/device.h> /* for struct device */
> +#include <linux/sched.h> /* for completion */
> +#include <linux/mutex.h>
> +#include <linux/interrupt.h>
> +
> +#define CHANNEL_8BIT_LIN 0 /* 8 bit linear */
> +#define CHANNEL_8BIT_ULAW 1 /* 8 bit Mu-law */
> +#define CHANNEL_8BIT_ALAW 2 /* 8 bit A-law */
> +#define CHANNEL_16BIT_LIN 3 /* 16 bit Linear */
> +
> +#define NUM_CHANNELS 16
> +#define NUM_SAMPLES_PER_MS 8 /* 8 samples per milli sec per
> + channel. Req for voice data */
> +#define NUM_MS 10
> +#define NUM_SAMPLES_PER_FRAME (NUM_MS * NUM_SAMPLES_PER_MS) /*
> Number of
> + samples for 1 client buffer */
> +#define NUM_OF_TDM_BUF 3
> +
> +/* General options */
> +
> +struct tdm_adapt_algorithm;
> +struct tdm_adapter;
> +struct tdm_port;
> +struct tdm_driver;
> +
> +/* Align addr on a size boundary - adjust address up if needed */
> +/* returns min value greater than size which is multiple of alignment
> +*/ static inline int ALIGN_SIZE(u64 size, u32 alignment) {
> + return (size + alignment - 1) & (~(alignment - 1)); }
> +
> +/**
> + * struct tdm_driver - represent an TDM device driver
> + * @class: What kind of tdm device we instantiate (for detect)
> + * @id:Driver id
> + * @name: Name of the driver
> + * @attach_adapter: Callback for device addition (for legacy drivers)
> + * @detach_adapter: Callback for device removal (for legacy drivers)
> + * @probe: Callback for device binding
> + * @remove: Callback for device unbinding
> + * @shutdown: Callback for device shutdown
> + * @suspend: Callback for device suspend
> + * @resume: Callback for device resume
> + * @command: Callback for sending commands to device
> + * @id_table: List of TDM devices supported by this driver
> + * @list: List of drivers created (for tdm-core use only) */ struct
> +tdm_driver {
> + unsigned int class;
> + unsigned int id;
> + char name[TDM_NAME_SIZE];
> +
> + int (*attach_adapter)(struct tdm_adapter *);
> + int (*detach_adapter)(struct tdm_adapter *);
> +
> + /* Standard driver model interfaces */
> + int (*probe)(const struct tdm_device_id *);
> + int (*remove)(void);
> +
> + /* driver model interfaces that don't relate to enumeration */
> + void (*shutdown)(void);
> + int (*suspend)(pm_message_t mesg);
> + int (*resume)(void);
> +
> + /* a ioctl like command that can be used to perform specific
> functions
> + * with the device.
> + */
> + int (*command)(unsigned int cmd, void *arg);
> +
> + const struct tdm_device_id *id_table;
> +
> + /* The associated adapter for this driver */
> + struct tdm_adapter *adapter;
> + struct list_head list;
> +};
> +
> +/* tdm per port statistics structure, used for providing and storing
> +tdm port
> + * statistics.
> + */
> +struct tdm_port_stats {
> + unsigned int rx_pkt_count; /* Rx frame count per channel */
> + unsigned int rx_pkt_drop_count; /* Rx drop count per channel to
> + clean space for new buffer */
> + unsigned int tx_pkt_count; /* Tx frame count per channel */
> + unsigned int tx_pkt_conf_count; /* Tx frame confirmation count
> per
> + channel */
> + unsigned int tx_pkt_drop_count; /* Tx drop count per channel
> due to
> + queue full */
> +};
> +
> +
> +/* tdm Buffer Descriptor, used for Creating Interleaved and
> +De-interleaved
> + * FIFOs
> + */
> +struct tdm_bd {
> + unsigned char flag; /* BD is full or empty */
> + unsigned char wrap; /* BD is last in the queue */
> + unsigned short length; /* Length of Data in BD */
> + /*TODO: use dyanmic memory */
> + unsigned short p_data[NUM_SAMPLES_PER_FRAME]; /* Data Pointer */
> + unsigned long offset; /* Offset of the Data Pointer to be used */
> +};
> +
> +#define TDM_CH_RX_BD_RING_SIZE 3
> +#define TDM_CH_TX_BD_RING_SIZE 3
> +
> +/* tdm RX-TX Channelised Data */
> +struct tdm_port_data {
> + struct tdm_bd rx_data_fifo[TDM_CH_RX_BD_RING_SIZE]; /* Rx Channel
> Data
> + BD Ring */
> + struct tdm_bd *rx_in_data; /* Current Channel Rx BD to be filled
> by
> + de-interleave function */
> + struct tdm_bd *rx_out_data; /* Current Channel Rx BD to be
> + read by App */
> + struct tdm_bd tx_data_fifo[TDM_CH_TX_BD_RING_SIZE]; /* Tx Channel
> Data
> + BD Ring */
> + struct tdm_bd *tx_in_data; /* Current Channel Tx BD to be
> + filled by App */
> + struct tdm_bd *tx_out_data; /* Current Channel Tx BD to be read
> by
> + interleave function */
> + spinlock_t rx_channel_lock; /* Spin Lock for Rx Channel */
> + spinlock_t tx_channel_lock; /* Spin Lock for Tx Channel */
> +};
> +
> +/* structure tdm_port_cfg - contains configuration params for a port */
> +struct tdm_port_cfg {
> + unsigned short port_mode;
> +};
> +
> +/* struct tdm_port - represent an TDM ports for a device */ struct
> +tdm_port {
> + unsigned short port_id;
> + unsigned short in_use; /* Port is enabled? */
> + uint16_t rx_max_frames; /* Received Port frames
> + before allowing Read Operation in
> + Port Mode */
> +
> + struct tdm_port_stats port_stat;/* A structure parameters defining
> + TDM port statistics. */
> + struct tdm_port_data *p_port_data; /* a structure parameters
> + defining tdm channelised data */
> + wait_queue_head_t ch_wait_queue; /* waitQueue for RX Port Data
> */
> +
> + struct tdm_driver *driver; /* driver for this port */
> + struct tdm_adapter *adapter; /* adapter for this port */
> + struct list_head list; /* list of ports */
> + struct list_head mychannels; /* list of channels, created on this
> + port*/
> + spinlock_t ch_list_lock; /* Spin Lock for channel_list */
> + struct tdm_port_cfg port_cfg;/* A structure parameters defining
> + TDM port configuration. */
> +};
> +
> +/* tdm RX-TX Channelised Data */
> +struct tdm_ch_data {
> + struct tdm_bd rx_data_fifo[TDM_CH_RX_BD_RING_SIZE]; /* Rx Port Data
> BD
> + Ring */
> + struct tdm_bd *rx_in_data; /* Current Port Rx BD to be filled by
> + de-interleave function */
> + struct tdm_bd *rx_out_data; /* Current Port Rx BD to be read by App
> */
> + struct tdm_bd tx_data_fifo[TDM_CH_TX_BD_RING_SIZE]; /* Tx Port Data
> BD
> + Ring */
> + struct tdm_bd *tx_in_data; /* Current Port Tx BD to be filled by
> + App */
> + struct tdm_bd *tx_out_data; /* Current Port Tx BD to be read by
> + interleave function */
> + spinlock_t rx_channel_lock; /* Spin Lock for Rx Port */
> + spinlock_t tx_channel_lock; /* Spin Lock for Tx Port */
> +};
> +
> +/* Channel config params */
> +struct tdm_ch_cfg {
> + unsigned short num_slots;
> + unsigned short first_slot;
> +};
> +
> +/* struct tdm_channel- represent a TDM channel for a port */ struct
> +tdm_channel {
> + u16 ch_id; /* logical channel number */
> + struct list_head list; /* list of channels in a port*/
> + struct tdm_port *port; /* port for this channel */
> + u16 in_use; /* channel is enabled? */
> + struct tdm_ch_cfg ch_cfg; /* channel configuration */
> + struct tdm_ch_data *p_ch_data; /* data storage space for
> channel */
> +};
> +
> +/* tdm_adapt_algorithm is for accessing the routines of device */
> +struct tdm_adapt_algorithm {
> + u32 (*tdm_read)(struct tdm_adapter *, u16 **);
> + u32 (*tdm_get_write_buf)(struct tdm_adapter *, u16 **);
> + u32 (*tdm_write)(struct tdm_adapter *, void * , unsigned int len);
> + int (*tdm_enable)(struct tdm_adapter *);
> + int (*tdm_disable)(struct tdm_adapter *); };
> +
> +/* tdm_adapter_mode is to define in mode of the device */ enum
> +tdm_adapter_mode {
> + e_TDM_ADAPTER_MODE_NONE =3D 0x00,
> + e_TDM_ADAPTER_MODE_T1 =3D 0x01,
> + e_TDM_ADAPTER_MODE_E1 =3D 0x02,
> + e_TDM_ADAPTER_MODE_T1_RAW =3D 0x10,
> + e_TDM_ADAPTER_MODE_E1_RAW =3D 0x20,
> +};
> +
> +/* tdm_port_mode defines the mode in which the port is configured to
> +operate
> + * It can be channelized/full/fractional.
> + */
> +enum tdm_port_mode {
> + e_TDM_PORT_CHANNELIZED =3D 0 /* Channelized mode */
> + , e_TDM_PORT_FULL =3D 1 /* Full mode */
> + , e_TDM_PORT_FRACTIONAL =3D 2 /* Fractional mode */
> +};
> +
> +/* tdm_process_mode used for testing the tdm device in normal mode or
> +internal
> + * loopback or external loopback
> + */
> +enum tdm_process_mode {
> + e_TDM_PROCESS_NORMAL =3D 0 /* Normal mode */
> + , e_TDM_PROCESS_INT_LPB =3D 1 /* Internal loop mode */
> + , e_TDM_PROCESS_EXT_LPB =3D 2 /* External Loopback mode */
> +};
> +
> +
> +/* TDM configuration parameters */
> +struct fsl_tdm_adapt_cfg {
> + u8 num_ch; /* Number of channels in this adpater */
> + u8 ch_size_type; /* reciever/transmit channel
> + size for all channels */
> + u8 slot_width; /* 1 or 2 Is defined by channel type */
> + u8 frame_len; /* Length of frame in samples */
> + u32 num_frames;
> + u8 loopback; /* loopback or normal */
> + u8 adap_mode; /* 0=3DNone, 1=3D T1, 2=3D T1-FULL, 3=3DE1,
> + 4 =3D E1-FULL */
> + int max_num_ports; /* Not Used: Max Number of ports that
> + can be created on this adapter */
> + int max_timeslots; /* Max Number of timeslots that are
> + supported on this adapter */
> +};
> +
> +/*
> + * tdm_adapter is the structure used to identify a physical tdm device
> +along
> + * with the access algorithms necessary to access it.
> + */
> +struct tdm_adapter {
> + struct module *owner; /* owner of the adapter module */
> + unsigned int id; /* Adapter Id */
> + unsigned int class; /* classes to allow probing for */
> + unsigned int drv_count; /* Number of drivers associated with the
> + adapter */
> +
> + const struct tdm_adapt_algorithm *algo; /* the algorithm to
> access the
> + adapter*/
> +
> + char name[TDM_NAME_SIZE]; /* Name of Adapter */
> + struct mutex adap_lock;
> + struct device *parent; /*Not Used*/
> +
> + struct tasklet_struct tdm_data_tasklet; /* tasklet handle to
> perform
> + data processing*/
> + int tasklet_conf; /* flag for tasklet configuration */
> + int tdm_rx_flag;
> +
> + struct list_head myports; /* list of ports, created on this
> + adapter */
> + struct list_head list;
> + spinlock_t portlist_lock; /* Spin Lock for port_list */
> + void *data;
> + struct fsl_tdm_adapt_cfg adapt_cfg;
> +};
> +
> +static inline void *tdm_get_adapdata(const struct tdm_adapter *dev) {
> + return dev->data;
> +}
> +
> +static inline void tdm_set_adapdata(struct tdm_adapter *dev, void
> +*data) {
> + dev->data =3D data;
> +}
> +
> +/* functions exported by tdm.o */
> +
> +extern int tdm_add_adapter(struct tdm_adapter *); extern int
> +tdm_del_adapter(struct tdm_adapter *); extern int
> +tdm_register_driver(struct tdm_driver *); extern void
> +tdm_del_driver(struct tdm_driver *); extern void
> +tdm_unregister_driver(struct tdm_driver *); extern void
> +init_config_adapter(struct tdm_adapter *);
> +
> +extern unsigned int tdm_port_open(struct tdm_driver *, void **); extern
> +unsigned int tdm_port_close(void *); extern unsigned int
> +tdm_port_ioctl(void *, unsigned int, unsigned long); extern unsigned
> +int tdm_channel_read(void *, void *, void *, u16 *); extern unsigned
> +int tdm_channel_write(void *, void * , void *, u16); extern unsigned
> +int tdm_port_poll(void *, unsigned int);
> +
> +extern int tdm_channel_open(u16, u16, struct tdm_port *, void **);
> +extern int tdm_channel_close(u16, u16, struct tdm_port *,
> + struct tdm_channel *);
> +
> +static inline int tdm_add_driver(struct tdm_driver *driver) {
> + return tdm_register_driver(driver);
> +}
> +
> +extern struct tdm_adapter *tdm_get_adapter(int id); extern void
> +tdm_put_adapter(struct tdm_adapter *adap);
> +
> +#endif /* __KERNEL__ */
> +
> +#define TDM_E_OK 0
> +
> +#endif /* _LINUX_TDM_H */
> --
> 1.6.5.6
^ permalink raw reply
* [PATCH 9/9] um: Properly check all process' threads for a live mm
From: Anton Vorontsov @ 2012-04-23 7:09 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: linaro-kernel, Mike Frysinger, Peter Zijlstra,
user-mode-linux-devel, linux-sh, Richard Weinberger, patches,
linux-kernel, uclinux-dist-devel, linux-mm, Paul Mundt,
John Stultz, KOSAKI Motohiro, Russell King, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20120423070641.GA27702@lizard>
kill_off_processes() might miss a valid process, this is because
checking for process->mm is not enough. Process' main thread may
exit or detach its mm via use_mm(), but other threads may still
have a valid mm.
To catch this we use find_lock_task_mm(), which walks up all
threads and returns an appropriate task (with task lock held).
Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
arch/um/kernel/reboot.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/arch/um/kernel/reboot.c b/arch/um/kernel/reboot.c
index 1411f4e..3d15243 100644
--- a/arch/um/kernel/reboot.c
+++ b/arch/um/kernel/reboot.c
@@ -6,6 +6,7 @@
#include "linux/sched.h"
#include "linux/spinlock.h"
#include "linux/slab.h"
+#include "linux/oom.h"
#include "kern_util.h"
#include "os.h"
#include "skas.h"
@@ -25,13 +26,13 @@ static void kill_off_processes(void)
read_lock(&tasklist_lock);
for_each_process(p) {
- task_lock(p);
- if (!p->mm) {
- task_unlock(p);
+ struct task_struct *t;
+
+ t = find_lock_task_mm(p);
+ if (!t)
continue;
- }
- pid = p->mm->context.id.u.pid;
- task_unlock(p);
+ pid = t->mm->context.id.u.pid;
+ task_unlock(t);
os_kill_ptraced_process(pid, 1);
}
read_unlock(&tasklist_lock);
--
1.7.9.2
^ permalink raw reply related
* [PATCH 8/9] um: Fix possible race on task->mm
From: Anton Vorontsov @ 2012-04-23 7:09 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: linaro-kernel, Mike Frysinger, Peter Zijlstra,
user-mode-linux-devel, linux-sh, Richard Weinberger, patches,
linux-kernel, uclinux-dist-devel, linux-mm, Paul Mundt,
John Stultz, KOSAKI Motohiro, Russell King, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20120423070641.GA27702@lizard>
Checking for task->mm is dangerous as ->mm might disappear (exit_mm()
assigns NULL under task_lock(), so tasklist lock is not enough).
We can't use get_task_mm()/mmput() pair as mmput() might sleep,
so let's take the task lock while we care about its mm.
Note that we should also use find_lock_task_mm() to check all process'
threads for a valid mm, but for uml we'll do it in a separate patch.
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
arch/um/kernel/reboot.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/um/kernel/reboot.c b/arch/um/kernel/reboot.c
index 66d754c..1411f4e 100644
--- a/arch/um/kernel/reboot.c
+++ b/arch/um/kernel/reboot.c
@@ -25,10 +25,13 @@ static void kill_off_processes(void)
read_lock(&tasklist_lock);
for_each_process(p) {
- if (p->mm == NULL)
+ task_lock(p);
+ if (!p->mm) {
+ task_unlock(p);
continue;
-
+ }
pid = p->mm->context.id.u.pid;
+ task_unlock(p);
os_kill_ptraced_process(pid, 1);
}
read_unlock(&tasklist_lock);
--
1.7.9.2
^ permalink raw reply related
* [PATCH 7/9] um: Should hold tasklist_lock while traversing processes
From: Anton Vorontsov @ 2012-04-23 7:09 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: linaro-kernel, Mike Frysinger, Peter Zijlstra,
user-mode-linux-devel, linux-sh, Richard Weinberger, patches,
linux-kernel, uclinux-dist-devel, linux-mm, Paul Mundt,
John Stultz, KOSAKI Motohiro, Russell King, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20120423070641.GA27702@lizard>
Traversing the tasks requires holding tasklist_lock, otherwise it
is unsafe.
p.s. However, I'm not sure that calling os_kill_ptraced_process()
in the atomic context is correct. It seem to work, but please
take a closer look.
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
arch/um/kernel/reboot.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/um/kernel/reboot.c b/arch/um/kernel/reboot.c
index 4d93dff..66d754c 100644
--- a/arch/um/kernel/reboot.c
+++ b/arch/um/kernel/reboot.c
@@ -4,6 +4,7 @@
*/
#include "linux/sched.h"
+#include "linux/spinlock.h"
#include "linux/slab.h"
#include "kern_util.h"
#include "os.h"
@@ -22,6 +23,7 @@ static void kill_off_processes(void)
struct task_struct *p;
int pid;
+ read_lock(&tasklist_lock);
for_each_process(p) {
if (p->mm == NULL)
continue;
@@ -29,6 +31,7 @@ static void kill_off_processes(void)
pid = p->mm->context.id.u.pid;
os_kill_ptraced_process(pid, 1);
}
+ read_unlock(&tasklist_lock);
}
}
--
1.7.9.2
^ permalink raw reply related
* [PATCH 6/9] blackfin: Fix possible deadlock in decode_address()
From: Anton Vorontsov @ 2012-04-23 7:09 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: linaro-kernel, Mike Frysinger, Peter Zijlstra,
user-mode-linux-devel, linux-sh, Richard Weinberger, patches,
linux-kernel, uclinux-dist-devel, linux-mm, Paul Mundt,
John Stultz, KOSAKI Motohiro, Russell King, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20120423070641.GA27702@lizard>
Oleg Nesterov found an interesting deadlock possibility:
> sysrq_showregs_othercpus() does smp_call_function(showacpu)
> and showacpu() show_stack()->decode_address(). Now suppose that IPI
> interrupts the task holding read_lock(tasklist).
To fix this, blackfin should not grab the write_ variant of the
tasklist lock, read_ one is enough.
Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
arch/blackfin/kernel/trace.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/blackfin/kernel/trace.c b/arch/blackfin/kernel/trace.c
index d08f0e3..f7f7a18 100644
--- a/arch/blackfin/kernel/trace.c
+++ b/arch/blackfin/kernel/trace.c
@@ -29,7 +29,7 @@ void decode_address(char *buf, unsigned long address)
{
struct task_struct *p;
struct mm_struct *mm;
- unsigned long flags, offset;
+ unsigned long offset;
struct rb_node *n;
#ifdef CONFIG_KALLSYMS
@@ -113,7 +113,7 @@ void decode_address(char *buf, unsigned long address)
* mappings of all our processes and see if we can't be a whee
* bit more specific
*/
- write_lock_irqsave(&tasklist_lock, flags);
+ read_lock(&tasklist_lock);
for_each_process(p) {
struct task_struct *t;
@@ -186,7 +186,7 @@ __continue:
sprintf(buf, "/* kernel dynamic memory */");
done:
- write_unlock_irqrestore(&tasklist_lock, flags);
+ read_unlock(&tasklist_lock);
}
#define EXPAND_LEN ((1 << CONFIG_DEBUG_BFIN_HWTRACE_EXPAND_LEN) * 256 - 1)
--
1.7.9.2
^ permalink raw reply related
* [PATCH 5/9] blackfin: A couple of task->mm handling fixes
From: Anton Vorontsov @ 2012-04-23 7:09 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: linaro-kernel, Mike Frysinger, Peter Zijlstra,
user-mode-linux-devel, linux-sh, Richard Weinberger, patches,
linux-kernel, uclinux-dist-devel, linux-mm, Paul Mundt,
John Stultz, KOSAKI Motohiro, Russell King, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20120423070641.GA27702@lizard>
The patch fixes two problems:
1. Working with task->mm w/o getting mm or grabing the task lock is
dangerous as ->mm might disappear (exit_mm() assigns NULL under
task_lock(), so tasklist lock is not enough).
We can't use get_task_mm()/mmput() pair as mmput() might sleep,
so we have to take the task lock while handle its mm.
2. Checking for process->mm is not enough because process' main
thread may exit or detach its mm via use_mm(), but other threads
may still have a valid mm.
To catch this we use find_lock_task_mm(), which walks up all
threads and returns an appropriate task (with task lock held).
Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
arch/blackfin/kernel/trace.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/arch/blackfin/kernel/trace.c b/arch/blackfin/kernel/trace.c
index 44bbf2f..d08f0e3 100644
--- a/arch/blackfin/kernel/trace.c
+++ b/arch/blackfin/kernel/trace.c
@@ -10,6 +10,8 @@
#include <linux/hardirq.h>
#include <linux/thread_info.h>
#include <linux/mm.h>
+#include <linux/oom.h>
+#include <linux/sched.h>
#include <linux/uaccess.h>
#include <linux/module.h>
#include <linux/kallsyms.h>
@@ -28,7 +30,6 @@ void decode_address(char *buf, unsigned long address)
struct task_struct *p;
struct mm_struct *mm;
unsigned long flags, offset;
- unsigned char in_atomic = (bfin_read_IPEND() & 0x10) || in_atomic();
struct rb_node *n;
#ifdef CONFIG_KALLSYMS
@@ -114,15 +115,15 @@ void decode_address(char *buf, unsigned long address)
*/
write_lock_irqsave(&tasklist_lock, flags);
for_each_process(p) {
- mm = (in_atomic ? p->mm : get_task_mm(p));
- if (!mm)
- continue;
+ struct task_struct *t;
- if (!down_read_trylock(&mm->mmap_sem)) {
- if (!in_atomic)
- mmput(mm);
+ t = find_lock_task_mm(p);
+ if (!t)
continue;
- }
+
+ mm = t->mm;
+ if (!down_read_trylock(&mm->mmap_sem))
+ goto __continue;
for (n = rb_first(&mm->mm_rb); n; n = rb_next(n)) {
struct vm_area_struct *vma;
@@ -131,7 +132,7 @@ void decode_address(char *buf, unsigned long address)
if (address >= vma->vm_start && address < vma->vm_end) {
char _tmpbuf[256];
- char *name = p->comm;
+ char *name = t->comm;
struct file *file = vma->vm_file;
if (file) {
@@ -164,8 +165,7 @@ void decode_address(char *buf, unsigned long address)
name, vma->vm_start, vma->vm_end);
up_read(&mm->mmap_sem);
- if (!in_atomic)
- mmput(mm);
+ task_unlock(t);
if (buf[0] == '\0')
sprintf(buf, "[ %s ] dynamic memory", name);
@@ -175,8 +175,8 @@ void decode_address(char *buf, unsigned long address)
}
up_read(&mm->mmap_sem);
- if (!in_atomic)
- mmput(mm);
+__continue:
+ task_unlock(t);
}
/*
--
1.7.9.2
^ permalink raw reply related
* [PATCH 4/9] sh: Use clear_tasks_mm_cpumask()
From: Anton Vorontsov @ 2012-04-23 7:08 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: linaro-kernel, Mike Frysinger, Peter Zijlstra,
user-mode-linux-devel, linux-sh, Richard Weinberger, patches,
linux-kernel, uclinux-dist-devel, linux-mm, Paul Mundt,
John Stultz, KOSAKI Motohiro, Russell King, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20120423070641.GA27702@lizard>
Checking for process->mm is not enough because process' main
thread may exit or detach its mm via use_mm(), but other threads
may still have a valid mm.
To fix this we would need to use find_lock_task_mm(), which would
walk up all threads and returns an appropriate task (with task
lock held).
clear_tasks_mm_cpumask() has the issue fixed, so let's use it.
Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
arch/sh/kernel/smp.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/arch/sh/kernel/smp.c b/arch/sh/kernel/smp.c
index eaebdf6..4664f76 100644
--- a/arch/sh/kernel/smp.c
+++ b/arch/sh/kernel/smp.c
@@ -123,7 +123,6 @@ void native_play_dead(void)
int __cpu_disable(void)
{
unsigned int cpu = smp_processor_id();
- struct task_struct *p;
int ret;
ret = mp_ops->cpu_disable(cpu);
@@ -153,11 +152,7 @@ int __cpu_disable(void)
flush_cache_all();
local_flush_tlb_all();
- read_lock(&tasklist_lock);
- for_each_process(p)
- if (p->mm)
- cpumask_clear_cpu(cpu, mm_cpumask(p->mm));
- read_unlock(&tasklist_lock);
+ clear_tasks_mm_cpumask(cpu);
return 0;
}
--
1.7.9.2
^ permalink raw reply related
* [PATCH 3/9] powerpc: Use clear_tasks_mm_cpumask()
From: Anton Vorontsov @ 2012-04-23 7:08 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: linaro-kernel, Mike Frysinger, Peter Zijlstra,
user-mode-linux-devel, linux-sh, Richard Weinberger, patches,
linux-kernel, uclinux-dist-devel, linux-mm, Paul Mundt,
John Stultz, KOSAKI Motohiro, Russell King, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20120423070641.GA27702@lizard>
Current CPU hotplug code has some task->mm handling issues:
1. Working with task->mm w/o getting mm or grabing the task lock is
dangerous as ->mm might disappear (exit_mm() assigns NULL under
task_lock(), so tasklist lock is not enough).
We can't use get_task_mm()/mmput() pair as mmput() might sleep,
so we must take the task lock while handle its mm.
2. Checking for process->mm is not enough because process' main
thread may exit or detach its mm via use_mm(), but other threads
may still have a valid mm.
To fix this we would need to use find_lock_task_mm(), which would
walk up all threads and returns an appropriate task (with task
lock held).
clear_tasks_mm_cpumask() has all the issues fixed, so let's use it.
Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
arch/powerpc/mm/mmu_context_nohash.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/mm/mmu_context_nohash.c b/arch/powerpc/mm/mmu_context_nohash.c
index 5b63bd3..e779642 100644
--- a/arch/powerpc/mm/mmu_context_nohash.c
+++ b/arch/powerpc/mm/mmu_context_nohash.c
@@ -333,9 +333,7 @@ static int __cpuinit mmu_context_cpu_notify(struct notifier_block *self,
unsigned long action, void *hcpu)
{
unsigned int cpu = (unsigned int)(long)hcpu;
-#ifdef CONFIG_HOTPLUG_CPU
- struct task_struct *p;
-#endif
+
/* We don't touch CPU 0 map, it's allocated at aboot and kept
* around forever
*/
@@ -358,12 +356,7 @@ static int __cpuinit mmu_context_cpu_notify(struct notifier_block *self,
stale_map[cpu] = NULL;
/* We also clear the cpu_vm_mask bits of CPUs going away */
- read_lock(&tasklist_lock);
- for_each_process(p) {
- if (p->mm)
- cpumask_clear_cpu(cpu, mm_cpumask(p->mm));
- }
- read_unlock(&tasklist_lock);
+ clear_tasks_mm_cpumask(cpu);
break;
#endif /* CONFIG_HOTPLUG_CPU */
}
--
1.7.9.2
^ permalink raw reply related
* [PATCH 2/9] arm: Use clear_tasks_mm_cpumask()
From: Anton Vorontsov @ 2012-04-23 7:08 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: linaro-kernel, Mike Frysinger, Peter Zijlstra,
user-mode-linux-devel, linux-sh, Richard Weinberger, patches,
linux-kernel, uclinux-dist-devel, linux-mm, Paul Mundt,
John Stultz, KOSAKI Motohiro, Russell King, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20120423070641.GA27702@lizard>
Checking for process->mm is not enough because process' main
thread may exit or detach its mm via use_mm(), but other threads
may still have a valid mm.
To fix this we would need to use find_lock_task_mm(), which would
walk up all threads and returns an appropriate task (with task
lock held).
clear_tasks_mm_cpumask() has this issue fixed, so let's use it.
Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
arch/arm/kernel/smp.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index addbbe8..e824ee4 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -130,7 +130,6 @@ static void percpu_timer_stop(void);
int __cpu_disable(void)
{
unsigned int cpu = smp_processor_id();
- struct task_struct *p;
int ret;
ret = platform_cpu_disable(cpu);
@@ -160,12 +159,7 @@ int __cpu_disable(void)
flush_cache_all();
local_flush_tlb_all();
- read_lock(&tasklist_lock);
- for_each_process(p) {
- if (p->mm)
- cpumask_clear_cpu(cpu, mm_cpumask(p->mm));
- }
- read_unlock(&tasklist_lock);
+ clear_tasks_mm_cpumask(cpu);
return 0;
}
--
1.7.9.2
^ permalink raw reply related
* [PATCH 1/9] cpu: Introduce clear_tasks_mm_cpumask() helper
From: Anton Vorontsov @ 2012-04-23 7:07 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: linaro-kernel, Mike Frysinger, Peter Zijlstra,
user-mode-linux-devel, linux-sh, Richard Weinberger, patches,
linux-kernel, uclinux-dist-devel, linux-mm, Paul Mundt,
John Stultz, KOSAKI Motohiro, Russell King, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20120423070641.GA27702@lizard>
Many architectures clear tasks' mm_cpumask like this:
read_lock(&tasklist_lock);
for_each_process(p) {
if (p->mm)
cpumask_clear_cpu(cpu, mm_cpumask(p->mm));
}
read_unlock(&tasklist_lock);
Depending on the context, the code above may have several problems,
such as:
1. Working with task->mm w/o getting mm or grabing the task lock is
dangerous as ->mm might disappear (exit_mm() assigns NULL under
task_lock(), so tasklist lock is not enough).
2. Checking for process->mm is not enough because process' main
thread may exit or detach its mm via use_mm(), but other threads
may still have a valid mm.
This patch implements a small helper function that does things
correctly, i.e.:
1. We take the task's lock while whe handle its mm (we can't use
get_task_mm()/mmput() pair as mmput() might sleep);
2. To catch exited main thread case, we use find_lock_task_mm(),
which walks up all threads and returns an appropriate task
(with task lock held).
Also, Per Peter Zijlstra's idea, now we don't grab tasklist_lock in
the new helper, instead we take the rcu read lock. We can do this
because the function is called after the cpu is taken down and marked
offline, so no new tasks will get this cpu set in their mm mask.
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
include/linux/cpu.h | 1 +
kernel/cpu.c | 26 ++++++++++++++++++++++++++
2 files changed, 27 insertions(+)
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index ee28844..d2ca49f 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -179,6 +179,7 @@ extern void put_online_cpus(void);
#define hotcpu_notifier(fn, pri) cpu_notifier(fn, pri)
#define register_hotcpu_notifier(nb) register_cpu_notifier(nb)
#define unregister_hotcpu_notifier(nb) unregister_cpu_notifier(nb)
+void clear_tasks_mm_cpumask(int cpu);
int cpu_down(unsigned int cpu);
#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 2060c6e..ecdf499 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -10,6 +10,8 @@
#include <linux/sched.h>
#include <linux/unistd.h>
#include <linux/cpu.h>
+#include <linux/oom.h>
+#include <linux/rcupdate.h>
#include <linux/export.h>
#include <linux/kthread.h>
#include <linux/stop_machine.h>
@@ -171,6 +173,30 @@ void __ref unregister_cpu_notifier(struct notifier_block *nb)
}
EXPORT_SYMBOL(unregister_cpu_notifier);
+void clear_tasks_mm_cpumask(int cpu)
+{
+ struct task_struct *p;
+
+ /*
+ * This function is called after the cpu is taken down and marked
+ * offline, so its not like new tasks will ever get this cpu set in
+ * their mm mask. -- Peter Zijlstra
+ * Thus, we may use rcu_read_lock() here, instead of grabbing
+ * full-fledged tasklist_lock.
+ */
+ rcu_read_lock();
+ for_each_process(p) {
+ struct task_struct *t;
+
+ t = find_lock_task_mm(p);
+ if (!t)
+ continue;
+ cpumask_clear_cpu(cpu, mm_cpumask(t->mm));
+ task_unlock(t);
+ }
+ rcu_read_unlock();
+}
+
static inline void check_for_tasks(int cpu)
{
struct task_struct *p;
--
1.7.9.2
^ permalink raw reply related
* [PATCH v3 0/9] Fixes for common mistakes w/ for_each_process and task->mm
From: Anton Vorontsov @ 2012-04-23 7:06 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: linaro-kernel, Mike Frysinger, Peter Zijlstra,
user-mode-linux-devel, linux-sh, Richard Weinberger, patches,
linux-kernel, uclinux-dist-devel, linux-mm, Paul Mundt,
John Stultz, KOSAKI Motohiro, Russell King, linuxppc-dev,
linux-arm-kernel
Hi all,
This is another resend of several task->mm fixes, the bugs I found
during LMK code audit. Architectures were traverse the tasklist
in an unsafe manner, plus there are a few cases of unsafe access to
task->mm in general.
There were no objections on the previous resend, and the final words
were somewhere along "the patches are fine" line.
In v3:
- Dropped a controversal 'Make find_lock_task_mm() sparse-aware' patch;
- Reword arm and sh commit messages, per Oleg Nesterov's suggestions;
- Added an optimization trick in clear_tasks_mm_cpumask(): take only
the rcu read lock, no need for the whole tasklist_lock.
Suggested by Peter Zijlstra.
In v2:
- introduced a small helper in cpu.c: most arches duplicate the
same [buggy] code snippet, so it's better to fix it and move the
logic into a common function.
Thanks,
--
Anton Vorontsov
Email: cbouatmailru@gmail.com
^ permalink raw reply
* RE: [RFC] powerpc/fsl-pci: Document the "fsl,has-isa" property for Freescale PCI
From: Jia Hongtao-B38951 @ 2012-04-23 3:34 UTC (permalink / raw)
To: Jia Hongtao-B38951, Kumar Gala, Benjamin Herrenschmidt
Cc: devicetree-discuss@lists.ozlabs.org,
linuxppc-dev@lists.ozlabs.org, Li Yang-R58472
In-Reply-To: <412C8208B4A0464FA894C5F0C278CD5D019DEA96@039-SN1MPN1-002.039d.mgd.msft.net>
> -----Original Message-----
> From: Jia Hongtao-B38951
> Sent: Tuesday, April 10, 2012 5:17 PM
> To: Kumar Gala
> Cc: devicetree-discuss@lists.ozlabs.org; linuxppc-dev@lists.ozlabs.org;
> Li Yang-R58472; Jia Hongtao-B38951
> Subject: RE: [RFC] powerpc/fsl-pci: Document the "fsl,has-isa" property
> for Freescale PCI
>=20
>=20
> > -----Original Message-----
> > From: linuxppc-dev-bounces+b38951=3Dfreescale.com@lists.ozlabs.org
> > [mailto:linuxppc-dev-bounces+b38951=3Dfreescale.com@lists.ozlabs.org] O=
n
> > Behalf Of Jia Hongtao-B38951
> > Sent: Friday, April 06, 2012 11:05 AM
> > To: Kumar Gala
> > Cc: devicetree-discuss@lists.ozlabs.org; linuxppc-dev@lists.ozlabs.org;
> > Li Yang-R58472
> > Subject: RE: [RFC] powerpc/fsl-pci: Document the "fsl,has-isa" property
> > for Freescale PCI
> >
> >
> > > -----Original Message-----
> > > From: Kumar Gala [mailto:galak@kernel.crashing.org]
> > > Sent: Wednesday, April 04, 2012 9:09 PM
> > > To: Jia Hongtao-B38951
> > > Cc: linuxppc-dev@lists.ozlabs.org; Li Yang-R58472; devicetree-
> > > discuss@lists.ozlabs.org
> > > Subject: Re: [RFC] powerpc/fsl-pci: Document the "fsl,has-isa"
> property
> > > for Freescale PCI
> > >
> > >
> > > On Apr 1, 2012, at 1:56 AM, Jia Hongtao wrote:
> > >
> > > > If PCI is primary bus we should set isa_io/mem_base when parsing
> PCI
> > > bridge
> > > > resources from device tree. The previous way to check the primary
> bus
> > > based
> > > > on a hard-coded address named primary_phb_addr. Now we add a
> property
> > > named
> > > > "fsl,has-isa" into device tree. In kernel we use this property to
> > find
> > > out
> > > > the bus is primary or not. This way is more flexible.
> > > >
> > > > Signed-off-by: Jia Hongtao <B38951@freescale.com>
> > > > Signed-off-by: Li Yang <leoli@freescale.com>
> > > > ---
> > > > .../devicetree/bindings/powerpc/fsl/pci.txt | 36
> > > ++++++++++++++++++++
> > > > 1 files changed, 36 insertions(+), 0 deletions(-)
> > > > create mode 100644
> > > Documentation/devicetree/bindings/powerpc/fsl/pci.txt
> > >
> > > This isn't freescale specific, its linux specific. If anything the
> > > property should be linux,has-isa.
> > >
> > > But in general I dont think this is a good idea. In truth one could
> > > search the device tree for:
> > >
> > > device_type =3D "isa";
> > >
> > > to try and set this dynamically.
> > >
> > > - k
> > >
> >
> > Yes, it's not Freescale specific, but it's only used by Freescale now
> in
> > the kernel. This is why I named the property as "fsl,has-isa".
> >
> > To indicate PCI bus is primary we have three ways to go and we now like
> > the 2nd solution:
> >
> > 1. As this patch said, we add a property to device tree manually.
> >
> > 2. Set this property dynamically in uboot when scanning PCI bridge.
> > Actually we have already done this. The problem is users should update
> > uboot and kernel together or it's not work. To support old uboot we
> > decide
> > to add this property into device tree too temporarily. We will remove
> it
> > from device tree at an appropriate future.
> >
> > 3. Just as you said we could search the device tree by device_type =3D
> > "isa".
> > There are two problems here:
> > * There is no OF API for searching just in PCI node now. That means
> > we can't easily find whether there is "isa" bridge or not under
> > this PCI controller while scanning it.
> > * Boards MPC8541CDS and MPC8555CDS has no "isa" node in device tree
> > but has ISA bridge under PCI controller.
> >
> > - Jia Hongtao
> >
>=20
> Hi Kumar,
>=20
> I agree with the name change from "fsl,has-isa" to "linux,has-isa".
> If this is ok I will update this patch and send the patches based on this
> new property.
>=20
> Also, I have a question about "isa" node in dts. Is this node being used
> by kernel? If not we can remove it from all boards and check the primary
> bus by uboot. If "isa" is useful why MPC8541CDS and MPC8555CDS has no
> "isa" node in device tree but has ISA bridge under PCI controller?
>=20
> Thanks.
> - Jia Hongtao
Hi Kumar and Ben,
Do you have any idea on this?
Thanks.
- Jia Hongtao
^ permalink raw reply
* [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2012-04-23 2:04 UTC (permalink / raw)
To: Linus Torvalds
Cc: linuxppc-dev list, Andrew Morton, Thomas Gleixner,
Linux Kernel list
Hi Linus !
Here are a few fixes for powerpc. Note the addition to the generic
irq.h. This is part of a 3-patches regression fix for mpic due to
changes in how IRQ_TYPE_NONE is being handled. Thomas agreed to the
addition of the new IRQ_TYPE_DEFAULT contant, however he hasn't
replied with an Ack to the actual patch yet. I don't to wait much
longer with these patches tho.
Cheers,
Ben.
The following changes since commit 932e9f352b5d685725076f21b237f7c7d804b29c:
Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security (2012-04-18 20:16:02 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git merge
for you to fetch changes up to 446f6d06fab0b49c61887ecbe8286d6aaa796637:
powerpc/mpic: Properly set default triggers (2012-04-23 11:04:30 +1000)
----------------------------------------------------------------
Baruch Siach (1):
powerpc: fix build when CONFIG_BOOKE_WDT is enabled
Benjamin Herrenschmidt (5):
Merge remote-tracking branch 'kumar/merge' into merge
powerpc/pmac: Don't add_timer() twice
powerpc/mpic: Fix confusion between hw_irq and virq
irq: Add IRQ_TYPE_DEFAULT for use by PIC drivers
powerpc/mpic: Properly set default triggers
Gavin Shan (1):
powerpc/eeh: Fix crash caused by null eeh_dev
Mingkai Hu (4):
powerpc/mpic_msgr: fix compile error when SMP disabled
powerpc/mpic_msgr: add lock for MPIC message global variable
powerpc/mpic_msgr: fix offset error when setting mer register
powerpc/mpc85xx: add MPIC message dts node
Timur Tabi (1):
powerpc/85xx: don't call of_platform_bus_probe() twice
arch/powerpc/boot/dts/fsl/pq3-mpic-message-B.dtsi | 43 ++++++++++++++++
arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi | 10 ++++
arch/powerpc/include/asm/mpic.h | 18 -------
arch/powerpc/include/asm/mpic_msgr.h | 1 +
arch/powerpc/include/asm/reg_booke.h | 5 --
arch/powerpc/kernel/setup_32.c | 3 ++
arch/powerpc/platforms/85xx/common.c | 6 +++
arch/powerpc/platforms/85xx/mpc85xx_mds.c | 11 +----
arch/powerpc/platforms/85xx/p1022_ds.c | 13 +----
arch/powerpc/platforms/powermac/low_i2c.c | 9 ++++
arch/powerpc/platforms/pseries/eeh.c | 2 +-
arch/powerpc/sysdev/mpic.c | 54 +++++++++++++--------
arch/powerpc/sysdev/mpic_msgr.c | 12 ++---
include/linux/irq.h | 7 +++
14 files changed, 122 insertions(+), 72 deletions(-)
create mode 100644 arch/powerpc/boot/dts/fsl/pq3-mpic-message-B.dtsi
^ permalink raw reply
* Re: [PATCH] gianfar: add GRO support
From: David Miller @ 2012-04-21 20:44 UTC (permalink / raw)
To: b06378; +Cc: netdev, linuxppc-dev
In-Reply-To: <1334912075-27073-1-git-send-email-b06378@freescale.com>
From: Jiajun Wu <b06378@freescale.com>
Date: Fri, 20 Apr 2012 16:54:35 +0800
> Replace netif_receive_skb with napi_gro_receive.
>
> Signed-off-by: Jiajun Wu <b06378@freescale.com>
Applied.
^ permalink raw reply
* Re: [PATCH] driver/mtd: IFC NAND: Add support of ONFI NAND flash
From: Artem Bityutskiy @ 2012-04-21 17:39 UTC (permalink / raw)
To: Prabhakar Kushwaha; +Cc: scottwood, linux-mtd, linuxppc-dev
In-Reply-To: <1333949122-10401-1-git-send-email-prabhakar@freescale.com>
On Mon, 2012-04-09 at 10:55 +0530, Prabhakar Kushwaha wrote:
> - Fix NAND_CMD_READID command for ONFI detect.
> - Add NAND_CMD_PARAM command to read the ONFI parameter page.
>
> Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
> Acked-by: Scott Wood <scottwood@freescale.com>
> ---
> Based upon git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
> Branch master
Pushed to l2-mtd.git, thanks!
--
Best Regards,
Artem Bityutskiy
^ permalink raw reply
* Re: pci node question
From: Benjamin Herrenschmidt @ 2012-04-20 21:24 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4F91CCCA.1050204@freescale.com>
> Between the root complex and whatever's plugged in?
Yes.
> > so that is what the node represents. Its always been a bit confusing to me as its not 100% standardized by PCI sig.
It's absolutely standard. The only case where you have "siblings" to
that RC is when it's some kind of integrated chipset with non-PCI
devices in it (still common in Intel world).
Any real PCIe device -must- have a P2P above it with the PCIe capability
& associated control registers.
> Is it documented anywhere (e.g. in the chip manual)? Is it common (even
> if 100% standardized), or a Freescale-ism?
PCIe spec.
> Is there an actual PCIe-to-PCIe bridge that shows up in the root
> complex's config space
Yes.
> (not talking about the host bridge PCI device
> that has always been there on FSL PCI controllers, which we didn't
> represent in the device tree on non-express PCI)? Why does this bridge
> need to be represented in the device tree (assuming no ISA devices need
> to be represented), when other PCI devices (including bridges) don't?
You don't have to, but we sometimes do it so we can put the
interrupt-map in the right place. But again, since on PCIe the device
underneath should always have dev 0 for non-SRIOV stuff, the swizzling
shall be NOP and so having the interrupt-map all the way at the top
might work. However I'm not sure if that will work with PFs and ARI
where the dev is non-0.
> > Maybe Ben's got some comments to add here from a generic PCIe point of view.
> >
> >>>> Do we really need the error interrupt specified twice?
> >>>
> >>> I put it twice because it has multiple purposes, one has to do with interrupts defined by the PCI spec vs ones defined via FSL controller.
> >>
> >> There are PCI-defined error conditions that cause a host controller
> >> interrupt. What does this have to do with the bridge node?
> >
> > Think of the "error" IRQ as shared between to classes of interrupts. One set are controller errors defined by FSL, the other are errors defined by PCI sig / bus point of view.
> >
> > I'd expect controller errors to be handled by something like EDAC would bind at "fsl,qoriq-pcie-v2.2" level node. The PCI bus code would looks at the virtual bridge down.
>
> This shouldn't be about where a specific piece of Linux code looks.
>
> I don't see why the split of PCI-defined errors versus FSL-defined
> errors maps to bridge node versus controller node. What if we didn't
> have the bridge? We'd still have PCI-defined errors, right?
The linux generic PCIe port driver looks for the interrupt of the bridge
for standardized PCIe AER interrupts.
Cheers,
Ben.
> -Scott
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: pci node question
From: Benjamin Herrenschmidt @ 2012-04-20 21:20 UTC (permalink / raw)
To: Kumar Gala; +Cc: Scott Wood, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <EDA55B32-FD14-4B5C-90AF-7FC5F77898E0@kernel.crashing.org>
On Fri, 2012-04-20 at 15:37 -0500, Kumar Gala wrote:
> > What does this node represent in the first place? Is there really a
> > PCI-to-PCI bridge? Are all other PCI devices underneath it?
>
> PCIe has this concept of a "virtual" bridge between the root-comples,
> so that is what the node represents. Its always been a bit confusing
> to me as its not 100% standardized by PCI sig.
It actually is. The root complex virtual p2p is absolutely part of the
standard.
Cheers,
Ben.
^ permalink raw reply
* Re: pci node question
From: Benjamin Herrenschmidt @ 2012-04-20 21:19 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4F91B335.4010501@freescale.com>
On Fri, 2012-04-20 at 14:04 -0500, Scott Wood wrote:
> That's not supposed to be how device tree bindings are determined.
Ugh ? This is nothing to do with Linux, I think Kumar is confused :-)
This has to do with PCI bindings.
If you put the interrupt-map in the parent, then crossing the virtual
p2p will cause a swizzling which is not what you want. As long as the
device has a device number (in devfn) of 0 that's fine but that may not
always be the case.
> It's causing us problems with virtualization device assignment, because
> if we just assign the parent bus we don't get the interrupt map, but if
> we assign the child we need to deal with what it means to assign an
> individual PCI device (e.g. on our internal KVM stuff we get an error on
> that reg property).
I'm not sure what you are doing with device-assignement in KVM, we are
doing something different for server I suppose since the existing
assignment code in qemu-kvm is a dead end... But you should probably
synthetize an interrupt-map for the guest.
> What does this node represent in the first place? Is there really a
> PCI-to-PCI bridge? Are all other PCI devices underneath it?
Yes, PCIe 101 :-) It's the root complex, it's "virtual" in that it's a
construct of the host bridge, there's no physical "bridge" in the
system, but yes, PCIe always has a virtual P2P at the top to represent
the root complex.
This was done to fix the long standing problem that there wasn't a
proper way to represent host bridges as parent of their devices in PCI
land.
It allows PCIe to guarantee that a device always has a bridge above
itself, with the corresponding link control registers etc... which is
useful for point to point links :-)
That's also why for example PCIe switches look like stacks of bridges,
for example, a 2 fork switch looks like:
|
P2P
/ \
P2P P2P
| |
That way each downstream device gets its own parent P2P with the
corresponding PCIe link control registers etc...
> >> Do we really need the error interrupt specified twice?
> >
> > I put it twice because it has multiple purposes, one has to do with interrupts defined by the PCI spec vs ones defined via FSL controller.
>
> There are PCI-defined error condition s that cause a host controller
> interrupt. What does this have to do with the bridge node?
>
> >> Why is there a zeroed out reg property: reg = <0 0 0 0 0> ??
> >
> > scratching my head, what happens if you remove it?
>
> Sigh.
As I said earlier, this is not some black magic, it's a proper reg value
for the root complex virtual bridge. It has bus 0, devfn 0, so the reg
property for the config space has a value of 0.
Without it, the kernel won't properly match it to the corresponding
pci_dev.
Cheers,
Ben.
> -Scott
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: pci node question
From: Benjamin Herrenschmidt @ 2012-04-20 21:11 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <D04C6167-5286-4675-A6D8-32BA63305424@kernel.crashing.org>
On Fri, 2012-04-20 at 13:53 -0500, Kumar Gala wrote:
> On Apr 20, 2012, at 1:37 PM, Yoder Stuart-B08248 wrote:
>
> > There was refactoring change a while back that moved
> > the interrupt map down into the virtual pci bridge.
> >
> > example:
> > 42 /* controller at 0x200000 */
> > 43 &pci0 {
> > 44 compatible = "fsl,p2041-pcie", "fsl,qoriq-pcie-v2.2";
> > 45 device_type = "pci";
> > 46 #size-cells = <2>;
> > 47 #address-cells = <3>;
> > 48 bus-range = <0x0 0xff>;
> > 49 clock-frequency = <33333333>;
> > 50 interrupts = <16 2 1 15>;
> > 51 pcie@0 {
> > 52 reg = <0 0 0 0 0>;
> > 53 #interrupt-cells = <1>;
> > 54 #size-cells = <2>;
> > 55 #address-cells = <3>;
> > 56 device_type = "pci";
> > 57 interrupts = <16 2 1 15>;
> > 58 interrupt-map-mask = <0xf800 0 0 7>;
> > 59 interrupt-map = <
> > 60 /* IDSEL 0x0 */
> > 61 0000 0 0 1 &mpic 40 1 0 0
> > 62 0000 0 0 2 &mpic 1 1 0 0
> > 63 0000 0 0 3 &mpic 2 1 0 0
> > 64 0000 0 0 4 &mpic 3 1 0 0
> > 65 >;
> > 66 };
> > 67 };
> >
> > Why was the interrupt-map moved here?
>
> Its been a while, but I think i moved it down because of which node is used for interrupt handling in linux.
Yeah, it would swizzle if going accross the virtual P2P bridge. On the
other hand, if it's PCIe, the children of the vP2P should always be at
device 0 and thus the swizzling should be a NOP no ? So we -could- leave
it in the PHB unless I'm mistaken...
On the other hand, we probably want to fix the mapping code to handle
things like SR-IOV PFs with different device numbers... do those get
swizzled ? In that case we might want to keep things down the virtual
bridge.
> > Do we really need the error interrupt specified twice?
>
> I put it twice because it has multiple purposes, one has to do with interrupts defined by the PCI spec vs ones defined via FSL controller.
>
> > Why is there a zeroed out reg property: reg = <0 0 0 0 0> ??
>
> scratching my head, what happens if you remove it?
If you remove it, the kernel will fail to match the vP2P with the
corresponding struct pci_dev.... It's not "zeroed out". It contains a
valid bus/dev/fn ... of 0,0,0 :-)
Cheers,
Ben.
> - k
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: pci node question
From: Scott Wood @ 2012-04-20 20:53 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <EDA55B32-FD14-4B5C-90AF-7FC5F77898E0@kernel.crashing.org>
On 04/20/2012 03:37 PM, Kumar Gala wrote:
>
> On Apr 20, 2012, at 2:04 PM, Scott Wood wrote:
>
>> On 04/20/2012 01:53 PM, Kumar Gala wrote:
>>>
>>> On Apr 20, 2012, at 1:37 PM, Yoder Stuart-B08248 wrote:
>>>
>>>> There was refactoring change a while back that moved
>>>> the interrupt map down into the virtual pci bridge.
>>>>
>>>> example:
>>>> 42 /* controller at 0x200000 */
>>>> 43 &pci0 {
>>>> 44 compatible = "fsl,p2041-pcie", "fsl,qoriq-pcie-v2.2";
>>>> 45 device_type = "pci";
>>>> 46 #size-cells = <2>;
>>>> 47 #address-cells = <3>;
>>>> 48 bus-range = <0x0 0xff>;
>>>> 49 clock-frequency = <33333333>;
>>>> 50 interrupts = <16 2 1 15>;
>>>> 51 pcie@0 {
>>>> 52 reg = <0 0 0 0 0>;
>>>> 53 #interrupt-cells = <1>;
>>>> 54 #size-cells = <2>;
>>>> 55 #address-cells = <3>;
>>>> 56 device_type = "pci";
>>>> 57 interrupts = <16 2 1 15>;
>>>> 58 interrupt-map-mask = <0xf800 0 0 7>;
>>>> 59 interrupt-map = <
>>>> 60 /* IDSEL 0x0 */
>>>> 61 0000 0 0 1 &mpic 40 1 0 0
>>>> 62 0000 0 0 2 &mpic 1 1 0 0
>>>> 63 0000 0 0 3 &mpic 2 1 0 0
>>>> 64 0000 0 0 4 &mpic 3 1 0 0
>>>> 65 >;
>>>> 66 };
>>>> 67 };
>>>>
>>>> Why was the interrupt-map moved here?
>>>
>>> Its been a while, but I think i moved it down because of which node is used for interrupt handling in linux.
>>
>> That's not supposed to be how device tree bindings are determined.
>>
>> It's causing us problems with virtualization device assignment, because
>> if we just assign the parent bus we don't get the interrupt map, but if
>> we assign the child we need to deal with what it means to assign an
>> individual PCI device (e.g. on our internal KVM stuff we get an error on
>> that reg property).
>>
>> What does this node represent in the first place? Is there really a
>> PCI-to-PCI bridge? Are all other PCI devices underneath it?
>
> PCIe has this concept of a "virtual" bridge between the root-comples,
Between the root complex and whatever's plugged in?
> so that is what the node represents. Its always been a bit confusing to me as its not 100% standardized by PCI sig.
Is it documented anywhere (e.g. in the chip manual)? Is it common (even
if 100% standardized), or a Freescale-ism?
Is there an actual PCIe-to-PCIe bridge that shows up in the root
complex's config space (not talking about the host bridge PCI device
that has always been there on FSL PCI controllers, which we didn't
represent in the device tree on non-express PCI)? Why does this bridge
need to be represented in the device tree (assuming no ISA devices need
to be represented), when other PCI devices (including bridges) don't?
> Maybe Ben's got some comments to add here from a generic PCIe point of view.
>
>>>> Do we really need the error interrupt specified twice?
>>>
>>> I put it twice because it has multiple purposes, one has to do with interrupts defined by the PCI spec vs ones defined via FSL controller.
>>
>> There are PCI-defined error conditions that cause a host controller
>> interrupt. What does this have to do with the bridge node?
>
> Think of the "error" IRQ as shared between to classes of interrupts. One set are controller errors defined by FSL, the other are errors defined by PCI sig / bus point of view.
>
> I'd expect controller errors to be handled by something like EDAC would bind at "fsl,qoriq-pcie-v2.2" level node. The PCI bus code would looks at the virtual bridge down.
This shouldn't be about where a specific piece of Linux code looks.
I don't see why the split of PCI-defined errors versus FSL-defined
errors maps to bridge node versus controller node. What if we didn't
have the bridge? We'd still have PCI-defined errors, right?
-Scott
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox