diff mbox series

[v7,4/5] usb: dwc3: dwc3-qcom: Enable tx-fifo-resize property by default

Message ID 1611895604-4496-5-git-send-email-wcheng@codeaurora.org
State New
Headers show
Series Re-introduce TX FIFO resize for larger EP bursting | expand

Commit Message

Wesley Cheng Jan. 29, 2021, 4:46 a.m. UTC
In order to take advantage of the TX fifo resizing logic, manually add
these properties to the DWC3 child node by default.  This will allow
the DWC3 gadget to resize the TX fifos for the IN endpoints, which
help with performance.

Signed-off-by: Wesley Cheng <wcheng@codeaurora.org>
---
 drivers/usb/dwc3/dwc3-qcom.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Bjorn Andersson Feb. 2, 2021, 4:23 p.m. UTC | #1
On Thu 28 Jan 22:46 CST 2021, Wesley Cheng wrote:

> In order to take advantage of the TX fifo resizing logic, manually add
> these properties to the DWC3 child node by default.  This will allow
> the DWC3 gadget to resize the TX fifos for the IN endpoints, which
> help with performance.
> 
> Signed-off-by: Wesley Cheng <wcheng@codeaurora.org>
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index d803ee9..4ea6be3 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -564,6 +564,7 @@ static int dwc3_qcom_clk_init(struct dwc3_qcom *qcom, int count)
>  
>  static const struct property_entry dwc3_qcom_acpi_properties[] = {
>  	PROPERTY_ENTRY_STRING("dr_mode", "host"),
> +	PROPERTY_ENTRY_BOOL("tx-fifo-resize"),

I checked the ACPI tables for Lenovo Miix 630, Yoga C630 and Flex 5G and
neither one has this property specified. So while we could just add this
here, it would have to be done in collaboration with the people who
actually define these. And as said before, I believe we want this to
always be enabled.

>  	{}
>  };
>  
> @@ -634,6 +635,7 @@ static int dwc3_qcom_of_register_core(struct platform_device *pdev)
>  	struct dwc3_qcom	*qcom = platform_get_drvdata(pdev);
>  	struct device_node	*np = pdev->dev.of_node, *dwc3_np;
>  	struct device		*dev = &pdev->dev;
> +	struct property		*prop;
>  	int			ret;
>  
>  	dwc3_np = of_get_child_by_name(np, "dwc3");
> @@ -642,6 +644,14 @@ static int dwc3_qcom_of_register_core(struct platform_device *pdev)
>  		return -ENODEV;
>  	}
>  
> +	prop = kzalloc(sizeof(*prop), GFP_KERNEL);
> +	if (prop) {
> +		prop->name = "tx-fifo-resize";
> +		ret = of_add_property(dwc3_np, prop);

Can't we come up with a way where the platform driver enables this on
the core driver without modifying DT?

Regards,
Bjorn

> +		if (ret < 0)
> +			dev_info(dev, "unable to add tx-fifo-resize prop\n");
> +	}
> +
>  	ret = of_platform_populate(np, NULL, NULL, dev);
>  	if (ret) {
>  		dev_err(dev, "failed to register dwc3 core - %d\n", ret);
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
Wesley Cheng Feb. 18, 2021, 12:49 a.m. UTC | #2
On 2/2/2021 8:23 AM, Bjorn Andersson wrote:
> On Thu 28 Jan 22:46 CST 2021, Wesley Cheng wrote:

> 

>> In order to take advantage of the TX fifo resizing logic, manually add

>> these properties to the DWC3 child node by default.  This will allow

>> the DWC3 gadget to resize the TX fifos for the IN endpoints, which

>> help with performance.

>>

>> Signed-off-by: Wesley Cheng <wcheng@codeaurora.org>

>> ---

>>  drivers/usb/dwc3/dwc3-qcom.c | 10 ++++++++++

>>  1 file changed, 10 insertions(+)

>>

>> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c

>> index d803ee9..4ea6be3 100644

>> --- a/drivers/usb/dwc3/dwc3-qcom.c

>> +++ b/drivers/usb/dwc3/dwc3-qcom.c

>> @@ -564,6 +564,7 @@ static int dwc3_qcom_clk_init(struct dwc3_qcom *qcom, int count)

>>  

>>  static const struct property_entry dwc3_qcom_acpi_properties[] = {

>>  	PROPERTY_ENTRY_STRING("dr_mode", "host"),

>> +	PROPERTY_ENTRY_BOOL("tx-fifo-resize"),

> 

> I checked the ACPI tables for Lenovo Miix 630, Yoga C630 and Flex 5G and

> neither one has this property specified. So while we could just add this

> here, it would have to be done in collaboration with the people who

> actually define these. And as said before, I believe we want this to

> always be enabled.

> 

>>  	{}

>>  };

>>  

>> @@ -634,6 +635,7 @@ static int dwc3_qcom_of_register_core(struct platform_device *pdev)

>>  	struct dwc3_qcom	*qcom = platform_get_drvdata(pdev);

>>  	struct device_node	*np = pdev->dev.of_node, *dwc3_np;

>>  	struct device		*dev = &pdev->dev;

>> +	struct property		*prop;

>>  	int			ret;

>>  

>>  	dwc3_np = of_get_child_by_name(np, "dwc3");

>> @@ -642,6 +644,14 @@ static int dwc3_qcom_of_register_core(struct platform_device *pdev)

>>  		return -ENODEV;

>>  	}

