Re: [PATCH net-next v2 05/21] motorcomm:yt6801: Implement the fxgmac_start function

[Andrew Lunn <andrew@lunn.ch>]

> +static  void fxgmac_phylink_handler(struct net_device *ndev)
> +{
> +	struct fxgmac_pdata *pdata = netdev_priv(ndev);
> +	struct fxgmac_hw_ops *hw_ops = &pdata->hw_ops;
> +
> +	pdata->phy_link = pdata->phydev->link;
> +	pdata->phy_speed = pdata->phydev->speed;
> +	pdata->phy_duplex = pdata->phydev->duplex;
> +
> +	yt_dbg(pdata, "EPHY_CTRL:%x, link:%d, speed:%d,  duplex:%x.\n",
> +	       rd32_mem(pdata, EPHY_CTRL), pdata->phy_link, pdata->phy_speed,
> +	       pdata->phy_duplex);
> +
> +	if (pdata->phy_link) {
> +		hw_ops->config_mac_speed(pdata);
> +		hw_ops->enable_rx(pdata);
> +		hw_ops->enable_tx(pdata);
> +		netif_carrier_on(pdata->netdev);

phylib controls the carrier, not the MAC driver.

> +static int fxgmac_phy_connect(struct fxgmac_pdata *pdata)
> +{
> +	struct phy_device *phydev = pdata->phydev;
> +	int ret;
> +
> +	ret = phy_connect_direct(pdata->netdev, phydev, fxgmac_phylink_handler,
> +				 PHY_INTERFACE_MODE_RGMII);

RGMII is unusual, you normally want RGMII_ID. Where are the 2ns delays
added?

> +int fxgmac_phy_irq_enable(struct fxgmac_pdata *pdata, bool clear_phy_interrupt)
> +{
> +	struct phy_device *phydev = pdata->phydev;
> +
> +	if (clear_phy_interrupt &&
> +	    phy_read(phydev, PHY_INT_STATUS) < 0)
> +		return -ETIMEDOUT;
> +
> +	return phy_modify(phydev, PHY_INT_MASK,
> +				     PHY_INT_MASK_LINK_UP |
> +					     PHY_INT_MASK_LINK_DOWN,
> +				     PHY_INT_MASK_LINK_UP |
> +					     PHY_INT_MASK_LINK_DOWN);

The PHY driver is in charge of PHY interrupts.

> +int fxgmac_start(struct fxgmac_pdata *pdata)
> +{
> +	struct fxgmac_hw_ops *hw_ops = &pdata->hw_ops;
> +	u32 val;
> +	int ret;
> +
> +	if (pdata->dev_state != FXGMAC_DEV_OPEN &&
> +	    pdata->dev_state != FXGMAC_DEV_STOP &&
> +	    pdata->dev_state != FXGMAC_DEV_RESUME) {
> +		yt_dbg(pdata, " dev_state err:%x\n", pdata->dev_state);
> +		return 0;
> +	}
> +
> +	if (pdata->dev_state != FXGMAC_DEV_STOP) {
> +		hw_ops->reset_phy(pdata);
> +		hw_ops->release_phy(pdata);
> +		yt_dbg(pdata, "reset phy.\n");
> +	}
> +
> +	if (pdata->dev_state == FXGMAC_DEV_OPEN) {
> +		ret = fxgmac_phy_connect(pdata);
> +		if (ret < 0)
> +			return ret;
> +
> +		yt_dbg(pdata, "fxgmac_phy_connect.\n");
> +	}
> +
> +	phy_init_hw(pdata->phydev);
> +	phy_resume(pdata->phydev);

The MAC should not be doing this.

> +
> +	hw_ops->pcie_init(pdata);
> +	if (test_bit(FXGMAC_POWER_STATE_DOWN, &pdata->powerstate)) {
> +		yt_err(pdata,
> +		       "fxgmac powerstate is %lu when config power up.\n",
> +		       pdata->powerstate);
> +	}
> +
> +	hw_ops->config_power_up(pdata);
> +	hw_ops->dismiss_all_int(pdata);
> +	ret = hw_ops->init(pdata);
> +	if (ret < 0) {
> +		yt_err(pdata, "fxgmac hw init error.\n");
> +		return ret;
> +	}
> +
> +	fxgmac_napi_enable(pdata);
> +	ret = fxgmac_request_irqs(pdata);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Config interrupt to level signal */
> +	val = rd32_mac(pdata, DMA_MR);
> +	fxgmac_set_bits(&val, DMA_MR_INTM_POS, DMA_MR_INTM_LEN, 2);
> +	fxgmac_set_bits(&val, DMA_MR_QUREAD_POS, DMA_MR_QUREAD_LEN, 1);
> +	wr32_mac(pdata, val, DMA_MR);
> +
> +	hw_ops->enable_mgm_irq(pdata);
> +	hw_ops->set_interrupt_moderation(pdata);
> +
> +	if (pdata->per_channel_irq) {
> +		fxgmac_enable_msix_irqs(pdata);
> +		ret = fxgmac_phy_irq_enable(pdata, true);
> +		if (ret < 0)
> +			goto dis_napi;
> +	}
> +
> +	fxgmac_enable_rx_tx_ints(pdata);
> +	phy_speed_up(pdata->phydev);
> +	genphy_soft_reset(pdata->phydev);

More things the MAC driver should not be doing.

	Andrew