Page MenuHomeFreeBSD

Call taskqueue sooner, adjust some DELAY(), add inversion heuristic
ClosedPublic

Authored by hlh_restart.be on May 27 2021, 12:41 PM.
Tags
None
Referenced Files
F102430481: D30499.diff
Tue, Nov 12, 4:19 AM
Unknown Object (File)
Sun, Nov 3, 8:25 AM
Unknown Object (File)
Sat, Oct 19, 6:10 PM
Unknown Object (File)
Sat, Oct 19, 6:09 PM
Unknown Object (File)
Sat, Oct 19, 6:09 PM
Unknown Object (File)
Sat, Oct 19, 6:09 PM
Unknown Object (File)
Sat, Oct 19, 6:08 PM
Unknown Object (File)
Sat, Oct 19, 6:08 PM
Subscribers

Details

Summary
  • Some configuration (e.g. HP EliteBook 840 G3) have a dummy plastic in the card reader which is detected as a valid SD card. This induce long timeout at boot time. I introduce a reduce timeout (1 second) during the setup phase to alleviate this problem. (ref. bug 255130)
  • Some configuration crash at boot if rtsx is defined in the kernel config. See this mail. At boot time, without a card inserted, the driver found that a card is present and just after that a "spontaneous" interrupt is generated showing that no card is present. To solve this I set the DELAY() to one quarter of a second before checking the card presence during driver attach.
  • As advised by Adrian Chadd I setup taskqueue and dma sooner during the driver attach.
  • I add a heuristic to try to detect configuration needing inversion.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 39502
Build 36391: arc lint + arc unit

Event Timeline

hlh_restart.be held this revision as a draft.
jkim published this revision for review.May 27 2021, 7:21 PM
sys/dev/rtsx/rtsx.c
3648

Can you please swap the order of timeout1 and timeout2?

3662

Maybe you should hide it under bootverbose or rtsx_debug?

3704

The braces look too excessive for the simple goto statement, i.e.,

if (error)
        goto destroy_rtsx_irq_res;
sys/dev/rtsx/rtsx.c
3648

I use this order to have timeout1 before timeout2 in
sysctl dev.rtsx wich can be seen by the user; e.g.:
dev.rtsx.0.wake: 0
dev.rtsx.0.write_count: 0
dev.rtsx.0.read_count: 0
dev.rtsx.0.debug: 0
dev.rtsx.0.force_timing: 0
dev.rtsx.0.inversion: 0
dev.rtsx.0.read_only: 0
dev.rtsx.0.req_timeout1: 1
dev.rtsx.0.req_timeout2: 10
dev.rtsx.0.%parent: pci3
dev.rtsx.0.%pnpinfo: vendor=0x10ec device=0x5287 subvendor=0x1025 subdevice=0x121a class=0xff0000
dev.rtsx.0.%location: slot=0 function=0 dbsf=pci0:3:0:0 handle=\_SB_.PCI0.RP12.PXSX
dev.rtsx.0.%driver: rtsx
dev.rtsx.0.%desc: 2.0i Realtek RTL8411B PCIe MMC/SD Card Reader
dev.rtsx.%parent:

If this seems not appropriate I will switch them.

Minor nit in the man page.

share/man/man4/rtsx.4
118

s/try/tries/

Small change for a better style(9) and a spelling mistake.

OK for the manpage part of the change.

Make rtsx_debug a tunable and so available during rtsx_attach().

On Mon, Jun 7 the port sysutils/rtsx-kmod has been updated to this version.
For now no problem has been encountered.

Add two new XPT for MMC (see commit af2253f61c43 on sdhci.c)

Oups... previous commit message should have been:

Add two new XPT for MMC (see commit af2253f61c43 on sdhci.c)

I just wonder what is blocking a commit of this update ?

jkim added inline comments.
sys/dev/rtsx/rtsx.c
3648

So, you did it to make it easier to read from user point of view? If so, please rename them to something more descriptive, e.g., timeout_io, timeout_cmd.

This revision is now accepted and ready to land.Sep 9 2021, 12:43 PM

Rename timeout1 by timeout_cmd & timeout2 by timeout_io

This revision now requires review to proceed.Sep 9 2021, 2:39 PM
This revision was not accepted when it landed; it landed in state Needs Review.Sep 9 2021, 6:27 PM
This revision was automatically updated to reflect the committed changes.
markj added inline comments.
sys/dev/rtsx/rtsx.c
3761

This panic is fixed by commit d5341d72a11be200e536ac7d8967449a3f521792, I believe. The delay should not be necessary anymore.

sys/dev/rtsx/rtsx.c
3761

This panic is fixed by commit d5341d72a11be200e536ac7d8967449a3f521792, I believe. The delay should not be necessary anymore.

@hlh_restart.be, please confirm. I'll revert this one when you do.

sys/dev/rtsx/rtsx.c
3761

I never encounter this panic on my development/test configuration (acer with RTL8411B) so I can't test if everything is OK with delay(500).

This delay is only during attach so I think it make no harm to keep it like this.