On Mon, Oct 4, 2021 at 9:04 AM Olivier Deprez Olivier.Deprez@arm.com wrote:

I left few comments.

I think the main question is how do you test those changes?

The only board supported in TF-A which has an UFS device on PCB is the Hikey960. I assume you have your own board against which you test those changes?

As the changes are related to sensitive timings at init / identification (and changing the logic from an infinite poll loop to a finite counting loop) I sense there may be a lot of variation from one board to another, or even one flash chip to another (also depends on flash vendor). So while this may work with one particular configuration, it's difficult to tell if it scales stable to a variable number of configurations. I'll let the Hikey960 / UFS maintainer comment more if need be.

Yea, I too will defer to Haojian as he has more expertise on the topic. But as I mentioned in Gerrit, HiKey960 has had a number of UFS related issues with the non-proprietary bootloader, especially with regards to different versions of the board that used different UFS chips. So this is a sensitive pathway.

Unfortunately thorough testing will take some time, and I'm a bit short on bandwidth, so maybe if Haojian and others don't see any issues in the logic, we can do some quick initial smoke tests and if it seems ok go ahead, as I am not personally eager to respin the bootloader for HiKey960 in the near future, and I'm not sure how much longer the board will be actively useful.

thanks -john

Hello! Gentle reminder, can I get code review from the UFS owner for the patches linked above?

On Wed, Oct 6, 2021 at 8:00 AM Jorge Troncoso jatron@google.com wrote:

Thanks Olivier and John for your feedback on the patches! I addressed your comments in the CLs. Let me know if you have additional questions.

Regarding testing, we verified these changes on our own board, but I would love to hear how your HiKey960 tests fair. Let me know if there is anything I can do to help in this area.

As a next step, can I get code review from the UFS owner for the patches linked above? Haojian?

Thanks! Jorge Troncoso

On Mon, Oct 4, 2021 at 10:10 AM John Stultz john.stultz@linaro.org wrote:

...

On Mon, Oct 4, 2021 at 9:04 AM Olivier Deprez Olivier.Deprez@arm.com wrote:

...

I left few comments.

I think the main question is how do you test those changes?

The only board supported in TF-A which has an UFS device on PCB is the

Hikey960.

...

I assume you have your own board against which you test those changes?

As the changes are related to sensitive timings at init /

identification (and changing the logic from an infinite poll loop to a finite counting loop) I sense there may be a lot of variation from one board to another, or even one flash chip to another (also depends on flash vendor). So while this may work with one particular configuration, it's difficult to tell if it scales stable to a variable number of configurations.

...

I'll let the Hikey960 / UFS maintainer comment more if need be.

Yea, I too will defer to Haojian as he has more expertise on the topic. But as I mentioned in Gerrit, HiKey960 has had a number of UFS related issues with the non-proprietary bootloader, especially with regards to different versions of the board that used different UFS chips. So this is a sensitive pathway.

Unfortunately thorough testing will take some time, and I'm a bit short on bandwidth, so maybe if Haojian and others don't see any issues in the logic, we can do some quick initial smoke tests and if it seems ok go ahead, as I am not personally eager to respin the bootloader for HiKey960 in the near future, and I'm not sure how much longer the board will be actively useful.

thanks -john

Hi all,

Thanks for your input on the patches! I received Code-Owner-Review+1 from Haojian on all three patches. What are the next steps for getting these changes submitted?

On Wed, Oct 6, 2021 at 7:25 PM John Stultz john.stultz@linaro.org wrote:

On Wed, Oct 6, 2021 at 8:01 AM Jorge Troncoso jatron@google.com wrote:

...

Thanks Olivier and John for your feedback on the patches! I addressed

your comments in the CLs. Let me know if you have additional questions.

...

Regarding testing, we verified these changes on our own board, but I

would love to hear how your HiKey960 tests fair. Let me know if there is anything I can do to help in this area.

Yea. I'm sorry, it will be a bit of effort to get the build environment up and going and then I have to flash and test the three variants I have. I'm not sure how you can help unless you have a HiKey960.

...

As a next step, can I get code review from the UFS owner for the patches

linked above? Haojian?

I believe its Golden Week this week, so Haojian may be out until next week (I don't know for sure though).

thanks -john

Hi John,

The process for getting a patch merged is mentioned https://trustedfirmware-a.readthedocs.io/en/latest/process/code-review-guide...

Regarding your patches, i ran CI yesterday and there were some static failures which you have fixed in latest version. The CI now passed and i have provided MR votes. CoR was provided by Haojian on older version of patch which needs to be provided again. Just to let you know that It is quite common that reviewers may take few days to review it again.

The guideline is, you request reviewers on gerrit first(and wait for few days) and if you don't get any replies then you can use mailing list to escalate it.

Thanks Manish ________________________________ From: TF-A tf-a-bounces@lists.trustedfirmware.org on behalf of Jorge Troncoso via TF-A tf-a@lists.trustedfirmware.org Sent: 13 October 2021 01:57 To: John Stultz john.stultz@linaro.org Cc: Anand Saminathan anans@google.com; tf-a@lists.trustedfirmware.org tf-a@lists.trustedfirmware.org; John Powell John.Powell@arm.com Subject: Re: [TF-A] [RFC]: UFS patches

Hello! Gentle reminder, what are the next steps for getting these changes submitted?

On Mon, Oct 11, 2021 at 4:40 PM Jorge Troncoso <jatron@google.commailto:jatron@google.com> wrote: Hi all,

Thanks for your input on the patches! I received Code-Owner-Review+1 from Haojian on all three patches. What are the next steps for getting these changes submitted?

On Wed, Oct 6, 2021 at 7:25 PM John Stultz <john.stultz@linaro.orgmailto:john.stultz@linaro.org> wrote: On Wed, Oct 6, 2021 at 8:01 AM Jorge Troncoso <jatron@google.commailto:jatron@google.com> wrote:

Thanks Olivier and John for your feedback on the patches! I addressed your comments in the CLs. Let me know if you have additional questions.

Regarding testing, we verified these changes on our own board, but I would love to hear how your HiKey960 tests fair. Let me know if there is anything I can do to help in this area.

Yea. I'm sorry, it will be a bit of effort to get the build environment up and going and then I have to flash and test the three variants I have. I'm not sure how you can help unless you have a HiKey960.

As a next step, can I get code review from the UFS owner for the patches linked above? Haojian?

I believe its Golden Week this week, so Haojian may be out until next week (I don't know for sure though).

thanks -john

Haojian,

As project Code Owner for this area and the HiSilicon platform that makes use of this can you provide the initial review? General code reviews and Maintainer reviews can then follow.

Thanks

Joanna

From: TF-A tf-a-bounces@lists.trustedfirmware.org on behalf of Jorge Troncoso via TF-A tf-a@lists.trustedfirmware.org Reply to: Jorge Troncoso jatron@google.com Date: Monday, 4 October 2021 at 16:01 To: "tf-a@lists.trustedfirmware.org" tf-a@lists.trustedfirmware.org, Soby Mathew Soby.Mathew@arm.com, Louis Mayencourt Louis.Mayencourt@arm.com, John Stultz john.stultz@linaro.org, Haojian Zhuang haojian.zhuang@linaro.org, John Powell John.Powell@arm.com, Anand Saminathan anans@google.com Subject: Re: [TF-A] [RFC]: UFS patches

Hello! Gentle reminder, can I get code review for the patches linked above?

On Fri, Oct 1, 2021 at 8:00 AM Jorge Troncoso <jatron@google.commailto:jatron@google.com> wrote: Hi all,

We made a few changes to the UFS driver. The proposed patches are posted here: https://review.trustedfirmware.org/q/topic:%22ufs_patches%22+(status:open%20...).

The patches mainly consist of the below changes: 1. Delete asserts. Return error values instead. 2. Add retry logic and timeouts. 3. Reuse ufshc_send_uic_cmd() for DME_GET and DME_SET commands.

Any feedback/comments on these patches would be greatly appreciated.

Thanks! Jorge Troncoso