Devicetree
 help / color / mirror / Atom feed
* [PATCH v5 9/9] dt-bindings: phy-mt65xx-usb: add support for new version phy
From: Chunfeng Yun @ 2017-03-31  7:35 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Matthias Brugger, Felipe Balbi, Rob Herring, Mark Rutland,
	Ian Campbell, Chunfeng Yun, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1490945735-9531-1-git-send-email-chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

add a new compatible string for "mt2712", and move reference clock
into each port node;

Signed-off-by: Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 .../devicetree/bindings/phy/phy-mt65xx-usb.txt     |   93 +++++++++++++++++---
 1 file changed, 80 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt b/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt
index 33a2b1e..0acc5a9 100644
--- a/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt
+++ b/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt
@@ -6,12 +6,11 @@ This binding describes a usb3.0 phy for mt65xx platforms of Medaitek SoC.
 Required properties (controller (parent) node):
  - compatible	: should be one of
 		  "mediatek,mt2701-u3phy"
+		  "mediatek,mt2712-u3phy"
 		  "mediatek,mt8173-u3phy"
- - reg		: offset and length of register for phy, exclude port's
-		  register.
- - clocks	: a list of phandle + clock-specifier pairs, one for each
-		  entry in clock-names
- - clock-names	: must contain
+ - clocks	: (deprecated, use port's clocks instead) a list of phandle +
+		  clock-specifier pairs, one for each entry in clock-names
+ - clock-names	: (deprecated, use port's one instead) must contain
 		  "u3phya_ref": for reference clock of usb3.0 analog phy.
 
 Required nodes	: a sub-node is required for each port the controller
@@ -19,8 +18,19 @@ Required nodes	: a sub-node is required for each port the controller
 		  'reg' property is used inside these nodes to describe
 		  the controller's topology.
 
+Optional properties (controller (parent) node):
+ - reg		: offset and length of register shared by multiple ports,
+		  exclude port's private register. It is needed on mt2701
+		  and mt8173, but not on mt2712.
+
 Required properties (port (child) node):
 - reg		: address and length of the register set for the port.
+- clocks	: a list of phandle + clock-specifier pairs, one for each
+		  entry in clock-names
+- clock-names	: must contain
+		  "ref": 48M reference clock for HighSpeed analog phy; and 26M
+			reference clock for SuperSpeed analog phy, sometimes is
+			24M, 25M or 27M, depended on platform.
 - #phy-cells	: should be 1 (See second example)
 		  cell after port phandle is phy type from:
 			- PHY_TYPE_USB2
@@ -31,21 +41,31 @@ Example:
 u3phy: usb-phy@11290000 {
 	compatible = "mediatek,mt8173-u3phy";
 	reg = <0 0x11290000 0 0x800>;
-	clocks = <&apmixedsys CLK_APMIXED_REF2USB_TX>;
-	clock-names = "u3phya_ref";
 	#address-cells = <2>;
 	#size-cells = <2>;
 	ranges;
 	status = "okay";
 
-	phy_port0: port@11290800 {
-		reg = <0 0x11290800 0 0x800>;
+	u2port0: usb-phy@11290800 {
+		reg = <0 0x11290800 0 0x100>;
+		clocks = <&apmixedsys CLK_APMIXED_REF2USB_TX>;
+		clock-names = "ref";
 		#phy-cells = <1>;
 		status = "okay";
 	};
 
-	phy_port1: port@11291000 {
-		reg = <0 0x11291000 0 0x800>;
+	u3port0: usb-phy@11290900 {
+		reg = <0 0x11290800 0 0x700>;
+		clocks = <&clk26m>;
+		clock-names = "ref";
+		#phy-cells = <1>;
+		status = "okay";
+	};
+
+	u2port1: usb-phy@11291000 {
+		reg = <0 0x11291000 0 0x100>;
+		clocks = <&apmixedsys CLK_APMIXED_REF2USB_TX>;
+		clock-names = "ref";
 		#phy-cells = <1>;
 		status = "okay";
 	};
@@ -64,7 +84,54 @@ Example:
 
 usb30: usb@11270000 {
 	...
-	phys = <&phy_port0 PHY_TYPE_USB3>;
-	phy-names = "usb3-0";
+	phys = <&u2port0 PHY_TYPE_USB2>, <&u3port0 PHY_TYPE_USB3>;
+	phy-names = "usb2-0", "usb3-0";
 	...
 };
+
+
+Layout differences of banks between mt8173/mt2701 and mt2712
+-------------------------------------------------------------
+mt8173 and mt2701:
+port        offset    bank
+shared      0x0000    SPLLC
+            0x0100    FMREG
+u2 port0    0x0800    U2PHY_COM
+u3 port0    0x0900    U3PHYD
+            0x0a00    U3PHYD_BANK2
+            0x0b00    U3PHYA
+            0x0c00    U3PHYA_DA
+u2 port1    0x1000    U2PHY_COM
+u3 port1    0x1100    U3PHYD
+            0x1200    U3PHYD_BANK2
+            0x1300    U3PHYA
+            0x1400    U3PHYA_DA
+u2 port2    0x1800    U2PHY_COM
+            ...
+
+mt2712:
+port        offset    bank
+u2 port0    0x0000    MISC
+            0x0100    FMREG
+            0x0300    U2PHY_COM
+u3 port0    0x0700    SPLLC
+            0x0800    CHIP
+            0x0900    U3PHYD
+            0x0a00    U3PHYD_BANK2
+            0x0b00    U3PHYA
+            0x0c00    U3PHYA_DA
+u2 port1    0x1000    MISC
+            0x1100    FMREG
+            0x1300    U2PHY_COM
+u3 port1    0x1700    SPLLC
+            0x1800    CHIP
+            0x1900    U3PHYD
+            0x1a00    U3PHYD_BANK2
+            0x1b00    U3PHYA
+            0x1c00    U3PHYA_DA
+u2 port2    0x2000    MISC
+            ...
+
+    SPLLC shared by u3 ports and FMREG shared by u2 ports on
+mt8173/mt2701 are put back into each port; a new bank MISC for
+u2 ports and CHIP for u3 ports are added on mt2712.
-- 
1.7.9.5

--
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 related

* [PATCH v5 8/9] arm64: dts: mt8173: move clock from phy node into port nodes
From: Chunfeng Yun @ 2017-03-31  7:35 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Matthias Brugger, Felipe Balbi, Rob Herring, Mark Rutland,
	Ian Campbell, Chunfeng Yun, linux-kernel, linux-arm-kernel,
	linux-usb, linux-mediatek, devicetree
In-Reply-To: <1490945735-9531-1-git-send-email-chunfeng.yun@mediatek.com>

there is a reference clock for each port, HighSpeed port is 48M,
and SuperSpeed port is usually 26M. it is flexible to move it
into port node, then unused clock can be disabled.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8173.dtsi |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index 1dc4629..1c9e0d5 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -764,8 +764,6 @@
 		u3phy: usb-phy@11290000 {
 			compatible = "mediatek,mt8173-u3phy";
 			reg = <0 0x11290000 0 0x800>;
-			clocks = <&apmixedsys CLK_APMIXED_REF2USB_TX>;
-			clock-names = "u3phya_ref";
 			#address-cells = <2>;
 			#size-cells = <2>;
 			ranges;
@@ -773,18 +771,24 @@
 
 			u2port0: usb-phy@11290800 {
 				reg = <0 0x11290800 0 0x100>;
+				clocks = <&apmixedsys CLK_APMIXED_REF2USB_TX>;
+				clock-names = "ref";
 				#phy-cells = <1>;
 				status = "okay";
 			};
 
 			u3port0: usb-phy@11290900 {
 				reg = <0 0x11290900 0 0x700>;
+				clocks = <&clk26m>;
+				clock-names = "ref";
 				#phy-cells = <1>;
 				status = "okay";
 			};
 
 			u2port1: usb-phy@11291000 {
 				reg = <0 0x11291000 0 0x100>;
+				clocks = <&apmixedsys CLK_APMIXED_REF2USB_TX>;
+				clock-names = "ref";
 				#phy-cells = <1>;
 				status = "okay";
 			};
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v5 7/9] arm64: dts: mt8173: split usb SuperSpeed port into two ports
From: Chunfeng Yun @ 2017-03-31  7:35 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Matthias Brugger, Felipe Balbi, Rob Herring, Mark Rutland,
	Ian Campbell, Chunfeng Yun, linux-kernel, linux-arm-kernel,
	linux-usb, linux-mediatek, devicetree
In-Reply-To: <1490945735-9531-1-git-send-email-chunfeng.yun@mediatek.com>

split the old SuperSpeed port node into a HighSpeed one and a new
SuperSpeed one.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8173.dtsi |   19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index 6922252..1dc4629 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -731,8 +731,9 @@
 			      <0 0x11280700 0 0x0100>;
 			reg-names = "mac", "ippc";
 			interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_LOW>;
-			phys = <&phy_port0 PHY_TYPE_USB3>,
-			       <&phy_port1 PHY_TYPE_USB2>;
+			phys = <&u2port0 PHY_TYPE_USB2>,
+			       <&u3port0 PHY_TYPE_USB3>,
+			       <&u2port1 PHY_TYPE_USB2>;
 			power-domains = <&scpsys MT8173_POWER_DOMAIN_USB>;
 			clocks = <&topckgen CLK_TOP_USB30_SEL>,
 				 <&clk26m>,
@@ -770,14 +771,20 @@
 			ranges;
 			status = "okay";
 
-			phy_port0: port@11290800 {
-				reg = <0 0x11290800 0 0x800>;
+			u2port0: usb-phy@11290800 {
+				reg = <0 0x11290800 0 0x100>;
 				#phy-cells = <1>;
 				status = "okay";
 			};
 
-			phy_port1: port@11291000 {
-				reg = <0 0x11291000 0 0x800>;
+			u3port0: usb-phy@11290900 {
+				reg = <0 0x11290900 0 0x700>;
+				#phy-cells = <1>;
+				status = "okay";
+			};
+
+			u2port1: usb-phy@11291000 {
+				reg = <0 0x11291000 0 0x100>;
 				#phy-cells = <1>;
 				status = "okay";
 			};
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v5 6/9] phy: phy-mt65xx-usb3: disable 100uA extraction from SS port to HS port
From: Chunfeng Yun @ 2017-03-31  7:35 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Matthias Brugger, Felipe Balbi, Rob Herring, Mark Rutland,
	Ian Campbell, Chunfeng Yun, linux-kernel, linux-arm-kernel,
	linux-usb, linux-mediatek, devicetree
In-Reply-To: <1490945735-9531-1-git-send-email-chunfeng.yun@mediatek.com>

There will be a problem if SS port is diasbled and HS port extracts
100uA from SS port, so disable extract 100uA from SS port in the case,
when disable it, PA0_RG_USB20_INTR_EN should be set, otherwise HS port
only works on LS.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/phy/phy-mt65xx-usb3.c |   24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/phy/phy-mt65xx-usb3.c b/drivers/phy/phy-mt65xx-usb3.c
index 5eea65c..ee83d93 100644
--- a/drivers/phy/phy-mt65xx-usb3.c
+++ b/drivers/phy/phy-mt65xx-usb3.c
@@ -46,6 +46,7 @@
 
 #define U3P_USBPHYACR0		0x000
 #define PA0_RG_U2PLL_FORCE_ON		BIT(15)
+#define PA0_RG_USB20_INTR_EN		BIT(5)
 
 #define U3P_USBPHYACR2		0x008
 #define PA2_RG_SIF_U2PLL_FORCE_EN	BIT(18)
@@ -339,6 +340,15 @@ static void phy_instance_init(struct mt65xx_u3phy *u3phy,
 	tmp &= ~P2C_RG_UART_EN;
 	writel(tmp, com + U3P_U2PHYDTM1);
 
+	tmp = readl(com + U3P_USBPHYACR0);
+	tmp |= PA0_RG_USB20_INTR_EN;
+	writel(tmp, com + U3P_USBPHYACR0);
+
+	/* disable switch 100uA current to SSUSB */
+	tmp = readl(com + U3P_USBPHYACR5);
+	tmp &= ~PA5_RG_U2_HS_100U_U3_EN;
+	writel(tmp, com + U3P_USBPHYACR5);
+
 	if (!index) {
 		tmp = readl(com + U3P_U2PHYACR4);
 		tmp &= ~P2C_U2_GPIO_CTR_MSK;
@@ -393,13 +403,6 @@ static void phy_instance_power_on(struct mt65xx_u3phy *u3phy,
 	tmp |= PA6_RG_U2_OTG_VBUSCMP_EN;
 	writel(tmp, com + U3P_USBPHYACR6);
 
-	if (!index) {
-		/* switch 100uA current to SSUSB */
-		tmp = readl(com + U3P_USBPHYACR5);
-		tmp |= PA5_RG_U2_HS_100U_U3_EN;
-		writel(tmp, com + U3P_USBPHYACR5);
-	}
-
 	tmp = readl(com + U3P_U2PHYDTM1);
 	tmp |= P2C_RG_VBUSVALID | P2C_RG_AVALID;
 	tmp &= ~P2C_RG_SESSEND;
@@ -435,13 +438,6 @@ static void phy_instance_power_off(struct mt65xx_u3phy *u3phy,
 	tmp &= ~PA6_RG_U2_OTG_VBUSCMP_EN;
 	writel(tmp, com + U3P_USBPHYACR6);
 
-	if (!index) {
-		/* switch 100uA current back to USB2.0 */
-		tmp = readl(com + U3P_USBPHYACR5);
-		tmp &= ~PA5_RG_U2_HS_100U_U3_EN;
-		writel(tmp, com + U3P_USBPHYACR5);
-	}
-
 	/* let suspendm=0, set utmi into analog power down */
 	tmp = readl(com + U3P_U2PHYDTM0);
 	tmp &= ~P2C_RG_SUSPENDM;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v5 5/9] phy: phy-mt65xx-usb3: add support for new version phy
From: Chunfeng Yun @ 2017-03-31  7:35 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Felipe Balbi,
	Ian Campbell, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Chunfeng Yun, Rob Herring,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Matthias Brugger,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1490945735-9531-1-git-send-email-chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

There are some variations from mt2701 to mt2712:
1. banks shared by multiple ports are put back into each port,
    such as SPLLC and U2FREQ;
2. add a new bank MISC for u2port, and CHIP for u3port;
3. bank's offset in each port are also rearranged;

Signed-off-by: Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/phy/phy-mt65xx-usb3.c |  344 ++++++++++++++++++++++++++---------------
 1 file changed, 217 insertions(+), 127 deletions(-)

diff --git a/drivers/phy/phy-mt65xx-usb3.c b/drivers/phy/phy-mt65xx-usb3.c
index 3403572..5eea65c 100644
--- a/drivers/phy/phy-mt65xx-usb3.c
+++ b/drivers/phy/phy-mt65xx-usb3.c
@@ -23,46 +23,54 @@
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 
-/*
- * for sifslv2 register, but exclude port's;
- * relative to USB3_SIF2_BASE base address
- */
-#define SSUSB_SIFSLV_SPLLC		0x0000
-#define SSUSB_SIFSLV_U2FREQ		0x0100
-
-/* offsets of banks in each u2phy registers */
-#define SSUSB_SIFSLV_U2PHY_COM_BASE	0x0000
-/* offsets of banks in each u3phy registers */
-#define SSUSB_SIFSLV_U3PHYD_BASE	0x0000
-#define SSUSB_SIFSLV_U3PHYA_BASE	0x0200
-
-#define U3P_USBPHYACR0		(SSUSB_SIFSLV_U2PHY_COM_BASE + 0x0000)
+/* version V1 sub-banks offset base address */
+/* banks shared by multiple phys */
+#define SSUSB_SIFSLV_V1_SPLLC		0x000	/* shared by u3 phys */
+#define SSUSB_SIFSLV_V1_U2FREQ		0x100	/* shared by u2 phys */
+/* u2 phy bank */
+#define SSUSB_SIFSLV_V1_U2PHY_COM	0x000
+/* u3 phy banks */
+#define SSUSB_SIFSLV_V1_U3PHYD		0x000
+#define SSUSB_SIFSLV_V1_U3PHYA		0x200
+
+/* version V2 sub-banks offset base address */
+/* u2 phy banks */
+#define SSUSB_SIFSLV_V2_MISC		0x000
+#define SSUSB_SIFSLV_V2_U2FREQ		0x100
+#define SSUSB_SIFSLV_V2_U2PHY_COM	0x300
+/* u3 phy banks */
+#define SSUSB_SIFSLV_V2_SPLLC		0x000
+#define SSUSB_SIFSLV_V2_CHIP		0x100
+#define SSUSB_SIFSLV_V2_U3PHYD		0x200
+#define SSUSB_SIFSLV_V2_U3PHYA		0x400
+
+#define U3P_USBPHYACR0		0x000
 #define PA0_RG_U2PLL_FORCE_ON		BIT(15)
 
-#define U3P_USBPHYACR2		(SSUSB_SIFSLV_U2PHY_COM_BASE + 0x0008)
+#define U3P_USBPHYACR2		0x008
 #define PA2_RG_SIF_U2PLL_FORCE_EN	BIT(18)
 
-#define U3P_USBPHYACR5		(SSUSB_SIFSLV_U2PHY_COM_BASE + 0x0014)
+#define U3P_USBPHYACR5		0x014
 #define PA5_RG_U2_HSTX_SRCAL_EN	BIT(15)
 #define PA5_RG_U2_HSTX_SRCTRL		GENMASK(14, 12)
 #define PA5_RG_U2_HSTX_SRCTRL_VAL(x)	((0x7 & (x)) << 12)
 #define PA5_RG_U2_HS_100U_U3_EN	BIT(11)
 
-#define U3P_USBPHYACR6		(SSUSB_SIFSLV_U2PHY_COM_BASE + 0x0018)
+#define U3P_USBPHYACR6		0x018
 #define PA6_RG_U2_BC11_SW_EN		BIT(23)
 #define PA6_RG_U2_OTG_VBUSCMP_EN	BIT(20)
 #define PA6_RG_U2_SQTH		GENMASK(3, 0)
 #define PA6_RG_U2_SQTH_VAL(x)	(0xf & (x))
 
-#define U3P_U2PHYACR4		(SSUSB_SIFSLV_U2PHY_COM_BASE + 0x0020)
+#define U3P_U2PHYACR4		0x020
 #define P2C_RG_USB20_GPIO_CTL		BIT(9)
 #define P2C_USB20_GPIO_MODE		BIT(8)
 #define P2C_U2_GPIO_CTR_MSK	(P2C_RG_USB20_GPIO_CTL | P2C_USB20_GPIO_MODE)
 
-#define U3D_U2PHYDCR0		(SSUSB_SIFSLV_U2PHY_COM_BASE + 0x0060)
+#define U3D_U2PHYDCR0		0x060
 #define P2C_RG_SIF_U2PLL_FORCE_ON	BIT(24)
 
-#define U3P_U2PHYDTM0		(SSUSB_SIFSLV_U2PHY_COM_BASE + 0x0068)
+#define U3P_U2PHYDTM0		0x068
 #define P2C_FORCE_UART_EN		BIT(26)
 #define P2C_FORCE_DATAIN		BIT(23)
 #define P2C_FORCE_DM_PULLDOWN		BIT(21)
@@ -84,59 +92,56 @@
 		P2C_FORCE_TERMSEL | P2C_RG_DMPULLDOWN | \
 		P2C_RG_DPPULLDOWN | P2C_RG_TERMSEL)
 
-#define U3P_U2PHYDTM1		(SSUSB_SIFSLV_U2PHY_COM_BASE + 0x006C)
+#define U3P_U2PHYDTM1		0x06C
 #define P2C_RG_UART_EN			BIT(16)
 #define P2C_RG_VBUSVALID		BIT(5)
 #define P2C_RG_SESSEND			BIT(4)
 #define P2C_RG_AVALID			BIT(2)
 
-#define U3P_U3_PHYA_REG0	(SSUSB_SIFSLV_U3PHYA_BASE + 0x0000)
-#define P3A_RG_U3_VUSB10_ON		BIT(5)
-
-#define U3P_U3_PHYA_REG6	(SSUSB_SIFSLV_U3PHYA_BASE + 0x0018)
+#define U3P_U3_PHYA_REG6	0x018
 #define P3A_RG_TX_EIDLE_CM		GENMASK(31, 28)
 #define P3A_RG_TX_EIDLE_CM_VAL(x)	((0xf & (x)) << 28)
 
-#define U3P_U3_PHYA_REG9	(SSUSB_SIFSLV_U3PHYA_BASE + 0x0024)
+#define U3P_U3_PHYA_REG9	0x024
 #define P3A_RG_RX_DAC_MUX		GENMASK(5, 1)
 #define P3A_RG_RX_DAC_MUX_VAL(x)	((0x1f & (x)) << 1)
 
-#define U3P_U3PHYA_DA_REG0	(SSUSB_SIFSLV_U3PHYA_BASE + 0x0100)
+#define U3P_U3_PHYA_DA_REG0	0x100
 #define P3A_RG_XTAL_EXT_EN_U3		GENMASK(11, 10)
 #define P3A_RG_XTAL_EXT_EN_U3_VAL(x)	((0x3 & (x)) << 10)
 
-#define U3P_U3_PHYD_LFPS1		(SSUSB_SIFSLV_U3PHYD_BASE + 0x000c)
+#define U3P_U3_PHYD_LFPS1		0x00c
 #define P3D_RG_FWAKE_TH		GENMASK(21, 16)
 #define P3D_RG_FWAKE_TH_VAL(x)	((0x3f & (x)) << 16)
 
-#define U3P_PHYD_CDR1		(SSUSB_SIFSLV_U3PHYD_BASE + 0x005c)
+#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)
 #define P3D_RG_CDR_BIR_LTD0		GENMASK(12, 8)
 #define P3D_RG_CDR_BIR_LTD0_VAL(x)	((0x1f & (x)) << 8)
 
-#define U3P_U3_PHYD_RXDET1		(SSUSB_SIFSLV_U3PHYD_BASE + 0x128)
+#define U3P_U3_PHYD_RXDET1		0x128
 #define P3D_RG_RXDET_STB2_SET		GENMASK(17, 9)
 #define P3D_RG_RXDET_STB2_SET_VAL(x)	((0x1ff & (x)) << 9)
 
-#define U3P_U3_PHYD_RXDET2		(SSUSB_SIFSLV_U3PHYD_BASE + 0x12c)
+#define U3P_U3_PHYD_RXDET2		0x12c
 #define P3D_RG_RXDET_STB2_SET_P3	GENMASK(8, 0)
 #define P3D_RG_RXDET_STB2_SET_P3_VAL(x)	(0x1ff & (x))
 
-#define U3P_XTALCTL3		(SSUSB_SIFSLV_SPLLC + 0x0018)
+#define U3P_SPLLC_XTALCTL3		0x018
 #define XC3_RG_U3_XTAL_RX_PWD		BIT(9)
 #define XC3_RG_U3_FRC_XTAL_RX_PWD	BIT(8)
 
-#define U3P_U2FREQ_FMCR0	(SSUSB_SIFSLV_U2FREQ + 0x00)
+#define U3P_U2FREQ_FMCR0	0x00
 #define P2F_RG_MONCLK_SEL	GENMASK(27, 26)
 #define P2F_RG_MONCLK_SEL_VAL(x)	((0x3 & (x)) << 26)
 #define P2F_RG_FREQDET_EN	BIT(24)
 #define P2F_RG_CYCLECNT		GENMASK(23, 0)
 #define P2F_RG_CYCLECNT_VAL(x)	((P2F_RG_CYCLECNT) & (x))
 
-#define U3P_U2FREQ_VALUE	(SSUSB_SIFSLV_U2FREQ + 0x0c)
+#define U3P_U2FREQ_VALUE	0x0c
 
-#define U3P_U2FREQ_FMMONR1	(SSUSB_SIFSLV_U2FREQ + 0x10)
+#define U3P_U2FREQ_FMMONR1	0x10
 #define P2F_USB_FM_VALID	BIT(0)
 #define P2F_RG_FRCK_EN		BIT(8)
 
@@ -145,14 +150,37 @@
 #define U3P_SR_COEF_DIVISOR	1000
 #define U3P_FM_DET_CYCLE_CNT	1024
 
+enum mt_phy_version {
+	MT_PHY_V1 = 1,
+	MT_PHY_V2,
+};
+
 struct mt65xx_phy_pdata {
 	/* avoid RX sensitivity level degradation only for mt8173 */
 	bool avoid_rx_sen_degradation;
+	enum mt_phy_version version;
+};
+
+struct u2phy_banks {
+	void __iomem *misc;
+	void __iomem *fmreg;
+	void __iomem *com;
+};
+
+struct u3phy_banks {
+	void __iomem *spllc;
+	void __iomem *chip;
+	void __iomem *phyd; /* include u3phyd_bank2 */
+	void __iomem *phya; /* include u3phya_da */
 };
 
 struct mt65xx_phy_instance {
 	struct phy *phy;
 	void __iomem *port_base;
+	union {
+		struct u2phy_banks u2_banks;
+		struct u3phy_banks u3_banks;
+	};
 	struct clk *ref_clk;	/* reference clock of anolog phy */
 	u32 index;
 	u8 type;
@@ -171,49 +199,53 @@ struct mt65xx_u3phy {
 static void hs_slew_rate_calibrate(struct mt65xx_u3phy *u3phy,
 	struct mt65xx_phy_instance *instance)
 {
-	void __iomem *sif_base = u3phy->sif_base;
+	struct u2phy_banks *u2_banks = &instance->u2_banks;
+	void __iomem *fmreg = u2_banks->fmreg;
+	void __iomem *com = u2_banks->com;
 	int calibration_val;
 	int fm_out;
 	u32 tmp;
 
 	/* enable USB ring oscillator */
-	tmp = readl(instance->port_base + U3P_USBPHYACR5);
+	tmp = readl(com + U3P_USBPHYACR5);
 	tmp |= PA5_RG_U2_HSTX_SRCAL_EN;
-	writel(tmp, instance->port_base + U3P_USBPHYACR5);
+	writel(tmp, com + U3P_USBPHYACR5);
 	udelay(1);
 
 	/*enable free run clock */
-	tmp = readl(sif_base + U3P_U2FREQ_FMMONR1);
+	tmp = readl(fmreg + U3P_U2FREQ_FMMONR1);
 	tmp |= P2F_RG_FRCK_EN;
-	writel(tmp, sif_base + U3P_U2FREQ_FMMONR1);
+	writel(tmp, fmreg + U3P_U2FREQ_FMMONR1);
 
 	/* set cycle count as 1024, and select u2 channel */
-	tmp = readl(sif_base + U3P_U2FREQ_FMCR0);
+	tmp = readl(fmreg + U3P_U2FREQ_FMCR0);
 	tmp &= ~(P2F_RG_CYCLECNT | P2F_RG_MONCLK_SEL);
 	tmp |= P2F_RG_CYCLECNT_VAL(U3P_FM_DET_CYCLE_CNT);
-	tmp |= P2F_RG_MONCLK_SEL_VAL(instance->index >> 1);
-	writel(tmp, sif_base + U3P_U2FREQ_FMCR0);
+	if (u3phy->pdata->version == MT_PHY_V1)
+		tmp |= P2F_RG_MONCLK_SEL_VAL(instance->index >> 1);
+
+	writel(tmp, fmreg + U3P_U2FREQ_FMCR0);
 
 	/* enable frequency meter */
-	tmp = readl(sif_base + U3P_U2FREQ_FMCR0);
+	tmp = readl(fmreg + U3P_U2FREQ_FMCR0);
 	tmp |= P2F_RG_FREQDET_EN;
-	writel(tmp, sif_base + U3P_U2FREQ_FMCR0);
+	writel(tmp, fmreg + U3P_U2FREQ_FMCR0);
 
 	/* ignore return value */
-	readl_poll_timeout(sif_base + U3P_U2FREQ_FMMONR1, tmp,
+	readl_poll_timeout(fmreg + U3P_U2FREQ_FMMONR1, tmp,
 		  (tmp & P2F_USB_FM_VALID), 10, 200);
 
-	fm_out = readl(sif_base + U3P_U2FREQ_VALUE);
+	fm_out = readl(fmreg + U3P_U2FREQ_VALUE);
 
 	/* disable frequency meter */
-	tmp = readl(sif_base + U3P_U2FREQ_FMCR0);
+	tmp = readl(fmreg + U3P_U2FREQ_FMCR0);
 	tmp &= ~P2F_RG_FREQDET_EN;
-	writel(tmp, sif_base + U3P_U2FREQ_FMCR0);
+	writel(tmp, fmreg + U3P_U2FREQ_FMCR0);
 
 	/*disable free run clock */
-	tmp = readl(sif_base + U3P_U2FREQ_FMMONR1);
+	tmp = readl(fmreg + U3P_U2FREQ_FMMONR1);
 	tmp &= ~P2F_RG_FRCK_EN;
-	writel(tmp, sif_base + U3P_U2FREQ_FMMONR1);
+	writel(tmp, fmreg + U3P_U2FREQ_FMMONR1);
 
 	if (fm_out) {
 		/* ( 1024 / FM_OUT ) x reference clock frequency x 0.028 */
@@ -228,63 +260,63 @@ static void hs_slew_rate_calibrate(struct mt65xx_u3phy *u3phy,
 		instance->index, fm_out, calibration_val);
 
 	/* set HS slew rate */
-	tmp = readl(instance->port_base + U3P_USBPHYACR5);
+	tmp = readl(com + U3P_USBPHYACR5);
 	tmp &= ~PA5_RG_U2_HSTX_SRCTRL;
 	tmp |= PA5_RG_U2_HSTX_SRCTRL_VAL(calibration_val);
-	writel(tmp, instance->port_base + U3P_USBPHYACR5);
+	writel(tmp, com + U3P_USBPHYACR5);
 
 	/* disable USB ring oscillator */
-	tmp = readl(instance->port_base + U3P_USBPHYACR5);
+	tmp = readl(com + U3P_USBPHYACR5);
 	tmp &= ~PA5_RG_U2_HSTX_SRCAL_EN;
-	writel(tmp, instance->port_base + U3P_USBPHYACR5);
+	writel(tmp, com + U3P_USBPHYACR5);
 }
 
 static void u3_phy_instance_init(struct mt65xx_u3phy *u3phy,
 	struct mt65xx_phy_instance *instance)
 {
-	void __iomem *port_base = instance->port_base;
+	struct u3phy_banks *u3_banks = &instance->u3_banks;
 	u32 tmp;
 
 	/* gating PCIe Analog XTAL clock */
-	tmp = readl(u3phy->sif_base + U3P_XTALCTL3);
+	tmp = readl(u3_banks->spllc + U3P_SPLLC_XTALCTL3);
 	tmp |= XC3_RG_U3_XTAL_RX_PWD | XC3_RG_U3_FRC_XTAL_RX_PWD;
-	writel(tmp, u3phy->sif_base + U3P_XTALCTL3);
+	writel(tmp, u3_banks->spllc + U3P_SPLLC_XTALCTL3);
 
 	/* gating XSQ */
-	tmp = readl(port_base + U3P_U3PHYA_DA_REG0);
+	tmp = readl(u3_banks->phya + U3P_U3_PHYA_DA_REG0);
 	tmp &= ~P3A_RG_XTAL_EXT_EN_U3;
 	tmp |= P3A_RG_XTAL_EXT_EN_U3_VAL(2);
-	writel(tmp, port_base + U3P_U3PHYA_DA_REG0);
+	writel(tmp, u3_banks->phya + U3P_U3_PHYA_DA_REG0);
 
-	tmp = readl(port_base + U3P_U3_PHYA_REG9);
+	tmp = readl(u3_banks->phya + U3P_U3_PHYA_REG9);
 	tmp &= ~P3A_RG_RX_DAC_MUX;
 	tmp |= P3A_RG_RX_DAC_MUX_VAL(4);
-	writel(tmp, port_base + U3P_U3_PHYA_REG9);
+	writel(tmp, u3_banks->phya + U3P_U3_PHYA_REG9);
 
-	tmp = readl(port_base + U3P_U3_PHYA_REG6);
+	tmp = readl(u3_banks->phya + U3P_U3_PHYA_REG6);
 	tmp &= ~P3A_RG_TX_EIDLE_CM;
 	tmp |= P3A_RG_TX_EIDLE_CM_VAL(0xe);
-	writel(tmp, port_base + U3P_U3_PHYA_REG6);
+	writel(tmp, u3_banks->phya + U3P_U3_PHYA_REG6);
 
-	tmp = readl(port_base + U3P_PHYD_CDR1);
+	tmp = readl(u3_banks->phyd + U3P_U3_PHYD_CDR1);
 	tmp &= ~(P3D_RG_CDR_BIR_LTD0 | P3D_RG_CDR_BIR_LTD1);
 	tmp |= P3D_RG_CDR_BIR_LTD0_VAL(0xc) | P3D_RG_CDR_BIR_LTD1_VAL(0x3);
-	writel(tmp, port_base + U3P_PHYD_CDR1);
+	writel(tmp, u3_banks->phyd + U3P_U3_PHYD_CDR1);
 
-	tmp = readl(port_base + U3P_U3_PHYD_LFPS1);
+	tmp = readl(u3_banks->phyd + U3P_U3_PHYD_LFPS1);
 	tmp &= ~P3D_RG_FWAKE_TH;
 	tmp |= P3D_RG_FWAKE_TH_VAL(0x34);
-	writel(tmp, port_base + U3P_U3_PHYD_LFPS1);
+	writel(tmp, u3_banks->phyd + U3P_U3_PHYD_LFPS1);
 
-	tmp = readl(port_base + U3P_U3_PHYD_RXDET1);
+	tmp = readl(u3_banks->phyd + U3P_U3_PHYD_RXDET1);
 	tmp &= ~P3D_RG_RXDET_STB2_SET;
 	tmp |= P3D_RG_RXDET_STB2_SET_VAL(0x10);
-	writel(tmp, port_base + U3P_U3_PHYD_RXDET1);
+	writel(tmp, u3_banks->phyd + U3P_U3_PHYD_RXDET1);
 
-	tmp = readl(port_base + U3P_U3_PHYD_RXDET2);
+	tmp = readl(u3_banks->phyd + U3P_U3_PHYD_RXDET2);
 	tmp &= ~P3D_RG_RXDET_STB2_SET_P3;
 	tmp |= P3D_RG_RXDET_STB2_SET_P3_VAL(0x10);
-	writel(tmp, port_base + U3P_U3_PHYD_RXDET2);
+	writel(tmp, u3_banks->phyd + U3P_U3_PHYD_RXDET2);
 
 	dev_dbg(u3phy->dev, "%s(%d)\n", __func__, instance->index);
 }
@@ -292,51 +324,52 @@ static void u3_phy_instance_init(struct mt65xx_u3phy *u3phy,
 static void phy_instance_init(struct mt65xx_u3phy *u3phy,
 	struct mt65xx_phy_instance *instance)
 {
-	void __iomem *port_base = instance->port_base;
+	struct u2phy_banks *u2_banks = &instance->u2_banks;
+	void __iomem *com = u2_banks->com;
 	u32 index = instance->index;
 	u32 tmp;
 
 	/* switch to USB function. (system register, force ip into usb mode) */
-	tmp = readl(port_base + U3P_U2PHYDTM0);
+	tmp = readl(com + U3P_U2PHYDTM0);
 	tmp &= ~P2C_FORCE_UART_EN;
 	tmp |= P2C_RG_XCVRSEL_VAL(1) | P2C_RG_DATAIN_VAL(0);
-	writel(tmp, port_base + U3P_U2PHYDTM0);
+	writel(tmp, com + U3P_U2PHYDTM0);
 
-	tmp = readl(port_base + U3P_U2PHYDTM1);
+	tmp = readl(com + U3P_U2PHYDTM1);
 	tmp &= ~P2C_RG_UART_EN;
-	writel(tmp, port_base + U3P_U2PHYDTM1);
+	writel(tmp, com + U3P_U2PHYDTM1);
 
 	if (!index) {
-		tmp = readl(port_base + U3P_U2PHYACR4);
+		tmp = readl(com + U3P_U2PHYACR4);
 		tmp &= ~P2C_U2_GPIO_CTR_MSK;
-		writel(tmp, port_base + U3P_U2PHYACR4);
+		writel(tmp, com + U3P_U2PHYACR4);
 	}
 
 	if (u3phy->pdata->avoid_rx_sen_degradation) {
 		if (!index) {
-			tmp = readl(port_base + U3P_USBPHYACR2);
+			tmp = readl(com + U3P_USBPHYACR2);
 			tmp |= PA2_RG_SIF_U2PLL_FORCE_EN;
-			writel(tmp, port_base + U3P_USBPHYACR2);
+			writel(tmp, com + U3P_USBPHYACR2);
 
-			tmp = readl(port_base + U3D_U2PHYDCR0);
+			tmp = readl(com + U3D_U2PHYDCR0);
 			tmp &= ~P2C_RG_SIF_U2PLL_FORCE_ON;
-			writel(tmp, port_base + U3D_U2PHYDCR0);
+			writel(tmp, com + U3D_U2PHYDCR0);
 		} else {
-			tmp = readl(port_base + U3D_U2PHYDCR0);
+			tmp = readl(com + U3D_U2PHYDCR0);
 			tmp |= P2C_RG_SIF_U2PLL_FORCE_ON;
-			writel(tmp, port_base + U3D_U2PHYDCR0);
+			writel(tmp, com + U3D_U2PHYDCR0);
 
-			tmp = readl(port_base + U3P_U2PHYDTM0);
+			tmp = readl(com + U3P_U2PHYDTM0);
 			tmp |= P2C_RG_SUSPENDM | P2C_FORCE_SUSPENDM;
-			writel(tmp, port_base + U3P_U2PHYDTM0);
+			writel(tmp, com + U3P_U2PHYDTM0);
 		}
 	}
 
-	tmp = readl(port_base + U3P_USBPHYACR6);
+	tmp = readl(com + U3P_USBPHYACR6);
 	tmp &= ~PA6_RG_U2_BC11_SW_EN;	/* DP/DM BC1.1 path Disable */
 	tmp &= ~PA6_RG_U2_SQTH;
 	tmp |= PA6_RG_U2_SQTH_VAL(2);
-	writel(tmp, port_base + U3P_USBPHYACR6);
+	writel(tmp, com + U3P_USBPHYACR6);
 
 	dev_dbg(u3phy->dev, "%s(%d)\n", __func__, index);
 }
@@ -344,41 +377,42 @@ static void phy_instance_init(struct mt65xx_u3phy *u3phy,
 static void phy_instance_power_on(struct mt65xx_u3phy *u3phy,
 	struct mt65xx_phy_instance *instance)
 {
-	void __iomem *port_base = instance->port_base;
+	struct u2phy_banks *u2_banks = &instance->u2_banks;
+	void __iomem *com = u2_banks->com;
 	u32 index = instance->index;
 	u32 tmp;
 
 	/* (force_suspendm=0) (let suspendm=1, enable usb 480MHz pll) */
-	tmp = readl(port_base + U3P_U2PHYDTM0);
+	tmp = readl(com + U3P_U2PHYDTM0);
 	tmp &= ~(P2C_FORCE_SUSPENDM | P2C_RG_XCVRSEL);
 	tmp &= ~(P2C_RG_DATAIN | P2C_DTM0_PART_MASK);
-	writel(tmp, port_base + U3P_U2PHYDTM0);
+	writel(tmp, com + U3P_U2PHYDTM0);
 
 	/* OTG Enable */
-	tmp = readl(port_base + U3P_USBPHYACR6);
+	tmp = readl(com + U3P_USBPHYACR6);
 	tmp |= PA6_RG_U2_OTG_VBUSCMP_EN;
-	writel(tmp, port_base + U3P_USBPHYACR6);
+	writel(tmp, com + U3P_USBPHYACR6);
 
 	if (!index) {
 		/* switch 100uA current to SSUSB */
-		tmp = readl(port_base + U3P_USBPHYACR5);
+		tmp = readl(com + U3P_USBPHYACR5);
 		tmp |= PA5_RG_U2_HS_100U_U3_EN;
-		writel(tmp, port_base + U3P_USBPHYACR5);
+		writel(tmp, com + U3P_USBPHYACR5);
 	}
 
-	tmp = readl(port_base + U3P_U2PHYDTM1);
+	tmp = readl(com + U3P_U2PHYDTM1);
 	tmp |= P2C_RG_VBUSVALID | P2C_RG_AVALID;
 	tmp &= ~P2C_RG_SESSEND;
-	writel(tmp, port_base + U3P_U2PHYDTM1);
+	writel(tmp, com + U3P_U2PHYDTM1);
 
 	if (u3phy->pdata->avoid_rx_sen_degradation && index) {
-		tmp = readl(port_base + U3D_U2PHYDCR0);
+		tmp = readl(com + U3D_U2PHYDCR0);
 		tmp |= P2C_RG_SIF_U2PLL_FORCE_ON;
-		writel(tmp, port_base + U3D_U2PHYDCR0);
+		writel(tmp, com + U3D_U2PHYDCR0);
 
-		tmp = readl(port_base + U3P_U2PHYDTM0);
+		tmp = readl(com + U3P_U2PHYDTM0);
 		tmp |= P2C_RG_SUSPENDM | P2C_FORCE_SUSPENDM;
-		writel(tmp, port_base + U3P_U2PHYDTM0);
+		writel(tmp, com + U3P_U2PHYDTM0);
 	}
 	dev_dbg(u3phy->dev, "%s(%d)\n", __func__, index);
 }
@@ -386,42 +420,43 @@ static void phy_instance_power_on(struct mt65xx_u3phy *u3phy,
 static void phy_instance_power_off(struct mt65xx_u3phy *u3phy,
 	struct mt65xx_phy_instance *instance)
 {
-	void __iomem *port_base = instance->port_base;
+	struct u2phy_banks *u2_banks = &instance->u2_banks;
+	void __iomem *com = u2_banks->com;
 	u32 index = instance->index;
 	u32 tmp;
 
-	tmp = readl(port_base + U3P_U2PHYDTM0);
+	tmp = readl(com + U3P_U2PHYDTM0);
 	tmp &= ~(P2C_RG_XCVRSEL | P2C_RG_DATAIN);
 	tmp |= P2C_FORCE_SUSPENDM;
-	writel(tmp, port_base + U3P_U2PHYDTM0);
+	writel(tmp, com + U3P_U2PHYDTM0);
 
 	/* OTG Disable */
-	tmp = readl(port_base + U3P_USBPHYACR6);
+	tmp = readl(com + U3P_USBPHYACR6);
 	tmp &= ~PA6_RG_U2_OTG_VBUSCMP_EN;
-	writel(tmp, port_base + U3P_USBPHYACR6);
+	writel(tmp, com + U3P_USBPHYACR6);
 
 	if (!index) {
 		/* switch 100uA current back to USB2.0 */
-		tmp = readl(port_base + U3P_USBPHYACR5);
+		tmp = readl(com + U3P_USBPHYACR5);
 		tmp &= ~PA5_RG_U2_HS_100U_U3_EN;
-		writel(tmp, port_base + U3P_USBPHYACR5);
+		writel(tmp, com + U3P_USBPHYACR5);
 	}
 
 	/* let suspendm=0, set utmi into analog power down */
-	tmp = readl(port_base + U3P_U2PHYDTM0);
+	tmp = readl(com + U3P_U2PHYDTM0);
 	tmp &= ~P2C_RG_SUSPENDM;
-	writel(tmp, port_base + U3P_U2PHYDTM0);
+	writel(tmp, com + U3P_U2PHYDTM0);
 	udelay(1);
 
-	tmp = readl(port_base + U3P_U2PHYDTM1);
+	tmp = readl(com + U3P_U2PHYDTM1);
 	tmp &= ~(P2C_RG_VBUSVALID | P2C_RG_AVALID);
 	tmp |= P2C_RG_SESSEND;
-	writel(tmp, port_base + U3P_U2PHYDTM1);
+	writel(tmp, com + U3P_U2PHYDTM1);
 
 	if (u3phy->pdata->avoid_rx_sen_degradation && index) {
-		tmp = readl(port_base + U3D_U2PHYDCR0);
+		tmp = readl(com + U3D_U2PHYDCR0);
 		tmp &= ~P2C_RG_SIF_U2PLL_FORCE_ON;
-		writel(tmp, port_base + U3D_U2PHYDCR0);
+		writel(tmp, com + U3D_U2PHYDCR0);
 	}
 
 	dev_dbg(u3phy->dev, "%s(%d)\n", __func__, index);
@@ -430,18 +465,55 @@ static void phy_instance_power_off(struct mt65xx_u3phy *u3phy,
 static void phy_instance_exit(struct mt65xx_u3phy *u3phy,
 	struct mt65xx_phy_instance *instance)
 {
-	void __iomem *port_base = instance->port_base;
+	struct u2phy_banks *u2_banks = &instance->u2_banks;
+	void __iomem *com = u2_banks->com;
 	u32 index = instance->index;
 	u32 tmp;
 
 	if (u3phy->pdata->avoid_rx_sen_degradation && index) {
-		tmp = readl(port_base + U3D_U2PHYDCR0);
+		tmp = readl(com + U3D_U2PHYDCR0);
 		tmp &= ~P2C_RG_SIF_U2PLL_FORCE_ON;
-		writel(tmp, port_base + U3D_U2PHYDCR0);
+		writel(tmp, com + U3D_U2PHYDCR0);
 
-		tmp = readl(port_base + U3P_U2PHYDTM0);
+		tmp = readl(com + U3P_U2PHYDTM0);
 		tmp &= ~P2C_FORCE_SUSPENDM;
-		writel(tmp, port_base + U3P_U2PHYDTM0);
+		writel(tmp, com + U3P_U2PHYDTM0);
+	}
+}
+
+static void phy_v1_banks_init(struct mt65xx_u3phy *u3phy,
+	struct mt65xx_phy_instance *instance)
+{
+	struct u2phy_banks *u2_banks = &instance->u2_banks;
+	struct u3phy_banks *u3_banks = &instance->u3_banks;
+
+	if (instance->type == PHY_TYPE_USB2) {
+		u2_banks->misc = NULL;
+		u2_banks->fmreg = u3phy->sif_base + SSUSB_SIFSLV_V1_U2FREQ;
+		u2_banks->com = instance->port_base + SSUSB_SIFSLV_V1_U2PHY_COM;
+	} else if (instance->type == PHY_TYPE_USB3) {
+		u3_banks->spllc = u3phy->sif_base + SSUSB_SIFSLV_V1_SPLLC;
+		u3_banks->chip = NULL;
+		u3_banks->phyd = instance->port_base + SSUSB_SIFSLV_V1_U3PHYD;
+		u3_banks->phya = instance->port_base + SSUSB_SIFSLV_V1_U3PHYA;
+	}
+}
+
+static void phy_v2_banks_init(struct mt65xx_u3phy *u3phy,
+	struct mt65xx_phy_instance *instance)
+{
+	struct u2phy_banks *u2_banks = &instance->u2_banks;
+	struct u3phy_banks *u3_banks = &instance->u3_banks;
+
+	if (instance->type == PHY_TYPE_USB2) {
+		u2_banks->misc = instance->port_base + SSUSB_SIFSLV_V2_MISC;
+		u2_banks->fmreg = instance->port_base + SSUSB_SIFSLV_V2_U2FREQ;
+		u2_banks->com = instance->port_base + SSUSB_SIFSLV_V2_U2PHY_COM;
+	} else if (instance->type == PHY_TYPE_USB3) {
+		u3_banks->spllc = instance->port_base + SSUSB_SIFSLV_V2_SPLLC;
+		u3_banks->chip = instance->port_base + SSUSB_SIFSLV_V2_CHIP;
+		u3_banks->phyd = instance->port_base + SSUSB_SIFSLV_V2_U3PHYD;
+		u3_banks->phya = instance->port_base + SSUSB_SIFSLV_V2_U3PHYA;
 	}
 }
 
@@ -515,7 +587,6 @@ static struct phy *mt65xx_phy_xlate(struct device *dev,
 	struct device_node *phy_np = args->np;
 	int index;
 
-
 	if (args->args_count != 1) {
 		dev_err(dev, "invalid number of cells in 'phy' property\n");
 		return ERR_PTR(-EINVAL);
@@ -533,13 +604,21 @@ static struct phy *mt65xx_phy_xlate(struct device *dev,
 	}
 
 	instance->type = args->args[0];
-
 	if (!(instance->type == PHY_TYPE_USB2 ||
 	      instance->type == PHY_TYPE_USB3)) {
 		dev_err(dev, "unsupported device type: %d\n", instance->type);
 		return ERR_PTR(-EINVAL);
 	}
 
+	if (u3phy->pdata->version == MT_PHY_V1) {
+		phy_v1_banks_init(u3phy, instance);
+	} else if (u3phy->pdata->version == MT_PHY_V2) {
+		phy_v2_banks_init(u3phy, instance);
+	} else {
+		dev_err(dev, "phy version is not supported\n");
+		return ERR_PTR(-EINVAL);
+	}
+
 	return instance->phy;
 }
 
@@ -553,14 +632,22 @@ static struct phy *mt65xx_phy_xlate(struct device *dev,
 
 static const struct mt65xx_phy_pdata mt2701_pdata = {
 	.avoid_rx_sen_degradation = false,
+	.version = MT_PHY_V1,
+};
+
+static const struct mt65xx_phy_pdata mt2712_pdata = {
+	.avoid_rx_sen_degradation = false,
+	.version = MT_PHY_V2,
 };
 
 static const struct mt65xx_phy_pdata mt8173_pdata = {
 	.avoid_rx_sen_degradation = true,
+	.version = MT_PHY_V1,
 };
 
 static const struct of_device_id mt65xx_u3phy_id_table[] = {
 	{ .compatible = "mediatek,mt2701-u3phy", .data = &mt2701_pdata },
+	{ .compatible = "mediatek,mt2712-u3phy", .data = &mt2712_pdata },
 	{ .compatible = "mediatek,mt8173-u3phy", .data = &mt8173_pdata },
 	{ },
 };
@@ -596,11 +683,14 @@ static int mt65xx_u3phy_probe(struct platform_device *pdev)
 	u3phy->dev = dev;
 	platform_set_drvdata(pdev, u3phy);
 
-	sif_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	u3phy->sif_base = devm_ioremap_resource(dev, sif_res);
-	if (IS_ERR(u3phy->sif_base)) {
-		dev_err(dev, "failed to remap sif regs\n");
-		return PTR_ERR(u3phy->sif_base);
+	if (u3phy->pdata->version == MT_PHY_V1) {
+		/* get banks shared by multiple phys */
+		sif_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+		u3phy->sif_base = devm_ioremap_resource(dev, sif_res);
+		if (IS_ERR(u3phy->sif_base)) {
+			dev_err(dev, "failed to remap sif regs\n");
+			return PTR_ERR(u3phy->sif_base);
+		}
 	}
 
 	/* it's deprecated, make it optional for backward compatibility */
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v5 4/9] phy: phy-mt65xx-usb3: move clock from phy node into port nodes
From: Chunfeng Yun @ 2017-03-31  7:35 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Matthias Brugger, Felipe Balbi, Rob Herring, Mark Rutland,
	Ian Campbell, Chunfeng Yun, linux-kernel, linux-arm-kernel,
	linux-usb, linux-mediatek, devicetree
In-Reply-To: <1490945735-9531-1-git-send-email-chunfeng.yun@mediatek.com>

each port has its own reference clock, the HighSpeed port is 48M,
and the SuperSpeed port is usually 26M, put them into port node for
flexibility, this can close clock if the port is not used.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/phy/phy-mt65xx-usb3.c |   27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/phy-mt65xx-usb3.c b/drivers/phy/phy-mt65xx-usb3.c
index 7fff482..3403572 100644
--- a/drivers/phy/phy-mt65xx-usb3.c
+++ b/drivers/phy/phy-mt65xx-usb3.c
@@ -153,6 +153,7 @@ struct mt65xx_phy_pdata {
 struct mt65xx_phy_instance {
 	struct phy *phy;
 	void __iomem *port_base;
+	struct clk *ref_clk;	/* reference clock of anolog phy */
 	u32 index;
 	u8 type;
 };
@@ -160,6 +161,7 @@ struct mt65xx_phy_instance {
 struct mt65xx_u3phy {
 	struct device *dev;
 	void __iomem *sif_base;	/* only shared sif */
+	/* deprecated, use @ref_clk instead in phy instance */
 	struct clk *u3phya_ref;	/* reference clock of usb3 anolog phy */
 	const struct mt65xx_phy_pdata *pdata;
 	struct mt65xx_phy_instance **phys;
@@ -455,6 +457,12 @@ static int mt65xx_phy_init(struct phy *phy)
 		return ret;
 	}
 
+	ret = clk_prepare_enable(instance->ref_clk);
+	if (ret) {
+		dev_err(u3phy->dev, "failed to enable ref_clk\n");
+		return ret;
+	}
+
 	if (instance->type == PHY_TYPE_USB2)
 		phy_instance_init(u3phy, instance);
 	else
@@ -494,6 +502,7 @@ static int mt65xx_phy_exit(struct phy *phy)
 	if (instance->type == PHY_TYPE_USB2)
 		phy_instance_exit(u3phy, instance);
 
+	clk_disable_unprepare(instance->ref_clk);
 	clk_disable_unprepare(u3phy->u3phya_ref);
 	return 0;
 }
@@ -594,10 +603,13 @@ static int mt65xx_u3phy_probe(struct platform_device *pdev)
 		return PTR_ERR(u3phy->sif_base);
 	}
 
+	/* it's deprecated, make it optional for backward compatibility */
 	u3phy->u3phya_ref = devm_clk_get(dev, "u3phya_ref");
 	if (IS_ERR(u3phy->u3phya_ref)) {
-		dev_err(dev, "error to get u3phya_ref\n");
-		return PTR_ERR(u3phy->u3phya_ref);
+		if (PTR_ERR(u3phy->u3phya_ref) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+
+		u3phy->u3phya_ref = NULL;
 	}
 
 	port = 0;
@@ -638,6 +650,17 @@ static int mt65xx_u3phy_probe(struct platform_device *pdev)
 		instance->index = port;
 		phy_set_drvdata(phy, instance);
 		port++;
+
+		/* if deprecated clock is provided, ignore instance's one */
+		if (u3phy->u3phya_ref)
+			continue;
+
+		instance->ref_clk = devm_clk_get(&phy->dev, "ref");
+		if (IS_ERR(instance->ref_clk)) {
+			dev_err(dev, "failed to get ref_clk(id-%d)\n", port);
+			retval = PTR_ERR(instance->ref_clk);
+			goto put_child;
+		}
 	}
 
 	provider = devm_of_phy_provider_register(dev, mt65xx_phy_xlate);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v5 3/9] phy: phy-mt65xx-usb3: split SuperSpeed port into two ones
From: Chunfeng Yun @ 2017-03-31  7:35 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Felipe Balbi,
	Ian Campbell, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Chunfeng Yun, Rob Herring,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Matthias Brugger,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1490945735-9531-1-git-send-email-chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

Currently usb3 port in fact includes two sub-ports, but it is not
flexible for some cases, such as following one:
    usb3 port0 includes u2port0 and u3port0;
    usb2 port0 includes u2port1;
If wants to support only HS, we can use u2port0 or u2port1, when
select u2port0, u3port0 is not needed;
If wants to support SS, we can compound u2port0 and u3port0,
or u2port1 and u3port0, if select latter one, u2port0 is not needed.

So it's more flexible to split usb3 port into two ones and also try
best to save power by disabling unnecessary ports.

Signed-off-by: Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/phy/phy-mt65xx-usb3.c |  149 +++++++++++++++++++++--------------------
 1 file changed, 75 insertions(+), 74 deletions(-)

diff --git a/drivers/phy/phy-mt65xx-usb3.c b/drivers/phy/phy-mt65xx-usb3.c
index 4fd47d0..7fff482 100644
--- a/drivers/phy/phy-mt65xx-usb3.c
+++ b/drivers/phy/phy-mt65xx-usb3.c
@@ -30,11 +30,11 @@
 #define SSUSB_SIFSLV_SPLLC		0x0000
 #define SSUSB_SIFSLV_U2FREQ		0x0100
 
-/* offsets of sub-segment in each port registers */
+/* offsets of banks in each u2phy registers */
 #define SSUSB_SIFSLV_U2PHY_COM_BASE	0x0000
-#define SSUSB_SIFSLV_U3PHYD_BASE	0x0100
-#define SSUSB_USB30_PHYA_SIV_B_BASE	0x0300
-#define SSUSB_SIFSLV_U3PHYA_DA_BASE	0x0400
+/* offsets of banks in each u3phy registers */
+#define SSUSB_SIFSLV_U3PHYD_BASE	0x0000
+#define SSUSB_SIFSLV_U3PHYA_BASE	0x0200
 
 #define U3P_USBPHYACR0		(SSUSB_SIFSLV_U2PHY_COM_BASE + 0x0000)
 #define PA0_RG_U2PLL_FORCE_ON		BIT(15)
@@ -49,7 +49,6 @@
 #define PA5_RG_U2_HS_100U_U3_EN	BIT(11)
 
 #define U3P_USBPHYACR6		(SSUSB_SIFSLV_U2PHY_COM_BASE + 0x0018)
-#define PA6_RG_U2_ISO_EN		BIT(31)
 #define PA6_RG_U2_BC11_SW_EN		BIT(23)
 #define PA6_RG_U2_OTG_VBUSCMP_EN	BIT(20)
 #define PA6_RG_U2_SQTH		GENMASK(3, 0)
@@ -91,18 +90,18 @@
 #define P2C_RG_SESSEND			BIT(4)
 #define P2C_RG_AVALID			BIT(2)
 
-#define U3P_U3_PHYA_REG0	(SSUSB_USB30_PHYA_SIV_B_BASE + 0x0000)
+#define U3P_U3_PHYA_REG0	(SSUSB_SIFSLV_U3PHYA_BASE + 0x0000)
 #define P3A_RG_U3_VUSB10_ON		BIT(5)
 
-#define U3P_U3_PHYA_REG6	(SSUSB_USB30_PHYA_SIV_B_BASE + 0x0018)
+#define U3P_U3_PHYA_REG6	(SSUSB_SIFSLV_U3PHYA_BASE + 0x0018)
 #define P3A_RG_TX_EIDLE_CM		GENMASK(31, 28)
 #define P3A_RG_TX_EIDLE_CM_VAL(x)	((0xf & (x)) << 28)
 
-#define U3P_U3_PHYA_REG9	(SSUSB_USB30_PHYA_SIV_B_BASE + 0x0024)
+#define U3P_U3_PHYA_REG9	(SSUSB_SIFSLV_U3PHYA_BASE + 0x0024)
 #define P3A_RG_RX_DAC_MUX		GENMASK(5, 1)
 #define P3A_RG_RX_DAC_MUX_VAL(x)	((0x1f & (x)) << 1)
 
-#define U3P_U3PHYA_DA_REG0	(SSUSB_SIFSLV_U3PHYA_DA_BASE + 0x0000)
+#define U3P_U3PHYA_DA_REG0	(SSUSB_SIFSLV_U3PHYA_BASE + 0x0100)
 #define P3A_RG_XTAL_EXT_EN_U3		GENMASK(11, 10)
 #define P3A_RG_XTAL_EXT_EN_U3_VAL(x)	((0x3 & (x)) << 10)
 
@@ -160,7 +159,7 @@ struct mt65xx_phy_instance {
 
 struct mt65xx_u3phy {
 	struct device *dev;
-	void __iomem *sif_base;	/* include sif2, but exclude port's */
+	void __iomem *sif_base;	/* only shared sif */
 	struct clk *u3phya_ref;	/* reference clock of usb3 anolog phy */
 	const struct mt65xx_phy_pdata *pdata;
 	struct mt65xx_phy_instance **phys;
@@ -190,7 +189,7 @@ static void hs_slew_rate_calibrate(struct mt65xx_u3phy *u3phy,
 	tmp = readl(sif_base + U3P_U2FREQ_FMCR0);
 	tmp &= ~(P2F_RG_CYCLECNT | P2F_RG_MONCLK_SEL);
 	tmp |= P2F_RG_CYCLECNT_VAL(U3P_FM_DET_CYCLE_CNT);
-	tmp |= P2F_RG_MONCLK_SEL_VAL(instance->index);
+	tmp |= P2F_RG_MONCLK_SEL_VAL(instance->index >> 1);
 	writel(tmp, sif_base + U3P_U2FREQ_FMCR0);
 
 	/* enable frequency meter */
@@ -238,6 +237,56 @@ static void hs_slew_rate_calibrate(struct mt65xx_u3phy *u3phy,
 	writel(tmp, instance->port_base + U3P_USBPHYACR5);
 }
 
+static void u3_phy_instance_init(struct mt65xx_u3phy *u3phy,
+	struct mt65xx_phy_instance *instance)
+{
+	void __iomem *port_base = instance->port_base;
+	u32 tmp;
+
+	/* gating PCIe Analog XTAL clock */
+	tmp = readl(u3phy->sif_base + U3P_XTALCTL3);
+	tmp |= XC3_RG_U3_XTAL_RX_PWD | XC3_RG_U3_FRC_XTAL_RX_PWD;
+	writel(tmp, u3phy->sif_base + U3P_XTALCTL3);
+
+	/* gating XSQ */
+	tmp = readl(port_base + U3P_U3PHYA_DA_REG0);
+	tmp &= ~P3A_RG_XTAL_EXT_EN_U3;
+	tmp |= P3A_RG_XTAL_EXT_EN_U3_VAL(2);
+	writel(tmp, port_base + U3P_U3PHYA_DA_REG0);
+
+	tmp = readl(port_base + U3P_U3_PHYA_REG9);
+	tmp &= ~P3A_RG_RX_DAC_MUX;
+	tmp |= P3A_RG_RX_DAC_MUX_VAL(4);
+	writel(tmp, port_base + U3P_U3_PHYA_REG9);
+
+	tmp = readl(port_base + U3P_U3_PHYA_REG6);
+	tmp &= ~P3A_RG_TX_EIDLE_CM;
+	tmp |= P3A_RG_TX_EIDLE_CM_VAL(0xe);
+	writel(tmp, port_base + U3P_U3_PHYA_REG6);
+
+	tmp = readl(port_base + U3P_PHYD_CDR1);
+	tmp &= ~(P3D_RG_CDR_BIR_LTD0 | P3D_RG_CDR_BIR_LTD1);
+	tmp |= P3D_RG_CDR_BIR_LTD0_VAL(0xc) | P3D_RG_CDR_BIR_LTD1_VAL(0x3);
+	writel(tmp, port_base + U3P_PHYD_CDR1);
+
+	tmp = readl(port_base + U3P_U3_PHYD_LFPS1);
+	tmp &= ~P3D_RG_FWAKE_TH;
+	tmp |= P3D_RG_FWAKE_TH_VAL(0x34);
+	writel(tmp, port_base + U3P_U3_PHYD_LFPS1);
+
+	tmp = readl(port_base + U3P_U3_PHYD_RXDET1);
+	tmp &= ~P3D_RG_RXDET_STB2_SET;
+	tmp |= P3D_RG_RXDET_STB2_SET_VAL(0x10);
+	writel(tmp, port_base + U3P_U3_PHYD_RXDET1);
+
+	tmp = readl(port_base + U3P_U3_PHYD_RXDET2);
+	tmp &= ~P3D_RG_RXDET_STB2_SET_P3;
+	tmp |= P3D_RG_RXDET_STB2_SET_P3_VAL(0x10);
+	writel(tmp, port_base + U3P_U3_PHYD_RXDET2);
+
+	dev_dbg(u3phy->dev, "%s(%d)\n", __func__, instance->index);
+}
+
 static void phy_instance_init(struct mt65xx_u3phy *u3phy,
 	struct mt65xx_phy_instance *instance)
 {
@@ -287,41 +336,6 @@ static void phy_instance_init(struct mt65xx_u3phy *u3phy,
 	tmp |= PA6_RG_U2_SQTH_VAL(2);
 	writel(tmp, port_base + U3P_USBPHYACR6);
 
-	tmp = readl(port_base + U3P_U3PHYA_DA_REG0);
-	tmp &= ~P3A_RG_XTAL_EXT_EN_U3;
-	tmp |= P3A_RG_XTAL_EXT_EN_U3_VAL(2);
-	writel(tmp, port_base + U3P_U3PHYA_DA_REG0);
-
-	tmp = readl(port_base + U3P_U3_PHYA_REG9);
-	tmp &= ~P3A_RG_RX_DAC_MUX;
-	tmp |= P3A_RG_RX_DAC_MUX_VAL(4);
-	writel(tmp, port_base + U3P_U3_PHYA_REG9);
-
-	tmp = readl(port_base + U3P_U3_PHYA_REG6);
-	tmp &= ~P3A_RG_TX_EIDLE_CM;
-	tmp |= P3A_RG_TX_EIDLE_CM_VAL(0xe);
-	writel(tmp, port_base + U3P_U3_PHYA_REG6);
-
-	tmp = readl(port_base + U3P_PHYD_CDR1);
-	tmp &= ~(P3D_RG_CDR_BIR_LTD0 | P3D_RG_CDR_BIR_LTD1);
-	tmp |= P3D_RG_CDR_BIR_LTD0_VAL(0xc) | P3D_RG_CDR_BIR_LTD1_VAL(0x3);
-	writel(tmp, port_base + U3P_PHYD_CDR1);
-
-	tmp = readl(port_base + U3P_U3_PHYD_LFPS1);
-	tmp &= ~P3D_RG_FWAKE_TH;
-	tmp |= P3D_RG_FWAKE_TH_VAL(0x34);
-	writel(tmp, port_base + U3P_U3_PHYD_LFPS1);
-
-	tmp = readl(port_base + U3P_U3_PHYD_RXDET1);
-	tmp &= ~P3D_RG_RXDET_STB2_SET;
-	tmp |= P3D_RG_RXDET_STB2_SET_VAL(0x10);
-	writel(tmp, port_base + U3P_U3_PHYD_RXDET1);
-
-	tmp = readl(port_base + U3P_U3_PHYD_RXDET2);
-	tmp &= ~P3D_RG_RXDET_STB2_SET_P3;
-	tmp |= P3D_RG_RXDET_STB2_SET_P3_VAL(0x10);
-	writel(tmp, port_base + U3P_U3_PHYD_RXDET2);
-
 	dev_dbg(u3phy->dev, "%s(%d)\n", __func__, index);
 }
 
@@ -332,13 +346,6 @@ static void phy_instance_power_on(struct mt65xx_u3phy *u3phy,
 	u32 index = instance->index;
 	u32 tmp;
 
-	if (!index) {
-		/* Set RG_SSUSB_VUSB10_ON as 1 after VUSB10 ready */
-		tmp = readl(port_base + U3P_U3_PHYA_REG0);
-		tmp |= P3A_RG_U3_VUSB10_ON;
-		writel(tmp, port_base + U3P_U3_PHYA_REG0);
-	}
-
 	/* (force_suspendm=0) (let suspendm=1, enable usb 480MHz pll) */
 	tmp = readl(port_base + U3P_U2PHYDTM0);
 	tmp &= ~(P2C_FORCE_SUSPENDM | P2C_RG_XCVRSEL);
@@ -351,10 +358,6 @@ static void phy_instance_power_on(struct mt65xx_u3phy *u3phy,
 	writel(tmp, port_base + U3P_USBPHYACR6);
 
 	if (!index) {
-		tmp = readl(u3phy->sif_base + U3P_XTALCTL3);
-		tmp |= XC3_RG_U3_XTAL_RX_PWD | XC3_RG_U3_FRC_XTAL_RX_PWD;
-		writel(tmp, u3phy->sif_base + U3P_XTALCTL3);
-
 		/* switch 100uA current to SSUSB */
 		tmp = readl(port_base + U3P_USBPHYACR5);
 		tmp |= PA5_RG_U2_HS_100U_U3_EN;
@@ -366,12 +369,6 @@ static void phy_instance_power_on(struct mt65xx_u3phy *u3phy,
 	tmp &= ~P2C_RG_SESSEND;
 	writel(tmp, port_base + U3P_U2PHYDTM1);
 
-	/* USB 2.0 slew rate calibration */
-	tmp = readl(port_base + U3P_USBPHYACR5);
-	tmp &= ~PA5_RG_U2_HSTX_SRCTRL;
-	tmp |= PA5_RG_U2_HSTX_SRCTRL_VAL(4);
-	writel(tmp, port_base + U3P_USBPHYACR5);
-
 	if (u3phy->pdata->avoid_rx_sen_degradation && index) {
 		tmp = readl(port_base + U3D_U2PHYDCR0);
 		tmp |= P2C_RG_SIF_U2PLL_FORCE_ON;
@@ -419,12 +416,6 @@ static void phy_instance_power_off(struct mt65xx_u3phy *u3phy,
 	tmp |= P2C_RG_SESSEND;
 	writel(tmp, port_base + U3P_U2PHYDTM1);
 
-	if (!index) {
-		tmp = readl(port_base + U3P_U3_PHYA_REG0);
-		tmp &= ~P3A_RG_U3_VUSB10_ON;
-		writel(tmp, port_base + U3P_U3_PHYA_REG0);
-	}
-
 	if (u3phy->pdata->avoid_rx_sen_degradation && index) {
 		tmp = readl(port_base + U3D_U2PHYDCR0);
 		tmp &= ~P2C_RG_SIF_U2PLL_FORCE_ON;
@@ -464,7 +455,11 @@ static int mt65xx_phy_init(struct phy *phy)
 		return ret;
 	}
 
-	phy_instance_init(u3phy, instance);
+	if (instance->type == PHY_TYPE_USB2)
+		phy_instance_init(u3phy, instance);
+	else
+		u3_phy_instance_init(u3phy, instance);
+
 	return 0;
 }
 
@@ -473,8 +468,10 @@ static int mt65xx_phy_power_on(struct phy *phy)
 	struct mt65xx_phy_instance *instance = phy_get_drvdata(phy);
 	struct mt65xx_u3phy *u3phy = dev_get_drvdata(phy->dev.parent);
 
-	phy_instance_power_on(u3phy, instance);
-	hs_slew_rate_calibrate(u3phy, instance);
+	if (instance->type == PHY_TYPE_USB2) {
+		phy_instance_power_on(u3phy, instance);
+		hs_slew_rate_calibrate(u3phy, instance);
+	}
 	return 0;
 }
 
@@ -483,7 +480,9 @@ static int mt65xx_phy_power_off(struct phy *phy)
 	struct mt65xx_phy_instance *instance = phy_get_drvdata(phy);
 	struct mt65xx_u3phy *u3phy = dev_get_drvdata(phy->dev.parent);
 
-	phy_instance_power_off(u3phy, instance);
+	if (instance->type == PHY_TYPE_USB2)
+		phy_instance_power_off(u3phy, instance);
+
 	return 0;
 }
 
@@ -492,7 +491,9 @@ static int mt65xx_phy_exit(struct phy *phy)
 	struct mt65xx_phy_instance *instance = phy_get_drvdata(phy);
 	struct mt65xx_u3phy *u3phy = dev_get_drvdata(phy->dev.parent);
 
-	phy_instance_exit(u3phy, instance);
+	if (instance->type == PHY_TYPE_USB2)
+		phy_instance_exit(u3phy, instance);
+
 	clk_disable_unprepare(u3phy->u3phya_ref);
 	return 0;
 }
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v5 2/9] phy: phy-mt65xx-usb3: increase LFPS filter threshold
From: Chunfeng Yun @ 2017-03-31  7:35 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Matthias Brugger, Felipe Balbi, Rob Herring, Mark Rutland,
	Ian Campbell, Chunfeng Yun, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1490945735-9531-1-git-send-email-chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

Increase LFPS filter threshold to avoid some fake remote wakeup
signal which cause U3 link fail and link to U2 only at about
0.01% probability.

Signed-off-by: Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/phy/phy-mt65xx-usb3.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/phy/phy-mt65xx-usb3.c b/drivers/phy/phy-mt65xx-usb3.c
index fe2392a..4fd47d0 100644
--- a/drivers/phy/phy-mt65xx-usb3.c
+++ b/drivers/phy/phy-mt65xx-usb3.c
@@ -106,6 +106,10 @@
 #define P3A_RG_XTAL_EXT_EN_U3		GENMASK(11, 10)
 #define P3A_RG_XTAL_EXT_EN_U3_VAL(x)	((0x3 & (x)) << 10)
 
+#define U3P_U3_PHYD_LFPS1		(SSUSB_SIFSLV_U3PHYD_BASE + 0x000c)
+#define P3D_RG_FWAKE_TH		GENMASK(21, 16)
+#define P3D_RG_FWAKE_TH_VAL(x)	((0x3f & (x)) << 16)
+
 #define U3P_PHYD_CDR1		(SSUSB_SIFSLV_U3PHYD_BASE + 0x005c)
 #define P3D_RG_CDR_BIR_LTD1		GENMASK(28, 24)
 #define P3D_RG_CDR_BIR_LTD1_VAL(x)	((0x1f & (x)) << 24)
@@ -303,6 +307,11 @@ static void phy_instance_init(struct mt65xx_u3phy *u3phy,
 	tmp |= P3D_RG_CDR_BIR_LTD0_VAL(0xc) | P3D_RG_CDR_BIR_LTD1_VAL(0x3);
 	writel(tmp, port_base + U3P_PHYD_CDR1);
 
+	tmp = readl(port_base + U3P_U3_PHYD_LFPS1);
+	tmp &= ~P3D_RG_FWAKE_TH;
+	tmp |= P3D_RG_FWAKE_TH_VAL(0x34);
+	writel(tmp, port_base + U3P_U3_PHYD_LFPS1);
+
 	tmp = readl(port_base + U3P_U3_PHYD_RXDET1);
 	tmp &= ~P3D_RG_RXDET_STB2_SET;
 	tmp |= P3D_RG_RXDET_STB2_SET_VAL(0x10);
-- 
1.7.9.5

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

* [PATCH v5 1/9] phy: phy-mt65xx-usb3: improve RX detection stable time
From: Chunfeng Yun @ 2017-03-31  7:35 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Matthias Brugger, Felipe Balbi, Rob Herring, Mark Rutland,
	Ian Campbell, Chunfeng Yun, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA

The default value of RX detection stable time is 10us, and this
margin is too big for some critical cases which cause U3 link fail
and link to U2(probability is about 1%). So change it to 5us.

Signed-off-by: Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/phy/phy-mt65xx-usb3.c |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/phy/phy-mt65xx-usb3.c b/drivers/phy/phy-mt65xx-usb3.c
index d972067..fe2392a 100644
--- a/drivers/phy/phy-mt65xx-usb3.c
+++ b/drivers/phy/phy-mt65xx-usb3.c
@@ -112,6 +112,14 @@
 #define P3D_RG_CDR_BIR_LTD0		GENMASK(12, 8)
 #define P3D_RG_CDR_BIR_LTD0_VAL(x)	((0x1f & (x)) << 8)
 
+#define U3P_U3_PHYD_RXDET1		(SSUSB_SIFSLV_U3PHYD_BASE + 0x128)
+#define P3D_RG_RXDET_STB2_SET		GENMASK(17, 9)
+#define P3D_RG_RXDET_STB2_SET_VAL(x)	((0x1ff & (x)) << 9)
+
+#define U3P_U3_PHYD_RXDET2		(SSUSB_SIFSLV_U3PHYD_BASE + 0x12c)
+#define P3D_RG_RXDET_STB2_SET_P3	GENMASK(8, 0)
+#define P3D_RG_RXDET_STB2_SET_P3_VAL(x)	(0x1ff & (x))
+
 #define U3P_XTALCTL3		(SSUSB_SIFSLV_SPLLC + 0x0018)
 #define XC3_RG_U3_XTAL_RX_PWD		BIT(9)
 #define XC3_RG_U3_FRC_XTAL_RX_PWD	BIT(8)
@@ -295,6 +303,16 @@ static void phy_instance_init(struct mt65xx_u3phy *u3phy,
 	tmp |= P3D_RG_CDR_BIR_LTD0_VAL(0xc) | P3D_RG_CDR_BIR_LTD1_VAL(0x3);
 	writel(tmp, port_base + U3P_PHYD_CDR1);
 
+	tmp = readl(port_base + U3P_U3_PHYD_RXDET1);
+	tmp &= ~P3D_RG_RXDET_STB2_SET;
+	tmp |= P3D_RG_RXDET_STB2_SET_VAL(0x10);
+	writel(tmp, port_base + U3P_U3_PHYD_RXDET1);
+
+	tmp = readl(port_base + U3P_U3_PHYD_RXDET2);
+	tmp &= ~P3D_RG_RXDET_STB2_SET_P3;
+	tmp |= P3D_RG_RXDET_STB2_SET_P3_VAL(0x10);
+	writel(tmp, port_base + U3P_U3_PHYD_RXDET2);
+
 	dev_dbg(u3phy->dev, "%s(%d)\n", __func__, index);
 }
 
-- 
1.7.9.5

--
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 related

* Re: [PATCH v6 4/5] i2c: aspeed: added driver for Aspeed I2C
From: Benjamin Herrenschmidt @ 2017-03-31  7:33 UTC (permalink / raw)
  To: Brendan Higgins, wsa-z923LK4zBo2bacvFa/9K2g,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	tglx-hfZtesqFncYOwBW4kG4KsQ, jason-NLaQJdtUoK4Be96aLqz0jA,
	marc.zyngier-5wv7dgnIgG8, joel-U3u1mxZcP9KHXe+LvDLADg,
	vz-ChpfBGZJDbMAvxtiuMwx3w, mouse-Pma6HLj0uuo, clg-Bxea+6Xhats
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <20170328051226.21677-5-brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

Allright, I finally found some time for reviewing some of this
after splitting the ftgmac100 patch into 54 smaller ones :)

On Mon, 2017-03-27 at 22:12 -0700, Brendan Higgins wrote:

 .../...

> +struct aspeed_i2c_bus {
> +	struct i2c_adapter		adap;
> +	struct device			*dev;
> +	void __iomem			*base;
> +	/* Synchronizes I/O mem access to base. */
> +	spinlock_t			lock;

I am not entirely convinced we need that lock. The i2c core will
take a mutex protecting all operations on the bus. So we only need
to synchronize between our "xfer" code and our interrupt handler.

This probably be done without a lock if we are careful. Not a huge
deal though as Aspeed SoC are currently not SMP so the lock compiles
down to not much unless you have all the debug crap enabled :-)

> +	struct completion		cmd_complete;
> +	int				irq;
> +	/* Transaction state. */
> +	enum aspeed_i2c_master_state	master_state;
> +	struct i2c_msg			*msgs;
> +	size_t				buf_index;
> +	size_t				msgs_index;
> +	size_t				msgs_size;
> +	bool				send_stop;
> +	int				cmd_err;
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> +	struct i2c_client		*slave;
> +	enum aspeed_i2c_slave_state	slave_state;
> +#endif
> +};

Minor nit but the above should probably be in the slave patch no ?

> +static inline void aspeed_i2c_write(struct aspeed_i2c_bus *bus, u32
> val,
> +				    u32 reg)
> +{
> +	writel(val, bus->base + reg);
> +}
> +
> +static inline u32 aspeed_i2c_read(struct aspeed_i2c_bus *bus, u32
> reg)
> +{
> +	return readl(bus->base + reg);
> +}

Another very minor nit, I'm not certain those accessors are a big
win in code size and/or readability but keep them if you want.

> +static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
> +{
> +	unsigned long time_left, flags;
> +	int ret = 0;
> +	u32 command;
> +
> +	spin_lock_irqsave(&bus->lock, flags);
> +	command = aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG);
> +
> +	if (command & ASPEED_I2CD_SDA_LINE_STS) {
> +		/* Bus is idle: no recovery needed. */
> +		if (command & ASPEED_I2CD_SCL_LINE_STS)
> +			goto out;
> +		dev_dbg(bus->dev, "bus hung (state %x), attempting recovery\n",
> +			command);
> +
> +		aspeed_i2c_write(bus, ASPEED_I2CD_M_STOP_CMD,
> +				 ASPEED_I2C_CMD_REG);
> +		reinit_completion(&bus->cmd_complete);
> +		spin_unlock_irqrestore(&bus->lock, flags);

See my comment further down in master_xfer, do the reinit before sending
the command, even if currently the lock protects you, it's cleaner.

Now, I don't completely get how your interrupt handler deals with these
"message-less" completions. See the review of the interrupt handler.

> +
> +		time_left = wait_for_completion_timeout(
> +				&bus->cmd_complete, bus->adap.timeout);
> +
> +		spin_lock_irqsave(&bus->lock, flags);
> +		if (time_left == 0)
> +			ret = -ETIMEDOUT;
> +		else if (bus->cmd_err)
> +			ret = -EIO;
> +	/* Bus error. */
> +	} else {
> +		dev_dbg(bus->dev, "bus hung (state %x), attempting recovery\n",
> +			command);
> +
> +		aspeed_i2c_write(bus, ASPEED_I2CD_BUS_RECOVER_CMD,
> +				 ASPEED_I2C_CMD_REG);
> +		reinit_completion(&bus->cmd_complete);

Same comments as above.

> +		spin_unlock_irqrestore(&bus->lock, flags);
> +
> +		time_left = wait_for_completion_timeout(
> +				&bus->cmd_complete, bus->adap.timeout);
> +
> +		spin_lock_irqsave(&bus->lock, flags);
> +		if (time_left == 0)
> +			ret = -ETIMEDOUT;
> +		else if (bus->cmd_err)
> +			ret = -EIO;
> +		/* Recovery failed. */
> +		else if (!(aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG) &
> +			   ASPEED_I2CD_SDA_LINE_STS))
> +			ret = -EIO;
> +	}

Some of those error states probably also warrant a reset of the controller,
I think aspeed does that in the SDK.

> +out:
> +	spin_unlock_irqrestore(&bus->lock, flags);
> +
> +	return ret;
> +}
> +
> +static void do_start(struct aspeed_i2c_bus *bus)
> +{
> +	u32 command = ASPEED_I2CD_M_START_CMD | ASPEED_I2CD_M_TX_CMD;
> +	struct i2c_msg *msg = &bus->msgs[bus->msgs_index];
> +	u8 slave_addr = msg->addr << 1;
> +
> +	bus->master_state = ASPEED_I2C_MASTER_START;
> +	bus->buf_index = 0;
> +
> +	if (msg->flags & I2C_M_RD) {
> +		slave_addr |= 1;
> +		command |= ASPEED_I2CD_M_RX_CMD;
> +		/* Need to let the hardware know to NACK after RX. */
> +		if (msg->len == 1 && !(msg->flags & I2C_M_RECV_LEN))
> +			command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
> +	}


What about I2C_M_NOSTART ? 

Not that I've ever seen it used... ;-)

> +	aspeed_i2c_write(bus, slave_addr, ASPEED_I2C_BYTE_BUF_REG);
> +	aspeed_i2c_write(bus, command, ASPEED_I2C_CMD_REG);
> +}
> +
> +static void do_stop(struct aspeed_i2c_bus *bus)
> +{
> +	bus->master_state = ASPEED_I2C_MASTER_STOP;
> +	aspeed_i2c_write(bus, ASPEED_I2CD_M_STOP_CMD,
> +			 ASPEED_I2C_CMD_REG);
> +}
> +
> +static void aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
> +{
> +	struct i2c_msg *msg = &bus->msgs[bus->msgs_index];
> +	u32 irq_status, status_ack = 0, command = 0;
> +	u8 recv_byte;

If your lock means anything you should probably capture bus->msgs[..]
with the lock held. That said, see my previous comment about the
lock possibly not being terribly useful.

Additionally, if you are doing a bus recovery, won't you be messing
around with a stale or NULL bus->msgs ?

I would at the very least make it

	msg = bus->msgs ? &bus->msgs[bus->msgs_index] : NULL;

That way msg is NULL in the recovery case rather than a random
crap pointer.

> +	spin_lock(&bus->lock);
> +	irq_status = aspeed_i2c_read(bus, ASPEED_I2C_INTR_STS_REG);
>

I would "ack" (write back to INTR_STS_REG) immediately. Otherwise
you have a race between status bits set as a result of what happened
before the interrupt handler vs. as a result of what you did.

For example, take TX. You get the TX bit in irq_status. You start
a new character transmission bcs there's more to send *then* you ack
the TX bit. That's racy. If that new transmission is fast enough,
you'll end up acking the wrong one. Again this is extremely unlikely
but code should be written in a way that is completely fool proof
from such races. They can happen for stupid reasons, such as huge
bus delays caused by a peripheral, FIQ going bonkers etc...

In general, you always ACK all interrupts first. Then you handle
the bits you have harvested.

> +	if (irq_status & ASPEED_I2CD_INTR_ERROR ||
> +	    (!bus->msgs && bus->master_state != ASPEED_I2C_MASTER_STOP)) {

What happen with recovery completion here ?

Won't we hit !bus->msgs && master state != stop ? Especially
if we hit a timeout where we haven't cleaned up any of our state.

> +		dev_dbg(bus->dev, "received error interrupt: 0x%08x",
> +			irq_status);

This is confusing too in the case of master_state != stop ... any
interrupt will trigger that. I think it would be worthwhile either
commenting a bit more here or having clearer messages depending
on the condition.

> +		bus->cmd_err = -EIO;
> +		do_stop(bus);
> +		goto out_no_complete;
> +	}
> +
> +	if (irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE) {
> +		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
> +		status_ack |= ASPEED_I2CD_INTR_BUS_RECOVER_DONE;
> +		goto out_complete;
> +	}

I would set master_state to "RECOVERY" (new state ?) and ensure
those things are caught if they happen outside of a recovery.

> +	if (bus->master_state == ASPEED_I2C_MASTER_START) {

Here a comment would be handy as to why you do this before the
switch/case. I understand why but it makes reading the code by
somebody else easier.

> +		if (!(irq_status & ASPEED_I2CD_INTR_TX_ACK)) {

Minor nit:

		if (unlikely(error case)) {
			...
			goto out;
		}
		...

Ie, you don't need the "else", and you make it clear that this
is an error case, allowing the compiler to potentially optimize
the likely branch.

In fact, I would have that on all the error cases above too.

I understand now why you have that 'status_ack'. You are trying
to catch the bits that may be set that shouldn't be.

I think you should still "ack early". However, you could have
status_ack called status_handled or something like that and
at the end, still catch "spurrious" bits.

That said, I notice a lot of duplication in your state machine.

You basically have each state starting with

	if (didn't get the bit I wanted) {
		error
	}

You are also not very consistent as to whether you generate a
stop as a result or not.

I would happily simplify that state machine by just completing
with an error and letting master_xfer() do a stop when done but
if you like to keep it the way it is, you could have a common
goto label that handle error + stop.

> +			dev_dbg(bus->dev,
> +				"no slave present at %02x", msg->addr);
> +			status_ack |= ASPEED_I2CD_INTR_TX_NAK;
> +			bus->cmd_err = -EIO;
> +			do_stop(bus);
> +			goto out_no_complete;
> +		} else {
> +			status_ack |= ASPEED_I2CD_INTR_TX_ACK;
> +			if (msg->flags & I2C_M_RD)
> +				bus->master_state = ASPEED_I2C_MASTER_RX;
> +			else
> +				bus->master_state = ASPEED_I2C_MASTER_TX_FIRST;

What about the SMBUS_QUICK case ? (0-len transfer). Do we need
to handle this here ? A quick look at the TX_FIRST case makes
me think we are ok there but I'm not sure about the RX case.

I'm not sure the RX case is tight also. What completion does the
HW give you for the address cycle ? Won't you get that before it
has received the first character ? IE. You fall through to
the read case of the state machine with the read potentially
not complete yet no ?

> +		}
> +	}
> +
> +	switch (bus->master_state) {
> +	case ASPEED_I2C_MASTER_TX:
> +		if (irq_status & ASPEED_I2CD_INTR_TX_NAK) {
> +			dev_dbg(bus->dev, "slave NACKed TX");
> +			status_ack |= ASPEED_I2CD_INTR_TX_NAK;
> +			bus->cmd_err = -EIO;
> +			do_stop(bus);
> +			goto out_no_complete;

As I said earlier, I would factor all the error cases. I would also
not worry too much about checking that the status bits meet expectation
in the error path.

> +		} else if (!(irq_status & ASPEED_I2CD_INTR_TX_ACK))
> {
> +			dev_err(bus->dev, "slave failed to ACK TX");
> +			goto out_complete;

You should still stop.

> +		}
> +		status_ack |= ASPEED_I2CD_INTR_TX_ACK;
> +		/* fallthrough intended */
> +	case ASPEED_I2C_MASTER_TX_FIRST:
> +		if (bus->buf_index < msg->len) {
> +			bus->master_state = ASPEED_I2C_MASTER_TX;
> +			aspeed_i2c_write(bus, msg->buf[bus->buf_index++],
> +					 ASPEED_I2C_BYTE_BUF_REG);
> +			aspeed_i2c_write(bus, ASPEED_I2CD_M_TX_CMD,
> +					 ASPEED_I2C_CMD_REG);
> +		} else if (bus->msgs_index + 1 < bus->msgs_size) {
> +			bus->msgs_index++;
> +			do_start(bus);
> +		} else {
> +			do_stop(bus);
> +		}
> +		goto out_no_complete;
> +	case ASPEED_I2C_MASTER_RX:
> +		if (!(irq_status & ASPEED_I2CD_INTR_RX_DONE)) {
> +			dev_err(bus->dev, "master failed to RX");
> +			goto out_complete;
> +		}

See my comment above for a bog standard i2c_read. Aren't you getting
the completion for the address before the read is even started ?

> +		status_ack |= ASPEED_I2CD_INTR_RX_DONE;
> +
> +		recv_byte = aspeed_i2c_read(bus, ASPEED_I2C_BYTE_BUF_REG) >> 8;
> +		msg->buf[bus->buf_index++] = recv_byte;
> +
> +		if (msg->flags & I2C_M_RECV_LEN &&
> +		    recv_byte <= I2C_SMBUS_BLOCK_MAX) {
> +			msg->len = recv_byte +
> +					((msg->flags & I2C_CLIENT_PEC) ? 2 : 1);
> +			msg->flags &= ~I2C_M_RECV_LEN;
> +		}

You need to error out with -EPROTO if the size is too large.

> +
> +		if (bus->buf_index < msg->len) {
> +			bus->master_state = ASPEED_I2C_MASTER_RX;
> +			command = ASPEED_I2CD_M_RX_CMD;
> +			if (bus->buf_index + 1 == msg->len)
> +				command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
> +			aspeed_i2c_write(bus, command, ASPEED_I2C_CMD_REG);
> +		} else if (bus->msgs_index + 1 < bus->msgs_size) {
> +			bus->msgs_index++;
> +			do_start(bus);
> +		} else {
> +			do_stop(bus);
> +		}

You have some duplication. You could have your "completed message,
switch to the next one" be either a helper or another goto statement.

I would do a little helper that check the index and calls stop or
start.

> +		goto out_no_complete;
> +	case ASPEED_I2C_MASTER_STOP:
> +		if (!(irq_status & ASPEED_I2CD_INTR_NORMAL_STOP)) {
> +			dev_err(bus->dev, "master failed to STOP");
> +			bus->cmd_err = -EIO;
> +		}
> +		status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP;
> +
> +		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
> +		goto out_complete;
> +	case ASPEED_I2C_MASTER_INACTIVE:
> +		dev_err(bus->dev,
> +			"master received interrupt 0x%08x, but is inactive",
> +			irq_status);
> +		bus->cmd_err = -EIO;
> +		goto out_complete;
> +	default:
> +		WARN(1, "unknown master state\n");
> +		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
> +		bus->cmd_err = -EIO;
> +		goto out_complete;
> +	}
> +
> +out_complete:
> +	complete(&bus->cmd_complete);
> +out_no_complete:
> +	if (irq_status != status_ack)
> +		dev_err(bus->dev,
> +			"irq handled != irq. expected 0x%08x, but was 0x%08x\n",
> +			irq_status, status_ack);
> +	aspeed_i2c_write(bus, irq_status, ASPEED_I2C_INTR_STS_REG);
> +	spin_unlock(&bus->lock);
> +}
> +
> +static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
> +{
> +	struct aspeed_i2c_bus *bus = dev_id;
> +
> +	aspeed_i2c_master_irq(bus);
> +	return IRQ_HANDLED;
> +}

In theory you want to only return IRQ_HANDLED if you indeed has at
least one IRQ status bit set... Not a huge deal here but it would
be cleaner.

> +static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
> +				  struct i2c_msg *msgs, int num)
> +{
> +	struct aspeed_i2c_bus *bus = adap->algo_data;
> +	unsigned long time_left, flags;
> +	int ret = 0;
> +
> +	bus->cmd_err = 0;
> +
> +	/* If bus is busy, attempt recovery. We assume a single master
> +	 * environment.
> +	 */
> +	if (aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG) &
> +	    ASPEED_I2CD_BUS_BUSY_STS) {
> +		ret = aspeed_i2c_recover_bus(bus);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	spin_lock_irqsave(&bus->lock, flags);

See previous comment about the lock.

I would also cleanup all the interrupts before we even start a transfer
(ie write all 1's to the interrupt status reg).

> +	bus->msgs = msgs;
> +	bus->msgs_index = 0;
> +	bus->msgs_size = num;

Minor nit: msgs_count rather than size ?

> +	do_start(bus);
> +	reinit_completion(&bus->cmd_complete);

The reinit_completion call should probably be before do_start.

Currently the spinlock avoids this being a real issue but if as I
suggest you take out the lock, then it will be racy (probably
impossible to hit in practice but still .. :-)
 
> +	spin_unlock_irqrestore(&bus->lock, flags);
> +
> +	time_left = wait_for_completion_timeout(&bus->cmd_complete,
> +						bus->adap.timeout);
> +
> +	spin_lock_irqsave(&bus->lock, flags);
> +	bus->msgs = NULL;
> +	if (time_left == 0)
> +		ret = -ETIMEDOUT;
> +	else
> +		ret = bus->cmd_err;

If we timed out we may want to sanitize the HW state. I would suggest
resetting the master. We should also sanitize master_state. I would
suggest adding a reset function that cleans everything up.

> +	spin_unlock_irqrestore(&bus->lock, flags);
> +
> +	/* If nothing went wrong, return number of messages transferred. */
> +	if (ret >= 0)
> +		return bus->msgs_index + 1;
> +	else
> +		return ret;
> +}
> +
> +static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
> +{
> +	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
> +}
> +
> +static const struct i2c_algorithm aspeed_i2c_algo = {
> +	.master_xfer	= aspeed_i2c_master_xfer,
> +	.functionality	= aspeed_i2c_functionality,
> +};
> +
> +static u32 aspeed_i2c_get_clk_reg_val(u32 divisor)
> +{
> +	u32 base_clk, clk_high, clk_low, tmp;
> +
> +	/*
> +	 * The actual clock frequency of SCL is:
> +	 *	SCL_freq = base_freq * (SCL_high + SCL_low)
> +	 *		 = APB_freq / divisor
> +	 * where base_freq is a programmable clock divider; its value is
> +	 *	base_freq = 1 << base_clk
> +	 * SCL_high is the number of base_freq clock cycles that SCL stays high
> +	 * and SCL_low is the number of base_freq clock cycles that SCL stays
> +	 * low for a period of SCL.
> +	 * The actual register has a minimum SCL_high and SCL_low minimum of 1;
> +	 * thus, they start counting at zero. So
> +	 *	SCL_high = clk_high + 1
> +	 *	SCL_low	 = clk_low + 1
> +	 * Thus,
> +	 *	SCL_freq = (1 << base_clk) * (clk_high + 1 + clk_low + 1)
> +	 * The documentation recommends clk_high >= 8 and clk_low >= 7 when
> +	 * possible; this last constraint gives us the following solution:
> +	 */
> +	base_clk = divisor > 32 ? ilog2(divisor / 16 - 1) : 0;
> +	tmp = divisor / (1 << base_clk);
> +	clk_high = tmp / 2 + tmp % 2;
> +	clk_low = tmp - clk_high;
> +
> +	clk_high -= 1;
> +	clk_low -= 1;
> +
> +	return ((clk_high << ASPEED_I2CD_TIME_SCL_HIGH_SHIFT)
> +		& ASPEED_I2CD_TIME_SCL_HIGH_MASK)
> +			| ((clk_low << ASPEED_I2CD_TIME_SCL_LOW_SHIFT)
> +			   & ASPEED_I2CD_TIME_SCL_LOW_MASK)
> +			| (base_clk & ASPEED_I2CD_TIME_BASE_DIVISOR_MASK);
> +}

As I think I mentioned earlier, the AST2500 has a slightly different
register layout which support larger values for high and low, thus
allowing a finer granularity.

BTW. In case you haven't, I would suggest you copy/paste the above in
a userspace app and run it for all frequency divisors and see if your
results match the aspeed table :)

> +static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus,
> +			       struct platform_device *pdev)
> +{
> +	u32 clk_freq, divisor;
> +	struct clk *pclk;
> +	int ret;
> +
> +	pclk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(pclk)) {
> +		dev_err(&pdev->dev, "clk_get failed\n");
> +		return PTR_ERR(pclk);
> +	}
> +	ret = of_property_read_u32(pdev->dev.of_node,
> +				   "clock-frequency", &clk_freq);

See my previous comment about calling that 'bus-frequency' rather
than 'clock-frequency'.

> +	if (ret < 0) {
> +		dev_err(&pdev->dev,
> +			"Could not read clock-frequency property\n");
> +		clk_freq = 100000;
> +	}
> +	divisor = clk_get_rate(pclk) / clk_freq;
> +	/* We just need the clock rate, we don't actually use the clk object. */
> +	devm_clk_put(&pdev->dev, pclk);
> +
> +	/* Set AC Timing */
> +	if (clk_freq / 1000 > 1000) {
> +		aspeed_i2c_write(bus, aspeed_i2c_read(bus,
> +						      ASPEED_I2C_FUN_CTRL_REG) |
> +				ASPEED_I2CD_M_HIGH_SPEED_EN |
> +				ASPEED_I2CD_M_SDA_DRIVE_1T_EN |
> +				ASPEED_I2CD_SDA_DRIVE_1T_EN,
> +				ASPEED_I2C_FUN_CTRL_REG);
> +
> +		aspeed_i2c_write(bus, 0x3, ASPEED_I2C_AC_TIMING_REG2);
> +		aspeed_i2c_write(bus, aspeed_i2c_get_clk_reg_val(divisor),
> +				 ASPEED_I2C_AC_TIMING_REG1);

I already discussed by doubts about the above. I can try to scope
it with the EVB if you don't get to it. For now I'd rather take the
code out.

We should ask aspeed from what frequency the "1T" stuff is useful.

> +	} else {
> +		aspeed_i2c_write(bus, aspeed_i2c_get_clk_reg_val(divisor),
> +				 ASPEED_I2C_AC_TIMING_REG1);
> +		aspeed_i2c_write(bus, ASPEED_NO_TIMEOUT_CTRL,
> +				 ASPEED_I2C_AC_TIMING_REG2);
> +	}
> +
> +	return 0;
> +}
> +
> +static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> +{
> +	struct aspeed_i2c_bus *bus;
> +	struct resource *res;
> +	int ret;
> +
> +	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
> +	if (!bus)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	bus->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(bus->base)) {
> +		dev_err(&pdev->dev, "failed to devm_ioremap_resource\n");
> +		return PTR_ERR(bus->base);
> +	}
> +
> +	bus->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> +	ret = devm_request_irq(&pdev->dev, bus->irq, aspeed_i2c_bus_irq,
> +			       IRQF_SHARED, dev_name(&pdev->dev), bus);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to request interrupt\n");
> +		return ret;
> +	}

Again, out of paranoia, make sure the HW is reset and interrupt
off *before* you register the interrupt handler, or a HW left in
a funny state (by uboot for example) might shoot interrupts before
you are ready to take them. I would move the reset you do below
to before devm_request_irq.

> +	/* Initialize the I2C adapter */
> +	spin_lock_init(&bus->lock);
> +	init_completion(&bus->cmd_complete);
> +	bus->adap.owner = THIS_MODULE;
> +	bus->adap.retries = 0;
> +	bus->adap.timeout = 5 * HZ;
> +	bus->adap.algo = &aspeed_i2c_algo;
> +	bus->adap.algo_data = bus;
> +	bus->adap.dev.parent = &pdev->dev;
> +	bus->adap.dev.of_node = pdev->dev.of_node;
> +	snprintf(bus->adap.name, sizeof(bus->adap.name), "Aspeed i2c");

Another trivial one, should we put some kind of bus number
in that string ?

> +	bus->dev = &pdev->dev;
> +
> +	/* reset device: disable master & slave functions */
> +	aspeed_i2c_write(bus, 0, ASPEED_I2C_FUN_CTRL_REG);
> +
> +	ret = aspeed_i2c_init_clk(bus, pdev);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Enable Master Mode */
> +	aspeed_i2c_write(bus, aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG) |
> +		      ASPEED_I2CD_MASTER_EN |
> +		      ASPEED_I2CD_MULTI_MASTER_DIS, ASPEED_I2C_FUN_CTRL_REG);
> +
> +	/* Set interrupt generation of I2C controller */
> +	aspeed_i2c_write(bus, ASPEED_I2CD_INTR_ALL, ASPEED_I2C_INTR_CTRL_REG);
> +
> +	ret = i2c_add_adapter(&bus->adap);
> +	if (ret < 0)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, bus);
> +
> +	dev_info(bus->dev, "i2c bus %d registered, irq %d\n",
> +		 bus->adap.nr, bus->irq);
> +
> +	return 0;
> +}
> +
> +static int aspeed_i2c_remove_bus(struct platform_device *pdev)
> +{
> +	struct aspeed_i2c_bus *bus = platform_get_drvdata(pdev);
> +
> +	i2c_del_adapter(&bus->adap);

Out of paranoia, should we turn off the function and mask the
interrupts here just in case ?

> +	return 0;
> +}
> +
> +static const struct of_device_id aspeed_i2c_bus_of_table[] = {
> +	{ .compatible = "aspeed,ast2400-i2c-bus", },
> +	{ .compatible = "aspeed,ast2500-i2c-bus", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, aspeed_i2c_bus_of_table);
> +
> +static struct platform_driver aspeed_i2c_bus_driver = {
> +	.probe		= aspeed_i2c_probe_bus,
> +	.remove		= aspeed_i2c_remove_bus,
> +	.driver		= {
> +		.name		= "ast-i2c-bus",
> +		.of_match_table	= aspeed_i2c_bus_of_table,
> +	},
> +};
> +module_platform_driver(aspeed_i2c_bus_driver);
> +
> +MODULE_AUTHOR("Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>");
> +MODULE_DESCRIPTION("Aspeed I2C Bus Driver");
> +MODULE_LICENSE("GPL v2");
--
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

* [PATCH V3 2/2] thermal: broadcom: add Northstar thermal driver
From: Rafał Miłecki @ 2017-03-31  7:31 UTC (permalink / raw)
  To: Zhang Rui, Eduardo Valentin
  Cc: Rob Herring, Mark Rutland, Stephen Warren, Lee Jones, Eric Anholt,
	Florian Fainelli, Ray Jui, Scott Branden,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Rafał Miłecki, Jon Mason
In-Reply-To: <20170331073132.21457-1-zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

Northstar is a SoC family commonly used in home routers. This commit
adds a driver for checking CPU temperature. As Northstar Plus seems to
also have this IP block this new symbol gets ARCH_BCM_IPROC dependency.

Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
Signed-off-by: Jon Mason <jon.mason-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
V2: Make it iProc specific as NSP can also use this driver
    Select proper symbols in config ARCH_BCM_IPROC
    Define PVTMON register bits
    Update code selecting temperature monitor mode
    Thank you Jon!
V3: More details in help message for BCM_NS_THERMAL
    Use slope & offset
    Drop arch code change (I'll be submitted using a proper tree)
    Thank you Eduardo!
---
 drivers/thermal/Kconfig               |   5 ++
 drivers/thermal/Makefile              |   1 +
 drivers/thermal/broadcom/Kconfig      |   8 +++
 drivers/thermal/broadcom/Makefile     |   1 +
 drivers/thermal/broadcom/ns-thermal.c | 101 ++++++++++++++++++++++++++++++++++
 5 files changed, 116 insertions(+)
 create mode 100644 drivers/thermal/broadcom/Kconfig
 create mode 100644 drivers/thermal/broadcom/Makefile
 create mode 100644 drivers/thermal/broadcom/ns-thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 776b34396144..008e173ec825 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -392,6 +392,11 @@ config MTK_THERMAL
 	  Enable this option if you want to have support for thermal management
 	  controller present in Mediatek SoCs
 
+menu "Broadcom thermal drivers"
+depends on ARCH_BCM || COMPILE_TEST
+source "drivers/thermal/broadcom/Kconfig"
+endmenu
+
 menu "Texas Instruments thermal drivers"
 depends on ARCH_HAS_BANDGAP || COMPILE_TEST
 depends on HAS_IOMEM
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 7adae2029355..549d81b6363c 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -27,6 +27,7 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL)	+= clock_cooling.o
 thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o
 
 # platform thermal drivers
+obj-y				+= broadcom/
 obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM)	+= qcom-spmi-temp-alarm.o
 obj-$(CONFIG_SPEAR_THERMAL)	+= spear_thermal.o
 obj-$(CONFIG_ROCKCHIP_THERMAL)	+= rockchip_thermal.o
diff --git a/drivers/thermal/broadcom/Kconfig b/drivers/thermal/broadcom/Kconfig
new file mode 100644
index 000000000000..f0dea8a8e002
--- /dev/null
+++ b/drivers/thermal/broadcom/Kconfig
@@ -0,0 +1,8 @@
+config BCM_NS_THERMAL
+	tristate "Northstar thermal driver"
+	depends on ARCH_BCM_IPROC || COMPILE_TEST
+	help
+	  Northstar is a family of SoCs that includes e.g. BCM4708, BCM47081,
+	  BCM4709 and BCM47094. It contains DMU (Device Management Unit) block
+	  with a thermal sensor that allows checking CPU temperature. This
+	  driver provides support for it.
diff --git a/drivers/thermal/broadcom/Makefile b/drivers/thermal/broadcom/Makefile
new file mode 100644
index 000000000000..059df9a0ed69
--- /dev/null
+++ b/drivers/thermal/broadcom/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_BCM_NS_THERMAL)		+= ns-thermal.o
diff --git a/drivers/thermal/broadcom/ns-thermal.c b/drivers/thermal/broadcom/ns-thermal.c
new file mode 100644
index 000000000000..245fe8c6012b
--- /dev/null
+++ b/drivers/thermal/broadcom/ns-thermal.c
@@ -0,0 +1,101 @@
+/*
+ * Copyright (C) 2017 Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/thermal.h>
+
+#define PVTMON_CONTROL0					0x00
+#define PVTMON_CONTROL0_SEL_MASK			0x0000000e
+#define PVTMON_CONTROL0_SEL_TEMP_MONITOR		0x00000000
+#define PVTMON_CONTROL0_SEL_TEST_MODE			0x0000000e
+#define PVTMON_STATUS					0x08
+
+struct ns_thermal {
+	struct thermal_zone_device *tz;
+	void __iomem *pvtmon;
+};
+
+static int ns_thermal_get_temp(void *data, int *temp)
+{
+	struct ns_thermal *ns_thermal = data;
+	int offset = thermal_zone_get_offset(ns_thermal->tz);
+	int slope = thermal_zone_get_slope(ns_thermal->tz);
+	u32 val;
+
+	val = readl(ns_thermal->pvtmon + PVTMON_CONTROL0);
+	if ((val & PVTMON_CONTROL0_SEL_MASK) != PVTMON_CONTROL0_SEL_TEMP_MONITOR) {
+		val &= ~PVTMON_CONTROL0_SEL_MASK;
+		val |= PVTMON_CONTROL0_SEL_TEMP_MONITOR;
+		writel(val, ns_thermal->pvtmon + PVTMON_CONTROL0);
+	}
+
+	val = readl(ns_thermal->pvtmon + PVTMON_STATUS);
+	*temp = slope * val + offset;
+
+	return 0;
+}
+
+const struct thermal_zone_of_device_ops ns_thermal_ops = {
+	.get_temp = ns_thermal_get_temp,
+};
+
+static int ns_thermal_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct ns_thermal *ns_thermal;
+
+	ns_thermal = devm_kzalloc(dev, sizeof(*ns_thermal), GFP_KERNEL);
+	if (!ns_thermal)
+		return -ENOMEM;
+
+	ns_thermal->pvtmon = of_iomap(dev_of_node(dev), 0);
+	if (WARN_ON(!ns_thermal->pvtmon))
+		return -ENOENT;
+
+	ns_thermal->tz = devm_thermal_zone_of_sensor_register(dev, 0,
+							      ns_thermal,
+							      &ns_thermal_ops);
+	if (IS_ERR(ns_thermal->tz)) {
+		iounmap(ns_thermal->pvtmon);
+		return PTR_ERR(ns_thermal->tz);
+	}
+
+	platform_set_drvdata(pdev, ns_thermal);
+
+	return 0;
+}
+
+static int ns_thermal_remove(struct platform_device *pdev)
+{
+	struct ns_thermal *ns_thermal = platform_get_drvdata(pdev);
+
+	iounmap(ns_thermal->pvtmon);
+
+	return 0;
+}
+
+static const struct of_device_id ns_thermal_of_match[] = {
+	{ .compatible = "brcm,ns-thermal", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ns_thermal_of_match);
+
+static struct platform_driver ns_thermal_driver = {
+	.probe		= ns_thermal_probe,
+	.remove		= ns_thermal_remove,
+	.driver = {
+		.name = "ns-thermal",
+		.of_match_table = ns_thermal_of_match,
+	},
+};
+module_platform_driver(ns_thermal_driver);
+
+MODULE_DESCRIPTION("Northstar thermal driver");
+MODULE_LICENSE("GPL v2");
-- 
2.11.0

--
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 related

* [PATCH V3 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal
From: Rafał Miłecki @ 2017-03-31  7:31 UTC (permalink / raw)
  To: Zhang Rui, Eduardo Valentin
  Cc: Rob Herring, Mark Rutland, Stephen Warren, Lee Jones, Eric Anholt,
	Florian Fainelli, Ray Jui, Scott Branden,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Rafał Miłecki
In-Reply-To: <20170323223045.15786-1-zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

This commit documents binding for thermal used in Northstar family SoCs.

Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
---
V3: Add thermal-zones to the example
    Rob: Because of this update, I didn't include Acked-by I got for V2
---
 .../devicetree/bindings/thermal/brcm,ns-thermal    | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/brcm,ns-thermal

diff --git a/Documentation/devicetree/bindings/thermal/brcm,ns-thermal b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
new file mode 100644
index 000000000000..c561c7349f17
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
@@ -0,0 +1,26 @@
+* Broadcom Northstar Thermal
+
+This binding describes thermal sensor that is part of Northstar's DMU (Device
+Management Unit).
+
+Required properties:
+- compatible : Must be "brcm,ns-thermal"
+- reg : iomem address range of PVTMON registers
+- #thermal-sensor-cells : Should be <0>
+
+Example:
+
+thermal: thermal@1800c2c0 {
+	compatible = "brcm,ns-thermal";
+	reg = <0x1800c2c0 0x10>;
+	#thermal-sensor-cells = <0>;
+};
+
+thermal-zones {
+	cpu_thermal: cpu-thermal {
+		polling-delay-passive = <0>;
+		polling-delay = <1000>;
+		coefficients = <(-556) 418000>;
+		thermal-sensors = <&thermal>;
+	};
+};
-- 
2.11.0

--
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 related

* Re: [PATCH 2/3] device property: Move fwnode graph ops to firmware specific locations
From: Mika Westerberg @ 2017-03-31  7:30 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Sakari Ailus, linux-acpi, devicetree, robh, sudeep.holla,
	lorenzo.pieralisi, rafael, mark.rutland, broonie, ahs3,
	frowand.list
In-Reply-To: <20170330151019.GL16657@valkosipuli.retiisi.org.uk>

On Thu, Mar 30, 2017 at 06:10:19PM +0300, Sakari Ailus wrote:
> Hi Mika,
> 
> Thank you for the review.
> 
> On Mon, Mar 27, 2017 at 02:52:00PM +0300, Mika Westerberg wrote:
> > On Fri, Mar 24, 2017 at 01:03:51PM +0200, Sakari Ailus wrote:
> > >  const struct fwnode_operations acpi_fwnode_ops = {
> > >  	.property_present = acpi_fwnode_property_present,
> > >  	.property_read_int_array = acpi_fwnode_property_read_int_array,
> > > @@ -1193,4 +1248,9 @@ const struct fwnode_operations acpi_fwnode_ops = {
> > >  	.get_parent = acpi_fwnode_get_parent,
> > >  	.get_next_child_node = acpi_get_next_subnode,
> > >  	.get_named_child_node = acpi_fwnode_get_named_child_node,
> > > +	.graph_get_next_endpoint = acpi_fwnode_graph_get_next_endpoint,
> > > +	.graph_get_remote_endpoint = acpi_fwnode_graph_get_remote_endpoint,
> > > +	.graph_get_remote_port = acpi_fwnode_graph_get_remote_port,
> > > +	.graph_get_remote_port_parent = acpi_fwnode_graph_get_remote_port_parent,
> > > +	.graph_parse_endpoint = acpi_fwnode_graph_parse_endpoint,
> > >  };
> > 
> > Not sure if it is possible but it would be nice to have a single
> > primitive implementation specific graph callback and then build
> > everything else on top of that in generic code. Here you have 5
> > callbacks just for graph support.
> 
> Getting the parent of the port in OF graph is OF specific, the port parent
> is not necessarily a direct parent node of the port (in presence of the
> "ports" node that contains all port nodes).
> 
> I could potentially remove graph_get_remote_port() and use
> graph_get_remote_endpoint() and graph_get_parent() instead. I didn't
> originally do that as I thought it could be better ot leave it up to the
> implementation.
> 
> What do you think?

Well we are dealing with two kinds of objects here, ports and endpoins.
Maybe we can the low level functions parse and return these and then add
generic code on top of that only deals with the port and endpoint
objects. Something like get_graph() which parses the firmware structure
and returns Linux port and endpoint object tree.

Not sure if it is overkill in this case but adding 5 callbacks just for
graphs smells like the design could be improved ;-)

^ permalink raw reply

* Re: [PATCH v5 2/2] mmc: host: s3cmci: allow probing from device tree
From: Arnd Bergmann @ 2017-03-31  7:25 UTC (permalink / raw)
  To: Sergio Prado
  Cc: Ulf Hansson, Rob Herring, Mark Rutland, linux-mmc, devicetree,
	Linux Kernel Mailing List, ben-linux@fluff.org, Linux ARM
In-Reply-To: <1489799633-29278-3-git-send-email-sergio.prado@e-labworks.com>

On Sat, Mar 18, 2017 at 2:13 AM, Sergio Prado
<sergio.prado@e-labworks.com> wrote:

>
> +static const struct of_device_id s3cmci_dt_match[] = {
> +       {
> +               .compatible = "samsung,s3c2410-sdi",
> +               .data = (void *)0,
> +       },
> +       {
> +               .compatible = "samsung,s3c2412-sdi",
> +               .data = (void *)1,
> +       },
> +       {
> +               .compatible = "samsung,s3c2440-sdi",
> +               .data = (void *)1,
> +       },
> +       { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, sdhci_s3c_dt_match);
> +

The names don't match:

In file included from /git/arm-soc/drivers/mmc/host/s3cmci.c:14:0:
/git/arm-soc/drivers/mmc/host/s3cmci.c:1844:25: error:
'sdhci_s3c_dt_match' undeclared here (not in a function); did you mean
's3cmci_dt_match'?
 MODULE_DEVICE_TABLE(of, sdhci_s3c_dt_match);
                         ^
/git/arm-soc/include/linux/module.h:212:21: note: in definition of
macro 'MODULE_DEVICE_TABLE'
 extern const typeof(name) __mod_##type##__##name##_device_table  \
                     ^~~~
/git/arm-soc/include/linux/module.h:212:27: error:
'__mod_of__sdhci_s3c_dt_match_device_table' aliased to undefined
symbol 'sdhci_s3c_dt_match'
 extern const typeof(name) __mod_##type##__##name##_device_table  \
                           ^
/git/arm-soc/drivers/mmc/host/s3cmci.c:1844:1: note: in expansion of
macro 'MODULE_DEVICE_TABLE'
 MODULE_DEVICE_TABLE(of, sdhci_s3c_dt_match);
 ^~~~~~~~~~~~~~~~~~~
/git/arm-soc/scripts/Makefile.build:314: recipe for target
'drivers/mmc/host/s3cmci.o' failed

Can you send a fix?

       Arnd

^ permalink raw reply

* Re: [PATCH V2 2/2] thermal: broadcom: add Northstar thermal driver
From: Rafał Miłecki @ 2017-03-31  7:08 UTC (permalink / raw)
  To: Eduardo Valentin
  Cc: Zhang Rui, Rob Herring, Mark Rutland, Stephen Warren, Lee Jones,
	Eric Anholt, Florian Fainelli, Ray Jui, Scott Branden,
	bcm-kernel-feedback-list, linux-pm, devicetree, linux-rpi-kernel,
	linux-arm-kernel, Rafał Miłecki, Jon Mason
In-Reply-To: <20170331031554.GA3161@localhost.localdomain>

Thanks for your review!

On 03/31/2017 05:15 AM, Eduardo Valentin wrote:
> On Thu, Mar 23, 2017 at 11:30:45PM +0100, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> Northstar is a SoC family commonly used in home routers. This commit
>> adds a driver for checking CPU temperature. As Northstar Plus seems to
>> also have this IP block this new symbol gets ARCH_BCM_IPROC dependency.
>>
>
> Can you please educate me on how different this driver is from
> https://patchwork.kernel.org/patch/9619579/
>
> ?

This is a different hardware block. Northstar and BCM283x are totally different
SoCs. Northstar is mostly used in home routers, BCM283x mostly in Rasperry Pi.


>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> index 776b34396144..008e173ec825 100644
>> --- a/drivers/thermal/Kconfig
>> +++ b/drivers/thermal/Kconfig
>> @@ -392,6 +392,11 @@ config MTK_THERMAL
>>  	  Enable this option if you want to have support for thermal management
>>  	  controller present in Mediatek SoCs
>>
>> +menu "Broadcom thermal drivers"
>> +depends on ARCH_BCM || COMPILE_TEST
>
> Does the above dependency also work for other BCM chips?

I believe so. If there ever appears a driver in drivers/thermal/broadcom/ that
can't be freely compiled with COMPILE_TEST, it can just have a proper
dependency.

^ permalink raw reply

* Re: [PATCH V2 2/2] thermal: broadcom: add Northstar thermal driver
From: Rafał Miłecki @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Jon Mason
  Cc: Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland,
	Stephen Warren, Lee Jones, Eric Anholt, Florian Fainelli, Ray Jui,
	Scott Branden, bcm-kernel-feedback-list, linux-pm, devicetree,
	linux-rpi-kernel, linux-arm-kernel, Rafał Miłecki
In-Reply-To: <20170324143535.GA31953@broadcom.com>

On 03/24/2017 03:35 PM, Jon Mason wrote:
> On Thu, Mar 23, 2017 at 11:30:45PM +0100, Rafał Miłecki wrote:
>> diff --git a/drivers/thermal/broadcom/ns-thermal.c b/drivers/thermal/broadcom/ns-thermal.c
>> new file mode 100644
>> index 000000000000..acf3a4de62a4
>> --- /dev/null
>> +++ b/drivers/thermal/broadcom/ns-thermal.c
>> @@ -0,0 +1,98 @@
>> +/*
>> + * Copyright (C) 2017 Rafał Miłecki <rafal@milecki.pl>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/of_address.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/thermal.h>
>> +
>> +#define PVTMON_CONTROL0					0x00
>> +#define PVTMON_CONTROL0_SEL_MASK			0x0000000e
>> +#define PVTMON_CONTROL0_SEL_TEMP_MONITOR		0x00000000
>> +#define PVTMON_CONTROL0_SEL_TEST_MODE			0x0000000e
>> +#define PVTMON_STATUS					0x08
>> +
>> +struct ns_thermal {
>> +	void __iomem *pvtmon;
>> +};
>> +
>> +static int ns_thermal_get_temp(void *data, int *temp)
>> +{
>> +	struct ns_thermal *ns_thermal = data;
>> +	u32 val;
>> +
>> +	val = readl(ns_thermal->pvtmon + PVTMON_CONTROL0);
>> +	if ((val & PVTMON_CONTROL0_SEL_MASK) != PVTMON_CONTROL0_SEL_TEMP_MONITOR) {
>> +		val &= ~PVTMON_CONTROL0_SEL_MASK;
>> +		val |= PVTMON_CONTROL0_SEL_TEMP_MONITOR;
>
> I think this is slightly confusing here.  If this was off, ORing in 0
> will not enable it.  I think we need to flip the #define to make it
> PVTMON_CONTROL0_SEL_TEMP_MONITOR_DISABLE (or a less crappy name).
> then use this line here to toggle it.  Something like
>
>                 val &= ~PVTMON_CONTROL0_SEL_TEMP_MONITOR_DISABLE;

I don't understand this, can you help me further?

OR-ing with 0 is only a cosmetic thing obviously - just to make it clear which
value we expect to be set in bits 1:3. The important part is:
val &= ~PVTMON_CONTROL0_SEL_MASK;

AFAIU selecting any mode other than TEMP_MONITOR will disable monitor access.
AFAIU even switchint to TEST_MODE will prevent access to temp, so I don't see
how we could use PVTMON_CONTROL0_SEL_TEMP_MONITOR_DISABLE. It could be any
value other than 0.

^ permalink raw reply

* Re: [PATCH V8 5/6] ACPI: Support the probing on the devices which apply indirect-IO
From: zhichang.yuan @ 2017-03-31  6:52 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: mark.rutland, benh, gabriele.paoloni, rafael, linux-pci,
	will.deacon, linuxarm, frowand.list, lorenzo.pieralisi, arnd,
	xuwei5, linux-acpi, catalin.marinas, devicetree, minyard,
	john.garry, zourongrong, robh+dt, bhelgaas, kantyzc,
	zhichang.yuan02, linux-arm-kernel, linux-kernel, olof,
	brian.starkey
In-Reply-To: <1908501.jAQQKvjW4f@aspire.rjw.lan>

Hi, Rafael,

Thanks for reviewing this!

On 2017/3/31 4:31, Rafael J. Wysocki wrote:
> On Thursday, March 30, 2017 11:26:58 PM zhichang.yuan wrote:
>> On some platforms(such as Hip06/Hip07), the legacy ISA/LPC devices access I/O
>> with some special host-local I/O ports known on x86. To access the I/O
>> peripherals, an indirect-IO mechanism is introduced to mapped the host-local
>> I/O to system logical/fake PIO similar the PCI MMIO on architectures where no
>> separate I/O space exists. Just as PCI MMIO, the host I/O range should be
>> registered before probing the downstream devices and set up the I/O mapping.
>> But current ACPI bus probing doesn't support these indirect-IO hosts/devices.
>>
>> This patch introdueces a new ACPI handler for this device category. Through the
>> handler attach callback, the indirect-IO hosts I/O registration is done and
>> all peripherals' I/O resources are translated into logic/fake PIO before
>> starting the enumeration.
> 
> Can you explain to me briefly what exactly this code is expected to be doing?

As you know currently for ARM architecture IO space is memory mapped and
is only used by pci devices. The port number is dynamically allocated
converting the device IO address into a PIO token: i.e.
http://lxr.free-electrons.com/source/drivers/acpi/pci_root.c#L745
This patch is meant to support a new class of IO host controller
that are not PCI based and that still require to have the IO addresses
be translated in the same PIO token space as the PCI controller

Thanks,
Zhichang

> 
> Thanks,
> Rafael
> 
> 
> .
> 

^ permalink raw reply

* Re: [PATCH V8 0/7] LPC: legacy ISA I/O support
From: zhichang.yuan @ 2017-03-31  6:36 UTC (permalink / raw)
  To: dann frazier
  Cc: Catalin Marinas, Will Deacon, Rob Herring, Frank Rowand,
	Bjorn Helgaas, rafael, Arnd Bergmann, linux-arm-kernel,
	Mark Rutland, devicetree@vger.kernel.org, lorenzo.pieralisi,
	Gabriele Paoloni, Corey Minyard, benh, John Garry,
	linux-kernel@vger.kernel.org, xuwei5, linuxarm, linux-acpi,
	Zou Rongrong, linux-pci
In-Reply-To: <CALdTtnsoxNrfmaRn=OtessSC=Ln7pHV5dUBd75uDax3watEoAw@mail.gmail.com>

Hi, Dann,

Many thanks for your tests!

Best,
Zhichang

On 2017/3/31 5:42, dann frazier wrote:
> On Thu, Mar 30, 2017 at 9:26 AM, zhichang.yuan
> <yuanzhichang@hisilicon.com> wrote:
>> This patchset supports the IPMI-bt device attached to the Low-Pin-Count
>> interface implemented on Hisilicon Hip06/Hip07 SoC.
>>                         -----------
>>                         | LPC host|
>>                         |         |
>>                         -----------
>>                              |
>>                 _____________V_______________LPC
>>                   |                       |
>>                   V                       V
>>                                      ------------
>>                                      |  BT(ipmi)|
>>                                      ------------
>>
>> When master accesses those peripherals beneath the Hip06/Hip07 LPC, a specific
>> LPC driver is needed to make LPC host generate the standard LPC I/O cycles with
>> the target peripherals'I/O port addresses. But on curent arm64 world, there is
>> no real I/O accesses. All the I/O operations through in/out pair are based on
>> MMIO which is not satisfied the I/O mechanism on Hip06/Hip07 LPC.
>> To solve this issue and keep the relevant existing peripherals' driver
>> untouched, this patchset implements:
>>   - introduces a generic I/O space management framwork, LIBIO, to support I/O
>>     operations of both MMIO buses and the host controllers which access their
>>     peripherals with host local I/O addresses;
>>   - redefines the in/out accessors to provide unified interfaces for MMIO and
>>     legacy I/O. Based on the LIBIO, the calling of in/out() from upper-layer
>>     drivers, such as ipmi-si, will be redirected to the corresponding
>>     device-specific I/O hooks to perfrom the I/O accesses.
>> Based on this patch-set, all the I/O accesses to Hip06/Hip07 LPC peripherals can
>> be supported without any changes on the existing ipmi-si driver.
>>
>> Changes from V7:
>>   - Based on Arnd's comment, rename the LIBIO as LOGIC_PIO;
>>   - Improved the mapping process in LOGIC_PIO to gain better efficiency when
>>     redirecting the I/O accesses to right device driver;
>>   - To reduce the impact on PCI MMIO to a minimum, add a new
>>     CONFIG_INDIRECT_PIO for indirect-IO hosts/devices;
>>   - Added a new ACPI handler for indirect-IO hosts/devices;
>>   - Fixed the compile issues on V6;
>>
>> Changes from V6:
>>   - According to the comments from Bjorn and Alex, merge PCI IO and indirect-IO
>>     into a generic I/O space management, LIBIO;
>>   - Adopted the '_DEP' to replace the platform bus notifier. In this way, we can
>>     ensure the LPC peripherals' I/O resources had been translated to logical IO
>>     before the LPC peripheral enumeration;
>>   - Replaced the rwlock with rcu list based on Alex's suggestion;
>>   - Applied relaxed write/read to LPC driver;
>>   - Some bugs fixing and some optimazations based on the comments of V6;
>>
>> Changes from V5:
>>   - Made the extio driver more generic and locate in lib/;
>>   - Supported multiple indirect-IO bus instances;
>>   - Extended the pci_register_io_range() to support indirect-IO, then dropped
>>   the I/O reservation used in previous patchset;
>>   - Reimplemented the ACPI LPC support;
>>   - Fixed some bugs, including the compile error on other archs, the module
>>   building failure found by Ming Lei, etc;
>>
>> Changes from V4:
>>   - Some revises based on the comments from Bjorn, Rob on V4;
>>   - Fixed the compile error on some platforms, such as openrisc;
>>
>> Changes from V3:
>>   - UART support deferred to a separate patchset; This patchset only support
>>   ipmi device under LPC;
>>   - LPC bus I/O range is fixed to 0 ~ (PCIBIOS_MIN_IO - 1), which is separeted
>>   from PCI/PCIE PIO space;
>>   - Based on Arnd's remarks, removed the ranges property from Hip06 lpc dts and
>>   added a new fixup function, of_isa_indirect_io(), to get the I/O address
>>   directly from LPC dts configurations;
>>   - Support in(w,l)/out(w,l) for Hip06 lpc I/O;
>>   - Decouple the header file dependency on the gerenic io.h by defining in/out
>>   as normal functions in c file;
>>   - removed unused macro definitions in the LPC driver;
>>
>> Changes from V2:
>>   - Support the PIO retrieval from the linux PIO generated by
>>   pci_address_to_pio. This method replace the 4K PIO reservation in V2;
>>   - Support the flat-tree earlycon;
>>   - Some revises based on Arnd's remarks;
>>   - Make sure the linux PIO range allocated to Hip06 LPC peripherals starts
>>   from non-ZERO;
>>
>> Changes from V1:
>>   - Support the ACPI LPC device;
>>   - Optimize the dts LPC driver in ISA compatible mode;
>>   - Reserve the IO range below 4K in avoid the possible conflict with PCI host
>>   IO ranges;
>>   - Support the LPC uart and relevant earlycon;
>>
>> V7 thread here: https://lkml.org/lkml/2017/3/12/279
>> v6 thread here: https://lkml.org/lkml/2017/1/24/25
>> v5 thread here: https://lkml.org/lkml/2016/11/7/955
>> v4 thread here: https://lkml.org/lkml/2016/10/20/149
>> v3 thread here: https://lkml.org/lkml/2016/9/14/326
>> v2 thread here: https://lkml.org/lkml/2016/9/7/356
>> v1 thread here: https://lkml.org/lkml/2015/12/29/154
>>
>>
>> Signed-off-by: Zhichang Yuan <yuanzhichang@hisilicon.com>
>> zhichang.yuan (6):
>>   LIBIO: Introduce a generic PIO mapping method
>>   PCI: Apply the new generic I/O management on PCI IO hosts
>>   OF: Add missing I/O range exception for indirect-IO devices
>>   LPC: Support the device-tree LPC host on Hip06/Hip07
>>   ACPI: Support the probing on the devices which apply indirect-IO
>>   LPC: Add the ACPI LPC support
>>
>>  .../arm/hisilicon/hisilicon-low-pin-count.txt      |  33 ++
>>  MAINTAINERS                                        |   8 +
>>  arch/arm64/boot/dts/hisilicon/hip06-d03.dts        |   4 +
>>  arch/arm64/boot/dts/hisilicon/hip06.dtsi           |  14 +
>>  arch/arm64/boot/dts/hisilicon/hip07-d05.dts        |   4 +
>>  arch/arm64/boot/dts/hisilicon/hip07.dtsi           |  14 +
>>  drivers/acpi/Makefile                              |   1 +
>>  drivers/acpi/acpi_indirectio.c                     | 344 +++++++++++++
>>  drivers/acpi/internal.h                            |   5 +
>>  drivers/acpi/pci_root.c                            |   8 +-
>>  drivers/acpi/scan.c                                |   1 +
>>  drivers/bus/Kconfig                                |   9 +
>>  drivers/bus/Makefile                               |   1 +
>>  drivers/bus/hisi_lpc.c                             | 547 +++++++++++++++++++++
>>  drivers/of/address.c                               |  95 +++-
>>  drivers/pci/pci.c                                  | 104 +---
>>  include/asm-generic/io.h                           |  50 ++
>>  include/linux/logic_pio.h                          | 174 +++++++
>>  include/linux/pci.h                                |   3 +-
>>  lib/Kconfig                                        |  26 +
>>  lib/Makefile                                       |   2 +
>>  lib/logic_pio.c                                    | 413 ++++++++++++++++
>>  22 files changed, 1758 insertions(+), 102 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
>>  create mode 100644 drivers/acpi/acpi_indirectio.c
>>  create mode 100644 drivers/bus/hisi_lpc.c
>>  create mode 100644 include/linux/logic_pio.h
>>  create mode 100644 lib/logic_pio.c
> 
> Booted up on a D05, was able to use the LPC-connected IPMI interface.
> 
> Tested-by: dann frazier <dann.frazier@canonical.com>
> 
> .
> 


^ permalink raw reply

* Re: [PATCH] Documentation: dt: Add binding for /secure-chosen/stdout-path
From: Jerome Forissier @ 2017-03-31  6:34 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	jens.wiklander-QSEj5FYQhm4dnm+yROfE0A,
	christoffer.dall-QSEj5FYQhm4dnm+yROfE0A,
	peter.maydell-QSEj5FYQhm4dnm+yROfE0A, robin.murphy-5wv7dgnIgG8
In-Reply-To: <20170330222845.ewddsnmkxwrsposl@rob-hp-laptop>



On 03/31/2017 12:28 AM, Rob Herring wrote:
> On Fri, Mar 03, 2017 at 10:24:33AM +0100, Jerome Forissier wrote:
>> Some platforms may use a single device tree to describe two address
>> spaces, as described in d9f43babb998 ("Documentation: dt: Add bindings
>> for Secure-only devices"). For these platforms it makes sense to define
>> a secure counterpart of /chosen, namely: /secure-chosen. This new node
>> is meant to be used by the secure firmware to pass data to the secure
>> OS. Only the stdout-path property is supported for now.
>>
>> Signed-off-by: Jerome Forissier <jerome.forissier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>  Documentation/devicetree/bindings/arm/secure.txt | 19 ++++++++++++++++++-
>>  1 file changed, 18 insertions(+), 1 deletion(-)
>>
>> v2
>>
>> - Clarify the expected behavior when /secure-chosen/stdout-path is not
>> present (either the property or the node itself), as proposed by Peter
>> and Robin. Note that I have made the fallback to /chosen optional.
> 
> I'm okay with this, but would like to hold off merging it until your are 
> really using it. You did say you were "experimenting" and it doesn't 
> sound like the DT usage in secure world is that far along. I just want 
> to see if there are other issues that come up.

Fair enough.

Thanks,
-- 
Jerome

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

* Re: [PATCH v3 14/37] mtd: nand: denali: support "nand-ecc-strength" DT property
From: Masahiro Yamada @ 2017-03-31  5:06 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: linux-mtd, Enrico Jorns, Artem Bityutskiy, Dinh Nguyen,
	Marek Vasut, Graham Moore, David Woodhouse, Masami Hiramatsu,
	Chuanxiao Dong, Jassi Brar, devicetree, Linux Kernel Mailing List,
	Brian Norris, Richard Weinberger, Cyrille Pitchen, Rob Herring,
	Mark Rutland
In-Reply-To: <20170330160238.59e5a2c1@bbrezillon>

Hi Boris,


2017-03-30 23:02 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
> On Thu, 30 Mar 2017 15:46:00 +0900
> Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
>
>> Historically, this driver tried to choose as big ECC strength as
>> possible, but it would be reasonable to allow DT to set a particular
>> ECC strength with "nand-ecc-strength" property.  This is useful
>> when a particular ECC setting is hard-coded by firmware (or hard-
>> wired by boot ROM).
>>
>> If no ECC strength is specified in DT, "nand-ecc-maximize" is implied
>> since this was the original behavior.
>
> You said there is currently no DT users,

Right.  No DT users ever in upstream.


> so how about changing the
> "fallback to ECC maximization" behavior for DT users, and instead of
> maximizing the ECC strength take the NAND requirements into account
> (chip->ecc_strength_ds).

This is difficult to judge in some cases.

As I said before, 4/512 and 8/1024 are not equivalent.

If chip's requirement  chip->ecc_step_ds matches
to the ecc->size supported by the controller,
this is easy.


If a chip requests 1024B, then the controller can only support 512B chunk
(or vice versa), it is difficult to simply compare
ecc strength.

Is it a bad thing if we use too strong ECC strength?

The disadvantage I see is we will have less OOB-free bytes,
but this will not be fatal, I guess.




-- 
Best Regards
Masahiro Yamada

^ permalink raw reply

* Re: [PATCH 2/2] dt-bindings: mtd: document linux,part-probe property
From: Rafał Miłecki @ 2017-03-31  5:03 UTC (permalink / raw)
  To: Marek Vasut, David Woodhouse, Brian Norris, Boris Brezillon,
	Richard Weinberger, Cyrille Pitchen
  Cc: Rob Herring, Mark Rutland, Frank Rowand, Linus Walleij,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rafał Miłecki
In-Reply-To: <74e3a663-2431-9b86-9d90-7f2fe6ce900f-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On 03/31/2017 01:26 AM, Marek Vasut wrote:
> On 03/30/2017 11:53 PM, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>
>> Support for this property has been introduced in 2010 with commit
>> 9d5da3a9b849 ("mtd: extend physmap_of to let the device tree specify the
>> parition probe") but it was never documented. Fix this by adding a
>> proper description and example.
>>
>> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>> ---
>>  Documentation/devicetree/bindings/mtd/common.txt | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/common.txt b/Documentation/devicetree/bindings/mtd/common.txt
>> index fc068b923d7a..1ada70e718b2 100644
>> --- a/Documentation/devicetree/bindings/mtd/common.txt
>> +++ b/Documentation/devicetree/bindings/mtd/common.txt
>> @@ -6,10 +6,17 @@ Optional properties:
>>    controller based name) in order to ease flash device identification
>>    and/or describe what they are used for.
>>
>> +- linux,part-probe: if present, this property should contain a list of strings
>> +  with partition probes to be used for the flash device. A role of partition
>> +  probe (parser) is to read/construct partition table and register found
>> +  partitions. Getting partition table may be platform or device specific so
>> +  various devices may use various Linux drivers for this purpose.
>
> Why don't you just have partition not within partition node ? Then you
> won't need this nonsense ...

Can you stop this negative approach ("nonsense") for every single f* thing I
submit? It doesn't help and makes people not want to work with upstream mtd.

The only partitioner with support for partitions node is "fixed-partitions".
Take a look at original linux,part-probe usage:
arch/arm64/boot/dts/arm/juno-motherboard.dtsi
It's used to specify "afs" partitioner. It's dynamic. You can't replace it with
fixed partitions.

So this property is needed to specify platform/device specific partitioner that
should be used with some standard flash driver (like physmap_of or m25p80).

I was hoping following part of documentation makes is clear:
 > Getting partition table may be platform or device specific so
 > various devices may use various Linux drivers for this purpose.
--
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

* [PATCH] arm64: dts: uniphier: add input-delay properties to Cadence eMMC node
From: Masahiro Yamada @ 2017-03-31  4:20 UTC (permalink / raw)
  To: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: Piotr Sroka, Masahiro Yamada, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Will Deacon,
	Mark Rutland, Catalin Marinas,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Since commit a04e2b383401 ("mmc: sdhci-cadence: Update PHY delay
configuration"), PHY parameters must be specified by DT.

The hard-coded settings have been converted as follows:
- SDHCI_CDNS_PHY_DLY_SD_DEFAULT -> cdns,phy-input-delay-legacy
- SDHCI_CDNS_PHY_DLY_EMMC_SDR   -> cdns,phy-input-delay-mmc-highspeed
- SDHCI_CDNS_PHY_DLY_EMMC_DDR   -> cdns,phy-input-delay-mmc-ddr

The following have not been moved:
- SDHCI_CDNS_PHY_DLY_SD_HS
   this is unneeded in the eMMC configuration
- SDHCI_CDNS_PHY_DLY_EMMC_LEGACY
   this is never enabled by the driver as it is covered by
   SDHCI_CDNS_PHY_DLY_SD_DEFAULT

Signed-off-by: Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
---

 arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi | 3 +++
 arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi b/arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi
index 5dc5124..b6ebdc9 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi
+++ b/arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi
@@ -310,6 +310,9 @@
 			bus-width = <8>;
 			mmc-ddr-1_8v;
 			mmc-hs200-1_8v;
+			cdns,phy-input-delay-legacy = <4>;
+			cdns,phy-input-delay-mmc-highspeed = <2>;
+			cdns,phy-input-delay-mmc-ddr = <3>;
 		};
 
 		usb0: usb@5a800100 {
diff --git a/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi b/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
index 6c9a72d..0ab6c2e 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
+++ b/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
@@ -394,6 +394,9 @@
 			bus-width = <8>;
 			mmc-ddr-1_8v;
 			mmc-hs200-1_8v;
+			cdns,phy-input-delay-legacy = <4>;
+			cdns,phy-input-delay-mmc-highspeed = <2>;
+			cdns,phy-input-delay-mmc-ddr = <3>;
 		};
 
 		sd: sdhc@5a400000 {
-- 
2.7.4

--
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 related

* Re: [PATCH v3 00/37] mtd: nand: denali: 2nd round of Denali NAND IP patch bomb
From: Masahiro Yamada @ 2017-03-31  4:05 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: linux-mtd, Enrico Jorns, Artem Bityutskiy, Dinh Nguyen,
	Marek Vasut, Graham Moore, David Woodhouse, Masami Hiramatsu,
	Chuanxiao Dong, Jassi Brar, devicetree, Linux Kernel Mailing List,
	Brian Norris, Richard Weinberger, Cyrille Pitchen, Rob Herring,
	Mark Rutland
In-Reply-To: <20170330183805.1c29504f@bbrezillon>

Hi Boris,


2017-03-31 1:38 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:

> The rest looks almost good, except for a few comments I had on patch
> 14, 18, 25, 26 and 32.
>
> I'll probably apply 33 and 34 soon.
>

Thank you!
Please note I left a minor comment on 34.
(Accidental addition of braces.)


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply

* Re: [RFC PATCH 2/5] soc/fsl/qbman: Use shared-dma-pool for QMan private memory allocations
From: Michael Ellerman @ 2017-03-31  3:27 UTC (permalink / raw)
  To: Robin Murphy, roy.pledge-3arQi8VN3Tc, oss-fOR+EgIDQEHk1uMJSBkQmQ,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: madalin.bucur-3arQi8VN3Tc
In-Reply-To: <b1cd8d44-8c74-ab50-5dc9-1f978cae5bdf-5wv7dgnIgG8@public.gmane.org>

Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> writes:

> Hi Roy,
>
> On 29/03/17 22:13, Roy Pledge wrote:
>> Use the shared-memory-pool mechanism for frame queue descriptor and
>> packed frame descriptor record area allocations.
>
> Thanks for persevering with this - in my opinion it's now looking like
> it was worth the effort :)
>
> AFAICS the ioremap_wc() that this leads to does appear to give back
> something non-cacheable on PPC (assuming "pgprot_noncached_wc" isn't
> horrendously misnamed), and "no-map" should rule out any cacheable
> linear map alias existing, so it would seem that this approach should
> avert Scott's concerns about attribute mismatches.

How does 'no-map' translate into something being excluded from the
linear mapping?

cheers
--
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

* Re: [PATCH V2 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal
From: Eduardo Valentin @ 2017-03-31  3:17 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Zhang Rui, Rob Herring, Mark Rutland, Stephen Warren, Lee Jones,
	Eric Anholt, Florian Fainelli, Ray Jui, Scott Branden,
	bcm-kernel-feedback-list, linux-pm, devicetree, linux-rpi-kernel,
	linux-arm-kernel, Rafał Miłecki
In-Reply-To: <20170323223045.15786-1-zajec5@gmail.com>

Hello Rafal,

On Thu, Mar 23, 2017 at 11:30:44PM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> This commit documents binding for thermal used in Northstar family SoCs.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  .../devicetree/bindings/thermal/brcm,ns-thermal         | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/brcm,ns-thermal
> 
> diff --git a/Documentation/devicetree/bindings/thermal/brcm,ns-thermal b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
> new file mode 100644
> index 000000000000..9a22dd76ec01
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
> @@ -0,0 +1,17 @@
> +* Broadcom Northstar Thermal
> +
> +This binding describes thermal sensor that is part of Northstar's DMU (Device
> +Management Unit).
> +
> +Required properties:
> +- compatible : Must be "brcm,ns-thermal"
> +- reg : iomem address range of PVTMON registers
> +- #thermal-sensor-cells : Should be <0>
> +
> +Example:
> +
> +thermal {
> +	compatible = "brcm,ns-thermal";
> +	reg = <0x1800c2c0 0x10>;
> +	#thermal-sensor-cells = <0>;
> +};


Could you please also include an example of the thermal zone one should
include in the board/cpu DTS while using this sensor? 

Besides, based on what I saw you in your driver, you could also describe
your offset and slope in DT, and read them using thermal helpers.

BR,
> -- 
> 2.11.0
> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox