Re: [PATCH linux dev-5.3 5/6] net: ftgmac100: Ungate RCLK for RMII on AST2600

[Andrew Jeffery]

On Mon, 23 Sep 2019, at 15:42, Cédric Le Goater wrote:
> On 22/09/2019 14:36, Andrew Jeffery wrote:
> > The 50MHz RMII RCLK has to be enabled before the interface will function.
> > 
> > Signed-off-by: Andrew Jeffery <andrew at aj.id.au>
> > ---
> >  drivers/net/ethernet/faraday/ftgmac100.c | 43 ++++++++++++++++++++----
> >  1 file changed, 36 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> > index 9b7af94a40bb..21a58aad1a19 100644
> > --- a/drivers/net/ethernet/faraday/ftgmac100.c
> > +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> > @@ -90,6 +90,9 @@ struct ftgmac100 {
> >  	struct mii_bus *mii_bus;
> >  	struct clk *clk;
> >  
> > +	/* 2600 RMII clock gate */
> > +	struct clk *rclk;
> > +
> >  	/* Link management */
> >  	int cur_speed;
> >  	int cur_duplex;
> > @@ -1718,11 +1721,27 @@ static void ftgmac100_ncsi_handler(struct ncsi_dev *nd)
> >  		   nd->link_up ? "up" : "down");
> >  }
> >  
> > -static void ftgmac100_setup_clk(struct ftgmac100 *priv)
> > +static int ftgmac100_setup_clk(struct ftgmac100 *priv)
> >  {
> > -	priv->clk = devm_clk_get(priv->dev, NULL);
> > -	if (IS_ERR(priv->clk))
> > -		return;
> > +	struct clk *clk;
> > +	bool is_ast2600;
> > +
> > +	is_ast2600 = of_device_is_compatible(priv->dev->of_node,
> > +					     "aspeed,ast2600-mac");
> 
> why is ast2600 particular ? 

It isn't, as Joel pointed out. We need this for at least the 2500 as well, but I didn't think to check as u-boot has been leaving the clock ungated.

> 
> > +	clk = devm_clk_get(priv->dev, NULL /* MACCLK */);
> > +	if (IS_ERR(clk))
> > +		return PTR_ERR(clk);
> > +	priv->clk = clk;
> > +
> > +	clk = devm_clk_get_optional(priv->dev, "RCLK");
> > +	if (!clk && is_ast2600 && priv->use_ncsi) {
> > +		dev_err(priv->dev, "Cannot ungate RCLK");
> > +		return -EINVAL;
> > +	}
> > +
> > +	priv->rclk = clk;
> > +	clk_prepare_enable(priv->rclk);
> >  
> >  	clk_prepare_enable(priv->clk);
> >  
> > @@ -1732,6 +1751,8 @@ static void ftgmac100_setup_clk(struct ftgmac100 *priv)
> >  	 */
> >  	clk_set_rate(priv->clk, priv->use_ncsi ? FTGMAC_25MHZ :
> >  			FTGMAC_100MHZ);
> > +
> > +	return 0;
> >  }
> >  
> >  static int ftgmac100_probe(struct platform_device *pdev)
> > @@ -1853,8 +1874,11 @@ static int ftgmac100_probe(struct platform_device *pdev)
> >  			goto err_setup_mdio;
> >  	}
> >  
> > -	if (priv->is_aspeed)
> > -		ftgmac100_setup_clk(priv);
> > +	if (priv->is_aspeed) {
> > +		err = ftgmac100_setup_clk(priv);
> > +		if (err)
> > +			goto err_ncsi_dev;
> > +	}
> >  
> >  	/* Default ring sizes */
> >  	priv->rx_q_entries = priv->new_rx_q_entries = DEF_RX_QUEUE_ENTRIES;
> > @@ -1886,8 +1910,11 @@ static int ftgmac100_probe(struct platform_device *pdev)
> >  
> >  	return 0;
> >  
> > -err_ncsi_dev:
> >  err_register_netdev:
> > +	if (priv->rclk)
> > +		clk_disable_unprepare(priv->rclk);
> > +	clk_disable_unprepare(priv->clk);
> > +err_ncsi_dev:
> >  	ftgmac100_destroy_mdio(netdev);
> >  err_setup_mdio:
> >  	iounmap(priv->base);
> > @@ -1909,6 +1936,8 @@ static int ftgmac100_remove(struct platform_device *pdev)
> >  
> >  	unregister_netdev(netdev);
> >  
> > +	if (priv->rclk)
> > +		clk_disable_unprepare(priv->rclk);
> >  	clk_disable_unprepare(priv->clk);
> >  
> >  	/* There's a small chance the reset task will have been re-queued,
> > 
> 
>