Messages in this thread | | | Date | Wed, 16 Jun 2021 14:28:59 +0100 | From | "Russell King (Oracle)" <> | Subject | Re: [PATCH net 1/2] net: fec_ptp: add clock rate zero check |
| |
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.
-- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
| |