Thread (15 messages) 15 messages, 2 authors, 2019-12-20

Re: [PATCH v2 3/6] phy: amlogic: Add Amlogic A1 USB2 PHY Driver

From: Hanjie Lin <hidden>
Date: 2019-12-20 07:38:18
Also in: linux-amlogic, linux-devicetree, linux-usb


On 2019/12/19 18:12, Neil Armstrong wrote:
On 19/12/2019 10:48, Hanjie Lin wrote:
quoted

On 2019/12/18 21:17, Neil Armstrong wrote:
quoted
Hi,

On 18/12/2019 03:42, Hanjie Lin wrote:
quoted
This adds support for the USB2 PHY found in the Amlogic A1 SoC Family.

It supports host mode only.

Signed-off-by: Hanjie Lin <redacted>
Signed-off-by: Yue Wang <yue.wang@amlogic.com>
---
 drivers/phy/amlogic/phy-meson-g12a-usb2.c | 102 ++++++++++++++++++++++--------
 1 file changed, 74 insertions(+), 28 deletions(-)
diff --git a/drivers/phy/amlogic/phy-meson-g12a-usb2.c b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
index 9065ffc..2c242d3 100644
--- a/drivers/phy/amlogic/phy-meson-g12a-usb2.c
+++ b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
@@ -146,11 +146,18 @@
 #define RESET_COMPLETE_TIME					1000
 #define PLL_RESET_COMPLETE_TIME					100
 
+enum {
+	MESON_USB2_PHY_VERSION_10 = 0,
+	MESON_USB2_PHY_VERSION_11,
Are these the real "versions" of the phy or it's made up ?
This version is made up and only for distinguish a1 and g12a.
No problem, in this case simply use the SoC family instead of 10 and 11.

Neil
Of course, it looks more accurate, I will get rid of phy version in next version.
quoted
quoted
quoted
+	MESON_USB2_PHY_VERSION_COUNT,
+};
+
 struct phy_meson_g12a_usb2_priv {
 	struct device		*dev;
 	struct regmap		*regmap;
 	struct clk		*clk;
 	struct reset_control	*reset;
+	int phy_version;
 };
 
 static const struct regmap_config phy_meson_g12a_usb2_regmap_conf = {
@@ -192,18 +199,33 @@ static int phy_meson_g12a_usb2_init(struct phy *phy)
 		     FIELD_PREP(PHY_CTRL_R17_MPLL_FILTER_PVT2, 2) |
 		     FIELD_PREP(PHY_CTRL_R17_MPLL_FILTER_PVT1, 9));
 
-	regmap_write(priv->regmap, PHY_CTRL_R18,
-		     FIELD_PREP(PHY_CTRL_R18_MPLL_LKW_SEL, 1) |
-		     FIELD_PREP(PHY_CTRL_R18_MPLL_LK_W, 9) |
-		     FIELD_PREP(PHY_CTRL_R18_MPLL_LK_S, 0x27) |
-		     FIELD_PREP(PHY_CTRL_R18_MPLL_PFD_GAIN, 1) |
-		     FIELD_PREP(PHY_CTRL_R18_MPLL_ROU, 7) |
-		     FIELD_PREP(PHY_CTRL_R18_MPLL_DATA_SEL, 3) |
-		     FIELD_PREP(PHY_CTRL_R18_MPLL_BIAS_ADJ, 1) |
-		     FIELD_PREP(PHY_CTRL_R18_MPLL_BB_MODE, 0) |
-		     FIELD_PREP(PHY_CTRL_R18_MPLL_ALPHA, 3) |
-		     FIELD_PREP(PHY_CTRL_R18_MPLL_ADJ_LDO, 1) |
-		     PHY_CTRL_R18_MPLL_ACG_RANGE);
+	if (priv->phy_version == MESON_USB2_PHY_VERSION_10)
+		regmap_write(priv->regmap, PHY_CTRL_R18,
+			     FIELD_PREP(PHY_CTRL_R18_MPLL_LKW_SEL, 1) |
+			     FIELD_PREP(PHY_CTRL_R18_MPLL_LK_W, 9) |
+			     FIELD_PREP(PHY_CTRL_R18_MPLL_LK_S, 0x27) |
+			     FIELD_PREP(PHY_CTRL_R18_MPLL_PFD_GAIN, 1) |
+			     FIELD_PREP(PHY_CTRL_R18_MPLL_ROU, 7) |
+			     FIELD_PREP(PHY_CTRL_R18_MPLL_DATA_SEL, 3) |
+			     FIELD_PREP(PHY_CTRL_R18_MPLL_BIAS_ADJ, 1) |
+			     FIELD_PREP(PHY_CTRL_R18_MPLL_BB_MODE, 0) |
+			     FIELD_PREP(PHY_CTRL_R18_MPLL_ALPHA, 3) |
+			     FIELD_PREP(PHY_CTRL_R18_MPLL_ADJ_LDO, 1) |
+			     PHY_CTRL_R18_MPLL_ACG_RANGE);
+	else if (priv->phy_version == MESON_USB2_PHY_VERSION_11)
+		regmap_write(priv->regmap, PHY_CTRL_R18,
+			     FIELD_PREP(PHY_CTRL_R18_MPLL_LKW_SEL, 1) |
+			     FIELD_PREP(PHY_CTRL_R18_MPLL_LK_W, 9) |
+			     FIELD_PREP(PHY_CTRL_R18_MPLL_LK_S, 0x27) |
+			     FIELD_PREP(PHY_CTRL_R18_MPLL_PFD_GAIN, 1) |
+			     FIELD_PREP(PHY_CTRL_R18_MPLL_ROU, 7) |
+			     FIELD_PREP(PHY_CTRL_R18_MPLL_DATA_SEL, 3) |
+			     FIELD_PREP(PHY_CTRL_R18_MPLL_BIAS_ADJ, 1) |
+			     FIELD_PREP(PHY_CTRL_R18_MPLL_BB_MODE, 0) |
+			     FIELD_PREP(PHY_CTRL_R18_MPLL_ALPHA, 3) |
+			     FIELD_PREP(PHY_CTRL_R18_MPLL_ADJ_LDO, 1) |
+			     PHY_CTRL_R18_MPLL_ACG_RANGE |
+			     PHY_CTRL_R18_MPLL_DCO_CLK_SEL);
The only difference is PHY_CTRL_R18_MPLL_ACG_RANGE | PHY_CTRL_R18_MPLL_DCO_CLK_SEL,
you can easily simplify the code here by using a temp variable.
Yes, it will looks more clearly.
quoted
quoted
 
 	udelay(PLL_RESET_COMPLETE_TIME);
 
@@ -227,13 +249,24 @@ static int phy_meson_g12a_usb2_init(struct phy *phy)
 		     FIELD_PREP(PHY_CTRL_R20_USB2_BGR_VREF_4_0, 0) |
 		     FIELD_PREP(PHY_CTRL_R20_USB2_BGR_DBG_1_0, 0));
 
-	regmap_write(priv->regmap, PHY_CTRL_R4,
-		     FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_7_0, 0xf) |
-		     FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_15_8, 0xf) |
-		     FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_23_16, 0xf) |
-		     PHY_CTRL_R4_TEST_BYPASS_MODE_EN |
-		     FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_1_0, 0) |
-		     FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_3_2, 0));
+	if (priv->phy_version == MESON_USB2_PHY_VERSION_10)
+		regmap_write(priv->regmap, PHY_CTRL_R4,
+			     FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_7_0, 0xf) |
+			     FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_15_8, 0xf) |
+			     FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_23_16, 0xf) |
+			     PHY_CTRL_R4_TEST_BYPASS_MODE_EN |
+			     FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_1_0, 0) |
+			     FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_3_2, 0));
+	else if (priv->phy_version == MESON_USB2_PHY_VERSION_11) {
+		regmap_write(priv->regmap, PHY_CTRL_R21,
+			     PHY_CTRL_R21_USB2_CAL_ACK_EN |
+			     PHY_CTRL_R21_USB2_TX_STRG_PD |
+			     FIELD_PREP(PHY_CTRL_R21_USB2_OTG_ACA_TRIM_1_0, 2));
+
+		/* Analog Settings */
+		regmap_write(priv->regmap, PHY_CTRL_R13,
+			     FIELD_PREP(PHY_CTRL_R13_MIN_COUNT_FOR_SYNC_DET, 7));
+	}
 
 	/* Tuning Disconnect Threshold */
 	regmap_write(priv->regmap, PHY_CTRL_R3,
@@ -241,11 +274,13 @@ static int phy_meson_g12a_usb2_init(struct phy *phy)
 		     FIELD_PREP(PHY_CTRL_R3_HSDIC_REF, 1) |
 		     FIELD_PREP(PHY_CTRL_R3_DISC_THRESH, 3));
 
-	/* Analog Settings */
-	regmap_write(priv->regmap, PHY_CTRL_R14, 0);
-	regmap_write(priv->regmap, PHY_CTRL_R13,
-		     PHY_CTRL_R13_UPDATE_PMA_SIGNALS |
-		     FIELD_PREP(PHY_CTRL_R13_MIN_COUNT_FOR_SYNC_DET, 7));
+	if (priv->phy_version == MESON_USB2_PHY_VERSION_10) {
+		/* Analog Settings */
+		regmap_write(priv->regmap, PHY_CTRL_R14, 0);
+		regmap_write(priv->regmap, PHY_CTRL_R13,
+			     PHY_CTRL_R13_UPDATE_PMA_SIGNALS |
+			     FIELD_PREP(PHY_CTRL_R13_MIN_COUNT_FOR_SYNC_DET, 7));
+	}
 
 	return 0;
 }
@@ -271,6 +306,7 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct phy_meson_g12a_usb2_priv *priv;
 	struct phy *phy;
+	struct device_node *np = dev->of_node;
 	void __iomem *base;
 	int ret;
 
@@ -286,14 +322,23 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
+	if (of_device_is_compatible(np, "amlogic,g12a-usb2-phy"))
+		priv->phy_version = MESON_USB2_PHY_VERSION_10;
+	else if (of_device_is_compatible(np, "amlogic,a1-usb2-phy"))
+		priv->phy_version = MESON_USB2_PHY_VERSION_11;
+	else
+		return -EINVAL;
Please use of_device_get_match_data() and a match data for each compatible instead.
OK, I will fix it in next version.
quoted
quoted
+
 	priv->regmap = devm_regmap_init_mmio(dev, base,
 					     &phy_meson_g12a_usb2_regmap_conf);
 	if (IS_ERR(priv->regmap))
 		return PTR_ERR(priv->regmap);
 
-	priv->clk = devm_clk_get(dev, "xtal");
-	if (IS_ERR(priv->clk))
-		return PTR_ERR(priv->clk);
+	if (priv->phy_version == MESON_USB2_PHY_VERSION_10) {
+		priv->clk = devm_clk_get(dev, "xtal");
+		if (IS_ERR(priv->clk))
+			return PTR_ERR(priv->clk);
+	}
 
 	priv->reset = devm_reset_control_get(dev, "phy");
 	if (IS_ERR(priv->reset))
@@ -322,7 +367,8 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
 
 static const struct of_device_id phy_meson_g12a_usb2_of_match[] = {
 	{ .compatible = "amlogic,g12a-usb2-phy", },
-	{ },
+	{ .compatible = "amlogic,a1-usb2-phy", },
+	{ /* Sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, phy_meson_g12a_usb2_of_match);
 
Thanks,
Neil

.
Thanks,
Hanjie.Lin
.

Thanks,
Hanjie.Lin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help