[PATCH 0/9] Fixups for issues when running 'make randconfig'

[PATCH 0/9] Fixups for issues when running 'make randconfig'

[Lee Jones]

This patch-set is the result of fixing some problems encountered when
building the kernel with randconfig and a when running a script I wrote
to identify unused structures.

 arch/arm/mach-at91/at91sam9263_devices.c |   20 --------------------
 arch/arm/mach-integrator/cpu.c           |   12 ------------
 arch/x86/kernel/quirks.c                 |    4 ++--
 drivers/isdn/i4l/isdn_common.c           |    1 -
 drivers/virtio/virtio_mmio.c             |    2 +-
 fs/quota/quota.c                         |    4 ++++
 kernel/sched/fair.c                      |    4 ++++
 net/bridge/br_private.h                  |    8 ++++----
 sound/soc/codecs/Kconfig                 |    2 +-
 9 files changed, 16 insertions(+), 41 deletions(-)

[PATCH 1/9] ARM: integrator: Remove unused icst_params lclk_params structure

[Lee Jones]

This was introduced way back before 2005 and has remained unused for
over 7 years. Let's remove it.

Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/mach-integrator/cpu.c |   12 ------------
 1 file changed, 12 deletions(-)

diff --git a/arch/arm/mach-integrator/cpu.c b/arch/arm/mach-integrator/cpu.c
index 590c192..4e0a689 100644
--- a/arch/arm/mach-integrator/cpu.c
+++ b/arch/arm/mach-integrator/cpu.c
@@ -30,18 +30,6 @@ static struct cpufreq_driver integrator_driver;
 #define CM_STAT __io_address(INTEGRATOR_HDR_STAT)
 #define CM_LOCK __io_address(INTEGRATOR_HDR_LOCK)
 
-static const struct icst_params lclk_params = {
-	.ref		= 24000000,
-	.vco_max	= ICST525_VCO_MAX_5V,
-	.vco_min	= ICST525_VCO_MIN,
-	.vd_min		= 8,
-	.vd_max		= 132,
-	.rd_min		= 24,
-	.rd_max		= 24,
-	.s2div		= icst525_s2div,
-	.idx2s		= icst525_idx2s,
-};
-
 static const struct icst_params cclk_params = {
 	.ref		= 24000000,
 	.vco_max	= ICST525_VCO_MAX_5V,
-- 
1.7.9.5

[PATCH 1/9] ARM: integrator: Remove unused icst_params lclk_params structure

[Lee Jones]

This was introduced way back before 2005 and has remained unused for
over 7 years. Let's remove it.

Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel at lists.infradead.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/mach-integrator/cpu.c |   12 ------------
 1 file changed, 12 deletions(-)

diff --git a/arch/arm/mach-integrator/cpu.c b/arch/arm/mach-integrator/cpu.c
index 590c192..4e0a689 100644
--- a/arch/arm/mach-integrator/cpu.c
+++ b/arch/arm/mach-integrator/cpu.c
@@ -30,18 +30,6 @@ static struct cpufreq_driver integrator_driver;
 #define CM_STAT __io_address(INTEGRATOR_HDR_STAT)
 #define CM_LOCK __io_address(INTEGRATOR_HDR_LOCK)
 
-static const struct icst_params lclk_params = {
-	.ref		= 24000000,
-	.vco_max	= ICST525_VCO_MAX_5V,
-	.vco_min	= ICST525_VCO_MIN,
-	.vd_min		= 8,
-	.vd_max		= 132,
-	.rd_min		= 24,
-	.rd_max		= 24,
-	.s2div		= icst525_s2div,
-	.idx2s		= icst525_idx2s,
-};
-
 static const struct icst_params cclk_params = {
 	.ref		= 24000000,
 	.vco_max	= ICST525_VCO_MAX_5V,
-- 
1.7.9.5

[PATCH 2/9] ARM: at91: Remove unused struct at91sam9g45_isi_device and its resources

This the at91sam9g45_isi_device structure and its associated resources
were added in 2008 and have been unused ever since. Let's remove them.

Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/mach-at91/at91sam9263_devices.c |   20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
index cb85da2..0562a9d 100644
--- a/arch/arm/mach-at91/at91sam9263_devices.c
+++ b/arch/arm/mach-at91/at91sam9263_devices.c
@@ -901,26 +901,6 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data) {}
 
 #if defined(CONFIG_VIDEO_AT91_ISI) || defined(CONFIG_VIDEO_AT91_ISI_MODULE)
 
-struct resource isi_resources[] = {
-	[0] = {
-		.start	= AT91SAM9263_BASE_ISI,
-		.end	= AT91SAM9263_BASE_ISI + SZ_16K - 1,
-		.flags	= IORESOURCE_MEM,
-	},
-	[1] = {
-		.start	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
-		.end	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
-		.flags	= IORESOURCE_IRQ,
-	},
-};
-
-static struct platform_device at91sam9263_isi_device = {
-	.name		= "at91_isi",
-	.id		= -1,
-	.resource	= isi_resources,
-	.num_resources	= ARRAY_SIZE(isi_resources),
-};
-
 void __init at91_add_device_isi(struct isi_platform_data *data,
 		bool use_pck_as_mck)
 {
-- 
1.7.9.5

[PATCH 2/9] ARM: at91: Remove unused struct at91sam9g45_isi_device and its resources

[Lee Jones]

This the at91sam9g45_isi_device structure and its associated resources
were added in 2008 and have been unused ever since. Let's remove them.

Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel at lists.infradead.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/mach-at91/at91sam9263_devices.c |   20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
index cb85da2..0562a9d 100644
--- a/arch/arm/mach-at91/at91sam9263_devices.c
+++ b/arch/arm/mach-at91/at91sam9263_devices.c
@@ -901,26 +901,6 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data) {}
 
 #if defined(CONFIG_VIDEO_AT91_ISI) || defined(CONFIG_VIDEO_AT91_ISI_MODULE)
 
-struct resource isi_resources[] = {
-	[0] = {
-		.start	= AT91SAM9263_BASE_ISI,
-		.end	= AT91SAM9263_BASE_ISI + SZ_16K - 1,
-		.flags	= IORESOURCE_MEM,
-	},
-	[1] = {
-		.start	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
-		.end	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
-		.flags	= IORESOURCE_IRQ,
-	},
-};
-
-static struct platform_device at91sam9263_isi_device = {
-	.name		= "at91_isi",
-	.id		= -1,
-	.resource	= isi_resources,
-	.num_resources	= ARRAY_SIZE(isi_resources),
-};
-
 void __init at91_add_device_isi(struct isi_platform_data *data,
 		bool use_pck_as_mck)
 {
-- 
1.7.9.5

[PATCH 3/9] x86: Suppress build warnings in quirks

[Lee Jones]

arch/x86/kernel/quirks.c: In function ‘ati_force_enable_hpet’:
arch/x86/kernel/quirks.c:364:4: warning: ‘d’ may be used uninitialised in this function [-Wuninitialized]
arch/x86/kernel/quirks.c:357:6: note: ‘d’ was declared here
arch/x86/kernel/quirks.c:407:21: warning: ‘val’ may be used uninitialised in this function [-Wuninitialized]

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: x86@kernel.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/x86/kernel/quirks.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/quirks.c b/arch/x86/kernel/quirks.c
index 1b27de5..d280b70 100644
--- a/arch/x86/kernel/quirks.c
+++ b/arch/x86/kernel/quirks.c
@@ -354,7 +354,7 @@ static void ati_force_hpet_resume(void)
 
 static u32 ati_ixp4x0_rev(struct pci_dev *dev)
 {
-	u32 d;
+	u32 d = 0;
 	u8  b;
 
 	pci_read_config_byte(dev, 0xac, &b);
@@ -371,7 +371,7 @@ static u32 ati_ixp4x0_rev(struct pci_dev *dev)
 
 static void ati_force_enable_hpet(struct pci_dev *dev)
 {
-	u32 d, val;
+	u32 d, val = 0;
 	u8  b;
 
 	if (hpet_address || force_hpet_address)
-- 
1.7.9.5

[PATCH 4/9] sched: Don't define unthrottle_offline_cfs_rqs when !CONFIG_SMP

[Lee Jones]

Since unthrottle_offline_cfs_rqs is only ever invoked when CONFIG_SMP
let's pre-processor it out when SMP is not configured. This change
suppresses the build error below when !CONFIG_SMP.

kernel/sched/fair.c:2055:13: warning: ‘unthrottle_offline_cfs_rqs’ defined but not used [-Wunused-function]

Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 kernel/sched/fair.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 6b800a1..c0aab13 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -2052,6 +2052,7 @@ static void destroy_cfs_bandwidth(struct cfs_bandwidth *cfs_b)
 	hrtimer_cancel(&cfs_b->slack_timer);
 }
 
+#if defined CONFIG_SMP
 static void unthrottle_offline_cfs_rqs(struct rq *rq)
 {
 	struct cfs_rq *cfs_rq;
@@ -2071,6 +2072,7 @@ static void unthrottle_offline_cfs_rqs(struct rq *rq)
 			unthrottle_cfs_rq(cfs_rq);
 	}
 }
+#endif /* CONFIG_SMP */
 
 #else /* CONFIG_CFS_BANDWIDTH */
 static __always_inline
@@ -2106,7 +2108,9 @@ static inline struct cfs_bandwidth *tg_cfs_bandwidth(struct task_group *tg)
 	return NULL;
 }
 static inline void destroy_cfs_bandwidth(struct cfs_bandwidth *cfs_b) {}
+#if defined CONFIG_SMP
 static inline void unthrottle_offline_cfs_rqs(struct rq *rq) {}
+#endif /* CONFIG_SMP */
 
 #endif /* CONFIG_CFS_BANDWIDTH */
 
-- 
1.7.9.5

[PATCH 5/9] ASoC: Only compile adav80x codec when a bus is defined

[Lee Jones]

If !CONFIG_SPI and !CONFIG_I2C* then there is no point adding support
for the adav80x codec as it relies on a bus to operate. This patch
fixes the build warnings below when SPI and I2C are not specified in
the configuration.

sound/soc/codecs/adav80x.c:842:22: warning: ‘adav80x_bus_probe’ defined but not used [-Wunused-function]
sound/soc/codecs/adav80x.c:863:22: warning: ‘adav80x_bus_remove’ defined but not used [-Wunused-function]

Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: alsa-devel@alsa-project.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 sound/soc/codecs/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index b92759a..0d8135a 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -19,7 +19,7 @@ config SND_SOC_ALL_CODECS
 	select SND_SOC_AD1980 if SND_SOC_AC97_BUS
 	select SND_SOC_AD73311
 	select SND_SOC_ADAU1373 if I2C
-	select SND_SOC_ADAV80X
+	select SND_SOC_ADAV80X if (I2C || I2C_MODULE || SPI_MASTER)
 	select SND_SOC_ADS117X
 	select SND_SOC_AK4104 if SPI_MASTER
 	select SND_SOC_AK4535 if I2C
-- 
1.7.9.5

[PATCH 5/9] ASoC: Only compile adav80x codec when a bus is defined

[Lee Jones]

If !CONFIG_SPI and !CONFIG_I2C* then there is no point adding support
for the adav80x codec as it relies on a bus to operate. This patch
fixes the build warnings below when SPI and I2C are not specified in
the configuration.

sound/soc/codecs/adav80x.c:842:22: warning: ‘adav80x_bus_probe’ defined but not used [-Wunused-function]
sound/soc/codecs/adav80x.c:863:22: warning: ‘adav80x_bus_remove’ defined but not used [-Wunused-function]

Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: alsa-devel@alsa-project.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 sound/soc/codecs/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index b92759a..0d8135a 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -19,7 +19,7 @@ config SND_SOC_ALL_CODECS
 	select SND_SOC_AD1980 if SND_SOC_AC97_BUS
 	select SND_SOC_AD73311
 	select SND_SOC_ADAU1373 if I2C
-	select SND_SOC_ADAV80X
+	select SND_SOC_ADAV80X if (I2C || I2C_MODULE || SPI_MASTER)
 	select SND_SOC_ADS117X
 	select SND_SOC_AK4104 if SPI_MASTER
 	select SND_SOC_AK4535 if I2C
-- 
1.7.9.5

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH 6/9] virtio_mmio: Cast &resources[1].start to ‘unsigned int *’ to rid compiler warning

[Lee Jones]

drivers/virtio/virtio_mmio.c: In function ‘vm_cmdline_set’:
drivers/virtio/virtio_mmio.c:535:4: warning: format ‘%u’ expects argument of type ‘unsigned int *’, but argument 4 has type ‘resource_size_t *’ [-Wformat]

Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/virtio/virtio_mmio.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 6b1b7e1..077e9ca 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -531,7 +531,7 @@ static int vm_cmdline_set(const char *device,
 	resources[0].end = memparse(device, &str) - 1;
 
 	processed = sscanf(str, "@%lli:%u%n:%d%n",
-			&base, &resources[1].start, &consumed,
+			&base, (unsigned int *)&resources[1].start, &consumed,
 			&vm_cmdline_id, &consumed);
 
 	if (processed < 2 || processed > 3 || str[consumed])
-- 
1.7.9.5

[PATCH 6/9] virtio_mmio: Cast &resources[1].start to ‘unsigned int *’ to rid compiler warning

[Lee Jones]

drivers/virtio/virtio_mmio.c: In function ‘vm_cmdline_set’:
drivers/virtio/virtio_mmio.c:535:4: warning: format ‘%u’ expects argument of type ‘unsigned int *’, but argument 4 has type ‘resource_size_t *’ [-Wformat]

Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/virtio/virtio_mmio.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 6b1b7e1..077e9ca 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -531,7 +531,7 @@ static int vm_cmdline_set(const char *device,
 	resources[0].end = memparse(device, &str) - 1;
 
 	processed = sscanf(str, "@%lli:%u%n:%d%n",
-			&base, &resources[1].start, &consumed,
+			&base, (unsigned int *)&resources[1].start, &consumed,
 			&vm_cmdline_id, &consumed);
 
 	if (processed < 2 || processed > 3 || str[consumed])
-- 
1.7.9.5

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 7/9] quota: Use the pre-processor to compile out quotactl_cmd_write when !CONFIG_BLOCK

[Lee Jones]

quotactl_cmd_write() is only ever invoked when BLOCK is configured. When
!CONFIG_BLOCK, the build warning below is displayed. Let's fix that.

fs/quota/quota.c:311:12: warning: ‘quotactl_cmd_write’ defined but not used [-Wunused-function]

Cc: Jan Kara <jack@suse.cz>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 fs/quota/quota.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/quota/quota.c b/fs/quota/quota.c
index af1661f..c7314f1 100644
--- a/fs/quota/quota.c
+++ b/fs/quota/quota.c
@@ -307,6 +307,8 @@ static int do_quotactl(struct super_block *sb, int type, int cmd, qid_t id,
 	}
 }
 
+#ifdef CONFIG_BLOCK
+
 /* Return 1 if 'cmd' will block on frozen filesystem */
 static int quotactl_cmd_write(int cmd)
 {
@@ -322,6 +324,8 @@ static int quotactl_cmd_write(int cmd)
 	return 1;
 }
 
+#endif /* CONFIG_BLOCK */
+
 /*
  * look up a superblock on which quota ops will be performed
  * - use the name of a block device to find the superblock thereon
-- 
1.7.9.5

[PATCH 8/9] isdn: Remove unused variable causing a compile build warning

[Lee Jones]

This patch fixes:
drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’:
drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable]

Cc: Karsten Keil <isdn@linux-pingi.de>
Cc: netdev@vger.kernel.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/isdn/i4l/isdn_common.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/isdn/i4l/isdn_common.c b/drivers/isdn/i4l/isdn_common.c
index 8c610fa..fb90d40 100644
--- a/drivers/isdn/i4l/isdn_common.c
+++ b/drivers/isdn/i4l/isdn_common.c
@@ -1275,7 +1275,6 @@ isdn_ioctl(struct file *file, uint cmd, ulong arg)
 	int ret;
 	int i;
 	char __user *p;
-	char *s;
 	union iocpar {
 		char name[10];
 		char bname[22];
-- 
1.7.9.5

[PATCH 9/9] Avoid 'statement with no effect' compiler warnings

[Lee Jones]

Instead of issuing (0) statements when !CONFIG_SYSFS which will cause
'warning: ', we'll use inline statements instead. This will effectively
do the same thing, but suppress any unnecessary warnings.

Cc: Stephen Hemminger <shemminger@vyatta.com>
Cc: bridge@lists.linux-foundation.org
Cc: netdev@vger.kernel.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 net/bridge/br_private.h |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 9b278c4..af5f584 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -566,10 +566,10 @@ extern void br_sysfs_delbr(struct net_device *dev);
 
 #else
 
-#define br_sysfs_addif(p)	(0)
-#define br_sysfs_renameif(p)	(0)
-#define br_sysfs_addbr(dev)	(0)
-#define br_sysfs_delbr(dev)	do { } while(0)
+static inline int br_sysfs_addif(struct net_bridge_port *p) { return 0; }
+static inline int br_sysfs_renameif(struct net_bridge_port *p) { return 0; }
+static inline int br_sysfs_addbr(struct net_device *dev) { return 0; }
+static inline void br_sysfs_delbr(struct net_device *dev) { return; }
 #endif /* CONFIG_SYSFS */
 
 #endif
-- 
1.7.9.5

[Bridge] [PATCH 9/9] Avoid 'statement with no effect' compiler warnings

[Lee Jones]

Instead of issuing (0) statements when !CONFIG_SYSFS which will cause
'warning: ', we'll use inline statements instead. This will effectively
do the same thing, but suppress any unnecessary warnings.

Cc: Stephen Hemminger <shemminger@vyatta.com>
Cc: bridge@lists.linux-foundation.org
Cc: netdev@vger.kernel.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 net/bridge/br_private.h |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 9b278c4..af5f584 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -566,10 +566,10 @@ extern void br_sysfs_delbr(struct net_device *dev);
 
 #else
 
-#define br_sysfs_addif(p)	(0)
-#define br_sysfs_renameif(p)	(0)
-#define br_sysfs_addbr(dev)	(0)
-#define br_sysfs_delbr(dev)	do { } while(0)
+static inline int br_sysfs_addif(struct net_bridge_port *p) { return 0; }
+static inline int br_sysfs_renameif(struct net_bridge_port *p) { return 0; }
+static inline int br_sysfs_addbr(struct net_device *dev) { return 0; }
+static inline void br_sysfs_delbr(struct net_device *dev) { return; }
 #endif /* CONFIG_SYSFS */
 
 #endif
-- 
1.7.9.5

Re: [PATCH 8/9] isdn: Remove unused variable causing a compile build warning

[Paul Bolle]

On Sat, 2012-11-03 at 23:02 +0100, Lee Jones wrote:
> This patch fixes:
> drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’:
> drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable]

Did you have CONFIG_NETDEVICES not set in this build?


Paul Bolle

Re: [PATCH 8/9] isdn: Remove unused variable causing a compile build warning

[Lee Jones]

On Sat, 03 Nov 2012, Paul Bolle wrote:

> On Sat, 2012-11-03 at 23:02 +0100, Lee Jones wrote:
> > This patch fixes:
> > drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’:
> > drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable]
> 
> Did you have CONFIG_NETDEVICES not set in this build?

Ah yes, I see it. The function went down further than I thought
it did. So the real fix is to ensure 's' is defined inside of
some ifdef CONFIG_NETDEVICES guards. 

I'll fix up and resend. Will likely be tomorrow now, as it's
fast approaching midnight here.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 9/9] Avoid 'statement with no effect' compiler warnings

[David Miller]

From: Lee Jones <lee.jones@linaro.org>
Date: Sat,  3 Nov 2012 23:02:30 +0100

> Instead of issuing (0) statements when !CONFIG_SYSFS which will cause
> 'warning: ', we'll use inline statements instead. This will effectively
> do the same thing, but suppress any unnecessary warnings.
> 
> Cc: Stephen Hemminger <shemminger@vyatta.com>
> Cc: bridge@lists.linux-foundation.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Applied, but please use more informative subject lines.

You should prefix your subject line after [PATCH ...] with
the name of the subsystem you are touching, a ": " then
the headline description.

So here you would have used "bridge: " and that's what I added when I
commited this patch.

Re: [Bridge] [PATCH 9/9] Avoid 'statement with no effect' compiler warnings

[David Miller]

From: Lee Jones <lee.jones@linaro.org>
Date: Sat,  3 Nov 2012 23:02:30 +0100

> Instead of issuing (0) statements when !CONFIG_SYSFS which will cause
> 'warning: ', we'll use inline statements instead. This will effectively
> do the same thing, but suppress any unnecessary warnings.
> 
> Cc: Stephen Hemminger <shemminger@vyatta.com>
> Cc: bridge@lists.linux-foundation.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Applied, but please use more informative subject lines.

You should prefix your subject line after [PATCH ...] with
the name of the subsystem you are touching, a ": " then
the headline description.

So here you would have used "bridge: " and that's what I added when I
commited this patch.

Re: [PATCH 9/9] Avoid 'statement with no effect' compiler warnings

[Lee Jones]

On Sun, 04 Nov 2012, David Miller wrote:

> From: Lee Jones <lee.jones@linaro.org>
> Date: Sat,  3 Nov 2012 23:02:30 +0100
> 
> > Instead of issuing (0) statements when !CONFIG_SYSFS which will cause
> > 'warning: ', we'll use inline statements instead. This will effectively
> > do the same thing, but suppress any unnecessary warnings.
> > 
> > Cc: Stephen Hemminger <shemminger@vyatta.com>
> > Cc: bridge@lists.linux-foundation.org
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> Applied, but please use more informative subject lines.
> 
> You should prefix your subject line after [PATCH ...] with
> the name of the subsystem you are touching, a ": " then
> the headline description.
> 
> So here you would have used "bridge: " and that's what I added when I
> commited this patch.

Yes, of course I should have done, and usually do.

This was an oversight, sorry about that.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [Bridge] [PATCH 9/9] Avoid 'statement with no effect' compiler warnings

[Lee Jones]

On Sun, 04 Nov 2012, David Miller wrote:

> From: Lee Jones <lee.jones@linaro.org>
> Date: Sat,  3 Nov 2012 23:02:30 +0100
> 
> > Instead of issuing (0) statements when !CONFIG_SYSFS which will cause
> > 'warning: ', we'll use inline statements instead. This will effectively
> > do the same thing, but suppress any unnecessary warnings.
> > 
> > Cc: Stephen Hemminger <shemminger@vyatta.com>
> > Cc: bridge@lists.linux-foundation.org
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> Applied, but please use more informative subject lines.
> 
> You should prefix your subject line after [PATCH ...] with
> the name of the subsystem you are touching, a ": " then
> the headline description.
> 
> So here you would have used "bridge: " and that's what I added when I
> commited this patch.

