Page MenuHomeFreeBSD

Add rtsx driver for Realtek SD card readers.

Authored by on Sep 15 2020, 10:09 AM.



The rtsx driver provides support for PCI Realtek SD card reader. Driver
attaches mmc bus on card insertion and detaches it on card removing.
The driver has been tested with RTS5209, RTS5227, RTS5229, RTS522A,
RTS525A and RTL8411B. It should also work with RTS5249, RTL8402 and

Test Plan
  • insert a card
  • prepare a random file of 8GB for a card >=8GB:
dd if=/dev/random of=/tmp/random0 bs=32768 count=262144
  • test the I/O using the ping-pong buffer (cmd buffer):
dd if=/tmp/random0 of=/dev/mmcsd0 bs=512
dd if=/dev/mmcsd0 of=/tmp/random1 bs=512
md5 /tmp/random0 /tmp/random1
  • test the I/O using the data buffer:
dd if=/tmp/random0 of=/dev/mmcsd0 bs=32768
dd if=/dev/mmcsd0 of=/tmp/random2 bs=8192
md5 /tmp/random0 /tmp/random2
  • create a zpool:
gpart create -s GPT /dev/mmcsd0
gpart add -t freebsd-zfs /dev/mmcsd0
zpool create testpool /dev/mmcsd0p1
rsync -av /usr/src/ /testpool
zpool scrub testpool
zpool status testpool
zpool export testpool
  • remove the card

with options MMCCAM replace /dev/mmcsd by /dev/sdda.

PR204521 contains bulk of exchanges for completion of the code.

Diff Detail

rS FreeBSD src repository - subversion
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline created this revision. edited the test plan for this revision. (Show Details) added inline comments.
111 ↗(On Diff #77043)

This really really should be a tunable/sysctl, not a compile time option. Recompiling the kernel just because you have a specific laptop model should not be required.

Update manual to add tunable for inversion. added inline comments.
111 ↗(On Diff #77043)

I complete the manual with the implemented tunable.

I think you are missing some files in this review that changed, probably got lost when updating the new manpage changes.

imp requested changes to this revision.Sep 21 2020, 7:24 PM

This review is missing a lot of files (even prior versions). rtsx.c at the very least is missing
Does this driver support MMCCAM?

2827 ↗(On Diff #77043)

This file is absent from the review.

31 ↗(On Diff #77043)

Is this really needed? Have you done extensive testing on 11?

This revision now requires changes to proceed.Sep 21 2020, 7:24 PM added inline comments.
2827 ↗(On Diff #77043)

I am struggling to correctly understand the arc command...

31 ↗(On Diff #77043)

I can't do testing on 11. I add this after this comment:
PR204521 - comment 101 added inline comments.
31 ↗(On Diff #77043)

Revision 320844: "Implement the MMC/SD/SDIO protocol within a CAM framework" added
IO_SEND_OP_COND to sys/dev/mmc/mmcreg.h. this was never MFC to 11.

27 ↗(On Diff #77349)

nit: before commit, we'll need to roll this date forward.
More generally, though, please make sure that 'igor' and 'mandoc -Tlint' pass w/o complaint

Amend rtsx.4 and modify Makefile and rtsx.c for MMCCAM better support. added inline comments.
27 ↗(On Diff #77349)

After this amend of rtsx.4:

[hlh@morzine rtsx]$ igor -D rtsx.4
[hlh@morzine rtsx]$ mandoc -Tlint rtsx.4
[hlh@morzine rtsx]$ marked an inline comment as done.

Modify rtsx_cam_* functions to return void and set better ccb status

  • Modify srtlcpy so that the serial number is not truncated.
388 ↗(On Diff #77623)

A style(9) nit-picking; `{' should be on a separate line.

The function type should be on a line by itself preceding the function.
The opening brace of the function body should be on a line by itself.
494 ↗(On Diff #77623)

Another style nit.

2167 ↗(On Diff #77623)

One more nit.

2205 ↗(On Diff #77623)


2267 ↗(On Diff #77623)


2956 ↗(On Diff #77623)


  • Style(9) rtsx.c (function definition).

Update MMCCAM implementation for 13.0-CURRENT.

  • return host_max_data in case XPT_GET_TRAN_SETTINGS,
  • add MMC_VCCQ in rtsx_cam_set_tran_settings().
2827 ↗(On Diff #77043)

This file is absent from the review.

I think that all files are in the last commit.

Sorry for the delay... This got put aside for some other things going on in $LIFE

Looking good and we're quite close. Thanks for addressing all the comments to date.

1311 ↗(On Diff #77921)

This is unrelated.... why's it needed?

1014 ↗(On Diff #77921)

this should live in opt_mmccam.h, or maybe opt_rtsx_inversion (though it's likely best if it is a tunable).

3599–3603 ↗(On Diff #77921)

These lines can be deleted if ....

3617 ↗(On Diff #77921) make this CTLFALG_RWTUN so that this is populated as a tunable.

Though you could also have a hw.rtsx.inversion global setting too, though that duplicates things.

31 ↗(On Diff #77043)

OK. If you could make this __FreeBSD_version < 1200000 that would be great. This depends on which compiler is used, not what version of the kernel you're building as it stands...

  • replace options INVERSION by tunable dev.rtsx.0.inversion. added inline comments.
1311 ↗(On Diff #77921)

This is unrelated.... why's it needed?

Just to have the complete sn in gpart disk-id.

Just wondering what next...

I am going to commit it to head today.

This revision was not accepted when it landed; it landed in state Needs Review.Nov 24 2020, 9:29 PM
This revision was automatically updated to reflect the committed changes.

@imp, if you have any more comments, please let me know. Thanks!