Messages in this thread | | | Date | Mon, 6 Feb 2023 23:30:49 +0900 | Subject | Re: [PATCH] bus: unifier-system-bus: Remove open coded "ranges" parsing | From | Kunihiko Hayashi <> |
| |
Hi Rob,
On 2023/02/04 0:06, Rob Herring wrote: > 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.
Ah, this is the code before DT validation, and it's no longer necessary.
>>> - >>> - 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 see. I understand that size is limited by value, not by cell width. It's also no longer necessary.
>> >> 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. Thank you,
--- Best Regards Kunihiko Hayashi
| |