RFC: [PATCH] platform device driver model support

RFC: [PATCH] platform device driver model support

[Kumar Gala]

All,

Please take a look at the following patch.  It adds driver model support 
via platform devices to 85xx.  This is originally based on patches from 
Jason M. 

The idea behind the code is that for a give family: 4xx, 8xx, 82xx, 83xx, 
85xx, 86xx we will have structure defns for the following:

enum ppc_soc_devices 
in asm-ppc/<family.h>:
  list of all unique devices in the family

struct platform_device soc_platform_devices[]
in arch/ppc/platforms/<family>/<family>_devices.c:
  describes all platform devices that exist in the family

struct soc_spec soc_specs[]
in arch/ppc/platforms/<family>/<family>_soc.c:
  describes each unique chip in the family and what devices it has

Plus the following functions:

identify_soc_by_id() -- determine soc by an int id
identify_soc_by_name() -- determin soc by name (useful in some 82xx cases)
ppc_soc_get_pdata() -- get platform_data pointer so board code can modify
ppc_soc_update_paddr() -- update iomem resources with a given paddr

Please provide feedback, I want to get this into 2.6.11 for 85xx.

thanks

- kumar

--

diff -Nru a/arch/ppc/platforms/85xx/Makefile b/arch/ppc/platforms/85xx/Makefile
--- a/arch/ppc/platforms/85xx/Makefile	2005-01-12 01:31:07 -06:00
+++ b/arch/ppc/platforms/85xx/Makefile	2005-01-12 01:31:07 -06:00
@@ -1,6 +1,7 @@
 #
 # Makefile for the PowerPC 85xx linux kernel.
 #
+obj-$(CONFIG_85xx)		+= mpc85xx_soc.o mpc85xx_devices.o
 
 obj-$(CONFIG_MPC8540_ADS)	+= mpc85xx_ads_common.o mpc8540_ads.o
 obj-$(CONFIG_MPC8555_CDS)	+= mpc85xx_cds_common.o
diff -Nru a/arch/ppc/platforms/85xx/mpc8540_ads.c b/arch/ppc/platforms/85xx/mpc8540_ads.c
--- a/arch/ppc/platforms/85xx/mpc8540_ads.c	2005-01-12 01:31:07 -06:00
+++ b/arch/ppc/platforms/85xx/mpc8540_ads.c	2005-01-12 01:31:07 -06:00
@@ -32,6 +32,7 @@
 #include <linux/serial_core.h>
 #include <linux/initrd.h>
 #include <linux/module.h>
+#include <linux/fsl_devices.h>
 
 #include <asm/system.h>
 #include <asm/pgtable.h>
@@ -49,45 +50,12 @@
 #include <asm/immap_85xx.h>
 #include <asm/kgdb.h>
 #include <asm/ocp.h>
+#include <asm/soctable.h>
 #include <mm/mmu_decl.h>
 
 #include <syslib/ppc85xx_common.h>
 #include <syslib/ppc85xx_setup.h>
 
-struct ocp_gfar_data mpc85xx_tsec1_def = {
-	.interruptTransmit = MPC85xx_IRQ_TSEC1_TX,
-	.interruptError = MPC85xx_IRQ_TSEC1_ERROR,
-	.interruptReceive = MPC85xx_IRQ_TSEC1_RX,
-	.interruptPHY = MPC85xx_IRQ_EXT5,
-	.flags = (GFAR_HAS_GIGABIT | GFAR_HAS_MULTI_INTR
-			| GFAR_HAS_RMON
-			| GFAR_HAS_PHY_INTR | GFAR_HAS_COALESCE),
-	.phyid = 0,
-	.phyregidx = 0,
-};
-
-struct ocp_gfar_data mpc85xx_tsec2_def = {
-	.interruptTransmit = MPC85xx_IRQ_TSEC2_TX,
-	.interruptError = MPC85xx_IRQ_TSEC2_ERROR,
-	.interruptReceive = MPC85xx_IRQ_TSEC2_RX,
-	.interruptPHY = MPC85xx_IRQ_EXT5,
-	.flags = (GFAR_HAS_GIGABIT | GFAR_HAS_MULTI_INTR
-			| GFAR_HAS_RMON
-			| GFAR_HAS_PHY_INTR | GFAR_HAS_COALESCE),
-	.phyid = 1,
-	.phyregidx = 0,
-};
-
-struct ocp_gfar_data mpc85xx_fec_def = {
-	.interruptTransmit = MPC85xx_IRQ_FEC,
-	.interruptError = MPC85xx_IRQ_FEC,
-	.interruptReceive = MPC85xx_IRQ_FEC,
-	.interruptPHY = MPC85xx_IRQ_EXT5,
-	.flags = 0,
-	.phyid = 3,
-	.phyregidx = 0,
-};
-
 struct ocp_fs_i2c_data mpc85xx_i2c1_def = {
 	.flags = FS_I2C_SEPARATE_DFSRR,
 };
