[PATCH 1/4] ide-pmac: media-bay support fixes

[PATCH 1/4] ide-pmac: media-bay support fixes

[Bartlomiej Zolnierkiewicz]

* If MB_CD device has already been detected and bay is in mb_up state just
  change bay's state to mb_ide_resetting and let probing thread do the rest
  instead of having open-coded waiting for IDE device to become ready in
  media_bay_set_ide_infos() and doing the probe by ide_device_add().

* Move media_bay_set_ide_infos() call after ide_device_add().

* Use check_media_bay() instead of check_media_bay_by_base(),
  then remove the latter function.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
unchanged patch

 drivers/ide/ppc/pmac.c         |   18 ++++++++----------
 drivers/macintosh/mediabay.c   |   33 +++++----------------------------
 include/asm-powerpc/mediabay.h |    1 -
 3 files changed, 13 insertions(+), 39 deletions(-)

Index: b/drivers/ide/ppc/pmac.c
===================================================================
--- a/drivers/ide/ppc/pmac.c
+++ b/drivers/ide/ppc/pmac.c
@@ -1030,10 +1030,6 @@ pmac_ide_setup_device(pmac_ide_hwif_t *p
 	/* XXX FIXME: Media bay stuff need re-organizing */
 	if (np->parent && np->parent->name
 	    && strcasecmp(np->parent->name, "media-bay") == 0) {
-#ifdef CONFIG_PMAC_MEDIABAY
-		media_bay_set_ide_infos(np->parent, pmif->regbase, pmif->irq,
-					hwif);
-#endif /* CONFIG_PMAC_MEDIABAY */
 		pmif->mediabay = 1;
 		if (!bidp)
 			pmif->aapl_bus_id = 1;
@@ -1067,19 +1063,21 @@ pmac_ide_setup_device(pmac_ide_hwif_t *p
 
 	if (pmif->mediabay) {
 #ifdef CONFIG_PMAC_MEDIABAY
-		if (check_media_bay_by_base(pmif->regbase, MB_CD)) {
-#else
-		if (1) {
+		if (check_media_bay(np->parent, MB_CD) == -ENODEV)
+			break;
 #endif
-			hwif->drives[0].noprobe = 1;
-			hwif->drives[1].noprobe = 1;
-		}
+		hwif->drives[0].noprobe = 1;
+		hwif->drives[1].noprobe = 1;
 	}
 
 	idx[0] = hwif->index;
 
 	ide_device_add(idx, &d);
 
+#ifdef CONFIG_PMAC_MEDIABAY
+	media_bay_set_ide_infos(np->parent, pmif->regbase, pmif->irq, hwif);
+#endif
+
 	return 0;
 }
 
Index: b/drivers/macintosh/mediabay.c
===================================================================
--- a/drivers/macintosh/mediabay.c
+++ b/drivers/macintosh/mediabay.c
@@ -433,21 +433,6 @@ int check_media_bay(struct device_node *
 EXPORT_SYMBOL(check_media_bay);
 
 #ifdef CONFIG_BLK_DEV_IDE_PMAC
-int check_media_bay_by_base(unsigned long base, int what)
-{
-	int	i;
-
-	for (i=0; i<media_bay_count; i++)
-		if (media_bays[i].mdev && base == (unsigned long) media_bays[i].cd_base) {
-			if ((what == media_bays[i].content_id) && media_bays[i].state == mb_up)
-				return 0;
-			media_bays[i].cd_index = -1;
-			return -EINVAL;
-		} 
-
-	return -ENODEV;
-}
-
 int media_bay_set_ide_infos(struct device_node* which_bay, unsigned long base,
 			    int irq, ide_hwif_t *hwif)
 {
@@ -457,8 +442,6 @@ int media_bay_set_ide_infos(struct devic
 		struct media_bay_info* bay = &media_bays[i];
 
 		if (bay->mdev && which_bay == bay->mdev->ofdev.node) {
-			int timeout = 5000, index = hwif->index;
-			
 			down(&bay->lock);
 
 			bay->cd_port	= hwif;
@@ -469,18 +452,12 @@ int media_bay_set_ide_infos(struct devic
 				up(&bay->lock);
 				return 0;
 			}
-			printk(KERN_DEBUG "Registered ide%d for media bay %d\n", index, i);
-			do {
-				if (MB_IDE_READY(i)) {
-					bay->cd_index	= index;
-					up(&bay->lock);
-					return 0;
-				}
-				mdelay(1);
-			} while(--timeout);
-			printk(KERN_DEBUG "Timeount waiting IDE in bay %d\n", i);
+
+			/* let probing thread do the rest */
+			bay->state = mb_ide_resetting;
+
 			up(&bay->lock);
-			return -ENODEV;
+			return 0;
 		}
 	}
 
Index: b/include/asm-powerpc/mediabay.h
===================================================================
--- a/include/asm-powerpc/mediabay.h
+++ b/include/asm-powerpc/mediabay.h
@@ -25,7 +25,6 @@ extern int media_bay_count;
 #ifdef CONFIG_BLK_DEV_IDE_PMAC
 #include <linux/ide.h>
 
-int check_media_bay_by_base(unsigned long base, int what);
 /* called by IDE PMAC host driver to register IDE controller for media bay */
 int media_bay_set_ide_infos(struct device_node *which_bay, unsigned long base,
 			    int irq, ide_hwif_t *hwif);

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Benjamin Herrenschmidt]

Your patches don't apply on today upstream, I'll try against
linux-next or is there a git I can clone to test things ?

Cheers,
Ben.

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Benjamin Herrenschmidt]

On Tue, 2008-06-17 at 13:39 +1000, Benjamin Herrenschmidt wrote:
> Your patches don't apply on today upstream, I'll try against
> linux-next or is there a git I can clone to test things ?

Ok, I'm lost. I have 3 different series of patches from you partially
overlapping with different amount of patches in them. I don't have
time to sort that out right now. Can you send me a single serie that
I can apply on top of either upstream or tonight linux-next which
should have your stuff removed hopefully ?

Ben.

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Bartlomiej Zolnierkiewicz]

On Tuesday 17 June 2008, Benjamin Herrenschmidt wrote:
> On Tue, 2008-06-17 at 13:39 +1000, Benjamin Herrenschmidt wrote:
> > Your patches don't apply on today upstream, I'll try against
> > linux-next or is there a git I can clone to test things ?
> 
> Ok, I'm lost. I have 3 different series of patches from you partially
> overlapping with different amount of patches in them. I don't have
> time to sort that out right now. Can you send me a single serie that
> I can apply on top of either upstream or tonight linux-next which
> should have your stuff removed hopefully ?

This patch series (from yesterday, the rest of can be ignored) should
apply fine to the next revision of linux-next (later than -next-20080616,
it is not out yet), if you prefer upstream patches I can look into it
tonight.

Thanks,
Bart

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Bartlomiej Zolnierkiewicz]

On Tuesday 17 June 2008, Bartlomiej Zolnierkiewicz wrote:
> On Tuesday 17 June 2008, Benjamin Herrenschmidt wrote:
> > On Tue, 2008-06-17 at 13:39 +1000, Benjamin Herrenschmidt wrote:
> > > Your patches don't apply on today upstream, I'll try against
> > > linux-next or is there a git I can clone to test things ?
> > 
> > Ok, I'm lost. I have 3 different series of patches from you partially
> > overlapping with different amount of patches in them. I don't have
> > time to sort that out right now. Can you send me a single serie that
> > I can apply on top of either upstream or tonight linux-next which
> > should have your stuff removed hopefully ?
> 
> This patch series (from yesterday, the rest of can be ignored) should
> apply fine to the next revision of linux-next (later than -next-20080616,
> it is not out yet), if you prefer upstream patches I can look into it

-next-20080617 is out now so please try it

> tonight.
> 
> Thanks,
> Bart

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Benjamin Herrenschmidt]

> This patch series (from yesterday, the rest of can be ignored) should
> apply fine to the next revision of linux-next (later than -next-20080616,
> it is not out yet), if you prefer upstream patches I can look into it
> tonight.

Whatever last patches from you I have in my mailbox (ie. a serie of 4,
one of them being superseeded by a "version 3" that you sent later on)
doesn't apply on current linux next (ie 20080620).

I'll see if I can fix it up.

Can you ever give me something that both -applies- and I don't have to
dig random amount of mailbox content to get to it ?

Cheers,
Ben.

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Benjamin Herrenschmidt]

On Mon, 2008-06-23 at 15:35 +1000, Benjamin Herrenschmidt wrote:
> > This patch series (from yesterday, the rest of can be ignored) should
> > apply fine to the next revision of linux-next (later than -next-20080616,
> > it is not out yet), if you prefer upstream patches I can look into it
> > tonight.
> 
> Whatever last patches from you I have in my mailbox (ie. a serie of 4,
> one of them being superseeded by a "version 3" that you sent later on)
> doesn't apply on current linux next (ie 20080620).
> 
> I'll see if I can fix it up.

Don't panic. It looks like it's something else in linux-next that's
changing some ifdef's in the media-bay code which is causing that.

I managed to pull linux next at the merge point with your tree and
things apply. I'll use that to test.

Cheers,
Ben.

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Benjamin Herrenschmidt]

> Don't panic. It looks like it's something else in linux-next that's
> changing some ifdef's in the media-bay code which is causing that.
> 
> I managed to pull linux next at the merge point with your tree and
> things apply. I'll use that to test.

Ok, it doesn't work properly. It gets error trying to register
the IDE device. Booting with a CD drive in and no disk in the drive
gives the log below.

I've verified that it works without your patches. If I apply only patch
1, it doesn't build due to some wrong construct in the probe code. I've
hand fixed it, but then I hit a BUG_ON in ide_probe_port() (line 773).

mediabay boot time messages (before IDE probing) are:

mediabay0: Registered Heathrow media-bay
mediabay0: powering down
mediabay0: switching to 3
mediabay0: powering up
mediabay0: enabling (kind:3)
mediabay0: waiting reset (kind:3)
mediabay0: waiting IDE reset (kind:3)
mediabay0: waiting IDE ready (kind:3)
mediabay0: up before IDE init
mediabay1: Registered Heathrow media-bay
mediabay1: powering down
mediabay1: switching to 0
mediabay1: powering up
mediabay1: enabling (kind:0)
mediabay1: waiting reset (kind:0)
mediabay1: bay is up (kind:0)

Later, IDE registers:

(ide0 is another controller, only ide1 and ide2 are media bay based, and
ide2 has no device on it at all).

ide-pmac: Found Apple Heathrow ATA controller (macio), bus ID 1 (mediabay), irq  35
ide1 at 0xc7020000-0xc7020070,0xc7020160 on irq 35
ide-pmac: Found Apple Heathrow ATA controller (macio), bus ID 4 (mediabay), irq  67
ide2 at 0xc7022000-0xc7022070,0xc7022160 on irq 67
mediabay0: waiting IDE ready (kind:3)
mediabay 0, registering IDE...
IDE register error
mediabay0: powering down

 .../...

mediabay0: end of power down
mediabay0: switching to 3
mediabay0: powering up
mediabay0: enabling (kind:3)
mediabay0: waiting reset (kind:3)
mediabay0: waiting IDE reset (kind:3)

 .../...

mediabay0: waiting IDE ready (kind:3)
mediabay 0, registering IDE...
IDE register error
mediabay0: powering down
mediabay0: end of power down
mediabay0: switching to 3
mediabay0: powering up
mediabay0: enabling (kind:3)
mediabay0: waiting reset (kind:3)
mediabay0: waiting IDE reset (kind:3)
mediabay0: waiting IDE ready (kind:3)
mediabay 0, registering IDE...
IDE register error
mediabay0: powering down
mediabay0: end of power down
mediabay0: switching to 3
mediabay0: powering up
mediabay0: enabling (kind:3)
mediabay0: waiting reset (kind:3)
mediabay0: waiting IDE reset (kind:3)
mediabay0: waiting IDE ready (kind:3)
mediabay 0, registering IDE...
IDE register error
mediabay0: powering down
media-bay 0, IDE device badly inserted or unrecognised
mediabay0: end of power down

Cheers,
Ben.

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Benjamin Herrenschmidt]

On Mon, 2008-06-23 at 16:41 +1000, Benjamin Herrenschmidt wrote:
> > Don't panic. It looks like it's something else in linux-next that's
> > changing some ifdef's in the media-bay code which is causing that.
> > 
> > I managed to pull linux next at the merge point with your tree and
> > things apply. I'll use that to test.
> 
> Ok, it doesn't work properly. It gets error trying to register
> the IDE device. Booting with a CD drive in and no disk in the drive
> gives the log below.
> 
> I've verified that it works without your patches. If I apply only patch
> 1, it doesn't build due to some wrong construct in the probe code. I've
> hand fixed it, but then I hit a BUG_ON in ide_probe_port() (line 773).
> 
> mediabay boot time messages (before IDE probing) are:

BTW. I didn't debug more today as I ran out of time but I'll do more
tomorrow.

On a side note, we need to change the way the mediabay stuff works. I've
started writing a pata_macio (ie. libata variant of the driver) a while
ago that I need to kick myself into finishing one of these days, and it
will want something different than having the mediabay code poke into
the IDE layer directly :-)

My current though is to add a media bay notification callback to the
macio_device structure (the pci variants of the device are never hooked
up on a media bay) and move the state query function to the macio core,
the media bay driver being then responsible for calling into the macio
core to update the state.

I still have to sort out some interesting locking issues vs. state
changes around driver probe() but I think that's the way to go. Thus,
the drivers/ide driver can do it's old ide core hackery locally and the
libata variant do what is needed for libata locally too.

It's been a bit low on my todo list as Apple stopped making machines
with hotswap mediabays something like 8 years ago :-) The two I have
still working are pretty scary old things and one of them is almost
falling apart..

Cheers,
Ben.

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Bartlomiej Zolnierkiewicz]

On Monday 23 June 2008, Benjamin Herrenschmidt wrote:
> On Mon, 2008-06-23 at 16:41 +1000, Benjamin Herrenschmidt wrote:
> > > Don't panic. It looks like it's something else in linux-next that's
> > > changing some ifdef's in the media-bay code which is causing that.
> > > 
> > > I managed to pull linux next at the merge point with your tree and
> > > things apply. I'll use that to test.
> > 
> > Ok, it doesn't work properly. It gets error trying to register
> > the IDE device. Booting with a CD drive in and no disk in the drive
> > gives the log below.
> > 
> > I've verified that it works without your patches. If I apply only patch
> > 1, it doesn't build due to some wrong construct in the probe code. I've
> > hand fixed it, but then I hit a BUG_ON in ide_probe_port() (line 773).
> > 
> > mediabay boot time messages (before IDE probing) are:
> 
> BTW. I didn't debug more today as I ran out of time but I'll do more
> tomorrow.
> 
> On a side note, we need to change the way the mediabay stuff works. I've
> started writing a pata_macio (ie. libata variant of the driver) a while

I (naively?) hope that you are using ide-pmac as a base (the driver looks
like a normal host driver now) and going to make the final API switch after
hardware specific changes are complete (so we will be able to use git-bisect
instead of guesswork combined with voodoo dance when dealing with potential
regressions :-).

> ago that I need to kick myself into finishing one of these days, and it
> will want something different than having the mediabay code poke into
> the IDE layer directly :-)
> 
> My current though is to add a media bay notification callback to the
> macio_device structure (the pci variants of the device are never hooked
> up on a media bay) and move the state query function to the macio core,
> the media bay driver being then responsible for calling into the macio
> core to update the state.

You may want to look at ACPI hotplug dock handling for inspiration
(if you haven't already).

> I still have to sort out some interesting locking issues vs. state
> changes around driver probe() but I think that's the way to go. Thus,
> the drivers/ide driver can do it's old ide core hackery locally and the
> libata variant do what is needed for libata locally too.

There is no longer any hackery on IDE core code side in 2.6.26, there are
just two nice methods: ide_port_unregister_devices() and ide_scan_port().

Therefore I would prefer to also update ide-pmac for new methods and just
add what is needed (if any) to IDE core code.

[ Please keep me in loop on these mediabay changes. ]

> It's been a bit low on my todo list as Apple stopped making machines
> with hotswap mediabays something like 8 years ago :-) The two I have
> still working are pretty scary old things and one of them is almost
> falling apart..

:)

Bart

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Benjamin Herrenschmidt]

> I (naively?) hope that you are using ide-pmac as a base (the driver looks
> like a normal host driver now) and going to make the final API switch after
> hardware specific changes are complete (so we will be able to use git-bisect
> instead of guesswork combined with voodoo dance when dealing with potential
> regressions :-).

I intend to yes :-) Along with my WIP pata_macio of course but I don't
like leaving a driver broken and/or non bisectable, believe me.

> You may want to look at ACPI hotplug dock handling for inspiration
> (if you haven't already).

Thanks, I will. I've been busy with all sorts of other things so no, I
haven't looked yet, and it's a bit less urgent, but thanks for the
pointer,

> There is no longer any hackery on IDE core code side in 2.6.26, there are
> just two nice methods: ide_port_unregister_devices() and ide_scan_port().

Sure but the fact that mediabay calls them directly makes it painful to
deal with 2 drivers, one using drivers/ide and one libata. So I want to
turn that into a notification into the low level driver so it does what
is appropriate.

> Therefore I would prefer to also update ide-pmac for new methods and just
> add what is needed (if any) to IDE core code.

Sure. I don't think I will need to change any of the core.

> [ Please keep me in loop on these mediabay changes. ]

I'll let you know when I get something done.

Cheers,
Ben.

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Bartlomiej Zolnierkiewicz]

On Monday 23 June 2008, Benjamin Herrenschmidt wrote:
> 
> > Don't panic. It looks like it's something else in linux-next that's
> > changing some ifdef's in the media-bay code which is causing that.
> > 
> > I managed to pull linux next at the merge point with your tree and
> > things apply. I'll use that to test.
> 
> Ok, it doesn't work properly. It gets error trying to register
> the IDE device. Booting with a CD drive in and no disk in the drive
> gives the log below.
> 
> I've verified that it works without your patches. If I apply only patch
> 1, it doesn't build due to some wrong construct in the probe code. I've
> hand fixed it, but then I hit a BUG_ON in ide_probe_port() (line 773).

Build error and/or your build fix would be useful here.

> mediabay boot time messages (before IDE probing) are:
> 
> mediabay0: Registered Heathrow media-bay
> mediabay0: powering down
> mediabay0: switching to 3
> mediabay0: powering up
> mediabay0: enabling (kind:3)
> mediabay0: waiting reset (kind:3)
> mediabay0: waiting IDE reset (kind:3)
> mediabay0: waiting IDE ready (kind:3)
> mediabay0: up before IDE init
> mediabay1: Registered Heathrow media-bay
> mediabay1: powering down
> mediabay1: switching to 0
> mediabay1: powering up
> mediabay1: enabling (kind:0)
> mediabay1: waiting reset (kind:0)
> mediabay1: bay is up (kind:0)
> 
> Later, IDE registers:
> 
> (ide0 is another controller, only ide1 and ide2 are media bay based, and
> ide2 has no device on it at all).
> 
> ide-pmac: Found Apple Heathrow ATA controller (macio), bus ID 1 (mediabay), irq  35
> ide1 at 0xc7020000-0xc7020070,0xc7020160 on irq 35
> ide-pmac: Found Apple Heathrow ATA controller (macio), bus ID 4 (mediabay), irq  67
> ide2 at 0xc7022000-0xc7022070,0xc7022160 on irq 67
> mediabay0: waiting IDE ready (kind:3)
> mediabay 0, registering IDE...
> IDE register error

Ok, I see the problem - we now need to clear dive->noprobe after
ide_device_add() call (pmac.c) or later ide_port_scan() call (mediabay.c)
will fail...  also there was a pmif->mediabay check missing...

Please try the new version of patch #1 (the other patches are unchanged):

[...]

From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [PATCH] ide-pmac: media-bay support fixes (take 2)

* If MB_CD device has already been detected and bay is in mb_up state just
  change bay's state to mb_ide_resetting and let probing thread do the rest
  instead of having open-coded waiting for IDE device to become ready in
  media_bay_set_ide_infos() and doing the probe by ide_device_add().

* Move media_bay_set_ide_infos() call after ide_device_add().

* Use check_media_bay() instead of check_media_bay_by_base(),
  then remove the latter function.

v2:
* Check pmif->mediabay and clear drive->noprobe before calling
  media_bay_set_ide_infos().

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ppc/pmac.c         |   23 +++++++++++++----------
 drivers/macintosh/mediabay.c   |   33 +++++----------------------------
 include/asm-powerpc/mediabay.h |    1 -
 3 files changed, 18 insertions(+), 39 deletions(-)

Index: b/drivers/ide/ppc/pmac.c
===================================================================
--- a/drivers/ide/ppc/pmac.c
+++ b/drivers/ide/ppc/pmac.c
@@ -1030,10 +1030,6 @@ pmac_ide_setup_device(pmac_ide_hwif_t *p
 	/* XXX FIXME: Media bay stuff need re-organizing */
 	if (np->parent && np->parent->name
 	    && strcasecmp(np->parent->name, "media-bay") == 0) {
-#ifdef CONFIG_PMAC_MEDIABAY
-		media_bay_set_ide_infos(np->parent, pmif->regbase, pmif->irq,
-					hwif);
-#endif /* CONFIG_PMAC_MEDIABAY */
 		pmif->mediabay = 1;
 		if (!bidp)
 			pmif->aapl_bus_id = 1;
@@ -1067,19 +1063,26 @@ pmac_ide_setup_device(pmac_ide_hwif_t *p
 
 	if (pmif->mediabay) {
 #ifdef CONFIG_PMAC_MEDIABAY
-		if (check_media_bay_by_base(pmif->regbase, MB_CD)) {
-#else
-		if (1) {
+		if (check_media_bay(np->parent, MB_CD) == -ENODEV)
+			break;
 #endif
-			hwif->drives[0].noprobe = 1;
-			hwif->drives[1].noprobe = 1;
-		}
+		hwif->drives[0].noprobe = 1;
+		hwif->drives[1].noprobe = 1;
 	}
 
 	idx[0] = hwif->index;
 
 	ide_device_add(idx, &d);
 
+	if (pmif->mediabay) {
+		hwif->drives[0].noprobe = 0;
+		hwif->drives[1].noprobe = 0;
+#ifdef CONFIG_PMAC_MEDIABAY
+		media_bay_set_ide_infos(np->parent, pmif->regbase, pmif->irq,
+					hwif);
+#endif
+	}
+
 	return 0;
 }
 
Index: b/drivers/macintosh/mediabay.c
===================================================================
--- a/drivers/macintosh/mediabay.c
+++ b/drivers/macintosh/mediabay.c
@@ -433,21 +433,6 @@ int check_media_bay(struct device_node *
 }
 EXPORT_SYMBOL(check_media_bay);
 
-int check_media_bay_by_base(unsigned long base, int what)
-{
-	int	i;
-
-	for (i=0; i<media_bay_count; i++)
-		if (media_bays[i].mdev && base == (unsigned long) media_bays[i].cd_base) {
-			if ((what == media_bays[i].content_id) && media_bays[i].state == mb_up)
-				return 0;
-			media_bays[i].cd_index = -1;
-			return -EINVAL;
-		} 
-
-	return -ENODEV;
-}
-
 int media_bay_set_ide_infos(struct device_node* which_bay, unsigned long base,
 			    int irq, ide_hwif_t *hwif)
 {
@@ -457,8 +442,6 @@ int media_bay_set_ide_infos(struct devic
 		struct media_bay_info* bay = &media_bays[i];
 
 		if (bay->mdev && which_bay == bay->mdev->ofdev.node) {
-			int timeout = 5000, index = hwif->index;
-			
 			down(&bay->lock);
 
 			bay->cd_port	= hwif;
@@ -469,18 +452,12 @@ int media_bay_set_ide_infos(struct devic
 				up(&bay->lock);
 				return 0;
 			}
-			printk(KERN_DEBUG "Registered ide%d for media bay %d\n", index, i);
-			do {
-				if (MB_IDE_READY(i)) {
-					bay->cd_index	= index;
-					up(&bay->lock);
-					return 0;
-				}
-				mdelay(1);
-			} while(--timeout);
-			printk(KERN_DEBUG "Timeount waiting IDE in bay %d\n", i);
+
+			/* let probing thread do the rest */
+			bay->state = mb_ide_resetting;
+
 			up(&bay->lock);
-			return -ENODEV;
+			return 0;
 		}
 	}
 
Index: b/include/asm-powerpc/mediabay.h
===================================================================
--- a/include/asm-powerpc/mediabay.h
+++ b/include/asm-powerpc/mediabay.h
@@ -23,7 +23,6 @@ extern int media_bay_count;
 #ifdef CONFIG_BLK_DEV_IDE_PMAC
 #include <linux/ide.h>
 
-int check_media_bay_by_base(unsigned long base, int what);
 /* called by IDE PMAC host driver to register IDE controller for media bay */
 int media_bay_set_ide_infos(struct device_node *which_bay, unsigned long base,
 			    int irq, ide_hwif_t *hwif);

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Benjamin Herrenschmidt]

> Build error and/or your build fix would be useful here.

Sorry. Fix was a quick hack. There is a check_media_bay() followed by a
break; outside of a break-able construct in there (off memory, I'm not
in front of the dev box right now).

> Ok, I see the problem - we now need to clear dive->noprobe after
> ide_device_add() call (pmac.c) or later ide_port_scan() call (mediabay.c)
> will fail...  also there was a pmif->mediabay check missing...
> 
> Please try the new version of patch #1 (the other patches are unchanged):

Thanks. Will do ASAP tomorrow. I've been kept busy with other more
urgent things today (gotta love having meetings starting at 6AM in the
morning !)

Cheers,
Ben.

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Bartlomiej Zolnierkiewicz]

On Tuesday 24 June 2008, Benjamin Herrenschmidt wrote:
> 
> > Build error and/or your build fix would be useful here.
> 
> Sorry. Fix was a quick hack. There is a check_media_bay() followed by a
> break; outside of a break-able construct in there (off memory, I'm not
> in front of the dev box right now).

Thanks, 'take 3' follows.

From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [PATCH] ide-pmac: media-bay support fixes (take 3)

* If MB_CD device has already been detected and bay is in mb_up state just
  change bay's state to mb_ide_resetting and let probing thread do the rest
  instead of having open-coded waiting for IDE device to become ready in
  media_bay_set_ide_infos() and doing the probe by ide_device_add().

* Move media_bay_set_ide_infos() call after ide_device_add().

* Use check_media_bay() instead of check_media_bay_by_base(),
  then remove the latter function.

v2:
* Check pmif->mediabay and clear drive->noprobe before calling
  media_bay_set_ide_infos().

v3:
* Fix build problem noticed by Ben.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ppc/pmac.c         |   15 ++++++++++-----
 drivers/macintosh/mediabay.c   |   33 +++++----------------------------
 include/asm-powerpc/mediabay.h |    1 -
 3 files changed, 15 insertions(+), 34 deletions(-)

Index: b/drivers/ide/ppc/pmac.c
===================================================================
--- a/drivers/ide/ppc/pmac.c
+++ b/drivers/ide/ppc/pmac.c
@@ -1030,10 +1030,6 @@ pmac_ide_setup_device(pmac_ide_hwif_t *p
 	/* XXX FIXME: Media bay stuff need re-organizing */
 	if (np->parent && np->parent->name
 	    && strcasecmp(np->parent->name, "media-bay") == 0) {
-#ifdef CONFIG_PMAC_MEDIABAY
-		media_bay_set_ide_infos(np->parent, pmif->regbase, pmif->irq,
-					hwif);
-#endif /* CONFIG_PMAC_MEDIABAY */
 		pmif->mediabay = 1;
 		if (!bidp)
 			pmif->aapl_bus_id = 1;
@@ -1067,7 +1063,7 @@ pmac_ide_setup_device(pmac_ide_hwif_t *p
 
 	if (pmif->mediabay) {
 #ifdef CONFIG_PMAC_MEDIABAY
-		if (check_media_bay_by_base(pmif->regbase, MB_CD)) {
+		if (check_media_bay(np->parent, MB_CD) != -ENODEV) {
 #else
 		if (1) {
 #endif
@@ -1080,6 +1076,15 @@ pmac_ide_setup_device(pmac_ide_hwif_t *p
 
 	ide_device_add(idx, &d);
 
+	if (pmif->mediabay) {
+		hwif->drives[0].noprobe = 0;
+		hwif->drives[1].noprobe = 0;
+#ifdef CONFIG_PMAC_MEDIABAY
+		media_bay_set_ide_infos(np->parent, pmif->regbase, pmif->irq,
+					hwif);
+#endif
+	}
+
 	return 0;
 }
 
Index: b/drivers/macintosh/mediabay.c
===================================================================
--- a/drivers/macintosh/mediabay.c
+++ b/drivers/macintosh/mediabay.c
@@ -433,21 +433,6 @@ int check_media_bay(struct device_node *
 }
 EXPORT_SYMBOL(check_media_bay);
 
-int check_media_bay_by_base(unsigned long base, int what)
-{
-	int	i;
-
-	for (i=0; i<media_bay_count; i++)
-		if (media_bays[i].mdev && base == (unsigned long) media_bays[i].cd_base) {
-			if ((what == media_bays[i].content_id) && media_bays[i].state == mb_up)
-				return 0;
-			media_bays[i].cd_index = -1;
-			return -EINVAL;
-		} 
-
-	return -ENODEV;
-}
-
 int media_bay_set_ide_infos(struct device_node* which_bay, unsigned long base,
 			    int irq, ide_hwif_t *hwif)
 {
@@ -457,8 +442,6 @@ int media_bay_set_ide_infos(struct devic
 		struct media_bay_info* bay = &media_bays[i];
 
 		if (bay->mdev && which_bay == bay->mdev->ofdev.node) {
-			int timeout = 5000, index = hwif->index;
-			
 			down(&bay->lock);
 
 			bay->cd_port	= hwif;
@@ -469,18 +452,12 @@ int media_bay_set_ide_infos(struct devic
 				up(&bay->lock);
 				return 0;
 			}
-			printk(KERN_DEBUG "Registered ide%d for media bay %d\n", index, i);
-			do {
-				if (MB_IDE_READY(i)) {
-					bay->cd_index	= index;
-					up(&bay->lock);
-					return 0;
-				}
-				mdelay(1);
-			} while(--timeout);
-			printk(KERN_DEBUG "Timeount waiting IDE in bay %d\n", i);
+
+			/* let probing thread do the rest */
+			bay->state = mb_ide_resetting;
+
 			up(&bay->lock);
-			return -ENODEV;
+			return 0;
 		}
 	}
 
Index: b/include/asm-powerpc/mediabay.h
===================================================================
--- a/include/asm-powerpc/mediabay.h
+++ b/include/asm-powerpc/mediabay.h
@@ -23,7 +23,6 @@ extern int media_bay_count;
 #ifdef CONFIG_BLK_DEV_IDE_PMAC
 #include <linux/ide.h>
 
-int check_media_bay_by_base(unsigned long base, int what);
 /* called by IDE PMAC host driver to register IDE controller for media bay */
 int media_bay_set_ide_infos(struct device_node *which_bay, unsigned long base,
 			    int irq, ide_hwif_t *hwif);

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Bartlomiej Zolnierkiewicz]

On Tuesday 24 June 2008, Bartlomiej Zolnierkiewicz wrote:
> On Tuesday 24 June 2008, Benjamin Herrenschmidt wrote:
> > 
> > > Build error and/or your build fix would be useful here.
> > 
> > Sorry. Fix was a quick hack. There is a check_media_bay() followed by a
> > break; outside of a break-able construct in there (off memory, I'm not
> > in front of the dev box right now).
> 
> Thanks, 'take 3' follows.
> 
> From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> Subject: [PATCH] ide-pmac: media-bay support fixes (take 3)

[...]

and patch #2/4 required a trivial fix for a reject

From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [PATCH] ide-pmac: add ->init_dev method (take 3)

v2/3:
* Build fixes from Stephen Rothwell.

There should be no functional changes caused by this patch.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ppc/pmac.c |   29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

Index: b/drivers/ide/ppc/pmac.c
===================================================================
--- a/drivers/ide/ppc/pmac.c
+++ b/drivers/ide/ppc/pmac.c
@@ -941,7 +941,23 @@ static u8 pmac_ide_cable_detect(ide_hwif
 	return ATA_CBL_PATA40;
 }
 
+static void pmac_ide_init_dev(ide_drive_t *drive)
+{
+	ide_hwif_t *hwif = drive->hwif;
+	pmac_ide_hwif_t *pmif =
+		(pmac_ide_hwif_t *)dev_get_drvdata(hwif->gendev.parent);
+
+	if (pmif->mediabay) {
+#ifdef CONFIG_PMAC_MEDIABAY
+		if (check_media_bay(pmif->node->parent, MB_CD) == -ENODEV)
+			return;
+#endif
+		drive->noprobe = 1;
+	}
+}
+
 static const struct ide_port_ops pmac_ide_ata6_port_ops = {
+	.init_dev		= pmac_ide_init_dev,
 	.set_pio_mode		= pmac_ide_set_pio_mode,
 	.set_dma_mode		= pmac_ide_set_dma_mode,
 	.selectproc		= pmac_ide_kauai_selectproc,
@@ -949,6 +965,7 @@ static const struct ide_port_ops pmac_id
 };
 
 static const struct ide_port_ops pmac_ide_ata4_port_ops = {
+	.init_dev		= pmac_ide_init_dev,
 	.set_pio_mode		= pmac_ide_set_pio_mode,
 	.set_dma_mode		= pmac_ide_set_dma_mode,
 	.selectproc		= pmac_ide_selectproc,
@@ -956,6 +973,7 @@ static const struct ide_port_ops pmac_id
 };
 
 static const struct ide_port_ops pmac_ide_port_ops = {
+	.init_dev		= pmac_ide_init_dev,
 	.set_pio_mode		= pmac_ide_set_pio_mode,
 	.set_dma_mode		= pmac_ide_set_dma_mode,
 	.selectproc		= pmac_ide_selectproc,
@@ -1065,17 +1083,6 @@ pmac_ide_setup_device(pmac_ide_hwif_t *p
 	       hwif->index, model_name[pmif->kind], pmif->aapl_bus_id,
 	       pmif->mediabay ? " (mediabay)" : "", hwif->irq);
 
-	if (pmif->mediabay) {
-#ifdef CONFIG_PMAC_MEDIABAY
-		if (check_media_bay(np->parent, MB_CD) != -ENODEV) {
-#else
-		if (1) {
-#endif
-			hwif->drives[0].noprobe = 1;
-			hwif->drives[1].noprobe = 1;
-		}
-	}
-
 	idx[0] = hwif->index;
 
 	ide_device_add(idx, &d);

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Benjamin Herrenschmidt]

BTW (usual problem), I'm having a very hard time applying your patches..

It looks like the 2 replacement ones you sent are no longer against
whatever merge point I found in linux-next history for your tree, so
I applied against linux-next 20080620, where 1/4 applies, 2/4 applies
with offsets and 3 fails...

For some reason, every time you send me new patches, I need more time
figuring out what they are supposed to apply to than actually testing
them :-)

Cheers,
Ben.

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Bartlomiej Zolnierkiewicz]

On Thursday 26 June 2008, Benjamin Herrenschmidt wrote:
> BTW (usual problem), I'm having a very hard time applying your patches..
> 
> It looks like the 2 replacement ones you sent are no longer against
> whatever merge point I found in linux-next history for your tree, so
> I applied against linux-next 20080620, where 1/4 applies, 2/4 applies
> with offsets and 3 fails...

I see:

http://lkml.org/lkml/2008/6/24/369

it should say patch #3/4 not #2/4

I've just checked and everything still applies fine to 20080625.

> For some reason, every time you send me new patches, I need more time
> figuring out what they are supposed to apply to than actually testing
> them :-)

The latest patches in one place:

http://www.kernel.org/pub/linux/kernel/people/bart/pata-2.6/

pmac-media-bay-support-fixes-take-3.patch
pmac-store-pmif-instead-of-hwif-in-driver_data-take-2.patch
pmac-add-init_dev-method-take-3.patch
pmac-move-ide_find_port-call-to-pmac_ide_setup_device-take-2.patch

IMO they are quite small and straightforward so setting git tree would
be overdoing it but if there are still problems I will set one.

Thanks,
Bart

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Benjamin Herrenschmidt]

On Thu, 2008-06-26 at 10:51 +0200, Bartlomiej Zolnierkiewicz wrote:
> 
> The latest patches in one place:
> 
> http://www.kernel.org/pub/linux/kernel/people/bart/pata-2.6/
> 
> pmac-media-bay-support-fixes-take-3.patch
> pmac-store-pmif-instead-of-hwif-in-driver_data-take-2.patch
> pmac-add-init_dev-method-take-3.patch
> pmac-move-ide_find_port-call-to-pmac_ide_setup_device-take-2.patch
> 
> IMO they are quite small and straightforward so setting git tree would
> be overdoing it but if there are still problems I will set one.

That's ok, just tell me a tree and an SHA1 ID of a commit on top of
which they apply, that will make my life easier :-)

I'll test them tomorrow morning.

Cheers,
Ben.

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Bartlomiej Zolnierkiewicz]

On Thursday 26 June 2008, Benjamin Herrenschmidt wrote:
> On Thu, 2008-06-26 at 10:51 +0200, Bartlomiej Zolnierkiewicz wrote:
> > 
> > The latest patches in one place:
> > 
> > http://www.kernel.org/pub/linux/kernel/people/bart/pata-2.6/
> > 
> > pmac-media-bay-support-fixes-take-3.patch
> > pmac-store-pmif-instead-of-hwif-in-driver_data-take-2.patch
> > pmac-add-init_dev-method-take-3.patch
> > pmac-move-ide_find_port-call-to-pmac_ide_setup_device-take-2.patch
> > 
> > IMO they are quite small and straightforward so setting git tree would
> > be overdoing it but if there are still problems I will set one.
> 
> That's ok, just tell me a tree and an SHA1 ID of a commit on top of
> which they apply, that will make my life easier :-)

They are still for *linux-next* (since they depend on other IDE changes)
and apply fine on top of -20080625.

SHA1 ID is: 9ab50ba2f4958b593802ce91395d07148b611b71 but I don't know
whether it is useful in linux-next context.

> I'll test them tomorrow morning.

Thanks.

Bart

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Benjamin Herrenschmidt]

> > That's ok, just tell me a tree and an SHA1 ID of a commit on top of
> > which they apply, that will make my life easier :-)
> 
> They are still for *linux-next* (since they depend on other IDE changes)
> and apply fine on top of -20080625.
> 
> SHA1 ID is: 9ab50ba2f4958b593802ce91395d07148b611b71 but I don't know
> whether it is useful in linux-next context.

Took me a while, kid was sick.

They apply on 20080625 (with various offset/fuzz but they do apply) and
the tree builds. The media bay however fails the same way as before with
IDE register errors.

I'll see if I can find where they come from.

Ben.

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Benjamin Herrenschmidt]

