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

[Jonathan Cameron]

On Wed, 12 Jun 2024 11:56:28 +0300
Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx>

LGTM - one comment inline.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>



> 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,