Hi Geert!
Op wo 30 okt. 2019 om 09:45 schreef Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>:
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 ;-)
Yes, it's been a while :-)
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.
Nice one. That's indeed even better.
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.
No, there are several more actually:
* "esp->rev > ESP100A" which basically means "HAS_CONFIG3".
* "esp->rev < ESP236" which basically means "!HAS_FAST_CLOCK".
* "esp->rev >= FAS236" which basically means "HAS_FAST_CLOCK"
* "esp->rev >= FAS100A" which basically means "HAS_OLD_CONFIG3"
So, perhaps having feature bits is not a bad idea at all...
Kind regards,
Kars.