>>  

>> +	prop = kzalloc(sizeof(*prop), GFP_KERNEL);

>> +	if (prop) {

>> +		prop->name = "tx-fifo-resize";

>> +		ret = of_add_property(dwc3_np, prop);

> 

> Can't we come up with a way where the platform driver enables this on

> the core driver without modifying DT?

> 

> Regards,

> Bjorn


Hi Bjorn,

Sorry for the late response.  As you know, its a little difficult to
access the DWC3 core device during DWC3 qcom probe time, as the DWC3
core will likely return deferred probe due to the PHY devices not being
ready.

This is why I went with the approach to modify the DWC3 node here, so
that when the DWC3 core is eventually probed, it wouldn't miss this
property setting.  If I tried to set this dynamically, say in
dwc3_qcom_vbus_override() (with proper NULL checks), then I'd miss this
setting for the first enumeration, but if cable plug out/in logic is
present, the setting would kick in on subsequent cable events.

Thanks
Wesley Cheng

> 

>> +		if (ret < 0)

>> +			dev_info(dev, "unable to add tx-fifo-resize prop\n");

>> +	}

>> +

>>  	ret = of_platform_populate(np, NULL, NULL, dev);

>>  	if (ret) {

>>  		dev_err(dev, "failed to register dwc3 core - %d\n", ret);

>> -- 

>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,

>> a Linux Foundation Collaborative Project

>>


-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index d803ee9..4ea6be3 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -564,6 +564,7 @@  static int dwc3_qcom_clk_init(struct dwc3_qcom *qcom, int count)
 
 static const struct property_entry dwc3_qcom_acpi_properties[] = {
 	PROPERTY_ENTRY_STRING("dr_mode", "host"),
+	PROPERTY_ENTRY_BOOL("tx-fifo-resize"),
 	{}
 };
 
@@ -634,6 +635,7 @@  static int dwc3_qcom_of_register_core(struct platform_device *pdev)
 	struct dwc3_qcom	*qcom = platform_get_drvdata(pdev);
 	struct device_node	*np = pdev->dev.of_node, *dwc3_np;
 	struct device		*dev = &pdev->dev;
+	struct property		*prop;
 	int			ret;
 
 	dwc3_np = of_get_child_by_name(np, "dwc3");
@@ -642,6 +644,14 @@  static int dwc3_qcom_of_register_core(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	prop = kzalloc(sizeof(*prop), GFP_KERNEL);
+	if (prop) {
+		prop->name = "tx-fifo-resize";
+		ret = of_add_property(dwc3_np, prop);
+		if (ret < 0)
+			dev_info(dev, "unable to add tx-fifo-resize prop\n");
+	}
+
 	ret = of_platform_populate(np, NULL, NULL, dev);
 	if (ret) {
 		dev_err(dev, "failed to register dwc3 core - %d\n", ret);