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

修复睡眠唤醒后初始化失败的问题 #18

Merged
merged 3 commits into from
Feb 9, 2021
Merged

修复睡眠唤醒后初始化失败的问题 #18

merged 3 commits into from
Feb 9, 2021

Conversation

CLAY-BIOS
Copy link

No description provided.

@CLAY-BIOS
Copy link
Author

Only 0x522710EC was tested.

@Sniki
Copy link

Sniki commented Jan 27, 2021

This is amazing, i have Lenovo ThinkPads:

  • L440
  • T440
  • T440S
  • X240-

And all of them have the same 0x522710EC card reader.
So with this pull request we should finally be able to still use the card reader after wake from sleep, that's amazing.

Will test within a few days and report back with results.

@cholonam
Copy link
Owner

@CLAY-BIOS Thanks for the PR. I'll wait to see @Sniki test results before merging. I did not have time to look carefully at the PR (my free time is very limited right now), but it seems it's adding a few seconds of wait every time there is a power change? I'm afraid this may make the kext too slow to respond for other chips which do not have this problem.. did you try with shorter times? Maybe this hardcoded value (1000?) could be made a boot parameter defaulting to zero? I am pretty sure for most people zero or some milliseconds will be enough. Thanks again.

@CLAY-BIOS
Copy link
Author

@CLAY-BIOS Thanks for the PR. I'll wait to see @Sniki test results before merging. I did not have time to look carefully at the PR (my free time is very limited right now), but it seems it's adding a few seconds of wait every time there is a power change? I'm afraid this may make the kext too slow to respond for other chips which do not have this problem.. did you try with shorter times? Maybe this hardcoded value (1000?) could be made a boot parameter defaulting to zero? I am pretty sure for most people zero or some milliseconds will be enough. Thanks again.

I did some tests and the card reader (0x522710EC) did not work properly when IOSleep() value was below 300.
Some chips with the same problem may require higher values.
I don't think setting IOSleep() to 1000 will have much impact, and even if I set IOSleep() to 5000, the card reader will still work.

@ryahpalma
Copy link

Merge, please.

@cholonam
Copy link
Owner

cholonam commented Feb 8, 2021

@Sniki any news on the testing? @ryahpalma could you test these changes? I cannot test since I do not have this chip.

@Sniki
Copy link

Sniki commented Feb 8, 2021

@cholonam not yet, i am all ready and prepared for testing but i have macOS only on my 13" MacbookPro 2017 which does only have USB-C Ports and i don't have any dongle as i just recently got it.
Waiting for a friend to bring me his Dongle from USB-C to USB3 so i can make the installer on my USB3 stick and place the prepared EFI for install on my Lenovo ThinkPad T440S.

I also got a couple of SD Cards to do some deep testing and play a bit more with that value and find the most optimal one.
I have both old slow SD Cards and newer ones, those should help find the lowest value that are just enough for even the slow cards to load.

So merging is up to you, i should be able to complete the testing by the end of this weekend.

@cholonam
Copy link
Owner

cholonam commented Feb 8, 2021

Sounds good. I think I'll have some time today to upload a pre-release version, in case somebody else can also test it.

{0x11, 0x11, 0x18},
{0x55, 0x55, 0x5C},
{0xFF, 0xFF, 0xFF},
{0x13, 0x13, 0x13},
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest adding a new file (e.g: rts5227.c) with its own methods instead of modifying this one.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest adding a new file (e.g: rts5227.c) with its own methods instead of modifying this one.

It doesn't seem to make sense to change here.
I have restored it before merging. What I did was add IOSleep() to Sinetek_rtsx::setPowerState.

Copy link
Owner

@cholonam cholonam Feb 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @CLAY-BIOS (I understand those changes are for rts5227, and they probably will affect 5249 and 525A). If you really need those, I think you should add a new file, rts5227.c, based on this one, and use its functions when the chip is detected to actually be rts5227. Otherwise maintenance is unmanageable.

@@ -5,6 +5,7 @@

#include <IOKit/IOLib.h>
#include <IOKit/pci/IOPCIDevice.h>
#include <IOKit/pci/IOPCIDevice.h>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

repeated

@cholonam cholonam merged commit bcd7ee8 into cholonam:master Feb 9, 2021
@cholonam
Copy link
Owner

cholonam commented Feb 9, 2021

I just uploaded a new relase. It basically contains @CLAY-BIOS pull request, but I made it boot-configurable, so to get @CLAY-BIOS code, just add rtsx_sleep_wake_delay_ms=1000 as kernel boot parameter. It should be easy to fine-tune the delay by playing with this parameter.

Since the default value is zero, I made it just a normal release and not a pre-release (nothing should break if the parameter is not used).

@Sniki
Copy link

Sniki commented Feb 11, 2021

@cholonam i installed macOS Big Sur 11.2.1 yesterday on my Lenovo ThinkPad T440S and i can confirm that the Card Reader works after wake from sleep correctly now.

I haven’t played with the values, will do some testing tonight by testing lower values.

If you leave the SD Card Plugged and you put laptop into sleep mode, once it wakes, it gives you the message that the device was incorrectly removed like it does if you remove a USB device without right click > eject.
But after a second it re-mounts correctly automatically.

But i do believe you do get the same message and same behavior even with standard USB ports.

No mimic linux needed, you can add 5227 to working/supported list.

Thanks a lot !

@cholonam cholonam mentioned this pull request Feb 15, 2021
@CLAY-BIOS
Copy link
Author

CLAY-BIOS commented Feb 17, 2021

@Sniki Do you have other chips to test? I believe this method may also be applicable to 0x522a. #12
@cholonam This project may be helpful to you, (https://github.com/hlh-restart/rtsx), Thank you for your contribution.

@Sniki
Copy link

Sniki commented Feb 25, 2021

@CLAY-BIOS unfortunately no, i don't have any other chip besides 0x5227 model.

I just wanted to mention that sleep values like these may be affected by the level of hackintosh optimizations done.
In my testing case i tested it on my X240 and T440S and they both working perfect with just a value of IOSleep(100)

So for Haswell generations ThinkPad(s), i think IOSleep value of (100) works perfect, tested many many times repeatedly and tried going below 100 but whatever went below 100, it didn't work.

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

Successfully merging this pull request may close these issues.

5 participants