From mboxrd@z Thu Jan 1 00:00:00 1970 From: Oliver Neukum Subject: Re: [PATCH net-next 05/19] net: usb: aqc111: Introduce PHY access Date: Mon, 08 Oct 2018 15:52:54 +0200 Message-ID: <1539006774.10342.16.camel@suse.com> References: <6b4837b970d7709bc2e11d89a7e21e5f10584e30.1538734658.git.igor.russkikh@aquantia.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: Dmitry Bezrukov , "linux-usb@vger.kernel.org" , "netdev@vger.kernel.org" To: Igor Russkikh , "David S . Miller" Return-path: Received: from mx2.suse.de ([195.135.220.15]:51990 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726450AbeJHVNg (ORCPT ); Mon, 8 Oct 2018 17:13:36 -0400 In-Reply-To: <6b4837b970d7709bc2e11d89a7e21e5f10584e30.1538734658.git.igor.russkikh@aquantia.com> Sender: netdev-owner@vger.kernel.org List-ID: On Fr, 2018-10-05 at 10:24 +0000, Igor Russkikh wrote: > From: Dmitry Bezrukov > > Implement PHY power up/down sequences. > AQC111, depending on FW used, may has PHY being controlled either > directly (dpa = 1) or via vendor command interface (dpa = 0). > Drivers supports both themes. > We determine this from firmware versioning agreement. > > Signed-off-by: Dmitry Bezrukov > Signed-off-by: Igor Russkikh > --- > drivers/net/usb/aqc111.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++ > drivers/net/usb/aqc111.h | 70 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 163 insertions(+) > > diff --git a/drivers/net/usb/aqc111.c b/drivers/net/usb/aqc111.c > index 22bb259d71fb..30219bb6ddfd 100644 > --- a/drivers/net/usb/aqc111.c > +++ b/drivers/net/usb/aqc111.c > @@ -138,15 +138,44 @@ static int aqc111_write_cmd(struct usbnet *dev, u8 cmd, u16 value, > return __aqc111_write_cmd(dev, cmd, value, index, size, data, 0); > } > > +static int aq_mdio_read_cmd(struct usbnet *dev, u16 value, u16 index, > + u16 size, void *data) > +{ > + return aqc111_read_cmd(dev, AQ_PHY_CMD, value, index, size, data); > +} > + > +static int aq_mdio_write_cmd(struct usbnet *dev, u16 value, u16 index, > + u16 size, void *data) > +{ > + return aqc111_write_cmd(dev, AQ_PHY_CMD, value, index, size, data); > +} > + > static const struct net_device_ops aqc111_netdev_ops = { > .ndo_open = usbnet_open, > .ndo_stop = usbnet_stop, > }; > > +static void aqc111_read_fw_version(struct usbnet *dev, > + struct aqc111_data *aqc111_data) > +{ > + aqc111_read_cmd(dev, AQ_ACCESS_MAC, AQ_FW_VER_MAJOR, > + 1, 1, &aqc111_data->fw_ver.major); > + aqc111_read_cmd(dev, AQ_ACCESS_MAC, AQ_FW_VER_MINOR, > + 1, 1, &aqc111_data->fw_ver.minor); > + aqc111_read_cmd(dev, AQ_ACCESS_MAC, AQ_FW_VER_REV, > + 1, 1, &aqc111_data->fw_ver.rev); Why read the stuff you don't need? > + > + if (aqc111_data->fw_ver.major & 0x80) > + aqc111_data->fw_ver.major &= ~0x80; > + else > + aqc111_data->dpa = 1; > +} > + > static int aqc111_bind(struct usbnet *dev, struct usb_interface *intf) > { > int ret; > struct usb_device *udev = interface_to_usbdev(intf); > + struct aqc111_data *aqc111_data; > > /* Check if vendor configuration */ > if (udev->actconfig->desc.bConfigurationValue != 1) { > @@ -162,8 +191,18 @@ static int aqc111_bind(struct usbnet *dev, struct usb_interface *intf) > return ret; > } > > + aqc111_data = kzalloc(sizeof(*aqc111_data), GFP_KERNEL); > + if (!aqc111_data) > + return -ENOMEM; > + > + /* store aqc111_data pointer in device data field */ > + dev->data[0] = (unsigned long)aqc111_data; > + memset(aqc111_data, 0, sizeof(*aqc111_data)); Either kzalloc() or a memset. Not both. > + > dev->net->netdev_ops = &aqc111_netdev_ops; > > + aqc111_read_fw_version(dev, aqc111_data); > + > return 0; > } Regards Oliver