[PATCH next] PCI: imx6: Add error handling in imx_pcie_cpu_addr_fixup()

[PATCH next] PCI: imx6: Add error handling in imx_pcie_cpu_addr_fixup()

[Riyan Dhiman]

Added error handling to the imx_pcie_cpu_addr_fixup function for cases
where the memory window retrieval fails. The initial implementation did
not have error handling, and the function simply returned cpu_addr without
checking for failure conditions.

I have added -0ULL as a error return code but what should be the ideal return
code for error handling in this function, given the u64 return type? Common
approaches include returning ~0ULL or another invalid address value, but
clarification on best practices would be appreciated.

Signed-off-by: Riyan Dhiman <riyandhiman14@gmail.com>
---
Compile tested only. I am new to the PCI subsystem and not sure how to test these 
modules. Do I need dedicated hardware, or is there another way to test these changes?

 drivers/pci/controller/dwc/pci-imx6.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 0dbc333adcff..6af39852d2c2 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1023,6 +1023,10 @@ static u64 imx_pcie_cpu_addr_fixup(struct dw_pcie *pcie, u64 cpu_addr)
 		return cpu_addr;
 
 	entry = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
+	if(!entry) {
+		dev_err(pcie->dev, "Unable to get memory window.");
+		return -0ULL;
+	}
 	offset = entry->offset;
 
 	return (cpu_addr - offset);
-- 
2.46.0

Re: [PATCH next] PCI: imx6: Add error handling in imx_pcie_cpu_addr_fixup()

[Frank Li]

On Thu, Sep 05, 2024 at 11:12:55AM +0530, Riyan Dhiman wrote:
> Added error handling to the imx_pcie_cpu_addr_fixup function for cases
> where the memory window retrieval fails. The initial implementation did
> not have error handling, and the function simply returned cpu_addr without
> checking for failure conditions.
>
> I have added -0ULL as a error return code but what should be the ideal return
> code for error handling in this function, given the u64 return type? Common
> approaches include returning ~0ULL or another invalid address value, but
> clarification on best practices would be appreciated.
>
> Signed-off-by: Riyan Dhiman <riyandhiman14@gmail.com>
> ---

It is not necessary, because pp->bridge->windows should be always success.
If it is wrong, whole PCI will not work.

Even it is wrong, return 0 also wrong. dwc use it do out-bound ATU setting.
Map address 0 to outbound range have unexpected behaviors.

If it is scanned by static tool, it should be false alarm. If you really
met the issue, let me know. It is too late check it here.

Frank

> Compile tested only. I am new to the PCI subsystem and not sure how to test these
> modules. Do I need dedicated hardware, or is there another way to test these changes?
>
>  drivers/pci/controller/dwc/pci-imx6.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 0dbc333adcff..6af39852d2c2 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1023,6 +1023,10 @@ static u64 imx_pcie_cpu_addr_fixup(struct dw_pcie *pcie, u64 cpu_addr)
>  		return cpu_addr;
>
>  	entry = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
> +	if(!entry) {
> +		dev_err(pcie->dev, "Unable to get memory window.");
> +		return -0ULL;
> +	}
>  	offset = entry->offset;
>
>  	return (cpu_addr - offset);
> --
> 2.46.0
>