Message-ID: <092f5109-ca31-4949-bda8-7e0d946c3aa0@kernel.org>
Date: Sun, 29 Jun 2025 14:20:19 +0200
From: Hans de Goede <hansg@...nel.org>
To: LiangCheng Wang <zaq14760@...il.com>, mchehab@...nel.org,
 sakari.ailus@...ux.intel.com, andy@...nel.org, gregkh@...uxfoundation.org,
 nathan@...nel.org, nick.desaulniers+lkml@...il.com, morbo@...gle.com,
 justinstitt@...gle.com
Cc: linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-staging@...ts.linux.dev, llvm@...ts.linux.dev
Subject: Re: [PATCH v7] staging: media: atomisp: code style cleanup series

Hi,

On 29-Jun-25 1:30 PM, LiangCheng Wang wrote:
> This series applies clang-format and fixes all checkpatch.pl-reported ERRORs in the AtomISP driver, excluding the i2c directory as advised by maintainers.
> 
> The changes include:
> - Applying clang-format (excluding drivers/staging/media/i2c)
> - Removing unnecessary parentheses in return statements
> - Removing unnecessary zero-initialized globals
> - Fixing space issues after unary minus operators
> - Wrapping complex macro values in parentheses
> - These patches focus solely on mechanical style cleanups with no functional changes.
> - WARNINGs reported by checkpatch.pl were intentionally left for future work to keep each patch clear and manageable.
> 
> The full series and corresponding commits are also available in my public Git repository:
> 
> https://github.com/lc-wang/linux/tree/b4/atomisp
> 
> To: Hans de Goede <hansg@...nel.org>
> To: Mauro Carvalho Chehab <mchehab@...nel.org>
> To: Sakari Ailus <sakari.ailus@...ux.intel.com>
> To: Andy Shevchenko <andy@...nel.org>
> To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> To: Nathan Chancellor <nathan@...nel.org>
> To: Nick Desaulniers <nick.desaulniers+lkml@...il.com>
> To: Bill Wendling <morbo@...gle.com>
> To: Justin Stitt <justinstitt@...gle.com>
> Cc: linux-media@...r.kernel.org
> Cc: linux-kernel@...r.kernel.org
> Cc: linux-staging@...ts.linux.dev
> Cc: llvm@...ts.linux.dev
> ---
> Changes in v7:
> - Split previous monolithic patch into multiple smaller patches
> - Applied clang-format to entire driver excluding i2c directory

I took a quick look at just the clang-format patch and looking
at the bits of the diff which were not collapsed by github because
the changes are too big, it looks like the changes which clang-format
makes are useless and often make things worse, e.g. just looking
at the first diff which github shows for:

https://github.com/lc-wang/linux/commit/8a3bbdba275e42dfcb0af2ddcc2f27463bb316d2

which is for drivers/staging/media/atomisp/include/hmm/hmm.h
then all of the changes are undesirable and unneeded.

so the running of clang-format just seems to make things worse.

I appreciate coding-style cleanups outside of the i2c dir,
but it looks like you need to do everything manually since
clang-format is just making a mess of things.

Also if you do manual code-style cleanups please do one
type of cleanup per patches, e.g. only fix indentation
using spaces instead of tabs and do so on groups of say
10 files at a time to keep things reviewable.

Regards,

Hans



> - Fixed checkpatch.pl-reported ERRORs (parentheses in macros, unnecessary return parentheses, zero-initialized globals, spaces after unary minus)
> - Left WARNINGS untouched for future cleanup
> - No functional logic changes
> - Link to v6: https://lore.kernel.org/r/20250627-bar-v6-1-b22b5ea3ced0@gmail.com
> 
> Changes in v6:
> - Applied clang-format across the entire AtomISP driver
> - Fixed all checkpatch.pl-reported ERRORs
> - Added explanation of tooling and scope
> - No functional logic modified
> - Moved 'Suggested-by' and 'Link' tags above Signed-off-by
> - Link to v5: https://lore.kernel.org/r/20250625-bar-v5-1-db960608b607@gmail.com
> 
> Changes in v5:
> - Replaced space-based indentation with tabs in output_1.0 directory
> - Used checkpatch.pl and grep to identify formatting issues
> - No functional changes made
> - This patch is now focused solely on tab/space issues
> - Link to v4: https://lore.kernel.org/r/20250624-bar-v4-1-9f9f9ae9f868@gmail.com
> 
> Changes in v4:
> - Moved assignment operator '=' to the same line for static struct definitions
> - Remove unnecessary line breaks in function definitions
> - Update commit message to reflect all the coding style fixes
> - Link to v3: https://lore.kernel.org/r/20250622-bar-v3-1-4cc91ef01c3a@gmail.com
> 
> Changes in v3:
> - Removed extra spaces between type and asterisk (e.g., `*to`) in function
>   declarations, as pointed out by Andy Shevchenko
> - Update commit message to reflect all the coding style fixes
> - Link to v2: https://lore.kernel.org/r/20250621-bar-v2-1-4e6cfc779614@gmail.com
> 
> Changes in v2:
> - Fix patch subject prefix to "staging: media: atomisp:" to comply with media CI style.
> - No other functional changes.
> 
> Link to v1: https://lore.kernel.org/r/20250621-bar-v1-1-5a3e7004462c@gmail.com
> 
> --- b4-submit-tracking ---
> # This section is used internally by b4 prep for tracking purposes.
> {
>   "series": {
>     "revision": 7,
>     "change-id": "20250621-bar-573b8b40fb80",
>     "prefixes": [],
>     "history": {
>       "v1": [
>         "20250621-bar-v1-1-5a3e7004462c@...il.com"
>       ],
>       "v2": [
>         "20250621-bar-v2-1-4e6cfc779614@...il.com"
>       ],
>       "v3": [
>         "20250622-bar-v3-1-4cc91ef01c3a@...il.com"
>       ],
>       "v4": [
>         "20250624-bar-v4-1-9f9f9ae9f868@...il.com"
>       ],
>       "v5": [
>         "20250625-bar-v5-1-db960608b607@...il.com"
>       ],
>       "v6": [
>         "20250627-bar-v6-1-b22b5ea3ced0@...il.com"
>       ]
>     }
>   }
> }

Powered by blists - more mailing lists