linux-kernel - Re: [PATCH 4/4] dmaengine: rcar-dmac: Add support for R-Car V3U

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210112170415.GU2771@vkoul-mobl>
Date:   Tue, 12 Jan 2021 22:34:15 +0530
From:   Vinod Koul <vkoul@...nel.org>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Dan Williams <dan.j.williams@...el.com>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
        Phong Hoang <phong.hoang.wz@...esas.com>,
        dmaengine <dmaengine@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/4] dmaengine: rcar-dmac: Add support for R-Car V3U

On 12-01-21, 16:54, Geert Uytterhoeven wrote:
> Hi Vinod,
> 
> On Tue, Jan 12, 2021 at 11:36 AM Vinod Koul <vkoul@...nel.org> 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@...esas.com>.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@...der.be>
> 
> > > --- 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 :)

-- 
~Vinod