staging: media: atomisp: fix indentation in aa, anr, and bh modules
Subsystems (4)
PATCH v10 1/1 staging: media: atomisp: fix indentation in aa, anr, and bh modules
Fix tab/space indentation and move a standalone kernel-doc comment of the 'strength' field of the struct ia_css_aa_config to the whole-structure one.
Align with kernel coding style guidelines.
No functional logic changes were made.
---
This patch focuses on cleaning up indentation (spaces, tabs) in the AtomISP driver under drivers/staging/media/atomisp/pci/isp/kernels/.
There is no functional logic change.
---
Changes in v10:
- Squashed previous aa/anr/bh patches into a single patch
- Rewrote commit message to reflect actual changes
- Removed unnecessary Suggested-by and Link tags as advised
- Added trailing comma to array initialization as suggested
- Link to v9: https://lore.kernel.org/r/[email protected]
Changes in v9:
- Reformatted struct comments to follow kernel-doc format
- Reverted macro definition changes that removed alignment
- Reverted multi-line array initializations to use brace-on-own-line style
- Added trailing comma in small array initializations
- Adjusted function declarations to follow kernel-style indentation
- Removed unnecessary churn in cases where code was already style-compliant
- Link to v8: https://lore.kernel.org/r/[email protected]
Changes in v8:
- Reorganized cleanup by subdirectory (one commit per directory)
- Focused only on indentation fixes (spaces, tabs)
- Removed all clang-format involvement
- No functional changes
- Link to v7: https://lore.kernel.org/all/[email protected]/
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/[email protected]
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/[email protected]
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/[email protected]
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/[email protected]
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/[email protected]
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/[email protected]
Thanks for your previous feedback.
Best regards,
LiangCheng Wang <[email protected]>
--- .../pci/isp/kernels/aa/aa_2/ia_css_aa2_types.h | 8 +++++--- .../pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c | 24 ++++++++-------------- .../pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.h | 20 +++++++----------- .../pci/isp/kernels/anr/anr_2/ia_css_anr2.host.c | 14 +++++-------- .../pci/isp/kernels/anr/anr_2/ia_css_anr2.host.h | 13 +++++------- .../pci/isp/kernels/bh/bh_2/ia_css_bh.host.c | 14 +++++-------- .../pci/isp/kernels/bh/bh_2/ia_css_bh.host.h | 14 +++++-------- 7 files changed, 41 insertions(+), 66 deletions(-)
drivers/staging/media/atomisp/pci/isp/kernels/aa/aa_2/ia_css_aa2_types.h +5 -3
@@ -28,11 +28,13 @@* ISP block: BAA2 * ISP1: BAA2 is used. * ISP2: BAA2 is used. +* +* @strength: Strength of the filter, in u0.13 fixed-point format. +* Valid range: [0, 8191]. A value of 0 means the filter is +* ineffective (default). */ struct ia_css_aa_config { -u16 strength;/** Strength of the filter. -u0.13, [0,8191], -default/ineffective 0 */ +u16 strength; };#endif /* __IA_CSS_AA2_TYPES_H */
drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c +9 -15
@@ -16,25 +16,21 @@ const struct ia_css_anr_config default_anr_config = { 0, 3, 1, 2, 3, 6, 4, 5, 1, 4, 2, 3, 2, 5, 3, 4, 0, 3, 1, 2, 3, 6, 4, 5, 1, 4, 2, 3, 2, 5, 3, 4, 0, 3, 1, 2, 3, 6, 4, 5, 1, 4, 2, 3, 2, 5, 3, 4, - 0, 3, 1, 2, 3, 6, 4, 5, 1, 4, 2, 3, 2, 5, 3, 4 + 0, 3, 1, 2, 3, 6, 4, 5, 1, 4, 2, 3, 2, 5, 3, 4, }, - {10, 20, 30} + { 10, 20, 30 }, }; -void -ia_css_anr_encode ( -struct sh_css_isp_anr_params *to, -const struct ia_css_anr_config *from, -unsigned int size) +void ia_css_anr_encode (struct sh_css_isp_anr_params *to, +const struct ia_css_anr_config *from, +unsigned int size) { (void )size; to->threshold = from->threshold; } -void -ia_css_anr_dump ( -const struct sh_css_isp_anr_params *anr, -unsigned int level) +void ia_css_anr_dump (const struct sh_css_isp_anr_params *anr, +unsigned int level) {if (!anr)return ;ia_css_debug_dtrace (level,"Advance Noise Reduction:\n" );
@@ -42,10 +38,8 @@ ia_css_anr_dump( "anr_threshold", anr->threshold); } -void -ia_css_anr_debug_dtrace ( -const struct ia_css_anr_config *config, -unsigned int level) +void ia_css_anr_debug_dtrace (const struct ia_css_anr_config *config, +unsigned int level) { ia_css_debug_dtrace(level, "config.threshold=%d\n",
drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.h +7 -13
@@ -12,20 +12,14 @@extern const struct ia_css_anr_config default_anr_config; -void -ia_css_anr_encode ( -struct sh_css_isp_anr_params *to, -const struct ia_css_anr_config *from, -unsigned int size); +void ia_css_anr_encode (struct sh_css_isp_anr_params *to, +const struct ia_css_anr_config *from, +unsigned int size); -void -ia_css_anr_dump ( -const struct sh_css_isp_anr_params *anr, -unsigned int level); +void ia_css_anr_dump (const struct sh_css_isp_anr_params *anr, +unsigned int level); -void -ia_css_anr_debug_dtrace ( -const struct ia_css_anr_config *config,unsigned int level) -; +void ia_css_anr_debug_dtrace (const struct ia_css_anr_config *config, +unsigned int level);#endif /* __IA_CSS_ANR_HOST_H */
drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_2/ia_css_anr2.host.c +5 -9
@@ -10,11 +10,9 @@#include "ia_css_anr2.host.h" -void -ia_css_anr2_vmem_encode ( -struct ia_css_isp_anr2_params *to, -const struct ia_css_anr_thres *from, -size_t size) +void ia_css_anr2_vmem_encode (struct ia_css_isp_anr2_params *to, +const struct ia_css_anr_thres *from, +size_t size) {unsigned int i;
@@ -28,10 +26,8 @@ ia_css_anr2_vmem_encode( } } -void -ia_css_anr2_debug_dtrace ( -const struct ia_css_anr_thres *config, -unsigned int level) +void ia_css_anr2_debug_dtrace (const struct ia_css_anr_thres *config, +unsigned int level) { (void )config; (void )level;
drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_2/ia_css_anr2.host.h +5 -8
@@ -13,15 +13,12 @@#include "ia_css_anr2_param.h" #include "ia_css_anr2_table.host.h" -void -ia_css_anr2_vmem_encode ( -struct ia_css_isp_anr2_params *to, -const struct ia_css_anr_thres *from, -size_t size); +void ia_css_anr2_vmem_encode (struct ia_css_isp_anr2_params *to, +const struct ia_css_anr_thres *from, +size_t size); -void -ia_css_anr2_debug_dtrace ( -const struct ia_css_anr_thres *config,unsigned int level) +void ia_css_anr2_debug_dtrace (const struct ia_css_anr_thres *config, +unsigned int level) ;#endif /* __IA_CSS_ANR2_HOST_H */
drivers/staging/media/atomisp/pci/isp/kernels/bh/bh_2/ia_css_bh.host.c +5 -9
@@ -12,10 +12,8 @@#include "ia_css_bh.host.h" -void -ia_css_bh_hmem_decode ( -struct ia_css_3a_rgby_output *out_ptr, -const struct ia_css_bh_table *hmem_buf) +void ia_css_bh_hmem_decode (struct ia_css_3a_rgby_output *out_ptr, +const struct ia_css_bh_table *hmem_buf) {int i;
@@ -37,11 +35,9 @@ ia_css_bh_hmem_decode( } } -void -ia_css_bh_encode ( -struct sh_css_isp_bh_params *to, -const struct ia_css_3a_config *from, -unsigned int size) +void ia_css_bh_encode (struct sh_css_isp_bh_params *to, +const struct ia_css_3a_config *from, +unsigned int size) { (void )size;/* coefficients to calculate Y */
drivers/staging/media/atomisp/pci/isp/kernels/bh/bh_2/ia_css_bh.host.h +5 -9
@@ -10,15 +10,11 @@#include "ia_css_bh_param.h" #include "s3a/s3a_1.0/ia_css_s3a_types.h" -void -ia_css_bh_hmem_decode ( -struct ia_css_3a_rgby_output *out_ptr, -const struct ia_css_bh_table *hmem_buf); +void ia_css_bh_hmem_decode (struct ia_css_3a_rgby_output *out_ptr, +const struct ia_css_bh_table *hmem_buf); -void -ia_css_bh_encode ( -struct sh_css_isp_bh_params *to, -const struct ia_css_3a_config *from, -unsigned int size); +void ia_css_bh_encode (struct sh_css_isp_bh_params *to, +const struct ia_css_3a_config *from, +unsigned int size);#endif /* __IA_CSS_BH_HOST_H */
---
base-commit: 8c2e52ebbe885c7eeaabd3b7ddcdc1246fc400d2
change-id: 20250704-new_atomisp-e73f471f3078
Best regards,
Signature · 1 line
LiangCheng Wang <[email protected]>
Quoted 4 lines · LiangCheng Wang
Fix tab/space indentation and move a standalone kernel-doc comment of the 'strength' field of the struct
ia_css_aa_configto the whole-structure one.
Align with kernel coding style guidelines.
There are too many changes all at once and some of the changes are not described in the commit message.
drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c const struct ia_css_anr_config default_anr_config = { · line 19
Quoted 10 lines
drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c +1
-1@@ -16,25 +16,21 @@ const struct ia_css_anr_config default_anr_config = {0 ,3 ,1 ,2 ,3 ,6 ,4 ,5 ,1 ,4 ,2 ,3 ,2 ,5 ,3 ,4 ,0 ,3 ,1 ,2 ,3 ,6 ,4 ,5 ,1 ,4 ,2 ,3 ,2 ,5 ,3 ,4 ,0 ,3 ,1 ,2 ,3 ,6 ,4 ,5 ,1 ,4 ,2 ,3 ,2 ,5 ,3 ,4 , -0 ,3 ,1 ,2 ,3 ,6 ,4 ,5 ,1 ,4 ,2 ,3 ,2 ,5 ,3 ,4 + 0, 3, 1, 2, 3, 6, 4, 5, 1, 4, 2, 3, 2, 5, 3, 4,
No need to add a comma to this line. The comma at the end of the line is useful when we might add another element to an array. But here the length is fixed.
If someone were to add a comma here and it was new code, then that's fine. But I don't want to have to review a separate patch which only adds a unnecessary comma.
drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c const struct ia_css_anr_config default_anr_config = { · line 21
Quoted 3 lines
drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c +1
-1}, - {10, 20, 30} + { 10, 20, 30 },
Same here. This comma serves no purpose. We can't actually add anything to this struct. What would be actually helpful would be to use designated initializers.
drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c +3 -3
@@ -11,14 +11,14 @@#include "ia_css_anr.host.h" const struct ia_css_anr_config default_anr_config = { -10 , - { + .threshold =10 , + .thresholds = {0 ,3 ,1 ,2 ,3 ,6 ,4 ,5 ,1 ,4 ,2 ,3 ,2 ,5 ,3 ,4 ,0 ,3 ,1 ,2 ,3 ,6 ,4 ,5 ,1 ,4 ,2 ,3 ,2 ,5 ,3 ,4 ,0 ,3 ,1 ,2 ,3 ,6 ,4 ,5 ,1 ,4 ,2 ,3 ,2 ,5 ,3 ,4 ,0 ,3 ,1 ,2 ,3 ,6 ,4 ,5 ,1 ,4 ,2 ,3 ,2 ,5 ,3 ,4 }, - {10 ,20 ,30 } + .factors = {10 ,20 ,30 }, };void
I added a comma to the end of .factors because there is a 1% change we will add a new member to the struct and it's the right thing to do. I was already changing that line, so I'm allowed to make tiny white space changes like this.
But notice how I left off the comma after the numbers. That array is a fixed size and nothing can be added. Leaving off the comma communicates that. Also there was no need to change that line. It's unrelated to using desgnated initializers. If you added a comma, you would need to send a separate patch for that with a commit message to describe and justify it. As a reviewer, I would need to go through the line carefully and verify that none of the other numbers had been changed.
The commit message for the above patch would say, "Use a designated initializer for default_anr_config. It helps readability." There would be no need to mention that "I added a comma" to the end of the .factors line because it's a minor thing that we're not really stressed about.
regards,
dan carpenter
Quoted 5 lines · Dan Carpenter
Quoted 4 lines · LiangCheng Wang
Fix tab/space indentation and move a standalone kernel-doc comment of the 'strength' field of the struct
ia_css_aa_configto the whole-structure one.
Align with kernel coding style guidelines.
...
Quoted 9 lines
drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c
Quoted 5 lines
drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c +1
-10 ,3 ,1 ,2 ,3 ,6 ,4 ,5 ,1 ,4 ,2 ,3 ,2 ,5 ,3 ,4 ,0 ,3 ,1 ,2 ,3 ,6 ,4 ,5 ,1 ,4 ,2 ,3 ,2 ,5 ,3 ,4 ,0 ,3 ,1 ,2 ,3 ,6 ,4 ,5 ,1 ,4 ,2 ,3 ,2 ,5 ,3 ,4 , -0 ,3 ,1 ,2 ,3 ,6 ,4 ,5 ,1 ,4 ,2 ,3 ,2 ,5 ,3 ,4 + 0, 3, 1, 2, 3, 6, 4, 5, 1, 4, 2, 3, 2, 5, 3, 4,No need to add a comma to this line. The comma at the end of the line is useful when we might add another element to an array. But here the length is fixed.
Still, it's good to have it to avoid any additional churn in case it might be extended. We can argue if this needs to be a separate commit from the main topic of this patch.
Quoted 11 lines
If someone were to add a comma here and it was new code, then that's fine. But I don't want to have to review a separate patch which only adds a unnecessary comma.
drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c
Quoted 3 lines
drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c +1
-1}, - {10, 20, 30} + { 10, 20, 30 },Same here. This comma serves no purpose. We can't actually add anything to this struct. What would be actually helpful would be to use designated initializers.
Here we touched the line, and adding trailing comma just reduces a potential churn in the future. I can show you plenty of changes when patch touches unrelated line just for the sake of adding a new one after the affected.
...
Quoted 6 lines
drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c +0
-0@@ -11,14 +11,14 @@#include "ia_css_anr.host.h"
drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c line 19
Quoted 10 lines
drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c +2
-2const struct ia_css_anr_config default_anr_config = { -10 , - { + .threshold =10 , + .thresholds = {0 ,3 ,1 ,2 ,3 ,6 ,4 ,5 ,1 ,4 ,2 ,3 ,2 ,5 ,3 ,4 ,0 ,3 ,1 ,2 ,3 ,6 ,4 ,5 ,1 ,4 ,2 ,3 ,2 ,5 ,3 ,4 ,0 ,3 ,1 ,2 ,3 ,6 ,4 ,5 ,1 ,4 ,2 ,3 ,2 ,5 ,3 ,4 ,0 ,3 ,1 ,2 ,3 ,6 ,4 ,5 ,1 ,4 ,2 ,3 ,2 ,5 ,3 ,4
With the trailing comma it will be better for the consistency in this case. Otherwise I like your approach.
Quoted 24 lines
drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c +1
-1}, - {10, 20, 30} + .factors = {10, 20, 30}, }; voidI added a comma to the end of .factors because there is a 1% change we will add a new member to the struct and it's the right thing to do. I was already changing that line, so I'm allowed to make tiny white space changes like this.
But notice how I left off the comma after the numbers. That array is a fixed size and nothing can be added. Leaving off the comma communicates that. Also there was no need to change that line. It's unrelated to using desgnated initializers. If you added a comma, you would need to send a separate patch for that with a commit message to describe and justify it. As a reviewer, I would need to go through the line carefully and verify that none of the other numbers had been changed.
The commit message for the above patch would say, "Use a designated initializer for
default_anr_config. It helps readability." There would be no need to mention that "I added a comma" to the end of the .factors line because it's a minor thing that we're not really stressed about.
Signature · 2 lines
With Best Regards, Andy Shevchenko