| Message ID | 20240216005756.762712-8-quic_kriskura@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add multiport support for DWC3 controllers | expand |
On Fri, Feb 16, 2024 at 06:27:54AM +0530, Krishna Kurapati wrote: > On multiport supported controllers, each port has its own DP/DM > and SS (if super speed capable) interrupts. As per the bindings, > their interrupt names differ from standard ones having "_x" added > as suffix (x indicates port number). Refactor dwc3_qcom_setup_irq() > call to parse multiport interrupts along with non-multiport ones. > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > Reviewed-by: Bjorn Andersson <andersson@kernel.org> > Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > --- > drivers/usb/dwc3/dwc3-qcom.c | 222 +++++++++++++++++++++++++---------- > 1 file changed, 161 insertions(+), 61 deletions(-) > > diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c > index 08df29584366..a20d63a791bd 100644 > --- a/drivers/usb/dwc3/dwc3-qcom.c > +++ b/drivers/usb/dwc3/dwc3-qcom.c > @@ -53,17 +53,33 @@ > #define APPS_USB_AVG_BW 0 > #define APPS_USB_PEAK_BW MBps_to_icc(40) > > +#define NUM_PHY_IRQ 4 > + > +enum dwc3_qcom_phy_index { > + DP_HS_PHY_IRQ_INDEX, > + DM_HS_PHY_IRQ_INDEX, > + SS_PHY_IRQ_INDEX, > + QUSB2_PHY_IRQ_INDEX, > +}; > + > struct dwc3_acpi_pdata { > u32 qscratch_base_offset; > u32 qscratch_base_size; > u32 dwc3_core_base_size; > - int qusb2_phy_irq_index; > - int dp_hs_phy_irq_index; > - int dm_hs_phy_irq_index; > - int ss_phy_irq_index; > + /* > + * The phy_irq_index corresponds to ACPI indexes of (in order) > + * DP/DM/SS/QUSB2 IRQ's respectively. > + */ > + int phy_irq_index[NUM_PHY_IRQ]; > bool is_urs; > }; I asked you to add a port structure and get rid of the PHY indexes in v13, and so you did for the diver data below, but you still have an array of indexes here for the ACPI data. I don't think ever got around to actually reviewing the ACPI hack (and maybe I was hoping that we'd be able to drop ACPI support before merging multi-port support), but removing these fields and replacing them with an array is a step in the wrong direction (e.g. making the code harder to read). Why can't you just add a helper function which returns one of these fields based on the interrupt name string? > +struct dwc3_qcom_port { > + int dp_hs_phy_irq; > + int dm_hs_phy_irq; > + int ss_phy_irq; > +}; And as I've explicitly said before, you should include hs_phy_irq here. It's a port interrupt and special casing just this one make no sense at all even if there are no multi-port controller that use it. > + > struct dwc3_qcom { > struct device *dev; > void __iomem *qscratch_base; > @@ -74,9 +90,7 @@ struct dwc3_qcom { > struct reset_control *resets; > > int qusb2_phy_irq; > - int dp_hs_phy_irq; > - int dm_hs_phy_irq; > - int ss_phy_irq; > + struct dwc3_qcom_port port_info[DWC3_MAX_PORTS]; Just name the array 'ports' as I already suggested. It's more succinct and makes the code that uses it easier to read. > enum usb_device_speed usb2_speed; > > struct extcon_dev *edev; > @@ -91,6 +105,7 @@ struct dwc3_qcom { > bool pm_suspended; > struct icc_path *icc_path_ddr; > struct icc_path *icc_path_apps; > + u8 num_ports; Any reason not to keep this one closer to the ports array? > }; > +static int dwc3_qcom_get_irq_index(const char *irq_name) > +{ > + /* > + * Parse IRQ index based on prefixes from interrupt name. > + * Return -1 incase of an invalid interrupt name. > + */ > + int irq_index = -1; > + > + if (strncmp(irq_name, "dp_hs_phy", strlen("dp_hs_phy")) == 0) > + irq_index = DP_HS_PHY_IRQ_INDEX; > + else if (strncmp(irq_name, "dm_hs_phy", strlen("dm_hs_phy")) == 0) > + irq_index = DM_HS_PHY_IRQ_INDEX; > + else if (strncmp(irq_name, "ss_phy", strlen("ss_phy")) == 0) > + irq_index = SS_PHY_IRQ_INDEX; > + else if (strncmp(irq_name, "qusb2_phy", strlen("qusb2_phy")) == 0) > + irq_index = QUSB2_PHY_IRQ_INDEX; > + return irq_index; > +} > + > +static int dwc3_qcom_get_port_index(const char *irq_name, int irq_index) > +{ > + int port_index = -1; > + > + switch (irq_index) { > + case DP_HS_PHY_IRQ_INDEX: > + if (strcmp(irq_name, "dp_hs_phy_irq") == 0) > + port_index = 1; > + else > + sscanf(irq_name, "dp_hs_phy_%d", &port_index); > + break; > + case DM_HS_PHY_IRQ_INDEX: > + if (strcmp(irq_name, "dm_hs_phy_irq") == 0) > + port_index = 1; > + else > + sscanf(irq_name, "dm_hs_phy_%d", &port_index); > + break; > + case SS_PHY_IRQ_INDEX: > + if (strcmp(irq_name, "ss_phy_irq") == 0) > + port_index = 1; > + else > + sscanf(irq_name, "ss_phy_%d", &port_index); > + break; > + case QUSB2_PHY_IRQ_INDEX: > + port_index = 1; > + break; > + } > + > + if (port_index <= 0 || port_index > DWC3_MAX_PORTS) > + port_index = -1; > + > + return port_index; > +} > + > +static int dwc3_qcom_get_acpi_index(struct dwc3_qcom *qcom, int irq_index, > + int port_index) > +{ > + const struct dwc3_acpi_pdata *pdata = qcom->acpi_pdata; > + > + /* > + * Currently multiport supported targets don't have an ACPI variant. > + * So return -1 if we are not dealing with first port of the controller. > + */ > + if (!pdata || port_index != 1) > + return -1; > + > + return pdata->phy_irq_index[irq_index]; > +} Yeah, you need to come some better solution than the above, which is just unnecessarily convoluted. > static int dwc3_qcom_request_irq(struct dwc3_qcom *qcom, int irq, > const char *name) > { > @@ -554,44 +637,67 @@ static int dwc3_qcom_request_irq(struct dwc3_qcom *qcom, int irq, > static int dwc3_qcom_setup_irq(struct platform_device *pdev) > { > struct dwc3_qcom *qcom = platform_get_drvdata(pdev); > - const struct dwc3_acpi_pdata *pdata = qcom->acpi_pdata; > + struct device_node *np = pdev->dev.of_node; > + const char **irq_names; > + int port_index; > + int acpi_index; > + int irq_count; > + int irq_index; > int irq; > int ret; > + int i; > > - irq = dwc3_qcom_get_irq(pdev, "qusb2_phy", > - pdata ? pdata->qusb2_phy_irq_index : -1); > - if (irq > 0) { > - ret = dwc3_qcom_request_irq(qcom, irq, "hs_phy_irq"); > - if (ret) > - return ret; > - qcom->qusb2_phy_irq = irq; > - } > + irq_count = of_property_count_strings(np, "interrupt-names"); > + if (irq_count < 0) > + return -EINVAL; > > - irq = dwc3_qcom_get_irq(pdev, "dp_hs_phy_irq", > - pdata ? pdata->dp_hs_phy_irq_index : -1); > - if (irq > 0) { > - ret = dwc3_qcom_request_irq(qcom, irq, "dp_hs_phy_irq"); > - if (ret) > - return ret; > - qcom->dp_hs_phy_irq = irq; > - } > + irq_names = devm_kcalloc(&pdev->dev, irq_count, sizeof(*irq_names), GFP_KERNEL); > + if (!irq_names) > + return -ENOMEM; > > - irq = dwc3_qcom_get_irq(pdev, "dm_hs_phy_irq", > - pdata ? pdata->dm_hs_phy_irq_index : -1); > - if (irq > 0) { > - ret = dwc3_qcom_request_irq(qcom, irq, "dm_hs_phy_irq"); > - if (ret) > - return ret; > - qcom->dm_hs_phy_irq = irq; > - } > + ret = of_property_read_string_array(np, "interrupt-names", > + irq_names, irq_count); > + if (!ret) > + return ret; > > - irq = dwc3_qcom_get_irq(pdev, "ss_phy_irq", > - pdata ? pdata->ss_phy_irq_index : -1); > - if (irq > 0) { > - ret = dwc3_qcom_request_irq(qcom, irq, "ss_phy_irq"); > - if (ret) > - return ret; > - qcom->ss_phy_irq = irq; > + for (i = 0; i < irq_count; i++) { > + irq_index = dwc3_qcom_get_irq_index(irq_names[i]); > + if (irq_index == -1) { > + dev_err(&pdev->dev, "Unknown interrupt-name \"%s\" found\n", irq_names[i]); This is now spamming the logs with errors like dwc3-qcom a6f8800.usb: Unknown interrupt-name "pwr_event" found which is clearly just broken. > + continue; > + } > + port_index = dwc3_qcom_get_port_index(irq_names[i], irq_index); > + if (port_index == -1) { > + dev_err(&pdev->dev, "Invalid interrupt-name suffix \"%s\"\n", irq_names[i]); > + continue; > + } > + > + acpi_index = dwc3_qcom_get_acpi_index(qcom, irq_index, port_index); > + > + irq = dwc3_qcom_get_irq(pdev, irq_names[i], acpi_index); > + if (irq > 0) { > + ret = dwc3_qcom_request_irq(qcom, irq, irq_names[i]); > + if (ret) > + return ret; > + > + switch (irq_index) { > + case DP_HS_PHY_IRQ_INDEX: > + qcom->port_info[port_index - 1].dp_hs_phy_irq = irq; > + break; > + case DM_HS_PHY_IRQ_INDEX: > + qcom->port_info[port_index - 1].dm_hs_phy_irq = irq; > + break; > + case SS_PHY_IRQ_INDEX: > + qcom->port_info[port_index - 1].ss_phy_irq = irq; > + break; > + case QUSB2_PHY_IRQ_INDEX: > + qcom->qusb2_phy_irq = irq; > + break; > + } > + > + if (qcom->num_ports < port_index) > + qcom->num_ports = port_index; > + } > } Why don't you add a port helper for fetching the interrupts instead? There are multiple ways that you can use to determine if this is a multiport controller or not; you can use OF match data, or simply look at one of the interrupts that would always be there for a multiport (or single port) controller (e.g. "dp_hs_phy_1"). You can even determine the number of ports first by parsing the interrupts names and looking for the highest port number. Then you can iterate over the ports and parse the interrupts for each port in turn, which should allow for a much cleaner and less error-prone implementation. Johan
On 3/1/2024 7:23 PM, Johan Hovold wrote: [...] >> + >> struct dwc3_acpi_pdata { >> u32 qscratch_base_offset; >> u32 qscratch_base_size; >> u32 dwc3_core_base_size; >> - int qusb2_phy_irq_index; >> - int dp_hs_phy_irq_index; >> - int dm_hs_phy_irq_index; >> - int ss_phy_irq_index; >> + /* >> + * The phy_irq_index corresponds to ACPI indexes of (in order) >> + * DP/DM/SS/QUSB2 IRQ's respectively. >> + */ >> + int phy_irq_index[NUM_PHY_IRQ]; >> bool is_urs; >> }; > > I asked you to add a port structure and get rid of the PHY indexes in > v13, and so you did for the diver data below, but you still have an > array of indexes here for the ACPI data. > > I don't think ever got around to actually reviewing the ACPI hack (and > maybe I was hoping that we'd be able to drop ACPI support before merging > multi-port support), but removing these fields and replacing them with > an array is a step in the wrong direction (e.g. making the code harder > to read). > > Why can't you just add a helper function which returns one of these > fields based on the interrupt name string? I think since [1] has been accepted, this comment has been taken care of. > >> +struct dwc3_qcom_port { >> + int dp_hs_phy_irq; >> + int dm_hs_phy_irq; >> + int ss_phy_irq; >> +}; > > And as I've explicitly said before, you should include hs_phy_irq here. > > It's a port interrupt and special casing just this one make no sense at > all even if there are no multi-port controller that use it. > Okay. Will add it to port structure. I only kept it outside because there are no real devices which has multiple ports and qusb2_phy_irq in them. >> + >> struct dwc3_qcom { >> struct device *dev; >> void __iomem *qscratch_base; >> @@ -74,9 +90,7 @@ struct dwc3_qcom { >> struct reset_control *resets; >> >> int qusb2_phy_irq; >> - int dp_hs_phy_irq; >> - int dm_hs_phy_irq; >> - int ss_phy_irq; >> + struct dwc3_qcom_port port_info[DWC3_MAX_PORTS]; > > Just name the array 'ports' as I already suggested. It's more succinct > and makes the code that uses it easier to read. > >> enum usb_device_speed usb2_speed; >> >> struct extcon_dev *edev; >> @@ -91,6 +105,7 @@ struct dwc3_qcom { >> bool pm_suspended; >> struct icc_path *icc_path_ddr; >> struct icc_path *icc_path_apps; >> + u8 num_ports; > > Any reason not to keep this one closer to the ports array? > >> }; > [...] >> - irq = dwc3_qcom_get_irq(pdev, "ss_phy_irq", >> - pdata ? pdata->ss_phy_irq_index : -1); >> - if (irq > 0) { >> - ret = dwc3_qcom_request_irq(qcom, irq, "ss_phy_irq"); >> - if (ret) >> - return ret; >> - qcom->ss_phy_irq = irq; >> + for (i = 0; i < irq_count; i++) { >> + irq_index = dwc3_qcom_get_irq_index(irq_names[i]); >> + if (irq_index == -1) { >> + dev_err(&pdev->dev, "Unknown interrupt-name \"%s\" found\n", irq_names[i]); > > This is now spamming the logs with errors like > > dwc3-qcom a6f8800.usb: Unknown interrupt-name "pwr_event" found > > which is clearly just broken. > >> + continue; >> + } >> + port_index = dwc3_qcom_get_port_index(irq_names[i], irq_index); >> + if (port_index == -1) { >> + dev_err(&pdev->dev, "Invalid interrupt-name suffix \"%s\"\n", irq_names[i]); >> + continue; >> + } >> + >> + acpi_index = dwc3_qcom_get_acpi_index(qcom, irq_index, port_index); >> + >> + irq = dwc3_qcom_get_irq(pdev, irq_names[i], acpi_index); >> + if (irq > 0) { >> + ret = dwc3_qcom_request_irq(qcom, irq, irq_names[i]); >> + if (ret) >> + return ret; >> + >> + switch (irq_index) { >> + case DP_HS_PHY_IRQ_INDEX: >> + qcom->port_info[port_index - 1].dp_hs_phy_irq = irq; >> + break; >> + case DM_HS_PHY_IRQ_INDEX: >> + qcom->port_info[port_index - 1].dm_hs_phy_irq = irq; >> + break; >> + case SS_PHY_IRQ_INDEX: >> + qcom->port_info[port_index - 1].ss_phy_irq = irq; >> + break; >> + case QUSB2_PHY_IRQ_INDEX: >> + qcom->qusb2_phy_irq = irq; >> + break; >> + } >> + >> + if (qcom->num_ports < port_index) >> + qcom->num_ports = port_index; >> + } >> } > > Why don't you add a port helper for fetching the interrupts instead? > > There are multiple ways that you can use to determine if this is a > multiport controller or not; you can use OF match data, or simply look > at one of the interrupts that would always be there for a multiport > (or single port) controller (e.g. "dp_hs_phy_1"). > > You can even determine the number of ports first by parsing the > interrupts names and looking for the highest port number. > > Then you can iterate over the ports and parse the interrupts for each > port in turn, which should allow for a much cleaner and less > error-prone implementation. > With [1] merged, I think I can use your suggestion of going through and checking if dp_hs_phy_X is present or not and if present, it is multiport and go through dp_hs_phy_{1/2/3/4} to see the num of ports present. That must simplify the code and make it clean as well. [1]: https://lore.kernel.org/all/20240305093216.3814787-1-quic_kriskura@quicinc.com/ Regards, Krishna,
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c index 08df29584366..a20d63a791bd 100644 --- a/drivers/usb/dwc3/dwc3-qcom.c +++ b/drivers/usb/dwc3/dwc3-qcom.c @@ -53,17 +53,33 @@ #define APPS_USB_AVG_BW 0 #define APPS_USB_PEAK_BW MBps_to_icc(40) +#define NUM_PHY_IRQ 4 + +enum dwc3_qcom_phy_index { + DP_HS_PHY_IRQ_INDEX, + DM_HS_PHY_IRQ_INDEX, + SS_PHY_IRQ_INDEX, + QUSB2_PHY_IRQ_INDEX, +}; + struct dwc3_acpi_pdata { u32 qscratch_base_offset; u32 qscratch_base_size; u32 dwc3_core_base_size; - int qusb2_phy_irq_index; - int dp_hs_phy_irq_index; - int dm_hs_phy_irq_index; - int ss_phy_irq_index; + /* + * The phy_irq_index corresponds to ACPI indexes of (in order) + * DP/DM/SS/QUSB2 IRQ's respectively. + */ + int phy_irq_index[NUM_PHY_IRQ]; bool is_urs; }; +struct dwc3_qcom_port { + int dp_hs_phy_irq; + int dm_hs_phy_irq; + int ss_phy_irq; +}; + struct dwc3_qcom { struct device *dev; void __iomem *qscratch_base; @@ -74,9 +90,7 @@ struct dwc3_qcom { struct reset_control *resets; int qusb2_phy_irq; - int dp_hs_phy_irq; - int dm_hs_phy_irq; - int ss_phy_irq; + struct dwc3_qcom_port port_info[DWC3_MAX_PORTS]; enum usb_device_speed usb2_speed; struct extcon_dev *edev; @@ -91,6 +105,7 @@ struct dwc3_qcom { bool pm_suspended; struct icc_path *icc_path_ddr; struct icc_path *icc_path_apps; + u8 num_ports; }; static inline void dwc3_qcom_setbits(void __iomem *base, u32 offset, u32 val) @@ -375,16 +390,16 @@ static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom) 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); + dwc3_qcom_disable_wakeup_irq(qcom->port_info[0].dm_hs_phy_irq); } else if ((qcom->usb2_speed == USB_SPEED_HIGH) || (qcom->usb2_speed == USB_SPEED_FULL)) { - dwc3_qcom_disable_wakeup_irq(qcom->dp_hs_phy_irq); + dwc3_qcom_disable_wakeup_irq(qcom->port_info[0].dp_hs_phy_irq); } else { - dwc3_qcom_disable_wakeup_irq(qcom->dp_hs_phy_irq); - dwc3_qcom_disable_wakeup_irq(qcom->dm_hs_phy_irq); + dwc3_qcom_disable_wakeup_irq(qcom->port_info[0].dp_hs_phy_irq); + dwc3_qcom_disable_wakeup_irq(qcom->port_info[0].dm_hs_phy_irq); } - dwc3_qcom_disable_wakeup_irq(qcom->ss_phy_irq); + dwc3_qcom_disable_wakeup_irq(qcom->port_info[0].ss_phy_irq); } static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom) @@ -401,20 +416,20 @@ static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom) */ if (qcom->usb2_speed == USB_SPEED_LOW) { - dwc3_qcom_enable_wakeup_irq(qcom->dm_hs_phy_irq, - IRQ_TYPE_EDGE_FALLING); + dwc3_qcom_enable_wakeup_irq(qcom->port_info[0].dm_hs_phy_irq, + IRQ_TYPE_EDGE_FALLING); } else if ((qcom->usb2_speed == USB_SPEED_HIGH) || (qcom->usb2_speed == USB_SPEED_FULL)) { - dwc3_qcom_enable_wakeup_irq(qcom->dp_hs_phy_irq, - IRQ_TYPE_EDGE_FALLING); + dwc3_qcom_enable_wakeup_irq(qcom->port_info[0].dp_hs_phy_irq, + IRQ_TYPE_EDGE_FALLING); } else { - dwc3_qcom_enable_wakeup_irq(qcom->dp_hs_phy_irq, - IRQ_TYPE_EDGE_RISING); - dwc3_qcom_enable_wakeup_irq(qcom->dm_hs_phy_irq, - IRQ_TYPE_EDGE_RISING); + dwc3_qcom_enable_wakeup_irq(qcom->port_info[0].dp_hs_phy_irq, + IRQ_TYPE_EDGE_RISING); + dwc3_qcom_enable_wakeup_irq(qcom->port_info[0].dm_hs_phy_irq, + IRQ_TYPE_EDGE_RISING); } - dwc3_qcom_enable_wakeup_irq(qcom->ss_phy_irq, 0); + dwc3_qcom_enable_wakeup_irq(qcom->port_info[0].ss_phy_irq, 0); } static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup) @@ -535,6 +550,74 @@ static int dwc3_qcom_get_irq(struct platform_device *pdev, return ret; } +static int dwc3_qcom_get_irq_index(const char *irq_name) +{ + /* + * Parse IRQ index based on prefixes from interrupt name. + * Return -1 incase of an invalid interrupt name. + */ + int irq_index = -1; + + if (strncmp(irq_name, "dp_hs_phy", strlen("dp_hs_phy")) == 0) + irq_index = DP_HS_PHY_IRQ_INDEX; + else if (strncmp(irq_name, "dm_hs_phy", strlen("dm_hs_phy")) == 0) + irq_index = DM_HS_PHY_IRQ_INDEX; + else if (strncmp(irq_name, "ss_phy", strlen("ss_phy")) == 0) + irq_index = SS_PHY_IRQ_INDEX; + else if (strncmp(irq_name, "qusb2_phy", strlen("qusb2_phy")) == 0) + irq_index = QUSB2_PHY_IRQ_INDEX; + return irq_index; +} + +static int dwc3_qcom_get_port_index(const char *irq_name, int irq_index) +{ + int port_index = -1; + + switch (irq_index) { + case DP_HS_PHY_IRQ_INDEX: + if (strcmp(irq_name, "dp_hs_phy_irq") == 0) + port_index = 1; + else + sscanf(irq_name, "dp_hs_phy_%d", &port_index); + break; + case DM_HS_PHY_IRQ_INDEX: + if (strcmp(irq_name, "dm_hs_phy_irq") == 0) + port_index = 1; + else + sscanf(irq_name, "dm_hs_phy_%d", &port_index); + break; + case SS_PHY_IRQ_INDEX: + if (strcmp(irq_name, "ss_phy_irq") == 0) + port_index = 1; + else + sscanf(irq_name, "ss_phy_%d", &port_index); + break; + case QUSB2_PHY_IRQ_INDEX: + port_index = 1; + break; + } + + if (port_index <= 0 || port_index > DWC3_MAX_PORTS) + port_index = -1; + + return port_index; +} + +static int dwc3_qcom_get_acpi_index(struct dwc3_qcom *qcom, int irq_index, + int port_index) +{ + const struct dwc3_acpi_pdata *pdata = qcom->acpi_pdata; + + /* + * Currently multiport supported targets don't have an ACPI variant. + * So return -1 if we are not dealing with first port of the controller. + */ + if (!pdata || port_index != 1) + return -1; + + return pdata->phy_irq_index[irq_index]; +} + static int dwc3_qcom_request_irq(struct dwc3_qcom *qcom, int irq, const char *name) { @@ -554,44 +637,67 @@ static int dwc3_qcom_request_irq(struct dwc3_qcom *qcom, int irq, static int dwc3_qcom_setup_irq(struct platform_device *pdev) { struct dwc3_qcom *qcom = platform_get_drvdata(pdev); - const struct dwc3_acpi_pdata *pdata = qcom->acpi_pdata; + struct device_node *np = pdev->dev.of_node; + const char **irq_names; + int port_index; + int acpi_index; + int irq_count; + int irq_index; int irq; int ret; + int i; - irq = dwc3_qcom_get_irq(pdev, "qusb2_phy", - pdata ? pdata->qusb2_phy_irq_index : -1); - if (irq > 0) { - ret = dwc3_qcom_request_irq(qcom, irq, "hs_phy_irq"); - if (ret) - return ret; - qcom->qusb2_phy_irq = irq; - } + irq_count = of_property_count_strings(np, "interrupt-names"); + if (irq_count < 0) + return -EINVAL; - irq = dwc3_qcom_get_irq(pdev, "dp_hs_phy_irq", - pdata ? pdata->dp_hs_phy_irq_index : -1); - if (irq > 0) { - ret = dwc3_qcom_request_irq(qcom, irq, "dp_hs_phy_irq"); - if (ret) - return ret; - qcom->dp_hs_phy_irq = irq; - } + irq_names = devm_kcalloc(&pdev->dev, irq_count, sizeof(*irq_names), GFP_KERNEL); + if (!irq_names) + return -ENOMEM; - irq = dwc3_qcom_get_irq(pdev, "dm_hs_phy_irq", - pdata ? pdata->dm_hs_phy_irq_index : -1); - if (irq > 0) { - ret = dwc3_qcom_request_irq(qcom, irq, "dm_hs_phy_irq"); - if (ret) - return ret; - qcom->dm_hs_phy_irq = irq; - } + ret = of_property_read_string_array(np, "interrupt-names", + irq_names, irq_count); + if (!ret) + return ret; - irq = dwc3_qcom_get_irq(pdev, "ss_phy_irq", - pdata ? pdata->ss_phy_irq_index : -1); - if (irq > 0) { - ret = dwc3_qcom_request_irq(qcom, irq, "ss_phy_irq"); - if (ret) - return ret; - qcom->ss_phy_irq = irq; + for (i = 0; i < irq_count; i++) { + irq_index = dwc3_qcom_get_irq_index(irq_names[i]); + if (irq_index == -1) { + dev_err(&pdev->dev, "Unknown interrupt-name \"%s\" found\n", irq_names[i]); + continue; + } + port_index = dwc3_qcom_get_port_index(irq_names[i], irq_index); + if (port_index == -1) { + dev_err(&pdev->dev, "Invalid interrupt-name suffix \"%s\"\n", irq_names[i]); + continue; + } + + acpi_index = dwc3_qcom_get_acpi_index(qcom, irq_index, port_index); + + irq = dwc3_qcom_get_irq(pdev, irq_names[i], acpi_index); + if (irq > 0) { + ret = dwc3_qcom_request_irq(qcom, irq, irq_names[i]); + if (ret) + return ret; + + switch (irq_index) { + case DP_HS_PHY_IRQ_INDEX: + qcom->port_info[port_index - 1].dp_hs_phy_irq = irq; + break; + case DM_HS_PHY_IRQ_INDEX: + qcom->port_info[port_index - 1].dm_hs_phy_irq = irq; + break; + case SS_PHY_IRQ_INDEX: + qcom->port_info[port_index - 1].ss_phy_irq = irq; + break; + case QUSB2_PHY_IRQ_INDEX: + qcom->qusb2_phy_irq = irq; + break; + } + + if (qcom->num_ports < port_index) + qcom->num_ports = port_index; + } } return 0; @@ -1053,20 +1159,14 @@ 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, - .qusb2_phy_irq_index = 1, - .dp_hs_phy_irq_index = 4, - .dm_hs_phy_irq_index = 3, - .ss_phy_irq_index = 2 + .phy_irq_index = {4, 3, 2, 1}, }; 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, - .qusb2_phy_irq_index = 1, - .dp_hs_phy_irq_index = 4, - .dm_hs_phy_irq_index = 3, - .ss_phy_irq_index = 2, + .phy_irq_index = {4, 3, 2, 1}, .is_urs = true, };