LinuxLists.cc - [PATCH] resubmitting sym53c8xx patches

2001-10-17 02:07:24

Subject: [PATCH] resubmitting sym53c8xx patches

diff -ruN dist-2.4.12+patches/drivers/scsi/sym53c8xx.c cvs-2.4.12+patches/drivers/scsi/sym53c8xx.c
--- dist-2.4.12+patches/drivers/scsi/sym53c8xx.c Mon Oct 15 10:22:43 2001
+++ cvs-2.4.12+patches/drivers/scsi/sym53c8xx.c Mon Oct 15 10:22:42 2001
@@ -111,6 +111,8 @@
#elif LINUX_VERSION_CODE >= LinuxVersionCode(2,1,93)
#include <asm/spinlock.h>
#endif
+#include <linux/notifier.h>
+#include <linux/reboot.h>
#include <linux/delay.h>
#include <linux/signal.h>
#include <linux/sched.h>
@@ -635,8 +637,11 @@
#if LINUX_VERSION_CODE >= LinuxVersionCode(2,1,93)

spinlock_t sym53c8xx_lock = SPIN_LOCK_UNLOCKED;
+spinlock_t sym53c8xx_host_lock = SPIN_LOCK_UNLOCKED;
#define NCR_LOCK_DRIVER(flags) spin_lock_irqsave(&sym53c8xx_lock, flags)
#define NCR_UNLOCK_DRIVER(flags) spin_unlock_irqrestore(&sym53c8xx_lock,flags)
+#define NCR_LOCK_HOSTS(flags) spin_lock_irqsave(&sym53c8xx_host_lock, flags)
+#define NCR_UNLOCK_HOSTS(flags) spin_unlock_irqrestore(&sym53c8xx_host_lock,flags)

#define NCR_INIT_LOCK_NCB(np) spin_lock_init(&np->smp_lock);
#define NCR_LOCK_NCB(np, flags) spin_lock_irqsave(&np->smp_lock, flags)
@@ -651,6 +656,8 @@

#define NCR_LOCK_DRIVER(flags) do { save_flags(flags); cli(); } while (0)
#define NCR_UNLOCK_DRIVER(flags) do { restore_flags(flags); } while (0)
+#define NCR_LOCK_HOSTS(flags) do { save_flags(flags); cli(); } while (0)
+#define NCR_UNLOCK_HOSTS(flags) do { restore_flags(flags); } while (0)

#define NCR_INIT_LOCK_NCB(np) do { } while (0)
#define NCR_LOCK_NCB(np, flags) do { save_flags(flags); cli(); } while (0)
@@ -696,7 +703,7 @@
return page_remapped? (page_remapped + page_offs) : 0UL;
}

-static void __init unmap_pci_mem(u_long vaddr, u_long size)
+static void unmap_pci_mem(u_long vaddr, u_long size)
{
if (vaddr)
iounmap((void *) (vaddr & PAGE_MASK));
@@ -1302,6 +1309,15 @@
int length, int hostno, int func);
#endif

+/*
+** reset the scsi controller on a "reboot." it should be way at the
+** end.
+*/
+static int sym53c8xx_halt(struct notifier_block *, ulong, void *);
+static struct notifier_block sym53c8xx_notifier = {
+ sym53c8xx_halt, NULL, 0
+};
+
/*
** Driver setup.
**
@@ -2250,7 +2266,6 @@
**----------------------------------------------------------------
*/
struct usrcmd user; /* Command from user */
- volatile u_char release_stage; /* Synchronisation stage on release */

/*----------------------------------------------------------------
** Fields that are used (primarily) for integrity check
@@ -5869,7 +5884,12 @@
** start the timeout daemon
*/
np->lasttime=0;
- ncr_timeout (np);
+#ifdef SCSI_NCR_PCIQ_BROKEN_INTR
+ np->timer.expires = ktime_get((HZ+9)/10);
+#else
+ np->timer.expires = ktime_get(SCSI_NCR_TIMER_INTERVAL);
+#endif
+ add_timer(&np->timer);

