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

[Lukas Timmermann <linux@timmermann.space>]

On Thu, Jan 08, 2026 at 10:41:30AM +0000, Lee Jones wrote:
> 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 [李琼斯]
> 
Acknowledged. That shouldn't have happened. I apologize.
I had no time to fix this issue since. I will try to make that happen in
the next days.

Best regards,
Lukas Timmermann