* [v1,3/4] dmaengine: imx-sdma: support dmatest
From: Vinod Koul @ 2018-07-11 7:19 UTC (permalink / raw)
To: s.hauer@pengutronix.de
Cc: Robin Gong, dan.j.williams@intel.com, shawnguo@kernel.org,
Fabio Estevam, linux@armlinux.org.uk,
linux-arm-kernel@lists.infradead.org, kernel@pengutronix.de,
dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
dl-linux-imx
On 11-07-18, 08:53, s.hauer@pengutronix.de wrote:
> On Wed, Jul 11, 2018 at 06:37:02AM +0000, Robin Gong wrote:
> >
> > > -----Original Message-----
> > > From: Vinod [mailto:vkoul@kernel.org]
> > > Sent: 2018年7月10日 23:33
> > > To: Robin Gong <yibin.gong@nxp.com>
> > > Cc: dan.j.williams@intel.com; shawnguo@kernel.org;
> > > s.hauer@pengutronix.de; Fabio Estevam <fabio.estevam@nxp.com>;
> > > linux@armlinux.org.uk; linux-arm-kernel@lists.infradead.org;
> > > kernel@pengutronix.de; dmaengine@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> > > Subject: Re: [PATCH v1 3/4] dmaengine: imx-sdma: support dmatest
> > >
> > > On 11-07-18, 00:23, Robin Gong wrote:
> > > > dmatest(memcpy) will never call dmaengine_slave_config before prep,
> > >
> > > and that should have been a hint to you that you should not expect that
> > >
> > > > so jobs in dmaengine_slave_config need to be moved into somewhere
> > > > before device_prep_dma_memcpy. Besides, dmatest never setup chan
> > > > ->private as other common case like uart/audio/spi will always setup
> > > > chan->private. Here check it to judge if it's dmatest case and do
> > > > jobs in slave_config.
> > >
> > > and you should not do anything for dmatest. Supporting it means memcpy
> > > implementation is not correct :)
> > Okay, I will any word about dmatest here since memcpy assume no calling
> > slave_config.
> > >
> > > >
> > > > Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> > > > ---
> > > > drivers/dma/imx-sdma.c | 37 ++++++++++++++++++++++++++++---------
> > > > 1 file changed, 28 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index
> > > > ed2267d..48f3749 100644
> > > > --- a/drivers/dma/imx-sdma.c
> > > > +++ b/drivers/dma/imx-sdma.c
> > > > @@ -1222,10 +1222,36 @@ static int sdma_alloc_chan_resources(struct
> > > > dma_chan *chan) {
> > > > struct sdma_channel *sdmac = to_sdma_chan(chan);
> > > > struct imx_dma_data *data = chan->private;
> > > > + struct imx_dma_data default_data;
> > > > int prio, ret;
> > > >
> > > > - if (!data)
> > > > - return -EINVAL;
> > > > + ret = clk_enable(sdmac->sdma->clk_ipg);
> > > > + if (ret)
> > > > + return ret;
> > > > + ret = clk_enable(sdmac->sdma->clk_ahb);
> > > > + if (ret)
> > > > + goto disable_clk_ipg;
> > > > + /*
> > > > + * dmatest(memcpy) will never call dmaengine_slave_config before prep,
> > > > + * so jobs in dmaengine_slave_config need to be moved into somewhere
> > > > + * before device_prep_dma_memcpy. Besides, dmatest never setup chan
> > > > + * ->private as other common cases like uart/audio/spi will setup
> > > > + * chan->private always. Here check it to judge if it's dmatest case
> > > > + * and do jobs in slave_config.
> > > > + */
> > > > + if (!data) {
> > > > + dev_warn(sdmac->sdma->dev, "dmatest is running?\n");
> > >
> > > why is that a warning!
> > Current SDMA driver assume filter function to set chan->private with specific data
> > (struct imx_dma_data dma_data)like below (sound/soc/fsl/fsl_asrc_dma.c):
> > static bool filter(struct dma_chan *chan, void *param)
> > {
> > if (!imx_dma_is_general_purpose(chan))
> > return false;
> > chan->private = param;
> > return true;
> > }
> >
> > But in memcpy case, at lease dmatest case, no chan->private set in its filter function.
> > So here take dmatest a special case and do some prepare jobs for memcpy. But if the
> > Upper device driver call dma_request_channel() with their specific filter without
> > 'chan->private' setting in the future. The warning message is a useful hint to them to
> > Add 'chan->private' in filter function. Or doc it somewhere?
>
> Instead of doing heuristics to guess whether we are doing memcpy you
> could instead make memcpy the default when slave_config is not called,
> i.e. drop the if (!data) check completely.
>
> > >
> > > > + sdmac->word_size = sdmac->sdma->dma_device.copy_align;
> > > > + default_data.priority = 2;
> > > > + default_data.peripheral_type = IMX_DMATYPE_MEMORY;
> > > > + default_data.dma_request = 0;
> > > > + default_data.dma_request2 = 0;
> > > > + data = &default_data;
> > > > +
> > > > + sdma_config_ownership(sdmac, false, true, false);
> > > > + sdma_get_pc(sdmac, IMX_DMATYPE_MEMORY);
> > > > + sdma_load_context(sdmac);
> > > > + }
> > >
> > > this needs to be default for memcpy
>
> The problem seems to be that we do not know whether we are doing memcpy
> or not. Normally we get the information how a channel is to be
> configured in dma_device->device_config, but this function is not called
> in the memcpy case.
Not really true, device_config only provides parameters to be
configured for next slave transaction
> An alternative might also be to do the setup in dma_device->device_prep_dma_memcpy.
Precisely, see how other drivers do this
Let's roll back a bit and foresee why is this required.
In case of memcpy, you need to tell DMA to do transfer from src to dstn
and size. Additional parameters like buswidth etc should be derived for
maximum throughput (after all we are dma, people want it to be done
fastest)
Now for slave, you are interfacing with a peripheral and don't know how
that is setup. So you need to match the parameters, otherwise you get
overflow or underflow and hence need for device_config
Please do not derive additional notions from these, please do not assume
anything else, unless provided in documentation :)
In doubt, just ask!
HTH
^ permalink raw reply
* [v1,3/4] dmaengine: imx-sdma: support dmatest
From: Robin Gong @ 2018-07-11 7:14 UTC (permalink / raw)
To: s.hauer@pengutronix.de
Cc: Vinod, dan.j.williams@intel.com, shawnguo@kernel.org,
Fabio Estevam, linux@armlinux.org.uk,
linux-arm-kernel@lists.infradead.org, kernel@pengutronix.de,
dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
dl-linux-imx
DQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IHMuaGF1ZXJAcGVuZ3V0cm9u
aXguZGUgW21haWx0bzpzLmhhdWVyQHBlbmd1dHJvbml4LmRlXQ0KPiBTZW50OiAyMDE45bm0N+ac
iDEx5pelIDE0OjU0DQo+IFRvOiBSb2JpbiBHb25nIDx5aWJpbi5nb25nQG54cC5jb20+DQo+IENj
OiBWaW5vZCA8dmtvdWxAa2VybmVsLm9yZz47IGRhbi5qLndpbGxpYW1zQGludGVsLmNvbTsNCj4g
c2hhd25ndW9Aa2VybmVsLm9yZzsgRmFiaW8gRXN0ZXZhbSA8ZmFiaW8uZXN0ZXZhbUBueHAuY29t
PjsNCj4gbGludXhAYXJtbGludXgub3JnLnVrOyBsaW51eC1hcm0ta2VybmVsQGxpc3RzLmluZnJh
ZGVhZC5vcmc7DQo+IGtlcm5lbEBwZW5ndXRyb25peC5kZTsgZG1hZW5naW5lQHZnZXIua2VybmVs
Lm9yZzsNCj4gbGludXgta2VybmVsQHZnZXIua2VybmVsLm9yZzsgZGwtbGludXgtaW14IDxsaW51
eC1pbXhAbnhwLmNvbT4NCj4gU3ViamVjdDogUmU6IFtQQVRDSCB2MSAzLzRdIGRtYWVuZ2luZTog
aW14LXNkbWE6IHN1cHBvcnQgZG1hdGVzdA0KPiANCj4gT24gV2VkLCBKdWwgMTEsIDIwMTggYXQg
MDY6Mzc6MDJBTSArMDAwMCwgUm9iaW4gR29uZyB3cm90ZToNCj4gPg0KPiA+ID4gLS0tLS1Pcmln
aW5hbCBNZXNzYWdlLS0tLS0NCj4gPiA+IEZyb206IFZpbm9kIFttYWlsdG86dmtvdWxAa2VybmVs
Lm9yZ10NCj4gPiA+IFNlbnQ6IDIwMTjlubQ35pyIMTDml6UgMjM6MzMNCj4gPiA+IFRvOiBSb2Jp
biBHb25nIDx5aWJpbi5nb25nQG54cC5jb20+DQo+ID4gPiBDYzogZGFuLmoud2lsbGlhbXNAaW50
ZWwuY29tOyBzaGF3bmd1b0BrZXJuZWwub3JnOw0KPiA+ID4gcy5oYXVlckBwZW5ndXRyb25peC5k
ZTsgRmFiaW8gRXN0ZXZhbSA8ZmFiaW8uZXN0ZXZhbUBueHAuY29tPjsNCj4gPiA+IGxpbnV4QGFy
bWxpbnV4Lm9yZy51azsgbGludXgtYXJtLWtlcm5lbEBsaXN0cy5pbmZyYWRlYWQub3JnOw0KPiA+
ID4ga2VybmVsQHBlbmd1dHJvbml4LmRlOyBkbWFlbmdpbmVAdmdlci5rZXJuZWwub3JnOw0KPiA+
ID4gbGludXgta2VybmVsQHZnZXIua2VybmVsLm9yZzsgZGwtbGludXgtaW14IDxsaW51eC1pbXhA
bnhwLmNvbT4NCj4gPiA+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggdjEgMy80XSBkbWFlbmdpbmU6IGlt
eC1zZG1hOiBzdXBwb3J0IGRtYXRlc3QNCj4gPiA+DQo+ID4gPiBPbiAxMS0wNy0xOCwgMDA6MjMs
IFJvYmluIEdvbmcgd3JvdGU6DQo+ID4gPiA+IGRtYXRlc3QobWVtY3B5KSB3aWxsIG5ldmVyIGNh
bGwgZG1hZW5naW5lX3NsYXZlX2NvbmZpZyBiZWZvcmUNCj4gPiA+ID4gcHJlcCwNCj4gPiA+DQo+
ID4gPiBhbmQgdGhhdCBzaG91bGQgaGF2ZSBiZWVuIGEgaGludCB0byB5b3UgdGhhdCB5b3Ugc2hv
dWxkIG5vdCBleHBlY3QNCj4gPiA+IHRoYXQNCj4gPiA+DQo+ID4gPiA+IHNvIGpvYnMgaW4gZG1h
ZW5naW5lX3NsYXZlX2NvbmZpZyBuZWVkIHRvIGJlIG1vdmVkIGludG8gc29tZXdoZXJlDQo+ID4g
PiA+IGJlZm9yZSBkZXZpY2VfcHJlcF9kbWFfbWVtY3B5LiBCZXNpZGVzLCBkbWF0ZXN0IG5ldmVy
IHNldHVwIGNoYW4NCj4gPiA+ID4gLT5wcml2YXRlIGFzIG90aGVyIGNvbW1vbiBjYXNlIGxpa2Ug
dWFydC9hdWRpby9zcGkgd2lsbCBhbHdheXMNCj4gPiA+ID4gLT5zZXR1cA0KPiA+ID4gPiBjaGFu
LT5wcml2YXRlLiBIZXJlIGNoZWNrIGl0IHRvIGp1ZGdlIGlmIGl0J3MgZG1hdGVzdCBjYXNlIGFu
ZCBkbw0KPiA+ID4gPiBqb2JzIGluIHNsYXZlX2NvbmZpZy4NCj4gPiA+DQo+ID4gPiBhbmQgeW91
IHNob3VsZCBub3QgZG8gYW55dGhpbmcgZm9yIGRtYXRlc3QuIFN1cHBvcnRpbmcgaXQgbWVhbnMN
Cj4gPiA+IG1lbWNweSBpbXBsZW1lbnRhdGlvbiBpcyBub3QgY29ycmVjdCA6KQ0KPiA+IE9rYXks
IEkgd2lsbCBhbnkgd29yZCBhYm91dCBkbWF0ZXN0IGhlcmUgc2luY2UgbWVtY3B5IGFzc3VtZSBu
bw0KPiA+IGNhbGxpbmcgc2xhdmVfY29uZmlnLg0KPiA+ID4NCj4gPiA+ID4NCj4gPiA+ID4gU2ln
bmVkLW9mZi1ieTogUm9iaW4gR29uZyA8eWliaW4uZ29uZ0BueHAuY29tPg0KPiA+ID4gPiAtLS0N
Cj4gPiA+ID4gIGRyaXZlcnMvZG1hL2lteC1zZG1hLmMgfCAzNyArKysrKysrKysrKysrKysrKysr
KysrKysrKysrLS0tLS0tLS0tDQo+ID4gPiA+ICAxIGZpbGUgY2hhbmdlZCwgMjggaW5zZXJ0aW9u
cygrKSwgOSBkZWxldGlvbnMoLSkNCj4gPiA+ID4NCj4gPiA+ID4gZGlmZiAtLWdpdCBhL2RyaXZl
cnMvZG1hL2lteC1zZG1hLmMgYi9kcml2ZXJzL2RtYS9pbXgtc2RtYS5jIGluZGV4DQo+ID4gPiA+
IGVkMjI2N2QuLjQ4ZjM3NDkgMTAwNjQ0DQo+ID4gPiA+IC0tLSBhL2RyaXZlcnMvZG1hL2lteC1z
ZG1hLmMNCj4gPiA+ID4gKysrIGIvZHJpdmVycy9kbWEvaW14LXNkbWEuYw0KPiA+ID4gPiBAQCAt
MTIyMiwxMCArMTIyMiwzNiBAQCBzdGF0aWMgaW50DQo+ID4gPiA+IHNkbWFfYWxsb2NfY2hhbl9y
ZXNvdXJjZXMoc3RydWN0IGRtYV9jaGFuICpjaGFuKSAgew0KPiA+ID4gPiAgCXN0cnVjdCBzZG1h
X2NoYW5uZWwgKnNkbWFjID0gdG9fc2RtYV9jaGFuKGNoYW4pOw0KPiA+ID4gPiAgCXN0cnVjdCBp
bXhfZG1hX2RhdGEgKmRhdGEgPSBjaGFuLT5wcml2YXRlOw0KPiA+ID4gPiArCXN0cnVjdCBpbXhf
ZG1hX2RhdGEgZGVmYXVsdF9kYXRhOw0KPiA+ID4gPiAgCWludCBwcmlvLCByZXQ7DQo+ID4gPiA+
DQo+ID4gPiA+IC0JaWYgKCFkYXRhKQ0KPiA+ID4gPiAtCQlyZXR1cm4gLUVJTlZBTDsNCj4gPiA+
ID4gKwlyZXQgPSBjbGtfZW5hYmxlKHNkbWFjLT5zZG1hLT5jbGtfaXBnKTsNCj4gPiA+ID4gKwlp
ZiAocmV0KQ0KPiA+ID4gPiArCQlyZXR1cm4gcmV0Ow0KPiA+ID4gPiArCXJldCA9IGNsa19lbmFi
bGUoc2RtYWMtPnNkbWEtPmNsa19haGIpOw0KPiA+ID4gPiArCWlmIChyZXQpDQo+ID4gPiA+ICsJ
CWdvdG8gZGlzYWJsZV9jbGtfaXBnOw0KPiA+ID4gPiArCS8qDQo+ID4gPiA+ICsJICogZG1hdGVz
dChtZW1jcHkpIHdpbGwgbmV2ZXIgY2FsbCBkbWFlbmdpbmVfc2xhdmVfY29uZmlnIGJlZm9yZQ0K
PiBwcmVwLA0KPiA+ID4gPiArCSAqIHNvIGpvYnMgaW4gZG1hZW5naW5lX3NsYXZlX2NvbmZpZyBu
ZWVkIHRvIGJlIG1vdmVkIGludG8NCj4gc29tZXdoZXJlDQo+ID4gPiA+ICsJICogYmVmb3JlIGRl
dmljZV9wcmVwX2RtYV9tZW1jcHkuIEJlc2lkZXMsIGRtYXRlc3QgbmV2ZXIgc2V0dXANCj4gY2hh
bg0KPiA+ID4gPiArCSAqIC0+cHJpdmF0ZSBhcyBvdGhlciBjb21tb24gY2FzZXMgbGlrZSB1YXJ0
L2F1ZGlvL3NwaSB3aWxsIHNldHVwDQo+ID4gPiA+ICsJICogY2hhbi0+cHJpdmF0ZSBhbHdheXMu
IEhlcmUgY2hlY2sgaXQgdG8ganVkZ2UgaWYgaXQncyBkbWF0ZXN0IGNhc2UNCj4gPiA+ID4gKwkg
KiBhbmQgZG8gam9icyBpbiBzbGF2ZV9jb25maWcuDQo+ID4gPiA+ICsJICovDQo+ID4gPiA+ICsJ
aWYgKCFkYXRhKSB7DQo+ID4gPiA+ICsJCWRldl93YXJuKHNkbWFjLT5zZG1hLT5kZXYsICJkbWF0
ZXN0IGlzIHJ1bm5pbmc/XG4iKTsNCj4gPiA+DQo+ID4gPiB3aHkgaXMgdGhhdCBhIHdhcm5pbmch
DQo+ID4gQ3VycmVudCBTRE1BIGRyaXZlciBhc3N1bWUgZmlsdGVyIGZ1bmN0aW9uIHRvIHNldCBj
aGFuLT5wcml2YXRlIHdpdGgNCj4gPiBzcGVjaWZpYyBkYXRhIChzdHJ1Y3QgaW14X2RtYV9kYXRh
IGRtYV9kYXRhKWxpa2UgYmVsb3cNCj4gKHNvdW5kL3NvYy9mc2wvZnNsX2FzcmNfZG1hLmMpOg0K
PiA+IHN0YXRpYyBib29sIGZpbHRlcihzdHJ1Y3QgZG1hX2NoYW4gKmNoYW4sIHZvaWQgKnBhcmFt
KSB7DQo+ID4gICAgICAgICBpZiAoIWlteF9kbWFfaXNfZ2VuZXJhbF9wdXJwb3NlKGNoYW4pKQ0K
PiA+ICAgICAgICAgICAgICAgICByZXR1cm4gZmFsc2U7DQo+ID4gICAgICAgICBjaGFuLT5wcml2
YXRlID0gcGFyYW07DQo+ID4gICAgICAgICByZXR1cm4gdHJ1ZTsNCj4gPiB9DQo+ID4NCj4gPiBC
dXQgaW4gbWVtY3B5IGNhc2UsIGF0IGxlYXNlIGRtYXRlc3QgY2FzZSwgbm8gY2hhbi0+cHJpdmF0
ZSBzZXQgaW4gaXRzIGZpbHRlcg0KPiBmdW5jdGlvbi4NCj4gPiBTbyBoZXJlIHRha2UgZG1hdGVz
dCBhIHNwZWNpYWwgY2FzZSBhbmQgZG8gc29tZSBwcmVwYXJlIGpvYnMgZm9yDQo+ID4gbWVtY3B5
LiBCdXQgaWYgdGhlIFVwcGVyIGRldmljZSBkcml2ZXIgY2FsbCBkbWFfcmVxdWVzdF9jaGFubmVs
KCkgd2l0aA0KPiA+IHRoZWlyIHNwZWNpZmljIGZpbHRlciB3aXRob3V0ICdjaGFuLT5wcml2YXRl
JyBzZXR0aW5nIGluIHRoZSBmdXR1cmUuDQo+ID4gVGhlIHdhcm5pbmcgbWVzc2FnZSBpcyBhIHVz
ZWZ1bCBoaW50IHRvIHRoZW0gdG8gQWRkICdjaGFuLT5wcml2YXRlJyBpbiBmaWx0ZXINCj4gZnVu
Y3Rpb24uICBPciBkb2MgaXQgc29tZXdoZXJlPw0KPiANCj4gSW5zdGVhZCBvZiBkb2luZyBoZXVy
aXN0aWNzIHRvIGd1ZXNzIHdoZXRoZXIgd2UgYXJlIGRvaW5nIG1lbWNweSB5b3UgY291bGQNCj4g
aW5zdGVhZCBtYWtlIG1lbWNweSB0aGUgZGVmYXVsdCB3aGVuIHNsYXZlX2NvbmZpZyBpcyBub3Qg
Y2FsbGVkLCBpLmUuIGRyb3AgdGhlDQo+IGlmICghZGF0YSkgY2hlY2sgY29tcGxldGVseS4NClll
cywgZm9yIG1lbWNweSBjYXNlLCB0aGF0J3MgYSBnb29kIHdheSwgYnV0IGhvdyB0byB3YXJuaW5n
IHRoZSBmdXR1cmUgY2FzZQ0KV2l0aG91dCBzZXR1cCAnY2hhbi0+cHJpdmF0ZScuLi4NCj4gDQo+
ID4gPg0KPiA+ID4gPiArCQlzZG1hYy0+d29yZF9zaXplICA9DQo+IHNkbWFjLT5zZG1hLT5kbWFf
ZGV2aWNlLmNvcHlfYWxpZ247DQo+ID4gPiA+ICsJCWRlZmF1bHRfZGF0YS5wcmlvcml0eSA9IDI7
DQo+ID4gPiA+ICsJCWRlZmF1bHRfZGF0YS5wZXJpcGhlcmFsX3R5cGUgPSBJTVhfRE1BVFlQRV9N
RU1PUlk7DQo+ID4gPiA+ICsJCWRlZmF1bHRfZGF0YS5kbWFfcmVxdWVzdCA9IDA7DQo+ID4gPiA+
ICsJCWRlZmF1bHRfZGF0YS5kbWFfcmVxdWVzdDIgPSAwOw0KPiA+ID4gPiArCQlkYXRhID0gJmRl
ZmF1bHRfZGF0YTsNCj4gPiA+ID4gKw0KPiA+ID4gPiArCQlzZG1hX2NvbmZpZ19vd25lcnNoaXAo
c2RtYWMsIGZhbHNlLCB0cnVlLCBmYWxzZSk7DQo+ID4gPiA+ICsJCXNkbWFfZ2V0X3BjKHNkbWFj
LCBJTVhfRE1BVFlQRV9NRU1PUlkpOw0KPiA+ID4gPiArCQlzZG1hX2xvYWRfY29udGV4dChzZG1h
Yyk7DQo+ID4gPiA+ICsJfQ0KPiA+ID4NCj4gPiA+IHRoaXMgbmVlZHMgdG8gYmUgZGVmYXVsdCBm
b3IgbWVtY3B5DQo+IA0KPiBUaGUgcHJvYmxlbSBzZWVtcyB0byBiZSB0aGF0IHdlIGRvIG5vdCBr
bm93IHdoZXRoZXIgd2UgYXJlIGRvaW5nIG1lbWNweQ0KPiBvciBub3QuIE5vcm1hbGx5IHdlIGdl
dCB0aGUgaW5mb3JtYXRpb24gaG93IGEgY2hhbm5lbCBpcyB0byBiZSBjb25maWd1cmVkIGluDQo+
IGRtYV9kZXZpY2UtPmRldmljZV9jb25maWcsIGJ1dCB0aGlzIGZ1bmN0aW9uIGlzIG5vdCBjYWxs
ZWQgaW4gdGhlIG1lbWNweSBjYXNlLg0KPiANCj4gQW4gYWx0ZXJuYXRpdmUgbWlnaHQgYWxzbyBi
ZSB0byBkbyB0aGUgc2V0dXAgaW4NCj4gZG1hX2RldmljZS0+ZGV2aWNlX3ByZXBfZG1hX21lbWNw
eS4NClllcywgSSd2ZSB0aGluayBhYm91dCBpdCBiZWZvcmUsIGJ1dCBzdWNoIHByZXBhcmUgc3Rl
cHMgb25seSBuZWVkZWQgaW4gb25jZSB0aW1lLi4uDQo+IA0KPiBTYXNjaGENCj4gDQo+IC0tDQo+
IFBlbmd1dHJvbml4IGUuSy4gICAgICAgICAgICAgICAgICAgICAgICAgICB8DQo+IHwNCj4gSW5k
dXN0cmlhbCBMaW51eCBTb2x1dGlvbnMgICAgICAgICAgICAgICAgIHwNCj4gaHR0cHM6Ly9lbWVh
MDEuc2FmZWxpbmtzLnByb3RlY3Rpb24ub3V0bG9vay5jb20vP3VybD1odHRwJTNBJTJGJTJGd3d3
Lg0KPiBwZW5ndXRyb25peC5kZSUyRiZhbXA7ZGF0YT0wMiU3QzAxJTdDeWliaW4uZ29uZyU0MG54
cC5jb20lN0MzZmNmMDMNCj4gZGIxMmY0NDEzOThmYmIwOGQ1ZTZmYjE0MmIlN0M2ODZlYTFkM2Jj
MmI0YzZmYTkyY2Q5OWM1YzMwMTYzNSU3QzANCj4gJTdDMCU3QzYzNjY2ODg4ODQxNjMyODk2MCZh
bXA7c2RhdGE9RTFEVDFCVzRiNVExVldna01OWnFBMjgNCj4gb0slMkZWVlF2aUM4cUYyJTJCcUcw
RmVvJTNEJmFtcDtyZXNlcnZlZD0wICB8DQo+IFBlaW5lciBTdHIuIDYtOCwgMzExMzcgSGlsZGVz
aGVpbSwgR2VybWFueSB8IFBob25lOiArNDktNTEyMS0yMDY5MTctMCAgICB8DQo+IEFtdHNnZXJp
Y2h0IEhpbGRlc2hlaW0sIEhSQSAyNjg2ICAgICAgICAgICB8IEZheDoNCj4gKzQ5LTUxMjEtMjA2
OTE3LTU1NTUgfA0K
---
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
^ permalink raw reply
* [v1,1/4] dmaengine: imx-sdma: add memcpy interface
From: Vinod Koul @ 2018-07-11 7:12 UTC (permalink / raw)
To: Robin Gong
Cc: dan.j.williams@intel.com, shawnguo@kernel.org,
s.hauer@pengutronix.de, Fabio Estevam, linux@armlinux.org.uk,
linux-arm-kernel@lists.infradead.org, kernel@pengutronix.de,
dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
dl-linux-imx
On 11-07-18, 05:34, Robin Gong wrote:
> > On 11-07-18, 00:23, Robin Gong wrote:
> > > Add MEMCPY support, meanwhile, add SDMA_BD_MAX_CNT instead of
> > > '0xffff'.
> >
> > latter part should be its own patch. Never mix things
> Okay, I will split it even for this minor change.
Yes, a patch should represent _one_ thing, describe that one thing and
do that one thing.
Anything else, doesn't matter how simple or complex should be an
individual patch
> > > + if (!desc)
> > > + goto err_out;
> > > +
> > > + do {
> > > + count = min_t(size_t, len, SDMA_BD_MAX_CNT);
> > > + bd = &desc->bd[i];
> > > + bd->buffer_addr = dma_src;
> > > + bd->ext_buffer_addr = dma_dst;
> > > + bd->mode.count = count;
> > > + desc->chn_count += count;
> > > +
> > > + switch (sdmac->word_size) {
> > > + case DMA_SLAVE_BUSWIDTH_4_BYTES:
> >
> > This looks wrong, we are in memcpy, there is no SLAVE so no SLAVE widths..
> >
> Okay, will remove check bus width.
it is not about bus_width but the fact that you are using slave
concepts. In memcpy we have _no_ slave, hence do not use anything
related to slave including dma_slave_config
^ permalink raw reply
* [v1,1/4] dmaengine: imx-sdma: add memcpy interface
From: Sascha Hauer @ 2018-07-11 7:08 UTC (permalink / raw)
To: Robin Gong
Cc: linux@armlinux.org.uk, linux-kernel@vger.kernel.org,
dmaengine@vger.kernel.org, vkoul@kernel.org, dl-linux-imx,
kernel@pengutronix.de, Fabio Estevam, dan.j.williams@intel.com,
shawnguo@kernel.org, linux-arm-kernel@lists.infradead.org
On Wed, Jul 11, 2018 at 07:05:23AM +0000, Robin Gong wrote:
>
>
> > -----Original Message-----
> > From: Sascha Hauer [mailto:s.hauer@pengutronix.de]
> > Sent: 2018年7月11日 15:01
> > To: Robin Gong <yibin.gong@nxp.com>
> > Cc: vkoul@kernel.org; dan.j.williams@intel.com; shawnguo@kernel.org; Fabio
> > Estevam <fabio.estevam@nxp.com>; linux@armlinux.org.uk;
> > linux-arm-kernel@lists.infradead.org; kernel@pengutronix.de;
> > dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
> > <linux-imx@nxp.com>
> > Subject: Re: [PATCH v1 1/4] dmaengine: imx-sdma: add memcpy interface
> >
> > On Wed, Jul 11, 2018 at 06:56:18AM +0000, Robin Gong wrote:
> > >
> > > > -----Original Message-----
> > > > From: Sascha Hauer [mailto:s.hauer@pengutronix.de]
> > > > Sent: 2018年7月11日 14:25
> > > > To: Robin Gong <yibin.gong@nxp.com>
> > > > Cc: vkoul@kernel.org; dan.j.williams@intel.com; shawnguo@kernel.org;
> > > > Fabio Estevam <fabio.estevam@nxp.com>; linux@armlinux.org.uk;
> > > > linux-arm-kernel@lists.infradead.org; kernel@pengutronix.de;
> > > > dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > > dl-linux-imx <linux-imx@nxp.com>
> > > > Subject: Re: [PATCH v1 1/4] dmaengine: imx-sdma: add memcpy
> > > > interface
> > > >
> > > > On Wed, Jul 11, 2018 at 12:23:10AM +0800, Robin Gong wrote:
> > > > > Add MEMCPY support, meanwhile, add SDMA_BD_MAX_CNT instead of
> > > > > '0xffff'.
> > > > >
> > > > > Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> > > > > ---
> > > > > +static struct dma_async_tx_descriptor *sdma_prep_memcpy(
> > > > > + struct dma_chan *chan, dma_addr_t dma_dst,
> > > > > + dma_addr_t dma_src, size_t len, unsigned long flags) {
> > > > > + struct sdma_channel *sdmac = to_sdma_chan(chan);
> > > > > + struct sdma_engine *sdma = sdmac->sdma;
> > > > > + int channel = sdmac->channel;
> > > > > + size_t count;
> > > > > + int i = 0, param;
> > > > > + struct sdma_buffer_descriptor *bd;
> > > > > + struct sdma_desc *desc;
> > > > > +
> > > > > + if (!chan || !len)
> > > > > + return NULL;
> > > > > +
> > > > > + dev_dbg(sdma->dev, "memcpy: %pad->%pad, len=%zu,
> > channel=%d.\n",
> > > > > + &dma_src, &dma_dst, len, channel);
> > > > > +
> > > > > + desc = sdma_transfer_init(sdmac, DMA_MEM_TO_MEM, len /
> > > > SDMA_BD_MAX_CNT
> > > > > + + 1);
> > > > > + if (!desc)
> > > > > + goto err_out;
> > > > > +
> > > > > + do {
> > > > > + count = min_t(size_t, len, SDMA_BD_MAX_CNT);
> > > >
> > > > When len is bigger than 0xffff you initialize count to 0xffff...
> > > In this case, the data will be split into several bds, for example, If
> > > the total count is 0x10000, two bd used then. One is for 0xffff,
> > > Another is for the last 1
> >
> > And you are doing byte size DMA? Wouldn't word size accesses be more
> > optimal?
> >
> > Sascha
> Default is words, and I'll force the buswidth to word and set into BD.
> sdma->dma_device.copy_align = DMAENGINE_ALIGN_4_BYTES;
So guess which alignment the second transfer has when the first has the
size 0xffff.
Sascha
^ permalink raw reply
* [v1,1/4] dmaengine: imx-sdma: add memcpy interface
From: Robin Gong @ 2018-07-11 7:05 UTC (permalink / raw)
To: Sascha Hauer
Cc: vkoul@kernel.org, dan.j.williams@intel.com, shawnguo@kernel.org,
Fabio Estevam, linux@armlinux.org.uk,
linux-arm-kernel@lists.infradead.org, kernel@pengutronix.de,
dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
dl-linux-imx
> -----Original Message-----
> From: Sascha Hauer [mailto:s.hauer@pengutronix.de]
> Sent: 2018年7月11日 15:01
> To: Robin Gong <yibin.gong@nxp.com>
> Cc: vkoul@kernel.org; dan.j.williams@intel.com; shawnguo@kernel.org; Fabio
> Estevam <fabio.estevam@nxp.com>; linux@armlinux.org.uk;
> linux-arm-kernel@lists.infradead.org; kernel@pengutronix.de;
> dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: Re: [PATCH v1 1/4] dmaengine: imx-sdma: add memcpy interface
>
> On Wed, Jul 11, 2018 at 06:56:18AM +0000, Robin Gong wrote:
> >
> > > -----Original Message-----
> > > From: Sascha Hauer [mailto:s.hauer@pengutronix.de]
> > > Sent: 2018年7月11日 14:25
> > > To: Robin Gong <yibin.gong@nxp.com>
> > > Cc: vkoul@kernel.org; dan.j.williams@intel.com; shawnguo@kernel.org;
> > > Fabio Estevam <fabio.estevam@nxp.com>; linux@armlinux.org.uk;
> > > linux-arm-kernel@lists.infradead.org; kernel@pengutronix.de;
> > > dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > dl-linux-imx <linux-imx@nxp.com>
> > > Subject: Re: [PATCH v1 1/4] dmaengine: imx-sdma: add memcpy
> > > interface
> > >
> > > On Wed, Jul 11, 2018 at 12:23:10AM +0800, Robin Gong wrote:
> > > > Add MEMCPY support, meanwhile, add SDMA_BD_MAX_CNT instead of
> > > > '0xffff'.
> > > >
> > > > Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> > > > ---
> > > > +static struct dma_async_tx_descriptor *sdma_prep_memcpy(
> > > > + struct dma_chan *chan, dma_addr_t dma_dst,
> > > > + dma_addr_t dma_src, size_t len, unsigned long flags) {
> > > > + struct sdma_channel *sdmac = to_sdma_chan(chan);
> > > > + struct sdma_engine *sdma = sdmac->sdma;
> > > > + int channel = sdmac->channel;
> > > > + size_t count;
> > > > + int i = 0, param;
> > > > + struct sdma_buffer_descriptor *bd;
> > > > + struct sdma_desc *desc;
> > > > +
> > > > + if (!chan || !len)
> > > > + return NULL;
> > > > +
> > > > + dev_dbg(sdma->dev, "memcpy: %pad->%pad, len=%zu,
> channel=%d.\n",
> > > > + &dma_src, &dma_dst, len, channel);
> > > > +
> > > > + desc = sdma_transfer_init(sdmac, DMA_MEM_TO_MEM, len /
> > > SDMA_BD_MAX_CNT
> > > > + + 1);
> > > > + if (!desc)
> > > > + goto err_out;
> > > > +
> > > > + do {
> > > > + count = min_t(size_t, len, SDMA_BD_MAX_CNT);
> > >
> > > When len is bigger than 0xffff you initialize count to 0xffff...
> > In this case, the data will be split into several bds, for example, If
> > the total count is 0x10000, two bd used then. One is for 0xffff,
> > Another is for the last 1
>
> And you are doing byte size DMA? Wouldn't word size accesses be more
> optimal?
>
> Sascha
Default is words, and I'll force the buswidth to word and set into BD.
sdma->dma_device.copy_align = DMAENGINE_ALIGN_4_BYTES;
>
>
> --
> Pengutronix e.K. |
> |
> Industrial Linux Solutions |
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> pengutronix.de%2F&data=02%7C01%7Cyibin.gong%40nxp.com%7C41fbd
> 3765b10424a7f1e08d5e6fc196d%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C
> 0%7C0%7C636668892793035822&sdata=mLKjTKaojuO1Zv%2F4ohwkkzeK
> FDFbmqYturqh6eSblIM%3D&reserved=0 |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax:
> +49-5121-206917-5555 |
^ permalink raw reply
* [v1,1/4] dmaengine: imx-sdma: add memcpy interface
From: Sascha Hauer @ 2018-07-11 7:01 UTC (permalink / raw)
To: Robin Gong
Cc: vkoul@kernel.org, dan.j.williams@intel.com, shawnguo@kernel.org,
Fabio Estevam, linux@armlinux.org.uk,
linux-arm-kernel@lists.infradead.org, kernel@pengutronix.de,
dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
dl-linux-imx
On Wed, Jul 11, 2018 at 06:56:18AM +0000, Robin Gong wrote:
>
> > -----Original Message-----
> > From: Sascha Hauer [mailto:s.hauer@pengutronix.de]
> > Sent: 2018年7月11日 14:25
> > To: Robin Gong <yibin.gong@nxp.com>
> > Cc: vkoul@kernel.org; dan.j.williams@intel.com; shawnguo@kernel.org; Fabio
> > Estevam <fabio.estevam@nxp.com>; linux@armlinux.org.uk;
> > linux-arm-kernel@lists.infradead.org; kernel@pengutronix.de;
> > dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
> > <linux-imx@nxp.com>
> > Subject: Re: [PATCH v1 1/4] dmaengine: imx-sdma: add memcpy interface
> >
> > On Wed, Jul 11, 2018 at 12:23:10AM +0800, Robin Gong wrote:
> > > Add MEMCPY support, meanwhile, add SDMA_BD_MAX_CNT instead of
> > > '0xffff'.
> > >
> > > Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> > > ---
> > > +static struct dma_async_tx_descriptor *sdma_prep_memcpy(
> > > + struct dma_chan *chan, dma_addr_t dma_dst,
> > > + dma_addr_t dma_src, size_t len, unsigned long flags) {
> > > + struct sdma_channel *sdmac = to_sdma_chan(chan);
> > > + struct sdma_engine *sdma = sdmac->sdma;
> > > + int channel = sdmac->channel;
> > > + size_t count;
> > > + int i = 0, param;
> > > + struct sdma_buffer_descriptor *bd;
> > > + struct sdma_desc *desc;
> > > +
> > > + if (!chan || !len)
> > > + return NULL;
> > > +
> > > + dev_dbg(sdma->dev, "memcpy: %pad->%pad, len=%zu, channel=%d.\n",
> > > + &dma_src, &dma_dst, len, channel);
> > > +
> > > + desc = sdma_transfer_init(sdmac, DMA_MEM_TO_MEM, len /
> > SDMA_BD_MAX_CNT
> > > + + 1);
> > > + if (!desc)
> > > + goto err_out;
> > > +
> > > + do {
> > > + count = min_t(size_t, len, SDMA_BD_MAX_CNT);
> >
> > When len is bigger than 0xffff you initialize count to 0xffff...
> In this case, the data will be split into several bds, for example,
> If the total count is 0x10000, two bd used then. One is for 0xffff,
> Another is for the last 1
And you are doing byte size DMA? Wouldn't word size accesses be more
optimal?
Sascha
^ permalink raw reply
* [v1,1/4] dmaengine: imx-sdma: add memcpy interface
From: Robin Gong @ 2018-07-11 6:56 UTC (permalink / raw)
To: Sascha Hauer
Cc: vkoul@kernel.org, dan.j.williams@intel.com, shawnguo@kernel.org,
Fabio Estevam, linux@armlinux.org.uk,
linux-arm-kernel@lists.infradead.org, kernel@pengutronix.de,
dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
dl-linux-imx
> -----Original Message-----
> From: Sascha Hauer [mailto:s.hauer@pengutronix.de]
> Sent: 2018年7月11日 14:25
> To: Robin Gong <yibin.gong@nxp.com>
> Cc: vkoul@kernel.org; dan.j.williams@intel.com; shawnguo@kernel.org; Fabio
> Estevam <fabio.estevam@nxp.com>; linux@armlinux.org.uk;
> linux-arm-kernel@lists.infradead.org; kernel@pengutronix.de;
> dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: Re: [PATCH v1 1/4] dmaengine: imx-sdma: add memcpy interface
>
> On Wed, Jul 11, 2018 at 12:23:10AM +0800, Robin Gong wrote:
> > Add MEMCPY support, meanwhile, add SDMA_BD_MAX_CNT instead of
> > '0xffff'.
> >
> > Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> > ---
> > +static struct dma_async_tx_descriptor *sdma_prep_memcpy(
> > + struct dma_chan *chan, dma_addr_t dma_dst,
> > + dma_addr_t dma_src, size_t len, unsigned long flags) {
> > + struct sdma_channel *sdmac = to_sdma_chan(chan);
> > + struct sdma_engine *sdma = sdmac->sdma;
> > + int channel = sdmac->channel;
> > + size_t count;
> > + int i = 0, param;
> > + struct sdma_buffer_descriptor *bd;
> > + struct sdma_desc *desc;
> > +
> > + if (!chan || !len)
> > + return NULL;
> > +
> > + dev_dbg(sdma->dev, "memcpy: %pad->%pad, len=%zu, channel=%d.\n",
> > + &dma_src, &dma_dst, len, channel);
> > +
> > + desc = sdma_transfer_init(sdmac, DMA_MEM_TO_MEM, len /
> SDMA_BD_MAX_CNT
> > + + 1);
> > + if (!desc)
> > + goto err_out;
> > +
> > + do {
> > + count = min_t(size_t, len, SDMA_BD_MAX_CNT);
>
> When len is bigger than 0xffff you initialize count to 0xffff...
In this case, the data will be split into several bds, for example,
If the total count is 0x10000, two bd used then. One is for 0xffff,
Another is for the last 1
>
> > + bd = &desc->bd[i];
> > + bd->buffer_addr = dma_src;
> > + bd->ext_buffer_addr = dma_dst;
> > + bd->mode.count = count;
> > + desc->chn_count += count;
> > +
> > + switch (sdmac->word_size) {
> > + case DMA_SLAVE_BUSWIDTH_4_BYTES:
> > + bd->mode.command = 0;
> > + if ((count | dma_src | dma_dst) & 3)
> > + goto err_bd_out;
> > + break;
>
> ...In which case you bail out here with an error.
In case dma_src/dma_dst is not align with bus width.
But I'll remove such bus width in v2 as Vinod comments.
>
> Please make sure bigger transfers are working.
>
> > + case DMA_SLAVE_BUSWIDTH_2_BYTES:
> > + bd->mode.command = 2;
> > + if ((count | dma_src | dma_dst) & 1)
> > + goto err_bd_out;
> > + break;
> > + case DMA_SLAVE_BUSWIDTH_1_BYTE:
> > + bd->mode.command = 1;
> > + break;
> > + default:
> > + goto err_bd_out;
> > + }
> > +
> > + dma_src += count;
> > + dma_dst += count;
> > + len -= count;
> > + i++;
> > +
> > + param = BD_DONE | BD_EXTD | BD_CONT;
>
> Probably better readable if you drop BD_CONT here and do a
>
> if (len) {
> param |= BD_CONT;
> } else {
> ...
> }
Okay. Will improve it.
>
> Sascha
>
> --
> Pengutronix e.K. |
> |
> Industrial Linux Solutions |
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> pengutronix.de%2F&data=02%7C01%7Cyibin.gong%40nxp.com%7C0aa96
> 135702b4414dbe708d5e6f709fe%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C
> 0%7C0%7C636668871060103585&sdata=k2h0RIWlujaCs8ioduJsnJAfGW0
> ZS7uzlHMMtjpQ%2Fw4%3D&reserved=0 |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax:
> +49-5121-206917-5555 |
^ permalink raw reply
* [v1,3/4] dmaengine: imx-sdma: support dmatest
From: Sascha Hauer @ 2018-07-11 6:53 UTC (permalink / raw)
To: Robin Gong
Cc: Vinod, dan.j.williams@intel.com, shawnguo@kernel.org,
Fabio Estevam, linux@armlinux.org.uk,
linux-arm-kernel@lists.infradead.org, kernel@pengutronix.de,
dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
dl-linux-imx
On Wed, Jul 11, 2018 at 06:37:02AM +0000, Robin Gong wrote:
>
> > -----Original Message-----
> > From: Vinod [mailto:vkoul@kernel.org]
> > Sent: 2018年7月10日 23:33
> > To: Robin Gong <yibin.gong@nxp.com>
> > Cc: dan.j.williams@intel.com; shawnguo@kernel.org;
> > s.hauer@pengutronix.de; Fabio Estevam <fabio.estevam@nxp.com>;
> > linux@armlinux.org.uk; linux-arm-kernel@lists.infradead.org;
> > kernel@pengutronix.de; dmaengine@vger.kernel.org;
> > linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> > Subject: Re: [PATCH v1 3/4] dmaengine: imx-sdma: support dmatest
> >
> > On 11-07-18, 00:23, Robin Gong wrote:
> > > dmatest(memcpy) will never call dmaengine_slave_config before prep,
> >
> > and that should have been a hint to you that you should not expect that
> >
> > > so jobs in dmaengine_slave_config need to be moved into somewhere
> > > before device_prep_dma_memcpy. Besides, dmatest never setup chan
> > > ->private as other common case like uart/audio/spi will always setup
> > > chan->private. Here check it to judge if it's dmatest case and do
> > > jobs in slave_config.
> >
> > and you should not do anything for dmatest. Supporting it means memcpy
> > implementation is not correct :)
> Okay, I will any word about dmatest here since memcpy assume no calling
> slave_config.
> >
> > >
> > > Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> > > ---
> > > drivers/dma/imx-sdma.c | 37 ++++++++++++++++++++++++++++---------
> > > 1 file changed, 28 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index
> > > ed2267d..48f3749 100644
> > > --- a/drivers/dma/imx-sdma.c
> > > +++ b/drivers/dma/imx-sdma.c
> > > @@ -1222,10 +1222,36 @@ static int sdma_alloc_chan_resources(struct
> > > dma_chan *chan) {
> > > struct sdma_channel *sdmac = to_sdma_chan(chan);
> > > struct imx_dma_data *data = chan->private;
> > > + struct imx_dma_data default_data;
> > > int prio, ret;
> > >
> > > - if (!data)
> > > - return -EINVAL;
> > > + ret = clk_enable(sdmac->sdma->clk_ipg);
> > > + if (ret)
> > > + return ret;
> > > + ret = clk_enable(sdmac->sdma->clk_ahb);
> > > + if (ret)
> > > + goto disable_clk_ipg;
> > > + /*
> > > + * dmatest(memcpy) will never call dmaengine_slave_config before prep,
> > > + * so jobs in dmaengine_slave_config need to be moved into somewhere
> > > + * before device_prep_dma_memcpy. Besides, dmatest never setup chan
> > > + * ->private as other common cases like uart/audio/spi will setup
> > > + * chan->private always. Here check it to judge if it's dmatest case
> > > + * and do jobs in slave_config.
> > > + */
> > > + if (!data) {
> > > + dev_warn(sdmac->sdma->dev, "dmatest is running?\n");
> >
> > why is that a warning!
> Current SDMA driver assume filter function to set chan->private with specific data
> (struct imx_dma_data dma_data)like below (sound/soc/fsl/fsl_asrc_dma.c):
> static bool filter(struct dma_chan *chan, void *param)
> {
> if (!imx_dma_is_general_purpose(chan))
> return false;
> chan->private = param;
> return true;
> }
>
> But in memcpy case, at lease dmatest case, no chan->private set in its filter function.
> So here take dmatest a special case and do some prepare jobs for memcpy. But if the
> Upper device driver call dma_request_channel() with their specific filter without
> 'chan->private' setting in the future. The warning message is a useful hint to them to
> Add 'chan->private' in filter function. Or doc it somewhere?
Instead of doing heuristics to guess whether we are doing memcpy you
could instead make memcpy the default when slave_config is not called,
i.e. drop the if (!data) check completely.
> >
> > > + sdmac->word_size = sdmac->sdma->dma_device.copy_align;
> > > + default_data.priority = 2;
> > > + default_data.peripheral_type = IMX_DMATYPE_MEMORY;
> > > + default_data.dma_request = 0;
> > > + default_data.dma_request2 = 0;
> > > + data = &default_data;
> > > +
> > > + sdma_config_ownership(sdmac, false, true, false);
> > > + sdma_get_pc(sdmac, IMX_DMATYPE_MEMORY);
> > > + sdma_load_context(sdmac);
> > > + }
> >
> > this needs to be default for memcpy
The problem seems to be that we do not know whether we are doing memcpy
or not. Normally we get the information how a channel is to be
configured in dma_device->device_config, but this function is not called
in the memcpy case.
An alternative might also be to do the setup in dma_device->device_prep_dma_memcpy.
Sascha
^ permalink raw reply
* [v1,3/4] dmaengine: imx-sdma: support dmatest
From: Robin Gong @ 2018-07-11 6:37 UTC (permalink / raw)
To: Vinod
Cc: dan.j.williams@intel.com, shawnguo@kernel.org,
s.hauer@pengutronix.de, Fabio Estevam, linux@armlinux.org.uk,
linux-arm-kernel@lists.infradead.org, kernel@pengutronix.de,
dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
dl-linux-imx
> -----Original Message-----
> From: Vinod [mailto:vkoul@kernel.org]
> Sent: 2018年7月10日 23:33
> To: Robin Gong <yibin.gong@nxp.com>
> Cc: dan.j.williams@intel.com; shawnguo@kernel.org;
> s.hauer@pengutronix.de; Fabio Estevam <fabio.estevam@nxp.com>;
> linux@armlinux.org.uk; linux-arm-kernel@lists.infradead.org;
> kernel@pengutronix.de; dmaengine@vger.kernel.org;
> linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH v1 3/4] dmaengine: imx-sdma: support dmatest
>
> On 11-07-18, 00:23, Robin Gong wrote:
> > dmatest(memcpy) will never call dmaengine_slave_config before prep,
>
> and that should have been a hint to you that you should not expect that
>
> > so jobs in dmaengine_slave_config need to be moved into somewhere
> > before device_prep_dma_memcpy. Besides, dmatest never setup chan
> > ->private as other common case like uart/audio/spi will always setup
> > chan->private. Here check it to judge if it's dmatest case and do
> > jobs in slave_config.
>
> and you should not do anything for dmatest. Supporting it means memcpy
> implementation is not correct :)
Okay, I will any word about dmatest here since memcpy assume no calling
slave_config.
>
> >
> > Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> > ---
> > drivers/dma/imx-sdma.c | 37 ++++++++++++++++++++++++++++---------
> > 1 file changed, 28 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index
> > ed2267d..48f3749 100644
> > --- a/drivers/dma/imx-sdma.c
> > +++ b/drivers/dma/imx-sdma.c
> > @@ -1222,10 +1222,36 @@ static int sdma_alloc_chan_resources(struct
> > dma_chan *chan) {
> > struct sdma_channel *sdmac = to_sdma_chan(chan);
> > struct imx_dma_data *data = chan->private;
> > + struct imx_dma_data default_data;
> > int prio, ret;
> >
> > - if (!data)
> > - return -EINVAL;
> > + ret = clk_enable(sdmac->sdma->clk_ipg);
> > + if (ret)
> > + return ret;
> > + ret = clk_enable(sdmac->sdma->clk_ahb);
> > + if (ret)
> > + goto disable_clk_ipg;
> > + /*
> > + * dmatest(memcpy) will never call dmaengine_slave_config before prep,
> > + * so jobs in dmaengine_slave_config need to be moved into somewhere
> > + * before device_prep_dma_memcpy. Besides, dmatest never setup chan
> > + * ->private as other common cases like uart/audio/spi will setup
> > + * chan->private always. Here check it to judge if it's dmatest case
> > + * and do jobs in slave_config.
> > + */
> > + if (!data) {
> > + dev_warn(sdmac->sdma->dev, "dmatest is running?\n");
>
> why is that a warning!
Current SDMA driver assume filter function to set chan->private with specific data
(struct imx_dma_data dma_data)like below (sound/soc/fsl/fsl_asrc_dma.c):
static bool filter(struct dma_chan *chan, void *param)
{
if (!imx_dma_is_general_purpose(chan))
return false;
chan->private = param;
return true;
}
But in memcpy case, at lease dmatest case, no chan->private set in its filter function.
So here take dmatest a special case and do some prepare jobs for memcpy. But if the
Upper device driver call dma_request_channel() with their specific filter without
'chan->private' setting in the future. The warning message is a useful hint to them to
Add 'chan->private' in filter function. Or doc it somewhere?
>
> > + sdmac->word_size = sdmac->sdma->dma_device.copy_align;
> > + default_data.priority = 2;
> > + default_data.peripheral_type = IMX_DMATYPE_MEMORY;
> > + default_data.dma_request = 0;
> > + default_data.dma_request2 = 0;
> > + data = &default_data;
> > +
> > + sdma_config_ownership(sdmac, false, true, false);
> > + sdma_get_pc(sdmac, IMX_DMATYPE_MEMORY);
> > + sdma_load_context(sdmac);
> > + }
>
> this needs to be default for memcpy
>
> --
> ~Vinod
^ permalink raw reply
* [v1,2/4] dmaengine: imx-sdma: add check_bd_buswidth() to kill the dulicated code
From: Sascha Hauer @ 2018-07-11 6:32 UTC (permalink / raw)
To: Robin Gong
Cc: vkoul, dan.j.williams, shawnguo, fabio.estevam, linux,
linux-arm-kernel, kernel, dmaengine, linux-kernel, linux-imx
On Wed, Jul 11, 2018 at 12:23:11AM +0800, Robin Gong wrote:
> Add check_bd_buswidth() to minimize the code size.
>
> Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> ---
> drivers/dma/imx-sdma.c | 64 +++++++++++++++++++++++---------------------------
> 1 file changed, 29 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 27ccabf..ed2267d 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -1326,6 +1326,33 @@ static struct sdma_desc *sdma_transfer_init(struct sdma_channel *sdmac,
> return NULL;
> }
>
> +static int check_bd_buswidth(struct sdma_buffer_descriptor *bd,
> + struct sdma_channel *sdmac, int count,
> + dma_addr_t dma_dst, dma_addr_t dma_src)
> +{
Better name this function set_bd_buswidth. I would assume that a
function named check_foo actually checks something, but doesn't set
anything.
Sascha
^ permalink raw reply
* [v1,1/4] dmaengine: imx-sdma: add memcpy interface
From: Sascha Hauer @ 2018-07-11 6:24 UTC (permalink / raw)
To: Robin Gong
Cc: vkoul, dan.j.williams, shawnguo, fabio.estevam, linux,
linux-arm-kernel, kernel, dmaengine, linux-kernel, linux-imx
On Wed, Jul 11, 2018 at 12:23:10AM +0800, Robin Gong wrote:
> Add MEMCPY support, meanwhile, add SDMA_BD_MAX_CNT instead
> of '0xffff'.
>
> Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> ---
> +static struct dma_async_tx_descriptor *sdma_prep_memcpy(
> + struct dma_chan *chan, dma_addr_t dma_dst,
> + dma_addr_t dma_src, size_t len, unsigned long flags)
> +{
> + struct sdma_channel *sdmac = to_sdma_chan(chan);
> + struct sdma_engine *sdma = sdmac->sdma;
> + int channel = sdmac->channel;
> + size_t count;
> + int i = 0, param;
> + struct sdma_buffer_descriptor *bd;
> + struct sdma_desc *desc;
> +
> + if (!chan || !len)
> + return NULL;
> +
> + dev_dbg(sdma->dev, "memcpy: %pad->%pad, len=%zu, channel=%d.\n",
> + &dma_src, &dma_dst, len, channel);
> +
> + desc = sdma_transfer_init(sdmac, DMA_MEM_TO_MEM, len / SDMA_BD_MAX_CNT
> + + 1);
> + if (!desc)
> + goto err_out;
> +
> + do {
> + count = min_t(size_t, len, SDMA_BD_MAX_CNT);
When len is bigger than 0xffff you initialize count to 0xffff...
> + bd = &desc->bd[i];
> + bd->buffer_addr = dma_src;
> + bd->ext_buffer_addr = dma_dst;
> + bd->mode.count = count;
> + desc->chn_count += count;
> +
> + switch (sdmac->word_size) {
> + case DMA_SLAVE_BUSWIDTH_4_BYTES:
> + bd->mode.command = 0;
> + if ((count | dma_src | dma_dst) & 3)
> + goto err_bd_out;
> + break;
...In which case you bail out here with an error.
Please make sure bigger transfers are working.
> + case DMA_SLAVE_BUSWIDTH_2_BYTES:
> + bd->mode.command = 2;
> + if ((count | dma_src | dma_dst) & 1)
> + goto err_bd_out;
> + break;
> + case DMA_SLAVE_BUSWIDTH_1_BYTE:
> + bd->mode.command = 1;
> + break;
> + default:
> + goto err_bd_out;
> + }
> +
> + dma_src += count;
> + dma_dst += count;
> + len -= count;
> + i++;
> +
> + param = BD_DONE | BD_EXTD | BD_CONT;
Probably better readable if you drop BD_CONT here and do a
if (len) {
param |= BD_CONT;
} else {
...
}
Sascha
^ permalink raw reply
* [v1,2/4] dmaengine: imx-sdma: add check_bd_buswidth() to kill the dulicated code
From: Robin Gong @ 2018-07-11 5:36 UTC (permalink / raw)
To: Vinod
Cc: dan.j.williams@intel.com, shawnguo@kernel.org,
s.hauer@pengutronix.de, Fabio Estevam, linux@armlinux.org.uk,
linux-arm-kernel@lists.infradead.org, kernel@pengutronix.de,
dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
dl-linux-imx
> -----Original Message-----
> From: Vinod [mailto:vkoul@kernel.org]
> Sent: 2018年7月10日 23:31
> To: Robin Gong <yibin.gong@nxp.com>
> Cc: dan.j.williams@intel.com; shawnguo@kernel.org;
> s.hauer@pengutronix.de; Fabio Estevam <fabio.estevam@nxp.com>;
> linux@armlinux.org.uk; linux-arm-kernel@lists.infradead.org;
> kernel@pengutronix.de; dmaengine@vger.kernel.org;
> linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH v1 2/4] dmaengine: imx-sdma: add check_bd_buswidth() to
> kill the dulicated code
>
> On 11-07-18, 00:23, Robin Gong wrote:
> > Add check_bd_buswidth() to minimize the code size.
>
> this looks mostly fine and I think this should be first patch..
Since no need to check bus width in memcpy case, I'll remove this patch too.
>
> >
> > Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> > ---
> > drivers/dma/imx-sdma.c | 64
> > +++++++++++++++++++++++---------------------------
> > 1 file changed, 29 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index
> > 27ccabf..ed2267d 100644
> > --- a/drivers/dma/imx-sdma.c
> > +++ b/drivers/dma/imx-sdma.c
> > @@ -1326,6 +1326,33 @@ static struct sdma_desc
> *sdma_transfer_init(struct sdma_channel *sdmac,
> > return NULL;
> > }
> >
> > +static int check_bd_buswidth(struct sdma_buffer_descriptor *bd,
> > + struct sdma_channel *sdmac, int count,
> > + dma_addr_t dma_dst, dma_addr_t dma_src) {
> > + int ret = 0;
> > +
> > + switch (sdmac->word_size) {
> > + case DMA_SLAVE_BUSWIDTH_4_BYTES:
> > + bd->mode.command = 0;
> > + if ((count | dma_dst | dma_src) & 3)
> > + ret = -EINVAL;
> > + break;
>
> empty line after each break please
>
> > + case DMA_SLAVE_BUSWIDTH_2_BYTES:
> > + bd->mode.command = 2;
> > + if ((count | dma_dst | dma_src) & 1)
> > + ret = -EINVAL;
> > + break;
> > + case DMA_SLAVE_BUSWIDTH_1_BYTE:
> > + bd->mode.command = 1;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return ret;
> > +}
> > +
> --
> ~Vinod
^ permalink raw reply
* [v1,1/4] dmaengine: imx-sdma: add memcpy interface
From: Robin Gong @ 2018-07-11 5:34 UTC (permalink / raw)
To: Vinod
Cc: dan.j.williams@intel.com, shawnguo@kernel.org,
s.hauer@pengutronix.de, Fabio Estevam, linux@armlinux.org.uk,
linux-arm-kernel@lists.infradead.org, kernel@pengutronix.de,
dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
dl-linux-imx
> Hi Robin,
>
> On 11-07-18, 00:23, Robin Gong wrote:
> > Add MEMCPY support, meanwhile, add SDMA_BD_MAX_CNT instead of
> > '0xffff'.
>
> latter part should be its own patch. Never mix things
Okay, I will split it even for this minor change.
>
> > +static struct dma_async_tx_descriptor *sdma_prep_memcpy(
> > + struct dma_chan *chan, dma_addr_t dma_dst,
> > + dma_addr_t dma_src, size_t len, unsigned long flags) {
> > + struct sdma_channel *sdmac = to_sdma_chan(chan);
> > + struct sdma_engine *sdma = sdmac->sdma;
> > + int channel = sdmac->channel;
> > + size_t count;
> > + int i = 0, param;
> > + struct sdma_buffer_descriptor *bd;
> > + struct sdma_desc *desc;
> > +
> > + if (!chan || !len)
> > + return NULL;
> > +
> > + dev_dbg(sdma->dev, "memcpy: %pad->%pad, len=%zu, channel=%d.\n",
> > + &dma_src, &dma_dst, len, channel);
> > +
> > + desc = sdma_transfer_init(sdmac, DMA_MEM_TO_MEM, len /
> SDMA_BD_MAX_CNT
> > + + 1);
>
> this looks quite odd to read consider:
>
> esc = sdma_transfer_init(sdmac, DMA_MEM_TO_MEM,
> len / SDMA_BD_MAX_CNT + 1);
>
Okay, will fix on v2.
> > + if (!desc)
> > + goto err_out;
> > +
> > + do {
> > + count = min_t(size_t, len, SDMA_BD_MAX_CNT);
> > + bd = &desc->bd[i];
> > + bd->buffer_addr = dma_src;
> > + bd->ext_buffer_addr = dma_dst;
> > + bd->mode.count = count;
> > + desc->chn_count += count;
> > +
> > + switch (sdmac->word_size) {
> > + case DMA_SLAVE_BUSWIDTH_4_BYTES:
>
> This looks wrong, we are in memcpy, there is no SLAVE so no SLAVE widths..
>
Okay, will remove check bus width.
> > static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
> > struct dma_chan *chan, struct scatterlist *sgl,
> > unsigned int sg_len, enum dma_transfer_direction direction, @@
> > -1344,9 +1431,9 @@ static struct dma_async_tx_descriptor
> > *sdma_prep_slave_sg(
> >
> > count = sg_dma_len(sg);
> >
> > - if (count > 0xffff) {
> > + if (count > SDMA_BD_MAX_CNT) {
> > dev_err(sdma->dev, "SDMA channel %d: maximum bytes for sg
> entry exceeded: %d > %d\n",
> > - channel, count, 0xffff);
> > + channel, count, SDMA_BD_MAX_CNT);
>
> these changes dont belong to this patch
Will split in v2.
>
> > @@ -1486,6 +1573,8 @@ static int sdma_config(struct dma_chan *chan,
> > sdmac->watermark_level |= (dmaengine_cfg->dst_maxburst << 16)
> &
> > SDMA_WATERMARK_LEVEL_HWML;
> > sdmac->word_size = dmaengine_cfg->dst_addr_width;
> > + } else if (dmaengine_cfg->direction == DMA_MEM_TO_MEM) {
> > + sdmac->word_size = dmaengine_cfg->dst_addr_width;
>
> same here too, we are in .device_config which deals with slave. Not memcpy!
Will remove it.
>
> > } else {
> > sdmac->per_address = dmaengine_cfg->dst_addr;
> > sdmac->watermark_level = dmaengine_cfg->dst_maxburst * @@
> -1902,6
> > +1991,7 @@ static int sdma_probe(struct platform_device *pdev)
> >
> > dma_cap_set(DMA_SLAVE, sdma->dma_device.cap_mask);
> > dma_cap_set(DMA_CYCLIC, sdma->dma_device.cap_mask);
> > + dma_cap_set(DMA_MEMCPY, sdma->dma_device.cap_mask);
> >
> > INIT_LIST_HEAD(&sdma->dma_device.channels);
> > /* Initialize channel parameters */
> > @@ -1968,9 +2058,11 @@ static int sdma_probe(struct platform_device
> *pdev)
> > sdma->dma_device.dst_addr_widths = SDMA_DMA_BUSWIDTHS;
> > sdma->dma_device.directions = SDMA_DMA_DIRECTIONS;
> > sdma->dma_device.residue_granularity =
> > DMA_RESIDUE_GRANULARITY_SEGMENT;
> > + sdma->dma_device.device_prep_dma_memcpy = sdma_prep_memcpy;
> > sdma->dma_device.device_issue_pending = sdma_issue_pending;
> > sdma->dma_device.dev->dma_parms = &sdma->dma_parms;
> > - dma_set_max_seg_size(sdma->dma_device.dev, 65535);
> > + sdma->dma_device.copy_align = DMAENGINE_ALIGN_4_BYTES;
> > + dma_set_max_seg_size(sdma->dma_device.dev, SDMA_BD_MAX_CNT);
>
> this line should not be part of this patch
>
> --
> ~Vinod
---
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
^ permalink raw reply
* [v2,2/2] dmaengine: sh: rcar-dmac: Add dma_pause operation
From: Yoshihiro Shimoda @ 2018-07-11 2:10 UTC (permalink / raw)
To: vinod.koul; +Cc: dmaengine, linux-renesas-soc, Yoshihiro Shimoda
This patch adds dma_pause operation. This patch is based on
Muhammad Hamza Farooq's patch.
After this patch applied, an issue that the sh-sci driver with
high baud rate might cause data lost disappeared because the DMAC
is possible to transmit between [1] and [2] below, and then
the residue of [1] is not true:
In rx_timer_fn() of the sh-sci.c:
dmaengine_pause();
...
dmaengine_tx_status(); /* [1] */
...
dmaengine_terminate_all(); /* [2] */
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
drivers/dma/sh/rcar-dmac.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index d3b7388..be82d69 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -834,6 +834,17 @@ static void rcar_dmac_stop_all_chan(struct rcar_dmac *dmac)
}
}
+static int rcar_dmac_chan_pause(struct dma_chan *chan)
+{
+ unsigned long flags;
+ struct rcar_dmac_chan *rchan = to_rcar_dmac_chan(chan);
+
+ spin_lock_irqsave(&rchan->lock, flags);
+ rcar_dmac_clear_chcr_de(rchan);
+ spin_unlock_irqrestore(&rchan->lock, flags);
+
+ return 0;
+}
/* -----------------------------------------------------------------------------
* Descriptors preparation
@@ -1864,6 +1875,7 @@ static int rcar_dmac_probe(struct platform_device *pdev)
engine->device_prep_slave_sg = rcar_dmac_prep_slave_sg;
engine->device_prep_dma_cyclic = rcar_dmac_prep_dma_cyclic;
engine->device_config = rcar_dmac_device_config;
+ engine->device_pause = rcar_dmac_chan_pause;
engine->device_terminate_all = rcar_dmac_chan_terminate_all;
engine->device_tx_status = rcar_dmac_tx_status;
engine->device_issue_pending = rcar_dmac_issue_pending;
^ permalink raw reply related
* [v2,1/2] dmaengine: sh: rcar-dmac: add a new function to clear CHCR.DE with barrier
From: Yoshihiro Shimoda @ 2018-07-11 2:10 UTC (permalink / raw)
To: vinod.koul; +Cc: dmaengine, linux-renesas-soc, Yoshihiro Shimoda
This patch adds a new function rcar_dmac_clear_chcr_de() to simplify
adding pause function later.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
drivers/dma/sh/rcar-dmac.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index 9906a9c..d3b7388 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -759,18 +759,25 @@ static void rcar_dmac_chcr_de_barrier(struct rcar_dmac_chan *chan)
dev_err(chan->chan.device->dev, "CHCR DE check error\n");
}
-static void rcar_dmac_sync_tcr(struct rcar_dmac_chan *chan)
+static void rcar_dmac_clear_chcr_de(struct rcar_dmac_chan *chan)
{
u32 chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR);
- if (!(chcr & RCAR_DMACHCR_DE))
- return;
-
/* set DE=0 and flush remaining data */
rcar_dmac_chan_write(chan, RCAR_DMACHCR, (chcr & ~RCAR_DMACHCR_DE));
/* make sure all remaining data was flushed */
rcar_dmac_chcr_de_barrier(chan);
+}
+
+static void rcar_dmac_sync_tcr(struct rcar_dmac_chan *chan)
+{
+ u32 chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR);
+
+ if (!(chcr & RCAR_DMACHCR_DE))
+ return;
+
+ rcar_dmac_clear_chcr_de(chan);
/* back DE if remain data exists */
if (rcar_dmac_chan_read(chan, RCAR_DMATCR))
^ permalink raw reply related
* [v1,4/4] ARM: configs: imx_v6_v7_defconfig: add DMATEST support
From: Robin Gong @ 2018-07-10 16:23 UTC (permalink / raw)
To: vkoul, dan.j.williams, shawnguo, s.hauer, fabio.estevam, linux
Cc: linux-arm-kernel, kernel, dmaengine, linux-kernel, linux-imx
Add DMATEST support and remove invalid options, such as
CONFIG_BT_HCIUART_H4 is default enabled and CONFIG_SND_SOC_IMX_WM8962
is out of date and not appear in any config file.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
arch/arm/configs/imx_v6_v7_defconfig | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
index e381d05..f28d4d9 100644
--- a/arch/arm/configs/imx_v6_v7_defconfig
+++ b/arch/arm/configs/imx_v6_v7_defconfig
@@ -81,7 +81,6 @@ CONFIG_CAN=y
CONFIG_CAN_FLEXCAN=y
CONFIG_BT=y
CONFIG_BT_HCIUART=y
-CONFIG_BT_HCIUART_H4=y
CONFIG_BT_HCIUART_LL=y
CONFIG_CFG80211=y
CONFIG_CFG80211_WEXT=y
@@ -282,7 +281,6 @@ CONFIG_SND_SOC_FSL_ASRC=y
CONFIG_SND_IMX_SOC=y
CONFIG_SND_SOC_PHYCORE_AC97=y
CONFIG_SND_SOC_EUKREA_TLV320=y
-CONFIG_SND_SOC_IMX_WM8962=y
CONFIG_SND_SOC_IMX_ES8328=y
CONFIG_SND_SOC_IMX_SGTL5000=y
CONFIG_SND_SOC_IMX_SPDIF=y
@@ -371,6 +369,7 @@ CONFIG_DMADEVICES=y
CONFIG_FSL_EDMA=y
CONFIG_IMX_SDMA=y
CONFIG_MXS_DMA=y
+CONFIG_DMATEST=m
CONFIG_STAGING=y
CONFIG_STAGING_MEDIA=y
CONFIG_VIDEO_IMX_MEDIA=y
^ permalink raw reply related
* [v1,3/4] dmaengine: imx-sdma: support dmatest
From: Robin Gong @ 2018-07-10 16:23 UTC (permalink / raw)
To: vkoul, dan.j.williams, shawnguo, s.hauer, fabio.estevam, linux
Cc: linux-arm-kernel, kernel, dmaengine, linux-kernel, linux-imx
dmatest(memcpy) will never call dmaengine_slave_config before prep,
so jobs in dmaengine_slave_config need to be moved into somewhere
before device_prep_dma_memcpy. Besides, dmatest never setup chan
->private as other common case like uart/audio/spi will always setup
chan->private. Here check it to judge if it's dmatest case and do
jobs in slave_config.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/dma/imx-sdma.c | 37 ++++++++++++++++++++++++++++---------
1 file changed, 28 insertions(+), 9 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index ed2267d..48f3749 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1222,10 +1222,36 @@ static int sdma_alloc_chan_resources(struct dma_chan *chan)
{
struct sdma_channel *sdmac = to_sdma_chan(chan);
struct imx_dma_data *data = chan->private;
+ struct imx_dma_data default_data;
int prio, ret;
- if (!data)
- return -EINVAL;
+ ret = clk_enable(sdmac->sdma->clk_ipg);
+ if (ret)
+ return ret;
+ ret = clk_enable(sdmac->sdma->clk_ahb);
+ if (ret)
+ goto disable_clk_ipg;
+ /*
+ * dmatest(memcpy) will never call dmaengine_slave_config before prep,
+ * so jobs in dmaengine_slave_config need to be moved into somewhere
+ * before device_prep_dma_memcpy. Besides, dmatest never setup chan
+ * ->private as other common cases like uart/audio/spi will setup
+ * chan->private always. Here check it to judge if it's dmatest case
+ * and do jobs in slave_config.
+ */
+ if (!data) {
+ dev_warn(sdmac->sdma->dev, "dmatest is running?\n");
+ sdmac->word_size = sdmac->sdma->dma_device.copy_align;
+ default_data.priority = 2;
+ default_data.peripheral_type = IMX_DMATYPE_MEMORY;
+ default_data.dma_request = 0;
+ default_data.dma_request2 = 0;
+ data = &default_data;
+
+ sdma_config_ownership(sdmac, false, true, false);
+ sdma_get_pc(sdmac, IMX_DMATYPE_MEMORY);
+ sdma_load_context(sdmac);
+ }
switch (data->priority) {
case DMA_PRIO_HIGH:
@@ -1244,13 +1270,6 @@ static int sdma_alloc_chan_resources(struct dma_chan *chan)
sdmac->event_id0 = data->dma_request;
sdmac->event_id1 = data->dma_request2;
- ret = clk_enable(sdmac->sdma->clk_ipg);
- if (ret)
- return ret;
- ret = clk_enable(sdmac->sdma->clk_ahb);
- if (ret)
- goto disable_clk_ipg;
-
ret = sdma_set_channel_priority(sdmac, prio);
if (ret)
goto disable_clk_ahb;
^ permalink raw reply related
* [v1,2/4] dmaengine: imx-sdma: add check_bd_buswidth() to kill the dulicated code
From: Robin Gong @ 2018-07-10 16:23 UTC (permalink / raw)
To: vkoul, dan.j.williams, shawnguo, s.hauer, fabio.estevam, linux
Cc: linux-arm-kernel, kernel, dmaengine, linux-kernel, linux-imx
Add check_bd_buswidth() to minimize the code size.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/dma/imx-sdma.c | 64 +++++++++++++++++++++++---------------------------
1 file changed, 29 insertions(+), 35 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 27ccabf..ed2267d 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1326,6 +1326,33 @@ static struct sdma_desc *sdma_transfer_init(struct sdma_channel *sdmac,
return NULL;
}
+static int check_bd_buswidth(struct sdma_buffer_descriptor *bd,
+ struct sdma_channel *sdmac, int count,
+ dma_addr_t dma_dst, dma_addr_t dma_src)
+{
+ int ret = 0;
+
+ switch (sdmac->word_size) {
+ case DMA_SLAVE_BUSWIDTH_4_BYTES:
+ bd->mode.command = 0;
+ if ((count | dma_dst | dma_src) & 3)
+ ret = -EINVAL;
+ break;
+ case DMA_SLAVE_BUSWIDTH_2_BYTES:
+ bd->mode.command = 2;
+ if ((count | dma_dst | dma_src) & 1)
+ ret = -EINVAL;
+ break;
+ case DMA_SLAVE_BUSWIDTH_1_BYTE:
+ bd->mode.command = 1;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
static struct dma_async_tx_descriptor *sdma_prep_memcpy(
struct dma_chan *chan, dma_addr_t dma_dst,
dma_addr_t dma_src, size_t len, unsigned long flags)
@@ -1357,23 +1384,8 @@ static struct dma_async_tx_descriptor *sdma_prep_memcpy(
bd->mode.count = count;
desc->chn_count += count;
- switch (sdmac->word_size) {
- case DMA_SLAVE_BUSWIDTH_4_BYTES:
- bd->mode.command = 0;
- if ((count | dma_src | dma_dst) & 3)
- goto err_bd_out;
- break;
- case DMA_SLAVE_BUSWIDTH_2_BYTES:
- bd->mode.command = 2;
- if ((count | dma_src | dma_dst) & 1)
- goto err_bd_out;
- break;
- case DMA_SLAVE_BUSWIDTH_1_BYTE:
- bd->mode.command = 1;
- break;
- default:
+ if (check_bd_buswidth(bd, sdmac, count, dma_dst, dma_src))
goto err_bd_out;
- }
dma_src += count;
dma_dst += count;
@@ -1440,27 +1452,9 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
bd->mode.count = count;
desc->chn_count += count;
- if (sdmac->word_size > DMA_SLAVE_BUSWIDTH_4_BYTES)
+ if (check_bd_buswidth(bd, sdmac, count, 0, sg->dma_address))
goto err_bd_out;
- switch (sdmac->word_size) {
- case DMA_SLAVE_BUSWIDTH_4_BYTES:
- bd->mode.command = 0;
- if (count & 3 || sg->dma_address & 3)
- goto err_bd_out;
- break;
- case DMA_SLAVE_BUSWIDTH_2_BYTES:
- bd->mode.command = 2;
- if (count & 1 || sg->dma_address & 1)
- goto err_bd_out;
- break;
- case DMA_SLAVE_BUSWIDTH_1_BYTE:
- bd->mode.command = 1;
- break;
- default:
- goto err_bd_out;
- }
-
param = BD_DONE | BD_EXTD | BD_CONT;
if (i + 1 == sg_len) {
^ permalink raw reply related
* [v1,1/4] dmaengine: imx-sdma: add memcpy interface
From: Robin Gong @ 2018-07-10 16:23 UTC (permalink / raw)
To: vkoul, dan.j.williams, shawnguo, s.hauer, fabio.estevam, linux
Cc: linux-arm-kernel, kernel, dmaengine, linux-kernel, linux-imx
Add MEMCPY support, meanwhile, add SDMA_BD_MAX_CNT instead
of '0xffff'.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/dma/imx-sdma.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 98 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 3b622d6..27ccabf 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -341,6 +341,7 @@ struct sdma_desc {
* @pc_from_device: script address for those device_2_memory
* @pc_to_device: script address for those memory_2_device
* @device_to_device: script address for those device_2_device
+ * @pc_to_pc: script address for those memory_2_memory
* @flags: loop mode or not
* @per_address: peripheral source or destination address in common case
* destination address in p_2_p case
@@ -366,6 +367,7 @@ struct sdma_channel {
enum dma_slave_buswidth word_size;
unsigned int pc_from_device, pc_to_device;
unsigned int device_to_device;
+ unsigned int pc_to_pc;
unsigned long flags;
dma_addr_t per_address, per_address2;
unsigned long event_mask[2];
@@ -385,6 +387,8 @@ struct sdma_channel {
#define SDMA_FIRMWARE_MAGIC 0x414d4453
+#define SDMA_BD_MAX_CNT 0xffff
+
/**
* struct sdma_firmware_header - Layout of the firmware image
*
@@ -868,14 +872,16 @@ static void sdma_get_pc(struct sdma_channel *sdmac,
* These are needed once we start to support transfers between
* two peripherals or memory-to-memory transfers
*/
- int per_2_per = 0;
+ int per_2_per = 0, emi_2_emi = 0;
sdmac->pc_from_device = 0;
sdmac->pc_to_device = 0;
sdmac->device_to_device = 0;
+ sdmac->pc_to_pc = 0;
switch (peripheral_type) {
case IMX_DMATYPE_MEMORY:
+ emi_2_emi = sdma->script_addrs->ap_2_ap_addr;
break;
case IMX_DMATYPE_DSP:
emi_2_per = sdma->script_addrs->bp_2_ap_addr;
@@ -948,6 +954,7 @@ static void sdma_get_pc(struct sdma_channel *sdmac,
sdmac->pc_from_device = per_2_emi;
sdmac->pc_to_device = emi_2_per;
sdmac->device_to_device = per_2_per;
+ sdmac->pc_to_pc = emi_2_emi;
}
static int sdma_load_context(struct sdma_channel *sdmac)
@@ -964,6 +971,8 @@ static int sdma_load_context(struct sdma_channel *sdmac)
load_address = sdmac->pc_from_device;
else if (sdmac->direction == DMA_DEV_TO_DEV)
load_address = sdmac->device_to_device;
+ else if (sdmac->direction == DMA_MEM_TO_MEM)
+ load_address = sdmac->pc_to_pc;
else
load_address = sdmac->pc_to_device;
@@ -1317,6 +1326,84 @@ static struct sdma_desc *sdma_transfer_init(struct sdma_channel *sdmac,
return NULL;
}
+static struct dma_async_tx_descriptor *sdma_prep_memcpy(
+ struct dma_chan *chan, dma_addr_t dma_dst,
+ dma_addr_t dma_src, size_t len, unsigned long flags)
+{
+ struct sdma_channel *sdmac = to_sdma_chan(chan);
+ struct sdma_engine *sdma = sdmac->sdma;
+ int channel = sdmac->channel;
+ size_t count;
+ int i = 0, param;
+ struct sdma_buffer_descriptor *bd;
+ struct sdma_desc *desc;
+
+ if (!chan || !len)
+ return NULL;
+
+ dev_dbg(sdma->dev, "memcpy: %pad->%pad, len=%zu, channel=%d.\n",
+ &dma_src, &dma_dst, len, channel);
+
+ desc = sdma_transfer_init(sdmac, DMA_MEM_TO_MEM, len / SDMA_BD_MAX_CNT
+ + 1);
+ if (!desc)
+ goto err_out;
+
+ do {
+ count = min_t(size_t, len, SDMA_BD_MAX_CNT);
+ bd = &desc->bd[i];
+ bd->buffer_addr = dma_src;
+ bd->ext_buffer_addr = dma_dst;
+ bd->mode.count = count;
+ desc->chn_count += count;
+
+ switch (sdmac->word_size) {
+ case DMA_SLAVE_BUSWIDTH_4_BYTES:
+ bd->mode.command = 0;
+ if ((count | dma_src | dma_dst) & 3)
+ goto err_bd_out;
+ break;
+ case DMA_SLAVE_BUSWIDTH_2_BYTES:
+ bd->mode.command = 2;
+ if ((count | dma_src | dma_dst) & 1)
+ goto err_bd_out;
+ break;
+ case DMA_SLAVE_BUSWIDTH_1_BYTE:
+ bd->mode.command = 1;
+ break;
+ default:
+ goto err_bd_out;
+ }
+
+ dma_src += count;
+ dma_dst += count;
+ len -= count;
+ i++;
+
+ param = BD_DONE | BD_EXTD | BD_CONT;
+ /* last bd */
+ if (!len) {
+ param |= BD_INTR;
+ param |= BD_LAST;
+ param &= ~BD_CONT;
+ }
+
+ dev_dbg(sdma->dev, "entry %d: count: %zd dma: 0x%x %s%s\n",
+ i, count, bd->buffer_addr,
+ param & BD_WRAP ? "wrap" : "",
+ param & BD_INTR ? " intr" : "");
+
+ bd->mode.status = param;
+ } while (len);
+
+ return vchan_tx_prep(&sdmac->vc, &desc->vd, flags);
+err_bd_out:
+ sdma_free_bd(desc);
+ kfree(desc);
+err_out:
+ return NULL;
+}
+
static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
struct dma_chan *chan, struct scatterlist *sgl,
unsigned int sg_len, enum dma_transfer_direction direction,
@@ -1344,9 +1431,9 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
count = sg_dma_len(sg);
- if (count > 0xffff) {
+ if (count > SDMA_BD_MAX_CNT) {
dev_err(sdma->dev, "SDMA channel %d: maximum bytes for sg entry exceeded: %d > %d\n",
- channel, count, 0xffff);
+ channel, count, SDMA_BD_MAX_CNT);
goto err_bd_out;
}
@@ -1421,9 +1508,9 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
sdmac->flags |= IMX_DMA_SG_LOOP;
- if (period_len > 0xffff) {
+ if (period_len > SDMA_BD_MAX_CNT) {
dev_err(sdma->dev, "SDMA channel %d: maximum period size exceeded: %zu > %d\n",
- channel, period_len, 0xffff);
+ channel, period_len, SDMA_BD_MAX_CNT);
goto err_bd_out;
}
@@ -1486,6 +1573,8 @@ static int sdma_config(struct dma_chan *chan,
sdmac->watermark_level |= (dmaengine_cfg->dst_maxburst << 16) &
SDMA_WATERMARK_LEVEL_HWML;
sdmac->word_size = dmaengine_cfg->dst_addr_width;
+ } else if (dmaengine_cfg->direction == DMA_MEM_TO_MEM) {
+ sdmac->word_size = dmaengine_cfg->dst_addr_width;
} else {
sdmac->per_address = dmaengine_cfg->dst_addr;
sdmac->watermark_level = dmaengine_cfg->dst_maxburst *
@@ -1902,6 +1991,7 @@ static int sdma_probe(struct platform_device *pdev)
dma_cap_set(DMA_SLAVE, sdma->dma_device.cap_mask);
dma_cap_set(DMA_CYCLIC, sdma->dma_device.cap_mask);
+ dma_cap_set(DMA_MEMCPY, sdma->dma_device.cap_mask);
INIT_LIST_HEAD(&sdma->dma_device.channels);
/* Initialize channel parameters */
@@ -1968,9 +2058,11 @@ static int sdma_probe(struct platform_device *pdev)
sdma->dma_device.dst_addr_widths = SDMA_DMA_BUSWIDTHS;
sdma->dma_device.directions = SDMA_DMA_DIRECTIONS;
sdma->dma_device.residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT;
+ sdma->dma_device.device_prep_dma_memcpy = sdma_prep_memcpy;
sdma->dma_device.device_issue_pending = sdma_issue_pending;
sdma->dma_device.dev->dma_parms = &sdma->dma_parms;
- dma_set_max_seg_size(sdma->dma_device.dev, 65535);
+ sdma->dma_device.copy_align = DMAENGINE_ALIGN_4_BYTES;
+ dma_set_max_seg_size(sdma->dma_device.dev, SDMA_BD_MAX_CNT);
platform_set_drvdata(pdev, sdma);
^ permalink raw reply related
* [06/14] dmaengine: dma-jz4780: Add support for the JZ4725B SoC
From: Paul Cercueil @ 2018-07-10 15:45 UTC (permalink / raw)
To: Vinod
Cc: Rob Herring, Mark Rutland, Ralf Baechle, Paul Burton, James Hogan,
Zubair Lutfullah Kakakhel, Mathieu Malaterre, Daniel Silsby,
dmaengine, devicetree, linux-kernel, linux-mips
Le lun. 9 juil. 2018 à 19:14, Vinod <vkoul@kernel.org> a écrit :
> On 03-07-18, 14:32, Paul Cercueil wrote:
>> The JZ4725B has one DMA core starring six DMA channels.
>> As for the JZ4770, each DMA channel's clock can be enabled with
>> a register write, the difference here being that once started, it
>> is not possible to turn it off.
>
> ok so disable for this, right..
>
>> @@ -204,6 +205,8 @@ static inline void
>> jz4780_dma_chan_enable(struct jz4780_dma_dev *jzdma,
>> {
>> if (jzdma->version == ID_JZ4770)
>> jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DCKES, BIT(chn));
>> + else if (jzdma->version == ID_JZ4725B)
>> + jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DCKE, BIT(chn));
>
> but you are writing to a different register here..
Yes. SoCs >= JZ4770 have the DCKE read-only register, and DCKES/DCKEC
to set/clear bits in DCKE.
On JZ4725B, DCKE is read/write, but the zeros written are ignored (at
least that's what the
documentation says).
> --
> ~Vinod
Thanks,
-Paul
---
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
^ permalink raw reply
* [04/14] dmaengine: dma-jz4780: Add support for the JZ4770 SoC
From: Paul Cercueil @ 2018-07-10 15:41 UTC (permalink / raw)
To: Vinod
Cc: Rob Herring, Mark Rutland, Ralf Baechle, Paul Burton, James Hogan,
Zubair Lutfullah Kakakhel, Mathieu Malaterre, Daniel Silsby,
dmaengine, devicetree, linux-kernel, linux-mips
Le lun. 9 juil. 2018 à 19:10, Vinod <vkoul@kernel.org> a écrit :
> On 03-07-18, 14:32, Paul Cercueil wrote:
>
>> +static inline void jz4780_dma_chan_disable(struct jz4780_dma_dev
>> *jzdma,
>> + unsigned int chn)
>> +{
>> + if (jzdma->version == ID_JZ4770)
>> + jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DCKEC, BIT(chn));
>> +}
>
> this sounds as hardware behaviour, so why not describe as a property
> in
> DT?
See my response to your message on patch 1.
>> +
>> static struct jz4780_dma_desc *jz4780_dma_desc_alloc(
>> struct jz4780_dma_chan *jzchan, unsigned int count,
>> enum dma_transaction_type type)
>> @@ -228,8 +246,15 @@ static void jz4780_dma_desc_free(struct
>> virt_dma_desc *vdesc)
>> kfree(desc);
>> }
>>
>> -static uint32_t jz4780_dma_transfer_size(unsigned long val,
>> uint32_t *shift)
>> +static const unsigned int jz4780_dma_ord_max[] = {
>> + [ID_JZ4770] = 6,
>> + [ID_JZ4780] = 7,
>> +};
>
> So this gives the transfer length supported?
Yes, exactly. The maximum transfer size is (1 << ord).
> --
> ~Vinod
---
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
^ permalink raw reply
* [v1] dmaengine: hsu: Support dmaengine_terminate_sync()
From: Vinod Koul @ 2018-07-10 15:40 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: dmaengine
On 10-07-18, 14:49, Andy Shevchenko wrote:
> It appears that the driver misses the support of dmaengine_terminate_sync().
> Since many of callers expects this behaviour implement the new
> device_synchronize() callback to allow proper synchronization when stopping
> a channel.
Applied, thanks
^ permalink raw reply
* [02/14] dmaengine: dma-jz4780: Separate chan/ctrl registers
From: Paul Cercueil @ 2018-07-10 15:36 UTC (permalink / raw)
To: Vinod
Cc: Rob Herring, Mark Rutland, Ralf Baechle, Paul Burton, James Hogan,
Zubair Lutfullah Kakakhel, Mathieu Malaterre, Daniel Silsby,
dmaengine, devicetree, linux-kernel, linux-mips
Le lun. 9 juil. 2018 à 19:03, Vinod <vkoul@kernel.org> a écrit :
> On 03-07-18, 14:32, Paul Cercueil wrote:
>> The register area of the JZ4780 DMA core can be split into different
>> sections for different purposes:
>>
>> * one set of registers is used to perform actions at the DMA core
>> level,
>> that will generally affect all channels;
>>
>> * one set of registers per DMA channel, to perform actions at the
>> DMA
>> channel level, that will only affect the channel in question.
>>
>> The problem rises when trying to support new versions of the JZ47xx
>> Ingenic SoC. For instance, the JZ4770 has two DMA cores, each one
>> with six DMA channels, and the register sets are interleaved:
>> <DMA0 chan regs> <DMA1 chan regs> <DMA0 ctrl regs> <DMA1 ctrl regs>
>>
>> By using one memory resource for the channel-specific registers and
>> one memory resource for the core-specific registers, we can support
>> the JZ4770, by initializing the driver once per DMA core with
>> different
>> addresses.
>>
>> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>> ---
>> .../devicetree/bindings/dma/jz4780-dma.txt | 6 +-
>
> Pls move to separate patch.
OK.
>> drivers/dma/dma-jz4780.c | 106
>> +++++++++++-------
>> 2 files changed, 69 insertions(+), 43 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/dma/jz4780-dma.txt
>> b/Documentation/devicetree/bindings/dma/jz4780-dma.txt
>> index f25feee62b15..f9b1864f5b77 100644
>> --- a/Documentation/devicetree/bindings/dma/jz4780-dma.txt
>> +++ b/Documentation/devicetree/bindings/dma/jz4780-dma.txt
>> @@ -3,7 +3,8 @@
>> Required properties:
>>
>> - compatible: Should be "ingenic,jz4780-dma"
>> -- reg: Should contain the DMA controller registers location and
>> length.
>> +- reg: Should contain the DMA channel registers location and
>> length, followed
>> + by the DMA controller registers location and length.
>> - interrupts: Should contain the interrupt specifier of the DMA
>> controller.
>> - interrupt-parent: Should be the phandle of the interrupt
>> controller that
>> - clocks: Should contain a clock specifier for the JZ4780 PDMA
>> clock.
>> @@ -22,7 +23,8 @@ Example:
>>
>> dma: dma@13420000 {
>> compatible = "ingenic,jz4780-dma";
>> - reg = <0x13420000 0x10000>;
>> + reg = <0x13420000 0x400
>> + 0x13421000 0x40>;
>
> Second should be optional or we break platform which may not have
> updated DT..
See comment below.
>> - jzdma->base = devm_ioremap_resource(dev, res);
>> - if (IS_ERR(jzdma->base))
>> - return PTR_ERR(jzdma->base);
>> + jzdma->chn_base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(jzdma->chn_base))
>> + return PTR_ERR(jzdma->chn_base);
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> + if (!res) {
>> + dev_err(dev, "failed to get I/O memory\n");
>> + return -EINVAL;
>> + }
>
> okay and this breaks if you happen to get probed on older DT. I think
> DT
> is treated as ABI so you need to continue support older method while
> finding if DT has split resources
See my response to PrasannaKumar. All the Ingenic-based boards do
compile
the devicetree within the kernel, so I think it's still fine to add
breaking
changes. I'll wait on @Rob to give his point of view on this, though.
(It's not something hard to change, but I'd like to know what's the
policy
in that case. I have other DT-breaking patches to submit)
> --
> ~Vinod
---
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
^ permalink raw reply
* [v1,3/4] dmaengine: imx-sdma: support dmatest
From: Vinod Koul @ 2018-07-10 15:33 UTC (permalink / raw)
To: Robin Gong
Cc: dan.j.williams, shawnguo, s.hauer, fabio.estevam, linux,
linux-arm-kernel, kernel, dmaengine, linux-kernel, linux-imx
On 11-07-18, 00:23, Robin Gong wrote:
> dmatest(memcpy) will never call dmaengine_slave_config before prep,
and that should have been a hint to you that you should not expect that
> so jobs in dmaengine_slave_config need to be moved into somewhere
> before device_prep_dma_memcpy. Besides, dmatest never setup chan
> ->private as other common case like uart/audio/spi will always setup
> chan->private. Here check it to judge if it's dmatest case and do
> jobs in slave_config.
and you should not do anything for dmatest. Supporting it means memcpy
implementation is not correct :)
>
> Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> ---
> drivers/dma/imx-sdma.c | 37 ++++++++++++++++++++++++++++---------
> 1 file changed, 28 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index ed2267d..48f3749 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -1222,10 +1222,36 @@ static int sdma_alloc_chan_resources(struct dma_chan *chan)
> {
> struct sdma_channel *sdmac = to_sdma_chan(chan);
> struct imx_dma_data *data = chan->private;
> + struct imx_dma_data default_data;
> int prio, ret;
>
> - if (!data)
> - return -EINVAL;
> + ret = clk_enable(sdmac->sdma->clk_ipg);
> + if (ret)
> + return ret;
> + ret = clk_enable(sdmac->sdma->clk_ahb);
> + if (ret)
> + goto disable_clk_ipg;
> + /*
> + * dmatest(memcpy) will never call dmaengine_slave_config before prep,
> + * so jobs in dmaengine_slave_config need to be moved into somewhere
> + * before device_prep_dma_memcpy. Besides, dmatest never setup chan
> + * ->private as other common cases like uart/audio/spi will setup
> + * chan->private always. Here check it to judge if it's dmatest case
> + * and do jobs in slave_config.
> + */
> + if (!data) {
> + dev_warn(sdmac->sdma->dev, "dmatest is running?\n");
why is that a warning!
> + sdmac->word_size = sdmac->sdma->dma_device.copy_align;
> + default_data.priority = 2;
> + default_data.peripheral_type = IMX_DMATYPE_MEMORY;
> + default_data.dma_request = 0;
> + default_data.dma_request2 = 0;
> + data = &default_data;
> +
> + sdma_config_ownership(sdmac, false, true, false);
> + sdma_get_pc(sdmac, IMX_DMATYPE_MEMORY);
> + sdma_load_context(sdmac);
> + }
this needs to be default for memcpy
^ permalink raw reply
* [v1,2/4] dmaengine: imx-sdma: add check_bd_buswidth() to kill the dulicated code
From: Vinod Koul @ 2018-07-10 15:31 UTC (permalink / raw)
To: Robin Gong
Cc: dan.j.williams, shawnguo, s.hauer, fabio.estevam, linux,
linux-arm-kernel, kernel, dmaengine, linux-kernel, linux-imx
On 11-07-18, 00:23, Robin Gong wrote:
> Add check_bd_buswidth() to minimize the code size.
this looks mostly fine and I think this should be first patch..
>
> Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> ---
> drivers/dma/imx-sdma.c | 64 +++++++++++++++++++++++---------------------------
> 1 file changed, 29 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 27ccabf..ed2267d 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -1326,6 +1326,33 @@ static struct sdma_desc *sdma_transfer_init(struct sdma_channel *sdmac,
> return NULL;
> }
>
> +static int check_bd_buswidth(struct sdma_buffer_descriptor *bd,
> + struct sdma_channel *sdmac, int count,
> + dma_addr_t dma_dst, dma_addr_t dma_src)
> +{
> + int ret = 0;
> +
> + switch (sdmac->word_size) {
> + case DMA_SLAVE_BUSWIDTH_4_BYTES:
> + bd->mode.command = 0;
> + if ((count | dma_dst | dma_src) & 3)
> + ret = -EINVAL;
> + break;
empty line after each break please
> + case DMA_SLAVE_BUSWIDTH_2_BYTES:
> + bd->mode.command = 2;
> + if ((count | dma_dst | dma_src) & 1)
> + ret = -EINVAL;
> + break;
> + case DMA_SLAVE_BUSWIDTH_1_BYTE:
> + bd->mode.command = 1;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return ret;
> +}
> +
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox