2021-04-15 17:09:48

by Aline Santana Cordeiro

[permalink] [raw]
Subject: [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line

Change line break to avoid an open parenthesis at the end of the line.
It consequently removed spaces at the start of the subsequent line.
Both issues detected by checkpatch.pl.

Signed-off-by: Aline Santana Cordeiro <[email protected]>
---

Changes since v1:
- Keep the * with the function return type
instead of left it with the function name

drivers/staging/media/atomisp/pci/atomisp_cmd.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.h b/drivers/staging/media/atomisp/pci/atomisp_cmd.h
index 1c0d464..e2b36fa 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.h
@@ -75,8 +75,8 @@ void atomisp_wdt(struct timer_list *t);
void atomisp_setup_flash(struct atomisp_sub_device *asd);
irqreturn_t atomisp_isr(int irq, void *dev);
irqreturn_t atomisp_isr_thread(int irq, void *isp_ptr);
-const struct atomisp_format_bridge *get_atomisp_format_bridge_from_mbus(
- u32 mbus_code);
+const struct atomisp_format_bridge*
+get_atomisp_format_bridge_from_mbus(u32 mbus_code);
bool atomisp_is_mbuscode_raw(uint32_t code);
int atomisp_get_frame_pgnr(struct atomisp_device *isp,
const struct ia_css_frame *frame, u32 *p_pgnr);
@@ -381,9 +381,9 @@ enum mipi_port_id __get_mipi_port(struct atomisp_device *isp,

bool atomisp_is_vf_pipe(struct atomisp_video_pipe *pipe);

-void atomisp_apply_css_parameters(
- struct atomisp_sub_device *asd,
- struct atomisp_css_params *css_param);
+void atomisp_apply_css_parameters(struct atomisp_sub_device *asd,
+ struct atomisp_css_params *css_param);
+
void atomisp_free_css_parameters(struct atomisp_css_params *css_param);

void atomisp_handle_parameter_and_buffer(struct atomisp_video_pipe *pipe);
--
2.7.4


2021-04-15 17:16:27

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [Outreachy kernel] [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line

On Thu, Apr 15, 2021 at 02:08:19PM -0300, Aline Santana Cordeiro wrote:
> -const struct atomisp_format_bridge *get_atomisp_format_bridge_from_mbus(
> - u32 mbus_code);
> +const struct atomisp_format_bridge*
> +get_atomisp_format_bridge_from_mbus(u32 mbus_code);

No, this does not match coding style. Probably best to break the
80-column guideline in this instance. Best would be to have a function
and/or struct name that isn't so ridiculously long, but that would
require some in-depth thinking.

> -void atomisp_apply_css_parameters(
> - struct atomisp_sub_device *asd,
> - struct atomisp_css_params *css_param);
> +void atomisp_apply_css_parameters(struct atomisp_sub_device *asd,
> + struct atomisp_css_params *css_param);
> +

Good.

2021-04-15 18:11:22

by Aline Santana Cordeiro

[permalink] [raw]
Subject: Re: [Outreachy kernel] [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line

Em qui, 2021-04-15 às 18:14 +0100, Matthew Wilcox escreveu:
> On Thu, Apr 15, 2021 at 02:08:19PM -0300, Aline Santana Cordeiro
> wrote:
> > -const struct atomisp_format_bridge
> > *get_atomisp_format_bridge_from_mbus(
> > -    u32 mbus_code);
> > +const struct atomisp_format_bridge*
> > +get_atomisp_format_bridge_from_mbus(u32 mbus_code);
>
> No, this does not match coding style.  Probably best to break the
> 80-column guideline in this instance.  Best would be to have a
> function
> and/or struct name that isn't so ridiculously long, but that would
> require some in-depth thinking.
>

I left the type of function and its name with the parameters in
different lines, following up some examples of other files, such as
atomisp_acc.c.

But I didn't pay attention and left the pointer with the function name
instead of left it with the type of the function in v1, so Hans
suggested it to a v2, as I did.

What should I do in this case?

Thank you in advance,
Aline

> > -void atomisp_apply_css_parameters(
> > -    struct atomisp_sub_device *asd,
> > -    struct atomisp_css_params *css_param);
> > +void atomisp_apply_css_parameters(struct atomisp_sub_device *asd,
> > +                                 struct atomisp_css_params
> > *css_param);
> > +
>
> Good.
>


2021-04-15 20:29:44

by Sakari Ailus

[permalink] [raw]
Subject: Re: [Outreachy kernel] [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line

On Thu, Apr 15, 2021 at 06:14:09PM +0100, Matthew Wilcox wrote:
> On Thu, Apr 15, 2021 at 02:08:19PM -0300, Aline Santana Cordeiro wrote:
> > -const struct atomisp_format_bridge *get_atomisp_format_bridge_from_mbus(
> > - u32 mbus_code);
> > +const struct atomisp_format_bridge*
> > +get_atomisp_format_bridge_from_mbus(u32 mbus_code);
>
> No, this does not match coding style. Probably best to break the
> 80-column guideline in this instance. Best would be to have a function

Having the return type on the previous line is perfectly fine. There should
be a space before the asterisk though.

--
Sakari Ailus

2021-04-15 22:02:06

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [Outreachy kernel] [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line

On Thu, Apr 15, 2021 at 10:49:55PM +0300, Sakari Ailus wrote:
> On Thu, Apr 15, 2021 at 06:14:09PM +0100, Matthew Wilcox wrote:
> > On Thu, Apr 15, 2021 at 02:08:19PM -0300, Aline Santana Cordeiro wrote:
> > > -const struct atomisp_format_bridge *get_atomisp_format_bridge_from_mbus(
> > > - u32 mbus_code);
> > > +const struct atomisp_format_bridge*
> > > +get_atomisp_format_bridge_from_mbus(u32 mbus_code);
> >
> > No, this does not match coding style. Probably best to break the
> > 80-column guideline in this instance. Best would be to have a function
>
> Having the return type on the previous line is perfectly fine. There should
> be a space before the asterisk though.

No, it's not. Linus has ranted about that before.

2021-04-15 22:03:29

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [Outreachy kernel] [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line

On Thu, Apr 15, 2021 at 08:57:04PM +0100, Matthew Wilcox wrote:
> On Thu, Apr 15, 2021 at 10:49:55PM +0300, Sakari Ailus wrote:
> > On Thu, Apr 15, 2021 at 06:14:09PM +0100, Matthew Wilcox wrote:
> > > On Thu, Apr 15, 2021 at 02:08:19PM -0300, Aline Santana Cordeiro wrote:
> > > > -const struct atomisp_format_bridge *get_atomisp_format_bridge_from_mbus(
> > > > - u32 mbus_code);
> > > > +const struct atomisp_format_bridge*
> > > > +get_atomisp_format_bridge_from_mbus(u32 mbus_code);
> > >
> > > No, this does not match coding style. Probably best to break the
> > > 80-column guideline in this instance. Best would be to have a function
> >
> > Having the return type on the previous line is perfectly fine. There should
> > be a space before the asterisk though.
>
> No, it's not. Linus has ranted about that before.

Found it. https://lore.kernel.org/lkml/[email protected]/

2021-04-20 12:44:00

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line

On 15/04/2021 19:08, Aline Santana Cordeiro wrote:
> Change line break to avoid an open parenthesis at the end of the line.
> It consequently removed spaces at the start of the subsequent line.
> Both issues detected by checkpatch.pl.
>
> Signed-off-by: Aline Santana Cordeiro <[email protected]>
> ---
>
> Changes since v1:
> - Keep the * with the function return type
> instead of left it with the function name
>
> drivers/staging/media/atomisp/pci/atomisp_cmd.h | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.h b/drivers/staging/media/atomisp/pci/atomisp_cmd.h
> index 1c0d464..e2b36fa 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_cmd.h
> +++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.h
> @@ -75,8 +75,8 @@ void atomisp_wdt(struct timer_list *t);
> void atomisp_setup_flash(struct atomisp_sub_device *asd);
> irqreturn_t atomisp_isr(int irq, void *dev);
> irqreturn_t atomisp_isr_thread(int irq, void *isp_ptr);
> -const struct atomisp_format_bridge *get_atomisp_format_bridge_from_mbus(
> - u32 mbus_code);
> +const struct atomisp_format_bridge*

Keep the space before the *!

Regards,

Hans

> +get_atomisp_format_bridge_from_mbus(u32 mbus_code);
> bool atomisp_is_mbuscode_raw(uint32_t code);
> int atomisp_get_frame_pgnr(struct atomisp_device *isp,
> const struct ia_css_frame *frame, u32 *p_pgnr);
> @@ -381,9 +381,9 @@ enum mipi_port_id __get_mipi_port(struct atomisp_device *isp,
>
> bool atomisp_is_vf_pipe(struct atomisp_video_pipe *pipe);
>
> -void atomisp_apply_css_parameters(
> - struct atomisp_sub_device *asd,
> - struct atomisp_css_params *css_param);
> +void atomisp_apply_css_parameters(struct atomisp_sub_device *asd,
> + struct atomisp_css_params *css_param);
> +
> void atomisp_free_css_parameters(struct atomisp_css_params *css_param);
>
> void atomisp_handle_parameter_and_buffer(struct atomisp_video_pipe *pipe);
>