Re: pci_disable_device() vs. arch
Benjamin Herrenschmidt wrote: > > > > >Its not clutter -- what you are doing is hiding pieces of the driver > >from the driver maintainer. pcibios_enable_device should not be > >cluttered up with such mess, too. > > Well... pcibios_enable_device() has to at least make sure the device > gets powered up as it's powered down after PCI probe. Except if we > end up calling pci_set_power_state() to power it up early in the > sungem driver. huh? pci_enable_device calls pci_set_power_state. sungem calls pci_enable_device. pcibios_enable_device shouldn't have to worry about power stuff. If it does, you need a pcibios_set_power_state, called from pci_set_power_state, instead. > >I point out that I recently fixed a bug where Via interrupts were being > >assigned incorrectly. If I had not done a global grep for Via > >irq-related code, I would have missed the spot where the PPC code was > >doing a kludge for one of the four on-board Via devices, hardcoding the > >USB irq number to 11. > > Hrm... interrupt routing on some PPC-based motherboard is quite a > mess, fortunately that's not the case on pmacs. The IRQ assignement > has to be part of the arch AFAIK, only the arch knows on which > interrupt line of the controller a given chip is wired and how > interrupt controllers are cascaded. Via is an exception > What I suggest if for pci_bus to have an optional set_power_state > function that is called when a device on that bus calls > pci_set_power_state(). This function would then be able to implement > those cases where power control is possible, while not done > via PCI PM caps. > A pci_bus structure exist for both "root" busses and busses under > PCI<->PCI bridges, so effectively, there's a pci_bus structure per > bridge (beeing host or PCI<->PCI). I beleive it makes sense for > the bridge to have a way to handle the child power state. Ok, agreed. There are always gonna be special case bridges, including (for my interest) multi-port NICs whose interfaces are presented as PCI devices downstream from a PCI-PCI bridge. Controlling power for these nics is sometimes done by messing around with the PM bits on the bridge, not on the downstream devices. Jeff -- Jeff Garzik | Andre the Giant has a posse. Building 1024| MandrakeSoft | - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: pci_disable_device() vs. arch
> >Its not clutter -- what you are doing is hiding pieces of the driver >from the driver maintainer. pcibios_enable_device should not be >cluttered up with such mess, too. Well... pcibios_enable_device() has to at least make sure the device gets powered up as it's powered down after PCI probe. Except if we end up calling pci_set_power_state() to power it up early in the sungem driver. >I point out that I recently fixed a bug where Via interrupts were being >assigned incorrectly. If I had not done a global grep for Via >irq-related code, I would have missed the spot where the PPC code was >doing a kludge for one of the four on-board Via devices, hardcoding the >USB irq number to 11. Hrm... interrupt routing on some PPC-based motherboard is quite a mess, fortunately that's not the case on pmacs. The IRQ assignement has to be part of the arch AFAIK, only the arch knows on which interrupt line of the controller a given chip is wired and how interrupt controllers are cascaded. >Correct. If your driver uses the API correctly, then when/if we want to >mess around with hotplug resource assignment, we can un-assign resources >as we like. Since there aren't too many users of pci_disable_device so >far, I want to make sure early adopters get it right. Well... at least with sungem, there's no such risk as the entire bus (up to the host bridge) where it lives is internal to the UniNorth ASIC. >Can you give a -specific- example of arch code that is -not- sungem >related, but needs to occur when one powers-down a sungem MAC? > >If the PM code is related to sungem, it belongs in sungem. >So far I don't see a need for arch-specific hooks anywhere... Hrm... let me try again... Powering down individual devices can be controlled by the PCI PM capabilities, or in some cases (at least 2 cases here on UniNorth based pmacs) by other bits in the host bridge. What I suggest if for pci_bus to have an optional set_power_state function that is called when a device on that bus calls pci_set_power_state(). This function would then be able to implement those cases where power control is possible, while not done via PCI PM caps. A pci_bus structure exist for both "root" busses and busses under PCI<->PCI bridges, so effectively, there's a pci_bus structure per bridge (beeing host or PCI<->PCI). I beleive it makes sense for the bridge to have a way to handle the child power state. Ben - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: pci_disable_device() vs. arch
Benjamin Herrenschmidt wrote: > > Hi ! > > Would it make sense to add a > > pcibios_disable_device(pci_dev*) called from the end of > pci_disable_device() ? > > I'm adding a call to it to sungem along with other pmac stuffs > so that the chip can be properly power down (actually it's not > really powered down but unclocked) after module removal. > Of course, the arch code must be able to catch it in order to > play with the various UniNorth control bits. What arch-specific things need to be done? arch-specific pcibios_disable_device may be a good idea... but in this case it sounds like you need to put #ifdef CONFIG_ALL_PPC code in sungem.c instead, if the power-down code is specific to gmacs. > Note that my current gmac driver does shut the chip down when > the interface is down, which makes it a bit more useful for > laptops as most users currently compile the driver in the kernel. Although some drivers already do this, you really need an inactivity timer instead of unconditionally powering-down the hardware on dev->stop(). dhcp and other applications will often bounce the interface... > I have nothing about changing the policy if you prefer so that > users will now have to rmmod the driver once done with the > interface to save power. For power-down specifically, you should use pci-set-power-state not pci-disable-device. pci_disable_device is the opposite of pci_enable_device. pci_enable_device not only wakes up the device, but also assigns resources. Which implies that pci-disable-device is allowed to un-assign resources. There shouldn't be a problem with a net device doing that per se, but you should be aware of the implications. Jeff -- Jeff Garzik | Andre the Giant has a posse. Building 1024| MandrakeSoft | - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
pci_disable_device() vs. arch
Hi ! Would it make sense to add a pcibios_disable_device(pci_dev*) called from the end of pci_disable_device() ? I'm adding a call to it to sungem along with other pmac stuffs so that the chip can be properly power down (actually it's not really powered down but unclocked) after module removal. Of course, the arch code must be able to catch it in order to play with the various UniNorth control bits. Note that my current gmac driver does shut the chip down when the interface is down, which makes it a bit more useful for laptops as most users currently compile the driver in the kernel. I have nothing about changing the policy if you prefer so that users will now have to rmmod the driver once done with the interface to save power. Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
pci_disable_device() vs. arch
Hi ! Would it make sense to add a pcibios_disable_device(pci_dev*) called from the end of pci_disable_device() ? I'm adding a call to it to sungem along with other pmac stuffs so that the chip can be properly power down (actually it's not really powered down but unclocked) after module removal. Of course, the arch code must be able to catch it in order to play with the various UniNorth control bits. Note that my current gmac driver does shut the chip down when the interface is down, which makes it a bit more useful for laptops as most users currently compile the driver in the kernel. I have nothing about changing the policy if you prefer so that users will now have to rmmod the driver once done with the interface to save power. Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: pci_disable_device() vs. arch
Its not clutter -- what you are doing is hiding pieces of the driver from the driver maintainer. pcibios_enable_device should not be cluttered up with such mess, too. Well... pcibios_enable_device() has to at least make sure the device gets powered up as it's powered down after PCI probe. Except if we end up calling pci_set_power_state() to power it up early in the sungem driver. I point out that I recently fixed a bug where Via interrupts were being assigned incorrectly. If I had not done a global grep for Via irq-related code, I would have missed the spot where the PPC code was doing a kludge for one of the four on-board Via devices, hardcoding the USB irq number to 11. Hrm... interrupt routing on some PPC-based motherboard is quite a mess, fortunately that's not the case on pmacs. The IRQ assignement has to be part of the arch AFAIK, only the arch knows on which interrupt line of the controller a given chip is wired and how interrupt controllers are cascaded. Correct. If your driver uses the API correctly, then when/if we want to mess around with hotplug resource assignment, we can un-assign resources as we like. Since there aren't too many users of pci_disable_device so far, I want to make sure early adopters get it right. Well... at least with sungem, there's no such risk as the entire bus (up to the host bridge) where it lives is internal to the UniNorth ASIC. Can you give a -specific- example of arch code that is -not- sungem related, but needs to occur when one powers-down a sungem MAC? If the PM code is related to sungem, it belongs in sungem. So far I don't see a need for arch-specific hooks anywhere... Hrm... let me try again... Powering down individual devices can be controlled by the PCI PM capabilities, or in some cases (at least 2 cases here on UniNorth based pmacs) by other bits in the host bridge. What I suggest if for pci_bus to have an optional set_power_state function that is called when a device on that bus calls pci_set_power_state(). This function would then be able to implement those cases where power control is possible, while not done via PCI PM caps. A pci_bus structure exist for both root busses and busses under PCI-PCI bridges, so effectively, there's a pci_bus structure per bridge (beeing host or PCI-PCI). I beleive it makes sense for the bridge to have a way to handle the child power state. Ben - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: pci_disable_device() vs. arch
Benjamin Herrenschmidt wrote: Its not clutter -- what you are doing is hiding pieces of the driver from the driver maintainer. pcibios_enable_device should not be cluttered up with such mess, too. Well... pcibios_enable_device() has to at least make sure the device gets powered up as it's powered down after PCI probe. Except if we end up calling pci_set_power_state() to power it up early in the sungem driver. huh? pci_enable_device calls pci_set_power_state. sungem calls pci_enable_device. pcibios_enable_device shouldn't have to worry about power stuff. If it does, you need a pcibios_set_power_state, called from pci_set_power_state, instead. I point out that I recently fixed a bug where Via interrupts were being assigned incorrectly. If I had not done a global grep for Via irq-related code, I would have missed the spot where the PPC code was doing a kludge for one of the four on-board Via devices, hardcoding the USB irq number to 11. Hrm... interrupt routing on some PPC-based motherboard is quite a mess, fortunately that's not the case on pmacs. The IRQ assignement has to be part of the arch AFAIK, only the arch knows on which interrupt line of the controller a given chip is wired and how interrupt controllers are cascaded. Via is an exception What I suggest if for pci_bus to have an optional set_power_state function that is called when a device on that bus calls pci_set_power_state(). This function would then be able to implement those cases where power control is possible, while not done via PCI PM caps. A pci_bus structure exist for both root busses and busses under PCI-PCI bridges, so effectively, there's a pci_bus structure per bridge (beeing host or PCI-PCI). I beleive it makes sense for the bridge to have a way to handle the child power state. Ok, agreed. There are always gonna be special case bridges, including (for my interest) multi-port NICs whose interfaces are presented as PCI devices downstream from a PCI-PCI bridge. Controlling power for these nics is sometimes done by messing around with the PM bits on the bridge, not on the downstream devices. Jeff -- Jeff Garzik | Andre the Giant has a posse. Building 1024| MandrakeSoft | - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: pci_disable_device() vs. arch
Benjamin Herrenschmidt wrote: Hi ! Would it make sense to add a pcibios_disable_device(pci_dev*) called from the end of pci_disable_device() ? I'm adding a call to it to sungem along with other pmac stuffs so that the chip can be properly power down (actually it's not really powered down but unclocked) after module removal. Of course, the arch code must be able to catch it in order to play with the various UniNorth control bits. What arch-specific things need to be done? arch-specific pcibios_disable_device may be a good idea... but in this case it sounds like you need to put #ifdef CONFIG_ALL_PPC code in sungem.c instead, if the power-down code is specific to gmacs. Note that my current gmac driver does shut the chip down when the interface is down, which makes it a bit more useful for laptops as most users currently compile the driver in the kernel. Although some drivers already do this, you really need an inactivity timer instead of unconditionally powering-down the hardware on dev-stop(). dhcp and other applications will often bounce the interface... I have nothing about changing the policy if you prefer so that users will now have to rmmod the driver once done with the interface to save power. For power-down specifically, you should use pci-set-power-state not pci-disable-device. pci_disable_device is the opposite of pci_enable_device. pci_enable_device not only wakes up the device, but also assigns resources. Which implies that pci-disable-device is allowed to un-assign resources. There shouldn't be a problem with a net device doing that per se, but you should be aware of the implications. Jeff -- Jeff Garzik | Andre the Giant has a posse. Building 1024| MandrakeSoft | - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
