[PATCH 1/2] i2c: amd8111: Remove spaces in MODULE_* macros

[PATCH 1/2] i2c: amd8111: Remove spaces in MODULE_* macros

[Filippo Muscherà]

Remove space between function name and open parenthesis in
MODULE_DEVICE_TABLE and MODULE_AUTHOR to comply with kernel
coding style.

Signed-off-by: Filippo Muscherà <filippo.muschera@gmail.com>
---
 drivers/i2c/busses/i2c-amd8111.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-amd8111.c b/drivers/i2c/busses/i2c-amd8111.c
index 42a9b1221065..51e3660e51c6 100644
--- a/drivers/i2c/busses/i2c-amd8111.c
+++ b/drivers/i2c/busses/i2c-amd8111.c
@@ -17,7 +17,7 @@
 #include <linux/io.h>
 
 MODULE_LICENSE("GPL");
-MODULE_AUTHOR ("Vojtech Pavlik <vojtech@suse.cz>");
+MODULE_AUTHOR("Vojtech Pavlik <vojtech@suse.cz>");
 MODULE_DESCRIPTION("AMD8111 SMBus 2.0 driver");
 
 struct amd_smbus {
@@ -417,7 +417,7 @@ static const struct pci_device_id amd8111_ids[] = {
 	{ 0, }
 };
 
-MODULE_DEVICE_TABLE (pci, amd8111_ids);
+MODULE_DEVICE_TABLE(pci, amd8111_ids);
 
 static int amd8111_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
-- 
2.52.0

[PATCH 2/2] i2c: amd8111: switch to devm_ functions

[Filippo Muscherà]

Use devm_kzalloc() to manage the memory allocation of the smbus structure
and devm_request_region() to manage the I/O port region.

This simplifies the error handling paths in the probe function by removing
manual cleanup and allows for the removal of the explicit cleanup in the
remove function.

Signed-off-by: Filippo Muscherà <filippo.muschera@gmail.com>
---
 drivers/i2c/busses/i2c-amd8111.c | 26 +++++++-------------------
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/drivers/i2c/busses/i2c-amd8111.c b/drivers/i2c/busses/i2c-amd8111.c
index 51e3660e51c6..dd9ac4bb6704 100644
--- a/drivers/i2c/busses/i2c-amd8111.c
+++ b/drivers/i2c/busses/i2c-amd8111.c
@@ -427,7 +427,7 @@ static int amd8111_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (!(pci_resource_flags(dev, 0) & IORESOURCE_IO))
 		return -ENODEV;
 
-	smbus = kzalloc(sizeof(struct amd_smbus), GFP_KERNEL);
+	smbus = devm_kzalloc(&dev->dev, sizeof(struct amd_smbus), GFP_KERNEL);
 	if (!smbus)
 		return -ENOMEM;
 
@@ -436,19 +436,15 @@ static int amd8111_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	smbus->size = pci_resource_len(dev, 0);
 
 	error = acpi_check_resource_conflict(&dev->resource[0]);
-	if (error) {
-		error = -ENODEV;
-		goto out_kfree;
-	}
+	if (error)
+		return -ENODEV;
 
-	if (!request_region(smbus->base, smbus->size, amd8111_driver.name)) {
-		error = -EBUSY;
-		goto out_kfree;
-	}
+	if (!devm_request_region(&dev->dev, smbus->base, smbus->size, amd8111_driver.name))
+		return -EBUSY;
 
 	smbus->adapter.owner = THIS_MODULE;
 	snprintf(smbus->adapter.name, sizeof(smbus->adapter.name),
-		"SMBus2 AMD8111 adapter at %04x", smbus->base);
+		 "SMBus2 AMD8111 adapter at %04x", smbus->base);
 	smbus->adapter.class = I2C_CLASS_HWMON;
 	smbus->adapter.algo = &smbus_algorithm;
 	smbus->adapter.algo_data = smbus;
@@ -459,16 +455,10 @@ static int amd8111_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	pci_write_config_dword(smbus->dev, AMD_PCI_MISC, 0);
 	error = i2c_add_adapter(&smbus->adapter);
 	if (error)
