From mboxrd@z Thu Jan 1 00:00:00 1970 From: mbrugger@suse.com (Matthias Brugger) Date: Wed, 8 Jun 2016 11:06:25 +0200 Subject: [PATCH v3 1/5] drivers: net: xgene: MAC and PHY configuration changes In-Reply-To: <1465236962-12131-2-git-send-email-isubramanian@apm.com> References: <1465236962-12131-1-git-send-email-isubramanian@apm.com> <1465236962-12131-2-git-send-email-isubramanian@apm.com> Message-ID: <5757E011.5090909@suse.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 06/06/16 20:15, Iyappan Subramanian wrote: > This patch fixes MAC configuration to support 10/100GbE for SGMII and > link_state call back. It also sets pdata->mdio_driver flag based on > ethernet mdio subnode and prepare for MDIO driver support. > > In summary, following are the changes, > > - Added set_speed function pointer in mac_ops This can be a seperate patch, or would it break the driver? In the end, the decision how to split up the patches is up to Dave. I prefer to have more small patches which do incrementally add new stuff to the driver, taking into account that every single patch does not break it. > - Changed link_state to call the set_speed > - Add 10/100 support for SGMII based 1G > - Fixed mac_init for 1G > > - Call mac_ops rx_enable/disable and tx_enable/disable function pointers For me, this is a seperate patch which does a cleanup of the driver. You just use the mac_ops for these function (enable/disable and init) and in a second patch you add the set_speed and delete the init call. > - Add acpi_phy_find_device to find PHY using phy-handle reference object > - Changing phy_start and phy_stop calls based on phy_dev object existence > - Calling phy_connect based on pdata->mdio_driver flag > > Signed-off-by: Iyappan Subramanian > Tested-by: Fushen Chen > Tested-by: Toan Le > Tested-by: Matthias Brugger > --- > drivers/net/ethernet/apm/xgene/xgene_enet_hw.c | 190 +++++++++++++--------- > drivers/net/ethernet/apm/xgene/xgene_enet_hw.h | 5 + > drivers/net/ethernet/apm/xgene/xgene_enet_main.c | 41 +++-- > drivers/net/ethernet/apm/xgene/xgene_enet_main.h | 2 + > drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c | 106 +++++++++++- > drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h | 8 + > drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h | 4 + > 7 files changed, 259 insertions(+), 97 deletions(-) > > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c > index 2f5638f..5d6d14b 100644 > --- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c > +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c > @@ -512,14 +512,11 @@ static void xgene_enet_configure_clock(struct xgene_enet_pdata *pdata) > #endif > } > > -static void xgene_gmac_init(struct xgene_enet_pdata *pdata) > +static void xgene_gmac_set_speed(struct xgene_enet_pdata *pdata) > { > struct device *dev = &pdata->pdev->dev; > - u32 value, mc2; > - u32 intf_ctl, rgmii; > - u32 icm0, icm2; > - > - xgene_gmac_reset(pdata); > + u32 icm0, icm2, mc2; > + u32 intf_ctl, rgmii, value; > > xgene_enet_rd_mcx_csr(pdata, ICM_CONFIG0_REG_0_ADDR, &icm0); > xgene_enet_rd_mcx_csr(pdata, ICM_CONFIG2_REG_0_ADDR, &icm2); > @@ -564,7 +561,18 @@ static void xgene_gmac_init(struct xgene_enet_pdata *pdata) > mc2 |= FULL_DUPLEX2 | PAD_CRC; > xgene_enet_wr_mcx_mac(pdata, MAC_CONFIG_2_ADDR, mc2); > xgene_enet_wr_mcx_mac(pdata, INTERFACE_CONTROL_ADDR, intf_ctl); > + xgene_enet_wr_csr(pdata, RGMII_REG_0_ADDR, rgmii); > + xgene_enet_configure_clock(pdata); > + > + xgene_enet_wr_mcx_csr(pdata, ICM_CONFIG0_REG_0_ADDR, icm0); > + xgene_enet_wr_mcx_csr(pdata, ICM_CONFIG2_REG_0_ADDR, icm2); > +} > > +static void xgene_gmac_init(struct xgene_enet_pdata *pdata) > +{ > + u32 value; > + > + xgene_gmac_set_speed(pdata); > xgene_gmac_set_mac_addr(pdata); > > /* Adjust MDC clock frequency */ > @@ -579,15 +587,10 @@ static void xgene_gmac_init(struct xgene_enet_pdata *pdata) > > /* Rtype should be copied from FP */ > xgene_enet_wr_csr(pdata, RSIF_RAM_DBG_REG0_ADDR, 0); > - xgene_enet_wr_csr(pdata, RGMII_REG_0_ADDR, rgmii); > - xgene_enet_configure_clock(pdata); > > /* Rx-Tx traffic resume */ > xgene_enet_wr_csr(pdata, CFG_LINK_AGGR_RESUME_0_ADDR, TX_PORT0); > > - xgene_enet_wr_mcx_csr(pdata, ICM_CONFIG0_REG_0_ADDR, icm0); > - xgene_enet_wr_mcx_csr(pdata, ICM_CONFIG2_REG_0_ADDR, icm2); > - Is this the part you refer to with "Fixed mac_init for 1G"? What does it fix? Does this fix something which might be worth getting backported to older kernel versions? > xgene_enet_rd_mcx_csr(pdata, RX_DV_GATE_REG_0_ADDR, &value); > value &= ~TX_DV_GATE_EN0; > value &= ~RX_DV_GATE_EN0; > @@ -671,25 +674,12 @@ bool xgene_ring_mgr_init(struct xgene_enet_pdata *p) > > static int xgene_enet_reset(struct xgene_enet_pdata *pdata) > { > - u32 val; > - > if (!xgene_ring_mgr_init(pdata)) > return -ENODEV; > > - if (!IS_ERR(pdata->clk)) { > - clk_prepare_enable(pdata->clk); > - clk_disable_unprepare(pdata->clk); > - clk_prepare_enable(pdata->clk); > - xgene_enet_ecc_init(pdata); > - } > + xgene_enet_ecc_init(pdata); > xgene_enet_config_ring_if_assoc(pdata); > > - /* Enable auto-incr for scanning */ > - xgene_enet_rd_mcx_mac(pdata, MII_MGMT_CONFIG_ADDR, &val); > - val |= SCAN_AUTO_INCR; > - MGMT_CLOCK_SEL_SET(&val, 1); > - xgene_enet_wr_mcx_mac(pdata, MII_MGMT_CONFIG_ADDR, val); > - > return 0; > } > > @@ -724,29 +714,49 @@ static int xgene_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum, > static void xgene_enet_adjust_link(struct net_device *ndev) > { > struct xgene_enet_pdata *pdata = netdev_priv(ndev); > + const struct xgene_mac_ops *mac_ops = pdata->mac_ops; > struct phy_device *phydev = pdata->phy_dev; > > if (phydev->link) { > if (pdata->phy_speed != phydev->speed) { > pdata->phy_speed = phydev->speed; > - xgene_gmac_init(pdata); > - xgene_gmac_rx_enable(pdata); > - xgene_gmac_tx_enable(pdata); > + mac_ops->set_speed(pdata); > + mac_ops->rx_enable(pdata); > + mac_ops->tx_enable(pdata); > phy_print_status(phydev); > } > } else { > - xgene_gmac_rx_disable(pdata); > - xgene_gmac_tx_disable(pdata); > + mac_ops->rx_disable(pdata); > + mac_ops->tx_disable(pdata); > pdata->phy_speed = SPEED_UNKNOWN; > phy_print_status(phydev); > } > } > > -static int xgene_enet_phy_connect(struct net_device *ndev) > +#ifdef CONFIG_ACPI > +static struct acpi_device *acpi_phy_find_device(struct device *dev) > +{ > + struct acpi_reference_args args; > + struct fwnode_handle *fw_node; > + int status; > + > + fw_node = acpi_fwnode_handle(ACPI_COMPANION(dev)); > + status = acpi_node_get_property_reference(fw_node, "phy-handle", 0, > + &args); > + if (ACPI_FAILURE(status)) { > + dev_dbg(dev, "No matching phy in ACPI table\n"); > + return NULL; > + } > + > + return args.adev; > +} > +#endif > + > +int xgene_enet_phy_connect(struct net_device *ndev) > { > struct xgene_enet_pdata *pdata = netdev_priv(ndev); > struct device_node *phy_np; > - struct phy_device *phy_dev; > + struct phy_device *phy_dev = NULL; > struct device *dev = &pdata->pdev->dev; > > if (dev->of_node) { > @@ -756,23 +766,25 @@ static int xgene_enet_phy_connect(struct net_device *ndev) > return -ENODEV; > } > > - phy_dev = of_phy_connect(ndev, phy_np, &xgene_enet_adjust_link, > - 0, pdata->phy_mode); > - if (!phy_dev) { > - netdev_err(ndev, "Could not connect to PHY\n"); > - return -ENODEV; > - } > - > - pdata->phy_dev = phy_dev; > + pdata->phy_dev = of_phy_find_device(phy_np); > } else { > - phy_dev = pdata->phy_dev; > +#ifdef CONFIG_ACPI > + if (pdata->mdio_driver) { > + struct acpi_device *adev; > > - if (!phy_dev || > - phy_connect_direct(ndev, phy_dev, &xgene_enet_adjust_link, > - pdata->phy_mode)) { > - netdev_err(ndev, "Could not connect to PHY\n"); > - return -ENODEV; > + adev = acpi_phy_find_device(dev); > + if (adev) > + pdata->phy_dev = adev->driver_data; > } > +#endif > + } > + > + phy_dev = pdata->phy_dev; > + if (!phy_dev || > + phy_connect_direct(ndev, phy_dev, &xgene_enet_adjust_link, > + pdata->phy_mode)) { > + netdev_err(ndev, "Could not connect to PHY\n"); > + return -ENODEV; > } > > pdata->phy_speed = SPEED_UNKNOWN; > @@ -788,12 +800,9 @@ static int xgene_mdiobus_register(struct xgene_enet_pdata *pdata, > struct mii_bus *mdio) > { > struct device *dev = &pdata->pdev->dev; > - struct net_device *ndev = pdata->ndev; > - struct phy_device *phy; > - struct device_node *child_np; > struct device_node *mdio_np = NULL; > - int ret; > - u32 phy_id; > + struct device_node *child_np; > + u32 phyid; > > if (dev->of_node) { > for_each_child_of_node(dev->of_node, child_np) { > @@ -805,38 +814,50 @@ static int xgene_mdiobus_register(struct xgene_enet_pdata *pdata, > } > > if (!mdio_np) { > - netdev_dbg(ndev, "No mdio node in the dts\n"); > - return -ENXIO; > + mdiobus_free(mdio); > + return 0; > } > > + pdata->mdio_driver = false; > + > return of_mdiobus_register(mdio, mdio_np); > - } > + } else { > +#ifdef CONFIG_ACPI > + struct phy_device *phy; > + int ret; > > - /* Mask out all PHYs from auto probing. */ > - mdio->phy_mask = ~0; > + if (pdata->mdio_driver) { > + mdiobus_free(mdio); > + return 0; > + } > > - /* Register the MDIO bus */ > - ret = mdiobus_register(mdio); > - if (ret) > - return ret; > + /* Mask out all PHYs from auto probing. */ > + mdio->phy_mask = ~0; > > - ret = device_property_read_u32(dev, "phy-channel", &phy_id); > - if (ret) > - ret = device_property_read_u32(dev, "phy-addr", &phy_id); > - if (ret) > - return -EINVAL; > + /* Register the MDIO bus */ > + ret = mdiobus_register(mdio); > + if (ret) > + return ret; > > - phy = get_phy_device(mdio, phy_id, false); > - if (IS_ERR(phy)) > - return -EIO; > + ret = device_property_read_u32(dev, "phy-channel", &phyid); > + if (ret) > + ret = device_property_read_u32(dev, "phy-addr", &phyid); > + if (ret) > + return -EINVAL; > > - ret = phy_device_register(phy); > - if (ret) > - phy_device_free(phy); > - else > - pdata->phy_dev = phy; > + phy = get_phy_device(mdio, phyid, false); > + if (IS_ERR(phy)) > + return -EIO; > > - return ret; > + ret = phy_device_register(phy); > + if (ret) > + phy_device_free(phy); > + else > + pdata->phy_dev = phy; > + > + return ret; > +#endif > + } > } > > int xgene_enet_mdio_config(struct xgene_enet_pdata *pdata) > @@ -861,7 +882,13 @@ int xgene_enet_mdio_config(struct xgene_enet_pdata *pdata) > ret = xgene_mdiobus_register(pdata, mdio_bus); > if (ret) { > netdev_err(ndev, "Failed to register MDIO bus\n"); > + if (mdio_bus->state == MDIOBUS_REGISTERED) > + mdiobus_unregister(pdata->mdio_bus); > mdiobus_free(mdio_bus); > + if (pdata->mdio_driver) { > + ret = xgene_enet_phy_connect(ndev); > + return 0; > + } > return ret; > } > pdata->mdio_bus = mdio_bus; > @@ -873,14 +900,22 @@ int xgene_enet_mdio_config(struct xgene_enet_pdata *pdata) > return ret; > } > > +void xgene_enet_phy_disconnect(struct xgene_enet_pdata *pdata) > +{ > + if (pdata->phy_dev) > + phy_disconnect(pdata->phy_dev); > +} > + > void xgene_enet_mdio_remove(struct xgene_enet_pdata *pdata) > { > if (pdata->phy_dev) > phy_disconnect(pdata->phy_dev); > > - mdiobus_unregister(pdata->mdio_bus); > - mdiobus_free(pdata->mdio_bus); > - pdata->mdio_bus = NULL; > + if (!pdata->mdio_driver) { > + mdiobus_unregister(pdata->mdio_bus); > + mdiobus_free(pdata->mdio_bus); > + pdata->mdio_bus = NULL; > + } > } > > const struct xgene_mac_ops xgene_gmac_ops = { > @@ -890,6 +925,7 @@ const struct xgene_mac_ops xgene_gmac_ops = { > .tx_enable = xgene_gmac_tx_enable, > .rx_disable = xgene_gmac_rx_disable, > .tx_disable = xgene_gmac_tx_disable, > + .set_speed = xgene_gmac_set_speed, > .set_mac_addr = xgene_gmac_set_mac_addr, > }; > > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h > index 45220be..5540db9 100644 > --- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h > +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h > @@ -104,6 +104,7 @@ enum xgene_enet_rm { > #define RECOMBBUF BIT(27) > > #define MAC_OFFSET 0x30 > +#define PORT_OFFSET 0x4 > > #define BLOCK_ETH_CSR_OFFSET 0x2000 > #define BLOCK_ETH_CLE_CSR_OFFSET 0x6000 > @@ -160,7 +161,9 @@ enum xgene_enet_rm { > #define CFG_CLE_DSTQID0(val) (val & GENMASK(11, 0)) > #define CFG_CLE_FPSEL0(val) ((val << 16) & GENMASK(19, 16)) > #define ICM_CONFIG0_REG_0_ADDR 0x0400 > +#define ICM_CONFIG0_REG_1_ADDR 0x0408 > #define ICM_CONFIG2_REG_0_ADDR 0x0410 > +#define ICM_CONFIG2_REG_1_ADDR 0x0414 > #define RX_DV_GATE_REG_0_ADDR 0x05fc > #define TX_DV_GATE_EN0 BIT(2) > #define RX_DV_GATE_EN0 BIT(1) > @@ -347,6 +350,8 @@ void xgene_enet_parse_error(struct xgene_enet_desc_ring *ring, > int xgene_enet_mdio_config(struct xgene_enet_pdata *pdata); > void xgene_enet_mdio_remove(struct xgene_enet_pdata *pdata); > bool xgene_ring_mgr_init(struct xgene_enet_pdata *p); > +int xgene_enet_phy_connect(struct net_device *ndev); > +void xgene_enet_phy_disconnect(struct xgene_enet_pdata *pdata); > > extern const struct xgene_mac_ops xgene_gmac_ops; > extern const struct xgene_port_ops xgene_gport_ops; > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c > index d208b17..d451e5d 100644 > --- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c > +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c > @@ -727,11 +727,12 @@ static int xgene_enet_open(struct net_device *ndev) > ret = xgene_enet_register_irq(ndev); > if (ret) > return ret; > - > - if (pdata->phy_mode == PHY_INTERFACE_MODE_RGMII) > + if (pdata->phy_dev) { > phy_start(pdata->phy_dev); > - else > + } else { > schedule_delayed_work(&pdata->link_work, PHY_POLL_LINK_OFF); > + netif_carrier_off(ndev); > + } > > netif_start_queue(ndev); > > @@ -746,7 +747,7 @@ static int xgene_enet_close(struct net_device *ndev) > > netif_stop_queue(ndev); > > - if (pdata->phy_mode == PHY_INTERFACE_MODE_RGMII) > + if (pdata->phy_dev) > phy_stop(pdata->phy_dev); > else > cancel_delayed_work_sync(&pdata->link_work); > @@ -1291,6 +1292,7 @@ static int xgene_enet_get_resources(struct xgene_enet_pdata *pdata) > struct resource *res; > void __iomem *base_addr; > u32 offset; > + const char *ph; > int ret = 0; > > pdev = pdata->pdev; > @@ -1368,13 +1370,18 @@ static int xgene_enet_get_resources(struct xgene_enet_pdata *pdata) > if (ret) > return ret; > > + ret = device_property_read_string(dev, "phy-handle", &ph); > + if (!ret) > + pdata->mdio_driver = true; > + > pdata->clk = devm_clk_get(&pdev->dev, NULL); > if (IS_ERR(pdata->clk)) { > /* Firmware may have set up the clock already. */ > dev_info(dev, "clocks have been setup already\n"); > } > > - if (pdata->phy_mode != PHY_INTERFACE_MODE_XGMII) > + if ((pdata->phy_mode != PHY_INTERFACE_MODE_XGMII) && > + (pdata->enet_id == XGENE_ENET1)) > base_addr = pdata->base_addr - (pdata->port_id * MAC_OFFSET); > else > base_addr = pdata->base_addr; > @@ -1577,7 +1584,7 @@ static int xgene_enet_probe(struct platform_device *pdev) > struct net_device *ndev; > struct xgene_enet_pdata *pdata; > struct device *dev = &pdev->dev; > - const struct xgene_mac_ops *mac_ops; > + void (*link_state)(struct work_struct *); > const struct of_device_id *of_id; > int ret; > > @@ -1603,15 +1610,18 @@ static int xgene_enet_probe(struct platform_device *pdev) > if (of_id) { > pdata->enet_id = (enum xgene_enet_id)of_id->data; > } > -#ifdef CONFIG_ACPI > else { > +#ifdef CONFIG_ACPI > const struct acpi_device_id *acpi_id; > + enum xgene_enet_id enet_id; > > acpi_id = acpi_match_device(xgene_enet_acpi_match, &pdev->dev); > - if (acpi_id) > - pdata->enet_id = (enum xgene_enet_id) acpi_id->driver_data; > - } > + if (acpi_id) { > + enet_id = (enum xgene_enet_id)acpi_id->driver_data; > + pdata->enet_id = enet_id; > + } > #endif > + } > if (!pdata->enet_id) { > free_netdev(ndev); > return -ENODEV; > @@ -1645,13 +1655,18 @@ static int xgene_enet_probe(struct platform_device *pdev) > if (ret) > goto err_netdev; > > - mac_ops = pdata->mac_ops; > + link_state = pdata->mac_ops->link_state; > if (pdata->phy_mode == PHY_INTERFACE_MODE_RGMII) { > ret = xgene_enet_mdio_config(pdata); > if (ret) > goto err_netdev; > + } else if (pdata->phy_mode == PHY_INTERFACE_MODE_SGMII) { > + if (pdata->mdio_driver) > + ret = xgene_enet_phy_connect(ndev); > + else > + INIT_DELAYED_WORK(&pdata->link_work, link_state); > } else { > - INIT_DELAYED_WORK(&pdata->link_work, mac_ops->link_state); > + INIT_DELAYED_WORK(&pdata->link_work, link_state); > } > > xgene_enet_napi_add(pdata); > @@ -1679,6 +1694,8 @@ static int xgene_enet_remove(struct platform_device *pdev) > xgene_enet_napi_del(pdata); > if (pdata->phy_mode == PHY_INTERFACE_MODE_RGMII) > xgene_enet_mdio_remove(pdata); > + else if (pdata->phy_mode == PHY_INTERFACE_MODE_SGMII) > + xgene_enet_phy_disconnect(pdata); > unregister_netdev(ndev); > xgene_enet_delete_desc_rings(pdata); > pdata->port_ops->shutdown(pdata); > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h > index 092fbec..0fe1a96 100644 > --- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h > +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h > @@ -140,6 +140,7 @@ struct xgene_mac_ops { > void (*rx_enable)(struct xgene_enet_pdata *pdata); > void (*tx_disable)(struct xgene_enet_pdata *pdata); > void (*rx_disable)(struct xgene_enet_pdata *pdata); > + void (*set_speed)(struct xgene_enet_pdata *pdata); > void (*set_mac_addr)(struct xgene_enet_pdata *pdata); > void (*set_mss)(struct xgene_enet_pdata *pdata); > void (*link_state)(struct work_struct *work); > @@ -211,6 +212,7 @@ struct xgene_enet_pdata { > u32 mss; > u8 tx_delay; > u8 rx_delay; > + bool mdio_driver; > }; > > struct xgene_indirect_ctl { > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c b/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c > index 7847551..a7a6c05 100644 > --- a/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c > +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c > @@ -28,6 +28,12 @@ static void xgene_enet_wr_csr(struct xgene_enet_pdata *p, u32 offset, u32 val) > iowrite32(val, p->eth_csr_addr + offset); > } > > +static void xgene_enet_wr_clkrst_csr(struct xgene_enet_pdata *p, u32 offset, > + u32 val) > +{ > + iowrite32(val, p->base_addr + offset); > +} > + > static void xgene_enet_wr_ring_if(struct xgene_enet_pdata *p, > u32 offset, u32 val) > { > @@ -93,6 +99,11 @@ static u32 xgene_enet_rd_diag_csr(struct xgene_enet_pdata *p, u32 offset) > return ioread32(p->eth_diag_csr_addr + offset); > } > > +static u32 xgene_enet_rd_mcx_csr(struct xgene_enet_pdata *p, u32 offset) > +{ > + return ioread32(p->mcx_mac_csr_addr + offset); > +} > + > static u32 xgene_enet_rd_indirect(struct xgene_indirect_ctl *ctl, u32 rd_addr) > { > u32 rd_data; > @@ -229,21 +240,97 @@ static u32 xgene_enet_link_status(struct xgene_enet_pdata *p) > > data = xgene_mii_phy_read(p, INT_PHY_ADDR, > SGMII_BASE_PAGE_ABILITY_ADDR >> 2); > + if (LINK_SPEED(data) == PHY_SPEED_1000) > + p->phy_speed = SPEED_1000; > + else if (LINK_SPEED(data) == PHY_SPEED_100) > + p->phy_speed = SPEED_100; > + else > + p->phy_speed = SPEED_10; > > return data & LINK_UP; > } > > +static void xgene_sgmac_set_speed(struct xgene_enet_pdata *p) > +{ > + u32 icm0_addr, icm2_addr, debug_addr; > + u32 icm0, icm2, intf_ctl; > + u32 mc2, value; > + > + if (p->phy_speed != SPEED_UNKNOWN) { > + value = xgene_mii_phy_read(p, INT_PHY_ADDR, > + SGMII_BASE_PAGE_ABILITY_ADDR >> 2); > + if (!(value & LINK_UP)) { > + xgene_mii_phy_write(p, INT_PHY_ADDR, > + SGMII_TBI_CONTROL_ADDR >> 2, > + 0x8000); > + xgene_mii_phy_write(p, INT_PHY_ADDR, > + SGMII_TBI_CONTROL_ADDR >> 2, 0x0); > + } > + } > + > + if (p->enet_id == XGENE_ENET1) { > + icm0_addr = (!p->port_id) ? > + ICM_CONFIG0_REG_0_ADDR : ICM_CONFIG0_REG_1_ADDR; > + icm2_addr = (!p->port_id) ? > + ICM_CONFIG2_REG_0_ADDR : ICM_CONFIG2_REG_1_ADDR; > + debug_addr = DEBUG_REG_ADDR; > + } else { > + icm0_addr = XG_MCX_ICM_CONFIG0_REG_0_ADDR; > + icm2_addr = XG_MCX_ICM_CONFIG2_REG_0_ADDR; > + debug_addr = XG_DEBUG_REG_ADDR; > + } > + > + icm0 = xgene_enet_rd_mcx_csr(p, icm0_addr); > + icm2 = xgene_enet_rd_mcx_csr(p, icm2_addr); > + mc2 = xgene_enet_rd_mac(p, MAC_CONFIG_2_ADDR); > + intf_ctl = xgene_enet_rd_mac(p, INTERFACE_CONTROL_ADDR); > + > + switch (p->phy_speed) { > + case SPEED_10: > + ENET_INTERFACE_MODE2_SET(&mc2, 1); > + intf_ctl &= ~(ENET_LHD_MODE | ENET_GHD_MODE); > + CFG_MACMODE_SET(&icm0, 0); > + CFG_WAITASYNCRD_SET(&icm2, 500); > + break; > + case SPEED_100: > + ENET_INTERFACE_MODE2_SET(&mc2, 1); > + intf_ctl &= ~ENET_GHD_MODE; > + intf_ctl |= ENET_LHD_MODE; > + CFG_MACMODE_SET(&icm0, 1); > + CFG_WAITASYNCRD_SET(&icm2, 80); > + break; > + default: > + ENET_INTERFACE_MODE2_SET(&mc2, 2); > + intf_ctl &= ~ENET_LHD_MODE; > + intf_ctl |= ENET_GHD_MODE; > + CFG_MACMODE_SET(&icm0, 2); > + CFG_WAITASYNCRD_SET(&icm2, 16); > + value = xgene_enet_rd_csr(p, debug_addr); > + value |= CFG_BYPASS_UNISEC_TX | CFG_BYPASS_UNISEC_RX; > + xgene_enet_wr_csr(p, debug_addr, value); > + break; > + } > + > + mc2 |= FULL_DUPLEX2 | PAD_CRC; > + xgene_enet_wr_mac(p, MAC_CONFIG_2_ADDR, mc2); > + xgene_enet_wr_mac(p, INTERFACE_CONTROL_ADDR, intf_ctl); > + xgene_enet_wr_mcx_csr(p, icm0_addr, icm0); > + xgene_enet_wr_mcx_csr(p, icm2_addr, icm2); > +} > + > static void xgene_sgmac_init(struct xgene_enet_pdata *p) > { > u32 data, loop = 10; > - u32 offset = p->port_id * 4; > + u32 offset = 0; > u32 enet_spare_cfg_reg, rsif_config_reg; > u32 cfg_bypass_reg, rx_dv_gate_reg; > > xgene_sgmac_reset(p); > > /* Enable auto-negotiation */ > - xgene_mii_phy_write(p, INT_PHY_ADDR, SGMII_CONTROL_ADDR >> 2, 0x1000); > + xgene_mii_phy_write(p, INT_PHY_ADDR, SGMII_TBI_CONTROL_ADDR >> 2, > + 0x8000); Please use a define like SGMII_TBI_CONTROL_RESET or something like this. > + xgene_mii_phy_write(p, INT_PHY_ADDR, SGMII_CONTROL_ADDR >> 2, 0x9000); Same here SGMII_CONTROL_RESET | SMGII_CONTROL_AUTO_NEG > xgene_mii_phy_write(p, INT_PHY_ADDR, SGMII_TBI_CONTROL_ADDR >> 2, 0); > > while (loop--) { > @@ -256,16 +343,14 @@ static void xgene_sgmac_init(struct xgene_enet_pdata *p) > if (!(data & AUTO_NEG_COMPLETE) || !(data & LINK_STATUS)) > netdev_err(p->ndev, "Auto-negotiation failed\n"); > > - data = xgene_enet_rd_mac(p, MAC_CONFIG_2_ADDR); > - ENET_INTERFACE_MODE2_SET(&data, 2); > - xgene_enet_wr_mac(p, MAC_CONFIG_2_ADDR, data | FULL_DUPLEX2); > - xgene_enet_wr_mac(p, INTERFACE_CONTROL_ADDR, ENET_GHD_MODE); > + xgene_sgmac_set_speed(p); Why don't we use the mac_ops->set_speed approach here? > > if (p->enet_id == XGENE_ENET1) { > enet_spare_cfg_reg = ENET_SPARE_CFG_REG_ADDR; > rsif_config_reg = RSIF_CONFIG_REG_ADDR; > cfg_bypass_reg = CFG_BYPASS_ADDR; > rx_dv_gate_reg = SG_RX_DV_GATE_REG_0_ADDR; > + offset = p->port_id * PORT_OFFSET; > } else { > enet_spare_cfg_reg = XG_ENET_SPARE_CFG_REG_ADDR; > rsif_config_reg = XG_RSIF_CONFIG_REG_ADDR; > @@ -334,6 +419,9 @@ static int xgene_enet_reset(struct xgene_enet_pdata *p) > if (!xgene_ring_mgr_init(p)) > return -ENODEV; > > + if (p->enet_id == XGENE_ENET2) > + xgene_enet_wr_clkrst_csr(p, XGENET_CONFIG_REG_ADDR, SGMII_EN); > + > if (!IS_ERR(p->clk)) { > clk_prepare_enable(p->clk); > clk_disable_unprepare(p->clk); > @@ -386,10 +474,11 @@ static void xgene_enet_link_state(struct work_struct *work) > if (link) { > if (!netif_carrier_ok(ndev)) { > netif_carrier_on(ndev); > - xgene_sgmac_init(p); > + xgene_sgmac_set_speed(p); > xgene_sgmac_rx_enable(p); > xgene_sgmac_tx_enable(p); Same here. > - netdev_info(ndev, "Link is Up - 1Gbps\n"); > + netdev_info(ndev, "Link is Up - %dMbps\n", > + p->phy_speed); > } > poll_interval = PHY_POLL_LINK_ON; > } else { > @@ -412,6 +501,7 @@ const struct xgene_mac_ops xgene_sgmac_ops = { > .tx_enable = xgene_sgmac_tx_enable, > .rx_disable = xgene_sgmac_rx_disable, > .tx_disable = xgene_sgmac_tx_disable, > + .set_speed = xgene_sgmac_set_speed, > .set_mac_addr = xgene_sgmac_set_mac_addr, > .link_state = xgene_enet_link_state > }; > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h b/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h > index 002df5a..3d0ba37 100644 > --- a/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h > +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h > @@ -24,6 +24,7 @@ > #define PHY_ADDR(src) (((src)<<8) & GENMASK(12, 8)) > #define REG_ADDR(src) ((src) & GENMASK(4, 0)) > #define PHY_CONTROL(src) ((src) & GENMASK(15, 0)) > +#define LINK_SPEED(src) (((src) & GENMASK(11, 10)) >> 10) > #define INT_PHY_ADDR 0x1e > #define SGMII_TBI_CONTROL_ADDR 0x44 > #define SGMII_CONTROL_ADDR 0x00 > @@ -34,6 +35,13 @@ > #define LINK_UP BIT(15) > #define MPA_IDLE_WITH_QMI_EMPTY BIT(12) > #define SG_RX_DV_GATE_REG_0_ADDR 0x05fc > +#define SGMII_EN 0x1 > + > +enum xgene_phy_speed { > + PHY_SPEED_10, > + PHY_SPEED_100, > + PHY_SPEED_1000 > +}; > > extern const struct xgene_mac_ops xgene_sgmac_ops; > extern const struct xgene_port_ops xgene_sgport_ops; > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h b/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h > index 0a2dca8..aba4c19 100644 > --- a/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h > +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h > @@ -65,9 +65,13 @@ > #define XG_CFG_LINK_AGGR_RESUME_0_ADDR 0x0214 > #define XG_LINK_STATUS_ADDR 0x0228 > #define XG_TSIF_MSS_REG0_ADDR 0x02a4 > +#define XG_DEBUG_REG_ADDR 0x0400 > #define XG_ENET_SPARE_CFG_REG_ADDR 0x040c > #define XG_ENET_SPARE_CFG_REG_1_ADDR 0x0410 > #define XGENET_RX_DV_GATE_REG_0_ADDR 0x0804 > +#define XG_MCX_ECM_CONFIG0_REG_0_ADDR 0x0070 This define is not used in this patch and as far as I can see not even in this series. You can delete this. > +#define XG_MCX_ICM_CONFIG0_REG_0_ADDR 0x00e0 > +#define XG_MCX_ICM_CONFIG2_REG_0_ADDR 0x00e8 > > extern const struct xgene_mac_ops xgene_xgmac_ops; > extern const struct xgene_port_ops xgene_xgport_ops; > Regards, Matthias From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matthias Brugger Subject: Re: [PATCH v3 1/5] drivers: net: xgene: MAC and PHY configuration changes Date: Wed, 8 Jun 2016 11:06:25 +0200 Message-ID: <5757E011.5090909@suse.com> References: <1465236962-12131-1-git-send-email-isubramanian@apm.com> <1465236962-12131-2-git-send-email-isubramanian@apm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1465236962-12131-2-git-send-email-isubramanian@apm.com> Sender: netdev-owner@vger.kernel.org To: Iyappan Subramanian , davem@davemloft.net, netdev@vger.kernel.org, devicetree@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org, patches@apm.com, andrew@lunn.ch List-Id: devicetree@vger.kernel.org On 06/06/16 20:15, Iyappan Subramanian wrote: > This patch fixes MAC configuration to support 10/100GbE for SGMII and > link_state call back. It also sets pdata->mdio_driver flag based on > ethernet mdio subnode and prepare for MDIO driver support. > > In summary, following are the changes, > > - Added set_speed function pointer in mac_ops This can be a seperate patch, or would it break the driver? In the end, the decision how to split up the patches is up to Dave. I prefer to have more small patches which do incrementally add new stuff to the driver, taking into account that every single patch does not break it. > - Changed link_state to call the set_speed > - Add 10/100 support for SGMII based 1G > - Fixed mac_init for 1G > > - Call mac_ops rx_enable/disable and tx_enable/disable function pointers For me, this is a seperate patch which does a cleanup of the driver. You just use the mac_ops for these function (enable/disable and init) and in a second patch you add the set_speed and delete the init call. > - Add acpi_phy_find_device to find PHY using phy-handle reference object > - Changing phy_start and phy_stop calls based on phy_dev object existence > - Calling phy_connect based on pdata->mdio_driver flag > > Signed-off-by: Iyappan Subramanian > Tested-by: Fushen Chen > Tested-by: Toan Le > Tested-by: Matthias Brugger > --- > drivers/net/ethernet/apm/xgene/xgene_enet_hw.c | 190 +++++++++++++--------- > drivers/net/ethernet/apm/xgene/xgene_enet_hw.h | 5 + > drivers/net/ethernet/apm/xgene/xgene_enet_main.c | 41 +++-- > drivers/net/ethernet/apm/xgene/xgene_enet_main.h | 2 + > drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c | 106 +++++++++++- > drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h | 8 + > drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h | 4 + > 7 files changed, 259 insertions(+), 97 deletions(-) > > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c > index 2f5638f..5d6d14b 100644 > --- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c > +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c > @@ -512,14 +512,11 @@ static void xgene_enet_configure_clock(struct xgene_enet_pdata *pdata) > #endif > } > > -static void xgene_gmac_init(struct xgene_enet_pdata *pdata) > +static void xgene_gmac_set_speed(struct xgene_enet_pdata *pdata) > { > struct device *dev = &pdata->pdev->dev; > - u32 value, mc2; > - u32 intf_ctl, rgmii; > - u32 icm0, icm2; > - > - xgene_gmac_reset(pdata); > + u32 icm0, icm2, mc2; > + u32 intf_ctl, rgmii, value; > > xgene_enet_rd_mcx_csr(pdata, ICM_CONFIG0_REG_0_ADDR, &icm0); > xgene_enet_rd_mcx_csr(pdata, ICM_CONFIG2_REG_0_ADDR, &icm2); > @@ -564,7 +561,18 @@ static void xgene_gmac_init(struct xgene_enet_pdata *pdata) > mc2 |= FULL_DUPLEX2 | PAD_CRC; > xgene_enet_wr_mcx_mac(pdata, MAC_CONFIG_2_ADDR, mc2); > xgene_enet_wr_mcx_mac(pdata, INTERFACE_CONTROL_ADDR, intf_ctl); > + xgene_enet_wr_csr(pdata, RGMII_REG_0_ADDR, rgmii); > + xgene_enet_configure_clock(pdata); > + > + xgene_enet_wr_mcx_csr(pdata, ICM_CONFIG0_REG_0_ADDR, icm0); > + xgene_enet_wr_mcx_csr(pdata, ICM_CONFIG2_REG_0_ADDR, icm2); > +} > > +static void xgene_gmac_init(struct xgene_enet_pdata *pdata) > +{ > + u32 value; > + > + xgene_gmac_set_speed(pdata); > xgene_gmac_set_mac_addr(pdata); > > /* Adjust MDC clock frequency */ > @@ -579,15 +587,10 @@ static void xgene_gmac_init(struct xgene_enet_pdata *pdata) > > /* Rtype should be copied from FP */ > xgene_enet_wr_csr(pdata, RSIF_RAM_DBG_REG0_ADDR, 0); > - xgene_enet_wr_csr(pdata, RGMII_REG_0_ADDR, rgmii); > - xgene_enet_configure_clock(pdata); > > /* Rx-Tx traffic resume */ > xgene_enet_wr_csr(pdata, CFG_LINK_AGGR_RESUME_0_ADDR, TX_PORT0); > > - xgene_enet_wr_mcx_csr(pdata, ICM_CONFIG0_REG_0_ADDR, icm0); > - xgene_enet_wr_mcx_csr(pdata, ICM_CONFIG2_REG_0_ADDR, icm2); > - Is this the part you refer to with "Fixed mac_init for 1G"? What does it fix? Does this fix something which might be worth getting backported to older kernel versions? > xgene_enet_rd_mcx_csr(pdata, RX_DV_GATE_REG_0_ADDR, &value); > value &= ~TX_DV_GATE_EN0; > value &= ~RX_DV_GATE_EN0; > @@ -671,25 +674,12 @@ bool xgene_ring_mgr_init(struct xgene_enet_pdata *p) > > static int xgene_enet_reset(struct xgene_enet_pdata *pdata) > { > - u32 val; > - > if (!xgene_ring_mgr_init(pdata)) > return -ENODEV; > > - if (!IS_ERR(pdata->clk)) { > - clk_prepare_enable(pdata->clk); > - clk_disable_unprepare(pdata->clk); > - clk_prepare_enable(pdata->clk); > - xgene_enet_ecc_init(pdata); > - } > + xgene_enet_ecc_init(pdata); > xgene_enet_config_ring_if_assoc(pdata); > > - /* Enable auto-incr for scanning */ > - xgene_enet_rd_mcx_mac(pdata, MII_MGMT_CONFIG_ADDR, &val); > - val |= SCAN_AUTO_INCR; > - MGMT_CLOCK_SEL_SET(&val, 1); > - xgene_enet_wr_mcx_mac(pdata, MII_MGMT_CONFIG_ADDR, val); > - > return 0; > } > > @@ -724,29 +714,49 @@ static int xgene_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum, > static void xgene_enet_adjust_link(struct net_device *ndev) > { > struct xgene_enet_pdata *pdata = netdev_priv(ndev); > + const struct xgene_mac_ops *mac_ops = pdata->mac_ops; > struct phy_device *phydev = pdata->phy_dev; > > if (phydev->link) { > if (pdata->phy_speed != phydev->speed) { > pdata->phy_speed = phydev->speed; > - xgene_gmac_init(pdata); > - xgene_gmac_rx_enable(pdata); > - xgene_gmac_tx_enable(pdata); > + mac_ops->set_speed(pdata); > + mac_ops->rx_enable(pdata); > + mac_ops->tx_enable(pdata); > phy_print_status(phydev); > } > } else { > - xgene_gmac_rx_disable(pdata); > - xgene_gmac_tx_disable(pdata); > + mac_ops->rx_disable(pdata); > + mac_ops->tx_disable(pdata); > pdata->phy_speed = SPEED_UNKNOWN; > phy_print_status(phydev); > } > } > > -static int xgene_enet_phy_connect(struct net_device *ndev) > +#ifdef CONFIG_ACPI > +static struct acpi_device *acpi_phy_find_device(struct device *dev) > +{ > + struct acpi_reference_args args; > + struct fwnode_handle *fw_node; > + int status; > + > + fw_node = acpi_fwnode_handle(ACPI_COMPANION(dev)); > + status = acpi_node_get_property_reference(fw_node, "phy-handle", 0, > + &args); > + if (ACPI_FAILURE(status)) { > + dev_dbg(dev, "No matching phy in ACPI table\n"); > + return NULL; > + } > + > + return args.adev; > +} > +#endif > + > +int xgene_enet_phy_connect(struct net_device *ndev) > { > struct xgene_enet_pdata *pdata = netdev_priv(ndev); > struct device_node *phy_np; > - struct phy_device *phy_dev; > + struct phy_device *phy_dev = NULL; > struct device *dev = &pdata->pdev->dev; > > if (dev->of_node) { > @@ -756,23 +766,25 @@ static int xgene_enet_phy_connect(struct net_device *ndev) > return -ENODEV; > } > > - phy_dev = of_phy_connect(ndev, phy_np, &xgene_enet_adjust_link, > - 0, pdata->phy_mode); > - if (!phy_dev) { > - netdev_err(ndev, "Could not connect to PHY\n"); > - return -ENODEV; > - } > - > - pdata->phy_dev = phy_dev; > + pdata->phy_dev = of_phy_find_device(phy_np); > } else { > - phy_dev = pdata->phy_dev; > +#ifdef CONFIG_ACPI > + if (pdata->mdio_driver) { > + struct acpi_device *adev; > > - if (!phy_dev || > - phy_connect_direct(ndev, phy_dev, &xgene_enet_adjust_link, > - pdata->phy_mode)) { > - netdev_err(ndev, "Could not connect to PHY\n"); > - return -ENODEV; > + adev = acpi_phy_find_device(dev); > + if (adev) > + pdata->phy_dev = adev->driver_data; > } > +#endif > + } > + > + phy_dev = pdata->phy_dev; > + if (!phy_dev || > + phy_connect_direct(ndev, phy_dev, &xgene_enet_adjust_link, > + pdata->phy_mode)) { > + netdev_err(ndev, "Could not connect to PHY\n"); > + return -ENODEV; > } > > pdata->phy_speed = SPEED_UNKNOWN; > @@ -788,12 +800,9 @@ static int xgene_mdiobus_register(struct xgene_enet_pdata *pdata, > struct mii_bus *mdio) > { > struct device *dev = &pdata->pdev->dev; > - struct net_device *ndev = pdata->ndev; > - struct phy_device *phy; > - struct device_node *child_np; > struct device_node *mdio_np = NULL; > - int ret; > - u32 phy_id; > + struct device_node *child_np; > + u32 phyid; > > if (dev->of_node) { > for_each_child_of_node(dev->of_node, child_np) { > @@ -805,38 +814,50 @@ static int xgene_mdiobus_register(struct xgene_enet_pdata *pdata, > } > > if (!mdio_np) { > - netdev_dbg(ndev, "No mdio node in the dts\n"); > - return -ENXIO; > + mdiobus_free(mdio); > + return 0; > } > > + pdata->mdio_driver = false; > + > return of_mdiobus_register(mdio, mdio_np); > - } > + } else { > +#ifdef CONFIG_ACPI > + struct phy_device *phy; > + int ret; > > - /* Mask out all PHYs from auto probing. */ > - mdio->phy_mask = ~0; > + if (pdata->mdio_driver) { > + mdiobus_free(mdio); > + return 0; > + } > > - /* Register the MDIO bus */ > - ret = mdiobus_register(mdio); > - if (ret) > - return ret; > + /* Mask out all PHYs from auto probing. */ > + mdio->phy_mask = ~0; > > - ret = device_property_read_u32(dev, "phy-channel", &phy_id); > - if (ret) > - ret = device_property_read_u32(dev, "phy-addr", &phy_id); > - if (ret) > - return -EINVAL; > + /* Register the MDIO bus */ > + ret = mdiobus_register(mdio); > + if (ret) > + return ret; > > - phy = get_phy_device(mdio, phy_id, false); > - if (IS_ERR(phy)) > - return -EIO; > + ret = device_property_read_u32(dev, "phy-channel", &phyid); > + if (ret) > + ret = device_property_read_u32(dev, "phy-addr", &phyid); > + if (ret) > + return -EINVAL; > > - ret = phy_device_register(phy); > - if (ret) > - phy_device_free(phy); > - else > - pdata->phy_dev = phy; > + phy = get_phy_device(mdio, phyid, false); > + if (IS_ERR(phy)) > + return -EIO; > > - return ret; > + ret = phy_device_register(phy); > + if (ret) > + phy_device_free(phy); > + else > + pdata->phy_dev = phy; > + > + return ret; > +#endif > + } > } > > int xgene_enet_mdio_config(struct xgene_enet_pdata *pdata) > @@ -861,7 +882,13 @@ int xgene_enet_mdio_config(struct xgene_enet_pdata *pdata) > ret = xgene_mdiobus_register(pdata, mdio_bus); > if (ret) { > netdev_err(ndev, "Failed to register MDIO bus\n"); > + if (mdio_bus->state == MDIOBUS_REGISTERED) > + mdiobus_unregister(pdata->mdio_bus); > mdiobus_free(mdio_bus); > + if (pdata->mdio_driver) { > + ret = xgene_enet_phy_connect(ndev); > + return 0; > + } > return ret; > } > pdata->mdio_bus = mdio_bus; > @@ -873,14 +900,22 @@ int xgene_enet_mdio_config(struct xgene_enet_pdata *pdata) > return ret; > } > > +void xgene_enet_phy_disconnect(struct xgene_enet_pdata *pdata) > +{ > + if (pdata->phy_dev) > + phy_disconnect(pdata->phy_dev); > +} > + > void xgene_enet_mdio_remove(struct xgene_enet_pdata *pdata) > { > if (pdata->phy_dev) > phy_disconnect(pdata->phy_dev); > > - mdiobus_unregister(pdata->mdio_bus); > - mdiobus_free(pdata->mdio_bus); > - pdata->mdio_bus = NULL; > + if (!pdata->mdio_driver) { > + mdiobus_unregister(pdata->mdio_bus); > + mdiobus_free(pdata->mdio_bus); > + pdata->mdio_bus = NULL; > + } > } > > const struct xgene_mac_ops xgene_gmac_ops = { > @@ -890,6 +925,7 @@ const struct xgene_mac_ops xgene_gmac_ops = { > .tx_enable = xgene_gmac_tx_enable, > .rx_disable = xgene_gmac_rx_disable, > .tx_disable = xgene_gmac_tx_disable, > + .set_speed = xgene_gmac_set_speed, > .set_mac_addr = xgene_gmac_set_mac_addr, > }; > > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h > index 45220be..5540db9 100644 > --- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h > +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h > @@ -104,6 +104,7 @@ enum xgene_enet_rm { > #define RECOMBBUF BIT(27) > > #define MAC_OFFSET 0x30 > +#define PORT_OFFSET 0x4 > > #define BLOCK_ETH_CSR_OFFSET 0x2000 > #define BLOCK_ETH_CLE_CSR_OFFSET 0x6000 > @@ -160,7 +161,9 @@ enum xgene_enet_rm { > #define CFG_CLE_DSTQID0(val) (val & GENMASK(11, 0)) > #define CFG_CLE_FPSEL0(val) ((val << 16) & GENMASK(19, 16)) > #define ICM_CONFIG0_REG_0_ADDR 0x0400 > +#define ICM_CONFIG0_REG_1_ADDR 0x0408 > #define ICM_CONFIG2_REG_0_ADDR 0x0410 > +#define ICM_CONFIG2_REG_1_ADDR 0x0414 > #define RX_DV_GATE_REG_0_ADDR 0x05fc > #define TX_DV_GATE_EN0 BIT(2) > #define RX_DV_GATE_EN0 BIT(1) > @@ -347,6 +350,8 @@ void xgene_enet_parse_error(struct xgene_enet_desc_ring *ring, > int xgene_enet_mdio_config(struct xgene_enet_pdata *pdata); > void xgene_enet_mdio_remove(struct xgene_enet_pdata *pdata); > bool xgene_ring_mgr_init(struct xgene_enet_pdata *p); > +int xgene_enet_phy_connect(struct net_device *ndev); > +void xgene_enet_phy_disconnect(struct xgene_enet_pdata *pdata); > > extern const struct xgene_mac_ops xgene_gmac_ops; > extern const struct xgene_port_ops xgene_gport_ops; > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c > index d208b17..d451e5d 100644 > --- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c > +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c > @@ -727,11 +727,12 @@ static int xgene_enet_open(struct net_device *ndev) > ret = xgene_enet_register_irq(ndev); > if (ret) > return ret; > - > - if (pdata->phy_mode == PHY_INTERFACE_MODE_RGMII) > + if (pdata->phy_dev) { > phy_start(pdata->phy_dev); > - else > + } else { > schedule_delayed_work(&pdata->link_work, PHY_POLL_LINK_OFF); > + netif_carrier_off(ndev); > + } > > netif_start_queue(ndev); > > @@ -746,7 +747,7 @@ static int xgene_enet_close(struct net_device *ndev) > > netif_stop_queue(ndev); > > - if (pdata->phy_mode == PHY_INTERFACE_MODE_RGMII) > + if (pdata->phy_dev) > phy_stop(pdata->phy_dev); > else > cancel_delayed_work_sync(&pdata->link_work); > @@ -1291,6 +1292,7 @@ static int xgene_enet_get_resources(struct xgene_enet_pdata *pdata) > struct resource *res; > void __iomem *base_addr; > u32 offset; > + const char *ph; > int ret = 0; > > pdev = pdata->pdev; > @@ -1368,13 +1370,18 @@ static int xgene_enet_get_resources(struct xgene_enet_pdata *pdata) > if (ret) > return ret; > > + ret = device_property_read_string(dev, "phy-handle", &ph); > + if (!ret) > + pdata->mdio_driver = true; > + > pdata->clk = devm_clk_get(&pdev->dev, NULL); > if (IS_ERR(pdata->clk)) { > /* Firmware may have set up the clock already. */ > dev_info(dev, "clocks have been setup already\n"); > } > > - if (pdata->phy_mode != PHY_INTERFACE_MODE_XGMII) > + if ((pdata->phy_mode != PHY_INTERFACE_MODE_XGMII) && > + (pdata->enet_id == XGENE_ENET1)) > base_addr = pdata->base_addr - (pdata->port_id * MAC_OFFSET); > else > base_addr = pdata->base_addr; > @@ -1577,7 +1584,7 @@ static int xgene_enet_probe(struct platform_device *pdev) > struct net_device *ndev; > struct xgene_enet_pdata *pdata; > struct device *dev = &pdev->dev; > - const struct xgene_mac_ops *mac_ops; > + void (*link_state)(struct work_struct *); > const struct of_device_id *of_id; > int ret; > > @@ -1603,15 +1610,18 @@ static int xgene_enet_probe(struct platform_device *pdev) > if (of_id) { > pdata->enet_id = (enum xgene_enet_id)of_id->data; > } > -#ifdef CONFIG_ACPI > else { > +#ifdef CONFIG_ACPI > const struct acpi_device_id *acpi_id; > + enum xgene_enet_id enet_id; > > acpi_id = acpi_match_device(xgene_enet_acpi_match, &pdev->dev); > - if (acpi_id) > - pdata->enet_id = (enum xgene_enet_id) acpi_id->driver_data; > - } > + if (acpi_id) { > + enet_id = (enum xgene_enet_id)acpi_id->driver_data; > + pdata->enet_id = enet_id; > + } > #endif > + } > if (!pdata->enet_id) { > free_netdev(ndev); > return -ENODEV; > @@ -1645,13 +1655,18 @@ static int xgene_enet_probe(struct platform_device *pdev) > if (ret) > goto err_netdev; > > - mac_ops = pdata->mac_ops; > + link_state = pdata->mac_ops->link_state; > if (pdata->phy_mode == PHY_INTERFACE_MODE_RGMII) { > ret = xgene_enet_mdio_config(pdata); > if (ret) > goto err_netdev; > + } else if (pdata->phy_mode == PHY_INTERFACE_MODE_SGMII) { > + if (pdata->mdio_driver) > + ret = xgene_enet_phy_connect(ndev); > + else > + INIT_DELAYED_WORK(&pdata->link_work, link_state); > } else { > - INIT_DELAYED_WORK(&pdata->link_work, mac_ops->link_state); > + INIT_DELAYED_WORK(&pdata->link_work, link_state); > } > > xgene_enet_napi_add(pdata); > @@ -1679,6 +1694,8 @@ static int xgene_enet_remove(struct platform_device *pdev) > xgene_enet_napi_del(pdata); > if (pdata->phy_mode == PHY_INTERFACE_MODE_RGMII) > xgene_enet_mdio_remove(pdata); > + else if (pdata->phy_mode == PHY_INTERFACE_MODE_SGMII) > + xgene_enet_phy_disconnect(pdata); > unregister_netdev(ndev); > xgene_enet_delete_desc_rings(pdata); > pdata->port_ops->shutdown(pdata); > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h > index 092fbec..0fe1a96 100644 > --- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h > +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h > @@ -140,6 +140,7 @@ struct xgene_mac_ops { > void (*rx_enable)(struct xgene_enet_pdata *pdata); > void (*tx_disable)(struct xgene_enet_pdata *pdata); > void (*rx_disable)(struct xgene_enet_pdata *pdata); > + void (*set_speed)(struct xgene_enet_pdata *pdata); > void (*set_mac_addr)(struct xgene_enet_pdata *pdata); > void (*set_mss)(struct xgene_enet_pdata *pdata); > void (*link_state)(struct work_struct *work); > @@ -211,6 +212,7 @@ struct xgene_enet_pdata { > u32 mss; > u8 tx_delay; > u8 rx_delay; > + bool mdio_driver; > }; > > struct xgene_indirect_ctl { > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c b/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c > index 7847551..a7a6c05 100644 > --- a/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c > +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c > @@ -28,6 +28,12 @@ static void xgene_enet_wr_csr(struct xgene_enet_pdata *p, u32 offset, u32 val) > iowrite32(val, p->eth_csr_addr + offset); > } > > +static void xgene_enet_wr_clkrst_csr(struct xgene_enet_pdata *p, u32 offset, > + u32 val) > +{ > + iowrite32(val, p->base_addr + offset); > +} > + > static void xgene_enet_wr_ring_if(struct xgene_enet_pdata *p, > u32 offset, u32 val) > { > @@ -93,6 +99,11 @@ static u32 xgene_enet_rd_diag_csr(struct xgene_enet_pdata *p, u32 offset) > return ioread32(p->eth_diag_csr_addr + offset); > } > > +static u32 xgene_enet_rd_mcx_csr(struct xgene_enet_pdata *p, u32 offset) > +{ > + return ioread32(p->mcx_mac_csr_addr + offset); > +} > + > static u32 xgene_enet_rd_indirect(struct xgene_indirect_ctl *ctl, u32 rd_addr) > { > u32 rd_data; > @@ -229,21 +240,97 @@ static u32 xgene_enet_link_status(struct xgene_enet_pdata *p) > > data = xgene_mii_phy_read(p, INT_PHY_ADDR, > SGMII_BASE_PAGE_ABILITY_ADDR >> 2); > + if (LINK_SPEED(data) == PHY_SPEED_1000) > + p->phy_speed = SPEED_1000; > + else if (LINK_SPEED(data) == PHY_SPEED_100) > + p->phy_speed = SPEED_100; > + else > + p->phy_speed = SPEED_10; > > return data & LINK_UP; > } > > +static void xgene_sgmac_set_speed(struct xgene_enet_pdata *p) > +{ > + u32 icm0_addr, icm2_addr, debug_addr; > + u32 icm0, icm2, intf_ctl; > + u32 mc2, value; > + > + if (p->phy_speed != SPEED_UNKNOWN) { > + value = xgene_mii_phy_read(p, INT_PHY_ADDR, > + SGMII_BASE_PAGE_ABILITY_ADDR >> 2); > + if (!(value & LINK_UP)) { > + xgene_mii_phy_write(p, INT_PHY_ADDR, > + SGMII_TBI_CONTROL_ADDR >> 2, > + 0x8000); > + xgene_mii_phy_write(p, INT_PHY_ADDR, > + SGMII_TBI_CONTROL_ADDR >> 2, 0x0); > + } > + } > + > + if (p->enet_id == XGENE_ENET1) { > + icm0_addr = (!p->port_id) ? > + ICM_CONFIG0_REG_0_ADDR : ICM_CONFIG0_REG_1_ADDR; > + icm2_addr = (!p->port_id) ? > + ICM_CONFIG2_REG_0_ADDR : ICM_CONFIG2_REG_1_ADDR; > + debug_addr = DEBUG_REG_ADDR; > + } else { > + icm0_addr = XG_MCX_ICM_CONFIG0_REG_0_ADDR; > + icm2_addr = XG_MCX_ICM_CONFIG2_REG_0_ADDR; > + debug_addr = XG_DEBUG_REG_ADDR; > + } > + > + icm0 = xgene_enet_rd_mcx_csr(p, icm0_addr); > + icm2 = xgene_enet_rd_mcx_csr(p, icm2_addr); > + mc2 = xgene_enet_rd_mac(p, MAC_CONFIG_2_ADDR); > + intf_ctl = xgene_enet_rd_mac(p, INTERFACE_CONTROL_ADDR); > + > + switch (p->phy_speed) { > + case SPEED_10: > + ENET_INTERFACE_MODE2_SET(&mc2, 1); > + intf_ctl &= ~(ENET_LHD_MODE | ENET_GHD_MODE); > + CFG_MACMODE_SET(&icm0, 0); > + CFG_WAITASYNCRD_SET(&icm2, 500); > + break; > + case SPEED_100: > + ENET_INTERFACE_MODE2_SET(&mc2, 1); > + intf_ctl &= ~ENET_GHD_MODE; > + intf_ctl |= ENET_LHD_MODE; > + CFG_MACMODE_SET(&icm0, 1); > + CFG_WAITASYNCRD_SET(&icm2, 80); > + break; > + default: > + ENET_INTERFACE_MODE2_SET(&mc2, 2); > + intf_ctl &= ~ENET_LHD_MODE; > + intf_ctl |= ENET_GHD_MODE; > + CFG_MACMODE_SET(&icm0, 2); > + CFG_WAITASYNCRD_SET(&icm2, 16); > + value = xgene_enet_rd_csr(p, debug_addr); > + value |= CFG_BYPASS_UNISEC_TX | CFG_BYPASS_UNISEC_RX; > + xgene_enet_wr_csr(p, debug_addr, value); > + break; > + } > + > + mc2 |= FULL_DUPLEX2 | PAD_CRC; > + xgene_enet_wr_mac(p, MAC_CONFIG_2_ADDR, mc2); > + xgene_enet_wr_mac(p, INTERFACE_CONTROL_ADDR, intf_ctl); > + xgene_enet_wr_mcx_csr(p, icm0_addr, icm0); > + xgene_enet_wr_mcx_csr(p, icm2_addr, icm2); > +} > + > static void xgene_sgmac_init(struct xgene_enet_pdata *p) > { > u32 data, loop = 10; > - u32 offset = p->port_id * 4; > + u32 offset = 0; > u32 enet_spare_cfg_reg, rsif_config_reg; > u32 cfg_bypass_reg, rx_dv_gate_reg; > > xgene_sgmac_reset(p); > > /* Enable auto-negotiation */ > - xgene_mii_phy_write(p, INT_PHY_ADDR, SGMII_CONTROL_ADDR >> 2, 0x1000); > + xgene_mii_phy_write(p, INT_PHY_ADDR, SGMII_TBI_CONTROL_ADDR >> 2, > + 0x8000); Please use a define like SGMII_TBI_CONTROL_RESET or something like this. > + xgene_mii_phy_write(p, INT_PHY_ADDR, SGMII_CONTROL_ADDR >> 2, 0x9000); Same here SGMII_CONTROL_RESET | SMGII_CONTROL_AUTO_NEG > xgene_mii_phy_write(p, INT_PHY_ADDR, SGMII_TBI_CONTROL_ADDR >> 2, 0); > > while (loop--) { > @@ -256,16 +343,14 @@ static void xgene_sgmac_init(struct xgene_enet_pdata *p) > if (!(data & AUTO_NEG_COMPLETE) || !(data & LINK_STATUS)) > netdev_err(p->ndev, "Auto-negotiation failed\n"); > > - data = xgene_enet_rd_mac(p, MAC_CONFIG_2_ADDR); > - ENET_INTERFACE_MODE2_SET(&data, 2); > - xgene_enet_wr_mac(p, MAC_CONFIG_2_ADDR, data | FULL_DUPLEX2); > - xgene_enet_wr_mac(p, INTERFACE_CONTROL_ADDR, ENET_GHD_MODE); > + xgene_sgmac_set_speed(p); Why don't we use the mac_ops->set_speed approach here? > > if (p->enet_id == XGENE_ENET1) { > enet_spare_cfg_reg = ENET_SPARE_CFG_REG_ADDR; > rsif_config_reg = RSIF_CONFIG_REG_ADDR; > cfg_bypass_reg = CFG_BYPASS_ADDR; > rx_dv_gate_reg = SG_RX_DV_GATE_REG_0_ADDR; > + offset = p->port_id * PORT_OFFSET; > } else { > enet_spare_cfg_reg = XG_ENET_SPARE_CFG_REG_ADDR; > rsif_config_reg = XG_RSIF_CONFIG_REG_ADDR; > @@ -334,6 +419,9 @@ static int xgene_enet_reset(struct xgene_enet_pdata *p) > if (!xgene_ring_mgr_init(p)) > return -ENODEV; > > + if (p->enet_id == XGENE_ENET2) > + xgene_enet_wr_clkrst_csr(p, XGENET_CONFIG_REG_ADDR, SGMII_EN); > + > if (!IS_ERR(p->clk)) { > clk_prepare_enable(p->clk); > clk_disable_unprepare(p->clk); > @@ -386,10 +474,11 @@ static void xgene_enet_link_state(struct work_struct *work) > if (link) { > if (!netif_carrier_ok(ndev)) { > netif_carrier_on(ndev); > - xgene_sgmac_init(p); > + xgene_sgmac_set_speed(p); > xgene_sgmac_rx_enable(p); > xgene_sgmac_tx_enable(p); Same here. > - netdev_info(ndev, "Link is Up - 1Gbps\n"); > + netdev_info(ndev, "Link is Up - %dMbps\n", > + p->phy_speed); > } > poll_interval = PHY_POLL_LINK_ON; > } else { > @@ -412,6 +501,7 @@ const struct xgene_mac_ops xgene_sgmac_ops = { > .tx_enable = xgene_sgmac_tx_enable, > .rx_disable = xgene_sgmac_rx_disable, > .tx_disable = xgene_sgmac_tx_disable, > + .set_speed = xgene_sgmac_set_speed, > .set_mac_addr = xgene_sgmac_set_mac_addr, > .link_state = xgene_enet_link_state > }; > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h b/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h > index 002df5a..3d0ba37 100644 > --- a/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h > +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h > @@ -24,6 +24,7 @@ > #define PHY_ADDR(src) (((src)<<8) & GENMASK(12, 8)) > #define REG_ADDR(src) ((src) & GENMASK(4, 0)) > #define PHY_CONTROL(src) ((src) & GENMASK(15, 0)) > +#define LINK_SPEED(src) (((src) & GENMASK(11, 10)) >> 10) > #define INT_PHY_ADDR 0x1e > #define SGMII_TBI_CONTROL_ADDR 0x44 > #define SGMII_CONTROL_ADDR 0x00 > @@ -34,6 +35,13 @@ > #define LINK_UP BIT(15) > #define MPA_IDLE_WITH_QMI_EMPTY BIT(12) > #define SG_RX_DV_GATE_REG_0_ADDR 0x05fc > +#define SGMII_EN 0x1 > + > +enum xgene_phy_speed { > + PHY_SPEED_10, > + PHY_SPEED_100, > + PHY_SPEED_1000 > +}; > > extern const struct xgene_mac_ops xgene_sgmac_ops; > extern const struct xgene_port_ops xgene_sgport_ops; > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h b/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h > index 0a2dca8..aba4c19 100644 > --- a/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h > +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h > @@ -65,9 +65,13 @@ > #define XG_CFG_LINK_AGGR_RESUME_0_ADDR 0x0214 > #define XG_LINK_STATUS_ADDR 0x0228 > #define XG_TSIF_MSS_REG0_ADDR 0x02a4 > +#define XG_DEBUG_REG_ADDR 0x0400 > #define XG_ENET_SPARE_CFG_REG_ADDR 0x040c > #define XG_ENET_SPARE_CFG_REG_1_ADDR 0x0410 > #define XGENET_RX_DV_GATE_REG_0_ADDR 0x0804 > +#define XG_MCX_ECM_CONFIG0_REG_0_ADDR 0x0070 This define is not used in this patch and as far as I can see not even in this series. You can delete this. > +#define XG_MCX_ICM_CONFIG0_REG_0_ADDR 0x00e0 > +#define XG_MCX_ICM_CONFIG2_REG_0_ADDR 0x00e8 > > extern const struct xgene_mac_ops xgene_xgmac_ops; > extern const struct xgene_port_ops xgene_xgport_ops; > Regards, Matthias