[PATCH] sata_nv: fix nmi intr or system hanging in rhel4u6 adma.
Hi, robert One customer reported that their system received a nmi interrupt after issuing "dd if=/dev/sdb of=/dev/null" on a defective disk in rhel4u6. I tested it and found that my system hung both in rhel4u6(2.6.9-67) and 2.6.24-rc7. The patch can work well, but I am not sure if the patch has other potential effect on adma. I attached a file in case of lines breaked. The below info comes from Gunther Mayer to reproduce the issue. " used a Seagate ST3500841NS 3.AE for my test; probably other seagate drives are also capable of creating media errors with the new hdparm-8.1: - compile hdparm-8.1 - hdparm -- yes-i-know-what-i-am-doing --make-bad-sector 6 /dev/sdb Unfortunately this does not succeed for nvidia sata controller (timeouts et al.), but it worked fine on AHCI machine (e.g. FSC R640). When I insert this newly created defective disk in Ultra 20, it reboots within seconds after issueing "dd if=/dev/sdb of=/dev/null". " Signed-off-by: [EMAIL PROTECTED] --- drivers/ata/sata_nv.c |5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/ata/sata_nv.c b/drivers/ata/sata_nv.c index ed5473b..e824260 100644 --- a/drivers/ata/sata_nv.c +++ b/drivers/ata/sata_nv.c @@ -837,9 +837,10 @@ static void nv_adma_tf_read(struct ata_port *ap, struct ata_taskfile *tf) all shortly be aborted anyway. We assume that NCQ commands are not issued via passthrough, which is the only way that switching into ADMA mode could abort outstanding commands. */ - nv_adma_register_mode(ap); + struct nv_adma_port_priv *pp = ap->private_data; - ata_tf_read(ap, tf); + if (pp->flags & NV_ADMA_PORT_REGISTER_MODE) + ata_tf_read(ap, tf); } static unsigned int nv_adma_tf_to_cpb(struct ata_taskfile *tf, __le16 *cpb) - Best regards, Kuan Luo --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- nmi-patch2 Description: nmi-patch2
[PATCH] sata_nv: fix nmi intr or system hanging in rhel4u6 adma.
Hi, robert One customer reported that their system received a nmi interrupt after issuing dd if=/dev/sdb of=/dev/null on a defective disk in rhel4u6. I tested it and found that my system hung both in rhel4u6(2.6.9-67) and 2.6.24-rc7. The patch can work well, but I am not sure if the patch has other potential effect on adma. I attached a file in case of lines breaked. The below info comes from Gunther Mayer to reproduce the issue. used a Seagate ST3500841NS 3.AE for my test; probably other seagate drives are also capable of creating media errors with the new hdparm-8.1: - compile hdparm-8.1 - hdparm -- yes-i-know-what-i-am-doing --make-bad-sector 6 /dev/sdb Unfortunately this does not succeed for nvidia sata controller (timeouts et al.), but it worked fine on AHCI machine (e.g. FSC R640). When I insert this newly created defective disk in Ultra 20, it reboots within seconds after issueing dd if=/dev/sdb of=/dev/null. Signed-off-by: [EMAIL PROTECTED] --- drivers/ata/sata_nv.c |5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/ata/sata_nv.c b/drivers/ata/sata_nv.c index ed5473b..e824260 100644 --- a/drivers/ata/sata_nv.c +++ b/drivers/ata/sata_nv.c @@ -837,9 +837,10 @@ static void nv_adma_tf_read(struct ata_port *ap, struct ata_taskfile *tf) all shortly be aborted anyway. We assume that NCQ commands are not issued via passthrough, which is the only way that switching into ADMA mode could abort outstanding commands. */ - nv_adma_register_mode(ap); + struct nv_adma_port_priv *pp = ap-private_data; - ata_tf_read(ap, tf); + if (pp-flags NV_ADMA_PORT_REGISTER_MODE) + ata_tf_read(ap, tf); } static unsigned int nv_adma_tf_to_cpb(struct ata_taskfile *tf, __le16 *cpb) - Best regards, Kuan Luo --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- nmi-patch2 Description: nmi-patch2
RE: fixed a bug of adma in rhel4u5 with HDS7250SASUN500G.
Robert wrote: > Kuan Luo wrote: > > Robert worte. > >> Kuan, does this patch (using the notifiers to see if the > command is > >> really done) still work if one port on the controller has > >> ADMA disabled > >> because it's in ATAPI mode? I seem to recall Allen Martin > mentioning > >> that notifiers wouldn't work in this case. > >> > > > > I just tried the 2.6.24-rc7 sata_nv driver with one hd and > one cdrom in > > the same controller. > > I mkfs hd and mounted the cdrom and no error happened. > > > > Allen, is there anything about notifier that we should pay > attention > > to? > > Assuming not, then this patch should be applied.. > > The patch should be applied. We use the notifier register and there is nothing to do with our notifier register in atapi mode. Allen wrote: I think that's one of the cases where memory notifiers don't work (one of the drives is not in ADMA mode either because it's ATAPI or it's in legacy mode). There's no issue with the notifier registers though. --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: fixed a bug of adma in rhel4u5 with HDS7250SASUN500G.
robert wrote: > Kuan Luo wrote: > > Robert worte. > >> Kuan, does this patch (using the notifiers to see if the > command is > >> really done) still work if one port on the controller has > >> ADMA disabled > >> because it's in ATAPI mode? I seem to recall Allen Martin > mentioning > >> that notifiers wouldn't work in this case. > >> > > > > I just tried the 2.6.24-rc7 sata_nv driver with one hd and > one cdrom in > > the same controller. > > I mkfs hd and mounted the cdrom and no error happened. > > > > Allen, is there anything about notifier that we should pay > attention > > to? > > Assuming not, then this patch should be applied.. > > I am asking someone about the issue. Soon i will be getting a concrete response. --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: fixed a bug of adma in rhel4u5 with HDS7250SASUN500G.
robert wrote: Kuan Luo wrote: Robert worte. Kuan, does this patch (using the notifiers to see if the command is really done) still work if one port on the controller has ADMA disabled because it's in ATAPI mode? I seem to recall Allen Martin mentioning that notifiers wouldn't work in this case. I just tried the 2.6.24-rc7 sata_nv driver with one hd and one cdrom in the same controller. I mkfs hd and mounted the cdrom and no error happened. Allen, is there anything about notifier that we should pay attention to? Assuming not, then this patch should be applied.. I am asking someone about the issue. Soon i will be getting a concrete response. --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: fixed a bug of adma in rhel4u5 with HDS7250SASUN500G.
Robert wrote: Kuan Luo wrote: Robert worte. Kuan, does this patch (using the notifiers to see if the command is really done) still work if one port on the controller has ADMA disabled because it's in ATAPI mode? I seem to recall Allen Martin mentioning that notifiers wouldn't work in this case. I just tried the 2.6.24-rc7 sata_nv driver with one hd and one cdrom in the same controller. I mkfs hd and mounted the cdrom and no error happened. Allen, is there anything about notifier that we should pay attention to? Assuming not, then this patch should be applied.. The patch should be applied. We use the notifier register and there is nothing to do with our notifier register in atapi mode. Allen wrote: I think that's one of the cases where memory notifiers don't work (one of the drives is not in ADMA mode either because it's ATAPI or it's in legacy mode). There's no issue with the notifier registers though. --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: fixed a bug of adma in rhel4u5 with HDS7250SASUN500G.
Robert worte. > > Kuan, does this patch (using the notifiers to see if the command is > really done) still work if one port on the controller has > ADMA disabled > because it's in ATAPI mode? I seem to recall Allen Martin mentioning > that notifiers wouldn't work in this case. > I just tried the 2.6.24-rc7 sata_nv driver with one hd and one cdrom in the same controller. I mkfs hd and mounted the cdrom and no error happened. Allen, is there anything about notifier that we should pay attention to? > > > > > * it sure seems like there are other open sata_nv ADMA > issues -- can we > > hard-confirm or deny this? bugzilla wasn't very helpful > for me. It > > doesn't seem like we can disable ADMA (to solve those > issues) and get > > enough test time in (which is what I said a week (or more?) > ago too...) > > The NCQ/non-NCQ command switching issue is still hitting some people > (last I heard Kuan was looking into this), also there's a > hotplug issue > that Tejun reported.. > I have not yet reproduced the switching issue even if i removed the udelay function according to your metholds. I tried the 2.6.24-rc7. I don't know what kernel version can easily reproduce the issue or mabye i omit some steps during test. --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: fixed a bug of adma in rhel4u5 with HDS7250SASUN500G.
Robert worte. Kuan, does this patch (using the notifiers to see if the command is really done) still work if one port on the controller has ADMA disabled because it's in ATAPI mode? I seem to recall Allen Martin mentioning that notifiers wouldn't work in this case. I just tried the 2.6.24-rc7 sata_nv driver with one hd and one cdrom in the same controller. I mkfs hd and mounted the cdrom and no error happened. Allen, is there anything about notifier that we should pay attention to? * it sure seems like there are other open sata_nv ADMA issues -- can we hard-confirm or deny this? bugzilla wasn't very helpful for me. It doesn't seem like we can disable ADMA (to solve those issues) and get enough test time in (which is what I said a week (or more?) ago too...) The NCQ/non-NCQ command switching issue is still hitting some people (last I heard Kuan was looking into this), also there's a hotplug issue that Tejun reported.. I have not yet reproduced the switching issue even if i removed the udelay function according to your metholds. I tried the 2.6.24-rc7. I don't know what kernel version can easily reproduce the issue or mabye i omit some steps during test. --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: fixed a bug of adma in rhel4u5 with HDS7250SASUN500G.
Robert Hancock wrote: > As I mentioned, this doesn't seem to resolve the problem we're seeing > with rapidly intermixed NCQ commands and cache flushes (at > least, if I > take out the arbitrary 20usec delay from the driver and add > this patch, > the problem still shows up). It could be a similar problem, > though, of > commands being issued before the controller is really ready > for them. If > you or others at NVIDIA could assist in tracking down that problem it > would be appreciated.. > Ok , i will track down that problem. --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: fixed a bug of adma in rhel4u5 with HDS7250SASUN500G.
Robert hancock wrote: > What problem does this resolve? I tested it against the cache > flush/NCQ > write switching problem we've been trying to solve, and it > doesn't look > like it fixes that one - if I apply this patch and then remove the > udelay(20) in sata_nv.c that I added which prevented me from > seeing this > problem before, it shows up. > First thank davide to help to send the attachment. Robert, The patch is to solve the error message "ata1: CPB flags CMD err, flags=0x11" when testing HDS7250SASUN500G in rhel4u5. I tested this hd in 2.6.24-rc7 which needed to remove the mask in blacklist to run the ncq and the same error also showed up. I traced the bug and found that the interrupt finished a command (for example, tag=0) when the driver got that adma status is NV_ADMA_STAT_DONE and cpb->resp_flags is NV_CPB_RESP_DONE. However, For this hd, the drive maybe didn't clear bit 0 at this moment. It meaned the hardware had not completely finished the command. If at the same time the driver freed the command(tag 0) and sended another command (tag 0), the error happened. The notifier register is 32-bit register containing notifier value. Value is bit vector containing one bit per tag number (0-31) in corresponding bit positions (bit 0 is for tag 0, etc). When bit is set then ADMA indicates that command with corresponding tag number completed execution. So i added the check notifier code. Sometimes i saw that the notifier reg set some bits , but the adma status set NV_ADMA_STAT_CMD_COMPLETE ,not NV_ADMA_STAT_DONE. So i added the NV_ADMA_STAT_CMD_COMPLETE check code. --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: fixed a bug of adma in rhel4u5 with HDS7250SASUN500G.
Robert hancock wrote: What problem does this resolve? I tested it against the cache flush/NCQ write switching problem we've been trying to solve, and it doesn't look like it fixes that one - if I apply this patch and then remove the udelay(20) in sata_nv.c that I added which prevented me from seeing this problem before, it shows up. First thank davide to help to send the attachment. Robert, The patch is to solve the error message ata1: CPB flags CMD err, flags=0x11 when testing HDS7250SASUN500G in rhel4u5. I tested this hd in 2.6.24-rc7 which needed to remove the mask in blacklist to run the ncq and the same error also showed up. I traced the bug and found that the interrupt finished a command (for example, tag=0) when the driver got that adma status is NV_ADMA_STAT_DONE and cpb-resp_flags is NV_CPB_RESP_DONE. However, For this hd, the drive maybe didn't clear bit 0 at this moment. It meaned the hardware had not completely finished the command. If at the same time the driver freed the command(tag 0) and sended another command (tag 0), the error happened. The notifier register is 32-bit register containing notifier value. Value is bit vector containing one bit per tag number (0-31) in corresponding bit positions (bit 0 is for tag 0, etc). When bit is set then ADMA indicates that command with corresponding tag number completed execution. So i added the check notifier code. Sometimes i saw that the notifier reg set some bits , but the adma status set NV_ADMA_STAT_CMD_COMPLETE ,not NV_ADMA_STAT_DONE. So i added the NV_ADMA_STAT_CMD_COMPLETE check code. --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: fixed a bug of adma in rhel4u5 with HDS7250SASUN500G.
Robert Hancock wrote: As I mentioned, this doesn't seem to resolve the problem we're seeing with rapidly intermixed NCQ commands and cache flushes (at least, if I take out the arbitrary 20usec delay from the driver and add this patch, the problem still shows up). It could be a similar problem, though, of commands being issued before the controller is really ready for them. If you or others at NVIDIA could assist in tracking down that problem it would be appreciated.. Ok , i will track down that problem. --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] sata_nv,adma: fix error when rmmod sata_nv
Tejun Heo wrote: > Which kernel version are you using? The following commit should have > fixed the problem. Please give a shot at 2.6.24-rc5. Thanks. > > commit 32ebbc0c0d5d18c0135b55d1eb0029f48c54aff0 > Author: Tejun Heo <[EMAIL PROTECTED]> > Date: Thu Nov 8 13:09:00 2007 +0900 > > libata: port and host should be stopped before hardware resources are > released > > -- > tejun > I have seen your commit. And i apologize for not updating the newest kernel. --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] sata_nv,adma: fix error when rmmod sata_nv
Tejun Heo wrote: Which kernel version are you using? The following commit should have fixed the problem. Please give a shot at 2.6.24-rc5. Thanks. commit 32ebbc0c0d5d18c0135b55d1eb0029f48c54aff0 Author: Tejun Heo [EMAIL PROTECTED] Date: Thu Nov 8 13:09:00 2007 +0900 libata: port and host should be stopped before hardware resources are released -- tejun I have seen your commit. And i apologize for not updating the newest kernel. --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] sata_nv,adma: fix error when rmmod sata_nv
hi,
The below error happens when i rmmod sata_nv in adma mode on ck804
chipset with 2.6.24 kernel.
I traced the code and found that the driver attempts to write device mem
that has been unmapped.
Only simply removing the code" writew(0, mmio + NV_ADMA_CTL);" in the
nv_adma_port_stop function or remove .port_stop field in nv_adma_ops,
rmmod is ok.
static void nv_adma_port_stop(struct ata_port *ap)
{
struct nv_adma_port_priv *pp = ap->private_data;
void __iomem *mmio = pp->ctl_block;
VPRINTK("ENTER\n");
- writew(0, mmio + NV_ADMA_CTL);
}
Or
Place pcim_iomap_regions before ata_pci_prepare_native_host in
nv_init_one function.
This can guarantee that the code "writew(0, mmio + NV_ADMA_CTL) " write
device mem before the device mem is unmapped.
messages:
Dec 19 02:23:24 localhost kernel: ata16.00: disabled
Dec 19 02:23:24 localhost kernel: sd 15:0:0:0: [sda] Synchronizing SCSI
cache
Dec 19 02:23:24 localhost kernel: sd 15:0:0:0: [sda] Result:
hostbyte=0x04 driverbyte=0x00
Dec 19 02:23:24 localhost kernel: sd 15:0:0:0: [sda] Stopping disk
Dec 19 02:23:24 localhost kernel: sd 15:0:0:0: [sda] START_STOP FAILED
Dec 19 02:23:24 localhost kernel: sd 15:0:0:0: [sda] Result:
hostbyte=0x04 driverbyte=0x00
Dec 19 02:23:24 localhost kernel: BUG: unable to handle kernel paging
request at virtual address e881a4c0
Dec 19 02:23:24 localhost kernel: printing eip: e88224e9 *pde = 0180c067
*pte =
Dec 19 02:23:24 localhost kernel: Oops: 0002 [#1] SMP
Dec 19 02:23:24 localhost kernel: Modules linked in: sata_nv libata
Dec 19 02:23:24 localhost kernel:
Dec 19 02:23:24 localhost kernel: Pid: 4915, comm: rmmod Not tainted
(2.6.24-rc1-ge2e031eb #2)
Dec 19 02:23:24 localhost kernel: EIP: 0060:[] EFLAGS:
00210292 CPU: 1
Dec 19 02:23:24 localhost kernel: EIP is at nv_adma_port_stop+0x3d/0x53
[sata_nv]
Dec 19 02:23:24 localhost kernel: EAX: 0015 EBX: e881a480 ECX:
00200046 EDX: 00200046
Dec 19 02:23:24 localhost kernel: ESI: c220b50c EDI: c1979154 EBP:
c197904c ESP: c2429ecc
Dec 19 02:23:24 localhost kernel: DS: 007b ES: 007b FS: 00d8 GS: 0033
SS: 0068
Dec 19 02:23:24 localhost kernel: Process rmmod (pid: 4915, ti=c2428000
task=c25e5a40 task.ti=c2428000)
Dec 19 02:23:24 localhost kernel: Stack: e8823e1b e881a480
e884398e c220b500 c18a3ec0 c1979154
Dec 19 02:23:24 localhost kernel:c02818be c1979154 c197904c
000d c18a3ec0 c18a3a40 c197904c e88259d0
Dec 19 02:23:24 localhost kernel:e88259d0 c2428000 c0281987
00200286 c197904c c027f8bb c1834600 c197904c
Dec 19 02:23:24 localhost kernel: Call Trace:
Dec 19 02:23:24 localhost kernel: []
ata_host_release+0x2f/0x92 [libata]
Dec 19 02:23:24 localhost kernel: []
release_nodes+0x10f/0x12f
Dec 19 02:23:24 localhost kernel: []
devres_release_all+0x27/0x2a
Dec 19 02:23:24 localhost kernel: []
__device_release_driver+0x72/0x88
Dec 19 02:23:24 localhost kernel: [] driver_detach+0x76/0xb3
Dec 19 02:23:24 localhost kernel: []
bus_remove_driver+0x63/0x81
Dec 19 02:23:24 localhost kernel: []
pci_unregister_driver+0xc/0x57
Dec 19 02:23:24 localhost kernel: []
sys_delete_module+0x197/0x1bd
Dec 19 02:23:24 localhost kernel: []
do_page_fault+0x202/0x581
Dec 19 02:23:24 localhost kernel: [] do_munmap+0x193/0x1ac
Dec 19 02:23:24 localhost kernel: []
sysenter_past_esp+0x5f/0x85
Dec 19 02:23:24 localhost kernel: ===
Dec 19 02:23:24 localhost kernel: Code: 3e 82 e8 e8 b8 30 90 d7 89 5c 24
04 c7 04 24 0f 3e 82 e8 e8 a8 30 90 d7 8b 5b 10 c7 04 24 1b 3e 82 e8 89
5c 24 04 e8 95 30 90 d7 <66> c7 43 40 00 00 c7 04 24 29 3e 82 e8 e8 83
30 90 d7 5b 58 5b
Dec 19 02:23:24 localhost kernel: EIP: []
nv_adma_port_stop+0x3d/0x53 [sata_nv] SS:ESP 0068:c2429ecc
---
This email message is for the sole use of the intended recipient(s) and may
contain
confidential information. Any unauthorized review, use, disclosure or
distribution
is prohibited. If you are not the intended recipient, please contact the
sender by
reply email and destroy all copies of the original message.
---
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] sata_nv,adma: fix error when rmmod sata_nv
hi,
The below error happens when i rmmod sata_nv in adma mode on ck804
chipset with 2.6.24 kernel.
I traced the code and found that the driver attempts to write device mem
that has been unmapped.
Only simply removing the code writew(0, mmio + NV_ADMA_CTL); in the
nv_adma_port_stop function or remove .port_stop field in nv_adma_ops,
rmmod is ok.
static void nv_adma_port_stop(struct ata_port *ap)
{
struct nv_adma_port_priv *pp = ap-private_data;
void __iomem *mmio = pp-ctl_block;
VPRINTK(ENTER\n);
- writew(0, mmio + NV_ADMA_CTL);
}
Or
Place pcim_iomap_regions before ata_pci_prepare_native_host in
nv_init_one function.
This can guarantee that the code writew(0, mmio + NV_ADMA_CTL) write
device mem before the device mem is unmapped.
messages:
Dec 19 02:23:24 localhost kernel: ata16.00: disabled
Dec 19 02:23:24 localhost kernel: sd 15:0:0:0: [sda] Synchronizing SCSI
cache
Dec 19 02:23:24 localhost kernel: sd 15:0:0:0: [sda] Result:
hostbyte=0x04 driverbyte=0x00
Dec 19 02:23:24 localhost kernel: sd 15:0:0:0: [sda] Stopping disk
Dec 19 02:23:24 localhost kernel: sd 15:0:0:0: [sda] START_STOP FAILED
Dec 19 02:23:24 localhost kernel: sd 15:0:0:0: [sda] Result:
hostbyte=0x04 driverbyte=0x00
Dec 19 02:23:24 localhost kernel: BUG: unable to handle kernel paging
request at virtual address e881a4c0
Dec 19 02:23:24 localhost kernel: printing eip: e88224e9 *pde = 0180c067
*pte =
Dec 19 02:23:24 localhost kernel: Oops: 0002 [#1] SMP
Dec 19 02:23:24 localhost kernel: Modules linked in: sata_nv libata
Dec 19 02:23:24 localhost kernel:
Dec 19 02:23:24 localhost kernel: Pid: 4915, comm: rmmod Not tainted
(2.6.24-rc1-ge2e031eb #2)
Dec 19 02:23:24 localhost kernel: EIP: 0060:[e88224e9] EFLAGS:
00210292 CPU: 1
Dec 19 02:23:24 localhost kernel: EIP is at nv_adma_port_stop+0x3d/0x53
[sata_nv]
Dec 19 02:23:24 localhost kernel: EAX: 0015 EBX: e881a480 ECX:
00200046 EDX: 00200046
Dec 19 02:23:24 localhost kernel: ESI: c220b50c EDI: c1979154 EBP:
c197904c ESP: c2429ecc
Dec 19 02:23:24 localhost kernel: DS: 007b ES: 007b FS: 00d8 GS: 0033
SS: 0068
Dec 19 02:23:24 localhost kernel: Process rmmod (pid: 4915, ti=c2428000
task=c25e5a40 task.ti=c2428000)
Dec 19 02:23:24 localhost kernel: Stack: e8823e1b e881a480
e884398e c220b500 c18a3ec0 c1979154
Dec 19 02:23:24 localhost kernel:c02818be c1979154 c197904c
000d c18a3ec0 c18a3a40 c197904c e88259d0
Dec 19 02:23:24 localhost kernel:e88259d0 c2428000 c0281987
00200286 c197904c c027f8bb c1834600 c197904c
Dec 19 02:23:24 localhost kernel: Call Trace:
Dec 19 02:23:24 localhost kernel: [e884398e]
ata_host_release+0x2f/0x92 [libata]
Dec 19 02:23:24 localhost kernel: [c02818be]
release_nodes+0x10f/0x12f
Dec 19 02:23:24 localhost kernel: [c0281987]
devres_release_all+0x27/0x2a
Dec 19 02:23:24 localhost kernel: [c027f8bb]
__device_release_driver+0x72/0x88
Dec 19 02:23:24 localhost kernel: [c027fccb] driver_detach+0x76/0xb3
Dec 19 02:23:24 localhost kernel: [c027f49f]
bus_remove_driver+0x63/0x81
Dec 19 02:23:24 localhost kernel: [c0228b5a]
pci_unregister_driver+0xc/0x57
Dec 19 02:23:24 localhost kernel: [c0143792]
sys_delete_module+0x197/0x1bd
Dec 19 02:23:24 localhost kernel: [c0417673]
do_page_fault+0x202/0x581
Dec 19 02:23:24 localhost kernel: [c01570ec] do_munmap+0x193/0x1ac
Dec 19 02:23:24 localhost kernel: [c0104e4e]
sysenter_past_esp+0x5f/0x85
Dec 19 02:23:24 localhost kernel: ===
Dec 19 02:23:24 localhost kernel: Code: 3e 82 e8 e8 b8 30 90 d7 89 5c 24
04 c7 04 24 0f 3e 82 e8 e8 a8 30 90 d7 8b 5b 10 c7 04 24 1b 3e 82 e8 89
5c 24 04 e8 95 30 90 d7 66 c7 43 40 00 00 c7 04 24 29 3e 82 e8 e8 83
30 90 d7 5b 58 5b
Dec 19 02:23:24 localhost kernel: EIP: [e88224e9]
nv_adma_port_stop+0x3d/0x53 [sata_nv] SS:ESP 0068:c2429ecc
---
This email message is for the sole use of the intended recipient(s) and may
contain
confidential information. Any unauthorized review, use, disclosure or
distribution
is prohibited. If you are not the intended recipient, please contact the
sender by
reply email and destroy all copies of the original message.
---
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ata: Add the SW NCQ support to sata_nv for MCP51/MCP55/MCP61
From: Kuan Luo <[EMAIL PROTECTED]>
Add the Software NCQ support to sata_nv.c for MCP51/MCP55/MCP61 SATA
controller. NCQ function is disable by default, you can enable it with
'swncq=1'. NCQ will be turned off if the drive is Maxtor on MCP51 or
MCP55 rev 0xa2 platform.
Signed-off-by: Kuan Luo <[EMAIL PROTECTED]>
Signed-off-by: Peer Chen <[EMAIL PROTECTED]>
---
drivers/ata/sata_nv.c | 859 +++-
1 file changed, 850 insertions(+), 9 deletions(-)
diff -purN a/drivers/ata/sata_nv.c b/drivers/ata/sata_nv.c
--- a/drivers/ata/sata_nv.c 2007-06-13 10:15:07.0 -0400
+++ b/drivers/ata/sata_nv.c 2007-07-05 10:20:29.0 -0400
@@ -169,6 +169,35 @@ enum {
NV_ADMA_PORT_REGISTER_MODE = (1 << 0),
NV_ADMA_ATAPI_SETUP_COMPLETE= (1 << 1),
+ /* MCP55 reg offset */
+ NV_CTL_MCP55= 0x400,
+ NV_INT_STATUS_MCP55 = 0x440,
+ NV_INT_ENABLE_MCP55 = 0x444,
+ NV_NCQ_REG_MCP55= 0x448,
+
+ /* MCP55 */
+ NV_INT_ALL_MCP55= 0x,
+ NV_INT_PORT_SHIFT_MCP55 = 16, /* each port occupies 16 bits */
+ NV_INT_MASK_MCP55 = NV_INT_ALL_MCP55 & 0xfffd,
+
+ /* SWNCQ ENABLE BITS*/
+ NV_CTL_PRI_SWNCQ= 0x02,
+ NV_CTL_SEC_SWNCQ= 0x04,
+
+ /* SW NCQ status bits*/
+ NV_SWNCQ_IRQ_DEV= (1 << 0),
+ NV_SWNCQ_IRQ_PM = (1 << 1),
+ NV_SWNCQ_IRQ_ADDED = (1 << 2),
+ NV_SWNCQ_IRQ_REMOVED= (1 << 3),
+
+ NV_SWNCQ_IRQ_BACKOUT= (1 << 4),
+ NV_SWNCQ_IRQ_SDBFIS = (1 << 5),
+ NV_SWNCQ_IRQ_DHREGFIS = (1 << 6),
+ NV_SWNCQ_IRQ_DMASETUP = (1 << 7),
+
+ NV_SWNCQ_IRQ_HOTPLUG= NV_SWNCQ_IRQ_ADDED |
+ NV_SWNCQ_IRQ_REMOVED,
+
};
/* ADMA Physical Region Descriptor - one SG segment */
@@ -226,6 +255,37 @@ struct nv_host_priv {
unsigned long type;
};
+struct defer_queue {
+ u32 defer_bits;
+ unsigned inthead;
+ unsigned inttail;
+ unsigned inttag[ATA_MAX_QUEUE];
+};
+
+struct nv_swncq_port_priv {
+ struct ata_prd *prd;/* our SG list */
+ dma_addr_t prd_dma; /* and its DMA mapping */
+ void __iomem*sactive_block;
+ void __iomem*irq_block;
+ void __iomem*tag_block;
+ u32 qc_active;
+ unsigned intlast_issue_tag;
+ spinlock_t lock;
+ /* fifo circular queue to store deferral command */
+ struct defer_queue defer_queue;
+
+ /* for NCQ interrupt analysis */
+ u32 dhfis_bits;
+ u32 dmafis_bits;
+ u32 sdbfis_bits;
+
+ unsigned intncq_saw_d2h:1;
+ unsigned intncq_saw_dmas:1;
+ unsigned intncq_saw_sdb:1;
+ unsigned intncq_saw_backout:1;
+};
+
+
#define NV_ADMA_CHECK_INTR(GCTL, PORT) ((GCTL) & ( 1 << (19 + (12 * (PORT)
static int nv_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
@@ -263,13 +323,29 @@ static void nv_adma_host_stop(struct ata
static void nv_adma_post_internal_cmd(struct ata_queued_cmd *qc);
static void nv_adma_tf_read(struct ata_port *ap, struct ata_taskfile *tf);
+static void nv_mcp55_thaw(struct ata_port *ap);
+static void nv_mcp55_freeze(struct ata_port *ap);
+static void nv_swncq_error_handler(struct ata_port *ap);
+static int nv_swncq_slave_config(struct scsi_device *sdev);
+static int nv_swncq_port_start(struct ata_port *ap);
+static void nv_swncq_qc_prep(struct ata_queued_cmd *qc);
+static void nv_swncq_fill_sg(struct ata_queued_cmd *qc);
+static unsigned int nv_swncq_qc_issue(struct ata_queued_cmd *qc);
+static void nv_swncq_irq_clear(struct ata_port *ap, u16 fis);
+static irqreturn_t nv_swncq_interrupt(int irq, void *dev_instance);
+#ifdef CONFIG_PM
+static int nv_swncq_port_suspend(struct ata_port *ap, pm_message_t mesg);
+static int nv_swncq_port_resume(struct ata_port *ap);
+#endif
+
enum nv_host_type
{
GENERIC,
NFORCE2,
NFORCE3 = NFORCE2, /* NF2 == NF3 as far as sata_nv is concerned */
CK804,
- ADMA
+ ADMA,
+ SWNCQ
};
static const struct pci_device_id nv_pci_tbl[] = {
@@ -280,13 +356,13 @@ static const struct pci_device_id nv_pci
{ PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_CK804_SATA2), CK804 },
{ PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP04_SATA), CK804 },
{ PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP04_SATA2), CK804 },
- { PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP51_SATA), GENERIC
},
- { PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP51_SATA2)
[PATCH] ata: Add the SW NCQ support to sata_nv for MCP51/MCP55/MCP61
From: Kuan Luo [EMAIL PROTECTED]
Add the Software NCQ support to sata_nv.c for MCP51/MCP55/MCP61 SATA
controller. NCQ function is disable by default, you can enable it with
'swncq=1'. NCQ will be turned off if the drive is Maxtor on MCP51 or
MCP55 rev 0xa2 platform.
Signed-off-by: Kuan Luo [EMAIL PROTECTED]
Signed-off-by: Peer Chen [EMAIL PROTECTED]
---
drivers/ata/sata_nv.c | 859 +++-
1 file changed, 850 insertions(+), 9 deletions(-)
diff -purN a/drivers/ata/sata_nv.c b/drivers/ata/sata_nv.c
--- a/drivers/ata/sata_nv.c 2007-06-13 10:15:07.0 -0400
+++ b/drivers/ata/sata_nv.c 2007-07-05 10:20:29.0 -0400
@@ -169,6 +169,35 @@ enum {
NV_ADMA_PORT_REGISTER_MODE = (1 0),
NV_ADMA_ATAPI_SETUP_COMPLETE= (1 1),
+ /* MCP55 reg offset */
+ NV_CTL_MCP55= 0x400,
+ NV_INT_STATUS_MCP55 = 0x440,
+ NV_INT_ENABLE_MCP55 = 0x444,
+ NV_NCQ_REG_MCP55= 0x448,
+
+ /* MCP55 */
+ NV_INT_ALL_MCP55= 0x,
+ NV_INT_PORT_SHIFT_MCP55 = 16, /* each port occupies 16 bits */
+ NV_INT_MASK_MCP55 = NV_INT_ALL_MCP55 0xfffd,
+
+ /* SWNCQ ENABLE BITS*/
+ NV_CTL_PRI_SWNCQ= 0x02,
+ NV_CTL_SEC_SWNCQ= 0x04,
+
+ /* SW NCQ status bits*/
+ NV_SWNCQ_IRQ_DEV= (1 0),
+ NV_SWNCQ_IRQ_PM = (1 1),
+ NV_SWNCQ_IRQ_ADDED = (1 2),
+ NV_SWNCQ_IRQ_REMOVED= (1 3),
+
+ NV_SWNCQ_IRQ_BACKOUT= (1 4),
+ NV_SWNCQ_IRQ_SDBFIS = (1 5),
+ NV_SWNCQ_IRQ_DHREGFIS = (1 6),
+ NV_SWNCQ_IRQ_DMASETUP = (1 7),
+
+ NV_SWNCQ_IRQ_HOTPLUG= NV_SWNCQ_IRQ_ADDED |
+ NV_SWNCQ_IRQ_REMOVED,
+
};
/* ADMA Physical Region Descriptor - one SG segment */
@@ -226,6 +255,37 @@ struct nv_host_priv {
unsigned long type;
};
+struct defer_queue {
+ u32 defer_bits;
+ unsigned inthead;
+ unsigned inttail;
+ unsigned inttag[ATA_MAX_QUEUE];
+};
+
+struct nv_swncq_port_priv {
+ struct ata_prd *prd;/* our SG list */
+ dma_addr_t prd_dma; /* and its DMA mapping */
+ void __iomem*sactive_block;
+ void __iomem*irq_block;
+ void __iomem*tag_block;
+ u32 qc_active;
+ unsigned intlast_issue_tag;
+ spinlock_t lock;
+ /* fifo circular queue to store deferral command */
+ struct defer_queue defer_queue;
+
+ /* for NCQ interrupt analysis */
+ u32 dhfis_bits;
+ u32 dmafis_bits;
+ u32 sdbfis_bits;
+
+ unsigned intncq_saw_d2h:1;
+ unsigned intncq_saw_dmas:1;
+ unsigned intncq_saw_sdb:1;
+ unsigned intncq_saw_backout:1;
+};
+
+
#define NV_ADMA_CHECK_INTR(GCTL, PORT) ((GCTL) ( 1 (19 + (12 * (PORT)
static int nv_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
@@ -263,13 +323,29 @@ static void nv_adma_host_stop(struct ata
static void nv_adma_post_internal_cmd(struct ata_queued_cmd *qc);
static void nv_adma_tf_read(struct ata_port *ap, struct ata_taskfile *tf);
+static void nv_mcp55_thaw(struct ata_port *ap);
+static void nv_mcp55_freeze(struct ata_port *ap);
+static void nv_swncq_error_handler(struct ata_port *ap);
+static int nv_swncq_slave_config(struct scsi_device *sdev);
+static int nv_swncq_port_start(struct ata_port *ap);
+static void nv_swncq_qc_prep(struct ata_queued_cmd *qc);
+static void nv_swncq_fill_sg(struct ata_queued_cmd *qc);
+static unsigned int nv_swncq_qc_issue(struct ata_queued_cmd *qc);
+static void nv_swncq_irq_clear(struct ata_port *ap, u16 fis);
+static irqreturn_t nv_swncq_interrupt(int irq, void *dev_instance);
+#ifdef CONFIG_PM
+static int nv_swncq_port_suspend(struct ata_port *ap, pm_message_t mesg);
+static int nv_swncq_port_resume(struct ata_port *ap);
+#endif
+
enum nv_host_type
{
GENERIC,
NFORCE2,
NFORCE3 = NFORCE2, /* NF2 == NF3 as far as sata_nv is concerned */
CK804,
- ADMA
+ ADMA,
+ SWNCQ
};
static const struct pci_device_id nv_pci_tbl[] = {
@@ -280,13 +356,13 @@ static const struct pci_device_id nv_pci
{ PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_CK804_SATA2), CK804 },
{ PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP04_SATA), CK804 },
{ PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP04_SATA2), CK804 },
- { PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP51_SATA), GENERIC
},
- { PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP51_SATA2), GENERIC
},
- { PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_SATA), GENERIC
},
- { PCI_VDEVICE(NVIDIA
RE: [PATCH] ata: Add the SW NCQ support to sata_nv for MCP51/MCP55/MCP61
A pretty good way. I will modify my code.
-Original Message-
From: Andrew Morton [mailto:[EMAIL PROTECTED]
Sent: Wednesday, June 27, 2007 5:21 PM
To: Kuan Luo
Cc: linux-kernel@vger.kernel.org; [EMAIL PROTECTED]; Peer Chen
Subject: Re: [PATCH] ata: Add the SW NCQ support to sata_nv for
MCP51/MCP55/MCP61
On Wed, 27 Jun 2007 17:09:29 +0800 "Kuan Luo" <[EMAIL PROTECTED]> wrote:
> > +static void nv_swncq_qc_to_dq(struct ata_port *ap, struct
> ata_queued_cmd *qc)
> > +{
> > + struct nv_swncq_port_priv *pp = ap->private_data;
> > + defer_queue_t *dq = >defer_queue;
> > +
> > + /* queue is full */
> > + WARN_ON((dq->rear + 1) % (ATA_MAX_QUEUE + 1) == dq->front);
>
> >This is peculiar. The array is sized ATA_MAX_QUEUE+1 (ie: 33) and
the
> code
> >uses ATA_MAX_QUEUE+1 everywhere.
>
> >It looks to me like the ata code was designed to queue up to 32
> elements
> >and all this code has taken that to 33. What exactly is going on
here?
>
>
> The code is designed to contain 32 elements. But the position of rear
> doesn't point to
> a valid element to check whether the queue is full or null. If front
==
> rear , queue is null.
> if (rear + 1) % (ATA_MAX_QUEUE + 1) == front, queue is full.
> So the value of the array is 32 + 1.
that's the wrong way of doing a circular buffer..
It's better to allow the head and tail indices to wrap all the way
through
0x and only mask them when they are actually used for
subscripting.
That way:
add:
array[head++ & (ATA_MAX_QUEUE-1)] = item;
remove:
item = array[tail++ & (ATA_MAX_QUEUE-1)];
full:
head - tail == ATA_MAX_QUEUE
empty:
head == tail
number of items:
head - tail
This requires that the array size be a power of two.
---
This email message is for the sole use of the intended recipient(s) and may
contain
confidential information. Any unauthorized review, use, disclosure or
distribution
is prohibited. If you are not the intended recipient, please contact the
sender by
reply email and destroy all copies of the original message.
---
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] ata: Add the SW NCQ support to sata_nv for MCP51/MCP55/MCP61
> +static void nv_swncq_qc_to_dq(struct ata_port *ap, struct
ata_queued_cmd *qc)
> +{
> + struct nv_swncq_port_priv *pp = ap->private_data;
> + defer_queue_t *dq = >defer_queue;
> +
> + /* queue is full */
> + WARN_ON((dq->rear + 1) % (ATA_MAX_QUEUE + 1) == dq->front);
>This is peculiar. The array is sized ATA_MAX_QUEUE+1 (ie: 33) and the
code
>uses ATA_MAX_QUEUE+1 everywhere.
>It looks to me like the ata code was designed to queue up to 32
elements
>and all this code has taken that to 33. What exactly is going on here?
The code is designed to contain 32 elements. But the position of rear
doesn't point to
a valid element to check whether the queue is full or null. If front ==
rear , queue is null.
if (rear + 1) % (ATA_MAX_QUEUE + 1) == front, queue is full.
So the value of the array is 32 + 1.
> +static void nv_swncq_fill_sg(struct ata_queued_cmd *qc)
> +{
> + struct ata_port *ap = qc->ap;
> + struct scatterlist *sg;
> + unsigned int idx;
> +
> + struct nv_swncq_port_priv *pp = ap->private_data;
> +
> + struct ata_prd *prd;
> +
> + WARN_ON(qc->__sg == NULL);
> + WARN_ON(qc->n_elem == 0 && qc->pad_len == 0);
> +
> + prd = (void*)pp->prd + (ATA_PRD_TBL_SZ * qc->tag);
>Are you sure that should have been ATA_PRD_TBL_SZ and not ATA_PRD_SZ?
>Can this expression be done in a more type-friendly way?
Yes, i am sure.
I allocated prdt memory of 32 commands.
pp->prd = dmam_alloc_coherent(dev, ATA_PRD_TBL_SZ * ATA_MAX_QUEUE,
>prd_dma, GFP_KERNEL);
Maybe below way is more type-friendly.
prd = pp->prd + ATA_MAX_PRD * qc->tag;
Best Regards,
Kuan Luo
-Original Message-
From: Andrew Morton [mailto:[EMAIL PROTECTED]
Sent: Wednesday, June 27, 2007 1:27 PM
To: kuan luo
Cc: linux-kernel@vger.kernel.org; [EMAIL PROTECTED]; Peer Chen; Kuan Luo
Subject: Re: [PATCH] ata: Add the SW NCQ support to sata_nv for
MCP51/MCP55/MCP61
On Wed, 27 Jun 2007 11:04:44 +0800 "kuan luo" <[EMAIL PROTECTED]> wrote:
> Add the Software NCQ support to sata_nv.c for MCP51/MCP55/MCP61 SATA
controller.
> NCQ function is disable by default, you can enable it with 'swncq=1'
>
This patch adds a large amount of new trailing whitespace.
> ---
> diff -Nurp a/sata_nv.c b/sata_nv.c
> --- a/sata_nv.c 2007-06-13 10:15:07.0 -0400
> +++ b/sata_nv.c 2007-06-26 12:52:27.0 -0400
Please prepare patches in `pathc -p1' form.
> +typedef struct {
> + u32 defer_bits;
> + u8 front;
> + u8 rear;
> + unsigned inttag[ATA_MAX_QUEUE + 1];
> +}defer_queue_t;
Avoid adding typedefs.
> +static int swncq_enabled = 0;
Don't initialise static storage to zero: it needlessly increases the
vmlinux size.
> nv_hardreset, ata_std_postreset);
> }
>
> +static void nv_swncq_qc_to_dq(struct ata_port *ap, struct
ata_queued_cmd *qc)
> +{
> + struct nv_swncq_port_priv *pp = ap->private_data;
> + defer_queue_t *dq = >defer_queue;
> +
> + /* queue is full */
> + WARN_ON((dq->rear + 1) % (ATA_MAX_QUEUE + 1) == dq->front);
This is peculiar. The array is sized ATA_MAX_QUEUE+1 (ie: 33) and the
code
uses ATA_MAX_QUEUE+1 everywhere.
It looks to me like the ata code was designed to queue up to 32 elements
and all this code has taken that to 33. What exactly is going on here?
> +
> + dq->defer_bits |= (1 << qc->tag);
> +
> + dq->tag[dq->rear] = qc->tag;
> + dq->rear = (dq->rear + 1) % (ATA_MAX_QUEUE + 1);
> +
> +}
> +
> +static struct ata_queued_cmd *nv_swncq_qc_from_dq(struct ata_port
*ap)
> +{
> + struct nv_swncq_port_priv *pp = ap->private_data;
> + defer_queue_t *dq = >defer_queue;
> + unsigned int tag;
> +
> + if (dq->front == dq->rear) /* null queue */
> + return NULL;
> +
> + tag = dq->tag[dq->front];
> + dq->tag[dq->front] = ATA_TAG_POISON;
> + dq->front = (dq->front + 1) % (ATA_MAX_QUEUE + 1);
etc.
> + WARN_ON(!(dq->defer_bits & (1 << tag)));
> + dq->defer_bits &= ~(1 << tag);
> +
> + return ata_qc_from_tag(ap, tag);
> +}
> +
> + dq->front = dq->rear = 0;
> + dq->defer_bits = 0;
> + pp->qc_active = 0;
> + pp->last_issue_tag = ATA_TAG_POISON;
> + nv_swncq_fis_reinit(ap);
> +}
> +
> +static void nv_swncq_irq_clear(struct ata_port *ap, u32 val)
> +{
> + void __iomem *mmio = ap->host->iomap[NV_MMIO_BAR];
> + u32 flags = (val << (ap->port_no * NV_INT_
RE: [PATCH] ata: Add the SW NCQ support to sata_nv for MCP51/MCP55/MCP61
+static void nv_swncq_qc_to_dq(struct ata_port *ap, struct
ata_queued_cmd *qc)
+{
+ struct nv_swncq_port_priv *pp = ap-private_data;
+ defer_queue_t *dq = pp-defer_queue;
+
+ /* queue is full */
+ WARN_ON((dq-rear + 1) % (ATA_MAX_QUEUE + 1) == dq-front);
This is peculiar. The array is sized ATA_MAX_QUEUE+1 (ie: 33) and the
code
uses ATA_MAX_QUEUE+1 everywhere.
It looks to me like the ata code was designed to queue up to 32
elements
and all this code has taken that to 33. What exactly is going on here?
The code is designed to contain 32 elements. But the position of rear
doesn't point to
a valid element to check whether the queue is full or null. If front ==
rear , queue is null.
if (rear + 1) % (ATA_MAX_QUEUE + 1) == front, queue is full.
So the value of the array is 32 + 1.
+static void nv_swncq_fill_sg(struct ata_queued_cmd *qc)
+{
+ struct ata_port *ap = qc-ap;
+ struct scatterlist *sg;
+ unsigned int idx;
+
+ struct nv_swncq_port_priv *pp = ap-private_data;
+
+ struct ata_prd *prd;
+
+ WARN_ON(qc-__sg == NULL);
+ WARN_ON(qc-n_elem == 0 qc-pad_len == 0);
+
+ prd = (void*)pp-prd + (ATA_PRD_TBL_SZ * qc-tag);
Are you sure that should have been ATA_PRD_TBL_SZ and not ATA_PRD_SZ?
Can this expression be done in a more type-friendly way?
Yes, i am sure.
I allocated prdt memory of 32 commands.
pp-prd = dmam_alloc_coherent(dev, ATA_PRD_TBL_SZ * ATA_MAX_QUEUE,
pp-prd_dma, GFP_KERNEL);
Maybe below way is more type-friendly.
prd = pp-prd + ATA_MAX_PRD * qc-tag;
Best Regards,
Kuan Luo
-Original Message-
From: Andrew Morton [mailto:[EMAIL PROTECTED]
Sent: Wednesday, June 27, 2007 1:27 PM
To: kuan luo
Cc: linux-kernel@vger.kernel.org; [EMAIL PROTECTED]; Peer Chen; Kuan Luo
Subject: Re: [PATCH] ata: Add the SW NCQ support to sata_nv for
MCP51/MCP55/MCP61
On Wed, 27 Jun 2007 11:04:44 +0800 kuan luo [EMAIL PROTECTED] wrote:
Add the Software NCQ support to sata_nv.c for MCP51/MCP55/MCP61 SATA
controller.
NCQ function is disable by default, you can enable it with 'swncq=1'
This patch adds a large amount of new trailing whitespace.
---
diff -Nurp a/sata_nv.c b/sata_nv.c
--- a/sata_nv.c 2007-06-13 10:15:07.0 -0400
+++ b/sata_nv.c 2007-06-26 12:52:27.0 -0400
Please prepare patches in `pathc -p1' form.
+typedef struct {
+ u32 defer_bits;
+ u8 front;
+ u8 rear;
+ unsigned inttag[ATA_MAX_QUEUE + 1];
+}defer_queue_t;
Avoid adding typedefs.
+static int swncq_enabled = 0;
Don't initialise static storage to zero: it needlessly increases the
vmlinux size.
nv_hardreset, ata_std_postreset);
}
+static void nv_swncq_qc_to_dq(struct ata_port *ap, struct
ata_queued_cmd *qc)
+{
+ struct nv_swncq_port_priv *pp = ap-private_data;
+ defer_queue_t *dq = pp-defer_queue;
+
+ /* queue is full */
+ WARN_ON((dq-rear + 1) % (ATA_MAX_QUEUE + 1) == dq-front);
This is peculiar. The array is sized ATA_MAX_QUEUE+1 (ie: 33) and the
code
uses ATA_MAX_QUEUE+1 everywhere.
It looks to me like the ata code was designed to queue up to 32 elements
and all this code has taken that to 33. What exactly is going on here?
+
+ dq-defer_bits |= (1 qc-tag);
+
+ dq-tag[dq-rear] = qc-tag;
+ dq-rear = (dq-rear + 1) % (ATA_MAX_QUEUE + 1);
+
+}
+
+static struct ata_queued_cmd *nv_swncq_qc_from_dq(struct ata_port
*ap)
+{
+ struct nv_swncq_port_priv *pp = ap-private_data;
+ defer_queue_t *dq = pp-defer_queue;
+ unsigned int tag;
+
+ if (dq-front == dq-rear) /* null queue */
+ return NULL;
+
+ tag = dq-tag[dq-front];
+ dq-tag[dq-front] = ATA_TAG_POISON;
+ dq-front = (dq-front + 1) % (ATA_MAX_QUEUE + 1);
etc.
+ WARN_ON(!(dq-defer_bits (1 tag)));
+ dq-defer_bits = ~(1 tag);
+
+ return ata_qc_from_tag(ap, tag);
+}
+
+ dq-front = dq-rear = 0;
+ dq-defer_bits = 0;
+ pp-qc_active = 0;
+ pp-last_issue_tag = ATA_TAG_POISON;
+ nv_swncq_fis_reinit(ap);
+}
+
+static void nv_swncq_irq_clear(struct ata_port *ap, u32 val)
+{
+ void __iomem *mmio = ap-host-iomap[NV_MMIO_BAR];
+ u32 flags = (val (ap-port_no * NV_INT_PORT_SHIFT_MCP55));
I hope we'll never need to support more than two ports...
+ writel(flags, mmio + NV_INT_STATUS_MCP55);
+}
+
+static void nv_swncq_ncq_stop(struct ata_port *ap)
+{
+ struct nv_swncq_port_priv *pp = ap-private_data;
+ unsigned int i;
+ u32 sactive;
+ u32 done_mask;
+
+ ata_port_printk(ap, KERN_ERR,
+ EH in SWNCQ mode,QC:qc_active 0x%X sactive 0x%X\n,
+ ap-qc_active, ap-sactive);
+ ata_port_printk(ap, KERN_ERR,
+ SWNCQ:qc_active 0x%X defer_bits 0x%X last_issue_tag
0x%x\n
RE: [PATCH] ata: Add the SW NCQ support to sata_nv for MCP51/MCP55/MCP61
A pretty good way. I will modify my code.
-Original Message-
From: Andrew Morton [mailto:[EMAIL PROTECTED]
Sent: Wednesday, June 27, 2007 5:21 PM
To: Kuan Luo
Cc: linux-kernel@vger.kernel.org; [EMAIL PROTECTED]; Peer Chen
Subject: Re: [PATCH] ata: Add the SW NCQ support to sata_nv for
MCP51/MCP55/MCP61
On Wed, 27 Jun 2007 17:09:29 +0800 Kuan Luo [EMAIL PROTECTED] wrote:
+static void nv_swncq_qc_to_dq(struct ata_port *ap, struct
ata_queued_cmd *qc)
+{
+ struct nv_swncq_port_priv *pp = ap-private_data;
+ defer_queue_t *dq = pp-defer_queue;
+
+ /* queue is full */
+ WARN_ON((dq-rear + 1) % (ATA_MAX_QUEUE + 1) == dq-front);
This is peculiar. The array is sized ATA_MAX_QUEUE+1 (ie: 33) and
the
code
uses ATA_MAX_QUEUE+1 everywhere.
It looks to me like the ata code was designed to queue up to 32
elements
and all this code has taken that to 33. What exactly is going on
here?
The code is designed to contain 32 elements. But the position of rear
doesn't point to
a valid element to check whether the queue is full or null. If front
==
rear , queue is null.
if (rear + 1) % (ATA_MAX_QUEUE + 1) == front, queue is full.
So the value of the array is 32 + 1.
that's the wrong way of doing a circular buffer..
It's better to allow the head and tail indices to wrap all the way
through
0x and only mask them when they are actually used for
subscripting.
That way:
add:
array[head++ (ATA_MAX_QUEUE-1)] = item;
remove:
item = array[tail++ (ATA_MAX_QUEUE-1)];
full:
head - tail == ATA_MAX_QUEUE
empty:
head == tail
number of items:
head - tail
This requires that the array size be a power of two.
---
This email message is for the sole use of the intended recipient(s) and may
contain
confidential information. Any unauthorized review, use, disclosure or
distribution
is prohibited. If you are not the intended recipient, please contact the
sender by
reply email and destroy all copies of the original message.
---
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ata: Add the SW NCQ support to sata_nv for MCP51/MCP55/MCP61
Add the Software NCQ support to sata_nv.c for MCP51/MCP55/MCP61 SATA controller.
NCQ function is disable by default, you can enable it with 'swncq=1'
Signed-off-by: Kuan Luo <[EMAIL PROTECTED]>
Signed-off-by: Peer Chen <[EMAIL PROTECTED]>
---
diff -Nurp a/sata_nv.c b/sata_nv.c
--- a/sata_nv.c 2007-06-13 10:15:07.0 -0400
+++ b/sata_nv.c 2007-06-26 12:52:27.0 -0400
@@ -169,6 +169,35 @@ enum {
NV_ADMA_PORT_REGISTER_MODE = (1 << 0),
NV_ADMA_ATAPI_SETUP_COMPLETE= (1 << 1),
+ /* MCP55 reg offset */
+ NV_CTL_MCP55= 0x400,
+ NV_INT_STATUS_MCP55 = 0x440,
+ NV_INT_ENABLE_MCP55 = 0x444,
+ NV_NCQ_REG_MCP55= 0x448,
+
+ /* MCP55 */
+ NV_INT_ALL_MCP55= 0x,
+ NV_INT_PORT_SHIFT_MCP55 = 16, /* each port occupies 16 bits */
+ NV_INT_MASK_MCP55 = NV_INT_ALL_MCP55 & 0xfffd,
+
+ /* SWNCQ ENABLE BITS*/
+ NV_CTL_PRI_SWNCQ= 0x02,
+ NV_CTL_SEC_SWNCQ= 0x04,
+
+ /* SW NCQ status bits*/
+ NV_SWNCQ_IRQ_DEV= (1 << 0),
+ NV_SWNCQ_IRQ_PM = (1 << 1),
+ NV_SWNCQ_IRQ_ADDED = (1 << 2),
+ NV_SWNCQ_IRQ_REMOVED= (1 << 3),
+
+ NV_SWNCQ_IRQ_BACKOUT= (1 << 4),
+ NV_SWNCQ_IRQ_SDBFIS = (1 << 5),
+ NV_SWNCQ_IRQ_DHREGFIS = (1 << 6),
+ NV_SWNCQ_IRQ_DMASETUP = (1 << 7),
+
+ NV_SWNCQ_IRQ_HOTPLUG= NV_SWNCQ_IRQ_ADDED |
+ NV_SWNCQ_IRQ_REMOVED,
+
};
/* ADMA Physical Region Descriptor - one SG segment */
@@ -226,6 +255,35 @@ struct nv_host_priv {
unsigned long type;
};
+typedef struct {
+ u32 defer_bits;
+ u8 front;
+ u8 rear;
+ unsigned inttag[ATA_MAX_QUEUE + 1];
+}defer_queue_t;
+
+struct nv_swncq_port_priv {
+ struct ata_prd *prd;/* our SG list */
+ dma_addr_t prd_dma; /* and its DMA mapping */
+ void __iomem*sactive_block;
+ u32 qc_active;
+ unsigned intlast_issue_tag;
+ spinlock_t lock;
+ /* fifo loop queue to store deferral command */
+ defer_queue_t defer_queue;
+
+ /* for NCQ interrupt analysis */
+ u32 dhfis_bits;
+ u32 dmafis_bits;
+ u32 sdbfis_bits;
+
+ unsigned intncq_saw_d2h:1;
+ unsigned intncq_saw_dmas:1;
+ unsigned intncq_saw_sdb:1;
+ unsigned intncq_saw_backout:1;
+};
+
+
#define NV_ADMA_CHECK_INTR(GCTL, PORT) ((GCTL) & ( 1 << (19 + (12 * (PORT)
static int nv_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
@@ -263,13 +321,28 @@ static void nv_adma_host_stop(struct ata
static void nv_adma_post_internal_cmd(struct ata_queued_cmd *qc);
static void nv_adma_tf_read(struct ata_port *ap, struct ata_taskfile *tf);
+static void nv_mcp55_thaw(struct ata_port *ap);
+static void nv_mcp55_freeze(struct ata_port *ap);
+static void nv_swncq_error_handler(struct ata_port *ap);
+static int nv_swncq_port_start(struct ata_port *ap);
+static void nv_swncq_qc_prep(struct ata_queued_cmd *qc);
+static void nv_swncq_fill_sg(struct ata_queued_cmd *qc);
+static unsigned int nv_swncq_qc_issue(struct ata_queued_cmd *qc);
+static void nv_swncq_irq_clear(struct ata_port *ap, u32 val);
+static irqreturn_t nv_swncq_interrupt(int irq, void *dev_instance);
+#ifdef CONFIG_PM
+static int nv_swncq_port_suspend(struct ata_port *ap, pm_message_t mesg);
+static int nv_swncq_port_resume(struct ata_port *ap);
+#endif
+
enum nv_host_type
{
GENERIC,
NFORCE2,
NFORCE3 = NFORCE2, /* NF2 == NF3 as far as sata_nv is concerned */
CK804,
- ADMA
+ ADMA,
+ SWNCQ
};
static const struct pci_device_id nv_pci_tbl[] = {
@@ -280,13 +353,13 @@ static const struct pci_device_id nv_pci
{ PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_CK804_SATA2), CK804 },
{ PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP04_SATA), CK804 },
{ PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP04_SATA2), CK804 },
- { PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP51_SATA), GENERIC
},
- { PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP51_SATA2), GENERIC
},
- { PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_SATA), GENERIC
},
- { PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_SATA2), GENERIC
},
- { PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP61_SATA), GENERIC
},
- { PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP61_SATA2), GENERIC
},
- { PCI_VDEVICE(NVID
[PATCH] ata: Add the SW NCQ support to sata_nv for MCP51/MCP55/MCP61
Add the Software NCQ support to sata_nv.c for MCP51/MCP55/MCP61 SATA controller.
NCQ function is disable by default, you can enable it with 'swncq=1'
Signed-off-by: Kuan Luo [EMAIL PROTECTED]
Signed-off-by: Peer Chen [EMAIL PROTECTED]
---
diff -Nurp a/sata_nv.c b/sata_nv.c
--- a/sata_nv.c 2007-06-13 10:15:07.0 -0400
+++ b/sata_nv.c 2007-06-26 12:52:27.0 -0400
@@ -169,6 +169,35 @@ enum {
NV_ADMA_PORT_REGISTER_MODE = (1 0),
NV_ADMA_ATAPI_SETUP_COMPLETE= (1 1),
+ /* MCP55 reg offset */
+ NV_CTL_MCP55= 0x400,
+ NV_INT_STATUS_MCP55 = 0x440,
+ NV_INT_ENABLE_MCP55 = 0x444,
+ NV_NCQ_REG_MCP55= 0x448,
+
+ /* MCP55 */
+ NV_INT_ALL_MCP55= 0x,
+ NV_INT_PORT_SHIFT_MCP55 = 16, /* each port occupies 16 bits */
+ NV_INT_MASK_MCP55 = NV_INT_ALL_MCP55 0xfffd,
+
+ /* SWNCQ ENABLE BITS*/
+ NV_CTL_PRI_SWNCQ= 0x02,
+ NV_CTL_SEC_SWNCQ= 0x04,
+
+ /* SW NCQ status bits*/
+ NV_SWNCQ_IRQ_DEV= (1 0),
+ NV_SWNCQ_IRQ_PM = (1 1),
+ NV_SWNCQ_IRQ_ADDED = (1 2),
+ NV_SWNCQ_IRQ_REMOVED= (1 3),
+
+ NV_SWNCQ_IRQ_BACKOUT= (1 4),
+ NV_SWNCQ_IRQ_SDBFIS = (1 5),
+ NV_SWNCQ_IRQ_DHREGFIS = (1 6),
+ NV_SWNCQ_IRQ_DMASETUP = (1 7),
+
+ NV_SWNCQ_IRQ_HOTPLUG= NV_SWNCQ_IRQ_ADDED |
+ NV_SWNCQ_IRQ_REMOVED,
+
};
/* ADMA Physical Region Descriptor - one SG segment */
@@ -226,6 +255,35 @@ struct nv_host_priv {
unsigned long type;
};
+typedef struct {
+ u32 defer_bits;
+ u8 front;
+ u8 rear;
+ unsigned inttag[ATA_MAX_QUEUE + 1];
+}defer_queue_t;
+
+struct nv_swncq_port_priv {
+ struct ata_prd *prd;/* our SG list */
+ dma_addr_t prd_dma; /* and its DMA mapping */
+ void __iomem*sactive_block;
+ u32 qc_active;
+ unsigned intlast_issue_tag;
+ spinlock_t lock;
+ /* fifo loop queue to store deferral command */
+ defer_queue_t defer_queue;
+
+ /* for NCQ interrupt analysis */
+ u32 dhfis_bits;
+ u32 dmafis_bits;
+ u32 sdbfis_bits;
+
+ unsigned intncq_saw_d2h:1;
+ unsigned intncq_saw_dmas:1;
+ unsigned intncq_saw_sdb:1;
+ unsigned intncq_saw_backout:1;
+};
+
+
#define NV_ADMA_CHECK_INTR(GCTL, PORT) ((GCTL) ( 1 (19 + (12 * (PORT)
static int nv_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
@@ -263,13 +321,28 @@ static void nv_adma_host_stop(struct ata
static void nv_adma_post_internal_cmd(struct ata_queued_cmd *qc);
static void nv_adma_tf_read(struct ata_port *ap, struct ata_taskfile *tf);
+static void nv_mcp55_thaw(struct ata_port *ap);
+static void nv_mcp55_freeze(struct ata_port *ap);
+static void nv_swncq_error_handler(struct ata_port *ap);
+static int nv_swncq_port_start(struct ata_port *ap);
+static void nv_swncq_qc_prep(struct ata_queued_cmd *qc);
+static void nv_swncq_fill_sg(struct ata_queued_cmd *qc);
+static unsigned int nv_swncq_qc_issue(struct ata_queued_cmd *qc);
+static void nv_swncq_irq_clear(struct ata_port *ap, u32 val);
+static irqreturn_t nv_swncq_interrupt(int irq, void *dev_instance);
+#ifdef CONFIG_PM
+static int nv_swncq_port_suspend(struct ata_port *ap, pm_message_t mesg);
+static int nv_swncq_port_resume(struct ata_port *ap);
+#endif
+
enum nv_host_type
{
GENERIC,
NFORCE2,
NFORCE3 = NFORCE2, /* NF2 == NF3 as far as sata_nv is concerned */
CK804,
- ADMA
+ ADMA,
+ SWNCQ
};
static const struct pci_device_id nv_pci_tbl[] = {
@@ -280,13 +353,13 @@ static const struct pci_device_id nv_pci
{ PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_CK804_SATA2), CK804 },
{ PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP04_SATA), CK804 },
{ PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP04_SATA2), CK804 },
- { PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP51_SATA), GENERIC
},
- { PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP51_SATA2), GENERIC
},
- { PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_SATA), GENERIC
},
- { PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_SATA2), GENERIC
},
- { PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP61_SATA), GENERIC
},
- { PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP61_SATA2), GENERIC
},
- { PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP61_SATA3), GENERIC
},
+ { PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP51_SATA), SWNCQ
RE: [PATCH] drivers/ata: Add the SW NCQ support to sata_nv for MCP51/MCP55/MCP61
Thanks for your comment, see the explaination inline.
We'll apply your advice in later patch.
-Original Message-
From: Robert Hancock [mailto:[EMAIL PROTECTED]
Sent: Friday, May 18, 2007 9:48 AM
To: Peer Chen
Cc: linux-kernel@vger.kernel.org; Jeff Garzik;
[EMAIL PROTECTED]; Kuan Luo; [EMAIL PROTECTED]
Subject: Re: [PATCH] drivers/ata: Add the SW NCQ support to sata_nv for
MCP51/MCP55/MCP61
Peer Chen wrote:
> Add the Software NCQ support to sata_nv.c for MCP51/MCP55/MCP61 SATA
> controller.
>
> This patch base on sata_nv.c file from kernel 2.6.22-rc1
>
> See attachment for the patch.
>
> Signed-off-by: Kuan Luo <[EMAIL PROTECTED]>
> Signed-off-by: Peer Chen <[EMAIL PROTECTED]>
Good to finally see this come out. I've pasted the code below (indented)
in order to make some comments:
--- linux-2.6.22-rc1/drivers/ata/sata_nv.c.orig 2007-05-17
14:48:26.0 -0400
+++ linux-2.6.22-rc1/drivers/ata/sata_nv.c 2007-05-17
17:07:28.0 -0400
@@ -46,6 +46,8 @@
#include
#include
#include
+#include
+#include
#include
#define DRV_NAME "sata_nv"
@@ -169,6 +171,36 @@ enum {
NV_ADMA_PORT_REGISTER_MODE = (1 << 0),
NV_ADMA_ATAPI_SETUP_COMPLETE= (1 << 1),
+ /* MCP55 reg offset */
+ NV_CTL_MCP55= 0x400,
+ NV_INT_STATUS_MCP55 = 0x440,
+ NV_INT_ENABLE_MCP55 = 0x444,
+ NV_NCQ_REG_MCP55= 0x448,
+ NV_CH1_SACTIVE_MCP55= 0x0C,
+
+ /* MCP55 */
+ NV_INT_ALL_MCP55= 0x,
+ NV_INT_PORT_SHIFT_MCP55 = 16, /* each port
occupies 16 bits */
+ NV_INT_MASK_MCP55 = NV_INT_ALL_MCP55 &
0xfffd,
+
+ /* NCQ ENABLE BITS*/
+ NV_CTL_PRI_SWNCQ= 0x02,
+ NV_CTL_SEC_SWNCQ= 0x04,
+
+ /* MCP55 status bits*/
+ NV_INT_DEV_MCP55= 0x01,
+ NV_INT_PM_MCP55 = 0x02,
+ NV_INT_ADDED_MCP55 = 0x04,
+ NV_INT_REMOVED_MCP55= 0x08,
+
+ NV_INT_BACKOUT_MCP55= 0x10,
+ NV_INT_SDBFIS_MCP55 = 0x20,
+ NV_INT_DHREGFIS_MCP55 = 0x40,
+ NV_INT_DMASETUP_MCP55 = 0x80,
+
+ NV_INT_HOTPLUG_MCP55= (NV_INT_ADDED_MCP55 |
+
NV_INT_REMOVED_MCP55),
+
};
/* ADMA Physical Region Descriptor - one SG segment */
@@ -264,13 +296,118 @@ static void nv_adma_host_stop(struct ata
static void nv_adma_post_internal_cmd(struct ata_queued_cmd
*qc);
static void nv_adma_tf_read(struct ata_port *ap, struct
ata_taskfile *tf);
+static void ncq_error_handler(struct ata_port *ap);
+static void nv_mcp55_thaw(struct ata_port *ap);
+static void nv_mcp55_freeze(struct ata_port *ap);
+static void ncq_host_init(struct ata_host *host);
+static void nv_bmdma_stop(struct ata_port *ap);
+static int nv_std_qc_defer(struct ata_port *ap);
+static int nv_port_start(struct ata_port *ap);
+static void nv_port_stop(struct ata_port *ap);
+static void ncq_clear(struct ata_port *ap);
+static void nv_qc_prep(struct ata_queued_cmd *qc);
+static void nv_fill_sg(struct ata_queued_cmd *qc);
+static void ncq_sactive_start (struct ata_queued_cmd *qc);
+static u32 ncq_sactive_value (struct ata_port *ap);
+static unsigned int nv_qc_issue_prot(struct ata_queued_cmd
*qc);
+static u32 ncq_tag_value(struct ata_port *ap);
+static int nv_ncqintr_sdbfis(struct ata_port *ap);
+static int nv_ncqintr_dmasetupfis(struct ata_port *ap);
+static void ncq_clear_singlefis(struct ata_port *ap, u32 val);
+static u32 ncq_ownfisintr_value (struct ata_port *ap);
+void ncq_hotplug(struct ata_port *ap, u32 fis);
+static irqreturn_t nv_mcp55_interrupt(int irq, void
*dev_instance);
+static int ncq_interrupt(struct ata_port *ap, u32 fis);
+static int nv_scsi_queuecmd(struct scsi_cmnd *cmd,
+ void (*done)(struct scsi_cmnd *));
These functions should use "mcp51" or "swncq" or something in the name
instead of "ncq", since the latter implies it may be related to ADMA as
well.
[kuan]:
I will use a better name for these function.
+
+#undef NCQ_DEBUG
+#undef NCQ_VERBOSE_DEBUG
+#ifdef NCQ_DEBUG
+#define NPRINTK(fmt,
RE: [PATCH] drivers/ata: Add the SW NCQ support to sata_nv for MCP51/MCP55/MCP61
Thanks for your comment, see the explaination inline.
We'll apply your advice in later patch.
-Original Message-
From: Robert Hancock [mailto:[EMAIL PROTECTED]
Sent: Friday, May 18, 2007 9:48 AM
To: Peer Chen
Cc: linux-kernel@vger.kernel.org; Jeff Garzik;
[EMAIL PROTECTED]; Kuan Luo; [EMAIL PROTECTED]
Subject: Re: [PATCH] drivers/ata: Add the SW NCQ support to sata_nv for
MCP51/MCP55/MCP61
Peer Chen wrote:
Add the Software NCQ support to sata_nv.c for MCP51/MCP55/MCP61 SATA
controller.
This patch base on sata_nv.c file from kernel 2.6.22-rc1
See attachment for the patch.
Signed-off-by: Kuan Luo [EMAIL PROTECTED]
Signed-off-by: Peer Chen [EMAIL PROTECTED]
Good to finally see this come out. I've pasted the code below (indented)
in order to make some comments:
--- linux-2.6.22-rc1/drivers/ata/sata_nv.c.orig 2007-05-17
14:48:26.0 -0400
+++ linux-2.6.22-rc1/drivers/ata/sata_nv.c 2007-05-17
17:07:28.0 -0400
@@ -46,6 +46,8 @@
#include linux/device.h
#include scsi/scsi_host.h
#include scsi/scsi_device.h
+#include scsi/scsi.h
+#include scsi/scsi_cmnd.h
#include linux/libata.h
#define DRV_NAME sata_nv
@@ -169,6 +171,36 @@ enum {
NV_ADMA_PORT_REGISTER_MODE = (1 0),
NV_ADMA_ATAPI_SETUP_COMPLETE= (1 1),
+ /* MCP55 reg offset */
+ NV_CTL_MCP55= 0x400,
+ NV_INT_STATUS_MCP55 = 0x440,
+ NV_INT_ENABLE_MCP55 = 0x444,
+ NV_NCQ_REG_MCP55= 0x448,
+ NV_CH1_SACTIVE_MCP55= 0x0C,
+
+ /* MCP55 */
+ NV_INT_ALL_MCP55= 0x,
+ NV_INT_PORT_SHIFT_MCP55 = 16, /* each port
occupies 16 bits */
+ NV_INT_MASK_MCP55 = NV_INT_ALL_MCP55
0xfffd,
+
+ /* NCQ ENABLE BITS*/
+ NV_CTL_PRI_SWNCQ= 0x02,
+ NV_CTL_SEC_SWNCQ= 0x04,
+
+ /* MCP55 status bits*/
+ NV_INT_DEV_MCP55= 0x01,
+ NV_INT_PM_MCP55 = 0x02,
+ NV_INT_ADDED_MCP55 = 0x04,
+ NV_INT_REMOVED_MCP55= 0x08,
+
+ NV_INT_BACKOUT_MCP55= 0x10,
+ NV_INT_SDBFIS_MCP55 = 0x20,
+ NV_INT_DHREGFIS_MCP55 = 0x40,
+ NV_INT_DMASETUP_MCP55 = 0x80,
+
+ NV_INT_HOTPLUG_MCP55= (NV_INT_ADDED_MCP55 |
+
NV_INT_REMOVED_MCP55),
+
};
/* ADMA Physical Region Descriptor - one SG segment */
@@ -264,13 +296,118 @@ static void nv_adma_host_stop(struct ata
static void nv_adma_post_internal_cmd(struct ata_queued_cmd
*qc);
static void nv_adma_tf_read(struct ata_port *ap, struct
ata_taskfile *tf);
+static void ncq_error_handler(struct ata_port *ap);
+static void nv_mcp55_thaw(struct ata_port *ap);
+static void nv_mcp55_freeze(struct ata_port *ap);
+static void ncq_host_init(struct ata_host *host);
+static void nv_bmdma_stop(struct ata_port *ap);
+static int nv_std_qc_defer(struct ata_port *ap);
+static int nv_port_start(struct ata_port *ap);
+static void nv_port_stop(struct ata_port *ap);
+static void ncq_clear(struct ata_port *ap);
+static void nv_qc_prep(struct ata_queued_cmd *qc);
+static void nv_fill_sg(struct ata_queued_cmd *qc);
+static void ncq_sactive_start (struct ata_queued_cmd *qc);
+static u32 ncq_sactive_value (struct ata_port *ap);
+static unsigned int nv_qc_issue_prot(struct ata_queued_cmd
*qc);
+static u32 ncq_tag_value(struct ata_port *ap);
+static int nv_ncqintr_sdbfis(struct ata_port *ap);
+static int nv_ncqintr_dmasetupfis(struct ata_port *ap);
+static void ncq_clear_singlefis(struct ata_port *ap, u32 val);
+static u32 ncq_ownfisintr_value (struct ata_port *ap);
+void ncq_hotplug(struct ata_port *ap, u32 fis);
+static irqreturn_t nv_mcp55_interrupt(int irq, void
*dev_instance);
+static int ncq_interrupt(struct ata_port *ap, u32 fis);
+static int nv_scsi_queuecmd(struct scsi_cmnd *cmd,
+ void (*done)(struct scsi_cmnd *));
These functions should use mcp51 or swncq or something in the name
instead of ncq, since the latter implies it may be related to ADMA as
well.
[kuan]:
I will use a better name for these function.
+
+#undef NCQ_DEBUG
+#undef NCQ_VERBOSE_DEBUG
+#ifdef NCQ_DEBUG
+#define NPRINTK(fmt, args...) printk(KERN_ERR %s: fmt