@@ -100,10 +68,11 @@
 static void __init
 mpc8540ads_setup_arch(void)
 {
-	struct ocp_def *def;
-	struct ocp_gfar_data *einfo;
 	bd_t *binfo = (bd_t *) __res;
 	unsigned int freq;
+	struct gianfar_platform_data *pdata;
+
+	identify_soc_by_id(mfspr(SVR));
 
 	/* get the core frequency */
 	freq = binfo->bi_intfreq;
@@ -130,23 +99,30 @@
 	invalidate_tlbcam_entry(NUM_TLBCAMS - 1);
 #endif
 
-	def = ocp_get_one_device(OCP_VENDOR_FREESCALE, OCP_FUNC_GFAR, 0);
-	if (def) {
-		einfo = (struct ocp_gfar_data *) def->additions;
-		memcpy(einfo->mac_addr, binfo->bi_enetaddr, 6);
-	}
-
-	def = ocp_get_one_device(OCP_VENDOR_FREESCALE, OCP_FUNC_GFAR, 1);
-	if (def) {
-		einfo = (struct ocp_gfar_data *) def->additions;
-		memcpy(einfo->mac_addr, binfo->bi_enet1addr, 6);
-	}
-
-	def = ocp_get_one_device(OCP_VENDOR_FREESCALE, OCP_FUNC_GFAR, 2);
-	if (def) {
-		einfo = (struct ocp_gfar_data *) def->additions;
-		memcpy(einfo->mac_addr, binfo->bi_enet2addr, 6);
-	}
+	/* setup the board related information for the enet controllers */
+	pdata = (struct gianfar_platform_data *) ppc_soc_get_pdata(MPC85xx_TSEC1);
+	pdata->board_flags = FSL_GIANFAR_BRD_HAS_PHY_INTR;
+	pdata->interruptPHY = MPC85xx_IRQ_EXT5;
+	pdata->phyid = 0;
+	/* fixup phy address */
+	pdata->phy_reg_addr += binfo->bi_immr_base;
+	memcpy(pdata->mac_addr, binfo->bi_enetaddr, 6);
+
+	pdata = (struct gianfar_platform_data *) ppc_soc_get_pdata(MPC85xx_TSEC2);
+	pdata->board_flags = FSL_GIANFAR_BRD_HAS_PHY_INTR;
+	pdata->interruptPHY = MPC85xx_IRQ_EXT5;
+	pdata->phyid = 1;
+	/* fixup phy address */
+	pdata->phy_reg_addr += binfo->bi_immr_base;
+	memcpy(pdata->mac_addr, binfo->bi_enet1addr, 6);
+	
+	pdata = (struct gianfar_platform_data *) ppc_soc_get_pdata(MPC85xx_FEC);
+	pdata->board_flags = FSL_GIANFAR_BRD_HAS_PHY_INTR;
+	pdata->interruptPHY = MPC85xx_IRQ_EXT5;
+	pdata->phyid = 3;
+	/* fixup phy address */
+	pdata->phy_reg_addr += binfo->bi_immr_base;
+	memcpy(pdata->mac_addr, binfo->bi_enet2addr, 6);
 
 #ifdef CONFIG_BLK_DEV_INITRD
 	if (initrd_start)
diff -Nru a/arch/ppc/platforms/85xx/mpc85xx_devices.c b/arch/ppc/platforms/85xx/mpc85xx_devices.c
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/arch/ppc/platforms/85xx/mpc85xx_devices.c	2005-01-12 01:31:07 -06:00
@@ -0,0 +1,245 @@
+/*
+ * arch/ppc/platforms/85xx/mpc85xx_devices.c
+ *
+ * MPC85xx Device descriptions
+ *
+ * Maintainer: Kumar Gala <kumar.gala@freescale.com>
+ *
+ * Copyright 2005 Freescale Semiconductor Inc.
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/fsl_devices.h>
+#include <asm/mpc85xx.h>
+#include <asm/soctable.h>
+
+/* We use offsets for IORESOURCE_MEM since we do not know at compile time
+ * what CCSRBAR is, platform code should fix this up in setup_arch
+ */
+
+static struct gianfar_platform_data mpc85xx_tsec1_pdata = {
+	.device_flags = FSL_GIANFAR_DEV_HAS_GIGABIT |
+		FSL_GIANFAR_DEV_HAS_COALESCE | FSL_GIANFAR_DEV_HAS_RMON |
+		FSL_GIANFAR_DEV_HAS_MULTI_INTR,
+	.phy_reg_addr = MPC85xx_ENET1_OFFSET,
+};
+
+static struct gianfar_platform_data mpc85xx_tsec2_pdata = {
+	.device_flags = FSL_GIANFAR_DEV_HAS_GIGABIT |
+		FSL_GIANFAR_DEV_HAS_COALESCE | FSL_GIANFAR_DEV_HAS_RMON |
+		FSL_GIANFAR_DEV_HAS_MULTI_INTR,
+	.phy_reg_addr = MPC85xx_ENET1_OFFSET,
+};
+
+static struct gianfar_platform_data mpc85xx_fec_pdata = {
+	.phy_reg_addr = MPC85xx_ENET1_OFFSET,
+};
+
+static struct fsl_i2c_platform_data mpc85xx_fsl_i2c_pdata = {
+	.device_flags = FSL_I2C_DEV_SEPARATE_DFSRR,
+};
+
+struct platform_device soc_platform_devices[] = {
+	[MPC85xx_TSEC1] = {
+		.name = "fsl-gianfar",
+		.id	= 1,
+		.dev.platform_data = &mpc85xx_tsec1_pdata,
+		.num_resources	 = 4,
+		.resource = (struct resource[]) {
+			{
+				.start	= MPC85xx_ENET1_OFFSET,
+				.end	= MPC85xx_ENET1_OFFSET +
+						MPC85xx_ENET1_SIZE - 1,
+				.flags	= IORESOURCE_MEM,
+			},
+			{
+				.name	= "tx",
+				.start	= MPC85xx_IRQ_TSEC1_TX,
+				.end	= MPC85xx_IRQ_TSEC1_TX,
+				.flags	= IORESOURCE_IRQ,
+			},
+			{
+				.name	= "rx",
+				.start	= MPC85xx_IRQ_TSEC1_RX,
+				.end	= MPC85xx_IRQ_TSEC1_RX,
+				.flags	= IORESOURCE_IRQ,
+			},
+			{
+				.name	= "error",
+				.start	= MPC85xx_IRQ_TSEC1_ERROR,
+				.end	= MPC85xx_IRQ_TSEC1_ERROR,
+				.flags	= IORESOURCE_IRQ,
+			},
+		},
+	},
+	[MPC85xx_TSEC2] = {
+		.name = "fsl-gianfar",
+		.id	= 2,
+		.dev.platform_data = &mpc85xx_tsec2_pdata,
+		.num_resources	 = 4,
+		.resource = (struct resource[]) {
+			{
+				.start	= MPC85xx_ENET2_OFFSET,
+				.end	= MPC85xx_ENET2_OFFSET +
+						MPC85xx_ENET2_SIZE - 1,
+				.flags	= IORESOURCE_MEM,
+			},
+			{
+				.name	= "tx",
+				.start	= MPC85xx_IRQ_TSEC2_TX,
+				.end	= MPC85xx_IRQ_TSEC2_TX,
+				.flags	= IORESOURCE_IRQ,
+			},
+			{
+				.name	= "rx",
+				.start	= MPC85xx_IRQ_TSEC2_RX,
+				.end	= MPC85xx_IRQ_TSEC2_RX,
+				.flags	= IORESOURCE_IRQ,
+			},
+			{
+				.name	= "error",
+				.start	= MPC85xx_IRQ_TSEC2_ERROR,
+				.end	= MPC85xx_IRQ_TSEC2_ERROR,
+				.flags	= IORESOURCE_IRQ,
+			},
+		},
+	},
+	[MPC85xx_FEC] =	{
+		.name = "fsl-gianfar",
+		.id	= 3,
+		.dev.platform_data = &mpc85xx_fec_pdata,
+		.num_resources	 = 2,
+		.resource = (struct resource[]) {
+			{
+				.start	= MPC85xx_ENET3_OFFSET,
+				.end	= MPC85xx_ENET3_OFFSET +
+						MPC85xx_ENET3_SIZE - 1,
+				.flags	= IORESOURCE_MEM,
+
+			},
+			{
+				.start	= MPC85xx_IRQ_FEC,
+				.end	= MPC85xx_IRQ_FEC,
+				.flags	= IORESOURCE_IRQ,
+			},
+		},
+	},
+	[MPC85xx_IIC1] = {
+		.name = "fsl-i2c",
+		.id	= 1,
+		.dev.platform_data = &mpc85xx_fsl_i2c_pdata,
+		.num_resources	 = 2,
+		.resource = (struct resource[]) {
+			{
+				.start	= MPC85xx_IIC1_OFFSET,
+				.end	= MPC85xx_IIC1_OFFSET +
+						MPC85xx_IIC1_SIZE - 1,
+				.flags	= IORESOURCE_MEM,
+			},
+			{
+				.start	= MPC85xx_IRQ_IIC1,
+				.end	= MPC85xx_IRQ_IIC1,
+				.flags	= IORESOURCE_IRQ,
+			},
+		},
+	},
+	[MPC85xx_DMA0] = {
+		.name = "fsl-dma",
+		.id	= 0,
+		.num_resources	 = 2,
+		.resource = (struct resource[]) {
+			{
+				.start	= MPC85xx_DMA0_OFFSET,
+				.end	= MPC85xx_DMA0_OFFSET +
+						MPC85xx_DMA0_SIZE - 1,
+				.flags	= IORESOURCE_MEM,
+			},
+			{
+				.start	= MPC85xx_IRQ_DMA0,
+				.end	= MPC85xx_IRQ_DMA0,
+				.flags	= IORESOURCE_IRQ,
+			},
+		},
+	},
+	[MPC85xx_DMA1] = {
+		.name = "fsl-dma",
+		.id	= 1,
+		.num_resources	 = 2,
+		.resource = (struct resource[]) {
+			{
+				.start	= MPC85xx_DMA1_OFFSET,
+				.end	= MPC85xx_DMA1_OFFSET +
+						MPC85xx_DMA1_SIZE - 1,
+				.flags	= IORESOURCE_MEM,
+			},
+			{
+				.start	= MPC85xx_IRQ_DMA1,
+				.end	= MPC85xx_IRQ_DMA1,
+				.flags	= IORESOURCE_IRQ,
+			},
+		},
+	},
+	[MPC85xx_DMA2] = {
+		.name = "fsl-dma",
+		.id	= 2,
+		.num_resources	 = 2,
+		.resource = (struct resource[]) {
+			{
+				.start	= MPC85xx_DMA2_OFFSET,
+				.end	= MPC85xx_DMA2_OFFSET +
+						MPC85xx_DMA2_SIZE - 1,
+				.flags	= IORESOURCE_MEM,
+			},
+			{
+				.start	= MPC85xx_IRQ_DMA2,
+				.end	= MPC85xx_IRQ_DMA2,
+				.flags	= IORESOURCE_IRQ,
+			},
+		},
+	},
+	[MPC85xx_DMA3] = {
+		.name = "fsl-dma",
+		.id	= 3,
+		.num_resources	 = 2,
+		.resource = (struct resource[]) {
+			{
+				.start	= MPC85xx_DMA3_OFFSET,
+				.end	= MPC85xx_DMA3_OFFSET +
+						MPC85xx_DMA3_SIZE - 1,
+				.flags	= IORESOURCE_MEM,
+			},
+			{
+				.start	= MPC85xx_IRQ_DMA3,
+				.end	= MPC85xx_IRQ_DMA3,
+				.flags	= IORESOURCE_IRQ,
+			},
+		},
+	},
+};
+
+static int __init mach_mpc85xx_init(void)
+{
+	unsigned int i, dev_id, ret = 0;
+
+	BUG_ON(cur_soc_spec == NULL);
+
+	for (i = 0; i < cur_soc_spec->num_devices; i++) {
+		dev_id = cur_soc_spec->device_list[i];
+		ppc_soc_update_paddr(&soc_platform_devices[dev_id], CCSRBAR);
+		if (platform_device_register(&soc_platform_devices[dev_id])) {
+			ret = 1;
+			printk (KERN_ERR "unable to register device %d\n", dev_id);
+		}
+	};
+
+	return ret;
+}
+
+subsys_initcall(mach_mpc85xx_init);
diff -Nru a/arch/ppc/platforms/85xx/mpc85xx_soc.c b/arch/ppc/platforms/85xx/mpc85xx_soc.c
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/arch/ppc/platforms/85xx/mpc85xx_soc.c	2005-01-12 01:31:07 -06:00
@@ -0,0 +1,50 @@
+/*
+ * arch/ppc/platforms/85xx/mpc85xx_soc.c
+ *
+ * MPC85xx SOC descriptions
+ *
+ * Maintainer: Kumar Gala <kumar.gala@freescale.com>
+ *
+ * Copyright 2005 Freescale Semiconductor Inc.
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <asm/soctable.h>
+
+struct soc_spec* cur_soc_spec;
+struct soc_spec soc_specs[] = {
+	{
+		.soc_name	= "MPC8540",
+		.soc_mask 	= 0xFFFF0000,
+		.soc_value 	= 0x80300000,
+		.num_devices	= 8,
+		.device_list	= (enum ppc_soc_devices[]) 
+		{ 
+			MPC85xx_TSEC1, MPC85xx_TSEC2, MPC85xx_FEC, MPC85xx_IIC1, 
+			MPC85xx_DMA0, MPC85xx_DMA1, MPC85xx_DMA2, MPC85xx_DMA3, 
+		},
+	},
+	{
+		.soc_name	= "MPC8560",
+		.soc_mask 	= 0xFFFF0000,
+		.soc_value 	= 0x80700000,
+		.num_devices	= 7,
+		.device_list	= (enum ppc_soc_devices[]) 
+		{ 
+			MPC85xx_TSEC1, MPC85xx_TSEC2, MPC85xx_IIC1, 
+			MPC85xx_DMA0, MPC85xx_DMA1, MPC85xx_DMA2, MPC85xx_DMA3, 
+		},
+	},
+	{	/* default match */
+		.soc_name	= "",
+		.soc_mask 	= 0x00000000,
+		.soc_value 	= 0x00000000,
+	},
+};
diff -Nru a/arch/ppc/syslib/Makefile b/arch/ppc/syslib/Makefile
--- a/arch/ppc/syslib/Makefile	2005-01-12 01:31:07 -06:00
+++ b/arch/ppc/syslib/Makefile	2005-01-12 01:31:07 -06:00
@@ -92,7 +92,8 @@
 obj-$(CONFIG_MPC10X_OPENPIC)	+= open_pic.o
 obj-$(CONFIG_40x)		+= dcr.o
 obj-$(CONFIG_BOOKE)		+= dcr.o
