ide-pmac: media-bay support fixes
Subsystems (3)
PATCH 1/4 ide-pmac: media-bay support fixes
* 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.
---
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(-)
drivers/ide/ppc/pmac.c +8 -10
@@ -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_ide_infos (np->parent, pmif->regbase, pmif->irq, hwif); +#endif +return 0 ; }
drivers/macintosh/mediabay.c +5 -28
@@ -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 devicstruct 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 devicup (&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 ; } }
include/asm-powerpc/mediabay.h +0 -1
@@ -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);
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.
Quoted 2 lines · 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 ?
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.
Quoted 9 lines · Benjamin Herrenschmidt
Quoted 2 lines · 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 ?
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
Quoted 11 lines · Bartlomiej Zolnierkiewicz
Quoted 9 lines · Benjamin Herrenschmidt
Quoted 2 lines · 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 ?
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 ?
Quoted 3 lines
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
Quoted 4 lines
tonight.
Thanks,
Bart
Quoted 5 lines
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.
Quoted 10 lines · Benjamin Herrenschmidt
Quoted 4 lines
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.
Quoted 6 lines
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.
Quoted 10 lines · Benjamin Herrenschmidt
Quoted 5 lines
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.
Quoted 5 lines
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_ONinide_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.
Quoted 15 lines · Benjamin Herrenschmidt
Quoted 14 lines · Benjamin Herrenschmidt
Quoted 5 lines
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_ONinide_probe_port()(line 773).
Quoted 7 lines
Quoted 1 line
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 :-).
Quoted 9 lines
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_devicestructure (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).
Quoted 4 lines
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, thedrivers/idedriver 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. ]
Quoted 4 lines
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
Quoted 6 lines
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.
Quoted 2 lines
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,
Quoted 2 lines
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()andide_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.
Quoted 2 lines
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.
Quoted 1 line
[ Please keep me in loop on these mediabay changes. ]
I'll let you know when I get something done.
Cheers,
Ben.
Quoted 7 lines · Benjamin Herrenschmidt
Quoted 6 lines
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.
Quoted 7 lines
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_ONinide_probe_port()(line 773).
Build error and/or your build fix would be useful here.
Quoted 24 lines
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).
Quoted 7 lines
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 <[email protected]>
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().
--- drivers/ide/ppc/pmac.c | 23 +++++++++++++---------- drivers/macintosh/mediabay.c | 33 +++++---------------------------- include/asm-powerpc/mediabay.h | 1 - 3 files changed, 18 insertions(+), 39 deletions(-)
drivers/ide/ppc/pmac.c +13 -10
@@ -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 *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); +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 ; }
drivers/macintosh/mediabay.c +5 -28
@@ -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 devicstruct 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 devicup (&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 ; } }
include/asm-powerpc/mediabay.h +0 -1
@@ -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);
Quoted 2 lines
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).
Quoted 5 lines
Ok, I see the problem - we now need to clear
dive->noprobeafteride_device_add()call (pmac.c) or lateride_port_scan()call (mediabay.c) will fail... also there was apmif->mediabaycheck 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.
Quoted 6 lines · Benjamin Herrenschmidt
Quoted 2 lines
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 <[email protected]>
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.
--- drivers/ide/ppc/pmac.c | 15 ++++++++++----- drivers/macintosh/mediabay.c | 33 +++++---------------------------- include/asm-powerpc/mediabay.h | 1 - 3 files changed, 15 insertions(+), 34 deletions(-)
drivers/ide/ppc/pmac.c +10 -5
@@ -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
@@ -1080,6 +1076,15 @@ pmac_ide_setup_device(pmac_ide_hwif_t *pide_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 ; }
drivers/macintosh/mediabay.c +5 -28
@@ -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 devicstruct 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 devicup (&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 ; } }
include/asm-powerpc/mediabay.h +0 -1
@@ -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);
Quoted 4 lines · Bartlomiej Zolnierkiewicz
Quoted 3 lines · Benjamin Herrenschmidt
Quoted 2 lines
Build error and/or your build fix would be useful here.
Quoted 8 lines
Quoted 3 lines
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 <[email protected]>
Subject: [PATCH] ide-pmac: media-bay support fixes (take 3)
[...]
and patch #2/4 required a trivial fix for a reject
From: Bartlomiej Zolnierkiewicz <[email protected]>
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.
--- drivers/ide/ppc/pmac.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-)
drivers/ide/ppc/pmac.c +18 -11
@@ -941,7 +941,23 @@ static u8 pmac_ide_cable_detect(ide_hwifreturn 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);
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.
Quoted 6 lines · 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...
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.
Quoted 3 lines
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
Quoted 5 lines · Bartlomiej Zolnierkiewicz
The latest patches in one place:
http://www.kernel.org/pub/linux/kernel/people/bart/pata-2.6/
Quoted 7 lines
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.
Quoted 11 lines · Benjamin Herrenschmidt
Quoted 10 lines · Bartlomiej Zolnierkiewicz
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
Quoted 5 lines
Quoted 2 lines
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.
Quoted 1 line
I'll test them tomorrow morning.
Thanks.
Bart
Quoted 9 lines
Quoted 3 lines
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:
9ab50ba2f4958b593802ce91395d07148b611b71but 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.
Quoted 8 lines
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.
Quoted 15 lines · Benjamin Herrenschmidt
Quoted 7 lines
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.
Quoted 2 lines
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.
drivers/ide/ide-probe.c +7 -2
@@ -770,11 +770,15 @@ static int ide_probe_port(ide_hwif_t *hwunsigned 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);
drivers/ide/ppc/pmac.c +3 -3
@@ -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_deviide_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);
drivers/macintosh/mediabay.c +10 -5
@@ -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(stmsleep (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);
Hi,
Quoted 16 lines · Benjamin Herrenschmidt
Quoted 15 lines · Benjamin Herrenschmidt
Quoted 7 lines
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.
Quoted 8 lines
Quoted 2 lines
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. ]
Quoted 4 lines
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!!
Quoted 11 lines
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_devwould 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... :)
Quoted 11 lines
Ben.
Index:
linux-work/drivers/ide/ide-probe.c
===================================================================drivers/ide/ide-probe.c +0
-1@@ -770,11 +770,15 @@ static int ide_probe_port(ide_hwif_t *hwunsigned int irqd;int unit, rc = -ENODEV ; -BUG_ON (hwif->present);
drivers/ide/ide-probe.c static int ide_probe_port(ide_hwif_t *hw · line 781
Quoted 10 lines
drivers/ide/ide-probe.c +6
-1- +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?
drivers/ide/ide-probe.c static int ide_probe_port(ide_hwif_t *hw · line 805
Quoted 8 lines
drivers/ide/ide-probe.c +1
-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);
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
Quoted 7 lines
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 duringide_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.
Quoted 6 lines
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 throughide_port_scan()...
Quoted 6 lines
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.
Quoted 3 lines
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.
Quoted 11 lines
drivers/ide/ide-probe.c static int ide_probe_port(ide_hwif_t *hw · line 805
Quoted 8 lines
drivers/ide/ide-probe.c +1
-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);I don't quite get this chunk.
Quoted 1 line
Is it a workaround for interrupt storm problem?
Yup, tho didn't work.
Quoted 11 lines
[...]
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.