Messages in this thread | | | From | Rob Herring <> | Date | Fri, 3 Feb 2023 09:06:17 -0600 | 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@socionext.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@kernel.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@socionext.com>
Thanks.
Rob
| |