Hi Patrice,

Thank you for the patch.

On jeu., janv. 16, 2025 at 09:17, Patrice Chotard <patrice.chot...@foss.st.com> 
wrote:

> Add STi glue logic to manage the DWC3 HC on STiH407
> SoC family. It configures the internal glue logic and
> syscfg registers.
>
> Signed-off-by: Patrice Chotard <patrice.chot...@foss.st.com>
> Cc: Marek Vasut <ma...@denx.de>
>
> ---
>
> Changes in v2:
>   - add dwc3-sti.c DWC3 wrapper as done for dwc3-am62.c
>
>  MAINTAINERS                         |   1 +
>  drivers/usb/dwc3/Kconfig            |   6 ++
>  drivers/usb/dwc3/Makefile           |   1 +
>  drivers/usb/dwc3/dwc3-generic-sti.c | 127 ++++++++++++++++++++++++++++
>  4 files changed, 135 insertions(+)
>  create mode 100644 drivers/usb/dwc3/dwc3-generic-sti.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8c6c0c2a4bc..5d7e251e601 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -656,6 +656,7 @@ F:        drivers/mmc/sti_sdhci.c
>  F:   drivers/reset/sti-reset.c
>  F:   drivers/serial/serial_sti_asc.c
>  F:   drivers/sysreset/sysreset_sti.c
> +F:   drivers/usb/host/dwc3-sti.c
>  F:   drivers/timer/arm_global_timer.c
>  F:   drivers/usb/host/dwc3-sti-glue.c
>  F:   include/dwc3-sti-glue.h
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 0100723a68b..7d58ae65fb6 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -87,6 +87,12 @@ config USB_DWC3_LAYERSCAPE
>         Host and Peripheral operation modes are supported. OTG is not
>         supported.
>  
> +config USB_DWC3_STI
> +     bool "STi USB wrapper"
> +     depends on DM_USB && USB_DWC3_GENERIC && SYSCON
> +     help
> +       Select this for STi Platforms.
> +
>  menu "PHY Subsystem"
>  
>  config USB_DWC3_PHY_OMAP
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index a085c9d4628..985206eafe4 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -15,3 +15,4 @@ obj-$(CONFIG_USB_DWC3_UNIPHIER)             += 
> dwc3-uniphier.o
>  obj-$(CONFIG_USB_DWC3_LAYERSCAPE)    += dwc3-layerscape.o
>  obj-$(CONFIG_USB_DWC3_PHY_OMAP)              += ti_usb_phy.o
>  obj-$(CONFIG_USB_DWC3_PHY_SAMSUNG)   += samsung_usb_phy.o
> +obj-$(CONFIG_USB_DWC3_STI)           += dwc3-generic-sti.o
> diff --git a/drivers/usb/dwc3/dwc3-generic-sti.c 
> b/drivers/usb/dwc3/dwc3-generic-sti.c
> new file mode 100644
> index 00000000000..e316e88d2f8
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3-generic-sti.c
> @@ -0,0 +1,127 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR BSD-3-Clause
> +/*
> + * STi specific glue layer for DWC3
> + *
> + * Copyright (C) 2025, STMicroelectronics - All Rights Reserved
> + */
> +
> +#include <reset.h>
> +#include <regmap.h>
> +#include <syscon.h>
> +#include <asm/io.h>
> +#include <dm/device.h>
> +#include <dm/read.h>
> +#include <linux/usb/otg.h>
> +#include "dwc3-generic.h"
> +
> +/* glue registers */
> +#define CLKRST_CTRL          0x00
> +#define AUX_CLK_EN           BIT(0)
> +#define SW_PIPEW_RESET_N     BIT(4)
> +#define EXT_CFG_RESET_N              BIT(8)
> +
> +#define XHCI_REVISION                BIT(12)
> +
> +#define USB2_VBUS_MNGMNT_SEL1        0x2C
> +#define USB2_VBUS_UTMIOTG    0x1
> +
> +#define SEL_OVERRIDE_VBUSVALID(n)    ((n) << 0)
> +#define SEL_OVERRIDE_POWERPRESENT(n) ((n) << 4)
> +#define SEL_OVERRIDE_BVALID(n)               ((n) << 8)
> +
> +/* Static DRD configuration */
> +#define USB3_CONTROL_MASK            0xf77
> +
> +#define USB3_DEVICE_NOT_HOST         BIT(0)
> +#define USB3_FORCE_VBUSVALID         BIT(1)
> +#define USB3_DELAY_VBUSVALID         BIT(2)
> +#define USB3_SEL_FORCE_OPMODE                BIT(4)
> +#define USB3_FORCE_OPMODE(n)         ((n) << 5)
> +#define USB3_SEL_FORCE_DPPULLDOWN2   BIT(8)
> +#define USB3_FORCE_DPPULLDOWN2               BIT(9)
> +#define USB3_SEL_FORCE_DMPULLDOWN2   BIT(10)
> +#define USB3_FORCE_DMPULLDOWN2               BIT(11)
> +
> +static void dwc3_stih407_glue_configure(struct udevice *dev, int index,
> +                                     enum usb_dr_mode mode)
> +{
> +     struct dwc3_glue_data *glue = dev_get_plat(dev);
> +     struct regmap *regmap;
> +     ulong syscfg_base;
> +     ulong syscfg_offset;
> +     ulong glue_base;
> +     int ret;
> +
> +     /* deassert both powerdown and softreset */
> +     ret = reset_deassert_bulk(&glue->resets);
> +     if (ret) {
> +             debug("reset_deassert_bulk error: %d\n", ret);

Maybe promote this to a warning message? debug() seems a little too low
priority for an error like this.

> +             return;
> +     }
> +
> +     regmap = syscon_regmap_lookup_by_phandle(dev, "st,syscfg");

syscon_regmap_lookup_by_phandle() can fail, in that case, error handling
should be done with IS_ERR(regmap). Can we add that, please?

Otherwise we might do PTR_ERR->ranges[0].start the line below.

> +
> +     syscfg_base = regmap->ranges[0].start;
> +     glue_base = dev_read_addr_index(dev, 0);
> +     syscfg_offset = dev_read_addr_index(dev, 1);
> +
> +     clrbits_le32(syscfg_base + syscfg_offset, USB3_CONTROL_MASK);
> +
> +     /* glue drd init */
> +     switch (mode) {
> +     case USB_DR_MODE_PERIPHERAL:
> +             clrbits_le32(syscfg_base + syscfg_offset,
> +                          USB3_DELAY_VBUSVALID | USB3_SEL_FORCE_OPMODE |
> +                          USB3_FORCE_OPMODE(0x3) | 
> USB3_SEL_FORCE_DPPULLDOWN2 |
> +                          USB3_FORCE_DPPULLDOWN2 | 
> USB3_SEL_FORCE_DMPULLDOWN2 |
> +                          USB3_FORCE_DMPULLDOWN2);
> +
> +             setbits_le32(syscfg_base + syscfg_offset,
> +                          USB3_DEVICE_NOT_HOST | USB3_FORCE_VBUSVALID);
> +             break;
> +
> +     case USB_DR_MODE_HOST:
> +             clrbits_le32(syscfg_base + syscfg_offset,
> +                          USB3_DEVICE_NOT_HOST | USB3_FORCE_VBUSVALID |
> +                          USB3_SEL_FORCE_OPMODE | USB3_FORCE_OPMODE(0x3) |
> +                          USB3_SEL_FORCE_DPPULLDOWN2 | 
> USB3_FORCE_DPPULLDOWN2 |
> +                          USB3_SEL_FORCE_DMPULLDOWN2 | 
> USB3_FORCE_DMPULLDOWN2);
> +
> +             setbits_le32(syscfg_base + syscfg_offset, USB3_DELAY_VBUSVALID);
> +             break;
> +
> +     default:
> +             debug("Unsupported mode of operation %d\n", mode);
> +             return;

Nitpick, I think that mode being an enum (with a finite number of values
possibles), we not necessarily have to check if the value is invalid.
But it's okay for me if this stays the same.

> +     }
> +
> +     /* glue init */
> +     setbits_le32(glue_base + CLKRST_CTRL, AUX_CLK_EN | EXT_CFG_RESET_N | 
> XHCI_REVISION);
> +     clrbits_le32(glue_base + CLKRST_CTRL, SW_PIPEW_RESET_N);
> +
> +     /* configure mux for vbus, powerpresent and bvalid signals */
> +     setbits_le32(glue_base + USB2_VBUS_MNGMNT_SEL1,
> +                  SEL_OVERRIDE_VBUSVALID(USB2_VBUS_UTMIOTG) |
> +                  SEL_OVERRIDE_POWERPRESENT(USB2_VBUS_UTMIOTG) |
> +                  SEL_OVERRIDE_BVALID(USB2_VBUS_UTMIOTG));
> +     setbits_le32(glue_base + CLKRST_CTRL, SW_PIPEW_RESET_N);
> +};
> +
> +struct dwc3_glue_ops stih407_ops = {
> +     .glue_configure = dwc3_stih407_glue_configure,
> +};
> +
> +static const struct udevice_id dwc3_sti_match[] = {
> +     { .compatible = "st,stih407-dwc3", .data = (ulong)&stih407_ops},
> +     { /* sentinel */ }
> +};
> +
> +U_BOOT_DRIVER(dwc3_sti_wrapper) = {
> +     .name = "dwc3-sti",
> +     .id = UCLASS_SIMPLE_BUS,
> +     .of_match = dwc3_sti_match,
> +     .bind = dwc3_glue_bind,
> +     .probe = dwc3_glue_probe,
> +     .remove = dwc3_glue_remove,
> +     .plat_auto = sizeof(struct dwc3_glue_data),
> +};
> -- 
> 2.25.1

Reply via email to