-obj-$(CONFIG_85xx)		+= open_pic.o ppc85xx_common.o ppc85xx_setup.o
+obj-$(CONFIG_85xx)		+= open_pic.o ppc85xx_common.o ppc85xx_setup.o \
+					soc.o
 ifeq ($(CONFIG_85xx),y)
 obj-$(CONFIG_PCI)		+= indirect_pci.o pci_auto.o
 endif
diff -Nru a/arch/ppc/syslib/soc.c b/arch/ppc/syslib/soc.c
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/arch/ppc/syslib/soc.c	2005-01-12 01:31:07 -06:00
@@ -0,0 +1,59 @@
+/*
+ * arch/ppc/syslib/soc.c
+ *
+ * System-on-chip (SOC) library functions
+ *
+ * Maintainer: Kumar Gala <kumar.gala@freescale.com>
+ *
+ * Copyright 2005 Freescale Semiconductor Inc.
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#include <asm/soctable.h>
+
+void __init
+identify_soc_by_id(unsigned int id)
+{
+	unsigned int i = 0;
+	while (1) {
+		if ((soc_specs[i].soc_mask & id) == soc_specs[i].soc_value)
+			break;
+		i++;
+	};
+
+	cur_soc_spec = &soc_specs[i];
+
+	return;
+}
+
+void __init
+identify_soc_by_name(char *name)
+{
+	/* TODO */
+	return;
+}
+
+/* Update all memory resources by paddr, call before platform_device_register */
+void __init
+ppc_soc_update_paddr(struct platform_device *pdev, phys_addr_t paddr)
+{
+	int i;
+	for (i = 0; i < pdev->num_resources; i++) {
+		struct resource *r = &pdev->resource[i];
+		if ((r->flags & IORESOURCE_MEM) == IORESOURCE_MEM) {
+			r->start += paddr;
+			r->end += paddr;
+		};
+	};
+}
+
+/* Get platform_data pointer out of platform device, call before platform_device_register */
+void * __init
+ppc_soc_get_pdata(enum ppc_soc_devices dev)
+{
+	return soc_platform_devices[dev].dev.platform_data;
+}
diff -Nru a/include/asm-ppc/mpc85xx.h b/include/asm-ppc/mpc85xx.h
--- a/include/asm-ppc/mpc85xx.h	2005-01-12 01:31:07 -06:00
+++ b/include/asm-ppc/mpc85xx.h	2005-01-12 01:31:07 -06:00
@@ -103,6 +103,14 @@
 #define MPC85xx_CPM_SIZE	(0x40000)
 #define MPC85xx_DMA_OFFSET	(0x21000)
 #define MPC85xx_DMA_SIZE	(0x01000)
