Re: [PATCH 04/10] common: add barebox TLV support

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hello Sascha,

On 4/14/25 16:49, Sascha Hauer wrote:
> On Fri, Apr 11, 2025 at 09:40:39AM +0200, Ahmad Fatoum wrote:
>> barebox TLV is a scheme for storing factory data on non-volatile
>> storage. Unlike state, it's meant to be read-only and if content
>> is limited to already specified tags, it can be extended later on,
>> without modifying the bootloader binary.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>> +struct tlv_mapping barebox_tlv_v1_mappings[] = {
>> +	{ 0x0002, tlv_format_str, "device-hardware-release" },
>> +	{ 0x0003, tlv_format_dec, "factory-timestamp" },
> 
> The numbers in the examples suggest that the format is in epoch time,
> but shouldn't this be documented?

Good point. Yes, it's seconds since the UNIX epoch.
Will document it for v2.

>> +	{ 0x0004, tlv_handle_serial, "device-serial-number"},
>> +	{ 0x0005, tlv_format_dec, "modification" },
>> +	{ 0x0006, tlv_format_str, "featureset" },
> 
> I think things like "device-hardware-release", "modification" and "featureset"
> need (better) documentation, at least when they are supposed to be generic.

In the end the revision scheme and the concrete interpretation of bits
depends on the project, but I can add some generic documentation inline,
e.g.:

device-hardware-release: Detailed release information string for the device

modification: 0: Device is unmodified. Any set bits indicate a
board-specific modification after assembly

featureset: Any set bits indicate enablement of board-specific features

> 
>> +	{ 0x0007, tlv_format_str, "pcba-serial-number"},
>> +	{ 0x0008, tlv_format_str, "pcba-hardware-release"},
> 
> Assuming 'pcb' is for printed circuit board, what is 'a'?

assembly as in the PCB with electronics components attached.

Cheers,
Ahmad

> 
> Sascha
> 
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |