Messages in this thread | | | Date | Tue, 8 Aug 2023 14:02:21 +0530 | Subject | Re: [PATCH v10 06/11] usb: dwc3: qcom: Refactor IRQ handling in QCOM Glue driver | From | Krishna Kurapati PSSNV <> |
| |
+ >> +enum dwc3_qcom_phy_irq_identifier { >> + HS_PHY_IRQ = 0, >> + DP_HS_PHY_IRQ, >> + DM_HS_PHY_IRQ, >> + SS_PHY_IRQ, >> }; > > This enum is unused. >
Hi Bjorn,
I didn't use the enum directly, but used its members in the get_port_irq call below.
> [..] >> +static int dwc3_get_acpi_index(const struct dwc3_acpi_pdata *pdata, int irq_index) >> +{ >> + int acpi_index = -1; >> + >> + if (!pdata) >> + return -1; >> + >> + if (irq_index == DP_HS_PHY_IRQ) >> + acpi_index = pdata->dp_hs_phy_irq_index; >> + else if (irq_index == DM_HS_PHY_IRQ) >> + acpi_index = pdata->dm_hs_phy_irq_index; >> + else if (irq_index == SS_PHY_IRQ) >> + acpi_index = pdata->ss_phy_irq_index; > > It looks favourable to put these in an array, instead of having to pull > them out of 4 different variables conditionally. > >> + >> + return acpi_index; >> +} >> + >> +static int dwc3_get_port_irq(struct platform_device *pdev, u8 port_index) >> +{ >> + struct dwc3_qcom *qcom = platform_get_drvdata(pdev); >> + bool is_mp_supported = (qcom->data->num_ports > 1) ? true : false; >> + const struct dwc3_acpi_pdata *pdata = qcom->acpi_pdata; >> + char *disp_name; >> + int acpi_index; >> + char *dt_name; >> + int ret; >> + int irq; >> + int i; >> + >> + /* >> + * We need to read only DP/DM/SS IRQ's here. >> + * So loop over from 1->3 and accordingly modify respective phy_irq[]. >> + */ >> + for (i = 1; i < MAX_PHY_IRQ; i++) { >> + >> + if (!is_mp_supported && (port_index == 0)) { >> + if (i == DP_HS_PHY_IRQ) { >> + dt_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, >> + "dp_hs_phy_irq"); >> + disp_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, >> + "qcom_dwc3 DP_HS"); >> + } else if (i == DM_HS_PHY_IRQ) { >> + dt_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, >> + "dm_hs_phy_irq"); >> + disp_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, >> + "qcom_dwc3 DM_HS"); >> + } else if (i == SS_PHY_IRQ) { >> + dt_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, >> + "ss_phy_irq"); >> + disp_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, >> + "qcom_dwc3 SS"); Bjorn, Konrad,
If we are to remove this repetitive loops, we might need to make a 2D array for all of Dp/Dm/Ss interrutps and make a global array of names to be used for irq lookup and use them to reduce the if-else-if stuff here. If that is fine, I can make those changes, else I would like to stick to this approach for now because if we don't add the global array of names, prepping them seperately for dp/dm/ss would again lead us to making if-else loops like above.
Please let me know your thoughts on this.
Regards, Krishna,
| |