2024-03-07 03:04:01

by Wayne Chang

[permalink] [raw]
Subject: [PATCH v2 0/2] Fix incorrect USB3 phy parsing in tegra-xudc

The patch series introduces a new API for retrieving the port number of the PHY.
And then utilized this API to address an issue in the USB3 PHY retrieval logic.

Wayne Chang (2):
phy: tegra: xusb: Add API to retrieve the port number of phy
usb: gadget: tegra-xudc: Fix USB3 PHY retrieval logic

drivers/phy/tegra/xusb.c | 13 ++++++++++
drivers/usb/gadget/udc/tegra-xudc.c | 39 ++++++++++++++++++-----------
include/linux/phy/tegra/xusb.h | 1 +
3 files changed, 39 insertions(+), 14 deletions(-)


base-commit: 90d35da658da8cff0d4ecbb5113f5fac9d00eb72
--
2.25.1



2024-03-07 03:04:18

by Wayne Chang

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

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: [email protected]
Signed-off-by: Wayne Chang <[email protected]>
---
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;
--
2.25.1


2024-03-07 03:04:36

by Wayne Chang

[permalink] [raw]
Subject: [PATCH v2 1/2] phy: tegra: xusb: Add API to retrieve the port number of phy

This patch introduces a new API, tegra_xusb_padctl_get_port_number,
to the Tegra XUSB Pad Controller driver. This API is used to identify
the USB port that is associated with a given PHY.

The function takes a PHY pointer for either a USB2 PHY or USB3 PHY as input
and returns the corresponding port number. If the PHY pointer is invalid,
it returns -ENODEV.

Cc: [email protected]
Signed-off-by: Wayne Chang <[email protected]>
---
V1 -> V2:cc stable
drivers/phy/tegra/xusb.c | 13 +++++++++++++
include/linux/phy/tegra/xusb.h | 1 +
2 files changed, 14 insertions(+)

diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
index 142ebe0247cc..983a6e6173bd 100644
--- a/drivers/phy/tegra/xusb.c
+++ b/drivers/phy/tegra/xusb.c
@@ -1531,6 +1531,19 @@ int tegra_xusb_padctl_get_usb3_companion(struct tegra_xusb_padctl *padctl,
}
EXPORT_SYMBOL_GPL(tegra_xusb_padctl_get_usb3_companion);

+int tegra_xusb_padctl_get_port_number(struct phy *phy)
+{
+ struct tegra_xusb_lane *lane;
+
+ if (!phy)
+ return -ENODEV;
+
+ lane = phy_get_drvdata(phy);
+
+ return lane->index;
+}
+EXPORT_SYMBOL_GPL(tegra_xusb_padctl_get_port_number);
+
MODULE_AUTHOR("Thierry Reding <[email protected]>");
MODULE_DESCRIPTION("Tegra XUSB Pad Controller driver");
MODULE_LICENSE("GPL v2");
diff --git a/include/linux/phy/tegra/xusb.h b/include/linux/phy/tegra/xusb.h
index 70998e6dd6fd..6ca51e0080ec 100644
--- a/include/linux/phy/tegra/xusb.h
+++ b/include/linux/phy/tegra/xusb.h
@@ -26,6 +26,7 @@ void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy);
int tegra_phy_xusb_utmi_port_reset(struct phy *phy);
int tegra_xusb_padctl_get_usb3_companion(struct tegra_xusb_padctl *padctl,
unsigned int port);
+int tegra_xusb_padctl_get_port_number(struct phy *phy);
int tegra_xusb_padctl_enable_phy_sleepwalk(struct tegra_xusb_padctl *padctl, struct phy *phy,
enum usb_device_speed speed);
int tegra_xusb_padctl_disable_phy_sleepwalk(struct tegra_xusb_padctl *padctl, struct phy *phy);
--
2.25.1


2024-03-07 13:49:35

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] phy: tegra: xusb: Add API to retrieve the port number of phy


On 07/03/2024 03:03, Wayne Chang wrote:
> This patch introduces a new API, tegra_xusb_padctl_get_port_number,
> to the Tegra XUSB Pad Controller driver. This API is used to identify
> the USB port that is associated with a given PHY.
>
> The function takes a PHY pointer for either a USB2 PHY or USB3 PHY as input
> and returns the corresponding port number. If the PHY pointer is invalid,
> it returns -ENODEV.
>
> Cc: [email protected]
> Signed-off-by: Wayne Chang <[email protected]>
> ---
> V1 -> V2:cc stable
> drivers/phy/tegra/xusb.c | 13 +++++++++++++
> include/linux/phy/tegra/xusb.h | 1 +
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
> index 142ebe0247cc..983a6e6173bd 100644
> --- a/drivers/phy/tegra/xusb.c
> +++ b/drivers/phy/tegra/xusb.c
> @@ -1531,6 +1531,19 @@ int tegra_xusb_padctl_get_usb3_companion(struct tegra_xusb_padctl *padctl,
> }
> EXPORT_SYMBOL_GPL(tegra_xusb_padctl_get_usb3_companion);
>
> +int tegra_xusb_padctl_get_port_number(struct phy *phy)
> +{
> + struct tegra_xusb_lane *lane;
> +
> + if (!phy)
> + return -ENODEV;
> +
> + lane = phy_get_drvdata(phy);
> +
> + return lane->index;
> +}
> +EXPORT_SYMBOL_GPL(tegra_xusb_padctl_get_port_number);
> +
> MODULE_AUTHOR("Thierry Reding <[email protected]>");
> MODULE_DESCRIPTION("Tegra XUSB Pad Controller driver");
> MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/phy/tegra/xusb.h b/include/linux/phy/tegra/xusb.h
> index 70998e6dd6fd..6ca51e0080ec 100644
> --- a/include/linux/phy/tegra/xusb.h
> +++ b/include/linux/phy/tegra/xusb.h
> @@ -26,6 +26,7 @@ void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy);
> int tegra_phy_xusb_utmi_port_reset(struct phy *phy);
> int tegra_xusb_padctl_get_usb3_companion(struct tegra_xusb_padctl *padctl,
> unsigned int port);
> +int tegra_xusb_padctl_get_port_number(struct phy *phy);
> int tegra_xusb_padctl_enable_phy_sleepwalk(struct tegra_xusb_padctl *padctl, struct phy *phy,
> enum usb_device_speed speed);
> int tegra_xusb_padctl_disable_phy_sleepwalk(struct tegra_xusb_padctl *padctl, struct phy *phy);



Reviewed-by: Jon Hunter <[email protected]>
Tested-by: Jon Hunter <[email protected]>

Thanks!

Jon

--
nvpublic

2024-03-07 14:00:25

by Jon Hunter

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


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: [email protected]
> Signed-off-by: Wayne Chang <[email protected]>
> ---
> 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 <[email protected]>
Tested-by: Jon Hunter <[email protected]>

Thanks!

Jon

--
nvpublic

2024-04-05 16:59:24

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] phy: tegra: xusb: Add API to retrieve the port number of phy

On 07-03-24, 11:03, Wayne Chang wrote:
> This patch introduces a new API, tegra_xusb_padctl_get_port_number,
> to the Tegra XUSB Pad Controller driver. This API is used to identify
> the USB port that is associated with a given PHY.
>
> The function takes a PHY pointer for either a USB2 PHY or USB3 PHY as input
> and returns the corresponding port number. If the PHY pointer is invalid,
> it returns -ENODEV.

Acked-by: Vinod Koul <[email protected]>

--
~Vinod