[U-Boot] [PATCH 6/7] mxc_spi: add support for i.MX35 processor

[Stefano Babic <sbabic@denx.de>]

On 01/19/2011 08:48 AM, Wolfgang Denk wrote:
> Dear Stefano Babic,
> 
> In message <1295012124-15551-6-git-send-email-sbabic@denx.de> you wrote:
>> Signed-off-by: Stefano Babic <sbabic@denx.de>
>> ---
>>  drivers/spi/mxc_spi.c |   96 +++++++++++++++++++++++++++++++++++++------------
>>  1 files changed, 73 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c
>> index d558137..b353c83 100644
>> --- a/drivers/spi/mxc_spi.c
>> +++ b/drivers/spi/mxc_spi.c
>> @@ -70,6 +70,8 @@ static unsigned long spi_bases[] = {
>>  	0x53f84000,
>>  };
>>  
>> +#define spi_cfg	spi_cfg_mx3
> ...
>> +#define spi_cfg	spi_cfg_mx51
> 
> Hm... this repeats below, but in the end both spi_cfg_mx3() and
> spi_cfg_mx51() are just static functions within the same source file,
> with #ifdef's around them so only one can ever be enabled at a time.

You are right, there is already an #ifdef. I think I had in mind to
remove the #ifdef surrounding the functions, but I give up because I
have added unneeded code (mx51 code for mx3 and viceversa). I will fix it.

> 
> I suggest you omit all these "#define spi_cfg" lines and rename both
> versions of these functions into spi_cfg_mx().
> 
>> +#define MXC_CSPIRXDATA		0x00
>> +#define MXC_CSPITXDATA		0x04
>> +#define MXC_CSPICTRL		0x08
>> +#define MXC_CSPIINT		0x0C
>> +#define MXC_CSPIDMA		0x10
>> +#define MXC_CSPISTAT		0x14
>> +#define MXC_CSPIPERIOD		0x18
>> +#define MXC_CSPITEST		0x1C
> 
> As mentioned before: please use a C struct.

This is another issue. I agree that is ugly code, but it comes from the
originally written driver for the i.MX31. This issue should be fixed,
but in a separate patch, and for all supported processors
(MX.31/MX.25/MX.51/MX.35).

There are at the moment two other patches by Anatolji regarding this
driver. I have already rebased one of them and post to the ML, but I
admit that, as they are not in the same patchset, it is quite difficult
to have an overview of the changes. My proposal is that I will add these
other two patches to my patchset to simplify review.

Best regards,.
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================