diff mbox series

[07/10] i2c: i801: Improve i801_acpi_probe/remove functions

Message ID 064c1f0b-9f79-3fb2-cac1-35ef26c33296@gmail.com
State Superseded
Headers show
Series [01/10] i2c: i801: Don't call pm_runtime_allow | expand

Commit Message

Heiner Kallweit Aug. 1, 2021, 2:21 p.m. UTC
By using ACPI_HANDLE() the handler argument can be retrieved directly.
Both address space handler functions check the handler argument and
return an error if it's NULL. This allows to further simplify the code.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/i2c/busses/i2c-i801.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

Comments

Jean Delvare Aug. 5, 2021, 1:38 p.m. UTC | #1
On Sun, 01 Aug 2021 16:21:54 +0200, Heiner Kallweit wrote:
> By using ACPI_HANDLE() the handler argument can be retrieved directly.

> Both address space handler functions check the handler argument and

> return an error if it's NULL. This allows to further simplify the code.

> 

> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

> ---

>  drivers/i2c/busses/i2c-i801.c | 23 +++++++----------------

>  1 file changed, 7 insertions(+), 16 deletions(-)

> 

> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c

> index 5b9eebc1c..5fa8dc1cb 100644

> --- a/drivers/i2c/busses/i2c-i801.c

> +++ b/drivers/i2c/busses/i2c-i801.c

> @@ -1633,31 +1633,22 @@ i801_acpi_io_handler(u32 function, acpi_physical_address address, u32 bits,

>  

>  static int i801_acpi_probe(struct i801_priv *priv)

>  {

> -	struct acpi_device *adev;

> +	acpi_handle ah = ACPI_HANDLE(&priv->pci_dev->dev);

>  	acpi_status status;

>  

> -	adev = ACPI_COMPANION(&priv->pci_dev->dev);

> -	if (adev) {

> -		status = acpi_install_address_space_handler(adev->handle,

> -				ACPI_ADR_SPACE_SYSTEM_IO, i801_acpi_io_handler,

> -				NULL, priv);

> -		if (ACPI_SUCCESS(status))

> -			return 0;

> -	}

> +	status = acpi_install_address_space_handler(ah, ACPI_ADR_SPACE_SYSTEM_IO,

> +						    i801_acpi_io_handler, NULL, priv);

> +	if (ACPI_SUCCESS(status))

> +		return 0;

>  

>  	return acpi_check_resource_conflict(&priv->pci_dev->resource[SMBBAR]);

>  }

>  

>  static void i801_acpi_remove(struct i801_priv *priv)

>  {

> -	struct acpi_device *adev;

> -

> -	adev = ACPI_COMPANION(&priv->pci_dev->dev);

> -	if (!adev)

> -		return;

> +	acpi_handle ah = ACPI_HANDLE(&priv->pci_dev->dev);

>  

> -	acpi_remove_address_space_handler(adev->handle,

> -		ACPI_ADR_SPACE_SYSTEM_IO, i801_acpi_io_handler);

> +	acpi_remove_address_space_handler(ah, ACPI_ADR_SPACE_SYSTEM_IO, i801_acpi_io_handler);

>  }

>  #else

>  static inline int i801_acpi_probe(struct i801_priv *priv) { return 0; }


Looks completely reasonable.

Reviewed-by: Jean Delvare <jdelvare@suse.de>

Tested-by: Jean Delvare <jdelvare@suse.de>


Mika, no objection?

-- 
Jean Delvare
SUSE L3 Support
Mika Westerberg Aug. 5, 2021, 2:24 p.m. UTC | #2
On Thu, Aug 05, 2021 at 03:38:57PM +0200, Jean Delvare wrote:
> On Sun, 01 Aug 2021 16:21:54 +0200, Heiner Kallweit wrote:

> > By using ACPI_HANDLE() the handler argument can be retrieved directly.

> > Both address space handler functions check the handler argument and

> > return an error if it's NULL. This allows to further simplify the code.

> > 

> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

> > ---

> >  drivers/i2c/busses/i2c-i801.c | 23 +++++++----------------

> >  1 file changed, 7 insertions(+), 16 deletions(-)

> > 

> > diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c

> > index 5b9eebc1c..5fa8dc1cb 100644

> > --- a/drivers/i2c/busses/i2c-i801.c

> > +++ b/drivers/i2c/busses/i2c-i801.c

> > @@ -1633,31 +1633,22 @@ i801_acpi_io_handler(u32 function, acpi_physical_address address, u32 bits,

> >  

> >  static int i801_acpi_probe(struct i801_priv *priv)

> >  {

> > -	struct acpi_device *adev;

> > +	acpi_handle ah = ACPI_HANDLE(&priv->pci_dev->dev);

> >  	acpi_status status;

> >  

> > -	adev = ACPI_COMPANION(&priv->pci_dev->dev);

> > -	if (adev) {

> > -		status = acpi_install_address_space_handler(adev->handle,

> > -				ACPI_ADR_SPACE_SYSTEM_IO, i801_acpi_io_handler,

> > -				NULL, priv);

> > -		if (ACPI_SUCCESS(status))

> > -			return 0;

> > -	}

> > +	status = acpi_install_address_space_handler(ah, ACPI_ADR_SPACE_SYSTEM_IO,

> > +						    i801_acpi_io_handler, NULL, priv);

> > +	if (ACPI_SUCCESS(status))

> > +		return 0;

> >  

> >  	return acpi_check_resource_conflict(&priv->pci_dev->resource[SMBBAR]);

> >  }

> >  

> >  static void i801_acpi_remove(struct i801_priv *priv)

> >  {

> > -	struct acpi_device *adev;

> > -

> > -	adev = ACPI_COMPANION(&priv->pci_dev->dev);

> > -	if (!adev)

> > -		return;

> > +	acpi_handle ah = ACPI_HANDLE(&priv->pci_dev->dev);

> >  

> > -	acpi_remove_address_space_handler(adev->handle,

> > -		ACPI_ADR_SPACE_SYSTEM_IO, i801_acpi_io_handler);

> > +	acpi_remove_address_space_handler(ah, ACPI_ADR_SPACE_SYSTEM_IO, i801_acpi_io_handler);

> >  }

> >  #else

> >  static inline int i801_acpi_probe(struct i801_priv *priv) { return 0; }

> 

> Looks completely reasonable.

> 

> Reviewed-by: Jean Delvare <jdelvare@suse.de>

> Tested-by: Jean Delvare <jdelvare@suse.de>

> 

> Mika, no objection?


No.

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 5b9eebc1c..5fa8dc1cb 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1633,31 +1633,22 @@  i801_acpi_io_handler(u32 function, acpi_physical_address address, u32 bits,
 
 static int i801_acpi_probe(struct i801_priv *priv)
 {
-	struct acpi_device *adev;
+	acpi_handle ah = ACPI_HANDLE(&priv->pci_dev->dev);
 	acpi_status status;
 
-	adev = ACPI_COMPANION(&priv->pci_dev->dev);
-	if (adev) {
-		status = acpi_install_address_space_handler(adev->handle,
-				ACPI_ADR_SPACE_SYSTEM_IO, i801_acpi_io_handler,
-				NULL, priv);
-		if (ACPI_SUCCESS(status))
-			return 0;
-	}
+	status = acpi_install_address_space_handler(ah, ACPI_ADR_SPACE_SYSTEM_IO,
+						    i801_acpi_io_handler, NULL, priv);
+	if (ACPI_SUCCESS(status))
+		return 0;
 
 	return acpi_check_resource_conflict(&priv->pci_dev->resource[SMBBAR]);
 }
 
 static void i801_acpi_remove(struct i801_priv *priv)
 {
-	struct acpi_device *adev;
-
-	adev = ACPI_COMPANION(&priv->pci_dev->dev);
-	if (!adev)
-		return;
+	acpi_handle ah = ACPI_HANDLE(&priv->pci_dev->dev);
 
-	acpi_remove_address_space_handler(adev->handle,
-		ACPI_ADR_SPACE_SYSTEM_IO, i801_acpi_io_handler);
+	acpi_remove_address_space_handler(ah, ACPI_ADR_SPACE_SYSTEM_IO, i801_acpi_io_handler);
 }
 #else
 static inline int i801_acpi_probe(struct i801_priv *priv) { return 0; }