Hi Andrei and Marcel, > > > > > > AMP status codes copied from Bluez patch sent by Peter Krystad > > > > > > <pkrystad@xxxxxxxxxxxxxx>. > > > > > > > > > > > > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@xxxxxxxxx> > > > > > > --- > > > > > > include/net/bluetooth/hci.h | 9 +++++++++ > > > > > > 1 files changed, 9 insertions(+), 0 deletions(-) > > > > > > > > > > > > diff --git a/include/net/bluetooth/hci.h > b/include/net/bluetooth/hci.h > > > > > > index 139ce2a..e79ed67 100644 > > > > > > --- a/include/net/bluetooth/hci.h > > > > > > +++ b/include/net/bluetooth/hci.h > > > > > > @@ -56,6 +56,15 @@ > > > > > > #define HCI_BREDR 0x00 > > > > > > #define HCI_AMP 0x01 > > > > > > > > > > > > +/* AMP controller status */ > > > > > > +#define AMP_CTRL_POWERED_DOWN 0x00 > > > > > > +#define AMP_CTRL_BLUETOOTH_ONLY 0x01 > > > > > > +#define AMP_CTRL_NO_CAPACITY 0x02 > > > > > > +#define AMP_CTRL_LOW_CAPACITY 0x03 > > > > > > +#define AMP_CTRL_MEDIUM_CAPACITY 0x04 > > > > > > +#define AMP_CTRL_HIGH_CAPACITY 0x05 > > > > > > +#define AMP_CTRL_FULL_CAPACITY 0x06 > > > > > > + > > > > > > > > > > is hci.h really the right place for these? It is not HCI specific > > > > > per-se. It is A2MP detail. And as mentioned earlier, I do not believe > we > > > > > should do it like this. > > > > > > > > I believe that it is HCI device specific since hci_dev structure is > > > > accountable for BR/EDR and AMP controllers and we currently keep > > > > controller-specific information in hci_dev. > > > > > > > > Those defines indicate AMP controller status like powered or not. > > > > > > > > What would be the better place? > > > > > > > > include/net/bluetooth/hci_core.h > > > > include/net/bluetooth/amp.h > > > > include/net/bluetooth/a2mp.h > > > > > > > > > > > > > I think we need to expose some sort of functionality that lets the > AMP > > > > > drivers handle this dynamically. > > > > > > > > This status and other AMP parameters would be normally returned when > > > > "read local amp info" HCI command. > > > > > > if these information come from the AMP controller, then this is clearly > > > an A2MP specific detail. There is no point in storing them in hci_dev at > > > all. > > > > I thought that A2MP specific details are details related to A2MP > > connection. Information about status of AMP controller does not depend on > > A2MP connection. > > the only part using this information will be A2MP and it has to read it > from the controller fresh almost every time. So yes, it belongs there > and not inside HCI. At least that is how I read the AMP controller > specification and how it is suppose to be used. This status is updated by the controller via unsolicited AMP status event, there is no reason to read it other than during init. I agree with Andrei that storing it in hci_dev makes sense, as this structure represents state of a local HCI controller. AMP Manager data structures are all related to A2MP connections, not devices. AMP Manager does need to know when the status changes, because it has connections to service when this status happens. > > > I think these definition can stay local in net/bluetooth/a2mp.c for now. > > > > maybe a2mp.h then? As we use existing HCI command infrastructure to handle > > AMP specific HCI commands we might need to source the header. > > As I said, leave it local for now. We can always move them around later. > > Regards > > Marcel Regards, Peter. -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
