2023-12-22 06:38:18

by Krishna Kurapati

[permalink] [raw]
Subject: [PATCH v5 0/2] Refine USB interrupt vectors on Qualcomm platforms

Qualcomm targets define the following interrupts for usb wakeup:
{dp/dm}_hs_phy_irq, hs_phy_irq, pwr_event, ss_phy_irq.

But QUSB2 Phy based targets have another interrupt which gets triggered
in response to J/K states on dp/dm pads. Its functionality is replaced
by dp/dm interrupts on Femto/m31/eusb2 phy based targets for wakeup
purposes. Exceptions are some targets like SDM845/SDM670/SM6350 where
dp/dm irq's are used although they are qusb2 phy targets.

Currently in QUSB2 Phy based DT's, te qusb2_phy interrupt is named and
used as "hs_phy_irq" when in fact it is a different interrupt (used by
HW validation folks for debug purposes and not used on any downstream
target qusb/non-qusb).

On some non-QUSB2 targets (like sm8450/sm8550), the pwr_event IRQ was
named as hs_phy_irq and actual pwr_event_irq was skipped.

This series tries to address the discrepancies in the interrupt numbering
adding the missing interrupts and correcting the existing ones.

This series has been compared with downstream counter part and hw specifics
to ensure the numbering is right. Since there is not functionality change
the code has been only compile tested.

Changes in v5:
Fixed commit header on v4-1 bindings patch.
Provide lore link instead of patchwork link for v3.

Changes in v4:
Udpated commit text indicating why pwr_event interrupt was added as the first
one and fixed some typos present in v3.
While at it, rebase on top of latest linux-next fixing merge conflicts.

Changes in v3:
Separated out the DT changes and pushed only bindings and driver update.
Modified order of irq descriptions to match them with permutations defined.
Fixed nitpicks mentioned by reviewers in v2.

Changes in v2:
Removed additional compatibles added for different targets in v1.
Specified permuations of interrupts possible for QC targets and regrouped
interrupts for most of the DT's.

Link to v4:
https://lore.kernel.org/all/[email protected]/

Link to v3:
https://lore.kernel.org/all/[email protected]/

Link to v2:
https://lore.kernel.org/all/[email protected]/

Link to v1: (providing patchwork link since threading was broken in v1)
https://patchwork.kernel.org/project/linux-arm-msm/cover/[email protected]/

Krishna Kurapati (2):
The high speed related interrupts present on QC targets are as
follows:
usb: dwc3: qcom: Rename hs_phy_irq to qusb2_phy_irq

.../devicetree/bindings/usb/qcom,dwc3.yaml | 138 ++++++++----------
drivers/usb/dwc3/dwc3-qcom.c | 22 +--
2 files changed, 70 insertions(+), 90 deletions(-)

--
2.42.0



2023-12-22 06:38:27

by Krishna Kurapati

[permalink] [raw]
Subject: [PATCH v5 2/2] usb: dwc3: qcom: Rename hs_phy_irq to qusb2_phy_irq

For wakeup to work, driver needs to enable interrupts that depict what is
happening on the DP/DM lines. On QUSB targets, this is identified by
qusb2_phy whereas on SoCs using Femto PHY, separate {dp,dm}_hs_phy_irq's
are used instead.

The implementation incorrectly names qusb2_phy interrupts as "hs_phy_irq".
Clean this up so that driver would be using only qusb2/(dp & dm) for wakeup
purposes.

For devices running older kernels, this won't break any functionality
because the interrupt configurations in QUSB2 PHY based SoCs is done
by configuring QUSB2PHY_INTR_CTRL register in PHY address space and it was
never armed properly right from the start.

