Page MenuHomeFreeBSD

ppbus: Set the lock for pps interface, update to latest api
ClosedPublic

Authored by imp on Sep 1 2021, 4:32 AM.

Details

Summary

Since we take a lock when we enter the ioctl, we need to set driver_mtx
in the pps structure so it can be dropped while sleeping during a call
to timepps_fetch() with a non-zero timeout (PPS_CANWAIT feature).

Sponsored by: Netflix

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

imp requested review of this revision.Sep 1 2021, 4:32 AM
imp created this revision.
ian added inline comments.
sys/dev/ppbus/pps.c
148

The code here also needs to indicate it's aware of the new pps abi that allows sharing the driver's mutex by calling pps_init_abi() instead of pps_init(). The code inside the loop below also needs to look like this.

	sc->pps[0].ppscap = PPS_CAPTUREASSERT | PPS_ECHOASSERT;
	sc->sc_pps.driver_abi = PPS_ABI_VERSION;
	sc->sc_pps.driver_mtx = ppb_get_lock(ppbus);
	pps_init_abi(&sc->pps[0]);
sys/dev/ppbus/pps.c
148

Thanks Ian! I missed that detail last night. I'll respin.

imp retitled this revision from ppbus: Set the lock for pps interface to ppbus: Set the lock for pps interface, update to latest api.Sep 1 2021, 5:44 PM
imp edited the summary of this revision. (Show Details)

Update with Ian's patch that Criag tested.

sys/dev/ppbus/pps.c
148

Done and updated.

others have suggested diffs to me by enclosing them in triple backticks which doesn't steal the review.

imp marked an inline comment as done.Sep 1 2021, 5:46 PM

I would be a bit more wordy in the commit, something like;

Since we take a lock when we enter the ioctl, we need to set driver_mtx
in the pps structure so it can be dropped while sleeping during a call
to timepps_fetch() with a non-zero timeout (PPS_CANWAIT feature).

This revision is now accepted and ready to land.Sep 1 2021, 5:56 PM
In D31763#716893, @ian wrote:

I would be a bit more wordy in the commit, something like...

I like it.

This revision now requires review to proceed.Sep 1 2021, 6:27 PM
This revision is now accepted and ready to land.Sep 1 2021, 6:54 PM