* [PATCH v10] staging: media: atomisp: fix indentation in aa, anr, and bh modules
@ 2025-07-18 15:02 LiangCheng Wang
2025-07-18 16:06 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: LiangCheng Wang @ 2025-07-18 15:02 UTC (permalink / raw)
To: Andy Shevchenko, Hans de Goede, Mauro Carvalho Chehab,
Sakari Ailus, Greg Kroah-Hartman, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt
Cc: linux-media, linux-kernel, linux-staging, llvm, LiangCheng Wang
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.
Signed-off-by: LiangCheng Wang <zaq14760@gmail.com>
---
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/20250711-new_atomisp-v9-0-a9dd62425ef6@gmail.com
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/20250704-new_atomisp-v8-0-2a8560cbd9be@gmail.com
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/20250629113050.58138-1-zaq14760@gmail.com/
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
Thanks for your previous feedback.
Best regards,
LiangCheng Wang <zaq14760@gmail.com>
Signed-off-by: LiangCheng Wang <zaq14760@gmail.com>
---
.../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(-)
diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/aa/aa_2/ia_css_aa2_types.h b/drivers/staging/media/atomisp/pci/isp/kernels/aa/aa_2/ia_css_aa2_types.h
index 2f568a7062da726397f55b1e73dadd27fcd1f2f8..f825f537a5366f6b97170dd44827436fd800d105 100644
--- a/drivers/staging/media/atomisp/pci/isp/kernels/aa/aa_2/ia_css_aa2_types.h
+++ b/drivers/staging/media/atomisp/pci/isp/kernels/aa/aa_2/ia_css_aa2_types.h
@@ -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 */
diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c
index 899d566234b9d3a35401666dcf0c7b1b80fd5b31..488807a161b9a6ba9ebc4a557221cd21bd1df108 100644
--- a/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c
+++ b/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c
@@ -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",
diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.h b/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.h
index 4f77900871c8b9d43bdc00308aa914eda3af7fa7..2f17d62b92087afd26aff2a49dadf4ee2fb7fcf5 100644
--- a/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.h
+++ b/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.h
@@ -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 */
diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_2/ia_css_anr2.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_2/ia_css_anr2.host.c
index 09599884bdaefe32f891f437274b96110888a675..3c6d99139cda6f12eb6ef6fe8f996f7ff11551c2 100644
--- a/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_2/ia_css_anr2.host.c
+++ b/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_2/ia_css_anr2.host.c
@@ -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;
diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_2/ia_css_anr2.host.h b/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_2/ia_css_anr2.host.h
index 2b1105f21c1e2b37144083b6bb8f9dd465a2f43b..36fb6c25969991ffcdd4424a695dacc5b5c2a9db 100644
--- a/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_2/ia_css_anr2.host.h
+++ b/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_2/ia_css_anr2.host.h
@@ -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 */
diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/bh/bh_2/ia_css_bh.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/bh/bh_2/ia_css_bh.host.c
index 69c87e53f3c22fade6c4c7914d1550f68dd8f5c2..b87eb1a21b216dc6132331d2a48d7cb56ddaea24 100644
--- a/drivers/staging/media/atomisp/pci/isp/kernels/bh/bh_2/ia_css_bh.host.c
+++ b/drivers/staging/media/atomisp/pci/isp/kernels/bh/bh_2/ia_css_bh.host.c
@@ -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 */
diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/bh/bh_2/ia_css_bh.host.h b/drivers/staging/media/atomisp/pci/isp/kernels/bh/bh_2/ia_css_bh.host.h
index 36b360cfe62e65037522e0037cf51fca0fabdfba..964d658ceec31b0b22fc3bc6d19b40b07b2fc06c 100644
--- a/drivers/staging/media/atomisp/pci/isp/kernels/bh/bh_2/ia_css_bh.host.h
+++ b/drivers/staging/media/atomisp/pci/isp/kernels/bh/bh_2/ia_css_bh.host.h
@@ -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,
--
LiangCheng Wang <zaq14760@gmail.com>
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v10] staging: media: atomisp: fix indentation in aa, anr, and bh modules
2025-07-18 15:02 [PATCH v10] staging: media: atomisp: fix indentation in aa, anr, and bh modules LiangCheng Wang
@ 2025-07-18 16:06 ` Dan Carpenter
2025-07-21 17:29 ` Andy Shevchenko
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2025-07-18 16:06 UTC (permalink / raw)
To: LiangCheng Wang
Cc: Andy Shevchenko, Hans de Goede, Mauro Carvalho Chehab,
Sakari Ailus, Greg Kroah-Hartman, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, linux-media,
linux-kernel, linux-staging, llvm
On Fri, Jul 18, 2025 at 11:02:14PM +0800, LiangCheng Wang wrote:
> 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.
There are too many changes all at once and some of the changes are not
described in the commit message.
> diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c
> index 899d566234b9d3a35401666dcf0c7b1b80fd5b31..488807a161b9a6ba9ebc4a557221cd21bd1df108 100644
> --- a/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c
> +++ b/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c
> @@ -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.
> },
> - {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.
diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c
index 899d566234b9..3de7ebea3d6e 100644
--- a/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c
+++ b/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c
@@ -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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v10] staging: media: atomisp: fix indentation in aa, anr, and bh modules
2025-07-18 16:06 ` Dan Carpenter
@ 2025-07-21 17:29 ` Andy Shevchenko
0 siblings, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2025-07-21 17:29 UTC (permalink / raw)
To: Dan Carpenter
Cc: LiangCheng Wang, Andy Shevchenko, Hans de Goede,
Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
linux-media, linux-kernel, linux-staging, llvm
On Fri, Jul 18, 2025 at 07:06:10PM +0300, Dan Carpenter wrote:
> On Fri, Jul 18, 2025 at 11:02:14PM +0800, LiangCheng Wang wrote:
> > 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.
...
> > 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.
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.
> 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.
>
> > },
> > - {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.
...
> diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c
> index 899d566234b9..3de7ebea3d6e 100644
> --- a/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c
> +++ b/drivers/staging/media/atomisp/pci/isp/kernels/anr/anr_1.0/ia_css_anr.host.c
> @@ -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
With the trailing comma it will be better for the consistency in this case.
Otherwise I like your approach.
> },
> - {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.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-07-21 17:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-18 15:02 [PATCH v10] staging: media: atomisp: fix indentation in aa, anr, and bh modules LiangCheng Wang
2025-07-18 16:06 ` Dan Carpenter
2025-07-21 17:29 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).