[PATCH v1 0/2] mfd: ls2kbmc: A fix and a refactoring

[PATCH v1 0/2] mfd: ls2kbmc: A fix and a refactoring

[Andy Shevchenko]

Just an ad-hoc fix and refactoring as I have noticed the problematic fix.

Andy Shevchenko (2):
  mfd: ls2kbmc: Fully convert to use managed resources
  mfd: ls2kbmc: Use PCI API instead of direct accesses

 drivers/mfd/ls2k-bmc-core.c | 38 ++++++++++---------------------------
 1 file changed, 10 insertions(+), 28 deletions(-)

-- 
2.50.1

[PATCH v1 1/2] mfd: ls2kbmc: Fully convert to use managed resources

[Andy Shevchenko]

The mixing managed and non-managed resources lay lead to all possible
use-after-free bugs. In this driver the problematic part is the configuration
space bits that may just gone behind the functions back, e.g., when interrupt
is being served. Fix this by switching to managed resources for PCI.

Fixes: 91a3e1f5453a ("mfd: ls2kbmc: Check for devm_mfd_add_devices() failure")
Fixes: d952bba3fbb5 ("mfd: ls2kbmc: Add Loongson-2K BMC reset function support")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/ls2k-bmc-core.c | 36 +++++++++---------------------------
 1 file changed, 9 insertions(+), 27 deletions(-)

diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
index 69387dad6661..616ff0a28b00 100644
--- a/drivers/mfd/ls2k-bmc-core.c
+++ b/drivers/mfd/ls2k-bmc-core.c
@@ -464,25 +464,23 @@ static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	resource_size_t base;
 	int ret;
 
-	ret = pci_enable_device(dev);
+	ret = pcim_enable_device(dev);
 	if (ret)
 		return ret;
 
 	ddata = devm_kzalloc(&dev->dev, sizeof(*ddata), GFP_KERNEL);
-	if (!ddata) {
-		ret = -ENOMEM;
-		goto disable_pci;
-	}
+	if (!ddata)
+		return -ENOMEM;
 
 	ddata->dev = &dev->dev;
 
 	ret = ls2k_bmc_init(ddata);
 	if (ret)
-		goto disable_pci;
+		return ret;
 
 	ret = ls2k_bmc_parse_mode(dev, &pd);
 	if (ret)
-		goto disable_pci;
+		return ret;
 
 	ls2k_bmc_cells[LS2K_BMC_DISPLAY].platform_data = &pd;
 	ls2k_bmc_cells[LS2K_BMC_DISPLAY].pdata_size = sizeof(pd);
@@ -490,27 +488,12 @@ static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	/* Remove conflicting efifb device */
 	ret = aperture_remove_conflicting_devices(base, SZ_4M, "simple-framebuffer");
-	if (ret) {
-		dev_err(&dev->dev, "Failed to removed firmware framebuffers: %d\n", ret);
-		goto disable_pci;
-	}
-
-	ret = devm_mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
-				   ls2k_bmc_cells, ARRAY_SIZE(ls2k_bmc_cells),
-				   &dev->resource[0], 0, NULL);
 	if (ret)
-		goto disable_pci;
+		return dev_err_probe(&dev->dev, ret, "Failed to removed firmware framebuffers\n");
 
