Wed, Feb 24, 2016 at 08:02:32AM CET, yuval.mi...@qlogic.com wrote:
>> + * An overall lock guarding every operation comming from userspace.
>> + * If also guards devlink devices list and it is taken when
>> + * driver registers/unregisters it.
>Several typos in comment.
Already fixed in v2.
>
>> +static void devlink_notify(struct devlink *devlink, enum
>> +devlink_command cmd) {
>...
>> + WARN_ON(cmd != DEVLINK_CMD_NEW && cmd !=
>> DEVLINK_CMD_DEL);
>Given this should never happen, shouldn't this be ONCE?
Given this should never happen and only warns the developer fiddling
with this, I don't think it matters.
>
>> +static void devlink_port_notify(struct devlink_port *devlink_port,
>...
>> + WARN_ON(cmd != DEVLINK_CMD_PORT_NEW && cmd !=
>> DEVLINK_CMD_PORT_DEL);
>Likewise
>
>> +static void __devlink_port_type_set(struct devlink_port *devlink_port,
>...
>> + devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW); }
>Why is this PORT_NEW? Shouldn't it be PORT_SET?
>Also, curly bracers are repeatedly on last line of function [if this file].
>Is this by design?
SET is only for userspace->kernel messages. NEW is for reporting events
back. This is consistend with rest of the netlink messages out there,
including rtnl and nl80211
