linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hayes Wang <hayeswang@realtek.com>
To: David Miller <davem@davemloft.net>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	nic_swsd <nic_swsd@realtek.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>
Subject: RE: [PATCH net-next 2/2] r8152: check the status before submittingrx
Date: Mon, 22 Dec 2014 02:53:42 +0000	[thread overview]
Message-ID: <0835B3720019904CB8F7AA43166CEEB2ED5A35@RTITMBSV03.realtek.com.tw> (raw)
In-Reply-To: <20141219.154402.54691026682141762.davem@davemloft.net>

 David Miller [mailto:davem@davemloft.net] 
> Sent: Saturday, December 20, 2014 4:44 AM
[...]
> > Don't submit the rx if the device is unplugged, linking down,
> > or stopped.
>  ...
> > @@ -1789,6 +1789,11 @@ int r8152_submit_rx(struct r8152 
> *tp, struct rx_agg *agg, gfp_t mem_flags)
> >  {
> >  	int ret;
> >  
> > +	/* The rx would be stopped, so skip submitting */
> > +	if (test_bit(RTL8152_UNPLUG, &tp->flags) ||
> > +	    !test_bit(WORK_ENABLE, &tp->flags) || !(tp->speed & LINK_STATUS))
> > +		return 0;
> > +
> 
> I think netif_carrier_off() should always be true in all three of those
> situations, and would be a much simpler test than what you've coded
> here.

When the device is unplugged or stopped, the linking status
may be true, so I add additional checks to avoid the submission.

Besides, in set_carrier() I set netif_carrier_on() after
ops.enable() to avoid any transmission before I finish
starting the tx/rx.

	tp->rtl_ops.enable(tp);
	set_bit(RTL8152_SET_RX_MODE, &tp->flags);
	netif_carrier_on(netdev);

However, the r8152_submit_rx() would be called in ops.enable(),
and the check of netif_carrier_ok() would be always false. That
is why I use tp->speed, not netif_carrier_ok(), to check the
linking stauts.
 
Best Regards,
Hayes

  reply	other threads:[~2014-12-22  2:53 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-19  8:55 [PATCH net-next 0/2] r8152: adjust r8152_submit_rx Hayes Wang
2014-12-19  8:55 ` [PATCH net-next 1/2] r8152: adjust set_carrier Hayes Wang
2014-12-19  8:56 ` [PATCH net-next 2/2] r8152: check the status before submitting rx Hayes Wang
2014-12-19 20:44   ` David Miller
2014-12-22  2:53     ` Hayes Wang [this message]
2014-12-22  5:22       ` [PATCH net-next 2/2] r8152: check the status before submittingrx David Miller
2014-12-22  6:52 ` [PATCH net-next v2 0/2] r8152: adjust r8152_submit_rx Hayes Wang
2014-12-22  6:52   ` [PATCH net-next v2 1/2] r8152: call rtl_start_rx after netif_carrier_on Hayes Wang
2014-12-22  6:52   ` [PATCH net-next v2 2/2] r8152: check the status before submitting rx Hayes Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0835B3720019904CB8F7AA43166CEEB2ED5A35@RTITMBSV03.realtek.com.tw \
    --to=hayeswang@realtek.com \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nic_swsd@realtek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).