Page MenuHomeFreeBSD
Differential D55986

ufshci: Support ACPI
ClosedPublic
Actions

Authored by jaeyoon on Fri, Mar 20, 7:25 AM.
Tags
None
Referenced Files
F150331641: D55986.id173996.diff
Tue, Mar 31, 6:36 AM
Unknown Object (File)
Mon, Mar 30, 1:01 AM
Unknown Object (File)
Sat, Mar 28, 11:12 PM
Unknown Object (File)
Sat, Mar 28, 10:55 PM
Unknown Object (File)
Sat, Mar 28, 3:46 AM
Unknown Object (File)
Fri, Mar 27, 12:57 AM
Unknown Object (File)
Wed, Mar 25, 9:13 AM
Unknown Object (File)
Tue, Mar 24, 4:14 AM
View All 10 Files
Subscribers
None

Details

Reviewers
imp
Commits
rGe2083e8d3a01: ufshci: Support ACPI
Summary

Supports UFS host controller attachment via ACPI. Tested on the
Samsung Galaxy Book 4 Edge using Qualcomm Snapdragon X Elite.
Additionally, a quirk related to power mode change has been added.

For reference, it doesn't reach maximum speed yet. I plan to improve
it later.

Sponsored by: Samsung Electronics

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 71578
Build 68461: arc lint + arc unit

Event Timeline

jaeyoon requested review of this revision.Fri, Mar 20, 7:25 AM
jaeyoon created this revision.
jaeyoon added a parent revision: D55985: ufshci: add uic powermode parameter to sysctl.
imp added inline comments.Fri, Mar 20, 1:46 PM
sys/dev/ufshci/ufshci_acpi.c
85

why is 0 treated special here? Generically, 0 is a fine UID number. Is there some guarantee that we won't need to match on UID 0, but no others (the way the code is now is 0 matches all UID, which seems sensible... though I wonder why we ever care about the UID to do quirk matching).

sys/dev/ufshci/ufshci_ctrlr.c
125

Would this code be better in its own function, even if it is a singleton?

358

This might be better on one line, even if it pushes past 80 columns. The alignment shown in phab is weird, we usually just indent 4 extra spaces.

360

Here I'd definitely fold the lines. Going over 80 columns by 1 or 2 characters is preferable.

jaeyoon updated this revision to Diff 174149.Mon, Mar 23, 6:06 AM

Apply review comments.

Harbormaster completed remote builds in B71659: Diff 174149.Mon, Mar 23, 6:06 AM
jaeyoon marked 2 inline comments as done.Mon, Mar 23, 6:08 AM

Thanks for the review!

sys/dev/ufshci/ufshci_acpi.c
85

It looks like I copied some incorrect code from sdhci_acpi. Since checking the UID is pointless, I will remove the UID field.

sys/dev/ufshci/ufshci_ctrlr.c
358

clang-format15 -i sys/dev/ufshci/ufshci_ctrlr.c

Clang-format enforces this awkward indentation. Do you have any ideas for how to avoid it?

360

clang-format15 -i sys/dev/ufshci/ufshci_ctrlr.c

Clang-format enforces this awkward indentation. Do you have any ideas for how to avoid it?

imp accepted this revision.Mon, Mar 23, 2:28 PM

If you are using clang-format, then go ahead and commit. I'm not sure how to get it to behave better in a way that isn't a big hack.

This revision is now accepted and ready to land.Mon, Mar 23, 2:28 PM
jaeyoon marked 3 inline comments as done.Tue, Mar 24, 12:21 AM
Closed by commit rGe2083e8d3a01: ufshci: Support ACPI (authored by jaeyoon). · Explain WhyWed, Mar 25, 4:47 AM
This revision was automatically updated to reflect the committed changes.
jaeyoon added a commit: rGe2083e8d3a01: ufshci: Support ACPI.

Revision Contents
Changeset List

PathSize
sys/
conf/
3 lines
dev/
ufshci/
259 lines
69 lines
12 lines
modules/
ufshci/
3 lines

Diff 173996

View Options

sys/conf/files

Loading...
View Options

sys/dev/ufshci/ufshci_acpi.c

Loading...
View Options

sys/dev/ufshci/ufshci_ctrlr.c

Loading...
View Options

sys/dev/ufshci/ufshci_private.h

Loading...
View Options

sys/modules/ufshci/Makefile

Loading...