* Re: [PATCH 2/2] phy: qualcomm: phy-qcom-eusb2-repeater: Don't zero-out registers
2025-06-16 9:45 ` [PATCH 2/2] phy: qualcomm: phy-qcom-eusb2-repeater: Don't zero-out registers Luca Weiss
@ 2025-06-16 11:40 ` Dmitry Baryshkov
2025-06-16 11:50 ` Luca Weiss
2025-06-16 11:41 ` neil.armstrong
2025-06-16 11:44 ` Konrad Dybcio
2 siblings, 1 reply; 7+ messages in thread
From: Dmitry Baryshkov @ 2025-06-16 11:40 UTC (permalink / raw)
To: Luca Weiss
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Abel Vesa, Konrad Dybcio,
linux-arm-msm, linux-phy, devicetree, linux-kernel
On Mon, Jun 16, 2025 at 11:45:12AM +0200, Luca Weiss wrote:
> Zeroing out registers does not happen in the downstream kernel, and will
> "tune" the repeater in surely unexpected ways since most registers don't
> have a reset value of 0x0.
>
> Stop doing that and instead just set the registers that are in the init
> sequence (though long term I don't think there's actually PMIC-specific
> init sequences, there's board specific tuning, but that's a story for
> another day).
>
> Fixes: 99a517a582fc ("phy: qualcomm: phy-qcom-eusb2-repeater: Zero out untouched tuning regs")
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
> drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c | 63 +++++++++++++-------------
> 1 file changed, 32 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c
> index 6bd1b3c75c779d2db2744703262e132cc439f76e..a246c897fedb2edfd376ac5fdc0423607f8c562b 100644
> --- a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c
> +++ b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c
> @@ -61,8 +61,13 @@ enum eusb2_reg_layout {
> LAYOUT_SIZE,
> };
>
> +struct eusb2_repeater_init_tbl_reg {
> + u8 reg;
> + u8 value;
> +};
> +
> struct eusb2_repeater_cfg {
> - const u32 *init_tbl;
> + const struct eusb2_repeater_init_tbl_reg *init_tbl;
> int init_tbl_num;
> const char * const *vreg_list;
> int num_vregs;
> @@ -82,16 +87,16 @@ static const char * const pm8550b_vreg_l[] = {
> "vdd18", "vdd3",
> };
>
> -static const u32 pm8550b_init_tbl[NUM_TUNE_FIELDS] = {
> - [TUNE_IUSB2] = 0x8,
> - [TUNE_SQUELCH_U] = 0x3,
> - [TUNE_USB2_PREEM] = 0x5,
> +static const struct eusb2_repeater_init_tbl_reg pm8550b_init_tbl[] = {
> + { TUNE_IUSB2, 0x8 },
> + { TUNE_SQUELCH_U, 0x3 },
> + { TUNE_USB2_PREEM, 0x5 },
> };
>
> -static const u32 smb2360_init_tbl[NUM_TUNE_FIELDS] = {
> - [TUNE_IUSB2] = 0x5,
> - [TUNE_SQUELCH_U] = 0x3,
> - [TUNE_USB2_PREEM] = 0x2,
> +static const struct eusb2_repeater_init_tbl_reg smb2360_init_tbl[] = {
> + { TUNE_IUSB2, 0x5 },
> + { TUNE_SQUELCH_U, 0x3 },
> + { TUNE_USB2_PREEM, 0x2 },
> };
>
> static const struct eusb2_repeater_cfg pm8550b_eusb2_cfg = {
> @@ -129,17 +134,10 @@ static int eusb2_repeater_init(struct phy *phy)
> struct eusb2_repeater *rptr = phy_get_drvdata(phy);
> struct device_node *np = rptr->dev->of_node;
> struct regmap *regmap = rptr->regmap;
> - const u32 *init_tbl = rptr->cfg->init_tbl;
> - u8 tune_usb2_preem = init_tbl[TUNE_USB2_PREEM];
> - u8 tune_hsdisc = init_tbl[TUNE_HSDISC];
> - u8 tune_iusb2 = init_tbl[TUNE_IUSB2];
> u32 base = rptr->base;
> - u32 val;
> + u32 poll_val;
> int ret;
> -
> - of_property_read_u8(np, "qcom,tune-usb2-amplitude", &tune_iusb2);
> - of_property_read_u8(np, "qcom,tune-usb2-disc-thres", &tune_hsdisc);
> - of_property_read_u8(np, "qcom,tune-usb2-preem", &tune_usb2_preem);
> + u8 val;
>
> ret = regulator_bulk_enable(rptr->cfg->num_vregs, rptr->vregs);
> if (ret)
> @@ -147,21 +145,24 @@ static int eusb2_repeater_init(struct phy *phy)
>
> regmap_write(regmap, base + EUSB2_EN_CTL1, EUSB2_RPTR_EN);
>
> - regmap_write(regmap, base + EUSB2_TUNE_EUSB_HS_COMP_CUR, init_tbl[TUNE_EUSB_HS_COMP_CUR]);
> - regmap_write(regmap, base + EUSB2_TUNE_EUSB_EQU, init_tbl[TUNE_EUSB_EQU]);
> - regmap_write(regmap, base + EUSB2_TUNE_EUSB_SLEW, init_tbl[TUNE_EUSB_SLEW]);
> - regmap_write(regmap, base + EUSB2_TUNE_USB2_HS_COMP_CUR, init_tbl[TUNE_USB2_HS_COMP_CUR]);
> - regmap_write(regmap, base + EUSB2_TUNE_USB2_EQU, init_tbl[TUNE_USB2_EQU]);
> - regmap_write(regmap, base + EUSB2_TUNE_USB2_SLEW, init_tbl[TUNE_USB2_SLEW]);
> - regmap_write(regmap, base + EUSB2_TUNE_SQUELCH_U, init_tbl[TUNE_SQUELCH_U]);
> - regmap_write(regmap, base + EUSB2_TUNE_RES_FSDIF, init_tbl[TUNE_RES_FSDIF]);
> - regmap_write(regmap, base + EUSB2_TUNE_USB2_CROSSOVER, init_tbl[TUNE_USB2_CROSSOVER]);
> + /* Write registers from init table */
> + for (int i = 0; i < rptr->cfg->init_tbl_num; i++)
> + regmap_write(regmap, base + rptr->cfg->init_tbl[i].reg,
Init tables have TUNE_foo values in the .reg field instead of
EUSB2_TUNE_foo, which means that writes go to a random location.
> + rptr->cfg->init_tbl[i].value);
>
> - regmap_write(regmap, base + EUSB2_TUNE_USB2_PREEM, tune_usb2_preem);
> - regmap_write(regmap, base + EUSB2_TUNE_HSDISC, tune_hsdisc);
> - regmap_write(regmap, base + EUSB2_TUNE_IUSB2, tune_iusb2);
> + /* Override registers from devicetree values */
> + if (!of_property_read_u8(np, "qcom,tune-usb2-amplitude", &val))
> + regmap_write(regmap, base + EUSB2_TUNE_USB2_PREEM, val);
>
> - ret = regmap_read_poll_timeout(regmap, base + EUSB2_RPTR_STATUS, val, val & RPTR_OK, 10, 5);
> + if (!of_property_read_u8(np, "qcom,tune-usb2-disc-thres", &val))
> + regmap_write(regmap, base + EUSB2_TUNE_HSDISC, val);
> +
> + if (!of_property_read_u8(np, "qcom,tune-usb2-preem", &val))
> + regmap_write(regmap, base + EUSB2_TUNE_IUSB2, val);
> +
> + /* Wait for status OK */
> + ret = regmap_read_poll_timeout(regmap, base + EUSB2_RPTR_STATUS, poll_val,
> + poll_val & RPTR_OK, 10, 5);
> if (ret)
> dev_err(rptr->dev, "initialization timed-out\n");
>
>
> --
> 2.49.0
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 2/2] phy: qualcomm: phy-qcom-eusb2-repeater: Don't zero-out registers
2025-06-16 11:40 ` Dmitry Baryshkov
@ 2025-06-16 11:50 ` Luca Weiss
0 siblings, 0 replies; 7+ messages in thread
From: Luca Weiss @ 2025-06-16 11:50 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Abel Vesa, Konrad Dybcio,
linux-arm-msm, linux-phy, devicetree, linux-kernel
On Mon Jun 16, 2025 at 1:40 PM CEST, Dmitry Baryshkov wrote:
> On Mon, Jun 16, 2025 at 11:45:12AM +0200, Luca Weiss wrote:
>> Zeroing out registers does not happen in the downstream kernel, and will
>> "tune" the repeater in surely unexpected ways since most registers don't
>> have a reset value of 0x0.
>>
>> Stop doing that and instead just set the registers that are in the init
>> sequence (though long term I don't think there's actually PMIC-specific
>> init sequences, there's board specific tuning, but that's a story for
>> another day).
>>
>> Fixes: 99a517a582fc ("phy: qualcomm: phy-qcom-eusb2-repeater: Zero out untouched tuning regs")
>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>> ---
>> drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c | 63 +++++++++++++-------------
>> 1 file changed, 32 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c
>> index 6bd1b3c75c779d2db2744703262e132cc439f76e..a246c897fedb2edfd376ac5fdc0423607f8c562b 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c
>> @@ -61,8 +61,13 @@ enum eusb2_reg_layout {
>> LAYOUT_SIZE,
>> };
>>
>> +struct eusb2_repeater_init_tbl_reg {
>> + u8 reg;
>> + u8 value;
>> +};
>> +
>> struct eusb2_repeater_cfg {
>> - const u32 *init_tbl;
>> + const struct eusb2_repeater_init_tbl_reg *init_tbl;
>> int init_tbl_num;
>> const char * const *vreg_list;
>> int num_vregs;
>> @@ -82,16 +87,16 @@ static const char * const pm8550b_vreg_l[] = {
>> "vdd18", "vdd3",
>> };
>>
>> -static const u32 pm8550b_init_tbl[NUM_TUNE_FIELDS] = {
>> - [TUNE_IUSB2] = 0x8,
>> - [TUNE_SQUELCH_U] = 0x3,
>> - [TUNE_USB2_PREEM] = 0x5,
>> +static const struct eusb2_repeater_init_tbl_reg pm8550b_init_tbl[] = {
>> + { TUNE_IUSB2, 0x8 },
>> + { TUNE_SQUELCH_U, 0x3 },
>> + { TUNE_USB2_PREEM, 0x5 },
>> };
>>
>> -static const u32 smb2360_init_tbl[NUM_TUNE_FIELDS] = {
>> - [TUNE_IUSB2] = 0x5,
>> - [TUNE_SQUELCH_U] = 0x3,
>> - [TUNE_USB2_PREEM] = 0x2,
>> +static const struct eusb2_repeater_init_tbl_reg smb2360_init_tbl[] = {
>> + { TUNE_IUSB2, 0x5 },
>> + { TUNE_SQUELCH_U, 0x3 },
>> + { TUNE_USB2_PREEM, 0x2 },
>> };
>>
>> static const struct eusb2_repeater_cfg pm8550b_eusb2_cfg = {
>> @@ -129,17 +134,10 @@ static int eusb2_repeater_init(struct phy *phy)
>> struct eusb2_repeater *rptr = phy_get_drvdata(phy);
>> struct device_node *np = rptr->dev->of_node;
>> struct regmap *regmap = rptr->regmap;
>> - const u32 *init_tbl = rptr->cfg->init_tbl;
>> - u8 tune_usb2_preem = init_tbl[TUNE_USB2_PREEM];
>> - u8 tune_hsdisc = init_tbl[TUNE_HSDISC];
>> - u8 tune_iusb2 = init_tbl[TUNE_IUSB2];
>> u32 base = rptr->base;
>> - u32 val;
>> + u32 poll_val;
>> int ret;
>> -
>> - of_property_read_u8(np, "qcom,tune-usb2-amplitude", &tune_iusb2);
>> - of_property_read_u8(np, "qcom,tune-usb2-disc-thres", &tune_hsdisc);
>> - of_property_read_u8(np, "qcom,tune-usb2-preem", &tune_usb2_preem);
>> + u8 val;
>>
>> ret = regulator_bulk_enable(rptr->cfg->num_vregs, rptr->vregs);
>> if (ret)
>> @@ -147,21 +145,24 @@ static int eusb2_repeater_init(struct phy *phy)
>>
>> regmap_write(regmap, base + EUSB2_EN_CTL1, EUSB2_RPTR_EN);
>>
>> - regmap_write(regmap, base + EUSB2_TUNE_EUSB_HS_COMP_CUR, init_tbl[TUNE_EUSB_HS_COMP_CUR]);
>> - regmap_write(regmap, base + EUSB2_TUNE_EUSB_EQU, init_tbl[TUNE_EUSB_EQU]);
>> - regmap_write(regmap, base + EUSB2_TUNE_EUSB_SLEW, init_tbl[TUNE_EUSB_SLEW]);
>> - regmap_write(regmap, base + EUSB2_TUNE_USB2_HS_COMP_CUR, init_tbl[TUNE_USB2_HS_COMP_CUR]);
>> - regmap_write(regmap, base + EUSB2_TUNE_USB2_EQU, init_tbl[TUNE_USB2_EQU]);
>> - regmap_write(regmap, base + EUSB2_TUNE_USB2_SLEW, init_tbl[TUNE_USB2_SLEW]);
>> - regmap_write(regmap, base + EUSB2_TUNE_SQUELCH_U, init_tbl[TUNE_SQUELCH_U]);
>> - regmap_write(regmap, base + EUSB2_TUNE_RES_FSDIF, init_tbl[TUNE_RES_FSDIF]);
>> - regmap_write(regmap, base + EUSB2_TUNE_USB2_CROSSOVER, init_tbl[TUNE_USB2_CROSSOVER]);
>> + /* Write registers from init table */
>> + for (int i = 0; i < rptr->cfg->init_tbl_num; i++)
>> + regmap_write(regmap, base + rptr->cfg->init_tbl[i].reg,
>
> Init tables have TUNE_foo values in the .reg field instead of
> EUSB2_TUNE_foo, which means that writes go to a random location.
Right, stupid mistake. Thanks for spotting!
I will fix the init tables to use EUSB2_TUNE_*, and probably drop this
"enum eusb2_reg_layout" completely.
Regards
Luca
>
>> + rptr->cfg->init_tbl[i].value);
>>
>> - regmap_write(regmap, base + EUSB2_TUNE_USB2_PREEM, tune_usb2_preem);
>> - regmap_write(regmap, base + EUSB2_TUNE_HSDISC, tune_hsdisc);
>> - regmap_write(regmap, base + EUSB2_TUNE_IUSB2, tune_iusb2);
>> + /* Override registers from devicetree values */
>> + if (!of_property_read_u8(np, "qcom,tune-usb2-amplitude", &val))
>> + regmap_write(regmap, base + EUSB2_TUNE_USB2_PREEM, val);
>>
>> - ret = regmap_read_poll_timeout(regmap, base + EUSB2_RPTR_STATUS, val, val & RPTR_OK, 10, 5);
>> + if (!of_property_read_u8(np, "qcom,tune-usb2-disc-thres", &val))
>> + regmap_write(regmap, base + EUSB2_TUNE_HSDISC, val);
>> +
>> + if (!of_property_read_u8(np, "qcom,tune-usb2-preem", &val))
>> + regmap_write(regmap, base + EUSB2_TUNE_IUSB2, val);
>> +
>> + /* Wait for status OK */
>> + ret = regmap_read_poll_timeout(regmap, base + EUSB2_RPTR_STATUS, poll_val,
>> + poll_val & RPTR_OK, 10, 5);
>> if (ret)
>> dev_err(rptr->dev, "initialization timed-out\n");
>>
>>
>> --
>> 2.49.0
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] phy: qualcomm: phy-qcom-eusb2-repeater: Don't zero-out registers
2025-06-16 9:45 ` [PATCH 2/2] phy: qualcomm: phy-qcom-eusb2-repeater: Don't zero-out registers Luca Weiss
2025-06-16 11:40 ` Dmitry Baryshkov
@ 2025-06-16 11:41 ` neil.armstrong
2025-06-16 11:44 ` Konrad Dybcio
2 siblings, 0 replies; 7+ messages in thread
From: neil.armstrong @ 2025-06-16 11:41 UTC (permalink / raw)
To: Luca Weiss, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Abel Vesa, Konrad Dybcio
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel
On 16/06/2025 11:45, Luca Weiss wrote:
> Zeroing out registers does not happen in the downstream kernel, and will
> "tune" the repeater in surely unexpected ways since most registers don't
> have a reset value of 0x0.
>
> Stop doing that and instead just set the registers that are in the init
> sequence (though long term I don't think there's actually PMIC-specific
> init sequences, there's board specific tuning, but that's a story for
> another day).
>
> Fixes: 99a517a582fc ("phy: qualcomm: phy-qcom-eusb2-repeater: Zero out untouched tuning regs")
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
> drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c | 63 +++++++++++++-------------
> 1 file changed, 32 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c
> index 6bd1b3c75c779d2db2744703262e132cc439f76e..a246c897fedb2edfd376ac5fdc0423607f8c562b 100644
> --- a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c
> +++ b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c
> @@ -61,8 +61,13 @@ enum eusb2_reg_layout {
> LAYOUT_SIZE,
> };
>
> +struct eusb2_repeater_init_tbl_reg {
> + u8 reg;
No need to be u8 here, could simply be unsigned int.
> + u8 value;
Same for this one since it's passed to regmap_write which takes unsigned int.
Using u8 here won't save any memory since u8 will be aligned on at least u32
reading u8 will involve at least an u32 read with some masking operations,
and perhaps the compiler will just drop u8 after all.
> +};
> +
> struct eusb2_repeater_cfg {
> - const u32 *init_tbl;
> + const struct eusb2_repeater_init_tbl_reg *init_tbl;
> int init_tbl_num;
> const char * const *vreg_list;
> int num_vregs;
> @@ -82,16 +87,16 @@ static const char * const pm8550b_vreg_l[] = {
> "vdd18", "vdd3",
> };
>
> -static const u32 pm8550b_init_tbl[NUM_TUNE_FIELDS] = {
> - [TUNE_IUSB2] = 0x8,
> - [TUNE_SQUELCH_U] = 0x3,
> - [TUNE_USB2_PREEM] = 0x5,
> +static const struct eusb2_repeater_init_tbl_reg pm8550b_init_tbl[] = {
> + { TUNE_IUSB2, 0x8 },
> + { TUNE_SQUELCH_U, 0x3 },
> + { TUNE_USB2_PREEM, 0x5 },
> };
>
> -static const u32 smb2360_init_tbl[NUM_TUNE_FIELDS] = {
> - [TUNE_IUSB2] = 0x5,
> - [TUNE_SQUELCH_U] = 0x3,
> - [TUNE_USB2_PREEM] = 0x2,
> +static const struct eusb2_repeater_init_tbl_reg smb2360_init_tbl[] = {
> + { TUNE_IUSB2, 0x5 },
> + { TUNE_SQUELCH_U, 0x3 },
> + { TUNE_USB2_PREEM, 0x2 },
> };
>
> static const struct eusb2_repeater_cfg pm8550b_eusb2_cfg = {
> @@ -129,17 +134,10 @@ static int eusb2_repeater_init(struct phy *phy)
> struct eusb2_repeater *rptr = phy_get_drvdata(phy);
> struct device_node *np = rptr->dev->of_node;
> struct regmap *regmap = rptr->regmap;
> - const u32 *init_tbl = rptr->cfg->init_tbl;
> - u8 tune_usb2_preem = init_tbl[TUNE_USB2_PREEM];
> - u8 tune_hsdisc = init_tbl[TUNE_HSDISC];
> - u8 tune_iusb2 = init_tbl[TUNE_IUSB2];
> u32 base = rptr->base;
> - u32 val;
> + u32 poll_val;
> int ret;
> -
> - of_property_read_u8(np, "qcom,tune-usb2-amplitude", &tune_iusb2);
> - of_property_read_u8(np, "qcom,tune-usb2-disc-thres", &tune_hsdisc);
> - of_property_read_u8(np, "qcom,tune-usb2-preem", &tune_usb2_preem);
> + u8 val;
>
> ret = regulator_bulk_enable(rptr->cfg->num_vregs, rptr->vregs);
> if (ret)
> @@ -147,21 +145,24 @@ static int eusb2_repeater_init(struct phy *phy)
>
> regmap_write(regmap, base + EUSB2_EN_CTL1, EUSB2_RPTR_EN);
>
> - regmap_write(regmap, base + EUSB2_TUNE_EUSB_HS_COMP_CUR, init_tbl[TUNE_EUSB_HS_COMP_CUR]);
> - regmap_write(regmap, base + EUSB2_TUNE_EUSB_EQU, init_tbl[TUNE_EUSB_EQU]);
> - regmap_write(regmap, base + EUSB2_TUNE_EUSB_SLEW, init_tbl[TUNE_EUSB_SLEW]);
> - regmap_write(regmap, base + EUSB2_TUNE_USB2_HS_COMP_CUR, init_tbl[TUNE_USB2_HS_COMP_CUR]);
> - regmap_write(regmap, base + EUSB2_TUNE_USB2_EQU, init_tbl[TUNE_USB2_EQU]);
> - regmap_write(regmap, base + EUSB2_TUNE_USB2_SLEW, init_tbl[TUNE_USB2_SLEW]);
> - regmap_write(regmap, base + EUSB2_TUNE_SQUELCH_U, init_tbl[TUNE_SQUELCH_U]);
> - regmap_write(regmap, base + EUSB2_TUNE_RES_FSDIF, init_tbl[TUNE_RES_FSDIF]);
> - regmap_write(regmap, base + EUSB2_TUNE_USB2_CROSSOVER, init_tbl[TUNE_USB2_CROSSOVER]);
> + /* Write registers from init table */
> + for (int i = 0; i < rptr->cfg->init_tbl_num; i++)
> + regmap_write(regmap, base + rptr->cfg->init_tbl[i].reg,
> + rptr->cfg->init_tbl[i].value);
>
> - regmap_write(regmap, base + EUSB2_TUNE_USB2_PREEM, tune_usb2_preem);
> - regmap_write(regmap, base + EUSB2_TUNE_HSDISC, tune_hsdisc);
> - regmap_write(regmap, base + EUSB2_TUNE_IUSB2, tune_iusb2);
> + /* Override registers from devicetree values */
> + if (!of_property_read_u8(np, "qcom,tune-usb2-amplitude", &val))
> + regmap_write(regmap, base + EUSB2_TUNE_USB2_PREEM, val);
>
> - ret = regmap_read_poll_timeout(regmap, base + EUSB2_RPTR_STATUS, val, val & RPTR_OK, 10, 5);
> + if (!of_property_read_u8(np, "qcom,tune-usb2-disc-thres", &val))
> + regmap_write(regmap, base + EUSB2_TUNE_HSDISC, val);
> +
> + if (!of_property_read_u8(np, "qcom,tune-usb2-preem", &val))
> + regmap_write(regmap, base + EUSB2_TUNE_IUSB2, val);
> +
> + /* Wait for status OK */
> + ret = regmap_read_poll_timeout(regmap, base + EUSB2_RPTR_STATUS, poll_val,
> + poll_val & RPTR_OK, 10, 5);
> if (ret)
> dev_err(rptr->dev, "initialization timed-out\n");
>
>
With that fixed:
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Thanks,
Neil
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 2/2] phy: qualcomm: phy-qcom-eusb2-repeater: Don't zero-out registers
2025-06-16 9:45 ` [PATCH 2/2] phy: qualcomm: phy-qcom-eusb2-repeater: Don't zero-out registers Luca Weiss
2025-06-16 11:40 ` Dmitry Baryshkov
2025-06-16 11:41 ` neil.armstrong
@ 2025-06-16 11:44 ` Konrad Dybcio
2 siblings, 0 replies; 7+ messages in thread
From: Konrad Dybcio @ 2025-06-16 11:44 UTC (permalink / raw)
To: Luca Weiss, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Abel Vesa, Konrad Dybcio
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel
On 6/16/25 11:45 AM, Luca Weiss wrote:
> Zeroing out registers does not happen in the downstream kernel, and will
> "tune" the repeater in surely unexpected ways since most registers don't
> have a reset value of 0x0.
>
> Stop doing that and instead just set the registers that are in the init
> sequence (though long term I don't think there's actually PMIC-specific
> init sequences, there's board specific tuning, but that's a story for
> another day).
>
> Fixes: 99a517a582fc ("phy: qualcomm: phy-qcom-eusb2-repeater: Zero out untouched tuning regs")
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
Riiight I thought this was effectively reverted already..
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply [flat|nested] 7+ messages in thread