[PATCH 2/2] Staging: media: atomisp: style corrections

[PATCH 2/2] Staging: media: atomisp: style corrections

[Thomas Andreatta]

Corrected consistent spacing around '*' and braces positions

Signed-off-by: Thomas Andreatta <thomas.andreatta2000@gmail.com>
---
 drivers/staging/media/atomisp/pci/sh_css.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index 5a8e8e67aa13..77c7245091bb 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -333,9 +333,9 @@ static struct sh_css_hmm_buffer_record
 				   enum ia_css_buffer_type type);
 
 static unsigned int get_crop_lines_for_bayer_order(const struct
-	ia_css_stream_config *config);
+	ia_css_stream_config * config);
 static unsigned int get_crop_columns_for_bayer_order(const struct
-	ia_css_stream_config *config);
+	ia_css_stream_config * config);
 static void get_pipe_extra_pixel(struct ia_css_pipe *pipe,
 				 unsigned int *extra_row, unsigned int *extra_column);
 
@@ -2260,8 +2260,7 @@ alloc_continuous_frames(struct ia_css_pipe *pipe, bool init_time)
 		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
 				    "alloc_continuous_frames() IA_CSS_FRAME_FORMAT_RAW_PACKED\n");
 		ref_info.format = IA_CSS_FRAME_FORMAT_RAW_PACKED;
-	} else
-	{
+	} else {
 		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
 				    "alloc_continuous_frames() IA_CSS_FRAME_FORMAT_RAW\n");
 		ref_info.format = IA_CSS_FRAME_FORMAT_RAW;
@@ -7889,8 +7888,7 @@ ia_css_stream_create(const struct ia_css_stream_config *stream_config,
 
 	/* check if mipi size specified */
 	if (stream_config->mode == IA_CSS_INPUT_MODE_BUFFERED_SENSOR)
-		if (!IS_ISP2401 || !stream_config->online)
-		{
+		if (!IS_ISP2401 || !stream_config->online) {
 			unsigned int port = (unsigned int)stream_config->source.port.port;
 
 			if (port >= N_MIPI_PORT_ID) {
-- 
2.34.1

Re: [PATCH 2/2] Staging: media: atomisp: style corrections

[Andy Shevchenko]

On Sat, Apr 26, 2025 at 11:15 PM Thomas Andreatta
<thomasandreatta2000@gmail.com> wrote:
>
> Corrected consistent spacing around '*' and braces positions

Missed period.
And what is the correct spacing and why?

...

>  static unsigned int get_crop_lines_for_bayer_order(const struct
> -       ia_css_stream_config *config);
> +       ia_css_stream_config * config);
>  static unsigned int get_crop_columns_for_bayer_order(const struct
> -       ia_css_stream_config *config);
> +       ia_css_stream_config * config);

No, this makes it the opposite. Please, read Coding Style if it sheds
a light on this. In any case the kernel style is to avoid spacing
between asterisk and name.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] Staging: media: atomisp: style corrections

[Thomas Andreatta]

On Sun, Apr 27, 2025 at 12:41:34PM +0300, Andy Shevchenko wrote:
> On Sat, Apr 26, 2025 at 11:15 PM Thomas Andreatta
> <thomasandreatta2000@gmail.com> wrote:
> >
> > Corrected consistent spacing around '*' and braces positions
> 
> Missed period.
> And what is the correct spacing and why?

I agree that the spacing looks weird and I questioned it too, but the script
checkpatch.pl highlights as error:
`sh_css.c:336: ERROR: need consistent spacing around '*' (ctx:WxV)`
`sh_css.c:338: ERROR: need consistent spacing around '*' (ctx:WxV)`

Should this be ignored because the script tries its best and it becomes common
sense that the suggested spacing is "wrong"?

> 
> ...
> 
> >  static unsigned int get_crop_lines_for_bayer_order(const struct
> > -       ia_css_stream_config *config);
> > +       ia_css_stream_config * config);
> >  static unsigned int get_crop_columns_for_bayer_order(const struct
> > -       ia_css_stream_config *config);
> > +       ia_css_stream_config * config);
> 
> No, this makes it the opposite. Please, read Coding Style if it sheds
> a light on this. In any case the kernel style is to avoid spacing
> between asterisk and name.

Understood. I'll resubmit with the correct spacing.

Thanks,
Thomas

Re: [PATCH 2/2] Staging: media: atomisp: style corrections

[Andy Shevchenko]

On Sun, Apr 27, 2025 at 5:24 PM Thomas Andreatta
<thomasandreatta2000@gmail.com> wrote:
> On Sun, Apr 27, 2025 at 12:41:34PM +0300, Andy Shevchenko wrote:
> > On Sat, Apr 26, 2025 at 11:15 PM Thomas Andreatta
> > <thomasandreatta2000@gmail.com> wrote:

> > > Corrected consistent spacing around '*' and braces positions
> >
> > Missed period.
> > And what is the correct spacing and why?
>
> I agree that the spacing looks weird and I questioned it too, but the script
> checkpatch.pl highlights as error:
> `sh_css.c:336: ERROR: need consistent spacing around '*' (ctx:WxV)`
> `sh_css.c:338: ERROR: need consistent spacing around '*' (ctx:WxV)`
>
> Should this be ignored because the script tries its best and it becomes common
> sense that the suggested spacing is "wrong"?

checkpatch sometimes gives false positives. Need to use common sense.

...

> > >  static unsigned int get_crop_lines_for_bayer_order(const struct
> > > -       ia_css_stream_config *config);
> > > +       ia_css_stream_config * config);
> > >  static unsigned int get_crop_columns_for_bayer_order(const struct
> > > -       ia_css_stream_config *config);
> > > +       ia_css_stream_config * config);
> >
> > No, this makes it the opposite. Please, read Coding Style if it sheds
> > a light on this. In any case the kernel style is to avoid spacing
> > between asterisk and name.
>
> Understood. I'll resubmit with the correct spacing.

Looking again at the above I think the best formatting should be like this:

static unsigned int
get_crop_columns_for_bayer_order(const struct ia_css_stream_config *config);

But looking even closer it seems that these are forward declarations
for the internal functions. The question here is can we rearrange the
functions that we can remove these forward declarations completely?

-- 
With Best Regards,
Andy Shevchenko