Re: [PATCH] kbuild: llvmlinux: Set appropriate compiler-flag for CONFIG_CC_OPTIMIZE_FOR_SIZE

[Behan Webster]

On 09/14/15 22:07, Sedat Dilek wrote:

On Tue, Sep 15, 2015 at 5:40 AM, Behan Webster
 wrote:

I haven't upstreamed this patch yet because we are still characterizing its
effects on other architectures.

Then embed this information into the changelog of *your* patch.
It's in our tree/build system, as a work in progress. Which is why I 
haven't upstreamed it yet.



So what is *your* solution in case of CONFIG_CC_OPTIMIZE_FOR_SIZE=y
(OptLevel '-Os' is wrong, '-Oz' is correct)?

I don't have one yet, which is precisely why I haven't upstreamed it yet.

Sedat, why don't you try working with us instead of making demands and 
accusing people?


Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] kbuild: llvmlinux: Set appropriate compiler-flag for CONFIG_CC_OPTIMIZE_FOR_SIZE

[Behan Webster]

Other than you changing the commit comment, this *is* my patch.

I haven't upstreamed this patch yet because we are still characterizing 
its effects on other architectures.


Clang certainly doesn't require -Oz to be used to be able to boot for 
arm nor arm64. So the commit message isn't correct either.


NAK

Behan

On 09/14/15 21:19, Sedat Dilek wrote:

Based on a patch of Behan Webster (see [1]).

CLANG (here: v3.7) requires '-Oz' as OptLevel to be set.

A Linux v4.3-rc1 kernel built fine with CONFIG_CC_OPTIMIZE_FOR_SIZE=y
and boots on bare metal.
This is a Ubuntu/precise AMD64 system.

Tested against Linux v4.3-rc1 and a refreshed llvmlinux patchset.

[1] 
http://git.linuxfoundation.org/?p=llvmlinux.git;a=blob;f=arch/all/patches/smaller.patch

CC: Behan Webster 
---
  Makefile | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 4249441e535d..a57fb6b39ed7 100644
--- a/Makefile
+++ b/Makefile
@@ -622,7 +622,8 @@ KBUILD_CFLAGS   += $(call 
cc-option,-fno-delete-null-pointer-checks,)
  endif
  
  ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE

-KBUILD_CFLAGS  += -Os $(call cc-disable-warning,maybe-uninitialized,)
+KBUILD_CFLAGS  += $(call cc-option,-Oz,-Os)
+KBUILD_CFLAGS  += $(call cc-disable-warning,maybe-uninitialized,)
  else
  KBUILD_CFLAGS += -O2
  endif



--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] kbuild: llvmlinux: Set appropriate compiler-flag for CONFIG_CC_OPTIMIZE_FOR_SIZE

[Behan Webster]

On 09/14/15 22:07, Sedat Dilek wrote:

On Tue, Sep 15, 2015 at 5:40 AM, Behan Webster
<beh...@converseincode.com> wrote:

I haven't upstreamed this patch yet because we are still characterizing its
effects on other architectures.

Then embed this information into the changelog of *your* patch.
It's in our tree/build system, as a work in progress. Which is why I 
haven't upstreamed it yet.



So what is *your* solution in case of CONFIG_CC_OPTIMIZE_FOR_SIZE=y
(OptLevel '-Os' is wrong, '-Oz' is correct)?

I don't have one yet, which is precisely why I haven't upstreamed it yet.

Sedat, why don't you try working with us instead of making demands and 
accusing people?


Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] kbuild: llvmlinux: Set appropriate compiler-flag for CONFIG_CC_OPTIMIZE_FOR_SIZE

[Behan Webster]

Other than you changing the commit comment, this *is* my patch.

I haven't upstreamed this patch yet because we are still characterizing 
its effects on other architectures.


Clang certainly doesn't require -Oz to be used to be able to boot for 
arm nor arm64. So the commit message isn't correct either.


NAK

Behan

On 09/14/15 21:19, Sedat Dilek wrote:

Based on a patch of Behan Webster (see [1]).

CLANG (here: v3.7) requires '-Oz' as OptLevel to be set.

A Linux v4.3-rc1 kernel built fine with CONFIG_CC_OPTIMIZE_FOR_SIZE=y
and boots on bare metal.
This is a Ubuntu/precise AMD64 system.

Tested against Linux v4.3-rc1 and a refreshed llvmlinux patchset.

[1] 
http://git.linuxfoundation.org/?p=llvmlinux.git;a=blob;f=arch/all/patches/smaller.patch

CC: Behan Webster <beh...@converseincode.com>
---
  Makefile | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 4249441e535d..a57fb6b39ed7 100644
--- a/Makefile
+++ b/Makefile
@@ -622,7 +622,8 @@ KBUILD_CFLAGS   += $(call 
cc-option,-fno-delete-null-pointer-checks,)
  endif
  
  ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE

-KBUILD_CFLAGS  += -Os $(call cc-disable-warning,maybe-uninitialized,)
+KBUILD_CFLAGS  += $(call cc-option,-Oz,-Os)
+KBUILD_CFLAGS  += $(call cc-disable-warning,maybe-uninitialized,)
  else
  KBUILD_CFLAGS += -O2
  endif



--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] Makefile: Fix detection of clang when cross-compiling

[Behan Webster]

On 08/19/15 08:41, Michal Marek wrote:

On Mon, Jul 13, 2015 at 08:59:33PM +1000, Anton Blanchard wrote:

Hi,


  > When the host's C compiler is clang, and when attempting to
  > cross-compile Linux e.g. to MIPS with mipsel-linux-gcc, the
  > Makefile would incorrectly detect the use of clang, which
  > resulted in clang-specific flags being passed to
  > mipsel-linux-gcc.
  >
  > This can be verified under Debian by installing the "clang"
  > package, and then using it as the default compiler with:
  > sudo update-alternatives --config cc
  >
  > This patch moves the detection of clang after the $(CC)
  > variable is initialized to the name of the cross-compiler, so
  > that the check applies
  > to the cross-compiler and not the host's C compiler.
  >
  > v2: Move the detection of clang after the inclusion of the
  > arch/*/Makefile (as they might set $(CROSS_COMPILE))
  >
  > Signed-off-by: Paul Cercueil mailto:p...@crapouillou.net>>

 Applied to kbuild.git#kbuild. I will push it after v4.1-rc1
 becomes available, though.

Drat. I wish I saw this earlier.

This breaks patches which check for the value of COMPILER in
arch/*/Makefile. This detection must be performed before the
inclusion of the arch Makefile.

Can I move this to after the initialization of CC but before the
include?

I'm not sure that being able to define the default compiler per arch
is necessary. But I know I need to be able to add arch specific flags
for clang.

I can confirm the patch breaks ppc64le clang builds.

Can you try the (untested) patch below?


 From 49bb3e66a9a7fc3685fb070bdfd31f2c3d78cc87 Mon Sep 17 00:00:00 2001
From: Michal Marek 
Date: Wed, 19 Aug 2015 17:36:41 +0200
Subject: [PATCH] kbuild: Fix clang detection

We cannot detect clang before including the arch Makefile, because that
can set the default cross compiler. We also cannot detect clang after
including the arch Makefile, because powerpc wants to know about clang.
Solve this by using an deferred variable. This costs us a few shell
invocations, but this is only a constant number.

Reported-by: Behan Webster 
Reported-by: Anton Blanchard 
Signed-off-by: Michal Marek 
---
  Makefile   | 9 +
  arch/powerpc/Makefile  | 8 
  scripts/Kbuild.include | 4 
  scripts/Makefile.extrawarn | 2 +-
  4 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/Makefile b/Makefile
index 13270c0..5ccbb58 100644
--- a/Makefile
+++ b/Makefile
@@ -661,14 +661,7 @@ endif
  endif
  KBUILD_CFLAGS += $(stackp-flag)
  
-ifeq ($(shell $(CC) -v 2>&1 | grep -c "clang version"), 1)

-COMPILER := clang
-else
-COMPILER := gcc
-endif
-export COMPILER
-
-ifeq ($(COMPILER),clang)
+ifeq ($(cc-name),clang)
  KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,)
  KBUILD_CPPFLAGS += $(call cc-option,-Wno-unknown-warning-option,)
  KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable)
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 05f464e..dfe88896 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -67,7 +67,7 @@ UTS_MACHINE := $(OLDARCH)
  
  ifeq ($(CONFIG_CPU_LITTLE_ENDIAN),y)

  override CC   += -mlittle-endian
-ifneq ($(COMPILER),clang)
+ifneq ($(cc-name),clang)
  override CC   += -mno-strict-align
  endif
  override AS   += -mlittle-endian
@@ -333,7 +333,7 @@ TOUT:= .tmp_gas_check
  # - Require gcc 4.0 or above on 64-bit
  # - gcc-4.2.0 has issues compiling modules on 64-bit
  checkbin:
-   @if test "${COMPILER}" != "clang" \
+   @if test "$(cc-name)" != "clang" \
&& test "$(cc-version)" = "0304" ; then \
if ! /bin/echo mftb 5 | $(AS) -v -mppc -many -o $(TOUT) >/dev/null 
2>&1 ; then \
echo -n '*** ${VERSION}.${PATCHLEVEL} kernels no longer 
build '; \
@@ -342,14 +342,14 @@ checkbin:
false; \
fi ; \
fi
-   @if test "${COMPILER}" != "clang" \
+   @if test "$(cc-name)" != "clang" \
&& test "$(cc-version)" -lt "0400" \
&& test "x${CONFIG_PPC64}" = "xy" ; then \
  echo -n "Sorry, GCC v4.0 or above is required to build " ; \
  echo "the 64-bit powerpc kernel." ; \
  false ; \
  fi
-   @if test "${COMPILER}" != "clang" \
+   @if test "$(cc-name)" != "clang" \
&& test "$(cc-fullversion)" = "040200" \
&& test "x${CONFIG_MODULES}${CONFIG_PPC64}" = "xyy" ; then \
echo -n '*** GCC-4.2.0 cannot compile the 6

Re: [PATCH v2] Makefile: Fix detection of clang when cross-compiling

[Behan Webster]

On 08/19/15 08:41, Michal Marek wrote:

On Mon, Jul 13, 2015 at 08:59:33PM +1000, Anton Blanchard wrote:

Hi,


   When the host's C compiler is clang, and when attempting to
   cross-compile Linux e.g. to MIPS with mipsel-linux-gcc, the
   Makefile would incorrectly detect the use of clang, which
   resulted in clang-specific flags being passed to
   mipsel-linux-gcc.
  
   This can be verified under Debian by installing the clang
   package, and then using it as the default compiler with:
   sudo update-alternatives --config cc
  
   This patch moves the detection of clang after the $(CC)
   variable is initialized to the name of the cross-compiler, so
   that the check applies
   to the cross-compiler and not the host's C compiler.
  
   v2: Move the detection of clang after the inclusion of the
   arch/*/Makefile (as they might set $(CROSS_COMPILE))
  
   Signed-off-by: Paul Cercueil p...@crapouillou.net
 mailto:p...@crapouillou.net

 Applied to kbuild.git#kbuild. I will push it after v4.1-rc1
 becomes available, though.

Drat. I wish I saw this earlier.

This breaks patches which check for the value of COMPILER in
arch/*/Makefile. This detection must be performed before the
inclusion of the arch Makefile.

Can I move this to after the initialization of CC but before the
include?

I'm not sure that being able to define the default compiler per arch
is necessary. But I know I need to be able to add arch specific flags
for clang.

I can confirm the patch breaks ppc64le clang builds.

Can you try the (untested) patch below?


 From 49bb3e66a9a7fc3685fb070bdfd31f2c3d78cc87 Mon Sep 17 00:00:00 2001
From: Michal Marek mma...@suse.com
Date: Wed, 19 Aug 2015 17:36:41 +0200
Subject: [PATCH] kbuild: Fix clang detection

We cannot detect clang before including the arch Makefile, because that
can set the default cross compiler. We also cannot detect clang after
including the arch Makefile, because powerpc wants to know about clang.
Solve this by using an deferred variable. This costs us a few shell
invocations, but this is only a constant number.

Reported-by: Behan Webster beh...@converseincode.com
Reported-by: Anton Blanchard an...@samba.org
Signed-off-by: Michal Marek mma...@suse.com
---
  Makefile   | 9 +
  arch/powerpc/Makefile  | 8 
  scripts/Kbuild.include | 4 
  scripts/Makefile.extrawarn | 2 +-
  4 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/Makefile b/Makefile
index 13270c0..5ccbb58 100644
--- a/Makefile
+++ b/Makefile
@@ -661,14 +661,7 @@ endif
  endif
  KBUILD_CFLAGS += $(stackp-flag)
  
-ifeq ($(shell $(CC) -v 21 | grep -c clang version), 1)

-COMPILER := clang
-else
-COMPILER := gcc
-endif
-export COMPILER
-
-ifeq ($(COMPILER),clang)
+ifeq ($(cc-name),clang)
  KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,)
  KBUILD_CPPFLAGS += $(call cc-option,-Wno-unknown-warning-option,)
  KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable)
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 05f464e..dfe88896 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -67,7 +67,7 @@ UTS_MACHINE := $(OLDARCH)
  
  ifeq ($(CONFIG_CPU_LITTLE_ENDIAN),y)

  override CC   += -mlittle-endian
-ifneq ($(COMPILER),clang)
+ifneq ($(cc-name),clang)
  override CC   += -mno-strict-align
  endif
  override AS   += -mlittle-endian
@@ -333,7 +333,7 @@ TOUT:= .tmp_gas_check
  # - Require gcc 4.0 or above on 64-bit
  # - gcc-4.2.0 has issues compiling modules on 64-bit
  checkbin:
-   @if test ${COMPILER} != clang \
+   @if test $(cc-name) != clang \
 test $(cc-version) = 0304 ; then \
if ! /bin/echo mftb 5 | $(AS) -v -mppc -many -o $(TOUT) /dev/null 
21 ; then \
echo -n '*** ${VERSION}.${PATCHLEVEL} kernels no longer 
build '; \
@@ -342,14 +342,14 @@ checkbin:
false; \
fi ; \
fi
-   @if test ${COMPILER} != clang \
+   @if test $(cc-name) != clang \
 test $(cc-version) -lt 0400 \
 test x${CONFIG_PPC64} = xy ; then \
  echo -n Sorry, GCC v4.0 or above is required to build  ; \
  echo the 64-bit powerpc kernel. ; \
  false ; \
  fi
-   @if test ${COMPILER} != clang \
+   @if test $(cc-name) != clang \
 test $(cc-fullversion) = 040200 \
 test x${CONFIG_MODULES}${CONFIG_PPC64} = xyy ; then \
echo -n '*** GCC-4.2.0 cannot compile the 64-bit powerpc ' ; \
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index d3437b8..3523df6 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -128,6 +128,10 @@ cc-option-align = $(subst -functions=0,,\
  cc-disable-warning = $(call try-run,\
$(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o 
$$TMP,-Wno

Re: [PATCH v2] Makefile: Fix detection of clang when cross-compiling

[Behan Webster]

Resent since gmail HTML-ified my previous email...

On Wed, Apr 22, 2015 at 7:33 AM, Michal Marek <mailto:mma...@suse.cz>> wrote:


   On Fri, Apr 17, 2015 at 11:35:04PM +0200, Paul Cercueil wrote:
> When the host's C compiler is clang, and when attempting to
> cross-compile Linux e.g. to MIPS with mipsel-linux-gcc, the Makefile
> would incorrectly detect the use of clang, which resulted in
> clang-specific flags being passed to mipsel-linux-gcc.
>
> This can be verified under Debian by installing the "clang" package,
> and then using it as the default compiler with:
> sudo update-alternatives --config cc
>
> This patch moves the detection of clang after the $(CC) variable is
> initialized to the name of the cross-compiler, so that the check
   applies
> to the cross-compiler and not the host's C compiler.
>
> v2: Move the detection of clang after the inclusion of the
> arch/*/Makefile (as they might set $(CROSS_COMPILE))
>
> Signed-off-by: Paul Cercueil mailto:p...@crapouillou.net>>

   Applied to kbuild.git#kbuild. I will push it after v4.1-rc1 becomes
   available, though.


Drat. I wish I saw this earlier.

This breaks patches which check for the value of COMPILER in 
arch/*/Makefile. This detection must be performed before the inclusion 
of the arch Makefile.


Can I move this to after the initialization of CC but before the include?

I'm not sure that being able to define the default compiler per arch is 
necessary. But I know I need to be able to add arch specific flags for 
clang.


Behan

--
Behan Webster
beh...@converseincode.com <mailto:beh...@converseincode.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] Makefile: Fix detection of clang when cross-compiling

[Behan Webster]

Resent since gmail HTML-ified my previous email...

On Wed, Apr 22, 2015 at 7:33 AM, Michal Marek mma...@suse.cz 
mailto:mma...@suse.cz wrote:


   On Fri, Apr 17, 2015 at 11:35:04PM +0200, Paul Cercueil wrote:
 When the host's C compiler is clang, and when attempting to
 cross-compile Linux e.g. to MIPS with mipsel-linux-gcc, the Makefile
 would incorrectly detect the use of clang, which resulted in
 clang-specific flags being passed to mipsel-linux-gcc.

 This can be verified under Debian by installing the clang package,
 and then using it as the default compiler with:
 sudo update-alternatives --config cc

 This patch moves the detection of clang after the $(CC) variable is
 initialized to the name of the cross-compiler, so that the check
   applies
 to the cross-compiler and not the host's C compiler.

 v2: Move the detection of clang after the inclusion of the
 arch/*/Makefile (as they might set $(CROSS_COMPILE))

 Signed-off-by: Paul Cercueil p...@crapouillou.net
   mailto:p...@crapouillou.net

   Applied to kbuild.git#kbuild. I will push it after v4.1-rc1 becomes
   available, though.


Drat. I wish I saw this earlier.

This breaks patches which check for the value of COMPILER in 
arch/*/Makefile. This detection must be performed before the inclusion 
of the arch Makefile.


Can I move this to after the initialization of CC but before the include?

I'm not sure that being able to define the default compiler per arch is 
necessary. But I know I need to be able to add arch specific flags for 
clang.


Behan

--
Behan Webster
beh...@converseincode.com mailto:beh...@converseincode.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] net, ethernet, LLVMLinux: Add missing MODULE_DEVICE_TABLE()

[Behan Webster]

On 01/28/15 22:42, David Miller wrote:
> From: Behan Webster 
> Date: Wed, 28 Jan 2015 17:36:14 -0800
>
>> Missing MODULE_DEVICE_TABLE for pci ids from benet driver found by clang.
>>
>> Signed-off-by: Behan Webster 
>> Suggested-by: Arnd Bergmann 
> Why are you removing the device table?
It is defined more than once; removing the duplicate (as Arnd
indicated). My commit message was just completely wrong. Brain fart. Sorry.

> Second of all, your Subject needs to be adjusted, using "net" and
> "LLVMLinux" in your subsystem prefix is not appropriate.  Simply
> "be2net: ", the name of this driver, is sufficient.
Will fix.

I've been in the habit of labelling the patches which go through the
LLVMLinux project like this so they are trivially identifiable in the
subject on lkml for reviewers and in the git log as being patches which
exist because of clang. If it's annoying I certainly don't need to do it.

Behan

-- 
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] net, ethernet, LLVMLinux: Add missing MODULE_DEVICE_TABLE()

[Behan Webster]

On 01/29/15 01:10, Arnd Bergmann wrote:
> On Wednesday 28 January 2015 22:42:28 David Miller wrote:
>> From: Behan Webster 
>> Date: Wed, 28 Jan 2015 17:36:14 -0800
>>
>>> Missing MODULE_DEVICE_TABLE for pci ids from benet driver found by clang.
>>>
>>> Signed-off-by: Behan Webster 
>>> Suggested-by: Arnd Bergmann 
>> Why are you removing the device table?
> Behan took a patch that I did earlier and split it up to add descriptions.
> The patch is correct, but he either misunderstood or misexpressed the
> intention.
I was tired and rushed this submission in my preparation for FOSDEM.
Apologies to all.

I neglected to write the commit log when I first split the patch, and
didn't look hard enough this time.

> This driver has two identical lines that both say 
>
> MODULE_DEVICE_TABLE(pci, be_dev_ids);
This is indeed the case.

> I don't remember the exact symptom, but llvm/clang trips over this, while gcc
> silently ignores the second one.
It claims that it is defined more than once.

Behan

-- 
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] net, ethernet, LLVMLinux: Add missing MODULE_DEVICE_TABLE()

[Behan Webster]

On 01/28/15 22:42, David Miller wrote:
 From: Behan Webster beh...@converseincode.com
 Date: Wed, 28 Jan 2015 17:36:14 -0800

 Missing MODULE_DEVICE_TABLE for pci ids from benet driver found by clang.

 Signed-off-by: Behan Webster beh...@converseincode.com
 Suggested-by: Arnd Bergmann a...@arndb.de
 Why are you removing the device table?
It is defined more than once; removing the duplicate (as Arnd
indicated). My commit message was just completely wrong. Brain fart. Sorry.

 Second of all, your Subject needs to be adjusted, using net and
 LLVMLinux in your subsystem prefix is not appropriate.  Simply
 be2net: , the name of this driver, is sufficient.
Will fix.

I've been in the habit of labelling the patches which go through the
LLVMLinux project like this so they are trivially identifiable in the
subject on lkml for reviewers and in the git log as being patches which
exist because of clang. If it's annoying I certainly don't need to do it.

Behan

-- 
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] net, ethernet, LLVMLinux: Add missing MODULE_DEVICE_TABLE()

[Behan Webster]

On 01/29/15 01:10, Arnd Bergmann wrote:
 On Wednesday 28 January 2015 22:42:28 David Miller wrote:
 From: Behan Webster beh...@converseincode.com
 Date: Wed, 28 Jan 2015 17:36:14 -0800

 Missing MODULE_DEVICE_TABLE for pci ids from benet driver found by clang.

 Signed-off-by: Behan Webster beh...@converseincode.com
 Suggested-by: Arnd Bergmann a...@arndb.de
 Why are you removing the device table?
 Behan took a patch that I did earlier and split it up to add descriptions.
 The patch is correct, but he either misunderstood or misexpressed the
 intention.
I was tired and rushed this submission in my preparation for FOSDEM.
Apologies to all.

I neglected to write the commit log when I first split the patch, and
didn't look hard enough this time.

 This driver has two identical lines that both say 

 MODULE_DEVICE_TABLE(pci, be_dev_ids);
This is indeed the case.

 I don't remember the exact symptom, but llvm/clang trips over this, while gcc
 silently ignores the second one.
It claims that it is defined more than once.

Behan

-- 
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] net, ethernet, LLVMLinux: Add missing MODULE_DEVICE_TABLE()

[Behan Webster]

Missing MODULE_DEVICE_TABLE for pci ids from benet driver found by clang.

Signed-off-by: Behan Webster 
Suggested-by: Arnd Bergmann 
Cc: Arnd Bergmann 
---
 drivers/net/ethernet/emulex/benet/be_main.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_main.c 
b/drivers/net/ethernet/emulex/benet/be_main.c
index d48806b..709400a 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -26,7 +26,6 @@
 #include 
 
 MODULE_VERSION(DRV_VER);
-MODULE_DEVICE_TABLE(pci, be_dev_ids);
 MODULE_DESCRIPTION(DRV_DESC " " DRV_VER);
 MODULE_AUTHOR("Emulex Corporation");
 MODULE_LICENSE("GPL");
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] scsi, be2iscsi, LLVMLinux: Add missing MODULE_DEVICE_TABLE()

[Behan Webster]

Missing MODULE_DEVICE_TABLE for pci ids from be2iscsi driver found by clang.

Signed-off-by: Behan Webster 
Reviewed-by: Mark Charlebois 
Suggested-by: Arnd Bergmann 
Cc: Arnd Bergmann 
---
 drivers/scsi/be2iscsi/be_main.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index f319340..96241b2 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -48,7 +48,6 @@ static unsigned int be_iopoll_budget = 10;
 static unsigned int be_max_phys_size = 64;
 static unsigned int enable_msix = 1;
 
-MODULE_DEVICE_TABLE(pci, beiscsi_pci_id_table);
 MODULE_DESCRIPTION(DRV_DESC " " BUILD_STR);
 MODULE_VERSION(BUILD_STR);
 MODULE_AUTHOR("Emulex Corporation");
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] bcm: address clang inline asm incompatibility

[Behan Webster]

On 01/28/15 12:11, Ard Biesheuvel wrote:
> On 28 January 2015 at 19:38, Ard Biesheuvel  wrote:
>> On 28 January 2015 at 19:27, Alex Elder  wrote:
>>> On 01/28/2015 01:17 PM, Ard Biesheuvel wrote:
>>>> On 28 January 2015 at 17:20, Ard Biesheuvel  
>>>> wrote:
>>>>> On 28 January 2015 at 17:08, Alex Elder  wrote:
>>>>>> On 01/28/2015 10:17 AM, Ard Biesheuvel wrote:
>>>>>>> On 28 January 2015 at 14:11, Alex Elder  wrote:
>>>>>>>> On 01/28/2015 05:15 AM, Ard Biesheuvel wrote:
>>>>>>>>> On 28 January 2015 at 05:18, Behan Webster 
>>>>>>>>>  wrote:
>>>>>>>>>> From: Alex Elder 
>>>>>>>>>>
>>>>>>>>>> My GCC-based build environment likes to call register r12 by the
>>>>>>>>>> name "ip" in inline asm.  Behan Webster informed me that his Clang-
>>>>>>>>>> based build environment likes "r12" instead.
>>>>>>>>>>
>>>>>>>>>> Try to make them both happy.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Alex Elder 
>>>>>>>>>> Signed-off-by: Behan Webster 
>>>>>>>>>> ---
>>>>>>>>>>  arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
>>>>>>>>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c 
>>>>>>>>>> b/arch/arm/mach-bcm/bcm_kona_smc.c
>>>>>>>>>> index a55a7ec..3937bd5 100644
>>>>>>>>>> --- a/arch/arm/mach-bcm/bcm_kona_smc.c
>>>>>>>>>> +++ b/arch/arm/mach-bcm/bcm_kona_smc.c
>>>>>>>>>> @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
>>>>>>>>>>   * request result appropriately.  This result value is found in r0
>>>>>>>>>>   * when the "smc" request completes.
>>>>>>>>>>   */
>>>>>>>>>> +#ifdef __clang__
>>>>>>>>>> +#define R12"r12"
>>>>>>>>>> +#else  /* !__clang__ */
>>>>>>>>>> +#define R12"ip"/* gcc calls r12 "ip" */
>>>>>>>>>> +#endif /* !__clang__ */
>>>>>>>>> Why not just use r12 for both?
>>>>>>>> Yes, that would have been an obvious fix.  But the
>>>>>>>> assembler (in the GCC environment) doesn't accept that.
>>>>>>>>
>>>>>>> Mine has no problems with it at all
>>>>>>>
>>>>>>> $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp 
>>>>>>> -
>>>>>>>
>>>>>>> and grepping for r12 under arch/arm suggests the same
>>>>>> The use of "r12" is fine.  But it's not just the assembler,
>>>>>> I believe it also involves gcc.
>>>>>>
>>>>>> The problem is with the use of the __asmeq(x, y) macro.
>>>>>>
>>>>> Ah right. Apologies for assuming that you had missed something obvious 
>>>>> here.
>>>>> But __asmeq is not the toolchain, it is a local construct #define'd in
>>>>> compiler.h
>>>>>
>>>>>> If I assign the "ip" variable with "r12":
>>>>>> register u32 ip asm("r12"); /* Also called ip */
>>>>>>
>>>>>> Then that's fine.  However, this line then causes an error:
>>>>>> __asmeq("%0", "r12")
>>>>>>
>>>>>> Apparently gcc uses register "ip" when it sees asm("r12").  So
>>>>>> attempting to verify the desired register got used with __asmeq()
>>>>>> causes a string mismatch--"ip" is not equal to "r12".
>>>>>>
>>>>>> So I could use:
>>>>>>
>>>>>> register u32 ip asm("r12"); /* Also called ip */
>>>>>> ...
>>>>>> __asmeq("%0", "ip")
>>>>

Re: [PATCH] bcm: address clang inline asm incompatibility

[Behan Webster]

On 01/28/15 11:38, Ard Biesheuvel wrote:
> On 28 January 2015 at 19:27, Alex Elder  wrote:
>> On 01/28/2015 01:17 PM, Ard Biesheuvel wrote:
>>> On 28 January 2015 at 17:20, Ard Biesheuvel  
>>> wrote:
>>>> On 28 January 2015 at 17:08, Alex Elder  wrote:
>>>>> On 01/28/2015 10:17 AM, Ard Biesheuvel wrote:
>>>>>> On 28 January 2015 at 14:11, Alex Elder  wrote:
>>>>>>> On 01/28/2015 05:15 AM, Ard Biesheuvel wrote:
>>>>>>>> On 28 January 2015 at 05:18, Behan Webster  
>>>>>>>> wrote:
>>>>>>>>> From: Alex Elder 
>>>>>>>>>
>>>>>>>>> My GCC-based build environment likes to call register r12 by the
>>>>>>>>> name "ip" in inline asm.  Behan Webster informed me that his Clang-
>>>>>>>>> based build environment likes "r12" instead.
>>>>>>>>>
>>>>>>>>> Try to make them both happy.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Alex Elder 
>>>>>>>>> Signed-off-by: Behan Webster 
>>>>>>>>> ---
>>>>>>>>>  arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
>>>>>>>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c 
>>>>>>>>> b/arch/arm/mach-bcm/bcm_kona_smc.c
>>>>>>>>> index a55a7ec..3937bd5 100644
>>>>>>>>> --- a/arch/arm/mach-bcm/bcm_kona_smc.c
>>>>>>>>> +++ b/arch/arm/mach-bcm/bcm_kona_smc.c
>>>>>>>>> @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
>>>>>>>>>   * request result appropriately.  This result value is found in r0
>>>>>>>>>   * when the "smc" request completes.
>>>>>>>>>   */
>>>>>>>>> +#ifdef __clang__
>>>>>>>>> +#define R12"r12"
>>>>>>>>> +#else  /* !__clang__ */
>>>>>>>>> +#define R12"ip"/* gcc calls r12 "ip" */
>>>>>>>>> +#endif /* !__clang__ */
>>>>>>>> Why not just use r12 for both?
>>>>>>> Yes, that would have been an obvious fix.  But the
>>>>>>> assembler (in the GCC environment) doesn't accept that.
>>>>>>>
>>>>>> Mine has no problems with it at all
>>>>>>
>>>>>> $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp -
>>>>>>
>>>>>> and grepping for r12 under arch/arm suggests the same
>>>>> The use of "r12" is fine.  But it's not just the assembler,
>>>>> I believe it also involves gcc.
>>>>>
>>>>> The problem is with the use of the __asmeq(x, y) macro.
>>>>>
>>>> Ah right. Apologies for assuming that you had missed something obvious 
>>>> here.
>>>> But __asmeq is not the toolchain, it is a local construct #define'd in
>>>> compiler.h
>>>>
>>>>> If I assign the "ip" variable with "r12":
>>>>> register u32 ip asm("r12"); /* Also called ip */
>>>>>
>>>>> Then that's fine.  However, this line then causes an error:
>>>>> __asmeq("%0", "r12")
>>>>>
>>>>> Apparently gcc uses register "ip" when it sees asm("r12").  So
>>>>> attempting to verify the desired register got used with __asmeq()
>>>>> causes a string mismatch--"ip" is not equal to "r12".
>>>>>
>>>>> So I could use:
>>>>>
>>>>> register u32 ip asm("r12"); /* Also called ip */
>>>>> ...
>>>>> __asmeq("%0", "ip")
>>>>>
>>>>> And that will build.  But it's a little non-intuitive, and
>>>>> I suspect that clang might (rightfully) have a failure in
>>>>> this __asmeq() call.
>>>>>
>>>> In that case, I would strongly suggest fixing the __asmeq () macro
>>>> instead, and teach it that ("r12","ip") and ("ip","r12") are fine too.
>>>>
>>>> The thing is, inline asm is a dodgy area to begin with in terms of
>>>> clang-to-gcc compatibility. On arm64, we have been seeing issues where
>>>> the width of the register -which is fixed on gcc- is selected based on
>>>> the size of that variable, i.e., an int32 gets a w# register and int64
>>>> gets a x# register. Imagine debugging that, e.g., a str %0, [xx] that
>>>> writes 8 bytes on GCC suddenly only writing 4 bytes when built with
>>>> clang.
>>>>
>>>> If we also start using the preprocessor to conditionalise what is
>>>> emitted by inline asm, the waters get even murkier and it becomes even
>>>> harder to claim parity between the two.
>>>>
>>> Something like this perhaps?
>> So __asmeq() yields true if the register names (strings) are
>> equal, or if one is "ip" and the other is "r12" (in either order).
>>
>> I can't comment on whether it's right in all build environments but
>> this looks OK to me, to handle this special case.
>>
>> I would much rather you generate that patch.  Is that OK?
>>
> Sure, I can cook up a patch if you guys can confirm that it fixes your
> use case. (I tested GCC myself but I don't have clang installed)
>
That appears to work with clang as well.

All in all a much better solution.

Thank you,

Behan

-- 
Behan Webster
beh...@converseincode.com


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] bcm: address clang inline asm incompatibility

[Behan Webster]

On 01/28/15 11:17, Ard Biesheuvel wrote:
> On 28 January 2015 at 17:20, Ard Biesheuvel  wrote:
>> On 28 January 2015 at 17:08, Alex Elder  wrote:
>>> On 01/28/2015 10:17 AM, Ard Biesheuvel wrote:
>>>> On 28 January 2015 at 14:11, Alex Elder  wrote:
>>>>> On 01/28/2015 05:15 AM, Ard Biesheuvel wrote:
>>>>>> On 28 January 2015 at 05:18, Behan Webster  
>>>>>> wrote:
>>>>>>> From: Alex Elder 
>>>>>>>
>>>>>>> My GCC-based build environment likes to call register r12 by the
>>>>>>> name "ip" in inline asm.  Behan Webster informed me that his Clang-
>>>>>>> based build environment likes "r12" instead.
>>>>>>>
>>>>>>> Try to make them both happy.
>>>>>>>
>>>>>>> Signed-off-by: Alex Elder 
>>>>>>> Signed-off-by: Behan Webster 
>>>>>>> ---
>>>>>>>  arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
>>>>>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c 
>>>>>>> b/arch/arm/mach-bcm/bcm_kona_smc.c
>>>>>>> index a55a7ec..3937bd5 100644
>>>>>>> --- a/arch/arm/mach-bcm/bcm_kona_smc.c
>>>>>>> +++ b/arch/arm/mach-bcm/bcm_kona_smc.c
>>>>>>> @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
>>>>>>>   * request result appropriately.  This result value is found in r0
>>>>>>>   * when the "smc" request completes.
>>>>>>>   */
>>>>>>> +#ifdef __clang__
>>>>>>> +#define R12"r12"
>>>>>>> +#else  /* !__clang__ */
>>>>>>> +#define R12"ip"/* gcc calls r12 "ip" */
>>>>>>> +#endif /* !__clang__ */
>>>>>> Why not just use r12 for both?
>>>>> Yes, that would have been an obvious fix.  But the
>>>>> assembler (in the GCC environment) doesn't accept that.
>>>>>
>>>> Mine has no problems with it at all
>>>>
>>>> $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp -
>>>>
>>>> and grepping for r12 under arch/arm suggests the same
>>> The use of "r12" is fine.  But it's not just the assembler,
>>> I believe it also involves gcc.
>>>
>>> The problem is with the use of the __asmeq(x, y) macro.
>>>
>> Ah right. Apologies for assuming that you had missed something obvious here.
>> But __asmeq is not the toolchain, it is a local construct #define'd in
>> compiler.h
>>
>>> If I assign the "ip" variable with "r12":
>>> register u32 ip asm("r12"); /* Also called ip */
>>>
>>> Then that's fine.  However, this line then causes an error:
>>> __asmeq("%0", "r12")
>>>
>>> Apparently gcc uses register "ip" when it sees asm("r12").  So
>>> attempting to verify the desired register got used with __asmeq()
>>> causes a string mismatch--"ip" is not equal to "r12".
>>>
>>> So I could use:
>>>
>>> register u32 ip asm("r12"); /* Also called ip */
>>> ...
>>> __asmeq("%0", "ip")
>>>
>>> And that will build.  But it's a little non-intuitive, and
>>> I suspect that clang might (rightfully) have a failure in
>>> this __asmeq() call.
>>>
>> In that case, I would strongly suggest fixing the __asmeq () macro
>> instead, and teach it that ("r12","ip") and ("ip","r12") are fine too.
>>
>> The thing is, inline asm is a dodgy area to begin with in terms of
>> clang-to-gcc compatibility. On arm64, we have been seeing issues where
>> the width of the register -which is fixed on gcc- is selected based on
>> the size of that variable, i.e., an int32 gets a w# register and int64
>> gets a x# register. Imagine debugging that, e.g., a str %0, [xx] that
>> writes 8 bytes on GCC suddenly only writing 4 bytes when built with
>> clang.
>>
>> If we also start using the preprocessor to conditionalise what is
>> emitted by inline asm, the waters get even murkier and it becomes even
>> harder to claim parity between the two.
>>
> Something like this perhaps?
>
> >8--
> diff --git a/arch/arm/include/asm/compiler.h b/arch/arm/include/asm/compiler.h
> index 8155db2f7fa1..f99c674b3751 100644
> --- a/arch/arm/include/asm/compiler.h
> +++ b/arch/arm/include/asm/compiler.h
> @@ -9,7 +9,8 @@
>   * will cause compilation to stop on mismatch.
>   * (for details, see gcc PR 15089)
>   */
> -#define __asmeq(x, y)  ".ifnc " x "," y " ; .err ; .endif\n\t"
> +#define __asmeq(x, y)  ".ifnc " x "," y " ; .ifnc " x y ",ipr12 ; " \
> +   ".ifnc " x y ",r12ip ; .err ; .endif ; .endif ; .endif\n\t"
>
>
>  #endif /* __ASM_ARM_COMPILER_H */
> >8--
If that is acceptable, that's fine by me.

In principal none of us *want* to use #ifdefs.

Behan

-- 
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] bcm: address clang inline asm incompatibility

[Behan Webster]

On 01/28/15 11:17, Ard Biesheuvel wrote:
 On 28 January 2015 at 17:20, Ard Biesheuvel ard.biesheu...@linaro.org wrote:
 On 28 January 2015 at 17:08, Alex Elder el...@linaro.org wrote:
 On 01/28/2015 10:17 AM, Ard Biesheuvel wrote:
 On 28 January 2015 at 14:11, Alex Elder el...@linaro.org wrote:
 On 01/28/2015 05:15 AM, Ard Biesheuvel wrote:
 On 28 January 2015 at 05:18, Behan Webster beh...@converseincode.com 
 wrote:
 From: Alex Elder el...@linaro.org

 My GCC-based build environment likes to call register r12 by the
 name ip in inline asm.  Behan Webster informed me that his Clang-
 based build environment likes r12 instead.

 Try to make them both happy.

 Signed-off-by: Alex Elder el...@linaro.org
 Signed-off-by: Behan Webster beh...@converseincode.com
 ---
  arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)

 diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c 
 b/arch/arm/mach-bcm/bcm_kona_smc.c
 index a55a7ec..3937bd5 100644
 --- a/arch/arm/mach-bcm/bcm_kona_smc.c
 +++ b/arch/arm/mach-bcm/bcm_kona_smc.c
 @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
   * request result appropriately.  This result value is found in r0
   * when the smc request completes.
   */
 +#ifdef __clang__
 +#define R12r12
 +#else  /* !__clang__ */
 +#define R12ip/* gcc calls r12 ip */
 +#endif /* !__clang__ */
 Why not just use r12 for both?
 Yes, that would have been an obvious fix.  But the
 assembler (in the GCC environment) doesn't accept that.

 Mine has no problems with it at all

 $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp -

 and grepping for r12 under arch/arm suggests the same
 The use of r12 is fine.  But it's not just the assembler,
 I believe it also involves gcc.

 The problem is with the use of the __asmeq(x, y) macro.

 Ah right. Apologies for assuming that you had missed something obvious here.
 But __asmeq is not the toolchain, it is a local construct #define'd in
 compiler.h

 If I assign the ip variable with r12:
 register u32 ip asm(r12); /* Also called ip */

 Then that's fine.  However, this line then causes an error:
 __asmeq(%0, r12)

 Apparently gcc uses register ip when it sees asm(r12).  So
 attempting to verify the desired register got used with __asmeq()
 causes a string mismatch--ip is not equal to r12.

 So I could use:

 register u32 ip asm(r12); /* Also called ip */
 ...
 __asmeq(%0, ip)

 And that will build.  But it's a little non-intuitive, and
 I suspect that clang might (rightfully) have a failure in
 this __asmeq() call.

 In that case, I would strongly suggest fixing the __asmeq () macro
 instead, and teach it that (r12,ip) and (ip,r12) are fine too.

 The thing is, inline asm is a dodgy area to begin with in terms of
 clang-to-gcc compatibility. On arm64, we have been seeing issues where
 the width of the register -which is fixed on gcc- is selected based on
 the size of that variable, i.e., an int32 gets a w# register and int64
 gets a x# register. Imagine debugging that, e.g., a str %0, [xx] that
 writes 8 bytes on GCC suddenly only writing 4 bytes when built with
 clang.

 If we also start using the preprocessor to conditionalise what is
 emitted by inline asm, the waters get even murkier and it becomes even
 harder to claim parity between the two.

 Something like this perhaps?

 8--
 diff --git a/arch/arm/include/asm/compiler.h b/arch/arm/include/asm/compiler.h
 index 8155db2f7fa1..f99c674b3751 100644
 --- a/arch/arm/include/asm/compiler.h
 +++ b/arch/arm/include/asm/compiler.h
 @@ -9,7 +9,8 @@
   * will cause compilation to stop on mismatch.
   * (for details, see gcc PR 15089)
   */
 -#define __asmeq(x, y)  .ifnc  x , y  ; .err ; .endif\n\t
 +#define __asmeq(x, y)  .ifnc  x , y  ; .ifnc  x y ,ipr12 ;  \
 +   .ifnc  x y ,r12ip ; .err ; .endif ; .endif ; .endif\n\t


  #endif /* __ASM_ARM_COMPILER_H */
 8--
If that is acceptable, that's fine by me.

In principal none of us *want* to use #ifdefs.

Behan

-- 
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] bcm: address clang inline asm incompatibility

[Behan Webster]

On 01/28/15 11:38, Ard Biesheuvel wrote:
 On 28 January 2015 at 19:27, Alex Elder el...@linaro.org wrote:
 On 01/28/2015 01:17 PM, Ard Biesheuvel wrote:
 On 28 January 2015 at 17:20, Ard Biesheuvel ard.biesheu...@linaro.org 
 wrote:
 On 28 January 2015 at 17:08, Alex Elder el...@linaro.org wrote:
 On 01/28/2015 10:17 AM, Ard Biesheuvel wrote:
 On 28 January 2015 at 14:11, Alex Elder el...@linaro.org wrote:
 On 01/28/2015 05:15 AM, Ard Biesheuvel wrote:
 On 28 January 2015 at 05:18, Behan Webster beh...@converseincode.com 
 wrote:
 From: Alex Elder el...@linaro.org

 My GCC-based build environment likes to call register r12 by the
 name ip in inline asm.  Behan Webster informed me that his Clang-
 based build environment likes r12 instead.

 Try to make them both happy.

 Signed-off-by: Alex Elder el...@linaro.org
 Signed-off-by: Behan Webster beh...@converseincode.com
 ---
  arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)

 diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c 
 b/arch/arm/mach-bcm/bcm_kona_smc.c
 index a55a7ec..3937bd5 100644
 --- a/arch/arm/mach-bcm/bcm_kona_smc.c
 +++ b/arch/arm/mach-bcm/bcm_kona_smc.c
 @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
   * request result appropriately.  This result value is found in r0
   * when the smc request completes.
   */
 +#ifdef __clang__
 +#define R12r12
 +#else  /* !__clang__ */
 +#define R12ip/* gcc calls r12 ip */
 +#endif /* !__clang__ */
 Why not just use r12 for both?
 Yes, that would have been an obvious fix.  But the
 assembler (in the GCC environment) doesn't accept that.

 Mine has no problems with it at all

 $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp -

 and grepping for r12 under arch/arm suggests the same
 The use of r12 is fine.  But it's not just the assembler,
 I believe it also involves gcc.

 The problem is with the use of the __asmeq(x, y) macro.

 Ah right. Apologies for assuming that you had missed something obvious 
 here.
 But __asmeq is not the toolchain, it is a local construct #define'd in
 compiler.h

 If I assign the ip variable with r12:
 register u32 ip asm(r12); /* Also called ip */

 Then that's fine.  However, this line then causes an error:
 __asmeq(%0, r12)

 Apparently gcc uses register ip when it sees asm(r12).  So
 attempting to verify the desired register got used with __asmeq()
 causes a string mismatch--ip is not equal to r12.

 So I could use:

 register u32 ip asm(r12); /* Also called ip */
 ...
 __asmeq(%0, ip)

 And that will build.  But it's a little non-intuitive, and
 I suspect that clang might (rightfully) have a failure in
 this __asmeq() call.

 In that case, I would strongly suggest fixing the __asmeq () macro
 instead, and teach it that (r12,ip) and (ip,r12) are fine too.

 The thing is, inline asm is a dodgy area to begin with in terms of
 clang-to-gcc compatibility. On arm64, we have been seeing issues where
 the width of the register -which is fixed on gcc- is selected based on
 the size of that variable, i.e., an int32 gets a w# register and int64
 gets a x# register. Imagine debugging that, e.g., a str %0, [xx] that
 writes 8 bytes on GCC suddenly only writing 4 bytes when built with
 clang.

 If we also start using the preprocessor to conditionalise what is
 emitted by inline asm, the waters get even murkier and it becomes even
 harder to claim parity between the two.

 Something like this perhaps?
 So __asmeq() yields true if the register names (strings) are
 equal, or if one is ip and the other is r12 (in either order).

 I can't comment on whether it's right in all build environments but
 this looks OK to me, to handle this special case.

 I would much rather you generate that patch.  Is that OK?

 Sure, I can cook up a patch if you guys can confirm that it fixes your
 use case. (I tested GCC myself but I don't have clang installed)

That appears to work with clang as well.

All in all a much better solution.

Thank you,

Behan

-- 
Behan Webster
beh...@converseincode.com


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] scsi, be2iscsi, LLVMLinux: Add missing MODULE_DEVICE_TABLE()

