[PATCH v2] fsl-ifc: fix compilation error on ARM's 42 bit VA

[PATCH v2] fsl-ifc: fix compilation error on ARM's 42 bit VA

[Lijun Pan]

Need to include sched.h to fix the following compilation error
if FSL_IFC is enabled on ARM64 machine.
Also, sort the include files by alphabetical order.

In file included from include/linux/mmzone.h:9:0,
                 from include/linux/gfp.h:5,
                 from include/linux/kmod.h:22,
                 from include/linux/module.h:13,
                 from drivers/memory/fsl_ifc.c:22:
drivers/memory/fsl_ifc.c: In function ‘check_nand_stat’:
include/linux/wait.h:165:35: error: ‘TASK_NORMAL’ undeclared (first use in this function)
 #define wake_up(x)   __wake_up(x, TASK_NORMAL, 1, NULL)
                                   ^
drivers/memory/fsl_ifc.c:136:3: note: in expansion of macro ‘wake_up’
   wake_up(&ctrl->nand_wait);
   ^
include/linux/wait.h:165:35: note: each undeclared identifier is reported only once for each function it appears in
 #define wake_up(x)   __wake_up(x, TASK_NORMAL, 1, NULL)
                                   ^
drivers/memory/fsl_ifc.c:136:3: note: in expansion of macro ‘wake_up’
   wake_up(&ctrl->nand_wait);
   ^

Analysis is as follows:
I put some instrumental code and get the
following .h files inclusion sequence:

In file included from ./arch/arm64/include/asm/compat.h:25:0,
                 from ./arch/arm64/include/asm/stat.h:23,
                 from include/linux/stat.h:5,
                 from include/linux/module.h:10,
                 from drivers/memory/fsl_ifc.c:23:
include/linux/sched.h:113:1: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘struct’
 struct sched_attr {
 ^

When 42 bit VA is selected, it does not enable CONFIG_COMPAT=y

In ./arch/arm64/include/asm/stat.h:23, it has
"#ifdef CONFIG_COMPAT"
"#include <asm/compat.h>"
"..."
"#endif"

Since ./arch/arm64/include/asm/stat.h does not
include ./arch/arm64/include/asm/compat.h,
it will not include include/linux/sched.h
Hence we have to manually add "#include <linux/sched.h>"
in drivers/memory/fsl_ifc.c

Signed-off-by: Lijun Pan <Lijun.Pan@freescale.com>
---
v2: reordering the include .h files according to Arnd's suggestion
    detailing why linux/sched.h is needed.
 
 drivers/memory/fsl_ifc.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/memory/fsl_ifc.c b/drivers/memory/fsl_ifc.c
index e87459f..defd62e 100644
--- a/drivers/memory/fsl_ifc.c
+++ b/drivers/memory/fsl_ifc.c
@@ -19,17 +19,18 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
-#include <linux/module.h>
-#include <linux/kernel.h>
 #include <linux/compiler.h>
-#include <linux/spinlock.h>
-#include <linux/types.h>
-#include <linux/slab.h>
+#include <linux/fsl_ifc.h>
 #include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
-#include <linux/fsl_ifc.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
 #include <asm/prom.h>
 
 struct fsl_ifc_ctrl *fsl_ifc_ctrl_dev;
-- 
2.3.3

Re: [PATCH v2] fsl-ifc: fix compilation error on ARM's 42 bit VA

[Arnd Bergmann]

On Wednesday 30 September 2015 17:55:42 Lijun Pan wrote:
> Need to include sched.h to fix the following compilation error
> if FSL_IFC is enabled on ARM64 machine.

That's good, but you did not change the subject line accordingly.

> Also, sort the include files by alphabetical order.

No need to reorder the other lines, just try to fit it in as good
as possible.

> Since ./arch/arm64/include/asm/stat.h does not
> include ./arch/arm64/include/asm/compat.h,
> it will not include include/linux/sched.h
> Hence we have to manually add "#include <linux/sched.h>"
> in drivers/memory/fsl_ifc.c

Ok, good job explaining what happened.

	Arnd

Re: [PATCH v2] fsl-ifc: fix compilation error on ARM's 42 bit VA

[Arnd Bergmann]

On Thursday 01 October 2015 10:55:16 Lijun Pan wrote:
> Lijun Pan
> On Oct 1, 2015 3:34 AM, "Arnd Bergmann" <arnd@arndb.de> wrote:
> >
> > On Wednesday 30 September 2015 17:55:42 Lijun Pan wrote:
> > > Need to include sched.h to fix the following compilation error
> > > if FSL_IFC is enabled on ARM64 machine.
> >
> > That's good, but you did not change the subject line accordingly.
> >
> 
> I actually add 'ARM's ' in the subject.
> How about I change it to
> fsl-ifc: fix compiler error with ARMv8's 42 bit VA and FSL_IFC enabled
> I assume above subject is longer than 65 characters. Is that OK to you?
> Do you have a better subject in mind?
> 
> Thanks for your kind review.

I think the '42 bit VA' part is what is actually misleading, that
is the part that confused me. From your description it sounds like
the problem is CONFIG_COMPAT being disabled, and that would
also be possible with 39-bit VA. Also, it would be clearer to
mention ARM64 instead of ARM, as from the kernel's perspective
those are two different architectures.

How about simply

fsl-ifc: add missing include on ARM64

that would be sufficient for readers, and they can get the
background from the changelog text. If you want to get more
specific, it could be

fsl-ifc: add missing include on ARM64 without CONFIG_COMPAT

Note that 'missing include' more or less implies a compile error
and better describes /what/ the patch does while the changelog
describes why we do it (the compiler output and why that happened).

	Arnd