Hi Luiz,
Thanks for the comments.
> -----Original Message-----
> From: Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx>
> Sent: Wednesday, March 6, 2024 3:57 AM
> To: K, Kiran <kiran.k@xxxxxxxxx>
> Cc: linux-bluetooth@xxxxxxxxxxxxxxx; Srivatsa, Ravishankar
> <ravishankar.srivatsa@xxxxxxxxx>; Tumkur Narayan, Chethan
> <chethan.tumkur.narayan@xxxxxxxxx>
> Subject: Re: [PATCH v3 2/2] Bluetooth: btintel: Add support for downloading
> intermediate loader
>
> Hi Kiran,
>
> On Tue, Mar 5, 2024 at 9:31 AM Kiran K <kiran.k@xxxxxxxxx> wrote:
> >
> > Some variants of Intel controllers like BlazarI supports downloading
> > of Intermediate bootloader (IML) image. IML gives flexibility to fix
> > issues as its not possible to fix issue in Primary bootloader once
> > flashed to ROM. This patch adds the support to download IML before
> downloading operational firmware image.
> >
> > dmesg logs:
> > [13.399003] Bluetooth: Core ver 2.22
> > [13.399006] Bluetooth: Starting self testing [13.401194] Bluetooth:
> > ECDH test passed in 2135 usecs [13.421175] Bluetooth: SMP test passed
> > in 597 usecs [13.421184] Bluetooth: Finished self testing [13.422919]
> > Bluetooth: HCI device and connection manager initialized [13.422923]
> > Bluetooth: HCI socket layer initialized [13.422925] Bluetooth: L2CAP
> > socket layer initialized [13.422930] Bluetooth: SCO socket layer
> > initialized [13.458065] Bluetooth: hci0: Device revision is 0
> > [13.458071] Bluetooth: hci0: Secure boot is disabled [13.458072]
> > Bluetooth: hci0: OTP lock is disabled [13.458072] Bluetooth: hci0: API
> > lock is enabled [13.458073] Bluetooth: hci0: Debug lock is disabled
> > [13.458073] Bluetooth: hci0: Minimum firmware build 1 week 10 2014
> > [13.458075] Bluetooth: hci0: Bootloader timestamp 2022.46 buildtype 1
> > build 26590 [13.458324] Bluetooth: hci0: DSM reset method type: 0x00
> > [13.460678] Bluetooth: hci0: Found device firmware:
> > intel/ibt-0090-0291-iml.sfi [13.460684] Bluetooth: hci0: Boot Address:
> > 0x30099000 [13.460685] Bluetooth: hci0: Firmware Version: 227-11.24
> > [13.562554] Bluetooth: hci0: Waiting for firmware download to complete
> > [13.563023] Bluetooth: hci0: Firmware loaded in 99941 usecs
> > [13.563057] Bluetooth: hci0: Waiting for device to boot [13.565029]
> > Bluetooth: hci0: Malformed MSFT vendor event: 0x02 [13.565148]
> > Bluetooth: hci0: Device booted in 2064 usecs [13.567065] Bluetooth:
> > hci0: No device address configured [13.569010] Bluetooth: hci0: Found
> > device firmware: intel/ibt-0090-0291.sfi [13.569061] Bluetooth: hci0:
> > Boot Address: 0x10000800 [13.569062] Bluetooth: hci0: Firmware
> > Version: 227-11.24 [13.788891] Bluetooth: BNEP (Ethernet Emulation)
> > ver 1.3 [13.788897] Bluetooth: BNEP filters: protocol multicast
> > [13.788902] Bluetooth: BNEP socket layer initialized [15.435905]
> > Bluetooth: hci0: Waiting for firmware download to complete [15.436016]
> > Bluetooth: hci0: Firmware loaded in 1823233 usecs [15.436258]
> > Bluetooth: hci0: Waiting for device to boot [15.471140] Bluetooth:
> > hci0: Device booted in 34277 usecs [15.471201] Bluetooth: hci0:
> > Malformed MSFT vendor event: 0x02 [15.471487] Bluetooth: hci0: Found
> > Intel DDC parameters: intel/ibt-0090-0291.ddc [15.474353] Bluetooth:
> > hci0: Applying Intel DDC parameters completed [15.474486] Bluetooth:
> > hci0: Found Intel DDC parameters: intel/bdaddress.cfg [15.475299]
> > Bluetooth: hci0: Applying Intel DDC parameters completed [15.479381]
> > Bluetooth: hci0: Firmware timestamp 2024.10 buildtype 3 build 58595
> > [15.479385] Bluetooth: hci0: Firmware SHA1: 0xb4f3cc46 [15.483243]
> > Bluetooth: hci0: Fseq status: Success (0x00) [15.483246] Bluetooth:
> > hci0: Fseq executed: 00.00.00.00 [15.483247] Bluetooth: hci0: Fseq BT
> > Top: 00.00.00.00 [15.578712] Bluetooth: MGMT ver 1.22 [15.822682]
> > Bluetooth: RFCOMM TTY layer initialized [15.822690] Bluetooth: RFCOMM
> > socket layer initialized [15.822695] Bluetooth: RFCOMM ver 1.11
> >
> > Signed-off-by: Kiran K <kiran.k@xxxxxxxxx>
> > ---
> > drivers/bluetooth/btintel.c | 38
> > ++++++++++++++++++++++++++++++++++++-
> > drivers/bluetooth/btintel.h | 3 +++
> > 2 files changed, 40 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> > index ed98bb867cff..00e98606cf02 100644
> > --- a/drivers/bluetooth/btintel.c
> > +++ b/drivers/bluetooth/btintel.c
> > @@ -521,6 +521,9 @@ static int btintel_version_info_tlv(struct hci_dev
> *hdev,
> > version->min_fw_build_nn, version->min_fw_build_cw,
> > 2000 + version->min_fw_build_yy);
> > break;
> > + case IMG_IML:
> > + variant = "Intermediate loader";
> > + break;
> > case IMG_OP:
> > variant = "Firmware";
> > break;
> > @@ -2194,10 +2197,26 @@ static void btintel_get_fw_name_tlv(const
> struct intel_version_tlv *ver,
> > char *fw_name, size_t len,
> > const char *suffix) {
> > + const char *format;
> > /* 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",
> > + switch (ver->cnvi_top & 0xfff) {
> > + /* Only Blazar product supports downloading of intermediate loader
> > + * image
> > + */
> > + case CNVI_BLAZARI:
> > + if (ver->img_type == IMG_BOOTLOADER)
> > + format = "intel/ibt-%04x-%04x-iml.%s";
>
> Shouldn't iml be the extension rather than the name? Like in intel/ibt-0090-
> 0291.iml which you can probably achieve by just replacing suffix, that said this
> function seems to be called with .ddc as suffix as well so I assume there is
I feel it's better to keep the extension as sfi as the image format for IML and OP binaries are same.
> some check preventing it to be called while version is IMG_BOOTLOADER?
There is no such restriction. The same function gets called even when version is IMG_BOOTLOADER. Inside this function we check for product and version to decide whether to load IML or OP image.
>
> > + else
> > + format = "intel/ibt-%04x-%04x.%s";
> > + break;
> > + default:
> > + format = "intel/ibt-%04x-%04x.%s";
> > + break;
> > + }
> > +
> > + snprintf(fw_name, len, format,
> > INTEL_CNVX_TOP_PACK_SWAB(INTEL_CNVX_TOP_TYPE(ver-
> >cnvi_top),
> > INTEL_CNVX_TOP_STEP(ver->cnvi_top)),
> >
> > INTEL_CNVX_TOP_PACK_SWAB(INTEL_CNVX_TOP_TYPE(ver->cnvr_top),
> > @@ -2607,6 +2626,23 @@ static int btintel_bootloader_setup_tlv(struct
> hci_dev *hdev,
> > if (err)
> > return err;
> >
> > + err = btintel_read_version_tlv(hdev, ver);
> > + if (err)
> > + return err;
> > +
> > + /* If image type returned is IMG_IML, then controller supports
> > + * intermediae loader image
> > + */
> > + if (ver->img_type == IMG_IML) {
> > + err = btintel_prepare_fw_download_tlv(hdev, ver, &boot_param);
> > + if (err)
> > + return err;
> > +
> > + err = btintel_boot(hdev, boot_param);
> > + if (err)
> > + return err;
> > + }
> > +
> > btintel_clear_flag(hdev, INTEL_BOOTLOADER);
> >
> > btintel_get_fw_name_tlv(ver, ddcname, sizeof(ddcname), "ddc");
> > diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
> > index 52b2f1986f85..ae15b2253b6d 100644
> > --- a/drivers/bluetooth/btintel.h
> > +++ b/drivers/bluetooth/btintel.h
> > @@ -51,7 +51,10 @@ struct intel_tlv {
> > u8 val[];
> > } __packed;
> >
> > +#define CNVI_BLAZARI 0x900
> > +
> > #define IMG_BOOTLOADER 0x01 /* Bootloader image */
> > +#define IMG_IML 0x02 /* Intermediate image */
> > #define IMG_OP 0x03 /* Operational image */
> >
> > struct intel_version_tlv {
> > --
> > 2.34.1
> >
> >
>
>
> --
> Luiz Augusto von Dentz
Regards,
Kiran
