On Mon, Jan 13, 2025 at 11:15:13PM +0800, Zijun Hu wrote:
> On 2025/1/13 23:04, Johan Hovold wrote:
> >> case QCA_WCN6855:
> >> - snprintf(config.fwname, sizeof(config.fwname),
> >> - "qca/hpnv%02x.bin", rom_ver);
> >> + qca_read_fw_board_id(hdev, &boardid);
> > For consistency, this should probably have been handled by amending the
> > conditional above the switch:
> >
> > if (soc_type == QCA_QCA2066 || soc_type == QCA_WCN7850)
> > qca_read_fw_board_id(hdev, &boardid);
> qca_read_fw_board_id() may be invoked twice if adding reading board ID here
>
> see below branch:
>
> config.type = TLV_TYPE_NVM;
> if (firmware_name) {
> /* The firmware name has an extension, use it directly */
> if (qca_filename_has_extension(firmware_name)) {
> snprintf(config.fwname, sizeof(config.fwname), "qca/%s", firmware_name);
> } else {
> *qca_read_fw_board_id(hdev, &boardid);*
> qca_get_nvm_name_by_board(config.fwname, sizeof(config.fwname),
> firmware_name, soc_type, ver, 0, boardid);
> }
Fair enough (but I guess that would be the case also for
QCA2066/WCN7850 currently).
But either way, this is not something that should block this fix.
> > but long term that should probably be moved into
> > qca_get_nvm_name_by_board() to avoid sprinkling conditionals all over
> > the driver.
> >
> > I'm fine with this as a stop gap unless you want to move the call to the
> > QCA2066/WCN7850 conditional:
> >
> > Reviewed-by: Johan Hovold <johan+linaro@xxxxxxxxxx>
Johan
