On 22.01.20 20:28, Joel Johnson wrote:
<snip>
It probably makes more sense to reverse the order of ORed conditions:
if (IS_ENABLED(TARGET_CLEARFOG_BASE) || sr_product_is(&cf_tlv_data, "Clearfog Base"))
IS_ENABLED() is evaluated at build time. When it is true, the compiler drops all other 'if' branches, thus saving space. That also means that the build time configuration overrides the EEPROM set value, which is a Good Thing I believe.
I'll take a look and do testing and size comparison in the next day or two.
Note that I intended (and wrote the Base target help docs accordingly) that runtime detection, if both enabled and supported in hardware, should be preferred to the static configuration, with static config being only a fallback.
This sounds reasonable.
This seems to be different from what your thought about it being good for build configuration to override EEPROM detected values, and I'm curious as to your reasoning.
There are a few gaps here related to what you point out (e.g. booting on a Pro with EEPROM and Base static config won't give the expected results). Relocating the static adjustment may be fine and help with that case as well.
I'll want to add support in such handling for the EEPROM Pro devices but don't have one available. You seem to have them available, can you confirm that "Clearfog Pro" would match those devices using sr_product_is?
I currently don't have any of these boards available, so I also would like to see some reviewed-by and even better tested-by comments from Baruch (or someone else at SolidRun).
Thanks, Stefan