[PATCH v2 0/4] PCI: Resource helper improvements

[PATCH v2 0/4] PCI: Resource helper improvements

[Ilpo Järvinen]

This series introduces resource_set_{range,size}() helpers to replace
often repeated code fragments to set resource start and end addresses.
The last two patches clean up nearby code.

For now, this series only converts PCI subsystem. There are plenty of
resource start/end setting code elsewhere in the kernel but those can
be converted once the helpers reach Linus' tree.

--
v2:
- Improved commit message
- Added patch to introduce ALIGN_DOWN_IF_NONZERO()

Ilpo Järvinen (4):
  resource: Add resource set range and size helpers
  PCI: Use resource_set_{range,size}() helpers
  PCI: Use align and resource helpers, and SZ_* in quirk_s3_64M()
  PCI: Introduce ALIGN_DOWN_IF_NONZERO() helper locally

 drivers/pci/controller/pci-tegra.c       |  2 +-
 drivers/pci/controller/pci-thunder-pem.c |  4 +--
 drivers/pci/ecam.c                       |  2 +-
 drivers/pci/iov.c                        |  6 ++--
 drivers/pci/pci.c                        |  3 +-
 drivers/pci/quirks.c                     | 23 +++++++---------
 drivers/pci/setup-bus.c                  | 35 ++++++++++--------------
 drivers/pci/setup-res.c                  |  7 ++---
 include/linux/ioport.h                   | 32 ++++++++++++++++++++++
 9 files changed, 68 insertions(+), 46 deletions(-)

-- 
2.39.2

[PATCH v2 1/4] resource: Add resource set range and size helpers

[Ilpo Järvinen]

Setting the end address for a resource with a given size lacks a helper
and is therefore coded manually unlike the getter side which has a
helper for resource size calculation. Also, almost all callsites that
calculate the end address for a resource also set the start address
right before it like this:

	res->start = start_addr;
	res->end = res->start + size - 1;

Thus, add resource_set_range(res, start_addr, size) that sets the start
address and calculates the end address to simplify this often repeated
fragment. In addition, introduce resource_set_size() for the cases
where setting the start address of the resource is not necessary but
mention in its kerneldoc resource_set_range() is preferred when setting
both addresses.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 include/linux/ioport.h | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index db7fe25f3370..2a1d33ad151c 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -216,6 +216,38 @@ struct resource *lookup_resource(struct resource *root, resource_size_t start);
 int adjust_resource(struct resource *res, resource_size_t start,
 		    resource_size_t size);
 resource_size_t resource_alignment(struct resource *res);
