arch: hexagon: Kconfig: add HAVE_DMA_ATTR in Kconfig and remove "linux/dma-mapping.h" from "asm/dma-mapping.h"

PATCH In review Chen Gang 9 participants 56 messages Nov 19, 2013 – Dec 6, 2013

Subsystems (7)

Networking Network drivers Media (V4L/DVB) SCSI +3 more subsystems

lore

Share & tools

lore

Chen Gang

author

2013-11-19 04:55 UTC #0 lore raw

PATCH 1/1 arch: hexagon: Kconfig: add HAVE_DMA_ATTR in Kconfig and remove "linux/dma-mapping.h" from "asm/dma-mapping.h"

When HAS_DMA, and also need use generic implementation, HAVE_DMA_ATTR must be enabled, or can not pass compiling with allmodconfig, the related error:

CC [M] drivers/ata/libata-core.o
drivers/ata/libata-core.c: In function 'ata_sg_clean':
drivers/ata/libata-core.c:4598:3: error: implicit declaration of function 'dma_unmap_sg' [-Werror=implicit-function-declaration] drivers/ata/libata-core.c: In function 'ata_sg_setup':
drivers/ata/libata-core.c:4708:2: error: implicit declaration of function 'dma_map_sg' [-Werror=implicit-function-declaration]

"linux/dma-mapping.h" will include "asm/dma-mapping.h", so need remove "linux/dma-mapping.h" from "asm/dma-mapping.h",

Signed-off-by Chen Gang <[email protected]>

---
 arch/hexagon/Kconfig                   |    1 +
 arch/hexagon/include/asm/dma-mapping.h |    1 -
 2 files changed, 1 insertions(+), 1 deletions(-)

arch/hexagon/Kconfig +1 -0

index 09df260..fbc5c78 100644
@@ -28,6 +28,7 @@ config HEXAGON
 	select GENERIC_CLOCKEVENTS_BROADCAST
 	select MODULES_USE_ELF_RELA
 	select GENERIC_CPU_DEVICES
+	select HAVE_DMA_ATTRS
 	---help---
 	  Qualcomm Hexagon is a processor architecture designed for high
 	  performance and low power across a wide variety of applications.

arch/hexagon/include/asm/dma-mapping.h +0 -1

index 85e9935..1696542 100644
@@ -25,7 +25,6 @@
 #include <linux/cache.h>
 #include <linux/mm.h>
 #include <linux/scatterlist.h>
-#include <linux/dma-mapping.h>
 #include <linux/dma-debug.h>
 #include <linux/dma-attrs.h>
 #include <asm/io.h>

Signature · 1 line

1.7.7.6

rkuo

Acked-by on v1 1/1

2013-11-25 01:19 UTC #1 lore raw

Quoted 10 lines · Chen Gang

When HAS_DMA, and also need use generic implementation, HAVE_DMA_ATTR must be enabled, or can not pass compiling with allmodconfig, the related error:
CC [M] drivers/ata/libata-core.o
drivers/ata/libata-core.c: In function 'ata_sg_clean':
drivers/ata/libata-core.c:4598:3: error: implicit declaration of function 'dma_unmap_sg' [-Werror=implicit-function-declaration] drivers/ata/libata-core.c: In function 'ata_sg_setup':
drivers/ata/libata-core.c:4708:2: error: implicit declaration of function 'dma_map_sg' [-Werror=implicit-function-declaration]

Quoted 5 lines

"linux/dma-mapping.h" will include "asm/dma-mapping.h", so need remove "linux/dma-mapping.h" from "asm/dma-mapping.h",
Signed-off-by Chen Gang <[email protected]>

Acked-by Richard Kuo <[email protected]>

Signature · 3 lines

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Chen Gang

author

2013-11-25 02:37 UTC #2 lore raw

cover arch: hexagon: include: asm: add prefix "vm_" for all enum members in "hexagon_vm.h"

Append "vm_" to all enum members (which are too common to make conflict with another sub-systems). The related error with allmodconfig:

CC [M] drivers/md/raid1.o
drivers/md/raid1.c:1440:13: error: 'status' redeclared as different kind of symbol arch/hexagon/include/asm/hexagon_vm.h:76:2: note: previous definition of 'status' was here

Also fix a typo issue: use 'affinity' instead of 'locdis' for __vmintop_affinity() in "hexagon_vm.h"

Signed-off-by Chen Gang <[email protected]>

---
 arch/hexagon/include/asm/hexagon_vm.h |   72 ++++++++++++++++----------------
 1 files changed, 36 insertions(+), 36 deletions(-)

Chen Gang

author

2013-11-25 02:38 UTC #3 lore raw

PATCH 1/2 arch: hexagon: include: asm: add prefix "vm_" for all enum members in "hexagon_vm.h"

Append "vm_" to all enum members (which are too common to make conflict with another sub-systems). The related error with allmodconfig:

Signed-off-by Chen Gang <[email protected]>

---
 arch/hexagon/include/asm/hexagon_vm.h |   70 ++++++++++++++++----------------
 1 files changed, 35 insertions(+), 35 deletions(-)

arch/hexagon/include/asm/hexagon_vm.h +35 ~~-35~~

index 67bb6d6..e1e0470 100644
@@ -55,27 +55,27 @@
 #ifndef __ASSEMBLY__
 
 enum VM_CACHE_OPS {
-	ickill,
-	dckill,
-	l2kill,
-	dccleaninva,
-	icinva,
-	idsync,
-	fetch_cfg
+	vm_ickill,
+	vm_dckill,
+	vm_l2kill,
+	vm_dccleaninva,
+	vm_icinva,
+	vm_idsync,
+	vm_fetch_cfg
 };
 
 enum VM_INT_OPS {
-	nop,
-	globen,
-	globdis,
-	locen,
-	locdis,
-	affinity,
-	get,
-	peek,
-	status,
-	post,
-	clear
+	vm_nop,
+	vm_globen,
+	vm_globdis,
+	vm_locen,
+	vm_locdis,
+	vm_affinity,
+	vm_get,
+	vm_peek,
+	vm_status,
+	vm_post,
+	vm_clear
 };
 
 extern void _K_VM_event_vector(void);

@@ -98,65 +98,65 @@ long __vmvpid(void);
 
 static inline long __vmcache_ickill(void)
 {
-	return __vmcache(ickill, 0, 0);
+	return __vmcache(vm_ickill, 0, 0);
 }
 
 static inline long __vmcache_dckill(void)
 {
-	return __vmcache(dckill, 0, 0);
+	return __vmcache(vm_dckill, 0, 0);
 }
 
 static inline long __vmcache_l2kill(void)
 {
-	return __vmcache(l2kill, 0, 0);
+	return __vmcache(vm_l2kill, 0, 0);
 }
 
 static inline long __vmcache_dccleaninva(unsigned long addr, unsigned long len)
 {
-	return __vmcache(dccleaninva, addr, len);
+	return __vmcache(vm_dccleaninva, addr, len);
 }
 
 static inline long __vmcache_icinva(unsigned long addr, unsigned long len)
 {
-	return __vmcache(icinva, addr, len);
+	return __vmcache(vm_icinva, addr, len);
 }
 
 static inline long __vmcache_idsync(unsigned long addr,
 					   unsigned long len)
 {
-	return __vmcache(idsync, addr, len);
+	return __vmcache(vm_idsync, addr, len);
 }
 
 static inline long __vmcache_fetch_cfg(unsigned long val)
 {
-	return __vmcache(fetch_cfg, val, 0);
+	return __vmcache(vm_fetch_cfg, val, 0);
 }
 
 /* interrupt operations  */
 
 static inline long __vmintop_nop(void)
 {
-	return __vmintop(nop, 0, 0, 0, 0);
+	return __vmintop(vm_nop, 0, 0, 0, 0);
 }
 
 static inline long __vmintop_globen(long i)
 {
-	return __vmintop(globen, i, 0, 0, 0);
+	return __vmintop(vm_globen, i, 0, 0, 0);
 }
 
 static inline long __vmintop_globdis(long i)
 {
-	return __vmintop(globdis, i, 0, 0, 0);
+	return __vmintop(vm_globdis, i, 0, 0, 0);
 }
 
 static inline long __vmintop_locen(long i)
 {
-	return __vmintop(locen, i, 0, 0, 0);
+	return __vmintop(vm_locen, i, 0, 0, 0);
 }
 
 static inline long __vmintop_locdis(long i)
 {
-	return __vmintop(locdis, i, 0, 0, 0);
+	return __vmintop(vm_locdis, i, 0, 0, 0);
 }
 
 static inline long __vmintop_affinity(long i, long cpu)

@@ -166,27 +166,27 @@ static inline long __vmintop_affinity(long i, long cpu)
 
 static inline long __vmintop_get(void)
 {
-	return __vmintop(get, 0, 0, 0, 0);
+	return __vmintop(vm_get, 0, 0, 0, 0);
 }
 
 static inline long __vmintop_peek(void)
 {
-	return __vmintop(peek, 0, 0, 0, 0);
+	return __vmintop(vm_peek, 0, 0, 0, 0);
 }
 
 static inline long __vmintop_status(long i)
 {
-	return __vmintop(status, i, 0, 0, 0);
+	return __vmintop(vm_status, i, 0, 0, 0);
 }
 
 static inline long __vmintop_post(long i)
 {
-	return __vmintop(post, i, 0, 0, 0);
+	return __vmintop(vm_post, i, 0, 0, 0);
 }
 
 static inline long __vmintop_clear(long i)
 {
-	return __vmintop(clear, i, 0, 0, 0);
+	return __vmintop(vm_clear, i, 0, 0, 0);
 }
 
 #else /* Only assembly code should reference these */

Signature · 1 line

1.7.7.6

Chen Gang

author

2013-11-25 02:39 UTC #4 lore raw

PATCH 2/2 arch: hexagon: include: asm: use 'affinity' instead of 'locdis' for __vmintop_affinity() in "hexagon_vm.h"

All __vmintop_*() use __vmintop(*, ...), so __vmintop_affinity() need use __vmintop(vm_affinity, ...).

Signed-off-by Chen Gang <[email protected]>

---
 arch/hexagon/include/asm/hexagon_vm.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

arch/hexagon/include/asm/hexagon_vm.h +1 -1

index e1e0470..3ea99c1 100644
@@ -161,7 +161,7 @@ static inline long __vmintop_locdis(long i)
 
 static inline long __vmintop_affinity(long i, long cpu)
 {
-	return __vmintop(locdis, i, cpu, 0, 0);
+	return __vmintop(vm_affinity, i, cpu, 0, 0);
 }
 
 static inline long __vmintop_get(void)

Signature · 1 line

1.7.7.6

Chen Gang

author

2013-11-28 08:49 UTC #5 lore raw

PATCH v2 1/1 arch: hexagon: include: asm: add prefix "hvm[ci]_" for all enum members in "hexagon_vm.h"

Append "hvmc_" or "hvmi_" to all related enum members (which are too common to make conflict with another sub-systems). The related error with allmodconfig:

Also use 'affinity' instead of 'locdis' for __vmintop_affinity().

Signed-off-by Chen Gang <[email protected]>

---
 arch/hexagon/include/asm/hexagon_vm.h |   72 ++++++++++++++++----------------
 1 files changed, 36 insertions(+), 36 deletions(-)

arch/hexagon/include/asm/hexagon_vm.h +36 ~~-36~~

index 67bb6d6..1f6918b 100644
@@ -55,27 +55,27 @@
 #ifndef __ASSEMBLY__
 
 enum VM_CACHE_OPS {
-	ickill,
-	dckill,
-	l2kill,
-	dccleaninva,
-	icinva,
-	idsync,
-	fetch_cfg
+	hvmc_ickill,
+	hvmc_dckill,
+	hvmc_l2kill,
+	hvmc_dccleaninva,
+	hvmc_icinva,
+	hvmc_idsync,
+	hvmc_fetch_cfg
 };
 
 enum VM_INT_OPS {
-	nop,
-	globen,
-	globdis,
-	locen,
-	locdis,
-	affinity,
-	get,
-	peek,
-	status,
-	post,
-	clear
+	hvmi_nop,
+	hvmi_globen,
+	hvmi_globdis,
+	hvmi_locen,
+	hvmi_locdis,
+	hvmi_affinity,
+	hvmi_get,
+	hvmi_peek,
+	hvmi_status,
+	hvmi_post,
+	hvmi_clear
 };
 
 extern void _K_VM_event_vector(void);

@@ -98,95 +98,95 @@ long __vmvpid(void);
 
 static inline long __vmcache_ickill(void)
 {
-	return __vmcache(ickill, 0, 0);
+	return __vmcache(hvmc_ickill, 0, 0);
 }
 
 static inline long __vmcache_dckill(void)
 {
-	return __vmcache(dckill, 0, 0);
+	return __vmcache(hvmc_dckill, 0, 0);
 }
 
 static inline long __vmcache_l2kill(void)
 {
-	return __vmcache(l2kill, 0, 0);
+	return __vmcache(hvmc_l2kill, 0, 0);
 }
 
 static inline long __vmcache_dccleaninva(unsigned long addr, unsigned long len)
 {
-	return __vmcache(dccleaninva, addr, len);
+	return __vmcache(hvmc_dccleaninva, addr, len);
 }
 
 static inline long __vmcache_icinva(unsigned long addr, unsigned long len)
 {
-	return __vmcache(icinva, addr, len);
+	return __vmcache(hvmc_icinva, addr, len);
 }
 
 static inline long __vmcache_idsync(unsigned long addr,
 					   unsigned long len)
 {
-	return __vmcache(idsync, addr, len);
+	return __vmcache(hvmc_idsync, addr, len);
 }
 
 static inline long __vmcache_fetch_cfg(unsigned long val)
 {
-	return __vmcache(fetch_cfg, val, 0);
+	return __vmcache(hvmc_fetch_cfg, val, 0);
 }
 
 /* interrupt operations  */
 
 static inline long __vmintop_nop(void)
 {
-	return __vmintop(nop, 0, 0, 0, 0);
+	return __vmintop(hvmi_nop, 0, 0, 0, 0);
 }
 
 static inline long __vmintop_globen(long i)
 {
-	return __vmintop(globen, i, 0, 0, 0);
+	return __vmintop(hvmi_globen, i, 0, 0, 0);
 }
 
 static inline long __vmintop_globdis(long i)
 {
-	return __vmintop(globdis, i, 0, 0, 0);
+	return __vmintop(hvmi_globdis, i, 0, 0, 0);
 }
 
 static inline long __vmintop_locen(long i)
 {
-	return __vmintop(locen, i, 0, 0, 0);
+	return __vmintop(hvmi_locen, i, 0, 0, 0);
 }
 
 static inline long __vmintop_locdis(long i)
 {
-	return __vmintop(locdis, i, 0, 0, 0);
+	return __vmintop(hvmi_locdis, i, 0, 0, 0);
 }
 
 static inline long __vmintop_affinity(long i, long cpu)
 {
-	return __vmintop(locdis, i, cpu, 0, 0);
+	return __vmintop(hvmi_affinity, i, cpu, 0, 0);
 }
 
 static inline long __vmintop_get(void)
 {
-	return __vmintop(get, 0, 0, 0, 0);
+	return __vmintop(hvmi_get, 0, 0, 0, 0);
 }
 
 static inline long __vmintop_peek(void)
 {
-	return __vmintop(peek, 0, 0, 0, 0);
+	return __vmintop(hvmi_peek, 0, 0, 0, 0);
 }
 
 static inline long __vmintop_status(long i)
 {
-	return __vmintop(status, i, 0, 0, 0);
+	return __vmintop(hvmi_status, i, 0, 0, 0);
 }
 
 static inline long __vmintop_post(long i)
 {
-	return __vmintop(post, i, 0, 0, 0);
+	return __vmintop(hvmi_post, i, 0, 0, 0);
 }
 
 static inline long __vmintop_clear(long i)
 {
-	return __vmintop(clear, i, 0, 0, 0);
+	return __vmintop(hvmi_clear, i, 0, 0, 0);
 }
 
 #else /* Only assembly code should reference these */

Signature · 1 line

1.7.7.6

rkuo

Acked-by on v2 1/1

2013-12-06 18:22 UTC #6 lore raw

Re: [PATCH v2] arch: hexagon: include: asm: add prefix "hvm[ci]_" for all enum members in "hexagon_vm.h"

Quoted 8 lines · Chen Gang

Append "hvmc_" or "hvmi_" to all related enum members (which are too common to make conflict with another sub-systems). The related error with allmodconfig:
CC [M] drivers/md/raid1.o
drivers/md/raid1.c:1440:13: error: 'status' redeclared as different kind of symbol arch/hexagon/include/asm/hexagon_vm.h:76:2: note: previous definition of 'status' was here

Quoted 8 lines

Also use 'affinity' instead of 'locdis' for __vmintop_affinity().
Signed-off-by Chen Gang <[email protected]>
---
 arch/hexagon/include/asm/hexagon_vm.h |   72 ++++++++++++++++----------------
 1 files changed, 36 insertions(+), 36 deletions(-)

Thanks for catching the affinity typo too.

Acked-by Richard Kuo <[email protected]>

Signature · 3 lines

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Chen Gang

author

2013-11-26 04:34 UTC #7 lore raw

Re: [PATCH 1/2] arch: hexagon: include: asm: add prefix "vm_" for all enum members in "hexagon_vm.h"

In reply to Chen Gang

Quoted 7 lines · Chen Gang

Append "vm_" to all enum members (which are too common to make conflict with another sub-systems). The related error with allmodconfig:
CC [M] drivers/md/raid1.o
drivers/md/raid1.c:1440:13: error: 'status' redeclared as different kind of symbol arch/hexagon/include/asm/hexagon_vm.h:76:2: note: previous definition of 'status' was here

Oh, sorry, The new prefix "vm_" is still conflict with others. One case is below (bottom of this mail)

I will use "hvmc_" for VM_CACHE_OPS and "hvmi_" for VM_INT_OPS instead of. If get no rejections within 1 day, I will send patch v2 for it.

And also I will merge the related 2 patches together (although they are for 2 issues).

Quoted 39 lines

Signed-off-by Chen Gang <[email protected]>

---
 arch/hexagon/include/asm/hexagon_vm.h |   70 ++++++++++++++++----------------
 1 files changed, 35 insertions(+), 35 deletions(-)

arch/hexagon/include/asm/hexagon_vm.h +7 ~~-15~~

index 67bb6d6..e1e0470 100644
@@ -55,27 +55,27 @@
 #ifndef __ASSEMBLY__
 
 enum VM_CACHE_OPS {
-	ickill,
-	dckill,
-	l2kill,
-	dccleaninva,
-	icinva,
-	idsync,
-	fetch_cfg
+	vm_ickill,
+	vm_dckill,
+	vm_l2kill,
+	vm_dccleaninva,
+	vm_icinva,
+	vm_idsync,
+	vm_fetch_cfg
 };
 
 enum VM_INT_OPS {
-	nop,
-	globen,
-	globdis,
-	locen,
-	locdis,
-	affinity,
-	get,
-	peek,

arch/hexagon/include/asm/hexagon_vm.h line 74

Quoted 10 lines

arch/hexagon/include/asm/hexagon_vm.h +7 -3

-	status,
-	post,
-	clear
+	vm_nop,
+	vm_globen,
+	vm_globdis,
+	vm_locen,
+	vm_locdis,
+	vm_affinity,
+	vm_get,

The new prefix "vm_" for "get" is still conflict with others:

CC [M] drivers/virtio/virtio_mmio.o
drivers/virtio/virtio_mmio.c:170:13: error: 'vm_get' redeclared as different kind of symbol arch/hexagon/include/asm/hexagon_vm.h:74:2: note: previous definition of 'vm_get' was here

Thanks.

Signature · 1 line

Chen Gang

Chen Gang

author

2013-11-27 02:27 UTC #8 lore raw

PATCH 1/1 drivers: scsi: scsi_lib.c: add prefix "SCSILIB_" to macro "SP"

the macro "SP" is too common to make conflict with some architectures, so recommend to add prefix for it.

The related warning (with allmodconfig for hexagon):

CC [M] drivers/scsi/scsi_lib.o
drivers/scsi/scsi_lib.c:46:0: warning: "SP" redefined [enabled by default] arch/hexagon/include/uapi/asm/registers.h:9:0: note: this is the location of the previous definition

Signed-off-by Chen Gang <[email protected]>

---
 drivers/scsi/scsi_lib.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

drivers/scsi/scsi_lib.c +8 -8

index 7bd7f0d..f78e21b 100644
@@ -43,28 +43,28 @@ struct scsi_host_sg_pool {
 	mempool_t	*pool;
 };
 
-#define SP(x) { x, "sgpool-" __stringify(x) }
+#define SCSILIB_SP(x) { x, "sgpool-" __stringify(x) }
 #if (SCSI_MAX_SG_SEGMENTS < 32)
 #error SCSI_MAX_SG_SEGMENTS is too small (must be 32 or greater)
 #endif
 static struct scsi_host_sg_pool scsi_sg_pools[] = {
-	SP(8),
-	SP(16),
+	SCSILIB_SP(8),
+	SCSILIB_SP(16),
 #if (SCSI_MAX_SG_SEGMENTS > 32)
-	SP(32),
+	SCSILIB_SP(32),
 #if (SCSI_MAX_SG_SEGMENTS > 64)
-	SP(64),
+	SCSILIB_SP(64),
 #if (SCSI_MAX_SG_SEGMENTS > 128)
-	SP(128),
+	SCSILIB_SP(128),
 #if (SCSI_MAX_SG_SEGMENTS > 256)
 #error SCSI_MAX_SG_SEGMENTS is too large (256 MAX)
 #endif
 #endif
 #endif
 #endif
-	SP(SCSI_MAX_SG_SEGMENTS)
+	SCSILIB_SP(SCSI_MAX_SG_SEGMENTS)
 };
-#undef SP
+#undef SCSILIB_SP
 
 struct kmem_cache *scsi_sdb_cache;

Signature · 1 line

1.7.7.6

Bart Van Assche

2013-12-01 16:17 UTC #9 lore raw

Re: [PATCH] drivers: scsi: scsi_lib.c: add prefix "SCSILIB_" to macro "SP"

Quoted 45 lines · Chen Gang

the macro "SP" is too common to make conflict with some architectures, so recommend to add prefix for it.
The related warning (with allmodconfig for hexagon):
CC [M] drivers/scsi/scsi_lib.o
drivers/scsi/scsi_lib.c:46:0: warning: "SP" redefined [enabled by default] arch/hexagon/include/uapi/asm/registers.h:9:0: note: this is the location of the previous definition
Signed-off-by Chen Gang <[email protected]>
---
 drivers/scsi/scsi_lib.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)
drivers/scsi/scsi_lib.c +6 -6
index 7bd7f0d..f78e21b 100644
@@ -43,28 +43,28 @@ struct scsi_host_sg_pool {
 	mempool_t	*pool;
 };
 
-#define SP(x) { x, "sgpool-" __stringify(x) }
+#define SCSILIB_SP(x) { x, "sgpool-" __stringify(x) }
 #if (SCSI_MAX_SG_SEGMENTS < 32)
 #error SCSI_MAX_SG_SEGMENTS is too small (must be 32 or greater)
 #endif
 static struct scsi_host_sg_pool scsi_sg_pools[] = {
-	SP(8),
-	SP(16),
+	SCSILIB_SP(8),
+	SCSILIB_SP(16),
 #if (SCSI_MAX_SG_SEGMENTS > 32)
-	SP(32),
+	SCSILIB_SP(32),
 #if (SCSI_MAX_SG_SEGMENTS > 64)
-	SP(64),
+	SCSILIB_SP(64),
 #if (SCSI_MAX_SG_SEGMENTS > 128)
-	SP(128),
+	SCSILIB_SP(128),
 #if (SCSI_MAX_SG_SEGMENTS > 256)
 #error SCSI_MAX_SG_SEGMENTS is too large (256 MAX)
 #endif

drivers/scsi/scsi_lib.c struct scsi_host_sg_pool { · line 69

Quoted 10 lines

drivers/scsi/scsi_lib.c +2 -2

 #endif
 #endif
 #endif
-	SP(SCSI_MAX_SG_SEGMENTS)
+	SCSILIB_SP(SCSI_MAX_SG_SEGMENTS)
 };
-#undef SP
+#undef SCSILIB_SP
 
 struct kmem_cache *scsi_sdb_cache;

Sorry but the "SCSILIB_SP" name doesn't look very descriptive to me. There are probably better choices possible. How about using e.g. SG_POOL() instead ?

Bart.

Chen Gang

author

2013-12-02 00:34 UTC #10 lore raw

Re: [PATCH] drivers: scsi: scsi_lib.c: add prefix "SCSILIB_" to macro "SP"

Quoted 55 lines · Bart Van Assche

Quoted 54 lines · Chen Gang

the macro "SP" is too common to make conflict with some architectures, so recommend to add prefix for it.

The related warning (with allmodconfig for hexagon):

Signed-off-by Chen Gang <[email protected]>

---
 drivers/scsi/scsi_lib.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

drivers/scsi/scsi_lib.c +8 -8

index 7bd7f0d..f78e21b 100644
@@ -43,28 +43,28 @@ struct scsi_host_sg_pool {
 	mempool_t	*pool;
 };
 
-#define SP(x) { x, "sgpool-" __stringify(x) }
+#define SCSILIB_SP(x) { x, "sgpool-" __stringify(x) }
 #if (SCSI_MAX_SG_SEGMENTS < 32)
 #error SCSI_MAX_SG_SEGMENTS is too small (must be 32 or greater)
 #endif
 static struct scsi_host_sg_pool scsi_sg_pools[] = {
-	SP(8),
-	SP(16),
+	SCSILIB_SP(8),
+	SCSILIB_SP(16),
 #if (SCSI_MAX_SG_SEGMENTS > 32)
-	SP(32),
+	SCSILIB_SP(32),
 #if (SCSI_MAX_SG_SEGMENTS > 64)
-	SP(64),
+	SCSILIB_SP(64),
 #if (SCSI_MAX_SG_SEGMENTS > 128)
-	SP(128),
+	SCSILIB_SP(128),
 #if (SCSI_MAX_SG_SEGMENTS > 256)
 #error SCSI_MAX_SG_SEGMENTS is too large (256 MAX)
 #endif
 #endif
 #endif
 #endif
-	SP(SCSI_MAX_SG_SEGMENTS)
+	SCSILIB_SP(SCSI_MAX_SG_SEGMENTS)
 };
-#undef SP
+#undef SCSILIB_SP

Quoted 6 lines

drivers/scsi/scsi_lib.c struct scsi_host_sg_pool { · line 69
Quoted 1 line
drivers/scsi/scsi_lib.c +0 -0
 struct kmem_cache *scsi_sdb_cache;
Sorry but the "SCSILIB_SP" name doesn't look very descriptive to me. There are probably better choices possible. How about using e.g. SG_POOL() instead ?

That sounds good to me, I will send patch v2, tomorrow (today I have to do some another urgent things, if this patch is also urgent, please help send). :-)

Thanks.

Signature · 1 line

Chen Gang

James Bottomley

maintainer

2013-12-02 00:49 UTC #11 lore raw

Re: [PATCH] drivers: scsi: scsi_lib.c: add prefix "SCSILIB_" to macro "SP"

Quoted 63 lines · Chen Gang

Quoted 55 lines · Bart Van Assche
Quoted 54 lines · Chen Gang
the macro "SP" is too common to make conflict with some architectures, so recommend to add prefix for it.
The related warning (with allmodconfig for hexagon):
CC [M] drivers/scsi/scsi_lib.o
drivers/scsi/scsi_lib.c:46:0: warning: "SP" redefined [enabled by default] arch/hexagon/include/uapi/asm/registers.h:9:0: note: this is the location of the previous definition
Signed-off-by Chen Gang <[email protected]>
---
 drivers/scsi/scsi_lib.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)
drivers/scsi/scsi_lib.c +8 -8
index 7bd7f0d..f78e21b 100644
@@ -43,28 +43,28 @@ struct scsi_host_sg_pool {
 	mempool_t	*pool;
 };
 
-#define SP(x) { x, "sgpool-" __stringify(x) }
+#define SCSILIB_SP(x) { x, "sgpool-" __stringify(x) }
 #if (SCSI_MAX_SG_SEGMENTS < 32)
 #error SCSI_MAX_SG_SEGMENTS is too small (must be 32 or greater)
 #endif
 static struct scsi_host_sg_pool scsi_sg_pools[] = {
-	SP(8),
-	SP(16),
+	SCSILIB_SP(8),
+	SCSILIB_SP(16),
 #if (SCSI_MAX_SG_SEGMENTS > 32)
-	SP(32),
+	SCSILIB_SP(32),
 #if (SCSI_MAX_SG_SEGMENTS > 64)
-	SP(64),
+	SCSILIB_SP(64),
 #if (SCSI_MAX_SG_SEGMENTS > 128)
-	SP(128),
+	SCSILIB_SP(128),
 #if (SCSI_MAX_SG_SEGMENTS > 256)
 #error SCSI_MAX_SG_SEGMENTS is too large (256 MAX)
 #endif
 #endif
 #endif
 #endif
-	SP(SCSI_MAX_SG_SEGMENTS)
+	SCSILIB_SP(SCSI_MAX_SG_SEGMENTS)
 };
-#undef SP
+#undef SCSILIB_SP
 
Quoted 6 lines
drivers/scsi/scsi_lib.c struct scsi_host_sg_pool { · line 69
Quoted 1 line
drivers/scsi/scsi_lib.c +0 -0
 struct kmem_cache *scsi_sdb_cache;
Sorry but the "SCSILIB_SP" name doesn't look very descriptive to me. There are probably better choices possible. How about using e.g. SG_POOL() instead ?

Quoted 3 lines

That sounds good to me, I will send patch v2, tomorrow (today I have to do some another urgent things, if this patch is also urgent, please help send). :-)

No, this is the wrong thing to do. Exported headers should be namespace protected, so the thing wrong is what's exporting the problem to us, namely hexagon.

James

Chen Gang

author

2013-12-02 10:14 UTC #12 lore raw

Re: [PATCH] drivers: scsi: scsi_lib.c: add prefix "SCSILIB_" to macro "SP"

Quoted 64 lines · James Bottomley

Quoted 63 lines · Chen Gang
Quoted 61 lines · Bart Van Assche
Quoted 45 lines · Chen Gang
the macro "SP" is too common to make conflict with some architectures, so recommend to add prefix for it.
The related warning (with allmodconfig for hexagon):
CC [M] drivers/scsi/scsi_lib.o
drivers/scsi/scsi_lib.c:46:0: warning: "SP" redefined [enabled by default] arch/hexagon/include/uapi/asm/registers.h:9:0: note: this is the location of the previous definition
Signed-off-by Chen Gang <[email protected]>
---
 drivers/scsi/scsi_lib.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)
drivers/scsi/scsi_lib.c +6 -6
index 7bd7f0d..f78e21b 100644
@@ -43,28 +43,28 @@ struct scsi_host_sg_pool {
 	mempool_t	*pool;
 };
 
-#define SP(x) { x, "sgpool-" __stringify(x) }
+#define SCSILIB_SP(x) { x, "sgpool-" __stringify(x) }
 #if (SCSI_MAX_SG_SEGMENTS < 32)
 #error SCSI_MAX_SG_SEGMENTS is too small (must be 32 or greater)
 #endif
 static struct scsi_host_sg_pool scsi_sg_pools[] = {
-	SP(8),
-	SP(16),
+	SCSILIB_SP(8),
+	SCSILIB_SP(16),
 #if (SCSI_MAX_SG_SEGMENTS > 32)
-	SP(32),
+	SCSILIB_SP(32),
 #if (SCSI_MAX_SG_SEGMENTS > 64)
-	SP(64),
+	SCSILIB_SP(64),
 #if (SCSI_MAX_SG_SEGMENTS > 128)
-	SP(128),
+	SCSILIB_SP(128),
 #if (SCSI_MAX_SG_SEGMENTS > 256)
 #error SCSI_MAX_SG_SEGMENTS is too large (256 MAX)
 #endif
drivers/scsi/scsi_lib.c struct scsi_host_sg_pool { · line 69
Quoted 10 lines
drivers/scsi/scsi_lib.c +2 -2
 #endif
 #endif
 #endif
-	SP(SCSI_MAX_SG_SEGMENTS)
+	SCSILIB_SP(SCSI_MAX_SG_SEGMENTS)
 };
-#undef SP
+#undef SCSILIB_SP
 
 struct kmem_cache *scsi_sdb_cache;
Sorry but the "SCSILIB_SP" name doesn't look very descriptive to me. There are probably better choices possible. How about using e.g. SG_POOL() instead ?

Quoted 8 lines

Quoted 3 lines
That sounds good to me, I will send patch v2, tomorrow (today I have to do some another urgent things, if this patch is also urgent, please help send). :-)
No, this is the wrong thing to do. Exported headers should be namespace protected, so the thing wrong is what's exporting the problem to us, namely hexagon.

If one issue occurs, normally, both sides need improvement.

For our issue:

- need try to keep uapi no touch ("arch/hexagon/uapi/asm/registers.h").

- improving our module is much easier than improving hexagon.

- for 'SP', it is really short enough to like a register name. SG_POOL seems more suitable for our 'sgpool' related operations.

It will be better to improve hexagon too, but it is not quit easy (maybe have to bear it), it is uapi :-(

Thanks

Signature · 1 line

Chen Gang

rkuo

2013-12-02 21:32 UTC #13 lore raw

Re: [PATCH] drivers: scsi: scsi_lib.c: add prefix "SCSILIB_" to macro "SP"

Quoted 6 lines · Chen Gang

If one issue occurs, normally, both sides need improvement.
For our issue:
- need try to keep uapi no touch ("arch/hexagon/uapi/asm/registers.h").

Quoted 8 lines

- improving our module is much easier than improving hexagon.
- for 'SP', it is really short enough to like a register name. SG_POOL seems more suitable for our 'sgpool' related operations.
It will be better to improve hexagon too, but it is not quit easy (maybe have to bear it), it is uapi :-(

I can't speak for SCSI, but that define in Hexagon isn't really used outside of that file anyways (the two SP macros themselves). I'm also pretty sure the userspace isn't currently using it anyways. Seeing as it's not really buying us anything, I'm fine with just removing it and continuing to review the other defines. I'll make that change and test it locally.

Thanks,
Richard Kuo

Signature · 3 lines

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Chen Gang

author

2013-12-03 11:42 UTC #14 lore raw

Re: [PATCH] drivers: scsi: scsi_lib.c: add prefix "SCSILIB_" to macro "SP"

Quoted 13 lines · rkuo

Quoted 12 lines · Chen Gang
If one issue occurs, normally, both sides need improvement.
For our issue:
- need try to keep uapi no touch ("arch/hexagon/uapi/asm/registers.h").
- improving our module is much easier than improving hexagon.
- for 'SP', it is really short enough to like a register name. SG_POOL seems more suitable for our 'sgpool' related operations.

Quoted 9 lines

Quoted 2 lines
It will be better to improve hexagon too, but it is not quit easy (maybe have to bear it), it is uapi :-(
I can't speak for SCSI, but that define in Hexagon isn't really used outside of that file anyways (the two SP macros themselves). I'm also pretty sure the userspace isn't currently using it anyways. Seeing as it's not really buying us anything, I'm fine with just removing it and continuing to review the other defines. I'll make that change and test it locally.

OK, thanks.

Quoted 5 lines

Thanks,
Richard Kuo

Signature · 1 line

Chen Gang

Chen Gang

author

2013-12-04 02:40 UTC #15 lore raw

PATCH v2 1/1 drivers: scsi: scsi_lib.c: use SG_POOL instead of SP

In reply to rkuo

the macro SP is too common to make conflict with others, so recommend to use another more readable and non-conflict macro SG_POOL instead of (and recommend others do not use SP either).

The related warning (with allmodconfig for hexagon):

Signed-off-by Chen Gang <[email protected]>

---
 drivers/scsi/scsi_lib.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

drivers/scsi/scsi_lib.c +8 -8

index 7bd7f0d..19967fa 100644
@@ -43,28 +43,28 @@ struct scsi_host_sg_pool {
 	mempool_t	*pool;
 };
 
-#define SP(x) { x, "sgpool-" __stringify(x) }
+#define SG_POOL(x) { x, "sgpool-" __stringify(x) }
 #if (SCSI_MAX_SG_SEGMENTS < 32)
 #error SCSI_MAX_SG_SEGMENTS is too small (must be 32 or greater)
 #endif
 static struct scsi_host_sg_pool scsi_sg_pools[] = {
-	SP(8),
-	SP(16),
+	SG_POOL(8),
+	SG_POOL(16),
 #if (SCSI_MAX_SG_SEGMENTS > 32)
-	SP(32),
+	SG_POOL(32),
 #if (SCSI_MAX_SG_SEGMENTS > 64)
-	SP(64),
+	SG_POOL(64),
 #if (SCSI_MAX_SG_SEGMENTS > 128)
-	SP(128),
+	SG_POOL(128),
 #if (SCSI_MAX_SG_SEGMENTS > 256)
 #error SCSI_MAX_SG_SEGMENTS is too large (256 MAX)
 #endif
 #endif
 #endif
 #endif
-	SP(SCSI_MAX_SG_SEGMENTS)
+	SG_POOL(SCSI_MAX_SG_SEGMENTS)
 };
-#undef SP
+#undef SG_POOL
 
 struct kmem_cache *scsi_sdb_cache;

Signature · 1 line

1.7.7.6

Chen Gang

author

2013-11-27 02:59 UTC #16 lore raw

PATCH 1/1 drivers: staging: ft1000: ft1000-usb: initialize 'status' with STATUS_SUCCESS in request_code_segment()

In reply to Chen Gang

If "!bool_case", it returns unexpected value instead of STATUS_SUCCESS, so need fix it, the related warning (with allmodconfig under hexagon):

    CC [M]  drivers/staging/ft1000/ft1000-usb/ft1000_download.o

drivers/staging/ft1000/ft1000-usb/ft1000_download.c: In function 'request_code_segment':
drivers/staging/ft1000/ft1000-usb/ft1000_download.c:581:6: warning: 'status' may be used uninitialized in this function [-Wuninitialized]

Signed-off-by Chen Gang <[email protected]>

---
 .../staging/ft1000/ft1000-usb/ft1000_download.c    |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/staging/ft1000/ft1000-usb/ft1000_download.c +1 -1

index 68ded17..15f3062 100644
@@ -578,7 +578,7 @@ static int request_code_segment(struct ft1000_usb *ft1000dev, u16 **s_file,
 		 u8 **c_file, const u8 *endpoint, bool boot_case)
 {
 	long word_length;
-	int status;
+	int status = STATUS_SUCCESS;
 
 	/*DEBUG("FT1000:REQUEST_CODE_SEGMENT\n");i*/
 	word_length = get_request_value(ft1000dev);

Signature · 1 line

1.7.7.6

Josh Triplett

Reviewed-by on v1 1/1

2013-11-27 09:18 UTC #17 lore raw

Re: [PATCH] drivers: staging: ft1000: ft1000-usb: initialize 'status' with STATUS_SUCCESS in request_code_segment()

Quoted 9 lines · Chen Gang

If "!bool_case", it returns unexpected value instead of STATUS_SUCCESS, so need fix it, the related warning (with allmodconfig under hexagon):
    CC [M]  drivers/staging/ft1000/ft1000-usb/ft1000_download.o
drivers/staging/ft1000/ft1000-usb/ft1000_download.c: In function 'request_code_segment':
drivers/staging/ft1000/ft1000-usb/ft1000_download.c:581:6: warning: 'status' may be used uninitialized in this function [-Wuninitialized]
Signed-off-by Chen Gang <[email protected]>

Reviewed-by Josh Triplett <[email protected]>

Quoted 18 lines

 .../staging/ft1000/ft1000-usb/ft1000_download.c    |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/staging/ft1000/ft1000-usb/ft1000_download.c +1 -1

index 68ded17..15f3062 100644
@@ -578,7 +578,7 @@ static int request_code_segment(struct ft1000_usb *ft1000dev, u16 **s_file,
 		 u8 **c_file, const u8 *endpoint, bool boot_case)
 {
 	long word_length;
-	int status;
+	int status = STATUS_SUCCESS;
 
 	/*DEBUG("FT1000:REQUEST_CODE_SEGMENT\n");i*/
 	word_length = get_request_value(ft1000dev);

Signature · 1 line

1.7.7.6

Chen Gang

author

2013-11-27 09:25 UTC #18 lore raw

Re: [PATCH] drivers: staging: ft1000: ft1000-usb: initialize 'status' with STATUS_SUCCESS in request_code_segment()

Quoted 4 lines · Josh Triplett

Quoted 3 lines · Chen Gang
If "!bool_case", it returns unexpected value instead of STATUS_SUCCESS, so need fix it, the related warning (with allmodconfig under hexagon):

Quoted 9 lines

Quoted 6 lines
    CC [M]  drivers/staging/ft1000/ft1000-usb/ft1000_download.o
drivers/staging/ft1000/ft1000-usb/ft1000_download.c: In function 'request_code_segment':
drivers/staging/ft1000/ft1000-usb/ft1000_download.c:581:6: warning: 'status' may be used uninitialized in this function [-Wuninitialized]
Signed-off-by Chen Gang <[email protected]>
Reviewed-by Josh Triplett <[email protected]>

Thanks. :-)

Quoted 18 lines

 .../staging/ft1000/ft1000-usb/ft1000_download.c    |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/staging/ft1000/ft1000-usb/ft1000_download.c +1 -1

index 68ded17..15f3062 100644
@@ -578,7 +578,7 @@ static int request_code_segment(struct ft1000_usb *ft1000dev, u16 **s_file,
 		 u8 **c_file, const u8 *endpoint, bool boot_case)
 {
 	long word_length;
-	int status;
+	int status = STATUS_SUCCESS;
 
 	/*DEBUG("FT1000:REQUEST_CODE_SEGMENT\n");i*/
 	word_length = get_request_value(ft1000dev);

Signature · 1 line

1.7.7.6

Signature · 1 line

Chen Gang

Chen Gang

author

2013-12-04 07:29 UTC #19 lore raw

Re: [PATCH] drivers: staging: ft1000: ft1000-usb: initialize 'status' with STATUS_SUCCESS in request_code_segment()

Oh, another member has already fixed it (found earlier than me), and integrated it into next-20131203 tree, so this patch is obsoleted.

The related git commit is "8aced95 staging: ft1000: fix use of potentially uninitialized variable"

Thanks.

Quoted 37 lines · Chen Gang

Quoted 4 lines · Josh Triplett
Quoted 3 lines · Chen Gang
If "!bool_case", it returns unexpected value instead of STATUS_SUCCESS, so need fix it, the related warning (with allmodconfig under hexagon):
Quoted 9 lines
Quoted 6 lines
    CC [M]  drivers/staging/ft1000/ft1000-usb/ft1000_download.o
drivers/staging/ft1000/ft1000-usb/ft1000_download.c: In function 'request_code_segment':
drivers/staging/ft1000/ft1000-usb/ft1000_download.c:581:6: warning: 'status' may be used uninitialized in this function [-Wuninitialized]
Signed-off-by Chen Gang <[email protected]>
Reviewed-by Josh Triplett <[email protected]>
Thanks. :-)
Quoted 18 lines
Quoted 18 lines
 .../staging/ft1000/ft1000-usb/ft1000_download.c    |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/staging/ft1000/ft1000-usb/ft1000_download.c +1 -1
index 68ded17..15f3062 100644
@@ -578,7 +578,7 @@ static int request_code_segment(struct ft1000_usb *ft1000dev, u16 **s_file,
 		 u8 **c_file, const u8 *endpoint, bool boot_case)
 {
 	long word_length;
-	int status;
+	int status = STATUS_SUCCESS;
 
 	/*DEBUG("FT1000:REQUEST_CODE_SEGMENT\n");i*/
 	word_length = get_request_value(ft1000dev);
Signature · 1 line
1.7.7.6

Signature · 1 line

Chen Gang

Chen Gang

author

2013-11-27 03:15 UTC #20 lore raw

PATCH 1/1 drivers: staging: media: go7007: go7007-usb.c use pr_*() instead of dev_*() before 'go' initialized in go7007_usb_probe()

In reply to Chen Gang

dev_*() assumes 'go' is already initialized, so need use pr_*() instead of before 'go' initialized. Related warning (with allmodconfig under hexagon):

    CC [M]  drivers/staging/media/go7007/go7007-usb.o

drivers/staging/media/go7007/go7007-usb.c: In function 'go7007_usb_probe':
drivers/staging/media/go7007/go7007-usb.c:1060:2: warning: 'go' may be used uninitialized in this function [-Wuninitialized]

Also remove useless code after 'return' statement.

Signed-off-by Chen Gang <[email protected]>

---
 drivers/staging/media/go7007/go7007-usb.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)

drivers/staging/media/go7007/go7007-usb.c +4 -7

index 58684da..30310e9 100644
@@ -1057,7 +1057,7 @@ static int go7007_usb_probe(struct usb_interface *intf,
 	char *name;
 	int video_pipe, i, v_urb_len;
 
-	dev_dbg(go->dev, "probing new GO7007 USB board\n");
+	pr_devel("probing new GO7007 USB board\n");
 
 	switch (id->driver_info) {
 	case GO7007_BOARDID_MATRIX_II:

@@ -1097,13 +1097,10 @@ static int go7007_usb_probe(struct usb_interface *intf,
 		board = &board_px_tv402u;
 		break;
 	case GO7007_BOARDID_LIFEVIEW_LR192:
-		dev_err(go->dev, "The Lifeview TV Walker Ultra is not supported. Sorry!\n");
+		pr_err("The Lifeview TV Walker Ultra is not supported. Sorry!\n");
 		return -ENODEV;
-		name = "Lifeview TV Walker Ultra";
-		board = &board_lifeview_lr192;
-		break;
 	case GO7007_BOARDID_SENSORAY_2250:
-		dev_info(go->dev, "Sensoray 2250 found\n");
+		pr_info("Sensoray 2250 found\n");
 		name = "Sensoray 2250/2251";
 		board = &board_sensoray_2250;
 		break;

@@ -1112,7 +1109,7 @@ static int go7007_usb_probe(struct usb_interface *intf,
 		board = &board_ads_usbav_709;
 		break;
 	default:
-		dev_err(go->dev, "unknown board ID %d!\n",
+		pr_err("unknown board ID %d!\n",
 				(unsigned int)id->driver_info);
 		return -ENODEV;
 	}

Signature · 1 line

1.7.7.6

Joe Perches

2013-11-27 03:21 UTC #21 lore raw

Re: [PATCH] drivers: staging: media: go7007: go7007-usb.c use pr_*() instead of dev_*() before 'go' initialized in go7007_usb_probe()

Quoted 2 lines · Chen Gang

dev_*() assumes 'go' is already initialized, so need use pr_*() instead of before 'go' initialized.

[]

drivers/staging/media/go7007/go7007-usb.c

Quoted 1 line

drivers/staging/media/go7007/go7007-usb.c +0 -0

[]

drivers/staging/media/go7007/go7007-usb.c static int go7007_usb_probe(struct usb_interface *intf, · line 1060

Quoted 6 lines

drivers/staging/media/go7007/go7007-usb.c +1 -1

@@ -1057,7 +1057,7 @@ static int go7007_usb_probe(struct usb_interface *intf,
 	char *name;
 	int video_pipe, i, v_urb_len;
 
-	dev_dbg(go->dev, "probing new GO7007 USB board\n");
+	pr_devel("probing new GO7007 USB board\n");

pr_devel is commonly compiled out completely unless DEBUG is #defined. You probably want to use pr_debug here.

Chen Gang

author

2013-11-27 03:38 UTC #22 lore raw

Re: [PATCH] drivers: staging: media: go7007: go7007-usb.c use pr_*() instead of dev_*() before 'go' initialized in go7007_usb_probe()

Quoted 10 lines · Joe Perches

Quoted 2 lines · Chen Gang
dev_*() assumes 'go' is already initialized, so need use pr_*() instead of before 'go' initialized.
[]
drivers/staging/media/go7007/go7007-usb.c
Quoted 1 line
drivers/staging/media/go7007/go7007-usb.c +0 -0
[]
Quoted 4 lines
drivers/staging/media/go7007/go7007-usb.c +0 -0
@@ -1057,7 +1057,7 @@ static int go7007_usb_probe(struct usb_interface *intf,
 	char *name;
 	int video_pipe, i, v_urb_len;
 

Quoted 7 lines

drivers/staging/media/go7007/go7007-usb.c static int go7007_usb_probe(struct usb_interface *intf, · line 1060
Quoted 2 lines
drivers/staging/media/go7007/go7007-usb.c +1 -1
-	dev_dbg(go->dev, "probing new GO7007 USB board\n");
+	pr_devel("probing new GO7007 USB board\n");
pr_devel is commonly compiled out completely unless DEBUG is #defined. You probably want to use pr_debug here.

Oh, yes, it is my fault, I will send patch v2. :-)

Thanks.

Signature · 1 line

Chen Gang

Chen Gang

author

2013-11-27 03:45 UTC #23 lore raw

PATCH v2 1/1 drivers: staging: media: go7007: go7007-usb.c use pr_*() instead of dev_*() before 'go' initialized in go7007_usb_probe()

dev_*() assumes 'go' is already initialized, so need use pr_*() instead of before 'go' initialized. Related warning (with allmodconfig under hexagon):

    CC [M]  drivers/staging/media/go7007/go7007-usb.o

Also remove useless code after 'return' statement.

Signed-off-by Chen Gang <[email protected]>

---
 drivers/staging/media/go7007/go7007-usb.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)

drivers/staging/media/go7007/go7007-usb.c +4 -7

index 58684da..2423643 100644
@@ -1057,7 +1057,7 @@ static int go7007_usb_probe(struct usb_interface *intf,
 	char *name;
 	int video_pipe, i, v_urb_len;
 
-	dev_dbg(go->dev, "probing new GO7007 USB board\n");
+	pr_debug("probing new GO7007 USB board\n");
 
 	switch (id->driver_info) {
 	case GO7007_BOARDID_MATRIX_II:

@@ -1097,13 +1097,10 @@ static int go7007_usb_probe(struct usb_interface *intf,
 		board = &board_px_tv402u;
 		break;
 	case GO7007_BOARDID_LIFEVIEW_LR192:
-		dev_err(go->dev, "The Lifeview TV Walker Ultra is not supported. Sorry!\n");
+		pr_err("The Lifeview TV Walker Ultra is not supported. Sorry!\n");
 		return -ENODEV;
-		name = "Lifeview TV Walker Ultra";
-		board = &board_lifeview_lr192;
-		break;
 	case GO7007_BOARDID_SENSORAY_2250:
-		dev_info(go->dev, "Sensoray 2250 found\n");
+		pr_info("Sensoray 2250 found\n");
 		name = "Sensoray 2250/2251";
 		board = &board_sensoray_2250;
 		break;

@@ -1112,7 +1109,7 @@ static int go7007_usb_probe(struct usb_interface *intf,
 		board = &board_ads_usbav_709;
 		break;
 	default:
-		dev_err(go->dev, "unknown board ID %d!\n",
+		pr_err("unknown board ID %d!\n",
 				(unsigned int)id->driver_info);
 		return -ENODEV;
 	}

Signature · 1 line

1.7.7.6

Greg KH

maintainer

2013-11-27 04:02 UTC #24 lore raw

Re: [PATCH v2] drivers: staging: media: go7007: go7007-usb.c use pr_*() instead of dev_*() before 'go' initialized in go7007_usb_probe()

Quoted 9 lines · Chen Gang

dev_*() assumes 'go' is already initialized, so need use pr_*() instead of before 'go' initialized. Related warning (with allmodconfig under hexagon):
    CC [M]  drivers/staging/media/go7007/go7007-usb.o
drivers/staging/media/go7007/go7007-usb.c: In function 'go7007_usb_probe':
drivers/staging/media/go7007/go7007-usb.c:1060:2: warning: 'go' may be used uninitialized in this function [-Wuninitialized]
Also remove useless code after 'return' statement.

This should all be fixed in my staging-linus branch already, right? No need for this anymore from what I can tell, sorry.

greg k-h

Chen Gang

author

2013-11-27 04:22 UTC #25 lore raw

Re: [PATCH v2] drivers: staging: media: go7007: go7007-usb.c use pr_*() instead of dev_*() before 'go' initialized in go7007_usb_probe()

Quoted 5 lines · Greg KH

Quoted 4 lines · Chen Gang
dev_*() assumes 'go' is already initialized, so need use pr_*() instead of before 'go' initialized. Related warning (with allmodconfig under hexagon):

Quoted 9 lines

Quoted 5 lines
    CC [M]  drivers/staging/media/go7007/go7007-usb.o
drivers/staging/media/go7007/go7007-usb.c: In function 'go7007_usb_probe':
drivers/staging/media/go7007/go7007-usb.c:1060:2: warning: 'go' may be used uninitialized in this function [-Wuninitialized]
Also remove useless code after 'return' statement.
This should all be fixed in my staging-linus branch already, right? No need for this anymore from what I can tell, sorry.

That's all right (in fact don't need sorry). :-)

And excuse me, I am not quite familiar upstream kernel version merging and branches. Is it still better/suitable/possible to sync some bug fix patches from staging brach to next brach?

Thanks.

Signature · 1 line

Chen Gang

Dan Carpenter

2013-11-27 10:43 UTC #26 lore raw

Re: [PATCH v2] drivers: staging: media: go7007: go7007-usb.c use pr_*() instead of dev_*() before 'go' initialized in go7007_usb_probe()

Quoted 16 lines · Chen Gang

Quoted 5 lines · Greg KH
Quoted 4 lines · Chen Gang
dev_*() assumes 'go' is already initialized, so need use pr_*() instead of before 'go' initialized. Related warning (with allmodconfig under hexagon):
Quoted 9 lines
Quoted 5 lines
    CC [M]  drivers/staging/media/go7007/go7007-usb.o
drivers/staging/media/go7007/go7007-usb.c: In function 'go7007_usb_probe':
drivers/staging/media/go7007/go7007-usb.c:1060:2: warning: 'go' may be used uninitialized in this function [-Wuninitialized]
Also remove useless code after 'return' statement.
This should all be fixed in my staging-linus branch already, right? No need for this anymore from what I can tell, sorry.

Quoted 5 lines

That's all right (in fact don't need sorry). :-)
And excuse me, I am not quite familiar upstream kernel version merging and branches. Is it still better/suitable/possible to sync some bug fix patches from staging brach to next brach?

next syncs with everyone once a day.

regards,
dan carpenter

Chen Gang

author

2013-11-28 01:46 UTC #27 lore raw

Re: [PATCH v2] drivers: staging: media: go7007: go7007-usb.c use pr_*() instead of dev_*() before 'go' initialized in go7007_usb_probe()

Quoted 17 lines · Dan Carpenter

Quoted 16 lines · Chen Gang
Quoted 14 lines · Greg KH
Quoted 9 lines · Chen Gang
dev_*() assumes 'go' is already initialized, so need use pr_*() instead of before 'go' initialized. Related warning (with allmodconfig under hexagon):
    CC [M]  drivers/staging/media/go7007/go7007-usb.o
drivers/staging/media/go7007/go7007-usb.c: In function 'go7007_usb_probe':
drivers/staging/media/go7007/go7007-usb.c:1060:2: warning: 'go' may be used uninitialized in this function [-Wuninitialized]
Also remove useless code after 'return' statement.
This should all be fixed in my staging-linus branch already, right? No need for this anymore from what I can tell, sorry.

Quoted 8 lines

Quoted 5 lines
That's all right (in fact don't need sorry). :-)
And excuse me, I am not quite familiar upstream kernel version merging and branches. Is it still better/suitable/possible to sync some bug fix patches from staging brach to next brach?
next syncs with everyone once a day.

OK, thanks. :-)

Signature · 1 line

Chen Gang

Chen Gang

author

2013-11-27 03:38 UTC #28 lore raw

PATCH 1/1 drivers: staging: ft1000: ft1000-usb: ft1000_debug.c: check return value of get_user() in ft1000_ioctl()

In reply to Chen Gang

get_user() may fail and cause 'msgsz' uninitialized, so need give a check. The related warning (with allmodconfig under hexagon):

    CC [M]  drivers/staging/ft1000/ft1000-usb/ft1000_debug.o

drivers/staging/ft1000/ft1000-usb/ft1000_debug.c: In function 'ft1000_ioctl':
include/uapi/linux/swab.h:53:9: warning: 'msgsz' may be used uninitialized in this function [-Wuninitialized] drivers/staging/ft1000/ft1000-usb/ft1000_debug.c:533:17: note: 'msgsz' was declared here

Signed-off-by Chen Gang <[email protected]>

---
 drivers/staging/ft1000/ft1000-usb/ft1000_debug.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

drivers/staging/ft1000/ft1000-usb/ft1000_debug.c +2 -0

index 68a55ce..ffdc7f5 100644
@@ -560,6 +560,8 @@ static long ft1000_ioctl(struct file *file, unsigned int command,
 
                 /* Get the length field to see how many bytes to copy */
                 result = get_user(msgsz, (__u16 __user *)argp);
+		if (result)
+			break;
                 msgsz = ntohs(msgsz);
                 /* DEBUG("FT1000:ft1000_ioctl: length of message = %d\n", msgsz); */

Signature · 1 line

1.7.7.6

Chen Gang

author

2013-11-27 04:51 UTC #29 lore raw

PATCH 1/1 net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

In reply to Chen Gang

In next-20131122 tree, if "sdata->vif.type != NL80211_IFTYPE_AP", 'chanctx_conf' will be not initialized, so need check it. Related warning (with allmodconfig under hexagon):

    CC [M]  net/mac80211/tx.o

net/mac80211/tx.c: In function 'ieee80211_subif_start_xmit':
net/mac80211/tx.c:1827:27: warning: 'chanctx_conf' may be used uninitialized in this function [-Wuninitialized]

Signed-off-by Chen Gang <[email protected]>

---
 net/mac80211/tx.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

net/mac80211/tx.c +3 -2

index c558b24..f3245d6 100644
@@ -1814,8 +1814,9 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
 			break;
 		/* fall through */
 	case NL80211_IFTYPE_AP:
-		if (sdata->vif.type == NL80211_IFTYPE_AP)
-			chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
+		if (sdata->vif.type != NL80211_IFTYPE_AP)
+			goto fail_rcu;
+		chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
 		if (!chanctx_conf)
 			goto fail_rcu;
 		fc |= cpu_to_le16(IEEE80211_FCTL_FROMDS);

Signature · 1 line

1.7.7.6

Johannes Berg

maintainer

2013-11-29 15:38 UTC #30 lore raw

Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

net/mac80211/tx.c netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb, · line 1819

Quoted 11 lines

+++ b/net/mac80211/tx.c

net/mac80211/tx.c +3 -2

@@ -1814,8 +1814,9 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
 			break;
 		/* fall through */
 	case NL80211_IFTYPE_AP:
-		if (sdata->vif.type == NL80211_IFTYPE_AP)
-			chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
+		if (sdata->vif.type != NL80211_IFTYPE_AP)
+			goto fail_rcu;
+		chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);

This change is completely wrong.

johannes

Chen Gang

author

2013-11-30 11:58 UTC #31 lore raw

Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

Quoted 14 lines · Johannes Berg

Review netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb, · line 1819

Quoted 11 lines

+++ b/net/mac80211/tx.c

(quoted diff) +3 -2

@@ -1814,8 +1814,9 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
 			break;
 		/* fall through */
 	case NL80211_IFTYPE_AP:
-		if (sdata->vif.type == NL80211_IFTYPE_AP)
-			chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
+		if (sdata->vif.type != NL80211_IFTYPE_AP)
+			goto fail_rcu;
+		chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);

This change is completely wrong.

Oh, it is.

Hmm... for me, this work flow still can be implemented with a little clearer way (at least it will avoid related warning):

-------------------------diff begin------------------------------

net/mac80211/tx.c +5 -5

index c558b24..7076128 100644
@@ -1810,14 +1810,14 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
 		if (!chanctx_conf)
 			goto fail_rcu;
 		band = chanctx_conf->def.chan->band;
-		if (sta)
-			break;
-		/* fall through */
+		if (!sta)
+			goto try_next;
+		break;
 	case NL80211_IFTYPE_AP:
-		if (sdata->vif.type == NL80211_IFTYPE_AP)
-			chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
+		chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
 		if (!chanctx_conf)
 			goto fail_rcu;
+try_next:
 		fc |= cpu_to_le16(IEEE80211_FCTL_FROMDS);
 		/* DA BSSID SA */
 		memcpy(hdr.addr1, skb->data, ETH_ALEN);

-------------------------diff end--------------------------------

Thanks.

Signature · 1 line

Chen Gang

Johannes Berg

maintainer

2013-11-30 12:53 UTC #32 lore raw

Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

Quoted 34 lines · Chen Gang

Quoted 14 lines · Johannes Berg
net/mac80211/tx.c netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb, · line 1819
Quoted 11 lines
+++ b/net/mac80211/tx.c
net/mac80211/tx.c +3 -2
@@ -1814,8 +1814,9 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
 			break;
 		/* fall through */
 	case NL80211_IFTYPE_AP:
-		if (sdata->vif.type == NL80211_IFTYPE_AP)
-			chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
+		if (sdata->vif.type != NL80211_IFTYPE_AP)
+			goto fail_rcu;
+		chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
This change is completely wrong.
Oh, it is.
Hmm... for me, this work flow still can be implemented with a little clearer way (at least it will avoid related warning):
-------------------------diff begin------------------------------
net/mac80211/tx.c +0 -3
index c558b24..7076128 100644
@@ -1810,14 +1810,14 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
 		if (!chanctx_conf)
 			goto fail_rcu;
 		band = chanctx_conf->def.chan->band;
-		if (sta)
-			break;
-		/* fall through */

net/mac80211/tx.c netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb, · line 1820

Quoted 10 lines

net/mac80211/tx.c +5 -2

+		if (!sta)
+			goto try_next;
+		break;
 	case NL80211_IFTYPE_AP:
-		if (sdata->vif.type == NL80211_IFTYPE_AP)
-			chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
+		chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
 		if (!chanctx_conf)
 			goto fail_rcu;
+try_next:

I don't think that's better than the (fairly obvious) fall-through, and has a pretty odd goto. Also, depending on the compiler, it still knows the previous case label and doesn't warn.

johannes

Chen Gang

author

2013-11-30 13:50 UTC #33 lore raw

Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

Quoted 41 lines · Johannes Berg

Quoted 40 lines · Chen Gang
Quoted 14 lines · Johannes Berg
net/mac80211/tx.c netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb, · line 1819
Quoted 11 lines
+++ b/net/mac80211/tx.c
net/mac80211/tx.c +3 -2
@@ -1814,8 +1814,9 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
 			break;
 		/* fall through */
 	case NL80211_IFTYPE_AP:
-		if (sdata->vif.type == NL80211_IFTYPE_AP)
-			chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
+		if (sdata->vif.type != NL80211_IFTYPE_AP)
+			goto fail_rcu;
+		chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
This change is completely wrong.
Oh, it is.
Hmm... for me, this work flow still can be implemented with a little clearer way (at least it will avoid related warning):
-------------------------diff begin------------------------------
net/mac80211/tx.c +3 -5
index c558b24..7076128 100644
@@ -1810,14 +1810,14 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
 		if (!chanctx_conf)
 			goto fail_rcu;
 		band = chanctx_conf->def.chan->band;
-		if (sta)
-			break;
-		/* fall through */
+		if (!sta)
+			goto try_next;
+		break;
 	case NL80211_IFTYPE_AP:
-		if (sdata->vif.type == NL80211_IFTYPE_AP)
-			chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);

Quoted 9 lines

net/mac80211/tx.c netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb, · line 1820
Quoted 4 lines
net/mac80211/tx.c +2 -0
+		chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
 		if (!chanctx_conf)
 			goto fail_rcu;
+try_next:
I don't think that's better than the (fairly obvious) fall-through, and has a pretty odd goto. Also, depending on the compiler, it still knows the previous case label and doesn't warn.

Yeah, fall-through is obvious. But check 'A' again just near by "case A" seems a little strange, and some of compilers (or some of versions) are really not quit smart enough to know it is not a warning.

Hmm... for me, if the code (implementation) can express real logical work flow as much as directly and simply, the code (implementation) is clear enough (don't mind whether use 'goto' or not).

And originally, at first, I am really not quite careful enough, and sent an incorrect patch after noticed the related compiler's warning. :-)

Thanks.

Signature · 1 line

Chen Gang

Chen Gang

author

2013-11-30 14:02 UTC #34 lore raw

Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

Quoted 52 lines · Chen Gang

Quoted 41 lines · Johannes Berg
Quoted 40 lines · Chen Gang
Quoted 14 lines · Johannes Berg
net/mac80211/tx.c netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb, · line 1819
Quoted 11 lines
+++ b/net/mac80211/tx.c
net/mac80211/tx.c +3 -2
@@ -1814,8 +1814,9 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
 			break;
 		/* fall through */
 	case NL80211_IFTYPE_AP:
-		if (sdata->vif.type == NL80211_IFTYPE_AP)
-			chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
+		if (sdata->vif.type != NL80211_IFTYPE_AP)
+			goto fail_rcu;
+		chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
This change is completely wrong.
Oh, it is.
Hmm... for me, this work flow still can be implemented with a little clearer way (at least it will avoid related warning):
-------------------------diff begin------------------------------
net/mac80211/tx.c +3 -5
index c558b24..7076128 100644
@@ -1810,14 +1810,14 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
 		if (!chanctx_conf)
 			goto fail_rcu;
 		band = chanctx_conf->def.chan->band;
-		if (sta)
-			break;
-		/* fall through */
+		if (!sta)
+			goto try_next;
+		break;
 	case NL80211_IFTYPE_AP:
-		if (sdata->vif.type == NL80211_IFTYPE_AP)
-			chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
Quoted 9 lines
net/mac80211/tx.c netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb, · line 1820
Quoted 4 lines
net/mac80211/tx.c +2 -0
+		chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
 		if (!chanctx_conf)
 			goto fail_rcu;
+try_next:
I don't think that's better than the (fairly obvious) fall-through, and has a pretty odd goto. Also, depending on the compiler, it still knows the previous case label and doesn't warn.

Quoted 4 lines

Yeah, fall-through is obvious. But check 'A' again just near by "case A" seems a little strange, and some of compilers (or some of versions) are really not quit smart enough to know it is not a warning.

Sorry, the paragraph above may lead misunderstanding, I repeated again:

- fall-through is obvious (although I did not notice it, originally).

- Check 'A' again just near by "case A" seems a little strange.

- Some compilers aren't quit smart enough to know 'chanctx_conf' is OK.

Thanks.

Quoted 11 lines

Hmm... for me, if the code (implementation) can express real logical work flow as much as directly and simply, the code (implementation) is clear enough (don't mind whether use 'goto' or not).
And originally, at first, I am really not quite careful enough, and sent an incorrect patch after noticed the related compiler's warning. :-)
Thanks.

Signature · 1 line

Chen Gang

Johannes Berg

maintainer

2013-11-30 20:08 UTC #35 lore raw

Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

Quoted 18 lines · Chen Gang

Quoted 13 lines
Quoted 12 lines
net/mac80211/tx.c netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb, · line 1820
Quoted 7 lines
net/mac80211/tx.c +2 -2
 	case NL80211_IFTYPE_AP:
-		if (sdata->vif.type == NL80211_IFTYPE_AP)
-			chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
+		chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
 		if (!chanctx_conf)
 			goto fail_rcu;
+try_next:
I don't think that's better than the (fairly obvious) fall-through, and has a pretty odd goto. Also, depending on the compiler, it still knows the previous case label and doesn't warn.
Quoted 4 lines
Yeah, fall-through is obvious. But check 'A' again just near by "case A" seems a little strange, and some of compilers (or some of versions) are really not quit smart enough to know it is not a warning.

Quoted 7 lines

Sorry, the paragraph above may lead misunderstanding, I repeated again:
- fall-through is obvious (although I did not notice it, originally).
- Check 'A' again just near by "case A" seems a little strange.
- Some compilers aren't quit smart enough to know 'chanctx_conf' is OK.

I know. If you have any good ideas of how to make it more obvious to the compiler, I'm all ears, I just don't like any of the solutions offered so far (and you aren't the first to do so either) :-)

FWIW, I find the label to be odd because if you're familiar with the code then AP/AP_VLAN *should* be identical except for two special things that are now linearly & neatly handled in the code (the first being the 4-addr station, the second the chanctx assignment which always has to be done regardless of 4-addr). IMHO the == check after case should be enough to make a human reader take a closer look. I understand that you didn't and that's OK since you were just trying to squelch compile warnings, but I don't see that this one warrants much attention.

johannes

Joe Perches

2013-11-30 20:39 UTC #36 lore raw

Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

Quoted 32 lines · Johannes Berg

Quoted 18 lines · Chen Gang
Quoted 13 lines
Quoted 12 lines
Review
Quoted 7 lines
(quoted diff) +2 -2
 	case NL80211_IFTYPE_AP:
-		if (sdata->vif.type == NL80211_IFTYPE_AP)
-			chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
+		chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
 		if (!chanctx_conf)
 			goto fail_rcu;
+try_next:
I don't think that's better than the (fairly obvious) fall-through, and has a pretty odd goto. Also, depending on the compiler, it still knows the previous case label and doesn't warn.
Quoted 4 lines
Yeah, fall-through is obvious. But check 'A' again just near by "case A" seems a little strange, and some of compilers (or some of versions) are really not quit smart enough to know it is not a warning.
Quoted 7 lines
Sorry, the paragraph above may lead misunderstanding, I repeated again:
- fall-through is obvious (although I did not notice it, originally).
- Check 'A' again just near by "case A" seems a little strange.
- Some compilers aren't quit smart enough to know 'chanctx_conf' is OK.
I know. If you have any good ideas of how to make it more obvious to the compiler, I'm all ears, I just don't like any of the solutions offered so far (and you aren't the first to do so either) :-)

Quoted 8 lines

FWIW, I find the label to be odd because if you're familiar with the code then AP/AP_VLAN *should* be identical except for two special things that are now linearly & neatly handled in the code (the first being the 4-addr station, the second the chanctx assignment which always has to be done regardless of 4-addr). IMHO the == check after case should be enough to make a human reader take a closer look. I understand that you didn't and that's OK since you were just trying to squelch compile warnings, but I don't see that this one warrants much attention.

The label/test could be moved to save a couple of lines of duplicated code.

Maybe:

 net/mac80211/tx.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

net/mac80211/tx.c +6 -8

index 70b5a05..b2160f4 100644
@@ -1777,18 +1777,16 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
 		}
 		ap_sdata = container_of(sdata->bss, struct ieee80211_sub_if_data,
 					u.ap);
-		chanctx_conf = rcu_dereference(ap_sdata->vif.chanctx_conf);
-		if (!chanctx_conf)
-			goto fail_rcu;
-		band = chanctx_conf->def.chan->band;
-		if (sta)
-			break;
 		/* fall through */
 	case NL80211_IFTYPE_AP:
-		if (sdata->vif.type == NL80211_IFTYPE_AP)
-			chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
+		chanctx_conf = rcu_dereference(ap_sdata->vif.chanctx_conf);
 		if (!chanctx_conf)
 			goto fail_rcu;
+		if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN) {
+			band = chanctx_conf->def.chan->band;
+			if (sta)
+				break;
+		}
 		fc |= cpu_to_le16(IEEE80211_FCTL_FROMDS);
 		/* DA BSSID SA */
 		memcpy(hdr.addr1, skb->data, ETH_ALEN);

Chen Gang

author

2013-11-30 23:48 UTC #37 lore raw

Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

Quoted 4 lines · Joe Perches

Quoted 3 lines · Johannes Berg
Quoted 2 lines · Chen Gang
- fall-through is obvious (although I did not notice it, originally).

Quoted 8 lines

Quoted 8 lines
Quoted 3 lines
- Check 'A' again just near by "case A" seems a little strange.
- Some compilers aren't quit smart enough to know 'chanctx_conf' is OK.
I know. If you have any good ideas of how to make it more obvious to the compiler, I'm all ears, I just don't like any of the solutions offered so far (and you aren't the first to do so either) :-)

OK, thanks.

Quoted 9 lines

Quoted 8 lines
FWIW, I find the label to be odd because if you're familiar with the code then AP/AP_VLAN *should* be identical except for two special things that are now linearly & neatly handled in the code (the first being the 4-addr station, the second the chanctx assignment which always has to be done regardless of 4-addr). IMHO the == check after case should be enough to make a human reader take a closer look. I understand that you didn't and that's OK since you were just trying to squelch compile warnings, but I don't see that this one warrants much attention.

Quoted 3 lines

The label/test could be moved to save a couple of lines of duplicated code.

Hmm... for me, it must be an implementation which can satisfy all of us.

If ieee80211_subif_start_xmit() is not performance sensitive (I guess so), we can use some short static functions instead of some code blocks within ieee80211_subif_start_xmit().

- ieee80211_subif_start_xmit() is a long function (600+ lines).

- use short static function can share some code.

- if code can be shared, the work flow can be more clearer too (don't need fall-through or goto).

And I guess, the implementation below can cause panic when "sdata->vif.type == NL80211_IFTYPE_AP_VLAN". :-(

Quoted 36 lines

Maybe:

 net/mac80211/tx.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

net/mac80211/tx.c +6 -8

index 70b5a05..b2160f4 100644
@@ -1777,18 +1777,16 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
 		}
 		ap_sdata = container_of(sdata->bss, struct ieee80211_sub_if_data,
 					u.ap);
-		chanctx_conf = rcu_dereference(ap_sdata->vif.chanctx_conf);
-		if (!chanctx_conf)
-			goto fail_rcu;
-		band = chanctx_conf->def.chan->band;
-		if (sta)
-			break;
 		/* fall through */
 	case NL80211_IFTYPE_AP:
-		if (sdata->vif.type == NL80211_IFTYPE_AP)
-			chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
+		chanctx_conf = rcu_dereference(ap_sdata->vif.chanctx_conf);
 		if (!chanctx_conf)
 			goto fail_rcu;
+		if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN) {
+			band = chanctx_conf->def.chan->band;
+			if (sta)
+				break;
+		}
 		fc |= cpu_to_le16(IEEE80211_FCTL_FROMDS);
 		/* DA BSSID SA */
 		memcpy(hdr.addr1, skb->data, ETH_ALEN);

Signature · 1 line

Chen Gang

Chen Gang

author

2013-11-30 23:59 UTC #38 lore raw

Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

Quoted 29 lines · Chen Gang

Quoted 4 lines · Joe Perches
Quoted 3 lines · Johannes Berg
Quoted 2 lines · Chen Gang
- fall-through is obvious (although I did not notice it, originally).
Quoted 8 lines
Quoted 8 lines
Quoted 3 lines
- Check 'A' again just near by "case A" seems a little strange.
- Some compilers aren't quit smart enough to know 'chanctx_conf' is OK.
I know. If you have any good ideas of how to make it more obvious to the compiler, I'm all ears, I just don't like any of the solutions offered so far (and you aren't the first to do so either) :-)
OK, thanks.
Quoted 9 lines
Quoted 8 lines
FWIW, I find the label to be odd because if you're familiar with the code then AP/AP_VLAN *should* be identical except for two special things that are now linearly & neatly handled in the code (the first being the 4-addr station, the second the chanctx assignment which always has to be done regardless of 4-addr). IMHO the == check after case should be enough to make a human reader take a closer look. I understand that you didn't and that's OK since you were just trying to squelch compile warnings, but I don't see that this one warrants much attention.
Quoted 3 lines
The label/test could be moved to save a couple of lines of duplicated code.

Quoted 8 lines

Hmm... for me, it must be an implementation which can satisfy all of us.
If ieee80211_subif_start_xmit() is not performance sensitive (I guess so), we can use some short static functions instead of some code blocks within ieee80211_subif_start_xmit().
- ieee80211_subif_start_xmit() is a long function (600+ lines).

Oh, one typo issue (need use 400+ instead of 600+), it is a long function (although not quite long).

Quoted 47 lines

- use short static function can share some code.

- if code can be shared, the work flow can be more clearer too (don't need fall-through or goto).

And I guess, the implementation below can cause panic when "sdata->vif.type == NL80211_IFTYPE_AP_VLAN". :-(

Quoted 36 lines

Maybe:

 net/mac80211/tx.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

net/mac80211/tx.c +6 -8

index 70b5a05..b2160f4 100644
@@ -1777,18 +1777,16 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
 		}
 		ap_sdata = container_of(sdata->bss, struct ieee80211_sub_if_data,
 					u.ap);
-		chanctx_conf = rcu_dereference(ap_sdata->vif.chanctx_conf);
-		if (!chanctx_conf)
-			goto fail_rcu;
-		band = chanctx_conf->def.chan->band;
-		if (sta)
-			break;
 		/* fall through */
 	case NL80211_IFTYPE_AP:
-		if (sdata->vif.type == NL80211_IFTYPE_AP)
-			chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
+		chanctx_conf = rcu_dereference(ap_sdata->vif.chanctx_conf);
 		if (!chanctx_conf)
 			goto fail_rcu;
+		if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN) {
+			band = chanctx_conf->def.chan->band;
+			if (sta)
+				break;
+		}
 		fc |= cpu_to_le16(IEEE80211_FCTL_FROMDS);
 		/* DA BSSID SA */
 		memcpy(hdr.addr1, skb->data, ETH_ALEN);

Signature · 1 line

Chen Gang

Johannes Berg

maintainer

2013-12-01 09:37 UTC #39 lore raw

Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

In reply to Chen Gang

Quoted 10 lines · Chen Gang

If ieee80211_subif_start_xmit() is not performance sensitive (I guess so), we can use some short static functions instead of some code blocks within ieee80211_subif_start_xmit().
- ieee80211_subif_start_xmit() is a long function (600+ lines).
- use short static function can share some code.
- if code can be shared, the work flow can be more clearer too (don't need fall-through or goto).

Frankly, I'm getting tired of discussing this. Please don't try to rewrite this code until you've understood it. You suggesting that "start_xmit()" isn't a performance sensitive function makes me realize you haven't even tried.

johannes

Chen Gang

author

2013-12-01 11:50 UTC #40 lore raw

Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

Quoted 10 lines · Johannes Berg

Quoted 8 lines · Chen Gang
If ieee80211_subif_start_xmit() is not performance sensitive (I guess so), we can use some short static functions instead of some code blocks within ieee80211_subif_start_xmit().
- ieee80211_subif_start_xmit() is a long function (600+ lines).
- use short static function can share some code.

Quoted 8 lines

Quoted 2 lines
- if code can be shared, the work flow can be more clearer too (don't need fall-through or goto).
Frankly, I'm getting tired of discussing this. Please don't try to rewrite this code until you've understood it. You suggesting that "start_xmit()" isn't a performance sensitive function makes me realize you haven't even tried.

OK, thank you for "you are getting tired ...". Please help try when you have time. :-)

No I didn't try -- so use 'if' and 'guess' for discussing.

Hmm... if it is performance sensitive:

- use static function instead of some code block and try to share them. it adds additional instructions, but can shrink binary code size. if the performance is acceptable, it is the best way to me.

- else (1st way not acceptable), use macro instead of static function. it expends binary code size, but can save some instructions. if the performance is acceptable, it is the acceptable way to me.

- If neither of static function nor macro are acceptable, I still prefer to use 'goto' instead of 'fall-through'. that can let all compilers feel well, and can not feel any strange.

normally, when prev case uses fall-through,
it need be sure of next case need not notice about it (prev case). or it will make a little strange for readers when read next case.

Welcome any suggestions or completions.

Thanks.

Signature · 5 lines

Chen Gang
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Johannes Berg

maintainer

2013-12-01 09:35 UTC #41 lore raw

Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

In reply to Joe Perches

Quoted 6 lines · Joe Perches

+++ b/net/mac80211/tx.c

net/mac80211/tx.c +0 -1

@@ -1777,18 +1777,16 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
 		}
 		ap_sdata = container_of(sdata->bss, struct ieee80211_sub_if_data,
 					u.ap);
-		chanctx_conf = rcu_dereference(ap_sdata->vif.chanctx_conf);

net/mac80211/tx.c netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb, · line 1782

Quoted 10 lines

net/mac80211/tx.c +1 -7

-		if (!chanctx_conf)
-			goto fail_rcu;
-		band = chanctx_conf->def.chan->band;
-		if (sta)
-			break;
 		/* fall through */
 	case NL80211_IFTYPE_AP:
-		if (sdata->vif.type == NL80211_IFTYPE_AP)
-			chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
+		chanctx_conf = rcu_dereference(ap_sdata->vif.chanctx_conf);

Good try, but no, now ap_sdata isn't even assigned. :)

johannes

Joe Perches

2013-12-01 22:38 UTC #42 lore raw

Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

Quoted 1 line · Johannes Berg

Good try, but no, now ap_sdata isn't even assigned. :)

Right. Oh well. There's no improving this without significant rewrite. Even then, there may not be much overall improvement.

btw: Chen, I think fall-throughs are fine as long as they are commented appropriately.

Chen Gang

author

2013-12-02 00:45 UTC #43 lore raw

Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

Quoted 10 lines · Joe Perches

Quoted 1 line · Johannes Berg
Good try, but no, now ap_sdata isn't even assigned. :)
Right. Oh well. There's no improving this without significant rewrite. Even then, there may not be much overall improvement.
btw: Chen, I think fall-throughs are fine as long as they are commented appropriately.

Hmm... for me, when use fall-through, need 2 things:

- need a comment.

- next case need not notice about prev case which fall-through.

If we can not fit the 2 things above, either can not use sub functions or macros for it because of performance reason, 'goto' is better than 'fall-through'.

For me, in most cases, when a function becomes a long function, it is always better to use sub-fuctions or macros instead of some blocks (but it is really not urgent :-) ).

Thanks.

Signature · 5 lines

Chen Gang
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Johannes Berg

maintainer

2013-12-02 14:48 UTC #44 lore raw

Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

In reply to Joe Perches

Quoted 6 lines · Joe Perches

Quoted 1 line · Johannes Berg
Good try, but no, now ap_sdata isn't even assigned. :)
Right. Oh well. There's no improving this without significant rewrite. Even then, there may not be much overall improvement.

I could see an improvement if we were to actually make changes to assign the pointer based on the iftype, when that changes, so that each function only has to handle the right type and we don't need the switch statement at all...

But that's tricky to get right and I'm between vacations and holidays and all that ...

johannes

Chen Gang

author

2013-12-04 02:10 UTC #45 lore raw

Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

Quoted 4 lines · Johannes Berg

Quoted 3 lines · Joe Perches
Quoted 1 line · Johannes Berg
Good try, but no, now ap_sdata isn't even assigned. :)

Quoted 9 lines

Quoted 3 lines
Right. Oh well. There's no improving this without significant rewrite. Even then, there may not be much overall improvement.
I could see an improvement if we were to actually make changes to assign the pointer based on the iftype, when that changes, so that each function only has to handle the right type and we don't need the switch statement at all...

OK, thanks, it is one of good ideas to me (which I can not find). :-)

Quoted 3 lines

But that's tricky to get right and I'm between vacations and holidays and all that ...

It is really not urgent, and for keeping quality, it is necessary to spend suitable time resource (e.g 1 hour or more) to make, review and test this kind of patch carefully by oneself.

So could you please help improve it when you have suitable related time resources?

Thanks.

Signature · 1 line

Chen Gang

Johannes Berg

maintainer

2013-12-04 08:04 UTC #46 lore raw

Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

Quoted 6 lines · Chen Gang

It is really not urgent, and for keeping quality, it is necessary to spend suitable time resource (e.g 1 hour or more) to make, review and test this kind of patch carefully by oneself.
So could you please help improve it when you have suitable related time resources?

No. Who are you to suggest what I need to work on?

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Chen Gang

author

2013-12-04 08:39 UTC #47 lore raw

Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

Quoted 6 lines · Johannes Berg

Quoted 4 lines · Chen Gang
It is really not urgent, and for keeping quality, it is necessary to spend suitable time resource (e.g 1 hour or more) to make, review and test this kind of patch carefully by oneself.

Quoted 7 lines

Quoted 2 lines
So could you please help improve it when you have suitable related time resources?
No. Who are you to suggest what I need to work on?
johannes

I am one of volunteers for Open Source, and I don't care about who I am.

Since this thread is started by me, I have duty to try my best to let it finish (but may fail).

According to our original discussion, it seems we agree that I am not the suitable member to finish it, so I suggest you or another members to try.

Thanks.

Signature · 1 line

Chen Gang

Johannes Berg

maintainer

2013-12-04 08:49 UTC #48 lore raw

Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

Quoted 3 lines · Chen Gang

According to our original discussion, it seems we agree that I am not the suitable member to finish it, so I suggest you or another members to try.

There's nothing to finish here. The code is fine. The compiler is wrong, but we haven't found a way to shut up the compiler without breaking the code. Please let's just let it rest.

johannes

Chen Gang

author

2013-12-04 08:58 UTC #49 lore raw

Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

Quoted 10 lines · Johannes Berg

Quoted 3 lines · Chen Gang
According to our original discussion, it seems we agree that I am not the suitable member to finish it, so I suggest you or another members to try.
There's nothing to finish here. The code is fine. The compiler is wrong, but we haven't found a way to shut up the compiler without breaking the code. Please let's just let it rest.

For me, I still stick to the code can be improved, although it is not urgent.

But just like you said, we can just stop discussing -- every members can save their own opinions.

And I am not the related maintainer, so if no additional suggestions, discussions or completions, I will/should stop here.

Thanks.

Signature · 1 line

Chen Gang

Chen Gang

author

2013-11-27 05:26 UTC #50 lore raw

PATCH 1/1 arch: hexagon: include: uapi: asm: setup.h add swith macro __KERNEL__

In reply to Chen Gang

Define dummy '__init' instead of include "linux/init.h" if !__KERNEL__, or can not pass checking. The related error (with allmodconfig under hexagon):

CHECK include/asm (34 files)
usr/include/asm/setup.h:22: included file 'linux/init.h' is not exported

Signed-off-by Chen Gang <[email protected]>

---
 arch/hexagon/include/uapi/asm/setup.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

arch/hexagon/include/uapi/asm/setup.h +5 -0

index e48285e4a..7e3952d 100644
@@ -19,7 +19,12 @@
 #ifndef _ASM_SETUP_H
 #define _ASM_SETUP_H
 
+#ifdef __KERNEL__
 #include <linux/init.h>
+#else
+#define __init
+#endif
+
 #include <asm-generic/setup.h>
 
 extern char external_cmdline_buffer;

Signature · 1 line

1.7.7.6

rkuo

Acked-by on v1 1/1

2013-12-06 18:21 UTC #51 lore raw

Re: [PATCH] arch: hexagon: include: uapi: asm: setup.h add swith macro __KERNEL__

Quoted 4 lines · Chen Gang

Define dummy '__init' instead of include "linux/init.h" if !__KERNEL__, or can not pass checking. The related error (with allmodconfig under hexagon):

Quoted 9 lines

CHECK include/asm (34 files)
usr/include/asm/setup.h:22: included file 'linux/init.h' is not exported
Signed-off-by Chen Gang <[email protected]>
---
 arch/hexagon/include/uapi/asm/setup.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

Acked-by Richard Kuo <[email protected]>

Signature · 3 lines

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Chen Gang

author

2013-11-26 04:07 UTC #52 lore raw

PATCH 1/1 arch: hexagon: include: asm: add generic macro 'mmiowb' in "io.h"

In reply to rkuo

Need dumy mmiowb(), or can not pass compiling, the related error with allmodconfig:

CC [M] drivers/mmc/host/sdhci.o
drivers/mmc/host/sdhci.c: In function 'sdhci_request':
drivers/mmc/host/sdhci.c:1409:2: error: implicit declaration of function 'mmiowb' [-Werror=implicit-function-declaration]

Signed-off-by Chen Gang <[email protected]>

---
 arch/hexagon/include/asm/io.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

arch/hexagon/include/asm/io.h +2 -0

index 1b7698e..7029899 100644
@@ -189,6 +189,8 @@ static inline void writel(u32 data, volatile void __iomem *addr)
 #define writew_relaxed __raw_writew
 #define writel_relaxed __raw_writel
 
+#define mmiowb()
+
 /*
  * Need an mtype somewhere in here, for cache type deals?
  * This is probably too long for an inline.

Signature · 1 line

1.7.7.6

Chen Gang

author

2013-11-26 04:09 UTC #53 lore raw

PATCH 1/1 arch: hexagon: include: asm: Kbuild: add generic "serial.h" in Kbuild

Add "serial.h" in Kbuild, or can not pass compiling with allmodconfig, the related error:

CC [M] drivers/staging/speakup/speakup_acntpc.o
In file included from drivers/staging/speakup/speakup_acntpc.c:33:0:
drivers/staging/speakup/serialio.h:7:24: fatal error: asm/serial.h: No such file or directory

Signed-off-by Chen Gang <[email protected]>

---
 arch/hexagon/include/asm/Kbuild |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

arch/hexagon/include/asm/Kbuild +1 -0

index 424e28e..0852230 100644
@@ -38,6 +38,7 @@ generic-y += scatterlist.h
 generic-y += sections.h
 generic-y += segment.h
 generic-y += sembuf.h
+generic-y += serial.h
 generic-y += shmbuf.h
 generic-y += shmparam.h
 generic-y += siginfo.h

Signature · 1 line

1.7.7.6

rkuo

Acked-by on v1 1/1

2013-12-06 18:20 UTC #54 lore raw

Re: [PATCH] arch: hexagon: include: asm: Kbuild: add generic "serial.h" in Kbuild

Quoted 8 lines · Chen Gang

Add "serial.h" in Kbuild, or can not pass compiling with allmodconfig, the related error:
CC [M] drivers/staging/speakup/speakup_acntpc.o
In file included from drivers/staging/speakup/speakup_acntpc.c:33:0:
drivers/staging/speakup/serialio.h:7:24: fatal error: asm/serial.h: No such file or directory

Quoted 5 lines

Signed-off-by Chen Gang <[email protected]>
---
 arch/hexagon/include/asm/Kbuild |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

Acked-by Richard Kuo <[email protected]>

Signature · 3 lines

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

rkuo

Acked-by on v1 1/1

2013-12-06 18:19 UTC #55 lore raw

Re: [PATCH] arch: hexagon: include: asm: add generic macro 'mmiowb' in "io.h"

In reply to Chen Gang

Quoted 8 lines · Chen Gang

Need dumy mmiowb(), or can not pass compiling, the related error with allmodconfig:
CC [M] drivers/mmc/host/sdhci.o
drivers/mmc/host/sdhci.c: In function 'sdhci_request':
drivers/mmc/host/sdhci.c:1409:2: error: implicit declaration of function 'mmiowb' [-Werror=implicit-function-declaration]

Quoted 5 lines

Signed-off-by Chen Gang <[email protected]>
---
 arch/hexagon/include/asm/io.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

Acked-by Richard Kuo <[email protected]>

Signature · 3 lines

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation