NAK: [SRU][F:linux-bluefield][Pull Request] Cherry-pick the upstreamed mlxbf-gige driver

[Asmaa Mnebhi]

Hi Tim,

PSB

-----Original Message-----
From: Tim Gardner <tim.gardner at canonical.com> 
Sent: Wednesday, July 14, 2021 8:55 AM
To: Asmaa Mnebhi <asmaa at nvidia.com>; kernel-team at lists.ubuntu.com
Cc: David Thompson <davthompson at nvidia.com>
Subject: NAK: [SRU][F:linux-bluefield][Pull Request] Cherry-pick the upstreamed mlxbf-gige driver

You're missing BugLink and S-o-b in all the patches. I also have some inline comments about how you've broken up the last 4 patches.

Asmaa>> what does S-o-b stand for? 

Next pull request should be noted with a version in the subject, e.g., '[SRU v3][F:linux-bluefield][Pull Request] Cherry-pick the upstreamed mlxbf-gige driver' so we don't spend time on older submissions. Its also an aid to reviewers in the pull request header to describe the differences between each version.

On 7/13/21 3:58 PM, Asmaa Mnebhi wrote:
> This pull request (26 commits) includes reverting all mlxbf-gige changes in linux-bluefield (21 commits).
> Then it cherry-picks the newly upstreamed version of the mlxbf-gige driver (1 commit).
> It also adds:
> * 1 commit which adds the mlxbf-gige driver version
> * 1 commit which adds the ethtool mlxbf_gige_set_ringparam function
> * 1 commit to restore the gpio-mlxbf2.c driver to its previous state before getting reverted by the 21 commits mentioned above.
> * 1 commit which removes the dependency between the gpio-mlxbf2.c driver and mlxbf-gige driver.
> 
> BugLink: 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs
> .launchpad.net%2Fbugs%2F1934923&data=04%7C01%7Casmaa%40nvidia.com%
> 7C8927e0cad5a74a5c20f408d946c69f39%7C43083d15727340c1b7db39efd9ccc17a%
> 7C0%7C0%7C637618641154180200%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM
> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=wFB
> rdDbDKvenTwg728s86qGQVoFVq1Ju18%2B3eRpHEYY%3D&reserved=0
> 
> The pull request page can be found at:
> 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcode
> .launchpad.net%2F~asmaam%2Fubuntu%2F%2Bsource%2Flinux-bluefield%2F%2Bg
> it%2Fversion-seeds%2F%2Bmerge%2F405646&data=04%7C01%7Casmaa%40nvid
> ia.com%7C8927e0cad5a74a5c20f408d946c69f39%7C43083d15727340c1b7db39efd9
> ccc17a%7C0%7C0%7C637618641154180200%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC
> 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sd
> ata=ZwxRHJKRc%2BvJLncnTPPZSKYV9%2Fcxy2RbVl7VKjby8po%3D&reserved=0
> 
> Git repo can be found at:
> 
> git://git.launchpad.net/~asmaam/ubuntu/+source/linux-bluefield 
> master-next
> 
> for you to fetch changes starting from commit 
> cfabdc1ef2b73553a7ecc5908902cbaf290f4f4b
> 
>      Revert "UBUNTU: SAUCE: mlxbf_gige: syncup with v1.23 content"
> 
> Signed-off-by: David Thompson <davthompson at nvidia.com>
> Signed-off-by: Asmaa Mnebhi <asmaa at nvidia.com>
> 
> Asmaa Mnebhi (25):
>    Revert "UBUNTU: SAUCE: mlxbf_gige: syncup with v1.23 content"
>    Revert "UBUNTU: SAUCE: mlxbf_gige: syncup with v1.21 content"
>    Revert "UBUNTU: SAUCE: Automate soft reset of BlueField ARM via GPIO7"
>    Revert "UBUNTU: SAUCE: Syncup with the latest gpio-mlxbf2 and
>      mlxbf-gige drivers"
>    Revert "Revert "UBUNTU: SAUCE: Fix intermittent OOB link up issue""
>    Revert "UBUNTU: SAUCE: Fix intermittent OOB link up issue"
>    Revert "UBUNTU: SAUCE: mlxbf_gige_mdio.c: Support PHY interrupt on
>      Bluesphere"
>    Revert "UBUNTU: SAUCE: mlxbf_gige_main.c: Fix OOB PHY interrupt"
>    Revert "UBUNTU: SAUCE: mlxbf_gige: use streaming DMA mapping for
>      packet buffers"
>    Revert "UBUNTU: SAUCE: mlxbf_gige: address upstream comments on RX and
>      TX"
>    Revert "UBUNTU: SAUCE: mlxbf-gige: remove gpio interrupt coalesce
>      resources"
>    Revert "UBUNTU: SAUCE: mlxbf_gige: add support for ndo_get_stats64"
>    Revert "UBUNTU: SAUCE: mlxbf_gige: address some general upstream
>      comments"
>    Revert "UBUNTU: SAUCE: Address upstream comments from patch v6 for PHY
>      driver"
>    Revert "UBUNTU: SAUCE: mlxbf-gige: cleanups from review"
>    Revert "UBUNTU: SAUCE: mlxbf-gige: v5 patch cleanup and napi_schedule"
>    Revert "UBUNTU: SAUCE: mlxbf_gige_main.c: Fix OOB's ethtool command"
>    Revert "UBUNTU: SAUCE: mlxbf_gige_main.c: Support ethtool options"
>    Revert "UBUNTU: SAUCE: mlxbf-gige: multiple fixes for stability"
>    Revert "UBUNTU: SAUCE: Remove built-in tests from mlxbf_gige driver"
>    Revert "UBUNTU: SAUCE: Add Mellanox BlueField Gigabit Ethernet driver"
>    UBUNTU: SAUCE: mlxbf-gige: add driver version

>    UBUNTU: SAUCE: mlxbf-gige: add ethtool mlxbf_gige_set_ringparam
>    UBUNTU: SAUCE: Restore gpio-mlxbf2.c driver from before reverting 21
>      commits.

Unless you are intimately familiar with the content and behavior of gpio-mlxbf2.c prior to this patch, the commit message is meaningless. 
There is all kinda stuff going on in this patch that needs description. 
While doing that I think you'll find that you can further break this commit into simpler sequential logical blocks.


>    UBUNTU: SAUCE: gpio-mlxbf2.c: remove dependencies with mlxbf-gige

This commit is somewhat under described. How does this commit remove dependencies with mlxbf-gige ? It may be obvious to you now, but its not to me, nor will it be to you in 6 or 12 months.

Also why are you changing DRV_VERSION in this commit ? It seems unrelated.

Asmaa>> every time we bring an important change to gpio-mlxbf2.c , we change the DRV_VERSION just for our personal tracking during testing so we needed to change this.

> 
> David Thompson (1):
>    Add Mellanox BlueField Gigabit Ethernet driver

This is an upstream cherry-pick, e.g., 'git cherry-pick -s -x f92e1869d74e1acc6551256eb084a1c14a054e19'

rtg
-----------
Tim Gardner
Canonical, Inc