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

[Michael Neuling]

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

Mikey

On Tue, 2019-05-28 at 23:28 -0700, Christoph Hellwig wrote:
> > +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.
>