[PATCH] FCC: fix confused base / offset

[PATCH] FCC: fix confused base / offset

[Sascha Hauer]

Hi All,

This patch fixes a mixup in struct fs_platform_info. The struct has a field
dpram_offset which really is no offset but an io pointer to the dpram
(casted to u32).
It also has a field mem_offset which is used as the offset from the dpram
to the fcc RAM but then in turn filled into fcc.mem (casted to void __iomem *)

This patch fixes this in the way that dpram_offset holds the offset to the
fcc dpram as the name suggests and the mem_offset field is renamed to mem of
type void __iomem *.

This way we get rid of some #ifdefs and type casts.


Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 2c5388c..578ced2 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -837,13 +837,13 @@ static int __init fs_enet_of_init(void)
 			int fcc_index = *id - 1;
 			const unsigned char *mdio_bb_prop;
 
-			fs_enet_data.dpram_offset = (u32)cpm_dpram_addr(0);
+			fs_enet_data.dpram_offset = FCC_MEM_OFFSET(fcc_index);
 			fs_enet_data.rx_ring = 32;
 			fs_enet_data.tx_ring = 32;
 			fs_enet_data.rx_copybreak = 240;
 			fs_enet_data.use_napi = 0;
 			fs_enet_data.napi_weight = 17;
-			fs_enet_data.mem_offset = FCC_MEM_OFFSET(fcc_index);
+			fs_enet_data.mem = cpm_dpram_addr(0);
 			fs_enet_data.cp_page = CPM_CR_FCC_PAGE(fcc_index);
 			fs_enet_data.cp_block = CPM_CR_FCC_SBLOCK(fcc_index);
 
diff --git a/drivers/net/fs_enet/mac-fcc.c b/drivers/net/fs_enet/mac-fcc.c
index e363211..1659ac7 100644
--- a/drivers/net/fs_enet/mac-fcc.c
+++ b/drivers/net/fs_enet/mac-fcc.c
@@ -157,7 +157,7 @@ out:
 	if (fep->fcc.fcccp == NULL)
 		return -EINVAL;
 
-	fep->fcc.mem = (void __iomem *)fep->fpi->mem_offset;
+	fep->fcc.mem = fep->fpi->mem;
 	if (fep->fcc.mem == NULL)
 		return -EINVAL;
 
@@ -304,9 +304,6 @@ static void restart(struct net_device *dev)
 	fcc_enet_t __iomem *ep = fep->fcc.ep;
 	dma_addr_t rx_bd_base_phys, tx_bd_base_phys;
 	u16 paddrh, paddrm, paddrl;
-#ifndef CONFIG_PPC_CPM_NEW_BINDING
-	u16 mem_addr;
-#endif
 	const unsigned char *mac;
 	int i;
 
@@ -338,19 +335,10 @@ static void restart(struct net_device *dev)
 	 * this area.
 	 */
 
-#ifdef CONFIG_PPC_CPM_NEW_BINDING
 	W16(ep, fen_genfcc.fcc_riptr, fpi->dpram_offset);
 	W16(ep, fen_genfcc.fcc_tiptr, fpi->dpram_offset + 32);
 
 	W16(ep, fen_padptr, fpi->dpram_offset + 64);
-#else
-	mem_addr = (u32) fep->fcc.mem;	/* de-fixup dpram offset */
-
-	W16(ep, fen_genfcc.fcc_riptr, (mem_addr & 0xffff));
-	W16(ep, fen_genfcc.fcc_tiptr, ((mem_addr + 32) & 0xffff));
-
-	W16(ep, fen_padptr, mem_addr + 64);
-#endif
 
 	/* fill with special symbol...  */
 	memset_io(fep->fcc.mem + fpi->dpram_offset + 64, 0x88, 32);
diff --git a/include/linux/fs_enet_pd.h b/include/linux/fs_enet_pd.h
index 9bc045b..70080a0 100644
--- a/include/linux/fs_enet_pd.h
+++ b/include/linux/fs_enet_pd.h
@@ -128,7 +128,7 @@ struct fs_platform_info {
 	u32 clk_route;
 	u32 clk_mask;
 
-	u32 mem_offset;
+	void __iomem *mem;
 	u32 dpram_offset;
 	u32 fcc_regs_c;
 	

-- 
Pengutronix e.K. - Linux Solutions for Science and Industry
-----------------------------------------------------------
Kontakt-Informationen finden Sie im Header dieser Mail oder
auf der Webseite -> http://www.pengutronix.de/impressum/ <-

Re: [PATCH] FCC: fix confused base / offset

[Scott Wood]

On Wed, Apr 09, 2008 at 03:06:10PM +0200, Sascha Hauer wrote:
> This patch fixes a mixup in struct fs_platform_info. The struct has a field
> dpram_offset which really is no offset but an io pointer to the dpram
> (casted to u32).

Right, I didn't want to touch that as the old-binding code should be
going away soon anyway.  Does this break arch/ppc, BTW?

-Scott

Re: [PATCH] FCC: fix confused base / offset

[Sascha Hauer]

On Wed, Apr 09, 2008 at 11:11:10AM -0500, Scott Wood wrote:
> On Wed, Apr 09, 2008 at 03:06:10PM +0200, Sascha Hauer wrote:
> > This patch fixes a mixup in struct fs_platform_info. The struct has a field
> > dpram_offset which really is no offset but an io pointer to the dpram
> > (casted to u32).
> 
> Right, I didn't want to touch that as the old-binding code should be
> going away soon anyway.

Right, so it's probably not worth the effort. I stumbled on this while
porting my board to the new binding code. I was quite confused when
seeing this so I made this fix to understand what's going on here.

BTW have you tested the FCC driver with the new binding code? I do not
manage to get it working. Everything seems to be fine but
fs_enet_start_xmit does not send a packet and no interrupts are
arriving.



> Does this break arch/ppc, BTW?

Yes. arch=ppc is out of my scope, so I forget about it regularly, sorry.
It breaks arch/ppc/platforms/mpc8272ads_setup.c which can be easily
fixed though. But as the mpc8272ads is supported with arch=powerpc,
shouldn't it go away anyway?

Sascha



-- 
Pengutronix e.K. - Linux Solutions for Science and Industry
-----------------------------------------------------------
Kontakt-Informationen finden Sie im Header dieser Mail oder
auf der Webseite -> http://www.pengutronix.de/impressum/ <-

Re: [PATCH] FCC: fix confused base / offset

[Scott Wood]

Sascha Hauer wrote:
> Right, so it's probably not worth the effort. I stumbled on this while
> porting my board to the new binding code. I was quite confused when
> seeing this so I made this fix to understand what's going on here.
> 
> BTW have you tested the FCC driver with the new binding code?

Yes.

> I do not
> manage to get it working. Everything seems to be fine but
> fs_enet_start_xmit does not send a packet and no interrupts are
> arriving.

What does your device tree look like?  Are all the pins and clocks set 
up properly?  Does the PHY negotiate OK?  Is any error reported in the 
descriptor?

>> Does this break arch/ppc, BTW?
> 
> Yes. arch=ppc is out of my scope, so I forget about it regularly, sorry.
> It breaks arch/ppc/platforms/mpc8272ads_setup.c which can be easily
> fixed though. But as the mpc8272ads is supported with arch=powerpc,
> shouldn't it go away anyway?

arch/ppc is going away entirely in June or so.  !CONFIG_PPC_NEW_BINDING 
in arch/powerpc is going away as soon as I can unlazy myself enough to 
send a patch. :-)

-Scott

Re: [PATCH] FCC: fix confused base / offset

[Sascha Hauer]

On Wed, Apr 09, 2008 at 12:39:48PM -0500, Scott Wood wrote:
> Sascha Hauer wrote:
>> Right, so it's probably not worth the effort. I stumbled on this while
>> porting my board to the new binding code. I was quite confused when
>> seeing this so I made this fix to understand what's going on here.
>>
>> BTW have you tested the FCC driver with the new binding code?
>
> Yes.
>
>> I do not
>> manage to get it working. Everything seems to be fine but
>> fs_enet_start_xmit does not send a packet and no interrupts are
>> arriving.
>
> What does your device tree look like?  

See bottom of this mail. The board really is a 8260 based board. Our
bootloader does not fill in the clock values, so they are hardcoded. I'm
not very sure about the muram entries because the dpram is organized
slightly different on the 8260. It has some dedicated FCC space and I
don't know how to properly encode this in the device tree.

> Are all the pins and clocks set up properly?

should be. I temporarily removed all gpio setup code from the board
file, so it should be same as the bootloader leaves it.

> Does the PHY negotiate OK?

Well I put some printks into the phy_read/write functions so I can say
that it at least properly talks to the phy.

> Is any error reported in the descriptor?

No


>
> arch/ppc is going away entirely in June or so.

I'm looking forward to it ;)

Sascha


/*
 * MPC8272 ADS Device Tree Source
 *
 * 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.
 */

/ {
	model = "rsdproto";
	compatible = "fsl,rsdproto";
	#address-cells = <1>;
	#size-cells = <1>;

	cpus {
		#address-cells = <1>;
		#size-cells = <0>;

		PowerPC,8260@0 {
			device_type = "cpu";
			reg = <0>;
			d-cache-line-size = <d#32>;
			i-cache-line-size = <d#32>;
			d-cache-size = <d#16384>;
			i-cache-size = <d#16384>;
			timebase-frequency = <d#125000000>;
			bus-frequency = <d#50000000>;
			clock-frequency = <d#150000000>;
		};
	};

	memory {
		device_type = "memory";
		reg = <0 8000000>;
	};

	soc@f0000000 {
		#address-cells = <1>;
		#size-cells = <1>;
		device_type = "soc";
		compatible = "fsl,mpc8272", "fsl,pq2-soc";
		ranges = <00000000 f0000000 00053000>;

		// Temporary -- will go away once kernel uses ranges for get_immrbase().
		reg = <f0000000 00053000>;

		cpm@119c0 {
			#address-cells = <1>;
			#size-cells = <1>;
			compatible = "fsl,mpc8272-cpm", "fsl,cpm2";
			reg = <119c0 30>;
			ranges;

			muram@0 {
				#address-cells = <1>;
				#size-cells = <1>;
				ranges = <0 0 10000>;

				data@0 {
					compatible = "fsl,cpm-muram-data";
					reg = <0 2000 8000 800>;
				};
			};

			brg@119f0 {
				compatible = "fsl,mpc8272-brg",
				             "fsl,cpm2-brg",
				             "fsl,cpm-brg";
				reg = <119f0 10 115f0 10>;
				clock-frequency = <d#50000000>;
			};

			serial@11a00 {
				device_type = "serial";
				compatible = "fsl,mpc8272-scc-uart",
				             "fsl,cpm2-scc-uart";
				reg = <11a00 20 8000 100>;
				interrupts = <28 8>;
				interrupt-parent = <&PIC>;
				fsl,cpm-brg = <1>;
				fsl,cpm-command = <00800000>;
			};

			serial@11a60 {
				device_type = "serial";
				compatible = "fsl,mpc8272-scc-uart",
				             "fsl,cpm2-scc-uart";
				reg = <11a60 20 8300 100>;
				interrupts = <2b 8>;
				interrupt-parent = <&PIC>;
				fsl,cpm-brg = <4>;
				fsl,cpm-command = <0ce00000>;
			};

			mdio@10d40 {
				device_type = "mdio";
				compatible = "fsl,mpc8272ads-mdio-bitbang",
				             "fsl,mpc8272-mdio-bitbang",
				             "fsl,cpm2-mdio-bitbang";
				reg = <10d40 14>;
				#address-cells = <1>;
				#size-cells = <0>;
				fsl,mdio-pin = <09>;
				fsl,mdc-pin = <0a>;

				PHY0: ethernet-phy@0 {
					interrupt-parent = <&PIC>;
					interrupts = <17 4>;
					reg = <0>;
					device_type = "ethernet-phy";
				};
			};

			ethernet@11300 {
				device_type = "network";
				compatible = "fsl,mpc8272-fcc-enet",
				             "fsl,cpm2-fcc-enet";
				reg = <11320 20 8500 100 113b0 30>;
				local-mac-address = [ 80 10 20 30 40 50 ];
				interrupts = <21 2>;
				interrupt-parent = <&PIC>;
				phy-handle = <&PHY0>;
				linux,network-index = <0>;
				fsl,cpm-command = <16200300>;
			};
		};

		PIC: interrupt-controller@10c00 {
			#interrupt-cells = <2>;
			interrupt-controller;
			reg = <10c00 80>;
			compatible = "fsl,mpc8272-pic", "fsl,cpm2-pic";
		};

	};
};


-- 
Pengutronix e.K. - Linux Solutions for Science and Industry
-----------------------------------------------------------
Kontakt-Informationen finden Sie im Header dieser Mail oder
auf der Webseite -> http://www.pengutronix.de/impressum/ <-

Re: [PATCH] FCC: fix confused base / offset

[Scott Wood]

Sascha Hauer wrote:
> See bottom of this mail. The board really is a 8260 based board. Our
> bootloader does not fill in the clock values, so they are hardcoded. I'm
> not very sure about the muram entries because the dpram is organized
> slightly different on the 8260. It has some dedicated FCC space and I
> don't know how to properly encode this in the device tree.

I think the FCC space should just be left out.

>> Does the PHY negotiate OK?
> 
> Well I put some printks into the phy_read/write functions so I can say
> that it at least properly talks to the phy.

Do you get a console message indicating that the link came up?

> 	soc@f0000000 {
> 		#address-cells = <1>;
> 		#size-cells = <1>;
> 		device_type = "soc";
> 		compatible = "fsl,mpc8272", "fsl,pq2-soc";

Change the 8272 references to 8260.

> 		// Temporary -- will go away once kernel uses ranges for get_immrbase().
> 		reg = <f0000000 00053000>;

This can go away now.

> 		cpm@119c0 {
> 			#address-cells = <1>;
> 			#size-cells = <1>;
> 			compatible = "fsl,mpc8272-cpm", "fsl,cpm2";
> 			reg = <119c0 30>;
> 			ranges;
> 
> 			muram@0 {
> 				#address-cells = <1>;
> 				#size-cells = <1>;
> 				ranges = <0 0 10000>;
> 
> 				data@0 {
> 					compatible = "fsl,cpm-muram-data";
> 					reg = <0 2000 8000 800>;

reg should be <0 4000>.  Don't include parameter RAM here.

> 			ethernet@11300 {

ethernet@11320

> 				device_type = "network";
> 				compatible = "fsl,mpc8272-fcc-enet",
> 				             "fsl,cpm2-fcc-enet";
> 				reg = <11320 20 8500 100 113b0 30>;

reg = <11320 20 8500 100 113b0 1>;

> 				local-mac-address = [ 80 10 20 30 40 50 ];
> 				interrupts = <21 2>;

interrupts = <21 8>;

-Scott

Re: [PATCH] FCC: fix confused base / offset

[Sascha Hauer]

On Thu, Apr 10, 2008 at 11:38:00AM -0500, Scott Wood wrote:

Hi Scott,

Thank you for your help so far

> Sascha Hauer wrote:
>> See bottom of this mail. The board really is a 8260 based board. Our
>> bootloader does not fill in the clock values, so they are hardcoded. I'm
>> not very sure about the muram entries because the dpram is organized
>> slightly different on the 8260. It has some dedicated FCC space and I
>> don't know how to properly encode this in the device tree.
>
> I think the FCC space should just be left out.

The old binding used the dpram offset 0xb080 for fcc2 while the new
binding uses cpm_dpalloc which returns 0x80. When I use the old binding
and change the hardcoded value from 0xb080 to 0x80 the fcc stopped
working. I then hardcoded the value for the new binding to the same
value as the old binding used, 0xb080, but no success.

>
>>> Does the PHY negotiate OK?
>>
>> Well I put some printks into the phy_read/write functions so I can say
>> that it at least properly talks to the phy.
>
> Do you get a console message indicating that the link came up?

No, but I didn't get a message for the old binding, too.

I changed the device tree as you suggested, but still no success.

BTW there is one thing I forgot to mention which could throw some light
into this. This commit broke the FCC driver for me although it does the
right thing:

commit c6565331b7162a8348c70c37b4c33bedb6d4f02d
Author: Scott Wood <scottwood@freescale.com>
Date:   Mon Oct 1 14:20:50 2007 -0500

    fs_enet: mac-fcc: Eliminate __fcc-* macros.

    These macros accomplish nothing other than defeating type checking.

    This patch also fixes one instance of the wrong register size being
    used that was revealed by enabling type checking.

    Signed-off-by: Scott Wood <scottwood@freescale.com>
    Signed-off-by: Jeff Garzik <jeff@garzik.org>

It fixes an access to the ftodr register in tx_kickstart(). After ftodr
access the next console message is truncated and my bdi2000 shows me that
the processor doesn't get out of cpm_uart_console_write(). Something
strange is going on here...

Sascha



-- 
Pengutronix e.K. - Linux Solutions for Science and Industry
-----------------------------------------------------------
Kontakt-Informationen finden Sie im Header dieser Mail oder
auf der Webseite -> http://www.pengutronix.de/impressum/ <-