-	return 0;
-
-disable_pci:
-	pci_disable_device(dev);
-	return ret;
-}
-
-static void ls2k_bmc_remove(struct pci_dev *dev)
-{
-	pci_disable_device(dev);
+	return devm_mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
+				    ls2k_bmc_cells, ARRAY_SIZE(ls2k_bmc_cells),
+				    &dev->resource[0], 0, NULL);
 }
 
 static struct pci_device_id ls2k_bmc_devices[] = {
@@ -523,7 +506,6 @@ static struct pci_driver ls2k_bmc_driver = {
 	.name = "ls2k-bmc",
 	.id_table = ls2k_bmc_devices,
 	.probe = ls2k_bmc_probe,
-	.remove = ls2k_bmc_remove,
 };
 module_pci_driver(ls2k_bmc_driver);
 
-- 
2.50.1

[PATCH v1 2/2] mfd: ls2kbmc: Use PCI API instead of direct accesses

[Andy Shevchenko]

There is PCI API to access device resources. Use it instead of
direct accesses.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/ls2k-bmc-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
index 616ff0a28b00..7b0a5fed1d1d 100644
--- a/drivers/mfd/ls2k-bmc-core.c
+++ b/drivers/mfd/ls2k-bmc-core.c
@@ -484,7 +484,7 @@ static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	ls2k_bmc_cells[LS2K_BMC_DISPLAY].platform_data = &pd;
 	ls2k_bmc_cells[LS2K_BMC_DISPLAY].pdata_size = sizeof(pd);
-	base = dev->resource[0].start + LS2K_DISPLAY_RES_START;
+	base = pci_resource_start(dev, 0) + LS2K_DISPLAY_RES_START;
 
 	/* Remove conflicting efifb device */
 	ret = aperture_remove_conflicting_devices(base, SZ_4M, "simple-framebuffer");
@@ -493,7 +493,7 @@ static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	return devm_mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
 				    ls2k_bmc_cells, ARRAY_SIZE(ls2k_bmc_cells),
-				    &dev->resource[0], 0, NULL);
+				    pci_resource_n(dev, 0), 0, NULL);
 }
 
 static struct pci_device_id ls2k_bmc_devices[] = {
-- 
2.50.1

Re: [PATCH v1 0/2] mfd: ls2kbmc: A fix and a refactoring

[Binbin Zhou]

Hi Andy:

Thanks for your patches.

On 2025/10/30 19:36, Andy Shevchenko wrote:
> Just an ad-hoc fix and refactoring as I have noticed the problematic fix.
>
> Andy Shevchenko (2):
>    mfd: ls2kbmc: Fully convert to use managed resources
>    mfd: ls2kbmc: Use PCI API instead of direct accesses
>
>   drivers/mfd/ls2k-bmc-core.c | 38 ++++++++++---------------------------
>   1 file changed, 10 insertions(+), 28 deletions(-)
>
> 	

For the whole series:

Reviewed-by: Binbin Zhou <zhoubinbin@loongson.cn>


Thanks.
Binbin

Re: [PATCH v1 1/2] mfd: ls2kbmc: Fully convert to use managed resources

[Lee Jones]

On Thu, 30 Oct 2025, Andy Shevchenko wrote:

> The mixing managed and non-managed resources lay lead to all possible

Nit: "The mixing of ... may lead"

"all" really?  Maybe drop that bit.

> use-after-free bugs. In this driver the problematic part is the configuration
> space bits that may just gone behind the functions back, e.g., when interrupt

"have gone", but is this the best way we can describe this?

> is being served. Fix this by switching to managed resources for PCI.
> 
> Fixes: 91a3e1f5453a ("mfd: ls2kbmc: Check for devm_mfd_add_devices() failure")
> Fixes: d952bba3fbb5 ("mfd: ls2kbmc: Add Loongson-2K BMC reset function support")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/ls2k-bmc-core.c | 36 +++++++++---------------------------
>  1 file changed, 9 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
> index 69387dad6661..616ff0a28b00 100644
> --- a/drivers/mfd/ls2k-bmc-core.c
> +++ b/drivers/mfd/ls2k-bmc-core.c
> @@ -464,25 +464,23 @@ static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  	resource_size_t base;
>  	int ret;
>  
> -	ret = pci_enable_device(dev);
> +	ret = pcim_enable_device(dev);
>  	if (ret)
>  		return ret;
>  
>  	ddata = devm_kzalloc(&dev->dev, sizeof(*ddata), GFP_KERNEL);
> -	if (!ddata) {
> -		ret = -ENOMEM;
> -		goto disable_pci;
> -	}
> +	if (!ddata)
> +		return -ENOMEM;
>  
>  	ddata->dev = &dev->dev;
>  
>  	ret = ls2k_bmc_init(ddata);
>  	if (ret)
> -		goto disable_pci;
> +		return ret;
>  
>  	ret = ls2k_bmc_parse_mode(dev, &pd);
>  	if (ret)
> -		goto disable_pci;
> +		return ret;
>  
>  	ls2k_bmc_cells[LS2K_BMC_DISPLAY].platform_data = &pd;
>  	ls2k_bmc_cells[LS2K_BMC_DISPLAY].pdata_size = sizeof(pd);
> @@ -490,27 +488,12 @@ static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  
>  	/* Remove conflicting efifb device */
>  	ret = aperture_remove_conflicting_devices(base, SZ_4M, "simple-framebuffer");
> -	if (ret) {
> -		dev_err(&dev->dev, "Failed to removed firmware framebuffers: %d\n", ret);
> -		goto disable_pci;
> -	}
> -
> -	ret = devm_mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
> -				   ls2k_bmc_cells, ARRAY_SIZE(ls2k_bmc_cells),
> -				   &dev->resource[0], 0, NULL);
>  	if (ret)
> -		goto disable_pci;
> +		return dev_err_probe(&dev->dev, ret, "Failed to removed firmware framebuffers\n");

Nit: May as well fix "remove".

... since the patch set doesn't apply.  Please rebase onto MFD or -next.

-- 
Lee Jones [李琼斯]

Re: [PATCH v1 2/2] mfd: ls2kbmc: Use PCI API instead of direct accesses

[Lee Jones]

On Thu, 30 Oct 2025, Andy Shevchenko wrote:

> There is PCI API to access device resources. Use it instead of

Nit: "There is a ..."

> direct accesses.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/ls2k-bmc-core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
> index 616ff0a28b00..7b0a5fed1d1d 100644
> --- a/drivers/mfd/ls2k-bmc-core.c
> +++ b/drivers/mfd/ls2k-bmc-core.c
> @@ -484,7 +484,7 @@ static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  
>  	ls2k_bmc_cells[LS2K_BMC_DISPLAY].platform_data = &pd;
>  	ls2k_bmc_cells[LS2K_BMC_DISPLAY].pdata_size = sizeof(pd);
> -	base = dev->resource[0].start + LS2K_DISPLAY_RES_START;
> +	base = pci_resource_start(dev, 0) + LS2K_DISPLAY_RES_START;
>  
>  	/* Remove conflicting efifb device */
>  	ret = aperture_remove_conflicting_devices(base, SZ_4M, "simple-framebuffer");
> @@ -493,7 +493,7 @@ static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  
>  	return devm_mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
>  				    ls2k_bmc_cells, ARRAY_SIZE(ls2k_bmc_cells),
> -				    &dev->resource[0], 0, NULL);
> +				    pci_resource_n(dev, 0), 0, NULL);
>  }
>  
>  static struct pci_device_id ls2k_bmc_devices[] = {
> -- 
> 2.50.1
> 

-- 
Lee Jones [李琼斯]

Re: [PATCH v1 0/2] mfd: ls2kbmc: A fix and a refactoring

[Lee Jones]

On Thu, 30 Oct 2025 12:36:32 +0100, Andy Shevchenko wrote:
> Just an ad-hoc fix and refactoring as I have noticed the problematic fix.
> 
> Andy Shevchenko (2):
>   mfd: ls2kbmc: Fully convert to use managed resources
>   mfd: ls2kbmc: Use PCI API instead of direct accesses
> 
> drivers/mfd/ls2k-bmc-core.c | 38 ++++++++++---------------------------
>  1 file changed, 10 insertions(+), 28 deletions(-)
> 
> [...]

Applied, thanks!

[1/2] mfd: ls2kbmc: Fully convert to use managed resources
      commit: 5bf5a793afc97fad001864d4268a45d355703956
[2/2] mfd: ls2kbmc: Use PCI API instead of direct accesses
      commit: 13f3c189c8d298930cfb92efaf037f9af0db5569

--
Lee Jones [李琼斯]