[PATCH 1/6] dt-bindings: nvmem: mediatek: add support bits property

[PATCH 1/6] dt-bindings: nvmem: mediatek: add support bits property

[Chunfeng Yun]

Add support bits property, will satisfy more consumers.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
Need convert mtk-efuse.txt into yaml first?
---
 Documentation/devicetree/bindings/nvmem/mtk-efuse.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt b/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt
index b6791702bcfc..ec70c5b7a340 100644
--- a/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt
+++ b/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt
@@ -10,6 +10,7 @@ Required properties:
 	      "mediatek,mt8192-efuse", "mediatek,efuse": for MT8192
 	      "mediatek,mt8516-efuse", "mediatek,efuse": for MT8516
 - reg: Should contain registers location and length
+- bits: contain the bits range by offset and size
 
 = Data cells =
 Are child nodes of MTK-EFUSE, bindings of which as described in
-- 
2.18.0

[PATCH 2/6] dt-bindings: nvmem: mediatek: add support for mt8195

[Chunfeng Yun]

Add compatible for mt8195

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 Documentation/devicetree/bindings/nvmem/mtk-efuse.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt b/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt
index ec70c5b7a340..39d529599444 100644
--- a/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt
+++ b/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt
@@ -8,6 +8,7 @@ Required properties:
 	      "mediatek,mt7623-efuse", "mediatek,efuse": for MT7623
 	      "mediatek,mt8173-efuse" or "mediatek,efuse": for MT8173
 	      "mediatek,mt8192-efuse", "mediatek,efuse": for MT8192
+	      "mediatek,mt8195-efuse", "mediatek,efuse": for MT8195
 	      "mediatek,mt8516-efuse", "mediatek,efuse": for MT8516
 - reg: Should contain registers location and length
 - bits: contain the bits range by offset and size
-- 
2.18.0

[PATCH 3/6] nvmem: mtk-efuse: support minimum one byte access stride and granularity

[Chunfeng Yun]

In order to support nvmem bits property, should support minimum 1 byte
read stride and minimum 1 byte read granularity at the same time.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/nvmem/mtk-efuse.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c
index 6a537d959f14..e9a375dd84af 100644
--- a/drivers/nvmem/mtk-efuse.c
+++ b/drivers/nvmem/mtk-efuse.c
@@ -19,11 +19,12 @@ static int mtk_reg_read(void *context,
 			unsigned int reg, void *_val, size_t bytes)
 {
 	struct mtk_efuse_priv *priv = context;
-	u32 *val = _val;
-	int i = 0, words = bytes / 4;
+	void __iomem *addr = priv->base + reg;
+	u8 *val = _val;
+	int i;
 
-	while (words--)
-		*val++ = readl(priv->base + reg + (i++ * 4));
+	for (i = 0; i < bytes; i++, val++)
+		*val = readb(addr + i);
 
 	return 0;
 }
@@ -45,8 +46,8 @@ static int mtk_efuse_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->base))
 		return PTR_ERR(priv->base);
 
-	econfig.stride = 4;
-	econfig.word_size = 4;
+	econfig.stride = 1;
+	econfig.word_size = 1;
 	econfig.reg_read = mtk_reg_read;
 	econfig.size = resource_size(res);
 	econfig.priv = priv;
-- 
2.18.0

[PATCH 4/6] dt-bindings: phy: mediatek: tphy: support software efuse load

[Chunfeng Yun]

Add optional property nvmem-cells and nvmem-cell-names to support
software efuse load, this helps to fix the efuse bit shift issue
on mt8195 etc.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 .../devicetree/bindings/phy/mediatek,tphy.yaml | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
index 9e6c0f43f1c6..05ee274b4b71 100644
--- a/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
+++ b/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
@@ -160,6 +160,24 @@ patternProperties:
                 - PHY_TYPE_PCIE
                 - PHY_TYPE_SATA
 
+      nvmem-cells:
+        items:
+          - description: internal R efuse for U2 PHY or U3/PCIe PHY
+          - description: rx_imp_sel efuse for U3/PCIe PHY
+          - description: tx_imp_sel efuse for U3/PCIe PHY
+        description: |
+          Phandles to nvmem cell that contains the efuse data;
+          Available only for U2 PHY or U3/PCIe PHY of version 2/3, these
+          three items should be provided at the same time for U3/PCIe PHY,
+          when use software to load efuse;
+          If unspecified, will use hardware auto-load efuse.
+
+      nvmem-cell-names:
+        items:
+          - const: intr
+          - const: rx_imp
+          - const: tx_imp
+
       # The following optional vendor properties are only for debug or HQA test
       mediatek,eye-src:
         description:
-- 
2.18.0

[PATCH 5/6] phy: phy-mtk-tphy: add support efuse setting

[Chunfeng Yun]

Due to some SoCs have a bit shift issue that will drop a bit for usb3
phy or pcie phy, fix it by adding software efuse reading and setting,
but only support it optionally for versoin 2/3.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/phy/mediatek/phy-mtk-tphy.c | 162 ++++++++++++++++++++++++++++
 1 file changed, 162 insertions(+)

diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c b/drivers/phy/mediatek/phy-mtk-tphy.c
index cdcef865fe9e..3b5b1c266595 100644
--- a/drivers/phy/mediatek/phy-mtk-tphy.c
+++ b/drivers/phy/mediatek/phy-mtk-tphy.c
@@ -12,6 +12,7 @@
 #include <linux/iopoll.h>
 #include <linux/mfd/syscon.h>
 #include <linux/module.h>
+#include <linux/nvmem-consumer.h>
 #include <linux/of_address.h>
 #include <linux/of_device.h>
 #include <linux/phy/phy.h>
@@ -41,6 +42,9 @@
 #define SSUSB_SIFSLV_V2_U3PHYD		0x200
 #define SSUSB_SIFSLV_V2_U3PHYA		0x400
 
+#define U3P_MISC_REG1		0x04
+#define MR1_EFUSE_AUTO_LOAD_DIS		BIT(6)
+
 #define U3P_USBPHYACR0		0x000
 #define PA0_RG_U2PLL_FORCE_ON		BIT(15)
 #define PA0_USB20_PLL_PREDIV		GENMASK(7, 6)
@@ -133,6 +137,8 @@
 #define P3C_RG_SWRST_U3_PHYD_FORCE_EN	BIT(24)
 
 #define U3P_U3_PHYA_REG0	0x000
+#define P3A_RG_IEXT_INTR		GENMASK(15, 10)
+#define P3A_RG_IEXT_INTR_VAL(x)		((0x3f & (x)) << 10)
 #define P3A_RG_CLKDRV_OFF		GENMASK(3, 2)
 #define P3A_RG_CLKDRV_OFF_VAL(x)	((0x3 & (x)) << 2)
 
@@ -187,6 +193,19 @@
 #define P3D_RG_FWAKE_TH		GENMASK(21, 16)
 #define P3D_RG_FWAKE_TH_VAL(x)	((0x3f & (x)) << 16)
 
+#define U3P_U3_PHYD_IMPCAL0		0x010
+#define P3D_RG_FORCE_TX_IMPEL		BIT(31)
+#define P3D_RG_TX_IMPEL			GENMASK(28, 24)
+#define P3D_RG_TX_IMPEL_VAL(x)		((0x1f & (x)) << 24)
+
+#define U3P_U3_PHYD_IMPCAL1		0x014
+#define P3D_RG_FORCE_RX_IMPEL		BIT(31)
+#define P3D_RG_RX_IMPEL			GENMASK(28, 24)
+#define P3D_RG_RX_IMPEL_VAL(x)		((0x1f & (x)) << 24)
+
+#define U3P_U3_PHYD_RSV			0x054
+#define P3D_RG_EFUSE_AUTO_LOAD_DIS	BIT(12)
+
 #define U3P_U3_PHYD_CDR1		0x05c
 #define P3D_RG_CDR_BIR_LTD1		GENMASK(28, 24)
 #define P3D_RG_CDR_BIR_LTD1_VAL(x)	((0x1f & (x)) << 24)
@@ -307,6 +326,11 @@ struct mtk_phy_pdata {
 	 * 48M PLL, fix it by switching PLL to 26M from default 48M
 	 */
 	bool sw_pll_48m_to_26m;
+	/*
+	 * Some SoCs (e.g. mt8195) drop a bit when use auto load efuse,
+	 * support sw way, also support it for v2/v3 optionally.
+	 */
+	bool sw_efuse_supported;
 	enum mtk_phy_version version;
 };
 
@@ -336,6 +360,10 @@ struct mtk_phy_instance {
 	struct regmap *type_sw;
 	u32 type_sw_reg;
 	u32 type_sw_index;
+	u32 efuse_sw_en;
+	u32 efuse_intr;
+	u32 efuse_tx_imp;
+	u32 efuse_rx_imp;
 	int eye_src;
 	int eye_vrt;
 	int eye_term;
@@ -1040,6 +1068,130 @@ static int phy_type_set(struct mtk_phy_instance *instance)
 	return 0;
 }
 
