On 13.12.2024 8:05 AM, Cheng Jiang (IOE) wrote: [...] >> /* >> * If the board-specific file is missing, try loading the default >> * one, unless that was attempted already >> */ >> >> But, even more importantly: >> >> a) do we want to load the "incorrect" file? > Normally, there is a default NVM file ending with .bin, which is suitable > for most boards. However, some boards have different configurations that > require a specific NVM. If a board-specific NVM is not found, a default > NVM is preferred over not loading any NVM. So, if one is specified, but not found, this should either be a loud error, or a very loud warning & fallback. Otherwise, the device may provide subpar user experience without the user getting a chance to know the reason. I think failing is better here, as that sends a clearer message, and would only happen if the DT has a specific path (meaning the user put some intentions behind that choice) >> b) why would we want to specify the .bin file if it's the default anyway? > The default NVM directory is the root of qca. The 'firmware-name' property > can specify an NVM file in another directory. This can be either a default > NVM like 'QCA6698/hpnv21.bin' or a board-specific NVM like 'QCA6698/hpnv21.b205'. Do we expect QCA6698/hpnv21.bin and QCAabcd/hpnv21.bin to be compatible? [...] >>> -static inline void qca_get_nvm_name_generic(struct qca_fw_config *cfg, >>> - const char *stem, u8 rom_ver, u16 bid) >>> -{ >>> - if (bid == 0x0) >>> - snprintf(cfg->fwname, sizeof(cfg->fwname), "qca/%snv%02x.bin", stem, rom_ver); >>> - else if (bid & 0xff00) >>> - snprintf(cfg->fwname, sizeof(cfg->fwname), >>> - "qca/%snv%02x.b%x", stem, rom_ver, bid); >>> - else >>> - snprintf(cfg->fwname, sizeof(cfg->fwname), >>> - "qca/%snv%02x.b%02x", stem, rom_ver, bid); >>> + if (soc_type == QCA_WCN6855 || soc_type == QCA_QCA2066) { >>> + /* hsp gf chip */ >> >> This is a good opportunity to explain what that means >> > Ack. This means the chip is produced by GlobalFoundries. Please update the comment there Konrad