OpenIPC / builder

Experimental system for building OpenIPC firmware for known devices
https://openipc.org
MIT License
16 stars 19 forks source link

Add JVS INGT10 GQS60EP T10A+OV9750 #8

Closed jimsmt closed 6 months ago

jimsmt commented 6 months ago

In my opinion, these actions are unnecessary We need to make sure that this driver is compiled from source directly into Firmware along with the rest.

Since it’s not a commonly used sensor, so I included the driver for this particular device. If we need to make sure all sensor drivers are compiled within the main repo, we need to enable them all by default, which will make the generic firmware too big, some SoCs have more than 100 sensor drivers in the openingenic repo

I’m doing this also to avoid making changes to the main repos, which may cause issue unexpectedly

cronyx commented 6 months ago

In my opinion, these actions are unnecessary We need to make sure that this driver is compiled from source directly into Firmware along with the rest.

devices/t10_lite_jvs-ingt10-gqs60ep/general/package/ingenic-opensdk/0001-add-ov9750.patch
devices/t10_lite_jvs-ingt10-gqs60ep/general/package/ingenic-osdrv-t20/ingenic-osdrv-t20.mk

At the moment there is no other solution, you need to either make a patch adding a sensor to openingenic or add an already compiled driver to the assembly.

flyrouter commented 6 months ago

Okay, then this is the proposal. We accept this patch, but at the same time open a ticket in the openingenic repo about the need to create a patch for this driver. Once created and verified in openingenic, I think we can remove the files from the builder repo. I mean that you need to try to do as much as possible in other repositories so that the builder repo remains minimalistic for each device. Thanks for understanding.

jimsmt commented 6 months ago

Okay, then this is the proposal. We accept this patch, but at the same time open a ticket in the openingenic repo about the need to create a patch for this driver. Once created and verified in openingenic, I think we can remove the files from the builder repo. I mean that you need to try to do as much as possible in other repositories so that the builder repo remains minimalistic for each device. Thanks for understanding.

There's already driver source code on openingenic repo, but not enabled for compiling by default, only some usually used sensors are enabled by default. This patch is just for enabling compiling for this particular device. If we don't use this kind of patch, all sensor drivers should be enabled for compiling by default, which may be huge. Or we may have some options in defconfig to specify which sensor drivers to compile?

flyrouter commented 6 months ago

You need to enable compilation of this sensor in the openingenic repository and, after checking, remove the files from the device to the ingenic repo. We need to strive for universality, so that as few change files as possible are in the Builder repo. Only a package configuration file, an unnecessary removal file, a settings file. Three files are the minimalism you need to strive for