Page MenuHomeFreeBSD

dwmmc: switch to double-buffer structure
AbandonedPublic

Authored by kjopek_gmail.com on Oct 9 2021, 1:54 AM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 20 2023, 2:36 AM
Unknown Object (File)
Dec 13 2023, 6:20 AM
Unknown Object (File)
Nov 21 2023, 1:07 PM
Unknown Object (File)
Oct 19 2023, 7:14 PM
Unknown Object (File)
Oct 14 2023, 4:26 PM
Unknown Object (File)
Sep 12 2023, 8:17 PM
Unknown Object (File)
Jun 21 2023, 3:15 PM
Unknown Object (File)
May 3 2023, 6:52 PM
Subscribers

Details

Reviewers
mmel
avg
manu
Summary

The current version of dwmmc uses chained structure where second pointer (des3) of each descriptor is a pointer to the next descriptor in the ring.

dwmmc works rather in request-completion mode and structure of descriptors is reset anyway during the setup of the next request. This makes double-buffer structure more natural and much easier to implement.
Migration from chained mode to double-buffer mode also increases capacity of single request up to 2MB (256 descriptors * 2 buffers per descriptor * 4096 bytes per buffer), which
was never hit in my own tests – the largest requests were up to 1MB in size. This means we have still a lot of space when upper layers of the stack decide to send larger requests.

Test Plan

Tested on RockPro64 (fsck + recoverdisk + dd).

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

mmel requested changes to this revision.Oct 18 2021, 2:14 PM
mmel added inline comments.
sys/dev/mmc/host/dwmmc.c
148

This should be committed in separately.

192

style: initialization in declaration

199

ditto

209

This is wrong. the OWN bit *must* be set after all fields of descriptor are filed and visible to iDMA. See old code for wmb() usage

222

ditto

224

ditto

299

Why? Initialization is not necessary.

307

why?

329

wrong unrelated change

983

why? Clearing SDMMC_BMOD_DE should be sufficient.

1236

style + unrelated change

This revision now requires changes to proceed.Oct 18 2021, 2:14 PM
kjopek_gmail.com added inline comments.
sys/dev/mmc/host/dwmmc.c
192

style(9) says:

Variables may be initialized where declared especially when they are constant for the rest of the scope.

Is there any exception here? In the spirit of this sentence I also initialized the other variables in the rest of similar places.

206

Comment here is invalid: the driver should either set DES0_ER | DES0_LD or DES0_DIC.

209

I would like to get to know a little more details on this. My current understanding of the code is that dwmmc starts execution of the transfer after we this function returns, that is, there are no other accesses to descriptors either by the host or by the before dwmmc explicitly starts the transfer. However, I did not find any precise information on this topic in documentation I own (rk3399 TRM) and I can be wrong...

299

Ah, yes. You are right.

329

Sorry for this (lack of proper cleanup before creation of MR)!

983

I just wanted to be sure that DSL is set to 0 unconditionally. Any other value of DSL may result in a (silent) memory corruption so it is always safer to enforce 0 value every time we access BMOD register.

1236

Sorry!

Update according to comments. IDMAC_MAX_SIZE will be changed in another MR.

There is no visible benefits so far from that change apart from more efficient usage of descriptors. Abandoning it for now.