Skip to content

Conversation

@cbiffle
Copy link
Collaborator

@cbiffle cbiffle commented Dec 11, 2025

This applies the fixes descrfibed in #2321

This adds logic to apply the BMR491 input filter defect mitigation
described in #2321. Note that the routine must still be called from
somewhere to be useful!
Comment on lines +117 to +118
// Assume the mitigation is unnecessary.
return Ok(());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be nice if we put something in the ringbuf indicating whether we've decided we have a revision that needs the mitigation or not, just so that we can have a record of what the driver decided to do someplace?

Comment on lines +131 to +135
if current_vin_off.0 == 0
&& current_vout_command.0 == 0x0060
&& current_max_duty.0 == 0xF8EA
&& current_vout_uv_fault_limit.0 == 0x0058
&& current_vout_uv_fault_response.0 == 0x80
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpicky, probably unnecessary, but i wonder if we ought to make the mitigation VIN_OFF/VOUT_COMMAND/MAX_DUTY/VOUT_UV_FAULT_LIMIT/VOUT_UV_FAULT_RESPONSE values constants, and use them both in this check and when setting the registers later on.

that way, if we ever decide to change these values, we don't run the risk of accidentally changing the value we set but not the one we check for.

Comment on lines +137 to +138
// The device configuration already reflects the mitigation.
return Ok(());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we ringbuf the driver's decisions as per my earlier comment, we should probably also say something when we believe the mitigation has already been applied.

VOUT_UV_FAULT_RESPONSE::CommandData(0x80)
)?;

pmbus_write!(self.device, STORE_USER_ALL)?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought RFD 630 determined we were not going to do this? Or has this PR not yet been updated to match?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants