Re: PATCH: pata_oldpiix rev 0.2

[Junio C Hamano <junkio@cox.net>]

Jeff Garzik <jgarzik@pobox.com> writes:

> Alan Cox wrote:
>> static void oldpiix_pata_phy_reset(struct ata_port *ap)
>...
>> static void oldpiix_set_piomode (struct ata_port *ap, struct
>>...
>> 	unsigned int pio	= adev->pio_mode - XFER_PIO_0;
>> 	struct pci_dev *dev	= to_pci_dev(ap->host_set->dev);
>> 	unsigned int idetm_port= ap->hard_port_no ? 0x42 : 0x40;
>
> space after '='?

I think you meant space before '='.

Did you have to quote the entire patch?  It was quite annoying
to read.

Ideally, "this applies on top of your patch, to address these
issues", would be the easiest to read, but that would require
you to actually apply Alan's patch on your tree, do your
modifications and produce another patch (IOW, it would be more
work on your part).  Just to provide quick comments, if you
trimmed the patch you quoted from Alan's when commenting,
leaving enough material for Alan and others to identify the
pieces you are commenting on, it would have been a lot easier to
read (it would be more work for the contributor, but it is his
patch after all).