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

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

[Aline Santana Cordeiro]

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 <alinesantanacordeiro@gmail.com>
---

Changes since v2:
 - Insert a space between the function type and pointer

Changes since v1:
 - Keep the pointer 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..639eca3 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

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

[Julia Lawall]

On Wed, 21 Apr 2021, 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.

The message is hard to understand.  There are a lot of singular nouns, but
actually there are two changes.  Which change is being described by the
above message?  What does "It" refer to?

julia


> Both issues detected by checkpatch.pl.
>
> Signed-off-by: Aline Santana Cordeiro <alinesantanacordeiro@gmail.com>
> ---
>
> Changes since v2:
>  - Insert a space between the function type and pointer
>
> Changes since v1:
>  - Keep the pointer 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..639eca3 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
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20210421123718.GA4597%40focaruja.
>

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

[Matthew Wilcox]

On Wed, Apr 21, 2021 at 09:37:18AM -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);

As I said, better to break the 80 column rule than do this.

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

[Aline Santana Cordeiro]

Em qua, 2021-04-21 às 15:08 +0200, Julia Lawall escreveu:
> 
> 
> On Wed, 21 Apr 2021, 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.
> 
> The message is hard to understand.  There are a lot of singular
> nouns, but
> actually there are two changes.  Which change is being described by
> the
> above message?  What does "It" refer to?
> 
> julia

Checkpatch indicated two problems with this function declaration:
1) The line ending with an open parenthesis, and
2) The following line - with the function parameters - has spaces in
its identation.

When I changed the line break to put the function name and its
parameter in the following line, both checkpath checks were eliminated.

So, the main change was the line break and, also, the line break (it)
removed the space in the following line.

Is it better to change the message and explain only about the line
break?

Thank you,
Aline
> 
> 
> > Both issues detected by checkpatch.pl.
> > 
> > Signed-off-by: Aline Santana Cordeiro <
> > alinesantanacordeiro@gmail.com>
> > ---
> > 
> > Changes since v2:
> >  - Insert a space between the function type and pointer
> > 
> > Changes since v1:
> >  - Keep the pointer 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..639eca3 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
> > 
> > --
> > You received this message because you are subscribed to the Google
> > Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it,
> > send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit 
> > https://groups.google.com/d/msgid/outreachy-kernel/20210421123718.GA4597%40focaruja
> > .
> > 

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

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 4408 bytes --]



On Wed, 21 Apr 2021, Aline Santana Cordeiro wrote:

> Em qua, 2021-04-21 às 15:08 +0200, Julia Lawall escreveu:
> >
> >
> > On Wed, 21 Apr 2021, 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.
> >
> > The message is hard to understand.  There are a lot of singular
> > nouns, but
> > actually there are two changes.  Which change is being described by
> > the
> > above message?  What does "It" refer to?
> >
> > julia
>
> Checkpatch indicated two problems with this function declaration:
> 1) The line ending with an open parenthesis, and
> 2) The following line - with the function parameters - has spaces in
> its identation.
>
> When I changed the line break to put the function name and its
> parameter in the following line, both checkpath checks were eliminated.
>
> So, the main change was the line break and, also, the line break (it)
> removed the space in the following line.
>
> Is it better to change the message and explain only about the line
> break?

The message should explain about the whole patch.  So if you change two
things, it should be clear that what you are saying covers both of them.

But it seems that Matthew doesn't think that the line break is a good idea
anyway.

julia

>
> Thank you,
> Aline
> >
> >
> > > Both issues detected by checkpatch.pl.
> > >
> > > Signed-off-by: Aline Santana Cordeiro <
> > > alinesantanacordeiro@gmail.com>
> > > ---
> > >
> > > Changes since v2:
> > >  - Insert a space between the function type and pointer
> > >
> > > Changes since v1:
> > >  - Keep the pointer 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..639eca3 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
> > >
> > > --
> > > You received this message because you are subscribed to the Google
> > > Groups "outreachy-kernel" group.
> > > To unsubscribe from this group and stop receiving emails from it,
> > > send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > > To view this discussion on the web visit
> > > https://groups.google.com/d/msgid/outreachy-kernel/20210421123718.GA4597%40focaruja
> > > .
> > >
>
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/7aeac7041a6f6d7b3d8563f0d0bf0a4d31f379b0.camel%40gmail.com.
>

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

[Aline Santana Cordeiro]

Em qua, 2021-04-21 às 15:56 +0200, Julia Lawall escreveu:
> 
> 
> On Wed, 21 Apr 2021, Aline Santana Cordeiro wrote:
> 
> > Em qua, 2021-04-21 às 15:08 +0200, Julia Lawall escreveu:
> > > 
> > > 
> > > On Wed, 21 Apr 2021, 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.
> > > 
> > > The message is hard to understand.  There are a lot of singular
> > > nouns, but
> > > actually there are two changes.  Which change is being described
> > > by
> > > the
> > > above message?  What does "It" refer to?
> > > 
> > > julia
> > 
> > Checkpatch indicated two problems with this function declaration:
> > 1) The line ending with an open parenthesis, and
> > 2) The following line - with the function parameters - has spaces
> > in
> > its identation.
> > 
> > When I changed the line break to put the function name and its
> > parameter in the following line, both checkpath checks were
> > eliminated.
> > 
> > So, the main change was the line break and, also, the line break
> > (it)
> > removed the space in the following line.
> > 
> > Is it better to change the message and explain only about the line
> > break?
> 
> The message should explain about the whole patch.  So if you change
> two
> things, it should be clear that what you are saying covers both of
> them.

Ok, I can do that. In the commit message I described just one issue
because it is only one patch, I didn't want it to look like I was
changing different issues in just one patch.

> 
> But it seems that Matthew doesn't think that the line break is a good
> idea
> anyway.

Yes, I'm sending this email to Matthew too, because I don't know
exactly how to proceed as Hans asked me to made some corrections too. 
I've made these changes because checkpatch has indicated and with this
line break, checkpatch does not indicate any check or warning anymore.
But I can undo that too, I just don't know what I'm supposed to do with
so many opposite opinions. 


Thank you all,
Aline 
> 
> julia
> 
> > 
> > Thank you,
> > Aline
> > > 
> > > 
> > > > Both issues detected by checkpatch.pl.
> > > > 
> > > > Signed-off-by: Aline Santana Cordeiro <
> > > > alinesantanacordeiro@gmail.com>
> > > > ---
> > > > 
> > > > Changes since v2:
> > > >  - Insert a space between the function type and pointer
> > > > 
> > > > Changes since v1:
> > > >  - Keep the pointer 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..639eca3 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
> > > > 
> > > > --
> > > > You received this message because you are subscribed to the
> > > > Google
> > > > Groups "outreachy-kernel" group.
> > > > To unsubscribe from this group and stop receiving emails from
> > > > it,
> > > > send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > > > To view this discussion on the web visit
> > > > https://groups.google.com/d/msgid/outreachy-kernel/20210421123718.GA4597%40focaruja
> > > > .
> > > > 
> > 
> > 
> > --
> > You received this message because you are subscribed to the Google
> > Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it,
> > send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit    
> > https://groups.google.com/d/msgid/outreachy-kernel/7aeac7041a6f6d7b3d8563f0d0bf0a4d31f379b0.camel%40gmail.com
> > .

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

[Hans Verkuil]

On 21/04/2021 16:21, Aline Santana Cordeiro wrote:
> Em qua, 2021-04-21 às 15:56 +0200, Julia Lawall escreveu:
>>
>>
>> On Wed, 21 Apr 2021, Aline Santana Cordeiro wrote:
>>
>>> Em qua, 2021-04-21 às 15:08 +0200, Julia Lawall escreveu:
>>>>
>>>>
>>>> On Wed, 21 Apr 2021, 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.
>>>>
>>>> The message is hard to understand.  There are a lot of singular
>>>> nouns, but
>>>> actually there are two changes.  Which change is being described
>>>> by
>>>> the
>>>> above message?  What does "It" refer to?
>>>>
>>>> julia
>>>
>>> Checkpatch indicated two problems with this function declaration:
>>> 1) The line ending with an open parenthesis, and
>>> 2) The following line - with the function parameters - has spaces
>>> in
>>> its identation.
>>>
>>> When I changed the line break to put the function name and its
>>> parameter in the following line, both checkpath checks were
>>> eliminated.
>>>
>>> So, the main change was the line break and, also, the line break
>>> (it)
>>> removed the space in the following line.
>>>
>>> Is it better to change the message and explain only about the line
>>> break?
>>
>> The message should explain about the whole patch.  So if you change
>> two
>> things, it should be clear that what you are saying covers both of
>> them.
> 
> Ok, I can do that. In the commit message I described just one issue
> because it is only one patch, I didn't want it to look like I was
> changing different issues in just one patch.
> 
>>
>> But it seems that Matthew doesn't think that the line break is a good
>> idea
>> anyway.
> 
> Yes, I'm sending this email to Matthew too, because I don't know
> exactly how to proceed as Hans asked me to made some corrections too. 
> I've made these changes because checkpatch has indicated and with this
> line break, checkpatch does not indicate any check or warning anymore.
> But I can undo that too, I just don't know what I'm supposed to do with
> so many opposite opinions. 

As one of the media maintainers I can say that in this case the preference
would be to split it up in two lines. It's one of those areas where
different maintainers have different opinions.

Just keep in mind that this is all nitpicking and normally we probably
wouldn't bother with this at all, but it is a good exercise to learn
about patches and contributing :-)

Regards,

	Hans

> 
> 
> Thank you all,
> Aline 
>>
>> julia
>>
>>>
>>> Thank you,
>>> Aline
>>>>
>>>>
>>>>> Both issues detected by checkpatch.pl.
>>>>>
>>>>> Signed-off-by: Aline Santana Cordeiro <
>>>>> alinesantanacordeiro@gmail.com>
>>>>> ---
>>>>>
>>>>> Changes since v2:
>>>>>  - Insert a space between the function type and pointer
>>>>>
>>>>> Changes since v1:
>>>>>  - Keep the pointer 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..639eca3 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
>>>>>
>>>>> --
>>>>> You received this message because you are subscribed to the
>>>>> Google
>>>>> Groups "outreachy-kernel" group.
>>>>> To unsubscribe from this group and stop receiving emails from
>>>>> it,
>>>>> send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>>>>> To view this discussion on the web visit
>>>>> https://groups.google.com/d/msgid/outreachy-kernel/20210421123718.GA4597%40focaruja
>>>>> .
>>>>>
>>>
>>>
>>> --
>>> You received this message because you are subscribed to the Google
>>> Groups "outreachy-kernel" group.
>>> To unsubscribe from this group and stop receiving emails from it,
>>> send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>>> To view this discussion on the web visit    
>>> https://groups.google.com/d/msgid/outreachy-kernel/7aeac7041a6f6d7b3d8563f0d0bf0a4d31f379b0.camel%40gmail.com
>>> .
> 
> 

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

[Aline Santana Cordeiro]

