[4/7] phy: qcom-qmp-combo: Introduce orientation switching

  The data lanes of the QMP PHY is swapped in order to handle changing
orientation of the USB Type-C cable. Register a typec_switch device to
allow a TCPM to configure the orientation.

The newly introduced orientation variable is adjusted based on the
request, and the initialized components are brought down and up again.
To keep track of what parts needs to be cycled new variables to keep
track of the individual init_count is introduced.

Both the USB and the DisplayPort altmode signals are properly switched.
For DisplayPort the controller will after the TCPM having established
orientation power on the PHY, so this is not done implicitly, but for
USB the PHY typically is kept initialized across the switch, and must
therefor then be reinitialized.

This is based on initial work by Wesley Cheng.

Link: https://lore.kernel.org/r/20201009082843.28503-3-wcheng@codeaurora.org/
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 92 ++++++++++++++++++++---
 1 file changed, 83 insertions(+), 9 deletions(-)
  

Hi,

On 25/04/2023 05:40, Bjorn Andersson wrote:
> The data lanes of the QMP PHY is swapped in order to handle changing
> orientation of the USB Type-C cable. Register a typec_switch device to
> allow a TCPM to configure the orientation.
> 
> The newly introduced orientation variable is adjusted based on the
> request, and the initialized components are brought down and up again.
> To keep track of what parts needs to be cycled new variables to keep
> track of the individual init_count is introduced.
> 
> Both the USB and the DisplayPort altmode signals are properly switched.
> For DisplayPort the controller will after the TCPM having established
> orientation power on the PHY, so this is not done implicitly, but for
> USB the PHY typically is kept initialized across the switch, and must
> therefor then be reinitialized.

   therefore
