Hi,

Am 14.07.2022 um 16:51 schrieb Simon Glass:
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

Does this series need a rework to be applied?

Regards
  Stefan

Reply via email to