* [PATCH] staging: octeon-usb: Fix 'should not end with '(''
@ 2022-02-06 7:46 joelcchangg
2022-02-06 14:16 ` David Laight
0 siblings, 1 reply; 6+ messages in thread
From: joelcchangg @ 2022-02-06 7:46 UTC (permalink / raw)
To: linux-staging, linux-kernel, gregkh; +Cc: Joel Camilo Chang Gonzalez
From: Joel Camilo Chang Gonzalez <joelcchangg@gmail.com>
This patch fixes the checkpatch.pl check:
CHECK: Line should not end with '('
at lines 1802, 1807 and at some function headers.
Signed-off-by: Joel Camilo Chang Gonzalez <joelcchangg@gmail.com>
---
drivers/staging/octeon-usb/octeon-hcd.c | 67 +++++++++++--------------
1 file changed, 29 insertions(+), 38 deletions(-)
diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c
index a1cd81d4a114..60e8354104e6 100644
--- a/drivers/staging/octeon-usb/octeon-hcd.c
+++ b/drivers/staging/octeon-usb/octeon-hcd.c
@@ -1792,23 +1792,18 @@ static void cvmx_usb_start_channel(struct octeon_hcd *usb, int channel,
case CVMX_USB_TRANSFER_INTERRUPT:
break;
case CVMX_USB_TRANSFER_ISOCHRONOUS:
- if (!cvmx_usb_pipe_needs_split(usb, pipe)) {
+ if (!cvmx_usb_pipe_needs_split(usb, pipe) &&
+ pipe->transfer_dir == CVMX_USB_DIRECTION_OUT) {
/*
* ISO transactions require different PIDs depending on
* direction and how many packets are needed
*/
- if (pipe->transfer_dir == CVMX_USB_DIRECTION_OUT) {
- if (pipe->multi_count < 2) /* Need DATA0 */
- USB_SET_FIELD32(
- CVMX_USBCX_HCTSIZX(channel,
- usb->index),
+ if (pipe->multi_count < 2) /* Need DATA0 */
+ USB_SET_FIELD32(CVMX_USBCX_HCTSIZX(channel, usb->index),
cvmx_usbcx_hctsizx, pid, 0);
- else /* Need MDATA */
- USB_SET_FIELD32(
- CVMX_USBCX_HCTSIZX(channel,
- usb->index),
+ else /* Need MDATA */
+ USB_SET_FIELD32(CVMX_USBCX_HCTSIZX(channel, usb->index),
cvmx_usbcx_hctsizx, pid, 3);
- }
}
break;
}
@@ -2121,17 +2116,17 @@ static void cvmx_usb_complete(struct octeon_hcd *usb,
*
* Returns: Transaction or NULL on failure.
*/
-static struct cvmx_usb_transaction *cvmx_usb_submit_transaction(
- struct octeon_hcd *usb,
- struct cvmx_usb_pipe *pipe,
- enum cvmx_usb_transfer type,
- u64 buffer,
- int buffer_length,
- u64 control_header,
- int iso_start_frame,
- int iso_number_packets,
- struct cvmx_usb_iso_packet *iso_packets,
- struct urb *urb)
+static struct cvmx_usb_transaction *cvmx_usb_submit_transaction(struct octeon_hcd *usb,
+ struct cvmx_usb_pipe *pipe,
+ enum cvmx_usb_transfer type,
+ u64 buffer,
+ int buffer_length,
+ u64 control_header,
+ int iso_start_frame,
+ int iso_number_packets,
+ struct cvmx_usb_iso_packet
+ *iso_packets,
+ struct urb *urb)
{
struct cvmx_usb_transaction *transaction;
@@ -2182,10 +2177,9 @@ static struct cvmx_usb_transaction *cvmx_usb_submit_transaction(
*
* Returns: A submitted transaction or NULL on failure.
*/
-static struct cvmx_usb_transaction *cvmx_usb_submit_bulk(
- struct octeon_hcd *usb,
- struct cvmx_usb_pipe *pipe,
- struct urb *urb)
+static struct cvmx_usb_transaction *cvmx_usb_submit_bulk(struct octeon_hcd *usb,
+ struct cvmx_usb_pipe *pipe,
+ struct urb *urb)
{
return cvmx_usb_submit_transaction(usb, pipe, CVMX_USB_TRANSFER_BULK,
urb->transfer_dma,
@@ -2206,10 +2200,9 @@ static struct cvmx_usb_transaction *cvmx_usb_submit_bulk(
*
* Returns: A submitted transaction or NULL on failure.
*/
-static struct cvmx_usb_transaction *cvmx_usb_submit_interrupt(
- struct octeon_hcd *usb,
- struct cvmx_usb_pipe *pipe,
- struct urb *urb)
+static struct cvmx_usb_transaction *cvmx_usb_submit_interrupt(struct octeon_hcd *usb,
+ struct cvmx_usb_pipe *pipe,
+ struct urb *urb)
{
return cvmx_usb_submit_transaction(usb, pipe,
CVMX_USB_TRANSFER_INTERRUPT,
@@ -2231,10 +2224,9 @@ static struct cvmx_usb_transaction *cvmx_usb_submit_interrupt(
*
* Returns: A submitted transaction or NULL on failure.
*/
-static struct cvmx_usb_transaction *cvmx_usb_submit_control(
- struct octeon_hcd *usb,
- struct cvmx_usb_pipe *pipe,
- struct urb *urb)
+static struct cvmx_usb_transaction *cvmx_usb_submit_control(struct octeon_hcd *usb,
+ struct cvmx_usb_pipe *pipe,
+ struct urb *urb)
{
int buffer_length = urb->transfer_buffer_length;
u64 control_header = urb->setup_dma;
@@ -2262,10 +2254,9 @@ static struct cvmx_usb_transaction *cvmx_usb_submit_control(
*
* Returns: A submitted transaction or NULL on failure.
*/
-static struct cvmx_usb_transaction *cvmx_usb_submit_isochronous(
- struct octeon_hcd *usb,
- struct cvmx_usb_pipe *pipe,
- struct urb *urb)
+static struct cvmx_usb_transaction *cvmx_usb_submit_isochronous(struct octeon_hcd *usb,
+ struct cvmx_usb_pipe *pipe,
+ struct urb *urb)
{
struct cvmx_usb_iso_packet *packets;
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH] staging: octeon-usb: Fix 'should not end with '(''
2022-02-06 7:46 [PATCH] staging: octeon-usb: Fix 'should not end with '('' joelcchangg
@ 2022-02-06 14:16 ` David Laight
2022-02-06 17:17 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: David Laight @ 2022-02-06 14:16 UTC (permalink / raw)
To: 'joelcchangg@gmail.com', linux-staging, linux-kernel, gregkh
From: joelcchangg@gmail.com
> Sent: 06 February 2022 07:46
>
> This patch fixes the checkpatch.pl check:
>
> CHECK: Line should not end with '('
>
> at lines 1802, 1807 and at some function headers.
>
> Signed-off-by: Joel Camilo Chang Gonzalez <joelcchangg@gmail.com>
> ---
> drivers/staging/octeon-usb/octeon-hcd.c | 67 +++++++++++--------------
> 1 file changed, 29 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c
> index a1cd81d4a114..60e8354104e6 100644
> --- a/drivers/staging/octeon-usb/octeon-hcd.c
> +++ b/drivers/staging/octeon-usb/octeon-hcd.c
> @@ -1792,23 +1792,18 @@ static void cvmx_usb_start_channel(struct octeon_hcd *usb, int channel,
> case CVMX_USB_TRANSFER_INTERRUPT:
> break;
> case CVMX_USB_TRANSFER_ISOCHRONOUS:
> - if (!cvmx_usb_pipe_needs_split(usb, pipe)) {
> + if (!cvmx_usb_pipe_needs_split(usb, pipe) &&
> + pipe->transfer_dir == CVMX_USB_DIRECTION_OUT) {
> /*
> * ISO transactions require different PIDs depending on
> * direction and how many packets are needed
> */
> - if (pipe->transfer_dir == CVMX_USB_DIRECTION_OUT) {
> - if (pipe->multi_count < 2) /* Need DATA0 */
> - USB_SET_FIELD32(
> - CVMX_USBCX_HCTSIZX(channel,
> - usb->index),
> + if (pipe->multi_count < 2) /* Need DATA0 */
> + USB_SET_FIELD32(CVMX_USBCX_HCTSIZX(channel, usb->index),
> cvmx_usbcx_hctsizx, pid, 0);
> - else /* Need MDATA */
> - USB_SET_FIELD32(
> - CVMX_USBCX_HCTSIZX(channel,
> - usb->index),
> + else /* Need MDATA */
> + USB_SET_FIELD32(CVMX_USBCX_HCTSIZX(channel, usb->index),
> cvmx_usbcx_hctsizx, pid, 3);
> - }
> }
> break;
> }
What has that hunk got to do with the patch description?
> @@ -2121,17 +2116,17 @@ static void cvmx_usb_complete(struct octeon_hcd *usb,
> *
> * Returns: Transaction or NULL on failure.
> */
> -static struct cvmx_usb_transaction *cvmx_usb_submit_transaction(
> - struct octeon_hcd *usb,
> - struct cvmx_usb_pipe *pipe,
> - enum cvmx_usb_transfer type,
> - u64 buffer,
> - int buffer_length,
> - u64 control_header,
> - int iso_start_frame,
> - int iso_number_packets,
> - struct cvmx_usb_iso_packet *iso_packets,
> - struct urb *urb)
> +static struct cvmx_usb_transaction *cvmx_usb_submit_transaction(struct octeon_hcd *usb,
> + struct cvmx_usb_pipe *pipe,
> + enum cvmx_usb_transfer type,
> + u64 buffer,
> + int buffer_length,
> + u64 control_header,
> + int iso_start_frame,
> + int iso_number_packets,
> + struct cvmx_usb_iso_packet
> + *iso_packets,
> + struct urb *urb)
Those lines are now far longer than is 'nice'.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: octeon-usb: Fix 'should not end with '(''
2022-02-06 14:16 ` David Laight
@ 2022-02-06 17:17 ` Joe Perches
2022-02-06 19:50 ` Joe Perches
2022-02-07 1:57 ` Joel Chang
0 siblings, 2 replies; 6+ messages in thread
From: Joe Perches @ 2022-02-06 17:17 UTC (permalink / raw)
To: David Laight, 'joelcchangg@gmail.com',
linux-staging, linux-kernel, gregkh
On Sun, 2022-02-06 at 14:16 +0000, David Laight wrote:
> From: joelcchangg@gmail.com
> > Sent: 06 February 2022 07:46
> >
> > This patch fixes the checkpatch.pl check:
> >
> > CHECK: Line should not end with '('
[]
> > diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c
[]
> > @@ -1792,23 +1792,18 @@ static void cvmx_usb_start_channel(struct octeon_hcd *usb, int channel,
> > case CVMX_USB_TRANSFER_INTERRUPT:
> > break;
> > case CVMX_USB_TRANSFER_ISOCHRONOUS:
> > - if (!cvmx_usb_pipe_needs_split(usb, pipe)) {
> > + if (!cvmx_usb_pipe_needs_split(usb, pipe) &&
> > + pipe->transfer_dir == CVMX_USB_DIRECTION_OUT) {
> > /*
> > * ISO transactions require different PIDs depending on
> > * direction and how many packets are needed
> > */
> > - if (pipe->transfer_dir == CVMX_USB_DIRECTION_OUT) {
> > - if (pipe->multi_count < 2) /* Need DATA0 */
> > - USB_SET_FIELD32(
> > - CVMX_USBCX_HCTSIZX(channel,
> > - usb->index),
> > + if (pipe->multi_count < 2) /* Need DATA0 */
> > + USB_SET_FIELD32(CVMX_USBCX_HCTSIZX(channel, usb->index),
> > cvmx_usbcx_hctsizx, pid, 0);
> > - else /* Need MDATA */
> > - USB_SET_FIELD32(
> > - CVMX_USBCX_HCTSIZX(channel,
> > - usb->index),
> > + else /* Need MDATA */
> > + USB_SET_FIELD32(CVMX_USBCX_HCTSIZX(channel, usb->index),
> > cvmx_usbcx_hctsizx, pid, 3);
> > - }
> > }
> > break;
> > }
>
> What has that hunk got to do with the patch description?
Nothing and this should be revisited or explained much more
as it seems to change the logic.
> > @@ -2121,17 +2116,17 @@ static void cvmx_usb_complete(struct octeon_hcd *usb,
> > *
> > * Returns: Transaction or NULL on failure.
> > */
> > -static struct cvmx_usb_transaction *cvmx_usb_submit_transaction(
> > - struct octeon_hcd *usb,
> > - struct cvmx_usb_pipe *pipe,
> > - enum cvmx_usb_transfer type,
> > - u64 buffer,
> > - int buffer_length,
> > - u64 control_header,
> > - int iso_start_frame,
> > - int iso_number_packets,
> > - struct cvmx_usb_iso_packet *iso_packets,
> > - struct urb *urb)
> > +static struct cvmx_usb_transaction *cvmx_usb_submit_transaction(struct octeon_hcd *usb,
> > + struct cvmx_usb_pipe *pipe,
> > + enum cvmx_usb_transfer type,
> > + u64 buffer,
> > + int buffer_length,
> > + u64 control_header,
> > + int iso_start_frame,
> > + int iso_number_packets,
> > + struct cvmx_usb_iso_packet
> > + *iso_packets,
> > + struct urb *urb)
>
> Those lines are now far longer than is 'nice'.
True.
This _might_ be better with the return type on a separate line
Something like:
static struct cvmx_usb_transaction *
cvmx_usb_submit_transaction(struct octeon_hcd *usb, struct cvmx_usb_pipe *pipe,
enum cvmx_usb_transfer type,
u64 buffer, int buffer_length,
u64 control_header, int iso_start_frame,
int iso_number_packets,
struct cvmx_usb_iso_packet *iso_packets,
struct urb *urb)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: octeon-usb: Fix 'should not end with '(''
2022-02-06 17:17 ` Joe Perches
@ 2022-02-06 19:50 ` Joe Perches
2022-02-07 1:57 ` Joel Chang
1 sibling, 0 replies; 6+ messages in thread
From: Joe Perches @ 2022-02-06 19:50 UTC (permalink / raw)
To: David Laight, 'joelcchangg@gmail.com',
linux-staging, linux-kernel, gregkh
On Sun, 2022-02-06 at 09:17 -0800, Joe Perches wrote:
> On Sun, 2022-02-06 at 14:16 +0000, David Laight wrote:
> > From: joelcchangg@gmail.com
> > > Sent: 06 February 2022 07:46
[]
> This _might_ be better with the return type on a separate line
>
> Something like:
>
> static struct cvmx_usb_transaction *
> cvmx_usb_submit_transaction(struct octeon_hcd *usb, struct cvmx_usb_pipe *pipe,
> enum cvmx_usb_transfer type,
> u64 buffer, int buffer_length,
> u64 control_header, int iso_start_frame,
> int iso_number_packets,
> struct cvmx_usb_iso_packet *iso_packets,
> struct urb *urb)
As this is a static function only used by this compilation unit,
it's maybe even better removing the unnecessary cvmx_usb_ prefix too.
Maybe a bunch of unnecessary prefix removals...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: octeon-usb: Fix 'should not end with '(''
2022-02-06 17:17 ` Joe Perches
2022-02-06 19:50 ` Joe Perches
@ 2022-02-07 1:57 ` Joel Chang
2022-02-07 8:09 ` Dan Carpenter
1 sibling, 1 reply; 6+ messages in thread
From: Joel Chang @ 2022-02-07 1:57 UTC (permalink / raw)
To: Joe Perches; +Cc: David Laight, linux-staging, linux-kernel, gregkh
The hunk has two lines that end with '('. The hunk was rewritten but I
believe the logic remains the same.
I agree they are longer than is 'nice', but they are still under 100 chars.
Maybe I should submit 2 patches instead of one?
On Mon, Feb 7, 2022 at 1:17 AM Joe Perches <joe@perches.com> wrote:
>
> On Sun, 2022-02-06 at 14:16 +0000, David Laight wrote:
> > From: joelcchangg@gmail.com
> > > Sent: 06 February 2022 07:46
> > >
> > > This patch fixes the checkpatch.pl check:
> > >
> > > CHECK: Line should not end with '('
> []
> > > diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c
> []
> > > @@ -1792,23 +1792,18 @@ static void cvmx_usb_start_channel(struct octeon_hcd *usb, int channel,
> > > case CVMX_USB_TRANSFER_INTERRUPT:
> > > break;
> > > case CVMX_USB_TRANSFER_ISOCHRONOUS:
> > > - if (!cvmx_usb_pipe_needs_split(usb, pipe)) {
> > > + if (!cvmx_usb_pipe_needs_split(usb, pipe) &&
> > > + pipe->transfer_dir == CVMX_USB_DIRECTION_OUT) {
> > > /*
> > > * ISO transactions require different PIDs depending on
> > > * direction and how many packets are needed
> > > */
> > > - if (pipe->transfer_dir == CVMX_USB_DIRECTION_OUT) {
> > > - if (pipe->multi_count < 2) /* Need DATA0 */
> > > - USB_SET_FIELD32(
> > > - CVMX_USBCX_HCTSIZX(channel,
> > > - usb->index),
> > > + if (pipe->multi_count < 2) /* Need DATA0 */
> > > + USB_SET_FIELD32(CVMX_USBCX_HCTSIZX(channel, usb->index),
> > > cvmx_usbcx_hctsizx, pid, 0);
> > > - else /* Need MDATA */
> > > - USB_SET_FIELD32(
> > > - CVMX_USBCX_HCTSIZX(channel,
> > > - usb->index),
> > > + else /* Need MDATA */
> > > + USB_SET_FIELD32(CVMX_USBCX_HCTSIZX(channel, usb->index),
> > > cvmx_usbcx_hctsizx, pid, 3);
> > > - }
> > > }
> > > break;
> > > }
> >
> > What has that hunk got to do with the patch description?
>
> Nothing and this should be revisited or explained much more
> as it seems to change the logic.
>
> > > @@ -2121,17 +2116,17 @@ static void cvmx_usb_complete(struct octeon_hcd *usb,
> > > *
> > > * Returns: Transaction or NULL on failure.
> > > */
> > > -static struct cvmx_usb_transaction *cvmx_usb_submit_transaction(
> > > - struct octeon_hcd *usb,
> > > - struct cvmx_usb_pipe *pipe,
> > > - enum cvmx_usb_transfer type,
> > > - u64 buffer,
> > > - int buffer_length,
> > > - u64 control_header,
> > > - int iso_start_frame,
> > > - int iso_number_packets,
> > > - struct cvmx_usb_iso_packet *iso_packets,
> > > - struct urb *urb)
> > > +static struct cvmx_usb_transaction *cvmx_usb_submit_transaction(struct octeon_hcd *usb,
> > > + struct cvmx_usb_pipe *pipe,
> > > + enum cvmx_usb_transfer type,
> > > + u64 buffer,
> > > + int buffer_length,
> > > + u64 control_header,
> > > + int iso_start_frame,
> > > + int iso_number_packets,
> > > + struct cvmx_usb_iso_packet
> > > + *iso_packets,
> > > + struct urb *urb)
> >
> > Those lines are now far longer than is 'nice'.
>
> True.
>
> This _might_ be better with the return type on a separate line
>
> Something like:
>
> static struct cvmx_usb_transaction *
> cvmx_usb_submit_transaction(struct octeon_hcd *usb, struct cvmx_usb_pipe *pipe,
> enum cvmx_usb_transfer type,
> u64 buffer, int buffer_length,
> u64 control_header, int iso_start_frame,
> int iso_number_packets,
> struct cvmx_usb_iso_packet *iso_packets,
> struct urb *urb)
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: octeon-usb: Fix 'should not end with '(''
2022-02-07 1:57 ` Joel Chang
@ 2022-02-07 8:09 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2022-02-07 8:09 UTC (permalink / raw)
To: Joel Chang; +Cc: Joe Perches, David Laight, linux-staging, linux-kernel, gregkh
On Mon, Feb 07, 2022 at 09:57:34AM +0800, Joel Chang wrote:
> The hunk has two lines that end with '('. The hunk was rewritten but I
> believe the logic remains the same.
>
> I agree they are longer than is 'nice', but they are still under 100 chars.
Just ignore checkpatch when it tempts you to make the code worse.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-07 8:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-06 7:46 [PATCH] staging: octeon-usb: Fix 'should not end with '('' joelcchangg
2022-02-06 14:16 ` David Laight
2022-02-06 17:17 ` Joe Perches
2022-02-06 19:50 ` Joe Perches
2022-02-07 1:57 ` Joel Chang
2022-02-07 8:09 ` Dan Carpenter
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.