| Message ID | 20230601141119.1658720-1-john@metanate.com |
|---|---|
| State | Accepted |
| Commit | 77224320f05834344581f8b5d939907925876b3d |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | core: read: fix dev_read_addr_size() | expand |
On Thu, 1 Jun 2023 at 15:11, John Keeping <john@metanate.com> wrote: > > The behaviour of dev_read_addr_size() is surprising as it does not > handle #address-cells and #size-cells but instead hardcodes the values > based on sizeof(fdt_addr_t). > > This is different from dev_read_addr_size_index() and > dev_read_addr_size_name() both of which do read the cell sizes from the > device tree. > > Since dev_read_addr_size() is only used by a single driver and this > driver is broken when CONFIG_FDT_64BIT does not match the address size > in the device tree, fix the function to behave like all of the other > similarly named functions. Drop the property name argument as the only > caller passes "reg" and this is the expected property name matching the > other similarly named functions. > > Signed-off-by: John Keeping <john@metanate.com> > --- > drivers/core/read.c | 5 ++--- > drivers/reset/reset-rockchip.c | 2 +- > include/dm/read.h | 12 +++--------- > 3 files changed, 6 insertions(+), 13 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org> Tested-by: Simon Glass <sjg@chromium.org> # chromebook_jerry Tested-by: Simon Glass <sjg@chromium.org> # chromebook_bob
On Thu, 1 Jun 2023 at 15:11, John Keeping <john@metanate.com> wrote: > > The behaviour of dev_read_addr_size() is surprising as it does not > handle #address-cells and #size-cells but instead hardcodes the values > based on sizeof(fdt_addr_t). > > This is different from dev_read_addr_size_index() and > dev_read_addr_size_name() both of which do read the cell sizes from the > device tree. > > Since dev_read_addr_size() is only used by a single driver and this > driver is broken when CONFIG_FDT_64BIT does not match the address size > in the device tree, fix the function to behave like all of the other > similarly named functions. Drop the property name argument as the only > caller passes "reg" and this is the expected property name matching the > other similarly named functions. > > Signed-off-by: John Keeping <john@metanate.com> > --- > drivers/core/read.c | 5 ++--- > drivers/reset/reset-rockchip.c | 2 +- > include/dm/read.h | 12 +++--------- > 3 files changed, 6 insertions(+), 13 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org> Tested-by: Simon Glass <sjg@chromium.org> # chromebook_jerry Tested-by: Simon Glass <sjg@chromium.org> # chromebook_bob Applied to u-boot-dm, thanks!
diff --git a/drivers/core/read.c b/drivers/core/read.c index 0289a2edb6..5749473a6c 100644 --- a/drivers/core/read.c +++ b/drivers/core/read.c @@ -211,10 +211,9 @@ void *dev_remap_addr(const struct udevice *dev) return dev_remap_addr_index(dev, 0); } -fdt_addr_t dev_read_addr_size(const struct udevice *dev, const char *property, - fdt_size_t *sizep) +fdt_addr_t dev_read_addr_size(const struct udevice *dev, fdt_size_t *sizep) { - return ofnode_get_addr_size(dev_ofnode(dev), property, sizep); + return dev_read_addr_size_index(dev, 0, sizep); } const char *dev_read_name(const struct udevice *dev) diff --git a/drivers/reset/reset-rockchip.c b/drivers/reset/reset-rockchip.c index 2ebe3382f7..6cabaa10a3 100644 --- a/drivers/reset/reset-rockchip.c +++ b/drivers/reset/reset-rockchip.c @@ -97,7 +97,7 @@ static int rockchip_reset_probe(struct udevice *dev) fdt_addr_t addr; fdt_size_t size; - addr = dev_read_addr_size(dev, "reg", &size); + addr = dev_read_addr_size(dev, &size); if (addr == FDT_ADDR_T_NONE) return -EINVAL; diff --git a/include/dm/read.h b/include/dm/read.h index 56ac076c9f..137f2a52a2 100644 --- a/include/dm/read.h +++ b/include/dm/read.h @@ -347,18 +347,13 @@ fdt_addr_t dev_read_addr_pci(const struct udevice *dev); void *dev_remap_addr(const struct udevice *dev); /** - * dev_read_addr_size() - get address and size from a device property - * - * This does no address translation. It simply reads an property that contains - * an address and a size value, one after the other. + * dev_read_addr_size() - Get the reg property of a device * * @dev: Device to read from - * @propname: property to read * @sizep: place to put size value (on success) * Return: address value, or FDT_ADDR_T_NONE on error */ -fdt_addr_t dev_read_addr_size(const struct udevice *dev, const char *propname, - fdt_size_t *sizep); +fdt_addr_t dev_read_addr_size(const struct udevice *dev, fdt_size_t *sizep); /** * dev_read_name() - get the name of a device's node @@ -1002,10 +997,9 @@ static inline void *dev_remap_addr_name(const struct udevice *dev, } static inline fdt_addr_t dev_read_addr_size(const struct udevice *dev, - const char *propname, fdt_size_t *sizep) { - return ofnode_get_addr_size(dev_ofnode(dev), propname, sizep); + return dev_read_addr_size_index(dev, 0, sizep); } static inline const char *dev_read_name(const struct udevice *dev)
The behaviour of dev_read_addr_size() is surprising as it does not handle #address-cells and #size-cells but instead hardcodes the values based on sizeof(fdt_addr_t). This is different from dev_read_addr_size_index() and dev_read_addr_size_name() both of which do read the cell sizes from the device tree. Since dev_read_addr_size() is only used by a single driver and this driver is broken when CONFIG_FDT_64BIT does not match the address size in the device tree, fix the function to behave like all of the other similarly named functions. Drop the property name argument as the only caller passes "reg" and this is the expected property name matching the other similarly named functions. Signed-off-by: John Keeping <john@metanate.com> --- drivers/core/read.c | 5 ++--- drivers/reset/reset-rockchip.c | 2 +- include/dm/read.h | 12 +++--------- 3 files changed, 6 insertions(+), 13 deletions(-)