Re: [dpdk-dev] [PATCH v2 06/12] net/bnxt: update HWRM structures

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 3/12/2021 12:17 AM, Ajit Khaparde wrote:
> On Thu, Mar 11, 2021 at 4:08 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> On 3/10/2021 9:26 PM, Ajit Khaparde wrote:
>>> From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
>>>
>>> Patch brings in the latest hsi_struct_def_dpdk.h.
>>> HWRM API is now updated to version 1.10.2.15.
>>>
>>> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
>>> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>> ---
>>>    doc/guides/rel_notes/release_21_05.rst |    4 +
>>>    drivers/net/bnxt/hsi_struct_def_dpdk.h | 3689 +++++++++++++++++++-----
>>>    2 files changed, 2972 insertions(+), 721 deletions(-)
>>>
>>> diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
>>> index 9fc5e0d6ca..d5cdb2cd2c 100644
>>> --- a/doc/guides/rel_notes/release_21_05.rst
>>> +++ b/doc/guides/rel_notes/release_21_05.rst
>>> @@ -80,6 +80,10 @@ New Features
>>>
>>>      * Added support for preferred busy polling.
>>>
>>> +* **Updated Broadcom bnxt driver.**
>>> +
>>> +  * Updated HWRM structures to 1.10.2.15 version.
>>> +
>>
>> Can you please chage the order in the release notes, the order is:
>> ethdev library, pysical PMDs (alphabetical order for vendor), vitual PMDs, apps
>> (like testpms)
>> Acording this order, in current release notes the PMD should be first item in
>> the section.
> Ok. Will do.
> 
>>
>>> @@ -45297,8 +47545,8 @@ struct hwrm_nvm_mod_dir_entry_input {
>>>         */
>>>        uint16_t        dir_ext;
>>>        /*
>>> -      * Directory Entry Attribute flags (see BNX_DIR_ATTR_* for attribute flag
>>> -      * definitions).
>>> +      * Directory Entry Attribute flags (see BNX_DIR_ATTR_* for attribute
>>> +      * flag definitions).
>>>         */
>>>        uint16_t        dir_attr;
>>>        /*
>>> @@ -45474,8 +47722,7 @@ struct hwrm_nvm_install_update_input {
>>>        #define HWRM_NVM_INSTALL_UPDATE_INPUT_FLAGS_REMOVE_UNUSED_PKG \
>>>                UINT32_C(0x2)
>>>        /*
>>> -      * If set to 1, FW will defragment the NVM if defragmentation is required
>>> -      * for the update.
>>> +      * If set to 1, FW will defragment the NVM if defragmentation is required for the update.
>>>         * Allow additional time for this command to complete if this bit is set to 1.
>>>         */
>>>        #define HWRM_NVM_INSTALL_UPDATE_INPUT_FLAGS_ALLOWED_TO_DEFRAG \
>>>
>>
>> These changes are only because of word wrapping, there are many samples in the
>> patch.
>> I guess the original code has long lines, as the first version, and when they
>> are converted to shorter lines, it has been done differently from previous time
>> causing these diffs.
>> Can you keep the previous version wrapping to prevent the unnecesarry diffs?
> Ideally I wanted to get this from the auto-generator script.

Agree that is better for long term.

> But that was going to take time, so I edited the file manually.
> And yes, I modified a few extra lines which checkpatch was not
> complaining about.

I think we can tolerate a few checkpatch warnings for base code changes if the 
line length is reasonable and if it prevents modifying exiting line.
First version was converting wrapped lines back to long lines and they were 
really long, so that was hard to tolerate.

> I will send another version to minimize the changes.
> 

Thank you.