Message ID | 20210616091426.13694-2-qiangqing.zhang@nxp.com |
---|---|
State | New |
Headers | show |
Series | [net,1/2] net: fec_ptp: add clock rate zero check | expand |
Hi Joakim, On Wed, Jun 16, 2021 at 11:40:29AM +0000, Joakim Zhang wrote: > Do you mean that print an error message then return directly? It seems better. Nearly - one has to ensure that the cleanup functions don't provoke a crash though. I notice fec_ptp_stop() makes use of fep->time_keep and also fep->ptp_clock. fep->time_keep is initialised after where you need to test for zero cycle_speed, so the initialisation would need moving earlier. I would have thought that ftp->ptp_clock should be NULL, so that's probably okay, but should be checked that this assumption is in fact true. > if (!fep->cycle_speed) { > dev_err(&fep->pdev->dev, "PTP clock rate should not be zero!\n"); I'd still say something like "PTP clock rate should not be zero, disabling PTP" - say what's wrong and what we are doing. Also, please avoid exclaimation marks in error messages. Thanks.
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c index 1753807cbf97..7326a0612823 100644 --- a/drivers/net/ethernet/freescale/fec_ptp.c +++ b/drivers/net/ethernet/freescale/fec_ptp.c @@ -604,6 +604,10 @@ void fec_ptp_init(struct platform_device *pdev, int irq_idx) fep->ptp_caps.enable = fec_ptp_enable; fep->cycle_speed = clk_get_rate(fep->clk_ptp); + if (!fep->cycle_speed) { + fep->cycle_speed = NSEC_PER_SEC; + dev_err(&fep->pdev->dev, "clk_ptp clock rate is zero\n"); + } fep->ptp_inc = NSEC_PER_SEC / fep->cycle_speed; spin_lock_init(&fep->tmreg_lock);