Hi Vinod,
On Tue, Jan 12, 2021 at 6:04 PM Vinod Koul <vkoul@xxxxxxxxxx> wrote:
> On 12-01-21, 16:54, Geert Uytterhoeven wrote:
> > On Tue, Jan 12, 2021 at 11:36 AM Vinod Koul <vkoul@xxxxxxxxxx> wrote:
> > > On 07-01-21, 19:15, Geert Uytterhoeven wrote:
> > > > The DMACs (both SYS-DMAC and RT-DMAC) on R-Car V3U differ slightly from
> > > > the DMACs on R-Car Gen2 and other R-Car Gen3 SoCs:
> > > > 1. The per-channel registers are located in a second register block.
> > > > Add support for mapping the second block, using the appropriate
> > > > offsets and stride.
> > > > 2. The common Channel Clear Register (DMACHCLR) was replaced by a
> > > > per-channel register.
> > > > Update rcar_dmac_chan_clear{,_all}() to handle this.
> > > > As rcar_dmac_init() needs to clear the status before the individual
> > > > channels are probed, channel index and base address initialization
> > > > are moved forward.
> > > >
> > > > Inspired by a patch in the BSP by Phong Hoang
> > > > <phong.hoang.wz@xxxxxxxxxxx>.
> > > >
> > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> >
> > > > --- a/drivers/dma/sh/rcar-dmac.c
> > > > +++ b/drivers/dma/sh/rcar-dmac.c
> > > > @@ -189,7 +189,7 @@ struct rcar_dmac_chan {
> > > > * struct rcar_dmac - R-Car Gen2 DMA Controller
> > > > * @engine: base DMA engine object
> > > > * @dev: the hardware device
> > > > - * @iomem: remapped I/O memory base
> > > > + * @iomem: remapped I/O memory bases (second is optional)
> > > > * @n_channels: number of available channels
> > > > * @channels: array of DMAC channels
> > > > * @channels_mask: bitfield of which DMA channels are managed by this driver
> > > > @@ -198,7 +198,7 @@ struct rcar_dmac_chan {
> > > > struct rcar_dmac {
> > > > struct dma_device engine;
> > > > struct device *dev;
> > > > - void __iomem *iomem;
> > > > + void __iomem *iomem[2];
> > >
> > > do you forsee many more memory regions, if not then why not add second
> >
> > No I don't. TBH, I didn't foresee this change either; you never know
> > what the hardware people have on their mind for the next SoC ;-)
> >
> > > region, that way changes in this patch will be lesser..?
> >
> > I did consider that option. However, doing so would imply that (a) the
> > code to map the memory regions can no longer be a loop, but has to be
> > unrolled manually, and (b) rcar_dmac_of_data.chan_reg_block can no
> > longer be used to index iomem[], but needs a conditional expression or
> > statement.
> >
> > > and it would be better to refer to a region by its name rather than
> > > iomem[1]..
> >
> > - * @iomem: remapped I/O memory base
> > + * @common_base: remapped common or combined I/O memory base
> > + * @channel_base: remapped optional channel I/O memory base
> >
> > - void __iomem *iomem;
> > + void __iomem *common_base;
> > + void __iomem *channel_base;
> >
> > If you still think this is worthwhile, I can make these changes.
>
> Either way suits me, TBH it is not a deal breaker, so i would leave it
> upto you :)
I managed to use named regions at the expense of only 6 more lines of
source code, even reducing the resulting binary size.
So stay ready for v2 ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
