Skip to contentSkip to conversation

phy: ti: j721e-wiz: Add support for DisplayPort mode

PATCH In review Jyri Sarha 2 participants 7 messages Jan 8, 2020 – Mar 24, 2020

Subsystems (2)
Device tree bindings Generic PHY framework
lore
Share & tools
lore

Jyri Sarha

author
#0 lore raw

v2 cover phy: ti: j721e-wiz: Add support for DisplayPort mode

Changes since the first version:
- Remove the ti,j721e-wiz-10g binding change ("lane<n>-mode"-property)
- Add code to dig lane phy-type from the serdes's link subnodes cdns,phy-type property, defined in sierra-phy-t0 binding and hopefully soon in torrent-phy binding too.
- Rebase the patches on top of phy-next

The wiz wrapper need slightly different configuration when the wrapped serdes is used for DisplayPort. This series adds devicetree properties for selecting the mode for each individual serdes lane separately.

Jyri Sarha (2):
dt-bindings: phy: Add PHY_TYPE_DP definition
phy: ti: j721e-wiz: Implement DisplayPort mode to the wiz driver

 drivers/phy/ti/phy-j721e-wiz.c | 59 +++++++++++++++++++++++++++++++---
 include/dt-bindings/phy/phy.h  |  1 +
 2 files changed, 56 insertions(+), 4 deletions(-)
Signature · 1 line
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
author
#1 lore raw

PATCH v2 1/2 dt-bindings: phy: Add PHY_TYPE_DP definition

Add definition for DisplayPort phy type.

Signed-off-by Jyri Sarha <[email protected]>
Reviewed-by Roger Quadros <[email protected]>
Reviewed-by Kishon Vijay Abraham I <[email protected]>
---
 include/dt-bindings/phy/phy.h | 1 +
 1 file changed, 1 insertion(+)
include/dt-bindings/phy/phy.h +1 -0
index b6a1eaf1b339..1f3f866fae7b 100644
@@ -16,5 +16,6 @@
 #define PHY_TYPE_USB2		3
 #define PHY_TYPE_USB3		4
 #define PHY_TYPE_UFS		5
+#define PHY_TYPE_DP		6
 
 #endif /* _DT_BINDINGS_PHY */
Signature · 1 line
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
author
#2 lore raw

Re: [PATCH v2 1/2] dt-bindings: phy: Add PHY_TYPE_DP definition

Quoted 5 lines · Jyri Sarha

Add definition for DisplayPort phy type.

Signed-off-by Jyri Sarha <[email protected]>
Reviewed-by Roger Quadros <[email protected]>
Reviewed-by Kishon Vijay Abraham I <[email protected]>

Kishon, maybe you could pick only this patch phy-next, so that we avoid nasty cross dependencies if I get to push DisplayPort dts patches to mainline early.

Best regards,
Jyri

Quoted 16 lines
---
 include/dt-bindings/phy/phy.h | 1 +
 1 file changed, 1 insertion(+)
include/dt-bindings/phy/phy.h +1 -0
index b6a1eaf1b339..1f3f866fae7b 100644
@@ -16,5 +16,6 @@
 #define PHY_TYPE_USB2		3
 #define PHY_TYPE_USB3		4
 #define PHY_TYPE_UFS		5
+#define PHY_TYPE_DP		6
 
 #endif /* _DT_BINDINGS_PHY */
Signature · 2 lines
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
#3 lore raw

Re: [PATCH v2 1/2] dt-bindings: phy: Add PHY_TYPE_DP definition

Quoted 10 lines · Jyri Sarha
Quoted 5 lines · Jyri Sarha

Add definition for DisplayPort phy type.

Signed-off-by Jyri Sarha <[email protected]>
Reviewed-by Roger Quadros <[email protected]>
Reviewed-by Kishon Vijay Abraham I <[email protected]>

Kishon, maybe you could pick only this patch phy-next, so that we avoid nasty cross dependencies if I get to push DisplayPort dts patches to mainline early.

Sure. Picked them now.

Thanks
Kishon

Quoted 22 lines

Best regards,
Jyri

Quoted 16 lines
---
 include/dt-bindings/phy/phy.h | 1 +
 1 file changed, 1 insertion(+)
include/dt-bindings/phy/phy.h +1 -0
index b6a1eaf1b339..1f3f866fae7b 100644
@@ -16,5 +16,6 @@
 #define PHY_TYPE_USB2		3
 #define PHY_TYPE_USB3		4
 #define PHY_TYPE_UFS		5
