diff mbox

[v1,1/1] dmaengine: acpi-dma: truly print message when appropriate

Message ID 1453306896-143051-1-git-send-email-andriy.shevchenko@linux.intel.com (mailing list archive)
State Changes Requested
Headers show

Commit Message

Andy Shevchenko Jan. 20, 2016, 4:21 p.m. UTC
Even if we are not going to get a channel the message is issued. Check the
result of acpi_dma_request_slave_chan_by_index() before issuing the message.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/dma/acpi-dma.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Mika Westerberg Jan. 21, 2016, 9:26 a.m. UTC | #1
On Wed, Jan 20, 2016 at 06:21:36PM +0200, Andy Shevchenko wrote:
> Even if we are not going to get a channel the message is issued. Check the
> result of acpi_dma_request_slave_chan_by_index() before issuing the message.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

The debug message is there because it prints out the index of the
FixedDMA descriptor we are trying to get channel for -- even if we
cannot get the channel.

Since it is just a debug message your reasoning works for me as well ;-)

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Shevchenko Jan. 22, 2016, 10:22 a.m. UTC | #2
On Thu, 2016-01-21 at 11:26 +0200, Westerberg, Mika wrote:
> On Wed, Jan 20, 2016 at 06:21:36PM +0200, Andy Shevchenko wrote:
> > Even if we are not going to get a channel the message is issued.
> > Check the
> > result of acpi_dma_request_slave_chan_by_index() before issuing the
> > message.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> The debug message is there because it prints out the index of the
> FixedDMA descriptor we are trying to get channel for -- even if we
> cannot get the channel.

Then it should sound a bit differently, shoudln't it?

Something like

dev_dbg(dev, "Looking for DMA channel \"%s\" at index %d...\n", name,
index);



> 
> Since it is just a debug message your reasoning works for me as well
> ;-)


> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Thanks!
Vinod Koul Feb. 8, 2016, 2:57 a.m. UTC | #3
On Fri, Jan 22, 2016 at 12:22:34PM +0200, Andy Shevchenko wrote:
> On Thu, 2016-01-21 at 11:26 +0200, Westerberg, Mika wrote:
> > On Wed, Jan 20, 2016 at 06:21:36PM +0200, Andy Shevchenko wrote:
> > > Even if we are not going to get a channel the message is issued.
> > > Check the
> > > result of acpi_dma_request_slave_chan_by_index() before issuing the
> > > message.
> > > 
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > 
> > The debug message is there because it prints out the index of the
> > FixedDMA descriptor we are trying to get channel for -- even if we
> > cannot get the channel.
> 
> Then it should sound a bit differently, shoudln't it?
> 
> Something like
> 
> dev_dbg(dev, "Looking for DMA channel \"%s\" at index %d...\n", name,
> index);

Yes that sound much better than orignal one :)
Andy Shevchenko Feb. 16, 2016, 9:18 a.m. UTC | #4
On Mon, 2016-02-08 at 08:27 +0530, Vinod Koul wrote:
> On Fri, Jan 22, 2016 at 12:22:34PM +0200, Andy Shevchenko wrote:
> > On Thu, 2016-01-21 at 11:26 +0200, Westerberg, Mika wrote:
> > > On Wed, Jan 20, 2016 at 06:21:36PM +0200, Andy Shevchenko wrote:
> > > > Even if we are not going to get a channel the message is
> > > > issued.
> > > > Check the
> > > > result of acpi_dma_request_slave_chan_by_index() before issuing
> > > > the
> > > > message.
> > > > 
> > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.c
> > > > om>
> > > 
> > > The debug message is there because it prints out the index of the
> > > FixedDMA descriptor we are trying to get channel for -- even if
> > > we
> > > cannot get the channel.
> > 
> > Then it should sound a bit differently, shoudln't it?
> > 
> > Something like
> > 
> > dev_dbg(dev, "Looking for DMA channel \"%s\" at index %d...\n",
> > name,
> > index);
> 
> Yes that sound much better than orignal one :)

Yes, since Mika wants to see something before matching, we probably
have to change just message. I will send v2 soon with Mika's Ack as
agreed with him.
diff mbox

Patch

diff --git a/drivers/dma/acpi-dma.c b/drivers/dma/acpi-dma.c
index eed6bda..9927101 100644
--- a/drivers/dma/acpi-dma.c
+++ b/drivers/dma/acpi-dma.c
@@ -426,6 +426,7 @@  EXPORT_SYMBOL_GPL(acpi_dma_request_slave_chan_by_index);
 struct dma_chan *acpi_dma_request_slave_chan_by_name(struct device *dev,
 		const char *name)
 {
+	struct dma_chan *chan;
 	int index;
 
 	index = device_property_match_string(dev, "dma-names", name);
@@ -438,8 +439,12 @@  struct dma_chan *acpi_dma_request_slave_chan_by_name(struct device *dev,
 			return ERR_PTR(-ENODEV);
 	}
 
+	chan = acpi_dma_request_slave_chan_by_index(dev, index);
+	if (IS_ERR(chan))
+		return chan;
+
 	dev_dbg(dev, "found DMA channel \"%s\" at index %d\n", name, index);
-	return acpi_dma_request_slave_chan_by_index(dev, index);
+	return chan;
 }
 EXPORT_SYMBOL_GPL(acpi_dma_request_slave_chan_by_name);