Re: [edk2-devel] [PATCH 0/3] use XHCI to replace EHCI

Re: [edk2-devel] [PATCH 0/3] use XHCI to replace EHCI

[Leif Lindholm]

Hi,

Apologies for the long delay reviewing this set.

I have rebased your patches onto:
https://github.com/leiflindholm/edk2-platforms/tree/sbsa-xhci

I had to move one hunk (.dec change) from 2/3 to 1/3 in order to
not break bisect.

If you're happy with the slightly modified version, I think this
is good to go in. Please let me know.

For the set:
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>

/
    Leif


On Wed, May 31, 2023 at 17:14:20 +0800, Yuquan Wang wrote:
> This patchset implements XHCI on sbsa-ref board to replace EHCI.
> As sbsa-ref does not have DRAM below 4G, it cannot utilize EHCI 
> that only has 32-bit DMA capablity. Now this board has XHCI as 
> an available usb controller with 64-bit DMA capablity.
>  
> Yuquan Wang (3):
>   Platform/Qemu/SbsaQemu/SbsaQemu.dsc: define XHCI Pcd settings
>   SbsaQemu: Drivers: Add initial support for XHCI
>   SbsaQemu: AcpiTables: Add XHCI info into DSDT


>  Platform/Qemu/SbsaQemu/SbsaQemu.dsc           |   6 +-
>  .../Qemu/SbsaQemu/AcpiTables/AcpiTables.inf   |   4 +-
>  Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl     | 125 ++----------------
>  .../SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c |  40 ++++--
>  .../SbsaQemuPlatformDxe.inf                   |   2 +
>  Silicon/Qemu/SbsaQemu/SbsaQemu.dec            |   4 +-
>  6 files changed, 53 insertions(+), 128 deletions(-)
> 
> -- 
> 2.34.1
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108189): https://edk2.groups.io/g/devel/message/108189
Mute This Topic: https://groups.io/mt/101073211/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 0/3] use XHCI to replace EHCI

[Ard Biesheuvel]

On Thu, 31 Aug 2023 at 15:06, Leif Lindholm <quic_llindhol@quicinc.com> wrote:
>
> Hi,
>
> Apologies for the long delay reviewing this set.
>
> I have rebased your patches onto:
> https://github.com/leiflindholm/edk2-platforms/tree/sbsa-xhci
>
> I had to move one hunk (.dec change) from 2/3 to 1/3 in order to
> not break bisect.
>
> If you're happy with the slightly modified version, I think this
> is good to go in. Please let me know.
>
> For the set:
> Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
>

I don't know where the original DSDT content came from but patch 3/3
is awfully light on details on why it is ok to rip out the whole
hierarchy and replace it with just the host controller node.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108190): https://edk2.groups.io/g/devel/message/108190
Mute This Topic: https://groups.io/mt/101073211/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 0/3] use XHCI to replace EHCI

[Yuquan Wang]

[-- Attachment #1: Type: text/plain, Size: 906 bytes --]

Hi, Leif
On 2023-08-31 21:06,  Leif wrote:

I had to move one hunk (.dec change) from 2/3 to 1/3 in order to
not break bisect.
 
If you're happy with the slightly modified version, I think this
is good to go in. Please let me know.
 

It' ok. 

In the second version of this series patches, in fact, I have moved the .dec change into 
1/3. 

It is my fault that I forgot to add "Cc" to Ard, Leif and Graeme in my latest version 
of  "xhci" patches.
The link is: https://edk2.groups.io/g/devel/message/106729

Many thanks
Yuquan


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108252): https://edk2.groups.io/g/devel/message/108252
Mute This Topic: https://groups.io/mt/101073211/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 2380 bytes --]

Re: [edk2-devel] [PATCH 0/3] use XHCI to replace EHCI

[Leif Lindholm]

Hi Yuquan,

On Mon, Sep 04, 2023 at 14:30:57 +0800, Yuquan Wang wrote:
> Hi, Leif
> On 2023-08-31 21:06,  Leif wrote:
> 
> I had to move one hunk (.dec change) from 2/3 to 1/3 in order to
> not break bisect.
>  
> If you're happy with the slightly modified version, I think this
> is good to go in. Please let me know.
>  
> 
> It' ok. 
> 
> In the second version of this series patches, in fact, I have moved the .dec change into 
> 1/3. 
> 
> It is my fault that I forgot to add "Cc" to Ard, Leif and Graeme in my latest version 
> of  "xhci" patches.
> The link is: https://edk2.groups.io/g/devel/message/106729