[Behan Webster]

Missing MODULE_DEVICE_TABLE for pci ids from be2iscsi driver found by clang.

Signed-off-by: Behan Webster beh...@converseincode.com
Reviewed-by: Mark Charlebois charl...@gmail.com
Suggested-by: Arnd Bergmann a...@arndb.de
Cc: Arnd Bergmann a...@arndb.de
---
 drivers/scsi/be2iscsi/be_main.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index f319340..96241b2 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -48,7 +48,6 @@ static unsigned int be_iopoll_budget = 10;
 static unsigned int be_max_phys_size = 64;
 static unsigned int enable_msix = 1;
 
-MODULE_DEVICE_TABLE(pci, beiscsi_pci_id_table);
 MODULE_DESCRIPTION(DRV_DESC   BUILD_STR);
 MODULE_VERSION(BUILD_STR);
 MODULE_AUTHOR(Emulex Corporation);
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] net, ethernet, LLVMLinux: Add missing MODULE_DEVICE_TABLE()

[Behan Webster]

Missing MODULE_DEVICE_TABLE for pci ids from benet driver found by clang.

Signed-off-by: Behan Webster beh...@converseincode.com
Suggested-by: Arnd Bergmann a...@arndb.de
Cc: Arnd Bergmann a...@arndb.de
---
 drivers/net/ethernet/emulex/benet/be_main.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_main.c 
b/drivers/net/ethernet/emulex/benet/be_main.c
index d48806b..709400a 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -26,7 +26,6 @@
 #include net/vxlan.h
 
 MODULE_VERSION(DRV_VER);
-MODULE_DEVICE_TABLE(pci, be_dev_ids);
 MODULE_DESCRIPTION(DRV_DESC   DRV_VER);
 MODULE_AUTHOR(Emulex Corporation);
 MODULE_LICENSE(GPL);
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] bcm: address clang inline asm incompatibility

[Behan Webster]

On 01/28/15 12:11, Ard Biesheuvel wrote:
 On 28 January 2015 at 19:38, Ard Biesheuvel ard.biesheu...@linaro.org wrote:
 On 28 January 2015 at 19:27, Alex Elder el...@linaro.org wrote:
 On 01/28/2015 01:17 PM, Ard Biesheuvel wrote:
 On 28 January 2015 at 17:20, Ard Biesheuvel ard.biesheu...@linaro.org 
 wrote:
 On 28 January 2015 at 17:08, Alex Elder el...@linaro.org wrote:
 On 01/28/2015 10:17 AM, Ard Biesheuvel wrote:
 On 28 January 2015 at 14:11, Alex Elder el...@linaro.org wrote:
 On 01/28/2015 05:15 AM, Ard Biesheuvel wrote:
 On 28 January 2015 at 05:18, Behan Webster 
 beh...@converseincode.com wrote:
 From: Alex Elder el...@linaro.org

 My GCC-based build environment likes to call register r12 by the
 name ip in inline asm.  Behan Webster informed me that his Clang-
 based build environment likes r12 instead.

 Try to make them both happy.

 Signed-off-by: Alex Elder el...@linaro.org
 Signed-off-by: Behan Webster beh...@converseincode.com
 ---
  arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)

 diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c 
 b/arch/arm/mach-bcm/bcm_kona_smc.c
 index a55a7ec..3937bd5 100644
 --- a/arch/arm/mach-bcm/bcm_kona_smc.c
 +++ b/arch/arm/mach-bcm/bcm_kona_smc.c
 @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
   * request result appropriately.  This result value is found in r0
   * when the smc request completes.
   */
 +#ifdef __clang__
 +#define R12r12
 +#else  /* !__clang__ */
 +#define R12ip/* gcc calls r12 ip */
 +#endif /* !__clang__ */
 Why not just use r12 for both?
 Yes, that would have been an obvious fix.  But the
 assembler (in the GCC environment) doesn't accept that.

 Mine has no problems with it at all

 $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp 
 -

 and grepping for r12 under arch/arm suggests the same
 The use of r12 is fine.  But it's not just the assembler,
 I believe it also involves gcc.

 The problem is with the use of the __asmeq(x, y) macro.

 Ah right. Apologies for assuming that you had missed something obvious 
 here.
 But __asmeq is not the toolchain, it is a local construct #define'd in
 compiler.h

 If I assign the ip variable with r12:
 register u32 ip asm(r12); /* Also called ip */

 Then that's fine.  However, this line then causes an error:
 __asmeq(%0, r12)

 Apparently gcc uses register ip when it sees asm(r12).  So
 attempting to verify the desired register got used with __asmeq()
 causes a string mismatch--ip is not equal to r12.

 So I could use:

 register u32 ip asm(r12); /* Also called ip */
 ...
 __asmeq(%0, ip)

 And that will build.  But it's a little non-intuitive, and
 I suspect that clang might (rightfully) have a failure in
 this __asmeq() call.

 In that case, I would strongly suggest fixing the __asmeq () macro
 instead, and teach it that (r12,ip) and (ip,r12) are fine too.

 The thing is, inline asm is a dodgy area to begin with in terms of
 clang-to-gcc compatibility. On arm64, we have been seeing issues where
 the width of the register -which is fixed on gcc- is selected based on
 the size of that variable, i.e., an int32 gets a w# register and int64
 gets a x# register. Imagine debugging that, e.g., a str %0, [xx] that
 writes 8 bytes on GCC suddenly only writing 4 bytes when built with
 clang.

 If we also start using the preprocessor to conditionalise what is
 emitted by inline asm, the waters get even murkier and it becomes even
 harder to claim parity between the two.

 Something like this perhaps?
 So __asmeq() yields true if the register names (strings) are
 equal, or if one is ip and the other is r12 (in either order).

 I can't comment on whether it's right in all build environments but
 this looks OK to me, to handle this special case.

 I would much rather you generate that patch.  Is that OK?

 Sure, I can cook up a patch if you guys can confirm that it fixes your
 use case. (I tested GCC myself but I don't have clang installed)

 Actually, if clang is guaranteed to emit the correct register name
 inside the inline asm for register asm variables used in input or
 output constraints, I think it makes sense to #define __asmeq as a nop
 if __clang__ is defined. (Note that __asmeq only exists to work around
 a specific GCC bug)
As far as I'm aware neither clang nor gcc will guarantee this completely
in all places where asmeq has been used. Register assignments are
handled differently, and at different levels of the architecture between
the 2 compilers.

Certainly asmeq has caught these kinds of bad assumptions in a number of
places in the kernel while we've ported to clang (like when naked
functions are used, and the calling convention for -O2 is assumed).

I personally would prefer code which doesn't rely on variables being in
the correct registers. However I'm aware that in the case of the smc
instruction there really isn't a choice, as that's

[PATCH] Mark inline functions as __maybe_unused

[Behan Webster]

clang warns if inline functions aren't used. By making them __maybe_unused
there is no warning for either gcc nor clang.

Signed-off-by: Behan Webster 
Suggested-by: Arnd Bergmann 
Cc: Arnd Bergmann 
Cc: "Christopher Li" 
---
 include/linux/compiler-gcc.h | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 02ae99e..c7b98fb 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -53,14 +53,14 @@
  */
 #if !defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) || \
 !defined(CONFIG_OPTIMIZE_INLINING) || (__GNUC__ < 4)
-# define inlineinline  __attribute__((always_inline)) 
notrace
-# define __inline____inline__  __attribute__((always_inline)) notrace
-# define __inline  __inline__attribute__((always_inline)) notrace
+# define inlineinline  __attribute__((always_inline)) 
notrace __maybe_unused
+# define __inline____inline__  __attribute__((always_inline)) notrace 
__maybe_unused
+# define __inline  __inline__attribute__((always_inline)) notrace 
__maybe_unused
 #else
 /* A lot of inline functions can cause havoc with function tracing */
-# define inlineinline  notrace
-# define __inline____inline__  notrace
-# define __inline  __inlinenotrace
+# define inlineinline  notrace __maybe_unused
+# define __inline____inline__  notrace __maybe_unused
+# define __inline  __inlinenotrace __maybe_unused
 #endif
 
 #define __deprecated   __attribute__((deprecated))
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] bcm: address clang inline asm incompatibility

[Behan Webster]

From: Alex Elder 

My GCC-based build environment likes to call register r12 by the
name "ip" in inline asm.  Behan Webster informed me that his Clang-
based build environment likes "r12" instead.

Try to make them both happy.

Signed-off-by: Alex Elder 
Signed-off-by: Behan Webster 
---
 arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c b/arch/arm/mach-bcm/bcm_kona_smc.c
index a55a7ec..3937bd5 100644
--- a/arch/arm/mach-bcm/bcm_kona_smc.c
+++ b/arch/arm/mach-bcm/bcm_kona_smc.c
@@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
  * request result appropriately.  This result value is found in r0
  * when the "smc" request completes.
  */
+#ifdef __clang__
+#define R12"r12"
+#else  /* !__clang__ */
+#define R12"ip"/* gcc calls r12 "ip" */
+#endif /* !__clang__ */
 static int bcm_kona_do_smc(u32 service_id, u32 buffer_phys)
 {
-   register u32 ip asm("ip");  /* Also called r12 */
+   register u32 ip asm(R12);   /* Also called r12 */
register u32 r0 asm("r0");
register u32 r4 asm("r4");
register u32 r5 asm("r5");
@@ -120,7 +125,7 @@ static int bcm_kona_do_smc(u32 service_id, u32 buffer_phys)
 
asm volatile (
/* Make sure we got the registers we want */
-   __asmeq("%0", "ip")
+   __asmeq("%0", R12)
__asmeq("%1", "r0")
__asmeq("%2", "r4")
__asmeq("%3", "r5")
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] bcm: address clang inline asm incompatibility

[Behan Webster]

From: Alex Elder el...@linaro.org

My GCC-based build environment likes to call register r12 by the
name ip in inline asm.  Behan Webster informed me that his Clang-
based build environment likes r12 instead.

Try to make them both happy.

Signed-off-by: Alex Elder el...@linaro.org
Signed-off-by: Behan Webster beh...@converseincode.com
---
 arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c b/arch/arm/mach-bcm/bcm_kona_smc.c
index a55a7ec..3937bd5 100644
--- a/arch/arm/mach-bcm/bcm_kona_smc.c
+++ b/arch/arm/mach-bcm/bcm_kona_smc.c
@@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
  * request result appropriately.  This result value is found in r0
  * when the smc request completes.
  */
+#ifdef __clang__
+#define R12r12
+#else  /* !__clang__ */
+#define R12ip/* gcc calls r12 ip */
+#endif /* !__clang__ */
 static int bcm_kona_do_smc(u32 service_id, u32 buffer_phys)
 {
-   register u32 ip asm(ip);  /* Also called r12 */
+   register u32 ip asm(R12);   /* Also called r12 */
register u32 r0 asm(r0);
register u32 r4 asm(r4);
register u32 r5 asm(r5);
@@ -120,7 +125,7 @@ static int bcm_kona_do_smc(u32 service_id, u32 buffer_phys)
 
asm volatile (
/* Make sure we got the registers we want */
-   __asmeq(%0, ip)
+   __asmeq(%0, R12)
__asmeq(%1, r0)
__asmeq(%2, r4)
__asmeq(%3, r5)
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] Mark inline functions as __maybe_unused

[Behan Webster]

clang warns if inline functions aren't used. By making them __maybe_unused
there is no warning for either gcc nor clang.

Signed-off-by: Behan Webster beh...@converseincode.com
Suggested-by: Arnd Bergmann a...@arndb.de
Cc: Arnd Bergmann a...@arndb.de
Cc: Christopher Li spa...@chrisli.org
---
 include/linux/compiler-gcc.h | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 02ae99e..c7b98fb 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -53,14 +53,14 @@
  */
 #if !defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) || \
 !defined(CONFIG_OPTIMIZE_INLINING) || (__GNUC__  4)
-# define inlineinline  __attribute__((always_inline)) 
notrace
-# define __inline____inline__  __attribute__((always_inline)) notrace
-# define __inline  __inline__attribute__((always_inline)) notrace
+# define inlineinline  __attribute__((always_inline)) 
notrace __maybe_unused
+# define __inline____inline__  __attribute__((always_inline)) notrace 
__maybe_unused
+# define __inline  __inline__attribute__((always_inline)) notrace 
__maybe_unused
 #else
 /* A lot of inline functions can cause havoc with function tracing */
-# define inlineinline  notrace
-# define __inline____inline__  notrace
-# define __inline  __inlinenotrace
+# define inlineinline  notrace __maybe_unused
+# define __inline____inline__  notrace __maybe_unused
+# define __inline  __inlinenotrace __maybe_unused
 #endif
 
 #define __deprecated   __attribute__((deprecated))
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 4/4] staging, rtl8192e, LLVMLinux: Make static local in inline function const

[Behan Webster]

rtllib_association_req is a (large) inline function which defines 2 constant
static arrays which aren't labelled as const. As a result clang complains with:

non-constant static local variable in inline function may be different in
different files
[-Wstatic-local-in-inline]
  static u8   AironetIeOui[] = {0x00, 0x01, 0x66};
  ^
The solution is making them "static const".

However doing so requires dropping const when being used with struct
octet_string. However the value is used in a const fashion thereafter, so no
harm done.

Signed-off-by: Behan Webster 
Suggested-by: Arnd Bergmann 
Cc: Arnd Bergmann 
---
 drivers/staging/rtl8192e/rtllib_softmac.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c 
b/drivers/staging/rtl8192e/rtllib_softmac.c
index 089a058..e970db4 100644
--- a/drivers/staging/rtl8192e/rtllib_softmac.c
+++ b/drivers/staging/rtl8192e/rtllib_softmac.c
@@ -1311,7 +1311,7 @@ inline struct sk_buff *rtllib_association_req(struct 
rtllib_network *beacon,
}
 
if (beacon->bCkipSupported) {
-   static u8   AironetIeOui[] = {0x00, 0x01, 0x66};
+   static const u8 AironetIeOui[] = {0x00, 0x01, 0x66};
u8  CcxAironetBuf[30];
struct octet_string osCcxAironetIE;
 
@@ -1331,10 +1331,11 @@ inline struct sk_buff *rtllib_association_req(struct 
rtllib_network *beacon,
}
 
if (beacon->bCcxRmEnable) {
-   static u8 CcxRmCapBuf[] = {0x00, 0x40, 0x96, 0x01, 0x01, 0x00};
+   static const u8 CcxRmCapBuf[] = {0x00, 0x40, 0x96, 0x01, 0x01,
+   0x00};
struct octet_string osCcxRmCap;
 
-   osCcxRmCap.Octet = CcxRmCapBuf;
+   osCcxRmCap.Octet = (u8 *) CcxRmCapBuf;
osCcxRmCap.Length = sizeof(CcxRmCapBuf);
tag = skb_put(skb, ccxrm_ie_len);
*tag++ = MFIE_TYPE_GENERIC;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/4] staging, rtl8192e, LLVMLinux: Remove unused inline prototype

[Behan Webster]

rtllib_probe_req is defined as "static inline" in rtllib_softmac.c however it
is declared differently as "extern inline" in rtllib_softmac.h. Since it isn't
used outside of the scope of rtllib_softmac, it makes sense to remove the
incorrect declaration.

Signed-off-by: Behan Webster 
Suggested-by: Arnd Bergmann 
Cc: Arnd Bergmann 
---
 drivers/staging/rtl8192e/rtllib.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/rtl8192e/rtllib.h 
b/drivers/staging/rtl8192e/rtllib.h
index 33995ac..1322782 100644
--- a/drivers/staging/rtl8192e/rtllib.h
+++ b/drivers/staging/rtl8192e/rtllib.h
@@ -2762,7 +2762,6 @@ extern void rtllib_stop_scan(struct rtllib_device *ieee);
 extern bool rtllib_act_scanning(struct rtllib_device *ieee, bool sync_scan);
 extern void rtllib_stop_scan_syncro(struct rtllib_device *ieee);
 extern void rtllib_start_scan_syncro(struct rtllib_device *ieee, u8 is_mesh);
-extern inline struct sk_buff *rtllib_probe_req(struct rtllib_device *ieee);
 extern u8 MgntQuery_MgntFrameTxRate(struct rtllib_device *ieee);
 extern void rtllib_sta_ps_send_null_frame(struct rtllib_device *ieee,
  short pwr);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/4] staging, rtl8192e, LLVMLinux: Remove unused prototype

[Behan Webster]

MgntQuery_MgntFrameTxRate is only used within rtllib_softmac.c, so it really
should be static instead of extern.

Since it is currently extern a warning is generated because a different
function of the same name is defined staticlly in ieee80211_softmac.c

Removing the incorrect extern declaration and defining the rtllib_softmac
version of this routine static fixes the warning.

Signed-off-by: Behan Webster 
Suggested-by: Arnd Bergmann 
Cc: Arnd Bergmann 
---
 drivers/staging/rtl8192e/rtllib.h | 1 -
 drivers/staging/rtl8192e/rtllib_softmac.c | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtllib.h 
b/drivers/staging/rtl8192e/rtllib.h
index 1322782..cef2dc2 100644
--- a/drivers/staging/rtl8192e/rtllib.h
+++ b/drivers/staging/rtl8192e/rtllib.h
@@ -2762,7 +2762,6 @@ extern void rtllib_stop_scan(struct rtllib_device *ieee);
 extern bool rtllib_act_scanning(struct rtllib_device *ieee, bool sync_scan);
 extern void rtllib_stop_scan_syncro(struct rtllib_device *ieee);
 extern void rtllib_start_scan_syncro(struct rtllib_device *ieee, u8 is_mesh);
-extern u8 MgntQuery_MgntFrameTxRate(struct rtllib_device *ieee);
 extern void rtllib_sta_ps_send_null_frame(struct rtllib_device *ieee,
  short pwr);
 extern void rtllib_sta_wakeup(struct rtllib_device *ieee, short nl);
diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c 
b/drivers/staging/rtl8192e/rtllib_softmac.c
index 067a45a..089a058 100644
--- a/drivers/staging/rtl8192e/rtllib_softmac.c
+++ b/drivers/staging/rtl8192e/rtllib_softmac.c
@@ -193,7 +193,7 @@ MgntQuery_TxRateExcludeCCKRates(struct rtllib_device *ieee)
return QueryRate;
 }
 
-u8 MgntQuery_MgntFrameTxRate(struct rtllib_device *ieee)
+static u8 MgntQuery_MgntFrameTxRate(struct rtllib_device *ieee)
 {
struct rt_hi_throughput *pHTInfo = ieee->pHTInfo;
u8 rate;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 0/4] staging, rtl8192e, LLVMLinux: Patches to enable the kernel to be compiled with clang/LLVM

[Behan Webster]

Removing a number of warnings generated from compiling stl8192e with clang.

The LLVMLinux project aims to fully build the Linux kernel using both gcc and
clang (the C front end for the LLVM compiler infrastructure project). 

Behan Webster (4):
  staging, rtl8192e, LLVMLinux: Change extern inline to static inline
  staging, rtl8192e, LLVMLinux: Remove unused inline prototype
  staging, rtl8192e, LLVMLinux: Remove unused prototype
  staging, rtl8192e, LLVMLinux: Make static local in inline function
const

 drivers/staging/rtl8192e/rtllib.h |  6 ++
 drivers/staging/rtl8192e/rtllib_softmac.c | 11 ++-
 2 files changed, 8 insertions(+), 9 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/4] staging, rtl8192e, LLVMLinux: Change extern inline to static inline

[Behan Webster]

With compilers which follow the C99 standard (like modern versions of gcc and
clang), "extern inline" does the opposite thing from older versions of gcc
(emits code for an externally linkable version of the inline function).

"static inline" does the intended behavior in all cases instead.

Signed-off-by: Behan Webster 
Suggested-by: Arnd Bergmann 
Cc: Arnd Bergmann 
---
 drivers/staging/rtl8192e/rtllib.h | 4 ++--
 drivers/staging/rtl8192e/rtllib_softmac.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtllib.h 
b/drivers/staging/rtl8192e/rtllib.h
index 2d82f89..33995ac 100644
--- a/drivers/staging/rtl8192e/rtllib.h
+++ b/drivers/staging/rtl8192e/rtllib.h
@@ -2944,12 +2944,12 @@ void rtllib_softmac_scan_syncro(struct rtllib_device 
*ieee, u8 is_mesh);
 
 extern const long rtllib_wlan_frequencies[];
 
-extern inline void rtllib_increment_scans(struct rtllib_device *ieee)
+static inline void rtllib_increment_scans(struct rtllib_device *ieee)
 {
ieee->scans++;
 }
 
-extern inline int rtllib_get_scans(struct rtllib_device *ieee)
+static inline int rtllib_get_scans(struct rtllib_device *ieee)
 {
return ieee->scans;
 }
diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c 
b/drivers/staging/rtl8192e/rtllib_softmac.c
index abb6729..067a45a 100644
--- a/drivers/staging/rtl8192e/rtllib_softmac.c
+++ b/drivers/staging/rtl8192e/rtllib_softmac.c
@@ -343,7 +343,7 @@ inline void softmac_ps_mgmt_xmit(struct sk_buff *skb,
}
 }
 
-inline struct sk_buff *rtllib_probe_req(struct rtllib_device *ieee)
+static inline struct sk_buff *rtllib_probe_req(struct rtllib_device *ieee)
 {
unsigned int len, rate_len;
u8 *tag;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] staging, rts5208, LLVMLinux: Change extern inline to static inline

[Behan Webster]

With compilers which follow the C99 standard (like modern versions of gcc and
clang), "extern inline" does the opposite thing from older versions of gcc
(emits code for an externally linkable version of the inline function). "static
inline" does the intended behavior in both gcc and clang.

Signed-off-by: Behan Webster 
Suggested-by: Arnd Bergmann 
Cc: Arnd Bergmann 
---
 drivers/staging/rts5208/rtsx_transport.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rts5208/rtsx_transport.h 
b/drivers/staging/rts5208/rtsx_transport.h
index b4b1123..899bc20 100644
--- a/drivers/staging/rts5208/rtsx_transport.h
+++ b/drivers/staging/rts5208/rtsx_transport.h
@@ -46,7 +46,7 @@ void rtsx_add_cmd(struct rtsx_chip *chip,
 void rtsx_send_cmd_no_wait(struct rtsx_chip *chip);
 int rtsx_send_cmd(struct rtsx_chip *chip, u8 card, int timeout);
 
-extern inline u8 *rtsx_get_cmd_data(struct rtsx_chip *chip)
+static inline u8 *rtsx_get_cmd_data(struct rtsx_chip *chip)
 {
 #ifdef CMD_USING_SG
return (u8 *)(chip->host_sg_tbl_ptr);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] staging, rts5208, LLVMLinux: Change extern inline to static inline

[Behan Webster]

With compilers which follow the C99 standard (like modern versions of gcc and
clang), extern inline does the opposite thing from older versions of gcc
(emits code for an externally linkable version of the inline function). static
inline does the intended behavior in both gcc and clang.

Signed-off-by: Behan Webster beh...@converseincode.com
Suggested-by: Arnd Bergmann a...@arndb.de
Cc: Arnd Bergmann a...@arndb.de
---
 drivers/staging/rts5208/rtsx_transport.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rts5208/rtsx_transport.h 
b/drivers/staging/rts5208/rtsx_transport.h
index b4b1123..899bc20 100644
--- a/drivers/staging/rts5208/rtsx_transport.h
+++ b/drivers/staging/rts5208/rtsx_transport.h
@@ -46,7 +46,7 @@ void rtsx_add_cmd(struct rtsx_chip *chip,
 void rtsx_send_cmd_no_wait(struct rtsx_chip *chip);
 int rtsx_send_cmd(struct rtsx_chip *chip, u8 card, int timeout);
 
-extern inline u8 *rtsx_get_cmd_data(struct rtsx_chip *chip)
+static inline u8 *rtsx_get_cmd_data(struct rtsx_chip *chip)
 {
 #ifdef CMD_USING_SG
return (u8 *)(chip-host_sg_tbl_ptr);
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/4] staging, rtl8192e, LLVMLinux: Change extern inline to static inline

[Behan Webster]

With compilers which follow the C99 standard (like modern versions of gcc and
clang), extern inline does the opposite thing from older versions of gcc
(emits code for an externally linkable version of the inline function).

static inline does the intended behavior in all cases instead.

Signed-off-by: Behan Webster beh...@converseincode.com
Suggested-by: Arnd Bergmann a...@arndb.de
Cc: Arnd Bergmann a...@arndb.de
---
 drivers/staging/rtl8192e/rtllib.h | 4 ++--
 drivers/staging/rtl8192e/rtllib_softmac.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtllib.h 
b/drivers/staging/rtl8192e/rtllib.h
index 2d82f89..33995ac 100644
--- a/drivers/staging/rtl8192e/rtllib.h
+++ b/drivers/staging/rtl8192e/rtllib.h
@@ -2944,12 +2944,12 @@ void rtllib_softmac_scan_syncro(struct rtllib_device 
*ieee, u8 is_mesh);
 
 extern const long rtllib_wlan_frequencies[];
 
-extern inline void rtllib_increment_scans(struct rtllib_device *ieee)
+static inline void rtllib_increment_scans(struct rtllib_device *ieee)
 {
ieee-scans++;
 }
 
-extern inline int rtllib_get_scans(struct rtllib_device *ieee)
+static inline int rtllib_get_scans(struct rtllib_device *ieee)
 {
return ieee-scans;
 }
diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c 
b/drivers/staging/rtl8192e/rtllib_softmac.c
index abb6729..067a45a 100644
--- a/drivers/staging/rtl8192e/rtllib_softmac.c
+++ b/drivers/staging/rtl8192e/rtllib_softmac.c
@@ -343,7 +343,7 @@ inline void softmac_ps_mgmt_xmit(struct sk_buff *skb,
}
 }
 
-inline struct sk_buff *rtllib_probe_req(struct rtllib_device *ieee)
+static inline struct sk_buff *rtllib_probe_req(struct rtllib_device *ieee)
 {
unsigned int len, rate_len;
u8 *tag;
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 0/4] staging, rtl8192e, LLVMLinux: Patches to enable the kernel to be compiled with clang/LLVM

[Behan Webster]

Removing a number of warnings generated from compiling stl8192e with clang.

The LLVMLinux project aims to fully build the Linux kernel using both gcc and
clang (the C front end for the LLVM compiler infrastructure project). 

Behan Webster (4):
  staging, rtl8192e, LLVMLinux: Change extern inline to static inline
  staging, rtl8192e, LLVMLinux: Remove unused inline prototype
  staging, rtl8192e, LLVMLinux: Remove unused prototype
  staging, rtl8192e, LLVMLinux: Make static local in inline function
const

 drivers/staging/rtl8192e/rtllib.h |  6 ++
 drivers/staging/rtl8192e/rtllib_softmac.c | 11 ++-
 2 files changed, 8 insertions(+), 9 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/4] staging, rtl8192e, LLVMLinux: Remove unused prototype

[Behan Webster]

MgntQuery_MgntFrameTxRate is only used within rtllib_softmac.c, so it really
should be static instead of extern.

Since it is currently extern a warning is generated because a different
function of the same name is defined staticlly in ieee80211_softmac.c

Removing the incorrect extern declaration and defining the rtllib_softmac
version of this routine static fixes the warning.

Signed-off-by: Behan Webster beh...@converseincode.com
Suggested-by: Arnd Bergmann a...@arndb.de
Cc: Arnd Bergmann a...@arndb.de
---
 drivers/staging/rtl8192e/rtllib.h | 1 -
 drivers/staging/rtl8192e/rtllib_softmac.c | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtllib.h 
b/drivers/staging/rtl8192e/rtllib.h
index 1322782..cef2dc2 100644
--- a/drivers/staging/rtl8192e/rtllib.h
+++ b/drivers/staging/rtl8192e/rtllib.h
@@ -2762,7 +2762,6 @@ extern void rtllib_stop_scan(struct rtllib_device *ieee);
 extern bool rtllib_act_scanning(struct rtllib_device *ieee, bool sync_scan);
 extern void rtllib_stop_scan_syncro(struct rtllib_device *ieee);
 extern void rtllib_start_scan_syncro(struct rtllib_device *ieee, u8 is_mesh);
-extern u8 MgntQuery_MgntFrameTxRate(struct rtllib_device *ieee);
 extern void rtllib_sta_ps_send_null_frame(struct rtllib_device *ieee,
  short pwr);
 extern void rtllib_sta_wakeup(struct rtllib_device *ieee, short nl);
diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c 
b/drivers/staging/rtl8192e/rtllib_softmac.c
index 067a45a..089a058 100644
--- a/drivers/staging/rtl8192e/rtllib_softmac.c
+++ b/drivers/staging/rtl8192e/rtllib_softmac.c
@@ -193,7 +193,7 @@ MgntQuery_TxRateExcludeCCKRates(struct rtllib_device *ieee)
return QueryRate;
 }
 
-u8 MgntQuery_MgntFrameTxRate(struct rtllib_device *ieee)
+static u8 MgntQuery_MgntFrameTxRate(struct rtllib_device *ieee)
 {
struct rt_hi_throughput *pHTInfo = ieee-pHTInfo;
u8 rate;
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/4] staging, rtl8192e, LLVMLinux: Remove unused inline prototype

[Behan Webster]

rtllib_probe_req is defined as static inline in rtllib_softmac.c however it
is declared differently as extern inline in rtllib_softmac.h. Since it isn't
used outside of the scope of rtllib_softmac, it makes sense to remove the
incorrect declaration.

Signed-off-by: Behan Webster beh...@converseincode.com
Suggested-by: Arnd Bergmann a...@arndb.de
Cc: Arnd Bergmann a...@arndb.de
---
 drivers/staging/rtl8192e/rtllib.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/rtl8192e/rtllib.h 
b/drivers/staging/rtl8192e/rtllib.h
index 33995ac..1322782 100644
--- a/drivers/staging/rtl8192e/rtllib.h
+++ b/drivers/staging/rtl8192e/rtllib.h
@@ -2762,7 +2762,6 @@ extern void rtllib_stop_scan(struct rtllib_device *ieee);
 extern bool rtllib_act_scanning(struct rtllib_device *ieee, bool sync_scan);
 extern void rtllib_stop_scan_syncro(struct rtllib_device *ieee);
 extern void rtllib_start_scan_syncro(struct rtllib_device *ieee, u8 is_mesh);
-extern inline struct sk_buff *rtllib_probe_req(struct rtllib_device *ieee);
 extern u8 MgntQuery_MgntFrameTxRate(struct rtllib_device *ieee);
 extern void rtllib_sta_ps_send_null_frame(struct rtllib_device *ieee,
  short pwr);
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 4/4] staging, rtl8192e, LLVMLinux: Make static local in inline function const

[Behan Webster]

rtllib_association_req is a (large) inline function which defines 2 constant
static arrays which aren't labelled as const. As a result clang complains with:

non-constant static local variable in inline function may be different in
different files
[-Wstatic-local-in-inline]
  static u8   AironetIeOui[] = {0x00, 0x01, 0x66};
  ^
The solution is making them static const.

However doing so requires dropping const when being used with struct
octet_string. However the value is used in a const fashion thereafter, so no
harm done.

Signed-off-by: Behan Webster beh...@converseincode.com
Suggested-by: Arnd Bergmann a...@arndb.de
Cc: Arnd Bergmann a...@arndb.de
---
 drivers/staging/rtl8192e/rtllib_softmac.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c 
b/drivers/staging/rtl8192e/rtllib_softmac.c
index 089a058..e970db4 100644
--- a/drivers/staging/rtl8192e/rtllib_softmac.c
+++ b/drivers/staging/rtl8192e/rtllib_softmac.c
@@ -1311,7 +1311,7 @@ inline struct sk_buff *rtllib_association_req(struct 
rtllib_network *beacon,
}
 
if (beacon-bCkipSupported) {
-   static u8   AironetIeOui[] = {0x00, 0x01, 0x66};
+   static const u8 AironetIeOui[] = {0x00, 0x01, 0x66};
u8  CcxAironetBuf[30];
struct octet_string osCcxAironetIE;
 
@@ -1331,10 +1331,11 @@ inline struct sk_buff *rtllib_association_req(struct 
rtllib_network *beacon,
}
 
if (beacon-bCcxRmEnable) {
-   static u8 CcxRmCapBuf[] = {0x00, 0x40, 0x96, 0x01, 0x01, 0x00};
+   static const u8 CcxRmCapBuf[] = {0x00, 0x40, 0x96, 0x01, 0x01,
+   0x00};
struct octet_string osCcxRmCap;
 
-   osCcxRmCap.Octet = CcxRmCapBuf;
+   osCcxRmCap.Octet = (u8 *) CcxRmCapBuf;
osCcxRmCap.Length = sizeof(CcxRmCapBuf);
tag = skb_put(skb, ccxrm_ie_len);
*tag++ = MFIE_TYPE_GENERIC;
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[GIT PULL] LLVMLinux patches for v3.18

[Behan Webster]

These patches remove the use of VLAIS using a new SHASH_DESC_ON_STACK macro.
Some of the previously accepted VLAIS removal patches haven't used this
macro. I will
push new patches to consistently use this macro in all those older cases
for 3.19

The following changes since commit 2d65a9f48fcdf7866aab6457bc707ca233e0c791:

  Merge branch 'drm-next' of git://people.freedesktop.org/~airlied/linux
(2014-10-14 09:39:08 +0200)

are available in the git repository at:

  git://git.linuxfoundation.org/llvmlinux/kernel.git
tags/llvmlinux-for-v3.18

for you to fetch changes up to 4c5c30249452aaebf258751ea4222eba3dd3da4c:

  crypto: LLVMLinux: Remove VLAIS usage from crypto/testmgr.c
(2014-10-14 10:51:24 +0200)


LLVMLinux patches for v3.18


Behan Webster (6):
  crypto: LLVMLinux: Add macro to remove use of VLAIS in crypto code
  crypto: LLVMLinux: Remove VLAIS from crypto/mv_cesa.c
  crypto: LLVMLinux: Remove VLAIS from crypto/n2_core.c
  crypto: LLVMLinux: Remove VLAIS from crypto/omap_sham.c
  crypto: LLVMLinux: Remove VLAIS from crypto/.../qat_algs.c
  security, crypto: LLVMLinux: Remove VLAIS from ima_crypto.c

Jan-Simon Möller (5):
  crypto: LLVMLinux: Remove VLAIS from crypto/ccp/ccp-crypto-sha.c
  crypto, dm: LLVMLinux: Remove VLAIS usage from dm-crypt
  crypto: LLVMLinux: Remove VLAIS usage from crypto/hmac.c
  crypto: LLVMLinux: Remove VLAIS usage from libcrc32c.c
  crypto: LLVMLinux: Remove VLAIS usage from crypto/testmgr.c

Vinícius Tinti (1):
  btrfs: LLVMLinux: Remove VLAIS

 crypto/hmac.c| 25 -
 crypto/testmgr.c | 14 --
 drivers/crypto/ccp/ccp-crypto-sha.c  | 13 -
 drivers/crypto/mv_cesa.c | 41 
 drivers/crypto/n2_core.c | 11 +++-
 drivers/crypto/omap-sham.c   | 28 ---
 drivers/crypto/qat/qat_common/qat_algs.c | 31 ++---
 drivers/md/dm-crypt.c| 34 ++-
 fs/btrfs/hash.c  | 16 +--
 include/crypto/hash.h|  5 
 lib/libcrc32c.c  | 16 +--
 security/integrity/ima/ima_crypto.c  | 47
+---
 12 files changed, 122 insertions(+), 159 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[GIT PULL] LLVMLinux patches for v3.18

[Behan Webster]

These patches remove the use of VLAIS using a new SHASH_DESC_ON_STACK macro.
Some of the previously accepted VLAIS removal patches haven't used this
macro. I will
push new patches to consistently use this macro in all those older cases
for 3.19

The following changes since commit 2d65a9f48fcdf7866aab6457bc707ca233e0c791:

  Merge branch 'drm-next' of git://people.freedesktop.org/~airlied/linux
(2014-10-14 09:39:08 +0200)

are available in the git repository at:

  git://git.linuxfoundation.org/llvmlinux/kernel.git
tags/llvmlinux-for-v3.18

for you to fetch changes up to 4c5c30249452aaebf258751ea4222eba3dd3da4c:

  crypto: LLVMLinux: Remove VLAIS usage from crypto/testmgr.c
(2014-10-14 10:51:24 +0200)


LLVMLinux patches for v3.18


Behan Webster (6):
  crypto: LLVMLinux: Add macro to remove use of VLAIS in crypto code
  crypto: LLVMLinux: Remove VLAIS from crypto/mv_cesa.c
  crypto: LLVMLinux: Remove VLAIS from crypto/n2_core.c
  crypto: LLVMLinux: Remove VLAIS from crypto/omap_sham.c
  crypto: LLVMLinux: Remove VLAIS from crypto/.../qat_algs.c
  security, crypto: LLVMLinux: Remove VLAIS from ima_crypto.c

Jan-Simon Möller (5):
  crypto: LLVMLinux: Remove VLAIS from crypto/ccp/ccp-crypto-sha.c
  crypto, dm: LLVMLinux: Remove VLAIS usage from dm-crypt
  crypto: LLVMLinux: Remove VLAIS usage from crypto/hmac.c
  crypto: LLVMLinux: Remove VLAIS usage from libcrc32c.c
  crypto: LLVMLinux: Remove VLAIS usage from crypto/testmgr.c

Vinícius Tinti (1):
  btrfs: LLVMLinux: Remove VLAIS

 crypto/hmac.c| 25 -
 crypto/testmgr.c | 14 --
 drivers/crypto/ccp/ccp-crypto-sha.c  | 13 -
 drivers/crypto/mv_cesa.c | 41 
 drivers/crypto/n2_core.c | 11 +++-
 drivers/crypto/omap-sham.c   | 28 ---
 drivers/crypto/qat/qat_common/qat_algs.c | 31 ++---
 drivers/md/dm-crypt.c| 34 ++-
 fs/btrfs/hash.c  | 16 +--
 include/crypto/hash.h|  5 
 lib/libcrc32c.c  | 16 +--
 security/integrity/ima/ima_crypto.c  | 47
+---
 12 files changed, 122 insertions(+), 159 deletions(-)

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/2] arm, fbdev, omap2, LLVMLinux: Remove nested function from omap2 dss

[Behan Webster]

On 09/27/14 09:46, Felipe Balbi wrote:

On Fri, Sep 26, 2014 at 06:10:52PM -0700, Behan Webster wrote:

Replace the use of nested functions where a normal function will suffice.

Nested functions are not liked by upstream kernel developers in general. Their
use breaks the use of clang as a compiler, and doesn't make the code any
better.

This code now works for both gcc and clang.

Signed-off-by: Behan Webster 
Suggested-by: Arnd Bergmann 
Cc: Arnd Bergmann 

another one that make sense :-) And probably also deserves
checkpatch/coccinelle/sparse.

Indeed! That would be very appreciated.

The clang static analyzer already points this one out. :)

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/2] arm, fbdev, omap2, LLVMLinux: Remove nested function from omap2 dss

[Behan Webster]

On 09/27/14 09:46, Felipe Balbi wrote:

On Fri, Sep 26, 2014 at 06:10:52PM -0700, Behan Webster wrote:

Replace the use of nested functions where a normal function will suffice.

Nested functions are not liked by upstream kernel developers in general. Their
use breaks the use of clang as a compiler, and doesn't make the code any
better.

This code now works for both gcc and clang.

Signed-off-by: Behan Webster beh...@converseincode.com
Suggested-by: Arnd Bergmann a...@arndb.de
Cc: Arnd Bergmann a...@arndb.de

another one that make sense :-) And probably also deserves
checkpatch/coccinelle/sparse.

Indeed! That would be very appreciated.

The clang static analyzer already points this one out. :)

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] media, platform, LLVMLinux: Remove nested function from ti-vpe

[Behan Webster]

Replace the use of nested functions where a normal function will suffice.

Nested functions are not liked by upstream kernel developers in general. Their
use breaks the use of clang as a compiler, and doesn't make the code any
better.

This code now works for both gcc and clang.

