Re: [PATCH] esp_scsi: Add support for FSC chip

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Kars,

Thanks for your patch!

Initially I wondere how the Apple MESH SCSI (also 53cf94 based) on
my CHRP Longtrail used to work with this, until I realized my Longtrail
died a few years before the new ESP SCSI driver was written ;-)

On Wed, Oct 30, 2019 at 8:26 AM Kars de Jong <jongk@xxxxxxxxxxxxxx> wrote:
Op wo 30 okt. 2019 om 02:06 schreef Michael Schmitz <schmitzmic@xxxxxxxxx>:
@@ -262,10 +263,11 @@ enum esp_rev {
     ESP100A    = 0x01,  /* NCR53C90A */
     ESP236     = 0x02,
     FAS236     = 0x03,
-    FAS100A    = 0x04,
-    FAST       = 0x05,
-    FASHME     = 0x06,
-    PCSCSI     = 0x07,  /* AM53c974 */
+    FSC        = 0x04,  /* NCR/Symbios Logic FSC */
+    FAS100A    = 0x05,
+    FAST       = 0x06,
+    FASHME     = 0x07,
+    PCSCSI     = 0x08,  /* AM53c974 */

Is anything caring about the actual values?
Doesn't look like that, so you can just drop the values, and let standard C
enum handling assign the numbers. That way you don't have to renumber
when adding a new part.

I'm guessing that you've placed it here because of the esp->rev >= FAS236
tests that appear in a couple of places relating to sync transfer period.
Might be worth mentioning that in the commit log.

Good catch. I think these tests would need to be changed to esp->rev
 >=FSC to reflect the fact that the FSC has the same CONFIG3 settings as
the FAS236.

Um, no, that would break the FAS236. FSC is defined after FAS236. It's
import that its values is lower than FAS100A and greater or equal than
FAS236. And as I wrote in reply to Finn, I think PCSCSI falls in the
same category.

You definitely want to add a comment like "all below use the same CONFIG3
settings", to avoid nasty surprises for future additions (if any).
Using feature bits might be even better, but may not be worthwhile, since
there seems to be only a single "esp->rev > ..." check.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds



[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux