Re: [PATCH] pci: rename *host* directory to *controller*

[Joao Pinto]

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/<all other files inside pci/ today>
>>>  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/<vendorN> -> here would be the drivers for vendorN controller
>>
>> Correction:
>> drivers/pci/host/<vendorN> -> 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/<vendorN> -> 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/<all other files inside pci/ today>
  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?

Thanks.

> 
> [1] -> https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2016_9_14_27&d=DgID-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=E9ExIpzr6vIxD1dZDQcUzR5Y-MLqjT1aO_A96qhd_Dk&s=9vvw2jAdXsYlzu0KriDWnmdHa3H_GPFP5Ti2dbM865A&e= 
> 
> Thanks
> Kishon
>