Thread (19 messages) flat view 19 messages, 4 authors, 2017-09-06

Re: [PATCH 1/2] net/mlx5: support device removal event

From: Nélio Laranjeiro <hidden>
Date: 2017-08-25 08:29:28

On Thu, Aug 24, 2017 at 02:33:43PM +0000, Matan Azrad wrote:
Hi Nelio
[...] 
quoted
I am expecting to find something related to a link update, what I see is an
alarm update.  I don't expect to update an alarm but a link.  The names and
action are inconsistent i.e. mlx5_dev_link_status_handler() should handle a
link not an alarm.

I understand there is a need to add more function levels, but the
priv_link_status_alarm_update() should be renamed to something like
priv_link_status_update().
OK, I think I understand you.

Because the alarm is a workaround you don't think it should be mentioned
in function description or function name.
(also the function subject should be the link status and not the alarm)
I can agree with you about it.
And I will create v2 with your suggestion - priv_link_status_update.
Thanks,
The return value description can stay as in old code semantic:
Zero if the callback process can be called immediately.

Are you agree?
Yes.
Maybe we can tell something about the alarm and inconsistent reason
In this function description or internal comment for future code review.
If you want it, please suggest comment.
Yes the comment can added internally.

Thanks,

-- 
Nélio Laranjeiro
6WIND
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help