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