On 03/18/2016 01:00 AM, Gavin Shan wrote:

quoted

Apart from below minor issues to be fixed, it looks good to me.

Reviewed-by: Gavin Shan <redacted>

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

quoted

quoted

+	} 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))".

quoted

+	/* 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 =)

quoted

Thanks,
Gavin

Cheers,


Guilherme