> Took me a while, kid was sick.
> 
> They apply on 20080625 (with various offset/fuzz but they do apply) and
> the tree builds. The media bay however fails the same way as before with
> IDE register errors.
> 
> I'll see if I can find where they come from.

Found multiple issues related to the ide-pmac <-> mediabay & ide core
interaction changes. I've done some fixes but it's not quite there yet.
It looks like it's getting IRQ issues with the mediabay CD, for some
reasons looks like something unmasks the interrupt before there's a
handler or around those lines... I get an irq "nobody cared" error, it
gets disabled, and then hdc gets lost interrupts.

I'll dig a bit more and if I can't find what's up tonight, will send
you my current patches in case you have some other idea.

Cheers,
Ben.

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Benjamin Herrenschmidt]

On Thu, 2008-07-03 at 16:47 +1000, Benjamin Herrenschmidt wrote:
> > Took me a while, kid was sick.
> > 
> > They apply on 20080625 (with various offset/fuzz but they do apply) and
> > the tree builds. The media bay however fails the same way as before with
> > IDE register errors.
> > 
> > I'll see if I can find where they come from.
> 
> Found multiple issues related to the ide-pmac <-> mediabay & ide core
> interaction changes. I've done some fixes but it's not quite there yet.
> It looks like it's getting IRQ issues with the mediabay CD, for some
> reasons looks like something unmasks the interrupt before there's a
> handler or around those lines... I get an irq "nobody cared" error, it
> gets disabled, and then hdc gets lost interrupts.
> 
> I'll dig a bit more and if I can't find what's up tonight, will send
> you my current patches in case you have some other idea.

Ok, so the interrupt stuff is weird, I need to dig more. I get basically
what looks like an interrupt storm in the enable_irq() after the probing
of the drives. I know the media-bay IDE has some weird behaviours at
probe time but that's not quite something I saw before. I'll have to
debug more.

In the meantime, here's the hacks I applied to your patch series to get
things mostly going (appart from that bug, which we -do- need to fix,
but give me a bit more time to track it down). You'll probably want to
integrate the fixes with your patches and remove the debug stuff :-)

You'll notice that I created a new state for when the media-bay is up
but IDE hasn't registered in yet. This might disappear in the future
when I do the libata bits but for now it fixes a few issues where
noprobe was never set properly, or if set, it would try to probe the
drives twice and blow up...

(The problem was either noprobe would stay set to 1 with your old code,
despite the clearing in the mediabay case because pmac_ide_init_dev
would set it back to 1. If you fix that you get into a case where
the bay is "up" before IDE is ready, and when IDE gets ready, both
the idea layer and the bay code race to trigger a probe).


Ben.

Index: linux-work/drivers/ide/ide-probe.c
===================================================================
--- linux-work.orig/drivers/ide/ide-probe.c	2008-07-03 15:50:24.000000000 +1000
+++ linux-work/drivers/ide/ide-probe.c	2008-07-03 17:14:42.000000000 +1000
@@ -770,11 +770,15 @@ static int ide_probe_port(ide_hwif_t *hw
 	unsigned int irqd;
 	int unit, rc = -ENODEV;
 
-	BUG_ON(hwif->present);
-
+	printk("ide_probe_port(%s) noprobe=%d,%d\n",
+	       hwif->name, hwif->drives[0].noprobe, hwif->drives[1].noprobe);
 	if (hwif->drives[0].noprobe && hwif->drives[1].noprobe)
 		return -EACCES;
 
+	WARN_ON(hwif->present);
+	if (hwif->present)
+		return 0;
+
 	/*
 	 * We must always disable IRQ, as probe_for_drive will assert IRQ, but
 	 * we'll install our IRQ driver much later...
@@ -798,6 +802,7 @@ static int ide_probe_port(ide_hwif_t *hw
 		(void) probe_for_drive(drive);
 		if (drive->present)
 			rc = 0;
+		ide_busy_sleep(hwif);
 	}
 
 	local_irq_restore(flags);
Index: linux-work/drivers/ide/ppc/pmac.c
===================================================================
--- linux-work.orig/drivers/ide/ppc/pmac.c	2008-07-03 15:54:43.000000000 +1000
+++ linux-work/drivers/ide/ppc/pmac.c	2008-07-03 15:57:12.000000000 +1000
@@ -951,8 +951,10 @@ static void pmac_ide_init_dev(ide_drive_
 
 	if (pmif->mediabay) {
 #ifdef CONFIG_PMAC_MEDIABAY
-		if (check_media_bay(pmif->node->parent, MB_CD) == -ENODEV)
+		if (check_media_bay(pmif->node->parent, MB_CD) == 0) {
+			drive->noprobe = 0;
 			return;
+		}
 #endif
 		drive->noprobe = 1;
 	}
@@ -1096,8 +1098,6 @@ static int __devinit pmac_ide_setup_devi
 	ide_device_add(idx, &d);
 
 	if (pmif->mediabay) {
-		hwif->drives[0].noprobe = 0;
-		hwif->drives[1].noprobe = 0;
 #ifdef CONFIG_PMAC_MEDIABAY
 		media_bay_set_ide_infos(np->parent, pmif->regbase, pmif->irq,
 					hwif);
Index: linux-work/drivers/macintosh/mediabay.c
===================================================================
--- linux-work.orig/drivers/macintosh/mediabay.c	2008-07-03 16:20:13.000000000 +1000
+++ linux-work/drivers/macintosh/mediabay.c	2008-07-03 16:41:07.000000000 +1000
@@ -156,7 +156,8 @@ enum {
 	mb_ide_resetting,	/* IDE reset bit unser, waiting MB_IDE_WAIT */
 	mb_ide_waiting,		/* Waiting for BUSY bit to go away until MB_IDE_TIMEOUT */
 	mb_up,			/* Media bay full */
-	mb_powering_down	/* Powering down (avoid too fast down/up) */
+	mb_powering_down,	/* Powering down (avoid too fast down/up) */
+	mb_wait_ide_init,	/* Wait for IDE layer to go up */
 };
 
 #define MB_POWER_SOUND		0x08
