Hello,

On 17.12.20 02:13, Jakub Kicinski wrote:
>> +                    netdev_warn(priv->dev, "HW Timestamping init failed: 
>> %pe\n",
>> +                                    ERR_PTR(ret));
> 
> why convert to ERR_PTR and use %pe and not just %d?

To get a symbolic error name if support is compiled in (note the `e' after %p).

> 
> also continuation misaligned
> 
>> +            } else {
>> +                    ret = stmmac_init_ptp(priv);
>> +                    if (ret == -EOPNOTSUPP)
>> +                            netdev_warn(priv->dev, "PTP not supported by 
>> HW\n");
>> +                    else if (ret)
>> +                            netdev_warn(priv->dev, "PTP init failed\n");
>> +            }
>>      }
>>  
>>      priv->eee_tw_timer = STMMAC_DEFAULT_TWT_LS;
>> @@ -5290,8 +5330,7 @@ int stmmac_resume(struct device *dev)
>>              /* enable the clk previously disabled */
>>              clk_prepare_enable(priv->plat->stmmac_clk);
>>              clk_prepare_enable(priv->plat->pclk);
>> -            if (priv->plat->clk_ptp_ref)
>> -                    clk_prepare_enable(priv->plat->clk_ptp_ref);
>> +            stmmac_init_hwtstamp(priv);
> 
> This was optional, now you always init?

Indeed, omitting the if condition here will lead to a needless warning on every 
reset.

Cheers,
Ahmad

> 
>>              /* reset the phy so that it's ready */
>>              if (priv->mii)
>>                      stmmac_mdio_reset(priv->mii);
>>
>> base-commit: 3db1a3fa98808aa90f95ec3e0fa2fc7abf28f5c9
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Reply via email to