Re: [PATCH] rfkill: Regulator consumer driver for rfkill
On Mit, 2011-04-06 at 20:46 +0200, Johannes Berg wrote: On Wed, 2011-04-06 at 20:12 +0200, Paul Bolle wrote: [...] But doesn't depends on RFKILL || !RFKILL always evaluate to true when running make *config? (Even if RFKILL is an unknown symbol when that expression is parsed!) No, it will not, you're forgetting that these things are tristate. Boolean operators for tristate logic isn't intuitive at all IMHO. Bernd -- Bernd Petrovitsch Email : be...@petrovitsch.priv.at LUGA : http://www.luga.at
Re: [PATCH] rfkill: Regulator consumer driver for rfkill
On Don, 2011-04-07 at 12:09 +0200, Johannes Berg wrote: On Thu, 2011-04-07 at 12:01 +0200, Bernd Petrovitsch wrote: On Mit, 2011-04-06 at 20:46 +0200, Johannes Berg wrote: On Wed, 2011-04-06 at 20:12 +0200, Paul Bolle wrote: [...] But doesn't depends on RFKILL || !RFKILL always evaluate to true when running make *config? (Even if RFKILL is an unknown symbol when that expression is parsed!) No, it will not, you're forgetting that these things are tristate. Boolean operators for tristate logic isn't intuitive at all IMHO. *shrug*. You're free to propose patches to the kconfig system to make it more intuitive. :-) FullACK;-) But no intuitive tristate logic operators come to my mind (otherwise I would have mentioned them above). And there are more logic implications in depends on RFKILL. Hmm, I have to think more about it Bernd -- Bernd Petrovitsch Email : be...@petrovitsch.priv.at LUGA : http://www.luga.at
Re: [PATCH] rfkill: Regulator consumer driver for rfkill
On Wed, 2011-04-06 at 11:21 +0200, Antonio Ospite wrote:
+ if (regulator_is_enabled(vcc)) {
+ dev_dbg(pdev-dev, Regulator already enabled\n);
+ rfkill_data-reg_enabled = 1;
+ }
+ rfkill_init_sw_state(rf_kill, !rfkill_data-reg_enabled);
+
+ ret = rfkill_register(rf_kill);
We recently had a thread about how rfkill_init_sw_state() isn't quite
working the right way. Also, it is indented to be used for devices that
keep their state over resume. I think you should remove it here and rely
on rfkill to sync you after registration.
Cf. the long thread here:
http://thread.gmane.org/gmane.linux.acpi.devel/49577
Other than that, no comments from me from an rfkill POV.
johannes
Re: [PATCH] rfkill: Regulator consumer driver for rfkill
On Wed, 06 Apr 2011 11:29:38 +0200
Johannes Berg johan...@sipsolutions.net wrote:
On Wed, 2011-04-06 at 11:21 +0200, Antonio Ospite wrote:
+ if (regulator_is_enabled(vcc)) {
+ dev_dbg(pdev-dev, Regulator already enabled\n);
+ rfkill_data-reg_enabled = 1;
+ }
+ rfkill_init_sw_state(rf_kill, !rfkill_data-reg_enabled);
+
+ ret = rfkill_register(rf_kill);
We recently had a thread about how rfkill_init_sw_state() isn't quite
working the right way. Also, it is indented to be used for devices that
keep their state over resume. I think you should remove it here and rely
on rfkill to sync you after registration.
Cf. the long thread here:
http://thread.gmane.org/gmane.linux.acpi.devel/49577
Ok, but I still need to replace that call with a rfkill_set_sw_state()
to expose the initial status of the regulator to the rfkill system,
right?
Other than that, no comments from me from an rfkill POV.
johannes
Thanks I am waiting a couple of days before sending a v2.
Regards,
Antonio
--
Antonio Ospite
http://ao2.it
PGP public key ID: 0x4553B001
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
pgpo9X2HWCTIB.pgp
Description: PGP signature
Re: [PATCH] rfkill: Regulator consumer driver for rfkill
On Wed, 2011-04-06 at 16:06 +0200, Antonio Ospite wrote:
+ if (regulator_is_enabled(vcc)) {
+ dev_dbg(pdev-dev, Regulator already enabled\n);
+ rfkill_data-reg_enabled = 1;
+ }
+ rfkill_init_sw_state(rf_kill, !rfkill_data-reg_enabled);
+
+ ret = rfkill_register(rf_kill);
We recently had a thread about how rfkill_init_sw_state() isn't quite
working the right way. Also, it is indented to be used for devices that
keep their state over resume. I think you should remove it here and rely
on rfkill to sync you after registration.
Cf. the long thread here:
http://thread.gmane.org/gmane.linux.acpi.devel/49577
Ok, but I still need to replace that call with a rfkill_set_sw_state()
to expose the initial status of the regulator to the rfkill system,
right?
Well, you could, but if you don't do that then the rfkill subsystem will
simply call set_block() shortly after registration to put it into the
state that it thinks it should be in, which is usually more useful.
johannes
Re: [PATCH] rfkill: Regulator consumer driver for rfkill
On Wed, 2011-04-06 at 23:11 +0900, Mark Brown wrote: On Wed, Apr 06, 2011 at 11:21:19AM +0200, Antonio Ospite wrote: + tristate Generic rfkill regulator driver + depends on RFKILL || !RFKILL That looks *odd*. That's normal for rfkill -- if RFKILL==n then this can be anything since the rfkill API goes all no-op inlines, but if RFKILL==m then this can't be ==y. depends on !RFKILL covers the former, depends on RFKILL the latter. johannes
Re: [PATCH] rfkill: Regulator consumer driver for rfkill
On Wed, 06 Apr 2011 16:09:28 +0200
Johannes Berg johan...@sipsolutions.net wrote:
On Wed, 2011-04-06 at 16:06 +0200, Antonio Ospite wrote:
+ if (regulator_is_enabled(vcc)) {
+ dev_dbg(pdev-dev, Regulator already enabled\n);
+ rfkill_data-reg_enabled = 1;
+ }
+ rfkill_init_sw_state(rf_kill, !rfkill_data-reg_enabled);
+
+ ret = rfkill_register(rf_kill);
We recently had a thread about how rfkill_init_sw_state() isn't quite
working the right way. Also, it is indented to be used for devices that
keep their state over resume. I think you should remove it here and rely
on rfkill to sync you after registration.
Cf. the long thread here:
http://thread.gmane.org/gmane.linux.acpi.devel/49577
Ok, but I still need to replace that call with a rfkill_set_sw_state()
to expose the initial status of the regulator to the rfkill system,
right?
Well, you could, but if you don't do that then the rfkill subsystem will
simply call set_block() shortly after registration to put it into the
state that it thinks it should be in, which is usually more useful.
I see, let's just drop rfkill_init_sw_state() then.
Regards,
Antonio
--
Antonio Ospite
http://ao2.it
PGP public key ID: 0x4553B001
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
pgpkYCWKv6SUL.pgp
Description: PGP signature
Re: [PATCH] rfkill: Regulator consumer driver for rfkill
On Wed, 6 Apr 2011 23:11:33 +0900 Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Wed, Apr 06, 2011 at 11:21:19AM +0200, Antonio Ospite wrote: + tristate Generic rfkill regulator driver + depends on RFKILL || !RFKILL That looks *odd*. Taken from Documentation/rfkill.txt section 3. Kernel API. I guess I can drop it if we want to be stricter and just require RFKILL to be enabled. Johannes? Otherwise this looks fine from a regulator API point of view. You use an exclusive get() so you could get away without remembering the enable state as nothing else could hold the device open but there's no harm in doing so and it's defensive against silly constraints that force the regulator on. Thanks Mark. Regards, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? pgpf4tvLZe0cn.pgp Description: PGP signature
Re: [PATCH] rfkill: Regulator consumer driver for rfkill
On Wed, 2011-04-06 at 16:29 +0200, Antonio Ospite wrote: On Wed, 6 Apr 2011 23:11:33 +0900 Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Wed, Apr 06, 2011 at 11:21:19AM +0200, Antonio Ospite wrote: + tristate Generic rfkill regulator driver + depends on RFKILL || !RFKILL That looks *odd*. Taken from Documentation/rfkill.txt section 3. Kernel API. I guess I can drop it if we want to be stricter and just require RFKILL to be enabled. Johannes? I guess it depends on what you're looking to do. Since all you implement is set_block() you might very well not need to be able to have this if nothing is ever going to invoke set_block(), in which case you can do depends on RFKILL. The reason for this usually is that a driver, like a wireless driver, should work even if there's no rfkill API available, but it shouldn't need to put #ifdefs into the code itself. johannes
Re: [PATCH] rfkill: Regulator consumer driver for rfkill
Hi Antonio,
於 三，2011-04-06 於 16:24 +0200，Antonio Ospite 提到：
On Wed, 06 Apr 2011 16:09:28 +0200
Johannes Berg johan...@sipsolutions.net wrote:
On Wed, 2011-04-06 at 16:06 +0200, Antonio Ospite wrote:
+ if (regulator_is_enabled(vcc)) {
+ dev_dbg(pdev-dev, Regulator already enabled\n);
+ rfkill_data-reg_enabled = 1;
+ }
+ rfkill_init_sw_state(rf_kill, !rfkill_data-reg_enabled);
+
+ ret = rfkill_register(rf_kill);
We recently had a thread about how rfkill_init_sw_state() isn't quite
working the right way. Also, it is indented to be used for devices that
keep their state over resume. I think you should remove it here and rely
on rfkill to sync you after registration.
Cf. the long thread here:
http://thread.gmane.org/gmane.linux.acpi.devel/49577
Ok, but I still need to replace that call with a rfkill_set_sw_state()
to expose the initial status of the regulator to the rfkill system,
right?
Well, you could, but if you don't do that then the rfkill subsystem will
simply call set_block() shortly after registration to put it into the
state that it thinks it should be in, which is usually more useful.
I see, let's just drop rfkill_init_sw_state() then.
Regards,
Antonio
Like Johannes's comment, the rfkill_init_sw_state is a bit tricky
especially when RFKILL_INPUT enabled. The rfkill_init_sw_state will
replicate the state to device global state, then rfkill will replicate
it to other killswitch.
If you want to use rfkill_init_sw_state to set rfkill initial state when
driver probed, then I suggest you need test it when:
- RFKILL_INPUT enabled
and
- when device initial state is disabled(BLOCKED)
If you want to maintain the rfkill initial state by your self, you can
reference this patch:
http://git.kernel.org/?p=linux/kernel/git/mjg59/platform-drivers-x86.git;a=commitdiff;h=8215af019040ce9182728afee9642d8fdeb17f59
The patch set intial state by rfkill_set_sw_state after rfkill register,
and don't touch the BIOS (firmware?) state in set_block until driver
probe finished.
Thank's a lot!
Joey Lee
Re: [PATCH] rfkill: Regulator consumer driver for rfkill
On Wed, 2011-04-06 at 20:12 +0200, Paul Bolle wrote: On Wed, 2011-04-06 at 16:21 +0200, Johannes Berg wrote: On Wed, 2011-04-06 at 23:11 +0900, Mark Brown wrote: On Wed, Apr 06, 2011 at 11:21:19AM +0200, Antonio Ospite wrote: + tristate Generic rfkill regulator driver + depends on RFKILL || !RFKILL That looks *odd*. That's normal for rfkill -- if RFKILL==n then this can be anything since the rfkill API goes all no-op inlines, but if RFKILL==m then this can't be ==y. depends on !RFKILL covers the former, depends on RFKILL the latter. But doesn't depends on RFKILL || !RFKILL always evaluate to true when running make *config? (Even if RFKILL is an unknown symbol when that expression is parsed!) No, it will not, you're forgetting that these things are tristate. johannes
Re: [PATCH] rfkill: Regulator consumer driver for rfkill
On Wed, Apr 06, 2011 at 08:12:22PM +0200, Paul Bolle wrote: On Wed, 2011-04-06 at 16:21 +0200, Johannes Berg wrote: On Wed, 2011-04-06 at 23:11 +0900, Mark Brown wrote: On Wed, Apr 06, 2011 at 11:21:19AM +0200, Antonio Ospite wrote: + tristate Generic rfkill regulator driver + depends on RFKILL || !RFKILL That looks *odd*. That's normal for rfkill -- if RFKILL==n then this can be anything since the rfkill API goes all no-op inlines, but if RFKILL==m then this can't be ==y. depends on !RFKILL covers the former, depends on RFKILL the latter. But doesn't depends on RFKILL || !RFKILL always evaluate to true when running make *config? (Even if RFKILL is an unknown symbol when that expression is parsed!) I'd say that dependencies such as this one might as well be dropped from their Kconfig file. The syntax may seem strange, but basically it just says don't let me by y if RFKILL is m. -- John W. LinvilleSomeday the world will need a hero, and you linvi...@tuxdriver.com might be all we have. Be ready.
Re: [PATCH] rfkill: Regulator consumer driver for rfkill
On Wed, 2011-04-06 at 14:38 -0400, John W. Linville wrote: The syntax may seem strange, It does! but basically it just says don't let me by y if RFKILL is m ... but, besides that, I can be any value. So in effect it's shorthand for depends on RFKILL=y || RFKILL=m m || RFKILL=n (which actually looks equally strange). Is that correct? Anyhow, perhaps this should be added to the Kconfig hints in Documentation/kbuild/kconfig-language.txt. Paul Bolle
Re: [PATCH] rfkill: Regulator consumer driver for rfkill
On Wed, 2011-04-06 at 22:10 +0200, Paul Bolle wrote: On Wed, 2011-04-06 at 14:38 -0400, John W. Linville wrote: The syntax may seem strange, It does! but basically it just says don't let me by y if RFKILL is m ... but, besides that, I can be any value. So in effect it's shorthand for depends on RFKILL=y || RFKILL=m m || RFKILL=n (which actually looks equally strange). Is that correct? I don't think it is, I believe that an expression like RFKILL=y has a bool type, and a tristate type value that depends on a bool type can still take the value m. johannes
Re: [PATCH] rfkill: Regulator consumer driver for rfkill
On Wed, 2011-04-06 at 22:15 +0200, Johannes Berg wrote: On Wed, 2011-04-06 at 22:10 +0200, Paul Bolle wrote: On Wed, 2011-04-06 at 14:38 -0400, John W. Linville wrote: The syntax may seem strange, It does! but basically it just says don't let me by y if RFKILL is m ... but, besides that, I can be any value. So in effect it's shorthand for depends on RFKILL=y || RFKILL=m m || RFKILL=n (which actually looks equally strange). Is that correct? I don't think it is, I believe that an expression like RFKILL=y has a bool type, and a tristate type value that depends on a bool type can still take the value m. Err, which is of course perfectly fine since if RFKILL is built in this can be any value, and in the RFKILL=m case you force it to m by making it depend on m directly. So yes, you're right. johannes
Re: [PATCH] rfkill: Regulator consumer driver for rfkill
On Wed, 2011-04-06 at 22:17 +0200, Johannes Berg wrote: On Wed, 2011-04-06 at 22:15 +0200, Johannes Berg wrote: On Wed, 2011-04-06 at 22:10 +0200, Paul Bolle wrote: On Wed, 2011-04-06 at 14:38 -0400, John W. Linville wrote: The syntax may seem strange, It does! but basically it just says don't let me by y if RFKILL is m ... but, besides that, I can be any value. So in effect it's shorthand for depends on RFKILL=y || RFKILL=m m || RFKILL=n (which actually looks equally strange). Is that correct? I don't think it is, I believe that an expression like RFKILL=y has a bool type, and a tristate type value that depends on a bool type can still take the value m. Err, which is of course perfectly fine since if RFKILL is built in this can be any value, and in the RFKILL=m case you force it to m by making it depend on m directly. So yes, you're right. Whoops ... sorry about the talking to self ... I still think the original is easier to understand. After all, just depends on RFKILL is trivial to understand even with tristates. And knowing that RFKILL will provide no-op inlines when it is unconfigured, you add depends on !RFKILL for that case. johannes
