| Message ID | 1411550642-12577-1-git-send-email-jingchang.lu@freescale.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
Hi, Jingchang Lu wrote: > The offset of all 8-/16-bit register in big-endian eDMA model are s/register/registers/ > swapped in a 32-bit size opposite those in the little-endian model. > > The hardware Scatter/Gather requires the subsequent TCDs in memory > to be auto loaded should retain little endian independent of the > register endian model, the dma engine will do the swap if need. > > This patch also use regular assignment for tcd variables r/w > instead of with io function previously that may not always be true. > > Signed-off-by: Jingchang Lu <jingchang.lu@freescale.com> > --- > changes in v3: > use unsigned long instead of u32 in reg offset swap cast to avoid warning. > > changes in v2: > simplify register offset swap calculation. > use regular assignment for tcd variables r/w instead of io function. > > drivers/dma/fsl-edma.c | 106 ++++++++++++++++++++++++++----------------------- > 1 file changed, 57 insertions(+), 49 deletions(-) > > diff --git a/drivers/dma/fsl-edma.c b/drivers/dma/fsl-edma.c > index 3c5711d..9ca55ee 100644 > --- a/drivers/dma/fsl-edma.c > +++ b/drivers/dma/fsl-edma.c > @@ -177,16 +177,10 @@ struct fsl_edma_engine { > /* > * R/W functions for big- or little-endian registers > * the eDMA controller's endian is independent of the CPU core's endian. > + * for the big-endian IP module, the offset for 8-bit or 16-bit register s/for/For/ > + * should also be swapped oposite to that in little-endian IP. s/oposite/opposite/ [...] > @@ -459,20 +460,27 @@ static void fill_tcd_params(struct fsl_edma_engine *edma, > u16 csr = 0; > > /* > - * eDMA hardware SGs require the TCD parameters stored in memory > - * the same endian as the eDMA module so that they can be loaded > - * automatically by the engine > + * eDMA hardware SGs requires the TCDs to be auto loaded > + * in the little endian whenver the register endian model, "in little endian whatever the register endian model" > + * So we put the value in little endian in memory, waitting s/waitting/waiting/ > + * for fsl_set_tcd_params doing the swap. fsl_set_tcd_params() Lothar Waßmann
On Wed, Sep 24, 2014 at 01:05:10PM +0200, Lothar Waßmann wrote: > Hi, > > Jingchang Lu wrote: > > + * eDMA hardware SGs requires the TCDs to be auto loaded > > + * in the little endian whenver the register endian model, > "in little endian whatever the register endian model" Even that took several reads to work it out. eDMA hardware SGs require the TCDs to be stored in little endian format irrespective of the register endian model. and I think that's all that needs to be said here. However, as I realdy suggested, running this ruddy thing through sparse would be a /very/ good idea, because it'll warn with: + tcd->daddr = cpu_to_le32(dst); The reason it'll warn on that is because daddr is not declared with __le32, etc - the types used in struct fsl_edma_hw_tcd tell sparse that the values to be stored there are in _host_ endian format, but they're being assigned little endian formatted data. > > + * for fsl_set_tcd_params doing the swap. > fsl_set_tcd_params() That's the wrong function name anyway. It's fsl_edma_set_tcd_params(). However, let's look at this a little more: fsl_edma_set_tcd_params(fsl_chan, tcd->saddr, tcd->daddr, tcd->attr, tcd->soff, tcd->nbytes, tcd->slast, tcd->citer, tcd->biter, tcd->doff, tcd->dlast_sga, tcd->csr); Is it /really/ a good idea to read all that data out of the structure, only to then have most of it saved into the stack, which fsl_edma_set_tcd_params() then has to read back off the stack again? With stuff like this, one has to wonder if there is much clue how to write optimal C code in this driver. This should be passing the tcd structure into fsl_edma_set_tcd_params(). Now, this function contains this comment: /* * TCD parameters have been swapped in fill_tcd_params(), * so just write them to registers in the cpu endian here */ Which is almost reasonable, but let's update it: TCD parameters are stored in struct fsl_edma_hw_tcd in little endian format. However, we need to load the registers in <insert appropriate endianness - big|little|cpu> endian. Now, remember that writel() and friends expect native CPU endian formatted data (and yes, sparse checks this too) so that needs to be considered more. Lastly, the driver seems to be a total mess of accessors. In some places it uses io{read,write}*, in other places, it uses plain {read,write}*. It should use either one set or the other set, and not mix these two. I just spotted this badly formatted code too: for (i = 0; i < fsl_desc->n_tcds; i++) dma_pool_free(fsl_desc->echan->tcd_pool, fsl_desc->tcd[i].vtcd, fsl_desc->tcd[i].ptcd); ... edma_writeb(fsl_chan->edma, EDMAMUX_CHCFG_ENBL | EDMAMUX_CHCFG_SOURCE(slot), muxaddr + ch_off);
Pi0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+RnJvbTogTG90aGFyIFdhw59tYW5uIFttYWls dG86TFdAS0FSTy1lbGVjdHJvbmljcy5kZV0NCj5TZW50OiBXZWRuZXNkYXksIFNlcHRlbWJlciAy NCwgMjAxNCA3OjA1IFBNDQo+VG86IEx1IEppbmdjaGFuZy1CMzUwODMNCj5DYzogdmlub2Qua291 bEBpbnRlbC5jb207IGxpbnV4QGFybS5saW51eC5vcmcudWs7IGFybmRAYXJuZGIuZGU7IGxpbnV4 LQ0KPmtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7IGRtYWVuZ2luZUB2Z2VyLmtlcm5lbC5vcmc7IGxp bnV4LWFybS0NCj5rZXJuZWxAbGlzdHMuaW5mcmFkZWFkLm9yZw0KPlN1YmplY3Q6IFJlOiBbUEFU Q0h2M10gZG1hZW5naW5lOiBmc2wtZWRtYTogZml4dXAgcmVnIG9mZnNldCBhbmQgaHcgUy9HDQo+ c3VwcG9ydCBpbiBiaWctZW5kaWFuIG1vZGVsDQo+DQo+SGksDQo+DQo+SmluZ2NoYW5nIEx1IHdy b3RlOg0KPj4gVGhlIG9mZnNldCBvZiBhbGwgOC0vMTYtYml0IHJlZ2lzdGVyIGluIGJpZy1lbmRp YW4gZURNQSBtb2RlbCBhcmUNCj5zL3JlZ2lzdGVyL3JlZ2lzdGVycy8NCj4NCj4+IHN3YXBwZWQg aW4gYSAzMi1iaXQgc2l6ZSBvcHBvc2l0ZSB0aG9zZSBpbiB0aGUgbGl0dGxlLWVuZGlhbiBtb2Rl bC4NCj4+DQo+PiBUaGUgaGFyZHdhcmUgU2NhdHRlci9HYXRoZXIgcmVxdWlyZXMgdGhlIHN1YnNl cXVlbnQgVENEcyBpbiBtZW1vcnkgdG8NCj4+IGJlIGF1dG8gbG9hZGVkIHNob3VsZCByZXRhaW4g bGl0dGxlIGVuZGlhbiBpbmRlcGVuZGVudCBvZiB0aGUgcmVnaXN0ZXINCj4+IGVuZGlhbiBtb2Rl bCwgdGhlIGRtYSBlbmdpbmUgd2lsbCBkbyB0aGUgc3dhcCBpZiBuZWVkLg0KPj4NCj4+IFRoaXMg cGF0Y2ggYWxzbyB1c2UgcmVndWxhciBhc3NpZ25tZW50IGZvciB0Y2QgdmFyaWFibGVzIHIvdyBp bnN0ZWFkDQo+PiBvZiB3aXRoIGlvIGZ1bmN0aW9uIHByZXZpb3VzbHkgdGhhdCBtYXkgbm90IGFs d2F5cyBiZSB0cnVlLg0KPj4NCj4+IFNpZ25lZC1vZmYtYnk6IEppbmdjaGFuZyBMdSA8amluZ2No YW5nLmx1QGZyZWVzY2FsZS5jb20+DQo+PiAtLS0NCj4+IGNoYW5nZXMgaW4gdjM6DQo+PiAgdXNl IHVuc2lnbmVkIGxvbmcgaW5zdGVhZCBvZiB1MzIgaW4gcmVnIG9mZnNldCBzd2FwIGNhc3QgdG8g YXZvaWQNCj53YXJuaW5nLg0KPj4NCj4+IGNoYW5nZXMgaW4gdjI6DQo+PiAgc2ltcGxpZnkgcmVn aXN0ZXIgb2Zmc2V0IHN3YXAgY2FsY3VsYXRpb24uDQo+PiAgdXNlIHJlZ3VsYXIgYXNzaWdubWVu dCBmb3IgdGNkIHZhcmlhYmxlcyByL3cgaW5zdGVhZCBvZiBpbyBmdW5jdGlvbi4NCj4+DQo+PiAg ZHJpdmVycy9kbWEvZnNsLWVkbWEuYyB8IDEwNg0KPj4gKysrKysrKysrKysrKysrKysrKysrKysr KystLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQ0KPj4gIDEgZmlsZSBjaGFuZ2VkLCA1NyBpbnNlcnRp b25zKCspLCA0OSBkZWxldGlvbnMoLSkNCj4+DQo+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9kbWEv ZnNsLWVkbWEuYyBiL2RyaXZlcnMvZG1hL2ZzbC1lZG1hLmMgaW5kZXgNCj4+IDNjNTcxMWQuLjlj YTU1ZWUgMTAwNjQ0DQo+PiAtLS0gYS9kcml2ZXJzL2RtYS9mc2wtZWRtYS5jDQo+PiArKysgYi9k cml2ZXJzL2RtYS9mc2wtZWRtYS5jDQo+PiBAQCAtMTc3LDE2ICsxNzcsMTAgQEAgc3RydWN0IGZz bF9lZG1hX2VuZ2luZSB7DQo+PiAgLyoNCj4+ICAgKiBSL1cgZnVuY3Rpb25zIGZvciBiaWctIG9y IGxpdHRsZS1lbmRpYW4gcmVnaXN0ZXJzDQo+PiAgICogdGhlIGVETUEgY29udHJvbGxlcidzIGVu ZGlhbiBpcyBpbmRlcGVuZGVudCBvZiB0aGUgQ1BVIGNvcmUncyBlbmRpYW4uDQo+PiArICogZm9y IHRoZSBiaWctZW5kaWFuIElQIG1vZHVsZSwgdGhlIG9mZnNldCBmb3IgOC1iaXQgb3IgMTYtYml0 DQo+PiArIHJlZ2lzdGVyDQo+cy9mb3IvRm9yLw0KPg0KPj4gKyAqIHNob3VsZCBhbHNvIGJlIHN3 YXBwZWQgb3Bvc2l0ZSB0byB0aGF0IGluIGxpdHRsZS1lbmRpYW4gSVAuDQo+cy9vcG9zaXRlL29w cG9zaXRlLw0KPg0KPlsuLi5dDQo+PiBAQCAtNDU5LDIwICs0NjAsMjcgQEAgc3RhdGljIHZvaWQg ZmlsbF90Y2RfcGFyYW1zKHN0cnVjdCBmc2xfZWRtYV9lbmdpbmUNCj4qZWRtYSwNCj4+ICAJdTE2 IGNzciA9IDA7DQo+Pg0KPj4gIAkvKg0KPj4gLQkgKiBlRE1BIGhhcmR3YXJlIFNHcyByZXF1aXJl IHRoZSBUQ0QgcGFyYW1ldGVycyBzdG9yZWQgaW4gbWVtb3J5DQo+PiAtCSAqIHRoZSBzYW1lIGVu ZGlhbiBhcyB0aGUgZURNQSBtb2R1bGUgc28gdGhhdCB0aGV5IGNhbiBiZSBsb2FkZWQNCj4+IC0J ICogYXV0b21hdGljYWxseSBieSB0aGUgZW5naW5lDQo+PiArCSAqIGVETUEgaGFyZHdhcmUgU0dz IHJlcXVpcmVzIHRoZSBUQ0RzIHRvIGJlIGF1dG8gbG9hZGVkDQo+PiArCSAqIGluIHRoZSBsaXR0 bGUgZW5kaWFuIHdoZW52ZXIgdGhlIHJlZ2lzdGVyIGVuZGlhbiBtb2RlbCwNCj4iaW4gbGl0dGxl IGVuZGlhbiB3aGF0ZXZlciB0aGUgcmVnaXN0ZXIgZW5kaWFuIG1vZGVsIg0KPg0KPj4gKwkgKiBT byB3ZSBwdXQgdGhlIHZhbHVlIGluIGxpdHRsZSBlbmRpYW4gaW4gbWVtb3J5LCB3YWl0dGluZw0K PnMvd2FpdHRpbmcvd2FpdGluZy8NCj4NCj4+ICsJICogZm9yIGZzbF9zZXRfdGNkX3BhcmFtcyBk b2luZyB0aGUgc3dhcC4NCj5mc2xfc2V0X3RjZF9wYXJhbXMoKQ0KPg0KPg0KPg0KPkxvdGhhciBX YcOfbWFubg0KPi0tDQpUaGFua3MsIEkgd2lsbCBhbWVuZCB0aGVtIGluIHRoZSBuZXh0IHZlcnNp b24gcGF0Y2guDQoNCkJlc3QgUmVnYXJkcywNCkppbmdjaGFuZw0K -- 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
Pi0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+RnJvbTogUnVzc2VsbCBLaW5nIC0gQVJNIExp bnV4IFttYWlsdG86bGludXhAYXJtLmxpbnV4Lm9yZy51a10NCj5TZW50OiBXZWRuZXNkYXksIFNl cHRlbWJlciAyNCwgMjAxNCA4OjA0IFBNDQo+VG86IExvdGhhciBXYcOfbWFubg0KPkNjOiBMdSBK aW5nY2hhbmctQjM1MDgzOyB2aW5vZC5rb3VsQGludGVsLmNvbTsgYXJuZEBhcm5kYi5kZTsgbGlu dXgtDQo+a2VybmVsQHZnZXIua2VybmVsLm9yZzsgZG1hZW5naW5lQHZnZXIua2VybmVsLm9yZzsg bGludXgtYXJtLQ0KPmtlcm5lbEBsaXN0cy5pbmZyYWRlYWQub3JnDQo+U3ViamVjdDogUmU6IFtQ QVRDSHYzXSBkbWFlbmdpbmU6IGZzbC1lZG1hOiBmaXh1cCByZWcgb2Zmc2V0IGFuZCBodyBTL0cN Cj5zdXBwb3J0IGluIGJpZy1lbmRpYW4gbW9kZWwNCj4NCj5PbiBXZWQsIFNlcCAyNCwgMjAxNCBh dCAwMTowNToxMFBNICswMjAwLCBMb3RoYXIgV2HDn21hbm4gd3JvdGU6DQo+PiBIaSwNCj4+DQo+ PiBKaW5nY2hhbmcgTHUgd3JvdGU6DQo+PiA+ICsJICogZURNQSBoYXJkd2FyZSBTR3MgcmVxdWly ZXMgdGhlIFRDRHMgdG8gYmUgYXV0byBsb2FkZWQNCj4+ID4gKwkgKiBpbiB0aGUgbGl0dGxlIGVu ZGlhbiB3aGVudmVyIHRoZSByZWdpc3RlciBlbmRpYW4gbW9kZWwsDQo+PiAiaW4gbGl0dGxlIGVu ZGlhbiB3aGF0ZXZlciB0aGUgcmVnaXN0ZXIgZW5kaWFuIG1vZGVsIg0KPg0KPkV2ZW4gdGhhdCB0 b29rIHNldmVyYWwgcmVhZHMgdG8gd29yayBpdCBvdXQuDQo+DQo+CWVETUEgaGFyZHdhcmUgU0dz IHJlcXVpcmUgdGhlIFRDRHMgdG8gYmUgc3RvcmVkIGluIGxpdHRsZSBlbmRpYW4NCj4JZm9ybWF0 IGlycmVzcGVjdGl2ZSBvZiB0aGUgcmVnaXN0ZXIgZW5kaWFuIG1vZGVsLg0KPg0KPmFuZCBJIHRo aW5rIHRoYXQncyBhbGwgdGhhdCBuZWVkcyB0byBiZSBzYWlkIGhlcmUuDQo+DQo+SG93ZXZlciwg YXMgSSByZWFsZHkgc3VnZ2VzdGVkLCBydW5uaW5nIHRoaXMgcnVkZHkgdGhpbmcgdGhyb3VnaCBz cGFyc2UNCj53b3VsZCBiZSBhIC92ZXJ5LyBnb29kIGlkZWEsIGJlY2F1c2UgaXQnbGwgd2FybiB3 aXRoOg0KPg0KPisgICAgICAgdGNkLT5kYWRkciA9IGNwdV90b19sZTMyKGRzdCk7DQo+DQo+VGhl IHJlYXNvbiBpdCdsbCB3YXJuIG9uIHRoYXQgaXMgYmVjYXVzZSBkYWRkciBpcyBub3QgZGVjbGFy ZWQgd2l0aCBfX2xlMzIsDQo+ZXRjIC0gdGhlIHR5cGVzIHVzZWQgaW4gc3RydWN0IGZzbF9lZG1h X2h3X3RjZCB0ZWxsIHNwYXJzZSB0aGF0IHRoZSB2YWx1ZXMNCj50byBiZSBzdG9yZWQgdGhlcmUg YXJlIGluIF9ob3N0XyBlbmRpYW4gZm9ybWF0LCBidXQgdGhleSdyZSBiZWluZyBhc3NpZ25lZA0K PmxpdHRsZSBlbmRpYW4gZm9ybWF0dGVkIGRhdGEuDQo+DQpJIGRpZG4ndCByZWFsaXplIHRoZSB0 eXBlIHdhcm5pbmcsIHRoZXkgaW5kZWVkIGV4aXN0LiBJIHdpbGwgdXNlIF9fbGUzMiBhbmQNCl9f bGUxNiBmb3IgZnNsX2VkbWFfaHdfdGNkIHN0cnVjdCBtZW1iZXJzIGFzIGJlbG93Og0Kc3RydWN0 IGZzbF9lZG1hX2h3X3RjZCB7DQogICAgICAgIF9fbGUzMiAgc2FkZHI7DQogICAgICAgIF9fbGUx NiAgc29mZjsNCiAgICAgICAgX19sZTE2ICBhdHRyOw0KICAgICAgICBfX2xlMzIgIG5ieXRlczsN CiAgICAgICAgX19sZTMyICBzbGFzdDsNCiAgICAgICAgX19sZTMyICBkYWRkcjsNCiAgICAgICAg X19sZTE2ICBkb2ZmOw0KICAgICAgICBfX2xlMTYgIGNpdGVyOw0KICAgICAgICBfX2xlMzIgIGRs YXN0X3NnYTsNCiAgICAgICAgX19sZTE2ICBjc3I7DQogICAgICAgIF9fbGUxNiAgYml0ZXI7DQp9 Ow0KDQo+PiA+ICsJICogZm9yIGZzbF9zZXRfdGNkX3BhcmFtcyBkb2luZyB0aGUgc3dhcC4NCj4+ IGZzbF9zZXRfdGNkX3BhcmFtcygpDQo+DQo+VGhhdCdzIHRoZSB3cm9uZyBmdW5jdGlvbiBuYW1l IGFueXdheS4gIEl0J3MgZnNsX2VkbWFfc2V0X3RjZF9wYXJhbXMoKS4NCj4NCj5Ib3dldmVyLCBs ZXQncyBsb29rIGF0IHRoaXMgYSBsaXR0bGUgbW9yZToNCj4NCj4gICAgICAgIGZzbF9lZG1hX3Nl dF90Y2RfcGFyYW1zKGZzbF9jaGFuLCB0Y2QtPnNhZGRyLCB0Y2QtPmRhZGRyLCB0Y2QtDQo+PmF0 dHIsDQo+ICAgICAgICAgICAgICAgICAgICAgICAgdGNkLT5zb2ZmLCB0Y2QtPm5ieXRlcywgdGNk LT5zbGFzdCwgdGNkLT5jaXRlciwNCj4gICAgICAgICAgICAgICAgICAgICAgICB0Y2QtPmJpdGVy LCB0Y2QtPmRvZmYsIHRjZC0+ZGxhc3Rfc2dhLCB0Y2QtPmNzcik7DQo+DQo+SXMgaXQgL3JlYWxs eS8gYSBnb29kIGlkZWEgdG8gcmVhZCBhbGwgdGhhdCBkYXRhIG91dCBvZiB0aGUgc3RydWN0dXJl LA0KPm9ubHkgdG8gdGhlbiBoYXZlIG1vc3Qgb2YgaXQgc2F2ZWQgaW50byB0aGUgc3RhY2ssIHdo aWNoDQo+ZnNsX2VkbWFfc2V0X3RjZF9wYXJhbXMoKSB0aGVuIGhhcyB0byByZWFkIGJhY2sgb2Zm IHRoZSBzdGFjayBhZ2Fpbj8NCj5XaXRoIHN0dWZmIGxpa2UgdGhpcywgb25lIGhhcyB0byB3b25k ZXIgaWYgdGhlcmUgaXMgbXVjaCBjbHVlIGhvdyB0byB3cml0ZQ0KPm9wdGltYWwgQyBjb2RlIGlu IHRoaXMgZHJpdmVyLg0KPg0KPlRoaXMgc2hvdWxkIGJlIHBhc3NpbmcgdGhlIHRjZCBzdHJ1Y3R1 cmUgaW50byBmc2xfZWRtYV9zZXRfdGNkX3BhcmFtcygpLg0KPg0KPk5vdywgdGhpcyBmdW5jdGlv biBjb250YWlucyB0aGlzIGNvbW1lbnQ6DQo+DQo+ICAgICAgICAvKg0KPiAgICAgICAgICogVENE IHBhcmFtZXRlcnMgaGF2ZSBiZWVuIHN3YXBwZWQgaW4gZmlsbF90Y2RfcGFyYW1zKCksDQo+ICAg ICAgICAgKiBzbyBqdXN0IHdyaXRlIHRoZW0gdG8gcmVnaXN0ZXJzIGluIHRoZSBjcHUgZW5kaWFu IGhlcmUNCj4gICAgICAgICAqLw0KPg0KPldoaWNoIGlzIGFsbW9zdCByZWFzb25hYmxlLCBidXQg bGV0J3MgdXBkYXRlIGl0Og0KPg0KPglUQ0QgcGFyYW1ldGVycyBhcmUgc3RvcmVkIGluIHN0cnVj dCBmc2xfZWRtYV9od190Y2QgaW4gbGl0dGxlDQo+CWVuZGlhbiBmb3JtYXQuICBIb3dldmVyLCB3 ZSBuZWVkIHRvIGxvYWQgdGhlIHJlZ2lzdGVycyBpbg0KPgk8aW5zZXJ0IGFwcHJvcHJpYXRlIGVu ZGlhbm5lc3MgLSBiaWd8bGl0dGxlfGNwdT4gZW5kaWFuLg0KPg0KPk5vdywgcmVtZW1iZXIgdGhh dCB3cml0ZWwoKSBhbmQgZnJpZW5kcyBleHBlY3QgbmF0aXZlIENQVSBlbmRpYW4gZm9ybWF0dGVk DQo+ZGF0YSAoYW5kIHllcywgc3BhcnNlIGNoZWNrcyB0aGlzIHRvbykgc28gdGhhdCBuZWVkcyB0 byBiZSBjb25zaWRlcmVkIG1vcmUuDQo+DQo+TGFzdGx5LCB0aGUgZHJpdmVyIHNlZW1zIHRvIGJl IGEgdG90YWwgbWVzcyBvZiBhY2Nlc3NvcnMuICBJbiBzb21lIHBsYWNlcw0KPml0IHVzZXMgaW97 cmVhZCx3cml0ZX0qLCBpbiBvdGhlciBwbGFjZXMsIGl0IHVzZXMgcGxhaW4ge3JlYWQsd3JpdGV9 Ki4gIEl0DQo+c2hvdWxkIHVzZSBlaXRoZXIgb25lIHNldCBvciB0aGUgb3RoZXIgc2V0LCBhbmQg bm90IG1peCB0aGVzZSB0d28uDQo+DQo+SSBqdXN0IHNwb3R0ZWQgdGhpcyBiYWRseSBmb3JtYXR0 ZWQgY29kZSB0b286DQo+DQo+ICAgICAgICBmb3IgKGkgPSAwOyBpIDwgZnNsX2Rlc2MtPm5fdGNk czsgaSsrKQ0KPiAgICAgICAgICAgICAgICAgICAgICAgIGRtYV9wb29sX2ZyZWUoZnNsX2Rlc2Mt PmVjaGFuLT50Y2RfcG9vbCwNCj4gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgZnNsX2Rlc2MtPnRjZFtpXS52dGNkLA0KPiAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICBmc2xfZGVzYy0+dGNkW2ldLnB0Y2QpOyAuLi4NCj4gICAgICAgICAgICAgICAg ZWRtYV93cml0ZWIoZnNsX2NoYW4tPmVkbWEsDQo+ICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICBFRE1BTVVYX0NIQ0ZHX0VOQkwgfA0KPkVETUFNVVhfQ0hDRkdfU09VUkNFKHNsb3QpLA0K PiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgbXV4YWRkciArIGNoX29mZik7DQo+DQo+ LS0NClRoYW5rcywgSSB3aWxsIHVzZSB0aGUgdGNkIHBvaW50ZXIgaW5zdGVhZC4gQW5kIEkgd2ls bCB1c2Ugb25lIHIvdyBzZXQuDQoNCkJlc3QgUmVnYXJkcywNCkppbmdjaGFuZw0K -- 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/fsl-edma.c b/drivers/dma/fsl-edma.c index 3c5711d..9ca55ee 100644 --- a/drivers/dma/fsl-edma.c +++ b/drivers/dma/fsl-edma.c @@ -177,16 +177,10 @@ struct fsl_edma_engine { /* * R/W functions for big- or little-endian registers * the eDMA controller's endian is independent of the CPU core's endian. + * for the big-endian IP module, the offset for 8-bit or 16-bit register + * should also be swapped oposite to that in little-endian IP. */ -static u16 edma_readw(struct fsl_edma_engine *edma, void __iomem *addr) -{ - if (edma->big_endian) - return ioread16be(addr); - else - return ioread16(addr); -} - static u32 edma_readl(struct fsl_edma_engine *edma, void __iomem *addr) { if (edma->big_endian) @@ -197,13 +191,18 @@ static u32 edma_readl(struct fsl_edma_engine *edma, void __iomem *addr) static void edma_writeb(struct fsl_edma_engine *edma, u8 val, void __iomem *addr) { - iowrite8(val, addr); + /* swap the reg offset for these in big-endian mode */ + if (edma->big_endian) + iowrite8(val, (void __iomem *)((unsigned long)addr ^ 0x3)); + else + iowrite8(val, addr); } static void edma_writew(struct fsl_edma_engine *edma, u16 val, void __iomem *addr) { + /* swap the reg offset for these in big-endian mode */ if (edma->big_endian) - iowrite16be(val, addr); + iowrite16be(val, (void __iomem *)((unsigned long)addr ^ 0x2)); else iowrite16(val, addr); } @@ -256,11 +255,10 @@ static void fsl_edma_chan_mux(struct fsl_edma_chan *fsl_chan, muxaddr = fsl_chan->edma->muxbase[ch / chans_per_mux]; if (enable) - edma_writeb(fsl_chan->edma, - EDMAMUX_CHCFG_ENBL | EDMAMUX_CHCFG_SOURCE(slot), + writeb(EDMAMUX_CHCFG_ENBL | EDMAMUX_CHCFG_SOURCE(slot), muxaddr + ch_off); else - edma_writeb(fsl_chan->edma, EDMAMUX_CHCFG_DIS, muxaddr + ch_off); + writeb(EDMAMUX_CHCFG_DIS, muxaddr + ch_off); } static unsigned int fsl_edma_get_tcd_attr(enum dma_slave_buswidth addr_width) @@ -363,8 +361,8 @@ static size_t fsl_edma_desc_residue(struct fsl_edma_chan *fsl_chan, /* calculate the total size in this desc */ for (len = i = 0; i < fsl_chan->edesc->n_tcds; i++) - len += edma_readl(fsl_chan->edma, &(edesc->tcd[i].vtcd->nbytes)) - * edma_readw(fsl_chan->edma, &(edesc->tcd[i].vtcd->biter)); + len += le32_to_cpu(edesc->tcd[i].vtcd->nbytes) + * le16_to_cpu(edesc->tcd[i].vtcd->biter); if (!in_progress) return len; @@ -376,14 +374,12 @@ static size_t fsl_edma_desc_residue(struct fsl_edma_chan *fsl_chan, /* figure out the finished and calculate the residue */ for (i = 0; i < fsl_chan->edesc->n_tcds; i++) { - size = edma_readl(fsl_chan->edma, &(edesc->tcd[i].vtcd->nbytes)) - * edma_readw(fsl_chan->edma, &(edesc->tcd[i].vtcd->biter)); + size = le32_to_cpu(edesc->tcd[i].vtcd->nbytes) + * le16_to_cpu(edesc->tcd[i].vtcd->biter); if (dir == DMA_MEM_TO_DEV) - dma_addr = edma_readl(fsl_chan->edma, - &(edesc->tcd[i].vtcd->saddr)); + dma_addr = le32_to_cpu(edesc->tcd[i].vtcd->saddr); else - dma_addr = edma_readl(fsl_chan->edma, - &(edesc->tcd[i].vtcd->daddr)); + dma_addr = le32_to_cpu(edesc->tcd[i].vtcd->daddr); len -= size; if (cur_addr > dma_addr && cur_addr < dma_addr + size) { @@ -433,21 +429,26 @@ static void fsl_edma_set_tcd_params(struct fsl_edma_chan *fsl_chan, u32 ch = fsl_chan->vchan.chan.chan_id; /* - * TCD parameters have been swapped in fill_tcd_params(), - * so just write them to registers in the cpu endian here + * TCD parameters should be swapped according the eDMA + * engine requirement. */ - writew(0, addr + EDMA_TCD_CSR(ch)); - writel(src, addr + EDMA_TCD_SADDR(ch)); - writel(dst, addr + EDMA_TCD_DADDR(ch)); - writew(attr, addr + EDMA_TCD_ATTR(ch)); - writew(soff, addr + EDMA_TCD_SOFF(ch)); - writel(nbytes, addr + EDMA_TCD_NBYTES(ch)); - writel(slast, addr + EDMA_TCD_SLAST(ch)); - writew(citer, addr + EDMA_TCD_CITER(ch)); - writew(biter, addr + EDMA_TCD_BITER(ch)); - writew(doff, addr + EDMA_TCD_DOFF(ch)); - writel(dlast_sga, addr + EDMA_TCD_DLAST_SGA(ch)); - writew(csr, addr + EDMA_TCD_CSR(ch)); + edma_writew(fsl_chan->edma, 0, addr + EDMA_TCD_CSR(ch)); + edma_writel(fsl_chan->edma, src, addr + EDMA_TCD_SADDR(ch)); + edma_writel(fsl_chan->edma, dst, addr + EDMA_TCD_DADDR(ch)); + + edma_writew(fsl_chan->edma, attr, addr + EDMA_TCD_ATTR(ch)); + edma_writew(fsl_chan->edma, soff, addr + EDMA_TCD_SOFF(ch)); + + edma_writel(fsl_chan->edma, nbytes, addr + EDMA_TCD_NBYTES(ch)); + edma_writel(fsl_chan->edma, slast, addr + EDMA_TCD_SLAST(ch)); + + edma_writew(fsl_chan->edma, citer, addr + EDMA_TCD_CITER(ch)); + edma_writew(fsl_chan->edma, biter, addr + EDMA_TCD_BITER(ch)); + edma_writew(fsl_chan->edma, doff, addr + EDMA_TCD_DOFF(ch)); + + edma_writel(fsl_chan->edma, dlast_sga, addr + EDMA_TCD_DLAST_SGA(ch)); + + edma_writew(fsl_chan->edma, csr, addr + EDMA_TCD_CSR(ch)); } static void fill_tcd_params(struct fsl_edma_engine *edma, @@ -459,20 +460,27 @@ static void fill_tcd_params(struct fsl_edma_engine *edma, u16 csr = 0; /* - * eDMA hardware SGs require the TCD parameters stored in memory - * the same endian as the eDMA module so that they can be loaded - * automatically by the engine + * eDMA hardware SGs requires the TCDs to be auto loaded + * in the little endian whenver the register endian model, + * So we put the value in little endian in memory, waitting + * for fsl_set_tcd_params doing the swap. */ - edma_writel(edma, src, &(tcd->saddr)); - edma_writel(edma, dst, &(tcd->daddr)); - edma_writew(edma, attr, &(tcd->attr)); - edma_writew(edma, EDMA_TCD_SOFF_SOFF(soff), &(tcd->soff)); - edma_writel(edma, EDMA_TCD_NBYTES_NBYTES(nbytes), &(tcd->nbytes)); - edma_writel(edma, EDMA_TCD_SLAST_SLAST(slast), &(tcd->slast)); - edma_writew(edma, EDMA_TCD_CITER_CITER(citer), &(tcd->citer)); - edma_writew(edma, EDMA_TCD_DOFF_DOFF(doff), &(tcd->doff)); - edma_writel(edma, EDMA_TCD_DLAST_SGA_DLAST_SGA(dlast_sga), &(tcd->dlast_sga)); - edma_writew(edma, EDMA_TCD_BITER_BITER(biter), &(tcd->biter)); + tcd->saddr = cpu_to_le32(src); + tcd->daddr = cpu_to_le32(dst); + + tcd->attr = cpu_to_le16(attr); + + tcd->soff = cpu_to_le16(EDMA_TCD_SOFF_SOFF(soff)); + + tcd->nbytes = cpu_to_le32(EDMA_TCD_NBYTES_NBYTES(nbytes)); + tcd->slast = cpu_to_le32(EDMA_TCD_SLAST_SLAST(slast)); + + tcd->citer = cpu_to_le16(EDMA_TCD_CITER_CITER(citer)); + tcd->doff = cpu_to_le16(EDMA_TCD_DOFF_DOFF(doff)); + + tcd->dlast_sga = cpu_to_le32(EDMA_TCD_DLAST_SGA_DLAST_SGA(dlast_sga)); + + tcd->biter = cpu_to_le16(EDMA_TCD_BITER_BITER(biter)); if (major_int) csr |= EDMA_TCD_CSR_INT_MAJOR; @@ -482,7 +490,7 @@ static void fill_tcd_params(struct fsl_edma_engine *edma, if (enable_sg) csr |= EDMA_TCD_CSR_E_SG; - edma_writew(edma, csr, &(tcd->csr)); + tcd->csr = cpu_to_le16(csr); } static struct fsl_edma_desc *fsl_edma_alloc_desc(struct fsl_edma_chan *fsl_chan,
The offset of all 8-/16-bit register in big-endian eDMA model are swapped in a 32-bit size opposite those in the little-endian model. The hardware Scatter/Gather requires the subsequent TCDs in memory to be auto loaded should retain little endian independent of the register endian model, the dma engine will do the swap if need. This patch also use regular assignment for tcd variables r/w instead of with io function previously that may not always be true. Signed-off-by: Jingchang Lu <jingchang.lu@freescale.com> --- changes in v3: use unsigned long instead of u32 in reg offset swap cast to avoid warning. changes in v2: simplify register offset swap calculation. use regular assignment for tcd variables r/w instead of io function. drivers/dma/fsl-edma.c | 106 ++++++++++++++++++++++++++----------------------- 1 file changed, 57 insertions(+), 49 deletions(-)