[PATCH 2/2] phy: qcom: usb-hs: honour qcom,vendor-init-seq raw ULPI writes

[Herman van Hazendonk <github.com@herrie.org>]

Add support for the optional qcom,vendor-init-seq DT property: a
list of u8 (addr, val) pairs written verbatim to raw ULPI register
addresses, rather than to ULPI_EXT_VENDOR_SPECIFIC + addr like the
existing qcom,init-seq sequence reaches. This lets boards reach the
standard vendor register range 0x30-0x3F where MSM8x60-era hardware
keeps pre-emphasis level / HS driver slope / CDR auto-reset, etc.

The new sequence is applied AFTER reset_control_reset() in
qcom_usb_hs_phy_power_on() so the values survive the register
restore the reset performs. The pre-existing qcom,init-seq is left
applied BEFORE the reset to preserve current behaviour for the
APQ8064/MSM8916 boards that already depend on it; an earlier review flagged
this ordering as a likely pre-existing bug, but blindly reordering
established behaviour for those boards is out of scope for this
patch and would risk regressing them.

While at it, harden the parse logic shared between qcom,init-seq
and qcom,vendor-init-seq:

  - Reject an odd byte count up-front rather than silently dropping
    the trailing byte and producing a half-pair the rest of the
    driver cannot describe.
  - Reject more than 32 bytes (the binding's maxItems) rather than
    making an unbounded devm_kmalloc_array() allocation driven by
    an untrusted-shaped DT property.

Factor the duplicated parse-and-allocate sequence into
qcom_usb_hs_phy_parse_init_seq() so both properties get the same
validation. The validation also fixes the pre-existing same bugs in
the qcom,init-seq parse path uniformly with the new property.

Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
---
 drivers/phy/qualcomm/phy-qcom-usb-hs.c | 109 ++++++++++++++++++++-----
 1 file changed, 90 insertions(+), 19 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs.c b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
index 98a18987f1be..89fbe8f8d001 100644
--- a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
+++ b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
@@ -35,6 +35,13 @@ struct qcom_usb_hs_phy {
 	struct regulator *v3p3;
 	struct reset_control *reset;
 	struct ulpi_seq *init_seq;
+	/*
+	 * Optional sequence of writes targeting raw ULPI addresses (no
+	 * ULPI_EXT_VENDOR_SPECIFIC base added). Populated from the
+	 * "qcom,vendor-init-seq" DT property. Applied after PHY reset
+	 * so the values survive the reset that follows init_seq writes.
+	 */
+	struct ulpi_seq *vendor_init_seq;
 	struct extcon_dev *vbus_edev;
 	struct notifier_block vbus_notify;
 };
@@ -154,6 +161,19 @@ static int qcom_usb_hs_phy_power_on(struct phy *phy)
 			goto err_ulpi;
 	}
 
+	/*
+	 * Apply board-specific raw-address ULPI writes after the PHY reset
+	 * so they survive register restore. Used to reach the standard
+	 * vendor range 0x30-0x3F which qcom,init-seq (above) cannot —
+	 * pre-emphasis level / HS driver slope / CDR auto-reset etc. live
+	 * there on MSM8660-class hardware.
+	 */
+	for (seq = uphy->vendor_init_seq; seq->addr; seq++) {
+		ret = ulpi_write(ulpi, seq->addr, seq->val);
+		if (ret)
+			goto err_ulpi;
+	}
+
 	if (uphy->vbus_edev) {
 		state = extcon_get_state(uphy->vbus_edev, EXTCON_USB);
 		/* setup initial state */
@@ -199,6 +219,59 @@ static const struct phy_ops qcom_usb_hs_phy_ops = {
 	.owner = THIS_MODULE,
 };
 
+/*
+ * The binding caps both qcom,init-seq and qcom,vendor-init-seq at
+ * maxItems: 32 (addr, val) pairs, i.e. 64 bytes total. Enforce that
+ * limit here so a malformed DT cannot drive an unbounded
+ * devm_kmalloc_array() and so the misconfiguration is visible at
+ * probe time instead of silently truncated.
+ */
+#define QCOM_USB_HS_PHY_INIT_SEQ_MAX_PAIRS	32
+#define QCOM_USB_HS_PHY_INIT_SEQ_MAX_BYTES	\
+	(QCOM_USB_HS_PHY_INIT_SEQ_MAX_PAIRS * 2)
+
+static int qcom_usb_hs_phy_parse_init_seq(struct ulpi *ulpi,
+					  const char *propname,
+					  struct ulpi_seq **out)
+{
+	struct ulpi_seq *seq;
+	int size;
+
+	size = of_property_count_u8_elems(ulpi->dev.of_node, propname);
+	if (size < 0)
+		size = 0;
+	if (size > QCOM_USB_HS_PHY_INIT_SEQ_MAX_BYTES) {
+		dev_err(&ulpi->dev,
+			"%s: %d bytes exceeds %d-byte maximum\n",
+			propname, size, QCOM_USB_HS_PHY_INIT_SEQ_MAX_BYTES);
+		return -EINVAL;
+	}
+	if (size % 2) {
+		dev_err(&ulpi->dev,
+			"%s: %d bytes is not a whole number of (addr, val) pairs\n",
+			propname, size);
+		return -EINVAL;
+	}
+
+	seq = devm_kmalloc_array(&ulpi->dev, (size / 2) + 1, sizeof(*seq),
+				 GFP_KERNEL);
+	if (!seq)
+		return -ENOMEM;
+
+	if (size) {
+		int ret = of_property_read_u8_array(ulpi->dev.of_node,
+						    propname, (u8 *)seq, size);
+		if (ret)
+			return ret;
+	}
+	/* NUL-terminate so the power_on loop's seq->addr-as-sentinel works. */
+	seq[size / 2].addr = 0;
+	seq[size / 2].val = 0;
+
+	*out = seq;
+	return 0;
+}
+
 static int qcom_usb_hs_phy_probe(struct ulpi *ulpi)
 {
 	struct qcom_usb_hs_phy *uphy;
@@ -206,7 +279,6 @@ static int qcom_usb_hs_phy_probe(struct ulpi *ulpi)
 	struct clk *clk;
 	struct regulator *reg;
 	struct reset_control *reset;
-	int size;
 	int ret;
 
 	uphy = devm_kzalloc(&ulpi->dev, sizeof(*uphy), GFP_KERNEL);
@@ -215,19 +287,20 @@ static int qcom_usb_hs_phy_probe(struct ulpi *ulpi)
 	ulpi_set_drvdata(ulpi, uphy);
 	uphy->ulpi = ulpi;
 
-	size = of_property_count_u8_elems(ulpi->dev.of_node, "qcom,init-seq");
-	if (size < 0)
-		size = 0;
-	uphy->init_seq = devm_kmalloc_array(&ulpi->dev, (size / 2) + 1,
-					   sizeof(*uphy->init_seq), GFP_KERNEL);
-	if (!uphy->init_seq)
-		return -ENOMEM;
-	ret = of_property_read_u8_array(ulpi->dev.of_node, "qcom,init-seq",
-					(u8 *)uphy->init_seq, size);
-	if (ret && size)
+	ret = qcom_usb_hs_phy_parse_init_seq(ulpi, "qcom,init-seq",
+					     &uphy->init_seq);
+	if (ret)
+		return ret;
+	/*
+	 * Optional raw-address vendor init sequence — same encoding as
+	 * qcom,init-seq (u8 addr/val pairs) but each pair is written to
+	 * the raw ULPI address rather than to ULPI_EXT_VENDOR_SPECIFIC +
+	 * addr. Lets boards reach the standard vendor range 0x30-0x3F.
+	 */
+	ret = qcom_usb_hs_phy_parse_init_seq(ulpi, "qcom,vendor-init-seq",
+					     &uphy->vendor_init_seq);
+	if (ret)
 		return ret;
-	/* NUL terminate */
-	uphy->init_seq[size / 2].addr = uphy->init_seq[size / 2].val = 0;
 
 	uphy->ref_clk = clk = devm_clk_get(&ulpi->dev, "ref");
 	if (IS_ERR(clk))
@@ -245,12 +318,10 @@ static int qcom_usb_hs_phy_probe(struct ulpi *ulpi)
 	if (IS_ERR(reg))
 		return PTR_ERR(reg);
 
-	uphy->reset = reset = devm_reset_control_get(&ulpi->dev, "por");
-	if (IS_ERR(reset)) {
-		if (PTR_ERR(reset) == -EPROBE_DEFER)
-			return PTR_ERR(reset);
-		uphy->reset = NULL;
-	}
+	uphy->reset = reset = devm_reset_control_get_optional_exclusive(&ulpi->dev, "por");
+	if (IS_ERR(reset))
+		return dev_err_probe(&ulpi->dev, PTR_ERR(reset),
+				     "failed to get reset control\n");
 
 	uphy->phy = devm_phy_create(&ulpi->dev, ulpi->dev.of_node,
 				    &qcom_usb_hs_phy_ops);
-- 
2.43.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy