Re: [U-Boot] [PATCH 2/2 v2] drivers: net: add NXP ENETC MDIO driver

Hi Bin,

On 6/10/2019 6:42 AM, Bin Meng wrote:

Hi Alex,

On Sat, Jun 8, 2019 at 12:13 AM Alex Marginean alexm.osslist@gmail.com wrote:

...

Adds a driver for the MDIO interface currently integrated in LS1028a SoC. This MDIO interface is shared by multiple ethernet interfaces and is presented as a stand-alone PCI function on the SoC ECAM.

Signed-off-by: Alex Marginean alexm.osslist@gmail.com

Changes in v2: - fix priv structure used (was using the eth driver structure) - simplified naming code in _bind - ENETC_DBG -> enetc_dbg - several styling and cosmetic updates to the header file

configs/ls1028aqds_tfa_defconfig | 1 + configs/ls1028ardb_tfa_defconfig | 1 + drivers/net/fsl_enetc.c | 169 +++++++++++++++++++++++++++++++ drivers/net/fsl_enetc.h | 18 ++++ include/pci_ids.h | 1 + 5 files changed, 190 insertions(+)

diff --git a/configs/ls1028aqds_tfa_defconfig b/configs/ls1028aqds_tfa_defconfig index 11fe344b04..84a1bf90bf 100644 --- a/configs/ls1028aqds_tfa_defconfig +++ b/configs/ls1028aqds_tfa_defconfig @@ -43,6 +43,7 @@ CONFIG_SPI_FLASH_STMICRO=y CONFIG_PHYLIB=y CONFIG_PHY_ATHEROS=y CONFIG_DM_ETH=y +CONFIG_DM_MDIO=y

See my comments in patch [1/2]

...

CONFIG_PHY_GIGE=y CONFIG_E1000=y CONFIG_FSL_ENETC=y diff --git a/configs/ls1028ardb_tfa_defconfig b/configs/ls1028ardb_tfa_defconfig index ab6f2a850c..3f5bc2e139 100644 --- a/configs/ls1028ardb_tfa_defconfig +++ b/configs/ls1028ardb_tfa_defconfig @@ -43,6 +43,7 @@ CONFIG_SPI_FLASH_STMICRO=y CONFIG_PHYLIB=y CONFIG_PHY_ATHEROS=y CONFIG_DM_ETH=y +CONFIG_DM_MDIO=y

ditto

...

CONFIG_PHY_GIGE=y CONFIG_E1000=y CONFIG_FSL_ENETC=y diff --git a/drivers/net/fsl_enetc.c b/drivers/net/fsl_enetc.c index 325e032746..8fe84949b8 100644 --- a/drivers/net/fsl_enetc.c +++ b/drivers/net/fsl_enetc.c @@ -12,6 +12,7 @@ #include <asm/io.h> #include <asm/processor.h> #include <pci.h> +#include <miiphy.h>

static int enetc_bind(struct udevice *dev) { @@ -23,6 +24,61 @@ static int enetc_bind(struct udevice *dev) return 0; }

+static void enetc_start_phy(struct udevice *dev) +{ +#ifdef CONFIG_DM_MDIO

•   int supported, if_type = PHY_INTERFACE_MODE_NONE;
  

•   struct udevice *miidev;
  

•   struct phy_device *phy;
  

•   u32 phandle, phy_id;
  

•   const char *if_str;
  

•   ofnode phy_node;
  

•   if (!ofnode_valid(dev->node)) {
  

•           enetc_dbg(dev, "no enetc ofnode found, skipping PHY set-up\n");
  

•           return;
  

•   }
  

•   if (ofnode_read_u32(dev->node, "phy-handle", &phandle)) {
  

•           enetc_dbg(dev, "phy-handle not found, skipping PHY set-up\n");
  

•           return;
  

•   }
  

•   phy_node = ofnode_get_by_phandle(phandle);
  

•   if (!ofnode_valid(phy_node)) {
  

•           enetc_dbg(dev, "invalid phy node, skipping PHY set-up\n");
  

•           return;
  

•   }
  

•   enetc_dbg(dev, "phy node: %s\n", ofnode_get_name(phy_node));
  

•   if (ofnode_read_u32(phy_node, "reg", &phy_id)) {
  

•           enetc_dbg(dev,
  

•                     "missing reg in PHY node, skipping PHY set-up\n");
  

•           return;
  

•   }
  

•   if_str = ofnode_read_string(phy_node, "phy-mode");
  

•   if (if_str)
  

•           if_type = phy_get_interface_by_name(if_str);
  

•   if (if_type < 0)
  

•           if_type = PHY_INTERFACE_MODE_NONE;
  

•   if (uclass_get_device_by_ofnode(UCLASS_MDIO,
  

•                                   ofnode_get_parent(phy_node),
  

•                                   &miidev))
  

•           return;
  

•   phy = dm_mdio_phy_connect(miidev, phy_id, dev, if_type);
  

•   if (!phy)
  

•           return;
  

•   supported = GENMASK(6, 0); /* speeds up to 1G & AN */
  

•   phy->advertising = phy->supported & supported;
  

•   phy_config(phy);
  

•   phy_startup(phy);
  

+#endif +}

• /*
  • Probe ENETC driver:
  • • initialize port and station interface BARs

@@ -223,6 +279,8 @@ static int enetc_start(struct udevice *dev) enetc_setup_tx_bdr(hw); enetc_setup_rx_bdr(dev, hw);

•   enetc_start_phy(dev);
  

•    return 0;
  

  }

@@ -342,3 +400,114 @@ static struct pci_device_id enetc_ids[] = { };

U_BOOT_PCI_DEVICE(eth_enetc, enetc_ids);

+#ifdef CONFIG_DM_MDIO

+static void enetc_mdio_wait_bsy(struct enetc_mdio_devfn *hw) +{

•   while (enetc_read(hw, ENETC_MDIO_CFG) & ENETC_EMDIO_CFG_BSY)
  

•           cpu_relax();
  

+}

+static int enetc_mdio_read(struct udevice *dev, int addr, int devad, int reg) +{

•   struct enetc_mdio_devfn *hw = dev_get_priv(dev);
  

•   if (devad == MDIO_DEVAD_NONE)
  

•           enetc_write(hw, ENETC_MDIO_CFG, ENETC_EMDIO_CFG_C22);
  

•   else
  

•           enetc_write(hw, ENETC_MDIO_CFG, ENETC_EMDIO_CFG_C45);
  

•   enetc_mdio_wait_bsy(hw);
  

•   if (devad == MDIO_DEVAD_NONE) {
  

•           enetc_write(hw, ENETC_MDIO_CTL, ENETC_MDIO_CTL_READ |
  

•                       (addr << 5) | reg);
  

•   } else {
  

•           enetc_write(hw, ENETC_MDIO_CTL, (addr << 5) + devad);
  

•           enetc_mdio_wait_bsy(hw);
  

•           enetc_write(hw, ENETC_MDIO_STAT, reg);
  

•           enetc_mdio_wait_bsy(hw);
  

•           enetc_write(hw, ENETC_MDIO_CTL, ENETC_MDIO_CTL_READ |
  

•                       (addr << 5) | devad);
  

•   }
  

•   enetc_mdio_wait_bsy(hw);
  

•   if (enetc_read(hw, ENETC_MDIO_CFG) & ENETC_EMDIO_CFG_RD_ER)
  

•           return ENETC_MDIO_READ_ERR;
  

•   return enetc_read(hw, ENETC_MDIO_DATA);
  

+}

+static int enetc_mdio_write(struct udevice *dev, int addr, int devad, int reg,

•                       u16 val)
  

+{

•   struct enetc_mdio_devfn *hw = dev_get_priv(dev);
  

•   if (devad == MDIO_DEVAD_NONE)
  

•           enetc_write(hw, ENETC_MDIO_CFG, ENETC_EMDIO_CFG_C22);
  

•   else
  

•           enetc_write(hw, ENETC_MDIO_CFG, ENETC_EMDIO_CFG_C45);
  

•   enetc_mdio_wait_bsy(hw);
  

•   if (devad != MDIO_DEVAD_NONE) {
  

•           enetc_write(hw, ENETC_MDIO_CTL, (addr << 5) + devad);
  

•           enetc_write(hw, ENETC_MDIO_STAT, reg);
  

•   } else {
  

•           enetc_write(hw, ENETC_MDIO_CTL, (addr << 5) + reg);
  

•   }
  

•   enetc_mdio_wait_bsy(hw);
  

•   enetc_write(hw, ENETC_MDIO_DATA, val);
  

•   enetc_mdio_wait_bsy(hw);
  

•   return 0;
  

+}

+static const struct mdio_ops enetc_mdio_ops = {

•   .read = enetc_mdio_read,
  

•   .write = enetc_mdio_write,
  

+};

+static int enetc_mdio_bind(struct udevice *dev) +{

•   char name[16];
  

•   sprintf(name, "emdio#%u", PCI_FUNC(dm_pci_get_bdf(dev)));
  

•   device_set_name(dev, name);
  

•   return 0;
  

+}

+static int enetc_mdio_probe(struct udevice *dev) +{

•   struct enetc_devfn *hw = dev_get_priv(dev);
  

•   hw->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0);
  

•   if (!hw->regs_base) {
  

•           enetc_dbg(dev, "failed to map BAR0\n");
  

•           return -EINVAL;
  

•   }
  

•   dm_pci_clrset_config16(dev, PCI_COMMAND, 0, PCI_COMMAND_MEMORY);
  

•   return 0;
  

+}

+U_BOOT_DRIVER(enetc_mdio) = {

•   .name   = "enetc_mdio",
  

•   .id     = UCLASS_MDIO,
  

•   .bind   = enetc_mdio_bind,
  

•   .probe  = enetc_mdio_probe,
  

•   .ops    = &enetc_mdio_ops,
  

•   .priv_auto_alloc_size = sizeof(struct enetc_mdio_devfn),
  

+};

+static struct pci_device_id enetc_mdio_ids[] = {

•   { PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, PCI_DEVICE_ID_MDIO) },
  

+};

+U_BOOT_PCI_DEVICE(enetc_mdio, enetc_mdio_ids);

We should create a separate file fsl_enetc_mdio.c for the ENETC MDIO driver, and create a Kconfig option that depends on DM_MDIO for that driver, and select the new MDIO driver in the board defconfig file, instead of select DM_MDIO.

...

+#endif /* CONFIG_DM_MDIO */ diff --git a/drivers/net/fsl_enetc.h b/drivers/net/fsl_enetc.h index 94f836760a..fb9d31bf79 100644 --- a/drivers/net/fsl_enetc.h +++ b/drivers/net/fsl_enetc.h @@ -169,4 +169,22 @@ struct enetc_devfn { #define enetc_bdr_write(hw, t, n, off, val) \ enetc_write(hw, ENETC_BDR(t, n, off), val)

+/* ENETC external MDIO registers */

+#define ENETC_MDIO_CFG 0x1c00 +#define ENETC_EMDIO_CFG_C22 0x00809508 +#define ENETC_EMDIO_CFG_C45 0x00809548 +#define ENETC_EMDIO_CFG_RD_ER BIT(1) +#define ENETC_EMDIO_CFG_BSY BIT(0) +#define ENETC_MDIO_CTL 0x1c04 +#define ENETC_MDIO_CTL_READ BIT(15) +#define ENETC_MDIO_DATA 0x1c08 +#define ENETC_MDIO_STAT 0x1c0c

+#define ENETC_MDIO_READ_ERR 0xffff

+struct enetc_mdio_devfn {

•   void *regs_base;
  

+};

• #endif /* _ENETC_H */

diff --git a/include/pci_ids.h b/include/pci_ids.h index 16cf0641c7..855136e89a 100644 --- a/include/pci_ids.h +++ b/include/pci_ids.h @@ -2484,6 +2484,7 @@ #define PCI_DEVICE_ID_MPC8641D 0x7011 #define PCI_DEVICE_ID_MPC8610 0x7018 #define PCI_DEVICE_ID_ENETC_PF 0xE100 +#define PCI_DEVICE_ID_MDIO 0xEE01

Suggest we put this macro in the fsl_enetc.h, and the name _MDIO is too generic. Maybe _ENETC_MDIO?

It is too generic, I'll rename it and move it to driver header. I'll send a v3 for these.

...

#define PCI_VENDOR_ID_PASEMI 0x1959

--

Regards, Bin

Thank you! Alex