* [PATCH v2 0/2] RZ/G2L DMA fix/improvements @ 2023-06-30 16:17 Biju Das 2023-06-30 16:17 ` [PATCH v2 1/2] dmaengine: sh: rz-dmac: Improve cleanup order in probe()/remove() Biju Das 2023-06-30 16:17 ` [PATCH v2 2/2] dma: rz-dmac: Fix destination and source data size setting Biju Das 0 siblings, 2 replies; 5+ messages in thread From: Biju Das @ 2023-06-30 16:17 UTC (permalink / raw) To: Vinod Koul, Philipp Zabel Cc: Biju Das, Geert Uytterhoeven, Hien Huynh, Lad Prabhakar, dmaengine, linux-renesas-soc This patch series aims to fix/improve RZ/DMAC driver. The improvement patch is related to fix cleanup order in probe/remove(). and fixes patch is related to wrong SDS/DDS settings, when we change/update the DMA bus width several times. v1->v2: * Update patch header and description. Biju Das (1): dmaengine: sh: rz-dmac: Improve cleanup order in probe()/remove() Hien Huynh (1): dma: rz-dmac: Fix destination and source data size setting drivers/dma/sh/rz-dmac.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] dmaengine: sh: rz-dmac: Improve cleanup order in probe()/remove() 2023-06-30 16:17 [PATCH v2 0/2] RZ/G2L DMA fix/improvements Biju Das @ 2023-06-30 16:17 ` Biju Das 2023-06-30 16:17 ` [PATCH v2 2/2] dma: rz-dmac: Fix destination and source data size setting Biju Das 1 sibling, 0 replies; 5+ messages in thread From: Biju Das @ 2023-06-30 16:17 UTC (permalink / raw) To: Vinod Koul, Philipp Zabel Cc: Biju Das, Geert Uytterhoeven, Hien Huynh, Lad Prabhakar, dmaengine, linux-renesas-soc, Pavel Machek We usually do cleanup in reverse order of init. Currently, in the case of error, this is not followed in rz_dmac_probe(), and similar case for remove(). This patch improves error handling in probe() and cleanup in reverse order of init in the remove(). Reported-by: Pavel Machek <pavel@denx.de> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- v1->v2: * Fixed typo in patch header * Updated patch description. --- drivers/dma/sh/rz-dmac.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c index 9479f29692d3..229f642fde6b 100644 --- a/drivers/dma/sh/rz-dmac.c +++ b/drivers/dma/sh/rz-dmac.c @@ -947,7 +947,6 @@ static int rz_dmac_probe(struct platform_device *pdev) dma_register_err: of_dma_controller_free(pdev->dev.of_node); err: - reset_control_assert(dmac->rstc); channel_num = i ? i - 1 : 0; for (i = 0; i < channel_num; i++) { struct rz_dmac_chan *channel = &dmac->channels[i]; @@ -958,6 +957,7 @@ static int rz_dmac_probe(struct platform_device *pdev) channel->lmdesc.base_dma); } + reset_control_assert(dmac->rstc); err_pm_runtime_put: pm_runtime_put(&pdev->dev); err_pm_disable: @@ -971,6 +971,8 @@ static int rz_dmac_remove(struct platform_device *pdev) struct rz_dmac *dmac = platform_get_drvdata(pdev); unsigned int i; + dma_async_device_unregister(&dmac->engine); + of_dma_controller_free(pdev->dev.of_node); for (i = 0; i < dmac->n_channels; i++) { struct rz_dmac_chan *channel = &dmac->channels[i]; @@ -979,8 +981,6 @@ static int rz_dmac_remove(struct platform_device *pdev) channel->lmdesc.base, channel->lmdesc.base_dma); } - of_dma_controller_free(pdev->dev.of_node); - dma_async_device_unregister(&dmac->engine); reset_control_assert(dmac->rstc); pm_runtime_put(&pdev->dev); pm_runtime_disable(&pdev->dev); -- 2.25.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] dma: rz-dmac: Fix destination and source data size setting 2023-06-30 16:17 [PATCH v2 0/2] RZ/G2L DMA fix/improvements Biju Das 2023-06-30 16:17 ` [PATCH v2 1/2] dmaengine: sh: rz-dmac: Improve cleanup order in probe()/remove() Biju Das @ 2023-06-30 16:17 ` Biju Das 2023-07-06 6:14 ` Vinod Koul 1 sibling, 1 reply; 5+ messages in thread From: Biju Das @ 2023-06-30 16:17 UTC (permalink / raw) To: Vinod Koul Cc: Hien Huynh, Biju Das, Geert Uytterhoeven, Lad Prabhakar, dmaengine, linux-renesas-soc From: Hien Huynh <hien.huynh.px@renesas.com> Before setting DDS and SDS values, we need to clear its value first otherwise, we get incorrect results when we change/update the DMA bus width several times due to the 'OR' expression. Fixes: 5000d37042a6 ("dmaengine: sh: Add DMAC driver for RZ/G2L SoC") Signed-off-by: Hien Huynh <hien.huynh.px@renesas.com> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- v1->v2: * Updated patch header. --- drivers/dma/sh/rz-dmac.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c index 229f642fde6b..331ea80f21b0 100644 --- a/drivers/dma/sh/rz-dmac.c +++ b/drivers/dma/sh/rz-dmac.c @@ -145,8 +145,10 @@ struct rz_dmac { #define CHCFG_REQD BIT(3) #define CHCFG_SEL(bits) ((bits) & 0x07) #define CHCFG_MEM_COPY (0x80400008) -#define CHCFG_FILL_DDS(a) (((a) << 16) & GENMASK(19, 16)) -#define CHCFG_FILL_SDS(a) (((a) << 12) & GENMASK(15, 12)) +#define CHCFG_FILL_DDS_MASK GENMASK(19, 16) +#define CHCFG_FILL_DDS(a) (((a) << 16) & CHCFG_FILL_DDS_MASK) +#define CHCFG_FILL_SDS_MASK GENMASK(15, 12) +#define CHCFG_FILL_SDS(a) (((a) << 12) & CHCFG_FILL_SDS_MASK) #define CHCFG_FILL_TM(a) (((a) & BIT(5)) << 22) #define CHCFG_FILL_AM(a) (((a) & GENMASK(4, 2)) << 6) #define CHCFG_FILL_LVL(a) (((a) & BIT(1)) << 5) @@ -607,12 +609,14 @@ static int rz_dmac_config(struct dma_chan *chan, if (val == CHCFG_DS_INVALID) return -EINVAL; + channel->chcfg &= ~CHCFG_FILL_DDS_MASK; channel->chcfg |= CHCFG_FILL_DDS(val); val = rz_dmac_ds_to_val_mapping(config->src_addr_width); if (val == CHCFG_DS_INVALID) return -EINVAL; + channel->chcfg &= ~CHCFG_FILL_SDS_MASK; channel->chcfg |= CHCFG_FILL_SDS(val); return 0; -- 2.25.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] dma: rz-dmac: Fix destination and source data size setting 2023-06-30 16:17 ` [PATCH v2 2/2] dma: rz-dmac: Fix destination and source data size setting Biju Das @ 2023-07-06 6:14 ` Vinod Koul 2023-07-06 7:06 ` Biju Das 0 siblings, 1 reply; 5+ messages in thread From: Vinod Koul @ 2023-07-06 6:14 UTC (permalink / raw) To: Biju Das Cc: Hien Huynh, Geert Uytterhoeven, Lad Prabhakar, dmaengine, linux-renesas-soc On 30-06-23, 17:17, Biju Das wrote: > From: Hien Huynh <hien.huynh.px@renesas.com> patch title should be dmaengine: xxx > > Before setting DDS and SDS values, we need to clear its value first > otherwise, we get incorrect results when we change/update the DMA bus > width several times due to the 'OR' expression. > > Fixes: 5000d37042a6 ("dmaengine: sh: Add DMAC driver for RZ/G2L SoC") > Signed-off-by: Hien Huynh <hien.huynh.px@renesas.com> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > v1->v2: > * Updated patch header. > --- > drivers/dma/sh/rz-dmac.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c > index 229f642fde6b..331ea80f21b0 100644 > --- a/drivers/dma/sh/rz-dmac.c > +++ b/drivers/dma/sh/rz-dmac.c > @@ -145,8 +145,10 @@ struct rz_dmac { > #define CHCFG_REQD BIT(3) > #define CHCFG_SEL(bits) ((bits) & 0x07) > #define CHCFG_MEM_COPY (0x80400008) > -#define CHCFG_FILL_DDS(a) (((a) << 16) & GENMASK(19, 16)) > -#define CHCFG_FILL_SDS(a) (((a) << 12) & GENMASK(15, 12)) > +#define CHCFG_FILL_DDS_MASK GENMASK(19, 16) > +#define CHCFG_FILL_DDS(a) (((a) << 16) & CHCFG_FILL_DDS_MASK) > +#define CHCFG_FILL_SDS_MASK GENMASK(15, 12) > +#define CHCFG_FILL_SDS(a) (((a) << 12) & CHCFG_FILL_SDS_MASK) Suggestion: Consider using FIELD_PREP and FIELD_GET for this > #define CHCFG_FILL_TM(a) (((a) & BIT(5)) << 22) > #define CHCFG_FILL_AM(a) (((a) & GENMASK(4, 2)) << 6) > #define CHCFG_FILL_LVL(a) (((a) & BIT(1)) << 5) > @@ -607,12 +609,14 @@ static int rz_dmac_config(struct dma_chan *chan, > if (val == CHCFG_DS_INVALID) > return -EINVAL; > > + channel->chcfg &= ~CHCFG_FILL_DDS_MASK; > channel->chcfg |= CHCFG_FILL_DDS(val); > > val = rz_dmac_ds_to_val_mapping(config->src_addr_width); > if (val == CHCFG_DS_INVALID) > return -EINVAL; > > + channel->chcfg &= ~CHCFG_FILL_SDS_MASK; > channel->chcfg |= CHCFG_FILL_SDS(val); > > return 0; > -- > 2.25.1 -- ~Vinod ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v2 2/2] dma: rz-dmac: Fix destination and source data size setting 2023-07-06 6:14 ` Vinod Koul @ 2023-07-06 7:06 ` Biju Das 0 siblings, 0 replies; 5+ messages in thread From: Biju Das @ 2023-07-06 7:06 UTC (permalink / raw) To: Vinod Koul Cc: Hien Huynh, Geert Uytterhoeven, Prabhakar Mahadev Lad, dmaengine@vger.kernel.org, linux-renesas-soc@vger.kernel.org Hi Vinod, Thanks for the feedback. > -----Original Message----- > From: Vinod Koul <vkoul@kernel.org> > Sent: Thursday, July 6, 2023 7:15 AM > To: Biju Das <biju.das.jz@bp.renesas.com> > Cc: Hien Huynh <hien.huynh.px@renesas.com>; Geert Uytterhoeven > <geert+renesas@glider.be>; Prabhakar Mahadev Lad <prabhakar.mahadev- > lad.rj@bp.renesas.com>; dmaengine@vger.kernel.org; linux-renesas- > soc@vger.kernel.org > Subject: Re: [PATCH v2 2/2] dma: rz-dmac: Fix destination and source > data size setting > > On 30-06-23, 17:17, Biju Das wrote: > > From: Hien Huynh <hien.huynh.px@renesas.com> > > patch title should be dmaengine: xxx Oops, missed that. > > > > > Before setting DDS and SDS values, we need to clear its value first > > otherwise, we get incorrect results when we change/update the DMA bus > > width several times due to the 'OR' expression. > > > > > > Fixes: 5000d37042a6 ("dmaengine: sh: Add DMAC driver for RZ/G2L SoC") > > Signed-off-by: Hien Huynh <hien.huynh.px@renesas.com> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > v1->v2: > > * Updated patch header. > > --- > > drivers/dma/sh/rz-dmac.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c index > > 229f642fde6b..331ea80f21b0 100644 > > --- a/drivers/dma/sh/rz-dmac.c > > +++ b/drivers/dma/sh/rz-dmac.c > > @@ -145,8 +145,10 @@ struct rz_dmac { > > #define CHCFG_REQD BIT(3) > > #define CHCFG_SEL(bits) ((bits) & 0x07) > > #define CHCFG_MEM_COPY (0x80400008) > > -#define CHCFG_FILL_DDS(a) (((a) << 16) & GENMASK(19, 16)) > > -#define CHCFG_FILL_SDS(a) (((a) << 12) & GENMASK(15, 12)) > > +#define CHCFG_FILL_DDS_MASK GENMASK(19, 16) > > +#define CHCFG_FILL_DDS(a) (((a) << 16) & CHCFG_FILL_DDS_MASK) > > +#define CHCFG_FILL_SDS_MASK GENMASK(15, 12) > > +#define CHCFG_FILL_SDS(a) (((a) << 12) & CHCFG_FILL_SDS_MASK) > > Suggestion: Consider using FIELD_PREP and FIELD_GET for this Agreed, will send v3 with these changes. Cheers, Biju > > > #define CHCFG_FILL_TM(a) (((a) & BIT(5)) << 22) > > #define CHCFG_FILL_AM(a) (((a) & GENMASK(4, 2)) << 6) > > #define CHCFG_FILL_LVL(a) (((a) & BIT(1)) << 5) > > @@ -607,12 +609,14 @@ static int rz_dmac_config(struct dma_chan *chan, > > if (val == CHCFG_DS_INVALID) > > return -EINVAL; > > > > + channel->chcfg &= ~CHCFG_FILL_DDS_MASK; > > channel->chcfg |= CHCFG_FILL_DDS(val); > > > > val = rz_dmac_ds_to_val_mapping(config->src_addr_width); > > if (val == CHCFG_DS_INVALID) > > return -EINVAL; > > > > + channel->chcfg &= ~CHCFG_FILL_SDS_MASK; > > channel->chcfg |= CHCFG_FILL_SDS(val); > > > > return 0; > > -- > > 2.25.1 > > -- > ~Vinod ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-07-06 7:07 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-06-30 16:17 [PATCH v2 0/2] RZ/G2L DMA fix/improvements Biju Das 2023-06-30 16:17 ` [PATCH v2 1/2] dmaengine: sh: rz-dmac: Improve cleanup order in probe()/remove() Biju Das 2023-06-30 16:17 ` [PATCH v2 2/2] dma: rz-dmac: Fix destination and source data size setting Biju Das 2023-07-06 6:14 ` Vinod Koul 2023-07-06 7:06 ` Biju Das
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox