[PATCH 0/2] Fixes to MPC5200 FEC driver

[PATCH 0/2] Fixes to MPC5200 FEC driver

[Grant Likely]

Oops, send this series yesterday but forgot to include jgarzik, domen
and netdev to the 'to:' list.

These are fixes which should go in for .24

Cheers,
g.

--
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.

[PATCH 1/2] [POWERPC] mpc5200: Fix Kconfig dependancies on MPC5200 FEC device driver

[Grant Likely]

From: Grant Likely <grant.likely@secretlab.ca>

When not building an arch/powerpc kernel, the mpc5200 FEC driver depends
on some symbols which are not defined (BESTCOMM & BESTCOMM_FEC).

This patch flips around the dependancy logic so that it cannot be
selected unless BESTCOMM_FEC is selected first.  Kconfig stops
complaining this way.

Also, the driver only works for arch/powerpc (not arch/ppc) anyway so
it should depend on PPC_MERGE also.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---

 drivers/net/Kconfig |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 867cb73..5f800a6 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -1883,9 +1883,7 @@ config FEC2
 
 config FEC_MPC52xx
 	tristate "MPC52xx FEC driver"
-	depends on PPC_MPC52xx
-	select PPC_BESTCOMM
-	select PPC_BESTCOMM_FEC
+	depends on PPC_MERGE && PPC_MPC52xx && PPC_BESTCOMM_FEC
 	select CRC32
 	select PHYLIB
 	---help---

[PATCH 2/2] [POWERPC] Fix region size check in mpc5200 FEC driver

[Grant Likely]

From: Grant Likely <grant.likely@secretlab.ca>

Driver shouldn't complain if the register range is larger than what
it expects.  This works around failures with some device trees.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---

 drivers/net/fec_mpc52xx.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
index fc1cf0b..a8a0ee2 100644
--- a/drivers/net/fec_mpc52xx.c
+++ b/drivers/net/fec_mpc52xx.c
@@ -879,9 +879,9 @@ mpc52xx_fec_probe(struct of_device *op, const struct of_device_id *match)
 				"Error while parsing device node resource\n" );
 		return rv;
 	}
-	if ((mem.end - mem.start + 1) != sizeof(struct mpc52xx_fec)) {
+	if ((mem.end - mem.start + 1) < sizeof(struct mpc52xx_fec)) {
 		printk(KERN_ERR DRIVER_NAME
-			" - invalid resource size (%lx != %x), check mpc52xx_devices.c\n",
+			" - invalid resource size (%lx < %x), check mpc52xx_devices.c\n",
 			(unsigned long)(mem.end - mem.start + 1), sizeof(struct mpc52xx_fec));
 		return -EINVAL;
 	}

Re: [PATCH 2/2] [POWERPC] Fix region size check in mpc5200 FEC driver

[Ingo Oeser]

Hi Grant,

Grant Likely schrieb:
> From: Grant Likely <grant.likely@secretlab.ca>
> 
> Driver shouldn't complain if the register range is larger than what
> it expects.  This works around failures with some device trees.
> 

But maybe the firmware guys like to know about it?
May I suggest putting this in front of the other check?

if ((mem.end - mem.start + 1) > sizeof(struct mpc52xx_fec)) {
  		printk(KERN_DEBUG DRIVER_NAME
			" - gratious resource size (%lx > %x), check mpc52xx_devices.c\n",
  			(unsigned long)(mem.end - mem.start + 1), sizeof(struct mpc52xx_fec));
}

> -	if ((mem.end - mem.start + 1) != sizeof(struct mpc52xx_fec)) {
> +	if ((mem.end - mem.start + 1) < sizeof(struct mpc52xx_fec)) {
>  		printk(KERN_ERR DRIVER_NAME
> -			" - invalid resource size (%lx != %x), check mpc52xx_devices.c\n",
> +			" - invalid resource size (%lx < %x), check mpc52xx_devices.c\n",
>  			(unsigned long)(mem.end - mem.start + 1), sizeof(struct mpc52xx_fec));
>  		return -EINVAL;
>  	}


Best Regards

Ingo Oeser

Re: [PATCH 2/2] [POWERPC] Fix region size check in mpc5200 FEC driver

[Grant Likely]

On 11/1/07, Ingo Oeser <netdev@axxeo.de> wrote:
> Hi Grant,
>
> Grant Likely schrieb:
> > From: Grant Likely <grant.likely@secretlab.ca>
> >
> > Driver shouldn't complain if the register range is larger than what
> > it expects.  This works around failures with some device trees.
> >
>
> But maybe the firmware guys like to know about it?
> May I suggest putting this in front of the other check?
>
> if ((mem.end - mem.start + 1) > sizeof(struct mpc52xx_fec)) {
>                 printk(KERN_DEBUG DRIVER_NAME
>                         " - gratious resource size (%lx > %x), check mpc52xx_devices.c\n",
>                         (unsigned long)(mem.end - mem.start + 1), sizeof(struct mpc52xx_fec));

Personally, I'm not concerned about it.  Even if the device tree says
the range is larger than what the driver knows about it is not
technically an error. If a new version of the chip appears that is
compatible, but defines a larger register range with extra feature
registers, then this message would be erroneously printed.  Finally,
depending on how you read the mpc5200 user guild, it can be 100% valid
to specify the reg size as 0x800 instead of 0x400.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195