Subject: Re: [PATCH] net: fec: ptp: avoid register access when ipg clock is disabled

From: Richard Cochran <[email-masked]>
Date: Mon, 21 Jul 2014 18:40:04 +0200

On Mon, Jul 21, 2014 at 10:46:31AM +0000, [email-masked] wrote:

> >> +	if (fep->bufdesc_ex && (fep->hwts_tx_en || fep->hwts_rx_en))
> >> +		fec_ptp_stop(ndev);
> >
> >So if user space turns time stamping off, then you keep running the timer.
> >That is wrong, isn't it?
> >
> If user close the ethx interface, or do suspend operation, fec driver will
> Close all clocks to save power, we also need to stop ptp, otherwise ptp timer
> Keep access register that cause system hang.
> 
> I don't think this is a good method, maybe I need some time to struct better solution.

What?

Look at the code that you wrote. You have:

	IF   x AND (fep->hwts_tx_en || fep->hwts_rx_en)
	THEN fec_ptp_stop;

So if the user clears 'hwts_tx_en' and 'hwts_rx_en' using the ioctl,
you don't stop the timer. But in order to fix the bug, you should stop
the timer in any case.

> >Why not use a work queue for this?
>
> There need one period timer to sync hw timer to avoid 32 bit hw counter overflow.

So why not use schedule_delayed_work?

Thanks,
Richard