+#define PHY_TYPE_DP		6
 
 #endif /* _DT_BINDINGS_PHY */
author
#4 lore raw

PATCH v2 2/2 phy: ti: j721e-wiz: Implement DisplayPort mode to the wiz driver

In reply to Jyri Sarha

For DisplayPort use we need to set WIZ_CONFIG_LANECTL register's P_STANDARD_MODE bits to "mode 3". In the DisplayPort use also the P_ENABLE bits of the same register are set to P_ENABLE instead of P_ENABLE_FORCE, so that the DisplayPort driver can enable and disable the lane as needed. The DisplayPort mode is selected according to "cdns,phy-type"-properties found in link subnodes under the managed serdes (see "ti,sierra-phy-t0" and "ti,j721e-serdes-10g" devicetree bindings for details). All other values of "cdns,phy-type"-property but PHY_TYPE_DP will set P_STANDARD_MODE bits to 0 and P_ENABLE bits to force enable.

Signed-off-by Jyri Sarha <[email protected]>
---
 drivers/phy/ti/phy-j721e-wiz.c | 59 +++++++++++++++++++++++++++++++---
 1 file changed, 55 insertions(+), 4 deletions(-)
drivers/phy/ti/phy-j721e-wiz.c +55 -4
index b5f6019e5c7d..22bc04846cdb 100644
@@ -20,6 +20,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/reset-controller.h>
+#include <dt-bindings/phy/phy.h>
 
 #define WIZ_SERDES_CTRL		0x404
 #define WIZ_SERDES_TOP_CTRL	0x408

@@ -78,6 +79,8 @@ static const struct reg_field p_enable[WIZ_MAX_LANES] = {
 	REG_FIELD(WIZ_LANECTL(3), 30, 31),
 };
 
+enum p_enable { P_ENABLE = 2, P_ENABLE_FORCE = 1, P_ENABLE_DISABLE = 0 };
+
 static const struct reg_field p_align[WIZ_MAX_LANES] = {
 	REG_FIELD(WIZ_LANECTL(0), 29, 29),
 	REG_FIELD(WIZ_LANECTL(1), 29, 29),

@@ -220,6 +223,7 @@ struct wiz {
 	struct reset_controller_dev wiz_phy_reset_dev;
 	struct gpio_desc	*gpio_typec_dir;
 	int			typec_dir_delay;
+	u32 lane_phy_type[WIZ_MAX_LANES];
 };
 
 static int wiz_reset(struct wiz *wiz)

@@ -242,12 +246,17 @@ static int wiz_reset(struct wiz *wiz)
 static int wiz_mode_select(struct wiz *wiz)
 {
 	u32 num_lanes = wiz->num_lanes;
+	enum wiz_lane_standard_mode mode;
 	int ret;
 	int i;
 
 	for (i = 0; i < num_lanes; i++) {
-		ret = regmap_field_write(wiz->p_standard_mode[i],
-					 LANE_MODE_GEN4);
+		if (wiz->lane_phy_type[i] == PHY_TYPE_DP)
+			mode = LANE_MODE_GEN1;
+		else
+			mode = LANE_MODE_GEN4;
+
+		ret = regmap_field_write(wiz->p_standard_mode[i], mode);
 		if (ret)
 			return ret;
 	}

@@ -707,7 +716,7 @@ static int wiz_phy_reset_assert(struct reset_controller_dev *rcdev,
 		return ret;
 	}
 
-	ret = regmap_field_write(wiz->p_enable[id - 1], false);
+	ret = regmap_field_write(wiz->p_enable[id - 1], P_ENABLE_DISABLE);
 	return ret;
 }
 

@@ -734,7 +743,11 @@ static int wiz_phy_reset_deassert(struct reset_controller_dev *rcdev,
 		return ret;
 	}
 
-	ret = regmap_field_write(wiz->p_enable[id - 1], true);
+	if (wiz->lane_phy_type[id - 1] == PHY_TYPE_DP)
+		ret = regmap_field_write(wiz->p_enable[id - 1], P_ENABLE);
+	else
+		ret = regmap_field_write(wiz->p_enable[id - 1], P_ENABLE_FORCE);
+
 	return ret;
 }
 

@@ -761,6 +774,40 @@ static const struct of_device_id wiz_id_table[] = {
 };
 MODULE_DEVICE_TABLE(of, wiz_id_table);
 
