Kconfig/Makefile redundancy

From Crashcourse Wiki

Jump to: navigation, search

Contents

[edit] Overview

This is a description of possible cleanup that could be applied throughout the kernel source tree, but I'll explain it in the context of just the drivers/mtd directory. All file references, unless further qualified, are relative to that directory.

[edit] How Kconfig subdirectories work

Here's how you typically include subdirectories in the Kbuild structure, as you can see from the bottom of the file Kconfig:

...
source "drivers/mtd/chips/Kconfig"
source "drivers/mtd/maps/Kconfig"
source "drivers/mtd/devices/Kconfig"
source "drivers/mtd/nand/Kconfig"
source "drivers/mtd/onenand/Kconfig"
source "drivers/mtd/lpddr/Kconfig"
source "drivers/mtd/ubi/Kconfig"

Perfectly reasonable -- the inclusion (or "sourcing") is not conditional, all of those subdirectories are included no matter what. But it doesn't mean they should all be processed since you still have the right to select or deselect a number of those features during the initial kernel config step.

[edit] How Kconfigs are done correctly (sort of)

Using the MTD nand subdirectory as an example, this is the right way to do things. See the Kconfig file in that directory:

menuconfig MTD_NAND
        tristate "NAND Device Support"
        depends on MTD
        select MTD_NAND_IDS
        help
          This enables support for accessing all type of NAND flash
          devices. For further information see
          <http://www.linux-mtd.infradead.org/doc/nand.html>.

if MTD_NAND
... most of file snipped ...
endif # MTD_NAND

See how that works? That Kconfig file defines the MTD_NAND variable that you can select via make menuconfig and, if you select it, the if directive brings the rest of the file into play. Run make menuconfig to see what I mean. And this is typically the right way to do this -- where a single select/deselect directive exposes an entire submenu of dependent choices. So far, so good?

[edit] Cleanup even when it's done right

Even in the above, there's potential cleanup since it's possible there's historical cruft where individual directives in that Kconfig file still have explicit (but now superfluous) dependencies, such as:

...
config MTD_NAND_CM_X270
        tristate "Support for NAND Flash on CM-X270 modules"
        depends on MTD_NAND && MACH_ARMCORE
                   ^^^^^^^^
...
config MTD_NAND_FSL_UPM
        tristate "Support for NAND on Freescale UPM"
        depends on MTD_NAND && (PPC_83xx || PPC_85xx)
                   ^^^^^^^^
        select FSL_LBC
...

Note the explicit dependencies on MTD_NAND in the above. Obviously, since the majority of the file is wrapped in an if MTD_NAND test, those individual dependencies are now unnecessary (harmless, of course, but unnecessary), and are almost certainly leftovers from days gone by before that file was restructured using that single if directive. So, as a first cleanup, any Kconfig file could be cleaned up by reducing such internal, unnecessary dependency references. As in, that first one would be rewritten as:

config MTD_NAND_CM_X270
        tristate "Support for NAND Flash on CM-X270 modules"
        depends on MACH_ARMCORE

So that's cleanup number one. (There's at least a little of that under the onenand subdirectory as well, and probably others.)

[edit] How subdirectories are processed

From the MTD Makefile, you can see two different ways to process subdirectories when you finally do the build:

obj-y           += chips/ lpddr/ maps/ devices/ nand/ onenand/ tests/

obj-$(CONFIG_MTD_UBI)           += ubi/

In the case of the ubi subdirectory, that's typically the right way to do things -- don't descend into that subdirectory for compilation unless it's been selected during the configuration step. And if you define the subdirectory processing that way, you don't need to further keep testing CONFIG_MTD_UBI within that subdirectory to decide what you have to compile.

But if you pop into that subdirectory and look at the Makefile, you see the first line:

obj-$(CONFIG_MTD_UBI) += ubi.o

But that's redundant -- the higher-level Makefile already tests that variable to decide whether to even enter this directory for compilation purposes, so the above could have (and should have) been written simply as:

obj-y += ubi.o

That's not to say that the rest of the Makefile might not have tests if those tests refer to more specific features, it just means that the top-level test in this case is redundant and could be simplified.

[edit] Making other subdirectories conditionally processed

As I mentioned, the way the ubi subdirectory is configured and processed is (almost) the right way to do things. So what happens with those other subdirectories? Well, if it's appropriate, those other subdirectories should be treated the same way, as in this:

obj-y           += ... onenand/ ...

should (once you verify that it's appropriate) be replaced by this:

obj-$(CONFIG_MTD_ONENAND)           += onenand/

But that's not the end of it. If that can be done (and, naturally, it can only be done with subdirectories that represent features that can be selected or deselected in their entirety), that means that all testing of that variable in that subdirectory is now superfluous. So in the onenand Makefile, you could now replace this:

# Makefile for the OneNAND MTD
#

# Core functionality.
obj-$(CONFIG_MTD_ONENAND)               += onenand.o

with this:

# Makefile for the OneNAND MTD
#

# Core functionality.
obj-y               += onenand.o

This is even more obvious in the tests directory, where the Makefile is:

obj-$(CONFIG_MTD_TESTS) += mtd_oobtest.o
obj-$(CONFIG_MTD_TESTS) += mtd_pagetest.o
obj-$(CONFIG_MTD_TESTS) += mtd_readtest.o
obj-$(CONFIG_MTD_TESTS) += mtd_speedtest.o
obj-$(CONFIG_MTD_TESTS) += mtd_stresstest.o
obj-$(CONFIG_MTD_TESTS) += mtd_subpagetest.o
obj-$(CONFIG_MTD_TESTS) += mtd_torturetest.o
obj-$(CONFIG_MTD_TESTS) += mtd_nandecctest.o

I'm sure you get the idea.

Personal tools