Em sex, 2021-04-23 às 11:21 +0200, Hans Verkuil escreveu:
> On 21/04/2021 16:21, Aline Santana Cordeiro wrote:
> > Em qua, 2021-04-21 às 15:56 +0200, Julia Lawall escreveu:
> > > 
> > > 
> > > On Wed, 21 Apr 2021, Aline Santana Cordeiro wrote:
> > > 
> > > > Em qua, 2021-04-21 às 15:08 +0200, Julia Lawall escreveu:
> > > > > 
> > > > > 
> > > > > On Wed, 21 Apr 2021, 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.
> > > > > 
> > > > > The message is hard to understand.  There are a lot of
> > > > > singular
> > > > > nouns, but
> > > > > actually there are two changes.  Which change is being
> > > > > described
> > > > > by
> > > > > the
> > > > > above message?  What does "It" refer to?
> > > > > 
> > > > > julia
> > > > 
> > > > Checkpatch indicated two problems with this function
> > > > declaration:
> > > > 1) The line ending with an open parenthesis, and
> > > > 2) The following line - with the function parameters - has
> > > > spaces
> > > > in
> > > > its identation.
> > > > 
> > > > When I changed the line break to put the function name and its
> > > > parameter in the following line, both checkpath checks were
> > > > eliminated.
> > > > 
> > > > So, the main change was the line break and, also, the line
> > > > break
> > > > (it)
> > > > removed the space in the following line.
> > > > 
> > > > Is it better to change the message and explain only about the
> > > > line
> > > > break?
> > > 
> > > The message should explain about the whole patch.  So if you
> > > change
> > > two
> > > things, it should be clear that what you are saying covers both
> > > of
> > > them.
> > 
> > Ok, I can do that. In the commit message I described just one issue
> > because it is only one patch, I didn't want it to look like I was
> > changing different issues in just one patch.
> > 
> > > 
> > > But it seems that Matthew doesn't think that the line break is a
> > > good
> > > idea
> > > anyway.
> > 
> > Yes, I'm sending this email to Matthew too, because I don't know
> > exactly how to proceed as Hans asked me to made some corrections
> > too. 
> > I've made these changes because checkpatch has indicated and with
> > this
> > line break, checkpatch does not indicate any check or warning
> > anymore.
> > But I can undo that too, I just don't know what I'm supposed to do
> > with
> > so many opposite opinions. 
> 
> As one of the media maintainers I can say that in this case the
> preference
> would be to split it up in two lines. It's one of those areas where
> different maintainers have different opinions.
> 
> Just keep in mind that this is all nitpicking and normally we
> probably
> wouldn't bother with this at all, but it is a good exercise to learn
> about patches and contributing :-)
> 
> Regards,
> 
>         Hans

I really appreciate all the feedbacks I've received :)
Indeed we can learn a lot about all the contributing process.

Thank you,
Aline
> 
> > 
> > 
> > Thank you all,
> > Aline 
> > > 
> > > julia
> > > 
> > > > 
> > > > Thank you,
> > > > Aline
> > > > > 
> > > > > 
> > > > > > Both issues detected by checkpatch.pl.
> > > > > > 
> > > > > > Signed-off-by: Aline Santana Cordeiro <
> > > > > > alinesantanacordeiro@gmail.com>
> > > > > > ---
> > > > > > 
> > > > > > Changes since v2:
> > > > > >  - Insert a space between the function type and pointer
> > > > > > 
> > > > > > Changes since v1:
> > > > > >  - Keep the pointer 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..639eca3 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
> > > > > > 
> > > > > > --
> > > > > > You received this message because you are subscribed to the
> > > > > > Google
> > > > > > Groups "outreachy-kernel" group.
> > > > > > To unsubscribe from this group and stop receiving emails
> > > > > > from
> > > > > > it,
> > > > > > send an email to 
> > > > > > outreachy-kernel+unsubscribe@googlegroups.com.
> > > > > > To view this discussion on the web visit
> > > > > > https://groups.google.com/d/msgid/outreachy-kernel/20210421123718.GA4597%40focaruja
> > > > > > .
> > > > > > 
> > > > 
> > > > 
> > > > --
> > > > You received this message because you are subscribed to the
> > > > Google
> > > > Groups "outreachy-kernel" group.
> > > > To unsubscribe from this group and stop receiving emails from
> > > > it,
> > > > send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > > > To view this discussion on the web visit    
> > > > https://groups.google.com/d/msgid/outreachy-kernel/7aeac7041a6f6d7b3d8563f0d0bf0a4d31f379b0.camel%40gmail.com
> > > > .
> > 
> > 
> 

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

[Sakari Ailus]

On Wed, Apr 21, 2021 at 02:15:22PM +0100, Matthew Wilcox wrote:
> On Wed, Apr 21, 2021 at 09:37:18AM -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);
> 
> As I said, better to break the 80 column rule than do this.

May depend on the maintainer, as well as other reviewers. It's a question
of priorities.

I'd suggest otherwise.

-- 
Sakari Ailus