Received-SPF: pass (google.com: domain of linux-kernel+bounces-19347-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: "sandeep.cs" <sandeep.cs@samsung.com>
To: "'Joe Perches'" <joe@perches.com>, "'Jiri Kosina'" <jikos@kernel.org>,
	"'Benjamin Tissoires'" <benjamin.tissoires@redhat.com>
Cc: <gaudium.lee@samsung.com>, <ih0923.kim@samsung.com>,
	<suhyun_.kim@samsung.com>, <jitender.s21@samsung.com>,
	<junwan.cho@samsung.com>, <linux-input@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
In-Reply-To: <486973921f89f70bcc5d42501eeca3fd105be2c4.camel@perches.com>
Subject: RE: [HID Patchsets for Samsung driver v2 2/6] HID: Samsung : Fix
 the checkpatch complain.
Date: Mon, 8 Jan 2024 16:14:58 +0530
Message-ID: <020e01da421f$c0d20660$42761320$@samsung.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Thread-Index: AQGnAMfWRo+zFj79MMHZQSbz6pOJPAGgO/sIAfyMtNQBptQ1H7EMH2NA
Content-Language: en-in
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 105P
DLP-Filter: Pass
References: <20240108091917.1552013-1-sandeep.cs@samsung.com>
	<CGME20240108091959epcas5p2559b779424e2fb7c7e268d1b24612b4f@epcas5p2.samsung.com>
	<20240108091917.1552013-3-sandeep.cs@samsung.com>
	<486973921f89f70bcc5d42501eeca3fd105be2c4.camel@perches.com>

>On Mon, 2024-01-08 at 14:49 +0530, Sandeep C S wrote:
>> Warning found by checkpatch.pl script.
>[]
>> diff --git a/drivers/hid/hid-samsung.c b/drivers/hid/hid-samsung.c
>[]
>> @@ -67,20 +67,17 @@ static __u8 *samsung_irda_report_fixup(struct
>hid_device *hdev, __u8 *rdesc,
>>  		rdesc[178] = 0x08;
>>  		rdesc[180] = 0x06;
>>  		rdesc[182] = 0x42;
>> -	} else
>> -	if (*rsize == 203 && rdesc[192] == 0x15 && rdesc[193] == 0x0 &&
>> +	} else if (*rsize == 203 && rdesc[192] == 0x15 && rdesc[193] == 0x0
>> +&&
>>  			rdesc[194] == 0x25 && rdesc[195] == 0x12) {
>>  		samsung_irda_dev_trace(hdev, 203);
>>  		rdesc[193] = 0x1;
>>  		rdesc[195] = 0xf;
>> -	} else
>> -	if (*rsize == 135 && rdesc[124] == 0x15 && rdesc[125] == 0x0 &&
>> +	} else if (*rsize == 135 && rdesc[124] == 0x15 && rdesc[125] == 0x0
>> +&&
>>  			rdesc[126] == 0x25 && rdesc[127] == 0x11) {
>>  		samsung_irda_dev_trace(hdev, 135);
>>  		rdesc[125] = 0x1;
>>  		rdesc[127] = 0xe;
>> -	} else
>> -	if (*rsize == 171 && rdesc[160] == 0x15 && rdesc[161] == 0x0 &&
>> +	} else if (*rsize == 171 && rdesc[160] == 0x15 && rdesc[161] == 0x0
>> +&&
>>  			rdesc[162] == 0x25 && rdesc[163] == 0x01) {
>>  		samsung_irda_dev_trace(hdev, 171);
>>  		rdesc[161] = 0x1;
>
>For this block, I think a rewrite using memcmp would be clearer.
>Something like: 
Okay . Thanks for your valuable feedback. We will promptly address your
suggestions and enhance our code accordingly.
And also please review other patch set as well.
Thank you
>---
> drivers/hid/hid-samsung.c | 34 +++++++++++++++-------------------
> 1 file changed, 15 insertions(+), 19 deletions(-)
>
>diff --git a/drivers/hid/hid-samsung.c b/drivers/hid/hid-samsung.c index
>cf5992e970940..cd84fb5e68f69 100644
>--- a/drivers/hid/hid-samsung.c
>+++ b/drivers/hid/hid-samsung.c
>@@ -58,33 +58,29 @@ static inline void samsung_irda_dev_trace(struct
>hid_device *hdev,  static __u8 *samsung_irda_report_fixup(struct hid_device
>*hdev, __u8 *rdesc,
> 		unsigned int *rsize)
> {
>-	if (*rsize == 184 && rdesc[175] == 0x25 && rdesc[176] == 0x40 &&
>-			rdesc[177] == 0x75 && rdesc[178] == 0x30 &&
>-			rdesc[179] == 0x95 && rdesc[180] == 0x01 &&
>-			rdesc[182] == 0x40) {
>+	if (*rsize == 184 &&
>+	    !memcmp(&rdesc[175], "\x25\x40\x75\x30\x95\x01", 6) &&
>+	    rdesc[182] == 0x40) {
> 		samsung_irda_dev_trace(hdev, 184);
> 		rdesc[176] = 0xff;
> 		rdesc[178] = 0x08;
> 		rdesc[180] = 0x06;
> 		rdesc[182] = 0x42;
>-	} else
>-	if (*rsize == 203 && rdesc[192] == 0x15 && rdesc[193] == 0x0 &&
>-			rdesc[194] == 0x25 && rdesc[195] == 0x12) {
>+	} else if (*rsize == 203 &&
>+		   !memcmp(&rdesc[192], "\x15\x00\x25\x12", 4)) {
> 		samsung_irda_dev_trace(hdev, 203);
>-		rdesc[193] = 0x1;
>-		rdesc[195] = 0xf;
>-	} else
>-	if (*rsize == 135 && rdesc[124] == 0x15 && rdesc[125] == 0x0 &&
>-			rdesc[126] == 0x25 && rdesc[127] == 0x11) {
>+		rdesc[193] = 0x01;
>+		rdesc[195] = 0x0f;
>+	} else if (*rsize == 135 &&
>+		   !memcmp(&rdesc[124], "\x15\x00\x25\x11", 4)) {
> 		samsung_irda_dev_trace(hdev, 135);
>-		rdesc[125] = 0x1;
>-		rdesc[127] = 0xe;
>-	} else
>-	if (*rsize == 171 && rdesc[160] == 0x15 && rdesc[161] == 0x0 &&
>-			rdesc[162] == 0x25 && rdesc[163] == 0x01) {
>+		rdesc[125] = 0x01;
>+		rdesc[127] = 0x0e;
>+	} else if (*rsize == 171 &&
>+		   !memcmp(&rdesc[160], "\x15\x00\x25\x01", 4)) {
> 		samsung_irda_dev_trace(hdev, 171);
>-		rdesc[161] = 0x1;
>-		rdesc[163] = 0x3;
>+		rdesc[161] = 0x01;
>+		rdesc[163] = 0x03;
> 	}
> 	return rdesc;
> }