[U-Boot] [PATCH] core: of_addr: Correct the size type of of_get_address to fdt_size_t

[Simon Glass]

Hi,

On Mon, 26 Aug 2019 at 11:16, Eugeniu Rosca <erosca at de.adit-jv.com> wrote:
>
> Hi Keerthy,
> Hi Simon,
> cc: Tom
>
> On Tue, Aug 20, 2019 at 09:39:32AM +0530, Keerthy wrote:
> > On 19/08/19 3:54 PM, Eugeniu Rosca wrote:
> [..]
> > > I took some time to also review the changes in addition to testing.
> > >
> > > I can see that, since its inception in Linux [1], of_get_address() used
> > > 'u64*' type for its 'size' argument. That's still valid in v5.3-rc5.
> > > So, it looks to me that with this patch we diverge from Linux.
> > >
> > > I would barely think that the ASAN issue being fixed in this patch
> > > exists in Linux, since the latter receives much more KASAN-enabled
> > > testing on regular basis.
> > >
> > > Do you foresee any alternative fix w/o diverging from Linux?
> >
> > I am afraid No but isn't fdt_size_t also right type to represent size?
>
> 'fdt_size_t' is a U-Boot-ism, i.e. it exists nowhere else but in U-Boot.
> Just for the record, it appears to be added by Simon's v2013.04 commit
> 4397a2a80baef ("fdt: Add fdtdec_get_addr_size() to read reg properties")
>
> IMHO injecting a U-Boot-specific data type into the prototype of a
> Linux-backported function has below major drawbacks:
>
>  - It is a non-upstream-able solution. Linux will unlikely accept
>    'fdt_size_t' as a new type simply b/c Linux OF code existed over
>    a decade and it didn't need this type so far.
>  - Mutilating Linux upstream code will make further U-Boot backports
>    painful, time consuming and error-prone.
>
> Are we on the same page here?
>
> > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=22ae782f86b7
> > >
>

Yes I agree.

What do people think about this approach?

https://gitlab.denx.de/u-boot/custodians/u-boot-dm/commit/82c65a88284c27a02d0958d8a5354390893b172a

Regards,
SImon