Re: [net-next v4 4/5] net: stmmac: starfive: Add jhb100 SGMII interface

[Andrew Lunn]

> +static int stmmac_starfive_sgmii_set_clk_rate(void *bsp_priv, struct clk *clk_tx_i,
> +					      phy_interface_t __maybe_unused interface,
> +					      int speed)
> +{
> +	struct starfive_dwmac *dwmac = bsp_priv;
> +	long rate = rgmii_clock(speed);
> +	int ret;
> +
> +	/* MAC clock rate the same as RGMII */
> +	if (rate < 0)
> +		return 0;

You probably should return the error code, because something has gone
wrong, you have been asked to do a rate you don't support.

> +	ret = clk_set_rate(clk_tx_i, rate);
> +	if (ret)
> +		return ret;
> +
> +	return clk_set_rate(dwmac->sgmii_rx, rate);
> +}
> +
>  static int starfive_dwmac_probe(struct platform_device *pdev)
>  {
>  	struct plat_stmmacenet_data *plat_dat;
> @@ -102,23 +122,33 @@ static int starfive_dwmac_probe(struct platform_device *pdev)
>  		return dev_err_probe(&pdev->dev, PTR_ERR(clk_gtx),
>  				     "error getting gtx clock\n");
>  
> -	/* Generally, the rgmii_tx clock is provided by the internal clock,
> -	 * which needs to match the corresponding clock frequency according
> -	 * to different speeds. If the rgmii_tx clock is provided by the
> -	 * external rgmii_rxin, there is no need to configure the clock
> -	 * internally, because rgmii_rxin will be adaptively adjusted.
> -	 */
> -	if (!device_property_read_bool(&pdev->dev, "starfive,tx-use-rgmii-clk"))
> -		plat_dat->set_clk_tx_rate = stmmac_set_clk_tx_rate;
> -
>  	dwmac->dev = &pdev->dev;
> -	plat_dat->flags |= STMMAC_FLAG_EN_TX_LPI_CLK_PHY_CAP;
>  	plat_dat->bsp_priv = dwmac;
> -	plat_dat->dma_cfg->dche = true;
> +	if (plat_dat->phy_interface == PHY_INTERFACE_MODE_SGMII) {

Does the PCS support 1000BaseX? It is not needed now, but it is
something to keep in mind, try to avoid making to code too SGMII
specific when it might need to be more generic to support 1000BaseX as
well.

	Andrew