LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* RE: WARN_ON() hit in fsl bitbanged phy driver
From: Rune Torgersen @ 2008-02-11 22:12 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, netdev
In-Reply-To: <47B0C30A.1030801@freescale.com>

Scott Wood wrote:
> Rune Torgersen wrote:
>> I hit the following WARN_ON when using mii-tools agains a ethernet
>> interface using a bit-banged mii interface
>=20
> It looks like the kernel thinks it's in an interrupt, even though it
> clearly isn't from the backtrace.  Presumably, something
> slept from an
> interrupt handler; try turning on sleep-in-spinlock debugging.

I turned on sleep-in-spinlock and it did not reveal anything. I'm trying
some other debug options.

> The root cause was probably something other than the phy code.
I'm not so sure, because it only happens when I run mii-tool agains the
interface useing the bit-banged driver, and then only the first time.

^ permalink raw reply

* Re: MPC8272 FCC2 MDIO on Port D and FS_ENET
From: NM @ 2008-02-11 22:04 UTC (permalink / raw)
  To: linuxppc-embedded
In-Reply-To: <47B0C099.5030205@freescale.com>

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

Thanks Scott,
I do boot from U-BOOT, via Ethernet, but I'd never set the MDIO/MDC in U-BOOT. Actually, looking at the u-boot 's MPC8260ADS.h now, it is still setting the default 8272ADS boards' PC18/19 for MDC/MDIO - and my unit is using PD23/29.
I guess U-BOOT is probably working (for me) since it probably defaults to the defaults of PHY and maybe not checking for PHY errors???

I guess, it is probably worthwhile having the MDIO/MDC pins also  set up explicitly in mpc8272_ads_pins as opposed to relying on the boot loader.







Scott Wood <scottwood@freescale.com> wrote: NM wrote:
> My problem is solved now after probing the lines, MDC was not showing 
> any activity.
> So, after explicitly setting the PD23 (MDC) to : OUTPUT/GPIO in 
> mpc8272_ads_pins and  it worked in
> 
> I'd previously thought about that, but I was sure that this was being 
> taken care of, but now that I'm thinking of it, I'm questioning my belief. 
> I think in the fs_enet/mii-bitbang.c: mdio_dir(..) it is supposed to 
> take care of this.

No, the mdc pin is static; only bits which need to be changed 
dynamically are dealt with in mii-bitbang.c.  Either the firmware or the 
platform code should set up static pins.

> But why I had to explictly set it initially beats me. 
> I wonder how this works in standard distribution.

It's probably set up by u-boot normally, at least if you boot from ethernet.

-Scott


       
---------------------------------
Looking for the perfect gift? Give the gift of Flickr!

[-- Attachment #2: Type: text/html, Size: 1823 bytes --]

^ permalink raw reply

* Re: [PATCH 1/2] powerpc: publish 85xx soc dts entries as of_device
From: Jon Loeliger @ 2008-02-11 22:01 UTC (permalink / raw)
  To: Dave Jiang; +Cc: linuxppc-dev
In-Reply-To: <20080211203243.GA5331@blade.az.mvista.com>

Dave Jiang wrote:

> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ads.c b/arch/powerpc/platforms/85xx/mpc85xx_ads.c
> index 4e03050..024393c 100644
> --- a/arch/powerpc/platforms/85xx/mpc85xx_ads.c
> +++ b/arch/powerpc/platforms/85xx/mpc85xx_ads.c
> @@ -26,6 +26,8 @@
>  #include <asm/mpic.h>
>  #include <mm/mmu_decl.h>
>  #include <asm/udbg.h>
> +#include <asm/of_device.h>
> +#include <asm/of_platform.h>

Please use <linux/of_{device,platform}.h> throughout instead.

Thanks.
jdl

^ permalink raw reply

* Re: [PATCH 2/2] powerpc: create mpc85xx pci err platform device for EDAC
From: Kumar Gala @ 2008-02-11 21:56 UTC (permalink / raw)
  To: Dave Jiang; +Cc: linuxppc-dev
In-Reply-To: <20080211203445.GB5331@blade.az.mvista.com>


On Feb 11, 2008, at 2:34 PM, Dave Jiang wrote:

> Creating a platform device for the PCI error registers in order for  
> the
> mpc85xx EDAC driver to pick up proper resources. This is to prevent  
> the
> EDAC driver from monopolizing the of_device and thus preventing  
> future PCI
> code from using the PCI of_device(s).
>
> Signed-off-by: Dave Jiang <djiang@mvista.com>

I'd rather we didn't add new platform devices, but do this via  
of_platform in the driver itself.

Also, we need to 'rename' the compatible field for some of these  
devices.

- k

^ permalink raw reply

* Re: WARN_ON() hit in fsl bitbanged phy driver
From: Scott Wood @ 2008-02-11 21:50 UTC (permalink / raw)
  To: Rune Torgersen; +Cc: linuxppc-dev, netdev
In-Reply-To: <DCEAAC0833DD314AB0B58112AD99B93B03F931E6@ismail.innsys.innovsys.com>

Rune Torgersen wrote:
> I hit the following WARN_ON when using mii-tools agains a ethernet
> interface using a bit-banged mii interface
> It is only diplayed once, and does not seem to impact usage at all
> 
> Does somebody know what is wrong, and how to fix it?

It looks like the kernel thinks it's in an interrupt, even though it 
clearly isn't from the backtrace.  Presumably, something slept from an 
interrupt handler; try turning on sleep-in-spinlock debugging.

The root cause was probably something other than the phy code.

-Scott

^ permalink raw reply

* Re: MPC8272 FCC2 MDIO on Port D and FS_ENET
From: Scott Wood @ 2008-02-11 21:39 UTC (permalink / raw)
  To: NM; +Cc: linuxppc-embedded
In-Reply-To: <235903.35949.qm@web32704.mail.mud.yahoo.com>

NM wrote:
> My problem is solved now after probing the lines, MDC was not showing 
> any activity.
> So, after explicitly setting the PD23 (MDC) to : OUTPUT/GPIO in 
> mpc8272_ads_pins and  it worked in
> 
> I'd previously thought about that, but I was sure that this was being 
> taken care of, but now that I'm thinking of it, I'm questioning my belief. 
> I think in the fs_enet/mii-bitbang.c: mdio_dir(..) it is supposed to 
> take care of this.

No, the mdc pin is static; only bits which need to be changed 
dynamically are dealt with in mii-bitbang.c.  Either the firmware or the 
platform code should set up static pins.

> But why I had to explictly set it initially beats me. 
> I wonder how this works in standard distribution.

It's probably set up by u-boot normally, at least if you boot from ethernet.

-Scott

^ permalink raw reply

* WARN_ON() hit in fsl bitbanged phy driver
From: Rune Torgersen @ 2008-02-11 21:28 UTC (permalink / raw)
  To: linuxppc-dev, netdev

I hit the following WARN_ON when using mii-tools agains a ethernet
interface using a bit-banged mii interface
It is only diplayed once, and does not seem to impact usage at all

Does somebody know what is wrong, and how to fix it?
The PHY is an Intel LXT973

Badness at kernel/softirq.c:139
NIP: c001f8bc LR: c0121170 CTR: c01269d8
REGS: dfaa5ce0 TRAP: 0700   Tainted: P         (2.6.24-cpu2)
MSR: 00021032 <ME,IR,DR>  CR: 20002222  XER: 20000000
TASK =3D de0c2b40[825] 'mii-tool' THREAD: dfaa4000
GPR00: 00000001 dfaa5d90 de0c2b40 0000782d 00000000 00000005 00000001
0ff6a37c
GPR08: 00000000 c0250000 df86bb60 08000000 80000242 1001aa20 00000000
100aa258
GPR16: 10090000 00000000 10090000 00000000 10070000 100aa170 10010000
bff3bd94
GPR24: 00000003 00000000 00000001 c0123234 0000782d dfaa4000 00000000
df003a00
NIP [c001f8bc] local_bh_enable+0x28/0x9c
LR [c0121170] phy_read+0x5c/0x74
Call Trace:
[dfaa5d90] [dfaa4000] 0xdfaa4000 (unreliable)
[dfaa5da0] [c0121170] phy_read+0x5c/0x74
drivers/net/phy/phy.c:80
[dfaa5dc0] [c012140c] phy_mii_ioctl+0x64/0x170
drivers/net/phy/phy.c:368
[dfaa5de0] [c0125394] fs_ioctl+0x4c/0x8c
[dfaa5e00] [c0143744] dev_ifsioc+0x2e8/0x314
[dfaa5e20] [c0144eb8] dev_ioctl+0x6f4/0x8a8
[dfaa5ea0] [c0136aa8] sock_ioctl+0x1fc/0x21c
[dfaa5ec0] [c00742e0] do_ioctl+0x44/0xa8
[dfaa5ee0] [c00746e8] vfs_ioctl+0x3a4/0x3e8
[dfaa5f10] [c007476c] sys_ioctl+0x40/0x70
[dfaa5f40] [c000edc8] ret_from_syscall+0x0/0x38
--- Exception: c01 at 0xff6a37c
    LR =3D 0xffecd58
Instruction dump:
7c0803a6 4e800020 9421fff0 7c0802a6 90010014 7c0000a6 70098000 40a2002c
3d20c025 8009d2a8 7c000034 5400d97e <0f000000> 2f800000 41be0010
38000001

(gdb) list *0xc0121170
0xc0121170 is in phy_read (drivers/net/phy/phy.c:80).
75              spin_lock_bh(&bus->mdio_lock);
76              retval =3D bus->read(bus, phydev->addr, regnum);
77              spin_unlock_bh(&bus->mdio_lock);
78
79              return retval;
80      }
81      EXPORT_SYMBOL(phy_read);
82
83      /**
84       * phy_write - Convenience function for writing a given PHY
register
(gdb) list *0xc012140c
0xc012140c is in phy_mii_ioctl (drivers/net/phy/phy.c:368).
363             switch (cmd) {
364             case SIOCGMIIPHY:
365                     mii_data->phy_id =3D phydev->addr;
366                     break;
367             case SIOCGMIIREG:
368                     mii_data->val_out =3D phy_read(phydev,
mii_data->reg_num);
369                     break;
370
371             case SIOCSMIIREG:
372                     if (!capable(CAP_NET_ADMIN))
(gdb) list *0xc001f8bc
0xc001f8bc is in local_bh_enable (kernel/softirq.c:139).
134     #ifdef CONFIG_TRACE_IRQFLAGS
135             unsigned long flags;
136
137             WARN_ON_ONCE(in_irq());
138     #endif
139             WARN_ON_ONCE(irqs_disabled());
140
141     #ifdef CONFIG_TRACE_IRQFLAGS
142             local_irq_save(flags);
143     #endif

the relevant part of my device tree is:
ethernet@11340 {
	device_type =3D "network";
	compatible =3D "fsl,mpc8280-fcc-enet",
		     "fsl,cpm2-fcc-enet";
	reg =3D <11340 20 8600 100 113d0 1>;
	interrupts =3D <22 8>;
	interrupt-parent =3D <&PIC>;
	phy-handle =3D <&PHY1>;
	linux,network-index =3D <1>;
	fsl,cpm-command =3D <1a400300>;
	local-mac-address =3D [00 00 00 00 00 00];
};

mdio@10d40 {
	device_type =3D "mdio";
	compatible =3D "fsl,pq2fads-mdio-bitbang",
		     "fsl,mpc8280-mdio-bitbang",
		     "fsl,cpm2-mdio-bitbang";
	#address-cells =3D <1>;
	#size-cells =3D <0>;
	reg =3D <10d40 14>;
	fsl,mdio-pin =3D <5>;
	fsl,mdc-pin =3D <4>;

	PHY1: ethernet-phy@1 {
		//interrupt-parent =3D <&PIC>;
		//interrupts =3D <19 2>;
		reg =3D <1>;
		device_type =3D "ethernet-phy";
	};
};

^ permalink raw reply

* Re: MPC8272 FCC2 MDIO on Port D and FS_ENET
From: NM @ 2008-02-11 21:26 UTC (permalink / raw)
  To: linuxppc-embedded
In-Reply-To: <399508.89089.qm@web32709.mail.mud.yahoo.com>

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

My problem is solved now after probing the lines, MDC was not showing any activity.
So, after explicitly setting the PD23 (MDC) to : OUTPUT/GPIO in mpc8272_ads_pins and  it worked in 

I'd previously thought about that, but I was sure that this was being taken care of, but now that I'm thinking of it, I'm questioning my belief.  
I think in the fs_enet/mii-bitbang.c: mdio_dir(..) it is supposed to take care of this. But why I had to explictly set it initially beats me. I wonder how this works in standard distribution.

thanks


       
---------------------------------
Be smarter than spam. See how smart SpamGuard is at giving junk email the boot with the All-new Yahoo! Mail  

[-- Attachment #2: Type: text/html, Size: 770 bytes --]

^ permalink raw reply

* [PATCH 2/2] powerpc: create mpc85xx pci err platform device for EDAC
From: Dave Jiang @ 2008-02-11 20:34 UTC (permalink / raw)
  To: galak, linuxppc-dev

Creating a platform device for the PCI error registers in order for the
mpc85xx EDAC driver to pick up proper resources. This is to prevent the
EDAC driver from monopolizing the of_device and thus preventing future PCI
code from using the PCI of_device(s).

Signed-off-by: Dave Jiang <djiang@mvista.com>

---
commit 9436d0792f1352e9a034436c82b2229622fc3364
tree 0beeed478786b16cdb33be41ac803f781b8ea0cc
parent 187841bf9dff25e4ac1a7174daa55bb036c724b1
author Dave Jiang <djiang@blade.(none)> Mon, 11 Feb 2008 12:55:45 -0700
committer Dave Jiang <djiang@blade.(none)> Mon, 11 Feb 2008 12:55:45 -0700

 arch/powerpc/sysdev/fsl_pci.c |   52 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index bf13c21..c064c91 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -19,6 +19,8 @@
 #include <linux/string.h>
 #include <linux/init.h>
 #include <linux/bootmem.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
 
 #include <asm/io.h>
 #include <asm/prom.h>
@@ -27,6 +29,56 @@
 #include <sysdev/fsl_soc.h>
 #include <sysdev/fsl_pci.h>
 
+#ifdef CONFIG_85xx
+/* setting up 85xx PCI error platform device for EDAC consumption */
+static int __init mpc85xx_pcierr_setup(void)
+{
+	struct device_node *np = NULL;
+	struct of_device *of_dev;
+	struct platform_device *pdev;
+	struct resource res[2];
+	int id = 0;
+	int err;
+
+	for_each_compatible_node(np, "pci", "fsl,mpc8540-pci") {
+		memset(res, 0, sizeof(struct resource) * 2);
+		if (of_address_to_resource(np, 0, &res[0])) {
+			printk(KERN_WARNING "Can't get pci register base!\n");
+			continue;
+		}
+
+		/* move start to where the error registers are */
+		res[0].start += 0xe00;
+
+		of_irq_to_resource(np, 0, &res[1]);
+
+		of_dev = of_find_device_by_node(np);
+		if (!of_dev)
+			return -ENODEV;
+
+		pdev = platform_device_alloc("mpc85xx_pci_err", id++);
+		if (!pdev)
+			return -ENOMEM;
+
+		err = platform_device_add_resources(pdev, res, 2);
+		if (err)
+			goto error;
+
+		pdev->dev.parent = &of_dev->dev;
+
+		err = platform_device_add(pdev);
+		if (err)
+			goto error;
+	}
+	return 0;
+
+error:
+	platform_device_put(pdev);
+	return err;
+}
+late_initcall(mpc85xx_pcierr_setup);
+#endif
+
 /* atmu setup for fsl pci/pcie controller */
 void __init setup_pci_atmu(struct pci_controller *hose, struct resource *rsrc)
 {

^ permalink raw reply related

* [PATCH 1/2] powerpc: publish 85xx soc dts entries as of_device
From: Dave Jiang @ 2008-02-11 20:32 UTC (permalink / raw)
  To: galak, linuxppc-dev

Publish the devices listed in dts under SOC as of_device just like what
mpc85xx_mds platforms do. The 85xx cds and ads platforms currently do not
export the devices in dts as of_device.

The memory controller, L2 cache-controller, and the PCI controller(s) are
published as of_device so the mpc85xx EDAC driver can claim them for usage.

Signed-off-by: Dave Jiang <djiang@mvista.com>

---
commit 187841bf9dff25e4ac1a7174daa55bb036c724b1
tree 3206ffbbfc5075d195345281f72f7b52f060b41b
parent a99824f327c748b2753f4fa570eb1fefcd6a9c4d
author Dave Jiang <djiang@mvista.com> Mon, 11 Feb 2008 12:51:33 -0700
committer Dave Jiang <djiang@blade.(none)> Mon, 11 Feb 2008 12:51:33 -0700

 arch/powerpc/platforms/85xx/mpc85xx_ads.c |   19 +++++++++++++++++++
 arch/powerpc/platforms/85xx/mpc85xx_cds.c |   19 +++++++++++++++++++
 2 files changed, 38 insertions(+), 0 deletions(-)

mpc85xx EDAC driver is in mainline kernel now

diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ads.c b/arch/powerpc/platforms/85xx/mpc85xx_ads.c
index 4e03050..024393c 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_ads.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_ads.c
@@ -26,6 +26,8 @@
 #include <asm/mpic.h>
 #include <mm/mmu_decl.h>
 #include <asm/udbg.h>
+#include <asm/of_device.h>
+#include <asm/of_platform.h>
 
 #include <sysdev/fsl_soc.h>
 #include <sysdev/fsl_pci.h>
@@ -249,6 +251,23 @@ static int __init mpc85xx_ads_probe(void)
         return of_flat_dt_is_compatible(root, "MPC85xxADS");
 }
 
+static struct of_device_id mpc85xx_ids[] = {
+	{ .type = "soc", },
+	{ .compatible = "soc", },
+	{},
+};
+
+static int __init mpc85xx_publish_devices(void)
+{
+	if (!machine_is(mpc85xx_ads))
+		return 0;
+
+	of_platform_bus_probe(NULL, mpc85xx_ids, NULL);
+
+	return 0;
+}
+device_initcall(mpc85xx_publish_devices);
+
 define_machine(mpc85xx_ads) {
 	.name			= "MPC85xx ADS",
 	.probe			= mpc85xx_ads_probe,
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_cds.c b/arch/powerpc/platforms/85xx/mpc85xx_cds.c
index 8b1de78..8525882 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_cds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_cds.c
@@ -42,6 +42,8 @@
 #include <asm/udbg.h>
 #include <asm/mpic.h>
 #include <asm/i8259.h>
+#include <asm/of_device.h>
+#include <asm/of_platform.h>
 
 #include <sysdev/fsl_soc.h>
 #include <sysdev/fsl_pci.h>
@@ -335,6 +337,23 @@ static int __init mpc85xx_cds_probe(void)
         return of_flat_dt_is_compatible(root, "MPC85xxCDS");
 }
 
+static struct of_device_id mpc85xx_ids[] = {
+	{ .type = "soc", },
+	{ .compatible = "soc", },
+	{},
+};
+
+static int __init mpc85xx_publish_devices(void)
+{
+	if (!machine_is(mpc85xx_cds))
+		return 0;
+
+	of_platform_bus_probe(NULL, mpc85xx_ids, NULL);
+
+	return 0;
+}
+device_initcall(mpc85xx_publish_devices);
+
 define_machine(mpc85xx_cds) {
 	.name		= "MPC85xx CDS",
 	.probe		= mpc85xx_cds_probe,

^ permalink raw reply related

* Re: [PATCH] Fix for Freescale ppc cores: major revision detection
From: Martin Langer @ 2008-02-11 20:29 UTC (permalink / raw)
  To: linuxppc-dev

On Mon, Feb 11, 2008 at 09:01:03AM -0600, Kumar Gala wrote:
> 
> On Feb 9, 2008, at 11:47 AM, Martin Langer wrote:
> 
> >Ppc cores by Freescale are using the configuration field instead of  
> >the
> >major revision field for their major revision number. Those field
> >definitions come from include/asm-powerpc/reg.h.
> >
> >Look at the pdf below and you will see that PVR_MAJ() does a wrong  
> >shift
> >for ppc cores by Freescale. This patch fixes it.
> >
> >http://www.freescale.com/files/archives/doc/support_info/PPCPVR.pdf
> 
> This doc doesn't encompass the e500/book-e cores that don't follow the  
> same pattern.
> 
> The code should probably be something like:
> 
> #ifdef CONFIG_FSL_BOOKE
> maj = PVR_MAJ(pvr);
> #else
> maj = (pvr >> 8) & 0xff;
> #endif

Thanks for the hint. Inspired by that I did the logic the other way 
round. So we have the same default cases as we had before and 
additionally it will fit for the cores mentioned in the doc.


--- arch/powerpc/kernel/setup-common.c.ORIGINAL	2008-02-08 22:22:56.000000000 +0100
+++ arch/powerpc/kernel/setup-common.c	2008-02-11 20:54:37.000000000 +0100
@@ -241,8 +241,21 @@
 	/* If we are a Freescale core do a simple check so
 	 * we dont have to keep adding cases in the future */
 	if (PVR_VER(pvr) & 0x8000) {
-		maj = PVR_MAJ(pvr);
-		min = PVR_MIN(pvr);
+		switch (PVR_VER(pvr)) {
+			case 0x8000:	/* 7441/7450/7451, Vger */
+			case 0x8001:	/* 7445/7455, Apollo 6 */
+			case 0x8002:	/* 7447/7457, Apollo 7 */
+			case 0x8003:	/* 7447A, Apollo 7 PM */
+			case 0x8004:	/* 7448, Apollo 8 */
+			case 0x800c:	/* 7410, Nitro */
+				maj = ((pvr >> 8) & 0xF);
+				min = PVR_MIN(pvr);
+				break;
+			default:	/* e500/book-e */
+				maj = PVR_MAJ(pvr);
+				min = PVR_MIN(pvr);
+				break;
+		}
 	} else {
 		switch (PVR_VER(pvr)) {
 			case 0x0020:	/* 403 family */

^ permalink raw reply

* MPC8272 FCC2 MDIO on Port D and FS_ENET
From: Show Man @ 2008-02-11 19:45 UTC (permalink / raw)
  To: linuxppc-embedded

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

As part of the port of our device (MPC8271 based) from 2.6.23 using arch/ppc to 2.6.24 arch/powerpc, I've hit a road block in the past few days in trying to make FS_ENET (freescale)/CPM2 recognize the BCM5221 PHY, which means I can't get eth0 up and running.
  It worked fine in 2.26.23 arch/ppc when configured as FEC_ENET.
   
  As far as I can tell, the "dts" is fine for FCC2 (see below), with the proper pins connected to the device/phy:
                          mdio@10d60 {
                                device_type = "mdio";
                                compatible = "fsl,mpc8272ads-mdio-bitbang",
                                             "fsl,mpc8272-mdio-bitbang",
                                             "fsl,cpm2-mdio-bitbang";
                                reg = <10d60 14>;
                                #address-cells = <1>;
                                #size-cells = <0>;
                                fsl,mdio-pin = <1d>; 
                                fsl,mdc-pin = <17>;
                                  PHY0: ethernet-phy@0 {
                                        interrupt-parent = <&PIC>;
                                        interrupts = <31 8>;
                                        reg = <4>;
                                        device_type = "ethernet-phy";
                                };
                          };
                          ethernet@11320 {
                                device_type = "network";
                                compatible = "fsl,mpc8272-fcc-enet",
                                             "fsl,cpm2-fcc-enet";
                                reg = <11320 20 8500 100 113b0 1>;
                                local-mac-address = [ 00 00 00 00 00 00 ];
                                interrupts = <21 8>;
                                interrupt-parent = <&PIC>;
                                phy-handle = <&PHY0>;
                                linux,network-index = <0>;
                                fsl,cpm-command = <16200300>;
                        };

   
  During my debugging, it appears to be that the PHY is not detected during 
  drivers/net/phy/phy_device.c: get_phy_device(..)
  Basically, the phy_id is 0xFFFFFFFF.
   
  I'm going to try to put a probe on the MDIO/MDC lines later on, but I was wondering whether anyone would have any ideas as to what could be wrong.
   
  thanks
   

       
---------------------------------
Looking for the perfect gift? Give the gift of Flickr!

[-- Attachment #2: Type: text/html, Size: 7730 bytes --]

^ permalink raw reply

* Re: [PATCH 3/5 v3] Tsi108_eth: remove not needed code
From: Jeff Garzik @ 2008-02-11 19:50 UTC (permalink / raw)
  To: Alex Bounine; +Cc: netdev, linuxppc-dev
In-Reply-To: <d5f62df20802111135q36e47070w611291b2cea2253e@mail.gmail.com>

Alex Bounine wrote:
> Code clean-up for tsi108_eth network driver.
> This patch removes not needed dummy read and the corresponding comment.
> The PHY logic requires two reads from the status register to get
> current link status. This is done correctly inside mii_check_media().
> 
> Signed-off-by: Alexandre Bounine <alexandreb@tundra.com>
> ---
> 
> diff -pNur linux-2.6.24/drivers/net/tsi108_eth.c
> linux-2.6.24-fix/drivers/net/tsi108_eth.c
> --- linux-2.6.24/drivers/net/tsi108_eth.c	2008-02-06 15:47:35.000000000 -0500
> +++ linux-2.6.24-fix/drivers/net/tsi108_eth.c	2008-02-06
> 15:54:14.000000000 -0500
> @@ -297,18 +297,11 @@ static void tsi108_check_phy(struct net_
>  	u32 speed;
>  	unsigned long flags;
> 
> -	/* Do a dummy read, as for some reason the first read
> -	 * after a link becomes up returns link down, even if
> -	 * it's been a while since the link came up.
> -	 */
> -
>  	spin_lock_irqsave(&phy_lock, flags);
> 
>  	if (!data->phy_ok)
>  		goto out;
> 
> -	tsi108_read_mii(data, MII_BMSR);
> -
>  	duplex = mii_check_media(&data->mii_if, netif_msg_link(data),
> data->init_media);

applied all five, but each one was still corrupted according to git-am. 
  I was able to apply four via patch(1), which is more forgiving, but 
there are still email problems.

As as you can see from the above quoted (last) line, your email client 
is still word-wrapping patches.

^ permalink raw reply

* Re: looking for 3U system with FPGA support
From: Ron Sass @ 2008-02-11 19:39 UTC (permalink / raw)
  To: Nicholas Mc Guire; +Cc: linuxppc-embedded
In-Reply-To: <Pine.LNX.4.58.0802111025100.1608@vlab.hofr.at>


I'm not sure why you are keen on a 3U form factor but
the Xilinx ML-410 board is an ATX form factor and fits
in various 1U chassis.  We have a couple in off-the-shelf
chassis and some in a chassis that Xilinx sells.

  http://www.xilinx.com/ml410
 
The chassis is part number HW-MLX10-RACK-G.  Coincidentally,
the board has all of the features you mentioned.

Ron

> 
> Hi !
> 
>  hope this is ok for this list - after searching quite some time for a
>  3U board with a PPC on it + more or less any FPGA - I only could find the
>  MEN F12 (5200) and F13 (8540) - does naybody know of other 3U systems ?
> 
>  Basically a FPGA based system like an ML405 would be of interest but I
>  could not locate any 3U products.
> 
>  Roughly:
> 
>   PPC roughly equivalent to a 500 MHz PIII/Celeron
>   2++ Ethernets
>   2 Seriell
>   4++ GPIO (via FPGA)
>   Linux available
>   U-boot if posible
> 
>  Any pointers would be appreciated.
> 
> thx!
> hofrat
> 
> 
> _______________________________________________
> Linuxppc-embedded mailing list
> Linuxppc-embedded@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-embedded

^ permalink raw reply

* [PATCH 5/5 v3] Tsi108_eth: Add ethtool support
From: Alex Bounine @ 2008-02-11 19:36 UTC (permalink / raw)
  To: jgarzik, netdev; +Cc: linuxppc-dev

Add ethtool support to tsi108_eth network driver.

Signed-off-by: Alexandre Bounine <alexandreb@tundra.com>
---

diff -pNur linux-2.6.24/drivers/net/tsi108_eth.c
linux-2.6.24-fix/drivers/net/tsi108_eth.c
--- linux-2.6.24/drivers/net/tsi108_eth.c	2008-02-06 17:10:53.000000000 -0500
+++ linux-2.6.24-fix/drivers/net/tsi108_eth.c	2008-02-06
18:09:43.000000000 -0500
@@ -36,6 +36,7 @@
 #include <linux/net.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
+#include <linux/ethtool.h>
 #include <linux/skbuff.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
@@ -1519,12 +1520,46 @@ static void tsi108_init_mac(struct net_d
 	TSI_WRITE(TSI108_EC_INTMASK, ~0);
 }

+static int tsi108_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+{
+	struct tsi108_prv_data *data = netdev_priv(dev);
+	unsigned long flags;
+	int rc;
+	
+	spin_lock_irqsave(&data->txlock, flags);
+	rc = mii_ethtool_gset(&data->mii_if, cmd);
+	spin_unlock_irqrestore(&data->txlock, flags);
+
+	return rc;
+}
+
+static int tsi108_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+{
+	struct tsi108_prv_data *data = netdev_priv(dev);
+	unsigned long flags;
+	int rc;
+
+	spin_lock_irqsave(&data->txlock, flags);
+	rc = mii_ethtool_sset(&data->mii_if, cmd);
+	spin_unlock_irqrestore(&data->txlock, flags);
+	
+	return rc;
+}
+
 static int tsi108_do_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 {
 	struct tsi108_prv_data *data = netdev_priv(dev);
+	if (!netif_running(dev))
+		return -EINVAL;
 	return generic_mii_ioctl(&data->mii_if, if_mii(rq), cmd, NULL);
 }

+static const struct ethtool_ops tsi108_ethtool_ops = {
+	.get_link 	= ethtool_op_get_link,
+	.get_settings	= tsi108_get_settings,
+	.set_settings	= tsi108_set_settings,
+};
+
 static int
 tsi108_init_one(struct platform_device *pdev)
 {
@@ -1589,6 +1624,7 @@ tsi108_init_one(struct platform_device *
 	dev->get_stats = tsi108_get_stats;
 	netif_napi_add(dev, &data->napi, tsi108_poll, 64);
 	dev->do_ioctl = tsi108_do_ioctl;
+	dev->ethtool_ops = &tsi108_ethtool_ops;

 	/* Apparently, the Linux networking code won't use scatter-gather
 	 * if the hardware doesn't do checksums.  However, it's faster

^ permalink raw reply

* [PATCH 4/5 v3] Tsi108_eth: fix link recovery after disconnect
From: Alex Bounine @ 2008-02-11 19:36 UTC (permalink / raw)
  To: jgarzik, netdev; +Cc: linuxppc-dev

Bug fix for tsi108_eth network driver.
This patch fixes a problem with link recovery after connection was lost.

Signed-off-by: Alexandre Bounine <alexandreb@tundra.com>
---

diff -pNur linux-2.6.24/drivers/net/tsi108_eth.c
linux-2.6.24-fix/drivers/net/tsi108_eth.c
--- linux-2.6.24/drivers/net/tsi108_eth.c	2008-02-06 16:16:00.000000000 -0500
+++ linux-2.6.24-fix/drivers/net/tsi108_eth.c	2008-02-06
16:57:41.000000000 -0500
@@ -338,22 +338,21 @@ static void tsi108_check_phy(struct net_

 			TSI_WRITE(TSI108_MAC_CFG2, mac_cfg2_reg);
 			TSI_WRITE(TSI108_EC_PORTCTRL, portctrl_reg);
+		}

-			if (data->link_up == 0) {
-				/* The manual says it can take 3-4 usecs for the speed change
-				 * to take effect.
-				 */
-				udelay(5);
-
-				spin_lock(&data->txlock);
-				if (is_valid_ether_addr(dev->dev_addr) && data->txfree)
-					netif_wake_queue(dev);
+		if (data->link_up == 0) {
+			/* The manual says it can take 3-4 usecs for the speed change
+			 * to take effect.
+			 */
+			udelay(5);

-				data->link_up = 1;
-				spin_unlock(&data->txlock);
-			}
-		}
+			spin_lock(&data->txlock);
+			if (is_valid_ether_addr(dev->dev_addr) && data->txfree)
+				netif_wake_queue(dev);

+			data->link_up = 1;
+			spin_unlock(&data->txlock);
+		}
 	} else {
 		if (data->link_up == 1) {
 			netif_stop_queue(dev);
@@ -1267,12 +1266,11 @@ static void tsi108_init_phy(struct net_d
 	 * PHY_STAT register before the link up status bit is set.
 	 */

-	data->link_up = 1;
+	data->link_up = 0;

 	while (!((phyval = tsi108_read_mii(data, MII_BMSR)) &
 		 BMSR_LSTATUS)) {
 		if (i++ > (MII_READ_DELAY / 10)) {
-			data->link_up = 0;
 			break;
 		}
 		spin_unlock_irqrestore(&phy_lock, flags);

^ permalink raw reply

* [PATCH 3/5 v3] Tsi108_eth: remove not needed code
From: Alex Bounine @ 2008-02-11 19:35 UTC (permalink / raw)
  To: jgarzik, netdev; +Cc: linuxppc-dev

Code clean-up for tsi108_eth network driver.
This patch removes not needed dummy read and the corresponding comment.
The PHY logic requires two reads from the status register to get
current link status. This is done correctly inside mii_check_media().

Signed-off-by: Alexandre Bounine <alexandreb@tundra.com>
---

diff -pNur linux-2.6.24/drivers/net/tsi108_eth.c
linux-2.6.24-fix/drivers/net/tsi108_eth.c
--- linux-2.6.24/drivers/net/tsi108_eth.c	2008-02-06 15:47:35.000000000 -0500
+++ linux-2.6.24-fix/drivers/net/tsi108_eth.c	2008-02-06
15:54:14.000000000 -0500
@@ -297,18 +297,11 @@ static void tsi108_check_phy(struct net_
 	u32 speed;
 	unsigned long flags;

-	/* Do a dummy read, as for some reason the first read
-	 * after a link becomes up returns link down, even if
-	 * it's been a while since the link came up.
-	 */
-
 	spin_lock_irqsave(&phy_lock, flags);

 	if (!data->phy_ok)
 		goto out;

-	tsi108_read_mii(data, MII_BMSR);
-
 	duplex = mii_check_media(&data->mii_if, netif_msg_link(data),
data->init_media);
 	data->init_media = 0;

^ permalink raw reply

* [PATCH 2/5 v3] Tsi108_eth: fix detection of 1000Mb mode
From: Alex Bounine @ 2008-02-11 19:35 UTC (permalink / raw)
  To: jgarzik, netdev; +Cc: linuxppc-dev

Bug fix for tsi108_eth network driver.
This patch fixes a problem with detection of 1000Mb speed.

Signed-off-by: Alexandre Bounine <alexandreb@tundra.com>
---

diff -pNur linux-2.6.24/drivers/net/tsi108_eth.c
linux-2.6.24-fix/drivers/net/tsi108_eth.c
--- linux-2.6.24/drivers/net/tsi108_eth.c	2008-02-06 15:09:19.000000000 -0500
+++ linux-2.6.24-fix/drivers/net/tsi108_eth.c	2008-02-06
15:34:12.000000000 -0500
@@ -1287,6 +1287,7 @@ static void tsi108_init_phy(struct net_d
 		spin_lock_irqsave(&phy_lock, flags);
 	}

+	data->mii_if.supports_gmii = mii_check_gmii_support(&data->mii_if);
 	printk(KERN_DEBUG "PHY_STAT reg contains %08x\n", phyval);
 	data->phy_ok = 1;
 	data->init_media = 1;
@@ -1584,7 +1585,6 @@ tsi108_init_one(struct platform_device *
 	data->mii_if.phy_id = einfo->phy;
 	data->mii_if.phy_id_mask = 0x1f;
 	data->mii_if.reg_num_mask = 0x1f;
-	data->mii_if.supports_gmii = mii_check_gmii_support(&data->mii_if);

 	data->phy = einfo->phy;
 	data->phy_type = einfo->phy_type;

^ permalink raw reply

* [PATCH 1/5 v3] Tsi108_eth: add missing linking to driver data
From: Alex Bounine @ 2008-02-11 19:33 UTC (permalink / raw)
  To: jgarzik, netdev; +Cc: linuxppc-dev

Bug fix for tsi108_eth network driver.
This patch adds missing linking to driver data.

Signed-off-by: Alexandre Bounine <alexandreb@tundra.com>
---

diff -pNur linux-2.6.24/drivers/net/tsi108_eth.c
linux-2.6.24-fix/drivers/net/tsi108_eth.c
--- linux-2.6.24/drivers/net/tsi108_eth.c	2008-01-24 17:58:37.000000000 -0500
+++ linux-2.6.24-fix/drivers/net/tsi108_eth.c	2008-02-06
14:30:04.000000000 -0500
@@ -1629,6 +1629,7 @@ tsi108_init_one(struct platform_device *
 		goto register_fail;
 	}

+	platform_set_drvdata(pdev, dev);
 	printk(KERN_INFO "%s: Tsi108 Gigabit Ethernet, MAC: %s\n",
 	       dev->name, print_mac(mac, dev->dev_addr));
 #ifdef DEBUG

^ permalink raw reply

* looking for 3U system with FPGA support
From: Nicholas Mc Guire @ 2008-02-11 19:31 UTC (permalink / raw)
  To: linuxppc-embedded


Hi !

 hope this is ok for this list - after searching quite some time for a
 3U board with a PPC on it + more or less any FPGA - I only could find the
 MEN F12 (5200) and F13 (8540) - does naybody know of other 3U systems ?

 Basically a FPGA based system like an ML405 would be of interest but I
 could not locate any 3U products.

 Roughly:

  PPC roughly equivalent to a 500 MHz PIII/Celeron
  2++ Ethernets
  2 Seriell
  4++ GPIO (via FPGA)
  Linux available
  U-boot if posible

 Any pointers would be appreciated.

thx!
hofrat

^ permalink raw reply

* Re: [PATCH 2/8] pseries: phyp dump: reserve-release proof-of-concept
From: Manish Ahuja @ 2008-02-11 18:29 UTC (permalink / raw)
  To: Paul Mackerras
  Cc: Manish Ahuja, ppc-dev, linasvepstas, Larry Kessler,
	Michael Strosaker
In-Reply-To: <18346.21469.512910.93325@cargo.ozlabs.ibm.com>

Sorry,

I think i sent the wrong patch file, it shouldn't have my printk statement in there. Let me re-send 
the correct file and let me test it once more to make sure it does the right thing.

-Manish



Paul Mackerras wrote:
> Manish Ahuja writes:
> 
>> Initial patch for reserving memory in early boot, and freeing it later.
>> If the previous boot had ended with a crash, the reserved memory would contain
>> a copy of the crashed kernel data.
> 
> [snip]
> 
>> +static void __init reserve_crashed_mem(void)
>> +{
>> +	unsigned long base, size;
>> +
>> +	if (phyp_dump_info->phyp_dump_is_active) {
>> +		/* Reserve *everything* above RMR. We'll free this real soon.*/
>> +		base = PHYP_DUMP_RMR_END;
>> +		size = lmb_end_of_DRAM() - base;
>> +
>> +		/* XXX crashed_ram_end is wrong, since it may be beyond
>> +	 	* the memory_limit, it will need to be adjusted. */
>> +		lmb_reserve(base, size);
>> +
>> +		phyp_dump_info->init_reserve_start = base;
>> +		phyp_dump_info->init_reserve_size = size;
>> +	}
>> +	else {
>> +		size = phyp_dump_info->cpu_state_size +
>> +			phyp_dump_info->hpte_region_size +
>> +			PHYP_DUMP_RMR_END;
>> +		base = lmb_end_of_DRAM() - size;
>> +	printk(KERN_ERR "Manish reserve regular kernel space is %ld %ld\n", base, size);
>> +		lmb_reserve(base, size);
> 
> This is still reserving memory even on systems that aren't running on
> pHyp at all.  Please rework this so that no memory is reserved if the
> system doesn't support phyp-assisted dump.
> 
> Paul.

^ permalink raw reply

* [PATCH] Xilinx: hwicap: cleanup
From: Stephen Neuendorffer @ 2008-02-11 18:24 UTC (permalink / raw)
  To: grant.likely, linuxppc-dev, git-dev, jirislaby, linux.kernel
In-Reply-To: <1202437061-12691-1-git-send-email-stephen.neuendorffer@xilinx.com>

Fix some missing __user tags and incorrect section tags.
Convert semaphores to mutexes.
Make probed_devices re-entrancy and error condition safe.
Fix some backwards memcpys.
Some other minor cleanups.
Use kerneldoc format.

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>

---

Grant, Since it appears that the driver will stay in as-is, here are
the updates against mainline, based on Jiri's comments.
---
 drivers/char/xilinx_hwicap/buffer_icap.c   |   80 ++++++++++----------
 drivers/char/xilinx_hwicap/fifo_icap.c     |   60 +++++++-------
 drivers/char/xilinx_hwicap/xilinx_hwicap.c |  113 ++++++++++++++++------------
 drivers/char/xilinx_hwicap/xilinx_hwicap.h |   24 +++---
 4 files changed, 148 insertions(+), 129 deletions(-)

diff --git a/drivers/char/xilinx_hwicap/buffer_icap.c b/drivers/char/xilinx_hwicap/buffer_icap.c
index dfea2bd..2c5d17d 100644
--- a/drivers/char/xilinx_hwicap/buffer_icap.c
+++ b/drivers/char/xilinx_hwicap/buffer_icap.c
@@ -73,8 +73,8 @@
 #define XHI_BUFFER_START 0
 
 /**
- * buffer_icap_get_status: Get the contents of the status register.
- * @parameter base_address: is the base address of the device
+ * buffer_icap_get_status - Get the contents of the status register.
+ * @base_address: is the base address of the device
  *
  * The status register contains the ICAP status and the done bit.
  *
@@ -94,9 +94,9 @@ static inline u32 buffer_icap_get_status(void __iomem *base_address)
 }
 
 /**
- * buffer_icap_get_bram: Reads data from the storage buffer bram.
- * @parameter base_address: contains the base address of the component.
- * @parameter offset: The word offset from which the data should be read.
+ * buffer_icap_get_bram - Reads data from the storage buffer bram.
+ * @base_address: contains the base address of the component.
+ * @offset: The word offset from which the data should be read.
  *
  * A bram is used as a configuration memory cache.  One frame of data can
  * be stored in this "storage buffer".
@@ -108,8 +108,8 @@ static inline u32 buffer_icap_get_bram(void __iomem *base_address,
 }
 
 /**
- * buffer_icap_busy: Return true if the icap device is busy
- * @parameter base_address: is the base address of the device
+ * buffer_icap_busy - Return true if the icap device is busy
+ * @base_address: is the base address of the device
  *
  * The queries the low order bit of the status register, which
  * indicates whether the current configuration or readback operation
@@ -121,8 +121,8 @@ static inline bool buffer_icap_busy(void __iomem *base_address)
 }
 
 /**
- * buffer_icap_busy: Return true if the icap device is not busy
- * @parameter base_address: is the base address of the device
+ * buffer_icap_busy - Return true if the icap device is not busy
+ * @base_address: is the base address of the device
  *
  * The queries the low order bit of the status register, which
  * indicates whether the current configuration or readback operation
@@ -134,9 +134,9 @@ static inline bool buffer_icap_done(void __iomem *base_address)
 }
 
 /**
- * buffer_icap_set_size: Set the size register.
- * @parameter base_address: is the base address of the device
- * @parameter data: The size in bytes.
+ * buffer_icap_set_size - Set the size register.
+ * @base_address: is the base address of the device
+ * @data: The size in bytes.
  *
  * The size register holds the number of 8 bit bytes to transfer between
  * bram and the icap (or icap to bram).
@@ -148,9 +148,9 @@ static inline void buffer_icap_set_size(void __iomem *base_address,
 }
 
 /**
- * buffer_icap_mSetoffsetReg: Set the bram offset register.
- * @parameter base_address: contains the base address of the device.
- * @parameter data: is the value to be written to the data register.
+ * buffer_icap_mSetoffsetReg - Set the bram offset register.
+ * @base_address: contains the base address of the device.
+ * @data: is the value to be written to the data register.
  *
  * The bram offset register holds the starting bram address to transfer
  * data from during configuration or write data to during readback.
@@ -162,9 +162,9 @@ static inline void buffer_icap_set_offset(void __iomem *base_address,
 }
 
 /**
- * buffer_icap_set_rnc: Set the RNC (Readback not Configure) register.
- * @parameter base_address: contains the base address of the device.
- * @parameter data: is the value to be written to the data register.
+ * buffer_icap_set_rnc - Set the RNC (Readback not Configure) register.
+ * @base_address: contains the base address of the device.
+ * @data: is the value to be written to the data register.
  *
  * The RNC register determines the direction of the data transfer.  It
  * controls whether a configuration or readback take place.  Writing to
@@ -178,10 +178,10 @@ static inline void buffer_icap_set_rnc(void __iomem *base_address,
 }
 
 /**
- * buffer_icap_set_bram: Write data to the storage buffer bram.
- * @parameter base_address: contains the base address of the component.
- * @parameter offset: The word offset at which the data should be written.
- * @parameter data: The value to be written to the bram offset.
+ * buffer_icap_set_bram - Write data to the storage buffer bram.
+ * @base_address: contains the base address of the component.
+ * @offset: The word offset at which the data should be written.
+ * @data: The value to be written to the bram offset.
  *
  * A bram is used as a configuration memory cache.  One frame of data can
  * be stored in this "storage buffer".
@@ -193,10 +193,10 @@ static inline void buffer_icap_set_bram(void __iomem *base_address,
 }
 
 /**
- * buffer_icap_device_read: Transfer bytes from ICAP to the storage buffer.
- * @parameter drvdata: a pointer to the drvdata.
- * @parameter offset: The storage buffer start address.
- * @parameter count: The number of words (32 bit) to read from the
+ * buffer_icap_device_read - Transfer bytes from ICAP to the storage buffer.
+ * @drvdata: a pointer to the drvdata.
+ * @offset: The storage buffer start address.
+ * @count: The number of words (32 bit) to read from the
  *           device (ICAP).
  **/
 static int buffer_icap_device_read(struct hwicap_drvdata *drvdata,
@@ -227,10 +227,10 @@ static int buffer_icap_device_read(struct hwicap_drvdata *drvdata,
 };
 
 /**
- * buffer_icap_device_write: Transfer bytes from ICAP to the storage buffer.
- * @parameter drvdata: a pointer to the drvdata.
- * @parameter offset: The storage buffer start address.
- * @parameter count: The number of words (32 bit) to read from the
+ * buffer_icap_device_write - Transfer bytes from ICAP to the storage buffer.
+ * @drvdata: a pointer to the drvdata.
+ * @offset: The storage buffer start address.
+ * @count: The number of words (32 bit) to read from the
  *           device (ICAP).
  **/
 static int buffer_icap_device_write(struct hwicap_drvdata *drvdata,
@@ -261,8 +261,8 @@ static int buffer_icap_device_write(struct hwicap_drvdata *drvdata,
 };
 
 /**
- * buffer_icap_reset: Reset the logic of the icap device.
- * @parameter drvdata: a pointer to the drvdata.
+ * buffer_icap_reset - Reset the logic of the icap device.
+ * @drvdata: a pointer to the drvdata.
  *
  * Writing to the status register resets the ICAP logic in an internal
  * version of the core.  For the version of the core published in EDK,
@@ -274,10 +274,10 @@ void buffer_icap_reset(struct hwicap_drvdata *drvdata)
 }
 
 /**
- * buffer_icap_set_configuration: Load a partial bitstream from system memory.
- * @parameter drvdata: a pointer to the drvdata.
- * @parameter data: Kernel address of the partial bitstream.
- * @parameter size: the size of the partial bitstream in 32 bit words.
+ * buffer_icap_set_configuration - Load a partial bitstream from system memory.
+ * @drvdata: a pointer to the drvdata.
+ * @data: Kernel address of the partial bitstream.
+ * @size: the size of the partial bitstream in 32 bit words.
  **/
 int buffer_icap_set_configuration(struct hwicap_drvdata *drvdata, u32 *data,
 			     u32 size)
@@ -333,10 +333,10 @@ int buffer_icap_set_configuration(struct hwicap_drvdata *drvdata, u32 *data,
 };
 
 /**
- * buffer_icap_get_configuration: Read configuration data from the device.
- * @parameter drvdata: a pointer to the drvdata.
- * @parameter data: Address of the data representing the partial bitstream
- * @parameter size: the size of the partial bitstream in 32 bit words.
+ * buffer_icap_get_configuration - Read configuration data from the device.
+ * @drvdata: a pointer to the drvdata.
+ * @data: Address of the data representing the partial bitstream
+ * @size: the size of the partial bitstream in 32 bit words.
  **/
 int buffer_icap_get_configuration(struct hwicap_drvdata *drvdata, u32 *data,
 			     u32 size)
diff --git a/drivers/char/xilinx_hwicap/fifo_icap.c b/drivers/char/xilinx_hwicap/fifo_icap.c
index 0988314..6f45dbd 100644
--- a/drivers/char/xilinx_hwicap/fifo_icap.c
+++ b/drivers/char/xilinx_hwicap/fifo_icap.c
@@ -94,9 +94,9 @@
 
 
 /**
- * fifo_icap_fifo_write: Write data to the write FIFO.
- * @parameter drvdata: a pointer to the drvdata.
- * @parameter data: the 32-bit value to be written to the FIFO.
+ * fifo_icap_fifo_write - Write data to the write FIFO.
+ * @drvdata: a pointer to the drvdata.
+ * @data: the 32-bit value to be written to the FIFO.
  *
  * This function will silently fail if the fifo is full.
  **/
@@ -108,8 +108,8 @@ static inline void fifo_icap_fifo_write(struct hwicap_drvdata *drvdata,
 }
 
 /**
- * fifo_icap_fifo_read: Read data from the Read FIFO.
- * @parameter drvdata: a pointer to the drvdata.
+ * fifo_icap_fifo_read - Read data from the Read FIFO.
+ * @drvdata: a pointer to the drvdata.
  *
  * This function will silently fail if the fifo is empty.
  **/
@@ -121,9 +121,9 @@ static inline u32 fifo_icap_fifo_read(struct hwicap_drvdata *drvdata)
 }
 
 /**
- * fifo_icap_set_read_size: Set the the size register.
- * @parameter drvdata: a pointer to the drvdata.
- * @parameter data: the size of the following read transaction, in words.
+ * fifo_icap_set_read_size - Set the the size register.
+ * @drvdata: a pointer to the drvdata.
+ * @data: the size of the following read transaction, in words.
  **/
 static inline void fifo_icap_set_read_size(struct hwicap_drvdata *drvdata,
 		u32 data)
@@ -132,8 +132,8 @@ static inline void fifo_icap_set_read_size(struct hwicap_drvdata *drvdata,
 }
 
 /**
- * fifo_icap_start_config: Initiate a configuration (write) to the device.
- * @parameter drvdata: a pointer to the drvdata.
+ * fifo_icap_start_config - Initiate a configuration (write) to the device.
+ * @drvdata: a pointer to the drvdata.
  **/
 static inline void fifo_icap_start_config(struct hwicap_drvdata *drvdata)
 {
@@ -142,8 +142,8 @@ static inline void fifo_icap_start_config(struct hwicap_drvdata *drvdata)
 }
 
 /**
- * fifo_icap_start_readback: Initiate a readback from the device.
- * @parameter drvdata: a pointer to the drvdata.
+ * fifo_icap_start_readback - Initiate a readback from the device.
+ * @drvdata: a pointer to the drvdata.
  **/
 static inline void fifo_icap_start_readback(struct hwicap_drvdata *drvdata)
 {
@@ -152,8 +152,8 @@ static inline void fifo_icap_start_readback(struct hwicap_drvdata *drvdata)
 }
 
 /**
- * fifo_icap_busy: Return true if the ICAP is still processing a transaction.
- * @parameter drvdata: a pointer to the drvdata.
+ * fifo_icap_busy - Return true if the ICAP is still processing a transaction.
+ * @drvdata: a pointer to the drvdata.
  **/
 static inline u32 fifo_icap_busy(struct hwicap_drvdata *drvdata)
 {
@@ -163,8 +163,8 @@ static inline u32 fifo_icap_busy(struct hwicap_drvdata *drvdata)
 }
 
 /**
- * fifo_icap_write_fifo_vacancy: Query the write fifo available space.
- * @parameter drvdata: a pointer to the drvdata.
+ * fifo_icap_write_fifo_vacancy - Query the write fifo available space.
+ * @drvdata: a pointer to the drvdata.
  *
  * Return the number of words that can be safely pushed into the write fifo.
  **/
@@ -175,8 +175,8 @@ static inline u32 fifo_icap_write_fifo_vacancy(
 }
 
 /**
- * fifo_icap_read_fifo_occupancy: Query the read fifo available data.
- * @parameter drvdata: a pointer to the drvdata.
+ * fifo_icap_read_fifo_occupancy - Query the read fifo available data.
+ * @drvdata: a pointer to the drvdata.
  *
  * Return the number of words that can be safely read from the read fifo.
  **/
@@ -187,11 +187,11 @@ static inline u32 fifo_icap_read_fifo_occupancy(
 }
 
 /**
- * fifo_icap_set_configuration: Send configuration data to the ICAP.
- * @parameter drvdata: a pointer to the drvdata.
- * @parameter frame_buffer: a pointer to the data to be written to the
+ * fifo_icap_set_configuration - Send configuration data to the ICAP.
+ * @drvdata: a pointer to the drvdata.
+ * @frame_buffer: a pointer to the data to be written to the
  *		ICAP device.
- * @parameter num_words: the number of words (32 bit) to write to the ICAP
+ * @num_words: the number of words (32 bit) to write to the ICAP
  *		device.
 
  * This function writes the given user data to the Write FIFO in
@@ -266,10 +266,10 @@ int fifo_icap_set_configuration(struct hwicap_drvdata *drvdata,
 }
 
 /**
- * fifo_icap_get_configuration: Read configuration data from the device.
- * @parameter drvdata: a pointer to the drvdata.
- * @parameter data: Address of the data representing the partial bitstream
- * @parameter size: the size of the partial bitstream in 32 bit words.
+ * fifo_icap_get_configuration - Read configuration data from the device.
+ * @drvdata: a pointer to the drvdata.
+ * @data: Address of the data representing the partial bitstream
+ * @size: the size of the partial bitstream in 32 bit words.
  *
  * This function reads the specified number of words from the ICAP device in
  * the polled mode.
@@ -335,8 +335,8 @@ int fifo_icap_get_configuration(struct hwicap_drvdata *drvdata,
 }
 
 /**
- * buffer_icap_reset: Reset the logic of the icap device.
- * @parameter drvdata: a pointer to the drvdata.
+ * buffer_icap_reset - Reset the logic of the icap device.
+ * @drvdata: a pointer to the drvdata.
  *
  * This function forces the software reset of the complete HWICAP device.
  * All the registers will return to the default value and the FIFO is also
@@ -360,8 +360,8 @@ void fifo_icap_reset(struct hwicap_drvdata *drvdata)
 }
 
 /**
- * fifo_icap_flush_fifo: This function flushes the FIFOs in the device.
- * @parameter drvdata: a pointer to the drvdata.
+ * fifo_icap_flush_fifo - This function flushes the FIFOs in the device.
+ * @drvdata: a pointer to the drvdata.
  */
 void fifo_icap_flush_fifo(struct hwicap_drvdata *drvdata)
 {
diff --git a/drivers/char/xilinx_hwicap/xilinx_hwicap.c b/drivers/char/xilinx_hwicap/xilinx_hwicap.c
index 24f6aef..eddaa26 100644
--- a/drivers/char/xilinx_hwicap/xilinx_hwicap.c
+++ b/drivers/char/xilinx_hwicap/xilinx_hwicap.c
@@ -84,7 +84,7 @@
 #include <linux/init.h>
 #include <linux/poll.h>
 #include <linux/proc_fs.h>
-#include <asm/semaphore.h>
+#include <linux/mutex.h>
 #include <linux/sysctl.h>
 #include <linux/version.h>
 #include <linux/fs.h>
@@ -119,6 +119,7 @@ module_param(xhwicap_minor, int, S_IRUGO);
 
 /* An array, which is set to true when the device is registered. */
 static bool probed_devices[HWICAP_DEVICES];
+static struct mutex icap_sem;
 
 static struct class *icap_class;
 
@@ -199,14 +200,14 @@ static const struct config_registers v5_config_registers = {
 };
 
 /**
- * hwicap_command_desync: Send a DESYNC command to the ICAP port.
- * @parameter drvdata: a pointer to the drvdata.
+ * hwicap_command_desync - Send a DESYNC command to the ICAP port.
+ * @drvdata: a pointer to the drvdata.
  *
  * This command desynchronizes the ICAP After this command, a
  * bitstream containing a NULL packet, followed by a SYNCH packet is
  * required before the ICAP will recognize commands.
  */
-int hwicap_command_desync(struct hwicap_drvdata *drvdata)
+static int hwicap_command_desync(struct hwicap_drvdata *drvdata)
 {
 	u32 buffer[4];
 	u32 index = 0;
@@ -228,14 +229,14 @@ int hwicap_command_desync(struct hwicap_drvdata *drvdata)
 }
 
 /**
- * hwicap_command_capture: Send a CAPTURE command to the ICAP port.
- * @parameter drvdata: a pointer to the drvdata.
+ * hwicap_command_capture - Send a CAPTURE command to the ICAP port.
+ * @drvdata: a pointer to the drvdata.
  *
  * This command captures all of the flip flop states so they will be
  * available during readback.  One can use this command instead of
  * enabling the CAPTURE block in the design.
  */
-int hwicap_command_capture(struct hwicap_drvdata *drvdata)
+static int hwicap_command_capture(struct hwicap_drvdata *drvdata)
 {
 	u32 buffer[7];
 	u32 index = 0;
@@ -252,7 +253,7 @@ int hwicap_command_capture(struct hwicap_drvdata *drvdata)
 	buffer[index++] = XHI_DUMMY_PACKET;
 
 	/*
-	 * Write the data to the FIFO and intiate the transfer of data
+	 * Write the data to the FIFO and initiate the transfer of data
 	 * present in the FIFO to the ICAP device.
 	 */
 	return drvdata->config->set_configuration(drvdata,
@@ -261,18 +262,18 @@ int hwicap_command_capture(struct hwicap_drvdata *drvdata)
 }
 
 /**
- * hwicap_get_configuration_register: Query a configuration register.
- * @parameter drvdata: a pointer to the drvdata.
- * @parameter reg: a constant which represents the configuration
+ * hwicap_get_configuration_register - Query a configuration register.
+ * @drvdata: a pointer to the drvdata.
+ * @reg: a constant which represents the configuration
  *		register value to be returned.
  * 		Examples:  XHI_IDCODE, XHI_FLR.
- * @parameter RegData: returns the value of the register.
+ * @reg_data: returns the value of the register.
  *
  * Sends a query packet to the ICAP and then receives the response.
  * The icap is left in Synched state.
  */
-int hwicap_get_configuration_register(struct hwicap_drvdata *drvdata,
-		u32 reg, u32 *RegData)
+static int hwicap_get_configuration_register(struct hwicap_drvdata *drvdata,
+		u32 reg, u32 *reg_data)
 {
 	int status;
 	u32 buffer[6];
@@ -300,14 +301,14 @@ int hwicap_get_configuration_register(struct hwicap_drvdata *drvdata,
 	/*
 	 * Read the configuration register
 	 */
-	status = drvdata->config->get_configuration(drvdata, RegData, 1);
+	status = drvdata->config->get_configuration(drvdata, reg_data, 1);
 	if (status)
 		return status;
 
 	return 0;
 }
 
-int hwicap_initialize_hwicap(struct hwicap_drvdata *drvdata)
+static int hwicap_initialize_hwicap(struct hwicap_drvdata *drvdata)
 {
 	int status;
 	u32 idcode;
@@ -344,7 +345,7 @@ int hwicap_initialize_hwicap(struct hwicap_drvdata *drvdata)
 }
 
 static ssize_t
-hwicap_read(struct file *file, char *buf, size_t count, loff_t *ppos)
+hwicap_read(struct file *file, __user char *buf, size_t count, loff_t *ppos)
 {
 	struct hwicap_drvdata *drvdata = file->private_data;
 	ssize_t bytes_to_read = 0;
@@ -353,8 +354,9 @@ hwicap_read(struct file *file, char *buf, size_t count, loff_t *ppos)
 	u32 bytes_remaining;
 	int status;
 
-	if (down_interruptible(&drvdata->sem))
-		return -ERESTARTSYS;
+	status = mutex_lock_interruptible(&drvdata->sem);
+	if (status)
+		return status;
 
 	if (drvdata->read_buffer_in_use) {
 		/* If there are leftover bytes in the buffer, just */
@@ -370,8 +372,9 @@ hwicap_read(struct file *file, char *buf, size_t count, loff_t *ppos)
 			goto error;
 		}
 		drvdata->read_buffer_in_use -= bytes_to_read;
-		memcpy(drvdata->read_buffer + bytes_to_read,
-				drvdata->read_buffer, 4 - bytes_to_read);
+		memmove(drvdata->read_buffer,
+		       drvdata->read_buffer + bytes_to_read,
+		       4 - bytes_to_read);
 	} else {
 		/* Get new data from the ICAP, and return was was requested. */
 		kbuf = (u32 *) get_zeroed_page(GFP_KERNEL);
@@ -414,18 +417,20 @@ hwicap_read(struct file *file, char *buf, size_t count, loff_t *ppos)
 			status = -EFAULT;
 			goto error;
 		}
-		memcpy(kbuf, drvdata->read_buffer, bytes_remaining);
+		memcpy(drvdata->read_buffer,
+		       kbuf,
+		       bytes_remaining);
 		drvdata->read_buffer_in_use = bytes_remaining;
 		free_page((unsigned long)kbuf);
 	}
 	status = bytes_to_read;
  error:
-	up(&drvdata->sem);
+	mutex_unlock(&drvdata->sem);
 	return status;
 }
 
 static ssize_t
-hwicap_write(struct file *file, const char *buf,
+hwicap_write(struct file *file, const __user char *buf,
 		size_t count, loff_t *ppos)
 {
 	struct hwicap_drvdata *drvdata = file->private_data;
@@ -435,8 +440,9 @@ hwicap_write(struct file *file, const char *buf,
 	ssize_t len;
 	ssize_t status;
 
-	if (down_interruptible(&drvdata->sem))
-		return -ERESTARTSYS;
+	status = mutex_lock_interruptible(&drvdata->sem);
+	if (status)
+		return status;
 
 	left += drvdata->write_buffer_in_use;
 
@@ -465,7 +471,7 @@ hwicap_write(struct file *file, const char *buf,
 			memcpy(kbuf, drvdata->write_buffer,
 					drvdata->write_buffer_in_use);
 			if (copy_from_user(
-			    (((char *)kbuf) + (drvdata->write_buffer_in_use)),
+			    (((char *)kbuf) + drvdata->write_buffer_in_use),
 			    buf + written,
 			    len - (drvdata->write_buffer_in_use))) {
 				free_page((unsigned long)kbuf);
@@ -508,7 +514,7 @@ hwicap_write(struct file *file, const char *buf,
 	free_page((unsigned long)kbuf);
 	status = written;
  error:
-	up(&drvdata->sem);
+	mutex_unlock(&drvdata->sem);
 	return status;
 }
 
@@ -519,8 +525,9 @@ static int hwicap_open(struct inode *inode, struct file *file)
 
 	drvdata = container_of(inode->i_cdev, struct hwicap_drvdata, cdev);
 
-	if (down_interruptible(&drvdata->sem))
-		return -ERESTARTSYS;
+	status = mutex_lock_interruptible(&drvdata->sem);
+	if (status)
+		return status;
 
 	if (drvdata->is_open) {
 		status = -EBUSY;
@@ -539,7 +546,7 @@ static int hwicap_open(struct inode *inode, struct file *file)
 	drvdata->is_open = 1;
 
  error:
-	up(&drvdata->sem);
+	mutex_unlock(&drvdata->sem);
 	return status;
 }
 
@@ -549,8 +556,7 @@ static int hwicap_release(struct inode *inode, struct file *file)
 	int i;
 	int status = 0;
 
-	if (down_interruptible(&drvdata->sem))
-		return -ERESTARTSYS;
+	mutex_lock(&drvdata->sem);
 
 	if (drvdata->write_buffer_in_use) {
 		/* Flush write buffer. */
@@ -569,7 +575,7 @@ static int hwicap_release(struct inode *inode, struct file *file)
 
  error:
 	drvdata->is_open = 0;
-	up(&drvdata->sem);
+	mutex_unlock(&drvdata->sem);
 	return status;
 }
 
@@ -592,31 +598,36 @@ static int __devinit hwicap_setup(struct device *dev, int id,
 
 	dev_info(dev, "Xilinx icap port driver\n");
 
+	mutex_lock(&icap_sem);
+
 	if (id < 0) {
 		for (id = 0; id < HWICAP_DEVICES; id++)
 			if (!probed_devices[id])
 				break;
 	}
 	if (id < 0 || id >= HWICAP_DEVICES) {
+		mutex_unlock(&icap_sem);
 		dev_err(dev, "%s%i too large\n", DRIVER_NAME, id);
 		return -EINVAL;
 	}
 	if (probed_devices[id]) {
+		mutex_unlock(&icap_sem);
 		dev_err(dev, "cannot assign to %s%i; it is already in use\n",
 			DRIVER_NAME, id);
 		return -EBUSY;
 	}
 
 	probed_devices[id] = 1;
+	mutex_unlock(&icap_sem);
 
 	devt = MKDEV(xhwicap_major, xhwicap_minor + id);
 
-	drvdata = kmalloc(sizeof(struct hwicap_drvdata), GFP_KERNEL);
+	drvdata = kzalloc(sizeof(struct hwicap_drvdata), GFP_KERNEL);
 	if (!drvdata) {
 		dev_err(dev, "Couldn't allocate device private record\n");
-		return -ENOMEM;
+		retval = -ENOMEM;
+		goto failed0;
 	}
-	memset((void *)drvdata, 0, sizeof(struct hwicap_drvdata));
 	dev_set_drvdata(dev, (void *)drvdata);
 
 	if (!regs_res) {
@@ -648,7 +659,7 @@ static int __devinit hwicap_setup(struct device *dev, int id,
 	drvdata->config = config;
 	drvdata->config_regs = config_regs;
 
-	init_MUTEX(&drvdata->sem);
+	mutex_init(&drvdata->sem);
 	drvdata->is_open = 0;
 
 	dev_info(dev, "ioremap %lx to %p with size %x\n",
@@ -663,7 +674,7 @@ static int __devinit hwicap_setup(struct device *dev, int id,
 		goto failed3;
 	}
 	/*  devfs_mk_cdev(devt, S_IFCHR|S_IRUGO|S_IWUGO, DRIVER_NAME); */
-	class_device_create(icap_class, NULL, devt, NULL, DRIVER_NAME);
+	device_create(icap_class, dev, devt, "%s%d", DRIVER_NAME, id);
 	return 0;		/* success */
 
  failed3:
@@ -675,6 +686,11 @@ static int __devinit hwicap_setup(struct device *dev, int id,
  failed1:
 	kfree(drvdata);
 
+ failed0:
+	mutex_lock(&icap_sem);
+	probed_devices[id] = 0;
+	mutex_unlock(&icap_sem);
+
 	return retval;
 }
 
@@ -699,14 +715,16 @@ static int __devexit hwicap_remove(struct device *dev)
 	if (!drvdata)
 		return 0;
 
-	class_device_destroy(icap_class, drvdata->devt);
+	device_destroy(icap_class, drvdata->devt);
 	cdev_del(&drvdata->cdev);
 	iounmap(drvdata->base_address);
 	release_mem_region(drvdata->mem_start, drvdata->mem_size);
 	kfree(drvdata);
 	dev_set_drvdata(dev, NULL);
-	probed_devices[MINOR(dev->devt)-xhwicap_minor] = 0;
 
+	mutex_lock(&icap_sem);
+	probed_devices[MINOR(dev->devt)-xhwicap_minor] = 0;
+	mutex_unlock(&icap_sem);
 	return 0;		/* success */
 }
 
@@ -821,28 +839,29 @@ static struct of_platform_driver hwicap_of_driver = {
 };
 
 /* Registration helpers to keep the number of #ifdefs to a minimum */
-static inline int __devinit hwicap_of_register(void)
+static inline int __init hwicap_of_register(void)
 {
 	pr_debug("hwicap: calling of_register_platform_driver()\n");
 	return of_register_platform_driver(&hwicap_of_driver);
 }
 
-static inline void __devexit hwicap_of_unregister(void)
+static inline void __exit hwicap_of_unregister(void)
 {
 	of_unregister_platform_driver(&hwicap_of_driver);
 }
 #else /* CONFIG_OF */
 /* CONFIG_OF not enabled; do nothing helpers */
-static inline int __devinit hwicap_of_register(void) { return 0; }
-static inline void __devexit hwicap_of_unregister(void) { }
+static inline int __init hwicap_of_register(void) { return 0; }
+static inline void __exit hwicap_of_unregister(void) { }
 #endif /* CONFIG_OF */
 
-static int __devinit hwicap_module_init(void)
+static int __init hwicap_module_init(void)
 {
 	dev_t devt;
 	int retval;
 
 	icap_class = class_create(THIS_MODULE, "xilinx_config");
+	mutex_init(&icap_sem);
 
 	if (xhwicap_major) {
 		devt = MKDEV(xhwicap_major, xhwicap_minor);
@@ -883,7 +902,7 @@ static int __devinit hwicap_module_init(void)
 	return retval;
 }
 
-static void __devexit hwicap_module_cleanup(void)
+static void __exit hwicap_module_cleanup(void)
 {
 	dev_t devt = MKDEV(xhwicap_major, xhwicap_minor);
 
diff --git a/drivers/char/xilinx_hwicap/xilinx_hwicap.h b/drivers/char/xilinx_hwicap/xilinx_hwicap.h
index ae771ca..eb8a827 100644
--- a/drivers/char/xilinx_hwicap/xilinx_hwicap.h
+++ b/drivers/char/xilinx_hwicap/xilinx_hwicap.h
@@ -48,9 +48,9 @@ struct hwicap_drvdata {
 	u8 write_buffer[4];
 	u32 read_buffer_in_use;	  /* Always in [0,3] */
 	u8 read_buffer[4];
-	u32 mem_start;		  /* phys. address of the control registers */
-	u32 mem_end;		  /* phys. address of the control registers */
-	u32 mem_size;
+	resource_size_t mem_start;/* phys. address of the control registers */
+	resource_size_t mem_end;  /* phys. address of the control registers */
+	resource_size_t mem_size;
 	void __iomem *base_address;/* virt. address of the control registers */
 
 	struct device *dev;
@@ -61,7 +61,7 @@ struct hwicap_drvdata {
 	const struct config_registers *config_regs;
 	void *private_data;
 	bool is_open;
-	struct semaphore sem;
+	struct mutex sem;
 };
 
 struct hwicap_driver_config {
@@ -164,29 +164,29 @@ struct config_registers {
 #define XHI_DISABLED_AUTO_CRC       0x0000DEFCUL
 
 /**
- * hwicap_type_1_read: Generates a Type 1 read packet header.
- * @parameter: Register is the address of the register to be read back.
+ * hwicap_type_1_read - Generates a Type 1 read packet header.
+ * @register: is the address of the register to be read back.
  *
  * Generates a Type 1 read packet header, which is used to indirectly
  * read registers in the configuration logic.  This packet must then
  * be sent through the icap device, and a return packet received with
  * the information.
  **/
-static inline u32 hwicap_type_1_read(u32 Register)
+static inline u32 hwicap_type_1_read(u32 register)
 {
 	return (XHI_TYPE_1 << XHI_TYPE_SHIFT) |
-		(Register << XHI_REGISTER_SHIFT) |
+		(register << XHI_REGISTER_SHIFT) |
 		(XHI_OP_READ << XHI_OP_SHIFT);
 }
 
 /**
- * hwicap_type_1_write: Generates a Type 1 write packet header
- * @parameter: Register is the address of the register to be read back.
+ * hwicap_type_1_write - Generates a Type 1 write packet header
+ * @register: is the address of the register to be read back.
  **/
-static inline u32 hwicap_type_1_write(u32 Register)
+static inline u32 hwicap_type_1_write(u32 register)
 {
 	return (XHI_TYPE_1 << XHI_TYPE_SHIFT) |
-		(Register << XHI_REGISTER_SHIFT) |
+		(register << XHI_REGISTER_SHIFT) |
 		(XHI_OP_WRITE << XHI_OP_SHIFT);
 }
 
-- 
1.5.3.4-dirty

^ permalink raw reply related

* Re: Could the DTS experts look at this?
From: Timur Tabi @ 2008-02-11 17:57 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev
In-Reply-To: <200802100647.56590.arnd@arndb.de>

Arnd Bergmann wrote:

> Maybe we can introduce a more generic way of having conditional
> device nodes in the tree that get knocked out in the boot wrapper.

I've been thinking about doing just this for quite some time now.  I've had a 
few informal discussions without people about.

One idea is to allow attaching simple conditional expressions (like X is <, =, 
or > than Y) to a node.  It is the responsibility of the code that parses the 
device tree to assign values to X and Y.  For instance, they could be the names 
of U-Boot environment variables.  If the expression is false, then the node is 
removed (or ignored) from the device tree.  If it's true, then it's kept in.

-- 
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply

* Re: [PATCH] drivers/base: export gpl (un)register_memory_notifier
From: Dave Hansen @ 2008-02-11 16:50 UTC (permalink / raw)
  To: Jan-Bernd Themann
  Cc: Themann, Jan-Bernd, netdev, linux-kernel, Thomas Klein, linux-ppc,
	Christoph Raisch, Greg KH
In-Reply-To: <200802111724.12416.ossthema@de.ibm.com>

On Mon, 2008-02-11 at 17:24 +0100, Jan-Bernd Themann wrote:
> Drivers like eHEA need memory notifiers in order to 
> update their internal DMA memory map when memory is added
> to or removed from the system.
> 
> Patch for eHEA memory hotplug support that uses these functions:
> http://www.spinics.net/lists/netdev/msg54484.html
> 
> Signed-off-by: Jan-Bernd Themann <themann@de.ibm.com>

How about you fix up the driver, first?  Then, this can go in.

-- Dave

^ permalink raw reply

* Re: [PATCH] drivers/base: export gpl (un)register_memory_notifier
From: Dave Hansen @ 2008-02-11 16:47 UTC (permalink / raw)
  To: Jan-Bernd Themann
  Cc: Thomas Klein, Themann, Jan-Bernd, netdev, apw, linux-kernel,
	Thomas Klein, linux-ppc, Christoph Raisch, Badari Pulavarty,
	Greg KH
In-Reply-To: <200802111724.12416.ossthema@de.ibm.com>

On Mon, 2008-02-11 at 17:24 +0100, Jan-Bernd Themann wrote:
>  the eHEA patch belongs to a patchset that is usually
>  added by Jeff Garzik once this dependency (EXPORTS)
>  is resolved.

I know that's already in mainline but, man, that code is nasty.  It has
stuff indented 7 levels or so and is virtually impossible to read.

This:

> int ehea_create_busmap(void)
> {
>         u64 vaddr = EHEA_BUSMAP_START;
>         unsigned long high_section_index = 0;
>         int i;
> 
>         /*
>          * Sections are not in ascending order -> Loop over all sections and
>          * find the highest PFN to compute the required map size.
>         */
>         ehea_bmap.valid_sections = 0;
> 
>         for (i = 0; i < NR_MEM_SECTIONS; i++)
>                 if (valid_section_nr(i))
>                         high_section_index = i;

is also completely bogus for arch-independent code.  If someone ever
wants to do ppc without sparsemem (or redoes internal sparsemem
interfaces), this code will break.  

Also, keeping your own mapping of all of memory is really nasty.  With
SPARSEMEM_EXTREME/VMEMMAP you can have extremely sparse physical memory,
and this:

>         ehea_bmap.entries = high_section_index + 1;
>         ehea_bmap.vaddr = vmalloc(ehea_bmap.entries * sizeof(*ehea_bmap.vaddr));

could theoretically consume all of memory if the sections are very sparse.

Could you please try to explain what the heck this driver is doing?
We'll try to fix up the generic interfaces so that you can deal with
things in a more generic fashion, but it can't stay this way.

Also, just ripping down and completely re-doing the entire mass of cards
every time a 16MB area of memory is added or removed seems like an
awfully big sledgehammer to me.  I would *HATE* to see anybody else
using this driver as an example to work off of?  Can't you just keep
track of which areas the driver is actually *USING* and only worry about
changing mappings if that intersects with an area having hotplug done on
it?

Can you please give it some CodingStyle love and make it so that it
doesn't indent so much?  Something like this:

diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
index c051c7e..72c5652 100644
--- a/drivers/net/ehea/ehea_main.c
+++ b/drivers/net/ehea/ehea_main.c
@@ -2638,38 +2638,40 @@ static void ehea_rereg_mrs(struct work_struct *work)
 	down(&dlpar_mem_lock);
 	ehea_info("LPAR memory enlarged - re-initializing driver");
 
-	list_for_each_entry(adapter, &adapter_list, list)
-		if (adapter->active_ports) {
-			/* Shutdown all ports */
-			for (i = 0; i < EHEA_MAX_PORTS; i++) {
-				struct ehea_port *port = adapter->port[i];
-
-				if (port) {
-					struct net_device *dev = port->netdev;
-
-					if (dev->flags & IFF_UP) {
-						down(&port->port_lock);
-						netif_stop_queue(dev);
-						ret = ehea_stop_qps(dev);
-						if (ret) {
-							up(&port->port_lock);
-							goto out;
-						}
-						port_napi_disable(port);
-						up(&port->port_lock);
-					}
-				}
-			}
-
-			/* Unregister old memory region */
-			ret = ehea_rem_mr(&adapter->mr);
+	list_for_each_entry(adapter, &adapter_list, list) {
+		if (!adapter->active_ports)
+			continue;
+		/* Shutdown all ports */
+		for (i = 0; i < EHEA_MAX_PORTS; i++) {
+			struct ehea_port *port = adapter->port[i];
+			struct net_device *dev;
+			if (!port)
+				continue;
+
+			dev = port->netdev;
+			if (!(dev->flags & IFF_UP))
+				continue;
+
+			down(&port->port_lock);
+			netif_stop_queue(dev);
+			ret = ehea_stop_qps(dev);
 			if (ret) {
-				ehea_error("unregister MR failed - driver"
-					   " inoperable!");
+				up(&port->port_lock);
 				goto out;
 			}
+			port_napi_disable(port);
+			up(&port->port_lock);
 		}
 
+		/* Unregister old memory region */
+		ret = ehea_rem_mr(&adapter->mr);
+		if (ret) {
+			ehea_error("unregister MR failed - driver"
+				   " inoperable!");
+			goto out;
+		}
+	}
+
 	ehea_destroy_busmap();
 	ret = ehea_create_busmap();
 	if (ret) {


-- Dave

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox