linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Shawn Guo <shawn.guo@linaro.org>
To: Eric Miao <eric.miao@linaro.org>
Cc: Richard Zhao <richard.zhao@freescale.com>,
	patches@linaro.org, vinod.koul@intel.com,
	linux-kernel@vger.kernel.org, kernel@pengutronix.de,
	dan.j.williams@intel.com, Richard Zhao <richard.zhao@linaro.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 6/6] dma/imx-sdma: check whether event_id0 < 32 when set event_mask
Date: Wed, 11 Jan 2012 21:16:17 +0800	[thread overview]
Message-ID: <20120111131615.GC20968@S2101-09.ap.freescale.net> (raw)
In-Reply-To: <CAMPhdO9sGWTomtyMSTe_jC2GfvtdLR=M-z3Uzpesm9efBzhSqA@mail.gmail.com>

On Wed, Jan 11, 2012 at 02:37:08PM +0800, Eric Miao wrote:
> I think Richard has made the issue quite clear here, the original

Yes, he has made it clear, but only after I asked for more comments,
not with the empty commit message.

> code does seem to have some problems even to me, who do not
> understand the very details of the SDMA:
> 
> -                       sdmac->event_mask0 = 1 << sdmac->event_id0;
> -                       sdmac->event_mask1 = 1 << (sdmac->event_id0 - 32);
> 
> 1. if sdmac->event_id0 >= 32, which will cause event_mask0 to be incorrect
> 2. if sdmac->event_id < 32, sdmac->event_mask1 will be incorrect
> 
My testing tells this is not the case.  The event_mask0 will be 0 in
case 1) and event_mask1 will be 0 in case 2), which is quite what we
expect.  And I do not believe you will see any functionality bug with
the existing code.

See, that's why we need verbose commit message to make the patch and
the problem it's trying to address very clear.

Regards,
Shawn

  parent reply	other threads:[~2012-01-11 13:05 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-10  7:01 [PATCH 1/6] dma/imx-sdma: let sdma_run_channel call sdma_enable_channel Richard Zhao
2012-01-10  7:01 ` [PATCH 2/6] dma/imx-sdma: use readl_relaxed/writel_relaxed and use writel when necessary Richard Zhao
2012-01-10  7:01 ` [PATCH 3/6] dma/imx-sdma: call sdma_set_channel_priority after sdma_request_channel Richard Zhao
2012-01-10  7:01 ` [PATCH 4/6] dma/imx-sdma: move clk_enable out of sdma_request_channel Richard Zhao
2012-01-10  7:01 ` [PATCH 5/6] dma/imx-sdma: use num_events to validate event_id0 Richard Zhao
2012-01-10 14:02   ` Dirk Behme
2012-01-10 14:16   ` Shawn Guo
2012-01-10  7:01 ` [PATCH 6/6] dma/imx-sdma: check whether event_id0 < 32 when set event_mask Richard Zhao
2012-01-10 14:20   ` Shawn Guo
2012-01-10 14:29     ` Richard Zhao
2012-01-10 15:38       ` Shawn Guo
2012-01-11  0:53         ` Richard Zhao
2012-01-11  1:47           ` Richard Zhao
2012-01-11  6:37             ` Eric Miao
2012-01-11 12:35               ` Richard Zhao
2012-01-11 12:53                 ` Richard Zhao
2012-01-12 14:23                   ` Richard Zhao
2012-01-11 13:16               ` Shawn Guo [this message]
2012-01-11 13:09                 ` Richard Zhao
2012-01-11 13:35                   ` Shawn Guo
2012-01-11 13:48                     ` Eric Miao
2012-01-11 13:11           ` Shawn Guo

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=20120111131615.GC20968@S2101-09.ap.freescale.net \
    --to=shawn.guo@linaro.org \
    --cc=dan.j.williams@intel.com \
    --cc=eric.miao@linaro.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@linaro.org \
    --cc=richard.zhao@freescale.com \
    --cc=richard.zhao@linaro.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).