On 12/20, Vivek Gautam wrote:

Qualcomm SOCs have QMP phy controller that provides support
to a number of controller, viz. PCIe, UFS, and USB.
Add a new driver, based on generic phy framework, for this
phy controller.

Signed-off-by: Vivek Gautam <redacted>
Tested-by: Srinivas Kandagatla <redacted>
---

+
+static struct phy *qcom_qmp_phy_xlate(struct device *dev,
+					struct of_phandle_args *args)
+{
+	struct qcom_qmp_phy *qphy = dev_get_drvdata(dev);
+	int i;
+
+	if (WARN_ON(args->args[0] >= qphy->cfg->nlanes))
+		return ERR_PTR(-ENODEV);
+
+	for (i = 0; i < qphy->cfg->nlanes; i++)
+		/* phys[i]->index */
+		if (i == args->args[0])
+			return qphy->phys[i]->phy;

What's the loop for? If args->arg[0] < qphy->cfg->nlanes then we
should be able to directly index the qphy->phys array with that
number and return it.
 

+
+	return ERR_PTR(-ENODEV);
+}
+

[...]

+
+/*
+ * The <s>_pipe_clksrc generated by PHY goes to the GCC that gate
+ * controls it. The <s>_pipe_clk coming out of the GCC is requested
+ * by the PHY driver for its operations.
+ * We register the <s>_pipe_clksrc here. The gcc driver takes care
+ * of assigning this <s>_pipe_clksrc as parent to <s>_pipe_clk.
+ * Below picture shows this relationship.
+ *
+ *	   +--------------+
+ *	   |  PHY block   |<<---------------------------------------+
+ *	   |              |					    |
+ *	   |   +-------+  |		      +-----+		    |
+ *   I/P---^-->|  PLL  |--^--->pipe_clksrc--->| GCC |--->pipe_clk---+
+ *   clk   |   +-------+  |		      +-----+
+ *	   +--------------+

There are mixed tabs and spaces in this diagram causing
confusion in my editor. Please make it only spaces so the picture
comes out correctly.

+ *
+ */
+static int phy_pipe_clk_register(struct qcom_qmp_phy *qphy, int id)
+{
+	char clk_name[MAX_PROP_NAME];

I'm not sure MAX_PROP_NAME is the same as some max clk name but
ok. We should be able to calculate that the maximum is length of
usb3_phy_pipe_clk_src for now though?

+	struct clk *clk;
+
+	memset(&clk_name, 0, sizeof(clk_name));
+	switch (qphy->cfg->type) {
+	case PHY_TYPE_USB3:
+		snprintf(clk_name, MAX_PROP_NAME, "usb3_phy_pipe_clk_src");
+		break;
+	case PHY_TYPE_PCIE:
+		snprintf(clk_name, MAX_PROP_NAME, "pcie_%d_pipe_clk_src", id);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* controllers using QMP phys use 125MHz pipe clock interface */
+	clk = clk_register_fixed_rate(qphy->dev, clk_name, NULL, 0, 125000000);

I was hoping you would be able to calculate the actual output
rate by reading hardware. This is ok too though. Just please use
clk_hw_register_fixed_rate() instead. And you'll probably need
some sort of devm() usage here to handle probe failure, so I
would probably roll my own and allocate a fixed_rate clk
structure and set the rate/name directly and then call
devm_clk_hw_register().

+
+	return PTR_ERR_OR_ZERO(clk);

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project