Re: [PATCH v2 2/2] usb: gadget: tegra-xudc: Fix USB3 PHY retrieval logic

[Jon Hunter]

On 07/03/2024 03:03, Wayne Chang wrote:
> This commit resolves an issue in the tegra-xudc USB gadget driver that
> incorrectly fetched USB3 PHY instances. The problem stemmed from the
> assumption of a one-to-one correspondence between USB2 and USB3 PHY
> names and their association with physical USB ports in the device tree.
>
> Previously, the driver associated USB3 PHY names directly with the USB3
> instance number, leading to mismatches when mapping the physical USB
> ports. For instance, if using USB3-1 PHY, the driver expect the
> corresponding PHY name as 'usb3-1'. However, the physical USB ports in
> the device tree were designated as USB2-0 and USB3-0 as we only have
> one device controller, causing a misalignment.
>
> This commit rectifies the issue by adjusting the PHY naming logic.
> Now, the driver correctly correlates the USB2 and USB3 PHY instances,
> allowing the USB2-0 and USB3-1 PHYs to form a physical USB port pair
> while accurately reflecting their configuration in the device tree by
> naming them USB2-0 and USB3-0, respectively.
>
> The change ensures that the PHY and PHY names align appropriately,
> resolving the mismatch between physical USB ports and their associated
> names in the device tree.
>
> Fixes: b4e19931c98a ("usb: gadget: tegra-xudc: Support multiple device modes")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Wayne Chang <waynec@xxxxxxxxxx>
> ---
> V1 -> V2:no change
>   drivers/usb/gadget/udc/tegra-xudc.c | 39 ++++++++++++++++++-----------
>   1 file changed, 25 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/tegra-xudc.c b/drivers/usb/gadget/udc/tegra-xudc.c
> index cb85168fd00c..7aa46d426f31 100644
> --- a/drivers/usb/gadget/udc/tegra-xudc.c
> +++ b/drivers/usb/gadget/udc/tegra-xudc.c
> @@ -3491,8 +3491,8 @@ static void tegra_xudc_device_params_init(struct tegra_xudc *xudc)
>   
>   static int tegra_xudc_phy_get(struct tegra_xudc *xudc)
>   {
> -	int err = 0, usb3;
> -	unsigned int i;
> +	int err = 0, usb3_companion_port;
> +	unsigned int i, j;
>   
>   	xudc->utmi_phy = devm_kcalloc(xudc->dev, xudc->soc->num_phys,
>   					   sizeof(*xudc->utmi_phy), GFP_KERNEL);
> @@ -3520,7 +3520,7 @@ static int tegra_xudc_phy_get(struct tegra_xudc *xudc)
>   		if (IS_ERR(xudc->utmi_phy[i])) {
>   			err = PTR_ERR(xudc->utmi_phy[i]);
>   			dev_err_probe(xudc->dev, err,
> -				      "failed to get usb2-%d PHY\n", i);
> +				"failed to get PHY for phy-name usb2-%d\n", i);
>   			goto clean_up;
>   		} else if (xudc->utmi_phy[i]) {
>   			/* Get usb-phy, if utmi phy is available */
> @@ -3539,19 +3539,30 @@ static int tegra_xudc_phy_get(struct tegra_xudc *xudc)
>   		}
>   
>   		/* Get USB3 phy */
> -		usb3 = tegra_xusb_padctl_get_usb3_companion(xudc->padctl, i);
> -		if (usb3 < 0)
> +		usb3_companion_port = tegra_xusb_padctl_get_usb3_companion(xudc->padctl, i);
> +		if (usb3_companion_port < 0)
>   			continue;
>   
> -		snprintf(phy_name, sizeof(phy_name), "usb3-%d", usb3);
> -		xudc->usb3_phy[i] = devm_phy_optional_get(xudc->dev, phy_name);
> -		if (IS_ERR(xudc->usb3_phy[i])) {
> -			err = PTR_ERR(xudc->usb3_phy[i]);
> -			dev_err_probe(xudc->dev, err,
> -				      "failed to get usb3-%d PHY\n", usb3);
> -			goto clean_up;
> -		} else if (xudc->usb3_phy[i])
> -			dev_dbg(xudc->dev, "usb3-%d PHY registered", usb3);
> +		for (j = 0; j < xudc->soc->num_phys; j++) {
> +			snprintf(phy_name, sizeof(phy_name), "usb3-%d", j);
> +			xudc->usb3_phy[i] = devm_phy_optional_get(xudc->dev, phy_name);
> +			if (IS_ERR(xudc->usb3_phy[i])) {
> +				err = PTR_ERR(xudc->usb3_phy[i]);
> +				dev_err_probe(xudc->dev, err,
> +					"failed to get PHY for phy-name usb3-%d\n", j);
> +				goto clean_up;
> +			} else if (xudc->usb3_phy[i]) {
> +				int usb2_port =
> +					tegra_xusb_padctl_get_port_number(xudc->utmi_phy[i]);
> +				int usb3_port =
> +					tegra_xusb_padctl_get_port_number(xudc->usb3_phy[i]);
> +				if (usb3_port == usb3_companion_port) {
> +					dev_dbg(xudc->dev, "USB2 port %d is paired with USB3 port %d for device mode port %d\n",
> +					 usb2_port, usb3_port, i);
> +					break;
> +				}
> +			}
> +		}
>   	}
>   
>   	return err;


Reviewed-by: Jon Hunter <jonathanh@xxxxxxxxxx>
Tested-by: Jon Hunter <jonathanh@xxxxxxxxxx>

Thanks!

Jon

--
nvpublic