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: <CAL_JsqLfq6U3OORsKE962f2RZ7kQXLNEb2ShzrWqGY+=oVsf=g@mail.gmail.com>
Date:   Fri, 3 Feb 2023 09:06:17 -0600
From:   Rob Herring <robh@...nel.org>
To:     Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>
Cc:     Masami Hiramatsu <mhiramat@...nel.org>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] bus: unifier-system-bus: Remove open coded "ranges" parsing

On Wed, Feb 1, 2023 at 11:50 PM Kunihiko Hayashi
<hayashi.kunihiko@...ionext.com> wrote:
>
> Hi Rob,
>
> On 2023/02/02 7:00, Rob Herring wrote:
> > "ranges" is a standard property and we have common helper functions for
> > parsing it, so let's use them.
> >
> > Signed-off-by: Rob Herring <robh@...nel.org>
> > ---
> > Compile tested only!
>
> Please fix the driver's name.
>
> s/unifier-system-bus/uniphier-system-bus/

doh!

>
> > ---
> >   drivers/bus/uniphier-system-bus.c | 54 +++++++------------------------
> >   1 file changed, 11 insertions(+), 43 deletions(-)
> >
> > diff --git a/drivers/bus/uniphier-system-bus.c
> > b/drivers/bus/uniphier-system-bus.c
> > index f70dedace20b..cb5c89ce7b86 100644
> > --- a/drivers/bus/uniphier-system-bus.c
> > +++ b/drivers/bus/uniphier-system-bus.c
> > @@ -176,10 +176,9 @@ static int uniphier_system_bus_probe(struct
> > platform_device *pdev)
> >   {
> >       struct device *dev = &pdev->dev;
> >       struct uniphier_system_bus_priv *priv;
> > -     const __be32 *ranges;
> > -     u32 cells, addr, size;
> > -     u64 paddr;
> > -     int pna, bank, rlen, rone, ret;
> > +     struct of_range_parser parser;
> > +     struct of_range range;
> > +     int ret;
> >
> >       priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> >       if (!priv)
> > @@ -191,48 +190,17 @@ static int uniphier_system_bus_probe(struct
> > platform_device *pdev)
> >
> >       priv->dev = dev;
> >
> > -     pna = of_n_addr_cells(dev->of_node);
> > -
> > -     ret = of_property_read_u32(dev->of_node, "#address-cells",
> > &cells);
> > -     if (ret) {
> > -             dev_err(dev, "failed to get #address-cells\n");
> > -             return ret;
> > -     }
> > -     if (cells != 2) {
> > -             dev_err(dev, "#address-cells must be 2\n");
> > -             return -EINVAL;
> > -     }
>
> Don't you need to check the value of "#address-cells"?

Doesn't your schema do that?

It's not the kernel's job to validate the DT. If it is, then it does a
terrible job.

> > -
> > -     ret = of_property_read_u32(dev->of_node, "#size-cells", &cells);
> > -     if (ret) {
> > -             dev_err(dev, "failed to get #size-cells\n");
> > +     ret = of_range_parser_init(&parser, dev->of_node);
> > +     if (ret)
> >               return ret;
> > -     }
> > -     if (cells != 1) {
> > -             dev_err(dev, "#size-cells must be 1\n");
> > -             return -EINVAL;
> > -     }
>
> Same as "#size-cells"

While the address clearly needs cells to hold the chip select, there's
no reason to restrict the size cells.

>
> I confirmed the value of all the arguments of uniphier_system_bus_add_bank()
> match the previous ones.
>
> Tested-by: Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>

Thanks.

Rob

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