devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] usb: phy: mxs: Add DT bindings to configure TX settings
       [not found] ` <1457040571-7775-1-git-send-email-jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
@ 2016-03-15 16:54   ` Jaret Cantu
       [not found]     ` <1458060853-15115-1-git-send-email-jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Jaret Cantu @ 2016-03-15 16:54 UTC (permalink / raw)
  To: linux-usb-u79uwXL29TY76Z2rM5mHXA, felipe.balbi-VuQAYsv1563Yd54FQh9/CA
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, hzpeterchen-Re5JQEeQqe8AvxtiuMwx3w

The TX settings can be calibrated for particular hardware.  The
phy is reset by Linux, so this cannot be handled by the bootloader.

The TRM mentions that the maximum resistance should be used for the
DN/DP calibration in order to pass USB certification.

The values for the TX registers are poorly described in the TRM.
The meanings of the register values were taken from another
Freescale-provided document:
https://community.freescale.com/message/566147#comment-566912

Signed-off-by: Jaret Cantu <jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
---
v2. Copying devicetree list
    Removed prettifying extra whitespace
    Removed unnecessary register rewrite on resume
    Use min and max constants for clarity

 .../devicetree/bindings/phy/mxs-usb-phy.txt        |   10 ++++
 drivers/usb/phy/phy-mxs-usb.c                      |   58 ++++++++++++++++++++
 2 files changed, 68 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
index 379b84a..2f11e27 100644
--- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
@@ -12,6 +12,16 @@ Required properties:
 - interrupts: Should contain phy interrupt
 - fsl,anatop: phandle for anatop register, it is only for imx6 SoC series
 
+Optional properties:
+- fsl,tx-cal-45-dn: Integer [30-55]. Resistance (in ohms) of switchable
+  high-speed trimming resistor connected in parallel with the 45 ohm resistor
+  that terminates the DN output signal. Default: 45
+- fsl,tx-cal-45-dp: Integer [30-55]. Resistance (in ohms) of switchable
+  high-speed trimming resistor connected in parallel with the 45 ohm resistor
+  that terminates the DP output signal. Default: 45
+- fsl,tx-d-cal: Integer [79-119]. Current trimming value (as a percentage) of
+  the 17.78mA TX reference current. Default: 100
+
 Example:
 usbphy1: usbphy@020c9000 {
 	compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index 00bfea0..c9a8f3c 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -27,6 +27,7 @@
 #define DRIVER_NAME "mxs_phy"
 
 #define HW_USBPHY_PWD				0x00
+#define HW_USBPHY_TX				0x10
 #define HW_USBPHY_CTRL				0x30
 #define HW_USBPHY_CTRL_SET			0x34
 #define HW_USBPHY_CTRL_CLR			0x38
@@ -38,6 +39,10 @@
 #define HW_USBPHY_IP_SET			0x94
 #define HW_USBPHY_IP_CLR			0x98
 
+#define GM_USBPHY_TX_TXCAL45DP(x)            (((x) & 0xf) << 16)
+#define GM_USBPHY_TX_TXCAL45DN(x)            (((x) & 0xf) << 8)
+#define GM_USBPHY_TX_D_CAL(x)                (((x) & 0xf) << 0)
+
 #define BM_USBPHY_CTRL_SFTRST			BIT(31)
 #define BM_USBPHY_CTRL_CLKGATE			BIT(30)
 #define BM_USBPHY_CTRL_OTG_ID_VALUE		BIT(27)
@@ -115,6 +120,12 @@
  */
 #define MXS_PHY_NEED_IP_FIX			BIT(3)
 
+/* Minimum and maximum values for device tree entries */
+#define MXS_PHY_TX_CAL45_MIN			30
+#define MXS_PHY_TX_CAL45_MAX			55
+#define MXS_PHY_TX_D_CAL_MIN			79
+#define MXS_PHY_TX_D_CAL_MAX			119
+
 struct mxs_phy_data {
 	unsigned int flags;
 };
@@ -164,6 +175,8 @@ struct mxs_phy {
 	const struct mxs_phy_data *data;
 	struct regmap *regmap_anatop;
 	int port_id;
+	u32 tx_reg_set;
+	u32 tx_reg_mask;
 };
 
 static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy)
@@ -185,6 +198,20 @@ static void mxs_phy_clock_switch_delay(void)
 	usleep_range(300, 400);
 }
 
+static void mxs_phy_tx_init(struct mxs_phy *mxs_phy)
+{
+	void __iomem *base = mxs_phy->phy.io_priv;
+	u32 phytx;
+
+	/* Update TX register if there is anything to write */
+	if (mxs_phy->tx_reg_mask) {
+		phytx = readl(base + HW_USBPHY_TX);
+		phytx &= ~mxs_phy->tx_reg_mask;
+		phytx |= mxs_phy->tx_reg_set;
+		writel(phytx, base + HW_USBPHY_TX);
+	}
+}
+
 static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
 {
 	int ret;
@@ -214,6 +241,8 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
 	if (mxs_phy->data->flags & MXS_PHY_NEED_IP_FIX)
 		writel(BM_USBPHY_IP_FIX, base + HW_USBPHY_IP_SET);
 
+	mxs_phy_tx_init(mxs_phy);
+
 	return 0;
 }
 
@@ -459,6 +488,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
 	int ret;
 	const struct of_device_id *of_id;
 	struct device_node *np = pdev->dev.of_node;
+	u32 val;
 
 	of_id = of_match_device(mxs_phy_dt_ids, &pdev->dev);
 	if (!of_id)
@@ -491,6 +521,34 @@ static int mxs_phy_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* Precompute which bits of the TX register are to be updated, if any */
+	if (!of_property_read_u32(np, "fsl,tx-cal-45-dn", &val) &&
+	    val >= MXS_PHY_TX_CAL45_MIN && val <= MXS_PHY_TX_CAL45_MAX) {
+		/* scale to 4-bit value */
+		val = (MXS_PHY_TX_CAL45_MAX - val) * 0xF
+			/ (MXS_PHY_TX_CAL45_MAX - MXS_PHY_TX_CAL45_MIN);
+		mxs_phy->tx_reg_mask |= GM_USBPHY_TX_TXCAL45DN(~0);
+		mxs_phy->tx_reg_set  |= GM_USBPHY_TX_TXCAL45DN(val);
+	}
+
+	if (!of_property_read_u32(np, "fsl,tx-cal-45-dp", &val) &&
+	    val >= MXS_PHY_TX_CAL45_MIN && val <= MXS_PHY_TX_CAL45_MAX) {
+		/* scale to 4-bit value */
+		val = (MXS_PHY_TX_CAL45_MAX - val) * 0xF
+			/ (MXS_PHY_TX_CAL45_MAX - MXS_PHY_TX_CAL45_MIN);
+		mxs_phy->tx_reg_mask |= GM_USBPHY_TX_TXCAL45DP(~0);
+		mxs_phy->tx_reg_set  |= GM_USBPHY_TX_TXCAL45DP(val);
+	}
+
+	if (!of_property_read_u32(np, "fsl,tx-d-cal", &val) &&
+	    val >= MXS_PHY_TX_D_CAL_MIN && val <= MXS_PHY_TX_D_CAL_MAX) {
+		/* scale to 4-bit value */
+		val = (MXS_PHY_TX_D_CAL_MAX - val) * 0xF
+			/ (MXS_PHY_TX_D_CAL_MAX - MXS_PHY_TX_D_CAL_MIN);
+		mxs_phy->tx_reg_mask |= GM_USBPHY_TX_D_CAL(~0);
+		mxs_phy->tx_reg_set  |= GM_USBPHY_TX_D_CAL(val);
+	}
+
 	ret = of_alias_get_id(np, "usbphy");
 	if (ret < 0)
 		dev_dbg(&pdev->dev, "failed to get alias id, errno %d\n", ret);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] usb: phy: mxs: Add DT bindings to configure TX settings
       [not found]     ` <1458060853-15115-1-git-send-email-jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
@ 2016-03-18 21:19       ` Rob Herring
  2016-03-21 16:32         ` [PATCH v3] " Jaret Cantu
  0 siblings, 1 reply; 17+ messages in thread
From: Rob Herring @ 2016-03-18 21:19 UTC (permalink / raw)
  To: Jaret Cantu
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
	felipe.balbi-VuQAYsv1563Yd54FQh9/CA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	hzpeterchen-Re5JQEeQqe8AvxtiuMwx3w

On Tue, Mar 15, 2016 at 12:54:13PM -0400, Jaret Cantu wrote:
> The TX settings can be calibrated for particular hardware.  The
> phy is reset by Linux, so this cannot be handled by the bootloader.
> 
> The TRM mentions that the maximum resistance should be used for the
> DN/DP calibration in order to pass USB certification.
> 
> The values for the TX registers are poorly described in the TRM.
> The meanings of the register values were taken from another
> Freescale-provided document:
> https://community.freescale.com/message/566147#comment-566912
> 
> Signed-off-by: Jaret Cantu <jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
> ---
> v2. Copying devicetree list
>     Removed prettifying extra whitespace
>     Removed unnecessary register rewrite on resume
>     Use min and max constants for clarity
> 
>  .../devicetree/bindings/phy/mxs-usb-phy.txt        |   10 ++++
>  drivers/usb/phy/phy-mxs-usb.c                      |   58 ++++++++++++++++++++
>  2 files changed, 68 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> index 379b84a..2f11e27 100644
> --- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> @@ -12,6 +12,16 @@ Required properties:
>  - interrupts: Should contain phy interrupt
>  - fsl,anatop: phandle for anatop register, it is only for imx6 SoC series
>  
> +Optional properties:
> +- fsl,tx-cal-45-dn: Integer [30-55]. Resistance (in ohms) of switchable
> +  high-speed trimming resistor connected in parallel with the 45 ohm resistor
> +  that terminates the DN output signal. Default: 45
> +- fsl,tx-cal-45-dp: Integer [30-55]. Resistance (in ohms) of switchable
> +  high-speed trimming resistor connected in parallel with the 45 ohm resistor
> +  that terminates the DP output signal. Default: 45

Add unit suffix (-ohms)

