Message-ID: <CAKEtaPAr6g5bGFwPJZXRHN6p9u5SrUo3kQi6Zcpp7hbFCp-J2A@mail.gmail.com>
Date: Sun, 29 Jun 2025 21:02:42 +0800
From: 王良丞 <zaq14760@...il.com>
To: Hans de Goede <hansg@...nel.org>
Cc: 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, 
	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

Dear Hans and Andy,

Thanks for the feedback. I'll wait for Andy's opinion on this before
taking further actions.

Meanwhile, I understand Hans' concerns about clang-format changes and
I can prepare manual cleanup patches as suggested if needed.

Best regards,

LiangCheng Wang

Hans de Goede <hansg@...nel.org> 於 2025年6月29日 週日 下午8:20寫道：
>
> 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