| Message ID | 20240818201533.89669-1-marex@denx.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] wifi: brcmfmac: add support for TRX firmware download | expand |
Marek Vasut <marex@denx.de> wrote: > From: Chung-Hsien Hsu <stanley.hsu@cypress.com> > > Add support to download TRX firmware for PCIe and SDIO. > > Signed-off-by: Chung-Hsien Hsu <chung-hsien.hsu@infineon.com> > Signed-off-by: Marek Vasut <marex@denx.de> # Upport to current linux-next Please fix the review comments, also Ping's comment that from and s-o-b needs to match. 2 patches set to Changes Requested. 13767592 [1/2] wifi: brcmfmac: add support for TRX firmware download 13767593 [2/2] wifi: brcmfmac: add support for CYW55572 PCIe chipset
On 8/22/24 10:37 AM, Kalle Valo wrote: > Marek Vasut <marex@denx.de> wrote: > >> From: Chung-Hsien Hsu <stanley.hsu@cypress.com> >> >> Add support to download TRX firmware for PCIe and SDIO. >> >> Signed-off-by: Chung-Hsien Hsu <chung-hsien.hsu@infineon.com> >> Signed-off-by: Marek Vasut <marex@denx.de> # Upport to current linux-next > > Please fix the review comments, also Ping's comment that from and s-o-b > needs to match. I have most of the changes addressed locally already. Regarding the SoB line, do I update the commit Author email (because that would make more sense, cypress got assimilated into infineon) or the SoB line email to the "old" cypress email address ? I am still hoping to get a bit more input on the TRX firmware handling from Arend ... or maybe there is no further feedback ?
+ Double Lo On August 22, 2024 5:24:09 PM Marek Vasut <marex@denx.de> wrote: > On 8/22/24 10:37 AM, Kalle Valo wrote: >> Marek Vasut <marex@denx.de> wrote: >> >>> From: Chung-Hsien Hsu <stanley.hsu@cypress.com> >>> >>> Add support to download TRX firmware for PCIe and SDIO. >>> >>> Signed-off-by: Chung-Hsien Hsu <chung-hsien.hsu@infineon.com> >>> Signed-off-by: Marek Vasut <marex@denx.de> # Upport to current linux-next >> >> Please fix the review comments, also Ping's comment that from and s-o-b >> needs to match. > > I have most of the changes addressed locally already. > > Regarding the SoB line, do I update the commit Author email (because > that would make more sense, cypress got assimilated into infineon) or > the SoB line email to the "old" cypress email address ? I would say the SoB line is leading as developer's certificate and that has the Infineon address so that seems more appropriate. > I am still hoping to get a bit more input on the TRX firmware handling > from Arend ... or maybe there is no further feedback ? Took me a while before I looked into this. Can not offer much info though. Within Broadcom code bases we only have a v1 and v2 defined for the TRX format. This patch specifies v4. The only difference seem to be the number of offsets and their meaning. Looking at the index definitions I suspect this version accommodates signed firmware images which is verified by the bootloader on the device. Hopefully Infineon can/will confirm my suspicion and explain what SE in .TRXSE stands for (security extension?). Anyways, this seem pretty specific to (some) Infineon devices. Despite its history I would keep this separate and specifically use trxse instead of trx. Regards, Arend
On 9/1/24 8:55 AM, Arend Van Spriel wrote: Hi, >> On 8/22/24 10:37 AM, Kalle Valo wrote: >>> Marek Vasut <marex@denx.de> wrote: >>> >>>> From: Chung-Hsien Hsu <stanley.hsu@cypress.com> >>>> >>>> Add support to download TRX firmware for PCIe and SDIO. >>>> >>>> Signed-off-by: Chung-Hsien Hsu <chung-hsien.hsu@infineon.com> >>>> Signed-off-by: Marek Vasut <marex@denx.de> # Upport to current >>>> linux-next >>> >>> Please fix the review comments, also Ping's comment that from and s-o-b >>> needs to match. >> >> I have most of the changes addressed locally already. >> >> Regarding the SoB line, do I update the commit Author email (because >> that would make more sense, cypress got assimilated into infineon) or >> the SoB line email to the "old" cypress email address ? > > I would say the SoB line is leading as developer's certificate and that > has the Infineon address so that seems more appropriate. Let's add both to be on the safe side. >> I am still hoping to get a bit more input on the TRX firmware handling >> from Arend ... or maybe there is no further feedback ? > > Took me a while before I looked into this. Can not offer much info > though. Within Broadcom code bases we only have a v1 and v2 defined for > the TRX format. This patch specifies v4. The only difference seem to be > the number of offsets and their meaning. Looking at the index > definitions I suspect this version accommodates signed firmware images > which is verified by the bootloader on the device. > > Hopefully Infineon can/will confirm my suspicion and explain what SE > in .TRXSE stands for (security extension?). Anyways, this seem pretty > specific to (some) Infineon devices. Despite its history I would keep > this separate and specifically use trxse instead of trx. OK
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c index ce482a3877e90..058a742d17eda 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c @@ -42,6 +42,7 @@ #include "chip.h" #include "core.h" #include "common.h" +#include "trxhdr.h" enum brcmf_pcie_state { @@ -1684,6 +1685,8 @@ static int brcmf_pcie_download_fw_nvram(struct brcmf_pciedev_info *devinfo, u32 nvram_len) { struct brcmf_bus *bus = dev_get_drvdata(&devinfo->pdev->dev); + struct trx_header_le *trx = (struct trx_header_le *)fw->data; + u32 fw_size; u32 sharedram_addr; u32 sharedram_addr_written; u32 loop_counter; @@ -1697,8 +1700,13 @@ static int brcmf_pcie_download_fw_nvram(struct brcmf_pciedev_info *devinfo, return err; brcmf_dbg(PCIE, "Download FW %s\n", devinfo->fw_name); - memcpy_toio(devinfo->tcm + devinfo->ci->rambase, - (void *)fw->data, fw->size); + address = devinfo->ci->rambase; + fw_size = fw->size; + if (trx->magic == cpu_to_le32(TRX_MAGIC)) { + address -= sizeof(struct trx_header_le); + fw_size = le32_to_cpu(trx->len); + } + memcpy_toio(devinfo->tcm + address, (void *)fw->data, fw_size); resetintr = get_unaligned_le32(fw->data); release_firmware(fw); diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c index 1461dc453ac22..08881e366cae2 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c @@ -35,6 +35,7 @@ #include "core.h" #include "common.h" #include "bcdc.h" +#include "trxhdr.h" #define DCMD_RESP_TIMEOUT msecs_to_jiffies(2500) #define CTL_DONE_TIMEOUT msecs_to_jiffies(2500) @@ -3346,17 +3347,26 @@ brcmf_sdio_verifymemory(struct brcmf_sdio_dev *sdiodev, u32 ram_addr, static int brcmf_sdio_download_code_file(struct brcmf_sdio *bus, const struct firmware *fw) { + struct trx_header_le *trx = (struct trx_header_le *)fw->data; + u32 fw_size; + u32 address; int err; brcmf_dbg(TRACE, "Enter\n"); - err = brcmf_sdiod_ramrw(bus->sdiodev, true, bus->ci->rambase, - (u8 *)fw->data, fw->size); + address = bus->ci->rambase; + fw_size = fw->size; + if (trx->magic == cpu_to_le32(TRX_MAGIC)) { + address -= sizeof(struct trx_header_le); + fw_size = le32_to_cpu(trx->len); + } + err = brcmf_sdiod_ramrw(bus->sdiodev, true, address, + (u8 *)fw->data, fw_size); if (err) brcmf_err("error %d on writing %d membytes at 0x%08x\n", - err, (int)fw->size, bus->ci->rambase); - else if (!brcmf_sdio_verifymemory(bus->sdiodev, bus->ci->rambase, - (u8 *)fw->data, fw->size)) + err, (int)fw_size, address); + else if (!brcmf_sdio_verifymemory(bus->sdiodev, address, + (u8 *)fw->data, fw_size)) err = -EIO; return err; diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/trxhdr.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/trxhdr.h new file mode 100644 index 0000000000000..0411c7c7ffb99 --- /dev/null +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/trxhdr.h @@ -0,0 +1,38 @@ +/* SPDX-License-Identifier: ISC */ +/* Copyright (c) 2020 Cypress Semiconductor Corporation */ + +#ifndef BRCMFMAC_TRXHDR_H +#define BRCMFMAC_TRXHDR_H + +/* Bootloader makes special use of trx header "offsets" array */ +enum { + TRX_OFFSET_SIGN_INFO_IDX = 0, + TRX_OFFSET_DATA_FOR_SIGN1_IDX = 1, + TRX_OFFSET_DATA_FOR_SIGN2_IDX = 2, + TRX_OFFSET_ROOT_MODULUS_IDX = 3, + TRX_OFFSET_ROOT_EXPONENT_IDX = 67, + TRX_OFFSET_CONT_MODULUS_IDX = 68, + TRX_OFFSET_CONT_EXPONENT_IDX = 132, + TRX_OFFSET_HASH_FW_IDX = 133, + TRX_OFFSET_FW_LEN_IDX = 149, + TRX_OFFSET_TR_RST_IDX = 150, + TRX_OFFSET_FW_VER_FOR_ANTIROOLBACK_IDX = 151, + TRX_OFFSET_IV_IDX = 152, + TRX_OFFSET_NONCE_IDX = 160, + TRX_OFFSET_SIGN_INFO2_IDX = 168, + TRX_OFFSET_MAX_IDX +}; + +#define TRX_MAGIC 0x30524448 /* "HDR0" */ +#define TRX_VERSION 4 /* Version 4 */ +#define TRX_MAX_OFFSET TRX_OFFSET_MAX_IDX /* Max number of file offsets */ + +struct trx_header_le { + __le32 magic; /* "HDR0" */ + __le32 len; /* Length of file including header */ + __le32 crc32; /* CRC from flag_version to end of file */ + __le32 flag_version; /* 0:15 flags, 16:31 version */ + __le32 offsets[TRX_MAX_OFFSET]; /* Offsets of partitions */ +}; + +#endif /* BRCMFMAC_TRXHDR_H */