Skip to contentSkip to conversation

bluecard_cs: Always enable LEDs (fix for Anycom CF-300)

PATCH In review Ondrej Zary 2 participants 4 messages Aug 5, 2017 – Aug 7, 2017 Bluetooth drivers

lore
Share & tools
lore

Ondrej Zary

author
#0 lore raw

PATCH 1/2 bluecard_cs: Always enable LEDs (fix for Anycom CF-300)

Anycom CF-300 (HP C8249A) has both power and activity LEDs. However the id read in bluecard_open() is 0x73 so the driver does not enable the LEDs.
Remove the CARD_HAS_PCCARD_ID check to enable LEDs.

Signed-off-by Ondrej Zary <[email protected]>
---
 drivers/bluetooth/bluecard_cs.c | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)
drivers/bluetooth/bluecard_cs.c +6 -18
index d4b0b655dde6..5cd1e164fde0 100644
@@ -160,9 +160,6 @@ static void bluecard_activity_led_timeout(u_long arg)
 	struct bluecard_info *info = (struct bluecard_info *)arg;
 	unsigned int iobase = info->p_dev->resource[0]->start;
 
-	if (!test_bit(CARD_HAS_PCCARD_ID, &(info->hw_state)))
-		return;
-
 	if (test_bit(CARD_HAS_ACTIVITY_LED, &(info->hw_state))) {
 		/* Disable activity LED */
 		outb(0x08 | 0x20, iobase + 0x30);

@@ -177,9 +174,6 @@ static void bluecard_enable_activity_led(struct bluecard_info *info)
 {
 	unsigned int iobase = info->p_dev->resource[0]->start;
 
-	if (!test_bit(CARD_HAS_PCCARD_ID, &(info->hw_state)))
-		return;
-
 	if (test_bit(CARD_HAS_ACTIVITY_LED, &(info->hw_state))) {
 		/* Enable activity LED */
 		outb(0x10 | 0x40, iobase + 0x30);

@@ -625,16 +619,13 @@ static int bluecard_hci_flush(struct hci_dev *hdev)
 static int bluecard_hci_open(struct hci_dev *hdev)
 {
 	struct bluecard_info *info = hci_get_drvdata(hdev);
+	unsigned int iobase = info->p_dev->resource[0]->start;
 
 	if (test_bit(CARD_HAS_PCCARD_ID, &(info->hw_state)))
 		bluecard_hci_set_baud_rate(hdev, DEFAULT_BAUD_RATE);
 
-	if (test_bit(CARD_HAS_PCCARD_ID, &(info->hw_state))) {
-		unsigned int iobase = info->p_dev->resource[0]->start;
-
-		/* Enable LED */
-		outb(0x08 | 0x20, iobase + 0x30);
-	}
+	/* Enable LED */
+	outb(0x08 | 0x20, iobase + 0x30);
 
 	return 0;
 }

@@ -643,15 +634,12 @@ static int bluecard_hci_open(struct hci_dev *hdev)
 static int bluecard_hci_close(struct hci_dev *hdev)
 {
 	struct bluecard_info *info = hci_get_drvdata(hdev);
+	unsigned int iobase = info->p_dev->resource[0]->start;
 
 	bluecard_hci_flush(hdev);
 
-	if (test_bit(CARD_HAS_PCCARD_ID, &(info->hw_state))) {
-		unsigned int iobase = info->p_dev->resource[0]->start;
-
-		/* Disable LED */
-		outb(0x00, iobase + 0x30);
-	}
+	/* Disable LED */
+	outb(0x00, iobase + 0x30);
 
 	return 0;
 }
Signature · 1 line
Ondrej Zary
author
#1 lore raw

PATCH 2/2 bluecard_cs: fix LED behavior

Keep power LED on during activity.

LED timer races with power LED disabling in hci_close(), resulting in power LED left on after closing.
Stop LED timer before disabling power LED.

BTW. On cards without an activity LED, the behavior is a bit weird:
The LED is on after hci_open() but only until the first data transfer. Then it's off in idle and on during activity.
It could be improved by keeping the LED on in idle and flashing during activity.

Signed-off-by Ondrej Zary <[email protected]>
---
 drivers/bluetooth/bluecard_cs.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)
drivers/bluetooth/bluecard_cs.c +8 -5
index 5cd1e164fde0..61ac48e1aa55 100644
@@ -161,7 +161,7 @@ static void bluecard_activity_led_timeout(u_long arg)
 	unsigned int iobase = info->p_dev->resource[0]->start;
 
 	if (test_bit(CARD_HAS_ACTIVITY_LED, &(info->hw_state))) {
-		/* Disable activity LED */
+		/* Disable activity LED, keep power LED enabled */
 		outb(0x08 | 0x20, iobase + 0x30);
 	} else {
 		/* Disable power LED */

@@ -175,8 +175,8 @@ static void bluecard_enable_activity_led(struct bluecard_info *info)
 	unsigned int iobase = info->p_dev->resource[0]->start;
 
 	if (test_bit(CARD_HAS_ACTIVITY_LED, &(info->hw_state))) {
-		/* Enable activity LED */
-		outb(0x10 | 0x40, iobase + 0x30);
+		/* Enable activity LED, keep power LED enabled */
+		outb(0x18 | 0x60, iobase + 0x30);
 
 		/* Stop the LED after HZ/4 */
 		mod_timer(&(info->timer), jiffies + HZ / 4);

@@ -624,7 +624,7 @@ static int bluecard_hci_open(struct hci_dev *hdev)
 	if (test_bit(CARD_HAS_PCCARD_ID, &(info->hw_state)))
 		bluecard_hci_set_baud_rate(hdev, DEFAULT_BAUD_RATE);
 
-	/* Enable LED */
+	/* Enable power LED */
 	outb(0x08 | 0x20, iobase + 0x30);
 
 	return 0;

@@ -638,7 +638,10 @@ static int bluecard_hci_close(struct hci_dev *hdev)
 
 	bluecard_hci_flush(hdev);
 
-	/* Disable LED */
+	/* Stop LED timer */
+	del_timer_sync(&(info->timer));
+
+	/* Disable power LED */
 	outb(0x00, iobase + 0x30);
 
 	return 0;
Signature · 1 line
Ondrej Zary
maintainer
#2 lore raw

Re: [PATCH 2/2] bluecard_cs: fix LED behavior

Hi Ondrej,

Quoted 12 lines

Keep power LED on during activity.

LED timer races with power LED disabling in hci_close(), resulting in power LED left on after closing.
Stop LED timer before disabling power LED.

BTW. On cards without an activity LED, the behavior is a bit weird:
The LED is on after hci_open() but only until the first data transfer. Then it's off in idle and on during activity.
It could be improved by keeping the LED on in idle and flashing during activity.

Quoted 4 lines
Signed-off-by Ondrej Zary <[email protected]>

---
drivers/bluetooth/bluecard_cs.c | 13 ++++++++-----

1 file changed, 8 insertions(+), 5 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel

maintainer
#3 lore raw
In reply to Ondrej Zary

Hi Ondrej,

Quoted 9 lines

Anycom CF-300 (HP C8249A) has both power and activity LEDs. However the id read in bluecard_open() is 0x73 so the driver does not enable the LEDs.
Remove the CARD_HAS_PCCARD_ID check to enable LEDs.

Signed-off-by Ondrej Zary <[email protected]>

---
drivers/bluetooth/bluecard_cs.c | 24 ++++++------------------

1 file changed, 6 insertions(+), 18 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel