Re: [PATCH v4 2/2] powerpc: Fix compile issue with force DAWR

I agree with all the below and will address in v5.

Mikey

On Tue, 2019-05-28 at 23:28 -0700, Christoph Hellwig wrote:

quoted

+config PPC_DAWR
+	bool
+	default n

"default n" is the default default.  No need to write this line.

quoted

+++ b/arch/powerpc/kernel/dawr.c

@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+// DAWR infrastructure
+//
+// Copyright 2019, Michael Neuling, IBM Corporation.

Normal top of file header should be /* */, //-style comments are only
for the actual SPDX heder line.

quoted

+	/* Send error to user if they hypervisor won't allow us to write DAWR */
+	if ((!dawr_force_enable) &&
+	    (firmware_has_feature(FW_FEATURE_LPAR)) &&
+	    (set_dawr(&null_brk) != H_SUCCESS))

None of the three inner brace sets here are required, and the code
becomes much easier to read without them.

quoted

+		return -1;

What about returning a proper error code?

quoted

+static int __init dawr_force_setup(void)
+{
+	dawr_force_enable = false;

This variable already is initialized to alse by default, so this line
is not required.

quoted

+	if (PVR_VER(mfspr(SPRN_PVR)) == PVR_POWER9) {
+		/* Turn DAWR off by default, but allow admin to turn it on */
+		dawr_force_enable = false;

.. and neither is this one.