[PATCH 0/4] Marvell Dove DRM for DT

[PATCH 0/4] Marvell Dove DRM for DT

[Sebastian Hesselbarth]

Thanks to Russell we have Armada DRM driver but currently DT support
for it is stalled in various discussions/improvements. We know there
are issues with the whole DRM chain (si5351,lcd,tda998x) that are not
easy to track down. To get at least some more code testing coverage,
this patch set picks up some patches from Russell slightly reworked
to allow Dove DT to register and use Armada DRM.

The approach taken is based on what we did in the past for mach-dove
to mach-mvebu/dove conversion: platform_devices are registered like
it was before DT and quirks are added to allow to register the
corresponsing device properly. Then we start removing each of the
quirks incrementally.

Russell currently has a quite huge gap [1,2,3] between his branch and
mainline Linux, so this is a tiny step forward to closen the gap and
free some of his time for more controversal discussion. Also, it will
allow us to participate in those discussions as we'll be able to test
them more easily.

Patches 1-3 add DT binding docs and corresponding nodes for the
LCD controllers found on Marvell Dove SoCs. The binding itself is
considered stable and not likely to change in the future.

Patch 4 then adds above platform_device registration for DRM driver
to Dove DT based setup. Currently, we require to steal some CMA mem
for the framebuffers, collect lcd resources, and clocks from
different nodes. As soon as DT for DRM driver and component framework
have settled, we can remove the quirks again.

Patches are based on v3.16-rc1 and have been tested on DT SolidRun
CuBox.

Sebastian

[1] http://lkml.kernel.org/r/20140630142516.GA32514@n2100.arm.linux.org.uk
[2] http://lkml.kernel.org/r/20140630165615.GE32514@n2100.arm.linux.org.uk
[3] http://lkml.kernel.org/r/20140630193540.GG32514@n2100.arm.linux.org.uk

Russell King (4):
  dt-bindings: add Marvell Dove LCD controller documentation
  ARM: dts: dove: add DT LCD controllers
  ARM: dts: dove: enable lcd0 on SolidRun CuBox
  ARM: mvebu: add armada drm init to Dove board setup

 .../bindings/drm/armada/marvell,dove-lcd.txt       |   14 ++++
 arch/arm/boot/dts/dove-cubox.dts                   |    6 ++
 arch/arm/boot/dts/dove.dtsi                        |   14 ++++
 arch/arm/mach-mvebu/dove.c                         |   70 +++++++++++++++++++-
 4 files changed, 103 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/drm/armada/marvell,dove-lcd.txt

---
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: devicetree@vger.kernel.org
Cc: linux-doc@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
-- 
1.7.2.5

[PATCH 0/4] Marvell Dove DRM for DT

[Sebastian Hesselbarth]

Thanks to Russell we have Armada DRM driver but currently DT support
for it is stalled in various discussions/improvements. We know there
are issues with the whole DRM chain (si5351,lcd,tda998x) that are not
easy to track down. To get at least some more code testing coverage,
this patch set picks up some patches from Russell slightly reworked
to allow Dove DT to register and use Armada DRM.

The approach taken is based on what we did in the past for mach-dove
to mach-mvebu/dove conversion: platform_devices are registered like
it was before DT and quirks are added to allow to register the
corresponsing device properly. Then we start removing each of the
quirks incrementally.

Russell currently has a quite huge gap [1,2,3] between his branch and
mainline Linux, so this is a tiny step forward to closen the gap and
free some of his time for more controversal discussion. Also, it will
allow us to participate in those discussions as we'll be able to test
them more easily.

Patches 1-3 add DT binding docs and corresponding nodes for the
LCD controllers found on Marvell Dove SoCs. The binding itself is
considered stable and not likely to change in the future.

Patch 4 then adds above platform_device registration for DRM driver
to Dove DT based setup. Currently, we require to steal some CMA mem
for the framebuffers, collect lcd resources, and clocks from
different nodes. As soon as DT for DRM driver and component framework
have settled, we can remove the quirks again.

Patches are based on v3.16-rc1 and have been tested on DT SolidRun
CuBox.

Sebastian

[1] http://lkml.kernel.org/r/20140630142516.GA32514 at n2100.arm.linux.org.uk
[2] http://lkml.kernel.org/r/20140630165615.GE32514 at n2100.arm.linux.org.uk
[3] http://lkml.kernel.org/r/20140630193540.GG32514 at n2100.arm.linux.org.uk

Russell King (4):
  dt-bindings: add Marvell Dove LCD controller documentation
  ARM: dts: dove: add DT LCD controllers
  ARM: dts: dove: enable lcd0 on SolidRun CuBox
  ARM: mvebu: add armada drm init to Dove board setup

 .../bindings/drm/armada/marvell,dove-lcd.txt       |   14 ++++
 arch/arm/boot/dts/dove-cubox.dts                   |    6 ++
 arch/arm/boot/dts/dove.dtsi                        |   14 ++++
 arch/arm/mach-mvebu/dove.c                         |   70 +++++++++++++++++++-
 4 files changed, 103 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/drm/armada/marvell,dove-lcd.txt

---
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: devicetree at vger.kernel.org
Cc: linux-doc at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-kernel at vger.kernel.org
-- 
1.7.2.5

[PATCH 1/4] dt-bindings: add Marvell Dove LCD controller documentation

[Sebastian Hesselbarth]

From: Russell King <rmk+kernel@arm.linux.org.uk>

Add the Marvell Dove LCD controller DT binding documentation.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: devicetree@vger.kernel.org
Cc: linux-doc@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 .../bindings/drm/armada/marvell,dove-lcd.txt       |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/drm/armada/marvell,dove-lcd.txt

diff --git a/Documentation/devicetree/bindings/drm/armada/marvell,dove-lcd.txt b/Documentation/devicetree/bindings/drm/armada/marvell,dove-lcd.txt
new file mode 100644
index 0000000..97c600d
--- /dev/null
+++ b/Documentation/devicetree/bindings/drm/armada/marvell,dove-lcd.txt
@@ -0,0 +1,14 @@
+Device Tree bindings for Armada DRM CRTC driver
+
+Required properties:
+ - compatible: value should be "marvell,dove-lcd".
+ - reg: base address and size of the LCD controller
+ - interrupts: single interrupt number for the LCD controller
+
+Example:
+
+	lcd0: lcd-controller@820000 {
+		compatible = "marvell,dove-lcd";
+		reg = <0x820000 0x1000>;
+		interrupts = <47>;
+	};
-- 
1.7.2.5

[PATCH 1/4] dt-bindings: add Marvell Dove LCD controller documentation

[Sebastian Hesselbarth]

From: Russell King <rmk+kernel@arm.linux.org.uk>

Add the Marvell Dove LCD controller DT binding documentation.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: devicetree at vger.kernel.org
Cc: linux-doc at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-kernel at vger.kernel.org
---
 .../bindings/drm/armada/marvell,dove-lcd.txt       |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/drm/armada/marvell,dove-lcd.txt

diff --git a/Documentation/devicetree/bindings/drm/armada/marvell,dove-lcd.txt b/Documentation/devicetree/bindings/drm/armada/marvell,dove-lcd.txt
new file mode 100644
index 0000000..97c600d
--- /dev/null
+++ b/Documentation/devicetree/bindings/drm/armada/marvell,dove-lcd.txt
@@ -0,0 +1,14 @@
+Device Tree bindings for Armada DRM CRTC driver
+
+Required properties:
+ - compatible: value should be "marvell,dove-lcd".
+ - reg: base address and size of the LCD controller
+ - interrupts: single interrupt number for the LCD controller
+
+Example:
+
+	lcd0: lcd-controller at 820000 {
+		compatible = "marvell,dove-lcd";
+		reg = <0x820000 0x1000>;
+		interrupts = <47>;
+	};
-- 
1.7.2.5

[PATCH 2/4] ARM: dts: dove: add DT LCD controllers

[Sebastian Hesselbarth]

From: Russell King <rmk+kernel@arm.linux.org.uk>

Add the DT fragment for the Marvell Dove LCD controllers.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/boot/dts/dove.dtsi |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
index 3b891dd..a5441d5 100644
--- a/arch/arm/boot/dts/dove.dtsi
+++ b/arch/arm/boot/dts/dove.dtsi
@@ -630,6 +630,20 @@
 				reg = <0xe8400 0x0c>;
 				ngpios = <8>;
 			};
+
+			lcd1: lcd-controller@810000 {
+				compatible = "marvell,dove-lcd";
+				reg = <0x810000 0x1000>;
+				interrupts = <46>;
+				status = "disabled";
+			};
+
+			lcd0: lcd-controller@820000 {
+				compatible = "marvell,dove-lcd";
+				reg = <0x820000 0x1000>;
+				interrupts = <47>;
+				status = "disabled";
+			};
 		};
 	};
 };
-- 
1.7.2.5

[PATCH 2/4] ARM: dts: dove: add DT LCD controllers

[Sebastian Hesselbarth]

From: Russell King <rmk+kernel@arm.linux.org.uk>

Add the DT fragment for the Marvell Dove LCD controllers.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/boot/dts/dove.dtsi |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
index 3b891dd..a5441d5 100644
--- a/arch/arm/boot/dts/dove.dtsi
+++ b/arch/arm/boot/dts/dove.dtsi
@@ -630,6 +630,20 @@
 				reg = <0xe8400 0x0c>;
 				ngpios = <8>;
 			};
+
+			lcd1: lcd-controller@810000 {
+				compatible = "marvell,dove-lcd";
+				reg = <0x810000 0x1000>;
+				interrupts = <46>;
+				status = "disabled";
+			};
+
+			lcd0: lcd-controller@820000 {
+				compatible = "marvell,dove-lcd";
+				reg = <0x820000 0x1000>;
+				interrupts = <47>;
+				status = "disabled";
+			};
 		};
 	};
 };
-- 
1.7.2.5

[PATCH 2/4] ARM: dts: dove: add DT LCD controllers

[Sebastian Hesselbarth]

From: Russell King <rmk+kernel@arm.linux.org.uk>

Add the DT fragment for the Marvell Dove LCD controllers.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: devicetree at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-kernel at vger.kernel.org
---
 arch/arm/boot/dts/dove.dtsi |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
index 3b891dd..a5441d5 100644
--- a/arch/arm/boot/dts/dove.dtsi
+++ b/arch/arm/boot/dts/dove.dtsi
@@ -630,6 +630,20 @@
 				reg = <0xe8400 0x0c>;
 				ngpios = <8>;
 			};
+
+			lcd1: lcd-controller at 810000 {
+				compatible = "marvell,dove-lcd";
+				reg = <0x810000 0x1000>;
+				interrupts = <46>;
+				status = "disabled";
+			};
+
+			lcd0: lcd-controller at 820000 {
+				compatible = "marvell,dove-lcd";
+				reg = <0x820000 0x1000>;
+				interrupts = <47>;
+				status = "disabled";
+			};
 		};
 	};
 };
-- 
1.7.2.5

[PATCH 3/4] ARM: dts: dove: enable lcd0 on SolidRun CuBox

[Sebastian Hesselbarth]

