[PATCH 1/2] ACPI / LPSS: Make condition local to lpt_register_clock_device

[PATCH 1/2] ACPI / LPSS: Make condition local to lpt_register_clock_device

[Andy Shevchenko]

It seems more logical to have the check of lpss_clk_dev variable in
lpt_register_clock_device() because last one actually assignes the variable.

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

diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index c87db0e..c695841 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -84,6 +84,9 @@ static struct platform_device *lpss_clk_dev;
 
 static inline void lpt_register_clock_device(void)
 {
+	if (lpss_clk_dev)
+		return;
+
 	lpss_clk_dev = platform_device_register_simple("clk-lpt", -1, NULL, 0);
 }
 
@@ -92,8 +95,7 @@ static int register_device_clock(struct acpi_device *adev,
 {
 	const struct lpss_device_desc *dev_desc = pdata->dev_desc;
 
-	if (!lpss_clk_dev)
-		lpt_register_clock_device();
+	lpt_register_clock_device();
 
 	if (!dev_desc->clk_parent || !pdata->mmio_base
 	    || pdata->mmio_size < dev_desc->prv_offset + LPSS_CLK_SIZE)
-- 
1.8.2.rc0.22.gb3600c3

[PATCH 2/2] ACPI / LPSS: make code less confusing for reader

[Andy Shevchenko]

The excerpt like this:

	if (err) {
		err = 0;
		goto error_out;
	}

makes a reader confused even if it's commented. Let's do necessary actions and
return no error explicitly.

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

diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index c695841..5d7ded2 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -152,8 +152,8 @@ static int acpi_lpss_create_device(struct acpi_device *adev,
 			 * Skip the device, but don't terminate the namespace
 			 * scan.
 			 */
-			ret = 0;
-			goto err_out;
+			kfree(pdata);
+			return 0;
 		}
 	}
 
-- 
1.8.2.rc0.22.gb3600c3

Re: [PATCH 2/2] ACPI / LPSS: make code less confusing for reader

[Rafael J. Wysocki]

On Wednesday, March 20, 2013 02:14:30 PM Andy Shevchenko wrote:
> The excerpt like this:
> 
> 	if (err) {
> 		err = 0;
> 		goto error_out;
> 	}
> 
> makes a reader confused even if it's commented. Let's do necessary actions and
> return no error explicitly.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied to linux-pm.git/linux-next.

Thanks,
Rafael


> ---
>  drivers/acpi/acpi_lpss.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
> index c695841..5d7ded2 100644
> --- a/drivers/acpi/acpi_lpss.c
> +++ b/drivers/acpi/acpi_lpss.c
> @@ -152,8 +152,8 @@ static int acpi_lpss_create_device(struct acpi_device *adev,
>  			 * Skip the device, but don't terminate the namespace
>  			 * scan.
>  			 */
> -			ret = 0;
> -			goto err_out;
> +			kfree(pdata);
> +			return 0;
>  		}
>  	}
>  
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 1/2] ACPI / LPSS: Make condition local to lpt_register_clock_device

[Rafael J. Wysocki]

On Wednesday, March 20, 2013 02:14:29 PM Andy Shevchenko wrote:
> It seems more logical to have the check of lpss_clk_dev variable in
> lpt_register_clock_device() because last one actually assignes the variable.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Actually, I came to the opposite conclusion when I was working on the code
in question. :-)

Not applied.

Thanks,
Rafael


> ---
>  drivers/acpi/acpi_lpss.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
> index c87db0e..c695841 100644
> --- a/drivers/acpi/acpi_lpss.c
> +++ b/drivers/acpi/acpi_lpss.c
> @@ -84,6 +84,9 @@ static struct platform_device *lpss_clk_dev;
>  
>  static inline void lpt_register_clock_device(void)
>  {
> +	if (lpss_clk_dev)
> +		return;
> +
>  	lpss_clk_dev = platform_device_register_simple("clk-lpt", -1, NULL, 0);
>  }
>  
> @@ -92,8 +95,7 @@ static int register_device_clock(struct acpi_device *adev,
>  {
>  	const struct lpss_device_desc *dev_desc = pdata->dev_desc;
>  
> -	if (!lpss_clk_dev)
> -		lpt_register_clock_device();
> +	lpt_register_clock_device();
>  
>  	if (!dev_desc->clk_parent || !pdata->mmio_base
>  	    || pdata->mmio_size < dev_desc->prv_offset + LPSS_CLK_SIZE)
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.