Re: [PATCH] NTB: Register and offset values fix for memory window

[Shyam Sundar S K <ssundark@amd.com>]

On 11/14/2016 8:05 PM, Allen Hubbe wrote:
> From: Shyam Sundar S K
>> Due to incorrect limit and translation register values, NTB link was
>> going down when the memory window translation was setup. Made appropriate
>> changes as per spec.
>>
>> Also, fixed the limit register values for BAR1, which was overlapping
>> with the BAR23 address.
>>
>> Reviewed-by: Sen, Pankaj <Pankaj.Sen@amd.com>
>> Reviewed-by: Shah, Nehal-bakulchandra <Nehal-bakulchandra.Shah@amd.com>
>> Acked-by: Xiangliang Yu <Xiangliang.Yu@amd.com>
>> Signed-off-by: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>
>> ---
> 
>> @@ -376,13 +371,11 @@ static u32 amd_ntb_spad_read(struct ntb_dev *ntb, int idx)
>>  {
>>  	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
>>  	void __iomem *mmio = ndev->self_mmio;
>> -	u32 offset;
>>
>> -	if (idx < 0 || idx >= ndev->spad_count)
>> +	if (idx < 0 || idx >= (ndev->spad_count + 4))
> 
> Why does this change add four to the upper end of the range check?  Does spad_count have the wrong number of spads?
> 
>>  		return 0;
>>
>> -	offset = ndev->self_spad + (idx << 2);
>> -	return readl(mmio + AMD_SPAD_OFFSET + offset);
>> +	return readl(mmio + AMD_SPAD_OFFSET + (idx << 2));
> 
> The self_spad is used for sharing the spads of a single ntb.  It is the offset of the first or second half of the spads, and the peer self_spad is the other half.  From this change, can we assume that a single-ntb topology will not be supported?
> 
This patch has more to with the NTB driver becoming functional on AMD platforms.
We were reading from the incorrect offsets than the one mentioned in the AMD NTB HW spec. So changed it accordingly.