USB for Surface Connect Port

[qzed]

The Surface Connect port is, for the most part, a USB. Adding support for that apparently requires some driver changes, and we also need to describe it in the DT. More research as to what specifically needs to be done is required.

[qzed]

As the Surface Connect USB(s?) is supported in ACPI (QCOM04A6, handled by dwc3-qcom), we should be able to piece everything together from that and the other USB implementations. @andersson mentioned that we will also need something like this as it's a multi-port/multi-phy controller. This could also serve as a reference. I'm trying to figure out the specifics now.

[qzed]

Here's what I have so far. I'm struggling to describe the PHYs as I have no idea how they're represented in ACPI. Also, I think the interrupts may not be correct/complete.

usb_mp: usb@a4f8800 {
	compatible = "qcom,sc8180x-dwc3", "qcom,dwc3";
	reg = <0 0xa4f8800 0 0x400>;
	status = "disabled";
	#address-cells = <2>;
	#size-cells = <2>;
	ranges;
	dma-ranges;

	// TODO: xo/clkref clock (gcc_usb3_mp0_clkref_en, gcc_usb3_mp1_clkref_en)?
	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_MOCK_UTMI_CLK>,
		 <&gcc GCC_USB30_MP_SLEEP_CLK>;

	clock-names = "cfg_noc", "core", "iface", "mock_utmi",
		      "sleep";

	assigned-clocks = <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>,
			  <&gcc GCC_USB30_MP_MASTER_CLK>;
	assigned-clock-rates = <19200000>, <200000000>;

	// TODO: are interrupts correct?
	//       currently computed as #ACPI - 32
	//
	// usb2 (this)	 dt	acpi
	//    ?		654?	 686
	//    ?		656?	 688
	//    ?		487?	 519	(conflict with old sec!)
	//    ?		655?	 687
	//    ?		510?	 542
	//    ?		526?	 558
	//    ?		539?	 571
	//    ?		548?	 580
	//    ?		551?	 583

	interrupts = <GIC_SPI 656 IRQ_TYPE_LEVEL_HIGH>,
		     <GIC_SPI 487 IRQ_TYPE_LEVEL_HIGH>,
		     <GIC_SPI 655 IRQ_TYPE_LEVEL_HIGH>,
		     <GIC_SPI 510 IRQ_TYPE_LEVEL_HIGH>;

	interrupt-names = "hs_phy_irq", "ss_phy_irq",
			  "dm_hs_phy_irq", "dp_hs_phy_irq";

	power-domains = <&gcc USB30_MP_GDSC>;

	interconnects = <&aggre1_noc MASTER_USB3_2 0 &mc_virt SLAVE_EBI_CH0 0>,
			<&gem_noc MASTER_AMPSS_M0 0 &config_noc SLAVE_USB3_2 0>;
	interconnect-names = "usb-ddr", "apps-usb";

	resets = <&gcc GCC_USB30_MP_BCR>;

	usb_mp_dwc3: dwc3@a400000 {
		compatible = "snps,dwc3";
		reg = <0 0x0a400000 0 0xcd00>;
		interrupts = <GIC_SPI 654 IRQ_TYPE_LEVEL_HIGH>;
		// TODO: iommus
		// TODO: quirks
		// TODO: phys, phy-names

		// qmpphy clocks:
		// - GCC_USB3_MP_PHY_AUX_CLK
		// - GCC_USB3_MP_PHY_COM_AUX_CLK
		// - GCC_USB3_MP_PHY_PIPE_0_CLK
		// - GCC_USB3_MP_PHY_PIPE_1_CLK

		// qmpphy resets:
		// - GCC_QUSB2PHY_MP0_BCR
		// - GCC_QUSB2PHY_MP1_BCR
	};
};

[qzed]

Currently looking through then Windows driver. A ton of stuff is hard-coded. From what I can tell there are two (single-lane?) QMP-PHYs associated with the MP controller (MP0 and MP1). Those PHYs should be USB-only, i.e. not combo USB/DP PHYs like PRIM and SEC.

The Windows driver uses one large MMIO range for all PHY access as base, with individual PHYs being accessed at different register offsets. That space is at 0x88e0000 with length 0x14000.

PHYs are accessed from that via base + phy_offset + reg_offset. Specific PHY offsets and register-subspaces are:

• PRIM (based on DT):

  hsphy_offset = 0x2000  (size: 0x400)
  dpcom_offset = 0x8000
  usb_serdes   = 0x9000  [=ssphy_offset]
  dp_serdes    = 0xa000
  
  ssphy_tx1      = base + ssphy_offset + 0x200 = 0x88e9200  (size: 0x200)
  ssphy_rx1      = base + ssphy_offset + 0x400 = 0x88e9400  (size: 0x200)
  ssphy_tx2      = base + ssphy_offset + 0x600 = 0x88e9600  (size: 0x200)
  ssphy_rx2      = base + ssphy_offset + 0x800 = 0x88e9800  (size: 0x200)
  ssphy_pcs_misc = base + ssphy_offset + 0xa00 = 0x88e9a00  (size: 0x100)
  ssphy_pcs      = base + ssphy_offset + 0xc00 = 0x88e9c00  (size: 0x218)
  

