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

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

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.

Thanks :)

Maxime