Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8150: Address UFS overvolting issue #454

Merged
merged 1 commit into from
Jan 30, 2025
Merged

Conversation

remtrik
Copy link
Contributor

@remtrik remtrik commented Jan 30, 2025

UFS3 with UFS2 settings → Successfully boots → Irreversible harm to UFS chip due to overvoltage
UFS2 with UFS3 settings → Doesn't boot → No harm to UFS chip

Based on that, always assume device is UFS3 unless PcdStorageHasUFS3 is set to 0

UFS3 with UFS2 settings - Successfully boots -> Irreversible harm to UFS chip due to overvoltage
UFS2 with UFS3 settings - Doesn't boot -> No harm to UFS chip

Based on that always assume device is UFS3 unless PcdStorageHasUFS3 is set to 0
@n00b69
Copy link

n00b69 commented Jan 30, 2025

@remtrik That's actually a good change because I've seen storage upgrades that also update the ufs e.g LG with UFS 3.1

@remtrik
Copy link
Contributor Author

remtrik commented Jan 30, 2025

@remtrik That's actually a good change because I've seen storage upgrades that also update the ufs e.g LG with UFS 3.1

LGs with such upgrade will now require a separate UEFI, because PcdHasUFS3 is 0 on all LG devices

@n00b69
Copy link

n00b69 commented Jan 30, 2025

@remtrik I assume that's automatically getting updated eventually anyways, whenever the next update may come

@gus33000
Copy link
Collaborator

QC Stock firmware does indeed dynamically set the PUS3 variable upon detection of an UFS version 3 or higher; this is done by UFSDxe registering event call backs for the ACPI AML Patch protocol, and is implemented on XBL baselines from 2021-02-18 to today. Most devices however would be using an older baseline.

For now this workaround is more than suitable.

@gus33000
Copy link
Collaborator

The following is a pseudocode and not real code (location: UFSDxe entry point on modern baselines):

if (UFSInformatioNStructure.UFSSpecificationVersion >= 0x300 && (0 == AsciiStrCmp("WP", OsTypeString))) {
    gBS->CreateEvent(EVT_NOTIFY_SIGNAL, TPL_CALLBACK, AMLProtocolRegistration, NULL, &AMLProtocolRegistrationHandle);
    gBS->RegisterProtocolNotify(&gQcomAcpiPlatformProtocolGuid, AMLProtocolRegistrationHandle, &AMLProtocolRegistrationToken);
}

@gus33000
Copy link
Collaborator

It appears the UFSInformationStructure var is retrievable by end consumers by using the EFI_MEM_CARDINFO_PROTOCOL EFI protocol installed by UFSDxe itself, this might be doable by:

Writing code in the ACPI lib to look for such protocol;
request the information via it
if >= 0x300, set value to 1, otherwise 0

@gus33000
Copy link
Collaborator

Lets merge this first and look into this later. We have the Daniel update to prepare afterall.

@gus33000 gus33000 merged commit 7698261 into Project-Aloha:main Jan 30, 2025
34 checks passed
@qaz6750 qaz6750 added the enhancement New feature or request label Jan 30, 2025
@remtrik
Copy link
Contributor Author

remtrik commented Jan 30, 2025

It appears the UFSInformationStructure var is retrievable by end consumers by using the EFI_MEM_CARDINFO_PROTOCOL EFI protocol installed by UFSDxe itself, this might be doable by:

Writing code in the ACPI lib to look for such protocol; request the information via it if >= 0x300, set value to 1, otherwise 0

Something like this probably would have to be implemented for newer platforms at some point, since some OEMs like Xiaomi and OPPO started making models with different UFS versions based on memory configuration. As for the older platforms, we need to check if what you suggested is actually appliable, I checked UFSDxe on my POCO X3 Pro and POCO X3 NFC and they seem to lack anything that resembles UFS specification in so called UFSInformationStructure. They do however check it but don't save the result anywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants