All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND RFC] mach:-s3c64xx:Output trace information with WARN_ON if calls for setting up gpio board configuration fail in s3c64xx_i2s_cfg_gpio
@ 2016-02-02 14:56 ` Nicholas Krause
  0 siblings, 0 replies; 4+ messages in thread
From: Nicholas Krause @ 2016-02-02 14:56 UTC (permalink / raw)
  To: kgene
  Cc: k.kozlowski, linux-samsung-soc, linux, linux-kernel, linux-arm-kernel

This fixes the function s3c64xx_i2c_cfg_gpio to log output to the
kernel log buff with WARN_ON if any of the calls to either
s3c_gpio_cfgpin_range or s3c_gpio_cfgpin fail as we cannot exit
from s3c64xx_i2s_cfg_gpio if any of these calls fail due to
other intended function work being required to complete. Further more
if a failure occurs allow the other users/devolopers of this driver
to properly check the kernel log buffers for a kernel trace for
when these calls failing to execute successfully arise.

Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
---
v3:Fix more wording issues and uncaught build failure with
previous build test
v2:Fix Patch Wording as it was clearly confusing and incorrect
 arch/arm/mach-s3c64xx/dev-audio.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-s3c64xx/dev-audio.c b/arch/arm/mach-s3c64xx/dev-audio.c
index ff780a8..796ac75 100644
--- a/arch/arm/mach-s3c64xx/dev-audio.c
+++ b/arch/arm/mach-s3c64xx/dev-audio.c
@@ -36,10 +36,10 @@ static int s3c64xx_i2s_cfg_gpio(struct platform_device *pdev)
 		base = S3C64XX_GPE(0);
 		break;
 	case 2:
-		s3c_gpio_cfgpin(S3C64XX_GPC(4), S3C_GPIO_SFN(5));
-		s3c_gpio_cfgpin(S3C64XX_GPC(5), S3C_GPIO_SFN(5));
-		s3c_gpio_cfgpin(S3C64XX_GPC(7), S3C_GPIO_SFN(5));
-		s3c_gpio_cfgpin_range(S3C64XX_GPH(6), 4, S3C_GPIO_SFN(5));
+		WARN_ON(s3c_gpio_cfgpin(S3C64XX_GPC(4), S3C_GPIO_SFN(5)));
+		WARN_ON(s3c_gpio_cfgpin(S3C64XX_GPC(5), S3C_GPIO_SFN(5)));
+		WARN_ON(s3c_gpio_cfgpin(S3C64XX_GPC(7), S3C_GPIO_SFN(5)));
+		WARN_ON(s3c_gpio_cfgpin_range(S3C64XX_GPH(6), 4, S3C_GPIO_SFN(5)));
 		return 0;
 	default:
 		printk(KERN_DEBUG "Invalid I2S Controller number: %d\n",
@@ -47,7 +47,7 @@ static int s3c64xx_i2s_cfg_gpio(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	s3c_gpio_cfgpin_range(base, 5, S3C_GPIO_SFN(3));
+	WARN_ON(s3c_gpio_cfgpin_range(base, 5, S3C_GPIO_SFN(3)));
 
 	return 0;
 }
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH RESEND RFC] mach:-s3c64xx:Output trace information with WARN_ON if calls for setting up gpio board configuration fail in s3c64xx_i2s_cfg_gpio
@ 2016-02-02 14:56 ` Nicholas Krause
  0 siblings, 0 replies; 4+ messages in thread
From: Nicholas Krause @ 2016-02-02 14:56 UTC (permalink / raw)
  To: linux-arm-kernel

This fixes the function s3c64xx_i2c_cfg_gpio to log output to the
kernel log buff with WARN_ON if any of the calls to either
s3c_gpio_cfgpin_range or s3c_gpio_cfgpin fail as we cannot exit
from s3c64xx_i2s_cfg_gpio if any of these calls fail due to
other intended function work being required to complete. Further more
if a failure occurs allow the other users/devolopers of this driver
to properly check the kernel log buffers for a kernel trace for
when these calls failing to execute successfully arise.

Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
---
v3:Fix more wording issues and uncaught build failure with
previous build test
v2:Fix Patch Wording as it was clearly confusing and incorrect
 arch/arm/mach-s3c64xx/dev-audio.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-s3c64xx/dev-audio.c b/arch/arm/mach-s3c64xx/dev-audio.c
index ff780a8..796ac75 100644
--- a/arch/arm/mach-s3c64xx/dev-audio.c
+++ b/arch/arm/mach-s3c64xx/dev-audio.c
@@ -36,10 +36,10 @@ static int s3c64xx_i2s_cfg_gpio(struct platform_device *pdev)
 		base = S3C64XX_GPE(0);
 		break;
 	case 2:
-		s3c_gpio_cfgpin(S3C64XX_GPC(4), S3C_GPIO_SFN(5));
-		s3c_gpio_cfgpin(S3C64XX_GPC(5), S3C_GPIO_SFN(5));
-		s3c_gpio_cfgpin(S3C64XX_GPC(7), S3C_GPIO_SFN(5));
-		s3c_gpio_cfgpin_range(S3C64XX_GPH(6), 4, S3C_GPIO_SFN(5));
+		WARN_ON(s3c_gpio_cfgpin(S3C64XX_GPC(4), S3C_GPIO_SFN(5)));
+		WARN_ON(s3c_gpio_cfgpin(S3C64XX_GPC(5), S3C_GPIO_SFN(5)));
+		WARN_ON(s3c_gpio_cfgpin(S3C64XX_GPC(7), S3C_GPIO_SFN(5)));
+		WARN_ON(s3c_gpio_cfgpin_range(S3C64XX_GPH(6), 4, S3C_GPIO_SFN(5)));
 		return 0;
 	default:
 		printk(KERN_DEBUG "Invalid I2S Controller number: %d\n",
@@ -47,7 +47,7 @@ static int s3c64xx_i2s_cfg_gpio(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	s3c_gpio_cfgpin_range(base, 5, S3C_GPIO_SFN(3));
+	WARN_ON(s3c_gpio_cfgpin_range(base, 5, S3C_GPIO_SFN(3)));
 
 	return 0;
 }
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH RESEND RFC] mach:-s3c64xx:Output trace information with WARN_ON if calls for setting up gpio board configuration fail in s3c64xx_i2s_cfg_gpio
  2016-02-02 14:56 ` Nicholas Krause
@ 2016-02-03  1:33   ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 4+ messages in thread
From: Krzysztof Kozlowski @ 2016-02-03  1:33 UTC (permalink / raw)
  To: Nicholas Krause, kgene
  Cc: linux, linux-arm-kernel, linux-samsung-soc, linux-kernel

On 02.02.2016 23:56, Nicholas Krause wrote:
> This fixes the function s3c64xx_i2c_cfg_gpio to log output to the
> kernel log buff with WARN_ON if any of the calls to either
> s3c_gpio_cfgpin_range or s3c_gpio_cfgpin fail as we cannot exit
> from s3c64xx_i2s_cfg_gpio if any of these calls fail due to
> other intended function work being required to complete. Further more
> if a failure occurs allow the other users/devolopers of this driver
> to properly check the kernel log buffers for a kernel trace for
> when these calls failing to execute successfully arise.

Quite complicated sentences, very difficult to understand. The commit
message should describe the logic behind the change (e.g. answer to
why?) and user-visible impact in a brief and understandable way.

Commit subject does not match subsystem and it is over complicated on
its own. It is also too long. Your editor should point this already (70
chars at most).

Anyway I don't feel the patch is needed and description fails to
convince me. Knowing your history, you did not encounter any errors here
but instead you are just spreading random "fixes" which compile or not...

Since this was not tested:
If it ain't broken, don't fix it.

Best regards,
Krzysztof

> 
> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
> ---
> v3:Fix more wording issues and uncaught build failure with
> previous build test
> v2:Fix Patch Wording as it was clearly confusing and incorrect
>  arch/arm/mach-s3c64xx/dev-audio.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/mach-s3c64xx/dev-audio.c b/arch/arm/mach-s3c64xx/dev-audio.c
> index ff780a8..796ac75 100644
> --- a/arch/arm/mach-s3c64xx/dev-audio.c
> +++ b/arch/arm/mach-s3c64xx/dev-audio.c
> @@ -36,10 +36,10 @@ static int s3c64xx_i2s_cfg_gpio(struct platform_device *pdev)
>  		base = S3C64XX_GPE(0);
>  		break;
>  	case 2:
> -		s3c_gpio_cfgpin(S3C64XX_GPC(4), S3C_GPIO_SFN(5));
> -		s3c_gpio_cfgpin(S3C64XX_GPC(5), S3C_GPIO_SFN(5));
> -		s3c_gpio_cfgpin(S3C64XX_GPC(7), S3C_GPIO_SFN(5));
> -		s3c_gpio_cfgpin_range(S3C64XX_GPH(6), 4, S3C_GPIO_SFN(5));
> +		WARN_ON(s3c_gpio_cfgpin(S3C64XX_GPC(4), S3C_GPIO_SFN(5)));
> +		WARN_ON(s3c_gpio_cfgpin(S3C64XX_GPC(5), S3C_GPIO_SFN(5)));
> +		WARN_ON(s3c_gpio_cfgpin(S3C64XX_GPC(7), S3C_GPIO_SFN(5)));
> +		WARN_ON(s3c_gpio_cfgpin_range(S3C64XX_GPH(6), 4, S3C_GPIO_SFN(5)));
>  		return 0;
>  	default:
>  		printk(KERN_DEBUG "Invalid I2S Controller number: %d\n",
> @@ -47,7 +47,7 @@ static int s3c64xx_i2s_cfg_gpio(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> -	s3c_gpio_cfgpin_range(base, 5, S3C_GPIO_SFN(3));
> +	WARN_ON(s3c_gpio_cfgpin_range(base, 5, S3C_GPIO_SFN(3)));
>  
>  	return 0;
>  }
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread
* [PATCH RESEND RFC] mach:-s3c64xx:Output trace information with WARN_ON if calls for setting up gpio board configuration fail in s3c64xx_i2s_cfg_gpio
@ 2016-02-03  1:33   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 4+ messages in thread
From: Krzysztof Kozlowski @ 2016-02-03  1:33 UTC (permalink / raw)
  To: linux-arm-kernel

On 02.02.2016 23:56, Nicholas Krause wrote:
> This fixes the function s3c64xx_i2c_cfg_gpio to log output to the
> kernel log buff with WARN_ON if any of the calls to either
> s3c_gpio_cfgpin_range or s3c_gpio_cfgpin fail as we cannot exit
> from s3c64xx_i2s_cfg_gpio if any of these calls fail due to
> other intended function work being required to complete. Further more
> if a failure occurs allow the other users/devolopers of this driver
> to properly check the kernel log buffers for a kernel trace for
> when these calls failing to execute successfully arise.

Quite complicated sentences, very difficult to understand. The commit
message should describe the logic behind the change (e.g. answer to
why?) and user-visible impact in a brief and understandable way.

Commit subject does not match subsystem and it is over complicated on
its own. It is also too long. Your editor should point this already (70
chars at most).

Anyway I don't feel the patch is needed and description fails to
convince me. Knowing your history, you did not encounter any errors here
but instead you are just spreading random "fixes" which compile or not...

Since this was not tested:
If it ain't broken, don't fix it.

Best regards,
Krzysztof

> 
> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
> ---
> v3:Fix more wording issues and uncaught build failure with
> previous build test
> v2:Fix Patch Wording as it was clearly confusing and incorrect
>  arch/arm/mach-s3c64xx/dev-audio.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/mach-s3c64xx/dev-audio.c b/arch/arm/mach-s3c64xx/dev-audio.c
> index ff780a8..796ac75 100644
> --- a/arch/arm/mach-s3c64xx/dev-audio.c
> +++ b/arch/arm/mach-s3c64xx/dev-audio.c
> @@ -36,10 +36,10 @@ static int s3c64xx_i2s_cfg_gpio(struct platform_device *pdev)
>  		base = S3C64XX_GPE(0);
>  		break;
>  	case 2:
> -		s3c_gpio_cfgpin(S3C64XX_GPC(4), S3C_GPIO_SFN(5));
> -		s3c_gpio_cfgpin(S3C64XX_GPC(5), S3C_GPIO_SFN(5));
> -		s3c_gpio_cfgpin(S3C64XX_GPC(7), S3C_GPIO_SFN(5));
> -		s3c_gpio_cfgpin_range(S3C64XX_GPH(6), 4, S3C_GPIO_SFN(5));
> +		WARN_ON(s3c_gpio_cfgpin(S3C64XX_GPC(4), S3C_GPIO_SFN(5)));
> +		WARN_ON(s3c_gpio_cfgpin(S3C64XX_GPC(5), S3C_GPIO_SFN(5)));
> +		WARN_ON(s3c_gpio_cfgpin(S3C64XX_GPC(7), S3C_GPIO_SFN(5)));
> +		WARN_ON(s3c_gpio_cfgpin_range(S3C64XX_GPH(6), 4, S3C_GPIO_SFN(5)));
>  		return 0;
>  	default:
>  		printk(KERN_DEBUG "Invalid I2S Controller number: %d\n",
> @@ -47,7 +47,7 @@ static int s3c64xx_i2s_cfg_gpio(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> -	s3c_gpio_cfgpin_range(base, 5, S3C_GPIO_SFN(3));
> +	WARN_ON(s3c_gpio_cfgpin_range(base, 5, S3C_GPIO_SFN(3)));
>  
>  	return 0;
>  }
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-02-03  1:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-02 14:56 [PATCH RESEND RFC] mach:-s3c64xx:Output trace information with WARN_ON if calls for setting up gpio board configuration fail in s3c64xx_i2s_cfg_gpio Nicholas Krause
2016-02-02 14:56 ` Nicholas Krause
2016-02-03  1:33 ` Krzysztof Kozlowski
2016-02-03  1:33   ` Krzysztof Kozlowski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.