[U-Boot] [PATCH v4 4/5] dm: core: Don't include ofnode functions with of-platdata

[Walter Lozano]

Hi Simon,

Thanks for your patch.

On 7/11/19 12:53, Simon Glass wrote:
> These functions cannot work with of-platdata since libfdt is not
> available. At present when dev_read_...() functions are used it produces
> error messages about ofnode which is confusing.
>
> Adjust the Makefile and header to produce an error message for the actual
> dev_read...() function which is called. This makes it easier to see what
> code needs to be converted for use with of-platdata.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> Changes in v4: None
> Changes in v3:
> - Fix eth_dev_get_mac_address() call dev_read...() only when available
>
>   drivers/core/Makefile | 4 +++-
>   include/dm/read.h     | 3 +--
>   net/eth-uclass.c      | 2 +-
>   3 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/core/Makefile b/drivers/core/Makefile
> index bce7467da1..b9e4a2aab1 100644
> --- a/drivers/core/Makefile
> +++ b/drivers/core/Makefile
> @@ -13,6 +13,8 @@ obj-$(CONFIG_OF_LIVE) += of_access.o of_addr.o
>   ifndef CONFIG_DM_DEV_READ_INLINE
>   obj-$(CONFIG_OF_CONTROL) += read.o
>   endif
> -obj-$(CONFIG_OF_CONTROL) += of_extra.o ofnode.o read_extra.o
> +ifdef CONFIG_$(SPL_TPL_)OF_LIBFDT
> +obj-$(CONFIG_$(SPL_TPL_)OF_CONTROL) += of_extra.o ofnode.o read_extra.o
> +endif
>   
>   ccflags-$(CONFIG_DM_DEBUG) += -DDEBUG
> diff --git a/include/dm/read.h b/include/dm/read.h
> index d37fcb504d..4f02d07d00 100644
> --- a/include/dm/read.h
> +++ b/include/dm/read.h
> @@ -43,8 +43,7 @@ static inline bool dev_of_valid(struct udevice *dev)
>   	return ofnode_valid(dev_ofnode(dev));
>   }
>   
> -#ifndef CONFIG_DM_DEV_READ_INLINE
> -
> +#if !defined(CONFIG_DM_DEV_READ_INLINE) || CONFIG_IS_ENABLED(OF_PLATDATA)
>   /**
>    * dev_read_u32() - read a 32-bit integer from a device's DT property
>    *


I don't know if it has much sense, but as I understand it should be 
possible to use DM without OF_CONTROL by adding U_BOOT_DEVICE entries 
manually in a board file. Probably this won't be useful in mainline but 
still could be useful in some contexts. If this is true maybe this 
condition should be changed. In other words why not use 
!CONFIG_IS_ENABLED(CONFIG_OF_LIBFDT) instead of 
CONFIG_IS_ENABLED(OF_PLATDATA)?


> diff --git a/net/eth-uclass.c b/net/eth-uclass.c
> index 3bd98b01ad..e3bfcdb6cc 100644
> --- a/net/eth-uclass.c
> +++ b/net/eth-uclass.c
> @@ -462,7 +462,7 @@ static int eth_pre_unbind(struct udevice *dev)
>   
>   static bool eth_dev_get_mac_address(struct udevice *dev, u8 mac[ARP_HLEN])
>   {
> -#if IS_ENABLED(CONFIG_OF_CONTROL)
> +#if CONFIG_IS_ENABLED(OF_CONTROL)
>   	const uint8_t *p;
>   
>   	p = dev_read_u8_array_ptr(dev, "mac-address", ARP_HLEN);


Should this kind of #if be changed to

#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)

Regards,

Walter