[PATCH 1/2] dm: core: Add functions to read 8/16-bit integers

[Simon Glass]

Hi Stefan,

On Wed, 13 Jul 2022 at 10:08, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss at weidmueller.com> wrote:
>
> Hi Simon,
>
> Am 13.07.2022 um 17:28 schrieb Simon Glass:
> > Hi Stefan,
> >
> > On Tue, 12 Jul 2022 at 12:31, Stefan Herbrechtsmeier
> > <stefan.herbrechtsmeier-oss at weidmueller.com> wrote:
> >>
> >> Hi Simon,
> >>
> >> Am 12.07.2022 um 12:58 schrieb Simon Glass:
> >>> Hi Stefan,
> >>>
> >>> On Tue, 14 Jun 2022 at 07:22, Stefan Herbrechtsmeier
> >>> <stefan.herbrechtsmeier-oss at weidmueller.com> wrote:
> >>>>
> >>>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier at weidmueller.com>
> >>>>
> >>>> Add functions to read 8/16-bit integers like the existing functions for
> >>>> 32/64-bit to simplify read of 8/16-bit integers from device tree
> >>>> properties.
> >>>>
> >>>> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier at weidmueller.com>
> >>>> ---
> >>>>
> >>>>    arch/sandbox/dts/test.dts |  2 ++
> >>>>    drivers/core/of_access.c  | 38 +++++++++++++++++++++++
> >>>>    drivers/core/ofnode.c     | 62 +++++++++++++++++++++++++++++++++++++
> >>>>    drivers/core/read.c       | 21 +++++++++++++
> >>>>    include/dm/of_access.h    | 32 +++++++++++++++++++
> >>>>    include/dm/ofnode.h       | 40 ++++++++++++++++++++++++
> >>>>    include/dm/read.h         | 65 +++++++++++++++++++++++++++++++++++++++
> >>>>    test/dm/test-fdt.c        | 19 ++++++++++++
> >>>>    8 files changed, 279 insertions(+)
> >>>
> >>> This looks good but is very expensive in terms of code size. Can you
> >>> update your u8 and u16 functions to reuse the existing u32 function
> >>> and just convert the value?
> >>
> >> The u32 function requires a 32 bit value inside the device tree because
> >> it checks the size and maybe swap the bytes.
> >>
> >> The u8 and u16 function requires only a 8 and 16 bit value inside the
> >> device tree.
> >
> > Yes that's true. What is the use case for these functions?
>
> The usb251xb driver requires this functions [1]. The usb251xb device
> tree binding [2] defines the ids as 16 bit values and the Linux driver
> use 8 bit for an unspecified property. Without this changes the driver
> doesn't satisfy the specification and is incompatible to the Linux driver.

I wonder if that binding is a bit ambiguous. From what I have seen we
normally use a single cell for int values, partly so that fdtdump
works and partly because the format doesn't allow using any less space
anyway.

IMO that binding should use a whole cell for the byte and u16 values.

Then we can use the u32() function and do a mask.

Regards,
Simon



>
> [1] https://lists.denx.de/pipermail/u-boot/2022-June/486424.html
> [2]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/usb/usb251xb.txt
>
> Regards
>    Stefan