[PATCH 0/3] of: address: Miscellaneous-improvements

[PATCH 0/3] of: address: Miscellaneous-improvements

[Geert Uytterhoeven]

	Hi Rob, Frank,

This patch series updates the linuxdoc comments for the return values
for of_address_to_resource() and of_pci_range_to_resource(), and removes
forward declarations.

Thanks for your comments!

Geert Uytterhoeven (3):
  of: address: Document return value of of_address_to_resource()
  of: address: Fix documented return value of of_pci_range_to_resource()
  of: address: Reshuffle to remove forward declarations

 drivers/of/address.c | 271 +++++++++++++++++++++----------------------
 1 file changed, 134 insertions(+), 137 deletions(-)

-- 
2.34.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH 1/3] of: address: Document return value of of_address_to_resource()

[Geert Uytterhoeven]

Add the missing return value documentation to the linuxdoc comment block
for the of_address_to_resource() function.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/of/address.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 4c5c3b7ab99449f2..6312b10885e85864 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -882,6 +882,8 @@ static int __of_address_to_resource(struct device_node *dev, int index, int bar_
  * @index:	Index into the array
  * @r:		Pointer to resource array
  *
+ * Returns -EINVAL if the range cannot be converted to resource.
+ *
  * Note that if your address is a PIO address, the conversion will fail if
  * the physical address can't be internally converted to an IO token with
  * pci_address_to_pio(), that is because it's either called too early or it
-- 
2.34.1

Re: [PATCH 1/3] of: address: Document return value of of_address_to_resource()

[Rob Herring]

On Fri, 31 Mar 2023 09:52:28 +0200, Geert Uytterhoeven wrote:
> Add the missing return value documentation to the linuxdoc comment block
> for the of_address_to_resource() function.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/of/address.c | 2 ++
>  1 file changed, 2 insertions(+)
> 

Applied, thanks!

[PATCH 2/3] of: address: Fix documented return value of of_pci_range_to_resource()

[Geert Uytterhoeven]

of_pci_range_to_resource() returns a negative instead of a positive
error code.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/of/address.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 6312b10885e85864..bd965d104c83a1ac 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -216,7 +216,7 @@ EXPORT_SYMBOL_GPL(of_pci_address_to_resource);
  * @res:	pointer to a valid resource that will be updated to
  *              reflect the values contained in the range.
  *
- * Returns EINVAL if the range cannot be converted to resource.
+ * Returns -EINVAL if the range cannot be converted to resource.
  *
  * Note that if the range is an IO range, the resource will be converted
  * using pci_address_to_pio() which can fail if it is called too early or
-- 
2.34.1

Re: [PATCH 2/3] of: address: Fix documented return value of of_pci_range_to_resource()

[Rob Herring]

On Fri, 31 Mar 2023 09:52:29 +0200, Geert Uytterhoeven wrote:
> of_pci_range_to_resource() returns a negative instead of a positive
> error code.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/of/address.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Applied, thanks!

[PATCH 3/3] of: address: Reshuffle to remove forward declarations

[Geert Uytterhoeven]

Reshuffle the code to get rid of the forward declarations, which
improves readability.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/of/address.c | 271 +++++++++++++++++++++----------------------
 1 file changed, 133 insertions(+), 138 deletions(-)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index bd965d104c83a1ac..80b36dfdb69c24c8 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -24,11 +24,6 @@
 #define OF_CHECK_ADDR_COUNT(na)	((na) > 0 && (na) <= OF_MAX_ADDR_CELLS)
 #define OF_CHECK_COUNTS(na, ns)	(OF_CHECK_ADDR_COUNT(na) && (ns) > 0)
 
-static struct of_bus *of_match_bus(struct device_node *np);
-static int __of_address_to_resource(struct device_node *dev, int index,
-		int bar_no, struct resource *r);
-static bool of_mmio_is_nonposted(struct device_node *np);
-
 /* Debug utility */
 #ifdef DEBUG
 static void of_dump_addr(const char *s, const __be32 *addr, int na)
@@ -198,17 +193,6 @@ static int of_bus_pci_translate(__be32 *addr, u64 offset, int na)
 }
 #endif /* CONFIG_PCI */
 