+static int wiz_get_lane_phy_types(struct device *dev, struct wiz *wiz)
+{
+	struct device_node *serdes, *subnode;
+
+	serdes = of_get_child_by_name(dev->of_node, "serdes");
+	if (!serdes) {
+		dev_err(dev, "%s: Getting \"serdes\"-node failed\n", __func__);
+		return -EINVAL;
+	}
+
+	for_each_child_of_node(serdes, subnode) {
+		u32 reg, num_lanes = 1, phy_type = PHY_NONE;
+		int ret, i;
+
+		ret = of_property_read_u32(subnode, "reg", &reg);
+		if (ret) {
+			dev_err(dev,
+				"%s: Reading \"reg\" from \"%s\" failed: %d\n",
+				__func__, subnode->name, ret);
+			return ret;
+		}
+		of_property_read_u32(subnode, "cdns,num-lanes", &num_lanes);
+		of_property_read_u32(subnode, "cdns,phy-type", &phy_type);
+
+		dev_dbg(dev, "%s: Lanes %u-%u have phy-type %u\n", __func__,
+			reg, reg + num_lanes - 1, phy_type);
+
+		for (i = reg; i < reg + num_lanes; i++)
+			wiz->lane_phy_type[i] = phy_type;
+	}
+
+	return 0;
+}
+
 static int wiz_probe(struct platform_device *pdev)
 {
 	struct reset_controller_dev *phy_reset_dev;

@@ -844,6 +891,10 @@ static int wiz_probe(struct platform_device *pdev)
 		}
 	}
 
+	ret = wiz_get_lane_phy_types(dev, wiz);
+	if (ret)
+		return ret;
+
 	wiz->dev = dev;
 	wiz->regmap = regmap;
 	wiz->num_lanes = num_lanes;
Signature · 1 line
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
#5 lore raw

Re: [PATCH v2 2/2] phy: ti: j721e-wiz: Implement DisplayPort mode to the wiz driver

Hi Jyri,

Quoted 104 lines · Jyri Sarha

For DisplayPort use we need to set WIZ_CONFIG_LANECTL register's P_STANDARD_MODE bits to "mode 3". In the DisplayPort use also the P_ENABLE bits of the same register are set to P_ENABLE instead of P_ENABLE_FORCE, so that the DisplayPort driver can enable and disable the lane as needed. The DisplayPort mode is selected according to "cdns,phy-type"-properties found in link subnodes under the managed serdes (see "ti,sierra-phy-t0" and "ti,j721e-serdes-10g" devicetree bindings for details). All other values of "cdns,phy-type"-property but PHY_TYPE_DP will set P_STANDARD_MODE bits to 0 and P_ENABLE bits to force enable.

Signed-off-by Jyri Sarha <[email protected]>
---
 drivers/phy/ti/phy-j721e-wiz.c | 59 +++++++++++++++++++++++++++++++---
 1 file changed, 55 insertions(+), 4 deletions(-)
drivers/phy/ti/phy-j721e-wiz.c +30 -4
index b5f6019e5c7d..22bc04846cdb 100644
@@ -20,6 +20,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/reset-controller.h>
+#include <dt-bindings/phy/phy.h>
 
 #define WIZ_SERDES_CTRL		0x404
 #define WIZ_SERDES_TOP_CTRL	0x408

@@ -78,6 +79,8 @@ static const struct reg_field p_enable[WIZ_MAX_LANES] = {
 	REG_FIELD(WIZ_LANECTL(3), 30, 31),
 };
 
+enum p_enable { P_ENABLE = 2, P_ENABLE_FORCE = 1, P_ENABLE_DISABLE = 0 };
+
 static const struct reg_field p_align[WIZ_MAX_LANES] = {
 	REG_FIELD(WIZ_LANECTL(0), 29, 29),
 	REG_FIELD(WIZ_LANECTL(1), 29, 29),

@@ -220,6 +223,7 @@ struct wiz {
 	struct reset_controller_dev wiz_phy_reset_dev;
 	struct gpio_desc	*gpio_typec_dir;
 	int			typec_dir_delay;
+	u32 lane_phy_type[WIZ_MAX_LANES];
 };
 
 static int wiz_reset(struct wiz *wiz)

@@ -242,12 +246,17 @@ static int wiz_reset(struct wiz *wiz)
 static int wiz_mode_select(struct wiz *wiz)
 {
 	u32 num_lanes = wiz->num_lanes;
+	enum wiz_lane_standard_mode mode;
 	int ret;
 	int i;
 
 	for (i = 0; i < num_lanes; i++) {
-		ret = regmap_field_write(wiz->p_standard_mode[i],
-					 LANE_MODE_GEN4);
+		if (wiz->lane_phy_type[i] == PHY_TYPE_DP)
+			mode = LANE_MODE_GEN1;
+		else
+			mode = LANE_MODE_GEN4;
+
+		ret = regmap_field_write(wiz->p_standard_mode[i], mode);
 		if (ret)
 			return ret;
 	}

@@ -707,7 +716,7 @@ static int wiz_phy_reset_assert(struct reset_controller_dev *rcdev,
 		return ret;
 	}
 
-	ret = regmap_field_write(wiz->p_enable[id - 1], false);
+	ret = regmap_field_write(wiz->p_enable[id - 1], P_ENABLE_DISABLE);
 	return ret;
 }
 

@@ -734,7 +743,11 @@ static int wiz_phy_reset_deassert(struct reset_controller_dev *rcdev,
 		return ret;
 	}
 
-	ret = regmap_field_write(wiz->p_enable[id - 1], true);
+	if (wiz->lane_phy_type[id - 1] == PHY_TYPE_DP)
+		ret = regmap_field_write(wiz->p_enable[id - 1], P_ENABLE);
+	else
+		ret = regmap_field_write(wiz->p_enable[id - 1], P_ENABLE_FORCE);
+
 	return ret;
 }
 

@@ -761,6 +774,40 @@ static const struct of_device_id wiz_id_table[] = {
 };
 MODULE_DEVICE_TABLE(of, wiz_id_table);
 
