| Message ID | 20220721144708.880293-1-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] dmaengine: sh: rz-dmac: Add device_synchronize callback | expand |
Hi Biju, On Thu, Jul 21, 2022 at 4:49 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > Some on-chip peripheral modules(for eg:- rspi) on RZ/G2L SoC > use the same signal for both interrupt and DMA transfer requests. > The signal works as a DMA transfer request signal by setting > DMARS, and subsequent interrupt requests to the interrupt controller > are masked. > > We can re-enable the interrupt by clearing the DMARS. > > This patch adds device_synchronize callback for clearing > DMARS and thereby allowing DMA consumers to switch to > interrupt mode. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > v2->v3: > * Fixed commit description > * Added check if the DMA operation has been completed or terminated, > and wait (sleep) if needed. Thanks for the uodate! > --- a/drivers/dma/sh/rz-dmac.c > +++ b/drivers/dma/sh/rz-dmac.c > @@ -12,6 +12,7 @@ > #include <linux/dma-mapping.h> > #include <linux/dmaengine.h> > #include <linux/interrupt.h> > +#include <linux/iopoll.h> > #include <linux/list.h> > #include <linux/module.h> > #include <linux/of.h> > @@ -630,6 +631,21 @@ static void rz_dmac_virt_desc_free(struct virt_dma_desc *vd) > */ > } > > +static void rz_dmac_device_synchronize(struct dma_chan *chan) > +{ > + struct rz_dmac_chan *channel = to_rz_dmac_chan(chan); > + struct rz_dmac *dmac = to_rz_dmac(chan->device); > + u32 chstat; > + int ret; > + > + ret = read_poll_timeout(rz_dmac_ch_readl, chstat, !(chstat & CHSTAT_EN), > + 10, 1000, false, channel, CHSTAT, 1); Isn't 1000 µs = 1 ms a bit short? IIUIC, I can submit a DMA operation for transfering a 64 KiB (or larger) block, and call dmaengine_synchronize() immediately after that? > + if (ret < 0) > + dev_warn(dmac->dev, "DMA Timeout"); > + > + rz_dmac_set_dmars_register(dmac, channel->index, 0); > +} > + > /* > * ----------------------------------------------------------------------------- > * IRQ handling 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
Hi Geert, > Subject: Re: [PATCH v3] dmaengine: sh: rz-dmac: Add device_synchronize > callback > > Hi Biju, > > On Thu, Jul 21, 2022 at 4:49 PM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > Some on-chip peripheral modules(for eg:- rspi) on RZ/G2L SoC use the > > same signal for both interrupt and DMA transfer requests. > > The signal works as a DMA transfer request signal by setting DMARS, > > and subsequent interrupt requests to the interrupt controller are > > masked. > > > > We can re-enable the interrupt by clearing the DMARS. > > > > This patch adds device_synchronize callback for clearing DMARS and > > thereby allowing DMA consumers to switch to interrupt mode. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > v2->v3: > > * Fixed commit description > > * Added check if the DMA operation has been completed or terminated, > > and wait (sleep) if needed. > > Thanks for the uodate! > > > --- a/drivers/dma/sh/rz-dmac.c > > +++ b/drivers/dma/sh/rz-dmac.c > > @@ -12,6 +12,7 @@ > > #include <linux/dma-mapping.h> > > #include <linux/dmaengine.h> > > #include <linux/interrupt.h> > > +#include <linux/iopoll.h> > > #include <linux/list.h> > > #include <linux/module.h> > > #include <linux/of.h> > > @@ -630,6 +631,21 @@ static void rz_dmac_virt_desc_free(struct > virt_dma_desc *vd) > > */ > > } > > > > +static void rz_dmac_device_synchronize(struct dma_chan *chan) { > > + struct rz_dmac_chan *channel = to_rz_dmac_chan(chan); > > + struct rz_dmac *dmac = to_rz_dmac(chan->device); > > + u32 chstat; > > + int ret; > > + > > + ret = read_poll_timeout(rz_dmac_ch_readl, chstat, !(chstat & > CHSTAT_EN), > > + 10, 1000, false, channel, CHSTAT, 1); > > Isn't 1000 µs = 1 ms a bit short? > IIUIC, I can submit a DMA operation for transfering a 64 KiB (or larger) > block, and call dmaengine_synchronize() immediately after that? Will increase to 100 msec?? is it ok? Cheers, biju > > > + if (ret < 0) > > + dev_warn(dmac->dev, "DMA Timeout"); > > + > > + rz_dmac_set_dmars_register(dmac, channel->index, 0); } > > + > > /* > > * ------------------------------------------------------------------ > ----------- > > * IRQ handling > > 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
Hi Biju, On Thu, Jul 21, 2022 at 6:06 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > Subject: Re: [PATCH v3] dmaengine: sh: rz-dmac: Add device_synchronize > > callback > > > > On Thu, Jul 21, 2022 at 4:49 PM Biju Das <biju.das.jz@bp.renesas.com> > > wrote: > > > Some on-chip peripheral modules(for eg:- rspi) on RZ/G2L SoC use the > > > same signal for both interrupt and DMA transfer requests. > > > The signal works as a DMA transfer request signal by setting DMARS, > > > and subsequent interrupt requests to the interrupt controller are > > > masked. > > > > > > We can re-enable the interrupt by clearing the DMARS. > > > > > > This patch adds device_synchronize callback for clearing DMARS and > > > thereby allowing DMA consumers to switch to interrupt mode. > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > --- > > > v2->v3: > > > * Fixed commit description > > > * Added check if the DMA operation has been completed or terminated, > > > and wait (sleep) if needed. > > > > Thanks for the uodate! > > > > > --- a/drivers/dma/sh/rz-dmac.c > > > +++ b/drivers/dma/sh/rz-dmac.c > > > @@ -12,6 +12,7 @@ > > > #include <linux/dma-mapping.h> > > > #include <linux/dmaengine.h> > > > #include <linux/interrupt.h> > > > +#include <linux/iopoll.h> > > > #include <linux/list.h> > > > #include <linux/module.h> > > > #include <linux/of.h> > > > @@ -630,6 +631,21 @@ static void rz_dmac_virt_desc_free(struct > > virt_dma_desc *vd) > > > */ > > > } > > > > > > +static void rz_dmac_device_synchronize(struct dma_chan *chan) { > > > + struct rz_dmac_chan *channel = to_rz_dmac_chan(chan); > > > + struct rz_dmac *dmac = to_rz_dmac(chan->device); > > > + u32 chstat; > > > + int ret; > > > + > > > + ret = read_poll_timeout(rz_dmac_ch_readl, chstat, !(chstat & > > CHSTAT_EN), > > > + 10, 1000, false, channel, CHSTAT, 1); > > > > Isn't 1000 µs = 1 ms a bit short? > > IIUIC, I can submit a DMA operation for transfering a 64 KiB (or larger) > > block, and call dmaengine_synchronize() immediately after that? > > Will increase to 100 msec?? is it ok? Probably. As this is a sleeping wait, it doesn't hurt to be conservative. Do you know what's the maximum transfer size/maximum time a DMA transfer could take? 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
Hi Geert, > Subject: Re: [PATCH v3] dmaengine: sh: rz-dmac: Add device_synchronize > callback > > Hi Biju, > > On Thu, Jul 21, 2022 at 6:06 PM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > > Subject: Re: [PATCH v3] dmaengine: sh: rz-dmac: Add > > > device_synchronize callback > > > > > > On Thu, Jul 21, 2022 at 4:49 PM Biju Das > > > <biju.das.jz@bp.renesas.com> > > > wrote: > > > > Some on-chip peripheral modules(for eg:- rspi) on RZ/G2L SoC use > > > > the same signal for both interrupt and DMA transfer requests. > > > > The signal works as a DMA transfer request signal by setting > > > > DMARS, and subsequent interrupt requests to the interrupt > > > > controller are masked. > > > > > > > > We can re-enable the interrupt by clearing the DMARS. > > > > > > > > This patch adds device_synchronize callback for clearing DMARS and > > > > thereby allowing DMA consumers to switch to interrupt mode. > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > --- > > > > v2->v3: > > > > * Fixed commit description > > > > * Added check if the DMA operation has been completed or > terminated, > > > > and wait (sleep) if needed. > > > > > > Thanks for the uodate! > > > > > > > --- a/drivers/dma/sh/rz-dmac.c > > > > +++ b/drivers/dma/sh/rz-dmac.c > > > > @@ -12,6 +12,7 @@ > > > > #include <linux/dma-mapping.h> > > > > #include <linux/dmaengine.h> > > > > #include <linux/interrupt.h> > > > > +#include <linux/iopoll.h> > > > > #include <linux/list.h> > > > > #include <linux/module.h> > > > > #include <linux/of.h> > > > > @@ -630,6 +631,21 @@ static void rz_dmac_virt_desc_free(struct > > > virt_dma_desc *vd) > > > > */ > > > > } > > > > > > > > +static void rz_dmac_device_synchronize(struct dma_chan *chan) { > > > > + struct rz_dmac_chan *channel = to_rz_dmac_chan(chan); > > > > + struct rz_dmac *dmac = to_rz_dmac(chan->device); > > > > + u32 chstat; > > > > + int ret; > > > > + > > > > + ret = read_poll_timeout(rz_dmac_ch_readl, chstat, !(chstat > > > > + & > > > CHSTAT_EN), > > > > + 10, 1000, false, channel, CHSTAT, > > > > + 1); > > > > > > Isn't 1000 µs = 1 ms a bit short? > > > IIUIC, I can submit a DMA operation for transfering a 64 KiB (or > > > larger) block, and call dmaengine_synchronize() immediately after > that? > > > > Will increase to 100 msec?? is it ok? > > Probably. As this is a sleeping wait, it doesn't hurt to be > conservative. > Do you know what's the maximum transfer size/maximum time a DMA transfer > could take? > RSPI interface connected to PMOD Flash, The rd/wr test showing 10K transfer size->21 msec. Cheers, Biju
Hi Geert, > Subject: RE: [PATCH v3] dmaengine: sh: rz-dmac: Add device_synchronize > callback > > Hi Geert, > > > Subject: Re: [PATCH v3] dmaengine: sh: rz-dmac: Add device_synchronize > > callback > > > > Hi Biju, > > > > On Thu, Jul 21, 2022 at 6:06 PM Biju Das <biju.das.jz@bp.renesas.com> > > wrote: > > > > Subject: Re: [PATCH v3] dmaengine: sh: rz-dmac: Add > > > > device_synchronize callback > > > > > > > > On Thu, Jul 21, 2022 at 4:49 PM Biju Das > > > > <biju.das.jz@bp.renesas.com> > > > > wrote: > > > > > Some on-chip peripheral modules(for eg:- rspi) on RZ/G2L SoC use > > > > > the same signal for both interrupt and DMA transfer requests. > > > > > The signal works as a DMA transfer request signal by setting > > > > > DMARS, and subsequent interrupt requests to the interrupt > > > > > controller are masked. > > > > > > > > > > We can re-enable the interrupt by clearing the DMARS. > > > > > > > > > > This patch adds device_synchronize callback for clearing DMARS > > > > > and thereby allowing DMA consumers to switch to interrupt mode. > > > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > > --- > > > > > v2->v3: > > > > > * Fixed commit description > > > > > * Added check if the DMA operation has been completed or > > terminated, > > > > > and wait (sleep) if needed. > > > > > > > > Thanks for the uodate! > > > > > > > > > --- a/drivers/dma/sh/rz-dmac.c > > > > > +++ b/drivers/dma/sh/rz-dmac.c > > > > > @@ -12,6 +12,7 @@ > > > > > #include <linux/dma-mapping.h> > > > > > #include <linux/dmaengine.h> > > > > > #include <linux/interrupt.h> > > > > > +#include <linux/iopoll.h> > > > > > #include <linux/list.h> > > > > > #include <linux/module.h> > > > > > #include <linux/of.h> > > > > > @@ -630,6 +631,21 @@ static void rz_dmac_virt_desc_free(struct > > > > virt_dma_desc *vd) > > > > > */ > > > > > } > > > > > > > > > > +static void rz_dmac_device_synchronize(struct dma_chan *chan) { > > > > > + struct rz_dmac_chan *channel = to_rz_dmac_chan(chan); > > > > > + struct rz_dmac *dmac = to_rz_dmac(chan->device); > > > > > + u32 chstat; > > > > > + int ret; > > > > > + > > > > > + ret = read_poll_timeout(rz_dmac_ch_readl, chstat, > > > > > + !(chstat & > > > > CHSTAT_EN), > > > > > + 10, 1000, false, channel, > > > > > + CHSTAT, 1); > > > > > > > > Isn't 1000 µs = 1 ms a bit short? > > > > IIUIC, I can submit a DMA operation for transfering a 64 KiB (or > > > > larger) block, and call dmaengine_synchronize() immediately after > > that? > > > > > > Will increase to 100 msec?? is it ok? > > > > Probably. As this is a sleeping wait, it doesn't hurt to be > > conservative. > > Do you know what's the maximum transfer size/maximum time a DMA > > transfer could take? > > > > RSPI interface connected to PMOD Flash, The rd/wr test showing 10K > transfer size->21 msec. Actually, it is 11 msec. Earlier result consists of prints during start. I will send v4 with 100 msec. [ 74.085348] [rspi_dma_complete] 544 end: 74080 ms duration: 3 ms 256 [ 74.097318] [rspi_dma_complete] 544 end: 74092 ms duration: 3 ms 256 [ 74.105547] [rspi_dma_complete] 544 end: 74100 ms duration: 0 ms 256 [ 74.114266] [rspi_dma_complete] 544 end: 74109 ms duration: 0 ms 256 [ 74.122506] [rspi_dma_complete] 544 end: 74117 ms duration: 0 ms 256 [ 74.141597] [rspi_dma_complete] 544 end: 74136 ms duration: 11 ms 10240 [ 74.160630] [rspi_dma_complete] 544 end: 74155 ms duration: 11 ms 10240 [ 74.179576] [rspi_dma_complete] 544 end: 74174 ms duration: 11 ms 10240 [ 74.198477] [rspi_dma_complete] 544 end: 74193 ms duration: 11 ms 10240 [ 74.217461] [rspi_dma_complete] 544 end: 74212 ms duration: 11 ms 10240 [ 74.236287] [rspi_dma_complete] 544 end: 74231 ms duration: 11 ms 10240 [ 74.255089] [rspi_dma_complete] 544 end: 74249 ms duration: 11 ms 10240 [ 74.273895] [rspi_dma_complete] 544 end: 74268 ms duration: 11 ms 10240 [ 74.292808] [rspi_dma_complete] 544 end: 74287 ms duration: 11 ms 10240 [ 74.311620] [rspi_dma_complete] 544 end: 74306 ms duration: 11 ms 10240 [ 74.330427] [rspi_dma_complete] 544 end: 74325 ms duration: 11 ms 10240 [ 74.349364] [rspi_dma_complete] 544 end: 74344 ms duration: 11 ms 10240 [ 74.368167] [rspi_dma_complete] 544 end: 74363 ms duration: 11 ms 10240 [ 74.386979] [rspi_dma_complete] 544 end: 74381 ms duration: 11 ms 10240 [ 74.405786] [rspi_dma_complete] 544 end: 74400 ms duration: 11 ms 10240 [ 74.424707] [rspi_dma_complete] 544 end: 74419 ms duration: 11 ms 10240 [ 74.443515] [rspi_dma_complete] 544 end: 74438 ms duration: 11 ms 10240 [ 74.462325] [rspi_dma_complete] 544 end: 74457 ms duration: 11 ms 10240 [ 74.481214] [rspi_dma_complete] 544 end: 74476 ms duration: 11 ms 10240 [ 74.500016] [rspi_dma_complete] 544 end: 74494 ms duration: 11 ms 10240 [ 74.518821] [rspi_dma_complete] 544 end: 74513 ms duration: 11 ms 10240 [ 74.537626] [rspi_dma_complete] 544 end: 74532 ms duration: 11 ms 10240 [ 74.556569] [rspi_dma_complete] 544 end: 74551 ms duration: 11 ms 10240 [ 74.575378] [rspi_dma_complete] 544 end: 74570 ms duration: 11 ms 10240 [ 74.594186] [rspi_dma_complete] 544 end: 74589 ms duration: 11 ms 10240 [ 74.613034] [rspi_dma_complete] 544 end: 74607 ms duration: 11 ms 10240 [ 74.631848] [rspi_dma_complete] 544 end: 74626 ms duration: 11 ms 10240 [ 74.650657] [rspi_dma_complete] 544 end: 74645 ms duration: 11 ms 10240 [ 74.669482] [rspi_dma_complete] 544 end: 74664 ms duration: 11 ms 10240 [ 74.688365] [rspi_dma_complete] 544 end: 74683 ms duration: 11 ms 10240 [ 74.707169] [rspi_dma_complete] 544 end: 74702 ms duration: 11 ms 10240 [ 74.725972] [rspi_dma_complete] 544 end: 74720 ms duration: 11 ms 10240 [ 74.953116] [rspi_dma_complete] 544 end: 74947 ms duration: 0 ms 12 [ 74.982539] [rspi_dma_complete] 544 end: 74977 ms duration: 0 ms 12 Cheers, Biju
diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c index ee2872e7d64c..799a6e670d60 100644 --- a/drivers/dma/sh/rz-dmac.c +++ b/drivers/dma/sh/rz-dmac.c @@ -12,6 +12,7 @@ #include <linux/dma-mapping.h> #include <linux/dmaengine.h> #include <linux/interrupt.h> +#include <linux/iopoll.h> #include <linux/list.h> #include <linux/module.h> #include <linux/of.h> @@ -630,6 +631,21 @@ static void rz_dmac_virt_desc_free(struct virt_dma_desc *vd) */ } +static void rz_dmac_device_synchronize(struct dma_chan *chan) +{ + struct rz_dmac_chan *channel = to_rz_dmac_chan(chan); + struct rz_dmac *dmac = to_rz_dmac(chan->device); + u32 chstat; + int ret; + + ret = read_poll_timeout(rz_dmac_ch_readl, chstat, !(chstat & CHSTAT_EN), + 10, 1000, false, channel, CHSTAT, 1); + if (ret < 0) + dev_warn(dmac->dev, "DMA Timeout"); + + rz_dmac_set_dmars_register(dmac, channel->index, 0); +} + /* * ----------------------------------------------------------------------------- * IRQ handling @@ -909,6 +925,7 @@ static int rz_dmac_probe(struct platform_device *pdev) engine->device_config = rz_dmac_config; engine->device_terminate_all = rz_dmac_terminate_all; engine->device_issue_pending = rz_dmac_issue_pending; + engine->device_synchronize = rz_dmac_device_synchronize; engine->copy_align = DMAENGINE_ALIGN_1_BYTE; dma_set_max_seg_size(engine->dev, U32_MAX);
Some on-chip peripheral modules(for eg:- rspi) on RZ/G2L SoC use the same signal for both interrupt and DMA transfer requests. The signal works as a DMA transfer request signal by setting DMARS, and subsequent interrupt requests to the interrupt controller are masked. We can re-enable the interrupt by clearing the DMARS. This patch adds device_synchronize callback for clearing DMARS and thereby allowing DMA consumers to switch to interrupt mode. Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- v2->v3: * Fixed commit description * Added check if the DMA operation has been completed or terminated, and wait (sleep) if needed. v1->v2: * No change --- drivers/dma/sh/rz-dmac.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)