[PATCH v7] staging: media: atomisp: code style cleanup series

[PATCH v7] staging: media: atomisp: code style cleanup series

[LiangCheng Wang]

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@kernel.org>
To: Mauro Carvalho Chehab <mchehab@kernel.org>
To: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Andy Shevchenko <andy@kernel.org>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Nathan Chancellor <nathan@kernel.org>
To: Nick Desaulniers <nick.desaulniers+lkml@gmail.com>
To: Bill Wendling <morbo@google.com>
To: Justin Stitt <justinstitt@google.com>
Cc: linux-media@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-staging@lists.linux.dev
Cc: llvm@lists.linux.dev
---
Changes in v7:
- Split previous monolithic patch into multiple smaller patches
- Applied clang-format to entire driver excluding i2c directory
- 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@gmail.com"
      ],
      "v2": [
        "20250621-bar-v2-1-4e6cfc779614@gmail.com"
      ],
      "v3": [
        "20250622-bar-v3-1-4cc91ef01c3a@gmail.com"
      ],
      "v4": [
        "20250624-bar-v4-1-9f9f9ae9f868@gmail.com"
      ],
      "v5": [
        "20250625-bar-v5-1-db960608b607@gmail.com"
      ],
      "v6": [
        "20250627-bar-v6-1-b22b5ea3ced0@gmail.com"
      ]
    }
  }
}
-- 
2.43.0

Re: [PATCH v7] staging: media: atomisp: code style cleanup series

[Hans de Goede]

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@kernel.org>
> To: Mauro Carvalho Chehab <mchehab@kernel.org>
> To: Sakari Ailus <sakari.ailus@linux.intel.com>
> To: Andy Shevchenko <andy@kernel.org>
> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> To: Nathan Chancellor <nathan@kernel.org>
> To: Nick Desaulniers <nick.desaulniers+lkml@gmail.com>
> To: Bill Wendling <morbo@google.com>
> To: Justin Stitt <justinstitt@google.com>
> Cc: linux-media@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-staging@lists.linux.dev
> Cc: llvm@lists.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@gmail.com"
>       ],
>       "v2": [
>         "20250621-bar-v2-1-4e6cfc779614@gmail.com"
>       ],
>       "v3": [
>         "20250622-bar-v3-1-4cc91ef01c3a@gmail.com"
>       ],
>       "v4": [
>         "20250624-bar-v4-1-9f9f9ae9f868@gmail.com"
>       ],
>       "v5": [
>         "20250625-bar-v5-1-db960608b607@gmail.com"
>       ],
>       "v6": [
>         "20250627-bar-v6-1-b22b5ea3ced0@gmail.com"
>       ]
>     }
>   }
> }

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@kernel.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@kernel.org>
> > To: Mauro Carvalho Chehab <mchehab@kernel.org>
> > To: Sakari Ailus <sakari.ailus@linux.intel.com>
> > To: Andy Shevchenko <andy@kernel.org>
> > To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > To: Nathan Chancellor <nathan@kernel.org>
> > To: Nick Desaulniers <nick.desaulniers+lkml@gmail.com>
> > To: Bill Wendling <morbo@google.com>
> > To: Justin Stitt <justinstitt@google.com>
> > Cc: linux-media@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linux-staging@lists.linux.dev
> > Cc: llvm@lists.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@gmail.com"
> >       ],
> >       "v2": [
> >         "20250621-bar-v2-1-4e6cfc779614@gmail.com"
> >       ],
> >       "v3": [
> >         "20250622-bar-v3-1-4cc91ef01c3a@gmail.com"
> >       ],
> >       "v4": [
> >         "20250624-bar-v4-1-9f9f9ae9f868@gmail.com"
> >       ],
> >       "v5": [
> >         "20250625-bar-v5-1-db960608b607@gmail.com"
> >       ],
> >       "v6": [
> >         "20250627-bar-v6-1-b22b5ea3ced0@gmail.com"
> >       ]
> >     }
> >   }
> > }
>

Re: [PATCH v7] staging: media: atomisp: code style cleanup series

[Andy Shevchenko]

On Sun, Jun 29, 2025 at 3:20 PM Hans de Goede <hansg@kernel.org> wrote:
> On 29-Jun-25 1:30 PM, LiangCheng Wang wrote:

...

> > 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.

Yeah, that what I was afraid of. I don't know clang-format well
enough, but it might be that tweaking existing .clang-format in the
kernel can give something better.

> 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.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7] staging: media: atomisp: code style cleanup series

[Andy Shevchenko]

On Sun, Jun 29, 2025 at 4:02 PM 王良丞 <zaq14760@gmail.com> wrote:
>
> Dear Hans and Andy,

First of all, please stop top-posting!

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

TBH I don't know what I can help with or should do. I expressed my
worries already in one of the previous emails where I suggested to
check clang-format and then when we see the first attempt of using it.

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

Or you can try (without sending each time the result) amending output
of clang-format, or you can do a set of scripts based on the problems
found... Take your time and experiment more, no need to send a new
version every couple of days or so.

> > 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

> > > 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.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7] staging: media: atomisp: code style cleanup series

[Miguel Ojeda]

On Sun, Jun 29, 2025 at 9:13 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> Yeah, that what I was afraid of. I don't know clang-format well
> enough, but it might be that tweaking existing .clang-format in the
> kernel can give something better.

Yeah, there are some proposed changes for the config that I have in
the backlog, and with a minimum upgrade we can also improve it further
(some things just couldn't be done well with the options that were
available at the time when I introduced it). That is why, at least so
far, clang-format worked best for formatting snippets or for new
files.

Cheers,
Miguel