Re: [RFC net-next 1/4] net: marvell: prestera: try to load previous fw version

[Vadym Kochan <vadym.kochan@plvision.eu>]

On Thu, May 20, 2021 at 03:12:02AM +0200, Andrew Lunn wrote:
> > +static int prestera_fw_get(struct prestera_fw *fw)
> > +{
> > +	int ver_maj = PRESTERA_SUPP_FW_MAJ_VER;
> > +	int ver_min = PRESTERA_SUPP_FW_MIN_VER;
> > +	char fw_path[128];
> > +	int err;
> > +
> > +pick_fw_ver:
> > +	snprintf(fw_path, sizeof(fw_path), PRESTERA_FW_PATH_FMT,
> > +		 ver_maj, ver_min);
> > +
> > +	err = request_firmware_direct(&fw->bin, fw_path, fw->dev.dev);
> > +	if (err) {
> > +		if (ver_maj == PRESTERA_SUPP_FW_MAJ_VER) {
> > +			ver_maj--;
> > +			goto pick_fw_ver;
> > +		} else {
> > +			dev_err(fw->dev.dev, "failed to request firmware file\n");
> > +			return err;
> > +		}
> > +	}
> 
> So lets say for the sake of the argument, you have version 3.0 and
> 2.42. It looks like this code will first try to load version 3.0. If
> that fails, ver_maj is decremented, so it tries 2.0, which also fails
> because it should be looking for 2.42. I don't think this decrement is
> a good idea.

Ahh, you are correct, I was too focused on a major version. So the only
option which I see is to hard-code also the previous version.

> 
> Also:
> 
> > +			dev_err(fw->dev.dev, "failed to request firmware file\n");
> 
> It would be useful to say what version you are actually looking for,
> so the user can go find the correct firmware.

I agree.

> 
>    Andrew

Thanks,