Fixed PHY problems

Fixed PHY problems

[Joakim Tjernlund]

Trying to boot todays linus tree with fixed PHY support enabled gets me this:
Fixed PHY: Registered new driver
Device 'fixed@100:1' does not have a release() function, it is broken and must be fixed.
------------[ cut here ]------------
Badness at drivers/base/core.c:107
Call Trace:
[cffc1dc0] [c0008324] show_stack+0x48/0x194 (unreliable)
[cffc1df0] [c00f4a4c] report_bug+0x84/0xec
[cffc1e00] [c000dde8] program_check_exception+0x274/0x55c
[cffc1e20] [c000f770] ret_from_except_full+0x0/0x4c
--- Exception: 700 at device_release+0x54/0x90
    LR = device_release+0x54/0x90
[cffc1ef0] [c00f5e1c] kobject_cleanup+0xb4/0xb8
[cffc1f10] [c00f6ca0] kref_put+0x68/0x80
[cffc1f20] [c00f5d58] kobject_put+0x24/0x34
[cffc1f30] [c01150bc] put_device+0x1c/0x2c
[cffc1f40] [c0243a9c] fixed_init+0x1cc/0x210
[cffc1f60] [c022e828] kernel_init+0x8c/0x2b4
[cffc1ff0] [c000ff4c] kernel_thread+0x44/0x60

The other thing is, how do I specify a fixed PHY
in OF?

 Jocke

Re: Fixed PHY problems

[Vitaly Bordug]

On Thu, 10 May 2007 19:41:53 +0200
Joakim Tjernlund wrote:

> Trying to boot todays linus tree with fixed PHY support enabled gets
> me this: Fixed PHY: Registered new driver
> Device 'fixed@100:1' does not have a release() function, it is broken
> and must be fixed. ------------[ cut here ]------------
> Badness at drivers/base/core.c:107
> Call Trace:
> [cffc1dc0] [c0008324] show_stack+0x48/0x194 (unreliable)
> [cffc1df0] [c00f4a4c] report_bug+0x84/0xec
> [cffc1e00] [c000dde8] program_check_exception+0x274/0x55c
> [cffc1e20] [c000f770] ret_from_except_full+0x0/0x4c
> --- Exception: 700 at device_release+0x54/0x90
>     LR = device_release+0x54/0x90
> [cffc1ef0] [c00f5e1c] kobject_cleanup+0xb4/0xb8
> [cffc1f10] [c00f6ca0] kref_put+0x68/0x80
> [cffc1f20] [c00f5d58] kobject_put+0x24/0x34
> [cffc1f30] [c01150bc] put_device+0x1c/0x2c
> [cffc1f40] [c0243a9c] fixed_init+0x1cc/0x210
> [cffc1f60] [c022e828] kernel_init+0x8c/0x2b4
> [cffc1ff0] [c000ff4c] kernel_thread+0x44/0x60
> 
> The other thing is, how do I specify a fixed PHY
> in OF?
> 
I have a patch that  fixes it but was busy with other stuff because of merge window.
Can send it to you meanwhile, going to deal with it after next rc released...

>  Jocke
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev


-- 
Sincerely, Vitaly

Re: Fixed PHY problems

[Joakim Tjernlund]

On Fri, 2007-05-11 at 12:07 +0400, Vitaly Bordug wrote:
> On Thu, 10 May 2007 19:41:53 +0200
> Joakim Tjernlund wrote:
> 
> > Trying to boot todays linus tree with fixed PHY support enabled gets
> > me this: Fixed PHY: Registered new driver
> > Device 'fixed@100:1' does not have a release() function, it is broken
> > and must be fixed. ------------[ cut here ]------------
> > Badness at drivers/base/core.c:107
> > Call Trace:
> > [cffc1dc0] [c0008324] show_stack+0x48/0x194 (unreliable)
> > [cffc1df0] [c00f4a4c] report_bug+0x84/0xec
> > [cffc1e00] [c000dde8] program_check_exception+0x274/0x55c
> > [cffc1e20] [c000f770] ret_from_except_full+0x0/0x4c
> > --- Exception: 700 at device_release+0x54/0x90
> >     LR = device_release+0x54/0x90
> > [cffc1ef0] [c00f5e1c] kobject_cleanup+0xb4/0xb8
> > [cffc1f10] [c00f6ca0] kref_put+0x68/0x80
> > [cffc1f20] [c00f5d58] kobject_put+0x24/0x34
> > [cffc1f30] [c01150bc] put_device+0x1c/0x2c
> > [cffc1f40] [c0243a9c] fixed_init+0x1cc/0x210
> > [cffc1f60] [c022e828] kernel_init+0x8c/0x2b4
> > [cffc1ff0] [c000ff4c] kernel_thread+0x44/0x60
> > 
> > The other thing is, how do I specify a fixed PHY
> > in OF?
> > 
> I have a patch that  fixes it but was busy with other stuff because of merge window.
> Can send it to you meanwhile, going to deal with it after next rc released...

Please do, thanks.

Do you have any idea what needs to go into the DTS file to make my
832x(ucc_geth) find it?

Re: Fixed PHY problems

[Joakim Tjernlund]

On Fri, 2007-05-11 at 11:04 +0200, Joakim Tjernlund wrote:
> On Fri, 2007-05-11 at 12:07 +0400, Vitaly Bordug wrote:
> > On Thu, 10 May 2007 19:41:53 +0200
> > Joakim Tjernlund wrote:
> > 
> > > Trying to boot todays linus tree with fixed PHY support enabled gets
> > > me this: Fixed PHY: Registered new driver
> > > Device 'fixed@100:1' does not have a release() function, it is broken
> > > and must be fixed. ------------[ cut here ]------------
> > > Badness at drivers/base/core.c:107
> > > Call Trace:
> > > [cffc1dc0] [c0008324] show_stack+0x48/0x194 (unreliable)
> > > [cffc1df0] [c00f4a4c] report_bug+0x84/0xec
> > > [cffc1e00] [c000dde8] program_check_exception+0x274/0x55c
> > > [cffc1e20] [c000f770] ret_from_except_full+0x0/0x4c
> > > --- Exception: 700 at device_release+0x54/0x90
> > >     LR = device_release+0x54/0x90
> > > [cffc1ef0] [c00f5e1c] kobject_cleanup+0xb4/0xb8
> > > [cffc1f10] [c00f6ca0] kref_put+0x68/0x80
> > > [cffc1f20] [c00f5d58] kobject_put+0x24/0x34
> > > [cffc1f30] [c01150bc] put_device+0x1c/0x2c
> > > [cffc1f40] [c0243a9c] fixed_init+0x1cc/0x210
> > > [cffc1f60] [c022e828] kernel_init+0x8c/0x2b4
> > > [cffc1ff0] [c000ff4c] kernel_thread+0x44/0x60
> > > 
> > > The other thing is, how do I specify a fixed PHY
> > > in OF?
> > > 
> > I have a patch that  fixes it but was busy with other stuff because of merge window.
> > Can send it to you meanwhile, going to deal with it after next rc released...
> 
> Please do, thanks.
> 
> Do you have any idea what needs to go into the DTS file to make my
> 832x(ucc_geth) find it?

hmm, been trying to figure out all day how to make ucc_geth find my
fixed PHY without success. OF experts, please give me some hints.
My board is derived from the in tree mpc832x_mds bord and I am trying to
convert this part:
	mdio@2320 {
			#address-cells = <1>;
			#size-cells = <0>;
			reg = <2320 18>;
			device_type = "mdio";
			compatible = "ucc_geth_phy";

			phy3: ethernet-phy@03 {
				interrupt-parent = < &ipic >;
				interrupts = <11 8>;
				reg = <3>;
				device_type = "ethernet-phy";
			};
			phy4: ethernet-phy@04 {
				interrupt-parent = < &ipic >;
				interrupts = <12 8>;
				reg = <4>;
				device_type = "ethernet-phy";
			};
		};
into fixed PHYs, how do I do that?

 Jocke

Re: Fixed PHY problems

[Vitaly Bordug]

> hmm, been trying to figure out all day how to make ucc_geth find my
> fixed PHY without success. OF experts, please give me some hints.
> My board is derived from the in tree mpc832x_mds bord and I am trying to
> convert this part:
> 	mdio@2320 {
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 			reg = <2320 18>;
> 			device_type = "mdio";
> 			compatible = "ucc_geth_phy";
> 
> 			phy3: ethernet-phy@03 {
> 				interrupt-parent = < &ipic >;
> 				interrupts = <11 8>;
> 				reg = <3>;
> 				device_type = "ethernet-phy";
> 			};
> 			phy4: ethernet-phy@04 {
> 				interrupt-parent = < &ipic >;
> 				interrupts = <12 8>;
> 				reg = <4>;
> 				device_type = "ethernet-phy";
> 			};
> 		};
> into fixed PHYs, how do I do that?

Well, fixed phy didn't made it to have any representation in dts as of now, at least
what I am aware of, and it is just a platform device so far.

Speaking about it, I am recalling long-pending enhancement to unify fixed interface with
bus_id representation in gianfar (that is having numeric bus:device way).

About dts, the only options fixed phy can have, are speed and duplex, it does not trigger irq and
so on, and I see something like 

phy1: fixed-phy@01 {
	reg = <100 1>; /*100Mps, full duplex*/
	device_type = "ethernet-phy";
};

-- 
Sincerely, 
Vitaly

Re: Fixed PHY problems

[Joakim Tjernlund]

On Fri, 2007-05-11 at 18:04 +0400, Vitaly Bordug wrote:
> > hmm, been trying to figure out all day how to make ucc_geth find my
> > fixed PHY without success. OF experts, please give me some hints.
> > My board is derived from the in tree mpc832x_mds bord and I am trying to
> > convert this part:
> > 	mdio@2320 {
> > 			#address-cells = <1>;
> > 			#size-cells = <0>;
> > 			reg = <2320 18>;
> > 			device_type = "mdio";
> > 			compatible = "ucc_geth_phy";
> > 
> > 			phy3: ethernet-phy@03 {
> > 				interrupt-parent = < &ipic >;
> > 				interrupts = <11 8>;
> > 				reg = <3>;
> > 				device_type = "ethernet-phy";
> > 			};
> > 			phy4: ethernet-phy@04 {
> > 				interrupt-parent = < &ipic >;
> > 				interrupts = <12 8>;
> > 				reg = <4>;
> > 				device_type = "ethernet-phy";
> > 			};
> > 		};
> > into fixed PHYs, how do I do that?
> 
> Well, fixed phy didn't made it to have any representation in dts as of now, at least
> what I am aware of, and it is just a platform device so far.
> 
> Speaking about it, I am recalling long-pending enhancement to unify fixed interface with
> bus_id representation in gianfar (that is having numeric bus:device way).
> 
> About dts, the only options fixed phy can have, are speed and duplex, it does not trigger irq and
> so on, and I see something like 
> 
> phy1: fixed-phy@01 {
> 	reg = <100 1>; /*100Mps, full duplex*/
> 	device_type = "ethernet-phy";
> };

Looks resonable to me, although I think reg needs to be <d#100 1>

How do make ucc_geth find it?

PS.
   Could you send me the patch that fixes:
Device 'fixed@100:1' does not have a release() function, it is broken
and must be fixed.

Is this warning harmless?

Re: Fixed PHY problems

[Segher Boessenkool]

>> 			phy3: ethernet-phy@03 {

>> 			phy4: ethernet-phy@04 {

@3 resp. @4 is more in line with existing practice.

> phy1: fixed-phy@01 {
> 	reg = <100 1>; /*100Mps, full duplex*/

The first entry in "reg" has to correspond to the
unit address (@01 in this case -- again, should be
@1 instead).

Never overload the meaning of well-known properties,
esp. not if it conflicts with the existing semantics,
like here.


Segher

Re: Fixed PHY problems

[Hammond Christian-r55079]

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

Hi Vitaly,
 
I was pointed in the direction of message chain "Fixed PHY problems" for
interfacing a fixed PHY to a 83xx UEC. Could you please forward on the
patch you mentioned in the trail?
 
Any ideas if the implementation  succeeded in the end?
 
Christian Hammond
 
NCSG Applications Engineer
 
Tel: +44 1355 355962
Fax: +44 1355 261790
 
Freescale Semiconductor
Kelvin Industrial Estate
East Kilbride
Glasgow G75 0TG
United Kingdom
 
 

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

Re: Fixed PHY problems

[Joakim Tjernlund]

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

On Sat, 2007-05-19 at 15:42 +0100, Hammond Christian-r55079 wrote:
> Hi Vitaly,
>  
> I was pointed in the direction of message chain "Fixed PHY problems"
> for interfacing a fixed PHY to a 83xx UEC. Could you please forward on
> the patch you mentioned in the trail?

Vitaly sent me the patch, had to do some minor tweaking to make it
compile. Attaching the patch.

>  
> Any ideas if the implementation  succeeded in the end?

No, the fixed PHY is on another bus than what ucc_geth expects.
See msg from me titled "Fixed  PHY and MPC832x based boards don't work"
from about a week ago.

 Jocke

>  
> Christian Hammond
>  
> NCSG Applications Engineer
>  
> Tel: +44 1355 355962
> Fax: +44 1355 261790
>  
> Freescale Semiconductor
> Kelvin Industrial Estate
> East Kilbride
> Glasgow G75 0TG
> United Kingdom


[-- Attachment #2: fixed_driver.patch --]
[-- Type: text/x-patch, Size: 3463 bytes --]

PHY fixed driver: reworked error handling and release path

device_bind_driver() error code returning has been fixed. 
release() function has been written, so that to free resources
in correct way.

Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>

---

 drivers/net/phy/fixed.c |   72 +++++++++++++++++++++++++++--------------------
 1 files changed, 41 insertions(+), 31 deletions(-)

diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
index 68c99b4..0d77805 100644
--- a/drivers/net/phy/fixed.c
+++ b/drivers/net/phy/fixed.c
@@ -187,6 +187,19 @@ static struct phy_driver fixed_mdio_driver = {
 	.driver 	= { .owner = THIS_MODULE,},
 };
 
+static void fixed_mdio_release (struct device * dev)
+{
+	struct phy_device *phydev = container_of(dev, struct phy_device, dev);
+	struct mii_bus *bus = phydev->bus;
+	struct fixed_info *fixed = bus->priv;
+
+	kfree(phydev);
+	kfree(bus->dev);
+	kfree(bus);
+	kfree(fixed->regs);
+	kfree(fixed);
+}
+
 /*-----------------------------------------------------------------------------
  *  This func is used to create all the necessary stuff, bind
  * the fixed phy driver and register all it on the mdio_bus_type.
@@ -221,6 +234,12 @@ static int fixed_mdio_register_device(int number, int speed, int duplex)
 	}
 
 	fixed->regs = kzalloc(MII_REGS_NUM*sizeof(int), GFP_KERNEL);
+	if (NULL == fixed->regs) {
+		kfree(dev);
+		kfree(new_bus);
+		kfree(fixed);
+		return -ENOMEM;
+	}
 	fixed->regs_num = MII_REGS_NUM;
 	fixed->phy_status.speed = speed;
 	fixed->phy_status.duplex = duplex;
@@ -249,8 +268,11 @@ static int fixed_mdio_register_device(int number, int speed, int duplex)
 	fixed->phydev = phydev;
 
 	if(NULL == phydev) {
-		err = -ENOMEM;
-		goto device_create_fail;
+		kfree(dev);
+		kfree(new_bus);
+		kfree(fixed->regs);
+		kfree(fixed);
+		return -ENOMEM;
 	}
 
 	phydev->irq = PHY_IGNORE_INTERRUPT;
@@ -262,44 +284,32 @@ static int fixed_mdio_register_device(int number, int speed, int duplex)
 	else
 		snprintf(phydev->dev.bus_id, BUS_ID_SIZE,
 				"fixed@%d:%d", speed, duplex);
+
 	phydev->bus = new_bus;
 
+	phydev->dev.driver = &fixed_mdio_driver.driver;
+ 	phydev->dev.release = fixed_mdio_release;
+ 
+ 	err = phydev->dev.driver->probe(&phydev->dev);
+ 	if(err < 0) {
+ 		printk(KERN_ERR "Phy %s: problems with fixed driver\n",
+				phydev->dev.bus_id);
+ 		kfree(phydev);
+ 		kfree(dev);
+ 		kfree(new_bus);
+ 		kfree(fixed->regs);
+ 		kfree(fixed);
+ 		return err;
+ 	}
+ 
+	down_write(&phydev->dev.bus->subsys.rwsem);
 	err = device_register(&phydev->dev);
 	if(err) {
 		printk(KERN_ERR "Phy %s failed to register\n",
 				phydev->dev.bus_id);
-		goto bus_register_fail;
-	}
-
-	/*
-	   the mdio bus has phy_id match... In order not to do it
-	   artificially, we are binding the driver here by hand;
-	   it will be the same for all the fixed phys anyway.
-	 */
-	phydev->dev.driver = &fixed_mdio_driver.driver;
-
-	err = phydev->dev.driver->probe(&phydev->dev);
-	if(err < 0) {
-		printk(KERN_ERR "Phy %s: problems with fixed driver\n",phydev->dev.bus_id);
-		goto probe_fail;
 	}
 
-	err = device_bind_driver(&phydev->dev);
-	if (err)
-		goto probe_fail;
-
 	return 0;
-
-probe_fail:
-	device_unregister(&phydev->dev);
-bus_register_fail:
-	kfree(phydev);
-device_create_fail:
-	kfree(dev);
-	kfree(new_bus);
-	kfree(fixed);
-
-	return err;
 }
 
 

RE: Fixed PHY problems

[Hammond Christian-r55079]

Thanks,

Regarding the PHY set-up in the dts once the patch has been applied, how
should it be implemented?

I had edited from...

phy1:	ethernet-phy@01 {
		reg =3D <0>;
		device_type =3D "ethernet-phy";
		interface =3D <6>;
	};=20

To...

phy1:	fixed-phy@64 {
		reg =3D <d#100>;
		device_type =3D "ethernet-phy";
		interface =3D <6>;
	};

When bringing up the fixed phy in ifconfig I get the following recurring
dump and prompt access is lost. Should this implementation not just
ignore the phy set-up?

~ # ifconfig eth1 10.137.32.175 netmask 255.255.255.0 up
read wrong value : mii_id 100,mii_reg 2, base d100c120
read wrong value : mii_id 100,mii_reg 3, base d100c120
eth1: PHY is Generic MII (ffffffff)
~ # read wrong value : mii_id 100,mii_reg 4, base d100c120
read wrong value : mii_id 100,mii_reg 0, base d100c120
read wrong value : mii_id 100,mii_reg 1, base d100c120
read wrong value : mii_id 100,mii_reg 1, base d100c120
read wrong value : mii_id 100,mii_reg 5, base d100c120
eth1: Full Duplex
eth1: Speed 100BT
eth1: Link is up
read wrong value : mii_id 100,mii_reg 1, base d100c120
read wrong value : mii_id 100,mii_reg 1, base d100c120
read wrong value : mii_id 100,mii_reg 5, base d100c120
=20

-----Original Message-----
From: Joakim Tjernlund [mailto:joakim.tjernlund@transmode.se]=20
Sent: 20 May 2007 11:17
To: Hammond Christian-r55079
Cc: linuxppc-dev@ozlabs.org
Subject: Re: Fixed PHY problems

On Sat, 2007-05-19 at 15:42 +0100, Hammond Christian-r55079 wrote:
> Hi Vitaly,
> =20
> I was pointed in the direction of message chain "Fixed PHY problems"
> for interfacing a fixed PHY to a 83xx UEC. Could you please forward on

> the patch you mentioned in the trail?

Vitaly sent me the patch, had to do some minor tweaking to make it
compile. Attaching the patch.

> =20
> Any ideas if the implementation  succeeded in the end?

No, the fixed PHY is on another bus than what ucc_geth expects.
See msg from me titled "Fixed  PHY and MPC832x based boards don't work"
from about a week ago.

 Jocke

> =20
> Christian Hammond
> =20
> NCSG Applications Engineer
> =20
> Tel: +44 1355 355962
> Fax: +44 1355 261790
> =20
> Freescale Semiconductor
> Kelvin Industrial Estate
> East Kilbride
> Glasgow G75 0TG
> United Kingdom

RE: Fixed PHY problems

[Joakim Tjernlund]

On Mon, 2007-05-21 at 13:46 +0100, Hammond Christian-r55079 wrote:
> Thanks,
>=20
> Regarding the PHY set-up in the dts once the patch has been applied, =
how
> should it be implemented?
>=20
> I had edited from...
>=20
> phy1:	ethernet-phy@01 {
> 		reg =3D <0>;
> 		device_type =3D "ethernet-phy";
> 		interface =3D <6>;
> 	};=20
>=20
> To...
>=20
> phy1:	fixed-phy@64 {
> 		reg =3D <d#100>;
> 		device_type =3D "ethernet-phy";
> 		interface =3D <6>;
> 	};
>=20
> When bringing up the fixed phy in ifconfig I get the following =
recurring
> dump and prompt access is lost. Should this implementation not just
> ignore the phy set-up?

Yes, but some bits are missing and I don't know OF/PHY lib/MDIO well
enough to make out what to do.

  Jocke