Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: ghKklTbIKyvOFEEWeMd8furUuy8+M/5G9byFg9o3yazntHVtLpUMtYxL0diUvV3DL/GXImbrKp
 5MLNKAV45R4Q==
IronPort-SDR: 2ebks4uBI2TVQXPBxnbgVifJGupaCnCT+vIVMBCNnUN/OoISwzmaq7DfmyjQTCHLIqM4sHc+4D
 UGx3GWCTJAWg==
From:   "K, Kiran" <kiran.k@intel.com>
To:     Marcel Holtmann <marcel@holtmann.org>
CC:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Subject: RE: [PATCH v9 07/10] Bluetooth: btintel: define callback to set data
 path
Thread-Topic: [PATCH v9 07/10] Bluetooth: btintel: define callback to set data
 path
Thread-Index: AQHXXGDBvoh7Lx/Y80O2QB58/K7zoKsVg5qAgAB8+yA=
Date:   Wed, 16 Jun 2021 03:10:59 +0000
Message-ID: <DM8PR11MB5573451838727540A14200D4F50F9@DM8PR11MB5573.namprd11.prod.outlook.com>
References: <20210608122455.19583-1-kiran.k@intel.com>
 <20210608122455.19583-7-kiran.k@intel.com>
 <4C624839-10D3-4114-9037-4FB9E93E1EB6@holtmann.org>
In-Reply-To: <4C624839-10D3-4114-9037-4FB9E93E1EB6@holtmann.org>
Accept-Language: en-US
Content-Language: en-US
dlp-reaction: no-action
dlp-version: 11.5.1.3
dlp-product: dlpe-windows
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?M69+rPCd/kcTSpWTZh+Ihm4QoBXxXZnfVXnKo8blDxs7Dem7cpiTaO/lmu/o?=
 =?us-ascii?Q?bS3uvanI1s8qmfpw3zvd+dUY0hc7Aq5+JBSs8qw47kpbL1ugdXBeEFqn+mw/?=
 =?us-ascii?Q?GQWLwkOG2oEm0z23NZ+29t/4M5ZX0In6Lg/z51Duts7qWHD7m7y0AsDiAv5y?=
 =?us-ascii?Q?4jy0g3yV8T6jptB4klzfq6B9JOJpHUa7oRwkh9qR2plsFAm45pE2eab6+UVZ?=
 =?us-ascii?Q?rGKhjY446h/YA1SKoMvhbehm7aMRgG1R75sBs1MvqL0+DfIIvH+RI8VIJH0P?=
 =?us-ascii?Q?XoLd+3RoPedlBh6101xZqs1kD2nvU06kuRbXMsTYc0Lw8f+4GK4iSWeN/Ndj?=
 =?us-ascii?Q?c/sbCVZmQbFfRKbA0b3RjQkiRMk4hB2LCd8CZyshIbsl/SyeHAAuT2mcdVbE?=
 =?us-ascii?Q?8Q7Q7aylos9NaJfljfv/j6UyMvHpNO0iNCDSq3imh2JrwrZEOZ9a39FVvTOZ?=
 =?us-ascii?Q?hdjllzLH/XKNHfLGp++Uv9aYo2pfRyebD8/muspGgoEaCwl907xoDTFXSFHs?=
 =?us-ascii?Q?pyTProoP+rGGlHoLv17xN0PzbRmuTcJKYUoox4Brk+t9Xw9AvgsYpNCLNUlO?=
 =?us-ascii?Q?4gkm1vlnyBTFzuhXpRXXbPBpFu4euZ4sUf/yWkPgIkPh8P1mx7GSK+0j4zap?=
 =?us-ascii?Q?0qiiQTKWNAfKnOi/dSAZi1QOKqcsxrSMHW7NqTc+Cow7W65Qf0ANvjnpFCZh?=
 =?us-ascii?Q?1bFZR1enoVHQHjk/UhU3Y1wvgwJdMo5PgIudi+uB8/eB7OiquCFaz7K4CuhZ?=
 =?us-ascii?Q?CBcy4t4vUxNykYboD+ilPv4UYHBws4074s6Gv+GtFRWU/HV3w3rPZAW702xx?=
 =?us-ascii?Q?isu7RY3yp5YjvwOhW7aGsgKAKDTH0UTeIhc9vqYRABZQlmIVKQFv0/xnR26b?=
 =?us-ascii?Q?f2MWNFX7gQ44Y6B4gU/MA80yuyaXVPLhfSy6PR3HoyoZbwhxKCMfrm+oBqsT?=
 =?us-ascii?Q?iUitARhMNrHPq0x90BoaLfqPluvlLxqVnMIshMBsPe80wIQZLW/xjioBwl7j?=
 =?us-ascii?Q?i2UP3qHbmXW3mU1H7ht3DqM3mdIh/Bly82rJhVMAFPSMfNLdEWnUAecdltBj?=
 =?us-ascii?Q?KVvVDQGQ6lQUjmdBYBL7pI9phoIpkcc9rB7+fcnij4nMq2reqS0PHa4jrhCH?=
 =?us-ascii?Q?zj6PZEsqpB3tQcU4qNufOjroXu8c/lO7PbGBOZTD2Z2DS95UOhPVU8K6Fqpv?=
 =?us-ascii?Q?1gYJYtrjFWUGHJzzy2vZo7VfWO7hqhDVNuo1D0Dn3zhlqhmskXZoqwrDv+jD?=
 =?us-ascii?Q?g15vb6fuczBfFwONrn5I4rZO4kmojHexFMuO0llIoXiA4Xp7rcERRgPFQuSa?=
 =?us-ascii?Q?ac5Sev2iP/Kx9cUl1ANaECca?=
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM8PR11MB5573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d4cbd7dd-c705-4a8d-4c91-08d930745dfe
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Jun 2021 03:11:00.0016
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: c3CRe0P5XnchdgPXrXZhcDmcFfMPepFjKj/HP/6Yj+LcxggP6tz4YzDChpXUiAvK2d/yQ4vSGHooAgd1eXFiwA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5568
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-bluetooth.vger.kernel.org>
X-Mailing-List: linux-bluetooth@vger.kernel.org

Hi Marcel,

>=20
> Hi Kiran,
>=20
> > Adds callback function which is called to set the data path for HFP
> > offload case before opening SCO connection
> >
> > 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>
> > ---
> > drivers/bluetooth/btintel.c | 50
> +++++++++++++++++++++++++++++++++++++
> > drivers/bluetooth/btintel.h |  8 ++++++
> > drivers/bluetooth/btusb.c   |  4 ++-
> > include/net/bluetooth/hci.h |  8 ++++++
> > 4 files changed, 69 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> > index 95c6a1bef35e..3eba5c587ef6 100644
> > --- a/drivers/bluetooth/btintel.c
> > +++ b/drivers/bluetooth/btintel.c
> > @@ -1308,6 +1308,56 @@ int btintel_read_offload_usecases(struct
> > hci_dev *hdev, } EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);
> >
> > +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
> > +			  struct bt_codec *codec)
> > +{
> > +	__u8 preset;
> > +	struct hci_op_configure_data_path *cmd;
> > +	__u8 buffer[255];
> > +	struct sk_buff *skb;
> > +
> > +	if (type !=3D SCO_LINK && type !=3D ESCO_LINK)
> > +		return -EINVAL;
> > +
> > +	switch (codec->id) {
> > +	case 0x02:
> > +		preset =3D 0x00;
> > +	break;
> > +	case 0x05:
> > +		preset =3D 0x01;
> > +	break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	cmd =3D (void *)buffer;
> > +	cmd->data_path_id =3D 0x01;
> > +	cmd->len =3D 1;
> > +	cmd->data[0] =3D preset;
> > +
> > +	cmd->direction =3D 0x00;
> > +	skb =3D __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,
> sizeof(*cmd) + 1,
> > +			     cmd, HCI_INIT_TIMEOUT);
> > +	if (IS_ERR(skb)) {
> > +		bt_dev_err(hdev, "configure input data path failed (%ld)",
> > +			   PTR_ERR(skb));
> > +		return PTR_ERR(skb);
> > +	}
> > +	kfree_skb(skb);
> > +
> > +	cmd->direction =3D 0x01;
> > +	skb =3D __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,
> sizeof(*cmd) + 1,
> > +			     cmd, HCI_INIT_TIMEOUT);
> > +	if (IS_ERR(skb)) {
> > +		bt_dev_err(hdev, "configure output data path failed (%ld)",
> > +			   PTR_ERR(skb));
> > +		return PTR_ERR(skb);
> > +	}
> > +	kfree_skb(skb);
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(btintel_set_data_path);
> > +
> > 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
> > 9bcc489680db..9806970c9871 100644
> > --- a/drivers/bluetooth/btintel.h
> > +++ b/drivers/bluetooth/btintel.h
> > @@ -183,6 +183,8 @@ int btintel_set_debug_features(struct hci_dev
> > *hdev, int btintel_read_offload_usecases(struct hci_dev *hdev,
> > 				  struct intel_offload_usecases *usecases); int
> > btintel_get_data_path(struct hci_dev *hdev);
> > +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
> > +			  struct bt_codec *codec);
> > #else
> >
> > static inline int btintel_check_bdaddr(struct hci_dev *hdev) @@ -325,4
> > +327,10 @@ static int btintel_get_data_path(struct hci_dev *hdev) {
> > 	return -EOPNOTSUPP;
> > }
> > +
> > +static int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
> > +				 struct bt_codec *codec)
> > +{
> > +	return -EOPNOTSUPP;
> > +}
> > #endif
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index 1e51beec5776..afafa44752a1 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -3012,8 +3012,10 @@ static int btusb_setup_intel_newgen(struct
> hci_dev *hdev)
> > 	err =3D btintel_read_offload_usecases(hdev, &usecases);
> > 	if (!err) {
> > 		/* set get_data_path callback if offload is supported */
> > -		if (usecases.preset[0] & 0x03)
> > +		if (usecases.preset[0] & 0x03) {
> > 			hdev->get_data_path =3D btintel_get_data_path;
> > +			hdev->set_data_path =3D btintel_set_data_path;
> > +		}
> > 	}
>=20
> > 	/* Read the Intel version information after loading the FW  */ diff
> > --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index 31a5ac8918fc..42963188dcea 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -1250,6 +1250,14 @@ struct hci_rp_read_local_oob_ext_data {
> > 	__u8     rand256[16];
> > } __packed;
> >
> > +#define HCI_CONFIGURE_DATA_PATH	0x0c83
> > +struct hci_op_configure_data_path {
> > +	__u8	direction;
> > +	__u8	data_path_id;
> > +	__u8	len;
> > +	__u8	data[];
> > +} __packed;
> > +
>=20
> if this is a standard HCI command, why is this done as hdev->set_data_pat=
h?
> This makes no sense too me.
Intel uses  HCI_CONFIGURE_DATA_PATH to command to set the preset ID (NBS, W=
BS, ...). Here len and data[] are vendor specific. I should have prefixed t=
hese fields with vnd_. I will address this in next patchset.
>=20
> Regards
>=20
> Marcel

Thanks,
Kiran