| Message ID | 1566904231-25486-2-git-send-email-yoshihiro.shimoda.uh@renesas.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | dmaengine: rcar-dmac: minor modifications | expand |
Hi Shimoda-san, On Tue, Aug 27, 2019 at 1:12 PM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Since we will have changed memory mapping of the DMAC in the future, > this patch uses of_data values instead of a macro to calculate > each channel's base offset. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- a/drivers/dma/sh/rcar-dmac.c > +++ b/drivers/dma/sh/rcar-dmac.c > @@ -208,12 +208,20 @@ struct rcar_dmac { > > #define to_rcar_dmac(d) container_of(d, struct rcar_dmac, engine) > > +/* > + * struct rcar_dmac_of_data - This driver's OF data > + * @chan_offset_base: DMAC channels base offset > + * @chan_offset_coefficient: DMAC channels offset coefficient Perhaps "stride" instead of "coefficient"? Or "step"? > @@ -1803,10 +1813,15 @@ static int rcar_dmac_probe(struct platform_device *pdev) > unsigned int channels_offset = 0; > struct dma_device *engine; > struct rcar_dmac *dmac; > + const struct rcar_dmac_of_data *data; > struct resource *mem; > unsigned int i; > int ret; > > + data = of_device_get_match_data(&pdev->dev); > + if (!data) > + return -EINVAL; This cannot fail, as the driver is DT only, and all entries in the match table have a data pointer. Gr{oetje,eeting}s, Geert
On Tue, Aug 27, 2019 at 03:08:16PM +0200, Geert Uytterhoeven wrote: > Hi Shimoda-san, > > On Tue, Aug 27, 2019 at 1:12 PM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > Since we will have changed memory mapping of the DMAC in the future, > > this patch uses of_data values instead of a macro to calculate > > each channel's base offset. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > --- a/drivers/dma/sh/rcar-dmac.c > > +++ b/drivers/dma/sh/rcar-dmac.c > > @@ -208,12 +208,20 @@ struct rcar_dmac { > > > > #define to_rcar_dmac(d) container_of(d, struct rcar_dmac, engine) > > > > +/* > > + * struct rcar_dmac_of_data - This driver's OF data > > + * @chan_offset_base: DMAC channels base offset > > + * @chan_offset_coefficient: DMAC channels offset coefficient > > Perhaps "stride" instead of "coefficient"? Or "step"? > > > @@ -1803,10 +1813,15 @@ static int rcar_dmac_probe(struct platform_device *pdev) > > unsigned int channels_offset = 0; > > struct dma_device *engine; > > struct rcar_dmac *dmac; > > + const struct rcar_dmac_of_data *data; > > struct resource *mem; > > unsigned int i; > > int ret; > > > > + data = of_device_get_match_data(&pdev->dev); > > + if (!data) > > + return -EINVAL; > > This cannot fail, as the driver is DT only, and all entries in the match table > have a data pointer. It seems to me that not including this check would make the code both more fragile and less intuitive for a marginal gain in simplicity. > > 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 >
diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c index eb6f231..1ff6d81 100644 --- a/drivers/dma/sh/rcar-dmac.c +++ b/drivers/dma/sh/rcar-dmac.c @@ -208,12 +208,20 @@ struct rcar_dmac { #define to_rcar_dmac(d) container_of(d, struct rcar_dmac, engine) +/* + * struct rcar_dmac_of_data - This driver's OF data + * @chan_offset_base: DMAC channels base offset + * @chan_offset_coefficient: DMAC channels offset coefficient + */ +struct rcar_dmac_of_data { + u32 chan_offset_base; + u32 chan_offset_coefficient; +}; + /* ----------------------------------------------------------------------------- * Registers */ -#define RCAR_DMAC_CHAN_OFFSET(i) (0x8000 + 0x80 * (i)) - #define RCAR_DMAISTA 0x0020 #define RCAR_DMASEC 0x0030 #define RCAR_DMAOR 0x0060 @@ -1721,6 +1729,7 @@ static const struct dev_pm_ops rcar_dmac_pm = { static int rcar_dmac_chan_probe(struct rcar_dmac *dmac, struct rcar_dmac_chan *rchan, + const struct rcar_dmac_of_data *data, unsigned int index) { struct platform_device *pdev = to_platform_device(dmac->dev); @@ -1730,7 +1739,8 @@ static int rcar_dmac_chan_probe(struct rcar_dmac *dmac, int ret; rchan->index = index; - rchan->iomem = dmac->iomem + RCAR_DMAC_CHAN_OFFSET(index); + rchan->iomem = dmac->iomem + data->chan_offset_base + + data->chan_offset_coefficient * index; rchan->mid_rid = -EINVAL; spin_lock_init(&rchan->lock); @@ -1803,10 +1813,15 @@ static int rcar_dmac_probe(struct platform_device *pdev) unsigned int channels_offset = 0; struct dma_device *engine; struct rcar_dmac *dmac; + const struct rcar_dmac_of_data *data; struct resource *mem; unsigned int i; int ret; + data = of_device_get_match_data(&pdev->dev); + if (!data) + return -EINVAL; + dmac = devm_kzalloc(&pdev->dev, sizeof(*dmac), GFP_KERNEL); if (!dmac) return -ENOMEM; @@ -1890,7 +1905,7 @@ static int rcar_dmac_probe(struct platform_device *pdev) INIT_LIST_HEAD(&engine->channels); for (i = 0; i < dmac->n_channels; ++i) { - ret = rcar_dmac_chan_probe(dmac, &dmac->channels[i], + ret = rcar_dmac_chan_probe(dmac, &dmac->channels[i], data, i + channels_offset); if (ret < 0) goto error; @@ -1938,8 +1953,16 @@ static void rcar_dmac_shutdown(struct platform_device *pdev) rcar_dmac_stop_all_chan(dmac); } +static const struct rcar_dmac_of_data rcar_dmac_data = { + .chan_offset_base = 0x8000, + .chan_offset_coefficient = 0x80, +}; + static const struct of_device_id rcar_dmac_of_ids[] = { - { .compatible = "renesas,rcar-dmac", }, + { + .compatible = "renesas,rcar-dmac", + .data = &rcar_dmac_data, + }, { /* Sentinel */ } }; MODULE_DEVICE_TABLE(of, rcar_dmac_of_ids);
Since we will have changed memory mapping of the DMAC in the future, this patch uses of_data values instead of a macro to calculate each channel's base offset. Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- drivers/dma/sh/rcar-dmac.c | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-)