RE: [PATCH v9 10/10] Bluetooth: Add support for msbc coding format

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

Hi Marcel,

> -----Original Message-----
> From: Marcel Holtmann <marcel@holtmann.org>
> Sent: Wednesday, June 16, 2021 1:14 AM
> To: K, Kiran <kiran.k@intel.com>
> Cc: linux-bluetooth@vger.kernel.org
> Subject: Re: [PATCH v9 10/10] Bluetooth: Add support for msbc coding
> format
> 
> Hi Kiran,
> 
> > In Enhanced_Setup_Synchronous_Command, add support for msbc coding
> > format
> >
> > 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>
> > ---
> > include/net/bluetooth/bluetooth.h |  1 +
> > net/bluetooth/hci_conn.c          | 27 ++++++++++++++++++++++++++-
> > 2 files changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/net/bluetooth/bluetooth.h
> > b/include/net/bluetooth/bluetooth.h
> > index af2809121571..056699224da7 100644
> > --- a/include/net/bluetooth/bluetooth.h
> > +++ b/include/net/bluetooth/bluetooth.h
> > @@ -175,6 +175,7 @@ struct bt_codecs {
> >
> > #define CODING_FORMAT_CVSD		0x02
> > #define CODING_FORMAT_TRANSPARENT	0x03
> > +#define CODING_FORMAT_MSBC		0x05
> >
> > __printf(1, 2)
> > void bt_info(const char *fmt, ...);
> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index
> > 9569b21adb88..73c134459361 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -328,6 +328,32 @@ bool hci_enhanced_setup_sync(struct hci_conn
> *conn, __u16 handle)
> > 	cp.rx_bandwidth   = cpu_to_le32(0x00001f40);
> >
> > 	switch (conn->codec.id) {
> > +	case CODING_FORMAT_MSBC:
> > +		if (!find_next_esco_param(conn, esco_param_msbc,
> > +					  ARRAY_SIZE(esco_param_msbc)))
> > +			return false;
> > +
> > +		param = &esco_param_msbc[conn->attempt - 1];
> > +		cp.tx_coding_format.id = 0x05;
> > +		cp.rx_coding_format.id = 0x05;
> > +		cp.tx_codec_frame_size = __cpu_to_le16(60);
> > +		cp.rx_codec_frame_size = __cpu_to_le16(60);
> > +		cp.in_bandwidth = __cpu_to_le32(32000);
> > +		cp.out_bandwidth = __cpu_to_le32(32000);
> > +		cp.in_coding_format.id = 0x04;
> > +		cp.out_coding_format.id = 0x04;
> > +		cp.in_coded_data_size = __cpu_to_le16(16);
> > +		cp.out_coded_data_size = __cpu_to_le16(16);
> > +		cp.in_pcm_data_format = 2;
> > +		cp.out_pcm_data_format = 2;
> > +		cp.in_pcm_sample_payload_msb_pos = 0;
> > +		cp.out_pcm_sample_payload_msb_pos = 0;
> > +		cp.in_data_path = conn->codec.data_path;
> > +		cp.out_data_path = conn->codec.data_path;
> > +		cp.in_trasnport_unit_size = 1;
> > +		cp.out_trasnport_unit_size = 1;
> 
> so s/trasnport/transport/
> 
> Please spellcheck your structs.

Ack.

> 
> > +		break;
> > +
> > 	case CODING_FORMAT_TRANSPARENT:
> > 		if (!find_next_esco_param(conn, esco_param_msbc,
> > 					  ARRAY_SIZE(esco_param_msbc)))
> > @@ -383,7 +409,6 @@ bool hci_enhanced_setup_sync(struct hci_conn
> *conn, __u16 handle)
> > 		cp.in_trasnport_unit_size = 16;
> > 		cp.out_trasnport_unit_size = 16;
> > 		break;
> > -
> 
> We can not have these random hunks in patches. You need to review your
> final set before sending it out.

Ack.

> 
> Regards
> 
> Marcel

Thanks,
Kiran