Good news on the si3124 driver bug I mentioned:
http://mail.opensolaris.org/pipermail/zfs-discuss/2007-May/028487.html
http://mail.opensolaris.org/pipermail/storage-discuss/2007-May/001288.html
Roger Fujii, a long time contributor to the Solaris on x86 list
(solarisx86 at yahoogroups.com), root caused the bug, devised and tested
a fix, and passed the required changes upstream.
I got his permission to forward our email exchange to this list.
PS: will send a separate copy of this email to storage-discuss.
Al Hopper Logical Approach Inc, Plano, TX. al at logical-approach.com
Voice: 972.379.2133 Fax: 972.379.2134 Timezone: US CDT
OpenSolaris Governing Board (OGB) Member - Apr 2005 to Mar 2007
http://www.opensolaris.org/os/community/ogb/ogb_2005-2007/
---------- Forwarded message ----------
Date: Wed, 06 Jun 2007 12:53:03 -0400
From: Roger Fujii <rmf at lookhere.com>
To: Al Hopper <al at logical-approach.com>
Subject: Re: [solarisx86] addonics / si3124 controller problem and fix
Al Hopper wrote:> On Wed, 6 Jun 2007, rogerfujii wrote:
>
>
> Excellent news Roger. Can you email me details of the fix?
it''s been assigned CR 6566207 by Linda Bernal. Basically, if you look
at si_intr and read the comments in the code, the bug is pretty
obvious.
> Are you working through the OpenSolaris contributor process to
> integrate the fix (if so can you send me a pointer to the case
> materials)?
Here''s what I wrote up:
si3124 driver''s interrupt routine is incorrectly coded. The ddi_put32
that clears the interrupts should be enclosed in an "else" block,
thereby making it consistent with the comment just below. Otherwise,
you would be double clearing the interrupts, thus losing pending
interrupts.
Since this is a simple fix, there''s really no point dealing it as a
contributor. There also another initialization "bug" I encountered (CR
6565894) which fixed a problem where the drives just didn''t show up
after a reboot.
> PS: how did you find the bug?
I was determined to add drives to my system. :) Luckily,
SiliconImage has the docs online, plus the code is (relatively) well
commented.
-r
Marko Milisavljevic
2007-Jun-07 17:49 UTC
[zfs-discuss] si3124 controller problem and fix (fwd)
Al, Do you know if this affects Sil3132 cards as well? According to this http://www.dansketcher.com/2006/12/09/solaris-sata-chipsets-i-can-run-zsf-with/Sil3124 driver can be persuaded to work with Sil3132 cards with change in /etc/driver_aliases. Marko On 6/7/07, Al Hopper <al at logical-approach.com> wrote:> > > Good news on the si3124 driver bug I mentioned: > > http://mail.opensolaris.org/pipermail/zfs-discuss/2007-May/028487.html > http://mail.opensolaris.org/pipermail/storage-discuss/2007-May/001288.html > > Roger Fujii, a long time contributor to the Solaris on x86 list > (solarisx86 at yahoogroups.com), root caused the bug, devised and tested > a fix, and passed the required changes upstream. > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://mail.opensolaris.org/pipermail/zfs-discuss/attachments/20070607/146787b8/attachment.html>
johansen-osdev at sun.com
2007-Jun-07 18:00 UTC
[zfs-discuss] si3124 controller problem and fix (fwd)
> it''s been assigned CR 6566207 by Linda Bernal. Basically, if you look > at si_intr and read the comments in the code, the bug is pretty > obvious. > > si3124 driver''s interrupt routine is incorrectly coded. The ddi_put32 > that clears the interrupts should be enclosed in an "else" block, > thereby making it consistent with the comment just below. Otherwise, > you would be double clearing the interrupts, thus losing pending > interrupts. > > Since this is a simple fix, there''s really no point dealing it as a > contributor.The bug report for 6566207 states that the submitter is an OpenSolaris contributor who wishes to work on the fix. If this is not the case, we should clarify this CR so it doesn''t languish. It''s still sitting in the dispatched state (hasn''t been accepted by anyone). -j
On Thu, 7 Jun 2007, Marko Milisavljevic wrote:> Al, > > Do you know if this affects Sil3132 cards as well? According to thisI''m sure it would affect any hardware that uses the driver - because it''ll drop interrupts from the device. But I have not verified this to be the case.> http://www.dansketcher.com/2006/12/09/solaris-sata-chipsets-i-can-run-zsf-with/Sil3124 > driver can be persuaded to work with Sil3132 cards with change in > /etc/driver_aliases.Regards, Al Hopper Logical Approach Inc, Plano, TX. al at logical-approach.com Voice: 972.379.2133 Fax: 972.379.2134 Timezone: US CDT OpenSolaris Governing Board (OGB) Member - Apr 2005 to Mar 2007 http://www.opensolaris.org/os/community/ogb/ogb_2005-2007/
johansen-osdev at sun.com wrote:>> it''s been assigned CR 6566207 by Linda Bernal. Basically, if you look >> at si_intr and read the comments in the code, the bug is pretty >> obvious. >> >> si3124 driver''s interrupt routine is incorrectly coded. The ddi_put32 >> that clears the interrupts should be enclosed in an "else" block, >> thereby making it consistent with the comment just below. Otherwise, >> you would be double clearing the interrupts, thus losing pending >> interrupts. >> >> Since this is a simple fix, there''s really no point dealing it as a >> contributor. > > The bug report for 6566207 states that the submitter is an OpenSolaris > contributor who wishes to work on the fix. If this is not the case, we > should clarify this CR so it doesn''t languish. It''s still sitting in > the dispatched state (hasn''t been accepted by anyone). >well, it *just* got filed at 10PM on the 5th... but I''ve raised its priority and added the SATA team to the notification. I''ll follow up with private email pointing out the urgency of the fix.
Does anyone have an update on this bugfix? I''m trying to use some 3124 cards in production, and its painful! Thanks, Murray This message posted from opensolaris.org
You can see the status of bug here: http://bugs.opensolaris.org/view_bug.do?bug_id=6566207 Unfortunately, it''s showing no progress since 20th June. This fix really could do to be in place for S10u4 and snv_70. Thanks Nigel Smith This message posted from opensolaris.org
On Tue, 17 Jul 2007, Nigel Smith wrote:> You can see the status of bug here: > > http://bugs.opensolaris.org/view_bug.do?bug_id=6566207 > > Unfortunately, it''s showing no progress since 20th June. > > This fix really could do to be in place for S10u4 and snv_70. >Drop Roger Fujii <rmf at lookhere.com> an email and ask him for a copy of the driver he has fixed. Tell him I sent you (he may get mad at me!). Roger is a long time contributor to the Solaris on x86 list at solarisx86.yahoogroups.com. Others x86ers have reported solid performance with Rogers updated driver. Regards, Al Hopper Logical Approach Inc, Plano, TX. al at logical-approach.com Voice: 972.379.2133 Fax: 972.379.2134 Timezone: US CDT OpenSolaris Governing Board (OGB) Member - Apr 2005 to Mar 2007 http://www.opensolaris.org/os/community/ogb/ogb_2005-2007/
johansen-osdev at sun.com
2007-Jul-18 00:17 UTC
[zfs-discuss] si3124 controller problem and fix (fwd)
In an attempt to speed up progress on some of the si3124 bugs that Roger reported, I''ve created a workspace with the fixes for: 6565894 sata drives are not identified by si3124 driver 6566207 si3124 driver loses interrupts. I''m attaching a driver which contains these fixes as well as a diff of the changes I used to produce them. I don''t have access to a si3124 chipset, unfortunately. Would somebody be able to review these changes and try the new driver on a si3124 card? Thanks, -j On Tue, Jul 17, 2007 at 02:39:00AM -0700, Nigel Smith wrote:> You can see the status of bug here: > > http://bugs.opensolaris.org/view_bug.do?bug_id=6566207 > > Unfortunately, it''s showing no progress since 20th June. > > This fix really could do to be in place for S10u4 and snv_70. > Thanks > Nigel Smith > > > This message posted from opensolaris.org > _______________________________________________ > zfs-discuss mailing list > zfs-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/zfs-discuss-------------- next part -------------- A non-text attachment was scrubbed... Name: si3124.tar.gz Type: application/x-tar-gz Size: 36678 bytes Desc: not available URL: <http://mail.opensolaris.org/pipermail/zfs-discuss/attachments/20070717/8bf82263/attachment.bin> -------------- next part -------------- ------- usr/src/uts/common/io/sata/adapters/si3124/si3124.c ------- Index: usr/src/uts/common/io/sata/adapters/si3124/si3124.c --- /ws/onnv-clone/usr/src/uts/common/io/sata/adapters/si3124/si3124.c Mon Nov 13 23:20:01 2006 +++ /export/johansen/si-fixes/usr/src/uts/common/io/sata/adapters/si3124/si3124.c Tue Jul 17 14:37:17 2007 @@ -22,11 +22,11 @@ /* * Copyright 2006 Sun Microsystems, Inc. All rights reserved. * Use is subject to license terms. */ -#pragma ident "@(#)si3124.c 1.4 06/11/14 SMI" +#pragma ident "@(#)si3124.c 1.5 07/07/17 SMI" /* * SiliconImage 3124/3132 sata controller driver @@ -381,11 +381,11 @@ extern struct mod_ops mod_driverops; static struct modldrv modldrv = { &mod_driverops, /* driverops */ - "si3124 driver v1.4", + "si3124 driver v1.5", &sictl_dev_ops, /* driver ops */ }; static struct modlinkage modlinkage = { MODREV_1, @@ -2808,10 +2808,13 @@ si_portp = si_ctlp->sictl_ports[port]; mutex_enter(&si_portp->siport_mutex); /* Clear Port Reset. */ ddi_put32(si_ctlp->sictl_port_acc_handle, + (uint32_t *)PORT_CONTROL_SET(si_ctlp, port), + PORT_CONTROL_SET_BITS_PORT_RESET); + ddi_put32(si_ctlp->sictl_port_acc_handle, (uint32_t *)PORT_CONTROL_CLEAR(si_ctlp, port), PORT_CONTROL_CLEAR_BITS_PORT_RESET); /* * Arm the interrupts for: Cmd completion, Cmd error, @@ -3509,16 +3512,16 @@ port); if (port_intr_status & INTR_COMMAND_COMPLETE) { (void) si_intr_command_complete(si_ctlp, si_portp, port); - } - + } else { /* Clear the interrupts */ ddi_put32(si_ctlp->sictl_port_acc_handle, (uint32_t *)(PORT_INTERRUPT_STATUS(si_ctlp, port)), port_intr_status & INTR_MASK); + } /* * Note that we did not clear the interrupt for command * completion interrupt. Reading of slot_status takes care * of clearing the interrupt for command completion case.
Hey Johansen, I can confirm that 6566207 has been fixed by the new driver! I haven''t been able to reproduce 6565894 so cannot confirm whether or not this is fixed. Thanks for the fix, open source rocks :) Murray This message posted from opensolaris.org