James Bottomley wrote: > On Fri, 2005-07-22 at 16:40 +0200, Hannes Reinecke wrote: >>I've finished the update of aic79xx to make use of the >>scsi_transport_spi infrastructure. >>The first patch is actually jgarzik's one, with some additions to make >>it work :-) >>The second patch is the integration proper. patch made a mess of it, >>however, so better to compare the files by hand ... >> >>Hope it finds your blessing. >>Comments etc welcome. > > This looks very good, thank you for doing it! > >>target27:0:10: FAST-160 WIDE SCSI 320.0 MB/s ST IU (6.25 ns, offset 127) > > There is a slight problem here: ST IU isn't a legal SCSI setting (DT is > a requirement for IU), so there's something slightly wrong in the > parameter setting somewhere. I'll take a look through the code and see > if I can spot it (probably post OLS). > You are, again, correct. The implied flags setting from SPI-3 is not taken into account. The applied patch (relative to the other two I've already sent) fixes this. But this is not the root problem. target5:0:10: FAST-160 WIDE SCSI 320.0 MB/s ST IU (6.25 ns, offset 127) scsi5: target 10 synchronous with period = 0x8, offset = 0x7f(RDSTRM|DT|IU|QAS) So, the ppr_options (which have generated the second line) are set ok, but they somehow haven't been transferred into the scsi_transport attribute. It appears that the fallback sequence doesn't quite work. But that'll warrant a new mail. Cheers, Hannes -- Dr. Hannes Reinecke hare@xxxxxxx SuSE Linux Products GmbH S390 & zSeries Maxfeldstraße 5 +49 911 74053 688 90409 Nürnberg http://www.suse.de
--- linux-2.6.12/drivers/scsi/aic7xxx/aic79xx_osm.c.jejb 2005-07-25 09:21:41.000000000 +0200 +++ linux-2.6.12/drivers/scsi/aic7xxx/aic79xx_osm.c 2005-07-25 09:22:37.000000000 +0200 @@ -2343,6 +2343,7 @@ static void ahd_linux_set_period(struct shost->this_id, starget->id, &tstate); struct ahd_devinfo devinfo; unsigned int ppr_options = tinfo->goal.ppr_options; + unsigned int dt; unsigned long flags; unsigned long offset = tinfo->goal.offset; @@ -2357,6 +2358,8 @@ static void ahd_linux_set_period(struct ppr_options |= MSG_EXT_PPR_IU_REQ; } + dt = ppr_options & MSG_EXT_PPR_DT_REQ; + ahd_compile_devinfo(&devinfo, shost->this_id, starget->id, 0, starget->channel + 'A', ROLE_INITIATOR); @@ -2366,7 +2369,8 @@ static void ahd_linux_set_period(struct ppr_options &= MSG_EXT_PPR_QAS_REQ; } - ahd_find_syncrate(ahd, &period, &ppr_options, AHD_SYNCRATE_MAX); + ahd_find_syncrate(ahd, &period, &ppr_options, + dt ? AHD_SYNCRATE_MAX : AHD_SYNCRATE_ULTRA2); ahd_lock(ahd, &flags); ahd_set_syncrate(ahd, &devinfo, period, offset, ppr_options, AHD_TRANS_GOAL, FALSE); @@ -2385,6 +2389,7 @@ static void ahd_linux_set_offset(struct struct ahd_devinfo devinfo; unsigned int ppr_options = 0; unsigned int period = 0; + unsigned int dt = ppr_options & MSG_EXT_PPR_DT_REQ; unsigned long flags; ahd_compile_devinfo(&devinfo, shost->this_id, starget->id, 0, @@ -2392,7 +2397,8 @@ static void ahd_linux_set_offset(struct if (offset != 0) { period = tinfo->goal.period; ppr_options = tinfo->goal.ppr_options; - ahd_find_syncrate(ahd, &period, &ppr_options, AHD_SYNCRATE_MAX); + ahd_find_syncrate(ahd, &period, &ppr_options, + dt ? AHD_SYNCRATE_MAX : AHD_SYNCRATE_ULTRA2); } ahd_lock(ahd, &flags); ahd_set_syncrate(ahd, &devinfo, period, offset, ppr_options, @@ -2419,9 +2425,13 @@ static void ahd_linux_set_dt(struct scsi ppr_options |= MSG_EXT_PPR_DT_REQ; if (period > 9) period = 9; /* at least 12.5ns for DT */ - } else if (period <= 9) - period = 10; /* If resetting DT, period must be >= 25ns */ - + } else { + if (period <= 9) + period = 10; /* If resetting DT, period must be >= 25ns */ + /* QAS and IU are invalid without DT set */ + ppr_options &= ~MSG_EXT_PPR_IU_REQ; + ppr_options &= ~MSG_EXT_PPR_QAS_REQ; + } ahd_compile_devinfo(&devinfo, shost->this_id, starget->id, 0, starget->channel + 'A', ROLE_INITIATOR); ahd_find_syncrate(ahd, &period, &ppr_options, @@ -2445,11 +2455,17 @@ static void ahd_linux_set_qas(struct scs unsigned int ppr_options = tinfo->goal.ppr_options & ~MSG_EXT_PPR_QAS_REQ; unsigned int period = tinfo->goal.period; - unsigned int dt = ppr_options & MSG_EXT_PPR_DT_REQ; + unsigned int dt; unsigned long flags; - if (qas) - ppr_options |= MSG_EXT_PPR_QAS_REQ; + if (qas) { + /* QAS requires IU and DT to be set */ + ppr_options |= MSG_EXT_PPR_QAS_REQ; + ppr_options |= MSG_EXT_PPR_IU_REQ; + ppr_options |= MSG_EXT_PPR_DT_REQ; + } + + dt = ppr_options & MSG_EXT_PPR_DT_REQ; ahd_compile_devinfo(&devinfo, shost->this_id, starget->id, 0, starget->channel + 'A', ROLE_INITIATOR); @@ -2474,11 +2490,17 @@ static void ahd_linux_set_iu(struct scsi unsigned int ppr_options = tinfo->goal.ppr_options & ~MSG_EXT_PPR_IU_REQ; unsigned int period = tinfo->goal.period; - unsigned int dt = ppr_options & MSG_EXT_PPR_DT_REQ; + unsigned int dt; unsigned long flags; - if (iu) + if (iu) { ppr_options |= MSG_EXT_PPR_IU_REQ; + ppr_options |= MSG_EXT_PPR_DT_REQ; /* IU requires DT */ + } else { + ppr_options &= ~MSG_EXT_PPR_QAS_REQ; /* No QAS without IU */ + } + + dt = ppr_options & MSG_EXT_PPR_DT_REQ; ahd_compile_devinfo(&devinfo, shost->this_id, starget->id, 0, starget->channel + 'A', ROLE_INITIATOR);