Signed-off-by: Behan Webster 
Suggested-by: Arnd Bergmann 
Cc: Arnd Bergmann 
---
 drivers/media/platform/ti-vpe/csc.c | 8 ++--
 drivers/media/platform/ti-vpe/sc.c  | 8 ++--
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/ti-vpe/csc.c 
b/drivers/media/platform/ti-vpe/csc.c
index 940df40..44fbf41 100644
--- a/drivers/media/platform/ti-vpe/csc.c
+++ b/drivers/media/platform/ti-vpe/csc.c
@@ -93,12 +93,8 @@ void csc_dump_regs(struct csc_data *csc)
 {
struct device *dev = >pdev->dev;
 
-   u32 read_reg(struct csc_data *csc, int offset)
-   {
-   return ioread32(csc->base + offset);
-   }
-
-#define DUMPREG(r) dev_dbg(dev, "%-35s %08x\n", #r, read_reg(csc, CSC_##r))
+#define DUMPREG(r) dev_dbg(dev, "%-35s %08x\n", #r, \
+   ioread32(csc->base + CSC_##r))
 
DUMPREG(CSC00);
DUMPREG(CSC01);
diff --git a/drivers/media/platform/ti-vpe/sc.c 
b/drivers/media/platform/ti-vpe/sc.c
index 6314171..1088381 100644
--- a/drivers/media/platform/ti-vpe/sc.c
+++ b/drivers/media/platform/ti-vpe/sc.c
@@ -24,12 +24,8 @@ void sc_dump_regs(struct sc_data *sc)
 {
struct device *dev = >pdev->dev;
 
-   u32 read_reg(struct sc_data *sc, int offset)
-   {
-   return ioread32(sc->base + offset);
-   }
-
-#define DUMPREG(r) dev_dbg(dev, "%-35s %08x\n", #r, read_reg(sc, CFG_##r))
+#define DUMPREG(r) dev_dbg(dev, "%-35s %08x\n", #r, \
+   ioread32(sc->base + CFG_##r))
 
DUMPREG(SC0);
DUMPREG(SC1);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/2] arm, fbdev, omap2, LLVMLinux: Remove nested function from omapfb

[Behan Webster]

Replace the use of nested functions where a normal function will suffice.

Nested functions are not liked by upstream kernel developers in general. Their
use breaks the use of clang as a compiler, and doesn't make the code any
better.

This code now works for both gcc and clang.

Signed-off-by: Behan Webster 
Suggested-by: Arnd Bergmann 
Cc: Arnd Bergmann 
---
 drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c 
b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
index ec2d132..1587243 100644
--- a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
+++ b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
@@ -273,16 +273,16 @@ static struct omapfb_colormode omapfb_colormodes[] = {
},
 };
 
+static bool cmp_component(struct fb_bitfield *f1, struct fb_bitfield *f2)
+{
+   return f1->length == f2->length &&
+   f1->offset == f2->offset &&
+   f1->msb_right == f2->msb_right;
+}
+
 static bool cmp_var_to_colormode(struct fb_var_screeninfo *var,
struct omapfb_colormode *color)
 {
-   bool cmp_component(struct fb_bitfield *f1, struct fb_bitfield *f2)
-   {
-   return f1->length == f2->length &&
-   f1->offset == f2->offset &&
-   f1->msb_right == f2->msb_right;
-   }
-
if (var->bits_per_pixel == 0 ||
var->red.length == 0 ||
var->blue.length == 0 ||
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 0/2] LLVMLinux: Patches to enable the kernel to be compiled with clang/LLVM

[Behan Webster]

Replace the use of nested functions where a normal function will suffice.

Nested functions are not liked by upstream kernel developers in general. Their
use breaks the use of clang as a compiler, and doesn't make the code any
better.

This code now works for both gcc and clang.

The LLVMLinux project aims to fully build the Linux kernel using both gcc and
clang (the C front end for the LLVM compiler infrastructure project). 

Behan Webster (2):
  arm, fbdev, omap2, LLVMLinux: Remove nested function from omap2 dss
  arm, fbdev, omap2, LLVMLinux: Remove nested function from omapfb

 drivers/video/fbdev/omap2/dss/dispc-compat.c   |  9 +
 drivers/video/fbdev/omap2/dss/manager-sysfs.c  | 16 +---
 drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 14 +++---
 3 files changed, 21 insertions(+), 18 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/2] arm, fbdev, omap2, LLVMLinux: Remove nested function from omap2 dss

[Behan Webster]

Replace the use of nested functions where a normal function will suffice.

Nested functions are not liked by upstream kernel developers in general. Their
use breaks the use of clang as a compiler, and doesn't make the code any
better.

This code now works for both gcc and clang.

Signed-off-by: Behan Webster 
Suggested-by: Arnd Bergmann 
Cc: Arnd Bergmann 
---
 drivers/video/fbdev/omap2/dss/dispc-compat.c  |  9 +
 drivers/video/fbdev/omap2/dss/manager-sysfs.c | 16 +---
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/video/fbdev/omap2/dss/dispc-compat.c 
b/drivers/video/fbdev/omap2/dss/dispc-compat.c
index 83779c2..633c461 100644
--- a/drivers/video/fbdev/omap2/dss/dispc-compat.c
+++ b/drivers/video/fbdev/omap2/dss/dispc-compat.c
@@ -634,13 +634,14 @@ void dispc_mgr_disable_sync(enum omap_channel channel)
WARN_ON(1);
 }
 
+static inline void dispc_irq_wait_handler(void *data, u32 mask)
+{
+   complete((struct completion *)data);
+}
+
 int omap_dispc_wait_for_irq_interruptible_timeout(u32 irqmask,
unsigned long timeout)
 {
-   void dispc_irq_wait_handler(void *data, u32 mask)
-   {
-   complete((struct completion *)data);
-   }
 
int r;
DECLARE_COMPLETION_ONSTACK(completion);
diff --git a/drivers/video/fbdev/omap2/dss/manager-sysfs.c 
b/drivers/video/fbdev/omap2/dss/manager-sysfs.c
index 37b59fe..a7414fb 100644
--- a/drivers/video/fbdev/omap2/dss/manager-sysfs.c
+++ b/drivers/video/fbdev/omap2/dss/manager-sysfs.c
@@ -44,6 +44,13 @@ static ssize_t manager_display_show(struct 
omap_overlay_manager *mgr, char *buf)
dssdev->name : "");
 }
 
+static int manager_display_match(struct omap_dss_device *dssdev, void *data)
+{
+   const char *str = data;
+
+   return sysfs_streq(dssdev->name, str);
+}
+
 static ssize_t manager_display_store(struct omap_overlay_manager *mgr,
const char *buf, size_t size)
 {
@@ -52,17 +59,12 @@ static ssize_t manager_display_store(struct 
omap_overlay_manager *mgr,
struct omap_dss_device *dssdev = NULL;
struct omap_dss_device *old_dssdev;
 
-   int match(struct omap_dss_device *dssdev, void *data)
-   {
-   const char *str = data;
-   return sysfs_streq(dssdev->name, str);
-   }
-
if (buf[size-1] == '\n')
--len;
 
if (len > 0)
-   dssdev = omap_dss_find_device((void *)buf, match);
+   dssdev = omap_dss_find_device((void *)buf,
+   manager_display_match);
 
if (len > 0 && dssdev == NULL)
return -EINVAL;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] md, sysfs, LLVMLinux: Remove nested function from bcache sysfs

[Behan Webster]

Replace the use of nested functions where a normal function will suffice.

Nested functions are not liked by upstream kernel developers in general. Their
use breaks the use of clang as a compiler, and doesn't make the code any
better.

This code now works for both gcc and clang.

Signed-off-by: Behan Webster 
Suggested-by: Arnd Bergmann 
Cc: Arnd Bergmann 
---
 drivers/md/bcache/sysfs.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/md/bcache/sysfs.c b/drivers/md/bcache/sysfs.c
index b3ff57d..53d8baa 100644
--- a/drivers/md/bcache/sysfs.c
+++ b/drivers/md/bcache/sysfs.c
@@ -731,6 +731,11 @@ static struct attribute *bch_cache_set_internal_files[] = {
 };
 KTYPE(bch_cache_set_internal);
 
+static int __bch_cache_cmp(const void *l, const void *r)
+{
+   return *((uint16_t *) r) - *((uint16_t *) l);
+}
+
 SHOW(__bch_cache)
 {
struct cache *ca = container_of(kobj, struct cache, kobj);
@@ -755,9 +760,6 @@ SHOW(__bch_cache)
   CACHE_REPLACEMENT(>sb));
 
if (attr == _priority_stats) {
-   int cmp(const void *l, const void *r)
-   {   return *((uint16_t *) r) - *((uint16_t *) l); }
-
struct bucket *b;
size_t n = ca->sb.nbuckets, i;
size_t unused = 0, available = 0, dirty = 0, meta = 0;
@@ -786,7 +788,7 @@ SHOW(__bch_cache)
p[i] = ca->buckets[i].prio;
mutex_unlock(>set->bucket_lock);
 
-   sort(p, n, sizeof(uint16_t), cmp, NULL);
+   sort(p, n, sizeof(uint16_t), __bch_cache_cmp, NULL);
 
while (n &&
   !cached[n - 1])
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk, ti, LLVMLinux: Move __init outside of type definition

[Behan Webster]

On 09/26/14 17:55, Felipe Balbi wrote:

On Fri, Sep 26, 2014 at 05:31:48PM -0700, Behan Webster wrote:

As written, the __init for ti_clk_get_div_table is in the middle of the return
type.

The gcc documentation indicates that section attributes should be added to the
end of the function declaration:

   extern void foobar (void) __attribute__ ((section ("bar")));

However gcc seems to be very permissive with where attributes can be placed.
clang on the other hand isn't so permissive, and fails if you put the section
definition in the middle of the return type:

drivers/clk/ti/divider.c:298:28: error: expected ';' after struct
static struct clk_div_table
^
;
drivers/clk/ti/divider.c:298:1: warning: 'static' ignored on this
   declaration [-Wmissing-declarations]
static struct clk_div_table
^
drivers/clk/ti/divider.c:299:9: error: type specifier missing,
   defaults to 'int' [-Werror,-Wimplicit-int]
__init *ti_clk_get_div_table(struct device_node *node)
~~  ^
drivers/clk/ti/divider.c:345:9: warning: incompatible pointer types
   returning 'struct clk_div_table *' from a function with result type 'int 
*' [-Wincompatible-pointer-types]
 return table;
^
drivers/clk/ti/divider.c:419:9: warning: incompatible pointer types
   assigning to 'const struct clk_div_table *' from 'int *' 
[-Wincompatible-pointer-types]
 *table = ti_clk_get_div_table(node);
^ ~~
3 warnings and 2 errors generated.

By convention, most of the kernel code puts section attributes between the
return type and function name. In the case where the return type is a pointer,
it's important to place the '*' on left of the __init.

This updated code works for both gcc and clang.

Signed-off-by: Behan Webster 
Reviewed-by: Mark Charlebois 

makes sense to me:

Reviewed-by: Felipe Balbi 

Thank you.


I wonder if we should add this a Sparse or Coccinelle rule.

+1

I'm hoping it can be added to checkpatch as well.

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] clk, ti, LLVMLinux: Move __init outside of type definition

[Behan Webster]

As written, the __init for ti_clk_get_div_table is in the middle of the return
type.

The gcc documentation indicates that section attributes should be added to the
end of the function declaration:

  extern void foobar (void) __attribute__ ((section ("bar")));

However gcc seems to be very permissive with where attributes can be placed.
clang on the other hand isn't so permissive, and fails if you put the section
definition in the middle of the return type:

drivers/clk/ti/divider.c:298:28: error: expected ';' after struct
static struct clk_div_table
   ^
   ;
drivers/clk/ti/divider.c:298:1: warning: 'static' ignored on this
  declaration [-Wmissing-declarations]
static struct clk_div_table
^
drivers/clk/ti/divider.c:299:9: error: type specifier missing,
  defaults to 'int' [-Werror,-Wimplicit-int]
__init *ti_clk_get_div_table(struct device_node *node)
~~  ^
drivers/clk/ti/divider.c:345:9: warning: incompatible pointer types
  returning 'struct clk_div_table *' from a function with result type 'int 
*' [-Wincompatible-pointer-types]
return table;
   ^
drivers/clk/ti/divider.c:419:9: warning: incompatible pointer types
  assigning to 'const struct clk_div_table *' from 'int *' 
[-Wincompatible-pointer-types]
*table = ti_clk_get_div_table(node);
   ^ ~~
3 warnings and 2 errors generated.

By convention, most of the kernel code puts section attributes between the
return type and function name. In the case where the return type is a pointer,
it's important to place the '*' on left of the __init.

This updated code works for both gcc and clang.

Signed-off-by: Behan Webster 
Reviewed-by: Mark Charlebois 
---
 drivers/clk/ti/divider.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/ti/divider.c b/drivers/clk/ti/divider.c
index a837f70..bff2b5b 100644
--- a/drivers/clk/ti/divider.c
+++ b/drivers/clk/ti/divider.c
@@ -300,8 +300,8 @@ static struct clk *_register_divider(struct device *dev, 
const char *name,
return clk;
 }
 
-static struct clk_div_table
-__init *ti_clk_get_div_table(struct device_node *node)
+static struct clk_div_table *
+__init ti_clk_get_div_table(struct device_node *node)
 {
struct clk_div_table *table;
const __be32 *divspec;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] clk, ti, LLVMLinux: Move __init outside of type definition

[Behan Webster]

As written, the __init for ti_clk_get_div_table is in the middle of the return
type.

The gcc documentation indicates that section attributes should be added to the
end of the function declaration:

  extern void foobar (void) __attribute__ ((section (bar)));

However gcc seems to be very permissive with where attributes can be placed.
clang on the other hand isn't so permissive, and fails if you put the section
definition in the middle of the return type:

drivers/clk/ti/divider.c:298:28: error: expected ';' after struct
static struct clk_div_table
   ^
   ;
drivers/clk/ti/divider.c:298:1: warning: 'static' ignored on this
  declaration [-Wmissing-declarations]
static struct clk_div_table
^
drivers/clk/ti/divider.c:299:9: error: type specifier missing,
  defaults to 'int' [-Werror,-Wimplicit-int]
__init *ti_clk_get_div_table(struct device_node *node)
~~  ^
drivers/clk/ti/divider.c:345:9: warning: incompatible pointer types
  returning 'struct clk_div_table *' from a function with result type 'int 
*' [-Wincompatible-pointer-types]
return table;
   ^
drivers/clk/ti/divider.c:419:9: warning: incompatible pointer types
  assigning to 'const struct clk_div_table *' from 'int *' 
[-Wincompatible-pointer-types]
*table = ti_clk_get_div_table(node);
   ^ ~~
3 warnings and 2 errors generated.

By convention, most of the kernel code puts section attributes between the
return type and function name. In the case where the return type is a pointer,
it's important to place the '*' on left of the __init.

This updated code works for both gcc and clang.

Signed-off-by: Behan Webster beh...@converseincode.com
Reviewed-by: Mark Charlebois charl...@gmail.com
---
 drivers/clk/ti/divider.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/ti/divider.c b/drivers/clk/ti/divider.c
index a837f70..bff2b5b 100644
--- a/drivers/clk/ti/divider.c
+++ b/drivers/clk/ti/divider.c
@@ -300,8 +300,8 @@ static struct clk *_register_divider(struct device *dev, 
const char *name,
return clk;
 }
 
-static struct clk_div_table
-__init *ti_clk_get_div_table(struct device_node *node)
+static struct clk_div_table *
+__init ti_clk_get_div_table(struct device_node *node)
 {
struct clk_div_table *table;
const __be32 *divspec;
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk, ti, LLVMLinux: Move __init outside of type definition

[Behan Webster]

On 09/26/14 17:55, Felipe Balbi wrote:

On Fri, Sep 26, 2014 at 05:31:48PM -0700, Behan Webster wrote:

As written, the __init for ti_clk_get_div_table is in the middle of the return
type.

The gcc documentation indicates that section attributes should be added to the
end of the function declaration:

   extern void foobar (void) __attribute__ ((section (bar)));

However gcc seems to be very permissive with where attributes can be placed.
clang on the other hand isn't so permissive, and fails if you put the section
definition in the middle of the return type:

drivers/clk/ti/divider.c:298:28: error: expected ';' after struct
static struct clk_div_table
^
;
drivers/clk/ti/divider.c:298:1: warning: 'static' ignored on this
   declaration [-Wmissing-declarations]
static struct clk_div_table
^
drivers/clk/ti/divider.c:299:9: error: type specifier missing,
   defaults to 'int' [-Werror,-Wimplicit-int]
__init *ti_clk_get_div_table(struct device_node *node)
~~  ^
drivers/clk/ti/divider.c:345:9: warning: incompatible pointer types
   returning 'struct clk_div_table *' from a function with result type 'int 
*' [-Wincompatible-pointer-types]
 return table;
^
drivers/clk/ti/divider.c:419:9: warning: incompatible pointer types
   assigning to 'const struct clk_div_table *' from 'int *' 
[-Wincompatible-pointer-types]
 *table = ti_clk_get_div_table(node);
^ ~~
3 warnings and 2 errors generated.

By convention, most of the kernel code puts section attributes between the
return type and function name. In the case where the return type is a pointer,
it's important to place the '*' on left of the __init.

This updated code works for both gcc and clang.

Signed-off-by: Behan Webster beh...@converseincode.com
Reviewed-by: Mark Charlebois charl...@gmail.com

makes sense to me:

Reviewed-by: Felipe Balbi ba...@ti.com

Thank you.


I wonder if we should add this a Sparse or Coccinelle rule.

+1

I'm hoping it can be added to checkpatch as well.

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] md, sysfs, LLVMLinux: Remove nested function from bcache sysfs

[Behan Webster]

Replace the use of nested functions where a normal function will suffice.

Nested functions are not liked by upstream kernel developers in general. Their
use breaks the use of clang as a compiler, and doesn't make the code any
better.

This code now works for both gcc and clang.

Signed-off-by: Behan Webster beh...@converseincode.com
Suggested-by: Arnd Bergmann a...@arndb.de
Cc: Arnd Bergmann a...@arndb.de
---
 drivers/md/bcache/sysfs.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/md/bcache/sysfs.c b/drivers/md/bcache/sysfs.c
index b3ff57d..53d8baa 100644
--- a/drivers/md/bcache/sysfs.c
+++ b/drivers/md/bcache/sysfs.c
@@ -731,6 +731,11 @@ static struct attribute *bch_cache_set_internal_files[] = {
 };
 KTYPE(bch_cache_set_internal);
 
+static int __bch_cache_cmp(const void *l, const void *r)
+{
+   return *((uint16_t *) r) - *((uint16_t *) l);
+}
+
 SHOW(__bch_cache)
 {
struct cache *ca = container_of(kobj, struct cache, kobj);
@@ -755,9 +760,6 @@ SHOW(__bch_cache)
   CACHE_REPLACEMENT(ca-sb));
 
if (attr == sysfs_priority_stats) {
-   int cmp(const void *l, const void *r)
-   {   return *((uint16_t *) r) - *((uint16_t *) l); }
-
struct bucket *b;
size_t n = ca-sb.nbuckets, i;
size_t unused = 0, available = 0, dirty = 0, meta = 0;
@@ -786,7 +788,7 @@ SHOW(__bch_cache)
p[i] = ca-buckets[i].prio;
mutex_unlock(ca-set-bucket_lock);
 
-   sort(p, n, sizeof(uint16_t), cmp, NULL);
+   sort(p, n, sizeof(uint16_t), __bch_cache_cmp, NULL);
 
while (n 
   !cached[n - 1])
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/2] arm, fbdev, omap2, LLVMLinux: Remove nested function from omap2 dss

[Behan Webster]

Replace the use of nested functions where a normal function will suffice.

Nested functions are not liked by upstream kernel developers in general. Their
use breaks the use of clang as a compiler, and doesn't make the code any
better.

This code now works for both gcc and clang.

Signed-off-by: Behan Webster beh...@converseincode.com
Suggested-by: Arnd Bergmann a...@arndb.de
Cc: Arnd Bergmann a...@arndb.de
---
 drivers/video/fbdev/omap2/dss/dispc-compat.c  |  9 +
 drivers/video/fbdev/omap2/dss/manager-sysfs.c | 16 +---
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/video/fbdev/omap2/dss/dispc-compat.c 
b/drivers/video/fbdev/omap2/dss/dispc-compat.c
index 83779c2..633c461 100644
--- a/drivers/video/fbdev/omap2/dss/dispc-compat.c
+++ b/drivers/video/fbdev/omap2/dss/dispc-compat.c
@@ -634,13 +634,14 @@ void dispc_mgr_disable_sync(enum omap_channel channel)
WARN_ON(1);
 }
 
+static inline void dispc_irq_wait_handler(void *data, u32 mask)
+{
+   complete((struct completion *)data);
+}
+
 int omap_dispc_wait_for_irq_interruptible_timeout(u32 irqmask,
unsigned long timeout)
 {
-   void dispc_irq_wait_handler(void *data, u32 mask)
-   {
-   complete((struct completion *)data);
-   }
 
int r;
DECLARE_COMPLETION_ONSTACK(completion);
diff --git a/drivers/video/fbdev/omap2/dss/manager-sysfs.c 
b/drivers/video/fbdev/omap2/dss/manager-sysfs.c
index 37b59fe..a7414fb 100644
--- a/drivers/video/fbdev/omap2/dss/manager-sysfs.c
+++ b/drivers/video/fbdev/omap2/dss/manager-sysfs.c
@@ -44,6 +44,13 @@ static ssize_t manager_display_show(struct 
omap_overlay_manager *mgr, char *buf)
dssdev-name : none);
 }
 
+static int manager_display_match(struct omap_dss_device *dssdev, void *data)
+{
+   const char *str = data;
+
+   return sysfs_streq(dssdev-name, str);
+}
+
 static ssize_t manager_display_store(struct omap_overlay_manager *mgr,
const char *buf, size_t size)
 {
@@ -52,17 +59,12 @@ static ssize_t manager_display_store(struct 
omap_overlay_manager *mgr,
struct omap_dss_device *dssdev = NULL;
struct omap_dss_device *old_dssdev;
 
-   int match(struct omap_dss_device *dssdev, void *data)
-   {
-   const char *str = data;
-   return sysfs_streq(dssdev-name, str);
-   }
-
if (buf[size-1] == '\n')
--len;
 
if (len  0)
-   dssdev = omap_dss_find_device((void *)buf, match);
+   dssdev = omap_dss_find_device((void *)buf,
+   manager_display_match);
 
if (len  0  dssdev == NULL)
return -EINVAL;
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/2] arm, fbdev, omap2, LLVMLinux: Remove nested function from omapfb

[Behan Webster]

Replace the use of nested functions where a normal function will suffice.

Nested functions are not liked by upstream kernel developers in general. Their
use breaks the use of clang as a compiler, and doesn't make the code any
better.

This code now works for both gcc and clang.

Signed-off-by: Behan Webster beh...@converseincode.com
Suggested-by: Arnd Bergmann a...@arndb.de
Cc: Arnd Bergmann a...@arndb.de
---
 drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c 
b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
index ec2d132..1587243 100644
--- a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
+++ b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
@@ -273,16 +273,16 @@ static struct omapfb_colormode omapfb_colormodes[] = {
},
 };
 
+static bool cmp_component(struct fb_bitfield *f1, struct fb_bitfield *f2)
+{
+   return f1-length == f2-length 
+   f1-offset == f2-offset 
+   f1-msb_right == f2-msb_right;
+}
+
 static bool cmp_var_to_colormode(struct fb_var_screeninfo *var,
struct omapfb_colormode *color)
 {
-   bool cmp_component(struct fb_bitfield *f1, struct fb_bitfield *f2)
-   {
-   return f1-length == f2-length 
-   f1-offset == f2-offset 
-   f1-msb_right == f2-msb_right;
-   }
-
if (var-bits_per_pixel == 0 ||
var-red.length == 0 ||
var-blue.length == 0 ||
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 0/2] LLVMLinux: Patches to enable the kernel to be compiled with clang/LLVM

[Behan Webster]

Replace the use of nested functions where a normal function will suffice.

Nested functions are not liked by upstream kernel developers in general. Their
use breaks the use of clang as a compiler, and doesn't make the code any
better.

This code now works for both gcc and clang.

The LLVMLinux project aims to fully build the Linux kernel using both gcc and
clang (the C front end for the LLVM compiler infrastructure project). 

Behan Webster (2):
  arm, fbdev, omap2, LLVMLinux: Remove nested function from omap2 dss
  arm, fbdev, omap2, LLVMLinux: Remove nested function from omapfb

 drivers/video/fbdev/omap2/dss/dispc-compat.c   |  9 +
 drivers/video/fbdev/omap2/dss/manager-sysfs.c  | 16 +---
 drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 14 +++---
 3 files changed, 21 insertions(+), 18 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] media, platform, LLVMLinux: Remove nested function from ti-vpe

[Behan Webster]

Replace the use of nested functions where a normal function will suffice.

Nested functions are not liked by upstream kernel developers in general. Their
use breaks the use of clang as a compiler, and doesn't make the code any
better.

This code now works for both gcc and clang.

Signed-off-by: Behan Webster beh...@converseincode.com
Suggested-by: Arnd Bergmann a...@arndb.de
Cc: Arnd Bergmann a...@arndb.de
---
 drivers/media/platform/ti-vpe/csc.c | 8 ++--
 drivers/media/platform/ti-vpe/sc.c  | 8 ++--
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/ti-vpe/csc.c 
b/drivers/media/platform/ti-vpe/csc.c
index 940df40..44fbf41 100644
--- a/drivers/media/platform/ti-vpe/csc.c
+++ b/drivers/media/platform/ti-vpe/csc.c
@@ -93,12 +93,8 @@ void csc_dump_regs(struct csc_data *csc)
 {
struct device *dev = csc-pdev-dev;
 
-   u32 read_reg(struct csc_data *csc, int offset)
-   {
-   return ioread32(csc-base + offset);
-   }
-
-#define DUMPREG(r) dev_dbg(dev, %-35s %08x\n, #r, read_reg(csc, CSC_##r))
+#define DUMPREG(r) dev_dbg(dev, %-35s %08x\n, #r, \
+   ioread32(csc-base + CSC_##r))
 
DUMPREG(CSC00);
DUMPREG(CSC01);
diff --git a/drivers/media/platform/ti-vpe/sc.c 
b/drivers/media/platform/ti-vpe/sc.c
index 6314171..1088381 100644
--- a/drivers/media/platform/ti-vpe/sc.c
+++ b/drivers/media/platform/ti-vpe/sc.c
@@ -24,12 +24,8 @@ void sc_dump_regs(struct sc_data *sc)
 {
struct device *dev = sc-pdev-dev;
 
-   u32 read_reg(struct sc_data *sc, int offset)
-   {
-   return ioread32(sc-base + offset);
-   }
-
-#define DUMPREG(r) dev_dbg(dev, %-35s %08x\n, #r, read_reg(sc, CFG_##r))
+#define DUMPREG(r) dev_dbg(dev, %-35s %08x\n, #r, \
+   ioread32(sc-base + CFG_##r))
 
DUMPREG(SC0);
DUMPREG(SC1);
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] kbuild, LLVMLinux: Add -Werror to cc-option to support clang

[Behan Webster]

On 09/25/14 06:34, Michal Marek wrote:

On 2014-09-24 20:50, Behan Webster wrote:

Getting clang to error on unused flags wasn't trivial (this change broke
a lot of builds apparently). Fortunately we weren't the only ones who
wanted it to behave like gcc in this case. I think it's going to be
*much* harder to do the same for warnings. The argument given by
supporters of the current situation is that if a warning isn't
supported, why break the build? *sigh*

I guess the reason to accept unknown warnings opentions is compatibility
with Makefiles with hardcoded gcc-isms. BTW, GCC at some point started
to ignore unknown -Wno-* options, for everyone's good of course. That's
why we ended up with the cc-disable-warning function. If -W* options for
clang need special care, then it might be a good idea to introduce
cc-warning with the conditional -Werror for clang. There are not that
many places where we add warnings, so the patch would be still short.
That way, the possible silent failure is limited only to warning options
with clang, which is not such a big deal.

I'll try this approach.

Thanks,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3] mmc, sdhci, bcm-kona, LLVMLinux: Remove use of __initconst

[Behan Webster]

The __initconst is in the wrong place, and when moved to the correct place
it uncovers an error where the variable is used by non-init data structures.

Instead merely make them const and put the const in the right spot.

Signed-off-by: Behan Webster 
Reviewed-by: Mark Charlebois 
Acked-by: Arnd Bergmann 
Acked-by: Matt Porter 
---
 drivers/mmc/host/sdhci-bcm-kona.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-bcm-kona.c 
b/drivers/mmc/host/sdhci-bcm-kona.c
index dd780c3..d085dfe 100644
--- a/drivers/mmc/host/sdhci-bcm-kona.c
+++ b/drivers/mmc/host/sdhci-bcm-kona.c
@@ -225,7 +225,7 @@ static struct sdhci_pltfm_data sdhci_pltfm_data_kona = {
SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
 };
 
-static struct __initconst of_device_id sdhci_bcm_kona_of_match[] = {
+static const struct of_device_id sdhci_bcm_kona_of_match[] = {
{ .compatible = "brcm,kona-sdhci"},
{ .compatible = "bcm,kona-sdhci"}, /* deprecated name */
{}
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3] mmc, sdhci, bcm-kona, LLVMLinux: Remove use of __initconst

[Behan Webster]

The __initconst is in the wrong place, and when moved to the correct place
it uncovers an error where the variable is used by non-init data structures.

Instead merely make them const and put the const in the right spot.

Signed-off-by: Behan Webster beh...@converseincode.com
Reviewed-by: Mark Charlebois charl...@gmail.com
Acked-by: Arnd Bergmann a...@arndb.de
Acked-by: Matt Porter mpor...@linaro.org
---
 drivers/mmc/host/sdhci-bcm-kona.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-bcm-kona.c 
b/drivers/mmc/host/sdhci-bcm-kona.c
index dd780c3..d085dfe 100644
--- a/drivers/mmc/host/sdhci-bcm-kona.c
+++ b/drivers/mmc/host/sdhci-bcm-kona.c
@@ -225,7 +225,7 @@ static struct sdhci_pltfm_data sdhci_pltfm_data_kona = {
SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
 };
 
-static struct __initconst of_device_id sdhci_bcm_kona_of_match[] = {
+static const struct of_device_id sdhci_bcm_kona_of_match[] = {
{ .compatible = brcm,kona-sdhci},
{ .compatible = bcm,kona-sdhci}, /* deprecated name */
{}
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] kbuild, LLVMLinux: Add -Werror to cc-option to support clang

[Behan Webster]

On 09/25/14 06:34, Michal Marek wrote:

On 2014-09-24 20:50, Behan Webster wrote:

Getting clang to error on unused flags wasn't trivial (this change broke
a lot of builds apparently). Fortunately we weren't the only ones who
wanted it to behave like gcc in this case. I think it's going to be
*much* harder to do the same for warnings. The argument given by
supporters of the current situation is that if a warning isn't
supported, why break the build? *sigh*

I guess the reason to accept unknown warnings opentions is compatibility
with Makefiles with hardcoded gcc-isms. BTW, GCC at some point started
to ignore unknown -Wno-* options, for everyone's good of course. That's
why we ended up with the cc-disable-warning function. If -W* options for
clang need special care, then it might be a good idea to introduce
cc-warning with the conditional -Werror for clang. There are not that
many places where we add warnings, so the patch would be still short.
That way, the possible silent failure is limited only to warning options
with clang, which is not such a big deal.

I'll try this approach.

Thanks,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: How to build the kernel with Clang?

[Behan Webster]

On 09/18/14 21:10, Masahiro Yamada wrote:

Hi Clang folks,


I'd like to know the status of Clang support
in the Linux mainline.
Still a work in progress.  You still need to use the LLVMLinux patches 
on top of mainline to get it to work. We're upstreaming those patches as 
fast as we can.



I can see some "clang" specific parts in makefiles,
so I guess Clang is already supported to a certain extent.
Some of the required parts are there. But mainline currently won't 
compile out of the box with clang.



I just tried to build with "HOSTCC=clang CC=clang"
but it would not work.

Is there any tips I am missing here?

Try building from our kernel repo listed on http://llvm.linuxfoundation.org

Thanks,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] kbuild, LLVMLinux: Add -Werror to cc-option to support clang

[Behan Webster]

On 09/24/14 05:07, Michal Marek wrote:

On 2014-09-23 21:28, beh...@converseincode.com wrote:

From: Mark Charlebois 

Clang will warn about unknown warnings but will not return false

You mean unknown options, right?
2 kinds of options: flags and warnings. clang used to merely warn about 
unused/unsupported flags/warnings. It now returns errors for unknown 
flags, but not warnings (unless you specify -Werror).


The issue is that a lot of existing projects which use clang expect the 
former behaviour (I agree that makes no sense, but there you go).



unless -Werror is set. GCC will return false if an unknown
warning is passed.

Adding -Werror make both compiler behave the same.

Can you please limit it to the clang case? Add an internal variable that
either contains -Werror or nothing, depending on the compiler.

I can do that. Will fix.


  What I
fear is that if we use -Werror unconditionally and the user (or some
automated build system) decides to add some silly option to KCFLAGS, we
will get silent failures in the cc-option tests.

A valid concern for sure.


BTW, is there a chance that this would be fixed in some later clang
version? Accepting unknown commandline options is a rather unusual
behavior. How are all the ./configure scripts going to cope with it?
Again, clang does error out on unknown compiler flags (as opposed to 
warnings).


Getting clang to error on unused flags wasn't trivial (this change broke 
a lot of builds apparently). Fortunately we weren't the only ones who 
wanted it to behave like gcc in this case. I think it's going to be 
*much* harder to do the same for warnings. The argument given by 
supporters of the current situation is that if a warning isn't 
supported, why break the build? *sigh*


The LLVMLinux project is pushing hard to fix these sorts of things in 
clang, but some changes are more likely than others.


Thanks,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] kbuild, LLVMLinux: Fix asm-offset generation to work with clang

[Behan Webster]

On 09/24/14 03:37, Arnd Bergmann wrote:

On Tuesday 23 September 2014 12:25:31 beh...@converseincode.com wrote:

  #define DEFINE(sym, val) \
-asm volatile("\n->" #sym " %0 " #val : : "i" (val))
+   asm volatile("\n@->" #sym " %0 " #val : : "i" (val))

Isn't the '@' character to start a comment architecture specific?
I had worried about that as well (as we discussed at LCU), but with the 
limited testing I tried this with it seemed to indicate that gas was 
smart enough to handle it in most cases. However the kbuild test robot 
indicates that this patch breaks MIPS. So indeed your concern is justified.


Gotta love the kbuild test robot. Thank you Fengguang Wu!


If this makes it work on ARM, what about other architectures?

For now, I think I need to try something else.

Thanks,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 2/2] mmc, sdhci, bcm-kona, LLVMLinux: Remove use of __initconst

[Behan Webster]

On 09/24/14 02:22, Arnd Bergmann wrote:

On Tuesday 23 September 2014 15:55:08 beh...@converseincode.com wrote:

--- a/drivers/mmc/host/sdhci-bcm-kona.c
+++ b/drivers/mmc/host/sdhci-bcm-kona.c
@@ -225,7 +225,7 @@ static struct sdhci_pltfm_data sdhci_pltfm_data_kona = {
 SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
  };
  
-static struct __initconst of_device_id sdhci_bcm_kona_of_match[] = {

+static struct of_device_id const sdhci_bcm_kona_of_match[] = {
 { .compatible = "brcm,kona-sdhci"},
 { .compatible = "bcm,kona-sdhci"}, /* deprecated name */
 {}


Sorry for giving you trouble over such a simple patch (especially
one that I have acked already), but I just noticed that this is
not following the common style we use in the kernel.
It's all good. It's not like you haven't saved me a tonne of time 
already! :)



Almost everywhere in Linux, we use

static const struct of_device_id sdhci_bcm_kona_of_match[] = {

not

static struct of_device_id const sdhci_bcm_kona_of_match[] = {
True enough. I put the const where I did to be in keeping with the 
intent of __initconst, making the array const instead of the contained type.



AFAICT they behave in identical ways,
Indeed. For C in both cases the resulting array of struct of_device_id 
ends up in .rodata, so functionally equivalent.



  but the first one seems
easier to read for someone familiar with kernel code.

No worries. Happy to post a v3.

Linus Walleij: Would you like me to respin the "gpio, bcm-kona, 
LLVMLinux: Remove use of __initconst" patch as well?


Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] arm, vt8500, LLVMLlinux: Use mcr instead of mcr% for mach-vt8500

[Behan Webster]

On 09/24/14 02:16, Arnd Bergmann wrote:

On Tuesday 23 September 2014 20:44:44 Behan Webster wrote:

The ASM below does not compile with clang and is not the way that the mcr
command is used in other parts of the kernel.

arch/arm/mach-vt8500/vt8500.c:72:11: error: invalid % escape in inline assembly 
string
 asm("mcr%? p15, 0, %0, c7, c0, 4" : : "r" (0));
 ~^~~~
1 error generated.

There are other forms that are supported on different ARM instruction sets but
generally the kernel just uses mcr as it is supported in all ARM instruction
sets.

Just for confirm: both forms are actually correct and we don't need this
backported for stable, right?
My understanding is that the %? carries a condition code to the next 
instruction (which in this case is then ignored). So essentially in this 
situation both are equivalent.



Signed-off-by: Behan Webster 
Reviewed-by: Mark Charlebois 
Acked-by: Will Deacon 

Acked-by: Arnd Bergmann 

Tony, would you like to pick this one up and send it in a pull request
to arm-soc, or should we apply it to fixes-non-critical directly?

Thanks,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] arm, vt8500, LLVMLlinux: Use mcr instead of mcr% for mach-vt8500

[Behan Webster]

On 09/24/14 02:16, Arnd Bergmann wrote:

On Tuesday 23 September 2014 20:44:44 Behan Webster wrote:

The ASM below does not compile with clang and is not the way that the mcr
command is used in other parts of the kernel.

arch/arm/mach-vt8500/vt8500.c:72:11: error: invalid % escape in inline assembly 
string
 asm(mcr%? p15, 0, %0, c7, c0, 4 : : r (0));
 ~^~~~
1 error generated.

There are other forms that are supported on different ARM instruction sets but
generally the kernel just uses mcr as it is supported in all ARM instruction
sets.

Just for confirm: both forms are actually correct and we don't need this
backported for stable, right?
My understanding is that the %? carries a condition code to the next 
instruction (which in this case is then ignored). So essentially in this 
situation both are equivalent.



Signed-off-by: Behan Webster beh...@converseincode.com
Reviewed-by: Mark Charlebois charl...@gmail.com
Acked-by: Will Deacon will.dea...@arm.com

Acked-by: Arnd Bergmann a...@arndb.de

Tony, would you like to pick this one up and send it in a pull request
to arm-soc, or should we apply it to fixes-non-critical directly?

Thanks,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 2/2] mmc, sdhci, bcm-kona, LLVMLinux: Remove use of __initconst

[Behan Webster]

On 09/24/14 02:22, Arnd Bergmann wrote:

On Tuesday 23 September 2014 15:55:08 beh...@converseincode.com wrote:

--- a/drivers/mmc/host/sdhci-bcm-kona.c
+++ b/drivers/mmc/host/sdhci-bcm-kona.c
@@ -225,7 +225,7 @@ static struct sdhci_pltfm_data sdhci_pltfm_data_kona = {
 SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
  };
  
-static struct __initconst of_device_id sdhci_bcm_kona_of_match[] = {

+static struct of_device_id const sdhci_bcm_kona_of_match[] = {
 { .compatible = brcm,kona-sdhci},
 { .compatible = bcm,kona-sdhci}, /* deprecated name */
 {}


Sorry for giving you trouble over such a simple patch (especially
one that I have acked already), but I just noticed that this is
not following the common style we use in the kernel.
It's all good. It's not like you haven't saved me a tonne of time 
already! :)



Almost everywhere in Linux, we use

static const struct of_device_id sdhci_bcm_kona_of_match[] = {

not

static struct of_device_id const sdhci_bcm_kona_of_match[] = {
True enough. I put the const where I did to be in keeping with the 
intent of __initconst, making the array const instead of the contained type.



AFAICT they behave in identical ways,
Indeed. For C in both cases the resulting array of struct of_device_id 
ends up in .rodata, so functionally equivalent.



  but the first one seems
easier to read for someone familiar with kernel code.

No worries. Happy to post a v3.

Linus Walleij: Would you like me to respin the gpio, bcm-kona, 
LLVMLinux: Remove use of __initconst patch as well?


Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] kbuild, LLVMLinux: Fix asm-offset generation to work with clang

[Behan Webster]

On 09/24/14 03:37, Arnd Bergmann wrote:

On Tuesday 23 September 2014 12:25:31 beh...@converseincode.com wrote:

  #define DEFINE(sym, val) \
-asm volatile(\n- #sym  %0  #val : : i (val))
+   asm volatile(\n@- #sym  %0  #val : : i (val))

Isn't the '@' character to start a comment architecture specific?
I had worried about that as well (as we discussed at LCU), but with the 
limited testing I tried this with it seemed to indicate that gas was 
smart enough to handle it in most cases. However the kbuild test robot 
indicates that this patch breaks MIPS. So indeed your concern is justified.


Gotta love the kbuild test robot. Thank you Fengguang Wu!


If this makes it work on ARM, what about other architectures?

For now, I think I need to try something else.

Thanks,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] kbuild, LLVMLinux: Add -Werror to cc-option to support clang

[Behan Webster]

On 09/24/14 05:07, Michal Marek wrote:

On 2014-09-23 21:28, beh...@converseincode.com wrote:

From: Mark Charlebois charl...@gmail.com

Clang will warn about unknown warnings but will not return false

You mean unknown options, right?
2 kinds of options: flags and warnings. clang used to merely warn about 
unused/unsupported flags/warnings. It now returns errors for unknown 
flags, but not warnings (unless you specify -Werror).


The issue is that a lot of existing projects which use clang expect the 
former behaviour (I agree that makes no sense, but there you go).



unless -Werror is set. GCC will return false if an unknown
warning is passed.

Adding -Werror make both compiler behave the same.

Can you please limit it to the clang case? Add an internal variable that
either contains -Werror or nothing, depending on the compiler.

I can do that. Will fix.


  What I
fear is that if we use -Werror unconditionally and the user (or some
automated build system) decides to add some silly option to KCFLAGS, we
will get silent failures in the cc-option tests.

A valid concern for sure.


BTW, is there a chance that this would be fixed in some later clang
version? Accepting unknown commandline options is a rather unusual
behavior. How are all the ./configure scripts going to cope with it?
Again, clang does error out on unknown compiler flags (as opposed to 
warnings).


Getting clang to error on unused flags wasn't trivial (this change broke 
a lot of builds apparently). Fortunately we weren't the only ones who 
wanted it to behave like gcc in this case. I think it's going to be 
*much* harder to do the same for warnings. The argument given by 
supporters of the current situation is that if a warning isn't 
supported, why break the build? *sigh*


The LLVMLinux project is pushing hard to fix these sorts of things in 
clang, but some changes are more likely than others.


Thanks,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: How to build the kernel with Clang?

[Behan Webster]

On 09/18/14 21:10, Masahiro Yamada wrote:

Hi Clang folks,


I'd like to know the status of Clang support
in the Linux mainline.
Still a work in progress.  You still need to use the LLVMLinux patches 
on top of mainline to get it to work. We're upstreaming those patches as 
fast as we can.



I can see some clang specific parts in makefiles,
so I guess Clang is already supported to a certain extent.
Some of the required parts are there. But mainline currently won't 
compile out of the box with clang.



I just tried to build with HOSTCC=clang CC=clang
but it would not work.

Is there any tips I am missing here?

Try building from our kernel repo listed on http://llvm.linuxfoundation.org

Thanks,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] fs, LLVMLinux: Remove warning from COMPATIBLE_IOCTL

[Behan Webster]

From: Mark Charlebois 

cmd in COMPATIBLE_IOCTL is always a u32, so cast it so there isn't a warning
about an overflow in XFORM.

Signed-off-by: Mark Charlebois 
Signed-off-by: Behan Webster 
Acked-by: Arnd Bergmann 
---
 fs/compat_ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index afec645..f6ce1aa 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -810,7 +810,7 @@ static int compat_ioctl_preallocate(struct file *file,
  */
 #define XFORM(i) (((i) ^ ((i) << 27) ^ ((i) << 17)) & 0x)
 
-#define COMPATIBLE_IOCTL(cmd) XFORM(cmd),
+#define COMPATIBLE_IOCTL(cmd) XFORM((u32)cmd),
 /* ioctl should not be warned about even if it's not implemented.
Valid reasons to use this:
- It is implemented with ->compat_ioctl on some device, but programs
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] arm, vt8500, LLVMLlinux: Use mcr instead of mcr% for mach-vt8500

[Behan Webster]

The ASM below does not compile with clang and is not the way that the mcr
command is used in other parts of the kernel.

arch/arm/mach-vt8500/vt8500.c:72:11: error: invalid % escape in inline assembly 
string
asm("mcr%? p15, 0, %0, c7, c0, 4" : : "r" (0));
~^~~~
1 error generated.

There are other forms that are supported on different ARM instruction sets but
generally the kernel just uses mcr as it is supported in all ARM instruction
sets.

Signed-off-by: Behan Webster 
Reviewed-by: Mark Charlebois 
Acked-by: Will Deacon 
---
 arch/arm/mach-vt8500/vt8500.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-vt8500/vt8500.c b/arch/arm/mach-vt8500/vt8500.c
index 2da7be3..3bc0dc9 100644
--- a/arch/arm/mach-vt8500/vt8500.c
+++ b/arch/arm/mach-vt8500/vt8500.c
@@ -69,7 +69,7 @@ static void vt8500_power_off(void)
 {
local_irq_disable();
writew(5, pmc_base + VT8500_HCR_REG);
-   asm("mcr%? p15, 0, %0, c7, c0, 4" : : "r" (0));
+   asm("mcr p15, 0, %0, c7, c0, 4" : : "r" (0));
 }
 
 static void __init vt8500_init(void)
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] gpio, bcm-kona, LLVMLinux: Remove use of __initconst

[Behan Webster]

On 09/23/14 14:29, Matt Porter wrote:

On Tue, Sep 23, 2014 at 12:30:16PM -0700, beh...@converseincode.com wrote:

From: Behan Webster 

The __initconst is in the wrong place, and when moved to the correct place
it uncovers an error where the variable is used by non-init data structures.

Instead merely make them const and put the const in the right spot.

Signed-off-by: Behan Webster 
Reviewed-by: Mark Charlebois 
Acked-by: Arnd Bergmann 
---
  drivers/gpio/gpio-bcm-kona.c  | 2 +-
  drivers/mmc/host/sdhci-bcm-kona.c | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

As mentioned on IRC, Linus, Chris, and Ulf probably would like this
split to go into each respective tree.

Thanks Matt.

I reposted as a 2 patch series. Strictly speaking one patch is for each 
tree.


Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] gpio, bcm-kona, LLVMLinux: Remove use of __initconst

[Behan Webster]

On 09/23/14 14:29, Matt Porter wrote:

On Tue, Sep 23, 2014 at 12:30:16PM -0700, beh...@converseincode.com wrote:

From: Behan Webster beh...@converseincode.com

The __initconst is in the wrong place, and when moved to the correct place
it uncovers an error where the variable is used by non-init data structures.

Instead merely make them const and put the const in the right spot.

Signed-off-by: Behan Webster beh...@converseincode.com
Reviewed-by: Mark Charlebois charl...@gmail.com
Acked-by: Arnd Bergmann a...@arndb.de
---
  drivers/gpio/gpio-bcm-kona.c  | 2 +-
  drivers/mmc/host/sdhci-bcm-kona.c | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

As mentioned on IRC, Linus, Chris, and Ulf probably would like this
split to go into each respective tree.

Thanks Matt.

I reposted as a 2 patch series. Strictly speaking one patch is for each 
tree.


Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] arm, vt8500, LLVMLlinux: Use mcr instead of mcr% for mach-vt8500

[Behan Webster]

The ASM below does not compile with clang and is not the way that the mcr
command is used in other parts of the kernel.

arch/arm/mach-vt8500/vt8500.c:72:11: error: invalid % escape in inline assembly 
string
asm(mcr%? p15, 0, %0, c7, c0, 4 : : r (0));
~^~~~
1 error generated.

There are other forms that are supported on different ARM instruction sets but
generally the kernel just uses mcr as it is supported in all ARM instruction
sets.

Signed-off-by: Behan Webster beh...@converseincode.com
Reviewed-by: Mark Charlebois charl...@gmail.com
Acked-by: Will Deacon will.dea...@arm.com
---
 arch/arm/mach-vt8500/vt8500.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-vt8500/vt8500.c b/arch/arm/mach-vt8500/vt8500.c
index 2da7be3..3bc0dc9 100644
--- a/arch/arm/mach-vt8500/vt8500.c
+++ b/arch/arm/mach-vt8500/vt8500.c
@@ -69,7 +69,7 @@ static void vt8500_power_off(void)
 {
local_irq_disable();
writew(5, pmc_base + VT8500_HCR_REG);
-   asm(mcr%? p15, 0, %0, c7, c0, 4 : : r (0));
+   asm(mcr p15, 0, %0, c7, c0, 4 : : r (0));
 }
 
 static void __init vt8500_init(void)
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] fs, LLVMLinux: Remove warning from COMPATIBLE_IOCTL

[Behan Webster]

From: Mark Charlebois charl...@gmail.com

cmd in COMPATIBLE_IOCTL is always a u32, so cast it so there isn't a warning
about an overflow in XFORM.

Signed-off-by: Mark Charlebois charl...@gmail.com
Signed-off-by: Behan Webster beh...@converseincode.com
Acked-by: Arnd Bergmann a...@arndb.de
---
 fs/compat_ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index afec645..f6ce1aa 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -810,7 +810,7 @@ static int compat_ioctl_preallocate(struct file *file,
  */
 #define XFORM(i) (((i) ^ ((i)  27) ^ ((i)  17))  0x)
 
-#define COMPATIBLE_IOCTL(cmd) XFORM(cmd),
+#define COMPATIBLE_IOCTL(cmd) XFORM((u32)cmd),
 /* ioctl should not be warned about even if it's not implemented.
Valid reasons to use this:
- It is implemented with -compat_ioctl on some device, but programs
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 01/12] crypto: LLVMLinux: Add macro to remove use of VLAIS in crypto code

[Behan Webster]

On 09/17/14 04:30, Herbert Xu wrote:

On Wed, Sep 17, 2014 at 02:15:40PM +0300, Dmitry Kasatkin wrote:

On 17/09/14 12:22, Herbert Xu wrote:

On Mon, Sep 15, 2014 at 12:30:23AM -0700, beh...@converseincode.com wrote:

From: Behan Webster 

Add a macro which replaces the use of a Variable Length Array In Struct (VLAIS)
with a C99 compliant equivalent. This macro instead allocates the appropriate
amount of memory using an char array.

The new code can be compiled with both gcc and clang.

struct shash_desc contains a flexible array member member ctx declared with
CRYPTO_MINALIGN_ATTR, so sizeof(struct shash_desc) aligns the beginning
of the array declared after struct shash_desc with long long.

No trailing padding is required because it is not a struct type that can
be used in an array.

The CRYPTO_MINALIGN_ATTR is required so that desc is aligned with long long
as would be the case for a struct containing a member with
CRYPTO_MINALIGN_ATTR.

Signed-off-by: Behan Webster 

Acked-by: Herbert Xu 

Thanks,

Just in case.
I would still follow advice from "Michał Mirosław" to use shash##__desc[]
Absolutely. I will be posting a v4 patchset . Just waiting a bit more 
for more comments on v3.


The macro from v4 will look like this which I believe will satisfy the 
concern and indeed be safer than my previous version.


+#define SHASH_DESC_ON_STACK(shash, tfm)  \
+   char __##shash##_desc[sizeof(struct shash_desc) +\
+   crypto_shash_descsize(tfm)] CRYPTO_MINALIGN_ATTR; \
+   struct shash_desc *shash = (struct shash_desc *)__##shash##_desc

Hmm. Is it worth adding a comment with this macro explaining the reason this 
works? Essentially much of what is in the commit message?
 


Oh yes of course.  My ack is more about the approach.


Wonderful!

Indeed. I would have asked for you to wait for v4 anyways. :)

Thank you,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 01/12] crypto: LLVMLinux: Add macro to remove use of VLAIS in crypto code

[Behan Webster]

On 09/17/14 04:30, Herbert Xu wrote:

On Wed, Sep 17, 2014 at 02:15:40PM +0300, Dmitry Kasatkin wrote:

On 17/09/14 12:22, Herbert Xu wrote:

On Mon, Sep 15, 2014 at 12:30:23AM -0700, beh...@converseincode.com wrote:

From: Behan Webster beh...@converseincode.com

Add a macro which replaces the use of a Variable Length Array In Struct (VLAIS)
with a C99 compliant equivalent. This macro instead allocates the appropriate
amount of memory using an char array.

The new code can be compiled with both gcc and clang.

struct shash_desc contains a flexible array member member ctx declared with
CRYPTO_MINALIGN_ATTR, so sizeof(struct shash_desc) aligns the beginning
of the array declared after struct shash_desc with long long.

No trailing padding is required because it is not a struct type that can
be used in an array.

The CRYPTO_MINALIGN_ATTR is required so that desc is aligned with long long
as would be the case for a struct containing a member with
CRYPTO_MINALIGN_ATTR.

Signed-off-by: Behan Webster beh...@converseincode.com

Acked-by: Herbert Xu herb...@gondor.apana.org.au

Thanks,

Just in case.
I would still follow advice from Michał Mirosław to use shash##__desc[]
Absolutely. I will be posting a v4 patchset . Just waiting a bit more 
for more comments on v3.


The macro from v4 will look like this which I believe will satisfy the 
concern and indeed be safer than my previous version.


+#define SHASH_DESC_ON_STACK(shash, tfm)  \
+   char __##shash##_desc[sizeof(struct shash_desc) +\
+   crypto_shash_descsize(tfm)] CRYPTO_MINALIGN_ATTR; \
+   struct shash_desc *shash = (struct shash_desc *)__##shash##_desc

Hmm. Is it worth adding a comment with this macro explaining the reason this 
works? Essentially much of what is in the commit message?
 


Oh yes of course.  My ack is more about the approach.


Wonderful!

Indeed. I would have asked for you to wait for v4 anyways. :)

Thank you,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 11/12] security, crypto: LLVMLinux: Remove VLAIS from ima_crypto.c

[Behan Webster]

On 09/15/14 07:21, Linus Torvalds wrote:

On Mon, Sep 15, 2014 at 12:30 AM,   wrote:

From: Behan Webster 

Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99
compliant equivalent. This patch allocates the appropriate amount of memory
using a char array using the SHASH_DESC_ON_STACK macro.

You only made the first case use SHASH_DESC_ON_STACK, the two other
cases you left in the ugly format. Was that just an oversight, or was
there some reason for it?


Oversight. Will Fix.

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: linux-next: manual merge of the llvmlinux tree with the tree

[Behan Webster]

On 09/14/14 23:38, Stephen Rothwell wrote:

Hi all,

Today's linux-next merge of the llvmlinux tree got a conflict in
drivers/gpu/drm/msm/hdmi/hdmi.c between commit fc886107c556 ("drm/msm:
Change nested function to static function") from the watchdog tree and
commit 8f2c494adad0 ("msm, hdmi: LLVMLinux: Remove nested function from
HDMI driver") from the llvmlinux tree.

I fixed it up (these patches are doing the same thing, so I used the
version in the watchdog tree) and can carry the fix as necessary (no
action is required).

P.S. Behan, I noticed that the llvmlinux patch has no Signed-off-by
from you even though you committed it ...

Drat. That patch wasn't supposed to be in there. Sorry about that.

Yeah. That one was accepted upstream already. I was carrying it in my 
repos so we could still build until it made it to mainline. I should not 
have put that in linux-next. I actually just had to pull it out of my 
own builds due to the same merge issue.


Hmm. Seems I put a couple of patches into the for-next branch late last 
night which shouldn't have been. Fixed.


Apologies,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [dm-devel] [PATCH v3 01/12] crypto: LLVMLinux: Add macro to remove use of VLAIS in crypto code

[Behan Webster]

On 09/15/14 01:06, Michał Mirosław wrote:

2014-09-15 9:30 GMT+02:00  :
[...]

+#define SHASH_DESC_ON_STACK(shash, tfm)  \
+   char __desc[sizeof(struct shash_desc) +   \
+   crypto_shash_descsize(tfm)] CRYPTO_MINALIGN_ATTR; \
+   struct shash_desc *shash = (struct shash_desc *)__desc
+

char shash##__desc[] or similar? Otherwise it won't work if you use
this macro twice in the same block.

Best Regards,
Michał Mirosław

Good thinking. Will fix.

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [dm-devel] [PATCH v3 01/12] crypto: LLVMLinux: Add macro to remove use of VLAIS in crypto code

[Behan Webster]

On 09/15/14 01:06, Michał Mirosław wrote:

2014-09-15 9:30 GMT+02:00  beh...@converseincode.com:
[...]

+#define SHASH_DESC_ON_STACK(shash, tfm)  \
+   char __desc[sizeof(struct shash_desc) +   \
+   crypto_shash_descsize(tfm)] CRYPTO_MINALIGN_ATTR; \
+   struct shash_desc *shash = (struct shash_desc *)__desc
+

char shash##__desc[] or similar? Otherwise it won't work if you use
this macro twice in the same block.

Best Regards,
Michał Mirosław

Good thinking. Will fix.

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: linux-next: manual merge of the llvmlinux tree with the tree

[Behan Webster]

On 09/14/14 23:38, Stephen Rothwell wrote:

Hi all,

Today's linux-next merge of the llvmlinux tree got a conflict in
drivers/gpu/drm/msm/hdmi/hdmi.c between commit fc886107c556 (drm/msm:
Change nested function to static function) from the watchdog tree and
commit 8f2c494adad0 (msm, hdmi: LLVMLinux: Remove nested function from
HDMI driver) from the llvmlinux tree.

I fixed it up (these patches are doing the same thing, so I used the
version in the watchdog tree) and can carry the fix as necessary (no
action is required).

P.S. Behan, I noticed that the llvmlinux patch has no Signed-off-by
from you even though you committed it ...

Drat. That patch wasn't supposed to be in there. Sorry about that.

Yeah. That one was accepted upstream already. I was carrying it in my 
repos so we could still build until it made it to mainline. I should not 
have put that in linux-next. I actually just had to pull it out of my 
own builds due to the same merge issue.


Hmm. Seems I put a couple of patches into the for-next branch late last 
night which shouldn't have been. Fixed.


Apologies,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 11/12] security, crypto: LLVMLinux: Remove VLAIS from ima_crypto.c

[Behan Webster]

On 09/15/14 07:21, Linus Torvalds wrote:

On Mon, Sep 15, 2014 at 12:30 AM,  beh...@converseincode.com wrote:

From: Behan Webster beh...@converseincode.com

Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99
compliant equivalent. This patch allocates the appropriate amount of memory
using a char array using the SHASH_DESC_ON_STACK macro.

You only made the first case use SHASH_DESC_ON_STACK, the two other
cases you left in the ugly format. Was that just an oversight, or was
there some reason for it?


Oversight. Will Fix.

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] btrfs: LLVMLinux: Remove VLAIS

[Behan Webster]

On 09/11/14 09:34, Chris Mason wrote:


On 09/05/2014 06:58 PM, beh...@converseincode.com wrote:

From: Vinícius Tinti 

Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99
compliant equivalent. This is the original VLAIS struct.

struct {
struct shash_desc shash;
char ctx[crypto_shash_descsize(tfm)];
} desc;

This patch instead allocates the appropriate amount of memory using an char
array.

The new code can be compiled with both gcc and clang.

struct shash_desc contains a flexible array member member ctx declared with
CRYPTO_MINALIGN_ATTR, so sizeof(struct shash_desc) aligns the beginning
of the array declared after struct shash_desc with long long.

No trailing padding is required because it is not a struct type that can
be used in an array.

The CRYPTO_MINALIGN_ATTR is required so that desc is aligned with long long
as would be the case for a struct containing a member with
CRYPTO_MINALIGN_ATTR.

We copied this from one of the other crypto api users, and I'm sure
cooking up all these patches was not a great way to your afternoon.

But, can I talk you into making a helper macro of some kind for this and
putting it into the crypto api headers?  Honestly this setup seems
really error prone.
You're not the only person to ask for this. I've got one already coded 
up based on suggestions from tglx.


I've regenerated all the patches. I will resubmit soon.

Will fix.

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] btrfs: LLVMLinux: Remove VLAIS

[Behan Webster]

On 09/11/14 09:34, Chris Mason wrote:


On 09/05/2014 06:58 PM, beh...@converseincode.com wrote:

From: Vinícius Tinti viniciusti...@gmail.com

Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99
compliant equivalent. This is the original VLAIS struct.

struct {
struct shash_desc shash;
char ctx[crypto_shash_descsize(tfm)];
} desc;

This patch instead allocates the appropriate amount of memory using an char
array.

The new code can be compiled with both gcc and clang.

struct shash_desc contains a flexible array member member ctx declared with
CRYPTO_MINALIGN_ATTR, so sizeof(struct shash_desc) aligns the beginning
of the array declared after struct shash_desc with long long.

No trailing padding is required because it is not a struct type that can
be used in an array.

The CRYPTO_MINALIGN_ATTR is required so that desc is aligned with long long
as would be the case for a struct containing a member with
CRYPTO_MINALIGN_ATTR.

We copied this from one of the other crypto api users, and I'm sure
cooking up all these patches was not a great way to your afternoon.

But, can I talk you into making a helper macro of some kind for this and
putting it into the crypto api headers?  Honestly this setup seems
really error prone.
You're not the only person to ask for this. I've got one already coded 
up based on suggestions from tglx.


I've regenerated all the patches. I will resubmit soon.

Will fix.

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] arm64: LLVMLinux: Provide __aeabi_* symbols which are needed for clang

[Behan Webster]

On 09/08/14 16:01, Mark Charlebois wrote:

On Sun, Sep 7, 2014 at 12:30 AM, Catalin Marinas
 wrote:

On 7 Sep 2014, at 03:30, Mark Charlebois  wrote:

On Sat, Sep 6, 2014 at 7:16 AM, Arnd Bergmann  wrote:

On Friday 05 September 2014 16:23:14 beh...@converseincode.com wrote:

+ *  Copyright (C) 2012  Mark Charlebois
+ */
+
+/*
+ * EABI routines

Does EABI specify these function names? I would think that they are
just random libgcc (whatever that is called in clang) functions.

http://infocenter.arm.com/help/topic/com.arm.doc.ihi0043d/IHI0043D_rtabi.pdf

See section 4.3.4 Memory copying, clearing, and setting

What does this document have to do with arm64 (AArch64, A64)? We don’t
need such symbols on arm64. Also, the arm64 kernel links with libgcc (no
immediate need AFAICT but the compiler does not guarantee the intrinsics
would always be generated inline).

[reposting in plain text]

This patch was made early in the arm64 kernel support. I just retested
and you are correct, it is no longer needed.  My apologies to all.


Whoops. I normally check whether a patch is still needed before posting 
them. I seem to have missed that step this time. My apologies as well.


That all being said, I prefer to see patches no longer required, than 
needing them to be upstreamed, so in that this is a win.


Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] security, crypto: LLVMLinux: Remove VLAIS from ima_crypto.c

[Behan Webster]

On 09/08/14 08:43, Mimi Zohar wrote:


Behan, thank you for the explanation.

No worries. I should have explained better. My apologies.


The same snippet of code used
here, and elsewhere in the kernel, is taken from the crypto subsystem.
Once it is resolved in the crypto subsystem, the same solution should be
propogated.

Mimi
Indeed that is my intention. I have tglx's suggested solution coded 
already. Just doing a bunch of allyesconfig builds to confirm all is 
compiling correctly.


I will post all patches as a single patch set this time (posted to all 
concerned).


I will repeat the explanation as well with the new patch set so everyone 
else in other subsystems sees those reasons as well.


If this works for everyone I'll also go back and update the crypto 
patches for the subsystems that have already accepted my previous patches.


Thanks,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] security, crypto: LLVMLinux: Remove VLAIS from ima_crypto.c

[Behan Webster]

On 09/08/14 04:15, Dmitry Kasatkin wrote:

On 07/09/14 05:06, Behan Webster wrote:

On 09/06/14 03:11, Thomas Gleixner wrote:

On Fri, 5 Sep 2014, Behan Webster wrote:

On 09/05/14 17:18, Thomas Gleixner wrote:

Signed-off-by: Behan Webster 
Signed-off-by: Mark Charlebois 
Signed-off-by: Jan-Simon Möller 

This SOB chain is completely ass backwards. See Documentation/...

"The Signed-off-by: tag indicates that the signer was involved in the
development of the patch, or that he/she was in the patch's delivery
path."

All three of us were involved. Does that not satisfy this rule?

No. Read #12

The sign-off is a simple line at the end of the explanation for the
patch, which certifies that you wrote it or otherwise have the right to
pass it on as an open-source patch.

So the above chain says:

 Written-by:   Behan
 Passed-on-by: Mark
 Passed-on-by: Jan

That would be correct if you sent the patch to Mark, Mark sent it to
Jan and Jan finally submitted it to LKML.

I suppose "Reviewed-by" is probably more appropriate for the last 2
then. Will fix.


-struct {
-struct shash_desc shash;
-char ctx[crypto_shash_descsize(tfm)];
-} desc;
+char desc[sizeof(struct shash_desc) +
+crypto_shash_descsize(tfm)] CRYPTO_MINALIGN_ATTR;
+struct shash_desc *shash = (struct shash_desc *)desc;

That anon struct should have never happened in the first place.

Sadly this is a design pattern used in many places through out the
kernel, and
appears to be fundamental to the crypto system. I was advised *not*
to change
it, so we haven't.

I agree that it's not a good practice.


Not
your problem, but you are not making it any better. You replace open
coded crap with even more unreadable crap.

Whats wrong with

 SHASH_DESC_ON_STACK(shash, tfm);

Nothing is wrong with that. I would have actually preferred that.
But it would
have fundamentally changed a lot more code.

Errm. Why is

#define SHASH_DESC_ON_STACK(shash, tfm)\
 char __shash[sizeof(.)];\
 struct shash_desc *shash = (struct shash_desc *) __shash

requiring more fundamental than open coding the same thing a gazillion
times. You still need to change ALL usage sides of the anon struct.

So in fact you could avoid the whole code change by making it

 SHASH_DESC_ON_STACK(desc, tfm);

and do the anon struct or a proper struct magic in the macro.

I see. I thought you meant a more fundamental change to the crypto
system API. My misunderstanding.

Ironically we tried to stay away from macros since the last time we
tried to replace VLAIS using macros (we've attempted patches to remove
VLAIS a few times) we were told *not* to hide the implementation with
macro magic. Though, to be fair, we were using more pointer math in
our other macro-based effort, and the non-crypto uses of VLAIS are a
lot more complex to replace.

Like I said I'm actually a fan of hiding ugliness in macros. Will fix.

Again, thanks for the feedback,

Behan


Hi,

Despite if it is crap or not, it was said already in this thread,
following "design pattern" is heavily used through out the kernel - by
crypto core itself and by many widely used clients.

 struct {
 struct shash_desc shash;
 char ctx[crypto_shash_descsize(tfm)];
 } desc;


My question why do you want to change this particular piece of code?
Because it employs Variable Length Arrays in Structs. A construct which 
is explicitly forbidden by the C standard (C89, C99, C11). Because the 
vast majority of kernel developers I've talked to about this have been 
unaware of the use of VLAIS in the kernel and most find its use 
objectionable (there is a similar objection to the use of nested 
functions). Because implementing VLAIS in a compiler can severely impact 
the generated instructions surrounding its use, which is why most 
compilers don't implement VLAIS as a feature. Because using such a 
construct precludes standards based compilers from competing with the 
incumbent (my interest is enabling the use of clang and LLVM based 
technologies as a toolchain choice to compile and develop the kernel).



What about rest of the kernel?
The LLVMLinux project is systematically working to remove the use of 
VLAIS from the kernel (already removed from ext4, USB Gadget, netfilter, 
mac802.11, apparmor, bluetooth, etc). Users of the crpyto subsystem are 
one of the last and heaviest users of VLAIS.



To solve your problem you probably need to change everything.
Essentially yes. Though I like to think of it as finding alternatives to 
where ever it is still used. "Changing everything" implies much larger 
changes which aren't necessary in most cases. Sometimes the alternative 
is merely using a flexible member (zero length array at the end of the 
struct, instead of a VLA in the struct). In several places several VLAs 
are used in the same struct. And recently we found that exofs is using a

Re: [PATCH] security, crypto: LLVMLinux: Remove VLAIS from ima_crypto.c

[Behan Webster]

On 09/08/14 04:15, Dmitry Kasatkin wrote:

On 07/09/14 05:06, Behan Webster wrote:

On 09/06/14 03:11, Thomas Gleixner wrote:

On Fri, 5 Sep 2014, Behan Webster wrote:

On 09/05/14 17:18, Thomas Gleixner wrote:

Signed-off-by: Behan Webster beh...@converseincode.com
Signed-off-by: Mark Charlebois charl...@gmail.com
Signed-off-by: Jan-Simon Möller dl...@gmx.de

This SOB chain is completely ass backwards. See Documentation/...

The Signed-off-by: tag indicates that the signer was involved in the
development of the patch, or that he/she was in the patch's delivery
path.

All three of us were involved. Does that not satisfy this rule?

No. Read #12

The sign-off is a simple line at the end of the explanation for the
patch, which certifies that you wrote it or otherwise have the right to
pass it on as an open-source patch.

So the above chain says:

 Written-by:   Behan
 Passed-on-by: Mark
 Passed-on-by: Jan

That would be correct if you sent the patch to Mark, Mark sent it to
Jan and Jan finally submitted it to LKML.

I suppose Reviewed-by is probably more appropriate for the last 2
then. Will fix.


-struct {
-struct shash_desc shash;
-char ctx[crypto_shash_descsize(tfm)];
-} desc;
+char desc[sizeof(struct shash_desc) +
+crypto_shash_descsize(tfm)] CRYPTO_MINALIGN_ATTR;
+struct shash_desc *shash = (struct shash_desc *)desc;

That anon struct should have never happened in the first place.

Sadly this is a design pattern used in many places through out the
kernel, and
appears to be fundamental to the crypto system. I was advised *not*
to change
it, so we haven't.

I agree that it's not a good practice.


Not
your problem, but you are not making it any better. You replace open
coded crap with even more unreadable crap.

Whats wrong with

 SHASH_DESC_ON_STACK(shash, tfm);

Nothing is wrong with that. I would have actually preferred that.
But it would
have fundamentally changed a lot more code.

Errm. Why is

#define SHASH_DESC_ON_STACK(shash, tfm)\
 char __shash[sizeof(.)];\
 struct shash_desc *shash = (struct shash_desc *) __shash

requiring more fundamental than open coding the same thing a gazillion
times. You still need to change ALL usage sides of the anon struct.

So in fact you could avoid the whole code change by making it

 SHASH_DESC_ON_STACK(desc, tfm);

and do the anon struct or a proper struct magic in the macro.

I see. I thought you meant a more fundamental change to the crypto
system API. My misunderstanding.

Ironically we tried to stay away from macros since the last time we
tried to replace VLAIS using macros (we've attempted patches to remove
VLAIS a few times) we were told *not* to hide the implementation with
macro magic. Though, to be fair, we were using more pointer math in
our other macro-based effort, and the non-crypto uses of VLAIS are a
lot more complex to replace.

Like I said I'm actually a fan of hiding ugliness in macros. Will fix.

Again, thanks for the feedback,

Behan


Hi,

Despite if it is crap or not, it was said already in this thread,
following design pattern is heavily used through out the kernel - by
crypto core itself and by many widely used clients.

 struct {
 struct shash_desc shash;
 char ctx[crypto_shash_descsize(tfm)];
 } desc;


My question why do you want to change this particular piece of code?
Because it employs Variable Length Arrays in Structs. A construct which 
is explicitly forbidden by the C standard (C89, C99, C11). Because the 
vast majority of kernel developers I've talked to about this have been 
unaware of the use of VLAIS in the kernel and most find its use 
objectionable (there is a similar objection to the use of nested 
functions). Because implementing VLAIS in a compiler can severely impact 
the generated instructions surrounding its use, which is why most 
compilers don't implement VLAIS as a feature. Because using such a 
construct precludes standards based compilers from competing with the 
incumbent (my interest is enabling the use of clang and LLVM based 
technologies as a toolchain choice to compile and develop the kernel).



What about rest of the kernel?
The LLVMLinux project is systematically working to remove the use of 
VLAIS from the kernel (already removed from ext4, USB Gadget, netfilter, 
mac802.11, apparmor, bluetooth, etc). Users of the crpyto subsystem are 
one of the last and heaviest users of VLAIS.



To solve your problem you probably need to change everything.
Essentially yes. Though I like to think of it as finding alternatives to 
where ever it is still used. Changing everything implies much larger 
changes which aren't necessary in most cases. Sometimes the alternative 
is merely using a flexible member (zero length array at the end of the 
struct, instead of a VLA in the struct). In several places several VLAs 
are used in the same struct. And recently we found that exofs is using

Re: [PATCH] security, crypto: LLVMLinux: Remove VLAIS from ima_crypto.c

[Behan Webster]

On 09/08/14 08:43, Mimi Zohar wrote:


Behan, thank you for the explanation.

No worries. I should have explained better. My apologies.


The same snippet of code used
here, and elsewhere in the kernel, is taken from the crypto subsystem.
Once it is resolved in the crypto subsystem, the same solution should be
propogated.

Mimi
Indeed that is my intention. I have tglx's suggested solution coded 
already. Just doing a bunch of allyesconfig builds to confirm all is 
compiling correctly.


I will post all patches as a single patch set this time (posted to all 
concerned).


I will repeat the explanation as well with the new patch set so everyone 
else in other subsystems sees those reasons as well.


If this works for everyone I'll also go back and update the crypto 
patches for the subsystems that have already accepted my previous patches.


Thanks,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] arm64: LLVMLinux: Provide __aeabi_* symbols which are needed for clang

[Behan Webster]

On 09/08/14 16:01, Mark Charlebois wrote:

On Sun, Sep 7, 2014 at 12:30 AM, Catalin Marinas
catalin.mari...@arm.com wrote:

On 7 Sep 2014, at 03:30, Mark Charlebois charl...@gmail.com wrote:

On Sat, Sep 6, 2014 at 7:16 AM, Arnd Bergmann a...@arndb.de wrote:

On Friday 05 September 2014 16:23:14 beh...@converseincode.com wrote:

+ *  Copyright (C) 2012  Mark Charlebois
+ */
+
+/*
+ * EABI routines

Does EABI specify these function names? I would think that they are
just random libgcc (whatever that is called in clang) functions.

http://infocenter.arm.com/help/topic/com.arm.doc.ihi0043d/IHI0043D_rtabi.pdf

See section 4.3.4 Memory copying, clearing, and setting

What does this document have to do with arm64 (AArch64, A64)? We don’t
need such symbols on arm64. Also, the arm64 kernel links with libgcc (no
immediate need AFAICT but the compiler does not guarantee the intrinsics
would always be generated inline).

[reposting in plain text]

This patch was made early in the arm64 kernel support. I just retested
and you are correct, it is no longer needed.  My apologies to all.


Whoops. I normally check whether a patch is still needed before posting 
them. I seem to have missed that step this time. My apologies as well.


That all being said, I prefer to see patches no longer required, than 
needing them to be upstreamed, so in that this is a win.


Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] arm64: LLVMLinux: Provide __aeabi_* symbols which are needed for clang

[Behan Webster]

On 09/06/14 07:16, Arnd Bergmann wrote:

On Friday 05 September 2014 16:23:14 beh...@converseincode.com wrote:

--- /dev/null
+++ b/arch/arm64/lib/eabi.c
@@ -0,0 +1,32 @@
+/*
+ *  linux/lib/eabi.c

Please don't put the file names in the files themselves, it's redundant
and in this case actually wrong.

Will fix.


+ *  Copyright (C) 2012  Mark Charlebois
+ */
+
+/*
+ * EABI routines

Does EABI specify these function names? I would think that they are
just random libgcc (whatever that is called in clang) functions.
These specialized functions are part of the ABI for the ARM architecture 
(AEABI). They aren't random.


Memcpy and memmove *could* might be satisfied with linker magic instead. 
But memset uses the reverse parameter list.


Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] crypto, dm: LLVMLinux: Remove VLAIS usage from dm-crypt

[Behan Webster]

On 09/06/14 01:46, Milan Broz wrote:

On 09/06/2014 01:02 AM, beh...@converseincode.com wrote:

From: Jan-Simon Möller 

The use of variable length arrays in structs (VLAIS) in the Linux Kernel code
precludes the use of compilers which don't implement VLAIS (for instance the
Clang compiler). This patch instead allocates the appropriate amount of memory
using an char array.

Well, if clang (or C99 code) is now preferred for kernel, why not.

Just please commit the patch series en bloc (together with the patches
removing VLAIS from crypto code you posted to cryptoapi list).
They seemed more separate than that. However, happy to post them as a 
patch set.



-   struct {
-   struct shash_desc desc;
-   char ctx[crypto_shash_descsize(lmk->hash_tfm)];
-   } sdesc;
+   char sdesc[sizeof(struct shash_desc) +
+   crypto_shash_descsize(lmk->hash_tfm)] CRYPTO_MINALIGN_ATTR;
+   struct shash_desc *desc = (struct shash_desc *)sdesc;

TBH, this looks even more uglier that the previous code :-)

I'm not claiming it's prettier. Merely C99. :)


(But tglx already complained on different patch and I fully agree that crypto 
code
should not use this kind of construction in the first place...
It would be very nice to introduce at least some macro hiding these crazy
stack allocations later.)

I actually agree with you and tglx. Will fix.

As I said to tglx, we were asked not to hide things with macro magic in 
some of our non-crypto VLAIS removal patches. Happy to use macros in 
this case.


Thanks,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] security, crypto: LLVMLinux: Remove VLAIS from ima_crypto.c

[Behan Webster]

On 09/06/14 03:11, Thomas Gleixner wrote:

On Fri, 5 Sep 2014, Behan Webster wrote:

On 09/05/14 17:18, Thomas Gleixner wrote:

Signed-off-by: Behan Webster 
Signed-off-by: Mark Charlebois 
Signed-off-by: Jan-Simon Möller 

This SOB chain is completely ass backwards. See Documentation/...

"The Signed-off-by: tag indicates that the signer was involved in the
development of the patch, or that he/she was in the patch's delivery path."

All three of us were involved. Does that not satisfy this rule?

No. Read #12

The sign-off is a simple line at the end of the explanation for the
patch, which certifies that you wrote it or otherwise have the right to
pass it on as an open-source patch.

So the above chain says:

Written-by:   Behan
Passed-on-by: Mark
Passed-on-by: Jan

That would be correct if you sent the patch to Mark, Mark sent it to
Jan and Jan finally submitted it to LKML.
I suppose "Reviewed-by" is probably more appropriate for the last 2 
then. Will fix.



-   struct {
-   struct shash_desc shash;
-   char ctx[crypto_shash_descsize(tfm)];
-   } desc;
+   char desc[sizeof(struct shash_desc) +
+   crypto_shash_descsize(tfm)] CRYPTO_MINALIGN_ATTR;
+   struct shash_desc *shash = (struct shash_desc *)desc;

That anon struct should have never happened in the first place.

Sadly this is a design pattern used in many places through out the kernel, and
appears to be fundamental to the crypto system. I was advised *not* to change
it, so we haven't.

I agree that it's not a good practice.


   Not
your problem, but you are not making it any better. You replace open
coded crap with even more unreadable crap.

Whats wrong with

SHASH_DESC_ON_STACK(shash, tfm);

Nothing is wrong with that. I would have actually preferred that. But it would
have fundamentally changed a lot more code.

Errm. Why is

#define SHASH_DESC_ON_STACK(shash, tfm) \
char __shash[sizeof(.)];\
struct shash_desc *shash = (struct shash_desc *) __shash

requiring more fundamental than open coding the same thing a gazillion
times. You still need to change ALL usage sides of the anon struct.

So in fact you could avoid the whole code change by making it

SHASH_DESC_ON_STACK(desc, tfm);

and do the anon struct or a proper struct magic in the macro.
I see. I thought you meant a more fundamental change to the crypto 
system API. My misunderstanding.


Ironically we tried to stay away from macros since the last time we 
tried to replace VLAIS using macros (we've attempted patches to remove 
VLAIS a few times) we were told *not* to hide the implementation with 
macro magic. Though, to be fair, we were using more pointer math in our 
other macro-based effort, and the non-crypto uses of VLAIS are a lot 
more complex to replace.


Like I said I'm actually a fan of hiding ugliness in macros. Will fix.

Again, thanks for the feedback,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] security, crypto: LLVMLinux: Remove VLAIS from ima_crypto.c

[Behan Webster]

On 09/06/14 03:11, Thomas Gleixner wrote:

On Fri, 5 Sep 2014, Behan Webster wrote:

On 09/05/14 17:18, Thomas Gleixner wrote:

Signed-off-by: Behan Webster beh...@converseincode.com
Signed-off-by: Mark Charlebois charl...@gmail.com
Signed-off-by: Jan-Simon Möller dl...@gmx.de

This SOB chain is completely ass backwards. See Documentation/...

The Signed-off-by: tag indicates that the signer was involved in the
development of the patch, or that he/she was in the patch's delivery path.

All three of us were involved. Does that not satisfy this rule?

No. Read #12

The sign-off is a simple line at the end of the explanation for the
patch, which certifies that you wrote it or otherwise have the right to
pass it on as an open-source patch.

So the above chain says:

Written-by:   Behan
Passed-on-by: Mark
Passed-on-by: Jan

That would be correct if you sent the patch to Mark, Mark sent it to
Jan and Jan finally submitted it to LKML.
I suppose Reviewed-by is probably more appropriate for the last 2 
then. Will fix.



-   struct {
-   struct shash_desc shash;
-   char ctx[crypto_shash_descsize(tfm)];
-   } desc;
+   char desc[sizeof(struct shash_desc) +
+   crypto_shash_descsize(tfm)] CRYPTO_MINALIGN_ATTR;
+   struct shash_desc *shash = (struct shash_desc *)desc;

That anon struct should have never happened in the first place.

Sadly this is a design pattern used in many places through out the kernel, and
appears to be fundamental to the crypto system. I was advised *not* to change
it, so we haven't.

I agree that it's not a good practice.


   Not
your problem, but you are not making it any better. You replace open
coded crap with even more unreadable crap.

Whats wrong with

SHASH_DESC_ON_STACK(shash, tfm);

Nothing is wrong with that. I would have actually preferred that. But it would
have fundamentally changed a lot more code.

Errm. Why is

#define SHASH_DESC_ON_STACK(shash, tfm) \
char __shash[sizeof(.)];\
struct shash_desc *shash = (struct shash_desc *) __shash

requiring more fundamental than open coding the same thing a gazillion
times. You still need to change ALL usage sides of the anon struct.

So in fact you could avoid the whole code change by making it

SHASH_DESC_ON_STACK(desc, tfm);

and do the anon struct or a proper struct magic in the macro.
I see. I thought you meant a more fundamental change to the crypto 
system API. My misunderstanding.


Ironically we tried to stay away from macros since the last time we 
tried to replace VLAIS using macros (we've attempted patches to remove 
VLAIS a few times) we were told *not* to hide the implementation with 
macro magic. Though, to be fair, we were using more pointer math in our 
other macro-based effort, and the non-crypto uses of VLAIS are a lot 
more complex to replace.


Like I said I'm actually a fan of hiding ugliness in macros. Will fix.

Again, thanks for the feedback,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] crypto, dm: LLVMLinux: Remove VLAIS usage from dm-crypt

[Behan Webster]

On 09/06/14 01:46, Milan Broz wrote:

On 09/06/2014 01:02 AM, beh...@converseincode.com wrote:

From: Jan-Simon Möller dl...@gmx.de

The use of variable length arrays in structs (VLAIS) in the Linux Kernel code
precludes the use of compilers which don't implement VLAIS (for instance the
Clang compiler). This patch instead allocates the appropriate amount of memory
using an char array.

Well, if clang (or C99 code) is now preferred for kernel, why not.

Just please commit the patch series en bloc (together with the patches
removing VLAIS from crypto code you posted to cryptoapi list).
They seemed more separate than that. However, happy to post them as a 
patch set.



-   struct {
-   struct shash_desc desc;
-   char ctx[crypto_shash_descsize(lmk-hash_tfm)];
-   } sdesc;
+   char sdesc[sizeof(struct shash_desc) +
+   crypto_shash_descsize(lmk-hash_tfm)] CRYPTO_MINALIGN_ATTR;
+   struct shash_desc *desc = (struct shash_desc *)sdesc;

TBH, this looks even more uglier that the previous code :-)

I'm not claiming it's prettier. Merely C99. :)


(But tglx already complained on different patch and I fully agree that crypto 
code
should not use this kind of construction in the first place...
It would be very nice to introduce at least some macro hiding these crazy
stack allocations later.)

I actually agree with you and tglx. Will fix.

As I said to tglx, we were asked not to hide things with macro magic in 
some of our non-crypto VLAIS removal patches. Happy to use macros in 
this case.


Thanks,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] arm64: LLVMLinux: Provide __aeabi_* symbols which are needed for clang

[Behan Webster]

On 09/06/14 07:16, Arnd Bergmann wrote:

On Friday 05 September 2014 16:23:14 beh...@converseincode.com wrote:

--- /dev/null
+++ b/arch/arm64/lib/eabi.c
@@ -0,0 +1,32 @@
+/*
+ *  linux/lib/eabi.c

Please don't put the file names in the files themselves, it's redundant
and in this case actually wrong.

Will fix.


+ *  Copyright (C) 2012  Mark Charlebois
+ */
+
+/*
+ * EABI routines

Does EABI specify these function names? I would think that they are
just random libgcc (whatever that is called in clang) functions.
These specialized functions are part of the ABI for the ARM architecture 
(AEABI). They aren't random.


Memcpy and memmove *could* might be satisfied with linker magic instead. 
But memset uses the reverse parameter list.


Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] security, crypto: LLVMLinux: Remove VLAIS from ima_crypto.c

[Behan Webster]

On 09/05/14 17:18, Thomas Gleixner wrote:

On Fri, 5 Sep 2014, beh...@converseincode.com wrote:


From: Behan Webster 

Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99
compliant equivalent. This patch allocates the appropriate amount of memory
using an char array.

The new code can be compiled with both gcc and clang.

struct shash_desc contains a flexible array member member ctx declared with
CRYPTO_MINALIGN_ATTR, so sizeof(struct shash_desc) aligns the beginning
of the array declared after struct shash_desc with long long.

No trailing padding is required because it is not a struct type that can
be used in an array.

The CRYPTO_MINALIGN_ATTR is required so that desc is aligned with long long
as would be the case for a struct containing a member with
CRYPTO_MINALIGN_ATTR.

Signed-off-by: Behan Webster 
Signed-off-by: Mark Charlebois 
Signed-off-by: Jan-Simon Möller 

This SOB chain is completely ass backwards. See Documentation/...

"The Signed-off-by: tag indicates that the signer was involved in the
development of the patch, or that he/she was in the patch's delivery path."

All three of us were involved. Does that not satisfy this rule?


---
  security/integrity/ima/ima_crypto.c | 53 +
  1 file changed, 25 insertions(+), 28 deletions(-)

diff --git a/security/integrity/ima/ima_crypto.c 
b/security/integrity/ima/ima_crypto.c
index 0bd7328..a6aa2b0 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -380,17 +380,16 @@ static int ima_calc_file_hash_tfm(struct file *file,
loff_t i_size, offset = 0;
char *rbuf;
int rc, read = 0;
-   struct {
-   struct shash_desc shash;
-   char ctx[crypto_shash_descsize(tfm)];
-   } desc;
+   char desc[sizeof(struct shash_desc) +
+   crypto_shash_descsize(tfm)] CRYPTO_MINALIGN_ATTR;
+   struct shash_desc *shash = (struct shash_desc *)desc;

That anon struct should have never happened in the first place.
Sadly this is a design pattern used in many places through out the 
kernel, and appears to be fundamental to the crypto system. I was 
advised *not* to change it, so we haven't.


I agree that it's not a good practice.


  Not
your problem, but you are not making it any better. You replace open
coded crap with even more unreadable crap.

Whats wrong with

   SHASH_DESC_ON_STACK(shash, tfm);
Nothing is wrong with that. I would have actually preferred that. But it 
would have fundamentally changed a lot more code.


I'm not sure making fundamental changes to the crypto code in order to 
make "my favourite compiler happy" is really a better plan. I prefer 
smaller more measured steps.



or something along that line and hide all the stack allocation, type
conversion crap and make my favourite compiler happy in a single
place?
At this point it is less about making a particular compiler happy, and 
more about making the code at least C99 compliant which enables a 
different compiler so that more people can use it to make more 
fundamental changes like you are suggesting.


I appreciate your feedback,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] security, crypto: LLVMLinux: Remove VLAIS from ima_crypto.c

[Behan Webster]

On 09/05/14 17:18, Thomas Gleixner wrote:

On Fri, 5 Sep 2014, beh...@converseincode.com wrote:


From: Behan Webster beh...@converseincode.com

Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99
compliant equivalent. This patch allocates the appropriate amount of memory
using an char array.

The new code can be compiled with both gcc and clang.

struct shash_desc contains a flexible array member member ctx declared with
CRYPTO_MINALIGN_ATTR, so sizeof(struct shash_desc) aligns the beginning
of the array declared after struct shash_desc with long long.

No trailing padding is required because it is not a struct type that can
be used in an array.

The CRYPTO_MINALIGN_ATTR is required so that desc is aligned with long long
as would be the case for a struct containing a member with
CRYPTO_MINALIGN_ATTR.

Signed-off-by: Behan Webster beh...@converseincode.com
Signed-off-by: Mark Charlebois charl...@gmail.com
Signed-off-by: Jan-Simon Möller dl...@gmx.de

This SOB chain is completely ass backwards. See Documentation/...

The Signed-off-by: tag indicates that the signer was involved in the
development of the patch, or that he/she was in the patch's delivery path.

All three of us were involved. Does that not satisfy this rule?


---
  security/integrity/ima/ima_crypto.c | 53 +
  1 file changed, 25 insertions(+), 28 deletions(-)

diff --git a/security/integrity/ima/ima_crypto.c 
b/security/integrity/ima/ima_crypto.c
index 0bd7328..a6aa2b0 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -380,17 +380,16 @@ static int ima_calc_file_hash_tfm(struct file *file,
loff_t i_size, offset = 0;
char *rbuf;
int rc, read = 0;
-   struct {
-   struct shash_desc shash;
-   char ctx[crypto_shash_descsize(tfm)];
-   } desc;
+   char desc[sizeof(struct shash_desc) +
+   crypto_shash_descsize(tfm)] CRYPTO_MINALIGN_ATTR;
+   struct shash_desc *shash = (struct shash_desc *)desc;

That anon struct should have never happened in the first place.
Sadly this is a design pattern used in many places through out the 
kernel, and appears to be fundamental to the crypto system. I was 
advised *not* to change it, so we haven't.


I agree that it's not a good practice.


  Not
your problem, but you are not making it any better. You replace open
coded crap with even more unreadable crap.

Whats wrong with

   SHASH_DESC_ON_STACK(shash, tfm);
Nothing is wrong with that. I would have actually preferred that. But it 
would have fundamentally changed a lot more code.


I'm not sure making fundamental changes to the crypto code in order to 
make my favourite compiler happy is really a better plan. I prefer 
smaller more measured steps.



or something along that line and hide all the stack allocation, type
conversion crap and make my favourite compiler happy in a single
place?
At this point it is less about making a particular compiler happy, and 
more about making the code at least C99 compliant which enables a 
different compiler so that more people can use it to make more 
fundamental changes like you are suggesting.


I appreciate your feedback,

Behan

--
Behan Webster
beh...@converseincode.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/