On Thursday 09 June 2005 8:39 pm, Todd Poynor wrote:
> Thanks, I've made your suggested changes below. Since I have an
> OMAP1510 handy I can make those same changes to ohci-omap too if you
> like.
Sure, that'd help. Thanks!
> So far as I thought I knew, pm_message_t will eventually become a struct
> with fields that can be used to properly describe what sort of driver
> behavior is needed at suspend and resume, but I might be behind the
> times.
That's one theory; I can't see it checking out though. That whole
episode of converting to pm_message_t required drivers to rip out
any such intelligence ... stuff that's actually hard to get right,
so the notion of breaking drivers _again_ just to put such code
back holds virtually no attraction. PMSG_FREEZE can't even kick
in without hitting the same problems.
If the idea was to have drivers look at the parameter and then do
the right thing, it would have been a LOT better to not go through
that detour where drivers are prevented from doing the right thing.
> Since system suspend currently forces a pm_message == ACPI D3
> for all suspend modes (and previously a 3 was forced as well), we'd have
> to work outside the system to apply any intelligence to system suspend
> handling anyhow, before or after the pm_message_t conversion. But I
> suppose we can battle that out on linux-pm if it comes to that.
It was only swsusp that forced "3"; the other 2.6 code, now deleted,
was doing PM_SUSPEND_MEM (3) or PM_SUSPEND_DISK (4). It never made
sense to me why swsusp didn't even try to use that framework, which
was altogether better factored, and worked sanely with PCI. When
everything except the swsusp stuff got ripped out, it wasn't a "best
parts of both schemes survive" thing. But yes, this isn't the list
to try fixing any of that stuff.
This patch reads fine to me except that you're not setting the
dev->power.power_state to (bleech) PMSG_SUSPEND or PMSG_ON,
which means that operations through sysfs power/state files will
be more borked than usual ... not all paths through the driver
core will maintain that field, so drivers need to. At least,
so long as the field exists. It's mostly just trouble. :)
- Dave
>
> Thanks -- Todd
>
> Index: linux-2.6.12-rc4/drivers/usb/host/ohci-pxa27x.c
> ===================================================================
> --- linux-2.6.12-rc4.orig/drivers/usb/host/ohci-pxa27x.c 2005-06-09
> 02:56:43.000000000 +0000
> +++ linux-2.6.12-rc4/drivers/usb/host/ohci-pxa27x.c 2005-06-10
> 03:02:38.000000000 +0000
> @@ -310,6 +310,7 @@
> .hub_suspend = ohci_hub_suspend,
> .hub_resume = ohci_hub_resume,
> #endif
> + .start_port_reset = ohci_start_port_reset,
> };
>
> /*-------------------------------------------------------------------------*/
> @@ -337,32 +338,65 @@
> return 0;
> }
>
> -static int ohci_hcd_pxa27x_drv_suspend(struct device *dev, pm_message_t
> state, u32 level)
> +
> +#ifdef CONFIG_PM
> +
> +static int ohci_hcd_pxa27x_drv_suspend(struct device *dev, pm_message_t
> message, u32 level)
> {
> -// struct platform_device *pdev = to_platform_device(dev);
> -// struct usb_hcd *hcd = dev_get_drvdata(dev);
> - printk("%s: not implemented yet\n", __FUNCTION__);
> + struct platform_device *pdev = to_platform_device(dev);
> + struct ohci_hcd *ohci = hcd_to_ohci(dev_get_drvdata(dev));
> + int status = -EINVAL;
>
> - return 0;
> + if (level != SUSPEND_POWER_DOWN)
> + return 0;
> +
> + down(&ohci_to_hcd(ohci)->self.root_hub->serialize);
> + status = ohci_hub_suspend(ohci_to_hcd(ohci));
> + if (status == 0) {
> + pxa27x_stop_hc(pdev);
> + ohci_to_hcd(ohci)->self.root_hub->state =
> + USB_STATE_SUSPENDED;
> + ohci_to_hcd(ohci)->state = HC_STATE_SUSPENDED;
> + }
> + up(&ohci_to_hcd(ohci)->self.root_hub->serialize);
> + return status;
> }
>
> static int ohci_hcd_pxa27x_drv_resume(struct device *dev, u32 level)
> {
> -// struct platform_device *pdev = to_platform_device(dev);
> -// struct usb_hcd *hcd = dev_get_drvdata(dev);
> - printk("%s: not implemented yet\n", __FUNCTION__);
> + struct platform_device *pdev = to_platform_device(dev);
> + struct ohci_hcd *ohci = hcd_to_ohci(dev_get_drvdata(dev));
> + int status = 0;
>
> - return 0;
> + if (level != RESUME_POWER_ON)
> + return 0;
> +
> + if (time_before(jiffies, ohci->next_statechange))
> + msleep(5);
> + ohci->next_statechange = jiffies;
> + pxa27x_start_hc(pdev);
> +#ifdef CONFIG_USB_SUSPEND
> + /* get extra cleanup even if remote wakeup isn't in use */
> + status = usb_resume_device(ohci_to_hcd(ohci)->self.root_hub);
> +#else
> + down(&ohci_to_hcd(ohci)->self.root_hub->serialize);
> + status = ohci_hub_resume(ohci_to_hcd(ohci));
> + up(&ohci_to_hcd(ohci)->self.root_hub->serialize);
> +#endif
> + return status;
> }
>
> +#endif
>
> static struct device_driver ohci_hcd_pxa27x_driver = {
> .name = "pxa27x-ohci",
> .bus = &platform_bus_type,
> .probe = ohci_hcd_pxa27x_drv_probe,
> .remove = ohci_hcd_pxa27x_drv_remove,
> +#ifdef CONFIG_PM
> .suspend = ohci_hcd_pxa27x_drv_suspend,
> - .resume = ohci_hcd_pxa27x_drv_resume,
> + .resume = ohci_hcd_pxa27x_drv_resume,
> +#endif
> };
>
> static int __init ohci_hcd_pxa27x_init (void)
>
>
-------------------------------------------------------
This SF.Net email is sponsored by: NEC IT Guy Games. How far can you shotput
a projector? How fast can you ride your desk chair down the office luge track?
If you want to score the big prize, get to know the little guy.
Play to win an NEC 61" plasma display: http://www.necitguy.com/?r=20
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