+
+/**
+ * resource_set_size - Calculates resource end address from size and start address
+ * @res: The resource descriptor
+ * @size: The size of the resource
+ *
+ * Calculates the end address for @res based on @size.
+ *
+ * Note: The start address of @res must be set when calling this function.
+ * Use resource_set_range() if setting both the start address and @size.
+ */
+static inline void resource_set_size(struct resource *res, resource_size_t size)
+{
+	res->end = res->start + size - 1;
+}
+
+/**
+ * resource_set_range - Sets resource start and end addresses
+ * @res: The resource descriptor
+ * @start: The start address for the resource
+ * @size: The size of the resource
+ *
+ * Sets @res start address and calculates the end address based on @size.
+ */
+static inline void resource_set_range(struct resource *res,
+				      resource_size_t start,
+				      resource_size_t size)
+{
+	res->start = start;
+	resource_set_size(res, size);
+}
+
 static inline resource_size_t resource_size(const struct resource *res)
 {
 	return res->end - res->start + 1;
-- 
2.39.2

[PATCH v2 2/4] PCI: Use resource_set_{range,size}() helpers

[Ilpo Järvinen]

Convert open-coded resource size calculations to use
resource_set_{range,size}() helpers.

While at it, use SZ_* for size parameter where appropriate which makes
the intent of code more obvious.

Also, cast sizes to resource_size_t, not u64.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/pci/controller/pci-tegra.c       |  2 +-
 drivers/pci/controller/pci-thunder-pem.c |  4 +--
 drivers/pci/ecam.c                       |  2 +-
 drivers/pci/iov.c                        |  6 ++--
 drivers/pci/pci.c                        |  3 +-
 drivers/pci/quirks.c                     | 20 ++++++--------
 drivers/pci/setup-bus.c                  | 35 ++++++++++--------------
 drivers/pci/setup-res.c                  |  7 ++---
 8 files changed, 34 insertions(+), 45 deletions(-)

diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
index 038d974a318e..2ce55ed1cd8b 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -1460,7 +1460,7 @@ static int tegra_pcie_get_resources(struct tegra_pcie *pcie)
 	pcie->cs = *res;
 
 	/* constrain configuration space to 4 KiB */
-	pcie->cs.end = pcie->cs.start + SZ_4K - 1;
+	resource_set_size(&pcie->cs, SZ_4K);
 
 	pcie->cfg = devm_ioremap_resource(dev, &pcie->cs);
 	if (IS_ERR(pcie->cfg)) {
diff --git a/drivers/pci/controller/pci-thunder-pem.c b/drivers/pci/controller/pci-thunder-pem.c
index 06a9855cb431..f1bd5de67997 100644
--- a/drivers/pci/controller/pci-thunder-pem.c
+++ b/drivers/pci/controller/pci-thunder-pem.c
@@ -400,9 +400,9 @@ static int thunder_pem_acpi_init(struct pci_config_window *cfg)
 		 * Reserve 64K size PEM specific resources. The full 16M range
 		 * size is required for thunder_pem_init() call.
 		 */
-		res_pem->end = res_pem->start + SZ_64K - 1;
+		resource_set_size(res_pem, SZ_64K);
 		thunder_pem_reserve_range(dev, root->segment, res_pem);
-		res_pem->end = res_pem->start + SZ_16M - 1;
+		resource_set_size(res_pem, SZ_16M);
 
 		/* Reserve PCI configuration space as well. */
 		thunder_pem_reserve_range(dev, root->segment, &cfg->res);
diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c
index 1c40d2506aef..260b7de2dbd5 100644
--- a/drivers/pci/ecam.c
+++ b/drivers/pci/ecam.c
@@ -55,7 +55,7 @@ struct pci_config_window *pci_ecam_create(struct device *dev,
 	bus_range_max = resource_size(cfgres) >> bus_shift;
 	if (bus_range > bus_range_max) {
 		bus_range = bus_range_max;
-		cfg->busr.end = busr->start + bus_range - 1;
+		resource_set_size(&cfg->busr, bus_range);
 		dev_warn(dev, "ECAM area %pR can only accommodate %pR (reduced from %pR desired)\n",
 			 cfgres, &cfg->busr, busr);
 	}
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index aaa33e8dc4c9..4be402fe9ab9 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -327,8 +327,8 @@ int pci_iov_add_virtfn(struct pci_dev *dev, int id)
 		virtfn->resource[i].name = pci_name(virtfn);
 		virtfn->resource[i].flags = res->flags;
 		size = pci_iov_resource_size(dev, i + PCI_IOV_RESOURCES);
-		virtfn->resource[i].start = res->start + size * id;
-		virtfn->resource[i].end = virtfn->resource[i].start + size - 1;
+		resource_set_range(&virtfn->resource[i],
+				   res->start + size * id, size);
 		rc = request_resource(res, &virtfn->resource[i]);
 		BUG_ON(rc);
 	}
@@ -804,7 +804,7 @@ static int sriov_init(struct pci_dev *dev, int pos)
 			goto failed;
 		}
 		iov->barsz[i] = resource_size(res);
-		res->end = res->start + resource_size(res) * total - 1;
+		resource_set_size(res, resource_size(res) * total);
 		pci_info(dev, "%s %pR: contains BAR %d for %d VFs\n",
 			 res_name, res, i, total);
 		i += bar64;
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 59e0949fb079..afcd8e49e82c 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -6559,8 +6559,7 @@ static void pci_request_resource_alignment(struct pci_dev *dev, int bar,
 	} else {
 		r->flags &= ~IORESOURCE_SIZEALIGN;
 		r->flags |= IORESOURCE_STARTALIGN;
-		r->start = align;
-		r->end = r->start + size - 1;
+		resource_set_range(r, align, size);
 	}
 	r->flags |= IORESOURCE_UNSET;
 }
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 568410e64ce6..bde0f5388d06 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -29,6 +29,7 @@
 #include <linux/nvme.h>
 #include <linux/platform_data/x86/apple.h>
 #include <linux/pm_runtime.h>
+#include <linux/sizes.h>
 #include <linux/suspend.h>
 #include <linux/switchtec.h>
 #include "pci.h"
