Re: [PATCH v4 7/7] phy: qcom-qusb2: Add QUSB2 PHYs support for sdm845

[Manu Gautam]

Hi,


On 3/30/2018 2:08 AM, Doug Anderson wrote:
> Hi,
>
> On Thu, Mar 29, 2018 at 4:04 AM, Manu Gautam  wrote:
>> @@ -241,6 +252,18 @@ struct qusb2_phy_cfg {
>>   * @tcsr: TCSR syscon register map
>>   * @cell: nvmem cell containing phy tuning value
>>   *
>> + * @override_imp_res_offset: PHY should use different rescode offset
>> + * @imp_res_offset_value: rescode offset to be updated in IMP_CTRL1 register
>> + *
>> + * @override_hstx_trim: PHY should use different HSTX o/p current value
>> + * @hstx_trim_value: HSTX_TRIM value to be updated in TUNE1 register
>> + *
>> + * @override_preemphasis: PHY should use different pre-amphasis amplitude
>> + * @preemphasis_level: Amplitude Pre-Emphasis to be updated in TUNE1 
>> register
>> + *
>> + * @override_preemphasis_width: PHY should use different pre-emphasis 
>> duration
>> + * @preemphasis_width: half/full-width Pre-Emphasis updated via TUNE1
>> + *
> nit: spacing here doesn't match spacing in the structure.  AKA: you've
> smashed together all 8 properties in the structure but not in the
> description.
Will fix it.
>
>
>>   * @cfg: phy config data
>>   * @has_se_clk_scheme: indicate if PHY has single-ended ref clock scheme
>>   * @phy_initialized: indicate if PHY has been initialized
>> @@ -259,12 +282,35 @@ struct qusb2_phy {
>> struct regmap *tcsr;
>> struct nvmem_cell *cell;
>>
>> +   bool override_imp_res_offset;
>> +   u8 imp_res_offset_value;
>> +   bool override_hstx_trim;
>> +   u8 hstx_trim_value;
>> +   bool override_preemphasis;
>> +   u8 preemphasis_level;
>> +   bool override_preemphasis_width;
>> +   u8 preemphasis_width;
>> +
>> const struct qusb2_phy_cfg *cfg;
>> bool has_se_clk_scheme;
>> bool phy_initialized;
>> enum phy_mode mode;
>>  };
>>
>> +static inline void qusb2_write_mask(void __iomem *base, u32 offset,
>> +   u32 val, u32 mask)
>> +{
>> +   u32 reg;
>> +
>> +   reg = readl(base + offset);
>> +   reg &= ~mask;
>> +   reg |= val;
> "reg |= (val & mask)" instead of just "reg |= val"
>
> You don't do any bounds checking of the device tree entries and this
> will at least make sure that a bad value for a field won't screw up
> other fields (and so, presumably, it will be easier to find the bug).
>
It makes sense. Will change accordingly.

>
>> +   writel(reg, base + offset);
>> +
>> +   /* Ensure above write is completed */
>> +   readl(base + offset);
> You're using readl() and writel() which have barriers.  Why do you
> need this extra readl()?

This requirement comes from AHB2PHY wrapper designers and HPG.
Also existing qusb2_setbits/clrbits() wrapper already have similar handling.

>
>
>> +}
>> +
>>  static inline void qusb2_setbits(void __iomem *base, u32 offset, u32 val)
>>  {
>> u32 reg;
>> @@ -305,6 +351,42 @@ void qcom_qusb2_phy_configure(void __iomem *base,
>>  }
>>
>>  /*
>> + * Update board specific PHY tuning override values if specified from
>> + * device tree.
>> + *
> nit: remove extra comment line with just a "*" on it.
Sure.
>
>
>> + */
>> +static void qusb2_phy_override_phy_params(struct qusb2_phy *qphy)
>> +{
>> +   const struct qusb2_phy_cfg *cfg = qphy->cfg;
>> +
>> +   if (qphy->override_imp_res_offset)
>> +   qusb2_write_mask(qphy->base, QUSB2PHY_IMP_CTRL1,
>> +qphy->imp_res_offset_value << 
>> IMP_RES_OFFSET_SHIFT,
>> +IMP_RES_OFFSET_MASK);
>> +
>> +   if (qphy->override_hstx_trim)
>> +   qusb2_write_mask(qphy->base, cfg->regs[QUSB2PHY_PORT_TUNE1],
>> +qphy->hstx_trim_value << HSTX_TRIM_SHIFT,
>> +HSTX_TRIM_MASK);
>> +
>> +   if (qphy->override_preemphasis)
>> +   qusb2_write_mask(qphy->base, cfg->regs[QUSB2PHY_PORT_TUNE1],
>> +   qphy->preemphasis_level << 
>> PREEMPHASIS_EN_SHIFT,
>> +   PREEMPHASIS_EN_MASK);
>> +
>> +   if (qphy->override_preemphasis_width) {
>> +   if (qphy->preemphasis_width)
>> +   qusb2_setbits(qphy->base,
>> + cfg->regs[QUSB2PHY_PORT_TUNE1],
>> + PREEMPH_HALF_WIDTH);
>> +   else
>> +   qusb2_clrbits(qphy->base,
>> + cfg->regs[QUSB2PHY_PORT_TUNE1],
>> + PREEMPH_HALF_WIDTH);
>> +   }
>> +}
>> +
>> +/*
>>   * Fetches HS Tx tuning value from nvmem and sets the
>>   * QUSB2PHY_PORT_TUNE1/2 register.
>>   * For error case, skip setting the value and use the default value.
>> @@ -525,6 +607,9 @@ static int qusb2_phy_init(struct phy *phy)
>> qcom_qusb2_phy_configure(qphy->base, cfg->regs, cfg->tbl,
>>  cfg->tbl_num);
>>
>> +   /* Override 

Re: [PATCH v4 7/7] phy: qcom-qusb2: Add QUSB2 PHYs support for sdm845

[Manu Gautam]

Hi,


On 3/30/2018 2:08 AM, Doug Anderson wrote:
> Hi,
>
> On Thu, Mar 29, 2018 at 4:04 AM, Manu Gautam  wrote:
>> @@ -241,6 +252,18 @@ struct qusb2_phy_cfg {
>>   * @tcsr: TCSR syscon register map
>>   * @cell: nvmem cell containing phy tuning value
>>   *
>> + * @override_imp_res_offset: PHY should use different rescode offset
>> + * @imp_res_offset_value: rescode offset to be updated in IMP_CTRL1 register
>> + *
>> + * @override_hstx_trim: PHY should use different HSTX o/p current value
>> + * @hstx_trim_value: HSTX_TRIM value to be updated in TUNE1 register
>> + *
>> + * @override_preemphasis: PHY should use different pre-amphasis amplitude
>> + * @preemphasis_level: Amplitude Pre-Emphasis to be updated in TUNE1 
>> register
>> + *
>> + * @override_preemphasis_width: PHY should use different pre-emphasis 
>> duration
>> + * @preemphasis_width: half/full-width Pre-Emphasis updated via TUNE1
>> + *
> nit: spacing here doesn't match spacing in the structure.  AKA: you've
> smashed together all 8 properties in the structure but not in the
> description.
Will fix it.
>
>
>>   * @cfg: phy config data
>>   * @has_se_clk_scheme: indicate if PHY has single-ended ref clock scheme
>>   * @phy_initialized: indicate if PHY has been initialized
>> @@ -259,12 +282,35 @@ struct qusb2_phy {
>> struct regmap *tcsr;
>> struct nvmem_cell *cell;
>>
>> +   bool override_imp_res_offset;
>> +   u8 imp_res_offset_value;
>> +   bool override_hstx_trim;
>> +   u8 hstx_trim_value;
>> +   bool override_preemphasis;
>> +   u8 preemphasis_level;
>> +   bool override_preemphasis_width;
>> +   u8 preemphasis_width;
>> +
>> const struct qusb2_phy_cfg *cfg;
>> bool has_se_clk_scheme;
>> bool phy_initialized;
>> enum phy_mode mode;
>>  };
>>
>> +static inline void qusb2_write_mask(void __iomem *base, u32 offset,
>> +   u32 val, u32 mask)
>> +{
>> +   u32 reg;
>> +
>> +   reg = readl(base + offset);
>> +   reg &= ~mask;
>> +   reg |= val;
> "reg |= (val & mask)" instead of just "reg |= val"
>
> You don't do any bounds checking of the device tree entries and this
> will at least make sure that a bad value for a field won't screw up
> other fields (and so, presumably, it will be easier to find the bug).
>
It makes sense. Will change accordingly.

>
>> +   writel(reg, base + offset);
>> +
>> +   /* Ensure above write is completed */
>> +   readl(base + offset);
> You're using readl() and writel() which have barriers.  Why do you
> need this extra readl()?

This requirement comes from AHB2PHY wrapper designers and HPG.
Also existing qusb2_setbits/clrbits() wrapper already have similar handling.

>
>
>> +}
>> +
>>  static inline void qusb2_setbits(void __iomem *base, u32 offset, u32 val)
>>  {
>> u32 reg;
>> @@ -305,6 +351,42 @@ void qcom_qusb2_phy_configure(void __iomem *base,
>>  }
>>
>>  /*
>> + * Update board specific PHY tuning override values if specified from
>> + * device tree.
>> + *
> nit: remove extra comment line with just a "*" on it.
Sure.
>
>
>> + */
>> +static void qusb2_phy_override_phy_params(struct qusb2_phy *qphy)
>> +{
>> +   const struct qusb2_phy_cfg *cfg = qphy->cfg;
>> +
>> +   if (qphy->override_imp_res_offset)
>> +   qusb2_write_mask(qphy->base, QUSB2PHY_IMP_CTRL1,
>> +qphy->imp_res_offset_value << 
>> IMP_RES_OFFSET_SHIFT,
>> +IMP_RES_OFFSET_MASK);
>> +
>> +   if (qphy->override_hstx_trim)
>> +   qusb2_write_mask(qphy->base, cfg->regs[QUSB2PHY_PORT_TUNE1],
>> +qphy->hstx_trim_value << HSTX_TRIM_SHIFT,
>> +HSTX_TRIM_MASK);
>> +
>> +   if (qphy->override_preemphasis)
>> +   qusb2_write_mask(qphy->base, cfg->regs[QUSB2PHY_PORT_TUNE1],
>> +   qphy->preemphasis_level << 
>> PREEMPHASIS_EN_SHIFT,
>> +   PREEMPHASIS_EN_MASK);
>> +
>> +   if (qphy->override_preemphasis_width) {
>> +   if (qphy->preemphasis_width)
>> +   qusb2_setbits(qphy->base,
>> + cfg->regs[QUSB2PHY_PORT_TUNE1],
>> + PREEMPH_HALF_WIDTH);
>> +   else
>> +   qusb2_clrbits(qphy->base,
>> + cfg->regs[QUSB2PHY_PORT_TUNE1],
>> + PREEMPH_HALF_WIDTH);
>> +   }
>> +}
>> +
>> +/*
>>   * Fetches HS Tx tuning value from nvmem and sets the
>>   * QUSB2PHY_PORT_TUNE1/2 register.
>>   * For error case, skip setting the value and use the default value.
>> @@ -525,6 +607,9 @@ static int qusb2_phy_init(struct phy *phy)
>> qcom_qusb2_phy_configure(qphy->base, cfg->regs, cfg->tbl,
>>  cfg->tbl_num);
>>
>> +   /* Override board specific PHY tuning 

Re: [PATCH v4 7/7] phy: qcom-qusb2: Add QUSB2 PHYs support for sdm845

[Doug Anderson]

Hi,

On Thu, Mar 29, 2018 at 4:04 AM, Manu Gautam  wrote:
> There are two QUSB2 PHYs present on sdm845. In order
> to improve eye diagram for both the PHYs some parameters
> need to be changed. Provide device tree properties to
> override these from board specific device tree files.
>
> Signed-off-by: Manu Gautam 
> ---
>  drivers/phy/qualcomm/phy-qcom-qusb2.c | 112 
> +-
>  1 file changed, 111 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qusb2.c 
> b/drivers/phy/qualcomm/phy-qcom-qusb2.c
> index 40fdef8..adcc3f8 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qusb2.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qusb2.c
> @@ -60,6 +60,17 @@
>  #define CORE_RESET BIT(5)
>  #define CORE_RESET_MUX BIT(6)
>
> +/* QUSB2PHY_IMP_CTRL1 register bits */
> +#define IMP_RES_OFFSET_MASKGENMASK(5, 0)
> +#define IMP_RES_OFFSET_SHIFT   0x0
> +
> +/* QUSB2PHY_PORT_TUNE1 register bits */
> +#define HSTX_TRIM_MASK GENMASK(7, 4)
> +#define HSTX_TRIM_SHIFT0x4
> +#define PREEMPH_HALF_WIDTH BIT(2)
> +#define PREEMPHASIS_EN_MASKGENMASK(1, 0)
> +#define PREEMPHASIS_EN_SHIFT   0x0
> +
>  #define QUSB2PHY_PLL_ANALOG_CONTROLS_TWO   0x04
>  #define QUSB2PHY_PLL_CLOCK_INVERTERS   0x18c
>  #define QUSB2PHY_PLL_CMODE 0x2c
> @@ -241,6 +252,18 @@ struct qusb2_phy_cfg {
>   * @tcsr: TCSR syscon register map
>   * @cell: nvmem cell containing phy tuning value
>   *
> + * @override_imp_res_offset: PHY should use different rescode offset
> + * @imp_res_offset_value: rescode offset to be updated in IMP_CTRL1 register
> + *
> + * @override_hstx_trim: PHY should use different HSTX o/p current value
> + * @hstx_trim_value: HSTX_TRIM value to be updated in TUNE1 register
> + *
> + * @override_preemphasis: PHY should use different pre-amphasis amplitude
> + * @preemphasis_level: Amplitude Pre-Emphasis to be updated in TUNE1 register
> + *
> + * @override_preemphasis_width: PHY should use different pre-emphasis 
> duration
> + * @preemphasis_width: half/full-width Pre-Emphasis updated via TUNE1
> + *

nit: spacing here doesn't match spacing in the structure.  AKA: you've
smashed together all 8 properties in the structure but not in the
description.


>   * @cfg: phy config data
>   * @has_se_clk_scheme: indicate if PHY has single-ended ref clock scheme
>   * @phy_initialized: indicate if PHY has been initialized
> @@ -259,12 +282,35 @@ struct qusb2_phy {
> struct regmap *tcsr;
> struct nvmem_cell *cell;
>
> +   bool override_imp_res_offset;
> +   u8 imp_res_offset_value;
> +   bool override_hstx_trim;
> +   u8 hstx_trim_value;
> +   bool override_preemphasis;
> +   u8 preemphasis_level;
> +   bool override_preemphasis_width;
> +   u8 preemphasis_width;
> +
> const struct qusb2_phy_cfg *cfg;
> bool has_se_clk_scheme;
> bool phy_initialized;
> enum phy_mode mode;
>  };
>
> +static inline void qusb2_write_mask(void __iomem *base, u32 offset,
> +   u32 val, u32 mask)
> +{
> +   u32 reg;
> +
> +   reg = readl(base + offset);
> +   reg &= ~mask;
> +   reg |= val;

"reg |= (val & mask)" instead of just "reg |= val"

You don't do any bounds checking of the device tree entries and this
will at least make sure that a bad value for a field won't screw up
other fields (and so, presumably, it will be easier to find the bug).


> +   writel(reg, base + offset);
> +
> +   /* Ensure above write is completed */
> +   readl(base + offset);

You're using readl() and writel() which have barriers.  Why do you
need this extra readl()?


> +}
> +
>  static inline void qusb2_setbits(void __iomem *base, u32 offset, u32 val)
>  {
> u32 reg;
> @@ -305,6 +351,42 @@ void qcom_qusb2_phy_configure(void __iomem *base,
>  }
>
>  /*
> + * Update board specific PHY tuning override values if specified from
> + * device tree.
> + *

nit: remove extra comment line with just a "*" on it.


> + */
> +static void qusb2_phy_override_phy_params(struct qusb2_phy *qphy)
> +{
> +   const struct qusb2_phy_cfg *cfg = qphy->cfg;
> +
> +   if (qphy->override_imp_res_offset)
> +   qusb2_write_mask(qphy->base, QUSB2PHY_IMP_CTRL1,
> +qphy->imp_res_offset_value << 
> IMP_RES_OFFSET_SHIFT,
> +IMP_RES_OFFSET_MASK);
> +
> +   if (qphy->override_hstx_trim)
> +   qusb2_write_mask(qphy->base, cfg->regs[QUSB2PHY_PORT_TUNE1],
> +qphy->hstx_trim_value << HSTX_TRIM_SHIFT,
> +HSTX_TRIM_MASK);
> +
> +   if (qphy->override_preemphasis)
> +   qusb2_write_mask(qphy->base, 

Re: [PATCH v4 7/7] phy: qcom-qusb2: Add QUSB2 PHYs support for sdm845

[Doug Anderson]

Hi,

On Thu, Mar 29, 2018 at 4:04 AM, Manu Gautam  wrote:
> There are two QUSB2 PHYs present on sdm845. In order
> to improve eye diagram for both the PHYs some parameters
> need to be changed. Provide device tree properties to
> override these from board specific device tree files.
>
> Signed-off-by: Manu Gautam 
> ---
>  drivers/phy/qualcomm/phy-qcom-qusb2.c | 112 
> +-
>  1 file changed, 111 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qusb2.c 
> b/drivers/phy/qualcomm/phy-qcom-qusb2.c
> index 40fdef8..adcc3f8 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qusb2.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qusb2.c
> @@ -60,6 +60,17 @@
>  #define CORE_RESET BIT(5)
>  #define CORE_RESET_MUX BIT(6)
>
> +/* QUSB2PHY_IMP_CTRL1 register bits */
> +#define IMP_RES_OFFSET_MASKGENMASK(5, 0)
> +#define IMP_RES_OFFSET_SHIFT   0x0
> +
> +/* QUSB2PHY_PORT_TUNE1 register bits */
> +#define HSTX_TRIM_MASK GENMASK(7, 4)
> +#define HSTX_TRIM_SHIFT0x4
> +#define PREEMPH_HALF_WIDTH BIT(2)
> +#define PREEMPHASIS_EN_MASKGENMASK(1, 0)
> +#define PREEMPHASIS_EN_SHIFT   0x0
> +
>  #define QUSB2PHY_PLL_ANALOG_CONTROLS_TWO   0x04
>  #define QUSB2PHY_PLL_CLOCK_INVERTERS   0x18c
>  #define QUSB2PHY_PLL_CMODE 0x2c
> @@ -241,6 +252,18 @@ struct qusb2_phy_cfg {
>   * @tcsr: TCSR syscon register map
>   * @cell: nvmem cell containing phy tuning value
>   *
> + * @override_imp_res_offset: PHY should use different rescode offset
> + * @imp_res_offset_value: rescode offset to be updated in IMP_CTRL1 register
> + *
> + * @override_hstx_trim: PHY should use different HSTX o/p current value
> + * @hstx_trim_value: HSTX_TRIM value to be updated in TUNE1 register
> + *
> + * @override_preemphasis: PHY should use different pre-amphasis amplitude
> + * @preemphasis_level: Amplitude Pre-Emphasis to be updated in TUNE1 register
> + *
> + * @override_preemphasis_width: PHY should use different pre-emphasis 
> duration
> + * @preemphasis_width: half/full-width Pre-Emphasis updated via TUNE1
> + *

nit: spacing here doesn't match spacing in the structure.  AKA: you've
smashed together all 8 properties in the structure but not in the
description.


>   * @cfg: phy config data
>   * @has_se_clk_scheme: indicate if PHY has single-ended ref clock scheme
>   * @phy_initialized: indicate if PHY has been initialized
> @@ -259,12 +282,35 @@ struct qusb2_phy {
> struct regmap *tcsr;
> struct nvmem_cell *cell;
>
> +   bool override_imp_res_offset;
> +   u8 imp_res_offset_value;
> +   bool override_hstx_trim;
> +   u8 hstx_trim_value;
> +   bool override_preemphasis;
> +   u8 preemphasis_level;
> +   bool override_preemphasis_width;
> +   u8 preemphasis_width;
> +
> const struct qusb2_phy_cfg *cfg;
> bool has_se_clk_scheme;
> bool phy_initialized;
> enum phy_mode mode;
>  };
>
> +static inline void qusb2_write_mask(void __iomem *base, u32 offset,
> +   u32 val, u32 mask)
> +{
> +   u32 reg;
> +
> +   reg = readl(base + offset);
> +   reg &= ~mask;
> +   reg |= val;

"reg |= (val & mask)" instead of just "reg |= val"

You don't do any bounds checking of the device tree entries and this
will at least make sure that a bad value for a field won't screw up
other fields (and so, presumably, it will be easier to find the bug).


> +   writel(reg, base + offset);
> +
> +   /* Ensure above write is completed */
> +   readl(base + offset);

You're using readl() and writel() which have barriers.  Why do you
need this extra readl()?


> +}
> +
>  static inline void qusb2_setbits(void __iomem *base, u32 offset, u32 val)
>  {
> u32 reg;
> @@ -305,6 +351,42 @@ void qcom_qusb2_phy_configure(void __iomem *base,
>  }
>
>  /*
> + * Update board specific PHY tuning override values if specified from
> + * device tree.
> + *

nit: remove extra comment line with just a "*" on it.


> + */
> +static void qusb2_phy_override_phy_params(struct qusb2_phy *qphy)
> +{
> +   const struct qusb2_phy_cfg *cfg = qphy->cfg;
> +
> +   if (qphy->override_imp_res_offset)
> +   qusb2_write_mask(qphy->base, QUSB2PHY_IMP_CTRL1,
> +qphy->imp_res_offset_value << 
> IMP_RES_OFFSET_SHIFT,
> +IMP_RES_OFFSET_MASK);
> +
> +   if (qphy->override_hstx_trim)
> +   qusb2_write_mask(qphy->base, cfg->regs[QUSB2PHY_PORT_TUNE1],
> +qphy->hstx_trim_value << HSTX_TRIM_SHIFT,
> +HSTX_TRIM_MASK);
> +
> +   if (qphy->override_preemphasis)
> +   qusb2_write_mask(qphy->base, cfg->regs[QUSB2PHY_PORT_TUNE1],
> + 

[PATCH v4 7/7] phy: qcom-qusb2: Add QUSB2 PHYs support for sdm845

[Manu Gautam]

There are two QUSB2 PHYs present on sdm845. In order
to improve eye diagram for both the PHYs some parameters
need to be changed. Provide device tree properties to
override these from board specific device tree files.

Signed-off-by: Manu Gautam 
---
 drivers/phy/qualcomm/phy-qcom-qusb2.c | 112 +-
 1 file changed, 111 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qusb2.c 
b/drivers/phy/qualcomm/phy-qcom-qusb2.c
index 40fdef8..adcc3f8 100644
--- a/drivers/phy/qualcomm/phy-qcom-qusb2.c
+++ b/drivers/phy/qualcomm/phy-qcom-qusb2.c
@@ -60,6 +60,17 @@
 #define CORE_RESET BIT(5)
 #define CORE_RESET_MUX BIT(6)
 
+/* QUSB2PHY_IMP_CTRL1 register bits */
+#define IMP_RES_OFFSET_MASKGENMASK(5, 0)
+#define IMP_RES_OFFSET_SHIFT   0x0
+
+/* QUSB2PHY_PORT_TUNE1 register bits */
+#define HSTX_TRIM_MASK GENMASK(7, 4)
+#define HSTX_TRIM_SHIFT0x4
+#define PREEMPH_HALF_WIDTH BIT(2)
+#define PREEMPHASIS_EN_MASKGENMASK(1, 0)
+#define PREEMPHASIS_EN_SHIFT   0x0
+
 #define QUSB2PHY_PLL_ANALOG_CONTROLS_TWO   0x04
 #define QUSB2PHY_PLL_CLOCK_INVERTERS   0x18c
 #define QUSB2PHY_PLL_CMODE 0x2c
@@ -241,6 +252,18 @@ struct qusb2_phy_cfg {
  * @tcsr: TCSR syscon register map
  * @cell: nvmem cell containing phy tuning value
  *
+ * @override_imp_res_offset: PHY should use different rescode offset
+ * @imp_res_offset_value: rescode offset to be updated in IMP_CTRL1 register
+ *
+ * @override_hstx_trim: PHY should use different HSTX o/p current value
+ * @hstx_trim_value: HSTX_TRIM value to be updated in TUNE1 register
+ *
+ * @override_preemphasis: PHY should use different pre-amphasis amplitude
+ * @preemphasis_level: Amplitude Pre-Emphasis to be updated in TUNE1 register
+ *
+ * @override_preemphasis_width: PHY should use different pre-emphasis duration
+ * @preemphasis_width: half/full-width Pre-Emphasis updated via TUNE1
+ *
  * @cfg: phy config data
  * @has_se_clk_scheme: indicate if PHY has single-ended ref clock scheme
  * @phy_initialized: indicate if PHY has been initialized
@@ -259,12 +282,35 @@ struct qusb2_phy {
struct regmap *tcsr;
struct nvmem_cell *cell;
 
+   bool override_imp_res_offset;
+   u8 imp_res_offset_value;
+   bool override_hstx_trim;
+   u8 hstx_trim_value;
+   bool override_preemphasis;
+   u8 preemphasis_level;
+   bool override_preemphasis_width;
+   u8 preemphasis_width;
+
const struct qusb2_phy_cfg *cfg;
bool has_se_clk_scheme;
bool phy_initialized;
enum phy_mode mode;
 };
 
+static inline void qusb2_write_mask(void __iomem *base, u32 offset,
+   u32 val, u32 mask)
+{
+   u32 reg;
+
+   reg = readl(base + offset);
+   reg &= ~mask;
+   reg |= val;
+   writel(reg, base + offset);
+
+   /* Ensure above write is completed */
+   readl(base + offset);
+}
+
 static inline void qusb2_setbits(void __iomem *base, u32 offset, u32 val)
 {
u32 reg;
@@ -305,6 +351,42 @@ void qcom_qusb2_phy_configure(void __iomem *base,
 }
 
 /*
+ * Update board specific PHY tuning override values if specified from
+ * device tree.
+ *
+ */
+static void qusb2_phy_override_phy_params(struct qusb2_phy *qphy)
+{
+   const struct qusb2_phy_cfg *cfg = qphy->cfg;
+
+   if (qphy->override_imp_res_offset)
+   qusb2_write_mask(qphy->base, QUSB2PHY_IMP_CTRL1,
+qphy->imp_res_offset_value << IMP_RES_OFFSET_SHIFT,
+IMP_RES_OFFSET_MASK);
+
+   if (qphy->override_hstx_trim)
+   qusb2_write_mask(qphy->base, cfg->regs[QUSB2PHY_PORT_TUNE1],
+qphy->hstx_trim_value << HSTX_TRIM_SHIFT,
+HSTX_TRIM_MASK);
+
+   if (qphy->override_preemphasis)
+   qusb2_write_mask(qphy->base, cfg->regs[QUSB2PHY_PORT_TUNE1],
+   qphy->preemphasis_level << PREEMPHASIS_EN_SHIFT,
+   PREEMPHASIS_EN_MASK);
+
+   if (qphy->override_preemphasis_width) {
+   if (qphy->preemphasis_width)
+   qusb2_setbits(qphy->base,
+ cfg->regs[QUSB2PHY_PORT_TUNE1],
+ PREEMPH_HALF_WIDTH);
+   else
+   qusb2_clrbits(qphy->base,
+ cfg->regs[QUSB2PHY_PORT_TUNE1],
+ PREEMPH_HALF_WIDTH);
+   }
+}
+
+/*
  * Fetches HS Tx tuning value from nvmem and sets the
  * QUSB2PHY_PORT_TUNE1/2 register.
  * For error case, skip setting the value and use the default value.
@@ -525,6 +607,9 @@ static int qusb2_phy_init(struct 

[PATCH v4 7/7] phy: qcom-qusb2: Add QUSB2 PHYs support for sdm845

[Manu Gautam]

There are two QUSB2 PHYs present on sdm845. In order
to improve eye diagram for both the PHYs some parameters
need to be changed. Provide device tree properties to
override these from board specific device tree files.

Signed-off-by: Manu Gautam 
---
 drivers/phy/qualcomm/phy-qcom-qusb2.c | 112 +-
 1 file changed, 111 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qusb2.c 
b/drivers/phy/qualcomm/phy-qcom-qusb2.c
index 40fdef8..adcc3f8 100644
--- a/drivers/phy/qualcomm/phy-qcom-qusb2.c
+++ b/drivers/phy/qualcomm/phy-qcom-qusb2.c
@@ -60,6 +60,17 @@
 #define CORE_RESET BIT(5)
 #define CORE_RESET_MUX BIT(6)
 
+/* QUSB2PHY_IMP_CTRL1 register bits */
+#define IMP_RES_OFFSET_MASKGENMASK(5, 0)
+#define IMP_RES_OFFSET_SHIFT   0x0
+
+/* QUSB2PHY_PORT_TUNE1 register bits */
+#define HSTX_TRIM_MASK GENMASK(7, 4)
+#define HSTX_TRIM_SHIFT0x4
+#define PREEMPH_HALF_WIDTH BIT(2)
+#define PREEMPHASIS_EN_MASKGENMASK(1, 0)
+#define PREEMPHASIS_EN_SHIFT   0x0
+
 #define QUSB2PHY_PLL_ANALOG_CONTROLS_TWO   0x04
 #define QUSB2PHY_PLL_CLOCK_INVERTERS   0x18c
 #define QUSB2PHY_PLL_CMODE 0x2c
@@ -241,6 +252,18 @@ struct qusb2_phy_cfg {
  * @tcsr: TCSR syscon register map
  * @cell: nvmem cell containing phy tuning value
  *
+ * @override_imp_res_offset: PHY should use different rescode offset
+ * @imp_res_offset_value: rescode offset to be updated in IMP_CTRL1 register
+ *
+ * @override_hstx_trim: PHY should use different HSTX o/p current value
+ * @hstx_trim_value: HSTX_TRIM value to be updated in TUNE1 register
+ *
+ * @override_preemphasis: PHY should use different pre-amphasis amplitude
+ * @preemphasis_level: Amplitude Pre-Emphasis to be updated in TUNE1 register
+ *
+ * @override_preemphasis_width: PHY should use different pre-emphasis duration
+ * @preemphasis_width: half/full-width Pre-Emphasis updated via TUNE1
+ *
  * @cfg: phy config data
  * @has_se_clk_scheme: indicate if PHY has single-ended ref clock scheme
  * @phy_initialized: indicate if PHY has been initialized
@@ -259,12 +282,35 @@ struct qusb2_phy {
struct regmap *tcsr;
struct nvmem_cell *cell;
 
+   bool override_imp_res_offset;
+   u8 imp_res_offset_value;
+   bool override_hstx_trim;
+   u8 hstx_trim_value;
+   bool override_preemphasis;
+   u8 preemphasis_level;
+   bool override_preemphasis_width;
+   u8 preemphasis_width;
+
const struct qusb2_phy_cfg *cfg;
bool has_se_clk_scheme;
bool phy_initialized;
enum phy_mode mode;
 };
 
+static inline void qusb2_write_mask(void __iomem *base, u32 offset,
+   u32 val, u32 mask)
+{
+   u32 reg;
+
+   reg = readl(base + offset);
+   reg &= ~mask;
+   reg |= val;
+   writel(reg, base + offset);
+
+   /* Ensure above write is completed */
+   readl(base + offset);
+}
+
 static inline void qusb2_setbits(void __iomem *base, u32 offset, u32 val)
 {
u32 reg;
@@ -305,6 +351,42 @@ void qcom_qusb2_phy_configure(void __iomem *base,
 }
 
 /*
+ * Update board specific PHY tuning override values if specified from
+ * device tree.
+ *
+ */
+static void qusb2_phy_override_phy_params(struct qusb2_phy *qphy)
+{
+   const struct qusb2_phy_cfg *cfg = qphy->cfg;
+
+   if (qphy->override_imp_res_offset)
+   qusb2_write_mask(qphy->base, QUSB2PHY_IMP_CTRL1,
+qphy->imp_res_offset_value << IMP_RES_OFFSET_SHIFT,
+IMP_RES_OFFSET_MASK);
+
+   if (qphy->override_hstx_trim)
+   qusb2_write_mask(qphy->base, cfg->regs[QUSB2PHY_PORT_TUNE1],
+qphy->hstx_trim_value << HSTX_TRIM_SHIFT,
+HSTX_TRIM_MASK);
+
+   if (qphy->override_preemphasis)
+   qusb2_write_mask(qphy->base, cfg->regs[QUSB2PHY_PORT_TUNE1],
+   qphy->preemphasis_level << PREEMPHASIS_EN_SHIFT,
+   PREEMPHASIS_EN_MASK);
+
+   if (qphy->override_preemphasis_width) {
+   if (qphy->preemphasis_width)
+   qusb2_setbits(qphy->base,
+ cfg->regs[QUSB2PHY_PORT_TUNE1],
+ PREEMPH_HALF_WIDTH);
+   else
+   qusb2_clrbits(qphy->base,
+ cfg->regs[QUSB2PHY_PORT_TUNE1],
+ PREEMPH_HALF_WIDTH);
+   }
+}
+
+/*
  * Fetches HS Tx tuning value from nvmem and sets the
  * QUSB2PHY_PORT_TUNE1/2 register.
  * For error case, skip setting the value and use the default value.
@@ -525,6 +607,9 @@ static int qusb2_phy_init(struct phy *phy)