| Message ID | 20230115114146.12628-1-quic_kriskura@quicinc.com |
|---|---|
| Headers | show |
| Series | Add multiport support for DWC3 controllers | expand |
On Sun, Jan 15, 2023 at 05:11:42PM +0530, Krishna Kurapati wrote: > Add bindings to indicate properties required to support multiport > on Snps Dwc3 controller. > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > --- > .../devicetree/bindings/usb/snps,dwc3.yaml | 53 ++++++++++++++++--- > 1 file changed, 47 insertions(+), 6 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > index 6d78048c4613..3ea051beb2f8 100644 > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > @@ -81,15 +81,26 @@ properties: > > phys: > minItems: 1 > - maxItems: 2 > + maxItems: 8 > > phy-names: > minItems: 1 > - maxItems: 2 > - items: > - enum: > - - usb2-phy > - - usb3-phy > + maxItems: 8 > + oneOf: > + - items: > + enum: > + - usb2-phy > + - usb3-phy > + - items: > + enum: > + - usb2-phy_port0 > + - usb2-phy_port1 > + - usb2-phy_port2 > + - usb2-phy_port3 > + - usb3-phy_port0 > + - usb3-phy_port1 > + - usb3-phy_port2 > + - usb3-phy_port3 usbN-portM > > resets: > minItems: 1 > @@ -360,6 +371,22 @@ properties: > description: > Enable USB remote wakeup. > > + num-ports: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + This property indicates the number of ports present on the target that > + are to be serviced by the DWC3 controller. > + minimum: 1 > + maximum: 4 > + > + num-ss-ports: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + This property indicates the number of SS capable ports present on the > + target that are to be serviced by the DWC3 controller. > + minimum: 1 > + maximum: 4 This information is redundant. 'phy-names' tells you how many ports of each. > + > unevaluatedProperties: false > > required: > @@ -388,4 +415,18 @@ examples: > snps,dis_u2_susphy_quirk; > snps,dis_enblslpm_quirk; > }; > + - | > + usb@4a000000 { > + compatible = "snps,dwc3"; > + reg = <0x4a000000 0xcfff>; > + interrupts = <0 92 4>; > + clocks = <&clk 1>, <&clk 2>, <&clk 3>; > + clock-names = "bus_early", "ref", "suspend"; > + num-ports = <2>; > + num-ss-ports = <1>; > + phys = <&usb2_phy0>, <&usb3_phy0>, <&usb2_phy1>; > + phy-names = "usb2-phy_port0", "usb3-phy_port0", "usb2-phy_port1"; > + snps,dis_u2_susphy_quirk; > + snps,dis_enblslpm_quirk; > + }; Does a different number of phys really need its own example? Rob
On 1/16/2023 10:04 PM, Rob Herring wrote: > On Sun, Jan 15, 2023 at 05:11:42PM +0530, Krishna Kurapati wrote: >> Add bindings to indicate properties required to support multiport >> on Snps Dwc3 controller. >> >> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> >> --- >> .../devicetree/bindings/usb/snps,dwc3.yaml | 53 ++++++++++++++++--- >> 1 file changed, 47 insertions(+), 6 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >> index 6d78048c4613..3ea051beb2f8 100644 >> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >> @@ -81,15 +81,26 @@ properties: >> >> phys: >> minItems: 1 >> - maxItems: 2 >> + maxItems: 8 >> >> phy-names: >> minItems: 1 >> - maxItems: 2 >> - items: >> - enum: >> - - usb2-phy >> - - usb3-phy >> + maxItems: 8 >> + oneOf: >> + - items: >> + enum: >> + - usb2-phy >> + - usb3-phy >> + - items: >> + enum: >> + - usb2-phy_port0 >> + - usb2-phy_port1 >> + - usb2-phy_port2 >> + - usb2-phy_port3 >> + - usb3-phy_port0 >> + - usb3-phy_port1 >> + - usb3-phy_port2 >> + - usb3-phy_port3 > > usbN-portM > >> >> resets: >> minItems: 1 >> @@ -360,6 +371,22 @@ properties: >> description: >> Enable USB remote wakeup. >> >> + num-ports: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: >> + This property indicates the number of ports present on the target that >> + are to be serviced by the DWC3 controller. >> + minimum: 1 >> + maximum: 4 >> + >> + num-ss-ports: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: >> + This property indicates the number of SS capable ports present on the >> + target that are to be serviced by the DWC3 controller. >> + minimum: 1 >> + maximum: 4 > > This information is redundant. 'phy-names' tells you how many ports of > each. > Hi Rob, Thanks for the review. The reason I wanted to introduce two more variables is to get info on number of ports and ss-capable ports present on hardware whether or not the user provides them in DTSI file. In the code there are two types of per port / per phy operations: a) Modifying GUSB2PFYCFG and GUSB3PIPECTL registers per phy. b) Generic Phy operations - per phy. In today's code, if someone doesn't mention the SSPHY in DTSI, dwc->usb3_generic_phy will be NULL and any call to phy operations will just bail out. And irrespective of whether we provide SS Phy in DTSI or not, we still configure GUSB3PIPECTL register. Consider the following cases: 1. There are 3 ports and 2 of them are SS capable and all phy's are mentioned in DTSI. phy-names= "usb2-port0", "usb3-port0", "usb2-port1", "usb3-port1", "usb2-port2" When we count them in the driver, we get num ports as 3 (presuming num-ports = num of hs ports) and num-ss-ports = 2. Since there is no ambiguity in which all ports to configure, we can modify GUSB2PHYCFG registers for all 3 HS Phy's and GUSB3PIPECTL for both SS Phy's. This is a proper scenario. 2. If the user skips providing SS Phy on Port-0, then: phy-names= "usb2-port0", "usb2-port1", "usb3-port1", "usb2-port2" If we count the phys, we end up getting num-ports=3 and num-ss-ports=1. Since in the driver code, we are not keeping track of which ports are SS capable and which ones are not, we end up configuring GUSB2PIPECTL(port-0) instead of port-1 as the num-ss-ports is "1" which is incorrect. 3. If the user skips providing one complete port, in this case port-1 is skipped, then: phy-names= "usb2-port0", "usb3-port0", "usb2-port2" If we count the phys, we end up getting num-ports=2 and num-ss-ports=1. Since in the driver code, we are not keeping track of which ports are SS capable and which ones are not, we end up configuring GUSB2PHYCFG(port-0) and GUSB2PHYCFG(port-1) instead of port-2 which is incorrect. To avoid these scenarios, if we can get the exact number of SS Ports and Ports in total present on the HW, we can configure all the registers whether the phy's are provided in DTSI or not. (This is of no harm I believe as it still works in today's code) Incase the 2nd and 3rd scenarios are not allowed and user *MUST* declare all the phy's in the DTSI, then I can go ahead and remove these properties and count them in the driver code. Thanks, Krishna, >> + >> unevaluatedProperties: false >> >> required: >> @@ -388,4 +415,18 @@ examples: >> snps,dis_u2_susphy_quirk; >> snps,dis_enblslpm_quirk; >> }; >> + - | >> + usb@4a000000 { >> + compatible = "snps,dwc3"; >> + reg = <0x4a000000 0xcfff>; >> + interrupts = <0 92 4>; >> + clocks = <&clk 1>, <&clk 2>, <&clk 3>; >> + clock-names = "bus_early", "ref", "suspend"; >> + num-ports = <2>; >> + num-ss-ports = <1>; >> + phys = <&usb2_phy0>, <&usb3_phy0>, <&usb2_phy1>; >> + phy-names = "usb2-phy_port0", "usb3-phy_port0", "usb2-phy_port1"; >> + snps,dis_u2_susphy_quirk; >> + snps,dis_enblslpm_quirk; >> + }; > > Does a different number of phys really need its own example? > > Rob
On 17/01/2023 10:01, Krishna Kurapati PSSNV wrote: > > > On 1/16/2023 10:04 PM, Rob Herring wrote: >> On Sun, Jan 15, 2023 at 05:11:42PM +0530, Krishna Kurapati wrote: >>> Add bindings to indicate properties required to support multiport >>> on Snps Dwc3 controller. >>> >>> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> >>> --- >>> .../devicetree/bindings/usb/snps,dwc3.yaml | 53 ++++++++++++++++--- >>> 1 file changed, 47 insertions(+), 6 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >>> index 6d78048c4613..3ea051beb2f8 100644 >>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >>> @@ -81,15 +81,26 @@ properties: >>> >>> phys: >>> minItems: 1 >>> - maxItems: 2 >>> + maxItems: 8 >>> >>> phy-names: >>> minItems: 1 >>> - maxItems: 2 >>> - items: >>> - enum: >>> - - usb2-phy >>> - - usb3-phy >>> + maxItems: 8 >>> + oneOf: >>> + - items: >>> + enum: >>> + - usb2-phy >>> + - usb3-phy >>> + - items: >>> + enum: >>> + - usb2-phy_port0 >>> + - usb2-phy_port1 >>> + - usb2-phy_port2 >>> + - usb2-phy_port3 >>> + - usb3-phy_port0 >>> + - usb3-phy_port1 >>> + - usb3-phy_port2 >>> + - usb3-phy_port3 >> >> usbN-portM >> >>> >>> resets: >>> minItems: 1 >>> @@ -360,6 +371,22 @@ properties: >>> description: >>> Enable USB remote wakeup. >>> >>> + num-ports: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: >>> + This property indicates the number of ports present on the target that >>> + are to be serviced by the DWC3 controller. >>> + minimum: 1 >>> + maximum: 4 >>> + >>> + num-ss-ports: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: >>> + This property indicates the number of SS capable ports present on the >>> + target that are to be serviced by the DWC3 controller. >>> + minimum: 1 >>> + maximum: 4 >> >> This information is redundant. 'phy-names' tells you how many ports of >> each. >> > Hi Rob, > > Thanks for the review. The reason I wanted to introduce two more > variables is to get info on number of ports and ss-capable ports > present on hardware whether or not the user provides them in DTSI file. > > In the code there are two types of per port / per phy operations: > a) Modifying GUSB2PFYCFG and GUSB3PIPECTL registers per phy. > b) Generic Phy operations - per phy. > > In today's code, if someone doesn't mention the SSPHY in DTSI, > dwc->usb3_generic_phy will be NULL and any call to phy operations will > just bail out. And irrespective of whether we provide SS Phy in DTSI or > not, we still configure GUSB3PIPECTL register. > > Consider the following cases: > > 1. There are 3 ports and 2 of them are SS capable and all phy's are > mentioned in DTSI. > > phy-names= "usb2-port0", "usb3-port0", "usb2-port1", "usb3-port1", > "usb2-port2" > > When we count them in the driver, we get num ports as 3 (presuming > num-ports = num of hs ports) and num-ss-ports = 2. > > Since there is no ambiguity in which all ports to configure, we can > modify GUSB2PHYCFG registers for all 3 HS Phy's and GUSB3PIPECTL for > both SS Phy's. > This is a proper scenario. > > 2. If the user skips providing SS Phy on Port-0, then: > > phy-names= "usb2-port0", "usb2-port1", "usb3-port1", "usb2-port2" > > If we count the phys, we end up getting num-ports=3 and num-ss-ports=1. > > Since in the driver code, we are not keeping track of which ports are SS > capable and which ones are not, we end up configuring > GUSB2PIPECTL(port-0) instead of port-1 as the num-ss-ports is "1" which > is incorrect. > > 3. If the user skips providing one complete port, in this case port-1 is > skipped, then: > > phy-names= "usb2-port0", "usb3-port0", "usb2-port2" > > If we count the phys, we end up getting num-ports=2 and num-ss-ports=1. > > Since in the driver code, we are not keeping track of which ports are SS > capable and which ones are not, we end up configuring > GUSB2PHYCFG(port-0) and GUSB2PHYCFG(port-1) instead of port-2 which is > incorrect. Why? You know you have port-2 from the phy name, so why would you ignore this information? > > To avoid these scenarios, if we can get the exact number of SS Ports and > Ports in total present on the HW, we can configure all the registers > whether the phy's are provided in DTSI or not. (This is of no harm I > believe as it still works in today's code) Doesn't the driver know how many phys it has in such case through respective compatible? > > Incase the 2nd and 3rd scenarios are not allowed and user *MUST* declare > all the phy's in the DTSI, then I can go ahead and remove these > properties and count them in the driver code. Why you cannot then configure all phys in the driver all ports as some safe default and then customize it depending on the actual port used? Best regards, Krzysztof
On 1/17/2023 4:32 PM, Krzysztof Kozlowski wrote: > On 17/01/2023 10:01, Krishna Kurapati PSSNV wrote: >> >> >> On 1/16/2023 10:04 PM, Rob Herring wrote: >>> On Sun, Jan 15, 2023 at 05:11:42PM +0530, Krishna Kurapati wrote: >>>> Add bindings to indicate properties required to support multiport >>>> on Snps Dwc3 controller. >>>> >>>> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> >>>> --- >>>> .../devicetree/bindings/usb/snps,dwc3.yaml | 53 ++++++++++++++++--- >>>> 1 file changed, 47 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >>>> index 6d78048c4613..3ea051beb2f8 100644 >>>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >>>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >>>> @@ -81,15 +81,26 @@ properties: >>>> >>>> phys: >>>> minItems: 1 >>>> - maxItems: 2 >>>> + maxItems: 8 >>>> >>>> phy-names: >>>> minItems: 1 >>>> - maxItems: 2 >>>> - items: >>>> - enum: >>>> - - usb2-phy >>>> - - usb3-phy >>>> + maxItems: 8 >>>> + oneOf: >>>> + - items: >>>> + enum: >>>> + - usb2-phy >>>> + - usb3-phy >>>> + - items: >>>> + enum: >>>> + - usb2-phy_port0 >>>> + - usb2-phy_port1 >>>> + - usb2-phy_port2 >>>> + - usb2-phy_port3 >>>> + - usb3-phy_port0 >>>> + - usb3-phy_port1 >>>> + - usb3-phy_port2 >>>> + - usb3-phy_port3 >>> >>> usbN-portM >>> >>>> >>>> resets: >>>> minItems: 1 >>>> @@ -360,6 +371,22 @@ properties: >>>> description: >>>> Enable USB remote wakeup. >>>> >>>> + num-ports: >>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>> + description: >>>> + This property indicates the number of ports present on the target that >>>> + are to be serviced by the DWC3 controller. >>>> + minimum: 1 >>>> + maximum: 4 >>>> + >>>> + num-ss-ports: >>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>> + description: >>>> + This property indicates the number of SS capable ports present on the >>>> + target that are to be serviced by the DWC3 controller. >>>> + minimum: 1 >>>> + maximum: 4 >>> >>> This information is redundant. 'phy-names' tells you how many ports of >>> each. >>> >> Hi Rob, >> >> Thanks for the review. The reason I wanted to introduce two more >> variables is to get info on number of ports and ss-capable ports >> present on hardware whether or not the user provides them in DTSI file. >> >> In the code there are two types of per port / per phy operations: >> a) Modifying GUSB2PFYCFG and GUSB3PIPECTL registers per phy. >> b) Generic Phy operations - per phy. >> >> In today's code, if someone doesn't mention the SSPHY in DTSI, >> dwc->usb3_generic_phy will be NULL and any call to phy operations will >> just bail out. And irrespective of whether we provide SS Phy in DTSI or >> not, we still configure GUSB3PIPECTL register. >> >> Consider the following cases: >> >> 1. There are 3 ports and 2 of them are SS capable and all phy's are >> mentioned in DTSI. >> >> phy-names= "usb2-port0", "usb3-port0", "usb2-port1", "usb3-port1", >> "usb2-port2" >> >> When we count them in the driver, we get num ports as 3 (presuming >> num-ports = num of hs ports) and num-ss-ports = 2. >> >> Since there is no ambiguity in which all ports to configure, we can >> modify GUSB2PHYCFG registers for all 3 HS Phy's and GUSB3PIPECTL for >> both SS Phy's. >> This is a proper scenario. >> >> 2. If the user skips providing SS Phy on Port-0, then: >> >> phy-names= "usb2-port0", "usb2-port1", "usb3-port1", "usb2-port2" >> >> If we count the phys, we end up getting num-ports=3 and num-ss-ports=1. >> >> Since in the driver code, we are not keeping track of which ports are SS >> capable and which ones are not, we end up configuring >> GUSB2PIPECTL(port-0) instead of port-1 as the num-ss-ports is "1" which >> is incorrect. >> >> 3. If the user skips providing one complete port, in this case port-1 is >> skipped, then: >> >> phy-names= "usb2-port0", "usb3-port0", "usb2-port2" >> >> If we count the phys, we end up getting num-ports=2 and num-ss-ports=1. >> >> Since in the driver code, we are not keeping track of which ports are SS >> capable and which ones are not, we end up configuring >> GUSB2PHYCFG(port-0) and GUSB2PHYCFG(port-1) instead of port-2 which is >> incorrect. > > Why? You know you have port-2 from the phy name, so why would you ignore > this information? > Hi Krzysztof, Thanks for the review, The concern I had with that approach is that, we need to keep track of per port supported speeds in some array /flags and check them whenever we are modifying the dwc3-phy registers. This makes the code a little unreadable. >> >> To avoid these scenarios, if we can get the exact number of SS Ports and >> Ports in total present on the HW, we can configure all the registers >> whether the phy's are provided in DTSI or not. (This is of no harm I >> believe as it still works in today's code) > > Doesn't the driver know how many phys it has in such case through > respective compatible? > The core driver has only one compatible currently "snps,dwc3". Are you suggesting to add new compatible to driver core in case any multiport device is being used and get this info from there ? >> >> Incase the 2nd and 3rd scenarios are not allowed and user *MUST* declare >> all the phy's in the DTSI, then I can go ahead and remove these >> properties and count them in the driver code. > > > Why you cannot then configure all phys in the driver all ports as some > safe default and then customize it depending on the actual port used? > To do this, we still need to get info on number of hs/ss phy's present on hardware and currently there is no register I believe in DWC3 core global address space that can give this info. I see that HCSPARAMS1 Reg gives some info but that is not accessible from driver core. According to databook: "Number of Ports (MaxPorts) -> Number of ports implemented is defined by the parameter (`DWC_USB3_HOST_NUM_U2_ROOT_PORTS + `DWC_USB3_HOST_NUM_U3_ROOT_PORTS) -> Number of ports enabled is controlled by the core input signals host_num_u2_port[3:0]+host_num_u3_port[3:0]" Regards, Krishna,
On Sun, Jan 15, 2023 at 05:11:42PM +0530, Krishna Kurapati wrote: > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml [..] > phy-names: > minItems: 1 > - maxItems: 2 > - items: > - enum: > - - usb2-phy > - - usb3-phy > + maxItems: 8 > + oneOf: > + - items: > + enum: > + - usb2-phy > + - usb3-phy > + - items: > + enum: > + - usb2-phy_port0 > + - usb2-phy_port1 > + - usb2-phy_port2 > + - usb2-phy_port3 > + - usb3-phy_port0 > + - usb3-phy_port1 > + - usb3-phy_port2 > + - usb3-phy_port3 How about expressing this as: oneOf: - items: enum: [ usb2-phy, usb3-phy ] - items: pattern: "^usb[23]-phy_port[0-3]$" Regards, Bjorn
On Sun, Jan 15, 2023 at 05:11:46PM +0530, Krishna Kurapati wrote: > Add USB and DWC3 node for teritiary port of SC8280 along > with multiport IRQ's and phy's. > Very nice. Please make the subject prefix "arm64: dts: qcom: sc8280xp:", to match other changes in the sc8280xp.dtsi. > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > --- > arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 49 +++++++++++++++++++ > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 60 ++++++++++++++++++++++++ > 2 files changed, 109 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts > index 84cb6f3eeb56..f9eb854c3444 100644 > --- a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts > +++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts > @@ -422,6 +422,20 @@ &usb_1_qmpphy { > status = "okay"; > }; > > +&usb_2 { > + status = "okay"; Please the status property last in the node. > + > + pinctrl-names = "default"; > + pinctrl-0 = <&usb2_en_state>, > + <&usb3_en_state>, > + <&usb4_en_state>, > + <&usb5_en_state>; > +}; > + > +&usb_2_dwc3 { > + dr_mode = "host"; > +}; > + > &usb_2_hsphy0 { > vdda-pll-supply = <&vreg_l5a>; > vdda18-supply = <&vreg_l7g>; > @@ -472,6 +486,41 @@ &xo_board_clk { > clock-frequency = <38400000>; > }; > > +/* PINCTRL */ No need to repeat this comment, its purpose is to indicate that nodes above are sorted alphabetically and pinctrl-related nodes are kept here at the end of the file. Please place your nodes below the existing /* PINCTRL */ comment below. Thanks, Bjorn > +&pm8450c_gpios { > + usb2_en_state: usb2-en-state { > + pins = "gpio9"; > + function = "normal"; > + output-high; > + power-source = <0>; > + }; > +}; > + > +&pm8450e_gpios { > + usb3_en_state: usb3-en-state { > + pins = "gpio5"; > + function = "normal"; > + output-high; > + power-source = <0>; > + }; > +}; > + > +&pm8450g_gpios { > + usb4_en_state: usb4-en-state { > + pins = "gpio5"; > + function = "normal"; > + output-high; > + power-source = <0>; > + }; > + > + usb5_en_state: usb5-en-state { > + pins = "gpio9"; > + function = "normal"; > + output-high; > + power-source = <0>; > + }; > +}; > + > /* PINCTRL */ > > &tlmm { > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > index 109c9d2b684d..e9866ab5c6e2 100644 > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > @@ -1969,6 +1969,66 @@ usb_1_dwc3: usb@a800000 { > }; > }; > > + usb_2: usb@a4f8800 { > + compatible = "qcom,sc8280xp-dwc3", "qcom,dwc3"; > + reg = <0 0x0a4f8800 0 0x400>; > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + clocks = <&gcc GCC_CFG_NOC_USB3_MP_AXI_CLK>, > + <&gcc GCC_USB30_MP_MASTER_CLK>, > + <&gcc GCC_AGGRE_USB3_MP_AXI_CLK>, > + <&gcc GCC_USB30_MP_SLEEP_CLK>, > + <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>, > + <&gcc GCC_AGGRE_USB_NOC_AXI_CLK>, > + <&gcc GCC_AGGRE_USB_NOC_NORTH_AXI_CLK>, > + <&gcc GCC_AGGRE_USB_NOC_SOUTH_AXI_CLK>, > + <&gcc GCC_SYS_NOC_USB_AXI_CLK>; > + clock-names = "cfg_noc", "core", "iface", "sleep", "mock_utmi", > + "noc_aggr", "noc_aggr_north", "noc_aggr_south", "noc_sys"; > + > + assigned-clocks = <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>, > + <&gcc GCC_USB30_MP_MASTER_CLK>; > + assigned-clock-rates = <19200000>, <200000000>; > + > + interrupts-extended = <&pdc 127 IRQ_TYPE_EDGE_RISING>, > + <&pdc 126 IRQ_TYPE_EDGE_RISING>, > + <&pdc 16 IRQ_TYPE_LEVEL_HIGH>; > + > + interrupt-names = "dp_hs_phy_irq", "dm_hs_phy_irq", > + "ss_phy_irq"; > + > + power-domains = <&gcc USB30_MP_GDSC>; > + > + resets = <&gcc GCC_USB30_MP_BCR>; > + > + interconnects = <&aggre1_noc MASTER_USB3_1 0 &mc_virt SLAVE_EBI1 0>, > + <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_USB3_1 0>; > + interconnect-names = "usb-ddr", "apps-usb"; > + > + required-opps = <&rpmhpd_opp_nom>; > + > + status = "disabled"; > + > + usb_2_dwc3: usb@a400000 { > + compatible = "snps,dwc3"; > + reg = <0 0x0a400000 0 0xcd00>; > + interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>; > + iommus = <&apps_smmu 0x800 0x0>; > + num-ports = <4>; > + num-ss-ports = <2>; > + phys = <&usb_2_hsphy0>, <&usb_2_qmpphy0>, > + <&usb_2_hsphy1>, <&usb_2_qmpphy1>, > + <&usb_2_hsphy2>, > + <&usb_2_hsphy3>; > + phy-names = "usb2-phy_port0", "usb3-phy_port0", > + "usb2-phy_port1", "usb3-phy_port1", > + "usb2-phy_port2", > + "usb2-phy_port3"; > + }; > + }; > + > pdc: interrupt-controller@b220000 { > compatible = "qcom,sc8280xp-pdc", "qcom,pdc"; > reg = <0 0x0b220000 0 0x30000>, <0 0x17c000f0 0 0x60>; > -- > 2.39.0 >
On 1/18/2023 11:58 PM, Bjorn Andersson wrote: > On Sun, Jan 15, 2023 at 05:11:46PM +0530, Krishna Kurapati wrote: >> Add USB and DWC3 node for teritiary port of SC8280 along >> with multiport IRQ's and phy's. >> > > Very nice. > > Please make the subject prefix "arm64: dts: qcom: sc8280xp:", to match > other changes in the sc8280xp.dtsi. >> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> >> --- >> arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 49 +++++++++++++++++++ >> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 60 ++++++++++++++++++++++++ >> 2 files changed, 109 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts >> index 84cb6f3eeb56..f9eb854c3444 100644 >> --- a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts >> +++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts >> @@ -422,6 +422,20 @@ &usb_1_qmpphy { >> status = "okay"; >> }; >> >> +&usb_2 { >> + status = "okay"; > > Please the status property last in the node. > >> + >> + pinctrl-names = "default"; >> + pinctrl-0 = <&usb2_en_state>, >> + <&usb3_en_state>, >> + <&usb4_en_state>, >> + <&usb5_en_state>; >> +}; >> + >> +&usb_2_dwc3 { >> + dr_mode = "host"; >> +}; >> + >> &usb_2_hsphy0 { >> vdda-pll-supply = <&vreg_l5a>; >> vdda18-supply = <&vreg_l7g>; >> @@ -472,6 +486,41 @@ &xo_board_clk { >> clock-frequency = <38400000>; >> }; >> >> +/* PINCTRL */ > > No need to repeat this comment, its purpose is to indicate that nodes > above are sorted alphabetically and pinctrl-related nodes are kept here > at the end of the file. Please place your nodes below the existing /* > PINCTRL */ comment below. > > Thanks, > Bjorn > >> +&pm8450c_gpios { >> + usb2_en_state: usb2-en-state { >> + pins = "gpio9"; >> + function = "normal"; >> + output-high; >> + power-source = <0>; >> + }; >> +}; >> + >> +&pm8450e_gpios { >> + usb3_en_state: usb3-en-state { >> + pins = "gpio5"; >> + function = "normal"; >> + output-high; >> + power-source = <0>; >> + }; >> +}; >> + >> +&pm8450g_gpios { >> + usb4_en_state: usb4-en-state { >> + pins = "gpio5"; >> + function = "normal"; >> + output-high; >> + power-source = <0>; >> + }; >> + >> + usb5_en_state: usb5-en-state { >> + pins = "gpio9"; >> + function = "normal"; >> + output-high; >> + power-source = <0>; >> + }; >> +}; >> + >> /* PINCTRL */ >> >> &tlmm { >> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >> index 109c9d2b684d..e9866ab5c6e2 100644 >> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >> @@ -1969,6 +1969,66 @@ usb_1_dwc3: usb@a800000 { >> }; >> }; >> >> + usb_2: usb@a4f8800 { >> + compatible = "qcom,sc8280xp-dwc3", "qcom,dwc3"; >> + reg = <0 0x0a4f8800 0 0x400>; >> + #address-cells = <2>; >> + #size-cells = <2>; >> + ranges; >> + >> + clocks = <&gcc GCC_CFG_NOC_USB3_MP_AXI_CLK>, >> + <&gcc GCC_USB30_MP_MASTER_CLK>, >> + <&gcc GCC_AGGRE_USB3_MP_AXI_CLK>, >> + <&gcc GCC_USB30_MP_SLEEP_CLK>, >> + <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>, >> + <&gcc GCC_AGGRE_USB_NOC_AXI_CLK>, >> + <&gcc GCC_AGGRE_USB_NOC_NORTH_AXI_CLK>, >> + <&gcc GCC_AGGRE_USB_NOC_SOUTH_AXI_CLK>, >> + <&gcc GCC_SYS_NOC_USB_AXI_CLK>; >> + clock-names = "cfg_noc", "core", "iface", "sleep", "mock_utmi", >> + "noc_aggr", "noc_aggr_north", "noc_aggr_south", "noc_sys"; >> + >> + assigned-clocks = <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>, >> + <&gcc GCC_USB30_MP_MASTER_CLK>; >> + assigned-clock-rates = <19200000>, <200000000>; >> + >> + interrupts-extended = <&pdc 127 IRQ_TYPE_EDGE_RISING>, >> + <&pdc 126 IRQ_TYPE_EDGE_RISING>, >> + <&pdc 16 IRQ_TYPE_LEVEL_HIGH>; >> + >> + interrupt-names = "dp_hs_phy_irq", "dm_hs_phy_irq", >> + "ss_phy_irq"; >> + >> + power-domains = <&gcc USB30_MP_GDSC>; >> + >> + resets = <&gcc GCC_USB30_MP_BCR>; >> + >> + interconnects = <&aggre1_noc MASTER_USB3_1 0 &mc_virt SLAVE_EBI1 0>, >> + <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_USB3_1 0>; >> + interconnect-names = "usb-ddr", "apps-usb"; >> + >> + required-opps = <&rpmhpd_opp_nom>; >> + >> + status = "disabled"; >> + >> + usb_2_dwc3: usb@a400000 { >> + compatible = "snps,dwc3"; >> + reg = <0 0x0a400000 0 0xcd00>; >> + interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>; >> + iommus = <&apps_smmu 0x800 0x0>; >> + num-ports = <4>; >> + num-ss-ports = <2>; >> + phys = <&usb_2_hsphy0>, <&usb_2_qmpphy0>, >> + <&usb_2_hsphy1>, <&usb_2_qmpphy1>, >> + <&usb_2_hsphy2>, >> + <&usb_2_hsphy3>; >> + phy-names = "usb2-phy_port0", "usb3-phy_port0", >> + "usb2-phy_port1", "usb3-phy_port1", >> + "usb2-phy_port2", >> + "usb2-phy_port3"; >> + }; >> + }; >> + >> pdc: interrupt-controller@b220000 { >> compatible = "qcom,sc8280xp-pdc", "qcom,pdc"; >> reg = <0 0x0b220000 0 0x30000>, <0 0x17c000f0 0 0x60>; >> -- >> 2.39.0 >> Hi Bjorn, Thanks for the review. Will make sure to incorporate these changes in the next version. Regards, Krishna,
On Sun, Jan 15, 2023, Krishna Kurapati wrote: > Multiport controllers being host-only capable do not have GEVNTADDR > HI/LO, SIZE, COUNT reigsters present. Accsesing them to setup event I think you should reword "present" to something else. They're still present, but those registers are to be set while operating in device mode. The rest looks fine. Thanks, Thinh > buffers during core_init can cause an SMMU Fault. Avoid event buffers > setup if the GHWPARAMS0 tells that the controller is host-only. > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > --- > drivers/usb/dwc3/core.c | 23 +++++++++++++++-------- > 1 file changed, 15 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 7e0a9a598dfd..f61ebddaecc0 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -871,9 +871,12 @@ static void dwc3_clk_disable(struct dwc3 *dwc) > > static void dwc3_core_exit(struct dwc3 *dwc) > { > - int i; > + int i; > + unsigned int hw_mode; > > - dwc3_event_buffers_cleanup(dwc); > + hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0); > + if (hw_mode != DWC3_GHWPARAMS0_MODE_HOST) > + dwc3_event_buffers_cleanup(dwc); > > usb_phy_set_suspend(dwc->usb2_phy, 1); > usb_phy_set_suspend(dwc->usb3_phy, 1); > @@ -1246,10 +1249,12 @@ static int dwc3_core_init(struct dwc3 *dwc) > } > } > > - ret = dwc3_event_buffers_setup(dwc); > - if (ret) { > - dev_err(dwc->dev, "failed to setup event buffers\n"); > - goto err4; > + if (hw_mode != DWC3_GHWPARAMS0_MODE_HOST) { > + ret = dwc3_event_buffers_setup(dwc); > + if (ret) { > + dev_err(dwc->dev, "failed to setup event buffers\n"); > + goto err4; > + } > } > > /* > @@ -1886,7 +1891,7 @@ static int dwc3_probe(struct platform_device *pdev) > struct resource *res, dwc_res; > struct dwc3 *dwc; > int ret, i; > - > + unsigned int hw_mode; > void __iomem *regs; > > dwc = devm_kzalloc(dev, sizeof(*dwc), GFP_KERNEL); > @@ -2063,7 +2068,9 @@ static int dwc3_probe(struct platform_device *pdev) > err5: > dwc3_debugfs_exit(dwc); > > - dwc3_event_buffers_cleanup(dwc); > + hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0); > + if (hw_mode != DWC3_GHWPARAMS0_MODE_HOST) > + dwc3_event_buffers_cleanup(dwc); > > usb_phy_set_suspend(dwc->usb2_phy, 1); > usb_phy_set_suspend(dwc->usb3_phy, 1); > -- > 2.39.0 >
Hi Thinh, On Thu, Jan 19, 2023 at 12:38:51AM +0000, Thinh Nguyen wrote: > On Sun, Jan 15, 2023, Krishna Kurapati wrote: > > Multiport controllers being host-only capable do not have GEVNTADDR Multiport may not be relevant here. Host-only is though. > > HI/LO, SIZE, COUNT reigsters present. Accsesing them to setup event > > I think you should reword "present" to something else. They're still > present In our case we have an instance where the IP is statically configured via coreConsultant with DWC_USB31_MODE==1 (host only) and we did observe that none of the registers pertaining to device mode (including GEVNT* and of course all the D* ones) are even *present* in the register map. If we try to access them we encounter some kind of access error or stall (or translation fault as described). So the approach here is to first verify by checking the HWPARAMS0 register if the HW is even capable of device mode in the first place. > but those registers are to be set while operating in device > mode. The rest looks fine. Are you suggesting only touching the GEVNT* registers when *operating* in device mode, even in the case of a dual-role capable controller? In that case would it make more sense to additionally move the calls to dwc3_event_buffers_{setup,cleanup} out of core.c and into dwc3_gadget_{init,exit} perhaps? That way we avoid them completely unless and until we switch into peripheral mode (assuming controller supports that, which we should already have checks for). Moreover, if the devicetree dr_mode property is set to host-only we'd also avoid calling these. > > buffers during core_init can cause an SMMU Fault. Avoid event buffers > > setup if the GHWPARAMS0 tells that the controller is host-only. > > > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > > --- > > drivers/usb/dwc3/core.c | 23 +++++++++++++++-------- > > 1 file changed, 15 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > > index 7e0a9a598dfd..f61ebddaecc0 100644 > > --- a/drivers/usb/dwc3/core.c > > +++ b/drivers/usb/dwc3/core.c > > @@ -871,9 +871,12 @@ static void dwc3_clk_disable(struct dwc3 *dwc) > > > > static void dwc3_core_exit(struct dwc3 *dwc) > > { > > - int i; > > + int i; > > + unsigned int hw_mode; > > > > - dwc3_event_buffers_cleanup(dwc); > > + hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0); > > + if (hw_mode != DWC3_GHWPARAMS0_MODE_HOST) If we stick with this approach, we probably could just check dwc->dr_mode instead as probe should have already set that to be an intersection between the values given in devicetree "dr_mode" and the HWPARAMS0 capability. Thanks, Jack > > + dwc3_event_buffers_cleanup(dwc); > > > > usb_phy_set_suspend(dwc->usb2_phy, 1); > > usb_phy_set_suspend(dwc->usb3_phy, 1); > > @@ -1246,10 +1249,12 @@ static int dwc3_core_init(struct dwc3 *dwc) > > } > > } > > > > - ret = dwc3_event_buffers_setup(dwc); > > - if (ret) { > > - dev_err(dwc->dev, "failed to setup event buffers\n"); > > - goto err4; > > + if (hw_mode != DWC3_GHWPARAMS0_MODE_HOST) { > > + ret = dwc3_event_buffers_setup(dwc); > > + if (ret) { > > + dev_err(dwc->dev, "failed to setup event buffers\n"); > > + goto err4; > > + } > > } > > > > /* > > @@ -1886,7 +1891,7 @@ static int dwc3_probe(struct platform_device *pdev) > > struct resource *res, dwc_res; > > struct dwc3 *dwc; > > int ret, i; > > - > > + unsigned int hw_mode; > > void __iomem *regs; > > > > dwc = devm_kzalloc(dev, sizeof(*dwc), GFP_KERNEL); > > @@ -2063,7 +2068,9 @@ static int dwc3_probe(struct platform_device *pdev) > > err5: > > dwc3_debugfs_exit(dwc); > > > > - dwc3_event_buffers_cleanup(dwc); > > + hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0); > > + if (hw_mode != DWC3_GHWPARAMS0_MODE_HOST) > > + dwc3_event_buffers_cleanup(dwc); > > > > usb_phy_set_suspend(dwc->usb2_phy, 1); > > usb_phy_set_suspend(dwc->usb3_phy, 1); > > -- > > 2.39.0 > >
On Wed, Jan 18, 2023, Jack Pham wrote: > Hi Thinh, > > On Thu, Jan 19, 2023 at 12:38:51AM +0000, Thinh Nguyen wrote: > > On Sun, Jan 15, 2023, Krishna Kurapati wrote: > > > Multiport controllers being host-only capable do not have GEVNTADDR > > Multiport may not be relevant here. Host-only is though. > > > > HI/LO, SIZE, COUNT reigsters present. Accsesing them to setup event > > > > I think you should reword "present" to something else. They're still > > present > > In our case we have an instance where the IP is statically configured > via coreConsultant with DWC_USB31_MODE==1 (host only) and we did observe > that none of the registers pertaining to device mode (including GEVNT* > and of course all the D* ones) are even *present* in the register map. > If we try to access them we encounter some kind of access error or stall > (or translation fault as described). So the approach here is to first > verify by checking the HWPARAMS0 register if the HW is even capable of > device mode in the first place. I see. > > > but those registers are to be set while operating in device > > mode. The rest looks fine. > > Are you suggesting only touching the GEVNT* registers when *operating* > in device mode, even in the case of a dual-role capable controller? In > that case would it make more sense to additionally move the calls to > dwc3_event_buffers_{setup,cleanup} out of core.c and into > dwc3_gadget_{init,exit} perhaps? That way we avoid them completely While it shouldn't be a problem for DRD, it may be cleaner to do that. > unless and until we switch into peripheral mode (assuming controller > supports that, which we should already have checks for). Moreover, if > the devicetree dr_mode property is set to host-only we'd also avoid > calling these. > > > > buffers during core_init can cause an SMMU Fault. Avoid event buffers > > > setup if the GHWPARAMS0 tells that the controller is host-only. > > > > > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > > > --- > > > drivers/usb/dwc3/core.c | 23 +++++++++++++++-------- > > > 1 file changed, 15 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > > > index 7e0a9a598dfd..f61ebddaecc0 100644 > > > --- a/drivers/usb/dwc3/core.c > > > +++ b/drivers/usb/dwc3/core.c > > > @@ -871,9 +871,12 @@ static void dwc3_clk_disable(struct dwc3 *dwc) > > > > > > static void dwc3_core_exit(struct dwc3 *dwc) > > > { > > > - int i; > > > + int i; > > > + unsigned int hw_mode; > > > > > > - dwc3_event_buffers_cleanup(dwc); > > > + hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0); > > > + if (hw_mode != DWC3_GHWPARAMS0_MODE_HOST) > > If we stick with this approach, we probably could just check > dwc->dr_mode instead as probe should have already set that to be an > intersection between the values given in devicetree "dr_mode" and the > HWPARAMS0 capability. > What we have here should not break DRD, so it's fine either way. Thanks, Thinh
On Sun, Jan 15, 2023 at 05:11:41PM +0530, Krishna Kurapati wrote: > Currently the DWC3 driver supports only single port controller which > requires at most two PHYs ie HS and SS PHYs. There are SoCs that has > DWC3 controller with multiple ports that can operate in host mode. > Some of the port supports both SS+HS and other port supports only HS > mode. > > This change primarily refactors the Phy logic in core driver to allow > multiport support with Generic Phy's. > > Chananges have been tested on QCOM SoC SA8295P which has 4 ports (2 > are HS+SS capable and 2 are HS only capable). > I can confirm that applying this series allow me to use all 6 USB ports on the ADP. Looking forward to v5. Thanks, Bjorn > Changes in v4: > Added DT support for SA8295p. > > Changes in v3: > Incase any PHY init fails, then clear/exit the PHYs that > are already initialized. > > Changes in v2: > Changed dwc3_count_phys to return the number of PHY Phandles in the node. > This will be used now in dwc3_extract_num_phys to increment num_usb2_phy > and num_usb3_phy. > > Added new parameter "ss_idx" in dwc3_core_get_phy_ny_node and changed its > structure such that the first half is for HS-PHY and second half is for > SS-PHY. > > In dwc3_core_get_phy, for multiport controller, only if SS-PHY phandle is > present, pass proper SS_IDX else pass -1. > > Link to v3: https://lore.kernel.org/all/1654709787-23686-1-git-send-email-quic_harshq@quicinc.com/#r > Link to v2: https://lore.kernel.org/all/1653560029-6937-1-git-send-email-quic_harshq@quicinc.com/#r > > Krishna Kurapati (5): > dt-bindings: usb: Add bindings to support multiport properties > usb: dwc3: core: Refactor PHY logic to support Multiport Controller > usb: dwc3: core: Do not setup event buffers for host only controllers > usb: dwc3: qcom: Add multiport controller support for qcom wrapper > arm: dts: msm: Add multiport controller node for usb > > .../devicetree/bindings/usb/snps,dwc3.yaml | 42 ++- > arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 49 +++ > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 60 ++++ > drivers/usb/dwc3/core.c | 325 +++++++++++++----- > drivers/usb/dwc3/core.h | 15 +- > drivers/usb/dwc3/drd.c | 14 +- > drivers/usb/dwc3/dwc3-qcom.c | 28 +- > 7 files changed, 429 insertions(+), 104 deletions(-) > > -- > 2.39.0 >
On 1/19/2023 9:13 AM, Bjorn Andersson wrote: > On Sun, Jan 15, 2023 at 05:11:41PM +0530, Krishna Kurapati wrote: >> Currently the DWC3 driver supports only single port controller which >> requires at most two PHYs ie HS and SS PHYs. There are SoCs that has >> DWC3 controller with multiple ports that can operate in host mode. >> Some of the port supports both SS+HS and other port supports only HS >> mode. >> >> This change primarily refactors the Phy logic in core driver to allow >> multiport support with Generic Phy's. >> >> Chananges have been tested on QCOM SoC SA8295P which has 4 ports (2 >> are HS+SS capable and 2 are HS only capable). >> > > I can confirm that applying this series allow me to use all 6 USB ports > on the ADP. Looking forward to v5. > > Thanks, > Bjorn > Thanks Bjorn for testing it out. Regards, Krishna, >> Changes in v4: >> Added DT support for SA8295p. >> >> Changes in v3: >> Incase any PHY init fails, then clear/exit the PHYs that >> are already initialized. >> >> Changes in v2: >> Changed dwc3_count_phys to return the number of PHY Phandles in the node. >> This will be used now in dwc3_extract_num_phys to increment num_usb2_phy >> and num_usb3_phy. >> >> Added new parameter "ss_idx" in dwc3_core_get_phy_ny_node and changed its >> structure such that the first half is for HS-PHY and second half is for >> SS-PHY. >> >> In dwc3_core_get_phy, for multiport controller, only if SS-PHY phandle is >> present, pass proper SS_IDX else pass -1. >> >> Link to v3: https://lore.kernel.org/all/1654709787-23686-1-git-send-email-quic_harshq@quicinc.com/#r >> Link to v2: https://lore.kernel.org/all/1653560029-6937-1-git-send-email-quic_harshq@quicinc.com/#r >> >> Krishna Kurapati (5): >> dt-bindings: usb: Add bindings to support multiport properties >> usb: dwc3: core: Refactor PHY logic to support Multiport Controller >> usb: dwc3: core: Do not setup event buffers for host only controllers >> usb: dwc3: qcom: Add multiport controller support for qcom wrapper >> arm: dts: msm: Add multiport controller node for usb >> >> .../devicetree/bindings/usb/snps,dwc3.yaml | 42 ++- >> arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 49 +++ >> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 60 ++++ >> drivers/usb/dwc3/core.c | 325 +++++++++++++----- >> drivers/usb/dwc3/core.h | 15 +- >> drivers/usb/dwc3/drd.c | 14 +- >> drivers/usb/dwc3/dwc3-qcom.c | 28 +- >> 7 files changed, 429 insertions(+), 104 deletions(-) >> >> -- >> 2.39.0 >>