This path series aims to add interconnect support in
dwc3-qcom driver on SDM845 and SC7180 SoCs.
Changes from v5 -> v6
> [PATCH 1/4] Addressed comments from Rob.
> [PATCH 2/4] Fixed review comments from Matthias in DWC3 driver.
> [PATCH 3/4] Ignoring 80 char limit in defining interconnect paths.
> Added [PATCH 4/4] in this series. Adding interconnect nodes for SC7180.
Depends on patch https://patchwork.kernel.org/patch/11417989/.
Changes from v4 -> v5
> [PATCH 1/3] Added the interconnect properties in yaml. This patch depends
on series https://patchwork.kernel.org/cover/11372641/.
> [PATCH 2/3] Fixed review comments from Matthias in DWC3 driver.
> [PATCH 3/3] Modified as per the new interconnect nodes in sdm845. Depends
on series https://patchwork.kernel.org/cover/11372211/.
Changes from v3 -> v4
> Fixed review comments from Matthias
> [PATCH 1/3] and [PATCH 3/3] remains unchanged
Changes from v2 -> v3
> Fixed review comments from Matthias and Manu
> changed the functions prefix from usb_* to dwc3_qcom_*
Changes since V1:
> Comments by Georgi Djakov on "[PATCH 2/3]" addressed
> [PATCH 1/3] and [PATCH 3/3] remains unchanged
Sandeep Maheswaram (4):
dt-bindings: usb: qcom,dwc3: Introduce interconnect properties for
Qualcomm DWC3 driver
usb: dwc3: qcom: Add interconnect support in dwc3 driver
arm64: dts: qcom: sdm845: Add interconnect properties for USB
arm64: dts: qcom: sc7180: Add interconnect properties for USB
.../devicetree/bindings/usb/qcom,dwc3.yaml | 8 ++
arch/arm64/boot/dts/qcom/sc7180.dtsi | 4 +
arch/arm64/boot/dts/qcom/sdm845.dtsi | 8 ++
drivers/usb/dwc3/dwc3-qcom.c | 128 ++++++++++++++++++++-
4 files changed, 146 insertions(+), 2 deletions(-)
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Add interconnect support in dwc3-qcom driver to vote for bus
bandwidth.
This requires for two different paths - from USB master to
DDR slave. The other is from APPS master to USB slave.
Signed-off-by: Sandeep Maheswaram <[email protected]>
Signed-off-by: Chandana Kishori Chiluveru <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
---
drivers/usb/dwc3/dwc3-qcom.c | 128 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 126 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 1dfd024..7e85fe6 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -13,6 +13,7 @@
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/extcon.h>
+#include <linux/interconnect.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/phy/phy.h>
@@ -43,6 +44,14 @@
#define SDM845_QSCRATCH_SIZE 0x400
#define SDM845_DWC3_CORE_SIZE 0xcd00
+/* Interconnect path bandwidths in MBps */
+#define USB_MEMORY_AVG_HS_BW MBps_to_icc(240)
+#define USB_MEMORY_PEAK_HS_BW MBps_to_icc(700)
+#define USB_MEMORY_AVG_SS_BW MBps_to_icc(1000)
+#define USB_MEMORY_PEAK_SS_BW MBps_to_icc(2500)
+#define APPS_USB_AVG_BW 0
+#define APPS_USB_PEAK_BW MBps_to_icc(40)
+
struct dwc3_acpi_pdata {
u32 qscratch_base_offset;
u32 qscratch_base_size;
@@ -76,8 +85,13 @@ struct dwc3_qcom {
enum usb_dr_mode mode;
bool is_suspended;
bool pm_suspended;
+ struct icc_path *usb_ddr_icc_path;
+ struct icc_path *apps_usb_icc_path;
};
+static int dwc3_qcom_interconnect_enable(struct dwc3_qcom *qcom);
+static int dwc3_qcom_interconnect_disable(struct dwc3_qcom *qcom);
+
static inline void dwc3_qcom_setbits(void __iomem *base, u32 offset, u32 val)
{
u32 reg;
@@ -239,7 +253,7 @@ static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom)
static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
{
u32 val;
- int i;
+ int i, ret;
if (qcom->is_suspended)
return 0;
@@ -251,6 +265,10 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
for (i = qcom->num_clocks - 1; i >= 0; i--)
clk_disable_unprepare(qcom->clks[i]);
+ ret = dwc3_qcom_interconnect_disable(qcom);
+ if (ret)
+ dev_warn(qcom->dev, "failed to disable interconnect %d\n", ret);
+
qcom->is_suspended = true;
dwc3_qcom_enable_interrupts(qcom);
@@ -276,6 +294,10 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom)
}
}
+ ret = dwc3_qcom_interconnect_enable(qcom);
+ if (ret)
+ dev_warn(qcom->dev, "failed to enable interconnect %d\n", ret);
+
/* Clear existing events from PHY related to L2 in/out */
dwc3_qcom_setbits(qcom->qscratch_base, PWR_EVNT_IRQ_STAT_REG,
PWR_EVNT_LPM_IN_L2_MASK | PWR_EVNT_LPM_OUT_L2_MASK);
@@ -285,6 +307,101 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom)
return 0;
}
+
+/**
+ * dwc3_qcom_interconnect_init() - Get interconnect path handles
+ * @qcom: Pointer to the concerned usb core.
+ *
+ */
+static int dwc3_qcom_interconnect_init(struct dwc3_qcom *qcom)
+{
+ struct device *dev = qcom->dev;
+ int ret;
+
+ if (!device_is_bound(&qcom->dwc3->dev))
+ return -EPROBE_DEFER;
+
+ qcom->usb_ddr_icc_path = of_icc_get(dev, "usb-ddr");
+ if (IS_ERR(qcom->usb_ddr_icc_path)) {
+ dev_err(dev, "Error: (%ld) failed getting usb-ddr path\n",
+ PTR_ERR(qcom->usb_ddr_icc_path));
+ return PTR_ERR(qcom->usb_ddr_icc_path);
+ }
+
+ qcom->apps_usb_icc_path = of_icc_get(dev, "apps-usb");
+ if (IS_ERR(qcom->apps_usb_icc_path)) {
+ dev_err(dev, "Error: (%ld) failed getting apps-usb path\n",
+ PTR_ERR(qcom->apps_usb_icc_path));
+ return PTR_ERR(qcom->apps_usb_icc_path);
+ }
+
+ ret = dwc3_qcom_interconnect_enable(qcom);
+ if (ret) {
+ dev_err(dev, "failed to enable interconnect %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+/**
+ * dwc3_qcom_interconnect_exit() - Release interconnect path handles
+ * @qcom: Pointer to the concerned usb core.
+ *
+ * This function is used to release interconnect path handle.
+ */
+static void dwc3_qcom_interconnect_exit(struct dwc3_qcom *qcom)
+{
+ icc_put(qcom->usb_ddr_icc_path);
+ icc_put(qcom->apps_usb_icc_path);
+}
+
+/* Currently we only use bandwidth level, so just "enable" interconnects */
+static int dwc3_qcom_interconnect_enable(struct dwc3_qcom *qcom)
+{
+ struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
+ int ret;
+
+ if (dwc->maximum_speed == USB_SPEED_SUPER)
+ ret = icc_set_bw(qcom->usb_ddr_icc_path,
+ USB_MEMORY_AVG_SS_BW, USB_MEMORY_PEAK_SS_BW);
+ else
+ ret = icc_set_bw(qcom->usb_ddr_icc_path,
+ USB_MEMORY_AVG_HS_BW, USB_MEMORY_PEAK_HS_BW);
+
+ if (ret)
+ return ret;
+
+ ret = icc_set_bw(qcom->apps_usb_icc_path,
+ APPS_USB_AVG_BW, APPS_USB_PEAK_BW);
+ if (ret)
+ icc_set_bw(qcom->usb_ddr_icc_path, 0, 0);
+
+ return ret;
+}
+
+/* To disable an interconnect, we just set its bandwidth to 0 */
+static int dwc3_qcom_interconnect_disable(struct dwc3_qcom *qcom)
+{
+ int ret;
+
+ ret = icc_set_bw(qcom->usb_ddr_icc_path, 0, 0);
+ if (ret)
+ return ret;
+
+ ret = icc_set_bw(qcom->apps_usb_icc_path, 0, 0);
+ if (ret)
+ goto err_reenable_memory_path;
+
+ return 0;
+
+ /* Re-enable things in the event of an error */
+err_reenable_memory_path:
+ ret = dwc3_qcom_interconnect_enable(qcom);
+
+ return ret;
+}
+
static irqreturn_t qcom_dwc3_resume_irq(int irq, void *data)
{
struct dwc3_qcom *qcom = data;
@@ -648,6 +765,10 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
goto depopulate;
}
+ ret = dwc3_qcom_interconnect_init(qcom);
+ if (ret)
+ goto depopulate;
+
qcom->mode = usb_get_dr_mode(&qcom->dwc3->dev);
/* enable vbus override for device mode */
@@ -657,7 +778,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
/* register extcon to override sw_vbus on Vbus change later */
ret = dwc3_qcom_register_extcon(qcom);
if (ret)
- goto depopulate;
+ goto interconnect_exit;
device_init_wakeup(&pdev->dev, 1);
qcom->is_suspended = false;
@@ -667,6 +788,8 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
return 0;
+interconnect_exit:
+ dwc3_qcom_interconnect_exit(qcom);
depopulate:
if (np)
of_platform_depopulate(&pdev->dev);
@@ -697,6 +820,7 @@ static int dwc3_qcom_remove(struct platform_device *pdev)
}
qcom->num_clocks = 0;
+ dwc3_qcom_interconnect_exit(qcom);
reset_control_assert(qcom->resets);
pm_runtime_allow(dev);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Populate USB DT nodes with interconnect properties.
Signed-off-by: Sandeep Maheswaram <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7180.dtsi | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 998f101..bd0d3a7 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -1447,6 +1447,10 @@
resets = <&gcc GCC_USB30_PRIM_BCR>;
+ interconnects = <&aggre2_noc MASTER_USB3 &mc_virt SLAVE_EBI1>,
+ <&gem_noc MASTER_APPSS_PROC &config_noc SLAVE_USB3>;
+ interconnect-names = "usb-ddr", "apps-usb";
+
usb_1_dwc3: dwc3@a600000 {
compatible = "snps,dwc3";
reg = <0 0x0a600000 0 0xe000>;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Populate USB DT nodes with interconnect properties.
Signed-off-by: Sandeep Maheswaram <[email protected]>
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 8f926b5..860d5c2 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -3097,6 +3097,10 @@
resets = <&gcc GCC_USB30_PRIM_BCR>;
+ interconnects = <&aggre2_noc MASTER_USB3_0 &mem_noc SLAVE_EBI1>,
+ <&gladiator_noc MASTER_APPSS_PROC &config_noc SLAVE_USB3_0>;
+ interconnect-names = "usb-ddr", "apps-usb";
+
usb_1_dwc3: dwc3@a600000 {
compatible = "snps,dwc3";
reg = <0 0x0a600000 0 0xcd00>;
@@ -3141,6 +3145,10 @@
resets = <&gcc GCC_USB30_SEC_BCR>;
+ interconnects = <&aggre2_noc MASTER_USB3_1 &mem_noc SLAVE_EBI1>,
+ <&gladiator_noc MASTER_APPSS_PROC &config_noc SLAVE_USB3_1>;
+ interconnect-names = "usb-ddr", "apps-usb";
+
usb_2_dwc3: dwc3@a800000 {
compatible = "snps,dwc3";
reg = <0 0x0a800000 0 0xcd00>;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Hi,
On Fri, Mar 27, 2020 at 03:13:21PM +0530, Sandeep Maheswaram wrote:
> Add interconnect support in dwc3-qcom driver to vote for bus
> bandwidth.
>
> This requires for two different paths - from USB master to
> DDR slave. The other is from APPS master to USB slave.
>
> Signed-off-by: Sandeep Maheswaram <[email protected]>
> Signed-off-by: Chandana Kishori Chiluveru <[email protected]>
> Reviewed-by: Matthias Kaehlcke <[email protected]>
> ---
> drivers/usb/dwc3/dwc3-qcom.c | 128 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 126 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index 1dfd024..7e85fe6 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
>
> ...
>
> +/* To disable an interconnect, we just set its bandwidth to 0 */
> +static int dwc3_qcom_interconnect_disable(struct dwc3_qcom *qcom)
> +{
> + int ret;
> +
> + ret = icc_set_bw(qcom->usb_ddr_icc_path, 0, 0);
> + if (ret)
> + return ret;
> +
> + ret = icc_set_bw(qcom->apps_usb_icc_path, 0, 0);
> + if (ret)
> + goto err_reenable_memory_path;
> +
> + return 0;
> +
> + /* Re-enable things in the event of an error */
> +err_reenable_memory_path:
> + ret = dwc3_qcom_interconnect_enable(qcom);
This overwrites the error that led to the execution of this code path.
The function should return original error, not the result of the
_interconnect_enable() call.
I saw Felipe queued the patch for v5.8. I think the main options to fix this
are:
- a v6 of this patch to replace v5 in Felipe's tree (which IIUC will be rebased
anyway once there is a v5.7-rc)
- send the fix as a separate patch
- Felipe amends the patch in his tree
Felipe, what would work best for you?
Thanks
Matthias
Hi,
Matthias Kaehlcke <[email protected]> writes:
>> Add interconnect support in dwc3-qcom driver to vote for bus
>> bandwidth.
>>
>> This requires for two different paths - from USB master to
>> DDR slave. The other is from APPS master to USB slave.
>>
>> Signed-off-by: Sandeep Maheswaram <[email protected]>
>> Signed-off-by: Chandana Kishori Chiluveru <[email protected]>
>> Reviewed-by: Matthias Kaehlcke <[email protected]>
>> ---
>> drivers/usb/dwc3/dwc3-qcom.c | 128 ++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 126 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
>> index 1dfd024..7e85fe6 100644
>> --- a/drivers/usb/dwc3/dwc3-qcom.c
>> +++ b/drivers/usb/dwc3/dwc3-qcom.c
>>
>> ...
>>
>> +/* To disable an interconnect, we just set its bandwidth to 0 */
>> +static int dwc3_qcom_interconnect_disable(struct dwc3_qcom *qcom)
>> +{
>> + int ret;
>> +
>> + ret = icc_set_bw(qcom->usb_ddr_icc_path, 0, 0);
>> + if (ret)
>> + return ret;
>> +
>> + ret = icc_set_bw(qcom->apps_usb_icc_path, 0, 0);
>> + if (ret)
>> + goto err_reenable_memory_path;
>> +
>> + return 0;
>> +
>> + /* Re-enable things in the event of an error */
>> +err_reenable_memory_path:
>> + ret = dwc3_qcom_interconnect_enable(qcom);
>
> This overwrites the error that led to the execution of this code path.
> The function should return original error, not the result of the
> _interconnect_enable() call.
>
> I saw Felipe queued the patch for v5.8. I think the main options to fix this
> are:
>
> - a v6 of this patch to replace v5 in Felipe's tree (which IIUC will be rebased
> anyway once there is a v5.7-rc)
> - send the fix as a separate patch
> - Felipe amends the patch in his tree
>
> Felipe, what would work best for you?
Let's go for a v6, which commits should I drop? I can't find anything
related to $subject in my queue:
$ git --no-pager log --oneline HEAD ^linus/master -- drivers/usb/dwc3/dwc3-qcom.c
201c26c08db4 usb: dwc3: qcom: Replace <linux/clk-provider.h> by <linux/of_clk.h>
--
balbi
On Mon, Mar 30, 2020 at 11:19:11AM +0300, Felipe Balbi wrote:
>
> Hi,
>
> Matthias Kaehlcke <[email protected]> writes:
> >> Add interconnect support in dwc3-qcom driver to vote for bus
> >> bandwidth.
> >>
> >> This requires for two different paths - from USB master to
> >> DDR slave. The other is from APPS master to USB slave.
> >>
> >> Signed-off-by: Sandeep Maheswaram <[email protected]>
> >> Signed-off-by: Chandana Kishori Chiluveru <[email protected]>
> >> Reviewed-by: Matthias Kaehlcke <[email protected]>
> >> ---
> >> drivers/usb/dwc3/dwc3-qcom.c | 128 ++++++++++++++++++++++++++++++++++++++++++-
> >> 1 file changed, 126 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> >> index 1dfd024..7e85fe6 100644
> >> --- a/drivers/usb/dwc3/dwc3-qcom.c
> >> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> >>
> >> ...
> >>
> >> +/* To disable an interconnect, we just set its bandwidth to 0 */
> >> +static int dwc3_qcom_interconnect_disable(struct dwc3_qcom *qcom)
> >> +{
> >> + int ret;
> >> +
> >> + ret = icc_set_bw(qcom->usb_ddr_icc_path, 0, 0);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + ret = icc_set_bw(qcom->apps_usb_icc_path, 0, 0);
> >> + if (ret)
> >> + goto err_reenable_memory_path;
> >> +
> >> + return 0;
> >> +
> >> + /* Re-enable things in the event of an error */
> >> +err_reenable_memory_path:
> >> + ret = dwc3_qcom_interconnect_enable(qcom);
> >
> > This overwrites the error that led to the execution of this code path.
> > The function should return original error, not the result of the
> > _interconnect_enable() call.
> >
> > I saw Felipe queued the patch for v5.8. I think the main options to fix this
> > are:
> >
> > - a v6 of this patch to replace v5 in Felipe's tree (which IIUC will be rebased
> > anyway once there is a v5.7-rc)
> > - send the fix as a separate patch
> > - Felipe amends the patch in his tree
> >
> > Felipe, what would work best for you?
>
> Let's go for a v6, which commits should I drop? I can't find anything
> related to $subject in my queue:
>
> $ git --no-pager log --oneline HEAD ^linus/master -- drivers/usb/dwc3/dwc3-qcom.c
> 201c26c08db4 usb: dwc3: qcom: Replace <linux/clk-provider.h> by <linux/of_clk.h>
I thought I saw a "queued for v5.8" message from you, but can't find that back.
I guess I saw the "queued" message for the "Add USB DWC3 support for SC7180"
series and thought it was for this one. Sorry for the confusion.
Hi,
Matthias Kaehlcke <[email protected]> writes:
>> Matthias Kaehlcke <[email protected]> writes:
>> >> Add interconnect support in dwc3-qcom driver to vote for bus
>> >> bandwidth.
>> >>
>> >> This requires for two different paths - from USB master to
>> >> DDR slave. The other is from APPS master to USB slave.
>> >>
>> >> Signed-off-by: Sandeep Maheswaram <[email protected]>
>> >> Signed-off-by: Chandana Kishori Chiluveru <[email protected]>
>> >> Reviewed-by: Matthias Kaehlcke <[email protected]>
>> >> ---
>> >> drivers/usb/dwc3/dwc3-qcom.c | 128 ++++++++++++++++++++++++++++++++++++++++++-
>> >> 1 file changed, 126 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
>> >> index 1dfd024..7e85fe6 100644
>> >> --- a/drivers/usb/dwc3/dwc3-qcom.c
>> >> +++ b/drivers/usb/dwc3/dwc3-qcom.c
>> >>
>> >> ...
>> >>
>> >> +/* To disable an interconnect, we just set its bandwidth to 0 */
>> >> +static int dwc3_qcom_interconnect_disable(struct dwc3_qcom *qcom)
>> >> +{
>> >> + int ret;
>> >> +
>> >> + ret = icc_set_bw(qcom->usb_ddr_icc_path, 0, 0);
>> >> + if (ret)
>> >> + return ret;
>> >> +
>> >> + ret = icc_set_bw(qcom->apps_usb_icc_path, 0, 0);
>> >> + if (ret)
>> >> + goto err_reenable_memory_path;
>> >> +
>> >> + return 0;
>> >> +
>> >> + /* Re-enable things in the event of an error */
>> >> +err_reenable_memory_path:
>> >> + ret = dwc3_qcom_interconnect_enable(qcom);
>> >
>> > This overwrites the error that led to the execution of this code path.
>> > The function should return original error, not the result of the
>> > _interconnect_enable() call.
>> >
>> > I saw Felipe queued the patch for v5.8. I think the main options to fix this
>> > are:
>> >
>> > - a v6 of this patch to replace v5 in Felipe's tree (which IIUC will be rebased
>> > anyway once there is a v5.7-rc)
>> > - send the fix as a separate patch
>> > - Felipe amends the patch in his tree
>> >
>> > Felipe, what would work best for you?
>>
>> Let's go for a v6, which commits should I drop? I can't find anything
>> related to $subject in my queue:
>>
>> $ git --no-pager log --oneline HEAD ^linus/master -- drivers/usb/dwc3/dwc3-qcom.c
>> 201c26c08db4 usb: dwc3: qcom: Replace <linux/clk-provider.h> by <linux/of_clk.h>
>
> I thought I saw a "queued for v5.8" message from you, but can't find that back.
> I guess I saw the "queued" message for the "Add USB DWC3 support for SC7180"
> series and thought it was for this one. Sorry for the confusion.
no worries :-)
--
balbi
Hi,
On 3/31/2020 3:05 AM, Felipe Balbi wrote:
> Hi,
>
> Matthias Kaehlcke <[email protected]> writes:
>>> Matthias Kaehlcke <[email protected]> writes:
>>>>> Add interconnect support in dwc3-qcom driver to vote for bus
>>>>> bandwidth.
>>>>>
>>>>> This requires for two different paths - from USB master to
>>>>> DDR slave. The other is from APPS master to USB slave.
>>>>>
>>>>> Signed-off-by: Sandeep Maheswaram <[email protected]>
>>>>> Signed-off-by: Chandana Kishori Chiluveru <[email protected]>
>>>>> Reviewed-by: Matthias Kaehlcke <[email protected]>
>>>>> ---
>>>>> drivers/usb/dwc3/dwc3-qcom.c | 128 ++++++++++++++++++++++++++++++++++++++++++-
>>>>> 1 file changed, 126 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
>>>>> index 1dfd024..7e85fe6 100644
>>>>> --- a/drivers/usb/dwc3/dwc3-qcom.c
>>>>> +++ b/drivers/usb/dwc3/dwc3-qcom.c
>>>>>
>>>>> ...
>>>>>
>>>>> +/* To disable an interconnect, we just set its bandwidth to 0 */
>>>>> +static int dwc3_qcom_interconnect_disable(struct dwc3_qcom *qcom)
>>>>> +{
>>>>> + int ret;
>>>>> +
>>>>> + ret = icc_set_bw(qcom->usb_ddr_icc_path, 0, 0);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + ret = icc_set_bw(qcom->apps_usb_icc_path, 0, 0);
>>>>> + if (ret)
>>>>> + goto err_reenable_memory_path;
>>>>> +
>>>>> + return 0;
>>>>> +
>>>>> + /* Re-enable things in the event of an error */
>>>>> +err_reenable_memory_path:
>>>>> + ret = dwc3_qcom_interconnect_enable(qcom);
>>>> This overwrites the error that led to the execution of this code path.
>>>> The function should return original error, not the result of the
>>>> _interconnect_enable() call.
>>>>
>>>> I saw Felipe queued the patch for v5.8. I think the main options to fix this
>>>> are:
>>>>
>>>> - a v6 of this patch to replace v5 in Felipe's tree (which IIUC will be rebased
>>>> anyway once there is a v5.7-rc)
>>>> - send the fix as a separate patch
>>>> - Felipe amends the patch in his tree
>>>>
>>>> Felipe, what would work best for you?
>>> Let's go for a v6, which commits should I drop? I can't find anything
>>> related to $subject in my queue:
>>>
>>> $ git --no-pager log --oneline HEAD ^linus/master -- drivers/usb/dwc3/dwc3-qcom.c
>>> 201c26c08db4 usb: dwc3: qcom: Replace <linux/clk-provider.h> by <linux/of_clk.h>
>> I thought I saw a "queued for v5.8" message from you, but can't find that back.
>> I guess I saw the "queued" message for the "Add USB DWC3 support for SC7180"
>> series and thought it was for this one. Sorry for the confusion.
> no worries :-)
>
Should I remove the ret from below line and send a new version?
+ ret = dwc3_qcom_interconnect_enable(qcom);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
On Tue, Mar 31, 2020 at 10:45:19AM +0530, Sandeep Maheswaram (Temp) wrote:
> Hi,
>
> On 3/31/2020 3:05 AM, Felipe Balbi wrote:
> > Hi,
> >
> > Matthias Kaehlcke <[email protected]> writes:
> > > > Matthias Kaehlcke <[email protected]> writes:
> > > > > > Add interconnect support in dwc3-qcom driver to vote for bus
> > > > > > bandwidth.
> > > > > >
> > > > > > This requires for two different paths - from USB master to
> > > > > > DDR slave. The other is from APPS master to USB slave.
> > > > > >
> > > > > > Signed-off-by: Sandeep Maheswaram <[email protected]>
> > > > > > Signed-off-by: Chandana Kishori Chiluveru <[email protected]>
> > > > > > Reviewed-by: Matthias Kaehlcke <[email protected]>
> > > > > > ---
> > > > > > drivers/usb/dwc3/dwc3-qcom.c | 128 ++++++++++++++++++++++++++++++++++++++++++-
> > > > > > 1 file changed, 126 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> > > > > > index 1dfd024..7e85fe6 100644
> > > > > > --- a/drivers/usb/dwc3/dwc3-qcom.c
> > > > > > +++ b/drivers/usb/dwc3/dwc3-qcom.c
> > > > > >
> > > > > > ...
> > > > > >
> > > > > > +/* To disable an interconnect, we just set its bandwidth to 0 */
> > > > > > +static int dwc3_qcom_interconnect_disable(struct dwc3_qcom *qcom)
> > > > > > +{
> > > > > > + int ret;
> > > > > > +
> > > > > > + ret = icc_set_bw(qcom->usb_ddr_icc_path, 0, 0);
> > > > > > + if (ret)
> > > > > > + return ret;
> > > > > > +
> > > > > > + ret = icc_set_bw(qcom->apps_usb_icc_path, 0, 0);
> > > > > > + if (ret)
> > > > > > + goto err_reenable_memory_path;
> > > > > > +
> > > > > > + return 0;
> > > > > > +
> > > > > > + /* Re-enable things in the event of an error */
> > > > > > +err_reenable_memory_path:
> > > > > > + ret = dwc3_qcom_interconnect_enable(qcom);
> > > > > This overwrites the error that led to the execution of this code path.
> > > > > The function should return original error, not the result of the
> > > > > _interconnect_enable() call.
> > > > >
> > > > > I saw Felipe queued the patch for v5.8. I think the main options to fix this
> > > > > are:
> > > > >
> > > > > - a v6 of this patch to replace v5 in Felipe's tree (which IIUC will be rebased
> > > > > anyway once there is a v5.7-rc)
> > > > > - send the fix as a separate patch
> > > > > - Felipe amends the patch in his tree
> > > > >
> > > > > Felipe, what would work best for you?
> > > > Let's go for a v6, which commits should I drop? I can't find anything
> > > > related to $subject in my queue:
> > > >
> > > > $ git --no-pager log --oneline HEAD ^linus/master -- drivers/usb/dwc3/dwc3-qcom.c
> > > > 201c26c08db4 usb: dwc3: qcom: Replace <linux/clk-provider.h> by <linux/of_clk.h>
> > > I thought I saw a "queued for v5.8" message from you, but can't find that back.
> > > I guess I saw the "queued" message for the "Add USB DWC3 support for SC7180"
> > > series and thought it was for this one. Sorry for the confusion.
> > no worries :-)
> >
> Should I remove the ret from below line and send a new version?
> + ret = dwc3_qcom_interconnect_enable(qcom);
Yes, please!