RE: [PATCH] DMAENGINE: runtime config for slave channels

[Linus WALLEIJ]

[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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/