[PATCH v1 1/1] usb: host: tegra: implement dts based xcvr setup

Marek Vasut marex at denx.de
Sun Aug 20 04:23:14 CEST 2023


On 8/19/23 17:06, Svyatoslav Ryhel wrote:
> Some devices (like ASUS TF201) may not have fuse based xcvr setup
> which will result in not working USB line. To fix this situation
> allow xcvr setup to be performed from device tree values if
> nvidia,xcvr-setup-use-fuses is not defined.
> 
> Tested-by: Svyatoslav Ryhel <clamor95 at gmail.com> # ASUS TF201

I would hope so. Usually T-B tags are not added by the patch author, but 
that's a detail, and here it has the added model value, so keep it.

> Signed-off-by: Svyatoslav Ryhel <clamor95 at gmail.com>
> ---
>   drivers/usb/host/ehci-tegra.c | 53 +++++++++++++++++++++++++++++++----
>   1 file changed, 48 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
> index 2cf1625670..f24baf8f0c 100644
> --- a/drivers/usb/host/ehci-tegra.c
> +++ b/drivers/usb/host/ehci-tegra.c
> @@ -81,6 +81,8 @@ struct fdt_usb {
>   	enum periph_id periph_id;/* peripheral id */
>   	struct gpio_desc vbus_gpio;	/* GPIO for vbus enable */
>   	struct gpio_desc phy_reset_gpio; /* GPIO to reset ULPI phy */
> +	bool xcvr_setup_use_fuses; /* Indicates that the value is read from the on-chip fuses */
> +	u32 xcvr_setup;	/* Input of XCVR cell, HS driver output control */
>   };
>   
>   /*
> @@ -464,10 +466,21 @@ static int init_utmi_usb_controller(struct fdt_usb *config,
>   
>   		/* Recommended PHY settings for EYE diagram */
>   		val = readl(&usbctlr->utmip_xcvr_cfg0);
> -		clrsetbits_le32(&val, UTMIP_XCVR_SETUP_MASK,
> -				0x4 << UTMIP_XCVR_SETUP_SHIFT);
> -		clrsetbits_le32(&val, UTMIP_XCVR_SETUP_MSB_MASK,
> -				0x3 << UTMIP_XCVR_SETUP_MSB_SHIFT);
> +
> +		if (!config->xcvr_setup_use_fuses) {
> +			clrsetbits_le32(&val, UTMIP_XCVR_SETUP_MASK,
> +					config->xcvr_setup <<
> +					UTMIP_XCVR_SETUP_SHIFT);
> +			clrsetbits_le32(&val, UTMIP_XCVR_SETUP_MSB_MASK,
> +					config->xcvr_setup <<
> +					UTMIP_XCVR_SETUP_MSB_SHIFT);
> +		} else {
> +			clrsetbits_le32(&val, UTMIP_XCVR_SETUP_MASK,
> +					0x4 << UTMIP_XCVR_SETUP_SHIFT);

What is this hard-coded value ?

Why not place this value into config->xcvr_setup in e.g. probe() and 
drop this if/else statement ?

> +			clrsetbits_le32(&val, UTMIP_XCVR_SETUP_MSB_MASK,
> +					0x3 << UTMIP_XCVR_SETUP_MSB_SHIFT);
> +		}
> +
>   		clrsetbits_le32(&val, UTMIP_XCVR_HSSLEW_MSB_MASK,
>   				0x8 << UTMIP_XCVR_HSSLEW_MSB_SHIFT);
>   		writel(val, &usbctlr->utmip_xcvr_cfg0);
> @@ -522,7 +535,9 @@ static int init_utmi_usb_controller(struct fdt_usb *config,
>   	setbits_le32(&usbctlr->utmip_bat_chrg_cfg0, UTMIP_PD_CHRG);
>   
>   	clrbits_le32(&usbctlr->utmip_xcvr_cfg0, UTMIP_XCVR_LSBIAS_SE);
> -	setbits_le32(&usbctlr->utmip_spare_cfg0, FUSE_SETUP_SEL);
> +
> +	if (config->xcvr_setup_use_fuses)
> +		setbits_le32(&usbctlr->utmip_spare_cfg0, FUSE_SETUP_SEL);
>   
>   	/*
>   	 * Configure the UTMIP_IDLE_WAIT and UTMIP_ELASTIC_LIMIT
> @@ -843,6 +858,8 @@ static const struct ehci_ops tegra_ehci_ops = {
>   static int ehci_usb_of_to_plat(struct udevice *dev)
>   {
>   	struct fdt_usb *priv = dev_get_priv(dev);
> +	u32 usb_phy_phandle;
> +	ofnode usb_phy_node;
>   	int ret;
>   
>   	ret = fdt_decode_usb(dev, priv);
> @@ -851,6 +868,32 @@ static int ehci_usb_of_to_plat(struct udevice *dev)
>   
>   	priv->type = dev_get_driver_data(dev);
>   
> +	ret = ofnode_read_u32(dev_ofnode(dev), "nvidia,phy", &usb_phy_phandle);
> +	if (ret) {
> +		log_debug("%s: required usb phy node isn't provided\n", __func__);
> +		priv->xcvr_setup_use_fuses = true;
> +		return 0;
> +	}
> +
> +	usb_phy_node = ofnode_get_by_phandle(usb_phy_phandle);
> +	if (!ofnode_valid(usb_phy_node)) {
> +		log_err("%s: failed to find usb phy node\n", __func__);
> +		return -EINVAL;
> +	}

dev_read_phandle() should replace the above

> +	priv->xcvr_setup_use_fuses = ofnode_read_bool(
> +		usb_phy_node, "nvidia,xcvr-setup-use-fuses");

dev_read_bool()

> +	if (!priv->xcvr_setup_use_fuses) {
> +		ret = ofnode_read_u32(usb_phy_node, "nvidia,xcvr-setup",
> +				      &priv->xcvr_setup);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	log_debug("%s: xcvr_setup_use_fuses %d, xcvr_setup %d\n",
> +		  __func__, priv->xcvr_setup_use_fuses, priv->xcvr_setup);

Is xcvr_setup always initialized, e.g. in case 
priv->xcvr_setup_use_fuses=true ?


More information about the U-Boot mailing list