Skip to contentSkip to conversation

mark virt_to_bus/bus_to_virt as __deprecated on i386

PATCH In review Adrian Bunk 9 participants 22 messages Nov 18, 2005 – Nov 21, 2005 Memory technology devices (MTD)

lore
Share & tools
lore

Adrian Bunk

author
#0 lore raw

PATCH 1/1 mark virt_to_bus/bus_to_virt as __deprecated on i386

virt_to_bus/bus_to_virt are long deprecated, mark them as __deprecated on i386.

Signed-off-by Adrian Bunk <[email protected]>

---

This patch was already sent on:
- 12 Nov 2005

include/asm-i386/io.h renamed from include/asm-i386/io.h.old +8 -2
@@ -144,8 +144,14 @@
  *
  * Allow them on x86 for legacy drivers, though.
  */
-#define virt_to_bus virt_to_phys
-#define bus_to_virt phys_to_virt
+static inline unsigned long __deprecated virt_to_bus(volatile void * address)
+{
+	return __pa(address);
+}
+static inline void * __deprecated bus_to_virt(unsigned long address)
+{
+	return __va(address);
+}
 
 /*
  * readX/writeX() are used to access memory mapped devices. On some
#1 lore raw
Quoted 4 lines · Adrian Bunk

virt_to_bus/bus_to_virt are long deprecated, mark them as __deprecated on i386.

Problem is, nobody's fixing these things. There's no point in adding spam to the kernel build unless it actually gets us some action, and I haven't seen any evidence that it does.

Stick it under CONFIG_I_AM_A_DEVELOPER_WHO_HAS_TIME_TO_FIX_STUFF.

author
#2 lore raw
Quoted 11 lines · Andrew Morton
Quoted 4 lines · Adrian Bunk

virt_to_bus/bus_to_virt are long deprecated, mark them as __deprecated on i386.

Problem is, nobody's fixing these things. There's no point in adding spam to the kernel build unless it actually gets us some action, and I haven't seen any evidence that it does.

Stick it under CONFIG_I_AM_A_DEVELOPER_WHO_HAS_TIME_TO_FIX_STUFF.

I'm used to the fact that every single BROKEN_ON_SMP driver generates tons of such warnings that I don't see why these warnings should be any bad...

If you dislike the warnings, you could move the whole __deprecated und a config option.

In the case of virt_to_bus/bus_to_virt I had the hope that e.g. the ATM drivers that seem to have an active maintainer might get fixed.

But I'm not religious regarding this issue as long as you accept my -Werror-implicit-function-declaration patch...

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
#3 lore raw
Quoted 12 lines · Adrian Bunk
Quoted 10 lines · Andrew Morton
Quoted 4 lines · Adrian Bunk

virt_to_bus/bus_to_virt are long deprecated, mark them as __deprecated on i386.

Problem is, nobody's fixing these things. There's no point in adding spam to the kernel build unless it actually gets us some action, and I haven't seen any evidence that it does.

Quoted 5 lines
Quoted 1 line

Stick it under CONFIG_I_AM_A_DEVELOPER_WHO_HAS_TIME_TO_FIX_STUFF.

I'm used to the fact that every single BROKEN_ON_SMP driver generates tons of such warnings that I don't see why these warnings should be any bad...

I frequently (daily) get patches which spit new warnings. Sometimes (~weekly) those warnings indicate real bugs in the patch.

I believe that the main reason for this is that the developers simply don't notice the new warning amongst all the noise.

Quoted 5 lines

If you dislike the warnings, you could move the whole __deprecated und a config option.

In the case of virt_to_bus/bus_to_virt I had the hope that e.g. the ATM drivers that seem to have an active maintainer might get fixed.

That would be good - but perhaps a better approach would be to send pointed emails to the maintainer. Or to merge lameo patches to remove virt_to_bus() so he has to fix it for real ;)

Quoted 2 lines

But I'm not religious regarding this issue as long as you accept my -Werror-implicit-function-declaration patch...

Problem is, I'm the sucker who takes the brunt of that change. It'd be best to fix up the warnings _before_ adding the make-it-break patch.

author
#4 lore raw
Quoted 19 lines · Andrew Morton
Quoted 12 lines · Adrian Bunk
Quoted 10 lines · Andrew Morton
Quoted 4 lines · Adrian Bunk

virt_to_bus/bus_to_virt are long deprecated, mark them as __deprecated on i386.

Problem is, nobody's fixing these things. There's no point in adding spam to the kernel build unless it actually gets us some action, and I haven't seen any evidence that it does.

Quoted 5 lines
Quoted 1 line

Stick it under CONFIG_I_AM_A_DEVELOPER_WHO_HAS_TIME_TO_FIX_STUFF.

I'm used to the fact that every single BROKEN_ON_SMP driver generates tons of such warnings that I don't see why these warnings should be any bad...

Quoted 5 lines

I frequently (daily) get patches which spit new warnings. Sometimes (~weekly) those warnings indicate real bugs in the patch.

I believe that the main reason for this is that the developers simply don't notice the new warning amongst all the noise.

There are few areas in the kernel that spit that many warnings that you might not see new ones .

The developers not noticing the warnings might often be the same developers who send patches that don't compile...

Quoted 9 lines
Quoted 5 lines

If you dislike the warnings, you could move the whole __deprecated und a config option.

In the case of virt_to_bus/bus_to_virt I had the hope that e.g. the ATM drivers that seem to have an active maintainer might get fixed.

That would be good - but perhaps a better approach would be to send pointed emails to the maintainer. Or to merge lameo patches to remove virt_to_bus() so he has to fix it for real ;)

In the case of virt_to_bus/bus_to_virt there are stil many places in the kernel using it, and several of them are well maintained.

IMHO the warnings are the best solution for getting a vast amount fixed, and then it's time to think about the rest.

Quoted 5 lines
Quoted 2 lines

But I'm not religious regarding this issue as long as you accept my -Werror-implicit-function-declaration patch...

Problem is, I'm the sucker who takes the brunt of that change. It'd be best to fix up the warnings _before_ adding the make-it-break patch.

-Werror-implicit-function-declaration doesn't add new warnings, it turns a specific kind of warnings that can indicate nasty runtime errors into compile errors.

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
#5 lore raw
Quoted 10 lines · Adrian Bunk
Quoted 4 lines

I believe that the main reason for this is that the developers simply don't notice the new warning amongst all the noise.

There are few areas in the kernel that spit that many warnings that you might not see new ones .

The developers not noticing the warnings might often be the same developers who send patches that don't compile...

Some architectures generate a lot more warnings than x86.

Quoted 10 lines
Quoted 9 lines
Quoted 5 lines

If you dislike the warnings, you could move the whole __deprecated und a config option.

In the case of virt_to_bus/bus_to_virt I had the hope that e.g. the ATM drivers that seem to have an active maintainer might get fixed.

That would be good - but perhaps a better approach would be to send pointed emails to the maintainer. Or to merge lameo patches to remove virt_to_bus() so he has to fix it for real ;)

Quoted 5 lines

In the case of virt_to_bus/bus_to_virt there are stil many places in the kernel using it, and several of them are well maintained.

IMHO the warnings are the best solution for getting a vast amount fixed, and then it's time to think about the rest.

But the warnings don't *work*. I'm *still* staring at stupid pm_register and intermodule_foo warnings. How long has that been?

Quoted 9 lines
Quoted 5 lines
Quoted 2 lines

But I'm not religious regarding this issue as long as you accept my -Werror-implicit-function-declaration patch...

Problem is, I'm the sucker who takes the brunt of that change. It'd be best to fix up the warnings _before_ adding the make-it-break patch.

-Werror-implicit-function-declaration doesn't add new warnings, it turns a specific kind of warnings that can indicate nasty runtime errors into compile errors.

I know, that's why the patch hurts so much. As I say, we'd be better off fixing up all the warnings we can before turning them into build errors.

#6 lore raw
Quoted 5 lines · Andrew Morton
Quoted 2 lines

IMHO the warnings are the best solution for getting a vast amount fixed, and then it's time to think about the rest.

But the warnings don't *work*. I'm *still* staring at stupid pm_register and intermodule_foo warnings. How long has that been?

Too long. I think the mtd stuff won't ever get fixed until after that function gets removed.

Dave

author
#7 lore raw

PATCH 1/1 build kernel/intermodule.c only when required

Quoted 10 lines · Dave Jones
Quoted 5 lines · Andrew Morton
Quoted 2 lines

IMHO the warnings are the best solution for getting a vast amount fixed, and then it's time to think about the rest.

But the warnings don't *work*. I'm *still* staring at stupid pm_register and intermodule_foo warnings. How long has that been?

Too long. I think the mtd stuff won't ever get fixed until after that function gets removed.

Let's limit the inclusion of kernel/intermodule.c to the users of these drivers.

Quoted 1 line

Dave

cu
Adrian

<-- snip -->

Let's build kernel/intermodule.c only when required.

Signed-off-by Adrian Bunk <[email protected]>

---

 drivers/mtd/chips/Kconfig   |    1 +
 drivers/mtd/devices/Kconfig |    1 +
 init/Kconfig                |    3 +++
 kernel/Makefile             |    3 ++-
 4 files changed, 7 insertions(+), 1 deletion(-)
init/Kconfig renamed from init/Kconfig.old +3 -0
@@ -456,6 +456,9 @@
 	default !SLAB
 	bool
 
+config OBSOLETE_INTERMODULE
+	tristate
+
 menu "Loadable module support"
 
 config MODULES
kernel/Makefile renamed from kernel/Makefile.old +2 -1
@@ -6,10 +6,11 @@
 	    exit.o itimer.o time.o softirq.o resource.o \
 	    sysctl.o capability.o ptrace.o timer.o user.o \
 	    signal.o sys.o kmod.o workqueue.o pid.o \
-	    rcupdate.o intermodule.o extable.o params.o posix-timers.o \
+	    rcupdate.o extable.o params.o posix-timers.o \
 	    kthread.o wait.o kfifo.o sys_ni.o posix-cpu-timers.o \
 	    ktimers.o
 
+obj-$(CONFIG_OBSOLETE_INTERMODULE) += intermodule.o
 obj-$(CONFIG_FUTEX) += futex.o
 obj-$(CONFIG_GENERIC_ISA_DMA) += dma.o
 obj-$(CONFIG_SMP) += cpu.o spinlock.o
drivers/mtd/chips/Kconfig renamed from drivers/mtd/chips/Kconfig.old +1 -0
@@ -31,6 +31,7 @@
 
 config MTD_GEN_PROBE
 	tristate
+	select OBSOLETE_INTERMODULE
 
 config MTD_CFI_ADV_OPTIONS
 	bool "Flash chip driver advanced configuration options"
drivers/mtd/devices/Kconfig renamed from drivers/mtd/devices/Kconfig.old +1 -0
@@ -202,6 +202,7 @@
 config MTD_DOCPROBE
 	tristate
 	select MTD_DOCECC
+	select OBSOLETE_INTERMODULE
 
 config MTD_DOCECC
 	tristate
#8 lore raw

Re: [2.6 patch] build kernel/intermodule.c only when required

Hi Adrian !

Quoted 3 lines

 config MTD_GEN_PROBE
        tristate
+       select OBSOLETE_INTERMODULE

MTD_GEN_PROBE get's selected by MTD_CFI. If I know my flash chips and don't need any CFI detected, can I (easily) put the info into some platform data and omit the compilation/exection of cfi_probe.c ?

#9 lore raw

Re: [2.6 patch] build kernel/intermodule.c only when required

Quoted 8 lines · Holger Schurig
Quoted 4 lines

 config MTD_GEN_PROBE
        tristate
+       select OBSOLETE_INTERMODULE

MTD_GEN_PROBE get's selected by MTD_CFI. If I know my flash chips and don't need any CFI detected, can I (easily) put the info into some platform data and omit the compilation/exection of cfi_probe.c ?

Not easily. Someone needs to clean up the whole spaghetti plate in drivers/mtd that currently still uses OBSOLETE_INTERMODULE. After that is done, it may be simple to solve your problem as well.

Jörn

Signature · 3 lines
He who knows others is wise.
He who knows himself is enlightened.
-- Lao Tsu
#10 lore raw
In reply to Dave Jones

From: Dave Jones <[email protected]>
Date: Thu, 17 Nov 2005 22:17:51 -0500

Quoted 10 lines · Andrew Morton
Quoted 6 lines
Quoted 3 lines

IMHO the warnings are the best solution for getting a vast amount fixed, and then it's time to think about the rest.

But the warnings don't *work*. I'm *still* staring at stupid pm_register and intermodule_foo warnings. How long has that been?

Too long. I think the mtd stuff won't ever get fixed until after that function gets removed.

That's unfortunate considering we did cure the DRM cases :-)

My only thought is that virt_to_bus() and friends are a special case because they mean compilation failure on most non-x86 platforms.

And frankly, __deprecated serves a different purpose as far as I'm concerned. It let's people working on stuff outside the tree know that "oops you shouldn't be using that interface".

The deprecated warnings are so easy to filter out, so I don't think noise is a good argument. I see them all the time too.

The whole DMA API we have today was added 4+ years ago specifically to get rid of virt_to_bus() and friends. It's been mostly successful, but one last nudge like this deprecation marking might help get us over the edge and finally delete the thing for good. :-)

Anyways, my 2cents.

#11 lore raw
Quoted 3 lines · David S. Miller

The deprecated warnings are so easy to filter out, so I don't think
noise is a good argument. I see them all the time too.

That works for you and me. But how to train all those people who write warny patches?

#12 lore raw
Quoted 7 lines · Andrew Morton
Quoted 3 lines · David S. Miller

The deprecated warnings are so easy to filter out, so I don't think
noise is a good argument. I see them all the time too.

That works for you and me. But how to train all those people who write warny patches?

Lots of poking with pointy sticks. And -Werror. (Just kidding, except for the bit about the sticks).

Seriously, you've caught me out pretty quickly after I've introduced warnings in cpufreq/agpgart, for which I'm thankful. This does put extra pressure on you though, and you've got better things to be doing than sending nag emails.

Dave

#13 lore raw
In reply to Andrew Morton
Quoted 7 lines · Andrew Morton
Quoted 3 lines · David S. Miller

The deprecated warnings are so easy to filter out, so I don't think
noise is a good argument. I see them all the time too.

That works for you and me. But how to train all those people who write warny patches?

Would it work to use -Werror only on some parts of the kernel. Thinking of teaching kbuild to recursively apply a flags to gcc.

Then we could say that kernel/ should be warning free (to a start).

Sam

#14 lore raw
Quoted 11 lines · Sam Ravnborg
Quoted 7 lines · Andrew Morton
Quoted 3 lines · David S. Miller

The deprecated warnings are so easy to filter out, so I don't think
noise is a good argument. I see them all the time too.

That works for you and me. But how to train all those people who write warny patches?

Would it work to use -Werror only on some parts of the kernel. Thinking of teaching kbuild to recursively apply a flags to gcc.

Only if you also add a load of gcc switches to disable some of the more pointless warnings, and also can live with released kernels breaking each time a new gcc gets released.

It's an uphill battle, which is why I only suggested it in a humourous context.

Dave

#15 lore raw
In reply to Sam Ravnborg

From: Sam Ravnborg <[email protected]>
Date: Sat, 19 Nov 2005 01:34:35 +0100

Quoted 6 lines · Andrew Morton
Quoted 5 lines
Quoted 3 lines · David S. Miller

The deprecated warnings are so easy to filter out, so I don't think
noise is a good argument. I see them all the time too.

Quoted 7 lines
Quoted 2 lines

That works for you and me. But how to train all those people who write warny patches?

Would it work to use -Werror only on some parts of the kernel. Thinking of teaching kbuild to recursively apply a flags to gcc.

Then we could say that kernel/ should be warning free (to a start).

Many ports already add -Werror to the CFLAGS via their arch/${ARCH}/* makefiles.

author
#16 lore raw
Quoted 12 lines · David S. Miller

From: Sam Ravnborg <[email protected]>
Date: Sat, 19 Nov 2005 01:34:35 +0100

Quoted 9 lines · Andrew Morton
Quoted 7 lines
Quoted 3 lines · David S. Miller

The deprecated warnings are so easy to filter out, so I don't think
noise is a good argument. I see them all the time too.

That works for you and me. But how to train all those people who write warny patches?

Quoted 7 lines
Quoted 4 lines

Would it work to use -Werror only on some parts of the kernel. Thinking of teaching kbuild to recursively apply a flags to gcc.

Then we could say that kernel/ should be warning free (to a start).

Many ports already add -Werror to the CFLAGS via their arch/${ARCH}/* makefiles.

They only add it to EXTRA_CFLAGS for one specifig subdirectory.

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
author
#17 lore raw
In reply to Sam Ravnborg
Quoted 6 lines · Sam Ravnborg
Quoted 5 lines · Andrew Morton
Quoted 3 lines · David S. Miller

The deprecated warnings are so easy to filter out, so I don't think
noise is a good argument. I see them all the time too.

Quoted 7 lines
Quoted 2 lines

That works for you and me. But how to train all those people who write warny patches?

Would it work to use -Werror only on some parts of the kernel. Thinking of teaching kbuild to recursively apply a flags to gcc.

Then we could say that kernel/ should be warning free (to a start).

We can do better as we do currently, but we cannever get the kernel 100% warning free for all supported kernel configurations and all supported gcc versions.

E.g. gcc emitting some "unused variable" warnings when compiling with CONFIG_PCI=n is quite common, and although they could all be fixed there will always be some warnings with unusual kernel configurations.

Quoted 1 line

Sam

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
#18 lore raw
Quoted 15 lines · Adrian Bunk
Quoted 6 lines · Sam Ravnborg
Quoted 5 lines · Andrew Morton
Quoted 3 lines · David S. Miller

The deprecated warnings are so easy to filter out, so I don't think
noise is a good argument. I see them all the time too.

Quoted 7 lines
Quoted 2 lines

That works for you and me. But how to train all those people who write warny patches?

Would it work to use -Werror only on some parts of the kernel. Thinking of teaching kbuild to recursively apply a flags to gcc.

Then we could say that kernel/ should be warning free (to a start).

Quoted 7 lines

We can do better as we do currently, but we cannever get the kernel 100% warning free for all supported kernel configurations and all supported gcc versions.

E.g. gcc emitting some "unused variable" warnings when compiling with CONFIG_PCI=n is quite common, and although they could all be fixed there will always be some warnings with unusual kernel configurations.

I had no issue with adding more gcc flags, but this is a very valid argument. So I will for now not do it.

Quoted 1 line

From a kbuild perspective it could be useful in other situations

to have the possibility to add a variable that was set also and only in sub-directories. But I will not dive into it before a better reason show up.

Sam

#19 lore raw
In reply to Sam Ravnborg
Quoted 6 lines · Sam Ravnborg
Quoted 5 lines · Andrew Morton
Quoted 3 lines · David S. Miller

The deprecated warnings are so easy to filter out, so I don't think
noise is a good argument. I see them all the time too.

Quoted 7 lines
Quoted 2 lines

That works for you and me. But how to train all those people who write warny patches?

Would it work to use -Werror only on some parts of the kernel. Thinking of teaching kbuild to recursively apply a flags to gcc.

Then we could say that kernel/ should be warning free (to a start).

Well, please don't do that. -Werror makes development very painful.

Pavel

Signature · 1 line
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms
#20 lore raw

From: Pavel Machek <[email protected]>
Date: Sun, 20 Nov 2005 23:27:32 +0000

Quoted 1 line

Well, please don't do that. -Werror makes development very painful.

I've had all of arch/sparc64 in -Werror mode for more than a year, and it's anything but painful. It's prevented me from introducing numerous bugs inadvertantly.

People miss warnings or flat out ignore them, if the build fails they will have to fix it up instead before sending in their changes.

Yes, for a spot like kernel/ it's more difficult since it's compilation is influenced by so many configuration and platform specific stuff, but that's just too bad. Getting it warning free is still something we can and should do.

Saying it's too hard, so we shouldn't even try, is a self-fufilling prophecy. I definitely want to add -Werror to net/ very very soon.

#21 lore raw
In reply to Andrew Morton
Quoted 10 lines · Andrew Morton
Quoted 4 lines · Adrian Bunk

virt_to_bus/bus_to_virt are long deprecated, mark them as __deprecated on i386.

Problem is, nobody's fixing these things. There's no point in adding spam to the kernel build unless it actually gets us some action, and I haven't seen any evidence that it does.

I for one was not aware that these functions were considered to be deprecated, some log "spam" would have alerted me to that fact.

I'll take a look at fixing up some of these, just need to go dig up some docs on what the replacements are first, so be patient...

--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html