On 2022. 08. 30. 14:29, Andrew Lunn wrote:

quoted

-	fec_ptp_gettime(&fep->ptp_caps, &fep->ptp_saved_state.ts_phc);
+	if (preempt_count_equals(0)) {

~/linux/drivers$ grep -r preempt_count_equals *
~/linux/drivers$

No other driver plays games like this.

Why not unconditionally take the lock?

Because then we would be back at the original problem (see Marc's message):

| [   14.001542] BUG: sleeping function called from invalid context at 
kernel/locking/mutex.c:283 

| [   14.010604] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 
13, name: kworker/0:1 

| [   14.018737] preempt_count: 201, expected: 0

We cannot take a mutex in atomic context. However, we also don't *need 
to* take a mutex in atomic context.

     Andrew

If someone has a better solution, I'm open to suggestions. But to me, it 
seems that there are only 3 options:

1. Unconditionally taking the mutex was what I originally did, but that 
caused issues in Marc's setup.
2. Not taking the mutex at all is what I proposed in v1 of this patch. 
But as Richard pointed out, `timecounter_read()` actually does a 
Read-Modify-Write on the `FEC_ATIME_CTRL` register, that *could* get 
interrupted if not guarded by the mutex (or atomic context).
3. The final option, check if we are in an atomic or otherwise 
non-interruptible context, and if not, take a mutex. Otherwise, proceed 
normally. Which is this version of the patch.

Bence