diff mbox series

[v7,2/4] Bluetooth: btusb: Define a function to construct firmware filename

Message ID 1602678517-9912-2-git-send-email-kiran.k@intel.com
State New
Headers show
Series [v7,1/4] Bluetooth: btusb: Add *setup* function for new generation Intel controllers | expand

Commit Message

Kiran K Oct. 14, 2020, 12:28 p.m. UTC 
Define a new function to construct firmware/ddc filename for new
generation Intel controllers

Signed-off-by: Kiran K <kiran.k@intel.com>
Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
---
 drivers/bluetooth/btintel.h |  6 ++++++
 drivers/bluetooth/btusb.c   | 19 +++++++++++++++++--
 2 files changed, 23 insertions(+), 2 deletions(-)

Comments

Marcel Holtmann Nov. 11, 2020, 11:02 a.m. UTC | #1 
Hi Kiran,

> Define a new function to construct firmware/ddc filename for new

> generation Intel controllers

> 

> Signed-off-by: Kiran K <kiran.k@intel.com>

> Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>

> ---

> drivers/bluetooth/btintel.h |  6 ++++++

> drivers/bluetooth/btusb.c   | 19 +++++++++++++++++--

> 2 files changed, 23 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))))

> +


how is the endian safe?

> #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 2e40885..ac532dd 100644

> --- a/drivers/bluetooth/btusb.c

> +++ b/drivers/bluetooth/btusb.c

> @@ -2360,6 +2360,21 @@ static bool btusb_setup_intel_new_get_fw_name(struct intel_version *ver,

> 	return true;

> }

> 

> +static void btusb_setup_intel_newgen_get_fw_name(const struct intel_version_tlv *ver_tlv,

> +						 char *fw_name, size_t len,

> +						 const char *suffix)

> +{

> +	/* The firmware file name for new generation controllers will be

> +	 * ibt-<cnvi_top type+cnvi_top step>-<cnvr_top type+cnvr_top step>

> +	 */

> +	snprintf(fw_name, len, "intel/ibt-%04x-%04x.%s",

> +		 INTEL_CNVX_TOP_PACK_SWAB(INTEL_CNVX_TOP_TYPE(ver_tlv->cnvi_top),

> +					  INTEL_CNVX_TOP_STEP(ver_tlv->cnvi_top)),

> +		 INTEL_CNVX_TOP_PACK_SWAB(INTEL_CNVX_TOP_TYPE(ver_tlv->cnvr_top),

> +					  INTEL_CNVX_TOP_STEP(ver_tlv->cnvr_top)),

> +		 suffix);

> +}

> +

> static int btusb_intel_download_firmware(struct hci_dev *hdev,

> 					 struct intel_version *ver,

> 					 struct intel_boot_params *params,

> @@ -2783,8 +2798,8 @@ static int btusb_setup_intel_newgen(struct hci_dev *hdev)

> 

> 	clear_bit(BTUSB_BOOTLOADER, &data->flags);

> 

> -	/* TODO: Insert function call here to get the ddc file name */

> -

> +	btusb_setup_intel_newgen_get_fw_name(&version, ddcname, sizeof(ddcname),

> +					     "ddc");

> 	/* Once the device is running in operational mode, it needs to

> 	 * apply the device configuration (DDC) parameters.


Regards

Marcel
Kiran K Nov. 19, 2020, 11:22 a.m. UTC | #2 
Hi Marcel,

> -----Original Message-----

> From: Marcel Holtmann <marcel@holtmann.org>

> Sent: Wednesday, November 11, 2020 4:33 PM

> To: Kiran K <kiraank@gmail.com>

> Cc: linux-bluetooth <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>

> Subject: Re: [PATCH v7 2/4] Bluetooth: btusb: Define a function to construct

> firmware filename

> 

> Hi Kiran,

> 

> > Define a new function to construct firmware/ddc filename for new

> > generation Intel controllers

> >

> > Signed-off-by: Kiran K <kiran.k@intel.com>

> > Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>

> > ---

> > drivers/bluetooth/btintel.h |  6 ++++++

> > drivers/bluetooth/btusb.c   | 19 +++++++++++++++++--

> > 2 files changed, 23 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))))

> > +

> 

> how is the endian safe?

> 


It's not. Thanks for catching this. I will add a new patch, which does __le32_to_cpu() on TLV data before storing the values in structure.

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

> > 2e40885..ac532dd 100644

> > --- a/drivers/bluetooth/btusb.c

> > +++ b/drivers/bluetooth/btusb.c

> > @@ -2360,6 +2360,21 @@ static bool

> btusb_setup_intel_new_get_fw_name(struct intel_version *ver,

> > 	return true;

> > }

> >

> > +static void btusb_setup_intel_newgen_get_fw_name(const struct

> intel_version_tlv *ver_tlv,

> > +						 char *fw_name, size_t len,

> > +						 const char *suffix)

> > +{

> > +	/* The firmware file name for new generation controllers will be

> > +	 * ibt-<cnvi_top type+cnvi_top step>-<cnvr_top type+cnvr_top step>

> > +	 */

> > +	snprintf(fw_name, len, "intel/ibt-%04x-%04x.%s",

> > +

> INTEL_CNVX_TOP_PACK_SWAB(INTEL_CNVX_TOP_TYPE(ver_tlv->cnvi_top),

> > +					  INTEL_CNVX_TOP_STEP(ver_tlv-

> >cnvi_top)),

> > +

> INTEL_CNVX_TOP_PACK_SWAB(INTEL_CNVX_TOP_TYPE(ver_tlv->cnvr_top),

> > +					  INTEL_CNVX_TOP_STEP(ver_tlv-

> >cnvr_top)),

> > +		 suffix);

> > +}

> > +

> > static int btusb_intel_download_firmware(struct hci_dev *hdev,

> > 					 struct intel_version *ver,

> > 					 struct intel_boot_params *params,

> @@ -2783,8 +2798,8 @@ static

> > int btusb_setup_intel_newgen(struct hci_dev *hdev)

> >

> > 	clear_bit(BTUSB_BOOTLOADER, &data->flags);

> >

> > -	/* TODO: Insert function call here to get the ddc file name */

> > -

> > +	btusb_setup_intel_newgen_get_fw_name(&version, ddcname,

> sizeof(ddcname),

> > +					     "ddc");

> > 	/* Once the device is running in operational mode, it needs to

> > 	 * apply the device configuration (DDC) parameters.

> 

> Regards

> 

> Marcel


Thanks,
Kiran
diff mbox series

Patch

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 2e40885..ac532dd 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2360,6 +2360,21 @@  static bool btusb_setup_intel_new_get_fw_name(struct intel_version *ver,
 	return true;
 }
 
+static void btusb_setup_intel_newgen_get_fw_name(const struct intel_version_tlv *ver_tlv,
+						 char *fw_name, size_t len,
+						 const char *suffix)
+{
+	/* The firmware file name for new generation controllers will be
+	 * ibt-<cnvi_top type+cnvi_top step>-<cnvr_top type+cnvr_top step>
+	 */
+	snprintf(fw_name, len, "intel/ibt-%04x-%04x.%s",
+		 INTEL_CNVX_TOP_PACK_SWAB(INTEL_CNVX_TOP_TYPE(ver_tlv->cnvi_top),
+					  INTEL_CNVX_TOP_STEP(ver_tlv->cnvi_top)),
+		 INTEL_CNVX_TOP_PACK_SWAB(INTEL_CNVX_TOP_TYPE(ver_tlv->cnvr_top),
+					  INTEL_CNVX_TOP_STEP(ver_tlv->cnvr_top)),
+		 suffix);
+}
+
 static int btusb_intel_download_firmware(struct hci_dev *hdev,
 					 struct intel_version *ver,
 					 struct intel_boot_params *params,
@@ -2783,8 +2798,8 @@  static int btusb_setup_intel_newgen(struct hci_dev *hdev)
 
 	clear_bit(BTUSB_BOOTLOADER, &data->flags);
 
-	/* TODO: Insert function call here to get the ddc file name */
-
+	btusb_setup_intel_newgen_get_fw_name(&version, ddcname, sizeof(ddcname),
+					     "ddc");
 	/* Once the device is running in operational mode, it needs to
 	 * apply the device configuration (DDC) parameters.
 	 *