[alsa-devel] [PATCH] ASoC: nau8825: jack connection decision with different insertion logic
The original design only covers the jack insertion logic is active low. Add more condition to cover no matter the logic is active low and high.
Signed-off-by: John Hsu KCHSU0@nuvoton.com --- sound/soc/codecs/nau8825.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/nau8825.c b/sound/soc/codecs/nau8825.c index 3f30e6e..a2f0d03 100644 --- a/sound/soc/codecs/nau8825.c +++ b/sound/soc/codecs/nau8825.c @@ -1345,10 +1345,18 @@ EXPORT_SYMBOL_GPL(nau8825_enable_jack_detect);
static bool nau8825_is_jack_inserted(struct regmap *regmap) { - int status; + int status, jkdet, res;
regmap_read(regmap, NAU8825_REG_I2C_DEVICE_ID, &status); - return !(status & NAU8825_GPIO2JD1); + regmap_read(regmap, NAU8825_REG_JACK_DET_CTRL, &jkdet); + + /* return jack connection status according to jack insertion logic + * active high or active low. + */ + res = !(status & NAU8825_GPIO2JD1) * !(jkdet & NAU8825_JACK_POLARITY) + + (status & NAU8825_GPIO2JD1) * (jkdet & NAU8825_JACK_POLARITY); + + return res ? true : false; }
static void nau8825_restart_jack_detection(struct regmap *regmap)
Hi
On Tue, Jun 28, 2016 at 8:20 PM, John Hsu KCHSU0@nuvoton.com wrote:
The original design only covers the jack insertion logic is active low. Add more condition to cover no matter the logic is active low and high.
Signed-off-by: John Hsu KCHSU0@nuvoton.com
sound/soc/codecs/nau8825.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/nau8825.c b/sound/soc/codecs/nau8825.c index 3f30e6e..a2f0d03 100644 --- a/sound/soc/codecs/nau8825.c +++ b/sound/soc/codecs/nau8825.c @@ -1345,10 +1345,18 @@ EXPORT_SYMBOL_GPL(nau8825_enable_jack_detect);
static bool nau8825_is_jack_inserted(struct regmap *regmap) {
int status;
int status, jkdet, res; regmap_read(regmap, NAU8825_REG_I2C_DEVICE_ID, &status);
return !(status & NAU8825_GPIO2JD1);
regmap_read(regmap, NAU8825_REG_JACK_DET_CTRL, &jkdet);/* return jack connection status according to jack insertion logic* active high or active low.*/res = !(status & NAU8825_GPIO2JD1) * !(jkdet & NAU8825_JACK_POLARITY) +(status & NAU8825_GPIO2JD1) * (jkdet & NAU8825_JACK_POLARITY);
It makes more sense to use a more boolean-like expression. Something like
return !!(status & NAU8825_GPIO2JD1) == !!(jkdet & NAU8825_JACK_POLARITY); (hope I translated expression above correctly)
or even better to introduce readable bool flags: bool active_high = !!(jkdet & NAU8825_JACK_POLARITY); bool is_high = !!(status & NAU8825_GPIO2JD1); return active_high == is_high;
return res ? true : false;}
static void nau8825_restart_jack_detection(struct regmap *regmap)
2.6.4
Hi On 6/29/2016 11:45 AM, Anatol Pomozov wrote:
Hi
On Tue, Jun 28, 2016 at 8:20 PM, John Hsu KCHSU0@nuvoton.com wrote:
The original design only covers the jack insertion logic is active low. Add more condition to cover no matter the logic is active low and high.
Signed-off-by: John Hsu KCHSU0@nuvoton.com
sound/soc/codecs/nau8825.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/nau8825.c b/sound/soc/codecs/nau8825.c index 3f30e6e..a2f0d03 100644 --- a/sound/soc/codecs/nau8825.c +++ b/sound/soc/codecs/nau8825.c @@ -1345,10 +1345,18 @@ EXPORT_SYMBOL_GPL(nau8825_enable_jack_detect);
static bool nau8825_is_jack_inserted(struct regmap *regmap) {
int status;
int status, jkdet, res; regmap_read(regmap, NAU8825_REG_I2C_DEVICE_ID, &status);
return !(status & NAU8825_GPIO2JD1);
regmap_read(regmap, NAU8825_REG_JACK_DET_CTRL, &jkdet);/* return jack connection status according to jack insertion logic* active high or active low.*/res = !(status & NAU8825_GPIO2JD1) * !(jkdet & NAU8825_JACK_POLARITY) +(status & NAU8825_GPIO2JD1) * (jkdet & NAU8825_JACK_POLARITY);It makes more sense to use a more boolean-like expression. Something like
return !!(status & NAU8825_GPIO2JD1) == !!(jkdet & NAU8825_JACK_POLARITY);(hope I translated expression above correctly)
or even better to introduce readable bool flags: bool active_high = !!(jkdet & NAU8825_JACK_POLARITY); bool is_high = !!(status & NAU8825_GPIO2JD1); return active_high == is_high;
Yes, it'll be more readable. I have a question. Why to add !! in front of bit wise operation? What does it mean?
On Thu, Jun 30, 2016 at 03:51:51PM +0800, John Hsu wrote:
Yes, it'll be more readable. I have a question. Why to add !! in front of bit wise operation? What does it mean?
It's redundant in almost all cases. It translates an integer value into a 0/1 value (so if you've got a value of 2 it'll end up as 1) by doing a double not. This very rarely matters unless you're storing the value in an integer and comparing it to 1 to check for truth, otherwise C will translate any non-zero value into true in any logic context.
On 7/1/2016 11:57 PM, Mark Brown wrote:
On Thu, Jun 30, 2016 at 03:51:51PM +0800, John Hsu wrote:
Yes, it'll be more readable. I have a question. Why to add !! in front of bit wise operation? What does it mean?
It's redundant in almost all cases. It translates an integer value into a 0/1 value (so if you've got a value of 2 it'll end up as 1) by doing a double not. This very rarely matters unless you're storing the value in an integer and comparing it to 1 to check for truth, otherwise C will translate any non-zero value into true in any logic context.
Thank you for the explanation. I get the purpose now and will change the expression of function for clear intent.
Hi
On Fri, Jul 1, 2016 at 8:57 AM, Mark Brown broonie@kernel.org wrote:
On Thu, Jun 30, 2016 at 03:51:51PM +0800, John Hsu wrote:
Yes, it'll be more readable. I have a question. Why to add !! in front of bit wise operation? What does it mean?
It's redundant in almost all cases. It translates an integer value into a 0/1 value (so if you've got a value of 2 it'll end up as 1) by doing a double not. This very rarely matters unless you're storing the value in an integer and comparing it to 1 to check for truth, otherwise C will translate any non-zero value into true in any logic context.
Thanks Mark for the comment. I added "!!" by following my muscle memory. That how I used to convert non-bools to bool.
But your comment forced me to research this question. C99 and C11 standards clarify this issue http://port70.net/~nsz/c/c11/n1570.html#6.3.1.2p1
..<QUOTE>.. When any scalar value is converted to _Bool, the result is 0 if the value compares equal to 0; otherwise, the result is 1
It is safe to drop "!!" idiom completely - compiler will take care of converting the result of bitmask to a proper bool value. My example above can be overwritten as bool active_high = jkdet & NAU8825_JACK_POLARITY; bool is_high = status & NAU8825_GPIO2JD1; return active_high == is_high;
participants (3)
-
Anatol Pomozov -
John Hsu -
Mark Brown