Re: [PATCH v4 13/19] reset: starfive: Add StarFive JH7110 reset driver

[Conor Dooley]

On Tue, Feb 21, 2023 at 04:33:09PM +0100, Emil Renner Berthing wrote:
> On Tue, 21 Feb 2023 at 03:47, Hal Feng <hal.feng@xxxxxxxxxxxxxxxx> wrote:
> >
> > Add auxiliary driver to support StarFive JH7110 system
> > and always-on resets.
> >
> > Reported-by: kernel test robot <lkp@xxxxxxxxx>

Drop the reported-by here too please Hal.

> > Signed-off-by: Hal Feng <hal.feng@xxxxxxxxxxxxxxxx>

> > +static int jh7110_reset_probe(struct auxiliary_device *adev,
> > +                             const struct auxiliary_device_id *id)
> > +{
> > +       struct reset_info *info = (struct reset_info *)(id->driver_data);
> > +       void __iomem **base = (void __iomem **)dev_get_drvdata(adev->dev.parent);
> 
> Hi Hal,
> 
> I saw the kernel test robot complain about this, but I still wonder if
> the extra level of indirection is really needed. Isn't it enough to
> just add the explicit casts, so
> 
> dev_set_drvdata(priv->dev, (void *)priv->base);
> 
> in the clock drivers and here just
> 
> void __iomem *base = (void __iomem *)dev_get_drvdata(adev->dev.parent);

I *think* if you do that, sparse will complain that you cast away the
__iomem. The complaint is something like "cast removes address space
qualifier from expression".

The other option is, rather than set the base as the drvdata, just pass
the whole priv struct. That's what I did for mpfs at least & I thought I
had suggested it on v3, but must not have.
It looks prettier than the casting madness at least ;)

> > +
> > +       if (!info || !base)
> > +               return -ENODEV;
> > +
> > +       return reset_starfive_jh71x0_register(&adev->dev, adev->dev.parent->of_node,
> > +                                             *base + info->assert_offset,
> > +                                             *base + info->status_offset,
> > +                                             NULL,
> > +                                             info->nr_resets,
> > +                                             NULL);
> > +}

Attachment: signature.asc
Description: PGP signature