[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240613144143.00003ec9@Huawei.com>
Date: Thu, 13 Jun 2024 14:41:43 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
CC: Bjorn Helgaas <bhelgaas@...gle.com>, <linux-pci@...r.kernel.org>, "Lorenzo
Pieralisi" <lpieralisi@...nel.org>, Rob Herring <robh@...nel.org>, Krzysztof
Wilczyński <kw@...ux.com>, Thierry Reding
<thierry.reding@...il.com>, Jonathan Hunter <jonathanh@...dia.com>, "Robert
Richter" <rric@...nel.org>, <linux-tegra@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 2/3] PCI: Use resource_set_{range,size}() helpers
On Wed, 12 Jun 2024 11:56:28 +0300
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com> wrote:
> 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@...ux.intel.com>
LGTM - one comment inline.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> 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
> @@ -1948,15 +1943,15 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
>
> 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);
I wonder. Maybe it's worth defining an ALIGN_DOWN_IF_NON_ZERO() as locally
at least this pattern is annoying common and we can't just change
pci_resource_alignment() to return 1 in those cases because we want a
clean way to check it's not set in a lot of places.
Bikeshedding that name might take longer than it's worth though.
> mmio_pref.start -= resource_size(res);
>
> pci_bus_distribute_available_resources(b, add_list, io, mmio,
Powered by blists - more mailing lists