On Mon, Sep 20, 2021 at 02:04:07PM -0700, Jakub Kicinski wrote:

On Mon, 20 Sep 2021 13:39:15 -0700 Jakub Kicinski wrote:

quoted

On Mon, 20 Sep 2021 17:41:44 +0300 Leon Romanovsky wrote:

quoted

From: Leon Romanovsky <leonro@nvidia.com>

devlink_register() can't fail and always returns success, but all drivers
are obligated to check returned status anyway. This adds a lot of boilerplate
code to handle impossible flow.

Make devlink_register() void and simplify the drivers that use that
API call.  

Unlike unused functions bringing back error handling may be
non-trivial. I'd rather you deferred such cleanups until you're 
ready to post your full rework and therefore give us some confidence 
the revert will not be needed.

If you disagree you gotta repost, new devlink_register call got added
in the meantime.

This is exactly what I afraid, new devlink API users are added faster
than I can cleanup them.

For example, let's take a look on newly added ipc_devlink_init(), it is
called conditionally "if (stage == IPC_MEM_EXEC_STAGE_BOOT) {". How can
it be different stage if we are in driver .probe() routine?

They also introduced devlink_sio.devlink_read_pend and
devlink_sio.read_sem to protect from something that right position of
devlink_register() will fix. I also have serious doubts that their
current protection is correct, once they called to devlink_params_publish()
the user can crash the system, because he can access the parameters before
they initialized their protection.

So yes, I disagree. We will need to make sure that devlink_register()
can't fail and it will make life easier for everyone (no need to unwind)
while we put that command  being last in probe sequence.

If I repost, will you take it? I don't want to waste anyone time if it
is not.

Thanks