[PATCH] platform/x86: intel-vbtn: Fix code style issues

[PATCH] platform/x86: intel-vbtn: Fix code style issues

[Xiang Shen]

Fix checkpatch code style errors:

ERROR: do not use assignment in if condition
+	if ((ke = sparse_keymap_entry_from_scancode(priv->buttons_dev, event))) {

ERROR: do not use assignment in if condition
+	} else if ((ke = sparse_keymap_entry_from_scancode(priv->switches_dev, event))) {

Signed-off-by: Xiang Shen <turyshen@gmail.com>
---
 drivers/platform/x86/intel/vbtn.c | 38 +++++++++++++++++--------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/drivers/platform/x86/intel/vbtn.c b/drivers/platform/x86/intel/vbtn.c
index 232cd12e3c9f..bcc97b06844e 100644
--- a/drivers/platform/x86/intel/vbtn.c
+++ b/drivers/platform/x86/intel/vbtn.c
@@ -160,30 +160,34 @@ static void notify_handler(acpi_handle handle, u32 event, void *context)
 
 	guard(mutex)(&priv->mutex);
 
-	if ((ke = sparse_keymap_entry_from_scancode(priv->buttons_dev, event))) {
+	ke = sparse_keymap_entry_from_scancode(priv->buttons_dev, event);
+	if (ke) {
 		if (!priv->has_buttons) {
 			dev_warn(&device->dev, "Warning: received 0x%02x button event on a device without buttons, please report this.\n",
 				 event);
 			return;
 		}
 		input_dev = priv->buttons_dev;
-	} else if ((ke = sparse_keymap_entry_from_scancode(priv->switches_dev, event))) {
-		if (!priv->has_switches) {
-			/* See dual_accel_detect.h for more info */
-			if (priv->dual_accel)
-				return;
-
-			dev_info(&device->dev, "Registering Intel Virtual Switches input-dev after receiving a switch event\n");
-			ret = input_register_device(priv->switches_dev);
-			if (ret)
-				return;
-
-			priv->has_switches = true;
-		}
-		input_dev = priv->switches_dev;
 	} else {
-		dev_dbg(&device->dev, "unknown event index 0x%x\n", event);
-		return;
+		ke = sparse_keymap_entry_from_scancode(priv->switches_dev, event);
+		if (ke) {
+			if (!priv->has_switches) {
+				/* See dual_accel_detect.h for more info */
+				if (priv->dual_accel)
+					return;
+
+				dev_info(&device->dev, "Registering Intel Virtual Switches input-dev after receiving a switch event\n");
+				ret = input_register_device(priv->switches_dev);
+				if (ret)
+					return;
+
+				priv->has_switches = true;
+			}
+			input_dev = priv->switches_dev;
+		} else {
+			dev_dbg(&device->dev, "unknown event index 0x%x\n", event);
+			return;
+		}
 	}
 
 	if (priv->wakeup_mode) {
-- 
2.46.0

Re: [PATCH] platform/x86: intel-vbtn: Fix code style issues

[Hans de Goede]

Hi,

On 20-Jun-25 2:38 AM, Xiang Shen wrote:
> Fix checkpatch code style errors:
> 
> ERROR: do not use assignment in if condition
> +	if ((ke = sparse_keymap_entry_from_scancode(priv->buttons_dev, event))) {
> 
> ERROR: do not use assignment in if condition
> +	} else if ((ke = sparse_keymap_entry_from_scancode(priv->switches_dev, event))) {
> 
> Signed-off-by: Xiang Shen <turyshen@gmail.com>

Thank you for your patch, but this change really does not make
the code more readable.

The contrary the suggested changes are making the code harder
to read, so NACK.

Note checkpatch is just a tool, sometimes there are good reasons
to deviate from the style checks done by checkpatch.

Next time when submitting a patch to fix checkpatch issues please
take a look at the resulting code after the patch and only submit
the patch upstream if it actually is an improvement.

Regards,

Hans



> ---
>  drivers/platform/x86/intel/vbtn.c | 38 +++++++++++++++++--------------
>  1 file changed, 21 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/vbtn.c b/drivers/platform/x86/intel/vbtn.c
> index 232cd12e3c9f..bcc97b06844e 100644
> --- a/drivers/platform/x86/intel/vbtn.c
> +++ b/drivers/platform/x86/intel/vbtn.c
> @@ -160,30 +160,34 @@ static void notify_handler(acpi_handle handle, u32 event, void *context)
>  
>  	guard(mutex)(&priv->mutex);
>  
> -	if ((ke = sparse_keymap_entry_from_scancode(priv->buttons_dev, event))) {
> +	ke = sparse_keymap_entry_from_scancode(priv->buttons_dev, event);
> +	if (ke) {
>  		if (!priv->has_buttons) {
>  			dev_warn(&device->dev, "Warning: received 0x%02x button event on a device without buttons, please report this.\n",
>  				 event);
>  			return;
>  		}
>  		input_dev = priv->buttons_dev;
> -	} else if ((ke = sparse_keymap_entry_from_scancode(priv->switches_dev, event))) {
> -		if (!priv->has_switches) {
> -			/* See dual_accel_detect.h for more info */
> -			if (priv->dual_accel)
> -				return;
> -
> -			dev_info(&device->dev, "Registering Intel Virtual Switches input-dev after receiving a switch event\n");
> -			ret = input_register_device(priv->switches_dev);
> -			if (ret)
> -				return;
> -
> -			priv->has_switches = true;
> -		}
> -		input_dev = priv->switches_dev;
>  	} else {
> -		dev_dbg(&device->dev, "unknown event index 0x%x\n", event);
> -		return;
> +		ke = sparse_keymap_entry_from_scancode(priv->switches_dev, event);
> +		if (ke) {
> +			if (!priv->has_switches) {
> +				/* See dual_accel_detect.h for more info */
> +				if (priv->dual_accel)
> +					return;
> +
> +				dev_info(&device->dev, "Registering Intel Virtual Switches input-dev after receiving a switch event\n");
> +				ret = input_register_device(priv->switches_dev);
> +				if (ret)
> +					return;
> +
> +				priv->has_switches = true;
> +			}
> +			input_dev = priv->switches_dev;
> +		} else {
> +			dev_dbg(&device->dev, "unknown event index 0x%x\n", event);
> +			return;
> +		}
>  	}
>  
>  	if (priv->wakeup_mode) {

Re: [PATCH] platform/x86: intel-vbtn: Fix code style issues

[Xiang Shen]

On Fri, Jun 20, 2025 at 12:00:03PM +1000, Hans de Goede wrote:
> Hi,
> 
> On 20-Jun-25 2:38 AM, Xiang Shen wrote:
> > Fix checkpatch code style errors:
> > 
> > ERROR: do not use assignment in if condition
> > +	if ((ke = sparse_keymap_entry_from_scancode(priv->buttons_dev, event))) {
> > 
> > ERROR: do not use assignment in if condition
> > +	} else if ((ke = sparse_keymap_entry_from_scancode(priv->switches_dev, event))) {
> > 
> > Signed-off-by: Xiang Shen <turyshen@gmail.com>
> 
> Thank you for your patch, but this change really does not make
> the code more readable.
> 
> The contrary the suggested changes are making the code harder
> to read, so NACK.
> 
> Note checkpatch is just a tool, sometimes there are good reasons
> to deviate from the style checks done by checkpatch.
> 
> Next time when submitting a patch to fix checkpatch issues please
> take a look at the resulting code after the patch and only submit
> the patch upstream if it actually is an improvement.
> 
> Regards,
> 
> Hans
> 
Hi Hans,

Thanks for the feedback. 

That's fine if breaking the "rule" is the only way to keep the file readable.

However, there are only three files (x86/sony-laptop.c and x86/dell/dell_rbu.c) out of 273 files in the whole drivers/platform folder that have such an error.
Perhaps there are other approaches to make them more readable without breaking the rule.


BRs,
Xiang

> 
> 
> > ---
> >  drivers/platform/x86/intel/vbtn.c | 38 +++++++++++++++++--------------
> >  1 file changed, 21 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/intel/vbtn.c b/drivers/platform/x86/intel/vbtn.c
> > index 232cd12e3c9f..bcc97b06844e 100644
> > --- a/drivers/platform/x86/intel/vbtn.c
> > +++ b/drivers/platform/x86/intel/vbtn.c
> > @@ -160,30 +160,34 @@ static void notify_handler(acpi_handle handle, u32 event, void *context)
> >  
> >  	guard(mutex)(&priv->mutex);
> >  
> > -	if ((ke = sparse_keymap_entry_from_scancode(priv->buttons_dev, event))) {
> > +	ke = sparse_keymap_entry_from_scancode(priv->buttons_dev, event);
> > +	if (ke) {
> >  		if (!priv->has_buttons) {
> >  			dev_warn(&device->dev, "Warning: received 0x%02x button event on a device without buttons, please report this.\n",
> >  				 event);
> >  			return;
> >  		}
> >  		input_dev = priv->buttons_dev;
> > -	} else if ((ke = sparse_keymap_entry_from_scancode(priv->switches_dev, event))) {
> > -		if (!priv->has_switches) {
> > -			/* See dual_accel_detect.h for more info */
> > -			if (priv->dual_accel)
> > -				return;
> > -
> > -			dev_info(&device->dev, "Registering Intel Virtual Switches input-dev after receiving a switch event\n");
> > -			ret = input_register_device(priv->switches_dev);
> > -			if (ret)
> > -				return;
> > -
> > -			priv->has_switches = true;
> > -		}
> > -		input_dev = priv->switches_dev;
> >  	} else {
> > -		dev_dbg(&device->dev, "unknown event index 0x%x\n", event);
> > -		return;
> > +		ke = sparse_keymap_entry_from_scancode(priv->switches_dev, event);
> > +		if (ke) {
> > +			if (!priv->has_switches) {
> > +				/* See dual_accel_detect.h for more info */
> > +				if (priv->dual_accel)
> > +					return;
> > +
> > +				dev_info(&device->dev, "Registering Intel Virtual Switches input-dev after receiving a switch event\n");
> > +				ret = input_register_device(priv->switches_dev);
> > +				if (ret)
> > +					return;
> > +
> > +				priv->has_switches = true;
> > +			}
> > +			input_dev = priv->switches_dev;
> > +		} else {
> > +			dev_dbg(&device->dev, "unknown event index 0x%x\n", event);
> > +			return;
> > +		}
> >  	}
> >  
> >  	if (priv->wakeup_mode) {
> 

Re: [PATCH] platform/x86: intel-vbtn: Fix code style issues

[Ilpo Järvinen]

On Sun, 22 Jun 2025, Xiang Shen wrote:
> On Fri, Jun 20, 2025 at 12:00:03PM +1000, Hans de Goede wrote:
> > On 20-Jun-25 2:38 AM, Xiang Shen wrote:
> > > Fix checkpatch code style errors:
> > > 
> > > ERROR: do not use assignment in if condition
> > > +	if ((ke = sparse_keymap_entry_from_scancode(priv->buttons_dev, event))) {
> > > 
> > > ERROR: do not use assignment in if condition
> > > +	} else if ((ke = sparse_keymap_entry_from_scancode(priv->switches_dev, event))) {
> > > 
> > > Signed-off-by: Xiang Shen <turyshen@gmail.com>
> > 
> > Thank you for your patch, but this change really does not make
> > the code more readable.
> > 
> > The contrary the suggested changes are making the code harder
> > to read, so NACK.
> > 
> > Note checkpatch is just a tool, sometimes there are good reasons
> > to deviate from the style checks done by checkpatch.
> > 
> > Next time when submitting a patch to fix checkpatch issues please
> > take a look at the resulting code after the patch and only submit
> > the patch upstream if it actually is an improvement.
> > 
> > Regards,
> > 
> > Hans
> > 
> Hi Hans,
> 
> Thanks for the feedback. 
> 
> That's fine if breaking the "rule" is the only way to keep the file readable.
> 
> However, there are only three files (x86/sony-laptop.c and 
> x86/dell/dell_rbu.c) out of 273 files in the whole drivers/platform 
> folder that have such an error. 

Hi,

Please don't call correct code "error" even if checkpatch may label it as 
such. The goal is NOT and will never be to have zero checkpatch warnings.

The fact that the checkpatch "rule" is broken only a few times does not 
mean those 3 places have a problem, it just tells it's good rule for the
general case. So I won't accept using such numbers as a leverage against 
the few places just for the sake of silencing checkpatch.

> Perhaps there are other approaches to make them more readable without 
> breaking the rule.

Perhaps, but I'm not sure the effort spent to find one is worthwhile 
investment.

> > > ---
> > >  drivers/platform/x86/intel/vbtn.c | 38 +++++++++++++++++--------------
> > >  1 file changed, 21 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/drivers/platform/x86/intel/vbtn.c b/drivers/platform/x86/intel/vbtn.c
> > > index 232cd12e3c9f..bcc97b06844e 100644
> > > --- a/drivers/platform/x86/intel/vbtn.c
> > > +++ b/drivers/platform/x86/intel/vbtn.c
> > > @@ -160,30 +160,34 @@ static void notify_handler(acpi_handle handle, u32 event, void *context)
> > >  
> > >  	guard(mutex)(&priv->mutex);
> > >  
> > > -	if ((ke = sparse_keymap_entry_from_scancode(priv->buttons_dev, event))) {
> > > +	ke = sparse_keymap_entry_from_scancode(priv->buttons_dev, event);
> > > +	if (ke) {
> > >  		if (!priv->has_buttons) {
> > >  			dev_warn(&device->dev, "Warning: received 0x%02x button event on a device without buttons, please report this.\n",
> > >  				 event);
> > >  			return;
> > >  		}
> > >  		input_dev = priv->buttons_dev;
> > > -	} else if ((ke = sparse_keymap_entry_from_scancode(priv->switches_dev, event))) {
> > > -		if (!priv->has_switches) {
> > > -			/* See dual_accel_detect.h for more info */
> > > -			if (priv->dual_accel)
> > > -				return;
> > > -
> > > -			dev_info(&device->dev, "Registering Intel Virtual Switches input-dev after receiving a switch event\n");
> > > -			ret = input_register_device(priv->switches_dev);
> > > -			if (ret)
> > > -				return;
> > > -
> > > -			priv->has_switches = true;
> > > -		}
> > > -		input_dev = priv->switches_dev;
> > >  	} else {
> > > -		dev_dbg(&device->dev, "unknown event index 0x%x\n", event);
> > > -		return;
> > > +		ke = sparse_keymap_entry_from_scancode(priv->switches_dev, event);
> > > +		if (ke) {
> > > +			if (!priv->has_switches) {
> > > +				/* See dual_accel_detect.h for more info */
> > > +				if (priv->dual_accel)
> > > +					return;
> > > +
> > > +				dev_info(&device->dev, "Registering Intel Virtual Switches input-dev after receiving a switch event\n");
> > > +				ret = input_register_device(priv->switches_dev);
> > > +				if (ret)
> > > +					return;
> > > +
> > > +				priv->has_switches = true;
> > > +			}
> > > +			input_dev = priv->switches_dev;
> > > +		} else {
> > > +			dev_dbg(&device->dev, "unknown event index 0x%x\n", event);
> > > +			return;
> > > +		}
> > >  	}
> > >  
> > >  	if (priv->wakeup_mode) {
> > 
> 

-- 
 i.

Re: [PATCH] platform/x86: intel-vbtn: Fix code style issues

[Xiang Shen]

On Tue, Jun 24, 2025 at 01:35:57PM +1000, Ilpo Järvinen wrote:
>On Sun, 22 Jun 2025, Xiang Shen wrote:
>> On Fri, Jun 20, 2025 at 12:00:03PM +1000, Hans de Goede wrote:
>> > On 20-Jun-25 2:38 AM, Xiang Shen wrote:
>> > > Fix checkpatch code style errors:
>> > >
>> > > ERROR: do not use assignment in if condition
>> > > +	if ((ke = sparse_keymap_entry_from_scancode(priv->buttons_dev, event))) {
>> > >
>> > > ERROR: do not use assignment in if condition
>> > > +	} else if ((ke = sparse_keymap_entry_from_scancode(priv->switches_dev, event))) {
>> > >
>> > > Signed-off-by: Xiang Shen <turyshen@gmail.com>
>> >
>> > Thank you for your patch, but this change really does not make
>> > the code more readable.
>> >
>> > The contrary the suggested changes are making the code harder
>> > to read, so NACK.
>> >
>> > Note checkpatch is just a tool, sometimes there are good reasons
>> > to deviate from the style checks done by checkpatch.
>> >
>> > Next time when submitting a patch to fix checkpatch issues please
>> > take a look at the resulting code after the patch and only submit
>> > the patch upstream if it actually is an improvement.
>> >
>> > Regards,
>> >
>> > Hans
>> >
>> Hi Hans,
>>
>> Thanks for the feedback.
>>
>> That's fine if breaking the "rule" is the only way to keep the file readable.
>>
>> However, there are only three files (x86/sony-laptop.c and
>> x86/dell/dell_rbu.c) out of 273 files in the whole drivers/platform
>> folder that have such an error.
>
>Hi,
>
>Please don't call correct code "error" even if checkpatch may label it as
>such. The goal is NOT and will never be to have zero checkpatch warnings.
>
>The fact that the checkpatch "rule" is broken only a few times does not
>mean those 3 places have a problem, it just tells it's good rule for the
>general case. So I won't accept using such numbers as a leverage against
>the few places just for the sake of silencing checkpatch.
>

Hi,

I just thought there must be a reason that the checkpatch categories 
findings as "ERROR", "WARNING" and "CHECK".

Sometimes the number does make sense and means the vast majority
follow the widely accepted "rule".

Curiously, isn't it the contributor's due diligence to pass checkpatch
in the first place before sending? 
Should any objection, submit a patch to checkpatch itself,
instead of sneaking into the upstream quietly for the sake of "readability".

>> Perhaps there are other approaches to make them more readable without
>> breaking the rule.
>
>Perhaps, but I'm not sure the effort spent to find one is worthwhile
>investment.
>

Indeed, that's precisely why it might be worth sacrificing a bit of "readability".

Re: [PATCH] platform/x86: intel-vbtn: Fix code style issues

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 4098 bytes --]

On Wed, 25 Jun 2025, Xiang Shen wrote:

> On Tue, Jun 24, 2025 at 01:35:57PM +1000, Ilpo Järvinen wrote:
> > On Sun, 22 Jun 2025, Xiang Shen wrote:
> > > On Fri, Jun 20, 2025 at 12:00:03PM +1000, Hans de Goede wrote:
> > > > On 20-Jun-25 2:38 AM, Xiang Shen wrote:
> > > > > Fix checkpatch code style errors:
> > > > >
> > > > > ERROR: do not use assignment in if condition
> > > > > +	if ((ke = sparse_keymap_entry_from_scancode(priv->buttons_dev,
> > > event))) {
> > > > >
> > > > > ERROR: do not use assignment in if condition
> > > > > +	} else if ((ke =
> > > sparse_keymap_entry_from_scancode(priv->switches_dev, event))) {
> > > > >
> > > > > Signed-off-by: Xiang Shen <turyshen@gmail.com>
> > > >
> > > > Thank you for your patch, but this change really does not make
> > > > the code more readable.
> > > >
> > > > The contrary the suggested changes are making the code harder
> > > > to read, so NACK.
> > > >
> > > > Note checkpatch is just a tool, sometimes there are good reasons
> > > > to deviate from the style checks done by checkpatch.
> > > >
> > > > Next time when submitting a patch to fix checkpatch issues please
> > > > take a look at the resulting code after the patch and only submit
> > > > the patch upstream if it actually is an improvement.
> > > >
> > > > Regards,
> > > >
> > > > Hans
> > > >
> > > Hi Hans,
> > > 
> > > Thanks for the feedback.
> > > 
> > > That's fine if breaking the "rule" is the only way to keep the file
> > > readable.
> > > 
> > > However, there are only three files (x86/sony-laptop.c and
> > > x86/dell/dell_rbu.c) out of 273 files in the whole drivers/platform
> > > folder that have such an error.
> > 
> > Hi,
> > 
> > Please don't call correct code "error" even if checkpatch may label it as
> > such. The goal is NOT and will never be to have zero checkpatch warnings.
> > 
> > The fact that the checkpatch "rule" is broken only a few times does not
> > mean those 3 places have a problem, it just tells it's good rule for the
> > general case. So I won't accept using such numbers as a leverage against
> > the few places just for the sake of silencing checkpatch.
> > 
>
> I just thought there must be a reason that the checkpatch categories findings
> as "ERROR", "WARNING" and "CHECK".

The checkpatch change submitter just picked one of the levels. They're 
humans too. :-)

> Sometimes the number does make sense and means the vast majority
> follow the widely accepted "rule".
>
> Curiously, isn't it the contributor's due diligence to pass checkpatch
> in the first place before sending?

Hey, that code you're changing is not being submitted. We don't want to 
waste our time on "fixing" checkpatch warnings on existing code when the 
"fix" results in worse readability that before.

Also, if you would be submitting a patch and checkpatch suggest you 
to make the patch worse, please don't follow checkpatch's advice even in 
that case!

Checker tools are there to help find potential issues, not rulebooks. This 
is often overlooked by many checker tool focused developers and results in 
low quality patches (there are exceptions too where they developer really 
tries to understand all the related code to see if the change makes sense 
/ improves things or not).

> Should any objection, submit a patch to
> checkpatch itself,
> instead of sneaking into the upstream quietly for the sake of "readability".
>
> > > Perhaps there are other approaches to make them more readable without
> > > breaking the rule.
> > 
> > Perhaps, but I'm not sure the effort spent to find one is worthwhile
> > investment.
> > 
> 
> Indeed, that's precisely why it might be worth sacrificing a bit of
> "readability".

Please stop trying to sneak your subpar change in.

NO, this a hard NO from the acting maintainer. We'll not be accepting 
misguided changes that are to silence ANY checker that should have know 
better (those checkers will never learn enough to be relied 100% so 
please stop spreading such illusion).


-- 
 i.

Re: [PATCH] platform/x86: intel-vbtn: Fix code style issues

[Xiang Shen]

On Wed, Jun 25, 2025 at 03:25:52PM +1000, Ilpo Järvinen wrote:
>On Wed, 25 Jun 2025, Xiang Shen wrote:
>
>> On Tue, Jun 24, 2025 at 01:35:57PM +1000, Ilpo Järvinen wrote:
>> > On Sun, 22 Jun 2025, Xiang Shen wrote:
>> > > On Fri, Jun 20, 2025 at 12:00:03PM +1000, Hans de Goede wrote:
>> > > > On 20-Jun-25 2:38 AM, Xiang Shen wrote:
>> > > > > Fix checkpatch code style errors:
>> > > > >
>> > > > > ERROR: do not use assignment in if condition
>> > > > > +	if ((ke = sparse_keymap_entry_from_scancode(priv->buttons_dev,
>> > > event))) {
>> > > > >
>> > > > > ERROR: do not use assignment in if condition
>> > > > > +	} else if ((ke =
>> > > sparse_keymap_entry_from_scancode(priv->switches_dev, event))) {
>> > > > >
>> > > > > Signed-off-by: Xiang Shen <turyshen@gmail.com>
>> > > >
>> > > > Thank you for your patch, but this change really does not make
>> > > > the code more readable.
>> > > >
>> > > > The contrary the suggested changes are making the code harder
>> > > > to read, so NACK.
>> > > >
>> > > > Note checkpatch is just a tool, sometimes there are good reasons
>> > > > to deviate from the style checks done by checkpatch.
>> > > >
>> > > > Next time when submitting a patch to fix checkpatch issues please
>> > > > take a look at the resulting code after the patch and only submit
>> > > > the patch upstream if it actually is an improvement.
>> > > >
>> > > > Regards,
>> > > >
>> > > > Hans
>> > > >
>> > > Hi Hans,
>> > >
>> > > Thanks for the feedback.
>> > >
>> > > That's fine if breaking the "rule" is the only way to keep the file
>> > > readable.
>> > >
>> > > However, there are only three files (x86/sony-laptop.c and
>> > > x86/dell/dell_rbu.c) out of 273 files in the whole drivers/platform
>> > > folder that have such an error.
>> >
>> > Hi,
>> >
>> > Please don't call correct code "error" even if checkpatch may label it as
>> > such. The goal is NOT and will never be to have zero checkpatch warnings.
>> >
>> > The fact that the checkpatch "rule" is broken only a few times does not
>> > mean those 3 places have a problem, it just tells it's good rule for the
>> > general case. So I won't accept using such numbers as a leverage against
>> > the few places just for the sake of silencing checkpatch.
>> >
>>
>> I just thought there must be a reason that the checkpatch categories findings
>> as "ERROR", "WARNING" and "CHECK".
>
>The checkpatch change submitter just picked one of the levels. They're
>humans too. :-)
>
>> Sometimes the number does make sense and means the vast majority
>> follow the widely accepted "rule".
>>
>> Curiously, isn't it the contributor's due diligence to pass checkpatch
>> in the first place before sending?
>
>Hey, that code you're changing is not being submitted. We don't want to
>waste our time on "fixing" checkpatch warnings on existing code when the
>"fix" results in worse readability that before.
>
>Also, if you would be submitting a patch and checkpatch suggest you
>to make the patch worse, please don't follow checkpatch's advice even in
>that case!
>
>Checker tools are there to help find potential issues, not rulebooks. This
>is often overlooked by many checker tool focused developers and results in
>low quality patches (there are exceptions too where they developer really
>tries to understand all the related code to see if the change makes sense
>/ improves things or not).
>

Hi Ilpo,

I've learned a lot — thanks for the thorough explanation rather than
just brushing me off.

The rookies are certainly more likely than the veterans to treat
the "rulebooks" as ironclad disciplines.

>> Should any objection, submit a patch to
>> checkpatch itself,
>> instead of sneaking into the upstream quietly for the sake of "readability".
>>
>> > > Perhaps there are other approaches to make them more readable without
>> > > breaking the rule.
>> >
>> > Perhaps, but I'm not sure the effort spent to find one is worthwhile
>> > investment.
>> >
>>
>> Indeed, that's precisely why it might be worth sacrificing a bit of
>> "readability".
>
>Please stop trying to sneak your subpar change in.
>
>NO, this a hard NO from the acting maintainer. We'll not be accepting
>misguided changes that are to silence ANY checker that should have know
>better (those checkers will never learn enough to be relied 100% so
>please stop spreading such illusion).
>

I wish I had known those checking errors were intentionally ignored.
I'm putting a full stop here.