That’s a good overview of the current state.
I can add just a bit of context: the reason the driver is split into 3 parts like this is to later allow other types of cards (SDIO) which will be at the same level as mmc_disk. And on the low-level side, some hardware may allow to send commands using a hardware interface other than the standard SDHCI.
This architecture makes sure we can combine any type of card and any type of controller, with the mmc bus manager in the middle providing the interface between the two.
This can however make the initialization sequence a little more complicated to follow. Ideally, the bus manager would do the early initialization until it assigns an RCA to the card, and then hand it over to a specific driver. But in reality, even before assigning an RCA, there are already several things in the sequence that depends on the card type.
And also we don’t yes use all that flexibility, for now it’s just one driver (mmc_disk) and one bus interface (sdhci). But it seems a good idea to keep this in mind and keep the modular architecture.
1 Like
Thank you, Oh okay I will keep that in mind and start solving the fixes .
Hey everybody,
- Currently I am working on my project proposal ,
- I had also submitted a patch https://review.haiku-os.org/c/haiku/+/10410, solving the TODO of unregistering devices, when the card is removed.
Along with that, I have also added changes allowing multiple devices to be registered, avoiding duplicates,selective unregisteration.
I had also added an enhancement for workerthread flow in the same patch.
Kindly review it and provide feedback so I can work on it further.
1 Like
@PulkoMandy Is there something that can be done with the patch.?
Sorry, I’ve been busy with my paid work and also preparing to move to a new appartment this weekend, so I’m not super available. I will try to have a look today, I hope other developers can help with the reviews as well, even if they don’t know the MMC stack as well as I do.
1 Like
Thank you for the reply. I will focus on the proposal part and try exploring some other issues in the meantime.
I have replied to the change request as well. Sorry again for the delay.
1 Like
Thank you for the review! I will go through the provided feedback and state diagrams to understand them thoroughly. I will drop the custom linked list, and come up with an updated interface that handles card insertion with CMD2/CMD3 and card removal with CMD13 in the next patch.
( A small doubt , should this new interface completely replace the scan semaphore or should be implemented as an additional new enchancement sort.?)
The semaphore alone worked fine as long as there was only one thing to do in the bus_manager thread: scan for new cards. But now there are two different things to do: scan for new cards, and ping existing cards to see if they are alive, depending on which interrupt was triggered.
You could do it with two separate threads and two semaphores, but that approach doesn’t really scale and gets too complicated quite quickly. So let’s keep only one thread.
We need a way to pause that thread until there is something to do. This can be a semaphore or a condition variable (we’re in the kernel and we want to trigger the event from an interrupt handler, so I think these are the only two options). Once the thread wakes up, it has to identifies which interrupt was triggered. Note that both interrupts may have triggered, or an interrupt event may have been triggered multiple times since the thread was last scheduled (maybe because the system was really slow to schedule the thread, or maybe because the hardware is a bit “bouncy” and triggers several insertion/removal events at once when inserting a card).
You could get away with doing this with a semaphore and a few atomic variables, but replacing the semaphore with a condition variable can simplify things a bit. At least that was my experience in other parts of the MMC stack, where semaphores were hard to do right.
1 Like
Thank you, yes handling two events on a single thread would make much more sense, will get on with this!!.
Hey @PulkoMandy , I have been working on my proposal and I have been digging deep into the project. I have doubts regarding the scope of my project.
A. I have covered the initialisation sequence for eMMC devices (which is mandatory)
B. ADMA2 support for devices , which would include checking if the device supports it , branch out from DoIo() to create descriptor tables , handle logic among the states as well as mange their interrupts in HandleInterrupt()(This i believe would consume major chunk of time).
C . I also came across an additonal support for eMMC devices with regard to the block size and geometry which requires a complete different procedure CMD 8 to reading the bit fields EXT_CSD compared to SD cards which require reading the CSD register ( which also requires also support.)
There are also subtasks across mmc_disk.cpp , sdhci.cpp, mmc_bus.cpp which includes interrupt clearly , returning proper error codes , unitialised device cleanup, moving ACMD41 to probing loop, powering off bus when card doesnot meet voltage requirements and more.
With A being mandatory how should I manage B and C to scope my project and are there any important subtasks that I need to follow.
I think we can consider t̂he project succesful if it’s possible to mount an eMMC device and read/write data to its user partition. ADMA2 is not needed for that, but could be part of “stretch goals” to make both SD and eMMC access faster. There are other things to do in that area, such as switching from 4 to 8 bit bus, switching to higher speed clocks if available, etc.
Another possible stretch goal is access to the reserved “boot” partition where some devices store their firmware.
It’s not a problem if the completed work in the end is less than initially planned, as long as there is a justification for it (any unexpected problems will be tracked). Wih hardware drivers development, it’s easy to waste a few days on a problem because the tools to debug and investigate aren’t always available. As long as you can show that you worked full time during the coding period and make reasonable progress, it will be fine. You will not be penalized for not completing all the goals listed in your proposal.
1 Like
Okay , I will push the ADMA2 part as “stretch goal” and focus on eMMC device optimisation for the timeline.
Hey everybody, past couple of days I have been working on my proposal and here is a draft copy of it. Formatting, adding document references and missed grammatical mistakes will be corrected .
Proposal
Looking forward to your suggestions and help in reviewing it.
@PulkoMandy I also updated my patch with the provided suggestion . As, it was a significant change in flow I have kept the patch simple ,not optimised upto haiku standards. I will be correcting them with the provided feeback.
Patch
Sorry for the trouble of creating a new commit for the issue , my local branch was accidently reset and my commit was lost.
Hi Dhrulian, If it is the same change request as your other one then please copy the “Change-ID” from your previous commit into the new one, so it updates the previous review and review comments are not lost.
You can also download your uploaded patches from gerrit if you loose them in your local branch. In the file list at the top on the right is a download button, if you click it you will get severall options to download the commit with git again. The third option “chery-pick” is usually a safe choice, it will download just the commit and apply it to the branch you are working on, without touching anything else.
1 Like
Thank you, I will change the commit id and follow the suggested methods in future.
@PulkoMandy I have updated the permissions to add comments
@PulkoMandy I have made changes to the project proposal.
I had submitted a patch regarding “unregistering of nodes” Can you please review it. Additionally what are the issues or TODOS that I can take up.?
I think rather than specific TODOs, you could start/continue experimenting with eMMC in QEMU and see if you can get the eMMC initialization going a little bit further, properly detecting eMMC vs SD cards and sending the correct initialization commands.
I have also reviewed your patch and made several comments there.
1 Like
Yes, @PulkoMandy I will start experimenting with it.