diff mbox series

[6/8] usb: misc: eud: Add High-Speed Phy control for EUD operations

Message ID 20240730222439.3469-7-quic_eserrao@quicinc.com (mailing list archive)
State New
Headers show
Series Enable EUD on Qualcomm sm8450 SoC | expand

Commit Message

Elson Serrao July 30, 2024, 10:24 p.m. UTC
The Embedded USB Debugger(EUD) is a HS-USB on-chip hub to support the
debug and trace capabilities on Qualcomm devices. It is physically
present in between the usb connector and the usb controller. Being a
HS USB hub, it relies on HS Phy for its functionality. Add HS phy
support in the eud driver and control the phy during eud enable/disable
operations.

Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com>
---
 drivers/usb/misc/qcom_eud.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

Comments

Krzysztof Kozlowski July 31, 2024, 5:39 a.m. UTC | #1
On 31/07/2024 00:24, Elson Roy Serrao wrote:
> The Embedded USB Debugger(EUD) is a HS-USB on-chip hub to support the
> debug and trace capabilities on Qualcomm devices. It is physically
> present in between the usb connector and the usb controller. Being a
> HS USB hub, it relies on HS Phy for its functionality. Add HS phy
> support in the eud driver and control the phy during eud enable/disable
> operations.
> 

...
>  static ssize_t enable_show(struct device *dev,
> @@ -186,6 +216,11 @@ static int eud_probe(struct platform_device *pdev)
>  
>  	chip->dev = &pdev->dev;
>  
> +	chip->usb2_phy = devm_phy_get(chip->dev, "usb2-phy");
> +	if (IS_ERR(chip->usb2_phy))
> +		return dev_err_probe(chip->dev, PTR_ERR(chip->usb2_phy),
> +				     "no usb2 phy configured\n");

This nicely breaks all users.

NAK

Best regards,
Krzysztof
Elson Serrao July 31, 2024, 10:38 p.m. UTC | #2
On 7/30/2024 10:39 PM, Krzysztof Kozlowski wrote:
> On 31/07/2024 00:24, Elson Roy Serrao wrote:
>> The Embedded USB Debugger(EUD) is a HS-USB on-chip hub to support the
>> debug and trace capabilities on Qualcomm devices. It is physically
>> present in between the usb connector and the usb controller. Being a
>> HS USB hub, it relies on HS Phy for its functionality. Add HS phy
>> support in the eud driver and control the phy during eud enable/disable
>> operations.
>>
> 
> ...
>>  static ssize_t enable_show(struct device *dev,
>> @@ -186,6 +216,11 @@ static int eud_probe(struct platform_device *pdev)
>>  
>>  	chip->dev = &pdev->dev;
>>  
>> +	chip->usb2_phy = devm_phy_get(chip->dev, "usb2-phy");
>> +	if (IS_ERR(chip->usb2_phy))
>> +		return dev_err_probe(chip->dev, PTR_ERR(chip->usb2_phy),
>> +				     "no usb2 phy configured\n");
> 
> This nicely breaks all users.
> 
> NAK
> 

As per my comment in [patch 1/8], phy would be a required property and hence I will first modify
and enable EUD on the existing user (sc7280 SoC) and then extend this to other users.

Thanks
Elson
Krzysztof Kozlowski Aug. 1, 2024, 7:45 a.m. UTC | #3
On 01/08/2024 00:38, Elson Serrao wrote:
> 
> 
> On 7/30/2024 10:39 PM, Krzysztof Kozlowski wrote:
>> On 31/07/2024 00:24, Elson Roy Serrao wrote:
>>> The Embedded USB Debugger(EUD) is a HS-USB on-chip hub to support the
>>> debug and trace capabilities on Qualcomm devices. It is physically
>>> present in between the usb connector and the usb controller. Being a
>>> HS USB hub, it relies on HS Phy for its functionality. Add HS phy
>>> support in the eud driver and control the phy during eud enable/disable
>>> operations.
>>>
>>
>> ...
>>>  static ssize_t enable_show(struct device *dev,
>>> @@ -186,6 +216,11 @@ static int eud_probe(struct platform_device *pdev)
>>>  
>>>  	chip->dev = &pdev->dev;
>>>  
>>> +	chip->usb2_phy = devm_phy_get(chip->dev, "usb2-phy");
>>> +	if (IS_ERR(chip->usb2_phy))
>>> +		return dev_err_probe(chip->dev, PTR_ERR(chip->usb2_phy),
>>> +				     "no usb2 phy configured\n");
>>
>> This nicely breaks all users.
>>
>> NAK
>>
> 
> As per my comment in [patch 1/8], phy would be a required property and hence I will first modify
> and enable EUD on the existing user (sc7280 SoC) and then extend this to other users.

NAK, you break existing users without clear reason.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
index 26e9b8749d8a..3de7d465912c 100644
--- a/drivers/usb/misc/qcom_eud.c
+++ b/drivers/usb/misc/qcom_eud.c
@@ -11,6 +11,7 @@ 
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/sysfs.h>
@@ -33,6 +34,7 @@ 
 struct eud_chip {
 	struct device			*dev;
 	struct usb_role_switch		*role_sw;
+	struct phy			*usb2_phy;
 	void __iomem			*base;
 	void __iomem			*mode_mgr;
 	unsigned int			int_status;
@@ -41,8 +43,35 @@  struct eud_chip {
 	bool				usb_attached;
 };
 
+static int eud_phy_enable(struct eud_chip *chip)
+{
+	int ret;
+
+	ret = phy_init(chip->usb2_phy);
+	if (ret)
+		return ret;
+
+	ret = phy_power_on(chip->usb2_phy);
+	if (ret)
+		phy_exit(chip->usb2_phy);
+
+	return ret;
+}
+
+static void eud_phy_disable(struct eud_chip *chip)
+{
+	phy_power_off(chip->usb2_phy);
+	phy_exit(chip->usb2_phy);
+}
+
 static int enable_eud(struct eud_chip *priv)
 {
+	int ret;
+
+	ret = eud_phy_enable(priv);
+	if (ret)
+		return ret;
+
 	writel(EUD_ENABLE, priv->base + EUD_REG_CSR_EUD_EN);
 	writel(EUD_INT_VBUS | EUD_INT_SAFE_MODE,
 			priv->base + EUD_REG_INT1_EN_MASK);
@@ -55,6 +84,7 @@  static void disable_eud(struct eud_chip *priv)
 {
 	writel(0, priv->base + EUD_REG_CSR_EUD_EN);
 	writel(0, priv->mode_mgr + EUD_REG_EUD_EN2);
+	eud_phy_disable(priv);
 }
 
 static ssize_t enable_show(struct device *dev,
@@ -186,6 +216,11 @@  static int eud_probe(struct platform_device *pdev)
 
 	chip->dev = &pdev->dev;
 
+	chip->usb2_phy = devm_phy_get(chip->dev, "usb2-phy");
+	if (IS_ERR(chip->usb2_phy))
+		return dev_err_probe(chip->dev, PTR_ERR(chip->usb2_phy),
+				     "no usb2 phy configured\n");
+
 	chip->role_sw = usb_role_switch_get(&pdev->dev);
 	if (IS_ERR(chip->role_sw))
 		return dev_err_probe(chip->dev, PTR_ERR(chip->role_sw),