Understood, thanks.

However, this version still breaks bisect: attempting to build at 1/3
or 2/3 leads to:
---
/work/git/edk2-platforms/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf(72):
error 3000: PCD
[gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformEhciBase] in
[/work/git/edk2-platforms/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf]
is not found in dependent packages:
---

Also, I think Ard's comments on the ACPI changes still apply.
Do you have a comment on that?

Best Regards,

Leif


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108259): https://edk2.groups.io/g/devel/message/108259
Mute This Topic: https://groups.io/mt/101073211/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 0/3] use XHCI to replace EHCI

[Yuquan Wang]

[-- Attachment #1: Type: text/plain, Size: 1391 bytes --]

Hi, Leif
On 2023-09-04 21:51,  quic_llindhol wrote:
> 
> However, this version still breaks bisect: attempting to build at 1/3
> or 2/3 leads to:
> ---
> /work/git/edk2-platforms/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf(72):
> error 3000: PCD
> [gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformEhciBase] in
> [/work/git/edk2-platforms/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf]
> is not found in dependent packages:
> ---

Maybe it is a stupid question: What is the meaning of bisect in here?

> 
> Also, I think Ard's comments on the ACPI changes still apply.
> Do you have a comment on that?
> 

Sorry, I' m honestly not knowledgable enough about the DSDT content so I just managed to use the 
"Device (XHCI)" node to support the xhci controller to work and remove the previous "Device (USB0)"
node. 

If you have time to discuss it, I'm glad to continue to fix and improve my patches. And I will go on to improve
myself and learn from experienced experts like yours.

Many thanks
Yuquan


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108274): https://edk2.groups.io/g/devel/message/108274
Mute This Topic: https://groups.io/mt/101073211/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 3090 bytes --]

Re: [edk2-devel] [PATCH 0/3] use XHCI to replace EHCI

[Marcin Juszkiewicz]

W dniu 5.09.2023 o 08:11, Yuquan Wang pisze:
> Hi, Leif
> On 2023-09-04 21:51,  quic_llindhol wrote:
>  >
>  > However, this version still breaks bisect: attempting to build at 1/3
>  > or 2/3 leads to:
>  > ---
>  > 
> /work/git/edk2-platforms/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf(72):
>  > error 3000: PCD
>  > [gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformEhciBase] in
>  > 
> [/work/git/edk2-platforms/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf]
>  > is not found in dependent packages:
>  > ---
> 
> Maybe it is a stupid question: What is the meaning of bisect in here?

Project is meant to be buildable at every commit. Your patchset breaks 
that rule.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108276): https://edk2.groups.io/g/devel/message/108276
Mute This Topic: https://groups.io/mt/101073211/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 0/3] use XHCI to replace EHCI

[Yuquan Wang]

[-- Attachment #1: Type: text/plain, Size: 692 bytes --]

On 2023-09-05 14:30,  marcin.juszkiewicz wrote:
 
Project is meant to be buildable at every commit. Your patchset breaks 
that rule.

Oh! I get it ! I did not consider this principle when designing these patches. 

Thanks for your explanation : ) I am going to recheck and combine some content together.

Many thanks
Yuquan


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108277): https://edk2.groups.io/g/devel/message/108277
Mute This Topic: https://groups.io/mt/101073211/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 1969 bytes --]

[PATCH 0/3] use XHCI to replace EHCI

[wangyuquan1236]

This patchset implements XHCI on sbsa-ref board to replace EHCI.
As sbsa-ref does not have DRAM below 4G, it cannot utilize EHCI 
that only has 32-bit DMA capablity. Now this board has XHCI as 
an available usb controller with 64-bit DMA capablity.
 
Yuquan Wang (3):
  Platform/Qemu/SbsaQemu/SbsaQemu.dsc: define XHCI Pcd settings
  SbsaQemu: Drivers: Add initial support for XHCI
  SbsaQemu: AcpiTables: Add XHCI info into DSDT

 Platform/Qemu/SbsaQemu/SbsaQemu.dsc           |   6 +-
 .../Qemu/SbsaQemu/AcpiTables/AcpiTables.inf   |   4 +-
 Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl     | 125 ++----------------
 .../SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c |  40 ++++--
 .../SbsaQemuPlatformDxe.inf                   |   2 +
 Silicon/Qemu/SbsaQemu/SbsaQemu.dec            |   4 +-
 6 files changed, 53 insertions(+), 128 deletions(-)

-- 
2.34.1

Re: [PATCH 0/3] use XHCI to replace EHCI

[Ard Biesheuvel]

On Mon, 5 Jun 2023 at 12:50, Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
>
> This patchset implements XHCI on sbsa-ref board to replace EHCI.
> As sbsa-ref does not have DRAM below 4G, it cannot utilize EHCI
> that only has 32-bit DMA capablity. Now this board has XHCI as
> an available usb controller with 64-bit DMA capablity.
>
> Yuquan Wang (3):
>   Platform/Qemu/SbsaQemu/SbsaQemu.dsc: define XHCI Pcd settings
>   SbsaQemu: Drivers: Add initial support for XHCI
>   SbsaQemu: AcpiTables: Add XHCI info into DSDT
>

Why are you sending the same series 3 times within an hour?


>  Platform/Qemu/SbsaQemu/SbsaQemu.dsc           |   6 +-
>  .../Qemu/SbsaQemu/AcpiTables/AcpiTables.inf   |   4 +-
>  Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl     | 125 ++----------------
>  .../SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c |  40 ++++--
>  .../SbsaQemuPlatformDxe.inf                   |   2 +
>  Silicon/Qemu/SbsaQemu/SbsaQemu.dec            |   4 +-
>  6 files changed, 53 insertions(+), 128 deletions(-)
>
> --
> 2.34.1
>

Re: [edk2-devel] [PATCH 0/3] use XHCI to replace EHCI

[wangyuquan1236]

[-- Attachment #1: Type: text/plain, Size: 129 bytes --]

Sorry for my awkward operations, because I felt the email refused problem with devel@edk2.groups.io; ( devel@edk2.groups.io, )

[-- Attachment #2: Type: text/html, Size: 323 bytes --]

Re: [edk2-devel] [PATCH 0/3] use XHCI to replace EHCI

[Ard Biesheuvel]

On Mon, 5 Jun 2023 at 12:53, wangyuquan1236
<wangyuquan1236@phytium.com.cn> wrote:
>
> Sorry for my awkward operations, because I felt the email refused problem with devel@edk2.groups.io;

Are you subscribed to the mailing list?

Re: [edk2-devel] [PATCH 0/3] use XHCI to replace EHCI

[wangyuquan1236]

[-- Attachment #1: Type: text/plain, Size: 148 bytes --]

Yes, at the third time I found I need to subscribe this channel, so I succeeded.
Now, I can send the patch normally.

Thanks for your guidance.

[-- Attachment #2: Type: text/html, Size: 210 bytes --]

Re: [edk2-devel] [PATCH 0/3] use XHCI to replace EHCI

[Leif Lindholm]

On 2023-06-05 11:51, Ard Biesheuvel wrote:
> On Mon, 5 Jun 2023 at 12:50, Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
>>
>> This patchset implements XHCI on sbsa-ref board to replace EHCI.
>> As sbsa-ref does not have DRAM below 4G, it cannot utilize EHCI
>> that only has 32-bit DMA capablity. Now this board has XHCI as
>> an available usb controller with 64-bit DMA capablity.
>>
>> Yuquan Wang (3):
>>    Platform/Qemu/SbsaQemu/SbsaQemu.dsc: define XHCI Pcd settings
>>    SbsaQemu: Drivers: Add initial support for XHCI
>>    SbsaQemu: AcpiTables: Add XHCI info into DSDT
>>
> 
> Why are you sending the same series 3 times within an hour?

I think there was unfortunate timing with my releasing from moderation 
and unmoderating Yuquan and them resubmitting because they didn't see 
patches hitting the list.

/
     Leif

>>   Platform/Qemu/SbsaQemu/SbsaQemu.dsc           |   6 +-
>>   .../Qemu/SbsaQemu/AcpiTables/AcpiTables.inf   |   4 +-
>>   Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl     | 125 ++----------------
>>   .../SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c |  40 ++++--
>>   .../SbsaQemuPlatformDxe.inf                   |   2 +
>>   Silicon/Qemu/SbsaQemu/SbsaQemu.dec            |   4 +-
>>   6 files changed, 53 insertions(+), 128 deletions(-)
>>
>> --
>> 2.34.1
>>
> 
> 
> 
> 
>