Re: [PATCH] resolve collision of generic ATA_FLAG_LOWTAG and driver specific flag

[Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>]

Hello.

On 04/10/2015 02:15 PM, Christoph Hellwig wrote:

>> The patch "libata: allow sata_sil24 to opt-out of tag ordered submission"
>> (72dd299d5039a336493993dcc63413cf31d0e662) introduces a regression with the
>> sata_sil24 driver.

>> The new flag ATA_FLAG_LOWTAG accidentially uses the same bit as
>> SIL24_FLAG_PCIX_IRQ_WOC in the driver. This activates code for Silicon Image
>> 3132, which is only suppossed to run under 3124.
>>
>> ATA_FLAG_LOWTAG is only used in sata_sil24 and is planned to be removed soon,
>> so lets just use another bit for the flag in sata_sil24.
>>
>> Signed-off-by: Ronny Hegewald <Ronny.Hegewald@online.de>
>> Cc: stable@vger.kernel.org
>>
>> --- linux-3.18.5/drivers/ata/sata_sil24.c.org
>> +++ linux-3.18.5/drivers/ata/sata_sil24.c
>> @@ -247,7 +247,7 @@
>>   	SIL24_COMMON_FLAGS	= ATA_FLAG_SATA | ATA_FLAG_PIO_DMA |
>>   				  ATA_FLAG_NCQ | ATA_FLAG_ACPI_SATA |
>>   				  ATA_FLAG_AN | ATA_FLAG_PMP | ATA_FLAG_LOWTAG,
>> -	SIL24_FLAG_PCIX_IRQ_WOC	= (1 << 24), /* IRQ loss errata on PCI-X */
>> +	SIL24_FLAG_PCIX_IRQ_WOC	= (1 << 25), /* IRQ loss errata on PCI-X */

    This one also collides with ATA_FLAG_SAS_HOST.

> And this will clash as soon as the next flag is added.  Please don't
> abuse the common flag space for driver specific ones, and add a separate
> flags field for driver specific flags.

    Actually, bits 24-31 are reserved for the low-level driver usage (see the 
comment below ATA_FLAG_*), so it's the new ATA_FLAG_LOWTAG and 
ATA_FLAG_SAS_HOST that have violated the convention and should be moved 
(there's plenty of lower bits due to the removal of some obsolete flags).

WBR, Sergei