From: Vadym Kochan <[email protected]>
Lets try to load previous fw version in case the latest one is missing on
existing system.
Signed-off-by: Vadym Kochan <[email protected]>
---
.../ethernet/marvell/prestera/prestera_pci.c | 75 ++++++++++++++-----
1 file changed, 56 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_pci.c b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
index 298110119272..d384dcacd579 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_pci.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
@@ -166,6 +166,8 @@ struct prestera_fw_evtq {
};
struct prestera_fw {
+ struct prestera_fw_rev rev_supp;
+ const struct firmware *bin;
struct workqueue_struct *wq;
struct prestera_device dev;
u8 __iomem *ldr_regs;
@@ -576,25 +578,24 @@ static void prestera_fw_rev_parse(const struct prestera_fw_header *hdr,
static int prestera_fw_rev_check(struct prestera_fw *fw)
{
struct prestera_fw_rev *rev = &fw->dev.fw_rev;
- u16 maj_supp = PRESTERA_SUPP_FW_MAJ_VER;
- u16 min_supp = PRESTERA_SUPP_FW_MIN_VER;
- if (rev->maj == maj_supp && rev->min >= min_supp)
+ if (rev->maj == fw->rev_supp.maj && rev->min >= fw->rev_supp.min)
return 0;
dev_err(fw->dev.dev, "Driver supports FW version only '%u.%u.x'",
- PRESTERA_SUPP_FW_MAJ_VER, PRESTERA_SUPP_FW_MIN_VER);
+ fw->rev_supp.maj, fw->rev_supp.min);
return -EINVAL;
}
-static int prestera_fw_hdr_parse(struct prestera_fw *fw,
- const struct firmware *img)
+static int prestera_fw_hdr_parse(struct prestera_fw *fw)
{
- struct prestera_fw_header *hdr = (struct prestera_fw_header *)img->data;
struct prestera_fw_rev *rev = &fw->dev.fw_rev;
+ struct prestera_fw_header *hdr;
u32 magic;
+ hdr = (struct prestera_fw_header *)fw->bin->data;
+
magic = be32_to_cpu(hdr->magic_number);
if (magic != PRESTERA_FW_HDR_MAGIC) {
dev_err(fw->dev.dev, "FW img hdr magic is invalid");
@@ -609,11 +610,51 @@ static int prestera_fw_hdr_parse(struct prestera_fw *fw,
return prestera_fw_rev_check(fw);
}
+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;
+ }
+ }
+
+ if (ver_maj < PRESTERA_SUPP_FW_MAJ_VER)
+ dev_warn(fw->dev.dev,
+ "using older FW version %u.%u than expected %u.%u\n",
+ ver_maj, ver_min,
+ PRESTERA_SUPP_FW_MAJ_VER, PRESTERA_SUPP_FW_MIN_VER);
+
+ dev_info(fw->dev.dev, "Loading %s ...", fw_path);
+
+ fw->rev_supp.maj = ver_maj;
+ fw->rev_supp.min = ver_min;
+ fw->rev_supp.sub = 0;
+
+ return 0;
+}
+
+static void prestera_fw_put(struct prestera_fw *fw)
+{
+ release_firmware(fw->bin);
+}
+
static int prestera_fw_load(struct prestera_fw *fw)
{
size_t hlen = sizeof(struct prestera_fw_header);
- const struct firmware *f;
- char fw_path[128];
int err;
err = prestera_ldr_wait_reg32(fw, PRESTERA_LDR_READY_REG,
@@ -632,30 +673,26 @@ static int prestera_fw_load(struct prestera_fw *fw)
fw->ldr_wr_idx = 0;
- snprintf(fw_path, sizeof(fw_path), PRESTERA_FW_PATH_FMT,
- PRESTERA_SUPP_FW_MAJ_VER, PRESTERA_SUPP_FW_MIN_VER);
-
- err = request_firmware_direct(&f, fw_path, fw->dev.dev);
+ err = prestera_fw_get(fw);
if (err) {
dev_err(fw->dev.dev, "failed to request firmware file\n");
return err;
}
- err = prestera_fw_hdr_parse(fw, f);
+ err = prestera_fw_hdr_parse(fw);
if (err) {
dev_err(fw->dev.dev, "FW image header is invalid\n");
goto out_release;
}
- prestera_ldr_write(fw, PRESTERA_LDR_IMG_SIZE_REG, f->size - hlen);
+ prestera_ldr_write(fw, PRESTERA_LDR_IMG_SIZE_REG, fw->bin->size - hlen);
prestera_ldr_write(fw, PRESTERA_LDR_CTL_REG, PRESTERA_LDR_CTL_DL_START);
- dev_info(fw->dev.dev, "Loading %s ...", fw_path);
-
- err = prestera_ldr_fw_send(fw, f->data + hlen, f->size - hlen);
+ err = prestera_ldr_fw_send(fw, fw->bin->data + hlen,
+ fw->bin->size - hlen);
out_release:
- release_firmware(f);
+ prestera_fw_put(fw);
return err;
}
--
2.17.1
> +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.
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.
Andrew
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,