-		goto out_release_region;
+		return error;
 
 	pci_set_drvdata(dev, smbus);
 	return 0;
-
- out_release_region:
-	release_region(smbus->base, smbus->size);
- out_kfree:
-	kfree(smbus);
-	return error;
 }
 
 static void amd8111_remove(struct pci_dev *dev)
@@ -476,8 +466,6 @@ static void amd8111_remove(struct pci_dev *dev)
 	struct amd_smbus *smbus = pci_get_drvdata(dev);
 
 	i2c_del_adapter(&smbus->adapter);
-	release_region(smbus->base, smbus->size);
-	kfree(smbus);
 }
 
 static struct pci_driver amd8111_driver = {
-- 
2.52.0

Re: [PATCH 2/2] i2c: amd8111: switch to devm_ functions

[Andi Shyti]

Hi Filippo,

On Mon, Feb 02, 2026 at 02:13:04PM +0100, Filippo Muscherà wrote:
> Use devm_kzalloc() to manage the memory allocation of the smbus structure
> and devm_request_region() to manage the I/O port region.

Merged to i2c/i2c-host-2.

Just a few notes for the next time:

1. I prefer two different patches for devm_kzalloc() and
   devm_request_region().

2. For series with more than one patch, I'd like to have a cover
   letter to introduce the series.

Thanks,
Andi

> This simplifies the error handling paths in the probe function by removing
> manual cleanup and allows for the removal of the explicit cleanup in the
> remove function.
> 
> Signed-off-by: Filippo Muscherà <filippo.muschera@gmail.com>

Re: [PATCH 2/2] i2c: amd8111: switch to devm_ functions

[Filippo Muscherà]

On Thu, Feb 05, 2026 at 12:03:51PM +0100, Andi Shyti wrote:
> Hi Filippo,
> 
> On Mon, Feb 02, 2026 at 02:13:04PM +0100, Filippo Muscherà wrote:
> > Use devm_kzalloc() to manage the memory allocation of the smbus structure
> > and devm_request_region() to manage the I/O port region.
> 
> Merged to i2c/i2c-host-2.
> 
> Just a few notes for the next time:
> 
> 1. I prefer two different patches for devm_kzalloc() and
>    devm_request_region().
> 
> 2. For series with more than one patch, I'd like to have a cover
>    letter to introduce the series.
> 
> Thanks,
> Andi

Hi Andi,

Thanks for the review and the notes.
I'll make sure to follow these guidelines for my future submissions.

Thanks,
Filippo 

Re: [PATCH 2/2] i2c: amd8111: switch to devm_ functions

[Andy Shevchenko]

On Mon, Feb 02, 2026 at 02:13:04PM +0100, Filippo Muscherà wrote:
> Use devm_kzalloc() to manage the memory allocation of the smbus structure
> and devm_request_region() to manage the I/O port region.
> 
> This simplifies the error handling paths in the probe function by removing
> manual cleanup and allows for the removal of the explicit cleanup in the
> remove function.

...

> -	if (!request_region(smbus->base, smbus->size, amd8111_driver.name)) {
> -		error = -EBUSY;
> -		goto out_kfree;
> -	}
> +	if (!devm_request_region(&dev->dev, smbus->base, smbus->size, amd8111_driver.name))
> +		return -EBUSY;

This is a PCI driver. Why not using the pcim_enable_device() with the
respective pcim_*() call for the IO region?

I believe the driver misses pci_disable_device() as of the current
implementation, switching to pcim_*() should address that as well.

...

>  static void amd8111_remove(struct pci_dev *dev)

>  	struct amd_smbus *smbus = pci_get_drvdata(dev);
>  
>  	i2c_del_adapter(&smbus->adapter);
> -	release_region(smbus->base, smbus->size);
> -	kfree(smbus);

(Somewhere here I would expect pci_disable_device() to be called.)

>  }

-- 
With Best Regards,
Andy Shevchenko

[PATCH] i2c: amd8111: Switch to pcim_enable_device() and pcim_request_region()

[Filippo Muscherà]

Following the conversion to managed devm_* APIs, update the driver to use
the PCI-specific managed APIs.

Use pcim_enable_device() to properly enable the PCI device and
pcim_request_region() to manage the I/O port region.

Switching to pcim_enable_device() also addresses the fact that
pci_disable_device() was  missing in the driver lifecycle, as the
managed API now automatically handles the disablement when the driver
unbinds.

Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Filippo Muscherà <filippo.muschera@gmail.com>
---
 drivers/i2c/busses/i2c-amd8111.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-amd8111.c b/drivers/i2c/busses/i2c-amd8111.c
index dd9ac4bb6704..3e8f7a59df51 100644
--- a/drivers/i2c/busses/i2c-amd8111.c
+++ b/drivers/i2c/busses/i2c-amd8111.c
@@ -427,6 +427,10 @@ static int amd8111_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (!(pci_resource_flags(dev, 0) & IORESOURCE_IO))
 		return -ENODEV;
 
+	error = pcim_enable_device(dev);
+	if (error)
+		return error;
+
 	smbus = devm_kzalloc(&dev->dev, sizeof(struct amd_smbus), GFP_KERNEL);
 	if (!smbus)
 		return -ENOMEM;
@@ -439,8 +443,9 @@ static int amd8111_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (error)
 		return -ENODEV;
 
-	if (!devm_request_region(&dev->dev, smbus->base, smbus->size, amd8111_driver.name))
-		return -EBUSY;
+	error = pcim_request_region(dev, 0, amd8111_driver.name);
+	if (error)
+		return error;
 
 	smbus->adapter.owner = THIS_MODULE;
 	snprintf(smbus->adapter.name, sizeof(smbus->adapter.name),
-- 
2.52.0

Re: [PATCH] i2c: amd8111: Switch to pcim_enable_device() and pcim_request_region()

[Andy Shevchenko]

On Tue, Feb 24, 2026 at 11:22:16AM +0100, Filippo Muscherà wrote:
> Following the conversion to managed devm_* APIs, update the driver to use
> the PCI-specific managed APIs.
> 
> Use pcim_enable_device() to properly enable the PCI device and
> pcim_request_region() to manage the I/O port region.
> 
> Switching to pcim_enable_device() also addresses the fact that
> pci_disable_device() was  missing in the driver lifecycle, as the
> managed API now automatically handles the disablement when the driver
> unbinds.

Looking at the code now I see the difference this patch may bring.
I was under impression that there is pci_enable_device() already in
the code. But it is not the case, which makes quite a different
enumeration flow (it will write CMD register and touch some bits
that might be sensitive). While the code looks okay and I can even
Acked-by: Andy Shevchenko <andriy.shevchenko@intel.com>
the change needs to be tested on real hardware before going in.

...

>  	if (!(pci_resource_flags(dev, 0) & IORESOURCE_IO))
>  		return -ENODEV;

While it seems now being unneeded, I would leave this check in place until
somebody who knows the platform better can come up with a clear justification
for its removal. It also sounds aligned with the above remark. It might be
we can not do this patch at all.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] i2c: amd8111: Switch to pcim_enable_device() and pcim_request_region()

[Filippo Muscherà]

On Tue, Feb 24, 2026 at 12:38:13PM +0200, Andy Shevchenko wrote:
> Looking at the code now I see the difference this patch may bring.
> I was under impression that there is pci_enable_device() already in
> the code. But it is not the case, which makes quite a different
> enumeration flow (it will write CMD register and touch some bits
> that might be sensitive). While the code looks okay and I can even
> Acked-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> the change needs to be tested on real hardware before going in.

Hi Andy,

Thanks for the review and the Acked-by.
Unfortunately I don't have access to an AMD8111 system to test if
pcim_enable_device() causes any regressions.

I completely see the point you're making now. I was focusing purely on
the API modernization and I didn't consider these legacy implications.

I leave it entirely up to you and the subsystem maintainers whether to
queue this patch for testing or just drop it to avoid any risks.

Best regards,
Filippo