Message ID | 1580305919-30946-1-git-send-email-sanm@codeaurora.org |
---|---|
Headers | show |
Series | Add QUSB2 PHY support for SC7180 | expand |
Hi Sandeep, On Wed, Jan 29, 2020 at 07:21:56PM +0530, Sandeep Maheswaram wrote: > Added new structure for overriding tuning parameters in QUSB2 V2 PHY as the > override params are increased due to usage of generic QUSB2 V2 phy table. > Also added bias-ctrl-value,charge-ctrl-value and hsdisc-trim-value params. > > Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org> > --- > drivers/phy/qualcomm/phy-qcom-qusb2.c | 125 +++++++++++++++++++++++++--------- > 1 file changed, 93 insertions(+), 32 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qusb2.c b/drivers/phy/qualcomm/phy-qcom-qusb2.c > index 70c9da6..f45fda3 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qusb2.c > +++ b/drivers/phy/qualcomm/phy-qcom-qusb2.c > @@ -66,6 +66,14 @@ > #define IMP_RES_OFFSET_MASK GENMASK(5, 0) > #define IMP_RES_OFFSET_SHIFT 0x0 > > +/* QUSB2PHY_PLL_BIAS_CONTROL_2 register bits */ > +#define BIAS_CTRL2_RES_OFFSET_MASK GENMASK(5, 0) > +#define BIAS_CTRL2_RES_OFFSET_SHIFT 0x0 > + > +/* QUSB2PHY_CHG_CONTROL_2 register bits */ > +#define CHG_CTRL2_OFFSET_MASK GENMASK(5, 4) > +#define CHG_CTRL2_OFFSET_SHIFT 0x4 > + > /* QUSB2PHY_PORT_TUNE1 register bits */ > #define HSTX_TRIM_MASK GENMASK(7, 4) > #define HSTX_TRIM_SHIFT 0x4 > @@ -73,6 +81,10 @@ > #define PREEMPHASIS_EN_MASK GENMASK(1, 0) > #define PREEMPHASIS_EN_SHIFT 0x0 > > +/* QUSB2PHY_PORT_TUNE2 register bits */ > +#define HSDISC_TRIM_MASK GENMASK(1, 0) > +#define HSDISC_TRIM_SHIFT 0x0 > + > #define QUSB2PHY_PLL_ANALOG_CONTROLS_TWO 0x04 > #define QUSB2PHY_PLL_CLOCK_INVERTERS 0x18c > #define QUSB2PHY_PLL_CMODE 0x2c > @@ -277,6 +289,34 @@ static const char * const qusb2_phy_vreg_names[] = { > > #define QUSB2_NUM_VREGS ARRAY_SIZE(qusb2_phy_vreg_names) > > +/* struct override_param - structure holding qusb2 v2 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 qusb2 v2 phy overriding params > + * @imp_res_offset: rescode offset to be updated in IMP_CTRL1 register > + * @hstx_trim: HSTX_TRIM to be updated in TUNE1 register > + * @preemphasis: Amplitude Pre-Emphasis to be updated in TUNE1 register > + * @preemphasis_width: half/full-width Pre-Emphasis updated via TUNE1 > + * @bias_ctrl: bias ctrl to be updated in BIAS_CONTROL_2 register > + * @charge_ctrl: charge ctrl to be updated in CHG_CTRL2 register > + * @hsdisc_trim: disconnect threshold to be updated in TUNE2 register > + */ > +struct override_params { > + struct override_param imp_res_offset; > + struct override_param hstx_trim; > + struct override_param preemphasis; > + struct override_param preemphasis_width; ideally the refactoring (struct override_param(s)) and the support for the new override paramters would be in two separate patches, which would make it easier to review the different steps. > + struct override_param bias_ctrl; > + struct override_param charge_ctrl; > + struct override_param hsdisc_trim; > +}; > + > /** > * struct qusb2_phy - structure holding qusb2 phy attributes > * > @@ -292,20 +332,15 @@ static const char * const qusb2_phy_vreg_names[] = { > * @tcsr: TCSR syscon register map > * @cell: nvmem cell containing phy tuning value > * > - * @override_imp_res_offset: PHY should use different rescode offset > - * @imp_res_offset_value: rescode offset to be updated in IMP_CTRL1 register > - * @override_hstx_trim: PHY should use different HSTX o/p current value > - * @hstx_trim_value: HSTX_TRIM value to be updated in TUNE1 register > - * @override_preemphasis: PHY should use different pre-amphasis amplitude > - * @preemphasis_level: Amplitude Pre-Emphasis to be updated in TUNE1 register > - * @override_preemphasis_width: PHY should use different pre-emphasis duration > - * @preemphasis_width: half/full-width Pre-Emphasis updated via TUNE1 > + * @overrides: pointer to structure for all overriding tuning params > * > * @cfg: phy config data > * @has_se_clk_scheme: indicate if PHY has single-ended ref clock scheme > * @phy_initialized: indicate if PHY has been initialized > * @mode: current PHY mode > */ > + > + > struct qusb2_phy { > struct phy *phy; > void __iomem *base; > @@ -319,14 +354,7 @@ struct qusb2_phy { > struct regmap *tcsr; > struct nvmem_cell *cell; > > - bool override_imp_res_offset; > - u8 imp_res_offset_value; > - bool override_hstx_trim; > - u8 hstx_trim_value; > - bool override_preemphasis; > - u8 preemphasis_level; > - bool override_preemphasis_width; > - u8 preemphasis_width; > + struct override_params overrides; > > const struct qusb2_phy_cfg *cfg; > bool has_se_clk_scheme; > @@ -395,23 +423,33 @@ static void qusb2_phy_override_phy_params(struct qusb2_phy *qphy) > { > const struct qusb2_phy_cfg *cfg = qphy->cfg; > > - if (qphy->override_imp_res_offset) > + if (qphy->overrides.imp_res_offset.override) you could consider introducing a local variable 'struct override_params overrides *or' and assign it to &qphy->overrides, which would make accessing the overrides slightly less clunky. > qusb2_write_mask(qphy->base, QUSB2PHY_IMP_CTRL1, > - qphy->imp_res_offset_value << IMP_RES_OFFSET_SHIFT, > + qphy->overrides.imp_res_offset.value << IMP_RES_OFFSET_SHIFT, > IMP_RES_OFFSET_MASK); > > - if (qphy->override_hstx_trim) > + if (qphy->overrides.bias_ctrl.override) > + qusb2_write_mask(qphy->base, QUSB2PHY_PLL_BIAS_CONTROL_2, > + qphy->overrides.bias_ctrl.value << BIAS_CTRL2_RES_OFFSET_SHIFT, > + BIAS_CTRL2_RES_OFFSET_MASK); > + > + if (qphy->overrides.charge_ctrl.override) > + qusb2_write_mask(qphy->base, QUSB2PHY_CHG_CTRL2, > + qphy->overrides.charge_ctrl.value << CHG_CTRL2_OFFSET_SHIFT, > + CHG_CTRL2_OFFSET_MASK); > + > + if (qphy->overrides.hstx_trim.override) > qusb2_write_mask(qphy->base, cfg->regs[QUSB2PHY_PORT_TUNE1], > - qphy->hstx_trim_value << HSTX_TRIM_SHIFT, > + qphy->overrides.hstx_trim.value << HSTX_TRIM_SHIFT, > HSTX_TRIM_MASK); > > - if (qphy->override_preemphasis) > + if (qphy->overrides.preemphasis.override) > qusb2_write_mask(qphy->base, cfg->regs[QUSB2PHY_PORT_TUNE1], > - qphy->preemphasis_level << PREEMPHASIS_EN_SHIFT, > + qphy->overrides.preemphasis.value << PREEMPHASIS_EN_SHIFT, > PREEMPHASIS_EN_MASK); > > - if (qphy->override_preemphasis_width) { > - if (qphy->preemphasis_width == > + if (qphy->overrides.preemphasis_width.override) { > + if (qphy->overrides.preemphasis_width.value == > QUSB2_V2_PREEMPHASIS_WIDTH_HALF_BIT) > qusb2_setbits(qphy->base, > cfg->regs[QUSB2PHY_PORT_TUNE1], > @@ -421,6 +459,11 @@ static void qusb2_phy_override_phy_params(struct qusb2_phy *qphy) > cfg->regs[QUSB2PHY_PORT_TUNE1], > PREEMPH_WIDTH_HALF_BIT); > } > + > + if (qphy->overrides.hsdisc_trim.override) > + qusb2_write_mask(qphy->base, cfg->regs[QUSB2PHY_PORT_TUNE2], > + qphy->overrides.hsdisc_trim.value << HSDISC_TRIM_SHIFT, > + HSDISC_TRIM_MASK); > } > > /* > @@ -864,26 +907,44 @@ static int qusb2_phy_probe(struct platform_device *pdev) > > if (!of_property_read_u32(dev->of_node, "qcom,imp-res-offset-value", > &value)) { > - qphy->imp_res_offset_value = (u8)value; > - qphy->override_imp_res_offset = true; > + qphy->overrides.imp_res_offset.value = (u8)value; same as above, consider whether 'or->imp_res_offset.value' is an improvement. > + qphy->overrides.imp_res_offset.override = true; > + } > + > + if (!of_property_read_u32(dev->of_node, "qcom,bias-ctrl-value", > + &value)) { > + qphy->overrides.bias_ctrl.value = (u8)value; > + qphy->overrides.bias_ctrl.override = true; > + } > + > + if (!of_property_read_u32(dev->of_node, "qcom,charge-ctrl-value", > + &value)) { > + qphy->overrides.charge_ctrl.value = (u8)value; > + qphy->overrides.charge_ctrl.override = true; > } > > if (!of_property_read_u32(dev->of_node, "qcom,hstx-trim-value", > &value)) { > - qphy->hstx_trim_value = (u8)value; > - qphy->override_hstx_trim = true; > + qphy->overrides.hstx_trim.value = (u8)value; > + qphy->overrides.hstx_trim.override = true; > } > > if (!of_property_read_u32(dev->of_node, "qcom,preemphasis-level", > &value)) { > - qphy->preemphasis_level = (u8)value; > - qphy->override_preemphasis = true; > + qphy->overrides.preemphasis.value = (u8)value; > + qphy->overrides.preemphasis.override = true; > } > > if (!of_property_read_u32(dev->of_node, "qcom,preemphasis-width", > &value)) { > - qphy->preemphasis_width = (u8)value; > - qphy->override_preemphasis_width = true; > + qphy->overrides.preemphasis_width.value = (u8)value; > + qphy->overrides.preemphasis_width.override = true; > + } > + > + if (!of_property_read_u32(dev->of_node, "qcom,hsdisc-trim-value", > + &value)) { > + qphy->overrides.hsdisc_trim.value = (u8)value; > + qphy->overrides.hsdisc_trim.override = true; > } > > pm_runtime_set_active(dev); Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
On Wed, Jan 29, 2020 at 07:21:53PM +0530, Sandeep Maheswaram wrote: > Add compatibles for generic QUSB2 V2 phy which can be used for > sdm845 and sc7180. > > Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org> > --- > Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml > index 90b3cc6..43082c8 100644 > --- a/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml > +++ b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml > @@ -15,10 +15,17 @@ description: > > properties: > compatible: > - enum: > - - qcom,msm8996-qusb2-phy > - - qcom,msm8998-qusb2-phy > - - qcom,sdm845-qusb2-phy > + oneOf: > + - items: > + - enum: > + - qcom,msm8996-qusb2-phy > + - qcom,msm8998-qusb2-phy > + - qcom,qusb2-v2-phy > + - items: > + - enum: > + - qcom,sc7180-qusb2-phy > + - qcom,sdm845-qusb2-phy > + - const: qcom,qusb2-v2-phy > reg: > maxItems: 1 Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
On Wed, Jan 29, 2020 at 07:21:57PM +0530, Sandeep Maheswaram wrote: > Use generic QUSB2 V2 Phy configuration for SC7180. > > Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org> > --- > arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi > index 8011c5f..0d6761b 100644 > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi > @@ -1052,7 +1052,7 @@ > }; > > usb_1_hsphy: phy@88e3000 { > - compatible = "qcom,sc7180-qusb2-phy"; > + compatible = "qcom,sc7180-qusb2-phy", "qcom,qusb2-v2-phy"; > reg = <0 0x088e3000 0 0x400>; > status = "disabled"; > #phy-cells = <0>; FWIW Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Quoting Sandeep Maheswaram (2020-01-29 05:51:52) > Convert QUSB2 phy bindings to DT schema format using json-schema. > > Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Quoting Sandeep Maheswaram (2020-01-29 05:51:57) > Use generic QUSB2 V2 Phy configuration for SC7180. > > Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org>
On Wed, Jan 29, 2020 at 07:21:52PM +0530, Sandeep Maheswaram wrote: > Convert QUSB2 phy bindings to DT schema format using json-schema. > > Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org> > --- > .../devicetree/bindings/phy/qcom,qusb2-phy.yaml | 142 +++++++++++++++++++++ > .../devicetree/bindings/phy/qcom-qusb2-phy.txt | 68 ---------- > 2 files changed, 142 insertions(+), 68 deletions(-) > create mode 100644 Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml > delete mode 100644 Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt > > diff --git a/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml > new file mode 100644 > index 0000000..90b3cc6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml > @@ -0,0 +1,142 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > + > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/phy/qcom,qusb2-phy.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > + > +title: Qualcomm QUSB2 phy controller > + > +maintainers: > + - Manu Gautam <mgautam@codeaurora.org> > + > +description: > + QUSB2 controller supports LS/FS/HS usb connectivity on Qualcomm chipsets. > + > +properties: > + compatible: > + enum: > + - qcom,msm8996-qusb2-phy > + - qcom,msm8998-qusb2-phy > + - qcom,sdm845-qusb2-phy > + reg: > + maxItems: 1 > + > + "#phy-cells": > + const: 0 > + > + clocks: > + minItems: 2 > + items: > + - description: phy config clock > + - description: 19.2 MHz ref clk > + - description: phy interface clock (Optional) > + > + clock-names: > + minItems: 2 > + items: > + - const: cfg_ahb > + - const: ref > + - const: iface > + > + vdda-pll-supply: > + description: > + Phandle to 1.8V regulator supply to PHY refclk pll block. > + > + vdda-phy-dpdm-supply: > + description: > + Phandle to 3.1V regulator supply to Dp/Dm port signals. > + > + resets: > + maxItems: 1 > + > + nvmem-cells: > + maxItems: 1 > + description: > + Phandle to nvmem cell that contains 'HS Tx trim' > + tuning parameter value for qusb2 phy. > + > + qcom,tcsr-syscon: > + description: > + Phandle to TCSR syscon register region. > + $ref: /schemas/types.yaml#/definitions/cell s/cell/phandle/ With that, Reviewed-by: Rob Herring <robh@kernel.org>