* [GIT PATCH] rfkill fixes for 2.6.28-rc3
@ 2008-11-03 16:42 Henrique de Moraes Holschuh
2008-11-03 16:42 ` [PATCH 1/2] rfkill: preserve state across suspend Henrique de Moraes Holschuh
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Henrique de Moraes Holschuh @ 2008-11-03 16:42 UTC (permalink / raw)
To: John Linville; +Cc: linux-wireless, Ivo van Doorn
John, Ivo,
This small patchset contains two fixes to issues in the suspend/resume
handling of the rfkill class core.
It needs to go to 2.6.28. These patches are based on 2.6.28-rc3.
Chances are they will *clash* with what is in wireless-testing, so I will
send a rebased patchset for wireless-testing with these patches on the
bottom of the stack (to make merging easier with mainline).
The patches are also needed on stable, but won't apply automatically. I
don't think I will bother with 2.6.26 since the suspend/resume restore of
radio state there is not going to work well anyway, unless I backport a bit
more stuff... but 2.6.27 deserves to get them.
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/2] rfkill: preserve state across suspend
2008-11-03 16:42 [GIT PATCH] rfkill fixes for 2.6.28-rc3 Henrique de Moraes Holschuh
@ 2008-11-03 16:42 ` Henrique de Moraes Holschuh
2008-11-03 17:20 ` Ivo van Doorn
2008-11-03 16:42 ` [PATCH 2/2] rfkill: always call get_state() hook on resume Henrique de Moraes Holschuh
2008-11-03 16:47 ` [GIT PATCH] rfkill fixes for 2.6.28-rc3 Ivo van Doorn
2 siblings, 1 reply; 18+ messages in thread
From: Henrique de Moraes Holschuh @ 2008-11-03 16:42 UTC (permalink / raw)
To: John Linville
Cc: linux-wireless, Ivo van Doorn, Henrique de Moraes Holschuh,
Ivo van Doorn, Matthew Garrett, Alan Jenkins
The rfkill class API requires that the driver connected to a class
call rfkill_force_state() on resume to update the real state of the
rfkill controller, OR that it provides a get_state() hook.
This means there is potentially a hidden call in the resume code flow
that changes rfkill->state (i.e. rfkill_force_state()), so the
previous state of the transmitter was being lost.
The simplest and most future-proof way to fix this is to explicitly
store the pre-sleep state on the rfkill structure, and restore from
that on resume.
Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Cc: Ivo van Doorn <IvDoorn@gmail.com>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
---
include/linux/rfkill.h | 1 +
net/rfkill/rfkill.c | 7 ++++++-
2 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/include/linux/rfkill.h b/include/linux/rfkill.h
index 4cd64b0..f376a93 100644
--- a/include/linux/rfkill.h
+++ b/include/linux/rfkill.h
@@ -108,6 +108,7 @@ struct rfkill {
struct device dev;
struct list_head node;
+ enum rfkill_state state_for_resume;
};
#define to_rfkill(d) container_of(d, struct rfkill, dev)
diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
index f949a48..caee717 100644
--- a/net/rfkill/rfkill.c
+++ b/net/rfkill/rfkill.c
@@ -511,10 +511,15 @@ static void rfkill_release(struct device *dev)
#ifdef CONFIG_PM
static int rfkill_suspend(struct device *dev, pm_message_t state)
{
+ struct rfkill *rfkill = to_rfkill(dev);
+
/* mark class device as suspended */
if (dev->power.power_state.event != state.event)
dev->power.power_state = state;
+ /* store state for the resume handler */
+ rfkill->state_for_resume = rfkill->state;
+
return 0;
}
@@ -528,7 +533,7 @@ static int rfkill_resume(struct device *dev)
dev->power.power_state.event = PM_EVENT_ON;
/* restore radio state AND notify everybody */
- rfkill_toggle_radio(rfkill, rfkill->state, 1);
+ rfkill_toggle_radio(rfkill, rfkill->state_for_resume, 1);
mutex_unlock(&rfkill->mutex);
}
--
1.5.6.5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/2] rfkill: always call get_state() hook on resume
2008-11-03 16:42 [GIT PATCH] rfkill fixes for 2.6.28-rc3 Henrique de Moraes Holschuh
2008-11-03 16:42 ` [PATCH 1/2] rfkill: preserve state across suspend Henrique de Moraes Holschuh
@ 2008-11-03 16:42 ` Henrique de Moraes Holschuh
2008-11-03 17:20 ` Ivo van Doorn
2008-11-03 16:47 ` [GIT PATCH] rfkill fixes for 2.6.28-rc3 Ivo van Doorn
2 siblings, 1 reply; 18+ messages in thread
From: Henrique de Moraes Holschuh @ 2008-11-03 16:42 UTC (permalink / raw)
To: John Linville
Cc: linux-wireless, Ivo van Doorn, Henrique de Moraes Holschuh,
Ivo van Doorn, Matthew Garrett, Alan Jenkins
We "optimize" away the get_state() hook call on rfkill_toggle_radio
when doing a forced state change. This means the resume path is not
calling get_state() as it should.
Call it manually on the resume handler, as we don't want to mess with
the EPO path by removing the optimization. This has the added benefit
of making it explicit that rfkill->state could have been modified
before we hit the rfkill_toggle_radio() call in the class resume
handler.
Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Cc: Ivo van Doorn <IvDoorn@gmail.com>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
---
net/rfkill/rfkill.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
index caee717..7a82a35 100644
--- a/net/rfkill/rfkill.c
+++ b/net/rfkill/rfkill.c
@@ -526,6 +526,7 @@ static int rfkill_suspend(struct device *dev, pm_message_t state)
static int rfkill_resume(struct device *dev)
{
struct rfkill *rfkill = to_rfkill(dev);
+ enum rfkill_state newstate;
if (dev->power.power_state.event != PM_EVENT_ON) {
mutex_lock(&rfkill->mutex);
@@ -533,6 +534,9 @@ static int rfkill_resume(struct device *dev)
dev->power.power_state.event = PM_EVENT_ON;
/* restore radio state AND notify everybody */
+ if (rfkill->get_state &&
+ !rfkill->get_state(rfkill->data, &newstate))
+ rfkill->state = newstate;
rfkill_toggle_radio(rfkill, rfkill->state_for_resume, 1);
mutex_unlock(&rfkill->mutex);
--
1.5.6.5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [GIT PATCH] rfkill fixes for 2.6.28-rc3
2008-11-03 16:42 [GIT PATCH] rfkill fixes for 2.6.28-rc3 Henrique de Moraes Holschuh
2008-11-03 16:42 ` [PATCH 1/2] rfkill: preserve state across suspend Henrique de Moraes Holschuh
2008-11-03 16:42 ` [PATCH 2/2] rfkill: always call get_state() hook on resume Henrique de Moraes Holschuh
@ 2008-11-03 16:47 ` Ivo van Doorn
2008-11-03 17:02 ` Henrique de Moraes Holschuh
2 siblings, 1 reply; 18+ messages in thread
From: Ivo van Doorn @ 2008-11-03 16:47 UTC (permalink / raw)
To: Henrique de Moraes Holschuh; +Cc: John Linville, linux-wireless
On Monday 03 November 2008, Henrique de Moraes Holschuh wrote:
>
> John, Ivo,
>
> This small patchset contains two fixes to issues in the suspend/resume
> handling of the rfkill class core.
>
> It needs to go to 2.6.28. These patches are based on 2.6.28-rc3.
Are they real regressions or normal bugfixes?
> Chances are they will *clash* with what is in wireless-testing, so I will
> send a rebased patchset for wireless-testing with these patches on the
> bottom of the stack (to make merging easier with mainline).
>
> The patches are also needed on stable, but won't apply automatically. I
> don't think I will bother with 2.6.26 since the suspend/resume restore of
> radio state there is not going to work well anyway, unless I backport a bit
> more stuff... but 2.6.27 deserves to get them.
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GIT PATCH] rfkill fixes for 2.6.28-rc3
2008-11-03 16:47 ` [GIT PATCH] rfkill fixes for 2.6.28-rc3 Ivo van Doorn
@ 2008-11-03 17:02 ` Henrique de Moraes Holschuh
2008-11-03 17:20 ` Ivo van Doorn
0 siblings, 1 reply; 18+ messages in thread
From: Henrique de Moraes Holschuh @ 2008-11-03 17:02 UTC (permalink / raw)
To: Ivo van Doorn
Cc: John Linville, linux-wireless, Matthew Garrett, Alan Jenkins
On Mon, 03 Nov 2008, Ivo van Doorn wrote:
> On Monday 03 November 2008, Henrique de Moraes Holschuh wrote:
> > This small patchset contains two fixes to issues in the suspend/resume
> > handling of the rfkill class core.
> >
> > It needs to go to 2.6.28. These patches are based on 2.6.28-rc3.
>
> Are they real regressions or normal bugfixes?
I am not sure if they regress anything, but I am pretty sure there is no
bugzilla entry about them yet. I have added two more interested parties to
the CC.
So, if there is a strong feeling this would best be held until the next
merge window, I can certainly respin the patches to on top of
wireless-testing...
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] rfkill: preserve state across suspend
2008-11-03 16:42 ` [PATCH 1/2] rfkill: preserve state across suspend Henrique de Moraes Holschuh
@ 2008-11-03 17:20 ` Ivo van Doorn
0 siblings, 0 replies; 18+ messages in thread
From: Ivo van Doorn @ 2008-11-03 17:20 UTC (permalink / raw)
To: Henrique de Moraes Holschuh
Cc: John Linville, linux-wireless, Matthew Garrett, Alan Jenkins
On Monday 03 November 2008, Henrique de Moraes Holschuh wrote:
> The rfkill class API requires that the driver connected to a class
> call rfkill_force_state() on resume to update the real state of the
> rfkill controller, OR that it provides a get_state() hook.
>
> This means there is potentially a hidden call in the resume code flow
> that changes rfkill->state (i.e. rfkill_force_state()), so the
> previous state of the transmitter was being lost.
>
> The simplest and most future-proof way to fix this is to explicitly
> store the pre-sleep state on the rfkill structure, and restore from
> that on resume.
>
> Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
> Cc: Ivo van Doorn <IvDoorn@gmail.com>
> Cc: Matthew Garrett <mjg59@srcf.ucam.org>
> Cc: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
Acked-by: Ivo van Doorn <IvDoorn@gmail.com>
> ---
> include/linux/rfkill.h | 1 +
> net/rfkill/rfkill.c | 7 ++++++-
> 2 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/rfkill.h b/include/linux/rfkill.h
> index 4cd64b0..f376a93 100644
> --- a/include/linux/rfkill.h
> +++ b/include/linux/rfkill.h
> @@ -108,6 +108,7 @@ struct rfkill {
>
> struct device dev;
> struct list_head node;
> + enum rfkill_state state_for_resume;
> };
> #define to_rfkill(d) container_of(d, struct rfkill, dev)
>
> diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
> index f949a48..caee717 100644
> --- a/net/rfkill/rfkill.c
> +++ b/net/rfkill/rfkill.c
> @@ -511,10 +511,15 @@ static void rfkill_release(struct device *dev)
> #ifdef CONFIG_PM
> static int rfkill_suspend(struct device *dev, pm_message_t state)
> {
> + struct rfkill *rfkill = to_rfkill(dev);
> +
> /* mark class device as suspended */
> if (dev->power.power_state.event != state.event)
> dev->power.power_state = state;
>
> + /* store state for the resume handler */
> + rfkill->state_for_resume = rfkill->state;
> +
> return 0;
> }
>
> @@ -528,7 +533,7 @@ static int rfkill_resume(struct device *dev)
> dev->power.power_state.event = PM_EVENT_ON;
>
> /* restore radio state AND notify everybody */
> - rfkill_toggle_radio(rfkill, rfkill->state, 1);
> + rfkill_toggle_radio(rfkill, rfkill->state_for_resume, 1);
>
> mutex_unlock(&rfkill->mutex);
> }
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] rfkill: always call get_state() hook on resume
2008-11-03 16:42 ` [PATCH 2/2] rfkill: always call get_state() hook on resume Henrique de Moraes Holschuh
@ 2008-11-03 17:20 ` Ivo van Doorn
0 siblings, 0 replies; 18+ messages in thread
From: Ivo van Doorn @ 2008-11-03 17:20 UTC (permalink / raw)
To: Henrique de Moraes Holschuh
Cc: John Linville, linux-wireless, Matthew Garrett, Alan Jenkins
On Monday 03 November 2008, Henrique de Moraes Holschuh wrote:
> We "optimize" away the get_state() hook call on rfkill_toggle_radio
> when doing a forced state change. This means the resume path is not
> calling get_state() as it should.
>
> Call it manually on the resume handler, as we don't want to mess with
> the EPO path by removing the optimization. This has the added benefit
> of making it explicit that rfkill->state could have been modified
> before we hit the rfkill_toggle_radio() call in the class resume
> handler.
>
> Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
> Cc: Ivo van Doorn <IvDoorn@gmail.com>
> Cc: Matthew Garrett <mjg59@srcf.ucam.org>
> Cc: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
Acked-by: Ivo van Doorn <IvDoorn@gmail.com>
> ---
> net/rfkill/rfkill.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
> index caee717..7a82a35 100644
> --- a/net/rfkill/rfkill.c
> +++ b/net/rfkill/rfkill.c
> @@ -526,6 +526,7 @@ static int rfkill_suspend(struct device *dev, pm_message_t state)
> static int rfkill_resume(struct device *dev)
> {
> struct rfkill *rfkill = to_rfkill(dev);
> + enum rfkill_state newstate;
>
> if (dev->power.power_state.event != PM_EVENT_ON) {
> mutex_lock(&rfkill->mutex);
> @@ -533,6 +534,9 @@ static int rfkill_resume(struct device *dev)
> dev->power.power_state.event = PM_EVENT_ON;
>
> /* restore radio state AND notify everybody */
> + if (rfkill->get_state &&
> + !rfkill->get_state(rfkill->data, &newstate))
> + rfkill->state = newstate;
> rfkill_toggle_radio(rfkill, rfkill->state_for_resume, 1);
>
> mutex_unlock(&rfkill->mutex);
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GIT PATCH] rfkill fixes for 2.6.28-rc3
2008-11-03 17:02 ` Henrique de Moraes Holschuh
@ 2008-11-03 17:20 ` Ivo van Doorn
2008-11-12 21:15 ` John W. Linville
0 siblings, 1 reply; 18+ messages in thread
From: Ivo van Doorn @ 2008-11-03 17:20 UTC (permalink / raw)
To: Henrique de Moraes Holschuh
Cc: John Linville, linux-wireless, Matthew Garrett, Alan Jenkins
On Monday 03 November 2008, Henrique de Moraes Holschuh wrote:
> On Mon, 03 Nov 2008, Ivo van Doorn wrote:
> > On Monday 03 November 2008, Henrique de Moraes Holschuh wrote:
> > > This small patchset contains two fixes to issues in the suspend/resume
> > > handling of the rfkill class core.
> > >
> > > It needs to go to 2.6.28. These patches are based on 2.6.28-rc3.
> >
> > Are they real regressions or normal bugfixes?
>
> I am not sure if they regress anything, but I am pretty sure there is no
> bugzilla entry about them yet. I have added two more interested parties to
> the CC.
>
> So, if there is a strong feeling this would best be held until the next
> merge window, I can certainly respin the patches to on top of
> wireless-testing...
Well I'll give my ACK to the 2 patches, but I'll let john decide if they
are "regression" enough for 2.6.28. ;)
Ivo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GIT PATCH] rfkill fixes for 2.6.28-rc3
2008-11-03 17:20 ` Ivo van Doorn
@ 2008-11-12 21:15 ` John W. Linville
2008-11-14 0:16 ` Henrique de Moraes Holschuh
2008-11-20 16:27 ` [GIT PATCH] rfkill fixes for 2.6.28-rc3 drago01
0 siblings, 2 replies; 18+ messages in thread
From: John W. Linville @ 2008-11-12 21:15 UTC (permalink / raw)
To: Ivo van Doorn
Cc: Henrique de Moraes Holschuh, linux-wireless, Matthew Garrett,
Alan Jenkins
On Mon, Nov 03, 2008 at 06:20:40PM +0100, Ivo van Doorn wrote:
> On Monday 03 November 2008, Henrique de Moraes Holschuh wrote:
> > On Mon, 03 Nov 2008, Ivo van Doorn wrote:
> > > On Monday 03 November 2008, Henrique de Moraes Holschuh wrote:
> > > > This small patchset contains two fixes to issues in the suspend/resume
> > > > handling of the rfkill class core.
> > > >
> > > > It needs to go to 2.6.28. These patches are based on 2.6.28-rc3.
> > >
> > > Are they real regressions or normal bugfixes?
> >
> > I am not sure if they regress anything, but I am pretty sure there is no
> > bugzilla entry about them yet. I have added two more interested parties to
> > the CC.
> >
> > So, if there is a strong feeling this would best be held until the next
> > merge window, I can certainly respin the patches to on top of
> > wireless-testing...
>
> Well I'll give my ACK to the 2 patches, but I'll let john decide if they
> are "regression" enough for 2.6.28. ;)
So I keep looking at these patches, and I'm not sure about them.
It seems that they restore the rfkill state after resume to what it
was before the suspend. What I am unsure about is whether or not
that is the appropriate thing to do. I suppose it makes sense under
the rule of least surprise.
I guess the safer alternative would be to always resume with rfkill
enabled, but that is probably undesirable in the most common case.
So, I have no objection to these patches.
Still, I don't see how they qualify for 2.6.28. How serious do you
think this potential problem really is?
John
--
John W. Linville Linux should be at the core
linville@tuxdriver.com of your literate lifestyle.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GIT PATCH] rfkill fixes for 2.6.28-rc3
2008-11-12 21:15 ` John W. Linville
@ 2008-11-14 0:16 ` Henrique de Moraes Holschuh
2008-11-20 2:00 ` John W. Linville
2008-11-20 16:27 ` [GIT PATCH] rfkill fixes for 2.6.28-rc3 drago01
1 sibling, 1 reply; 18+ messages in thread
From: Henrique de Moraes Holschuh @ 2008-11-14 0:16 UTC (permalink / raw)
To: John W. Linville
Cc: Ivo van Doorn, linux-wireless, Matthew Garrett, Alan Jenkins
On Wed, 12 Nov 2008, John W. Linville wrote:
> Still, I don't see how they qualify for 2.6.28. How serious do you
> think this potential problem really is?
It is not very grave, but it IS annoying... The only reason I don't
outright call it a regression, is that it was so broken before, it is just
broken in a different way without these patches.
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GIT PATCH] rfkill fixes for 2.6.28-rc3
2008-11-14 0:16 ` Henrique de Moraes Holschuh
@ 2008-11-20 2:00 ` John W. Linville
2008-11-20 10:51 ` Henrique de Moraes Holschuh
0 siblings, 1 reply; 18+ messages in thread
From: John W. Linville @ 2008-11-20 2:00 UTC (permalink / raw)
To: Henrique de Moraes Holschuh
Cc: Ivo van Doorn, linux-wireless, Matthew Garrett, Alan Jenkins
On Thu, Nov 13, 2008 at 10:16:24PM -0200, Henrique de Moraes Holschuh wrote:
> On Wed, 12 Nov 2008, John W. Linville wrote:
> > Still, I don't see how they qualify for 2.6.28. How serious do you
> > think this potential problem really is?
>
> It is not very grave, but it IS annoying... The only reason I don't
> outright call it a regression, is that it was so broken before, it is just
> broken in a different way without these patches.
I'm sorry, but I would prefer not to bother with this for 2.6.28.
Could you send versions intended for wireless-testing/2.6.29?
Thanks,
John
--
John W. Linville Linux should be at the core
linville@tuxdriver.com of your literate lifestyle.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GIT PATCH] rfkill fixes for 2.6.28-rc3
2008-11-20 2:00 ` John W. Linville
@ 2008-11-20 10:51 ` Henrique de Moraes Holschuh
2008-11-20 15:03 ` John W. Linville
0 siblings, 1 reply; 18+ messages in thread
From: Henrique de Moraes Holschuh @ 2008-11-20 10:51 UTC (permalink / raw)
To: John W. Linville
Cc: Ivo van Doorn, linux-wireless, Matthew Garrett, Alan Jenkins
On Wed, 19 Nov 2008, John W. Linville wrote:
> I'm sorry, but I would prefer not to bother with this for 2.6.28.
> Could you send versions intended for wireless-testing/2.6.29?
Sure, I can do that.
Which tree/branch you want them based on? wireless-testing master? Linus
mainline?
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GIT PATCH] rfkill fixes for 2.6.28-rc3
2008-11-20 10:51 ` Henrique de Moraes Holschuh
@ 2008-11-20 15:03 ` John W. Linville
2008-11-21 22:40 ` [PATCH 1/2] rfkill: preserve state across suspend Henrique de Moraes Holschuh
0 siblings, 1 reply; 18+ messages in thread
From: John W. Linville @ 2008-11-20 15:03 UTC (permalink / raw)
To: Henrique de Moraes Holschuh
Cc: Ivo van Doorn, linux-wireless, Matthew Garrett, Alan Jenkins
On Thu, Nov 20, 2008 at 08:51:07AM -0200, Henrique de Moraes Holschuh wrote:
> On Wed, 19 Nov 2008, John W. Linville wrote:
> > I'm sorry, but I would prefer not to bother with this for 2.6.28.
> > Could you send versions intended for wireless-testing/2.6.29?
>
> Sure, I can do that.
>
> Which tree/branch you want them based on? wireless-testing master? Linus
> mainline?
wireless-testing, please.
Thanks!
John
--
John W. Linville Linux should be at the core
linville@tuxdriver.com of your literate lifestyle.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GIT PATCH] rfkill fixes for 2.6.28-rc3
2008-11-12 21:15 ` John W. Linville
2008-11-14 0:16 ` Henrique de Moraes Holschuh
@ 2008-11-20 16:27 ` drago01
2008-11-20 16:31 ` Matthew Garrett
1 sibling, 1 reply; 18+ messages in thread
From: drago01 @ 2008-11-20 16:27 UTC (permalink / raw)
To: John W. Linville
Cc: Ivo van Doorn, Henrique de Moraes Holschuh, linux-wireless,
Matthew Garrett, Alan Jenkins
On Wed, Nov 12, 2008 at 10:15 PM, John W. Linville
<linville@tuxdriver.com> wrote:
> On Mon, Nov 03, 2008 at 06:20:40PM +0100, Ivo van Doorn wrote:
>> On Monday 03 November 2008, Henrique de Moraes Holschuh wrote:
>> > On Mon, 03 Nov 2008, Ivo van Doorn wrote:
>> > > On Monday 03 November 2008, Henrique de Moraes Holschuh wrote:
>> > > > This small patchset contains two fixes to issues in the suspend/resume
>> > > > handling of the rfkill class core.
>> > > >
>> > > > It needs to go to 2.6.28. These patches are based on 2.6.28-rc3.
>> > >
>> > > Are they real regressions or normal bugfixes?
>> >
>> > I am not sure if they regress anything, but I am pretty sure there is no
>> > bugzilla entry about them yet. I have added two more interested parties to
>> > the CC.
>> >
>> > So, if there is a strong feeling this would best be held until the next
>> > merge window, I can certainly respin the patches to on top of
>> > wireless-testing...
>>
>> Well I'll give my ACK to the 2 patches, but I'll let john decide if they
>> are "regression" enough for 2.6.28. ;)
>
> So I keep looking at these patches, and I'm not sure about them.
> It seems that they restore the rfkill state after resume to what it
> was before the suspend.
Wouldn't this break hw kill switches?
If I move the switch while the system is suspended it will ignore this
event but the hw will still not work.
> What I am unsure about is whether or not
> that is the appropriate thing to do. I suppose it makes sense under
> the rule of least surprise.
Well the above scenario would be just broken.
Unless the driver rereads the real hw state and updates the rfkill
state on resume.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GIT PATCH] rfkill fixes for 2.6.28-rc3
2008-11-20 16:27 ` [GIT PATCH] rfkill fixes for 2.6.28-rc3 drago01
@ 2008-11-20 16:31 ` Matthew Garrett
2008-11-20 16:54 ` drago01
0 siblings, 1 reply; 18+ messages in thread
From: Matthew Garrett @ 2008-11-20 16:31 UTC (permalink / raw)
To: drago01
Cc: John W. Linville, Ivo van Doorn, Henrique de Moraes Holschuh,
linux-wireless, Alan Jenkins
On Thu, Nov 20, 2008 at 05:27:38PM +0100, drago01 wrote:
> On Wed, Nov 12, 2008 at 10:15 PM, John W. Linville
> <linville@tuxdriver.com> wrote:
> > So I keep looking at these patches, and I'm not sure about them.
> > It seems that they restore the rfkill state after resume to what it
> > was before the suspend.
>
> Wouldn't this break hw kill switches?
> If I move the switch while the system is suspended it will ignore this
> event but the hw will still not work.
Whichever driver is responsible for listening to the hardware events
should handle that in its own resume method. The rfkill state
restoration code will only be relevant for restoring state set by
sofware.
> Well the above scenario would be just broken.
> Unless the driver rereads the real hw state and updates the rfkill
> state on resume.
Quite. How else are you going to know if a switch has moved in the
intervening time?
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GIT PATCH] rfkill fixes for 2.6.28-rc3
2008-11-20 16:31 ` Matthew Garrett
@ 2008-11-20 16:54 ` drago01
0 siblings, 0 replies; 18+ messages in thread
From: drago01 @ 2008-11-20 16:54 UTC (permalink / raw)
To: Matthew Garrett
Cc: John W. Linville, Ivo van Doorn, Henrique de Moraes Holschuh,
linux-wireless, Alan Jenkins
On Thu, Nov 20, 2008 at 5:31 PM, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> On Thu, Nov 20, 2008 at 05:27:38PM +0100, drago01 wrote:
>> On Wed, Nov 12, 2008 at 10:15 PM, John W. Linville
>> <linville@tuxdriver.com> wrote:
>> > So I keep looking at these patches, and I'm not sure about them.
>> > It seems that they restore the rfkill state after resume to what it
>> > was before the suspend.
>>
>> Wouldn't this break hw kill switches?
>> If I move the switch while the system is suspended it will ignore this
>> event but the hw will still not work.
>
> Whichever driver is responsible for listening to the hardware events
> should handle that in its own resume method. The rfkill state
> restoration code will only be relevant for restoring state set by
> sofware.
>
>> Well the above scenario would be just broken.
>> Unless the driver rereads the real hw state and updates the rfkill
>> state on resume.
>
> Quite. How else are you going to know if a switch has moved in the
> intervening time?
Yeah that makes sense, please ignore my comment ;)
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/2] rfkill: preserve state across suspend
2008-11-20 15:03 ` John W. Linville
@ 2008-11-21 22:40 ` Henrique de Moraes Holschuh
2008-11-21 22:40 ` [PATCH 2/2] rfkill: always call get_state() hook on resume Henrique de Moraes Holschuh
0 siblings, 1 reply; 18+ messages in thread
From: Henrique de Moraes Holschuh @ 2008-11-21 22:40 UTC (permalink / raw)
To: John Linville
Cc: linux-wireless, Ivo van Doorn, Henrique de Moraes Holschuh,
Matthew Garrett, Alan Jenkins
The rfkill class API requires that the driver connected to a class
call rfkill_force_state() on resume to update the real state of the
rfkill controller, OR that it provides a get_state() hook.
This means there is potentially a hidden call in the resume code flow
that changes rfkill->state (i.e. rfkill_force_state()), so the
previous state of the transmitter was being lost.
The simplest and most future-proof way to fix this is to explicitly
store the pre-sleep state on the rfkill structure, and restore from
that on resume.
Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Acked-by: Ivo van Doorn <IvDoorn@gmail.com>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
---
include/linux/rfkill.h | 1 +
net/rfkill/rfkill.c | 7 ++++++-
2 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/include/linux/rfkill.h b/include/linux/rfkill.h
index 4cd64b0..f376a93 100644
--- a/include/linux/rfkill.h
+++ b/include/linux/rfkill.h
@@ -108,6 +108,7 @@ struct rfkill {
struct device dev;
struct list_head node;
+ enum rfkill_state state_for_resume;
};
#define to_rfkill(d) container_of(d, struct rfkill, dev)
diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
index 69f3a3b..4113454 100644
--- a/net/rfkill/rfkill.c
+++ b/net/rfkill/rfkill.c
@@ -565,10 +565,15 @@ static void rfkill_release(struct device *dev)
#ifdef CONFIG_PM
static int rfkill_suspend(struct device *dev, pm_message_t state)
{
+ struct rfkill *rfkill = to_rfkill(dev);
+
/* mark class device as suspended */
if (dev->power.power_state.event != state.event)
dev->power.power_state = state;
+ /* store state for the resume handler */
+ rfkill->state_for_resume = rfkill->state;
+
return 0;
}
@@ -590,7 +595,7 @@ static int rfkill_resume(struct device *dev)
rfkill_toggle_radio(rfkill,
rfkill_epo_lock_active ?
RFKILL_STATE_SOFT_BLOCKED :
- rfkill->state,
+ rfkill->state_for_resume,
1);
mutex_unlock(&rfkill->mutex);
--
1.5.6.5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/2] rfkill: always call get_state() hook on resume
2008-11-21 22:40 ` [PATCH 1/2] rfkill: preserve state across suspend Henrique de Moraes Holschuh
@ 2008-11-21 22:40 ` Henrique de Moraes Holschuh
0 siblings, 0 replies; 18+ messages in thread
From: Henrique de Moraes Holschuh @ 2008-11-21 22:40 UTC (permalink / raw)
To: John Linville
Cc: linux-wireless, Ivo van Doorn, Henrique de Moraes Holschuh,
Matthew Garrett, Alan Jenkins
We "optimize" away the get_state() hook call on rfkill_toggle_radio
when doing a forced state change. This means the resume path is not
calling get_state() as it should.
Call it manually on the resume handler, as we don't want to mess with
the EPO path by removing the optimization. This has the added benefit
of making it explicit that rfkill->state could have been modified
before we hit the rfkill_toggle_radio() call in the class resume
handler.
Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Acked-by: Ivo van Doorn <IvDoorn@gmail.com>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
---
net/rfkill/rfkill.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
index 4113454..ff3af0e 100644
--- a/net/rfkill/rfkill.c
+++ b/net/rfkill/rfkill.c
@@ -580,6 +580,7 @@ static int rfkill_suspend(struct device *dev, pm_message_t state)
static int rfkill_resume(struct device *dev)
{
struct rfkill *rfkill = to_rfkill(dev);
+ enum rfkill_state newstate;
if (dev->power.power_state.event != PM_EVENT_ON) {
mutex_lock(&rfkill->mutex);
@@ -587,6 +588,15 @@ static int rfkill_resume(struct device *dev)
dev->power.power_state.event = PM_EVENT_ON;
/*
+ * rfkill->state could have been modified before we got
+ * called, and won't be updated by rfkill_toggle_radio()
+ * in force mode. Sync it FIRST.
+ */
+ if (rfkill->get_state &&
+ !rfkill->get_state(rfkill->data, &newstate))
+ rfkill->state = newstate;
+
+ /*
* If we are under EPO, kick transmitter offline,
* otherwise restore to pre-suspend state.
*
--
1.5.6.5
^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2008-11-21 22:40 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-11-03 16:42 [GIT PATCH] rfkill fixes for 2.6.28-rc3 Henrique de Moraes Holschuh
2008-11-03 16:42 ` [PATCH 1/2] rfkill: preserve state across suspend Henrique de Moraes Holschuh
2008-11-03 17:20 ` Ivo van Doorn
2008-11-03 16:42 ` [PATCH 2/2] rfkill: always call get_state() hook on resume Henrique de Moraes Holschuh
2008-11-03 17:20 ` Ivo van Doorn
2008-11-03 16:47 ` [GIT PATCH] rfkill fixes for 2.6.28-rc3 Ivo van Doorn
2008-11-03 17:02 ` Henrique de Moraes Holschuh
2008-11-03 17:20 ` Ivo van Doorn
2008-11-12 21:15 ` John W. Linville
2008-11-14 0:16 ` Henrique de Moraes Holschuh
2008-11-20 2:00 ` John W. Linville
2008-11-20 10:51 ` Henrique de Moraes Holschuh
2008-11-20 15:03 ` John W. Linville
2008-11-21 22:40 ` [PATCH 1/2] rfkill: preserve state across suspend Henrique de Moraes Holschuh
2008-11-21 22:40 ` [PATCH 2/2] rfkill: always call get_state() hook on resume Henrique de Moraes Holschuh
2008-11-20 16:27 ` [GIT PATCH] rfkill fixes for 2.6.28-rc3 drago01
2008-11-20 16:31 ` Matthew Garrett
2008-11-20 16:54 ` drago01
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).