Manouchehri / smi2021

smi2021 v4l2 kernel driver
25 stars 13 forks source link

The module doesn't add all dependencies when build out of tree #13

Closed jonjonarnearne closed 9 years ago

jonjonarnearne commented 9 years ago

Hi, I finally had some time to look at the changes you and Mastervolkov have done to the driver. It looks really nice, and the ability to build out of tree is great, but I encountered an issue, and I believe #11 is caused by the same problem.

The makefile that I use for building in kernel tree specifies: The Kconfig file I use when building in the kernel-tree specifies:

depends on VIDEO_DEV && I2C && SND && USB
        select VIDEOBUF2_VMALLOC
        select VIDEO_SAA711X
        select SND_PCM

But all this is ignored in the out of tree build. When I build the driver on my computer that has a kernel without the saa7115 module, the driver will compile without warnings, and will be installed without problems. But I only get a blank screen when you try to do captures, and the different controls for brightness and contrast are missing from qv4l2.

I haven't had time to look for solutions to this problem yet, so if anyone have any ideas, please step up :)

Manouchehri commented 9 years ago

Kconfig still has the depends line, but I'm guessing it's something in @4c4ed811434d7e9?

jonjonarnearne commented 9 years ago

Ah, my bad. I meant to say that Kconfg contains the lines.

I think the problem is that the makefile ignores the kconfig file if you're not building the driver as part of the kernel-tree.

Manouchehri commented 9 years ago

I don't think kerpz/smi2021#2 is related to any new changes, since it looks like that repository is based off of @157b9ce82361c (it's a pain to check since there's no proper history).

I can't figure out how to force Kconfig to be obeyed for out of tree builds, most of "solutions" I found only suggest adding the extra modules manually. Going to have to wait for somebody else to chime in on this one.

mastervolkov commented 9 years ago

Adding needed modules manualy is simplest and right way on out of tree build. String with required add dependent modules manually in menuconfig before build added to readme because of this.

jonjonarnearne commented 9 years ago

I've done some research, and it looks like we could add a small script to check that the needed modules/symbols are available.

/proc/kallsyms contains a list of all exported symbols from current kernel. I'm not sure if this file is present in all distributions.

Also, /lib/modules/$KERNEL_VERSION/modules.symbols contains a list of what symbols are present in the loadable modules.

The script should only have to look for these files and grep for some of the needed symbols. and then return to the user what modules are missing.

I could try to code something :)

PS: @mastervolkov. I'm sorry, it seems I didn't read the readme thoroughly enough.

Manouchehri commented 9 years ago

Nice, I can definitely hack something up in bash as well to check if you don't have time. Is there a more "proper" way of checking for symbols instead of just grep'ing over everything?

jonjonarnearne commented 9 years ago

I don't know of a more "proper" way, but it doesn't sound so hackish does it? But please go ahead and implement something if you have the time :)

mastervolkov commented 9 years ago

Can you check: https://www.kernel.org/doc/Documentation/kbuild/modules.txt

Seems like need do something like in 6.1, because in 7.1 is:

External modules have traditionally used "grep" to check for
    specific CONFIG_ settings directly in .config. This usage is
    broken. As introduced before, external modules should use
    kbuild for building and can therefore use the same methods as
    in-tree modules when testing for CONFIG_ definitions. 
mastervolkov commented 9 years ago

For all of the above:

For Manouchehri/smi2021#11 - after finish fix readme he can rebuild yet by new instruction with new deendency check in code and (in my opinion) issue can be closed

Is it possible to close the task or I missed something and need to finish it?

PS: @jonjonarnearne I'm sorry, too... I incorrectly added information about the build in readme and information was not readable ( bad perceived)... New, fixed and improved version already in https://github.com/Manouchehri/smi2021/tree/readme_fix

mastervolkov commented 9 years ago

Readmy fixed. Warning about chip version if chip not supported added. all must work now as expected in staging branch