Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752680AbcL2L77 (ORCPT ); Thu, 29 Dec 2016 06:59:59 -0500 Received: from fllnx210.ext.ti.com ([198.47.19.17]:12961 "EHLO fllnx210.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752311AbcL2L75 (ORCPT ); Thu, 29 Dec 2016 06:59:57 -0500 Subject: Re: [PATCH] pci: rename *host* directory to *controller* To: Joao Pinto , Bjorn Helgaas References: <1482912577-31356-1-git-send-email-kishon@ti.com> <20161228092228.GA14025@infradead.org> <20161228164118.GB19653@bhelgaas-glaptop.roam.corp.google.com> <1ee66ce2-c321-2341-c964-c8b32218ca7e@synopsys.com> <426f0cb8-b9d4-7c0d-5eea-0ae0d4b01668@synopsys.com> <5864A335.9000203@ti.com> <5864F812.5050605@ti.com> <1d5f9c4b-b619-36fc-082c-962c34cc3194@synopsys.com> CC: Christoph Hellwig , Bjorn Helgaas , , , , Linus Torvalds From: Kishon Vijay Abraham I Message-ID: <5864FA60.2070603@ti.com> Date: Thu, 29 Dec 2016 17:28:24 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <1d5f9c4b-b619-36fc-082c-962c34cc3194@synopsys.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5386 Lines: 131 Hi, On Thursday 29 December 2016 05:23 PM, Joao Pinto wrote: > ?s 11:48 AM de 12/29/2016, Kishon Vijay Abraham I escreveu: >> Hi, >> >> On Thursday 29 December 2016 04:08 PM, Joao Pinto wrote: >>> >>> Hi, >>> >>> ?s 5:46 AM de 12/29/2016, Kishon Vijay Abraham I escreveu: >>>> Hi, >>>> >>>> On Wednesday 28 December 2016 10:50 PM, Joao Pinto wrote: >>>>> ?s 5:17 PM de 12/28/2016, Joao Pinto escreveu: >>>>>> ?s 4:41 PM de 12/28/2016, Bjorn Helgaas escreveu: >>>>>>> On Wed, Dec 28, 2016 at 01:57:13PM +0000, Joao Pinto wrote: >>>>>>>> ?s 9:22 AM de 12/28/2016, Christoph Hellwig escreveu: >>>>>>>>> On Wed, Dec 28, 2016 at 01:39:37PM +0530, Kishon Vijay Abraham I wrote: >>>>>>>>>> As discussed during our LPC discussions, I'm posting the rename patch >>>>>>>>>> here. I'll post the rest of EP series before the next merge window. >>>>>>>>>> >>>>>>>>>> There might be hiccups because of this renaming but feel this is >>>>>>>>>> necessary for long-term maintenance. >>>>>>>>> >>>>>>>>> if we do this rename it would be great to get it to Linus NOW after >>>>>>>>> -rc1 as that minimizes the impact on the 4.11 merge window. >>>>>>>> >>>>>>>> Rename it to controller is a bit vague I thing since we have the PCI Endpoint IP >>>>>>>> also. Wouldn't be better to name it rc_controller? >>>>>>> >>>>>>> I think Kishon's whole goal is to add PCI Endpoint IP, so he wants a >>>>>>> neutral name that can cover both RC and Endpoint. >>>> >>>> right. >>>>>>> >>>>>>> I'm not a huge fan of "controller" because it feels a little bit long >>>>>>> and while I suppose it technically does include the concept of the PCI >>>>>>> interface of an endpoint, it still suggests more of the host side to >>>>>>> me. >>>>>>> >>>>>>> Doesn't USB have a similar situation? I see there's a >>>>>>> drivers/usb/host/ (probably where we copied from in the first place). >>>>>>> Is a USB gadget the USB analog of what you're doing? How do they >>>>>>> share code between the master/slave sides? >>>>>>> >>>>>> >>>>>> The usb/host contains the implemnentations by the spec of the several >>>>>> *hci (USB Host) and then you can have for example the USB 3.0 Designware >>>>>> Host specific ops in dwc3 and for USB 2.0 in dwc2/. >>>> >>>> right, each IP have a separate directory in USB. I thought of doing something >>>> similar for PCI but decided against it since that would involve identifying all >>>> the PCI IPs used and eventually result in more directories. >>>>>> For device purposes it uses the core/ and then some of the device functions >>>>>> are extended from the gadget/ package in which you can use mass_storage and >>>>>> other types of functions. >>>> >>>> That would be similar for PCI endpoint. All endpoint specific core >>>> functionality will be added in drivers/pci/endpoint (see RFC [1]). >>>>>> >>>>>> In our case in PCI we have the core functions inside /drivers/pci and the host >>>>>> mangled inside host. I suggest: >>>>>> >>>>>> drivers/pci >>>>>> drivers/pci/core/ >>>>>> drivers/pci/core/hotplug >>>>>> drivers/pci/core/pcie >>>>>> drivers/pci/core/ >>>>>> drivers/pci/host >>>>>> drivers/pci/dwc -> here would be pcie-designware and the specific vendor drivers >>>>> >>>>> Correction: >>>>> drivers/pci/host/dwc -> here would be pcie-designware and the specific vendor >>>>> drivers >>>>> >>>>>> drivers/pci/ -> here would be the drivers for vendorN controller >>>>> >>>>> Correction: >>>>> drivers/pci/host/ -> here would be the drivers for vendorN controller >>>>> >>>>>> drivers/pci/endpoint -> common code >>>>>> drivers/pci/endpoint/dwc -> implementation of Synopsys specific endpoint ops >>>>>> drivers/pci/ -> implementation of other vendors specific endpoint ops >>>> >>>> There are some parts of the dwc driver that is common to both root complex and >>>> endpoint. Where should that be? I'm sure no one wants to duplicate the common >>>> piece in both root complex and endpoint. >>> >>> You are right, the config space is almost the same and some ops also common. >>> I would suggest: >>> >>> drivers/pci >>> drivers/pci/core/ >>> drivers/pci/core/hotplug >>> drivers/pci/core/pcie >>> drivers/pci/core/ >>> drivers/pci/dwc >>> drivers/pci/dwc/common.c -> common ops and registers between RC and endpoint >>> drivers/pci/dwc/host/ >>> drivers/pci/dwc/endpoint/ >>> >>> What do you think? >> >> I don't think we should have sub-directories within dwc (USB too doesn't have >> sub-directories). Where should the platform specific driver be kept? For >> example pci-dra7xx.c (which use dwc) has both rc and ep specific parts but the >> changes are so minimal that splitting the file won't make much sense. >> >> And such a change would also mean we create a separate directory for every >> other driver present right now in pci/host. > > I understand you idea. We can simplify it this way: > > drivers/pci > drivers/pci/core/ > drivers/pci/core/hotplug > drivers/pci/core/pcie > drivers/pci/core/ > drivers/pci/dwc -> Common files (RC and EP), specific vendor drivers for EP > and EP > > BTW dwc states for DesigWare Controller. > > What do you think? I'd like to avoid using different directory structures for different IPs. Lets try to make it uniform. Thanks Kishon