All of lore.kernel.org
 help / color / mirror / Atom feed
From: Leon Romanovsky <leon@kernel.org>
To: Jakub Kicinski <kuba@kernel.org>
Cc: "David S . Miller" <davem@davemloft.net>,
	Amit Cohen <amcohen@nvidia.com>, Jiri Pirko <jiri@nvidia.com>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH net] devlink: Remove extra assertion from flash notification logic
Date: Wed, 17 Nov 2021 16:38:58 +0200	[thread overview]
Message-ID: <YZUUAszdQ8jiZE10@unreal> (raw)
In-Reply-To: <20211117060228.65947629@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

On Wed, Nov 17, 2021 at 06:02:28AM -0800, Jakub Kicinski wrote:
> On Wed, 17 Nov 2021 15:23:18 +0200 Leon Romanovsky wrote:
> > > I'd drop these notifications, the user didn't ask to flash the device,
> > > it's just code reuse in the driver, right?  
> > 
> > Sorry, I missed your reply.
> > 
> > I'm not sure about code reuse, from the code, it looks like attempt to
> > burn FW during mlxsw register.
> > 
> > __mlxsw_core_bus_device_register
> >  -> mlxsw_core_fw_rev_validate
> >   -> mlxsw_core_fw_flash
> >    -> mlxfw_firmware_flash
> >     -> mlxfw_status_notify
> >      -> devlink_flash_update_status_notify
> >       -> __devlink_flash_update_notify
> >        -> WARN_ON(...)  
> > 
> > The mlxfw_firmware_flash() routine is called by mlx5 too, so I can't
> > remove mlxfw_status_notify() calls without too much changes.
> > 
> > Easiest solution was to remove WARN_ON(), because no one really
> > interested in these events anyway. I searched in github and didn't
> > find any user who listened to them.
> 
> Drop in the core. Like this?

Of course, sorry for overlooking of such simple solution.
I'm sending new version right now.

Thanks

> 
> 
>  	WARN_ON(cmd != DEVLINK_CMD_FLASH_UPDATE &&
>  		cmd != DEVLINK_CMD_FLASH_UPDATE_END &&
>  		cmd != DEVLINK_CMD_FLASH_UPDATE_STATUS);
> -	WARN_ON(!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED));
> +
> +	if (!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED))
> +		return;
>  
>  	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>  	if (!msg)

      reply	other threads:[~2021-11-17 14:39 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-15 18:07 [PATCH net] devlink: Remove extra assertion from flash notification logic Leon Romanovsky
2021-11-15 18:14 ` Jakub Kicinski
2021-11-15 18:27   ` Leon Romanovsky
2021-11-16  1:15     ` Jakub Kicinski
2021-11-17 13:23       ` Leon Romanovsky
2021-11-17 14:02         ` Jakub Kicinski
2021-11-17 14:38           ` Leon Romanovsky [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YZUUAszdQ8jiZE10@unreal \
    --to=leon@kernel.org \
    --cc=amcohen@nvidia.com \
    --cc=davem@davemloft.net \
    --cc=jiri@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.