[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <02428901-1941-f104-e55b-d197d589a93f@quicinc.com>
Date: Mon, 17 Jul 2023 20:45:14 +0530
From: Krishna Kurapati PSSNV <quic_kriskura@...cinc.com>
To: Johan Hovold <johan@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
<quic_jackp@...cinc.com>, Wesley Cheng <quic_wcheng@...cinc.com>
CC: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
Andy Gross <agross@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Felipe Balbi <balbi@...nel.org>, <linux-usb@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
<devicetree@...r.kernel.org>, <quic_pkondeti@...cinc.com>,
<quic_ppratap@...cinc.com>, <ahalaney@...hat.com>,
<quic_shazhuss@...cinc.com>
Subject: Re: [PATCH v9 06/10] usb: dwc3: qcom: Add support to read IRQ's
related to multiport
On 7/16/2023 12:31 AM, Krishna Kurapati PSSNV wrote:
>
>
> On 7/14/2023 4:10 PM, Krishna Kurapati PSSNV wrote:
>>
>>
>> On 7/14/2023 2:35 PM, Johan Hovold wrote:
>>> On Wed, Jul 12, 2023 at 11:56:33PM +0530, Krishna Kurapati PSSNV wrote:
>>>> On 7/12/2023 5:42 PM, Johan Hovold wrote:
>>>>> On Wed, Jun 21, 2023 at 10:06:24AM +0530, Krishna Kurapati wrote:
>>>>>> Add support to read Multiport IRQ's related to quad port controller
>>>>>> of SA8295 Device.
>>>>>>
>>>>>> Signed-off-by: Krishna Kurapati <quic_kriskura@...cinc.com>
>>>>>> ---
>>>>>> drivers/usb/dwc3/dwc3-qcom.c | 108
>>>>>> +++++++++++++++++++++++++++++------
>>>>>> 1 file changed, 91 insertions(+), 17 deletions(-)
>>>>>
>>>>>> +static int dwc3_qcom_setup_mp_irq(struct platform_device *pdev)
>>>>>> +{
>>>>>> + struct dwc3_qcom *qcom = platform_get_drvdata(pdev);
>>>>>> + char irq_name[15];
>>>>>
>>>>> The interrupt device-name string can not be allocated on the stack or
>>>>> reused as it is stored directly in each irqaction structure.
>>>>>
>>>>> This can otherwise lead to random crashes when accessing
>>>>> /proc/interrupts:
>>>>>
>>>>> https://lore.kernel.org/lkml/ZK6IV_jJPICX5r53@hovoldconsulting.com/
>>>
>>>> Sure, will create a static array of names if possible in global
>>>> section of file and use it to read interrupts.
>>>
>>> Or just use devm_kasprintf(), which should allow for a cleaner
>>> implementation.
>>>
>>> I've fixed it up like this for my X13s wip branches:
>>>
>>> https://github.com/jhovold/linux/commit/0898b54456bc2f4bd4d420480db98e6758771ace
>>>> Are you fine with seperating out setup_irq and setup_mp_irq
>>>> functions
>>>> ? Can you please review comments and suggestion on [1].
>>>
>>> I haven't had time to look at your latest replies yet, but as I already
>>> said when reviewing v9, it seems you should be using a common helper for
>>> non-mp and mp.
>>>
>> Hi Johan,
>>
>> The gist of my mail was to see if I can defer qcom probe when dwc3
>> probe fails/or doesn't happen on of_plat_pop (which is logical) so
>> that we can move setup_irq to after dwc3_register_core so that we know
>> whether we are MP capable or not. This would help us move all IRQ
>> reading into one function.
>>
> Hi Johan,
>
> I see it is difficult to write a common helper. To do so, we need to
> know whether the device is MP capable or not in advance. And since it is
> not possible to know it before of_plat_pop is done, I see only few ways
> to do it:
>
> 1. Based on qcom node compatible string, I can read whether the device
> is MP capable or not and get IRQ's accordingly.
>
> 2. Read the port_info in advance but it needs me to go through some DT
> props and try getting this info. Or read xhci regs like we are doing in
> core (which is not good). Also since some Dt props can be missing, is it
> difficult to get the MP capability info before of_plat_pop is done.
>
> 3. Remove IRQ handling completely. Just because the device has IRQ's
> present, I don't see a point in adding them to bindings, and because we
> added them to bindings, we are making a patch to read them (and since
> this is a little challenging, the whole of multiport series is blocked
> although I don't need wakeup support on these interrupts right away).
>
> Can't we let the rest of the patches go through and let interrupt
> handling for 2nd, 3rd and 4rth ports be taken care later ? I am asking
> this because I want the rest of the patches which are in good shape now
> (after fixing the nits mentioned) to get merged atleast. I will make
> sure to add interrupt handling later in a different series once this is
> merged once I send v10.
>
> Or if there is a simpler way to do it, I would be happy to take any
> suggestions and complete this missing part in this series itself.
>
Hi Konrad, Bjorn,
Can you also let me know your review on this. Can we add a compatible
data to dwc3-qcom to identify whether we need to read MP irq's or non-NP
irq's and also if it is better to split this series into two. (One for
multiport and one for dwc3-qcom IRQ's)
Regards,
Krishna,
Powered by blists - more mailing lists