[PATCH] i2c: sandbox: Avoid calling dev_read_*() if CONFIG_OF_PLATDATA=y

[Tom Rini]

On Fri, Nov 21, 2025 at 12:09:21PM -0700, Simon Glass wrote:
> Hi Marek,
> 
> On Fri, 21 Nov 2025 at 11:01, Marek Vasut <marek.vasut at mailbox.org> wrote:
> >
> > On 11/21/25 6:40 PM, Simon Glass wrote:
> >
> > Hello Simon,
> >
> > >>>>> - create a new VALID_OFFSET assumption (split out from VALID_DTB)
> > >>>>> which checks offsets in fdt_next_tag()
> > >>>>>
> > >>>>> The latter might be best. It would likely be very cheap in terms of
> > >>>>> code size. In fact, I should have thought of this at the time.
> > >>>> Wouldn't it be better to not misuse libfdt ?
> > >>>
> > >>> It actually isn't a misuse. It is perfectly OK to pass an invalid
> > >>> offset and libfdt has defined behaviour in that case, absent any
> > >>> 'assumptions' we force.
> > >>
> > >> It actually isn't OK to pass in invalid offset if we compile FDT with
> > >> disabled tree validity checks.
> > >
> > > We are saying exactly the same thing, if you read both of the above :-)
> >
> > What I wrote is, that it is NOT OK to pass invalid offset to libfdt IFF
> > the DT validity checks are disabled. This is yes a misuse.
> 
> Yes, of course. It IS OK if you enable the validity checks.
> 
> >
> > >>> Remember, in Linux these checks are enabled. We added the 'assume'
> > >>> thing to reduce code size. Here you have found a situation where it
> > >>> breaks stuff, so we should back off the assumption a bit.
> > >> U-Boot also has those checks enabled, SPL does not, and things break
> > >> when OF_PLATDATA is enabled and there is no valid node associated with
> > >> the udevice.
> > >
> > > Right, so add SPL_OF_LIBFDT_ASSUME_MASK for SPL and check the code
> > > size. If it is too much, add a new level FDT_ASSUME_EXISTS before
> > > FDT_ASSUME_SANE and use that for the necessary checks (there will only
> > > be a few, from my reading of the code).
> > The SPL assumes the FDT is valid, and disables FDT validation in libfdt,
> > to keep the code size low.
> >
> > U-Boot does enable that validation in libfdt, and grows its size a bit
> > because of that (and also because the DT might come from outside), but
> > SPL can not do that (and control DT does not generally come from outside
> > in SPL).
> >
> > That is why these few fixes actually repair the misuse, instead of
> > re-enabling the validation in libfdt.
> 
> Enable basic validation if !OF_REAL ? Create a new level of 'assume'
> for just what you need?
> 
> It just seems really odd to disable validation and then try to not
> call the functions in case they will crash. The next person who comes
> along will have an unexplained crash. It's just too brittle.

You seem to have missed the entire context of the problem here. Today,
in upstream exactly one platform disables validation in full U-Boot.
Exactly zero platforms enable any form of validation in xPL. Upstream
dtc has made a large number of changes since v1.4.6. Whatever we have
been assuming could be done cannot be done anymore.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20251121/bfcbc90f/attachment.sig>