+static int wiz_get_lane_phy_types(struct device *dev, struct wiz *wiz)
+{
+	struct device_node *serdes, *subnode;
+
+	serdes = of_get_child_by_name(dev->of_node, "serdes");
+	if (!serdes) {
+		dev_err(dev, "%s: Getting \"serdes\"-node failed\n", __func__);
+		return -EINVAL;
+	}
+
+	for_each_child_of_node(serdes, subnode) {
+		u32 reg, num_lanes = 1, phy_type = PHY_NONE;
+		int ret, i;

drivers/phy/ti/phy-j721e-wiz.c static const struct of_device_id wiz_id_table[] = { · line 799

Quoted 10 lines
drivers/phy/ti/phy-j721e-wiz.c +10 -0
+
+		ret = of_property_read_u32(subnode, "reg", &reg);
+		if (ret) {
+			dev_err(dev,
+				"%s: Reading \"reg\" from \"%s\" failed: %d\n",
+				__func__, subnode->name, ret);
+			return ret;
+		}
+		of_property_read_u32(subnode, "cdns,num-lanes", &num_lanes);
+		of_property_read_u32(subnode, "cdns,phy-type", &phy_type);

This would require the Torrent bindings to be Reviewed and Acked.

Thanks
Kishon

author
#6 lore raw

Re: [PATCH v2 2/2] phy: ti: j721e-wiz: Implement DisplayPort mode to the wiz driver

Quoted 110 lines · Kishon Vijay Abraham I

Hi Jyri,

Quoted 107 lines · Jyri Sarha

For DisplayPort use we need to set WIZ_CONFIG_LANECTL register's P_STANDARD_MODE bits to "mode 3". In the DisplayPort use also the P_ENABLE bits of the same register are set to P_ENABLE instead of P_ENABLE_FORCE, so that the DisplayPort driver can enable and disable the lane as needed. The DisplayPort mode is selected according to "cdns,phy-type"-properties found in link subnodes under the managed serdes (see "ti,sierra-phy-t0" and "ti,j721e-serdes-10g" devicetree bindings for details). All other values of "cdns,phy-type"-property but PHY_TYPE_DP will set P_STANDARD_MODE bits to 0 and P_ENABLE bits to force enable.

Signed-off-by Jyri Sarha <[email protected]>
---
 drivers/phy/ti/phy-j721e-wiz.c | 59 +++++++++++++++++++++++++++++++---
 1 file changed, 55 insertions(+), 4 deletions(-)
drivers/phy/ti/phy-j721e-wiz.c +33 -4
index b5f6019e5c7d..22bc04846cdb 100644
@@ -20,6 +20,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/reset-controller.h>
+#include <dt-bindings/phy/phy.h>
 
 #define WIZ_SERDES_CTRL		0x404
 #define WIZ_SERDES_TOP_CTRL	0x408

@@ -78,6 +79,8 @@ static const struct reg_field p_enable[WIZ_MAX_LANES] = {
 	REG_FIELD(WIZ_LANECTL(3), 30, 31),
 };
 
+enum p_enable { P_ENABLE = 2, P_ENABLE_FORCE = 1, P_ENABLE_DISABLE = 0 };
+
 static const struct reg_field p_align[WIZ_MAX_LANES] = {
 	REG_FIELD(WIZ_LANECTL(0), 29, 29),
 	REG_FIELD(WIZ_LANECTL(1), 29, 29),

@@ -220,6 +223,7 @@ struct wiz {
 	struct reset_controller_dev wiz_phy_reset_dev;
 	struct gpio_desc	*gpio_typec_dir;
 	int			typec_dir_delay;
+	u32 lane_phy_type[WIZ_MAX_LANES];
 };
 
 static int wiz_reset(struct wiz *wiz)

@@ -242,12 +246,17 @@ static int wiz_reset(struct wiz *wiz)
 static int wiz_mode_select(struct wiz *wiz)
 {
 	u32 num_lanes = wiz->num_lanes;
+	enum wiz_lane_standard_mode mode;
 	int ret;
 	int i;
 
 	for (i = 0; i < num_lanes; i++) {
-		ret = regmap_field_write(wiz->p_standard_mode[i],
-					 LANE_MODE_GEN4);
+		if (wiz->lane_phy_type[i] == PHY_TYPE_DP)
+			mode = LANE_MODE_GEN1;
+		else
+			mode = LANE_MODE_GEN4;
+
+		ret = regmap_field_write(wiz->p_standard_mode[i], mode);
 		if (ret)
 			return ret;
 	}

@@ -707,7 +716,7 @@ static int wiz_phy_reset_assert(struct reset_controller_dev *rcdev,
 		return ret;
 	}
 
-	ret = regmap_field_write(wiz->p_enable[id - 1], false);
+	ret = regmap_field_write(wiz->p_enable[id - 1], P_ENABLE_DISABLE);
 	return ret;
 }
 

@@ -734,7 +743,11 @@ static int wiz_phy_reset_deassert(struct reset_controller_dev *rcdev,
 		return ret;
 	}
 
-	ret = regmap_field_write(wiz->p_enable[id - 1], true);
+	if (wiz->lane_phy_type[id - 1] == PHY_TYPE_DP)
+		ret = regmap_field_write(wiz->p_enable[id - 1], P_ENABLE);
+	else
+		ret = regmap_field_write(wiz->p_enable[id - 1], P_ENABLE_FORCE);
+
 	return ret;
 }
 

@@ -761,6 +774,40 @@ static const struct of_device_id wiz_id_table[] = {
 };
 MODULE_DEVICE_TABLE(of, wiz_id_table);
 
+static int wiz_get_lane_phy_types(struct device *dev, struct wiz *wiz)
+{
+	struct device_node *serdes, *subnode;
+
+	serdes = of_get_child_by_name(dev->of_node, "serdes");
+	if (!serdes) {
+		dev_err(dev, "%s: Getting \"serdes\"-node failed\n", __func__);
+		return -EINVAL;
+	}
+
+	for_each_child_of_node(serdes, subnode) {
+		u32 reg, num_lanes = 1, phy_type = PHY_NONE;
+		int ret, i;
+
+		ret = of_property_read_u32(subnode, "reg", &reg);
+		if (ret) {
Quoted 9 lines

drivers/phy/ti/phy-j721e-wiz.c static const struct of_device_id wiz_id_table[] = { · line 799

Quoted 7 lines
drivers/phy/ti/phy-j721e-wiz.c +7 -0
+			dev_err(dev,
+				"%s: Reading \"reg\" from \"%s\" failed: %d\n",
+				__func__, subnode->name, ret);
+			return ret;
+		}
+		of_property_read_u32(subnode, "cdns,num-lanes", &num_lanes);
+		of_property_read_u32(subnode, "cdns,phy-type", &phy_type);

This would require the Torrent bindings to be Reviewed and Acked.

I think they are now in your next branch. Could you please pick this one up too?

BR,
Jyri

Signature · 2 lines
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki