Hi Simon, Thank you for your review!
On Sat, Oct 3, 2026 at 8:47 AM Simon Glass <[email protected]> wrote: > > Hi Alexey, > > On 2026-09-18T12:13:49, Alexey Charkov <[email protected]> wrote: > > > Patches 3 and 4 add the C tests, which are complementary rather than > > alternatives. Patch 3 drives gpt_fill_header()/gpt_fill_pte() with a > > synthetic descriptor, which is the only practical way to sweep block > > sizes from 512 to 32768, but it can never reach anything that touches the > > medium. Patch 5 backs a blkmap device with plain memory and does a full > > write/verify/parse round trip > > The blkmap round trip is patch 4; patch 5 is the malta64 defconfig > change. In the change log, the 'unbind the 4096-byte device' item is > listed under patch 6 but is in patch 7, and there are two 'Patch 3' > entries, so it is hard to tell which numbering (v1 or v2) each line > uses. Please can you fix the numbering if there is a v3? Yes, will fix. Deleting a patch between versions is messy in this regard :) > > Partitions given without an explicit start= will be placed > > lower, though, and on sunxi first_usable_lba lands exactly on the 8 KiB > > SPL offset, so sunxi and Rockchip folks may want to look at patch 1 > > specifically (none of the devices I have override the default). > > It would be worth copying those maintainers. Ack. > Just to check, has this been tried on a sunxi or Rockchip board where > the first partition has no explicit start= ? If that case can > overwrite the SPL, I wonder whether the series should deal with it, > e.g. by having those boards set CONFIG_EFI_PARTITION_ENTRIES_OFF, or > at least mention the change in the documentation, rather than leaving > it as a note in the commit message. What do you think? I've rechecked, and all of those boards that set $partitions also have a fixed start=, so no existing users should really be affected after all. Without $partitions any gpt write would fail anyway, as seems to be the case on Khadas Edge2 (hinting that it was likely never tested in the first place). > > Patch 5 enables EFI_PARTITION and BLKMAP on malta64 so both tests are > > actually built and run on a big-endian target. > > The blkmap can only be created with 512-byte blocks, so the round-trip > test in patch 4 has a single case and the 4096-byte write/read path is > only covered on sandbox, by the python test in patch 7. So the full > write/verify/parse path at that block size has no big-endian coverage. > That is fine for this series, but please can you list it in the 'Still > not covered' section? Yes, full big-endian coverage is quite involved - will mention this in the cover letter on v3. > BTW the discussion with Heinrich on patch 2 about converting names > with the charset functions is still open. If the outcome is to leave > the lossy conversion as is due to SPL size (I tend to agree), please > can you mention that in the cover letter as a known limitation? Will do, thanks. Best regards, Alexey
