From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932421AbcBCBdp (ORCPT ); Tue, 2 Feb 2016 20:33:45 -0500 Received: from mailout2.w1.samsung.com ([210.118.77.12]:42827 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754992AbcBCBdn (ORCPT ); Tue, 2 Feb 2016 20:33:43 -0500 X-AuditID: cbfec7f5-f79b16d000005389-3f-56b158f40899 Subject: 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 To: Nicholas Krause , kgene@kernel.org References: <1454424992-14619-1-git-send-email-xerofoify@gmail.com> Cc: linux@arm.linux.org.uk, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org From: Krzysztof Kozlowski X-Enigmail-Draft-Status: N1110 Message-id: <56B158ED.8090109@samsung.com> Date: Wed, 03 Feb 2016 10:33:33 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-version: 1.0 In-reply-to: <1454424992-14619-1-git-send-email-xerofoify@gmail.com> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrJLMWRmVeSWpSXmKPExsVy+t/xK7pfIjaGGZxcLWXx+oWhRf/j18wW mx5fY7W4vGsOm8WM8/uYLG5f5rX4NUXWgd2jpbmHzWPnrLvsHptWdbJ5bF5S79G3ZRWjx+dN cgFsUVw2Kak5mWWpRfp2CVwZF+f9Yyp4KVrRtbibsYGxU6iLkZNDQsBEon3faXYIW0ziwr31 bF2MXBxCAksZJZ5tngrlPGWU+HV0GiuIIwyS2brsNSNIi4iAtcT8+zfBbCEBZ4k9l1YzgRQx CzQzSnRfvAE2l03AWGLz8iVsEDvkJHq7J7GA2LwCWhJzZ7wCa2YRUJWYdO0+K4gtKhAhcbiz ix2iRlDix+R7YPWcAi4SP15+AprDAbRAT+L+RS2QMLOAvMTmNW+ZJzAKzkLSMQuhahaSqgWM zKsYRVNLkwuKk9JzjfSKE3OLS/PS9ZLzczcxQiLg6w7GpcesDjEKcDAq8fBG/NoQJsSaWFZc mXuIUYKDWUmEt9JjY5gQb0piZVVqUX58UWlOavEhRmkOFiVx3pm73ocICaQnlqRmp6YWpBbB ZJk4OKUaGGe4BHMttTr+NMD4y6zaE/wHHs1iXWcUcDW5WGfW2ivcQTHXXYLeFJ11mr97xjbj FG05L4crX1f53mxzUmKZ8Pv87s5/YbaZHOcNOf0y1bdWe+uLRsfp1BU+qL877e2dX4n6n1iP TjIXPPFpWcpBz4avzec7reTZbjN5ev9oM1ruHpgl4/FFS4mlOCPRUIu5qDgRAPaGB4V8AgAA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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; > } >