-
Notifications
You must be signed in to change notification settings - Fork 14
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
EmbeddedPkg:Application:AndroidFastboot Request data length clearly #61
base: hikey970_v1.0
Are you sure you want to change the base?
Conversation
Signed-off-by: chenyu <[email protected]>
@chenyu56 Could you help to give more comments on this pull request? |
This patch sets exact data length that need to receive from host when "mState == ExpectDataState". |
AsciiSPrint (Response, sizeof Response, "DATA%x", | ||
(UINT32)mNumDataBytes); | ||
mTransport->Send (sizeof Response - 1, Response, &mFatalSendErrorEvent); | ||
|
||
mState = ExpectDataState; | ||
mBytesReceivedSoFar = 0; | ||
if (mTransport->RequestReceive) { | ||
mTransport->RequestReceive (mNumDataBytes); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chenyu56 Your comments didn't hit the point. You change the sequence at here. You give a response first before receiving data. Could you explain the reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving these codes after sending response to host and set "mBytesReceivedSoFar = 0" is more reasonable.
} else { | ||
ASSERT (FALSE); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to move the code? If not, what happen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because of the patchs in OpenPlatformPkg/Drivers/Usb/DwUsb3Dxe,we need to call "mTransport->RequestReceive" every time we want to start a receiving which was doing in OpenPlatformPkg/Drivers/Usb/DwUsb3Dxe before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chenyu56 AndroidFastbootApp is a common application. HiKey, HiKey960 and ArmVExpress are also using AndroidFastbootApp. Do they need to update code when your patch is merged?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HiKey and ArmVExpress need to update code if you apply the patch to their branch.
Hikey960 do not need to update code because it share code of OpenPlatformPkg/Drivers/Usb/DwUsb3Dxe with Hikey970.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then it's a problem to merge this patch. Our goal is to upstream everything. If it blocks other platform, it can't be upstream. Please help to prepare patches for other platform.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for replying late, I need some time to have a look at the code of Hikey and ArmVExpress. And I want to know how to verify the code of Hikey and ArmVExpress?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can follow it to build the firmware on HiKey (https://github.com/ARM-software/arm-trusted-firmware/blob/master/docs/plat/hikey.rst). There're some HiKey boards in Hisilicon lab. Since the document is for latest code, you could get a similar one from the same directory in your HiKey970 code base.
I'm not sure whether you have ArmVExpress. Maybe I can help to verify it on ArmVExpress.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chenyu56 Any update?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently I look at the OpenPlatformPkg/Drivers/Usb/DwUsb3Dxe/DwUsbDxe.c of HiKey, and found that my patch need to be modified to adjust HiKey. I can do adjusting and verifying and HiKey, but I do not have ArmVExpress and I don't know where is the code of ArmVExpres.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EmbeddedPkg/Drivers/Isp1761UsbDxe/
Signed-off-by: chenyu [email protected]