Re: [PATCH v9 03/11] ACPI/IORT: Make iort_iommu_msi_get_resv_regions() return void

Lorenzo Pieralisi
2022-04-06T17:15:46Z

On Mon, Apr 04, 2022 at 01:42:01PM +0100, Shameer Kolothum wrote:
> At present iort_iommu_msi_get_resv_regions() returns the number of
> MSI reserved regions on success and there are no users for this.
> The reserved region list will get populated anyway for platforms
> that require the HW MSI region reservation. Hence, change the
> function to return void instead.
>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
> drivers/acpi/arm64/iort.c | 26 ++++++++++----------------
> include/linux/acpi_iort.h | 6 +++---
> 2 files changed, 13 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index fd06cf43ba31..c5ebb2be9a19 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -832,25 +832,23 @@ static struct acpi_iort_node *iort_get_msi_resv_iommu(struct device *dev)
>
> /**
> * iort_iommu_msi_get_resv_regions - Reserved region driver helper
> + * for HW MSI regions.

Very minor but is this a spurious change ?

> * @dev: Device from iommu_get_resv_regions()
> * @head: Reserved region list from iommu_get_resv_regions()
> *
> - * Returns: Number of msi reserved regions on success (0 if platform
> - * doesn't require the reservation or no associated msi regions),
> - * appropriate error value otherwise. The ITS interrupt translation
> - * spaces (ITS_base + SZ_64K, SZ_64K) associated with the device
> - * are the msi reserved regions.
> + * The ITS interrupt translation spaces (ITS_base + SZ_64K, SZ_64K)
> + * associated with the device are the HW MSI reserved regions.
> */
> -int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
> +void iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
> {
> struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> struct acpi_iort_its_group *its;
> struct acpi_iort_node *iommu_node, *its_node = NULL;
> - int i, resv = 0;
> + int i;
>
> iommu_node = iort_get_msi_resv_iommu(dev);
> if (!iommu_node)
> - return 0;
> + return;
>
> /*
> * Current logic to reserve ITS regions relies on HW topologies
> @@ -870,7 +868,7 @@ int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
> }
>
> if (!its_node)
> - return 0;
> + return;
>
> /* Move to ITS specific data */
> its = (struct acpi_iort_its_group *)its_node->node_data;
> @@ -884,14 +882,10 @@ int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
>
> region = iommu_alloc_resv_region(base + SZ_64K, SZ_64K,
> prot, IOMMU_RESV_MSI);
> - if (region) {
> + if (region)
> list_add_tail(&region->list, head);
> - resv++;
> - }
> }
> }
> -
> - return (resv == its->its_count) ? resv : -ENODEV;
> }
>
> static inline bool iort_iommu_driver_enabled(u8 type)
> @@ -1058,8 +1052,8 @@ int iort_iommu_configure_id(struct device *dev, const u32 *id_in)
> }
>
> #else
> -int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
> -{ return 0; }
> +void iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
> +{ }
> int iort_iommu_configure_id(struct device *dev, const u32 *input_id)
> { return -ENODEV; }
> #endif
> diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
> index f1f0842a2cb2..a8198b83753d 100644
> --- a/include/linux/acpi_iort.h
> +++ b/include/linux/acpi_iort.h
> @@ -36,7 +36,7 @@ int iort_pmsi_get_dev_id(struct device *dev, u32 *dev_id);
> /* IOMMU interface */
> int iort_dma_get_ranges(struct device *dev, u64 *size);
> int iort_iommu_configure_id(struct device *dev, const u32 *id_in);
> -int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head);
> +void iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head);
> phys_addr_t acpi_iort_dma_get_max_cpu_address(void);
> #else
> static inline void acpi_iort_init(void) { }
> @@ -52,8 +52,8 @@ static inline int iort_dma_get_ranges(struct device *dev, u64 *size)
> static inline int iort_iommu_configure_id(struct device *dev, const u32 *id_in)
> { return -ENODEV; }
> static inline
> -int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
> -{ return 0; }
> +void iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
> +{ }
>
> static inline phys_addr_t acpi_iort_dma_get_max_cpu_address(void)
> { return PHYS_ADDR_MAX; }
> --
> 2.25.1
>