Re: [PATCH v13 05/10] usb: dwc3: qcom: Refactor IRQ handling in QCOM Glue driver
From: Krishna Kurapati PSSNV
Date: Mon Oct 23 2023 - 07:24:55 EST
On 10/23/2023 2:51 PM, Johan Hovold wrote:
On Mon, Oct 23, 2023 at 12:11:45AM +0530, Krishna Kurapati PSSNV wrote:
On 10/20/2023 6:53 PM, Johan Hovold wrote:
On Sat, Oct 07, 2023 at 09:18:01PM +0530, Krishna Kurapati wrote:
+#define NUM_PHY_IRQ 4
+
+enum dwc3_qcom_ph_index {
"phy_index"
+ DP_HS_PHY_IRQ_INDEX = 0,
+ DM_HS_PHY_IRQ_INDEX,
+ SS_PHY_IRQ_INDEX,
+ HS_PHY_IRQ_INDEX,
+};
+
struct dwc3_acpi_pdata {
u32 qscratch_base_offset;
u32 qscratch_base_size;
u32 dwc3_core_base_size;
+ /*
+ * The phy_irq_index corresponds to ACPI indexes of (in order) DP/DM/SS
+ * IRQ's respectively.
+ */
+ int phy_irq_index[NUM_PHY_IRQ - 1];
int hs_phy_irq_index;
- int dp_hs_phy_irq_index;
- int dm_hs_phy_irq_index;
- int ss_phy_irq_index;
bool is_urs;
};
@@ -73,10 +84,12 @@ struct dwc3_qcom {
int num_clocks;
struct reset_control *resets;
+ /*
+ * The phy_irq corresponds to IRQ's registered for (in order) DP/DM/SS
+ * respectively.
+ */
+ int phy_irq[NUM_PHY_IRQ - 1][DWC3_MAX_PORTS];
int hs_phy_irq;
- int dp_hs_phy_irq;
- int dm_hs_phy_irq;
- int ss_phy_irq;
I'm not sure using arrays like this is a good idea (and haven't you
switched the indexes above?).
Why not add a port structure instead?
struct dwc3_qcom_port {
int hs_phy_irq;
int dp_hs_phy_irq;
int dm_hs_phy_irq;
int ss_phy_irq;
};
and then have
struct dwc3_qcom_port ports[DWC3_MAX_PORTS];
in dwc3_qcom. The port structure can the later also be amended with
whatever other additional per-port data there is need for.
This should make the implementation cleaner.
I also don't like the special handling of hs_phy_irq; if this is really
just another name for the pwr_event_irq then this should be cleaned up
before making the code more complicated than it needs to be.
Make sure to clarify this before posting a new revision.
hs_phy_irq is different from pwr_event_irq.
How is it different and how are they used?
AFAIK, there is only one of this per controller.
But previous controllers were all single port so this interrupt is
likely also per-port, even if your comment below seems to suggest even
SC8280XP has one, which is unexpected (and not described in the updated
binding):
Yes, all targets have the same IRQ's. Just that MP one's have
multiple IRQ's of each type. But hs-phy_irq is only one in
SC8280 as well.
https://lore.kernel.org/lkml/70b2495f-1305-05b1-2039-9573d171fe24@xxxxxxxxxxx/
Please clarify.
For sure pwr_event_irq and hs_phy_irq are different. I assumed it was
per-controller and not per-phy because I took reference from software
code we have on downstream and hs_phy for multiport is not used
anywhere. I don't see any functionality implemented in downstream for
that IRQ. And it is only one for single port controllers.
But I got the following info from HW page and these are all the
interrupts (on apss processor) for multiport (extra details removed):
u_usb31_scnd_mvs_pipe_wrapper_usb31_power_event_irq_0 SYS_apcsQgicSPI[130]
u_usb31_scnd_mvs_pipe_wrapper_usb31_power_event_irq_1 SYS_apcsQgicSPI[135]
u_usb31_scnd_mvs_pipe_wrapper_usb31_power_event_irq_3 SYS_apcsQgicSPI[856]
u_usb31_scnd_mvs_pipe_wrapper_usb31_power_event_irq_2 SYS_apcsQgicSPI[857]
u_usb31_scnd_mvs_pipe_wrapper_usb31_ctrl_irq[0] SYS_apcsQgicSPI[133]
u_usb31_scnd_mvs_pipe_wrapper_usb31_ctrl_irq[1] SYS_apcsQgicSPI[134]
u_cm_usb3_uni_wrapper_mp0_usb3phy_debug_irq SYS_apcsQgicSPI[668]
u_usb31_scnd_mvs_pipe_wrapper_usb31_bam_irq[0] SYS_apcsQgicSPI[830]
u_cm_usb3_uni_wrapper_mp1_usb3phy_debug_irq SYS_apcsQgicSPI[855]
u_usb31_scnd_mvs_pipe_wrapper_usb31_hs_phy_irq_0 SYS_apcsQgicSPI[131]
u_usb31_scnd_mvs_pipe_wrapper_usb31_hs_phy_irq_1 SYS_apcsQgicSPI[136]
u_usb31_scnd_mvs_pipe_wrapper_usb31_hs_phy_irq_3 SYS_apcsQgicSPI[859]
u_usb31_scnd_mvs_pipe_wrapper_usb31_hs_phy_irq_2 SYS_apcsQgicSPI[860]
u_cm_dwc_usb2_hs0_usb2_dpse apps_pdc_irq_out[127]
u_cm_dwc_usb2_hs0_usb2_dmse apps_pdc_irq_out[126]
u_cm_dwc_usb2_hs1_usb2_dpse apps_pdc_irq_out[129]
u_cm_dwc_usb2_hs1_usb2_dmse apps_pdc_irq_out[128]
u_cm_dwc_usb2_hs2_usb2_dpse apps_pdc_irq_out[131]
u_cm_dwc_usb2_hs2_usb2_dmse apps_pdc_irq_out[130]
u_cm_dwc_usb2_hs3_usb2_dpse apps_pdc_irq_out[133]
u_cm_dwc_usb2_hs3_usb2_dmse apps_pdc_irq_out[132]
u_cm_usb3_uni_wrapper_mp0_qmp_usb3_lfps_rxterm_irq apps_pdc_irq_out[16]
u_cm_usb3_uni_wrapper_mp1_qmp_usb3_lfps_rxterm_irq apps_pdc_irq_out[17]
Seems like there are 4 IRQ's for HS.
Regards,
Krishna,