| Message ID | 1442232160-2615-2-git-send-email-hamzahfrq.sub@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Hi Hamza, On Mon, Sep 14, 2015 at 2:02 PM, <hamzahfrq.sub@gmail.com> wrote: > DMA engine does not stop instantaneously when transaction is going on > (see datasheet). Wait has been added > > Signed-off-by: Muhammad Hamza Farooq <mfarooq@visteon.com> Thanks for your patch! > diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c > index 7820d07..920f977 100644 > --- a/drivers/dma/sh/rcar-dmac.c > +++ b/drivers/dma/sh/rcar-dmac.c > @@ -716,14 +716,37 @@ static int rcar_dmac_fill_hwdesc(struct rcar_dmac_chan *chan, > /* ----------------------------------------------------------------------------- > * Stop and reset > */ > +#define NR_READS_TO_WAIT (5) /* number of reads to perform to check DE = 0 */ > +static inline int rcar_dmac_wait_stop(struct rcar_dmac_chan *chan) > +{ > + unsigned int i = 0; > + do { > + u32 chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR); > + > + if(!(chcr & RCAR_DMACHCR_DE)) > + return 0; > + dev_dbg(chan->chan.device->dev, "DMA transfer couldn't be stopped"); I would add a cpu_relax() to the loop. > + } > + while(++i < NR_READS_TO_WAIT); BTW, please run scripts/checkpatch.pl on your patches before submission. Thanks! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- 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
SGkgR2VlcnQsDQoNClRoYW5rcyBmb3IgdGhlIGZlZWRiYWNrLg0KDQo+IEkgd291bGQgYWRkIGEg Y3B1X3JlbGF4KCkgdG8gdGhlIGxvb3AuDQpUaGlzIHJvdXRpbmUgaXMgYmVpbmcgY2FsbGVkIGZy b20gc29mdCBJUlEgY29udGV4dCAocnhfdGltZXJfZm4gaW4gY2FzZSBvZiBzaC1zY2kpLiBEb2Vz IGl0IHN0aWxsIG1ha2Ugc2Vuc2UgdG8gYWRkIGNwdV9yZWxheD8NCg0KPiBCVFcsIHBsZWFzZSBy dW4gc2NyaXB0cy9jaGVja3BhdGNoLnBsIG9uIHlvdXIgcGF0Y2hlcyBiZWZvcmUgc3VibWlzc2lv bi4NCkkgZGlkIHJ1biB0aGF0IGFuZCBnb3Qgbm8gd2FybmluZy9lcnJvci4gQW0gSSBtaXNzaW5n IHNvbWV0aGluZz8NCg0KQ2hlZXJzIQ0KSGFtemENCg0KLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0t LS0NCkZyb206IGdlZXJ0LnV5dHRlcmhvZXZlbkBnbWFpbC5jb20gW21haWx0bzpnZWVydC51eXR0 ZXJob2V2ZW5AZ21haWwuY29tXSBPbiBCZWhhbGYgT2YgR2VlcnQgVXl0dGVyaG9ldmVuDQpTZW50 OiBEaWVuc3RhZywgMTUuIFNlcHRlbWJlciAyMDE1IDE3OjExDQpUbzogaGFtemFoZnJxLnN1YkBn bWFpbC5jb20NCkNjOiBMYXVyZW50IFBpbmNoYXJ0IDxsYXVyZW50LnBpbmNoYXJ0K3JlbmVzYXNA aWRlYXNvbmJvYXJkLmNvbT47IGRtYWVuZ2luZUB2Z2VyLmtlcm5lbC5vcmc7IErDvHJnIEJpbGxl dGVyIDxqQGJpdHJvbi5jaD47IEpvZSBQZXJjaGVzIDxqb2VAcGVyY2hlcy5jb20+OyBLdW5pbm9y aSBNb3JpbW90byA8a3VuaW5vcmkubW9yaW1vdG8uZ3hAcmVuZXNhcy5jb20+OyBNYXJrIEJyb3du IDxicm9vbmllQGtlcm5lbC5vcmc+OyBHZWVydCBVeXR0ZXJob2V2ZW4gPGdlZXJ0K3JlbmVzYXNA Z2xpZGVyLmJlPjsgVmlub2QgS291bCA8dmlub2Qua291bEBpbnRlbC5jb20+OyBGYXJvb3EsIE11 aGFtbWFkIChNLkhBTVpBLikgPG1mYXJvb3FAdmlzdGVvbi5jb20+DQpTdWJqZWN0OiBSZTogW1BB VENIIDEvNl0gZG1hOiByY2FyLWRtYTogYWRkIHdhaXQgYWZ0ZXIgc3RvcHBpbmcgZG1hIGVuZ2lu ZQ0KDQpIaSBIYW16YSwNCg0KT24gTW9uLCBTZXAgMTQsIDIwMTUgYXQgMjowMiBQTSwgIDxoYW16 YWhmcnEuc3ViQGdtYWlsLmNvbT4gd3JvdGU6DQo+IERNQSBlbmdpbmUgZG9lcyBub3Qgc3RvcCBp bnN0YW50YW5lb3VzbHkgd2hlbiB0cmFuc2FjdGlvbiBpcyBnb2luZyBvbiANCj4gKHNlZSBkYXRh c2hlZXQpLiBXYWl0IGhhcyBiZWVuIGFkZGVkDQo+DQo+IFNpZ25lZC1vZmYtYnk6IE11aGFtbWFk IEhhbXphIEZhcm9vcSA8bWZhcm9vcUB2aXN0ZW9uLmNvbT4NCg0KVGhhbmtzIGZvciB5b3VyIHBh dGNoIQ0KDQo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2RtYS9zaC9yY2FyLWRtYWMuYyBiL2RyaXZl cnMvZG1hL3NoL3JjYXItZG1hYy5jIA0KPiBpbmRleCA3ODIwZDA3Li45MjBmOTc3IDEwMDY0NA0K PiAtLS0gYS9kcml2ZXJzL2RtYS9zaC9yY2FyLWRtYWMuYw0KPiArKysgYi9kcml2ZXJzL2RtYS9z aC9yY2FyLWRtYWMuYw0KPiBAQCAtNzE2LDE0ICs3MTYsMzcgQEAgc3RhdGljIGludCByY2FyX2Rt YWNfZmlsbF9od2Rlc2Moc3RydWN0IA0KPiByY2FyX2RtYWNfY2hhbiAqY2hhbiwNCj4gIC8qIC0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tDQo+ICAgKiBTdG9wIGFuZCByZXNldA0KPiAgICovDQo+ICsjZGVm aW5lICAgICAgICBOUl9SRUFEU19UT19XQUlUICAgICAgICAoNSkgICAgIC8qIG51bWJlciBvZiBy ZWFkcyB0byBwZXJmb3JtIHRvIGNoZWNrIERFID0gMCAqLw0KPiArc3RhdGljIGlubGluZSBpbnQg cmNhcl9kbWFjX3dhaXRfc3RvcChzdHJ1Y3QgcmNhcl9kbWFjX2NoYW4gKmNoYW4pIHsNCj4gKyAg ICAgICB1bnNpZ25lZCBpbnQgaSA9IDA7DQo+ICsgICAgICAgZG8gew0KPiArICAgICAgICAgICAg ICAgdTMyIGNoY3IgPSByY2FyX2RtYWNfY2hhbl9yZWFkKGNoYW4sIFJDQVJfRE1BQ0hDUik7DQo+ ICsNCj4gKyAgICAgICAgICAgICAgIGlmKCEoY2hjciAmIFJDQVJfRE1BQ0hDUl9ERSkpDQo+ICsg ICAgICAgICAgICAgICAgICAgICAgIHJldHVybiAwOw0KPiArICAgICAgICAgICAgICAgZGV2X2Ri ZyhjaGFuLT5jaGFuLmRldmljZS0+ZGV2LCAiRE1BIHRyYW5zZmVyIGNvdWxkbid0IA0KPiArIGJl IHN0b3BwZWQiKTsNCg0KSSB3b3VsZCBhZGQgYSBjcHVfcmVsYXgoKSB0byB0aGUgbG9vcC4NCg0K PiArICAgICAgIH0NCj4gKyAgICAgICB3aGlsZSgrK2kgPCBOUl9SRUFEU19UT19XQUlUKTsNCg0K QlRXLCBwbGVhc2UgcnVuIHNjcmlwdHMvY2hlY2twYXRjaC5wbCBvbiB5b3VyIHBhdGNoZXMgYmVm b3JlIHN1Ym1pc3Npb24uDQoNClRoYW5rcyENCg0KR3J7b2V0amUsZWV0aW5nfXMsDQoNCiAgICAg ICAgICAgICAgICAgICAgICAgIEdlZXJ0DQoNCi0tDQpHZWVydCBVeXR0ZXJob2V2ZW4gLS0gVGhl cmUncyBsb3RzIG9mIExpbnV4IGJleW9uZCBpYTMyIC0tIGdlZXJ0QGxpbnV4LW02OGsub3JnDQoN CkluIHBlcnNvbmFsIGNvbnZlcnNhdGlvbnMgd2l0aCB0ZWNobmljYWwgcGVvcGxlLCBJIGNhbGwg bXlzZWxmIGEgaGFja2VyLiBCdXQgd2hlbiBJJ20gdGFsa2luZyB0byBqb3VybmFsaXN0cyBJIGp1 c3Qgc2F5ICJwcm9ncmFtbWVyIiBvciBzb21ldGhpbmcgbGlrZSB0aGF0Lg0KICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAtLSBMaW51cyBUb3J2YWxkcw0K -- 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
Hi Hamza, On Tue, Sep 15, 2015 at 5:44 PM, Farooq, Muhammad (M.HAMZA.) <mfarooq@visteon.com> wrote: >> I would add a cpu_relax() to the loop. > This routine is being called from soft IRQ context (rx_timer_fn in case of sh-sci). Does it still make sense to add cpu_relax? Yes, it will add an explicit barrier. Possibly the implicit barrier in readw() in rcar_dmac_chan_read() is already good enough, but it's better to make this explicit. >> BTW, please run scripts/checkpatch.pl on your patches before submission. > I did run that and got no warning/error. Am I missing something? Strange. I get several warnings, e.g. for a missing space after the "if" in "if(!(chcr & RCAR_DMACHCR_DE))". Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- 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
Hi Geert, On Tue, Sep 15, 2015 at 6:03 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Tue, Sep 15, 2015 at 5:44 PM, Farooq, Muhammad (M.HAMZA.) > <mfarooq@visteon.com> wrote: >>> I would add a cpu_relax() to the loop. >> This routine is being called from soft IRQ context (rx_timer_fn in case of sh-sci). Does it still make sense to add cpu_relax? > > Yes, it will add an explicit barrier. > Possibly the implicit barrier in readw() in rcar_dmac_chan_read() is already > good enough, but it's better to make this explicit. > >>> BTW, please run scripts/checkpatch.pl on your patches before submission. >> I did run that and got no warning/error. Am I missing something? > > Strange. I get several warnings, e.g. for a missing space after the "if" in > "if(!(chcr & RCAR_DMACHCR_DE))". I used that with -f flag which somehow misses many warnings. I will fix these issues and send the patch series again Thanks for your input! Best, Hamza -- 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/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c index 7820d07..920f977 100644 --- a/drivers/dma/sh/rcar-dmac.c +++ b/drivers/dma/sh/rcar-dmac.c @@ -716,14 +716,37 @@ static int rcar_dmac_fill_hwdesc(struct rcar_dmac_chan *chan, /* ----------------------------------------------------------------------------- * Stop and reset */ +#define NR_READS_TO_WAIT (5) /* number of reads to perform to check DE = 0 */ +static inline int rcar_dmac_wait_stop(struct rcar_dmac_chan *chan) +{ + unsigned int i = 0; + do { + u32 chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR); + + if(!(chcr & RCAR_DMACHCR_DE)) + return 0; + dev_dbg(chan->chan.device->dev, "DMA transfer couldn't be stopped"); + } + while(++i < NR_READS_TO_WAIT); -static void rcar_dmac_chan_halt(struct rcar_dmac_chan *chan) + return -EBUSY; +} + +/* Called with chan lock held */ +static int rcar_dmac_chan_halt(struct rcar_dmac_chan *chan) { - u32 chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR); + u32 chcr; + int ret; + chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR); chcr &= ~(RCAR_DMACHCR_DSE | RCAR_DMACHCR_DSIE | RCAR_DMACHCR_IE | - RCAR_DMACHCR_TE | RCAR_DMACHCR_DE); + RCAR_DMACHCR_TE | RCAR_DMACHCR_DE); rcar_dmac_chan_write(chan, RCAR_DMACHCR, chcr); + ret = rcar_dmac_wait_stop(chan); + + WARN_ON(ret < 0); + + return ret; } static void rcar_dmac_chan_reinit(struct rcar_dmac_chan *chan)