Re: [PATCH v3] powerpc/powernv: Use darn instr for random_seed on p9

Matt Brown [off-list ref] writes:

On Sat, Aug 5, 2017 at 3:06 AM, Tyrel Datwyler [off-list ref] wrote:

quoted

On 08/03/2017 06:12 PM, Matt Brown wrote:

quoted

@@ -135,8 +152,9 @@ static __init int rng_create(struct device_node *dn)

 static __init int rng_init(void)
 {
+     unsigned long darn_test;
      struct device_node *dn;
-     int rc;
+     int rc, i;

      for_each_compatible_node(dn, NULL, "ibm,power-rng") {
              rc = rng_create(dn);

@@ -150,6 +168,21 @@ static __init int rng_init(void)
              of_platform_device_create(dn, NULL, NULL);
      }

+     if (cpu_has_feature(CPU_FTR_ARCH_300)) {
+             for (i = 0; i < 10; i++) {
+                     if (powernv_get_random_darn(&darn_test)) {
+                             ppc_md.get_random_seed =
+                                     powernv_get_random_darn;
+                             break;

If you return directly here you can avoid the (i == 9) conditional every iteration of the
loop by moving the pr_warn to just outside the loop.

That's true, although it is very unlikely for the
powernv_get_random_darn to fail. So in practice we should never reach
the (i == 9) conditional.
The loop is more of a backup in the rare case that it does fail.

All true, the runtime overhead is really not an issue. It's more that
testing for loop almost-termination in the loop is a bit gross :)

I think the best solution is to just pull it out into a separate
function, it also avoids adding sometimes unused variables to the outer
function, eg:

static int initialise_darn(void)
{
	unsigned long val;

	if (!cpu_has_feature(CPU_FTR_ARCH_300))
		return -ENODEV;

	for (i = 0; i < 10; i++) {
		if (powernv_get_random_darn(&val)) {
			ppc_md.get_random_seed = powernv_get_random_darn;
			return 0;
		}
	}

	return -EIO;
}

cheers