| Message ID | 20200520070227.3392100-1-jeffrey.t.kirsher@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | 100GbE Intel Wired LAN Driver Updates 2020-05-19 | expand |
On Wed, May 20, 2020 at 12:02:15AM -0700, Jeff Kirsher wrote: > This series contains the initial implementation of the Virtual Bus, > virtbus_device, virtbus_driver, updates to 'ice' and 'i40e' to use the new > Virtual Bus. > > The primary purpose of the Virtual bus is to put devices on it and hook the > devices up to drivers. This will allow drivers, like the RDMA drivers, to > hook up to devices via this Virtual bus. > > The associated irdma driver designed to use this new interface, is still > in RFC currently and was sent in a separate series. The latest RFC > series follows this series, named "Intel RDMA Driver Updates 2020-05-19". > > This series currently builds against net-next tree. > > Revision history: > v2: Made changes based on community feedback, like Pierre-Louis's and > Jason's comments to update virtual bus interface. > v3: Updated the virtual bus interface based on feedback from Jason and > Greg KH. Also updated the initial ice driver patch to handle the > virtual bus changes and changes requested by Jason and Greg KH. > v4: Updated the kernel documentation based on feedback from Greg KH. > Also added PM interface updates to satisfy the sound driver > requirements. Added the sound driver changes that makes use of the > virtual bus. Why didn't you change patch 2 like I asked you to? And I still have no idea why you all are not using the virtual bus in the "ice" driver implementation. Why is it even there if you don't need it? I thought that was the whole reason you wrote this code, not for the sound drivers. How can you get away with just using a virtual device but not the bus? What does that help out with? What "bus" do those devices belong to? Again, please fix up patch 2 to only add virtual device/bus support to, right now it is just too much of a mess with all of the other functionality you are adding in there to be able to determine if you are using the new api correctly. And again, didn't I ask for this last time? greg k-h
> -----Original Message----- > From: Greg KH <gregkh@linuxfoundation.org> > Sent: Wednesday, May 20, 2020 00:17 > To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com> > Cc: davem@davemloft.net; netdev@vger.kernel.org; linux- > rdma@vger.kernel.org; nhorman@redhat.com; sassmann@redhat.com; > jgg@ziepe.ca; parav@mellanox.com; galpress@amazon.com; > selvin.xavier@broadcom.com; sriharsha.basavapatna@broadcom.com; > benve@cisco.com; bharat@chelsio.com; xavier.huwei@huawei.com; > yishaih@mellanox.com; leonro@mellanox.com; mkalderon@marvell.com; > aditr@vmware.com; ranjani.sridharan@linux.intel.com; pierre- > louis.bossart@linux.intel.com > Subject: Re: [net-next v4 00/12][pull request] 100GbE Intel Wired LAN Driver > Updates 2020-05-19 > > On Wed, May 20, 2020 at 12:02:15AM -0700, Jeff Kirsher wrote: > > This series contains the initial implementation of the Virtual Bus, > > virtbus_device, virtbus_driver, updates to 'ice' and 'i40e' to use the > > new Virtual Bus. > > > > The primary purpose of the Virtual bus is to put devices on it and > > hook the devices up to drivers. This will allow drivers, like the > > RDMA drivers, to hook up to devices via this Virtual bus. > > > > The associated irdma driver designed to use this new interface, is > > still in RFC currently and was sent in a separate series. The latest > > RFC series follows this series, named "Intel RDMA Driver Updates 2020-05- > 19". > > > > This series currently builds against net-next tree. > > > > Revision history: > > v2: Made changes based on community feedback, like Pierre-Louis's and > > Jason's comments to update virtual bus interface. > > v3: Updated the virtual bus interface based on feedback from Jason and > > Greg KH. Also updated the initial ice driver patch to handle the > > virtual bus changes and changes requested by Jason and Greg KH. > > v4: Updated the kernel documentation based on feedback from Greg KH. > > Also added PM interface updates to satisfy the sound driver > > requirements. Added the sound driver changes that makes use of the > > virtual bus. > > Why didn't you change patch 2 like I asked you to? > > And I still have no idea why you all are not using the virtual bus in the "ice" > driver implementation. Why is it even there if you don't need it? I thought that > was the whole reason you wrote this code, not for the sound drivers. > > How can you get away with just using a virtual device but not the bus? > What does that help out with? What "bus" do those devices belong to? > > Again, please fix up patch 2 to only add virtual device/bus support to, right now > it is just too much of a mess with all of the other functionality you are adding in > there to be able to determine if you are using the new api correctly. > > And again, didn't I ask for this last time? [Kirsher, Jeffrey T] We apologize, but last submission you only commented on the first patch and the documentation. In v1 & v2, you and Jason made comments on the LAN driver implementation (patch 2), which we addressed all the comments and did not hear any comments to the contrary in v3 until now. If you give constructive feedback, will work to fix any issues you find.
On Wed, May 20, 2020 at 07:25:39AM +0000, Kirsher, Jeffrey T wrote: > > -----Original Message----- > > From: Greg KH <gregkh@linuxfoundation.org> > > Sent: Wednesday, May 20, 2020 00:17 > > To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com> > > Cc: davem@davemloft.net; netdev@vger.kernel.org; linux- > > rdma@vger.kernel.org; nhorman@redhat.com; sassmann@redhat.com; > > jgg@ziepe.ca; parav@mellanox.com; galpress@amazon.com; > > selvin.xavier@broadcom.com; sriharsha.basavapatna@broadcom.com; > > benve@cisco.com; bharat@chelsio.com; xavier.huwei@huawei.com; > > yishaih@mellanox.com; leonro@mellanox.com; mkalderon@marvell.com; > > aditr@vmware.com; ranjani.sridharan@linux.intel.com; pierre- > > louis.bossart@linux.intel.com > > Subject: Re: [net-next v4 00/12][pull request] 100GbE Intel Wired LAN Driver > > Updates 2020-05-19 > > > > On Wed, May 20, 2020 at 12:02:15AM -0700, Jeff Kirsher wrote: > > > This series contains the initial implementation of the Virtual Bus, > > > virtbus_device, virtbus_driver, updates to 'ice' and 'i40e' to use the > > > new Virtual Bus. > > > > > > The primary purpose of the Virtual bus is to put devices on it and > > > hook the devices up to drivers. This will allow drivers, like the > > > RDMA drivers, to hook up to devices via this Virtual bus. > > > > > > The associated irdma driver designed to use this new interface, is > > > still in RFC currently and was sent in a separate series. The latest > > > RFC series follows this series, named "Intel RDMA Driver Updates 2020-05- > > 19". > > > > > > This series currently builds against net-next tree. > > > > > > Revision history: > > > v2: Made changes based on community feedback, like Pierre-Louis's and > > > Jason's comments to update virtual bus interface. > > > v3: Updated the virtual bus interface based on feedback from Jason and > > > Greg KH. Also updated the initial ice driver patch to handle the > > > virtual bus changes and changes requested by Jason and Greg KH. > > > v4: Updated the kernel documentation based on feedback from Greg KH. > > > Also added PM interface updates to satisfy the sound driver > > > requirements. Added the sound driver changes that makes use of the > > > virtual bus. > > > > Why didn't you change patch 2 like I asked you to? > > > > And I still have no idea why you all are not using the virtual bus in the "ice" > > driver implementation. Why is it even there if you don't need it? I thought that > > was the whole reason you wrote this code, not for the sound drivers. > > > > How can you get away with just using a virtual device but not the bus? > > What does that help out with? What "bus" do those devices belong to? > > > > Again, please fix up patch 2 to only add virtual device/bus support to, right now > > it is just too much of a mess with all of the other functionality you are adding in > > there to be able to determine if you are using the new api correctly. > > > > And again, didn't I ask for this last time? > [Kirsher, Jeffrey T] > > We apologize, but last submission you only commented on the first patch and the documentation. It's as if I am shouting into the wind... {sigh} : https://lore.kernel.org/linux-rdma/20200507081737.GC1024567@kroah.com/ Ok, as the above text was too kind and nice and not explicit enough, let me try this again: - this patch series makes no sense to me in that you are creating a virtual bus, but not using it in your driver at all. Why create it at all then? - If a virtual device can be used without a virtual driver, what driver binds to that device, and what "bus" does it live on? - This patch 2 is a total mess of new functionality and virtual device additions, making it impossible to review. Please split it up into tiny, easy to understand and review pieces. - Why is there sound driver code being submitted to netdev? This virtual bus code should stand on-its-own, and if it is not needed, then let the code that adds it come in through a patch series that actually needs it (i.e. the sound code.) - As I can't understand how you are using the virtual bus/dev code as I can't review patch 2, I have no idea if patch 1 is even written correctly. So, your action items now are: - make patch 2 sane, in tiny pieces, and use the virtual bus code. - review the documentation on patch 1 to see if it actually makes sense (i.e. get a s-o-b from another kernel developer who has never seen it before). - if patch 2 does not need the virtual bus code, explain the heck out of it as to why that is so, and where the driver and bus lives instead, when you add support for that code to patch 2 (as part of the split up patch series). - stop sending these patches out as a "pull request" for netdev maintainers to pull from. Get my ack on them all before you even attempt to get a networking maintainer's review to be included in their tree. By doing this you are making me jump in order to keep from this code getting merged before it should be, which just makes me grumpy (as you would be if you were in my position here.) - send Greg a bottle of good whisky in penance for wasting all of his time with these reviews that seem to be ignored. Expense it to Intel, it's the least they could do. Sound reasonable? greg k-h
This series contains the initial implementation of the Virtual Bus, virtbus_device, virtbus_driver, updates to 'ice' and 'i40e' to use the new Virtual Bus. The primary purpose of the Virtual bus is to put devices on it and hook the devices up to drivers. This will allow drivers, like the RDMA drivers, to hook up to devices via this Virtual bus. The associated irdma driver designed to use this new interface, is still in RFC currently and was sent in a separate series. The latest RFC series follows this series, named "Intel RDMA Driver Updates 2020-05-19". This series currently builds against net-next tree. Revision history: v2: Made changes based on community feedback, like Pierre-Louis's and Jason's comments to update virtual bus interface. v3: Updated the virtual bus interface based on feedback from Jason and Greg KH. Also updated the initial ice driver patch to handle the virtual bus changes and changes requested by Jason and Greg KH. v4: Updated the kernel documentation based on feedback from Greg KH. Also added PM interface updates to satisfy the sound driver requirements. Added the sound driver changes that makes use of the virtual bus. The following are changes since commit 2de499258659823b3c7207c5bda089c822b67d69: Merge branch 's390-next' and are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 100GbE Dave Ertman (7): Implementation of Virtual Bus ice: Create and register virtual bus for RDMA ice: Complete RDMA peer registration ice: Support resource allocation requests ice: Enable event notifications ice: Allow reset operations ice: Pass through communications to VF Ranjani Sridharan (3): ASoC: SOF: Introduce descriptors for SOF client ASoC: SOF: Create client driver for IPC test ASoC: SOF: ops: Add new op for client registration Shiraz Saleem (2): i40e: Move client header location i40e: Register a virtbus device to provide RDMA Documentation/driver-api/index.rst | 1 + Documentation/driver-api/virtual_bus.rst | 93 ++ MAINTAINERS | 1 + drivers/bus/Kconfig | 10 + drivers/bus/Makefile | 2 + drivers/bus/virtual_bus.c | 215 +++ drivers/infiniband/hw/i40iw/Makefile | 1 - drivers/infiniband/hw/i40iw/i40iw.h | 2 +- drivers/net/ethernet/intel/Kconfig | 2 + drivers/net/ethernet/intel/i40e/i40e.h | 2 +- drivers/net/ethernet/intel/i40e/i40e_client.c | 133 +- drivers/net/ethernet/intel/ice/Makefile | 1 + drivers/net/ethernet/intel/ice/ice.h | 14 + .../net/ethernet/intel/ice/ice_adminq_cmd.h | 33 + drivers/net/ethernet/intel/ice/ice_common.c | 206 ++- drivers/net/ethernet/intel/ice/ice_common.h | 9 + drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 68 + drivers/net/ethernet/intel/ice/ice_dcb_lib.h | 3 + .../net/ethernet/intel/ice/ice_hw_autogen.h | 1 + drivers/net/ethernet/intel/ice/ice_idc.c | 1344 +++++++++++++++++ drivers/net/ethernet/intel/ice/ice_idc_int.h | 105 ++ drivers/net/ethernet/intel/ice/ice_lib.c | 50 + drivers/net/ethernet/intel/ice/ice_lib.h | 4 + drivers/net/ethernet/intel/ice/ice_main.c | 105 +- drivers/net/ethernet/intel/ice/ice_sched.c | 69 +- drivers/net/ethernet/intel/ice/ice_switch.c | 27 + drivers/net/ethernet/intel/ice/ice_switch.h | 4 + drivers/net/ethernet/intel/ice/ice_type.h | 4 + .../net/ethernet/intel/ice/ice_virtchnl_pf.c | 59 +- include/linux/mod_devicetable.h | 8 + .../linux/net/intel}/i40e_client.h | 15 + include/linux/net/intel/iidc.h | 337 +++++ include/linux/virtual_bus.h | 62 + scripts/mod/devicetable-offsets.c | 3 + scripts/mod/file2alias.c | 7 + sound/soc/sof/Kconfig | 30 + sound/soc/sof/Makefile | 6 +- sound/soc/sof/core.c | 10 + sound/soc/sof/intel/Kconfig | 1 + sound/soc/sof/intel/apl.c | 26 + sound/soc/sof/intel/bdw.c | 25 + sound/soc/sof/intel/byt.c | 28 + sound/soc/sof/intel/cnl.c | 26 + sound/soc/sof/ops.h | 34 + sound/soc/sof/sof-client.c | 91 ++ sound/soc/sof/sof-client.h | 84 ++ sound/soc/sof/sof-ipc-test-client.c | 325 ++++ sound/soc/sof/sof-priv.h | 9 + 48 files changed, 3630 insertions(+), 65 deletions(-) create mode 100644 Documentation/driver-api/virtual_bus.rst create mode 100644 drivers/bus/virtual_bus.c create mode 100644 drivers/net/ethernet/intel/ice/ice_idc.c create mode 100644 drivers/net/ethernet/intel/ice/ice_idc_int.h rename {drivers/net/ethernet/intel/i40e => include/linux/net/intel}/i40e_client.h (94%) create mode 100644 include/linux/net/intel/iidc.h create mode 100644 include/linux/virtual_bus.h create mode 100644 sound/soc/sof/sof-client.c create mode 100644 sound/soc/sof/sof-client.h create mode 100644 sound/soc/sof/sof-ipc-test-client.c