Re: [PATCH 5/5] PCI: cadence: add EndPoint Controller driver for Cadence PCIe controller

From: Cyrille Pitchen
Date: Thu Dec 14 2017 - 12:03:09 EST


Le 13/12/2017 à 17:50, Cyrille Pitchen a écrit :
> Hi Kishon,
>
> Le 05/12/2017 à 10:19, Kishon Vijay Abraham I a écrit :
>> Hi,
>>
>> On Friday 01 December 2017 05:50 PM, Lorenzo Pieralisi wrote:
>>> On Thu, Nov 23, 2017 at 04:01:50PM +0100, Cyrille Pitchen wrote:
>>>> This patch adds support to the Cadence PCIe controller in endpoint mode.
>>>
>>> Please add a brief description to the log to describe the most salient
>>> features.
>>>
>>>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@xxxxxxxxxxxxxxxxxx>
>>>> ---
>>>> drivers/pci/cadence/Kconfig | 9 +
>>>> drivers/pci/cadence/Makefile | 1 +
>>>> drivers/pci/cadence/pcie-cadence-ep.c | 553 ++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 563 insertions(+)
>>>> create mode 100644 drivers/pci/cadence/pcie-cadence-ep.c
[...]
>>>> +static int cdns_pcie_ep_write_header(struct pci_epc *epc,
>>>> + struct pci_epf_header *hdr)
>>>> +{
>>>> + struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
>>>> + struct cdns_pcie *pcie = &ep->pcie;
>>>> + u8 fn = 0;
>>>> +
>>>> + if (fn == 0) {
>>>
>>> I think there is some code to retrieve fn missing here.
>>
>> hmm.. the endpoint core has to send the function number which right now it's
>> not doing though it has the function number info in pci_epf.
>
> Would it be OK if I add a new patch in the next series adding a
> 'struct pcie_epf *epf' as a 2nd argument to all handlers in the
> 'struct pcie_epc_ops'? This way I could have access to epf->func_no as needed.
>

Except for pci_epc_start() and pci_epc_stop(), both only called from
pci_epc_start_store(), I don't have trouble getting the epf value to be passed
as a 2nd argument to all other handlers in 'struct pcie_epc_ops'.

Now my next question is: is it better to keep the 'struct pci_epc *epc' as
the 1st argument of all those handlers or do you prefer me to remove it as
the value can always be retrieved from epf->epc, since now we provide epf as
a new argument ?

I have no personal preference. Please let me know your choice :)

> Best regards,
>
> Cyrille
>