+static int phy_efuse_get(struct mtk_tphy *tphy, struct mtk_phy_instance *instance)
+{
+	struct device *dev = &instance->phy->dev;
+	int ret = 0;
+
+	/* tphy v1 doesn't support sw efuse, skip it */
+	if (!tphy->pdata->sw_efuse_supported) {
+		instance->efuse_sw_en = 0;
+		return 0;
+	}
+
+	/* software efuse is optional */
+	instance->efuse_sw_en = device_property_read_bool(dev, "nvmem-cells");
+	if (!instance->efuse_sw_en)
+		return 0;
+
+	switch (instance->type) {
+	case PHY_TYPE_USB2:
+		ret = nvmem_cell_read_variable_le_u32(dev, "intr", &instance->efuse_intr);
+		if (ret) {
+			dev_err(dev, "fail to get u2 intr efuse, %d\n", ret);
+			break;
+		}
+
+		/* no efuse, ignore it */
+		if (!instance->efuse_intr) {
+			dev_warn(dev, "no u2 intr efuse, but dts enable it\n");
+			instance->efuse_sw_en = 0;
+			break;
+		}
+
+		dev_info(dev, "u2 efuse - intr %x\n", instance->efuse_intr);
+		break;
+	case PHY_TYPE_USB3:
+	case PHY_TYPE_PCIE:
+		ret = nvmem_cell_read_variable_le_u32(dev, "intr", &instance->efuse_intr);
+		if (ret) {
+			dev_err(dev, "fail to get u3 intr efuse, %d\n", ret);
+			break;
+		}
+
+		ret = nvmem_cell_read_variable_le_u32(dev, "rx_imp", &instance->efuse_rx_imp);
+		if (ret) {
+			dev_err(dev, "fail to get u3 rx_imp efuse, %d\n", ret);
+			break;
+		}
+
+		ret = nvmem_cell_read_variable_le_u32(dev, "tx_imp", &instance->efuse_tx_imp);
+		if (ret) {
+			dev_err(dev, "fail to get u3 tx_imp efuse, %d\n", ret);
+			break;
+		}
+
+		/* no efuse, ignore it */
+		if (!instance->efuse_intr &&
+		    !instance->efuse_rx_imp &&
+		    !instance->efuse_rx_imp) {
+			dev_warn(dev, "no u3 intr efuse, but dts enable it\n");
+			instance->efuse_sw_en = 0;
+			break;
+		}
+
+		dev_info(dev, "u3 efuse - intr %x, rx_imp %x, tx_imp %x\n",
+			 instance->efuse_intr, instance->efuse_rx_imp,
+			 instance->efuse_tx_imp);
+		break;
+	default:
+		dev_err(dev, "no sw efuse for type %d\n", instance->type);
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+static void phy_efuse_set(struct mtk_phy_instance *instance)
+{
+	struct device *dev = &instance->phy->dev;
+	struct u2phy_banks *u2_banks = &instance->u2_banks;
+	struct u3phy_banks *u3_banks = &instance->u3_banks;
+	u32 tmp;
+
+	if (!instance->efuse_sw_en)
+		return;
+
+	switch (instance->type) {
+	case PHY_TYPE_USB2:
+		tmp = readl(u2_banks->misc + U3P_MISC_REG1);
+		tmp |= MR1_EFUSE_AUTO_LOAD_DIS;
+		writel(tmp, u2_banks->misc + U3P_MISC_REG1);
+
+		tmp = readl(u2_banks->com + U3P_USBPHYACR1);
+		tmp &= ~PA1_RG_INTR_CAL;
+		tmp |= PA1_RG_INTR_CAL_VAL(instance->efuse_intr);
+		writel(tmp, u2_banks->com + U3P_USBPHYACR1);
+		break;
+	case PHY_TYPE_USB3:
+	case PHY_TYPE_PCIE:
+		tmp = readl(u3_banks->phyd + U3P_U3_PHYD_RSV);
+		tmp |= P3D_RG_EFUSE_AUTO_LOAD_DIS;
+		writel(tmp, u3_banks->phyd + U3P_U3_PHYD_RSV);
+
+		tmp = readl(u3_banks->phyd + U3P_U3_PHYD_IMPCAL0);
+		tmp &= ~P3D_RG_TX_IMPEL;
+		tmp |= P3D_RG_TX_IMPEL_VAL(instance->efuse_tx_imp);
+		tmp |= P3D_RG_FORCE_TX_IMPEL;
+		writel(tmp, u3_banks->phyd + U3P_U3_PHYD_IMPCAL0);
+
+		tmp = readl(u3_banks->phyd + U3P_U3_PHYD_IMPCAL1);
+		tmp &= ~P3D_RG_RX_IMPEL;
+		tmp |= P3D_RG_RX_IMPEL_VAL(instance->efuse_rx_imp);
+		tmp |= P3D_RG_FORCE_RX_IMPEL;
+		writel(tmp, u3_banks->phyd + U3P_U3_PHYD_IMPCAL1);
+
+		tmp = readl(u3_banks->phya + U3P_U3_PHYA_REG0);
+		tmp &= ~P3A_RG_IEXT_INTR;
+		tmp |= P3A_RG_IEXT_INTR_VAL(instance->efuse_intr);
+		writel(tmp, u3_banks->phya + U3P_U3_PHYA_REG0);
+		break;
+	default:
+		dev_warn(dev, "no sw efuse for type %d\n", instance->type);
+		break;
+	}
+}
+
 static int mtk_phy_init(struct phy *phy)
 {
 	struct mtk_phy_instance *instance = phy_get_drvdata(phy);
@@ -1050,6 +1202,8 @@ static int mtk_phy_init(struct phy *phy)
 	if (ret)
 		return ret;
 
+	phy_efuse_set(instance);
+
 	switch (instance->type) {
 	case PHY_TYPE_USB2:
 		u2_phy_instance_init(tphy, instance);
@@ -1134,6 +1288,7 @@ static struct phy *mtk_phy_xlate(struct device *dev,
 	struct mtk_phy_instance *instance = NULL;
 	struct device_node *phy_np = args->np;
 	int index;
+	int ret;
 
 	if (args->args_count != 1) {
 		dev_err(dev, "invalid number of cells in 'phy' property\n");
@@ -1174,6 +1329,10 @@ static struct phy *mtk_phy_xlate(struct device *dev,
 		return ERR_PTR(-EINVAL);
 	}
 
+	ret = phy_efuse_get(tphy, instance);
+	if (ret)
+		return ERR_PTR(ret);
+
 	phy_parse_property(tphy, instance);
 	phy_type_set(instance);
 
@@ -1196,10 +1355,12 @@ static const struct mtk_phy_pdata tphy_v1_pdata = {
 
 static const struct mtk_phy_pdata tphy_v2_pdata = {
 	.avoid_rx_sen_degradation = false,
+	.sw_efuse_supported = true,
 	.version = MTK_PHY_V2,
 };
 
 static const struct mtk_phy_pdata tphy_v3_pdata = {
+	.sw_efuse_supported = true,
 	.version = MTK_PHY_V3,
 };
 
@@ -1210,6 +1371,7 @@ static const struct mtk_phy_pdata mt8173_pdata = {
 
 static const struct mtk_phy_pdata mt8195_pdata = {
 	.sw_pll_48m_to_26m = true,
+	.sw_efuse_supported = true,
 	.version = MTK_PHY_V3,
 };
 
-- 
2.18.0

[PATCH 6/6] arm64: dts: mediatek: mt8195: add efuse node and cells

[Chunfeng Yun]

Add efuse node and cells used by t-phy to fix the bit shift issue

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
Depend on:

  [v4,1/1] arm64: dts: Add Mediatek SoC MT8195 and evaluation board dts and Makefile
  https://patchwork.kernel.org/patch/12509911
---
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 61 ++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 263eebfd2ea1..7fb23c1cb8cc 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -933,6 +933,55 @@
 			status = "disabled";
 		};
 
+		efuse: efuse@11c10000 {
+			compatible = "mediatek,mt8195-efuse", "mediatek,efuse";
+			reg = <0 0x11c10000 0 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			status = "disabled";
+
+			u3_tx_imp_p0: usb3-tx-imp@184 {
+				reg = <0x184 0x1>;
+				bits = <0 5>;
+			};
+			u3_rx_imp_p0: usb3-rx-imp@184 {
+				reg = <0x184 0x2>;
+				bits = <5 5>;
+			};
+			u3_intr_p0: usb3-intr@185 {
+				reg = <0x185 0x1>;
+				bits = <2 6>;
+			};
+			comb_tx_imp_p1: usb3-tx-imp@186 {
+				reg = <0x186 0x1>;
+				bits = <0 5>;
+			};
+			comb_rx_imp_p1: usb3-rx-imp@186 {
+				reg = <0x186 0x2>;
+				bits = <5 5>;
+			};
+			comb_intr_p1: usb3-intr@187 {
+				reg = <0x187 0x1>;
+				bits = <2 6>;
+			};
+			u2_intr_p0: usb2-intr-p0@188 {
+				reg = <0x188 0x1>;
+				bits = <0 5>;
+			};
+			u2_intr_p1: usb2-intr-p1@188 {
+				reg = <0x188 0x2>;
+				bits = <5 5>;
+			};
+			u2_intr_p2: usb2-intr-p2@189 {
+				reg = <0x189 0x1>;
+				bits = <2 5>;
+			};
+			u2_intr_p3: usb2-intr-p3@189 {
+				reg = <0x189 0x2>;
+				bits = <7 5>;
+			};
+		};
+
 		u3phy2: t-phy@11c40000 {
 			compatible = "mediatek,mt8195-tphy", "mediatek,generic-tphy-v2";
 			#address-cells = <1>;
@@ -986,6 +1035,8 @@
 				reg = <0x0 0x700>;
 				clocks = <&clk26m>;
 				clock-names = "ref";
+				nvmem-cells = <&u2_intr_p1>;
+				nvmem-cell-names = "intr";
 				#phy-cells = <1>;
 			};
 
@@ -993,6 +1044,10 @@
 				reg = <0x700 0x700>;
 				clocks = <&clk26m>;
 				clock-names = "ref";
+				nvmem-cells = <&comb_intr_p1>,
+					      <&comb_rx_imp_p1>,
+					      <&comb_tx_imp_p1>;
+				nvmem-cell-names = "intr", "rx_imp", "tx_imp";
 				#phy-cells = <1>;
 			};
 		};
@@ -1008,6 +1063,8 @@
 				reg = <0x0 0x700>;
 				clocks = <&clk26m>;
 				clock-names = "ref";
+				nvmem-cells = <&u2_intr_p0>;
+				nvmem-cell-names = "intr";
 				#phy-cells = <1>;
 			};
 
@@ -1015,6 +1072,10 @@
 				reg = <0x700 0x700>;
 				clocks = <&clk26m>;
 				clock-names = "ref";
+				nvmem-cells = <&u3_intr_p0>,
+					      <&u3_rx_imp_p0>,
+					      <&u3_tx_imp_p0>;
+				nvmem-cell-names = "intr", "rx_imp", "tx_imp";
 				#phy-cells = <1>;
 			};
 		};
-- 
2.18.0

Re: [PATCH 1/6] dt-bindings: nvmem: mediatek: add support bits property

[Rob Herring]

On Sun, 07 Nov 2021 15:56:41 +0800, Chunfeng Yun wrote:
> Add support bits property, will satisfy more consumers.
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> Need convert mtk-efuse.txt into yaml first?
> ---
>  Documentation/devicetree/bindings/nvmem/mtk-efuse.txt | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 2/6] dt-bindings: nvmem: mediatek: add support for mt8195

[Rob Herring]

On Sun, 07 Nov 2021 15:56:42 +0800, Chunfeng Yun wrote:
> Add compatible for mt8195
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  Documentation/devicetree/bindings/nvmem/mtk-efuse.txt | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 4/6] dt-bindings: phy: mediatek: tphy: support software efuse load

