Hi Sebastian,
On 2024-08-03 18:09, Sebastian Reichel wrote:
> Hi Jonas,
>
> On Fri, Aug 02, 2024 at 11:04:05PM GMT, Jonas Karlman wrote:
>> Hi Sebastian,
>>
>> On 2024-08-02 19:59, Sebastian Reichel wrote:
>>> Since older U-Boot releases do not negotiate USB PD, the kernel
>>> DT may not enable the USB-C controller by default to avoid a
>>> regression. The plan is to upstream it with 'status = "fail";'
>>> instead. U-Boot should then mark it as 'status = "okay";' if
>>> it negotiated USB PD. Currently existing upstream kernel DTs do
>>> not yet have the USB-C controller at all, so we ignore any
>>> failures.
>>
>> I do not really understand why you need to upstream it this way, there
>> are plenty of other rk3588 boards with fcs,fusb302 already having status
>> "okay" that possible are affected by same USB PD issue that ROCK 5B
>> have.
>>
>> I guess you want to try and protect users that upgrade kernel DT and
>> ignore upgrading to a newer U-Boot, use vendor U-Boot or any other
>> bootloader from having possible issues with those USB PD power supplies
>> that have issues?
>
> Correct. Existing upstream U-Boot with new kernel would result in a
> boot loop. That's precisly why we need this U-Boot series. Since the
> Rock 5B is usually powered via USB-C and Radxa explicitly requests a
> USB-C PD power-supply almost everyone would be affected. IDK how
> other boards solve this considering U-Boot does not yet have a
> fusb302 driver. For me **all** of my power-supplies trigger a hard
> reset and thus a power-loss when booting a kernel with fusb302
> combined with an old U-Boot version not supporting fusb302.
>
> -- Sebastian
>
>>> Signed-off-by: Sebastian Reichel <sebastian.reic...@collabora.com>
>>> ---
>>> board/radxa/rock5b-rk3588/rock5b-rk3588.c | 11 +++++++++++
>>> 1 file changed, 11 insertions(+)
>>>
>>> diff --git a/board/radxa/rock5b-rk3588/rock5b-rk3588.c
>>> b/board/radxa/rock5b-rk3588/rock5b-rk3588.c
>>> index 1c17ae93c76c..4e926ebf2cb0 100644
>>> --- a/board/radxa/rock5b-rk3588/rock5b-rk3588.c
>>> +++ b/board/radxa/rock5b-rk3588/rock5b-rk3588.c
>>> @@ -3,6 +3,8 @@
>>> * Copyright (c) 2023-2024 Collabora Ltd.
>>> */
>>>
>>> +#include <fdtdec.h>
>>> +#include <fdt_support.h>
>>> #include <usb/tcpm.h>
>>>
>>> #ifdef CONFIG_MISC_INIT_R
>>> @@ -33,3 +35,12 @@ int misc_init_r(void)
>>> return 0;
>>> }
>>> #endif
>>> +
>>> +#ifdef CONFIG_OF_BOARD_SETUP
>>> +int ft_board_setup(void *blob, struct bd_info *bd)
>>> +{
>>> + if (IS_ENABLED(CONFIG_MISC_INIT_R))
>>
>> It make very little sense to check for this, it is implied for almost
>> all Rockchip boards since it is used in rockchip board common code
>> to setup serial and Ethernet mac addresses env vars.
As noted, please drop this check, MISC_INIT_R is implied for almost all
Rockchip targets and has nothing to do with when status should change
for fcs,fusb302.
Regards,
Jonas
>>
>> Regards,
>> Jonas
>>
>>> + fdt_status_okay_by_compatible(blob, "fcs,fusb302");
>>> + return 0;
>>> +}
>>> +#endif
>>
