Hi Marcel, > -----Original Message----- > From: Marcel Holtmann <marcel@xxxxxxxxxxxx> > Sent: Wednesday, November 11, 2020 4:33 PM > To: Kiran K <kiraank@xxxxxxxxx> > Cc: linux-bluetooth <linux-bluetooth@xxxxxxxxxxxxxxx>; Narasimman, Sathish > <sathish.narasimman@xxxxxxxxx>; Tumkur Narayan, Chethan > <chethan.tumkur.narayan@xxxxxxxxx>; Srivatsa, Ravishankar > <ravishankar.srivatsa@xxxxxxxxx>; K, Kiran <kiran.k@xxxxxxxxx> > 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@xxxxxxxxx> > > Reviewed-by: Chethan T N <chethan.tumkur.narayan@xxxxxxxxx> > > --- > > 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