[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZLo9flzTCha5iU-K@hovoldconsulting.com>
Date: Fri, 21 Jul 2023 10:10:38 +0200
From: Johan Hovold <johan@...nel.org>
To: Krishna Kurapati PSSNV <quic_kriskura@...cinc.com>
Cc: Rob Herring <robh+dt@...nel.org>,
Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
Andy Gross <agross@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Felipe Balbi <balbi@...nel.org>,
Wesley Cheng <quic_wcheng@...cinc.com>,
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,
quic_jackp@...cinc.com, quic_harshq@...cinc.com,
ahalaney@...hat.com, quic_shazhuss@...cinc.com
Subject: Re: [PATCH v9 01/10] dt-bindings: usb: qcom,dwc3: Add bindings for
SC8280 Multiport
On Mon, Jul 03, 2023 at 12:41:59AM +0530, Krishna Kurapati PSSNV wrote:
> On 6/27/2023 9:08 PM, Johan Hovold wrote:
> > On Tue, Jun 27, 2023 at 01:20:59PM +0200, Johan Hovold wrote:
> >> On Wed, Jun 21, 2023 at 10:06:19AM +0530, Krishna Kurapati wrote:
> >
> >>> + items:
> >>> + - const: dp1_hs_phy_irq
> >>> + - const: dm1_hs_phy_irq
> >>> + - const: dp2_hs_phy_irq
> >>> + - const: dm2_hs_phy_irq
> >>> + - const: dp3_hs_phy_irq
> >>> + - const: dm4_hs_phy_irq
> >>> + - const: dp4_hs_phy_irq
> >>> + - const: dm4_hs_phy_irq
> >>> + - const: ss1_phy_irq
> >>> + - const: ss2_phy_irq
> >>> + - const: pwr_event_1
> >>> + - const: pwr_event_2
> >>> + - const: pwr_event_3
> >>> + - const: pwr_event_4
> >>
> >> The naming here is inconsistent and interrupts should not have "_irq"
> >> suffixes (even if some of the current ones do for historical reasons).
> >>
> >> I believe these should be named
> >>
> >> pwr_event_1
> >> dp_hs_phy_1
> >> dm_hs_phy_1
> >> ss_phy_1
> >>
> >> pwr_event_2
> >> dp_hs_phy_2
> >> dm_hs_phy_2
> >> ss_phy_2
> >>
> >> pwr_event_3
> >> dp_hs_phy_3
> >> dm_hs_phy_3
> >>
> >> pwr_event_4
> >> dp_hs_phy_4
> >> dm_hs_phy_4
> >>
> >> or similar and be grouped by port while using the the
> >> qcom,sc8280xp-dwc ordering for the individual lines.
> >
> > Perhaps the ordering you suggested is fine too, but I'd probably move
> > the pwr_event ones first to match qcom,sc8280xp-dwc then, that is:
> >
> > pwr_event_1
> > pwr_event_2
> > pwr_event_3
> > pwr_event_4
> > dp_hs_phy_1
> > dm_hs_phy_1
> > dp_hs_phy_2
> > dm_hs_phy_2
> > dp_hs_phy_3
> > dm_hs_phy_3
> > dp_hs_phy_4
> > dm_hs_phy_4
> > ss_phy_1
> > ss_phy_2
> >
> > so we have them grouped as pwr_event followed by HS and with SS last.
> >
> >> Side note: Please note how the above interrupt properties can also be
> >> used to infer the number of HS and SS ports.
> Can't we just cleanup all at once later ? Might not be a good idea for
> some properties in the file to have _irq and for some to not have it. I
> will modify the order though.
No, DT bindings generally need to be as correct as possible from the
start as they form an ABI. So please drop the _irq suffix from all of
the new indexed names.
Johan
Powered by blists - more mailing lists