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(-)
This was introduced way back before 2005 and has remained unused for
over 7 years. Let's remove it.
Cc: Russell King <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
---
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
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 <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
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
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 <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
---
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
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 <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
---
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
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 <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
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
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 <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
---
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
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 <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
---
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
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 <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
---
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
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
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
From: Lee Jones <[email protected]>
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 <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>
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.
On Sun, 04 Nov 2012, David Miller wrote:
> From: Lee Jones <[email protected]>
> 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 <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Signed-off-by: Lee Jones <[email protected]>
>
> 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
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 <[email protected]>
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>
> ---
> 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
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
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 <[email protected]>
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>
> ---
> 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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Thanks for reviewing Paul. Here's the result:
Author: Lee Jones <[email protected]>
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 <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
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
Thanks for reviewing Lars. Here's the result:
commit c4f60d00d04abfb730a54a97204873dafc6473ae
Author: Lee Jones <[email protected]>
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 <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
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
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
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 <[email protected]> (maintainer:ARM PORT)
[email protected] (moderated list:ARM SUB-ARCHITECT...)
[email protected] (open list)
> NACK
Any particular reason?
> > Cc: Russell King <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > 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
> > [email protected]
> > 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
From: Lee Jones <[email protected]>
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?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
Lee Jones <[email protected]> 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 <[email protected]>
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>
> ---
> 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.
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 <[email protected]>
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>
> ---
> 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)
> {
> >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 <[email protected]>
> >Cc: [email protected]
> >Signed-off-by: Lee Jones <[email protected]>
> >---
> > 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
On Mon, 05 Nov 2012, Rusty Russell wrote:
> Lee Jones <[email protected]> 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 <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > 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
On Sun, 04 Nov 2012, David Miller wrote:
> From: Lee Jones <[email protected]>
> 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
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
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
Does something like look like a better solution?
Author: Lee Jones <[email protected]>
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 <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
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);
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ł
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 <[email protected]>
Signed-off-by: Pawel Moll <[email protected]>
---
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
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 <[email protected]>
> Signed-off-by: Pawel Moll <[email protected]>
> ---
> 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
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 <[email protected]>
> > Signed-off-by: Pawel Moll <[email protected]>
> > ---
> > 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ł
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 <[email protected]>
> > > Signed-off-by: Pawel Moll <[email protected]>
> > > ---
> > > 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
From: Lee Jones <[email protected]>
Date: Mon, 5 Nov 2012 09:44:19 +0100
> On Sun, 04 Nov 2012, David Miller wrote:
>
>> From: Lee Jones <[email protected]>
>> 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?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
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 <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> 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 <[email protected]>
SUSE Labs, CR
From: Lee Jones <[email protected]>
Date: Mon, 5 Nov 2012 11:31:26 +0100
> Does something like look like a better solution?
>
> Author: Lee Jones <[email protected]>
> 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.
On Tue, 06 Nov 2012, David Miller wrote:
> From: Lee Jones <[email protected]>
> Date: Mon, 5 Nov 2012 11:31:26 +0100
>
> > Does something like look like a better solution?
> >
> > Author: Lee Jones <[email protected]>
> > 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
Lee Jones <[email protected]> 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 <[email protected]>
>> Signed-off-by: Pawel Moll <[email protected]>
>> ---
>> 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.
Lee Jones <[email protected]> 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.