From: Russell King <rmk+kernel@arm.linux.org.uk>

This enables lcd0 on SolidRun CuBox which is connected to TDA998x
HDMI transmitter. Also, set the external video clock connected to
Si5351 clock generator.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/boot/dts/dove-cubox.dts |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/dove-cubox.dts b/arch/arm/boot/dts/dove-cubox.dts
index 7a70f4c..303a354 100644
--- a/arch/arm/boot/dts/dove-cubox.dts
+++ b/arch/arm/boot/dts/dove-cubox.dts
@@ -134,3 +134,9 @@
 	pinctrl-0 = <&pmx_audio1_i2s1_spdifo &pmx_audio1_extclk>;
 	pinctrl-names = "default";
 };
+
+&lcd0 {
+	status = "okay";
+	clocks = <&si5351 0>;
+	clock-names = "ext_ref_clk_1";
+};
-- 
1.7.2.5

[PATCH 3/4] ARM: dts: dove: enable lcd0 on SolidRun CuBox

[Sebastian Hesselbarth]

From: Russell King <rmk+kernel@arm.linux.org.uk>

This enables lcd0 on SolidRun CuBox which is connected to TDA998x
HDMI transmitter. Also, set the external video clock connected to
Si5351 clock generator.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/boot/dts/dove-cubox.dts |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/dove-cubox.dts b/arch/arm/boot/dts/dove-cubox.dts
index 7a70f4c..303a354 100644
--- a/arch/arm/boot/dts/dove-cubox.dts
+++ b/arch/arm/boot/dts/dove-cubox.dts
@@ -134,3 +134,9 @@
 	pinctrl-0 = <&pmx_audio1_i2s1_spdifo &pmx_audio1_extclk>;
 	pinctrl-names = "default";
 };
+
+&lcd0 {
+	status = "okay";
+	clocks = <&si5351 0>;
+	clock-names = "ext_ref_clk_1";
+};
-- 
1.7.2.5

[PATCH 3/4] ARM: dts: dove: enable lcd0 on SolidRun CuBox

[Sebastian Hesselbarth]

From: Russell King <rmk+kernel@arm.linux.org.uk>

This enables lcd0 on SolidRun CuBox which is connected to TDA998x
HDMI transmitter. Also, set the external video clock connected to
Si5351 clock generator.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: devicetree at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-kernel at vger.kernel.org
---
 arch/arm/boot/dts/dove-cubox.dts |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/dove-cubox.dts b/arch/arm/boot/dts/dove-cubox.dts
index 7a70f4c..303a354 100644
--- a/arch/arm/boot/dts/dove-cubox.dts
+++ b/arch/arm/boot/dts/dove-cubox.dts
@@ -134,3 +134,9 @@
 	pinctrl-0 = <&pmx_audio1_i2s1_spdifo &pmx_audio1_extclk>;
 	pinctrl-names = "default";
 };
+
+&lcd0 {
+	status = "okay";
+	clocks = <&si5351 0>;
+	clock-names = "ext_ref_clk_1";
+};
-- 
1.7.2.5

[PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Sebastian Hesselbarth]

From: Russell King <rmk+kernel@arm.linux.org.uk>

Adding both, driver and proper DT representation, especially when
subsystem DT bindings are not settled, is almost impossible right
now. To get some more code testing coverage on Armada DRM driver,
we add plain old platform_device registration now and get rid of
it incrementally as we did often in the past.

This adds DRM platform_device and required quirks to get it up
and running (framebuffer CMA, lcd resources, clks) which allows
us to remove it step-by-step again when bindings have settled.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/mach-mvebu/dove.c |   70 +++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 69 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-mvebu/dove.c b/arch/arm/mach-mvebu/dove.c
index b50464e..c8c2284 100644
--- a/arch/arm/mach-mvebu/dove.c
+++ b/arch/arm/mach-mvebu/dove.c
@@ -11,11 +11,75 @@
 #include <linux/init.h>
 #include <linux/mbus.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/of_platform.h>
 #include <asm/hardware/cache-tauros2.h>
 #include <asm/mach/arch.h>
+#include <asm/memblock.h>
 #include "common.h"
 
