Re: [RFC PATCH 09/15] Introduce structured tag value definition

Hi Luca,

On Wed, 01 Apr 2026 17:11:35 +0200
"Luca Ceresoli" <[email protected]> wrote:

> On Tue Feb 10, 2026 at 6:33 PM CET, Herve Codina wrote:
> > The goal of structured tag values is to ease the introduction of new
> > tags in future releases with the capability for an already existing
> > release to ignore those structured tags. In order to do that data length
> > related to the unknown tag needs to be identify.  
>                                          ^
> 					 identified

Will be fixed in the next iteration.

> 
> > Also a flag is present  
>  "Also add a flag"

Will be updated in the next iteration.

> 
> > to tell an old release if this tag can be simply skipped or must lead to
> > an error.
> >
> > Structured tag value is defined on 32bit and is defined as follow:
> >
> > Bits  | 31 | 30       | 29             28 | 27    0|
> > ------+----+----------+-------------------+--------+
> > Fields| 1  | CAN_SKIP | DATA_LNG_ENCODING | TAG_ID |
> > ------+----+----------+-------------------+--------+
> >
> > Bit 31 is always set to 1 to identified a structured tag value.  
>                                ^
> 			       identify
> 
> > Bit 30 (CAN_SKIP) is set to 1 if the tag can be safely ignore when its  
>                                                          ^
> 							 ignored

Both will be fixed in the next iteration.

> 
> 
> > TAG_ID value is not a known value (unknown tag). If the CAN_SKIP bit is
> > set to 0 this tag must not be ignored and an error should be reported
> > when its TAG_ID value is not a known value (unknown tag).
> >
> > Bits 29..28 (DATA_LNG_ENCODING) indicates the length of the data related  
> 
> I think "LEN" is more common than "LNG".

Agree, will be changed.

...
> >
> > +/* Tag values flags */
> > +#define FDT_TAG_STRUCTURED	(1<<31)
> > +#define FDT_TAG_SKIP_SAFE	(1<<30)  
> 
> This is called CAN_SKIP in the commit message and SKIP_SAFE here. Using a
> consistent name would be better IMO.

I will use SKIP_SAFE and so update the commit message accordingly in the next
iteration.

> 
> > +#define FDT_TAG_DATA_MASK	(3<<28)
> > +#define FDT_TAG_DATA_NONE	(0<<28)
> > +#define FDT_TAG_DATA_1CELL	(1<<28)
> > +#define FDT_TAG_DATA_2CELLS	(2<<28)
> > +#define FDT_TAG_DATA_LNG	(3<<28)  
> 
> I find _LNG (or _LEN) misleading: this is not the length, but rather an
> enum value telling you the length is stored in the next cell. What about
> FDT_TAG_DATA_VARLEN?

Yes indeed, VARLEN is better.
I will use FDT_TAG_DATA_VARLEN in the next iteration.

Best regards,
Hervé