diff mbox

[1/2,v2] dma: pl08x: support fixed signal assignment

Message ID 1458949858-4679-1-git-send-email-linus.walleij@linaro.org (mailing list archive)
State Superseded
Headers show

Commit Message

Linus Walleij March 25, 2016, 11:50 p.m. UTC
Some implementations such as the Nomadik do not place a mux
in front of the DMA single/burst request signals into the
PL08x DMA controller, instead they lock a certain signal to
a certain device. This makes things simpler and the platform
does not need to implement a muxing function.

Implement this scheme and flag that the Nomadik uses this
method.

While we're at it, add a missing kerneldoc entry.

Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Joachim Eastwood <manabian@gmail.com>
Cc: Johannes Stezenbach <js@sig21.net>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Also encode the number of outgoing request signals into
  the vendor data. This will be used by the next patch to
  allocate slave channels for the DT probe.
---
 drivers/dma/amba-pl08x.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

Comments

Johannes Stezenbach March 29, 2016, 1:03 p.m. UTC | #1
On Sat, Mar 26, 2016 at 12:50:58AM +0100, Linus Walleij wrote:
> + * @signals: the number of request signals available from the hardware
...
>  struct vendor_data {
>  	u8 config_offset;
>  	u8 channels;
> +	u8 signals;

OK about that, but since nothing in this patch uses it,
it belongs in the next patch.

>  	bool dualmaster;
>  	bool nomadik;
>  	bool pl080s;
> +	bool fixed_signals;

But for this I don't see the need, as I've already said.

>  	u32 max_transfer_size;
>  };
>  
> @@ -319,6 +327,15 @@ static int pl08x_request_mux(struct pl08x_dma_chan *plchan)
>  	const struct pl08x_platform_data *pd = plchan->host->pd;
>  	int ret;
>  
> +	/*
> +	 * For engines with fixed signal assignment for slave transfers,
> +	 * this signal is assigned during probe.
> +	 */
> +	if (plchan->host->vd->fixed_signals) {
> +		BUG_ON(plchan->signal < 0);
> +		return 0;
> +	}
> +
>  	if (plchan->mux_use++ == 0 && pd->get_xfer_signal) {

If the platform doesn't have a mux (and thus has fixed signals),
so far pd->get_xfer_signal returned the signal.
Now with dt, pd->get/put_xfer_signal aren't set and thus
pl08x_request/release_mux() won't do anything.  So I think
the check above is not needed.

> @@ -1909,6 +1929,18 @@ static int pl08x_dma_init_virtual_channels(struct pl08x_driver_data *pl08x,
>  
>  		if (slave) {
>  			chan->cd = &pl08x->pd->slave_channels[i];
> +			/*
> +			 * Some implementations have muxed signals, whereas some
> +			 * use a mux in front of the signals and need dynamic
> +			 * assignment of signals.
> +			 */
> +			if (pl08x->vd->fixed_signals) {
> +				chan->signal = i;

As commented earlier, I'd just unconditionally do this assignment.

If the platform uses a mux, pd->get_xfer_signal will
return the real signal (or an error, in which case chan->signal
won't be used).  If it doesn't use a mux (i.e. uses
fixed signals), then this initialization is needed.

> +				chan->name = chan->cd->bus_id;

as commented earlier, pl08x_dma_slave_init() already does this

> +				dev_dbg(&pl08x->adev->dev,
> +					"assign fixed signal %u to channel %s",
> +					chan->signal, chan->name);
> +			}
>  			pl08x_dma_slave_init(chan);
>  		} else {
>  			chan->cd = &pl08x->pd->memcpy_channel;


Johannes
--
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
Linus Walleij April 3, 2016, 7:09 p.m. UTC | #2
On Tue, Mar 29, 2016 at 3:03 PM, Johannes Stezenbach <js@sig21.net> wrote:

>> +     /*
>> +      * For engines with fixed signal assignment for slave transfers,
>> +      * this signal is assigned during probe.
>> +      */
>> +     if (plchan->host->vd->fixed_signals) {
>> +             BUG_ON(plchan->signal < 0);
>> +             return 0;
>> +     }
>> +
>>       if (plchan->mux_use++ == 0 && pd->get_xfer_signal) {
>
> If the platform doesn't have a mux (and thus has fixed signals),
> so far pd->get_xfer_signal returned the signal.
> Now with dt, pd->get/put_xfer_signal aren't set and thus
> pl08x_request/release_mux() won't do anything.  So I think
> the check above is not needed.

It's there because I think it is not logical to keep counting
up/down mux_use when you don't have a mux.

But I have dropped this for now, the important is that we
can get our systems running.

Yours,
Linus Walleij
--
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
diff mbox

Patch

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index 9b42c0588550..2ef80200d110 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -107,19 +107,27 @@  struct pl08x_driver_data;
 /**
  * struct vendor_data - vendor-specific config parameters for PL08x derivatives
  * @channels: the number of channels available in this variant
+ * @signals: the number of request signals available from the hardware
  * @dualmaster: whether this version supports dual AHB masters or not.
  * @nomadik: whether the channels have Nomadik security extension bits
  *	that need to be checked for permission before use and some registers are
  *	missing
  * @pl080s: whether this version is a PL080S, which has separate register and
  *	LLI word for transfer size.
+ * @fixed_signals: if the channel signal allocation on the PL08x is fixed,
+ *	i.e. each device burst/single control signals are connected directly
+ *	to the DMAC signal lines without any mux.
+ * @max_transfer_size: the maximum single element transfer size for this
+ *	PL08x variant.
  */
 struct vendor_data {
 	u8 config_offset;
 	u8 channels;
+	u8 signals;
 	bool dualmaster;
 	bool nomadik;
 	bool pl080s;
+	bool fixed_signals;
 	u32 max_transfer_size;
 };
 
@@ -319,6 +327,15 @@  static int pl08x_request_mux(struct pl08x_dma_chan *plchan)
 	const struct pl08x_platform_data *pd = plchan->host->pd;
 	int ret;
 
+	/*
+	 * For engines with fixed signal assignment for slave transfers,
+	 * this signal is assigned during probe.
+	 */
+	if (plchan->host->vd->fixed_signals) {
+		BUG_ON(plchan->signal < 0);
+		return 0;
+	}
+
 	if (plchan->mux_use++ == 0 && pd->get_xfer_signal) {
 		ret = pd->get_xfer_signal(plchan->cd);
 		if (ret < 0) {
@@ -335,6 +352,9 @@  static void pl08x_release_mux(struct pl08x_dma_chan *plchan)
 {
 	const struct pl08x_platform_data *pd = plchan->host->pd;
 
+	if (plchan->host->vd->fixed_signals)
+		return;
+
 	if (plchan->signal >= 0) {
 		WARN_ON(plchan->mux_use == 0);
 
@@ -1909,6 +1929,18 @@  static int pl08x_dma_init_virtual_channels(struct pl08x_driver_data *pl08x,
 
 		if (slave) {
 			chan->cd = &pl08x->pd->slave_channels[i];
+			/*
+			 * Some implementations have muxed signals, whereas some
+			 * use a mux in front of the signals and need dynamic
+			 * assignment of signals.
+			 */
+			if (pl08x->vd->fixed_signals) {
+				chan->signal = i;
+				chan->name = chan->cd->bus_id;
+				dev_dbg(&pl08x->adev->dev,
+					"assign fixed signal %u to channel %s",
+					chan->signal, chan->name);
+			}
 			pl08x_dma_slave_init(chan);
 		} else {
 			chan->cd = &pl08x->pd->memcpy_channel;
@@ -2438,6 +2470,7 @@  out_no_pl08x:
 static struct vendor_data vendor_pl080 = {
 	.config_offset = PL080_CH_CONFIG,
 	.channels = 8,
+	.signals = 16,
 	.dualmaster = true,
 	.max_transfer_size = PL080_CONTROL_TRANSFER_SIZE_MASK,
 };
@@ -2445,14 +2478,17 @@  static struct vendor_data vendor_pl080 = {
 static struct vendor_data vendor_nomadik = {
 	.config_offset = PL080_CH_CONFIG,
 	.channels = 8,
+	.signals = 32,
 	.dualmaster = true,
 	.nomadik = true,
+	.fixed_signals = true,
 	.max_transfer_size = PL080_CONTROL_TRANSFER_SIZE_MASK,
 };
 
 static struct vendor_data vendor_pl080s = {
 	.config_offset = PL080S_CH_CONFIG,
 	.channels = 8,
+	.signals = 32,
 	.pl080s = true,
 	.max_transfer_size = PL080S_CONTROL_TRANSFER_SIZE_MASK,
 };
@@ -2460,6 +2496,7 @@  static struct vendor_data vendor_pl080s = {
 static struct vendor_data vendor_pl081 = {
 	.config_offset = PL080_CH_CONFIG,
 	.channels = 2,
+	.signals = 16,
 	.dualmaster = false,
 	.max_transfer_size = PL080_CONTROL_TRANSFER_SIZE_MASK,
 };