Skip to contentSkip to conversation

Updated platform additions for 82xx

RFC PATCH In review Vitaly Bordug 3 participants 9 messages May 6, 2005

lore
Share & tools
lore

Vitaly Bordug

author
#0 lore raw

RFC PATCH 1/1 Updated platform additions for 82xx

Kumar,
This is just the base platform additions as you asked. I only included changes to mentioned files. All board-specific stuff should be redefined in the relative board-specific header if it does not suit the default values.

Signed-off-by Vitaly Bordug<[email protected]>
Signature · 1 line
Sincerely, Vitaly
arch/ppc/syslib/mpc82xx_devices.c new file +209 -0
diff -Nru a/arch/ppc/syslib/mpc82xx_devices.c b/arch/ppc/syslib/mpc82xx_devices.c
@@ -0,0 +1,209 @@
+/*
+ * arch/ppc/syslib/mpc82xx_devices.c
+ *
+ * MPC82xx Device descriptions
+ *
+ * Maintainer: Kumar Gala <[email protected]>
+ *
+ * 2005 (c) MontaVista Software, Inc. 
+ * Vitaly Bordug <[email protected]>
+ * 
+ * This file is licensed under the terms of the GNU General Public License 
+ * version 2. This program is licensed "as is" without any warranty of any 
+ * kind, whether express or implied.
+ */
+
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/ioport.h>
+#include <linux/fsl_devices.h>
+#include <asm/mpc8260.h>
+#include <asm/cpm2.h>
+#include <asm/immap_cpm2.h>
+#include <asm/irq.h>
+#include <asm/ppc_sys.h>
+#include <asm/ppcboot.h>
+
+
+/* FCC1 Clock Source Configuration.  These can be redefined in the board specific file.
+   Can only choose from CLK9-12 */
+#ifndef F1_RXCLK
+#define F1_RXCLK	12
+#endif
+
+#ifndef F1_TXCLK
+#define F1_TXCLK	11
+#endif
+
+/* FCC2 Clock Source Configuration.  These can be redefined in the board specific file.
+   Can only choose from CLK13-16 */
+#ifndef F2_RXCLK
+#define F2_RXCLK	13
+#endif
+
+#ifndef F2_TXCLK
+#define F2_TXCLK	14
+#endif
+
+/* FCC3 Clock Source Configuration.  These can be redefined in the board specific file.
+   Can only choose from CLK13-16 */
+#ifndef F3_RXCLK
+#define F3_RXCLK	15
+#endif
+
+#ifndef F3_TXCLK
+#define F3_TXCLK	16
+#endif
+
+/* MDIO and MDCK settings. These can be redefined in the board specific file.*/
+#ifndef PC_MDIO
+#define PC_MDIO		0x00000004U
+#endif
+
+#ifndef PC_MDCK
+#define PC_MDCK		0x00000020U
+#endif
+
+/* Automatically generates register configurations */
+#define PC_CLK(x)	((uint)(1<<(x-1)))	/* FCC CLK I/O ports */
+
+#define CMXFCR_RF1CS(x)	((uint)((x-5)<<27))	/* FCC1 Receive Clock Source */
+#define CMXFCR_TF1CS(x)	((uint)((x-5)<<24))	/* FCC1 Transmit Clock Source */
+#define CMXFCR_RF2CS(x)	((uint)((x-9)<<19))	/* FCC2 Receive Clock Source */
+#define CMXFCR_TF2CS(x) ((uint)((x-9)<<16))	/* FCC2 Transmit Clock Source */
+#define CMXFCR_RF3CS(x)	((uint)((x-9)<<11))	/* FCC3 Receive Clock Source */
+#define CMXFCR_TF3CS(x) ((uint)((x-9)<<8))	/* FCC3 Transmit Clock Source */
+
+#define PC_F1RXCLK	PC_CLK(F1_RXCLK)
+#define PC_F1TXCLK	PC_CLK(F1_TXCLK)
+#define CMX1_CLK_ROUTE	(CMXFCR_RF1CS(F1_RXCLK) | CMXFCR_TF1CS(F1_TXCLK))
+#define CMX1_CLK_MASK	((uint)0xff000000)
+
+#define PC_F2RXCLK	PC_CLK(F2_RXCLK)
+#define PC_F2TXCLK	PC_CLK(F2_TXCLK)
+#define CMX2_CLK_ROUTE	(CMXFCR_RF2CS(F2_RXCLK) | CMXFCR_TF2CS(F2_TXCLK))
+#define CMX2_CLK_MASK	((uint)0x00ff0000)
+
+#define PC_F3RXCLK	PC_CLK(F3_RXCLK)
+#define PC_F3TXCLK	PC_CLK(F3_TXCLK)
+#define CMX3_CLK_ROUTE	(CMXFCR_RF3CS(F3_RXCLK) | CMXFCR_TF3CS(F3_TXCLK))
+#define CMX3_CLK_MASK	((uint)0x0000ff00)
+
+/* We use offsets for IORESOURCE_MEM since we do not know at compile time
+ * what IMMRBAR is, will get fixed up by mach_mpc82xx_fixup
+ */
+
+static struct cpm2_fec_platform_data mpc82xx_fcc1_pdata = {
+	.cp_page	= CPM_CR_FCC1_PAGE,
+	.cp_block 	= CPM_CR_FCC1_SBLOCK,
+	.clk_trx 	= (PC_F1RXCLK | PC_F1TXCLK),
+	.clk_route	= CMX1_CLK_ROUTE,
+	.clk_mask	= CMX1_CLK_MASK,
+	.mdio		= PC_MDIO,
+	.mdck		= PC_MDCK,
+	
+	.device_flags	= FSL_CPM2_FEC_BRD_HAS_PHY_INTR,
+	.phyid 		= 0,
+	.interruptPHY	= SIU_INT_IRQ5,
+};
+
+static struct cpm2_fec_platform_data mpc82xx_fcc2_pdata = {
+	.cp_page	= CPM_CR_FCC2_PAGE,
+	.cp_block 	= CPM_CR_FCC2_SBLOCK,
+	.clk_trx 	= (PC_F2RXCLK | PC_F2TXCLK),
+	.clk_route	= CMX2_CLK_ROUTE,
+	.clk_mask	= CMX2_CLK_MASK,
+	.mdio		= PC_MDIO,
+	.mdck		= PC_MDCK,
+	
+	.device_flags	= FSL_CPM2_FEC_BRD_HAS_PHY_INTR,	
+	.phyid 		= 3,
+	.interruptPHY	= SIU_INT_IRQ5,
+};
+
+
+
+struct platform_device ppc_sys_platform_devices[] = {
+	[MPC82xx_FCC1] = {
+		.name = "cpm2_fcc",
+		.id	= 1,
+		.dev.platform_data = &mpc82xx_fcc1_pdata,
+		.num_resources	 = 4,
+		.resource = (struct resource[]) {
+			{
+				.name	= "fcc_regs",
+				.start	= 0x11300,
+				.end	= 0x1131f,
+				.flags	= IORESOURCE_MEM,
+			},
+			{
+				.name	= "fcc_pram",
+				.start	= 0x8400,
+				.end	= 0x84ff,
+				.flags	= IORESOURCE_MEM,
+			},
+			{
+				.name	= "fcc_mem",
+				.start	= 0x9000,
+				.end	= 0x907f,
+				.flags	= IORESOURCE_MEM,
+			},
+			{
+				.name	= "interrupt",
+				.start	= SIU_INT_FCC1,
+				.end	= SIU_INT_FCC1,
+				.flags	= IORESOURCE_IRQ,
+			},
+		},
+	},
+	[MPC82xx_FCC2] = {
+		.name = "cpm2_fcc",
+		.id	= 2,
+		.dev.platform_data = &mpc82xx_fcc2_pdata,
+		.num_resources	 = 4,
+		.resource = (struct resource[]) {
+			{
+				.name	= "fcc_regs",
+				.start	= 0x11320,
+				.end	= 0x1133f,
+				.flags	= IORESOURCE_MEM,
+			},
+			{
+				.name	= "fcc_pram",
+				.start	= 0x8500,
+				.end	= 0x85ff,
+				.flags	= IORESOURCE_MEM,
+			},
+			{
+				.name	= "fcc_mem",
+				.start	= 0x9080,
+				.end	= 0x90ff,
+				.flags	= IORESOURCE_MEM,
+			},
+			{
+				.name	= "interrupt",
+				.start	= SIU_INT_FCC2,
+				.end	= SIU_INT_FCC2,
+				.flags	= IORESOURCE_IRQ,
+			},
+		},
+	}
+};
+
+static int __init mach_mpc82xx_fixup(struct platform_device *pdev)
+{
+	ppc_sys_fixup_mem_resource(pdev, CPM_MAP_ADDR);
+	return 0;
+}
+
+static int __init mach_mpc82xx_init(void)
+{
+	if (ppc_md.progress)
+		ppc_md.progress("mach_mpc82xx_init:enter", 0);
+	ppc_sys_device_fixup = mach_mpc82xx_fixup;
+	return 0;
+}
+
+postcore_initcall(mach_mpc82xx_init);
arch/ppc/syslib/mpc82xx_sys.c new file +42 -0

42 lines not on this page · raw message

include/asm-ppc/mpc8260.h +5 -0
diff -Nru a/include/asm-ppc/mpc8260.h b/include/asm-ppc/mpc8260.h
@@ -74,6 +74,11 @@
 extern unsigned char __res[];
 #endif
 
+enum ppc_sys_devices {
+	MPC82xx_FCC1,
+	MPC82xx_FCC2,
+};
+
 #endif /* CONFIG_8260 */
 #endif /* !__ASM_PPC_MPC8260_H__ */
 #endif /* __KERNEL__ */
include/asm-ppc/ppc_sys.h +2 -0
diff -Nru a/include/asm-ppc/ppc_sys.h b/include/asm-ppc/ppc_sys.h
@@ -27,6 +27,8 @@
 #include <asm/mpc85xx.h>
 #elif defined(CONFIG_PPC_MPC52xx)
 #include <asm/mpc52xx.h>
+#elif defined(CONFIG_8260)
+#include <asm/mpc8260.h>
 #else
 #error "need definition of ppc_sys_devices"
 #endif
#1 lore raw

On Fri, May 06, 2005 at 11:54:08AM +0400, Vitaly Bordug wrote:

[snip]

arch/ppc/syslib/mpc82xx_devices.c line 110

Quoted 13 lines
arch/ppc/syslib/mpc82xx_devices.c new file +13 -0
+static struct cpm2_fec_platform_data mpc82xx_fcc1_pdata = {
+	.cp_page	= CPM_CR_FCC1_PAGE,
+	.cp_block 	= CPM_CR_FCC1_SBLOCK,
+	.clk_trx 	= (PC_F1RXCLK | PC_F1TXCLK),
+	.clk_route	= CMX1_CLK_ROUTE,
+	.clk_mask	= CMX1_CLK_MASK,
+	.mdio		= PC_MDIO,
+	.mdck		= PC_MDCK,
+	
+	.device_flags	= FSL_CPM2_FEC_BRD_HAS_PHY_INTR,
+	.phyid 		= 0,
+	.interruptPHY	= SIU_INT_IRQ5,
+};

Hmm, interruptPHY here still looks somewhat odd. Why not use the same approach as for the rest of the fields - "default" if corresponding define wasn't defined by the board port?

Not that I like this approach, IMHO there should be _NO_ defines at all here and board setup code must fill them, but I guess it's OK for now, mostly because there is no board specific files for 82xx boards yet and this makes the patch quite small.

Signature · 1 line
Eugene
author
#2 lore raw
Quoted 24 lines · Eugene Surovegin

On Fri, May 06, 2005 at 11:54:08AM +0400, Vitaly Bordug wrote:

[snip]

Quoted 5 lines
(quoted diff) +3 -0
+static struct cpm2_fec_platform_data mpc82xx_fcc1_pdata = {
+	.cp_page	= CPM_CR_FCC1_PAGE,
+	.cp_block 	= CPM_CR_FCC1_SBLOCK,

Review

Quoted 10 lines
(quoted diff) +10 -0
+	.clk_trx 	= (PC_F1RXCLK | PC_F1TXCLK),
+	.clk_route	= CMX1_CLK_ROUTE,
+	.clk_mask	= CMX1_CLK_MASK,
+	.mdio		= PC_MDIO,
+	.mdck		= PC_MDCK,
+	
+	.device_flags	= FSL_CPM2_FEC_BRD_HAS_PHY_INTR,
+	.phyid 		= 0,
+	.interruptPHY	= SIU_INT_IRQ5,
+};

Hmm, interruptPHY here still looks somewhat odd. Why not use the same approach as for the rest of the fields - "default" if corresponding define wasn't defined by the board port?

Quoted 4 lines

Not that I like this approach, IMHO there should be _NO_ defines at all here and board setup code must fill them, but I guess it's OK for now, mostly because there is no board specific files for 82xx boards yet and this makes the patch quite small.

Eugene,
Probably this is a better way?

PHY_INTERRUPT is defined in the board-specific platforms/pq2ads.h (likewise approach was used in 2.4). Others from this define crowd should reside as they are IMHO, until we have some common board-specific header for 82xx (or all these will be scattered along different 82xx platform defines such as tqm8260.h etc. which is even worse). What do you think?

Signature · 1 line
Sincerely, Vitaly
arch/ppc/syslib/mpc82xx_devices.c new file +213 -0
diff -Nru a/arch/ppc/syslib/mpc82xx_devices.c b/arch/ppc/syslib/mpc82xx_devices.c
@@ -0,0 +1,213 @@
+/*
+ * arch/ppc/syslib/mpc82xx_devices.c
+ *
+ * MPC82xx Device descriptions
+ *
+ * Maintainer: Kumar Gala <[email protected]>
+ *
+ * 2005 (c) MontaVista Software, Inc. 
+ * Vitaly Bordug <[email protected]>
+ * 
+ * This file is licensed under the terms of the GNU General Public License 
+ * version 2. This program is licensed "as is" without any warranty of any 
+ * kind, whether express or implied.
+ */
+
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/ioport.h>
+#include <linux/fsl_devices.h>
+#include <asm/mpc8260.h>
+#include <asm/cpm2.h>
+#include <asm/immap_cpm2.h>
+#include <asm/irq.h>
+#include <asm/ppc_sys.h>
+#include <asm/ppcboot.h>
+
+
+/* FCC1 Clock Source Configuration.  These can be redefined in the board specific file.
+   Can only choose from CLK9-12 */
+#ifndef F1_RXCLK
+#define F1_RXCLK	12
+#endif
+
+#ifndef F1_TXCLK
+#define F1_TXCLK	11
+#endif
+
+/* FCC2 Clock Source Configuration.  These can be redefined in the board specific file.
+   Can only choose from CLK13-16 */
+#ifndef F2_RXCLK
+#define F2_RXCLK	13
+#endif
+
+#ifndef F2_TXCLK
+#define F2_TXCLK	14
+#endif
+
+/* FCC3 Clock Source Configuration.  These can be redefined in the board specific file.
+   Can only choose from CLK13-16 */
+#ifndef F3_RXCLK
+#define F3_RXCLK	15
+#endif
+
+#ifndef F3_TXCLK
+#define F3_TXCLK	16
+#endif
+
+/* MDIO and MDCK settings. These can be redefined in the board specific file.*/
+#ifndef PC_MDIO
+#define PC_MDIO		0x00000004U
+#endif
+
+#ifndef PC_MDCK
+#define PC_MDCK		0x00000020U
+#endif
+
+/* Automatically generates register configurations */
+#define PC_CLK(x)	((uint)(1<<(x-1)))	/* FCC CLK I/O ports */
+
+#define CMXFCR_RF1CS(x)	((uint)((x-5)<<27))	/* FCC1 Receive Clock Source */
+#define CMXFCR_TF1CS(x)	((uint)((x-5)<<24))	/* FCC1 Transmit Clock Source */
+#define CMXFCR_RF2CS(x)	((uint)((x-9)<<19))	/* FCC2 Receive Clock Source */
+#define CMXFCR_TF2CS(x) ((uint)((x-9)<<16))	/* FCC2 Transmit Clock Source */
+#define CMXFCR_RF3CS(x)	((uint)((x-9)<<11))	/* FCC3 Receive Clock Source */
+#define CMXFCR_TF3CS(x) ((uint)((x-9)<<8))	/* FCC3 Transmit Clock Source */
+
+#define PC_F1RXCLK	PC_CLK(F1_RXCLK)
+#define PC_F1TXCLK	PC_CLK(F1_TXCLK)
+#define CMX1_CLK_ROUTE	(CMXFCR_RF1CS(F1_RXCLK) | CMXFCR_TF1CS(F1_TXCLK))
+#define CMX1_CLK_MASK	((uint)0xff000000)
+
+#define PC_F2RXCLK	PC_CLK(F2_RXCLK)
+#define PC_F2TXCLK	PC_CLK(F2_TXCLK)
+#define CMX2_CLK_ROUTE	(CMXFCR_RF2CS(F2_RXCLK) | CMXFCR_TF2CS(F2_TXCLK))
+#define CMX2_CLK_MASK	((uint)0x00ff0000)
+
+#define PC_F3RXCLK	PC_CLK(F3_RXCLK)
+#define PC_F3TXCLK	PC_CLK(F3_TXCLK)
+#define CMX3_CLK_ROUTE	(CMXFCR_RF3CS(F3_RXCLK) | CMXFCR_TF3CS(F3_TXCLK))
+#define CMX3_CLK_MASK	((uint)0x0000ff00)
+
+/* We use offsets for IORESOURCE_MEM since we do not know at compile time
+ * what IMMRBAR is, will get fixed up by mach_mpc82xx_fixup
+ */
+
+static struct cpm2_fec_platform_data mpc82xx_fcc1_pdata = {
+	.cp_page	= CPM_CR_FCC1_PAGE,
+	.cp_block 	= CPM_CR_FCC1_SBLOCK,
+	.clk_trx 	= (PC_F1RXCLK | PC_F1TXCLK),
+	.clk_route	= CMX1_CLK_ROUTE,
+	.clk_mask	= CMX1_CLK_MASK,
+	.mdio		= PC_MDIO,
+	.mdck		= PC_MDCK,
+	
+	.phyid 		= 0,
+#ifdef PHY_INTERRUPT
+	.device_flags	= FSL_CPM2_FEC_BRD_HAS_PHY_INTR,
+	.interruptPHY	= PHY_INTERRUPT,
+#endif
+};
+
+static struct cpm2_fec_platform_data mpc82xx_fcc2_pdata = {
+	.cp_page	= CPM_CR_FCC2_PAGE,
+	.cp_block 	= CPM_CR_FCC2_SBLOCK,
+	.clk_trx 	= (PC_F2RXCLK | PC_F2TXCLK),
+	.clk_route	= CMX2_CLK_ROUTE,
+	.clk_mask	= CMX2_CLK_MASK,
+	.mdio		= PC_MDIO,
+	.mdck		= PC_MDCK,
+	
+	.phyid 		= 3,
+#ifdef PHY_INTERRUPT
+	.device_flags	= FSL_CPM2_FEC_BRD_HAS_PHY_INTR,
+	.interruptPHY	= PHY_INTERRUPT,
+#endif
+};
+
+
+
+struct platform_device ppc_sys_platform_devices[] = {
+	[MPC82xx_FCC1] = {
+		.name = "cpm2_fcc",
+		.id	= 1,
+		.dev.platform_data = &mpc82xx_fcc1_pdata,
+		.num_resources	 = 4,
+		.resource = (struct resource[]) {
+			{
+				.name	= "fcc_regs",
+				.start	= 0x11300,
+				.end	= 0x1131f,
+				.flags	= IORESOURCE_MEM,
+			},
+			{
+				.name	= "fcc_pram",
+				.start	= 0x8400,
+				.end	= 0x84ff,
+				.flags	= IORESOURCE_MEM,
+			},
+			{
+				.name	= "fcc_mem",
+				.start	= 0x9000,
+				.end	= 0x907f,
+				.flags	= IORESOURCE_MEM,
+			},
+			{
+				.name	= "interrupt",
+				.start	= SIU_INT_FCC1,
+				.end	= SIU_INT_FCC1,
+				.flags	= IORESOURCE_IRQ,
+			},
+		},
+	},
+	[MPC82xx_FCC2] = {
+		.name = "cpm2_fcc",
+		.id	= 2,
+		.dev.platform_data = &mpc82xx_fcc2_pdata,
+		.num_resources	 = 4,
+		.resource = (struct resource[]) {
+			{
+				.name	= "fcc_regs",
+				.start	= 0x11320,
+				.end	= 0x1133f,
+				.flags	= IORESOURCE_MEM,
+			},
+			{
+				.name	= "fcc_pram",
+				.start	= 0x8500,
+				.end	= 0x85ff,
+				.flags	= IORESOURCE_MEM,
+			},
+			{
+				.name	= "fcc_mem",
+				.start	= 0x9080,
+				.end	= 0x90ff,
+				.flags	= IORESOURCE_MEM,
+			},
+			{
+				.name	= "interrupt",
+				.start	= SIU_INT_FCC2,
+				.end	= SIU_INT_FCC2,
+				.flags	= IORESOURCE_IRQ,
+			},
+		},
+	}
+};
+
+static int __init mach_mpc82xx_fixup(struct platform_device *pdev)
+{
+	ppc_sys_fixup_mem_resource(pdev, CPM_MAP_ADDR);
+	return 0;
+}
+
+static int __init mach_mpc82xx_init(void)
+{
+	if (ppc_md.progress)
+		ppc_md.progress("mach_mpc82xx_init:enter", 0);
+	ppc_sys_device_fixup = mach_mpc82xx_fixup;
+	return 0;
+}
+
+postcore_initcall(mach_mpc82xx_init);
arch/ppc/syslib/mpc82xx_sys.c new file +42 -0

42 lines not on this page · raw message

include/asm-ppc/mpc8260.h +5 -0
diff -Nru a/include/asm-ppc/mpc8260.h b/include/asm-ppc/mpc8260.h
@@ -74,6 +74,11 @@
 extern unsigned char __res[];
 #endif
 
+enum ppc_sys_devices {
+	MPC82xx_FCC1,
+	MPC82xx_FCC2,
+};
+
 #endif /* CONFIG_8260 */
 #endif /* !__ASM_PPC_MPC8260_H__ */
 #endif /* __KERNEL__ */
include/asm-ppc/ppc_sys.h +2 -0
diff -Nru a/include/asm-ppc/ppc_sys.h b/include/asm-ppc/ppc_sys.h
@@ -27,6 +27,8 @@
 #include <asm/mpc85xx.h>
 #elif defined(CONFIG_PPC_MPC52xx)
 #include <asm/mpc52xx.h>
+#elif defined(CONFIG_8260)
+#include <asm/mpc8260.h>
 #else
 #error "need definition of ppc_sys_devices"
 #endif
#3 lore raw
Quoted 6 lines · Vitaly Bordug

PHY_INTERRUPT is defined in the board-specific platforms/pq2ads.h (likewise approach was used in 2.4). Others from this define crowd should reside as they are IMHO, until we have some common board-specific header for 82xx (or all these will be scattered along different 82xx platform defines such as tqm8260.h etc. which is even worse). What do you think?

Yeah, it's better, I'm only wondering what if some board has separate PHY interrupt lines for each FCC? Define name - "PHY_INTERRUPT" was chosen quite badly IMHO :).

Signature · 1 line
Eugene
#4 lore raw
Quoted 3 lines · Eugene Surovegin

Yeah, it's better, I'm only wondering what if some board has separate PHY interrupt lines for each FCC? Define name - "PHY_INTERRUPT" was chosen quite badly IMHO :).

One of my feature_calls is:
embed_feature_call(CPM_FTR_FCC1, REQ_FTR_PHY_INT, &irq)

and so on for every Ethernet port. This presents my current coding challenge, do I want this to be a varargs function or we just always have three parameters, where the last one is ignored if not necessary, a pointer for an out value, or a pointer to a data structure for more complex information. Right now I have it coded as always three parameters.

In addition, this feature call will set up whatever interrupt routing is necessary. The driver only needs to check if a value is returned, if not, no interrupt. It also may want to test if the returned irq is the same for all PHYS, since it may be needed in the interrupt handler.

Thanks.

	-- Dan
#5 lore raw
Quoted 10 lines · Dan Malek
Quoted 3 lines · Eugene Surovegin

Yeah, it's better, I'm only wondering what if some board has separate PHY interrupt lines for each FCC? Define name - "PHY_INTERRUPT" was chosen quite badly IMHO :).

One of my feature_calls is:
embed_feature_call(CPM_FTR_FCC1, REQ_FTR_PHY_INT, &irq)

Quoted 7 lines

and so on for every Ethernet port. This presents my current coding challenge, do I want this to be a varargs function or we just always have
three parameters, where the last one is ignored if not necessary, a pointer for an out value, or a pointer to a data structure for more complex information. Right now I have it coded as always three parameters.

Yeah, I think 3 parameters approach is OK. IOCTL handlers are made like this and so far they proved to be quite flexible.

Signature · 1 line
Eugene
#6 lore raw
Quoted 2 lines · Eugene Surovegin

Yeah, I think 3 parameters approach is OK. IOCTL handlers are made like this and so far they proved to be quite flexible.

I thought we also discussed placing the PHY irq numbers in the platform data structure at one time? :-) Of course, we still need the feature_call to do any routing of the signal that may be required.

Thanks.

	-- Dan
#7 lore raw
Quoted 8 lines · Dan Malek
Quoted 2 lines · Eugene Surovegin

Yeah, I think 3 parameters approach is OK. IOCTL handlers are made like this and so far they proved to be quite flexible.

I thought we also discussed placing the PHY irq numbers in the platform data structure at one time? :-)

This is exactly what Vitaly is doing in his patches. My comment was about general use of 3 parameters "feature" calls vs variadic ones, not about you particular example for PHY IRQ :).

Signature · 1 line
Eugene
#8 lore raw
In reply to Vitaly Bordug
Quoted 7 lines · Vitaly Bordug

PHY_INTERRUPT is defined in the board-specific platforms/pq2ads.h (likewise approach was used in 2.4). Others from this define crowd should reside as they are IMHO, until we have some common board-specific header for 82xx (or all these will be scattered along different 82xx platform defines such as tqm8260.h etc. which is even worse). What do you think?

The best way I think is to NOT use defines like PHY_INTERRUPT at all. This is what I mean in my previous e-mail when I said I didn't quite like approach your chosen but thought it was OK for now.

I think that platform files should initialize only stuff which is common for all boards using this platform.

Board specific stuff should be _filled_ in board .c file. Basically we have two different ways to do board specific stuff (I think feature call stuff which Dan is proposing is orthogonal to these):

1) Add some defines with generic name like PHY_INTERRUPT_FCC1, allow board specific _header_ file to define them and use some default if it wasn't overridden by the board port. This is I believe approach you have chosen.

2) _Fill_ corresponding platform_device fields in the board C file. This is what I think is better.

Approach 1) is less intrusive because it's similar to what we were doing so far.

Approach 2) is more flexible (e.g. I wasn't to use the same kernel image for all boards from some family, and they may use different IRQ routing). Also, 2) avoids "header file hell", when board info scattered around different headers and sometime it's not that obvious where particular define comes from.

Signature · 1 line
Eugene