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-oss@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-oss@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@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@weidmueller.com wrote: >> >> From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@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@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 sjg@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/Docu...
StefanRegards, Simon
Does this series need a rework to be applied?
Regards Stefan