Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Message-ID: <1fbb9812-ca50-42eb-95af-1f7c8a2714e3@quicinc.com>
Date:   Thu, 14 Dec 2023 16:24:01 +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:     Johan Hovold <johan@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
CC:     Rob Herring <robh+dt@kernel.org>,
        Wesley Cheng <quic_wcheng@quicinc.com>,
        <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>
 <c5d85c84-3783-4262-a379-1f28e13ae4ce@quicinc.com>
 <ZXrRS7O0Cv1sAJdk@hovoldconsulting.com>
Content-Language: en-US
From:   Krishna Kurapati PSSNV <quic_kriskura@quicinc.com>
In-Reply-To: <ZXrRS7O0Cv1sAJdk@hovoldconsulting.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 12/14/2023 3:26 PM, Johan Hovold wrote:
> On Wed, Dec 13, 2023 at 09:48:57PM +0530, Krishna Kurapati PSSNV wrote:
>> 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:
> 
>>>> Classiffy SoC's into four groups based on whether qusb2_phy interrupt
> 
> typo: Classify
> 
>>>> 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 ?
> 
> Since all SoCs have the pwr_event (HS) interrupt, but not all
> controllers have the SS PHY interrupt, this would prevent expressing
> that the SS PHY is optional by keeping it last in the binding schema and
> making sure that minItem = maxItems - 1.
> 
> And as we discussed, the aim here is to group the three classes of SoCs
> (qusb2, qusb2+, femto) and fix the order of these interrupts once and
> for all so that random reorderings, renames and omissions do not make it
> into the bindings next time someone grabs a downstream DT and sends it
> upstream.
> 

Hi Krzysztof,

  One more reason is that all targets do have a pwr_event interrupts for 
sure and ss_phy is optional as Johan mentioned. So with this reasoning, 
can we put pwr_event first followed by others and push ss_phy to the end 
of list ?

Regards,
Krishna,