All of lore.kernel.org
 help / color / mirror / Atom feed
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V2] dma: imx-dma: Add oftree support
Date: Mon, 25 Feb 2013 14:31:12 +0000	[thread overview]
Message-ID: <201302251431.12829.arnd@arndb.de> (raw)
In-Reply-To: <20130224122748.GF10660@pengutronix.de>

On Sunday 24 February 2013, Markus Pargmann wrote:
> On Sat, Feb 23, 2013 at 10:16:21PM +0000, Arnd Bergmann wrote:
> > On Saturday 23 February 2013, Markus Pargmann wrote:

> > 
> > There are actually two more members in the imx_dma_data structure. Shouldn't those
> > be encoded in the dma specifier as well?
> 
> imx_dma_data is used by imx-dma and imx-sdma, but imx-dma does not use
> peripheral_type and priority. When not loaded from devicetree
> imx_dma_data is constructed by imx drivers without knowledge about the
> dma driver. This patch moves the initialization to the driver, so I
> think there is no need to fill all fields. I didn't want to use a new
> struct because the imx drivers could still use this old way.

Ok, got it.

> > >  static inline int imx_dma_is_ipu(struct dma_chan *chan)
> > > @@ -63,7 +66,8 @@ static inline int imx_dma_is_general_purpose(struct dma_chan *chan)
> > >  	return strstr(dev_name(chan->device->dev), "sdma") ||
> > >  		!strcmp(dev_name(chan->device->dev), "imx1-dma") ||
> > >  		!strcmp(dev_name(chan->device->dev), "imx21-dma") ||
> > > -		!strcmp(dev_name(chan->device->dev), "imx27-dma");
> > > +		!strcmp(dev_name(chan->device->dev), "imx27-dma") ||
> > > +		!strcmp(chan->device->dev->driver->name, "imx-dma");
> > >  }
> > 
> > Also, your filter function does not actually check
> > imx_dma_is_general_purpose() as the old style filter functions
> > in the slave drivers do, which breaks when you have more than one dma engine
> > in the system.
> 
> Oh yes, in the filter function should be a device driver comparison. But
> I would prefer a direct check instead of imx_dma_is_general_purpose because
> I think the driver should not define a filter function that initializes data
> for another driver (sdma).

Yes, that is also how I did it in the dwdma patches. Basically  I have
the dma engine object passed into xlate and then from there into the
filter, where I can compare it against chan->device.

	Arnd

WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
To: Markus Pargmann <mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org,
	rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org,
	djbw-b10kYP2dOMg@public.gmane.org,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH V2] dma: imx-dma: Add oftree support
Date: Mon, 25 Feb 2013 14:31:12 +0000	[thread overview]
Message-ID: <201302251431.12829.arnd@arndb.de> (raw)
In-Reply-To: <20130224122748.GF10660-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

On Sunday 24 February 2013, Markus Pargmann wrote:
> On Sat, Feb 23, 2013 at 10:16:21PM +0000, Arnd Bergmann wrote:
> > On Saturday 23 February 2013, Markus Pargmann wrote:

> > 
> > There are actually two more members in the imx_dma_data structure. Shouldn't those
> > be encoded in the dma specifier as well?
> 
> imx_dma_data is used by imx-dma and imx-sdma, but imx-dma does not use
> peripheral_type and priority. When not loaded from devicetree
> imx_dma_data is constructed by imx drivers without knowledge about the
> dma driver. This patch moves the initialization to the driver, so I
> think there is no need to fill all fields. I didn't want to use a new
> struct because the imx drivers could still use this old way.

Ok, got it.

> > >  static inline int imx_dma_is_ipu(struct dma_chan *chan)
> > > @@ -63,7 +66,8 @@ static inline int imx_dma_is_general_purpose(struct dma_chan *chan)
> > >  	return strstr(dev_name(chan->device->dev), "sdma") ||
> > >  		!strcmp(dev_name(chan->device->dev), "imx1-dma") ||
> > >  		!strcmp(dev_name(chan->device->dev), "imx21-dma") ||
> > > -		!strcmp(dev_name(chan->device->dev), "imx27-dma");
> > > +		!strcmp(dev_name(chan->device->dev), "imx27-dma") ||
> > > +		!strcmp(chan->device->dev->driver->name, "imx-dma");
> > >  }
> > 
> > Also, your filter function does not actually check
> > imx_dma_is_general_purpose() as the old style filter functions
> > in the slave drivers do, which breaks when you have more than one dma engine
> > in the system.
> 
> Oh yes, in the filter function should be a device driver comparison. But
> I would prefer a direct check instead of imx_dma_is_general_purpose because
> I think the driver should not define a filter function that initializes data
> for another driver (sdma).

Yes, that is also how I did it in the dwdma patches. Basically  I have
the dma engine object passed into xlate and then from there into the
filter, where I can compare it against chan->device.

	Arnd

  reply	other threads:[~2013-02-25 14:31 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-22 14:37 [PATCH] dma: imx-dma: Add oftree support Markus Pargmann
2013-02-22 14:37 ` Markus Pargmann
2013-02-22 14:46 ` Arnd Bergmann
2013-02-22 14:46   ` Arnd Bergmann
2013-02-23 16:57   ` Markus Pargmann
2013-02-23 16:57     ` Markus Pargmann
2013-02-23 16:59 ` [PATCH V2] " Markus Pargmann
2013-02-23 16:59   ` Markus Pargmann
2013-02-23 22:16   ` Arnd Bergmann
2013-02-23 22:16     ` Arnd Bergmann
2013-02-24 12:27     ` Markus Pargmann
2013-02-24 12:27       ` Markus Pargmann
2013-02-25 14:31       ` Arnd Bergmann [this message]
2013-02-25 14:31         ` Arnd Bergmann
2013-02-25 16:04 ` [PATCH v3 1/2] " Markus Pargmann
2013-02-25 16:04   ` Markus Pargmann
2013-02-25 16:04   ` [PATCH v3 2/2] DMA: imx-dma: imxdma->dev used uninitialized Markus Pargmann
2013-02-25 16:04     ` Markus Pargmann
2013-02-25 16:24   ` [PATCH v3 1/2] dma: imx-dma: Add oftree support Arnd Bergmann
2013-02-25 16:24     ` Arnd Bergmann
2013-02-25 16:53     ` Markus Pargmann
2013-02-25 16:53       ` Markus Pargmann
2013-02-28  6:30   ` Shawn Guo
2013-02-28  6:30     ` Shawn Guo
2013-03-01 13:56     ` Markus Pargmann
2013-03-01 13:56       ` Markus Pargmann

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=201302251431.12829.arnd@arndb.de \
    --to=arnd@arndb.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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.