[Rob Herring]

On Sun, 07 Nov 2021 15:56:44 +0800, Chunfeng Yun wrote:
> Add optional property nvmem-cells and nvmem-cell-names to support
> software efuse load, this helps to fix the efuse bit shift issue
> on mt8195 etc.
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  .../devicetree/bindings/phy/mediatek,tphy.yaml | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 3/6] nvmem: mtk-efuse: support minimum one byte access stride and granularity

[Srinivas Kandagatla]

On 07/11/2021 07:56, Chunfeng Yun wrote:
> In order to support nvmem bits property, should support minimum 1 byte
> read stride and minimum 1 byte read granularity at the same time.
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
Applied thanks,

--srini

> ---
>   drivers/nvmem/mtk-efuse.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c
> index 6a537d959f14..e9a375dd84af 100644
> --- a/drivers/nvmem/mtk-efuse.c
> +++ b/drivers/nvmem/mtk-efuse.c
> @@ -19,11 +19,12 @@ static int mtk_reg_read(void *context,
>   			unsigned int reg, void *_val, size_t bytes)
>   {
>   	struct mtk_efuse_priv *priv = context;
> -	u32 *val = _val;
> -	int i = 0, words = bytes / 4;
> +	void __iomem *addr = priv->base + reg;
> +	u8 *val = _val;
> +	int i;
>   
> -	while (words--)
> -		*val++ = readl(priv->base + reg + (i++ * 4));
> +	for (i = 0; i < bytes; i++, val++)
> +		*val = readb(addr + i);
>   
>   	return 0;
>   }
> @@ -45,8 +46,8 @@ static int mtk_efuse_probe(struct platform_device *pdev)
>   	if (IS_ERR(priv->base))
>   		return PTR_ERR(priv->base);
>   
> -	econfig.stride = 4;
> -	econfig.word_size = 4;
> +	econfig.stride = 1;
> +	econfig.word_size = 1;
>   	econfig.reg_read = mtk_reg_read;
>   	econfig.size = resource_size(res);
>   	econfig.priv = priv;
> 

Re: [PATCH 1/6] dt-bindings: nvmem: mediatek: add support bits property

[Srinivas Kandagatla]

On 07/11/2021 07:56, Chunfeng Yun wrote:
> Add support bits property, will satisfy more consumers.
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>


Applied thanks,

--srini
> ---
> Need convert mtk-efuse.txt into yaml first?
> ---
>   Documentation/devicetree/bindings/nvmem/mtk-efuse.txt | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt b/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt
> index b6791702bcfc..ec70c5b7a340 100644
> --- a/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt
> +++ b/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt
> @@ -10,6 +10,7 @@ Required properties:
>   	      "mediatek,mt8192-efuse", "mediatek,efuse": for MT8192
>   	      "mediatek,mt8516-efuse", "mediatek,efuse": for MT8516
>   - reg: Should contain registers location and length
> +- bits: contain the bits range by offset and size
>   
>   = Data cells =
>   Are child nodes of MTK-EFUSE, bindings of which as described in
> 

Re: [PATCH 4/6] dt-bindings: phy: mediatek: tphy: support software efuse load

[Srinivas Kandagatla]

On 07/11/2021 07:56, Chunfeng Yun wrote:
> Add optional property nvmem-cells and nvmem-cell-names to support
> software efuse load, this helps to fix the efuse bit shift issue
> on mt8195 etc.
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>

Applied thanks,

--srini

> ---
>   .../devicetree/bindings/phy/mediatek,tphy.yaml | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
> index 9e6c0f43f1c6..05ee274b4b71 100644
> --- a/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
> @@ -160,6 +160,24 @@ patternProperties:
>                   - PHY_TYPE_PCIE
>                   - PHY_TYPE_SATA
>   
> +      nvmem-cells:
> +        items:
> +          - description: internal R efuse for U2 PHY or U3/PCIe PHY
> +          - description: rx_imp_sel efuse for U3/PCIe PHY
> +          - description: tx_imp_sel efuse for U3/PCIe PHY
> +        description: |
> +          Phandles to nvmem cell that contains the efuse data;
> +          Available only for U2 PHY or U3/PCIe PHY of version 2/3, these
> +          three items should be provided at the same time for U3/PCIe PHY,
> +          when use software to load efuse;
> +          If unspecified, will use hardware auto-load efuse.
> +
> +      nvmem-cell-names:
> +        items:
> +          - const: intr
> +          - const: rx_imp
> +          - const: tx_imp
> +
>         # The following optional vendor properties are only for debug or HQA test
>         mediatek,eye-src:
>           description:
> 

Re: [PATCH 2/6] dt-bindings: nvmem: mediatek: add support for mt8195

[Srinivas Kandagatla]

On 07/11/2021 07:56, Chunfeng Yun wrote:
> Add compatible for mt8195
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>

Applied thanks,

--srini

> ---
>   Documentation/devicetree/bindings/nvmem/mtk-efuse.txt | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt b/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt
> index ec70c5b7a340..39d529599444 100644
> --- a/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt
> +++ b/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt
> @@ -8,6 +8,7 @@ Required properties:
>   	      "mediatek,mt7623-efuse", "mediatek,efuse": for MT7623
>   	      "mediatek,mt8173-efuse" or "mediatek,efuse": for MT8173
>   	      "mediatek,mt8192-efuse", "mediatek,efuse": for MT8192
> +	      "mediatek,mt8195-efuse", "mediatek,efuse": for MT8195
>   	      "mediatek,mt8516-efuse", "mediatek,efuse": for MT8516
>   - reg: Should contain registers location and length
>   - bits: contain the bits range by offset and size
> 

Re: [PATCH 4/6] dt-bindings: phy: mediatek: tphy: support software efuse load

[Chen-Yu Tsai]

On Sat, Nov 20, 2021 at 1:19 AM Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
> On 07/11/2021 07:56, Chunfeng Yun wrote:
> > Add optional property nvmem-cells and nvmem-cell-names to support
> > software efuse load, this helps to fix the efuse bit shift issue
> > on mt8195 etc.
> >
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
>
> Applied thanks,

This is a PHY DT binding change. Shouldn't it go in with patch 5, the
phy driver patch, through the PHY tree instead?

ChenYu

> --srini
>
> > ---
> >   .../devicetree/bindings/phy/mediatek,tphy.yaml | 18 ++++++++++++++++++
> >   1 file changed, 18 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
> > index 9e6c0f43f1c6..05ee274b4b71 100644
> > --- a/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
> > +++ b/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
> > @@ -160,6 +160,24 @@ patternProperties:
> >                   - PHY_TYPE_PCIE
> >                   - PHY_TYPE_SATA
> >
> > +      nvmem-cells:
> > +        items:
> > +          - description: internal R efuse for U2 PHY or U3/PCIe PHY
> > +          - description: rx_imp_sel efuse for U3/PCIe PHY
> > +          - description: tx_imp_sel efuse for U3/PCIe PHY
> > +        description: |
> > +          Phandles to nvmem cell that contains the efuse data;
> > +          Available only for U2 PHY or U3/PCIe PHY of version 2/3, these
> > +          three items should be provided at the same time for U3/PCIe PHY,
> > +          when use software to load efuse;
> > +          If unspecified, will use hardware auto-load efuse.
> > +
> > +      nvmem-cell-names:
> > +        items:
> > +          - const: intr
> > +          - const: rx_imp
> > +          - const: tx_imp
> > +
> >         # The following optional vendor properties are only for debug or HQA test
> >         mediatek,eye-src:
> >           description:
> >
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 4/6] dt-bindings: phy: mediatek: tphy: support software efuse load

[Srinivas Kandagatla]

