Home » Mailing list archives » linux-kernel » 2007 » October » 13

Re: [git patches] IDE updates (part 2)

Previous thread: [patch 3/8] m68k: ignore restart_syscall by Geert Uytterhoeven on Saturday, October 13, 2007 - 5:31 am. (2 messages)

Next thread: [PATCH] mmc: possible leak in mmc_read_ext_csd by Florin Malita on Saturday, October 13, 2007 - 9:27 am. (2 messages)
[view original message]
From: Bartlomiej Zolnierkiewicz
Subject: [git patches] IDE updates (part 2)
Date: Saturday, October 13, 2007 - 9:25 am

Hi,

highlights of this update:

* Rework of IDE PMAC host driver: bugfixes, removal of the code
  duplicated from the IDE core and conversion to use the generic
  DMA tuning code path (the rework cuts ide-pmac.c by ~200 LOC).

  Thanks to Ben Herrenschmidt for help with it.

* Move setting device transfer mode from host drivers to the core
  code, this makes IDE host drivers very similar to libata one's
  w.r.t. PIO/DMA tuning and cuts another ~200 LOC from IDE code.

* Backport cable fix from libata (original fix by Jeff & Tejun)
  and remove no longer needed CONFIG_IDEDMA_IVB config option.

* More host driver fixes.


Please pull from:

master.kernel.org:/pub/scm/linux/kernel/git/bart/ide-2.6.git/

to receive the following updates:

 drivers/ide/Kconfig               |   16 --
 drivers/ide/arm/icside.c          |   45 +-----
 drivers/ide/cris/ide-cris.c       |    8 +-
 drivers/ide/ide-acpi.c            |    1 -
 drivers/ide/ide-dma.c             |   28 ++--
 drivers/ide/ide-io.c              |   10 +-
 drivers/ide/ide-iops.c            |  133 +++++----------
 drivers/ide/ide-lib.c             |   78 +++++++--
 drivers/ide/ide-probe.c           |    7 +-
 drivers/ide/ide.c                 |    2 +-
 drivers/ide/legacy/ide_platform.c |    2 +-
 drivers/ide/mips/au1xxx-ide.c     |   28 +---
 drivers/ide/pci/aec62xx.c         |   12 +-
 drivers/ide/pci/alim15x3.c        |   58 +++-----
 drivers/ide/pci/amd74xx.c         |   26 +--
 drivers/ide/pci/atiixp.c          |   33 ++---
 drivers/ide/pci/cmd64x.c          |    9 +-
 drivers/ide/pci/cs5520.c          |   32 ++---
 drivers/ide/pci/cs5530.c          |   50 +-----
 drivers/ide/pci/cs5535.c          |   38 ++---
 drivers/ide/pci/hpt34x.c          |    9 +-
 drivers/ide/pci/hpt366.c          |   18 +--
 drivers/ide/pci/it8213.c          |   34 ++---
 drivers/ide/pci/it821x.c          |   90 ++++-------
 drivers/ide/pci/jmicron.c         |   15 +-
 drivers/ide/pci/pdc202xx_new.c    |   24 ++--
 ...
[ message continues ]
[view original message]
From: Alan Cox
Subject: Re: [git patches] IDE updates (part 2)
Date: Saturday, October 13, 2007 - 12:56 pm

On Sat, 13 Oct 2007 18:25:24 +0200

Reading the current driver from the git tree I don't see how the PCI
driver ever sets the ctl register base. It seems to always be set to zero
which means you can't issue SRST and reset sequences ?

-


[ message continues ]
[view original message]
From: Bartlomiej Zolnierkiewicz
Subject: Re: [git patches] IDE updates (part 2)
Date: Saturday, October 13, 2007 - 1:59 pm

Hi Alan,

Comment in pmac_ide_init_hwif_ports() is highly misleading as this function
returns early only for "normal" IDE PCI devices (pmac_ide_init_hwif_ports()
can be called outside ide-pmac driver through ppc_ide_md).

static int __devinit
pmac_ide_pci_attach(struct pci_dev *pdev, const struct pci_device_id *id)
{
...
	pmif = &pmac_ide[i];
...
	pmif->regbase = (unsigned long) base + 0x2000;
...
	rc = pmac_ide_setup_device(pmif, hwif);
...
}

static int
pmac_ide_setup_device(pmac_ide_hwif_t *pmif, ide_hwif_t *hwif)
{
...
	pmac_ide_init_hwif_ports(&hwif->hw, pmif->regbase, 0, &hwif->irq);
...
}

void
pmac_ide_init_hwif_ports(hw_regs_t *hw,
			      unsigned long data_port, unsigned long ctrl_port,
			      int *irq)
{
...
	for (ix = 0; ix < MAX_HWIFS; ++ix)
		if (data_port == pmac_ide[ix].regbase)
			break;
---> since pmif->regbase was set earlier ix will be < MAX_HWIFS
	if (ix >= MAX_HWIFS) {
		/* Probably a PCI interface... */
		for (i = IDE_DATA_OFFSET; i <= IDE_STATUS_OFFSET; ++i)
			hw->io_ports[i] = data_port + i - IDE_DATA_OFFSET;
		hw->io_ports[IDE_CONTROL_OFFSET] = ctrl_port;
		return;
	}

	for (i = 0; i < 8; ++i)
		hw->io_ports[i] = data_port + i * 0x10;
---> ctl register base will be set here
	hw->io_ports[8] = data_port + 0x160;
...
}

Bart
-


[ message continues ]
[view original message]
From: Alan Cox
Subject: Re: [git patches] IDE updates (part 2)
Date: Saturday, October 13, 2007 - 2:44 pm

Which is zero..


See the problem ?

Alan
-


[ message continues ]
[view original message]
From: Bartlomiej Zolnierkiewicz
Subject: Re: [git patches] IDE updates (part 2)
Date: Saturday, October 13, 2007 - 3:09 pm

-


[ message continues ]
[view original message]
From: Alan Cox
Subject: Re: [git patches] IDE updates (part 2)
Date: Saturday, October 13, 2007 - 3:29 pm

Ok so in actual fact

- The piece of code above can't be executed anyway
- The ctrl_port argument is not needed ?

Alan
-


[ message continues ]
[view original message]
From: Bartlomiej Zolnierkiewicz
Subject: Re: [git patches] IDE updates (part 2)
Date: Saturday, October 13, 2007 - 3:41 pm

pmac_ide_init_hwif_ports() is also called by ide_init_hwif_ports()
through ppc_ide_md.init_hwif.

Bart
-


[ message continues ]
[view original message]
From: Benjamin Herrenschmidt
Subject: Re: [git patches] IDE updates (part 2)
Date: Saturday, October 13, 2007 - 5:08 pm

In which case it should be called with a ctrl_port right ?

Ben.


-


[ message continues ]
[view original message]
From: Bartlomiej Zolnierkiewicz
Subject: Re: [git patches] IDE updates (part 2)
Date: Sunday, October 14, 2007 - 9:07 am

Yep.

How's about this patch?

[PATCH] ide-pmac: fix pmac_ide_init_hwif_ports()

* pmac_ide_init_hwif_ports() can be called by ide_init_hwif_ports()
  (through ppc_ide_md.ide_init_hwif hook) for non IDE PMAC interfaces.
  If this is the case the hw->io_ports[] should be already setup by
  ide_init_hwif_ports()->ide_std_init_ports() so remove redundant code
  from pmac_ide_init_hwif_ports().

  As side-effect this change fixes ctl_addr == 0 special handling in
  ide_init_hwif_ports().

* Fix misleading comment while at it.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ppc/pmac.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

Index: b/drivers/ide/ppc/pmac.c
===================================================================
--- a/drivers/ide/ppc/pmac.c
+++ b/drivers/ide/ppc/pmac.c
@@ -438,13 +438,8 @@ pmac_ide_init_hwif_ports(hw_regs_t *hw,
 		if (data_port == pmac_ide[ix].regbase)
 			break;
 
-	if (ix >= MAX_HWIFS) {
-		/* Probably a PCI interface... */
-		for (i = IDE_DATA_OFFSET; i <= IDE_STATUS_OFFSET; ++i)
-			hw->io_ports[i] = data_port + i - IDE_DATA_OFFSET;
-		hw->io_ports[IDE_CONTROL_OFFSET] = ctrl_port;
-		return;
-	}
+	if (ix >= MAX_HWIFS)
+		return;		/* not an IDE PMAC interface */
 
 	for (i = 0; i < 8; ++i)
 		hw->io_ports[i] = data_port + i * 0x10;
-


[ message continues ]
[view original message]
From: Benjamin Herrenschmidt
Subject: Re: [git patches] IDE updates (part 2)
Date: Sunday, October 14, 2007 - 2:15 pm

I would have to try it. Problem is, I don't actually have any powermac
with a PCI IDE controller at hand.. ouch. I'll see what I can find.

Ben.


-


[ message continues ]
[view original message]
From: Bartlomiej Zolnierkiewicz
Subject: Re: [git patches] IDE updates (part 2)
Date: Tuesday, October 16, 2007 - 12:41 pm

Same problem here, would be great if somebody with PMAC could test
this patch (should apply cleanly to Linus' tree).

Well, since the change is pretty safe I'm adding the patch to IDE tree
(so it hopefully gets tested through -mm).

Thanks,
Bart
-


[ message continues ]
[view original message]
From: Al Viro
Subject: Re: [git patches] IDE updates (part 2)
Date: Saturday, October 13, 2007 - 5:12 pm

Proposed addition to icside part, provided that ARM folks ACK it - gets
icside to build and AFAICS it's correct:

diff --git a/drivers/ata/pata_icside.c b/drivers/ata/pata_icside.c
index be30923..842fe08 100644
--- a/drivers/ata/pata_icside.c
+++ b/drivers/ata/pata_icside.c
@@ -332,12 +332,13 @@ static void ata_dummy_noret(struct ata_port *port)
 {
 }
 
-static void pata_icside_postreset(struct ata_port *ap, unsigned int *classes)
+static void pata_icside_postreset(struct ata_link *link, unsigned int *classes)
 {
+	struct ata_port *ap = link->ap;
 	struct pata_icside_state *state = ap->host->private_data;
 
 	if (classes[0] != ATA_DEV_NONE || classes[1] != ATA_DEV_NONE)
-		return ata_std_postreset(ap, classes);
+		return ata_std_postreset(link, classes);
 
 	state->port[ap->port_no].disabled = 1;
 
@@ -395,29 +396,30 @@ static struct ata_port_operations pata_icside_port_ops = {
 
 static void __devinit
 pata_icside_setup_ioaddr(struct ata_port *ap, void __iomem *base,
-			 const struct portinfo *info)
+			 struct pata_icside_info *info,
+			 const struct portinfo *port)
 {
 	struct ata_ioports *ioaddr = &ap->ioaddr;
-	void __iomem *cmd = base + info->dataoffset;
+	void __iomem *cmd = base + port->dataoffset;
 
 	ioaddr->cmd_addr	= cmd;
-	ioaddr->data_addr	= cmd + (ATA_REG_DATA    << info->stepping);
-	ioaddr->error_addr	= cmd + (ATA_REG_ERR     << info->stepping);
-	ioaddr->feature_addr	= cmd + (ATA_REG_FEATURE << info->stepping);
-	ioaddr->nsect_addr	= cmd + (ATA_REG_NSECT   << info->stepping);
-	ioaddr->lbal_addr	= cmd + (ATA_REG_LBAL    << info->stepping);
-	ioaddr->lbam_addr	= cmd + (ATA_REG_LBAM    << info->stepping);
-	ioaddr->lbah_addr	= cmd + (ATA_REG_LBAH    << info->stepping);
-	ioaddr->device_addr	= cmd + (ATA_REG_DEVICE  << info->stepping);
-	ioaddr->status_addr	= cmd + (ATA_REG_STATUS  << info->stepping);
-	ioaddr->command_addr	= cmd + (ATA_REG_CMD     << info->stepping);
-
-	ioaddr->ctl_addr	= base + info->ctrloffset;
+	ioaddr->data_addr	= cmd + ...
[ message continues ]
[view original message]
From: Bartlomiej Zolnierkiewicz
Subject: Re: [git patches] IDE updates (part 2)
Date: Sunday, October 14, 2007 - 9:21 am

-


[ message continues ]
[view original message]
From: Jeff Garzik
Subject: Re: [git patches] IDE updates (part 2)
Date: Monday, October 15, 2007 - 12:54 pm

ACK from me too...  ARM folks?




-


[ message continues ]
[view original message]
From: Russell King
Subject: Re: [git patches] IDE updates (part 2)
Date: Monday, October 15, 2007 - 12:58 pm

Will test tomorrow/Wednesday.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
-


[ message continues ]
[view original message]
From: Jeff Garzik
Subject: Re: [git patches] IDE updates (part 2)
Date: Monday, October 15, 2007 - 1:07 pm

Thanks.  Sorry about missing your CC on the libata-wide ata_link 
changes.  We should have poked the maintainers on that, for the drivers 
we cannot build ourselves.




-


[ message continues ]
[view original message]
From: Russell King
Subject: Re: [git patches] IDE updates (part 2)
Date: Wednesday, October 24, 2007 - 5:38 am

I think Viro's patch got an ack from everyone.  Who's dealing with getting
this regression fix into mainline?  It seems at the moment the patch is in
limbo.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
-


[ message continues ]
[view original message]
From: Jeff Garzik
Subject: Re: [git patches] IDE updates (part 2)
Date: Wednesday, October 24, 2007 - 5:53 am

I was waiting on you, who last said "Will test tomorrow/Wednesday" (with 
presumably an ACK forthcoming after test results)

If it now has your ACK, it's ready and waiting in my mbox queue to be 
merged upstream.

	Jeff



-


[ message continues ]
[view original message]
From: Russell King
Subject: Re: [git patches] IDE updates (part 2)
Date: Wednesday, October 24, 2007 - 6:12 am

Looks like it got my ack more than a week ago:

| Date: Tue, 16 Oct 2007 16:55:07 +0100
| From: Russell King <rmk+lkml@arm.linux.org.uk>
| To: Al Viro <viro@ftp.linux.org.uk>
| Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>,
|         Linus Torvalds <torvalds@linux-foundation.org>,
|         linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
| Subject: Re: [git patches] IDE updates (part 2)
| 
| On Sun, Oct 14, 2007 at 01:12:39AM +0100, Al Viro wrote:
| > Proposed addition to icside part, provided that ARM folks ACK it - gets
| > icside to build and AFAICS it's correct:
| 
| Booted and tested as working.
| 
| Whatever-tag-that-is: Russell King <rmk+kernel@arm.linux.org.uk>

which I gave to the _initial_ message asking for my ack.  And no, before
you whinge and moan, I didn't drop any CC's.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
-


[ message continues ]
[view original message]
From: Jeff Garzik
Subject: Re: [git patches] IDE updates (part 2)
Date: Wednesday, October 24, 2007 - 6:20 am

Groovy.

	Jeff



-


[ message continues ]
[view original message]
From: Russell King
Subject: Re: [git patches] IDE updates (part 2)
Date: Tuesday, October 16, 2007 - 8:55 am

Booted and tested as working.


-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
-


[ message continues ]
[view original message]
From: Jeff Garzik
Subject: Re: [git patches] IDE updates (part 2)
Date: Wednesday, October 24, 2007 - 11:05 pm

applied


-


[ message continues ]
Previous thread: [patch 3/8] m68k: ignore restart_syscall by Geert Uytterhoeven on Saturday, October 13, 2007 - 5:31 am. (2 messages)

Next thread: [PATCH] mmc: possible leak in mmc_read_ext_csd by Florin Malita on Saturday, October 13, 2007 - 9:27 am. (2 messages)