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

Stefano Babic
2011-01-19T10:09:09Z

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
=====================================================================