| Message ID | 1469939084-15603-1-git-send-email-guodong.xu@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Ping. :) Would you give me some review opinions on this? In this revision, I chose to limit LED blinking to tx/rx traffic packets only. For other types of over-the-air packets, like scanning, it's not covered. Thank you. -Guodong On 31 July 2016 at 12:24, Guodong Xu <guodong.xu@linaro.org> wrote: > Two LED triggers are added into hci_dev: tx_led and rx_led. Upon ACL/SCO > packets available in tx or rx, the LEDs will blink. > > For each hci registration, two triggers are added into LED subsystem: > [hdev->name]-tx and [hdev-name]-rx. > Refer to Documentation/leds/leds-class.txt for usage. > > Verified on HiKey 96boards, which uses HiSilicon hi6220 SoC and TI > WL1835 WiFi/BT combo chip. > > Signed-off-by: Guodong Xu <guodong.xu@linaro.org> > --- > include/net/bluetooth/hci_core.h | 1 + > net/bluetooth/hci_core.c | 6 ++++++ > net/bluetooth/leds.c | 17 +++++++++++++++++ > net/bluetooth/leds.h | 2 ++ > 4 files changed, 26 insertions(+) > > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h > index dc71473..37b8dd9 100644 > --- a/include/net/bluetooth/hci_core.h > +++ b/include/net/bluetooth/hci_core.h > @@ -398,6 +398,7 @@ struct hci_dev { > bdaddr_t rpa; > > struct led_trigger *power_led; > + struct led_trigger *tx_led, *rx_led; > > int (*open)(struct hci_dev *hdev); > int (*close)(struct hci_dev *hdev); > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > index 45a9fc6..956dce1 100644 > --- a/net/bluetooth/hci_core.c > +++ b/net/bluetooth/hci_core.c > @@ -3833,6 +3833,7 @@ static void hci_sched_acl(struct hci_dev *hdev) > if (!hci_conn_num(hdev, AMP_LINK) && hdev->dev_type == HCI_AMP) > return; > > + hci_leds_blink_oneshot(hdev->tx_led); > switch (hdev->flow_ctl_mode) { > case HCI_FLOW_CTL_MODE_PACKET_BASED: > hci_sched_acl_pkt(hdev); > @@ -3856,6 +3857,7 @@ static void hci_sched_sco(struct hci_dev *hdev) > if (!hci_conn_num(hdev, SCO_LINK)) > return; > > + hci_leds_blink_oneshot(hdev->tx_led); > while (hdev->sco_cnt && (conn = hci_low_sent(hdev, SCO_LINK, "e))) { > while (quote-- && (skb = skb_dequeue(&conn->data_q))) { > BT_DBG("skb %p len %d", skb, skb->len); > @@ -3879,6 +3881,7 @@ static void hci_sched_esco(struct hci_dev *hdev) > if (!hci_conn_num(hdev, ESCO_LINK)) > return; > > + hci_leds_blink_oneshot(hdev->tx_led); > while (hdev->sco_cnt && (conn = hci_low_sent(hdev, ESCO_LINK, > "e))) { > while (quote-- && (skb = skb_dequeue(&conn->data_q))) { > @@ -3911,6 +3914,7 @@ static void hci_sched_le(struct hci_dev *hdev) > hci_link_tx_to(hdev, LE_LINK); > } > > + hci_leds_blink_oneshot(hdev->tx_led); > cnt = hdev->le_pkts ? hdev->le_cnt : hdev->acl_cnt; > tmp = cnt; > while (cnt && (chan = hci_chan_sent(hdev, LE_LINK, "e))) { > @@ -3990,6 +3994,7 @@ static void hci_acldata_packet(struct hci_dev *hdev, struct sk_buff *skb) > > if (conn) { > hci_conn_enter_active_mode(conn, BT_POWER_FORCE_ACTIVE_OFF); > + hci_leds_blink_oneshot(hdev->rx_led); > > /* Send to upper protocol */ > l2cap_recv_acldata(conn, skb, flags); > @@ -4022,6 +4027,7 @@ static void hci_scodata_packet(struct hci_dev *hdev, struct sk_buff *skb) > hci_dev_unlock(hdev); > > if (conn) { > + hci_leds_blink_oneshot(hdev->rx_led); > /* Send to upper protocol */ > sco_recv_scodata(conn, skb); > return; > diff --git a/net/bluetooth/leds.c b/net/bluetooth/leds.c > index 8319c84..ae10c5d 100644 > --- a/net/bluetooth/leds.c > +++ b/net/bluetooth/leds.c > @@ -19,6 +19,8 @@ struct hci_basic_led_trigger { > #define to_hci_basic_led_trigger(arg) container_of(arg, \ > struct hci_basic_led_trigger, led_trigger) > > +#define BLUETOOTH_BLINK_DELAY 50 /* ms */ > + > void hci_leds_update_powered(struct hci_dev *hdev, bool enabled) > { > if (hdev->power_led) > @@ -37,6 +39,17 @@ static void power_activate(struct led_classdev *led_cdev) > led_trigger_event(led_cdev->trigger, powered ? LED_FULL : LED_OFF); > } > > +void hci_leds_blink_oneshot(struct led_trigger *trig) > +{ > + unsigned long led_delay = BLUETOOTH_BLINK_DELAY; > + > + if (!trig) > + return; > + > + BT_DBG("led_trig %p", trig); > + led_trigger_blink_oneshot(trig, &led_delay, &led_delay, 0); > +} > + > static struct led_trigger *led_allocate_basic(struct hci_dev *hdev, > void (*activate)(struct led_classdev *led_cdev), > const char *name) > @@ -71,4 +84,8 @@ void hci_leds_init(struct hci_dev *hdev) > { > /* initialize power_led */ > hdev->power_led = led_allocate_basic(hdev, power_activate, "power"); > + /* initialize tx_led */ > + hdev->tx_led = led_allocate_basic(hdev, NULL, "tx"); > + /* initialize rx_led */ > + hdev->rx_led = led_allocate_basic(hdev, NULL, "rx"); > } > diff --git a/net/bluetooth/leds.h b/net/bluetooth/leds.h > index a9c4d6e..9b1cccd 100644 > --- a/net/bluetooth/leds.h > +++ b/net/bluetooth/leds.h > @@ -9,8 +9,10 @@ > #if IS_ENABLED(CONFIG_BT_LEDS) > void hci_leds_update_powered(struct hci_dev *hdev, bool enabled); > void hci_leds_init(struct hci_dev *hdev); > +void hci_leds_blink_oneshot(struct led_trigger *trig); > #else > static inline void hci_leds_update_powered(struct hci_dev *hdev, > bool enabled) {} > static inline void hci_leds_init(struct hci_dev *hdev) {} > +static inline void hci_leds_blink_oneshot(struct led_trigger *trig) {} > #endif > -- > 1.9.1 >
Hi Guodong, > Two LED triggers are added into hci_dev: tx_led and rx_led. Upon ACL/SCO > packets available in tx or rx, the LEDs will blink. > > For each hci registration, two triggers are added into LED subsystem: > [hdev->name]-tx and [hdev-name]-rx. > Refer to Documentation/leds/leds-class.txt for usage. > > Verified on HiKey 96boards, which uses HiSilicon hi6220 SoC and TI > WL1835 WiFi/BT combo chip. so I have no idea what to do with adding adding hci0-rx and hci0-tx triggers. Combined with hci0-power trigger these are already 3 triggers. And if you have 2 Bluetooth controllers in your system, then you have 6 triggers. If we then maybe add another trigger, then this number just goes up and up. As far as I can tell you can only assign a single trigger to a LED. So this means to even use these triggers, you need now 3 LEDs per Bluetooth controller. How is that useful for anybody in a real system? Maybe I am missing something here and somehow there is magic to combine triggers, but I have not found it yet. So please someone enlighten me on how this is suppose to be used with real devices. Recently I have added a simple bluetooth-power trigger that combines all Bluetooth controllers into a single trigger. If any of them is enabled, then you can control your LED. Which makes a lot more sense to me since you most likely have a single Bluetooth LED on your system. And you want it to show the correct state no matter what Bluetooth controller is in use. However I can see the case that someone might want to assign one specific Bluetooth controller to a LED status. So instead of adding many independent triggers to each controller, why not create one global bluetooth trigger and one individual bluetooth-hci0 trigger for each controller. And the combine power, tx, rx and whatever else we need to trigger the LED for? Regards Marcel
Hi, Marcel On 16 August 2016 at 14:03, Marcel Holtmann <marcel@holtmann.org> wrote: > Hi Guodong, > >> Two LED triggers are added into hci_dev: tx_led and rx_led. Upon ACL/SCO >> packets available in tx or rx, the LEDs will blink. >> >> For each hci registration, two triggers are added into LED subsystem: >> [hdev->name]-tx and [hdev-name]-rx. >> Refer to Documentation/leds/leds-class.txt for usage. >> >> Verified on HiKey 96boards, which uses HiSilicon hi6220 SoC and TI >> WL1835 WiFi/BT combo chip. > > so I have no idea what to do with adding adding hci0-rx and hci0-tx triggers. Combined with hci0-power trigger these are already 3 triggers. And if you have 2 Bluetooth controllers in your system, then you have 6 triggers. > True, 6 triggers. But, taking example for other subsytems, eg. cpu cores. On my board, I have "heartbeat cpu0 cpu1 cpu2 cpu3 cpu4 cpu5 cpu6 cpu7". It doesn't have to mean you need all of them connected to some LED(s). Actually, in most of the case, I only need heartbeat. > If we then maybe add another trigger, then this number just goes up and up. > > As far as I can tell you can only assign a single trigger to a LED. > That's true. And people got a choice of which feature he wants to visualize. > So this means to even use these triggers, you need now 3 LEDs per Bluetooth controller. How is that useful for anybody in a real system? Maybe I am missing something here and somehow there is magic to combine triggers, but I have not found it yet. So please someone enlighten me on how this is suppose to be used with real devices. > > Recently I have added a simple bluetooth-power trigger that combines all Bluetooth controllers into a single trigger. If any of them is enabled, then you can control your LED. Which makes a lot more sense to me since you most likely have a single Bluetooth LED on your system. And you want it to show the correct state no matter what Bluetooth controller is in use. However I can see the case that someone might want to assign one specific Bluetooth controller to a LED status. > > So instead of adding many independent triggers to each controller, why not create one global bluetooth trigger and one individual bluetooth-hci0 trigger for each controller. And the combine power, tx, rx and whatever else we need to trigger the LED for? > When I starting this work, I referred to WiFi system. See CONFIG_MAC80211_LEDS. WiFi system implements these types of triggers " phy0rx phy0tx phy0assoc phy0radio" for each 'controller'. Besides, there are also RFKILL which stands for WiFi/BT power status. RFKILL adds triggers for each module too. Eg. in the below example, I have one WiFi (phy0), one BT (hci0). Trigger rfkill1 equals to hci0-power. Ref: here are all LED triggers I found in my 96boards/HiKey: # cat trigger none kbd-scrollock kbd-numlock kbd-capslock kbd-kanalock kbd-shiftlock kbd-altgrlock kbd-ctrllock kbd-altlock kbd-shiftllock kbd-shiftrlock kbd-ctrlllock kbd-ctrlrlock mmc0 mmc1 heartbeat cpu0 cpu1 cpu2 cpu3 cpu4 cpu5 cpu6 cpu7 mmc2 rfkill0 phy0rx phy0tx phy0assoc phy0radio hci0-power hci0-tx [hci0-rx] rfkill1 -Guodong > Regards > > Marcel >
Hi Guodong, >>> Two LED triggers are added into hci_dev: tx_led and rx_led. Upon ACL/SCO >>> packets available in tx or rx, the LEDs will blink. >>> >>> For each hci registration, two triggers are added into LED subsystem: >>> [hdev->name]-tx and [hdev-name]-rx. >>> Refer to Documentation/leds/leds-class.txt for usage. >>> >>> Verified on HiKey 96boards, which uses HiSilicon hi6220 SoC and TI >>> WL1835 WiFi/BT combo chip. >> >> so I have no idea what to do with adding adding hci0-rx and hci0-tx triggers. Combined with hci0-power trigger these are already 3 triggers. And if you have 2 Bluetooth controllers in your system, then you have 6 triggers. >> > > True, 6 triggers. But, taking example for other subsytems, eg. cpu > cores. On my board, I have "heartbeat cpu0 cpu1 cpu2 cpu3 cpu4 cpu5 > cpu6 cpu7". It doesn't have to mean you need all of them connected to > some LED(s). Actually, in most of the case, I only need heartbeat. > > > >> If we then maybe add another trigger, then this number just goes up and up. >> >> As far as I can tell you can only assign a single trigger to a LED. >> > > That's true. And people got a choice of which feature he wants to visualize. and as a result we keep adding senseless triggers to the kernel and bloating it up for no reason. Especially since it feels like 99% of the LED triggers are not used at all. This makes no sense to me. >> So this means to even use these triggers, you need now 3 LEDs per Bluetooth controller. How is that useful for anybody in a real system? Maybe I am missing something here and somehow there is magic to combine triggers, but I have not found it yet. So please someone enlighten me on how this is suppose to be used with real devices. >> >> Recently I have added a simple bluetooth-power trigger that combines all Bluetooth controllers into a single trigger. If any of them is enabled, then you can control your LED. Which makes a lot more sense to me since you most likely have a single Bluetooth LED on your system. And you want it to show the correct state no matter what Bluetooth controller is in use. However I can see the case that someone might want to assign one specific Bluetooth controller to a LED status. >> >> So instead of adding many independent triggers to each controller, why not create one global bluetooth trigger and one individual bluetooth-hci0 trigger for each controller. And the combine power, tx, rx and whatever else we need to trigger the LED for? >> > > When I starting this work, I referred to WiFi system. See > CONFIG_MAC80211_LEDS. WiFi system implements these types of triggers " > phy0rx phy0tx phy0assoc phy0radio" for each 'controller'. And I actually wonder who ever used these triggers. You need 4 LEDs to visualize the WiFi status. Which systems has 4 LEDs to spare to visualize this. > Besides, there are also RFKILL which stands for WiFi/BT power status. > RFKILL adds triggers for each module too. Eg. in the below example, I > have one WiFi (phy0), one BT (hci0). Trigger rfkill1 equals to > hci0-power. > > Ref: here are all LED triggers I found in my 96boards/HiKey: > > # cat trigger > none kbd-scrollock kbd-numlock kbd-capslock kbd-kanalock kbd-shiftlock > kbd-altgrlock kbd-ctrllock kbd-altlock kbd-shiftllock kbd-shiftrlock > kbd-ctrlllock kbd-ctrlrlock mmc0 mmc1 heartbeat cpu0 cpu1 cpu2 cpu3 > cpu4 cpu5 cpu6 cpu7 mmc2 rfkill0 phy0rx phy0tx phy0assoc phy0radio > hci0-power hci0-tx [hci0-rx] rfkill1 And how many LEDs do you have in the your system? I think you are making my point here. So I think what we need to do is to not add to this madness and instead create one "bluetooth" LED trigger that combines power and TX/RX for all controllers. And then allow for individual "bluetooth-hci0" LED triggers so that you can bind a single Bluetooth controller to a single LED. For me, if I can not combine hci0-power, hci0-tx and hci0-rx into a single LED, it becomes utterly useless on pretty much every system that is out there. Regards Marcel
On 16 August 2016 at 20:33, Marcel Holtmann <marcel@holtmann.org> wrote: > > Hi Guodong, > > >>> Two LED triggers are added into hci_dev: tx_led and rx_led. Upon ACL/SCO > >>> packets available in tx or rx, the LEDs will blink. > >>> > >>> For each hci registration, two triggers are added into LED subsystem: > >>> [hdev->name]-tx and [hdev-name]-rx. > >>> Refer to Documentation/leds/leds-class.txt for usage. > >>> > >>> Verified on HiKey 96boards, which uses HiSilicon hi6220 SoC and TI > >>> WL1835 WiFi/BT combo chip. > >> > >> so I have no idea what to do with adding adding hci0-rx and hci0-tx triggers. Combined with hci0-power trigger these are already 3 triggers. And if you have 2 Bluetooth controllers in your system, then you have 6 triggers. > >> > > > > True, 6 triggers. But, taking example for other subsytems, eg. cpu > > cores. On my board, I have "heartbeat cpu0 cpu1 cpu2 cpu3 cpu4 cpu5 > > cpu6 cpu7". It doesn't have to mean you need all of them connected to > > some LED(s). Actually, in most of the case, I only need heartbeat. > > > > > > > >> If we then maybe add another trigger, then this number just goes up and up. > >> > >> As far as I can tell you can only assign a single trigger to a LED. > >> > > > > That's true. And people got a choice of which feature he wants to visualize. > > and as a result we keep adding senseless triggers to the kernel and bloating it up for no reason. Especially since it feels like 99% of the LED triggers are not used at all. This makes no sense to me. > > >> So this means to even use these triggers, you need now 3 LEDs per Bluetooth controller. How is that useful for anybody in a real system? Maybe I am missing something here and somehow there is magic to combine triggers, but I have not found it yet. So please someone enlighten me on how this is suppose to be used with real devices. > >> > >> Recently I have added a simple bluetooth-power trigger that combines all Bluetooth controllers into a single trigger. If any of them is enabled, then you can control your LED. Which makes a lot more sense to me since you most likely have a single Bluetooth LED on your system. And you want it to show the correct state no matter what Bluetooth controller is in use. However I can see the case that someone might want to assign one specific Bluetooth controller to a LED status. > >> > >> So instead of adding many independent triggers to each controller, why not create one global bluetooth trigger and one individual bluetooth-hci0 trigger for each controller. And the combine power, tx, rx and whatever else we need to trigger the LED for? > >> > > > > When I starting this work, I referred to WiFi system. See > > CONFIG_MAC80211_LEDS. WiFi system implements these types of triggers " > > phy0rx phy0tx phy0assoc phy0radio" for each 'controller'. > > And I actually wonder who ever used these triggers. You need 4 LEDs to visualize the WiFi status. Which systems has 4 LEDs to spare to visualize this. > > > Besides, there are also RFKILL which stands for WiFi/BT power status. > > RFKILL adds triggers for each module too. Eg. in the below example, I > > have one WiFi (phy0), one BT (hci0). Trigger rfkill1 equals to > > hci0-power. > > > > Ref: here are all LED triggers I found in my 96boards/HiKey: > > > > # cat trigger > > none kbd-scrollock kbd-numlock kbd-capslock kbd-kanalock kbd-shiftlock > > kbd-altgrlock kbd-ctrllock kbd-altlock kbd-shiftllock kbd-shiftrlock > > kbd-ctrlllock kbd-ctrlrlock mmc0 mmc1 heartbeat cpu0 cpu1 cpu2 cpu3 > > cpu4 cpu5 cpu6 cpu7 mmc2 rfkill0 phy0rx phy0tx phy0assoc phy0radio > > hci0-power hci0-tx [hci0-rx] rfkill1 > > And how many LEDs do you have in the your system? I think you are making my point here. > > So I think what we need to do is to not add to this madness and instead create one "bluetooth" LED trigger that combines power and TX/RX for all controllers. And then allow for individual "bluetooth-hci0" LED triggers so that you can bind a single Bluetooth controller to a single LED. > > For me, if I can not combine hci0-power, hci0-tx and hci0-rx into a single LED, By combining them into a single LED, do you mean such a use case? - when hci0 is powered on, this LED starts on. - then, when there is tx/rx traffic, this LED should blink (reversely of course). - when hci0 is powered off, this LED turns off. -Guodong > it becomes utterly useless on pretty much every system that is out there. > > Regards > > Marcel >
Hi Guodong, >>>>> Two LED triggers are added into hci_dev: tx_led and rx_led. Upon ACL/SCO >>>>> packets available in tx or rx, the LEDs will blink. >>>>> >>>>> For each hci registration, two triggers are added into LED subsystem: >>>>> [hdev->name]-tx and [hdev-name]-rx. >>>>> Refer to Documentation/leds/leds-class.txt for usage. >>>>> >>>>> Verified on HiKey 96boards, which uses HiSilicon hi6220 SoC and TI >>>>> WL1835 WiFi/BT combo chip. >>>> >>>> so I have no idea what to do with adding adding hci0-rx and hci0-tx triggers. Combined with hci0-power trigger these are already 3 triggers. And if you have 2 Bluetooth controllers in your system, then you have 6 triggers. >>>> >>> >>> True, 6 triggers. But, taking example for other subsytems, eg. cpu >>> cores. On my board, I have "heartbeat cpu0 cpu1 cpu2 cpu3 cpu4 cpu5 >>> cpu6 cpu7". It doesn't have to mean you need all of them connected to >>> some LED(s). Actually, in most of the case, I only need heartbeat. >>> >>> >>> >>>> If we then maybe add another trigger, then this number just goes up and up. >>>> >>>> As far as I can tell you can only assign a single trigger to a LED. >>>> >>> >>> That's true. And people got a choice of which feature he wants to visualize. >> >> and as a result we keep adding senseless triggers to the kernel and bloating it up for no reason. Especially since it feels like 99% of the LED triggers are not used at all. This makes no sense to me. >> >>>> So this means to even use these triggers, you need now 3 LEDs per Bluetooth controller. How is that useful for anybody in a real system? Maybe I am missing something here and somehow there is magic to combine triggers, but I have not found it yet. So please someone enlighten me on how this is suppose to be used with real devices. >>>> >>>> Recently I have added a simple bluetooth-power trigger that combines all Bluetooth controllers into a single trigger. If any of them is enabled, then you can control your LED. Which makes a lot more sense to me since you most likely have a single Bluetooth LED on your system. And you want it to show the correct state no matter what Bluetooth controller is in use. However I can see the case that someone might want to assign one specific Bluetooth controller to a LED status. >>>> >>>> So instead of adding many independent triggers to each controller, why not create one global bluetooth trigger and one individual bluetooth-hci0 trigger for each controller. And the combine power, tx, rx and whatever else we need to trigger the LED for? >>>> >>> >>> When I starting this work, I referred to WiFi system. See >>> CONFIG_MAC80211_LEDS. WiFi system implements these types of triggers " >>> phy0rx phy0tx phy0assoc phy0radio" for each 'controller'. >> >> And I actually wonder who ever used these triggers. You need 4 LEDs to visualize the WiFi status. Which systems has 4 LEDs to spare to visualize this. >> >>> Besides, there are also RFKILL which stands for WiFi/BT power status. >>> RFKILL adds triggers for each module too. Eg. in the below example, I >>> have one WiFi (phy0), one BT (hci0). Trigger rfkill1 equals to >>> hci0-power. >>> >>> Ref: here are all LED triggers I found in my 96boards/HiKey: >>> >>> # cat trigger >>> none kbd-scrollock kbd-numlock kbd-capslock kbd-kanalock kbd-shiftlock >>> kbd-altgrlock kbd-ctrllock kbd-altlock kbd-shiftllock kbd-shiftrlock >>> kbd-ctrlllock kbd-ctrlrlock mmc0 mmc1 heartbeat cpu0 cpu1 cpu2 cpu3 >>> cpu4 cpu5 cpu6 cpu7 mmc2 rfkill0 phy0rx phy0tx phy0assoc phy0radio >>> hci0-power hci0-tx [hci0-rx] rfkill1 >> >> And how many LEDs do you have in the your system? I think you are making my point here. >> >> So I think what we need to do is to not add to this madness and instead create one "bluetooth" LED trigger that combines power and TX/RX for all controllers. And then allow for individual "bluetooth-hci0" LED triggers so that you can bind a single Bluetooth controller to a single LED. >> >> For me, if I can not combine hci0-power, hci0-tx and hci0-rx into a single LED, > > By combining them into a single LED, do you mean such a use case? > - when hci0 is powered on, this LED starts on. > - then, when there is tx/rx traffic, this LED should blink (reversely > of course). > - when hci0 is powered off, this LED turns off. yes, that is what I am thinking of. Regards Marcel
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index dc71473..37b8dd9 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -398,6 +398,7 @@ struct hci_dev { bdaddr_t rpa; struct led_trigger *power_led; + struct led_trigger *tx_led, *rx_led; int (*open)(struct hci_dev *hdev); int (*close)(struct hci_dev *hdev); diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 45a9fc6..956dce1 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -3833,6 +3833,7 @@ static void hci_sched_acl(struct hci_dev *hdev) if (!hci_conn_num(hdev, AMP_LINK) && hdev->dev_type == HCI_AMP) return; + hci_leds_blink_oneshot(hdev->tx_led); switch (hdev->flow_ctl_mode) { case HCI_FLOW_CTL_MODE_PACKET_BASED: hci_sched_acl_pkt(hdev); @@ -3856,6 +3857,7 @@ static void hci_sched_sco(struct hci_dev *hdev) if (!hci_conn_num(hdev, SCO_LINK)) return; + hci_leds_blink_oneshot(hdev->tx_led); while (hdev->sco_cnt && (conn = hci_low_sent(hdev, SCO_LINK, "e))) { while (quote-- && (skb = skb_dequeue(&conn->data_q))) { BT_DBG("skb %p len %d", skb, skb->len); @@ -3879,6 +3881,7 @@ static void hci_sched_esco(struct hci_dev *hdev) if (!hci_conn_num(hdev, ESCO_LINK)) return; + hci_leds_blink_oneshot(hdev->tx_led); while (hdev->sco_cnt && (conn = hci_low_sent(hdev, ESCO_LINK, "e))) { while (quote-- && (skb = skb_dequeue(&conn->data_q))) { @@ -3911,6 +3914,7 @@ static void hci_sched_le(struct hci_dev *hdev) hci_link_tx_to(hdev, LE_LINK); } + hci_leds_blink_oneshot(hdev->tx_led); cnt = hdev->le_pkts ? hdev->le_cnt : hdev->acl_cnt; tmp = cnt; while (cnt && (chan = hci_chan_sent(hdev, LE_LINK, "e))) { @@ -3990,6 +3994,7 @@ static void hci_acldata_packet(struct hci_dev *hdev, struct sk_buff *skb) if (conn) { hci_conn_enter_active_mode(conn, BT_POWER_FORCE_ACTIVE_OFF); + hci_leds_blink_oneshot(hdev->rx_led); /* Send to upper protocol */ l2cap_recv_acldata(conn, skb, flags); @@ -4022,6 +4027,7 @@ static void hci_scodata_packet(struct hci_dev *hdev, struct sk_buff *skb) hci_dev_unlock(hdev); if (conn) { + hci_leds_blink_oneshot(hdev->rx_led); /* Send to upper protocol */ sco_recv_scodata(conn, skb); return; diff --git a/net/bluetooth/leds.c b/net/bluetooth/leds.c index 8319c84..ae10c5d 100644 --- a/net/bluetooth/leds.c +++ b/net/bluetooth/leds.c @@ -19,6 +19,8 @@ struct hci_basic_led_trigger { #define to_hci_basic_led_trigger(arg) container_of(arg, \ struct hci_basic_led_trigger, led_trigger) +#define BLUETOOTH_BLINK_DELAY 50 /* ms */ + void hci_leds_update_powered(struct hci_dev *hdev, bool enabled) { if (hdev->power_led) @@ -37,6 +39,17 @@ static void power_activate(struct led_classdev *led_cdev) led_trigger_event(led_cdev->trigger, powered ? LED_FULL : LED_OFF); } +void hci_leds_blink_oneshot(struct led_trigger *trig) +{ + unsigned long led_delay = BLUETOOTH_BLINK_DELAY; + + if (!trig) + return; + + BT_DBG("led_trig %p", trig); + led_trigger_blink_oneshot(trig, &led_delay, &led_delay, 0); +} + static struct led_trigger *led_allocate_basic(struct hci_dev *hdev, void (*activate)(struct led_classdev *led_cdev), const char *name) @@ -71,4 +84,8 @@ void hci_leds_init(struct hci_dev *hdev) { /* initialize power_led */ hdev->power_led = led_allocate_basic(hdev, power_activate, "power"); + /* initialize tx_led */ + hdev->tx_led = led_allocate_basic(hdev, NULL, "tx"); + /* initialize rx_led */ + hdev->rx_led = led_allocate_basic(hdev, NULL, "rx"); } diff --git a/net/bluetooth/leds.h b/net/bluetooth/leds.h index a9c4d6e..9b1cccd 100644 --- a/net/bluetooth/leds.h +++ b/net/bluetooth/leds.h @@ -9,8 +9,10 @@ #if IS_ENABLED(CONFIG_BT_LEDS) void hci_leds_update_powered(struct hci_dev *hdev, bool enabled); void hci_leds_init(struct hci_dev *hdev); +void hci_leds_blink_oneshot(struct led_trigger *trig); #else static inline void hci_leds_update_powered(struct hci_dev *hdev, bool enabled) {} static inline void hci_leds_init(struct hci_dev *hdev) {} +static inline void hci_leds_blink_oneshot(struct led_trigger *trig) {} #endif
Two LED triggers are added into hci_dev: tx_led and rx_led. Upon ACL/SCO packets available in tx or rx, the LEDs will blink. For each hci registration, two triggers are added into LED subsystem: [hdev->name]-tx and [hdev-name]-rx. Refer to Documentation/leds/leds-class.txt for usage. Verified on HiKey 96boards, which uses HiSilicon hi6220 SoC and TI WL1835 WiFi/BT combo chip. Signed-off-by: Guodong Xu <guodong.xu@linaro.org> --- include/net/bluetooth/hci_core.h | 1 + net/bluetooth/hci_core.c | 6 ++++++ net/bluetooth/leds.c | 17 +++++++++++++++++ net/bluetooth/leds.h | 2 ++ 4 files changed, 26 insertions(+)