RE: [PATCH 1/3] Freescale QE UCC gigabit ethernet driver

2006-07-07 Thread Li Yang-r58472
 -Original Message-
 From: Kumar Gala [mailto:[EMAIL PROTECTED]
 Sent: Thursday, July 06, 2006 9:45 PM
 To: Li Yang-r58472
 Cc: Andrew Morton; [EMAIL PROTECTED]; netdev@vger.kernel.org;
 [EMAIL PROTECTED]
 Subject: Re: [PATCH 1/3] Freescale QE UCC gigabit ethernet driver
 
 Nack.  Here are some high level things that should be addressed:
 * There is a ton of debug code that should probably be stripped out,
 such as dump_regs, etc..

The reason I left these debug code in is that, QE is very flexible.
User probably needs to fine tune parameters for their own application.
It will be good to give them some clue doing that.  It's fine to remove
them, if you do think they are too verbose.
 * The phy support should use the phylib instead

I'll do this when I do get some time. ;)
 * convert from being a platform_device to of_device

I'm not up against moving to use of_device.  But why are we putting
extra effort in closing the door to backward compatibility with ppc
arch, and doesn't provide any new feature. ;)
 
 - kumar
 
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] Freescale QE UCC gigabit ethernet driver

2006-07-07 Thread Kumar Gala


On Jul 7, 2006, at 3:59 AM, Li Yang-r58472 wrote:


-Original Message-
From: Kumar Gala [mailto:[EMAIL PROTECTED]
Sent: Thursday, July 06, 2006 9:45 PM
To: Li Yang-r58472
Cc: Andrew Morton; [EMAIL PROTECTED]; netdev@vger.kernel.org;
[EMAIL PROTECTED]
Subject: Re: [PATCH 1/3] Freescale QE UCC gigabit ethernet driver

Nack.  Here are some high level things that should be addressed:
* There is a ton of debug code that should probably be stripped out,
such as dump_regs, etc..


The reason I left these debug code in is that, QE is very flexible.
User probably needs to fine tune parameters for their own application.
It will be good to give them some clue doing that.  It's fine to  
remove

them, if you do think they are too verbose.


Then I suggest you look at debugfs.


* The phy support should use the phylib instead


I'll do this when I do get some time. ;)

* convert from being a platform_device to of_device


I'm not up against moving to use of_device.  But why are we putting
extra effort in closing the door to backward compatibility with ppc
arch, and doesn't provide any new feature. ;)


Because the intent is for arch/ppc to go away in the future.  You are  
introducing a new driver and a new platform, this is the ideal time  
for us to use of_device.  The benefits are that the driver is the  
ideal place to parse the details of the device node rather than  
platform code in arch/powerpc.


- kumar
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html