[regression] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[regression] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[Frans Pop]

[-- Attachment #1: Type: text/plain, Size: 1055 bytes --]

Starting with kernel 2.6.26 I've noticed that the harddisk (HDD) led on my 
QNAP TS-109 (which has a Marvel SATA controller as platform device) no 
longer flashes when there is disk activity. Instead it will just 
stay "on" all the time. The led worked fine with 2.6.25.

I have bisected this to the following series of commits, which are all 
libata or sata_mv changes:
9363c3825ea9ad7..094e50b2f74146d8ee
Further bisection is not possible because between those commits the 
harddisk is not recognized at all.

Lennert Buytenhek suggested to try disabling NCQ, and that does indeed 
make a difference.

After booting the system I did 'echo 1 >/sys/block/sdX/device/queue_depth' 
and after that the HDD led showed disk activity again.

After enabling NCQ again ('echo 31 >/sys/...'), the led again becomes 
unresponsive and just stays on.

Attached a diff between sata_mv hardware registers between 2.6.25 and 
2.6.26 obtained using a patch provided by Mark Lord. (For some reason
"Host" and "ATA Shadow" regs were not available on 2.6.25.)

Cheers,
FJP


[-- Attachment #2: qnap_mv0.regs.diff --]
[-- Type: text/x-diff, Size: 9944 bytes --]

--- qnap_mv0_2.6.25.regs	2008-08-11 20:13:14.000000000 +0200
+++ qnap_mv0.regs	2008-08-11 19:08:18.000000000 +0200
@@ -1,102 +1,106 @@
 CRPBs:
-  0: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=52630
-  1: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=54883
-  2: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=57132
-  3: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=59382
-  4: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=61878
-  5: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=64515
-  6: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=1121
-  7: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=3279
-  8: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=5368
-  9: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=7277
- 10: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=9196
- 11: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=11005
- 12: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=12797
- 13: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=14946
- 14: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=16923
- 15: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=18722
- 16: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=20525
- 17: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=30791
- 18: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=11866
- 19: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=56565
- 20: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=64129
- 21: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=4382
- 22: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=52366
- 23: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=28236
-=24: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=30786
- 25: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=33358
- 26: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=35931
- 27: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=38729
- 28: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=41622
- 29: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=44619
- 30: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=47195
- 31: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=49739
+  0: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=54641
+  1: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=55675
+  2: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=62732
+  3: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=3073
+  4: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=8681
+  5: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=9721
+  6: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=10762
+  7: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=17302
+  8: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=23001
+  9: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=52336
+ 10: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=53359
+ 11: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=56477
+ 12: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=37642
+ 13: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=38704
+ 14: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=39764
+ 15: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=46417
+ 16: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=52200
+ 17: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=32877
+ 18: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=62100
+ 19: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=1606
+ 20: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=34537
+ 21: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=35359
+ 22: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=36419
+ 23: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=51224
+ 24: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=58938
+=25: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=10808
+ 26: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=11848
+ 27: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=12877
+ 28: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=22037
+ 29: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=28404
+ 30: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=26248
+ 31: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=53609
 
 CRQBs:
-  0: 07d47000 00000000 00000000 00000000 00ca0000 ..
-  1: 07d47000 00000000 00000000 00000000 00ca0000 ..
-  2: 07d47000 00000000 00000000 00000000 00ca0000 ..
-  3: 07d47000 00000000 00000000 00000000 00ca0000 ..
-  4: 07d47000 00000000 00000000 00000000 00ca0000 ..
-  5: 07d47000 00000000 00000000 00000000 00ca0000 ..
-  6: 07d47000 00000000 00000000 00000000 00ca0000 ..
-  7: 07d47000 00000000 00000000 00000000 00ca0000 ..
-  8: 07d47000 00000000 00000000 00000000 00ca0000 ..
-  9: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 10: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 11: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 12: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 13: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 14: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 15: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 16: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 17: 07d47000 00000000 00000001 00000000 00c80000 ..
- 18: 07d47000 00000000 00000001 00000000 00c80000 ..
- 19: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 20: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 21: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 22: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 23: 07d47000 00000000 00000001 00000000 00c80000 ..
- 24: 07d47000 00000000 00000001 00000000 00c80000 ..
-=25: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 26: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 27: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 28: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 29: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 30: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 31: 07d47000 00000000 00000000 00000000 00ca0000 ..
+  0: 07d59000 00000000 00020002 00000000 08610000 ..
+  1: 07d04000 00000000 00040004 00000000 08610000 ..
+  2: 07d58000 00000000 00000000 00000000 18610000 ..
+  3: 07d58000 00000000 00000000 00000000 08610000 ..
+  4: 07d58000 00000000 00000000 00000000 08610000 ..
+  5: 07d59000 00000000 00020002 00000000 08610000 ..
+  6: 07d04000 00000000 00040004 00000000 08610000 ..
+  7: 07d58000 00000000 00000000 00000000 10610000 ..
+  8: 07d58000 00000000 00000000 00000000 08610000 ..
+  9: 07d58000 00000000 00000000 00000000 10610000 ..
+ 10: 07d59000 00000000 00020002 00000000 08610000 ..
+ 11: 07d58000 00000000 00000000 00000000 08610000 ..
+ 12: 07d58000 00000000 00000000 00000000 08610000 ..
+ 13: 07d59000 00000000 00020002 00000000 08610000 ..
+ 14: 07d04000 00000000 00040004 00000000 08610000 ..
+ 15: 07d58000 00000000 00000000 00000000 10610000 ..
+ 16: 07d58000 00000000 00000000 00000000 08610000 ..
+ 17: 07d58000 00000000 00000000 00000000 08610000 ..
+ 18: 07d58000 00000000 00000001 00000000 08600000 ..
+ 19: 07d58000 00000000 00000001 00000000 08600000 ..
+ 20: 07d58000 00000000 00000001 00000000 08600000 ..
+ 21: 07d58000 00000000 00000000 00000000 08610000 ..
+ 22: 07d59000 00000000 00020002 00000000 08610000 ..
+ 23: 07d58000 00000000 00000000 00000000 10610000 ..
+ 24: 07d58000 00000000 00000000 00000000 08610000 ..
+=25: 07d58000 00000000 00000000 00000000 08610000 ..
+ 26: 07d59000 00000000 00020002 00000000 08610000 ..
+ 27: 07d04000 00000000 00040004 00000000 08610000 ..
+ 28: 07d58000 00000000 00000000 00000000 18610000 ..
+ 29: 07d58000 00000000 00000000 00000000 08610000 ..
+ 30: 07d58000 00000000 00000001 00000000 08600000 ..
+ 31: 07d58000 00000000 00000000 00000000 08610000 ..
 
 EDMA regs:
-000:   Config: 0000001f Qlen=32 BSz=128 WBlen=256
-004:    Timer: 3824da8b
+000:   Config: 0000003f NCQ Qlen=32 BSz=128 WBlen=256
+004:    Timer: d9ac2965
 008:  IErrCau: 00000000
 00c:  IErrMsk: fc1e9fff pDAT pPRD DevErr DevDis DevCon SerrInt bit6 SelfDis TrInt pCQRB pCRPB pInt IORdy cRx2 dRx0 dRx1 dRx2 dRx3 dTx0 dTx1 dTx2 dTx3 dTx4 Proto
 010: RqBaseHi: 00000000
-014:  RqInPtr: 07c91320 Index=25 Base=0x7c91
+014:  RqInPtr: 07d56320 Index=25 Base=0x7d56
 018: RqOutPtr: 00000320 Index=25 Base=0x0
 01c: RsBaseHi: 00000000
 020:  RsInPtr: 000000c8 Index=25 Base=0x0
-024: RsOutPtr: 07d460c8 Index=25 Base=0x7d460
+024: RsOutPtr: 07d570c8 Index=25 Base=0x7d570
 028:  Command: 00000001 Active
 02c: TestCtrl: 00000000
-030:   Status: 000000e0 Tag=0 D2M cEmpty Idle State=0x0 IOID=0
+030:   Status: 000000c0 Tag=0 M2D cEmpty Idle State=0x0 IOID=0
 034:  IORdyTO: 000000bc
 038:  Arbiter: 00032190
 040: DelayThr: 00000000
 060: HaltCond: fc1e0e1f
-064: IErrCau2: 00000000
+064: IErrCau2: 00000004
 068: IErrMsk2: 00000000
 06c: Reserved: ff000000
 080:  FsmDctl: 0000ffff
 084: FsmDstat: 00000000
 088: BusyStat: 00000000
 08c:  TcqStat: 00000000
-090: RxFISprs: 00504034
+090: RxFISprs: 015040a1
 094: Ncq0Done: 00000000
 098: Ncq1Done: 00000000
 09c: Ncq2Done: 00000000
 0a0: Ncq3Done: 00000000
-0a8: DsMemory: 10100001
+0a8: DsMemory: 10100000
+
+HOST regs:
+1d60: MainCaus: 00000000
+1d64: MainMask: 00000000
 
 SATA regs:
 300:  SStatus: 00000113 DET=Dev+Phy SPD=0x1 IPM=0x1
@@ -104,26 +108,37 @@
 340:  SErrMsk: 019c0000 W B D S T
 308: SControl: 00000300 DET=Off IPM=0x3
 30c:   LTMode: 010300b0
-310:  PhyMod3: aaaac02a
+310:  PhyMod3: aaaac022
 314:  PhyMod4: 00010005
 32c:  PhyMod1: 40555aa0
 330:  PhyMod2: 3cf5944f
 334:  BistCtl: 00800000
 338:  BistDW1: 00000000
 33c:  BistDW2: 00000000
-050:    IFCfg: 009b7095 RefClk=25Mhz RefClkDiv=2 FeedDiv=1 Gen2iEn bits_12-23=0x9b7
+050:    IFCfg: 009b1095 RefClk=25Mhz RefClkDiv=2 FeedDiv=1 Gen2iEn bits_12-23=0x9b1
 344:    IFCtl: 00000000 PmPortTx=0
 348: IFTstCtl: 00000000
-34c: IFStatus: 00404034 FIStypeRx=0x34 PMportRx=0 MBistRdy DevPres LinkRdy
+34c: IFStatus: 004040a1 FIStypeRx=0xa1 PMportRx=0 MBistRdy DevPres LinkRdy
 35c:  VUnique: 00000000
 360:   FISCfg: 00000000
 364: FISiCaus: 00008000
 368: FISiMask: 00000a00
-370: RxFISDW0: 00504034
-374: RxFISDW1: e6bc2800
-378: RxFISDW2: 00000006
+370: RxFISDW0: 005040a1
+374: RxFISDW1: 00000001
+378: RxFISDW2: 00000000
 37c: RxFISDW3: 00000000
 380: RxFISDW4: 00000000
-384: RxFISDW5: 00000000
+384: RxFISDW5: 00001000
 388: RxFISDW6: 00000000
 
+ATA Shadow regs:
+100:     Data: 16a516a5
+104:    Error: 00000000
+108:    NSect: 08080808
+10c:     LbaL: c8c8c8c8
+110:     LbaM: 3e3e3e3e
+114:     LbaH: 2a2a2a2a
+118:      Dev: 40404040
+11c:     Stat: 50505050
+120:  AltStat: 50505050
+

[regression] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[Frans Pop]

[-- Attachment #1: Type: text/plain, Size: 1122 bytes --]

(Resend with correct address for linux-arm list; with apologies.)

Starting with kernel 2.6.26 I've noticed that the harddisk (HDD) led on my 
QNAP TS-109 (which has a Marvel SATA controller as platform device) no 
longer flashes when there is disk activity. Instead it will just 
stay "on" all the time. The led worked fine with 2.6.25.

I have bisected this to the following series of commits, which are all 
libata or sata_mv changes:
9363c3825ea9ad7..094e50b2f74146d8ee
Further bisection is not possible because between those commits the 
harddisk is not recognized at all.

Lennert Buytenhek suggested to try disabling NCQ, and that does indeed 
make a difference.

After booting the system I did 'echo 1 >/sys/block/sdX/device/queue_depth' 
and after that the HDD led showed disk activity again.

After enabling NCQ again ('echo 31 >/sys/...'), the led again becomes 
unresponsive and just stays on.

Attached a diff between sata_mv hardware registers between 2.6.25 and 
2.6.26 obtained using a patch provided by Mark Lord. (For some reason
"Host" and "ATA Shadow" regs were not available on 2.6.25.)

Cheers,
FJP


[-- Attachment #2: qnap_mv0.regs.diff --]
[-- Type: text/x-diff, Size: 9944 bytes --]

--- qnap_mv0_2.6.25.regs	2008-08-11 20:13:14.000000000 +0200
+++ qnap_mv0.regs	2008-08-11 19:08:18.000000000 +0200
@@ -1,102 +1,106 @@
 CRPBs:
-  0: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=52630
-  1: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=54883
-  2: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=57132
-  3: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=59382
-  4: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=61878
-  5: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=64515
-  6: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=1121
-  7: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=3279
-  8: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=5368
-  9: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=7277
- 10: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=9196
- 11: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=11005
- 12: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=12797
- 13: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=14946
- 14: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=16923
- 15: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=18722
- 16: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=20525
- 17: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=30791
- 18: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=11866
- 19: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=56565
- 20: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=64129
- 21: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=4382
- 22: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=52366
- 23: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=28236
-=24: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=30786
- 25: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=33358
- 26: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=35931
- 27: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=38729
- 28: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=41622
- 29: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=44619
- 30: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=47195
- 31: tag=0  ioid=0  estat=0x00 dstat=0x50 tstmp=49739
+  0: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=54641
+  1: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=55675
+  2: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=62732
+  3: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=3073
+  4: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=8681
+  5: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=9721
+  6: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=10762
+  7: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=17302
+  8: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=23001
+  9: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=52336
+ 10: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=53359
+ 11: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=56477
+ 12: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=37642
+ 13: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=38704
+ 14: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=39764
+ 15: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=46417
+ 16: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=52200
+ 17: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=32877
+ 18: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=62100
+ 19: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=1606
+ 20: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=34537
+ 21: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=35359
+ 22: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=36419
+ 23: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=51224
+ 24: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=58938
+=25: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=10808
+ 26: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=11848
+ 27: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=12877
+ 28: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=22037
+ 29: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=28404
+ 30: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=26248
+ 31: tag=0  ioid=0  estat=0x00 dstat=0x40 tstmp=53609
 
 CRQBs:
-  0: 07d47000 00000000 00000000 00000000 00ca0000 ..
-  1: 07d47000 00000000 00000000 00000000 00ca0000 ..
-  2: 07d47000 00000000 00000000 00000000 00ca0000 ..
-  3: 07d47000 00000000 00000000 00000000 00ca0000 ..
-  4: 07d47000 00000000 00000000 00000000 00ca0000 ..
-  5: 07d47000 00000000 00000000 00000000 00ca0000 ..
-  6: 07d47000 00000000 00000000 00000000 00ca0000 ..
-  7: 07d47000 00000000 00000000 00000000 00ca0000 ..
-  8: 07d47000 00000000 00000000 00000000 00ca0000 ..
-  9: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 10: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 11: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 12: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 13: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 14: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 15: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 16: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 17: 07d47000 00000000 00000001 00000000 00c80000 ..
- 18: 07d47000 00000000 00000001 00000000 00c80000 ..
- 19: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 20: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 21: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 22: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 23: 07d47000 00000000 00000001 00000000 00c80000 ..
- 24: 07d47000 00000000 00000001 00000000 00c80000 ..
-=25: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 26: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 27: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 28: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 29: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 30: 07d47000 00000000 00000000 00000000 00ca0000 ..
- 31: 07d47000 00000000 00000000 00000000 00ca0000 ..
+  0: 07d59000 00000000 00020002 00000000 08610000 ..
+  1: 07d04000 00000000 00040004 00000000 08610000 ..
+  2: 07d58000 00000000 00000000 00000000 18610000 ..
+  3: 07d58000 00000000 00000000 00000000 08610000 ..
+  4: 07d58000 00000000 00000000 00000000 08610000 ..
+  5: 07d59000 00000000 00020002 00000000 08610000 ..
+  6: 07d04000 00000000 00040004 00000000 08610000 ..
+  7: 07d58000 00000000 00000000 00000000 10610000 ..
+  8: 07d58000 00000000 00000000 00000000 08610000 ..
+  9: 07d58000 00000000 00000000 00000000 10610000 ..
+ 10: 07d59000 00000000 00020002 00000000 08610000 ..
+ 11: 07d58000 00000000 00000000 00000000 08610000 ..
+ 12: 07d58000 00000000 00000000 00000000 08610000 ..
+ 13: 07d59000 00000000 00020002 00000000 08610000 ..
+ 14: 07d04000 00000000 00040004 00000000 08610000 ..
+ 15: 07d58000 00000000 00000000 00000000 10610000 ..
+ 16: 07d58000 00000000 00000000 00000000 08610000 ..
+ 17: 07d58000 00000000 00000000 00000000 08610000 ..
+ 18: 07d58000 00000000 00000001 00000000 08600000 ..
+ 19: 07d58000 00000000 00000001 00000000 08600000 ..
+ 20: 07d58000 00000000 00000001 00000000 08600000 ..
+ 21: 07d58000 00000000 00000000 00000000 08610000 ..
+ 22: 07d59000 00000000 00020002 00000000 08610000 ..
+ 23: 07d58000 00000000 00000000 00000000 10610000 ..
+ 24: 07d58000 00000000 00000000 00000000 08610000 ..
+=25: 07d58000 00000000 00000000 00000000 08610000 ..
+ 26: 07d59000 00000000 00020002 00000000 08610000 ..
+ 27: 07d04000 00000000 00040004 00000000 08610000 ..
+ 28: 07d58000 00000000 00000000 00000000 18610000 ..
+ 29: 07d58000 00000000 00000000 00000000 08610000 ..
+ 30: 07d58000 00000000 00000001 00000000 08600000 ..
+ 31: 07d58000 00000000 00000000 00000000 08610000 ..
 
 EDMA regs:
-000:   Config: 0000001f Qlen=32 BSz=128 WBlen=256
-004:    Timer: 3824da8b
+000:   Config: 0000003f NCQ Qlen=32 BSz=128 WBlen=256
+004:    Timer: d9ac2965
 008:  IErrCau: 00000000
 00c:  IErrMsk: fc1e9fff pDAT pPRD DevErr DevDis DevCon SerrInt bit6 SelfDis TrInt pCQRB pCRPB pInt IORdy cRx2 dRx0 dRx1 dRx2 dRx3 dTx0 dTx1 dTx2 dTx3 dTx4 Proto
 010: RqBaseHi: 00000000
-014:  RqInPtr: 07c91320 Index=25 Base=0x7c91
+014:  RqInPtr: 07d56320 Index=25 Base=0x7d56
 018: RqOutPtr: 00000320 Index=25 Base=0x0
 01c: RsBaseHi: 00000000
 020:  RsInPtr: 000000c8 Index=25 Base=0x0
-024: RsOutPtr: 07d460c8 Index=25 Base=0x7d460
+024: RsOutPtr: 07d570c8 Index=25 Base=0x7d570
 028:  Command: 00000001 Active
 02c: TestCtrl: 00000000
-030:   Status: 000000e0 Tag=0 D2M cEmpty Idle State=0x0 IOID=0
+030:   Status: 000000c0 Tag=0 M2D cEmpty Idle State=0x0 IOID=0
 034:  IORdyTO: 000000bc
 038:  Arbiter: 00032190
 040: DelayThr: 00000000
 060: HaltCond: fc1e0e1f
-064: IErrCau2: 00000000
+064: IErrCau2: 00000004
 068: IErrMsk2: 00000000
 06c: Reserved: ff000000
 080:  FsmDctl: 0000ffff
 084: FsmDstat: 00000000
 088: BusyStat: 00000000
 08c:  TcqStat: 00000000
-090: RxFISprs: 00504034
+090: RxFISprs: 015040a1
 094: Ncq0Done: 00000000
 098: Ncq1Done: 00000000
 09c: Ncq2Done: 00000000
 0a0: Ncq3Done: 00000000
-0a8: DsMemory: 10100001
+0a8: DsMemory: 10100000
+
+HOST regs:
+1d60: MainCaus: 00000000
+1d64: MainMask: 00000000
 
 SATA regs:
 300:  SStatus: 00000113 DET=Dev+Phy SPD=0x1 IPM=0x1
@@ -104,26 +108,37 @@
 340:  SErrMsk: 019c0000 W B D S T
 308: SControl: 00000300 DET=Off IPM=0x3
 30c:   LTMode: 010300b0
-310:  PhyMod3: aaaac02a
+310:  PhyMod3: aaaac022
 314:  PhyMod4: 00010005
 32c:  PhyMod1: 40555aa0
 330:  PhyMod2: 3cf5944f
 334:  BistCtl: 00800000
 338:  BistDW1: 00000000
 33c:  BistDW2: 00000000
-050:    IFCfg: 009b7095 RefClk=25Mhz RefClkDiv=2 FeedDiv=1 Gen2iEn bits_12-23=0x9b7
+050:    IFCfg: 009b1095 RefClk=25Mhz RefClkDiv=2 FeedDiv=1 Gen2iEn bits_12-23=0x9b1
 344:    IFCtl: 00000000 PmPortTx=0
 348: IFTstCtl: 00000000
-34c: IFStatus: 00404034 FIStypeRx=0x34 PMportRx=0 MBistRdy DevPres LinkRdy
+34c: IFStatus: 004040a1 FIStypeRx=0xa1 PMportRx=0 MBistRdy DevPres LinkRdy
 35c:  VUnique: 00000000
 360:   FISCfg: 00000000
 364: FISiCaus: 00008000
 368: FISiMask: 00000a00
-370: RxFISDW0: 00504034
-374: RxFISDW1: e6bc2800
-378: RxFISDW2: 00000006
+370: RxFISDW0: 005040a1
+374: RxFISDW1: 00000001
+378: RxFISDW2: 00000000
 37c: RxFISDW3: 00000000
 380: RxFISDW4: 00000000
-384: RxFISDW5: 00000000
+384: RxFISDW5: 00001000
 388: RxFISDW6: 00000000
 
+ATA Shadow regs:
+100:     Data: 16a516a5
+104:    Error: 00000000
+108:    NSect: 08080808
+10c:     LbaL: c8c8c8c8
+110:     LbaM: 3e3e3e3e
+114:     LbaH: 2a2a2a2a
+118:      Dev: 40404040
+11c:     Stat: 50505050
+120:  AltStat: 50505050
+

Re: [regression] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[Frans Pop]

On Tuesday 26 August 2008, Frans Pop wrote:
> After booting the system I did 'echo 1 >/sys/block/sdX/device/queue_depth'
> and after that the HDD led showed disk activity again.

And to confirm that, applying the following patch to sata_mv for 2.6.26.3
makes the HDD led work again as well:

--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -131,7 +131,7 @@ enum {

        MV_GENIIE_FLAGS         = MV_COMMON_FLAGS | MV_6XXX_FLAGS |
                                  ATA_FLAG_PMP | ATA_FLAG_ACPI_SATA |
-                                 ATA_FLAG_NCQ | ATA_FLAG_AN,
+                                 ATA_FLAG_AN,

        CRQB_FLAG_READ          = (1 << 0),
        CRQB_TAG_SHIFT          = 1,

So somehow enabling NCQ is at the root of the issue.

Cheers,
FJP

Re: [regression] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[Mark Lord]

Frans Pop wrote:
> (Resend with correct address for linux-arm list; with apologies.)
> 
> Starting with kernel 2.6.26 I've noticed that the harddisk (HDD) led on my 
> QNAP TS-109 (which has a Marvel SATA controller as platform device) no 
> longer flashes when there is disk activity. Instead it will just 
> stay "on" all the time. The led worked fine with 2.6.25.
..
> Lennert Buytenhek suggested to try disabling NCQ, and that does indeed 
> make a difference.
..

Well, if you can get us the schematics for that QNAP TS-109 board,
then perhaps we can figure out what controls that LED, and then
determine whether or not it is possible to have it working with NCQ.

Or maybe you can get Saeed Bishara (of Marvell) to investigate.
There may be a standard LED control bit in the chipset somewhere
that is known to internal Marvell engineers.

Cheers

Re: [regression] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[Martin Michlmayr]

* Mark Lord <liml@rtr.ca> [2008-08-26 10:03]:
> Well, if you can get us the schematics for that QNAP TS-109 board,
> then perhaps we can figure out what controls that LED, and then
> determine whether or not it is possible to have it working with NCQ.
>
> Or maybe you can get Saeed Bishara (of Marvell) to investigate.
> There may be a standard LED control bit in the chipset somewhere
> that is known to internal Marvell engineers.

I'm being told that there's a standard LED control bit on Orion, but
hopefully Saeed can comment.  If it's a QNAP specific problem, I can
talk to my contacts at QNAP.
-- 
Martin Michlmayr
http://www.cyrius.com/

Re: [regression] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[saeed bishara]

>
> I'm being told that there's a standard LED control bit on Orion, but
> hopefully Saeed can comment.  If it's a QNAP specific problem, I can
> talk to my contacts at QNAP.

bit 0 of 0x8002c controls the active LED, 1 makes it blink slower
(more user friendly).
I checked 2.6.26 on rd5182 and both LEDs work fine. it's probably QNAP
specific problem.
Frans, can you please try the patch at
http://www.spinics.net/lists/linux-ide/msg25093.html
if it still doesn't work please check the value of the register at
0xf1010004 (MPPs control).


saeed

Re: [regression] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[saeed bishara]

On Tue, Aug 26, 2008 at 11:45 AM, Frans Pop <elendil@planet.nl> wrote:
> Starting with kernel 2.6.26 I've noticed that the harddisk (HDD) led on my
> QNAP TS-109 (which has a Marvel SATA controller as platform device) no
> longer flashes when there is disk activity. Instead it will just
> stay "on" all the time. The led worked fine with 2.6.25.
>
Frans, does the LED stay "on" even when there is no disk activity?

Re: [regression] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[Frans Pop]

On Wednesday 27 August 2008, saeed bishara wrote:
> Frans, does the LED stay "on" even when there is no disk activity?

Yes, it does.

On Wednesday 27 August 2008, saeed bishara wrote:
> > I'm being told that there's a standard LED control bit on Orion, but
> > hopefully Saeed can comment.  If it's a QNAP specific problem, I can
> > talk to my contacts at QNAP.
>
> bit 0 of 0x8002c controls the active LED, 1 makes it blink slower
> (more user friendly).
> I checked 2.6.26 on rd5182 and both LEDs work fine. it's probably QNAP
> specific problem.
> Frans, can you please try the patch at
> http://www.spinics.net/lists/linux-ide/msg25093.html

That patch does not seem to make any difference.

> if it still doesn't work please check the value of the register at
> 0xf1010004 (MPPs control).

$ sudo ./bin/devmem2 0xf1010004
/dev/mem opened.
Memory mapped at address 0x4001f000.
Value at address 0xF1010004 (0x4001f004): 0x55550000

Thanks,
FJP

Re: [regression] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[saeed bishara]

On Thu, Aug 28, 2008 at 12:25 PM, Frans Pop <elendil@planet.nl> wrote:
> On Wednesday 27 August 2008, saeed bishara wrote:
>> Frans, does the LED stay "on" even when there is no disk activity?
>
> Yes, it does.
>
> On Wednesday 27 August 2008, saeed bishara wrote:
>> > I'm being told that there's a standard LED control bit on Orion, but
>> > hopefully Saeed can comment.  If it's a QNAP specific problem, I can
>> > talk to my contacts at QNAP.
>>
>> bit 0 of 0x8002c controls the active LED, 1 makes it blink slower
>> (more user friendly).
>> I checked 2.6.26 on rd5182 and both LEDs work fine. it's probably QNAP
>> specific problem.
>> Frans, can you please try the patch at
>> http://www.spinics.net/lists/linux-ide/msg25093.html
>
> That patch does not seem to make any difference.
>
>> if it still doesn't work please check the value of the register at
>> 0xf1010004 (MPPs control).
>
> $ sudo ./bin/devmem2 0xf1010004
> /dev/mem opened.
> Memory mapped at address 0x4001f000.
> Value at address 0xF1010004 (0x4001f004): 0x55550000
thanks for the check, the register seems all right.
can you try setting bit 0 of the 0xf108002c then run some io?
which disk you have there? is there any chance that you can replace it
with different model that support NCQ?
saeed

Re: [regression] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[Frans Pop]

On Thursday 28 August 2008, saeed bishara wrote:
> can you try setting bit 0 of the 0xf108002c then run some io?

How can I set that bit?

> which disk you have there?

ata1.00: ATA-7: HDT722516DLA380, V43OA91A, max UDMA/133
ata1.00: 321672960 sectors, multi 0: LBA48 NCQ (depth 31/32)
ata1.00: configured for UDMA/133
ata2: SATA link down (SStatus 0 SControl 300)
scsi 0:0:0:0: Direct-Access     ATA      HDT722516DLA380  V43O PQ: 0 ANSI: 5
sd 0:0:0:0: [sda] 321672960 512-byte hardware sectors (164697 MB)
sd 0:0:0:0: [sda] Write Protect is off
sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00
sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
sd 0:0:0:0: [sda] 321672960 512-byte hardware sectors (164697 MB)
sd 0:0:0:0: [sda] Write Protect is off
sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00
sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
 sda: sda1 sda2 sda3 sda4 < sda5 sda6 >
sd 0:0:0:0: [sda] Attached SCSI disk
sd 0:0:0:0: Attached scsi generic sg0 type 0

>From smartctl:
Model Family:     Hitachi Deskstar T7K250 series
Device Model:     HDT722516DLA380
Firmware Version: V43OA91A
User Capacity:    164,696,555,520 bytes
ATA Version is:   7
ATA Standard is:  ATA/ATAPI-7 T13 1532D revision 1

> is there any chance that you can replace it with different model that
> support NCQ? 

No, sorry. This is the only model SATA disk I have available. However, I use the
exact same model in a x86_64 desktop without any problems.

Possibly another owner of a TS-109 or maybe TS-209 can confirm the issue with a
different model disk.

Re: [regression] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[Grant Grundler]

On Thu, Aug 28, 2008 at 4:17 AM, Frans Pop <elendil@planet.nl> wrote:
> On Thursday 28 August 2008, saeed bishara wrote:
>> can you try setting bit 0 of the 0xf108002c then run some io?
>
> How can I set that bit?

Try mmio_rd/mmio_wr at  http://svn.gnumonks.org/trunk/mmio_test/

grant

>
>> which disk you have there?
>
> ata1.00: ATA-7: HDT722516DLA380, V43OA91A, max UDMA/133
> ata1.00: 321672960 sectors, multi 0: LBA48 NCQ (depth 31/32)
> ata1.00: configured for UDMA/133
> ata2: SATA link down (SStatus 0 SControl 300)
> scsi 0:0:0:0: Direct-Access     ATA      HDT722516DLA380  V43O PQ: 0 ANSI: 5
> sd 0:0:0:0: [sda] 321672960 512-byte hardware sectors (164697 MB)
> sd 0:0:0:0: [sda] Write Protect is off
> sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00
> sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
> sd 0:0:0:0: [sda] 321672960 512-byte hardware sectors (164697 MB)
> sd 0:0:0:0: [sda] Write Protect is off
> sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00
> sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
>  sda: sda1 sda2 sda3 sda4 < sda5 sda6 >
> sd 0:0:0:0: [sda] Attached SCSI disk
> sd 0:0:0:0: Attached scsi generic sg0 type 0
>
> From smartctl:
> Model Family:     Hitachi Deskstar T7K250 series
> Device Model:     HDT722516DLA380
> Firmware Version: V43OA91A
> User Capacity:    164,696,555,520 bytes
> ATA Version is:   7
> ATA Standard is:  ATA/ATAPI-7 T13 1532D revision 1
>
>> is there any chance that you can replace it with different model that
>> support NCQ?
>
> No, sorry. This is the only model SATA disk I have available. However, I use the
> exact same model in a x86_64 desktop without any problems.
>
> Possibly another owner of a TS-109 or maybe TS-209 can confirm the issue with a
> different model disk.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [regression] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[Frans Pop]

On Thursday 28 August 2008, saeed bishara wrote:
> can you try setting bit 0 of the 0xf108002c then run some io?

Tried that using Grant's suggestion (thanks!), but setting the bit does 
not seem to take:

$ sudo mmio_rd f108002c 1
0x0000000000000000
$ sudo mmio_wr f108002c 1
0000000000000000 -> 0000000000000001
$ sudo mmio_rd f108002c 1
0x0000000000000000

Am I doing something wrong here?

Re: [regression] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[Grant Grundler]

[-- Attachment #1: Type: text/plain, Size: 721 bytes --]

On Thu, Aug 28, 2008 at 10:11 AM, Frans Pop <elendil@planet.nl> wrote:
> On Thursday 28 August 2008, saeed bishara wrote:
>> can you try setting bit 0 of the 0xf108002c then run some io?
>
> Tried that using Grant's suggestion (thanks!), but setting the bit does
> not seem to take:
>
> $ sudo mmio_rd f108002c 1
> 0x0000000000000000
> $ sudo mmio_wr f108002c 1
> 0000000000000000 -> 0000000000000001
> $ sudo mmio_rd f108002c 1
> 0x0000000000000000
>
> Am I doing something wrong here?

Dunno. Is this a 64-bit kjernel?

I still have a patch outstanding against mmio_test fix to issues with
64-bit addressing from 32-bit user space. I've attached those (and
cc'd Harald Welte) in case that's the problem.

thanks,
grant

[-- Attachment #2: diff-mmio_test-02 --]
[-- Type: application/octet-stream, Size: 4658 bytes --]

Index: mmio_test.xml
===================================================================
--- mmio_test.xml	(revision 2099)
+++ mmio_test.xml	(working copy)
@@ -203,6 +203,20 @@
 	<register name="ISRC2 (Yukon2 only)" resource="0" offset="0x1c"/>
 </device>
 
+<device name="sata_mv">
+	<!-- Marvell SATA II ("Thor") -->
+	<pci_id vendor_id="0x11AB" device_id="0x6042"/>
+	<pci_id vendor_id="0x11AB" device_id="0x7042"/>
+
+	<!-- older Marvell controllers -->
+	<!-- pci_id vendor_id="0x11AB" device_id="0x6041"/ -->
+	<!-- pci_id vendor_id="0x11AB" device_id="0x6081"/ -->
+
+	<register name="main_cause" resource="0" offset="0x1d60"/>
+	<register name="main_mask" resource="0" offset="0x1d64"/>
+	<register name="SATAHC0Cause" resource="0" offset="0x20014" />
+</device>
+
 <device name="sym2">
 	<!-- Symbios SCSI -->
 	<pci_id vendor_id="0x1000" device_id="0x0001"/>
Index: mmio_test.c
===================================================================
--- mmio_test.c	(revision 2099)
+++ mmio_test.c	(working copy)
@@ -127,23 +127,29 @@
 		cycles_t avg;
 		unsigned long long regs = 
 			p->base_addr[reg->resource] & PCI_ADDR_MEM_MASK;
+		unsigned int regoff = reg->offset;
 
-		//printf("Using regs = 0x%llx (%d)\n", regs, reg->mmap_size);
-		x = mmap(NULL, reg->mmap_size, PROT_READ, MAP_SHARED, 
-			 dev_mem_fd, regs);
+		/* We do not map the entire register space for a device.
+		 * Do some basic math to reduce that to one 4k page. 
+		 */
+		/* printf("Using regs = 0x%llx + %x", regs, reg->offset); */
+		regs += regoff & ~0xfff;
+		regoff &= 0xfff;
+		/* printf(" --> 0x%llx + %x\n", regs, regoff); */
+		x = mmap(NULL, 0x1000, PROT_READ, MAP_SHARED, dev_mem_fd, regs);
 		if (x == MAP_FAILED) {
 			perror("mmap");
 			return -1;
 		}
 
-		avg = test_backend(x, reg->offset);
+		avg = test_backend(x, regoff);
 		if (output_xml) {
 			printf("<testresult dev_name=\"%s\" "
 				"pci_vendor_id=\"0x%x\" "
 				"pci_device_id=\"0x%x\" "
 				"pci_addr=\"%d:%02d:%02d.%d\" "
 				"cpu=\"%d\" resource=\"%d\" "
-				"reg_offset=\"%d\" cycles=\"%d\"", 
+				"reg_offset=\"%d\" cycles=\"%lld\"", 
 				n->name, p->vendor_id, p->device_id, 
 				p->domain, p->bus, p->dev, p->func, 
 				cpu, reg->resource, reg->offset, avg);
Index: mmio_rd.c
===================================================================
--- mmio_rd.c	(revision 2099)
+++ mmio_rd.c	(working copy)
@@ -13,8 +13,10 @@
 int main(int argc, char **argv)
 {
 	int fd = open("/dev/mem", O_RDONLY | O_SYNC);
-	unsigned long phys_addr, len;	/* arg1, arg2 */
-	unsigned long i, page_addr, page_size = getpagesize();
+	unsigned long long phys_addr;	/* arg1 */
+	unsigned long long page_addr;
+	unsigned long len;		/* arg2 */
+	unsigned long i, page_size = getpagesize();
 	volatile unsigned long *mem;
 
 	if (fd < 0)
@@ -23,7 +25,7 @@
 	if (argc < 3)
 		exit(-2);
 
-	phys_addr = strtoul(argv[1], NULL, 16);
+	phys_addr = strtoull(argv[1], NULL, 0);
 	phys_addr &= ~(sizeof(mem[0]) - 1);
 
 	len = strtoul(argv[2], NULL, 16);
@@ -43,7 +45,7 @@
 
 	for (; len && (i < (page_size / sizeof(mem[0]))); i++, len--) {
 /*  printf("%016lx: ", page_addr + i*sizeof(mem[0])); */
-		printf("0x%016lx\n", mem[i]);
+		printf("%016lx\n", mem[i]);
 	}
 
 	return 0;
Index: mmio_wr.c
===================================================================
--- mmio_wr.c	(revision 2099)
+++ mmio_wr.c	(working copy)
@@ -26,7 +26,8 @@
 int main(int argc, char **argv)
 {
 	int fd = open("/dev/mem", O_RDWR | O_SYNC);
-	unsigned long i, addr, val, offset, old, page_size = getpagesize();
+	unsigned long long i, addr, val, offset, old;
+	unsigned long page_size = getpagesize();
 	volatile unsigned long *mem;
 
 	if (fd < 0)
@@ -35,8 +36,8 @@
 	if (argc < 3)
 		exit(-2);
 
-	addr = strtol(argv[1], NULL, 16);
-	val = strtol(argv[2], NULL, 16);
+	addr = strtoull(argv[1], NULL, 16);
+	val = strtoull(argv[2], NULL, 16);
 
 	offset = addr & ~(page_size - 1);
 	i = (addr - offset) / sizeof(mem[0]);
@@ -48,7 +49,7 @@
 	}
 
 	old = mem[i];
-	printf("%016lx -> %016lx\n", old, val);
+	/* printf("%016llx -> %016llx\n", old, val); */
 	mem[i] = val;
 
 	return 0;
Index: Makefile
===================================================================
--- Makefile	(revision 2099)
+++ Makefile	(working copy)
@@ -2,18 +2,13 @@
 CFLAGS += -I/usr/include/libxml2
 # on SPARC64, plese add the below line
 #CFLAGS += -Wa,-Av9a
+LDFLAGS += -static
 
-LIBS:=-lpci -lxml2
+LIBS:=-lpci -lxml2 -lz -lm -lpthread
 #LIBS+=-lrt
 
 all: mmio_rd mmio_wr mmio_test
 
-mmio_rd: mmio_rd.o
-	$(CC) -o $@  $^
-
-mmio_wr: mmio_wr.o
-	$(CC) -o $@  $^
-
 mmio_test: mmio_test.o get_clock.o xmlin.o
 	$(CC) $(LDFLAGS) -o $@ $^ $(LIBS)
 

Re: [regression] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[Frans Pop]

On Thursday 28 August 2008, Grant Grundler wrote:
> On Thu, Aug 28, 2008 at 10:11 AM, Frans Pop <elendil@planet.nl> wrote:
> > On Thursday 28 August 2008, saeed bishara wrote:
> >> can you try setting bit 0 of the 0xf108002c then run some io?
> >
> > Tried that using Grant's suggestion (thanks!), but setting the bit
> > does not seem to take:
> >
> > $ sudo mmio_rd f108002c 1
> > 0x0000000000000000
> > $ sudo mmio_wr f108002c 1
> > 0000000000000000 -> 0000000000000001
> > $ sudo mmio_rd f108002c 1
> > 0x0000000000000000
> >
> > Am I doing something wrong here?
>
> Dunno. Is this a 64-bit kjernel?

No, it's a 32-bit armel kernel.

Re: [regression] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[Lennert Buytenhek]

On Thu, Aug 28, 2008 at 07:11:27PM +0200, Frans Pop wrote:

> > can you try setting bit 0 of the 0xf108002c then run some io?
> 
> Tried that using Grant's suggestion (thanks!), but setting the bit does 
> not seem to take:
> 
> $ sudo mmio_rd f108002c 1
> 0x0000000000000000
> $ sudo mmio_wr f108002c 1
> 0000000000000000 -> 0000000000000001
> $ sudo mmio_rd f108002c 1
> 0x0000000000000000
> 
> Am I doing something wrong here?

Can you try devmem2?  (That should be in Debian, I think.)

Re: [regression] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[Frans Pop]

On Thursday 28 August 2008, Lennert Buytenhek wrote:
> On Thu, Aug 28, 2008 at 07:11:27PM +0200, Frans Pop wrote:
> > > can you try setting bit 0 of the 0xf108002c then run some io?
>
> Can you try devmem2?

OK, that works better:

$ sudo devmem2 0xf108002c w 1
/dev/mem opened.
Memory mapped at address 0x4001f000.
Value at address 0xF108002C (0x4001f02c): 0x0
Written 0x1; readback 0x1

After that the HDD led does react again to io activity.

Re: [regression] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[saeed bishara]

> $ sudo devmem2 0xf108002c w 1
> /dev/mem opened.
> Memory mapped at address 0x4001f000.
> Value at address 0xF108002C (0x4001f02c): 0x0
> Written 0x1; readback 0x1
>
> After that the HDD led does react again to io activity.
do you mean it's blinking on io activity? is it off when no io running?

saeed

Re: [regression] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[Frans Pop]

On Friday 29 August 2008, saeed bishara wrote:
> > $ sudo devmem2 0xf108002c w 1
> > /dev/mem opened.
> > Memory mapped at address 0x4001f000.
> > Value at address 0xF108002C (0x4001f02c): 0x0
> > Written 0x1; readback 0x1
> >
> > After that the HDD led does react again to io activity.
>
> do you mean it's blinking on io activity?

Yes.

> is it off when no io running? 

No. It is on when there is no activity, but that is how it has always 
been. Same as the LAN led which is also always on when there is a 
connection and blinks on activity.

I don't know if that's how that's intended, but IMHO it makes sense for an 
embedded device to have the led on by default to show the device is
"present and recognized" and to blink when there is activity.

Cheers,
FJP

Re: [regression] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 630 bytes --]

On Thu, Aug 28, 2008 at 10:26:32AM -0700, Grant Grundler wrote:
> I still have a patch outstanding against mmio_test fix to issues with
> 64-bit addressing from 32-bit user space. I've attached those (and
> cc'd Harald Welte) in case that's the problem.

Thanks, I'll merge your patches to mmio_test right now.

-- 
- Harald Welte <laforge@gnumonks.org>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [regression] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[Mark Lord]

Frans Pop wrote:
> On Tuesday 26 August 2008, Frans Pop wrote:
>> After booting the system I did 'echo 1 >/sys/block/sdX/device/queue_depth'
>> and after that the HDD led showed disk activity again.
> 
> And to confirm that, applying the following patch to sata_mv for 2.6.26.3
> makes the HDD led work again as well:
> 
> --- a/drivers/ata/sata_mv.c
> +++ b/drivers/ata/sata_mv.c
..
>                                   ATA_FLAG_PMP | ATA_FLAG_ACPI_SATA |
> -                                 ATA_FLAG_NCQ | ATA_FLAG_AN,
> +                                 ATA_FLAG_AN,
..
> So somehow enabling NCQ is at the root of the issue.
..

Pardon me digging up this old thread again,
but I'm back working on sata_mv now.

There is a reported chipset errata for *exactly* this symptom,
though the errata is supposedly for the older 6041/6081 variants.
But it is possible that the same problem still exists in the
chip you are using there (SOC, right?).

The problem is exactly as you described it:  turning on NCQ
causes the activity LEDs to remain on constantly.
No known workaround or fix is available from Marvell,
so "just live with it" is the only solution.  :)

Cheers

[RFC][PATCH] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[Frans Pop]

On Saturday 24 January 2009, Mark Lord wrote:
> Frans Pop wrote:
> > So somehow enabling NCQ is at the root of the issue.
>
> Pardon me digging up this old thread again,
> but I'm back working on sata_mv now.

No problem. I've only recently picked up on this issue again myself.

> There is a reported chipset errata for *exactly* this symptom,
> though the errata is supposedly for the older 6041/6081 variants.
> But it is possible that the same problem still exists in the
> chip you are using there (SOC, right?).

Correct.

> The problem is exactly as you described it:  turning on NCQ
> causes the activity LEDs to remain on constantly.
> No known workaround or fix is available from Marvell,
> so "just live with it" is the only solution.  :)

Well, the attached patch works for me on my QNAP TS-109. It's based on an
earlier patch proposed by Saeed Bishara to unconditionally enable blink
mode for the led for SOC.

The attached patch adds a quirk for SOC that enables the blink mode as
needed when there's at least one port on a host controller for which NCQ
is enabled. 

In the blink mode the led is not as responsive as when NCQ is disabled,
but at least it does show when there is I/O. If I disable NCQ by setting
queue_depth to 1, blink mode is disabled and the led behaves as with
pre-2.6.26 kernels.

I've done my best to make the implementation as cheap as possible.

One question I have is whether _all_ SOCs need this quirk or if we should
test for a model or revision. Is there model/revision info available for SOC?

Cheers,
FJP

---
Subject: sata-mv: enable HDD led blinking when NCQ is active for SOC
    
For some Marvell chips the HDD led does not blink when there is
disk I/O if NCQ is enabled. Add a quirk that enables blink mode for
the led when NCQ is used on any of the ports of a host controller.
    
Currently the quirk is only enabled for SOC, but it can be extended
to other chip models.
    
The code to enable the blink mode is based on an earlier patch
proposed by Saeed Bishara.
    
Signed-off-by: Frans Pop <elendil@planet.nl>
Cc: Mark Lord <liml@rtr.ca>
Cc: Saeed Bishara <saeed.bishara@gmail.com>

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 4ae1a41..c5072e6 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -205,6 +205,11 @@ enum {
 	HC_COAL_IRQ		= (1 << 4),	/* IRQ coalescing */
 	DEV_IRQ			= (1 << 8),	/* shift by port # */
 
+	SOC_LED_CTRL_OFS	= 0x2c,
+	SOC_LED_CTRL_BLINK	= (1 << 0),	/* Active LED blink */
+	SOC_LED_CTRL_ACT_PRESENCE = (1 << 2),	/* Multiplex presence with */
+						/* the active LED */
+
 	/* Shadow block registers */
 	SHD_BLK_OFS		= 0x100,
 	SHD_CTL_AST_OFS		= 0x20,		/* ofs from SHD_BLK_OFS */
@@ -359,6 +364,7 @@ enum {
 	MV_HP_PCIE		= (1 << 9),	/* PCIe bus/regs: 7042 */
 	MV_HP_CUT_THROUGH	= (1 << 10),	/* can use EDMA cut-through */
 	MV_HP_FLAG_SOC		= (1 << 11),	/* SystemOnChip, no PCI */
+	MV_HP_QUIRK_LED_BL_EN	= (1 << 12),	/* is led blinking enabled? */
 
 	/* Port private flags (pp_flags) */
 	MV_PP_FLAG_EDMA_EN	= (1 << 0),	/* is EDMA engine enabled? */
@@ -523,6 +529,8 @@ static int mv6_reset_hc(struct mv_host_priv *hpriv, void __iomem *mmio,
 static void mv6_reset_flash(struct mv_host_priv *hpriv, void __iomem *mmio);
 static void mv_soc_enable_leds(struct mv_host_priv *hpriv,
 				      void __iomem *mmio);
+static void mv_soc_led_blink_enable(struct mv_host_priv *hpriv,
+				      void __iomem *mmio, int blink_enable);
 static void mv_soc_read_preamp(struct mv_host_priv *hpriv, int idx,
 				      void __iomem *mmio);
 static int mv_soc_reset_hc(struct mv_host_priv *hpriv,
@@ -852,6 +860,52 @@ static void mv_enable_port_irqs(struct ata_port *ap,
 	mv_set_main_irq_mask(ap->host, disable_bits, enable_bits);
 }
 
+static int mv_has_port_using_ncq(struct ata_host *host)
+{
+	int port;
+	struct mv_host_priv *hpriv = host->private_data;
+
+	for (port = 0; port < hpriv->n_ports; port++) {
+		struct ata_port *ap = host->ports[port];
+		struct mv_port_priv *pp = ap->private_data;
+
+		if ((pp->pp_flags & MV_PP_FLAG_EDMA_EN) &&
+		    (pp->pp_flags & MV_PP_FLAG_NCQ_EN))
+			return 1;
+	}
+
+	return 0;
+}
+
+/**
+ * Some chips have an erratum which causes the HDD led not to blink
+ * during I/O when NCQ is enabled. Enabling the blink mode of the
+ * led makes activity visible in that case.
+ */
+static void mv_quirk_blink_led_when_ncq(struct ata_port *ap,
+					int enable)
+{
+	struct mv_host_priv *hpriv = ap->host->private_data;
+	void __iomem *mmio = hpriv->base;
+
+	if (enable) {
+		if (!(hpriv->hp_flags & MV_HP_QUIRK_LED_BL_EN)) {
+			mv_soc_led_blink_enable(hpriv, mmio, true);
+			hpriv->hp_flags |= MV_HP_QUIRK_LED_BL_EN;
+		}
+		return;
+	} else {
+		if (!(hpriv->hp_flags & MV_HP_QUIRK_LED_BL_EN))
+			return;
+	}
+
+	/* Does any other port still use NCQ? */
+	if (!mv_has_port_using_ncq(ap->host)) {
+		mv_soc_led_blink_enable(hpriv, mmio, false);
+		hpriv->hp_flags &= ~MV_HP_QUIRK_LED_BL_EN;
+	}
+}
+
 /**
  *      mv_start_dma - Enable eDMA engine
  *      @base: port base address
@@ -961,6 +1015,9 @@ static int mv_stop_edma(struct ata_port *ap)
 		ata_port_printk(ap, KERN_ERR, "Unable to stop eDMA\n");
 		return -EIO;
 	}
+
+	mv_quirk_blink_led_when_ncq(ap, false);
+
 	return 0;
 }
 
@@ -1219,6 +1276,9 @@ static void mv_edma_cfg(struct ata_port *ap, int want_ncq)
 			cfg |= (1 << 18);	/* enab early completion */
 		if (hpriv->hp_flags & MV_HP_CUT_THROUGH)
 			cfg |= (1 << 17); /* enab cut-thru (dis stor&forwrd) */
+
+		if (IS_SOC(hpriv) && want_ncq)
+			mv_quirk_blink_led_when_ncq(ap, true);
 	}
 
 	if (want_ncq) {
@@ -2613,6 +2673,19 @@ static void mv_soc_enable_leds(struct mv_host_priv *hpriv,
 	return;
 }
 
+static void mv_soc_led_blink_enable(struct mv_host_priv *hpriv,
+				      void __iomem *mmio, int blink_enable)
+{
+	void __iomem *hc_mmio = mv_hc_base(mmio, 0);
+	u32 tmp = readl(hc_mmio + SOC_LED_CTRL_OFS);
+
+	/* enable/disable blinking mode */
+	if (blink_enable)
+		writel(tmp | SOC_LED_CTRL_BLINK, hc_mmio + SOC_LED_CTRL_OFS);
+	else
+		writel(tmp & ~SOC_LED_CTRL_BLINK, hc_mmio + SOC_LED_CTRL_OFS);
+}
+
 static void mv_soc_read_preamp(struct mv_host_priv *hpriv, int idx,
 			   void __iomem *mmio)
 {

Re: [RFC][PATCH] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[saeed bishara]

>> There is a reported chipset errata for *exactly* this symptom,
>> though the errata is supposedly for the older 6041/6081 variants.
>> But it is possible that the same problem still exists in the
>> chip you are using there (SOC, right?).
That erratum was fixed in the production revisions, beside, that the
blink mode working, but it's responsivness is not good enough as when
NCQ disabled.
   What I think is that this issue is seen with the specific disk that
you have, the chip blinks the LED depending on the BUSY bit status,
and I suspect that with the disk you have does clear that bit
immediately after receiving the command FIS, so the LED will be off,
though the command is not completed yet, and this makes the led
responsiveness differ that in NCQ-disabled mode.

> In the blink mode the led is not as responsive as when NCQ is disabled,
> but at least it does show when there is I/O. If I disable NCQ by setting
> queue_depth to 1, blink mode is disabled and the led behaves as with
> pre-2.6.26 kernels.
>
> I've done my best to make the implementation as cheap as possible.
>
> One question I have is whether _all_ SOCs need this quirk or if we should
> test for a model or revision. Is there model/revision info available for SOC?
   if my understanding if the issue is correct, then this should apply
for all devices besides to the SOC.

Re: [RFC][PATCH] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[Martin Michlmayr]

* saeed bishara <saeed.bishara@gmail.com> [2009-02-24 11:33]:
> What I think is that this issue is seen with the specific disk that
> you have, the chip blinks the LED depending on the BUSY bit status,
> and I suspect that with the disk you have does clear that bit
> immediately after receiving the command FIS, so the LED will be off,
> though the command is not completed yet, and this makes the led
> responsiveness differ that in NCQ-disabled mode.

The LED issue was reported by two people on the QNAP forum the other
day.  I asked what disks they had and one has a WDC WD7500AACS-00D6B0
whereas the other one has a WDC WD10EADS-00L5B1.  Do you also use a
WDC disk, Frans?

References:
http://forum.qnap.com/viewtopic.php?f=147&t=1344&start=220#p55925
http://forum.qnap.com/viewtopic.php?f=147&t=1344&start=220#p56027
http://forum.qnap.com/viewtopic.php?f=147&t=1344&start=230#p56351
-- 
Martin Michlmayr
http://www.cyrius.com/

Re: [RFC][PATCH] sata_mv: harddisk activity led no longer responsive on QNAP TS-109

[Frans Pop]

On Sunday 01 March 2009, Martin Michlmayr wrote:
> * saeed bishara <saeed.bishara@gmail.com> [2009-02-24 11:33]:
> > What I think is that this issue is seen with the specific disk that
> > you have, the chip blinks the LED depending on the BUSY bit status,
> > and I suspect that with the disk you have does clear that bit
> > immediately after receiving the command FIS, so the LED will be off,
> > though the command is not completed yet, and this makes the led
> > responsiveness differ that in NCQ-disabled mode.
>
> The LED issue was reported by two people on the QNAP forum the other
> day.  I asked what disks they had and one has a WDC WD7500AACS-00D6B0
> whereas the other one has a WDC WD10EADS-00L5B1.  Do you also use a
> WDC disk, Frans?

I always was rather doubtful of Saeed's analysis (with all due respect), 
but it's good to see confirmed that the same problem manifests with other 
disks too.

I have a completely different HD:
Model Family:     Hitachi Deskstar T7K250 series
Device Model:     HDT722516DLA380
Firmware Version: V43OA91A
User Capacity:    164,696,555,520 bytes

Cheers,
FJP

[RFC][PATCH] sata-mv: add module parameter msq_blink_led to enable quirk for SOC

[Frans Pop]

For some Marvell chips the HDD led does not blink when there is disk
I/O if NCQ is enabled. Only enable the quirk that works around this
(by enabling blink mode for the led) if parameter msq_blink_led is
set as it is not clear whether this is a general erratum or related
to the type of hard disk connected to the controller.
    
Signed-off-by: Frans Pop <elendil@planet.nl>
Cc: Mark Lord <liml@rtr.ca>
Cc: Saeed Bishara <saeed.bishara@gmail.com>
---

This patch applies on top of my previous patch:
     sata_mv: enable HDD led blinking when NCQ is active for SOC
(see: http://marc.info/?l=linux-ide&m=123546443414518&w=2)

I hope this will make that patch more acceptable: it makes the use of the
quirk more flexible, allowing users who see the issue to activate it while
allowing others to ignore it and also allowing easy testing whether the
issue is (still) present.

I've chosen to keep the quirk disabled by default so that we'll continue
to get reports when someone's led is not working. Hopefully that will
help find out whether or not the issue is hard disk related or not.

Use of the parameter could be documented on websites such as
Martin Michlmayr's pages for the QNAP NAS devices on Debian
(http://www.cyrius.com/debian/orion/qnap/ts-109/).

Cheers,
FJP

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 59355d9..4382d2c 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -723,6 +723,15 @@ static const struct mv_hw_ops mv_soc_ops = {
 };
 
 /*
+ * module options
+ */
+static int msi;			/* Use PCI msi; either zero (off, default)
+				 * or non-zero */
+static int msq_blink_led;	/* Use blink mode for HDD activity led when
+				 * NCQ is enabled (SOC);
+				 * either zero (off, default) or non-zero */
+
+/*
  * Functions
  */
 
@@ -885,6 +894,9 @@ static int mv_has_port_using_ncq(struct ata_host *host)
 static void mv_quirk_blink_led_when_ncq(struct ata_port *ap,
 					int enable)
 {
+	if (!msq_blink_led)
+		return;
+
 	struct mv_host_priv *hpriv = ap->host->private_data;
 	void __iomem *mmio = hpriv->base;
 
@@ -3359,12 +3371,6 @@ static struct pci_driver mv_pci_driver = {
 	.remove			= ata_pci_remove_one,
 };
 
-/*
- * module options
- */
-static int msi;	      /* Use PCI msi; either zero (off, default) or non-zero */
-
-
 /* move to PCI layer or libata core? */
 static int pci_go_64(struct pci_dev *pdev)
 {
@@ -3550,6 +3556,10 @@ MODULE_ALIAS("platform:" DRV_NAME);
 module_param(msi, int, 0444);
 MODULE_PARM_DESC(msi, "Enable use of PCI MSI (0=off, 1=on)");
 #endif
+/* msq_blink_led only has effect for SOC */
+module_param(msq_blink_led, int, 0444);
+MODULE_PARM_DESC(msq_blink_led,
+	"Use blink mode quirk for HDD led when MSQ is enabled (0=off, 1=on)");
 
 module_init(mv_init);
 module_exit(mv_exit);

Re: [RFC][PATCH] sata-mv: add module parameter msq_blink_led to enable quirk for SOC

[Mark Lord]

Frans Pop wrote:
> For some Marvell chips the HDD led does not blink when there is disk
> I/O if NCQ is enabled. Only enable the quirk that works around this
> (by enabling blink mode for the led) if parameter msq_blink_led is
> set as it is not clear whether this is a general erratum or related
> to the type of hard disk connected to the controller.
...
+/* msq_blink_led only has effect for SOC */
+module_param(msq_blink_led, int, 0444);
+MODULE_PARM_DESC(msq_blink_led,
+	"Use blink mode quirk for HDD led when MSQ is enabled (0=off, 1=on)");
...

Well, I don't understand the parameter name (msq??),
and I'm not sure if we need a parameter or not.

Saeed said the original patch should apply "for all devices besides
to the SOC", which I think means all GenIIe chip variants.

So if you want to rework the first patch so that the blink fix
works for all GEN_IIE chips, then I'll test it here with a few
different drives from different vendors and see how the LEDs behave.
I also have boards from several different vendors to try it on.

I'd also like to investigate whether the same fix is necessary
for GEN_II chips as well as GEN_IIE.  Perhaps Saeed might provide
further information there, but we shouldn't hold our breaths waiting. :)

Cheers

Re: [RFC][PATCH] sata-mv: add module parameter msq_blink_led to enable quirk for SOC

[Martin Michlmayr]

* Mark Lord <liml@rtr.ca> [2009-03-10 11:06]:
> Saeed said the original patch should apply "for all devices besides
> to the SOC", which I think means all GenIIe chip variants.

Unfortunately, we're seeing the problem with the SoC...
-- 
Martin Michlmayr
http://www.cyrius.com/

Re: [RFC][PATCH] sata-mv: add module parameter msq_blink_led to enable quirk for SOC

[Mark Lord]

Martin Michlmayr wrote:
> * Mark Lord <liml@rtr.ca> [2009-03-10 11:06]:
>> Saeed said the original patch should apply "for all devices besides
>> to the SOC", which I think means all GenIIe chip variants.
> 
> Unfortunately, we're seeing the problem with the SoC...
..

That's fine, it will still fix that, as well as the other chips.

Re: [RFC][PATCH] sata-mv: add module parameter msq_blink_led to enable quirk for SOC

[Frans Pop]

On Tuesday 10 March 2009, Martin Michlmayr wrote:
> * Mark Lord <liml@rtr.ca> [2009-03-10 11:06]:
> > Saeed said the original patch should apply "for all devices besides
> > to the SOC", which I think means all GenIIe chip variants.
>
> Unfortunately, we're seeing the problem with the SoC...

AIUI the SoC _is_ a GenIIe chip variant. Mark is asking me to extend the 
code to the PCI-based GenIIe chips.

Re: [RFC][PATCH] sata-mv: add module parameter msq_blink_led to enable quirk for SOC

[Frans Pop]

On Tuesday 10 March 2009, Mark Lord wrote:
> Frans Pop wrote:
> > For some Marvell chips the HDD led does not blink when there is disk
> > I/O if NCQ is enabled. Only enable the quirk that works around this
> > (by enabling blink mode for the led) if parameter msq_blink_led is
> > set as it is not clear whether this is a general erratum or related
> > to the type of hard disk connected to the controller.
>
> ...
> +/* msq_blink_led only has effect for SOC */
> +module_param(msq_blink_led, int, 0444);
> +MODULE_PARM_DESC(msq_blink_led,
> +	"Use blink mode quirk for HDD led when MSQ is enabled (0=off,
> 1=on)"); ...
>
> Well, I don't understand the parameter name (msq??),

Brainfart: contamination from msi and phonetically ncq and msq are 
somewhat similar :-/
Will fix for next version.

> and I'm not sure if we need a parameter or not.

I don't either. But given the uncertainty surrounding this it seemed like 
as useful feature. I'll leave it as a separate patch so it can either be 
omitted or more easily reverted later.

> Saeed said the original patch should apply "for all devices besides
> to the SOC", which I think means all GenIIe chip variants.
>
> So if you want to rework the first patch so that the blink fix
> works for all GEN_IIE chips, then I'll test it here with a few
> different drives from different vendors and see how the LEDs behave.
> I also have boards from several different vendors to try it on.

Cool. Will do. I'm going to assume I can use the same code to actually 
enable the blink mode.
For the testing at least the parameter will be useful.

> I'd also like to investigate whether the same fix is necessary
> for GEN_II chips as well as GEN_IIE.  Perhaps Saeed might provide
> further information there, but we shouldn't hold our breaths waiting.

OK. I'll implement it as an op so it can be extended as needed.

Thanks,
FJP