linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 2.6.14-rc3 1/1] chrp_pegasos_eth: Added Marvell Discovery II SRAM support
@ 2005-10-13 20:00 Nicolas DET
  2005-10-14  6:50 ` Nicolas DET
  2005-10-14 13:56 ` Dale Farnsworth
  0 siblings, 2 replies; 7+ messages in thread
From: Nicolas DET @ 2005-10-13 20:00 UTC (permalink / raw)
  To: Sven Luther, Andrew Morton, linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 183 bytes --]

Added proper entry to support the Marvell MV64361 (Marvell Discovery II)
SRAM.

This feature may be used by the mv643xx_eth driver.

Signed-off-by: Nicolas DET <det.nicolas@free.fr)


[-- Attachment #2: chrp_pegasos_eth.2.6.14-rc4.patch --]
[-- Type: text/x-patch, Size: 5110 bytes --]

diff -ur a/arch/ppc/platforms/chrp_pegasos_eth.c b/arch/ppc/platforms/chrp_pegasos_eth.c
--- a/arch/ppc/platforms/chrp_pegasos_eth.c	2005-10-13 21:19:46.590643250 +0200
+++ b/arch/ppc/platforms/chrp_pegasos_eth.c	2005-10-13 21:29:54.976665000 +0200
@@ -17,6 +17,21 @@
 #include <linux/mv643xx.h>
 #include <linux/pci.h>
 
+// Pegasos II location and size for the SRAM stuff
+// Only used for the ethernet driver ATM
+#define PEGASOS2_MARVELL_REGBASE 		(0xf1000000)
+#define PEGASOS2_MARVELL_REGSIZE 		(0x00004000)
+#define PEGASOS2_SRAM_BASE 			(0xf2000000)
+#define PEGASOS2_SRAM_SIZE			(0x00040000)
+
+#define PEGASOS2_SRAM_BASE_ETH0			(PEGASOS2_SRAM_BASE)
+#define PEGASOS2_SRAM_BASE_ETH1			(PEGASOS2_SRAM_BASE_ETH0 + (PEGASOS2_SRAM_SIZE / 2) )
+
+#define PEGASOS2_SRAM_RXRING_SIZE		(PEGASOS2_SRAM_SIZE/4)
+#define PEGASOS2_SRAM_TXRING_SIZE		(PEGASOS2_SRAM_SIZE/4)
+
+#undef BE_VERBOSE
+
 /* Pegasos 2 specific Marvell MV 64361 gigabit ethernet port setup */
 static struct resource mv643xx_eth_shared_resources[] = {
 	[0] = {
@@ -44,7 +59,15 @@
 	},
 };
 
-static struct mv643xx_eth_platform_data eth0_pd;
+static struct mv643xx_eth_platform_data eth0_pd = {
+	.tx_sram_addr = PEGASOS2_SRAM_BASE_ETH0,
+	.tx_sram_size = PEGASOS2_SRAM_TXRING_SIZE,
+	.tx_queue_size = PEGASOS2_SRAM_TXRING_SIZE/16,
+
+	.rx_sram_addr = PEGASOS2_SRAM_BASE + PEGASOS2_SRAM_TXRING_SIZE,
+	.rx_sram_size = PEGASOS2_SRAM_RXRING_SIZE,
+	.rx_queue_size = PEGASOS2_SRAM_RXRING_SIZE/16,
+};
 
 static struct platform_device eth0_device = {
 	.name		= MV643XX_ETH_NAME,
@@ -65,7 +88,15 @@
 	},
 };
 
-static struct mv643xx_eth_platform_data eth1_pd;
+static struct mv643xx_eth_platform_data eth1_pd = {
+	.tx_sram_addr = PEGASOS2_SRAM_BASE_ETH1,
+	.tx_sram_size = PEGASOS2_SRAM_TXRING_SIZE,
+	.tx_queue_size = PEGASOS2_SRAM_TXRING_SIZE/16,
+	
+	.rx_sram_addr = PEGASOS2_SRAM_BASE_ETH1 + PEGASOS2_SRAM_TXRING_SIZE,
+	.rx_sram_size = PEGASOS2_SRAM_RXRING_SIZE,
+	.rx_queue_size = PEGASOS2_SRAM_RXRING_SIZE/16,
+};
 
 static struct platform_device eth1_device = {
 	.name		= MV643XX_ETH_NAME,
@@ -83,19 +114,111 @@
 	&eth1_device,
 };
 