> +- fsl,tx-d-cal: Integer [79-119]. Current trimming value (as a percentage) of
> +  the 17.78mA TX reference current. Default: 100
> +
>  Example:
>  usbphy1: usbphy@020c9000 {
>  	compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v3] usb: phy: mxs: Add DT bindings to configure TX settings
  2016-03-18 21:19       ` Rob Herring
@ 2016-03-21 16:32         ` Jaret Cantu
       [not found]           ` <1458577947-12614-1-git-send-email-jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Jaret Cantu @ 2016-03-21 16:32 UTC (permalink / raw)
  To: linux-usb-u79uwXL29TY76Z2rM5mHXA, felipe.balbi-VuQAYsv1563Yd54FQh9/CA
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	hzpeterchen-Re5JQEeQqe8AvxtiuMwx3w, robh-DgEjT+Ai2ygdnm+yROfE0A

The TX settings can be calibrated for particular hardware.  The
phy is reset by Linux, so this cannot be handled by the bootloader.

The TRM mentions that the maximum resistance should be used for the
DN/DP calibration in order to pass USB certification.

The values for the TX registers are poorly described in the TRM.
The meanings of the register values were taken from another
Freescale-provided document:
https://community.freescale.com/message/566147#comment-566912

Signed-off-by: Jaret Cantu <jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
---
v3. Added unit suffix (-ohms) to tx-cal-45-d*

v2. Copying devicetree list
    Removed prettifying extra whitespace
    Removed unnecessary register rewrite on resume
    Use min and max constants for clarity

 .../devicetree/bindings/phy/mxs-usb-phy.txt        |   10 ++++
 drivers/usb/phy/phy-mxs-usb.c                      |   58 ++++++++++++++++++++
 2 files changed, 68 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
index 379b84a..1d25b04 100644
--- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
@@ -12,6 +12,16 @@ Required properties:
 - interrupts: Should contain phy interrupt
 - fsl,anatop: phandle for anatop register, it is only for imx6 SoC series
 
+Optional properties:
+- fsl,tx-cal-45-dn-ohms: Integer [30-55]. Resistance (in ohms) of switchable
+  high-speed trimming resistor connected in parallel with the 45 ohm resistor
+  that terminates the DN output signal. Default: 45
+- fsl,tx-cal-45-dp-ohms: Integer [30-55]. Resistance (in ohms) of switchable
+  high-speed trimming resistor connected in parallel with the 45 ohm resistor
+  that terminates the DP output signal. Default: 45
+- fsl,tx-d-cal: Integer [79-119]. Current trimming value (as a percentage) of
+  the 17.78mA TX reference current. Default: 100
+
 Example:
 usbphy1: usbphy@020c9000 {
 	compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index 00bfea0..6c6b12c 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -27,6 +27,7 @@
 #define DRIVER_NAME "mxs_phy"
 
 #define HW_USBPHY_PWD				0x00
+#define HW_USBPHY_TX				0x10
 #define HW_USBPHY_CTRL				0x30
 #define HW_USBPHY_CTRL_SET			0x34
 #define HW_USBPHY_CTRL_CLR			0x38
@@ -38,6 +39,10 @@
 #define HW_USBPHY_IP_SET			0x94
 #define HW_USBPHY_IP_CLR			0x98
 
+#define GM_USBPHY_TX_TXCAL45DP(x)            (((x) & 0xf) << 16)
+#define GM_USBPHY_TX_TXCAL45DN(x)            (((x) & 0xf) << 8)
+#define GM_USBPHY_TX_D_CAL(x)                (((x) & 0xf) << 0)
+
 #define BM_USBPHY_CTRL_SFTRST			BIT(31)
 #define BM_USBPHY_CTRL_CLKGATE			BIT(30)
 #define BM_USBPHY_CTRL_OTG_ID_VALUE		BIT(27)
@@ -115,6 +120,12 @@
  */
 #define MXS_PHY_NEED_IP_FIX			BIT(3)
 
+/* Minimum and maximum values for device tree entries */
+#define MXS_PHY_TX_CAL45_MIN			30
+#define MXS_PHY_TX_CAL45_MAX			55
+#define MXS_PHY_TX_D_CAL_MIN			79
+#define MXS_PHY_TX_D_CAL_MAX			119
+
 struct mxs_phy_data {
 	unsigned int flags;
 };
@@ -164,6 +175,8 @@ struct mxs_phy {
 	const struct mxs_phy_data *data;
 	struct regmap *regmap_anatop;
 	int port_id;
+	u32 tx_reg_set;
+	u32 tx_reg_mask;
 };
 
 static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy)
@@ -185,6 +198,20 @@ static void mxs_phy_clock_switch_delay(void)
 	usleep_range(300, 400);
 }
 
+static void mxs_phy_tx_init(struct mxs_phy *mxs_phy)
+{
+	void __iomem *base = mxs_phy->phy.io_priv;
+	u32 phytx;
+
+	/* Update TX register if there is anything to write */
+	if (mxs_phy->tx_reg_mask) {
+		phytx = readl(base + HW_USBPHY_TX);
+		phytx &= ~mxs_phy->tx_reg_mask;
+		phytx |= mxs_phy->tx_reg_set;
+		writel(phytx, base + HW_USBPHY_TX);
+	}
+}
+
 static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
 {
 	int ret;
@@ -214,6 +241,8 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
 	if (mxs_phy->data->flags & MXS_PHY_NEED_IP_FIX)
 		writel(BM_USBPHY_IP_FIX, base + HW_USBPHY_IP_SET);
 
+	mxs_phy_tx_init(mxs_phy);
+
 	return 0;
 }
 
@@ -459,6 +488,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
 	int ret;
 	const struct of_device_id *of_id;
 	struct device_node *np = pdev->dev.of_node;
+	u32 val;
 
 	of_id = of_match_device(mxs_phy_dt_ids, &pdev->dev);
 	if (!of_id)
@@ -491,6 +521,34 @@ static int mxs_phy_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* Precompute which bits of the TX register are to be updated, if any */
+	if (!of_property_read_u32(np, "fsl,tx-cal-45-dn-ohms", &val) &&
+	    val >= MXS_PHY_TX_CAL45_MIN && val <= MXS_PHY_TX_CAL45_MAX) {
+		/* scale to 4-bit value */
+		val = (MXS_PHY_TX_CAL45_MAX - val) * 0xF
+			/ (MXS_PHY_TX_CAL45_MAX - MXS_PHY_TX_CAL45_MIN);
+		mxs_phy->tx_reg_mask |= GM_USBPHY_TX_TXCAL45DN(~0);
+		mxs_phy->tx_reg_set  |= GM_USBPHY_TX_TXCAL45DN(val);
+	}
+
+	if (!of_property_read_u32(np, "fsl,tx-cal-45-dp-ohms", &val) &&
+	    val >= MXS_PHY_TX_CAL45_MIN && val <= MXS_PHY_TX_CAL45_MAX) {
+		/* scale to 4-bit value */
+		val = (MXS_PHY_TX_CAL45_MAX - val) * 0xF
+			/ (MXS_PHY_TX_CAL45_MAX - MXS_PHY_TX_CAL45_MIN);
+		mxs_phy->tx_reg_mask |= GM_USBPHY_TX_TXCAL45DP(~0);
+		mxs_phy->tx_reg_set  |= GM_USBPHY_TX_TXCAL45DP(val);
+	}
+
+	if (!of_property_read_u32(np, "fsl,tx-d-cal", &val) &&
+	    val >= MXS_PHY_TX_D_CAL_MIN && val <= MXS_PHY_TX_D_CAL_MAX) {
+		/* scale to 4-bit value */
+		val = (MXS_PHY_TX_D_CAL_MAX - val) * 0xF
+			/ (MXS_PHY_TX_D_CAL_MAX - MXS_PHY_TX_D_CAL_MIN);
+		mxs_phy->tx_reg_mask |= GM_USBPHY_TX_D_CAL(~0);
+		mxs_phy->tx_reg_set  |= GM_USBPHY_TX_D_CAL(val);
+	}
+
 	ret = of_alias_get_id(np, "usbphy");
 	if (ret < 0)
 		dev_dbg(&pdev->dev, "failed to get alias id, errno %d\n", ret);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] usb: phy: mxs: Add DT bindings to configure TX settings
       [not found]           ` <1458577947-12614-1-git-send-email-jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
@ 2016-03-23  1:53             ` Peter Chen
  2016-03-23  4:36             ` Peter Chen
  2016-03-23 14:54             ` Rob Herring
  2 siblings, 0 replies; 17+ messages in thread
From: Peter Chen @ 2016-03-23  1:53 UTC (permalink / raw)
  To: Jaret Cantu
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
	felipe.balbi-VuQAYsv1563Yd54FQh9/CA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A

On Tue, Mar 22, 2016 at 12:32 AM, Jaret Cantu <jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org> wrote:
> The TX settings can be calibrated for particular hardware.  The
> phy is reset by Linux, so this cannot be handled by the bootloader.
>
> The TRM mentions that the maximum resistance should be used for the
> DN/DP calibration in order to pass USB certification.
>
> The values for the TX registers are poorly described in the TRM.
> The meanings of the register values were taken from another
> Freescale-provided document:
> https://community.freescale.com/message/566147#comment-566912
>

I am checking with IC team about this value, and if this value can be
adapted for all SoCs
which use this PHY.  Felipe, please hold on queue this patch, thanks.

Peter

> Signed-off-by: Jaret Cantu <jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
> ---
> v3. Added unit suffix (-ohms) to tx-cal-45-d*
>
> v2. Copying devicetree list
>     Removed prettifying extra whitespace
>     Removed unnecessary register rewrite on resume
>     Use min and max constants for clarity
>
>  .../devicetree/bindings/phy/mxs-usb-phy.txt        |   10 ++++
>  drivers/usb/phy/phy-mxs-usb.c                      |   58 ++++++++++++++++++++
>  2 files changed, 68 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> index 379b84a..1d25b04 100644
> --- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> @@ -12,6 +12,16 @@ Required properties:
>  - interrupts: Should contain phy interrupt
>  - fsl,anatop: phandle for anatop register, it is only for imx6 SoC series
>
> +Optional properties:
> +- fsl,tx-cal-45-dn-ohms: Integer [30-55]. Resistance (in ohms) of switchable
> +  high-speed trimming resistor connected in parallel with the 45 ohm resistor
> +  that terminates the DN output signal. Default: 45
> +- fsl,tx-cal-45-dp-ohms: Integer [30-55]. Resistance (in ohms) of switchable
> +  high-speed trimming resistor connected in parallel with the 45 ohm resistor
> +  that terminates the DP output signal. Default: 45
> +- fsl,tx-d-cal: Integer [79-119]. Current trimming value (as a percentage) of
> +  the 17.78mA TX reference current. Default: 100
> +
>  Example:
>  usbphy1: usbphy@020c9000 {
>         compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> index 00bfea0..6c6b12c 100644
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -27,6 +27,7 @@
>  #define DRIVER_NAME "mxs_phy"
>
>  #define HW_USBPHY_PWD                          0x00
> +#define HW_USBPHY_TX                           0x10
>  #define HW_USBPHY_CTRL                         0x30
>  #define HW_USBPHY_CTRL_SET                     0x34
>  #define HW_USBPHY_CTRL_CLR                     0x38
> @@ -38,6 +39,10 @@
>  #define HW_USBPHY_IP_SET                       0x94
>  #define HW_USBPHY_IP_CLR                       0x98
>
> +#define GM_USBPHY_TX_TXCAL45DP(x)            (((x) & 0xf) << 16)
> +#define GM_USBPHY_TX_TXCAL45DN(x)            (((x) & 0xf) << 8)
> +#define GM_USBPHY_TX_D_CAL(x)                (((x) & 0xf) << 0)
> +
>  #define BM_USBPHY_CTRL_SFTRST                  BIT(31)
>  #define BM_USBPHY_CTRL_CLKGATE                 BIT(30)
>  #define BM_USBPHY_CTRL_OTG_ID_VALUE            BIT(27)
> @@ -115,6 +120,12 @@
>   */
>  #define MXS_PHY_NEED_IP_FIX                    BIT(3)
>
> +/* Minimum and maximum values for device tree entries */
> +#define MXS_PHY_TX_CAL45_MIN                   30
> +#define MXS_PHY_TX_CAL45_MAX                   55
> +#define MXS_PHY_TX_D_CAL_MIN                   79
> +#define MXS_PHY_TX_D_CAL_MAX                   119
> +
>  struct mxs_phy_data {
>         unsigned int flags;
>  };
> @@ -164,6 +175,8 @@ struct mxs_phy {
>         const struct mxs_phy_data *data;
>         struct regmap *regmap_anatop;
>         int port_id;
> +       u32 tx_reg_set;
> +       u32 tx_reg_mask;
>  };
>
>  static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy)
> @@ -185,6 +198,20 @@ static void mxs_phy_clock_switch_delay(void)
>         usleep_range(300, 400);
>  }
>
> +static void mxs_phy_tx_init(struct mxs_phy *mxs_phy)
> +{
> +       void __iomem *base = mxs_phy->phy.io_priv;
> +       u32 phytx;
> +
> +       /* Update TX register if there is anything to write */
> +       if (mxs_phy->tx_reg_mask) {
> +               phytx = readl(base + HW_USBPHY_TX);
> +               phytx &= ~mxs_phy->tx_reg_mask;
> +               phytx |= mxs_phy->tx_reg_set;
> +               writel(phytx, base + HW_USBPHY_TX);
> +       }
> +}
> +
>  static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
>  {
>         int ret;
> @@ -214,6 +241,8 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
>         if (mxs_phy->data->flags & MXS_PHY_NEED_IP_FIX)
>                 writel(BM_USBPHY_IP_FIX, base + HW_USBPHY_IP_SET);
>
> +       mxs_phy_tx_init(mxs_phy);
> +
>         return 0;
>  }
>
> @@ -459,6 +488,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
>         int ret;
>         const struct of_device_id *of_id;
>         struct device_node *np = pdev->dev.of_node;
> +       u32 val;
>
>         of_id = of_match_device(mxs_phy_dt_ids, &pdev->dev);
>         if (!of_id)
> @@ -491,6 +521,34 @@ static int mxs_phy_probe(struct platform_device *pdev)
>                 }
>         }
>
> +       /* Precompute which bits of the TX register are to be updated, if any */
> +       if (!of_property_read_u32(np, "fsl,tx-cal-45-dn-ohms", &val) &&
> +           val >= MXS_PHY_TX_CAL45_MIN && val <= MXS_PHY_TX_CAL45_MAX) {
> +               /* scale to 4-bit value */
> +               val = (MXS_PHY_TX_CAL45_MAX - val) * 0xF
> +                       / (MXS_PHY_TX_CAL45_MAX - MXS_PHY_TX_CAL45_MIN);
> +               mxs_phy->tx_reg_mask |= GM_USBPHY_TX_TXCAL45DN(~0);
> +               mxs_phy->tx_reg_set  |= GM_USBPHY_TX_TXCAL45DN(val);
> +       }
> +
> +       if (!of_property_read_u32(np, "fsl,tx-cal-45-dp-ohms", &val) &&
> +           val >= MXS_PHY_TX_CAL45_MIN && val <= MXS_PHY_TX_CAL45_MAX) {
> +               /* scale to 4-bit value */
> +               val = (MXS_PHY_TX_CAL45_MAX - val) * 0xF
> +                       / (MXS_PHY_TX_CAL45_MAX - MXS_PHY_TX_CAL45_MIN);
> +               mxs_phy->tx_reg_mask |= GM_USBPHY_TX_TXCAL45DP(~0);
> +               mxs_phy->tx_reg_set  |= GM_USBPHY_TX_TXCAL45DP(val);
> +       }
> +
> +       if (!of_property_read_u32(np, "fsl,tx-d-cal", &val) &&
> +           val >= MXS_PHY_TX_D_CAL_MIN && val <= MXS_PHY_TX_D_CAL_MAX) {
> +               /* scale to 4-bit value */
> +               val = (MXS_PHY_TX_D_CAL_MAX - val) * 0xF
> +                       / (MXS_PHY_TX_D_CAL_MAX - MXS_PHY_TX_D_CAL_MIN);
> +               mxs_phy->tx_reg_mask |= GM_USBPHY_TX_D_CAL(~0);
> +               mxs_phy->tx_reg_set  |= GM_USBPHY_TX_D_CAL(val);
> +       }
> +
>         ret = of_alias_get_id(np, "usbphy");
>         if (ret < 0)
>                 dev_dbg(&pdev->dev, "failed to get alias id, errno %d\n", ret);
> --
> 1.7.10.4
>



-- 
BR,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] usb: phy: mxs: Add DT bindings to configure TX settings
       [not found]           ` <1458577947-12614-1-git-send-email-jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
  2016-03-23  1:53             ` Peter Chen
@ 2016-03-23  4:36             ` Peter Chen
  2016-03-23 17:37               ` Jaret Cantu
  2016-03-23 14:54             ` Rob Herring
  2 siblings, 1 reply; 17+ messages in thread
