netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 11/11] asmlinkage, wan/sbni: Make inline assembler symbols visible and assembler global
       [not found] <1382458346-24811-1-git-send-email-andi@firstfloor.org>
@ 2013-10-22 16:12 ` Andi Kleen
  2013-10-22 17:59   ` David Miller
  0 siblings, 1 reply; 10+ messages in thread
From: Andi Kleen @ 2013-10-22 16:12 UTC (permalink / raw)
  To: akpm; +Cc: linux-kernel, Andi Kleen, netdev

From: Andi Kleen <ak@linux.intel.com>

- Inline assembler defining C callable code has to be global
- The function has to be visible

Do this in wan/sbni

Cc: netdev@vger.kernel.org
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 drivers/net/wan/sbni.c | 6 +++---
 drivers/net/wan/sbni.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wan/sbni.c b/drivers/net/wan/sbni.c
index 5bbcb5e..571db25 100644
--- a/drivers/net/wan/sbni.c
+++ b/drivers/net/wan/sbni.c
@@ -160,7 +160,7 @@ static int  scandone	__initdata = 0;
 static int  num		__initdata = 0;
 
 static unsigned char  rxl_tab[];
-static u32  crc32tab[];
+__visible u32  sbni_crc32tab[];
 
 /* A list of all installed devices, for removing the driver module. */
 static struct net_device  *sbni_cards[ SBNI_MAX_NUM_CARDS ];
@@ -1563,7 +1563,7 @@ calc_crc32( u32  crc,  u8  *p,  u32  len )
 		"xorl	%%ebx, %%ebx\n"
 		"movl	%2, %%esi\n" 
 		"movl	%3, %%ecx\n" 
-		"movl	$crc32tab, %%edi\n"
+		"movl	$sbni_crc32tab, %%edi\n"
 		"shrl	$2, %%ecx\n"
 		"jz	1f\n"
 
@@ -1645,7 +1645,7 @@ calc_crc32( u32  crc,  u8  *p,  u32  len )
 #endif	/* ASM_CRC */
 
 
-static u32  crc32tab[] __attribute__ ((aligned(8))) = {
+__visible u32  sbni_crc32tab[] __attribute__ ((aligned(8))) = {
 	0xD202EF8D,  0xA505DF1B,  0x3C0C8EA1,  0x4B0BBE37,
 	0xD56F2B94,  0xA2681B02,  0x3B614AB8,  0x4C667A2E,
 	0xDCD967BF,  0xABDE5729,  0x32D70693,  0x45D03605,
diff --git a/drivers/net/wan/sbni.h b/drivers/net/wan/sbni.h
index 8426451..7e6d980 100644
--- a/drivers/net/wan/sbni.h
+++ b/drivers/net/wan/sbni.h
@@ -132,7 +132,7 @@ struct sbni_flags {
 /*
  * CRC-32 stuff
  */
-#define CRC32(c,crc) (crc32tab[((size_t)(crc) ^ (c)) & 0xff] ^ (((crc) >> 8) & 0x00FFFFFF))
+#define CRC32(c,crc) (sbni_crc32tab[((size_t)(crc) ^ (c)) & 0xff] ^ (((crc) >> 8) & 0x00FFFFFF))
       /* CRC generator 0xEDB88320 */
       /* CRC remainder 0x2144DF1C */
       /* CRC initial value 0x00000000 */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 11/11] asmlinkage, wan/sbni: Make inline assembler symbols visible and assembler global
  2013-10-22 16:12 ` [PATCH 11/11] asmlinkage, wan/sbni: Make inline assembler symbols visible and assembler global Andi Kleen
@ 2013-10-22 17:59   ` David Miller
  2013-10-22 18:36     ` [PATCH] net: wan: sbni: remove assembly crc32 code Sebastian Andrzej Siewior
  2013-10-23  2:09     ` [PATCH 11/11] asmlinkage, wan/sbni: Make inline assembler symbols visible and assembler global Andi Kleen
  0 siblings, 2 replies; 10+ messages in thread
From: David Miller @ 2013-10-22 17:59 UTC (permalink / raw)
  To: andi; +Cc: akpm, linux-kernel, ak, netdev

From: Andi Kleen <andi@firstfloor.org>
Date: Tue, 22 Oct 2013 09:12:26 -0700

> From: Andi Kleen <ak@linux.intel.com>
> 
> - Inline assembler defining C callable code has to be global
> - The function has to be visible
> 
> Do this in wan/sbni
> 
> Cc: netdev@vger.kernel.org
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

Is it really impossible to use the generic crc32 support instead of this
unsightly custom inline assembler?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] net: wan: sbni: remove assembly crc32 code
  2013-10-22 17:59   ` David Miller
@ 2013-10-22 18:36     ` Sebastian Andrzej Siewior
  2013-10-22 18:38       ` David Miller
                         ` (2 more replies)
  2013-10-23  2:09     ` [PATCH 11/11] asmlinkage, wan/sbni: Make inline assembler symbols visible and assembler global Andi Kleen
  1 sibling, 3 replies; 10+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-10-22 18:36 UTC (permalink / raw)
  To: David Miller; +Cc: andi, akpm, linux-kernel, ak, netdev


There is also a C function doing the same thing. Unless the asm code is
110% faster we could stick to the C function.

Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
---

On Tue, Oct 22, 2013 at 01:59:28PM -0400, David Miller wrote:
> 
> Is it really impossible to use the generic crc32 support instead of this
> unsightly custom inline assembler?

Since you asked for this here step 1.

 drivers/net/wan/sbni.c | 89 --------------------------------------------------
 1 file changed, 89 deletions(-)

diff --git a/drivers/net/wan/sbni.c b/drivers/net/wan/sbni.c
index 5bbcb5e..388ddf6 100644
--- a/drivers/net/wan/sbni.c
+++ b/drivers/net/wan/sbni.c
@@ -148,10 +148,6 @@ static int  enslave( struct net_device *, struct net_device * );
 static int  emancipate( struct net_device * );
 #endif
 
-#ifdef __i386__
-#define ASM_CRC 1
-#endif
-
 static const char  version[] =
 	"Granch SBNI12 driver ver 5.0.1  Jun 22 2001  Denis I.Timofeev.\n";
 
@@ -1551,88 +1547,6 @@ __setup( "sbni=", sbni_setup );
 
 /* -------------------------------------------------------------------------- */
 
-#ifdef ASM_CRC
-
-static u32
-calc_crc32( u32  crc,  u8  *p,  u32  len )
-{
-	register u32  _crc;
-	_crc = crc;
-	
-	__asm__ __volatile__ (
-		"xorl	%%ebx, %%ebx\n"
-		"movl	%2, %%esi\n" 
-		"movl	%3, %%ecx\n" 
-		"movl	$crc32tab, %%edi\n"
-		"shrl	$2, %%ecx\n"
-		"jz	1f\n"
-
-		".align 4\n"
-	"0:\n"
-		"movb	%%al, %%bl\n"
-		"movl	(%%esi), %%edx\n"
-		"shrl	$8, %%eax\n"
-		"xorb	%%dl, %%bl\n"
-		"shrl	$8, %%edx\n"
-		"xorl	(%%edi,%%ebx,4), %%eax\n"
-
-		"movb	%%al, %%bl\n"
-		"shrl	$8, %%eax\n"
-		"xorb	%%dl, %%bl\n"
-		"shrl	$8, %%edx\n"
-		"xorl	(%%edi,%%ebx,4), %%eax\n"
-
-		"movb	%%al, %%bl\n"
-		"shrl	$8, %%eax\n"
-		"xorb	%%dl, %%bl\n"
-		"movb	%%dh, %%dl\n" 
-		"xorl	(%%edi,%%ebx,4), %%eax\n"
-
-		"movb	%%al, %%bl\n"
-		"shrl	$8, %%eax\n"
-		"xorb	%%dl, %%bl\n"
-		"addl	$4, %%esi\n"
-		"xorl	(%%edi,%%ebx,4), %%eax\n"
-
-		"decl	%%ecx\n"
-		"jnz	0b\n"
-
-	"1:\n"
-		"movl	%3, %%ecx\n"
-		"andl	$3, %%ecx\n"
-		"jz	2f\n"
-
-		"movb	%%al, %%bl\n"
-		"shrl	$8, %%eax\n"
-		"xorb	(%%esi), %%bl\n"
-		"xorl	(%%edi,%%ebx,4), %%eax\n"
-
-		"decl	%%ecx\n"
-		"jz	2f\n"
-
-		"movb	%%al, %%bl\n"
-		"shrl	$8, %%eax\n"
-		"xorb	1(%%esi), %%bl\n"
-		"xorl	(%%edi,%%ebx,4), %%eax\n"
-
-		"decl	%%ecx\n"
-		"jz	2f\n"
-
-		"movb	%%al, %%bl\n"
-		"shrl	$8, %%eax\n"
-		"xorb	2(%%esi), %%bl\n"
-		"xorl	(%%edi,%%ebx,4), %%eax\n"
-	"2:\n"
-		: "=a" (_crc)
-		: "0" (_crc), "g" (p), "g" (len)
-		: "bx", "cx", "dx", "si", "di"
-	);
-
-	return  _crc;
-}
-
-#else	/* ASM_CRC */
-
 static u32
 calc_crc32( u32  crc,  u8  *p,  u32  len )
 {
@@ -1642,9 +1556,6 @@ calc_crc32( u32  crc,  u8  *p,  u32  len )
 	return  crc;
 }
 
-#endif	/* ASM_CRC */
-
-
 static u32  crc32tab[] __attribute__ ((aligned(8))) = {
 	0xD202EF8D,  0xA505DF1B,  0x3C0C8EA1,  0x4B0BBE37,
 	0xD56F2B94,  0xA2681B02,  0x3B614AB8,  0x4C667A2E,
-- 
1.8.4.rc3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] net: wan: sbni: remove assembly crc32 code
  2013-10-22 18:36     ` [PATCH] net: wan: sbni: remove assembly crc32 code Sebastian Andrzej Siewior
@ 2013-10-22 18:38       ` David Miller
  2013-10-23  2:11       ` Andi Kleen
  2013-10-25 23:27       ` David Miller
  2 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2013-10-22 18:38 UTC (permalink / raw)
  To: sebastian; +Cc: andi, akpm, linux-kernel, ak, netdev

From: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
Date: Tue, 22 Oct 2013 20:36:25 +0200

> 
> There is also a C function doing the same thing. Unless the asm code is
> 110% faster we could stick to the C function.
> 
> Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
> ---
> 
> On Tue, Oct 22, 2013 at 01:59:28PM -0400, David Miller wrote:
>> 
>> Is it really impossible to use the generic crc32 support instead of this
>> unsightly custom inline assembler?
> 
> Since you asked for this here step 1.

If it's that simple, this is definitely what we should do.

On the off chance, is there anyone actually able to test this
change? :-)))

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 11/11] asmlinkage, wan/sbni: Make inline assembler symbols visible and assembler global
  2013-10-22 17:59   ` David Miller
  2013-10-22 18:36     ` [PATCH] net: wan: sbni: remove assembly crc32 code Sebastian Andrzej Siewior
@ 2013-10-23  2:09     ` Andi Kleen
  2013-10-23  2:15       ` Florian Fainelli
  1 sibling, 1 reply; 10+ messages in thread
From: Andi Kleen @ 2013-10-23  2:09 UTC (permalink / raw)
  To: David Miller; +Cc: andi, akpm, linux-kernel, ak, netdev

On Tue, Oct 22, 2013 at 01:59:28PM -0400, David Miller wrote:
> From: Andi Kleen <andi@firstfloor.org>
> Date: Tue, 22 Oct 2013 09:12:26 -0700
> 
> > From: Andi Kleen <ak@linux.intel.com>
> > 
> > - Inline assembler defining C callable code has to be global
> > - The function has to be visible
> > 
> > Do this in wan/sbni
> > 
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> Is it really impossible to use the generic crc32 support instead of this
> unsightly custom inline assembler?

Yes it's impossible.

There's no way for me to test it, and this would be a far too big change
to submit untested.

Also my only interest here is this thing not breaking my LTO
allyesconfig build. I'm not even sure what it does.

Just because some legacy driver breaks my build you cannot make me
accountable for maintaining it now. 

If it's not possible to get this trivial change in I would need
to mark it "depends on !LTO" in Kconfig.

-Andi

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] net: wan: sbni: remove assembly crc32 code
  2013-10-22 18:36     ` [PATCH] net: wan: sbni: remove assembly crc32 code Sebastian Andrzej Siewior
  2013-10-22 18:38       ` David Miller
@ 2013-10-23  2:11       ` Andi Kleen
  2013-10-25 23:27       ` David Miller
  2 siblings, 0 replies; 10+ messages in thread
From: Andi Kleen @ 2013-10-23  2:11 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: David Miller, andi, akpm, linux-kernel, ak, netdev

On Tue, Oct 22, 2013 at 08:36:25PM +0200, Sebastian Andrzej Siewior wrote:
> 
> There is also a C function doing the same thing. Unless the asm code is
> 110% faster we could stick to the C function.

I have no idea if it is or not. But doing it this way would be
acceptable for me.

Thanks for the patch.

-Andi

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 11/11] asmlinkage, wan/sbni: Make inline assembler symbols visible and assembler global
  2013-10-23  2:09     ` [PATCH 11/11] asmlinkage, wan/sbni: Make inline assembler symbols visible and assembler global Andi Kleen
@ 2013-10-23  2:15       ` Florian Fainelli
  2013-10-23  2:22         ` Andi Kleen
  2013-10-23  6:28         ` Francois Romieu
  0 siblings, 2 replies; 10+ messages in thread
From: Florian Fainelli @ 2013-10-23  2:15 UTC (permalink / raw)
  To: Andi Kleen
  Cc: David Miller, Andrew Morton, linux-kernel@vger.kernel.org, ak,
	netdev

2013/10/22 Andi Kleen <andi@firstfloor.org>:
> On Tue, Oct 22, 2013 at 01:59:28PM -0400, David Miller wrote:
>> From: Andi Kleen <andi@firstfloor.org>
>> Date: Tue, 22 Oct 2013 09:12:26 -0700
>>
>> > From: Andi Kleen <ak@linux.intel.com>
>> >
>> > - Inline assembler defining C callable code has to be global
>> > - The function has to be visible
>> >
>> > Do this in wan/sbni
>> >
>> > Cc: netdev@vger.kernel.org
>> > Signed-off-by: Andi Kleen <ak@linux.intel.com>
>>
>> Is it really impossible to use the generic crc32 support instead of this
>> unsightly custom inline assembler?
>
> Yes it's impossible.
>
> There's no way for me to test it, and this would be a far too big change
> to submit untested.
>
> Also my only interest here is this thing not breaking my LTO
> allyesconfig build. I'm not even sure what it does.
>
> Just because some legacy driver breaks my build you cannot make me
> accountable for maintaining it now.
>
> If it's not possible to get this trivial change in I would need
> to mark it "depends on !LTO" in Kconfig.

How about scheduling the WAN drivers for removal? Does anybody
actually use them nowadays?
--
Florian

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 11/11] asmlinkage, wan/sbni: Make inline assembler symbols visible and assembler global
  2013-10-23  2:15       ` Florian Fainelli
@ 2013-10-23  2:22         ` Andi Kleen
  2013-10-23  6:28         ` Francois Romieu
  1 sibling, 0 replies; 10+ messages in thread
From: Andi Kleen @ 2013-10-23  2:22 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Andi Kleen, David Miller, Andrew Morton,
	linux-kernel@vger.kernel.org, ak, netdev

> How about scheduling the WAN drivers for removal? Does anybody
> actually use them nowadays?

Perhaps at least the ISA devices? ISA hardware should be nearing
end of live by now.

Looking at the git log there doesn't seem to be any real changes,
other than tree sweeps or build fixes.

But I must admit I don't really know much about them.

-Andi

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 11/11] asmlinkage, wan/sbni: Make inline assembler symbols visible and assembler global
  2013-10-23  2:15       ` Florian Fainelli
  2013-10-23  2:22         ` Andi Kleen
