RE: [PATCH v11 03/10] Bluetooth: btintel: Read supported offload usecases

From: "K, Kiran" <kiran.k@intel.com>
To: Marcel Holtmann <marcel@holtmann.org>
Cc: "linux-bluetooth@vger.kernel.org"
	<linux-bluetooth@vger.kernel.org>,
	"Srivatsa, Ravishankar" <ravishankar.srivatsa@intel.com>,
	"Tumkur Narayan, Chethan" <chethan.tumkur.narayan@intel.com>
Subject: RE: [PATCH v11 03/10] Bluetooth: btintel: Read supported offload usecases
Date: Sat, 14 Aug 2021 06:32:50 +0000	[thread overview]
Message-ID: <DM8PR11MB5573F9B6EDDBB7999D1860E4F5FB9@DM8PR11MB5573.namprd11.prod.outlook.com> (raw)
In-Reply-To: <3710CF06-1420-4C39-ADD1-FA39E4909FA9@holtmann.org>

Hi Marcel,

> -----Original Message-----
> From: Marcel Holtmann <marcel@holtmann.org>
> Sent: Friday, July 30, 2021 7:40 PM
> To: K, Kiran <kiran.k@intel.com>
> Cc: linux-bluetooth@vger.kernel.org; Srivatsa, Ravishankar
> <ravishankar.srivatsa@intel.com>; Tumkur Narayan, Chethan
> <chethan.tumkur.narayan@intel.com>
> Subject: Re: [PATCH v11 03/10] Bluetooth: btintel: Read supported offload
> usecases
> 
> Hi Kiran,
> 
> > Read offload usecases supported by controller.
> >
> > Signed-off-by: Kiran K <kiran.k@intel.com>
> > Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> > Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
> > ---
> > * changes in v11:
> >  - Remove Kconfig related changes
> > * changes in v10:
> >  - restructure patch to have  definition and call of callaback in the
> >    same patch
> > * changes in v9:
> >  - define a separate patch for core changes
> >
> > drivers/bluetooth/btintel.c | 30 ++++++++++++++++++++++++++++++
> > drivers/bluetooth/btintel.h | 10 ++++++++++
> > drivers/bluetooth/btusb.c   |  3 +++
> > 3 files changed, 43 insertions(+)
> >
> > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> > index e44b6993cf91..a6b81914766e 100644
> > --- a/drivers/bluetooth/btintel.c
> > +++ b/drivers/bluetooth/btintel.c
> > @@ -1272,6 +1272,36 @@ int btintel_set_debug_features(struct hci_dev
> > *hdev, } EXPORT_SYMBOL_GPL(btintel_set_debug_features);
> >
> > +int btintel_configure_offload_usecases(struct hci_dev *hdev) {
> > +	struct sk_buff *skb;
> > +	int err = 0;
> > +	struct intel_offload_usecases *usecases;
> > +
> > +	skb = __hci_cmd_sync(hdev, 0xfc86, 0, NULL, HCI_INIT_TIMEOUT);
> > +	if (IS_ERR(skb)) {
> > +		bt_dev_err(hdev, "Reading offload usecases failed (%ld)",
> > +			   PTR_ERR(skb));
> > +		return PTR_ERR(skb);
> > +	}
> > +
> > +	if (skb->len < sizeof(*usecases)) {
> > +		err = -EIO;
> > +		goto error;
> > +	}
> > +
> > +	usecases = (void *)skb->data;
> > +
> > +	if (usecases->status) {
> > +		err = -bt_to_errno(skb->data[0]);
> > +		goto error;
> > +	}
> > +error:
> > +	kfree_skb(skb);
> > +	return err;
> > +}
> > +EXPORT_SYMBOL_GPL(btintel_configure_offload_usecases);
> > +
> > MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
> > MODULE_DESCRIPTION("Bluetooth support for Intel devices ver "
> > VERSION); MODULE_VERSION(VERSION); diff --git
> > a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h index
> > d184064a5e7c..45ca5c4ccf37 100644
> > --- a/drivers/bluetooth/btintel.h
> > +++ b/drivers/bluetooth/btintel.h
> > @@ -132,6 +132,11 @@ struct intel_debug_features {
> > 	__u8    page1[16];
> > } __packed;
> >
> > +struct intel_offload_usecases {
> > +	__u8	status;
> > +	__u8	preset[8];
> > +} __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)
> > @@ -175,6 +180,7 @@ int btintel_read_debug_features(struct hci_dev
> *hdev,
> > 				struct intel_debug_features *features); int
> > btintel_set_debug_features(struct hci_dev *hdev,
> > 			       const struct intel_debug_features *features);
> > +int btintel_configure_offload_usecases(struct hci_dev *hdev);
> 
> so usecases is not, it would be offload_use_cases. However
> btintel_configure_offload is just fine as a function name. With the work from
> Tedd, this would be removed anyway and all done internally via the hdev-
> >setup().

Ok. I will rebase my patches over Tedd's patches.
> 
> Regards
> 
> Marcel

Regards,
Kiran