+/***********/
+/***********/
+#define MV_READ(offset,val) 	{ val = readl(mv643xx_reg_base + offset); }
+#define MV_WRITE(offset,data) writel(data, mv643xx_reg_base + offset)
+
+static void __iomem *mv643xx_reg_base = NULL;
 
-int
-mv643xx_eth_add_pds(void)
+
+static int Enable_SRAM(void)
+{
+	u32 ALong;
+	
+	// Let's io remap the mv register to touch the SRAM config
+	if (mv643xx_reg_base == NULL)
+		mv643xx_reg_base = ioremap(PEGASOS2_MARVELL_REGBASE, PEGASOS2_MARVELL_REGSIZE);
+
+	if (mv643xx_reg_base == NULL)
+	return -ENOMEM;
+
+#ifdef BE_VERBOSE
+	printk("Pegasos II/Marvell MV64361: register remapped from %p to %p\n", (void *)PEGASOS2_MARVELL_REGBASE, (void *)mv643xx_reg_base);
+#endif
+
+	// First the SRAM config register
+	// We set it to 0 ATM -> No cache coherency, no parity check
+	MV_WRITE(MV64340_SRAM_CONFIG, 0);
+
+	// set the SRAM address on the CPU side
+	MV_WRITE(MV64340_INTEGRATED_SRAM_BASE_ADDR, PEGASOS2_SRAM_BASE >> 16);
+
+	// Now enable it (CPU side)
+	MV_READ(MV64340_BASE_ADDR_ENABLE, ALong);
+	ALong &= ~(1 << 19);
+	MV_WRITE(MV64340_BASE_ADDR_ENABLE, ALong);
+
+	// And now to the GB side on WB 4 (0->3) can be use for DRAM stuff
+	ALong = 0x02; // Integrated SRAM value
+	ALong |= PEGASOS2_SRAM_BASE & 0xffff0000; // Finally set the SRAM adress in the uppter par of the register
+	MV_WRITE(MV643XX_ETH_BAR_4, ALong);
+
+	// and the size ...
+	MV_WRITE(MV643XX_ETH_SIZE_REG_4, PEGASOS2_SRAM_SIZE & 0xffff0000);
+
+	// Finaly enable the window
+	MV_READ(MV643XX_ETH_BASE_ADDR_ENABLE_REG, ALong);
+	ALong &= ~(1 << 4);
+	MV_WRITE(MV643XX_ETH_BASE_ADDR_ENABLE_REG, ALong);
+
+#ifdef BE_VERBOSE
+	printk("Pegasos II/Marvell MV64361: register unmapped\n");
+	printk("Pegasos II/Marvell MV64361: SRAM at %p, size=%x\n", (void*) PEGASOS2_SRAM_BASE, PEGASOS2_SRAM_SIZE);
+#endif
+
+	iounmap(mv643xx_reg_base);
+	mv643xx_reg_base = NULL;
+
+	return 1;
+}
+
+
+/***********/
+/***********/
+int mv643xx_eth_add_pds(void)
 {
 	int ret = 0;
-	static struct pci_device_id pci_marvell_mv64360[] = {
+	static struct pci_device_id pci_marvell_mv64360[] =
+	{
 		{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL, PCI_DEVICE_ID_MARVELL_MV64360) },
 		{ }
 	};
 
-	if (pci_dev_present(pci_marvell_mv64360)) {
+#ifdef BE_VERBOSE
+	printk("Pegasos II/Marvell MV64361: init\n");
+#endif
+
+	if (pci_dev_present(pci_marvell_mv64360))
+	{
 		ret = platform_add_devices(mv643xx_eth_pd_devs, ARRAY_SIZE(mv643xx_eth_pd_devs));
+		
+		if ( Enable_SRAM() < 0)
+		{
+			// Humm, disable SRAM stuff
+			eth0_pd.tx_sram_addr = 0;
+			eth0_pd.tx_sram_size = 0;
+			eth0_pd.rx_sram_addr = 0;
+			eth0_pd.rx_sram_size = 0;
+			
+			eth1_pd.tx_sram_addr = 0;
+			eth1_pd.tx_sram_size = 0;
+			eth1_pd.rx_sram_addr = 0;
+			eth1_pd.rx_sram_size = 0;
+			
+#ifdef BE_VERBOSE
+			printk("Pegasos II/Marvell MV64361: Can't enable the SRAM\n");
+#endif
+		}	
 	}
+	
+#ifdef BE_VERBOSE
+		
+	printk("Pegasos II/Marvell MV64361: init is over\n");
+#endif
+	
 	return ret;
 }
+
 device_initcall(mv643xx_eth_add_pds);
+

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2.6.14-rc3 1/1] chrp_pegasos_eth: Added Marvell Discovery II SRAM support
  2005-10-13 20:00 [PATCH 2.6.14-rc3 1/1] chrp_pegasos_eth: Added Marvell Discovery II SRAM support Nicolas DET
