Re: [PATCH] ACPI/IORT: fix output id retrieval for platform devices

* Re: [PATCH] ACPI/IORT: fix output id retrieval for platform devices
  2020-02-17 14:15 [PATCH] ACPI/IORT: fix output id retrieval for platform devices Pankaj Bansal
@ 2020-02-17 11:04 ` Lorenzo Pieralisi
  0 siblings, 0 replies; 2+ messages in thread
From: Lorenzo Pieralisi @ 2020-02-17 11:04 UTC (permalink / raw)
  To: Pankaj Bansal
  Cc: Hanjun Guo, Sudeep Holla, Rafael J . Wysocki, Len Brown,
	Varun Sethi, linux-acpi, linux-arm-kernel, linux-kernel

On Mon, Feb 17, 2020 at 07:45:04PM +0530, Pankaj Bansal wrote:
> As per IORT spec if Single mapping flag is not defined for
> a node, the output id ought to to be retrieved using input
> id from an ID mapping.
> 
> Current code of output id retrieval for platform devices
> ignored this case. fixed the same.

This patch fixes nothing, if anything it may break existing
platforms. Please answer the question I asked:

https://lore.kernel.org/lkml/20200214174949.GA30484@e121166-lin.cambridge.arm.com/

> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> ---
>  drivers/acpi/arm64/iort.c | 36 +++++++++++++++++-------------------
>  1 file changed, 17 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index ed3d2d1a7ae9..00e562dadd2a 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -358,7 +358,9 @@ static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in,
>  	/* Single mapping does not care for input id */
>  	if (map->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
>  		if (type == ACPI_IORT_NODE_NAMED_COMPONENT ||
> -		    type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX) {
> +		    type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX ||
> +		    type == ACPI_IORT_NODE_SMMU_V3 ||
> +		    type == ACPI_IORT_NODE_PMCG) {
>  			*rid_out = map->output_base;
>  			return 0;
>  		}
> @@ -376,7 +378,8 @@ static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in,
>  }
>  
>  static struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
> -					       u32 *id_out, int index)
> +					       u32 id_in, u32 *id_out,
> +					       int index)
>  {
>  	struct acpi_iort_node *parent;
>  	struct acpi_iort_id_mapping *map;
> @@ -398,15 +401,8 @@ static struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
>  	parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
>  			       map->output_reference);
>  
> -	if (map->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
> -		if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT ||
> -		    node->type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX ||
> -		    node->type == ACPI_IORT_NODE_SMMU_V3 ||
> -		    node->type == ACPI_IORT_NODE_PMCG) {
> -			*id_out = map->output_base;
> -			return parent;
> -		}
> -	}
> +	if (!iort_id_map(map, node->type, id_in, id_out))
> +		return parent;
>  
>  	return NULL;
>  }
> @@ -510,14 +506,14 @@ static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
>  }
>  
>  static struct acpi_iort_node *iort_node_map_platform_id(
> -		struct acpi_iort_node *node, u32 *id_out, u8 type_mask,
> -		int index)
> +		struct acpi_iort_node *node, u32 id_in, u32 *id_out,
> +		u8 type_mask, int index)
>  {
>  	struct acpi_iort_node *parent;
>  	u32 id;
>  
>  	/* step 1: retrieve the initial dev id */
> -	parent = iort_node_get_id(node, &id, index);
> +	parent = iort_node_get_id(node, id_in, &id, index);
>  	if (!parent)
>  		return NULL;
>  
> @@ -592,7 +588,8 @@ u32 iort_msi_map_rid(struct device *dev, u32 req_id)
>  /**
>   * iort_pmsi_get_dev_id() - Get the device id for a device
>   * @dev: The device for which the mapping is to be done.
> - * @dev_id: The device ID found.
> + * @dev_id: On function entry the input id for device.
> + *          On successful function exit, the output device ID found.
>   *
>   * Returns: 0 for successful find a dev id, -ENODEV on error
>   */
> @@ -608,11 +605,11 @@ int iort_pmsi_get_dev_id(struct device *dev, u32 *dev_id)
>  	index = iort_get_id_mapping_index(node);
>  	/* if there is a valid index, go get the dev_id directly */
>  	if (index >= 0) {
> -		if (iort_node_get_id(node, dev_id, index))
> +		if (iort_node_get_id(node, *dev_id, dev_id, index))
>  			return 0;
>  	} else {
>  		for (i = 0; i < node->mapping_count; i++) {
> -			if (iort_node_map_platform_id(node, dev_id,
> +			if (iort_node_map_platform_id(node, *dev_id, dev_id,
>  						      IORT_MSI_TYPE, i))
>  				return 0;
>  		}
> @@ -761,7 +758,7 @@ static struct irq_domain *iort_get_platform_device_domain(struct device *dev)
>  
>  	/* then find its msi parent node */
>  	for (i = 0; i < node->mapping_count; i++) {
> -		msi_parent = iort_node_map_platform_id(node, NULL,
> +		msi_parent = iort_node_map_platform_id(node, 0, NULL,
>  						       IORT_MSI_TYPE, i);
>  		if (msi_parent)
>  			break;
> @@ -1038,7 +1035,8 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev)
>  			return NULL;
>  
>  		do {
> -			parent = iort_node_map_platform_id(node, &streamid,
> +			parent = iort_node_map_platform_id(node, streamid,
> +							   &streamid,
>  							   IORT_IOMMU_TYPE,
>  							   i++);
>  
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread