| Message ID | 1458949858-4679-1-git-send-email-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
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
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 --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, };
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(+)