Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1569050 > unrolled thread

Re: [PATCH 0/6] ncr5380: Miscellaneous minor patches

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 0/6] ncr5380: Miscellaneous minor patches Ondrej Zary <linux@rainbow-software.org> - 2017-01-28 23:40 +0100
    Re: [PATCH 0/6] ncr5380: Miscellaneous minor patches Finn Thain <fthain@telegraphics.com.au> - 2017-01-29 02:10 +0100
      Re: [PATCH 0/6] ncr5380: Miscellaneous minor patches Ondrej Zary <linux@rainbow-software.org> - 2017-01-30 23:00 +0100
        g_NCR5380 PDMA, was Re: [PATCH 0/6] ncr5380: Miscellaneous minor  patches Finn Thain <fthain@telegraphics.com.au> - 2017-01-31 02:40 +0100

#1569050 — Re: [PATCH 0/6] ncr5380: Miscellaneous minor patches

On Monday 16 January 2017 00:50:57 Finn Thain wrote:
> This series removes some unused code and related comments,
> addresses the warnings generated by 'make W=1' and 'make C=1'
> and fixes a theoretical bug in the bus reset method in atari_scsi.
>
> There's also a patch to add a missing error check during target
> selection. The only target I tested was a QUANTUM DAYTONA514S disk
> as that's all I have access to right now. Some testing with other
> targets would be prudent.
>
> Michael, Ondrej, can I get you to review/test please?

Tested on HP C2502 (53C400A chip), Canon FG2-5202 (53C400 chip), DTC-3181L 
(DTCT-436P chip) and MS-PNR (53C400A chip) ISA cards - everything works fine!

Targets tested:
QUANTUM  LP240S GM240S01X
IBM      DORS-32160
IBM      0663L12

Thanks.

Tested-by: Ondrej Zary <linux@rainbow-software.org>

-- 
Ondrej Zary

[toc] | [next] | [standalone]

#1569147

On Sat, 28 Jan 2017, Ondrej Zary wrote:

> On Monday 16 January 2017 00:50:57 Finn Thain wrote:
> > This series removes some unused code and related comments, addresses 
> > the warnings generated by 'make W=1' and 'make C=1' and fixes a 
> > theoretical bug in the bus reset method in atari_scsi.
> >
> > There's also a patch to add a missing error check during target 
> > selection. The only target I tested was a QUANTUM DAYTONA514S disk as 
> > that's all I have access to right now. Some testing with other targets 
> > would be prudent.
> >
> > Michael, Ondrej, can I get you to review/test please?
> 
> Tested on HP C2502 (53C400A chip), Canon FG2-5202 (53C400 chip), 
> DTC-3181L (DTCT-436P chip) and MS-PNR (53C400A chip) ISA cards - 
> everything works fine!
> 
> Targets tested:
> QUANTUM  LP240S GM240S01X
> IBM      DORS-32160
> IBM      0663L12
> 
> Thanks.
> 
> Tested-by: Ondrej Zary <linux@rainbow-software.org>
> 

Very helpful. Thank you, Ondrej.

-- 

[toc] | [prev] | [next] | [standalone]

#1570133

On Sunday 29 January 2017 02:05:02 Finn Thain wrote:
> On Sat, 28 Jan 2017, Ondrej Zary wrote:
> > On Monday 16 January 2017 00:50:57 Finn Thain wrote:
> > > This series removes some unused code and related comments, addresses
> > > the warnings generated by 'make W=1' and 'make C=1' and fixes a
> > > theoretical bug in the bus reset method in atari_scsi.
> > >
> > > There's also a patch to add a missing error check during target
> > > selection. The only target I tested was a QUANTUM DAYTONA514S disk as
> > > that's all I have access to right now. Some testing with other targets
> > > would be prudent.
> > >
> > > Michael, Ondrej, can I get you to review/test please?
> >
> > Tested on HP C2502 (53C400A chip), Canon FG2-5202 (53C400 chip),
> > DTC-3181L (DTCT-436P chip) and MS-PNR (53C400A chip) ISA cards -
> > everything works fine!
> >
> > Targets tested:
> > QUANTUM  LP240S GM240S01X
> > IBM      DORS-32160
> > IBM      0663L12
> >
> > Thanks.
> >
> > Tested-by: Ondrej Zary <linux@rainbow-software.org>
>
> Very helpful. Thank you, Ondrej.

Also tested two CD-ROM drives and they didn't work (machine hangs). They
didn't work before and looks like they never worked with PDMA.

The fundamental problem of PDMA is that it either transfers all required data
or it breaks horribly. Even when I added timeouts to the possibly infinite
loops (to avoid hangs), the chip remained in some bad state. Sometimes the
53C80 gated IRQ check triggers. This can be hopefully fixed but there is a HW
limitation: if less than 128 bytes were read from SCSI device, they get lost
in chip buffer (the 128B buffers don't swap until full).

Fortunately, most of the requests for too much data are bogus. The problem is
that generic_NCR5380_dma_xfer_len() uses cmd->transfersize which seems to be
wrong. All other NCR5380 drivers use cmd->SCp.this_residual.

This is also why rescan-scsi-bus hangs (cmd->transfersize is 8192 but
cmd->SCp.this_residual is only 96).

This quick fix allows CD-ROM and also rescan-scsi-bus to work.
But it's not complete. Seems that we need:
 - fix pread and pwrite to terminate gracefully
 - something like atari_scsi_dma_xfer_len to allow DMA only for block commands

@@ -588,22 +619,19 @@ static inline int generic_NCR5380_pwrite(struct NCR5380_hostdata *hostdata,
 static int generic_NCR5380_dma_xfer_len(struct NCR5380_hostdata *hostdata,
                                         struct scsi_cmnd *cmd)
 {
-       int transfersize = cmd->transfersize;
+       int transfersize = cmd->SCp.this_residual;

        if (hostdata->flags & FLAG_NO_PSEUDO_DMA)
                return 0;

-       /* Limit transfers to 32K, for xx400 & xx406
-        * pseudoDMA that transfers in 128 bytes blocks.
-        */
-       if (transfersize > 32 * 1024 && cmd->SCp.this_residual &&
-           !(cmd->SCp.this_residual % transfersize))
-               transfersize = 32 * 1024;
-
        /* 53C400 datasheet: non-modulo-128-byte transfers should use PIO */
        if (transfersize % 128)
                transfersize = 0;

+       /* Limit transfers to 32K */
+       if (transfersize > 32 * 1024)
+               transfersize = 32 * 1024;
+
        return transfersize;
 }




-- 
Ondrej Zary

[toc] | [prev] | [next] | [standalone]

#1570243 — g_NCR5380 PDMA, was Re: [PATCH 0/6] ncr5380: Miscellaneous minor patches

On Mon, 30 Jan 2017, Ondrej Zary wrote:

> 
> Also tested two CD-ROM drives and they didn't work (machine hangs). They 
> didn't work before and looks like they never worked with PDMA.
> 
> The fundamental problem of PDMA is that it either transfers all required 
> data or it breaks horribly. Even when I added timeouts to the possibly 
> infinite loops (to avoid hangs), the chip remained in some bad state. 
> Sometimes the 53C80 gated IRQ check triggers. This can be hopefully 
> fixed but there is a HW limitation: if less than 128 bytes were read 
> from SCSI device, they get lost in chip buffer (the 128B buffers don't 
> swap until full).
> 

The core driver allows wrapper drivers to inhibit PDMA e.g. for inbound 
transfers smaller than 128 bytes. This is used by atari_scsi and sun3_scsi 
(as you probably realize). For example, atari_scsi now uses 
cmd->sc_data_direction to find out the direction of the DMA transfer.

> Fortunately, most of the requests for too much data are bogus. The 
> problem is that generic_NCR5380_dma_xfer_len() uses cmd->transfersize 
> which seems to be wrong. All other NCR5380 drivers use 
> cmd->SCp.this_residual.
> 

Yes, this always looked wrong to me. Much of the logic in 
generic_NCR5380_dma_xfer_len() looks suspicious but I see you've addressed 
this below. Thanks for looking into this.

> This is also why rescan-scsi-bus hangs (cmd->transfersize is 8192 but 
> cmd->SCp.this_residual is only 96).
> 

Great, that resolves that mystery.

> This quick fix allows CD-ROM and also rescan-scsi-bus to work.
> But it's not complete. Seems that we need:
>  - fix pread and pwrite to terminate gracefully

mac_scsi.c probably offers a reasonable example of a PDMA transfer 
algorithm that is resiliant to timeouts.

See also the cmd->device->borken logic in the core driver. This way, 
generic_NCR5380_pread() or generic_NCR5380_pwrite() can inhibit further 
PDMA to that device if need be.

>  - something like atari_scsi_dma_xfer_len to allow DMA only for block 
>    commands
> 

Are you trying to figure out which commands are going to disconnect during 
a transfer? This is really a function of the firmware in the target; there 
are no good heuristics AFAICT, so the PDMA algorithm has to be robust. 
mac_scsi has to cope with this too.

Does the problem go away when you assign no IRQ? When instance->irq == 
NO_IRQ, the core driver will inhibit disconnect privileges.

> @@ -588,22 +619,19 @@ static inline int generic_NCR5380_pwrite(struct NCR5380_hostdata *hostdata,
>  static int generic_NCR5380_dma_xfer_len(struct NCR5380_hostdata *hostdata,
>                                          struct scsi_cmnd *cmd)
>  {
> -       int transfersize = cmd->transfersize;
> +       int transfersize = cmd->SCp.this_residual;
> 
>         if (hostdata->flags & FLAG_NO_PSEUDO_DMA)
>                 return 0;
> 
> -       /* Limit transfers to 32K, for xx400 & xx406
> -        * pseudoDMA that transfers in 128 bytes blocks.
> -        */
> -       if (transfersize > 32 * 1024 && cmd->SCp.this_residual &&
> -           !(cmd->SCp.this_residual % transfersize))
> -               transfersize = 32 * 1024;
> -
>         /* 53C400 datasheet: non-modulo-128-byte transfers should use PIO */
>         if (transfersize % 128)
>                 transfersize = 0;
> 
> +       /* Limit transfers to 32K */
> +       if (transfersize > 32 * 1024)
> +               transfersize = 32 * 1024;
> +
>         return transfersize;

I would prefer to see,

#define G_NCR5380_DMA_MAX_SIZE	32768

...

	return min(transfersize, G_NCR5380_DMA_MAX_SIZE);

Thanks.

-- 

>  }
> 
> 
> 
> 
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web