@@ -573,8 +574,7 @@ static void quirk_extend_bar_to_page(struct pci_dev *dev)
 		const char *r_name = pci_resource_name(dev, i);
 
 		if (r->flags & IORESOURCE_MEM && resource_size(r) < PAGE_SIZE) {
-			r->end = PAGE_SIZE - 1;
-			r->start = 0;
+			resource_set_range(r, 0, PAGE_SIZE);
 			r->flags |= IORESOURCE_UNSET;
 			pci_info(dev, "%s %pR: expanded to page size\n",
 				 r_name, r);
@@ -593,8 +593,7 @@ static void quirk_s3_64M(struct pci_dev *dev)
 
 	if ((r->start & 0x3ffffff) || r->end != r->start + 0x3ffffff) {
 		r->flags |= IORESOURCE_UNSET;
-		r->start = 0;
-		r->end = 0x3ffffff;
+		resource_set_range(r, 0, SZ_64M);
 	}
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_S3,	PCI_DEVICE_ID_S3_868,		quirk_s3_64M);
@@ -1329,8 +1328,7 @@ static void quirk_dunord(struct pci_dev *dev)
 	struct resource *r = &dev->resource[1];
 
 	r->flags |= IORESOURCE_UNSET;
-	r->start = 0;
-	r->end = 0xffffff;
+	resource_set_range(r, 0, SZ_16M);
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_DUNORD,	PCI_DEVICE_ID_DUNORD_I3000,	quirk_dunord);
 
@@ -2327,8 +2325,7 @@ static void quirk_tc86c001_ide(struct pci_dev *dev)
 
 	if (r->start & 0x8) {
 		r->flags |= IORESOURCE_UNSET;
-		r->start = 0;
-		r->end = 0xf;
+		resource_set_range(r, 0, SZ_16);
 	}
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TOSHIBA_2,
@@ -2356,8 +2353,7 @@ static void quirk_plx_pci9050(struct pci_dev *dev)
 			pci_info(dev, "Re-allocating PLX PCI 9050 BAR %u to length 256 to avoid bit 7 bug\n",
 				 bar);
 			r->flags |= IORESOURCE_UNSET;
-			r->start = 0;
-			r->end = 0xff;
+			resource_set_range(r, 0, SZ_256);
 		}
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9050,
@@ -3509,13 +3505,13 @@ static void quirk_intel_ntb(struct pci_dev *dev)
 	if (rc)
 		return;
 
-	dev->resource[2].end = dev->resource[2].start + ((u64) 1 << val) - 1;
+	resource_set_size(&dev->resource[2], (resource_size_t)1 << val);
 
 	rc = pci_read_config_byte(dev, 0x00D1, &val);
 	if (rc)
 		return;
 
-	dev->resource[4].end = dev->resource[4].start + ((u64) 1 << val) - 1;
+	resource_set_size(&dev->resource[4], (resource_size_t)1 << val);
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0e08, quirk_intel_ntb);
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0e0d, quirk_intel_ntb);
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 909e6a7c3cc3..004405edf290 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -243,8 +243,7 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
 		add_size = add_res->add_size;
 		align = add_res->min_align;
 		if (!resource_size(res)) {
-			res->start = align;
-			res->end = res->start + add_size - 1;
+			resource_set_range(res, align, add_size);
 			if (pci_assign_resource(add_res->dev, idx))
 				reset_resource(res);
 		} else {
@@ -937,8 +936,7 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size,
 		return;
 	}
 
