Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Date:   Thu, 7 Dec 2023 17:13:01 +0100
From:   Johan Hovold <johan@kernel.org>
To:     Krishna Kurapati PSSNV <quic_kriskura@quicinc.com>
Cc:     Thinh Nguyen <Thinh.Nguyen@synopsys.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Wesley Cheng <quic_wcheng@quicinc.com>,
        Conor Dooley <conor+dt@kernel.org>,
        cros-qcom-dts-watchers@chromium.org, linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        devicetree@vger.kernel.org, quic_ppratap@quicinc.com,
        quic_jackp@quicinc.com
Subject: Re: [PATCH v2 1/6] dt-bindings: usb: dwc3: Clean up hs_phy_irq in
 bindings
Message-ID: <ZXHvDaeoG1SuZ9xj@hovoldconsulting.com>
References: <20231204100950.28712-1-quic_kriskura@quicinc.com>
 <20231204100950.28712-2-quic_kriskura@quicinc.com>
 <ZXHjXGEbdtbCiOck@hovoldconsulting.com>
 <028097f3-9056-4c07-a868-4eeac9bc8c94@quicinc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <028097f3-9056-4c07-a868-4eeac9bc8c94@quicinc.com>
Precedence: bulk

On Thu, Dec 07, 2023 at 09:14:55PM +0530, Krishna Kurapati PSSNV wrote:
 
> >> +        - qusb2_phy:: 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.
> >> +        - {dp/dm}_hs_phy_irq:: These IRQ's directly reflect changes on the DP/
> >> +                               DM pads of the SoC. These are used for wakeup
> >> +                               only on SoCs with non-QUSBb2 targets with
> > 
> > QUSB2 typo
> > 
> >> +                               exception of SDM670/SDM845/SM6350.
> >> +        - ss_phy_irq:: When in super speed mode of operation, interrupts are
> > 
> > Capitalise 'Super Speed'
> > 
> >> +                       received when a wakeup event is received on ss_phy_irq.
> > 
> > The description as it stands sounds circular. And this one is only used
> > for remote wakeup right?
> > 
> Yes. It is used for remote wakeup. Mentioning it as wakeup event should 
> be changed ?

It would be good to clarify that this one is the IIUC not used for
connect/disconnect events but just for remote wakeup, that is, unlike
the qusb2_phy and dp/dm_hs_phy interrupts.

The old descriptions just vaguely said "wakeup event" and
connect/disconnect events aren't necessarily wakeup events.

> > Also have you set up the tools so that you can verify your bindings
> > before posing them? I assume the above wouldn't pass (e.g. due to the
> > "(optional)" strings).
> > 
> > There's some more details here:
> > 
> > 	https://docs.kernel.org/devicetree/bindings/writing-schema.html
> > 
> > under "Running checks".
> 
> I did do a dt-binding check and got the following line as well:
> 
>    DTC_CHK Documentation/devicetree/bindings/usb/qcom,dwc3.example.dtb
> /local/mnt/workspace/sriramd/upstream/torvalds/linux/Documentation/devicetree/bindings/usb/qcom,dwc3.example.dtb: 
> usb@a6f8800: interrupt-names:4: 'ss_phy_irq (optional)' was expected
>          From schema:

Good that you got that set up.

Johan