Hi Ilias, On Wed, Dec 27, 2023 at 11:12 AM Ilias Apalodimas <ilias.apalodi...@linaro.org> wrote: > > Hi Simon, > > I commented on v3 as well, but in case you miss that > > On Wed, 27 Dec 2023 at 09:40, Simon Glass <s...@chromium.org> wrote: > > > > All callers handle this alignment, so drop the unnecessary code. This > > simplifies things a little. > > > > Signed-off-by: Simon Glass <s...@chromium.org> > > Reviewed-by: Heinrich Schuchardt <xypron.g...@gmx.de> > > --- > > > > (no changes since v1) > > > > include/smbios.h | 5 +---- > > lib/smbios.c | 2 -- > > 2 files changed, 1 insertion(+), 6 deletions(-) > > > > diff --git a/include/smbios.h b/include/smbios.h > > index 77be58887a2..879b8a814b8 100644 > > --- a/include/smbios.h > > +++ b/include/smbios.h > > @@ -258,12 +258,9 @@ static inline void fill_smbios_header(void *table, int > > type, > > * > > * This writes SMBIOS table at a given address. > > * > > - * @addr: start address to write SMBIOS table. If this is not > > - * 16-byte-aligned then it will be aligned before the table is > > - * written. > > + * @addr: start address to write SMBIOS table (must be > > 16-byte-aligned) > > * Return: end address of SMBIOS table (and start address for next > > entry) > > * or NULL in case of an error > > - * > > */ > > ulong write_smbios_table(ulong addr); > > > > diff --git a/lib/smbios.c b/lib/smbios.c > > index 7f79d969c92..cfd451e4088 100644 > > --- a/lib/smbios.c > > +++ b/lib/smbios.c > > @@ -563,8 +563,6 @@ ulong write_smbios_table(ulong addr) > > ctx.dev = NULL; > > } > > > > - /* 16 byte align the table address */ > > - addr = ALIGN(addr, 16); > > I think this is wrong. It will break SMBIOS on a user error. I am > fine replacing that with a check instead and error out if the address > is not 16b aligned
Well this is why the comment says it must be aligned. This function is not called willy nilly, from code we control. Checking for alignment at runtime creates confusion and adds to code size. > > Thanks > /Ilias > > start_addr = addr; > > > > /* > > -- > > 2.34.1 > > Regards, Simon
