[PATCH v3 10/14] mach-snapdragon: fixup USB nodes

Sumit Garg sumit.garg at linaro.org
Tue Mar 19 14:49:25 CET 2024


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 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
>


More information about the U-Boot mailing list