| Message ID | 1477407617-20745-1-git-send-email-saeedm@mellanox.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
I really disalike pull requests of this form. You add lots of datastructures and helper functions but no actual users of these facilities to the driver. Do this instead: 1) Add TSAR infrastructure 2) Add use of TSAR facilities to the driver That's one pull request. I don't care if this is hard, or if there are entanglements with Infiniband or whatever, you must submit changes in this manner. I will not accept additions to a driver that don't even get really used.
On Fri, Oct 28, 2016 at 7:53 PM, David Miller <davem@davemloft.net> wrote: > > I really disalike pull requests of this form. > > You add lots of datastructures and helper functions but no actual > users of these facilities to the driver. > > Do this instead: > > 1) Add TSAR infrastructure > 2) Add use of TSAR facilities to the driver > > That's one pull request. > > I don't care if this is hard, or if there are entanglements with > Infiniband or whatever, you must submit changes in this manner. > It is not hard, it is just not right, we have lots of IB and ETH features that we would like to submit in the same kernel cycle, with your suggestion I will have to almost submit every feature (core infrastructure and netdev/RDMA usage) to you and Doug. Same for rdma features, you will receive PULL request for them as well, I am sure you and the netdev list don't need such noise. do not forget that this will slow down mlx5 progress since netde will block rdma and vise-versa. > I will not accept additions to a driver that don't even get really > used. For logic/helper functions containing patches such as "Add TSAR infrastructure" I agree and i can find a way to move some code around to avoid future conflicts and remove them from such pull requests. but you need to at least accept hardware related structures infrastructure patches for shared code such as include/linux/mlx5/mlx5_ifc.h where we have only hardware definitions and those patches are really minimal. So bottom line, I will do my best to ensure future PULL requests will contain only include/linux/mlx5/*.h hardware related definitions or fully implemented features. Can we agree on that ? Thanks, Saeed.
From: Saeed Mahameed <saeedm@dev.mellanox.co.il> Date: Sun, 30 Oct 2016 11:59:57 +0200 > On Fri, Oct 28, 2016 at 7:53 PM, David Miller <davem@davemloft.net> wrote: >> >> I really disalike pull requests of this form. >> >> You add lots of datastructures and helper functions but no actual >> users of these facilities to the driver. >> >> Do this instead: >> >> 1) Add TSAR infrastructure >> 2) Add use of TSAR facilities to the driver >> >> That's one pull request. >> >> I don't care if this is hard, or if there are entanglements with >> Infiniband or whatever, you must submit changes in this manner. >> > > It is not hard, it is just not right, we have lots of IB and ETH > features that we would like to submit in the same kernel cycle, > with your suggestion I will have to almost submit every feature (core > infrastructure and netdev/RDMA usage) > to you and Doug. Nobody can properly review an API addition without seeing how that API is _USED_. This is a simple fundamental fact. And I'm not pulling in code that can't be reviewed properly. Also, so many times people have added new junk to drivers and months later never added the users of that new code and interfaces. Forcing you to provide the use with the API addition makes sure that it is absolutely impossible for that to happen. Whatever issues you think prevent this are your issues, not mine. I want high quality submissions that can be properly reviewed, and you have to find a way to satisfy that requirement.
Hi Dave and Doug, This series contains some low level and API updates for mlx5 core driver interface and mlx5_ifc.h, to be shared as base code for net-next and rdma mlx5 4.10 submissions. From Artemy Kovalyov: - Update hardware struct mlx5_ifc_xrqc_bits - Ensure SRQ physical address structure endianness - Add KSM mlx5_ifc hardware bits support From Eugenia Emantayev: - Fix length of async_event_mask - MTPPS (Pulse-per-second) hardware events and registers support From Gal Pressman: - Add PPCNT physical layer statistical group infrastructure - Add MPCNT register infrastructure From Huy Nguyen: - Port module event hardware structures From Leon Romanovsky: - Report multi packet WQE hardware capabilities From Mohamad Haj Yahia: - Add support to s-tag in mlx5 firmware interface - Introduce TSAR (Transmit Scheduling Arbiter) manipulation firmware commands From Saeed Mahameed: - mlx5 IB: Skip handling unknown mlx5 events - Set driver version infrastructure - Add ConnectX-5 PCIe 4.0 VF device ID The following changes since commit 1001354ca34179f3db924eb66672442a173147dc: Linux 4.9-rc1 are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git tags/shared-for-4.10-1 for you to fetch changes up to 46f6d2a982cd83679b18d0bebe2425fe65552c58: net/mlx5: Add ConnectX-5 PCIe 4.0 VF device ID Thanks, Saeed & Leon. Artemy Kovalyov (3): net/mlx5: Update struct mlx5_ifc_xrqc_bits net/mlx5: Ensure SRQ physical address structure endianness net/mlx5: Add KSM support Eugenia Emantayev (2): net/mlx5: Fix length of async_event_mask net/mlx5: MTPPS (Pulse-per-second) events and registers support Gal Pressman (2): net/mlx5: Add PPCNT physical layer statistical group infrastructure net/mlx5: Add MPCNT register infrastructure Huy Nguyen (1): net/mlx5: Port module event hardware structures Leon Romanovsky (1): net/mlx5: Report multi packet WQE capabilities Mohamad Haj Yahia (2): net/mlx5: Add support to s-tag in mlx5 firmware interface net/mlx5: Introduce TSAR manipulation firmware commands Saeed Mahameed (3): IB/mlx5: Skip handling unknown events net/mlx5: Set driver version infrastructure net/mlx5: Add ConnectX-5 PCIe 4.0 VF device ID drivers/infiniband/hw/mlx5/main.c | 6 +- drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 13 +- drivers/net/ethernet/mellanox/mlx5/core/en_fs.c | 10 +- .../ethernet/mellanox/mlx5/core/en_fs_ethtool.c | 4 +- drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 4 +- drivers/net/ethernet/mellanox/mlx5/core/eq.c | 2 +- drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 12 +- drivers/net/ethernet/mellanox/mlx5/core/main.c | 1 + .../net/ethernet/mellanox/mlx5/core/mlx5_core.h | 7 + drivers/net/ethernet/mellanox/mlx5/core/rl.c | 65 +++ include/linux/mlx5/device.h | 35 ++ include/linux/mlx5/driver.h | 4 + include/linux/mlx5/mlx5_ifc.h | 440 ++++++++++++++++++++- include/linux/mlx5/port.h | 3 + include/linux/mlx5/srq.h | 2 +- 15 files changed, 572 insertions(+), 36 deletions(-)