linux-kernel - Re: [PATCH v2 04/14] reset: starfive: Factor out common JH71X0 reset code

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]

Date:   Mon, 21 Nov 2022 17:23:24 +0800
From:   Hal Feng <hal.feng@...rfivetech.com>
To:     Emil Renner Berthing <emil.renner.berthing@...onical.com>
CC:     "linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
        Conor Dooley <conor@...nel.org>,
        Palmer Dabbelt <palmer@...belt.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Stephen Boyd <sboyd@...nel.org>,
        "Michael Turquette" <mturquette@...libre.com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 04/14] reset: starfive: Factor out common JH71X0 reset
 code

On Sat, 19 Nov 2022 00:39:35 +0800, Emil Renner Berthing wrote:
> On Fri, 18 Nov 2022 at 02:06, Hal Feng <hal.feng@...rfivetech.com> wrote:
> > diff --git a/drivers/reset/starfive/reset-starfive-jh7100.c b/drivers/reset/starfive/reset-starfive-jh71x0.c
> > similarity index 50%
> > copy from drivers/reset/starfive/reset-starfive-jh7100.c
> > copy to drivers/reset/starfive/reset-starfive-jh71x0.c
> > index fc44b2fb3e03..1e230f3f9841 100644
> > --- a/drivers/reset/starfive/reset-starfive-jh7100.c
> > +++ b/drivers/reset/starfive/reset-starfive-jh71x0.c

[...]

> > -static int __init jh7100_reset_probe(struct platform_device *pdev)
> > +int reset_starfive_jh7100_register(struct device *dev, struct device_node *of_node,
> > +                                  void __iomem *assert, void __iomem *status,
> > +                                  const u64 *asserted, unsigned int nr_resets,
> > +                                  bool is_module)
> >  {
> >         struct jh7100_reset *data;
> >
> > -       data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> > +       data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> >         if (!data)
> >                 return -ENOMEM;
> >
> > -       data->base = devm_platform_ioremap_resource(pdev, 0);
> > -       if (IS_ERR(data->base))
> > -               return PTR_ERR(data->base);
> > -
> >         data->rcdev.ops = &jh7100_reset_ops;
> > -       data->rcdev.owner = THIS_MODULE;
> > -       data->rcdev.nr_resets = JH7100_RSTN_END;
> > -       data->rcdev.dev = &pdev->dev;
> > -       data->rcdev.of_node = pdev->dev.of_node;
> > +       if (is_module)
> > +               data->rcdev.owner = THIS_MODULE;
> 
> nit: consider just passing the owner directly, so this would just be
> data->rcdev.owner = owner;
> 
> ..and callers that used false can just pass NULL.

Yeah, will fix it.

> 
> > +       data->rcdev.nr_resets = nr_resets;
> > +       data->rcdev.dev = dev;
> > +       data->rcdev.of_node = of_node;
> 
> Is it important to register this with the auxiliary device and not
> just use the parent device?

I'm not sure whether it still works if we use the same device, but
it's general to separate the devices of clock and reset. They have
different device names and different drivers.

Best regards,
Hal

> If not you can just always pass the device that has the right of_node
> and have this be
> 
> data->rcdev.of_node = dev->of_node;
> 
> > +
> >         spin_lock_init(&data->lock);
> > +       data->assert = assert;
> > +       data->status = status;
> > +       data->asserted = asserted;
> >
> > -       return devm_reset_controller_register(&pdev->dev, &data->rcdev);
> > +       return devm_reset_controller_register(dev, &data->rcdev);
> >  }