From: =?utf-8?Q?Bj=C3=B8rn_Mork?= <bjorn@mork.no>
To: Kristian Evensen <kristian.evensen@gmail.com>
Cc: oliver@neukum.org, linux-usb@vger.kernel.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, henning.schild@siemens.com
Subject: Re: [PATCH net] cdc_ether: Fix handling connection notification
Organization: m
References: <20161130184216.4224-1-kristian.evensen@gmail.com>
Date: Wed, 30 Nov 2016 22:51:16 +0100
In-Reply-To: <20161130184216.4224-1-kristian.evensen@gmail.com> (Kristian
        Evensen's message of "Wed, 30 Nov 2016 19:42:16 +0100")
Message-ID: <87shq83acr.fsf@miraculix.mork.no>
User-Agent: Gnus/5.130015 (Ma Gnus v0.15) Emacs/24.5 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Sender: linux-kernel-owner@vger.kernel.org
Content-Transfer-Encoding: 8bit
Content-Length: 1092
Lines: 36

Kristian Evensen <kristian.evensen@gmail.com> writes:

> +void usbnet_cdc_zte_status(struct usbnet *dev, struct urb *urb)
> +{
> +	struct usb_cdc_notification *event;
> +
> +	if (urb->actual_length < sizeof(*event))
> +		return;
> +
> +	event = urb->transfer_buffer;
> +
> +	if (event->bNotificationType != USB_CDC_NOTIFY_NETWORK_CONNECTION) {
> +		usbnet_cdc_status(dev, urb);
> +		return;
> +	}
> +
> +	netif_dbg(dev, timer, dev->net, "CDC: carrier %s\n",
> +		  event->wValue ? "on" : "off");
> +
> +	if (event->wValue &&
> +	    !test_bit(__LINK_STATE_NOCARRIER, &dev->net->state))
> +		usbnet_link_change(dev, 0, 0);
> +
> +	usbnet_link_change(dev, !!event->wValue, 0);
> +}

As Henning said: Use netif_carrier_ok instead of open coding it.

But I also think you need to replace the first usbnet_link_change() with
a plain netif_carrier_off(dev->net).  Calling usbnet_link_change() twice
here is only going to set off the "kevent XX may have been dropped"
message since you call schedule_work() twice without giving the work
queue a chance to be processed.  No need to do that.


Bjørn