On Fri, 2023-05-26 at 18:27 -0700, Ceraolo Spurio, Daniele wrote: > <snip> > > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_meu_headers.h b/drivers/gpu/drm/i915/gt/uc/intel_gsc_meu_headers.h > > > index d55a66202576..8bce2b8aed84 100644 > > > --- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_meu_headers.h > > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_meu_headers.h alan:snip > > alan: structure layout seems unnecessarily repetitive... why not -> > > struct partition_info { > > u32 offset; > > u32 size; > > }; > > struct intel_gsc_layout_pointers { > > u8 rom_bypass_vector[16]; > > ... > > struct partition_info datap; > > struct partition_info bootregion[5]; > > struct partition_info trace; > > }__packed; > > > > I've just realized that I didn't reply to this comment. The specs have > the structure defined that way, so I tried to keep a 1:1 match like we > usually do. I think switching to a partition_info structure is ok, but > I'll avoid the array because the GSC partition are 1-based, which could > cause confusion (i.e. partition boot1 would be bootregion[0]). alan: sure - that's fine - let's stick to align with the spec definitions