Re: [PATCH 1/4] PCI: dwc: Add new feature to skip core initialization

From: Vidya Sagar <vidyas@nvidia.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: <jingoohan1@gmail.com>, <gustavo.pimentel@synopsys.com>,
	<lorenzo.pieralisi@arm.com>, <andrew.murray@arm.com>,
	<bhelgaas@google.com>, <kishon@ti.com>,
	<thierry.reding@gmail.com>, <Jisheng.Zhang@synaptics.com>,
	<jonathanh@nvidia.com>, <linux-pci@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <kthota@nvidia.com>,
	<mmaddireddy@nvidia.com>, <sagar.tv@gmail.com>
Subject: Re: [PATCH 1/4] PCI: dwc: Add new feature to skip core initialization
Date: Fri, 29 Nov 2019 20:10:10 +0530	[thread overview]
Message-ID: <80d610bf-71d8-d2c1-9c75-b0a58cb5c8ed@nvidia.com> (raw)
In-Reply-To: <20191127094844.GA21122@infradead.org>

On 11/27/2019 3:18 PM, Christoph Hellwig wrote:
> On Wed, Nov 13, 2019 at 02:38:48PM +0530, Vidya Sagar wrote:
>> +	if (ep->ops->get_features) {
>> +		epc_features = ep->ops->get_features(ep);
>> +		if (epc_features->skip_core_init)
>> +			return 0;
>>   	}
>>   
>> +	return dw_pcie_ep_init_complete(ep);
> 
> This calling convention is strange.  Just split the early part of
> dw_pcie_ep_init into an dw_pcie_ep_early and either add a tiny
> wrapper like:
> 
> int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> {
> 	int error;
> 
> 	error = dw_pcie_ep_init_early(ep);
> 	if (error)
> 		return error;
> 	return dw_pcie_ep_init_late(ep);
> }
> 
> or just open code that in the few callers.  That keeps the calling
> conventions much simpler and avoids relying on a callback and flag.
I'm not sure if I got this right. I think in any case, code that is going to be
part of dw_pcie_ep_init_late() needs to depend on callback and flag right?
I mean, unless it is confirmed (by calling the get_features() callback and
checking whether or not the core is available for programming) dw_pcie_ep_init_late()
can't be called right?
Please let me know if I'm missing something here.

- Vidya Sagar
>