login
Search
* 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 patchdrivers/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 *pif (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...
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.--
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.
--
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
--
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.--
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.--
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, registeri...
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_de...
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 notThanks. 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.--
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 *pif (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
@@...
[...]
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,
@@ -10...
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.--
I see:
http://lkml.org/lkml/2008/6/24/369
it should say patch #3/4 not #2/4
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.patchIMO 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
--
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.--
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
Thanks.
Bart
--
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.
--
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.--
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
...
Hi,
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 skippedArrghhh, 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...This is a kind of tangential issue to pmac stuff.
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
--
I'm pretty sure I added some reads of the status reg before enable_irq()
Yup. Later, I though about ways not to add a new state but the
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 itOk. Will look at it on monday (ie. tomorrow for me).
Cheers,
Ben.--
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.--
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 potentialYou may want to look at ACPI hotplug dock handling for inspiration
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.Bart
--
I intend to yes :-) Along with my WIP pata_macio of course but I don't
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 theSure 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 whatI'll let you know when I get something done.
Cheers,
Ben.--