Signed-off-by: Krishna Kurapati <[email protected]>
---
drivers/usb/dwc3/dwc3-qcom.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index fdf6d5d3c2ad..dbd6a5b2b289 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -57,7 +57,7 @@ struct dwc3_acpi_pdata {
u32 qscratch_base_offset;
u32 qscratch_base_size;
u32 dwc3_core_base_size;
- int hs_phy_irq_index;
+ int qusb2_phy_irq_index;
int dp_hs_phy_irq_index;
int dm_hs_phy_irq_index;
int ss_phy_irq_index;
@@ -73,7 +73,7 @@ struct dwc3_qcom {
int num_clocks;
struct reset_control *resets;

- int hs_phy_irq;
+ int qusb2_phy_irq;
int dp_hs_phy_irq;
int dm_hs_phy_irq;
int ss_phy_irq;
@@ -372,7 +372,7 @@ static void dwc3_qcom_disable_wakeup_irq(int irq)

static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom)
{
- dwc3_qcom_disable_wakeup_irq(qcom->hs_phy_irq);
+ dwc3_qcom_disable_wakeup_irq(qcom->qusb2_phy_irq);

if (qcom->usb2_speed == USB_SPEED_LOW) {
dwc3_qcom_disable_wakeup_irq(qcom->dm_hs_phy_irq);
@@ -389,7 +389,7 @@ static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom)

static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom)
{
- dwc3_qcom_enable_wakeup_irq(qcom->hs_phy_irq, 0);
+ dwc3_qcom_enable_wakeup_irq(qcom->qusb2_phy_irq, 0);

/*
* Configure DP/DM line interrupts based on the USB2 device attached to
@@ -542,19 +542,19 @@ static int dwc3_qcom_setup_irq(struct platform_device *pdev)
int irq;
int ret;

- irq = dwc3_qcom_get_irq(pdev, "hs_phy_irq",
- pdata ? pdata->hs_phy_irq_index : -1);
+ irq = dwc3_qcom_get_irq(pdev, "qusb2_phy",
+ pdata ? pdata->qusb2_phy_irq_index : -1);
if (irq > 0) {
/* Keep wakeup interrupts disabled until suspend */
ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
qcom_dwc3_resume_irq,
IRQF_ONESHOT | IRQF_NO_AUTOEN,
- "qcom_dwc3 HS", qcom);
+ "qcom_dwc3 QUSB2", qcom);
if (ret) {
- dev_err(qcom->dev, "hs_phy_irq failed: %d\n", ret);
+ dev_err(qcom->dev, "qusb2_phy_irq failed: %d\n", ret);
return ret;
}
- qcom->hs_phy_irq = irq;
+ qcom->qusb2_phy_irq = irq;
}

irq = dwc3_qcom_get_irq(pdev, "dp_hs_phy_irq",
@@ -1058,7 +1058,7 @@ static const struct dwc3_acpi_pdata sdm845_acpi_pdata = {
.qscratch_base_offset = SDM845_QSCRATCH_BASE_OFFSET,
.qscratch_base_size = SDM845_QSCRATCH_SIZE,
.dwc3_core_base_size = SDM845_DWC3_CORE_SIZE,
- .hs_phy_irq_index = 1,
+ .qusb2_phy_irq_index = 1,
.dp_hs_phy_irq_index = 4,
.dm_hs_phy_irq_index = 3,
.ss_phy_irq_index = 2
@@ -1068,7 +1068,7 @@ static const struct dwc3_acpi_pdata sdm845_acpi_urs_pdata = {
.qscratch_base_offset = SDM845_QSCRATCH_BASE_OFFSET,
.qscratch_base_size = SDM845_QSCRATCH_SIZE,
.dwc3_core_base_size = SDM845_DWC3_CORE_SIZE,
- .hs_phy_irq_index = 1,
+ .qusb2_phy_irq_index = 1,
.dp_hs_phy_irq_index = 4,
.dm_hs_phy_irq_index = 3,
.ss_phy_irq_index = 2,
--
2.42.0


2023-12-22 22:14:46

by Thinh Nguyen

[permalink] [raw]
Subject: Re: [PATCH v5 2/2] usb: dwc3: qcom: Rename hs_phy_irq to qusb2_phy_irq

On Fri, Dec 22, 2023, Krishna Kurapati wrote:
> For wakeup to work, driver needs to enable interrupts that depict what is
> happening on the DP/DM lines. On QUSB targets, this is identified by
> qusb2_phy whereas on SoCs using Femto PHY, separate {dp,dm}_hs_phy_irq's
> are used instead.
>
> The implementation incorrectly names qusb2_phy interrupts as "hs_phy_irq".
> Clean this up so that driver would be using only qusb2/(dp & dm) for wakeup
> purposes.
>
> For devices running older kernels, this won't break any functionality
> because the interrupt configurations in QUSB2 PHY based SoCs is done
> by configuring QUSB2PHY_INTR_CTRL register in PHY address space and it was
> never armed properly right from the start.
>
> Signed-off-by: Krishna Kurapati <[email protected]>
> ---
> drivers/usb/dwc3/dwc3-qcom.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index fdf6d5d3c2ad..dbd6a5b2b289 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -57,7 +57,7 @@ struct dwc3_acpi_pdata {
> u32 qscratch_base_offset;
> u32 qscratch_base_size;
> u32 dwc3_core_base_size;
> - int hs_phy_irq_index;
> + int qusb2_phy_irq_index;
> int dp_hs_phy_irq_index;
> int dm_hs_phy_irq_index;
> int ss_phy_irq_index;
> @@ -73,7 +73,7 @@ struct dwc3_qcom {
> int num_clocks;
> struct reset_control *resets;
>
> - int hs_phy_irq;
> + int qusb2_phy_irq;
> int dp_hs_phy_irq;
> int dm_hs_phy_irq;
> int ss_phy_irq;
> @@ -372,7 +372,7 @@ static void dwc3_qcom_disable_wakeup_irq(int irq)
>
> static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom)
> {
> - dwc3_qcom_disable_wakeup_irq(qcom->hs_phy_irq);
> + dwc3_qcom_disable_wakeup_irq(qcom->qusb2_phy_irq);
>
> if (qcom->usb2_speed == USB_SPEED_LOW) {
> dwc3_qcom_disable_wakeup_irq(qcom->dm_hs_phy_irq);
> @@ -389,7 +389,7 @@ static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom)
>
> static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom)
> {
> - dwc3_qcom_enable_wakeup_irq(qcom->hs_phy_irq, 0);
> + dwc3_qcom_enable_wakeup_irq(qcom->qusb2_phy_irq, 0);
>
> /*
> * Configure DP/DM line interrupts based on the USB2 device attached to
> @@ -542,19 +542,19 @@ static int dwc3_qcom_setup_irq(struct platform_device *pdev)
> int irq;
> int ret;
>
> - irq = dwc3_qcom_get_irq(pdev, "hs_phy_irq",
> - pdata ? pdata->hs_phy_irq_index : -1);
> + irq = dwc3_qcom_get_irq(pdev, "qusb2_phy",
> + pdata ? pdata->qusb2_phy_irq_index : -1);
> if (irq > 0) {
> /* Keep wakeup interrupts disabled until suspend */
> ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
> qcom_dwc3_resume_irq,
> IRQF_ONESHOT | IRQF_NO_AUTOEN,
> - "qcom_dwc3 HS", qcom);
> + "qcom_dwc3 QUSB2", qcom);
> if (ret) {
> - dev_err(qcom->dev, "hs_phy_irq failed: %d\n", ret);
> + dev_err(qcom->dev, "qusb2_phy_irq failed: %d\n", ret);
> return ret;
> }
> - qcom->hs_phy_irq = irq;
> + qcom->qusb2_phy_irq = irq;
> }
>
> irq = dwc3_qcom_get_irq(pdev, "dp_hs_phy_irq",
> @@ -1058,7 +1058,7 @@ static const struct dwc3_acpi_pdata sdm845_acpi_pdata = {
> .qscratch_base_offset = SDM845_QSCRATCH_BASE_OFFSET,
> .qscratch_base_size = SDM845_QSCRATCH_SIZE,
> .dwc3_core_base_size = SDM845_DWC3_CORE_SIZE,
> - .hs_phy_irq_index = 1,
> + .qusb2_phy_irq_index = 1,
> .dp_hs_phy_irq_index = 4,
> .dm_hs_phy_irq_index = 3,
> .ss_phy_irq_index = 2
> @@ -1068,7 +1068,7 @@ static const struct dwc3_acpi_pdata sdm845_acpi_urs_pdata = {
> .qscratch_base_offset = SDM845_QSCRATCH_BASE_OFFSET,
> .qscratch_base_size = SDM845_QSCRATCH_SIZE,
> .dwc3_core_base_size = SDM845_DWC3_CORE_SIZE,
> - .hs_phy_irq_index = 1,
> + .qusb2_phy_irq_index = 1,
> .dp_hs_phy_irq_index = 4,
> .dm_hs_phy_irq_index = 3,
> .ss_phy_irq_index = 2,
> --
> 2.42.0
>

Acked-by: Thinh Nguyen <[email protected]>

Thanks,
Thinh