[U-Boot] [PATCH v1 1/2] NET: Move MDIO regs out of TSEC Space

[Kumar Gala]

>>> diff --git a/include/asm-ppc/immap_85xx.h b/include/asm-ppc/
>>> immap_85xx.h index 4194295..6c9baac 100644
>>> --- a/include/asm-ppc/immap_85xx.h
>>> +++ b/include/asm-ppc/immap_85xx.h
>>> @@ -1932,4 +1932,14 @@ typedef struct ccsr_gur { #define
>>> CONFIG_SYS_MPC85xx_USB_ADDR \
>>> 	(CONFIG_SYS_IMMR + CONFIG_SYS_MPC85xx_USB_OFFSET)
>>>
>>> +/* TSEC and MDIO OFFSETS */
>>> +#define CONFIG_SYS_TSEC1_OFFSET		(0x24000)
>>> +#define TSEC_SIZE			(0x01000)
>>> +
>>> +#define CONFIG_SYS_MDIO1_OFFSET		(0x24520)
>>> +#define MDIO_OFFSET     		(0x01000)
>>> +
>>> +#define TSEC_BASE_ADDR		(CONFIG_SYS_IMMR +
>> CONFIG_SYS_TSEC1_OFFSET)
>>> +#define MDIO_BASE_ADDR		(CONFIG_SYS_IMMR +
>> CONFIG_SYS_MDIO1_OFFSET)
>>> +
>>
>> Do we even need these here?  We haven't historically.
>>
> Since these are platform dependent, we want the above to be defined
> here.

than we should remove them from tsec.h

>>> #endif /*__IMMAP_85xx__*/
>>> diff --git a/include/tsec.h b/include/tsec.h index 0ac3034..342c07e
>>> 100644
>>> --- a/include/tsec.h
>>> +++ b/include/tsec.h
>>> @@ -7,7 +7,7 @@
>>> *  terms of the GNU Public License, Version 2, incorporated
>>> *  herein by reference.
>>> *
>>> - * Copyright 2004, 2007 Freescale Semiconductor, Inc.
>>> + * Copyright 2004, 2007, 2009  Freescale Semiconductor, Inc.
>>> * (C) Copyright 2003, Motorola, Inc.
>>> * maintained by Xianghua Xiao (x.xiao at motorola.com)
>>> * author Andy Fleming
>>> @@ -24,18 +24,34 @@
>>>    #define CONFIG_SYS_TSEC1_OFFSET	(0x24000)
>>> #endif
>>>
>>> -#define TSEC_SIZE	0x01000
>>> +#ifndef TSEC_SIZE
>>> +	#define TSEC_SIZE	0x01000
>>> +#endif
>>
>> Do we really need a #ifndef here?
>>
> Yes. In case it is not defined in immap_85xx, wefall back to the older
> options.

This duplication is pointless.  When is TSEC_SIZE anything about 0x1000?

>>> +
>>> +#ifndef CONFIG_SYS_MDIO1_OFFSET
>>> +    #define CONFIG_SYS_MDIO1_OFFSET	(0x24520)
>>> +#endif

The only time this will change is between TSEC2 and TSEC1.  Nothing  
else has caused this to change.

>>
>> the parens aren't needed.
>>
> OK.
>
>>> +
>>> +#ifndef MDIO_OFFSET
>>> +	#define MDIO_OFFSET	0x01000
>>> +#endif
>>
>> How about calling this TSEC_MDIO_OFFSET
>>
> OK, I will change it (I just carried forward the existing #define
> variable )
>>>
>>> /* FIXME:  Should these be pushed back to 83xx and 85xx
>> config files?
>>> */ #if defined(CONFIG_MPC85xx) || defined(CONFIG_MPC86xx) \
>>> 	|| defined(CONFIG_MPC83xx)
>>> +#ifndef TSEC_BASE_ADDR
>>>    #define TSEC_BASE_ADDR	(CONFIG_SYS_IMMR +
>> CONFIG_SYS_TSEC1_OFFSET)
>>> #endif
>>> +#ifndef MDIO_BASE_ADDR
>>> +    #define MDIO_BASE_ADDR	(CONFIG_SYS_IMMR +
>>> CONFIG_SYS_MDIO1_OFFSET)
>>> +#endif
>>> +#endif
>>
>> Do we really need the #ifndef's here?
>>
> The correct place for defining the MDIO and TSEC base addr is the
> platformm file as they are
> dependent on the platform not on the IP itself ( although in most  
> cases
> for a particular version of IP,
> this may not change) and since these were earlier defined here, I have
> defined these as a part of immap_85xx.h
> But for all other platforms they are not defined yet, hence #ifndefs  
> are
> required.
> Once these base addrs are moved to platform specific files,they can be
> removed complatelly form here.

I'm ok if you want to move them out of tsec.h into immap_85xx.h,  
imap_83xx.h, and imap_86xx.h


- k