Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756053AbdCGTVu (ORCPT ); Tue, 7 Mar 2017 14:21:50 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:48070 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755709AbdCGTV0 (ORCPT ); Tue, 7 Mar 2017 14:21:26 -0500 Date: Tue, 7 Mar 2017 18:14:03 +0100 From: "gregkh@linuxfoundation.org" To: Bart Van Assche Cc: "linux-kernel@vger.kernel.org" , "linux-rdma@vger.kernel.org" , "parav@mellanox.com" , "sebott@linux.vnet.ibm.com" , "linux@armlinux.org.uk" , "hpa@zytor.com" , "mingo@redhat.com" , "dwmw2@infradead.org" , "bhelgaas@google.com" , "dledford@redhat.com" , "benh@kernel.crashing.org" Subject: Re: [PATCH 1/2] device: Stop requiring that struct device is embedded in struct pci_dev Message-ID: <20170307171403.GA19293@kroah.com> References: <20170307003549.3872-1-bart.vanassche@sandisk.com> <20170307003549.3872-2-bart.vanassche@sandisk.com> <20170307045236.GC3913@kroah.com> <1488905685.2739.1.camel@sandisk.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1488905685.2739.1.camel@sandisk.com> User-Agent: Mutt/1.8.0 (2017-02-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2359 Lines: 57 On Tue, Mar 07, 2017 at 04:54:58PM +0000, Bart Van Assche wrote: > On Tue, 2017-03-07 at 05:52 +0100, Greg Kroah-Hartman wrote: > > Somehow all other subsystems work just fine, don't instantly think that > > the driver core needs to bend to the will of the IB code, because you > > are somehow "special". Hint, you aren't :) > > Hi Greg, > > In another e-mail Parav compared IB drivers with networking drivers. Great, then notice that networking drivers don't need to do this type of crud :) > But I think that's a bad comparison: in the networking stack it's the > network driver itself that sets up and triggers DMA while in the IB > stack it's the upper layer protocol (ULP) driver that calls the > functions defined in struct dma_ops. For some IB HW drivers (hfi1, qib > and rdma_rxe) the ULP driver must > use the DMA mapping operations from lib/dma-virt.c while for all other IB HW > drivers the ULP driver must use the PCI DMA mapping functions. The ib_dma_*() > functions select the right DMA mapping operations - either the PCI DMA > mapping operations or those from lib/dma-virt.c. My question to you is how we > should organize struct ib_device such that we can get rid of the ib_dma_*() > helper functions. How to make sure that the to_pci_dev() translation works > correctly for the device structure that is embedded in struct ib_device? > Should a pointer to struct pci_dev be embedded in struct device (as done in > patch 1/2 in this series) I already said no to this, why do you think that it is still ok? > or should the struct device in ib_device be changed > into a struct pci_dev Ick, no. > and should the pci_dev information from /sys/devices/pci*/*/* be > duplicated into the pci_dev information in struct ib_device > (/sys/devices/pci*/*/*/infiniband/*)? I don't think you really thought that one through :) > For the latter approach, would > there be a risk that the duplicated information becomes inconsistent? No, it just wouldn't work :) Why not just save off a pointer to your pci_dev in your ib_device structure? That way you know what the type is, and you have access to everything you need. But hey, I know nothing about IB and I really want to keep it that way. You do what you want to, as long as you don't abuse the driver model, like your patch 1/2 did. Remember, not all the world is IB. thanks, greg k-h