Re: [PATCH net-next v2 2/6] devlink: Adding SR-IOV enablement NVRAM config param

[Steve Lin]

On Thu, Oct 19, 2017 at 3:33 PM, Jiri Pirko  wrote:
> Thu, Oct 19, 2017 at 09:17:06PM CEST, steven.l...@broadcom.com wrote:
>>Adding DEVLINK_PERM_CONFIG_SRIOV_ENABLED permanent config
>>parameter.  If value is 1, SR-IOV is enabled.  If value is 0,
>>SR-IOV is disabled on this device.  Value is permanent (stored
>>in NVRAM), so becomes the new default value for this device.
>>
>>Signed-off-by: Steve Lin 
>>Acked-by: Andy Gospodarek 
>>---
>> include/uapi/linux/devlink.h | 5 +
>> 1 file changed, 5 insertions(+)
>>
>>diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>>index 47cc584..2640203 100644
>>--- a/include/uapi/linux/devlink.h
>>+++ b/include/uapi/linux/devlink.h
>>@@ -255,4 +255,9 @@ enum devlink_dpipe_header_id {
>>   DEVLINK_DPIPE_HEADER_IPV6,
>> };
>>
>>+/* Permanent (NVRAM) config parameters */
>
> We need the decription here in the header as well. Commit message alone
> is no good for this.
>
> Also, there should not be mention of "NVRAM". It is up to the device
> implementation where is stores the value.
>
>

Will do, I'll add descriptions in the header file and reword as
requested.  Thanks!

Re: [PATCH net-next v2 2/6] devlink: Adding SR-IOV enablement NVRAM config param

[Jiri Pirko]

Thu, Oct 19, 2017 at 09:17:06PM CEST, steven.l...@broadcom.com wrote:
>Adding DEVLINK_PERM_CONFIG_SRIOV_ENABLED permanent config
>parameter.  If value is 1, SR-IOV is enabled.  If value is 0,
>SR-IOV is disabled on this device.  Value is permanent (stored
>in NVRAM), so becomes the new default value for this device.
>
>Signed-off-by: Steve Lin 
>Acked-by: Andy Gospodarek 
>---
> include/uapi/linux/devlink.h | 5 +
> 1 file changed, 5 insertions(+)
>
>diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>index 47cc584..2640203 100644
>--- a/include/uapi/linux/devlink.h
>+++ b/include/uapi/linux/devlink.h
>@@ -255,4 +255,9 @@ enum devlink_dpipe_header_id {
>   DEVLINK_DPIPE_HEADER_IPV6,
> };
> 
>+/* Permanent (NVRAM) config parameters */

We need the decription here in the header as well. Commit message alone
is no good for this.

Also, there should not be mention of "NVRAM". It is up to the device
implementation where is stores the value.


>+enum devlink_perm_config_param {
>+  DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
>+};
>+
> #endif /* _UAPI_LINUX_DEVLINK_H_ */
>-- 
>2.7.4
>