Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Refactored FSP message validation incorrectly bubbles up enum conversion errors via `?`, bypassing diagnostic logging and changing the return code from `-EIO` to `-EINVAL`. --
commit affc1bd88bbaf7649b7fc1c329019739603b4580 Author: Alexandre Courbot <[email protected]> gpu: nova-core: convert to kernel bitfield macro This commit replaces uses of the Nova-local bitfield! macro with the kernel one. > diff --git a/drivers/gpu/nova-core/fsp.rs b/drivers/gpu/nova-core/fsp.rs > index 8fc243c66e356..91885b355d462 100644 > --- a/drivers/gpu/nova-core/fsp.rs > +++ b/drivers/gpu/nova-core/fsp.rs [ ... ] > @@ -274,7 +275,12 @@ fn send_sync_fsp<M>(&mut self, dev: &device::Device, > bar: Bar0<'_>, msg: &M) -> > return Err(EIO); > } > > - if command_nvdm_type != u8::from(M::NVDM_TYPE).into() { > + if command_nvdm_type > + .try_into_bounded() > + .ok_or(EINVAL) > + .and_then(NvdmType::try_from)? > + != M::NVDM_TYPE > + { > dev_err!( > dev, > "Expected NVDM type {:?} in reply, got {:#x}\n", [Severity: Medium] Does the ? operator here prematurely return EINVAL if the firmware returns an unknown or invalid NVDM type? It looks like if NvdmType::try_from fails, this will immediately propagate the error out of send_sync_fsp(). Does this bypass the dev_err! diagnostic block below that was intended to log the unexpected hardware value? By returning early here, this also appears to alter the original error reporting semantics by returning EINVAL instead of the expected EIO for a device failure. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