@ 2005-10-14  6:50 ` Nicolas DET
  2005-10-14 13:56 ` Dale Farnsworth
  1 sibling, 0 replies; 7+ messages in thread
From: Nicolas DET @ 2005-10-14  6:50 UTC (permalink / raw)
  To: Sven Luther; +Cc: Andrew Morton, linuxppc-dev

Le jeudi 13 octobre 2005 à 22:00 +0200, Nicolas DET a écrit :
> Added proper entry to support the Marvell MV64361 (Marvell Discovery II)
> SRAM.
> 
> This feature may be used by the mv643xx_eth driver.
> 
> Signed-off-by: Nicolas DET <det.nicolas@free.fr)
> 

I hope the patch is ok now. I apologized for the text format issue.

By the way, I compiled and booted the 2.6.14-rc4. It feels a bit faster
than rc3 (maybe it's just an impression). I'm also glad the Firewire
supported has been improved since rc3.

Regards
Nicolas DET

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2.6.14-rc3 1/1] chrp_pegasos_eth: Added Marvell Discovery II SRAM support
  2005-10-13 20:00 [PATCH 2.6.14-rc3 1/1] chrp_pegasos_eth: Added Marvell Discovery II SRAM support Nicolas DET
  2005-10-14  6:50 ` Nicolas DET
@ 2005-10-14 13:56 ` Dale Farnsworth
  2005-10-14 15:18   ` Sven Luther
  1 sibling, 1 reply; 7+ messages in thread
From: Dale Farnsworth @ 2005-10-14 13:56 UTC (permalink / raw)
  To: nd, linuxppc-dev

In article <1129233624.4161.30.camel@localhost.localdomain> Nicolas DET wrote:
> Added proper entry to support the Marvell MV64361 (Marvell Discovery II)
> SRAM.

Hi Nicolas,

I noticed a couple of style issues.

> +// Pegasos II location and size for the SRAM stuff

C++ style comments are discouraged in the kernel source.

> -	static struct pci_device_id pci_marvell_mv64360[] = {
> +	static struct pci_device_id pci_marvell_mv64360[] =
> +	{

> +	if (pci_dev_present(pci_marvell_mv64360))
> +	{

Open brace should not be on a line by itself except in function definitions.


-Dale Farnsworth

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2.6.14-rc3 1/1] chrp_pegasos_eth: Added Marvell Discovery II SRAM support
  2005-10-14 13:56 ` Dale Farnsworth
@ 2005-10-14 15:18   ` Sven Luther
  2005-10-14 21:42     ` Dale Farnsworth
  0 siblings, 1 reply; 7+ messages in thread
From: Sven Luther @ 2005-10-14 15:18 UTC (permalink / raw)
  To: Dale Farnsworth; +Cc: linuxppc-dev, sl

On Fri, Oct 14, 2005 at 01:56:41PM -0000, Dale Farnsworth wrote:
> > -	static struct pci_device_id pci_marvell_mv64360[] = {
> > +	static struct pci_device_id pci_marvell_mv64360[] =
> > +	{
> 
> > +	if (pci_dev_present(pci_marvell_mv64360))
> > +	{
> 
> Open brace should not be on a line by itself except in function definitions.

Oh, neat, i knew i should not use same-line-braces for function but didn't
know i should use them for non-functions :)

Friendly,

Sven Luther

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2.6.14-rc3 1/1] chrp_pegasos_eth: Added Marvell Discovery II SRAM support
@ 2005-10-14 17:38 Nicolas DET
  2005-10-14 17:45 ` Nicolas DET
  0 siblings, 1 reply; 7+ messages in thread
From: Nicolas DET @ 2005-10-14 17:38 UTC (permalink / raw)
  To: Sven Luther, Andrew Morton, linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 183 bytes --]

Added proper entry to support the Marvell MV64361 (Marvell Discovery II)
SRAM.

This feature may be used by the mv643xx_eth driver.

Signed-off-by: Nicolas DET <det.nicolas@free.fr)


[-- Attachment #2: chrp_pegasos_eth.2.6.14-rc4.patch --]
[-- Type: text/x-patch, Size: 6312 bytes --]

diff -ur a/arch/ppc/platforms/chrp_pegasos_eth.c b/arch/ppc/platforms/chrp_pegasos_eth.c
--- a/arch/ppc/platforms/chrp_pegasos_eth.c	2005-10-14 19:35:45.339667250 +0200
+++ b/arch/ppc/platforms/chrp_pegasos_eth.c	2005-10-14 19:36:29.682438500 +0200
@@ -17,8 +17,27 @@
 #include <linux/mv643xx.h>
 #include <linux/pci.h>
 
+/*
+ * Pegasos II location and size for the SRAM stuff
+ * Only used for the ethernet driver ATM
+*/
+
+#define PEGASOS2_MARVELL_REGBASE 		(0xf1000000)
+#define PEGASOS2_MARVELL_REGSIZE 		(0x00004000)
+#define PEGASOS2_SRAM_BASE 			(0xf2000000)
+#define PEGASOS2_SRAM_SIZE			(0x00040000)
+
+#define PEGASOS2_SRAM_BASE_ETH0			(PEGASOS2_SRAM_BASE)
+#define PEGASOS2_SRAM_BASE_ETH1			(PEGASOS2_SRAM_BASE_ETH0 + (PEGASOS2_SRAM_SIZE / 2) )
+
+#define PEGASOS2_SRAM_RXRING_SIZE		(PEGASOS2_SRAM_SIZE/4)
+#define PEGASOS2_SRAM_TXRING_SIZE		(PEGASOS2_SRAM_SIZE/4)
+
+#undef BE_VERBOSE
+
 /* Pegasos 2 specific Marvell MV 64361 gigabit ethernet port setup */
-static struct resource mv643xx_eth_shared_resources[] = {
+static struct resource mv643xx_eth_shared_resources[] = 
+{
 	[0] = {
 		.name	= "ethernet shared base",
 		.start	= 0xf1000000 + MV643XX_ETH_SHARED_REGS,
@@ -28,14 +47,16 @@
 	},
 };
 
-static struct platform_device mv643xx_eth_shared_device = {
+static struct platform_device mv643xx_eth_shared_device = 
+{
 	.name		= MV643XX_ETH_SHARED_NAME,
 	.id		= 0,
 	.num_resources	= ARRAY_SIZE(mv643xx_eth_shared_resources),
 	.resource	= mv643xx_eth_shared_resources,
 };
 
-static struct resource mv643xx_eth0_resources[] = {
+static struct resource mv643xx_eth0_resources[] = 
+{
 	[0] = {
 		.name	= "eth0 irq",
 		.start	= 9,
@@ -44,9 +65,19 @@
 	},
 };
 
-static struct mv643xx_eth_platform_data eth0_pd;
+static struct mv643xx_eth_platform_data eth0_pd =
+{
+	.tx_sram_addr = PEGASOS2_SRAM_BASE_ETH0,
+	.tx_sram_size = PEGASOS2_SRAM_TXRING_SIZE,
+	.tx_queue_size = PEGASOS2_SRAM_TXRING_SIZE/16,
+
+	.rx_sram_addr = PEGASOS2_SRAM_BASE + PEGASOS2_SRAM_TXRING_SIZE,
+	.rx_sram_size = PEGASOS2_SRAM_RXRING_SIZE,
+	.rx_queue_size = PEGASOS2_SRAM_RXRING_SIZE/16,
+};
 
-static struct platform_device eth0_device = {
+static struct platform_device eth0_device =
+{
 	.name		= MV643XX_ETH_NAME,
 	.id		= 0,
 	.num_resources	= ARRAY_SIZE(mv643xx_eth0_resources),
@@ -56,7 +87,8 @@
 	},
 };
 
-static struct resource mv643xx_eth1_resources[] = {
+static struct resource mv643xx_eth1_resources[] =
+{
 	[0] = {
 		.name	= "eth1 irq",
 		.start	= 9,
@@ -65,9 +97,19 @@
 	},
 };
 
-static struct mv643xx_eth_platform_data eth1_pd;
+static struct mv643xx_eth_platform_data eth1_pd =
+{
+	.tx_sram_addr = PEGASOS2_SRAM_BASE_ETH1,
+	.tx_sram_size = PEGASOS2_SRAM_TXRING_SIZE,
+	.tx_queue_size = PEGASOS2_SRAM_TXRING_SIZE/16,
+	
+	.rx_sram_addr = PEGASOS2_SRAM_BASE_ETH1 + PEGASOS2_SRAM_TXRING_SIZE,
+	.rx_sram_size = PEGASOS2_SRAM_RXRING_SIZE,
+	.rx_queue_size = PEGASOS2_SRAM_RXRING_SIZE/16,
+};
 
-static struct platform_device eth1_device = {
+static struct platform_device eth1_device =
+{
 	.name		= MV643XX_ETH_NAME,
 	.id		= 1,
 	.num_resources	= ARRAY_SIZE(mv643xx_eth1_resources),
@@ -77,25 +119,120 @@
 	},
 };
 
-static struct platform_device *mv643xx_eth_pd_devs[] __initdata = {
+static struct platform_device *mv643xx_eth_pd_devs[] __initdata =
+{
 	&mv643xx_eth_shared_device,
 	&eth0_device,
 	&eth1_device,
 };
 
+/***********/
+/***********/
+#define MV_READ(offset,val) 	{ val = readl(mv643xx_reg_base + offset); }
+#define MV_WRITE(offset,data) writel(data, mv643xx_reg_base + offset)
+
+static void __iomem *mv643xx_reg_base = NULL;
+
 
-int
-mv643xx_eth_add_pds(void)
+static int Enable_SRAM(void)
+{
+	u32 ALong;
+	
+	/* Let's io remap the mv register to touch the SRAM config */
+	if (mv643xx_reg_base == NULL)
+		mv643xx_reg_base = ioremap(PEGASOS2_MARVELL_REGBASE, PEGASOS2_MARVELL_REGSIZE);
+
+	if (mv643xx_reg_base == NULL)
+	return -ENOMEM;
+
+#ifdef BE_VERBOSE
+	printk("Pegasos II/Marvell MV64361: register remapped from %p to %p\n", (void *)PEGASOS2_MARVELL_REGBASE, (void *)mv643xx_reg_base);
+#endif
+
+	/*
+	 * First the SRAM config register
+	 * We set it to 0 ATM -> No cache coherency, no parity check
+	*/
+	MV_WRITE(MV64340_SRAM_CONFIG, 0);
+
+	/* set the SRAM address on the CPU side */
+	MV_WRITE(MV64340_INTEGRATED_SRAM_BASE_ADDR, PEGASOS2_SRAM_BASE >> 16);
+
+	/* Now enable it (CPU side) */
+	MV_READ(MV64340_BASE_ADDR_ENABLE, ALong);
+	ALong &= ~(1 << 19);
+	MV_WRITE(MV64340_BASE_ADDR_ENABLE, ALong);
+
+	/* And now to the GB side on WB 4 (0->3) can be use for DRAM stuff */
+	ALong = 0x02; // Integrated SRAM value
+	ALong |= PEGASOS2_SRAM_BASE & 0xffff0000; 
+	MV_WRITE(MV643XX_ETH_BAR_4, ALong);
+
+	/* and the size ... */
+	MV_WRITE(MV643XX_ETH_SIZE_REG_4, PEGASOS2_SRAM_SIZE & 0xffff0000);
+
+	/* Finaly enable the window */
+	MV_READ(MV643XX_ETH_BASE_ADDR_ENABLE_REG, ALong);
+	ALong &= ~(1 << 4);
+	MV_WRITE(MV643XX_ETH_BASE_ADDR_ENABLE_REG, ALong);
+
+#ifdef BE_VERBOSE
+	printk("Pegasos II/Marvell MV64361: register unmapped\n");
+	printk("Pegasos II/Marvell MV64361: SRAM at %p, size=%x\n", (void*) PEGASOS2_SRAM_BASE, PEGASOS2_SRAM_SIZE);
+#endif
+
+	iounmap(mv643xx_reg_base);
+	mv643xx_reg_base = NULL;
+
+	return 1;
+}
+
+
+/***********/
+/***********/
+int mv643xx_eth_add_pds(void)
 {
 	int ret = 0;
-	static struct pci_device_id pci_marvell_mv64360[] = {
+	static struct pci_device_id pci_marvell_mv64360[] =
+	{
 		{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL, PCI_DEVICE_ID_MARVELL_MV64360) },
 		{ }
 	};
 
-	if (pci_dev_present(pci_marvell_mv64360)) {
+#ifdef BE_VERBOSE
+	printk("Pegasos II/Marvell MV64361: init\n");
+#endif
+
+	if (pci_dev_present(pci_marvell_mv64360))
+	{
 		ret = platform_add_devices(mv643xx_eth_pd_devs, ARRAY_SIZE(mv643xx_eth_pd_devs));
+		
+		if ( Enable_SRAM() < 0)
+		{
+			// Humm, disable SRAM stuff
+			eth0_pd.tx_sram_addr = 0;
+			eth0_pd.tx_sram_size = 0;
+			eth0_pd.rx_sram_addr = 0;
+			eth0_pd.rx_sram_size = 0;
+			
+			eth1_pd.tx_sram_addr = 0;
+			eth1_pd.tx_sram_size = 0;
+			eth1_pd.rx_sram_addr = 0;
+			eth1_pd.rx_sram_size = 0;
+			
+#ifdef BE_VERBOSE
+			printk("Pegasos II/Marvell MV64361: Can't enable the SRAM\n");
+#endif
+		}	
 	}
+	
+#ifdef BE_VERBOSE
+		
+	printk("Pegasos II/Marvell MV64361: init is over\n");
+#endif
+	
 	return ret;
 }
+
 device_initcall(mv643xx_eth_add_pds);
+

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2.6.14-rc3 1/1] chrp_pegasos_eth: Added Marvell Discovery II SRAM support
  2005-10-14 17:38 Nicolas DET
@ 2005-10-14 17:45 ` Nicolas DET
  0 siblings, 0 replies; 7+ messages in thread
From: Nicolas DET @ 2005-10-14 17:45 UTC (permalink / raw)
  To: Sven Luther; +Cc: Andrew Morton, linuxppc-dev

Le vendredi 14 octobre 2005 à 19:38 +0200, Nicolas DET a écrit :
> Added proper entry to support the Marvell MV64361 (Marvell Discovery II)
> SRAM.
> 
> This feature may be used by the mv643xx_eth driver.
> 
> Signed-off-by: Nicolas DET <det.nicolas@free.fr)
> 


I hope this is ok this time.
The open brackes were not mine ;-P.

Regards
Nicolas DET

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2.6.14-rc3 1/1] chrp_pegasos_eth: Added Marvell Discovery II SRAM support
  2005-10-14 15:18   ` Sven Luther
@ 2005-10-14 21:42     ` Dale Farnsworth
  0 siblings, 0 replies; 7+ messages in thread
From: Dale Farnsworth @ 2005-10-14 21:42 UTC (permalink / raw)
  To: Sven Luther; +Cc: linuxppc-dev

On Fri, Oct 14, 2005 at 05:18:30PM +0200, Sven Luther wrote:
> On Fri, Oct 14, 2005 at 01:56:41PM -0000, Dale Farnsworth wrote:
> > > -	static struct pci_device_id pci_marvell_mv64360[] = {
> > > +	static struct pci_device_id pci_marvell_mv64360[] =
> > > +	{
> > 
> > > +	if (pci_dev_present(pci_marvell_mv64360))
> > > +	{
> > 
> > Open brace should not be on a line by itself except in function definitions.
> 
> Oh, neat, i knew i should not use same-line-braces for function but didn't
> know i should use them for non-functions :)

???

I guess what I wrote wasn't clear, since in his most recent patch,
Nicolas added more places like the above where the "{" has been moved
to a line by itself.  Just the opposite of what I meant to suggest.

>From Documentation/CodingStyle:
>		Chapter 3: Placing Braces
>
>The other issue that always comes up in C styling is the placement of
>braces.  Unlike the indent size, there are few technical reasons to
>choose one placement strategy over the other, but the preferred way, as
>shown to us by the prophets Kernighan and Ritchie, is to put the opening
>brace last on the line, and put the closing brace first, thusly:
>
>	if (x is true) {
>		we do y
>	}
>
>However, there is one special case, namely functions: they have the
>opening brace at the beginning of the next line, thus:
>
>	int function(int x)
>	{
>		body of function
>	}

-Dale

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2005-10-14 21:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-10-13 20:00 [PATCH 2.6.14-rc3 1/1] chrp_pegasos_eth: Added Marvell Discovery II SRAM support Nicolas DET
2005-10-14  6:50 ` Nicolas DET
2005-10-14 13:56 ` Dale Farnsworth
2005-10-14 15:18   ` Sven Luther
2005-10-14 21:42     ` Dale Farnsworth
  -- strict thread matches above, loose matches on Subject: below --
2005-10-14 17:38 Nicolas DET
2005-10-14 17:45 ` Nicolas DET

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).