Return-Path: Message-ID: <1331148191.3392.185.camel@aeonflux> Subject: Re: [PATCH 1/2] Bluetooth: Add hci_cancel_le_scan() to hci_core From: Marcel Holtmann To: Andre Guedes Cc: linux-bluetooth@vger.kernel.org Date: Wed, 07 Mar 2012 11:23:11 -0800 In-Reply-To: References: <1331074474-12868-1-git-send-email-andre.guedes@openbossa.org> <1331074474-12868-2-git-send-email-andre.guedes@openbossa.org> <20120306230730.GA8386@joana> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Sender: linux-bluetooth-owner@vger.kernel.org List-ID: Hi Andre, > > * Andre Guedes [2012-03-06 19:54:33 -0300]: > > > >> This patch adds to hci_core the hci_cancel_le_scan function which > >> should be used to cancel an ongoing LE scan. > >> > >> Signed-off-by: Andre Guedes > >> --- > >> include/net/bluetooth/hci_core.h | 1 + > >> net/bluetooth/hci_core.c | 21 +++++++++++++++++++++ > >> 2 files changed, 22 insertions(+) > >> > >> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h > >> index 25cb0a1..0db2934 100644 > >> --- a/include/net/bluetooth/hci_core.h > >> +++ b/include/net/bluetooth/hci_core.h > >> @@ -1072,5 +1072,6 @@ int hci_do_inquiry(struct hci_dev *hdev, u8 length); > >> int hci_cancel_inquiry(struct hci_dev *hdev); > >> int hci_le_scan(struct hci_dev *hdev, u8 type, u16 interval, u16 window, > >> int timeout); > >> +int hci_cancel_le_scan(struct hci_dev *hdev); > >> > >> #endif /* __HCI_CORE_H */ > >> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > >> index 661d65f..0c2ceaa 100644 > >> --- a/net/bluetooth/hci_core.c > >> +++ b/net/bluetooth/hci_core.c > >> @@ -1672,6 +1672,27 @@ static int hci_do_le_scan(struct hci_dev *hdev, u8 type, u16 interval, > >> return 0; > >> } > >> > >> +int hci_cancel_le_scan(struct hci_dev *hdev) > >> +{ > >> + bool canceled; > >> + > >> + BT_DBG("%s", hdev->name); > >> + > >> + if (!test_bit(HCI_LE_SCAN, &hdev->dev_flags)) > >> + return -EPERM; > > > > Are you sure about this -EPERM return error here? At a glance -EALREADY looks > > better to me. > > This function cancels an operation (LE scan). If the operation is not > running, it makes more sense to me returning "Operation not permitted" > instead of "Operation already in progress". actually EPERM is for operation not permitted because you do not have rights to access is. Not because it is invalid operation. You need to find a better error code. Regards Marcel