| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 02 Apr 2018 10:45:40 +0100
From: Marc Zyngier <marc.zyngier@....com>
To: Alexander Kurz <akurz@...la.de>
Cc: "David S . Miller" <davem@...emloft.net>,
"Andrew F . Davis" <afd@...com>, <linux-usb@...r.kernel.org>,
<netdev@...r.kernel.org>, Freddy Xin <freddy@...x.com.tw>
Subject: Re: [PATCH v3 2/2] net: usb: asix88179_178a: de-duplicate code
On Mon, 02 Apr 2018 08:43:49 +0100,
Alexander Kurz wrote:
Alexander,
>
> Remove the duplicated code for asix88179_178a bind and reset methods.
>
> Signed-off-by: Alexander Kurz <akurz@...la.de>
> ---
> drivers/net/usb/ax88179_178a.c | 137 ++++++++++-------------------------------
> 1 file changed, 31 insertions(+), 106 deletions(-)
What has changed between this patch and the previous one? Having a bit
of a change-log would certainly help. Also, I would have appreciated a
reply to the questions I had on v2 before you posted a third version.
>
> diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
> index a6ef75907ae9..fea4c7b877cc 100644
> --- a/drivers/net/usb/ax88179_178a.c
> +++ b/drivers/net/usb/ax88179_178a.c
> @@ -1223,7 +1223,7 @@ static int ax88179_led_setting(struct usbnet *dev)
> return 0;
> }
>
> -static int ax88179_bind(struct usbnet *dev, struct usb_interface *intf)
> +static int ax88179_bind_or_reset(struct usbnet *dev, bool do_reset)
> {
> u8 buf[5];
> u16 *tmp16;
> @@ -1231,12 +1231,11 @@ static int ax88179_bind(struct usbnet *dev, struct usb_interface *intf)
> struct ax88179_data *ax179_data = (struct ax88179_data *)dev->data;
> struct ethtool_eee eee_data;
>
> - usbnet_get_endpoints(dev, intf);
> -
> tmp16 = (u16 *)buf;
> tmp = (u8 *)buf;
>
> - memset(ax179_data, 0, sizeof(*ax179_data));
> + if (!do_reset)
> + memset(ax179_data, 0, sizeof(*ax179_data));
>
> /* Power up ethernet PHY */
> *tmp16 = 0;
> @@ -1249,9 +1248,13 @@ static int ax88179_bind(struct usbnet *dev, struct usb_interface *intf)
> ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_CLK_SELECT, 1, 1, tmp);
> msleep(100);
>
> + if (do_reset)
> + ax88179_auto_detach(dev, 0);
> +
> ax88179_read_cmd(dev, AX_ACCESS_MAC, AX_NODE_ID, ETH_ALEN,
> ETH_ALEN, dev->net->dev_addr);
> - memcpy(dev->net->perm_addr, dev->net->dev_addr, ETH_ALEN);
> + if (!do_reset)
> + memcpy(dev->net->perm_addr, dev->net->dev_addr, ETH_ALEN);
>
> /* RX bulk configuration */
> memcpy(tmp, &AX88179_BULKIN_SIZE[0], 5);
> @@ -1266,19 +1269,21 @@ static int ax88179_bind(struct usbnet *dev, struct usb_interface *intf)
> ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_PAUSE_WATERLVL_HIGH,
> 1, 1, tmp);
>
> - dev->net->netdev_ops = &ax88179_netdev_ops;
> - dev->net->ethtool_ops = &ax88179_ethtool_ops;
> - dev->net->needed_headroom = 8;
> - dev->net->max_mtu = 4088;
> -
> - /* Initialize MII structure */
> - dev->mii.dev = dev->net;
> - dev->mii.mdio_read = ax88179_mdio_read;
> - dev->mii.mdio_write = ax88179_mdio_write;
> - dev->mii.phy_id_mask = 0xff;
> - dev->mii.reg_num_mask = 0xff;
> - dev->mii.phy_id = 0x03;
> - dev->mii.supports_gmii = 1;
> + if (!do_reset) {
> + dev->net->netdev_ops = &ax88179_netdev_ops;
> + dev->net->ethtool_ops = &ax88179_ethtool_ops;
> + dev->net->needed_headroom = 8;
> + dev->net->max_mtu = 4088;
> +
> + /* Initialize MII structure */
> + dev->mii.dev = dev->net;
> + dev->mii.mdio_read = ax88179_mdio_read;
> + dev->mii.mdio_write = ax88179_mdio_write;
> + dev->mii.phy_id_mask = 0xff;
> + dev->mii.reg_num_mask = 0xff;
> + dev->mii.phy_id = 0x03;
> + dev->mii.supports_gmii = 1;
> + }
>
> dev->net->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
> NETIF_F_RXCSUM;
> @@ -1330,6 +1335,13 @@ static int ax88179_bind(struct usbnet *dev, struct usb_interface *intf)
> return 0;
> }
>
> +static int ax88179_bind(struct usbnet *dev, struct usb_interface *intf)
> +{
> + usbnet_get_endpoints(dev, intf);
> +
> + return ax88179_bind_or_reset(dev, false);
> +}
> +
> static void ax88179_unbind(struct usbnet *dev, struct usb_interface *intf)
> {
> u16 tmp16;
> @@ -1530,94 +1542,7 @@ static int ax88179_link_reset(struct usbnet *dev)
>
> static int ax88179_reset(struct usbnet *dev)
> {
> - u8 buf[5];
> - u16 *tmp16;
> - u8 *tmp;
> - struct ax88179_data *ax179_data = (struct ax88179_data *)dev->data;
> - struct ethtool_eee eee_data;
> -
> - tmp16 = (u16 *)buf;
> - tmp = (u8 *)buf;
> -
> - /* Power up ethernet PHY */
> - *tmp16 = 0;
> - ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_PHYPWR_RSTCTL, 2, 2, tmp16);
> -
> - *tmp16 = AX_PHYPWR_RSTCTL_IPRL;
> - ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_PHYPWR_RSTCTL, 2, 2, tmp16);
> - msleep(200);
> -
> - *tmp = AX_CLK_SELECT_ACS | AX_CLK_SELECT_BCS;
> - ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_CLK_SELECT, 1, 1, tmp);
> - msleep(100);
> -
> - /* Ethernet PHY Auto Detach*/
> - ax88179_auto_detach(dev, 0);
> -
> - ax88179_read_cmd(dev, AX_ACCESS_MAC, AX_NODE_ID, ETH_ALEN, ETH_ALEN,
> - dev->net->dev_addr);
> -
> - /* RX bulk configuration */
> - memcpy(tmp, &AX88179_BULKIN_SIZE[0], 5);
> - ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_RX_BULKIN_QCTRL, 5, 5, tmp);
> -
> - dev->rx_urb_size = 1024 * 20;
> -
> - *tmp = 0x34;
> - ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_PAUSE_WATERLVL_LOW, 1, 1, tmp);
> -
> - *tmp = 0x52;
> - ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_PAUSE_WATERLVL_HIGH,
> - 1, 1, tmp);
> -
> - dev->net->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
> - NETIF_F_RXCSUM;
> -
> - dev->net->hw_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
> - NETIF_F_RXCSUM;
> -
> - /* Enable checksum offload */
> - *tmp = AX_RXCOE_IP | AX_RXCOE_TCP | AX_RXCOE_UDP |
> - AX_RXCOE_TCPV6 | AX_RXCOE_UDPV6;
> - ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_RXCOE_CTL, 1, 1, tmp);
> -
> - *tmp = AX_TXCOE_IP | AX_TXCOE_TCP | AX_TXCOE_UDP |
> - AX_TXCOE_TCPV6 | AX_TXCOE_UDPV6;
> - ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_TXCOE_CTL, 1, 1, tmp);
> -
> - /* Configure RX control register => start operation */
> - *tmp16 = AX_RX_CTL_DROPCRCERR | AX_RX_CTL_IPE | AX_RX_CTL_START |
> - AX_RX_CTL_AP | AX_RX_CTL_AMALL | AX_RX_CTL_AB;
> - ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_RX_CTL, 2, 2, tmp16);
> -
> - *tmp = AX_MONITOR_MODE_PMETYPE | AX_MONITOR_MODE_PMEPOL |
> - AX_MONITOR_MODE_RWMP;
> - ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_MONITOR_MOD, 1, 1, tmp);
> -
> - /* Configure default medium type => giga */
> - *tmp16 = AX_MEDIUM_RECEIVE_EN | AX_MEDIUM_TXFLOW_CTRLEN |
> - AX_MEDIUM_RXFLOW_CTRLEN | AX_MEDIUM_FULL_DUPLEX |
> - AX_MEDIUM_GIGAMODE;
> - ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_MEDIUM_STATUS_MODE,
> - 2, 2, tmp16);
> -
> - ax88179_led_setting(dev);
> -
> - ax179_data->eee_enabled = 0;
> - ax179_data->eee_active = 0;
> -
> - ax88179_disable_eee(dev);
> -
> - ax88179_ethtool_get_eee(dev, &eee_data);
> - eee_data.advertised = 0;
> - ax88179_ethtool_set_eee(dev, &eee_data);
> -
> - /* Restart autoneg */
> - mii_nway_restart(&dev->mii);
> -
> - usbnet_link_change(dev, 0, 0);
> -
> - return 0;
> + return ax88179_bind_or_reset(dev, true);
> }
>
> static int ax88179_stop(struct usbnet *dev)
Overall, this patch makes much more sense than the previous one (I can
actually see duplicated code being removed). I'll give it a go later
today.
Thanks,
M.
--
Jazz is not dead, it just smell funny.
Powered by blists - more mailing lists