+/*
+ * Armada DRM framebuffer
+ */
+static struct resource armada_drm_resources[3] = {
+	DEFINE_RES_MEM(0, 0),
+};
+
+static const char *armada_drm_devices[4];
+
+static const struct platform_device_info armada_drm_dev_info __initconst = {
+	.name = "armada-510-drm",
+	.id = -1,
+	.res = armada_drm_resources,
+	.num_res = ARRAY_SIZE(armada_drm_resources),
+	.data = armada_drm_devices,
+	.size_data = sizeof(armada_drm_devices),
+};
+
+static void __init armada_drm_reserve(void)
+{
+	phys_addr_t phys;
+
+	/* Steal 32MB for the drm framebuffers */
+	phys = arm_memblock_steal(SZ_32M, SZ_2M);
+	armada_drm_resources[0].start = phys;
+	armada_drm_resources[0].end = phys + SZ_32M - 1;
+}
+
+static void __init armada_drm_init(void)
+{
+	struct device_node *np, *clknp = NULL;
+	struct platform_device *pdev;
+	int i = 0;
+
+	for_each_compatible_node(np, NULL, "marvell,dove-lcd") {
+		if (!of_device_is_available(np))
+			continue;
+
+		/* add lcd controller mmio resources */
+		if (of_address_to_resource(np, 0, &armada_drm_resources[i+1]))
+			continue;
+
+		/* get device name for component framework */
+		pdev = of_find_device_by_node(np);
+		if (pdev)
+			armada_drm_devices[i] = dev_name(&pdev->dev);
+		clknp = np;
+		i++;
+	}
+	armada_drm_devices[i++] = NULL;
+
+	pdev = platform_device_register_full(&armada_drm_dev_info);
+	/* assign last found lcd node to drm device for clk lookup */
+	pdev->dev.of_node = clknp;
+}
+
+static void __init dove_reserve(void)
+{
+	if (IS_ENABLED(CONFIG_DRM_ARMADA))
+		armada_drm_reserve();
+}
+
 static void __init dove_init(void)
 {
 	pr_info("Dove 88AP510 SoC\n");
@@ -25,6 +89,9 @@ static void __init dove_init(void)
 #endif
 	BUG_ON(mvebu_mbus_dt_init(false));
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+
+	if (IS_ENABLED(CONFIG_DRM_ARMADA))
+		armada_drm_init();
 }
 
 static const char * const dove_dt_compat[] = {
@@ -33,7 +100,8 @@ static const char * const dove_dt_compat[] = {
 };
 
 DT_MACHINE_START(DOVE_DT, "Marvell Dove")
+	.dt_compat	= dove_dt_compat,
 	.init_machine	= dove_init,
+	.reserve	= dove_reserve,
 	.restart	= mvebu_restart,
-	.dt_compat	= dove_dt_compat,
 MACHINE_END
-- 
1.7.2.5

[PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Sebastian Hesselbarth]

From: Russell King <rmk+kernel@arm.linux.org.uk>

Adding both, driver and proper DT representation, especially when
subsystem DT bindings are not settled, is almost impossible right
now. To get some more code testing coverage on Armada DRM driver,
we add plain old platform_device registration now and get rid of
it incrementally as we did often in the past.

This adds DRM platform_device and required quirks to get it up
and running (framebuffer CMA, lcd resources, clks) which allows
us to remove it step-by-step again when bindings have settled.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-kernel at vger.kernel.org
---
 arch/arm/mach-mvebu/dove.c |   70 +++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 69 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-mvebu/dove.c b/arch/arm/mach-mvebu/dove.c
index b50464e..c8c2284 100644
--- a/arch/arm/mach-mvebu/dove.c
+++ b/arch/arm/mach-mvebu/dove.c
@@ -11,11 +11,75 @@
 #include <linux/init.h>
 #include <linux/mbus.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/of_platform.h>
 #include <asm/hardware/cache-tauros2.h>
 #include <asm/mach/arch.h>
+#include <asm/memblock.h>
 #include "common.h"
 
+/*
+ * Armada DRM framebuffer
+ */
+static struct resource armada_drm_resources[3] = {
+	DEFINE_RES_MEM(0, 0),
+};
+
+static const char *armada_drm_devices[4];
+
+static const struct platform_device_info armada_drm_dev_info __initconst = {
+	.name = "armada-510-drm",
+	.id = -1,
+	.res = armada_drm_resources,
+	.num_res = ARRAY_SIZE(armada_drm_resources),
+	.data = armada_drm_devices,
+	.size_data = sizeof(armada_drm_devices),
+};
+
+static void __init armada_drm_reserve(void)
+{
+	phys_addr_t phys;
+
+	/* Steal 32MB for the drm framebuffers */
+	phys = arm_memblock_steal(SZ_32M, SZ_2M);
+	armada_drm_resources[0].start = phys;
+	armada_drm_resources[0].end = phys + SZ_32M - 1;
+}
+
+static void __init armada_drm_init(void)
+{
+	struct device_node *np, *clknp = NULL;
+	struct platform_device *pdev;
+	int i = 0;
+
+	for_each_compatible_node(np, NULL, "marvell,dove-lcd") {
+		if (!of_device_is_available(np))
+			continue;
+
+		/* add lcd controller mmio resources */
+		if (of_address_to_resource(np, 0, &armada_drm_resources[i+1]))
+			continue;
+
+		/* get device name for component framework */
+		pdev = of_find_device_by_node(np);
+		if (pdev)
+			armada_drm_devices[i] = dev_name(&pdev->dev);
+		clknp = np;
+		i++;
+	}
+	armada_drm_devices[i++] = NULL;
+
+	pdev = platform_device_register_full(&armada_drm_dev_info);
+	/* assign last found lcd node to drm device for clk lookup */
+	pdev->dev.of_node = clknp;
+}
+
+static void __init dove_reserve(void)
+{
+	if (IS_ENABLED(CONFIG_DRM_ARMADA))
+		armada_drm_reserve();
+}
+
 static void __init dove_init(void)
 {
 	pr_info("Dove 88AP510 SoC\n");
@@ -25,6 +89,9 @@ static void __init dove_init(void)
 #endif
 	BUG_ON(mvebu_mbus_dt_init(false));
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+
+	if (IS_ENABLED(CONFIG_DRM_ARMADA))
+		armada_drm_init();
 }
 
 static const char * const dove_dt_compat[] = {
@@ -33,7 +100,8 @@ static const char * const dove_dt_compat[] = {
 };
 
 DT_MACHINE_START(DOVE_DT, "Marvell Dove")
+	.dt_compat	= dove_dt_compat,
 	.init_machine	= dove_init,
+	.reserve	= dove_reserve,
 	.restart	= mvebu_restart,
-	.dt_compat	= dove_dt_compat,
 MACHINE_END
-- 
1.7.2.5

Re: [PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Russell King - ARM Linux]

On Tue, Jul 01, 2014 at 03:04:31PM +0200, Sebastian Hesselbarth wrote:
> +	pdev = platform_device_register_full(&armada_drm_dev_info);
> +	/* assign last found lcd node to drm device for clk lookup */
> +	pdev->dev.of_node = clknp;

NAK.  This really isn't a good way to deal with this, even in a
temporary basis.  While assigning a DT node to a manually created
platform device does solve that problem, it also introduces the
problem that this platform device will now match any platform driver
which recognises the "marvell,dove-lcd" compatible type, which may
occur _before_ we find the driver to match using the legacy strings.

There really isn't an easy solution to this other than doing the thing
properly.

The other problem in this series is that while you introduce some
bindings which may work today, they're not going to work tomorrow, and
that's a problem.  Don't do DT piecemeal like this and end up having to
break the bindings (which we will have to do to add the endpoints.)

If you want to do this then you need to add the endpoints from the start
even though the driver doesn't yet make use of them - or don't add the
DT bits at all.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Russell King - ARM Linux]

On Tue, Jul 01, 2014 at 03:04:31PM +0200, Sebastian Hesselbarth wrote:
> +	pdev = platform_device_register_full(&armada_drm_dev_info);
> +	/* assign last found lcd node to drm device for clk lookup */
> +	pdev->dev.of_node = clknp;

NAK.  This really isn't a good way to deal with this, even in a
temporary basis.  While assigning a DT node to a manually created
platform device does solve that problem, it also introduces the
problem that this platform device will now match any platform driver
which recognises the "marvell,dove-lcd" compatible type, which may
occur _before_ we find the driver to match using the legacy strings.

There really isn't an easy solution to this other than doing the thing
properly.

The other problem in this series is that while you introduce some
bindings which may work today, they're not going to work tomorrow, and
that's a problem.  Don't do DT piecemeal like this and end up having to
break the bindings (which we will have to do to add the endpoints.)

If you want to do this then you need to add the endpoints from the start
even though the driver doesn't yet make use of them - or don't add the
DT bits at all.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [PATCH 3/4] ARM: dts: dove: enable lcd0 on SolidRun CuBox

[Mark Rutland]

On Tue, Jul 01, 2014 at 02:04:30PM +0100, Sebastian Hesselbarth wrote:
> From: Russell King <rmk+kernel@arm.linux.org.uk>
> 
> This enables lcd0 on SolidRun CuBox which is connected to TDA998x
> HDMI transmitter. Also, set the external video clock connected to
> Si5351 clock generator.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> ---
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Cc: Russell King <rmk+kernel@arm.linux.org.uk>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Gregory Clement <gregory.clement@free-electrons.com>
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  arch/arm/boot/dts/dove-cubox.dts |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/dove-cubox.dts b/arch/arm/boot/dts/dove-cubox.dts
> index 7a70f4c..303a354 100644
> --- a/arch/arm/boot/dts/dove-cubox.dts
> +++ b/arch/arm/boot/dts/dove-cubox.dts
> @@ -134,3 +134,9 @@
>  	pinctrl-0 = <&pmx_audio1_i2s1_spdifo &pmx_audio1_extclk>;
>  	pinctrl-names = "default";
>  };
> +
> +&lcd0 {
> +	status = "okay";
> +	clocks = <&si5351 0>;
> +	clock-names = "ext_ref_clk_1";

This clock wasn't mentioned in the binding document. Could you please
amend that?

Thanks,
Mark.

Re: [PATCH 3/4] ARM: dts: dove: enable lcd0 on SolidRun CuBox

[Mark Rutland]

On Tue, Jul 01, 2014 at 02:04:30PM +0100, Sebastian Hesselbarth wrote:
> From: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
> 
> This enables lcd0 on SolidRun CuBox which is connected to TDA998x
> HDMI transmitter. Also, set the external video clock connected to
> Si5351 clock generator.
> 
> Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
> Cc: Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Cc: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
> Cc: Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org>
> Cc: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
> Cc: Gregory Clement <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  arch/arm/boot/dts/dove-cubox.dts |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/dove-cubox.dts b/arch/arm/boot/dts/dove-cubox.dts
> index 7a70f4c..303a354 100644
> --- a/arch/arm/boot/dts/dove-cubox.dts
> +++ b/arch/arm/boot/dts/dove-cubox.dts
> @@ -134,3 +134,9 @@
>  	pinctrl-0 = <&pmx_audio1_i2s1_spdifo &pmx_audio1_extclk>;
>  	pinctrl-names = "default";
>  };
> +
> +&lcd0 {
> +	status = "okay";
> +	clocks = <&si5351 0>;
> +	clock-names = "ext_ref_clk_1";

This clock wasn't mentioned in the binding document. Could you please
amend that?

Thanks,
Mark.
--
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

[PATCH 3/4] ARM: dts: dove: enable lcd0 on SolidRun CuBox

[Mark Rutland]

On Tue, Jul 01, 2014 at 02:04:30PM +0100, Sebastian Hesselbarth wrote:
> From: Russell King <rmk+kernel@arm.linux.org.uk>
> 
> This enables lcd0 on SolidRun CuBox which is connected to TDA998x
> HDMI transmitter. Also, set the external video clock connected to
> Si5351 clock generator.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> ---
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Cc: Russell King <rmk+kernel@arm.linux.org.uk>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Gregory Clement <gregory.clement@free-electrons.com>
> Cc: devicetree at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-kernel at vger.kernel.org
> ---
>  arch/arm/boot/dts/dove-cubox.dts |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/dove-cubox.dts b/arch/arm/boot/dts/dove-cubox.dts
> index 7a70f4c..303a354 100644
> --- a/arch/arm/boot/dts/dove-cubox.dts
> +++ b/arch/arm/boot/dts/dove-cubox.dts
> @@ -134,3 +134,9 @@
>  	pinctrl-0 = <&pmx_audio1_i2s1_spdifo &pmx_audio1_extclk>;
>  	pinctrl-names = "default";
>  };
> +
> +&lcd0 {
> +	status = "okay";
> +	clocks = <&si5351 0>;
> +	clock-names = "ext_ref_clk_1";

This clock wasn't mentioned in the binding document. Could you please
amend that?

Thanks,
Mark.

Re: [PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Sebastian Hesselbarth]

On 07/01/2014 03:10 PM, Russell King - ARM Linux wrote:
> On Tue, Jul 01, 2014 at 03:04:31PM +0200, Sebastian Hesselbarth wrote:
>> +	pdev = platform_device_register_full(&armada_drm_dev_info);
>> +	/* assign last found lcd node to drm device for clk lookup */
>> +	pdev->dev.of_node = clknp;
>
> NAK.  This really isn't a good way to deal with this, even in a
> temporary basis.  While assigning a DT node to a manually created
> platform device does solve that problem, it also introduces the
> problem that this platform device will now match any platform driver
> which recognises the "marvell,dove-lcd" compatible type, which may
> occur _before_ we find the driver to match using the legacy strings.

Right, I never said it is a good solution but there is no driver for
"marvell,dove-lcd" *and* there is no way to assign clock aliases for
clocks not yet registered.

> There really isn't an easy solution to this other than doing the thing
> properly.

Well, you may have noticed that three moving subsystems plus new
bindings plus non-DT/DT drivers quickly create some kind of patch
deadlock. This is a dirty but tiny step to resolve one of those
deadlocks.

> The other problem in this series is that while you introduce some
> bindings which may work today, they're not going to work tomorrow, and
> that's a problem.  Don't do DT piecemeal like this and end up having to
> break the bindings (which we will have to do to add the endpoints.)

Adding new properties/subnodes never has been a problem for us at all.
New generic bindings were introduced *often* in the past and added to
existing bindings, e.g. clocks, gpio, pinctrl.

The proposed binding for dove-lcd simply reflects the tiny part that
is mandatory for identifying the lcd controllers. It only contains
reg and interrupts which would also be in the corresponding
platform_device.

> If you want to do this then you need to add the endpoints from the start
> even though the driver doesn't yet make use of them - or don't add the
> DT bits at all.

If you really think that way, I definitely give up on mainline Dove and
SolidRun Cubox. You are /really/ proposing to wait for *all* related
subsystem bindings to settle before even starting to add DT support?

Sebastian

[PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Sebastian Hesselbarth]

On 07/01/2014 03:10 PM, Russell King - ARM Linux wrote:
> On Tue, Jul 01, 2014 at 03:04:31PM +0200, Sebastian Hesselbarth wrote:
>> +	pdev = platform_device_register_full(&armada_drm_dev_info);
>> +	/* assign last found lcd node to drm device for clk lookup */
>> +	pdev->dev.of_node = clknp;
>
> NAK.  This really isn't a good way to deal with this, even in a
> temporary basis.  While assigning a DT node to a manually created
> platform device does solve that problem, it also introduces the
> problem that this platform device will now match any platform driver
> which recognises the "marvell,dove-lcd" compatible type, which may
> occur _before_ we find the driver to match using the legacy strings.

Right, I never said it is a good solution but there is no driver for
"marvell,dove-lcd" *and* there is no way to assign clock aliases for
clocks not yet registered.

> There really isn't an easy solution to this other than doing the thing
> properly.

Well, you may have noticed that three moving subsystems plus new
bindings plus non-DT/DT drivers quickly create some kind of patch
deadlock. This is a dirty but tiny step to resolve one of those
deadlocks.

> The other problem in this series is that while you introduce some
> bindings which may work today, they're not going to work tomorrow, and
> that's a problem.  Don't do DT piecemeal like this and end up having to
> break the bindings (which we will have to do to add the endpoints.)

Adding new properties/subnodes never has been a problem for us at all.
New generic bindings were introduced *often* in the past and added to
existing bindings, e.g. clocks, gpio, pinctrl.

The proposed binding for dove-lcd simply reflects the tiny part that
is mandatory for identifying the lcd controllers. It only contains
reg and interrupts which would also be in the corresponding
platform_device.

> If you want to do this then you need to add the endpoints from the start
> even though the driver doesn't yet make use of them - or don't add the
> DT bits at all.

If you really think that way, I definitely give up on mainline Dove and
SolidRun Cubox. You are /really/ proposing to wait for *all* related
subsystem bindings to settle before even starting to add DT support?

Sebastian

Re: [PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Russell King - ARM Linux]

On Tue, Jul 01, 2014 at 03:21:41PM +0200, Sebastian Hesselbarth wrote:
> On 07/01/2014 03:10 PM, Russell King - ARM Linux wrote:
>> On Tue, Jul 01, 2014 at 03:04:31PM +0200, Sebastian Hesselbarth wrote:
>>> +	pdev = platform_device_register_full(&armada_drm_dev_info);
>>> +	/* assign last found lcd node to drm device for clk lookup */
>>> +	pdev->dev.of_node = clknp;
>>
>> NAK.  This really isn't a good way to deal with this, even in a
>> temporary basis.  While assigning a DT node to a manually created
>> platform device does solve that problem, it also introduces the
>> problem that this platform device will now match any platform driver
>> which recognises the "marvell,dove-lcd" compatible type, which may
>> occur _before_ we find the driver to match using the legacy strings.
>
> Right, I never said it is a good solution but there is no driver for
> "marvell,dove-lcd" *and* there is no way to assign clock aliases for
> clocks not yet registered.

Now consider what happens when I get to push the updates which do add
that to David Airlie.

> Well, you may have noticed that three moving subsystems plus new
> bindings plus non-DT/DT drivers quickly create some kind of patch
> deadlock. This is a dirty but tiny step to resolve one of those
> deadlocks.

The deadlock only exists because it is impossible to get anyone to look
outside their own little world and review patches.  This is a growing
problem, and there's a growing number of articles and blogs on the
Internet about how kernel development now sucks rocks because of it.

Where do you get three moving subsystems from anyway?  There's the
component stuff and the DRM stuff - that's two subsystems.  The component
stuff is something which I maintain, but needs to go through Greg.  I'm
seriously thinking about just pushing those three patches to Greg in the
next few days.  Then I'll start talking to David about the rest of it.

The Armada DRM stuff shouldn't be a problem, but the TDA998x may end up
being so /if/ Jean-Francois decides to object to my approach there to
allow tilcdc to continue working with the driver... but then as I now
maintain that as well, that's not much of an issue because I can make
the decision to overrule that point.

>> The other problem in this series is that while you introduce some
>> bindings which may work today, they're not going to work tomorrow, and
>> that's a problem.  Don't do DT piecemeal like this and end up having to
>> break the bindings (which we will have to do to add the endpoints.)
>
> Adding new properties/subnodes never has been a problem for us at all.
> New generic bindings were introduced *often* in the past and added to
> existing bindings, e.g. clocks, gpio, pinctrl.
>
> The proposed binding for dove-lcd simply reflects the tiny part that
> is mandatory for identifying the lcd controllers. It only contains
> reg and interrupts which would also be in the corresponding
> platform_device.

There is a difference between augmenting an existing driver binding for
new features (such as adding a GPIO to allow a new platform to work) and
adding new bindings which are required becaues we've updated the driver.

The former adds a new feature but allows existing users to continue
working.  The latter adds a new feature which breaks existing users.
That's a problem.

In this case, it can be trivially avoided by adding all the properties
that we need right from the start.  We know what we need, and the
of_graph stuff is now established and in use by other DRM drivers.

Just because the existing driver may not make use of it is no excuse to
omit it - it's part of the description of how the hardware is connected,
it conforms to agreed methods to describe this connectivity, we know that
it works, and we know that the driver will need it in the very near future.

Omitting it now just results in additional pain later on that we can
avoid.

>> If you want to do this then you need to add the endpoints from the start
>> even though the driver doesn't yet make use of them - or don't add the
>> DT bits at all.
>
> If you really think that way, I definitely give up on mainline Dove and
> SolidRun Cubox. You are /really/ proposing to wait for *all* related
> subsystem bindings to settle before even starting to add DT support?

*Sigh*.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Russell King - ARM Linux]

On Tue, Jul 01, 2014 at 03:21:41PM +0200, Sebastian Hesselbarth wrote:
> On 07/01/2014 03:10 PM, Russell King - ARM Linux wrote:
>> On Tue, Jul 01, 2014 at 03:04:31PM +0200, Sebastian Hesselbarth wrote:
>>> +	pdev = platform_device_register_full(&armada_drm_dev_info);
>>> +	/* assign last found lcd node to drm device for clk lookup */
>>> +	pdev->dev.of_node = clknp;
>>
>> NAK.  This really isn't a good way to deal with this, even in a
>> temporary basis.  While assigning a DT node to a manually created
>> platform device does solve that problem, it also introduces the
>> problem that this platform device will now match any platform driver
>> which recognises the "marvell,dove-lcd" compatible type, which may
>> occur _before_ we find the driver to match using the legacy strings.
>
> Right, I never said it is a good solution but there is no driver for
> "marvell,dove-lcd" *and* there is no way to assign clock aliases for
> clocks not yet registered.

Now consider what happens when I get to push the updates which do add
that to David Airlie.

> Well, you may have noticed that three moving subsystems plus new
> bindings plus non-DT/DT drivers quickly create some kind of patch
> deadlock. This is a dirty but tiny step to resolve one of those
> deadlocks.

The deadlock only exists because it is impossible to get anyone to look
outside their own little world and review patches.  This is a growing
problem, and there's a growing number of articles and blogs on the
Internet about how kernel development now sucks rocks because of it.

Where do you get three moving subsystems from anyway?  There's the
component stuff and the DRM stuff - that's two subsystems.  The component
stuff is something which I maintain, but needs to go through Greg.  I'm
seriously thinking about just pushing those three patches to Greg in the
next few days.  Then I'll start talking to David about the rest of it.

The Armada DRM stuff shouldn't be a problem, but the TDA998x may end up
being so /if/ Jean-Francois decides to object to my approach there to
allow tilcdc to continue working with the driver... but then as I now
maintain that as well, that's not much of an issue because I can make
the decision to overrule that point.

>> The other problem in this series is that while you introduce some
>> bindings which may work today, they're not going to work tomorrow, and
>> that's a problem.  Don't do DT piecemeal like this and end up having to
>> break the bindings (which we will have to do to add the endpoints.)
>
> Adding new properties/subnodes never has been a problem for us at all.
> New generic bindings were introduced *often* in the past and added to
> existing bindings, e.g. clocks, gpio, pinctrl.
>
> The proposed binding for dove-lcd simply reflects the tiny part that
> is mandatory for identifying the lcd controllers. It only contains
> reg and interrupts which would also be in the corresponding
> platform_device.

There is a difference between augmenting an existing driver binding for
new features (such as adding a GPIO to allow a new platform to work) and
adding new bindings which are required becaues we've updated the driver.

The former adds a new feature but allows existing users to continue
working.  The latter adds a new feature which breaks existing users.
That's a problem.

In this case, it can be trivially avoided by adding all the properties
that we need right from the start.  We know what we need, and the
of_graph stuff is now established and in use by other DRM drivers.

Just because the existing driver may not make use of it is no excuse to
omit it - it's part of the description of how the hardware is connected,
it conforms to agreed methods to describe this connectivity, we know that
it works, and we know that the driver will need it in the very near future.

Omitting it now just results in additional pain later on that we can
avoid.

>> If you want to do this then you need to add the endpoints from the start
>> even though the driver doesn't yet make use of them - or don't add the
>> DT bits at all.
>
> If you really think that way, I definitely give up on mainline Dove and
> SolidRun Cubox. You are /really/ proposing to wait for *all* related
> subsystem bindings to settle before even starting to add DT support?

*Sigh*.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Sebastian Hesselbarth]

On 07/01/2014 03:36 PM, Russell King - ARM Linux wrote:
> On Tue, Jul 01, 2014 at 03:21:41PM +0200, Sebastian Hesselbarth wrote:
>> On 07/01/2014 03:10 PM, Russell King - ARM Linux wrote:
>>> On Tue, Jul 01, 2014 at 03:04:31PM +0200, Sebastian Hesselbarth wrote:
>>> If you want to do this then you need to add the endpoints from the start
>>> even though the driver doesn't yet make use of them - or don't add the
>>> DT bits at all.
>>
>> If you really think that way, I definitely give up on mainline Dove and
>> SolidRun Cubox. You are /really/ proposing to wait for *all* related
>> subsystem bindings to settle before even starting to add DT support?
>
> *Sigh*.

Russell,

honestly, I am out. Feel free to take over anything you want. I
understand you are in a bad mood because you feel ignored on your
patches, but for me your attitude is unbearable and *the* reason
to step back now.

Sebastian

[PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Sebastian Hesselbarth]

On 07/01/2014 03:36 PM, Russell King - ARM Linux wrote:
> On Tue, Jul 01, 2014 at 03:21:41PM +0200, Sebastian Hesselbarth wrote:
>> On 07/01/2014 03:10 PM, Russell King - ARM Linux wrote:
>>> On Tue, Jul 01, 2014 at 03:04:31PM +0200, Sebastian Hesselbarth wrote:
>>> If you want to do this then you need to add the endpoints from the start
>>> even though the driver doesn't yet make use of them - or don't add the
>>> DT bits at all.
>>
>> If you really think that way, I definitely give up on mainline Dove and
>> SolidRun Cubox. You are /really/ proposing to wait for *all* related
>> subsystem bindings to settle before even starting to add DT support?
>
> *Sigh*.

Russell,

honestly, I am out. Feel free to take over anything you want. I
understand you are in a bad mood because you feel ignored on your
patches, but for me your attitude is unbearable and *the* reason
to step back now.

Sebastian

Re: [PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Russell King - ARM Linux]

On Tue, Jul 01, 2014 at 03:40:14PM +0200, Sebastian Hesselbarth wrote:
> On 07/01/2014 03:36 PM, Russell King - ARM Linux wrote:
>> On Tue, Jul 01, 2014 at 03:21:41PM +0200, Sebastian Hesselbarth wrote:
>>> On 07/01/2014 03:10 PM, Russell King - ARM Linux wrote:
>>>> On Tue, Jul 01, 2014 at 03:04:31PM +0200, Sebastian Hesselbarth wrote:
>>>> If you want to do this then you need to add the endpoints from the start
>>>> even though the driver doesn't yet make use of them - or don't add the
>>>> DT bits at all.
>>>
>>> If you really think that way, I definitely give up on mainline Dove and
>>> SolidRun Cubox. You are /really/ proposing to wait for *all* related
>>> subsystem bindings to settle before even starting to add DT support?
>>
>> *Sigh*.
>
> Russell,
>
> honestly, I am out. Feel free to take over anything you want. I
> understand you are in a bad mood because you feel ignored on your
> patches, but for me your attitude is unbearable and *the* reason
> to step back now.

That's fine, and probably for the best, since you appear not to be able
to take part in the completely reasonable technical discussion in this
thread.

I wish you all the best in your future endeavours.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Russell King - ARM Linux]

On Tue, Jul 01, 2014 at 03:40:14PM +0200, Sebastian Hesselbarth wrote:
> On 07/01/2014 03:36 PM, Russell King - ARM Linux wrote:
>> On Tue, Jul 01, 2014 at 03:21:41PM +0200, Sebastian Hesselbarth wrote:
>>> On 07/01/2014 03:10 PM, Russell King - ARM Linux wrote:
>>>> On Tue, Jul 01, 2014 at 03:04:31PM +0200, Sebastian Hesselbarth wrote:
>>>> If you want to do this then you need to add the endpoints from the start
>>>> even though the driver doesn't yet make use of them - or don't add the
>>>> DT bits at all.
>>>
>>> If you really think that way, I definitely give up on mainline Dove and
>>> SolidRun Cubox. You are /really/ proposing to wait for *all* related
>>> subsystem bindings to settle before even starting to add DT support?
>>
>> *Sigh*.
>
> Russell,
>
> honestly, I am out. Feel free to take over anything you want. I
> understand you are in a bad mood because you feel ignored on your
> patches, but for me your attitude is unbearable and *the* reason
> to step back now.

That's fine, and probably for the best, since you appear not to be able
to take part in the completely reasonable technical discussion in this
thread.

I wish you all the best in your future endeavours.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [PATCH 3/4] ARM: dts: dove: enable lcd0 on SolidRun CuBox

[Jean-Francois Moine]

On Tue,  1 Jul 2014 15:04:30 +0200
Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> wrote:

> From: Russell King <rmk+kernel@arm.linux.org.uk>
> 
> This enables lcd0 on SolidRun CuBox which is connected to TDA998x
> HDMI transmitter. Also, set the external video clock connected to
> Si5351 clock generator.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> ---
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Cc: Russell King <rmk+kernel@arm.linux.org.uk>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Gregory Clement <gregory.clement@free-electrons.com>
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  arch/arm/boot/dts/dove-cubox.dts |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/dove-cubox.dts b/arch/arm/boot/dts/dove-cubox.dts
> index 7a70f4c..303a354 100644
> --- a/arch/arm/boot/dts/dove-cubox.dts
> +++ b/arch/arm/boot/dts/dove-cubox.dts
> @@ -134,3 +134,9 @@
>  	pinctrl-0 = <&pmx_audio1_i2s1_spdifo &pmx_audio1_extclk>;
>  	pinctrl-names = "default";
>  };
> +
> +&lcd0 {
> +	status = "okay";
> +	clocks = <&si5351 0>;
> +	clock-names = "ext_ref_clk_1";
> +};

My driver has "axiclk", "extclk0", "lcdpll" and "extclk1" as the clock
names. It is a bit shorter.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

Re: [PATCH 3/4] ARM: dts: dove: enable lcd0 on SolidRun CuBox

[Jean-Francois Moine]

On Tue,  1 Jul 2014 15:04:30 +0200
Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> From: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
> 
> This enables lcd0 on SolidRun CuBox which is connected to TDA998x
> HDMI transmitter. Also, set the external video clock connected to
> Si5351 clock generator.
> 
> Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
> Cc: Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Cc: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
> Cc: Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org>
> Cc: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
> Cc: Gregory Clement <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  arch/arm/boot/dts/dove-cubox.dts |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/dove-cubox.dts b/arch/arm/boot/dts/dove-cubox.dts
> index 7a70f4c..303a354 100644
> --- a/arch/arm/boot/dts/dove-cubox.dts
> +++ b/arch/arm/boot/dts/dove-cubox.dts
> @@ -134,3 +134,9 @@
>  	pinctrl-0 = <&pmx_audio1_i2s1_spdifo &pmx_audio1_extclk>;
>  	pinctrl-names = "default";
>  };
> +
> +&lcd0 {
> +	status = "okay";
> +	clocks = <&si5351 0>;
> +	clock-names = "ext_ref_clk_1";
> +};

My driver has "axiclk", "extclk0", "lcdpll" and "extclk1" as the clock
names. It is a bit shorter.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
--
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

[PATCH 3/4] ARM: dts: dove: enable lcd0 on SolidRun CuBox

[Jean-Francois Moine]

On Tue,  1 Jul 2014 15:04:30 +0200
Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> wrote:

> From: Russell King <rmk+kernel@arm.linux.org.uk>
> 
> This enables lcd0 on SolidRun CuBox which is connected to TDA998x
> HDMI transmitter. Also, set the external video clock connected to
> Si5351 clock generator.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> ---
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Cc: Russell King <rmk+kernel@arm.linux.org.uk>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Gregory Clement <gregory.clement@free-electrons.com>
> Cc: devicetree at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-kernel at vger.kernel.org
> ---
>  arch/arm/boot/dts/dove-cubox.dts |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/dove-cubox.dts b/arch/arm/boot/dts/dove-cubox.dts
> index 7a70f4c..303a354 100644
> --- a/arch/arm/boot/dts/dove-cubox.dts
> +++ b/arch/arm/boot/dts/dove-cubox.dts
> @@ -134,3 +134,9 @@
>  	pinctrl-0 = <&pmx_audio1_i2s1_spdifo &pmx_audio1_extclk>;
>  	pinctrl-names = "default";
>  };
> +
> +&lcd0 {
> +	status = "okay";
> +	clocks = <&si5351 0>;
> +	clock-names = "ext_ref_clk_1";
> +};

My driver has "axiclk", "extclk0", "lcdpll" and "extclk1" as the clock
names. It is a bit shorter.

-- 
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

Re: [PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Jean-Francois Moine]

On Tue, 1 Jul 2014 14:36:27 +0100
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> The Armada DRM stuff shouldn't be a problem, but the TDA998x may end up
> being so /if/ Jean-Francois decides to object to my approach there to
> allow tilcdc to continue working with the driver... but then as I now
> maintain that as well, that's not much of an issue because I can make
> the decision to overrule that point.

I proposed a patch to move the tda998x from a slave_encoder to a simple
encoder/connector. This patch included the associated changes to the
tilcdc. It has been violently refused. So, I will keep out of tree my
tda998x and Dove DRM drivers.

