Nice fix! As Michael mentioned already, you'll need to send this to all the relevant recipients: $ scripts/get_maintainer.pl add-support-for-fsc-chip.patch "James E.J. Bottomley" <jejb@xxxxxxxxxxxxx> (maintainer:SCSI SUBSYSTEM) "Martin K. Petersen" <martin.petersen@xxxxxxxxxx> (maintainer:SCSI SUBSYSTEM) linux-scsi@xxxxxxxxxxxxxxx (open list:SCSI SUBSYSTEM) linux-kernel@xxxxxxxxxxxxxxx (open list) A few more suggestions follow. On Tue, 29 Oct 2019, Kars de Jong wrote:
The FSC (NCR53CF9x-2 / SYM53CF9x-2) has a different family code than QLogic or Emulex parts. This caused it to be detected as a FAS100A. Unforunately, this meant the configuration of the CONFIG3 register was incorrect. This causes data transfer issues. The FSC also has a feature called Active Negation which should always be enabled according to the data manual. This is done using the CONFIG4 register. Signed-off-by: Kars de Jong <jongk@xxxxxxxxxxxxxx> --- drivers/scsi/esp_scsi.c | 10 ++++++++-- drivers/scsi/esp_scsi.h | 10 ++++++---- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c index bb88995a12c7..6b34a5764de5 100644 --- a/drivers/scsi/esp_scsi.c +++ b/drivers/scsi/esp_scsi.c @@ -263,7 +263,11 @@ static void esp_reset_esp(struct esp *esp) esp->rev = FAS236; else if (family_code == 0x0a) esp->rev = FASHME; /* Version is usually '5'. */ - else + else if (family_code == 0x14) { + esp->rev = FSC; + /* Enable Active Negation */ + esp_write8(ESP_CONFIG4_RADE, ESP_CFG4);
There is a comment in esp_scsi.h which seems to contradict the above logic, "ESP config register 4 read-write, found only on am53c974 chips." I think the comment should be corrected now.
+ } else esp->rev = FAS100A; esp->min_period = ((4 * esp->ccycle) / 1000); } else { @@ -308,7 +312,8 @@ static void esp_reset_esp(struct esp *esp) case FAS236: case PCSCSI: - /* Fast 236, AM53c974 or HME */ + case FSC: + /* Fast 236, AM53c974, FSC or HME */ esp_write8(esp->config2, ESP_CFG2); if (esp->rev == FASHME) { u8 cfg3 = esp->target[0].esp_config3; @@ -2373,6 +2378,7 @@ static const char *esp_chip_names[] = { "ESP100A", "ESP236", "FAS236", + "FSC", "FAS100A", "FAST", "FASHME", diff --git a/drivers/scsi/esp_scsi.h b/drivers/scsi/esp_scsi.h index 91b32f2a1a1b..b60ea3e5e0eb 100644 --- a/drivers/scsi/esp_scsi.h +++ b/drivers/scsi/esp_scsi.h @@ -211,6 +211,7 @@ /* ESP unique ID register read-only, found on fas236+fas100a only */ #define ESP_UID_F100A 0x00 /* ESP FAS100A */ #define ESP_UID_F236 0x02 /* ESP FAS236 */ +#define ESP_UID_FSC 0x14 /* NCR/Symbios Logic FSC */ #define ESP_UID_REV 0x07 /* ESP revision */ #define ESP_UID_FAM 0xf8 /* ESP family */
I think these should be in numerical order.
@@ -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 */
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. (No idea why this list uses hexadecimal, it's not a value from a chip register. It would be more readable in decimal, IMHO.) --
}; struct esp_cmd_entry {