Re: [PATCH v3] powerpc/pci: Assign fixed PHB number based on device-tree properties

["Guilherme G. Piccoli" <gpiccoli@linux.vnet.ibm.com>]

On 03/18/2016 01:00 AM, Gavin Shan wrote:
>> Apart from below minor issues to be fixed, it looks good to me.
>>
>> Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com>

Thanks for the review Gavin, I'll fix the else/if detail and send a v4.


>>> +	} else {
>>> +		if (machine_is(powernv)) {
>>> +			prop64 = of_get_property(dn, "ibm,opal-phbid", NULL);
>>> +			if (prop64)
>>> +				return (int)(be64_to_cpup(prop64) & 0xFFFF);
>>> +		}
>>> +	}
>>> +
>>
>> The nested statements can be merged to one with "else if (machine_is(powernv))".
>>
>>> +	/* if not pSeries nor PowerNV, fallback to dynamic PHB numbering */
>>> +	phb_id = find_first_zero_bit(phb_bitmap, MAX_PHBS);
>>> +	BUG_ON(phb_id >= MAX_PHBS); /* reached maximum number of PHBs */
>>> +	set_bit(phb_id, phb_bitmap);
>>> +	return phb_id;
>>
>> It's possible another CPU sets this bit before current CPU updates it, which
>> will cause same domain number used by two PHBs though it's very rare. I guess
>> it might be worthwhile to check if the bit is reserved by somebody else to
>> avoid the issue.
>>
>
> Please ignore this comment: there is a lock (hose_spinlock) avoiding the issue.
>

Ok =)


>> Thanks,
>> Gavin

Cheers,


Guilherme