* [PATCH 0/2] staging: media: omap4iss: Patchsets cleans up in iss.c
@ 2021-04-01 15:07 Beatriz Martins de Carvalho
2021-04-01 15:07 ` [PATCH 1/2] staging: media: omap4iss: Ending line with argument Beatriz Martins de Carvalho
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Beatriz Martins de Carvalho @ 2021-04-01 15:07 UTC (permalink / raw)
To: laurent.pinchart, mchehab, gregkh, linux-media, linux-staging,
linux-kernel, outreachy-kernel
Clean up checks of "Alignment should match open parenthesis" and
"CHECK: Lines should not end with a '('" in iss.c
Beatriz Martins de Carvalho (2):
staging: media: omap4iss: Ending line with argument
staging: media: omap4iss: align arguments with open parenthesis
drivers/staging/media/omap4iss/iss.c | 48 +++++++++++++---------------
1 file changed, 23 insertions(+), 25 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/2] staging: media: omap4iss: Ending line with argument 2021-04-01 15:07 [PATCH 0/2] staging: media: omap4iss: Patchsets cleans up in iss.c Beatriz Martins de Carvalho @ 2021-04-01 15:07 ` Beatriz Martins de Carvalho 2021-04-01 15:28 ` [Outreachy kernel] " Matthew Wilcox 2021-04-07 13:46 ` Hans Verkuil 2021-04-01 15:07 ` [PATCH 2/2] staging: media: omap4iss: align arguments with open parenthesis Beatriz Martins de Carvalho 2021-04-01 15:25 ` [PATCH 0/2] staging: media: omap4iss: Patchsets cleans up in iss.c Greg KH 2 siblings, 2 replies; 11+ messages in thread From: Beatriz Martins de Carvalho @ 2021-04-01 15:07 UTC (permalink / raw) To: laurent.pinchart, mchehab, gregkh, linux-media, linux-staging, linux-kernel, outreachy-kernel Remove checkpatch check "CHECK: Lines should not end with a '('" with argument present in next line and reorganize characters so the lines are under 100 columns Signed-off-by: Beatriz Martins de Carvalho <martinsdecarvalhobeatriz@gmail.com> --- drivers/staging/media/omap4iss/iss.c | 46 +++++++++++++--------------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/drivers/staging/media/omap4iss/iss.c b/drivers/staging/media/omap4iss/iss.c index dae9073e7d3c..e8f724dbf810 100644 --- a/drivers/staging/media/omap4iss/iss.c +++ b/drivers/staging/media/omap4iss/iss.c @@ -559,9 +559,10 @@ static int iss_reset(struct iss_device *iss) iss_reg_set(iss, OMAP4_ISS_MEM_TOP, ISS_HL_SYSCONFIG, ISS_HL_SYSCONFIG_SOFTRESET); - timeout = iss_poll_condition_timeout( - !(iss_reg_read(iss, OMAP4_ISS_MEM_TOP, ISS_HL_SYSCONFIG) & - ISS_HL_SYSCONFIG_SOFTRESET), 1000, 10, 100); + timeout = iss_poll_condition_timeout(!(iss_reg_read(iss, + OMAP4_ISS_MEM_TOP, ISS_HL_SYSCONFIG) + & ISS_HL_SYSCONFIG_SOFTRESET), + 1000, 10, 100); if (timeout) { dev_err(iss->dev, "ISS reset timeout\n"); return -ETIMEDOUT; @@ -583,9 +584,10 @@ static int iss_isp_reset(struct iss_device *iss) iss_reg_set(iss, OMAP4_ISS_MEM_ISP_SYS1, ISP5_CTRL, ISP5_CTRL_MSTANDBY); - timeout = iss_poll_condition_timeout( - iss_reg_read(iss, OMAP4_ISS_MEM_ISP_SYS1, ISP5_CTRL) & - ISP5_CTRL_MSTANDBY_WAIT, 1000000, 1000, 1500); + timeout = iss_poll_condition_timeout(iss_reg_read(iss, + OMAP4_ISS_MEM_ISP_SYS1, ISP5_CTRL) + & ISP5_CTRL_MSTANDBY_WAIT, 1000000, + 1000, 1500); if (timeout) { dev_err(iss->dev, "ISP5 standby timeout\n"); return -ETIMEDOUT; @@ -595,9 +597,10 @@ static int iss_isp_reset(struct iss_device *iss) iss_reg_set(iss, OMAP4_ISS_MEM_ISP_SYS1, ISP5_SYSCONFIG, ISP5_SYSCONFIG_SOFTRESET); - timeout = iss_poll_condition_timeout( - !(iss_reg_read(iss, OMAP4_ISS_MEM_ISP_SYS1, ISP5_SYSCONFIG) & - ISP5_SYSCONFIG_SOFTRESET), 1000000, 1000, 1500); + timeout = iss_poll_condition_timeout(!(iss_reg_read(iss, OMAP4_ISS_MEM_ISP_SYS1, + ISP5_SYSCONFIG) & + ISP5_SYSCONFIG_SOFTRESET), 1000000, + 1000, 1500); if (timeout) { dev_err(iss->dev, "ISP5 reset timeout\n"); return -ETIMEDOUT; @@ -1104,33 +1107,28 @@ static int iss_create_links(struct iss_device *iss) } /* Connect the submodules. */ - ret = media_create_pad_link( - &iss->csi2a.subdev.entity, CSI2_PAD_SOURCE, - &iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0); + ret = media_create_pad_link(&iss->csi2a.subdev.entity, CSI2_PAD_SOURCE, + &iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0); if (ret < 0) return ret; - ret = media_create_pad_link( - &iss->csi2b.subdev.entity, CSI2_PAD_SOURCE, - &iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0); + ret = media_create_pad_link(&iss->csi2b.subdev.entity, CSI2_PAD_SOURCE, + &iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0); if (ret < 0) return ret; - ret = media_create_pad_link( - &iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP, - &iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0); + ret = media_create_pad_link(&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP, + &iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0); if (ret < 0) return ret; - ret = media_create_pad_link( - &iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP, - &iss->ipipe.subdev.entity, IPIPE_PAD_SINK, 0); + ret = media_create_pad_link(&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP, + &iss->ipipe.subdev.entity, IPIPE_PAD_SINK, 0); if (ret < 0) return ret; - ret = media_create_pad_link( - &iss->ipipe.subdev.entity, IPIPE_PAD_SOURCE_VP, - &iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0); + ret = media_create_pad_link(&iss->ipipe.subdev.entity, IPIPE_PAD_SOURCE_VP, + &iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0); if (ret < 0) return ret; -- 2.25.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] [PATCH 1/2] staging: media: omap4iss: Ending line with argument 2021-04-01 15:07 ` [PATCH 1/2] staging: media: omap4iss: Ending line with argument Beatriz Martins de Carvalho @ 2021-04-01 15:28 ` Matthew Wilcox 2021-04-07 10:00 ` Beatriz Martins de Carvalho 2021-04-07 13:46 ` Hans Verkuil 1 sibling, 1 reply; 11+ messages in thread From: Matthew Wilcox @ 2021-04-01 15:28 UTC (permalink / raw) To: Beatriz Martins de Carvalho Cc: laurent.pinchart, mchehab, gregkh, linux-media, linux-staging, linux-kernel, outreachy-kernel On Thu, Apr 01, 2021 at 04:07:38PM +0100, Beatriz Martins de Carvalho wrote: > diff --git a/drivers/staging/media/omap4iss/iss.c b/drivers/staging/media/omap4iss/iss.c > index dae9073e7d3c..e8f724dbf810 100644 > --- a/drivers/staging/media/omap4iss/iss.c > +++ b/drivers/staging/media/omap4iss/iss.c > @@ -559,9 +559,10 @@ static int iss_reset(struct iss_device *iss) > iss_reg_set(iss, OMAP4_ISS_MEM_TOP, ISS_HL_SYSCONFIG, > ISS_HL_SYSCONFIG_SOFTRESET); > > - timeout = iss_poll_condition_timeout( > - !(iss_reg_read(iss, OMAP4_ISS_MEM_TOP, ISS_HL_SYSCONFIG) & > - ISS_HL_SYSCONFIG_SOFTRESET), 1000, 10, 100); > + timeout = iss_poll_condition_timeout(!(iss_reg_read(iss, > + OMAP4_ISS_MEM_TOP, ISS_HL_SYSCONFIG) > + & ISS_HL_SYSCONFIG_SOFTRESET), > + 1000, 10, 100); This is not a readability improvment. I would factor it out into its own function. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] [PATCH 1/2] staging: media: omap4iss: Ending line with argument 2021-04-01 15:28 ` [Outreachy kernel] " Matthew Wilcox @ 2021-04-07 10:00 ` Beatriz Martins de Carvalho 2021-04-07 10:15 ` Julia Lawall 0 siblings, 1 reply; 11+ messages in thread From: Beatriz Martins de Carvalho @ 2021-04-07 10:00 UTC (permalink / raw) To: Matthew Wilcox Cc: laurent.pinchart, mchehab, gregkh, linux-media, linux-staging, linux-kernel, outreachy-kernel Em 01/04/21 16:28, Matthew Wilcox escreveu: > On Thu, Apr 01, 2021 at 04:07:38PM +0100, Beatriz Martins de Carvalho wrote: >> diff --git a/drivers/staging/media/omap4iss/iss.c b/drivers/staging/media/omap4iss/iss.c >> index dae9073e7d3c..e8f724dbf810 100644 >> --- a/drivers/staging/media/omap4iss/iss.c >> +++ b/drivers/staging/media/omap4iss/iss.c >> @@ -559,9 +559,10 @@ static int iss_reset(struct iss_device *iss) >> iss_reg_set(iss, OMAP4_ISS_MEM_TOP, ISS_HL_SYSCONFIG, >> ISS_HL_SYSCONFIG_SOFTRESET); >> >> - timeout = iss_poll_condition_timeout( >> - !(iss_reg_read(iss, OMAP4_ISS_MEM_TOP, ISS_HL_SYSCONFIG) & >> - ISS_HL_SYSCONFIG_SOFTRESET), 1000, 10, 100); >> + timeout = iss_poll_condition_timeout(!(iss_reg_read(iss, >> + OMAP4_ISS_MEM_TOP, ISS_HL_SYSCONFIG) >> + & ISS_HL_SYSCONFIG_SOFTRESET), >> + 1000, 10, 100); > This is not a readability improvment. I would factor it out into its > own function. Thanks for the review. How can I do this? I don't know how to do this. Beatriz Martins de Carvalho ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] [PATCH 1/2] staging: media: omap4iss: Ending line with argument 2021-04-07 10:00 ` Beatriz Martins de Carvalho @ 2021-04-07 10:15 ` Julia Lawall 0 siblings, 0 replies; 11+ messages in thread From: Julia Lawall @ 2021-04-07 10:15 UTC (permalink / raw) To: Beatriz Martins de Carvalho Cc: Matthew Wilcox, laurent.pinchart, mchehab, gregkh, linux-media, linux-staging, linux-kernel, outreachy-kernel On Wed, 7 Apr 2021, Beatriz Martins de Carvalho wrote: > > Em 01/04/21 16:28, Matthew Wilcox escreveu: > > On Thu, Apr 01, 2021 at 04:07:38PM +0100, Beatriz Martins de Carvalho wrote: > > > diff --git a/drivers/staging/media/omap4iss/iss.c > > > b/drivers/staging/media/omap4iss/iss.c > > > index dae9073e7d3c..e8f724dbf810 100644 > > > --- a/drivers/staging/media/omap4iss/iss.c > > > +++ b/drivers/staging/media/omap4iss/iss.c > > > @@ -559,9 +559,10 @@ static int iss_reset(struct iss_device *iss) > > > iss_reg_set(iss, OMAP4_ISS_MEM_TOP, ISS_HL_SYSCONFIG, > > > ISS_HL_SYSCONFIG_SOFTRESET); > > > - timeout = iss_poll_condition_timeout( > > > - !(iss_reg_read(iss, OMAP4_ISS_MEM_TOP, ISS_HL_SYSCONFIG) & > > > - ISS_HL_SYSCONFIG_SOFTRESET), 1000, 10, 100); > > > + timeout = iss_poll_condition_timeout(!(iss_reg_read(iss, > > > + OMAP4_ISS_MEM_TOP, > > > ISS_HL_SYSCONFIG) > > > + & > > > ISS_HL_SYSCONFIG_SOFTRESET), > > > + 1000, 10, 100); > > This is not a readability improvment. I would factor it out into its > > own function. > > Thanks for the review. How can I do this? I don't know how to do this. Copy the code into a new function. Then see what parameters this function needs for the various information it requires. The code will produce some results that are needed by the rest of the program. So you have to arrange via the return value that the proper variables are initialized after the function call. For example, x = a + b; could become int f(int a, int b) { return a + b; } x = f(a,b); That is a pretty silly change, but it gives the idea. julia > Beatriz Martins de Carvalho > > -- > 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/e175859f-4f68-0408-415f-a3e19f7e4874%40gmail.com. > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] staging: media: omap4iss: Ending line with argument 2021-04-01 15:07 ` [PATCH 1/2] staging: media: omap4iss: Ending line with argument Beatriz Martins de Carvalho 2021-04-01 15:28 ` [Outreachy kernel] " Matthew Wilcox @ 2021-04-07 13:46 ` Hans Verkuil 2021-04-07 14:19 ` Beatriz Martins de Carvalho 1 sibling, 1 reply; 11+ messages in thread From: Hans Verkuil @ 2021-04-07 13:46 UTC (permalink / raw) To: Beatriz Martins de Carvalho, laurent.pinchart, mchehab, gregkh, linux-media, linux-staging, linux-kernel, outreachy-kernel Hi Beatriz, I'm now reviewing staging/media patches instead of Greg KH. See Vaishali's email from today: "Sending patches for the drivers/staging/media". On 01/04/2021 17:07, Beatriz Martins de Carvalho wrote: > Remove checkpatch check "CHECK: Lines should not end with a '('" with > argument present in next line and reorganize characters so the lines > are under 100 columns > > Signed-off-by: Beatriz Martins de Carvalho <martinsdecarvalhobeatriz@gmail.com> > --- > drivers/staging/media/omap4iss/iss.c | 46 +++++++++++++--------------- > 1 file changed, 22 insertions(+), 24 deletions(-) > > diff --git a/drivers/staging/media/omap4iss/iss.c b/drivers/staging/media/omap4iss/iss.c > index dae9073e7d3c..e8f724dbf810 100644 > --- a/drivers/staging/media/omap4iss/iss.c > +++ b/drivers/staging/media/omap4iss/iss.c > @@ -559,9 +559,10 @@ static int iss_reset(struct iss_device *iss) > iss_reg_set(iss, OMAP4_ISS_MEM_TOP, ISS_HL_SYSCONFIG, > ISS_HL_SYSCONFIG_SOFTRESET); > > - timeout = iss_poll_condition_timeout( > - !(iss_reg_read(iss, OMAP4_ISS_MEM_TOP, ISS_HL_SYSCONFIG) & > - ISS_HL_SYSCONFIG_SOFTRESET), 1000, 10, 100); > + timeout = iss_poll_condition_timeout(!(iss_reg_read(iss, > + OMAP4_ISS_MEM_TOP, ISS_HL_SYSCONFIG) > + & ISS_HL_SYSCONFIG_SOFTRESET), > + 1000, 10, 100); > if (timeout) { > dev_err(iss->dev, "ISS reset timeout\n"); > return -ETIMEDOUT; > @@ -583,9 +584,10 @@ static int iss_isp_reset(struct iss_device *iss) > > iss_reg_set(iss, OMAP4_ISS_MEM_ISP_SYS1, ISP5_CTRL, ISP5_CTRL_MSTANDBY); > > - timeout = iss_poll_condition_timeout( > - iss_reg_read(iss, OMAP4_ISS_MEM_ISP_SYS1, ISP5_CTRL) & > - ISP5_CTRL_MSTANDBY_WAIT, 1000000, 1000, 1500); > + timeout = iss_poll_condition_timeout(iss_reg_read(iss, > + OMAP4_ISS_MEM_ISP_SYS1, ISP5_CTRL) > + & ISP5_CTRL_MSTANDBY_WAIT, 1000000, > + 1000, 1500); > if (timeout) { > dev_err(iss->dev, "ISP5 standby timeout\n"); > return -ETIMEDOUT; > @@ -595,9 +597,10 @@ static int iss_isp_reset(struct iss_device *iss) > iss_reg_set(iss, OMAP4_ISS_MEM_ISP_SYS1, ISP5_SYSCONFIG, > ISP5_SYSCONFIG_SOFTRESET); > > - timeout = iss_poll_condition_timeout( > - !(iss_reg_read(iss, OMAP4_ISS_MEM_ISP_SYS1, ISP5_SYSCONFIG) & > - ISP5_SYSCONFIG_SOFTRESET), 1000000, 1000, 1500); > + timeout = iss_poll_condition_timeout(!(iss_reg_read(iss, OMAP4_ISS_MEM_ISP_SYS1, > + ISP5_SYSCONFIG) & > + ISP5_SYSCONFIG_SOFTRESET), 1000000, > + 1000, 1500); As several other people already commented, these changes do not improve readability. Just leave this code alone, it's good enough. Even splitting up the condition into a separate function would degrade readability since that would make it harder to discover the exact condition that will be polled for. Not everything that checkpatch.pl flags is necessarily bad code :-) > if (timeout) { > dev_err(iss->dev, "ISP5 reset timeout\n"); > return -ETIMEDOUT; > @@ -1104,33 +1107,28 @@ static int iss_create_links(struct iss_device *iss) > } > > /* Connect the submodules. */ > - ret = media_create_pad_link( > - &iss->csi2a.subdev.entity, CSI2_PAD_SOURCE, > - &iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0); > + ret = media_create_pad_link(&iss->csi2a.subdev.entity, CSI2_PAD_SOURCE, > + &iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0); > if (ret < 0) > return ret; > > - ret = media_create_pad_link( > - &iss->csi2b.subdev.entity, CSI2_PAD_SOURCE, > - &iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0); > + ret = media_create_pad_link(&iss->csi2b.subdev.entity, CSI2_PAD_SOURCE, > + &iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0); > if (ret < 0) > return ret; > > - ret = media_create_pad_link( > - &iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP, > - &iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0); > + ret = media_create_pad_link(&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP, > + &iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0); > if (ret < 0) > return ret; > > - ret = media_create_pad_link( > - &iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP, > - &iss->ipipe.subdev.entity, IPIPE_PAD_SINK, 0); > + ret = media_create_pad_link(&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP, > + &iss->ipipe.subdev.entity, IPIPE_PAD_SINK, 0); > if (ret < 0) > return ret; > > - ret = media_create_pad_link( > - &iss->ipipe.subdev.entity, IPIPE_PAD_SOURCE_VP, > - &iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0); > + ret = media_create_pad_link(&iss->ipipe.subdev.entity, IPIPE_PAD_SOURCE_VP, > + &iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0); > if (ret < 0) > return ret; > > These, however, are readability improvements, so I'm happy with that. Regards, Hans ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] staging: media: omap4iss: Ending line with argument 2021-04-07 13:46 ` Hans Verkuil @ 2021-04-07 14:19 ` Beatriz Martins de Carvalho 0 siblings, 0 replies; 11+ messages in thread From: Beatriz Martins de Carvalho @ 2021-04-07 14:19 UTC (permalink / raw) To: Hans Verkuil, laurent.pinchart, mchehab, gregkh, linux-media, linux-staging, linux-kernel, outreachy-kernel Em 07/04/21 14:46, Hans Verkuil escreveu: > Hi Beatriz, Hi, > > I'm now reviewing staging/media patches instead of Greg KH. See Vaishali's > email from today: "Sending patches for the drivers/staging/media". Yes, I saw the email! Thanks for helping us! > > On 01/04/2021 17:07, Beatriz Martins de Carvalho wrote: >> Remove checkpatch check "CHECK: Lines should not end with a '('" with >> argument present in next line and reorganize characters so the lines >> are under 100 columns >> >> Signed-off-by: Beatriz Martins de Carvalho <martinsdecarvalhobeatriz@gmail.com> >> --- >> drivers/staging/media/omap4iss/iss.c | 46 +++++++++++++--------------- >> 1 file changed, 22 insertions(+), 24 deletions(-) >> >> diff --git a/drivers/staging/media/omap4iss/iss.c b/drivers/staging/media/omap4iss/iss.c >> index dae9073e7d3c..e8f724dbf810 100644 >> --- a/drivers/staging/media/omap4iss/iss.c >> +++ b/drivers/staging/media/omap4iss/iss.c >> @@ -559,9 +559,10 @@ static int iss_reset(struct iss_device *iss) >> iss_reg_set(iss, OMAP4_ISS_MEM_TOP, ISS_HL_SYSCONFIG, >> ISS_HL_SYSCONFIG_SOFTRESET); >> >> - timeout = iss_poll_condition_timeout( >> - !(iss_reg_read(iss, OMAP4_ISS_MEM_TOP, ISS_HL_SYSCONFIG) & >> - ISS_HL_SYSCONFIG_SOFTRESET), 1000, 10, 100); >> + timeout = iss_poll_condition_timeout(!(iss_reg_read(iss, >> + OMAP4_ISS_MEM_TOP, ISS_HL_SYSCONFIG) >> + & ISS_HL_SYSCONFIG_SOFTRESET), >> + 1000, 10, 100); >> if (timeout) { >> dev_err(iss->dev, "ISS reset timeout\n"); >> return -ETIMEDOUT; >> @@ -583,9 +584,10 @@ static int iss_isp_reset(struct iss_device *iss) >> >> iss_reg_set(iss, OMAP4_ISS_MEM_ISP_SYS1, ISP5_CTRL, ISP5_CTRL_MSTANDBY); >> >> - timeout = iss_poll_condition_timeout( >> - iss_reg_read(iss, OMAP4_ISS_MEM_ISP_SYS1, ISP5_CTRL) & >> - ISP5_CTRL_MSTANDBY_WAIT, 1000000, 1000, 1500); >> + timeout = iss_poll_condition_timeout(iss_reg_read(iss, >> + OMAP4_ISS_MEM_ISP_SYS1, ISP5_CTRL) >> + & ISP5_CTRL_MSTANDBY_WAIT, 1000000, >> + 1000, 1500); >> if (timeout) { >> dev_err(iss->dev, "ISP5 standby timeout\n"); >> return -ETIMEDOUT; >> @@ -595,9 +597,10 @@ static int iss_isp_reset(struct iss_device *iss) >> iss_reg_set(iss, OMAP4_ISS_MEM_ISP_SYS1, ISP5_SYSCONFIG, >> ISP5_SYSCONFIG_SOFTRESET); >> >> - timeout = iss_poll_condition_timeout( >> - !(iss_reg_read(iss, OMAP4_ISS_MEM_ISP_SYS1, ISP5_SYSCONFIG) & >> - ISP5_SYSCONFIG_SOFTRESET), 1000000, 1000, 1500); >> + timeout = iss_poll_condition_timeout(!(iss_reg_read(iss, OMAP4_ISS_MEM_ISP_SYS1, >> + ISP5_SYSCONFIG) & >> + ISP5_SYSCONFIG_SOFTRESET), 1000000, >> + 1000, 1500); > As several other people already commented, these changes do not improve readability. > Just leave this code alone, it's good enough. Even splitting up the condition into > a separate function would degrade readability since that would make it harder to > discover the exact condition that will be polled for. > > Not everything that checkpatch.pl flags is necessarily bad code :-) Yes, I learning about this. And I will using this for the next patches. > >> if (timeout) { >> dev_err(iss->dev, "ISP5 reset timeout\n"); >> return -ETIMEDOUT; >> @@ -1104,33 +1107,28 @@ static int iss_create_links(struct iss_device *iss) >> } >> >> /* Connect the submodules. */ >> - ret = media_create_pad_link( >> - &iss->csi2a.subdev.entity, CSI2_PAD_SOURCE, >> - &iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0); >> + ret = media_create_pad_link(&iss->csi2a.subdev.entity, CSI2_PAD_SOURCE, >> + &iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0); >> if (ret < 0) >> return ret; >> >> - ret = media_create_pad_link( >> - &iss->csi2b.subdev.entity, CSI2_PAD_SOURCE, >> - &iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0); >> + ret = media_create_pad_link(&iss->csi2b.subdev.entity, CSI2_PAD_SOURCE, >> + &iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0); >> if (ret < 0) >> return ret; >> >> - ret = media_create_pad_link( >> - &iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP, >> - &iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0); >> + ret = media_create_pad_link(&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP, >> + &iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0); >> if (ret < 0) >> return ret; >> >> - ret = media_create_pad_link( >> - &iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP, >> - &iss->ipipe.subdev.entity, IPIPE_PAD_SINK, 0); >> + ret = media_create_pad_link(&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP, >> + &iss->ipipe.subdev.entity, IPIPE_PAD_SINK, 0); >> if (ret < 0) >> return ret; >> >> - ret = media_create_pad_link( >> - &iss->ipipe.subdev.entity, IPIPE_PAD_SOURCE_VP, >> - &iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0); >> + ret = media_create_pad_link(&iss->ipipe.subdev.entity, IPIPE_PAD_SOURCE_VP, >> + &iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0); >> if (ret < 0) >> return ret; >> >> > These, however, are readability improvements, so I'm happy with that. Thank! > > Regards, > > Hans Best wishes, Beatriz Carvalho ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] staging: media: omap4iss: align arguments with open parenthesis 2021-04-01 15:07 [PATCH 0/2] staging: media: omap4iss: Patchsets cleans up in iss.c Beatriz Martins de Carvalho 2021-04-01 15:07 ` [PATCH 1/2] staging: media: omap4iss: Ending line with argument Beatriz Martins de Carvalho @ 2021-04-01 15:07 ` Beatriz Martins de Carvalho 2021-04-01 15:29 ` [Outreachy kernel] " Matthew Wilcox 2021-04-07 15:23 ` Hans Verkuil 2021-04-01 15:25 ` [PATCH 0/2] staging: media: omap4iss: Patchsets cleans up in iss.c Greg KH 2 siblings, 2 replies; 11+ messages in thread From: Beatriz Martins de Carvalho @ 2021-04-01 15:07 UTC (permalink / raw) To: laurent.pinchart, mchehab, gregkh, linux-media, linux-staging, linux-kernel, outreachy-kernel Cleans up checks of "Alignment should match open parenthesis" in iss.c:96. Signed-off-by: Beatriz Martins de Carvalho <martinsdecarvalhobeatriz@gmail.com> --- drivers/staging/media/omap4iss/iss.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/media/omap4iss/iss.c b/drivers/staging/media/omap4iss/iss.c index e8f724dbf810..472f1837632e 100644 --- a/drivers/staging/media/omap4iss/iss.c +++ b/drivers/staging/media/omap4iss/iss.c @@ -963,7 +963,7 @@ iss_register_subdev_group(struct iss_device *iss, } subdev = v4l2_i2c_new_subdev_board(&iss->v4l2_dev, adapter, - board_info->board_info, NULL); + board_info->board_info, NULL); if (!subdev) { dev_err(iss->dev, "Unable to register subdev %s\n", board_info->board_info->type); -- 2.25.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] [PATCH 2/2] staging: media: omap4iss: align arguments with open parenthesis 2021-04-01 15:07 ` [PATCH 2/2] staging: media: omap4iss: align arguments with open parenthesis Beatriz Martins de Carvalho @ 2021-04-01 15:29 ` Matthew Wilcox 2021-04-07 15:23 ` Hans Verkuil 1 sibling, 0 replies; 11+ messages in thread From: Matthew Wilcox @ 2021-04-01 15:29 UTC (permalink / raw) To: Beatriz Martins de Carvalho Cc: laurent.pinchart, mchehab, gregkh, linux-media, linux-staging, linux-kernel, outreachy-kernel On Thu, Apr 01, 2021 at 04:07:39PM +0100, Beatriz Martins de Carvalho wrote: > subdev = v4l2_i2c_new_subdev_board(&iss->v4l2_dev, adapter, > - board_info->board_info, NULL); > + board_info->board_info, NULL); Also not a readability improvement. This checkpatch warning should be disregarded. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] staging: media: omap4iss: align arguments with open parenthesis 2021-04-01 15:07 ` [PATCH 2/2] staging: media: omap4iss: align arguments with open parenthesis Beatriz Martins de Carvalho 2021-04-01 15:29 ` [Outreachy kernel] " Matthew Wilcox @ 2021-04-07 15:23 ` Hans Verkuil 1 sibling, 0 replies; 11+ messages in thread From: Hans Verkuil @ 2021-04-07 15:23 UTC (permalink / raw) To: Beatriz Martins de Carvalho, laurent.pinchart, mchehab, gregkh, linux-media, linux-staging, linux-kernel, outreachy-kernel Hi Beatriz, On 01/04/2021 17:07, Beatriz Martins de Carvalho wrote: > Cleans up checks of "Alignment should match open parenthesis" > in iss.c:96. Looks good. I'll take this patch. You can use this reference to record your contribution: https://patchwork.linuxtv.org/project/linux-media/patch/475dbbe5774cbfed2d924807d8a3cfeb84b3d845.1617287509.git.martinsdecarvalhobeatriz@gmail.com/ The state will change to Accepted once it is merged in our media tree here: https://git.linuxtv.org/media_tree.git/log/ Regards, Hans > > Signed-off-by: Beatriz Martins de Carvalho <martinsdecarvalhobeatriz@gmail.com> > --- > drivers/staging/media/omap4iss/iss.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/media/omap4iss/iss.c b/drivers/staging/media/omap4iss/iss.c > index e8f724dbf810..472f1837632e 100644 > --- a/drivers/staging/media/omap4iss/iss.c > +++ b/drivers/staging/media/omap4iss/iss.c > @@ -963,7 +963,7 @@ iss_register_subdev_group(struct iss_device *iss, > } > > subdev = v4l2_i2c_new_subdev_board(&iss->v4l2_dev, adapter, > - board_info->board_info, NULL); > + board_info->board_info, NULL); > if (!subdev) { > dev_err(iss->dev, "Unable to register subdev %s\n", > board_info->board_info->type); > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] staging: media: omap4iss: Patchsets cleans up in iss.c 2021-04-01 15:07 [PATCH 0/2] staging: media: omap4iss: Patchsets cleans up in iss.c Beatriz Martins de Carvalho 2021-04-01 15:07 ` [PATCH 1/2] staging: media: omap4iss: Ending line with argument Beatriz Martins de Carvalho 2021-04-01 15:07 ` [PATCH 2/2] staging: media: omap4iss: align arguments with open parenthesis Beatriz Martins de Carvalho @ 2021-04-01 15:25 ` Greg KH 2 siblings, 0 replies; 11+ messages in thread From: Greg KH @ 2021-04-01 15:25 UTC (permalink / raw) To: Beatriz Martins de Carvalho Cc: laurent.pinchart, mchehab, linux-media, linux-staging, linux-kernel, outreachy-kernel On Thu, Apr 01, 2021 at 04:07:37PM +0100, Beatriz Martins de Carvalho wrote: > Clean up checks of "Alignment should match open parenthesis" and > "CHECK: Lines should not end with a '('" in iss.c > > Beatriz Martins de Carvalho (2): > staging: media: omap4iss: Ending line with argument > staging: media: omap4iss: align arguments with open parenthesis > > drivers/staging/media/omap4iss/iss.c | 48 +++++++++++++--------------- Please read the outreachy introduction email again :( thanks, greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-04-07 15:23 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-04-01 15:07 [PATCH 0/2] staging: media: omap4iss: Patchsets cleans up in iss.c Beatriz Martins de Carvalho 2021-04-01 15:07 ` [PATCH 1/2] staging: media: omap4iss: Ending line with argument Beatriz Martins de Carvalho 2021-04-01 15:28 ` [Outreachy kernel] " Matthew Wilcox 2021-04-07 10:00 ` Beatriz Martins de Carvalho 2021-04-07 10:15 ` Julia Lawall 2021-04-07 13:46 ` Hans Verkuil 2021-04-07 14:19 ` Beatriz Martins de Carvalho 2021-04-01 15:07 ` [PATCH 2/2] staging: media: omap4iss: align arguments with open parenthesis Beatriz Martins de Carvalho 2021-04-01 15:29 ` [Outreachy kernel] " Matthew Wilcox 2021-04-07 15:23 ` Hans Verkuil 2021-04-01 15:25 ` [PATCH 0/2] staging: media: omap4iss: Patchsets cleans up in iss.c Greg KH
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.