From: Peter Chen @ 2016-03-23  4:36 UTC (permalink / raw)
  To: Jaret Cantu
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
	felipe.balbi-VuQAYsv1563Yd54FQh9/CA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A

On Mon, Mar 21, 2016 at 12:32:27PM -0400, Jaret Cantu wrote:
> The TX settings can be calibrated for particular hardware.  The
> phy is reset by Linux, so this cannot be handled by the bootloader.
> 
> The TRM mentions that the maximum resistance should be used for the
> DN/DP calibration in order to pass USB certification.
> 
> The values for the TX registers are poorly described in the TRM.
> The meanings of the register values were taken from another
> Freescale-provided document:
> https://community.freescale.com/message/566147#comment-566912
> 
> Signed-off-by: Jaret Cantu <jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
> ---
> v3. Added unit suffix (-ohms) to tx-cal-45-d*
> 
> v2. Copying devicetree list
>     Removed prettifying extra whitespace
>     Removed unnecessary register rewrite on resume
>     Use min and max constants for clarity
> 
>  .../devicetree/bindings/phy/mxs-usb-phy.txt        |   10 ++++
>  drivers/usb/phy/phy-mxs-usb.c                      |   58 ++++++++++++++++++++
>  2 files changed, 68 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> index 379b84a..1d25b04 100644
> --- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> @@ -12,6 +12,16 @@ Required properties:
>  - interrupts: Should contain phy interrupt
>  - fsl,anatop: phandle for anatop register, it is only for imx6 SoC series
>  
> +
> +	if (!of_property_read_u32(np, "fsl,tx-d-cal", &val) &&
> +	    val >= MXS_PHY_TX_D_CAL_MIN && val <= MXS_PHY_TX_D_CAL_MAX) {
> +		/* scale to 4-bit value */
> +		val = (MXS_PHY_TX_D_CAL_MAX - val) * 0xF
> +			/ (MXS_PHY_TX_D_CAL_MAX - MXS_PHY_TX_D_CAL_MIN);
> +		mxs_phy->tx_reg_mask |= GM_USBPHY_TX_D_CAL(~0);
> +		mxs_phy->tx_reg_set  |= GM_USBPHY_TX_D_CAL(val);
> +	}
> +

I have tested "tx-d-cal", but it seems incorrect according to the xls you
have provided, would you please check it again or am I wrong?

dts changes:

+&usbphy1 {
+       fsl,tx-d-cal = <109>;
+};
+
+&usbphy2 {
+       fsl,tx-d-cal = <106>;
+};
+

The phy1's tx-d-cal is 0x3, and phy2's tx-d-cal is 0x4 after PHY initialization, 
but according to xls, it should be 0x4 and 0x5.

-- 
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] usb: phy: mxs: Add DT bindings to configure TX settings
       [not found]           ` <1458577947-12614-1-git-send-email-jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
  2016-03-23  1:53             ` Peter Chen
  2016-03-23  4:36             ` Peter Chen
@ 2016-03-23 14:54             ` Rob Herring
  2 siblings, 0 replies; 17+ messages in thread
From: Rob Herring @ 2016-03-23 14:54 UTC (permalink / raw)
  To: Jaret Cantu
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
	felipe.balbi-VuQAYsv1563Yd54FQh9/CA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	hzpeterchen-Re5JQEeQqe8AvxtiuMwx3w

On Mon, Mar 21, 2016 at 12:32:27PM -0400, Jaret Cantu wrote:
> The TX settings can be calibrated for particular hardware.  The
> phy is reset by Linux, so this cannot be handled by the bootloader.
> 
> The TRM mentions that the maximum resistance should be used for the
> DN/DP calibration in order to pass USB certification.
> 
> The values for the TX registers are poorly described in the TRM.
> The meanings of the register values were taken from another
> Freescale-provided document:
> https://community.freescale.com/message/566147#comment-566912
> 
> Signed-off-by: Jaret Cantu <jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
> ---
> v3. Added unit suffix (-ohms) to tx-cal-45-d*
> 
> v2. Copying devicetree list
>     Removed prettifying extra whitespace
>     Removed unnecessary register rewrite on resume
>     Use min and max constants for clarity
> 
>  .../devicetree/bindings/phy/mxs-usb-phy.txt        |   10 ++++
>  drivers/usb/phy/phy-mxs-usb.c                      |   58 ++++++++++++++++++++
>  2 files changed, 68 insertions(+)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] usb: phy: mxs: Add DT bindings to configure TX settings
  2016-03-23  4:36             ` Peter Chen
@ 2016-03-23 17:37               ` Jaret Cantu
       [not found]                 ` <56F2D43D.1040404-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Jaret Cantu @ 2016-03-23 17:37 UTC (permalink / raw)
  To: Peter Chen
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
	felipe.balbi-VuQAYsv1563Yd54FQh9/CA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A

On 03/23/2016 12:36 AM, Peter Chen wrote:
> On Mon, Mar 21, 2016 at 12:32:27PM -0400, Jaret Cantu wrote:
>> The TX settings can be calibrated for particular hardware.  The
>> phy is reset by Linux, so this cannot be handled by the bootloader.
>>
>> The TRM mentions that the maximum resistance should be used for the
>> DN/DP calibration in order to pass USB certification.
>>
>> The values for the TX registers are poorly described in the TRM.
>> The meanings of the register values were taken from another
>> Freescale-provided document:
>> https://community.freescale.com/message/566147#comment-566912
>>
>> Signed-off-by: Jaret Cantu <jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
>> ---
>> v3. Added unit suffix (-ohms) to tx-cal-45-d*
>>
>> v2. Copying devicetree list
>>      Removed prettifying extra whitespace
>>      Removed unnecessary register rewrite on resume
>>      Use min and max constants for clarity
>>
>>   .../devicetree/bindings/phy/mxs-usb-phy.txt        |   10 ++++
>>   drivers/usb/phy/phy-mxs-usb.c                      |   58 ++++++++++++++++++++
>>   2 files changed, 68 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>> index 379b84a..1d25b04 100644
>> --- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>> +++ b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>> @@ -12,6 +12,16 @@ Required properties:
>>   - interrupts: Should contain phy interrupt
>>   - fsl,anatop: phandle for anatop register, it is only for imx6 SoC series
>>
>> +
>> +	if (!of_property_read_u32(np, "fsl,tx-d-cal", &val) &&
>> +	    val >= MXS_PHY_TX_D_CAL_MIN && val <= MXS_PHY_TX_D_CAL_MAX) {
>> +		/* scale to 4-bit value */
>> +		val = (MXS_PHY_TX_D_CAL_MAX - val) * 0xF
>> +			/ (MXS_PHY_TX_D_CAL_MAX - MXS_PHY_TX_D_CAL_MIN);
>> +		mxs_phy->tx_reg_mask |= GM_USBPHY_TX_D_CAL(~0);
>> +		mxs_phy->tx_reg_set  |= GM_USBPHY_TX_D_CAL(val);
>> +	}
>> +
>
> I have tested "tx-d-cal", but it seems incorrect according to the xls you
> have provided, would you please check it again or am I wrong?

Gah! You're right. Some of the D_CAL values need to be rounded up to 
match the xls.  And even then, the value for 86 still doesn't play nice. 
  I was really hoping to avoid using a table for these values.

The TXCALDP/DN values use a much simpler 1-to-1 scale across the 16 
possible register values and so are unaffected by a similar issue.  I 
rechecked their numbers just to be sure.

>
> dts changes:
>
> +&usbphy1 {
> +       fsl,tx-d-cal = <109>;
> +};
> +
> +&usbphy2 {
> +       fsl,tx-d-cal = <106>;
> +};
> +
>
> The phy1's tx-d-cal is 0x3, and phy2's tx-d-cal is 0x4 after PHY initialization,
> but according to xls, it should be 0x4 and 0x5.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] usb: phy: mxs: Add DT bindings to configure TX settings
       [not found]                 ` <56F2D43D.1040404-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
@ 2016-03-23 18:17                   ` Jaret Cantu
       [not found]                     ` <56F2DDB7.7070700-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Jaret Cantu @ 2016-03-23 18:17 UTC (permalink / raw)
  To: Peter Chen
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
	felipe.balbi-VuQAYsv1563Yd54FQh9/CA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A

On 03/23/2016 01:37 PM, Jaret Cantu wrote:
> On 03/23/2016 12:36 AM, Peter Chen wrote:
>> On Mon, Mar 21, 2016 at 12:32:27PM -0400, Jaret Cantu wrote:
>>> The TX settings can be calibrated for particular hardware.  The
>>> phy is reset by Linux, so this cannot be handled by the bootloader.
>>>
>>> The TRM mentions that the maximum resistance should be used for the
>>> DN/DP calibration in order to pass USB certification.
>>>
>>> The values for the TX registers are poorly described in the TRM.
>>> The meanings of the register values were taken from another
>>> Freescale-provided document:
>>> https://community.freescale.com/message/566147#comment-566912
>>>
>>> Signed-off-by: Jaret Cantu <jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
>>> ---
>>> v3. Added unit suffix (-ohms) to tx-cal-45-d*
>>>
>>> v2. Copying devicetree list
>>>      Removed prettifying extra whitespace
>>>      Removed unnecessary register rewrite on resume
>>>      Use min and max constants for clarity
>>>
>>>   .../devicetree/bindings/phy/mxs-usb-phy.txt        |   10 ++++
>>>   drivers/usb/phy/phy-mxs-usb.c                      |   58
>>> ++++++++++++++++++++
>>>   2 files changed, 68 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>> b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>> index 379b84a..1d25b04 100644
>>> --- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>> +++ b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>> @@ -12,6 +12,16 @@ Required properties:
>>>   - interrupts: Should contain phy interrupt
>>>   - fsl,anatop: phandle for anatop register, it is only for imx6 SoC
>>> series
>>>
>>> +
>>> +    if (!of_property_read_u32(np, "fsl,tx-d-cal", &val) &&
>>> +        val >= MXS_PHY_TX_D_CAL_MIN && val <= MXS_PHY_TX_D_CAL_MAX) {
>>> +        /* scale to 4-bit value */
>>> +        val = (MXS_PHY_TX_D_CAL_MAX - val) * 0xF
>>> +            / (MXS_PHY_TX_D_CAL_MAX - MXS_PHY_TX_D_CAL_MIN);
>>> +        mxs_phy->tx_reg_mask |= GM_USBPHY_TX_D_CAL(~0);
>>> +        mxs_phy->tx_reg_set  |= GM_USBPHY_TX_D_CAL(val);
>>> +    }
>>> +
>>
>> I have tested "tx-d-cal", but it seems incorrect according to the xls you
>> have provided, would you please check it again or am I wrong?
>
> Gah! You're right. Some of the D_CAL values need to be rounded up to
> match the xls.  And even then, the value for 86 still doesn't play nice.
>   I was really hoping to avoid using a table for these values.
>
> The TXCALDP/DN values use a much simpler 1-to-1 scale across the 16
> possible register values and so are unaffected by a similar issue.  I
> rechecked their numbers just to be sure.

The solution looks to be to scale D_CAL starting from 80 instead of 79. 
  If you look at the xls listing, the jump from 79 to 83 is the only 
time two adjacent register values result in a change greater than 2% or 3%.

This will result in some code ugliness as the minimum allowed percentage 
(79), per the Freescale document, and the point at which we are scaling 
the percentage values to register values (80) are different.

And, as mentioned before, the values will also have to be rounded up.

This quick shell code confirms that these sorts of calculations match up 
with the values in the spreadsheet:

for d in 119 116 114 112 109 106 103 100 97 95 93 90 88 86 83 79; do 
echo "$d="$(( ( (119-$d) * 0xf + (119-80)/2 ) / (119-80) )); d=$((d+1)); 
done


I can't find any formula which would hit all of those same percentages 
without rounding up.


>
>>
>> dts changes:
>>
>> +&usbphy1 {
>> +       fsl,tx-d-cal = <109>;
>> +};
>> +
>> +&usbphy2 {
>> +       fsl,tx-d-cal = <106>;
>> +};
>> +
>>
>> The phy1's tx-d-cal is 0x3, and phy2's tx-d-cal is 0x4 after PHY
>> initialization,
>> but according to xls, it should be 0x4 and 0x5.
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] usb: phy: mxs: Add DT bindings to configure TX settings
       [not found]                     ` <56F2DDB7.7070700-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
@ 2016-03-24  2:21                       ` Peter Chen
  2016-03-30 10:29                         ` Felipe Balbi
  2016-06-08 21:27                         ` Jaret Cantu
  0 siblings, 2 replies; 17+ messages in thread
From: Peter Chen @ 2016-03-24  2:21 UTC (permalink / raw)
  To: Jaret Cantu
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
	felipe.balbi-VuQAYsv1563Yd54FQh9/CA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A

On Wed, Mar 23, 2016 at 02:17:27PM -0400, Jaret Cantu wrote:
> On 03/23/2016 01:37 PM, Jaret Cantu wrote:
> >On 03/23/2016 12:36 AM, Peter Chen wrote:
> >>On Mon, Mar 21, 2016 at 12:32:27PM -0400, Jaret Cantu wrote:
> >>>The TX settings can be calibrated for particular hardware.  The
> >>>phy is reset by Linux, so this cannot be handled by the bootloader.
> >>>
> >>>The TRM mentions that the maximum resistance should be used for the
> >>>DN/DP calibration in order to pass USB certification.
> >>>
> >>>The values for the TX registers are poorly described in the TRM.
> >>>The meanings of the register values were taken from another
> >>>Freescale-provided document:
> >>>https://community.freescale.com/message/566147#comment-566912
> >>>
> >>>Signed-off-by: Jaret Cantu <jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
> >>>---
> >>>v3. Added unit suffix (-ohms) to tx-cal-45-d*
> >>>
> >>>v2. Copying devicetree list
> >>>     Removed prettifying extra whitespace
> >>>     Removed unnecessary register rewrite on resume
> >>>     Use min and max constants for clarity
> >>>
> >>>  .../devicetree/bindings/phy/mxs-usb-phy.txt        |   10 ++++
> >>>  drivers/usb/phy/phy-mxs-usb.c                      |   58
> >>>++++++++++++++++++++
> >>>  2 files changed, 68 insertions(+)
> >>>
> >>>diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> >>>b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> >>>index 379b84a..1d25b04 100644
> >>>--- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> >>>+++ b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> >>>@@ -12,6 +12,16 @@ Required properties:
> >>>  - interrupts: Should contain phy interrupt
> >>>  - fsl,anatop: phandle for anatop register, it is only for imx6 SoC
> >>>series
> >>>
> >>>+
> >>>+    if (!of_property_read_u32(np, "fsl,tx-d-cal", &val) &&
> >>>+        val >= MXS_PHY_TX_D_CAL_MIN && val <= MXS_PHY_TX_D_CAL_MAX) {
> >>>+        /* scale to 4-bit value */
> >>>+        val = (MXS_PHY_TX_D_CAL_MAX - val) * 0xF
> >>>+            / (MXS_PHY_TX_D_CAL_MAX - MXS_PHY_TX_D_CAL_MIN);
> >>>+        mxs_phy->tx_reg_mask |= GM_USBPHY_TX_D_CAL(~0);
> >>>+        mxs_phy->tx_reg_set  |= GM_USBPHY_TX_D_CAL(val);
> >>>+    }
> >>>+
> >>
> >>I have tested "tx-d-cal", but it seems incorrect according to the xls you
> >>have provided, would you please check it again or am I wrong?
> >
> >Gah! You're right. Some of the D_CAL values need to be rounded up to
> >match the xls.  And even then, the value for 86 still doesn't play nice.
> >  I was really hoping to avoid using a table for these values.
> >
> >The TXCALDP/DN values use a much simpler 1-to-1 scale across the 16
> >possible register values and so are unaffected by a similar issue.  I
> >rechecked their numbers just to be sure.
> 
> The solution looks to be to scale D_CAL starting from 80 instead of
> 79.  If you look at the xls listing, the jump from 79 to 83 is the
> only time two adjacent register values result in a change greater
> than 2% or 3%.
> 
> This will result in some code ugliness as the minimum allowed
> percentage (79), per the Freescale document, and the point at which
> we are scaling the percentage values to register values (80) are
> different.
> 
> And, as mentioned before, the values will also have to be rounded up.
> 
> This quick shell code confirms that these sorts of calculations
> match up with the values in the spreadsheet:
> 
> for d in 119 116 114 112 109 106 103 100 97 95 93 90 88 86 83 79; do
> echo "$d="$(( ( (119-$d) * 0xf + (119-80)/2 ) / (119-80) ));
> d=$((d+1)); done
> 
> 
> I can't find any formula which would hit all of those same
> percentages without rounding up.
> 

Then, we had to use table for it. Besides, IC team confirms the default 
value and the step for TXCAL45DP/DN are changed at next generation SoCs,
so I am wondering how we describe it at binding doc.

-- 
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] usb: phy: mxs: Add DT bindings to configure TX settings
  2016-03-24  2:21                       ` Peter Chen
@ 2016-03-30 10:29                         ` Felipe Balbi
       [not found]                           ` <874mboqmwh.fsf-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
  2016-06-08 21:27                         ` Jaret Cantu
  1 sibling, 1 reply; 17+ messages in thread
From: Felipe Balbi @ 2016-03-30 10:29 UTC (permalink / raw)
  To: Peter Chen, Jaret Cantu
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A

