Subject: Re: dev/scsipi/ch.c:chattach() fix ? (and approval for commit?)
To: Stoned Elipot <seb@ssr.univ-paris7.fr>
From: Manuel Bouyer <bouyer@antioche.eu.org>
List: tech-kern
Date: 06/15/2004 22:49:08
--gKMricLos+KVdGMg
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
On Tue, Jun 15, 2004 at 04:48:24PM +0200, Stoned Elipot wrote:
> Hi,
> When I updated the box to which my scsi changer is attached from 1.6.2_STABLE
> to 2.0_BETA I've lost the pretty autoconf boot message about its
> characteristics.
>
> With straight 2.0_BETA I have:
> ch0 at scsibus1 target 4 lun 0: <QUALSTAR, RLS-4221, 0047> changer removable
> ch0: 0 slots, 0 drives, 0 pickers, 0 portals
>
> A first chio params after boot gives correct informations expect the current
> picker is somewhat wrong:
> # chio params
> /dev/ch0: 22 slots, 1 drive, 1 picker
> /dev/ch0: current picker: -65000
>
> After digging around I've found that the first call to ch_get_params() from
> ch.c:chattach() gathers mostly zero values.
>
> Reading the diffs of dev/scsipi/ between 1.6 branch and current I tried
> to add a dummy scsipi_test_unit_ready() in chattach(). Such a call was
> previously done in scsiconf.c:scsi_probe_device() and was removed in its
> revision 1.216. That solved my autoconf information gathering problem
> so then things like current picker right after boot is ok.
>
> I now have:
> ch0 at scsibus1 target 4 lun 0: <QUALSTAR, RLS-4221, 0047> changer removable
> ch0: 22 slots, 1 drive, 1 picker, 0 portals
>
> Attached is my diff. I'd like to have your comments on it.
>
> Cheers, Stoned.
> Index: ch.c
> ===================================================================
> RCS file: /cvsroot/src/sys/dev/scsipi/ch.c,v
> retrieving revision 1.59
> diff -u -u -r1.59 ch.c
> --- ch.c 23 Apr 2004 21:52:17 -0000 1.59
> +++ ch.c 15 Jun 2004 12:53:53 -0000
> @@ -225,6 +225,14 @@
> }
>
> /*
> + * Do a dummy TEST UNIT READY to shake the changer, it helps
> + * gathering information about the device for some model.
> + */
> + (void)scsipi_test_unit_ready(sc->sc_periph, XS_CTL_DISCOVERY |
> + XS_CTL_IGNORE_ILLEGAL_REQUEST | XS_CTL_IGNORE_NOT_READY |
> + XS_CTL_IGNORE_MEDIA_CHANGE);
> +
> + /*
> * Get information about the device. Note we can't use
> * interrupts yet.
> */
BTW, this comment is wrong now :)
I think this just hides the problem. We should find why we don't get an error
while the data was not updated. I suspect it's related to a Unit Attention
condition that would be cleared by the scsipi_test_unit_ready(), but I
don't see how it could be a problem.
Can you test the attached patch ? It should emit messages when a command ends
with Unit Attention and has XS_CTL_IGNORE_MEDIA_CHANGE set, and another
message when such a command is restarted.
--
Manuel Bouyer <bouyer@antioche.eu.org>
NetBSD: 26 ans d'experience feront toujours la difference
--
--gKMricLos+KVdGMg
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=diff
Index: scsipi_base.c
===================================================================
RCS file: /cvsroot/src/sys/dev/scsipi/scsipi_base.c,v
retrieving revision 1.105
diff -u -r1.105 scsipi_base.c
--- scsipi_base.c 27 Apr 2004 18:15:37 -0000 1.105
+++ scsipi_base.c 15 Jun 2004 20:46:50 -0000
@@ -947,6 +947,8 @@
/* XXX Should reupload any transient state. */
(periph->periph_flags &
PERIPH_REMOVABLE) == 0) {
+ scsipi_printaddr(periph);
+ printf("got Unit Attention, returning ERESTART\n");
return (ERESTART);
}
if ((xs->xs_control & XS_CTL_SILENT) != 0)
@@ -1651,6 +1653,8 @@
s = splbio();
if (error == ERESTART) {
+ scsipi_printaddr(periph);
+ printf("restarting command\n");
/*
* If we get here, the periph has been thawed and frozen
* again if we had to issue recovery commands. Alternatively,
--gKMricLos+KVdGMg--