On Tue, 29 Oct 2024, Magnus Lindholm wrote:
> Signed-off-by: Magnus Lindholm <linmag7@xxxxxxxxx>
You'll need some text in your change description for your patch to be
accepted.
> diff --git a/drivers/scsi/qla1280.c b/drivers/scsi/qla1280.c
> index 8958547ac111..70409a140461 100644
> --- a/drivers/scsi/qla1280.c
> +++ b/drivers/scsi/qla1280.c
> @@ -368,6 +374,7 @@
>
> #define MEMORY_MAPPED_IO 1
>
> +
Please don't add an extra line.
> @@ -655,18 +662,51 @@ qla1280_info(struct Scsi_Host *host)
> {
> static char qla1280_scsi_name_buffer[125];
> char *bp;
> + char hwrev = ' ';
> + int bits;
> struct scsi_qla_host *ha;
> struct qla_boards *bdp;
> + struct device_reg __iomem *reg;
>
> bp = &qla1280_scsi_name_buffer[0];
> ha = (struct scsi_qla_host *)host->hostdata;
> + reg = ha->iobase;
> bdp = &ql1280_board_tbl[ha->devnum];
> memset(bp, 0, sizeof(qla1280_scsi_name_buffer));
>
> +
Likewise.
> + if (IS_ISP1040(ha))
> + switch (ha->revision) {
> + case 1:
> + hwrev = ' ';
> + break;
> + case 2:
> + hwrev = 'A';
> + break;
> + case 3:
> + hwrev = ' ';
> + break;
> + case 4:
> + hwrev = 'A';
> + break;
> + case 5:
> + hwrev = 'B';
> + break;
> + case 6:
> + hwrev = 'C';
> + break;
> + default:
> + hwrev = '?';
> + break;
> + }
Currently QLA1040 is always printed even for ISP1020 devices, so I think
adding a revision to that will only be confusing. Can this reporting be
improved by any chance?
> @@ -2177,9 +2224,9 @@ qla1280_nvram_config(struct scsi_qla_host *ha)
>
> if (IS_ISP1040(ha)) {
> uint16_t hwrev, cfg1, cdma_conf;
> -
Please don't remove the line.
> hwrev = RD_REG_WORD(®->cfg_0) & ISP_CFG0_HWMSK;
>
> + ha->revision = hwrev;
Notice that masking with ISP_CFG0_HWMSK clobbers the revision. It does
not matter for existing code, but it breaks your reporting improvement.
I'm leaving the rest for the time being as you'll need to change the DMA
mask selection logic as per the other thread. Overall you'll have to
split your change into a patch series, with the DMA mask fix and reporting
improvements each as an individual self-contained update. This is how we
do development in principle, but in this particular case the fix also asks
for being backported, as it addresses data corruption, so it has to be on
its own.
NB please run updated patches through scripts/get_maintainer.pl for the
complete list of recipients to send to.
Maciej
