lkml.org 
[lkml]   [2023]   [Oct]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v13 05/10] usb: dwc3: qcom: Refactor IRQ handling in QCOM Glue driver
From


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@quicinc.com/
>
> 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,

\
 
 \ /
  Last update: 2023-10-23 13:25    [W:0.231 / U:0.120 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site