• SEC (based on DT):

  hsphy_offset = 0x3000  (size: 0x400)
  dpcom_offset = 0xd000
  usb_serdes   = 0xe000  [=ssphy_offset]
  dp_serdes    = 0xf000
  
  ssphy_tx1      = base + ssphy_offset + 0x200 = 0x88ee200  (size: 0x200)
  ssphy_rx1      = base + ssphy_offset + 0x400 = 0x88ee400  (size: 0x200)
  ssphy_tx2      = base + ssphy_offset + 0x600 = 0x88ee600  (size: 0x200)
  ssphy_rx2      = base + ssphy_offset + 0x800 = 0x88ee800  (size: 0x200)
  ssphy_pcs_misc = base + ssphy_offset + 0xa00 = 0x88eea00  (size: 0x100)
  ssphy_pcs      = base + ssphy_offset + 0xc00 = 0x88eec00  (size: 0x218)
  

• MP0:

  hsphy_offset = 0x4000
  ssphy_offset = 0xb000  [=usb_serdes]
  
  ssphy_tx1      = base + ssphy_offset + ?     = 0x88eb200 ?
  ssphy_rx1      = base + ssphy_offset + ?     = 0x88eb400 ?
  ssphy_pcs_misc = base + ssphy_offset + ?     = 0x88eb600 ?
  ssphy_pcs      = base + ssphy_offset + 0x800 = 0x88eb800
  

• MP1:

  hsphy_offset = 0x5000
  ssphy_offset = 0xc000  [=usb_serdes]
  
  ssphy_tx1      = base + ssphy_offset + ?     = 0x88ec200 ?
  ssphy_rx1      = base + ssphy_offset + ?     = 0x88ec400 ?
  ssphy_pcs_misc = base + ssphy_offset + ?     = 0x88ec600 ?
  ssphy_pcs      = base + ssphy_offset + 0x800 = 0x88ec800
  

[qzed]

I've created a separate branch for this issue. See linux-surface/kernel@spx/next-20220616...spx/next-20220616-usbmp.

[qzed]

I've added the bindings and some code to enable and configure the additional PHYs, but things still don't work properly. If I understand it correctly, it seems that the buses get set up properly but XHCI fails setting up devices with Error while assigning device slot ID. That command seems to time out based on log timestamps and the command status code.

I suspect we may be missing a couple of config bits... there are some additional register writes in the multi-port code paths of the Windows driver that don't seem to be present in the Linux driver.

Log attached: dmesg.log

[qzed]

According to the Windows driver, there should be some control for the MP1 pipe clock. This is similar to https://elixir.bootlin.com/linux/v5.19-rc1/source/drivers/usb/dwc3/dwc3-qcom.c#L420, accessing the same register but values shifted by 16 bit. Pseudo-code from the Windows driver below:

#define PIPE0_UTMI_CLK_SEL  BIT(0)
#define PIPE0_UTMI_CLK_DIS  BIT(8)
#define PIPE1_UTMI_CLK_SEL  BIT(16)
#define PIPE1_UTMI_CLK_DIS  BIT(25)

// pipe 0
set_bits(ctrl->dwc3_mem_virt, QSCRATCH_GENERAL_CFG, PIPE0_UTMI_CLK_DIS);
delay_ms(1);
set_bits(ctrl->dwc3_mem_virt, QSCRATCH_GENERAL_CFG, PIPE0_UTMI_CLK_SEL);
delay_ms(1);
clear_bits(ctrl->dwc3_mem_virt, QSCRATCH_GENERAL_CFG, PIPE0_UTMI_CLK_DIS);

// pipe 1
set_bits(ctrl->dwc3_mem_virt, QSCRATCH_GENERAL_CFG, PIPE1_UTMI_CLK_DIS);
delay_ms(1);
set_bits(ctrl->dwc3_mem_virt, QSCRATCH_GENERAL_CFG, PIPE1_UTMI_CLK_SEL);
delay_ms(1);
clear_bits(ctrl->dwc3_mem_virt, QSCRATCH_GENERAL_CFG, PIPE1_UTMI_CLK_DIS);

Note: This should only be called in case no SSPHY is present...

[qzed]

USB works! Turns out I either had a missing clock or an invalid IOMMU spec. My bet is on the IOMMU. DT fixes are based on the 8180-based automotive reference found at https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/auto-kernel.lnx.4.14.c31/arch/arm64/boot/dts/qcom/sdmshrike-usb.dtsi.

[qzed]

Implemented in linux-surface/kernel#129.

Some remaining things to maybe look at: Both ACPI and the driver provide some init value tables, similar to what we currently use in Linux based on the sm8150. I've created a new issue for this in #33.