Re: [PATCH v10 06/11] usb: dwc3: qcom: Refactor IRQ handling in QCOM Glue driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 8/11/2023 10:35 PM, Konrad Dybcio wrote:
+
+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.
Can we not just reuse the associated interrupt-names from the devicetree
if present?

Hi Konrad,

  Thanks for the comments but one more confirmation.
We can read the interrupts from DT but I believe the compatible would still need to stay. We need the num_ports information not just for registering interrupts but for modifying the pwr_event_irq registers during suspend/resume. If we rely on the interrupts to find the number of ports, the user is free to remove any IRQ and we might end up in a situation where glue and core are not having same view of how many number of ports present. So I believe its best to keep the compatible and get num_ports info from there and rely on reading interrupt-names to get interrupts cleanly. Can you let me know your view on the same.
So is "is it okay to add SoC-specific compatibles and add the port number in
match data" what you're asking?

If so, that doesn't seem right.

The user should not "feel free to remove any IRQ", modifying the devicetree to
depict a subset of the hardware is not something we want to support. The driver
has to work with the "full" description in accordance with the bindings.


Hi Konrad, Johan,

Thanks for the suggestions / review comments and taking the time out for reviewing them. Pushed v11 implementing the above suggestion of identifying port_count by reading through the interrupt-names. Please help let know your review on the same.

Regards,
Krishna,



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux