[U-Boot] [PATCH v4 0/5] of-platdata: Avoid building libfdt
The original patch of this series was sent in September but unfortunately caused build problems on some boards, since they don't comply with the of-platdata rules.
With of-platdata, the idea is to compile the device tree into C structures to save space and avoid needing to use libfdt. But some boards use of-platdata while also using libfdt in a few areas, thus defeating the purpose of of-platdata.
This series includes the original two patches
http://patchwork.ozlabs.org/patch/1167420/ http://patchwork.ozlabs.org/patch/1167367/
as well as a few other patches to fix the build errors. Overall this reduces code size and provides better error messages when unavailable functions are used.
Board maintainers should still take a look at the result, adjusting the of-platdata support as needed.
Changes in v4: - Add new patch for rockchip build errors - Add new patch for omap MMC build errors - Add new patch for rockchip chromebook build errors - Pull out patches into a new series - Add new patches to handle build failures
Changes in v3: - Fix eth_dev_get_mac_address() call dev_read...() only when available
Simon Glass (5): rockchip: Avoid using libfdt with of-platdata omap: mmc: Avoid using libfdt with of-platdata rockchip: pinctrl: Disable full pinctrl for SPL dm: core: Don't include ofnode functions with of-platdata spl: Allow SPL/TPL to use of-platdata without libfdt
configs/chromebit_mickey_defconfig | 1 + configs/chromebook_jerry_defconfig | 1 + configs/chromebook_minnie_defconfig | 1 + configs/chromebook_speedy_defconfig | 1 + drivers/clk/rockchip/clk_rk3328.c | 14 ++++++++++++-- drivers/core/Makefile | 4 +++- drivers/mmc/davinci_mmc.c | 6 ++++++ drivers/pinctrl/rockchip/pinctrl-rockchip-core.c | 6 ++++-- include/dm/read.h | 3 +-- lib/Kconfig | 4 ++-- net/eth-uclass.c | 2 +- 11 files changed, 33 insertions(+), 10 deletions(-)
At present a few of the clk and pinctrl drivers use libfdt routines (via dev_read_..()) when of-platdata is enabled. This is not permitted.
Correct this by returning errors instead. The drivers may need to be modified to add full support.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v4: - Add new patch for rockchip build errors
Changes in v3: None
drivers/clk/rockchip/clk_rk3328.c | 14 ++++++++++++-- drivers/pinctrl/rockchip/pinctrl-rockchip-core.c | 6 ++++-- 2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/rockchip/clk_rk3328.c b/drivers/clk/rockchip/clk_rk3328.c index a89e2ecc4a..e86c17e6d6 100644 --- a/drivers/clk/rockchip/clk_rk3328.c +++ b/drivers/clk/rockchip/clk_rk3328.c @@ -669,6 +669,10 @@ static int rk3328_gmac2io_set_parent(struct clk *clk, struct clk *parent) return 0; }
+ /* FIXME: Device tree should be read in ofdata_to_platdata() */ + if (CONFIG_IS_ENABLED(OF_PLATDATA)) + return -EDEADLK; + /* * Otherwise, we need to check the clock-output-names of the * requested parent to see if the requested id is "gmac_clkin". @@ -706,6 +710,10 @@ static int rk3328_gmac2io_ext_set_parent(struct clk *clk, struct clk *parent) return 0; }
+ /* FIXME: Device tree should be read in ofdata_to_platdata() */ + if (CONFIG_IS_ENABLED(OF_PLATDATA)) + return -EDEADLK; + /* * Otherwise, we need to check the clock-output-names of the * requested parent to see if the requested id is "gmac_clkin". @@ -762,9 +770,11 @@ static int rk3328_clk_probe(struct udevice *dev)
static int rk3328_clk_ofdata_to_platdata(struct udevice *dev) { - struct rk3328_clk_priv *priv = dev_get_priv(dev); + if (!CONFIG_IS_ENABLED(OF_PLATDATA)) { + struct rk3328_clk_priv *priv = dev_get_priv(dev);
- priv->cru = dev_read_addr_ptr(dev); + priv->cru = dev_read_addr_ptr(dev); + }
return 0; } diff --git a/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c b/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c index 80dc431d20..dccc54e95f 100644 --- a/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c +++ b/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c @@ -383,8 +383,8 @@ static int rockchip_pinconf_prop_name_to_param(const char *property, return -EPERM; }
-static int rockchip_pinctrl_set_state(struct udevice *dev, - struct udevice *config) +static int __maybe_unused rockchip_pinctrl_set_state(struct udevice *dev, + struct udevice *config) { struct rockchip_pinctrl_priv *priv = dev_get_priv(dev); struct rockchip_pin_ctrl *ctrl = priv->ctrl; @@ -474,7 +474,9 @@ static int rockchip_pinctrl_set_state(struct udevice *dev, }
const struct pinctrl_ops rockchip_pinctrl_ops = { +#if !CONFIG_IS_ENABLED(PLATDATA) .set_state = rockchip_pinctrl_set_state, +#endif .get_gpio_mux = rockchip_pinctrl_get_gpio_mux, };
At present this driver is enabled in SPL on omapl138_lcdk, which uses of-platdata. The driver needs to be ported to use of-platdata properly. For now, avoid a build error by returning an error.
Signed-off-by: Simon Glass sjg@chromium.org
---
Changes in v4: - Add new patch for omap MMC build errors
Changes in v3: None
drivers/mmc/davinci_mmc.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/mmc/davinci_mmc.c b/drivers/mmc/davinci_mmc.c index 0d63279db0..79a7f50d25 100644 --- a/drivers/mmc/davinci_mmc.c +++ b/drivers/mmc/davinci_mmc.c @@ -507,6 +507,12 @@ static int davinci_mmc_probe(struct udevice *dev) priv->version = data->version; }
+ /* FIXME: Cannot read from device tree with of-platdata */ + if (CONFIG_IS_ENABLED(OF_PLATDATA)) { + printf("Please fix this driver to use of-platdata"); + return -ENOSYS; + } + priv->reg_base = (struct davinci_mmc_regs *)dev_read_addr(dev); priv->input_clk = clk_get(DAVINCI_MMCSD_CLKID);
On Thu, Nov 07, 2019 at 08:53:09AM -0700, Simon Glass wrote:
At present this driver is enabled in SPL on omapl138_lcdk, which uses of-platdata. The driver needs to be ported to use of-platdata properly. For now, avoid a build error by returning an error.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v4:
- Add new patch for omap MMC build errors
Changes in v3: None
drivers/mmc/davinci_mmc.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/mmc/davinci_mmc.c b/drivers/mmc/davinci_mmc.c index 0d63279db0..79a7f50d25 100644 --- a/drivers/mmc/davinci_mmc.c +++ b/drivers/mmc/davinci_mmc.c @@ -507,6 +507,12 @@ static int davinci_mmc_probe(struct udevice *dev) priv->version = data->version; }
- /* FIXME: Cannot read from device tree with of-platdata */
- if (CONFIG_IS_ENABLED(OF_PLATDATA)) {
printf("Please fix this driver to use of-platdata");return -ENOSYS;- }
- priv->reg_base = (struct davinci_mmc_regs *)dev_read_addr(dev); priv->input_clk = clk_get(DAVINCI_MMCSD_CLKID);
Let me add the board maintainer here. Peter, are we even using MMC in SPL on the omapl138_lcdk? If so, I believe we need to add platdata ala other platforms like board/ti/am335x/board.c for example. Thanks!
On Thu, 2019-11-07 at 15:05 -0500, Tom Rini wrote:
On Thu, Nov 07, 2019 at 08:53:09AM -0700, Simon Glass wrote:
At present this driver is enabled in SPL on omapl138_lcdk, which uses of-platdata. The driver needs to be ported to use of-platdata properly. For now, avoid a build error by returning an error.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v4:
- Add new patch for omap MMC build errors
Changes in v3: None
drivers/mmc/davinci_mmc.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/mmc/davinci_mmc.c b/drivers/mmc/davinci_mmc.c index 0d63279db0..79a7f50d25 100644 --- a/drivers/mmc/davinci_mmc.c +++ b/drivers/mmc/davinci_mmc.c @@ -507,6 +507,12 @@ static int davinci_mmc_probe(struct udevice *dev) priv->version = data->version; }
- /* FIXME: Cannot read from device tree with of-platdata */
- if (CONFIG_IS_ENABLED(OF_PLATDATA)) {
printf("Please fix this driver to use of-platdata");return -ENOSYS;- }
- priv->reg_base = (struct davinci_mmc_regs *)dev_read_addr(dev); priv->input_clk = clk_get(DAVINCI_MMCSD_CLKID);
Let me add the board maintainer here.
Re-replying (and not from %&$! Outlook) after having a look at the situation.
Peter, are we even using MMC in SPL on the omapl138_lcdk?
The OMAP L138 LCDK can boot from MMC - so the MMC driver is used inthe SPL. However, going back over the patches from this year, MMC usage was broken back in July - refer https://patchwork.ozlabs.org/patch/1138200/
Bartosz is working on this but has hit problems.
Given that MMC access in the SPL is effectively broken right now, I don't see that _this_ patch makes things any worse, so if this is the only problem with the series I'd let it go through. Bartosz, Adam: Given you were discussing this at almost the same time Tom sent his email, do you have any problem with this patch being applied. If we solve the overall problem with the davinci driver, adding the platdata is trivial.
Peter
If so, I believe we need to add platdata ala other platforms like board/ti/am335x/board.c for example. Thanks!
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
On Sun, Nov 10, 2019 at 1:22 AM Peter Howard pjh@northern-ridge.com.au wrote:
On Thu, 2019-11-07 at 15:05 -0500, Tom Rini wrote:
On Thu, Nov 07, 2019 at 08:53:09AM -0700, Simon Glass wrote:
At present this driver is enabled in SPL on omapl138_lcdk, which uses of-platdata. The driver needs to be ported to use of-platdata properly. For now, avoid a build error by returning an error.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v4:
- Add new patch for omap MMC build errors
Changes in v3: None
drivers/mmc/davinci_mmc.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/mmc/davinci_mmc.c b/drivers/mmc/davinci_mmc.c index 0d63279db0..79a7f50d25 100644 --- a/drivers/mmc/davinci_mmc.c +++ b/drivers/mmc/davinci_mmc.c @@ -507,6 +507,12 @@ static int davinci_mmc_probe(struct udevice *dev) priv->version = data->version; }
- /* FIXME: Cannot read from device tree with of-platdata */
- if (CONFIG_IS_ENABLED(OF_PLATDATA)) {
printf("Please fix this driver to use of-platdata");return -ENOSYS;- }
- priv->reg_base = (struct davinci_mmc_regs *)dev_read_addr(dev); priv->input_clk = clk_get(DAVINCI_MMCSD_CLKID);
Let me add the board maintainer here.
Re-replying (and not from %&$! Outlook) after having a look at the situation.
Peter, are we even using MMC in SPL on the omapl138_lcdk?
The OMAP L138 LCDK can boot from MMC - so the MMC driver is used inthe SPL. However, going back over the patches from this year, MMC usage was broken back in July - refer https://patchwork.ozlabs.org/patch/1138200/
Bartosz is working on this but has hit problems.
Given that MMC access in the SPL is effectively broken right now, I don't see that _this_ patch makes things any worse, so if this is the only problem with the series I'd let it go through. Bartosz, Adam: Given you were discussing this at almost the same time Tom sent his email, do you have any problem with this patch being applied. If we solve the overall problem with the davinci driver, adding the platdata is trivial.
I don't have an issue on the surface, but if we have a known regression (ie, the converstion to DM in SPL) that breaks functionality, I think it should be reverted until it's addressed. I know the da850-0evm can boot using the device tree, but it doesn't use OF_PLATDATA. It also doesn't use MMC as a boot source, because it doesn't have the ability to select the proper boot pin combination. I can also see that by not using OF_PLATDATA, the SPL code is too large for the use on the LCDK.
When I look at the source for the lcdk, it looks like it has manual driver info in the board file instead of using the auto-generated stuff that should be produced as part of using OF_PLATDATA. I wonder if the FIXME should be applied to more than just the MMC driver.
adam
Peter
If so, I believe we need to add platdata ala other platforms like board/ti/am335x/board.c for example. Thanks!
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
On Sun, Nov 10, 2019 at 5:08 AM Adam Ford aford173@gmail.com wrote:
On Sun, Nov 10, 2019 at 1:22 AM Peter Howard pjh@northern-ridge.com.au wrote:
On Thu, 2019-11-07 at 15:05 -0500, Tom Rini wrote:
On Thu, Nov 07, 2019 at 08:53:09AM -0700, Simon Glass wrote:
At present this driver is enabled in SPL on omapl138_lcdk, which uses of-platdata. The driver needs to be ported to use of-platdata properly. For now, avoid a build error by returning an error.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v4:
- Add new patch for omap MMC build errors
Changes in v3: None
drivers/mmc/davinci_mmc.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/mmc/davinci_mmc.c b/drivers/mmc/davinci_mmc.c index 0d63279db0..79a7f50d25 100644 --- a/drivers/mmc/davinci_mmc.c +++ b/drivers/mmc/davinci_mmc.c @@ -507,6 +507,12 @@ static int davinci_mmc_probe(struct udevice *dev) priv->version = data->version; }
- /* FIXME: Cannot read from device tree with of-platdata */
- if (CONFIG_IS_ENABLED(OF_PLATDATA)) {
printf("Please fix this driver to use of-platdata");return -ENOSYS;- }
- priv->reg_base = (struct davinci_mmc_regs *)dev_read_addr(dev); priv->input_clk = clk_get(DAVINCI_MMCSD_CLKID);
Let me add the board maintainer here.
Re-replying (and not from %&$! Outlook) after having a look at the situation.
Peter, are we even using MMC in SPL on the omapl138_lcdk?
The OMAP L138 LCDK can boot from MMC - so the MMC driver is used inthe SPL. However, going back over the patches from this year, MMC usage was broken back in July - refer https://patchwork.ozlabs.org/patch/1138200/
Bartosz is working on this but has hit problems.
Given that MMC access in the SPL is effectively broken right now, I don't see that _this_ patch makes things any worse, so if this is the only problem with the series I'd let it go through. Bartosz, Adam: Given you were discussing this at almost the same time Tom sent his email, do you have any problem with this patch being applied. If we solve the overall problem with the davinci driver, adding the platdata is trivial.
I don't have an issue on the surface, but if we have a known regression (ie, the converstion to DM in SPL) that breaks functionality, I think it should be reverted until it's addressed. I know the da850-0evm can boot using the device tree, but it doesn't use OF_PLATDATA. It also doesn't use MMC as a boot source, because it doesn't have the ability to select the proper boot pin combination. I can also see that by not using OF_PLATDATA, the SPL code is too large for the use on the LCDK.
I just submitted 4 patches this morning for the omapl138_lcdk. This gets it 90% of the way to supporting device tree. For some reason, the last chunk is causing me issues.
As of now the 4 patches I sent should still have the board booting from 'master' but there is more to do to get it to fully boot from device tree without OF_PLATDATA.
adam
When I look at the source for the lcdk, it looks like it has manual driver info in the board file instead of using the auto-generated stuff that should be produced as part of using OF_PLATDATA. I wonder if the FIXME should be applied to more than just the MMC driver.
adam
Peter
If so, I believe we need to add platdata ala other platforms like board/ti/am335x/board.c for example. Thanks!
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
We don't need full pinctrl for SPL on these chrombook devices. Disable it so that of-platdata can be used without calling libfdt routines.
Signed-off-by: Simon Glass sjg@chromium.org
---
Changes in v4: - Add new patch for rockchip chromebook build errors
Changes in v3: None
configs/chromebit_mickey_defconfig | 1 + configs/chromebook_jerry_defconfig | 1 + configs/chromebook_minnie_defconfig | 1 + configs/chromebook_speedy_defconfig | 1 + 4 files changed, 4 insertions(+)
diff --git a/configs/chromebit_mickey_defconfig b/configs/chromebit_mickey_defconfig index 6a1ea049f9..2e93344bda 100644 --- a/configs/chromebit_mickey_defconfig +++ b/configs/chromebit_mickey_defconfig @@ -72,6 +72,7 @@ CONFIG_SPI_FLASH_GIGADEVICE=y CONFIG_PINCTRL=y CONFIG_PINCONF=y CONFIG_SPL_PINCTRL=y +# CONFIG_SPL_PINCTRL_FULL is not set # CONFIG_SPL_PINMUX is not set CONFIG_SPL_PINCONF=y CONFIG_DM_PMIC=y diff --git a/configs/chromebook_jerry_defconfig b/configs/chromebook_jerry_defconfig index 1b7751cc6a..2ed9921e4e 100644 --- a/configs/chromebook_jerry_defconfig +++ b/configs/chromebook_jerry_defconfig @@ -75,6 +75,7 @@ CONFIG_SPI_FLASH_GIGADEVICE=y CONFIG_PINCTRL=y CONFIG_PINCONF=y CONFIG_SPL_PINCTRL=y +# CONFIG_SPL_PINCTRL_FULL is not set # CONFIG_SPL_PINMUX is not set CONFIG_SPL_PINCONF=y CONFIG_DM_PMIC=y diff --git a/configs/chromebook_minnie_defconfig b/configs/chromebook_minnie_defconfig index 28ae61847f..c62ebaca31 100644 --- a/configs/chromebook_minnie_defconfig +++ b/configs/chromebook_minnie_defconfig @@ -74,6 +74,7 @@ CONFIG_SPI_FLASH_GIGADEVICE=y CONFIG_PINCTRL=y CONFIG_PINCONF=y CONFIG_SPL_PINCTRL=y +# CONFIG_SPL_PINCTRL_FULL is not set # CONFIG_SPL_PINMUX is not set CONFIG_SPL_PINCONF=y CONFIG_DM_PMIC=y diff --git a/configs/chromebook_speedy_defconfig b/configs/chromebook_speedy_defconfig index 0284e31bcf..fd707e283d 100644 --- a/configs/chromebook_speedy_defconfig +++ b/configs/chromebook_speedy_defconfig @@ -74,6 +74,7 @@ CONFIG_SPI_FLASH_GIGADEVICE=y CONFIG_PINCTRL=y CONFIG_PINCONF=y CONFIG_SPL_PINCTRL=y +# CONFIG_SPL_PINCTRL_FULL is not set CONFIG_DM_PMIC=y # CONFIG_SPL_PMIC_CHILDREN is not set CONFIG_PMIC_RK8XX=y
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@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 * 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);
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@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
Hi Walter,
On Thu, 7 Nov 2019 at 12:47, Walter Lozano walter.lozano@collabora.com wrote:
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@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)?
Well if a U_BOOT_DEVICE is used (as you say, not in mainline), then dev_read_...() cannot be used anyway, since there is no device-tree node.
My goal here is to cause a compile/link error showing the code that calls dev_read_...(). At present the error shows up in this header instead, which is not very helpful.
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)
Here's my thinking:
It is an error to call it with of-platdata, so my cause is to cause an error (due to the unavailability of dev_read...() functions).
That way people see at build-time that they are doing something wrong.
If we change these to avoid the problem at build time, then it becomes a runtime problem....
Regards, Simon
Hi Simon
On 29/12/19 22:21, Simon Glass wrote:
Hi Walter,
On Thu, 7 Nov 2019 at 12:47, Walter Lozano walter.lozano@collabora.com wrote:
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@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)?
Well if a U_BOOT_DEVICE is used (as you say, not in mainline), then dev_read_...() cannot be used anyway, since there is no device-tree node.
My goal here is to cause a compile/link error showing the code that calls dev_read_...(). At present the error shows up in this header instead, which is not very helpful.
I now understand your point, thanks for your clarification.
I think that your approach will be help to improve the OF_PLATDATA support, which is great, however I'm still looking for a nice way to allow to take advantage of this kind of improvements when using U_BOOT_DEVICE. If I get some time I will work on this and ask for your review.
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)
Here's my thinking:
It is an error to call it with of-platdata, so my cause is to cause an error (due to the unavailability of dev_read...() functions).
That way people see at build-time that they are doing something wrong.
If we change these to avoid the problem at build time, then it becomes a runtime problem....
Yes, you are right, thanks for your explanation.
Regards,
Regards, Simon
At present libfdt is included in SPL/TPL if SPL/TPL_OF_CONTROL is enabled. But if of-platdata is in use this is not required. Update the condition to avoid building this extra code. This ensures that if a libfdt function is used it will produce a link error rather than silently increasing the build size.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v4: - Pull out patches into a new series - Add new patches to handle build failures
Changes in v3: None
lib/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/Kconfig b/lib/Kconfig index b8a8509d72..1cae2d5cc8 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -484,7 +484,7 @@ config OF_LIBFDT_OVERLAY
config SPL_OF_LIBFDT bool "Enable the FDT library for SPL" - default y if SPL_OF_CONTROL + default y if SPL_OF_CONTROL && !SPL_OF_PLATDATA help This enables the FDT library (libfdt). It provides functions for accessing binary device tree images in memory, such as adding and @@ -505,7 +505,7 @@ config SPL_OF_LIBFDT_ASSUME_MASK
config TPL_OF_LIBFDT bool "Enable the FDT library for TPL" - default y if TPL_OF_CONTROL + default y if TPL_OF_CONTROL && !TPL_OF_PLATDATA help This enables the FDT library (libfdt). It provides functions for accessing binary device tree images in memory, such as adding and
participants (5)
-
Adam Ford -
Peter Howard -
Simon Glass -
Tom Rini -
Walter Lozano