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

[Simon Glass]

Hi Marek,

On Thu, 20 Nov 2025 at 04:56, Marek Vasut <marek.vasut at mailbox.org> wrote:
>
> On 11/20/25 10:08 AM, Heiko Schocher wrote:
>
> Hello Heiko,
>
> >> diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
> >> index 380a9f8f3ad..d0e9dad7202 100644
> >> --- a/drivers/i2c/i2c-uclass.c
> >> +++ b/drivers/i2c/i2c-uclass.c
> >> @@ -749,8 +749,11 @@ static int i2c_post_probe(struct udevice *dev)
> >>   #if CONFIG_IS_ENABLED(OF_REAL)
> >>       struct dm_i2c_bus *i2c = dev_get_uclass_priv(dev);
> >> -    i2c->speed_hz = dev_read_u32_default(dev, "clock-frequency",
> >> -                         I2C_SPEED_STANDARD_RATE);
> >> +    if (!dev_has_ofnode(dev))
> >> +        i2c->speed_hz = I2C_SPEED_STANDARD_RATE;
> >> +    else
> >> +        i2c->speed_hz = dev_read_u32_default(dev, "clock-frequency",
> >> +                             I2C_SPEED_STANDARD_RATE);
> >>       return dm_i2c_set_bus_speed(dev, i2c->speed_hz);
> >>   #else
> >>
> >
> > This sounds more as a bug of dev_read_u32_default() to me and should
> > be fixed there, else we need to make this check around each call of
> > dev_read_u32_default() ...
> >
> > And looking into u-boot:/drivers/core/read.c there are more
> > candidates for such a fix (all with "default" in function name?)
> >
> > and such a fix would also make the dev_has_ofnode(dev) check in
> > i2c_child_post_bind() obsolete.
> >
> > Or may we fix the default functions in drivers/core/ofnode.c ?
> No, this was the previous implementation which is now superseded by this
> one, see [1] [2]. The goal is to fix the entry points which pass invalid
> content further into U-Boot DT handling code, so the DT validation
> happens only one, on DT input, and not again later.

Do the libfdt functions work correctly with *any* negative numbers? Do
you have a pointer to the libfdt commit which changed this? All of
driver model relies on that behaviour in libfdt:

int fdt_check_node_offset_(const void *fdt, int offset)
{
if ((offset < 0) || (offset % FDT_TAGSIZE)
    || (fdt_next_tag(fdt, offset, &offset) != FDT_BEGIN_NODE))
return -FDT_ERR_BADOFFSET;

return offset;
}

(Note the check for offset < 0)

>
> Also, please wait for feedback on [3] [4] [5] [6] before possibly
> applying this one.

Regards,
Simon


>
> [1]
> https://lore.kernel.org/u-boot/20251113122145.949112-2-marek.vasut+renesas@mailbox.org/
> [2]
> https://lore.kernel.org/u-boot/20251113122145.949112-4-marek.vasut+renesas@mailbox.org/
>
> [3]
> https://lore.kernel.org/u-boot/20251120041439.817233-1-marek.vasut+renesas@mailbox.org/
> [4]
> https://lore.kernel.org/u-boot/20251120041504.817264-1-marek.vasut+renesas@mailbox.org/
> [5]
> https://lore.kernel.org/u-boot/20251120041526.817288-1-marek.vasut+renesas@mailbox.org/
> [6]
> https://lore.kernel.org/u-boot/20251120041613.817311-1-marek.vasut+renesas@mailbox.org/