Turn open-coded checks in piix_set_timings() into PIIX_FLAG_* flags
per suggestion from Alan Cox.
Suggested-by: Alan Cox <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
On top of https://lkml.org/lkml/2011/2/8/97 patchset.
drivers/ata/ata_piix.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
Index: b/drivers/ata/ata_piix.c
===================================================================
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -133,6 +133,8 @@ enum {
PIIX_FLAG_CHECKINTR = (1 << 28), /* make sure PCI INTx enabled */
PIIX_FLAG_SIDPR = (1 << 29), /* SATA idx/data pair regs */
+ PIIX_FLAG_NO_SITRE = (1 << 30), /* no SITRE register */
+ PIIX_FLAG_RADISYS = (1 << 31), /* host is Radisys R82600 */
PIIX_PATA_FLAGS = ATA_FLAG_SLAVE_POSS,
PIIX_SATA_FLAGS = ATA_FLAG_SATA | PIIX_FLAG_CHECKINTR,
@@ -698,7 +700,7 @@ static struct ata_port_info piix_port_in
[oldpiix_pata] =
{
- .flags = PIIX_PATA_FLAGS,
+ .flags = PIIX_PATA_FLAGS | PIIX_FLAG_NO_SITRE,
.pio_mask = ATA_PIO4,
.mwdma_mask = ATA_MWDMA12_ONLY,
.port_ops = &oldpiix_pata_ops,
@@ -706,7 +708,8 @@ static struct ata_port_info piix_port_in
[radisys_pata] =
{
- .flags = PIIX_PATA_FLAGS,
+ .flags = PIIX_PATA_FLAGS | PIIX_FLAG_NO_SITRE |
+ PIIX_FLAG_RADISYS,
.pio_mask = ATA_PIO4,
.mwdma_mask = ATA_MWDMA12_ONLY,
.udma_mask = ATA_UDMA24_ONLY,
@@ -859,10 +862,8 @@ static void piix_set_timings(struct ata_
struct pci_dev *dev = to_pci_dev(ap->host->dev);
unsigned long flags;
unsigned int is_slave = (adev->devno != 0);
- unsigned int is_radisys = (dev->vendor == PCI_VENDOR_ID_RADISYS &&
- dev->device == 0x8201);
- unsigned int has_sitre = (dev->vendor != PCI_VENDOR_ID_INTEL ||
- dev->device != 0x1230) && !is_radisys;
+ unsigned int is_radisys = (ap->flags & PIIX_FLAG_RADISYS) ? 1 : 0;
+ unsigned int has_sitre = (ap->flags & PIIX_FLAG_NO_SITRE) ? 1 : 0;
unsigned int master_port= ap->port_no ? 0x42 : 0x40;
unsigned int slave_port = 0x44;
u16 master_data;
Turn open-coded checks in piix_set_timings() into PIIX_FLAG_* flags
per suggestion from Alan Cox.
Suggested-by: Alan Cox <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
- On top of https://lkml.org/lkml/2011/2/8/97 patchset.
- Get the reverse logic correct this time.
drivers/ata/ata_piix.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
Index: b/drivers/ata/ata_piix.c
===================================================================
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -133,6 +133,8 @@ enum {
PIIX_FLAG_CHECKINTR = (1 << 28), /* make sure PCI INTx enabled */
PIIX_FLAG_SIDPR = (1 << 29), /* SATA idx/data pair regs */
+ PIIX_FLAG_NO_SITRE = (1 << 30), /* no SITRE register */
+ PIIX_FLAG_RADISYS = (1 << 31), /* host is Radisys R82600 */
PIIX_PATA_FLAGS = ATA_FLAG_SLAVE_POSS,
PIIX_SATA_FLAGS = ATA_FLAG_SATA | PIIX_FLAG_CHECKINTR,
@@ -698,7 +700,7 @@ static struct ata_port_info piix_port_in
[oldpiix_pata] =
{
- .flags = PIIX_PATA_FLAGS,
+ .flags = PIIX_PATA_FLAGS | PIIX_FLAG_NO_SITRE,
.pio_mask = ATA_PIO4,
.mwdma_mask = ATA_MWDMA12_ONLY,
.port_ops = &oldpiix_pata_ops,
@@ -706,7 +708,8 @@ static struct ata_port_info piix_port_in
[radisys_pata] =
{
- .flags = PIIX_PATA_FLAGS,
+ .flags = PIIX_PATA_FLAGS | PIIX_FLAG_NO_SITRE |
+ PIIX_FLAG_RADISYS,
.pio_mask = ATA_PIO4,
.mwdma_mask = ATA_MWDMA12_ONLY,
.udma_mask = ATA_UDMA24_ONLY,
@@ -859,10 +862,8 @@ static void piix_set_timings(struct ata_
struct pci_dev *dev = to_pci_dev(ap->host->dev);
unsigned long flags;
unsigned int is_slave = (adev->devno != 0);
- unsigned int is_radisys = (dev->vendor == PCI_VENDOR_ID_RADISYS &&
- dev->device == 0x8201);
- unsigned int has_sitre = (dev->vendor != PCI_VENDOR_ID_INTEL ||
- dev->device != 0x1230) && !is_radisys;
+ unsigned int is_radisys = (ap->flags & PIIX_FLAG_RADISYS) ? 1 : 0;
+ unsigned int has_sitre = (ap->flags & PIIX_FLAG_NO_SITRE) ? 0 : 1;
unsigned int master_port= ap->port_no ? 0x42 : 0x40;
unsigned int slave_port = 0x44;
u16 master_data;
Hello.
Bartlomiej Zolnierkiewicz wrote:
> Turn open-coded checks in piix_set_timings() into PIIX_FLAG_* flags
> per suggestion from Alan Cox.
> Suggested-by: Alan Cox <[email protected]>
> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
[...]
> Index: b/drivers/ata/ata_piix.c
> ===================================================================
> --- a/drivers/ata/ata_piix.c
> +++ b/drivers/ata/ata_piix.c
> @@ -133,6 +133,8 @@ enum {
>
> PIIX_FLAG_CHECKINTR = (1 << 28), /* make sure PCI INTx enabled */
> PIIX_FLAG_SIDPR = (1 << 29), /* SATA idx/data pair regs */
> + PIIX_FLAG_NO_SITRE = (1 << 30), /* no SITRE register */
The register in question is called SIDETIM, SITRE is a bit that enables its use.
WBR, Sergei
Hi,
On Tue, Feb 8, 2011 at 6:12 PM, Sergei Shtylyov <[email protected]> wrote:
> Hello.
>
> Bartlomiej Zolnierkiewicz wrote:
>
>> Turn open-coded checks in piix_set_timings() into PIIX_FLAG_* flags
>> per suggestion from Alan Cox.
>
>> Suggested-by: Alan Cox <[email protected]>
>> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
>
> [...]
>
>> Index: b/drivers/ata/ata_piix.c
>> ===================================================================
>> --- a/drivers/ata/ata_piix.c
>> +++ b/drivers/ata/ata_piix.c
>> @@ -133,6 +133,8 @@ enum {
>> ? ? ? ?PIIX_FLAG_CHECKINTR ? ? = (1 << 28), /* make sure PCI INTx enabled
>> */
>> ? ? ? ?PIIX_FLAG_SIDPR ? ? ? ? = (1 << 29), /* SATA idx/data pair regs */
>> + ? ? ? PIIX_FLAG_NO_SITRE ? ? ?= (1 << 30), /* no SITRE register */
>
> ? The register in question is called SIDETIM, SITRE is a bit that enables
> its use.
ICH4-M databook that I have at hand (Intel IDE PRM seems to be gone
from Intel's website, though I'm sure I have a backup _somewhere_) it
is called SLV_IDETIM so lets just stick with the current naming for
now..
Thanks,
Bartlomiej
Bartlomiej Zolnierkiewicz wrote:
>>> Turn open-coded checks in piix_set_timings() into PIIX_FLAG_* flags
>>> per suggestion from Alan Cox.
>>> Suggested-by: Alan Cox <[email protected]>
>>> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
>> [...]
>>> Index: b/drivers/ata/ata_piix.c
>>> ===================================================================
>>> --- a/drivers/ata/ata_piix.c
>>> +++ b/drivers/ata/ata_piix.c
>>> @@ -133,6 +133,8 @@ enum {
>>> PIIX_FLAG_CHECKINTR = (1 << 28), /* make sure PCI INTx enabled
>>> */
>>> PIIX_FLAG_SIDPR = (1 << 29), /* SATA idx/data pair regs */
>>> + PIIX_FLAG_NO_SITRE = (1 << 30), /* no SITRE register */
>> The register in question is called SIDETIM, SITRE is a bit that enables
>> its use.
> ICH4-M databook that I have at hand (Intel IDE PRM seems to be gone
> from Intel's website, though I'm sure I have a backup _somewhere_) it
> is called SLV_IDETIM so lets just stick with the current naming for
> now..
Why not just say that SITRE is a bit, not register? :-)
> Thanks,
> Bartlomiej
WBR, Sergei
Sergei Shtylyov wrote:
> Bartlomiej Zolnierkiewicz wrote:
>
> >>> Turn open-coded checks in piix_set_timings() into PIIX_FLAG_* flags
> >>> per suggestion from Alan Cox.
> >>> Suggested-by: Alan Cox <[email protected]>
> >>> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> >> [...]
>
> >>> Index: b/drivers/ata/ata_piix.c
> >>> ===================================================================
> >>> --- a/drivers/ata/ata_piix.c
> >>> +++ b/drivers/ata/ata_piix.c
> >>> @@ -133,6 +133,8 @@ enum {
> >>> PIIX_FLAG_CHECKINTR = (1 << 28), /* make sure PCI INTx enabled
> >>> */
> >>> PIIX_FLAG_SIDPR = (1 << 29), /* SATA idx/data pair regs */
> >>> + PIIX_FLAG_NO_SITRE = (1 << 30), /* no SITRE register */
>
> >> The register in question is called SIDETIM, SITRE is a bit that enables
> >> its use.
>
> > ICH4-M databook that I have at hand (Intel IDE PRM seems to be gone
> > from Intel's website, though I'm sure I have a backup _somewhere_) it
> > is called SLV_IDETIM so lets just stick with the current naming for
> > now..
>
> Why not just say that SITRE is a bit, not register? :-)
Good point.. :-)
From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH v2 21/20] ata_piix: add new PIIX_FLAG_* flags
Turn open-coded checks in piix_set_timings() into PIIX_FLAG_* flags
per suggestion from Alan Cox.
Suggested-by: Alan Cox <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
On top of https://lkml.org/lkml/2011/2/8/97 patchset.
v2: s/SITRE register/SITRE bit/ per Sergei's comment
drivers/ata/ata_piix.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
Index: b/drivers/ata/ata_piix.c
===================================================================
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -133,6 +133,8 @@ enum {
PIIX_FLAG_CHECKINTR = (1 << 28), /* make sure PCI INTx enabled */
PIIX_FLAG_SIDPR = (1 << 29), /* SATA idx/data pair regs */
+ PIIX_FLAG_NO_SITRE = (1 << 30), /* no SITRE bit */
+ PIIX_FLAG_RADISYS = (1 << 31), /* host is Radisys R82600 */
PIIX_PATA_FLAGS = ATA_FLAG_SLAVE_POSS,
PIIX_SATA_FLAGS = ATA_FLAG_SATA | PIIX_FLAG_CHECKINTR,
@@ -698,7 +700,7 @@ static struct ata_port_info piix_port_in
[oldpiix_pata] =
{
- .flags = PIIX_PATA_FLAGS,
+ .flags = PIIX_PATA_FLAGS | PIIX_FLAG_NO_SITRE,
.pio_mask = ATA_PIO4,
.mwdma_mask = ATA_MWDMA12_ONLY,
.port_ops = &oldpiix_pata_ops,
@@ -706,7 +708,8 @@ static struct ata_port_info piix_port_in
[radisys_pata] =
{
- .flags = PIIX_PATA_FLAGS,
+ .flags = PIIX_PATA_FLAGS | PIIX_FLAG_NO_SITRE |
+ PIIX_FLAG_RADISYS,
.pio_mask = ATA_PIO4,
.mwdma_mask = ATA_MWDMA12_ONLY,
.udma_mask = ATA_UDMA24_ONLY,
@@ -859,10 +862,8 @@ static void piix_set_timings(struct ata_
struct pci_dev *dev = to_pci_dev(ap->host->dev);
unsigned long flags;
unsigned int is_slave = (adev->devno != 0);
- unsigned int is_radisys = (dev->vendor == PCI_VENDOR_ID_RADISYS &&
- dev->device == 0x8201);
- unsigned int has_sitre = (dev->vendor != PCI_VENDOR_ID_INTEL ||
- dev->device != 0x1230) && !is_radisys;
+ unsigned int is_radisys = (ap->flags & PIIX_FLAG_RADISYS) ? 1 : 0;
+ unsigned int has_sitre = (ap->flags & PIIX_FLAG_NO_SITRE) ? 0 : 1;
unsigned int master_port= ap->port_no ? 0x42 : 0x40;
unsigned int slave_port = 0x44;
u16 master_data;
Hello.
On 08-02-2011 21:18, Bartlomiej Zolnierkiewicz wrote:
>>> Turn open-coded checks in piix_set_timings() into PIIX_FLAG_* flags
>>> per suggestion from Alan Cox.
>>> Suggested-by: Alan Cox<[email protected]>
>>> Signed-off-by: Bartlomiej Zolnierkiewicz<[email protected]>
>> [...]
>>> Index: b/drivers/ata/ata_piix.c
>>> ===================================================================
>>> --- a/drivers/ata/ata_piix.c
>>> +++ b/drivers/ata/ata_piix.c
>>> @@ -133,6 +133,8 @@ enum {
>>> PIIX_FLAG_CHECKINTR = (1<< 28), /* make sure PCI INTx enabled
>>> */
>>> PIIX_FLAG_SIDPR = (1<< 29), /* SATA idx/data pair regs */
>>> + PIIX_FLAG_NO_SITRE = (1<< 30), /* no SITRE register */
>> The register in question is called SIDETIM, SITRE is a bit that enables
>> its use.
> ICH4-M databook that I have at hand (Intel IDE PRM seems to be gone
> from Intel's website, though I'm sure I have a backup _somewhere_) it
If you mean ICH IDE Controller PRM (29860004.pdf), it indeed calls the
register *SITR* (yet it has SIDETIM mentioned in the table 37 :-). Intel seems
not very consistent in its documentation. :-)
> Thanks,
> Bartlomiej
WBR, Sergei