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@xxxxxxxxxxx> > --- > 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