> 
> This is based on initial work by Wesley Cheng.
> 
> Link: https://lore.kernel.org/r/20201009082843.28503-3-wcheng@codeaurora.org/
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>   drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 92 ++++++++++++++++++++---
>   1 file changed, 83 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 6748f31da7a3..5d6d6ef3944b 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -20,6 +20,7 @@
>   #include <linux/reset.h>
>   #include <linux/slab.h>
>   #include <linux/usb/typec.h>
> +#include <linux/usb/typec_mux.h>
>   
>   #include <dt-bindings/phy/phy-qcom-qmp.h>
>   
> @@ -1320,15 +1321,18 @@ struct qmp_combo {
>   
>   	struct phy *usb_phy;
>   	enum phy_mode mode;
> +	unsigned int usb_init_count;
>   
>   	struct phy *dp_phy;
>   	unsigned int dp_aux_cfg;
>   	struct phy_configure_opts_dp dp_opts;
> +	unsigned int dp_init_count;
>   
>   	struct clk_fixed_rate pipe_clk_fixed;
>   	struct clk_hw dp_link_hw;
>   	struct clk_hw dp_pixel_hw;
>   
> +	struct typec_switch_dev *sw;
>   	enum typec_orientation orientation;
>   };
>   
> @@ -2458,14 +2462,14 @@ static int qmp_combo_dp_calibrate(struct phy *phy)
>   	return 0;
>   }
>   
> -static int qmp_combo_com_init(struct qmp_combo *qmp)
> +static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
>   {
>   	const struct qmp_phy_cfg *cfg = qmp->cfg;
>   	void __iomem *com = qmp->com;
>   	int ret;
>   	u32 val;
>   
> -	if (qmp->init_count++)
> +	if (!force && qmp->init_count++)
>   		return 0;
>   
>   	ret = regulator_bulk_enable(cfg->num_vregs, qmp->vregs);
> @@ -2526,11 +2530,11 @@ static int qmp_combo_com_init(struct qmp_combo *qmp)
>   	return ret;
>   }
>   
> -static int qmp_combo_com_exit(struct qmp_combo *qmp)
> +static int qmp_combo_com_exit(struct qmp_combo *qmp, bool force)
>   {
>   	const struct qmp_phy_cfg *cfg = qmp->cfg;
>   
> -	if (--qmp->init_count)
> +	if (!force && --qmp->init_count)
>   		return 0;
>   
>   	reset_control_bulk_assert(cfg->num_resets, qmp->resets);
> @@ -2550,12 +2554,14 @@ static int qmp_combo_dp_init(struct phy *phy)
>   
>   	mutex_lock(&qmp->phy_mutex);
>   
> -	ret = qmp_combo_com_init(qmp);
> +	ret = qmp_combo_com_init(qmp, false);
>   	if (ret)
>   		goto out_unlock;
>   
>   	cfg->dp_aux_init(qmp);
>   
> +	qmp->dp_init_count++;
> +
>   out_unlock:
>   	mutex_unlock(&qmp->phy_mutex);
>   	return ret;
> @@ -2567,8 +2573,9 @@ static int qmp_combo_dp_exit(struct phy *phy)
>   
>   	mutex_lock(&qmp->phy_mutex);
>   
> -	qmp_combo_com_exit(qmp);
> +	qmp_combo_com_exit(qmp, false);
>   
> +	qmp->dp_init_count--;
>   	mutex_unlock(&qmp->phy_mutex);
>   
>   	return 0;
> @@ -2688,16 +2695,18 @@ static int qmp_combo_usb_init(struct phy *phy)
>   	int ret;
>   
>   	mutex_lock(&qmp->phy_mutex);
> -	ret = qmp_combo_com_init(qmp);
> +	ret = qmp_combo_com_init(qmp, false);
>   	if (ret)
>   		goto out_unlock;
>   
>   	ret = qmp_combo_usb_power_on(phy);
>   	if (ret) {
> -		qmp_combo_com_exit(qmp);
> +		qmp_combo_com_exit(qmp, false);
>   		goto out_unlock;
>   	}
>   
> +	qmp->usb_init_count++;
> +
>   out_unlock:
>   	mutex_unlock(&qmp->phy_mutex);
>   	return ret;
> @@ -2713,10 +2722,12 @@ static int qmp_combo_usb_exit(struct phy *phy)
>   	if (ret)
>   		goto out_unlock;
>   
> -	ret = qmp_combo_com_exit(qmp);
> +	ret = qmp_combo_com_exit(qmp, false);
>   	if (ret)
>   		goto out_unlock;
>   
> +	qmp->usb_init_count--;
> +
>   out_unlock:
>   	mutex_unlock(&qmp->phy_mutex);
>   	return ret;
> @@ -3351,6 +3362,65 @@ static struct phy *qmp_combo_phy_xlate(struct device *dev, struct of_phandle_arg
>   	return ERR_PTR(-EINVAL);
>   }
>   
> +#if IS_ENABLED(CONFIG_TYPEC)
> +static int qmp_combo_typec_switch_set(struct typec_switch_dev *sw,
> +				      enum typec_orientation orientation)
> +{
> +	struct qmp_combo *qmp = typec_switch_get_drvdata(sw);
> +	const struct qmp_phy_cfg *cfg = qmp->cfg;
> +
> +	if (orientation == qmp->orientation || orientation == TYPEC_ORIENTATION_NONE)
> +		return 0;
> +
> +	mutex_lock(&qmp->phy_mutex);
> +	qmp->orientation = orientation;
> +
> +	if (qmp->init_count) {
> +		if (qmp->usb_init_count)
> +			qmp_combo_usb_power_off(qmp->usb_phy);
> +		qmp_combo_com_exit(qmp, true);
> +
> +		qmp_combo_com_init(qmp, true);
> +		if (qmp->usb_init_count)
> +			qmp_combo_usb_power_on(qmp->usb_phy);
> +		if (qmp->dp_init_count)
> +			cfg->dp_aux_init(qmp);
> +	}
> +	mutex_unlock(&qmp->phy_mutex);
> +
> +	return 0;
> +}
> +
> +static void qmp_combo_typec_unregister(void *data)
> +{
> +	struct qmp_combo *qmp = data;
> +
> +	typec_switch_unregister(qmp->sw);
> +}
> +
> +static int qmp_combo_typec_switch_register(struct qmp_combo *qmp)
> +{
> +	struct typec_switch_desc sw_desc = {};
> +	struct device *dev = qmp->dev;
> +
> +	sw_desc.drvdata = qmp;
> +	sw_desc.fwnode = dev->fwnode;
> +	sw_desc.set = qmp_combo_typec_switch_set;
> +	qmp->sw = typec_switch_register(dev, &sw_desc);
> +	if (IS_ERR(qmp->sw)) {
> +		dev_err(dev, "Unable to register typec switch: %pe\n", qmp->sw);
> +		return PTR_ERR(qmp->sw);
> +	}
> +
> +	return devm_add_action_or_reset(dev, qmp_combo_typec_unregister, qmp);
> +}
> +#else
> +static int qmp_combo_typec_switch_register(struct qmp_combo *qmp)
> +{
> +	return 0;
> +}
> +#endif
> +
>   static int qmp_combo_probe(struct platform_device *pdev)
>   {
>   	struct qmp_combo *qmp;
> @@ -3385,6 +3455,10 @@ static int qmp_combo_probe(struct platform_device *pdev)
>   	if (ret)
>   		return ret;
>   
> +	ret = qmp_combo_typec_switch_register(qmp);
> +	if (ret)
> +		return ret;
> +
>   	/* Check for legacy binding with child nodes. */
>   	usb_np = of_get_child_by_name(dev->of_node, "usb3-phy");
>   	if (usb_np) {

Thanks for taking care of this, with the commit typo fix:

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
  

On Mon, Apr 24, 2023 at 08:40:07PM -0700, Bjorn Andersson wrote:
> The data lanes of the QMP PHY is swapped in order to handle changing
> orientation of the USB Type-C cable. Register a typec_switch device to
> allow a TCPM to configure the orientation.
> 
> The newly introduced orientation variable is adjusted based on the
> request, and the initialized components are brought down and up again.
> To keep track of what parts needs to be cycled new variables to keep
> track of the individual init_count is introduced.
> 
> Both the USB and the DisplayPort altmode signals are properly switched.
> For DisplayPort the controller will after the TCPM having established
> orientation power on the PHY, so this is not done implicitly, but for
> USB the PHY typically is kept initialized across the switch, and must
> therefor then be reinitialized.
> 
> This is based on initial work by Wesley Cheng.
> 
> Link: https://lore.kernel.org/r/20201009082843.28503-3-wcheng@codeaurora.org/
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 92 ++++++++++++++++++++---
>  1 file changed, 83 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 6748f31da7a3..5d6d6ef3944b 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c

> @@ -2567,8 +2573,9 @@ static int qmp_combo_dp_exit(struct phy *phy)
>  
>  	mutex_lock(&qmp->phy_mutex);
>  
> -	qmp_combo_com_exit(qmp);
> +	qmp_combo_com_exit(qmp, false);
>  
> +	qmp->dp_init_count--;

Nit: add a newline for symmetry.

>  	mutex_unlock(&qmp->phy_mutex);
>  
>  	return 0;

> @@ -3351,6 +3362,65 @@ static struct phy *qmp_combo_phy_xlate(struct device *dev, struct of_phandle_arg
>  	return ERR_PTR(-EINVAL);
>  }
>  
> +#if IS_ENABLED(CONFIG_TYPEC)
> +static int qmp_combo_typec_switch_set(struct typec_switch_dev *sw,
> +				      enum typec_orientation orientation)
> +{
> +	struct qmp_combo *qmp = typec_switch_get_drvdata(sw);
> +	const struct qmp_phy_cfg *cfg = qmp->cfg;
> +
> +	if (orientation == qmp->orientation || orientation == TYPEC_ORIENTATION_NONE)
> +		return 0;
> +
> +	mutex_lock(&qmp->phy_mutex);
> +	qmp->orientation = orientation;
> +
> +	if (qmp->init_count) {
> +		if (qmp->usb_init_count)
> +			qmp_combo_usb_power_off(qmp->usb_phy);
> +		qmp_combo_com_exit(qmp, true);
> +
> +		qmp_combo_com_init(qmp, true);
> +		if (qmp->usb_init_count)
> +			qmp_combo_usb_power_on(qmp->usb_phy);
> +		if (qmp->dp_init_count)
> +			cfg->dp_aux_init(qmp);
> +	}
> +	mutex_unlock(&qmp->phy_mutex);
> +
> +	return 0;
> +}
> +
> +static void qmp_combo_typec_unregister(void *data)
> +{
> +	struct qmp_combo *qmp = data;
> +
> +	typec_switch_unregister(qmp->sw);
> +}
> +
> +static int qmp_combo_typec_switch_register(struct qmp_combo *qmp)
> +{
> +	struct typec_switch_desc sw_desc = {};
> +	struct device *dev = qmp->dev;
> +
> +	sw_desc.drvdata = qmp;
> +	sw_desc.fwnode = dev->fwnode;
> +	sw_desc.set = qmp_combo_typec_switch_set;

Nit: I'd add a newline here for readability.

> +	qmp->sw = typec_switch_register(dev, &sw_desc);
> +	if (IS_ERR(qmp->sw)) {
> +		dev_err(dev, "Unable to register typec switch: %pe\n", qmp->sw);
> +		return PTR_ERR(qmp->sw);
> +	}
> +
> +	return devm_add_action_or_reset(dev, qmp_combo_typec_unregister, qmp);
> +}
> +#else
> +static int qmp_combo_typec_switch_register(struct qmp_combo *qmp)
> +{
> +	return 0;
> +}
> +#endif

Perhaps you can move the type-c block after qmp_combo_register_clocks()
above to keep the type-c and later added bridge functions together and
better reflect the probe init order (e.g. keeping the dt-functions just
above probe()).

> +
>  static int qmp_combo_probe(struct platform_device *pdev)
>  {
>  	struct qmp_combo *qmp;
> @@ -3385,6 +3455,10 @@ static int qmp_combo_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	ret = qmp_combo_typec_switch_register(qmp);
> +	if (ret)
> +		return ret;
> +
>  	/* Check for legacy binding with child nodes. */
>  	usb_np = of_get_child_by_name(dev->of_node, "usb3-phy");
>  	if (usb_np) {

Looks good otherwise:

Reviewed-by: Johan Hovold <johan+linaro@kernel.org>

Johan