lkml.org 
[lkml]   [2023]   [Feb]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] bus: unifier-system-bus: Remove open coded "ranges" parsing
From
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

\
 
 \ /
  Last update: 2023-03-27 00:11    [W:0.161 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site