Prev: cpumask: fix compat getaffinity
Next: [PATCH] perf: Fix argument of perf_arch_fetch_caller_regs
From: Dan Williams on 22 Jun 2010 20:00
On Sun, Jun 20, 2010 at 3:19 PM, Linus Walleij
<linus.walleij(a)stericsson.com> wrote:
> This adds an interface to the DMAengine to make it possible to
> reconfigure a channel at runtime. We add a few foreseen config
> parameters to the passed struct, with a void * pointer for custom
> per-device or per-platform data.
>
> Cc: Viresh Kumar <viresh.kumar(a)st.com>
> Signed-off-by: Linus Walleij <linus.walleij(a)stericsson.com>
> ---
> OK at night I realized that maybe what we do for runtime
> re-configuration of DMA for the AMBA PrimeCells is actually generic
> and should maybe be lifted into the DMAengine instead.
>
> This conclusion came from Viresh stating that the SPEAr platform
> will need runtime reconfiguration for other things than PrimeCells,
> (on the PL08x driver) and actually we will likely need it ourselves
> too.
>
> So before starting to abuse that PrimeCell interface for things
> not PrimeCell, this way I suggest making this generic from day 1.
>
> Doing so makes it possible for me to merge the runtime
> re-configuration support for DMA40, COH 901 318 and the current
> iteration of the PL08x driver as well, and break that out of the
> PrimeCell series and drop the include/linux/amba/dma.h entirely
> while still getting some real nice and generic functionality
> in place.
>
> The PrimeCell patchset can then be nicely wrapped around this.
>
> More genric run-time configurations can be added to the struct if
> they are really generic like these, else the private_config can
> be used for local obscurities.
>
> Need your help on how to proceed on this one Dan.

Is there a guarantee that all devices that implement this command will
support all the fields? The presence of private_config concerns me
because that still seems to imply some hard coded knowledge about the
specific dma engine from the client.

....which is fine, but only if this is a common way to express
arch-specific extensions. In other words it's not clear where the
line is drawn between "this is a common operation that different dma
drivers can use / extend incompatibly" versus "this truly a generic
api that a client driver can depend on to run unmodified across
architectures that have a supporting dma driver". Clarifying that
would help end point device driver authors to know what they can rely
on... to date all the slave implementations have been written by the
same author who wrote the driver, so the information hiding has been
blurred. It would be nice to define / document what is and is not
expected to work for this command across implementations.

--
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Linus WALLEIJ on 23 Jun 2010 02:30
[Dan Williams]

> Is there a guarantee that all devices that implement this command will
> support all the fields?

No. They may be ignored on some controllers.

> The presence of private_config concerns me
> because that still seems to imply some hard coded knowledge about the
> specific dma engine from the client.

Yes that is the idea.

> ...which is fine, but only if this is a common way to express
> arch-specific extensions. In other words it's not clear where the
> line is drawn between "this is a common operation that different dma
> drivers can use / extend incompatibly" versus "this truly a generic
> api that a client driver can depend on to run unmodified across
> architectures that have a supporting dma driver". Clarifying that
> would help end point device driver authors to know what they can rely
> on... to date all the slave implementations have been written by the
> same author who wrote the driver, so the information hiding has been
> blurred. It would be nice to define / document what is and is not
> expected to work for this command across implementations.

OK the idea is to have the generic stuff in
struct dma_slave_channel_config, and the only criteria I have for
deciding what is generic is what COH901318, DMA40 and PL08x need
to runtime configure to handle things like UART RX/TX and SPI
DMA with varying wordwidth which is currently my most complex
usecases.

There may be DMA controllers that cannot specify the word width
for example, but most can. Those that cannot do this will ignore
that field, and as a consequence cannot be used to feed drivers
that make use of it with DMA.

This is a hardware restriction though: if your DMA controller cannot
control the word width you cannot use the PL022 driver with DMA
for example.

The private_config is intended for stuff that can never be
expressed in a generic way. For example Viresh told me that they
may need to runtime-control which bus master the PL080 is using.
Then the driver needs to take platform-specific actions, and
there will be some struct pl08x_slave_config passed in this
opaque pointer so the PL08x can be set up properly.

Since you don't seem to be totally against this approach I
will attempt to submit a more elaborate patch set.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: cpumask: fix compat getaffinity
Next: [PATCH] perf: Fix argument of perf_arch_fetch_caller_regs