* [v3 0/3] Add QCOM SNPS PHY overriding params support
@ 2022-05-08 12:12 Krishna Kurapati
2022-05-08 12:12 ` [v3 1/3] dt-bindings: phy: qcom,usb-snps-femto-v2: Add phy override params bindings Krishna Kurapati
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Krishna Kurapati @ 2022-05-08 12:12 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Andy Gross, Bjorn Andersson,
Greg Kroah-Hartman, Stephen Boyd, Doug Anderson,
Matthias Kaehlcke, Wesley Cheng
Cc: devicetree, linux-arm-msm, linux-usb, linux-kernel, linux-phy,
quic_pkondeti, quic_ppratap, quic_vpulyala, Krishna Kurapati
Added support for overriding tuning parameters in QCOM SNPS PHY
from device tree.
changes in v3:
Added support for phy tuning paramters 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 | 87 +++++++
arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 6 +
drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c | 252 ++++++++++++++++++++-
3 files changed, 343 insertions(+), 2 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 11+ messages in thread* [v3 1/3] dt-bindings: phy: qcom,usb-snps-femto-v2: Add phy override params bindings 2022-05-08 12:12 [v3 0/3] Add QCOM SNPS PHY overriding params support Krishna Kurapati @ 2022-05-08 12:12 ` Krishna Kurapati 2022-05-09 2:52 ` Pavan Kondeti 2022-05-08 12:12 ` [v3 2/3] phy: qcom-snps: Add support for overriding phy tuning parameters Krishna Kurapati 2022-05-08 12:12 ` [v3 3/3] arm64: dts: qcom: sc7280: Update SNPS Phy params for SC7280 IDP device Krishna Kurapati 2 siblings, 1 reply; 11+ messages in thread From: Krishna Kurapati @ 2022-05-08 12:12 UTC (permalink / raw) To: Krzysztof Kozlowski, Rob Herring, Andy Gross, Bjorn Andersson, Greg Kroah-Hartman, Stephen Boyd, Doug Anderson, Matthias Kaehlcke, Wesley Cheng Cc: devicetree, linux-arm-msm, linux-usb, linux-kernel, linux-phy, quic_pkondeti, quic_ppratap, quic_vpulyala, Sandeep Maheswaram, Krishna Kurapati From: Sandeep Maheswaram <quic_c_sanm@quicinc.com> Add device tree bindings for SNPS phy tuning parameters. Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> --- .../bindings/phy/qcom,usb-snps-femto-v2.yaml | 87 ++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml index 1ce251d..6c2ecdd 100644 --- a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml +++ b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml @@ -53,6 +53,93 @@ properties: vdda33-supply: description: phandle to the regulator 3.3V supply node. + qcom,hs-disconnect-bps: + $ref: /schemas/types.yaml#/definitions/int32 + description: + This adjusts the voltage level for the threshold used to + detect a disconnect event at the host. Possible values are. + The values defined are in multiples of basis points (1bp = 0.01%). + The hardware accepts only discrete values. The value closest to the + provided input will be chosen as the override value for this param. + + qcom,squelch-detector-bps: + $ref: /schemas/types.yaml#/definitions/int32 + description: + This adjusts the voltage level for the threshold used to + detect valid high-speed data. + The values defined are in multiples of basis points (1bp = 0.01%). + The hardware accepts only discrete values. The value closest to the + provided input will be chosen as the override value for this param. + + qcom,hs-amplitude-bps: + $ref: /schemas/types.yaml#/definitions/int32 + description: + This adjusts the high-speed DC level voltage. + The values defined are in multiples of basis points (1bp = 0.01%). + The hardware accepts only discrete values. The value closest to the + provided input will be chosen as the override value for this param. + + qcom,pre-emphasis-duration-bps: + $ref: /schemas/types.yaml#/definitions/int32 + description: + This signal controls the duration for which the + HS pre-emphasis current is sourced onto DP<#> or DM<#>. + The HS Transmitter pre-emphasis duration is defined in terms of + unit amounts. One unit of pre-emphasis duration is approximately + 650 ps and is defined as 1X pre-emphasis duration. + The values defined are in multiples of basis points (1bp = 0.01%). + The hardware accepts only discrete values. The value closest to the + provided input will be chosen as the override value for this param. + + qcom,pre-emphasis-amplitude-bps: + $ref: /schemas/types.yaml#/definitions/int32 + description: + This signal controls the amount of current sourced to + DP<#> and DM<#> after a J-to-K or K-to-J transition. + The HS Transmitter pre-emphasis current is defined in terms of unit + amounts. One unit amount is approximately 2 mA and is defined as + 1X pre-emphasis current. + The values defined are in multiples of basis points (1bp = 0.01%). + The hardware accepts only discrete values. The value closest to the + provided input will be chosen as the override value for this param. + + qcom,hs-rise-fall-time-bps: + $ref: /schemas/types.yaml#/definitions/int32 + description: + This adjusts the rise/fall times of the high-speed waveform. + The values defined are in multiples of basis points (1bp = 0.01%). + The hardware accepts only discrete values. The value closest to the + provided input will be chosen as the override value for this param. + + qcom,hs-crossover-voltage: + $ref: /schemas/types.yaml#/definitions/int32 + description: + This adjusts the voltage at which the DP<#> and DM<#> + signals cross while transmitting in HS mode. + The values defined are in milli volts. The hardware accepts only + discrete values. The value closest to the provided input will be + chosen as the override value for this param. + + qcom,hs-output-impedance: + $ref: /schemas/types.yaml#/definitions/int32 + description: + In some applications, there can be significant series resistance + on the D+ and D- paths between the transceiver and cable. This adjusts + the driver source impedance to compensate for added series + resistance on the USB. The values defined are in milliohms. + The hardware accepts only discrete values. The value closest to the + provided input will be chosen as the override value for this param. + + qcom,ls-fs-output-impedance-bps: + $ref: /schemas/types.yaml#/definitions/int32 + description: + This adjusts the low- and full-speed single-ended source + impedance while driving high. The following adjustment values are based + on nominal process, voltage, and temperature. + The values defined are in multiples of basis points (1bp = 0.01%). + The hardware accepts only discrete values. The value closest to the + provided input will be chosen as the override value for this param. + required: - compatible - reg -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [v3 1/3] dt-bindings: phy: qcom,usb-snps-femto-v2: Add phy override params bindings 2022-05-08 12:12 ` [v3 1/3] dt-bindings: phy: qcom,usb-snps-femto-v2: Add phy override params bindings Krishna Kurapati @ 2022-05-09 2:52 ` Pavan Kondeti 0 siblings, 0 replies; 11+ messages in thread From: Pavan Kondeti @ 2022-05-09 2:52 UTC (permalink / raw) To: Krishna Kurapati Cc: Krzysztof Kozlowski, Rob Herring, Andy Gross, Bjorn Andersson, Greg Kroah-Hartman, Stephen Boyd, Doug Anderson, Matthias Kaehlcke, Wesley Cheng, devicetree, linux-arm-msm, linux-usb, linux-kernel, linux-phy, quic_pkondeti, quic_ppratap, quic_vpulyala, Sandeep Maheswaram Hi Krishna, On Sun, May 08, 2022 at 05:42:25PM +0530, Krishna Kurapati wrote: > From: Sandeep Maheswaram <quic_c_sanm@quicinc.com> > > Add device tree bindings for SNPS phy tuning parameters. > > Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com> > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > --- > .../bindings/phy/qcom,usb-snps-femto-v2.yaml | 87 ++++++++++++++++++++++ > 1 file changed, 87 insertions(+) > > diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml > index 1ce251d..6c2ecdd 100644 > --- a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml > +++ b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml > @@ -53,6 +53,93 @@ properties: > vdda33-supply: > description: phandle to the regulator 3.3V supply node. > <snip> > + > + qcom,hs-rise-fall-time-bps: > + $ref: /schemas/types.yaml#/definitions/int32 > + description: > + This adjusts the rise/fall times of the high-speed waveform. > + The values defined are in multiples of basis points (1bp = 0.01%). > + The hardware accepts only discrete values. The value closest to the > + provided input will be chosen as the override value for this param. > + > + qcom,hs-crossover-voltage: > + $ref: /schemas/types.yaml#/definitions/int32 > + description: > + This adjusts the voltage at which the DP<#> and DM<#> > + signals cross while transmitting in HS mode. > + The values defined are in milli volts. The hardware accepts only > + discrete values. The value closest to the provided input will be > + chosen as the override value for this param. > + > + qcom,hs-output-impedance: > + $ref: /schemas/types.yaml#/definitions/int32 > + description: > + In some applications, there can be significant series resistance > + on the D+ and D- paths between the transceiver and cable. This adjusts > + the driver source impedance to compensate for added series > + resistance on the USB. The values defined are in milliohms. %s/milliohms/mill ohms > + The hardware accepts only discrete values. The value closest to the > + provided input will be chosen as the override value for this param. > + What are the units for HS crossover voltage and output impedence? can you add units as a suffix like other parameters? Thanks, Pavan ^ permalink raw reply [flat|nested] 11+ messages in thread
* [v3 2/3] phy: qcom-snps: Add support for overriding phy tuning parameters 2022-05-08 12:12 [v3 0/3] Add QCOM SNPS PHY overriding params support Krishna Kurapati 2022-05-08 12:12 ` [v3 1/3] dt-bindings: phy: qcom,usb-snps-femto-v2: Add phy override params bindings Krishna Kurapati @ 2022-05-08 12:12 ` Krishna Kurapati 2022-05-08 13:22 ` kernel test robot 2022-05-09 3:18 ` Pavan Kondeti 2022-05-08 12:12 ` [v3 3/3] arm64: dts: qcom: sc7280: Update SNPS Phy params for SC7280 IDP device Krishna Kurapati 2 siblings, 2 replies; 11+ messages in thread From: Krishna Kurapati @ 2022-05-08 12:12 UTC (permalink / raw) To: Krzysztof Kozlowski, Rob Herring, Andy Gross, Bjorn Andersson, Greg Kroah-Hartman, Stephen Boyd, Doug Anderson, Matthias Kaehlcke, Wesley Cheng Cc: devicetree, linux-arm-msm, linux-usb, linux-kernel, linux-phy, quic_pkondeti, quic_ppratap, quic_vpulyala, Krishna Kurapati Add support for overriding electrical signal tuning parameters for SNPS HS Phy. Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> --- drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c | 252 +++++++++++++++++++++++++- 1 file changed, 250 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..bed2c90 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,6 +66,16 @@ #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", }; @@ -173,10 +189,195 @@ static int qcom_snps_hsphy_set_mode(struct phy *phy, enum phy_mode mode, return 0; } +struct override_param { + s32 value; + u8 reg; +}; + +#define OVERRIDE_PARAM(bps, val)\ +{ \ + .value = bps, \ + .reg = val, \ +} + +struct override_param_map { + struct override_param *param_table; + u8 table_size; + u8 reg_offset; + u8 param_mask; +}; + +#define OVERRIDE_PARAM_MAP(table, numElements, offset, mask) \ +{ \ + .param_table = table, \ + .table_size = numElements, \ + .reg_offset = offset, \ + .param_mask = mask, \ +} + +static const char *phy_seq_props[] = +{ + "qcom,hs-rise-fall-time-bps", + "qcom,squelch-detector-bps", + "qcom,preemphasis-duration", + "qcom,preemphasis-amplitude", + "qcom,hs-disconnect-bps", + "qcom,hs-amplitude-bps", + "qcom,hs-crossover-voltage", + "qcom,hs-output-impedance", + "qcom,ls-fs-output-impedance-bps", +}; + +static struct override_param hs_rise_fall_time_sc7280[] = { + OVERRIDE_PARAM(-4100, 3), + OVERRIDE_PARAM(0, 2), + OVERRIDE_PARAM(2810, 1), + OVERRIDE_PARAM(5430, 0), +}; + +static struct override_param squelch_det_threshold_sc7280[] = { + OVERRIDE_PARAM(-2090, 7), + OVERRIDE_PARAM(-1560, 6), + OVERRIDE_PARAM(-1030, 5), + OVERRIDE_PARAM(-530, 4), + OVERRIDE_PARAM(0, 3), + OVERRIDE_PARAM(530, 2), + OVERRIDE_PARAM(1060, 1), + OVERRIDE_PARAM(1590, 0), +}; + +static struct override_param hs_disconnect_sc7280[] = { + OVERRIDE_PARAM(-272, 0), + OVERRIDE_PARAM(0, 1), + OVERRIDE_PARAM(317, 2), + OVERRIDE_PARAM(630, 3), + OVERRIDE_PARAM(973, 4), + OVERRIDE_PARAM(1332, 5), + OVERRIDE_PARAM(1743, 6), + OVERRIDE_PARAM(2156, 7), +}; + +static struct override_param hs_amplitude_sc7280[] = { + OVERRIDE_PARAM(-660, 0), + OVERRIDE_PARAM(-440, 1), + OVERRIDE_PARAM(-220, 2), + OVERRIDE_PARAM(0, 3), + OVERRIDE_PARAM(230, 4), + OVERRIDE_PARAM(440, 5), + OVERRIDE_PARAM(650, 6), + OVERRIDE_PARAM(890, 7), + OVERRIDE_PARAM(1110, 8), + OVERRIDE_PARAM(1330, 9), + OVERRIDE_PARAM(1560, 10), + OVERRIDE_PARAM(1780, 11), + OVERRIDE_PARAM(2000, 12), + OVERRIDE_PARAM(2220, 13), + OVERRIDE_PARAM(2430, 14), + OVERRIDE_PARAM(2670, 15), +}; + +static struct override_param preemphasis_duration_sc7280[] = { + OVERRIDE_PARAM(100, 1), + OVERRIDE_PARAM(200, 0), +}; + +static struct override_param preemphasis_amplitude_sc7280[] = { + OVERRIDE_PARAM(100, 1), + OVERRIDE_PARAM(200, 2), + OVERRIDE_PARAM(300, 3), + OVERRIDE_PARAM(400, 0), +}; + +static struct override_param hs_crossover_voltage_sc7280[] = { + OVERRIDE_PARAM(-31, 1), + OVERRIDE_PARAM(28, 2), + OVERRIDE_PARAM(0, 3), +}; + +static struct override_param hs_output_impedance_sc7280[] = { + OVERRIDE_PARAM(-2300, 3), + OVERRIDE_PARAM(0, 2), + OVERRIDE_PARAM(2600, 1), + OVERRIDE_PARAM(6100, 0), +}; + +static struct override_param ls_fs_output_impedance_sc7280[] = { + OVERRIDE_PARAM(-1053, 15), + OVERRIDE_PARAM(-557, 7), + OVERRIDE_PARAM(0, 3), + OVERRIDE_PARAM(612, 1), + OVERRIDE_PARAM(1310, 0), +}; + +struct override_param_map sc7280_idp[] = { + OVERRIDE_PARAM_MAP( + hs_rise_fall_time_sc7280, + ARRAY_SIZE(hs_rise_fall_time_sc7280), + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X2, + HS_RISE_FALL_MASK), + + OVERRIDE_PARAM_MAP( + squelch_det_threshold_sc7280, + ARRAY_SIZE(squelch_det_threshold_sc7280), + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X0, + SQUELCH_DETECTOR_MASK), + + OVERRIDE_PARAM_MAP( + preemphasis_duration_sc7280, + ARRAY_SIZE(preemphasis_duration_sc7280), + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X1, + PREEMPHASIS_DURATION_MASK), + + OVERRIDE_PARAM_MAP( + preemphasis_amplitude_sc7280, + ARRAY_SIZE(preemphasis_amplitude_sc7280), + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X1, + PREEMPHASIS_AMPLITUDE_MASK), + + OVERRIDE_PARAM_MAP( + hs_disconnect_sc7280, + ARRAY_SIZE(hs_disconnect_sc7280), + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X0, + HS_DISCONNECT_MASK), + + OVERRIDE_PARAM_MAP( + hs_amplitude_sc7280, + ARRAY_SIZE(hs_amplitude_sc7280), + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X1, + HS_AMPLITUDE_MASK), + + OVERRIDE_PARAM_MAP( + hs_crossover_voltage_sc7280, + ARRAY_SIZE(hs_crossover_voltage_sc7280), + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X2, + HS_CROSSOVER_VOLTAGE_MASK), + + OVERRIDE_PARAM_MAP( + hs_output_impedance_sc7280, + ARRAY_SIZE(hs_output_impedance_sc7280), + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X2, + HS_OUTPUT_IMPEDANCE_MASK), + + OVERRIDE_PARAM_MAP( + 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), +}; + +struct phy_override_seq { + bool need_update; + u8 offset; + u8 value; + u8 mask; +}; + +struct phy_override_seq update_seq_cfg[ARRAY_SIZE(phy_seq_props)]; + 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 +424,12 @@ 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(update_seq_cfg); i++) { + if (update_seq_cfg[i].need_update) + qcom_snps_hsphy_write_mask(hsphy->base, update_seq_cfg[i].offset, + update_seq_cfg[i].mask, update_seq_cfg[i].value); + } + qcom_snps_hsphy_write_mask(hsphy->base, USB2_PHY_USB_PHY_HS_PHY_CTRL_COMMON2, VREGBYPASS, VREGBYPASS); @@ -280,7 +487,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_idp + }, { .compatible = "qcom,usb-snps-femto-v2-phy", }, { } }; @@ -291,6 +501,43 @@ static const struct dev_pm_ops qcom_snps_hsphy_pm_ops = { qcom_snps_hsphy_runtime_resume, NULL) }; +static void hsphy_override_param_update_val(const struct override_param_map map, + s32 dt_val, struct phy_override_seq *seq_entry) +{ + int i; + + /* + * We prepare the param table for each param in increasing order + * of dt values. So we need to iterate over the list once to get + * the required 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 << __ffs(map.param_mask); +} + +static void phy_read_param_override_seq(struct device *dev) +{ + struct device_node *node = dev->of_node; + s32 val; + int ret, i; + struct qcom_snps_hsphy *hsphy; + struct override_param_map *cfg = (struct override_param_map* ) of_device_get_match_data(dev); + hsphy = dev_get_drvdata(dev); + + for (i = 0; i < ARRAY_SIZE(phy_seq_props); i++) { + ret = of_property_read_s32(node, phy_seq_props[i], &val); + if (!ret) + hsphy_override_param_update_val(cfg[i], val, &update_seq_cfg[i]); + } +} + static int qcom_snps_hsphy_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -352,6 +599,7 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev) dev_set_drvdata(dev, hsphy); phy_set_drvdata(generic_phy, hsphy); + phy_read_param_override_seq(dev); phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate); if (!IS_ERR(phy_provider)) -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [v3 2/3] phy: qcom-snps: Add support for overriding phy tuning parameters 2022-05-08 12:12 ` [v3 2/3] phy: qcom-snps: Add support for overriding phy tuning parameters Krishna Kurapati @ 2022-05-08 13:22 ` kernel test robot 2022-05-09 3:18 ` Pavan Kondeti 1 sibling, 0 replies; 11+ messages in thread From: kernel test robot @ 2022-05-08 13:22 UTC (permalink / raw) To: Krishna Kurapati, Krzysztof Kozlowski, Rob Herring, Andy Gross, Bjorn Andersson, Greg Kroah-Hartman, Stephen Boyd, Doug Anderson, Matthias Kaehlcke, Wesley Cheng Cc: kbuild-all, devicetree, linux-arm-msm, linux-usb, linux-kernel, linux-phy, quic_pkondeti, quic_ppratap, quic_vpulyala, Krishna Kurapati Hi Krishna, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on robh/for-next] [also build test WARNING on krzk/for-next linus/master v5.18-rc5 next-20220506] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/intel-lab-lkp/linux/commits/Krishna-Kurapati/Add-QCOM-SNPS-PHY-overriding-params-support/20220508-201506 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20220508/202205082118.cd3B5WOH-lkp@intel.com/config) compiler: m68k-linux-gcc (GCC) 11.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/4f3771418951e9e3fcb6357959dbd668cdb54fb1 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Krishna-Kurapati/Add-QCOM-SNPS-PHY-overriding-params-support/20220508-201506 git checkout 4f3771418951e9e3fcb6357959dbd668cdb54fb1 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/phy/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c: In function 'phy_read_param_override_seq': >> drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c:530:33: warning: variable 'hsphy' set but not used [-Wunused-but-set-variable] 530 | struct qcom_snps_hsphy *hsphy; | ^~~~~ vim +/hsphy +530 drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c 524 525 static void phy_read_param_override_seq(struct device *dev) 526 { 527 struct device_node *node = dev->of_node; 528 s32 val; 529 int ret, i; > 530 struct qcom_snps_hsphy *hsphy; 531 struct override_param_map *cfg = (struct override_param_map* ) of_device_get_match_data(dev); 532 hsphy = dev_get_drvdata(dev); 533 534 for (i = 0; i < ARRAY_SIZE(phy_seq_props); i++) { 535 ret = of_property_read_s32(node, phy_seq_props[i], &val); 536 if (!ret) 537 hsphy_override_param_update_val(cfg[i], val, &update_seq_cfg[i]); 538 } 539 } 540 -- 0-DAY CI Kernel Test Service https://01.org/lkp ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [v3 2/3] phy: qcom-snps: Add support for overriding phy tuning parameters 2022-05-08 12:12 ` [v3 2/3] phy: qcom-snps: Add support for overriding phy tuning parameters Krishna Kurapati 2022-05-08 13:22 ` kernel test robot @ 2022-05-09 3:18 ` Pavan Kondeti 2022-05-10 17:34 ` Krishna Kurapati PSSNV 1 sibling, 1 reply; 11+ messages in thread From: Pavan Kondeti @ 2022-05-09 3:18 UTC (permalink / raw) To: Krishna Kurapati Cc: Krzysztof Kozlowski, Rob Herring, Andy Gross, Bjorn Andersson, Greg Kroah-Hartman, Stephen Boyd, Doug Anderson, Matthias Kaehlcke, Wesley Cheng, devicetree, linux-arm-msm, linux-usb, linux-kernel, linux-phy, quic_pkondeti, quic_ppratap, quic_vpulyala, Vinod Koul Hi Krishna, On Sun, May 08, 2022 at 05:42:26PM +0530, Krishna Kurapati wrote: > Add support for overriding electrical signal tuning parameters for > SNPS HS Phy. > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > --- > drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c | 252 +++++++++++++++++++++++++- > 1 file changed, 250 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..bed2c90 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,6 +66,16 @@ > #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) > + LGTM. All these definitions are matching with the databook. > static const char * const qcom_snps_hsphy_vreg_names[] = { > "vdda-pll", "vdda33", "vdda18", > }; > @@ -173,10 +189,195 @@ static int qcom_snps_hsphy_set_mode(struct phy *phy, enum phy_mode mode, > return 0; > } > > +struct override_param { > + s32 value; > + u8 reg; > +}; > + > +#define OVERRIDE_PARAM(bps, val)\ > +{ \ > + .value = bps, \ > + .reg = val, \ > +} > + > +struct override_param_map { > + struct override_param *param_table; > + u8 table_size; > + u8 reg_offset; > + u8 param_mask; > +}; > + > +#define OVERRIDE_PARAM_MAP(table, numElements, offset, mask) \ No camelcase please. > +{ \ > + .param_table = table, \ > + .table_size = numElements, \ > + .reg_offset = offset, \ > + .param_mask = mask, \ > +} > + > +static const char *phy_seq_props[] = > +{ > + "qcom,hs-rise-fall-time-bps", > + "qcom,squelch-detector-bps", > + "qcom,preemphasis-duration", > + "qcom,preemphasis-amplitude", What are the units for pre-emphasis duration and amplitude? Can you add the units as suffix? > + "qcom,hs-disconnect-bps", > + "qcom,hs-amplitude-bps", > + "qcom,hs-crossover-voltage", > + "qcom,hs-output-impedance", What are the units for these above two parameters? > + "qcom,ls-fs-output-impedance-bps", > +}; > + > +static struct override_param hs_rise_fall_time_sc7280[] = { > + OVERRIDE_PARAM(-4100, 3), > + OVERRIDE_PARAM(0, 2), > + OVERRIDE_PARAM(2810, 1), > + OVERRIDE_PARAM(5430, 0), > +}; > + > +static struct override_param squelch_det_threshold_sc7280[] = { > + OVERRIDE_PARAM(-2090, 7), > + OVERRIDE_PARAM(-1560, 6), > + OVERRIDE_PARAM(-1030, 5), > + OVERRIDE_PARAM(-530, 4), > + OVERRIDE_PARAM(0, 3), > + OVERRIDE_PARAM(530, 2), > + OVERRIDE_PARAM(1060, 1), > + OVERRIDE_PARAM(1590, 0), > +}; > + > +static struct override_param hs_disconnect_sc7280[] = { > + OVERRIDE_PARAM(-272, 0), > + OVERRIDE_PARAM(0, 1), > + OVERRIDE_PARAM(317, 2), > + OVERRIDE_PARAM(630, 3), > + OVERRIDE_PARAM(973, 4), > + OVERRIDE_PARAM(1332, 5), > + OVERRIDE_PARAM(1743, 6), > + OVERRIDE_PARAM(2156, 7), > +}; > + > +static struct override_param hs_amplitude_sc7280[] = { > + OVERRIDE_PARAM(-660, 0), > + OVERRIDE_PARAM(-440, 1), > + OVERRIDE_PARAM(-220, 2), > + OVERRIDE_PARAM(0, 3), > + OVERRIDE_PARAM(230, 4), > + OVERRIDE_PARAM(440, 5), > + OVERRIDE_PARAM(650, 6), > + OVERRIDE_PARAM(890, 7), > + OVERRIDE_PARAM(1110, 8), > + OVERRIDE_PARAM(1330, 9), > + OVERRIDE_PARAM(1560, 10), > + OVERRIDE_PARAM(1780, 11), > + OVERRIDE_PARAM(2000, 12), > + OVERRIDE_PARAM(2220, 13), > + OVERRIDE_PARAM(2430, 14), > + OVERRIDE_PARAM(2670, 15), > +}; > + > +static struct override_param preemphasis_duration_sc7280[] = { > + OVERRIDE_PARAM(100, 1), > + OVERRIDE_PARAM(200, 0), > +}; > + > +static struct override_param preemphasis_amplitude_sc7280[] = { > + OVERRIDE_PARAM(100, 1), > + OVERRIDE_PARAM(200, 2), > + OVERRIDE_PARAM(300, 3), > + OVERRIDE_PARAM(400, 0), > +}; > + > +static struct override_param hs_crossover_voltage_sc7280[] = { > + OVERRIDE_PARAM(-31, 1), > + OVERRIDE_PARAM(28, 2), > + OVERRIDE_PARAM(0, 3), > +}; > + > +static struct override_param hs_output_impedance_sc7280[] = { > + OVERRIDE_PARAM(-2300, 3), > + OVERRIDE_PARAM(0, 2), > + OVERRIDE_PARAM(2600, 1), > + OVERRIDE_PARAM(6100, 0), > +}; > + > +static struct override_param ls_fs_output_impedance_sc7280[] = { > + OVERRIDE_PARAM(-1053, 15), > + OVERRIDE_PARAM(-557, 7), > + OVERRIDE_PARAM(0, 3), > + OVERRIDE_PARAM(612, 1), > + OVERRIDE_PARAM(1310, 0), > +}; > + LGTM. I have cross checked with the data book. > +struct override_param_map sc7280_idp[] = { > + OVERRIDE_PARAM_MAP( > + hs_rise_fall_time_sc7280, > + ARRAY_SIZE(hs_rise_fall_time_sc7280), > + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X2, > + HS_RISE_FALL_MASK), > + The bitmask definitions like HS_DISCONNECT_MASK are defined as they appear in the databook. However, this struct is defined out of order. Can you defined X0, X1, X2 and X3 registers in an order. Easier to check/compare against data book. > + OVERRIDE_PARAM_MAP( > + squelch_det_threshold_sc7280, > + ARRAY_SIZE(squelch_det_threshold_sc7280), > + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X0, > + SQUELCH_DETECTOR_MASK), > + > + OVERRIDE_PARAM_MAP( > + preemphasis_duration_sc7280, > + ARRAY_SIZE(preemphasis_duration_sc7280), > + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X1, > + PREEMPHASIS_DURATION_MASK), > + > + OVERRIDE_PARAM_MAP( > + preemphasis_amplitude_sc7280, > + ARRAY_SIZE(preemphasis_amplitude_sc7280), > + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X1, > + PREEMPHASIS_AMPLITUDE_MASK), > + > + OVERRIDE_PARAM_MAP( > + hs_disconnect_sc7280, > + ARRAY_SIZE(hs_disconnect_sc7280), > + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X0, > + HS_DISCONNECT_MASK), > + > + OVERRIDE_PARAM_MAP( > + hs_amplitude_sc7280, > + ARRAY_SIZE(hs_amplitude_sc7280), > + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X1, > + HS_AMPLITUDE_MASK), > + > + OVERRIDE_PARAM_MAP( > + hs_crossover_voltage_sc7280, > + ARRAY_SIZE(hs_crossover_voltage_sc7280), > + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X2, > + HS_CROSSOVER_VOLTAGE_MASK), > + > + OVERRIDE_PARAM_MAP( > + hs_output_impedance_sc7280, > + ARRAY_SIZE(hs_output_impedance_sc7280), > + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X2, > + HS_OUTPUT_IMPEDANCE_MASK), > + > + OVERRIDE_PARAM_MAP( > + 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), > +}; > + > +struct phy_override_seq { > + bool need_update; > + u8 offset; > + u8 value; > + u8 mask; > +}; > + > +struct phy_override_seq update_seq_cfg[ARRAY_SIZE(phy_seq_props)]; This needs to be wrapped up in qcom_snps_hsphy structure so that we can support override sequence per phy instance. The electrical complaince tuning is heavily influenced by the board design and where the ports are located etc. So it is possible that two instances of the phy can use different tunning parameters. > + > 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 +424,12 @@ 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(update_seq_cfg); i++) { > + if (update_seq_cfg[i].need_update) > + qcom_snps_hsphy_write_mask(hsphy->base, update_seq_cfg[i].offset, > + update_seq_cfg[i].mask, update_seq_cfg[i].value); > + } > + > qcom_snps_hsphy_write_mask(hsphy->base, > USB2_PHY_USB_PHY_HS_PHY_CTRL_COMMON2, > VREGBYPASS, VREGBYPASS); > @@ -280,7 +487,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_idp > + }, can you check the indentation please? > { .compatible = "qcom,usb-snps-femto-v2-phy", }, > { } > }; > @@ -291,6 +501,43 @@ static const struct dev_pm_ops qcom_snps_hsphy_pm_ops = { > qcom_snps_hsphy_runtime_resume, NULL) > }; > > +static void hsphy_override_param_update_val(const struct override_param_map map, > + s32 dt_val, struct phy_override_seq *seq_entry) > +{ > + int i; > + > + /* > + * We prepare the param table for each param in increasing order > + * of dt values. So we need to iterate over the list once to get > + * the required register value. > + */ > + for (i = 0 ; i < map.table_size-1; i++) { > + if (map.param_table[i].value >= dt_val) > + break; > + } The comment is confusing. param_table[] already has values in the increasing order. we have to find *closet* match here. To keep it simple, we select the entry that has equal or the next highest (round up) value. clarify the comment. > + > + 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 << __ffs(map.param_mask); > +} > + > +static void phy_read_param_override_seq(struct device *dev) > +{ > + struct device_node *node = dev->of_node; > + s32 val; > + int ret, i; > + struct qcom_snps_hsphy *hsphy; > + struct override_param_map *cfg = (struct override_param_map* ) of_device_get_match_data(dev); nit pick. No explicit type case is needed here. of_device_get_match_data() returns a void pointer. so it can be written as struct override_param_map *cfg = of_device_get_match_data(dev); > + hsphy = dev_get_drvdata(dev); > + > + for (i = 0; i < ARRAY_SIZE(phy_seq_props); i++) { > + ret = of_property_read_s32(node, phy_seq_props[i], &val); > + if (!ret) > + hsphy_override_param_update_val(cfg[i], val, &update_seq_cfg[i]); > + } > +} > + Can we add a dev_dbg here to aid the debugging? All the functions defined in this driver has qcom_snps_hsphy_xxxx convention. Lets follow the same. > static int qcom_snps_hsphy_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > @@ -352,6 +599,7 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev) > > dev_set_drvdata(dev, hsphy); > phy_set_drvdata(generic_phy, hsphy); > + phy_read_param_override_seq(dev); > > phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate); > if (!IS_ERR(phy_provider)) > -- > 2.7.4 > Bjorn/Vinod, I have reviewed this patch on our internal review system. The basic design looks good to me. Can you please review the patch and provide your inputs. Thanks, Pavan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [v3 2/3] phy: qcom-snps: Add support for overriding phy tuning parameters 2022-05-09 3:18 ` Pavan Kondeti @ 2022-05-10 17:34 ` Krishna Kurapati PSSNV 0 siblings, 0 replies; 11+ messages in thread From: Krishna Kurapati PSSNV @ 2022-05-10 17:34 UTC (permalink / raw) To: Pavan Kondeti Cc: Krzysztof Kozlowski, Rob Herring, Andy Gross, Bjorn Andersson, Greg Kroah-Hartman, Stephen Boyd, Doug Anderson, Matthias Kaehlcke, Wesley Cheng, devicetree, linux-arm-msm, linux-usb, linux-kernel, linux-phy, quic_ppratap, quic_vpulyala, Vinod Koul On 5/9/2022 8:48 AM, Pavan Kondeti wrote: > Hi Krishna, > > On Sun, May 08, 2022 at 05:42:26PM +0530, Krishna Kurapati wrote: >> Add support for overriding electrical signal tuning parameters for >> SNPS HS Phy. >> >> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> >> --- >> drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c | 252 +++++++++++++++++++++++++- >> 1 file changed, 250 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..bed2c90 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,6 +66,16 @@ >> #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) >> + > LGTM. All these definitions are matching with the databook. > >> static const char * const qcom_snps_hsphy_vreg_names[] = { >> "vdda-pll", "vdda33", "vdda18", >> }; >> @@ -173,10 +189,195 @@ static int qcom_snps_hsphy_set_mode(struct phy *phy, enum phy_mode mode, >> return 0; >> } >> >> +struct override_param { >> + s32 value; >> + u8 reg; >> +}; >> + >> +#define OVERRIDE_PARAM(bps, val)\ >> +{ \ >> + .value = bps, \ >> + .reg = val, \ >> +} >> + >> +struct override_param_map { >> + struct override_param *param_table; >> + u8 table_size; >> + u8 reg_offset; >> + u8 param_mask; >> +}; >> + >> +#define OVERRIDE_PARAM_MAP(table, numElements, offset, mask) \ > No camelcase please. > >> +{ \ >> + .param_table = table, \ >> + .table_size = numElements, \ >> + .reg_offset = offset, \ >> + .param_mask = mask, \ >> +} >> + >> +static const char *phy_seq_props[] = >> +{ >> + "qcom,hs-rise-fall-time-bps", >> + "qcom,squelch-detector-bps", >> + "qcom,preemphasis-duration", >> + "qcom,preemphasis-amplitude", > What are the units for pre-emphasis duration and amplitude? Can you add the > units as suffix? My bad, its bps. Will add it in next patchset. >> + "qcom,hs-disconnect-bps", >> + "qcom,hs-amplitude-bps", >> + "qcom,hs-crossover-voltage", >> + "qcom,hs-output-impedance", > What are the units for these above two parameters? mV and mohm, Will add them as suffix. >> + "qcom,ls-fs-output-impedance-bps", >> +}; >> + >> +static struct override_param hs_rise_fall_time_sc7280[] = { >> + OVERRIDE_PARAM(-4100, 3), >> + OVERRIDE_PARAM(0, 2), >> + OVERRIDE_PARAM(2810, 1), >> + OVERRIDE_PARAM(5430, 0), >> +}; >> + >> +static struct override_param squelch_det_threshold_sc7280[] = { >> + OVERRIDE_PARAM(-2090, 7), >> + OVERRIDE_PARAM(-1560, 6), >> + OVERRIDE_PARAM(-1030, 5), >> + OVERRIDE_PARAM(-530, 4), >> + OVERRIDE_PARAM(0, 3), >> + OVERRIDE_PARAM(530, 2), >> + OVERRIDE_PARAM(1060, 1), >> + OVERRIDE_PARAM(1590, 0), >> +}; >> + >> +static struct override_param hs_disconnect_sc7280[] = { >> + OVERRIDE_PARAM(-272, 0), >> + OVERRIDE_PARAM(0, 1), >> + OVERRIDE_PARAM(317, 2), >> + OVERRIDE_PARAM(630, 3), >> + OVERRIDE_PARAM(973, 4), >> + OVERRIDE_PARAM(1332, 5), >> + OVERRIDE_PARAM(1743, 6), >> + OVERRIDE_PARAM(2156, 7), >> +}; >> + >> +static struct override_param hs_amplitude_sc7280[] = { >> + OVERRIDE_PARAM(-660, 0), >> + OVERRIDE_PARAM(-440, 1), >> + OVERRIDE_PARAM(-220, 2), >> + OVERRIDE_PARAM(0, 3), >> + OVERRIDE_PARAM(230, 4), >> + OVERRIDE_PARAM(440, 5), >> + OVERRIDE_PARAM(650, 6), >> + OVERRIDE_PARAM(890, 7), >> + OVERRIDE_PARAM(1110, 8), >> + OVERRIDE_PARAM(1330, 9), >> + OVERRIDE_PARAM(1560, 10), >> + OVERRIDE_PARAM(1780, 11), >> + OVERRIDE_PARAM(2000, 12), >> + OVERRIDE_PARAM(2220, 13), >> + OVERRIDE_PARAM(2430, 14), >> + OVERRIDE_PARAM(2670, 15), >> +}; >> + >> +static struct override_param preemphasis_duration_sc7280[] = { >> + OVERRIDE_PARAM(100, 1), >> + OVERRIDE_PARAM(200, 0), >> +}; >> + >> +static struct override_param preemphasis_amplitude_sc7280[] = { >> + OVERRIDE_PARAM(100, 1), >> + OVERRIDE_PARAM(200, 2), >> + OVERRIDE_PARAM(300, 3), >> + OVERRIDE_PARAM(400, 0), >> +}; >> + >> +static struct override_param hs_crossover_voltage_sc7280[] = { >> + OVERRIDE_PARAM(-31, 1), >> + OVERRIDE_PARAM(28, 2), >> + OVERRIDE_PARAM(0, 3), >> +}; >> + >> +static struct override_param hs_output_impedance_sc7280[] = { >> + OVERRIDE_PARAM(-2300, 3), >> + OVERRIDE_PARAM(0, 2), >> + OVERRIDE_PARAM(2600, 1), >> + OVERRIDE_PARAM(6100, 0), >> +}; >> + >> +static struct override_param ls_fs_output_impedance_sc7280[] = { >> + OVERRIDE_PARAM(-1053, 15), >> + OVERRIDE_PARAM(-557, 7), >> + OVERRIDE_PARAM(0, 3), >> + OVERRIDE_PARAM(612, 1), >> + OVERRIDE_PARAM(1310, 0), >> +}; >> + > LGTM. I have cross checked with the data book. > >> +struct override_param_map sc7280_idp[] = { >> + OVERRIDE_PARAM_MAP( >> + hs_rise_fall_time_sc7280, >> + ARRAY_SIZE(hs_rise_fall_time_sc7280), >> + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X2, >> + HS_RISE_FALL_MASK), >> + > The bitmask definitions like HS_DISCONNECT_MASK are defined as they appear > in the databook. However, this struct is defined out of order. Can you > defined X0, X1, X2 and X3 registers in an order. Easier to check/compare > against data book. Sure, Will make this change. >> + OVERRIDE_PARAM_MAP( >> + squelch_det_threshold_sc7280, >> + ARRAY_SIZE(squelch_det_threshold_sc7280), >> + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X0, >> + SQUELCH_DETECTOR_MASK), >> + >> + OVERRIDE_PARAM_MAP( >> + preemphasis_duration_sc7280, >> + ARRAY_SIZE(preemphasis_duration_sc7280), >> + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X1, >> + PREEMPHASIS_DURATION_MASK), >> + >> + OVERRIDE_PARAM_MAP( >> + preemphasis_amplitude_sc7280, >> + ARRAY_SIZE(preemphasis_amplitude_sc7280), >> + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X1, >> + PREEMPHASIS_AMPLITUDE_MASK), >> + >> + OVERRIDE_PARAM_MAP( >> + hs_disconnect_sc7280, >> + ARRAY_SIZE(hs_disconnect_sc7280), >> + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X0, >> + HS_DISCONNECT_MASK), >> + >> + OVERRIDE_PARAM_MAP( >> + hs_amplitude_sc7280, >> + ARRAY_SIZE(hs_amplitude_sc7280), >> + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X1, >> + HS_AMPLITUDE_MASK), >> + >> + OVERRIDE_PARAM_MAP( >> + hs_crossover_voltage_sc7280, >> + ARRAY_SIZE(hs_crossover_voltage_sc7280), >> + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X2, >> + HS_CROSSOVER_VOLTAGE_MASK), >> + >> + OVERRIDE_PARAM_MAP( >> + hs_output_impedance_sc7280, >> + ARRAY_SIZE(hs_output_impedance_sc7280), >> + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X2, >> + HS_OUTPUT_IMPEDANCE_MASK), >> + >> + OVERRIDE_PARAM_MAP( >> + 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), >> +}; >> + >> +struct phy_override_seq { >> + bool need_update; >> + u8 offset; >> + u8 value; >> + u8 mask; >> +}; >> + >> +struct phy_override_seq update_seq_cfg[ARRAY_SIZE(phy_seq_props)]; > This needs to be wrapped up in qcom_snps_hsphy structure so that we can > support override sequence per phy instance. The electrical complaince tuning > is heavily influenced by the board design and where the ports are located etc. > So it is possible that two instances of the phy can use different tunning > parameters. Makes sense. Thanks for the correction. >> + >> 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 +424,12 @@ 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(update_seq_cfg); i++) { >> + if (update_seq_cfg[i].need_update) >> + qcom_snps_hsphy_write_mask(hsphy->base, update_seq_cfg[i].offset, >> + update_seq_cfg[i].mask, update_seq_cfg[i].value); >> + } >> + >> qcom_snps_hsphy_write_mask(hsphy->base, >> USB2_PHY_USB_PHY_HS_PHY_CTRL_COMMON2, >> VREGBYPASS, VREGBYPASS); >> @@ -280,7 +487,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_idp >> + }, > can you check the indentation please? > >> { .compatible = "qcom,usb-snps-femto-v2-phy", }, >> { } >> }; >> @@ -291,6 +501,43 @@ static const struct dev_pm_ops qcom_snps_hsphy_pm_ops = { >> qcom_snps_hsphy_runtime_resume, NULL) >> }; >> >> +static void hsphy_override_param_update_val(const struct override_param_map map, >> + s32 dt_val, struct phy_override_seq *seq_entry) >> +{ >> + int i; >> + >> + /* >> + * We prepare the param table for each param in increasing order >> + * of dt values. So we need to iterate over the list once to get >> + * the required register value. >> + */ >> + for (i = 0 ; i < map.table_size-1; i++) { >> + if (map.param_table[i].value >= dt_val) >> + break; >> + } > The comment is confusing. param_table[] already has values in the increasing > order. we have to find *closet* match here. To keep it simple, we select the > entry that has equal or the next highest (round up) value. clarify the > comment. Sure. >> + >> + 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 << __ffs(map.param_mask); >> +} >> + >> +static void phy_read_param_override_seq(struct device *dev) >> +{ >> + struct device_node *node = dev->of_node; >> + s32 val; >> + int ret, i; >> + struct qcom_snps_hsphy *hsphy; >> + struct override_param_map *cfg = (struct override_param_map* ) of_device_get_match_data(dev); > nit pick. No explicit type case is needed here. of_device_get_match_data() > returns a void pointer. so it can be written as > > struct override_param_map *cfg = of_device_get_match_data(dev); > >> + hsphy = dev_get_drvdata(dev); >> + >> + for (i = 0; i < ARRAY_SIZE(phy_seq_props); i++) { >> + ret = of_property_read_s32(node, phy_seq_props[i], &val); >> + if (!ret) >> + hsphy_override_param_update_val(cfg[i], val, &update_seq_cfg[i]); >> + } >> +} >> + > Can we add a dev_dbg here to aid the debugging? > > All the functions defined in this driver has qcom_snps_hsphy_xxxx convention. > Lets follow the same. Sure, Will rename the function accordingly. >> static int qcom_snps_hsphy_probe(struct platform_device *pdev) >> { >> struct device *dev = &pdev->dev; >> @@ -352,6 +599,7 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev) >> >> dev_set_drvdata(dev, hsphy); >> phy_set_drvdata(generic_phy, hsphy); >> + phy_read_param_override_seq(dev); >> >> phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate); >> if (!IS_ERR(phy_provider)) >> -- >> 2.7.4 >> > Bjorn/Vinod, > > I have reviewed this patch on our internal review system. The basic design > looks good to me. Can you please review the patch and provide your inputs. > > Thanks, > Pavan ^ permalink raw reply [flat|nested] 11+ messages in thread
* [v3 3/3] arm64: dts: qcom: sc7280: Update SNPS Phy params for SC7280 IDP device 2022-05-08 12:12 [v3 0/3] Add QCOM SNPS PHY overriding params support Krishna Kurapati 2022-05-08 12:12 ` [v3 1/3] dt-bindings: phy: qcom,usb-snps-femto-v2: Add phy override params bindings Krishna Kurapati 2022-05-08 12:12 ` [v3 2/3] phy: qcom-snps: Add support for overriding phy tuning parameters Krishna Kurapati @ 2022-05-08 12:12 ` Krishna Kurapati 2022-05-09 3:20 ` Pavan Kondeti 2 siblings, 1 reply; 11+ messages in thread From: Krishna Kurapati @ 2022-05-08 12:12 UTC (permalink / raw) To: Krzysztof Kozlowski, Rob Herring, Andy Gross, Bjorn Andersson, Greg Kroah-Hartman, Stephen Boyd, Doug Anderson, Matthias Kaehlcke, Wesley Cheng Cc: devicetree, linux-arm-msm, linux-usb, linux-kernel, linux-phy, quic_pkondeti, quic_ppratap, quic_vpulyala, Krishna Kurapati, Sandeep Maheswaram Overriding the SNPS Phy tuning parameters for SC7280 IDP device. Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> --- arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi index 5eb6689..ad85ffb 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi @@ -325,6 +325,12 @@ vdda-pll-supply = <&vreg_l10c_0p8>; vdda33-supply = <&vreg_l2b_3p0>; vdda18-supply = <&vreg_l1c_1p8>; + qcom,hs-rise-fall-time-bps = <0>; + qcom,squelch-detector-bps = <(-2090)>; + qcom,hs-disconnect-bps = <1743>; + qcom,hs-amplitude-bps = <1780>; + qcom,hs-crossover-voltage = <(-31)>; + qcom,hs-output-impedance = <2600>; }; &usb_1_qmpphy { -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [v3 3/3] arm64: dts: qcom: sc7280: Update SNPS Phy params for SC7280 IDP device 2022-05-08 12:12 ` [v3 3/3] arm64: dts: qcom: sc7280: Update SNPS Phy params for SC7280 IDP device Krishna Kurapati @ 2022-05-09 3:20 ` Pavan Kondeti 2022-05-10 17:35 ` Krishna Kurapati PSSNV 0 siblings, 1 reply; 11+ messages in thread From: Pavan Kondeti @ 2022-05-09 3:20 UTC (permalink / raw) To: Krishna Kurapati Cc: Krzysztof Kozlowski, Rob Herring, Andy Gross, Bjorn Andersson, Greg Kroah-Hartman, Stephen Boyd, Doug Anderson, Matthias Kaehlcke, Wesley Cheng, devicetree, linux-arm-msm, linux-usb, linux-kernel, linux-phy, quic_pkondeti, quic_ppratap, quic_vpulyala, Sandeep Maheswaram Hi Krishna, On Sun, May 08, 2022 at 05:42:27PM +0530, Krishna Kurapati wrote: > Overriding the SNPS Phy tuning parameters for SC7280 IDP device. > > Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com> > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > --- > arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi > index 5eb6689..ad85ffb 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi > @@ -325,6 +325,12 @@ > vdda-pll-supply = <&vreg_l10c_0p8>; > vdda33-supply = <&vreg_l2b_3p0>; > vdda18-supply = <&vreg_l1c_1p8>; > + qcom,hs-rise-fall-time-bps = <0>; > + qcom,squelch-detector-bps = <(-2090)>; > + qcom,hs-disconnect-bps = <1743>; > + qcom,hs-amplitude-bps = <1780>; > + qcom,hs-crossover-voltage = <(-31)>; > + qcom,hs-output-impedance = <2600>; > }; Is this an example change or do we see any HS electrical compliance failures on SC7280 IDP that will get fixed with these override sequence? Thanks, Pavan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [v3 3/3] arm64: dts: qcom: sc7280: Update SNPS Phy params for SC7280 IDP device 2022-05-09 3:20 ` Pavan Kondeti @ 2022-05-10 17:35 ` Krishna Kurapati PSSNV 2022-05-11 2:31 ` Pavan Kondeti 0 siblings, 1 reply; 11+ messages in thread From: Krishna Kurapati PSSNV @ 2022-05-10 17:35 UTC (permalink / raw) To: Pavan Kondeti Cc: Krzysztof Kozlowski, Rob Herring, Andy Gross, Bjorn Andersson, Greg Kroah-Hartman, Stephen Boyd, Doug Anderson, Matthias Kaehlcke, Wesley Cheng, devicetree, linux-arm-msm, linux-usb, linux-kernel, linux-phy, quic_ppratap, quic_vpulyala On 5/9/2022 8:50 AM, Pavan Kondeti wrote: > Hi Krishna, > > On Sun, May 08, 2022 at 05:42:27PM +0530, Krishna Kurapati wrote: >> Overriding the SNPS Phy tuning parameters for SC7280 IDP device. >> >> Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com> >> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> >> --- >> arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi >> index 5eb6689..ad85ffb 100644 >> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi >> @@ -325,6 +325,12 @@ >> vdda-pll-supply = <&vreg_l10c_0p8>; >> vdda33-supply = <&vreg_l2b_3p0>; >> vdda18-supply = <&vreg_l1c_1p8>; >> + qcom,hs-rise-fall-time-bps = <0>; >> + qcom,squelch-detector-bps = <(-2090)>; >> + qcom,hs-disconnect-bps = <1743>; >> + qcom,hs-amplitude-bps = <1780>; >> + qcom,hs-crossover-voltage = <(-31)>; >> + qcom,hs-output-impedance = <2600>; >> }; > Is this an example change or do we see any HS electrical compliance failures > on SC7280 IDP that will get fixed with these override sequence? > > Thanks, > Pavan Hi Pavan, These results were based on compliance testing results. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [v3 3/3] arm64: dts: qcom: sc7280: Update SNPS Phy params for SC7280 IDP device 2022-05-10 17:35 ` Krishna Kurapati PSSNV @ 2022-05-11 2:31 ` Pavan Kondeti 0 siblings, 0 replies; 11+ messages in thread From: Pavan Kondeti @ 2022-05-11 2:31 UTC (permalink / raw) To: Krishna Kurapati PSSNV Cc: Pavan Kondeti, Krzysztof Kozlowski, Rob Herring, Andy Gross, Bjorn Andersson, Greg Kroah-Hartman, Stephen Boyd, Doug Anderson, Matthias Kaehlcke, Wesley Cheng, devicetree, linux-arm-msm, linux-usb, linux-kernel, linux-phy, quic_ppratap, quic_vpulyala On Tue, May 10, 2022 at 11:05:42PM +0530, Krishna Kurapati PSSNV wrote: > > On 5/9/2022 8:50 AM, Pavan Kondeti wrote: > >Hi Krishna, > > > >On Sun, May 08, 2022 at 05:42:27PM +0530, Krishna Kurapati wrote: > >>Overriding the SNPS Phy tuning parameters for SC7280 IDP device. > >> > >>Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com> > >>Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > >>--- > >> arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 6 ++++++ > >> 1 file changed, 6 insertions(+) > >> > >>diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi > >>index 5eb6689..ad85ffb 100644 > >>--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi > >>+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi > >>@@ -325,6 +325,12 @@ > >> vdda-pll-supply = <&vreg_l10c_0p8>; > >> vdda33-supply = <&vreg_l2b_3p0>; > >> vdda18-supply = <&vreg_l1c_1p8>; > >>+ qcom,hs-rise-fall-time-bps = <0>; > >>+ qcom,squelch-detector-bps = <(-2090)>; > >>+ qcom,hs-disconnect-bps = <1743>; > >>+ qcom,hs-amplitude-bps = <1780>; > >>+ qcom,hs-crossover-voltage = <(-31)>; > >>+ qcom,hs-output-impedance = <2600>; > >> }; > >Is this an example change or do we see any HS electrical compliance failures > >on SC7280 IDP that will get fixed with these override sequence? > > > >Thanks, > >Pavan > > Hi Pavan, > > These results were based on compliance testing results. > Ok, Do we know what tests are failing and getting fixed with these settings? Can you mention it in the changelog? Thanks, Pavan ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-05-11 2:32 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-05-08 12:12 [v3 0/3] Add QCOM SNPS PHY overriding params support Krishna Kurapati 2022-05-08 12:12 ` [v3 1/3] dt-bindings: phy: qcom,usb-snps-femto-v2: Add phy override params bindings Krishna Kurapati 2022-05-09 2:52 ` Pavan Kondeti 2022-05-08 12:12 ` [v3 2/3] phy: qcom-snps: Add support for overriding phy tuning parameters Krishna Kurapati 2022-05-08 13:22 ` kernel test robot 2022-05-09 3:18 ` Pavan Kondeti 2022-05-10 17:34 ` Krishna Kurapati PSSNV 2022-05-08 12:12 ` [v3 3/3] arm64: dts: qcom: sc7280: Update SNPS Phy params for SC7280 IDP device Krishna Kurapati 2022-05-09 3:20 ` Pavan Kondeti 2022-05-10 17:35 ` Krishna Kurapati PSSNV 2022-05-11 2:31 ` Pavan Kondeti
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).