Hi,

Manish Narani [off-list ref] writes:

quoted

-----Original Message-----
From: Felipe Balbi <balbi@kernel.org>
Sent: Tuesday, September 1, 2020 5:45 PM

quoted

quoted

quoted

+		goto err;
+	}
+
+	ret = dwc3_xlnx_rst_assert(priv_data->apbrst);
+	if (ret < 0) {
+		dev_err(dev, "%s: %d: Failed to assert reset\n",
+			__func__, __LINE__);

		dev_err(dev, "Failed to assert APB reset\n");

quoted

+		goto err;
+	}
+
+	ret = phy_init(priv_data->usb3_phy);

dwc3 core should be handling this already

The USB controller used in Xilinx ZynqMP platform uses xilinx GT phy
which has 4 GT lanes and can used by 4 peripherals at a time.

At the same time or are they mutually exclusive?

The lanes are mutually exclusive.

Thank you for confirming :-)

 [...]

quoted

quoted

quoted

quoted

+	if (ret < 0) {
+		dev_err(dev, "%s: %d: Failed to release reset\n",
+			__func__, __LINE__);
+		goto err;
+	}
+
+	/* Set PIPE power present signal */
+	writel(PIPE_POWER_ON, priv_data->regs + PIPE_POWER_OFFSET);
+
+	/* Clear PIPE CLK signal */
+	writel(PIPE_CLK_OFF, priv_data->regs + PIPE_CLK_OFFSET);

shouldn't this be hidden under clk_enable()?

Though its naming suggests something related to clock framework, it is
a register in the Xilinx USB controller space which configures the
PIPE clock coming from Serdes.

PIPE clock is a clock. It just so happens that the source is the PHY
itself.

This bit is used to choose between PIPE clock coming from SerDes
and the Suspend Clock. When the controller is out of reset, this bit
needs to be reset in order to make the USB controller work. This
register is added in Xilinx USB controller register space. I will
add more description about the same in v2.

Aha! That clarifies. It's just a clock selection from clocks that are
generated elsewhere :-) I guess a clk driver would be overkill, indeed.

Thanks for explaining. Could you add some of this information to commit
log, then?

cheers

-- 
balbi