Yes, of course I should have done, and usually do.

This was an oversight, sorry about that.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [alsa-devel] [PATCH 5/9] ASoC: Only compile adav80x codec when a bus is defined

[Lars-Peter Clausen]

On 11/03/2012 11:02 PM, Lee Jones wrote:
> If !CONFIG_SPI and !CONFIG_I2C* then there is no point adding support
> for the adav80x codec as it relies on a bus to operate. This patch
> fixes the build warnings below when SPI and I2C are not specified in
> the configuration.
> 
> sound/soc/codecs/adav80x.c:842:22: warning: ‘adav80x_bus_probe’ defined but not used [-Wunused-function]
> sound/soc/codecs/adav80x.c:863:22: warning: ‘adav80x_bus_remove’ defined but not used [-Wunused-function]
> 
> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Cc: alsa-devel@alsa-project.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  sound/soc/codecs/Kconfig |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index b92759a..0d8135a 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -19,7 +19,7 @@ config SND_SOC_ALL_CODECS
>  	select SND_SOC_AD1980 if SND_SOC_AC97_BUS
>  	select SND_SOC_AD73311
>  	select SND_SOC_ADAU1373 if I2C
> -	select SND_SOC_ADAV80X
> +	select SND_SOC_ADAV80X if (I2C || I2C_MODULE || SPI_MASTER)

That should be 'if SND_SOC_I2C_AND_SPI', but otherwise agreed.

>  	select SND_SOC_ADS117X
>  	select SND_SOC_AK4104 if SPI_MASTER
>  	select SND_SOC_AK4535 if I2C

Re: [PATCH 8/9] isdn: Remove unused variable causing a compile build warning

[Paul Bolle]

On Sat, 2012-11-03 at 23:48 +0100, Lee Jones wrote:
> On Sat, 03 Nov 2012, Paul Bolle wrote:
> > On Sat, 2012-11-03 at 23:02 +0100, Lee Jones wrote:
> > > This patch fixes:
> > > drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’:
> > > drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable]
> > 
> > Did you have CONFIG_NETDEVICES not set in this build?
> 
> Ah yes, I see it. The function went down further than I thought
> it did. So the real fix is to ensure 's' is defined inside of
> some ifdef CONFIG_NETDEVICES guards. 

What puzzles me is that we only find these "#ifdef CONFIG_NETDEVICES"
guards in this file and not in isdn_net.c, were all the ioctl commands
guarded that way seem to be calling into. On first glance that doesn't
make much sense.

(Actually the idea of having ISDN without NETDEVICES is a bit puzzling
too. But there are too many parts of the isdn subsystem that I'm
unfamiliar with to say whether that can make sense.)


Paul Bolle

Re: [PATCH 2/9] ARM: at91: Remove unused struct at91sam9g45_isi_device and its resources

[Jean-Christophe PLAGNIOL-VILLARD]

On 23:02 Sat 03 Nov     , Lee Jones wrote:
> This the at91sam9g45_isi_device structure and its associated resources
> were added in 2008 and have been unused ever since. Let's remove them.
next time you need to Cc the matinaers

NACK

Best Regards,
J.
> 
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  arch/arm/mach-at91/at91sam9263_devices.c |   20 --------------------
>  1 file changed, 20 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
> index cb85da2..0562a9d 100644
> --- a/arch/arm/mach-at91/at91sam9263_devices.c
> +++ b/arch/arm/mach-at91/at91sam9263_devices.c
> @@ -901,26 +901,6 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data) {}
>  
>  #if defined(CONFIG_VIDEO_AT91_ISI) || defined(CONFIG_VIDEO_AT91_ISI_MODULE)
>  
> -struct resource isi_resources[] = {
> -	[0] = {
> -		.start	= AT91SAM9263_BASE_ISI,
> -		.end	= AT91SAM9263_BASE_ISI + SZ_16K - 1,
> -		.flags	= IORESOURCE_MEM,
> -	},
> -	[1] = {
> -		.start	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
> -		.end	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
> -		.flags	= IORESOURCE_IRQ,
> -	},
> -};
> -
> -static struct platform_device at91sam9263_isi_device = {
> -	.name		= "at91_isi",
> -	.id		= -1,
> -	.resource	= isi_resources,
> -	.num_resources	= ARRAY_SIZE(isi_resources),
> -};
> -
>  void __init at91_add_device_isi(struct isi_platform_data *data,
>  		bool use_pck_as_mck)
>  {
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/9] ARM: at91: Remove unused struct at91sam9g45_isi_device and its resources

[Jean-Christophe PLAGNIOL-VILLARD]

On 23:02 Sat 03 Nov     , Lee Jones wrote:
> This the at91sam9g45_isi_device structure and its associated resources
> were added in 2008 and have been unused ever since. Let's remove them.
next time you need to Cc the matinaers

NACK

Best Regards,
J.
> 
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: linux-arm-kernel at lists.infradead.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  arch/arm/mach-at91/at91sam9263_devices.c |   20 --------------------
>  1 file changed, 20 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
> index cb85da2..0562a9d 100644
> --- a/arch/arm/mach-at91/at91sam9263_devices.c
> +++ b/arch/arm/mach-at91/at91sam9263_devices.c
> @@ -901,26 +901,6 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data) {}
>  
>  #if defined(CONFIG_VIDEO_AT91_ISI) || defined(CONFIG_VIDEO_AT91_ISI_MODULE)
>  
> -struct resource isi_resources[] = {
> -	[0] = {
> -		.start	= AT91SAM9263_BASE_ISI,
> -		.end	= AT91SAM9263_BASE_ISI + SZ_16K - 1,
> -		.flags	= IORESOURCE_MEM,
> -	},
> -	[1] = {
> -		.start	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
> -		.end	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
> -		.flags	= IORESOURCE_IRQ,
> -	},
> -};
> -
> -static struct platform_device at91sam9263_isi_device = {
> -	.name		= "at91_isi",
> -	.id		= -1,
> -	.resource	= isi_resources,
> -	.num_resources	= ARRAY_SIZE(isi_resources),
> -};
> -
>  void __init at91_add_device_isi(struct isi_platform_data *data,
>  		bool use_pck_as_mck)
>  {
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 8/9] isdn: Remove unused variable causing a compile build warning

[Lee Jones]

Thanks for reviewing Paul. Here's the result:

Author: Lee Jones <lee.jones@linaro.org>
Date:   Sat Nov 3 22:06:02 2012 +0100

    isdn: Encapsulate variable 's' in same CONFIG_NETDEVICES guards as code using it
    
    In the current case, variable 's' of type 'char *' is defined but then
    not used if !CONFIG_NETDEVICES, casing the compile-time warning below.
    In this change we surround the declaration using the same guards as
    the pre-processed code which makes use of it.
    
    This patch fixes:
    drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’:
    drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable]
    
    Cc: Karsten Keil <isdn@linux-pingi.de>
    Cc: netdev@vger.kernel.org
    Signed-off-by: Lee Jones <lee.jones@linaro.org>

diff --git a/drivers/isdn/i4l/isdn_common.c b/drivers/isdn/i4l/isdn_common.c
index 8c610fa..2875f31 100644
--- a/drivers/isdn/i4l/isdn_common.c
+++ b/drivers/isdn/i4l/isdn_common.c
@@ -1275,7 +1275,6 @@ isdn_ioctl(struct file *file, uint cmd, ulong arg)
 	int ret;
 	int i;
 	char __user *p;
-	char *s;
 	union iocpar {
 		char name[10];
 		char bname[22];
@@ -1284,6 +1283,9 @@ isdn_ioctl(struct file *file, uint cmd, ulong arg)
 		isdn_net_ioctl_cfg cfg;
 	} iocpar;
 	void __user *argp = (void __user *)arg;
+#ifdef CONFIG_NETDEVICES
+	char *s;
+#endif
 
 #define name  iocpar.name
 #define bname iocpar.bname

Re: [alsa-devel] [PATCH 5/9] ASoC: Only compile adav80x codec when a bus is defined

[Lee Jones]

Thanks for reviewing Lars. Here's the result:

commit c4f60d00d04abfb730a54a97204873dafc6473ae
Author: Lee Jones <lee.jones@linaro.org>
Date:   Sat Nov 3 22:54:23 2012 +0100

    ASoC: Only compile adav80x codec when a bus is defined
    
    If !CONFIG_SPI and !CONFIG_I2C* then there is no point adding support
    for the adav80x codec as it relies on a bus to operate. This patch
    fixes the build warnings below when SPI and I2C are not specified in
    the configuration.
    
    sound/soc/codecs/adav80x.c:842:22: warning: ‘adav80x_bus_probe’ defined but not used [-Wunused-function]
    sound/soc/codecs/adav80x.c:863:22: warning: ‘adav80x_bus_remove’ defined but not used [-Wunused-function]
    
    Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
    Cc: alsa-devel@alsa-project.org
    Signed-off-by: Lee Jones <lee.jones@linaro.org>

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index b92759a..3ae3a4b 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -19,7 +19,7 @@ config SND_SOC_ALL_CODECS
 	select SND_SOC_AD1980 if SND_SOC_AC97_BUS
 	select SND_SOC_AD73311
 	select SND_SOC_ADAU1373 if I2C
-	select SND_SOC_ADAV80X
+	select SND_SOC_ADAV80X if SND_SOC_I2C_AND_SPI
 	select SND_SOC_ADS117X
 	select SND_SOC_AK4104 if SPI_MASTER
 	select SND_SOC_AK4535 if I2C

Re: [PATCH 8/9] isdn: Remove unused variable causing a compile build warning

[Lee Jones]

On Sun, 04 Nov 2012, Paul Bolle wrote:

> On Sat, 2012-11-03 at 23:48 +0100, Lee Jones wrote:
> > On Sat, 03 Nov 2012, Paul Bolle wrote:
> > > On Sat, 2012-11-03 at 23:02 +0100, Lee Jones wrote:
> > > > This patch fixes:
> > > > drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’:
> > > > drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable]
> > > 
> > > Did you have CONFIG_NETDEVICES not set in this build?
> > 
> > Ah yes, I see it. The function went down further than I thought
> > it did. So the real fix is to ensure 's' is defined inside of
> > some ifdef CONFIG_NETDEVICES guards. 
> 
> What puzzles me is that we only find these "#ifdef CONFIG_NETDEVICES"
> guards in this file and not in isdn_net.c, were all the ioctl commands
> guarded that way seem to be calling into. On first glance that doesn't
> make much sense.
> 
> (Actually the idea of having ISDN without NETDEVICES is a bit puzzling
> too. But there are too many parts of the isdn subsystem that I'm
> unfamiliar with to say whether that can make sense.)

I'm in the same position as you Paul. I just noticed the warning so
fixed it following the current way of doing things. Any, more
substantial changes requiring greater knowledge of the subsystem
would have to be done by someone else.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/9] ARM: at91: Remove unused struct at91sam9g45_isi_device and its resources

[Lee Jones]

On Sun, 04 Nov 2012, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 23:02 Sat 03 Nov     , Lee Jones wrote:
> > This the at91sam9g45_isi_device structure and its associated resources
> > were added in 2008 and have been unused ever since. Let's remove them.
> next time you need to Cc the matinaers

I did:

$ git show <sha1> | scripts/get_maintainer.pl
  Russell King <linux@arm.linux.org.uk> (maintainer:ARM PORT)
  linux-arm-kernel@lists.infradead.org (moderated list:ARM SUB-ARCHITECT...)
  linux-kernel@vger.kernel.org (open list)

> NACK

Any particular reason?

> > Cc: Russell King <linux@arm.linux.org.uk>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  arch/arm/mach-at91/at91sam9263_devices.c |   20 --------------------
> >  1 file changed, 20 deletions(-)
> > 
> > diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
> > index cb85da2..0562a9d 100644
> > --- a/arch/arm/mach-at91/at91sam9263_devices.c
> > +++ b/arch/arm/mach-at91/at91sam9263_devices.c
> > @@ -901,26 +901,6 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data) {}
> >  
> >  #if defined(CONFIG_VIDEO_AT91_ISI) || defined(CONFIG_VIDEO_AT91_ISI_MODULE)
> >  
> > -struct resource isi_resources[] = {
> > -	[0] = {
> > -		.start	= AT91SAM9263_BASE_ISI,
> > -		.end	= AT91SAM9263_BASE_ISI + SZ_16K - 1,
> > -		.flags	= IORESOURCE_MEM,
> > -	},
> > -	[1] = {
> > -		.start	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
> > -		.end	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
> > -		.flags	= IORESOURCE_IRQ,
> > -	},
> > -};
> > -
> > -static struct platform_device at91sam9263_isi_device = {
> > -	.name		= "at91_isi",
> > -	.id		= -1,
> > -	.resource	= isi_resources,
> > -	.num_resources	= ARRAY_SIZE(isi_resources),
> > -};
> > -
> >  void __init at91_add_device_isi(struct isi_platform_data *data,
> >  		bool use_pck_as_mck)
> >  {
> > -- 
> > 1.7.9.5
> > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 2/9] ARM: at91: Remove unused struct at91sam9g45_isi_device and its resources

[Lee Jones]

On Sun, 04 Nov 2012, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 23:02 Sat 03 Nov     , Lee Jones wrote:
> > This the at91sam9g45_isi_device structure and its associated resources
> > were added in 2008 and have been unused ever since. Let's remove them.
> next time you need to Cc the matinaers

I did:

$ git show <sha1> | scripts/get_maintainer.pl
  Russell King <linux@arm.linux.org.uk> (maintainer:ARM PORT)
  linux-arm-kernel at lists.infradead.org (moderated list:ARM SUB-ARCHITECT...)
  linux-kernel at vger.kernel.org (open list)

> NACK

Any particular reason?

> > Cc: Russell King <linux@arm.linux.org.uk>
> > Cc: linux-arm-kernel at lists.infradead.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  arch/arm/mach-at91/at91sam9263_devices.c |   20 --------------------
> >  1 file changed, 20 deletions(-)
> > 
> > diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
> > index cb85da2..0562a9d 100644
> > --- a/arch/arm/mach-at91/at91sam9263_devices.c
> > +++ b/arch/arm/mach-at91/at91sam9263_devices.c
> > @@ -901,26 +901,6 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data) {}
> >  
> >  #if defined(CONFIG_VIDEO_AT91_ISI) || defined(CONFIG_VIDEO_AT91_ISI_MODULE)
> >  
> > -struct resource isi_resources[] = {
> > -	[0] = {
> > -		.start	= AT91SAM9263_BASE_ISI,
> > -		.end	= AT91SAM9263_BASE_ISI + SZ_16K - 1,
> > -		.flags	= IORESOURCE_MEM,
> > -	},
> > -	[1] = {
> > -		.start	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
> > -		.end	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
> > -		.flags	= IORESOURCE_IRQ,
> > -	},
> > -};
> > -
> > -static struct platform_device at91sam9263_isi_device = {
> > -	.name		= "at91_isi",
> > -	.id		= -1,
> > -	.resource	= isi_resources,
> > -	.num_resources	= ARRAY_SIZE(isi_resources),
> > -};
> > -
> >  void __init at91_add_device_isi(struct isi_platform_data *data,
> >  		bool use_pck_as_mck)
> >  {
> > -- 
> > 1.7.9.5
> > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 8/9] isdn: Remove unused variable causing a compile build warning

[David Miller]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: Text/Plain; charset=iso-8859-7, Size: 1689 bytes --]

From: Lee Jones <lee.jones@linaro.org>
Date: Sun, 4 Nov 2012 11:53:32 +0100

> On Sun, 04 Nov 2012, Paul Bolle wrote:
> 
>> On Sat, 2012-11-03 at 23:48 +0100, Lee Jones wrote:
>> > On Sat, 03 Nov 2012, Paul Bolle wrote:
>> > > On Sat, 2012-11-03 at 23:02 +0100, Lee Jones wrote:
>> > > > This patch fixes:
>> > > > drivers/isdn/i4l/isdn_common.c: In function ¡isdn_ioctl¢:
>> > > > drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ¡s¢ [-Wunused-variable]
>> > > 
>> > > Did you have CONFIG_NETDEVICES not set in this build?
>> > 
>> > Ah yes, I see it. The function went down further than I thought
>> > it did. So the real fix is to ensure 's' is defined inside of
>> > some ifdef CONFIG_NETDEVICES guards. 
>> 
>> What puzzles me is that we only find these "#ifdef CONFIG_NETDEVICES"
>> guards in this file and not in isdn_net.c, were all the ioctl commands
>> guarded that way seem to be calling into. On first glance that doesn't
>> make much sense.
>> 
>> (Actually the idea of having ISDN without NETDEVICES is a bit puzzling
>> too. But there are too many parts of the isdn subsystem that I'm
>> unfamiliar with to say whether that can make sense.)
> 
> I'm in the same position as you Paul. I just noticed the warning so
> fixed it following the current way of doing things. Any, more
> substantial changes requiring greater knowledge of the subsystem
> would have to be done by someone else.

I think the most appropriate thing to do is make CONFIG_ISDN depend
upon CONFIG_NETDEVICES in the Kconfig file.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 8/9] isdn: Remove unused variable causing a compile build warning

[David Miller]

From: Lee Jones <lee.jones@linaro.org>
Date: Sun, 4 Nov 2012 11:53:32 +0100

> On Sun, 04 Nov 2012, Paul Bolle wrote:
> 
>> On Sat, 2012-11-03 at 23:48 +0100, Lee Jones wrote:
>> > On Sat, 03 Nov 2012, Paul Bolle wrote:
>> > > On Sat, 2012-11-03 at 23:02 +0100, Lee Jones wrote:
>> > > > This patch fixes:
>> > > > drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’:
>> > > > drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable]
>> > > 
>> > > Did you have CONFIG_NETDEVICES not set in this build?
>> > 
>> > Ah yes, I see it. The function went down further than I thought
>> > it did. So the real fix is to ensure 's' is defined inside of
>> > some ifdef CONFIG_NETDEVICES guards. 
>> 
>> What puzzles me is that we only find these "#ifdef CONFIG_NETDEVICES"
>> guards in this file and not in isdn_net.c, were all the ioctl commands
>> guarded that way seem to be calling into. On first glance that doesn't
>> make much sense.
>> 
>> (Actually the idea of having ISDN without NETDEVICES is a bit puzzling
>> too. But there are too many parts of the isdn subsystem that I'm
>> unfamiliar with to say whether that can make sense.)
> 
> I'm in the same position as you Paul. I just noticed the warning so
> fixed it following the current way of doing things. Any, more
> substantial changes requiring greater knowledge of the subsystem
> would have to be done by someone else.

I think the most appropriate thing to do is make CONFIG_ISDN depend
upon CONFIG_NETDEVICES in the Kconfig file.

Re: [PATCH 6/9] virtio_mmio: Cast &resources[1].start to ‘unsigned int *’ to rid compiler warning

[Rusty Russell]

