[PATCH v3 3/7] board: rock5b-rk3588: add USB-C controller support

[Jonas Karlman]

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.reichel at 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 at 22", &dev);
>>> +	if (ret) {
>>> +		printf("Failed to probe Type-C controller\n");
>>> +		return 0;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +#endif
>>