[-- Attachment #1: Type: text/plain, Size: 4249 bytes --]


Hi,

Peter Chen <hzpeterchen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
> [ text/plain ]
> On Wed, Mar 23, 2016 at 02:17:27PM -0400, Jaret Cantu wrote:
>> On 03/23/2016 01:37 PM, Jaret Cantu wrote:
>> >On 03/23/2016 12:36 AM, Peter Chen wrote:
>> >>On Mon, Mar 21, 2016 at 12:32:27PM -0400, Jaret Cantu wrote:
>> >>>The TX settings can be calibrated for particular hardware.  The
>> >>>phy is reset by Linux, so this cannot be handled by the bootloader.
>> >>>
>> >>>The TRM mentions that the maximum resistance should be used for the
>> >>>DN/DP calibration in order to pass USB certification.
>> >>>
>> >>>The values for the TX registers are poorly described in the TRM.
>> >>>The meanings of the register values were taken from another
>> >>>Freescale-provided document:
>> >>>https://community.freescale.com/message/566147#comment-566912
>> >>>
>> >>>Signed-off-by: Jaret Cantu <jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
>> >>>---
>> >>>v3. Added unit suffix (-ohms) to tx-cal-45-d*
>> >>>
>> >>>v2. Copying devicetree list
>> >>>     Removed prettifying extra whitespace
>> >>>     Removed unnecessary register rewrite on resume
>> >>>     Use min and max constants for clarity
>> >>>
>> >>>  .../devicetree/bindings/phy/mxs-usb-phy.txt        |   10 ++++
>> >>>  drivers/usb/phy/phy-mxs-usb.c                      |   58
>> >>>++++++++++++++++++++
>> >>>  2 files changed, 68 insertions(+)
>> >>>
>> >>>diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>> >>>b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>> >>>index 379b84a..1d25b04 100644
>> >>>--- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>> >>>+++ b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>> >>>@@ -12,6 +12,16 @@ Required properties:
>> >>>  - interrupts: Should contain phy interrupt
>> >>>  - fsl,anatop: phandle for anatop register, it is only for imx6 SoC
>> >>>series
>> >>>
>> >>>+
>> >>>+    if (!of_property_read_u32(np, "fsl,tx-d-cal", &val) &&
>> >>>+        val >= MXS_PHY_TX_D_CAL_MIN && val <= MXS_PHY_TX_D_CAL_MAX) {
>> >>>+        /* scale to 4-bit value */
>> >>>+        val = (MXS_PHY_TX_D_CAL_MAX - val) * 0xF
>> >>>+            / (MXS_PHY_TX_D_CAL_MAX - MXS_PHY_TX_D_CAL_MIN);
>> >>>+        mxs_phy->tx_reg_mask |= GM_USBPHY_TX_D_CAL(~0);
>> >>>+        mxs_phy->tx_reg_set  |= GM_USBPHY_TX_D_CAL(val);
>> >>>+    }
>> >>>+
>> >>
>> >>I have tested "tx-d-cal", but it seems incorrect according to the xls you
>> >>have provided, would you please check it again or am I wrong?
>> >
>> >Gah! You're right. Some of the D_CAL values need to be rounded up to
>> >match the xls.  And even then, the value for 86 still doesn't play nice.
>> >  I was really hoping to avoid using a table for these values.
>> >
>> >The TXCALDP/DN values use a much simpler 1-to-1 scale across the 16
>> >possible register values and so are unaffected by a similar issue.  I
>> >rechecked their numbers just to be sure.
>> 
>> The solution looks to be to scale D_CAL starting from 80 instead of
>> 79.  If you look at the xls listing, the jump from 79 to 83 is the
>> only time two adjacent register values result in a change greater
>> than 2% or 3%.
>> 
>> This will result in some code ugliness as the minimum allowed
>> percentage (79), per the Freescale document, and the point at which
>> we are scaling the percentage values to register values (80) are
>> different.
>> 
>> And, as mentioned before, the values will also have to be rounded up.
>> 
>> This quick shell code confirms that these sorts of calculations
>> match up with the values in the spreadsheet:
>> 
>> for d in 119 116 114 112 109 106 103 100 97 95 93 90 88 86 83 79; do
>> echo "$d="$(( ( (119-$d) * 0xf + (119-80)/2 ) / (119-80) ));
>> d=$((d+1)); done
>> 
>> 
>> I can't find any formula which would hit all of those same
>> percentages without rounding up.
>> 
>
> Then, we had to use table for it. Besides, IC team confirms the default 
> value and the step for TXCAL45DP/DN are changed at next generation SoCs,
> so I am wondering how we describe it at binding doc.

so I take it we're not yet ready to move forward with this ?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] usb: phy: mxs: Add DT bindings to configure TX settings
       [not found]                           ` <874mboqmwh.fsf-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
@ 2016-03-31  6:49                             ` Peter Chen
  0 siblings, 0 replies; 17+ messages in thread
From: Peter Chen @ 2016-03-31  6:49 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Jaret Cantu, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A

On Wed, Mar 30, 2016 at 01:29:34PM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Peter Chen <hzpeterchen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
> > [ text/plain ]
> > On Wed, Mar 23, 2016 at 02:17:27PM -0400, Jaret Cantu wrote:
> >> On 03/23/2016 01:37 PM, Jaret Cantu wrote:
> >> >On 03/23/2016 12:36 AM, Peter Chen wrote:
> >> >>On Mon, Mar 21, 2016 at 12:32:27PM -0400, Jaret Cantu wrote:
> >> >>>The TX settings can be calibrated for particular hardware.  The
> >> >>>phy is reset by Linux, so this cannot be handled by the bootloader.
> >> >>>
> >> >>>The TRM mentions that the maximum resistance should be used for the
> >> >>>DN/DP calibration in order to pass USB certification.
> >> >>>
> >> >>>The values for the TX registers are poorly described in the TRM.
> >> >>>The meanings of the register values were taken from another
> >> >>>Freescale-provided document:
> >> >>>https://community.freescale.com/message/566147#comment-566912
> >> >>>
> >> >>>Signed-off-by: Jaret Cantu <jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
> >> >>>---
> >> >>>v3. Added unit suffix (-ohms) to tx-cal-45-d*
> >> >>>
> >> >>>v2. Copying devicetree list
> >> >>>     Removed prettifying extra whitespace
> >> >>>     Removed unnecessary register rewrite on resume
> >> >>>     Use min and max constants for clarity
> >> >>>
> >> >>>  .../devicetree/bindings/phy/mxs-usb-phy.txt        |   10 ++++
> >> >>>  drivers/usb/phy/phy-mxs-usb.c                      |   58
> >> >>>++++++++++++++++++++
> >> >>>  2 files changed, 68 insertions(+)
> >> >>>
> >> >>>diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> >> >>>b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> >> >>>index 379b84a..1d25b04 100644
> >> >>>--- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> >> >>>+++ b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> >> >>>@@ -12,6 +12,16 @@ Required properties:
> >> >>>  - interrupts: Should contain phy interrupt
> >> >>>  - fsl,anatop: phandle for anatop register, it is only for imx6 SoC
> >> >>>series
> >> >>>
> >> >>>+
> >> >>>+    if (!of_property_read_u32(np, "fsl,tx-d-cal", &val) &&
> >> >>>+        val >= MXS_PHY_TX_D_CAL_MIN && val <= MXS_PHY_TX_D_CAL_MAX) {
> >> >>>+        /* scale to 4-bit value */
> >> >>>+        val = (MXS_PHY_TX_D_CAL_MAX - val) * 0xF
> >> >>>+            / (MXS_PHY_TX_D_CAL_MAX - MXS_PHY_TX_D_CAL_MIN);
> >> >>>+        mxs_phy->tx_reg_mask |= GM_USBPHY_TX_D_CAL(~0);
> >> >>>+        mxs_phy->tx_reg_set  |= GM_USBPHY_TX_D_CAL(val);
> >> >>>+    }
> >> >>>+
> >> >>
> >> >>I have tested "tx-d-cal", but it seems incorrect according to the xls you
> >> >>have provided, would you please check it again or am I wrong?
> >> >
> >> >Gah! You're right. Some of the D_CAL values need to be rounded up to
> >> >match the xls.  And even then, the value for 86 still doesn't play nice.
> >> >  I was really hoping to avoid using a table for these values.
> >> >
> >> >The TXCALDP/DN values use a much simpler 1-to-1 scale across the 16
> >> >possible register values and so are unaffected by a similar issue.  I
> >> >rechecked their numbers just to be sure.
> >> 
> >> The solution looks to be to scale D_CAL starting from 80 instead of
> >> 79.  If you look at the xls listing, the jump from 79 to 83 is the
> >> only time two adjacent register values result in a change greater
> >> than 2% or 3%.
> >> 
> >> This will result in some code ugliness as the minimum allowed
> >> percentage (79), per the Freescale document, and the point at which
> >> we are scaling the percentage values to register values (80) are
> >> different.
> >> 
> >> And, as mentioned before, the values will also have to be rounded up.
> >> 
> >> This quick shell code confirms that these sorts of calculations
> >> match up with the values in the spreadsheet:
> >> 
> >> for d in 119 116 114 112 109 106 103 100 97 95 93 90 88 86 83 79; do
> >> echo "$d="$(( ( (119-$d) * 0xf + (119-80)/2 ) / (119-80) ));
> >> d=$((d+1)); done
> >> 
> >> 
> >> I can't find any formula which would hit all of those same
> >> percentages without rounding up.
> >> 
> >
> > Then, we had to use table for it. Besides, IC team confirms the default 
> > value and the step for TXCAL45DP/DN are changed at next generation SoCs,
> > so I am wondering how we describe it at binding doc.
> 
> so I take it we're not yet ready to move forward with this ?
> 

No, we still haven't a formal solution.

-- 
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] usb: phy: mxs: Add DT bindings to configure TX settings
  2016-03-24  2:21                       ` Peter Chen
  2016-03-30 10:29                         ` Felipe Balbi
@ 2016-06-08 21:27                         ` Jaret Cantu
       [not found]                           ` <57588DA8.4050000-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
  1 sibling, 1 reply; 17+ messages in thread
From: Jaret Cantu @ 2016-06-08 21:27 UTC (permalink / raw)
  To: Peter Chen
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
	felipe.balbi-VuQAYsv1563Yd54FQh9/CA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A

On 03/23/2016 10:21 PM, Peter Chen wrote:
> On Wed, Mar 23, 2016 at 02:17:27PM -0400, Jaret Cantu wrote:
>> On 03/23/2016 01:37 PM, Jaret Cantu wrote:
>>> On 03/23/2016 12:36 AM, Peter Chen wrote:
>>>> On Mon, Mar 21, 2016 at 12:32:27PM -0400, Jaret Cantu wrote:
>>>>> The TX settings can be calibrated for particular hardware.  The
>>>>> phy is reset by Linux, so this cannot be handled by the bootloader.
>>>>>
>>>>> The TRM mentions that the maximum resistance should be used for the
>>>>> DN/DP calibration in order to pass USB certification.
>>>>>
>>>>> The values for the TX registers are poorly described in the TRM.
>>>>> The meanings of the register values were taken from another
>>>>> Freescale-provided document:
>>>>> https://community.freescale.com/message/566147#comment-566912
>>>>>
>>>>> Signed-off-by: Jaret Cantu <jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
>>>>> ---
>>>>> v3. Added unit suffix (-ohms) to tx-cal-45-d*
>>>>>
>>>>> v2. Copying devicetree list
>>>>>      Removed prettifying extra whitespace
>>>>>      Removed unnecessary register rewrite on resume
>>>>>      Use min and max constants for clarity
>>>>>
>>>>>   .../devicetree/bindings/phy/mxs-usb-phy.txt        |   10 ++++
>>>>>   drivers/usb/phy/phy-mxs-usb.c                      |   58
>>>>> ++++++++++++++++++++
>>>>>   2 files changed, 68 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>>>> b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>>>> index 379b84a..1d25b04 100644
>>>>> --- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>>>> +++ b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>>>> @@ -12,6 +12,16 @@ Required properties:
>>>>>   - interrupts: Should contain phy interrupt
>>>>>   - fsl,anatop: phandle for anatop register, it is only for imx6 SoC
>>>>> series
>>>>>
>>>>> +
>>>>> +    if (!of_property_read_u32(np, "fsl,tx-d-cal", &val) &&
>>>>> +        val >= MXS_PHY_TX_D_CAL_MIN && val <= MXS_PHY_TX_D_CAL_MAX) {
>>>>> +        /* scale to 4-bit value */
>>>>> +        val = (MXS_PHY_TX_D_CAL_MAX - val) * 0xF
>>>>> +            / (MXS_PHY_TX_D_CAL_MAX - MXS_PHY_TX_D_CAL_MIN);
>>>>> +        mxs_phy->tx_reg_mask |= GM_USBPHY_TX_D_CAL(~0);
>>>>> +        mxs_phy->tx_reg_set  |= GM_USBPHY_TX_D_CAL(val);
>>>>> +    }
>>>>> +
>>>>
>>>> I have tested "tx-d-cal", but it seems incorrect according to the xls you
>>>> have provided, would you please check it again or am I wrong?
>>>
>>> Gah! You're right. Some of the D_CAL values need to be rounded up to
>>> match the xls.  And even then, the value for 86 still doesn't play nice.
>>>   I was really hoping to avoid using a table for these values.
>>>
>>> The TXCALDP/DN values use a much simpler 1-to-1 scale across the 16
>>> possible register values and so are unaffected by a similar issue.  I
>>> rechecked their numbers just to be sure.
>>
>> The solution looks to be to scale D_CAL starting from 80 instead of
>> 79.  If you look at the xls listing, the jump from 79 to 83 is the
>> only time two adjacent register values result in a change greater
>> than 2% or 3%.
>>
>> This will result in some code ugliness as the minimum allowed
>> percentage (79), per the Freescale document, and the point at which
>> we are scaling the percentage values to register values (80) are
>> different.
>>
>> And, as mentioned before, the values will also have to be rounded up.
>>
>> This quick shell code confirms that these sorts of calculations
>> match up with the values in the spreadsheet:
>>
>> for d in 119 116 114 112 109 106 103 100 97 95 93 90 88 86 83 79; do
>> echo "$d="$(( ( (119-$d) * 0xf + (119-80)/2 ) / (119-80) ));
>> d=$((d+1)); done
>>
>>
>> I can't find any formula which would hit all of those same
>> percentages without rounding up.
>>
>
> Then, we had to use table for it. Besides, IC team confirms the default
> value and the step for TXCAL45DP/DN are changed at next generation SoCs,
> so I am wondering how we describe it at binding doc.
>

