diff mbox series

[v3,03/10] cxl/pci: Fix NULL vs ERR_PTR confusion

Message ID 163379785305.692348.7804260538462033304.stgit@dwillia2-desk3.amr.corp.intel.com
State New, archived
Headers show
Series cxl_pci refactor for reusability | expand

Commit Message

Dan Williams Oct. 9, 2021, 4:44 p.m. UTC
cxl_pci_map_regblock() may return an ERR_PTR(), but cxl_pci_setup_regs()
is only prepared for NULL as the error case.

Fixes: f8a7e8c29be8 ("cxl/pci: Reserve all device regions at once")
Cc: <stable@vger.kernel.org>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/cxl/pci.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ira Weiny Oct. 10, 2021, 3:44 a.m. UTC | #1
On Sat, Oct 09, 2021 at 09:44:13AM -0700, Dan Williams wrote:
> cxl_pci_map_regblock() may return an ERR_PTR(), but cxl_pci_setup_regs()
> is only prepared for NULL as the error case.
> 
> Fixes: f8a7e8c29be8 ("cxl/pci: Reserve all device regions at once")
> Cc: <stable@vger.kernel.org>
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Ira Weiny <ira.weiny@intel.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/cxl/pci.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index ccc7c2573ddc..9c178002d49e 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -317,7 +317,7 @@ static void __iomem *cxl_pci_map_regblock(struct cxl_mem *cxlm,
>  	if (pci_resource_len(pdev, bar) < offset) {
>  		dev_err(dev, "BAR%d: %pr: too small (offset: %#llx)\n", bar,
>  			&pdev->resource[bar], (unsigned long long)offset);
> -		return IOMEM_ERR_PTR(-ENXIO);
> +		return NULL;
>  	}
>  
>  	addr = pci_iomap(pdev, bar, 0);
>
Jonathan Cameron Oct. 15, 2021, 4:15 p.m. UTC | #2
On Sat, 9 Oct 2021 09:44:13 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> cxl_pci_map_regblock() may return an ERR_PTR(), but cxl_pci_setup_regs()
> is only prepared for NULL as the error case.
> 

What's the logic behind doing this rather than adjusting the call site to
check for an error pointer?

Either approach is fine as far as I'm concerned though so this is really
just a request for a bit more info in this patch description.

FWIW

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> Fixes: f8a7e8c29be8 ("cxl/pci: Reserve all device regions at once")
> Cc: <stable@vger.kernel.org>
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/cxl/pci.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index ccc7c2573ddc..9c178002d49e 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -317,7 +317,7 @@ static void __iomem *cxl_pci_map_regblock(struct cxl_mem *cxlm,
>  	if (pci_resource_len(pdev, bar) < offset) {
>  		dev_err(dev, "BAR%d: %pr: too small (offset: %#llx)\n", bar,
>  			&pdev->resource[bar], (unsigned long long)offset);
> -		return IOMEM_ERR_PTR(-ENXIO);
> +		return NULL;
>  	}
>  
>  	addr = pci_iomap(pdev, bar, 0);
>
Dan Williams Oct. 15, 2021, 8:16 p.m. UTC | #3
On Fri, Oct 15, 2021 at 9:16 AM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Sat, 9 Oct 2021 09:44:13 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
>
> > cxl_pci_map_regblock() may return an ERR_PTR(), but cxl_pci_setup_regs()
> > is only prepared for NULL as the error case.
> >
>
> What's the logic behind doing this rather than adjusting the call site to
> check for an error pointer?

Minimize the fix for the stable backport. In the later patches the
cxl_pci_map_regblock() => cxl_map_regblock() conversion goes from
returning a pointer to an error code.

> Either approach is fine as far as I'm concerned though so this is really
> just a request for a bit more info in this patch description.

I can include that note above to clarify.

>
> FWIW
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Thanks.
diff mbox series

Patch

diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index ccc7c2573ddc..9c178002d49e 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -317,7 +317,7 @@  static void __iomem *cxl_pci_map_regblock(struct cxl_mem *cxlm,
 	if (pci_resource_len(pdev, bar) < offset) {
 		dev_err(dev, "BAR%d: %pr: too small (offset: %#llx)\n", bar,
 			&pdev->resource[bar], (unsigned long long)offset);
-		return IOMEM_ERR_PTR(-ENXIO);
+		return NULL;
 	}
 
 	addr = pci_iomap(pdev, bar, 0);