Page MenuHomeFreeBSD

MMC stack on top of CAM framework
AbandonedPublic

Authored by kibab on Jan 3 2016, 9:39 AM.
Tags
Referenced Files
Unknown Object (File)
Tue, Dec 17, 8:42 PM
Unknown Object (File)
Thu, Dec 12, 3:25 AM
Unknown Object (File)
Thu, Nov 28, 3:24 AM
Unknown Object (File)
Wed, Nov 27, 1:33 AM
Unknown Object (File)
Wed, Nov 27, 1:33 AM
Unknown Object (File)
Nov 25 2024, 4:05 AM
Unknown Object (File)
Nov 25 2024, 4:05 AM
Unknown Object (File)
Nov 25 2024, 4:05 AM

Details

Summary

An attempt to create a new version of MMC stack that uses a CAM framework to route the requests and replies.
This will allow us to handle SDIO cards that generate interrupts when the stack doesn't expect it, and further unify FreeBSD device model.

Runs on the BeagleBone Black and uses a modified version of ti_sdhci driver to communicate with the card. Changes to the driver are minimal and can be ported to another SDHCI implementation easily.

  • The card is initialized only if it is inserted in the MMC slot on boot.
  • SD[HC] and MMC cards are supported in read-write mode.
  • There are some sleeps in the initialization path that should better be moved elsewhere to avoid blocking main CAM thread.
  • SDIO cards are detected, minimal initialization is done, at least N devices are created where N is the number of functions on the card.
Test Plan

Build armv6 kernel , kernel config file is BEAGLEBONE-CAM
Boot the BBB with the SDHC card inserted

You should see FREAKING LOTS OF DEBUG OUTPUT -- sorry for that :-) After boot you should be able to access two MMC/SD cards on the system, one is built-in eMMC and another one is inserted SD.

Try playing around: diskinfo -v sdda{0,1}, geom disk list, camcontrol devlist.

Unfortunately camcontrol devlist doesn't display useful data about the cards right now, this requires modifying camcontrol userspace utility, this is WIP.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

kibab retitled this revision from to MMC stack on top of CAM framework.
kibab updated this object.
kibab edited the test plan for this revision. (Show Details)
kibab added reviewers: imp, ian, adrian, mav.
kibab set the repository for this revision to rS FreeBSD src repository - subversion.
kibab added a project: ARM.

here's a few trivial comments to get my feet wet.

etc/mtree/BSD.include.dist
228–230

This change is unrelated and should be committed separately.

include/Makefile
43

this should be in alphabetical order.

lib/libcam/Makefile
38–39

This should be in alphabetical order, even though the old version isn't.

sys/Makefile
59

this is unrelated.

kibab edited edge metadata.
kibab removed rS FreeBSD src repository - subversion as the repository for this revision.

I've fixed the scanning and attaching of new devices. Additionally pass(4) attaches to all found devices now.

for the next upload can you include context please -- e.g.

git diff -U999999 other-branch
svn diff --diff-cmd=diff -x -U999999

(or less than full context if you want, e.g. -U1000)

etc/mtree/BSD.include.dist
95–96

keep sorted

sys/arm/conf/BEAGLEBONE-CAM
2

probably don't want to keep this copy of the header comment -- but perhaps the intent is to drop this later?

sys/cam/mmc/mmc.h
3–4

This comment ought to go after the copyright and license block, I think.

First off, this is looking quite good. I had a number of nit-picky and some
substantive changes (the sleeps gotta go). Some code organization
stuff too. I like it. It also might be easier to review in smaller chunks
too, which would make for better commits, but we can sort that out
iteratively. It's nice to have, but isn't needed for progress.

sys/arm/conf/BEAGLEBONE-CAM
2

Not sure this file is useful to commit.

sys/cam/mmc/mmc_xpt.c
583–584

This isn't allowed in the start routine. You'll need more states to manage this transition.

589

Though no sleeps here, chances are you need a new state for this and clock through it in the done routine.

601–602

ditto

609–610

Bad sleep, see above :)

617–618

Bad sleep, see above :)

743–744

I'm not sure why you freeze the queue here. That doesn't make sense to me.

1075

I'd add a comment that the queue was frozen in the probe start routine. Tracking this down was a pita in other drivers / xpts

1101

This is a very odd style.

Also, this function belongs elsewhere, like cam_xpt.c

sys/cam/mmc/mmcreg.h
57

any reason not to use the one in dev? why move it here?

sys/conf/files
1560–1563

This looks like format tweaks. Should be done in another commit.

1966

This, and all the related files, look like a good candidate for a separate commit

sys/dev/sdhci/sdhci.c
63

This shouldn't be there...

526

This is gratuitous.

741–744

likely should be inside debug

793

stylistically, this should be at the top of this function, or at thetop of this block, not right here.

987–991

debug?

1555

I'd put this in a separate file.

Thanks for the review. I will fix all your comments over the next few days and upload a new version.

sys/cam/mmc/mmc_xpt.c
1101

:-) Yes, it was auto-generated and there was an extra semicolon somewhere.

sys/cam/mmc/mmcreg.h
57

Yes: the one if dev/ depends on "mmc", and can be excluded from the build, whereas MMC CAM stuff cannot be excluded because it's CAM and it's not modular. Additionally, in the future it makes more sense to keep all related headers in sys/cam/mmc.

sys/dev/sdhci/sdhci.c
63

Agreed -- this is there because I didn't use ubdr to boot the kernel on BBB, so I couldn't really tune this. Now my main dev platform is Wandboard and it should be possible to set sdhci_debug via loader hints.

kibab set the repository for this revision to rS FreeBSD src repository - subversion.
kibab marked 4 inline comments as done.

I have started to cleanup my diff. This upload removes excessive kernel config files and fixes issues with alphabetical ordering in several places. So, no functional changes at all.

This was merged with some modifications.