Message ID | 1315336919-2737-1-git-send-email-dpmcgee@gmail.com |
---|---|
State | Not Applicable |
Delegated to: | David Miller |
Headers | show |
On Wed, Sep 7, 2011 at 4:58 AM, Sergei Shtylyov <sshtylyov@mvista.com> wrote: > Hello. > > On 06-09-2011 23:21, Dan McGee wrote: > >> This is similar to the existing sis_old_port_base() method. We do this >> same calculation and logic in multiple places (with one more to come in >> a future patch), so extracting it into a method makes sense. > >> Reviewed-by: Bartlomiej Zolnierkiewicz<bzolnier@gmail.com> >> Signed-off-by: Dan McGee<dpmcgee@gmail.com> >> --- >> drivers/ata/pata_sis.c | 46 >> ++++++++++++++++++++++++++++------------------ >> 1 files changed, 28 insertions(+), 18 deletions(-) > >> diff --git a/drivers/ata/pata_sis.c b/drivers/ata/pata_sis.c >> index 533f2ae..9374400 100644 >> --- a/drivers/ata/pata_sis.c >> +++ b/drivers/ata/pata_sis.c >> @@ -89,6 +89,29 @@ static int sis_old_port_base(struct ata_device *adev) >> + /* If bit 14 is set then the registers are mapped at 0x70 not 0x40 >> */ >> + pci_read_config_dword(pdev, 0x54,®54); >> + if (reg54 & 0x40000000) > > This is bit 30, not bit 14. You've snipped the rest of the patch where I simply cut this comment from the three other places it was stated. I'm simply moving code and text around here. I will update the apparently erroneous comment. > >> + port = 0x70; >> + >> + return port + (8 * ap->port_no) + (4 * adev->devno); > > Why two spaces at some places? It is from the odd style of the method right above it. This driver has a lot of odd styling. I'll "fix" it. > > [...] >> >> @@ -465,21 +482,14 @@ static void sis_133_early_set_dmamode (struct >> ata_port *ap, struct ata_device *a >> static void sis_133_set_dmamode (struct ata_port *ap, struct ata_device >> *adev) >> { >> struct pci_dev *pdev = to_pci_dev(ap->host->dev); >> - int speed = adev->dma_mode - XFER_MW_DMA_0; > > Unrelated change. In context this change makes perfect sense- I'm cleaning up the rest of the locals after extracting the method, and this one is out of place and a totally dead assignment. > > [...] >> >> @@ -487,7 +497,7 @@ static void sis_133_set_dmamode (struct ata_port *ap, >> struct ata_device *adev) >> /* FIXME: need data sheet to add MWDMA here. Also lacking >> on >> ide/pci driver */ >> } else { >> - speed = adev->dma_mode - XFER_UDMA_0; >> + int speed = adev->dma_mode - XFER_UDMA_0; > > Same here. > > WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/ata/pata_sis.c b/drivers/ata/pata_sis.c index 533f2ae..9374400 100644 --- a/drivers/ata/pata_sis.c +++ b/drivers/ata/pata_sis.c @@ -89,6 +89,29 @@ static int sis_old_port_base(struct ata_device *adev) } /** + * sis_port_base - return PCI configuration base for dev + * @adev: device + * + * Returns the base of the PCI configuration registers for this port + * number. + */ + +static int sis_port_base(struct ata_device *adev) +{ + struct ata_port *ap = adev->link->ap; + struct pci_dev *pdev = to_pci_dev(ap->host->dev); + int port = 0x40; + u32 reg54; + + /* If bit 14 is set then the registers are mapped at 0x70 not 0x40 */ + pci_read_config_dword(pdev, 0x54, ®54); + if (reg54 & 0x40000000) + port = 0x70; + + return port + (8 * ap->port_no) + (4 * adev->devno); +} + +/** * sis_133_cable_detect - check for 40/80 pin * @ap: Port * @deadline: deadline jiffies for the operation @@ -266,9 +289,8 @@ static void sis_100_set_piomode (struct ata_port *ap, struct ata_device *adev) static void sis_133_set_piomode (struct ata_port *ap, struct ata_device *adev) { struct pci_dev *pdev = to_pci_dev(ap->host->dev); - int port = 0x40; + int port; u32 t1; - u32 reg54; int speed = adev->pio_mode - XFER_PIO_0; const u32 timing133[] = { @@ -288,12 +310,7 @@ static void sis_133_set_piomode (struct ata_port *ap, struct ata_device *adev) sis_set_fifo(ap, adev); - /* If bit 14 is set then the registers are mapped at 0x70 not 0x40 */ - pci_read_config_dword(pdev, 0x54, ®54); - if (reg54 & 0x40000000) - port = 0x70; - port += 8 * ap->port_no + 4 * adev->devno; - + port = sis_port_base(adev); pci_read_config_dword(pdev, port, &t1); t1 &= 0xC0C00FFF; /* Mask out timing */ @@ -465,21 +482,14 @@ static void sis_133_early_set_dmamode (struct ata_port *ap, struct ata_device *a static void sis_133_set_dmamode (struct ata_port *ap, struct ata_device *adev) { struct pci_dev *pdev = to_pci_dev(ap->host->dev); - int speed = adev->dma_mode - XFER_MW_DMA_0; - int port = 0x40; + int port; u32 t1; - u32 reg54; /* bits 4- cycle time 8 - cvs time */ static const u32 timing_u100[] = { 0x6B0, 0x470, 0x350, 0x140, 0x120, 0x110, 0x000 }; static const u32 timing_u133[] = { 0x9F0, 0x6A0, 0x470, 0x250, 0x230, 0x220, 0x210 }; - /* If bit 14 is set then the registers are mapped at 0x70 not 0x40 */ - pci_read_config_dword(pdev, 0x54, ®54); - if (reg54 & 0x40000000) - port = 0x70; - port += (8 * ap->port_no) + (4 * adev->devno); - + port = sis_port_base(adev); pci_read_config_dword(pdev, port, &t1); if (adev->dma_mode < XFER_UDMA_0) { @@ -487,7 +497,7 @@ static void sis_133_set_dmamode (struct ata_port *ap, struct ata_device *adev) /* FIXME: need data sheet to add MWDMA here. Also lacking on ide/pci driver */ } else { - speed = adev->dma_mode - XFER_UDMA_0; + int speed = adev->dma_mode - XFER_UDMA_0; /* if & 8 no UDMA133 - need info for ... */ t1 &= ~0x00000FF0; t1 |= 0x00000004;