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

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: <Y/TygwbxyuhbDooX@spud>
Date:   Tue, 21 Feb 2023 16:34:11 +0000
From:   Conor Dooley <conor@...nel.org>
To:     Emil Renner Berthing <emil.renner.berthing@...onical.com>
Cc:     Hal Feng <hal.feng@...rfivetech.com>, linux-clk@...r.kernel.org,
        devicetree@...r.kernel.org, linux-riscv@...ts.infradead.org,
        Stephen Boyd <sboyd@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Palmer Dabbelt <palmer@...belt.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        Ben Dooks <ben.dooks@...ive.com>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Marc Zyngier <maz@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 13/19] reset: starfive: Add StarFive JH7110 reset
 driver

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@...rfivetech.com> wrote:
> >
> > Add auxiliary driver to support StarFive JH7110 system
> > and always-on resets.
> >
> > Reported-by: kernel test robot <lkp@...el.com>

Drop the reported-by here too please Hal.

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

> > +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);
> > +}

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)