On 11:44-20240129, Dhruva Gole wrote: > On Jan 24, 2024 at 12:09:06 -0600, Nishanth Menon wrote: > > On 18:37-20240124, Dhruva Gole wrote: > > > The secure_hdr needs to be 0 init-ed however this was never being put > > > into the secure_buf, leading to possibility of the first 4 bytes of > > > secure_buf being possibly garbage. > > > > > > Fix this by initialising the secure_hdr itself to the secure_buf > > > location, thus when we make it 0, it automatically ensures the first 4 > > > bytes are 0. > > > > > > Fixes: 32cd25128bd849 ("firmware: Add basic support for TI System Control > > > Interface (TI SCI)") > > > Signed-off-by: Dhruva Gole <d-g...@ti.com> > > > --- > > > > > > Boot tested for sanity on AM62x SK > > > https://gist.github.com/DhruvaG2000/724ceba3a0db03f4b0bff47de1160074 > > > > > > Changelog: > > > v2 --> v3 > > > Address Kamlesh's comment on v2: use sizeof(struct ti_sci_secure_msg_hdr) > > > > Lets finish discussing in: > > https://lore.kernel.org/all/20240124163910.sp7gt56lihoujm7k@etching/ > > Bringing the conversation back to this latest patch revision, > Based on where we left off: > https://lore.kernel.org/all/20240125171335.qoxphnemadkh7xjd@gullible/ > > Would it be better to add a comment above ``if (info->is_secure) {`` in > drivers/firmware/ti_sci.c as follows: > The secure path will be used by R5 SPL bcause it starts of in "secure mode" > when it hands > off from Boot ROM over to the Secondary bootloader. > > Kindly advise if the patch needs respin with that minor change, and if the > rest > of it seems okay? >
improve the commit message and add a documentation patch to add information around if (info->is_secure) please, so that we don't yet again need to dig up history. -- Regards, Nishanth Menon Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D