Re: [PATCH net v3 1/4] net: stmmac: propagate PTP init failures in stmmac_setup_ptp()

[Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>]

[-- Attachment #1: Type: text/plain, Size: 1273 bytes --]

> Hi,
> 
> >>> -static void stmmac_setup_ptp(struct stmmac_priv *priv)
> >>> +static int stmmac_setup_ptp(struct stmmac_priv *priv)
> >>>  {
> >>>  	int ret;
> >>>  
> >>>  	ret = clk_prepare_enable(priv->plat->clk_ptp_ref);
> >>> -	if (ret < 0)
> >>> +	if (ret < 0) {
> >>>  		netdev_warn(priv->dev,
> >>>  			    "failed to enable PTP reference clock: %pe\n",
> >>>  			    ERR_PTR(ret));
> >>> +		return ret;
> >>> +	}
> >>> +
> >>> +	ret = stmmac_init_timestamping(priv);
> >>> +	if (ret) {
> >>> +		clk_disable_unprepare(priv->plat->clk_ptp_ref);
> >>> +		return ret;
> >>> +	}
> >>
> >> This can legitimately fail with -EOPNOTSUPP, we musn't failt the whole open
> >> sequence in this case.
> > 
> > ops, right. I guess we should just skip -EOPNOTSUPP in this case, or do you
> > prefer to always not report the error from stmmac_init_timestamping()?
> 
> The clean solution IMO is to gracefully handle -EOPNOTSUPP, here I think you
> pass it on to the caller exactly like you do (I don't think it makes sense to
> still enable clk_ptp_ref if you don't have timestamping available), and in the
> __stmmac_open() path you don't fail on -EOPNOTSUPP.

ack, I will fix it in v4.

Regards,
Lorenzo

> 
> Thanks :)
> 
> Maxime
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]