On Tue, 4 Apr 2023 at 10:23, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote: > You can mostly forget about CONFIG_PREEMPT=n (or more specifically > CONFIG_PREMPT_COUNT=n) things that work for PREEMPT typically also work > for !PREEMPT. Thanks for the clarification! > The question here seems to be if i2c_in_atomic_xfer_mode() should have > an in_atomic() / !preemptible() check, right? IIUC Wolfram doesn't like > it being used outside of extra special cicumstances? Sorry for expressing things more complicated than needed. Yes, exactly. Wolfram expressed considerations of adding preemptible() as check, as the now existing irq_disabled() check is lost with !PREEMPT. I tried to clarify that the check seems to be there for "performance reasons" and that the check essentially was !preemptible() before v5.2, so nothing should break. However, what came to my mind during my "research", is that it might make sense to handle all i2c transfers atomically when in a post RUNNING state (namely HALT, POWER_OFF, RESTART, SUSPEND). The outcome would basically be the same as with this patch series applied, but I guess the reasoning behind it would be simplified: If in a restart handler, the i2c transfer is atomic, independent of current IRQ or preemption state. Currently, the state depends on from where the handler is triggered. As you have stated, most of the i2c transfers in the mentioned states will just update single bits for power-off/sleep/suspend, so IMHO nothing where not using a DMA would matter from a performance point of view. But there might be other reasons for not doing so - I guess in this case the provided patch is fine (at least from my side).
