ARM: vexpress: initial device tree support

This patch implements initial support for booting using a flattened device tree on the Versatile Express platform.

Eventually, it should be possible to present a single, core-tile- independent board, but in this transitional patch the baseboard + Cortex-A9x4 core tile combination is the only directly supported platform, since the implementation is not yet fully generic.

For now, clocks and timers are not handled via the device tree. Implementation of these can follow in later patches.

Thanks to Lorenzo Pieralisi, Grant Likely and Pawe? Moll for their help and contributions.

Signed-off-by Dave Martin <[email protected]>

Acked-by Pawe? Moll <[email protected]>

---

There are some outstanding issues which need to be discussed, listed below.

* This patch is not currently based on the GIC bindings being discussed by Rob Herring et al. Once that discussion reaches a conclusion, it should be straightforward to rebase onto the result.

* The following added bindings are not present upstream and need documentation / discussion:

* arm,vexpress -- the global board binding for all platform combinations using the Versatile Express motherboard.

* arm,vexpress-v2p-ca9 -- the specific binding for the Versatile Express motherboard with Cortex-A9x4 core tile installed. It is only mentioned as the most-specific match in vexpress-v2p- ca9.dts

Since it's intended that the motherboard code should be fully generic, and because no other core tiles are upstream yet, perhaps we can get rid of this binding right away.

* edid -- It should be possible to have a fairly generic binding for EDID interfaces, but none seems to exist yet. Discussion is needed regarding what form this should take.

This might more appropriately be called "ddc" (or some variation on that), since EDID seems only to describe the format of the ID data retrievable via this interface; not the interface itself.

* arm,vexpress-flash -- Needed because of the requirement to provide the physmap_flash driver with a special .set_vpp handler.

* idt,89hpes32h8 -- This is the IDT 89HPES32H8 PCI express interconnect switch. This isn't needed for the Versatile Express to work, but would be needed if using PCI-e peripherals for real. I expect that more driver support needs to go upstream before this is actually usable.

* nxp,isp1761 -- The driver support for this is already upstream (with some minor issues for ARM support).

* arm,amba-bus -- widely used by other boards and patchsets, but seems not to be documented.

* The following bindings for ARM primecell peripherals are used elsewhere but not documented. They should be pretty simple and uncontraversial.
* arm,pl031
* arm,pl041
* arm,pl050
* arm,pl180
* arm,sp805

Rob Herring suggested documenting simple bindings for these (and others) along with his initial amba device tree probe patches, but these bindings don't seem to be documented upstream for now.

* Shawn Guo's smsc911x patch is needed for Ethernet to work. This is headed upstream but not yet in mainline. It is available in -next.

* Minor patches are needed to the isp1760 and pata_generic drivers, to allow OF-based initialisation across a wider group of architectures. These are being discussed independently, but are not yet accepted for merging upstream.

* Most core-tile peripherals are currently not described in the core- tile device tree fragment. This is a lower-priority issue since the motherboard code already autodetects the core-tile (though only one core-tile is fully upstream at the moment).

* Static peripheral mappings are not yet handled in a generic way in the board support code. This is a prerequisite for supporting multiple core-tiles int the same kernel. It well need to get fixed later, when extra core tile support is merged (or before).

Pawe? Moll is looking into this separately.

* The Kconfig logic for ensuring that at least one boot protocol and at least one core tile are selected is a bit ugly. Suggestions for improving this are certainly welcome.

 arch/arm/Kconfig                           |    1 +
 arch/arm/boot/dts/vexpress-v2m-legacy.dtsi |  163 ++++++++++++++++++++++++++++
 arch/arm/boot/dts/vexpress-v2p-ca9.dts     |   80 ++++++++++++++
 arch/arm/configs/vexpress_defconfig        |    1 +
 arch/arm/mach-vexpress/Kconfig             |   45 ++++++++-
 arch/arm/mach-vexpress/ct-ca9x4.c          |    7 ++
 arch/arm/mach-vexpress/v2m.c               |   54 +++++++++-
 7 files changed, 349 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/boot/dts/vexpress-v2m-legacy.dtsi
 create mode 100644 arch/arm/boot/dts/vexpress-v2p-ca9.dts

arch/arm/Kconfig +1 -0

index 5ebc5d9..a6e90d5 100644
@@ -282,6 +282,7 @@ config ARCH_VERSATILE
 
 config ARCH_VEXPRESS
 	bool "ARM Ltd. Versatile Express family"
+	select ARCH_VEXPRESS_SANE_CONFIG
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select ARM_AMBA
 	select ARM_TIMER_SP804

arch/arm/boot/dts/vexpress-v2m-legacy.dtsi new file +0 -0

new file mode 100644
index 0000000..fd6e4e4

arch/arm/boot/dts/vexpress-v2m-legacy.dtsi new file +113 -0

+
+/ {
+	aliases {
+		serial0 = &uart0;
+		serial1 = &uart1;
+		serial2 = &uart2;
+		serial3 = &uart3;
+		i2c0 = &i2c0;
+		i2c1 = &i2c1;
+	};
+
+	motherboard {
+		compatible = "simple-bus";
+		#address-cells = <2>; // SMB chipselect number and offset
+		#size-cells = <1>;
+		#interrupt-cells = <1>;
+
+		flash at 0,00000000 {
+			compatible = "arm,vexpress-flash", "cfi-flash";
+			reg = <0 0x00000000 0x04000000
+			       1 0x00000000 0x04000000>;
+			bank-width = <4>;
+		};
+
+		psram at 2,00000000 {
+			compatible = "mtd-ram";
+			reg = <2 0x00000000 0x02000000>;
+			bank-width = <4>;
+		};
+
+		ethernet at 3,02000000 {
+			compatible = "smsc,lan9118", "smsc,lan9115";
+			reg = <3 0x02000000 0x10000>;
+			reg-io-width = <4>;
+			interrupts = <15>;
+			smsc,irq-active-high;
+			smsc,irq-push-pull;
+		};
+
+		usb at 3,03000000 {
+			compatible = "nxp,usb-isp1761";
+			reg = <3 0x03000000 0x20000>;
+			interrupts = <16>;
+			port1-otg;
+		};
+
+		peripherals at 7,00000000 {
+			compatible = "arm,amba-bus", "simple-bus";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges = <0 7 0 0x20000>;
+
+			// PCI-E I2C bus
+			i2c0: i2c at 02000 {
+				compatible = "arm,versatile-i2c";
+				reg = <0x02000 0x1000>;
+
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				pcie-switch at 60 {
+					compatible = "idt,89hpes32h8";
+					reg = <0x60>;
+				};
+			};
+
+			aaci at 04000 {
+				compatible = "arm,pl041", "arm,primecell";
+				reg = <0x04000 0x1000>;
+				interrupts = <11>;
+			};
+
+			mmci at 05000 {
+				compatible = "arm,pl180", "arm,primecell";
+				reg = <0x05000 0x1000>;
+				interrupts = <9 10>;
+			};
+
+			kmi at 06000 {
+				compatible = "arm,pl050", "arm,primecell";
+				reg = <0x06000 0x1000>;
+				interrupts = <12>;
+			};
+
+			kmi at 07000 {
+				compatible = "arm,pl050", "arm,primecell";
+				reg = <0x07000 0x1000>;
+				interrupts = <13>;
+			};
+
+			uart0: uart at 09000 {
+				compatible = "arm,pl011", "arm,primecell";
+				reg = <0x09000 0x1000>;
+				interrupts = <5>;
+			};
+
+			uart1: uart at 0a000 {
+				compatible = "arm,pl011", "arm,primecell";
+				reg = <0x0a000 0x1000>;
+				interrupts = <6>;
+			};
+
+			uart2: uart at 0b000 {
+				compatible = "arm,pl011", "arm,primecell";
+				reg = <0x0b000 0x1000>;
+				interrupts = <7>;
+			};
+
+			uart3: uart at 0c000 {
+				compatible = "arm,pl011", "arm,primecell";
+				reg = <0x0c000 0x1000>;
+				interrupts = <8>;
+			};

arch/arm/boot/dts/vexpress-v2m-legacy.dtsi new file +30 -0

+
+			// DVI I2C bus (DDC)
+			i2c1: i2c at 16000 {
+				compatible = "arm,versatile-i2c";
+				reg = <0x16000 0x1000>;
+
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				edid at 50 {
+					compatible = "edid";
+					reg = <0x50>;
+				};
+			};
+
+			rtc at 17000 {
+				compatible = "arm,pl031", "arm,primecell";
+				reg = <0x017000 0x1000>;
+				interrupts = <4>;
+			};
+
+			compact-flash at 1a000 {
+				compatible = "ata-generic";
+				reg = <0x1a000 0x100
+				       0x1a100 0xf00>;
+				reg-shift = <2>;
+			};
+		};
+	};
+};

arch/arm/boot/dts/vexpress-v2p-ca9.dts new file +14 -0

new file mode 100644
index 0000000..059be97
@@ -0,0 +1,80 @@
+// ARM Ltd. Versatile Express Corex-A9 (Quad Core) Core Tile V2P-CA9 (HBI-0191B)
+
+/dts-v1/;
+
+/include/ "skeleton.dtsi"
+
+/ {
+	model = "ARM Versatile Express (Cortex-A9 Quad Core Tile)";
+	compatible = "arm,vexpress-v2p-ca9", "arm,vexpress";
+	interrupt-parent = <&intc>;
+
+	memory {
+		device_type = "memory";
+		reg = <0x60000000 0x40000000>;

arch/arm/boot/dts/vexpress-v2p-ca9.dts new file +56 -0

+
+	motherboard {
+		ranges = <0 0 0x40000000 0x04000000
+			  1 0 0x44000000 0x04000000
+			  2 0 0x48000000 0x04000000
+			  3 0 0x4c000000 0x04000000
+			  7 0 0x10000000 0x00020000>;
+
+		interrupt-map-mask = <0 0 63>;
+		interrupt-map = <0 0 0 &intc 32 8
+				 0 0 1 &intc 33 4
+				 0 0 2 &intc 34 4
+				 0 0 3 &intc 35 4
+				 0 0 4 &intc 36 4
+				 0 0 5 &intc 37 4
+				 0 0 6 &intc 38 4
+				 0 0 7 &intc 39 4
+				 0 0 8 &intc 40 4
+				 0 0 9 &intc 41 4
+				 0 0 10 &intc 42 4
+				 0 0 11 &intc 43 4
+				 0 0 12 &intc 44 4
+				 0 0 13 &intc 45 4
+				 0 0 14 &intc 46 4
+				 0 0 15 &intc 47 4
+				 0 0 16 &intc 48 4
+				 0 0 17 &intc 49 4
+				 0 0 18 &intc 50 4
+				 0 0 19 &intc 51 4
+				 0 0 20 &intc 52 4
+				 0 0 21 &intc 53 4
+				 0 0 22 &intc 54 4
+				 0 0 23 &intc 55 4
+				 0 0 24 &intc 56 4
+				 0 0 25 &intc 57 4
+				 0 0 26 &intc 58 4
+				 0 0 27 &intc 59 4
+				 0 0 28 &intc 60 4
+				 0 0 29 &intc 61 4
+				 0 0 30 &intc 62 4
+				 0 0 31 &intc 63 4
+				 0 0 32 &intc 64 4
+				 0 0 33 &intc 65 4
+				 0 0 34 &intc 66 4
+				 0 0 35 &intc 67 4
+				 0 0 36 &intc 68 4
+				 0 0 37 &intc 69 4
+				 0 0 38 &intc 70 4
+				 0 0 39 &intc 71 4
+				 0 0 40 &intc 72 4
+				 0 0 41 &intc 73 4
+				 0 0 42 &intc 74 4>;
+	};
+};
+
+/include/ "vexpress-v2m-legacy.dtsi"

arch/arm/configs/vexpress_defconfig +1 -0

index f2de51f..6c3c5f6 100644
@@ -22,6 +22,7 @@ CONFIG_MODULE_UNLOAD=y
 # CONFIG_IOSCHED_DEADLINE is not set
 # CONFIG_IOSCHED_CFQ is not set
 CONFIG_ARCH_VEXPRESS=y
+CONFIG_ARCH_VEXPRESS_ATAGS=y
 CONFIG_ARCH_VEXPRESS_CA9X4=y
 # CONFIG_SWP_EMULATE is not set
 CONFIG_SMP=y

arch/arm/mach-vexpress/Kconfig +44 -1

index 9311484..ea64630 100644
@@ -1,12 +1,55 @@
 menu "Versatile Express platform type"
 	depends on ARCH_VEXPRESS
 
+# ARCH_VEXPRESS ensures a sane minimal config is selected by selecting
+# ARCH_VEXPRESS_SANE_CONFIG.
+# Extend the logic here when adding new core tiles.
+
+config ARCH_VEXPRESS_SANE_CONFIG
+	bool
+	select ARCH_VEXPRESS_CA9X4
+	select ARCH_VEXPRESS_ATAGS if !ARCH_VEXPRESS_DT
+
+
+comment "At least one boot type must be selected"
+
+config ARCH_VEXPRESS_ATAGS
+	bool "Boot via ATAGs"
+	default y
+	help
+	  This option enables support for the board using the standard
+	  ATAGs boot protocol.
+
+	  If your bootloader supports FDT-based booting and you do not
+	  intend ever to boot via the traditional ATAGs method, you can say
+	  N here.
+
+config ARCH_VEXPRESS_DT
+	bool "Boot via Device Tree"
+	select USE_OF
+	help
+	  This option enables support for the board, and enables booting
+	  via a Flattened Device Tree provided by the bootloader.
+
+	  If your bootloader supports FDT-based booting, you can say Y
+	  here, otherwise, say N.
+
+
+# Core Tile support options
+
+comment "At least one core tile must be selected"
+
 config ARCH_VEXPRESS_CA9X4
-	bool "Versatile Express Cortex-A9x4 tile"
+	bool "Versatile Express Cortex-A9x4 Core Tile"
+	default y
 	select CPU_V7
 	select ARM_GIC
 	select ARM_ERRATA_720789
 	select ARM_ERRATA_751472
 	select ARM_ERRATA_753970
+	help
+	  Include support for the Cortex-A9x4 Core Tile (HBI-0191B).
+
+	  If unsure, say Y.
 
 endmenu

arch/arm/mach-vexpress/ct-ca9x4.c +7 -0

index bfd32f5..e2fe2c9 100644
@@ -9,6 +9,7 @@
 #include <linux/amba/bus.h>
 #include <linux/amba/clcd.h>
 #include <linux/clkdev.h>
+#include <linux/irqdomain.h>
 
 #include <asm/hardware/arm_timer.h>
 #include <asm/hardware/cache-l2x0.h>

@@ -59,10 +60,16 @@ static void __init ct_ca9x4_map_io(void)
 	iotable_init(ct_ca9x4_io_desc, ARRAY_SIZE(ct_ca9x4_io_desc));
 }
 
+static const struct of_device_id gic_of_match[] __initconst = {
+	{ .compatible = "arm,cortex-a9-gic", },
+	{}
+};
+
 static void __init ct_ca9x4_init_irq(void)
 {
 	gic_init(0, 29, MMIO_P2V(A9_MPCORE_GIC_DIST),
 		 MMIO_P2V(A9_MPCORE_GIC_CPU));
+	irq_domain_generate_simple(gic_of_match, A9_MPCORE_GIC_DIST, 0);
 }
 
 #if 0

arch/arm/mach-vexpress/v2m.c +43 -1

index 9e6b93b..6defce6 100644
@@ -6,6 +6,8 @@
 #include <linux/amba/mmci.h>
 #include <linux/io.h>
 #include <linux/init.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/ata_platform.h>
 #include <linux/smsc911x.h>

@@ -118,7 +120,7 @@ int v2m_cfg_read(u32 devfn, u32 *data)
 	return !!(val & SYS_CFG_ERR);
 }
 
-
+#ifdef CONFIG_ARCH_VEXPRESS_ATAGS
 static struct resource v2m_pcie_i2c_resource = {
 	.start	= V2M_SERIAL_BUS_PCI,
 	.end	= V2M_SERIAL_BUS_PCI + SZ_4K - 1,

@@ -200,6 +202,7 @@ static struct platform_device v2m_usb_device = {
 	.num_resources	= ARRAY_SIZE(v2m_usb_resources),
 	.dev.platform_data = &v2m_usb_config,
 };
+#endif /* CONFIG_ARCH_VEXPRESS_ATAGS */
 
 static void v2m_flash_set_vpp(struct platform_device *pdev, int on)
 {

@@ -211,6 +214,7 @@ static struct physmap_flash_data v2m_flash_data = {
 	.set_vpp	= v2m_flash_set_vpp,
 };
 
+#ifdef CONFIG_ARCH_VEXPRESS_ATAGS
 static struct resource v2m_flash_resources[] = {
 	{
 		.start	= V2M_NOR0,

@@ -254,6 +258,7 @@ static struct platform_device v2m_cf_device = {
 	.num_resources	= ARRAY_SIZE(v2m_pata_resources),
 	.dev.platform_data = &v2m_pata_data,
 };
+#endif /* CONFIG_ARCH_VEXPRESS_ATAGS */
 
 static unsigned int v2m_mmci_status(struct device *dev)
 {

@@ -265,6 +270,7 @@ static struct mmci_platform_data v2m_mmci_data = {
 	.status		= v2m_mmci_status,
 };
 
+#ifdef CONFIG_ARCH_VEXPRESS_ATAGS
 static AMBA_DEVICE(aaci,  "mb:aaci",  V2M_AACI, NULL);
 static AMBA_DEVICE(mmci,  "mb:mmci",  V2M_MMCI, &v2m_mmci_data);
 static AMBA_DEVICE(kmi0,  "mb:kmi0",  V2M_KMI0, NULL);

@@ -288,6 +294,7 @@ static struct amba_device *v2m_amba_devs[] __initdata = {
 	&wdt_device,
 	&rtc_device,
 };
+#endif /* CONFIG_ARCH_VEXPRESS_ATAGS */
 
 
 static long v2m_osc_round(struct clk *clk, unsigned long rate)

@@ -415,6 +422,8 @@ static void __init v2m_init_irq(void)
 	ct_desc->init_irq();
 }
 
+
+#ifdef CONFIG_ARCH_VEXPRESS_ATAGS
 static void __init v2m_init(void)
 {
 	int i;

@@ -443,3 +452,46 @@ MACHINE_START(VEXPRESS, "ARM-Versatile Express")
 	.timer		= &v2m_timer,
 	.init_machine	= v2m_init,
 MACHINE_END
+#endif /* CONFIG_ARCH_VEXPRESS_ATAGS */
+
+#ifdef CONFIG_ARCH_VEXPRESS_DT
+struct of_dev_auxdata v2m_dt_auxdata_lookup[] __initdata = {
+	OF_DEV_AUXDATA("arm,vexpress-flash", V2M_NOR0, "physmap-flash", &v2m_flash_data),
+	OF_DEV_AUXDATA("arm,primecell", V2M_AACI, "mb:aaci", NULL),
+	OF_DEV_AUXDATA("arm,primecell", V2M_WDT, "mb:wdt", NULL),
+	OF_DEV_AUXDATA("arm,primecell", V2M_MMCI, "mb:mmci", &v2m_mmci_data),
+	OF_DEV_AUXDATA("arm,primecell", V2M_KMI0, "mb:kmi0", NULL),
+	OF_DEV_AUXDATA("arm,primecell", V2M_KMI1, "mb:kmi1", NULL),
+	OF_DEV_AUXDATA("arm,primecell", V2M_UART0, "mb:uart0", NULL),
+	OF_DEV_AUXDATA("arm,primecell", V2M_UART1, "mb:uart1", NULL),
+	OF_DEV_AUXDATA("arm,primecell", V2M_UART2, "mb:uart2", NULL),
+	OF_DEV_AUXDATA("arm,primecell", V2M_UART3, "mb:uart3", NULL),
+	OF_DEV_AUXDATA("arm,primecell", V2M_RTC, "mb:rtc", NULL),
+	{}
+};
+
+static void __init v2m_dt_init(void)
+{
+	of_platform_populate(NULL, of_default_bus_match_table,
+			     v2m_dt_auxdata_lookup, NULL);
+
+	pm_power_off = v2m_power_off;
+	arm_pm_restart = v2m_restart;
+
+	ct_desc->init_tile();
+}
+
+static const char *v2m_dt_match[] __initconst = {
+	"arm,vexpress",
+	NULL,
+};

Dave,

Quoted 62 lines · Dave Martin

This patch implements initial support for booting using a flattened device tree on the Versatile Express platform.

Eventually, it should be possible to present a single, core-tile- independent board, but in this transitional patch the baseboard + Cortex-A9x4 core tile combination is the only directly supported platform, since the implementation is not yet fully generic.

For now, clocks and timers are not handled via the device tree. Implementation of these can follow in later patches.

Thanks to Lorenzo Pieralisi, Grant Likely and Pawe? Moll for their help and contributions.

Signed-off-by Dave Martin <[email protected]>

Acked-by Pawe? Moll <[email protected]>

---

There are some outstanding issues which need to be discussed, listed below.

* This patch is not currently based on the GIC bindings being discussed by Rob Herring et al. Once that discussion reaches a conclusion, it should be straightforward to rebase onto the result.

* The following added bindings are not present upstream and need documentation / discussion:

* arm,vexpress -- the global board binding for all platform combinations using the Versatile Express motherboard.

* arm,vexpress-v2p-ca9 -- the specific binding for the Versatile Express motherboard with Cortex-A9x4 core tile installed. It is only mentioned as the most-specific match in vexpress-v2p- ca9.dts

Since it's intended that the motherboard code should be fully generic, and because no other core tiles are upstream yet, perhaps we can get rid of this binding right away.

* edid -- It should be possible to have a fairly generic binding for EDID interfaces, but none seems to exist yet. Discussion is needed regarding what form this should take.

This might more appropriately be called "ddc" (or some variation on that), since EDID seems only to describe the format of the ID data retrievable via this interface; not the interface itself.

* arm,vexpress-flash -- Needed because of the requirement to provide the physmap_flash driver with a special .set_vpp handler.

* idt,89hpes32h8 -- This is the IDT 89HPES32H8 PCI express interconnect switch. This isn't needed for the Versatile Express to work, but would be needed if using PCI-e peripherals for real. I expect that more driver support needs to go upstream before this is actually usable.

* nxp,isp1761 -- The driver support for this is already upstream (with some minor issues for ARM support).

Quoted 3 lines

* arm,amba-bus -- widely used by other boards and patchsets, but seems not to be documented.

This should be dropped. There's not really any bus component to an amba bus. All the probing info is within the primecell peripherals.

Quoted 9 lines

* The following bindings for ARM primecell peripherals are used elsewhere but not documented. They should be pretty simple and uncontraversial.
* arm,pl031
* arm,pl041
* arm,pl050
* arm,pl180
* arm,sp805

Plus pl011, pl010, sp804, pl022, pl061

Quoted 5 lines

Rob Herring suggested documenting simple bindings for these (and others) along with his initial amba device tree probe patches, but these bindings don't seem to be documented upstream for now.

pl330 went the other route with a file for itself. That may be better to avoid conflicts. But yes, ARM should document all their peripherals. ;)

I'll do the ones on highbank if you want to do the rest on VExp.

Quoted 55 lines

* Shawn Guo's smsc911x patch is needed for Ethernet to work. This is headed upstream but not yet in mainline. It is available in -next.

* Minor patches are needed to the isp1760 and pata_generic drivers, to allow OF-based initialisation across a wider group of architectures. These are being discussed independently, but are not yet accepted for merging upstream.

* Most core-tile peripherals are currently not described in the core- tile device tree fragment. This is a lower-priority issue since the motherboard code already autodetects the core-tile (though only one core-tile is fully upstream at the moment).

* Static peripheral mappings are not yet handled in a generic way in the board support code. This is a prerequisite for supporting multiple core-tiles int the same kernel. It well need to get fixed later, when extra core tile support is merged (or before).

Pawe? Moll is looking into this separately.

* The Kconfig logic for ensuring that at least one boot protocol and at least one core tile are selected is a bit ugly. Suggestions for improving this are certainly welcome.

 arch/arm/Kconfig                           |    1 +
 arch/arm/boot/dts/vexpress-v2m-legacy.dtsi |  163 ++++++++++++++++++++++++++++
 arch/arm/boot/dts/vexpress-v2p-ca9.dts     |   80 ++++++++++++++
 arch/arm/configs/vexpress_defconfig        |    1 +
 arch/arm/mach-vexpress/Kconfig             |   45 ++++++++-
 arch/arm/mach-vexpress/ct-ca9x4.c          |    7 ++
 arch/arm/mach-vexpress/v2m.c               |   54 +++++++++-
 7 files changed, 349 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/boot/dts/vexpress-v2m-legacy.dtsi
 create mode 100644 arch/arm/boot/dts/vexpress-v2p-ca9.dts

arch/arm/Kconfig +1 -0

index 5ebc5d9..a6e90d5 100644
@@ -282,6 +282,7 @@ config ARCH_VERSATILE
 
 config ARCH_VEXPRESS
 	bool "ARM Ltd. Versatile Express family"
+	select ARCH_VEXPRESS_SANE_CONFIG
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select ARM_AMBA
 	select ARM_TIMER_SP804

arch/arm/boot/dts/vexpress-v2m-legacy.dtsi new file +0 -0

new file mode 100644
index 0000000..fd6e4e4

arch/arm/boot/dts/vexpress-v2m-legacy.dtsi line 2

Quoted 2 lines

arch/arm/boot/dts/vexpress-v2m-legacy.dtsi new file +2 -0

@@ -0,0 +1,163 @@
+// ARM Ltd. Versatile Express Motherboard V2M-P1 (HBI-0190D)
+// Legacy memory map

Not sure, but C++ style comments are probably frowned upon in dts too.

Quoted 118 lines

arch/arm/boot/dts/vexpress-v2m-legacy.dtsi new file +118 -0

+
+/ {
+	aliases {
+		serial0 = &uart0;
+		serial1 = &uart1;
+		serial2 = &uart2;
+		serial3 = &uart3;
+		i2c0 = &i2c0;
+		i2c1 = &i2c1;
+	};
+
+	motherboard {
+		compatible = "simple-bus";
+		#address-cells = <2>; // SMB chipselect number and offset
+		#size-cells = <1>;
+		#interrupt-cells = <1>;
+
+		flash at 0,00000000 {
+			compatible = "arm,vexpress-flash", "cfi-flash";
+			reg = <0 0x00000000 0x04000000
+			       1 0x00000000 0x04000000>;
+			bank-width = <4>;
+		};
+
+		psram at 2,00000000 {
+			compatible = "mtd-ram";
+			reg = <2 0x00000000 0x02000000>;
+			bank-width = <4>;
+		};
+
+		ethernet at 3,02000000 {
+			compatible = "smsc,lan9118", "smsc,lan9115";
+			reg = <3 0x02000000 0x10000>;
+			reg-io-width = <4>;
+			interrupts = <15>;
+			smsc,irq-active-high;
+			smsc,irq-push-pull;
+		};
+
+		usb at 3,03000000 {
+			compatible = "nxp,usb-isp1761";
+			reg = <3 0x03000000 0x20000>;
+			interrupts = <16>;
+			port1-otg;
+		};
+
+		peripherals at 7,00000000 {
+			compatible = "arm,amba-bus", "simple-bus";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges = <0 7 0 0x20000>;
+
+			// PCI-E I2C bus
+			i2c0: i2c at 02000 {
+				compatible = "arm,versatile-i2c";
+				reg = <0x02000 0x1000>;
+
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				pcie-switch at 60 {
+					compatible = "idt,89hpes32h8";
+					reg = <0x60>;
+				};
+			};
+
+			aaci at 04000 {
+				compatible = "arm,pl041", "arm,primecell";
+				reg = <0x04000 0x1000>;
+				interrupts = <11>;
+			};
+
+			mmci at 05000 {
+				compatible = "arm,pl180", "arm,primecell";
+				reg = <0x05000 0x1000>;
+				interrupts = <9 10>;
+			};
+
+			kmi at 06000 {
+				compatible = "arm,pl050", "arm,primecell";
+				reg = <0x06000 0x1000>;
+				interrupts = <12>;
+			};
+
+			kmi at 07000 {
+				compatible = "arm,pl050", "arm,primecell";
+				reg = <0x07000 0x1000>;
+				interrupts = <13>;
+			};
+
+			uart0: uart at 09000 {
+				compatible = "arm,pl011", "arm,primecell";
+				reg = <0x09000 0x1000>;
+				interrupts = <5>;
+			};
+
+			uart1: uart at 0a000 {
+				compatible = "arm,pl011", "arm,primecell";
+				reg = <0x0a000 0x1000>;
+				interrupts = <6>;
+			};
+
+			uart2: uart at 0b000 {
+				compatible = "arm,pl011", "arm,primecell";
+				reg = <0x0b000 0x1000>;
+				interrupts = <7>;
+			};
+
+			uart3: uart at 0c000 {
+				compatible = "arm,pl011", "arm,primecell";
+				reg = <0x0c000 0x1000>;
+				interrupts = <8>;
+			};
+
+			wdt at 0f000 {
+				compatible = "arm,sp805", "arm,primecell";
+				reg = <0x0f000 0x1000>;
+				interrupts = <0>;

arch/arm/boot/dts/vexpress-v2m-legacy.dtsi line 125

Quoted 5 lines

arch/arm/boot/dts/vexpress-v2m-legacy.dtsi new file +5 -0

+			};
+
+			// Timer init is hardcoded in v2m_timer_init(), for now.
+			// timer at 11000 {
+			//	compatible = "arm,arm-sp804";

arm,sp804 is more consistent. I believe the sp804 does have the periphid registers, so arm,primecell should also be added.

arch/arm/boot/dts/vexpress-v2m-legacy.dtsi line 133

Quoted 8 lines

arch/arm/boot/dts/vexpress-v2m-legacy.dtsi new file +8 -0

+			//	reg = <0x11000 0x1000>;
+			//	interrupts = <2>;
+			// };
+
+			// timer at 12000 {
+			//	compatible = "arm,arm-sp804";
+			//	reg = <0x12000 0x1000>;
+			// };

Just because Linux is not using it, doesn't mean you should comment it out.

OK, I'll try to propose documentation for these:
* arm,pl180

Quoted 6 lines

arch/arm/boot/dts/vexpress-v2m-legacy.dtsi line 125

Quoted 3 lines

arch/arm/boot/dts/vexpress-v2m-legacy.dtsi new file +3 -0

+			// Timer init is hardcoded in v2m_timer_init(), for now.
+			// timer at 11000 {
+			//	compatible = "arm,arm-sp804";

arm,sp804 is more consistent. I believe the sp804 does have the periphid registers, so arm,primecell should also be added.

Do you mean "does not have"? If so, the periphid will be needed -- thanks for pointing it out in that case.

I will make the names consistent. These were pasted from someone Lorenzo's older patches, and failed to sport e the inconsistency since I wasn't actually making use of these entries yet.

Quoted 10 lines

arch/arm/boot/dts/vexpress-v2m-legacy.dtsi line 133

Quoted 8 lines

arch/arm/boot/dts/vexpress-v2m-legacy.dtsi new file +8 -0

+			//	reg = <0x11000 0x1000>;
+			//	interrupts = <2>;
+			// };
+
+			// timer at 12000 {
+			//	compatible = "arm,arm-sp804";
+			//	reg = <0x12000 0x1000>;
+			// };

Just because Linux is not using it, doesn't mean you should comment it out.

From the point of view of describing the hardware, yes. However, I was a bit worried that if sp804 is turned into a full driver, it will get initialised twice -- once explicitly and once in of_platform_populate()... at least until the baord code is adapted to work properly with the new driver.

Commenting these entries out for now seemed a good idea to avoid the flag-day hazard. Am I being too cautious?

Quoted 2 lines

OK, I'll try to propose documentation for these:
* arm,pl180

You can skip this one - I'll add the description together with the MMCI driver bindings (it will be 180 and 181, by the way :-)

Quoted 7 lines

Quoted 6 lines

arch/arm/boot/dts/vexpress-v2m-legacy.dtsi line 125

Quoted 3 lines

arch/arm/boot/dts/vexpress-v2m-legacy.dtsi new file +3 -0

+			// Timer init is hardcoded in v2m_timer_init(), for now.
+			// timer at 11000 {
+			//	compatible = "arm,arm-sp804";

arm,sp804 is more consistent. I believe the sp804 does have the periphid registers, so arm,primecell should also be added.

Quoted 2 lines

Do you mean "does not have"? If so, the periphid will be needed -- thanks for pointing it out in that case.

I think Rob meant it should be

	compatible = "arm,sp804", "arm,primecell",

as SP804 contains the PrimeCell periphid registers, so will be recognized by amba bus driver.

Quoted 21 lines

I will make the names consistent. These were pasted from someone Lorenzo's older patches, and failed to sport e the inconsistency since I wasn't actually making use of these entries yet.

Quoted 10 lines

arch/arm/boot/dts/vexpress-v2m-legacy.dtsi line 133

Quoted 8 lines

arch/arm/boot/dts/vexpress-v2m-legacy.dtsi new file +8 -0

+			//	reg = <0x11000 0x1000>;
+			//	interrupts = <2>;
+			// };
+
+			// timer at 12000 {
+			//	compatible = "arm,arm-sp804";
+			//	reg = <0x12000 0x1000>;
+			// };

Just because Linux is not using it, doesn't mean you should comment it out.

From the point of view of describing the hardware, yes. However, I was a bit worried that if sp804 is turned into a full driver, it will get initialised twice -- once explicitly and once in of_platform_populate()... at least until the baord code is adapted to work properly with the new driver.

Quoted 2 lines

Commenting these entries out for now seemed a good idea to avoid the flag-day hazard. Am I being too cautious?

I think you are ;-) Besides my static-mapping-rework is already using those...

Quoted 3 lines · Dave Martin

? ? ? * arm,amba-bus -- widely used by other boards and patchsets, but ? ? ? ? seems not to be documented.

This should be dropped. There's not really any bus component to an amba bus. All the probing info is within the primecell peripherals.

Quoted 5 lines · Rob Herring

Quoted 3 lines · Dave Martin

* arm,amba-bus -- widely used by other boards and patchsets, but seems not to be documented.

Quoted 2 lines

This should be dropped. There's not really any bus component to an amba bus. All the probing info is within the primecell peripherals.

No, if it is an AMBA bus, then it is entirely appropriate to declare it as an amba bus, but to also be compatible with "simple-bus". In fact, it would be better to use a compatible string that specifies the specific implementation of AMBA bus since there are several versions of the spec.

Quoted 6 lines · Grant Likely

Quoted 5 lines · Rob Herring

Quoted 3 lines · Dave Martin

? ? ? * arm,amba-bus -- widely used by other boards and patchsets, but ? ? ? ? seems not to be documented.

Quoted 8 lines

Quoted 2 lines

This should be dropped. There's not really any bus component to an amba bus. All the probing info is within the primecell peripherals.

No, if it is an AMBA bus, then it is entirely appropriate to declare it as an amba bus, but to also be compatible with "simple-bus". ?In fact, it would be better to use a compatible string that specifies the specific implementation of AMBA bus since there are several versions of the spec.

Dave asked me about details of the VE implementation. It's sort-of-complicated... ;-)

1. Core talks to Static Memory Controller via AMBA (AXI)

? SOC { core --AXI--> SMC }

2. SMC generates transaction on Static Memory Bus talking to the IO FPGA

? tile/motherboard connector { SMC --SMB--> IOFPGA }

3. Now, depending on the device being accessed:

a) Transactions accessing SMSC9118, ISP1761, NOR Flash and PSRAM are routed directly to the devices

? IOFPGA { SMB --> SMSC9118 et al. }

b) The rest of the traffic is converted back to AMBA (AHB/APB) transactions and sent to the devices connected to internal AMBA matrix.

? IOFPGA { SMB --> AHB/APB bus master --AHB/APB--> PL180 }

I don't believe, though, that the DTS must reflect such level of details. That's why I think that:

+ ? ? ? motherboard {
+ ? ? ? ? ? ? ? compatible = "simple-bus";

and

+ ? ? ? ? ? ? ? peripherals at 7,00000000 {
+ ? ? ? ? ? ? ? ? ? ? ? compatible = "arm,amba-bus", "simple-bus";

is the best description of the reality :-)

Quoted 29 lines

Dave asked me about details of the VE implementation. It's sort-of-complicated... ;-)

1. Core talks to Static Memory Controller via AMBA (AXI)

  SOC { core --AXI--> SMC }

2. SMC generates transaction on Static Memory Bus talking to the IO FPGA

  tile/motherboard connector { SMC --SMB--> IOFPGA }

3. Now, depending on the device being accessed:

a) Transactions accessing SMSC9118, ISP1761, NOR Flash and PSRAM are routed directly to the devices

IOFPGA { SMB --> SMSC9118 et al. }

b) The rest of the traffic is converted back to AMBA (AHB/APB) transactions and sent to the devices connected to internal AMBA matrix.

  IOFPGA { SMB --> AHB/APB bus master --AHB/APB--> PL180 }

I don't believe, though, that the DTS must reflect such level of details. That's why I think that:

arch/arm/boot/dts/vexpress-v2m-legacy.dtsi new file +2 -0

+       motherboard {
+               compatible = "simple-bus";

Quoted 6 lines

and

arch/arm/boot/dts/vexpress-v2m-legacy.dtsi new file +2 -0

+               peripherals at 7,00000000 {
+                       compatible = "arm,amba-bus", "simple-bus";

is the best description of the reality :-)

I wonder whether an OS will ever need to know this detail.

Am I right in understanding that these buses are just interconnect logic, with no OS-visible control/configuration interface?

Quoted 5 lines · Rob Herring

Quoted 3 lines · Dave Martin

* arm,amba-bus -- widely used by other boards and patchsets, but seems not to be documented.

Quoted 2 lines

This should be dropped. There's not really any bus component to an amba bus. All the probing info is within the primecell peripherals.

No, if it is an AMBA bus, then it is entirely appropriate to declare it as an amba bus, but to also be compatible with "simple-bus". In fact, it would be better to use a compatible string that specifies the specific implementation of AMBA bus since there are several versions of the spec.

Quoted 6 lines · Grant Likely

Quoted 5 lines · Rob Herring

Quoted 3 lines · Dave Martin

* arm,amba-bus -- widely used by other boards and patchsets, but seems not to be documented.

Quoted 8 lines

Quoted 2 lines

This should be dropped. There's not really any bus component to an amba bus. All the probing info is within the primecell peripherals.

No, if it is an AMBA bus, then it is entirely appropriate to declare it as an amba bus, but to also be compatible with "simple-bus". In fact, it would be better to use a compatible string that specifies the specific implementation of AMBA bus since there are several versions of the spec.

And type of AMBA bus as the spec includes AXI, AHB, and APB. None of which have any sort of programmability or software view.

If this is required, then the policy should be simple-bus should never be allowed alone as every bus has some underlying type. Seems like overkill for buses like this.

Quoted 16 lines · Rob Herring

Quoted 6 lines · Grant Likely

Quoted 5 lines · Rob Herring

Quoted 3 lines · Dave Martin

* arm,amba-bus -- widely used by other boards and patchsets, but seems not to be documented.

Quoted 8 lines

Quoted 2 lines

This should be dropped. There's not really any bus component to an amba bus. All the probing info is within the primecell peripherals.

No, if it is an AMBA bus, then it is entirely appropriate to declare it as an amba bus, but to also be compatible with "simple-bus". In fact, it would be better to use a compatible string that specifies the specific implementation of AMBA bus since there are several versions of the spec.

Quoted 6 lines

And type of AMBA bus as the spec includes AXI, AHB, and APB. None of which have any sort of programmability or software view.

If this is required, then the policy should be simple-bus should never be allowed alone as every bus has some underlying type. Seems like overkill for buses like this.

The key question is _where_ to draw the line between generic and specific. By definition, the DT can never be a comprehensive description of the hardware -- rather a good DT is a description of those details of the hardware which could relevant to any hypothetical OS.

The flipside is that details which were thought to be irrelevant at design/implementation time can turn out to be relevant in practice, due to errata and implementation issues etc. So taking the description slightly beyond what the OS needs to know can still have some merit.

I still don't know how to say where the line should be drawn in this particular case though.

Cheers
---Dave
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss at lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Quoted 29 lines · Dave Martin

Quoted 16 lines · Rob Herring

Quoted 6 lines · Grant Likely

Quoted 5 lines · Rob Herring

Quoted 3 lines · Dave Martin

* arm,amba-bus -- widely used by other boards and patchsets, but seems not to be documented.

Quoted 8 lines

Quoted 2 lines

This should be dropped. There's not really any bus component to an amba bus. All the probing info is within the primecell peripherals.

No, if it is an AMBA bus, then it is entirely appropriate to declare it as an amba bus, but to also be compatible with "simple-bus". In fact, it would be better to use a compatible string that specifies the specific implementation of AMBA bus since there are several versions of the spec.

Quoted 6 lines

And type of AMBA bus as the spec includes AXI, AHB, and APB. None of which have any sort of programmability or software view.

If this is required, then the policy should be simple-bus should never be allowed alone as every bus has some underlying type. Seems like overkill for buses like this.

The key question is _where_ to draw the line between generic and specific. By definition, the DT can never be a comprehensive description of the hardware -- rather a good DT is a description of those details of the hardware which could relevant to any hypothetical OS.

Quoted 8 lines

The flipside is that details which were thought to be irrelevant at design/implementation time can turn out to be relevant in practice, due to errata and implementation issues etc. So taking the description slightly beyond what the OS needs to know can still have some merit.

I still don't know how to say where the line should be drawn in this particular case though.

Here are some criteria:

If the controller for the bus itself has registers, include the bus node

If it is possible to plug new stuff into the bus, include the bus node

If the base address for the bus can be changed, thereby changing all the addresses of its subordinates by the same offset, include the bus node (this usually goes along with "has registers".)

ARM buses typically don't have any of those attributes, but there are some weaker criteria that can be used to justify including a bus node. The SoC on which I'm currently working has some peripherals on AXI and others on APB. That doesn't matter from that addressing standpoint - the individual peripherals can each be viewed as having an address, end of story - but it does matter from a power management standpoint. The clock tree is quite related to the bus layout. Including bus nodes in the device tree might provide useful place-holders for properties describing power or clock domains.

So, on the whole, I'm in favor of including bus nodes for ARM standard buses. There is little down side to doing so, and a fair chance that it might come in handy in the future.

? ? ?* edid -- It should be possible to have a fairly generic binding ? ? ? ?for EDID interfaces, but none seems to exist yet. ?Discussion ? ? ? ?is needed regarding what form this should take.

? ? ? ?This might more appropriately be called "ddc" (or some ? ? ? ?variation on that), since EDID seems only to describe the ? ? ? ?format of the ID data retrievable via this interface; not the ? ? ? ?interface itself.

Quoted 5 lines · Dave Martin

* edid -- It should be possible to have a fairly generic binding for EDID interfaces, but none seems to exist yet. Discussion is needed regarding what form this should take.

Quoted 4 lines

This might more appropriately be called "ddc" (or some variation on that), since EDID seems only to describe the format of the ID data retrievable via this interface; not the interface itself.

Has there been any progress on this issue? I'm looking to add EDID support to a PowerPC device tree. A TI developer is using "ti,eeprom", but I'm not sure that's a good choice.

The way it works for many "graphics cards" is that the display hardware subsystem includes an I2C (also called "SMBUS") interface that connects to the EDID ROM on the monitor. In this model, the EDID interface is not a standalone device, but rather a feature of the display device.

In that scenario, the EDID-reading code is just part of the display driver, so you don't need a separate device node.

If the display hardware does not include a dedicated I2C interface intended for EDID, then I think what you need is a way to associate an external I2C interface with the display driver for that hardware. The interpretation of the data as EDID is not really part of the hardware interface, but rather a function of the display driver. Therefore, I think the right way to look at this is not to have a binding for "EDID interfaces", but rather a convention for associating a specific instance of an I2C interface with a display driver.

The obvious way to do that would be to have a property in the display driver whose value is the phandle of an i2c device node. The display driver can then use that to read and interpret the EDID bytes.

In my opinion, pushing the EDID abstraction into a node by itself is not worthwhile.

The EDID spec says that you read either 128 or 256 bytes from an I2C device at I2C address 0x50; you hardly need an abstraction for that, given that you have a "read from I2C" method.
The right level of abstraction at the device node level is "this hardware implements an I2C bus master", for which there is already a binding. Then all you need is a reference to that device from a display device node.

The display device driver will need to interpret the EDID data in an device-dependent manner. That is inherent in the fact that the driver for the given display hardware must map the EDID description of the monitor into display-hardware-dependent timing settings.

Some I2C interfaces are implemented by bit-banging GPIOs, while others use dedicated hardware protocol engines. The display driver need not know or care about that, as it should be hidden by the i2c bus abstraction.

In my opinion, pushing the EDID abstraction into a node by itself is not worthwhile. The EDID spec says that you read either 128 or 256 bytes from an I2C device at I2C address 0x50; you hardly need an abstraction for that, given that you have a "read from I2C" method.

The right level of abstraction at the device node level is "this hardware implements an I2C bus master", for which there is already a binding. Then all you need is a reference to that device from a display device node.

The display device driver will need to interpret the EDID data in an device-dependent manner. That is inherent in the fact that the driver for the given display hardware must map the EDID description of the monitor into display-hardware-dependent timing settings.

It still needs to know *which* I2C bus master is connected to the display. Some graphics hardware has multiple, e.g. one to talk with the DVI/HDMI transmitter, another connected by cable to the display.

I have worked with devices that shared the same I2C for DDC and talking with on-board devices. (That was a bad idea, as some monitors clamp the lines high or low excessively even when switched off.) Point here is sometimes there's a dedicated one for DDC; sometimes there isn't.

Quoted 3 lines · Jamie Lokier

It still needs to know *which* I2C bus master is connected to the display. Some graphics hardware has multiple, e.g. one to talk with the DVI/HDMI transmitter, another connected by cable to the display.

Yes, this is my problem. I have multiple I2C busses on my system, and there's no way to guess which one is connected to the DVI port. A phandle in the device tree is the only way I can figure out which I2C bus to use. But there's another problem. I can use of_find_i2c_device_by_node() to determine the i2c_client (and therefore, the i2c_adapter) to use for fb_ddc_read(). However, that function only works if the I2C device was probed. That typically is done by the I2C driver for the device, but there is no "edid" device driver. So my framebuffer driver is going to have to pretend to be one.

I wish there were some way to obtain the i2c_adapter struct more easily.

Quoted 4 lines

I have worked with devices that shared the same I2C for DDC and talking with on-board devices. (That was a bad idea, as some monitors clamp the lines high or low excessively even when switched off.) Point here is sometimes there's a dedicated one for DDC; sometimes there isn't.

I also have this problem. I need to toggle a GPIO in order to get the I2C bus connected to the DVI port. The same I2C bus is used for the audio codec and a bunch of other devices. Our SOC has four I2C busses, so I don't understand why the board designed didn't dedicate one for the DDC. But I can easily work around this issue.

Quoted 7 lines

Quoted 3 lines · Jamie Lokier

It still needs to know *which* I2C bus master is connected to the display. Some graphics hardware has multiple, e.g. one to talk with the DVI/HDMI transmitter, another connected by cable to the display.

Yes, this is my problem. I have multiple I2C busses on my system, and there's no way to guess which

one is connected to the DVI port. A phandle in the device tree is the only way I can figure out which I2C bus to use. But there's another problem. I can use of_find_i2c_device_by_node() to determine the i2c_client (and therefore, the i2c_adapter) to use for fb_ddc_read(). However, that function only works if the I2C device was probed. That typically is done by the I2C driver for the device, but there is no "edid" device driver. So my framebuffer driver is going to have to pretend to be one.

It seems to me that having the framebuffer driver handle the EDID "driver" function is exactly the right thing. The framebuffer driver is the only thing that cares about EDID information. Given a phandle reference to an I2C interface, the "driver" is just "call the I2C read routine" - plus, in your case, toggling the GPIO pin.

That GPIO pin thing is annoying, but not sufficiently complex or common that it warrants having a separate EDID driver. You could define a platform-specific property to tell your framebuffer driver that it needs to do that GPIO thing. It's a hack, but the GPIO thing is inherently a hack, so there will be some ugliness somewhere as a result.

Quoted 8 lines

I wish there were some way to obtain the i2c_adapter struct more easily.

Quoted 4 lines

I have worked with devices that shared the same I2C for DDC and talking with on-board devices. (That was a bad idea, as some monitors clamp the lines high or low excessively even when switched off.) Point here is sometimes there's a dedicated one for DDC; sometimes there isn't.

I also have this problem. I need to toggle a GPIO in order to get the I2C bus connected to the DVI

port. The same I2C bus is used for the audio codec and a bunch of other devices. Our SOC has four I2C busses, so I don't understand why the board designed didn't dedicate one for the DDC. But I can easily work around this issue.

Shouldn't there be an I2C bus mux driver to handle this? The kernel already has support for bus muxes controller over I2C.

It seems to me that having the framebuffer driver handle the EDID "driver" function is exactly the right thing. The framebuffer driver is the only thing that cares about EDID information. Given a phandle reference to an I2C interface, the "driver" is just "call the I2C read routine" - plus, in your case, toggling the GPIO pin.

That GPIO pin thing is annoying, but not sufficiently complex or common that it warrants having a separate EDID driver. You could define a platform-specific property to tell your framebuffer driver that it needs to do that GPIO thing. It's a hack, but the GPIO thing is inherently a hack, so there will be some ugliness somewhere as a result.

Quoted 5 lines · Mitch Bradley

It seems to me that having the framebuffer driver handle the EDID "driver" function is exactly the right thing. The framebuffer driver is the only thing that cares about EDID information. Given a phandle reference to an I2C interface, the "driver" is just "call the I2C read routine" - plus, in your case, toggling the GPIO pin.

Do you think I should have the I2C probe function read the EDID data, and then store it in some global variable?

That won't work if I have multiple video controllers, since I won't know which EDID data goes with which video controller. I tried adding a call to i2c_add_driver() in my framebuffer driver, but the I2C probe function was called *after* the framebuffer's probe function, so that doesn't help me.

Quoted 6 lines

That GPIO pin thing is annoying, but not sufficiently complex or common that it warrants having a separate EDID driver. You could define a platform-specific property to tell your framebuffer driver that it needs to do that GPIO thing. It's a hack, but the GPIO thing is inherently a hack, so there will be some ugliness somewhere as a result.

I have two platform-specific functions, "enabled_edid" and "disable_edid", that I call before/after calling fb_ddc_read(). This seems to work well, and I already have a mechanism for calling platform-specific functions from the framebuffer driver.

However, Stephen Warren said I should be using the I2C mux feature instead.

I think that would not be a good design. Presence and location of EDID data is not something that a generic I2C driver should know. It's the video controller that knows that EDID exists, where it is located (device ID 0x50 and addresses within that device), and what it means.

Quoted 1 line

That won't work if I have multiple video controllers, since I won't know which EDID data goes with which video controller. I tried adding a call to i2c_add_driver() in my framebuffer driver, but the I2C probe function was called *after* the framebuffer's probe function, so that doesn't help me.

Then there needs to be some sort of ordering or dependency to ensure that the I2C driver is activated before the framebuffer driver tries to use it.

Either way, you need platform-dependent functions to do the switching, and you need to select the appropriate channel. Personally, I don't see the advantage of using the mux device in this case. It's just adding complexity with no payback.

Quoted 4 lines · Mitch Bradley

I think that would not be a good design. Presence and location of EDID data is not something that a generic I2C driver should know. It's the video controller that knows that EDID exists, where it is located (device ID 0x50 and addresses within that device), and what it means.

But the video driver does not know what I2C *bus* it's on. I have been unable to come up with a way to obtain the proper i2c_adapter object to use when looking for EDID data.

Quoted 6 lines

Quoted 2 lines

That won't work if I have multiple video controllers, since I won't know which EDID data goes with which video controller. I tried adding a call to i2c_add_driver() in my framebuffer driver, but the I2C probe function was called *after* the framebuffer's probe function, so that doesn't help me.

Then there needs to be some sort of ordering or dependency to ensure that the I2C driver is activated before the framebuffer driver tries to use it.

I don't know how to do that, either.

Quoted 4 lines

Either way, you need platform-dependent functions to do the switching, and you need to select the appropriate channel. Personally, I don't see the advantage of using the mux device in this case. It's just adding complexity with no payback.

I'm always in favor of doing things the simpler way.

You make device nodes for each of the i2c adapters

(possibly with a GPIO
I2C mux node underneath the EDID one),

then you point to the correct
adapter (or mux) node with a phandle-valued property in the video node.

Of course, you need the deferral patch cited in another message.

Quoted 1 line

Mitch Bradley wrote:

Quoted 11 lines

Quoted 5 lines

That GPIO pin thing is annoying, but not sufficiently complex or common that it warrants having a separate EDID driver. You could define a platform-specific property to tell your framebuffer driver that it needs to do that GPIO thing. It's a hack, but the GPIO thing is inherently a hack, so there will be some ugliness somewhere as a result.

I have two platform-specific functions, "enabled_edid" and "disable_edid", that I call before/after calling fb_ddc_read(). This seems to work well, and I already have a mechanism for calling platform- specific functions from the framebuffer driver.

However, Stephen Warren said I should be using the I2C mux feature instead.

I2C mux is a plausible solution, as is your enable/disable thing. At some level they are equivalent. I2C mux is a formalization of your solution, in which the mux device's select method must be written to perform the function of your enable/disable edid functions.

Either way, you need platform-dependent functions to do the switching, and you need to select the appropriate channel. Personally, I don't see the advantage of using the mux device in this case.

It's just adding
complexity with no payback. If there were several channels that needed to be accessed in an interspersed fashion, the mux device would be much cleaner. But in this case, there is a single "back channel" that only needs to be accessed once and can subsequently be ignored.

The video
driver can grab a lock, call enable_edid(), read out the EDID data into an array, call disable_edid(), release the lock, and that is it. The other users of that I2C bus can ignore the hidden EDID.

Well, the EDID needs to be read on every hotplug event, so it's certainly not a one-time thing.

Quoted 2 lines · Stephen Warren

Well, the EDID needs to be read on every hotplug event, so it's certainly not a one-time thing.

What is the hotplug event that triggers an EDID read?

For monitors attached to user-accessible connectors, whenever the user physically plugs in a new monitor, you need to read the EDID since it could be a new monitor. DVI and HDMI have explicit hotplug detect lines to trigger this. I'm not sure how it works for VGA; perhaps you have to manually reprobe.

Of course, if this thread is talking about built-in LCD displays where the connectivity is fixed, this is a non-issue, but it's still a relevant general design discussion; hopefully whatever solution that's picked would work for DVI/HDMI even if you're dealing with LCDs right now.

I also noticed that the I2C interface is not enabled until after the console opens the framebuffer. That means it's impossible for EDID data to be available before the console switches to the video device. That doesn't seem right.

Timur Tabi wrote at Wednesday, January 11, 2012 1:33 PM:

Quoted 4 lines

Quoted 2 lines · Stephen Warren

Well, the EDID needs to be read on every hotplug event, so it's certainly not a one-time thing.

Quoted 1 line

What is the hotplug event that triggers an EDID read?

For monitors attached to user-accessible connectors, whenever the user physically plugs in a new monitor, you need to read the EDID since it could be a new monitor. DVI and HDMI have explicit hotplug detect lines to trigger this. I'm not sure how it works for VGA; perhaps you have to manually reprobe.

Of course, if this thread is talking about built-in LCD displays where the connectivity is fixed, this is a non-issue, but it's still a relevant general design discussion; hopefully whatever solution that's picked would work for DVI/HDMI even if you're dealing with LCDs right now.

For all the above reasons I think the DDC/EDID support in the kernel should be moving gently towards a (very small) DDC/EDID mini-subsystem shared by video drivers. I.e. it should be exposed as a well-known I2C bus name, and if it's GPIO bit-banging I2C, that should be exposed as well.

Mitch Bradley wrote at Tuesday, January 10, 2012 11:43 PM:

Quoted 2 lines · Timur Tabi

Quoted 1 line

Mitch Bradley wrote:

...

Quoted 17 lines

Quoted 11 lines

Quoted 5 lines

That GPIO pin thing is annoying, but not sufficiently complex or common that it warrants having a separate EDID driver. You could define a platform-specific property to tell your framebuffer driver that it needs to do that GPIO thing. It's a hack, but the GPIO thing is inherently a hack, so there will be some ugliness somewhere as a result.

I have two platform-specific functions, "enabled_edid" and "disable_edid", that I call before/after calling fb_ddc_read(). This seems to work well, and I already have a mechanism for calling platform- specific functions from the framebuffer driver.

However, Stephen Warren said I should be using the I2C mux feature instead.

I2C mux is a plausible solution, as is your enable/disable thing. At some level they are equivalent. I2C mux is a formalization of your solution, in which the mux device's select method must be written to perform the function of your enable/disable edid functions.

Quoted 3 lines

Either way, you need platform-dependent functions to do the switching, and you need to select the appropriate channel. Personally, I don't see the advantage of using the mux device in this case.

The main advantage I see is that you explicitly don't need any platform- specific functions to do the switching; you end up with platform-agnostic code (the I2C GPIO mux driver) and platform-specific configuration for that driver (the GPIO ID to use).

The display driver just talks to the
I2C API for the DDC I2C bus, and doesn't do anything to switch between the busses; the I2C GPIO mux driver does all that internally. Thus, the display driver will work fine on boards that don't need this muxing with zero changes; the board simply wouldn't register the mux driver.

Quoted 5 lines

It's just adding
complexity with no payback. If there were several channels that needed to be accessed in an interspersed fashion, the mux device would be much cleaner. But in this case, there is a single "back channel" that only needs to be accessed once and can subsequently be ignored.

Well, the EDID needs to be read on every hotplug event, so it's certainly not a one-time thing.

Quoted 4 lines

The video
driver can grab a lock, call enable_edid(), read out the EDID data into an array, call disable_edid(), release the lock, and that is it. The other users of that I2C bus can ignore the hidden EDID.

Other I2C users/devices also shouldn't be impacted by the mux; they would continue to use the existing I2C APIs for the bus their devices are attached to, and not know about the mux.

The overhead should be
almost zero; the I2C GPIO mux driver could remember the previous bus selection and only modify it (gpio_set_value) when switching between busses. I guess there might be a small overhead to taking a lock to protect the bus selection; I'm not sure whether the I2C core serializes access already or you'd need to add this; check the existing I2C I2C bus mux implementation I guess.

Quoted 11 lines

Mitch Bradley wrote at Tuesday, January 10, 2012 11:43 PM:

Quoted 2 lines · Timur Tabi

Quoted 1 line

Mitch Bradley wrote:

...

Quoted 7 lines

Quoted 7 lines

Quoted 5 lines

That GPIO pin thing is annoying, but not sufficiently complex or common that it warrants having a separate EDID driver. You could define a platform-specific property to tell your framebuffer driver that it needs to do that GPIO thing. It's a hack, but the GPIO thing is inherently a hack, so there will be some ugliness somewhere as a result.

I have two platform-specific functions, "enabled_edid" and "disable_edid", that I call

before/after

Quoted 1 line

Quoted 1 line

Quoted 1 line

calling fb_ddc_read(). This seems to work well, and I already have a mechanism for calling

platform-

Quoted 9 lines

Quoted 9 lines

Quoted 3 lines

specific functions from the framebuffer driver.

However, Stephen Warren said I should be using the I2C mux feature instead.

I2C mux is a plausible solution, as is your enable/disable thing. At some level they are equivalent. I2C mux is a formalization of your solution, in which the mux device's select method must be written to perform the function of your enable/disable edid functions.

Quoted 8 lines

Quoted 3 lines

Either way, you need platform-dependent functions to do the switching, and you need to select the appropriate channel. Personally, I don't see the advantage of using the mux device in this case.

The main advantage I see is that you explicitly don't need any platform- specific functions to do the switching; you end up with platform-agnostic code (the I2C GPIO mux driver) and platform-specific configuration for that driver (the GPIO ID to use).

Oh, I didn't know about the I2C GPIO Mux driver. I was looking at i2c-mux.c . I now see gpio-i2cmux.c, which indeed seems to do the right thing.

Quoted 20 lines

The display driver just talks to the
I2C API for the DDC I2C bus, and doesn't do anything to switch between the busses; the I2C GPIO mux driver does all that internally. Thus, the display driver will work fine on boards that don't need this muxing with zero changes; the board simply wouldn't register the mux driver.

Quoted 5 lines

It's just adding
complexity with no payback. If there were several channels that needed to be accessed in an interspersed fashion, the mux device would be much cleaner. But in this case, there is a single "back channel" that only needs to be accessed once and can subsequently be ignored.

Well, the EDID needs to be read on every hotplug event, so it's certainly not a one-time thing.

Quoted 4 lines

The video
driver can grab a lock, call enable_edid(), read out the EDID data into an array, call disable_edid(), release the lock, and that is it. The other users of that I2C bus can ignore the hidden EDID.

Quoted 3 lines

Other I2C users/devices also shouldn't be impacted by the mux; they would continue to use the existing I2C APIs for the bus their devices are attached to, and not know about the mux.

If other devices that are on the same bus as the EDID don't use the mux, how does one ensure that the GPIO is restored to the non-EDID setting when the display driver is finished?

Perhaps I'm missing something, but it appears to me that the model is to set the correct GPIO state before each use, instead of a save-set-use-restore model.

In any case, if there is a good way to instantiate the GPIO mux device from the device tree, it certainly provides a ready-made solution. Each device that is on the bus in question would have a device node that is a child of the GPIO mux node, and the display driver could have a phandle-valued property pointing to the mux node, plus a property declaring the selection value (or perhaps a single 2-cell property with phandle, selection-value).

Mitch Bradley wrote at Wednesday, January 11, 2012 4:16 PM:

Quoted 58 lines · Stephen Warren

Quoted 11 lines

Mitch Bradley wrote at Tuesday, January 10, 2012 11:43 PM:

Quoted 2 lines · Timur Tabi

Quoted 1 line

Mitch Bradley wrote:

...

Quoted 7 lines

Quoted 7 lines

Quoted 5 lines

That GPIO pin thing is annoying, but not sufficiently complex or common that it warrants having a separate EDID driver. You could define a platform-specific property to tell your framebuffer driver that it needs to do that GPIO thing. It's a hack, but the GPIO thing is inherently a hack, so there will be some ugliness somewhere as a result.

I have two platform-specific functions, "enabled_edid" and "disable_edid", that I call

before/after

Quoted 1 line

Quoted 1 line

Quoted 1 line

calling fb_ddc_read(). This seems to work well, and I already have a mechanism for calling

platform-

Quoted 9 lines

Quoted 9 lines

Quoted 3 lines

specific functions from the framebuffer driver.

However, Stephen Warren said I should be using the I2C mux feature instead.

I2C mux is a plausible solution, as is your enable/disable thing. At some level they are equivalent. I2C mux is a formalization of your solution, in which the mux device's select method must be written to perform the function of your enable/disable edid functions.

Quoted 8 lines

Quoted 3 lines

Either way, you need platform-dependent functions to do the switching, and you need to select the appropriate channel. Personally, I don't see the advantage of using the mux device in this case.

The main advantage I see is that you explicitly don't need any platform- specific functions to do the switching; you end up with platform-agnostic code (the I2C GPIO mux driver) and platform-specific configuration for that driver (the GPIO ID to use).

Oh, I didn't know about the I2C GPIO Mux driver. I was looking at i2c-mux.c . I now see gpio-i2cmux.c, which indeed seems to do the right thing.

Quoted 20 lines

The display driver just talks to the
I2C API for the DDC I2C bus, and doesn't do anything to switch between the busses; the I2C GPIO mux driver does all that internally. Thus, the display driver will work fine on boards that don't need this muxing with zero changes; the board simply wouldn't register the mux driver.

Quoted 5 lines

It's just adding
complexity with no payback. If there were several channels that needed to be accessed in an interspersed fashion, the mux device would be much cleaner. But in this case, there is a single "back channel" that only needs to be accessed once and can subsequently be ignored.

Well, the EDID needs to be read on every hotplug event, so it's certainly not a one-time thing.

Quoted 4 lines

The video
driver can grab a lock, call enable_edid(), read out the EDID data into an array, call disable_edid(), release the lock, and that is it. The other users of that I2C bus can ignore the hidden EDID.

Quoted 7 lines

Quoted 3 lines

Other I2C users/devices also shouldn't be impacted by the mux; they would continue to use the existing I2C APIs for the bus their devices are attached to, and not know about the mux.

If other devices that are on the same bus as the EDID don't use the mux, how does one ensure that the GPIO is restored to the non-EDID setting when the display driver is finished?

The I2C busses are set up like this:

bus 0 bus 1
I2C controller -------> mux ------> dev a, dev b, dev c, ...

                            \------>  dev x, dev y, dev z, ...
                              bus 2

Thus all devices are on a child I2C bus of the mux and none on the raw HW bus exposed by the I2C controller itself.

The I2C core will always call gpiomux_select before each transaction, which will set the GPIOs appropriately for bus 1 or bus 2, depending on which device is being communicated with.

Quoted 3 lines

Perhaps I'm missing something, but it appears to me that the model is to set the correct GPIO state before each use, instead of a save-set-use-restore model.

That's true, but the select action happens implicitly inside the I2C core for any and all transactions, AIUI, so the two modes are equivalent.

Quoted 7 lines

In any case, if there is a good way to instantiate the GPIO mux device from the device tree, it certainly provides a ready-made solution. Each device that is on the bus in question would have a device node that is a child of the GPIO mux node, and the display driver could have a phandle-valued property pointing to the mux node, plus a property declaring the selection value (or perhaps a single 2-cell property with phandle, selection-value).

That's probably the difficult part.

For an I2C mux that is controlled via I2C, you can just add the mux node as a child of the I2C controller, since it has an I2C address, and so putting it there makes sense.

But for an I2C mux that's controlled using GPIOs or pinmux, there's no I2C address so I guess the mux shouldn't be directly underneath the I2C controller.

Perhaps the DT binding for such an I2C mux can refer to the parent I2C controller by phandle?

Inside the I2C mux DT node, I think we can have a child node for each bus, and then use standard I2C child node addressing for all the nodes within these bus nodes.

Perhaps:

i2c1: i2c at 7000c000 {
     #address-cells =<1>;
     #size-cells =<0>;
     compatible = "nvidia,tegra20-i2c";
     reg =<0x7000C000 0x100>;
     interrupts =<0 38 0x04>;
};

mux at 0 {
     #address-cells =<1>;
     #size-cells =<0>;
     compatible = "nvidia,tegra20-i2c";

     parent-bus =<&i2c1>;
     gpios =<&gpio 100 0&gpio 101 0>;
     gpio-values-idle =<0>; /* bitmask of values */

     bus at 0 {
         #address-cells =<1>;
         #size-cells =<0>;
         /*
          * The GPIO values to set as a bitmask.
          * Formatted like gpio-i2cmux.c's mux->data.values[i].
          * Or name this gpio-values?
          */

         reg =<1>;

         wm8903: wm8903 at 1a {
             compatible = "wlf,wm8903";
             reg =<0x1a>;
             ...
         };
     };

     bus at 1 {
         #address-cells =<1>;
         #size-cells =<0>;
         reg =<2>;

         light-sensor at 44 {
             compatible = "isil,isl29018";
             reg =<0x44>;
             ...
         };
     };
};

i2c1: i2c at 7000c000 {
     #address-cells =<1>;
     #size-cells =<0>;
     compatible = "nvidia,tegra20-i2c";
     reg =<0x7000C000 0x100>;
     interrupts =<0 38 0x04>;
};

mux at 0 {

     #address-cells =<1>;
     #size-cells =<0>;
     compatible = "nvidia,tegra20-i2c";
     parent-bus =<&i2c1>;
     gpios =<&gpio 100 0&gpio 101 0>;
     gpio-values-idle =<0>; /* bitmask of values */

     bus at 0 {

         #address-cells =<1>;
         #size-cells =<0>;
         /*
          * The GPIO values to set as a bitmask.
          * Formatted like gpio-i2cmux.c's mux->data.values[i].
          * Or name this gpio-values?
          */
         reg =<1>;

         wm8903: wm8903 at 1a {
             compatible = "wlf,wm8903";
             reg =<0x1a>;
             ...
         };
     };

     bus at 1 {

         #address-cells =<1>;
         #size-cells =<0>;
         reg =<2>;

         light-sensor at 44 {
             compatible = "isil,isl29018";
             reg =<0x44>;
             ...
         };
     };
};

Quoted 1 line

Mitch Bradley wrote at Wednesday, January 11, 2012 4:16 PM:

Quoted 18 lines

Quoted 7 lines

In any case, if there is a good way to instantiate the GPIO mux device from the device tree, it certainly provides a ready-made solution. Each device that is on the bus in question would have a device node that is a child of the GPIO mux node, and the display driver could have a phandle-valued property pointing to the mux node, plus a property declaring the selection value (or perhaps a single 2-cell property with phandle, selection-value).

That's probably the difficult part.

For an I2C mux that is controlled via I2C, you can just add the mux node as a child of the I2C controller, since it has an I2C address, and so putting it there makes sense.

But for an I2C mux that's controlled using GPIOs or pinmux, there's no I2C address so I guess the mux shouldn't be directly underneath the I2C controller.

Quoted 8 lines

Perhaps the DT binding for such an I2C mux can refer to the parent I2C controller by phandle?

Inside the I2C mux DT node, I think we can have a child node for each bus, and then use standard I2C child node addressing for all the nodes within these bus nodes.

Perhaps:

The scheme below looks good to me, with minor nits picked...

Quoted 8 lines

i2c1: i2c at 7000c000 {
     #address-cells =<1>;
     #size-cells =<0>;
     compatible = "nvidia,tegra20-i2c";
     reg =<0x7000C000 0x100>;
     interrupts =<0 38 0x04>;
};

Quoted 4 lines

mux at 0 {
     #address-cells =<1>;
     #size-cells =<0>;
     compatible = "nvidia,tegra20-i2c";

Shouldn't this compatible value be set up to bind to gpio_i2cmux? The node doesn't seem to be hardware-specific.

Quoted 4 lines

     parent-bus =<&i2c1>;
     gpios =<&gpio 100 0&gpio 101 0>;
     gpio-values-idle =<0>; /* bitmask of values */

Quoted 8 lines

     bus at 0 {
         #address-cells =<1>;
         #size-cells =<0>;
         /*
          * The GPIO values to set as a bitmask.
          * Formatted like gpio-i2cmux.c's mux->data.values[i].
          * Or name this gpio-values?
          */

Did you mean for the comment above to be associated with the "gpio-values-idle" property? It seems out of place here.

Quoted 1 line

         reg =<1>;

reg =<0> because this is bus at 0

Quoted 12 lines

         wm8903: wm8903 at 1a {
             compatible = "wlf,wm8903";
             reg =<0x1a>;
             ...
         };
     };

     bus at 1 {
         #address-cells =<1>;
         #size-cells =<0>;
         reg =<2>;

reg =<1> because this is bus at 1

Quoted 8 lines

         light-sensor at 44 {
             compatible = "isil,isl29018";
             reg =<0x44>;
             ...
         };
     };
};

Mitch Bradley wrote at Wednesday, January 11, 2012 4:16 PM:

Quoted 3 lines

Perhaps I'm missing something, but it appears to me that the model is to set the correct GPIO state before each use, instead of a save-set-use-restore model.

That's true, but the select action happens implicitly inside the I2C core for any and all transactions, AIUI, so the two modes are equivalent.

For an I2C mux that is controlled via I2C, you can just add the mux node as a child of the I2C controller, since it has an I2C address, and so putting it there makes sense.

But for an I2C mux that's controlled using GPIOs or pinmux, there's no I2C address so I guess the mux shouldn't be directly underneath the I2C controller.

Quoted 5 lines · Stephen Warren

Mitch Bradley wrote at Wednesday, January 11, 2012 4:16 PM:

Quoted 3 lines

Perhaps I'm missing something, but it appears to me that the model is to set the correct GPIO state before each use, instead of a save-set-use-restore model.

Quoted 2 lines

That's true, but the select action happens implicitly inside the I2C core for any and all transactions, AIUI, so the two modes are equivalent.

If there is some reason why it would be desirable to deselect the DDC I2C bus after transactions, that could be added as an option to the GPIO I2C mux, but I can't think of a reason.

Quoted 3 lines

For an I2C mux that is controlled via I2C, you can just add the mux node as a child of the I2C controller, since it has an I2C address, and so putting it there makes sense.

Only if the mux's I2C slave is on the near side of the mux so that it's always addressable regardless of mux state, and if it's on the same I2C.

Quoted 3 lines

But for an I2C mux that's controlled using GPIOs or pinmux, there's no I2C address so I guess the mux shouldn't be directly underneath the I2C controller.

It's connected to: (a) an I2C, and (b) a GPIO.

It can't be child of both, but I don't see why the GPIO can't be referred by phandle and the I2C it's muxing be the direct parent. That seems more natural to me.

Quoted 5 lines · Dave Martin

? ? ?* edid -- It should be possible to have a fairly generic binding ? ? ? ?for EDID interfaces, but none seems to exist yet. ?Discussion ? ? ? ?is needed regarding what form this should take.

Quoted 4 lines

? ? ? ?This might more appropriately be called "ddc" (or some ? ? ? ?variation on that), since EDID seems only to describe the ? ? ? ?format of the ID data retrievable via this interface; not the ? ? ? ?interface itself.

Has there been any progress on this issue? I'm looking to add EDID support to a PowerPC device tree. A TI developer is using "ti,eeprom", but I'm not sure that's a good choice.