Lee Jones <lee.jones@linaro.org> writes:
> drivers/virtio/virtio_mmio.c: In function ‘vm_cmdline_set’:
> drivers/virtio/virtio_mmio.c:535:4: warning: format ‘%u’ expects argument of type ‘unsigned int *’, but argument 4 has type ‘resource_size_t *’ [-Wformat]
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: virtualization@lists.linux-foundation.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/virtio/virtio_mmio.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index 6b1b7e1..077e9ca 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -531,7 +531,7 @@ static int vm_cmdline_set(const char *device,
>  	resources[0].end = memparse(device, &str) - 1;
>  
>  	processed = sscanf(str, "@%lli:%u%n:%d%n",
> -			&base, &resources[1].start, &consumed,
> +			&base, (unsigned int *)&resources[1].start, &consumed,
>  			&vm_cmdline_id, &consumed);

This suppresses a valid warning, leaving our code no less wrong than
before.  But now it's silently wrong!

Do you want to try again?

Cheers,
Rusty.

Re: [PATCH 6/9] virtio_mmio: Cast &resources[1].start to ‘unsigned int *’ to rid compiler warning

[Rusty Russell]

Lee Jones <lee.jones@linaro.org> writes:
> drivers/virtio/virtio_mmio.c: In function ‘vm_cmdline_set’:
> drivers/virtio/virtio_mmio.c:535:4: warning: format ‘%u’ expects argument of type ‘unsigned int *’, but argument 4 has type ‘resource_size_t *’ [-Wformat]
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: virtualization@lists.linux-foundation.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/virtio/virtio_mmio.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index 6b1b7e1..077e9ca 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -531,7 +531,7 @@ static int vm_cmdline_set(const char *device,
>  	resources[0].end = memparse(device, &str) - 1;
>  
>  	processed = sscanf(str, "@%lli:%u%n:%d%n",
> -			&base, &resources[1].start, &consumed,
> +			&base, (unsigned int *)&resources[1].start, &consumed,
>  			&vm_cmdline_id, &consumed);

This suppresses a valid warning, leaving our code no less wrong than
before.  But now it's silently wrong!

Do you want to try again?

Cheers,
Rusty.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 2/9] ARM: at91: Remove unused struct at91sam9g45_isi_device and its resources

[Josh Wu]

Hi, Lee Jones

On 11/4/2012 6:02 AM, Lee Jones wrote:
> This the at91sam9g45_isi_device structure and its associated resources
> were added in 2008 and have been unused ever since. Let's remove them.

I'm the maintainer of the Atmel ISI driver. Currently the ISI still not 
work on at91sam9263 board.
But this task is in my plan. So keep those code and I will enable ISI 
support for 9263 in the future. Thanks.

Best Regards,
Josh Wu

> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>   arch/arm/mach-at91/at91sam9263_devices.c |   20 --------------------
>   1 file changed, 20 deletions(-)
>
> diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
> index cb85da2..0562a9d 100644
> --- a/arch/arm/mach-at91/at91sam9263_devices.c
> +++ b/arch/arm/mach-at91/at91sam9263_devices.c
> @@ -901,26 +901,6 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data) {}
>   
>   #if defined(CONFIG_VIDEO_AT91_ISI) || defined(CONFIG_VIDEO_AT91_ISI_MODULE)
>   
> -struct resource isi_resources[] = {
> -	[0] = {
> -		.start	= AT91SAM9263_BASE_ISI,
> -		.end	= AT91SAM9263_BASE_ISI + SZ_16K - 1,
> -		.flags	= IORESOURCE_MEM,
> -	},
> -	[1] = {
> -		.start	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
> -		.end	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
> -		.flags	= IORESOURCE_IRQ,
> -	},
> -};
> -
> -static struct platform_device at91sam9263_isi_device = {
> -	.name		= "at91_isi",
> -	.id		= -1,
> -	.resource	= isi_resources,
> -	.num_resources	= ARRAY_SIZE(isi_resources),
> -};
> -
>   void __init at91_add_device_isi(struct isi_platform_data *data,
>   		bool use_pck_as_mck)
>   {

[PATCH 2/9] ARM: at91: Remove unused struct at91sam9g45_isi_device and its resources

[Josh Wu]

Hi, Lee Jones

On 11/4/2012 6:02 AM, Lee Jones wrote:
> This the at91sam9g45_isi_device structure and its associated resources
> were added in 2008 and have been unused ever since. Let's remove them.

I'm the maintainer of the Atmel ISI driver. Currently the ISI still not 
work on at91sam9263 board.
But this task is in my plan. So keep those code and I will enable ISI 
support for 9263 in the future. Thanks.

Best Regards,
Josh Wu

> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: linux-arm-kernel at lists.infradead.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>   arch/arm/mach-at91/at91sam9263_devices.c |   20 --------------------
>   1 file changed, 20 deletions(-)
>
> diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
> index cb85da2..0562a9d 100644
> --- a/arch/arm/mach-at91/at91sam9263_devices.c
> +++ b/arch/arm/mach-at91/at91sam9263_devices.c
> @@ -901,26 +901,6 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data) {}
>   
>   #if defined(CONFIG_VIDEO_AT91_ISI) || defined(CONFIG_VIDEO_AT91_ISI_MODULE)
>   
> -struct resource isi_resources[] = {
> -	[0] = {
> -		.start	= AT91SAM9263_BASE_ISI,
> -		.end	= AT91SAM9263_BASE_ISI + SZ_16K - 1,
> -		.flags	= IORESOURCE_MEM,
> -	},
> -	[1] = {
> -		.start	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
> -		.end	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
> -		.flags	= IORESOURCE_IRQ,
> -	},
> -};
> -
> -static struct platform_device at91sam9263_isi_device = {
> -	.name		= "at91_isi",
> -	.id		= -1,
> -	.resource	= isi_resources,
> -	.num_resources	= ARRAY_SIZE(isi_resources),
> -};
> -
>   void __init at91_add_device_isi(struct isi_platform_data *data,
>   		bool use_pck_as_mck)
>   {

Re: [PATCH 2/9] ARM: at91: Remove unused struct at91sam9g45_isi_device and its resources

[Lee Jones]

> >This the at91sam9g45_isi_device structure and its associated resources
> >were added in 2008 and have been unused ever since. Let's remove them.
> 
> I'm the maintainer of the Atmel ISI driver. Currently the ISI still
> not work on at91sam9263 board.
> But this task is in my plan. So keep those code and I will enable
> ISI support for 9263 in the future. Thanks.

No problem at all. Thanks for getting back to me Josh.

Kind regards,
Lee

> >Cc: Russell King <linux@arm.linux.org.uk>
> >Cc: linux-arm-kernel@lists.infradead.org
> >Signed-off-by: Lee Jones <lee.jones@linaro.org>
> >---
> >  arch/arm/mach-at91/at91sam9263_devices.c |   20 --------------------
> >  1 file changed, 20 deletions(-)
> >
> >diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
> >index cb85da2..0562a9d 100644
> >--- a/arch/arm/mach-at91/at91sam9263_devices.c
> >+++ b/arch/arm/mach-at91/at91sam9263_devices.c
> >@@ -901,26 +901,6 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data) {}
> >  #if defined(CONFIG_VIDEO_AT91_ISI) || defined(CONFIG_VIDEO_AT91_ISI_MODULE)
> >-struct resource isi_resources[] = {
> >-	[0] = {
> >-		.start	= AT91SAM9263_BASE_ISI,
> >-		.end	= AT91SAM9263_BASE_ISI + SZ_16K - 1,
> >-		.flags	= IORESOURCE_MEM,
> >-	},
> >-	[1] = {
> >-		.start	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
> >-		.end	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
> >-		.flags	= IORESOURCE_IRQ,
> >-	},
> >-};
> >-
> >-static struct platform_device at91sam9263_isi_device = {
> >-	.name		= "at91_isi",
> >-	.id		= -1,
> >-	.resource	= isi_resources,
> >-	.num_resources	= ARRAY_SIZE(isi_resources),
> >-};
> >-
> >  void __init at91_add_device_isi(struct isi_platform_data *data,
> >  		bool use_pck_as_mck)
> >  {
> 

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 2/9] ARM: at91: Remove unused struct at91sam9g45_isi_device and its resources

[Lee Jones]

> >This the at91sam9g45_isi_device structure and its associated resources
> >were added in 2008 and have been unused ever since. Let's remove them.
> 
> I'm the maintainer of the Atmel ISI driver. Currently the ISI still
> not work on at91sam9263 board.
> But this task is in my plan. So keep those code and I will enable
> ISI support for 9263 in the future. Thanks.

No problem at all. Thanks for getting back to me Josh.

Kind regards,
Lee

> >Cc: Russell King <linux@arm.linux.org.uk>
> >Cc: linux-arm-kernel at lists.infradead.org
> >Signed-off-by: Lee Jones <lee.jones@linaro.org>
> >---
> >  arch/arm/mach-at91/at91sam9263_devices.c |   20 --------------------
> >  1 file changed, 20 deletions(-)
> >
> >diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
> >index cb85da2..0562a9d 100644
> >--- a/arch/arm/mach-at91/at91sam9263_devices.c
> >+++ b/arch/arm/mach-at91/at91sam9263_devices.c
> >@@ -901,26 +901,6 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data) {}
> >  #if defined(CONFIG_VIDEO_AT91_ISI) || defined(CONFIG_VIDEO_AT91_ISI_MODULE)
> >-struct resource isi_resources[] = {
> >-	[0] = {
> >-		.start	= AT91SAM9263_BASE_ISI,
> >-		.end	= AT91SAM9263_BASE_ISI + SZ_16K - 1,
> >-		.flags	= IORESOURCE_MEM,
> >-	},
> >-	[1] = {
> >-		.start	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
> >-		.end	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
> >-		.flags	= IORESOURCE_IRQ,
> >-	},
> >-};
> >-
> >-static struct platform_device at91sam9263_isi_device = {
> >-	.name		= "at91_isi",
> >-	.id		= -1,
> >-	.resource	= isi_resources,
> >-	.num_resources	= ARRAY_SIZE(isi_resources),
> >-};
> >-
> >  void __init at91_add_device_isi(struct isi_platform_data *data,
> >  		bool use_pck_as_mck)
> >  {
> 

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 6/9] virtio_mmio: Cast &resources[1].start to ‘unsigned int *’ to rid compiler warning

[Lee Jones]

On Mon, 05 Nov 2012, Rusty Russell wrote:

> Lee Jones <lee.jones@linaro.org> writes:
> > drivers/virtio/virtio_mmio.c: In function ‘vm_cmdline_set’:
> > drivers/virtio/virtio_mmio.c:535:4: warning: format ‘%u’ expects argument of type ‘unsigned int *’, but argument 4 has type ‘resource_size_t *’ [-Wformat]
> >
> > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > Cc: virtualization@lists.linux-foundation.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  drivers/virtio/virtio_mmio.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> > index 6b1b7e1..077e9ca 100644
> > --- a/drivers/virtio/virtio_mmio.c
> > +++ b/drivers/virtio/virtio_mmio.c
> > @@ -531,7 +531,7 @@ static int vm_cmdline_set(const char *device,
> >  	resources[0].end = memparse(device, &str) - 1;
> >  
> >  	processed = sscanf(str, "@%lli:%u%n:%d%n",
> > -			&base, &resources[1].start, &consumed,
> > +			&base, (unsigned int *)&resources[1].start, &consumed,
> >  			&vm_cmdline_id, &consumed);
> 
> This suppresses a valid warning, leaving our code no less wrong than
> before.  But now it's silently wrong!
> 
> Do you want to try again?

For sure I'll try again.

How does `s/%u/%p/` suit?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 6/9] virtio_mmio: Cast &resources[1].start to ‘unsigned int *’ to rid compiler warning

[Lee Jones]

On Mon, 05 Nov 2012, Rusty Russell wrote:

> Lee Jones <lee.jones@linaro.org> writes:
> > drivers/virtio/virtio_mmio.c: In function ‘vm_cmdline_set’:
> > drivers/virtio/virtio_mmio.c:535:4: warning: format ‘%u’ expects argument of type ‘unsigned int *’, but argument 4 has type ‘resource_size_t *’ [-Wformat]
> >
> > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > Cc: virtualization@lists.linux-foundation.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  drivers/virtio/virtio_mmio.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> > index 6b1b7e1..077e9ca 100644
> > --- a/drivers/virtio/virtio_mmio.c
> > +++ b/drivers/virtio/virtio_mmio.c
> > @@ -531,7 +531,7 @@ static int vm_cmdline_set(const char *device,
> >  	resources[0].end = memparse(device, &str) - 1;
> >  
> >  	processed = sscanf(str, "@%lli:%u%n:%d%n",
> > -			&base, &resources[1].start, &consumed,
> > +			&base, (unsigned int *)&resources[1].start, &consumed,
> >  			&vm_cmdline_id, &consumed);
> 
> This suppresses a valid warning, leaving our code no less wrong than
> before.  But now it's silently wrong!
> 
> Do you want to try again?

For sure I'll try again.

How does `s/%u/%p/` suit?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 8/9] isdn: Remove unused variable causing a compile build warning

[Lee Jones]

On Sun, 04 Nov 2012, David Miller wrote:

> From: Lee Jones <lee.jones@linaro.org>
> Date: Sun, 4 Nov 2012 11:53:32 +0100
> 
> > On Sun, 04 Nov 2012, Paul Bolle wrote:
> > 
> >> On Sat, 2012-11-03 at 23:48 +0100, Lee Jones wrote:
> >> > On Sat, 03 Nov 2012, Paul Bolle wrote:
> >> > > On Sat, 2012-11-03 at 23:02 +0100, Lee Jones wrote:
> >> > > > This patch fixes:
> >> > > > drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’:
> >> > > > drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable]
> >> > > 
> >> > > Did you have CONFIG_NETDEVICES not set in this build?
> >> > 
> >> > Ah yes, I see it. The function went down further than I thought
> >> > it did. So the real fix is to ensure 's' is defined inside of
> >> > some ifdef CONFIG_NETDEVICES guards. 
> >> 
> >> What puzzles me is that we only find these "#ifdef CONFIG_NETDEVICES"
> >> guards in this file and not in isdn_net.c, were all the ioctl commands
> >> guarded that way seem to be calling into. On first glance that doesn't
> >> make much sense.
> >> 
> >> (Actually the idea of having ISDN without NETDEVICES is a bit puzzling
> >> too. But there are too many parts of the isdn subsystem that I'm
> >> unfamiliar with to say whether that can make sense.)
> > 
> > I'm in the same position as you Paul. I just noticed the warning so
> > fixed it following the current way of doing things. Any, more
> > substantial changes requiring greater knowledge of the subsystem
> > would have to be done by someone else.
> 
> I think the most appropriate thing to do is make CONFIG_ISDN depend
> upon CONFIG_NETDEVICES in the Kconfig file.

... and then remove the CONFIG_NETDEVICES guards in isdn_common.c?

If that's suitable more suitable I can do that instead?

Just need a yay or nay and I'll make it happen.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 8/9] isdn: Remove unused variable causing a compile build warning

