[PATCH v2 0/2] phy: ti: j721e-wiz: Add support for DisplayPort mode

[PATCH v2 0/2] phy: ti: j721e-wiz: Add support for DisplayPort mode

[Jyri Sarha]

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(-)

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

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

[Jyri Sarha]

Add definition for DisplayPort phy type.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
Reviewed-by: Roger Quadros <rogerq@ti.com>
Reviewed-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 include/dt-bindings/phy/phy.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/dt-bindings/phy/phy.h b/include/dt-bindings/phy/phy.h
index b6a1eaf1b339..1f3f866fae7b 100644
--- a/include/dt-bindings/phy/phy.h
+++ b/include/dt-bindings/phy/phy.h
@@ -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 */
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[Jyri Sarha]

On 08/01/2020 10:30, Jyri Sarha wrote:
> Add definition for DisplayPort phy type.
> 
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> Reviewed-by: Roger Quadros <rogerq@ti.com>
> Reviewed-by: Kishon Vijay Abraham I <kishon@ti.com>

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

> ---
>  include/dt-bindings/phy/phy.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/dt-bindings/phy/phy.h b/include/dt-bindings/phy/phy.h
> index b6a1eaf1b339..1f3f866fae7b 100644
> --- a/include/dt-bindings/phy/phy.h
> +++ b/include/dt-bindings/phy/phy.h
> @@ -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 */
> 


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

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

[Kishon Vijay Abraham I]

On 09/01/20 4:57 PM, Jyri Sarha wrote:
> On 08/01/2020 10:30, Jyri Sarha wrote:
>> Add definition for DisplayPort phy type.
>>
>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>> Reviewed-by: Roger Quadros <rogerq@ti.com>
>> Reviewed-by: Kishon Vijay Abraham I <kishon@ti.com>
> 
> 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

> 
> Best regards,
> Jyri
> 
>> ---
>>  include/dt-bindings/phy/phy.h | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/include/dt-bindings/phy/phy.h b/include/dt-bindings/phy/phy.h
>> index b6a1eaf1b339..1f3f866fae7b 100644
>> --- a/include/dt-bindings/phy/phy.h
>> +++ b/include/dt-bindings/phy/phy.h
>> @@ -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 */
>>
> 
> 

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

[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 <jsarha@ti.com>
---
 drivers/phy/ti/phy-j721e-wiz.c | 59 +++++++++++++++++++++++++++++++---
 1 file changed, 55 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c
index b5f6019e5c7d..22bc04846cdb 100644
--- a/drivers/phy/ti/phy-j721e-wiz.c
+++ b/drivers/phy/ti/phy-j721e-wiz.c
@@ -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;
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[Kishon Vijay Abraham I]

Hi Jyri,

On 08/01/20 2:00 PM, Jyri Sarha wrote:
> 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 <jsarha@ti.com>
> ---
>  drivers/phy/ti/phy-j721e-wiz.c | 59 +++++++++++++++++++++++++++++++---
>  1 file changed, 55 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c
> index b5f6019e5c7d..22bc04846cdb 100644
> --- a/drivers/phy/ti/phy-j721e-wiz.c
> +++ b/drivers/phy/ti/phy-j721e-wiz.c
> @@ -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);
This would require the Torrent bindings to be Reviewed and Acked.

Thanks
Kishon

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

[Jyri Sarha]

On 09/01/2020 09:44, Kishon Vijay Abraham I wrote:
> Hi Jyri,
> 
> On 08/01/20 2:00 PM, Jyri Sarha wrote:
>> 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 <jsarha@ti.com>
>> ---
>>  drivers/phy/ti/phy-j721e-wiz.c | 59 +++++++++++++++++++++++++++++++---
>>  1 file changed, 55 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c
>> index b5f6019e5c7d..22bc04846cdb 100644
>> --- a/drivers/phy/ti/phy-j721e-wiz.c
>> +++ b/drivers/phy/ti/phy-j721e-wiz.c
>> @@ -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);
> 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


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