All of lore.kernel.org
 help / color / mirror / Atom feed
From: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
To: Lars-Peter Clausen <lars@metafoo.de>,
	Dan Williams <dan.j.williams@intel.com>
Cc: Vinod Koul <vinod.koul@intel.com>,
	dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] dmaengine: pl330: Check if the DMA descriptor is NULL
Date: Mon, 21 Jul 2014 10:44:15 +0200	[thread overview]
Message-ID: <53CCD2DF.4010307@collabora.co.uk> (raw)
In-Reply-To: <53CCCDAD.5080001@metafoo.de>

On 07/21/2014 10:22 AM, Lars-Peter Clausen wrote:
> On 07/20/2014 07:58 PM, Javier Martinez Canillas wrote:
>>
>> If this should not really happen and this patch is only a workaround since the
>> bug is elsewhere, please give me some hints and I'll try to fix it properly. I'm
>> not familiar with the PL330 DMA controller but just found what was the NULL
>> pointer being dereferenced and looked at your changes to see what was different now.
> 
> I think the patch is fine as a quick workaround since it is simple and the 
> previous commit broke previously working code.
> 

Agreed, it matches what the old code was doing and other code is relying on this
behavior so works as a quick fix to avoid the current kernel oops.

> The long term fix is to stop calling dma_pl330_rqcb() from 
> pl330_release_channel(). The first thing is you wouldn't expect any transfer 
> to be active when the channel is released. And even if it was by accident we 
> should not call the descriptor callback, but rather but it just back onto 
> the descriptor pool.

Thanks a lot for the clarification. I thought it was something along the lines
of not calling the callback from pl330_release_channel() but preferred to not
change anything that I could not completely understand its side effects.

I'll study the driver more deeply and try to come up with a patch on top of this
one that fixes the actual cause rather than the consequence.

> 
> - Lars
> 

Best regards,
Javier

  reply	other threads:[~2014-07-21  8:44 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-19  1:21 [PATCH 1/1] dmaengine: pl330: Check if the DMA descriptor is NULL Javier Martinez Canillas
2014-07-20 14:18 ` Lars-Peter Clausen
2014-07-20 17:58   ` Javier Martinez Canillas
2014-07-21  8:22     ` Lars-Peter Clausen
2014-07-21  8:44       ` Javier Martinez Canillas [this message]
2014-07-22 16:09 ` Vinod Koul

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53CCD2DF.4010307@collabora.co.uk \
    --to=javier.martinez@collabora.co.uk \
    --cc=dan.j.williams@intel.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=vinod.koul@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.