linux-kernel - Re: [PATCH v13 0/2] Support for Osram as3668 LED driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20260108104130.GC302752@google.com>
Date: Thu, 8 Jan 2026 10:41:30 +0000
From: Lee Jones <lee@...nel.org>
To: Lukas Timmermann <linux@...mermann.space>
Cc: pavel@...nel.org, robh@...nel.org, krzk+dt@...nel.org,
	conor+dt@...nel.org, linux-leds@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v13 0/2] Support for Osram as3668 LED driver

On Tue, 02 Dec 2025, Lukas Timmermann wrote:

Running through checkpatch.pl
total: 0 errors, 0 warnings, 86 lines checked

"[PATCH v13 1/2] dt-bindings: leds: Add new as3668 support" has no obvious style problems and is ready for submission.
ERROR: Macros with complex values should be enclosed in parentheses
#115: FILE: drivers/leds/leds-as3668.c:35:
+#define AS3668_CURR_MODE_PACK(mode)	((mode) << 0) | \
+					((mode) << 2) | \
+					((mode) << 4) | \
+					((mode) << 6)

BUT SEE:

   do {} while (0) advice is over-stated in a few situations:

   The more obvious case is macros, like MODULE_PARM_DESC, invoked at
   file-scope, where C disallows code (it must be in functions).  See
   $exceptions if you have one to add by name.

   More troublesome is declarative macros used at top of new scope,
   like DECLARE_PER_CPU.  These might just compile with a do-while-0
   wrapper, but would be incorrect.  Most of these are handled by
   detecting struct,union,etc declaration primitives in $exceptions.

   Theres also macros called inside an if (block), which "return" an
   expression.  These cannot do-while, and need a ({}) wrapper.

   Enjoy this qualification while we work to improve our heuristics.

WARNING: please, no spaces at the start of a line
#135: FILE: drivers/leds/leds-as3668.c:55:
+    int ret;$

WARNING: please, no spaces at the start of a line
#136: FILE: drivers/leds/leds-as3668.c:56:
+    u8 channel_modes;$

WARNING: please, no spaces at the start of a line
#138: FILE: drivers/leds/leds-as3668.c:58:
+    ret = i2c_smbus_read_byte_data(led->chip->client, AS3668_CURR_MODE_REG);$

WARNING: please, no spaces at the start of a line
#139: FILE: drivers/leds/leds-as3668.c:59:
+    if (ret < 0) {$

ERROR: code indent should use tabs where possible
#140: FILE: drivers/leds/leds-as3668.c:60:
+        dev_err(led->cdev.dev, "failed to read channel modes\n");$

WARNING: please, no spaces at the start of a line
#140: FILE: drivers/leds/leds-as3668.c:60:
+        dev_err(led->cdev.dev, "failed to read channel modes\n");$

ERROR: code indent should use tabs where possible
#141: FILE: drivers/leds/leds-as3668.c:61:
+        return ret;$

WARNING: please, no spaces at the start of a line
#141: FILE: drivers/leds/leds-as3668.c:61:
+        return ret;$

WARNING: please, no spaces at the start of a line
#142: FILE: drivers/leds/leds-as3668.c:62:
+    }$

WARNING: please, no spaces at the start of a line
#143: FILE: drivers/leds/leds-as3668.c:63:
+    channel_modes = (u8)ret;$

WARNING: please, no spaces at the start of a line
#145: FILE: drivers/leds/leds-as3668.c:65:
+    channel_modes &= ~led->mode_mask;$

WARNING: please, no spaces at the start of a line
#146: FILE: drivers/leds/leds-as3668.c:66:
+    channel_modes |= led->mode_mask & (AS3668_CURR_MODE_PACK(mode));$

WARNING: please, no spaces at the start of a line
#148: FILE: drivers/leds/leds-as3668.c:68:
+    return i2c_smbus_write_byte_data(led->chip->client, AS3668_CURR_MODE_REG, channel_modes);$

WARNING: line length of 104 exceeds 100 columns
#247: FILE: drivers/leds/leds-as3668.c:167:
+		return dev_err_probe(&client->dev, -EIO, "failed to set zero initial current levels\n");

WARNING: DT compatible string "ams,as3668" appears un-documented -- check ./Documentation/devicetree/bindings/
#264: FILE: drivers/leds/leds-as3668.c:184:
+	{ .compatible = "ams,as3668" },

total: 3 errors, 13 warnings, 235 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

NOTE: Whitespace errors detected.
      You may wish to use scripts/cleanpatch or scripts/cleanfile

"[PATCH v13 2/2] leds: as3668: Driver for the ams Osram 4-channel" has style problems, please review.

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

-- 
Lee Jones [李琼斯]