Hi Marek,

>-----Original Message-----
>From: Marek Vasut [mailto:ma...@denx.de]
>Sent: Wednesday, August 9, 2017 1:12 PM
>To: Yuiko Oshino - C18177; u-boot@lists.denx.de
>Cc: Joe Hershberger
>Subject: Re: [PATCH v3 1/2] usb: net: Add support for Microchip LAN75xx and
>LAN78xx
>
>On 08/09/2017 06:25 PM, yuiko.osh...@microchip.com wrote:
>> From: Yuiko Oshino <yuiko.osh...@microchip.com>
>>
>> Series-Changes: 3
>
>FYI, this will end in the commit message when applied, remove it or move it
>below the --- . Also commit message is missing.

I did my best to follow the patman instructions and I added "commit-notes:" 
tag, but I guess it wasn't good enough.
Should I always manually edit the patch before sending email in the patman?
Also, when I am ready to update this patch again, should I do a series or just 
this patch?
How can I update the [PATCH v] number? If just his patch, then will it be 
[PATCH v4]?

>
>>    - All #ifdef CONFIG_DM_ETH and #endif are removed.
>>    - The lan7x_eth_recv() is modifed to correctly support the Driver Model
>>      and returns packet_en.
>>    - Add mii_resolve_flowctrl_fdx() patch in the series.
>>
>> Series-Changes: 2
>>    - The wait_for_bit functions copy the real one.
>>    - Uses phylib
>>    - Unnecessary variables are removed
>>    - All return values are checked
>>    - Uses mii_resolve_flowctrl_fdx() from linux/mii.h
>>
>> Signed-off-by: Yuiko Oshino <yuiko.osh...@microchip.com>
>> ---
>> Add support for Microchip LAN7500, LAN7800 and LAN7850, USB to
>> 10/100/1000 Ethernet Controllers.
>>
>>
>>  drivers/usb/Kconfig       |   2 +
>>  drivers/usb/eth/Kconfig   |  17 ++
>>  drivers/usb/eth/Makefile  |   2 +
>>  drivers/usb/eth/lan75xx.c | 318 +++++++++++++++++++++++++++++
>> drivers/usb/eth/lan78xx.c | 477
>++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/usb/eth/lan7x.c   | 495
>++++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/usb/eth/lan7x.h   | 230 +++++++++++++++++++++
>>  7 files changed, 1541 insertions(+)
>>  create mode 100644 drivers/usb/eth/Kconfig  create mode 100644
>> drivers/usb/eth/lan75xx.c  create mode 100644
>> drivers/usb/eth/lan78xx.c  create mode 100644 drivers/usb/eth/lan7x.c
>> create mode 100644 drivers/usb/eth/lan7x.h
>>
>> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig index
>> da3ec2f..62126aa 100644
>> --- a/drivers/usb/Kconfig
>> +++ b/drivers/usb/Kconfig
>> @@ -94,4 +94,6 @@ endif
>>
>>  source "drivers/usb/gadget/Kconfig"
>>
>> +source "drivers/usb/eth/Kconfig"
>> +
>>  endif
>> diff --git a/drivers/usb/eth/Kconfig b/drivers/usb/eth/Kconfig new
>> file mode 100644 index 0000000..14cfa26
>> --- /dev/null
>> +++ b/drivers/usb/eth/Kconfig
>> @@ -0,0 +1,17 @@
>> +comment "USB to Ethernet Controller Drivers"
>> +
>> +config USB_ETHER_LAN75XX
>> +    bool "Microchip LAN75XX support"
>> +    ---help---
>> +      Say Y here if you would like to support Microchip LAN75XX Hi-Speed
>> +      USB 2.0 to 10/100/1000 Gigabit Ethernet controller.
>> +      Supports 10Base-T/ 100Base-TX/1000Base-T.
>> +      This driver supports the internal PHY.
>> +
>> +config USB_ETHER_LAN78XX
>> +    bool "Microchip LAN78XX support"
>> +    ---help---
>> +      Say Y here if you would like to support Microchip LAN78XX USB 3.1
>> +      Gen 1 to 10/100/1000 Gigabit Ethernet controller.
>> +      Supports 10Base-T/ 100Base-TX/1000Base-T.
>> +      This driver supports the internal PHY.
>> diff --git a/drivers/usb/eth/Makefile b/drivers/usb/eth/Makefile index
>> 4c44efc..4b935a3 100644
>> --- a/drivers/usb/eth/Makefile
>> +++ b/drivers/usb/eth/Makefile
>> @@ -9,4 +9,6 @@ obj-$(CONFIG_USB_ETHER_ASIX) += asix.o
>>  obj-$(CONFIG_USB_ETHER_ASIX88179) += asix88179.o
>>  obj-$(CONFIG_USB_ETHER_MCS7830) += mcs7830.o
>>  obj-$(CONFIG_USB_ETHER_SMSC95XX) += smsc95xx.o
>> +obj-$(CONFIG_USB_ETHER_LAN75XX) += lan7x.o lan75xx.o
>> +obj-$(CONFIG_USB_ETHER_LAN78XX) += lan7x.o lan78xx.o
>>  obj-$(CONFIG_USB_ETHER_RTL8152) += r8152.o r8152_fw.o diff --git
>> a/drivers/usb/eth/lan75xx.c b/drivers/usb/eth/lan75xx.c new file mode
>> 100644 index 0000000..a3c1411
>> --- /dev/null
>> +++ b/drivers/usb/eth/lan75xx.c
>> @@ -0,0 +1,318 @@
>> +/*
>> + * Copyright (c) 2017 Microchip Technology Inc. All rights reserved.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#include <dm.h>
>> +#include <usb.h>
>> +#include <linux/mii.h>
>> +#include "usb_ether.h"
>> +#include "lan7x.h"
>> +
>> +/* LAN75xx specific register/bit defines */
>> +#define LAN75XX_HW_CFG_BIR          BIT(7)
>> +
>> +#define LAN75XX_BURST_CAP           0x034
>> +
>> +#define LAN75XX_BULK_IN_DLY         0x03C
>> +
>> +#define LAN75XX_RFE_CTL                     0x060
>> +
>> +#define LAN75XX_FCT_RX_CTL          0x090
>> +
>> +#define LAN75XX_FCT_TX_CTL          0x094
>> +
>> +#define LAN75XX_FCT_RX_FIFO_END             0x098
>> +
>> +#define LAN75XX_FCT_TX_FIFO_END             0x09C
>> +
>> +#define LAN75XX_FCT_FLOW            0x0A0
>> +
>> +/* MAC ADDRESS PERFECT FILTER For LAN75xx */
>> +#define LAN75XX_ADDR_FILTX          0x300
>> +#define LAN75XX_ADDR_FILTX_FB_VALID BIT(31)
>> +
>> +/*
>> + * Lan75xx infrastructure commands
>> + */
>> +static int lan75xx_phy_gig_workaround(struct usb_device *udev,
>> +                                  struct ueth_data *dev)
>> +{
>> +    int ret = 0;
>> +
>> +    /* Only internal phy */
>> +    /* Set the phy in Gig loopback */
>> +    lan7x_mdio_write(udev, dev->phy_id, MII_BMCR,
>> +                     (BMCR_LOOPBACK | BMCR_SPEED1000));
>> +
>> +    /* Wait for the link up */
>> +    ret = lan7x_mdio_wait_for_bit(udev, "BMSR_LSTATUS",
>> +                                  dev->phy_id, MII_BMSR, BMSR_LSTATUS,
>> +                                  true, PHY_CONNECT_TIMEOUT_MS, 1);
>> +    if (ret)
>> +            return ret;
>> +
>> +    /* phy reset */
>> +    return lan7x_pmt_phy_reset(udev, dev); }
>> +
>> +static int lan75xx_update_flowcontrol(struct usb_device *udev,
>> +                                  struct ueth_data *dev)
>> +{
>> +    uint32_t flow = 0, fct_flow = 0;
>> +    int ret;
>> +
>> +    ret = lan7x_update_flowcontrol(udev, dev, &flow, &fct_flow);
>> +    if (ret)
>> +            return ret;
>> +
>> +    ret = lan7x_write_reg(udev, LAN75XX_FCT_FLOW, fct_flow);
>> +    if (ret)
>> +            return ret;
>> +    return lan7x_write_reg(udev, FLOW, flow); }
>> +
>> +static int lan75xx_read_mac(unsigned char *enetaddr,
>> +                        struct usb_device *udev)
>> +{
>> +    /*
>> +     * Refer to the doc/README.enetaddr and doc/README.usb for
>> +     * the U-Boot MAC address policy
>> +     */
>> +    return lan7x_read_eeprom_mac(enetaddr, udev);
>
>Is this function needed ?

I wanted to maintain the same function name convention among our drivers.
Therefore the wrapper for this device.
But I can remove it.

>
>> +}
>> +
>> +static int lan75xx_set_receive_filter(struct usb_device *udev) {
>> +    /* No multicast in u-boot */
>> +    return lan7x_write_reg(udev, LAN75XX_RFE_CTL,
>> +                           RFE_CTL_BCAST_EN | RFE_CTL_DA_PERFECT); }
>> +
>> +/* starts the TX path */
>> +static void lan75xx_start_tx_path(struct usb_device *udev) {
>> +    /* Enable Tx at MAC */
>> +    lan7x_write_reg(udev, MAC_TX, MAC_TX_TXEN);
>> +
>> +    /* Enable Tx at SCSRs */
>> +    lan7x_write_reg(udev, LAN75XX_FCT_TX_CTL, FCT_TX_CTL_EN); }
>> +
>> +/* Starts the Receive path */
>> +static void lan75xx_start_rx_path(struct usb_device *udev) {
>> +    /* Enable Rx at MAC */
>> +    lan7x_write_reg(udev, MAC_RX,
>> +                    LAN7X_MAC_RX_MAX_SIZE_DEFAULT |
>> +                    MAC_RX_FCS_STRIP | MAC_RX_RXEN);
>> +
>> +    /* Enable Rx at SCSRs */
>> +    lan7x_write_reg(udev, LAN75XX_FCT_RX_CTL, FCT_RX_CTL_EN); }
>> +
>> +static int lan75xx_basic_reset(struct usb_device *udev,
>> +                           struct ueth_data *dev,
>> +                           struct lan7x_private *priv) {
>> +    int ret;
>> +    u32 val;
>> +
>> +    ret = lan7x_basic_reset(udev, dev);
>> +    if (ret)
>> +            return ret;
>> +
>> +    /* Keep the chip ID */
>> +    ret = lan7x_read_reg(udev, ID_REV, &val);
>> +    if (ret)
>> +            return ret;
>> +    debug("LAN75xx ID_REV = 0x%08x\n", val);
>
>Some sort of USB bus ID would be useful in the debug message to identify the
>chip in case multiple are present.

I can add this to the lan7x_basic_reset()
        debug("USB devnum %d portnr %d\n", udev->devnum, udev->portnr);
This shows device number and port number.
Is this ok?

>
>> +    priv->chipid = (val & ID_REV_CHIP_ID_MASK) >> 16;
>> +
>> +    /* Respond to the IN token with a NAK */
>> +    ret = lan7x_read_reg(udev, HW_CFG, &val);
>> +    if (ret)
>> +            return ret;
>> +    val |= LAN75XX_HW_CFG_BIR;
>> +    return lan7x_write_reg(udev, HW_CFG, val); }
>[...]
>--
>Best regards,
>Marek Vasut

Thank you!
Yuiko
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to