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

From: Christoph Hellwig <[email-masked]>
Date: Tue, 28 May 2019 23:28:42 -0700

> +config PPC_DAWR
> +	bool
> +	default n

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

> +++ 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.

> +	/* 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.

> +		return -1;

What about returning a proper error code?

> +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.

> +	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.