ROOL (4b14cbfe) at 22 Feb 09:09
Add NVMeFS SWI numbers to swis.h
!NoChange
ROOL (ce95fa0c) at 22 Feb 08:54
Add NVMeFS SWI numbers to swis.h
!NoChange
Detail: Use the same const-ness as ANSI specify for vprintf in our prototype. Correct a surprising number of out of step comment heading blocks.
ROOL (e71fc953) at 17 Feb 13:08
Match up vprintf and dvprintf argument qualifiers
Detail: Use the same const-ness as ANSI specify for vprintf in our prototype. Correct a surprising number of out of step comment heading blocks.
OK. I've ensured the pollword is in the rma block. No change to aborting behaviour. At present it really seems that it is unsafe to do an upcall 6 in a code sequence that is launched through the IRQ. Same code is fine when launched in, e.g. the module init callback, or subsequent task.
FWIW this is corroborated I guess: I use DADEBUG output for my debugging. Code runs fine in normal context. Same code if run from the irq launch seems to get an invalid output routine pointer out of static storage in the debug output routines. This would have been initialised in the main context. It isnt synchronised to the rtsupport launched context.
Ben,I accept whatyou say, BUT, I did have to make mods in smhclib, and one of those mods is to clear the card int when it is read. Without doing that further ints, that would occur as a result of the complete command sequences actually run as a result of the card int would get lost, which I have frequently observed. Does the card int behave as you expect when going via your sdiodriver as on pi4b? If I get time I'll check, and now we are where we are It'll be worth me double checking if the 3399 behaviour I currently have is still needed. FWIW throughput in the pi4b appears to take about twice as long...
yes... BUT... it is needed in c.sdio_io and nowhere in the actual obsd code have I come across anything that turns on the global bit. From that I have to assume it is set on as a part of the low level chip init.. the functionality you perform in sdiodriver. I suggest this mod provides a practical compromise, as the global bit is then NOT turned on unless you actually need it. Safer?
I'm happy without the atomic thing. Thought I'd left it out. The real mod is clearing the request bit on entering the thread. This is because there is lots of code run in obsd within this thread that is dependant on this card interrupt to signal completion. If the only thing that the irq to obsd did was to set a semaphore then clearing at the end of the routine would be ok. BUT the code is more demanding. Clearing the semaphore once you have started to act on it has proved the practical answer. Agreed this may, on the rk3399, be a consequence of my changes in smhclib, BUT, on the Pi4b I found the same need, and that is purely your sdiodriver module.
Oops.. Looks like I'm mudding the allways return state.. so a pointer to a null word obliges return? OK. will resolve.
forget review request.. finger trouble
Can't we just invent a new function to do this (manipulate the global interrupt bit, I mean)? I'm still not happy with having functions called sdmmc_intr_enable()
and sdmmc_intr_disable()
do something different from the one of the same name in OpenBSD.
I really think you should revert this one. I realised that SyncLib needs to look up a function pointer in a static variable to choose how to implement atomic_update()
unless it knows at compile time exactly which architecture is in use, and the veneer around this function was only expecting it to consist of a STR instruction so didn't bother with setting up static relocations. This would be a functional difference between a Pinebook ROM (known architecture) vs Pi ROM (multi-architecture). As previously discussed, atomic_update()
doesn't gain you anything over a simple assignment operation in this case.
The only reason why this function is implemented in C is to avoid having a separate assembly definition of the int_function_t
struct, which would have the potential to get out of step with the C definition.
Is this left over from a test version? We shouldn't be sleeping on a pollword that is always nonzero here, or we'll never exit the kernel's main interrupt code for as long as an operation initiated from a RTSupport thread lasts. Same goes for similar changes in other functions.
Clear interrupt semaphore on entering irq task, rather than on exit. (Client task can take a while to complete and can cause further legitimate card interrupts. This ensures none are missed.)
This still doesn't make sense. SDIO spec, section 6.3, says
once the function has signaled an interrupt, it shall not release (stop signaling) the interrupt until the cause of the interrupt is removed or commanded to do so by the host
It therefore should be impossible to miss any interrupts - if they are masked in the SD controller, they would immediately trigger as soon as they were unmasked.
In OpenBSD (sys/dev/sdmmc/sdmmc_io.c
): function sdmmc_intr_task()
dispatches all the callbacks that were earlier registered using sdmmc_intr_establish()
before it calls through the function pointer sc->sct->intr_ack
. The only function I can ever see being assigned to this pointer is sdhc_card_intr_ack()
, which is for SDHCI controllers such as that used in the Pi, and it sets the card interrupt bit in the interrupt enable register.
In Linux (driver/mmc/core/sdio_irq.c
): function sdio_irq_thread()
calls process_sdio_pending_irqs()
(which in turn dispatches all the callbacks that were earlier registered using sdio_claim_irq()
) before it calls through the function pointer host->ops->enable_sdio_irq
. For an SDHCI controller such as that used in the Pi, this function pointer is sdhci_enable_sdio_irq()
which sets the card interrupt bit in the interrupt enable and interrupt signal enable registers. For the DesignWare SD controller featured on RK3399, it's dw_mci_enable_sdio_irq()
which calls __dw_mci_enable_sdio_irq()
to unmask the card interrupt bit in an exactly equivalent way.
In RISC OS, SDIO_Control_AckSDIOCardInt is analogous to these function pointers - it unmasks the card interrupt in the SD host controller.
If you're seeing the timing of when the sempahore being cleared making any difference - at least amongst times before SDIO_Control_AckSDIOCardInt is called - then either the card interrupt is being dispatched whilst it is supposed to be masked (if so, we need to work out why - that should be impossible) or something else is scribbling over that memory (again, we need to work out why).
I find onthe Pi4b the only reliable solution ATM is to remove all 5 upcall6 calls. We need a solution, and it isnt my code. Specifically it is openbsd code interfacing just as expected to the SDIODriver library.
I presume then that one should be able to modify the sdiodriver interrupt task to simply add a callback to run the function supplied to the intr_establish call such that it then runs in conventional callback time
Unfortunately, that's a non-starter, unless you want to throw away the OpenBSD driver code and start again. When you're writing stuff that runs in transient callbacks, you have to write it to be event-driven - you can't call a function that says "get on with stuff and return when something interesting has happened" in the manner that OpenBSD and Linux driver code expects. You might think that you could copy away the SVC stack contents and restore them within the next transient callback, a bit like TaskWindow does for applications, but that's unlikely to work because callbacks aren't always entered with the same amount of data on the SVC stack, so any pointers to local variables (including things like stack frames) wouldn't remain valid across the call. That's one of the reasons RTSupport had to be developed in the first place...
I did try using RMA for the pollword structure. This would work fine for the 3 instances in c.common, using the R5 command, BUT did not help in the 1 instance in c.sdmmc_io which uses the R1 command. As far as I can tell that is liable to be called back in an unhelpful mode (I assumed user).
The only R1 command used in SDIOLib is CMD7_SELECT_DESELECT_CARD
issued from ensure_selected()
in c.common - is that what you mean?
All the commands in c.common register the same callback routine with SDIODriver - sdiolib_op_callback
- but use different handles in the r5 register (those handles are the pointers that we need to ensure are not allocated from the SVC stack).
I sense some confusion about the processor mode in which this callback is entered. Perhaps this stems from the wording here "The call is made in privileged mode". I think the reason it was worded this way was that the callback API is modelled on the equivalent one in SCSIDriver which says "The call is made in IRQ mode", coupled with the fact that since the HAL was introduced, entry points across RISC OS that used to be documented as IRQ mode are, at least for some platforms, called in SVC mode.
In the specific case of SDIODriver, the only mode change instructions are in its CMHG veneers, so in practice it only uses SVC mode within the body of its code. If you're genuinely seeing sdiolib_op_callback
called in any other mode, then my best guess would be that stack contents are getting corrupted somewhere in an interrupt routine or RTSupport thread, because SDIODriver isn't changing mode itself.
I call a module from the command line to load it (e.g. $.WLanB) within a task window. Your comments rather suggest it is liable to be invoked in possibly even user mode
Sorry, talk of USR mode is probably a distraction for the case of a WiFi driver. I would expect in that case for functions like sdmmc_io_read_1()
to either be called in SVC mode (from module init, a SWI handler, transient callback handler etc) or SYS mode (from a routine which was registered using sdmmc_intr_establish()
and which is actually executing in RTSupport thread context). All I meant is that there's nothing which inherently requires a privileged mode in sdmmc_io_read_1()
- it doesn't directly poke memory-mapped peripherals, manipulate the CPSR, use CP15 operations etc - all such things are delegated to SDIODriver's SWIs, and you can call SWIs from USR mode. So it's conceivable that something might be written one day that could usefully use the library in USR mode.
I run a command in the task window, e.g. ifconfig wb0 nwid rosery, and I get a similar failure in the R1 command when it is run
I don't think you've actually said so far how the failure actually manifests itself - just "fails miserably". Can you be more specific? It might help to identify what's going wrong.
The only thought I've had so far is that perhaps you haven't called synclib_init()
at any point in your module, because sdiolib_op_callback
relies on it unless you've built it for an ARMv8-only ROM. I'm not sure how that would cause a different types of failure under TaskWindow though.
In !3[...]
Probably best not to conflate the two issues. All the places that call OS_UpCall 6 are waiting for a pollword set by a callback registered with SDIO_Op which occurs when a background SD command has completed. The semaphore in !3 is set by a callback registered with SDIO_ClaimDeviceVector which occurs when a card interrupt has been detected (and we sleep on it with RT_Yield rather than OS_UpCall 6 because we know in that case that we're in RTSupport thread context).
We may not be out of the woods yet I fear. This solution is happy on the RK3399, but on the Pi4b it still misbrhaves causing task window crash if, typically, one does an ifconfig command that issues ioctl calls to the module that uses sdiodriver.
Setting to draft again for now, awaiting feedback please.