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)
Sun, Apr 28, 11:01 PM
Unknown Object (File)
Mon, Apr 22, 2:02 PM
Unknown Object (File)
Wed, Apr 17, 7:43 AM
Unknown Object (File)
Wed, Apr 17, 7:43 AM
Unknown Object (File)
Wed, Apr 17, 6:29 AM
Unknown Object (File)
Feb 24 2024, 4:23 PM
Unknown Object (File)
Dec 20 2023, 2:36 AM
Unknown Object (File)
Dec 13 2023, 6:20 AM
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–153

This should be committed in separately.

192–193

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

237

ditto

239

ditto

302

Why? Initialization is not necessary.

304

why?

326–327

wrong unrelated change

981

why? Clearing SDMMC_BMOD_DE should be sufficient.

1235

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–193

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...

302

Ah, yes. You are right.

326–327

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

981

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.

1235

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.