* [PATCH] resolve collision of generic ATA_FLAG_LOWTAG and driver specific flag
@ 2015-04-09 20:09 Ronny Hegewald
2015-04-10 11:15 ` Christoph Hellwig
2015-04-10 17:02 ` Sergei Shtylyov
0 siblings, 2 replies; 6+ messages in thread
From: Ronny Hegewald @ 2015-04-09 20:09 UTC (permalink / raw)
To: linux-ide
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 */
IRQ_STAT_4PORTS = 0xf,
};
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] resolve collision of generic ATA_FLAG_LOWTAG and driver specific flag
2015-04-09 20:09 [PATCH] resolve collision of generic ATA_FLAG_LOWTAG and driver specific flag Ronny Hegewald
@ 2015-04-10 11:15 ` Christoph Hellwig
2015-04-10 16:55 ` Sergei Shtylyov
2015-04-10 17:02 ` Sergei Shtylyov
1 sibling, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2015-04-10 11:15 UTC (permalink / raw)
To: Ronny Hegewald; +Cc: linux-ide
On Thu, Apr 09, 2015 at 08:09:08PM +0000, Ronny Hegewald 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 */
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] resolve collision of generic ATA_FLAG_LOWTAG and driver specific flag
2015-04-10 11:15 ` Christoph Hellwig
@ 2015-04-10 16:55 ` Sergei Shtylyov
2015-04-13 15:29 ` Tejun Heo
0 siblings, 1 reply; 6+ messages in thread
From: Sergei Shtylyov @ 2015-04-10 16:55 UTC (permalink / raw)
To: Christoph Hellwig, Ronny Hegewald; +Cc: linux-ide
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] resolve collision of generic ATA_FLAG_LOWTAG and driver specific flag
2015-04-09 20:09 [PATCH] resolve collision of generic ATA_FLAG_LOWTAG and driver specific flag Ronny Hegewald
2015-04-10 11:15 ` Christoph Hellwig
@ 2015-04-10 17:02 ` Sergei Shtylyov
2015-04-15 22:56 ` Ronny Hegewald
1 sibling, 1 reply; 6+ messages in thread
From: Sergei Shtylyov @ 2015-04-10 17:02 UTC (permalink / raw)
To: Ronny Hegewald, linux-ide
Hello.
On 04/09/2015 11:09 PM, Ronny Hegewald 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
Accidentally.
> SIL24_FLAG_PCIX_IRQ_WOC in the driver. This activates code for Silicon Image
> 3132, which is only suppossed to run under 3124.
Supposed.
> 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.
Actually, ATA_FLAG_LOWTAG has been removed in the 'for-next' branch, so
this patch is no longer needed. Moreover, the newly assigned flag clashes with
the new ATA_FLAG_SATA_PORT.
> Signed-off-by: Ronny Hegewald <Ronny.Hegewald@online.de>
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] resolve collision of generic ATA_FLAG_LOWTAG and driver specific flag
2015-04-10 16:55 ` Sergei Shtylyov
@ 2015-04-13 15:29 ` Tejun Heo
0 siblings, 0 replies; 6+ messages in thread
From: Tejun Heo @ 2015-04-13 15:29 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: Christoph Hellwig, Ronny Hegewald, linux-ide
On Fri, Apr 10, 2015 at 07:55:25PM +0300, Sergei Shtylyov wrote:
> >>- 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).
Yeah, this something dumb I did way back. Can we please add a
separate field for per-controller flags in port_info and all related
structs so that we don't do this sharing?
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] resolve collision of generic ATA_FLAG_LOWTAG and driver specific flag
2015-04-10 17:02 ` Sergei Shtylyov
@ 2015-04-15 22:56 ` Ronny Hegewald
0 siblings, 0 replies; 6+ messages in thread
From: Ronny Hegewald @ 2015-04-15 22:56 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linux-ide
On Friday 10 April 2015, you wrote
> Actually, ATA_FLAG_LOWTAG has been removed in the 'for-next' branch, so
> this patch is no longer needed.
Its still needed for kernel <= 3.19, else the driver will malfunction for
SI3132 based chipsets.
> Moreover, the newly assigned flag clashes with the new ATA_FLAG_SATA_PORT.
As far i can tell its not really a problem in practice for this driver, as
they will not be used together. But its certainly better to keep the flags
clash-free nonetheless. I will change it to bit 31 in the next version of the
patch.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-04-15 22:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-09 20:09 [PATCH] resolve collision of generic ATA_FLAG_LOWTAG and driver specific flag Ronny Hegewald
2015-04-10 11:15 ` Christoph Hellwig
2015-04-10 16:55 ` Sergei Shtylyov
2015-04-13 15:29 ` Tejun Heo
2015-04-10 17:02 ` Sergei Shtylyov
2015-04-15 22:56 ` Ronny Hegewald
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.