On Tue, 27 Jun 2017, Ondrej Zary wrote:
> BTW. I've probably found the DTC write corruption. Added the following
> check (13 is host buffer index register) -
That register is not mentioned in my 53c400 datasheet.
> and it triggers sometimes: the value is 1 instead of 0. As we use only
> 16-bit writes, I don't see how the value could ever be odd. Looks like a
> bug in the chip. The index register corrupts during the transfer, not
> after IRQ or timeout. The same check at beginning of pwrite() did not
> trigger.
>
Are you reading this register at the right moment? Have you tried waiting
for it to reach zero, as in,
if (NCR5380_poll_politely(hostdata, 13, 0xff, 0, HZ / 64) < 0)
/* printk, reset etc */;
Even if this is a reliable way to detect a short transfer, it would be
nice to know the root cause. But I'm being unrealistic: the DTC436 vendor
never responded to my requests for technical documentation.
> The index register is not writable so we must(?) reset the PDMA engine
> to recover. However, this quick attempt to fix does not work, maybe we
> should reload the block count and continue?
>
I don't know if it is possible to recover. If the last byte never reached
the scsi bus, then once you reset the 53c400 core, you need the driver to
perform a single-byte PIO transfer after the short PDMA transfer. This
would require that you set the residual appropriately (though in my
experience that may not be sufficient).
It may be better to simply limit the transfer to 512 bytes instead of
attempting to recover based on an undocumented (?) register, etc. Seems
like a bit of a hack.
> --- a/drivers/scsi/g_NCR5380.c
> +++ b/drivers/scsi/g_NCR5380.c
> @@ -595,7 +603,13 @@ static inline int generic_NCR5380_pwrite(struct
> NCR5380_hostdata *hostdata,
> goto out_wait;
> }
> }
> -
> + idx = NCR5380_read(13);
> + if (idx != 0) {
> + printk("host idx=%d, start=%d\n", idx, start);
> + NCR5380_write(hostdata->c400_ctl_status, CSR_RESET);
> + NCR5380_write(hostdata->c400_ctl_status, CSR_BASE);
> + goto out_wait;
> + }
> if (hostdata->io_port && hostdata->io_width == 2)
> outsw(hostdata->io_port + hostdata->c400_host_buf,
> src + start, 64);
>
I find it hard to reason about this code. For example, out_wait is to be
removed. Let's get the preceding patches working and signed-off. Please go
ahead and use a 512 B transfer for DTC436 testing if that will help get
this patch series over the line.
--
