Page MenuHomeFreeBSD
Differential D55986

ufshci: Support ACPI
ClosedPublic
Actions

Authored by jaeyoon on Fri, Mar 20, 7:25 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 14, 2:15 PM
Unknown Object (File)
Tue, Apr 14, 2:05 PM
Unknown Object (File)
Tue, Apr 14, 9:13 AM
Unknown Object (File)
Sun, Apr 12, 9:05 PM
Unknown Object (File)
Sat, Apr 11, 10:41 AM
Unknown Object (File)
Wed, Apr 8, 9:57 AM
Unknown Object (File)
Sun, Apr 5, 7:07 PM
Unknown Object (File)
Tue, Mar 31, 6:36 AM
View All 17 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 Not Applicable
Unit
Tests Not Applicable

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
86

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
169

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

360

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.

362

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
86

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

362

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/
248 lines
71 lines
12 lines
modules/
ufshci/
3 lines

Diff 174273

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