Hi Robin, On 01/18/2018 09:09 PM, Robin Murphy wrote: >> -#define FORCE_RESET_TIMEOUT 100 /* ms */ >> +#define FORCE_RESET_TIMEOUT 100000 /* us */ >> +#define POLL_TIMEOUT 1000 /* us */ > > Nit: the callsites look a bit odd with the combination of POLL_TIMEOUT > and the magic number 100 - should we not also define something like > POLL_PERIOD for that? Also POLL_TIMEOUT is a rather generic name and > overlaps with several other drivers, so a namespace prefix would be > helpful (i.e. RK_IOMMU_POLL*, or at least RK_POLL*). > > FWIW, my personal preference would be to also suffix these with _US for > absolute clarity, but it's not essential (especially if longer names > lead to more linebreaks at the callsites). right, that make sense, will fix it in next version, thanks :) > > With those undocumented "100"s fixed up, > > Reviewed-by: Robin Murphy <robin.murphy at arm.com>
