* [PATCH] Libertas: Fix issues while configuring host sleep
@ 2009-11-05 1:06 Bing Zhao
2009-11-05 21:36 ` Dan Williams
2009-11-06 20:19 ` John W. Linville
0 siblings, 2 replies; 7+ messages in thread
From: Bing Zhao @ 2009-11-05 1:06 UTC (permalink / raw)
To: libertas-dev; +Cc: linux-wireless, Bing Zhao, Amitkumar Karwar
From: Amitkumar Karwar <akarwar@marvell.com>
Configuration of wake-on-lan for unicast, multicast, broadcast, physical
activity was not working. Kernel panic issue was there when user tries to
disable WOL. Fixed them.
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
Signed-off-by: Bing Zhao <bzhao@marvell.com>
---
drivers/net/wireless/libertas/ethtool.c | 16 +++++++++++-----
1 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/libertas/ethtool.c b/drivers/net/wireless/libertas/ethtool.c
index 039b555..eeda6d7 100644
--- a/drivers/net/wireless/libertas/ethtool.c
+++ b/drivers/net/wireless/libertas/ethtool.c
@@ -169,16 +169,22 @@ static int lbs_ethtool_set_wol(struct net_device *dev,
struct lbs_private *priv = dev->ml_priv;
uint32_t criteria = 0;
- if (priv->wol_criteria == 0xffffffff && wol->wolopts)
+ if (priv->wol_criteria != 0xffffffff && wol->wolopts)
return -EOPNOTSUPP;
if (wol->wolopts & ~(WAKE_UCAST|WAKE_MCAST|WAKE_BCAST|WAKE_PHY))
return -EOPNOTSUPP;
- if (wol->wolopts & WAKE_UCAST) criteria |= EHS_WAKE_ON_UNICAST_DATA;
- if (wol->wolopts & WAKE_MCAST) criteria |= EHS_WAKE_ON_MULTICAST_DATA;
- if (wol->wolopts & WAKE_BCAST) criteria |= EHS_WAKE_ON_BROADCAST_DATA;
- if (wol->wolopts & WAKE_PHY) criteria |= EHS_WAKE_ON_MAC_EVENT;
+ if (wol->wolopts & WAKE_UCAST)
+ criteria |= EHS_WAKE_ON_UNICAST_DATA;
+ if (wol->wolopts & WAKE_MCAST)
+ criteria |= EHS_WAKE_ON_MULTICAST_DATA;
+ if (wol->wolopts & WAKE_BCAST)
+ criteria |= EHS_WAKE_ON_BROADCAST_DATA;
+ if (wol->wolopts & WAKE_PHY)
+ criteria |= EHS_WAKE_ON_MAC_EVENT;
+ if (wol->wolopts == 0)
+ criteria |= EHS_REMOVE_WAKEUP;
return lbs_host_sleep_cfg(priv, criteria, (struct wol_config *)NULL);
}
--
1.5.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Libertas: Fix issues while configuring host sleep
2009-11-05 1:06 [PATCH] Libertas: Fix issues while configuring host sleep Bing Zhao
@ 2009-11-05 21:36 ` Dan Williams
2009-11-06 20:19 ` John W. Linville
1 sibling, 0 replies; 7+ messages in thread
From: Dan Williams @ 2009-11-05 21:36 UTC (permalink / raw)
To: Bing Zhao; +Cc: libertas-dev, Amitkumar Karwar, linux-wireless
On Wed, 2009-11-04 at 17:06 -0800, Bing Zhao wrote:
> From: Amitkumar Karwar <akarwar@marvell.com>
>
> Configuration of wake-on-lan for unicast, multicast, broadcast, physical
> activity was not working. Kernel panic issue was there when user tries to
> disable WOL. Fixed them.
>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> Signed-off-by: Bing Zhao <bzhao@marvell.com>
Acked-by: Dan Williams <dcbw@redhat.com>
> ---
> drivers/net/wireless/libertas/ethtool.c | 16 +++++++++++-----
> 1 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/libertas/ethtool.c b/drivers/net/wireless/libertas/ethtool.c
> index 039b555..eeda6d7 100644
> --- a/drivers/net/wireless/libertas/ethtool.c
> +++ b/drivers/net/wireless/libertas/ethtool.c
> @@ -169,16 +169,22 @@ static int lbs_ethtool_set_wol(struct net_device *dev,
> struct lbs_private *priv = dev->ml_priv;
> uint32_t criteria = 0;
>
> - if (priv->wol_criteria == 0xffffffff && wol->wolopts)
> + if (priv->wol_criteria != 0xffffffff && wol->wolopts)
> return -EOPNOTSUPP;
>
> if (wol->wolopts & ~(WAKE_UCAST|WAKE_MCAST|WAKE_BCAST|WAKE_PHY))
> return -EOPNOTSUPP;
>
> - if (wol->wolopts & WAKE_UCAST) criteria |= EHS_WAKE_ON_UNICAST_DATA;
> - if (wol->wolopts & WAKE_MCAST) criteria |= EHS_WAKE_ON_MULTICAST_DATA;
> - if (wol->wolopts & WAKE_BCAST) criteria |= EHS_WAKE_ON_BROADCAST_DATA;
> - if (wol->wolopts & WAKE_PHY) criteria |= EHS_WAKE_ON_MAC_EVENT;
> + if (wol->wolopts & WAKE_UCAST)
> + criteria |= EHS_WAKE_ON_UNICAST_DATA;
> + if (wol->wolopts & WAKE_MCAST)
> + criteria |= EHS_WAKE_ON_MULTICAST_DATA;
> + if (wol->wolopts & WAKE_BCAST)
> + criteria |= EHS_WAKE_ON_BROADCAST_DATA;
> + if (wol->wolopts & WAKE_PHY)
> + criteria |= EHS_WAKE_ON_MAC_EVENT;
> + if (wol->wolopts == 0)
> + criteria |= EHS_REMOVE_WAKEUP;
>
> return lbs_host_sleep_cfg(priv, criteria, (struct wol_config *)NULL);
> }
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Libertas: Fix issues while configuring host sleep
2009-11-05 1:06 [PATCH] Libertas: Fix issues while configuring host sleep Bing Zhao
2009-11-05 21:36 ` Dan Williams
@ 2009-11-06 20:19 ` John W. Linville
2009-11-06 21:27 ` Bing Zhao
1 sibling, 1 reply; 7+ messages in thread
From: John W. Linville @ 2009-11-06 20:19 UTC (permalink / raw)
To: Bing Zhao; +Cc: libertas-dev, linux-wireless, Amitkumar Karwar
On Wed, Nov 04, 2009 at 05:06:35PM -0800, Bing Zhao wrote:
> From: Amitkumar Karwar <akarwar@marvell.com>
>
> Configuration of wake-on-lan for unicast, multicast, broadcast, physical
> activity was not working. Kernel panic issue was there when user tries to
> disable WOL. Fixed them.
>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> Signed-off-by: Bing Zhao <bzhao@marvell.com>
> ---
> drivers/net/wireless/libertas/ethtool.c | 16 +++++++++++-----
> 1 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/libertas/ethtool.c b/drivers/net/wireless/libertas/ethtool.c
> index 039b555..eeda6d7 100644
> --- a/drivers/net/wireless/libertas/ethtool.c
> +++ b/drivers/net/wireless/libertas/ethtool.c
> @@ -169,16 +169,22 @@ static int lbs_ethtool_set_wol(struct net_device *dev,
> struct lbs_private *priv = dev->ml_priv;
> uint32_t criteria = 0;
>
> - if (priv->wol_criteria == 0xffffffff && wol->wolopts)
> + if (priv->wol_criteria != 0xffffffff && wol->wolopts)
> return -EOPNOTSUPP;
Are you sure about this? This makes me think that you won't be able
to change WoL parameters without going through a disable step first.
Am I misreading?
> if (wol->wolopts & ~(WAKE_UCAST|WAKE_MCAST|WAKE_BCAST|WAKE_PHY))
> return -EOPNOTSUPP;
>
> - if (wol->wolopts & WAKE_UCAST) criteria |= EHS_WAKE_ON_UNICAST_DATA;
> - if (wol->wolopts & WAKE_MCAST) criteria |= EHS_WAKE_ON_MULTICAST_DATA;
> - if (wol->wolopts & WAKE_BCAST) criteria |= EHS_WAKE_ON_BROADCAST_DATA;
> - if (wol->wolopts & WAKE_PHY) criteria |= EHS_WAKE_ON_MAC_EVENT;
> + if (wol->wolopts & WAKE_UCAST)
> + criteria |= EHS_WAKE_ON_UNICAST_DATA;
> + if (wol->wolopts & WAKE_MCAST)
> + criteria |= EHS_WAKE_ON_MULTICAST_DATA;
> + if (wol->wolopts & WAKE_BCAST)
> + criteria |= EHS_WAKE_ON_BROADCAST_DATA;
> + if (wol->wolopts & WAKE_PHY)
> + criteria |= EHS_WAKE_ON_MAC_EVENT;
> + if (wol->wolopts == 0)
> + criteria |= EHS_REMOVE_WAKEUP;
>
> return lbs_host_sleep_cfg(priv, criteria, (struct wol_config *)NULL);
> }
The reformatting is a distraction. It would be better to do just
the fix part separately, especially if you are targetting 2.6.32.
John
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] Libertas: Fix issues while configuring host sleep
2009-11-06 20:19 ` John W. Linville
@ 2009-11-06 21:27 ` Bing Zhao
2009-11-07 10:53 ` Kalle Valo
2009-11-10 0:06 ` Bing Zhao
0 siblings, 2 replies; 7+ messages in thread
From: Bing Zhao @ 2009-11-06 21:27 UTC (permalink / raw)
To: John W. Linville; +Cc: libertas-dev, linux-wireless, Amitkumar Karwar
Hi John,
> -----Original Message-----
> From: John W. Linville [mailto:linville@tuxdriver.com]
> Sent: Friday, November 06, 2009 12:19 PM
> To: Bing Zhao
> Cc: libertas-dev@lists.infradead.org; linux-wireless@vger.kernel.org; Amitkumar Karwar
> Subject: Re: [PATCH] Libertas: Fix issues while configuring host sleep
>
> On Wed, Nov 04, 2009 at 05:06:35PM -0800, Bing Zhao wrote:
> > From: Amitkumar Karwar <akarwar@marvell.com>
> >
> > Configuration of wake-on-lan for unicast, multicast, broadcast, physical
> > activity was not working. Kernel panic issue was there when user tries to
> > disable WOL. Fixed them.
> >
> > Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> > Signed-off-by: Bing Zhao <bzhao@marvell.com>
> > ---
> > drivers/net/wireless/libertas/ethtool.c | 16 +++++++++++-----
> > 1 files changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/wireless/libertas/ethtool.c b/drivers/net/wireless/libertas/ethtool.c
> > index 039b555..eeda6d7 100644
> > --- a/drivers/net/wireless/libertas/ethtool.c
> > +++ b/drivers/net/wireless/libertas/ethtool.c
> > @@ -169,16 +169,22 @@ static int lbs_ethtool_set_wol(struct net_device *dev,
> > struct lbs_private *priv = dev->ml_priv;
> > uint32_t criteria = 0;
> >
> > - if (priv->wol_criteria == 0xffffffff && wol->wolopts)
> > + if (priv->wol_criteria != 0xffffffff && wol->wolopts)
> > return -EOPNOTSUPP;
>
> Are you sure about this? This makes me think that you won't be able
> to change WoL parameters without going through a disable step first.
> Am I misreading?
I'm not sure if this is the best fix or not. But without this change, "ethtool -s wlan0 wol u" would return "Cannot set new wake-on-lan settings: Operation not supported". Yes, you have to disable it first in order to change the WoL parameters.
>
> > if (wol->wolopts & ~(WAKE_UCAST|WAKE_MCAST|WAKE_BCAST|WAKE_PHY))
> > return -EOPNOTSUPP;
> >
> > - if (wol->wolopts & WAKE_UCAST) criteria |= EHS_WAKE_ON_UNICAST_DATA;
> > - if (wol->wolopts & WAKE_MCAST) criteria |= EHS_WAKE_ON_MULTICAST_DATA;
> > - if (wol->wolopts & WAKE_BCAST) criteria |= EHS_WAKE_ON_BROADCAST_DATA;
> > - if (wol->wolopts & WAKE_PHY) criteria |= EHS_WAKE_ON_MAC_EVENT;
> > + if (wol->wolopts & WAKE_UCAST)
> > + criteria |= EHS_WAKE_ON_UNICAST_DATA;
> > + if (wol->wolopts & WAKE_MCAST)
> > + criteria |= EHS_WAKE_ON_MULTICAST_DATA;
> > + if (wol->wolopts & WAKE_BCAST)
> > + criteria |= EHS_WAKE_ON_BROADCAST_DATA;
> > + if (wol->wolopts & WAKE_PHY)
> > + criteria |= EHS_WAKE_ON_MAC_EVENT;
> > + if (wol->wolopts == 0)
> > + criteria |= EHS_REMOVE_WAKEUP;
> >
> > return lbs_host_sleep_cfg(priv, criteria, (struct wol_config *)NULL);
> > }
>
> The reformatting is a distraction. It would be better to do just
> the fix part separately, especially if you are targetting 2.6.32.
Without the reformatting, the change would be like this:
+ if (wol->wolopts == 0) criteria |= EHS_REMOVE_WAKEUP;
But the "checkpatch.pl" script gave me an error on that:
"ERROR: trailing statements should be on next line"
Thanks,
Bing
> John
> --
> John W. Linville Someday the world will need a hero, and you
> linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Libertas: Fix issues while configuring host sleep
2009-11-06 21:27 ` Bing Zhao
@ 2009-11-07 10:53 ` Kalle Valo
2009-11-10 0:00 ` Bing Zhao
2009-11-10 0:06 ` Bing Zhao
1 sibling, 1 reply; 7+ messages in thread
From: Kalle Valo @ 2009-11-07 10:53 UTC (permalink / raw)
To: Bing Zhao
Cc: John W. Linville, libertas-dev, linux-wireless, Amitkumar Karwar
Bing Zhao <bzhao@marvell.com> writes:
>> The reformatting is a distraction. It would be better to do just
>> the fix part separately, especially if you are targetting 2.6.32.
>
> Without the reformatting, the change would be like this:
>
> + if (wol->wolopts == 0) criteria |= EHS_REMOVE_WAKEUP;
>
> But the "checkpatch.pl" script gave me an error on that:
> "ERROR: trailing statements should be on next line"
You can do the reformatting in the first patch and the second patch
would contain the actual fix.
--
Kalle Valo
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] Libertas: Fix issues while configuring host sleep
2009-11-07 10:53 ` Kalle Valo
@ 2009-11-10 0:00 ` Bing Zhao
0 siblings, 0 replies; 7+ messages in thread
From: Bing Zhao @ 2009-11-10 0:00 UTC (permalink / raw)
To: Kalle Valo
Cc: John W. Linville, libertas-dev, linux-wireless, Amitkumar Karwar
Hi Kalle,
> -----Original Message-----
> From: Kalle Valo [mailto:kalle.valo@gmail.com] On Behalf Of Kalle Valo
> Sent: Saturday, November 07, 2009 2:54 AM
> To: Bing Zhao
> Cc: John W. Linville; libertas-dev@lists.infradead.org; linux-wireless@vger.kernel.org; Amitkumar
> Karwar
> Subject: Re: [PATCH] Libertas: Fix issues while configuring host sleep
>
> Bing Zhao <bzhao@marvell.com> writes:
>
> >> The reformatting is a distraction. It would be better to do just
> >> the fix part separately, especially if you are targetting 2.6.32.
> >
> > Without the reformatting, the change would be like this:
> >
> > + if (wol->wolopts == 0) criteria |= EHS_REMOVE_WAKEUP;
> >
> > But the "checkpatch.pl" script gave me an error on that:
> > "ERROR: trailing statements should be on next line"
>
> You can do the reformatting in the first patch and the second patch
> would contain the actual fix.
I will re-submit as suggested.
Thanks,
Bing
>
> --
> Kalle Valo
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] Libertas: Fix issues while configuring host sleep
2009-11-06 21:27 ` Bing Zhao
2009-11-07 10:53 ` Kalle Valo
@ 2009-11-10 0:06 ` Bing Zhao
1 sibling, 0 replies; 7+ messages in thread
From: Bing Zhao @ 2009-11-10 0:06 UTC (permalink / raw)
To: John W. Linville; +Cc: Amitkumar Karwar, linux-wireless, libertas-dev
Hi John,
> -----Original Message-----
> From: libertas-dev-bounces@lists.infradead.org [mailto:libertas-dev-bounces@lists.infradead.org] On
> Behalf Of Bing Zhao
> Sent: Friday, November 06, 2009 1:28 PM
> To: John W. Linville
> Cc: Amitkumar Karwar; linux-wireless@vger.kernel.org; libertas-dev@lists.infradead.org
> Subject: RE: [PATCH] Libertas: Fix issues while configuring host sleep
>
> Hi John,
>
> > -----Original Message-----
> > From: John W. Linville [mailto:linville@tuxdriver.com]
> > Sent: Friday, November 06, 2009 12:19 PM
> > To: Bing Zhao
> > Cc: libertas-dev@lists.infradead.org; linux-wireless@vger.kernel.org; Amitkumar Karwar
> > Subject: Re: [PATCH] Libertas: Fix issues while configuring host sleep
> >
> > On Wed, Nov 04, 2009 at 05:06:35PM -0800, Bing Zhao wrote:
> > > From: Amitkumar Karwar <akarwar@marvell.com>
> > >
> > > Configuration of wake-on-lan for unicast, multicast, broadcast, physical
> > > activity was not working. Kernel panic issue was there when user tries to
> > > disable WOL. Fixed them.
> > >
> > > Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> > > Signed-off-by: Bing Zhao <bzhao@marvell.com>
> > > ---
> > > drivers/net/wireless/libertas/ethtool.c | 16 +++++++++++-----
> > > 1 files changed, 11 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/libertas/ethtool.c b/drivers/net/wireless/libertas/ethtool.c
> > > index 039b555..eeda6d7 100644
> > > --- a/drivers/net/wireless/libertas/ethtool.c
> > > +++ b/drivers/net/wireless/libertas/ethtool.c
> > > @@ -169,16 +169,22 @@ static int lbs_ethtool_set_wol(struct net_device *dev,
> > > struct lbs_private *priv = dev->ml_priv;
> > > uint32_t criteria = 0;
> > >
> > > - if (priv->wol_criteria == 0xffffffff && wol->wolopts)
> > > + if (priv->wol_criteria != 0xffffffff && wol->wolopts)
> > > return -EOPNOTSUPP;
> >
> > Are you sure about this? This makes me think that you won't be able
> > to change WoL parameters without going through a disable step first.
> > Am I misreading?
>
> I'm not sure if this is the best fix or not. But without this change, "ethtool -s wlan0 wol u" would
> return "Cannot set new wake-on-lan settings: Operation not supported". Yes, you have to disable it
> first in order to change the WoL parameters.
>
We can probably just remove the check so that we can change the WoL settings without disabling first.
- if (priv->wol_criteria == 0xffffffff && wol->wolopts)
- return -EOPNOTSUPP;
I will re-send the patch.
Thanks,
Bing
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-11-10 0:06 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-05 1:06 [PATCH] Libertas: Fix issues while configuring host sleep Bing Zhao
2009-11-05 21:36 ` Dan Williams
2009-11-06 20:19 ` John W. Linville
2009-11-06 21:27 ` Bing Zhao
2009-11-07 10:53 ` Kalle Valo
2009-11-10 0:00 ` Bing Zhao
2009-11-10 0:06 ` Bing Zhao
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.