[Paul Bolle]

On Mon, 2012-11-05 at 09:44 +0100, Lee Jones wrote:
> On Sun, 04 Nov 2012, David Miller wrote:
> > I think the most appropriate thing to do is make CONFIG_ISDN depend
> > upon CONFIG_NETDEVICES in the Kconfig file.
> 
> ... and then remove the CONFIG_NETDEVICES guards in isdn_common.c?

If ISDN would depend on NETDEVICES, ISDN_I4L would too, since it depends
on ISDN. In that case CONFIG_NETDEVICES would always be true when
compiling isdn_common.c. That would make these guards pointless. (The
dependency of ISDN_PPP on NETDEVICES would then also be pointless.)


Paul Bolle

Re: [PATCH 8/9] isdn: Remove unused variable causing a compile build warning

[Lee Jones]

On Mon, 05 Nov 2012, Paul Bolle wrote:

> On Mon, 2012-11-05 at 09:44 +0100, Lee Jones wrote:
> > On Sun, 04 Nov 2012, David Miller wrote:
> > > I think the most appropriate thing to do is make CONFIG_ISDN depend
> > > upon CONFIG_NETDEVICES in the Kconfig file.
> > 
> > ... and then remove the CONFIG_NETDEVICES guards in isdn_common.c?
> 
> If ISDN would depend on NETDEVICES, ISDN_I4L would too, since it depends
> on ISDN. In that case CONFIG_NETDEVICES would always be true when
> compiling isdn_common.c. That would make these guards pointless. (The
> dependency of ISDN_PPP on NETDEVICES would then also be pointless.)

I'll submit a patch based on your comments.

Thanks for your time Paul.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 8/9] isdn: Make CONFIG_ISDN depend on CONFIG_NETDEVICES

[Lee Jones]

Does something like look like a better solution?

Author: Lee Jones <lee.jones@linaro.org>
Date:   Sat Nov 3 22:06:02 2012 +0100

    isdn: Make CONFIG_ISDN depend on CONFIG_NETDEVICES
    
    It doesn't make much sense to enable ISDN services if you don't
    intend to connect to a network. Therefore insisting that ISDN
    depends on NETDEVICES seems logical. We can then remove any
    guards mentioning NETDEVICES inside all subordinate drivers.
    
    This also has the nice side-effect of fixing the warning below
    when ISDN_I4L && !CONFIG_NETDEVICES at compile time.
    
    This patch fixes:
    drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’:
    drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable]
    
    Cc: Karsten Keil <isdn@linux-pingi.de>
    Cc: netdev@vger.kernel.org
    Signed-off-by: Lee Jones <lee.jones@linaro.org>

diff --git a/drivers/isdn/Kconfig b/drivers/isdn/Kconfig
index a233ed5..86cd75a 100644
--- a/drivers/isdn/Kconfig
+++ b/drivers/isdn/Kconfig
@@ -4,7 +4,7 @@
 
 menuconfig ISDN
 	bool "ISDN support"
-	depends on NET
+	depends on NET && NETDEVICES
 	depends on !S390 && !UML
 	---help---
 	  ISDN ("Integrated Services Digital Network", called RNIS in France)
diff --git a/drivers/isdn/i4l/Kconfig b/drivers/isdn/i4l/Kconfig
index 2302fbe..9c6650e 100644
--- a/drivers/isdn/i4l/Kconfig
+++ b/drivers/isdn/i4l/Kconfig
@@ -6,7 +6,7 @@ if ISDN_I4L
 
 config ISDN_PPP
 	bool "Support synchronous PPP"
-	depends on INET && NETDEVICES
+	depends on INET
 	select SLHC
 	help
 	  Over digital connections such as ISDN, there is no need to
diff --git a/drivers/isdn/i4l/isdn_common.c b/drivers/isdn/i4l/isdn_common.c
index 8c610fa..e2a945e 100644
--- a/drivers/isdn/i4l/isdn_common.c
+++ b/drivers/isdn/i4l/isdn_common.c
@@ -1312,7 +1312,6 @@ isdn_ioctl(struct file *file, uint cmd, ulong arg)
 			} else
 				return -EINVAL;
 			break;
-#ifdef CONFIG_NETDEVICES
 		case IIOCNETGPN:
 			/* Get peer phone number of a connected
 			 * isdn network interface */
@@ -1322,7 +1321,6 @@ isdn_ioctl(struct file *file, uint cmd, ulong arg)
 				return isdn_net_getpeer(&phone, argp);
 			} else
 				return -EINVAL;
-#endif
 		default:
 			return -EINVAL;
 		}
@@ -1352,7 +1350,6 @@ isdn_ioctl(struct file *file, uint cmd, ulong arg)
 		case IIOCNETLCR:
 			printk(KERN_INFO "INFO: ISDN_ABC_LCR_SUPPORT not enabled\n");
 			return -ENODEV;
-#ifdef CONFIG_NETDEVICES
 		case IIOCNETAIF:
 			/* Add a network-interface */
 			if (arg) {
@@ -1491,7 +1488,6 @@ isdn_ioctl(struct file *file, uint cmd, ulong arg)
 				return -EFAULT;
 			return isdn_net_force_hangup(name);
 			break;
-#endif                          /* CONFIG_NETDEVICES */
 		case IIOCSETVER:
 			dev->net_verbose = arg;
 			printk(KERN_INFO "isdn: Verbose-Level is %d\n", dev->net_verbose);

Re: [PATCH 6/9] virtio_mmio: Cast &resources[1].start to ‘unsigned int *’ to rid compiler warning

[Pawel Moll]

On Mon, 2012-11-05 at 08:21 +0000, Lee Jones wrote:
> > >  	processed = sscanf(str, "@%lli:%u%n:%d%n",
> > > -			&base, &resources[1].start, &consumed,
> > > +			&base, (unsigned int *)&resources[1].start, &consumed,
> > >  			&vm_cmdline_id, &consumed);
> > 
> > This suppresses a valid warning, leaving our code no less wrong than
> > before.  But now it's silently wrong!
> > 
> > Do you want to try again?
> 
> For sure I'll try again.
> 
> How does `s/%u/%p/` suit?

sscanf doesn't do "%p"... The only reasonable way of fixing this is a
intermediate local variable - will post a patch in a second.

Thanks for spotting this!

Paweł

Re: [PATCH 6/9] virtio_mmio: Cast &resources[1].start to ‘unsigned int *’ to rid compiler warning

[Pawel Moll]

On Mon, 2012-11-05 at 08:21 +0000, Lee Jones wrote:
> > >  	processed = sscanf(str, "@%lli:%u%n:%d%n",
> > > -			&base, &resources[1].start, &consumed,
> > > +			&base, (unsigned int *)&resources[1].start, &consumed,
> > >  			&vm_cmdline_id, &consumed);
> > 
> > This suppresses a valid warning, leaving our code no less wrong than
> > before.  But now it's silently wrong!
> > 
> > Do you want to try again?
> 
> For sure I'll try again.
> 
> How does `s/%u/%p/` suit?

sscanf doesn't do "%p"... The only reasonable way of fixing this is a
intermediate local variable - will post a patch in a second.

Thanks for spotting this!

Paweł


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH] virtio-mmio: Fix irq parsing in command line parameter

[Pawel Moll]

On 64-bit machines resource_size_t is a 64-bit value, while
sscanf() format for this argument was defined as "%u". Fixed
by using an intermediate local value of a known length.

Also added cleaned up the resource creation and adde extra
comments to make the parameters parsing easier to follow.

Reported-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Pawel Moll <pawel.moll@arm.com>
---
 drivers/virtio/virtio_mmio.c |   26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 6b1b7e1..0d08843 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -521,25 +521,35 @@ static int vm_cmdline_set(const char *device,
 	int err;
 	struct resource resources[2] = {};
 	char *str;
-	long long int base;
+	long long int base, size;
+	unsigned int irq;
 	int processed, consumed = 0;
 	struct platform_device *pdev;
 
-	resources[0].flags = IORESOURCE_MEM;
-	resources[1].flags = IORESOURCE_IRQ;
-
-	resources[0].end = memparse(device, &str) - 1;
+	/* Get "size" part of the command line parameter */
+	size = memparse(device, &str) - 1;
 
+	/* Get "@<base>:<irq>[:<id>]" chunks */
 	processed = sscanf(str, "@%lli:%u%n:%d%n",
-			&base, &resources[1].start, &consumed,
+			&base, &irq, &consumed,
 			&vm_cmdline_id, &consumed);
 
+	/*
+	 * sscanf() processes 3 chunks if "<id>" is given, 2 if not;
+	 * also there must be no extra characters after the last
+	 * chunk, so str[consumed] should be '\0'
+	 */
 	if (processed < 2 || processed > 3 || str[consumed])
 		return -EINVAL;
 
+	/* Memory resource */
+	resources[0].flags = IORESOURCE_MEM;
 	resources[0].start = base;
-	resources[0].end += base;
-	resources[1].end = resources[1].start;
+	resources[0].end = base + size;
+
+	/* Interrupt resource */
+	resources[1].flags = IORESOURCE_IRQ;
+	resources[1].start = resources[1].end = irq;
 
 	if (!vm_cmdline_parent_registered) {
 		err = device_register(&vm_cmdline_parent);
-- 
1.7.10.4

[PATCH] virtio-mmio: Fix irq parsing in command line parameter

[Pawel Moll]

On 64-bit machines resource_size_t is a 64-bit value, while
sscanf() format for this argument was defined as "%u". Fixed
by using an intermediate local value of a known length.

Also added cleaned up the resource creation and adde extra
comments to make the parameters parsing easier to follow.

Reported-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Pawel Moll <pawel.moll@arm.com>
---
 drivers/virtio/virtio_mmio.c |   26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 6b1b7e1..0d08843 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -521,25 +521,35 @@ static int vm_cmdline_set(const char *device,
 	int err;
 	struct resource resources[2] = {};
 	char *str;
-	long long int base;
+	long long int base, size;
+	unsigned int irq;
 	int processed, consumed = 0;
 	struct platform_device *pdev;
 
-	resources[0].flags = IORESOURCE_MEM;
-	resources[1].flags = IORESOURCE_IRQ;
-
-	resources[0].end = memparse(device, &str) - 1;
+	/* Get "size" part of the command line parameter */
+	size = memparse(device, &str) - 1;
 
+	/* Get "@<base>:<irq>[:<id>]" chunks */
 	processed = sscanf(str, "@%lli:%u%n:%d%n",
-			&base, &resources[1].start, &consumed,
+			&base, &irq, &consumed,
 			&vm_cmdline_id, &consumed);
 
+	/*
+	 * sscanf() processes 3 chunks if "<id>" is given, 2 if not;
+	 * also there must be no extra characters after the last
+	 * chunk, so str[consumed] should be '\0'
+	 */
 	if (processed < 2 || processed > 3 || str[consumed])
 		return -EINVAL;
 
+	/* Memory resource */
+	resources[0].flags = IORESOURCE_MEM;
 	resources[0].start = base;
-	resources[0].end += base;
-	resources[1].end = resources[1].start;
+	resources[0].end = base + size;
+
+	/* Interrupt resource */
+	resources[1].flags = IORESOURCE_IRQ;
+	resources[1].start = resources[1].end = irq;
 
 	if (!vm_cmdline_parent_registered) {
 		err = device_register(&vm_cmdline_parent);
-- 
1.7.10.4

Re: [PATCH] virtio-mmio: Fix irq parsing in command line parameter

[Lee Jones]

On Mon, 05 Nov 2012, Pawel Moll wrote:

> On 64-bit machines resource_size_t is a 64-bit value, while
> sscanf() format for this argument was defined as "%u". Fixed
> by using an intermediate local value of a known length.
> 
> Also added cleaned up the resource creation and adde extra
> comments to make the parameters parsing easier to follow.
> 
> Reported-by: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> ---
>  drivers/virtio/virtio_mmio.c |   26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)

Resorted to poaching now have we Pawel? ;)

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] virtio-mmio: Fix irq parsing in command line parameter

[Lee Jones]

On Mon, 05 Nov 2012, Pawel Moll wrote:

> On 64-bit machines resource_size_t is a 64-bit value, while
> sscanf() format for this argument was defined as "%u". Fixed
> by using an intermediate local value of a known length.
> 
> Also added cleaned up the resource creation and adde extra
> comments to make the parameters parsing easier to follow.
> 
> Reported-by: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> ---
>  drivers/virtio/virtio_mmio.c |   26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)

Resorted to poaching now have we Pawel? ;)

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio-mmio: Fix irq parsing in command line parameter

[Pawel Moll]

On Mon, 2012-11-05 at 13:44 +0000, Lee Jones wrote:
> On Mon, 05 Nov 2012, Pawel Moll wrote:
> 
> > On 64-bit machines resource_size_t is a 64-bit value, while
> > sscanf() format for this argument was defined as "%u". Fixed
> > by using an intermediate local value of a known length.
> > 
> > Also added cleaned up the resource creation and adde extra
> > comments to make the parameters parsing easier to follow.
> > 
> > Reported-by: Lee Jones <lee.jones@linaro.org>
> > Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> > ---
> >  drivers/virtio/virtio_mmio.c |   26 ++++++++++++++++++--------
> >  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> Resorted to poaching now have we Pawel? ;)

Sure thing - the poached eggs are my firm favourite :-P ;-)

But seriously speaking - it's an old patch (all the comments and
resources creation, the IRQ stuff is a new thing :-), but never had
enough motivation to push it out...

Cheers!

Paweł

Re: [PATCH] virtio-mmio: Fix irq parsing in command line parameter

[Pawel Moll]

On Mon, 2012-11-05 at 13:44 +0000, Lee Jones wrote:
> On Mon, 05 Nov 2012, Pawel Moll wrote:
> 
> > On 64-bit machines resource_size_t is a 64-bit value, while
> > sscanf() format for this argument was defined as "%u". Fixed
> > by using an intermediate local value of a known length.
> > 
> > Also added cleaned up the resource creation and adde extra
> > comments to make the parameters parsing easier to follow.
> > 
> > Reported-by: Lee Jones <lee.jones@linaro.org>
> > Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> > ---
> >  drivers/virtio/virtio_mmio.c |   26 ++++++++++++++++++--------
> >  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> Resorted to poaching now have we Pawel? ;)

Sure thing - the poached eggs are my firm favourite :-P ;-)

But seriously speaking - it's an old patch (all the comments and
resources creation, the IRQ stuff is a new thing :-), but never had
enough motivation to push it out...

Cheers!

Paweł



_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio-mmio: Fix irq parsing in command line parameter

[Lee Jones]

On Mon, 05 Nov 2012, Pawel Moll wrote:

> On Mon, 2012-11-05 at 13:44 +0000, Lee Jones wrote:
> > On Mon, 05 Nov 2012, Pawel Moll wrote:
> > 
> > > On 64-bit machines resource_size_t is a 64-bit value, while
> > > sscanf() format for this argument was defined as "%u". Fixed
> > > by using an intermediate local value of a known length.
> > > 
> > > Also added cleaned up the resource creation and adde extra
> > > comments to make the parameters parsing easier to follow.
> > > 
> > > Reported-by: Lee Jones <lee.jones@linaro.org>
> > > Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> > > ---
> > >  drivers/virtio/virtio_mmio.c |   26 ++++++++++++++++++--------
> > >  1 file changed, 18 insertions(+), 8 deletions(-)
> > 
> > Resorted to poaching now have we Pawel? ;)
> 
> Sure thing - the poached eggs are my firm favourite :-P ;-)
> 
> But seriously speaking - it's an old patch (all the comments and
> resources creation, the IRQ stuff is a new thing :-), but never had
> enough motivation to push it out...

I believe you, many would not. ;)

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] virtio-mmio: Fix irq parsing in command line parameter

[Lee Jones]

On Mon, 05 Nov 2012, Pawel Moll wrote:

> On Mon, 2012-11-05 at 13:44 +0000, Lee Jones wrote:
> > On Mon, 05 Nov 2012, Pawel Moll wrote:
> > 
> > > On 64-bit machines resource_size_t is a 64-bit value, while
> > > sscanf() format for this argument was defined as "%u". Fixed
> > > by using an intermediate local value of a known length.
> > > 
> > > Also added cleaned up the resource creation and adde extra
> > > comments to make the parameters parsing easier to follow.
> > > 
> > > Reported-by: Lee Jones <lee.jones@linaro.org>
> > > Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> > > ---
> > >  drivers/virtio/virtio_mmio.c |   26 ++++++++++++++++++--------
> > >  1 file changed, 18 insertions(+), 8 deletions(-)
> > 
> > Resorted to poaching now have we Pawel? ;)
> 
> Sure thing - the poached eggs are my firm favourite :-P ;-)
> 
> But seriously speaking - it's an old patch (all the comments and
> resources creation, the IRQ stuff is a new thing :-), but never had
> enough motivation to push it out...

I believe you, many would not. ;)

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 8/9] isdn: Remove unused variable causing a compile build warning

[David Miller]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: Text/Plain; charset=iso-8859-7, Size: 2107 bytes --]

From: Lee Jones <lee.jones@linaro.org>
Date: Mon, 5 Nov 2012 09:44:19 +0100

> On Sun, 04 Nov 2012, David Miller wrote:
> 
>> From: Lee Jones <lee.jones@linaro.org>
>> Date: Sun, 4 Nov 2012 11:53:32 +0100
>> 
>> > On Sun, 04 Nov 2012, Paul Bolle wrote:
>> > 
>> >> On Sat, 2012-11-03 at 23:48 +0100, Lee Jones wrote:
>> >> > On Sat, 03 Nov 2012, Paul Bolle wrote:
>> >> > > On Sat, 2012-11-03 at 23:02 +0100, Lee Jones wrote:
>> >> > > > This patch fixes:
>> >> > > > drivers/isdn/i4l/isdn_common.c: In function ¡isdn_ioctl¢:
>> >> > > > drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ¡s¢ [-Wunused-variable]
>> >> > > 
>> >> > > Did you have CONFIG_NETDEVICES not set in this build?
>> >> > 
>> >> > Ah yes, I see it. The function went down further than I thought
>> >> > it did. So the real fix is to ensure 's' is defined inside of
>> >> > some ifdef CONFIG_NETDEVICES guards. 
>> >> 
>> >> What puzzles me is that we only find these "#ifdef CONFIG_NETDEVICES"
>> >> guards in this file and not in isdn_net.c, were all the ioctl commands
>> >> guarded that way seem to be calling into. On first glance that doesn't
>> >> make much sense.
>> >> 
>> >> (Actually the idea of having ISDN without NETDEVICES is a bit puzzling
>> >> too. But there are too many parts of the isdn subsystem that I'm
>> >> unfamiliar with to say whether that can make sense.)
>> > 
>> > I'm in the same position as you Paul. I just noticed the warning so
>> > fixed it following the current way of doing things. Any, more
>> > substantial changes requiring greater knowledge of the subsystem
>> > would have to be done by someone else.
>> 
>> I think the most appropriate thing to do is make CONFIG_ISDN depend
>> upon CONFIG_NETDEVICES in the Kconfig file.
> 
> ... and then remove the CONFIG_NETDEVICES guards in isdn_common.c?
> 
> If that's suitable more suitable I can do that instead?
> 
> Just need a yay or nay and I'll make it happen.

"yay"
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 8/9] isdn: Remove unused variable causing a compile build warning

[David Miller]

From: Lee Jones <lee.jones@linaro.org>
Date: Mon, 5 Nov 2012 09:44:19 +0100

> On Sun, 04 Nov 2012, David Miller wrote:
> 
>> From: Lee Jones <lee.jones@linaro.org>
>> Date: Sun, 4 Nov 2012 11:53:32 +0100
>> 
>> > On Sun, 04 Nov 2012, Paul Bolle wrote:
>> > 
>> >> On Sat, 2012-11-03 at 23:48 +0100, Lee Jones wrote:
>> >> > On Sat, 03 Nov 2012, Paul Bolle wrote:
>> >> > > On Sat, 2012-11-03 at 23:02 +0100, Lee Jones wrote:
>> >> > > > This patch fixes:
>> >> > > > drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’:
>> >> > > > drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable]
>> >> > > 
>> >> > > Did you have CONFIG_NETDEVICES not set in this build?
>> >> > 
>> >> > Ah yes, I see it. The function went down further than I thought
>> >> > it did. So the real fix is to ensure 's' is defined inside of
>> >> > some ifdef CONFIG_NETDEVICES guards. 
>> >> 
>> >> What puzzles me is that we only find these "#ifdef CONFIG_NETDEVICES"
>> >> guards in this file and not in isdn_net.c, were all the ioctl commands
>> >> guarded that way seem to be calling into. On first glance that doesn't
>> >> make much sense.
>> >> 
>> >> (Actually the idea of having ISDN without NETDEVICES is a bit puzzling
>> >> too. But there are too many parts of the isdn subsystem that I'm
>> >> unfamiliar with to say whether that can make sense.)
>> > 
>> > I'm in the same position as you Paul. I just noticed the warning so
>> > fixed it following the current way of doing things. Any, more
>> > substantial changes requiring greater knowledge of the subsystem
>> > would have to be done by someone else.
>> 
>> I think the most appropriate thing to do is make CONFIG_ISDN depend
>> upon CONFIG_NETDEVICES in the Kconfig file.
> 
> ... and then remove the CONFIG_NETDEVICES guards in isdn_common.c?
> 
> If that's suitable more suitable I can do that instead?
> 
> Just need a yay or nay and I'll make it happen.

"yay"

Re: [PATCH 7/9] quota: Use the pre-processor to compile out quotactl_cmd_write when !CONFIG_BLOCK

[Jan Kara]

On Sat 03-11-12 23:02:28, Lee Jones wrote:
> quotactl_cmd_write() is only ever invoked when BLOCK is configured. When
> !CONFIG_BLOCK, the build warning below is displayed. Let's fix that.
> 
> fs/quota/quota.c:311:12: warning: ‘quotactl_cmd_write’ defined but not used [-Wunused-function]
  Looks good. I've added the patch to my tree.

								Honza

