2020-10-14 16:18:19

by Kiran K

[permalink] [raw]
Subject: [PATCH v7 2/4] Bluetooth: btusb: Define a function to construct firmware filename

Define a new function to construct firmware/ddc filename for new
generation Intel controllers

Signed-off-by: Kiran K <[email protected]>
Reviewed-by: Chethan T N <[email protected]>
---
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))))
+
#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.
*
--
2.7.4


2020-11-11 11:03:24

by Marcel Holtmann

[permalink] [raw]
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 <[email protected]>
> Reviewed-by: Chethan T N <[email protected]>
> ---
> 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

2020-11-19 11:24:27

by Kiran K

[permalink] [raw]
Subject: RE: [PATCH v7 2/4] Bluetooth: btusb: Define a function to construct firmware filename

Hi Marcel,

> -----Original Message-----
> From: Marcel Holtmann <[email protected]>
> Sent: Wednesday, November 11, 2020 4:33 PM
> To: Kiran K <[email protected]>
> Cc: linux-bluetooth <[email protected]>; Narasimman, Sathish
> <[email protected]>; Tumkur Narayan, Chethan
> <[email protected]>; Srivatsa, Ravishankar
> <[email protected]>; K, Kiran <[email protected]>
> 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 <[email protected]>
> > Reviewed-by: Chethan T N <[email protected]>
> > ---
> > 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