/*
** use SIMPLE TAG messages by default
@@ -7232,23 +7252,14 @@
**==========================================================
*/

-#ifdef MODULE
static int ncr_detach(ncb_p np)
{
- int i;
-
printk("%s: detaching ...\n", ncr_name(np));

/*
** Stop the ncr_timeout process
-** Set release_stage to 1 and wait that ncr_timeout() set it to 2.
*/
- np->release_stage = 1;
- for (i = 50 ; i && np->release_stage != 2 ; i--) MDELAY (100);
- if (np->release_stage != 2)
- printk("%s: the timer seems to be already stopped\n",
- ncr_name(np));
- else np->release_stage = 2;
+ del_timer_sync(&np->timer);

/*
** Reset NCR chip.
@@ -7278,7 +7289,6 @@

return 1;
}
-#endif

/*==========================================================
**
@@ -8605,23 +8615,11 @@
{
u_long thistime = ktime_get(0);

- /*
- ** If release process in progress, let's go
- ** Set the release stage from 1 to 2 to synchronize
- ** with the release process.
- */
-
- if (np->release_stage) {
- if (np->release_stage == 1) np->release_stage = 2;
- return;
- }
-
#ifdef SCSI_NCR_PCIQ_BROKEN_INTR
- np->timer.expires = ktime_get((HZ+9)/10);
+ mod_timer(&np->timer, ktime_get((HZ+9)/10));
#else
- np->timer.expires = ktime_get(SCSI_NCR_TIMER_INTERVAL);
+ mod_timer(&np->timer, ktime_get(SCSI_NCR_TIMER_INTERVAL));
#endif
- add_timer(&np->timer);

/*
** If we are resetting the ncr, wait for settle_time before
@@ -13070,7 +13075,8 @@
}

m_free(devtbl, PAGE_SIZE, "devtbl");
-
+ if (attach_count)
+ register_reboot_notifier(&sym53c8xx_notifier);
return attach_count;
}

@@ -13802,19 +13808,49 @@
}

+static int sym53c8xx_halt(struct notifier_block *block, ulong event, void *buf)
+{
+ unsigned long flags;
+ struct Scsi_Host *host, *next;
+
+ switch (event) {
+ case SYS_RESTART:
+ case SYS_HALT:
+ case SYS_POWER_OFF:
+ break;
+ default:
+ return NOTIFY_DONE;
+ }
+
+ printk(KERN_INFO "resetting sym53c8xx scsi bus(es)\n");
+ NCR_LOCK_HOSTS(flags);
+ next = first_host;
+ while ((host = next)) {
+ struct host_data *data = (struct host_data *)host->hostdata;
+
+ next = host->next;
+ if (data && data->ncb)
+ ncr_detach(data->ncb);
+ }
+ NCR_UNLOCK_HOSTS(flags);
+
+ return NOTIFY_OK;
+}
+
#ifdef MODULE
int sym53c8xx_release(struct Scsi_Host *host)
{
#ifdef DEBUG_SYM53C8XX
printk("sym53c8xx : release\n");
#endif
+ if (first_host)
+ unregister_reboot_notifier(&sym53c8xx_notifier);
ncr_detach(((struct host_data *) host->hostdata)->ncb);

return 1;
}
#endif

-
/*
** Scsi command waiting list management.
**
@@ -14209,13 +14245,15 @@
{
struct Scsi_Host *host;
struct host_data *host_data;
+ unsigned long flags;
ncb_p ncb = 0;
- int retv;
+ int retv = -EINVAL;

#ifdef DEBUG_PROC_INFO
printk("sym53c8xx_proc_info: hostno=%d, func=%d\n", hostno, func);
#endif

+ NCR_LOCK_HOSTS(flags);
for (host = first_host; host; host = host->next) {
if (host->hostt != first_host->hostt)
continue;
@@ -14227,25 +14265,22 @@
}

if (!ncb)
- return -EINVAL;
+ goto proc_done;

if (func) {
#ifdef SCSI_NCR_USER_COMMAND_SUPPORT
retv = ncr_user_command(ncb, buffer, length);
-#else
- retv = -EINVAL;
#endif
- }
- else {
+ } else {
if (start)
*start = buffer;
#ifdef SCSI_NCR_USER_INFO_SUPPORT
retv = ncr_host_info(ncb, buffer, offset, length);
-#else
- retv = -EINVAL;
#endif
}
+proc_done:
+ NCR_UNLOCK_HOSTS(flags);

return retv;
}

Attachments:

sym53c8xx.diff (6.11 kB)

2001-10-17 18:41:03

by Gérard Roudier

[permalink] [raw]

Subject: Re: [PATCH] resubmitting sym53c8xx patches

On Tue, 16 Oct 2001, Tim Hockin wrote:

> All,
>
> I've submitted this patch a few times, and had it OKed each time, but it
> hasn't made it in.
>
> it does:
> * cleanup timer handling
> * spin lock host list
> * adds a reboot handler
> * remove __init from a function called from a non __init (needed for reboot
> handler)
>
>
> Please apply this for the next 2.4.x.

As long as the kernel does not look trustable to me, I avoid to send
driver changes that are not absolutely needed. Doing so just makes user
suspect the driver for any breakage that occurs after driver changes and
this wastes my time for no valuable reasons.

About your proposal, it has not been NOKed, but it is not the way I would
have implemented it. By the way, I already have cleaned up the module
timer killing ins sym-2.1.15 driver (easily back-portable to sym53c8xx).
You may look at it (ftp.tux.org) if you are interested in knowing how I
would like it to have been done. For example, there are a couple of some
_smart_ #if that allows to preserve kernel 2.2 compatibility with the same
source. People who donnot like cpp should switch to Java in my opinion.
Compiling lot of stuff like inlines that will not in fact be used is false
cosmetic in my opinion.

The reboot handler stuff is useless in my opinion and OTOH last time I
looked in the kernel code related to reboot handler stuff it looks to me
very incomplete. Useless stuff implies additionnal bugs that could have
been avoided.

I am not opposed to your patch and will not complain if it is applied to
kernel 2.4. I just haven't time for submitting another patch quickly nor
have time for following any breakage due to its new interactions with the
kernel.

Regards,
G?rard.

2001-10-18 01:14:16

by Tim Hockin

[permalink] [raw]

Subject: Re: [PATCH] resubmitting sym53c8xx patches

G?rard Roudier wrote:

> About your proposal, it has not been NOKed, but it is not the way I would
> have implemented it. By the way, I already have cleaned up the module
> timer killing ins sym-2.1.15 driver (easily back-portable to sym53c8xx).
> You may look at it (ftp.tux.org) if you are interested in knowing how I

I will look at it.

> The reboot handler stuff is useless in my opinion and OTOH last time I
> looked in the kernel code related to reboot handler stuff it looks to me
> very incomplete. Useless stuff implies additionnal bugs that could have
> been avoided.

We have a bootloader that loads a kernel, then loads another kernel.
Without the reboot handler, we get weird behavior, and SCSI that does not
initialize properly for the second kernel. It may not be useful to
everyone, but it is essential to some.

> I am not opposed to your patch and will not complain if it is applied to
> kernel 2.4. I just haven't time for submitting another patch quickly nor
> have time for following any breakage due to its new interactions with the
> kernel.

I appreciate your concern on this. I'll re-examine the timer stuff, and
if possible, I'd like to get to agreement ASAP, so we can X it off our list
of changes.

Thanks

Tim

--
Tim Hockin
Systems Software Engineer
Sun Microsystems, Cobalt Server Appliances
[email protected]

2001-10-22 18:25:19

by Tim Hockin

[permalink] [raw]

Subject: Re: [PATCH] resubmitting sym53c8xx patches

Attachments:

sym53c8xx.diff (6.11 kB)