Now, the last thing for me is to put the TDA998x codec in the kernel
(it is also working in an other machine with 2 tda998x's).

So, you may do what you want...

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

[PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Jean-Francois Moine]

On Tue, 1 Jul 2014 14:36:27 +0100
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> The Armada DRM stuff shouldn't be a problem, but the TDA998x may end up
> being so /if/ Jean-Francois decides to object to my approach there to
> allow tilcdc to continue working with the driver... but then as I now
> maintain that as well, that's not much of an issue because I can make
> the decision to overrule that point.

I proposed a patch to move the tda998x from a slave_encoder to a simple
encoder/connector. This patch included the associated changes to the
tilcdc. It has been violently refused. So, I will keep out of tree my
tda998x and Dove DRM drivers.

Now, the last thing for me is to put the TDA998x codec in the kernel
(it is also working in an other machine with 2 tda998x's).

So, you may do what you want...

-- 
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

Re: [PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Russell King - ARM Linux]

On Tue, Jul 01, 2014 at 06:04:26PM +0200, Jean-Francois Moine wrote:
> On Tue, 1 Jul 2014 14:36:27 +0100
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> 
> > The Armada DRM stuff shouldn't be a problem, but the TDA998x may end up
> > being so /if/ Jean-Francois decides to object to my approach there to
> > allow tilcdc to continue working with the driver... but then as I now
> > maintain that as well, that's not much of an issue because I can make
> > the decision to overrule that point.
> 
> I proposed a patch to move the tda998x from a slave_encoder to a simple
> encoder/connector. This patch included the associated changes to the
> tilcdc. It has been violently refused. So, I will keep out of tree my
> tda998x and Dove DRM drivers.

Let's tell the full story rather than just presenting half of it.

You indeed wanted to do what you said above, but you also wanted to
completely change the component helpers in a way that I was not happy
with.  You wanted to add all sorts of DT specific gunk into the
helpers, which would have tied it to DT.

While DT is the current thing on ARM, it is /not/ the current thing
everywhere - a point which you failed to grasp.

You wanted to make the component operations optional, which I pointed
out makes no sense (because then components have no way to know what
happens to their master device - which is /really/ important for DRM.)

You refused to listen to those concerns, and refused to look at the
patch which I proposed, which did exactly the same as your patch,
while keeping the DRM slave interfaces for tilcdc to use, until they
have a chance to convert over.

You kept telling me that I had "opened the door" to your changes.  I
claimed that your changes abused the code which I had written - a point
which I still maintain to this day.

You also claimed that deferred probing didn't work.  Since the component
helpers were designed with the deferred probing problem in mind at the
time, and include the solution to that problem - which has been well
tested hundreds if not thousands of times by now - and you did not
provide the technical details as to why you thought it didn't work,
there was nothing that could be done to progress that point.

Moreover, your abuse of the component layer would have made it more
difficult to maintain it into the future - which is a fundamental
point which has to be considered when accepting any patch into the
kernel.  If a patch makes some code unable to be maintained, then an
alternative approach has to be found.  Since you were not willing to
compromise on finding or considering alternative approaches such as
the one I presented.

Since the component layer had had various comments which were in
progress, and your abuse of the component layer would have also
prevented those changes taking place (which are - in part - the set
of component patches which are on the list now) there is no way that
your uncompromising set of patches would be merged - at least not
until you start accepting some of the comments being given to you.

> Now, the last thing for me is to put the TDA998x codec in the kernel
> (it is also working in an other machine with 2 tda998x's).

Yes, supporting the I2S connection is something that is need, but we
/also/ need to support SPDIF, and SPDIF is the preferred method on
the Cubox.  SPDIF should be used to talk to the TDA998x whenever
possible because it opens up the possibility for sending out
compressed MPEG2 and AC-3 audio streams, thus offloading the decode
of these formats to external hardware.

This works today, and is a feature that people have been using with
platforms such as xbmc and various other installations on the Cubox.

Limiting to I2S means that you can't send out these compressed audio
streams.  In fact, the Dove manual tells you that you must disable
the I2S playback stream if you're sending non-PCM - non-PCM is only
supported via SPDIF.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Russell King - ARM Linux]

On Tue, Jul 01, 2014 at 06:04:26PM +0200, Jean-Francois Moine wrote:
> On Tue, 1 Jul 2014 14:36:27 +0100
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> 
> > The Armada DRM stuff shouldn't be a problem, but the TDA998x may end up
> > being so /if/ Jean-Francois decides to object to my approach there to
> > allow tilcdc to continue working with the driver... but then as I now
> > maintain that as well, that's not much of an issue because I can make
> > the decision to overrule that point.
> 
> I proposed a patch to move the tda998x from a slave_encoder to a simple
> encoder/connector. This patch included the associated changes to the
> tilcdc. It has been violently refused. So, I will keep out of tree my
> tda998x and Dove DRM drivers.

Let's tell the full story rather than just presenting half of it.

You indeed wanted to do what you said above, but you also wanted to
completely change the component helpers in a way that I was not happy
with.  You wanted to add all sorts of DT specific gunk into the
helpers, which would have tied it to DT.

While DT is the current thing on ARM, it is /not/ the current thing
everywhere - a point which you failed to grasp.

You wanted to make the component operations optional, which I pointed
out makes no sense (because then components have no way to know what
happens to their master device - which is /really/ important for DRM.)

You refused to listen to those concerns, and refused to look at the
patch which I proposed, which did exactly the same as your patch,
while keeping the DRM slave interfaces for tilcdc to use, until they
have a chance to convert over.

You kept telling me that I had "opened the door" to your changes.  I
claimed that your changes abused the code which I had written - a point
which I still maintain to this day.

You also claimed that deferred probing didn't work.  Since the component
helpers were designed with the deferred probing problem in mind at the
time, and include the solution to that problem - which has been well
tested hundreds if not thousands of times by now - and you did not
provide the technical details as to why you thought it didn't work,
there was nothing that could be done to progress that point.

Moreover, your abuse of the component layer would have made it more
difficult to maintain it into the future - which is a fundamental
point which has to be considered when accepting any patch into the
kernel.  If a patch makes some code unable to be maintained, then an
alternative approach has to be found.  Since you were not willing to
compromise on finding or considering alternative approaches such as
the one I presented.

Since the component layer had had various comments which were in
progress, and your abuse of the component layer would have also
prevented those changes taking place (which are - in part - the set
of component patches which are on the list now) there is no way that
your uncompromising set of patches would be merged - at least not
until you start accepting some of the comments being given to you.

> Now, the last thing for me is to put the TDA998x codec in the kernel
> (it is also working in an other machine with 2 tda998x's).

Yes, supporting the I2S connection is something that is need, but we
/also/ need to support SPDIF, and SPDIF is the preferred method on
the Cubox.  SPDIF should be used to talk to the TDA998x whenever
possible because it opens up the possibility for sending out
compressed MPEG2 and AC-3 audio streams, thus offloading the decode
of these formats to external hardware.

This works today, and is a feature that people have been using with
platforms such as xbmc and various other installations on the Cubox.

Limiting to I2S means that you can't send out these compressed audio
streams.  In fact, the Dove manual tells you that you must disable
the I2S playback stream if you're sending non-PCM - non-PCM is only
supported via SPDIF.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Jason Cooper]

Russell,

On Tue, Jul 01, 2014 at 02:10:26PM +0100, Russell King - ARM Linux wrote:
> On Tue, Jul 01, 2014 at 03:04:31PM +0200, Sebastian Hesselbarth wrote:
> > +	pdev = platform_device_register_full(&armada_drm_dev_info);
> > +	/* assign last found lcd node to drm device for clk lookup */
> > +	pdev->dev.of_node = clknp;
> 
> NAK.  This really isn't a good way to deal with this, even in a
> temporary basis.  While assigning a DT node to a manually created
> platform device does solve that problem, it also introduces the
> problem that this platform device will now match any platform driver
> which recognises the "marvell,dove-lcd" compatible type, which may
> occur _before_ we find the driver to match using the legacy strings.
> 
> There really isn't an easy solution to this other than doing the thing
> properly.

And this creates our current deadlock.

> The other problem in this series is that while you introduce some
> bindings which may work today, they're not going to work tomorrow, and
> that's a problem.  Don't do DT piecemeal like this and end up having to
> break the bindings (which we will have to do to add the endpoints.)

Seriously, we've got to do something to get off of the X.  Why not just
declare the binding Sebastian just sent as unstable?  Then we change it
as needed until we get all the pieces in place.

At KS2013, we discussed, and agreed upon the fact that we _can_ have
unstable bindings to keep the bus moving.  Perfection being the enemy of
getting it done and all.

Once we reach the goal of how we want the bindings to look, then we
declare it stable.  I know it's not ideal, but "plan it out perfectly
before pushing anything" isn't realistic either.

thx,

Jason.

[PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Jason Cooper]

Russell,

On Tue, Jul 01, 2014 at 02:10:26PM +0100, Russell King - ARM Linux wrote:
> On Tue, Jul 01, 2014 at 03:04:31PM +0200, Sebastian Hesselbarth wrote:
> > +	pdev = platform_device_register_full(&armada_drm_dev_info);
> > +	/* assign last found lcd node to drm device for clk lookup */
> > +	pdev->dev.of_node = clknp;
> 
> NAK.  This really isn't a good way to deal with this, even in a
> temporary basis.  While assigning a DT node to a manually created
> platform device does solve that problem, it also introduces the
> problem that this platform device will now match any platform driver
> which recognises the "marvell,dove-lcd" compatible type, which may
> occur _before_ we find the driver to match using the legacy strings.
> 
> There really isn't an easy solution to this other than doing the thing
> properly.

And this creates our current deadlock.

> The other problem in this series is that while you introduce some
> bindings which may work today, they're not going to work tomorrow, and
> that's a problem.  Don't do DT piecemeal like this and end up having to
> break the bindings (which we will have to do to add the endpoints.)

Seriously, we've got to do something to get off of the X.  Why not just
declare the binding Sebastian just sent as unstable?  Then we change it
as needed until we get all the pieces in place.

At KS2013, we discussed, and agreed upon the fact that we _can_ have
unstable bindings to keep the bus moving.  Perfection being the enemy of
getting it done and all.

Once we reach the goal of how we want the bindings to look, then we
declare it stable.  I know it's not ideal, but "plan it out perfectly
before pushing anything" isn't realistic either.

thx,

Jason.

Re: [PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Russell King - ARM Linux]

On Tue, Jul 01, 2014 at 12:53:19PM -0400, Jason Cooper wrote:
> Russell,
> 
> On Tue, Jul 01, 2014 at 02:10:26PM +0100, Russell King - ARM Linux wrote:
> > On Tue, Jul 01, 2014 at 03:04:31PM +0200, Sebastian Hesselbarth wrote:
> > > +	pdev = platform_device_register_full(&armada_drm_dev_info);
> > > +	/* assign last found lcd node to drm device for clk lookup */
> > > +	pdev->dev.of_node = clknp;
> > 
> > NAK.  This really isn't a good way to deal with this, even in a
> > temporary basis.  While assigning a DT node to a manually created
> > platform device does solve that problem, it also introduces the
> > problem that this platform device will now match any platform driver
> > which recognises the "marvell,dove-lcd" compatible type, which may
> > occur _before_ we find the driver to match using the legacy strings.
> > 
> > There really isn't an easy solution to this other than doing the thing
> > properly.
> 
> And this creates our current deadlock.

Right, and what it does is it creates /another/ deadlock.  Let's say
that we get those changes in place in the mvebu tree.

I'm pushing to get the component stuff in place right now.  Once it's
in place, I'll be pushing the Armada DRM stuff forward.  Let's say
that gets in.

We now have a problem.  The mvebu stuff now creates a platform device
which will bind with the CRTC part of Armada DRM, rather than the
master part and everything now breaks.

So, rather than having a component / DRM problem, it ends up becoming
a component / DRM / mvebu problem - rather than just two subsystems
needing coordination, we end up with /another/ subsystem to contend
with as well.

> > The other problem in this series is that while you introduce some
> > bindings which may work today, they're not going to work tomorrow, and
> > that's a problem.  Don't do DT piecemeal like this and end up having to
> > break the bindings (which we will have to do to add the endpoints.)
> 
> Seriously, we've got to do something to get off of the X.  Why not just
> declare the binding Sebastian just sent as unstable?  Then we change it
> as needed until we get all the pieces in place.

As I've already pointed out, why not add the complete binding right now
and be done with it?  That would sort out my concern about DT, and -
as I've already said (again) - we know exactly how this should look
because we have the of_graph stuff in drivers/of as standard helpers,
and we have imx-drm and v4l2 media stuff using it already.  It's
well established, and I've demonstrated that Armada DRM _will_ also
conform.

In fact, I have plans to consolidate the code between Armada DRM and
imx-drm into some standard DRM helpers (if you read through the patches
I linked to on cgit, you will notice that has already started.)

But... the DT issue is relatively minor compared to the one which I
raise above, which adds to the problems of moving forward.  It
effectively means I need to then carry a patch which undoes that,
and that patch would need to go through the DRM tree along with the
rest of the Armada DRM updates, which means more people need to be
involved, which then makes the process even slower than it already
is.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Russell King - ARM Linux]

On Tue, Jul 01, 2014 at 12:53:19PM -0400, Jason Cooper wrote:
> Russell,
> 
> On Tue, Jul 01, 2014 at 02:10:26PM +0100, Russell King - ARM Linux wrote:
> > On Tue, Jul 01, 2014 at 03:04:31PM +0200, Sebastian Hesselbarth wrote:
> > > +	pdev = platform_device_register_full(&armada_drm_dev_info);
> > > +	/* assign last found lcd node to drm device for clk lookup */
> > > +	pdev->dev.of_node = clknp;
> > 
> > NAK.  This really isn't a good way to deal with this, even in a
> > temporary basis.  While assigning a DT node to a manually created
> > platform device does solve that problem, it also introduces the
> > problem that this platform device will now match any platform driver
> > which recognises the "marvell,dove-lcd" compatible type, which may
> > occur _before_ we find the driver to match using the legacy strings.
> > 
> > There really isn't an easy solution to this other than doing the thing
> > properly.
> 
> And this creates our current deadlock.

Right, and what it does is it creates /another/ deadlock.  Let's say
that we get those changes in place in the mvebu tree.

I'm pushing to get the component stuff in place right now.  Once it's
in place, I'll be pushing the Armada DRM stuff forward.  Let's say
that gets in.

We now have a problem.  The mvebu stuff now creates a platform device
which will bind with the CRTC part of Armada DRM, rather than the
master part and everything now breaks.

So, rather than having a component / DRM problem, it ends up becoming
a component / DRM / mvebu problem - rather than just two subsystems
needing coordination, we end up with /another/ subsystem to contend
with as well.

> > The other problem in this series is that while you introduce some
> > bindings which may work today, they're not going to work tomorrow, and
> > that's a problem.  Don't do DT piecemeal like this and end up having to
> > break the bindings (which we will have to do to add the endpoints.)
> 
> Seriously, we've got to do something to get off of the X.  Why not just
> declare the binding Sebastian just sent as unstable?  Then we change it
> as needed until we get all the pieces in place.

As I've already pointed out, why not add the complete binding right now
and be done with it?  That would sort out my concern about DT, and -
as I've already said (again) - we know exactly how this should look
because we have the of_graph stuff in drivers/of as standard helpers,
and we have imx-drm and v4l2 media stuff using it already.  It's
well established, and I've demonstrated that Armada DRM _will_ also
conform.

In fact, I have plans to consolidate the code between Armada DRM and
imx-drm into some standard DRM helpers (if you read through the patches
I linked to on cgit, you will notice that has already started.)

But... the DT issue is relatively minor compared to the one which I
raise above, which adds to the problems of moving forward.  It
effectively means I need to then carry a patch which undoes that,
and that patch would need to go through the DRM tree along with the
rest of the Armada DRM updates, which means more people need to be
involved, which then makes the process even slower than it already
is.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Jean-Francois Moine]

On Tue, 1 Jul 2014 17:45:27 +0100
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> Let's tell the full story rather than just presenting half of it.
> 
> You indeed wanted to do what you said above, but you also wanted to
> completely change the component helpers in a way that I was not happy
> with.  You wanted to add all sorts of DT specific gunk into the
> helpers, which would have tied it to DT.
> 
> While DT is the current thing on ARM, it is /not/ the current thing
> everywhere - a point which you failed to grasp.

I don't think that there will ever be a x86 DT.
Yes, I did not know what was the DT. For me, when entering in the ARM
world, it was a marvellous tool which could have ended in a unique ARM
kernel. I'm a dreamer!

> You wanted to make the component operations optional, which I pointed
> out makes no sense (because then components have no way to know what
> happens to their master device - which is /really/ important for DRM.)

You did not explain too much that you wanted to keep it for DRM only.

> You refused to listen to those concerns, and refused to look at the
> patch which I proposed, which did exactly the same as your patch,
> while keeping the DRM slave interfaces for tilcdc to use, until they
> have a chance to convert over.

The change in tilcdc was easy and the code was greatly simplified.

> You kept telling me that I had "opened the door" to your changes.  I
> claimed that your changes abused the code which I had written - a point
> which I still maintain to this day.

Your code was offering a simple way to synchronize the system
initialization without this lot of 'probe defer's. It could have been
used so.

> You also claimed that deferred probing didn't work.  Since the component
> helpers were designed with the deferred probing problem in mind at the
> time, and include the solution to that problem - which has been well
> tested hundreds if not thousands of times by now - and you did not
> provide the technical details as to why you thought it didn't work,
> there was nothing that could be done to progress that point.

AFAIR, you also said that the probe defer was not working. But, thanks
again for your component layer: all my init problems are gone, even if
there are still some prove defer's...

> Moreover, your abuse of the component layer would have made it more
> difficult to maintain it into the future - which is a fundamental
> point which has to be considered when accepting any patch into the
> kernel.  If a patch makes some code unable to be maintained, then an
> alternative approach has to be found.  Since you were not willing to
> compromise on finding or considering alternative approaches such as
> the one I presented.

I don't see what you are talking about.

> Since the component layer had had various comments which were in
> progress, and your abuse of the component layer would have also
> prevented those changes taking place (which are - in part - the set
> of component patches which are on the list now) there is no way that
> your uncompromising set of patches would be merged - at least not
> until you start accepting some of the comments being given to you.
> 
> > Now, the last thing for me is to put the TDA998x codec in the kernel
> > (it is also working in an other machine with 2 tda998x's).  
> 
> Yes, supporting the I2S connection is something that is need, but we
> /also/ need to support SPDIF, and SPDIF is the preferred method on
> the Cubox.  SPDIF should be used to talk to the TDA998x whenever
> possible because it opens up the possibility for sending out
> compressed MPEG2 and AC-3 audio streams, thus offloading the decode
> of these formats to external hardware.
> 
> This works today, and is a feature that people have been using with
> platforms such as xbmc and various other installations on the Cubox.
> 
> Limiting to I2S means that you can't send out these compressed audio
> streams.  In fact, the Dove manual tells you that you must disable
> the I2S playback stream if you're sending non-PCM - non-PCM is only
> supported via SPDIF.

I don't understand: both I2S and S/PDIF may work in the kirkwood audio
subsystem as it is (yes, actually not at the same time, and this is a
choice because DPCM does not work for the Cubox). I have the 3 ways in
my system:

$ cat /proc/asound/pcm 
00-00: i2s-i2s-hifi i2s-hifi-0 :  : playback 1
00-01: spdif-spdif-hifi spdif-hifi-1 :  : playback 1
00-02: spdif-dit-hifi dit-hifi-2 :  : playback 1

So, S/PDIF should work, but I don't know it: I have no optical connector.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

[PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Jean-Francois Moine]

On Tue, 1 Jul 2014 17:45:27 +0100
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> Let's tell the full story rather than just presenting half of it.
> 
> You indeed wanted to do what you said above, but you also wanted to
> completely change the component helpers in a way that I was not happy
> with.  You wanted to add all sorts of DT specific gunk into the
> helpers, which would have tied it to DT.
> 
> While DT is the current thing on ARM, it is /not/ the current thing
> everywhere - a point which you failed to grasp.

I don't think that there will ever be a x86 DT.
Yes, I did not know what was the DT. For me, when entering in the ARM
world, it was a marvellous tool which could have ended in a unique ARM
kernel. I'm a dreamer!

> You wanted to make the component operations optional, which I pointed
> out makes no sense (because then components have no way to know what
> happens to their master device - which is /really/ important for DRM.)

You did not explain too much that you wanted to keep it for DRM only.

> You refused to listen to those concerns, and refused to look at the
> patch which I proposed, which did exactly the same as your patch,
> while keeping the DRM slave interfaces for tilcdc to use, until they
> have a chance to convert over.

The change in tilcdc was easy and the code was greatly simplified.

> You kept telling me that I had "opened the door" to your changes.  I
> claimed that your changes abused the code which I had written - a point
> which I still maintain to this day.

Your code was offering a simple way to synchronize the system
initialization without this lot of 'probe defer's. It could have been
used so.

> You also claimed that deferred probing didn't work.  Since the component
> helpers were designed with the deferred probing problem in mind at the
> time, and include the solution to that problem - which has been well
> tested hundreds if not thousands of times by now - and you did not
> provide the technical details as to why you thought it didn't work,
> there was nothing that could be done to progress that point.

AFAIR, you also said that the probe defer was not working. But, thanks
again for your component layer: all my init problems are gone, even if
there are still some prove defer's...

> Moreover, your abuse of the component layer would have made it more
> difficult to maintain it into the future - which is a fundamental
> point which has to be considered when accepting any patch into the
> kernel.  If a patch makes some code unable to be maintained, then an
> alternative approach has to be found.  Since you were not willing to
> compromise on finding or considering alternative approaches such as
> the one I presented.

I don't see what you are talking about.

> Since the component layer had had various comments which were in
> progress, and your abuse of the component layer would have also
> prevented those changes taking place (which are - in part - the set
> of component patches which are on the list now) there is no way that
> your uncompromising set of patches would be merged - at least not
> until you start accepting some of the comments being given to you.
> 
> > Now, the last thing for me is to put the TDA998x codec in the kernel
> > (it is also working in an other machine with 2 tda998x's).  
> 
> Yes, supporting the I2S connection is something that is need, but we
> /also/ need to support SPDIF, and SPDIF is the preferred method on
> the Cubox.  SPDIF should be used to talk to the TDA998x whenever
> possible because it opens up the possibility for sending out
> compressed MPEG2 and AC-3 audio streams, thus offloading the decode
> of these formats to external hardware.
> 
> This works today, and is a feature that people have been using with
> platforms such as xbmc and various other installations on the Cubox.
> 
> Limiting to I2S means that you can't send out these compressed audio
> streams.  In fact, the Dove manual tells you that you must disable
> the I2S playback stream if you're sending non-PCM - non-PCM is only
> supported via SPDIF.

I don't understand: both I2S and S/PDIF may work in the kirkwood audio
subsystem as it is (yes, actually not at the same time, and this is a
choice because DPCM does not work for the Cubox). I have the 3 ways in
my system:

$ cat /proc/asound/pcm 
00-00: i2s-i2s-hifi i2s-hifi-0 :  : playback 1
00-01: spdif-spdif-hifi spdif-hifi-1 :  : playback 1
00-02: spdif-dit-hifi dit-hifi-2 :  : playback 1

So, S/PDIF should work, but I don't know it: I have no optical connector.

-- 
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

Re: [PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Russell King - ARM Linux]

On Tue, Jul 01, 2014 at 07:49:53PM +0200, Jean-Francois Moine wrote:
> On Tue, 1 Jul 2014 17:45:27 +0100
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> > Let's tell the full story rather than just presenting half of it.
> > 
> > You indeed wanted to do what you said above, but you also wanted to
> > completely change the component helpers in a way that I was not happy
> > with.  You wanted to add all sorts of DT specific gunk into the
> > helpers, which would have tied it to DT.
> > 
> > While DT is the current thing on ARM, it is /not/ the current thing
> > everywhere - a point which you failed to grasp.
> 
> I don't think that there will ever be a x86 DT.

x86 is ACPI, which is a completely different firmware interface.
Now, we are starting to see ARM peripherals (even some rather old
ones) being used on some x86 platforms.

You can't assume that just because something is used on ARM, it's never
going to be used on any other architecture.  IP which gets used with one
host CPU, even where it's integrated into a SoC, can find its way to
other CPU architectures very easily, and this really does happen.

So don't be surprised one day to find (eg) that the Dove/Kirkwood I2S
audio interface ends up being connected to an x86 CPU at some point.
Or the TDA998x ends up being used on x86 in a set top box.

You really can't think that just because something seems to be ARM
specific that it's absolutely fine to assume that it will always be
specific to ARM - /especially/ when adding core kernel facilities.
Core kernel facilities have to be completely independent of any CPU
architecture.  If they aren't, by definition they aren't core kernel
facilities.

> > You wanted to make the component operations optional, which I pointed
> > out makes no sense (because then components have no way to know what
> > happens to their master device - which is /really/ important for DRM.)
> 
> You did not explain too much that you wanted to keep it for DRM only.

It /isn't/ just for DRM, I've no idea what gave you that idea.  I've
said that I want to keep DT out of the core component helpers (see above
why.)  I also want to keep subsystem specifics out of it as well.

> > You refused to listen to those concerns, and refused to look at the
> > patch which I proposed, which did exactly the same as your patch,
> > while keeping the DRM slave interfaces for tilcdc to use, until they
> > have a chance to convert over.
> 
> The change in tilcdc was easy and the code was greatly simplified.

The same applies to mine once tilcdc is converted to the component
helpers - provided there are no other users of TDA998x, then the
non-component code can all be dropped and the driver reduced down -
and at that point it becomes purely a drm encoder with an associated
drm connector with just one setup methodology.

> > You kept telling me that I had "opened the door" to your changes.  I
> > claimed that your changes abused the code which I had written - a point
> > which I still maintain to this day.
> 
> Your code was offering a simple way to synchronize the system
> initialization without this lot of 'probe defer's. It could have been
> used so.

My code does /today/ offer a simple way to sort out the initialisation
without lots of deferred probes.  This is what happens:

1. There are zero deferred probes until all components for the master
   are present.

2. You will only get a deferred probe once all components are present
   _unless_ the master bind function returns a deferred probe, which
   _may_ come from a component driver.

So, the deferred probes are reduced to an absolute minimum.  Again, this
is by intention and design.

When resources are not available, we have to rely on the deferred probe
mechanism, because the deferred probe mechanism knows when other non-
component drivers are probed - and thus when resources may become
available.  This is not really any different from a single driver
trying to claim resources, finding one that isn't available, and
returning -EPROBE_DEFER - in fact, only the last driver to be probed
in a set of components which satisfies the master will suffer deferred
probes.

I'm now pushing changes to the component helper which change the way
the matching works, so that master doesn't have to keep scanning
whatever it needs to in order to work out which components are
required.  Masters will be required to gather that information
before registering themselves and supply it when registering into the
component helper.  This is beneficial for two reasons - not only does
it avoid wasting time rescanning (eg) the device tree multiple times,
but it also means that we can eventually allow partial subsystem
bring-up which is something the v4l2 people would like.

So, there's quite some work planned here which will require a number of
changes to the visible API.  So, it's really important that all users
make use of the component layer as it is intended to be used, so that
changes can be made.

Right now, that hasn't happened with Exynos, and that is right now giving
problems pushing this work forward - and that's the /exact/ problem when
stuff starts getting abused.  It means that it can't be maintained and it
blocks other work.  As the person who created this problem isn't
responding to my emails, it's pretty hard to see how to retire the old
interfaces - except maybe by forcing the issue which I really don't want
to do.

> > You also claimed that deferred probing didn't work.  Since the component
> > helpers were designed with the deferred probing problem in mind at the
> > time, and include the solution to that problem - which has been well
> > tested hundreds if not thousands of times by now - and you did not
> > provide the technical details as to why you thought it didn't work,
> > there was nothing that could be done to progress that point.
> 
> AFAIR, you also said that the probe defer was not working. But, thanks
> again for your component layer: all my init problems are gone, even if
> there are still some prove defer's...

That's really strange, because imx-drm relies upon it since day one,
and I even sent you the kernel boot log which provided evidence of it
working correctly.  Yes, there have been a few corner cases where the
devm resources were not handled quite correctly, but those have been
corrected.

> > Moreover, your abuse of the component layer would have made it more
> > difficult to maintain it into the future - which is a fundamental
> > point which has to be considered when accepting any patch into the
> > kernel.  If a patch makes some code unable to be maintained, then an
> > alternative approach has to be found.  Since you were not willing to
> > compromise on finding or considering alternative approaches such as
> > the one I presented.
> 
> I don't see what you are talking about.

See my explanation of the upcoming changes above - some of those
changes are required for Armada DRM (required in the sense that I
want Armada DRM to use the new way right now rather than have to
rework yet another DRM driver for those changes, pushing the
component changes even later - allowing more users to come along
potentially blocking the work.)

> > Since the component layer had had various comments which were in
> > progress, and your abuse of the component layer would have also
> > prevented those changes taking place (which are - in part - the set
> > of component patches which are on the list now) there is no way that
> > your uncompromising set of patches would be merged - at least not
> > until you start accepting some of the comments being given to you.
> > 
> > > Now, the last thing for me is to put the TDA998x codec in the kernel
> > > (it is also working in an other machine with 2 tda998x's).  
> > 
> > Yes, supporting the I2S connection is something that is need, but we
> > /also/ need to support SPDIF, and SPDIF is the preferred method on
> > the Cubox.  SPDIF should be used to talk to the TDA998x whenever
> > possible because it opens up the possibility for sending out
> > compressed MPEG2 and AC-3 audio streams, thus offloading the decode
> > of these formats to external hardware.
> > 
> > This works today, and is a feature that people have been using with
> > platforms such as xbmc and various other installations on the Cubox.
> > 
> > Limiting to I2S means that you can't send out these compressed audio
> > streams.  In fact, the Dove manual tells you that you must disable
> > the I2S playback stream if you're sending non-PCM - non-PCM is only
> > supported via SPDIF.
> 
> I don't understand: both I2S and S/PDIF may work in the kirkwood audio
> subsystem as it is (yes, actually not at the same time, and this is a
> choice because DPCM does not work for the Cubox). I have the 3 ways in
> my system:
> 
> $ cat /proc/asound/pcm 
> 00-00: i2s-i2s-hifi i2s-hifi-0 :  : playback 1
> 00-01: spdif-spdif-hifi spdif-hifi-1 :  : playback 1
> 00-02: spdif-dit-hifi dit-hifi-2 :  : playback 1
> 
> So, S/PDIF should work, but I don't know it: I have no optical connector.

How does that even work - I mean, pulseaudio will try and open all three
at startup and it sends samples through each...  Have you tested this
with pulseaudio?  Have you tested it with other programs such as an
audio player going direct to the ALSA interfaces (as would happen with
a compressed audio stream) - and checked what happens if pulse wants to
use it?  You don't need an actual compressed audio stream to test that
- you can just send audio direct to one of your other devices.

Second point - if you have the TDA998x only using i2s, then (as you say)
you can't use spdif, so simultaneous output via both HDMI and optical
is impossible with your driver on the Cubox.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH 4/4] ARM: mvebu: add armada drm init to Dove board setup

[Russell King - ARM Linux]

On Tue, Jul 01, 2014 at 07:49:53PM +0200, Jean-Francois Moine wrote:
> On Tue, 1 Jul 2014 17:45:27 +0100
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> > Let's tell the full story rather than just presenting half of it.
> > 
> > You indeed wanted to do what you said above, but you also wanted to
> > completely change the component helpers in a way that I was not happy
> > with.  You wanted to add all sorts of DT specific gunk into the
> > helpers, which would have tied it to DT.
> > 
> > While DT is the current thing on ARM, it is /not/ the current thing
> > everywhere - a point which you failed to grasp.
> 
> I don't think that there will ever be a x86 DT.

x86 is ACPI, which is a completely different firmware interface.
Now, we are starting to see ARM peripherals (even some rather old
ones) being used on some x86 platforms.

You can't assume that just because something is used on ARM, it's never
going to be used on any other architecture.  IP which gets used with one
host CPU, even where it's integrated into a SoC, can find its way to
other CPU architectures very easily, and this really does happen.

So don't be surprised one day to find (eg) that the Dove/Kirkwood I2S
audio interface ends up being connected to an x86 CPU at some point.
Or the TDA998x ends up being used on x86 in a set top box.

You really can't think that just because something seems to be ARM
specific that it's absolutely fine to assume that it will always be
specific to ARM - /especially/ when adding core kernel facilities.
Core kernel facilities have to be completely independent of any CPU
architecture.  If they aren't, by definition they aren't core kernel
facilities.

> > You wanted to make the component operations optional, which I pointed
> > out makes no sense (because then components have no way to know what
> > happens to their master device - which is /really/ important for DRM.)
> 
> You did not explain too much that you wanted to keep it for DRM only.

It /isn't/ just for DRM, I've no idea what gave you that idea.  I've
said that I want to keep DT out of the core component helpers (see above
why.)  I also want to keep subsystem specifics out of it as well.

> > You refused to listen to those concerns, and refused to look at the
> > patch which I proposed, which did exactly the same as your patch,
> > while keeping the DRM slave interfaces for tilcdc to use, until they
> > have a chance to convert over.
> 
> The change in tilcdc was easy and the code was greatly simplified.

The same applies to mine once tilcdc is converted to the component
helpers - provided there are no other users of TDA998x, then the
non-component code can all be dropped and the driver reduced down -
and at that point it becomes purely a drm encoder with an associated
drm connector with just one setup methodology.

> > You kept telling me that I had "opened the door" to your changes.  I
> > claimed that your changes abused the code which I had written - a point
> > which I still maintain to this day.
> 
> Your code was offering a simple way to synchronize the system
> initialization without this lot of 'probe defer's. It could have been
> used so.

My code does /today/ offer a simple way to sort out the initialisation
without lots of deferred probes.  This is what happens:

1. There are zero deferred probes until all components for the master
   are present.

2. You will only get a deferred probe once all components are present
   _unless_ the master bind function returns a deferred probe, which
   _may_ come from a component driver.

So, the deferred probes are reduced to an absolute minimum.  Again, this
is by intention and design.

When resources are not available, we have to rely on the deferred probe
mechanism, because the deferred probe mechanism knows when other non-
component drivers are probed - and thus when resources may become
available.  This is not really any different from a single driver
trying to claim resources, finding one that isn't available, and
returning -EPROBE_DEFER - in fact, only the last driver to be probed
in a set of components which satisfies the master will suffer deferred
probes.

I'm now pushing changes to the component helper which change the way
the matching works, so that master doesn't have to keep scanning
whatever it needs to in order to work out which components are
required.  Masters will be required to gather that information
before registering themselves and supply it when registering into the
component helper.  This is beneficial for two reasons - not only does
it avoid wasting time rescanning (eg) the device tree multiple times,
but it also means that we can eventually allow partial subsystem
bring-up which is something the v4l2 people would like.

So, there's quite some work planned here which will require a number of
changes to the visible API.  So, it's really important that all users
make use of the component layer as it is intended to be used, so that
changes can be made.

Right now, that hasn't happened with Exynos, and that is right now giving
problems pushing this work forward - and that's the /exact/ problem when
stuff starts getting abused.  It means that it can't be maintained and it
blocks other work.  As the person who created this problem isn't
responding to my emails, it's pretty hard to see how to retire the old
interfaces - except maybe by forcing the issue which I really don't want
to do.

> > You also claimed that deferred probing didn't work.  Since the component
> > helpers were designed with the deferred probing problem in mind at the
> > time, and include the solution to that problem - which has been well
> > tested hundreds if not thousands of times by now - and you did not
> > provide the technical details as to why you thought it didn't work,
> > there was nothing that could be done to progress that point.
> 
> AFAIR, you also said that the probe defer was not working. But, thanks
> again for your component layer: all my init problems are gone, even if
> there are still some prove defer's...

That's really strange, because imx-drm relies upon it since day one,
and I even sent you the kernel boot log which provided evidence of it
working correctly.  Yes, there have been a few corner cases where the
devm resources were not handled quite correctly, but those have been
corrected.

> > Moreover, your abuse of the component layer would have made it more
> > difficult to maintain it into the future - which is a fundamental
> > point which has to be considered when accepting any patch into the
> > kernel.  If a patch makes some code unable to be maintained, then an
> > alternative approach has to be found.  Since you were not willing to
> > compromise on finding or considering alternative approaches such as
> > the one I presented.
> 
> I don't see what you are talking about.

See my explanation of the upcoming changes above - some of those
changes are required for Armada DRM (required in the sense that I
want Armada DRM to use the new way right now rather than have to
rework yet another DRM driver for those changes, pushing the
component changes even later - allowing more users to come along
potentially blocking the work.)

> > Since the component layer had had various comments which were in
> > progress, and your abuse of the component layer would have also
> > prevented those changes taking place (which are - in part - the set
> > of component patches which are on the list now) there is no way that
> > your uncompromising set of patches would be merged - at least not
> > until you start accepting some of the comments being given to you.
> > 
> > > Now, the last thing for me is to put the TDA998x codec in the kernel
> > > (it is also working in an other machine with 2 tda998x's).  
> > 
> > Yes, supporting the I2S connection is something that is need, but we
> > /also/ need to support SPDIF, and SPDIF is the preferred method on
> > the Cubox.  SPDIF should be used to talk to the TDA998x whenever
> > possible because it opens up the possibility for sending out
> > compressed MPEG2 and AC-3 audio streams, thus offloading the decode
> > of these formats to external hardware.
> > 
> > This works today, and is a feature that people have been using with
> > platforms such as xbmc and various other installations on the Cubox.
> > 
> > Limiting to I2S means that you can't send out these compressed audio
> > streams.  In fact, the Dove manual tells you that you must disable
> > the I2S playback stream if you're sending non-PCM - non-PCM is only
> > supported via SPDIF.
> 
> I don't understand: both I2S and S/PDIF may work in the kirkwood audio
> subsystem as it is (yes, actually not at the same time, and this is a
> choice because DPCM does not work for the Cubox). I have the 3 ways in
> my system:
> 
> $ cat /proc/asound/pcm 
> 00-00: i2s-i2s-hifi i2s-hifi-0 :  : playback 1
> 00-01: spdif-spdif-hifi spdif-hifi-1 :  : playback 1
> 00-02: spdif-dit-hifi dit-hifi-2 :  : playback 1
> 
> So, S/PDIF should work, but I don't know it: I have no optical connector.

How does that even work - I mean, pulseaudio will try and open all three
at startup and it sends samples through each...  Have you tested this
with pulseaudio?  Have you tested it with other programs such as an
audio player going direct to the ALSA interfaces (as would happen with
a compressed audio stream) - and checked what happens if pulse wants to
use it?  You don't need an actual compressed audio stream to test that
- you can just send audio direct to one of your other devices.

Second point - if you have the TDA998x only using i2s, then (as you say)
you can't use spdif, so simultaneous output via both HDMI and optical
is impossible with your driver on the Cubox.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.