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

[Minda Chen]

> 
> > +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.
> 
Okay. I think return -EINVAL is correct.

> > +	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

No. do NOT support 1000BaseX in jhb100 soc. I think 1000BaseX need new serdes PHY
be Intergrated it and maybe new setting.