Re: [PATCH v2 1/2] pcie-designware: add iATU Unroll feature

[Joao Pinto <Joao.Pinto@synopsys.com>]

On 8/2/2016 1:53 PM, Bjorn Helgaas wrote:
> On Tue, Aug 02, 2016 at 11:27:45AM +0100, Joao Pinto wrote:
>> On 7/25/2016 9:44 PM, Bjorn Helgaas wrote:
>>> On Fri, Jul 22, 2016 at 02:29:38PM +0100, Joao Pinto wrote:
>>>> This patch adds the support to the new iATU mechanism that will be used
>>>> from Core version 4.80, which is called iATU Unroll.
>>>> The new Cores can support the iATU Unroll or support the "old" iATU 
>>>> method now called Legacy Mode. The driver is perfectly capable of
>>>> performing well for both.
>>>>
>>>> In order to make sure that the iATU is really enabled a for loop was
>>>> introduced in dw_pcie_prog_outbound_atu() to improve reliability.
>>>>

(snip...)

>> ...
>>>> +static inline void dw_pcie_readl_unroll(struct pcie_port *pp, u32 type,
>>>> +						u32 index, u32 reg, u32 *val)
>>>> +{
>>>> +	u32 reg_addr = 0;
>>>> +
>>>> +	if (type == PCIE_TRANSL_OUTB)
>>>> +		reg_addr = PCIE_GET_ATU_OUTB_UNR_REG_ADDR(index, reg);
>>>> +	else if  (type == PCIE_TRANSL_INB)
>>>> +		reg_addr = PCIE_GET_ATU_INB_UNR_REG_ADDR(index, reg);
>>>
>>> PCIE_TRANSL_INB is never used.  In fact, dw_pcie_readl_unroll() is
>>> *always* called with PCIE_TRANSL_OUTB, so it's not clear what the
>>> value of passing it as an argument is.
>>
>> I included The Inbound translation mechanism because you can have Inbound or
>> Outbound translation in the iATU. The old mechanism also had Inbound properties
>> that are not used in the code. I suggest we keep the Inbound mechanism to avoid
>> future rework. Agree?
> 
> Again, not a huge deal, but since PCIE_TRANSL_INB is not used, it
> makes life harder for the reader, and the related code is not tested.
> So personally, I would remove it and add it back if/when it is needed.
> 
> Bjorn
> 

Ok, I will do that and send a new version soon. Thanks!

Joao