[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190215101514.GD2343@nanopsycho>
Date: Fri, 15 Feb 2019 11:15:14 +0100
From: Jiri Pirko <jiri@...nulli.us>
To: Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc: davem@...emloft.net, netdev@...r.kernel.org,
oss-drivers@...ronome.com, mkubecek@...e.cz, andrew@...n.ch
Subject: Re: [PATCH net-next 3/3] nfp: devlink: allow flashing the device via
devlink
Thu, Feb 14, 2019 at 10:40:46PM CET, jakub.kicinski@...ronome.com wrote:
>Devlink now allows updating device flash. Implement this
>callback.
>
>Compared to ethtool update we no longer have to release
>the networking locks - devlink doesn't take them.
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
>---
> .../net/ethernet/netronome/nfp/nfp_devlink.c | 10 +++++
> drivers/net/ethernet/netronome/nfp/nfp_main.c | 41 +++++++++++++++++++
> drivers/net/ethernet/netronome/nfp/nfp_main.h | 2 +
> .../ethernet/netronome/nfp/nfp_net_ethtool.c | 35 ++--------------
> 4 files changed, 56 insertions(+), 32 deletions(-)
>
>diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
>index 080a301f379e..db2da99f6aa7 100644
>--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
>+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
>@@ -330,6 +330,15 @@ nfp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
> return err;
> }
>
>+static int
>+nfp_devlink_flash_update(struct devlink *devlink, const char *path,
devlink code calles "path" a "file_name". It is good to be consistent in
this.
Other than this, the patch looks fine:
>+ const char *component, struct netlink_ext_ack *extack)
>+{
>+ if (component)
>+ return -EOPNOTSUPP;
>+ return nfp_flash_update_common(devlink_priv(devlink), path, extack);
>+}
>+
> const struct devlink_ops nfp_devlink_ops = {
> .port_split = nfp_devlink_port_split,
> .port_unsplit = nfp_devlink_port_unsplit,
>@@ -338,6 +347,7 @@ const struct devlink_ops nfp_devlink_ops = {
> .eswitch_mode_get = nfp_devlink_eswitch_mode_get,
> .eswitch_mode_set = nfp_devlink_eswitch_mode_set,
> .info_get = nfp_devlink_info_get,
>+ .flash_update = nfp_devlink_flash_update,
> };
>
> int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port)
>diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.c b/drivers/net/ethernet/netronome/nfp/nfp_main.c
>index 6c10e8d119e4..f4c8776e42b6 100644
>--- a/drivers/net/ethernet/netronome/nfp/nfp_main.c
>+++ b/drivers/net/ethernet/netronome/nfp/nfp_main.c
>@@ -300,6 +300,47 @@ static int nfp_pcie_sriov_configure(struct pci_dev *pdev, int num_vfs)
> return nfp_pcie_sriov_enable(pdev, num_vfs);
> }
>
>+int nfp_flash_update_common(struct nfp_pf *pf, const char *path,
>+ struct netlink_ext_ack *extack)
>+{
>+ struct device *dev = &pf->pdev->dev;
>+ const struct firmware *fw;
>+ struct nfp_nsp *nsp;
>+ int err;
>+
>+ nsp = nfp_nsp_open(pf->cpp);
>+ if (IS_ERR(nsp)) {
>+ err = PTR_ERR(nsp);
>+ if (extack)
>+ NL_SET_ERR_MSG_MOD(extack, "can't access NSP");
>+ else
>+ dev_err(dev, "Failed to access the NSP: %d\n", err);
>+ return err;
>+ }
>+
>+ err = request_firmware_direct(&fw, path, dev);
>+ if (err) {
>+ NL_SET_ERR_MSG_MOD(extack,
>+ "unable to read flash file from disk");
>+ goto exit_close_nsp;
>+ }
>+
>+ dev_info(dev, "Please be patient while writing flash image: %s\n",
>+ path);
>+
>+ err = nfp_nsp_write_flash(nsp, fw);
>+ if (err < 0)
>+ goto exit_release_fw;
>+ dev_info(dev, "Finished writing flash image\n");
>+ err = 0;
>+
>+exit_release_fw:
>+ release_firmware(fw);
>+exit_close_nsp:
>+ nfp_nsp_close(nsp);
>+ return err;
>+}
>+
> static const struct firmware *
> nfp_net_fw_request(struct pci_dev *pdev, struct nfp_pf *pf, const char *name)
> {
>diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.h b/drivers/net/ethernet/netronome/nfp/nfp_main.h
>index a3613a2e0aa5..b7211f200d22 100644
>--- a/drivers/net/ethernet/netronome/nfp/nfp_main.h
>+++ b/drivers/net/ethernet/netronome/nfp/nfp_main.h
>@@ -164,6 +164,8 @@ nfp_pf_map_rtsym(struct nfp_pf *pf, const char *name, const char *sym_fmt,
> unsigned int min_size, struct nfp_cpp_area **area);
> int nfp_mbox_cmd(struct nfp_pf *pf, u32 cmd, void *in_data, u64 in_length,
> void *out_data, u64 out_length);
>+int nfp_flash_update_common(struct nfp_pf *pf, const char *path,
>+ struct netlink_ext_ack *extack);
>
> enum nfp_dump_diag {
> NFP_DUMP_NSP_DIAG = 0,
>diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
>index cb9c512abc76..8f189149efc5 100644
>--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
>+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
>@@ -1237,11 +1237,8 @@ static int nfp_net_set_channels(struct net_device *netdev,
> static int
> nfp_net_flash_device(struct net_device *netdev, struct ethtool_flash *flash)
> {
>- const struct firmware *fw;
> struct nfp_app *app;
>- struct nfp_nsp *nsp;
>- struct device *dev;
>- int err;
>+ int ret;
>
> if (flash->region != ETHTOOL_FLASH_ALL_REGIONS)
> return -EOPNOTSUPP;
>@@ -1250,39 +1247,13 @@ nfp_net_flash_device(struct net_device *netdev, struct ethtool_flash *flash)
> if (!app)
> return -EOPNOTSUPP;
>
>- dev = &app->pdev->dev;
>-
>- nsp = nfp_nsp_open(app->cpp);
>- if (IS_ERR(nsp)) {
>- err = PTR_ERR(nsp);
>- dev_err(dev, "Failed to access the NSP: %d\n", err);
>- return err;
>- }
>-
>- err = request_firmware_direct(&fw, flash->data, dev);
>- if (err)
>- goto exit_close_nsp;
>-
>- dev_info(dev, "Please be patient while writing flash image: %s\n",
>- flash->data);
> dev_hold(netdev);
> rtnl_unlock();
>-
>- err = nfp_nsp_write_flash(nsp, fw);
>- if (err < 0) {
>- dev_err(dev, "Flash write failed: %d\n", err);
>- goto exit_rtnl_lock;
>- }
>- dev_info(dev, "Finished writing flash image\n");
>-
>-exit_rtnl_lock:
>+ ret = nfp_flash_update_common(app->pf, flash->data, NULL);
> rtnl_lock();
> dev_put(netdev);
>- release_firmware(fw);
>
>-exit_close_nsp:
>- nfp_nsp_close(nsp);
>- return err;
>+ return ret;
> }
>
> static const struct ethtool_ops nfp_net_ethtool_ops = {
Why don't you use the compat fallback? I think you should.
>--
>2.19.2
>
Powered by blists - more mailing lists