Re: [PATCH 2/3] dma: fsl_raid: keep MMIO bases as void __iomem and cast at access

[Frank Li]

On Thu, Jul 16, 2026 at 11:35:31AM -0700, Rosen Penev wrote:
> On Thu, Jul 16, 2026 at 10:03 AM Frank Li <Frank.li@xxxxxxxxxxx> wrote:
> >
> > On Tue, Jul 14, 2026 at 04:38:54PM -0700, Rosen Penev wrote:
> > > The fsl_re_ctrl and fsl_re_chan_cfg structures describe memory-mapped
> > > RAID Engine registers accessed only via ioread32be()/iowrite32be(), yet
> > > the pointers to them (re_regs in struct fsl_re_drv_private, and jrregs
> > > in struct fsl_re_chan) were not __iomem-qualified, so sparse emitted
> > > "different address spaces" warnings for every register access.
> > >
> > > Store both MMIO bases as a plain void __iomem * and derive jrregs with
> > > void __iomem * arithmetic from re_regs, rather than carrying typed
> > > register struct pointers through the driver. Each function that touches
> > > the registers introduces a local typed pointer (struct fsl_re_ctrl
> > > __iomem *ctrl / struct fsl_re_chan_cfg __iomem *jr) and uses ->field,
> > > which is the idiomatic kernel pattern and keeps the registers' __iomem
> > > qualification intact.
> >
> > Any document or reference show this is idiomatic kernel pattern?
> Closest thing I can find is
>
> https://lkml.indiana.edu/hypermail/linux/kernel/0409.1/2592.html
>
> As the link mentions, it's real nice to avoid the unnecessary casting in
>
> chan->jrregs = (struct fsl_re_chan_cfg *)((u8 *)re_priv->re_regs +
>                off + ptr);
>
> sparse would flag that anyway. __iomem would need to be added to both pointers.

I see.
...
> > >               return -EBUSY;
> > > +     re_regs = re_priv->re_regs;
> > >
> > >       /* Program the RE mode */
> > > -     out_be32(&re_priv->re_regs->global_config, FSL_RE_NON_DPAA_MODE);
> > > +     out_be32(&re_regs->global_config, FSL_RE_NON_DPAA_MODE);
> > >
> > >       /* Program Galois Field polynomial */
> > > -     out_be32(&re_priv->re_regs->galois_field_config, FSL_RE_GFM_POLY);
> > > +     out_be32(&re_regs->galois_field_config, FSL_RE_GFM_POLY);
> > >
> > >       dev_info(dev, "version %x, mode %x, gfp %x\n",
> > > -              in_be32(&re_priv->re_regs->re_version_id),
> > > -              in_be32(&re_priv->re_regs->global_config),
> > > -              in_be32(&re_priv->re_regs->galois_field_config));
> > > +              in_be32(&re_regs->re_version_id),
> > > +              in_be32(&re_regs->global_config),
> > > +              in_be32(&re_regs->galois_field_config));
> > >
> > >       dma_dev = &re_priv->dma_dev;
> > >       dma_dev->dev = dev;
> > > diff --git a/drivers/dma/fsl_raid.h b/drivers/dma/fsl_raid.h
> > > index 69d743c04973..6069615e2e1e 100644
> > > --- a/drivers/dma/fsl_raid.h
> > > +++ b/drivers/dma/fsl_raid.h
> > > @@ -256,7 +256,7 @@ struct fsl_re_hw_desc {
> > >  struct fsl_re_drv_private {
> > >       u8 total_chans;
> > >       struct dma_device dma_dev;
> > > -     struct fsl_re_ctrl *re_regs;
> > > +     void __iomem *re_regs;

Suggest rename 're_regs' to 'base' to disinguish easily typed or void pointer.


> > >       struct fsl_re_chan *re_jrs[FSL_RE_MAX_CHANS];
> > >       struct dma_pool *cf_desc_pool;
> > >       struct dma_pool *hw_desc_pool;
> > > @@ -273,7 +273,7 @@ struct fsl_re_chan {
> > >       struct device *dev;
> > >       struct fsl_re_drv_private *re_dev;
> > >       struct dma_chan chan;
> > > -     struct fsl_re_chan_cfg *jrregs;
> > > +     void __iomem *jrregs;

struct fsl_re_chan_cfg __iomem *jregs;

reduce many change. still can use
	chan->jrregs = re_priv->base + off + ptr

Frank

> > >       int irq;
> > >       struct tasklet_struct irqtask;
> > >       u32 alloc_count;
> > > --
> > > 2.55.0
> > >