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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 15 Dec 2017 11:19:06 +0530
From:   Kishon Vijay Abraham I <kishon@...com>
To:     Cyrille Pitchen <cyrille.pitchen@...e-electrons.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>
CC:     <bhelgaas@...gle.com>, <linux-pci@...r.kernel.org>,
        <adouglas@...ence.com>, <stelford@...ence.com>,
        <dgary@...ence.com>, <kgopi@...ence.com>, <eandrews@...ence.com>,
        <thomas.petazzoni@...e-electrons.com>, <sureshp@...ence.com>,
        <nsekhar@...com>, <linux-kernel@...r.kernel.org>,
        <robh@...nel.org>, <devicetree@...r.kernel.org>
Subject: Re: [PATCH 5/5] PCI: cadence: add EndPoint Controller driver for
 Cadence PCIe controller

Hi Cyrille,

On Thursday 14 December 2017 10:33 PM, Cyrille Pitchen wrote:
> 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@...e-electrons.com>
>>>>> ---
>>>>>  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.

I prefer we just pass the func_no as the second argument. Do you see a problem
with that?
>>
> 
> 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'.

pci_epc_start()/pci_epc_stop() is used to start/stop the end point controller
as a whole and shouldn't need epf.
> 
> 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 ?

Do we really need to pass epf when func_no is all we need?

Thanks
Kishon