search

availink / amlogic_meson_dvb4linux

Availink demod (and tuner) Linux drivers for Amlogic Meson SoC
GNU General Public License v2.0
2 stars 8 forks source link

Restructure the source code tree #3

Open moham96 opened 4 years ago

moham96 commented 4 years ago

Hi, I want to suggest that the source code be splitted according to the linux kernel standards. The current method of writing a .c file to choose the demod+tuner combination and compile it as a module is messy and will not be accepted upstream. The correct way to do this is to split everything, In linux source tree the demodulator drivers should go into drivers/media/dvb-frontends/, the tuner drivers should go into drivers/media/tuners/ and then the kernel should auto-load the suitable drivers for the combination in the system based on the compatible string in the device tree.

Regards

tomrich82 commented 4 years ago

Mohammad,

I realized the same thing when hacking on the aml_fe_XXX module, but it was just a means to an end - verifying our new avl62x1 driver. However, I would like to do it the 'right way' in case it requires changes to our demod driver(s). I'm unsure how to go about this, however, and have many questions - I don't even know what I don't know at this point. Do you know of a good example of this that I can use as reference?

Regards, Tom Richardson

moham96 commented 4 years ago

Hi Tom,

First of all, the usual way to mainline a driver would be to work on a mainline tree and not scattering the work on multiple repos with many pieces.So the first step would be to fork a mainline tree to your github account like https://github.com/torvalds/linux/ and work on that. now regarding a reference driver, the only driver that I know of in mainline linux that is close to the situation of meson and avl is c8sectpfe (device tree for the board) The difference is that this driver is very specific in what frontends it supports since it was written for a specific board that only comes with handful of demod/tuner configurations this is not the case with meson boards since they come with many different combination of demods/tuners so the hw-demuxer driver for meson should not depend on any specific frontend driver.

The hw-demuxer for meson should go into a similar path in the tree (e.g drivers/media/platform/meson/dvb), the demod drivers should go into drivers/media/dvb-frontends/ and tuner drivers into drivers/media/tuners/.

The device tree of a board should then describe the hardware, it should conatain a node for the hw-demux and subnodes for the demod/tuner combination then the kernel will match the .compatible in the device tree with the .compatible in the drivers and load the hw-demuxer driver and the relevant demod/tuner drivers, a very nice example of this is present in the dt-binding of the c8sectpfe driver here of course instead of matching the frontends by the dvb-card property it should be something modular(.compatible)

For the firmwares you should also clone the mainline linux-firmware tree and add your firmwares to the relevant path with a LICENSE and a WHENCE files, the license should clearly allow redistribution of the binaries, take a look at this recent firmware submision by maxime for the meson-vdec IP.

Regards

tomrich82 commented 4 years ago

Mohammad,

Our intent is to, in the near future, fork media_tree and linux-firmware and integrate our code in there and retire the temporary repos.

I looked through the c8sectpfe-dvb.c code and understand it, but I don't see how that fundamentally fixes the "hardcoding" issue. Yes, the device tree defines the peripheral configuration and the driver uses it to configure the hardware, but at the end of the day, c8sectpfe-dvb.c still has hard-coded references to symbols in driver modules for all possible demods and tuners that it might encounter. I believe this means that all of those modules will be loaded into memory as the c8sectpfe-dvb driver needs to be fully linked.

I was thinking more along the lines of this: c8sectpfe-dvb loads the needed demux/tuner modules at runtime kind of the way a DLL works, but I don't believe this is possible in the kernel. Since we're not hard-coding things specific to demux/tuners any more, this implies that those drivers then need to examine their own device tree nodes to configure themselves. That would work for static configuration tasks, but precludes configuring something dynamic. Thoughts?

I was not aware of the WHENCE file. Thanks for the heads-up.

Regards, Tom

moham96 commented 4 years ago

Hi Tom,

Yes as I said the c8sectpfe driver is different in that regards and we need to come up with a good way to dynamically load frontends and tuner drivers(I'm not sure if that is possible currently since I can't find a driver that does that). I think we should move this discussion to the LE slack channel since we have some mainline linux dvb experts there that can give some ideas. In the meantime you can just start with something like a c8sectpfe for meson, any barebone driver should do it at this point at least for testing, we can worry about the dynamic loading of frontend driver later, if you have a fully working driver(without dynamic loading) you can even submit it as an RFC patch (Request For Comment) which should get you better ideas from linux dvb maintainers/experts in the mailing list.

Regards