On 22/11/2021 04:05, Chen-Yu Tsai wrote:
> On Sat, Nov 20, 2021 at 1:19 AM Srinivas Kandagatla
> <srinivas.kandagatla@linaro.org> wrote:
>> On 07/11/2021 07:56, Chunfeng Yun wrote:
>>> Add optional property nvmem-cells and nvmem-cell-names to support
>>> software efuse load, this helps to fix the efuse bit shift issue
>>> on mt8195 etc.
>>>
>>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
>>
>> Applied thanks,
> 
> This is a PHY DT binding change. Shouldn't it go in with patch 5, the
> phy driver patch, through the PHY tree instead?

That's true, this is dropped from nvmem tree now!

--srini
> 
> ChenYu
> 
>> --srini
>>
>>> ---
>>>    .../devicetree/bindings/phy/mediatek,tphy.yaml | 18 ++++++++++++++++++
>>>    1 file changed, 18 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
>>> index 9e6c0f43f1c6..05ee274b4b71 100644
>>> --- a/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
>>> +++ b/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
>>> @@ -160,6 +160,24 @@ patternProperties:
>>>                    - PHY_TYPE_PCIE
>>>                    - PHY_TYPE_SATA
>>>
>>> +      nvmem-cells:
>>> +        items:
>>> +          - description: internal R efuse for U2 PHY or U3/PCIe PHY
>>> +          - description: rx_imp_sel efuse for U3/PCIe PHY
>>> +          - description: tx_imp_sel efuse for U3/PCIe PHY
>>> +        description: |
>>> +          Phandles to nvmem cell that contains the efuse data;
>>> +          Available only for U2 PHY or U3/PCIe PHY of version 2/3, these
>>> +          three items should be provided at the same time for U3/PCIe PHY,
>>> +          when use software to load efuse;
>>> +          If unspecified, will use hardware auto-load efuse.
>>> +
>>> +      nvmem-cell-names:
>>> +        items:
>>> +          - const: intr
>>> +          - const: rx_imp
>>> +          - const: tx_imp
>>> +
>>>          # The following optional vendor properties are only for debug or HQA test
>>>          mediatek,eye-src:
>>>            description:
>>>
>>
>> _______________________________________________
>> Linux-mediatek mailing list
>> Linux-mediatek@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 5/6] phy: phy-mtk-tphy: add support efuse setting

[Vinod Koul]

On 07-11-21, 15:56, Chunfeng Yun wrote:
> Due to some SoCs have a bit shift issue that will drop a bit for usb3
> phy or pcie phy, fix it by adding software efuse reading and setting,
> but only support it optionally for versoin 2/3.

s/versoin/version

> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  drivers/phy/mediatek/phy-mtk-tphy.c | 162 ++++++++++++++++++++++++++++
>  1 file changed, 162 insertions(+)
> 
> diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c b/drivers/phy/mediatek/phy-mtk-tphy.c
> index cdcef865fe9e..3b5b1c266595 100644
> --- a/drivers/phy/mediatek/phy-mtk-tphy.c
> +++ b/drivers/phy/mediatek/phy-mtk-tphy.c
> @@ -12,6 +12,7 @@
>  #include <linux/iopoll.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
>  #include <linux/of_address.h>
>  #include <linux/of_device.h>
>  #include <linux/phy/phy.h>
> @@ -41,6 +42,9 @@
>  #define SSUSB_SIFSLV_V2_U3PHYD		0x200
>  #define SSUSB_SIFSLV_V2_U3PHYA		0x400
>  
> +#define U3P_MISC_REG1		0x04
> +#define MR1_EFUSE_AUTO_LOAD_DIS		BIT(6)
> +
>  #define U3P_USBPHYACR0		0x000
>  #define PA0_RG_U2PLL_FORCE_ON		BIT(15)
>  #define PA0_USB20_PLL_PREDIV		GENMASK(7, 6)
> @@ -133,6 +137,8 @@
>  #define P3C_RG_SWRST_U3_PHYD_FORCE_EN	BIT(24)
>  
>  #define U3P_U3_PHYA_REG0	0x000
> +#define P3A_RG_IEXT_INTR		GENMASK(15, 10)
> +#define P3A_RG_IEXT_INTR_VAL(x)		((0x3f & (x)) << 10)
>  #define P3A_RG_CLKDRV_OFF		GENMASK(3, 2)
>  #define P3A_RG_CLKDRV_OFF_VAL(x)	((0x3 & (x)) << 2)
>  
> @@ -187,6 +193,19 @@
>  #define P3D_RG_FWAKE_TH		GENMASK(21, 16)
>  #define P3D_RG_FWAKE_TH_VAL(x)	((0x3f & (x)) << 16)
>  
> +#define U3P_U3_PHYD_IMPCAL0		0x010
> +#define P3D_RG_FORCE_TX_IMPEL		BIT(31)
> +#define P3D_RG_TX_IMPEL			GENMASK(28, 24)
> +#define P3D_RG_TX_IMPEL_VAL(x)		((0x1f & (x)) << 24)
> +
> +#define U3P_U3_PHYD_IMPCAL1		0x014
> +#define P3D_RG_FORCE_RX_IMPEL		BIT(31)
> +#define P3D_RG_RX_IMPEL			GENMASK(28, 24)
> +#define P3D_RG_RX_IMPEL_VAL(x)		((0x1f & (x)) << 24)
> +
> +#define U3P_U3_PHYD_RSV			0x054
> +#define P3D_RG_EFUSE_AUTO_LOAD_DIS	BIT(12)
> +
>  #define U3P_U3_PHYD_CDR1		0x05c
>  #define P3D_RG_CDR_BIR_LTD1		GENMASK(28, 24)
>  #define P3D_RG_CDR_BIR_LTD1_VAL(x)	((0x1f & (x)) << 24)
> @@ -307,6 +326,11 @@ struct mtk_phy_pdata {
>  	 * 48M PLL, fix it by switching PLL to 26M from default 48M
>  	 */
>  	bool sw_pll_48m_to_26m;
> +	/*
> +	 * Some SoCs (e.g. mt8195) drop a bit when use auto load efuse,
> +	 * support sw way, also support it for v2/v3 optionally.
> +	 */
> +	bool sw_efuse_supported;
>  	enum mtk_phy_version version;
>  };
>  
> @@ -336,6 +360,10 @@ struct mtk_phy_instance {
>  	struct regmap *type_sw;
>  	u32 type_sw_reg;
>  	u32 type_sw_index;
> +	u32 efuse_sw_en;
> +	u32 efuse_intr;
> +	u32 efuse_tx_imp;
> +	u32 efuse_rx_imp;
>  	int eye_src;
>  	int eye_vrt;
>  	int eye_term;
> @@ -1040,6 +1068,130 @@ static int phy_type_set(struct mtk_phy_instance *instance)
>  	return 0;
>  }
>  
> +static int phy_efuse_get(struct mtk_tphy *tphy, struct mtk_phy_instance *instance)
> +{
> +	struct device *dev = &instance->phy->dev;
> +	int ret = 0;
> +
> +	/* tphy v1 doesn't support sw efuse, skip it */
> +	if (!tphy->pdata->sw_efuse_supported) {
> +		instance->efuse_sw_en = 0;
> +		return 0;
> +	}
> +
> +	/* software efuse is optional */
> +	instance->efuse_sw_en = device_property_read_bool(dev, "nvmem-cells");
> +	if (!instance->efuse_sw_en)
> +		return 0;
> +
> +	switch (instance->type) {
> +	case PHY_TYPE_USB2:
> +		ret = nvmem_cell_read_variable_le_u32(dev, "intr", &instance->efuse_intr);
> +		if (ret) {
> +			dev_err(dev, "fail to get u2 intr efuse, %d\n", ret);
> +			break;
> +		}
> +
> +		/* no efuse, ignore it */
> +		if (!instance->efuse_intr) {
> +			dev_warn(dev, "no u2 intr efuse, but dts enable it\n");
> +			instance->efuse_sw_en = 0;
> +			break;
> +		}

What does this check do...? so a zero value is not valid..?

> +
> +		dev_info(dev, "u2 efuse - intr %x\n", instance->efuse_intr);

dev_dbg()?

> +		break;

empty line after break improves readability, pls add

> +	case PHY_TYPE_USB3:
> +	case PHY_TYPE_PCIE:
> +		ret = nvmem_cell_read_variable_le_u32(dev, "intr", &instance->efuse_intr);
> +		if (ret) {
> +			dev_err(dev, "fail to get u3 intr efuse, %d\n", ret);
> +			break;
> +		}

This seems to be common, why not read this before switch?

> +
> +		ret = nvmem_cell_read_variable_le_u32(dev, "rx_imp", &instance->efuse_rx_imp);
> +		if (ret) {
> +			dev_err(dev, "fail to get u3 rx_imp efuse, %d\n", ret);
> +			break;
> +		}
> +
> +		ret = nvmem_cell_read_variable_le_u32(dev, "tx_imp", &instance->efuse_tx_imp);
> +		if (ret) {
> +			dev_err(dev, "fail to get u3 tx_imp efuse, %d\n", ret);
> +			break;
> +		}
> +
> +		/* no efuse, ignore it */
> +		if (!instance->efuse_intr &&
> +		    !instance->efuse_rx_imp &&
> +		    !instance->efuse_rx_imp) {
> +			dev_warn(dev, "no u3 intr efuse, but dts enable it\n");
> +			instance->efuse_sw_en = 0;
> +			break;
> +		}

again, zero values are not valid?

> +
> +		dev_info(dev, "u3 efuse - intr %x, rx_imp %x, tx_imp %x\n",
> +			 instance->efuse_intr, instance->efuse_rx_imp,
> +			 instance->efuse_tx_imp);

dbg pls

> +		break;
> +	default:
> +		dev_err(dev, "no sw efuse for type %d\n", instance->type);
> +		ret = -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static void phy_efuse_set(struct mtk_phy_instance *instance)
> +{
> +	struct device *dev = &instance->phy->dev;
> +	struct u2phy_banks *u2_banks = &instance->u2_banks;
> +	struct u3phy_banks *u3_banks = &instance->u3_banks;
> +	u32 tmp;
> +
> +	if (!instance->efuse_sw_en)
> +		return;
> +
> +	switch (instance->type) {
> +	case PHY_TYPE_USB2:
> +		tmp = readl(u2_banks->misc + U3P_MISC_REG1);
> +		tmp |= MR1_EFUSE_AUTO_LOAD_DIS;
> +		writel(tmp, u2_banks->misc + U3P_MISC_REG1);
> +
> +		tmp = readl(u2_banks->com + U3P_USBPHYACR1);
> +		tmp &= ~PA1_RG_INTR_CAL;
> +		tmp |= PA1_RG_INTR_CAL_VAL(instance->efuse_intr);
> +		writel(tmp, u2_banks->com + U3P_USBPHYACR1);
> +		break;
> +	case PHY_TYPE_USB3:
> +	case PHY_TYPE_PCIE:
> +		tmp = readl(u3_banks->phyd + U3P_U3_PHYD_RSV);
> +		tmp |= P3D_RG_EFUSE_AUTO_LOAD_DIS;
> +		writel(tmp, u3_banks->phyd + U3P_U3_PHYD_RSV);

add a updatel() macro and use this here and other places?

-- 
~Vinod

Re: [PATCH 5/6] phy: phy-mtk-tphy: add support efuse setting

[Chunfeng Yun]

On Mon, 2021-11-22 at 17:05 +0530, Vinod Koul wrote:
> On 07-11-21, 15:56, Chunfeng Yun wrote:
> > Due to some SoCs have a bit shift issue that will drop a bit for
> > usb3
> > phy or pcie phy, fix it by adding software efuse reading and
> > setting,
> > but only support it optionally for versoin 2/3.
> 
> s/versoin/version
Ok
> 
> > 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> >  drivers/phy/mediatek/phy-mtk-tphy.c | 162
> > ++++++++++++++++++++++++++++
> >  1 file changed, 162 insertions(+)
> > 
> > diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c
> > b/drivers/phy/mediatek/phy-mtk-tphy.c
> > index cdcef865fe9e..3b5b1c266595 100644
> > --- a/drivers/phy/mediatek/phy-mtk-tphy.c
> > +++ b/drivers/phy/mediatek/phy-mtk-tphy.c
> > @@ -12,6 +12,7 @@
> >  #include <linux/iopoll.h>
> >  #include <linux/mfd/syscon.h>
> >  #include <linux/module.h>
> > +#include <linux/nvmem-consumer.h>
> >  #include <linux/of_address.h>
> >  #include <linux/of_device.h>
> >  #include <linux/phy/phy.h>
> > @@ -41,6 +42,9 @@
> >  #define SSUSB_SIFSLV_V2_U3PHYD		0x200
> >  #define SSUSB_SIFSLV_V2_U3PHYA		0x400
> >  
> > +#define U3P_MISC_REG1		0x04
> > +#define MR1_EFUSE_AUTO_LOAD_DIS		BIT(6)
> > +
> >  #define U3P_USBPHYACR0		0x000
> >  #define PA0_RG_U2PLL_FORCE_ON		BIT(15)
> >  #define PA0_USB20_PLL_PREDIV		GENMASK(7, 6)
> > @@ -133,6 +137,8 @@
> >  #define P3C_RG_SWRST_U3_PHYD_FORCE_EN	BIT(24)
> >  
> >  #define U3P_U3_PHYA_REG0	0x000
> > +#define P3A_RG_IEXT_INTR		GENMASK(15, 10)
> > +#define P3A_RG_IEXT_INTR_VAL(x)		((0x3f & (x)) << 10)
> >  #define P3A_RG_CLKDRV_OFF		GENMASK(3, 2)
> >  #define P3A_RG_CLKDRV_OFF_VAL(x)	((0x3 & (x)) << 2)
> >  
> > @@ -187,6 +193,19 @@
> >  #define P3D_RG_FWAKE_TH		GENMASK(21, 16)
> >  #define P3D_RG_FWAKE_TH_VAL(x)	((0x3f & (x)) << 16)
> >  
> > +#define U3P_U3_PHYD_IMPCAL0		0x010
> > +#define P3D_RG_FORCE_TX_IMPEL		BIT(31)
> > +#define P3D_RG_TX_IMPEL			GENMASK(28, 24)
> > +#define P3D_RG_TX_IMPEL_VAL(x)		((0x1f & (x)) << 24)
> > +
> > +#define U3P_U3_PHYD_IMPCAL1		0x014
> > +#define P3D_RG_FORCE_RX_IMPEL		BIT(31)
> > +#define P3D_RG_RX_IMPEL			GENMASK(28, 24)
> > +#define P3D_RG_RX_IMPEL_VAL(x)		((0x1f & (x)) << 24)
> > +
> > +#define U3P_U3_PHYD_RSV			0x054
> > +#define P3D_RG_EFUSE_AUTO_LOAD_DIS	BIT(12)
> > +
> >  #define U3P_U3_PHYD_CDR1		0x05c
> >  #define P3D_RG_CDR_BIR_LTD1		GENMASK(28, 24)
> >  #define P3D_RG_CDR_BIR_LTD1_VAL(x)	((0x1f & (x)) << 24)
> > @@ -307,6 +326,11 @@ struct mtk_phy_pdata {
> >  	 * 48M PLL, fix it by switching PLL to 26M from default 48M
> >  	 */
> >  	bool sw_pll_48m_to_26m;
> > +	/*
> > +	 * Some SoCs (e.g. mt8195) drop a bit when use auto load efuse,
> > +	 * support sw way, also support it for v2/v3 optionally.
> > +	 */
> > +	bool sw_efuse_supported;
> >  	enum mtk_phy_version version;
> >  };
> >  
> > @@ -336,6 +360,10 @@ struct mtk_phy_instance {
> >  	struct regmap *type_sw;
> >  	u32 type_sw_reg;
> >  	u32 type_sw_index;
> > +	u32 efuse_sw_en;
> > +	u32 efuse_intr;
> > +	u32 efuse_tx_imp;
> > +	u32 efuse_rx_imp;
> >  	int eye_src;
> >  	int eye_vrt;
> >  	int eye_term;
> > @@ -1040,6 +1068,130 @@ static int phy_type_set(struct
> > mtk_phy_instance *instance)
> >  	return 0;
> >  }
> >  
> > +static int phy_efuse_get(struct mtk_tphy *tphy, struct
> > mtk_phy_instance *instance)
> > +{
> > +	struct device *dev = &instance->phy->dev;
> > +	int ret = 0;
> > +
> > +	/* tphy v1 doesn't support sw efuse, skip it */
> > +	if (!tphy->pdata->sw_efuse_supported) {
> > +		instance->efuse_sw_en = 0;
> > +		return 0;
> > +	}
> > +
> > +	/* software efuse is optional */
> > +	instance->efuse_sw_en = device_property_read_bool(dev, "nvmem-
> > cells");
> > +	if (!instance->efuse_sw_en)
> > +		return 0;
> > +
> > +	switch (instance->type) {
> > +	case PHY_TYPE_USB2:
> > +		ret = nvmem_cell_read_variable_le_u32(dev, "intr",
> > &instance->efuse_intr);
> > +		if (ret) {
> > +			dev_err(dev, "fail to get u2 intr efuse, %d\n",
> > ret);
> > +			break;
> > +		}
> > +
> > +		/* no efuse, ignore it */
> > +		if (!instance->efuse_intr) {
> > +			dev_warn(dev, "no u2 intr efuse, but dts enable
> > it\n");
> > +			instance->efuse_sw_en = 0;
> > +			break;
> > +		}
> 
> What does this check do...? so a zero value is not valid..?
Yes, because it's also zero for SoC without efuse, prefer to use
default value instead.

> 
> > +
> > +		dev_info(dev, "u2 efuse - intr %x\n", instance-
> > >efuse_intr);
> 
> dev_dbg()?
We usually use HW auto load way, print this log can help to point out
that it's specific SoC.

> 
> > +		break;
> 
> empty line after break improves readability, pls add
Ok
> 
> > +	case PHY_TYPE_USB3:
> > +	case PHY_TYPE_PCIE:
> > +		ret = nvmem_cell_read_variable_le_u32(dev, "intr",
> > &instance->efuse_intr);
> > +		if (ret) {
> > +			dev_err(dev, "fail to get u3 intr efuse, %d\n",
> > ret);
> > +			break;
> > +		}
> 
> This seems to be common, why not read this before switch?
It's not used for SGMII or SATA, but not applied until now

> 
> > +
> > +		ret = nvmem_cell_read_variable_le_u32(dev, "rx_imp",
> > &instance->efuse_rx_imp);
> > +		if (ret) {
> > +			dev_err(dev, "fail to get u3 rx_imp efuse,
> > %d\n", ret);
> > +			break;
> > +		}
> > +
> > +		ret = nvmem_cell_read_variable_le_u32(dev, "tx_imp",
> > &instance->efuse_tx_imp);
> > +		if (ret) {
> > +			dev_err(dev, "fail to get u3 tx_imp efuse,
> > %d\n", ret);
> > +			break;
> > +		}
> > +
> > +		/* no efuse, ignore it */
> > +		if (!instance->efuse_intr &&
> > +		    !instance->efuse_rx_imp &&
> > +		    !instance->efuse_rx_imp) {
> > +			dev_warn(dev, "no u3 intr efuse, but dts enable
> > it\n");
> > +			instance->efuse_sw_en = 0;
> > +			break;
> > +		}
> 
> again, zero values are not valid?
It's also zero for SoC without efuse, treat it as invalid value

> 
> > +
> > +		dev_info(dev, "u3 efuse - intr %x, rx_imp %x, tx_imp
> > %x\n",
> > +			 instance->efuse_intr, instance->efuse_rx_imp,
> > +			 instance->efuse_tx_imp);
> 
> dbg pls
Ok
> 
> > +		break;
> > +	default:
> > +		dev_err(dev, "no sw efuse for type %d\n", instance-
> > >type);
> > +		ret = -EINVAL;
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static void phy_efuse_set(struct mtk_phy_instance *instance)
> > +{
> > +	struct device *dev = &instance->phy->dev;
> > +	struct u2phy_banks *u2_banks = &instance->u2_banks;
> > +	struct u3phy_banks *u3_banks = &instance->u3_banks;
> > +	u32 tmp;
> > +
> > +	if (!instance->efuse_sw_en)
> > +		return;
> > +
> > +	switch (instance->type) {
> > +	case PHY_TYPE_USB2:
> > +		tmp = readl(u2_banks->misc + U3P_MISC_REG1);
> > +		tmp |= MR1_EFUSE_AUTO_LOAD_DIS;
> > +		writel(tmp, u2_banks->misc + U3P_MISC_REG1);
> > +
> > +		tmp = readl(u2_banks->com + U3P_USBPHYACR1);
> > +		tmp &= ~PA1_RG_INTR_CAL;
> > +		tmp |= PA1_RG_INTR_CAL_VAL(instance->efuse_intr);
> > +		writel(tmp, u2_banks->com + U3P_USBPHYACR1);
> > +		break;
> > +	case PHY_TYPE_USB3:
> > +	case PHY_TYPE_PCIE:
> > +		tmp = readl(u3_banks->phyd + U3P_U3_PHYD_RSV);
> > +		tmp |= P3D_RG_EFUSE_AUTO_LOAD_DIS;
> > +		writel(tmp, u3_banks->phyd + U3P_U3_PHYD_RSV);
> 
> add a updatel() macro and use this here and other places?
You mean add macro to set/clear bits?
If so, I need prepare another patch for this purpose

Thanks a lot

> 

Re: [PATCH 4/6] dt-bindings: phy: mediatek: tphy: support software efuse load

[Chunfeng Yun]

On Mon, 2021-11-22 at 09:21 +0000, Srinivas Kandagatla wrote:
> 
> On 22/11/2021 04:05, Chen-Yu Tsai wrote:
> > On Sat, Nov 20, 2021 at 1:19 AM Srinivas Kandagatla
> > <srinivas.kandagatla@linaro.org> wrote:
> > > On 07/11/2021 07:56, Chunfeng Yun wrote:
> > > > Add optional property nvmem-cells and nvmem-cell-names to
> > > > support
> > > > software efuse load, this helps to fix the efuse bit shift
> > > > issue
> > > > on mt8195 etc.
> > > > 
> > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > 
> > > Applied thanks,
> > 
> > This is a PHY DT binding change. Shouldn't it go in with patch 5,
> > the
> > phy driver patch, through the PHY tree instead?
> 
> That's true, this is dropped from nvmem tree now!

Thanks

> 
> --srini
> > 
> > ChenYu
> > 
> > > --srini
> > > 
> > > > ---
> > > >    .../devicetree/bindings/phy/mediatek,tphy.yaml | 18
> > > > ++++++++++++++++++
> > > >    1 file changed, 18 insertions(+)
> > > > 
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
> > > > b/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
> > > > index 9e6c0f43f1c6..05ee274b4b71 100644
> > > > --- a/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
> > > > +++ b/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
> > > > @@ -160,6 +160,24 @@ patternProperties:
> > > >                    - PHY_TYPE_PCIE
> > > >                    - PHY_TYPE_SATA
> > > > 
> > > > +      nvmem-cells:
> > > > +        items:
> > > > +          - description: internal R efuse for U2 PHY or
> > > > U3/PCIe PHY
> > > > +          - description: rx_imp_sel efuse for U3/PCIe PHY
> > > > +          - description: tx_imp_sel efuse for U3/PCIe PHY
> > > > +        description: |
> > > > +          Phandles to nvmem cell that contains the efuse data;
> > > > +          Available only for U2 PHY or U3/PCIe PHY of version
> > > > 2/3, these
> > > > +          three items should be provided at the same time for
> > > > U3/PCIe PHY,
> > > > +          when use software to load efuse;
> > > > +          If unspecified, will use hardware auto-load efuse.
> > > > +
> > > > +      nvmem-cell-names:
> > > > +        items:
> > > > +          - const: intr
> > > > +          - const: rx_imp
> > > > +          - const: tx_imp
> > > > +
> > > >          # The following optional vendor properties are only
> > > > for debug or HQA test
> > > >          mediatek,eye-src:
> > > >            description:
> > > > 
> > > 
> > > _______________________________________________
> > > Linux-mediatek mailing list
> > > Linux-mediatek@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 5/6] phy: phy-mtk-tphy: add support efuse setting

[Vinod Koul]

On 24-11-21, 14:54, Chunfeng Yun wrote:
> On Mon, 2021-11-22 at 17:05 +0530, Vinod Koul wrote:
> > On 07-11-21, 15:56, Chunfeng Yun wrote:
> > > Due to some SoCs have a bit shift issue that will drop a bit for
> > > usb3
> > > phy or pcie phy, fix it by adding software efuse reading and
> > > setting,
> > > but only support it optionally for versoin 2/3.
> > 
> > s/versoin/version
> Ok
> > 
> > > 
> > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > ---
> > >  drivers/phy/mediatek/phy-mtk-tphy.c | 162
> > > ++++++++++++++++++++++++++++
> > >  1 file changed, 162 insertions(+)
> > > 
> > > diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c
> > > b/drivers/phy/mediatek/phy-mtk-tphy.c
> > > index cdcef865fe9e..3b5b1c266595 100644
> > > --- a/drivers/phy/mediatek/phy-mtk-tphy.c
> > > +++ b/drivers/phy/mediatek/phy-mtk-tphy.c
> > > @@ -12,6 +12,7 @@
> > >  #include <linux/iopoll.h>
> > >  #include <linux/mfd/syscon.h>
> > >  #include <linux/module.h>
> > > +#include <linux/nvmem-consumer.h>
> > >  #include <linux/of_address.h>
> > >  #include <linux/of_device.h>
> > >  #include <linux/phy/phy.h>
> > > @@ -41,6 +42,9 @@
> > >  #define SSUSB_SIFSLV_V2_U3PHYD		0x200
> > >  #define SSUSB_SIFSLV_V2_U3PHYA		0x400
> > >  
> > > +#define U3P_MISC_REG1		0x04
> > > +#define MR1_EFUSE_AUTO_LOAD_DIS		BIT(6)
> > > +
> > >  #define U3P_USBPHYACR0		0x000
> > >  #define PA0_RG_U2PLL_FORCE_ON		BIT(15)
> > >  #define PA0_USB20_PLL_PREDIV		GENMASK(7, 6)
> > > @@ -133,6 +137,8 @@
> > >  #define P3C_RG_SWRST_U3_PHYD_FORCE_EN	BIT(24)
> > >  
> > >  #define U3P_U3_PHYA_REG0	0x000
> > > +#define P3A_RG_IEXT_INTR		GENMASK(15, 10)
> > > +#define P3A_RG_IEXT_INTR_VAL(x)		((0x3f & (x)) << 10)
> > >  #define P3A_RG_CLKDRV_OFF		GENMASK(3, 2)
> > >  #define P3A_RG_CLKDRV_OFF_VAL(x)	((0x3 & (x)) << 2)
> > >  
> > > @@ -187,6 +193,19 @@
> > >  #define P3D_RG_FWAKE_TH		GENMASK(21, 16)
> > >  #define P3D_RG_FWAKE_TH_VAL(x)	((0x3f & (x)) << 16)
> > >  
> > > +#define U3P_U3_PHYD_IMPCAL0		0x010
> > > +#define P3D_RG_FORCE_TX_IMPEL		BIT(31)
> > > +#define P3D_RG_TX_IMPEL			GENMASK(28, 24)
> > > +#define P3D_RG_TX_IMPEL_VAL(x)		((0x1f & (x)) << 24)
> > > +
> > > +#define U3P_U3_PHYD_IMPCAL1		0x014
> > > +#define P3D_RG_FORCE_RX_IMPEL		BIT(31)
> > > +#define P3D_RG_RX_IMPEL			GENMASK(28, 24)
> > > +#define P3D_RG_RX_IMPEL_VAL(x)		((0x1f & (x)) << 24)
> > > +
> > > +#define U3P_U3_PHYD_RSV			0x054
> > > +#define P3D_RG_EFUSE_AUTO_LOAD_DIS	BIT(12)
> > > +
> > >  #define U3P_U3_PHYD_CDR1		0x05c
> > >  #define P3D_RG_CDR_BIR_LTD1		GENMASK(28, 24)
> > >  #define P3D_RG_CDR_BIR_LTD1_VAL(x)	((0x1f & (x)) << 24)
> > > @@ -307,6 +326,11 @@ struct mtk_phy_pdata {
> > >  	 * 48M PLL, fix it by switching PLL to 26M from default 48M
> > >  	 */
> > >  	bool sw_pll_48m_to_26m;
> > > +	/*
> > > +	 * Some SoCs (e.g. mt8195) drop a bit when use auto load efuse,
> > > +	 * support sw way, also support it for v2/v3 optionally.
> > > +	 */
> > > +	bool sw_efuse_supported;
> > >  	enum mtk_phy_version version;
> > >  };
> > >  
> > > @@ -336,6 +360,10 @@ struct mtk_phy_instance {
> > >  	struct regmap *type_sw;
> > >  	u32 type_sw_reg;
> > >  	u32 type_sw_index;
> > > +	u32 efuse_sw_en;
> > > +	u32 efuse_intr;
> > > +	u32 efuse_tx_imp;
> > > +	u32 efuse_rx_imp;
> > >  	int eye_src;
> > >  	int eye_vrt;
> > >  	int eye_term;
> > > @@ -1040,6 +1068,130 @@ static int phy_type_set(struct
> > > mtk_phy_instance *instance)
> > >  	return 0;
> > >  }
> > >  
> > > +static int phy_efuse_get(struct mtk_tphy *tphy, struct
> > > mtk_phy_instance *instance)
> > > +{
> > > +	struct device *dev = &instance->phy->dev;
> > > +	int ret = 0;
> > > +
> > > +	/* tphy v1 doesn't support sw efuse, skip it */
> > > +	if (!tphy->pdata->sw_efuse_supported) {
> > > +		instance->efuse_sw_en = 0;
> > > +		return 0;
> > > +	}
> > > +
> > > +	/* software efuse is optional */
> > > +	instance->efuse_sw_en = device_property_read_bool(dev, "nvmem-
> > > cells");
> > > +	if (!instance->efuse_sw_en)
> > > +		return 0;
> > > +
> > > +	switch (instance->type) {
> > > +	case PHY_TYPE_USB2:
> > > +		ret = nvmem_cell_read_variable_le_u32(dev, "intr",
> > > &instance->efuse_intr);
> > > +		if (ret) {
> > > +			dev_err(dev, "fail to get u2 intr efuse, %d\n",
> > > ret);
> > > +			break;
> > > +		}
> > > +
> > > +		/* no efuse, ignore it */
> > > +		if (!instance->efuse_intr) {
> > > +			dev_warn(dev, "no u2 intr efuse, but dts enable
> > > it\n");
> > > +			instance->efuse_sw_en = 0;
> > > +			break;
> > > +		}
> > 
> > What does this check do...? so a zero value is not valid..?
> Yes, because it's also zero for SoC without efuse, prefer to use
> default value instead.
> 
> > 
> > > +
> > > +		dev_info(dev, "u2 efuse - intr %x\n", instance-
> > > >efuse_intr);
> > 
> > dev_dbg()?
> We usually use HW auto load way, print this log can help to point out
> that it's specific SoC.

You can enable this print thru dynamic debug or enable debug prints when
debugging, let us not spam message log

> 
> > 
> > > +		break;
> > 
> > empty line after break improves readability, pls add
> Ok
> > 
> > > +	case PHY_TYPE_USB3:
> > > +	case PHY_TYPE_PCIE:
> > > +		ret = nvmem_cell_read_variable_le_u32(dev, "intr",
> > > &instance->efuse_intr);
> > > +		if (ret) {
> > > +			dev_err(dev, "fail to get u3 intr efuse, %d\n",
> > > ret);
> > > +			break;
> > > +		}
> > 
> > This seems to be common, why not read this before switch?
> It's not used for SGMII or SATA, but not applied until now
> 
> > 
> > > +
> > > +		ret = nvmem_cell_read_variable_le_u32(dev, "rx_imp",
> > > &instance->efuse_rx_imp);
> > > +		if (ret) {
> > > +			dev_err(dev, "fail to get u3 rx_imp efuse,
> > > %d\n", ret);
> > > +			break;
> > > +		}
> > > +
> > > +		ret = nvmem_cell_read_variable_le_u32(dev, "tx_imp",
> > > &instance->efuse_tx_imp);
> > > +		if (ret) {
> > > +			dev_err(dev, "fail to get u3 tx_imp efuse,
> > > %d\n", ret);
> > > +			break;
> > > +		}
> > > +
> > > +		/* no efuse, ignore it */
> > > +		if (!instance->efuse_intr &&
> > > +		    !instance->efuse_rx_imp &&
> > > +		    !instance->efuse_rx_imp) {
> > > +			dev_warn(dev, "no u3 intr efuse, but dts enable
> > > it\n");
> > > +			instance->efuse_sw_en = 0;
> > > +			break;
> > > +		}
> > 
> > again, zero values are not valid?
> It's also zero for SoC without efuse, treat it as invalid value
> 
> > 
> > > +
> > > +		dev_info(dev, "u3 efuse - intr %x, rx_imp %x, tx_imp
> > > %x\n",
> > > +			 instance->efuse_intr, instance->efuse_rx_imp,
> > > +			 instance->efuse_tx_imp);
> > 
> > dbg pls
> Ok
> > 
> > > +		break;
> > > +	default:
> > > +		dev_err(dev, "no sw efuse for type %d\n", instance-
> > > >type);
> > > +		ret = -EINVAL;
> > > +	}
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > +static void phy_efuse_set(struct mtk_phy_instance *instance)
> > > +{
> > > +	struct device *dev = &instance->phy->dev;
> > > +	struct u2phy_banks *u2_banks = &instance->u2_banks;
> > > +	struct u3phy_banks *u3_banks = &instance->u3_banks;
> > > +	u32 tmp;
> > > +
> > > +	if (!instance->efuse_sw_en)
> > > +		return;
> > > +
> > > +	switch (instance->type) {
> > > +	case PHY_TYPE_USB2:
> > > +		tmp = readl(u2_banks->misc + U3P_MISC_REG1);
> > > +		tmp |= MR1_EFUSE_AUTO_LOAD_DIS;
> > > +		writel(tmp, u2_banks->misc + U3P_MISC_REG1);
> > > +
> > > +		tmp = readl(u2_banks->com + U3P_USBPHYACR1);
> > > +		tmp &= ~PA1_RG_INTR_CAL;
> > > +		tmp |= PA1_RG_INTR_CAL_VAL(instance->efuse_intr);
> > > +		writel(tmp, u2_banks->com + U3P_USBPHYACR1);
> > > +		break;
> > > +	case PHY_TYPE_USB3:
> > > +	case PHY_TYPE_PCIE:
> > > +		tmp = readl(u3_banks->phyd + U3P_U3_PHYD_RSV);
> > > +		tmp |= P3D_RG_EFUSE_AUTO_LOAD_DIS;
> > > +		writel(tmp, u3_banks->phyd + U3P_U3_PHYD_RSV);
> > 
> > add a updatel() macro and use this here and other places?
> You mean add macro to set/clear bits?
> If so, I need prepare another patch for this purpose

Sure, that would be nice

-- 
~Vinod

Re: [PATCH 5/6] phy: phy-mtk-tphy: add support efuse setting

[Chunfeng Yun]

On Thu, 2021-11-25 at 10:42 +0530, Vinod Koul wrote:
> On 24-11-21, 14:54, Chunfeng Yun wrote:
> > On Mon, 2021-11-22 at 17:05 +0530, Vinod Koul wrote:
> > > On 07-11-21, 15:56, Chunfeng Yun wrote:
> > > > Due to some SoCs have a bit shift issue that will drop a bit
> > > > for
> > > > usb3
> > > > phy or pcie phy, fix it by adding software efuse reading and
> > > > setting,
> > > > but only support it optionally for versoin 2/3.
> > > 
> > > s/versoin/version
> > 
> > Ok
> > > 
> > > > 
> > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > > ---
> > > >  drivers/phy/mediatek/phy-mtk-tphy.c | 162
> > > > ++++++++++++++++++++++++++++
> > > >  1 file changed, 162 insertions(+)
> > > > 
> > > > diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c
> > > > b/drivers/phy/mediatek/phy-mtk-tphy.c
> > > > index cdcef865fe9e..3b5b1c266595 100644
> > > > --- a/drivers/phy/mediatek/phy-mtk-tphy.c
> > > > +++ b/drivers/phy/mediatek/phy-mtk-tphy.c
> > > > @@ -12,6 +12,7 @@
> > > >  #include <linux/iopoll.h>
> > > >  #include <linux/mfd/syscon.h>
> > > >  #include <linux/module.h>
> > > > +#include <linux/nvmem-consumer.h>
> > > >  #include <linux/of_address.h>
> > > >  #include <linux/of_device.h>
> > > >  #include <linux/phy/phy.h>
> > > > @@ -41,6 +42,9 @@
> > > >  #define SSUSB_SIFSLV_V2_U3PHYD		0x200
> > > >  #define SSUSB_SIFSLV_V2_U3PHYA		0x400
> > > >  
> > > > +#define U3P_MISC_REG1		0x04
> > > > +#define MR1_EFUSE_AUTO_LOAD_DIS		BIT(6)
> > > > +
> > > >  #define U3P_USBPHYACR0		0x000
> > > >  #define PA0_RG_U2PLL_FORCE_ON		BIT(15)
> > > >  #define PA0_USB20_PLL_PREDIV		GENMASK(7, 6)
> > > > @@ -133,6 +137,8 @@
> > > >  #define P3C_RG_SWRST_U3_PHYD_FORCE_EN	BIT(24)
> > > >  
> > > >  #define U3P_U3_PHYA_REG0	0x000
> > > > +#define P3A_RG_IEXT_INTR		GENMASK(15, 10)
> > > > +#define P3A_RG_IEXT_INTR_VAL(x)		((0x3f & (x))
> > > > << 10)
> > > >  #define P3A_RG_CLKDRV_OFF		GENMASK(3, 2)
> > > >  #define P3A_RG_CLKDRV_OFF_VAL(x)	((0x3 & (x)) << 2)
> > > >  
> > > > @@ -187,6 +193,19 @@
> > > >  #define P3D_RG_FWAKE_TH		GENMASK(21, 16)
> > > >  #define P3D_RG_FWAKE_TH_VAL(x)	((0x3f & (x)) << 16)
> > > >  
> > > > +#define U3P_U3_PHYD_IMPCAL0		0x010
> > > > +#define P3D_RG_FORCE_TX_IMPEL		BIT(31)
> > > > +#define P3D_RG_TX_IMPEL			GENMASK(28, 24)
> > > > +#define P3D_RG_TX_IMPEL_VAL(x)		((0x1f & (x)) << 24)
> > > > +
> > > > +#define U3P_U3_PHYD_IMPCAL1		0x014
> > > > +#define P3D_RG_FORCE_RX_IMPEL		BIT(31)
> > > > +#define P3D_RG_RX_IMPEL			GENMASK(28, 24)
> > > > +#define P3D_RG_RX_IMPEL_VAL(x)		((0x1f & (x)) << 24)
> > > > +
> > > > +#define U3P_U3_PHYD_RSV			0x054
> > > > +#define P3D_RG_EFUSE_AUTO_LOAD_DIS	BIT(12)
> > > > +
> > > >  #define U3P_U3_PHYD_CDR1		0x05c
> > > >  #define P3D_RG_CDR_BIR_LTD1		GENMASK(28, 24)
> > > >  #define P3D_RG_CDR_BIR_LTD1_VAL(x)	((0x1f & (x)) << 24)
> > > > @@ -307,6 +326,11 @@ struct mtk_phy_pdata {
> > > >  	 * 48M PLL, fix it by switching PLL to 26M from default
> > > > 48M
> > > >  	 */
> > > >  	bool sw_pll_48m_to_26m;
> > > > +	/*
> > > > +	 * Some SoCs (e.g. mt8195) drop a bit when use auto
> > > > load efuse,
> > > > +	 * support sw way, also support it for v2/v3
> > > > optionally.
> > > > +	 */
> > > > +	bool sw_efuse_supported;
> > > >  	enum mtk_phy_version version;
> > > >  };
> > > >  
> > > > @@ -336,6 +360,10 @@ struct mtk_phy_instance {
> > > >  	struct regmap *type_sw;
> > > >  	u32 type_sw_reg;
> > > >  	u32 type_sw_index;
> > > > +	u32 efuse_sw_en;
> > > > +	u32 efuse_intr;
> > > > +	u32 efuse_tx_imp;
> > > > +	u32 efuse_rx_imp;
> > > >  	int eye_src;
> > > >  	int eye_vrt;
> > > >  	int eye_term;
> > > > @@ -1040,6 +1068,130 @@ static int phy_type_set(struct
> > > > mtk_phy_instance *instance)
> > > >  	return 0;
> > > >  }
> > > >  
> > > > +static int phy_efuse_get(struct mtk_tphy *tphy, struct
> > > > mtk_phy_instance *instance)
> > > > +{
> > > > +	struct device *dev = &instance->phy->dev;
> > > > +	int ret = 0;
> > > > +
> > > > +	/* tphy v1 doesn't support sw efuse, skip it */
> > > > +	if (!tphy->pdata->sw_efuse_supported) {
> > > > +		instance->efuse_sw_en = 0;
> > > > +		return 0;
> > > > +	}
> > > > +
> > > > +	/* software efuse is optional */
> > > > +	instance->efuse_sw_en = device_property_read_bool(dev,
> > > > "nvmem-
> > > > cells");
> > > > +	if (!instance->efuse_sw_en)
> > > > +		return 0;
> > > > +
> > > > +	switch (instance->type) {
> > > > +	case PHY_TYPE_USB2:
> > > > +		ret = nvmem_cell_read_variable_le_u32(dev,
> > > > "intr",
> > > > &instance->efuse_intr);
> > > > +		if (ret) {
> > > > +			dev_err(dev, "fail to get u2 intr
> > > > efuse, %d\n",
> > > > ret);
> > > > +			break;
> > > > +		}
> > > > +
> > > > +		/* no efuse, ignore it */
> > > > +		if (!instance->efuse_intr) {
> > > > +			dev_warn(dev, "no u2 intr efuse, but
> > > > dts enable
> > > > it\n");
> > > > +			instance->efuse_sw_en = 0;
> > > > +			break;
> > > > +		}
> > > 
> > > What does this check do...? so a zero value is not valid..?
> > 
> > Yes, because it's also zero for SoC without efuse, prefer to use
> > default value instead.
> > 
> > > 
> > > > +
> > > > +		dev_info(dev, "u2 efuse - intr %x\n", instance-
> > > > > efuse_intr);
> > > 
> > > dev_dbg()?
> > 
> > We usually use HW auto load way, print this log can help to point
> > out
> > that it's specific SoC.
> 
> You can enable this print thru dynamic debug or enable debug prints
> when
> debugging, let us not spam message log
Ok, will use dev_dbg()
> 
> > 
> > > 
> > > > +		break;
> > > 
> > > empty line after break improves readability, pls add
> > 
> > Ok
> > > 
> > > > +	case PHY_TYPE_USB3:
> > > > +	case PHY_TYPE_PCIE:
> > > > +		ret = nvmem_cell_read_variable_le_u32(dev,
> > > > "intr",
> > > > &instance->efuse_intr);
> > > > +		if (ret) {
> > > > +			dev_err(dev, "fail to get u3 intr
> > > > efuse, %d\n",
> > > > ret);
> > > > +			break;
> > > > +		}
> > > 
> > > This seems to be common, why not read this before switch?
> > 
> > It's not used for SGMII or SATA, but not applied until now
> > 
> > > 
> > > > +
> > > > +		ret = nvmem_cell_read_variable_le_u32(dev,
> > > > "rx_imp",
> > > > &instance->efuse_rx_imp);
> > > > +		if (ret) {
> > > > +			dev_err(dev, "fail to get u3 rx_imp
> > > > efuse,
> > > > %d\n", ret);
> > > > +			break;
> > > > +		}
> > > > +
> > > > +		ret = nvmem_cell_read_variable_le_u32(dev,
> > > > "tx_imp",
> > > > &instance->efuse_tx_imp);
> > > > +		if (ret) {
> > > > +			dev_err(dev, "fail to get u3 tx_imp
> > > > efuse,
> > > > %d\n", ret);
> > > > +			break;
> > > > +		}
> > > > +
> > > > +		/* no efuse, ignore it */
> > > > +		if (!instance->efuse_intr &&
> > > > +		    !instance->efuse_rx_imp &&
> > > > +		    !instance->efuse_rx_imp) {
> > > > +			dev_warn(dev, "no u3 intr efuse, but
> > > > dts enable
> > > > it\n");
> > > > +			instance->efuse_sw_en = 0;
> > > > +			break;
> > > > +		}
> > > 
> > > again, zero values are not valid?
> > 
> > It's also zero for SoC without efuse, treat it as invalid value
> > 
> > > 
> > > > +
> > > > +		dev_info(dev, "u3 efuse - intr %x, rx_imp %x,
> > > > tx_imp
> > > > %x\n",
> > > > +			 instance->efuse_intr, instance-
> > > > >efuse_rx_imp,
> > > > +			 instance->efuse_tx_imp);
> > > 
> > > dbg pls
> > 
> > Ok
> > > 
> > > > +		break;
> > > > +	default:
> > > > +		dev_err(dev, "no sw efuse for type %d\n",
> > > > instance-
> > > > > type);
> > > > 
> > > > +		ret = -EINVAL;
> > > > +	}
> > > > +
> > > > +	return ret;
> > > > +}
> > > > +
> > > > +static void phy_efuse_set(struct mtk_phy_instance *instance)
> > > > +{
> > > > +	struct device *dev = &instance->phy->dev;
> > > > +	struct u2phy_banks *u2_banks = &instance->u2_banks;
> > > > +	struct u3phy_banks *u3_banks = &instance->u3_banks;
> > > > +	u32 tmp;
> > > > +
> > > > +	if (!instance->efuse_sw_en)
> > > > +		return;
> > > > +
> > > > +	switch (instance->type) {
> > > > +	case PHY_TYPE_USB2:
> > > > +		tmp = readl(u2_banks->misc + U3P_MISC_REG1);
> > > > +		tmp |= MR1_EFUSE_AUTO_LOAD_DIS;
> > > > +		writel(tmp, u2_banks->misc + U3P_MISC_REG1);
> > > > +
> > > > +		tmp = readl(u2_banks->com + U3P_USBPHYACR1);
> > > > +		tmp &= ~PA1_RG_INTR_CAL;
> > > > +		tmp |= PA1_RG_INTR_CAL_VAL(instance-
> > > > >efuse_intr);
> > > > +		writel(tmp, u2_banks->com + U3P_USBPHYACR1);
> > > > +		break;
> > > > +	case PHY_TYPE_USB3:
> > > > +	case PHY_TYPE_PCIE:
> > > > +		tmp = readl(u3_banks->phyd + U3P_U3_PHYD_RSV);
> > > > +		tmp |= P3D_RG_EFUSE_AUTO_LOAD_DIS;
> > > > +		writel(tmp, u3_banks->phyd + U3P_U3_PHYD_RSV);
> > > 
> > > add a updatel() macro and use this here and other places?
> > 
> > You mean add macro to set/clear bits?
> > If so, I need prepare another patch for this purpose
> 
> Sure, that would be nice
Ok
>