Re: [PATCH v2 00/10] rockchip: rk3588: add support for DFU in SPL

[Eugen Hristev <eugen.hristev@collabora.com>]

On 9/22/23 03:48, Kever Yang wrote:
> 
> On 2023/9/21 22:47, Eugen Hristev wrote:
>> On 8/1/23 10:28, Eugen Hristev wrote:
>>> This series adds support for DFU in SPL for rockchip rk3588 on rock5b
>>> board.
>>>
>>> Namely, when SPL is loaded via rockusb (thus via USB), having the
>>> `same-as-spl` boot order item, after having detected that it was loaded
>>> from USB, it will lookup the gadget USB node in DT and boot via DFU.
>>>
>>> Some changes were required namely:
>>> - DFU needs environment, hence adding environment variables into DFU
>>> - added bootph-all to nodes such that they are available in SPL
>>> - insert gadget into boot order
>>>
>>> I had to port one patch for DWC3 from Linux, and include in this series
>>> the patches that are floating from Venkatesh that fixup the DWC3
>>> (https://marc.info/?l=u-boot&m=168351919807081&w=2 )
>>>
>>> I know that Marek NAKed them and I am fine with it, I am not trying to
>>> sneak in any patches, they are not to be merged, also this patch
>>> `usb: dwc3: Increase DWC3 controller halt timeout` is in the same bucket
>>> so Marek you can NAK this one as well, no problem, I am just sending out
>>> all the series so maybe the rockchip part for the gadget can be 
>>> picked up
>>> and if people want to use the DFU SPL gadget can also manually pick the
>>> DWC3 patches. The branch with all the patches is available here :
>>>
>>> https://gitlab.collabora.com/hardware-enablement/rockchip-3588/u-boot
>>>
>>> Thanks!
>>>
>>> Changes in v2:
>>> - rebased on latest upstream which caused a change in the config patch.
>>>
>>>
>>> Eugen Hristev (7):
>>>    rockchip: allow env defines for SPL build
>>>    usb: dwc3: Increase DWC3 controller halt timeout
>>>    ARM: dts: rockchip: rk3588-rock-5b-u-boot: add bootph-all to gadget
>>>      nodes
>>>    ARM: mach-rockchip: spl-boot-order: add possibility to DFU
>>>    ARM: mach-rockchip: rk3588: add gadget device to the boot order
>>>    rockchip: rk3588: prepare env for DFU
>>>    configs: rockchip: rock5b-rk3588: enable DFU and related configs
>>>
>>> Venkatesh Yadav Abbarapu (3):
>>>    usb: dwc3: core: improve reset sequence
>>>    usb: dwc3: gadget: Don't send unintended link state change
>>>    usb: dwc3: core: Only handle soft-reset in DCTL
>>>
>>>   arch/arm/dts/rk3588-rock-5b-u-boot.dtsi |  6 +++++
>>>   arch/arm/mach-rockchip/rk3588/rk3588.c  |  1 +
>>>   arch/arm/mach-rockchip/spl-boot-order.c |  3 +++
>>>   configs/rock5b-rk3588_defconfig         | 18 +++++++++++---
>>>   drivers/usb/dwc3/core.c                 | 32 +++++++++++++++----------
>>>   drivers/usb/dwc3/gadget.c               | 20 +++++++---------
>>>   drivers/usb/dwc3/gadget.h               | 14 +++++++++++
>>>   include/configs/rk3588_common.h         |  9 ++++++-
>>>   include/configs/rockchip-common.h       |  4 ----
>>>   9 files changed, 76 insertions(+), 31 deletions(-)
>>>
>>
>>
>> Hello Kever,
>>
>> I see in patchwork this series is marked as 'Changes requested'.
>> Do you wish to tell me which are the changes you requested, as I did 
>> not see anything in your replies.
>> I am interested to see the patches related to rockchip (not the DWC3), 
>> if it's fine for you to merge them.
> 
> Hi Eugen,
> 
>      For rockchip platform part is fine to me, but as a patchset, the 
> dwc3 part is NAKed by Marek, so patch set
> 
> not able to merge.
> 
>      If rockchip part can work without dwc3 change, you can send a 
> separate patch set for it, so that I can merge it.

Hi Kever,

The rockchip part works except the fact that sometimes the dwc3 gadget 
does not power up correctly (hence the three patches that fix the problem).
If you are fine to take the rockchip part I can resend it as a separate 
series. It would be useful for people to have the gadget devicetree and 
configs in upstream, and only the DWC3 part missing. Otherwise, this 
series will float until someone brings DWC3 up to date in U-boot as 
Marek requested. Let me know what do you think.

Thanks,
Eugen

> 
> 
> Thanks,
> 
> - Kever
> 
>>
>> Thanks,
>> Eugen