[PATCH v3 10/14] mach-snapdragon: fixup USB nodes
Caleb Connolly
caleb.connolly at linaro.org
Tue Mar 19 15:45:48 CET 2024
Hi Sumit,
On 19/03/2024 13:49, Sumit Garg wrote:
> Hi Caleb,
>
> On Tue, 19 Mar 2024 at 17:52, Caleb Connolly <caleb.connolly at linaro.org> wrote:
>>
>> We don't support USB super-speed in U-Boot yet, we lack the SS PHY
>> drivers, however from my testing even with a PHY driver there seem to be
>> other issues when talking to super-speed peripherals.
>>
>> In pursuit of maintaining upstream DT compatibility,
>
> I can understand the reasoning behind this but since we enable these
> fixups for every Qcom platform it may turn out to be counter
> productive. There can be embedded use-cases where bootup times have
> stringent requirements. Also, depending upon CPU speed/freq the add-on
> times due to these can be significant.
I have measured this on a few different boards:
On DB410c, the slowest board we currently support
* of_live_build took 7228us
* Fixing up USB nodes took 131us
* Fixing up power domains took 88us
On RB1 (the slowest new platform with a QCM2290 SoC)
* of_live_build took 2078us
* Fixing up USB nodes took 39us
* Fixing up power domains took 27us
The time taken initially to build the livetree is likely made back as it
is much much faster to query than the flat tree (as you can see, walking
every single node takes <100us on db410c). I took some rough
measurements of the boot-to-console time on sdm845 (see the "enable
livetree" patch) and basically concluded that the delta between live and
flat trees is within the margin of error.
If a specific board for whatever reason wants to avoid using OF_LIVE
then I'm fine with that, db410c isn't using it for example.
>
> I would suggest we rather solve these USB issues either via a common
> Qcom *-u-boot.dtsi file which can be included for every board.
>
> -Sumit
>
>> and simplifying
>> porting for new devices, let's implement the DT fixups necessary to
>> configure USB in high-speed only mode at runtime. The pattern is
>> identical for all Qualcomm boards that use the Synaptics DWC3
>> controller:
>>
>> * Add an additional property on the Qualcomm wrapper node
>> * Remove the super-speed phy phandle and phy-name entries.
>>
>> Signed-off-by: Caleb Connolly <caleb.connolly at linaro.org>
>> ---
>> arch/arm/mach-snapdragon/Makefile | 1 +
>> arch/arm/mach-snapdragon/board.c | 3 +
>> arch/arm/mach-snapdragon/of_fixup.c | 115 +++++++++++++++++++++++++++++++++++
>> arch/arm/mach-snapdragon/qcom-priv.h | 19 ++++++
>> 4 files changed, 138 insertions(+)
>>
>> diff --git a/arch/arm/mach-snapdragon/Makefile b/arch/arm/mach-snapdragon/Makefile
>> index 857171e593da..7a4495c8108f 100644
>> --- a/arch/arm/mach-snapdragon/Makefile
>> +++ b/arch/arm/mach-snapdragon/Makefile
>> @@ -2,4 +2,5 @@
>> #
>> # (C) Copyright 2015 Mateusz Kulikowski <mateusz.kulikowski at gmail.com>
>>
>> obj-y += board.o
>> +obj-$(CONFIG_OF_LIVE) += of_fixup.o
>> diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c
>> index 6f762fc948bf..65e4c61e866a 100644
>> --- a/arch/arm/mach-snapdragon/board.c
>> +++ b/arch/arm/mach-snapdragon/board.c
>> @@ -27,8 +27,10 @@
>> #include <fdt_support.h>
>> #include <usb.h>
>> #include <sort.h>
>>
>> +#include "qcom-priv.h"
>> +
>> DECLARE_GLOBAL_DATA_PTR;
>>
>> static struct mm_region rbx_mem_map[CONFIG_NR_DRAM_BANKS + 2] = { { 0 } };
>>
>> @@ -159,8 +161,9 @@ void __weak qcom_board_init(void)
>>
>> int board_init(void)
>> {
>> show_psci_version();
>> + qcom_of_fixup_nodes();
>> qcom_board_init();
>> return 0;
>> }
>>
>> diff --git a/arch/arm/mach-snapdragon/of_fixup.c b/arch/arm/mach-snapdragon/of_fixup.c
>> new file mode 100644
>> index 000000000000..6810c8617cc0
>> --- /dev/null
>> +++ b/arch/arm/mach-snapdragon/of_fixup.c
>> @@ -0,0 +1,115 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * OF_LIVE devicetree fixup.
>> + *
>> + * This file implements runtime fixups for Qualcomm DT to improve
>> + * compatibility with U-Boot. This includes adjusting the USB nodes
>> + * to only use USB high-speed, as well as remapping volume buttons
>> + * to behave as up/down for navigating U-Boot.
>> + *
>> + * We use OF_LIVE for this rather than early FDT fixup for a couple
>> + * of reasons: it has a much nicer API, is most likely more efficient,
>> + * and our changes are only applied to U-Boot. This allows us to use a
>> + * DT designed for Linux, run U-Boot with a modified version, and then
>> + * boot Linux with the original FDT.
>> + *
>> + * Copyright (c) 2024 Linaro Ltd.
>> + * Author: Caleb Connolly <caleb.connolly at linaro.org>
>> + */
>> +
>> +#include <dt-bindings/input/linux-event-codes.h>
>> +#include <dm/of_access.h>
>> +#include <dm/of.h>
>> +#include <fdt_support.h>
>> +#include <linux/errno.h>
>> +
>> +/* U-Boot only supports USB high-speed mode on Qualcomm platforms with DWC3
>> + * USB controllers. Rather than requiring source level DT changes, we fix up
>> + * DT here. This improves compatibility with upstream DT and simplifies the
>> + * porting process for new devices.
>> + */
>> +static int fixup_qcom_dwc3(struct device_node *glue_np)
>> +{
>> + struct device_node *dwc3;
>> + int ret, len, hsphy_idx = 1;
>> + const __be32 *phandles;
>> + const char *second_phy_name;
>> +
>> + debug("Fixing up %s\n", glue_np->name);
>> +
>> + /* Tell the glue driver to configure the wrapper for high-speed only operation */
>> + ret = of_write_prop(glue_np, "qcom,select-utmi-as-pipe-clk", 0, NULL);
>> + if (ret) {
>> + log_err("Failed to add property 'qcom,select-utmi-as-pipe-clk': %d\n", ret);
>> + return ret;
>> + }
>> +
>> + /* Find the DWC3 node itself */
>> + dwc3 = of_find_compatible_node(glue_np, NULL, "snps,dwc3");
>> + if (!dwc3) {
>> + log_err("Failed to find dwc3 node\n");
>> + return -ENOENT;
>> + }
>> +
>> + phandles = of_get_property(dwc3, "phys", &len);
>> + len /= sizeof(*phandles);
>> + if (len == 1) {
>> + log_debug("Only one phy, not a superspeed controller\n");
>> + return 0;
>> + }
>> +
>> + /* Figure out if the superspeed phy is present and if so then which phy is it? */
>> + ret = of_property_read_string_index(dwc3, "phy-names", 1, &second_phy_name);
>> + if (ret == -ENODATA) {
>> + log_debug("Only one phy, not a super-speed controller\n");
>> + return 0;
>> + } else if (ret) {
>> + log_err("Failed to read second phy name: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + if (!strncmp("usb3-phy", second_phy_name, strlen("usb3-phy"))) {
>> + log_debug("Second phy isn't superspeed (is '%s') assuming first phy is SS\n",
>> + second_phy_name);
>> + hsphy_idx = 0;
>> + }
>> +
>> + /* Overwrite the "phys" property to only contain the high-speed phy */
>> + ret = of_write_prop(dwc3, "phys", sizeof(*phandles), phandles + hsphy_idx);
>> + if (ret) {
>> + log_err("Failed to overwrite 'phys' property: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + /* Overwrite "phy-names" to only contain a single entry */
>> + ret = of_write_prop(dwc3, "phy-names", strlen("usb2-phy"), "usb2-phy");
>> + if (ret) {
>> + log_err("Failed to overwrite 'phy-names' property: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ret = of_write_prop(dwc3, "maximum-speed", strlen("high-speed"), "high-speed");
>> + if (ret) {
>> + log_err("Failed to set 'maximum-speed' property: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void fixup_usb_nodes(void)
>> +{
>> + struct device_node *glue_np = NULL;
>> + int ret;
>> +
>> + while ((glue_np = of_find_compatible_node(glue_np, NULL, "qcom,dwc3"))) {
>> + ret = fixup_qcom_dwc3(glue_np);
>> + if (ret)
>> + log_warning("Failed to fixup node %s: %d\n", glue_np->name, ret);
>> + }
>> +}
>> +
>> +void qcom_of_fixup_nodes(void)
>> +{
>> + fixup_usb_nodes();
>> +}
>> diff --git a/arch/arm/mach-snapdragon/qcom-priv.h b/arch/arm/mach-snapdragon/qcom-priv.h
>> new file mode 100644
>> index 000000000000..d18fd1883b7f
>> --- /dev/null
>> +++ b/arch/arm/mach-snapdragon/qcom-priv.h
>> @@ -0,0 +1,19 @@
>> +
>> +#ifndef __QCOM_PRIV_H__
>> +#define __QCOM_PRIV_H__
>> +
>> +#if CONFIG_IS_ENABLED(OF_LIVE)
>> +/**
>> + * qcom_of_fixup_nodes() - Fixup Qualcomm DT nodes
>> + *
>> + * Adjusts nodes in the live tree to improve compatibility with U-Boot.
>> + */
>> +void qcom_of_fixup_nodes(void);
>> +#else
>> +static inline void qcom_of_fixup_nodes(void)
>> +{
>> + log_debug("Unable to dynamically fixup USB nodes, please enable CONFIG_OF_LIVE\n");
>> +}
>> +#endif /* OF_LIVE */
>> +
>> +#endif /* __QCOM_PRIV_H__ */
>>
>> --
>> 2.44.0
>>
--
// Caleb (they/them)
More information about the U-Boot
mailing list