On Wed, Nov 04, 2020 at 11:29:28AM -0600, Rob Herring wrote:
> On Wed, Nov 4, 2020 at 7:46 AM Tom Rini <trini@xxxxxxxxxxxx> wrote:
> >
> > Commits 6dcb8ba4 "libfdt: Add helpers for accessing unaligned words"
> > introduced changes to support unaligned reads for ARM platforms and
> > 11738cf01f15 "libfdt: Don't use memcpy to handle unaligned reads on ARM"
> > improved the performance of these helpers.
> >
> > On further discussion, while there are potential cases where we could be
> > used on platforms that do not fixup unaligned reads for us, making this
> > choice the default is very expensive in terms of binary size and access
> > time. To address this, suffix the existing fdt{32,64}_ld functions with
> > _unaligned and introduce new load functions that call
> > fdt{32,64}_to_cpu() as was done prior to the above mentioned commits.
> >
> > Signed-off-by: Tom Rini <trini@xxxxxxxxxxxx>
> > ---
> > libfdt/libfdt.h | 21 +++++++++++++++------
> > 1 file changed, 15 insertions(+), 6 deletions(-)
> >
> > diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> > index b600c8d6dd41..307ba745c92f 100644
> > --- a/libfdt/libfdt.h
> > +++ b/libfdt/libfdt.h
> > @@ -126,13 +126,22 @@ static inline void *fdt_offset_ptr_w(void *fdt, int offset, int checklen)
> > uint32_t fdt_next_tag(const void *fdt, int offset, int *nextoffset);
> >
> > /*
> > - * Alignment helpers:
> > - * These helpers access words from a device tree blob. They're
> > - * built to work even with unaligned pointers on platforms (ike
> > - * ARM) that don't like unaligned loads and stores
> > + * Load functions. Assume that we are on a platform where unaligned memory
> > + * reads will be handled in a graceful manner and that we do not need to ensure
> > + * our reads are aligned. If this is not the case there are _unaligned
> > + * versions of these functions that follow and can be used.
> > */
> > -
> > static inline uint32_t fdt32_ld(const fdt32_t *p)
> > +{
> > + return fdt32_to_cpu(*p);
>
> This changes the public behavior of fdt32_ld() which is one of the
> things David was against.
>
> I think we want a _fdt32_ld or fdt32_ld_internal or ?? which is
> internal only and doesn't create another ABI.
I thought it was minimal code change? But anyhow, yes, I can easily
respin things. Should I do that now or wait for David's comments?
--
Tom
