Re: [RFC/Review] libata driver for Apple "macio" pata

[Alan Cox <alan@redhat.com>]

> I know and I believe it should still be ok ... As I said, the chipset
> should use the PIO field in the register for PIO transfers. And if the
> above unknown bit is set, I suspect it's just going to increase the
> setup time a bit or something like that, which won't hurt other than
> perfs.

What I do with some other drivers is set the PIO mode in the pio mode function
but defer DMA timing setup to bmdma_start/stop methods. Some chips need this
in the PC world and that works nicely.

> > > +static unsigned long pata_macio_mode_filter(struct ata_device *adev,
> > > +					    unsigned long xfer_mask)
> > > +{
> > > +	struct pata_macio_priv *priv = adev->link->ap->private_data;
> > > +
> > > +	if (priv->dma_regs == NULL)
> > > +		xfer_mask &= ~(ATA_MASK_MWDMA | ATA_MASK_UDMA);
> > > +	return xfer_mask;
> > > +}
> > 
> > Wouldn't it be better to clear these during initialization?
> 
> I could. Doesn't matter much where it's done, does it ? Doing there
> allows to deal with a failure in my port_start() callback, if the
> allocation of the DMA table fails, I clear dma_regs.

For x86 we clear it later in some cases too - because the allocation is
done after we pass the sht and ata parameters to the setup functions.

> The reason is that I can only have 64K-4K per transfer (I don't think I
> can do 64K per DBDMA entry). So the above routine can potentially
> breakup, in the worst case scenario, the table into twice as many
> entries if they are all 64K precisely.

See ata_sff_dumb_qc_prep - we have PC chips with the same bug!

--
   "Engineers are 'small children' when it comes to product warning labels"
					-- John Duino