Fixing 80 character limit warnings in octeon/ethernet-rx.c
Signed-off-by: Luis de Bethencourt <[email protected]>
---
drivers/staging/octeon/ethernet-rx.c | 51 +++++++++++++++++++++++++-----------
1 file changed, 35 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
index 1789a12..e387eb1 100644
--- a/drivers/staging/octeon/ethernet-rx.c
+++ b/drivers/staging/octeon/ethernet-rx.c
@@ -109,6 +109,7 @@ static inline int cvm_oct_check_rcv_error(cvmx_wqe_t *work)
int interface = cvmx_helper_get_interface_num(work->ipprt);
int index = cvmx_helper_get_interface_index_num(work->ipprt);
union cvmx_gmxx_rxx_frm_ctl gmxx_rxx_frm_ctl;
+
gmxx_rxx_frm_ctl.u64 =
cvmx_read_csr(CVMX_GMXX_RXX_FRM_CTL(index, interface));
if (gmxx_rxx_frm_ctl.s.pre_chk == 0) {
@@ -126,13 +127,15 @@ static inline int cvm_oct_check_rcv_error(cvmx_wqe_t *work)
if (*ptr == 0xd5) {
/*
- printk_ratelimited("Port %d received 0xd5 preamble\n", work->ipprt);
+ printk_ratelimited("Port %d received 0xd5 preamble\n",
+ work->ipprt);
*/
work->packet_ptr.s.addr += i + 1;
work->len -= i + 5;
} else if ((*ptr & 0xf) == 0xd) {
/*
- printk_ratelimited("Port %d received 0x?d preamble\n", work->ipprt);
+ printk_ratelimited("Port %d received 0x?d preamble\n",
+ work->ipprt);
*/
work->packet_ptr.s.addr += i;
work->len -= i + 4;
@@ -212,17 +215,20 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
did_work_request = 0;
if (work == NULL) {
union cvmx_pow_wq_int wq_int;
+
wq_int.u64 = 0;
wq_int.s.iq_dis = 1 << pow_receive_group;
wq_int.s.wq_int = 1 << pow_receive_group;
cvmx_write_csr(CVMX_POW_WQ_INT, wq_int.u64);
break;
}
- pskb = (struct sk_buff **)(cvm_oct_get_buffer_ptr(work->packet_ptr) - sizeof(void *));
+ pskb = (struct sk_buff **)(cvm_oct_get_buffer_ptr(work->packet_ptr) -
+ sizeof(void *));
prefetch(pskb);
if (USE_ASYNC_IOBDMA && rx_count < (budget - 1)) {
- cvmx_pow_work_request_async_nocheck(CVMX_SCR_SCRATCH, CVMX_POW_NO_WAIT);
+ cvmx_pow_work_request_async_nocheck(CVMX_SCR_SCRATCH,
+ CVMX_POW_NO_WAIT);
did_work_request = 1;
}
rx_count++;
@@ -247,7 +253,8 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
* buffer.
*/
if (likely(skb_in_hw)) {
- skb->data = skb->head + work->packet_ptr.s.addr - cvmx_ptr_to_phys(skb->head);
+ skb->data = skb->head + work->packet_ptr.s.addr -
+ cvmx_ptr_to_phys(skb->head);
prefetch(skb->data);
skb->len = work->len;
skb_set_tail_pointer(skb, skb->len);
@@ -284,7 +291,8 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
/* No packet buffers to free */
} else {
int segments = work->word2.s.bufs;
- union cvmx_buf_ptr segment_ptr = work->packet_ptr;
+ union cvmx_buf_ptr segment_ptr =
+ work->packet_ptr;
int len = work->len;
while (segments--) {
@@ -300,8 +308,11 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
* one: int segment_size =
* segment_ptr.s.size;
*/
- int segment_size = CVMX_FPA_PACKET_POOL_SIZE -
- (segment_ptr.s.addr - (((segment_ptr.s.addr >> 7) - segment_ptr.s.back) << 7));
+ int segment_size =
+ CVMX_FPA_PACKET_POOL_SIZE -
+ (segment_ptr.s.addr -
+ (((segment_ptr.s.addr >> 7) -
+ segment_ptr.s.back) << 7));
/*
* Don't copy more than what
* is left in the packet.
@@ -332,8 +343,10 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
skb->protocol = eth_type_trans(skb, dev);
skb->dev = dev;
- if (unlikely(work->word2.s.not_IP || work->word2.s.IP_exc ||
- work->word2.s.L4_error || !work->word2.s.tcp_or_udp))
+ if (unlikely(work->word2.s.not_IP ||
+ work->word2.s.IP_exc ||
+ work->word2.s.L4_error ||
+ !work->word2.s.tcp_or_udp))
skb->ip_summed = CHECKSUM_NONE;
else
skb->ip_summed = CHECKSUM_UNNECESSARY;
@@ -341,11 +354,15 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
/* Increment RX stats for virtual ports */
if (work->ipprt >= CVMX_PIP_NUM_INPUT_PORTS) {
#ifdef CONFIG_64BIT
- atomic64_add(1, (atomic64_t *)&priv->stats.rx_packets);
- atomic64_add(skb->len, (atomic64_t *)&priv->stats.rx_bytes);
+ atomic64_add(1,
+ (atomic64_t *)&priv->stats.rx_packets);
+ atomic64_add(skb->len,
+ (atomic64_t *)&priv->stats.rx_bytes);
#else
- atomic_add(1, (atomic_t *)&priv->stats.rx_packets);
- atomic_add(skb->len, (atomic_t *)&priv->stats.rx_bytes);
+ atomic_add(1,
+ (atomic_t *)&priv->stats.rx_packets);
+ atomic_add(skb->len,
+ (atomic_t *)&priv->stats.rx_bytes);
#endif
}
netif_receive_skb(skb);
@@ -356,9 +373,11 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
dev->name);
*/
#ifdef CONFIG_64BIT
- atomic64_add(1, (atomic64_t *)&priv->stats.rx_dropped);
+ atomic64_add(1,
+ (atomic64_t *)&priv->stats.rx_dropped);
#else
- atomic_add(1, (atomic_t *)&priv->stats.rx_dropped);
+ atomic_add(1,
+ (atomic_t *)&priv->stats.rx_dropped);
#endif
dev_kfree_skb_irq(skb);
}
--
2.1.3
On Thu, 2014-11-27 at 17:18 +0100, Luis de Bethencourt wrote:
> Fixing 80 character limit warnings in octeon/ethernet-rx.c
Hello again Luis.
Another thing you might consider is to align
multiple line statements to the appropriate
open parenthesis.
> diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
[]
> @@ -126,13 +127,15 @@ static inline int cvm_oct_check_rcv_error(cvmx_wqe_t *work)
>
> if (*ptr == 0xd5) {
> /*
> - printk_ratelimited("Port %d received 0xd5 preamble\n", work->ipprt);
> + printk_ratelimited("Port %d received 0xd5 preamble\n",
> + work->ipprt);
> */
This is in a commented out block, but this
would look better like:
printk_ratelimited("format",
args...);
> @@ -212,17 +215,20 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
> did_work_request = 0;
> if (work == NULL) {
> union cvmx_pow_wq_int wq_int;
> +
> wq_int.u64 = 0;
> wq_int.s.iq_dis = 1 << pow_receive_group;
> wq_int.s.wq_int = 1 << pow_receive_group;
> cvmx_write_csr(CVMX_POW_WQ_INT, wq_int.u64);
> break;
> }
> - pskb = (struct sk_buff **)(cvm_oct_get_buffer_ptr(work->packet_ptr) - sizeof(void *));
> + pskb = (struct sk_buff **)(cvm_oct_get_buffer_ptr(work->packet_ptr) -
> + sizeof(void *));
cvm_oct_get_buffer_ptr returns a void pointer
so it doesn't need a cast.
a possible fix is just to remove the cast
pskb = cvm_oct_get_buffer_ptr(work->packet_ptr) - sizeof(void *);
> prefetch(pskb);
If the cast type above to a ** is to be believed,
maybe this should be prefetch(*pskb)
If you do these, they should be separate patches.
On Thu, Nov 27, 2014 at 08:34:00AM -0800, Joe Perches wrote:
> On Thu, 2014-11-27 at 17:18 +0100, Luis de Bethencourt wrote:
> > Fixing 80 character limit warnings in octeon/ethernet-rx.c
>
> Hello again Luis.
>
> Another thing you might consider is to align
> multiple line statements to the appropriate
> open parenthesis.
Sure :) I will do this and merge it into the patch in a few hours.
>
> > diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
> []
> > @@ -126,13 +127,15 @@ static inline int cvm_oct_check_rcv_error(cvmx_wqe_t *work)
> >
> > if (*ptr == 0xd5) {
> > /*
> > - printk_ratelimited("Port %d received 0xd5 preamble\n", work->ipprt);
> > + printk_ratelimited("Port %d received 0xd5 preamble\n",
> > + work->ipprt);
> > */
>
> This is in a commented out block, but this
> would look better like:
>
> printk_ratelimited("format",
> args...);
>
I agree. Thanks for the pointer.
>
> > @@ -212,17 +215,20 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
> > did_work_request = 0;
> > if (work == NULL) {
> > union cvmx_pow_wq_int wq_int;
> > +
> > wq_int.u64 = 0;
> > wq_int.s.iq_dis = 1 << pow_receive_group;
> > wq_int.s.wq_int = 1 << pow_receive_group;
> > cvmx_write_csr(CVMX_POW_WQ_INT, wq_int.u64);
> > break;
> > }
> > - pskb = (struct sk_buff **)(cvm_oct_get_buffer_ptr(work->packet_ptr) - sizeof(void *));
> > + pskb = (struct sk_buff **)(cvm_oct_get_buffer_ptr(work->packet_ptr) -
> > + sizeof(void *));
>
> cvm_oct_get_buffer_ptr returns a void pointer
> so it doesn't need a cast.
>
> a possible fix is just to remove the cast
>
> pskb = cvm_oct_get_buffer_ptr(work->packet_ptr) - sizeof(void *);
>
> > prefetch(pskb);
>
> If the cast type above to a ** is to be believed,
> maybe this should be prefetch(*pskb)
>
> If you do these, they should be separate patches.
>
Cool! Once I do the above I will fix this in a separate patch.
Thanks again for your time reviewing Joe,
Luis
On Thu, Nov 27, 2014 at 05:18:10PM +0100, Luis de Bethencourt wrote:
> Fixing 80 character limit warnings in octeon/ethernet-rx.c
>
> Signed-off-by: Luis de Bethencourt <[email protected]>
> ---
> drivers/staging/octeon/ethernet-rx.c | 51 +++++++++++++++++++++++++-----------
> 1 file changed, 35 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
> index 1789a12..e387eb1 100644
> --- a/drivers/staging/octeon/ethernet-rx.c
> +++ b/drivers/staging/octeon/ethernet-rx.c
> @@ -109,6 +109,7 @@ static inline int cvm_oct_check_rcv_error(cvmx_wqe_t *work)
> int interface = cvmx_helper_get_interface_num(work->ipprt);
> int index = cvmx_helper_get_interface_index_num(work->ipprt);
> union cvmx_gmxx_rxx_frm_ctl gmxx_rxx_frm_ctl;
> +
Do this in a separate patch.
> @@ -332,8 +343,10 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
> skb->protocol = eth_type_trans(skb, dev);
> skb->dev = dev;
>
> - if (unlikely(work->word2.s.not_IP || work->word2.s.IP_exc ||
> - work->word2.s.L4_error || !work->word2.s.tcp_or_udp))
> + if (unlikely(work->word2.s.not_IP ||
> + work->word2.s.IP_exc ||
> + work->word2.s.L4_error ||
> + !work->word2.s.tcp_or_udp))
> skb->ip_summed = CHECKSUM_NONE;
Your patch aligns everything with tabs but it should be slightly
different. Run your patch through checkpatch.pl --strict to see what I
mean and then redo it all. This one should look like:
if (unlikely(work->word2.s.not_IP ||
work->word2.s.IP_exc ||
work->word2.s.L4_error ||
!work->word2.s.tcp_or_udp))
skb->ip_summed = CHECKSUM_NONE;
[tab][tab][tab][tab][tab][space][space][space][space][space]work->...
regards,
dan carpenter
On Fri, Nov 28, 2014 at 01:29:28PM +0300, Dan Carpenter wrote:
> On Thu, Nov 27, 2014 at 05:18:10PM +0100, Luis de Bethencourt wrote:
> > Fixing 80 character limit warnings in octeon/ethernet-rx.c
> >
> > Signed-off-by: Luis de Bethencourt <[email protected]>
> > ---
> > drivers/staging/octeon/ethernet-rx.c | 51 +++++++++++++++++++++++++-----------
> > 1 file changed, 35 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
> > index 1789a12..e387eb1 100644
> > --- a/drivers/staging/octeon/ethernet-rx.c
> > +++ b/drivers/staging/octeon/ethernet-rx.c
> > @@ -109,6 +109,7 @@ static inline int cvm_oct_check_rcv_error(cvmx_wqe_t *work)
> > int interface = cvmx_helper_get_interface_num(work->ipprt);
> > int index = cvmx_helper_get_interface_index_num(work->ipprt);
> > union cvmx_gmxx_rxx_frm_ctl gmxx_rxx_frm_ctl;
> > +
>
> Do this in a separate patch.
Will do within the hour.
>
> > @@ -332,8 +343,10 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
> > skb->protocol = eth_type_trans(skb, dev);
> > skb->dev = dev;
> >
> > - if (unlikely(work->word2.s.not_IP || work->word2.s.IP_exc ||
> > - work->word2.s.L4_error || !work->word2.s.tcp_or_udp))
> > + if (unlikely(work->word2.s.not_IP ||
> > + work->word2.s.IP_exc ||
> > + work->word2.s.L4_error ||
> > + !work->word2.s.tcp_or_udp))
> > skb->ip_summed = CHECKSUM_NONE;
>
> Your patch aligns everything with tabs but it should be slightly
> different. Run your patch through checkpatch.pl --strict to see what I
> mean and then redo it all. This one should look like:
>
> if (unlikely(work->word2.s.not_IP ||
> work->word2.s.IP_exc ||
> work->word2.s.L4_error ||
> !work->word2.s.tcp_or_udp))
> skb->ip_summed = CHECKSUM_NONE;
>
> [tab][tab][tab][tab][tab][space][space][space][space][space]work->...
>
> regards,
> dan carpenter
>
>
Fixed the aligns and resubmitting the patch as v3.
Thanks for the review!
Luis
On Fri, Nov 28, 2014 at 02:30:34PM +0100, Luis de Bethencourt wrote:
> On Fri, Nov 28, 2014 at 01:29:28PM +0300, Dan Carpenter wrote:
> > On Thu, Nov 27, 2014 at 05:18:10PM +0100, Luis de Bethencourt wrote:
> > > Fixing 80 character limit warnings in octeon/ethernet-rx.c
> > >
> > > Signed-off-by: Luis de Bethencourt <[email protected]>
> > > ---
> > > drivers/staging/octeon/ethernet-rx.c | 51 +++++++++++++++++++++++++-----------
> > > 1 file changed, 35 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
> > > index 1789a12..e387eb1 100644
> > > --- a/drivers/staging/octeon/ethernet-rx.c
> > > +++ b/drivers/staging/octeon/ethernet-rx.c
> > > @@ -109,6 +109,7 @@ static inline int cvm_oct_check_rcv_error(cvmx_wqe_t *work)
> > > int interface = cvmx_helper_get_interface_num(work->ipprt);
> > > int index = cvmx_helper_get_interface_index_num(work->ipprt);
> > > union cvmx_gmxx_rxx_frm_ctl gmxx_rxx_frm_ctl;
> > > +
> >
> > Do this in a separate patch.
>
> Will do within the hour.
>
:( You should always wait overnight or a few hours before sending a
patch. It's not a race. Go slowly.
regards,
dan carpenter
On Fri, Nov 28, 2014 at 05:14:32PM +0300, Dan Carpenter wrote:
> On Fri, Nov 28, 2014 at 02:30:34PM +0100, Luis de Bethencourt wrote:
> > On Fri, Nov 28, 2014 at 01:29:28PM +0300, Dan Carpenter wrote:
> > > On Thu, Nov 27, 2014 at 05:18:10PM +0100, Luis de Bethencourt wrote:
> > > > Fixing 80 character limit warnings in octeon/ethernet-rx.c
> > > >
> > > > Signed-off-by: Luis de Bethencourt <[email protected]>
> > > > ---
> > > > drivers/staging/octeon/ethernet-rx.c | 51 +++++++++++++++++++++++++-----------
> > > > 1 file changed, 35 insertions(+), 16 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
> > > > index 1789a12..e387eb1 100644
> > > > --- a/drivers/staging/octeon/ethernet-rx.c
> > > > +++ b/drivers/staging/octeon/ethernet-rx.c
> > > > @@ -109,6 +109,7 @@ static inline int cvm_oct_check_rcv_error(cvmx_wqe_t *work)
> > > > int interface = cvmx_helper_get_interface_num(work->ipprt);
> > > > int index = cvmx_helper_get_interface_index_num(work->ipprt);
> > > > union cvmx_gmxx_rxx_frm_ctl gmxx_rxx_frm_ctl;
> > > > +
> > >
> > > Do this in a separate patch.
> >
> > Will do within the hour.
> >
>
> :( You should always wait overnight or a few hours before sending a
> patch. It's not a race. Go slowly.
>
> regards,
> dan carpenter
>
Is the wait so people have more time to review the patch as it is?
I wanted to show my interest by keeping up with reviews but now that you
mention that, it makes sense.
I screwed up and didn't submit the version 3 of this patch with [01/02] for the
character limit warnings. Will wait, since the patch for the empty lines after
variable declarations needs to be applied on-top of this. I will wait until that
one is accepted or rejected before rebasing and submitting the second patch.
Sorry about this.
Thanks for all the help :)
Luis