+#define MPC85xx_DMA0_OFFSET	(0x21100)
+#define MPC85xx_DMA0_SIZE	(0x00080)
+#define MPC85xx_DMA1_OFFSET	(0x21180)
+#define MPC85xx_DMA1_SIZE	(0x00080)
+#define MPC85xx_DMA2_OFFSET	(0x21200)
+#define MPC85xx_DMA2_SIZE	(0x00080)
+#define MPC85xx_DMA3_OFFSET	(0x21280)
+#define MPC85xx_DMA3_SIZE	(0x00080)
 #define MPC85xx_ENET1_OFFSET	(0x24000)
 #define MPC85xx_ENET1_SIZE	(0x01000)
 #define MPC85xx_ENET2_OFFSET	(0x25000)
@@ -138,6 +146,17 @@
 #else
 #define CCSRBAR BOARD_CCSRBAR
 #endif
+
+enum ppc_soc_devices {
+	MPC85xx_TSEC1,
+	MPC85xx_TSEC2,
+	MPC85xx_FEC,
+	MPC85xx_IIC1,
+	MPC85xx_DMA0,
+	MPC85xx_DMA1,
+	MPC85xx_DMA2,
+	MPC85xx_DMA3,
+};
 
 #endif /* CONFIG_85xx */
 #endif /* __ASM_MPC85xx_H__ */
diff -Nru a/include/asm-ppc/soctable.h b/include/asm-ppc/soctable.h
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/include/asm-ppc/soctable.h	2005-01-12 01:31:07 -06:00
@@ -0,0 +1,56 @@
+/*
+ * include/asm-ppc/soctable.h
+ *
+ * SOC definitions and library functions
+ *
+ * Maintainer: Kumar Gala <kumar.gala@freescale.com>
+ *
+ * Copyright 2005 Freescale Semiconductor, Inc
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#ifdef __KERNEL__
+#ifndef __ASM_PPC_SOCTABLE_H
+#define __ASM_PPC_SOCTABLE_H
+
+#include <linux/init.h>
+#include <linux/device.h>
+#if defined(CONFIG_85xx)
+#include <asm/mpc85xx.h>
+#else
+#error "need definition of ppc_soc_devices"
+#endif
+
+struct soc_spec {
+	/* SOC is matched via (ID & soc_mask) == soc_value, id could be
+	 * PVR, SVR, IMMR, * etc. */
+	unsigned int		soc_mask;
+	unsigned int		soc_value;
+	unsigned int		num_devices;
+	char			*soc_name;
+	enum ppc_soc_devices	*device_list;
+};
+
+/* describes all specific chips and which devices they have on them */
+extern struct soc_spec		soc_specs[];
+extern struct soc_spec		*cur_soc_spec;
+
+/* determine which specific SOC we are */
+extern void identify_soc_by_id(unsigned int id) __init;
+extern void identify_soc_by_name(char *name) __init;
+
+/* describes all devices that may exist in a given family of processors */
+extern struct platform_device soc_platform_devices[];
+
+/* Update all memory resources by paddr, call before platform_device_register */
+extern void ppc_soc_update_paddr(struct platform_device *pdev, phys_addr_t paddr) __init;
+
+/* Get platform_data pointer out of platform device, call before platform_device_register */
+extern void * ppc_soc_get_pdata(enum ppc_soc_devices dev) __init;
+
+#endif /* __ASM_PPC_SOCTABLE_H */
+#endif /* __KERNEL__ */
diff -Nru a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/include/linux/fsl_devices.h	2005-01-12 01:31:07 -06:00
@@ -0,0 +1,77 @@
+/*
+ * include/linux/fsl_devices.h
+ *
+ * Definitions for any platform device related flags or structures for 
+ * Freescale processor devices
+ *
+ * Maintainer: Kumar Gala (kumar.gala@freescale.com)
+ *
+ * Copyright 2004 Freescale Semiconductor, Inc
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#ifdef __KERNEL__
+#ifndef _FSL_DEVICE_H_
+#define _FSL_DEVICE_H_
+
+/* 
+ * Some conventions on how we handle peripherals on Freescale chips
+ *
+ * unique device: a platform_device entry in fsl_plat_devs[] plus 
+ * associated device information in its platform_data structure.
+ *
+ * A chip is described by a set of unique devices.
+ *
+ * Each sub-arch has its own master list of unique devices and
+ * enumerates them by enum fsl_devices in a sub-arch specific header
+ *
+ * The platform data structure is broken into two parts.  The
+ * first is device specific information that help identify any
+ * unique features of a peripheral.  The second is any
+ * information that may be defined by the board or how the device
+ * is connected externally of the chip.
+ *
+ * naming conventions:
+ * - platform data structures: <driver>_platform_data
+ * - platform data device flags: FSL_<driver>_DEV_<FLAG>
+ * - platform data board flags: FSL_<driver>_BRD_<FLAG>
+ * 
+ *
+ */
+
+struct gianfar_platform_data {
+	/* device specific information */
+	u32 device_flags;
+	u32 phy_reg_addr;
+
+	/* board specific information */
+	u32 board_flags;
+	u32 phyid;
+	u32 interruptPHY;
+	u8 mac_addr[6];
+};
+
+/* Flags related to gianfar device features */
+#define FSL_GIANFAR_DEV_HAS_GIGABIT		0x00000001
+#define FSL_GIANFAR_DEV_HAS_COALESCE		0x00000002
+#define FSL_GIANFAR_DEV_HAS_RMON		0x00000004
+#define FSL_GIANFAR_DEV_HAS_MULTI_INTR		0x00000008
+
+/* Flags in gianfar_platform_data */
+#define FSL_GIANFAR_BRD_HAS_PHY_INTR	0x00000001	/* if not set use a timer */
+
+struct fsl_i2c_platform_data {
+	/* device specific information */
+	u32 device_flags;
+};
+
+/* Flags related to I2C device features */
+#define FSL_I2C_DEV_SEPARATE_DFSRR	0x00000001
+#define FSL_I2C_DEV_CLOCK_5200		0x00000002
+
+#endif	/* _FSL_DEVICE_H_ */
+#endif	/* __KERNEL__ */