Which next gen SoC would this be?  The MX7?  The documentation for the 
USB PHY in that reference manual is even sparser than the one for the 
MX6 in regards to these register values.

The MX7 manual does still mention that HW_USBPHY_TX_TXCAL45DP and 
HW_USBPHY_TX_TXCAL45DN should both be set to zero, but there is no 
listing as to the location of these registers, let alone their 
defaults/step values.

Do you know where we could get the default and step values for the TXCAL 
registers on the new SoC?  This information had to come from a Freescale 
community thread for the MX6 since it wasn't defined clearly elsewhere.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] usb: phy: mxs: Add DT bindings to configure TX settings
       [not found]                           ` <57588DA8.4050000-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
@ 2016-06-09  2:41                             ` Peter Chen
       [not found]                               ` <CAL411-of2Phqx18LBaGt6FMg5DxaJesRYvrRzsfSNyJRopsUZQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Peter Chen @ 2016-06-09  2:41 UTC (permalink / raw)
  To: Jaret Cantu
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
	felipe.balbi-VuQAYsv1563Yd54FQh9/CA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A

On Thu, Jun 9, 2016 at 5:27 AM, Jaret Cantu <jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org> wrote:
> On 03/23/2016 10:21 PM, Peter Chen wrote:
>>
>> On Wed, Mar 23, 2016 at 02:17:27PM -0400, Jaret Cantu wrote:
>>>
>>> On 03/23/2016 01:37 PM, Jaret Cantu wrote:
>>>>
>>>> On 03/23/2016 12:36 AM, Peter Chen wrote:
>>>>>
>>>>> On Mon, Mar 21, 2016 at 12:32:27PM -0400, Jaret Cantu wrote:
>>>>>>
>>>>>> The TX settings can be calibrated for particular hardware.  The
>>>>>> phy is reset by Linux, so this cannot be handled by the bootloader.
>>>>>>
>>>>>> The TRM mentions that the maximum resistance should be used for the
>>>>>> DN/DP calibration in order to pass USB certification.
>>>>>>
>>>>>> The values for the TX registers are poorly described in the TRM.
>>>>>> The meanings of the register values were taken from another
>>>>>> Freescale-provided document:
>>>>>> https://community.freescale.com/message/566147#comment-566912
>>>>>>
>>>>>> Signed-off-by: Jaret Cantu <jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
>>>>>> ---
>>>>>> v3. Added unit suffix (-ohms) to tx-cal-45-d*
>>>>>>
>>>>>> v2. Copying devicetree list
>>>>>>      Removed prettifying extra whitespace
>>>>>>      Removed unnecessary register rewrite on resume
>>>>>>      Use min and max constants for clarity
>>>>>>
>>>>>>   .../devicetree/bindings/phy/mxs-usb-phy.txt        |   10 ++++
>>>>>>   drivers/usb/phy/phy-mxs-usb.c                      |   58
>>>>>> ++++++++++++++++++++
>>>>>>   2 files changed, 68 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>>>>> b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>>>>> index 379b84a..1d25b04 100644
>>>>>> --- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>>>>> +++ b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>>>>> @@ -12,6 +12,16 @@ Required properties:
>>>>>>   - interrupts: Should contain phy interrupt
>>>>>>   - fsl,anatop: phandle for anatop register, it is only for imx6 SoC
>>>>>> series
>>>>>>
>>>>>> +
>>>>>> +    if (!of_property_read_u32(np, "fsl,tx-d-cal", &val) &&
>>>>>> +        val >= MXS_PHY_TX_D_CAL_MIN && val <= MXS_PHY_TX_D_CAL_MAX) {
>>>>>> +        /* scale to 4-bit value */
>>>>>> +        val = (MXS_PHY_TX_D_CAL_MAX - val) * 0xF
>>>>>> +            / (MXS_PHY_TX_D_CAL_MAX - MXS_PHY_TX_D_CAL_MIN);
>>>>>> +        mxs_phy->tx_reg_mask |= GM_USBPHY_TX_D_CAL(~0);
>>>>>> +        mxs_phy->tx_reg_set  |= GM_USBPHY_TX_D_CAL(val);
>>>>>> +    }
>>>>>> +
>>>>>
>>>>>
>>>>> I have tested "tx-d-cal", but it seems incorrect according to the xls
>>>>> you
>>>>> have provided, would you please check it again or am I wrong?
>>>>
>>>>
>>>> Gah! You're right. Some of the D_CAL values need to be rounded up to
>>>> match the xls.  And even then, the value for 86 still doesn't play nice.
>>>>   I was really hoping to avoid using a table for these values.
>>>>
>>>> The TXCALDP/DN values use a much simpler 1-to-1 scale across the 16
>>>> possible register values and so are unaffected by a similar issue.  I
>>>> rechecked their numbers just to be sure.
>>>
>>>
>>> The solution looks to be to scale D_CAL starting from 80 instead of
>>> 79.  If you look at the xls listing, the jump from 79 to 83 is the
>>> only time two adjacent register values result in a change greater
>>> than 2% or 3%.
>>>
>>> This will result in some code ugliness as the minimum allowed
>>> percentage (79), per the Freescale document, and the point at which
>>> we are scaling the percentage values to register values (80) are
>>> different.
>>>
>>> And, as mentioned before, the values will also have to be rounded up.
>>>
>>> This quick shell code confirms that these sorts of calculations
>>> match up with the values in the spreadsheet:
>>>
>>> for d in 119 116 114 112 109 106 103 100 97 95 93 90 88 86 83 79; do
>>> echo "$d="$(( ( (119-$d) * 0xf + (119-80)/2 ) / (119-80) ));
>>> d=$((d+1)); done
>>>
>>>
>>> I can't find any formula which would hit all of those same
>>> percentages without rounding up.
>>>
>>
>> Then, we had to use table for it. Besides, IC team confirms the default
>> value and the step for TXCAL45DP/DN are changed at next generation SoCs,
>> so I am wondering how we describe it at binding doc.
>>
>
> Which next gen SoC would this be?  The MX7?  The documentation for the USB
> PHY in that reference manual is even sparser than the one for the MX6 in
> regards to these register values.
>

Here, I mean i.mx8

> The MX7 manual does still mention that HW_USBPHY_TX_TXCAL45DP and
> HW_USBPHY_TX_TXCAL45DN should both be set to zero, but there is no listing
> as to the location of these registers, let alone their defaults/step values.
>
> Do you know where we could get the default and step values for the TXCAL
> registers on the new SoC?  This information had to come from a Freescale
> community thread for the MX6 since it wasn't defined clearly elsewhere.

In theory, these information should be listed at SoC reference manual.

BR,
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] usb: phy: mxs: Add DT bindings to configure TX settings
       [not found]                               ` <CAL411-of2Phqx18LBaGt6FMg5DxaJesRYvrRzsfSNyJRopsUZQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2016-06-09 18:07                                 ` Justin Waters
       [not found]                                   ` <CAENNV6+978syhexzQber58xuai=grC2_TsVF3GmHCAp_Zj=HZQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Justin Waters @ 2016-06-09 18:07 UTC (permalink / raw)
  To: Peter Chen
  Cc: Jaret Cantu, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	felipe.balbi-VuQAYsv1563Yd54FQh9/CA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A

Peter,

On Wed, Jun 8, 2016 at 10:41 PM, Peter Chen <hzpeterchen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Thu, Jun 9, 2016 at 5:27 AM, Jaret Cantu <jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org> wrote:
>> On 03/23/2016 10:21 PM, Peter Chen wrote:
>>>
>>> On Wed, Mar 23, 2016 at 02:17:27PM -0400, Jaret Cantu wrote:
>>>>
>>>> On 03/23/2016 01:37 PM, Jaret Cantu wrote:
>>>>>
>>>>> On 03/23/2016 12:36 AM, Peter Chen wrote:
>>>>>>
>>>>>> On Mon, Mar 21, 2016 at 12:32:27PM -0400, Jaret Cantu wrote:
>>>>>>>
>>>>>>> The TX settings can be calibrated for particular hardware.  The
>>>>>>> phy is reset by Linux, so this cannot be handled by the bootloader.
>>>>>>>
>>>>>>> The TRM mentions that the maximum resistance should be used for the
>>>>>>> DN/DP calibration in order to pass USB certification.
>>>>>>>
>>>>>>> The values for the TX registers are poorly described in the TRM.
>>>>>>> The meanings of the register values were taken from another
>>>>>>> Freescale-provided document:
>>>>>>> https://community.freescale.com/message/566147#comment-566912
>>>>>>>
>>>>>>> Signed-off-by: Jaret Cantu <jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
>>>>>>> ---
>>>>>>> v3. Added unit suffix (-ohms) to tx-cal-45-d*
>>>>>>>
>>>>>>> v2. Copying devicetree list
>>>>>>>      Removed prettifying extra whitespace
>>>>>>>      Removed unnecessary register rewrite on resume
>>>>>>>      Use min and max constants for clarity
>>>>>>>
>>>>>>>   .../devicetree/bindings/phy/mxs-usb-phy.txt        |   10 ++++
>>>>>>>   drivers/usb/phy/phy-mxs-usb.c                      |   58
>>>>>>> ++++++++++++++++++++
>>>>>>>   2 files changed, 68 insertions(+)
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>>>>>> b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>>>>>> index 379b84a..1d25b04 100644
>>>>>>> --- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>>>>>> +++ b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>>>>>> @@ -12,6 +12,16 @@ Required properties:
>>>>>>>   - interrupts: Should contain phy interrupt
>>>>>>>   - fsl,anatop: phandle for anatop register, it is only for imx6 SoC
>>>>>>> series
>>>>>>>
>>>>>>> +
>>>>>>> +    if (!of_property_read_u32(np, "fsl,tx-d-cal", &val) &&
>>>>>>> +        val >= MXS_PHY_TX_D_CAL_MIN && val <= MXS_PHY_TX_D_CAL_MAX) {
>>>>>>> +        /* scale to 4-bit value */
>>>>>>> +        val = (MXS_PHY_TX_D_CAL_MAX - val) * 0xF
>>>>>>> +            / (MXS_PHY_TX_D_CAL_MAX - MXS_PHY_TX_D_CAL_MIN);
>>>>>>> +        mxs_phy->tx_reg_mask |= GM_USBPHY_TX_D_CAL(~0);
>>>>>>> +        mxs_phy->tx_reg_set  |= GM_USBPHY_TX_D_CAL(val);
>>>>>>> +    }
>>>>>>> +
>>>>>>
>>>>>>
>>>>>> I have tested "tx-d-cal", but it seems incorrect according to the xls
>>>>>> you
>>>>>> have provided, would you please check it again or am I wrong?
>>>>>
>>>>>
>>>>> Gah! You're right. Some of the D_CAL values need to be rounded up to
>>>>> match the xls.  And even then, the value for 86 still doesn't play nice.
>>>>>   I was really hoping to avoid using a table for these values.
>>>>>
>>>>> The TXCALDP/DN values use a much simpler 1-to-1 scale across the 16
>>>>> possible register values and so are unaffected by a similar issue.  I
>>>>> rechecked their numbers just to be sure.
>>>>
>>>>
>>>> The solution looks to be to scale D_CAL starting from 80 instead of
>>>> 79.  If you look at the xls listing, the jump from 79 to 83 is the
>>>> only time two adjacent register values result in a change greater
>>>> than 2% or 3%.
>>>>
>>>> This will result in some code ugliness as the minimum allowed
>>>> percentage (79), per the Freescale document, and the point at which
>>>> we are scaling the percentage values to register values (80) are
>>>> different.
>>>>
>>>> And, as mentioned before, the values will also have to be rounded up.
>>>>
>>>> This quick shell code confirms that these sorts of calculations
>>>> match up with the values in the spreadsheet:
>>>>
>>>> for d in 119 116 114 112 109 106 103 100 97 95 93 90 88 86 83 79; do
>>>> echo "$d="$(( ( (119-$d) * 0xf + (119-80)/2 ) / (119-80) ));
>>>> d=$((d+1)); done
>>>>
>>>>
>>>> I can't find any formula which would hit all of those same
>>>> percentages without rounding up.
>>>>
>>>
>>> Then, we had to use table for it. Besides, IC team confirms the default
>>> value and the step for TXCAL45DP/DN are changed at next generation SoCs,
>>> so I am wondering how we describe it at binding doc.
>>>
>>
>> Which next gen SoC would this be?  The MX7?  The documentation for the USB
>> PHY in that reference manual is even sparser than the one for the MX6 in
>> regards to these register values.
>>
>
> Here, I mean i.mx8
>

