On Monday 07 December 2009 04:05:28 pm Bartlomiej Zolnierkiewicz wrote:
> On Monday 07 December 2009 02:26:00 pm Sergei Shtylyov wrote:
> > Hello.
> >
> > Alan Cox wrote:
> >
> > > Bartlomiej pointed out that while this got fixed in the old driver whoever
> > > did it didn't port it across.
> >
> > > Signed-off-by: Alan Cox <alan@xxxxxxxxxxxxxxx>
> >
> > [...]
> >
> > > +static int mwdma_clip_to_pio(struct ata_device *adev)
> > > +{
> > > + const int mwdma_to_pio[3] = {
> > > + XFER_PIO_0, XFER_PIO_3, XFER_PIO_4
> > > + };
> > > + return min(mwdma_to_pio[adev->dma_mode - XFER_MW_DMA_0],
> > > + adev->pio_mode - XFER_PIO_0);
> > > +}
> >
> > You call min() on uncomparables, i.e. mwdma_to_pio[] contains XFER_PIO_*
> > and adev->pio_mode - XFER_PIO_0 yields you a mode number. Thus the second
> > argument will always "win" as a minimal one.
>
> There are more issues with the patch related to mwdma_clip_to_pio().
>
> The function can return values between 0 and 4 which obviously won't work
> well for the new code below for values > 2 (i.e. resulting in out-of-bounds
Fortunately above bugs won't be triggered because the patch forgot to update
MWDMA masks.. ;)
What is even more interesting is that the driver currently also lacks support
for MWDMA modes on UDMA66 chips (though it was implemented in the driver).
static const struct ata_port_info sis_info66 = {
.flags = ATA_FLAG_SLAVE_POSS,
.pio_mask = ATA_PIO4,
/* No MWDMA */
.udma_mask = ATA_UDMA4,
.port_ops = &sis_66_ops,
};
static const struct ata_port_info sis_info100 = {
.flags = ATA_FLAG_SLAVE_POSS,
.pio_mask = ATA_PIO4,
/* No MWDMA */
.udma_mask = ATA_UDMA5,
.port_ops = &sis_100_ops,
};
static const struct ata_port_info sis_info100_early = {
.flags = ATA_FLAG_SLAVE_POSS,
.pio_mask = ATA_PIO4,
/* No MWDMA */
.udma_mask = ATA_UDMA5,
.port_ops = &sis_66_ops,
};
static const struct ata_port_info sis_info133 = {
.flags = ATA_FLAG_SLAVE_POSS,
.pio_mask = ATA_PIO4,
/* No MWDMA */
.udma_mask = ATA_UDMA6,
.port_ops = &sis_133_ops,
};
const struct ata_port_info sis_info133_for_sata = {
.flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST,
.pio_mask = ATA_PIO4,
/* No MWDMA */
.udma_mask = ATA_UDMA6,
.port_ops = &sis_133_for_sata_ops,
};
static const struct ata_port_info sis_info133_early = {
.flags = ATA_FLAG_SLAVE_POSS,
.pio_mask = ATA_PIO4,
/* No MWDMA */
.udma_mask = ATA_UDMA6,
.port_ops = &sis_133_early_ops,
};
--
Bartlomiej Zolnierkiewicz
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
