From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: Rasmus Andersen <rasmus@jaquet.dk>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>,
linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org
Subject: Re: [NEW SCSI DRIVER] for 53c700 chip and NCR_D700 card against 2.4.4
Date: Mon, 14 May 2001 20:51:08 -0500 [thread overview]
Message-ID: <200105150151.UAA08944@jet.il.steeleye.com> (raw)
In-Reply-To: Message from Rasmus Andersen <rasmus@jaquet.dk> of "Mon, 14 May 2001 22:23:06 +0200." <20010514222306.B840@jaquet.dk>
I've implemented most of these as you suggest, with these exceptions:
> > + default:
> > + printk(KERN_INFO "scsi%d (%d:%d): Unexpected message %s: ",
> > + host->host_no, pun, lun,
> > + NCR_700_phase[(dsps & 0xf00) >> 8]);
> > + print_msg(hostdata->msgin);
> > + printk("\n");
>
> It would be nice with a KERN_XX before "\n" (yes, I recognize that
> print_msg does not do this :( )
The KERN_XX only go at the beginning of an output line (otherwise you see rather annoying <n> embedded in the log---the smc-mca driver has a hugely irritating example of this). print_msg doesn't do a carriage return by itself, so these three prints are really only one line (and hence all done under the KERN_INFO).
The "OUTSTANDING TAGS.." message is another one of these---it prints a list of tags all on one line.
rasmus@jaquet.dk said:
> + if(request_irq(irq, NCR_700_intr, SA_SHIRQ, "NCR_D700",
> +host)) {
> + printk(KERN_ERR "NCR D700, channel %d: irq
> +problem, detatching\n", i);
> + NCR_700_release(host);
> + release_region(host->base, 64);
> + continue;
> + }
>
> You need to kfree(hostdata) here. I would also recommend putting
> a scsi_unregister into NCR_700_release since you need to balance
> the scsi_register done in NCR_700_detect.
Actually, I need to remove the release_region here (since it's done in NCR_700_release) and add a scsi_unregister.
There's no need to do a scsi_unregister in _release; scsi_unregister is only really used to tell the mid layer to relinquish a host after an error. In _release, the mid layer is telling you that it has released the host and we need to clean up.
Thanks for the updates, I'll post another patch once I've got a few other people's suggestions rolled in.
James
next prev parent reply other threads:[~2001-05-15 1:51 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2001-05-12 16:43 James Bottomley
2001-05-14 20:23 ` Rasmus Andersen
2001-05-15 1:51 ` James Bottomley [this message]
2001-05-13 21:43 Andries.Brouwer
2001-05-13 22:40 ` Alan Cox
2001-05-14 12:41 ` Richard Hirst
2001-05-14 14:58 ` Alan Cox
2001-05-13 23:57 Andries.Brouwer
2001-05-14 0:55 ` James Bottomley
2001-05-14 12:43 ` Richard Hirst
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200105150151.UAA08944@jet.il.steeleye.com \
--to=james.bottomley@hansenpartnership.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=rasmus@jaquet.dk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®