Currently, there is no support in the mainline kernel for the i.MX8.
Do you mean to say that this feature is blocked until the i.MX8 is
supported in the mainline kernel? Or that we would be required to add
the register definitions for the i.MX8 as a prerequisite? Wouldn't it
make more sense to add support to the driver as part of the i.MX8
enablement, especially considering no documentation is freely
available at this time?

>> The MX7 manual does still mention that HW_USBPHY_TX_TXCAL45DP and
>> HW_USBPHY_TX_TXCAL45DN should both be set to zero, but there is no listing
>> as to the location of these registers, let alone their defaults/step values.
>>
>> Do you know where we could get the default and step values for the TXCAL
>> registers on the new SoC?  This information had to come from a Freescale
>> community thread for the MX6 since it wasn't defined clearly elsewhere.
>
> In theory, these information should be listed at SoC reference manual.
>

I have not seen the reference manual, so I'm not sure if it's in there
or not. However, if the i.MX6 manual is any indication, I'm not
certain it will be documented anyway.

How do we unblock this issue? We currently have hardware that cannot
meet specifications without this feature, and based on the app note
from NXP, I'm certain plenty of other people do as well.

Thank you,

-- 
Justin Waters
Director of Engineering
Timesys Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] usb: phy: mxs: Add DT bindings to configure TX settings
       [not found]                                   ` <CAENNV6+978syhexzQber58xuai=grC2_TsVF3GmHCAp_Zj=HZQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2016-06-12  3:25                                     ` Peter Chen
  2016-06-13 20:40                                       ` Jaret Cantu
  0 siblings, 1 reply; 17+ messages in thread
From: Peter Chen @ 2016-06-12  3:25 UTC (permalink / raw)
  To: Justin Waters
  Cc: Jaret Cantu, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	felipe.balbi-VuQAYsv1563Yd54FQh9/CA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A

On Thu, Jun 09, 2016 at 02:07:52PM -0400, Justin Waters wrote:
> Peter,
> 
> On Wed, Jun 8, 2016 at 10:41 PM, Peter Chen <hzpeterchen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Thu, Jun 9, 2016 at 5:27 AM, Jaret Cantu <jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org> wrote:
> >> On 03/23/2016 10:21 PM, Peter Chen wrote:
> >>>
> >>> On Wed, Mar 23, 2016 at 02:17:27PM -0400, Jaret Cantu wrote:
> >>>>
> >>>> On 03/23/2016 01:37 PM, Jaret Cantu wrote:
> >>>>>
> >>>>> On 03/23/2016 12:36 AM, Peter Chen wrote:
> >>>>>>
> >>>>>> On Mon, Mar 21, 2016 at 12:32:27PM -0400, Jaret Cantu wrote:
> >>>>>>>
> >>>>>>> The TX settings can be calibrated for particular hardware.  The
> >>>>>>> phy is reset by Linux, so this cannot be handled by the bootloader.
> >>>>>>>
> >>>>>>> The TRM mentions that the maximum resistance should be used for the
> >>>>>>> DN/DP calibration in order to pass USB certification.
> >>>>>>>
> >>>>>>> The values for the TX registers are poorly described in the TRM.
> >>>>>>> The meanings of the register values were taken from another
> >>>>>>> Freescale-provided document:
> >>>>>>> https://community.freescale.com/message/566147#comment-566912
> >>>>>>>
> >>>>>>> Signed-off-by: Jaret Cantu <jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
> >>>>>>> ---
> >>>>>>> v3. Added unit suffix (-ohms) to tx-cal-45-d*
> >>>>>>>
> >>>>>>> v2. Copying devicetree list
> >>>>>>>      Removed prettifying extra whitespace
> >>>>>>>      Removed unnecessary register rewrite on resume
> >>>>>>>      Use min and max constants for clarity
> >>>>>>>
> >>>>>>>   .../devicetree/bindings/phy/mxs-usb-phy.txt        |   10 ++++
> >>>>>>>   drivers/usb/phy/phy-mxs-usb.c                      |   58
> >>>>>>> ++++++++++++++++++++
> >>>>>>>   2 files changed, 68 insertions(+)
> >>>>>>>
> >>>>>>> diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> >>>>>>> b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> >>>>>>> index 379b84a..1d25b04 100644
> >>>>>>> --- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> >>>>>>> +++ b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> >>>>>>> @@ -12,6 +12,16 @@ Required properties:
> >>>>>>>   - interrupts: Should contain phy interrupt
> >>>>>>>   - fsl,anatop: phandle for anatop register, it is only for imx6 SoC
> >>>>>>> series
> >>>>>>>
> >>>>>>> +
> >>>>>>> +    if (!of_property_read_u32(np, "fsl,tx-d-cal", &val) &&
> >>>>>>> +        val >= MXS_PHY_TX_D_CAL_MIN && val <= MXS_PHY_TX_D_CAL_MAX) {
> >>>>>>> +        /* scale to 4-bit value */
> >>>>>>> +        val = (MXS_PHY_TX_D_CAL_MAX - val) * 0xF
> >>>>>>> +            / (MXS_PHY_TX_D_CAL_MAX - MXS_PHY_TX_D_CAL_MIN);
> >>>>>>> +        mxs_phy->tx_reg_mask |= GM_USBPHY_TX_D_CAL(~0);
> >>>>>>> +        mxs_phy->tx_reg_set  |= GM_USBPHY_TX_D_CAL(val);
> >>>>>>> +    }
> >>>>>>> +
> >>>>>>
> >>>>>>
> >>>>>> I have tested "tx-d-cal", but it seems incorrect according to the xls
> >>>>>> you
> >>>>>> have provided, would you please check it again or am I wrong?
> >>>>>
> >>>>>
> >>>>> Gah! You're right. Some of the D_CAL values need to be rounded up to
> >>>>> match the xls.  And even then, the value for 86 still doesn't play nice.
> >>>>>   I was really hoping to avoid using a table for these values.
> >>>>>
> >>>>> The TXCALDP/DN values use a much simpler 1-to-1 scale across the 16
> >>>>> possible register values and so are unaffected by a similar issue.  I
> >>>>> rechecked their numbers just to be sure.
> >>>>
> >>>>
> >>>> The solution looks to be to scale D_CAL starting from 80 instead of
> >>>> 79.  If you look at the xls listing, the jump from 79 to 83 is the
> >>>> only time two adjacent register values result in a change greater
> >>>> than 2% or 3%.
> >>>>
> >>>> This will result in some code ugliness as the minimum allowed
> >>>> percentage (79), per the Freescale document, and the point at which
> >>>> we are scaling the percentage values to register values (80) are
> >>>> different.
> >>>>
> >>>> And, as mentioned before, the values will also have to be rounded up.
> >>>>
> >>>> This quick shell code confirms that these sorts of calculations
> >>>> match up with the values in the spreadsheet:
> >>>>
> >>>> for d in 119 116 114 112 109 106 103 100 97 95 93 90 88 86 83 79; do
> >>>> echo "$d="$(( ( (119-$d) * 0xf + (119-80)/2 ) / (119-80) ));
> >>>> d=$((d+1)); done
> >>>>
> >>>>
> >>>> I can't find any formula which would hit all of those same
> >>>> percentages without rounding up.
> >>>>
> >>>
> >>> Then, we had to use table for it. Besides, IC team confirms the default
> >>> value and the step for TXCAL45DP/DN are changed at next generation SoCs,
> >>> so I am wondering how we describe it at binding doc.
> >>>
> >>
> >> Which next gen SoC would this be?  The MX7?  The documentation for the USB
> >> PHY in that reference manual is even sparser than the one for the MX6 in
> >> regards to these register values.
> >>
> >
> > Here, I mean i.mx8
> >
> 
> Currently, there is no support in the mainline kernel for the i.MX8.
> Do you mean to say that this feature is blocked until the i.MX8 is
> supported in the mainline kernel? Or that we would be required to add
> the register definitions for the i.MX8 as a prerequisite? Wouldn't it
> make more sense to add support to the driver as part of the i.MX8
> enablement, especially considering no documentation is freely
> available at this time?
> 
> >> The MX7 manual does still mention that HW_USBPHY_TX_TXCAL45DP and
> >> HW_USBPHY_TX_TXCAL45DN should both be set to zero, but there is no listing
> >> as to the location of these registers, let alone their defaults/step values.
> >>
> >> Do you know where we could get the default and step values for the TXCAL
> >> registers on the new SoC?  This information had to come from a Freescale
> >> community thread for the MX6 since it wasn't defined clearly elsewhere.
> >
> > In theory, these information should be listed at SoC reference manual.
> >
> 
> I have not seen the reference manual, so I'm not sure if it's in there
> or not. However, if the i.MX6 manual is any indication, I'm not
> certain it will be documented anyway.
> 
> How do we unblock this issue? We currently have hardware that cannot
> meet specifications without this feature, and based on the app note
> from NXP, I'm certain plenty of other people do as well.
> 

I agree with you that we don't need to consider i.mx8 case now.

According to your previous information, the single formula can't be used
to get D_CAL value, do you have any good ways except for using a table?
Sending the patch for new algorithm to speed up this issue please.

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] usb: phy: mxs: Add DT bindings to configure TX settings
  2016-06-12  3:25                                     ` Peter Chen
@ 2016-06-13 20:40                                       ` Jaret Cantu
       [not found]                                         ` <575F1A57.6030807-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Jaret Cantu @ 2016-06-13 20:40 UTC (permalink / raw)
  To: Peter Chen, Justin Waters
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
	felipe.balbi-VuQAYsv1563Yd54FQh9/CA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A