-	b_res->start = min_align;
-	b_res->end = b_res->start + size0 - 1;
+	resource_set_range(b_res, min_align, size0);
 	b_res->flags |= IORESOURCE_STARTALIGN;
 	if (bus->self && size1 > size0 && realloc_head) {
 		add_to_list(realloc_head, bus->self, b_res, size1-size0,
@@ -1127,8 +1125,7 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
 	 * Reserve some resources for CardBus.  We reserve a fixed amount
 	 * of bus space for CardBus bridges.
 	 */
-	b_res->start = pci_cardbus_io_size;
-	b_res->end = b_res->start + pci_cardbus_io_size - 1;
+	resource_set_range(b_res, pci_cardbus_io_size, pci_cardbus_io_size);
 	b_res->flags |= IORESOURCE_IO | IORESOURCE_STARTALIGN;
 	if (realloc_head) {
 		b_res->end -= pci_cardbus_io_size;
@@ -1140,8 +1137,7 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
 	b_res = &bridge->resource[PCI_CB_BRIDGE_IO_1_WINDOW];
 	if (b_res->parent)
 		goto handle_b_res_2;
-	b_res->start = pci_cardbus_io_size;
-	b_res->end = b_res->start + pci_cardbus_io_size - 1;
+	resource_set_range(b_res, pci_cardbus_io_size, pci_cardbus_io_size);
 	b_res->flags |= IORESOURCE_IO | IORESOURCE_STARTALIGN;
 	if (realloc_head) {
 		b_res->end -= pci_cardbus_io_size;
@@ -1174,8 +1170,8 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
 	 * Otherwise, allocate one region of twice the size.
 	 */
 	if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM0) {
-		b_res->start = pci_cardbus_mem_size;
-		b_res->end = b_res->start + pci_cardbus_mem_size - 1;
+		resource_set_range(b_res, pci_cardbus_mem_size,
+				   pci_cardbus_mem_size);
 		b_res->flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH |
 				    IORESOURCE_STARTALIGN;
 		if (realloc_head) {
@@ -1192,8 +1188,7 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
 	b_res = &bridge->resource[PCI_CB_BRIDGE_MEM_1_WINDOW];
 	if (b_res->parent)
 		goto handle_done;
-	b_res->start = pci_cardbus_mem_size;
-	b_res->end = b_res->start + b_res_3_size - 1;
+	resource_set_range(b_res, pci_cardbus_mem_size, b_res_3_size);
 	b_res->flags |= IORESOURCE_MEM | IORESOURCE_STARTALIGN;
 	if (realloc_head) {
 		b_res->end -= b_res_3_size;
@@ -1772,7 +1767,7 @@ static void adjust_bridge_window(struct pci_dev *bridge, struct resource *res,
 		return;
 	}
 
-	res->end = res->start + new_size - 1;
+	resource_set_size(res, new_size);
 
 	/* If the resource is part of the add_list, remove it now */
 	if (add_list)
@@ -1935,8 +1930,8 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
 		 * what is available).
 		 */
 		align = pci_resource_alignment(dev, res);
-		io.end = align ? io.start + ALIGN_DOWN(io_per_b, align) - 1
-			       : io.start + io_per_b - 1;
+		resource_set_size(&io, align ? ALIGN_DOWN(io_per_b, align)
+					     : io_per_b);
 
 		/*
 		 * The x_per_b holds the extra resource space that can be
@@ -1948,15 +1943,15 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
 
 		res = &dev->resource[PCI_BRIDGE_MEM_WINDOW];
 		align = pci_resource_alignment(dev, res);
-		mmio.end = align ? mmio.start + ALIGN_DOWN(mmio_per_b, align) - 1
-				 : mmio.start + mmio_per_b - 1;
+		resource_set_size(&mmio, align ? ALIGN_DOWN(mmio_per_b, align)
+					       : mmio_per_b);
 		mmio.start -= resource_size(res);
 
 		res = &dev->resource[PCI_BRIDGE_PREF_MEM_WINDOW];
 		align = pci_resource_alignment(dev, res);
-		mmio_pref.end = align ? mmio_pref.start +
-					ALIGN_DOWN(mmio_pref_per_b, align) - 1
-				      : mmio_pref.start + mmio_pref_per_b - 1;
+		resource_set_size(&mmio_pref,
+				  align ? ALIGN_DOWN(mmio_pref_per_b, align)
+					: mmio_pref_per_b);
 		mmio_pref.start -= resource_size(res);
 
 		pci_bus_distribute_available_resources(b, add_list, io, mmio,
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index c6d933ddfd46..ca14576bf2bf 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -211,8 +211,7 @@ static int pci_revert_fw_address(struct resource *res, struct pci_dev *dev,
 
 	start = res->start;
 	end = res->end;
-	res->start = fw_addr;
-	res->end = res->start + size - 1;
+	resource_set_range(res, fw_addr, size);
 	res->flags &= ~IORESOURCE_UNSET;
 
 	root = pci_find_parent_resource(dev, res);
@@ -463,7 +462,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size)
 	if (ret)
 		return ret;
 
-	res->end = res->start + pci_rebar_size_to_bytes(size) - 1;
+	resource_set_size(res, pci_rebar_size_to_bytes(size));
 
 	/* Check if the new config works by trying to assign everything. */
 	if (dev->bus->self) {
@@ -475,7 +474,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size)
 
 error_resize:
 	pci_rebar_set_size(dev, resno, old);
-	res->end = res->start + pci_rebar_size_to_bytes(old) - 1;
+	resource_set_size(res, pci_rebar_size_to_bytes(old));
 	return ret;
 }
 EXPORT_SYMBOL(pci_resize_resource);
-- 
2.39.2

[PATCH v2 3/4] PCI: Use align and resource helpers, and SZ_* in quirk_s3_64M()

[Ilpo Järvinen]

Use IS_ALIGNED(), resource_size(), and SZ_* defines in quirk_s3_64M().

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/pci/quirks.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index bde0f5388d06..125fc1cbad95 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -12,6 +12,7 @@
  * file, where their drivers can use them.
  */
 
+#include <linux/align.h>
 #include <linux/bitfield.h>
 #include <linux/types.h>
 #include <linux/kernel.h>
@@ -591,7 +592,7 @@ static void quirk_s3_64M(struct pci_dev *dev)
 {
 	struct resource *r = &dev->resource[0];
 
-	if ((r->start & 0x3ffffff) || r->end != r->start + 0x3ffffff) {
+	if (!IS_ALIGNED(r->start, SZ_64M) || resource_size(r) != SZ_64M) {
 		r->flags |= IORESOURCE_UNSET;
 		resource_set_range(r, 0, SZ_64M);
 	}
-- 
2.39.2

[PATCH v2 4/4] PCI: Introduce ALIGN_DOWN_IF_NONZERO() helper locally

[Ilpo Järvinen]

pci_bus_distribute_available_resources() performs alignment in case of
non-zero alignment requirement on 3 occasions. Introduce
ALIGN_DOWN_IF_NONZERO() helper to avoid coding the non-zero check 3
times.

Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---

I tried to look other similar cases for both ALIGN() and ALIGN_DOWN()
kernel-wide but it seems this is not very common so I did not put
ALIGN_DOWN_IF_NONZERO() into the generic header.
---
 drivers/pci/setup-bus.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 004405edf290..39552d1a1793 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1819,6 +1819,9 @@ static void remove_dev_resources(struct pci_dev *dev, struct resource *io,
 	}
 }
 
+#define ALIGN_DOWN_IF_NONZERO(addr, align) \
+			((align) ? ALIGN_DOWN((addr), (align)) : (addr))
+
 /*
  * io, mmio and mmio_pref contain the total amount of bridge window space
  * available. This includes the minimal space needed to cover all the
@@ -1930,8 +1933,7 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
 		 * what is available).
 		 */
 		align = pci_resource_alignment(dev, res);
-		resource_set_size(&io, align ? ALIGN_DOWN(io_per_b, align)
-					     : io_per_b);
+		resource_set_size(&io, ALIGN_DOWN_IF_NONZERO(io_per_b, align));
 
 		/*
 		 * The x_per_b holds the extra resource space that can be
@@ -1943,15 +1945,13 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
 
 		res = &dev->resource[PCI_BRIDGE_MEM_WINDOW];
 		align = pci_resource_alignment(dev, res);
-		resource_set_size(&mmio, align ? ALIGN_DOWN(mmio_per_b, align)
-					       : mmio_per_b);
+		resource_set_size(&mmio, ALIGN_DOWN_IF_NONZERO(mmio_per_b, align));
 		mmio.start -= resource_size(res);
 
 		res = &dev->resource[PCI_BRIDGE_PREF_MEM_WINDOW];
 		align = pci_resource_alignment(dev, res);
 		resource_set_size(&mmio_pref,
-				  align ? ALIGN_DOWN(mmio_pref_per_b, align)
-					: mmio_pref_per_b);
+				  ALIGN_DOWN_IF_NONZERO(mmio_pref_per_b, align));
 		mmio_pref.start -= resource_size(res);
 
 		pci_bus_distribute_available_resources(b, add_list, io, mmio,
-- 
2.39.2

Re: [PATCH v2 4/4] PCI: Introduce ALIGN_DOWN_IF_NONZERO() helper locally

[Jonathan Cameron]

On Fri, 14 Jun 2024 13:06:06 +0300
Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:

> pci_bus_distribute_available_resources() performs alignment in case of
> non-zero alignment requirement on 3 occasions. Introduce
> ALIGN_DOWN_IF_NONZERO() helper to avoid coding the non-zero check 3
> times.
> 
> Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
> 
> I tried to look other similar cases for both ALIGN() and ALIGN_DOWN()
> kernel-wide but it seems this is not very common so I did not put
> ALIGN_DOWN_IF_NONZERO() into the generic header.
Makes sense. It's a weird quirk of the return value of
pci_resource_alignement() to return 0 when it really means 1.

LGTM

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/pci/setup-bus.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 004405edf290..39552d1a1793 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1819,6 +1819,9 @@ static void remove_dev_resources(struct pci_dev *dev, struct resource *io,
>  	}
>  }
>  
> +#define ALIGN_DOWN_IF_NONZERO(addr, align) \
> +			((align) ? ALIGN_DOWN((addr), (align)) : (addr))
> +
>  /*
>   * io, mmio and mmio_pref contain the total amount of bridge window space
>   * available. This includes the minimal space needed to cover all the
> @@ -1930,8 +1933,7 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
>  		 * what is available).
>  		 */
>  		align = pci_resource_alignment(dev, res);
> -		resource_set_size(&io, align ? ALIGN_DOWN(io_per_b, align)
> -					     : io_per_b);
> +		resource_set_size(&io, ALIGN_DOWN_IF_NONZERO(io_per_b, align));
>  
>  		/*
>  		 * The x_per_b holds the extra resource space that can be
> @@ -1943,15 +1945,13 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
>  
>  		res = &dev->resource[PCI_BRIDGE_MEM_WINDOW];
>  		align = pci_resource_alignment(dev, res);
> -		resource_set_size(&mmio, align ? ALIGN_DOWN(mmio_per_b, align)
> -					       : mmio_per_b);
> +		resource_set_size(&mmio, ALIGN_DOWN_IF_NONZERO(mmio_per_b, align));
>  		mmio.start -= resource_size(res);
>  
>  		res = &dev->resource[PCI_BRIDGE_PREF_MEM_WINDOW];
>  		align = pci_resource_alignment(dev, res);
>  		resource_set_size(&mmio_pref,
> -				  align ? ALIGN_DOWN(mmio_pref_per_b, align)
> -					: mmio_pref_per_b);
> +				  ALIGN_DOWN_IF_NONZERO(mmio_pref_per_b, align));
>  		mmio_pref.start -= resource_size(res);
>  
>  		pci_bus_distribute_available_resources(b, add_list, io, mmio,

Re: [PATCH v2 0/4] PCI: Resource helper improvements

[Bjorn Helgaas]

On Fri, Jun 14, 2024 at 01:06:02PM +0300, Ilpo Järvinen wrote:
> This series introduces resource_set_{range,size}() helpers to replace
> often repeated code fragments to set resource start and end addresses.
> The last two patches clean up nearby code.
> 
> For now, this series only converts PCI subsystem. There are plenty of
> resource start/end setting code elsewhere in the kernel but those can
> be converted once the helpers reach Linus' tree.
> 
> --
> v2:
> - Improved commit message
> - Added patch to introduce ALIGN_DOWN_IF_NONZERO()
> 
> Ilpo Järvinen (4):
>   resource: Add resource set range and size helpers
>   PCI: Use resource_set_{range,size}() helpers
>   PCI: Use align and resource helpers, and SZ_* in quirk_s3_64M()
>   PCI: Introduce ALIGN_DOWN_IF_NONZERO() helper locally
> 
>  drivers/pci/controller/pci-tegra.c       |  2 +-
>  drivers/pci/controller/pci-thunder-pem.c |  4 +--
>  drivers/pci/ecam.c                       |  2 +-
>  drivers/pci/iov.c                        |  6 ++--
>  drivers/pci/pci.c                        |  3 +-
>  drivers/pci/quirks.c                     | 23 +++++++---------
>  drivers/pci/setup-bus.c                  | 35 ++++++++++--------------
>  drivers/pci/setup-res.c                  |  7 ++---
>  include/linux/ioport.h                   | 32 ++++++++++++++++++++++
>  9 files changed, 68 insertions(+), 46 deletions(-)

Sorry this slipped through the cracks.  Applied to pci/resource for
v6.13, thanks!