* [PATCH v2 0/2] phy: qualcomm: phy-qcom-eusb2-repeater: rework reg override handler
@ 2025-04-16 12:01 Pengyu Luo
2025-04-16 12:02 ` [PATCH v2 1/2] dt-bindings: phy: qcom,snps-eusb2-repeater: Add more tuning overrides Pengyu Luo
2025-04-16 12:02 ` [PATCH v2 2/2] phy: qualcomm: phy-qcom-eusb2-repeater: rework reg override handler Pengyu Luo
0 siblings, 2 replies; 8+ messages in thread
From: Pengyu Luo @ 2025-04-16 12:01 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Pengyu Luo, Stephen Rothwell,
Abel Vesa
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel
In downstream tree, many registers need to be overrided, it varies
from devices and platforms, with these registers getting more, adding
a handler for this is helpful.
Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
---
Changes in v2:
- Drop dts changes, backwards compatible now
- Changes for dt-bindings (Krzysztof)
- Drop sequence overrides to use single property (Dmitry)
- Link to v1: https://lore.kernel.org/linux-arm-msm/20250405174319.405975-1-mitltlatltl@gmail.com
---
Pengyu Luo (2):
dt-bindings: phy: qcom,snps-eusb2-repeater: Add more tuning overrides
phy: qualcomm: phy-qcom-eusb2-repeater: rework reg override handler
.../phy/qcom,snps-eusb2-repeater.yaml | 41 ++++++-
.../phy/qualcomm/phy-qcom-eusb2-repeater.c | 105 +++++++++++++++---
2 files changed, 130 insertions(+), 16 deletions(-)
---
base-commit: afd30f00ef898643491ce0c0c2571b3498f1a61e
--
2.49.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] dt-bindings: phy: qcom,snps-eusb2-repeater: Add more tuning overrides
2025-04-16 12:01 [PATCH v2 0/2] phy: qualcomm: phy-qcom-eusb2-repeater: rework reg override handler Pengyu Luo
@ 2025-04-16 12:02 ` Pengyu Luo
2025-04-21 21:32 ` Rob Herring
2025-04-16 12:02 ` [PATCH v2 2/2] phy: qualcomm: phy-qcom-eusb2-repeater: rework reg override handler Pengyu Luo
1 sibling, 1 reply; 8+ messages in thread
From: Pengyu Luo @ 2025-04-16 12:02 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Pengyu Luo, Stephen Rothwell,
Abel Vesa
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel
In downstream tree, many registers need to be overridden, it varies
from devices and platforms, not only HS trasmit amplitude(0x51),
HS disconnect threshold(0x53), Tx pre-emphasis tuning(0x57). And
some devices require different overrides for different dr_mode. So
add property entries for host mode, original entries will be used
for device mode.
Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
---
.../phy/qcom,snps-eusb2-repeater.yaml | 41 +++++++++++++++++--
1 file changed, 38 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/qcom,snps-eusb2-repeater.yaml b/Documentation/devicetree/bindings/phy/qcom,snps-eusb2-repeater.yaml
index d16a543a7..d0f5e770d 100644
--- a/Documentation/devicetree/bindings/phy/qcom,snps-eusb2-repeater.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,snps-eusb2-repeater.yaml
@@ -36,25 +36,60 @@ properties:
qcom,tune-usb2-disc-thres:
$ref: /schemas/types.yaml#/definitions/uint8
- description: High-Speed disconnect threshold
+ description: High-Speed disconnect threshold for device mode
minimum: 0
maximum: 7
default: 0
qcom,tune-usb2-amplitude:
$ref: /schemas/types.yaml#/definitions/uint8
- description: High-Speed transmit amplitude
+ description: High-Speed transmit amplitude for device mode
minimum: 0
maximum: 15
default: 8
qcom,tune-usb2-preem:
$ref: /schemas/types.yaml#/definitions/uint8
- description: High-Speed TX pre-emphasis tuning
+ description: High-Speed TX pre-emphasis tuning for device mode
minimum: 0
maximum: 7
default: 5
+ qcom,tune-usb2-squelch:
+ $ref: /schemas/types.yaml#/definitions/uint8
+ description: Squelch detection threshold tuning for device mode
+ minimum: 0
+ maximum: 7
+ default: 3
+
+ qcom,tune-usb2-disc-thres-host:
+ $ref: /schemas/types.yaml#/definitions/uint8
+ description: High-Speed disconnect threshold for host mode
+ minimum: 0
+ maximum: 7
+ default: 0
+
+ qcom,tune-usb2-amplitude-host:
+ $ref: /schemas/types.yaml#/definitions/uint8
+ description: High-Speed transmit amplitude for host mode
+ minimum: 0
+ maximum: 15
+ default: 8
+
+ qcom,tune-usb2-preem-host:
+ $ref: /schemas/types.yaml#/definitions/uint8
+ description: High-Speed TX pre-emphasis tuning for host mode
+ minimum: 0
+ maximum: 7
+ default: 5
+
+ qcom,tune-usb2-squelch-host:
+ $ref: /schemas/types.yaml#/definitions/uint8
+ description: Squelch detection threshold tuning for host mode
+ minimum: 0
+ maximum: 7
+ default: 3
+
required:
- compatible
- reg
--
2.49.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] phy: qualcomm: phy-qcom-eusb2-repeater: rework reg override handler
2025-04-16 12:01 [PATCH v2 0/2] phy: qualcomm: phy-qcom-eusb2-repeater: rework reg override handler Pengyu Luo
2025-04-16 12:02 ` [PATCH v2 1/2] dt-bindings: phy: qcom,snps-eusb2-repeater: Add more tuning overrides Pengyu Luo
@ 2025-04-16 12:02 ` Pengyu Luo
2025-04-25 19:41 ` Dmitry Baryshkov
1 sibling, 1 reply; 8+ messages in thread
From: Pengyu Luo @ 2025-04-16 12:02 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Pengyu Luo, Stephen Rothwell,
Abel Vesa
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel
In downstream tree, many registers need to be overridden, it varies
from devices and platforms, with these registers getting more, adding
a handler for this is helpful.
Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
---
.../phy/qualcomm/phy-qcom-eusb2-repeater.c | 105 +++++++++++++++---
1 file changed, 92 insertions(+), 13 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c
index 6bd1b3c75..a4b75e70e 100644
--- a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c
+++ b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c
@@ -68,12 +68,27 @@ struct eusb2_repeater_cfg {
int num_vregs;
};
+struct tune {
+ const char * const tune_name;
+ const char * const host_tune_name;
+ enum eusb2_reg_layout init_tbl_idx;
+ u8 tune_reg;
+ u8 tune;
+ u8 host_tune;
+};
+
+struct tune_cfg {
+ struct tune *tune_tbl;
+ int tune_tbl_size;
+};
+
struct eusb2_repeater {
struct device *dev;
struct regmap *regmap;
struct phy *phy;
struct regulator_bulk_data *vregs;
const struct eusb2_repeater_cfg *cfg;
+ struct tune_cfg *tune_cfg;
u32 base;
enum phy_mode mode;
};
@@ -108,6 +123,79 @@ static const struct eusb2_repeater_cfg smb2360_eusb2_cfg = {
.num_vregs = ARRAY_SIZE(pm8550b_vreg_l),
};
+static struct tune tune_tbl[] = {
+ {
+ .tune_name = "qcom,tune-usb2-amplitude",
+ .host_tune_name = "qcom,tune-usb2-amplitude-host",
+ .init_tbl_idx = TUNE_IUSB2,
+ .tune_reg = EUSB2_TUNE_IUSB2,
+ },
+ {
+ .tune_name = "qcom,tune-usb2-disc-thres",
+ .host_tune_name = "qcom,tune-usb2-disc-thres-host",
+ .init_tbl_idx = TUNE_HSDISC,
+ .tune_reg = EUSB2_TUNE_HSDISC,
+
+ },
+ {
+ .tune_name = "qcom,tune-usb2-squelch",
+ .host_tune_name = "qcom,tune-usb2-squelch-host",
+ .init_tbl_idx = TUNE_SQUELCH_U,
+ .tune_reg = EUSB2_TUNE_SQUELCH_U,
+ },
+ {
+ .tune_name = "qcom,tune-usb2-preem",
+ .host_tune_name = "qcom,tune-usb2-preem-host",
+ .init_tbl_idx = TUNE_USB2_PREEM,
+ .tune_reg = EUSB2_TUNE_USB2_PREEM,
+ },
+};
+
+static struct tune_cfg tune_cfg = {
+ .tune_tbl = tune_tbl,
+ .tune_tbl_size = ARRAY_SIZE(tune_tbl),
+};
+
+static void eusb2_repeater_write_overrides(struct eusb2_repeater *rptr,
+ bool is_host_mode)
+{
+ struct tune *tune_tbl;
+ int size, i;
+
+ tune_tbl = rptr->tune_cfg->tune_tbl;
+ size = rptr->tune_cfg->tune_tbl_size;
+
+ for (i = 0; i < size; ++i)
+ regmap_write(rptr->regmap, rptr->base + tune_tbl[i].tune_reg,
+ is_host_mode ? tune_tbl[i].host_tune : tune_tbl[i].tune);
+}
+
+static void eusb2_repeater_parse_dt(struct eusb2_repeater *rptr)
+{
+ struct device_node *np = rptr->dev->of_node;
+ const u32 *init_tbl = rptr->cfg->init_tbl;
+ struct tune *tune_tbl;
+ int size, i, idx;
+
+ tune_tbl = tune_cfg.tune_tbl;
+ size = tune_cfg.tune_tbl_size;
+
+ for (i = 0; i < size; ++i) {
+ /* set default values from init_tbl */
+ idx = tune_tbl[i].init_tbl_idx;
+ tune_tbl[i].tune = init_tbl[idx];
+ tune_tbl[i].host_tune = init_tbl[idx];
+
+ /* update values from dt */
+ of_property_read_u8(np, tune_tbl[i].tune_name,
+ &tune_tbl[i].tune);
+ of_property_read_u8(np, tune_tbl[i].host_tune_name,
+ &tune_tbl[i].host_tune);
+ }
+
+ rptr->tune_cfg = &tune_cfg;
+}
+
static int eusb2_repeater_init_vregs(struct eusb2_repeater *rptr)
{
int num = rptr->cfg->num_vregs;
@@ -127,20 +215,12 @@ static int eusb2_repeater_init_vregs(struct eusb2_repeater *rptr)
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;
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);
-
ret = regulator_bulk_enable(rptr->cfg->num_vregs, rptr->vregs);
if (ret)
return ret;
@@ -153,14 +233,9 @@ static int eusb2_repeater_init(struct phy *phy)
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]);
- 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);
-
ret = regmap_read_poll_timeout(regmap, base + EUSB2_RPTR_STATUS, val, val & RPTR_OK, 10, 5);
if (ret)
dev_err(rptr->dev, "initialization timed-out\n");
@@ -177,6 +252,7 @@ static int eusb2_repeater_set_mode(struct phy *phy,
switch (mode) {
case PHY_MODE_USB_HOST:
+ eusb2_repeater_write_overrides(rptr, true);
/*
* CM.Lx is prohibited when repeater is already into Lx state as
* per eUSB 1.2 Spec. Below implement software workaround until
@@ -186,6 +262,7 @@ static int eusb2_repeater_set_mode(struct phy *phy,
regmap_write(regmap, base + EUSB2_FORCE_VAL_5, V_CLK_19P2M_EN);
break;
case PHY_MODE_USB_DEVICE:
+ eusb2_repeater_write_overrides(rptr, false);
/*
* In device mode clear host mode related workaround as there
* is no repeater reset available, and enable/disable of
@@ -252,6 +329,8 @@ static int eusb2_repeater_probe(struct platform_device *pdev)
return ret;
}
+ eusb2_repeater_parse_dt(rptr);
+
rptr->phy = devm_phy_create(dev, np, &eusb2_repeater_ops);
if (IS_ERR(rptr->phy)) {
dev_err(dev, "failed to create PHY: %d\n", ret);
--
2.49.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: phy: qcom,snps-eusb2-repeater: Add more tuning overrides
2025-04-16 12:02 ` [PATCH v2 1/2] dt-bindings: phy: qcom,snps-eusb2-repeater: Add more tuning overrides Pengyu Luo
@ 2025-04-21 21:32 ` Rob Herring
0 siblings, 0 replies; 8+ messages in thread
From: Rob Herring @ 2025-04-21 21:32 UTC (permalink / raw)
To: Pengyu Luo
Cc: Vinod Koul, Kishon Vijay Abraham I, Krzysztof Kozlowski,
Conor Dooley, Stephen Rothwell, Abel Vesa, linux-arm-msm,
linux-phy, devicetree, linux-kernel
On Wed, Apr 16, 2025 at 08:02:00PM +0800, Pengyu Luo wrote:
> In downstream tree, many registers need to be overridden, it varies
> from devices and platforms, not only HS trasmit amplitude(0x51),
spelling
> HS disconnect threshold(0x53), Tx pre-emphasis tuning(0x57). And
> some devices require different overrides for different dr_mode. So
> add property entries for host mode, original entries will be used
> for device mode.
>
> Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
> ---
> .../phy/qcom,snps-eusb2-repeater.yaml | 41 +++++++++++++++++--
> 1 file changed, 38 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom,snps-eusb2-repeater.yaml b/Documentation/devicetree/bindings/phy/qcom,snps-eusb2-repeater.yaml
> index d16a543a7..d0f5e770d 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,snps-eusb2-repeater.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,snps-eusb2-repeater.yaml
> @@ -36,25 +36,60 @@ properties:
>
> qcom,tune-usb2-disc-thres:
> $ref: /schemas/types.yaml#/definitions/uint8
> - description: High-Speed disconnect threshold
> + description: High-Speed disconnect threshold for device mode
> minimum: 0
> maximum: 7
> default: 0
>
> qcom,tune-usb2-amplitude:
> $ref: /schemas/types.yaml#/definitions/uint8
> - description: High-Speed transmit amplitude
> + description: High-Speed transmit amplitude for device mode
> minimum: 0
> maximum: 15
> default: 8
>
> qcom,tune-usb2-preem:
> $ref: /schemas/types.yaml#/definitions/uint8
> - description: High-Speed TX pre-emphasis tuning
> + description: High-Speed TX pre-emphasis tuning for device mode
> minimum: 0
> maximum: 7
> default: 5
>
> + qcom,tune-usb2-squelch:
> + $ref: /schemas/types.yaml#/definitions/uint8
> + description: Squelch detection threshold tuning for device mode
> + minimum: 0
> + maximum: 7
> + default: 3
> +
> + qcom,tune-usb2-disc-thres-host:
Put this and the others after each existing property (for device mode).
Ideally, these would all be in alphabetical order.
> + $ref: /schemas/types.yaml#/definitions/uint8
> + description: High-Speed disconnect threshold for host mode
> + minimum: 0
> + maximum: 7
> + default: 0
> +
> + qcom,tune-usb2-amplitude-host:
> + $ref: /schemas/types.yaml#/definitions/uint8
> + description: High-Speed transmit amplitude for host mode
> + minimum: 0
> + maximum: 15
> + default: 8
> +
> + qcom,tune-usb2-preem-host:
> + $ref: /schemas/types.yaml#/definitions/uint8
> + description: High-Speed TX pre-emphasis tuning for host mode
> + minimum: 0
> + maximum: 7
> + default: 5
> +
> + qcom,tune-usb2-squelch-host:
> + $ref: /schemas/types.yaml#/definitions/uint8
> + description: Squelch detection threshold tuning for host mode
> + minimum: 0
> + maximum: 7
> + default: 3
> +
> required:
> - compatible
> - reg
> --
> 2.49.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] phy: qualcomm: phy-qcom-eusb2-repeater: rework reg override handler
2025-04-16 12:02 ` [PATCH v2 2/2] phy: qualcomm: phy-qcom-eusb2-repeater: rework reg override handler Pengyu Luo
@ 2025-04-25 19:41 ` Dmitry Baryshkov
2025-04-26 8:14 ` Pengyu Luo
0 siblings, 1 reply; 8+ messages in thread
From: Dmitry Baryshkov @ 2025-04-25 19:41 UTC (permalink / raw)
To: Pengyu Luo
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Stephen Rothwell, Abel Vesa,
linux-arm-msm, linux-phy, devicetree, linux-kernel
On Wed, Apr 16, 2025 at 08:02:01PM +0800, Pengyu Luo wrote:
> In downstream tree, many registers need to be overridden, it varies
> from devices and platforms, with these registers getting more, adding
> a handler for this is helpful.
It should be noted that previously all values were applied during _init
phase, before checking the status etc. Now the overrides are programmed
from the set_mode. Should you still program sane defaults at the init
stage too?
BTW, is there a real need to override those for the platform you are
working on? Could you please provide some details, maybe in the cover
letter.
> Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
> ---
> .../phy/qualcomm/phy-qcom-eusb2-repeater.c | 105 +++++++++++++++---
> 1 file changed, 92 insertions(+), 13 deletions(-)
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] phy: qualcomm: phy-qcom-eusb2-repeater: rework reg override handler
2025-04-25 19:41 ` Dmitry Baryshkov
@ 2025-04-26 8:14 ` Pengyu Luo
2025-04-28 16:58 ` Dmitry Baryshkov
0 siblings, 1 reply; 8+ messages in thread
From: Pengyu Luo @ 2025-04-26 8:14 UTC (permalink / raw)
To: dmitry.baryshkov
Cc: abel.vesa, conor+dt, devicetree, kishon, krzk+dt, linux-arm-msm,
linux-kernel, linux-phy, mitltlatltl, robh, sfr, vkoul
On Sat, Apr 26, 2025 at 3:41 AM Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote:
> On Wed, Apr 16, 2025 at 08:02:01PM +0800, Pengyu Luo wrote:
> > In downstream tree, many registers need to be overridden, it varies
> > from devices and platforms, with these registers getting more, adding
> > a handler for this is helpful.
>
> It should be noted that previously all values were applied during _init
> phase, before checking the status etc. Now the overrides are programmed
> from the set_mode. Should you still program sane defaults at the init
> stage too?
>
I think programming in set_mode is ok. When we init(dwc3_core_init), we
set_mode(dwc3_core_init_mode) later, please check
https://elixir.bootlin.com/linux/v6.14.3/source/drivers/usb/dwc3/core.c#L2287
Actually, in the downstream, all the things are done in init, it
overrides first, then masked write the deaults, finally it set_mode,
you can check here
https://github.com/OnePlusOSS/android_kernel_oneplus_sm8650/blob/oneplus/sm8650_v_15.0.0_pad_pro/drivers/usb/repeater/repeater-qti-pmic-eusb2.c#L356
> BTW, is there a real need to override those for the platform you are
> working on? Could you please provide some details, maybe in the cover
> letter.
I am not quite sure, recently, I expirenced mode switching failure,
when I `echo device > /sys/kernel/debug/usb/a600000.usb/mode`, Ethernet
Gadget wouldn't work again, my desktop can't connect to it.
BTW, as you can see in line 356, it is most likely that overrides
related to charging feature. I have not ported charging yet, but adding
more overrides seems harmless, and if overriding, distinguishing which
mode seems necessary, even if some devices use the same sequence. So I
sent the patch.
Best wishes,
Pengyu
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] phy: qualcomm: phy-qcom-eusb2-repeater: rework reg override handler
2025-04-26 8:14 ` Pengyu Luo
@ 2025-04-28 16:58 ` Dmitry Baryshkov
2025-04-29 9:23 ` Pengyu Luo
0 siblings, 1 reply; 8+ messages in thread
From: Dmitry Baryshkov @ 2025-04-28 16:58 UTC (permalink / raw)
To: Pengyu Luo
Cc: abel.vesa, conor+dt, devicetree, kishon, krzk+dt, linux-arm-msm,
linux-kernel, linux-phy, robh, sfr, vkoul
On Sat, Apr 26, 2025 at 04:14:23PM +0800, Pengyu Luo wrote:
> On Sat, Apr 26, 2025 at 3:41 AM Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote:
> > On Wed, Apr 16, 2025 at 08:02:01PM +0800, Pengyu Luo wrote:
> > > In downstream tree, many registers need to be overridden, it varies
> > > from devices and platforms, with these registers getting more, adding
> > > a handler for this is helpful.
> >
> > It should be noted that previously all values were applied during _init
> > phase, before checking the status etc. Now the overrides are programmed
> > from the set_mode. Should you still program sane defaults at the init
> > stage too?
> >
>
> I think programming in set_mode is ok. When we init(dwc3_core_init), we
> set_mode(dwc3_core_init_mode) later, please check
> https://elixir.bootlin.com/linux/v6.14.3/source/drivers/usb/dwc3/core.c#L2287
Yes, but that happens after reading status regs, etc.
> Actually, in the downstream, all the things are done in init, it
> overrides first, then masked write the deaults, finally it set_mode,
> you can check here
> https://github.com/OnePlusOSS/android_kernel_oneplus_sm8650/blob/oneplus/sm8650_v_15.0.0_pad_pro/drivers/usb/repeater/repeater-qti-pmic-eusb2.c#L356
I'd stick to this approach too. Program everything in init, then
program mode-dependent regs in set_mode.
>
> > BTW, is there a real need to override those for the platform you are
> > working on? Could you please provide some details, maybe in the cover
> > letter.
>
> I am not quite sure, recently, I expirenced mode switching failure,
> when I `echo device > /sys/kernel/debug/usb/a600000.usb/mode`, Ethernet
> Gadget wouldn't work again, my desktop can't connect to it.
Do you have at least a list of the properties / registers that
downstream programs on your platform? I mean, it's not infrequent that
vendor kernel is more versatile than necessary, as it is being used
during bringup / etc. I'd suggest to limit supported overrides to those
necessary for your platform (and add a comment that there were other
available).
>
> BTW, as you can see in line 356, it is most likely that overrides
> related to charging feature. I have not ported charging yet, but adding
> more overrides seems harmless, and if overriding, distinguishing which
> mode seems necessary, even if some devices use the same sequence. So I
> sent the patch.
>
> Best wishes,
> Pengyu
>
> --
> linux-phy mailing list
> linux-phy@lists.infradead.org
> https://lists.infradead.org/mailman/listinfo/linux-phy
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] phy: qualcomm: phy-qcom-eusb2-repeater: rework reg override handler
2025-04-28 16:58 ` Dmitry Baryshkov
@ 2025-04-29 9:23 ` Pengyu Luo
0 siblings, 0 replies; 8+ messages in thread
From: Pengyu Luo @ 2025-04-29 9:23 UTC (permalink / raw)
To: dmitry.baryshkov
Cc: abel.vesa, conor+dt, devicetree, kishon, krzk+dt, linux-arm-msm,
linux-kernel, linux-phy, mitltlatltl, robh, sfr, vkoul
On Tue, Apr 29, 2025 at 12:58 AM Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote:
> On Sat, Apr 26, 2025 at 04:14:23PM +0800, Pengyu Luo wrote:
> > On Sat, Apr 26, 2025 at 3:41 AM Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote:
> > > On Wed, Apr 16, 2025 at 08:02:01PM +0800, Pengyu Luo wrote:
> > > > In downstream tree, many registers need to be overridden, it varies
> > > > from devices and platforms, with these registers getting more, adding
> > > > a handler for this is helpful.
> > >
> > > It should be noted that previously all values were applied during _init
> > > phase, before checking the status etc. Now the overrides are programmed
> > > from the set_mode. Should you still program sane defaults at the init
> > > stage too?
> > >
> >
> > I think programming in set_mode is ok. When we init(dwc3_core_init), we
> > set_mode(dwc3_core_init_mode) later, please check
> > https://elixir.bootlin.com/linux/v6.14.3/source/drivers/usb/dwc3/core.c#L2287
>
> Yes, but that happens after reading status regs, etc.
>
> > Actually, in the downstream, all the things are done in init, it
> > overrides first, then masked write the deaults, finally it set_mode,
> > you can check here
> > https://github.com/OnePlusOSS/android_kernel_oneplus_sm8650/blob/oneplus/sm8650_v_15.0.0_pad_pro/drivers/usb/repeater/repeater-qti-pmic-eusb2.c#L356
>
> I'd stick to this approach too. Program everything in init, then
> program mode-dependent regs in set_mode.
>
If so, I don't mind programming the defaults in init, then programming
overrides in set_mode.
> >
> > > BTW, is there a real need to override those for the platform you are
> > > working on? Could you please provide some details, maybe in the cover
> > > letter.
> >
> > I am not quite sure, recently, I expirenced mode switching failure,
> > when I `echo device > /sys/kernel/debug/usb/a600000.usb/mode`, Ethernet
> > Gadget wouldn't work again, my desktop can't connect to it.
>
> Do you have at least a list of the properties / registers that
> downstream programs on your platform? I mean, it's not infrequent that
> vendor kernel is more versatile than necessary, as it is being used
> during bringup / etc. I'd suggest to limit supported overrides to those
> necessary for your platform (and add a comment that there were other
> available).
>
Yes, I only added the necessary overrides(0x54, and mode handling).
Please check
https://github.com/OnePlusOSS/android_kernel_modules_and_devicetree_oneplus_sm8650/blob/oneplus/sm8650_v_15.0.0_pad_pro/kernel_platform/qcom/proprietary/devicetree/oplus/oplus_misc/oplus-misc-23926.dtsi#L26
Best wishes,
Pengyu
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-04-29 9:25 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-16 12:01 [PATCH v2 0/2] phy: qualcomm: phy-qcom-eusb2-repeater: rework reg override handler Pengyu Luo
2025-04-16 12:02 ` [PATCH v2 1/2] dt-bindings: phy: qcom,snps-eusb2-repeater: Add more tuning overrides Pengyu Luo
2025-04-21 21:32 ` Rob Herring
2025-04-16 12:02 ` [PATCH v2 2/2] phy: qualcomm: phy-qcom-eusb2-repeater: rework reg override handler Pengyu Luo
2025-04-25 19:41 ` Dmitry Baryshkov
2025-04-26 8:14 ` Pengyu Luo
2025-04-28 16:58 ` Dmitry Baryshkov
2025-04-29 9:23 ` Pengyu Luo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).