| Message ID | 20230405125759.4201-1-quic_kriskura@quicinc.com |
|---|---|
| Headers | show |
| Series | Add multiport support for DWC3 controllers | expand |
On Wed, Apr 05, 2023, Krishna Kurapati wrote: > Currently host-only capable DWC3 controllers support Multiport. Temporarily > map XHCI address space for host-only controllers and parse XHCI Extended > Capabilities registers to read number of usb2 ports and usb3 ports present on > multiport controller. Each USB Port is atleast HS capable. > > The port info for usb2 and usb3 phy are identified as num_usb2_ports and > num_usb3_ports. The intention is as follows: > > Wherever we need to perform phy operations like: > > LOOP_OVER_NUMBER_OF_AVAILABLE_PORTS() > { > phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST); > phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST); > } > > If number of usb2 ports is 3, loop can go from index 0-2 for usb2_generic_phy. > If number of usb3-ports is 2, we don't know for sure, if the first 2 ports are > SS capable or some other ports like (2 and 3) are SS capable. So instead, > num_usb2_ports is used to loop around all phy's (both hs and ss) for > performing phy operations. If any usb3_generic_phy turns out to be NULL, phy > operation just bails out. > > num_usb3_ports is used to modify GUSB3PIPECTL registers while setting up > phy's as we need to know how many SS capable ports are there for this. > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > --- > Link to v5: https://urldefense.com/v3/__https://lore.kernel.org/all/20230310163420.7582-3-quic_kriskura@quicinc.com/__;!!A4F2R9G_pg!eXaKJejrT053M_6af46mC8jjyBmdZBBhJ0bVKlyxFiIMR_V1RwlBs_9VdvXhpJuNBDHi0d8kdZDrI0MdFKnKA6VA0i6Fww$ > > drivers/usb/dwc3/core.c | 69 +++++++++++++++++++++++++++++++++++++++++ > drivers/usb/dwc3/core.h | 60 +++++++++++++++++++++++++++++++++++ > 2 files changed, 129 insertions(+) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 476b63618511..567ae79389a1 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -1750,6 +1750,60 @@ static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc) > return edev; > } > > +static int dwc3_read_port_info(struct dwc3 *dwc) > +{ > + void __iomem *regs; > + u32 offset; > + u32 temp; > + u8 major_revision; > + int ret = 0; > + > + /* > + * Remap xHCI address space to access XHCI ext cap regs, > + * since it is needed to get port info. > + */ > + regs = ioremap(dwc->xhci_resources[0].start, > + resource_size(&dwc->xhci_resources[0])); > + if (IS_ERR(regs)) { > + return PTR_ERR(regs); > + } > + > + offset = dwc3_xhci_find_next_ext_cap(regs, 0, > + XHCI_EXT_CAPS_PROTOCOL); > + while (offset) { > + temp = readl(regs + offset); > + major_revision = XHCI_EXT_PORT_MAJOR(temp);; > + > + temp = readl(regs + offset + 0x08); > + if (major_revision == 0x03) { > + dwc->num_usb3_ports += XHCI_EXT_PORT_COUNT(temp); > + } else if (major_revision <= 0x02) { > + dwc->num_usb2_ports += XHCI_EXT_PORT_COUNT(temp); > + } else { > + dev_err(dwc->dev, "port revision seems wrong\n"); > + ret = -EINVAL; > + goto unmap_reg; > + } > + > + offset = dwc3_xhci_find_next_ext_cap(regs, offset, > + XHCI_EXT_CAPS_PROTOCOL); > + } > + > + temp = readl(regs + DWC3_XHCI_HCSPARAMS1); > + if (HCS_MAX_PORTS(temp) != (dwc->num_usb3_ports + dwc->num_usb2_ports)) { > + dev_err(dwc->dev, "inconsistency in port info\n"); > + ret = -EINVAL; > + goto unmap_reg; > + } > + > + dev_dbg(dwc->dev, > + "hs-ports: %d ss-ports: %d\n", dwc->num_usb2_ports, dwc->num_usb3_ports); > + > +unmap_reg: > + iounmap(regs); > + return ret; > +} > + > static int dwc3_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > @@ -1757,6 +1811,7 @@ static int dwc3_probe(struct platform_device *pdev) > struct dwc3 *dwc; > > int ret; > + unsigned int hw_mode; > > void __iomem *regs; > > @@ -1880,6 +1935,20 @@ static int dwc3_probe(struct platform_device *pdev) > goto disable_clks; > } > > + /* > + * Currently DWC3 controllers that are host-only capable > + * support Multiport > + */ > + hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0); > + if (hw_mode == DWC3_GHWPARAMS0_MODE_HOST) { > + ret = dwc3_read_port_info(dwc); > + if (ret) > + goto disable_clks; > + } else { > + dwc->num_usb2_ports = 1; > + dwc->num_usb3_ports = 1; > + } > + > spin_lock_init(&dwc->lock); > mutex_init(&dwc->mutex); > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index 4743e918dcaf..229b7da8c5bc 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -35,6 +35,17 @@ > > #define DWC3_MSG_MAX 500 > > +/* Define XHCI Extcap register offsets for getting multiport info */ > +#define XHCI_HCC_PARAMS_OFFSET 0x10 > +#define DWC3_XHCI_HCSPARAMS1 0x04 > +#define XHCI_EXT_CAPS_PROTOCOL 2 > +#define XHCI_HCC_EXT_CAPS(p) (((p)>>16)&0xffff) > +#define XHCI_EXT_CAPS_ID(p) (((p)>>0)&0xff) > +#define XHCI_EXT_CAPS_NEXT(p) (((p)>>8)&0xff) Fix these spacing and naming inconsistency (x vs p). > +#define XHCI_EXT_PORT_MAJOR(x) (((x) >> 24) & 0xff) > +#define XHCI_EXT_PORT_COUNT(x) (((x) >> 8) & 0xff) > +#define HCS_MAX_PORTS(p) (((p) >> 24) & 0x7f) > + > /* Global constants */ > #define DWC3_PULL_UP_TIMEOUT 500 /* ms */ > #define DWC3_BOUNCE_SIZE 1024 /* size of a superspeed bulk */ > @@ -1023,6 +1034,10 @@ struct dwc3_scratchpad_array { > * @usb_psy: pointer to power supply interface. > * @usb2_phy: pointer to USB2 PHY > * @usb3_phy: pointer to USB3 PHY > + * @num_usb2_ports: Indicates the number of usb2 ports to be serviced by the > + * controller. Can we just say "number of usb2 ports". > + * @num_usb3_ports: Indicates the number of usb3 ports to be serviced by the > + * controller. > * @usb2_generic_phy: pointer to USB2 PHY > * @usb3_generic_phy: pointer to USB3 PHY > * @phys_ready: flag to indicate that PHYs are ready > @@ -1158,6 +1173,8 @@ struct dwc3 { > struct usb_phy *usb2_phy; > struct usb_phy *usb3_phy; > > + u32 num_usb2_ports; > + u32 num_usb3_ports; > struct phy *usb2_generic_phy; > struct phy *usb3_generic_phy; > > @@ -1645,4 +1662,47 @@ static inline void dwc3_ulpi_exit(struct dwc3 *dwc) > { } > #endif > > +/** > + * Find the offset of the extended capabilities with capability ID id. > + * > + * @base PCI MMIO registers base address. > + * @start address at which to start looking, (0 or HCC_PARAMS to start at > + * beginning of list) > + * @id Extended capability ID to search for, or 0 for the next > + * capability > + * > + * Returns the offset of the next matching extended capability structure. > + * Some capabilities can occur several times, e.g., the XHCI_EXT_CAPS_PROTOCOL, > + * and this provides a way to find them all. > + */ Documentation style is different? > + > +static inline int dwc3_xhci_find_next_ext_cap(void __iomem *base, u32 start, int id) > +{ > + u32 val; > + u32 next; > + u32 offset; > + > + offset = start; > + if (!start || start == XHCI_HCC_PARAMS_OFFSET) { > + val = readl(base + XHCI_HCC_PARAMS_OFFSET); > + if (val == ~0) > + return 0; > + offset = XHCI_HCC_EXT_CAPS(val) << 2; > + if (!offset) > + return 0; > + } > + do { > + val = readl(base + offset); > + if (val == ~0) > + return 0; > + if (offset != start && (id == 0 || XHCI_EXT_CAPS_ID(val) == id)) > + return offset; > + > + next = XHCI_EXT_CAPS_NEXT(val); > + offset += next << 2; > + } while (next); > + > + return 0; > +} > + > #endif /* __DRIVERS_USB_DWC3_CORE_H */ > -- > 2.40.0 > My checkpatch reports some errors. Can you check? Thanks, Thinh
On Wed, Apr 05, 2023, 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). > > Changes in v6: > Updated comments in code after. > Updated variables names appropriately as per review comments. > Updated commit text in patch-2 and added additional info as per review > comments. > The patch header in v5 doesn't have "PATHCH v5" notation present. Corrected > it in this version. > > Changes in v5: > Added DT support for first port of Teritiary USB controller on SA8540-Ride > Added support for reading port info from XHCI Extended Params registers. > > Changes in RFC v4: > Added DT support for SA8295p. > > Changes in RFC v3: > Incase any PHY init fails, then clear/exit the PHYs that > are already initialized. > > Changes in RFC 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 v5: https://urldefense.com/v3/__https://lore.kernel.org/all/20230310163420.7582-1-quic_kriskura@quicinc.com/__;!!A4F2R9G_pg!b6YnNIov1GQE0nNkw05sW71n3ZpTM04st-Y-J5ksBUel2ZZfWr9ZA_AE4ZtBAADuCpJ4C4RCr9Di1-fOfqJk1O7oBPDywQ$ > Link to RFC v4: https://urldefense.com/v3/__https://lore.kernel.org/all/20230115114146.12628-1-quic_kriskura@quicinc.com/__;!!A4F2R9G_pg!b6YnNIov1GQE0nNkw05sW71n3ZpTM04st-Y-J5ksBUel2ZZfWr9ZA_AE4ZtBAADuCpJ4C4RCr9Di1-fOfqJk1O5p58Ga7A$ > Link to RFC v3: https://urldefense.com/v3/__https://lore.kernel.org/all/1654709787-23686-1-git-send-email-quic_harshq@quicinc.com/*r__;Iw!!A4F2R9G_pg!b6YnNIov1GQE0nNkw05sW71n3ZpTM04st-Y-J5ksBUel2ZZfWr9ZA_AE4ZtBAADuCpJ4C4RCr9Di1-fOfqJk1O5eLTSOZg$ > Link to RFC v2: https://urldefense.com/v3/__https://lore.kernel.org/all/1653560029-6937-1-git-send-email-quic_harshq@quicinc.com/*r__;Iw!!A4F2R9G_pg!b6YnNIov1GQE0nNkw05sW71n3ZpTM04st-Y-J5ksBUel2ZZfWr9ZA_AE4ZtBAADuCpJ4C4RCr9Di1-fOfqJk1O5CAsP83Q$ > > Krishna Kurapati (8): > dt-bindings: usb: Add bindings for multiport properties on DWC3 > controller > usb: dwc3: core: Access XHCI address space temporarily to read port > info > usb: dwc3: core: Skip setting event buffers for host only controllers > usb: dwc3: core: Refactor PHY logic to support Multiport Controller > usb: dwc3: qcom: Add multiport controller support for qcom wrapper > arm64: dts: qcom: sc8280xp: Add multiport controller node for SC8280 > arm64: dts: qcom: sa8295p: Enable tertiary controller and its 4 USB > ports > arm64: dts: qcom: sa8540-ride: Enable first port of tertiary usb > controller > > .../devicetree/bindings/usb/snps,dwc3.yaml | 13 +- > arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 47 +++ > arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 22 ++ > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 58 +++ > drivers/usb/dwc3/core.c | 373 ++++++++++++++---- > drivers/usb/dwc3/core.h | 71 +++- > drivers/usb/dwc3/drd.c | 13 +- > drivers/usb/dwc3/dwc3-qcom.c | 28 +- > 8 files changed, 523 insertions(+), 102 deletions(-) > > -- > 2.40.0 > Please check if your patches and mailing client. Looks like they are corrupted. Thanks, Thinh
On 08/04/2023 03:42, Thinh Nguyen wrote: >> Krishna Kurapati (8): >> dt-bindings: usb: Add bindings for multiport properties on DWC3 >> controller >> usb: dwc3: core: Access XHCI address space temporarily to read port >> info >> usb: dwc3: core: Skip setting event buffers for host only controllers >> usb: dwc3: core: Refactor PHY logic to support Multiport Controller >> usb: dwc3: qcom: Add multiport controller support for qcom wrapper >> arm64: dts: qcom: sc8280xp: Add multiport controller node for SC8280 >> arm64: dts: qcom: sa8295p: Enable tertiary controller and its 4 USB >> ports >> arm64: dts: qcom: sa8540-ride: Enable first port of tertiary usb >> controller >> >> .../devicetree/bindings/usb/snps,dwc3.yaml | 13 +- >> arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 47 +++ >> arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 22 ++ >> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 58 +++ >> drivers/usb/dwc3/core.c | 373 ++++++++++++++---- >> drivers/usb/dwc3/core.h | 71 +++- >> drivers/usb/dwc3/drd.c | 13 +- >> drivers/usb/dwc3/dwc3-qcom.c | 28 +- >> 8 files changed, 523 insertions(+), 102 deletions(-) >> >> -- >> 2.40.0 >> > > Please check if your patches and mailing client. Looks like they are > corrupted. All patches look from patch-syntax and apply fine. What is exactly corrupted? Best regards, Krzysztof
On Sat, Apr 08, 2023, Krzysztof Kozlowski wrote: > On 08/04/2023 03:42, Thinh Nguyen wrote: > >> Krishna Kurapati (8): > >> dt-bindings: usb: Add bindings for multiport properties on DWC3 > >> controller > >> usb: dwc3: core: Access XHCI address space temporarily to read port > >> info > >> usb: dwc3: core: Skip setting event buffers for host only controllers > >> usb: dwc3: core: Refactor PHY logic to support Multiport Controller > >> usb: dwc3: qcom: Add multiport controller support for qcom wrapper > >> arm64: dts: qcom: sc8280xp: Add multiport controller node for SC8280 > >> arm64: dts: qcom: sa8295p: Enable tertiary controller and its 4 USB > >> ports > >> arm64: dts: qcom: sa8540-ride: Enable first port of tertiary usb > >> controller > >> > >> .../devicetree/bindings/usb/snps,dwc3.yaml | 13 +- > >> arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 47 +++ > >> arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 22 ++ > >> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 58 +++ > >> drivers/usb/dwc3/core.c | 373 ++++++++++++++---- > >> drivers/usb/dwc3/core.h | 71 +++- > >> drivers/usb/dwc3/drd.c | 13 +- > >> drivers/usb/dwc3/dwc3-qcom.c | 28 +- > >> 8 files changed, 523 insertions(+), 102 deletions(-) > >> > >> -- > >> 2.40.0 > >> > > > > Please check if your patches and mailing client. Looks like they are > > corrupted. > > All patches look from patch-syntax and apply fine. What is exactly > corrupted? > Hm... perhaps it's an encoding issue from my mail client then. I get this from my automated checks: <snip> ERROR: spaces required around that '=' (ctx:WxV) #429: FILE: drivers/usb/dwc3/core.h:1697: + if (offset !=3D start && (id =3D=3D 0 || XHCI_EXT_CAPS_ID(val) =3D=3D id= ^ ERROR: spaces required around that '=' (ctx:VxV) #429: FILE: drivers/usb/dwc3/core.h:1697: + if (offset !=3D start && (id =3D=3D 0 || XHCI_EXT_CAPS_ID(val) =3D=3D id= ^ ERROR: spaces required around that '=' (ctx:VxE) #429: FILE: drivers/usb/dwc3/core.h:1697: + if (offset !=3D start && (id =3D=3D 0 || XHCI_EXT_CAPS_ID(val) =3D=3D id= ^ ERROR: do not use assignment in if condition #429: FILE: drivers/usb/dwc3/core.h:1697: + if (offset !=3D start && (id =3D=3D 0 || XHCI_EXT_CAPS_ID(val) =3D=3D id= ERROR: spaces required around that '=' (ctx:WxV) #433: FILE: drivers/usb/dwc3/core.h:1700: + next =3D XHCI_EXT_CAPS_NEXT(val); ^ ERROR: spaces required around that '+=' (ctx:WxV) #434: FILE: drivers/usb/dwc3/core.h:1701: + offset +=3D next << 2; </snip> The "=" gets encoded to =3D, which is strange. It never happened before. I need to check my mail client. Sorry for the noise. Thanks, Thinh
On 09/04/2023 01:09, Thinh Nguyen wrote: > On Sat, Apr 08, 2023, Krzysztof Kozlowski wrote: >> On 08/04/2023 03:42, Thinh Nguyen wrote: >>>> Krishna Kurapati (8): >>>> dt-bindings: usb: Add bindings for multiport properties on DWC3 >>>> controller >>>> usb: dwc3: core: Access XHCI address space temporarily to read port >>>> info >>>> usb: dwc3: core: Skip setting event buffers for host only controllers >>>> usb: dwc3: core: Refactor PHY logic to support Multiport Controller >>>> usb: dwc3: qcom: Add multiport controller support for qcom wrapper >>>> arm64: dts: qcom: sc8280xp: Add multiport controller node for SC8280 >>>> arm64: dts: qcom: sa8295p: Enable tertiary controller and its 4 USB >>>> ports >>>> arm64: dts: qcom: sa8540-ride: Enable first port of tertiary usb >>>> controller >>>> >>>> .../devicetree/bindings/usb/snps,dwc3.yaml | 13 +- >>>> arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 47 +++ >>>> arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 22 ++ >>>> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 58 +++ >>>> drivers/usb/dwc3/core.c | 373 ++++++++++++++---- >>>> drivers/usb/dwc3/core.h | 71 +++- >>>> drivers/usb/dwc3/drd.c | 13 +- >>>> drivers/usb/dwc3/dwc3-qcom.c | 28 +- >>>> 8 files changed, 523 insertions(+), 102 deletions(-) >>>> >>>> -- >>>> 2.40.0 >>>> >>> >>> Please check if your patches and mailing client. Looks like they are >>> corrupted. >> >> All patches look from patch-syntax and apply fine. What is exactly >> corrupted? >> > > Hm... perhaps it's an encoding issue from my mail client then. I get > this from my automated checks: > > <snip> > > ERROR: spaces required around that '=' (ctx:WxV) > #429: FILE: drivers/usb/dwc3/core.h:1697: > + if (offset !=3D start && (id =3D=3D 0 || XHCI_EXT_CAPS_ID(val) =3D=3D id= > ^ > > ERROR: spaces required around that '=' (ctx:VxV) > #429: FILE: drivers/usb/dwc3/core.h:1697: > + if (offset !=3D start && (id =3D=3D 0 || XHCI_EXT_CAPS_ID(val) =3D=3D id= > ^ > > ERROR: spaces required around that '=' (ctx:VxE) > #429: FILE: drivers/usb/dwc3/core.h:1697: > + if (offset !=3D start && (id =3D=3D 0 || XHCI_EXT_CAPS_ID(val) =3D=3D id= > ^ > > ERROR: do not use assignment in if condition > #429: FILE: drivers/usb/dwc3/core.h:1697: > + if (offset !=3D start && (id =3D=3D 0 || XHCI_EXT_CAPS_ID(val) =3D=3D id= > > ERROR: spaces required around that '=' (ctx:WxV) > #433: FILE: drivers/usb/dwc3/core.h:1700: > + next =3D XHCI_EXT_CAPS_NEXT(val); > ^ > > ERROR: spaces required around that '+=' (ctx:WxV) > #434: FILE: drivers/usb/dwc3/core.h:1701: > + offset +=3D next << 2; > > </snip> > > > The "=" gets encoded to =3D, which is strange. It never happened before. > I need to check my mail client. Sorry for the noise. I don't see it, but I did not check each patch thoroughly. I also do not know to which patch do you refer to. It is the easiest to reply inline under the block which is corrupted. If you suspect you email client is the cause, just check on lore. Best regards, Krzysztof
Hi, > Krishna Kurapati (8): > dt-bindings: usb: Add bindings for multiport properties on DWC3 > controller > usb: dwc3: core: Access XHCI address space temporarily to read port > info > usb: dwc3: core: Skip setting event buffers for host only controllers > usb: dwc3: core: Refactor PHY logic to support Multiport Controller > usb: dwc3: qcom: Add multiport controller support for qcom wrapper > arm64: dts: qcom: sc8280xp: Add multiport controller node for SC8280 > arm64: dts: qcom: sa8295p: Enable tertiary controller and its 4 USB > ports > arm64: dts: qcom: sa8540-ride: Enable first port of tertiary usb > controller > > .../devicetree/bindings/usb/snps,dwc3.yaml | 13 +- > arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 47 +++ > arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 22 ++ > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 58 +++ > drivers/usb/dwc3/core.c | 373 ++++++++++++++---- > drivers/usb/dwc3/core.h | 71 +++- > drivers/usb/dwc3/drd.c | 13 +- > drivers/usb/dwc3/dwc3-qcom.c | 28 +- > 8 files changed, 523 insertions(+), 102 deletions(-) I tested this series on the sa8540p-ride, with a USB Ethernet adapter plugged into the board. The device shows up as expected: # lsusb -tv /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/2p, 10000M ID 1d6b:0003 Linux Foundation 3.0 root hub |__ Port 1: Dev 2, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M ID 0bda:8153 Realtek Semiconductor Corp. RTL8153 Gigabit Ethernet Adapter /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/4p, 480M ID 1d6b:0002 Linux Foundation 2.0 root hub Tested-by: Adrien Thierry <athierry@redhat.com> # sa8540p-ride
On 4/14/2023 12:24 AM, Adrien Thierry wrote: > Hi, > >> Krishna Kurapati (8): >> dt-bindings: usb: Add bindings for multiport properties on DWC3 >> controller >> usb: dwc3: core: Access XHCI address space temporarily to read port >> info >> usb: dwc3: core: Skip setting event buffers for host only controllers >> usb: dwc3: core: Refactor PHY logic to support Multiport Controller >> usb: dwc3: qcom: Add multiport controller support for qcom wrapper >> arm64: dts: qcom: sc8280xp: Add multiport controller node for SC8280 >> arm64: dts: qcom: sa8295p: Enable tertiary controller and its 4 USB >> ports >> arm64: dts: qcom: sa8540-ride: Enable first port of tertiary usb >> controller >> >> .../devicetree/bindings/usb/snps,dwc3.yaml | 13 +- >> arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 47 +++ >> arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 22 ++ >> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 58 +++ >> drivers/usb/dwc3/core.c | 373 ++++++++++++++---- >> drivers/usb/dwc3/core.h | 71 +++- >> drivers/usb/dwc3/drd.c | 13 +- >> drivers/usb/dwc3/dwc3-qcom.c | 28 +- >> 8 files changed, 523 insertions(+), 102 deletions(-) > > I tested this series on the sa8540p-ride, with a USB Ethernet adapter > plugged into the board. The device shows up as expected: > > # lsusb -tv > /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/2p, 10000M > ID 1d6b:0003 Linux Foundation 3.0 root hub > |__ Port 1: Dev 2, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M > ID 0bda:8153 Realtek Semiconductor Corp. RTL8153 Gigabit Ethernet Adapter > /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/4p, 480M > ID 1d6b:0002 Linux Foundation 2.0 root hub > > Tested-by: Adrien Thierry <athierry@redhat.com> # sa8540p-ride > Hi Adrien, Thanks for testing out the patches. Regards, Krishna,
On Wed, Apr 05, 2023 at 06:27:57PM +0530, Krishna Kurapati wrote: > Add USB and DWC3 node for tertiary port of SC8280 along with multiport > IRQ's and phy's. This will be used as a base for SA8295P and SA8295-Ride > platforms. > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > --- > Link to v5: https://lore.kernel.org/all/20230310163420.7582-7-quic_kriskura@quicinc.com/ > > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 58 ++++++++++++++++++++++++++ > 1 file changed, 58 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > index 42bfa9fa5b96..7b81f2b0449d 100644 > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > @@ -3108,6 +3108,64 @@ usb_1_role_switch: endpoint { > }; > }; > > + 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"; > + This is breaking the current schema (with the full series applied), I am not sure if a pwr_event IRQ exists or but it maybe necessary to modify qcom,dwc3.yaml in order to explain hardware if it doesn't exist: (dtschema) ahalaney@halaney-x13s ~/git/linux-next (git)-[718f2024524f] % make CHECK_DTBS=y DT_SCHEMA_FILES=/usb/qcom,dwc3.yaml qcom/sa8540p-ride.dtb :( LINT Documentation/devicetree/bindings CHKDT Documentation/devicetree/bindings/processed-schema.json SCHEMA Documentation/devicetree/bindings/processed-schema.json DTC_CHK arch/arm64/boot/dts/qcom/sa8540p-ride.dtb /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:0: 'pwr_event' was expected From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:1: 'dp_hs_phy_irq' was expected From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:2: 'dm_hs_phy_irq' was expected From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names: ['dp_hs_phy_irq', 'dm_hs_phy_irq', 'ss_phy_irq'] is too short From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupts-extended: [[99, 127, 1], [99, 126, 1], [99, 16, 4]] is too short From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml make CHECK_DTBS=y DT_SCHEMA_FILES=/usb/qcom,dwc3.yaml qcom/sa8540p-ride.dtb 22.61s user 0.54s system 99% cpu 23.172 total (dtschema) ahalaney@halaney-x13s ~/git/linux-next (git)-[718f2024524f] % Thanks, Andrew
On Wed, Apr 05, 2023 at 06:27:59PM +0530, Krishna Kurapati wrote: > Enable first port of Quad port Tertiary USB controller for SA8540 Ride. > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> This is nitpicky, but I liked some of the description in the first[0] version of this patch that I authored for you: From dcb27d07f079194ebd7efe1c9bec64da78beb290 Mon Sep 17 00:00:00 2001 From: Andrew Halaney <ahalaney@redhat.com> Date: Thu, 19 Jan 2023 14:53:38 -0600 Subject: [PATCH] arm64: dts: qcom: sa8540p-ride: Enable usb_2 Content-type: text/plain There is now support for the multiport USB controller this uses so enable it. The board only has a single port hooked up (despite it being wired up to the multiport IP on the SoC). There's also a USB 2.0 mux hooked up, which by default on boot is selected to mux properly. Grab the gpio controlling that and ensure it stays in the right position so USB 2.0 continues to be routed from the external port to the SoC. Signed-off-by: Andrew Halaney <ahalaney@redhat.com> Specifically the bit helping explain what the mux, its default state, etc are things I find explain some of the hardware/patch better. Personal opinion of course but I'll highlight it since you dropped it out :) [0] https://lore.kernel.org/linux-arm-msm/20230119220942.ja5gbo3t3fl63gpy@halaney-x13s/ Either way, thanks for taking the patch along and working on this. Thanks, Andrew
On 4/14/2023 9:15 PM, Andrew Halaney wrote: > On Wed, Apr 05, 2023 at 06:27:57PM +0530, Krishna Kurapati wrote: >> Add USB and DWC3 node for tertiary port of SC8280 along with multiport >> IRQ's and phy's. This will be used as a base for SA8295P and SA8295-Ride >> platforms. >> >> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> >> --- >> Link to v5: https://lore.kernel.org/all/20230310163420.7582-7-quic_kriskura@quicinc.com/ >> >> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 58 ++++++++++++++++++++++++++ >> 1 file changed, 58 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >> index 42bfa9fa5b96..7b81f2b0449d 100644 >> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >> @@ -3108,6 +3108,64 @@ usb_1_role_switch: endpoint { >> }; >> }; >> >> + 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"; >> + > > This is breaking the current schema (with the full series applied), > I am not sure if a pwr_event IRQ exists or but it maybe necessary to > modify qcom,dwc3.yaml in order to explain hardware if it doesn't exist: > > (dtschema) ahalaney@halaney-x13s ~/git/linux-next (git)-[718f2024524f] % make CHECK_DTBS=y DT_SCHEMA_FILES=/usb/qcom,dwc3.yaml qcom/sa8540p-ride.dtb :( > LINT Documentation/devicetree/bindings > CHKDT Documentation/devicetree/bindings/processed-schema.json > SCHEMA Documentation/devicetree/bindings/processed-schema.json > DTC_CHK arch/arm64/boot/dts/qcom/sa8540p-ride.dtb > /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:0: 'pwr_event' was expected > From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml > /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:1: 'dp_hs_phy_irq' was expected > From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml > /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:2: 'dm_hs_phy_irq' was expected > From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml > /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names: ['dp_hs_phy_irq', 'dm_hs_phy_irq', 'ss_phy_irq'] is too short > From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml > /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupts-extended: [[99, 127, 1], [99, 126, 1], [99, 16, 4]] is too short > From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml > make CHECK_DTBS=y DT_SCHEMA_FILES=/usb/qcom,dwc3.yaml qcom/sa8540p-ride.dtb 22.61s user 0.54s system 99% cpu 23.172 total > (dtschema) ahalaney@halaney-x13s ~/git/linux-next (git)-[718f2024524f] % > > Thanks, > Andrew > Hi Andrew, Thanks for pointing it out. Let me check and get back on the pwr_event_irq. Probably I might have missed it 😅. If so, will make sure to add it in next version. Regards, Krishna,
On 4/14/2023 9:21 PM, Andrew Halaney wrote: > On Wed, Apr 05, 2023 at 06:27:59PM +0530, Krishna Kurapati wrote: >> Enable first port of Quad port Tertiary USB controller for SA8540 Ride. >> >> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > > This is nitpicky, but I liked some of the description in the first[0] > version of this patch that I authored for you: > > From dcb27d07f079194ebd7efe1c9bec64da78beb290 Mon Sep 17 00:00:00 2001 > From: Andrew Halaney <ahalaney@redhat.com> > Date: Thu, 19 Jan 2023 14:53:38 -0600 > Subject: [PATCH] arm64: dts: qcom: sa8540p-ride: Enable usb_2 > Content-type: text/plain > > There is now support for the multiport USB controller this uses > so enable it. > > The board only has a single port hooked up (despite it being wired up to > the multiport IP on the SoC). There's also a USB 2.0 mux hooked up, > which by default on boot is selected to mux properly. Grab the gpio > controlling that and ensure it stays in the right position so USB 2.0 > continues to be routed from the external port to the SoC. > > Signed-off-by: Andrew Halaney <ahalaney@redhat.com> > > Specifically the bit helping explain what the mux, its default state, > etc are things I find explain some of the hardware/patch better. Personal > opinion of course but I'll highlight it since you dropped it out :) > > [0] https://lore.kernel.org/linux-arm-msm/20230119220942.ja5gbo3t3fl63gpy@halaney-x13s/ > > Either way, thanks for taking the patch along and working on this. > > Thanks, > Andrew > Hi Andrew, Sorry for that. Will make sure to update the commit text with this info in the next version. Regards, Krishna,
On 4/16/2023 12:34 AM, Krishna Kurapati PSSNV wrote: > > > On 4/14/2023 9:15 PM, Andrew Halaney wrote: >> On Wed, Apr 05, 2023 at 06:27:57PM +0530, Krishna Kurapati wrote: >>> Add USB and DWC3 node for tertiary port of SC8280 along with multiport >>> IRQ's and phy's. This will be used as a base for SA8295P and SA8295-Ride >>> platforms. >>> >>> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> >>> --- >>> Link to v5: >>> https://lore.kernel.org/all/20230310163420.7582-7-quic_kriskura@quicinc.com/ >>> >>> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 58 ++++++++++++++++++++++++++ >>> 1 file changed, 58 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >>> b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >>> index 42bfa9fa5b96..7b81f2b0449d 100644 >>> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >>> @@ -3108,6 +3108,64 @@ usb_1_role_switch: endpoint { >>> }; >>> }; >>> + 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"; >>> + >> >> This is breaking the current schema (with the full series applied), >> I am not sure if a pwr_event IRQ exists or but it maybe necessary to >> modify qcom,dwc3.yaml in order to explain hardware if it doesn't exist: >> >> (dtschema) ahalaney@halaney-x13s ~/git/linux-next (git)-[718f2024524f] >> % make CHECK_DTBS=y DT_SCHEMA_FILES=/usb/qcom,dwc3.yaml >> qcom/sa8540p-ride.dtb :( >> LINT Documentation/devicetree/bindings >> CHKDT Documentation/devicetree/bindings/processed-schema.json >> SCHEMA Documentation/devicetree/bindings/processed-schema.json >> DTC_CHK arch/arm64/boot/dts/qcom/sa8540p-ride.dtb >> /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:0: 'pwr_event' was expected >> From schema: >> /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml >> /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:1: 'dp_hs_phy_irq' was expected >> From schema: >> /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml >> /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:2: 'dm_hs_phy_irq' was expected >> From schema: >> /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml >> /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names: ['dp_hs_phy_irq', 'dm_hs_phy_irq', 'ss_phy_irq'] is too short >> From schema: >> /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml >> /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupts-extended: [[99, 127, 1], [99, 126, 1], [99, 16, 4]] is too short >> From schema: >> /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml >> make CHECK_DTBS=y DT_SCHEMA_FILES=/usb/qcom,dwc3.yaml >> qcom/sa8540p-ride.dtb 22.61s user 0.54s system 99% cpu 23.172 total >> (dtschema) ahalaney@halaney-x13s ~/git/linux-next (git)-[718f2024524f] % >> >> Thanks, >> Andrew >> > > Hi Andrew, > > Thanks for pointing it out. Let me check and get back on the > pwr_event_irq. > > Probably I might have missed it 😅. If so, will make sure to add it in > next version. > > Regards, > Krishna, Hi Andrew, Johan, I was looking at the pwr_event_irq interrupts for Multiport controller and see that there are two of them as per HW specs. All targets till date have only 1 pwr_event_irq required. The reason I thought I missed pwr_event_irq in my patches is because in downstream this is a required IRQ for all targets, so I was under assumption that we need it for upstream targets as well. But upstream qcom driver doesn't have support for this IRQ yet. And this has been made a required one only for SC8280 [1]/[2]. Probably we can proceed in one of the following ways: 1. Remove pwr_event_irq in both bindings and DT as driver support is not present currently. 2. Update the bindings for SC8280 to include an optional secondary pwr_event_irq for multiport controller. I would prefer option-1 as removing them would be better because they are not being used. Please let me know your thoughts on this. [1]: https://lore.kernel.org/all/20220713131340.29401-2-johan+linaro@kernel.org/ [2]: https://lore.kernel.org/all/20220713131340.29401-6-johan+linaro@kernel.org/ Regards, Krishna,
On Sat, Apr 22, 2023 at 09:38:44PM +0530, Krishna Kurapati PSSNV wrote: > > > On 4/16/2023 12:34 AM, Krishna Kurapati PSSNV wrote: > > > > > > On 4/14/2023 9:15 PM, Andrew Halaney wrote: > > > On Wed, Apr 05, 2023 at 06:27:57PM +0530, Krishna Kurapati wrote: > > > > Add USB and DWC3 node for tertiary port of SC8280 along with multiport > > > > IRQ's and phy's. This will be used as a base for SA8295P and SA8295-Ride > > > > platforms. > > > > > > > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > > > > --- > > > > Link to v5: https://lore.kernel.org/all/20230310163420.7582-7-quic_kriskura@quicinc.com/ > > > > > > > > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 58 ++++++++++++++++++++++++++ > > > > 1 file changed, 58 insertions(+) > > > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > > > b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > > > index 42bfa9fa5b96..7b81f2b0449d 100644 > > > > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > > > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > > > @@ -3108,6 +3108,64 @@ usb_1_role_switch: endpoint { > > > > }; > > > > }; > > > > + 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"; > > > > + > > > > > > This is breaking the current schema (with the full series applied), > > > I am not sure if a pwr_event IRQ exists or but it maybe necessary to > > > modify qcom,dwc3.yaml in order to explain hardware if it doesn't exist: > > > > > > (dtschema) ahalaney@halaney-x13s ~/git/linux-next > > > (git)-[718f2024524f] % make CHECK_DTBS=y > > > DT_SCHEMA_FILES=/usb/qcom,dwc3.yaml qcom/sa8540p-ride.dtb > > > :( > > > LINT Documentation/devicetree/bindings > > > CHKDT Documentation/devicetree/bindings/processed-schema.json > > > SCHEMA Documentation/devicetree/bindings/processed-schema.json > > > DTC_CHK arch/arm64/boot/dts/qcom/sa8540p-ride.dtb > > > /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:0: 'pwr_event' was expected > > > From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml > > > /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:1: 'dp_hs_phy_irq' was expected > > > From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml > > > /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:2: 'dm_hs_phy_irq' was expected > > > From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml > > > /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names: ['dp_hs_phy_irq', 'dm_hs_phy_irq', 'ss_phy_irq'] is too short > > > From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml > > > /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupts-extended: [[99, 127, 1], [99, 126, 1], [99, 16, 4]] is too short > > > From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml > > > make CHECK_DTBS=y DT_SCHEMA_FILES=/usb/qcom,dwc3.yaml > > > qcom/sa8540p-ride.dtb 22.61s user 0.54s system 99% cpu 23.172 total > > > (dtschema) ahalaney@halaney-x13s ~/git/linux-next (git)-[718f2024524f] % > > > > > > Thanks, > > > Andrew > > > > > > > Hi Andrew, > > > > Thanks for pointing it out. Let me check and get back on the > > pwr_event_irq. > > > > Probably I might have missed it 😅. If so, will make sure to add it in > > next version. > > > > Regards, > > Krishna, > > > Hi Andrew, Johan, > > I was looking at the pwr_event_irq interrupts for Multiport controller and > see that there are two of them as per HW specs. All targets till date have > only 1 pwr_event_irq required. > > The reason I thought I missed pwr_event_irq in my patches is because in > downstream this is a required IRQ for all targets, so I was under assumption > that we need it for upstream targets as well. But upstream qcom driver > doesn't have support for this IRQ yet. And this has been made a required one > only for SC8280 [1]/[2]. > > Probably we can proceed in one of the following ways: > 1. Remove pwr_event_irq in both bindings and DT as driver support is not > present currently. > 2. Update the bindings for SC8280 to include an optional secondary > pwr_event_irq for multiport controller. > > I would prefer option-1 as removing them would be better because they are > not being used. Please let me know your thoughts on this. > > [1]: > https://lore.kernel.org/all/20220713131340.29401-2-johan+linaro@kernel.org/ > [2]: > https://lore.kernel.org/all/20220713131340.29401-6-johan+linaro@kernel.org/ > Personally, I prefer option 2 since the IRQ does exist technically (although it isn't currently used), I like it being described... it makes the dt-binding a more complete description of the hardware. I am unsure of the rules wrt dt-bindings and usage in drivers, but I always like to view it as "this is a description of the hardware", and the driver bit is just nice to have to ensure that whoever is adding the binding is actually describing things sufficiently. You could probably add a new compatible string for the multiport sc8280xp IP as well, and make the second IRQ non-optional in dt-binding evaluation? That seems more inline with reality, the regular IP has 1 pwr_event_irq, multiport on this platform has 2, they behave slightly differently and thus they deserve their own string to match on despite being on the same platform. Please note, I'm just a contributor -- I would not be surprised to find out that my view is not aligned with what maintainers here think, but that's my interpretation of things! Hope that helps, Andrew
On Tue, Apr 25, 2023 at 03:33:28PM -0500, Andrew Halaney wrote: > On Sat, Apr 22, 2023 at 09:38:44PM +0530, Krishna Kurapati PSSNV wrote: > > Hi Andrew, Johan, > > > > I was looking at the pwr_event_irq interrupts for Multiport controller and > > see that there are two of them as per HW specs. All targets till date have > > only 1 pwr_event_irq required. > > > > The reason I thought I missed pwr_event_irq in my patches is because in > > downstream this is a required IRQ for all targets, so I was under assumption > > that we need it for upstream targets as well. But upstream qcom driver > > doesn't have support for this IRQ yet. And this has been made a required one > > only for SC8280 [1]/[2]. > > > > Probably we can proceed in one of the following ways: > > 1. Remove pwr_event_irq in both bindings and DT as driver support is not > > present currently. > > 2. Update the bindings for SC8280 to include an optional secondary > > pwr_event_irq for multiport controller. > > > > I would prefer option-1 as removing them would be better because they are > > not being used. Please let me know your thoughts on this. > > > > [1]: > > https://lore.kernel.org/all/20220713131340.29401-2-johan+linaro@kernel.org/ > > [2]: > > https://lore.kernel.org/all/20220713131340.29401-6-johan+linaro@kernel.org/ > > > > Personally, I prefer option 2 since the IRQ does exist technically > (although it isn't currently used), I like it being described... it > makes the dt-binding a more complete description of the hardware. > > I am unsure of the rules wrt dt-bindings and usage in drivers, but I > always like to view it as "this is a description of the hardware", and > the driver bit is just nice to have to ensure that whoever is adding the > binding is actually describing things sufficiently. As Andrew mentioned, the binding should reflect the hardware and not what is currently supported in some version of software. It looks like you even had four of these pwr_event interrupt line judging from your last iteration of this series. Johan
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). Changes in v6: Updated comments in code after. Updated variables names appropriately as per review comments. Updated commit text in patch-2 and added additional info as per review comments. The patch header in v5 doesn't have "PATHCH v5" notation present. Corrected it in this version. Changes in v5: Added DT support for first port of Teritiary USB controller on SA8540-Ride Added support for reading port info from XHCI Extended Params registers. Changes in RFC v4: Added DT support for SA8295p. Changes in RFC v3: Incase any PHY init fails, then clear/exit the PHYs that are already initialized. Changes in RFC 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 v5: https://lore.kernel.org/all/20230310163420.7582-1-quic_kriskura@quicinc.com/ Link to RFC v4: https://lore.kernel.org/all/20230115114146.12628-1-quic_kriskura@quicinc.com/ Link to RFC v3: https://lore.kernel.org/all/1654709787-23686-1-git-send-email-quic_harshq@quicinc.com/#r Link to RFC v2: https://lore.kernel.org/all/1653560029-6937-1-git-send-email-quic_harshq@quicinc.com/#r Krishna Kurapati (8): dt-bindings: usb: Add bindings for multiport properties on DWC3 controller usb: dwc3: core: Access XHCI address space temporarily to read port info usb: dwc3: core: Skip setting event buffers for host only controllers usb: dwc3: core: Refactor PHY logic to support Multiport Controller usb: dwc3: qcom: Add multiport controller support for qcom wrapper arm64: dts: qcom: sc8280xp: Add multiport controller node for SC8280 arm64: dts: qcom: sa8295p: Enable tertiary controller and its 4 USB ports arm64: dts: qcom: sa8540-ride: Enable first port of tertiary usb controller .../devicetree/bindings/usb/snps,dwc3.yaml | 13 +- arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 47 +++ arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 22 ++ arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 58 +++ drivers/usb/dwc3/core.c | 373 ++++++++++++++---- drivers/usb/dwc3/core.h | 71 +++- drivers/usb/dwc3/drd.c | 13 +- drivers/usb/dwc3/dwc3-qcom.c | 28 +- 8 files changed, 523 insertions(+), 102 deletions(-)