From: Neil Armstrong <narmstrong@baylibre.com>
To: Hanjie Lin <hanjie.lin@amlogic.com>,
Jerome Brunet <jbrunet@baylibre.com>,
Rob Herring <robh@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Felipe Balbi <felipe.balbi@linux.intel.com>,
Kevin Hilman <khilman@baylibre.com>
Cc: Yue Wang <yue.wang@amlogic.com>,
linux-amlogic@lists.infradead.org,
linux-arm-kernel@lists.infradead.org, linux-usb@vger.kernel.org,
devicetree@vger.kernel.org, Carlo Caione <carlo@caione.org>,
Michael Turquette <mturquette@baylibre.com>,
Stephen Boyd <sboyd@kernel.org>,
Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
Liang Yang <liang.yang@amlogic.com>,
Jianxin Pan <jianxin.pan@amlogic.com>,
Qiufang Dai <qiufang.dai@amlogic.com>,
Jian Hu <jian.hu@amlogic.com>,
Victor Wan <victor.wan@amlogic.com>,
Xingyu Chen <xingyu.chen@amlogic.com>
Subject: Re: [PATCH v2 3/6] phy: amlogic: Add Amlogic A1 USB2 PHY Driver
Date: Wed, 18 Dec 2019 14:17:05 +0100 [thread overview]
Message-ID: <4cbc4216-4f1b-dab2-fccd-4ece7cfedb77@baylibre.com> (raw)
In-Reply-To: <1576636944-196192-4-git-send-email-hanjie.lin@amlogic.com>
Hi,
On 18/12/2019 03:42, Hanjie Lin wrote:
> This adds support for the USB2 PHY found in the Amlogic A1 SoC Family.
>
> It supports host mode only.
>
> Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com>
> Signed-off-by: Yue Wang <yue.wang@amlogic.com>
> ---
> drivers/phy/amlogic/phy-meson-g12a-usb2.c | 102 ++++++++++++++++++++++--------
> 1 file changed, 74 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/phy/amlogic/phy-meson-g12a-usb2.c b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
> index 9065ffc..2c242d3 100644
> --- a/drivers/phy/amlogic/phy-meson-g12a-usb2.c
> +++ b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
> @@ -146,11 +146,18 @@
> #define RESET_COMPLETE_TIME 1000
> #define PLL_RESET_COMPLETE_TIME 100
>
> +enum {
> + MESON_USB2_PHY_VERSION_10 = 0,
> + MESON_USB2_PHY_VERSION_11,
Are these the real "versions" of the phy or it's made up ?
> + MESON_USB2_PHY_VERSION_COUNT,
> +};
> +
> struct phy_meson_g12a_usb2_priv {
> struct device *dev;
> struct regmap *regmap;
> struct clk *clk;
> struct reset_control *reset;
> + int phy_version;
> };
>
> static const struct regmap_config phy_meson_g12a_usb2_regmap_conf = {
> @@ -192,18 +199,33 @@ static int phy_meson_g12a_usb2_init(struct phy *phy)
> FIELD_PREP(PHY_CTRL_R17_MPLL_FILTER_PVT2, 2) |
> FIELD_PREP(PHY_CTRL_R17_MPLL_FILTER_PVT1, 9));
>
> - regmap_write(priv->regmap, PHY_CTRL_R18,
> - FIELD_PREP(PHY_CTRL_R18_MPLL_LKW_SEL, 1) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_LK_W, 9) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_LK_S, 0x27) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_PFD_GAIN, 1) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_ROU, 7) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_DATA_SEL, 3) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_BIAS_ADJ, 1) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_BB_MODE, 0) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_ALPHA, 3) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_ADJ_LDO, 1) |
> - PHY_CTRL_R18_MPLL_ACG_RANGE);
> + if (priv->phy_version == MESON_USB2_PHY_VERSION_10)
> + regmap_write(priv->regmap, PHY_CTRL_R18,
> + FIELD_PREP(PHY_CTRL_R18_MPLL_LKW_SEL, 1) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_LK_W, 9) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_LK_S, 0x27) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_PFD_GAIN, 1) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_ROU, 7) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_DATA_SEL, 3) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_BIAS_ADJ, 1) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_BB_MODE, 0) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_ALPHA, 3) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_ADJ_LDO, 1) |
> + PHY_CTRL_R18_MPLL_ACG_RANGE);
> + else if (priv->phy_version == MESON_USB2_PHY_VERSION_11)
> + regmap_write(priv->regmap, PHY_CTRL_R18,
> + FIELD_PREP(PHY_CTRL_R18_MPLL_LKW_SEL, 1) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_LK_W, 9) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_LK_S, 0x27) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_PFD_GAIN, 1) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_ROU, 7) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_DATA_SEL, 3) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_BIAS_ADJ, 1) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_BB_MODE, 0) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_ALPHA, 3) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_ADJ_LDO, 1) |
> + PHY_CTRL_R18_MPLL_ACG_RANGE |
> + PHY_CTRL_R18_MPLL_DCO_CLK_SEL);
The only difference is PHY_CTRL_R18_MPLL_ACG_RANGE | PHY_CTRL_R18_MPLL_DCO_CLK_SEL,
you can easily simplify the code here by using a temp variable.
>
> udelay(PLL_RESET_COMPLETE_TIME);
>
> @@ -227,13 +249,24 @@ static int phy_meson_g12a_usb2_init(struct phy *phy)
> FIELD_PREP(PHY_CTRL_R20_USB2_BGR_VREF_4_0, 0) |
> FIELD_PREP(PHY_CTRL_R20_USB2_BGR_DBG_1_0, 0));
>
> - regmap_write(priv->regmap, PHY_CTRL_R4,
> - FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_7_0, 0xf) |
> - FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_15_8, 0xf) |
> - FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_23_16, 0xf) |
> - PHY_CTRL_R4_TEST_BYPASS_MODE_EN |
> - FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_1_0, 0) |
> - FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_3_2, 0));
> + if (priv->phy_version == MESON_USB2_PHY_VERSION_10)
> + regmap_write(priv->regmap, PHY_CTRL_R4,
> + FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_7_0, 0xf) |
> + FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_15_8, 0xf) |
> + FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_23_16, 0xf) |
> + PHY_CTRL_R4_TEST_BYPASS_MODE_EN |
> + FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_1_0, 0) |
> + FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_3_2, 0));
> + else if (priv->phy_version == MESON_USB2_PHY_VERSION_11) {
> + regmap_write(priv->regmap, PHY_CTRL_R21,
> + PHY_CTRL_R21_USB2_CAL_ACK_EN |
> + PHY_CTRL_R21_USB2_TX_STRG_PD |
> + FIELD_PREP(PHY_CTRL_R21_USB2_OTG_ACA_TRIM_1_0, 2));
> +
> + /* Analog Settings */
> + regmap_write(priv->regmap, PHY_CTRL_R13,
> + FIELD_PREP(PHY_CTRL_R13_MIN_COUNT_FOR_SYNC_DET, 7));
> + }
>
> /* Tuning Disconnect Threshold */
> regmap_write(priv->regmap, PHY_CTRL_R3,
> @@ -241,11 +274,13 @@ static int phy_meson_g12a_usb2_init(struct phy *phy)
> FIELD_PREP(PHY_CTRL_R3_HSDIC_REF, 1) |
> FIELD_PREP(PHY_CTRL_R3_DISC_THRESH, 3));
>
> - /* Analog Settings */
> - regmap_write(priv->regmap, PHY_CTRL_R14, 0);
> - regmap_write(priv->regmap, PHY_CTRL_R13,
> - PHY_CTRL_R13_UPDATE_PMA_SIGNALS |
> - FIELD_PREP(PHY_CTRL_R13_MIN_COUNT_FOR_SYNC_DET, 7));
> + if (priv->phy_version == MESON_USB2_PHY_VERSION_10) {
> + /* Analog Settings */
> + regmap_write(priv->regmap, PHY_CTRL_R14, 0);
> + regmap_write(priv->regmap, PHY_CTRL_R13,
> + PHY_CTRL_R13_UPDATE_PMA_SIGNALS |
> + FIELD_PREP(PHY_CTRL_R13_MIN_COUNT_FOR_SYNC_DET, 7));
> + }
>
> return 0;
> }
> @@ -271,6 +306,7 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
> struct resource *res;
> struct phy_meson_g12a_usb2_priv *priv;
> struct phy *phy;
> + struct device_node *np = dev->of_node;
> void __iomem *base;
> int ret;
>
> @@ -286,14 +322,23 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
> if (IS_ERR(base))
> return PTR_ERR(base);
>
> + if (of_device_is_compatible(np, "amlogic,g12a-usb2-phy"))
> + priv->phy_version = MESON_USB2_PHY_VERSION_10;
> + else if (of_device_is_compatible(np, "amlogic,a1-usb2-phy"))
> + priv->phy_version = MESON_USB2_PHY_VERSION_11;
> + else
> + return -EINVAL;
Please use of_device_get_match_data() and a match data for each compatible instead.
> +
> priv->regmap = devm_regmap_init_mmio(dev, base,
> &phy_meson_g12a_usb2_regmap_conf);
> if (IS_ERR(priv->regmap))
> return PTR_ERR(priv->regmap);
>
> - priv->clk = devm_clk_get(dev, "xtal");
> - if (IS_ERR(priv->clk))
> - return PTR_ERR(priv->clk);
> + if (priv->phy_version == MESON_USB2_PHY_VERSION_10) {
> + priv->clk = devm_clk_get(dev, "xtal");
> + if (IS_ERR(priv->clk))
> + return PTR_ERR(priv->clk);
> + }
>
> priv->reset = devm_reset_control_get(dev, "phy");
> if (IS_ERR(priv->reset))
> @@ -322,7 +367,8 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
>
> static const struct of_device_id phy_meson_g12a_usb2_of_match[] = {
> { .compatible = "amlogic,g12a-usb2-phy", },
> - { },
> + { .compatible = "amlogic,a1-usb2-phy", },
> + { /* Sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, phy_meson_g12a_usb2_of_match);
>
>
Thanks,
Neil
WARNING: multiple messages have this Message-ID (diff)
From: Neil Armstrong <narmstrong@baylibre.com>
To: Hanjie Lin <hanjie.lin@amlogic.com>,
Jerome Brunet <jbrunet@baylibre.com>,
Rob Herring <robh@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Felipe Balbi <felipe.balbi@linux.intel.com>,
Kevin Hilman <khilman@baylibre.com>
Cc: devicetree@vger.kernel.org, Victor Wan <victor.wan@amlogic.com>,
Jianxin Pan <jianxin.pan@amlogic.com>,
Stephen Boyd <sboyd@kernel.org>,
Michael Turquette <mturquette@baylibre.com>,
linux-usb@vger.kernel.org, Yue Wang <yue.wang@amlogic.com>,
Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
Liang Yang <liang.yang@amlogic.com>,
Qiufang Dai <qiufang.dai@amlogic.com>,
Xingyu Chen <xingyu.chen@amlogic.com>,
Carlo Caione <carlo@caione.org>,
linux-amlogic@lists.infradead.org,
linux-arm-kernel@lists.infradead.org,
Jian Hu <jian.hu@amlogic.com>
Subject: Re: [PATCH v2 3/6] phy: amlogic: Add Amlogic A1 USB2 PHY Driver
Date: Wed, 18 Dec 2019 14:17:05 +0100 [thread overview]
Message-ID: <4cbc4216-4f1b-dab2-fccd-4ece7cfedb77@baylibre.com> (raw)
In-Reply-To: <1576636944-196192-4-git-send-email-hanjie.lin@amlogic.com>
Hi,
On 18/12/2019 03:42, Hanjie Lin wrote:
> This adds support for the USB2 PHY found in the Amlogic A1 SoC Family.
>
> It supports host mode only.
>
> Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com>
> Signed-off-by: Yue Wang <yue.wang@amlogic.com>
> ---
> drivers/phy/amlogic/phy-meson-g12a-usb2.c | 102 ++++++++++++++++++++++--------
> 1 file changed, 74 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/phy/amlogic/phy-meson-g12a-usb2.c b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
> index 9065ffc..2c242d3 100644
> --- a/drivers/phy/amlogic/phy-meson-g12a-usb2.c
> +++ b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
> @@ -146,11 +146,18 @@
> #define RESET_COMPLETE_TIME 1000
> #define PLL_RESET_COMPLETE_TIME 100
>
> +enum {
> + MESON_USB2_PHY_VERSION_10 = 0,
> + MESON_USB2_PHY_VERSION_11,
Are these the real "versions" of the phy or it's made up ?
> + MESON_USB2_PHY_VERSION_COUNT,
> +};
> +
> struct phy_meson_g12a_usb2_priv {
> struct device *dev;
> struct regmap *regmap;
> struct clk *clk;
> struct reset_control *reset;
> + int phy_version;
> };
>
> static const struct regmap_config phy_meson_g12a_usb2_regmap_conf = {
> @@ -192,18 +199,33 @@ static int phy_meson_g12a_usb2_init(struct phy *phy)
> FIELD_PREP(PHY_CTRL_R17_MPLL_FILTER_PVT2, 2) |
> FIELD_PREP(PHY_CTRL_R17_MPLL_FILTER_PVT1, 9));
>
> - regmap_write(priv->regmap, PHY_CTRL_R18,
> - FIELD_PREP(PHY_CTRL_R18_MPLL_LKW_SEL, 1) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_LK_W, 9) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_LK_S, 0x27) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_PFD_GAIN, 1) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_ROU, 7) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_DATA_SEL, 3) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_BIAS_ADJ, 1) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_BB_MODE, 0) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_ALPHA, 3) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_ADJ_LDO, 1) |
> - PHY_CTRL_R18_MPLL_ACG_RANGE);
> + if (priv->phy_version == MESON_USB2_PHY_VERSION_10)
> + regmap_write(priv->regmap, PHY_CTRL_R18,
> + FIELD_PREP(PHY_CTRL_R18_MPLL_LKW_SEL, 1) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_LK_W, 9) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_LK_S, 0x27) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_PFD_GAIN, 1) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_ROU, 7) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_DATA_SEL, 3) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_BIAS_ADJ, 1) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_BB_MODE, 0) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_ALPHA, 3) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_ADJ_LDO, 1) |
> + PHY_CTRL_R18_MPLL_ACG_RANGE);
> + else if (priv->phy_version == MESON_USB2_PHY_VERSION_11)
> + regmap_write(priv->regmap, PHY_CTRL_R18,
> + FIELD_PREP(PHY_CTRL_R18_MPLL_LKW_SEL, 1) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_LK_W, 9) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_LK_S, 0x27) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_PFD_GAIN, 1) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_ROU, 7) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_DATA_SEL, 3) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_BIAS_ADJ, 1) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_BB_MODE, 0) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_ALPHA, 3) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_ADJ_LDO, 1) |
> + PHY_CTRL_R18_MPLL_ACG_RANGE |
> + PHY_CTRL_R18_MPLL_DCO_CLK_SEL);
The only difference is PHY_CTRL_R18_MPLL_ACG_RANGE | PHY_CTRL_R18_MPLL_DCO_CLK_SEL,
you can easily simplify the code here by using a temp variable.
>
> udelay(PLL_RESET_COMPLETE_TIME);
>
> @@ -227,13 +249,24 @@ static int phy_meson_g12a_usb2_init(struct phy *phy)
> FIELD_PREP(PHY_CTRL_R20_USB2_BGR_VREF_4_0, 0) |
> FIELD_PREP(PHY_CTRL_R20_USB2_BGR_DBG_1_0, 0));
>
> - regmap_write(priv->regmap, PHY_CTRL_R4,
> - FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_7_0, 0xf) |
> - FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_15_8, 0xf) |
> - FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_23_16, 0xf) |
> - PHY_CTRL_R4_TEST_BYPASS_MODE_EN |
> - FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_1_0, 0) |
> - FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_3_2, 0));
> + if (priv->phy_version == MESON_USB2_PHY_VERSION_10)
> + regmap_write(priv->regmap, PHY_CTRL_R4,
> + FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_7_0, 0xf) |
> + FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_15_8, 0xf) |
> + FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_23_16, 0xf) |
> + PHY_CTRL_R4_TEST_BYPASS_MODE_EN |
> + FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_1_0, 0) |
> + FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_3_2, 0));
> + else if (priv->phy_version == MESON_USB2_PHY_VERSION_11) {
> + regmap_write(priv->regmap, PHY_CTRL_R21,
> + PHY_CTRL_R21_USB2_CAL_ACK_EN |
> + PHY_CTRL_R21_USB2_TX_STRG_PD |
> + FIELD_PREP(PHY_CTRL_R21_USB2_OTG_ACA_TRIM_1_0, 2));
> +
> + /* Analog Settings */
> + regmap_write(priv->regmap, PHY_CTRL_R13,
> + FIELD_PREP(PHY_CTRL_R13_MIN_COUNT_FOR_SYNC_DET, 7));
> + }
>
> /* Tuning Disconnect Threshold */
> regmap_write(priv->regmap, PHY_CTRL_R3,
> @@ -241,11 +274,13 @@ static int phy_meson_g12a_usb2_init(struct phy *phy)
> FIELD_PREP(PHY_CTRL_R3_HSDIC_REF, 1) |
> FIELD_PREP(PHY_CTRL_R3_DISC_THRESH, 3));
>
> - /* Analog Settings */
> - regmap_write(priv->regmap, PHY_CTRL_R14, 0);
> - regmap_write(priv->regmap, PHY_CTRL_R13,
> - PHY_CTRL_R13_UPDATE_PMA_SIGNALS |
> - FIELD_PREP(PHY_CTRL_R13_MIN_COUNT_FOR_SYNC_DET, 7));
> + if (priv->phy_version == MESON_USB2_PHY_VERSION_10) {
> + /* Analog Settings */
> + regmap_write(priv->regmap, PHY_CTRL_R14, 0);
> + regmap_write(priv->regmap, PHY_CTRL_R13,
> + PHY_CTRL_R13_UPDATE_PMA_SIGNALS |
> + FIELD_PREP(PHY_CTRL_R13_MIN_COUNT_FOR_SYNC_DET, 7));
> + }
>
> return 0;
> }
> @@ -271,6 +306,7 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
> struct resource *res;
> struct phy_meson_g12a_usb2_priv *priv;
> struct phy *phy;
> + struct device_node *np = dev->of_node;
> void __iomem *base;
> int ret;
>
> @@ -286,14 +322,23 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
> if (IS_ERR(base))
> return PTR_ERR(base);
>
> + if (of_device_is_compatible(np, "amlogic,g12a-usb2-phy"))
> + priv->phy_version = MESON_USB2_PHY_VERSION_10;
> + else if (of_device_is_compatible(np, "amlogic,a1-usb2-phy"))
> + priv->phy_version = MESON_USB2_PHY_VERSION_11;
> + else
> + return -EINVAL;
Please use of_device_get_match_data() and a match data for each compatible instead.
> +
> priv->regmap = devm_regmap_init_mmio(dev, base,
> &phy_meson_g12a_usb2_regmap_conf);
> if (IS_ERR(priv->regmap))
> return PTR_ERR(priv->regmap);
>
> - priv->clk = devm_clk_get(dev, "xtal");
> - if (IS_ERR(priv->clk))
> - return PTR_ERR(priv->clk);
> + if (priv->phy_version == MESON_USB2_PHY_VERSION_10) {
> + priv->clk = devm_clk_get(dev, "xtal");
> + if (IS_ERR(priv->clk))
> + return PTR_ERR(priv->clk);
> + }
>
> priv->reset = devm_reset_control_get(dev, "phy");
> if (IS_ERR(priv->reset))
> @@ -322,7 +367,8 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
>
> static const struct of_device_id phy_meson_g12a_usb2_of_match[] = {
> { .compatible = "amlogic,g12a-usb2-phy", },
> - { },
> + { .compatible = "amlogic,a1-usb2-phy", },
> + { /* Sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, phy_meson_g12a_usb2_of_match);
>
>
Thanks,
Neil
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Neil Armstrong <narmstrong@baylibre.com>
To: Hanjie Lin <hanjie.lin@amlogic.com>,
Jerome Brunet <jbrunet@baylibre.com>,
Rob Herring <robh@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Felipe Balbi <felipe.balbi@linux.intel.com>,
Kevin Hilman <khilman@baylibre.com>
Cc: devicetree@vger.kernel.org, Victor Wan <victor.wan@amlogic.com>,
Jianxin Pan <jianxin.pan@amlogic.com>,
Stephen Boyd <sboyd@kernel.org>,
Michael Turquette <mturquette@baylibre.com>,
linux-usb@vger.kernel.org, Yue Wang <yue.wang@amlogic.com>,
Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
Liang Yang <liang.yang@amlogic.com>,
Qiufang Dai <qiufang.dai@amlogic.com>,
Xingyu Chen <xingyu.chen@amlogic.com>,
Carlo Caione <carlo@caione.org>,
linux-amlogic@lists.infradead.org,
linux-arm-kernel@lists.infradead.org,
Jian Hu <jian.hu@amlogic.com>
Subject: Re: [PATCH v2 3/6] phy: amlogic: Add Amlogic A1 USB2 PHY Driver
Date: Wed, 18 Dec 2019 14:17:05 +0100 [thread overview]
Message-ID: <4cbc4216-4f1b-dab2-fccd-4ece7cfedb77@baylibre.com> (raw)
In-Reply-To: <1576636944-196192-4-git-send-email-hanjie.lin@amlogic.com>
Hi,
On 18/12/2019 03:42, Hanjie Lin wrote:
> This adds support for the USB2 PHY found in the Amlogic A1 SoC Family.
>
> It supports host mode only.
>
> Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com>
> Signed-off-by: Yue Wang <yue.wang@amlogic.com>
> ---
> drivers/phy/amlogic/phy-meson-g12a-usb2.c | 102 ++++++++++++++++++++++--------
> 1 file changed, 74 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/phy/amlogic/phy-meson-g12a-usb2.c b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
> index 9065ffc..2c242d3 100644
> --- a/drivers/phy/amlogic/phy-meson-g12a-usb2.c
> +++ b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
> @@ -146,11 +146,18 @@
> #define RESET_COMPLETE_TIME 1000
> #define PLL_RESET_COMPLETE_TIME 100
>
> +enum {
> + MESON_USB2_PHY_VERSION_10 = 0,
> + MESON_USB2_PHY_VERSION_11,
Are these the real "versions" of the phy or it's made up ?
> + MESON_USB2_PHY_VERSION_COUNT,
> +};
> +
> struct phy_meson_g12a_usb2_priv {
> struct device *dev;
> struct regmap *regmap;
> struct clk *clk;
> struct reset_control *reset;
> + int phy_version;
> };
>
> static const struct regmap_config phy_meson_g12a_usb2_regmap_conf = {
> @@ -192,18 +199,33 @@ static int phy_meson_g12a_usb2_init(struct phy *phy)
> FIELD_PREP(PHY_CTRL_R17_MPLL_FILTER_PVT2, 2) |
> FIELD_PREP(PHY_CTRL_R17_MPLL_FILTER_PVT1, 9));
>
> - regmap_write(priv->regmap, PHY_CTRL_R18,
> - FIELD_PREP(PHY_CTRL_R18_MPLL_LKW_SEL, 1) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_LK_W, 9) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_LK_S, 0x27) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_PFD_GAIN, 1) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_ROU, 7) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_DATA_SEL, 3) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_BIAS_ADJ, 1) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_BB_MODE, 0) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_ALPHA, 3) |
> - FIELD_PREP(PHY_CTRL_R18_MPLL_ADJ_LDO, 1) |
> - PHY_CTRL_R18_MPLL_ACG_RANGE);
> + if (priv->phy_version == MESON_USB2_PHY_VERSION_10)
> + regmap_write(priv->regmap, PHY_CTRL_R18,
> + FIELD_PREP(PHY_CTRL_R18_MPLL_LKW_SEL, 1) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_LK_W, 9) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_LK_S, 0x27) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_PFD_GAIN, 1) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_ROU, 7) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_DATA_SEL, 3) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_BIAS_ADJ, 1) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_BB_MODE, 0) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_ALPHA, 3) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_ADJ_LDO, 1) |
> + PHY_CTRL_R18_MPLL_ACG_RANGE);
> + else if (priv->phy_version == MESON_USB2_PHY_VERSION_11)
> + regmap_write(priv->regmap, PHY_CTRL_R18,
> + FIELD_PREP(PHY_CTRL_R18_MPLL_LKW_SEL, 1) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_LK_W, 9) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_LK_S, 0x27) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_PFD_GAIN, 1) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_ROU, 7) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_DATA_SEL, 3) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_BIAS_ADJ, 1) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_BB_MODE, 0) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_ALPHA, 3) |
> + FIELD_PREP(PHY_CTRL_R18_MPLL_ADJ_LDO, 1) |
> + PHY_CTRL_R18_MPLL_ACG_RANGE |
> + PHY_CTRL_R18_MPLL_DCO_CLK_SEL);
The only difference is PHY_CTRL_R18_MPLL_ACG_RANGE | PHY_CTRL_R18_MPLL_DCO_CLK_SEL,
you can easily simplify the code here by using a temp variable.
>
> udelay(PLL_RESET_COMPLETE_TIME);
>
> @@ -227,13 +249,24 @@ static int phy_meson_g12a_usb2_init(struct phy *phy)
> FIELD_PREP(PHY_CTRL_R20_USB2_BGR_VREF_4_0, 0) |
> FIELD_PREP(PHY_CTRL_R20_USB2_BGR_DBG_1_0, 0));
>
> - regmap_write(priv->regmap, PHY_CTRL_R4,
> - FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_7_0, 0xf) |
> - FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_15_8, 0xf) |
> - FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_23_16, 0xf) |
> - PHY_CTRL_R4_TEST_BYPASS_MODE_EN |
> - FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_1_0, 0) |
> - FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_3_2, 0));
> + if (priv->phy_version == MESON_USB2_PHY_VERSION_10)
> + regmap_write(priv->regmap, PHY_CTRL_R4,
> + FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_7_0, 0xf) |
> + FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_15_8, 0xf) |
> + FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_23_16, 0xf) |
> + PHY_CTRL_R4_TEST_BYPASS_MODE_EN |
> + FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_1_0, 0) |
> + FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_3_2, 0));
> + else if (priv->phy_version == MESON_USB2_PHY_VERSION_11) {
> + regmap_write(priv->regmap, PHY_CTRL_R21,
> + PHY_CTRL_R21_USB2_CAL_ACK_EN |
> + PHY_CTRL_R21_USB2_TX_STRG_PD |
> + FIELD_PREP(PHY_CTRL_R21_USB2_OTG_ACA_TRIM_1_0, 2));
> +
> + /* Analog Settings */
> + regmap_write(priv->regmap, PHY_CTRL_R13,
> + FIELD_PREP(PHY_CTRL_R13_MIN_COUNT_FOR_SYNC_DET, 7));
> + }
>
> /* Tuning Disconnect Threshold */
> regmap_write(priv->regmap, PHY_CTRL_R3,
> @@ -241,11 +274,13 @@ static int phy_meson_g12a_usb2_init(struct phy *phy)
> FIELD_PREP(PHY_CTRL_R3_HSDIC_REF, 1) |
> FIELD_PREP(PHY_CTRL_R3_DISC_THRESH, 3));
>
> - /* Analog Settings */
> - regmap_write(priv->regmap, PHY_CTRL_R14, 0);
> - regmap_write(priv->regmap, PHY_CTRL_R13,
> - PHY_CTRL_R13_UPDATE_PMA_SIGNALS |
> - FIELD_PREP(PHY_CTRL_R13_MIN_COUNT_FOR_SYNC_DET, 7));
> + if (priv->phy_version == MESON_USB2_PHY_VERSION_10) {
> + /* Analog Settings */
> + regmap_write(priv->regmap, PHY_CTRL_R14, 0);
> + regmap_write(priv->regmap, PHY_CTRL_R13,
> + PHY_CTRL_R13_UPDATE_PMA_SIGNALS |
> + FIELD_PREP(PHY_CTRL_R13_MIN_COUNT_FOR_SYNC_DET, 7));
> + }
>
> return 0;
> }
> @@ -271,6 +306,7 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
> struct resource *res;
> struct phy_meson_g12a_usb2_priv *priv;
> struct phy *phy;
> + struct device_node *np = dev->of_node;
> void __iomem *base;
> int ret;
>
> @@ -286,14 +322,23 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
> if (IS_ERR(base))
> return PTR_ERR(base);
>
> + if (of_device_is_compatible(np, "amlogic,g12a-usb2-phy"))
> + priv->phy_version = MESON_USB2_PHY_VERSION_10;
> + else if (of_device_is_compatible(np, "amlogic,a1-usb2-phy"))
> + priv->phy_version = MESON_USB2_PHY_VERSION_11;
> + else
> + return -EINVAL;
Please use of_device_get_match_data() and a match data for each compatible instead.
> +
> priv->regmap = devm_regmap_init_mmio(dev, base,
> &phy_meson_g12a_usb2_regmap_conf);
> if (IS_ERR(priv->regmap))
> return PTR_ERR(priv->regmap);
>
> - priv->clk = devm_clk_get(dev, "xtal");
> - if (IS_ERR(priv->clk))
> - return PTR_ERR(priv->clk);
> + if (priv->phy_version == MESON_USB2_PHY_VERSION_10) {
> + priv->clk = devm_clk_get(dev, "xtal");
> + if (IS_ERR(priv->clk))
> + return PTR_ERR(priv->clk);
> + }
>
> priv->reset = devm_reset_control_get(dev, "phy");
> if (IS_ERR(priv->reset))
> @@ -322,7 +367,8 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
>
> static const struct of_device_id phy_meson_g12a_usb2_of_match[] = {
> { .compatible = "amlogic,g12a-usb2-phy", },
> - { },
> + { .compatible = "amlogic,a1-usb2-phy", },
> + { /* Sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, phy_meson_g12a_usb2_of_match);
>
>
Thanks,
Neil
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
next prev parent reply other threads:[~2019-12-18 13:17 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-18 2:42 [PATCH v2 0/6] arm64: meson: Add support for USB on Amlogic A1 Hanjie Lin
2019-12-18 2:42 ` Hanjie Lin
2019-12-18 2:42 ` Hanjie Lin
2019-12-18 2:42 ` [PATCH v2 1/6] dt-bindings: phy: Add Amlogic A1 USB2 PHY Bindings Hanjie Lin
2019-12-18 2:42 ` Hanjie Lin
2019-12-18 2:42 ` Hanjie Lin
2019-12-18 2:42 ` [PATCH v2 2/6] dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings Hanjie Lin
2019-12-18 2:42 ` Hanjie Lin
2019-12-18 2:42 ` Hanjie Lin
2019-12-18 13:13 ` Neil Armstrong
2019-12-18 13:13 ` Neil Armstrong
2019-12-18 13:13 ` Neil Armstrong
2019-12-19 8:38 ` Hanjie Lin
2019-12-19 8:38 ` Hanjie Lin
2019-12-19 8:38 ` Hanjie Lin
2019-12-18 2:42 ` [PATCH v2 3/6] phy: amlogic: Add Amlogic A1 USB2 PHY Driver Hanjie Lin
2019-12-18 2:42 ` Hanjie Lin
2019-12-18 2:42 ` Hanjie Lin
2019-12-18 13:17 ` Neil Armstrong [this message]
2019-12-18 13:17 ` Neil Armstrong
2019-12-18 13:17 ` Neil Armstrong
2019-12-19 9:48 ` Hanjie Lin
2019-12-19 9:48 ` Hanjie Lin
2019-12-19 9:48 ` Hanjie Lin
2019-12-19 10:12 ` Neil Armstrong
2019-12-19 10:12 ` Neil Armstrong
2019-12-19 10:12 ` Neil Armstrong
2019-12-20 7:38 ` Hanjie Lin
2019-12-20 7:38 ` Hanjie Lin
2019-12-20 7:38 ` Hanjie Lin
2019-12-18 2:42 ` [PATCH v2 4/6] usb: dwc3: Add Amlogic A1 DWC3 glue Hanjie Lin
2019-12-18 2:42 ` Hanjie Lin
2019-12-18 2:42 ` Hanjie Lin
2019-12-18 13:23 ` Neil Armstrong
2019-12-18 13:23 ` Neil Armstrong
2019-12-18 13:23 ` Neil Armstrong
2019-12-19 9:51 ` Hanjie Lin
2019-12-19 9:51 ` Hanjie Lin
2019-12-19 9:51 ` Hanjie Lin
2019-12-18 2:42 ` [PATCH v2 5/6] arm64: dts: meson: a1: Enable USB2 PHY Hanjie Lin
2019-12-18 2:42 ` Hanjie Lin
2019-12-18 2:42 ` Hanjie Lin
2019-12-18 2:42 ` [PATCH v2 6/6] arm64: dts: meson: a1: Enable DWC3 controller Hanjie Lin
2019-12-18 2:42 ` Hanjie Lin
2019-12-18 2:42 ` Hanjie Lin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4cbc4216-4f1b-dab2-fccd-4ece7cfedb77@baylibre.com \
--to=narmstrong@baylibre.com \
--cc=carlo@caione.org \
--cc=devicetree@vger.kernel.org \
--cc=felipe.balbi@linux.intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=hanjie.lin@amlogic.com \
--cc=jbrunet@baylibre.com \
--cc=jian.hu@amlogic.com \
--cc=jianxin.pan@amlogic.com \
--cc=khilman@baylibre.com \
--cc=liang.yang@amlogic.com \
--cc=linux-amlogic@lists.infradead.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-usb@vger.kernel.org \
--cc=martin.blumenstingl@googlemail.com \
--cc=mturquette@baylibre.com \
--cc=qiufang.dai@amlogic.com \
--cc=robh@kernel.org \
--cc=sboyd@kernel.org \
--cc=victor.wan@amlogic.com \
--cc=xingyu.chen@amlogic.com \
--cc=yue.wang@amlogic.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.