RE: [PATCH v4] Bluetooth: btintel: Add *setup* function for new generation Intel controllers

["K, Kiran" <kiran.k@intel.com>]

Hi Marcel, Luiz,

> -----Original Message-----
> From: Marcel Holtmann <marcel@holtmann.org>
> Sent: Saturday, October 3, 2020 12:12 PM
> To: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> Cc: Kiran K <kiraank@gmail.com>; linux-bluetooth@vger.kernel.org;
> Narasimman, Sathish <sathish.narasimman@intel.com>; Tumkur Narayan,
> Chethan <chethan.tumkur.narayan@intel.com>; Srivatsa, Ravishankar
> <ravishankar.srivatsa@intel.com>; K, Kiran <kiran.k@intel.com>; Bag, Amit K
> <amit.k.bag@intel.com>; Raghuram Hegde <raghuram.hegde@intel.com>
> Subject: Re: [PATCH v4] Bluetooth: btintel: Add *setup* function for new
> generation Intel controllers
> 
> Hi Luiz,
> 
> >> Signed-off-by: Kiran K <kiran.k@intel.com>
> >> Signed-off-by: Amit K Bag <amit.k.bag@intel.com>
> >> Signed-off-by: Raghuram Hegde <raghuram.hegde@intel.com>
> >> Reviewed-by: Sathish Narasimman <Sathish.Narasimman@intel.com>
> >> Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> >> Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
> >> ---
> >> drivers/bluetooth/btintel.h |   6 +
> >> drivers/bluetooth/btusb.c   | 324
> +++++++++++++++++++++++++++++++++++++++++++-
> >> 2 files changed, 328 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/bluetooth/btintel.h
> >> b/drivers/bluetooth/btintel.h index 09346ae..c4e28a8 100644
> >> --- a/drivers/bluetooth/btintel.h
> >> +++ b/drivers/bluetooth/btintel.h
> >> @@ -132,6 +132,12 @@ struct intel_debug_features {
> >>        __u8    page1[16];
> >> } __packed;
> >>
> >> +#define INTEL_HW_PLATFORM(cnvx_bt)     ((u8)(((cnvx_bt) & 0x0000ff00)
> >> 8))
> >> +#define INTEL_HW_VARIANT(cnvx_bt)      ((u8)(((cnvx_bt) & 0x003f0000)
> >> 16))
> >> +#define INTEL_CNVX_TOP_TYPE(cnvx_top)  ((cnvx_top) & 0x00000fff)
> >> +#define INTEL_CNVX_TOP_STEP(cnvx_top)  (((cnvx_top) & 0x0f000000)
> >>
> >> +24) #define INTEL_CNVX_TOP_PACK_SWAB(t, s) __swab16(((__u16)(((t)
> <<
> >> +4) | (s))))
> >> +
> >> #if IS_ENABLED(CONFIG_BT_INTEL)
> >>
> >> int btintel_check_bdaddr(struct hci_dev *hdev); diff --git
> >> a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index
> >> 1005b6e..5e51749 100644
> >> --- a/drivers/bluetooth/btusb.c
> >> +++ b/drivers/bluetooth/btusb.c
> >> @@ -60,6 +60,7 @@ static struct usb_driver btusb_driver; #define
> >> BTUSB_WIDEBAND_SPEECH  0x400000
> >> #define BTUSB_VALID_LE_STATES   0x800000
> >> #define BTUSB_QCA_WCN6855      0x1000000
> >> +#define BTUSB_INTEL_NEWGEN     0x2000000
> >
> > I wonder if it is a good idea to keep adding such flags per model
> > here, it should be possible to pass the pid/vid so we don't have to
> > add generation after generation here.
> 
> We agreed to first add the next gen controller as an independent setup
> method. And then later combine them into a single one that detects what
> version it is. We also need to unify it with our first generation ROM products.

We can still have an independent setup function for next gen controllers. Only the method to identify the controller type is changed to use PID/VID instead of BTUSB_INTEL_NEW. I will submit the refined patch in V5. Please review it. If you still feel that we need to use BTUSB_INTEL_NEW, I will revert to v4.
> 
> Regards
> 
> Marcel

Thanks,
Kiran