mbox series

[v13,0/3] Add QCOM SNPS PHY overriding params support

Message ID 1662480933-12326-1-git-send-email-quic_kriskura@quicinc.com
Headers show
Series Add QCOM SNPS PHY overriding params support | expand

Message

Krishna Kurapati Sept. 6, 2022, 4:15 p.m. UTC
Added support for overriding tuning parameters in QCOM SNPS PHY
from device tree. This parameter tuning is required to tune the
hs signal on dp/dm lines for electrical compliance to be successful.

Changes in v13:
Fixed NULL pointer check in driver code.

Changes in v12:
Fixed nitpicks in driver code.

Changes in v11:
Made changes to logs added in phy driver.
Fixed nitpicks in code.

Changes in v10:
Fixed patch headers.

changes in v9:
Fixed nitpick in driver code.

changes in v8:
Fixed nitpick in driver code.

changes in v7:
Fixed nitpick in driver code and dtsi file.

changes in v6:
Fixed errors in dt-bindings.
Fixed nitpick in driver code.

changes in v5:
Fixed nitpicks in code.
Added minimum and maximum for each parameter added in dt-bindings.
Added proper suffixes to each parameter as per dtschema.

changes in v4:
Fixed nitpicks in code.
Initial compliance test results showed overshoot in the middle of eye
diagram. The current dt values were put in place to correct it and fix
overshoot issue.

changes in v3:
Added support for phy tuning parameters to be represented in bps and
corresponding register values to be written are obtained by traversing
through data map declared in the driver.

changes in v2:
Reading the individual fields in each overriding register from
device tree.

Krishna Kurapati (2):
  phy: qcom-snps: Add support for overriding phy tuning parameters
  arm64: dts: qcom: sc7280: Update SNPS Phy params for SC7280 IDP device

Sandeep Maheswaram (1):
  dt-bindings: phy: qcom,usb-snps-femto-v2: Add phy override params
    bindings

 .../bindings/phy/qcom,usb-snps-femto-v2.yaml       |  88 +++++++
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi           |   6 +
 drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c      | 255 ++++++++++++++++++++-
 3 files changed, 347 insertions(+), 2 deletions(-)

Comments

Bjorn Andersson Sept. 8, 2022, 11:17 p.m. UTC | #1
On Tue, Sep 06, 2022 at 09:45:32PM +0530, Krishna Kurapati wrote:
> Add support for overriding electrical signal tuning parameters for
> SNPS HS Phy.
> 

Reviewed-by: Bjorn Andersson <andersson@kernel.org>

Regards,
Bjorn

> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
> ---
>  drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c | 255 +++++++++++++++++++++++++-
>  1 file changed, 253 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
> index 5d20378..2502294 100644
> --- a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
> +++ b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
> @@ -52,6 +52,12 @@
>  #define USB2_SUSPEND_N				BIT(2)
>  #define USB2_SUSPEND_N_SEL			BIT(3)
>  
> +#define USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X0		(0x6c)
> +#define USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X1		(0x70)
> +#define USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X2		(0x74)
> +#define USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X3		(0x78)
> +#define PARAM_OVRD_MASK				0xFF
> +
>  #define USB2_PHY_USB_PHY_CFG0			(0x94)
>  #define UTMI_PHY_DATAPATH_CTRL_OVERRIDE_EN	BIT(0)
>  #define UTMI_PHY_CMN_CTRL_OVERRIDE_EN		BIT(1)
> @@ -60,12 +66,47 @@
>  #define REFCLK_SEL_MASK				GENMASK(1, 0)
>  #define REFCLK_SEL_DEFAULT			(0x2 << 0)
>  
> +#define HS_DISCONNECT_MASK			GENMASK(2, 0)
> +#define SQUELCH_DETECTOR_MASK			GENMASK(7, 5)
> +
> +#define HS_AMPLITUDE_MASK			GENMASK(3, 0)
> +#define PREEMPHASIS_DURATION_MASK		BIT(5)
> +#define PREEMPHASIS_AMPLITUDE_MASK		GENMASK(7, 6)
> +
> +#define HS_RISE_FALL_MASK			GENMASK(1, 0)
> +#define HS_CROSSOVER_VOLTAGE_MASK		GENMASK(3, 2)
> +#define HS_OUTPUT_IMPEDANCE_MASK		GENMASK(5, 4)
> +
> +#define LS_FS_OUTPUT_IMPEDANCE_MASK		GENMASK(3, 0)
> +
>  static const char * const qcom_snps_hsphy_vreg_names[] = {
>  	"vdda-pll", "vdda33", "vdda18",
>  };
>  
>  #define SNPS_HS_NUM_VREGS		ARRAY_SIZE(qcom_snps_hsphy_vreg_names)
>  
> +struct override_param {
> +	s32	value;
> +	u8	reg_val;
> +};
> +
> +struct override_param_map {
> +	const char *prop_name;
> +	const struct override_param *param_table;
> +	u8 table_size;
> +	u8 reg_offset;
> +	u8 param_mask;
> +};
> +
> +struct phy_override_seq {
> +	bool	need_update;
> +	u8	offset;
> +	u8	value;
> +	u8	mask;
> +};
> +
> +#define NUM_HSPHY_TUNING_PARAMS	(9)
> +
>  /**
>   * struct qcom_snps_hsphy - snps hs phy attributes
>   *
> @@ -91,6 +132,7 @@ struct qcom_snps_hsphy {
>  
>  	bool phy_initialized;
>  	enum phy_mode mode;
> +	struct phy_override_seq update_seq_cfg[NUM_HSPHY_TUNING_PARAMS];
>  };
>  
>  static inline void qcom_snps_hsphy_write_mask(void __iomem *base, u32 offset,
> @@ -173,10 +215,158 @@ static int qcom_snps_hsphy_set_mode(struct phy *phy, enum phy_mode mode,
>  	return 0;
>  }
>  
> +static const struct override_param hs_disconnect_sc7280[] = {
> +	{ -272, 0 },
> +	{ 0, 1 },
> +	{ 317, 2 },
> +	{ 630, 3 },
> +	{ 973, 4 },
> +	{ 1332, 5 },
> +	{ 1743, 6 },
> +	{ 2156, 7 },
> +};
> +
> +static const struct override_param squelch_det_threshold_sc7280[] = {
> +	{ -2090, 7 },
> +	{ -1560, 6 },
> +	{ -1030, 5 },
> +	{ -530, 4 },
> +	{ 0, 3 },
> +	{ 530, 2 },
> +	{ 1060, 1 },
> +	{ 1590, 0 },
> +};
> +
> +static const struct override_param hs_amplitude_sc7280[] = {
> +	{ -660, 0 },
> +	{ -440, 1 },
> +	{ -220, 2 },
> +	{ 0, 3 },
> +	{ 230, 4 },
> +	{ 440, 5 },
> +	{ 650, 6 },
> +	{ 890, 7 },
> +	{ 1110, 8 },
> +	{ 1330, 9 },
> +	{ 1560, 10 },
> +	{ 1780, 11 },
> +	{ 2000, 12 },
> +	{ 2220, 13 },
> +	{ 2430, 14 },
> +	{ 2670, 15 },
> +};
> +
> +static const struct override_param preemphasis_duration_sc7280[] = {
> +	{ 10000, 1 },
> +	{ 20000, 0 },
> +};
> +
> +static const struct override_param preemphasis_amplitude_sc7280[] = {
> +	{ 10000, 1 },
> +	{ 20000, 2 },
> +	{ 30000, 3 },
> +	{ 40000, 0 },
> +};
> +
> +static const struct override_param hs_rise_fall_time_sc7280[] = {
> +	{ -4100, 3 },
> +	{ 0, 2 },
> +	{ 2810, 1 },
> +	{ 5430, 0 },
> +};
> +
> +static const struct override_param hs_crossover_voltage_sc7280[] = {
> +	{ -31000, 1 },
> +	{ 0, 3 },
> +	{ 28000, 2 },
> +};
> +
> +static const struct override_param hs_output_impedance_sc7280[] = {
> +	{ -2300000, 3 },
> +	{ 0, 2 },
> +	{ 2600000, 1 },
> +	{ 6100000, 0 },
> +};
> +
> +static const struct override_param ls_fs_output_impedance_sc7280[] = {
> +	{ -1053, 15 },
> +	{ -557, 7 },
> +	{ 0, 3 },
> +	{ 612, 1 },
> +	{ 1310, 0 },
> +};
> +
> +static const struct override_param_map sc7280_snps_7nm_phy[] = {
> +	{
> +		"qcom,hs-disconnect-bp",
> +		hs_disconnect_sc7280,
> +		ARRAY_SIZE(hs_disconnect_sc7280),
> +		USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X0,
> +		HS_DISCONNECT_MASK
> +	},
> +	{
> +		"qcom,squelch-detector-bp",
> +		squelch_det_threshold_sc7280,
> +		ARRAY_SIZE(squelch_det_threshold_sc7280),
> +		USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X0,
> +		SQUELCH_DETECTOR_MASK
> +	},
> +	{
> +		"qcom,hs-amplitude-bp",
> +		hs_amplitude_sc7280,
> +		ARRAY_SIZE(hs_amplitude_sc7280),
> +		USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X1,
> +		HS_AMPLITUDE_MASK
> +	},
> +	{
> +		"qcom,pre-emphasis-duration-bp",
> +		preemphasis_duration_sc7280,
> +		ARRAY_SIZE(preemphasis_duration_sc7280),
> +		USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X1,
> +		PREEMPHASIS_DURATION_MASK,
> +	},
> +	{
> +		"qcom,pre-emphasis-amplitude-bp",
> +		preemphasis_amplitude_sc7280,
> +		ARRAY_SIZE(preemphasis_amplitude_sc7280),
> +		USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X1,
> +		PREEMPHASIS_AMPLITUDE_MASK,
> +	},
> +	{
> +		"qcom,hs-rise-fall-time-bp",
> +		hs_rise_fall_time_sc7280,
> +		ARRAY_SIZE(hs_rise_fall_time_sc7280),
> +		USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X2,
> +		HS_RISE_FALL_MASK
> +	},
> +	{
> +		"qcom,hs-crossover-voltage-microvolt",
> +		hs_crossover_voltage_sc7280,
> +		ARRAY_SIZE(hs_crossover_voltage_sc7280),
> +		USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X2,
> +		HS_CROSSOVER_VOLTAGE_MASK
> +	},
> +	{
> +		"qcom,hs-output-impedance-micro-ohms",
> +		hs_output_impedance_sc7280,
> +		ARRAY_SIZE(hs_output_impedance_sc7280),
> +		USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X2,
> +		HS_OUTPUT_IMPEDANCE_MASK,
> +	},
> +	{
> +		"qcom,ls-fs-output-impedance-bp",
> +		ls_fs_output_impedance_sc7280,
> +		ARRAY_SIZE(ls_fs_output_impedance_sc7280),
> +		USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X3,
> +		LS_FS_OUTPUT_IMPEDANCE_MASK,
> +	},
> +	{},
> +};
> +
>  static int qcom_snps_hsphy_init(struct phy *phy)
>  {
>  	struct qcom_snps_hsphy *hsphy = phy_get_drvdata(phy);
> -	int ret;
> +	int ret, i;
>  
>  	dev_vdbg(&phy->dev, "%s(): Initializing SNPS HS phy\n", __func__);
>  
> @@ -223,6 +413,14 @@ static int qcom_snps_hsphy_init(struct phy *phy)
>  	qcom_snps_hsphy_write_mask(hsphy->base, USB2_PHY_USB_PHY_HS_PHY_CTRL1,
>  					VBUSVLDEXT0, VBUSVLDEXT0);
>  
> +	for (i = 0; i < ARRAY_SIZE(hsphy->update_seq_cfg); i++) {
> +		if (hsphy->update_seq_cfg[i].need_update)
> +			qcom_snps_hsphy_write_mask(hsphy->base,
> +					hsphy->update_seq_cfg[i].offset,
> +					hsphy->update_seq_cfg[i].mask,
> +					hsphy->update_seq_cfg[i].value);
> +	}
> +
>  	qcom_snps_hsphy_write_mask(hsphy->base,
>  					USB2_PHY_USB_PHY_HS_PHY_CTRL_COMMON2,
>  					VREGBYPASS, VREGBYPASS);
> @@ -280,7 +478,10 @@ static const struct phy_ops qcom_snps_hsphy_gen_ops = {
>  static const struct of_device_id qcom_snps_hsphy_of_match_table[] = {
>  	{ .compatible	= "qcom,sm8150-usb-hs-phy", },
>  	{ .compatible	= "qcom,usb-snps-hs-5nm-phy", },
> -	{ .compatible	= "qcom,usb-snps-hs-7nm-phy", },
> +	{
> +		.compatible	= "qcom,usb-snps-hs-7nm-phy",
> +		.data		= &sc7280_snps_7nm_phy,
> +	},
>  	{ .compatible	= "qcom,usb-snps-femto-v2-phy",	},
>  	{ }
>  };
> @@ -291,6 +492,55 @@ static const struct dev_pm_ops qcom_snps_hsphy_pm_ops = {
>  			   qcom_snps_hsphy_runtime_resume, NULL)
>  };
>  
> +static void qcom_snps_hsphy_override_param_update_val(
> +			const struct override_param_map map,
> +			s32 dt_val, struct phy_override_seq *seq_entry)
> +{
> +	int i;
> +
> +	/*
> +	 * Param table for each param is in increasing order
> +	 * of dt values. We need to iterate over the list to
> +	 * select the entry that matches the dt value and pick
> +	 * up the corresponding register value.
> +	 */
> +	for (i = 0; i < map.table_size - 1; i++) {
> +		if (map.param_table[i].value == dt_val)
> +			break;
> +	}
> +
> +	seq_entry->need_update = true;
> +	seq_entry->offset = map.reg_offset;
> +	seq_entry->mask = map.param_mask;
> +	seq_entry->value = map.param_table[i].reg_val << __ffs(map.param_mask);
> +}
> +
> +static void qcom_snps_hsphy_read_override_param_seq(struct device *dev)
> +{
> +	struct device_node *node = dev->of_node;
> +	s32 val;
> +	int ret, i;
> +	struct qcom_snps_hsphy *hsphy;
> +	const struct override_param_map *cfg = of_device_get_match_data(dev);
> +
> +	if (!cfg)
> +		return;
> +
> +	hsphy = dev_get_drvdata(dev);
> +
> +	for (i = 0; cfg[i].prop_name != NULL; i++) {
> +		ret = of_property_read_s32(node, cfg[i].prop_name, &val);
> +		if (ret)
> +			continue;
> +
> +		qcom_snps_hsphy_override_param_update_val(cfg[i], val,
> +					&hsphy->update_seq_cfg[i]);
> +		dev_dbg(&hsphy->phy->dev, "Read param: %s dt_val: %d reg_val: 0x%x\n",
> +			cfg[i].prop_name, val, hsphy->update_seq_cfg[i].value);
> +
> +	}
> +}
> +
>  static int qcom_snps_hsphy_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -352,6 +602,7 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev)
>  
>  	dev_set_drvdata(dev, hsphy);
>  	phy_set_drvdata(generic_phy, hsphy);
> +	qcom_snps_hsphy_read_override_param_seq(dev);
>  
>  	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>  	if (!IS_ERR(phy_provider))
> -- 
> 2.7.4
>
Bjorn Andersson Oct. 18, 2022, 3:14 a.m. UTC | #2
On Tue, 6 Sep 2022 21:45:30 +0530, Krishna Kurapati wrote:
> Added support for overriding tuning parameters in QCOM SNPS PHY
> from device tree. This parameter tuning is required to tune the
> hs signal on dp/dm lines for electrical compliance to be successful.
> 
> Changes in v13:
> Fixed NULL pointer check in driver code.
> 
> [...]

Applied, thanks!

[3/3] arm64: dts: qcom: sc7280: Update SNPS Phy params for SC7280 IDP device
      commit: 3b08e3fdf056cf30ecb1413d2bcb1353a333024b

Best regards,