Re: [PATCH 3/3] Add support to M54xx DMA FEC Driver

[Philippe De Muyter <phdm@macqel.be>]

Hi Stany & Greg

Seeing that I was not the only one wanting to have the m54xx fec dma
driver merged in, and hoping to compare Stany's version to mine,
I have rebased (step by step) my patch from v2.38 to v3.6rc2.
The driver still works and perhaps even better due to some fixes
in other m68k area.

Unfortunately I have not being able to compare it yet fully with Stany's
version because Stany's patch 2/2 did not apply (using `git am') to v3.5
or v3.6rc2.

I have checked my patch using a recent version of checkpatch.pl (not the
v3.5 version, because v3.5 version of checkpatch.pl fails with :
Nested quantifiers in regex; marked by <-- HERE in m/(\((?:[^\(\)]++ <-- HERE |(
?-1))*\))/ at scripts/checkpatch.pl line 340.))

and I am now at :
    464 WARNING: line over 80 characters
     90 WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt

Many "volatile" warnings are about such definitions :

#define FEC_FECFRST(x)                 (*(volatile unsigned int *)(x + 0x1C4))
which are afterwards used with

+       FEC_FECFRST(base_addr) |= FEC_SW_RST;
+       FEC_FECFRST(base_addr) &= ~FEC_SW_RST;
+               FEC_FECFRST(base_addr) |= FEC_SW_RST;
+               FEC_FECFRST(base_addr) &= ~FEC_SW_RST;
+       FEC_FECFRST(base_addr) |= FEC_SW_RST | FEC_RST_CTL;
+       FEC_FECFRST(base_addr) &= ~FEC_SW_RST;

Any advice about those ones ?

while many "80 characters" ones are about :
#4014: FILE: arch/m68k/platform/coldfire/MCD_tasks.c:2406:
+       0x6000000b, /* 0098(:1560):      DRD2A: EU0=0 EU1=0 EU2=0 EU3=11 EXT ini
t=0 WS=0 RS=0 */

I would like to keep those lines intact because the comment seems to actually
be the assembler source of the hex value at left, which seems to be a
microcode, and it makes sense to me to keep that on one line.  What do
you think about that ?

I did not include the current status of the patch because of its size
(I did not separate the dma part of the ethernet driver part because
the dma part is useless without the ethernet driver, and linking the
ethernet driver cannot succeed without the dma part), but if you ask,
I'll send it privately.

Best regards

Philippe

-- 
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles