[PATCH] [Rev2] MPC5121 FEC support

Sam Ravnborg sam at ravnborg.org
Wed Jun 18 07:00:55 EST 2008


On Tue, Jun 17, 2008 at 03:03:54PM -0500, Scott Wood wrote:
> Sam Ravnborg wrote:
> >On Tue, Jun 17, 2008 at 02:33:28PM -0500, Scott Wood wrote:
> >>John Rigby wrote:
> >>>config FS_ENET
> >>>       tristate "Freescale Ethernet Driver"
> >>>-       depends on CPM1 || CPM2
> >>>+       depends on CPM1 || CPM2 || FS_ENET_MPC5121_FEC
> >>>       select MII
> >>>       select PHYLIB
> >>>
> >>>+config FS_ENET_MPC5121_FEC
> >>>+	bool "Freescale MPC512x FEC driver"
> >>>+	depends on PPC_MPC512x
> >>>+	select FS_ENET
> >>>+	select PPC_CPM_NEW_BINDING
> >>>+	default y
> >>No default y.
> >I by the way do not see the need for the prompt of FS_ENET.
> 
> Agreed, especially since it's overly broad (there is Freescale ethernet 
> hardware that this driver doesn't support).  We'd need to change depends 
> into selects in the more specific entries.
> 
> >Do you ever want to change it if one of the dependencies
> >are selected?
> 
> Do you mean if CPM1 or CPM2 is selected?  Yes, it's quite possible that 
> the user has no need for the CPM ethernet and would rather reclaim the 
> memory (especially on CPM1, which has boards as small as 8MiB).

I took a closer look now.
And I can see that FS_ENET is indeed a driver and CPM1, CPM2,  FS_ENET_MPC5121_FEC
are all booleans that say that it may make sense to use this driver.

But I was misguided by:
>+config FS_ENET_MPC5121_FEC
>+  select FS_ENET
This is not good.

In general when you select a symbol that has dependencies you are almost
always on the wrong track.
Use a dependency here with a sane default - then people can 
set it to 'n' if they really do not want this driver.

Spreading selects too much is just causing you pain in the long run.

	Sam



More information about the Linuxppc-dev mailing list