bus: unifier-system-bus: Remove open coded "ranges" parsing
PATCH 1/1 bus: unifier-system-bus: Remove open coded "ranges" parsing
"ranges" is a standard property and we have common helper functions for parsing it, so let's use them.
---
Compile tested only!
--- drivers/bus/uniphier-system-bus.c | 54 +++++++------------------------ 1 file changed, 11 insertions(+), 43 deletions(-)
drivers/bus/uniphier-system-bus.c +11 -43
@@ -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 ; - } - - 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 ; - } - ranges =of_get_property (dev->of_node,"ranges" , &rlen); -if (!ranges) { -dev_err (dev,"failed to get ranges property\n" ); -return -ENOENT ; - } - - rlen /=sizeof (*ranges); - rone = pna +2 ; - -for (; rlen >= rone; rlen -= rone) { - bank =be32_to_cpup (ranges++); - addr =be32_to_cpup (ranges++); - paddr =of_translate_address (dev->of_node, ranges); -if (paddr ==OF_BAD_ADDR ) +for_each_of_range (&parser, &range) { +if (range.cpu_addr ==OF_BAD_ADDR )return -EINVAL ; - ranges += pna; - size =be32_to_cpup (ranges++); - - ret =uniphier_system_bus_add_bank (priv, bank, addr, - paddr, size); + ret =uniphier_system_bus_add_bank (priv, +upper_32_bits (range.bus_addr), +lower_32_bits (range.bus_addr), + range.cpu_addr, range.size);if (ret)return ret; }
Signature · 1 line
2.39.0
Hi Rob,
Quoted 6 lines · Rob Herring
"ranges" is a standard property and we have common helper functions for parsing it, so let's use them.
Signed-off-by Rob Herring <[email protected]>---
Compile tested only!
Please fix the driver's name.
s/unifier-system-bus/uniphier-system-bus/
Quoted 31 lines
--- drivers/bus/uniphier-system-bus.c | 54 +++++++------------------------ 1 file changed, 11 insertions(+), 43 deletions(-)drivers/bus/uniphier-system-bus.c +3
-4@@ -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)
drivers/bus/uniphier-system-bus.c +0
-2@@ -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); -
drivers/bus/uniphier-system-bus.c static int uniphier_system_bus_probe(struct · line 206
Quoted 10 lines
drivers/bus/uniphier-system-bus.c +0
-9- 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"?
drivers/bus/uniphier-system-bus.c static int uniphier_system_bus_probe(struct platform_device *pdev) · line 214
Quoted 12 lines
drivers/bus/uniphier-system-bus.c +2
-9- - 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"
Quoted 28 lines
drivers/bus/uniphier-system-bus.c +6
-19- ranges =of_get_property (dev->of_node,"ranges" , &rlen); -if (!ranges) { -dev_err (dev,"failed to get ranges property\n" ); -return -ENOENT ; - } - - rlen /=sizeof (*ranges); - rone = pna +2 ; - -for (; rlen >= rone; rlen -= rone) { - bank =be32_to_cpup (ranges++); - addr =be32_to_cpup (ranges++); - paddr =of_translate_address (dev->of_node, ranges); -if (paddr ==OF_BAD_ADDR ) +for_each_of_range (&parser, &range) { +if (range.cpu_addr ==OF_BAD_ADDR )return -EINVAL ; - ranges += pna; - size =be32_to_cpup (ranges++); - - ret =uniphier_system_bus_add_bank (priv, bank, addr, - paddr, size); + ret = uniphier_system_bus_add_bank(priv, + upper_32_bits(range.bus_addr), + lower_32_bits(range.bus_addr), + range.cpu_addr,
Quoted 4 lines
range.size);
if (ret)
return ret;
}
I confirmed the value of all the arguments of uniphier_system_bus_add_bank() match the previous ones.
Thank you,
---
Best Regards
Kunihiko Hayashi
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 7 lines · Kunihiko Hayashi
Hi Rob,
Quoted 3 lines · Rob Herring
"ranges" is a standard property and we have common helper functions for parsing it, so let's use them.
Quoted 7 lines
Quoted 3 lines
Signed-off-by Rob Herring <[email protected]>---
Compile tested only!Please fix the driver's name.
s/unifier-system-bus/uniphier-system-bus/
doh!
Quoted 30 lines
Quoted 30 lines
--- drivers/bus/uniphier-system-bus.c | 54 +++++++------------------------ 1 file changed, 11 insertions(+), 43 deletions(-)drivers/bus/uniphier-system-bus.c +3
-4@@ -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)drivers/bus/uniphier-system-bus.c +0
-0@@ -191,48 +190,17 @@ static int uniphier_system_bus_probe(struct platform_device *pdev) priv->dev = dev;
Quoted 14 lines
drivers/bus/uniphier-system-bus.c static int uniphier_system_bus_probe(struct platform_device *pdev) · line 204
Quoted 12 lines
drivers/bus/uniphier-system-bus.c +0
-11- 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.
Quoted 13 lines
drivers/bus/uniphier-system-bus.c static int uniphier_system_bus_probe(struct platform_device *pdev) · line 214
Quoted 12 lines
drivers/bus/uniphier-system-bus.c +2
-9- - 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 ; - }
Quoted 1 line
Same as "#size-cells"
While the address clearly needs cells to hold the chip select, there's no reason to restrict the size cells.
Quoted 5 lines
I confirmed the value of all the arguments of
uniphier_system_bus_add_bank()match the previous ones.Tested-by Kunihiko Hayashi <[email protected]>
Thanks.
Rob
Kunihiko Hayashi
maintainerHi Rob,
Quoted 64 lines · Rob Herring
Quoted 7 lines · Kunihiko Hayashi
Hi Rob,
Quoted 3 lines · Rob Herring
"ranges" is a standard property and we have common helper functions for parsing it, so let's use them.
Quoted 7 lines
Quoted 3 lines
Signed-off-by Rob Herring <[email protected]>---
Compile tested only!Please fix the driver's name.
s/unifier-system-bus/uniphier-system-bus/
doh!
Quoted 30 lines
Quoted 30 lines
--- drivers/bus/uniphier-system-bus.c | 54 +++++++------------------------ 1 file changed, 11 insertions(+), 43 deletions(-)drivers/bus/uniphier-system-bus.c +3
-4@@ -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)drivers/bus/uniphier-system-bus.c +0
-0@@ -191,48 +190,17 @@ static int uniphier_system_bus_probe(struct platform_device *pdev) priv->dev = dev;Quoted 14 lines
drivers/bus/uniphier-system-bus.c static int uniphier_system_bus_probe(struct platform_device *pdev) · line 204
Quoted 12 lines
drivers/bus/uniphier-system-bus.c +0
-11- 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"?
Quoted 4 lines
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.
Quoted 13 lines
Quoted 13 lines
Quoted 12 lines
drivers/bus/uniphier-system-bus.c +2
-9- - 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 ; - }
Quoted 4 lines
Quoted 1 line
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.
Quoted 8 lines
Quoted 6 lines
I confirmed the value of all the arguments of
uniphier_system_bus_add_bank()
match the previous ones.Tested-by Kunihiko Hayashi <[email protected]>Thanks.
Thank you,
---
Best Regards
Kunihiko Hayashi