Re: [PATCH 1/6] Bluetooth: Add hci_flags to struct hci_dev

[Marcel Holtmann <marcel@holtmann.org>]

Hi Andre,

> >> This patch adds the hci_flags field to struct hci_dev. This new
> >> flags variable should be used to define flags related to BR/EDR
> >> and/or LE controller itself. It should be used to define flags
> >> which represents states from the controller. The hci_flags is
> >> cleared in case the controller sends a Reset Command Complete
> >> Event to the host.
> >> 
> >> Also, this patch adds the HCI_LE_SCAN flag which was created to
> >> track if the controller is performing LE scan or not. The flag
> >> is set/cleared when the controller starts/stops scanning.
> >> 
> >> This is an initial effort to stop using hdev->flags to define
> >> internal flags since it is exported to userspace by an ioctl.
> >> 
> >> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> >> ---
> >> include/net/bluetooth/hci.h      |    8 ++++++++
> >> include/net/bluetooth/hci_core.h |    2 ++
> >> net/bluetooth/hci_core.c         |    1 +
> >> net/bluetooth/hci_event.c        |    6 ++++++
> >> 4 files changed, 17 insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> >> index 139ce2a..70321a1 100644
> >> --- a/include/net/bluetooth/hci.h
> >> +++ b/include/net/bluetooth/hci.h
> >> @@ -88,6 +88,14 @@ enum {
> >> 	HCI_RESET,
> >> };
> >> 
> >> +/*
> >> + * BR/EDR and/or LE controller flags: the flags defined here should represent
> >> + * states from the controller.
> >> + */
> >> +enum {
> >> +	HCI_LE_SCAN,
> >> +};
> >> +
> >> /* HCI ioctl defines */
> >> #define HCIDEVUP	_IOW('H', 201, int)
> >> #define HCIDEVDOWN	_IOW('H', 202, int)
> >> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> >> index 1795257..f6d5d90 100644
> >> --- a/include/net/bluetooth/hci_core.h
> >> +++ b/include/net/bluetooth/hci_core.h
> >> @@ -250,6 +250,8 @@ struct hci_dev {
> >> 
> >> 	struct module		*owner;
> >> 
> >> +	unsigned long		hci_flags;
> >> +
> > 
> > so I remember that I said, we call these mgmt_flags and make sure that
> > all the flags are bound the mgmt interface. Why are we calling this
> > hci_flags now?
> 
> I realized this flags variable is more related to the controller
> itself than to management interface. For instance, HCI_LE_SCAN,
> HCI_INQUIRY, HCI_PSCAN, HCI_ISCAN and others flags pretty much
> related to the _controller_. Additionally, the PINQUIRY flag, which
> we might add soon, might be defined in hci_flags too, since it is
> related to the controller.
> 
> About the mgmt_flags, I was thinking in using this flags variable
> to define management interface related flags. Flags such as
> HCI_MGMT, HCI_PAIRABLE, HCI_SERVICE_CACHE and HCI_LINK_KEYS could
> be added to mgmt_flags since they are all related to management
> interface itself. In a patch in interleaved discovery support series
> (as I said, I'll send it to ML soon), I create the mgmt_flags variable
> and define the MGMT_DISCOV flags to track if we are carrying out a
> discovery or not. 
> 
> So, summarizing we would have two flags variables: hci_flags (which
> holds flags related to the controller) and mgmt_flags (which holds
> flags related to management interface).
> 
> Do we keep hci_flags or rename it to mgmt_flags and mix up controller
> and management interface flags?

then just call them dev_flags (like we have dev_type). Prefixing things
with hci_ seems wrong to me.

Regards

Marcel