@@ -448,7 +449,7 @@ int media_bay_set_ide_infos(struct devic
  			bay->cd_base	= (void __iomem *) base;
 			bay->cd_irq	= irq;
 
-			if ((MB_CD != bay->content_id) || bay->state != mb_up) {
+			if ((MB_CD != bay->content_id) || bay->state != mb_wait_ide_init) {
 				up(&bay->lock);
 				return 0;
 			}
@@ -522,8 +523,8 @@ static void media_bay_step(int i)
 	    	break;
 	case mb_ide_waiting:
 		if (bay->cd_base == NULL) {
-			bay->timer = 0;
-			bay->state = mb_up;
+			bay->timer = -1;
+			bay->state = mb_wait_ide_init;
 			MBDBG("mediabay%d: up before IDE init\n", i);
 			break;
 		} else if (MB_IDE_READY(i)) {
@@ -557,6 +558,9 @@ static void media_bay_step(int i)
 			bay->timer = 0;
 	    	}
 		break;
+	case mb_wait_ide_init:
+		bay->timer = -1;
+		break;
 #endif /* CONFIG_BLK_DEV_IDE_PMAC */
 	case mb_powering_down:
 	    	bay->state = mb_empty;
@@ -656,7 +660,8 @@ static int __devinit media_bay_attach(st
 		msleep(MB_POLL_DELAY);
 		media_bay_step(i);
 	} while((bay->state != mb_empty) &&
-		(bay->state != mb_up));
+		(bay->state != mb_up) &&
+		(bay->state != mb_wait_ide_init));
 
 	/* Mark us ready by filling our mdev data */
 	macio_set_drvdata(mdev, bay);

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Bartlomiej Zolnierkiewicz]

Hi,

On Thursday 03 July 2008, Benjamin Herrenschmidt wrote:
> On Thu, 2008-07-03 at 16:47 +1000, Benjamin Herrenschmidt wrote:
> > > Took me a while, kid was sick.
> > > 
> > > They apply on 20080625 (with various offset/fuzz but they do apply) and
> > > the tree builds. The media bay however fails the same way as before with
> > > IDE register errors.
> > > 
> > > I'll see if I can find where they come from.
> > 
> > Found multiple issues related to the ide-pmac <-> mediabay & ide core
> > interaction changes. I've done some fixes but it's not quite there yet.
> > It looks like it's getting IRQ issues with the mediabay CD, for some
> > reasons looks like something unmasks the interrupt before there's a
> > handler or around those lines... I get an irq "nobody cared" error, it
> > gets disabled, and then hdc gets lost interrupts.
> > 
> > I'll dig a bit more and if I can't find what's up tonight, will send
> > you my current patches in case you have some other idea.
> 
> Ok, so the interrupt stuff is weird, I need to dig more. I get basically
> what looks like an interrupt storm in the enable_irq() after the probing
> of the drives. I know the media-bay IDE has some weird behaviours at
> probe time but that's not quite something I saw before. I'll have to
> debug more.

This may be generic ide problem uncovered by media-bay changes.

In init_irq() we unmask IRQs just before registering IRQ handler but we
we don't clear pending IRQs before the unmask (simply reading the Status
register should be enough).

[ Previously ide_port_wait_ready() would do it during ide_device_add()
  call and before the IRQ handler is registered but now it will be skipped
  because of ->noprobe being set. ]

> In the meantime, here's the hacks I applied to your patch series to get
> things mostly going (appart from that bug, which we -do- need to fix,
> but give me a bit more time to track it down). You'll probably want to
> integrate the fixes with your patches and remove the debug stuff :-)

Thanks!!

> You'll notice that I created a new state for when the media-bay is up
> but IDE hasn't registered in yet. This might disappear in the future
> when I do the libata bits but for now it fixes a few issues where
> noprobe was never set properly, or if set, it would try to probe the
> drives twice and blow up...
> 
> (The problem was either noprobe would stay set to 1 with your old code,
> despite the clearing in the mediabay case because pmac_ide_init_dev
> would set it back to 1. If you fix that you get into a case where
> the bay is "up" before IDE is ready, and when IDE gets ready, both
> the idea layer and the bay code race to trigger a probe).

Arrghhh, this was actually caused by a brain glitch on my side...

While preparing this patch I was under the impression that ->init_dev can
be called only by ide through ide_device_add(), which is of course untrue
since it can be called by mediabay through ide_port_scan()...

However when I think deeper about it I recall that I first implemented
it as ->init_hwif (for which the assumption was true) and later converted
the patch to ->init_dev because I noticed the assumption and realized
that it needs to be ->init_dev to make it work for warm-plug...

Scary... :)

> Ben.
> 
> Index: linux-work/drivers/ide/ide-probe.c
> ===================================================================
> --- linux-work.orig/drivers/ide/ide-probe.c	2008-07-03 15:50:24.000000000 +1000
> +++ linux-work/drivers/ide/ide-probe.c	2008-07-03 17:14:42.000000000 +1000
> @@ -770,11 +770,15 @@ static int ide_probe_port(ide_hwif_t *hw
>  	unsigned int irqd;
>  	int unit, rc = -ENODEV;
>  
> -	BUG_ON(hwif->present);
> -
> +	printk("ide_probe_port(%s) noprobe=%d,%d\n",
> +	       hwif->name, hwif->drives[0].noprobe, hwif->drives[1].noprobe);
>  	if (hwif->drives[0].noprobe && hwif->drives[1].noprobe)
>  		return -EACCES;
>  
> +	WARN_ON(hwif->present);
> +	if (hwif->present)
> +		return 0;
> +

This is a kind of tangential issue to pmac stuff.

Could you resend it as a separate patch with your S-o-b: line?

>  	/*
>  	 * We must always disable IRQ, as probe_for_drive will assert IRQ, but
>  	 * we'll install our IRQ driver much later...
> @@ -798,6 +802,7 @@ static int ide_probe_port(ide_hwif_t *hw
>  		(void) probe_for_drive(drive);
>  		if (drive->present)
>  			rc = 0;
> +		ide_busy_sleep(hwif);

I don't quite get this chunk.

Is it a workaround for interrupt storm problem?

[...]

I integrated the rest in a verbatim form with pmac patches
(two of them got 'take 4' as a result, the other two remain unchanged):

pmac-media-bay-support-fixes-take-4.patch
pmac-store-pmif-instead-of-hwif-in-driver_data-take-2.patch
pmac-add-init_dev-method-take-4.patch
pmac-move-ide_find_port-call-to-pmac_ide_setup_device-take-2.patch

[ in the usual place ]

Thanks,
Bart

Re: [PATCH 1/4] ide-pmac: media-bay support fixes

[Benjamin Herrenschmidt]

> In init_irq() we unmask IRQs just before registering IRQ handler but we
> we don't clear pending IRQs before the unmask (simply reading the Status
> register should be enough).
> 
> [ Previously ide_port_wait_ready() would do it during ide_device_add()
>   call and before the IRQ handler is registered but now it will be skipped
>   because of ->noprobe being set. ]

I'm pretty sure I added some reads of the status reg before enable_irq()
and that didn't fix it but I may have fubar'ed. I'll dbl check.

> Arrghhh, this was actually caused by a brain glitch on my side...
> 
> While preparing this patch I was under the impression that ->init_dev can
> be called only by ide through ide_device_add(), which is of course untrue
> since it can be called by mediabay through ide_port_scan()...
> 
> However when I think deeper about it I recall that I first implemented
> it as ->init_hwif (for which the assumption was true) and later converted
> the patch to ->init_dev because I noticed the assumption and realized
> that it needs to be ->init_dev to make it work for warm-plug...
> 
> Scary... :)

Yup. Later, I though about ways not to add a new state but the
patch was done, so let's go with it for now.

> This is a kind of tangential issue to pmac stuff.
> 
> Could you resend it as a separate patch with your S-o-b: line?

Oh, that's just debug stuff for me to track down the bug. Do you want
to merge it ? I told you I just send whatever hacks I did to get it
going so far, I'll clean things up when I have the irq stuff solved.

> >  	/*
> >  	 * We must always disable IRQ, as probe_for_drive will assert IRQ, but
> >  	 * we'll install our IRQ driver much later...
> > @@ -798,6 +802,7 @@ static int ide_probe_port(ide_hwif_t *hw
> >  		(void) probe_for_drive(drive);
> >  		if (drive->present)
> >  			rc = 0;
> > +		ide_busy_sleep(hwif);
> 
> I don't quite get this chunk.
> 
> Is it a workaround for interrupt storm problem?

Yup, tho didn't work.

> [...]
> 
> I integrated the rest in a verbatim form with pmac patches
> (two of them got 'take 4' as a result, the other two remain unchanged):
> 
> pmac-media-bay-support-fixes-take-4.patch
> pmac-store-pmif-instead-of-hwif-in-driver_data-take-2.patch
> pmac-add-init_dev-method-take-4.patch
> pmac-move-ide_find_port-call-to-pmac_ide_setup_device-take-2.patch
> 
> [ in the usual place ]

Ok. Will look at it on monday (ie. tomorrow for me).

Cheers,
Ben.