Re: RFC: [PATCH] platform device driver model support

[Eugene Surovegin]

On Wed, Jan 12, 2005 at 01:43:09AM -0600, Kumar Gala wrote:
> 
> Please take a look at the following patch.  It adds driver model support 
> via platform devices to 85xx.  This is originally based on patches from 
> Jason M. 
> 
> The idea behind the code is that for a give family: 4xx, 8xx, 82xx, 83xx, 
> 85xx, 86xx we will have structure defns for the following:
> 
> enum ppc_soc_devices 
> in asm-ppc/<family.h>:
>   list of all unique devices in the family
> 
> struct platform_device soc_platform_devices[]
> in arch/ppc/platforms/<family>/<family>_devices.c:
>   describes all platform devices that exist in the family
> 
> struct soc_spec soc_specs[]
> in arch/ppc/platforms/<family>/<family>_soc.c:
>   describes each unique chip in the family and what devices it has

Well, there is a problem right here at least for 4xx. 
Current OCP implementation is much more flexible IMHO.

For 4xx is not uncommon when you have the same "logical" device at the 
different places with different "properties" (e.h. different channel, 
etc).

Your case (85xx) looks simpler - all you need is a list of devices 
which particular SoC supports, without significant differences in 
"properties". This will not work that easy for 4xx.

In fact, I don't see any gain (at least for 4xx) in all these changes. 
It makes 4xx much more tangled IMHO, because we'll still need all 
those ibm405gp.c, ibm405ep.c, ibm440gp.c etc.

Please note, using platform_device is orthogonal to the way we 
describe each SoC (this is what I don't quite like in your patch), and 
I don't have any strong objections to using platform_device instead of 
OCP or feature_call or whatever for communication with device drivers.

> Plus the following functions:
> 
> identify_soc_by_id() -- determine soc by an int id
> identify_soc_by_name() -- determin soc by name (useful in some 82xx cases)
> ppc_soc_get_pdata() -- get platform_data pointer so board code can modify
> ppc_soc_update_paddr() -- update iomem resources with a given paddr

IMHO, ppc_soc_update_paddr - is a very confusing name, in fact, from 
first read I though it _changes_ paddr to the new value, not _adds_ it 
:)

Probably this function should be made 85xx specific as I cannot come 
up with any use for it in 4xx (we don't have anything similar to 
CCSRBAR ;).


[snip]

> +
> +struct platform_device soc_platform_devices[] = {
> +	[MPC85xx_TSEC1] = {
> +		.name = "fsl-gianfar",
> +		.id	= 1,
> +		.dev.platform_data = &mpc85xx_tsec1_pdata,
> +		.num_resources	 = 4,
> +		.resource = (struct resource[]) {
> +			{
> +				.start	= MPC85xx_ENET1_OFFSET,
> +				.end	= MPC85xx_ENET1_OFFSET +
> +						MPC85xx_ENET1_SIZE - 1,
> +				.flags	= IORESOURCE_MEM,
> +			},
> +			{
> +				.name	= "tx",
> +				.start	= MPC85xx_IRQ_TSEC1_TX,
> +				.end	= MPC85xx_IRQ_TSEC1_TX,
> +				.flags	= IORESOURCE_IRQ,
> +			},
> +			{
> +				.name	= "rx",
> +				.start	= MPC85xx_IRQ_TSEC1_RX,
> +				.end	= MPC85xx_IRQ_TSEC1_RX,
> +				.flags	= IORESOURCE_IRQ,
> +			},
> +			{
> +				.name	= "error",
> +				.start	= MPC85xx_IRQ_TSEC1_ERROR,
> +				.end	= MPC85xx_IRQ_TSEC1_ERROR,
> +				.flags	= IORESOURCE_IRQ,
> +			},
[snip]


I already wrote about this but repeat again :(. 

Why put all these defines (e.g. for memory regions) into header when 
the only user is this particular file. It doesn't help readability and 
only obfuscates sources (and 4xx is a very good example of such mess 
:)


--
Eugene

Re: RFC: [PATCH] platform device driver model support

[Kumar Gala]

On Jan 12, 2005, at 2:36 AM, Eugene Surovegin wrote:

> On Wed, Jan 12, 2005 at 01:43:09AM -0600, Kumar Gala wrote:
>  >
> > Please take a look at the following patch.=A0 It adds driver model=20=

> support
> > via platform devices to 85xx.=A0 This is originally based on patches=20=

> from
> > Jason M.
> >
> > The idea behind the code is that for a give family: 4xx, 8xx, 82xx,=20=

> 83xx,
> > 85xx, 86xx we will have structure defns for the following:
>  >
> > enum ppc_soc_devices
>  > in asm-ppc/<family.h>:
> >=A0=A0 list of all unique devices in the family
>  >
> > struct platform_device soc_platform_devices[]
> > in arch/ppc/platforms/<family>/<family>_devices.c:
> >=A0=A0 describes all platform devices that exist in the family
>  >
> > struct soc_spec soc_specs[]
>  > in arch/ppc/platforms/<family>/<family>_soc.c:
> >=A0=A0 describes each unique chip in the family and what devices it =
has
>
> Well, there is a problem right here at least for 4xx.
> Current OCP implementation is much more flexible IMHO.
>
> For 4xx is not uncommon when you have the same "logical" device at the
> different places with different "properties" (e.h. different channel,
> etc).
>
> Your case (85xx) looks simpler - all you need is a list of devices
> which particular SoC supports, without significant differences in
> "properties". This will not work that easy for 4xx.
>
> In fact, I don't see any gain (at least for 4xx) in all these changes.
> It makes 4xx much more tangled IMHO, because we'll still need all
> those ibm405gp.c, ibm405ep.c, ibm440gp.c etc.
>
> Please note, using platform_device is orthogonal to the way we
> describe each SoC (this is what I don't quite like in your patch), and
> I don't have any strong objections to using platform_device instead of
> OCP or feature_call or whatever for communication with device drivers.

I need to understand a bit more about how 4xx does things.  When I=20
started the SOC stuff, it was freescale specific.  I agree that its=20
orthogonal to the use of platform device.  Does this some like a bad=20
idea for the fsl case?

> > Plus the following functions:
>  >
> > identify_soc_by_id() -- determine soc by an int id
> > identify_soc_by_name() -- determin soc by name (useful in some 82xx=20=

> cases)
>  > ppc_soc_get_pdata() -- get platform_data pointer so board code can=20=

> modify
>  > ppc_soc_update_paddr() -- update iomem resources with a given paddr
>
> IMHO, ppc_soc_update_paddr - is a very confusing name, in fact, from
> first read I though it _changes_ paddr to the new value, not _adds_ it
> :)
>
> Probably this function should be made 85xx specific as I cannot come
> up with any use for it in 4xx (we don't have anything similar to
> CCSRBAR ;).

Not an issue, any suggestions on renaming to make it clear what it=20
does?  Also, I can make this fsl_increment_paddr() since its useful to=20=

more than on family of fsl processors.

> [snip]
>
> > +
>  > +struct platform_device soc_platform_devices[] =3D {
>  > +=A0=A0=A0=A0 [MPC85xx_TSEC1] =3D {
>  > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 .name =3D "fsl-gianfar",
>  > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 .id=A0=A0=A0=A0 =3D 1,
>  > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 .dev.platform_data =3D =
&mpc85xx_tsec1_pdata,
> > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 .num_resources=A0=A0 =3D 4,
>  > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 .resource =3D (struct =
resource[]) {
>  > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 {
>  > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=
=A0=A0=A0 .start=A0 =3D MPC85xx_ENET1_OFFSET,
> > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=
=A0=A0=A0 .end=A0=A0=A0 =3D MPC85xx_ENET1_OFFSET +
>  > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=
=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 MPC85xx_ENET1_SIZE =
-=20
> 1,
>  > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=
=A0=A0=A0 .flags=A0 =3D IORESOURCE_MEM,
> > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 },
>  > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 {
>  > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=
=A0=A0=A0 .name=A0=A0 =3D "tx",
>  > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=
=A0=A0=A0 .start=A0 =3D MPC85xx_IRQ_TSEC1_TX,
> > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=
=A0=A0=A0 .end=A0=A0=A0 =3D MPC85xx_IRQ_TSEC1_TX,
> > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=
=A0=A0=A0 .flags=A0 =3D IORESOURCE_IRQ,
> > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 },
>  > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 {
>  > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=
=A0=A0=A0 .name=A0=A0 =3D "rx",
>  > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=
=A0=A0=A0 .start=A0 =3D MPC85xx_IRQ_TSEC1_RX,
> > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=
=A0=A0=A0 .end=A0=A0=A0 =3D MPC85xx_IRQ_TSEC1_RX,
> > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=
=A0=A0=A0 .flags=A0 =3D IORESOURCE_IRQ,
> > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 },
>  > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 {
>  > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=
=A0=A0=A0 .name=A0=A0 =3D "error",
>  > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=
=A0=A0=A0 .start=A0 =3D MPC85xx_IRQ_TSEC1_ERROR,
> > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=
=A0=A0=A0 .end=A0=A0=A0 =3D MPC85xx_IRQ_TSEC1_ERROR,
> > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=
=A0=A0=A0 .flags=A0 =3D IORESOURCE_IRQ,
> > +=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 },
>  [snip]
>
>
>
> I already wrote about this but repeat again :(.
>
>  Why put all these defines (e.g. for memory regions) into header when
> the only user is this particular file. It doesn't help readability and
>  only obfuscates sources (and 4xx is a very good example of such mess
> :)

Understood, I forgot about this.  I've got now issue with changing it,=20=

just trying to minimize changes.

thanks

- kumar

Re: RFC: [PATCH] platform device driver model support

[Matt Porter]

On Wed, Jan 12, 2005 at 12:36:37AM -0800, Eugene Surovegin wrote:
> On Wed, Jan 12, 2005 at 01:43:09AM -0600, Kumar Gala wrote:
> > 
> > Please take a look at the following patch.  It adds driver model support 
> > via platform devices to 85xx.  This is originally based on patches from 
> > Jason M. 
> > 
> > The idea behind the code is that for a give family: 4xx, 8xx, 82xx, 83xx, 
> > 85xx, 86xx we will have structure defns for the following:
> > 
> > enum ppc_soc_devices 
> > in asm-ppc/<family.h>:
> >   list of all unique devices in the family
> > 
> > struct platform_device soc_platform_devices[]
> > in arch/ppc/platforms/<family>/<family>_devices.c:
> >   describes all platform devices that exist in the family
> > 
> > struct soc_spec soc_specs[]
> > in arch/ppc/platforms/<family>/<family>_soc.c:
> >   describes each unique chip in the family and what devices it has
> 
> Well, there is a problem right here at least for 4xx. 
> Current OCP implementation is much more flexible IMHO.
> 
> For 4xx is not uncommon when you have the same "logical" device at the 
> different places with different "properties" (e.h. different channel, 
> etc).
> 
> Your case (85xx) looks simpler - all you need is a list of devices 
> which particular SoC supports, without significant differences in 
> "properties". This will not work that easy for 4xx.
> 
> In fact, I don't see any gain (at least for 4xx) in all these changes. 
> It makes 4xx much more tangled IMHO, because we'll still need all 
> those ibm405gp.c, ibm405ep.c, ibm440gp.c etc.

Summarizing some stuff from IRC (now that I'm caught up after time
off :P), I think we can live with this on 4xx. What seems to be
acceptable is that we can have an soc_specs[] and soc_platform_devices[]
in each 4xx SoC platform file. So, core_ocp[] can be merged and split
into soc_specs[]/soc_platform_devices[]. The active one will be selected
at build time just like we do now. This is due to the static nature
of the 4xx memory map (per SoC) and well as the variation in location
of iomem and irq resources as well as platform_data. Our soc_specs[]
will only have one SoC in it, since there is one per file. Doing
something like 85xx will scatter info about as you point out
above...and that doesn't make sense for 4xx.

> Please note, using platform_device is orthogonal to the way we 
> describe each SoC (this is what I don't quite like in your patch), and 
> I don't have any strong objections to using platform_device instead of 
> OCP or feature_call or whatever for communication with device drivers.

Hurray. ;)
 
> > Plus the following functions:
> > 
> > identify_soc_by_id() -- determine soc by an int id
> > identify_soc_by_name() -- determin soc by name (useful in some 82xx cases)
> > ppc_soc_get_pdata() -- get platform_data pointer so board code can modify
> > ppc_soc_update_paddr() -- update iomem resources with a given paddr
> 
> IMHO, ppc_soc_update_paddr - is a very confusing name, in fact, from 
> first read I though it _changes_ paddr to the new value, not _adds_ it 
> :)

I think it should be ppc_soc_update_mem_resource() or some such.
You are updating the IOMEM resource.
 
> Probably this function should be made 85xx specific as I cannot come 
> up with any use for it in 4xx (we don't have anything similar to 
> CCSRBAR ;).

Can't be, it can be used on Marvell and others.

> [snip]
> 
> > +
> > +struct platform_device soc_platform_devices[] = {
> > +	[MPC85xx_TSEC1] = {
> > +		.name = "fsl-gianfar",
> > +		.id	= 1,
> > +		.dev.platform_data = &mpc85xx_tsec1_pdata,
> > +		.num_resources	 = 4,
> > +		.resource = (struct resource[]) {
> > +			{
> > +				.start	= MPC85xx_ENET1_OFFSET,
> > +				.end	= MPC85xx_ENET1_OFFSET +
> > +						MPC85xx_ENET1_SIZE - 1,
> > +				.flags	= IORESOURCE_MEM,
> > +			},
> > +			{
> > +				.name	= "tx",
> > +				.start	= MPC85xx_IRQ_TSEC1_TX,
> > +				.end	= MPC85xx_IRQ_TSEC1_TX,
> > +				.flags	= IORESOURCE_IRQ,
> > +			},
> > +			{
> > +				.name	= "rx",
> > +				.start	= MPC85xx_IRQ_TSEC1_RX,
> > +				.end	= MPC85xx_IRQ_TSEC1_RX,
> > +				.flags	= IORESOURCE_IRQ,
> > +			},
> > +			{
> > +				.name	= "error",
> > +				.start	= MPC85xx_IRQ_TSEC1_ERROR,
> > +				.end	= MPC85xx_IRQ_TSEC1_ERROR,
> > +				.flags	= IORESOURCE_IRQ,
> > +			},
> [snip]
> 
> 
> I already wrote about this but repeat again :(. 
> 
> Why put all these defines (e.g. for memory regions) into header when 
> the only user is this particular file. It doesn't help readability and 
> only obfuscates sources (and 4xx is a very good example of such mess 
> :)

Having been previously convinced of this style, I'd say it's the The
Right Thing To Do(tm).

-Matt

Re: RFC: [PATCH] platform device driver model support

[Matt Porter]

On Wed, Jan 12, 2005 at 02:14:49PM -0700, Matt Porter wrote:
> On Wed, Jan 12, 2005 at 12:36:37AM -0800, Eugene Surovegin wrote:
> > IMHO, ppc_soc_update_paddr - is a very confusing name, in fact, from 
> > first read I though it _changes_ paddr to the new value, not _adds_ it 
> > :)
> 
> I think it should be ppc_soc_update_mem_resource() or some such.
> You are updating the IOMEM resource.

More IRC chatter and ppc_soc_fixup_mem_resource() seems to be more
descriptive.

-Matt

Re: RFC: [PATCH] platform device driver model support

[Mark A. Greer]

Kumar Gala wrote:

>All,
>
>Please take a look at the following patch.  It adds driver model support 
>via platform devices to 85xx.  This is originally based on patches from 
>Jason M. 
>
>The idea behind the code is that for a give family: 4xx, 8xx, 82xx, 83xx, 
>85xx, 86xx we will have structure defns for the following:
>
>enum ppc_soc_devices 
>in asm-ppc/<family.h>:
>  list of all unique devices in the family
>
>struct platform_device soc_platform_devices[]
>in arch/ppc/platforms/<family>/<family>_devices.c:
>  describes all platform devices that exist in the family
>
>struct soc_spec soc_specs[]
>in arch/ppc/platforms/<family>/<family>_soc.c:
>  describes each unique chip in the family and what devices it has
>
>Plus the following functions:
>
>identify_soc_by_id() -- determine soc by an int id
>identify_soc_by_name() -- determin soc by name (useful in some 82xx cases)
>ppc_soc_get_pdata() -- get platform_data pointer so board code can modify
>ppc_soc_update_paddr() -- update iomem resources with a given paddr
>
>Please provide feedback, I want to get this into 2.6.11 for 85xx.
>  
>

My $0.02.

I didn't go thru in complete detail but I like the idea.  I have a 
couple minor comments, though.

1) Can we pick something other than 'soc' since the Marvell bridges 
really aren't SOCs?  I don't really know what is better but just to 
throw something out, how about haing them all look like ppc_pd_xxx()?

2) In 8540_ads.c you're digging out platform_device entries and 
modifying them in your mpc8540ads_setup_arch() routine.  I think the 
platform_device "way" of doing that would be to make your mods via the 
platform_notify() hook (eventually called by device_add() which was 
ultimately called from platform_add_devices()).

Mark

Re: RFC: [PATCH] platform device driver model support

[Kumar Gala]

> My $0.02.
>
> I didn't go thru in complete detail but I like the idea.=A0 I have a
> couple minor comments, though.
>
> 1) Can we pick something other than 'soc' since the Marvell bridges
>  really aren't SOCs?=A0 I don't really know what is better but just to
> throw something out, how about haing them all look like ppc_pd_xxx()?

What about ppc_plat_xxx() or ppc_sys_xxx() [for system]?  'sys' maybe=20
more consistent with our naming conventions in that arch/ppc/platforms=20=

is more board focused, and arch/ppc/syslib is bridge and non-core chip=20=

functionality.

> 2) In 8540_ads.c you're digging out platform_device entries and
> modifying them in your mpc8540ads_setup_arch() routine.=A0 I think the
> platform_device "way" of doing that would be to make your mods via the
> platform_notify() hook (eventually called by device_add() which was
> ultimately called from platform_add_devices()).

This is problematic for some things like the updating of the IOMEM=20
resources since that needs to occur before platform_device_register is=20=

called.  However, the updates in mpc8540_ads.c could possibly handled=20
by platform_notify(), will look into that.

- kumar=

Re: RFC: [PATCH] platform device driver model support

[Mark A. Greer]

Kumar Gala wrote:

>> My $0.02.
>>
>> I didn't go thru in complete detail but I like the idea.  I have a
>> couple minor comments, though.
>>
>> 1) Can we pick something other than 'soc' since the Marvell bridges
>>  really aren't SOCs?  I don't really know what is better but just to
>> throw something out, how about haing them all look like ppc_pd_xxx()?
>
>
> What about ppc_plat_xxx() or ppc_sys_xxx() [for system]?  'sys' maybe 
> more consistent with our naming conventions in that arch/ppc/platforms 
> is more board focused, and arch/ppc/syslib is bridge and non-core chip 
> functionality. 


ppc_sys_xxx() sounds good to me.

>
>
>> 2) In 8540_ads.c you're digging out platform_device entries and
>> modifying them in your mpc8540ads_setup_arch() routine.  I think the
>> platform_device "way" of doing that would be to make your mods via the
>> platform_notify() hook (eventually called by device_add() which was
>> ultimately called from platform_add_devices()).
>
>
> This is problematic for some things like the updating of the IOMEM 
> resources since that needs to occur before platform_device_register is 
> called.


I don't think so.  In fact, the platform_notify() will be called from 
[platform_add_devices()]/platform_device_register()/device_register()/device_add()/platform_notify().  
There is an example in the bk://source.mvista.com/linux-2.5-marvell tree 
inside arch/ppc/platforms/katana.c.  It was only a suggestion anyway, so 
its not a big deal.

Mark

Re: RFC: [PATCH] platform device driver model support

[Andrew May]

On Wed, 2005-01-12 at 14:14 -0700, Matt Porter wrote:
> On Wed, Jan 12, 2005 at 12:36:37AM -0800, Eugene Surovegin wrote:
> > On Wed, Jan 12, 2005 at 01:43:09AM -0600, Kumar Gala wrote:
> > 
> > In fact, I don't see any gain (at least for 4xx) in all these changes. 
> > It makes 4xx much more tangled IMHO, because we'll still need all 
> > those ibm405gp.c, ibm405ep.c, ibm440gp.c etc.
> 
> Summarizing some stuff from IRC (now that I'm caught up after time
> off :P), I think we can live with this on 4xx. What seems to be
> acceptable is that we can have an soc_specs[] and soc_platform_devices[]
> in each 4xx SoC platform file. So, core_ocp[] can be merged and split
> into soc_specs[]/soc_platform_devices[]. The active one will be selected
> at build time just like we do now. This is due to the static nature
> of the 4xx memory map (per SoC) and well as the variation in location
> of iomem and irq resources as well as platform_data. Our soc_specs[]
> will only have one SoC in it, since there is one per file. Doing
> something like 85xx will scatter info about as you point out
> above...and that doesn't make sense for 4xx.

I would like to bring the Virtex-II Pro, into the thinking as well. It
is an FPGA around the 405 so it can be much more flexible on what makes
up the SoC.

I am stuck working on 2.4 for a non-released product (so no code to
submit) and we have 2 of these chips on 1 board.

One has only 1 UART and the other has 2. The rest of the standard
devices are the same, but they all have different IRQ mappings.

I really don't want to build 2 different kernels just to handle this. So
far I have just overwritten the OCP struct at boot time to deal with it,
based on a HW reg that tells me which chip I am running on. I also did
a kernel cmd line option saved in u-boot before I got the HW reg.

If FPGAs start to make up more of the SoC market it don't think a simple
array of the devices is a good model to have. The FPGA could be loaded
differently for special modes with a very different setup.

I am not sure what the trade off should be for the simple build time
array compared to the run time list that is built up.

Would something like this be OK?

struct chip_basic_features[] = { {}, {},....... };

struct chip_ext1_features[] = { {}, {},....... };
struct chip_ext2_features[] = { {}, {},....... };

LIST_HEAD(system_features);


board_init()
{
	list_add_tail( &system_features, &chip_basic_features );

	if( board1 ){
		list_add_tail( &system_features, &chip_ext1_features );
	}else{
		list_add_tail( &system_features, &chip_ext2_features );
	}
}

Re: RFC: [PATCH] platform device driver model support

[Kumar Gala]

Andrew,

What I have already done should be able to handle your situation,=20
assuming you can identify the two processor variants with a 32-bit=20
integer or a string.  In your board code you determine which variant=20
you are and then the device list will get registered based on that.

- kumar

On Jan 14, 2005, at 1:13 PM, Andrew May wrote:

> On Wed, 2005-01-12 at 14:14 -0700, Matt Porter wrote:
>  > On Wed, Jan 12, 2005 at 12:36:37AM -0800, Eugene Surovegin wrote:
>  > > On Wed, Jan 12, 2005 at 01:43:09AM -0600, Kumar Gala wrote:
>  > >
> > > In fact, I don't see any gain (at least for 4xx) in all these=20
> changes.
> > > It makes 4xx much more tangled IMHO, because we'll still need all
> > > those ibm405gp.c, ibm405ep.c, ibm440gp.c etc.
>  >
> > Summarizing some stuff from IRC (now that I'm caught up after time
>  > off :P), I think we can live with this on 4xx. What seems to be
>  > acceptable is that we can have an soc_specs[] and=20
> soc_platform_devices[]
> > in each 4xx SoC platform file. So, core_ocp[] can be merged and =
split
>  > into soc_specs[]/soc_platform_devices[]. The active one will be=20
> selected
> > at build time just like we do now. This is due to the static nature
>  > of the 4xx memory map (per SoC) and well as the variation in=20
> location
>  > of iomem and irq resources as well as platform_data. Our =
soc_specs[]
>  > will only have one SoC in it, since there is one per file. Doing
>  > something like 85xx will scatter info about as you point out
>  > above...and that doesn't make sense for 4xx.
>
> I would like to bring the Virtex-II Pro, into the thinking as well. It
>  is an FPGA around the 405 so it can be much more flexible on what=20
> makes
>  up the SoC.
>
> I am stuck working on 2.4 for a non-released product (so no code to
>  submit) and we have 2 of these chips on 1 board.
>
> One has only 1 UART and the other has 2. The rest of the standard
>  devices are the same, but they all have different IRQ mappings.
>
> I really don't want to build 2 different kernels just to handle this.=20=

> So
> far I have just overwritten the OCP struct at boot time to deal with=20=

> it,
>  based on a HW reg that tells me which chip I am running on. I also =
did
>  a kernel cmd line option saved in u-boot before I got the HW reg.
>
> If FPGAs start to make up more of the SoC market it don't think a=20
> simple
>  array of the devices is a good model to have. The FPGA could be =
loaded
>  differently for special modes with a very different setup.
>
> I am not sure what the trade off should be for the simple build time
>  array compared to the run time list that is built up.
>
> Would something like this be OK?
>
> struct chip_basic_features[] =3D { {}, {},....... };
>
> struct chip_ext1_features[] =3D { {}, {},....... };
>  struct chip_ext2_features[] =3D { {}, {},....... };
>
> LIST_HEAD(system_features);
>
>
>
> board_init()
>  {
>  =A0=A0=A0=A0=A0=A0=A0 list_add_tail( &system_features, =
&chip_basic_features );
>
> =A0=A0=A0=A0=A0=A0=A0 if( board1 ){
>  =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 list_add_tail( =
&system_features, &chip_ext1_features=20
> );
>  =A0=A0=A0=A0=A0=A0=A0 }else{
>  =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 list_add_tail( =
&system_features, &chip_ext2_features=20
> );
>  =A0=A0=A0=A0=A0=A0=A0 }
>  }