> 
> Cc: Jan Kara <jack@suse.cz>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  fs/quota/quota.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/quota/quota.c b/fs/quota/quota.c
> index af1661f..c7314f1 100644
> --- a/fs/quota/quota.c
> +++ b/fs/quota/quota.c
> @@ -307,6 +307,8 @@ static int do_quotactl(struct super_block *sb, int type, int cmd, qid_t id,
>  	}
>  }
>  
> +#ifdef CONFIG_BLOCK
> +
>  /* Return 1 if 'cmd' will block on frozen filesystem */
>  static int quotactl_cmd_write(int cmd)
>  {
> @@ -322,6 +324,8 @@ static int quotactl_cmd_write(int cmd)
>  	return 1;
>  }
>  
> +#endif /* CONFIG_BLOCK */
> +
>  /*
>   * look up a superblock on which quota ops will be performed
>   * - use the name of a block device to find the superblock thereon
> -- 
> 1.7.9.5
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 8/9] isdn: Make CONFIG_ISDN depend on CONFIG_NETDEVICES

[David Miller]

From: Lee Jones <lee.jones@linaro.org>
Date: Mon, 5 Nov 2012 11:31:26 +0100

> Does something like look like a better solution?
> 
> Author: Lee Jones <lee.jones@linaro.org>
> Date:   Sat Nov 3 22:06:02 2012 +0100
> 
>     isdn: Make CONFIG_ISDN depend on CONFIG_NETDEVICES

Yes, it looks good, please resubmit it formally.

Re: [PATCH 8/9] isdn: Make CONFIG_ISDN depend on CONFIG_NETDEVICES

[Lee Jones]

On Tue, 06 Nov 2012, David Miller wrote:

> From: Lee Jones <lee.jones@linaro.org>
> Date: Mon, 5 Nov 2012 11:31:26 +0100
> 
> > Does something like look like a better solution?
> > 
> > Author: Lee Jones <lee.jones@linaro.org>
> > Date:   Sat Nov 3 22:06:02 2012 +0100
> > 
> >     isdn: Make CONFIG_ISDN depend on CONFIG_NETDEVICES
> 
> Yes, it looks good, please resubmit it formally.

Done. Thanks for reviewing.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v2] virtio-mmio: Fix irq parsing in the command line

[Pawel Moll]

On 64-bit machines resource_size_t is a 64-bit value, while
sscanf() format for this argument was defined as "%u". Fixed
by using an intermediate local value of a known length.

Also added cleaned up the resource creation and adde extra
comments to make the parameters parsing easier to follow.

Reported-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Pawel Moll <pawel.moll@arm.com>
---
 drivers/virtio/virtio_mmio.c |   24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

Comparing to v1 I've simply "polished" the size assignment
(moved -1 from one place to another) and removed two of the
new comments, as they weren't really useful any more.

diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 6b1b7e1..e807c2a 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -521,25 +521,33 @@ static int vm_cmdline_set(const char *device,
 	int err;
 	struct resource resources[2] = {};
 	char *str;
-	long long int base;
+	long long int base, size;
+	unsigned int irq;
 	int processed, consumed = 0;
 	struct platform_device *pdev;
 
-	resources[0].flags = IORESOURCE_MEM;
-	resources[1].flags = IORESOURCE_IRQ;
-
-	resources[0].end = memparse(device, &str) - 1;
+	/* Consume "size" part of the command line parameter */
+	size = memparse(device, &str);
 
+	/* Get "@<base>:<irq>[:<id>]" chunks */
 	processed = sscanf(str, "@%lli:%u%n:%d%n",
-			&base, &resources[1].start, &consumed,
+			&base, &irq, &consumed,
 			&vm_cmdline_id, &consumed);
 
+	/*
+	 * sscanf() processes 3 chunks if "<id>" is given, 2 if not;
+	 * also there must be no extra characters after the last
+	 * chunk, so str[consumed] should be '\0'
+	 */
 	if (processed < 2 || processed > 3 || str[consumed])
 		return -EINVAL;
 
+	resources[0].flags = IORESOURCE_MEM;
 	resources[0].start = base;
-	resources[0].end += base;
-	resources[1].end = resources[1].start;
+	resources[0].end = base + size - 1;
+
+	resources[1].flags = IORESOURCE_IRQ;
+	resources[1].start = resources[1].end = irq;
 
 	if (!vm_cmdline_parent_registered) {
 		err = device_register(&vm_cmdline_parent);
-- 
1.7.10.4

Re: [PATCH] virtio-mmio: Fix irq parsing in command line parameter

[Rusty Russell]

Lee Jones <lee.jones@linaro.org> writes:
> On Mon, 05 Nov 2012, Pawel Moll wrote:
>
>> On 64-bit machines resource_size_t is a 64-bit value, while
>> sscanf() format for this argument was defined as "%u". Fixed
>> by using an intermediate local value of a known length.
>> 
>> Also added cleaned up the resource creation and adde extra
>> comments to make the parameters parsing easier to follow.
>> 
>> Reported-by: Lee Jones <lee.jones@linaro.org>
>> Signed-off-by: Pawel Moll <pawel.moll@arm.com>
>> ---
>>  drivers/virtio/virtio_mmio.c |   26 ++++++++++++++++++--------
>>  1 file changed, 18 insertions(+), 8 deletions(-)
>
> Resorted to poaching now have we Pawel? ;)

I hope you were joking!  Doing your work for you isn't poaching.

Your correct response was "thanks" followed by 'Tested-by:'.

Cheers,
Rusty.

Re: [PATCH v2] virtio-mmio: Fix irq parsing in the command line

[Rusty Russell]

Pawel Moll <pawel.moll@arm.com> writes:

> On 64-bit machines resource_size_t is a 64-bit value, while
> sscanf() format for this argument was defined as "%u". Fixed
> by using an intermediate local value of a known length.

Actually, on 32-bit machines, too (eg. x86 with PAE).  Otherwise we'd
just change it to %lu.

> +	/* Get "@<base>:<irq>[:<id>]" chunks */
>  	processed = sscanf(str, "@%lli:%u%n:%d%n",
> -			&base, &resources[1].start, &consumed,
> +			&base, &irq, &consumed,
>  			&vm_cmdline_id, &consumed);
>  
> +	/*
> +	 * sscanf() processes 3 chunks if "<id>" is given, 2 if not;
> +	 * also there must be no extra characters after the last
> +	 * chunk, so str[consumed] should be '\0'
> +	 */
>  	if (processed < 2 || processed > 3 || str[consumed])
>  		return -EINVAL;

I would drop the > 3 case.  It's unnecessary, and you're assuming
consumed doesn't add to the count, which may be true but is a documented
sscanf weirdness so I don't like to rely on it.

Thanks,
Rusty.

Re: [PATCH] virtio-mmio: Fix irq parsing in command line parameter

[Lee Jones]

[-- Attachment #1.1: Type: text/plain, Size: 676 bytes --]

> > Resorted to poaching now have we Pawel?

> I hope you were joking!

Yes, of course. I thought that was clearly indicated by the jovial winking
smiley. :)

I realise it wasn't obvious soley by this exchange, but Pawel and I are
actually ol' friends.

> Doing your work for you isn't poaching.

This isn't related to my work, as I have no professional interest in
virtio. In this particular instance I was trying to help out by fixing bugs
uncovered by the use of randconfig, which I'm doing in my own time, as a
hobby. I guess the use my work address clouds this fact, so I apologise for
that.

Sorry for any confusion or offence caused though Rusty. :)

Kind regards,
Lee

[-- Attachment #1.2: Type: text/html, Size: 780 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio-mmio: Fix irq parsing in command line parameter

[Rusty Russell]

Lee Jones <lee.jones@linaro.org> writes:

>> > Resorted to poaching now have we Pawel?
>
>> I hope you were joking!
>
> Yes, of course. I thought that was clearly indicated by the jovial winking
> smiley. :)
>
> I realise it wasn't obvious soley by this exchange, but Pawel and I are
> actually ol' friends.
>
>> Doing your work for you isn't poaching.
>
> This isn't related to my work, as I have no professional interest in
> virtio. In this particular instance I was trying to help out by fixing bugs
> uncovered by the use of randconfig, which I'm doing in my own time, as a
> hobby. I guess the use my work address clouds this fact, so I apologise for
> that.
>
> Sorry for any confusion or offence caused though Rusty. :)
>
> Kind regards,
> Lee

No offence for me.  I just don't want bystanders to believe that there
is some etiquette other than "best patch wins".

Thanks for the clarification!
Rusty.

Re: [PATCH] virtio-mmio: Fix irq parsing in command line parameter

[Rusty Russell]

Lee Jones <lee.jones@linaro.org> writes:

>> > Resorted to poaching now have we Pawel?
>
>> I hope you were joking!
>
> Yes, of course. I thought that was clearly indicated by the jovial winking
> smiley. :)
>
> I realise it wasn't obvious soley by this exchange, but Pawel and I are
> actually ol' friends.
>
>> Doing your work for you isn't poaching.
>
> This isn't related to my work, as I have no professional interest in
> virtio. In this particular instance I was trying to help out by fixing bugs
> uncovered by the use of randconfig, which I'm doing in my own time, as a
> hobby. I guess the use my work address clouds this fact, so I apologise for
> that.
>
> Sorry for any confusion or offence caused though Rusty. :)
>
> Kind regards,
> Lee

No offence for me.  I just don't want bystanders to believe that there
is some etiquette other than "best patch wins".

Thanks for the clarification!
Rusty.

[PATCH v3] virtio-mmio: Fix irq parsing in command line parameter

[Pawel Moll]

When the resource_size_t is 64-bit long, the sscanf() on
the virtio device command line paramter string may return
wrong value because its format was defined as "%u". Fixed
by using an intermediate local value of a known length.

Also added cleaned up the resource creation and added extra
comments to make the parameters parsing easier to follow.

Reported-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Pawel Moll <pawel.moll@arm.com>
---
 drivers/virtio/virtio_mmio.c |   26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 6b1b7e1..9df4578 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -521,25 +521,33 @@ static int vm_cmdline_set(const char *device,
 	int err;
 	struct resource resources[2] = {};
 	char *str;
-	long long int base;
+	long long int base, size;
+	unsigned int irq;
 	int processed, consumed = 0;
 	struct platform_device *pdev;
 
-	resources[0].flags = IORESOURCE_MEM;
-	resources[1].flags = IORESOURCE_IRQ;
-
-	resources[0].end = memparse(device, &str) - 1;
+	/* Consume "size" part of the command line parameter */
+	size = memparse(device, &str);
 
+	/* Get "@<base>:<irq>[:<id>]" chunks */
 	processed = sscanf(str, "@%lli:%u%n:%d%n",
-			&base, &resources[1].start, &consumed,
+			&base, &irq, &consumed,
 			&vm_cmdline_id, &consumed);
 
-	if (processed < 2 || processed > 3 || str[consumed])
+	/*
+	 * sscanf() must processes at least 2 chunks; also there
+	 * must be no extra characters after the last chunk, so
+	 * str[consumed] must be '\0'
+	 */
+	if (processed < 2 || str[consumed])
 		return -EINVAL;
 
+	resources[0].flags = IORESOURCE_MEM;
 	resources[0].start = base;
-	resources[0].end += base;
-	resources[1].end = resources[1].start;
+	resources[0].end = base + size - 1;
+
+	resources[1].flags = IORESOURCE_IRQ;
+	resources[1].start = resources[1].end = irq;
 
 	if (!vm_cmdline_parent_registered) {
 		err = device_register(&vm_cmdline_parent);
-- 
1.7.10.4

Re: [PATCH v3] virtio-mmio: Fix irq parsing in command line parameter

[Rusty Russell]

Pawel Moll <pawel.moll@arm.com> writes:
> When the resource_size_t is 64-bit long, the sscanf() on
> the virtio device command line paramter string may return
> wrong value because its format was defined as "%u". Fixed
> by using an intermediate local value of a known length.
>
> Also added cleaned up the resource creation and added extra
> comments to make the parameters parsing easier to follow.

Applied.

Thanks,
Rusty.