Re: [PATCH v2 2/4] ata_piix: unify code for programming PIO and MWDMAtimings

[Sergei Shtylyov]

Hello.

On 10/14/2011 08:43 PM, Sergei Shtylyov wrote:

> > > > From: Bartlomiej Zolnierkiewicz<bzolnier@xxxxxxxxx>
> > > > Subject: [PATCH v2] ata_piix: unify code for programming PIO and MWDMA timings

> > > > Besides making things noticably simpler it results in ~2% decrease in
> > > > the driver LOC count and also ~2% decrease in the driver binary size
> > > > (as measured on x86-32).

> > > > Fix piix_set_piomode() documentation while at it.

> > > > Signed-off-by: Bartlomiej Zolnierkiewicz<bzolnier@xxxxxxxxx>

> > > I'm still inclined to NAK this patch.

> > > > ---
> > > > v2: remove use_mwdma variable and fix comment style

> > > > earlier references:
> > > > https://lkml.org/lkml/2011/2/8/99

> > > > drivers/ata/ata_piix.c | 111 ++++++++++++++++---------------------------------
> > > > 1 file changed, 38 insertions(+), 73 deletions(-)

> > > > Index: b/drivers/ata/ata_piix.c
> > > > ===================================================================
> > > > --- a/drivers/ata/ata_piix.c
> > > > +++ b/drivers/ata/ata_piix.c
[...]

> > We still have following code at the end of piix_set_timings():

> > /* Ensure the UDMA bit is off - it will be turned back on if
> > UDMA is selected */
> >
> > if (ap->udma_mask) {
> > pci_read_config_byte(dev, 0x48,&udma_enable);
> > udma_enable&= ~(1<< (2 * ap->port_no + adev->devno));
> > pci_write_config_byte(dev, 0x48, udma_enable);
> > }

> > so everything should be OK.

> Ah, I should have looked... Didn't expect it to be there, and it in fact
> shouldn't be there, but now I'm OK with your patch. :-)

   But I would have preferred the above code to be removed, of course.

WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/