Re: [PATCHv4 1/5] Bluetooth: Define AMP controller statuses

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Andrei,

> > > > > 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.

> > 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


--
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


[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux