* [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line @ 2021-04-15 17:08 Aline Santana Cordeiro 2021-04-15 17:14 ` [Outreachy kernel] " Matthew Wilcox 2021-04-20 12:41 ` Hans Verkuil 0 siblings, 2 replies; 14+ messages in thread From: Aline Santana Cordeiro @ 2021-04-15 17:08 UTC (permalink / raw) To: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman Cc: linux-media, linux-staging, linux-kernel, outreachy-kernel 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 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line 2021-04-15 17:08 [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line Aline Santana Cordeiro @ 2021-04-15 17:14 ` Matthew Wilcox 2021-04-15 18:09 ` ascordeiro 2021-04-15 19:49 ` Sakari Ailus 2021-04-20 12:41 ` Hans Verkuil 1 sibling, 2 replies; 14+ messages in thread From: Matthew Wilcox @ 2021-04-15 17:14 UTC (permalink / raw) To: Aline Santana Cordeiro Cc: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman, linux-media, linux-staging, linux-kernel, outreachy-kernel 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line 2021-04-15 17:14 ` [Outreachy kernel] " Matthew Wilcox @ 2021-04-15 18:09 ` ascordeiro 2021-04-15 19:49 ` Sakari Ailus 1 sibling, 0 replies; 14+ messages in thread From: ascordeiro @ 2021-04-15 18:09 UTC (permalink / raw) To: Matthew Wilcox Cc: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman, linux-media, linux-staging, linux-kernel, outreachy-kernel 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. > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line 2021-04-15 17:14 ` [Outreachy kernel] " Matthew Wilcox 2021-04-15 18:09 ` ascordeiro @ 2021-04-15 19:49 ` Sakari Ailus 2021-04-15 19:57 ` Matthew Wilcox 1 sibling, 1 reply; 14+ messages in thread From: Sakari Ailus @ 2021-04-15 19:49 UTC (permalink / raw) To: Matthew Wilcox Cc: Aline Santana Cordeiro, Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, linux-staging, linux-kernel, outreachy-kernel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line 2021-04-15 19:49 ` Sakari Ailus @ 2021-04-15 19:57 ` Matthew Wilcox 2021-04-15 19:59 ` Matthew Wilcox 0 siblings, 1 reply; 14+ messages in thread From: Matthew Wilcox @ 2021-04-15 19:57 UTC (permalink / raw) To: Sakari Ailus Cc: Aline Santana Cordeiro, Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, linux-staging, linux-kernel, outreachy-kernel 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line 2021-04-15 19:57 ` Matthew Wilcox @ 2021-04-15 19:59 ` Matthew Wilcox 2021-04-15 21:21 ` Sakari Ailus 0 siblings, 1 reply; 14+ messages in thread From: Matthew Wilcox @ 2021-04-15 19:59 UTC (permalink / raw) To: Sakari Ailus Cc: Aline Santana Cordeiro, Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, linux-staging, linux-kernel, outreachy-kernel 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/1054519757.161606@palladium.transmeta.com/ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line 2021-04-15 19:59 ` Matthew Wilcox @ 2021-04-15 21:21 ` Sakari Ailus 2021-04-16 5:49 ` Dan Carpenter 0 siblings, 1 reply; 14+ messages in thread From: Sakari Ailus @ 2021-04-15 21:21 UTC (permalink / raw) To: Matthew Wilcox Cc: Aline Santana Cordeiro, Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, linux-staging, linux-kernel, outreachy-kernel On Thu, Apr 15, 2021 at 08:59:41PM +0100, Matthew Wilcox wrote: > 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/1054519757.161606@palladium.transmeta.com/ Two decades ago, really? This is simply one of the practical means how you split long function declarations and avoid overly long lines. Not my favourite though, but still better than those long lines. My personal preference would be to wrap at the opening parenthesis and indent by just a tab, but I know many people who disagree with that... -- Sakari Ailus ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line 2021-04-15 21:21 ` Sakari Ailus @ 2021-04-16 5:49 ` Dan Carpenter 2021-04-16 8:37 ` Sakari Ailus 0 siblings, 1 reply; 14+ messages in thread From: Dan Carpenter @ 2021-04-16 5:49 UTC (permalink / raw) To: Sakari Ailus Cc: Matthew Wilcox, Aline Santana Cordeiro, Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, linux-staging, linux-kernel, outreachy-kernel On Fri, Apr 16, 2021 at 12:21:58AM +0300, Sakari Ailus wrote: > On Thu, Apr 15, 2021 at 08:59:41PM +0100, Matthew Wilcox wrote: > > 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/1054519757.161606@palladium.transmeta.com/ > > Two decades ago, really? > > This is simply one of the practical means how you split long function > declarations and avoid overly long lines. Not my favourite though, but > still better than those long lines. I've always thought we allow either style, but it has to be done consistently within the file. I was pretty sure that was policy but it's another thing that goes back decades so I don't have a reference. It shouldn't be about breaking up long lines. > > My personal preference would be to wrap at the opening parenthesis and > indent by just a tab, but I know many people who disagree with that... If you're running into the 80 character limit, then it's fine to use two tabs. I think we have been rejecting patches that push align the parameters but push past the 80 character limit. Using one tab is confusing because it makes the decalarations line up with the code. regards, dan carpenter ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line 2021-04-16 5:49 ` Dan Carpenter @ 2021-04-16 8:37 ` Sakari Ailus 2021-04-16 8:46 ` Julia Lawall 2021-04-16 9:13 ` Dan Carpenter 0 siblings, 2 replies; 14+ messages in thread From: Sakari Ailus @ 2021-04-16 8:37 UTC (permalink / raw) To: Dan Carpenter Cc: Matthew Wilcox, Aline Santana Cordeiro, Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, linux-staging, linux-kernel, outreachy-kernel Hi Dan, On Fri, Apr 16, 2021 at 08:49:41AM +0300, Dan Carpenter wrote: > On Fri, Apr 16, 2021 at 12:21:58AM +0300, Sakari Ailus wrote: > > On Thu, Apr 15, 2021 at 08:59:41PM +0100, Matthew Wilcox wrote: > > > 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/1054519757.161606@palladium.transmeta.com/ > > > > Two decades ago, really? > > > > This is simply one of the practical means how you split long function > > declarations and avoid overly long lines. Not my favourite though, but > > still better than those long lines. > > I've always thought we allow either style, but it has to be done > consistently within the file. I was pretty sure that was policy but > it's another thing that goes back decades so I don't have a reference. > It shouldn't be about breaking up long lines. > > > > > My personal preference would be to wrap at the opening parenthesis and > > indent by just a tab, but I know many people who disagree with that... > > If you're running into the 80 character limit, then it's fine to use > two tabs. I think we have been rejecting patches that push align the > parameters but push past the 80 character limit. Using one tab is > confusing because it makes the decalarations line up with the code. Interesting. Do you have an example of this? I've thought checkpatch.pl gave a warning if the line ended with an opening parenthesis no matter what. -- Kind regards, Sakari Ailus ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line 2021-04-16 8:37 ` Sakari Ailus @ 2021-04-16 8:46 ` Julia Lawall 2021-04-16 8:54 ` Sakari Ailus 2021-04-16 9:13 ` Dan Carpenter 1 sibling, 1 reply; 14+ messages in thread From: Julia Lawall @ 2021-04-16 8:46 UTC (permalink / raw) To: Sakari Ailus Cc: Dan Carpenter, Matthew Wilcox, Aline Santana Cordeiro, Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, linux-staging, linux-kernel, outreachy-kernel > > If you're running into the 80 character limit, then it's fine to use > > two tabs. I think we have been rejecting patches that push align the > > parameters but push past the 80 character limit. Using one tab is > > confusing because it makes the decalarations line up with the code. > > Interesting. Do you have an example of this? I've thought checkpatch.pl > gave a warning if the line ended with an opening parenthesis no matter > what. Checkpatch is a perl script that searches for patterns that often indicate code that is hard to understand. It is not a precise definition of what is allowed in the Linux kernel. Humans have to amke compromises. julia ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line 2021-04-16 8:46 ` Julia Lawall @ 2021-04-16 8:54 ` Sakari Ailus 2021-04-16 9:03 ` Julia Lawall 0 siblings, 1 reply; 14+ messages in thread From: Sakari Ailus @ 2021-04-16 8:54 UTC (permalink / raw) To: Julia Lawall Cc: Dan Carpenter, Matthew Wilcox, Aline Santana Cordeiro, Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, linux-staging, linux-kernel, outreachy-kernel On Fri, Apr 16, 2021 at 10:46:54AM +0200, Julia Lawall wrote: > > > If you're running into the 80 character limit, then it's fine to use > > > two tabs. I think we have been rejecting patches that push align the > > > parameters but push past the 80 character limit. Using one tab is > > > confusing because it makes the decalarations line up with the code. > > > > Interesting. Do you have an example of this? I've thought checkpatch.pl > > gave a warning if the line ended with an opening parenthesis no matter > > what. > > Checkpatch is a perl script that searches for patterns that often indicate > code that is hard to understand. It is not a precise definition of what > is allowed in the Linux kernel. Humans have to amke compromises. Yeah... but I'd think if this is a preferred style then the warning could be omitted. It might not be that difficult. -- Sakari Ailus ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line 2021-04-16 8:54 ` Sakari Ailus @ 2021-04-16 9:03 ` Julia Lawall 0 siblings, 0 replies; 14+ messages in thread From: Julia Lawall @ 2021-04-16 9:03 UTC (permalink / raw) To: Sakari Ailus Cc: Julia Lawall, Dan Carpenter, Matthew Wilcox, Aline Santana Cordeiro, Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, linux-staging, linux-kernel, outreachy-kernel On Fri, 16 Apr 2021, Sakari Ailus wrote: > On Fri, Apr 16, 2021 at 10:46:54AM +0200, Julia Lawall wrote: > > > > If you're running into the 80 character limit, then it's fine to use > > > > two tabs. I think we have been rejecting patches that push align the > > > > parameters but push past the 80 character limit. Using one tab is > > > > confusing because it makes the decalarations line up with the code. > > > > > > Interesting. Do you have an example of this? I've thought checkpatch.pl > > > gave a warning if the line ended with an opening parenthesis no matter > > > what. > > > > Checkpatch is a perl script that searches for patterns that often indicate > > code that is hard to understand. It is not a precise definition of what > > is allowed in the Linux kernel. Humans have to amke compromises. > > Yeah... but I'd think if this is a preferred style then the warning could > be omitted. It might not be that difficult. No idea. It involves looking at two successive lines, which may make it more complicated. Probably the biggest problem would be knowing whether the line being looked at represents a function header. Maybe that could be detected by the fact that there is normally no space at the beginning of the line? julia ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line 2021-04-16 8:37 ` Sakari Ailus 2021-04-16 8:46 ` Julia Lawall @ 2021-04-16 9:13 ` Dan Carpenter 1 sibling, 0 replies; 14+ messages in thread From: Dan Carpenter @ 2021-04-16 9:13 UTC (permalink / raw) To: Sakari Ailus Cc: Matthew Wilcox, Aline Santana Cordeiro, Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, linux-staging, linux-kernel, outreachy-kernel On Fri, Apr 16, 2021 at 11:37:28AM +0300, Sakari Ailus wrote: > Hi Dan, > > On Fri, Apr 16, 2021 at 08:49:41AM +0300, Dan Carpenter wrote: > > On Fri, Apr 16, 2021 at 12:21:58AM +0300, Sakari Ailus wrote: > > > On Thu, Apr 15, 2021 at 08:59:41PM +0100, Matthew Wilcox wrote: > > > > 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/1054519757.161606@palladium.transmeta.com/ > > > > > > Two decades ago, really? > > > > > > This is simply one of the practical means how you split long function > > > declarations and avoid overly long lines. Not my favourite though, but > > > still better than those long lines. > > > > I've always thought we allow either style, but it has to be done > > consistently within the file. I was pretty sure that was policy but > > it's another thing that goes back decades so I don't have a reference. > > It shouldn't be about breaking up long lines. > > > > > > > > My personal preference would be to wrap at the opening parenthesis and > > > indent by just a tab, but I know many people who disagree with that... > > > > If you're running into the 80 character limit, then it's fine to use > > two tabs. I think we have been rejecting patches that push align the > > parameters but push past the 80 character limit. Using one tab is > > confusing because it makes the decalarations line up with the code. > > Interesting. Do you have an example of this? I've thought checkpatch.pl > gave a warning if the line ended with an opening parenthesis no matter > what. The prefered style is still aligning with the parentheses but if you have to choose between a warning about going over the limit or a warning about aligning then probably it's fine to not align. I can't find an example, but I'm pretty sure we've been rejecting patches that align the parameters but now go over the 80/100 char limit. regards, dan carpenter ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line 2021-04-15 17:08 [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line Aline Santana Cordeiro 2021-04-15 17:14 ` [Outreachy kernel] " Matthew Wilcox @ 2021-04-20 12:41 ` Hans Verkuil 1 sibling, 0 replies; 14+ messages in thread From: Hans Verkuil @ 2021-04-20 12:41 UTC (permalink / raw) To: Aline Santana Cordeiro, Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman Cc: linux-media, linux-staging, linux-kernel, outreachy-kernel 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 <alinesantanacordeiro@gmail.com> > --- > > 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); > ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-04-20 12:41 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-15 17:08 [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line Aline Santana Cordeiro 2021-04-15 17:14 ` [Outreachy kernel] " Matthew Wilcox 2021-04-15 18:09 ` ascordeiro 2021-04-15 19:49 ` Sakari Ailus 2021-04-15 19:57 ` Matthew Wilcox 2021-04-15 19:59 ` Matthew Wilcox 2021-04-15 21:21 ` Sakari Ailus 2021-04-16 5:49 ` Dan Carpenter 2021-04-16 8:37 ` Sakari Ailus 2021-04-16 8:46 ` Julia Lawall 2021-04-16 8:54 ` Sakari Ailus 2021-04-16 9:03 ` Julia Lawall 2021-04-16 9:13 ` Dan Carpenter 2021-04-20 12:41 ` Hans Verkuil
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).