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                     [EMAIL PROTECTED]
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);

Reply via email to