From mboxrd@z Thu Jan  1 00:00:00 1970
From: Oliver Neukum <oneukum@suse.com>
Subject: Re: [PATCH net-next 06/19] net: usb: aqc111: Introduce link
 management
Date: Mon, 08 Oct 2018 15:59:22 +0200
Message-ID: <1539007162.10342.19.camel@suse.com>
References: <cover.1538734658.git.igor.russkikh@aquantia.com>
         <cec5cd88988e0985e3fdd1343906ef649b01f646.1538734658.git.igor.russkikh@aquantia.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Cc: Dmitry Bezrukov <Dmitry.Bezrukov@aquantia.com>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>
To: Igor Russkikh <Igor.Russkikh@aquantia.com>,
        "David S . Miller" <davem@davemloft.net>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mx2.suse.de ([195.135.220.15]:53012 "EHLO mx1.suse.de"
        rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP
        id S1726056AbeJHVUE (ORCPT <rfc822;netdev@vger.kernel.org>);
        Mon, 8 Oct 2018 17:20:04 -0400
In-Reply-To: <cec5cd88988e0985e3fdd1343906ef649b01f646.1538734658.git.igor.russkikh@aquantia.com>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On Fr, 2018-10-05 at 10:24 +0000, Igor Russkikh wrote:
> From: Dmitry Bezrukov <dmitry.bezrukov@aquantia.com>
> 
> +static void aqc111_configure_rx(struct usbnet *dev,
> +				struct aqc111_data *aqc111_data)
> +{
> +	u8 reg8 = 0;
> +	u8 queue_num = 0;
> +	u16 reg16 = 0;
> +	u16 link_speed = 0, usb_host = 0;
> +	u8 buf[5] = { 0 };

DMA on stack.

> +	enum usb_device_speed usb_speed = dev->udev->speed;
> +
> +	buf[0] = 0x00;
> +	buf[1] = 0xF8;
> +	buf[2] = 0x07;
> +	switch (aqc111_data->link_speed) {
> +	case AQ_INT_SPEED_5G:
> +	{
> +		link_speed = 5000;
> +		reg8 = 0x05;
> +		reg16 = 0x001F;
> +		break;
> +	}
> +	case AQ_INT_SPEED_2_5G:
> +	{
> +		link_speed = 2500;
> +		reg16 = 0x003F;
> +		break;
> +	}
> +	case AQ_INT_SPEED_1G:
> +	{
> +		link_speed = 1000;
> +		reg16 = 0x009F;
> +		break;
> +	}
> +	case AQ_INT_SPEED_100M:
> +	{
> +		link_speed = 100;
> +		queue_num = 1;
> +		reg16 = 0x063F;
> +		buf[1] = 0xFB;
> +		buf[2] = 0x4;
> +		break;
> +	}
> +	}
> +
> +	if (aqc111_data->dpa) {
> +		/* Set Phy Flow control */
> +		aq_mdio_write_cmd(dev, AQ_GLB_ING_PAUSE_CTRL_REG,
> +				  AQ_PHY_AUTONEG_ADDR, 2, &reg16);
> +		aq_mdio_write_cmd(dev, AQ_GLB_EGR_PAUSE_CTRL_REG,
> +				  AQ_PHY_AUTONEG_ADDR, 2, &reg16);
> +	}
> +
> +	aqc111_write_cmd(dev, AQ_ACCESS_MAC, SFR_INTER_PACKET_GAP_0,
> +			 1, 1, &reg8);
> +
> +	aqc111_write_cmd(dev, AQ_ACCESS_MAC, SFR_TX_PAUSE_RESEND_T, 3, 3, buf);
> +
> +	switch (usb_speed) {

Needs to handle all speeds. And please add a comment explaining the
rationale.

> +	case USB_SPEED_SUPER:
> +	{
> +		usb_host = 3;
> +		break;
> +	}
> +	case USB_SPEED_HIGH:
> +	{
> +		usb_host = 2;
> +		break;
> +	}
> +	case USB_SPEED_FULL:
> +	case USB_SPEED_LOW:
> +	{
> +		usb_host = 1;
> +		queue_num = 0;
> +		break;
> +	}
> +	default:
> +	{
> +		usb_host = 0;
> +		break;
> +	}
> +	}
> +
> +	memcpy(buf, &AQC111_BULKIN_SIZE[queue_num], 5);
> +	/* RX bulk configuration */
> +	aqc111_write_cmd(dev, AQ_ACCESS_MAC, SFR_RX_BULKIN_QCTRL, 5, 5, buf);
> +
> +	/* Set high low water level */
> +	reg16 = 0x0810;
> +
> +	aqc111_write_cmd(dev, AQ_ACCESS_MAC, SFR_PAUSE_WATERLVL_LOW,
> +			 2, 2, &reg16);
> +	netdev_info(dev->net, "Link Speed %d, USB %d", link_speed, usb_host);
> +}

	Regards
		Oliver