Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Message-ID: <c5d85c84-3783-4262-a379-1f28e13ae4ce@quicinc.com>
Date:   Wed, 13 Dec 2023 21:48:57 +0530
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3 1/2] dt-bindings: usb: dwc3: Clean up hs_phy_irq in
 bindings
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Wesley Cheng <quic_wcheng@quicinc.com>,
        Johan Hovold <johan@kernel.org>
CC:     <linux-usb@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-arm-msm@vger.kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Andy Gross <agross@kernel.org>,
        "Conor Dooley" <conor+dt@kernel.org>, <devicetree@vger.kernel.org>,
        Thinh Nguyen <Thinh.Nguyen@synopsys.com>,
        <quic_ppratap@quicinc.com>, <quic_jackp@quicinc.com>,
        Bjorn Andersson <andersson@kernel.org>
References: <20231211121124.4194-1-quic_kriskura@quicinc.com>
 <20231211121124.4194-2-quic_kriskura@quicinc.com>
 <24fb0b25-0139-4370-864c-839ae931f847@linaro.org>
Content-Language: en-US
From:   Krishna Kurapati PSSNV <quic_kriskura@quicinc.com>
In-Reply-To: <24fb0b25-0139-4370-864c-839ae931f847@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 12/13/2023 12:45 PM, Krzysztof Kozlowski wrote:
> On 11/12/2023 13:11, Krishna Kurapati wrote:
>> The high speed related interrupts present on QC targets are as follows:
>>
>> dp/dm irq's
>> These IRQ's directly reflect changes on the DP/DM pads of the SoC. These
>> are used as wakeup interrupts only on SoCs with non-QUSB2 targets with
>> exception of SDM670/SDM845/SM6350.
>>
>> qusb2_phy irq
>> SoCs with QUSB2 PHY do not have separate DP/DM IRQs and expose only a
>> single IRQ whose behavior can be modified by the QUSB2PHY_INTR_CTRL
>> register. The required DPSE/DMSE configuration is done in
>> QUSB2PHY_INTR_CTRL register of phy address space.
>>
>> hs_phy_irq
>> This is completely different from the above two and is present on all
>> targets with exception of a few IPQ ones. The interrupt is not enabled by
>> default and its functionality is mutually exclusive of qusb2_phy on QUSB
>> targets and DP/DM on femto phy targets.
>>
>> The DTs of several QUSB2 PHY based SoCs incorrectly define "hs_phy_irq"
>> when they should have been "qusb2_phy_irq". On Femto phy targets, the
>> "hs_phy_irq" mentioned is either the actual "hs_phy_irq" or "pwr_event",
>> neither of which would never be triggered directly are non-functional
>> currently. The implementation tries to clean up this issue by addressing
>> the discrepencies involved and fixing the hs_phy_irq's in respective DT's.
>>
>> Classiffy SoC's into four groups based on whether qusb2_phy interrupt
>> or {dp/dm}_hs_phy_irq is used for wakeup in high speed and whether the
>> SoCs have hs_phy_irq present in them or not.
>>
>> The ss_phy_irq is optional interrupt because there are mutliple SoC's
>> which either support only High Speed or there are multiple controllers
>> within same Soc and the secondary controller is High Speed only capable.
>>
>> This breaks ABI on targets running older kernels, but since the interrupt
>> definitions are given wrong on many targets and to establish proper rules
>> for usage of DWC3 interrupts on Qualcomm platforms, DT binding update is
>> necessary.
> 
> This still does not explain why missing property has to be added as
> first one, causing huge reordering of everything here and in DTS.
> 
> If pwr_event is required and we already break the ABI, reduce the impact
> of the change by putting it after all required interrupts. Otherwise
> please explain here and in commit msg why different approach is taken.
> 

Hi Krzysztof. I don't know much about the effect of the ordering on ABI. 
I will try to learn up on it. Would the series be good if we just move 
the pwr_event to the end and keep everything in v3 as it is, and push v4 
for now ?

Regards,
Krishna,