-int of_pci_address_to_resource(struct device_node *dev, int bar,
-			       struct resource *r)
-{
-
-	if (!IS_ENABLED(CONFIG_PCI))
-		return -ENOSYS;
-
-	return __of_address_to_resource(dev, -1, bar, r);
-}
-EXPORT_SYMBOL_GPL(of_pci_address_to_resource);
-
 /*
  * of_pci_range_to_resource - Create a resource from an of_pci_range
  * @range:	the PCI range that describes the resource
@@ -837,128 +821,6 @@ static u64 of_translate_ioport(struct device_node *dev, const __be32 *in_addr,
 	return port;
 }
 
-static int __of_address_to_resource(struct device_node *dev, int index, int bar_no,
-		struct resource *r)
-{
-	u64 taddr;
-	const __be32	*addrp;
-	u64		size;
-	unsigned int	flags;
-	const char	*name = NULL;
-
-	addrp = __of_get_address(dev, index, bar_no, &size, &flags);
-	if (addrp == NULL)
-		return -EINVAL;
-
-	/* Get optional "reg-names" property to add a name to a resource */
-	if (index >= 0)
-		of_property_read_string_index(dev, "reg-names",	index, &name);
-
-	if (flags & IORESOURCE_MEM)
-		taddr = of_translate_address(dev, addrp);
-	else if (flags & IORESOURCE_IO)
-		taddr = of_translate_ioport(dev, addrp, size);
-	else
-		return -EINVAL;
-
-	if (taddr == OF_BAD_ADDR)
-		return -EINVAL;
-	memset(r, 0, sizeof(struct resource));
-
-	if (of_mmio_is_nonposted(dev))
-		flags |= IORESOURCE_MEM_NONPOSTED;
-
-	r->start = taddr;
-	r->end = taddr + size - 1;
-	r->flags = flags;
-	r->name = name ? name : dev->full_name;
-
-	return 0;
-}
-
-/**
- * of_address_to_resource - Translate device tree address and return as resource
- * @dev:	Caller's Device Node
- * @index:	Index into the array
- * @r:		Pointer to resource array
- *
- * Returns -EINVAL if the range cannot be converted to resource.
- *
- * Note that if your address is a PIO address, the conversion will fail if
- * the physical address can't be internally converted to an IO token with
- * pci_address_to_pio(), that is because it's either called too early or it
- * can't be matched to any host bridge IO space
- */
-int of_address_to_resource(struct device_node *dev, int index,
-			   struct resource *r)
-{
-	return __of_address_to_resource(dev, index, -1, r);
-}
-EXPORT_SYMBOL_GPL(of_address_to_resource);
-
-/**
- * of_iomap - Maps the memory mapped IO for a given device_node
- * @np:		the device whose io range will be mapped
- * @index:	index of the io range
- *
- * Returns a pointer to the mapped memory
- */
-void __iomem *of_iomap(struct device_node *np, int index)
-{
-	struct resource res;
-
-	if (of_address_to_resource(np, index, &res))
-		return NULL;
-
-	if (res.flags & IORESOURCE_MEM_NONPOSTED)
-		return ioremap_np(res.start, resource_size(&res));
-	else
-		return ioremap(res.start, resource_size(&res));
-}
-EXPORT_SYMBOL(of_iomap);
-
-/*
- * of_io_request_and_map - Requests a resource and maps the memory mapped IO
- *			   for a given device_node
- * @device:	the device whose io range will be mapped
- * @index:	index of the io range
- * @name:	name "override" for the memory region request or NULL
- *
- * Returns a pointer to the requested and mapped memory or an ERR_PTR() encoded
- * error code on failure. Usage example:
- *
- *	base = of_io_request_and_map(node, 0, "foo");
- *	if (IS_ERR(base))
- *		return PTR_ERR(base);
- */
-void __iomem *of_io_request_and_map(struct device_node *np, int index,
-				    const char *name)
-{
-	struct resource res;
-	void __iomem *mem;
-
-	if (of_address_to_resource(np, index, &res))
-		return IOMEM_ERR_PTR(-EINVAL);
-
-	if (!name)
-		name = res.name;
-	if (!request_mem_region(res.start, resource_size(&res), name))
-		return IOMEM_ERR_PTR(-EBUSY);
-
-	if (res.flags & IORESOURCE_MEM_NONPOSTED)
-		mem = ioremap_np(res.start, resource_size(&res));
-	else
-		mem = ioremap(res.start, resource_size(&res));
-
-	if (!mem) {
-		release_mem_region(res.start, resource_size(&res));
-		return IOMEM_ERR_PTR(-ENOMEM);
-	}
-
-	return mem;
-}
-EXPORT_SYMBOL(of_io_request_and_map);
-
 #ifdef CONFIG_HAS_DMA
 /**
  * of_dma_get_range - Get DMA range info and put it into a map array
@@ -1155,3 +1017,136 @@ static bool of_mmio_is_nonposted(struct device_node *np)
 	of_node_put(parent);
 	return nonposted;
 }
+
+static int __of_address_to_resource(struct device_node *dev, int index, int bar_no,
+		struct resource *r)
+{
+	u64 taddr;
+	const __be32	*addrp;
+	u64		size;
+	unsigned int	flags;
+	const char	*name = NULL;
+
+	addrp = __of_get_address(dev, index, bar_no, &size, &flags);
+	if (addrp == NULL)
+		return -EINVAL;
+
+	/* Get optional "reg-names" property to add a name to a resource */
+	if (index >= 0)
+		of_property_read_string_index(dev, "reg-names",	index, &name);
+
+	if (flags & IORESOURCE_MEM)
+		taddr = of_translate_address(dev, addrp);
+	else if (flags & IORESOURCE_IO)
+		taddr = of_translate_ioport(dev, addrp, size);
+	else
+		return -EINVAL;
+
+	if (taddr == OF_BAD_ADDR)
+		return -EINVAL;
+	memset(r, 0, sizeof(struct resource));
+
+	if (of_mmio_is_nonposted(dev))
+		flags |= IORESOURCE_MEM_NONPOSTED;
+
+	r->start = taddr;
+	r->end = taddr + size - 1;
+	r->flags = flags;
+	r->name = name ? name : dev->full_name;
+
+	return 0;
+}
+
+/**
+ * of_address_to_resource - Translate device tree address and return as resource
+ * @dev:	Caller's Device Node
+ * @index:	Index into the array
+ * @r:		Pointer to resource array
+ *
+ * Returns -EINVAL if the range cannot be converted to resource.
+ *
+ * Note that if your address is a PIO address, the conversion will fail if
+ * the physical address can't be internally converted to an IO token with
+ * pci_address_to_pio(), that is because it's either called too early or it
+ * can't be matched to any host bridge IO space
+ */
+int of_address_to_resource(struct device_node *dev, int index,
+			   struct resource *r)
+{
+	return __of_address_to_resource(dev, index, -1, r);
+}
+EXPORT_SYMBOL_GPL(of_address_to_resource);
+
+int of_pci_address_to_resource(struct device_node *dev, int bar,
+			       struct resource *r)
+{
+
+	if (!IS_ENABLED(CONFIG_PCI))
+		return -ENOSYS;
+
+	return __of_address_to_resource(dev, -1, bar, r);
+}
+EXPORT_SYMBOL_GPL(of_pci_address_to_resource);
+
+/**
+ * of_iomap - Maps the memory mapped IO for a given device_node
+ * @np:		the device whose io range will be mapped
+ * @index:	index of the io range
+ *
+ * Returns a pointer to the mapped memory
+ */
+void __iomem *of_iomap(struct device_node *np, int index)
+{
+	struct resource res;
+
+	if (of_address_to_resource(np, index, &res))
+		return NULL;
+
+	if (res.flags & IORESOURCE_MEM_NONPOSTED)
+		return ioremap_np(res.start, resource_size(&res));
+	else
+		return ioremap(res.start, resource_size(&res));
+}
+EXPORT_SYMBOL(of_iomap);
+
+/*
+ * of_io_request_and_map - Requests a resource and maps the memory mapped IO
+ *			   for a given device_node
+ * @device:	the device whose io range will be mapped
+ * @index:	index of the io range
+ * @name:	name "override" for the memory region request or NULL
+ *
+ * Returns a pointer to the requested and mapped memory or an ERR_PTR() encoded
+ * error code on failure. Usage example:
+ *
+ *	base = of_io_request_and_map(node, 0, "foo");
+ *	if (IS_ERR(base))
+ *		return PTR_ERR(base);
+ */
+void __iomem *of_io_request_and_map(struct device_node *np, int index,
+				    const char *name)
+{
+	struct resource res;
+	void __iomem *mem;
+
+	if (of_address_to_resource(np, index, &res))
+		return IOMEM_ERR_PTR(-EINVAL);
+
+	if (!name)
+		name = res.name;
+	if (!request_mem_region(res.start, resource_size(&res), name))
+		return IOMEM_ERR_PTR(-EBUSY);
+
+	if (res.flags & IORESOURCE_MEM_NONPOSTED)
+		mem = ioremap_np(res.start, resource_size(&res));
+	else
+		mem = ioremap(res.start, resource_size(&res));
+
+	if (!mem) {
+		release_mem_region(res.start, resource_size(&res));
+		return IOMEM_ERR_PTR(-ENOMEM);
+	}
+
+	return mem;
+}
+EXPORT_SYMBOL(of_io_request_and_map);
-- 
2.34.1

Re: [PATCH 3/3] of: address: Reshuffle to remove forward declarations

[Rob Herring]

On Fri, Mar 31, 2023 at 2:52 AM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
>
> Reshuffle the code to get rid of the forward declarations, which
> improves readability.

Is git blame smart enough to ignore this move? If not, I'd rather keep
the blame than save 3 lines.

>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/of/address.c | 271 +++++++++++++++++++++----------------------
>  1 file changed, 133 insertions(+), 138 deletions(-)

Re: [PATCH 3/3] of: address: Reshuffle to remove forward declarations

[Geert Uytterhoeven]

Hi Rob,

On Fri, Mar 31, 2023 at 6:28 PM Rob Herring <robh+dt@kernel.org> wrote:
> On Fri, Mar 31, 2023 at 2:52 AM Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
> > Reshuffle the code to get rid of the forward declarations, which
> > improves readability.
>
> Is git blame smart enough to ignore this move? If not, I'd rather keep
> the blame than save 3 lines.

Apparently (I didn't know before ;-), it is, when using the -M option:

$ git blame -- drivers/of/address.c | tail -3
1db096f9167bc267e (Geert Uytterhoeven        2023-03-30 13:42:05 +0200
1150) return mem;
1db096f9167bc267e (Geert Uytterhoeven        2023-03-30 13:42:05 +0200 1151) }
1db096f9167bc267e (Geert Uytterhoeven        2023-03-30 13:42:05 +0200
1152) EXPORT_SYMBOL(of_io_request_and_map);

$ git blame -M -- drivers/of/address.c | tail -3
efd342fb0031a1775 (Matthias Brugger          2014-07-18 11:36:39 +0200
1150) return mem;
efd342fb0031a1775 (Matthias Brugger          2014-07-18 11:36:39 +0200 1151) }
efd342fb0031a1775 (Matthias Brugger          2014-07-18 11:36:39 +0200
1152) EXPORT_SYMBOL(of_io_request_and_map);

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 3/3] of: address: Reshuffle to remove forward declarations

[Rob Herring]

On Fri, 31 Mar 2023 09:52:30 +0200, Geert Uytterhoeven wrote:
> Reshuffle the code to get rid of the forward declarations, which
> improves readability.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/of/address.c | 271 +++++++++++++++++++++----------------------
>  1 file changed, 133 insertions(+), 138 deletions(-)
> 

Applied, thanks!