On 06/11/2016 11:25 PM, Peter Chen wrote:
> On Thu, Jun 09, 2016 at 02:07:52PM -0400, Justin Waters wrote:
>> Peter,
>>
>> On Wed, Jun 8, 2016 at 10:41 PM, Peter Chen <hzpeterchen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> On Thu, Jun 9, 2016 at 5:27 AM, Jaret Cantu <jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org> wrote:
>>>> On 03/23/2016 10:21 PM, Peter Chen wrote:
>>>>>
>>>>> On Wed, Mar 23, 2016 at 02:17:27PM -0400, Jaret Cantu wrote:
>>>>>>
>>>>>> On 03/23/2016 01:37 PM, Jaret Cantu wrote:
>>>>>>>
>>>>>>> On 03/23/2016 12:36 AM, Peter Chen wrote:
>>>>>>>>
>>>>>>>> On Mon, Mar 21, 2016 at 12:32:27PM -0400, Jaret Cantu wrote:
>>>>>>>>>
>>>>>>>>> The TX settings can be calibrated for particular hardware.  The
>>>>>>>>> phy is reset by Linux, so this cannot be handled by the bootloader.
>>>>>>>>>
>>>>>>>>> The TRM mentions that the maximum resistance should be used for the
>>>>>>>>> DN/DP calibration in order to pass USB certification.
>>>>>>>>>
>>>>>>>>> The values for the TX registers are poorly described in the TRM.
>>>>>>>>> The meanings of the register values were taken from another
>>>>>>>>> Freescale-provided document:
>>>>>>>>> https://community.freescale.com/message/566147#comment-566912
>>>>>>>>>
>>>>>>>>> Signed-off-by: Jaret Cantu <jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
>>>>>>>>> ---
>>>>>>>>> v3. Added unit suffix (-ohms) to tx-cal-45-d*
>>>>>>>>>
>>>>>>>>> v2. Copying devicetree list
>>>>>>>>>       Removed prettifying extra whitespace
>>>>>>>>>       Removed unnecessary register rewrite on resume
>>>>>>>>>       Use min and max constants for clarity
>>>>>>>>>
>>>>>>>>>    .../devicetree/bindings/phy/mxs-usb-phy.txt        |   10 ++++
>>>>>>>>>    drivers/usb/phy/phy-mxs-usb.c                      |   58
>>>>>>>>> ++++++++++++++++++++
>>>>>>>>>    2 files changed, 68 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>>>>>>>> b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>>>>>>>> index 379b84a..1d25b04 100644
>>>>>>>>> --- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>>>>>>>> +++ b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>>>>>>>> @@ -12,6 +12,16 @@ Required properties:
>>>>>>>>>    - interrupts: Should contain phy interrupt
>>>>>>>>>    - fsl,anatop: phandle for anatop register, it is only for imx6 SoC
>>>>>>>>> series
>>>>>>>>>
>>>>>>>>> +
>>>>>>>>> +    if (!of_property_read_u32(np, "fsl,tx-d-cal", &val) &&
>>>>>>>>> +        val >= MXS_PHY_TX_D_CAL_MIN && val <= MXS_PHY_TX_D_CAL_MAX) {
>>>>>>>>> +        /* scale to 4-bit value */
>>>>>>>>> +        val = (MXS_PHY_TX_D_CAL_MAX - val) * 0xF
>>>>>>>>> +            / (MXS_PHY_TX_D_CAL_MAX - MXS_PHY_TX_D_CAL_MIN);
>>>>>>>>> +        mxs_phy->tx_reg_mask |= GM_USBPHY_TX_D_CAL(~0);
>>>>>>>>> +        mxs_phy->tx_reg_set  |= GM_USBPHY_TX_D_CAL(val);
>>>>>>>>> +    }
>>>>>>>>> +
>>>>>>>>
>>>>>>>>
>>>>>>>> I have tested "tx-d-cal", but it seems incorrect according to the xls
>>>>>>>> you
>>>>>>>> have provided, would you please check it again or am I wrong?
>>>>>>>
>>>>>>>
>>>>>>> Gah! You're right. Some of the D_CAL values need to be rounded up to
>>>>>>> match the xls.  And even then, the value for 86 still doesn't play nice.
>>>>>>>    I was really hoping to avoid using a table for these values.
>>>>>>>
>>>>>>> The TXCALDP/DN values use a much simpler 1-to-1 scale across the 16
>>>>>>> possible register values and so are unaffected by a similar issue.  I
>>>>>>> rechecked their numbers just to be sure.
>>>>>>
>>>>>>
>>>>>> The solution looks to be to scale D_CAL starting from 80 instead of
>>>>>> 79.  If you look at the xls listing, the jump from 79 to 83 is the
>>>>>> only time two adjacent register values result in a change greater
>>>>>> than 2% or 3%.
>>>>>>
>>>>>> This will result in some code ugliness as the minimum allowed
>>>>>> percentage (79), per the Freescale document, and the point at which
>>>>>> we are scaling the percentage values to register values (80) are
>>>>>> different.
>>>>>>
>>>>>> And, as mentioned before, the values will also have to be rounded up.
>>>>>>
>>>>>> This quick shell code confirms that these sorts of calculations
>>>>>> match up with the values in the spreadsheet:
>>>>>>
>>>>>> for d in 119 116 114 112 109 106 103 100 97 95 93 90 88 86 83 79; do
>>>>>> echo "$d="$(( ( (119-$d) * 0xf + (119-80)/2 ) / (119-80) ));
>>>>>> d=$((d+1)); done
>>>>>>
>>>>>>
>>>>>> I can't find any formula which would hit all of those same
>>>>>> percentages without rounding up.
>>>>>>
>>>>>
>>>>> Then, we had to use table for it. Besides, IC team confirms the default
>>>>> value and the step for TXCAL45DP/DN are changed at next generation SoCs,
>>>>> so I am wondering how we describe it at binding doc.
>>>>>
>>>>
>>>> Which next gen SoC would this be?  The MX7?  The documentation for the USB
>>>> PHY in that reference manual is even sparser than the one for the MX6 in
>>>> regards to these register values.
>>>>
>>>
>>> Here, I mean i.mx8
>>>
>>
>> Currently, there is no support in the mainline kernel for the i.MX8.
>> Do you mean to say that this feature is blocked until the i.MX8 is
>> supported in the mainline kernel? Or that we would be required to add
>> the register definitions for the i.MX8 as a prerequisite? Wouldn't it
>> make more sense to add support to the driver as part of the i.MX8
>> enablement, especially considering no documentation is freely
>> available at this time?
>>
>>>> The MX7 manual does still mention that HW_USBPHY_TX_TXCAL45DP and
>>>> HW_USBPHY_TX_TXCAL45DN should both be set to zero, but there is no listing
>>>> as to the location of these registers, let alone their defaults/step values.
>>>>
>>>> Do you know where we could get the default and step values for the TXCAL
>>>> registers on the new SoC?  This information had to come from a Freescale
>>>> community thread for the MX6 since it wasn't defined clearly elsewhere.
>>>
>>> In theory, these information should be listed at SoC reference manual.
>>>
>>
>> I have not seen the reference manual, so I'm not sure if it's in there
>> or not. However, if the i.MX6 manual is any indication, I'm not
>> certain it will be documented anyway.
>>
>> How do we unblock this issue? We currently have hardware that cannot
>> meet specifications without this feature, and based on the app note
>> from NXP, I'm certain plenty of other people do as well.
>>
>
> I agree with you that we don't need to consider i.mx8 case now.
>
> According to your previous information, the single formula can't be used
> to get D_CAL value, do you have any good ways except for using a table?

That is not exactly what I said:

"I can't find any formula which would hit all of those same percentages 
**without rounding up.**"

I did find a formula that hits all of the percentages with rounding up, 
and I listed this formula (in shell form) previously. The rounding (+ 
(119-80)/2) just makes it look a little more awkward than I would like:

$ for d in 119 116 114 112 109 106 103 100 97 95 93 90 88 86 83 79; do 
echo "$d="$(( ( (119-$d) * 0xf + (119-80)/2 ) / (119-80) )); d=$((d+1)); 
done
119=0
116=1
114=2
112=3
109=4
106=5
103=6
100=7
97=8
95=9
93=10
90=11
88=12
86=13
83=14
79=15

These current percentage/register value pairs match the D_CAL table.


 > Sending the patch for new algorithm to speed up this issue please.
 >
There were also some Documentation changes from between when I created 
this patch and the current HOL for which I had to modify the patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] usb: phy: mxs: Add DT bindings to configure TX settings
       [not found]                                         ` <575F1A57.6030807-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
@ 2016-06-14  2:06                                           ` Peter Chen
  0 siblings, 0 replies; 17+ messages in thread
From: Peter Chen @ 2016-06-14  2:06 UTC (permalink / raw)
  To: Jaret Cantu
  Cc: Justin Waters, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	felipe.balbi-VuQAYsv1563Yd54FQh9/CA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A

On Mon, Jun 13, 2016 at 04:40:55PM -0400, Jaret Cantu wrote:
> >
> >According to your previous information, the single formula can't be used
> >to get D_CAL value, do you have any good ways except for using a table?
> 
> That is not exactly what I said:
> 
> "I can't find any formula which would hit all of those same
> percentages **without rounding up.**"
> 

Sorry, my careless.

> I did find a formula that hits all of the percentages with rounding
> up, and I listed this formula (in shell form) previously. The
> rounding (+ (119-80)/2) just makes it look a little more awkward
> than I would like:
> 
> $ for d in 119 116 114 112 109 106 103 100 97 95 93 90 88 86 83 79;
> do echo "$d="$(( ( (119-$d) * 0xf + (119-80)/2 ) / (119-80) ));
> d=$((d+1)); done
> 119=0
> 116=1
> 114=2
> 112=3
> 109=4
> 106=5
> 103=6
> 100=7
> 97=8
> 95=9
> 93=10
> 90=11
> 88=12
> 86=13
> 83=14
> 79=15
> 
> These current percentage/register value pairs match the D_CAL table.
> 
> 
> > Sending the patch for new algorithm to speed up this issue please.
> >
> There were also some Documentation changes from between when I
> created this patch and the current HOL for which I had to modify the
> patch.

I will review and test your v4 patch, thanks.

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2016-06-14  2:06 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1457040571-7775-1-git-send-email-jaret.cantu@timesys.com>
     [not found] ` <1457040571-7775-1-git-send-email-jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
2016-03-15 16:54   ` [PATCH v2] usb: phy: mxs: Add DT bindings to configure TX settings Jaret Cantu
     [not found]     ` <1458060853-15115-1-git-send-email-jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
2016-03-18 21:19       ` Rob Herring
2016-03-21 16:32         ` [PATCH v3] " Jaret Cantu
     [not found]           ` <1458577947-12614-1-git-send-email-jaret.cantu-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
2016-03-23  1:53             ` Peter Chen
2016-03-23  4:36             ` Peter Chen
2016-03-23 17:37               ` Jaret Cantu
     [not found]                 ` <56F2D43D.1040404-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
2016-03-23 18:17                   ` Jaret Cantu
     [not found]                     ` <56F2DDB7.7070700-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
2016-03-24  2:21                       ` Peter Chen
2016-03-30 10:29                         ` Felipe Balbi
     [not found]                           ` <874mboqmwh.fsf-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2016-03-31  6:49                             ` Peter Chen
2016-06-08 21:27                         ` Jaret Cantu
     [not found]                           ` <57588DA8.4050000-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
2016-06-09  2:41                             ` Peter Chen
     [not found]                               ` <CAL411-of2Phqx18LBaGt6FMg5DxaJesRYvrRzsfSNyJRopsUZQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-06-09 18:07                                 ` Justin Waters
     [not found]                                   ` <CAENNV6+978syhexzQber58xuai=grC2_TsVF3GmHCAp_Zj=HZQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-06-12  3:25                                     ` Peter Chen
2016-06-13 20:40                                       ` Jaret Cantu
     [not found]                                         ` <575F1A57.6030807-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
2016-06-14  2:06                                           ` Peter Chen
2016-03-23 14:54             ` Rob Herring

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).