Hi Sebastian,
On 2024-08-03 18:17, Sebastian Reichel wrote:
> Hi Jonas,
>
> On Fri, Aug 02, 2024 at 10:42:56PM GMT, Jonas Karlman wrote:
>> On 2024-08-02 19:59, Sebastian Reichel wrote:
>>> Enable support for the fusb302 USB Type-C controller.
>>>
>>> This will do early USB PD (power deliver) negotiation, which must happen
>>> within 5 seconds after the USB-C connector has plugged in according to
>>> the specification. It takes almost 5 seconds to go through the bootchain
>>> on Rock 5B and jump to the operating system. When the Linux initializes
>>> the fusb302 usually 20-30 seconds have gone since the device has been
>>> plugged, which is far too late. The USB PD power source reacts with a
>>> hard reset, which disables VBUS for some time. This is not a problem for
>>> a battery driven device, but Rock 5B will loose its power-supply and
>>> reset. By initializing PD in U-Boot, this can be avoided.
>>>
>>> Signed-off-by: Sebastian Reichel <sebastian.reic...@collabora.com>
>>> ---
>>> board/radxa/rock5b-rk3588/Makefile | 6 ++++
>>> board/radxa/rock5b-rk3588/rock5b-rk3588.c | 35 +++++++++++++++++++++++
>>> 2 files changed, 41 insertions(+)
>>> create mode 100644 board/radxa/rock5b-rk3588/Makefile
>>> create mode 100644 board/radxa/rock5b-rk3588/rock5b-rk3588.c
>>>
>>> diff --git a/board/radxa/rock5b-rk3588/Makefile
>>> b/board/radxa/rock5b-rk3588/Makefile
>>> new file mode 100644
>>> index 000000000000..95d813596da4
>>> --- /dev/null
>>> +++ b/board/radxa/rock5b-rk3588/Makefile
>>> @@ -0,0 +1,6 @@
>>> +# SPDX-License-Identifier: GPL-2.0+
>>> +#
>>> +# Copyright (c) 2022 Collabora Ltd.
>>> +#
>>> +
>>> +obj-y += rock5b-rk3588.o
>>> diff --git a/board/radxa/rock5b-rk3588/rock5b-rk3588.c
>>> b/board/radxa/rock5b-rk3588/rock5b-rk3588.c
>>> new file mode 100644
>>> index 000000000000..1c17ae93c76c
>>> --- /dev/null
>>> +++ b/board/radxa/rock5b-rk3588/rock5b-rk3588.c
>>> @@ -0,0 +1,35 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * Copyright (c) 2023-2024 Collabora Ltd.
>>> + */
>>> +
>>> +#include <usb/tcpm.h>
>>> +
>>> +#ifdef CONFIG_MISC_INIT_R
>>> +int misc_init_r(void)
>>
>> You should not override misc_init_r() here as it will override the
>> rockchip board common misc_init_r() function that read cpuid, set serial
>> and ethernet mac address env vars.
This needs to be addressed if you must add this code.
>>
>> I would suggest you instead of tcpm_get() add something similar to
>>
>> int tcpm_probe_all()
>> {
>> struct udevice *dev;
>> struct uclass *uc;
>> int ret;
>>
>> ret = uclass_get(UCLASS_TCPM, &uc);
>> if (ret)
>> return ret;
>>
>> for (ret = uclass_first_device_check(UCLASS_TCPM, &dev);
>> dev;
>> ret = uclass_next_device_check(&dev)) {
>> if (ret)
>> printf("Failed to probe Type-C controller '%s'
>> (ret=%d)\n",
>> dev->name, ret);
>> }
>>
>> return 0;
>> }
>>
>> or if we do not care about the error message this could use
>>
>> uclass_probe_all(UCLASS_TCPM);
>>
>> and call it from the rockchip board common code in mach-rockchip/board.c
>> directly after the call to regulators_enable_boot_on() in board_init().
>>
>> Alternatively you could call following in a tcpm_post_bind()
>>
>> dev_or_flags(dev, DM_FLAG_PROBE_AFTER_BIND);
>>
>> to automatically probe the device as soon as possible after all devices
>> have been bind, this is how the rockchip-io-domain driver does it.
>>
>> Otherwise we need to add custom board code for each board using USB PD,
>> something to avoid.
>
> All of your suggestions result in any USB-PD controllers being
> probed automatically. I intentionally did not go that way, since
> probing can be quite slow depending on the remote side. For most
> power-supplies it seems to be quite fast, but I've seen a boot
> with one of the supplies being delayed by 2-3 seconds. Seems not
> nice to add this penality for a device not depending on it.
Since you add a cmd you could possible just add a PREBOOT to run that
cmd on boot.
We should try to avoid having to add new code for each board that needs
to negotiate USB PD.
For a more elaborate solution maybe a u-boot specific prop could be
added to the config node, similar to sysreset-gpio,
see doc/device-tree-bindings/config.txt.
Regards,
Jonas
>
> Greetings,
>
> -- Sebastian
>
>>
>> Regards,
>> Jonas
>>
>>> +{
>>> + struct udevice *dev;
>>> + int ret;
>>> +
>>> + /*
>>> + * This will do early USB PD (power deliver) negotiation, which must
>>> + * happen within 5 seconds after the USB-C connector has plugged in
>>> + * according to the specification. It takes almost 5 seconds to go
>>> + * through the bootchain on Rock 5B and jump to the operating system.
>>> + * When the Linux initializes the fusb302 usually 20-30 seconds have
>>> + * gone since the device has been plugged, which is far too late.
>>> + *
>>> + * The USB PD power source reacts with a hard reset, which disables
>>> + * VBUS for some time. This is not a problem for a battery driven
>>> + * device, but Rock 5B will loose its power-supply and reset. By
>>> + * initializing PD in U-Boot, this can be avoided.
>>> + */
>>> + ret = tcpm_get("usb-typec@22", &dev);
>>> + if (ret) {
>>> + printf("Failed to probe Type-C controller\n");
>>> + return 0;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +#endif
>>
