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
