Hi Stefan, On Thu, 14 Jul 2022 at 04:37, Stefan Herbrechtsmeier <stefan.herbrechtsmeier-...@weidmueller.com> wrote: > > Hi Simon, > > Am 14.07.2022 um 12:22 schrieb Simon Glass: > > Hi Stefan, > > > > On Wed, 13 Jul 2022 at 10:08, Stefan Herbrechtsmeier > > <stefan.herbrechtsmeier-...@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-...@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-...@weidmueller.com> wrote: > >>>>>> > >>>>>> From: Stefan Herbrechtsmeier <stefan.herbrechtsme...@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.herbrechtsme...@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. > > How should we go on? The specification is 5 years old. I can ignore the > specification and remove the "/bits/ 16" from my device tree source.
I believe either would work, since reading a u32 from the device tree and masking it would achieve the same result. The problem with the current u32 function is that it requires 4 bytes. IMO the binding is odd, but I don't think you can just change it. > > > Then we can use the u32() function and do a mask. > > The driver cast away the unseeded bits. OK. Anyway as Heinrich says below, most boards won't use these functions. I hope that other drivers don't start using u8 and u16 values when a u32 will do. Reviewed-by: Simon Glass <s...@chromium.org> > > >> > >> [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 > Stefan > Regards, Simon