Hi,
the commit in subject make the kernel with CONFIG_PM_RUNTIME unset fail
during compilation, as struct dev_pm_info doesn't have whole bunch of
members in such case.
The commit in question adds this code:
/* we have to throw away the rest */
do {
unbusy_queued_urb(urb, portdata);
//extremely dirty
atomic_dec(&port->serial->interface->dev.power.usage_count);
} while ((urb = usb_get_from_anchor(&portdata->delayed)));
The 'extermely dirty' comment makes me a bit nervous whether the patch
below is correct or some more thinking would be necessary.
From: Jiri Kosina <[email protected]>
Subject: [PATCH] USB: usb_wvan: fix compilation for !CONFIG_PM_RUNTIME case
With CONFIG_PM_RUNTIME unset, struct dev_pm_info doesn't have
usage count which is used only for run-time power management
purposes.
Signed-off-by: Jiri Kosina <[email protected]>
---
drivers/usb/serial/usb_wwan.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c
index a65ddd5..8b456dc 100644
--- a/drivers/usb/serial/usb_wwan.c
+++ b/drivers/usb/serial/usb_wwan.c
@@ -698,8 +698,10 @@ static void play_delayed(struct usb_serial_port *port)
/* we have to throw away the rest */
do {
unbusy_queued_urb(urb, portdata);
- //extremely dirty
+#ifdef CONFIG_PM_RUNTIME
+ /* extremely dirty */
atomic_dec(&port->serial->interface->dev.power.usage_count);
+#endif
} while ((urb = usb_get_from_anchor(&portdata->delayed)));
break;
}
--
1.7.3.1
[ fixing Oliver's address in CC ]
On Mon, 21 Mar 2011, Jiri Kosina wrote:
> Hi,
>
> the commit in subject make the kernel with CONFIG_PM_RUNTIME unset fail
> during compilation, as struct dev_pm_info doesn't have whole bunch of
> members in such case.
>
> The commit in question adds this code:
>
> /* we have to throw away the rest */
> do {
> unbusy_queued_urb(urb, portdata);
> //extremely dirty
> atomic_dec(&port->serial->interface->dev.power.usage_count);
> } while ((urb = usb_get_from_anchor(&portdata->delayed)));
>
> The 'extermely dirty' comment makes me a bit nervous whether the patch
> below is correct or some more thinking would be necessary.
>
>
>
> From: Jiri Kosina <[email protected]>
> Subject: [PATCH] USB: usb_wvan: fix compilation for !CONFIG_PM_RUNTIME case
>
> With CONFIG_PM_RUNTIME unset, struct dev_pm_info doesn't have
> usage count which is used only for run-time power management
> purposes.
>
> Signed-off-by: Jiri Kosina <[email protected]>
> ---
> drivers/usb/serial/usb_wwan.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c
> index a65ddd5..8b456dc 100644
> --- a/drivers/usb/serial/usb_wwan.c
> +++ b/drivers/usb/serial/usb_wwan.c
> @@ -698,8 +698,10 @@ static void play_delayed(struct usb_serial_port *port)
> /* we have to throw away the rest */
> do {
> unbusy_queued_urb(urb, portdata);
> - //extremely dirty
> +#ifdef CONFIG_PM_RUNTIME
> + /* extremely dirty */
> atomic_dec(&port->serial->interface->dev.power.usage_count);
> +#endif
> } while ((urb = usb_get_from_anchor(&portdata->delayed)));
> break;
> }
> --
> 1.7.3.1
>
>
--
Jiri Kosina
SUSE Labs, Novell Inc.
On Mon, 21 Mar 2011 01:41:14 +0100 (CET) Jiri Kosina wrote:
>
> [ fixing Oliver's address in CC ]
>
> On Mon, 21 Mar 2011, Jiri Kosina wrote:
>
> > Hi,
> >
> > the commit in subject make the kernel with CONFIG_PM_RUNTIME unset fail
> > during compilation, as struct dev_pm_info doesn't have whole bunch of
> > members in such case.
> >
> > The commit in question adds this code:
> >
> > /* we have to throw away the rest */
> > do {
> > unbusy_queued_urb(urb, portdata);
> > //extremely dirty
> > atomic_dec(&port->serial->interface->dev.power.usage_count);
> > } while ((urb = usb_get_from_anchor(&portdata->delayed)));
> >
> > The 'extermely dirty' comment makes me a bit nervous whether the patch
> > below is correct or some more thinking would be necessary.
> >
> >
> >
> > From: Jiri Kosina <[email protected]>
> > Subject: [PATCH] USB: usb_wvan: fix compilation for !CONFIG_PM_RUNTIME case
> >
> > With CONFIG_PM_RUNTIME unset, struct dev_pm_info doesn't have
> > usage count which is used only for run-time power management
> > purposes.
> >
> > Signed-off-by: Jiri Kosina <[email protected]>
> > ---
> > drivers/usb/serial/usb_wwan.c | 4 +++-
> > 1 files changed, 3 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c
> > index a65ddd5..8b456dc 100644
> > --- a/drivers/usb/serial/usb_wwan.c
> > +++ b/drivers/usb/serial/usb_wwan.c
> > @@ -698,8 +698,10 @@ static void play_delayed(struct usb_serial_port *port)
> > /* we have to throw away the rest */
> > do {
> > unbusy_queued_urb(urb, portdata);
> > - //extremely dirty
> > +#ifdef CONFIG_PM_RUNTIME
> > + /* extremely dirty */
> > atomic_dec(&port->serial->interface->dev.power.usage_count);
> > +#endif
> > } while ((urb = usb_get_from_anchor(&portdata->delayed)));
> > break;
> > }
> > --
> > 1.7.3.1
> >
> >
Please see Oliver's patch for this at
http://marc.info/?l=linux-usb&m=130044862323463&w=2
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
Am Montag, 21. M?rz 2011, 01:36:45 schrieb Jiri Kosina:
> Hi,
>
> the commit in subject make the kernel with CONFIG_PM_RUNTIME unset fail
> during compilation, as struct dev_pm_info doesn't have whole bunch of
> members in such case.
>
> The commit in question adds this code:
>
> /* we have to throw away the rest */
> do {
> unbusy_queued_urb(urb, portdata);
> //extremely dirty
> atomic_dec(&port->serial->interface->dev.power.usage_count);
> } while ((urb = usb_get_from_anchor(&portdata->delayed)));
>
> The 'extermely dirty' comment makes me a bit nervous whether the patch
> below is correct or some more thinking would be necessary.
I've since posted a clean patch, although your fix looks correct to me.
The problem was that I was unsure about the locking during resume().
I rather wrote a layering violation than risk a deadlock in an error path
hard to test. But I did not feel well doing it.
Regards
Oliver
On Mon, 21 Mar 2011, Oliver Neukum wrote:
> Am Montag, 21. M?rz 2011, 01:36:45 schrieb Jiri Kosina:
> > Hi,
> >
> > the commit in subject make the kernel with CONFIG_PM_RUNTIME unset fail
> > during compilation, as struct dev_pm_info doesn't have whole bunch of
> > members in such case.
> >
> > The commit in question adds this code:
> >
> > /* we have to throw away the rest */
> > do {
> > unbusy_queued_urb(urb, portdata);
> > //extremely dirty
> > atomic_dec(&port->serial->interface->dev.power.usage_count);
> > } while ((urb = usb_get_from_anchor(&portdata->delayed)));
> >
> > The 'extermely dirty' comment makes me a bit nervous whether the patch
> > below is correct or some more thinking would be necessary.
>
> I've since posted a clean patch, although your fix looks correct to me.
In fact the original code and Jiri's fix are both wrong. The layering
violation here is not only inelegant, it is downright buggy.
The USB core contains its own usage_count for interfaces, in parallel
with the usage_count in interface->dev.power. Bypassing the USB layer
in this way causes the two counts to get out of sync, which will lead
to errors.
Oliver's new fix is correct.
Alan Stern