Hi Eric, On Tue, 10 Feb 2026 at 07:38, Eric Schikschneit <[email protected]> wrote: > > Hello Simon, Tom, > > 1. Tom - > > And we should spell out that device tree can't set these any more in the > > commit message, along with spelling out why we can't do that anymore > > safely (why can't we?). Thanks. > I would be happy to move the code comment into the commit message: > /* NOTE: This breaks u-boot ability to set board options via * > * device tree, but guarantees specific known values.. * > * you wouldnt use an uninitialized variable would you?? */ > These settings could be set by device tree, but every fdtdec* function must > set a valid value. If a user has something set differently than called out in > the > FSP_MR5 Integration Guide the board stability will be compromised. Many of > these settings are co-dependant so we would need much more robust error > checking. This task would not be trivial. By using the specific values called > out > by the Integration guide we allow users to begin from a documented known > good configuration. Maybe we could set all the values, and then check the > device tree to see if the user specified a different value? If the device > tree does > not call out a specific setting we would not want to just set it to zero for > example > as this can cause undefined behavior.
The device tree is actually where these values are set for all x86 boards. If you change any value in a device tree you risk bricking any board. It should be considered an integral part of the build. If you change the C code you can also brick a board. So I don't like the idea of changing this to C, sorry. NAK from me sorry. > > Eric Schikschneit > Senior Embedded Linux Engineer III > > ________________________________________ > From: Simon Glass <[email protected]> > Sent: Tuesday, February 3, 2026 3:29 PM > To: Tom Rini > Cc: Eric Schikschneit; [email protected]; [email protected] > Subject: Re: [PATCH 2/2] Fix x86 baytrail boot flow, UPD/VPD Updates > > Hi, > > On Wed, 4 Feb 2026 at 07:14, Tom Rini <[email protected]> wrote: > > > > On Mon, Jul 14, 2025 at 04:02:49PM -0500, Eric Schikschneit wrote: > > > > > The VPD has been updated to include additional values found in the Intel > > > FSP_MR5 Integration Guide. > > > > > > Disable MRC_CACHE for the baytrail platform as this will break the boot > > > flow > > > as well. This issue has not been diagnosed further, but I suspect it is > > > similar > > > to my previous patch and related to how modern GCC generates the low level > > > op-codes. > > > > > > Patch 2 of 2 > > > > > > Upstream-Status: Pending > > > > > > Signed-off-by: Eric Schikschneit > > > <[email protected]> > > [snip] > > > + /* Set all values to known good defaults as documented in * > > > + * Intel FSP_MR5 file: BAYTRAIL_FSP.bsf */ > > > + > > > + /* NOTE: This breaks u-boot ability to set board options via * > > > + * device tree, but guarantees specific known values.. * > > > + * you wouldnt use an uninitialized variable would you?? */ > > > > /* > > * Multi-line comments like this please per > > * https://docs.u-boot.org/en/latest/develop/codingstyle.html > > */ > > > > And we should spell out that device tree can't set these any more in the > > commit message, along with spelling out why we can't do that anymore > > safely (why can't we?). Thanks. > > Again I am not seeing this problem with minnowmax, so would like a few > more details. > Which board is this? Regards, Simon
