Devicetree
 help / color / mirror / Atom feed
* [PATCH 2/2 v4] dt-bindings: display: imx: entry for AUS mode
From: Martin Kaiser @ 2017-04-21 12:29 UTC (permalink / raw)
  To: linux-fbdev-u79uwXL29TY76Z2rM5mHXA
  Cc: Bartlomiej Zolnierkiewicz, Sascha Hauer,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Martin Kaiser
In-Reply-To: <1492770549-7347-1-git-send-email-martin-XxZfDwE/svGeZLLa646FqQ@public.gmane.org>

Allow setting AUS mode for a display from the device tree. Use an
optional boolean property. AUS mode can be set only on imx21 and
compatible chipsets.

Signed-off-by: Martin Kaiser <martin-XxZfDwE/svGeZLLa646FqQ@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
sending this again, I missed the v4 in the Subject line

v4:
   rename the DT property to fsl,aus-mode

v3:
   use a boolean DT property instead of the register value
   separate patches for DT binding and code changes

v2:
   re-sending DT bindings and code changes as one patch

 Documentation/devicetree/bindings/display/imx/fsl,imx-fb.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/imx/fsl,imx-fb.txt b/Documentation/devicetree/bindings/display/imx/fsl,imx-fb.txt
index 7a5c0e2..e5a8b36 100644
--- a/Documentation/devicetree/bindings/display/imx/fsl,imx-fb.txt
+++ b/Documentation/devicetree/bindings/display/imx/fsl,imx-fb.txt
@@ -13,6 +13,8 @@ Required nodes:
 	Additional, the display node has to define properties:
 	- bits-per-pixel: Bits per pixel
 	- fsl,pcr: LCDC PCR value
+	A display node may optionally define
+	- fsl,aus-mode: boolean to enable AUS mode (only for imx21)
 
 Optional properties:
 - lcd-supply: Regulator for LCD supply voltage.
-- 
2.1.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

* [PATCH 1/2] gpio: move tca9554 from pcf857x to pca953x
From: Anders Darander @ 2017-04-21 12:46 UTC (permalink / raw)
  To: linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8
  Cc: Anders Darander

The TCA9554 doesn't work with the pcf857x driver, trying to change the direction
gives a NAK bailout error.

TCA9554 is similar to the PCA9554, thus change the driver.

Signed-off-by: Anders Darander <anders-7UjN0b3lYz2SbKU13Z4Etw@public.gmane.org>
---
 drivers/gpio/Kconfig        | 2 +-
 drivers/gpio/gpio-pca953x.c | 1 +
 drivers/gpio/gpio-pcf857x.c | 2 --
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 05043071fc98..684f887173e9 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -753,7 +753,7 @@ config GPIO_PCA953X
 	  4 bits:	pca9536, pca9537
 
 	  8 bits:	max7310, max7315, pca6107, pca9534, pca9538, pca9554,
-			pca9556, pca9557, pca9574, tca6408, xra1202
+			pca9556, pca9557, pca9574, tca6408, tca9554, xra1202
 
 	  16 bits:	max7312, max7313, pca9535, pca9539, pca9555, pca9575,
 			tca6416
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index d44232aadb6c..13d895264fc8 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -81,6 +81,7 @@ static const struct i2c_device_id pca953x_id[] = {
 	{ "tca6416", 16 | PCA953X_TYPE | PCA_INT, },
 	{ "tca6424", 24 | PCA953X_TYPE | PCA_INT, },
 	{ "tca9539", 16 | PCA953X_TYPE | PCA_INT, },
+	{ "tca9554", 8  | PCA953X_TYPE | PCA_INT, },
 	{ "xra1202", 8  | PCA953X_TYPE },
 	{ }
 };
diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
index 895af42a4513..8ddf9302ce3b 100644
--- a/drivers/gpio/gpio-pcf857x.c
+++ b/drivers/gpio/gpio-pcf857x.c
@@ -46,7 +46,6 @@ static const struct i2c_device_id pcf857x_id[] = {
 	{ "pca9675", 16 },
 	{ "max7328", 8 },
 	{ "max7329", 8 },
-	{ "tca9554", 8 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, pcf857x_id);
@@ -66,7 +65,6 @@ static const struct of_device_id pcf857x_of_table[] = {
 	{ .compatible = "nxp,pca9675" },
 	{ .compatible = "maxim,max7328" },
 	{ .compatible = "maxim,max7329" },
-	{ .compatible = "ti,tca9554" },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, pcf857x_of_table);
-- 
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 2/2] gpio: DT bindings, move tca9554 from pcf857x to pca953x
From: Anders Darander @ 2017-04-21 12:46 UTC (permalink / raw)
  To: linus.walleij, gnurou, robh+dt, linux-gpio, devicetree,
	linux-kernel, mark.rutland
  Cc: Anders Darander
In-Reply-To: <20170421124631.19269-1-anders@chargestorm.se>

The TCA9554 is similar to the PCA9554. Update the DT binding docs.

Signed-off-by: Anders Darander <anders@chargestorm.se>
---
 Documentation/devicetree/bindings/gpio/gpio-pca953x.txt | 1 +
 Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
index e63935710011..7f57271df2bc 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
@@ -26,6 +26,7 @@ Required properties:
 	ti,tca6416
 	ti,tca6424
 	ti,tca9539
+	ti,tca9554
 	onsemi,pca9654
 	exar,xra1202
 
diff --git a/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
index ada4e2973323..7d3bd631d011 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
@@ -25,7 +25,6 @@ Required Properties:
     - "nxp,pcf8574": For the NXP PCF8574
     - "nxp,pcf8574a": For the NXP PCF8574A
     - "nxp,pcf8575": For the NXP PCF8575
-    - "ti,tca9554": For the TI TCA9554
 
   - reg: I2C slave address.
 
-- 
2.11.0


^ permalink raw reply related

* [PATCH/RFC 0/5] arm64: dts: renesas: Break out common board support
From: Geert Uytterhoeven @ 2017-04-21 12:55 UTC (permalink / raw)
  To: Simon Horman, Magnus Damm, Kuninori Morimoto, Yoshihiro Shimoda,
	Rob Herring, Mark Rutland
  Cc: linux-renesas-soc, devicetree, linux-arm-kernel,
	Geert Uytterhoeven

	Hi all,

The Renesas Salvator-X and ULCB development board can be equipped with
either an R-Car H3 or M3-W SiP, which are pin-compatible.  All boards
use separate DTBs, but currently there's no sharing of board-specific
devices in DTS.

This series reduces duplication by extracting common board support into
their own .dtsi files.  As the level of support varies across boards and
SoCs, this requires the addition of a few external clocks and
placeholder devices on R-Car M3-W, so the common board support DTS can
refer to them.

  - Patches 1 and 2 add the external audio and PCIe bus clocks on R-Car
    M3-W, which are present in r8a7795.dtsi, and used in
    r8a7795-salvator-x.dts,
  - RFC patch 3 adds placeholders for devices that are not yet supported
    and/or tested on R-Car M3-W, but used on R-Car H3,
  - RFC patch 4 extracts common Salvator-X board support,
  - RFC patch 5 extracts common ULCB board support.

For R-Car H3 based boards, there are no functional changes.
For R-Car M3-W based boards, some new devices are now described in DT.

Dependencies:
  - renesas-devel-20170420-v4.11-rc7,
  - Patches 1 and 2 can be applied as-is,
  - Patches 4 and 5 depend on "[PATCH 0/8] arm64: dts: renesas: Break
    out R-Car H3 and M3-W SiP"
    (http://www.spinics.net/lists/devicetree/msg173820.html).

DTB changes have been inspected using scripts/dtc/dtx_diff.
This has been tested on Salvator-X (both H3 and M3-W).
This has not been tested on H3ULCB and M3ULCB due to lack of hardware.

Thanks for your comments!

Geert Uytterhoeven (5):
  arm64: renesas: r8a7796: Add external audio clocks
  arm64: renesas: r8a7796: Add external PCIe bus clock
  [RFC] arm64: dts: r8a7796: Add placeholders for various devices
  [RFC] arm64: dts: renesas: Extract common Salvator-X board support
  [RFC] arm64: dts: renesas: Extract common ULCB board support

 arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts     | 341 +-------------
 arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts | 522 +--------------------
 arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts     | 201 +-------
 arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 259 +---------
 arch/arm64/boot/dts/renesas/r8a7796.dtsi           | 112 +++++
 .../{r8a7795-salvator-x.dts => salvator-x.dtsi}    | 372 +++++++--------
 .../dts/renesas/{r8a7795-h3ulcb.dts => ulcb.dtsi}  | 243 +++++-----
 7 files changed, 421 insertions(+), 1629 deletions(-)
 copy arch/arm64/boot/dts/renesas/{r8a7795-salvator-x.dts => salvator-x.dtsi} (95%)
 copy arch/arm64/boot/dts/renesas/{r8a7795-h3ulcb.dts => ulcb.dtsi} (96%)

-- 
2.7.4

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

^ permalink raw reply

* [PATCH 1/5] arm64: renesas: r8a7796: Add external audio clocks
From: Geert Uytterhoeven @ 2017-04-21 12:55 UTC (permalink / raw)
  To: Simon Horman, Magnus Damm, Kuninori Morimoto, Yoshihiro Shimoda,
	Rob Herring, Mark Rutland
  Cc: linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Geert Uytterhoeven
In-Reply-To: <1492779321-23939-1-git-send-email-geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>

Add the external audio clocks as zero Hz fixed-frequency clocks.
Boards that provide these clocks should override them.

Based on commit 623197b90c7aa97c ("arm64: renesas: r8a7795: Sound SSI
PIO support").

Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
---
 arch/arm64/boot/dts/renesas/r8a7796.dtsi | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index 2ec1ed5f499165ad..101cd41d693a7ab5 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -120,6 +120,29 @@
 		clock-frequency = <0>;
 	};
 
+	/*
+	 * The external audio clocks are configured as 0 Hz fixed frequency
+	 * clocks by default.
+	 * Boards that provide audio clocks should override them.
+	 */
+	audio_clk_a: audio_clk_a {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <0>;
+	};
+
+	audio_clk_b: audio_clk_b {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <0>;
+	};
+
+	audio_clk_c: audio_clk_c {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <0>;
+	};
+
 	/* External CAN clock - to be overridden by boards that provide it */
 	can_clk: can {
 		compatible = "fixed-clock";
-- 
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

* [PATCH 2/5] arm64: renesas: r8a7796: Add external PCIe bus clock
From: Geert Uytterhoeven @ 2017-04-21 12:55 UTC (permalink / raw)
  To: Simon Horman, Magnus Damm, Kuninori Morimoto, Yoshihiro Shimoda,
	Rob Herring, Mark Rutland
  Cc: linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Geert Uytterhoeven
In-Reply-To: <1492779321-23939-1-git-send-email-geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>

Add the external PCIe bus clock as a zero Hz fixed-frequency clock.
Boards that provide this clock should override it.

Based on r8a7795.dtsi.

Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
---
 arch/arm64/boot/dts/renesas/r8a7796.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index 101cd41d693a7ab5..8e2aab8b6b103cc9 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -157,6 +157,13 @@
 		clock-frequency = <0>;
 	};
 
+	/* External PCIe clock - can be overridden by the board */
+	pcie_bus_clk: pcie_bus {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <0>;
+	};
+
 	soc {
 		compatible = "simple-bus";
 		interrupt-parent = <&gic>;
-- 
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

* [PATCH/RFC 3/5] arm64: dts: r8a7796: Add placeholders for various devices
From: Geert Uytterhoeven @ 2017-04-21 12:55 UTC (permalink / raw)
  To: Simon Horman, Magnus Damm, Kuninori Morimoto, Yoshihiro Shimoda,
	Rob Herring, Mark Rutland
  Cc: linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Geert Uytterhoeven
In-Reply-To: <1492779321-23939-1-git-send-email-geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>

Add empty device nodes serving as placeholders for devices that are not
yet supported and/or tested on R-Car M3-W, but are supported and used on
Salvator-X or H3ULCB boards equipped with an R-Car H3 SoC.

Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
---
 arch/arm64/boot/dts/renesas/r8a7796.dtsi | 82 ++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index 8e2aab8b6b103cc9..60a4289d0b14fe50 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -961,6 +961,38 @@
 			dma-channels = <16>;
 		};
 
+		hsusb: usb@e6590000 {
+			/* placeholder */
+		};
+
+		xhci0: usb@ee000000 {
+			/* placeholder */
+		};
+
+		ohci0: usb@ee080000 {
+			/* placeholder */
+		};
+
+		ehci0: usb@ee080100 {
+			/* placeholder */
+		};
+
+		usb2_phy0: usb-phy@ee080200 {
+			/* placeholder */
+		};
+
+		ohci1: usb@ee0a0000 {
+			/* placeholder */
+		};
+
+		ehci1: usb@ee0a0100 {
+			/* placeholder */
+		};
+
+		usb2_phy1: usb-phy@ee0a0200 {
+			/* placeholder */
+		};
+
 		sdhi0: sd@ee100000 {
 			compatible = "renesas,sdhi-r8a7796";
 			reg = <0 0xee100000 0 0x2000>;
@@ -1063,5 +1095,55 @@
 				};
 			};
 		};
+
+		rcar_sound: sound@ec500000 {
+			/* placeholder */
+
+			rcar_sound,dvc {
+				dvc0: dvc-0 {
+				};
+
+				dvc1: dvc-1 {
+				};
+			};
+
+			rcar_sound,src {
+				src0: src-0 {
+				};
+				src1: src-1 {
+				};
+			};
+
+			rcar_sound,ssi {
+				ssi0: ssi-0 {
+				};
+
+				ssi1: ssi-1 {
+				};
+			};
+		};
+
+		pciec0: pcie@fe000000 {
+			/* placeholder */
+		};
+
+		pciec1: pcie@ee800000 {
+			/* placeholder */
+		};
+
+		du: display@feb00000 {
+			/* placeholder */
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					du_out_rgb: endpoint {
+					};
+				};
+			};
+		};
 	};
 };
-- 
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

* [PATCH/RFC 4/5] arm64: dts: renesas: Extract common Salvator-X board support
From: Geert Uytterhoeven @ 2017-04-21 12:55 UTC (permalink / raw)
  To: Simon Horman, Magnus Damm, Kuninori Morimoto, Yoshihiro Shimoda,
	Rob Herring, Mark Rutland
  Cc: linux-renesas-soc, devicetree, linux-arm-kernel,
	Geert Uytterhoeven
In-Reply-To: <1492779321-23939-1-git-send-email-geert+renesas@glider.be>

The Renesas Salvator-X development board can be equipped with either an
R-Car H3 or M3-W SiP, which are pin-compatible.  Both boards use
different DTBs.

Reduce duplication by extracting common Salvator-X board support into
its own .dtsi file.  References to SoC-specific clocks are handled
through cpp definitions.  Sort device nodes while at it.

For boards with an R-Car H3 SiP, there are no functional changes.

For boards with an R-Car M3-W SiP, the following new devices are now
described in DT:
  - External audio, CAN, and PCIe clocks,
  - USB Vbus regulator,
  - CS2000 clock generator,
  - AK4613 Audio Codec,
  - VGA.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts | 522 +--------------------
 arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 259 +---------
 .../{r8a7795-salvator-x.dts => salvator-x.dtsi}    | 372 +++++++--------
 3 files changed, 183 insertions(+), 970 deletions(-)
 copy arch/arm64/boot/dts/renesas/{r8a7795-salvator-x.dts => salvator-x.dtsi} (95%)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
index e5b9409bf2d218d8..c2fab17a3c2e87f7 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
@@ -31,549 +31,39 @@
  *	amixer set "DVC Out" 100%  // Volume Up
  */
 
+#define CPG_AUDIO_CLK_I		R8A7795_CLK_S0D4
+
 /dts-v1/;
 #include "r8j7795-4x1g.dtsi"
-#include <dt-bindings/gpio/gpio.h>
+#include "salvator-x.dtsi"
 
 / {
 	model = "Renesas Salvator-X board based on r8j7795";
 	compatible = "renesas,salvator-x", "renesas,r8j7795", "renesas,r8a7795";
-
-	aliases {
-		serial0 = &scif2;
-		serial1 = &scif1;
-		ethernet0 = &avb;
-	};
-
-	chosen {
-		bootargs = "ignore_loglevel rw root=/dev/nfs ip=dhcp";
-		stdout-path = "serial0:115200n8";
-	};
-
-	x12_clk: x12 {
-		compatible = "fixed-clock";
-		#clock-cells = <0>;
-		clock-frequency = <24576000>;
-	};
-
-	reg_1p8v: regulator0 {
-		compatible = "regulator-fixed";
-		regulator-name = "fixed-1.8V";
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <1800000>;
-		regulator-boot-on;
-		regulator-always-on;
-	};
-
-	reg_3p3v: regulator1 {
-		compatible = "regulator-fixed";
-		regulator-name = "fixed-3.3V";
-		regulator-min-microvolt = <3300000>;
-		regulator-max-microvolt = <3300000>;
-		regulator-boot-on;
-		regulator-always-on;
-	};
-
-	vcc_sdhi0: regulator-vcc-sdhi0 {
-		compatible = "regulator-fixed";
-
-		regulator-name = "SDHI0 Vcc";
-		regulator-min-microvolt = <3300000>;
-		regulator-max-microvolt = <3300000>;
-
-		gpio = <&gpio5 2 GPIO_ACTIVE_HIGH>;
-		enable-active-high;
-	};
-
-	vccq_sdhi0: regulator-vccq-sdhi0 {
-		compatible = "regulator-gpio";
-
-		regulator-name = "SDHI0 VccQ";
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <3300000>;
-
-		gpios = <&gpio5 1 GPIO_ACTIVE_HIGH>;
-		gpios-states = <1>;
-		states = <3300000 1
-			  1800000 0>;
-	};
-
-	vcc_sdhi3: regulator-vcc-sdhi3 {
-		compatible = "regulator-fixed";
-
-		regulator-name = "SDHI3 Vcc";
-		regulator-min-microvolt = <3300000>;
-		regulator-max-microvolt = <3300000>;
-
-		gpio = <&gpio3 15 GPIO_ACTIVE_HIGH>;
-		enable-active-high;
-	};
-
-	vccq_sdhi3: regulator-vccq-sdhi3 {
-		compatible = "regulator-gpio";
-
-		regulator-name = "SDHI3 VccQ";
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <3300000>;
-
-		gpios = <&gpio3 14 GPIO_ACTIVE_HIGH>;
-		gpios-states = <1>;
-		states = <3300000 1
-			  1800000 0>;
-	};
-
-	vbus0_usb2: regulator-vbus0-usb2 {
-		compatible = "regulator-fixed";
-
-		regulator-name = "USB20_VBUS0";
-		regulator-min-microvolt = <5000000>;
-		regulator-max-microvolt = <5000000>;
-
-		gpio = <&gpio6 16 GPIO_ACTIVE_HIGH>;
-		enable-active-high;
-	};
-
-	audio_clkout: audio_clkout {
-		/*
-		 * This is same as <&rcar_sound 0>
-		 * but needed to avoid cs2000/rcar_sound probe dead-lock
-		 */
-		compatible = "fixed-clock";
-		#clock-cells = <0>;
-		clock-frequency = <11289600>;
-	};
-
-	rsnd_ak4613: sound {
-		compatible = "simple-audio-card";
-
-		simple-audio-card,format = "left_j";
-		simple-audio-card,bitclock-master = <&sndcpu>;
-		simple-audio-card,frame-master = <&sndcpu>;
-
-		sndcpu: simple-audio-card,cpu {
-			sound-dai = <&rcar_sound>;
-		};
-
-		sndcodec: simple-audio-card,codec {
-			sound-dai = <&ak4613>;
-		};
-	};
-
-	vga-encoder {
-		compatible = "adi,adv7123";
-
-		ports {
-			#address-cells = <1>;
-			#size-cells = <0>;
-
-			port@0 {
-				reg = <0>;
-				adv7123_in: endpoint {
-					remote-endpoint = <&du_out_rgb>;
-				};
-			};
-			port@1 {
-				reg = <1>;
-				adv7123_out: endpoint {
-					remote-endpoint = <&vga_in>;
-				};
-			};
-		};
-	};
-
-	vga {
-		compatible = "vga-connector";
-
-		port {
-			vga_in: endpoint {
-				remote-endpoint = <&adv7123_out>;
-			};
-		};
-	};
 };
 
-&du {
-	pinctrl-0 = <&du_pins>;
-	pinctrl-names = "default";
+&ehci2 {
 	status = "okay";
-
-	ports {
-		port@0 {
-			endpoint {
-				remote-endpoint = <&adv7123_in>;
-			};
-		};
-		port@3 {
-			lvds_connector: endpoint {
-			};
-		};
-	};
 };
 
-&extal_clk {
-	clock-frequency = <16666666>;
-};
-
-&extalr_clk {
-	clock-frequency = <32768>;
+&ohci2 {
+	status = "okay";
 };
 
 &pfc {
-	pinctrl-0 = <&scif_clk_pins>;
-	pinctrl-names = "default";
-
-	scif1_pins: scif1 {
-		groups = "scif1_data_a", "scif1_ctrl";
-		function = "scif1";
-	};
-	scif2_pins: scif2 {
-		groups = "scif2_data_a";
-		function = "scif2";
-	};
-	scif_clk_pins: scif_clk {
-		groups = "scif_clk_a";
-		function = "scif_clk";
-	};
-
-	i2c2_pins: i2c2 {
-		groups = "i2c2_a";
-		function = "i2c2";
-	};
-
-	avb_pins: avb {
-		mux {
-			groups = "avb_link", "avb_phy_int", "avb_mdc",
-				 "avb_mii";
-			function = "avb";
-		};
-
-		pins_mdc {
-			groups = "avb_mdc";
-			drive-strength = <24>;
-		};
-
-		pins_mii_tx {
-			pins = "PIN_AVB_TX_CTL", "PIN_AVB_TXC", "PIN_AVB_TD0",
-			       "PIN_AVB_TD1", "PIN_AVB_TD2", "PIN_AVB_TD3";
-			drive-strength = <12>;
-		};
-	};
-
-	du_pins: du {
-		groups = "du_rgb888", "du_sync", "du_oddf", "du_clk_out_0";
-		function = "du";
-	};
-
-	sdhi0_pins: sd0 {
-		groups = "sdhi0_data4", "sdhi0_ctrl";
-		function = "sdhi0";
-		power-source = <3300>;
-	};
-
-	sdhi0_pins_uhs: sd0_uhs {
-		groups = "sdhi0_data4", "sdhi0_ctrl";
-		function = "sdhi0";
-		power-source = <1800>;
-	};
-
-	sdhi2_pins: sd2 {
-		groups = "sdhi2_data8", "sdhi2_ctrl";
-		function = "sdhi2";
-		power-source = <3300>;
-	};
-
-	sdhi2_pins_uhs: sd2_uhs {
-		groups = "sdhi2_data8", "sdhi2_ctrl";
-		function = "sdhi2";
-		power-source = <1800>;
-	};
-
-	sdhi3_pins: sd3 {
-		groups = "sdhi3_data4", "sdhi3_ctrl";
-		function = "sdhi3";
-		power-source = <3300>;
-	};
-
-	sdhi3_pins_uhs: sd3_uhs {
-		groups = "sdhi3_data4", "sdhi3_ctrl";
-		function = "sdhi3";
-		power-source = <1800>;
-	};
-
-	sound_pins: sound {
-		groups = "ssi01239_ctrl", "ssi0_data", "ssi1_data_a";
-		function = "ssi";
-	};
-
-	sound_clk_pins: sound_clk {
-		groups = "audio_clk_a_a", "audio_clk_b_a", "audio_clk_c_a",
-			 "audio_clkout_a", "audio_clkout3_a";
-		function = "audio_clk";
-	};
-
-	usb0_pins: usb0 {
-		groups = "usb0";
-		function = "usb0";
-	};
-
-	usb1_pins: usb1 {
-		mux {
-			groups = "usb1";
-			function = "usb1";
-		};
-
-		ovc {
-			pins = "GP_6_27";
-			bias-pull-up;
-		};
-
-		pwen {
-			pins = "GP_6_26";
-			bias-pull-down;
-		};
-	};
-
 	usb2_pins: usb2 {
 		groups = "usb2";
 		function = "usb2";
 	};
 };
 
-&scif1 {
-	pinctrl-0 = <&scif1_pins>;
-	pinctrl-names = "default";
-
-	uart-has-rtscts;
-	status = "okay";
-};
-
-&scif2 {
-	pinctrl-0 = <&scif2_pins>;
-	pinctrl-names = "default";
-
-	status = "okay";
-};
-
-&scif_clk {
-	clock-frequency = <14745600>;
-};
-
-&i2c2 {
-	pinctrl-0 = <&i2c2_pins>;
-	pinctrl-names = "default";
-
-	status = "okay";
-
-	clock-frequency = <100000>;
-
-	ak4613: codec@10 {
-		compatible = "asahi-kasei,ak4613";
-		#sound-dai-cells = <0>;
-		reg = <0x10>;
-		clocks = <&rcar_sound 3>;
-
-		asahi-kasei,in1-single-end;
-		asahi-kasei,in2-single-end;
-		asahi-kasei,out1-single-end;
-		asahi-kasei,out2-single-end;
-		asahi-kasei,out3-single-end;
-		asahi-kasei,out4-single-end;
-		asahi-kasei,out5-single-end;
-		asahi-kasei,out6-single-end;
-	};
-
-	cs2000: clk_multiplier@4f {
-		#clock-cells = <0>;
-		compatible = "cirrus,cs2000-cp";
-		reg = <0x4f>;
-		clocks = <&audio_clkout>, <&x12_clk>;
-		clock-names = "clk_in", "ref_clk";
-
-		assigned-clocks = <&cs2000>;
-		assigned-clock-rates = <24576000>; /* 1/1 divide */
-	};
-};
-
-&rcar_sound {
-	pinctrl-0 = <&sound_pins &sound_clk_pins>;
-	pinctrl-names = "default";
-
-	/* Single DAI */
-	#sound-dai-cells = <0>;
-
-	/* audio_clkout0/1/2/3 */
-	#clock-cells = <1>;
-	clock-frequency = <11289600>;
-
-	status = "okay";
-
-	/* update <audio_clk_b> to <cs2000> */
-	clocks = <&cpg CPG_MOD 1005>,
-		 <&cpg CPG_MOD 1006>, <&cpg CPG_MOD 1007>,
-		 <&cpg CPG_MOD 1008>, <&cpg CPG_MOD 1009>,
-		 <&cpg CPG_MOD 1010>, <&cpg CPG_MOD 1011>,
-		 <&cpg CPG_MOD 1012>, <&cpg CPG_MOD 1013>,
-		 <&cpg CPG_MOD 1014>, <&cpg CPG_MOD 1015>,
-		 <&cpg CPG_MOD 1022>, <&cpg CPG_MOD 1023>,
-		 <&cpg CPG_MOD 1024>, <&cpg CPG_MOD 1025>,
-		 <&cpg CPG_MOD 1026>, <&cpg CPG_MOD 1027>,
-		 <&cpg CPG_MOD 1028>, <&cpg CPG_MOD 1029>,
-		 <&cpg CPG_MOD 1030>, <&cpg CPG_MOD 1031>,
-		 <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>,
-		 <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>,
-		 <&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>,
-		 <&audio_clk_a>, <&cs2000>,
-		 <&audio_clk_c>,
-		 <&cpg CPG_CORE R8A7795_CLK_S0D4>;
-
-	rcar_sound,dai {
-		dai0 {
-			playback = <&ssi0 &src0 &dvc0>;
-			capture  = <&ssi1 &src1 &dvc1>;
-		};
-	};
-};
-
 &sata {
 	status = "okay";
 };
 
-&sdhi0 {
-	pinctrl-0 = <&sdhi0_pins>;
-	pinctrl-1 = <&sdhi0_pins_uhs>;
-	pinctrl-names = "default", "state_uhs";
-
-	vmmc-supply = <&vcc_sdhi0>;
-	vqmmc-supply = <&vccq_sdhi0>;
-	cd-gpios = <&gpio3 12 GPIO_ACTIVE_LOW>;
-	wp-gpios = <&gpio3 13 GPIO_ACTIVE_HIGH>;
-	bus-width = <4>;
-	sd-uhs-sdr50;
-	status = "okay";
-};
-
-&sdhi2 {
-	/* used for on-board 8bit eMMC */
-	pinctrl-0 = <&sdhi2_pins>;
-	pinctrl-1 = <&sdhi2_pins_uhs>;
-	pinctrl-names = "default", "state_uhs";
-
-	vmmc-supply = <&reg_3p3v>;
-	vqmmc-supply = <&reg_1p8v>;
-	bus-width = <8>;
-	mmc-hs200-1_8v;
-	non-removable;
-	status = "okay";
-};
-
-&sdhi3 {
-	pinctrl-0 = <&sdhi3_pins>;
-	pinctrl-1 = <&sdhi3_pins_uhs>;
-	pinctrl-names = "default", "state_uhs";
-
-	vmmc-supply = <&vcc_sdhi3>;
-	vqmmc-supply = <&vccq_sdhi3>;
-	cd-gpios = <&gpio4 15 GPIO_ACTIVE_LOW>;
-	wp-gpios = <&gpio4 16 GPIO_ACTIVE_HIGH>;
-	bus-width = <4>;
-	sd-uhs-sdr50;
-	status = "okay";
-};
-
-&ssi1 {
-	shared-pin;
-};
-
-&wdt0 {
-	timeout-sec = <60>;
-	status = "okay";
-};
-
-&audio_clk_a {
-	clock-frequency = <22579200>;
-};
-
-&i2c_dvfs {
-	status = "okay";
-};
-
-&avb {
-	pinctrl-0 = <&avb_pins>;
-	pinctrl-names = "default";
-	renesas,no-ether-link;
-	phy-handle = <&phy0>;
-	status = "okay";
-
-	phy0: ethernet-phy@0 {
-		rxc-skew-ps = <1500>;
-		reg = <0>;
-		interrupt-parent = <&gpio2>;
-		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
-	};
-};
-
-&xhci0 {
-	status = "okay";
-};
-
-&usb2_phy0 {
-	pinctrl-0 = <&usb0_pins>;
-	pinctrl-names = "default";
-
-	vbus-supply = <&vbus0_usb2>;
-	status = "okay";
-};
-
-&usb2_phy1 {
-	pinctrl-0 = <&usb1_pins>;
-	pinctrl-names = "default";
-
-	status = "okay";
-};
-
 &usb2_phy2 {
 	pinctrl-0 = <&usb2_pins>;
 	pinctrl-names = "default";
 
 	status = "okay";
 };
-
-&ehci0 {
-	status = "okay";
-};
-
-&ehci1 {
-	status = "okay";
-};
-
-&ehci2 {
-	status = "okay";
-};
-
-&ohci0 {
-	status = "okay";
-};
-
-&ohci1 {
-	status = "okay";
-};
-
-&ohci2 {
-	status = "okay";
-};
-
-&hsusb {
-	status = "okay";
-};
-
-&pcie_bus_clk {
-	clock-frequency = <100000000>;
-};
-
-&pciec0 {
-	status = "okay";
-};
-
-&pciec1 {
-	status = "okay";
-};
diff --git a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
index 8d9814f91ef4a345..c8e715768ca82b45 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
@@ -8,266 +8,13 @@
  * kind, whether express or implied.
  */
 
+#define CPG_AUDIO_CLK_I		R8A7796_CLK_S0D4
+
 /dts-v1/;
 #include "r8j7796-2x2g.dtsi"
-#include <dt-bindings/gpio/gpio.h>
+#include "salvator-x.dtsi"
 
 / {
 	model = "Renesas Salvator-X board based on r8j7796";
 	compatible = "renesas,salvator-x", "renesas,r8j7796", "renesas,r8a7796";
-
-	aliases {
-		serial0 = &scif2;
-		serial1 = &scif1;
-		ethernet0 = &avb;
-	};
-
-	chosen {
-		bootargs = "ignore_loglevel rw root=/dev/nfs ip=dhcp";
-		stdout-path = "serial0:115200n8";
-	};
-
-	reg_1p8v: regulator0 {
-		compatible = "regulator-fixed";
-		regulator-name = "fixed-1.8V";
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <1800000>;
-		regulator-boot-on;
-		regulator-always-on;
-	};
-
-	reg_3p3v: regulator1 {
-		compatible = "regulator-fixed";
-		regulator-name = "fixed-3.3V";
-		regulator-min-microvolt = <3300000>;
-		regulator-max-microvolt = <3300000>;
-		regulator-boot-on;
-		regulator-always-on;
-	};
-
-	vcc_sdhi0: regulator-vcc-sdhi0 {
-		compatible = "regulator-fixed";
-
-		regulator-name = "SDHI0 Vcc";
-		regulator-min-microvolt = <3300000>;
-		regulator-max-microvolt = <3300000>;
-
-		gpio = <&gpio5 2 GPIO_ACTIVE_HIGH>;
-		enable-active-high;
-	};
-
-	vccq_sdhi0: regulator-vccq-sdhi0 {
-		compatible = "regulator-gpio";
-
-		regulator-name = "SDHI0 VccQ";
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <3300000>;
-
-		gpios = <&gpio5 1 GPIO_ACTIVE_HIGH>;
-		gpios-states = <1>;
-		states = <3300000 1
-			  1800000 0>;
-	};
-
-	vcc_sdhi3: regulator-vcc-sdhi3 {
-		compatible = "regulator-fixed";
-
-		regulator-name = "SDHI3 Vcc";
-		regulator-min-microvolt = <3300000>;
-		regulator-max-microvolt = <3300000>;
-
-		gpio = <&gpio3 15 GPIO_ACTIVE_HIGH>;
-		enable-active-high;
-	};
-
-	vccq_sdhi3: regulator-vccq-sdhi3 {
-		compatible = "regulator-gpio";
-
-		regulator-name = "SDHI3 VccQ";
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <3300000>;
-
-		gpios = <&gpio3 14 GPIO_ACTIVE_HIGH>;
-		gpios-states = <1>;
-		states = <3300000 1
-			  1800000 0>;
-	};
-};
-
-&pfc {
-	pinctrl-0 = <&scif_clk_pins>;
-	pinctrl-names = "default";
-
-	avb_pins: avb {
-		mux {
-			groups = "avb_link", "avb_phy_int", "avb_mdc",
-				 "avb_mii";
-			function = "avb";
-		};
-
-		pins_mdc {
-			groups = "avb_mdc";
-			drive-strength = <24>;
-		};
-
-		pins_mii_tx {
-			pins = "PIN_AVB_TX_CTL", "PIN_AVB_TXC", "PIN_AVB_TD0",
-			       "PIN_AVB_TD1", "PIN_AVB_TD2", "PIN_AVB_TD3";
-			drive-strength = <12>;
-		};
-	};
-
-	scif1_pins: scif1 {
-		groups = "scif1_data_a", "scif1_ctrl";
-		function = "scif1";
-	};
-
-	scif2_pins: scif2 {
-		groups = "scif2_data_a";
-		function = "scif2";
-	};
-	scif_clk_pins: scif_clk {
-		groups = "scif_clk_a";
-		function = "scif_clk";
-	};
-
-	i2c2_pins: i2c2 {
-		groups = "i2c2_a";
-		function = "i2c2";
-	};
-
-	sdhi0_pins: sd0 {
-		groups = "sdhi0_data4", "sdhi0_ctrl";
-		function = "sdhi0";
-		power-source = <3300>;
-	};
-
-	sdhi0_pins_uhs: sd0_uhs {
-		groups = "sdhi0_data4", "sdhi0_ctrl";
-		function = "sdhi0";
-		power-source = <1800>;
-	};
-
-	sdhi2_pins: sd2 {
-		groups = "sdhi2_data8", "sdhi2_ctrl";
-		function = "sdhi2";
-		power-source = <3300>;
-	};
-
-	sdhi2_pins_uhs: sd2_uhs {
-		groups = "sdhi2_data8", "sdhi2_ctrl";
-		function = "sdhi2";
-		power-source = <1800>;
-	};
-
-	sdhi3_pins: sd3 {
-		groups = "sdhi3_data4", "sdhi3_ctrl";
-		function = "sdhi3";
-		power-source = <3300>;
-	};
-
-	sdhi3_pins_uhs: sd3_uhs {
-		groups = "sdhi3_data4", "sdhi3_ctrl";
-		function = "sdhi3";
-		power-source = <1800>;
-	};
-};
-
-&avb {
-	pinctrl-0 = <&avb_pins>;
-	pinctrl-names = "default";
-	renesas,no-ether-link;
-	phy-handle = <&phy0>;
-	status = "okay";
-
-	phy0: ethernet-phy@0 {
-		rxc-skew-ps = <1500>;
-		reg = <0>;
-		interrupt-parent = <&gpio2>;
-		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
-	};
-};
-
-&extal_clk {
-	clock-frequency = <16666666>;
-};
-
-&extalr_clk {
-	clock-frequency = <32768>;
-};
-
-&sdhi0 {
-	pinctrl-0 = <&sdhi0_pins>;
-	pinctrl-1 = <&sdhi0_pins_uhs>;
-	pinctrl-names = "default", "state_uhs";
-
-	vmmc-supply = <&vcc_sdhi0>;
-	vqmmc-supply = <&vccq_sdhi0>;
-	cd-gpios = <&gpio3 12 GPIO_ACTIVE_LOW>;
-	wp-gpios = <&gpio3 13 GPIO_ACTIVE_HIGH>;
-	bus-width = <4>;
-	sd-uhs-sdr50;
-	status = "okay";
-};
-
-&sdhi2 {
-	/* used for on-board 8bit eMMC */
-	pinctrl-0 = <&sdhi2_pins>;
-	pinctrl-1 = <&sdhi2_pins_uhs>;
-	pinctrl-names = "default", "state_uhs";
-
-	vmmc-supply = <&reg_3p3v>;
-	vqmmc-supply = <&reg_1p8v>;
-	bus-width = <8>;
-	mmc-hs200-1_8v;
-	non-removable;
-	status = "okay";
-};
-
-&sdhi3 {
-	pinctrl-0 = <&sdhi3_pins>;
-	pinctrl-1 = <&sdhi3_pins_uhs>;
-	pinctrl-names = "default", "state_uhs";
-
-	vmmc-supply = <&vcc_sdhi3>;
-	vqmmc-supply = <&vccq_sdhi3>;
-	cd-gpios = <&gpio4 15 GPIO_ACTIVE_LOW>;
-	wp-gpios = <&gpio4 16 GPIO_ACTIVE_HIGH>;
-	bus-width = <4>;
-	sd-uhs-sdr50;
-	status = "okay";
-};
-
-&scif1 {
-	pinctrl-0 = <&scif1_pins>;
-	pinctrl-names = "default";
-
-	uart-has-rtscts;
-	status = "okay";
-};
-
-&scif2 {
-	pinctrl-0 = <&scif2_pins>;
-	pinctrl-names = "default";
-	status = "okay";
-};
-
-&scif_clk {
-	clock-frequency = <14745600>;
-};
-
-&i2c2 {
-	pinctrl-0 = <&i2c2_pins>;
-	pinctrl-names = "default";
-
-	status = "okay";
-};
-
-&wdt0 {
-	timeout-sec = <60>;
-	status = "okay";
-};
-
-&i2c_dvfs {
-	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts b/arch/arm64/boot/dts/renesas/salvator-x.dtsi
similarity index 95%
copy from arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
copy to arch/arm64/boot/dts/renesas/salvator-x.dtsi
index e5b9409bf2d218d8..47a482f20c9d511f 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/salvator-x.dtsi
@@ -1,7 +1,7 @@
 /*
  * Device Tree Source for the Salvator-X board
  *
- * Copyright (C) 2015 Renesas Electronics Corp.
+ * Copyright (C) 2015-2016 Renesas Electronics Corp.
  *
  * This file is licensed under the terms of the GNU General Public License
  * version 2.  This program is licensed "as is" without any warranty of any
@@ -31,13 +31,11 @@
  *	amixer set "DVC Out" 100%  // Volume Up
  */
 
-/dts-v1/;
-#include "r8j7795-4x1g.dtsi"
 #include <dt-bindings/gpio/gpio.h>
 
 / {
-	model = "Renesas Salvator-X board based on r8j7795";
-	compatible = "renesas,salvator-x", "renesas,r8j7795", "renesas,r8a7795";
+	model = "Renesas Salvator-X board";
+	compatible = "renesas,salvator-x";
 
 	aliases {
 		serial0 = &scif2;
@@ -50,10 +48,14 @@
 		stdout-path = "serial0:115200n8";
 	};
 
-	x12_clk: x12 {
+	audio_clkout: audio_clkout {
+		/*
+		 * This is same as <&rcar_sound 0>
+		 * but needed to avoid cs2000/rcar_sound probe dead-lock
+		 */
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
-		clock-frequency = <24576000>;
+		clock-frequency = <11289600>;
 	};
 
 	reg_1p8v: regulator0 {
@@ -74,6 +76,33 @@
 		regulator-always-on;
 	};
 
+	rsnd_ak4613: sound {
+		compatible = "simple-audio-card";
+
+		simple-audio-card,format = "left_j";
+		simple-audio-card,bitclock-master = <&sndcpu>;
+		simple-audio-card,frame-master = <&sndcpu>;
+
+		sndcpu: simple-audio-card,cpu {
+			sound-dai = <&rcar_sound>;
+		};
+
+		sndcodec: simple-audio-card,codec {
+			sound-dai = <&ak4613>;
+		};
+	};
+
+	vbus0_usb2: regulator-vbus0-usb2 {
+		compatible = "regulator-fixed";
+
+		regulator-name = "USB20_VBUS0";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+
+		gpio = <&gpio6 16 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
+
 	vcc_sdhi0: regulator-vcc-sdhi0 {
 		compatible = "regulator-fixed";
 
@@ -122,40 +151,13 @@
 			  1800000 0>;
 	};
 
-	vbus0_usb2: regulator-vbus0-usb2 {
-		compatible = "regulator-fixed";
-
-		regulator-name = "USB20_VBUS0";
-		regulator-min-microvolt = <5000000>;
-		regulator-max-microvolt = <5000000>;
-
-		gpio = <&gpio6 16 GPIO_ACTIVE_HIGH>;
-		enable-active-high;
-	};
-
-	audio_clkout: audio_clkout {
-		/*
-		 * This is same as <&rcar_sound 0>
-		 * but needed to avoid cs2000/rcar_sound probe dead-lock
-		 */
-		compatible = "fixed-clock";
-		#clock-cells = <0>;
-		clock-frequency = <11289600>;
-	};
-
-	rsnd_ak4613: sound {
-		compatible = "simple-audio-card";
-
-		simple-audio-card,format = "left_j";
-		simple-audio-card,bitclock-master = <&sndcpu>;
-		simple-audio-card,frame-master = <&sndcpu>;
-
-		sndcpu: simple-audio-card,cpu {
-			sound-dai = <&rcar_sound>;
-		};
+	vga {
+		compatible = "vga-connector";
 
-		sndcodec: simple-audio-card,codec {
-			sound-dai = <&ak4613>;
+		port {
+			vga_in: endpoint {
+				remote-endpoint = <&adv7123_out>;
+			};
 		};
 	};
 
@@ -181,14 +183,29 @@
 		};
 	};
 
-	vga {
-		compatible = "vga-connector";
+	x12_clk: x12 {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <24576000>;
+	};
+};
 
-		port {
-			vga_in: endpoint {
-				remote-endpoint = <&adv7123_out>;
-			};
-		};
+&audio_clk_a {
+	clock-frequency = <22579200>;
+};
+
+&avb {
+	pinctrl-0 = <&avb_pins>;
+	pinctrl-names = "default";
+	renesas,no-ether-link;
+	phy-handle = <&phy0>;
+	status = "okay";
+
+	phy0: ethernet-phy@0 {
+		rxc-skew-ps = <1500>;
+		reg = <0>;
+		interrupt-parent = <&gpio2>;
+		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
 	};
 };
 
@@ -210,6 +227,14 @@
 	};
 };
 
+&ehci0 {
+	status = "okay";
+};
+
+&ehci1 {
+	status = "okay";
+};
+
 &extal_clk {
 	clock-frequency = <16666666>;
 };
@@ -218,27 +243,73 @@
 	clock-frequency = <32768>;
 };
 
-&pfc {
-	pinctrl-0 = <&scif_clk_pins>;
+&hsusb {
+	status = "okay";
+};
+
+&i2c2 {
+	pinctrl-0 = <&i2c2_pins>;
 	pinctrl-names = "default";
 
-	scif1_pins: scif1 {
-		groups = "scif1_data_a", "scif1_ctrl";
-		function = "scif1";
-	};
-	scif2_pins: scif2 {
-		groups = "scif2_data_a";
-		function = "scif2";
-	};
-	scif_clk_pins: scif_clk {
-		groups = "scif_clk_a";
-		function = "scif_clk";
+	status = "okay";
+
+	clock-frequency = <100000>;
+
+	ak4613: codec@10 {
+		compatible = "asahi-kasei,ak4613";
+		#sound-dai-cells = <0>;
+		reg = <0x10>;
+		clocks = <&rcar_sound 3>;
+
+		asahi-kasei,in1-single-end;
+		asahi-kasei,in2-single-end;
+		asahi-kasei,out1-single-end;
+		asahi-kasei,out2-single-end;
+		asahi-kasei,out3-single-end;
+		asahi-kasei,out4-single-end;
+		asahi-kasei,out5-single-end;
+		asahi-kasei,out6-single-end;
 	};
 
-	i2c2_pins: i2c2 {
-		groups = "i2c2_a";
-		function = "i2c2";
+	cs2000: clk_multiplier@4f {
+		#clock-cells = <0>;
+		compatible = "cirrus,cs2000-cp";
+		reg = <0x4f>;
+		clocks = <&audio_clkout>, <&x12_clk>;
+		clock-names = "clk_in", "ref_clk";
+
+		assigned-clocks = <&cs2000>;
+		assigned-clock-rates = <24576000>; /* 1/1 divide */
 	};
+};
+
+&i2c_dvfs {
+	status = "okay";
+};
+
+&ohci0 {
+	status = "okay";
+};
+
+&ohci1 {
+	status = "okay";
+};
+
+&pcie_bus_clk {
+	clock-frequency = <100000000>;
+};
+
+&pciec0 {
+	status = "okay";
+};
+
+&pciec1 {
+	status = "okay";
+};
+
+&pfc {
+	pinctrl-0 = <&scif_clk_pins>;
+	pinctrl-names = "default";
 
 	avb_pins: avb {
 		mux {
@@ -264,6 +335,26 @@
 		function = "du";
 	};
 
+	i2c2_pins: i2c2 {
+		groups = "i2c2_a";
+		function = "i2c2";
+	};
+
+	scif1_pins: scif1 {
+		groups = "scif1_data_a", "scif1_ctrl";
+		function = "scif1";
+	};
+
+	scif2_pins: scif2 {
+		groups = "scif2_data_a";
+		function = "scif2";
+	};
+
+	scif_clk_pins: scif_clk {
+		groups = "scif_clk_a";
+		function = "scif_clk";
+	};
+
 	sdhi0_pins: sd0 {
 		groups = "sdhi0_data4", "sdhi0_ctrl";
 		function = "sdhi0";
@@ -332,66 +423,6 @@
 			bias-pull-down;
 		};
 	};
-
-	usb2_pins: usb2 {
-		groups = "usb2";
-		function = "usb2";
-	};
-};
-
-&scif1 {
-	pinctrl-0 = <&scif1_pins>;
-	pinctrl-names = "default";
-
-	uart-has-rtscts;
-	status = "okay";
-};
-
-&scif2 {
-	pinctrl-0 = <&scif2_pins>;
-	pinctrl-names = "default";
-
-	status = "okay";
-};
-
-&scif_clk {
-	clock-frequency = <14745600>;
-};
-
-&i2c2 {
-	pinctrl-0 = <&i2c2_pins>;
-	pinctrl-names = "default";
-
-	status = "okay";
-
-	clock-frequency = <100000>;
-
-	ak4613: codec@10 {
-		compatible = "asahi-kasei,ak4613";
-		#sound-dai-cells = <0>;
-		reg = <0x10>;
-		clocks = <&rcar_sound 3>;
-
-		asahi-kasei,in1-single-end;
-		asahi-kasei,in2-single-end;
-		asahi-kasei,out1-single-end;
-		asahi-kasei,out2-single-end;
-		asahi-kasei,out3-single-end;
-		asahi-kasei,out4-single-end;
-		asahi-kasei,out5-single-end;
-		asahi-kasei,out6-single-end;
-	};
-
-	cs2000: clk_multiplier@4f {
-		#clock-cells = <0>;
-		compatible = "cirrus,cs2000-cp";
-		reg = <0x4f>;
-		clocks = <&audio_clkout>, <&x12_clk>;
-		clock-names = "clk_in", "ref_clk";
-
-		assigned-clocks = <&cs2000>;
-		assigned-clock-rates = <24576000>; /* 1/1 divide */
-	};
 };
 
 &rcar_sound {
@@ -424,7 +455,7 @@
 		 <&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>,
 		 <&audio_clk_a>, <&cs2000>,
 		 <&audio_clk_c>,
-		 <&cpg CPG_CORE R8A7795_CLK_S0D4>;
+		 <&cpg CPG_CORE CPG_AUDIO_CLK_I>;
 
 	rcar_sound,dai {
 		dai0 {
@@ -434,10 +465,25 @@
 	};
 };
 
-&sata {
+&scif1 {
+	pinctrl-0 = <&scif1_pins>;
+	pinctrl-names = "default";
+
+	uart-has-rtscts;
+	status = "okay";
+};
+
+&scif2 {
+	pinctrl-0 = <&scif2_pins>;
+	pinctrl-names = "default";
+
 	status = "okay";
 };
 
+&scif_clk {
+	clock-frequency = <14745600>;
+};
+
 &sdhi0 {
 	pinctrl-0 = <&sdhi0_pins>;
 	pinctrl-1 = <&sdhi0_pins_uhs>;
@@ -484,38 +530,6 @@
 	shared-pin;
 };
 
-&wdt0 {
-	timeout-sec = <60>;
-	status = "okay";
-};
-
-&audio_clk_a {
-	clock-frequency = <22579200>;
-};
-
-&i2c_dvfs {
-	status = "okay";
-};
-
-&avb {
-	pinctrl-0 = <&avb_pins>;
-	pinctrl-names = "default";
-	renesas,no-ether-link;
-	phy-handle = <&phy0>;
-	status = "okay";
-
-	phy0: ethernet-phy@0 {
-		rxc-skew-ps = <1500>;
-		reg = <0>;
-		interrupt-parent = <&gpio2>;
-		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
-	};
-};
-
-&xhci0 {
-	status = "okay";
-};
-
 &usb2_phy0 {
 	pinctrl-0 = <&usb0_pins>;
 	pinctrl-names = "default";
@@ -531,49 +545,11 @@
 	status = "okay";
 };
 
-&usb2_phy2 {
-	pinctrl-0 = <&usb2_pins>;
-	pinctrl-names = "default";
-
-	status = "okay";
-};
-
-&ehci0 {
-	status = "okay";
-};
-
-&ehci1 {
-	status = "okay";
-};
-
-&ehci2 {
-	status = "okay";
-};
-
-&ohci0 {
-	status = "okay";
-};
-
-&ohci1 {
-	status = "okay";
-};
-
-&ohci2 {
-	status = "okay";
-};
-
-&hsusb {
-	status = "okay";
-};
-
-&pcie_bus_clk {
-	clock-frequency = <100000000>;
-};
-
-&pciec0 {
+&wdt0 {
+	timeout-sec = <60>;
 	status = "okay";
 };
 
-&pciec1 {
+&xhci0 {
 	status = "okay";
 };
-- 
2.7.4

^ permalink raw reply related

* [PATCH/RFC 5/5] arm64: dts: renesas: Extract common ULCB board support
From: Geert Uytterhoeven @ 2017-04-21 12:55 UTC (permalink / raw)
  To: Simon Horman, Magnus Damm, Kuninori Morimoto, Yoshihiro Shimoda,
	Rob Herring, Mark Rutland
  Cc: linux-renesas-soc, devicetree, linux-arm-kernel,
	Geert Uytterhoeven
In-Reply-To: <1492779321-23939-1-git-send-email-geert+renesas@glider.be>

The Renesas ULCB development board can be equipped with either an R-Car
H3 or M3-W SiP, which are pin-compatible.  Both boards use different
DTBs.

Reduce duplication by extracting common ULCB board support into its own
.dtsi file.  References to SoC-specific clocks are handled through cpp
definitions.  Sort device nodes while at it.

For H3ULCB, there are no functional changes.

For M3ULCB, the following new devices are now described in DT:
  - External audio, CAN, and PCIe clocks,
  - CS2000 clock generator,
  - AK4613 Audio Codec.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Untested due to lack of hardware, but dtx_diff is my friend.
---
 arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts     | 341 +--------------------
 arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts     | 201 +-----------
 .../dts/renesas/{r8a7795-h3ulcb.dts => ulcb.dtsi}  | 243 ++++++++-------
 3 files changed, 126 insertions(+), 659 deletions(-)
 copy arch/arm64/boot/dts/renesas/{r8a7795-h3ulcb.dts => ulcb.dtsi} (96%)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts b/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts
index fe7eca39490eb1a2..534a9d1bf2e6b1fd 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts
@@ -9,348 +9,13 @@
  * kind, whether express or implied.
  */
 
+#define CPG_AUDIO_CLK_I	R8A7795_CLK_S0D4
+
 /dts-v1/;
 #include "r8j7795-4x1g.dtsi"
-#include <dt-bindings/gpio/gpio.h>
-#include <dt-bindings/input/input.h>
+#include "ulcb.dtsi"
 
 / {
 	model = "Renesas H3ULCB board based on r8j7795";
 	compatible = "renesas,h3ulcb", "renesas,r8j7795", "renesas,r8a7795";
-
-	aliases {
-		serial0 = &scif2;
-		ethernet0 = &avb;
-	};
-
-	chosen {
-		stdout-path = "serial0:115200n8";
-	};
-
-	leds {
-		compatible = "gpio-leds";
-
-		led5 {
-			gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;
-		};
-		led6 {
-			gpios = <&gpio6 13 GPIO_ACTIVE_HIGH>;
-		};
-	};
-
-	keyboard {
-		compatible = "gpio-keys";
-
-		key-1 {
-			linux,code = <KEY_1>;
-			label = "SW3";
-			wakeup-source;
-			debounce-interval = <20>;
-			gpios = <&gpio6 11 GPIO_ACTIVE_LOW>;
-		};
-	};
-
-	x12_clk: x12 {
-		compatible = "fixed-clock";
-		#clock-cells = <0>;
-		clock-frequency = <24576000>;
-	};
-
-	reg_1p8v: regulator0 {
-		compatible = "regulator-fixed";
-		regulator-name = "fixed-1.8V";
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <1800000>;
-		regulator-boot-on;
-		regulator-always-on;
-	};
-
-	reg_3p3v: regulator1 {
-		compatible = "regulator-fixed";
-		regulator-name = "fixed-3.3V";
-		regulator-min-microvolt = <3300000>;
-		regulator-max-microvolt = <3300000>;
-		regulator-boot-on;
-		regulator-always-on;
-	};
-
-	vcc_sdhi0: regulator-vcc-sdhi0 {
-		compatible = "regulator-fixed";
-
-		regulator-name = "SDHI0 Vcc";
-		regulator-min-microvolt = <3300000>;
-		regulator-max-microvolt = <3300000>;
-
-		gpio = <&gpio5 2 GPIO_ACTIVE_HIGH>;
-		enable-active-high;
-	};
-
-	vccq_sdhi0: regulator-vccq-sdhi0 {
-		compatible = "regulator-gpio";
-
-		regulator-name = "SDHI0 VccQ";
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <3300000>;
-
-		gpios = <&gpio5 1 GPIO_ACTIVE_HIGH>;
-		gpios-states = <1>;
-		states = <3300000 1
-			  1800000 0>;
-	};
-
-	audio_clkout: audio-clkout {
-		/*
-		 * This is same as <&rcar_sound 0>
-		 * but needed to avoid cs2000/rcar_sound probe dead-lock
-		 */
-		compatible = "fixed-clock";
-		#clock-cells = <0>;
-		clock-frequency = <11289600>;
-	};
-
-	rsnd_ak4613: sound {
-		compatible = "simple-audio-card";
-
-		simple-audio-card,format = "left_j";
-		simple-audio-card,bitclock-master = <&sndcpu>;
-		simple-audio-card,frame-master = <&sndcpu>;
-
-		sndcpu: simple-audio-card,cpu {
-			sound-dai = <&rcar_sound>;
-		};
-
-		sndcodec: simple-audio-card,codec {
-			sound-dai = <&ak4613>;
-		};
-	};
-};
-
-&extal_clk {
-	clock-frequency = <16666666>;
-};
-
-&extalr_clk {
-	clock-frequency = <32768>;
-};
-
-&pfc {
-	pinctrl-0 = <&scif_clk_pins>;
-	pinctrl-names = "default";
-
-	scif2_pins: scif2 {
-		groups = "scif2_data_a";
-		function = "scif2";
-	};
-
-	scif_clk_pins: scif_clk {
-		groups = "scif_clk_a";
-		function = "scif_clk";
-	};
-
-	i2c2_pins: i2c2 {
-		groups = "i2c2_a";
-		function = "i2c2";
-	};
-
-	avb_pins: avb {
-		groups = "avb_mdc";
-		function = "avb";
-	};
-
-	sdhi0_pins: sd0 {
-		groups = "sdhi0_data4", "sdhi0_ctrl";
-		function = "sdhi0";
-		power-source = <3300>;
-	};
-
-	sdhi0_pins_uhs: sd0_uhs {
-		groups = "sdhi0_data4", "sdhi0_ctrl";
-		function = "sdhi0";
-		power-source = <1800>;
-	};
-
-	sdhi2_pins: sd2 {
-		groups = "sdhi2_data8", "sdhi2_ctrl";
-		function = "sdhi2";
-		power-source = <3300>;
-	};
-
-	sdhi2_pins_uhs: sd2_uhs {
-		groups = "sdhi2_data8", "sdhi2_ctrl";
-		function = "sdhi2";
-		power-source = <1800>;
-	};
-
-	sound_pins: sound {
-		groups = "ssi01239_ctrl", "ssi0_data", "ssi1_data_a";
-		function = "ssi";
-	};
-
-	sound_clk_pins: sound-clk {
-		groups = "audio_clk_a_a", "audio_clk_b_a", "audio_clk_c_a",
-			 "audio_clkout_a", "audio_clkout3_a";
-		function = "audio_clk";
-	};
-
-	usb1_pins: usb1 {
-		groups = "usb1";
-		function = "usb1";
-	};
-};
-
-&scif2 {
-	pinctrl-0 = <&scif2_pins>;
-	pinctrl-names = "default";
-
-	status = "okay";
-};
-
-&scif_clk {
-	clock-frequency = <14745600>;
-};
-
-&i2c2 {
-	pinctrl-0 = <&i2c2_pins>;
-	pinctrl-names = "default";
-
-	status = "okay";
-
-	clock-frequency = <100000>;
-
-	ak4613: codec@10 {
-		compatible = "asahi-kasei,ak4613";
-		#sound-dai-cells = <0>;
-		reg = <0x10>;
-		clocks = <&rcar_sound 3>;
-
-		asahi-kasei,in1-single-end;
-		asahi-kasei,in2-single-end;
-		asahi-kasei,out1-single-end;
-		asahi-kasei,out2-single-end;
-		asahi-kasei,out3-single-end;
-		asahi-kasei,out4-single-end;
-		asahi-kasei,out5-single-end;
-		asahi-kasei,out6-single-end;
-	};
-
-	cs2000: clk-multiplier@4f {
-		#clock-cells = <0>;
-		compatible = "cirrus,cs2000-cp";
-		reg = <0x4f>;
-		clocks = <&audio_clkout>, <&x12_clk>;
-		clock-names = "clk_in", "ref_clk";
-
-		assigned-clocks = <&cs2000>;
-		assigned-clock-rates = <24576000>; /* 1/1 divide */
-	};
-};
-
-&rcar_sound {
-	pinctrl-0 = <&sound_pins &sound_clk_pins>;
-	pinctrl-names = "default";
-
-	/* Single DAI */
-	#sound-dai-cells = <0>;
-
-	/* audio_clkout0/1/2/3 */
-	#clock-cells = <1>;
-	clock-frequency = <11289600>;
-
-	status = "okay";
-
-	/* update <audio_clk_b> to <cs2000> */
-	clocks = <&cpg CPG_MOD 1005>,
-		 <&cpg CPG_MOD 1006>, <&cpg CPG_MOD 1007>,
-		 <&cpg CPG_MOD 1008>, <&cpg CPG_MOD 1009>,
-		 <&cpg CPG_MOD 1010>, <&cpg CPG_MOD 1011>,
-		 <&cpg CPG_MOD 1012>, <&cpg CPG_MOD 1013>,
-		 <&cpg CPG_MOD 1014>, <&cpg CPG_MOD 1015>,
-		 <&cpg CPG_MOD 1022>, <&cpg CPG_MOD 1023>,
-		 <&cpg CPG_MOD 1024>, <&cpg CPG_MOD 1025>,
-		 <&cpg CPG_MOD 1026>, <&cpg CPG_MOD 1027>,
-		 <&cpg CPG_MOD 1028>, <&cpg CPG_MOD 1029>,
-		 <&cpg CPG_MOD 1030>, <&cpg CPG_MOD 1031>,
-		 <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>,
-		 <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>,
-		 <&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>,
-		 <&audio_clk_a>, <&cs2000>,
-		 <&audio_clk_c>,
-		 <&cpg CPG_CORE R8A7795_CLK_S0D4>;
-
-	rcar_sound,dai {
-		dai0 {
-			playback = <&ssi0 &src0 &dvc0>;
-			capture  = <&ssi1 &src1 &dvc1>;
-		};
-	};
-};
-
-&sdhi0 {
-	pinctrl-0 = <&sdhi0_pins>;
-	pinctrl-1 = <&sdhi0_pins_uhs>;
-	pinctrl-names = "default", "state_uhs";
-
-	vmmc-supply = <&vcc_sdhi0>;
-	vqmmc-supply = <&vccq_sdhi0>;
-	cd-gpios = <&gpio3 12 GPIO_ACTIVE_LOW>;
-	bus-width = <4>;
-	sd-uhs-sdr50;
-	status = "okay";
-};
-
-&sdhi2 {
-	/* used for on-board 8bit eMMC */
-	pinctrl-0 = <&sdhi2_pins>;
-	pinctrl-1 = <&sdhi2_pins_uhs>;
-	pinctrl-names = "default", "state_uhs";
-
-	vmmc-supply = <&reg_3p3v>;
-	vqmmc-supply = <&reg_1p8v>;
-	bus-width = <8>;
-	mmc-hs200-1_8v;
-	non-removable;
-	status = "okay";
-};
-
-&ssi1 {
-	shared-pin;
-};
-
-&wdt0 {
-	timeout-sec = <60>;
-	status = "okay";
-};
-
-&audio_clk_a {
-	clock-frequency = <22579200>;
-};
-
-&avb {
-	pinctrl-0 = <&avb_pins>;
-	pinctrl-names = "default";
-	renesas,no-ether-link;
-	phy-handle = <&phy0>;
-	status = "okay";
-
-	phy0: ethernet-phy@0 {
-		rxc-skew-ps = <1500>;
-		reg = <0>;
-		interrupt-parent = <&gpio2>;
-		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
-	};
-};
-
-&usb2_phy1 {
-	pinctrl-0 = <&usb1_pins>;
-	pinctrl-names = "default";
-
-	status = "okay";
-};
-
-&ehci1 {
-	status = "okay";
-};
-
-&ohci1 {
-	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts b/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts
index d25b85dc3344b01f..c01e857216506e06 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts
@@ -9,208 +9,13 @@
  * kind, whether express or implied.
  */
 
+#define CPG_AUDIO_CLK_I	R8A7796_CLK_S0D4
+
 /dts-v1/;
 #include "r8j7796-2x1g.dtsi"
-#include <dt-bindings/gpio/gpio.h>
-#include <dt-bindings/input/input.h>
+#include "ulcb.dtsi"
 
 / {
 	model = "Renesas M3ULCB board based on r8j7796";
 	compatible = "renesas,m3ulcb", "renesas,r8j7796", "renesas,r8a7796";
-
-	aliases {
-		serial0 = &scif2;
-		ethernet0 = &avb;
-	};
-
-	chosen {
-		stdout-path = "serial0:115200n8";
-	};
-
-	leds {
-		compatible = "gpio-leds";
-
-		led5 {
-			gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;
-		};
-		led6 {
-			gpios = <&gpio6 13 GPIO_ACTIVE_HIGH>;
-		};
-	};
-
-	keyboard {
-		compatible = "gpio-keys";
-
-		key-1 {
-			linux,code = <KEY_1>;
-			label = "SW3";
-			wakeup-source;
-			debounce-interval = <20>;
-			gpios = <&gpio6 11 GPIO_ACTIVE_LOW>;
-		};
-	};
-
-	reg_1p8v: regulator0 {
-		compatible = "regulator-fixed";
-		regulator-name = "fixed-1.8V";
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <1800000>;
-		regulator-boot-on;
-		regulator-always-on;
-	};
-
-	reg_3p3v: regulator1 {
-		compatible = "regulator-fixed";
-		regulator-name = "fixed-3.3V";
-		regulator-min-microvolt = <3300000>;
-		regulator-max-microvolt = <3300000>;
-		regulator-boot-on;
-		regulator-always-on;
-	};
-
-	vcc_sdhi0: regulator-vcc-sdhi0 {
-		compatible = "regulator-fixed";
-
-		regulator-name = "SDHI0 Vcc";
-		regulator-min-microvolt = <3300000>;
-		regulator-max-microvolt = <3300000>;
-
-		gpio = <&gpio5 2 GPIO_ACTIVE_HIGH>;
-		enable-active-high;
-	};
-
-	vccq_sdhi0: regulator-vccq-sdhi0 {
-		compatible = "regulator-gpio";
-
-		regulator-name = "SDHI0 VccQ";
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <3300000>;
-
-		gpios = <&gpio5 1 GPIO_ACTIVE_HIGH>;
-		gpios-states = <1>;
-		states = <3300000 1
-			  1800000 0>;
-	};
-};
-
-&extal_clk {
-	clock-frequency = <16666666>;
-};
-
-&extalr_clk {
-	clock-frequency = <32768>;
-};
-
-&pfc {
-	pinctrl-0 = <&scif_clk_pins>;
-	pinctrl-names = "default";
-
-	avb_pins: avb {
-		groups = "avb_mdc";
-		function = "avb";
-	};
-
-	scif2_pins: scif2 {
-		groups = "scif2_data_a";
-		function = "scif2";
-	};
-
-	scif_clk_pins: scif_clk {
-		groups = "scif_clk_a";
-		function = "scif_clk";
-	};
-
-	i2c2_pins: i2c2 {
-		groups = "i2c2_a";
-		function = "i2c2";
-	};
-
-	sdhi0_pins: sd0 {
-		groups = "sdhi0_data4", "sdhi0_ctrl";
-		function = "sdhi0";
-		power-source = <3300>;
-	};
-
-	sdhi0_pins_uhs: sd0_uhs {
-		groups = "sdhi0_data4", "sdhi0_ctrl";
-		function = "sdhi0";
-		power-source = <1800>;
-	};
-
-	sdhi2_pins: sd2 {
-		groups = "sdhi2_data8", "sdhi2_ctrl";
-		function = "sdhi2";
-		power-source = <3300>;
-	};
-
-	sdhi2_pins_uhs: sd2_uhs {
-		groups = "sdhi2_data8", "sdhi2_ctrl";
-		function = "sdhi2";
-		power-source = <1800>;
-	};
-};
-
-&avb {
-	pinctrl-0 = <&avb_pins>;
-	pinctrl-names = "default";
-	renesas,no-ether-link;
-	phy-handle = <&phy0>;
-	status = "okay";
-
-	phy0: ethernet-phy@0 {
-		rxc-skew-ps = <1500>;
-		reg = <0>;
-		interrupt-parent = <&gpio2>;
-		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
-	};
-};
-
-&sdhi0 {
-	pinctrl-0 = <&sdhi0_pins>;
-	pinctrl-1 = <&sdhi0_pins_uhs>;
-	pinctrl-names = "default", "state_uhs";
-
-	vmmc-supply = <&vcc_sdhi0>;
-	vqmmc-supply = <&vccq_sdhi0>;
-	cd-gpios = <&gpio3 12 GPIO_ACTIVE_LOW>;
-	bus-width = <4>;
-	sd-uhs-sdr50;
-	status = "okay";
-};
-
-&sdhi2 {
-	/* used for on-board 8bit eMMC */
-	pinctrl-0 = <&sdhi2_pins>;
-	pinctrl-1 = <&sdhi2_pins_uhs>;
-	pinctrl-names = "default", "state_uhs";
-
-	vmmc-supply = <&reg_3p3v>;
-	vqmmc-supply = <&reg_1p8v>;
-	bus-width = <8>;
-	mmc-hs200-1_8v;
-	non-removable;
-	status = "okay";
-};
-
-&scif2 {
-	pinctrl-0 = <&scif2_pins>;
-	pinctrl-names = "default";
-
-	status = "okay";
-};
-
-&scif_clk {
-	clock-frequency = <14745600>;
-};
-
-&i2c2 {
-	pinctrl-0 = <&i2c2_pins>;
-	pinctrl-names = "default";
-
-	status = "okay";
-};
-
-&wdt0 {
-	timeout-sec = <60>;
-	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts b/arch/arm64/boot/dts/renesas/ulcb.dtsi
similarity index 96%
copy from arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts
copy to arch/arm64/boot/dts/renesas/ulcb.dtsi
index fe7eca39490eb1a2..2bc7ceb2efa45598 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts
+++ b/arch/arm64/boot/dts/renesas/ulcb.dtsi
@@ -1,5 +1,5 @@
 /*
- * Device Tree Source for the H3ULCB (R-Car Starter Kit Premier) board
+ * Device Tree Source for the R-Car Gen3 ULCB board
  *
  * Copyright (C) 2016 Renesas Electronics Corp.
  * Copyright (C) 2016 Cogent Embedded, Inc.
@@ -9,14 +9,11 @@
  * kind, whether express or implied.
  */
 
-/dts-v1/;
-#include "r8j7795-4x1g.dtsi"
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
 
 / {
-	model = "Renesas H3ULCB board based on r8j7795";
-	compatible = "renesas,h3ulcb", "renesas,r8j7795", "renesas,r8a7795";
+	model = "Renesas R-Car Gen3 ULCB board";
 
 	aliases {
 		serial0 = &scif2;
@@ -27,15 +24,14 @@
 		stdout-path = "serial0:115200n8";
 	};
 
-	leds {
-		compatible = "gpio-leds";
-
-		led5 {
-			gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;
-		};
-		led6 {
-			gpios = <&gpio6 13 GPIO_ACTIVE_HIGH>;
-		};
+	audio_clkout: audio-clkout {
+		/*
+		 * This is same as <&rcar_sound 0>
+		 * but needed to avoid cs2000/rcar_sound probe dead-lock
+		 */
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <11289600>;
 	};
 
 	keyboard {
@@ -50,10 +46,15 @@
 		};
 	};
 
-	x12_clk: x12 {
-		compatible = "fixed-clock";
-		#clock-cells = <0>;
-		clock-frequency = <24576000>;
+	leds {
+		compatible = "gpio-leds";
+
+		led5 {
+			gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;
+		};
+		led6 {
+			gpios = <&gpio6 13 GPIO_ACTIVE_HIGH>;
+		};
 	};
 
 	reg_1p8v: regulator0 {
@@ -74,6 +75,22 @@
 		regulator-always-on;
 	};
 
+	rsnd_ak4613: sound {
+		compatible = "simple-audio-card";
+
+		simple-audio-card,format = "left_j";
+		simple-audio-card,bitclock-master = <&sndcpu>;
+		simple-audio-card,frame-master = <&sndcpu>;
+
+		sndcpu: simple-audio-card,cpu {
+			sound-dai = <&rcar_sound>;
+		};
+
+		sndcodec: simple-audio-card,codec {
+			sound-dai = <&ak4613>;
+		};
+	};
+
 	vcc_sdhi0: regulator-vcc-sdhi0 {
 		compatible = "regulator-fixed";
 
@@ -98,33 +115,36 @@
 			  1800000 0>;
 	};
 
-	audio_clkout: audio-clkout {
-		/*
-		 * This is same as <&rcar_sound 0>
-		 * but needed to avoid cs2000/rcar_sound probe dead-lock
-		 */
+	x12_clk: x12 {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
-		clock-frequency = <11289600>;
+		clock-frequency = <24576000>;
 	};
+};
 
-	rsnd_ak4613: sound {
-		compatible = "simple-audio-card";
-
-		simple-audio-card,format = "left_j";
-		simple-audio-card,bitclock-master = <&sndcpu>;
-		simple-audio-card,frame-master = <&sndcpu>;
+&audio_clk_a {
+	clock-frequency = <22579200>;
+};
 
-		sndcpu: simple-audio-card,cpu {
-			sound-dai = <&rcar_sound>;
-		};
+&avb {
+	pinctrl-0 = <&avb_pins>;
+	pinctrl-names = "default";
+	renesas,no-ether-link;
+	phy-handle = <&phy0>;
+	status = "okay";
 
-		sndcodec: simple-audio-card,codec {
-			sound-dai = <&ak4613>;
-		};
+	phy0: ethernet-phy@0 {
+		rxc-skew-ps = <1500>;
+		reg = <0>;
+		interrupt-parent = <&gpio2>;
+		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
 	};
 };
 
+&ehci1 {
+	status = "okay";
+};
+
 &extal_clk {
 	clock-frequency = <16666666>;
 };
@@ -133,10 +153,60 @@
 	clock-frequency = <32768>;
 };
 
+&i2c2 {
+	pinctrl-0 = <&i2c2_pins>;
+	pinctrl-names = "default";
+
+	status = "okay";
+
+	clock-frequency = <100000>;
+
+	ak4613: codec@10 {
+		compatible = "asahi-kasei,ak4613";
+		#sound-dai-cells = <0>;
+		reg = <0x10>;
+		clocks = <&rcar_sound 3>;
+
+		asahi-kasei,in1-single-end;
+		asahi-kasei,in2-single-end;
+		asahi-kasei,out1-single-end;
+		asahi-kasei,out2-single-end;
+		asahi-kasei,out3-single-end;
+		asahi-kasei,out4-single-end;
+		asahi-kasei,out5-single-end;
+		asahi-kasei,out6-single-end;
+	};
+
+	cs2000: clk-multiplier@4f {
+		#clock-cells = <0>;
+		compatible = "cirrus,cs2000-cp";
+		reg = <0x4f>;
+		clocks = <&audio_clkout>, <&x12_clk>;
+		clock-names = "clk_in", "ref_clk";
+
+		assigned-clocks = <&cs2000>;
+		assigned-clock-rates = <24576000>; /* 1/1 divide */
+	};
+};
+
+&ohci1 {
+	status = "okay";
+};
+
 &pfc {
 	pinctrl-0 = <&scif_clk_pins>;
 	pinctrl-names = "default";
 
+	avb_pins: avb {
+		groups = "avb_mdc";
+		function = "avb";
+	};
+
+	i2c2_pins: i2c2 {
+		groups = "i2c2_a";
+		function = "i2c2";
+	};
+
 	scif2_pins: scif2 {
 		groups = "scif2_data_a";
 		function = "scif2";
@@ -147,16 +217,6 @@
 		function = "scif_clk";
 	};
 
-	i2c2_pins: i2c2 {
-		groups = "i2c2_a";
-		function = "i2c2";
-	};
-
-	avb_pins: avb {
-		groups = "avb_mdc";
-		function = "avb";
-	};
-
 	sdhi0_pins: sd0 {
 		groups = "sdhi0_data4", "sdhi0_ctrl";
 		function = "sdhi0";
@@ -198,53 +258,6 @@
 	};
 };
 
-&scif2 {
-	pinctrl-0 = <&scif2_pins>;
-	pinctrl-names = "default";
-
-	status = "okay";
-};
-
-&scif_clk {
-	clock-frequency = <14745600>;
-};
-
-&i2c2 {
-	pinctrl-0 = <&i2c2_pins>;
-	pinctrl-names = "default";
-
-	status = "okay";
-
-	clock-frequency = <100000>;
-
-	ak4613: codec@10 {
-		compatible = "asahi-kasei,ak4613";
-		#sound-dai-cells = <0>;
-		reg = <0x10>;
-		clocks = <&rcar_sound 3>;
-
-		asahi-kasei,in1-single-end;
-		asahi-kasei,in2-single-end;
-		asahi-kasei,out1-single-end;
-		asahi-kasei,out2-single-end;
-		asahi-kasei,out3-single-end;
-		asahi-kasei,out4-single-end;
-		asahi-kasei,out5-single-end;
-		asahi-kasei,out6-single-end;
-	};
-
-	cs2000: clk-multiplier@4f {
-		#clock-cells = <0>;
-		compatible = "cirrus,cs2000-cp";
-		reg = <0x4f>;
-		clocks = <&audio_clkout>, <&x12_clk>;
-		clock-names = "clk_in", "ref_clk";
-
-		assigned-clocks = <&cs2000>;
-		assigned-clock-rates = <24576000>; /* 1/1 divide */
-	};
-};
-
 &rcar_sound {
 	pinctrl-0 = <&sound_pins &sound_clk_pins>;
 	pinctrl-names = "default";
@@ -275,7 +288,7 @@
 		 <&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>,
 		 <&audio_clk_a>, <&cs2000>,
 		 <&audio_clk_c>,
-		 <&cpg CPG_CORE R8A7795_CLK_S0D4>;
+		 <&cpg CPG_CORE CPG_AUDIO_CLK_I>;
 
 	rcar_sound,dai {
 		dai0 {
@@ -285,6 +298,17 @@
 	};
 };
 
+&scif2 {
+	pinctrl-0 = <&scif2_pins>;
+	pinctrl-names = "default";
+
+	status = "okay";
+};
+
+&scif_clk {
+	clock-frequency = <14745600>;
+};
+
 &sdhi0 {
 	pinctrl-0 = <&sdhi0_pins>;
 	pinctrl-1 = <&sdhi0_pins_uhs>;
@@ -316,30 +340,6 @@
 	shared-pin;
 };
 
-&wdt0 {
-	timeout-sec = <60>;
-	status = "okay";
-};
-
-&audio_clk_a {
-	clock-frequency = <22579200>;
-};
-
-&avb {
-	pinctrl-0 = <&avb_pins>;
-	pinctrl-names = "default";
-	renesas,no-ether-link;
-	phy-handle = <&phy0>;
-	status = "okay";
-
-	phy0: ethernet-phy@0 {
-		rxc-skew-ps = <1500>;
-		reg = <0>;
-		interrupt-parent = <&gpio2>;
-		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
-	};
-};
-
 &usb2_phy1 {
 	pinctrl-0 = <&usb1_pins>;
 	pinctrl-names = "default";
@@ -347,10 +347,7 @@
 	status = "okay";
 };
 
-&ehci1 {
-	status = "okay";
-};
-
-&ohci1 {
+&wdt0 {
+	timeout-sec = <60>;
 	status = "okay";
 };
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH 2/5] mtd: nand: gpmi: add i.MX 7 SoC support
From: Marek Vasut @ 2017-04-21 13:08 UTC (permalink / raw)
  To: Stefan Agner
  Cc: dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
	computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
	boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	richard-/L3Ra7n9ekc, cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	han.xu-3arQi8VN3Tc, fabio.estevam-KZfg59tc24xl57MIdRCFDg,
	LW-AvR2QvxeiV7DiMYJYoSAnRvVK+yQ3ZXh,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5cf26d2b020392c875464c7504a9fb5b-XLVq0VzYD2Y@public.gmane.org>

On 04/21/2017 05:15 AM, Stefan Agner wrote:
> On 2017-04-20 19:03, Marek Vasut wrote:
>> On 04/21/2017 03:07 AM, Stefan Agner wrote:
>>> Add support for i.MX 7 SoC. The i.MX 7 has a slightly different
>>> clock architecture requiring only two clocks to be referenced.
>>> The IP is slightly different compared to i.MX 6SoloX, but currently
>>> none of this differences are in use so there is no detection needed
>>> and the driver can reuse IS_MX6SX.
>>>
>>> Signed-off-by: Stefan Agner <stefan-XLVq0VzYD2Y@public.gmane.org>
>>> ---
>>>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 15 +++++++++++++++
>>>  1 file changed, 15 insertions(+)
>>>
>>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>> index c8bbf5da2ab8..4a45d37ddc80 100644
>>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>> @@ -127,6 +127,18 @@ static const struct gpmi_devdata gpmi_devdata_imx6sx = {
>>>  	.clks_count = ARRAY_SIZE(gpmi_clks_for_mx6),
>>>  };
>>>
>>> +static const char * const gpmi_clks_for_mx7d[] = {
>>> +	"gpmi_io", "gpmi_bch_apb",
>>> +};
>>> +
>>> +static const struct gpmi_devdata gpmi_devdata_imx7d = {
>>> +	.type = IS_MX6SX,
>>
>> Would it make sense to use IS_MX7 here already to prevent future surprises ?
>>
> 
> Yeah I was thinking we can do it once we have an actual reason to
> distinguish.

So what are the differences anyway ?

> But then, adding the type would only require 2-3 lines of change if I
> add it to the GPMI_IS_MX6 macro...

Then at least add a comment because using type = IMX6SX right under
gpmi_data_mx7d can trigger some head-scratching. And put my R-B on V2.

-- 
Best regards,
Marek Vasut
--
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 0/2] hwrng: mtk: add support for hardware random generator on MT7623 SoC
From: Herbert Xu @ 2017-04-21 13:16 UTC (permalink / raw)
  To: sean.wang
  Cc: mpm, robh+dt, mark.rutland, clabbe.montjoie, prasannatsmkumar,
	romain.perier, shannon.nelson, weiyongjun1, devicetree,
	linux-crypto, linux-mediatek, linux-arm-kernel, linux-kernel,
	keyhaede
In-Reply-To: <1492705466-27287-1-git-send-email-sean.wang@mediatek.com>

On Fri, Apr 21, 2017 at 12:24:24AM +0800, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
> 
> This patchset introduces support for Mediatek hardware random generator (RNG)
> Currently, the driver is already tested successfully with rng-tools on MT7623
> SoC. And it should also be workable on other similar Mediatek SoCs.
> 
> Changes since v1:

All applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH v13 03/10] mux: minimal mux subsystem and gpio-based mux controller
From: Philipp Zabel @ 2017-04-21 14:18 UTC (permalink / raw)
  To: Peter Rosin
  Cc: linux-kernel, Greg Kroah-Hartman, Wolfram Sang, Rob Herring,
	Mark Rutland, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	linux-i2c, devicetree, linux-iio, linux-doc, Andrew Morton,
	Colin Ian King, Paul Gortmaker, kernel
In-Reply-To: <1492101794-13444-4-git-send-email-peda@axentia.se>

Hi Peter,

On Thu, 2017-04-13 at 18:43 +0200, Peter Rosin wrote:
[...]
> +int mux_control_select(struct mux_control *mux, int state)

state could be unsigned int for the consumer facing API.

> +{
> +	int ret;

And mux_control_select should check that (0 <= state < mux->states).

regards
Philipp

^ permalink raw reply

* Re: [PATCH v13 03/10] mux: minimal mux subsystem and gpio-based mux controller
From: Philipp Zabel @ 2017-04-21 14:23 UTC (permalink / raw)
  To: Peter Rosin
  Cc: linux-kernel, Greg Kroah-Hartman, Wolfram Sang, Rob Herring,
	Mark Rutland, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	linux-i2c, devicetree, linux-iio, linux-doc, Andrew Morton,
	Colin Ian King, Paul Gortmaker, kernel
In-Reply-To: <1492101794-13444-4-git-send-email-peda@axentia.se>

On Thu, 2017-04-13 at 18:43 +0200, Peter Rosin wrote:
[...]
> +int mux_chip_register(struct mux_chip *mux_chip)
> +{
> +	int i;
> +	int ret;
> +
> +	for (i = 0; i < mux_chip->controllers; ++i) {
> +		struct mux_control *mux = &mux_chip->mux[i];
> +
> +		if (mux->idle_state == mux->cached_state)
> +			continue;

I think this should be changed to
 
-               if (mux->idle_state == mux->cached_state)
+               if (mux->idle_state == mux->cached_state ||
+                   mux->idle_state == MUX_IDLE_AS_IS)
                        continue;

or the following mux_control_set will be called with state ==
MUX_IDLE_AS_IS. Alternatively, mux_control_set should return when passed
this value.

> +		ret = mux_control_set(mux, mux->idle_state);
> +		if (ret < 0) {
> +			dev_err(&mux_chip->dev, "unable to set idle state\n");
> +			return ret;
> +		}
> +	}
> +
> +	ret = device_add(&mux_chip->dev);
> +	if (ret < 0)
> +		dev_err(&mux_chip->dev,
> +			"device_add failed in mux_chip_register: %d\n", ret);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(mux_chip_register);

regards
Philipp

^ permalink raw reply

* Re: [PATCH v2 2/2] dmaengine: Add DW AXI DMAC driver
From: Eugeniy Paltsev @ 2017-04-21 14:29 UTC (permalink / raw)
  To: andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org
  Cc: vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	Alexey.Brodkin-HKixBCOQz3hWk0Htik3J/w@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Eugeniy.Paltsev-HKixBCOQz3hWk0Htik3J/w@public.gmane.org,
	linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	dmaengine-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1492518695.24567.56.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Hi Andy,
thanks for respond.
My comments are inlined below.

On Tue, 2017-04-18 at 15:31 +0300, Andy Shevchenko wrote:
> On Fri, 2017-04-07 at 17:04 +0300, Eugeniy Paltsev wrote:
> > This patch adds support for the DW AXI DMAC controller.
> >
> > DW AXI DMAC is a part of upcoming development board from Synopsys.
> >
> > In this driver implementation only DMA_MEMCPY and DMA_SG transfers
> > are supported.
> >
> > +++ b/drivers/dma/axi_dma_platform.c
> > @@ -0,0 +1,1044 @@
> > +#include <linux/bitops.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/dmaengine.h>
> > +#include <linux/dmapool.h>
> > +#include <linux/err.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
>
> Are you sure you still need of.h along with depends OF ?
"of_match_ptr" used from of.h

> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/property.h>
> > +#include <linux/types.h>
> > +
> > +#include "axi_dma_platform.h"
> > +#include "axi_dma_platform_reg.h"
>
> Can't you have this in one header?
Sure.

> > +#include "dmaengine.h"
> > +#include "virt-dma.h"
> > +#define AXI_DMA_BUSWIDTHS		  \
> > +	(DMA_SLAVE_BUSWIDTH_1_BYTE	| \
> > +	DMA_SLAVE_BUSWIDTH_2_BYTES	| \
> > +	DMA_SLAVE_BUSWIDTH_4_BYTES	| \
> > +	DMA_SLAVE_BUSWIDTH_8_BYTES	| \
> > +	DMA_SLAVE_BUSWIDTH_16_BYTES	| \
> > +	DMA_SLAVE_BUSWIDTH_32_BYTES	| \
> > +	DMA_SLAVE_BUSWIDTH_64_BYTES)
> > +/* TODO: check: do we need to use BIT() macro here? */
>
> Still TODO? I remember I answered to this on the first round.
Yes, I remember it.
I left this TODO as a reminder because src_addr_widths and
dst_addr_widths are
not used anywhere and they are set differently in different drivers
(with or without BIT macro).

> > +
> > +static inline void
> > +axi_dma_iowrite32(struct axi_dma_chip *chip, u32 reg, u32 val)
> > +{
> > +	iowrite32(val, chip->regs + reg);
>
> Are you going to use IO ports for this IP? I don't think so.
> Wouldn't be better to call readl()/writel() instead?
As I understand, it's better to use ioread/iowrite as more universal IO
access way. Am I wrong?

> > +}
> > +static inline void
> > +axi_chan_iowrite64(struct axi_dma_chan *chan, u32 reg, u64 val)
> > +{
> > +	iowrite32(val & 0xFFFFFFFF, chan->chan_regs + reg);
> Useless conjunction.
>
> > +	iowrite32(val >> 32, chan->chan_regs + reg + 4);
> > +}
>
> Can your hardware get 8 bytes at once?
> For such cases we have iowrite64() for 64-bit kernels
>
> But with readq()/writeq() we have specific helpers to make this
> pretty,
> i.e. lo_hi_readq() / lo_hi_writeq() (or hi_lo_*() variants).
Ok, I possibly can use lo_hi_writeq here.

> > +static inline void axi_chan_irq_disable(struct axi_dma_chan *chan,
> > u32 irq_mask)
> > +{
> > +	u32 val;
> > +
> > +	if (likely(irq_mask == DWAXIDMAC_IRQ_ALL)) {
> > +		axi_chan_iowrite32(chan, CH_INTSTATUS_ENA,
> > DWAXIDMAC_IRQ_NONE);
> > +	} else {
>
> I don't see the benefit. (Yes, I see one read-less path, I think it
> makes perplexity for nothing here)
This function is called mostly with irq_mask = DWAXIDMAC_IRQ_ALL.
Actually it is called only with irq_mask = DWAXIDMAC_IRQ_ALL until I
add DMA_SLAVE support.
But I can cut off this 'if' statment, if it is necessery.

> > +		val = axi_chan_ioread32(chan, CH_INTSTATUS_ENA);
> > +		val &= ~irq_mask;
> > +		axi_chan_iowrite32(chan, CH_INTSTATUS_ENA, val);
> > +	}
> > +}
> > +static inline void axi_chan_disable(struct axi_dma_chan *chan)
> > +{
> > +}
> > +
> > +static inline void axi_chan_enable(struct axi_dma_chan *chan)
> > +{
> > +}
> > +static u32 axi_chan_get_xfer_width(struct axi_dma_chan *chan,
> > dma_addr_t src,
> > +				   dma_addr_t dst, size_t len)
> > +{
> > +	u32 max_width = chan->chip->dw->hdata->m_data_width;
> > +	size_t sdl = (src | dst | len);
>
> Redundant parens, redundant temporary variable (you may do this in
> place).
Ok.

> > +
> > +	return min_t(size_t, __ffs(sdl), max_width);
> > +}
> > +static void axi_desc_put(struct axi_dma_desc *desc)
> > +{
> > +	struct axi_dma_chan *chan = desc->chan;
> > +	struct dw_axi_dma *dw = chan->chip->dw;
> > +	struct axi_dma_desc *child, *_next;
> > +	unsigned int descs_put = 0;
> > +	list_for_each_entry_safe(child, _next, &desc->xfer_list,
> > xfer_list) {
>
> xfer_list looks redundant.
> Can you elaborate why virtual channel management is not working for
> you?
Each virtual descriptor encapsulates several hardware descriptors,
which belong to same transfer.
This list (xfer_list) is used only for allocating/freeing these
descriptors and it doesn't affect on virtual dma work logic.
I can see this approach in several drivers with VirtDMA (but they
mostly use array instead of list)

> > +		list_del(&child->xfer_list);
> > +		dma_pool_free(dw->desc_pool, child, child-
> > > vd.tx.phys);
> >
> > +		descs_put++;
> > +	}
> > +}
> > +/* Called in chan locked context */
> > +static void axi_chan_block_xfer_start(struct axi_dma_chan *chan,
> > +				      struct axi_dma_desc *first)
> > +{
> > +	u32 reg, irq_mask;
> > +	u8 lms = 0;
>
> Does it make any sense? It looks like lms is always 0.
> Or I miss the source of its value?
lms variable uset to determine axi bus for reading lli descriptors. It
is equal to 0 for mem-to-mem transfers. Perhaps it is better to use
define instead of this variable.

> > +	u32 priority = chan->chip->dw->hdata->priority[chan->id];
>
> Reversed xmas tree, please.
>
> Btw, are you planning to use priority at all? For now on I didn't see
> a single driver (from the set I have checked, like 4-5 of them) that
> uses priority anyhow. It makes driver more complex for nothing.
Only for dma slave operations.

>
> > +
> > +	if (unlikely(axi_chan_is_hw_enable(chan))) {
> > +		dev_err(chan2dev(chan), "%s is non-idle!\n",
> > +			axi_chan_name(chan));
> > +
> > +		return;
> > +	}
> > +}
> > +static void dma_chan_free_chan_resources(struct dma_chan *dchan)
> > +{
> > +	struct axi_dma_chan *chan = dchan_to_axi_dma_chan(dchan);
> > +
> > +	/* ASSERT: channel hw is idle */
> > +	if (axi_chan_is_hw_enable(chan))
> > +		dev_err(dchan2dev(dchan), "%s is non-idle!\n",
> > +			axi_chan_name(chan));
> > +
> > +	axi_chan_disable(chan);
> > +	axi_chan_irq_disable(chan, DWAXIDMAC_IRQ_ALL);
> > +
> > +	vchan_free_chan_resources(&chan->vc);
> > +
> > +	dev_vdbg(dchan2dev(dchan), "%s: %s: descriptor still
> > allocated: %u\n",
> > +		__func__, axi_chan_name(chan),
>
> Redundant __func__ parameter for debug prints.
>
> > +		atomic_read(&chan->descs_allocated));
> > +
> > +	pm_runtime_put(chan->chip->dev);
> > +}
> > +static struct dma_async_tx_descriptor *
> > +dma_chan_prep_dma_sg(struct dma_chan *dchan,
> > +		     struct scatterlist *dst_sg, unsigned int
> > dst_nents,
> > +		     struct scatterlist *src_sg, unsigned int
> > src_nents,
> > +		     unsigned long flags)
> > +{
> > +	struct axi_dma_chan *chan = dchan_to_axi_dma_chan(dchan);
> > +	struct axi_dma_desc *first = NULL, *desc = NULL, *prev =
> > NULL;
> > +	size_t dst_len = 0, src_len = 0, xfer_len = 0;
> > +	dma_addr_t dst_adr = 0, src_adr = 0;
> > +	u32 src_width, dst_width;
> > +	size_t block_ts, max_block_ts;
> > +	u32 reg;
> > +	u8 lms = 0;
>
> Same about lms.
>
> > +
> > +	dev_dbg(chan2dev(chan), "%s: %s: sn: %d dn: %d flags:
> > 0x%lx",
> > +		__func__, axi_chan_name(chan), src_nents,
> > dst_nents,
> > flags);
>
> Ditto for __func__.
>
> > +
> >
> > +	if (unlikely(dst_nents == 0 || src_nents == 0))
> > +		return NULL;
> > +
> > +	if (unlikely(dst_sg == NULL || src_sg == NULL))
> > +		return NULL;
>
> If we need those checks they should go to dmaengine.h/dmaengine.c.
I checked several drivers, which implements device_prep_dma_sg and they
implements this checkers.
Should I add something like "dma_sg_desc_invalid" function to
dmaengine.h ?

> > +static void axi_chan_list_dump_lli(struct axi_dma_chan *chan,
> > +				   struct axi_dma_desc *desc_head)
> > +{
> > +	struct axi_dma_desc *desc;
> > +
> > +	axi_chan_dump_lli(chan, desc_head);
> > +	list_for_each_entry(desc, &desc_head->xfer_list,
> > xfer_list)
> > +		axi_chan_dump_lli(chan, desc);
> > +}
> > +
> > +static void axi_chan_handle_err(struct axi_dma_chan *chan, u32
> > status)
> > +{
> > +	/* WARN about bad descriptor */
> >
> > +	dev_err(chan2dev(chan),
> > +		"Bad descriptor submitted for %s, cookie: %d, irq:
> > 0x%08x\n",
> > +		axi_chan_name(chan), vd->tx.cookie, status);
> > +	axi_chan_list_dump_lli(chan, vd_to_axi_desc(vd));
>
> As I said earlier dw_dmac is *bad* example of the (virtual channel
> based) DMA driver.
>
> I guess you may just fail the descriptor and don't pretend it has
> been processed successfully.
What do you mean by saying "fail the descriptor"?
After I get error I cancel current transfer and free all descriptors
from it (by calling vchan_cookie_complete).
I can't store error status in descriptor structure because it will be
freed by vchan_cookie_complete.
I can't store error status in channel structure because it will be
overwritten by next transfer.


> > +static int dma_chan_pause(struct dma_chan *dchan)
> > +{
> > +	struct axi_dma_chan *chan = dchan_to_axi_dma_chan(dchan);
> > +	unsigned long flags;
> > +	unsigned int timeout = 20; /* timeout iterations */
> > +	int ret = -EAGAIN;
> > +	u32 val;
> > +
> > +	spin_lock_irqsave(&chan->vc.lock, flags);
> > +
> > +	val = axi_dma_ioread32(chan->chip, DMAC_CHEN);
> > +	val |= BIT(chan->id) << DMAC_CHAN_SUSP_SHIFT |
> > +	       BIT(chan->id) << DMAC_CHAN_SUSP_WE_SHIFT;
> > +	axi_dma_iowrite32(chan->chip, DMAC_CHEN, val);
>
> You have helpers which you don't use. Why?
Ok, will use.

> > +
> > +	while (timeout--) {
>
> In such cases I prefer do {} while (); to explicitly show that body
> goes at least once.
Good idea. Will change to do {} while () here.

> > +		if (axi_chan_irq_read(chan) &
> > DWAXIDMAC_IRQ_SUSPENDED) {
> > +			ret = 0;
> > +			break;
> > +		}
> > +		udelay(2);
> > +	}
> > +
> > +	axi_chan_irq_clear(chan, DWAXIDMAC_IRQ_SUSPENDED);
> > +
> > +	chan->is_paused = true;
> > +
> > +	spin_unlock_irqrestore(&chan->vc.lock, flags);
> > +
> > +	return ret;
> > +}
> > +
> > +/* Called in chan locked context */
> > +static inline void axi_chan_resume(struct axi_dma_chan *chan)
> > +{
> > +	u32 val;
> > +
> > +	val = axi_dma_ioread32(chan->chip, DMAC_CHEN);
> > +	val &= ~(BIT(chan->id) << DMAC_CHAN_SUSP_SHIFT);
> > +	val |=  (BIT(chan->id) << DMAC_CHAN_SUSP_WE_SHIFT);
> > +	axi_dma_iowrite32(chan->chip, DMAC_CHEN, val);
> > +
> > +	chan->is_paused = false;
> > +}
> > +static int axi_dma_runtime_suspend(struct device *dev)
> > +{
> > +	struct axi_dma_chip *chip = dev_get_drvdata(dev);
> > +
> > +	dev_info(dev, "PAL: %s\n", __func__);
>
> Noisy and useless.
> We have functional tracer in kernel. Use it.
Ok.

> > +
> > +	axi_dma_irq_disable(chip);
> > +	axi_dma_disable(chip);
> > +
> > +	clk_disable_unprepare(chip->clk);
> > +
> > +	return 0;
> > +}

[snip]

> > +
> > +static const struct dev_pm_ops dw_axi_dma_pm_ops = {
> > +	SET_RUNTIME_PM_OPS(axi_dma_runtime_suspend,
> > axi_dma_runtime_resume, NULL)
> > +};
>
> Have you tried to build with CONFIG_PM disabled?
Yes.

> I'm pretty sure you need __maybe_unused applied to your PM ops.
I call axi_dma_runtime_suspend / axi_dma_runtime_resume even I dont't
use PM.
(I call them in probe / remove function.)
So I don't need to declare them with __maybe_unused.

> > +struct axi_dma_chan {
> > +	struct axi_dma_chip		*chip;
> > +	void __iomem			*chan_regs;
> > +	u8				id;
> > +	atomic_t			descs_allocated;
> > +
> > +	struct virt_dma_chan		vc;
> > +
> > +	/* these other elements are all protected by vc.lock */
> > +	bool				is_paused;
>
> I still didn't get (already forgot) why you can't use dma_status
> instead for the active descriptor?
As I said before, I checked several driver, which have status variable
in their channel structure - it is used *only* for determinating is
channel paused or not. So there is no much sense in replacing
"is_paused" to "status" and I left "is_paused" variable untouched.

(I described above why we can't use status in channel structure for
error handling)

> > +};
> > +/* LLI == Linked List Item */
> > +struct __attribute__ ((__packed__)) axi_dma_lli {
>
> ...
>
> > +	__le64		sar;
> > +	__le64		dar;
> > +	__le32		block_ts_lo;
> > +	__le32		block_ts_hi;
> > +	__le64		llp;
> > +	__le32		ctl_lo;
> > +	__le32		ctl_hi;
> > +	__le32		sstat;
> > +	__le32		dstat;
> > +	__le32		status_lo;
> > +	__le32		ststus_hi;
> > +	__le32		reserved_lo;
> > +	__le32		reserved_hi;
> > +};
>
> Just __packed here.
>
Ok.

> > +
> > +struct axi_dma_desc {
> > +	struct axi_dma_lli		lli;
> > +
> > +	struct virt_dma_desc		vd;
> > +	struct axi_dma_chan		*chan;
> > +	struct list_head		xfer_list;
>
> This looks redundant. Already asked above about it.
Answered above.

> > +};
> > +
> > +/* Common registers offset */
> > +#define DMAC_ID			0x000 /* R DMAC ID */
> > +#define DMAC_COMPVER		0x008 /* R DMAC Component
> > Version
> > */
> > +#define DMAC_CFG		0x010 /* R/W DMAC Configuration */
> > +#define DMAC_CHEN		0x018 /* R/W DMAC Channel Enable
> > */
> > +#define DMAC_CHEN_L		0x018 /* R/W DMAC Channel
> > Enable
> > 00-31 */
> > +#define DMAC_CHEN_H		0x01C /* R/W DMAC Channel
> > Enable
> > 32-63 */
> > +#define DMAC_INTSTATUS		0x030 /* R DMAC Interrupt
> > Status */
> > +#define DMAC_COMMON_INTCLEAR	0x038 /* W DMAC Interrupt
> > Clear
> > */
> > +#define DMAC_COMMON_INTSTATUS_ENA 0x040 /* R DMAC Interrupt Status
> > Enable */
> > +#define DMAC_COMMON_INTSIGNAL_ENA 0x048 /* R/W DMAC Interrupt
> > Signal
> > Enable */
> > +#define DMAC_COMMON_INTSTATUS	0x050 /* R DMAC Interrupt
> > Status
> > */
> > +#define DMAC_RESET		0x058 /* R DMAC Reset Register1
> > */
> > +
> > +/* DMA channel registers offset */
> > +#define CH_SAR			0x000 /* R/W Chan Source
> > Address */
> > +#define CH_DAR			0x008 /* R/W Chan
> > Destination
> > Address */
> > +#define CH_BLOCK_TS		0x010 /* R/W Chan Block
> > Transfer
> > Size */
> > +#define CH_CTL			0x018 /* R/W Chan Control */
> > +#define CH_CTL_L		0x018 /* R/W Chan Control 00-31 */
> > +#define CH_CTL_H		0x01C /* R/W Chan Control 32-63 */
> > +#define CH_CFG			0x020 /* R/W Chan
> > Configuration
> > */
> > +#define CH_CFG_L		0x020 /* R/W Chan Configuration
> > 00-31
> > */
> > +#define CH_CFG_H		0x024 /* R/W Chan Configuration
> > 32-63
> > */
> > +#define CH_LLP			0x028 /* R/W Chan Linked
> > List
> > Pointer */
> > +#define CH_STATUS		0x030 /* R Chan Status */
> > +#define CH_SWHSSRC		0x038 /* R/W Chan SW Handshake
> > Source */
> > +#define CH_SWHSDST		0x040 /* R/W Chan SW Handshake
> > Destination */
> > +#define CH_BLK_TFR_RESUMEREQ	0x048 /* W Chan Block Transfer
> > Resume Req */
> > +#define CH_AXI_ID		0x050 /* R/W Chan AXI ID */
> > +#define CH_AXI_QOS		0x058 /* R/W Chan AXI QOS */
> > +#define CH_SSTAT		0x060 /* R Chan Source Status */
> > +#define CH_DSTAT		0x068 /* R Chan Destination Status
> > */
> > +#define CH_SSTATAR		0x070 /* R/W Chan Source Status
> > Fetch Addr */
> > +#define CH_DSTATAR		0x078 /* R/W Chan Destination
> > Status Fetch Addr */
> > +#define CH_INTSTATUS_ENA	0x080 /* R/W Chan Interrupt Status
> > Enable */
> > +#define CH_INTSTATUS		0x088 /* R/W Chan Interrupt
> > Status */
> > +#define CH_INTSIGNAL_ENA	0x090 /* R/W Chan Interrupt Signal
> > Enable */
> > +#define CH_INTCLEAR		0x098 /* W Chan Interrupt Clear
> > */
>
> I'm wondering if you can use regmap API instead.
Is it really necessary? It'll make driver more complex for nothing.
>
> > +/* DMAC_CFG */
> > +#define DMAC_EN_MASK		0x00000001U
>
> GENMASK()
Ok.

> > +#define DMAC_EN_POS		0
>
> Usually _SHIFT, but it's up to you.
>
> > +enum {
> > +	DWAXIDMAC_BURST_TRANS_LEN_1	= 0x0,
> > +	DWAXIDMAC_BURST_TRANS_LEN_4,
> > +	DWAXIDMAC_BURST_TRANS_LEN_8,
> > +	DWAXIDMAC_BURST_TRANS_LEN_16,
> > +	DWAXIDMAC_BURST_TRANS_LEN_32,
> > +	DWAXIDMAC_BURST_TRANS_LEN_64,
> > +	DWAXIDMAC_BURST_TRANS_LEN_128,
> > +	DWAXIDMAC_BURST_TRANS_LEN_256,
> > +	DWAXIDMAC_BURST_TRANS_LEN_512,
> > +	DWAXIDMAC_BURST_TRANS_LEN_1024
>
> ..._1024, ?
What exactly you are asking about?

> > +};
>
> Hmm... do you need them in the header?
I use some of these definitions in my code so I guess yes.
/* Maybe I misunderstood your question... */

> > +#define CH_CFG_H_TT_FC_POS	0
> > +enum {
> > +	DWAXIDMAC_TT_FC_MEM_TO_MEM_DMAC	= 0x0,
> > +	DWAXIDMAC_TT_FC_MEM_TO_PER_DMAC,
> > +	DWAXIDMAC_TT_FC_PER_TO_MEM_DMAC,
> > +	DWAXIDMAC_TT_FC_PER_TO_PER_DMAC,
> > +	DWAXIDMAC_TT_FC_PER_TO_MEM_SRC,
> > +	DWAXIDMAC_TT_FC_PER_TO_PER_SRC,
> > +	DWAXIDMAC_TT_FC_MEM_TO_PER_DST,
> > +	DWAXIDMAC_TT_FC_PER_TO_PER_DST
> > +};
>
> Some of definitions are the same as for dw_dmac, right? We might
> split them to a common header, though I have no strong opinion about
it.
> Vinod?
APB DMAC and AXI DMAC have completely different regmap. So there is no
much sense to do that.

--
 Eugeniy Paltsev

^ permalink raw reply

* Re: [PATCH v13 03/10] mux: minimal mux subsystem and gpio-based mux controller
From: Peter Rosin @ 2017-04-21 14:32 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: linux-kernel, Greg Kroah-Hartman, Wolfram Sang, Rob Herring,
	Mark Rutland, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	linux-i2c, devicetree, linux-iio, linux-doc, Andrew Morton,
	Colin Ian King, Paul Gortmaker, kernel
In-Reply-To: <1492784582.2364.10.camel@pengutronix.de>

On 2017-04-21 16:23, Philipp Zabel wrote:
> On Thu, 2017-04-13 at 18:43 +0200, Peter Rosin wrote:
> [...]
>> +int mux_chip_register(struct mux_chip *mux_chip)
>> +{
>> +	int i;
>> +	int ret;
>> +
>> +	for (i = 0; i < mux_chip->controllers; ++i) {
>> +		struct mux_control *mux = &mux_chip->mux[i];
>> +
>> +		if (mux->idle_state == mux->cached_state)
>> +			continue;
> 
> I think this should be changed to
>  
> -               if (mux->idle_state == mux->cached_state)
> +               if (mux->idle_state == mux->cached_state ||
> +                   mux->idle_state == MUX_IDLE_AS_IS)
>                         continue;
> 
> or the following mux_control_set will be called with state ==
> MUX_IDLE_AS_IS. Alternatively, mux_control_set should return when passed
> this value.

That cannot happen because ->cached_state is initialized to -1
in mux_chip_alloc, so should always be == MUX_IDLE_AS_IS when
registering. And drivers are not supposed to touch ->cached_state.
I.e., ->cached_state is "owned" by the core.

Cheers,
peda

>> +		ret = mux_control_set(mux, mux->idle_state);
>> +		if (ret < 0) {
>> +			dev_err(&mux_chip->dev, "unable to set idle state\n");
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	ret = device_add(&mux_chip->dev);
>> +	if (ret < 0)
>> +		dev_err(&mux_chip->dev,
>> +			"device_add failed in mux_chip_register: %d\n", ret);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(mux_chip_register);
> 
> regards
> Philipp
> 

^ permalink raw reply

* Re: [PATCH v13 03/10] mux: minimal mux subsystem and gpio-based mux controller
From: Philipp Zabel @ 2017-04-21 14:41 UTC (permalink / raw)
  To: Peter Rosin
  Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Lars-Peter Clausen, kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Wolfram Sang,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman,
	Jonathan Corbet, linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Paul Gortmaker, Rob Herring,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA, Peter Meerwald-Stadler,
	Hartmut Knaack, Colin Ian King, Andrew Morton, Jonathan Cameron
In-Reply-To: <9e3d48c4-0dbc-3e80-c653-b0357abf1d6f-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

On Fri, 2017-04-21 at 16:32 +0200, Peter Rosin wrote:
> On 2017-04-21 16:23, Philipp Zabel wrote:
> > On Thu, 2017-04-13 at 18:43 +0200, Peter Rosin wrote:
> > [...]
> >> +int mux_chip_register(struct mux_chip *mux_chip)
> >> +{
> >> +	int i;
> >> +	int ret;
> >> +
> >> +	for (i = 0; i < mux_chip->controllers; ++i) {
> >> +		struct mux_control *mux = &mux_chip->mux[i];
> >> +
> >> +		if (mux->idle_state == mux->cached_state)
> >> +			continue;
> > 
> > I think this should be changed to
> >  
> > -               if (mux->idle_state == mux->cached_state)
> > +               if (mux->idle_state == mux->cached_state ||
> > +                   mux->idle_state == MUX_IDLE_AS_IS)
> >                         continue;
> > 
> > or the following mux_control_set will be called with state ==
> > MUX_IDLE_AS_IS. Alternatively, mux_control_set should return when passed
> > this value.
> 
> That cannot happen because ->cached_state is initialized to -1
> in mux_chip_alloc, so should always be == MUX_IDLE_AS_IS when
> registering. And drivers are not supposed to touch ->cached_state.
> I.e., ->cached_state is "owned" by the core.

So this was caused by me filling cached_state from register reads in the
mmio driver. Makes me wonder why I am not allowed to do this, though, if
I am able to read back the initial state?

regards
Philipp

--
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 1/2 v4] video: fbdev: imxfb: support AUS mode
From: Bartlomiej Zolnierkiewicz @ 2017-04-21 14:49 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Sascha Hauer,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1492770549-7347-1-git-send-email-martin-XxZfDwE/svGeZLLa646FqQ@public.gmane.org>

On Friday, April 21, 2017 12:29:08 PM Martin Kaiser wrote:
> Some displays require setting AUS mode in the LDCD AUS Mode Control
> Register to work with the imxfb driver. Like the value of the Panel
> Configuration Register, the AUS mode setting depends on the display
> mode.
> 
> Allow setting AUS mode from the device tree by adding a boolean
> property. Make this property optional to keep the DT ABI stable.
> AUS mode can be set only on imx21 and compatible chipsets.
> 
> Signed-off-by: Martin Kaiser <martin-XxZfDwE/svGeZLLa646FqQ@public.gmane.org>

Patch queued for 4.12, thanks.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
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 2/2 v4] dt-bindings: display: imx: entry for AUS mode
From: Bartlomiej Zolnierkiewicz @ 2017-04-21 14:50 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Sascha Hauer,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1492777790-12748-1-git-send-email-martin-XxZfDwE/svGeZLLa646FqQ@public.gmane.org>

On Friday, April 21, 2017 02:29:50 PM Martin Kaiser wrote:
> Allow setting AUS mode for a display from the device tree. Use an
> optional boolean property. AUS mode can be set only on imx21 and
> compatible chipsets.
> 
> Signed-off-by: Martin Kaiser <martin-XxZfDwE/svGeZLLa646FqQ@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Patch queued for 4.12, thanks.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
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 v13 03/10] mux: minimal mux subsystem and gpio-based mux controller
From: Peter Rosin @ 2017-04-21 14:55 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Mark Rutland, devicetree, Lars-Peter Clausen, kernel,
	Wolfram Sang, linux-iio, Greg Kroah-Hartman, Jonathan Corbet,
	linux-doc, linux-kernel, Paul Gortmaker, Rob Herring, linux-i2c,
	Peter Meerwald-Stadler, Hartmut Knaack, Colin Ian King,
	Andrew Morton, Jonathan Cameron
In-Reply-To: <1492785664.2364.13.camel@pengutronix.de>

On 2017-04-21 16:41, Philipp Zabel wrote:
> On Fri, 2017-04-21 at 16:32 +0200, Peter Rosin wrote:
>> On 2017-04-21 16:23, Philipp Zabel wrote:
>>> On Thu, 2017-04-13 at 18:43 +0200, Peter Rosin wrote:
>>> [...]
>>>> +int mux_chip_register(struct mux_chip *mux_chip)
>>>> +{
>>>> +	int i;
>>>> +	int ret;
>>>> +
>>>> +	for (i = 0; i < mux_chip->controllers; ++i) {
>>>> +		struct mux_control *mux = &mux_chip->mux[i];
>>>> +
>>>> +		if (mux->idle_state == mux->cached_state)
>>>> +			continue;
>>>
>>> I think this should be changed to
>>>  
>>> -               if (mux->idle_state == mux->cached_state)
>>> +               if (mux->idle_state == mux->cached_state ||
>>> +                   mux->idle_state == MUX_IDLE_AS_IS)
>>>                         continue;
>>>
>>> or the following mux_control_set will be called with state ==
>>> MUX_IDLE_AS_IS. Alternatively, mux_control_set should return when passed
>>> this value.
>>
>> That cannot happen because ->cached_state is initialized to -1
>> in mux_chip_alloc, so should always be == MUX_IDLE_AS_IS when
>> registering. And drivers are not supposed to touch ->cached_state.
>> I.e., ->cached_state is "owned" by the core.
> 
> So this was caused by me filling cached_state from register reads in the
> mmio driver. Makes me wonder why I am not allowed to do this, though, if
> I am able to read back the initial state?

You gain fairly little by reading back the original state. If the mux
should idle-as-is, you can avoid a maximum of one mux update if the first
consumer happens to starts by requesting the previously active state.
Similarly, if the mux should idle in a specific state, you can avoid a
maximum of one mux update.

In both cases it costs one unconditional read of the mux state.

Sure, in some cases reads are cheaper than writes, but I didn't think
support for seeding the cache was worth it. Is it worth it?

Cheers,
peda

^ permalink raw reply

* [PATCH] iio: adc: add driver for the ti-adc084s021 chip
From: Mårten Lindahl @ 2017-04-21 14:58 UTC (permalink / raw)
  To: jic23-DgEjT+Ai2ygdnm+yROfE0A
  Cc: knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
	pmeerw-jW+XmwGofnusTnJN9+BGXg, linux-iio-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Mårten Lindahl

From: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>

This adds support for the Texas Instruments ADC084S021 ADC chip.

Signed-off-by: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
---
 .../devicetree/bindings/iio/adc/ti-adc084s021.txt  |  25 ++
 drivers/iio/adc/Kconfig                            |  12 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/ti-adc084s021.c                    | 342 +++++++++++++++++++++
 4 files changed, 380 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
 create mode 100644 drivers/iio/adc/ti-adc084s021.c

diff --git a/Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt b/Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
new file mode 100644
index 0000000..921eb46
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
@@ -0,0 +1,25 @@
+* Texas Instruments' ADC084S021
+
+Required properties:
+ - compatible        : Must be "ti,adc084s021"
+ - reg               : SPI chip select number for the device
+ - vref-supply       : The regulator supply for ADC reference voltage
+ - spi-max-frequency : Definition as per Documentation/devicetree/bindings/spi/spi-bus.txt
+
+Optional properties:
+ - spi-cpol          : SPI inverse clock polarity, as per spi-bus bindings
+ - spi-cpha          : SPI shifted clock phase (CPHA), as per spi-bus bindings
+ - spi-cs-high       : SPI chip select active high, as per spi-bus bindings
+
+
+Example:
+adc@0 {
+	compatible = "ti,adc084s021";
+	reg = <0>;
+	vref-supply = <&adc_vref>;
+	spi-cpol;
+	spi-cpha;
+	spi-cs-high;
+	spi-max-frequency = <16000000>;
+	pl022,com-mode = <0x2>; /* DMA */
+};
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index dedae7a..13141e5 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -560,6 +560,18 @@ config TI_ADC0832
 	  This driver can also be built as a module. If so, the module will be
 	  called ti-adc0832.
 
+config TI_ADC084S021
+	tristate "Texas Instruments ADC084S021"
+	depends on SPI
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	help
+	  If you say yes here you get support for Texas Instruments ADC084S021
+	  chips.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called ti-adc084s021.
+
 config TI_ADC12138
 	tristate "Texas Instruments ADC12130/ADC12132/ADC12138"
 	depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index d001262..b1a6158 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_STM32_ADC_CORE) += stm32-adc-core.o
 obj-$(CONFIG_STM32_ADC) += stm32-adc.o
 obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
 obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
+obj-$(CONFIG_TI_ADC084S021) += ti-adc084s021.o
 obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
 obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
 obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o
diff --git a/drivers/iio/adc/ti-adc084s021.c b/drivers/iio/adc/ti-adc084s021.c
new file mode 100644
index 0000000..4f33b91
--- /dev/null
+++ b/drivers/iio/adc/ti-adc084s021.c
@@ -0,0 +1,342 @@
+/**
+ * Copyright (C) 2017 Axis Communications AB
+ *
+ * Driver for Texas Instruments' ADC084S021 ADC chip.
+ * Datasheets can be found here:
+ * http://www.ti.com/lit/ds/symlink/adc084s021.pdf
+ *
+ * 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/err.h>
+#include <linux/spi/spi.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/events.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/regulator/consumer.h>
+
+#define MODULE_NAME     "adc084s021"
+#define DRIVER_VERSION  "1.0"
+#define ADC_RESOLUTION  8
+#define ADC_N_CHANNELS  4
+
+struct adc084s021_configuration {
+	const struct iio_chan_spec *channels;
+	u8 num_channels;
+};
+
+struct adc084s021 {
+	struct spi_device *spi;
+	struct spi_message message;
+	struct spi_transfer spi_trans[2];
+	struct regulator *reg;
+	struct mutex lock;
+	/*
+	 * DMA (thus cache coherency maintenance) requires the
+	 * transfer buffers to live in their own cache lines.
+	 */
+	union {
+		u8 tx_buf[2];
+		u8 rx_buf[2];
+	} ____cacheline_aligned;
+	u8 cur_adc_values[ADC_N_CHANNELS];
+};
+
+/**
+ * Event triggered when value changes on a channel
+ */
+static const struct iio_event_spec adc084s021_event = {
+	.type = IIO_EV_TYPE_CHANGE,
+	.dir = IIO_EV_DIR_NONE,
+};
+
+/**
+ * Channel specification
+ */
+#define ADC084S021_VOLTAGE_CHANNEL(num)                  \
+	{                                                      \
+		.type = IIO_VOLTAGE,                                 \
+		.channel = (num),                                    \
+		.address = (num << 3),                               \
+		.indexed = 1,                                        \
+		.scan_index = num,                                   \
+		.scan_type = {                                       \
+			.sign = 'u',                                       \
+			.realbits = 8,                                     \
+			.storagebits = 32,                                 \
+			.shift = 24 - ((num << 3)),                        \
+		},                                                   \
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),        \
+		.event_spec = &adc084s021_event,                     \
+		.num_event_specs = 1,                                \
+	}
+
+static const struct iio_chan_spec adc084s021_channels[] = {
+	ADC084S021_VOLTAGE_CHANNEL(0),
+	ADC084S021_VOLTAGE_CHANNEL(1),
+	ADC084S021_VOLTAGE_CHANNEL(2),
+	ADC084S021_VOLTAGE_CHANNEL(3),
+	IIO_CHAN_SOFT_TIMESTAMP(4),
+};
+
+static const struct adc084s021_configuration adc084s021_config[] = {
+	{ adc084s021_channels, ARRAY_SIZE(adc084s021_channels) },
+};
+
+/**
+ * Read an ADC channel and return its value.
+ *
+ * @adc: The ADC SPI data.
+ * @channel: The IIO channel data structure.
+ */
+static int adc084s021_adc_conversion(struct adc084s021 *adc,
+			   struct iio_chan_spec const *channel)
+{
+	u16 value;
+	int ret;
+
+	mutex_lock(&adc->lock);
+	adc->tx_buf[0] = channel->address;
+
+	/* Do the transfer */
+	ret = spi_sync(adc->spi, &adc->message);
+
+	if (ret < 0) {
+		mutex_unlock(&adc->lock);
+		return ret;
+	}
+
+	value = (adc->rx_buf[0] << 4) | (adc->rx_buf[1] >> 4);
+	mutex_unlock(&adc->lock);
+
+	dev_dbg(&adc->spi->dev, "value 0x%02X on channel %d\n",
+			     value, channel->channel);
+	return value;
+}
+
+/**
+ * Make a readout of requested IIO channel info.
+ *
+ * @indio_dev: The industrial I/O device.
+ * @channel: The IIO channel data structure.
+ * @val: First element of value (integer).
+ * @val2: Second element of value (fractional).
+ * @mask: The info_mask to read.
+ */
+static int adc084s021_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *channel, int *val,
+			   int *val2, long mask)
+{
+	struct adc084s021 *adc = iio_priv(indio_dev);
+	int retval;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		retval = adc084s021_adc_conversion(adc, channel);
+		if (retval < 0)
+			return retval;
+
+		*val = retval;
+		return IIO_VAL_INT;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+/**
+ * Read enabled ADC channels and push data to the buffer.
+ *
+ * @irq: The interrupt number (not used).
+ * @pollfunc: Pointer to the poll func.
+ */
+static irqreturn_t adc084s021_trigger_handler(int irq, void *pollfunc)
+{
+	struct iio_poll_func *pf = pollfunc;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct adc084s021 *adc = iio_priv(indio_dev);
+	u8 *data;
+	s64 timestamp;
+	int value, scan_index;
+
+	data = kzalloc(indio_dev->scan_bytes, GFP_KERNEL);
+	if (!data) {
+		iio_trigger_notify_done(indio_dev->trig);
+		return IRQ_NONE;
+	}
+
+	timestamp = iio_get_time_ns(indio_dev);
+
+	for_each_set_bit(scan_index, indio_dev->active_scan_mask,
+			 indio_dev->masklength) {
+		const struct iio_chan_spec *channel =
+			&indio_dev->channels[scan_index];
+		value = adc084s021_adc_conversion(adc, channel);
+		data[scan_index] = value;
+
+		/*
+		 * Compare read data to previous read. If it differs send
+		 * event notification for affected channel.
+		 */
+		if (adc->cur_adc_values[scan_index] != (u8)value) {
+			adc->cur_adc_values[scan_index] = (u8)value;
+			iio_push_event(indio_dev,
+						  IIO_EVENT_CODE(IIO_VOLTAGE, 0,
+						    IIO_NO_MOD, IIO_EV_DIR_NONE,
+						    IIO_EV_TYPE_CHANGE,
+						    channel->channel, 0, 0),
+						  timestamp);
+			dev_dbg(&indio_dev->dev,
+					    "new value on ch%d: 0x%02X (ts %llu)\n",
+					    channel->channel, value, timestamp);
+		}
+	}
+
+	iio_push_to_buffers_with_timestamp(indio_dev, data, timestamp);
+	iio_trigger_notify_done(indio_dev->trig);
+	kfree(data);
+	return IRQ_HANDLED;
+}
+
+static const struct iio_info adc084s021_info = {
+	.read_raw = adc084s021_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
+/**
+ * Create and register ADC IIO device for SPI.
+ */
+static int adc084s021_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct adc084s021 *adc;
+	int config = spi_get_device_id(spi)->driver_data;
+	int retval;
+
+	/* Allocate an Industrial I/O device */
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
+	if (!indio_dev) {
+		dev_err(&spi->dev, "Failed to allocate IIO device\n");
+		return -ENOMEM;
+	}
+
+	adc = iio_priv(indio_dev);
+	adc->spi = spi;
+	spi->bits_per_word = ADC_RESOLUTION;
+
+	/* Update the SPI device with config and connect the iio dev */
+	retval = spi_setup(spi);
+	if (retval) {
+		dev_err(&spi->dev, "Failed to update SPI device\n");
+		return retval;
+	}
+	spi_set_drvdata(spi, indio_dev);
+
+	/* Initiate the Industrial I/O device */
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->dev.of_node = spi->dev.of_node;
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &adc084s021_info;
+	indio_dev->channels = adc084s021_config[config].channels;
+	indio_dev->num_channels = adc084s021_config[config].num_channels;
+
+	/* Create SPI transfer for channel reads */
+	adc->spi_trans[0].tx_buf = &adc->tx_buf[0];
+	adc->spi_trans[0].len = 2;
+	adc->spi_trans[0].speed_hz = spi->max_speed_hz;
+	adc->spi_trans[0].bits_per_word = spi->bits_per_word;
+	adc->spi_trans[1].rx_buf = &adc->rx_buf[0];
+	adc->spi_trans[1].len = 2;
+	adc->spi_trans[1].speed_hz = spi->max_speed_hz;
+	adc->spi_trans[1].bits_per_word = spi->bits_per_word;
+
+	/* Setup SPI message for channel reads */
+	spi_message_init(&adc->message);
+	spi_message_add_tail(&adc->spi_trans[0], &adc->message);
+	spi_message_add_tail(&adc->spi_trans[1], &adc->message);
+
+	adc->reg = devm_regulator_get(&spi->dev, "vref");
+	if (IS_ERR(adc->reg))
+		return PTR_ERR(adc->reg);
+
+	retval = regulator_enable(adc->reg);
+	if (retval < 0)
+		return retval;
+
+	mutex_init(&adc->lock);
+
+	/* Setup triggered buffer with pollfunction */
+	retval = iio_triggered_buffer_setup(indio_dev, NULL,
+					    adc084s021_trigger_handler, NULL);
+	if (retval) {
+		dev_err(&spi->dev, "Failed to setup triggered buffer\n");
+		goto buffer_setup_failed;
+	}
+
+	retval = iio_device_register(indio_dev);
+	if (retval) {
+		dev_err(&spi->dev, "Failed to register IIO device\n");
+		goto device_register_failed;
+	}
+
+	dev_info(&spi->dev, "probed!\n");
+	return 0;
+
+device_register_failed:
+	iio_triggered_buffer_cleanup(indio_dev);
+buffer_setup_failed:
+	regulator_disable(adc->reg);
+	return retval;
+}
+
+/**
+ * Unregister ADC IIO device for SPI.
+ */
+static int adc084s021_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct adc084s021 *adc = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+	iio_triggered_buffer_cleanup(indio_dev);
+	regulator_disable(adc->reg);
+	return 0;
+}
+
+static const struct of_device_id adc084s021_of_match[] = {
+	{ .compatible = "ti,adc084s021", },
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, adc084s021_of_match);
+
+static const struct spi_device_id adc084s021_id[] = {
+	{ MODULE_NAME, 0},
+	{}
+};
+
+MODULE_DEVICE_TABLE(spi, adc084s021_id);
+
+static struct spi_driver adc084s021_driver = {
+	.driver = {
+		.name = MODULE_NAME,
+		.of_match_table = of_match_ptr(adc084s021_of_match),
+	},
+	.probe = adc084s021_probe,
+	.remove = adc084s021_remove,
+	.id_table = adc084s021_id,
+};
+
+module_spi_driver(adc084s021_driver);
+
+MODULE_AUTHOR("Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>");
+MODULE_DESCRIPTION("Texas Instruments ADC084S021");
+MODULE_LICENSE("GPL v2");
+MODULE_VERSION(DRIVER_VERSION);
-- 
2.1.4

^ permalink raw reply related

* Re: [PATCH v13 03/10] mux: minimal mux subsystem and gpio-based mux controller
From: Peter Rosin @ 2017-04-21 15:08 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: linux-kernel, Greg Kroah-Hartman, Wolfram Sang, Rob Herring,
	Mark Rutland, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	linux-i2c, devicetree, linux-iio, linux-doc, Andrew Morton,
	Colin Ian King, Paul Gortmaker, kernel
In-Reply-To: <1492784318.2364.8.camel@pengutronix.de>

On 2017-04-21 16:18, Philipp Zabel wrote:
> Hi Peter,
> 
> On Thu, 2017-04-13 at 18:43 +0200, Peter Rosin wrote:
> [...]
>> +int mux_control_select(struct mux_control *mux, int state)
> 
> state could be unsigned int for the consumer facing API.
> 
>> +{
>> +	int ret;
> 
> And mux_control_select should check that (0 <= state < mux->states).

Yes, that makes sense. I worried that we might end up with
signed/unsigned comparisons since the internal state still needs
to be signed, but that didn't happen when I tried...

I'll include this change in v14.

Cheers,
peda

^ permalink raw reply

* Re: [PATCH v2 2/2] dmaengine: Add DW AXI DMAC driver
From: Andy Shevchenko @ 2017-04-21 15:13 UTC (permalink / raw)
  To: Eugeniy Paltsev
  Cc: devicetree@vger.kernel.org, vinod.koul@intel.com,
	Alexey.Brodkin@synopsys.com, linux-kernel@vger.kernel.org,
	robh+dt@kernel.org, dmaengine@vger.kernel.org,
	dan.j.williams@intel.com, linux-snps-arc@lists.infradead.org
In-Reply-To: <1492784977.16657.6.camel@synopsys.com>

On Fri, 2017-04-21 at 14:29 +0000, Eugeniy Paltsev wrote:
> On Tue, 2017-04-18 at 15:31 +0300, Andy Shevchenko wrote:
> > On Fri, 2017-04-07 at 17:04 +0300, Eugeniy Paltsev wrote:
> > > This patch adds support for the DW AXI DMAC controller.
> > > 

> > > +#include <linux/of.h>
> > 
> > Are you sure you still need of.h along with depends OF ?
> 
> "of_match_ptr" used from of.h

It safe not to use it and always have a table. In this case the driver
even would be available for ACPI-enabled platforms (I suppose some ARM64
might find this useful).

> > > +#define AXI_DMA_BUSWIDTHS		  \
> > > +	(DMA_SLAVE_BUSWIDTH_1_BYTE	| \
> > > +	DMA_SLAVE_BUSWIDTH_2_BYTES	| \
> > > +	DMA_SLAVE_BUSWIDTH_4_BYTES	| \
> > > +	DMA_SLAVE_BUSWIDTH_8_BYTES	| \
> > > +	DMA_SLAVE_BUSWIDTH_16_BYTES	| \
> > > +	DMA_SLAVE_BUSWIDTH_32_BYTES	| \
> > > +	DMA_SLAVE_BUSWIDTH_64_BYTES)
> > > +/* TODO: check: do we need to use BIT() macro here? */
> > 
> > Still TODO? I remember I answered to this on the first round.
> 
> Yes, I remember it.
> I left this TODO as a reminder because src_addr_widths and
> dst_addr_widths are
> not used anywhere and they are set differently in different drivers
> (with or without BIT macro).

Strange. AFAIK they are representing bits (which is not the best idea)
in the resulting u64 field. So, anything bigger than 63 doesn't make
sense. In drivers where they are not bits quite likely a bug is hidden.

> 
> > > +
> > > +static inline void
> > > +axi_dma_iowrite32(struct axi_dma_chip *chip, u32 reg, u32 val)
> > > +{
> > > +	iowrite32(val, chip->regs + reg);
> > 
> > Are you going to use IO ports for this IP? I don't think so.
> > Wouldn't be better to call readl()/writel() instead?
> 
> As I understand, it's better to use ioread/iowrite as more universal
> IO
> access way. Am I wrong?

As I said above the ioreadX/iowriteX makes only sense when your IP would
be accessed via IO region or MMIO. I'm pretty sure IO is not the case at
all for this IP.

> > > +static inline void axi_chan_irq_disable(struct axi_dma_chan
> > > *chan,
> > > u32 irq_mask)
> > > +{
> > > +	u32 val;
> > > +
> > > +	if (likely(irq_mask == DWAXIDMAC_IRQ_ALL)) {
> > > +		axi_chan_iowrite32(chan, CH_INTSTATUS_ENA,
> > > DWAXIDMAC_IRQ_NONE);
> > > +	} else {
> > 
> > I don't see the benefit. (Yes, I see one read-less path, I think it
> > makes perplexity for nothing here)
> 
> This function is called mostly with irq_mask = DWAXIDMAC_IRQ_ALL.
> Actually it is called only with irq_mask = DWAXIDMAC_IRQ_ALL until I
> add DMA_SLAVE support.
> But I can cut off this 'if' statment, if it is necessery.

It's not necessary, but from my point of view increases readability of
the code a lot for the price of an additional readl().

> 
> > > +		val = axi_chan_ioread32(chan, CH_INTSTATUS_ENA);
> > > +		val &= ~irq_mask;
> > > +		axi_chan_iowrite32(chan, CH_INTSTATUS_ENA, val);
> > > +	}

> > > +
> > > +	return min_t(size_t, __ffs(sdl), max_width);
> > > +}
> > > +static void axi_desc_put(struct axi_dma_desc *desc)
> > > +{
> > > +	struct axi_dma_chan *chan = desc->chan;
> > > +	struct dw_axi_dma *dw = chan->chip->dw;
> > > +	struct axi_dma_desc *child, *_next;
> > > +	unsigned int descs_put = 0;
> > > +	list_for_each_entry_safe(child, _next, &desc->xfer_list,
> > > xfer_list) {
> > 
> > xfer_list looks redundant.
> > Can you elaborate why virtual channel management is not working for
> > you?
> 
> Each virtual descriptor encapsulates several hardware descriptors,
> which belong to same transfer.
> This list (xfer_list) is used only for allocating/freeing these
> descriptors and it doesn't affect on virtual dma work logic.
> I can see this approach in several drivers with VirtDMA (but they
> mostly use array instead of list)

You described how most of the DMA drivers are implemented, though they
are using just sg_list directly. I would recommend to do the same and
get rid of this list.

> > Btw, are you planning to use priority at all? For now on I didn't
> > see
> > a single driver (from the set I have checked, like 4-5 of them) that
> > uses priority anyhow. It makes driver more complex for nothing.
> 
> Only for dma slave operations.

So, in other words you *have* an actual two or more users that *need*
prioritization?

> > > +	if (unlikely(dst_nents == 0 || src_nents == 0))
> > > +		return NULL;
> > > +
> > > +	if (unlikely(dst_sg == NULL || src_sg == NULL))
> > > +		return NULL;
> > 
> > If we need those checks they should go to dmaengine.h/dmaengine.c.
> 
> I checked several drivers, which implements device_prep_dma_sg and
> they
> implements this checkers.
> Should I add something like "dma_sg_desc_invalid" function to
> dmaengine.h ?

I suppose it's better to implement them in the corresponding helpers in
dmaengine.h.

> > > +static void axi_chan_list_dump_lli(struct axi_dma_chan *chan,
> > > +				   struct axi_dma_desc
> > > *desc_head)
> > > +{
> > > +	struct axi_dma_desc *desc;
> > > +
> > > +	axi_chan_dump_lli(chan, desc_head);
> > > +	list_for_each_entry(desc, &desc_head->xfer_list,
> > > xfer_list)
> > > +		axi_chan_dump_lli(chan, desc);
> > > +}
> > > +
> > > +static void axi_chan_handle_err(struct axi_dma_chan *chan, u32
> > > status)
> > > +{
> > > +	/* WARN about bad descriptor */
> > > 
> > > +	dev_err(chan2dev(chan),
> > > +		"Bad descriptor submitted for %s, cookie: %d,
> > > irq:
> > > 0x%08x\n",
> > > +		axi_chan_name(chan), vd->tx.cookie, status);
> > > +	axi_chan_list_dump_lli(chan, vd_to_axi_desc(vd));
> > 
> > As I said earlier dw_dmac is *bad* example of the (virtual channel
> > based) DMA driver.
> > 
> > I guess you may just fail the descriptor and don't pretend it has
> > been processed successfully.
> 
> What do you mean by saying "fail the descriptor"?
> After I get error I cancel current transfer and free all descriptors
> from it (by calling vchan_cookie_complete).
> I can't store error status in descriptor structure because it will be
> freed by vchan_cookie_complete.
> I can't store error status in channel structure because it will be
> overwritten by next transfer.

Better not to pretend that it has been processed successfully. Don't
call callback on it and set its status to DMA_ERROR (that's why
descriptors in many drivers have dma_status field). When user asks for
status (using cookie) the saved value would be returned until descriptor
is active. 

Do you have some other workflow in mind?

> > > +
> > > +static const struct dev_pm_ops dw_axi_dma_pm_ops = {
> > > +	SET_RUNTIME_PM_OPS(axi_dma_runtime_suspend,
> > > axi_dma_runtime_resume, NULL)
> > > +};
> > 
> > Have you tried to build with CONFIG_PM disabled?
> 
> Yes.
> 
> > I'm pretty sure you need __maybe_unused applied to your PM ops.
> 
> I call axi_dma_runtime_suspend / axi_dma_runtime_resume even I dont't
> use PM.
> (I call them in probe / remove function.)

Hmm... I didn't check your ->probe() and ->remove(). Do you mean you
call them explicitly by those names?

If so, please don't do that. Use pm_runtime_*() instead. And...

> So I don't need to declare them with __maybe_unused.

...in that case it's possible you have them defined but not used.


> >> +struct axi_dma_chan {
> > > +	struct axi_dma_chip		*chip;
> > > +	void __iomem			*chan_regs;
> > > +	u8				id;
> > > +	atomic_t			descs_allocated;
> > > +
> > > +	struct virt_dma_chan		vc;
> > > +
> > > +	/* these other elements are all protected by vc.lock */
> > > +	bool				is_paused;
> > 
> > I still didn't get (already forgot) why you can't use dma_status
> > instead for the active descriptor?
> 
> As I said before, I checked several driver, which have status variable
> in their channel structure - it is used *only* for determinating is
> channel paused or not. So there is no much sense in replacing
> "is_paused" to "status" and I left "is_paused" variable untouched.

Not only (see above), the errored descriptor keeps that status.

> (I described above why we can't use status in channel structure for
> error handling)

Ah, I'm talking about descriptor.

> > > Status Fetch Addr */
> > > +#define CH_INTSTATUS_ENA	0x080 /* R/W Chan Interrupt
> > > Status
> > > Enable */
> > > +#define CH_INTSTATUS		0x088 /* R/W Chan Interrupt
> > > Status */
> > > +#define CH_INTSIGNAL_ENA	0x090 /* R/W Chan Interrupt
> > > Signal
> > > Enable */
> > > +#define CH_INTCLEAR		0x098 /* W Chan Interrupt
> > > Clear
> > > */
> > 
> > I'm wondering if you can use regmap API instead.
> 
> Is it really necessary? It'll make driver more complex for nothing.

That's why I'm not suggesting but wondering.

> > > +	DWAXIDMAC_BURST_TRANS_LEN_1024
> > 
> > ..._1024, ?
> 
> What exactly you are asking about?

Comma at the end.

> 
> > > +};
> > 
> > Hmm... do you need them in the header?
> 
> I use some of these definitions in my code so I guess yes.
> /* Maybe I misunderstood your question... */

I mean, who are the users of them? If it's only one module, there is no
need to put them in header.

> 
> > > +#define CH_CFG_H_TT_FC_POS	0
> > > +enum {
> > > +	DWAXIDMAC_TT_FC_MEM_TO_MEM_DMAC	= 0x0,
> > > +	DWAXIDMAC_TT_FC_MEM_TO_PER_DMAC,
> > > +	DWAXIDMAC_TT_FC_PER_TO_MEM_DMAC,
> > > +	DWAXIDMAC_TT_FC_PER_TO_PER_DMAC,
> > > +	DWAXIDMAC_TT_FC_PER_TO_MEM_SRC,
> > > +	DWAXIDMAC_TT_FC_PER_TO_PER_SRC,
> > > +	DWAXIDMAC_TT_FC_MEM_TO_PER_DST,
> > > +	DWAXIDMAC_TT_FC_PER_TO_PER_DST
> > > +};
> > 
> > Some of definitions are the same as for dw_dmac, right? We might
> > split them to a common header, though I have no strong opinion about
> 
> it.
> > Vinod?
> 
> APB DMAC and AXI DMAC have completely different regmap. So there is no
> much sense to do that.

I'm not talking about registers, I'm talking about definitions like
above. Though it would be indeed little sense to split some common code
between those two.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

^ permalink raw reply

* Re: [PATCH 13/15] drm/sun4i: Add HDMI support
From: Chen-Yu Tsai @ 2017-04-21 15:17 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mike Turquette, Stephen Boyd, Chen-Yu Tsai, dri-devel,
	Daniel Vetter, David Airlie, Mark Rutland, Rob Herring,
	devicetree, linux-clk, linux-arm-kernel, linux-kernel,
	linux-sunxi
In-Reply-To: <c63d01aedeccc58bf8d6f5bfd66d8595156e9491.1488876832.git-series.maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Hi,

On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard
<maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> The earlier Allwinner SoCs (A10, A10s, A20, A31) have an embedded HDMI
> controller.
>
> That HDMI controller is able to do audio and CEC, but those have been left
> out for now.
>
> Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  drivers/gpu/drm/sun4i/Makefile              |   5 +-
>  drivers/gpu/drm/sun4i/sun4i_hdmi.h          | 124 ++++++-
>  drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c  | 128 ++++++-
>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c      | 449 +++++++++++++++++++++-
>  drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c | 236 +++++++++++-
>  5 files changed, 942 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi.h
>  create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c
>  create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
>  create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c

Applying patch #9608371 using 'git am'
Description: [13/15] drm/sun4i: Add HDMI support
Applying: drm/sun4i: Add HDMI support
.git/rebase-apply/patch:116: trailing whitespace.

.git/rebase-apply/patch:531: trailing whitespace.

.git/rebase-apply/patch:701: trailing whitespace.

warning: 3 lines add whitespace errors.

> diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile
> index 59b757350a1f..68a0f6244a59 100644
> --- a/drivers/gpu/drm/sun4i/Makefile
> +++ b/drivers/gpu/drm/sun4i/Makefile
> @@ -7,7 +7,12 @@ sun4i-tcon-y += sun4i_dotclock.o
>  sun4i-tcon-y += sun4i_crtc.o
>  sun4i-tcon-y += sun4i_layer.o
>
> +sun4i-drm-hdmi-y += sun4i_hdmi_enc.o
> +sun4i-drm-hdmi-y += sun4i_hdmi_ddc_clk.o
> +sun4i-drm-hdmi-y += sun4i_hdmi_tmds_clk.o
> +
>  obj-$(CONFIG_DRM_SUN4I)                += sun4i-drm.o sun4i-tcon.o
>  obj-$(CONFIG_DRM_SUN4I)                += sun4i_backend.o
>  obj-$(CONFIG_DRM_SUN4I)                += sun6i_drc.o
> +obj-$(CONFIG_DRM_SUN4I)                += sun4i-drm-hdmi.o
>  obj-$(CONFIG_DRM_SUN4I)                += sun4i_tv.o
> diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi.h b/drivers/gpu/drm/sun4i/sun4i_hdmi.h
> new file mode 100644
> index 000000000000..2ad25b8fd3cd
> --- /dev/null
> +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi.h
> @@ -0,0 +1,124 @@
> +/*
> + * Copyright (C) 2016 Maxime Ripard
> + *
> + * Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@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 as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +
> +#ifndef _SUN4I_HDMI_H_
> +#define _SUN4I_HDMI_H_
> +
> +#include <drm/drm_connector.h>
> +#include <drm/drm_encoder.h>
> +
> +#define SUN4I_HDMI_CTRL_REG            0x004
> +#define SUN4I_HDMI_CTRL_ENABLE                 BIT(31)
> +
> +#define SUN4I_HDMI_IRQ_REG             0x008
> +#define SUN4I_HDMI_IRQ_STA_MASK                        0x73
> +#define SUN4I_HDMI_IRQ_STA_FIFO_OF             BIT(1)
> +#define SUN4I_HDMI_IRQ_STA_FIFO_UF             BIT(0)
> +
> +#define SUN4I_HDMI_HPD_REG             0x00c
> +#define SUN4I_HDMI_HPD_HIGH                    BIT(0)
> +
> +#define SUN4I_HDMI_VID_CTRL_REG                0x010
> +#define SUN4I_HDMI_VID_CTRL_ENABLE             BIT(31)
> +#define SUN4I_HDMI_VID_CTRL_HDMI_MODE          BIT(30)
> +
> +#define SUN4I_HDMI_VID_TIMING_ACT_REG  0x014
> +#define SUN4I_HDMI_VID_TIMING_BP_REG   0x018
> +#define SUN4I_HDMI_VID_TIMING_FP_REG   0x01c
> +#define SUN4I_HDMI_VID_TIMING_SPW_REG  0x020
> +
> +#define SUN4I_HDMI_VID_TIMING_X(x)             ((((x) - 1) & GENMASK(11, 0)))
> +#define SUN4I_HDMI_VID_TIMING_Y(y)             ((((y) - 1) & GENMASK(11, 0)) << 16)
> +
> +#define SUN4I_HDMI_VID_TIMING_POL_REG  0x024
> +#define SUN4I_HDMI_VID_TIMING_POL_TX_CLK        (0x3e0 << 16)
> +#define SUN4I_HDMI_VID_TIMING_POL_VSYNC                BIT(1)
> +#define SUN4I_HDMI_VID_TIMING_POL_HSYNC                BIT(0)
> +
> +#define SUN4I_HDMI_AVI_INFOFRAME_REG(n)        (0x080 + (n))
> +
> +#define SUN4I_HDMI_PAD_CTRL0_REG       0x200
> +
> +#define SUN4I_HDMI_PAD_CTRL1_REG       0x204
> +#define SUN4I_HDMI_PAD_CTRL1_HALVE_CLK         BIT(6)
> +
> +#define SUN4I_HDMI_PLL_CTRL_REG                0x208
> +#define SUN4I_HDMI_PLL_CTRL_DIV(n)             ((n) << 4)
> +#define SUN4I_HDMI_PLL_CTRL_DIV_MASK           GENMASK(7, 4)
> +
> +#define SUN4I_HDMI_PLL_DBG0_REG                0x20c
> +#define SUN4I_HDMI_PLL_DBG0_TMDS_PARENT(n)     (((n) & 1) << 21)
> +#define SUN4I_HDMI_PLL_DBG0_TMDS_PARENT_MASK   BIT(21)
> +#define SUN4I_HDMI_PLL_DBG0_TMDS_PARENT_SHIFT  21
> +
> +#define SUN4I_HDMI_PKT_CTRL_REG(n)     (0x2f0 + (4 * (n)))
> +#define SUN4I_HDMI_PKT_CTRL_TYPE(n, t)         ((t) << (((n) % 4) * 4))
> +
> +#define SUN4I_HDMI_UNKNOWN_REG         0x300
> +#define SUN4I_HDMI_UNKNOWN_INPUT_SYNC          BIT(27)
> +
> +#define SUN4I_HDMI_DDC_CTRL_REG                0x500
> +#define SUN4I_HDMI_DDC_CTRL_ENABLE             BIT(31)
> +#define SUN4I_HDMI_DDC_CTRL_START_CMD          BIT(30)
> +#define SUN4I_HDMI_DDC_CTRL_RESET              BIT(0)
> +
> +#define SUN4I_HDMI_DDC_ADDR_REG                0x504
> +#define SUN4I_HDMI_DDC_ADDR_SEGMENT(seg)       (((seg) & 0xff) << 24)
> +#define SUN4I_HDMI_DDC_ADDR_EDDC(addr)         (((addr) & 0xff) << 16)
> +#define SUN4I_HDMI_DDC_ADDR_OFFSET(off)                (((off) & 0xff) << 8)
> +#define SUN4I_HDMI_DDC_ADDR_SLAVE(addr)                ((addr) & 0xff)
> +
> +#define SUN4I_HDMI_DDC_FIFO_CTRL_REG   0x510
> +#define SUN4I_HDMI_DDC_FIFO_CTRL_CLEAR         BIT(31)
> +
> +#define SUN4I_HDMI_DDC_FIFO_DATA_REG   0x518
> +#define SUN4I_HDMI_DDC_BYTE_COUNT_REG  0x51c
> +
> +#define SUN4I_HDMI_DDC_CMD_REG         0x520
> +#define SUN4I_HDMI_DDC_CMD_EXPLICIT_EDDC_READ  6
> +
> +#define SUN4I_HDMI_DDC_CLK_REG         0x528
> +#define SUN4I_HDMI_DDC_CLK_M(m)                        (((m) & 0x7) << 3)
> +#define SUN4I_HDMI_DDC_CLK_N(n)                        ((n) & 0x7)
> +
> +#define SUN4I_HDMI_DDC_LINE_CTRL_REG   0x540
> +#define SUN4I_HDMI_DDC_LINE_CTRL_SDA_ENABLE    BIT(9)
> +#define SUN4I_HDMI_DDC_LINE_CTRL_SCL_ENABLE    BIT(8)
> +
> +#define SUN4I_HDMI_DDC_FIFO_SIZE       16
> +
> +enum sun4i_hdmi_pkt_type {
> +       SUN4I_HDMI_PKT_AVI = 2,
> +       SUN4I_HDMI_PKT_END = 15,
> +};
> +
> +struct sun4i_hdmi {
> +       struct drm_connector    connector;
> +       struct drm_encoder      encoder;
> +       struct device           *dev;
> +
> +       void __iomem            *base;
> +       struct clk              *bus_clk;
> +       struct clk              *ddc_clk;
> +       struct clk              *mod_clk;
> +       struct clk              *pll0_clk;
> +       struct clk              *pll1_clk;
> +       struct clk              *tmds_clk;
> +
> +       struct sun4i_drv        *drv;
> +
> +       bool                    hdmi_monitor;
> +};
> +
> +int sun4i_ddc_create(struct sun4i_hdmi *hdmi, struct clk *clk);
> +int sun4i_tmds_create(struct sun4i_hdmi *hdmi);
> +
> +#endif /* _SUN4I_HDMI_H_ */
> diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c
> new file mode 100644
> index 000000000000..5125b14ea7a5
> --- /dev/null
> +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c
> @@ -0,0 +1,128 @@
> +/*
> + * Copyright (C) 2016 Free Electrons
> + * Copyright (C) 2016 NextThing Co
> + *
> + * Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@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 as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +
> +#include <linux/clk-provider.h>
> +
> +#include "sun4i_tcon.h"
> +#include "sun4i_hdmi.h"
> +
> +struct sun4i_ddc {
> +       struct clk_hw           hw;
> +       struct sun4i_hdmi       *hdmi;
> +};
> +
> +static inline struct sun4i_ddc *hw_to_ddc(struct clk_hw *hw)
> +{
> +       return container_of(hw, struct sun4i_ddc, hw);
> +}
> +
> +static unsigned long sun4i_ddc_calc_divider(unsigned long rate,
> +                                           unsigned long parent_rate,
> +                                           u8 *m, u8 *n)
> +{
> +       unsigned long best_rate = 0;
> +       u8 best_m = 0, best_n = 0, _m, _n;
> +
> +       for (_m = 0; _m < 8; _m++) {
> +               for (_n = 0; _n < 8; _n++) {
> +                       unsigned long tmp_rate;
> +
> +                       tmp_rate = (((parent_rate / 2) / 10) >> _n) / (_m + 1);
> +
> +                       if (tmp_rate > rate)
> +                               continue;
> +
> +                       if (abs(rate - tmp_rate) < abs(rate - best_rate)) {
> +                               best_rate = tmp_rate;
> +                               best_m = _m;
> +                               best_n = _n;
> +                       }
> +               }
> +       }
> +
> +       if (m && n) {
> +               *m = best_m;
> +               *n = best_n;
> +       }
> +
> +       return best_rate;
> +}
> +
> +static long sun4i_ddc_round_rate(struct clk_hw *hw, unsigned long rate,
> +                                unsigned long *prate)
> +{
> +       return sun4i_ddc_calc_divider(rate, *prate, NULL, NULL);
> +}
> +
> +static unsigned long sun4i_ddc_recalc_rate(struct clk_hw *hw,
> +                                           unsigned long parent_rate)
> +{
> +       struct sun4i_ddc *ddc = hw_to_ddc(hw);
> +       u32 reg;
> +       u8 m, n;
> +
> +       reg = readl(ddc->hdmi->base + SUN4I_HDMI_DDC_CLK_REG);
> +       m = (reg >> 3) & 0x7;
> +       n = reg & 0x7;
> +
> +       return (((parent_rate / 2) / 10) >> n) / (m + 1);
> +}
> +
> +static int sun4i_ddc_set_rate(struct clk_hw *hw, unsigned long rate,
> +                             unsigned long parent_rate)
> +{
> +       struct sun4i_ddc *ddc = hw_to_ddc(hw);
> +       u8 div_m, div_n;
> +
> +       sun4i_ddc_calc_divider(rate, parent_rate, &div_m, &div_n);
> +
> +       writel(SUN4I_HDMI_DDC_CLK_M(div_m) | SUN4I_HDMI_DDC_CLK_N(div_n),
> +              ddc->hdmi->base + SUN4I_HDMI_DDC_CLK_REG);
> +
> +       return 0;
> +}
> +
> +static const struct clk_ops sun4i_ddc_ops = {
> +       .recalc_rate    = sun4i_ddc_recalc_rate,
> +       .round_rate     = sun4i_ddc_round_rate,
> +       .set_rate       = sun4i_ddc_set_rate,
> +};
> +
> +int sun4i_ddc_create(struct sun4i_hdmi *hdmi, struct clk *parent)
> +{
> +       struct clk_init_data init;
> +       struct sun4i_ddc *ddc;
> +       const char *parent_name;
> +
> +       parent_name = __clk_get_name(parent);
> +       if (!parent_name)
> +               return -ENODEV;
> +
> +       ddc = devm_kzalloc(hdmi->dev, sizeof(*ddc), GFP_KERNEL);
> +       if (!ddc)
> +               return -ENOMEM;
> +
> +       init.name = "hdmi-ddc";
> +       init.ops = &sun4i_ddc_ops;
> +       init.parent_names = &parent_name;
> +       init.num_parents = 1;
> +       init.flags = CLK_SET_RATE_PARENT;

I don't think this is really needed. It probably doesn't hurt though,
since DDC is used when HDMI is not used for displaying, but it might
affect any upstream PLLs, which theoretically may affect other users
of said PLLs. The DDC clock is slow enough that we should be able to
generate a usable clock rate anyway.

> +
> +       ddc->hdmi = hdmi;
> +       ddc->hw.init = &init;
> +
> +       hdmi->ddc_clk = devm_clk_register(hdmi->dev, &ddc->hw);
> +       if (IS_ERR(hdmi->ddc_clk))
> +               return PTR_ERR(hdmi->ddc_clk);
> +
> +       return 0;
> +}
> diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> new file mode 100644
> index 000000000000..33175308c2ed
> --- /dev/null
> +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> @@ -0,0 +1,449 @@
> +/*
> + * Copyright (C) 2016 Maxime Ripard
> + *
> + * Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@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 as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_edid.h>
> +#include <drm/drm_encoder.h>
> +#include <drm/drm_panel.h>
> +
> +#include <linux/clk.h>
> +#include <linux/component.h>
> +#include <linux/iopoll.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +
> +#include "sun4i_backend.h"
> +#include "sun4i_drv.h"
> +#include "sun4i_hdmi.h"
> +#include "sun4i_tcon.h"
> +
> +static inline struct sun4i_hdmi *
> +drm_encoder_to_sun4i_hdmi(struct drm_encoder *encoder)
> +{
> +       return container_of(encoder, struct sun4i_hdmi,
> +                           encoder);
> +}
> +
> +static inline struct sun4i_hdmi *
> +drm_connector_to_sun4i_hdmi(struct drm_connector *connector)
> +{
> +       return container_of(connector, struct sun4i_hdmi,
> +                           connector);
> +}
> +
> +static int sun4i_hdmi_setup_avi_infoframes(struct sun4i_hdmi *hdmi,
> +                                          struct drm_display_mode *mode)
> +{
> +       struct hdmi_avi_infoframe frame;
> +       u8 buffer[17];
> +       int i, ret;
> +
> +       ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> +       if (ret < 0) {
> +               DRM_ERROR("Failed to get infoframes from mode\n");
> +               return ret;
> +       }
> +
> +       ret = hdmi_avi_infoframe_pack(&frame, buffer, sizeof(buffer));
> +       if (ret < 0) {
> +               DRM_ERROR("Failed to pack infoframes\n");
> +               return ret;
> +       }
> +
> +       for (i = 0; i < sizeof(buffer); i++)
> +               writeb(buffer[i], hdmi->base + SUN4I_HDMI_AVI_INFOFRAME_REG(i));
> +
> +       return 0;
> +}
> +
> +static void sun4i_hdmi_disable(struct drm_encoder *encoder)
> +{
> +       struct sun4i_hdmi *hdmi = drm_encoder_to_sun4i_hdmi(encoder);
> +       struct sun4i_drv *drv = hdmi->drv;
> +       struct sun4i_tcon *tcon = drv->tcon;
> +       u32 val;
> +
> +       DRM_DEBUG_DRIVER("Disabling the HDMI Output\n");
> +
> +       val = readl(hdmi->base + SUN4I_HDMI_VID_CTRL_REG);
> +       val &= ~SUN4I_HDMI_VID_CTRL_ENABLE;
> +       writel(val, hdmi->base + SUN4I_HDMI_VID_CTRL_REG);
> +
> +       sun4i_tcon_channel_disable(tcon, 1);
> +}
> +
> +static void sun4i_hdmi_enable(struct drm_encoder *encoder)
> +{
> +       struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
> +       struct sun4i_hdmi *hdmi = drm_encoder_to_sun4i_hdmi(encoder);
> +       struct sun4i_drv *drv = hdmi->drv;
> +       struct sun4i_tcon *tcon = drv->tcon;
> +       u32 val = 0;
> +
> +       DRM_DEBUG_DRIVER("Enabling the HDMI Output\n");
> +
> +       sun4i_tcon_channel_enable(tcon, 1);
> +
> +       sun4i_hdmi_setup_avi_infoframes(hdmi, mode);
> +       val |= SUN4I_HDMI_PKT_CTRL_TYPE(0, SUN4I_HDMI_PKT_AVI);
> +       val |= SUN4I_HDMI_PKT_CTRL_TYPE(1, SUN4I_HDMI_PKT_END);
> +       writel(val, hdmi->base + SUN4I_HDMI_PKT_CTRL_REG(0));
> +
> +       val = SUN4I_HDMI_VID_CTRL_ENABLE;
> +       if (hdmi->hdmi_monitor)
> +               val |= SUN4I_HDMI_VID_CTRL_HDMI_MODE;
> +
> +       writel(val, hdmi->base + SUN4I_HDMI_VID_CTRL_REG);
> +}
> +
> +static void sun4i_hdmi_mode_set(struct drm_encoder *encoder,
> +                               struct drm_display_mode *mode,
> +                               struct drm_display_mode *adjusted_mode)
> +{
> +       struct sun4i_hdmi *hdmi = drm_encoder_to_sun4i_hdmi(encoder);
> +       struct sun4i_drv *drv = hdmi->drv;
> +       struct sun4i_tcon *tcon = drv->tcon;
> +       unsigned int x, y;
> +       u32 val;
> +
> +       sun4i_tcon1_mode_set(tcon, encoder, mode);
> +       clk_set_rate(tcon->sclk1, mode->crtc_clock * 1000);
> +       clk_set_rate(hdmi->tmds_clk, mode->crtc_clock * 1000);
> +
> +       /* Set input sync enable */
> +       writel(SUN4I_HDMI_UNKNOWN_INPUT_SYNC,
> +              hdmi->base + SUN4I_HDMI_UNKNOWN_REG);
> +
> +       /* Setup timing registers */
> +       writel(SUN4I_HDMI_VID_TIMING_X(mode->hdisplay) |
> +              SUN4I_HDMI_VID_TIMING_Y(mode->vdisplay),
> +              hdmi->base + SUN4I_HDMI_VID_TIMING_ACT_REG);
> +
> +       x = mode->htotal - mode->hsync_start;
> +       y = mode->vtotal - mode->vsync_start;

I'm a bit skeptical about this one. All the other parameters are not
inclusive of other, why would this one be different? Shouldn't it
be "Xtotal - Xsync_end" instead?

> +       writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),
> +              hdmi->base + SUN4I_HDMI_VID_TIMING_BP_REG);
> +
> +       x = mode->hsync_start - mode->hdisplay;
> +       y = mode->vsync_start - mode->vdisplay;
> +       writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),
> +              hdmi->base + SUN4I_HDMI_VID_TIMING_FP_REG);
> +
> +       x = mode->hsync_end - mode->hsync_start;
> +       y = mode->vsync_end - mode->vsync_start;
> +       writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),
> +              hdmi->base + SUN4I_HDMI_VID_TIMING_SPW_REG);
> +
> +       val = SUN4I_HDMI_VID_TIMING_POL_TX_CLK;
> +       if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> +               val |= SUN4I_HDMI_VID_TIMING_POL_HSYNC;
> +
> +       if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> +               val |= SUN4I_HDMI_VID_TIMING_POL_VSYNC;
> +
> +       writel(val, hdmi->base + SUN4I_HDMI_VID_TIMING_POL_REG);

You don't handle the interlaced video here, even though you set

    hdmi->connector.interlace_allowed = true

later.

The double clock and double scan flags aren't handled either, though
I don't understand which one is supposed to represent the need for the
HDMI pixel repeater. AFAIK this is required for resolutions with pixel
clocks lower than 25 MHz, the lower limit of HDMI's TMDS link.

> +}
> +
> +static struct drm_encoder_helper_funcs sun4i_hdmi_helper_funcs = {
> +       .disable        = sun4i_hdmi_disable,
> +       .enable         = sun4i_hdmi_enable,
> +       .mode_set       = sun4i_hdmi_mode_set,
> +};
> +
> +static struct drm_encoder_funcs sun4i_hdmi_funcs = {
> +       .destroy        = drm_encoder_cleanup,
> +};
> +
> +static int sun4i_hdmi_read_sub_block(struct sun4i_hdmi *hdmi,
> +                                    unsigned int blk, unsigned int offset,
> +                                    u8 *buf, unsigned int count)
> +{
> +       unsigned long reg;
> +       int i;
> +
> +       reg = readl(hdmi->base + SUN4I_HDMI_DDC_FIFO_CTRL_REG);
> +       writel(reg | SUN4I_HDMI_DDC_FIFO_CTRL_CLEAR,
> +              hdmi->base + SUN4I_HDMI_DDC_FIFO_CTRL_REG);
> +       writel(SUN4I_HDMI_DDC_ADDR_SEGMENT(offset >> 8) |
> +              SUN4I_HDMI_DDC_ADDR_EDDC(0x60) |
> +              SUN4I_HDMI_DDC_ADDR_OFFSET(offset) |
> +              SUN4I_HDMI_DDC_ADDR_SLAVE(0x50),

You can use DDC_ADDR from drm_edid.h.

> +              hdmi->base + SUN4I_HDMI_DDC_ADDR_REG);
> +
> +       writel(count, hdmi->base + SUN4I_HDMI_DDC_BYTE_COUNT_REG);
> +       writel(SUN4I_HDMI_DDC_CMD_EXPLICIT_EDDC_READ,
> +              hdmi->base + SUN4I_HDMI_DDC_CMD_REG);
> +
> +       reg = readl(hdmi->base + SUN4I_HDMI_DDC_CTRL_REG);
> +       writel(reg | SUN4I_HDMI_DDC_CTRL_START_CMD,
> +              hdmi->base + SUN4I_HDMI_DDC_CTRL_REG);
> +
> +       if (readl_poll_timeout(hdmi->base + SUN4I_HDMI_DDC_CTRL_REG, reg,
> +                              !(reg & SUN4I_HDMI_DDC_CTRL_START_CMD),
> +                              100, 2000))
> +               return -EIO;
> +
> +       for (i = 0; i < count; i++)
> +               buf[i] = readb(hdmi->base + SUN4I_HDMI_DDC_FIFO_DATA_REG);
> +
> +       return 0;
> +}
> +
> +static int sun4i_hdmi_read_edid_block(void *data, u8 *buf, unsigned int blk,
> +                                     size_t length)
> +{
> +       struct sun4i_hdmi *hdmi = data;
> +       int retry = 2, i;
> +
> +       do {
> +               for (i = 0; i < length; i += SUN4I_HDMI_DDC_FIFO_SIZE) {
> +                       unsigned char offset = blk * EDID_LENGTH + i;
> +                       unsigned int count = min((unsigned int)SUN4I_HDMI_DDC_FIFO_SIZE,
> +                                                length - i);
> +                       int ret;
> +
> +                       ret = sun4i_hdmi_read_sub_block(hdmi, blk, offset,
> +                                                       buf + i, count);
> +                       if (ret)
> +                               return ret;
> +               }
> +       } while (!drm_edid_block_valid(buf, blk, true, NULL) && (retry--));
> +
> +       return 0;
> +}
> +
> +static int sun4i_hdmi_get_modes(struct drm_connector *connector)
> +{
> +       struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector);
> +       unsigned long reg;
> +       struct edid *edid;
> +       int ret;
> +
> +       /* Reset i2c controller */
> +       writel(SUN4I_HDMI_DDC_CTRL_ENABLE | SUN4I_HDMI_DDC_CTRL_RESET,
> +              hdmi->base + SUN4I_HDMI_DDC_CTRL_REG);
> +       if (readl_poll_timeout(hdmi->base + SUN4I_HDMI_DDC_CTRL_REG, reg,
> +                              !(reg & SUN4I_HDMI_DDC_CTRL_RESET),
> +                              100, 2000))
> +               return -EIO;
> +
> +       writel(SUN4I_HDMI_DDC_LINE_CTRL_SDA_ENABLE |
> +              SUN4I_HDMI_DDC_LINE_CTRL_SCL_ENABLE,
> +              hdmi->base + SUN4I_HDMI_DDC_LINE_CTRL_REG);
> +
> +       clk_set_rate(hdmi->ddc_clk, 100000);
> +
> +       edid = drm_do_get_edid(connector, sun4i_hdmi_read_edid_block, hdmi);
> +       if (!edid)
> +               return 0;
> +
> +       hdmi->hdmi_monitor = drm_detect_hdmi_monitor(edid);
> +       DRM_DEBUG_DRIVER("Monitor is %s monitor\n",
> +                        hdmi->hdmi_monitor ? "an HDMI" : "a DVI");
> +
> +       drm_mode_connector_update_edid_property(connector, edid);
> +       ret = drm_add_edid_modes(connector, edid);
> +       kfree(edid);
> +
> +       return ret;
> +}
> +
> +static struct drm_connector_helper_funcs sun4i_hdmi_connector_helper_funcs = {
> +       .get_modes      = sun4i_hdmi_get_modes,
> +};
> +
> +static enum drm_connector_status
> +sun4i_hdmi_connector_detect(struct drm_connector *connector, bool force)
> +{
> +       struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector);
> +       unsigned long reg;
> +
> +       if (readl_poll_timeout(hdmi->base + SUN4I_HDMI_HPD_REG, reg,
> +                              reg & SUN4I_HDMI_HPD_HIGH,
> +                              0, 500000))

We shouldn't need to do polling here. It should just return the status
at the instance it's called. Instead we should have a worker that does
polling to check if something is plugged or unplugged. I don't see any
interrupt bits for this though. :(

> +               return connector_status_disconnected;
> +
> +       return connector_status_connected;
> +}
> +
> +static struct drm_connector_funcs sun4i_hdmi_connector_funcs = {
> +       .dpms                   = drm_atomic_helper_connector_dpms,
> +       .detect                 = sun4i_hdmi_connector_detect,
> +       .fill_modes             = drm_helper_probe_single_connector_modes,
> +       .destroy                = drm_connector_cleanup,
> +       .reset                  = drm_atomic_helper_connector_reset,
> +       .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +       .atomic_destroy_state   = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static int sun4i_hdmi_bind(struct device *dev, struct device *master,
> +                        void *data)
> +{
> +       struct drm_device *drm = data;
> +       struct sun4i_drv *drv = drm->dev_private;
> +       struct sun4i_hdmi *hdmi = dev_get_drvdata(dev);
> +       int ret;
> +
> +       hdmi->drv = drv;
> +       drm_encoder_helper_add(&hdmi->encoder,
> +                              &sun4i_hdmi_helper_funcs);
> +       ret = drm_encoder_init(drm,
> +                              &hdmi->encoder,
> +                              &sun4i_hdmi_funcs,
> +                              DRM_MODE_ENCODER_TMDS,
> +                              NULL);
> +       if (ret) {
> +               dev_err(dev, "Couldn't initialise the HDMI encoder\n");
> +               return ret;
> +       }
> +
> +       hdmi->encoder.possible_crtcs = BIT(0);

You can use drm_of_find_possible_crtcs() now. See the TV encoder driver.

> +
> +       drm_connector_helper_add(&hdmi->connector,
> +                                &sun4i_hdmi_connector_helper_funcs);
> +       ret = drm_connector_init(drm, &hdmi->connector,
> +                                &sun4i_hdmi_connector_funcs,
> +                                DRM_MODE_CONNECTOR_HDMIA);
> +       if (ret) {
> +               dev_err(dev,
> +                       "Couldn't initialise the Composite connector\n");

Wrong connector.

> +               goto err_cleanup_connector;
> +       }
> +       hdmi->connector.interlace_allowed = true;
> +
> +       drm_mode_connector_attach_encoder(&hdmi->connector, &hdmi->encoder);
> +
> +       return 0;
> +
> +err_cleanup_connector:
> +       drm_encoder_cleanup(&hdmi->encoder);
> +       return ret;
> +}
> +
> +static void sun4i_hdmi_unbind(struct device *dev, struct device *master,
> +                           void *data)
> +{
> +       struct sun4i_hdmi *hdmi = dev_get_drvdata(dev);
> +
> +       drm_connector_cleanup(&hdmi->connector);
> +       drm_encoder_cleanup(&hdmi->encoder);
> +}
> +
> +static struct component_ops sun4i_hdmi_ops = {
> +       .bind   = sun4i_hdmi_bind,
> +       .unbind = sun4i_hdmi_unbind,
> +};
> +
> +static int sun4i_hdmi_probe(struct platform_device *pdev)
> +{
> +       struct sun4i_hdmi *hdmi;
> +       struct resource *res;
> +       int ret;
> +
> +       hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL);
> +       if (!hdmi)
> +               return -ENOMEM;
> +       dev_set_drvdata(&pdev->dev, hdmi);
> +       hdmi->dev = &pdev->dev;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       hdmi->base = devm_ioremap_resource(&pdev->dev, res);
> +       if (IS_ERR(hdmi->base)) {
> +               dev_err(&pdev->dev, "Couldn't map the HDMI encoder registers\n");
> +               return PTR_ERR(hdmi->base);
> +       }
> +
> +       hdmi->bus_clk = devm_clk_get(&pdev->dev, "ahb");
> +       if (IS_ERR(hdmi->bus_clk)) {
> +               dev_err(&pdev->dev, "Couldn't get the HDMI bus clock\n");
> +               return PTR_ERR(hdmi->bus_clk);
> +       }
> +       clk_prepare_enable(hdmi->bus_clk);
> +
> +       hdmi->mod_clk = devm_clk_get(&pdev->dev, "mod");
> +       if (IS_ERR(hdmi->mod_clk)) {
> +               dev_err(&pdev->dev, "Couldn't get the HDMI mod clock\n");
> +               return PTR_ERR(hdmi->mod_clk);
> +       }
> +       clk_prepare_enable(hdmi->mod_clk);
> +
> +       hdmi->pll0_clk = devm_clk_get(&pdev->dev, "pll-0");
> +       if (IS_ERR(hdmi->pll0_clk)) {
> +               dev_err(&pdev->dev, "Couldn't get the HDMI PLL 0 clock\n");
> +               return PTR_ERR(hdmi->pll0_clk);
> +       }
> +
> +       hdmi->pll1_clk = devm_clk_get(&pdev->dev, "pll-1");
> +       if (IS_ERR(hdmi->pll1_clk)) {
> +               dev_err(&pdev->dev, "Couldn't get the HDMI PLL 1 clock\n");
> +               return PTR_ERR(hdmi->pll1_clk);
> +       }
> +
> +       ret = sun4i_tmds_create(hdmi);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Couldn't create the TMDS clock\n");
> +               return ret;
> +       }
> +
> +       writel(SUN4I_HDMI_CTRL_ENABLE, hdmi->base + SUN4I_HDMI_CTRL_REG);
> +
> +#define SUN4I_HDMI_PAD_CTRL0 0xfe800000
> +
> +       writel(SUN4I_HDMI_PAD_CTRL0, hdmi->base + SUN4I_HDMI_PAD_CTRL0_REG);
> +
> +       /* TODO: defines */
> +       writel((6 << 3) | (2 << 10) | BIT(14) | BIT(15) |
> +              BIT(19) | BIT(20) | BIT(22) | BIT(23),
> +              hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);
> +
> +       /* TODO: defines */
> +       writel((8 << 0) | (7 << 8) | (239 << 12) | (7 << 17) | (4 << 20) |
> +              BIT(25) | BIT(27) | BIT(28) | BIT(29) | BIT(30) | BIT(31),
> +              hdmi->base + SUN4I_HDMI_PLL_CTRL_REG);

FYI some bits in this register look a lot like the MIPI PLL on the A33.
Bit 31 looks like the enable bit.

> +
> +       ret = sun4i_ddc_create(hdmi, hdmi->tmds_clk);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Couldn't create the DDC clock\n");
> +               return ret;
> +       }

We do all this in the bind function for all the other components.
Any particular reason to do it differently here?

> +
> +       return component_add(&pdev->dev, &sun4i_hdmi_ops);
> +}
> +
> +static int sun4i_hdmi_remove(struct platform_device *pdev)
> +{
> +       component_del(&pdev->dev, &sun4i_hdmi_ops);
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id sun4i_hdmi_of_table[] = {
> +       { .compatible = "allwinner,sun5i-a10s-hdmi" },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, sun4i_hdmi_of_table);
> +
> +static struct platform_driver sun4i_hdmi_driver = {
> +       .probe          = sun4i_hdmi_probe,
> +       .remove         = sun4i_hdmi_remove,
> +       .driver         = {
> +               .name           = "sun4i-hdmi",
> +               .of_match_table = sun4i_hdmi_of_table,
> +       },
> +};
> +module_platform_driver(sun4i_hdmi_driver);
> +
> +MODULE_AUTHOR("Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>");
> +MODULE_DESCRIPTION("Allwinner A10 HDMI Driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c
> new file mode 100644
> index 000000000000..40f48f1d4685
> --- /dev/null
> +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c
> @@ -0,0 +1,236 @@
> +/*
> + * Copyright (C) 2016 Free Electrons
> + * Copyright (C) 2016 NextThing Co
> + *
> + * Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@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 as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +
> +#include <linux/clk-provider.h>
> +
> +#include "sun4i_tcon.h"
> +#include "sun4i_hdmi.h"
> +
> +struct sun4i_tmds {
> +       struct clk_hw           hw;
> +       struct sun4i_hdmi       *hdmi;
> +};
> +
> +static inline struct sun4i_tmds *hw_to_tmds(struct clk_hw *hw)
> +{
> +       return container_of(hw, struct sun4i_tmds, hw);
> +}
> +
> +
> +static unsigned long sun4i_tmds_calc_divider(unsigned long rate,
> +                                            unsigned long parent_rate,
> +                                            u8 *div,
> +                                            bool *half)
> +{
> +       unsigned long best_rate = 0;
> +       u8 best_m = 0, m;
> +       bool is_double;
> +
> +       for (m = 1; m < 16; m++) {
> +               u8 d;
> +
> +               for (d = 1; d < 3; d++) {
> +                       unsigned long tmp_rate;
> +
> +                       tmp_rate = parent_rate / m / d;
> +
> +                       if (tmp_rate > rate)
> +                               continue;
> +
> +                       if (!best_rate ||
> +                           (rate - tmp_rate) < (rate - best_rate)) {
> +                               best_rate = tmp_rate;
> +                               best_m = m;
> +                               is_double = d;
> +                       }
> +               }
> +       }
> +
> +       if (div && half) {
> +               *div = best_m;
> +               *half = is_double;
> +       }
> +
> +       return best_rate;
> +}
> +
> +
> +static int sun4i_tmds_determine_rate(struct clk_hw *hw,
> +                                    struct clk_rate_request *req)
> +{
> +       struct clk_hw *parent;
> +       unsigned long best_parent = 0;
> +       unsigned long rate = req->rate;
> +       int best_div = 1, best_half = 1;
> +       int i, j;
> +
> +       printk("%s %d rate %lu\n", __func__, __LINE__, rate);

Stray printk?

> +
> +       /*
> +        * We only consider PLL3, since the TCON is very likely to be
> +        * clocked from it, and to have the same rate than our HDMI
> +        * clock, so we should not need to do anything.
> +        */
> +
> +       parent = clk_hw_get_parent_by_index(hw, 0);
> +       if (!parent)
> +               return -EINVAL;
> +
> +       for (i = 1; i < 3; i++) {
> +               for (j = 1; j < 16; j++) {
> +                       unsigned long ideal = rate * i * j;
> +                       unsigned long rounded;
> +
> +                       rounded = clk_hw_round_rate(parent, ideal);
> +
> +                       if (rounded == ideal) {
> +                               best_parent = rounded;
> +                               best_half = i;
> +                               best_div = j;
> +                               goto out;
> +                       }
> +
> +                       if (abs(rate - rounded / i) <
> +                           abs(rate - best_parent / best_div)) {
> +                               best_parent = rounded;
> +                               best_div = i;
> +                       }
> +               }
> +       }
> +
> +out:
> +       req->rate = best_parent / best_half / best_div;
> +       req->best_parent_rate = best_parent;
> +       req->best_parent_hw = parent;
> +
> +       printk("%s %d rate %lu parent rate %lu (%s) div %d half %d\n",
> +              __func__, __LINE__, req->rate, req->best_parent_rate,
> +              clk_hw_get_name(req->best_parent_hw),
> +              best_div, best_half);

Stray printk?

> +
> +       return 0;
> +}
> +
> +static unsigned long sun4i_tmds_recalc_rate(struct clk_hw *hw,
> +                                           unsigned long parent_rate)
> +{
> +       struct sun4i_tmds *tmds = hw_to_tmds(hw);
> +       u32 reg;
> +
> +       reg = readl(tmds->hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);
> +       if (reg & SUN4I_HDMI_PAD_CTRL1_HALVE_CLK)
> +               parent_rate /= 2;
> +
> +       reg = readl(tmds->hdmi->base + SUN4I_HDMI_PLL_CTRL_REG);
> +       reg = (reg >> 4) & 0xf;
> +       if (!reg)
> +               reg = 1;
> +
> +       return parent_rate / reg;
> +}
> +
> +static int sun4i_tmds_set_rate(struct clk_hw *hw, unsigned long rate,
> +                              unsigned long parent_rate)
> +{
> +       struct sun4i_tmds *tmds = hw_to_tmds(hw);
> +       bool half;
> +       u32 reg;
> +       u8 div;
> +
> +       sun4i_tmds_calc_divider(rate, parent_rate, &div, &half);
> +
> +       printk("%s %d rate %lu parent rate %lu div %d half %s\n",
> +              __func__, __LINE__, rate, parent_rate, div,
> +              half ? "yes" : "no");

Stray printk?

> +
> +       reg = readl(tmds->hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);
> +       reg &= ~SUN4I_HDMI_PAD_CTRL1_HALVE_CLK;
> +       if (half)
> +               reg |= SUN4I_HDMI_PAD_CTRL1_HALVE_CLK;
> +       writel(reg, tmds->hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);
> +
> +       reg = readl(tmds->hdmi->base + SUN4I_HDMI_PLL_CTRL_REG);
> +       reg &= ~SUN4I_HDMI_PLL_CTRL_DIV_MASK;
> +       writel(reg | SUN4I_HDMI_PLL_CTRL_DIV(div),
> +              tmds->hdmi->base + SUN4I_HDMI_PLL_CTRL_REG);
> +
> +       return 0;
> +}
> +
> +static u8 sun4i_tmds_get_parent(struct clk_hw *hw)
> +{
> +       struct sun4i_tmds *tmds = hw_to_tmds(hw);
> +       u32 reg;
> +
> +       reg = readl(tmds->hdmi->base + SUN4I_HDMI_PLL_DBG0_REG);
> +       return ((reg & SUN4I_HDMI_PLL_DBG0_TMDS_PARENT_MASK) >>
> +               SUN4I_HDMI_PLL_DBG0_TMDS_PARENT_SHIFT);
> +}
> +
> +static int sun4i_tmds_set_parent(struct clk_hw *hw, u8 index)
> +{
> +       struct sun4i_tmds *tmds = hw_to_tmds(hw);
> +       u32 reg;
> +
> +       if (index > 1)
> +               return -EINVAL;
> +
> +       reg = readl(tmds->hdmi->base + SUN4I_HDMI_PLL_DBG0_REG);
> +       reg &= ~SUN4I_HDMI_PLL_DBG0_TMDS_PARENT_MASK;
> +       writel(reg | SUN4I_HDMI_PLL_DBG0_TMDS_PARENT(index),
> +              tmds->hdmi->base + SUN4I_HDMI_PLL_DBG0_REG);
> +
> +       return 0;
> +}
> +
> +static const struct clk_ops sun4i_tmds_ops = {
> +       .determine_rate = sun4i_tmds_determine_rate,
> +       .recalc_rate    = sun4i_tmds_recalc_rate,
> +       .set_rate       = sun4i_tmds_set_rate,
> +
> +       .get_parent     = sun4i_tmds_get_parent,
> +       .set_parent     = sun4i_tmds_set_parent,
> +};
> +
> +int sun4i_tmds_create(struct sun4i_hdmi *hdmi)
> +{
> +       struct clk_init_data init;
> +       struct sun4i_tmds *tmds;
> +       const char *parents[2];
> +
> +       parents[0] = __clk_get_name(hdmi->pll0_clk);
> +       if (!parents[0])
> +               return -ENODEV;
> +
> +       parents[1] = __clk_get_name(hdmi->pll1_clk);
> +       if (!parents[1])
> +               return -ENODEV;
> +
> +       tmds = devm_kzalloc(hdmi->dev, sizeof(*tmds), GFP_KERNEL);
> +       if (!tmds)
> +               return -ENOMEM;
> +
> +       init.name = "hdmi-tmds";
> +       init.ops = &sun4i_tmds_ops;
> +       init.parent_names = parents;
> +       init.num_parents = 2;
> +       init.flags = CLK_SET_RATE_PARENT;
> +
> +       tmds->hdmi = hdmi;
> +       tmds->hw.init = &init;
> +
> +       hdmi->tmds_clk = devm_clk_register(hdmi->dev, &tmds->hw);
> +       if (IS_ERR(hdmi->tmds_clk))
> +               return PTR_ERR(hdmi->tmds_clk);
> +
> +       return 0;
> +}
> --

I also compared the manuals of A20 and A31, and the existing U-boot driver.

So far it looks like the DDC bits are quite different. We could probably
use regfields to work around it, but the DDC clock formula is completely
different. The TMDS clock pre-divider is also different, your usual sun4i
vs sun6i factor offset. Last, the initial values for the 3 PLL related
registers are different.

I'm currently working on an A31 variant for this, basically just copying
the DDC and TMDS bits.

Regards
ChenYu

> git-series 0.8.11
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
> For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH v13 03/10] mux: minimal mux subsystem and gpio-based mux controller
From: Philipp Zabel @ 2017-04-21 15:19 UTC (permalink / raw)
  To: Peter Rosin
  Cc: Mark Rutland, devicetree, Lars-Peter Clausen, kernel,
	Wolfram Sang, linux-iio, Greg Kroah-Hartman, Jonathan Corbet,
	linux-doc, linux-kernel, Paul Gortmaker, Rob Herring, linux-i2c,
	Peter Meerwald-Stadler, Hartmut Knaack, Colin Ian King,
	Andrew Morton, Jonathan Cameron
In-Reply-To: <6bc3120a-81dd-3b6c-d246-559a3c072969@axentia.se>

On Fri, 2017-04-21 at 16:55 +0200, Peter Rosin wrote:
> On 2017-04-21 16:41, Philipp Zabel wrote:
> > On Fri, 2017-04-21 at 16:32 +0200, Peter Rosin wrote:
> >> On 2017-04-21 16:23, Philipp Zabel wrote:
> >>> On Thu, 2017-04-13 at 18:43 +0200, Peter Rosin wrote:
> >>> [...]
> >>>> +int mux_chip_register(struct mux_chip *mux_chip)
> >>>> +{
> >>>> +	int i;
> >>>> +	int ret;
> >>>> +
> >>>> +	for (i = 0; i < mux_chip->controllers; ++i) {
> >>>> +		struct mux_control *mux = &mux_chip->mux[i];
> >>>> +
> >>>> +		if (mux->idle_state == mux->cached_state)
> >>>> +			continue;
> >>>
> >>> I think this should be changed to
> >>>  
> >>> -               if (mux->idle_state == mux->cached_state)
> >>> +               if (mux->idle_state == mux->cached_state ||
> >>> +                   mux->idle_state == MUX_IDLE_AS_IS)
> >>>                         continue;
> >>>
> >>> or the following mux_control_set will be called with state ==
> >>> MUX_IDLE_AS_IS. Alternatively, mux_control_set should return when passed
> >>> this value.
> >>
> >> That cannot happen because ->cached_state is initialized to -1
> >> in mux_chip_alloc, so should always be == MUX_IDLE_AS_IS when
> >> registering. And drivers are not supposed to touch ->cached_state.
> >> I.e., ->cached_state is "owned" by the core.
> > 
> > So this was caused by me filling cached_state from register reads in the
> > mmio driver. Makes me wonder why I am not allowed to do this, though, if
> > I am able to read back the initial state?
> 
> You gain fairly little by reading back the original state. If the mux
> should idle-as-is, you can avoid a maximum of one mux update if the first
> consumer happens to starts by requesting the previously active state.
> Similarly, if the mux should idle in a specific state, you can avoid a
> maximum of one mux update.
> 
> In both cases it costs one unconditional read of the mux state.
> 
> Sure, in some cases reads are cheaper than writes, but I didn't think
> support for seeding the cache was worth it. Is it worth it?

Probably not, I'll just drop the cached_state initialization. It should
be documented in the mux.h that this field is framework internal and not
to be touched by the drivers. At least I was surprised.

regards
Philipp

^ permalink raw reply

* [PATCH v2] power: tps65217_charger: Add properties like voltage and current charge
From: Enric Balletbo i Serra @ 2017-04-21 15:50 UTC (permalink / raw)
  To: Sebastian Reichel, Rob Herring, Mark Rutland
  Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Allow the possibility to configure the charge and the current voltage of
the charger and also the NTC type for battery temperature measurement.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
Changes since v1:
 - Requested by Rob Herring
   - Rename ti,charge-* to charge-* to be standard properties.
   - Use unit suffixes as per bindings/property-units.txt
---
 .../bindings/power/supply/tps65217_charger.txt     |  15 ++
 drivers/power/supply/tps65217_charger.c            | 187 +++++++++++++++++++--
 include/linux/mfd/tps65217.h                       |   2 +
 3 files changed, 192 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/supply/tps65217_charger.txt b/Documentation/devicetree/bindings/power/supply/tps65217_charger.txt
index a11072c..4415618 100644
--- a/Documentation/devicetree/bindings/power/supply/tps65217_charger.txt
+++ b/Documentation/devicetree/bindings/power/supply/tps65217_charger.txt
@@ -6,6 +6,18 @@ Required Properties:
              Should be <0> for the USB charger and <1> for the AC adapter.
 -interrupt-names: Should be "USB" and "AC"
 
+Optional properties:
+-charge-voltage-microvolt: set the charge voltage. The value can be: 4100000,
+	4150000, 4200000, 4250000; default: 4100000
+
+-charge-current-microamp: set the charging current. The value can be: 300000,
+	400000, 500000, 700000; default: 500000
+
+-ti,ntc-type: set the NTC type for battery temperature measurement. The value
+	must be 0 or 1, where:
+	  0 – 100k (curve 1, B = 3960)
+	  1 – 10k  (curve 2, B = 3480) (default)
+
 This node is a subnode of the tps65217 PMIC.
 
 Example:
@@ -14,4 +26,7 @@ Example:
 		compatible = "ti,tps65217-charger";
 		interrupts = <0>, <1>;
 		interrupt-names = "USB", "AC";
+		charge-voltage-microvolt = <4100000>;
+		charge-current-microamp = <500000>;
+		ti,ntc-type = <1>;
 	};
diff --git a/drivers/power/supply/tps65217_charger.c b/drivers/power/supply/tps65217_charger.c
index 1f52340..087f29c 100644
--- a/drivers/power/supply/tps65217_charger.c
+++ b/drivers/power/supply/tps65217_charger.c
@@ -39,6 +39,12 @@
 #define NUM_CHARGER_IRQS	2
 #define POLL_INTERVAL		(HZ * 2)
 
+struct tps65217_charger_platform_data {
+	u32	charge_current_uamp;
+	u32	charge_voltage_uvolt;
+	int	ntc_type;
+};
+
 struct tps65217_charger {
 	struct tps65217 *tps;
 	struct device *dev;
@@ -48,16 +54,82 @@ struct tps65217_charger {
 	int	prev_online;
 
 	struct task_struct	*poll_task;
+	struct tps65217_charger_platform_data *pdata;
 };
 
 static enum power_supply_property tps65217_charger_props[] = {
 	POWER_SUPPLY_PROP_ONLINE,
 };
 
-static int tps65217_config_charger(struct tps65217_charger *charger)
+static int tps65217_set_charge_current(struct tps65217_charger *charger,
+				       unsigned int uamp)
+{
+	int ret, val;
+
+	dev_dbg(charger->dev, "setting charge current to %d uA\n", uamp);
+
+	if (uamp == 300000)
+		val = 0x00;
+	else if (uamp == 400000)
+		val = 0x01;
+	else if (uamp == 500000)
+		val = 0x02;
+	else if (uamp == 700000)
+		val = 0x03;
+	else
+		return -EINVAL;
+
+	ret = tps65217_set_bits(charger->tps, TPS65217_REG_CHGCONFIG3,
+				TPS65217_CHGCONFIG3_ICHRG_MASK,
+				val << TPS65217_CHGCONFIG3_ICHRG_SHIFT,
+				TPS65217_PROTECT_NONE);
+	if (ret) {
+		dev_err(charger->dev,
+			"failed to set ICHRG setting to 0x%02x (err: %d)\n",
+			val, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int tps65217_set_charge_voltage(struct tps65217_charger *charger,
+				       unsigned int uvolt)
+{
+	int ret, val;
+
+	dev_dbg(charger->dev, "setting charge voltage to %d uV\n", uvolt);
+
+	if (uvolt != 4100000 && uvolt != 4150000 &&
+	    uvolt != 4200000 && uvolt != 4250000)
+		return -EINVAL;
+
+	val = (uvolt - 4100000) / 50000;
+
+	ret = tps65217_set_bits(charger->tps, TPS65217_REG_CHGCONFIG2,
+				TPS65217_CHGCONFIG2_VOREG_MASK,
+				val << TPS65217_CHGCONFIG2_VOREG_SHIFT,
+				TPS65217_PROTECT_NONE);
+	if (ret) {
+		dev_err(charger->dev,
+			"failed to set VOCHG setting to 0x%02x (err: %d)\n",
+			val, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int tps65217_set_ntc_type(struct tps65217_charger *charger,
+				 unsigned int ntc)
 {
 	int ret;
 
+	dev_dbg(charger->dev, "setting NTC type to %d\n", ntc);
+
+	if (ntc != 0 && ntc != 1)
+		return -EINVAL;
+
 	/*
 	 * tps65217 rev. G, p. 31 (see p. 32 for NTC schematic)
 	 *
@@ -74,14 +146,57 @@ static int tps65217_config_charger(struct tps65217_charger *charger)
 	 * NTC TYPE (for battery temperature measurement)
 	 *   0 – 100k (curve 1, B = 3960)
 	 *   1 – 10k  (curve 2, B = 3480) (default on reset)
-	 *
 	 */
-	ret = tps65217_clear_bits(charger->tps, TPS65217_REG_CHGCONFIG1,
-				  TPS65217_CHGCONFIG1_NTC_TYPE,
-				  TPS65217_PROTECT_NONE);
+	if (ntc) {
+		ret = tps65217_set_bits(charger->tps, TPS65217_REG_CHGCONFIG1,
+					TPS65217_CHGCONFIG1_NTC_TYPE,
+					TPS65217_CHGCONFIG1_NTC_TYPE,
+					TPS65217_PROTECT_NONE);
+		if (ret) {
+			dev_err(charger->dev,
+				"failed to set NTC type to 10K: %d\n", ret);
+			return ret;
+		}
+	} else {
+		ret = tps65217_clear_bits(charger->tps, TPS65217_REG_CHGCONFIG1,
+					  TPS65217_CHGCONFIG1_NTC_TYPE,
+					  TPS65217_PROTECT_NONE);
+		if (ret) {
+			dev_err(charger->dev,
+				"failed to set NTC type to 100K: %d\n", ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int tps65217_config_charger(struct tps65217_charger *charger)
+{
+	int ret;
+	struct tps65217_charger_platform_data *pdata = charger->pdata;
+
+	if (!charger->pdata)
+		return -EINVAL;
+
+	ret = tps65217_set_charge_voltage(charger, pdata->charge_voltage_uvolt);
 	if (ret) {
 		dev_err(charger->dev,
-			"failed to set 100k NTC setting: %d\n", ret);
+			"failed to set charge voltage setting: %d\n", ret);
+		return ret;
+	}
+
+	ret = tps65217_set_charge_current(charger, pdata->charge_current_uamp);
+	if (ret) {
+		dev_err(charger->dev,
+			"failed to set charge current setting: %d\n", ret);
+		return ret;
+	}
+
+	ret = tps65217_set_ntc_type(charger, pdata->ntc_type);
+	if (ret) {
+		dev_err(charger->dev,
+			"failed to set NTC type setting: %d\n", ret);
 		return ret;
 	}
 
@@ -185,6 +300,48 @@ static int tps65217_charger_poll_task(void *data)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static struct tps65217_charger_platform_data *tps65217_charger_pdata_init(
+		struct platform_device *pdev)
+{
+	struct tps65217_charger_platform_data *pdata;
+	struct device_node *np = pdev->dev.of_node;
+	int ret;
+
+	if (!np) {
+		dev_err(&pdev->dev, "No charger OF node\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	ret = of_property_read_u32(np, "charge-voltage-microvolt",
+				   &pdata->charge_voltage_uvolt);
+	if (ret)
+		pdata->charge_voltage_uvolt = 4100000;
+
+	ret = of_property_read_u32(np, "charge-current-microamp",
+				   &pdata->charge_current_uamp);
+	if (ret)
+		pdata->charge_current_uamp = 500000;
+
+	ret = of_property_read_u32(np, "ti,ntc-type",
+				   &pdata->ntc_type);
+	if (ret)
+		pdata->ntc_type = 1;	/* 10k  (curve 2, B = 3480) */
+
+	return pdata;
+}
+#else /* CONFIG_OF */
+static struct tps65217_charger_platform_data *tps65217_charger_pdata_init(
+		struct platform_device *pdev)
+{
+	return NULL;
+}
+#endif /* CONFIG_OF */
+
 static const struct power_supply_desc tps65217_charger_desc = {
 	.name			= "tps65217-charger",
 	.type			= POWER_SUPPLY_TYPE_MAINS,
@@ -214,6 +371,18 @@ static int tps65217_charger_probe(struct platform_device *pdev)
 	cfg.of_node = pdev->dev.of_node;
 	cfg.drv_data = charger;
 
+	charger->pdata = tps65217_charger_pdata_init(pdev);
+	if (IS_ERR(charger->pdata)) {
+		dev_err(charger->dev, "failed: getting platform data\n");
+		return PTR_ERR(charger->pdata);
+	}
+
+	ret = tps65217_config_charger(charger);
+	if (ret < 0) {
+		dev_err(charger->dev, "charger config failed, err %d\n", ret);
+		return ret;
+	}
+
 	charger->psy = devm_power_supply_register(&pdev->dev,
 						  &tps65217_charger_desc,
 						  &cfg);
@@ -225,12 +394,6 @@ static int tps65217_charger_probe(struct platform_device *pdev)
 	irq[0] = platform_get_irq_byname(pdev, "USB");
 	irq[1] = platform_get_irq_byname(pdev, "AC");
 
-	ret = tps65217_config_charger(charger);
-	if (ret < 0) {
-		dev_err(charger->dev, "charger config failed, err %d\n", ret);
-		return ret;
-	}
-
 	/* Create a polling thread if an interrupt is invalid */
 	if (irq[0] < 0 || irq[1] < 0) {
 		poll_task = kthread_run(tps65217_charger_poll_task,
diff --git a/include/linux/mfd/tps65217.h b/include/linux/mfd/tps65217.h
index eac2857..d040062 100644
--- a/include/linux/mfd/tps65217.h
+++ b/include/linux/mfd/tps65217.h
@@ -103,8 +103,10 @@
 #define TPS65217_CHGCONFIG2_DYNTMR	BIT(7)
 #define TPS65217_CHGCONFIG2_VPREGHG	BIT(6)
 #define TPS65217_CHGCONFIG2_VOREG_MASK	0x30
+#define TPS65217_CHGCONFIG2_VOREG_SHIFT	4
 
 #define TPS65217_CHGCONFIG3_ICHRG_MASK	0xC0
+#define TPS65217_CHGCONFIG3_ICHRG_SHIFT	6
 #define TPS65217_CHGCONFIG3_DPPMTH_MASK	0x30
 #define TPS65217_CHGCONFIG2_PCHRGT	BIT(3)
 #define TPS65217_CHGCONFIG2_TERMIF	0x06
-- 
2.9.3

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


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