@ 2013-10-23  6:28         ` Francois Romieu
  1 sibling, 0 replies; 10+ messages in thread
From: Francois Romieu @ 2013-10-23  6:28 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Andi Kleen, David Miller, Andrew Morton,
	linux-kernel@vger.kernel.org, ak, netdev, Christophe Leroy

Florian Fainelli <f.fainelli@gmail.com> :
[...]
> How about scheduling the WAN drivers for removal? Does anybody
> actually use them nowadays?

There is still some interest in WAN drivers, see Christophe Leroy
posting on 2013/10/13 "[PATCH] WAN: Adding support for Infineon PEF2256
E1 chipset" (buggy but it's a different topic).

-- 
Ueimor

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] net: wan: sbni: remove assembly crc32 code
  2013-10-22 18:36     ` [PATCH] net: wan: sbni: remove assembly crc32 code Sebastian Andrzej Siewior
  2013-10-22 18:38       ` David Miller
  2013-10-23  2:11       ` Andi Kleen
@ 2013-10-25 23:27       ` David Miller
  2 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2013-10-25 23:27 UTC (permalink / raw)
  To: sebastian; +Cc: andi, akpm, linux-kernel, ak, netdev

From: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
Date: Tue, 22 Oct 2013 20:36:25 +0200

> There is also a C function doing the same thing. Unless the asm code is
> 110% faster we could stick to the C function.
> 
> Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>

Applied.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-10-25 23:27 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <1382458346-24811-1-git-send-email-andi@firstfloor.org>
2013-10-22 16:12 ` [PATCH 11/11] asmlinkage, wan/sbni: Make inline assembler symbols visible and assembler global Andi Kleen
2013-10-22 17:59   ` David Miller
2013-10-22 18:36     ` [PATCH] net: wan: sbni: remove assembly crc32 code Sebastian Andrzej Siewior
2013-10-22 18:38       ` David Miller
2013-10-23  2:11       ` Andi Kleen
2013-10-25 23:27       ` David Miller
2013-10-23  2:09     ` [PATCH 11/11] asmlinkage, wan/sbni: Make inline assembler symbols visible and assembler global Andi Kleen
2013-10-23  2:15       ` Florian Fainelli
2013-10-23  2:22         ` Andi Kleen
2013-10-23  6:28         ` Francois Romieu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).