| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 13 Apr 2022 15:12:09 +0530
From: Vinod Koul <vkoul@...nel.org>
To: Sandeep Maheswaram <quic_c_sanm@...cinc.com>,
Bjorn Andersson <bjorn.andersson@...aro.org>
Cc: Rob Herring <robh+dt@...nel.org>, Andy Gross <agross@...nel.org>,
Kishon Vijay Abraham I <kishon@...com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Wesley Cheng <wcheng@...eaurora.org>,
Stephen Boyd <swboyd@...omium.org>,
Doug Anderson <dianders@...omium.org>,
Matthias Kaehlcke <mka@...omium.org>,
Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
devicetree@...r.kernel.org, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-phy@...ts.infradead.org,
linux-usb@...r.kernel.org, quic_pkondeti@...cinc.com,
quic_ppratap@...cinc.com
Subject: Re: [PATCH v2 2/3] phy: qcom-snps: Add support for overriding phy
tuning parameters
On 03-03-22, 11:43, Sandeep Maheswaram wrote:
> Added support for overriding x0,x1,x2,x3 params for SNPS PHY by reading
> values from device tree.
>
> Signed-off-by: Sandeep Maheswaram <quic_c_sanm@...cinc.com>
> ---
> drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c | 192 ++++++++++++++++++++++++++
> 1 file changed, 192 insertions(+)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
> index 7e61202..b5aa06d 100644
> --- a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
> +++ b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
> @@ -51,6 +51,48 @@
> #define USB2_SUSPEND_N BIT(2)
> #define USB2_SUSPEND_N_SEL BIT(3)
>
> +#define USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X0 (0x6c)
> +
> +/*USB_PHY_HS_PHY_OVERRIDE_X0 register bits*/
space after /* and before */ (checkpatch.pl --strict would warn you)
Pls fix it everywhere
> +#define HS_DISCONNECT_MASK GENMASK(2, 0)
> +#define HS_DISCONNECT_SHIFT 0x0
> +
> +#define SQUELCH_DETECTOR_MASK GENMASK(7, 5)
> +#define SQUELCH_DETECTOR_SHIFT 0x5
> +
> +
> +#define USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X1 (0x70)
> +
> +/*USB_PHY_HS_PHY_OVERRIDE_X1 register bits*/
> +#define HS_AMPLITUDE_MASK GENMASK(3, 0)
> +#define HS_AMPLITUDE_SHIFT 0x0
> +
> +#define PREEMPHASIS_DURATION_MASK BIT(5)
> +#define PREEMPHASIS_DURATION_SHIFT 0x5
> +
> +#define PREEMPHASIS_AMPLITUDE_MASK GENMASK(7, 6)
> +#define PREEMPHASIS_AMPLITUDE_SHIFT 0x6
> +
> +
> +#define USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X2 (0x74)
> +
> +/*USB_PHY_HS_PHY_OVERRIDE_X2 register bits*/
> +#define HS_RISE_FALL_MASK GENMASK(1, 0)
> +#define HS_RISE_FALL_SHIFT 0x0
> +
> +#define HS_CROSSOVER_VOLTAGE_MASK GENMASK(3, 2)
> +#define HS_CROSSOVER_VOLTAGE_SHIFT 0x2
> +
> +#define HS_OUTPUT_IMPEDANCE_MASK GENMASK(5, 4)
> +#define HS_OUTPUT_IMPEDANCE_SHIFT 0x4
> +
> +
> +#define USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X3 (0x78)
> +
> +/*USB_PHY_HS_PHY_OVERRIDE_X3 register bits*/
> +#define LS_FS_OUTPUT_IMPEDANCE_MASK GENMASK(3, 0)
> +#define LS_FS_OUTPUT_IMPEDANCE_SHIFT 0x0
> +
> #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)
> @@ -65,6 +107,43 @@ static const char * const qcom_snps_hsphy_vreg_names[] = {
>
> #define SNPS_HS_NUM_VREGS ARRAY_SIZE(qcom_snps_hsphy_vreg_names)
>
> +/* struct override_param - structure holding snps phy overriding param
> + * set override true if the device tree property exists and read and assign
> + * to value
> + */
> +struct override_param {
> + bool override;
> + u8 value;
> +};
> +
> +/*struct override_params - structure holding snps phy overriding params
> + * @hs_disconnect: disconnect threshold
> + * @squelch_detector: threshold to detect valid high-speed data
> + * @hs_amplitude: high-speed DC level voltage
> + * @preemphasis_duration: duration for which the HS pre-emphasis current
> + * is sourced onto DP<#> or DM<#>
> + * @preemphasis_amplitude: current sourced to DP<#> and DM<#> after
> + * a J-to-K or K-to-J transition.
> + * @hs_rise_fall_time: rise/fall times of the high-speed waveform
> + * @hs_crossover_voltage: voltage at which the DP<#> and DM<#>
> + * signals cross while transmitting in HS mode
> + * @hs_output_impedance: driver source impedance to compensate for added series
> + * resistance on the USB
> + * @ls_fs_output_impedance: low and full-speed single-ended source
> + * impedance while driving high
> + */
> +struct override_params {
> + struct override_param hs_disconnect;
> + struct override_param squelch_detector;
> + struct override_param hs_amplitude;
> + struct override_param preemphasis_duration;
> + struct override_param preemphasis_amplitude;
> + struct override_param hs_rise_fall_time;
> + struct override_param hs_crossover_voltage;
> + struct override_param hs_output_impedance;
> + struct override_param ls_fs_output_impedance;
> +};
> +
> /**
> * struct qcom_snps_hsphy - snps hs phy attributes
> *
> @@ -87,6 +166,7 @@ struct qcom_snps_hsphy {
> struct clk *ref_clk;
> struct reset_control *phy_reset;
> struct regulator_bulk_data vregs[SNPS_HS_NUM_VREGS];
> + struct override_params overrides;
>
> bool phy_initialized;
> enum phy_mode mode;
> @@ -175,6 +255,7 @@ static int qcom_snps_hsphy_set_mode(struct phy *phy, enum phy_mode mode,
> static int qcom_snps_hsphy_init(struct phy *phy)
> {
> struct qcom_snps_hsphy *hsphy = phy_get_drvdata(phy);
> + struct override_params *or = &hsphy->overrides;
> int ret;
>
> dev_vdbg(&phy->dev, "%s(): Initializing SNPS HS phy\n", __func__);
> @@ -222,6 +303,60 @@ 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);
>
> + if (or->hs_disconnect.override)
> + qcom_snps_hsphy_write_mask(hsphy->base,
> + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X0,
> + HS_DISCONNECT_MASK,
> + or->hs_disconnect.value << HS_DISCONNECT_SHIFT);
> +
> + if (or->squelch_detector.override)
> + qcom_snps_hsphy_write_mask(hsphy->base,
> + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X0,
> + SQUELCH_DETECTOR_MASK,
> + or->squelch_detector.value << SQUELCH_DETECTOR_SHIFT);
> +
> + if (or->hs_amplitude.override)
> + qcom_snps_hsphy_write_mask(hsphy->base,
> + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X1,
> + HS_AMPLITUDE_MASK,
> + or->hs_amplitude.value << HS_AMPLITUDE_SHIFT);
> +
> + if (or->preemphasis_duration.override)
> + qcom_snps_hsphy_write_mask(hsphy->base,
> + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X1,
> + PREEMPHASIS_DURATION_MASK,
> + or->preemphasis_duration.value << PREEMPHASIS_DURATION_SHIFT);
> +
> + if (or->preemphasis_amplitude.override)
> + qcom_snps_hsphy_write_mask(hsphy->base,
> + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X1,
> + PREEMPHASIS_AMPLITUDE_MASK,
> + or->preemphasis_amplitude.value << PREEMPHASIS_AMPLITUDE_SHIFT);
> +
> + if (or->hs_rise_fall_time.override)
> + qcom_snps_hsphy_write_mask(hsphy->base,
> + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X2,
> + HS_RISE_FALL_MASK,
> + or->hs_rise_fall_time.value << HS_RISE_FALL_SHIFT);
> +
> + if (or->hs_crossover_voltage.override)
> + qcom_snps_hsphy_write_mask(hsphy->base,
> + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X2,
> + HS_CROSSOVER_VOLTAGE_MASK,
> + or->hs_crossover_voltage.value << HS_CROSSOVER_VOLTAGE_SHIFT);
> +
> + if (or->hs_output_impedance.override)
> + qcom_snps_hsphy_write_mask(hsphy->base,
> + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X2,
> + HS_OUTPUT_IMPEDANCE_MASK,
> + or->hs_output_impedance.value << HS_OUTPUT_IMPEDANCE_SHIFT);
> +
> + if (or->ls_fs_output_impedance.override)
> + qcom_snps_hsphy_write_mask(hsphy->base,
> + USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X3,
> + LS_FS_OUTPUT_IMPEDANCE_MASK,
> + or->ls_fs_output_impedance.value << LS_FS_OUTPUT_IMPEDANCE_SHIFT);
> +
> qcom_snps_hsphy_write_mask(hsphy->base,
> USB2_PHY_USB_PHY_HS_PHY_CTRL_COMMON2,
> VREGBYPASS, VREGBYPASS);
> @@ -292,12 +427,15 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev)
> struct qcom_snps_hsphy *hsphy;
> struct phy_provider *phy_provider;
> struct phy *generic_phy;
> + struct override_params *or;
> int ret, i;
> int num;
> + u32 value;
>
> hsphy = devm_kzalloc(dev, sizeof(*hsphy), GFP_KERNEL);
> if (!hsphy)
> return -ENOMEM;
> + or = &hsphy->overrides;
>
> hsphy->base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(hsphy->base))
> @@ -329,6 +467,60 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev)
> return ret;
> }
>
> + if (!of_property_read_u32(dev->of_node, "qcom,hs-disconnect",
> + &value)) {
> + or->hs_disconnect.value = (u8)value;
> + or->hs_disconnect.override = true;
> + }
> +
> + if (!of_property_read_u32(dev->of_node, "qcom,squelch-detector",
> + &value)) {
> + or->squelch_detector.value = (u8)value;
> + or->squelch_detector.override = true;
> + }
> +
> + if (!of_property_read_u32(dev->of_node, "qcom,hs-amplitude",
> + &value)) {
> + or->hs_amplitude.value = (u8)value;
> + or->hs_amplitude.override = true;
> + }
> +
> + if (!of_property_read_u32(dev->of_node, "qcom,preemphasis-duration",
> + &value)) {
> + or->preemphasis_duration.value = (u8)value;
> + or->preemphasis_duration.override = true;
> + }
> +
> + if (!of_property_read_u32(dev->of_node, "qcom,preemphasis-amplitude",
> + &value)) {
> + or->preemphasis_amplitude.value = (u8)value;
> + or->preemphasis_amplitude.override = true;
> + }
> +
> + if (!of_property_read_u32(dev->of_node, "qcom,hs-rise-fall-time",
> + &value)) {
> + or->hs_rise_fall_time.value = (u8)value;
> + or->hs_rise_fall_time.override = true;
> + }
> +
> + if (!of_property_read_u32(dev->of_node, "qcom,hs-crossover-voltage",
> + &value)) {
> + or->hs_crossover_voltage.value = (u8)value;
> + or->hs_crossover_voltage.override = true;
> + }
> +
> + if (!of_property_read_u32(dev->of_node, "qcom,hs-output-impedance",
> + &value)) {
> + or->hs_output_impedance.value = (u8)value;
> + or->hs_output_impedance.override = true;
> + }
> +
> + if (!of_property_read_u32(dev->of_node, "qcom,ls-fs-output-impedance",
> + &value)) {
> + or->ls_fs_output_impedance.value = (u8)value;
> + or->ls_fs_output_impedance.override = true;
> + }
Are all these values board specific or IP specific? Can we add these
values as tables in driver?
Bjorn what do you think about the above proposal?
--
~Vinod
Powered by blists - more mailing lists