On Fri, 3 Oct 2014, Geert Uytterhoeven wrote:
> > + if (ATARIHW_PRESENT(TT_SCSI)) {
> > + atari_scsi_reg_read = atari_scsi_tt_reg_read;
> > + atari_scsi_reg_write = atari_scsi_tt_reg_write;
> > + } else if (ATARIHW_PRESENT(ST_SCSI)) {
> > + atari_scsi_reg_read = atari_scsi_falcon_reg_read;
> > + atari_scsi_reg_write = atari_scsi_falcon_reg_write;
>
> Can these be handled through the platform_device's resources?
>
I don't know.
> > + /* The values for CMD_PER_LUN and CAN_QUEUE are somehow arbitrary.
> > + * Higher values should work, too; try it!
> > + * (But cmd_per_lun costs memory!)
> > + *
> > + * But there seems to be a bug somewhere that requires CAN_QUEUE to be
> > + * 2*CMD_PER_LUN. At least on a TT, no spurious timeouts seen since
> > + * changed CMD_PER_LUN...
> > + *
> > + * Note: The Falcon currently uses 8/1 setting due to unsolved problems
> > + * with cmd_per_lun != 1
> > + */
> > + if (IS_A_TT()) {
> > + atari_scsi_template.can_queue = 16;
> > + atari_scsi_template.cmd_per_lun = 8;
> > + atari_scsi_template.sg_tablesize = SG_ALL;
> > + } else {
> > + atari_scsi_template.can_queue = 8;
> > + atari_scsi_template.cmd_per_lun = 1;
> > + atari_scsi_template.sg_tablesize = SG_NONE;
> > + }
>
> Shouldn't this be in platform data?
>
> > + /* Leave sg_tablesize at 0 on a Falcon! */
> > + if (IS_A_TT() && setup_sg_tablesize >= 0)
> > + atari_scsi_template.sg_tablesize = setup_sg_tablesize;
>
> I think the IS_A_TT() check can just be removed.
Well, ST DMA will break if scatter/gather is enabled by the user.
>
> > +#ifdef REAL_DMA
> > + /* If running on a Falcon and if there's TT-Ram (i.e., more than one
> > + * memory block, since there's always ST-Ram in a Falcon), then
> > + * allocate a STRAM_BUFFER_SIZE byte dribble buffer for transfers
> > + * from/to alternative Ram.
> > + */
> > + if (ATARIHW_PRESENT(ST_SCSI) && !ATARIHW_PRESENT(EXTD_DMA) &&
> > + m68k_num_memory > 1) {
> > + atari_dma_buffer = atari_stram_alloc(STRAM_BUFFER_SIZE, "SCSI");
> > + if (!atari_dma_buffer) {
> > + pr_err(PFX "can't allocate ST-RAM double buffer\n");
> > + return -ENOMEM;
> > + }
> > + atari_dma_phys_buffer = atari_stram_to_phys(atari_dma_buffer);
> > + atari_dma_orig_addr = 0;
> > + }
> > +#endif
>
> More platform data?
>
> > + if (IS_A_TT())
> > + instance->irq = IRQ_TT_MFP_SCSI;
> > + else
> > + instance->irq = IRQ_NONE;
>
> platform_device resource?
If I thought it possible to parameterize the driver such that it never had
to test IS_A_TT(), I'd probably agree that this would be more elegant.
Otherwise I'd prefer not to have parts of the logic separated off into the
platform resources while the remaining logic remains in the driver itself.
While I don't think platform resources are desirable in this driver, I
would like to hear Michael's views.
Aside from TT and ST, is there a third configuration that might benefit
from a more data driven configuration?
>
> (and IRQ_NONE is wrong, you should use 0)
>
> > + if (IS_A_TT()) {
>
> Check for instance->irq instead?
Yes, you'd think so, but a later patch (not in this set) would have
to change it back to IS_A_TT().
Further patches align atari_NCR5380.c with NCR5380.c, such that the core
driver then checks host->irq to find out whether an IRQ is in use. For
Atari ST, request_irq() is not called even though the ST DMA irq is in
use.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
