Linux-3.10 ARM randconfig fixes
Subsystems (22)
cover Linux-3.10 ARM randconfig fixes
Hi subsystem maintainers,
These are a few patches left over from doing randconfig tests a couple of weeks ago. Please apply them directly into your trees unless you see problems. All patches can theoretically be seen as bug fixes for 3.10, but they are not critical, so applying them for 3.11 is fine as well.
Arnd
Arnd Bergmann (15):
irqdomain: export irq_domain_add_simple
mtd: omap2: allow bulding as a module drm/nouveau: use mdelay instead of large udelay constants [SCSI] nsp32: use mdelay instead of large udelay constants
hwrng: bcm2835: fix MODULE_LICENSE tag
cpuidle: calxeda: select ARM_CPU_SUSPEND
cpufreq: spear needs cpufreq table
thermal: cpu_cooling: fix stub function
drm: always provide debugfs function prototypes drm/tilcd: select BACKLIGHT_LCD_SUPPORT
iwlegacy: il_pm_ops is only provided for PM_SLEEP [media] davinci: vpfe_capture needs i2c
[media] omap3isp: include linux/mm_types.h
clk: tegra: provide tegra_periph_reset_assert alternative
OF: remove #ifdef from linux/of_platform.h
drivers/char/hw_random/bcm2835-rng.c | 2 +- drivers/cpufreq/Kconfig.arm | 1 + drivers/cpuidle/Kconfig | 1 + drivers/gpu/drm/nouveau/core/engine/disp/dacnv50.c | 3 ++- drivers/gpu/drm/tilcdc/Kconfig | 1 + drivers/media/platform/davinci/Kconfig | 3 +++ drivers/media/platform/omap3isp/ispqueue.h | 1 + drivers/mtd/nand/Kconfig | 2 +- drivers/net/wireless/iwlegacy/common.h | 6 +++--- drivers/scsi/nsp32.c | 2 +- include/drm/drmP.h | 3 +-- include/linux/clk/tegra.h | 5 +++++ include/linux/cpu_cooling.h | 4 ++-- include/linux/of_platform.h | 14 +++----------- kernel/irq/irqdomain.c | 1 + 15 files changed, 27 insertions(+), 22 deletions(-)
Signature · 1 line
1.8.1.2
PATCH 1/15 irqdomain: export irq_domain_add_simple
All other irq_domain_add_* functions are exported already, and apparently this one got left out by mistake, which causes build errors for ARM allmodconfig kernels:
ERROR: "irq_domain_add_simple" [drivers/gpio/gpio-rcar.ko] undefined!
ERROR: "irq_domain_add_simple" [drivers/gpio/gpio-em.ko] undefined!
--- kernel/irq/irqdomain.c | 1 + 1 file changed, 1 insertion(+)
kernel/irq/irqdomain.c +1 -0
@@ -194,6 +194,7 @@ struct irq_domain *irq_domain_add_simple(struct device_node *of_node,/* A linear domain is the default */ return irq_domain_add_linear (of_node, size, ops, host_data); } +EXPORT_SYMBOL_GPL (irq_domain_add_simple);/** * irq_domain_add_legacy() - Allocate and register a legacy revmap irq_domain.
Signature · 1 line
1.8.1.2
PATCH 2/15 mtd: omap2: allow bulding as a module
In reply to Arnd BergmannThe omap2 nand device driver calls into the the elm code, which can be a loadable module, and in that case it cannot be built-in itself. I can see no reason why the omap2 driver cannot also be a module, so let's make the option "tristate" in Kconfig to fix this allmodconfig build error:
ERROR: "elm_config" [drivers/mtd/nand/omap2.ko] undefined!
ERROR: "elm_decode_bch_error_page" [drivers/mtd/nand/omap2.ko] undefined!
--- drivers/mtd/nand/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mtd/nand/Kconfig +1 -1
@@ -95,7 +95,7 @@ config MTD_NAND_OMAP2 config MTD_NAND_OMAP_BCH depends on MTD_NAND && MTD_NAND_OMAP2 && ARCH_OMAP3 - bool "Enable support for hardware BCH error correction" + tristate "Enable support for hardware BCH error correction" default n select BCH select BCH_CONST_PARAMS
Signature · 1 line
1.8.1.2
PATCH 3/15 drm/nouveau: use mdelay instead of large udelay constants
In reply to Arnd BergmannARM cannot handle udelay for more than 2 miliseconds, so we should use mdelay instead for those.
--- drivers/gpu/drm/nouveau/core/engine/disp/dacnv50.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/gpu/drm/nouveau/core/engine/disp/dacnv50.c +2 -1
@@ -51,7 +51,8 @@ nv50_dac_sense(struct nv50_disp_priv *priv, int or, u32 loadval)const u32 doff = (or *0x800 );int load = -EINVAL ;nv_wr32 (priv,0x61a00c + doff,0x00100000 | loadval); -udelay (9500 ); +mdelay (9 ); +udelay (500 );nv_wr32 (priv,0x61a00c + doff,0x80000000 ); load = (nv_rd32 (priv,0x61a00c + doff) &0x38000000 ) >>27 ;nv_wr32 (priv,0x61a00c + doff,0x00000000 );
Signature · 1 line
1.8.1.2
Re: [PATCH 03/15] drm/nouveau: use mdelay instead of large udelay constants
You probably want to patch up mclk_wait in nva3_pm.c as well for now (until this code is ported to HWSQ or PDAEMON), even though it's not used by a lot of people.
Op 01-06-13 00:22, Arnd Bergmann schreef:
Quoted 25 lines
ARM cannot handle udelay for more than 2 miliseconds, so we should use mdelay instead for those.
Signed-off-by Arnd Bergmann <[email protected]>Cc David Airlie <[email protected]>Cc Ben Skeggs <[email protected]>--- drivers/gpu/drm/nouveau/core/engine/disp/dacnv50.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/gpu/drm/nouveau/core/engine/disp/dacnv50.c +2
-1@@ -51,7 +51,8 @@ nv50_dac_sense(struct nv50_disp_priv *priv, int or, u32 loadval)const u32 doff = (or *0x800 );int load = -EINVAL ;nv_wr32 (priv,0x61a00c + doff,0x00100000 | loadval); -udelay (9500 ); +mdelay (9 ); +udelay (500 );nv_wr32 (priv,0x61a00c + doff,0x80000000 ); load = (nv_rd32 (priv,0x61a00c + doff) &0x38000000 ) >>27 ;nv_wr32 (priv,0x61a00c + doff,0x00000000 );
Signature · 4 lines
Roy Spliet BSc. MSc. Student Delft University of Technology http://www.pds.ewi.tudelft.nl/msc/mallocinopencl/
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/15] drm/nouveau: use mdelay instead of large udelay constants
In reply to Arnd BergmannHi,
Quoted 1 line · Arnd Bergmann
ARM cannot handle udelay for more than 2 miliseconds, so we
^^^^
There's l missing here.
Quoted 1 line
should use mdelay instead for those.
Could this be handled inside ARM udelay() instead? Probably most of the delay values are compile-time constants.
A.
PATCH 4/15 nsp32: use mdelay instead of large udelay constants
In reply to Arnd BergmannARM cannot handle udelay for more than 2 miliseconds, so we should use mdelay instead for those.
--- drivers/scsi/nsp32.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/nsp32.c +1 -1
@@ -2899,7 +2899,7 @@ static void nsp32_do_bus_reset(nsp32_hw_data *data)* reset SCSI bus */ nsp32_write1 (base,SCSI_BUS_CONTROL ,BUSCTL_RST ); -udelay (RESET_HOLD_TIME ); +mdelay (RESET_HOLD_TIME /1000 );nsp32_write1 (base,SCSI_BUS_CONTROL ,0 );for (i =0 ; i <5 ; i++) { intrdat =nsp32_read2 (base,IRQ_STATUS );/* dummy read */
Signature · 1 line
1.8.1.2
PATCH 5/15 hwrng: bcm2835: fix MODULE_LICENSE tag
In reply to Arnd BergmannThe MODULE_LICENSE macro invocation must use either "GPL" or "GPL v2", but not "GPLv2" in order to be detected by the module loader.
This fixes the allmodconfig build error:
FATAL: modpost: GPL-incompatible module bcm2835-rng.ko uses GPL-only symbol 'platform_driver_unregister'
--- drivers/char/hw_random/bcm2835-rng.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/char/hw_random/bcm2835-rng.c +1 -1
@@ -110,4 +110,4 @@ module_platform_driver(bcm2835_rng_driver);MODULE_AUTHOR ("Lubomir Rintel <[email protected]>" );MODULE_DESCRIPTION ("BCM2835 Random Number Generator (RNG) driver" ); -MODULE_LICENSE ("GPLv2" ); +MODULE_LICENSE ("GPL v2" );
Signature · 1 line
1.8.1.2
Re: [PATCH 05/15] hwrng: bcm2835: fix MODULE_LICENSE tag
Quoted 23 lines · Arnd Bergmann
The
MODULE_LICENSEmacro invocation must use either "GPL" or "GPL v2", but not "GPLv2" in order to be detected by the module loader.This fixes the allmodconfig build error:
FATAL: modpost: GPL-incompatible module bcm2835-rng.ko uses GPL-only symbol '
platform_driver_unregister'Signed-off-by Arnd Bergmann <[email protected]>Cc Dom Cobley <[email protected]>Cc Lubomir Rintel <[email protected]>Cc Stephen Warren <[email protected]>Cc Matt Mackall <[email protected]>Cc linux-rpi-kernel at lists.infradead.orgCc Herbert Xu <[email protected]>--- drivers/char/hw_random/bcm2835-rng.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/char/hw_random/bcm2835-rng.c +0
-0
drivers/char/hw_random/bcm2835-rng.c module_platform_driver(bcm2835_rng_driver); · line 113
Quoted 5 lines
drivers/char/hw_random/bcm2835-rng.c +1
-1@@ -110,4 +110,4 @@ module_platform_driver(bcm2835_rng_driver);MODULE_AUTHOR ("Lubomir Rintel <[email protected]>" );MODULE_DESCRIPTION ("BCM2835 Random Number Generator (RNG) driver" ); -MODULE_LICENSE ("GPLv2" ); +MODULE_LICENSE ("GPL v2" );
Patch applied. Thanks!
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
PATCH 6/15 cpuidle: calxeda: select ARM_CPU_SUSPEND
In reply to Arnd BergmannLike other ARM specific drivers, this one requires ARM_CPU_SUSPEND, as shown by this linker error:
drivers/built-in.o: In function `calxeda_pwrdown_idle':drivers/cpuidle/cpuidle-calxeda.c:84: undefined reference to cpu_suspend' drivers/cpuidle/cpuidle-calxeda.c:86: undefined reference to cpu_resume'
--- drivers/cpuidle/Kconfig | 1 + 1 file changed, 1 insertion(+)
drivers/cpuidle/Kconfig +1 -0
@@ -36,6 +36,7 @@ if CPU_IDLE config CPU_IDLE_CALXEDA bool "CPU Idle Driver for Calxeda processors" depends on ARCH_HIGHBANK + select ARM_CPU_SUSPEND help Select this to enable cpuidle on Calxeda processors.
Signature · 1 line
1.8.1.2
PATCH 7/15 cpufreq: spear needs cpufreq table
In reply to Arnd BergmannLike a lot of the other cpufreq drivers, this one needs to select CONFIG_CPU_FREQ_TABLE to avoid a build error like
drivers/built-in.o: In function `spear_cpufreq_exit':
spear-cpufreq.c:198: undefined reference to cpufreq_frequency_table_put_attr' drivers/built-in.o: In function spear_cpufreq_verify':
spear-cpufreq.c:35: undefined reference to cpufreq_frequency_table_verify' drivers/built-in.o: In function spear_cpufreq_init':
spear-cpufreq.c:181: undefined reference to cpufreq_frequency_table_cpuinfo' spear-cpufreq.c:187: undefined reference to cpufreq_frequency_table_get_attr' drivers/built-in.o: In function `spear_cpufreq_target':
spear-cpufreq.c:120: undefined reference to cpufreq_frequency_table_target' drivers/built-in.o:(.data+0x5e63c): undefined reference to cpufreq_freq_attr_scaling_available_freqs'
--- drivers/cpufreq/Kconfig.arm | 1 + 1 file changed, 1 insertion(+)
drivers/cpufreq/Kconfig.arm +1 -0
@@ -205,6 +205,7 @@ config ARM_SA1110_CPUFREQ config ARM_SPEAR_CPUFREQ bool "SPEAr CPUFreq support" depends on PLAT_SPEAR + select CPU_FREQ_TABLE default y help This adds the CPUFreq driver support for SPEAr SOCs.
Signature · 1 line
1.8.1.2
Re: [PATCH 07/15] cpufreq: spear needs cpufreq table
Quoted 14 lines · Arnd Bergmann
Like a lot of the other cpufreq drivers, this one needs to select
CONFIG_CPU_FREQ_TABLEto avoid a build error like
drivers/built-in.o: In function `spear_cpufreq_exit':
spear-cpufreq.c:198: undefined reference tocpufreq_frequency_table_put_attr' drivers/built-in.o: In functionspear_cpufreq_verify':
spear-cpufreq.c:35: undefined reference tocpufreq_frequency_table_verify' drivers/built-in.o: In functionspear_cpufreq_init':
spear-cpufreq.c:181: undefined reference tocpufreq_frequency_table_cpuinfo' spear-cpufreq.c:187: undefined reference tocpufreq_frequency_table_get_attr'drivers/built-in.o: In function `spear_cpufreq_target':
spear-cpufreq.c:120: undefined reference tocpufreq_frequency_table_target' drivers/built-in.o:(.data+0x5e63c): undefined reference tocpufreq_freq_attr_scaling_available_freqs'
Quoted 8 lines
Signed-off-by Arnd Bergmann <[email protected]>Cc Rafael J. Wysocki <[email protected]>Cc Viresh Kumar <[email protected]>--- drivers/cpufreq/Kconfig.arm | 1 + 1 file changed, 1 insertion(+)
BTW, you want me to apply this one or take it through arm-soc tree?
Re: [PATCH 07/15] cpufreq: spear needs cpufreq table
Quoted 24 lines · On Saturday 01 June 2013 07:28:01 Viresh Kumar wrote:
Quoted 14 lines · Arnd Bergmann
Like a lot of the other cpufreq drivers, this one needs to select
CONFIG_CPU_FREQ_TABLEto avoid a build error like
drivers/built-in.o: In function `spear_cpufreq_exit':
spear-cpufreq.c:198: undefined reference tocpufreq_frequency_table_put_attr' drivers/built-in.o: In functionspear_cpufreq_verify':
spear-cpufreq.c:35: undefined reference tocpufreq_frequency_table_verify' drivers/built-in.o: In functionspear_cpufreq_init':
spear-cpufreq.c:181: undefined reference tocpufreq_frequency_table_cpuinfo' spear-cpufreq.c:187: undefined reference tocpufreq_frequency_table_get_attr'drivers/built-in.o: In function `spear_cpufreq_target':
spear-cpufreq.c:120: undefined reference tocpufreq_frequency_table_target' drivers/built-in.o:(.data+0x5e63c): undefined reference tocpufreq_freq_attr_scaling_available_freqs'Quoted 8 lines
Signed-off-by Arnd Bergmann <[email protected]>Cc Rafael J. Wysocki <[email protected]>Cc Viresh Kumar <[email protected]>--- drivers/cpufreq/Kconfig.arm | 1 + 1 file changed, 1 insertion(+)
Quoted 3 lines
Acked-by Viresh Kumar <[email protected]>BTW, you want me to apply this one or take it through arm-soc tree?
As I said in the introductory mail, I would prefer it if you can pick it up. Thanks!
Arnd
Re: [PATCH 07/15] cpufreq: spear needs cpufreq table
Quoted 25 lines · Arnd Bergmann
Quoted 24 lines · On Saturday 01 June 2013 07:28:01 Viresh Kumar wrote:
Quoted 22 lines · Arnd Bergmann
Like a lot of the other cpufreq drivers, this one needs to select
CONFIG_CPU_FREQ_TABLEto avoid a build error like
drivers/built-in.o: In function `spear_cpufreq_exit':
spear-cpufreq.c:198: undefined reference tocpufreq_frequency_table_put_attr' drivers/built-in.o: In functionspear_cpufreq_verify':
spear-cpufreq.c:35: undefined reference tocpufreq_frequency_table_verify' drivers/built-in.o: In functionspear_cpufreq_init':
spear-cpufreq.c:181: undefined reference tocpufreq_frequency_table_cpuinfo' spear-cpufreq.c:187: undefined reference tocpufreq_frequency_table_get_attr'drivers/built-in.o: In function `spear_cpufreq_target':
spear-cpufreq.c:120: undefined reference tocpufreq_frequency_table_target' drivers/built-in.o:(.data+0x5e63c): undefined reference tocpufreq_freq_attr_scaling_available_freqs'Signed-off-by Arnd Bergmann <[email protected]>Cc Rafael J. Wysocki <[email protected]>Cc Viresh Kumar <[email protected]>--- drivers/cpufreq/Kconfig.arm | 1 + 1 file changed, 1 insertion(+)
Quoted 6 lines
Quoted 3 lines
Acked-by Viresh Kumar <[email protected]>BTW, you want me to apply this one or take it through arm-soc tree?
As I said in the introductory mail, I would prefer it if you can pick it up. Thanks!
I saw that mail later :(
Applied to cpufreq-next. Thanks.
PATCH 7/15 cpufreq: spear needs cpufreq table
In reply to Arnd BergmannQuoted 3 lines · Arnd Bergmann
Like a lot of the other cpufreq drivers, this one needs to select
CONFIG_CPU_FREQ_TABLEto avoid a build error like
Indeed. I found the same thing while playing with Kirkwood's.
@Andrew, I have a patch sitting here, but feel free to send it if you want.
Signature · 1 line
Ezequiel
PATCH 7/15 cpufreq: spear needs cpufreq table
Quoted 8 lines · Ezequiel Garcia
Quoted 3 lines · Arnd Bergmann
Like a lot of the other cpufreq drivers, this one needs to select
CONFIG_CPU_FREQ_TABLEto avoid a build error likeIndeed. I found the same thing while playing with Kirkwood's.
@Andrew, I have a patch sitting here, but feel free to send it if you want.
Hi Ezequiel
Please send your patch.
Thanks
Andrew
PATCH 8/15 thermal: cpu_cooling: fix stub function
In reply to Arnd BergmannThe function stub for cpufreq_cooling_get_level introduced in 57df81069 "Thermal: exynos: fix cooling state translation" is not syntactically correct C and needs to be fixed to avoid this error:
In file included from drivers/thermal/db8500_thermal.c:20:0:
include/linux/cpu_cooling.h: In function 'cpufreq_cooling_get_level':include/linux/cpu_cooling.h:57:1:
error: parameter name omitted unsigned long cpufreq_cooling_get_level(unsigned int, unsigned int) ^ include/linux/cpu_cooling.h:57:1: error: parameter name omitted
--- include/linux/cpu_cooling.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
include/linux/cpu_cooling.h +2 -2
@@ -41,7 +41,7 @@ cpufreq_cooling_register(const struct cpumask *clip_cpus);*/ void cpufreq_cooling_unregister (struct thermal_cooling_device *cdev); -unsigned long cpufreq_cooling_get_level (unsigned int ,unsigned int ); +unsigned long cpufreq_cooling_get_level (unsigned int cpu,unsigned int freq);#else /* !CONFIG_CPU_THERMAL */ static inline struct thermal_cooling_device *cpufreq_cooling_register (const struct cpumask *clip_cpus)
@@ -54,7 +54,7 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)return ; }static inline -unsigned long cpufreq_cooling_get_level (unsigned int ,unsigned int ) +unsigned long cpufreq_cooling_get_level (unsigned int cpu,unsigned int freq) {return THERMAL_CSTATE_INVALID ; }
Signature · 1 line
1.8.1.2
Re: [PATCH 08/15] thermal: cpu_cooling: fix stub function
Arnd,
Quoted 5 lines · Arnd Bergmann
The function stub for
cpufreq_cooling_get_levelintroduced in57df81069"Thermal: exynos: fix cooling state translation" is not syntactically correct C and needs to be fixed to avoid this error:
Quoted 8 lines
In file included from
drivers/thermal/db8500_thermal.c:20:0:
include/linux/cpu_cooling.h: In function 'cpufreq_cooling_get_level':include/linux/cpu_cooling.h:57:1:
error: parameter name omitted unsigned longcpufreq_cooling_get_level(unsigned int, unsigned int)^include/linux/cpu_cooling.h:57:1: error: parameter name omittedSigned-off-by Arnd Bergmann <[email protected]>Acked-by Eduardo Valentin <[email protected]>
What is the status of this patch?
I believe Rui is supposed to queue this one right?
Quoted 30 lines
Cc Eduardo Valentin <[email protected]>Cc Zhang Rui <[email protected]>Cc Amit Daniel kachhap <[email protected]>--- include/linux/cpu_cooling.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)include/linux/cpu_cooling.h +2
-2@@ -41,7 +41,7 @@ cpufreq_cooling_register(const struct cpumask *clip_cpus);*/ void cpufreq_cooling_unregister (struct thermal_cooling_device *cdev); -unsigned long cpufreq_cooling_get_level (unsigned int ,unsigned int ); +unsigned long cpufreq_cooling_get_level (unsigned int cpu,unsigned int freq);#else /* !CONFIG_CPU_THERMAL */ static inline struct thermal_cooling_device *cpufreq_cooling_register (const struct cpumask *clip_cpus)@@ -54,7 +54,7 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)return ; }static inline -unsigned long cpufreq_cooling_get_level (unsigned int ,unsigned int ) +unsigned long cpufreq_cooling_get_level (unsigned int cpu,unsigned int freq) {return THERMAL_CSTATE_INVALID ; }
Signature · 11 lines
You have got to be excited about what you are doing. (L. Lamport) Eduardo Valentin -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 295 bytes Desc: OpenPGP digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130619/bf87a86a/attachment.sig>
Re: [PATCH 08/15] thermal: cpu_cooling: fix stub function
Quoted 12 lines · Eduardo Valentin
Quoted 11 lines · Arnd Bergmann
The function stub for
cpufreq_cooling_get_levelintroduced in57df81069"Thermal: exynos: fix cooling state translation" is not syntactically correct C and needs to be fixed to avoid this error:In file included from
drivers/thermal/db8500_thermal.c:20:0:
include/linux/cpu_cooling.h: In function 'cpufreq_cooling_get_level':include/linux/cpu_cooling.h:57:1:
error: parameter name omitted unsigned longcpufreq_cooling_get_level(unsigned int, unsigned int)^include/linux/cpu_cooling.h:57:1: error: parameter name omitted
Quoted 8 lines
Quoted 2 lines
Signed-off-by Arnd Bergmann <[email protected]>Acked-by Eduardo Valentin <[email protected]>What is the status of this patch?
I believe Rui is supposed to queue this one right?
Yes, that would be good. 3.10 is broken without the patch.
Arnd
PATCH 9/15 drm: always provide debugfs function prototypes
In reply to Arnd BergmannIt is generally considered bad style to enclose function prototypes in header files in #ifdef. This case illustrates why that is:
The tegra host1x driver calls into the debugfs functions if CONFIG_DEBUG_FS is enabled, but that code is otherwise already discarded by the compiler, so leaving the prototype in place actually makes everything work.
drivers/gpu/host1x/drm/dc.c: In function 'tegra_dc_debugfs_init':drivers/gpu/host1x/drm/dc.c:1004:2: error: implicit declaration of function 'drm_debugfs_create_files' [-Werror=implicit-function-declaration]drivers/gpu/host1x/drm/dc.c: In function 'tegra_dc_debugfs_exit': drivers/gpu/host1x/drm/dc.c:1026:2: error: implicit declaration of function 'drm_debugfs_remove_files' [-Werror=implicit-function-declaration]
--- include/drm/drmP.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
include/drm/drmP.h +1 -2
@@ -1551,7 +1551,7 @@ extern int drm_proc_init(struct drm_minor *minor, struct proc_dir_entry *root);extern int drm_proc_cleanup (struct drm_minor *minor,struct proc_dir_entry *root);/* Debugfs support */ -#if defined(CONFIG_DEBUG_FS ) +extern int drm_debugfs_init (struct drm_minor *minor,int minor_id,struct dentry *root); extern int drm_debugfs_create_files(struct drm_info_list *files, int count,
@@ -1559,7 +1559,6 @@ extern int drm_debugfs_create_files(struct drm_info_list *files, int count,extern int drm_debugfs_remove_files (struct drm_info_list *files,int count,struct drm_minor *minor);extern int drm_debugfs_cleanup (struct drm_minor *minor); -#endif /* Info file support */ extern int drm_name_info (struct seq_file *m,void *data);
Signature · 1 line
1.8.1.2
PATCH 10/15 drm/tilcd: select BACKLIGHT_LCD_SUPPORT
In reply to Arnd BergmannThe dependecies for BACKLIGHT_CLASS_DEVICE are defined a bit strange, but it seems one has to always select both BACKLIGHT_CLASS_DEVICE and BACKLIGHT_LCD_SUPPORT to avoid this error:
drivers/gpu/drm/tilcdc/tilcdc_panel.c:396:
undefined reference to `of_find_backlight_by_node'
--- drivers/gpu/drm/tilcdc/Kconfig | 1 + 1 file changed, 1 insertion(+)
drivers/gpu/drm/tilcdc/Kconfig +1 -0
@@ -6,6 +6,7 @@ config DRM_TILCDC select DRM_GEM_CMA_HELPER select VIDEOMODE_HELPERS select BACKLIGHT_CLASS_DEVICE + select BACKLIGHT_LCD_SUPPORT help Choose this option if you have an TI SoC with LCDC display controller, for example AM33xx in beagle-bone, DA8xx, or
Signature · 1 line
1.8.1.2
PATCH 11/15 iwlegacy: il_pm_ops is only provided for PM_SLEEP
In reply to Arnd BergmannThis makes sure the conditionals for the declaration match the definition, so we provide the alternative
#else
#define IL_LEGACY_PM_OPS NULL
#endif
in the correct cases. As suggested by Borislav Petkov, this removes the comments for the #else and #endif lines.
drivers/built-in.o:(.data+0x57974): undefined reference to `il_pm_ops'
--- drivers/net/wireless/iwlegacy/common.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/net/wireless/iwlegacy/common.h +3 -3
@@ -1832,14 +1832,14 @@ u32 il_usecs_to_beacons(struct il_priv *il, u32 usec, u32 beacon_interval);__le32 il_add_beacon_time (struct il_priv *il,u32 base,u32 addon,u32 beacon_interval); -#ifdef CONFIG_PM +#ifdef CONFIG_PM_SLEEP extern const struct dev_pm_ops il_pm_ops;#define IL_LEGACY_PM_OPS (&il_pm_ops) -#else /* !CONFIG_PM */ +#else -#define IL_LEGACY_PM_OPS NULL +#define#endif /* !CONFIG_PM */
Signature · 1 line
1.8.1.2
Re: [PATCH 11/15] iwlegacy: il_pm_ops is only provided for PM_SLEEP
Hi Arnd,
Quoted 9 lines · Arnd Bergmann
This makes sure the conditionals for the declaration match the definition, so we provide the alternative
#else
#defineIL_LEGACY_PM_OPSNULL
#endifin the correct cases. As suggested by Borislav Petkov, this removes the comments for the #else and #endif lines.
You seem to have missed the #endif line.
Quoted 21 lines
drivers/built-in.o:(.data+0x57974): undefined reference to `il_pm_ops'
Cc Johannes Berg <[email protected]>Cc John W. Linville <[email protected]>Cc Borislav Petkov <[email protected]>Signed-off-by Arnd Bergmann <[email protected]>--- drivers/net/wireless/iwlegacy/common.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)drivers/net/wireless/iwlegacy/common.h +1
-1@@ -1832,14 +1832,14 @@ u32 il_usecs_to_beacons(struct il_priv *il, u32 usec, u32 beacon_interval);__le32 il_add_beacon_time (struct il_priv *il,u32 base,u32 addon,u32 beacon_interval); -#ifdef CONFIG_PM +#ifdef CONFIG_PM_SLEEP extern const struct dev_pm_ops il_pm_ops;
drivers/net/wireless/iwlegacy/common.h u32 il_usecs_to_beacons(struct il_priv *il, u32 usec, u32 beacon_interval); · line 1844
Quoted 10 lines
drivers/net/wireless/iwlegacy/common.h +2
-2#define IL_LEGACY_PM_OPS (&il_pm_ops) -#else /* !CONFIG_PM */ +#else -#define IL_LEGACY_PM_OPS NULL +#define#endif /* !CONFIG_PM */
This should not be "!CONFIG_PM" any more.
Brian
Re: [PATCH 11/15] iwlegacy: il_pm_ops is only provided for PM_SLEEP
Quoted 4 lines · On Friday 31 May 2013 22:43:23 Brian Norris wrote:
Quoted 3 lines · Arnd Bergmann
This makes sure the conditionals for the declaration match the definition, so we provide the alternative
Quoted 8 lines
Quoted 6 lines
#else
#defineIL_LEGACY_PM_OPSNULL
#endifin the correct cases. As suggested by Borislav Petkov, this removes the comments for the #else and #endif lines.
You seem to have missed the #endif line.
Yes, I screwed it up with a last minute change, sorry about this. Will follow up with a new one.
Arnd
PATCH v2 1/1 iwlegacy: il_pm_ops is only provided for PM_SLEEP
In reply to Brian NorrisThis makes sure the conditionals for the declaration match the definition, so we provide the alternative
#else
#define IL_LEGACY_PM_OPS NULL
#endif
in the correct cases. As suggested by Borislav Petkov, this removes the comments for the #else and #endif lines.
drivers/built-in.o:(.data+0x57974): undefined reference to `il_pm_ops'
---
v2: Fixed the obvious bug pointed out by Brian Norris, and actually
build-tested the new version.
drivers/net/wireless/iwlegacy/common.h +3 -3
@@ -1832,16 +1832,16 @@ u32 il_usecs_to_beacons(struct il_priv *il, u32 usec, u32 beacon_interval);__le32 il_add_beacon_time (struct il_priv *il,u32 base,u32 addon,u32 beacon_interval); -#ifdef CONFIG_PM +#ifdef CONFIG_PM_SLEEP extern const struct dev_pm_ops il_pm_ops;#define IL_LEGACY_PM_OPS (&il_pm_ops) -#else /* !CONFIG_PM */ +#else #define IL_LEGACY_PM_OPS NULL -#endif /* !CONFIG_PM */ +#endif /***************************************************** * Error Handling Debugging
PATCH 12/15 davinci: vpfe_capture needs i2c
In reply to Arnd BergmannThe vpfe_capture driver is implicitly built for three of the davinci capture drivers but depends on i2c, so we need to add the dependency in Kconfig for each driver using this.
drivers/media/platform/davinci/vpfe_capture.c: In function 'vpfe_probe':
drivers/media/platform/davinci/vpfe_capture.c:1934:2: error: implicit declaration of function 'i2c_get_adapter' [-Werror=implicit-function-declaration] drivers/media/platform/davinci/vpfe_capture.c:1934:11: warning: assignment makes pointer from integer without a cast [enabled by default]
--- drivers/media/platform/davinci/Kconfig | 3 +++ 1 file changed, 3 insertions(+)
drivers/media/platform/davinci/Kconfig +3 -0
@@ -27,6 +27,7 @@ config VIDEO_DAVINCI_VPIF_CAPTURE config VIDEO_DM6446_CCDC tristate "TI DM6446 CCDC video capture driver" depends on VIDEO_V4L2 && (ARCH_DAVINCI || ARCH_OMAP3) + depends on I2C select VIDEOBUF_DMA_CONTIG help Enables DaVinci CCD hw module. DaVinci CCDC hw interfaces
@@ -41,6 +42,7 @@ config VIDEO_DM6446_CCDC config VIDEO_DM355_CCDC tristate "TI DM355 CCDC video capture driver" depends on VIDEO_V4L2 && ARCH_DAVINCI + depends on I2C select VIDEOBUF_DMA_CONTIG help Enables DM355 CCD hw module. DM355 CCDC hw interfaces
@@ -55,6 +57,7 @@ config VIDEO_DM355_CCDC config VIDEO_DM365_ISIF tristate "TI DM365 ISIF video capture driver" depends on VIDEO_V4L2 && ARCH_DAVINCI + depends on I2C select VIDEOBUF_DMA_CONTIG help Enables ISIF hw module. This is the hardware module for
Signature · 1 line
1.8.1.2
PATCH 13/15 omap3isp: include linux/mm_types.h
In reply to Arnd BergmannThe ispqueue.h file uses vm_flags_t, which is defined in linux/mm_types.h, so we must include that header in order to build in all configurations.
--- drivers/media/platform/omap3isp/ispqueue.h | 1 + 1 file changed, 1 insertion(+)
drivers/media/platform/omap3isp/ispqueue.h +1 -0
@@ -31,6 +31,7 @@#include <linux/mutex.h> #include <linux/videodev2.h> #include <linux/wait.h> +#include <linux/mm_types.h> struct isp_video_queue ;struct page ;
Signature · 1 line
1.8.1.2
Re: [PATCH 13/15] [media] omap3isp: include linux/mm_types.h
Hi Arnd,
Thank you for the patch.
Quoted 9 lines · On Saturday 01 June 2013 00:22:50 Arnd Bergmann wrote:
The
ispqueue.hfile usesvm_flags_t, which is defined inlinux/mm_types.h, so we must include that header in order to build in all configurations.Signed-off-by Arnd Bergmann <[email protected]>Cc Mauro Carvalho Chehab <[email protected]>Cc linux-media at vger.kernel.orgCc Konstantin Khlebnikov <[email protected]>Cc Laurent Pinchart <[email protected]>
(with a minor nitpick below)
drivers/media/platform/omap3isp/ispqueue.h line 34
Quoted 13 lines
--- drivers/media/platform/omap3isp/ispqueue.h | 1 + 1 file changed, 1 insertion(+)(quoted diff) +0
-0
b/drivers/media/platform/omap3isp/ispqueue.hindex 908dfd7..e6e720c 100644drivers/media/platform/omap3isp/ispqueue.h +1
-0@@ -31,6 +31,7 @@#include <linux/mutex.h> #include <linux/videodev2.h> #include <linux/wait.h> +#include <linux/mm_types.h>
Could you please make sure the headers are sorted alphabetically ?
Would you like me to take the patch in my tree ? If so I'll sort the headers myself.
Quoted 2 lines
drivers/media/platform/omap3isp/ispqueue.h +0
-0struct isp_video_queue ;struct page ;
Signature · 3 lines
Regards, Laurent Pinchart
Re: [PATCH 13/15] [media] omap3isp: include linux/mm_types.h
Quoted 11 lines · Laurent Pinchart
drivers/media/platform/omap3isp/ispqueue.h line 34
Quoted 9 lines
(quoted diff) +0
-0
b/drivers/media/platform/omap3isp/ispqueue.hindex 908dfd7..e6e720c 100644drivers/media/platform/omap3isp/ispqueue.h +1
-0@@ -31,6 +31,7 @@#include <linux/mutex.h> #include <linux/videodev2.h> #include <linux/wait.h> +#include <linux/mm_types.h> Could you please make sure the headers are sorted alphabetically ?
I normally do. Sorry for missing it this time.
Quoted 2 lines
Would you like me to take the patch in my tree ? If so I'll sort the headers myself.
Yes, that would be nice, thanks!
Arnd
PATCH 14/15 clk: tegra: provide tegra_periph_reset_assert alternative
In reply to Arnd BergmannWe have some tegra device drivers that are written to be platform independent but still use the tegra specific tegra_periph_reset_assert function. In order to build and link them without errors, this provides a static inline version of these functions that does nothing when Tegra support is disabled.
--- include/linux/clk/tegra.h | 5 +++++ 1 file changed, 5 insertions(+)
include/linux/clk/tegra.h +5 -0
@@ -120,8 +120,13 @@ static inline void tegra_cpu_clock_resume(void) }#endif +#ifdef ARCH_TEGRA void tegra_periph_reset_deassert (struct clk *c);void tegra_periph_reset_assert (struct clk *c); +#else +static inline void tegra_periph_reset_deassert (struct clk *c) {} +static inline void tegra_periph_reset_assert (struct clk *c) {} +#endif void tegra_clocks_init (void );void tegra_clocks_apply_init_table (void );
Signature · 1 line
1.8.1.2
PATCH 15/15 OF: remove #ifdef from linux/of_platform.h
In reply to Arnd BergmannA lot of code uses the functions from of_platform.h when built for devicetree-enabled platforms but can also be built without them. In order to avoid using #ifdef everywhere in drivers, this makes all the function declarations visible, which means we can use "if (IS_ENABLED(CONFIG_OF))" in driver code and get build coverage over the code but let the compiler drop the reference in the object code.
--- include/linux/of_platform.h | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-)
include/linux/of_platform.h +3 -11
@@ -13,8 +13,6 @@#include <linux/device.h> #include <linux/mod_devicetable.h> - -#ifdef CONFIG_OF_DEVICE #include <linux/pm.h> #include <linux/of_device.h> #include <linux/platform_device.h>
@@ -82,7 +80,6 @@ extern struct platform_device *of_device_alloc(struct device_node *np, struct device *parent);extern struct platform_device *of_find_device_by_node (struct device_node *np); -#ifdef CONFIG_OF_ADDRESS /* device reg helpers depend on OF_ADDRESS */ /* Platform devices and busses creation */ extern struct platform_device *of_platform_device_create(struct device_node *np, const char *bus_id,
@@ -91,17 +88,12 @@ extern struct platform_device *of_platform_device_create(struct device_node *np,extern int of_platform_bus_probe (struct device_node *root,const struct of_device_id *matches,struct device *parent); +#ifdef CONFIG_OF_ADDRESS extern int of_platform_populate (struct device_node *root,const struct of_device_id *matches,const struct of_dev_auxdata *lookup,struct device *parent); -#endif /* CONFIG_OF_ADDRESS */ - -#endif /* CONFIG_OF_DEVICE */ - -#if !defined(CONFIG_OF_ADDRESS ) -struct of_dev_auxdata ; -struct device_node ; +#else static inline int of_platform_populate(struct device_node *root, const struct of_device_id *matches, const struct of_dev_auxdata *lookup,
@@ -109,6 +101,6 @@ static inline int of_platform_populate(struct device_node *root, {return -ENODEV ; } -#endif /* !CONFIG_OF_ADDRESS */ +#endif #endif /* _LINUX_OF_PLATFORM_H */
Signature · 1 line
1.8.1.2
Re: [PATCH 15/15] OF: remove #ifdef from linux/of_platform.h
Quoted 11 lines · Arnd Bergmann
A lot of code uses the functions from
of_platform.hwhen built for devicetree-enabled platforms but can also be built without them. In order to avoid using #ifdef everywhere in drivers, this makes all the function declarations visible, which means we can use "if (IS_ENABLED(CONFIG_OF))" in driver code and get build coverage over the code but let the compiler drop the reference in the object code.Signed-off-by Arnd Bergmann <[email protected]>Cc Grant Likely <[email protected]>Cc Rob Herring <[email protected]>
I've got a more complete series for 3.11 that removes OF_DEVICE and of_platform_driver completely. I'm waiting on ack from Ben H.
Rob
Quoted 61 lines
--- include/linux/of_platform.h | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-)include/linux/of_platform.h +3
-12@@ -13,8 +13,6 @@#include <linux/device.h> #include <linux/mod_devicetable.h> - -#ifdef CONFIG_OF_DEVICE #include <linux/pm.h> #include <linux/of_device.h> #include <linux/platform_device.h> @@ -82,7 +80,6 @@ extern struct platform_device *of_device_alloc(struct device_node *np, struct device *parent);extern struct platform_device *of_find_device_by_node (struct device_node *np); -#ifdef CONFIG_OF_ADDRESS /* device reg helpers depend on OF_ADDRESS */ /* Platform devices and busses creation */ extern struct platform_device *of_platform_device_create(struct device_node *np, const char *bus_id,@@ -91,17 +88,12 @@ extern struct platform_device *of_platform_device_create(struct device_node *np,extern int of_platform_bus_probe (struct device_node *root,const struct of_device_id *matches,struct device *parent); +#ifdef CONFIG_OF_ADDRESS extern int of_platform_populate (struct device_node *root,const struct of_device_id *matches,const struct of_dev_auxdata *lookup,struct device *parent); -#endif /* CONFIG_OF_ADDRESS */ - -#endif /* CONFIG_OF_DEVICE */ - -#if !defined(CONFIG_OF_ADDRESS ) -struct of_dev_auxdata ; -struct device_node ; +#else static inline int of_platform_populate(struct device_node *root, const struct of_device_id *matches, const struct of_dev_auxdata *lookup,@@ -109,6 +101,6 @@ static inline int of_platform_populate(struct device_node *root, {return -ENODEV ; } -#endif /* !CONFIG_OF_ADDRESS */ +#endif #endif /* _LINUX_OF_PLATFORM_H */ --1.8.1.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH 15/15] OF: remove #ifdef from linux/of_platform.h
Quoted 9 lines · Rob Herring
Quoted 8 lines · Arnd Bergmann
A lot of code uses the functions from
of_platform.hwhen built for devicetree-enabled platforms but can also be built without them. In order to avoid using #ifdef everywhere in drivers, this makes all the function declarations visible, which means we can use "if (IS_ENABLED(CONFIG_OF))" in driver code and get build coverage over the code but let the compiler drop the reference in the object code.
Quoted 6 lines
Quoted 3 lines
Signed-off-by Arnd Bergmann <[email protected]>Cc Grant Likely <[email protected]>Cc Rob Herring <[email protected]>I've got a more complete series for 3.11 that removes
OF_DEVICEandof_platform_drivercompletely. I'm waiting on ack from Ben H.
Ok. Does your series also remove the #ifdef CONFIG_OF part from this header?
Arnd
Re: [PATCH 15/15] OF: remove #ifdef from linux/of_platform.h
Quoted 14 lines · Arnd Bergmann
Quoted 13 lines · Rob Herring
Quoted 11 lines · Arnd Bergmann
A lot of code uses the functions from
of_platform.hwhen built for devicetree-enabled platforms but can also be built without them. In order to avoid using #ifdef everywhere in drivers, this makes all the function declarations visible, which means we can use "if (IS_ENABLED(CONFIG_OF))" in driver code and get build coverage over the code but let the compiler drop the reference in the object code.Signed-off-by Arnd Bergmann <[email protected]>Cc Grant Likely <[email protected]>Cc Rob Herring <[email protected]>
Quoted 5 lines
Quoted 2 lines
I've got a more complete series for 3.11 that removes
OF_DEVICEandof_platform_drivercompletely. I'm waiting on ack from Ben H.Ok. Does your series also remove the #ifdef
CONFIG_OFpart from this header?
No, we still need empty functions. Here is what of_device.h looks like:
BTW, it has your ack.
Rob
Re: [PATCH 15/15] OF: remove #ifdef from linux/of_platform.h
Quoted 6 lines · Rob Herring
No, we still need empty functions. Here is what
of_device.hlooks like:BTW, it has your ack.
Could you add a patch on top that only puts the function declarations inside of #ifdef that don't have an inline wrapper?
It's really annoying to have to change the header file every time one needs to call a function from a driver in the DT-only case.
Arnd
Re: [PATCH 15/15] OF: remove #ifdef from linux/of_platform.h
Quoted 10 lines · Arnd Bergmann
Quoted 6 lines · Rob Herring
No, we still need empty functions. Here is what
of_device.hlooks like:BTW, it has your ack.
Could you add a patch on top that only puts the function declarations inside of #ifdef that don't have an inline wrapper?
I'm confused. You mean that DO have an inline? Like this:
void foo(void);
#ifdef CONFIG_OF
void bar(void);
#else
static inline void bar(void) {}
#endif
Quoted 2 lines
It's really annoying to have to change the header file every time one needs to call a function from a driver in the DT-only case.
The functions without inlines are ones that drivers should not call and should only be called from OF enabled code. That's why we have not done a complete pass of adding inlines for everything.
Rob
Re: [PATCH 15/15] OF: remove #ifdef from linux/of_platform.h
Quoted 14 lines · Rob Herring
Quoted 10 lines · Arnd Bergmann
Quoted 6 lines · Rob Herring
No, we still need empty functions. Here is what
of_device.hlooks like:BTW, it has your ack.
Could you add a patch on top that only puts the function declarations inside of #ifdef that don't have an inline wrapper?
I'm confused. You mean that DO have an inline? Like this:
Quoted 7 lines
void foo(void);
#ifdef
CONFIG_OF
void bar(void);
#else
static inline void bar(void) {}
#endif
Yes, sorry. That's what I meant.
Quoted 6 lines
Quoted 2 lines
It's really annoying to have to change the header file every time one needs to call a function from a driver in the DT-only case.
The functions without inlines are ones that drivers should not call and should only be called from OF enabled code. That's why we have not done a complete pass of adding inlines for everything.
The problem is that it's a bad default. The convention in kernel code is not to hide declarations in #ifdef, for multiple reasons:
* It avoids unnecessary code rebuilds when the symbol changes between two 'make' runs.
* It lets drivers and platform code call the function from dead code without causing a build error, thus improving compile coverage.
* It's much nicer to read when can write your code like
void __init exynos_init_io(struct map_desc *mach_desc, int size)
{
if (IS_ENABLED_(CONFIG_OF) && initial_boot_params)
of_scan_flat_dt(exynos_fdt_map_chipid, NULL);
else
iotable_init(exynos_iodesc, ARRAY_SIZE(exynos_iodesc));
...
}instead of
void __init exynos_init_io(struct map_desc *mach_desc, int size) {
#ifdef CONFIG_OF
if (initial_boot_params)
of_scan_flat_dt(exynos_fdt_map_chipid, NULL);
else
#endif
iotable_init(exynos_iodesc, ARRAY_SIZE(exynos_iodesc));
...
}The first one looks like actual C code, the second is really ugly, and an inline wrapper wouldn't even do the right thing here.
Arnd
Re: [PATCH 15/15] OF: remove #ifdef from linux/of_platform.h
Quoted 65 lines · Arnd Bergmann
Quoted 14 lines · Rob Herring
Quoted 10 lines · Arnd Bergmann
Quoted 6 lines · Rob Herring
No, we still need empty functions. Here is what
of_device.hlooks like:BTW, it has your ack.
Could you add a patch on top that only puts the function declarations inside of #ifdef that don't have an inline wrapper?
I'm confused. You mean that DO have an inline? Like this:
Quoted 7 lines
void foo(void);
#ifdef
CONFIG_OF
void bar(void);
#else
static inline void bar(void) {}
#endifYes, sorry. That's what I meant.
Quoted 6 lines
Quoted 2 lines
It's really annoying to have to change the header file every time one needs to call a function from a driver in the DT-only case.
The functions without inlines are ones that drivers should not call and should only be called from OF enabled code. That's why we have not done a complete pass of adding inlines for everything.
The problem is that it's a bad default. The convention in kernel code is not to hide declarations in #ifdef, for multiple reasons:
* It avoids unnecessary code rebuilds when the symbol changes between two 'make' runs.
* It lets drivers and platform code call the function from dead code without causing a build error, thus improving compile coverage.
* It's much nicer to read when can write your code like
void
__initexynos_init_io(struct map_desc *mach_desc, int size){ if (IS_ENABLED_(CONFIG_OF) && initial_boot_params) of_scan_flat_dt(exynos_fdt_map_chipid, NULL); else iotable_init(exynos_iodesc, ARRAY_SIZE(exynos_iodesc)); ... }instead of
void
__initexynos_init_io(struct map_desc *mach_desc, int size){#ifdef CONFIG_OF if (initial_boot_params) of_scan_flat_dt(exynos_fdt_map_chipid, NULL); else #endif iotable_init(exynos_iodesc, ARRAY_SIZE(exynos_iodesc)); ... }
Quoted 2 lines
The first one looks like actual C code, the second is really ugly, and an inline wrapper wouldn't even do the right thing here.
Right. I get all that. You still have to go add inlines if you want to make:
if (IS_ENABLED(CONFIG_OF))
of_foo();
just be:
of_foo();
There are situations for both and only inlines cover both cases. I don't see a reason we would want to allow the first case and not allow the second case. I am tired of taking patches adding the inlines 1 by 1, so perhaps we need to refactor the OF headers to better separate core infrastructure includes vs. driver only includes if that is really a concern.
Rob
Re: [PATCH 15/15] OF: remove #ifdef from linux/of_platform.h
Quoted 8 lines · On Tuesday 04 June 2013 17:24:51 Rob Herring wrote:
Right. I get all that. You still have to go add inlines if you want to make:
if (
IS_ENABLED(CONFIG_OF))of_foo();just be:
Quoted 8 lines
of_foo();There are situations for both and only inlines cover both cases. I don't see a reason we would want to allow the first case and not allow the second case. I am tired of taking patches adding the inlines 1 by 1, so perhaps we need to refactor the OF headers to better separate core infrastructure includes vs. driver only includes if that is really a concern.
Ok, fair enough. Putting internal declarations into a local header file would be best of course.
Arnd
Re: [PATCH 15/15] OF: remove #ifdef from linux/of_platform.h
In reply to Rob HerringQuoted 55 lines · Rob Herring
Quoted 54 lines · Arnd Bergmann
Quoted 14 lines · Rob Herring
Quoted 10 lines · Arnd Bergmann
Quoted 6 lines · Rob Herring
No, we still need empty functions. Here is what
of_device.hlooks like:BTW, it has your ack.
Could you add a patch on top that only puts the function declarations inside of #ifdef that don't have an inline wrapper?
I'm confused. You mean that DO have an inline? Like this:
Quoted 7 lines
void foo(void);
#ifdef
CONFIG_OF
void bar(void);
#else
static inline void bar(void) {}
#endifYes, sorry. That's what I meant.
Quoted 6 lines
Quoted 2 lines
It's really annoying to have to change the header file every time one needs to call a function from a driver in the DT-only case.
The functions without inlines are ones that drivers should not call and should only be called from OF enabled code. That's why we have not done a complete pass of adding inlines for everything.
The problem is that it's a bad default. The convention in kernel code is not to hide declarations in #ifdef, for multiple reasons:
* It avoids unnecessary code rebuilds when the symbol changes between two 'make' runs.
* It lets drivers and platform code call the function from dead code without causing a build error, thus improving compile coverage.
* It's much nicer to read when can write your code like
void
__initexynos_init_io(struct map_desc *mach_desc, int size){ if (IS_ENABLED_(CONFIG_OF) && initial_boot_params) of_scan_flat_dt(exynos_fdt_map_chipid, NULL); else iotable_init(exynos_iodesc, ARRAY_SIZE(exynos_iodesc)); ... }instead of
Quoted 10 lines
Quoted 10 lines
void
__initexynos_init_io(struct map_desc *mach_desc, int size){#ifdef CONFIG_OF if (initial_boot_params) of_scan_flat_dt(exynos_fdt_map_chipid, NULL); else #endif iotable_init(exynos_iodesc, ARRAY_SIZE(exynos_iodesc)); ... }
A big part of the reason for not having the headers was to discourage code like the above; so instead of having a whole bunch of DT functions inline in a drivers probe hook, they would be collected off in a DT parse function that can all get compiled away as a block.
However, that's the first time I've thought about the code coverage issue and it is a good point.
Quoted 11 lines
Quoted 3 lines
The first one looks like actual C code, the second is really ugly, and an inline wrapper wouldn't even do the right thing here.
Right. I get all that. You still have to go add inlines if you want to make:
if (
IS_ENABLED(CONFIG_OF))of_foo();just be:
Quoted 8 lines
of_foo();There are situations for both and only inlines cover both cases. I don't see a reason we would want to allow the first case and not allow the second case. I am tired of taking patches adding the inlines 1 by 1, so perhaps we need to refactor the OF headers to better separate core infrastructure includes vs. driver only includes if that is really a concern.
I'm fine with that. Attitudes have changed quite a bit in the last few years about DT code in device drivers so it isn't as important to make a hard distinction about when DT functions can be called.
g.