Re: [PATCH v2 1/1] clk: clk-gpio: add actual gated clock
On 12/16/23 19:19, Marek Vasut wrote: On 12/16/23 17:56, Sean Anderson wrote: On 12/16/23 11:52, Marek Vasut wrote: On 12/16/23 16:37, Sean Anderson wrote: On 12/16/23 03:48, Svyatoslav Ryhel wrote: Existing gpio-gate-clock driver acts like a simple GPIO switch without any effect on gated clock. Add actual clock actions into enable/disable ops and implement get_rate op by passing gated clock if it is enabled. Signed-off-by: Svyatoslav Ryhel --- drivers/clk/clk-gpio.c | 44 ++ 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/drivers/clk/clk-gpio.c b/drivers/clk/clk-gpio.c index 26d795b978..72d9747a47 100644 --- a/drivers/clk/clk-gpio.c +++ b/drivers/clk/clk-gpio.c @@ -3,19 +3,23 @@ * Copyright (C) 2023 Marek Vasut */ -#include -#include -#include +#include #include +#include +#include + +#include struct clk_gpio_priv { - struct gpio_desc enable; + struct gpio_desc enable; /* GPIO, controlling the gate */ + struct clk *clk; /* Gated clock */ }; static int clk_gpio_enable(struct clk *clk) { struct clk_gpio_priv *priv = dev_get_priv(clk->dev); + clk_enable(priv->clk); dm_gpio_set_value(>enable, 1); return 0; @@ -26,21 +30,45 @@ static int clk_gpio_disable(struct clk *clk) struct clk_gpio_priv *priv = dev_get_priv(clk->dev); dm_gpio_set_value(>enable, 0); + clk_disable(priv->clk); return 0; } +static ulong clk_gpio_get_rate(struct clk *clk) +{ + struct clk_gpio_priv *priv = dev_get_priv(clk->dev); + + return clk_get_rate(priv->clk); +} + const struct clk_ops clk_gpio_ops = { .enable = clk_gpio_enable, .disable = clk_gpio_disable, + .get_rate = clk_gpio_get_rate, }; -static int clk_gpio_probe(struct udevice *dev) +static int clk_gpio_of_to_plat(struct udevice *dev) { struct clk_gpio_priv *priv = dev_get_priv(dev); + int ret; - return gpio_request_by_name(dev, "enable-gpios", 0, - >enable, GPIOD_IS_OUT); + priv->clk = devm_clk_get(dev, NULL); + if (IS_ERR(priv->clk)) { + log_debug("%s: Could not get gated clock: %ld\n", + __func__, PTR_ERR(priv->clk)); + return PTR_ERR(priv->clk); + } + + ret = gpio_request_by_name(dev, "enable-gpios", 0, + >enable, GPIOD_IS_OUT); + if (ret) { + log_debug("%s: Could not decode enable-gpios (%d)\n", + __func__, ret); + return ret; + } + + return 0; All this forwarding of clock enable/disable/get_rate for parent clock should already be done in clock core. If it is not, then it should be fixed in clock core. Only CCF forwards things. It's up to the driver to do it in the non-CCF case. Shall we add dependency on CCF instead of duplicating code in drivers ? I'd rather not. This will be useful on non-CCF systems. --Sean
[PATCH v2 RESEND] pci: Enable dm_pci_map_bar() for 64-bit BARs
Allow dm_pci_map_bar() usage on systems with CONFIG_SYS_PCI_64BIT. Reviewed-by: Philip Oberfichtner Reviewed-by: Simon Glass Signed-off-by: Moritz Fischer --- drivers/pci/pci-uclass.c | 11 +++ include/pci.h| 4 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index e0d01f6a85..82308c7477 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1611,6 +1611,17 @@ void *dm_pci_map_bar(struct udevice *dev, int bar, size_t offset, size_t len, dm_pci_read_config32(udev, bar, _response); pci_bus_addr = (pci_addr_t)(bar_response & ~0xf); + /* +* This assumes that dm_pciauto_setup_device() will allocate +* a 64-bit address if CONFIG_SYS_PCI_64BIT is enabled and +* the device advertises that it supports it. +*/ + if (IS_ENABLED(CONFIG_SYS_PCI_64BIT) && + (bar_response & PCI_BASE_ADDRESS_MEM_TYPE_64)) { + dm_pci_read_config32(udev, bar + 4, _response); + pci_bus_addr |= (pci_addr_t)bar_response << 32; + } + if (~((pci_addr_t)0) - pci_bus_addr < offset) return NULL; diff --git a/include/pci.h b/include/pci.h index 2f5eb30b83..0d1ac7b015 100644 --- a/include/pci.h +++ b/include/pci.h @@ -1350,8 +1350,8 @@ pci_addr_t dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t addr, size_t len, * * Looks up a base address register and finds the physical memory address * that corresponds to it. - * Can be used for 32b BARs 0-5 on type 0 functions and for 32b BARs 0-1 on - * type 1 functions. + * Can be used for 32b/64b BARs 0-5 on type 0 functions and for 32b BARs 0-1 + * on type 1 functions. * Can also be used on type 0 functions that support Enhanced Allocation for * 32b/64b BARs. Note that duplicate BEI entries are not supported. * -- 2.43.0.472.g3155946c3a-goog
[PATCH v2] arm: Add OVERLAY command to BSS section on ARM64
Avoid allocating and loading the BSS section. $ aarch64-linux-gnu-objdump -Sh u-boot Before: 10 .bss_start 000f21d8 000f21d8 001021d8 2**0 CONTENTS, ALLOC, LOAD, DATA 11 .bss 68f8 000f2200 000f2200 001021d8 2**6 ALLOC 12 .bss_end 000f8af8 000f8af8 00108af8 2**0 CONTENTS, ALLOC, LOAD, DATA After: 10 .bss_start 000bf990 000bf990 001021e0 2**0 CONTENTS 11 .bss 68e8 000bf990 000bf990 001021e0 2**4 CONTENTS 12 .bss_end 000c6278 000c6278 00108ac8 2**0 CONTENTS Signed-off-by: Marek Vasut --- Cc: Simon Glass Cc: Tom Rini --- V2: Replicate arch/arm/cpu/u-boot.lds BSS part verbatim --- arch/arm/cpu/armv8/u-boot.lds | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds index fb6a30c922f..ebdc079552d 100644 --- a/arch/arm/cpu/armv8/u-boot.lds +++ b/arch/arm/cpu/armv8/u-boot.lds @@ -151,16 +151,18 @@ SECTIONS . = ALIGN(8); - .bss_start : { + .bss_start __rel_dyn_start (OVERLAY) : { KEEP(*(.__bss_start)); + __bss_base = .; } - .bss : { + .bss __bss_base (OVERLAY) : { *(.bss*) . = ALIGN(8); +__bss_limit = .; } - .bss_end : { + .bss_end __bss_limit (OVERLAY) : { KEEP(*(.__bss_end)); } -- 2.43.0
Re: [PATCH v2 1/1] clk: clk-gpio: add actual gated clock
On 12/16/23 17:56, Sean Anderson wrote: On 12/16/23 11:52, Marek Vasut wrote: On 12/16/23 16:37, Sean Anderson wrote: On 12/16/23 03:48, Svyatoslav Ryhel wrote: Existing gpio-gate-clock driver acts like a simple GPIO switch without any effect on gated clock. Add actual clock actions into enable/disable ops and implement get_rate op by passing gated clock if it is enabled. Signed-off-by: Svyatoslav Ryhel --- drivers/clk/clk-gpio.c | 44 ++ 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/drivers/clk/clk-gpio.c b/drivers/clk/clk-gpio.c index 26d795b978..72d9747a47 100644 --- a/drivers/clk/clk-gpio.c +++ b/drivers/clk/clk-gpio.c @@ -3,19 +3,23 @@ * Copyright (C) 2023 Marek Vasut */ -#include -#include -#include +#include #include +#include +#include + +#include struct clk_gpio_priv { - struct gpio_desc enable; + struct gpio_desc enable; /* GPIO, controlling the gate */ + struct clk *clk; /* Gated clock */ }; static int clk_gpio_enable(struct clk *clk) { struct clk_gpio_priv *priv = dev_get_priv(clk->dev); + clk_enable(priv->clk); dm_gpio_set_value(>enable, 1); return 0; @@ -26,21 +30,45 @@ static int clk_gpio_disable(struct clk *clk) struct clk_gpio_priv *priv = dev_get_priv(clk->dev); dm_gpio_set_value(>enable, 0); + clk_disable(priv->clk); return 0; } +static ulong clk_gpio_get_rate(struct clk *clk) +{ + struct clk_gpio_priv *priv = dev_get_priv(clk->dev); + + return clk_get_rate(priv->clk); +} + const struct clk_ops clk_gpio_ops = { .enable = clk_gpio_enable, .disable = clk_gpio_disable, + .get_rate = clk_gpio_get_rate, }; -static int clk_gpio_probe(struct udevice *dev) +static int clk_gpio_of_to_plat(struct udevice *dev) { struct clk_gpio_priv *priv = dev_get_priv(dev); + int ret; - return gpio_request_by_name(dev, "enable-gpios", 0, - >enable, GPIOD_IS_OUT); + priv->clk = devm_clk_get(dev, NULL); + if (IS_ERR(priv->clk)) { + log_debug("%s: Could not get gated clock: %ld\n", + __func__, PTR_ERR(priv->clk)); + return PTR_ERR(priv->clk); + } + + ret = gpio_request_by_name(dev, "enable-gpios", 0, + >enable, GPIOD_IS_OUT); + if (ret) { + log_debug("%s: Could not decode enable-gpios (%d)\n", + __func__, ret); + return ret; + } + + return 0; All this forwarding of clock enable/disable/get_rate for parent clock should already be done in clock core. If it is not, then it should be fixed in clock core. Only CCF forwards things. It's up to the driver to do it in the non-CCF case. Shall we add dependency on CCF instead of duplicating code in drivers ?
Re: [PATCH 13/17] video: rockchip: Add rk3328 vop support
Hi Jagan, In your patch U-boot users must add a new file for each new Rockchip SoC. With the VOP2 introduction the VOP1 structures and functions are frozen/stabilized. My proposal would be to use a file simular to Linux rockchip_vop_reg.c and port it to U-boot as is done in the manufacturer tree. Together with a simple basic rockchip_vop.c to start with. Not sure if we need a kind of DRM frame work. Question: What do the U-boot maintainers think of this DRM implementation in use by Rockchip. Is that a route that useful for mainline? Let me know your ideas. Johan On 12/11/23 09:59, Jagan Teki wrote: > From: Jagan Teki > > Add support for Rockchip RK3328 VOP. > > Signed-off-by: Jagan Teki > --- > drivers/video/rockchip/Makefile | 1 + > drivers/video/rockchip/rk3328_vop.c | 66 + > 2 files changed, 67 insertions(+) > create mode 100644 drivers/video/rockchip/rk3328_vop.c > > diff --git a/drivers/video/rockchip/Makefile b/drivers/video/rockchip/Makefile > index 4991303c73..f55beceebf 100644 > --- a/drivers/video/rockchip/Makefile > +++ b/drivers/video/rockchip/Makefile > @@ -6,6 +6,7 @@ > ifdef CONFIG_VIDEO_ROCKCHIP > obj-y += rk_vop.o > obj-$(CONFIG_ROCKCHIP_RK3288) += rk3288_vop.o > +obj-$(CONFIG_ROCKCHIP_RK3328) += rk3328_vop.o > obj-$(CONFIG_ROCKCHIP_RK3399) += rk3399_vop.o > obj-$(CONFIG_DISPLAY_ROCKCHIP_EDP) += rk_edp.o > obj-$(CONFIG_DISPLAY_ROCKCHIP_LVDS) += rk_lvds.o > diff --git a/drivers/video/rockchip/rk3328_vop.c > b/drivers/video/rockchip/rk3328_vop.c > new file mode 100644 > index 00..2512314e64 > --- /dev/null > +++ b/drivers/video/rockchip/rk3328_vop.c > @@ -0,0 +1,66 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (c) 2023 Edgeble AI Technologies Pvt. Ltd. > + */ > + > +#include > +#include > +#include > +#include > +#include "rk_vop.h" > + > +DECLARE_GLOBAL_DATA_PTR; > + > +static void rk3328_set_pin_polarity(struct udevice *dev, > + enum vop_modes mode, u32 polarity) > +{ > + struct rk_vop_priv *priv = dev_get_priv(dev); > + struct rk3288_vop *regs = priv->regs; > + > + switch (mode) { > + case VOP_MODE_HDMI: > + clrsetbits_le32(>dsp_ctrl1, > + M_RK3399_DSP_HDMI_POL, > + V_RK3399_DSP_HDMI_POL(polarity)); > + break; > + default: > + debug("%s: unsupported output mode %x\n", __func__, mode); > + } > +} > + > +static int rk3328_vop_probe(struct udevice *dev) > +{ > + /* Before relocation we don't need to do anything */ > + if (!(gd->flags & GD_FLG_RELOC)) > + return 0; > + > + return rk_vop_probe(dev); > +} > + > +struct rkvop_driverdata rk3328_driverdata = { > + .dsp_offset = 0x490, > + .win_offset = 0xd0, > + .features = VOP_FEATURE_OUTPUT_10BIT, > + .set_pin_polarity = rk3328_set_pin_polarity, > +}; > + > +static const struct udevice_id rk3328_vop_ids[] = { > + { > + .compatible = "rockchip,rk3328-vop", > + .data = (ulong)_driverdata > + }, > + { /* sentile */ } > +}; > + > +static const struct video_ops rk3328_vop_ops = { > +}; > + > +U_BOOT_DRIVER(rk3328_vop) = { > + .name = "rk3328_vop", > + .id = UCLASS_VIDEO, > + .of_match = rk3328_vop_ids, > + .ops= _vop_ops, > + .bind = rk_vop_bind, > + .probe = rk3328_vop_probe, > + .priv_auto = sizeof(struct rk_vop_priv), > +};
Re: [PATCH v2 3/3] efi: Avoid using dm_scan_other()
Am 16. Dezember 2023 23:14:20 MEZ schrieb Tom Rini : >On Sat, Dec 16, 2023 at 09:57:41PM +0100, Heinrich Schuchardt wrote: >> On 12/16/23 21:46, Simon Glass wrote: >> > Hi, >> > >> > On Tue, 21 Nov 2023 at 06:21, Tom Rini wrote: >> > > >> > > On Tue, Nov 21, 2023 at 01:18:09PM +0100, Heinrich Schuchardt wrote: >> > > > On 11/12/23 21:55, Simon Glass wrote: >> > > > > This function is defined by bootstd so using it precludes using that >> > > > > feature. Use the board_early_init_r() feature instead. >> > > > > >> > > > > This requires moving quite a lot of code into the board directory, >> > > > > butt >> > > > > this is the normal place for code called by board_early_init_r() >> > > > > >> > > > > Signed-off-by: Simon Glass >> > > > > --- >> > > > > >> > > > > Changes in v2: >> > > > > - Drop duplicate acpi_xsdt patch >> > > > > - Put the board_early_init_r code into board/ >> > > > > >> > > > >board/efi/efi-x86_app/Makefile | 5 + >> > > > >board/efi/efi-x86_app/efi_app.c | 205 >> > > > > >> > > > >> > > > Our target should be to enable building the EFI app on all >> > > > architectures. >> > > > >> > > > Only x86 specific code should be added to >> > > > board/efi/efi-x86_app/efi_app.c. >> > > >> > > A later enhancement to make U-Boot as an EFI app more generic would be >> > > good, but outside the scope of this patch which is moving generic code >> > > out from "lib" and in to "board". >> >> I cannot see that this patch is moving any code our of lib/. >> >> But why should we move generic code into board? >> >> > >> > This patch was marked as old /archived in patchwork so has been >> > forgotten. I have marked it new and non-archived in the hope that it >> > can be applied to -master soon. >> >> There is nothing x86 specific about the code. Generic code should be in >> lib/. Please, provide a new version of the patch. > >It's not generic EFI code, it's generic "U-Boot as EFI application" code >and so the whole "board" needs a bit of a clean and re-work as we all >agree that there shouldn't be anything architecture specific about it, >only building the binary for the correct architecture. However, we just >aren't well structured to support that right now. Where on the list of >priorities does that have to fall rather than just allowing for >functionality to be tested / used? There's not yet IIRC efi-arm64_app >but what we have today (and would have had with Simon's patch) should >have just built and worked. > We have /lib/efi/ for that code. Regards Heinrich
Re: [PATCH v2 3/3] efi: Avoid using dm_scan_other()
On Sat, Dec 16, 2023 at 09:57:41PM +0100, Heinrich Schuchardt wrote: > On 12/16/23 21:46, Simon Glass wrote: > > Hi, > > > > On Tue, 21 Nov 2023 at 06:21, Tom Rini wrote: > > > > > > On Tue, Nov 21, 2023 at 01:18:09PM +0100, Heinrich Schuchardt wrote: > > > > On 11/12/23 21:55, Simon Glass wrote: > > > > > This function is defined by bootstd so using it precludes using that > > > > > feature. Use the board_early_init_r() feature instead. > > > > > > > > > > This requires moving quite a lot of code into the board directory, > > > > > butt > > > > > this is the normal place for code called by board_early_init_r() > > > > > > > > > > Signed-off-by: Simon Glass > > > > > --- > > > > > > > > > > Changes in v2: > > > > > - Drop duplicate acpi_xsdt patch > > > > > - Put the board_early_init_r code into board/ > > > > > > > > > >board/efi/efi-x86_app/Makefile | 5 + > > > > >board/efi/efi-x86_app/efi_app.c | 205 > > > > > > > > > > > > > Our target should be to enable building the EFI app on all > > > > architectures. > > > > > > > > Only x86 specific code should be added to > > > > board/efi/efi-x86_app/efi_app.c. > > > > > > A later enhancement to make U-Boot as an EFI app more generic would be > > > good, but outside the scope of this patch which is moving generic code > > > out from "lib" and in to "board". > > I cannot see that this patch is moving any code our of lib/. > > But why should we move generic code into board? > > > > > This patch was marked as old /archived in patchwork so has been > > forgotten. I have marked it new and non-archived in the hope that it > > can be applied to -master soon. > > There is nothing x86 specific about the code. Generic code should be in > lib/. Please, provide a new version of the patch. It's not generic EFI code, it's generic "U-Boot as EFI application" code and so the whole "board" needs a bit of a clean and re-work as we all agree that there shouldn't be anything architecture specific about it, only building the binary for the correct architecture. However, we just aren't well structured to support that right now. Where on the list of priorities does that have to fall rather than just allowing for functionality to be tested / used? There's not yet IIRC efi-arm64_app but what we have today (and would have had with Simon's patch) should have just built and worked. -- Tom signature.asc Description: PGP signature
Re: Booting fails on the sandbox
On 12/16/23 19:45, Simon Glass wrote: Hi Heinrich, On Sat, 16 Dec 2023 at 07:50, Heinrich Schuchardt wrote: Hello Simon, I have built sandbox_defconfig with PREBOOT='host bind 0 ../sct.img' On the image I have an ESP with file EFI/BOOT/BOOTX64.EFI. I would expect the sandbox to boot from the ESP. But when starting the sandbox is does not boot. Instead it shows a menu with some Fedora related entry: Fedora-Workstation-armhfp-31-1.9 Boot Options. 1: Fedora-Workstation-armhfp-31-1.9 (5.3.7-301.fc31.armv7hl) This makes absolutely no sense on an x86_64 system. Could you, please, fix sandbox_defconfig to boot according to distroboot. What sort of 'fix' are you thinking of? You should be able to discover all the options with 'bootflow scan' and then use 'bootflow select' or 'bootflow menu' to choose which one you want. The Fedora thing is used for testing and development. I want to autoboot the sandbox without manual intervention. This requires a sane state. If that Fedora thingy is only needed in CI, can't we put it into a config segment that only used by the CI scripts and have the sandbox in a generally usable state? Or can we let the Python tests add the otherwise not needed bootflow? Best regards Heinrich
Re: [PATCH v2 3/3] efi: Avoid using dm_scan_other()
On 12/16/23 21:46, Simon Glass wrote: Hi, On Tue, 21 Nov 2023 at 06:21, Tom Rini wrote: On Tue, Nov 21, 2023 at 01:18:09PM +0100, Heinrich Schuchardt wrote: On 11/12/23 21:55, Simon Glass wrote: This function is defined by bootstd so using it precludes using that feature. Use the board_early_init_r() feature instead. This requires moving quite a lot of code into the board directory, butt this is the normal place for code called by board_early_init_r() Signed-off-by: Simon Glass --- Changes in v2: - Drop duplicate acpi_xsdt patch - Put the board_early_init_r code into board/ board/efi/efi-x86_app/Makefile | 5 + board/efi/efi-x86_app/efi_app.c | 205 Our target should be to enable building the EFI app on all architectures. Only x86 specific code should be added to board/efi/efi-x86_app/efi_app.c. A later enhancement to make U-Boot as an EFI app more generic would be good, but outside the scope of this patch which is moving generic code out from "lib" and in to "board". I cannot see that this patch is moving any code our of lib/. But why should we move generic code into board? This patch was marked as old /archived in patchwork so has been forgotten. I have marked it new and non-archived in the hope that it can be applied to -master soon. There is nothing x86 specific about the code. Generic code should be in lib/. Please, provide a new version of the patch. I believe that having the EFI app support bootstd could be a useful addition. That was not disputed. Best regards Heinrich
Re: [PATCH v2 3/3] efi: Avoid using dm_scan_other()
Hi, On Tue, 21 Nov 2023 at 06:21, Tom Rini wrote: > > On Tue, Nov 21, 2023 at 01:18:09PM +0100, Heinrich Schuchardt wrote: > > On 11/12/23 21:55, Simon Glass wrote: > > > This function is defined by bootstd so using it precludes using that > > > feature. Use the board_early_init_r() feature instead. > > > > > > This requires moving quite a lot of code into the board directory, butt > > > this is the normal place for code called by board_early_init_r() > > > > > > Signed-off-by: Simon Glass > > > --- > > > > > > Changes in v2: > > > - Drop duplicate acpi_xsdt patch > > > - Put the board_early_init_r code into board/ > > > > > > board/efi/efi-x86_app/Makefile | 5 + > > > board/efi/efi-x86_app/efi_app.c | 205 > > > > Our target should be to enable building the EFI app on all architectures. > > > > Only x86 specific code should be added to board/efi/efi-x86_app/efi_app.c. > > A later enhancement to make U-Boot as an EFI app more generic would be > good, but outside the scope of this patch which is moving generic code > out from "lib" and in to "board". This patch was marked as old /archived in patchwork so has been forgotten. I have marked it new and non-archived in the hope that it can be applied to -master soon. I believe that having the EFI app support bootstd could be a useful addition. Regards, Simon
[PATCH v2 2/2] nvme: Update scan namespace to keep trying on busy
A busy controller shouldn't be game-over for all controllers, so keep trying on hitting -EBUSY. This change brings the actual behavior of the routine in line with what the descriptions says. Fixes: 982388eaa991 ("nvme: Add NVM Express driver support") Reviewed-by: Simon Glass Signed-off-by: Moritz Fischer --- Changes from V1: - Added Simon's Reviewed-by --- drivers/nvme/nvme.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c index ec45f831a3..59a139baa0 100644 --- a/drivers/nvme/nvme.c +++ b/drivers/nvme/nvme.c @@ -695,7 +695,9 @@ int nvme_scan_namespace(void) if (ret) { log_err("Failed to probe '%s': err=%dE\n", dev->name, ret); - return ret; + /* Bail if we ran out of memory, else keep trying */ + if (ret != -EBUSY) + return ret; } } -- 2.43.0.472.g3155946c3a-goog
[PATCH v2 1/2] nvme: Fix error code and log to indicate busy
Return -EBUSY if controller is found busy rather than -ENOMEM and update the error message accordingly. Fixes: 982388eaa991 ("nvme: Add NVM Express driver support") Reviewed-by: Simon Glass Signed-off-by: Moritz Fischer --- Changes from V1: - Added Simon's Reviewed-by --- drivers/nvme/nvme.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c index c39cd41aa3..ec45f831a3 100644 --- a/drivers/nvme/nvme.c +++ b/drivers/nvme/nvme.c @@ -835,8 +835,8 @@ int nvme_init(struct udevice *udev) ndev->udev = udev; INIT_LIST_HEAD(>namespaces); if (readl(>bar->csts) == -1) { - ret = -ENODEV; - printf("Error: %s: Out of memory!\n", udev->name); + ret = -EBUSY; + printf("Error: %s: Controller not ready!\n", udev->name); goto free_nvme; } -- 2.43.0.472.g3155946c3a-goog
Re: [PATCH v6 0/5] Populate kaslr seed with RNG
On Sat, Dec 16, 2023 at 12:09:49PM -0500, Tom Rini wrote: > On Thu, Nov 30, 2023 at 04:54:39PM -0800, Sean Edmond wrote: > > > This patch series creates a common API (fdt_fixup_kaslr_seed()) for > > populating the kaslr seed in the DTB. Existing users (kaslrseed, > > and ARMv8 sec firmware) have been updated to use this common API. > > > > New functionality has been introduced to populate the kaslr using > > the RNG. This can be enabled with CONFIG_RNG_TPM_SEED. > > This series introduces a fail to build on ls1012afrdm_tfa and others. > Also one of the patches has a double Signed-off-by, please fix that too. Also, a large number of platforms grow by over 1kB, see beelink-gtking for example (and use buildman's size comparison tools). -- Tom signature.asc Description: PGP signature
Re: [PATCH v3 22/22] bootm: Create a new boot_run() function to handle booting
On Sat, Dec 16, 2023 at 11:45:36AM -0700, Simon Glass wrote: > Hi Tom, > > On Sat, 16 Dec 2023 at 10:12, Tom Rini wrote: > > > > On Fri, Dec 15, 2023 at 08:14:26PM -0700, Simon Glass wrote: > > > Create a common function used by the three existing bootz/i/m_run() > > > functions, to reduce duplicated code. > > > > > > Signed-off-by: Simon Glass > > > Suggested-by: Tom Rini > > > --- > > > > > > Changes in v3: > > > - Add new boot_run() function > > > > > > boot/bootm.c| 40 ++-- > > > include/bootm.h | 18 ++ > > > 2 files changed, 32 insertions(+), 26 deletions(-) > > > > > > diff --git a/boot/bootm.c b/boot/bootm.c > > > index 53236136f489..6a4cebcf7a08 100644 > > > --- a/boot/bootm.c > > > +++ b/boot/bootm.c > > > @@ -1123,47 +1123,35 @@ err: > > > return ret; > > > } > > > > > > -int bootm_run(struct bootm_info *bmi) > > > +int boot_run(struct bootm_info *bmi, const char *cmd, int extra_states) > > > { > > > int states; > > > > > > - bmi->cmd_name = "bootm"; > > > - states = BOOTM_STATE_START | BOOTM_STATE_FINDOS | > > > BOOTM_STATE_PRE_LOAD | > > > - BOOTM_STATE_FINDOTHER | BOOTM_STATE_LOADOS | > > > - BOOTM_STATE_OS_PREP | BOOTM_STATE_OS_FAKE_GO | > > > - BOOTM_STATE_OS_GO | BOOTM_STATE_MEASURE; > > > + bmi->cmd_name = cmd; > > > + states = BOOTM_STATE_MEASURE | BOOTM_STATE_OS_PREP | > > > + BOOTM_STATE_OS_FAKE_GO | BOOTM_STATE_OS_GO; > > > if (IS_ENABLED(CONFIG_SYS_BOOT_RAMDISK_HIGH)) > > > states |= BOOTM_STATE_RAMDISK; > > > - if (IS_ENABLED(CONFIG_PPC) || IS_ENABLED(CONFIG_MIPS)) > > > - states |= BOOTM_STATE_OS_CMDLINE; > > > + states |= extra_states; > > > > > > return bootm_run_states(bmi, states); > > > } > > > > > > -int bootz_run(struct bootm_info *bmi) > > > +int bootm_run(struct bootm_info *bmi) > > > { > > > - int states; > > > - > > > - bmi->cmd_name = "bootz"; > > > - states = BOOTM_STATE_MEASURE | BOOTM_STATE_OS_PREP | > > > - BOOTM_STATE_OS_FAKE_GO | BOOTM_STATE_OS_GO; > > > - if (IS_ENABLED(CONFIG_SYS_BOOT_RAMDISK_HIGH)) > > > - states |= BOOTM_STATE_RAMDISK; > > > + return boot_run(bmi, "bootm", BOOTM_STATE_START | > > > BOOTM_STATE_FINDOS | > > > + BOOTM_STATE_PRE_LOAD | BOOTM_STATE_FINDOTHER | > > > + BOOTM_STATE_LOADOS); > > > +} > > > > > > - return bootm_run_states(bmi, states); > > > +int bootz_run(struct bootm_info *bmi) > > > +{ > > > + return boot_run(bmi, "bootz", 0); > > > } > > > > > > int booti_run(struct bootm_info *bmi) > > > { > > > - int states; > > > - > > > - bmi->cmd_name = "booti"; > > > - states = BOOTM_STATE_MEASURE | BOOTM_STATE_OS_PREP | > > > - BOOTM_STATE_OS_FAKE_GO | BOOTM_STATE_OS_GO; > > > - if (IS_ENABLED(CONFIG_SYS_BOOT_RAMDISK_HIGH)) > > > - states |= BOOTM_STATE_RAMDISK; > > > - > > > - return bootm_run_states(bmi, states); > > > + return boot_run(bmi, "booti", 0); > > > } > > > > > > int bootm_boot_start(ulong addr, const char *cmdline) > > > diff --git a/include/bootm.h b/include/bootm.h > > > index eba35b33b4e5..9e0f8d60de0a 100644 > > > --- a/include/bootm.h > > > +++ b/include/bootm.h > > > @@ -150,6 +150,24 @@ int bootm_measure(struct bootm_headers *images); > > > */ > > > int bootm_run_states(struct bootm_info *bmi, int states); > > > > > > +/** > > > + * boot_run() - Run the entire bootm/booti/bootz process > > > + * > > > + * This runs through the boot process from start to finish, with a base > > > set of > > > + * states, along with the extra ones supplied. > > > + * > > > + * This uses bootm_run_states(). > > > + * > > > + * Note that it is normally easier to use bootm_run(), etc. since they > > > handle > > > + * the extra states correctly. > > > + * > > > + * @bmi: bootm information > > > + * @cmd: command being run, NULL if none > > > + * @extra_states: Mask of extra states to use for the boot > > > + * Return: 0 if ok, something else on error > > > + */ > > > +int boot_run(struct bootm_info *bmi, const char *cmd, int extra_states); > > > + > > > /** > > > * bootm_run() - Run the entire bootm process > > > * > > > > Overall good, thanks. My question is, should we mark the others as > > static, or is this new helper static and not to be called externally? > > The others are used externally. > > For bootm_run() I toyed with the idea of it being static, but then > wondered if we might use that programmatically instead of the > bootz/i/m(_run) versions. It is quite nice that the differences are > now pretty minor between them. > > I don't mind either way, though. OK, lets put it on the TODO list for once the CMDLINE=n case is fully flushed out and code otherwise cleaned up and organized, it'll perhaps be clearer then if the answer is old functions, new function or neither function. Reviewed-by:
[PATCH 1/3] clk: Remove rfree
Nothing uses this function. Remove it. Since clk_free no longer does anything, just stub it out. Signed-off-by: Sean Anderson --- arch/sandbox/include/asm/clk.h | 8 drivers/clk/clk-uclass.c | 14 -- drivers/clk/clk_sandbox.c | 12 drivers/clk/clk_sandbox_test.c | 12 include/clk-uclass.h | 10 -- include/clk.h | 18 -- test/dm/clk.c | 9 - 7 files changed, 4 insertions(+), 79 deletions(-) diff --git a/arch/sandbox/include/asm/clk.h b/arch/sandbox/include/asm/clk.h index 2b7dbca8f75..b8204227023 100644 --- a/arch/sandbox/include/asm/clk.h +++ b/arch/sandbox/include/asm/clk.h @@ -180,14 +180,6 @@ int sandbox_clk_test_disable(struct udevice *dev, int id); * @return:0 if OK, or a negative error code. */ int sandbox_clk_test_disable_bulk(struct udevice *dev); -/** - * sandbox_clk_test_free - Ask the sandbox clock test device to free its - * clocks. - * - * @dev: The sandbox clock test (client) device. - * @return:0 if OK, or a negative error code. - */ -int sandbox_clk_test_free(struct udevice *dev); /** * sandbox_clk_test_release_bulk - Ask the sandbox clock test device to release * all clocks in it's clock bulk struct. diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index 3b5e3f9c86b..9260e434988 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -461,20 +461,6 @@ int clk_request(struct udevice *dev, struct clk *clk) return ops->request(clk); } -void clk_free(struct clk *clk) -{ - const struct clk_ops *ops; - - debug("%s(clk=%p)\n", __func__, clk); - if (!clk_valid(clk)) - return; - ops = clk_dev_ops(clk->dev); - - if (ops->rfree) - ops->rfree(clk); - return; -} - ulong clk_get_rate(struct clk *clk) { const struct clk_ops *ops; diff --git a/drivers/clk/clk_sandbox.c b/drivers/clk/clk_sandbox.c index 636914db8ca..73d943f9e09 100644 --- a/drivers/clk/clk_sandbox.c +++ b/drivers/clk/clk_sandbox.c @@ -101,17 +101,6 @@ static int sandbox_clk_request(struct clk *clk) return 0; } -static void sandbox_clk_free(struct clk *clk) -{ - struct sandbox_clk_priv *priv = dev_get_priv(clk->dev); - - if (clk->id >= SANDBOX_CLK_ID_COUNT) - return; - - priv->requested[clk->id] = false; - return; -} - static struct clk_ops sandbox_clk_ops = { .round_rate = sandbox_clk_round_rate, .get_rate = sandbox_clk_get_rate, @@ -119,7 +108,6 @@ static struct clk_ops sandbox_clk_ops = { .enable = sandbox_clk_enable, .disable= sandbox_clk_disable, .request= sandbox_clk_request, - .rfree = sandbox_clk_free, }; static int sandbox_clk_probe(struct udevice *dev) diff --git a/drivers/clk/clk_sandbox_test.c b/drivers/clk/clk_sandbox_test.c index 5807a454f3b..d9ec070110e 100644 --- a/drivers/clk/clk_sandbox_test.c +++ b/drivers/clk/clk_sandbox_test.c @@ -134,18 +134,6 @@ int sandbox_clk_test_disable_bulk(struct udevice *dev) return clk_disable_bulk(>bulk); } -int sandbox_clk_test_free(struct udevice *dev) -{ - struct sandbox_clk_test *sbct = dev_get_priv(dev); - int i; - - devm_clk_put(dev, sbct->clkps[SANDBOX_CLK_TEST_ID_DEVM1]); - for (i = 0; i < SANDBOX_CLK_TEST_NON_DEVM_COUNT; i++) - clk_free(>clks[i]); - - return 0; -} - int sandbox_clk_test_release_bulk(struct udevice *dev) { struct sandbox_clk_test *sbct = dev_get_priv(dev); diff --git a/include/clk-uclass.h b/include/clk-uclass.h index cd62848bece..4353b664800 100644 --- a/include/clk-uclass.h +++ b/include/clk-uclass.h @@ -18,7 +18,6 @@ struct ofnode_phandle_args; * struct clk_ops - The functions that a clock driver must implement. * @of_xlate: Translate a client's device-tree (OF) clock specifier. * @request: Request a translated clock. - * @rfree: Free a previously requested clock. * @round_rate: Adjust a rate to the exact rate a clock can provide. * @get_rate: Get current clock rate. * @set_rate: Set current clock rate. @@ -33,7 +32,6 @@ struct clk_ops { int (*of_xlate)(struct clk *clock, struct ofnode_phandle_args *args); int (*request)(struct clk *clock); - void (*rfree)(struct clk *clock); ulong (*round_rate)(struct clk *clk, ulong rate); ulong (*get_rate)(struct clk *clk); ulong (*set_rate)(struct clk *clk, ulong rate); @@ -81,14 +79,6 @@ int of_xlate(struct clk *clock, struct ofnode_phandle_args *args); */ int request(struct clk *clock); -/** - * rfree() - Free a previously requested clock. - * @clock: The clock to free. - * - * Free any resources allocated in request(). - */ -void rfree(struct clk *clock); - /** * round_rate() - Adjust a rate to the exact rate a clock can provide. * @clk: The
[PATCH 2/3] treewide: Remove clk_free
This function is a no-op. Remove it. Signed-off-by: Sean Anderson --- arch/arm/mach-rockchip/rk3288/rk3288.c| 2 - arch/arm/mach-socfpga/clock_manager_agilex.c | 2 - arch/arm/mach-socfpga/clock_manager_arria10.c | 7 +-- arch/arm/mach-socfpga/clock_manager_n5x.c | 2 - arch/arm/mach-zynq/clk.c | 2 - arch/mips/mach-pic32/cpu.c| 7 +-- board/microchip/pic32mzda/pic32mzda.c | 2 - board/sipeed/maix/maix.c | 1 - board/synopsys/hsdk/clk-lib.c | 2 - drivers/clk/aspeed/clk_ast2600.c | 2 - drivers/clk/at91/compat.c | 14 +- drivers/clk/clk-uclass.c | 26 +-- drivers/clk/clk-xlnx-clock-wizard.c | 1 - drivers/clk/clk_versaclock.c | 12 +- drivers/clk/clk_zynq.c| 2 - drivers/clk/clk_zynqmp.c | 2 - drivers/clk/imx/clk-imx8.c| 2 - drivers/clk/mvebu/armada-37xx-periph.c| 2 - drivers/cpu/riscv_cpu.c | 2 - drivers/dma/bcm6348-iudma.c | 2 - drivers/gpio/at91_gpio.c | 2 - drivers/gpio/atmel_pio4.c | 2 - drivers/gpio/gpio-rcar.c | 1 - drivers/hwspinlock/stm32_hwspinlock.c | 6 +-- drivers/i2c/at91_i2c.c| 2 - drivers/i2c/designware_i2c.c | 2 - drivers/i2c/i2c-microchip.c | 2 - drivers/i2c/npcm_i2c.c| 1 - drivers/i2c/ocores_i2c.c | 2 - drivers/i2c/stm32f7_i2c.c | 4 +- drivers/mailbox/stm32-ipcc.c | 7 +-- drivers/misc/ls2_sfp.c| 1 - drivers/mmc/arm_pl180_mmci.c | 1 - drivers/mmc/aspeed_sdhci.c| 4 +- drivers/mmc/atmel_sdhci.c | 2 - drivers/mmc/gen_atmel_mci.c | 19 +++- drivers/mmc/msm_sdhci.c | 1 - drivers/mmc/pic32_sdhci.c | 1 - drivers/mmc/renesas-sdhi.c| 21 +++-- drivers/mmc/snps_dw_mmc.c | 8 +--- drivers/mmc/socfpga_dw_mmc.c | 1 - drivers/mmc/stm32_sdmmc2.c| 4 +- drivers/mmc/uniphier-sd.c | 1 - drivers/mtd/nand/raw/atmel/nand-controller.c | 4 +- drivers/mtd/renesas_rpc_hf.c | 1 - drivers/net/bcm6348-eth.c | 2 - drivers/net/bcm6368-eth.c | 2 - drivers/net/designware.c | 1 - drivers/net/dwc_eth_qos.c | 43 +++ drivers/net/dwc_eth_qos_imx.c | 21 ++--- drivers/net/dwc_eth_qos_qcom.c| 1 - drivers/net/dwc_eth_qos_rockchip.c| 6 +-- drivers/net/sni_ave.c | 5 +-- drivers/net/ti/am65-cpsw-nuss.c | 1 - drivers/phy/bcm6318-usbh-phy.c| 2 - drivers/phy/bcm6348-usbh-phy.c| 2 - drivers/phy/bcm6368-usbh-phy.c| 4 -- drivers/phy/meson-axg-mipi-dphy.c | 1 - drivers/phy/meson-g12a-usb3-pcie.c| 1 - drivers/phy/meson-gxl-usb2.c | 1 - drivers/phy/phy-rcar-gen2.c | 1 - drivers/phy/phy-rcar-gen3.c | 1 - drivers/pinctrl/pinctrl-k210.c| 20 +++-- drivers/power/domain/imx8mp-hsiomix.c | 4 +- drivers/rtc/stm32_rtc.c | 16 ++- drivers/serial/atmel_usart.c | 2 - drivers/serial/serial_bcm6345.c | 1 - drivers/serial/serial_msm.c | 1 - drivers/serial/serial_pic32.c | 1 - drivers/spi/atcspi200_spi.c | 1 - drivers/spi/atmel-quadspi.c | 14 ++ drivers/spi/atmel_spi.c | 2 - drivers/spi/bcm63xx_hsspi.c | 4 -- drivers/spi/bcm63xx_spi.c | 2 - drivers/spi/bcmbca_hsspi.c| 4 -- drivers/spi/cadence_qspi.c| 1 - drivers/spi/designware_spi.c | 5 --- drivers/spi/meson_spifc_a1.c | 10 - drivers/spi/mvebu_a3700_spi.c | 10 - drivers/spi/spi-aspeed-smc.c | 1 - drivers/spi/stm32_spi.c | 19 ++-- drivers/timer/dw-apb-timer.c | 2 - drivers/timer/ostm_timer.c| 2 - drivers/usb/dwc3/dwc3-meson-g12a.c| 4 +- drivers/usb/dwc3/dwc3-meson-gxl.c | 4 +- drivers/usb/host/ehci-atmel.c | 8 +--- drivers/usb/host/ohci-da8xx.c | 1 - drivers/usb/host/xhci-rcar.c | 5 +-- drivers/video/atmel_hlcdfb.c
[PATCH 3/3] clk: Document clk_ops return codes and behavior
Currently, clock consumers cannot take any programmatic action based on the return code of a clock function. This is because there is no standardization, and generally no way of separating e.g. "there was a major problem setting the rate for this clock" which usually should not be recovered from, from "this clock doesn't support setting its rate" or "this clock doesn't support *this* rate" which could be absolutely fine depending on the driver. This commit aims to standardize the acceptable codes which may be returned from clock operations. In general, - ENOSYS should be returned when an operation is not supported for a particular clock. - ENOENT may be returned if the clock ID is invalid. However, it is encouraged to move any checks to request() to reduce code duplication. - EINVAL should be returned for logical errors only (such as requesting an invalid rate). Each function has had specific guidance added for when to return each error code. This is just guidance for now; most of the clock subsystem does not yet conform to this standard. However, it is expected that new clock drivers return these error codes. Additionally, this commit adds expected behavior for each of the clock operations. I believe these should be mostly straightforward and correspond to existing behavior. I remember not understanding what the expected invariants were for several clock functions, so hopefully this should help out new driver authors. In the future, some of these invariants could be checked via an optional config option. Signed-off-by: Sean Anderson --- include/clk-uclass.h | 113 +++ 1 file changed, 103 insertions(+), 10 deletions(-) diff --git a/include/clk-uclass.h b/include/clk-uclass.h index 4353b664800..8c07e723cff 100644 --- a/include/clk-uclass.h +++ b/include/clk-uclass.h @@ -56,11 +56,20 @@ struct clk_ops { * default implementation, which assumes #clock-cells = <1>, and that * the DT cell contains a simple integer clock ID. * + * This function should be a simple translation of @args into @clock->id and + * (optionally) @clock->data. All other processing, allocation, or error + * checking should take place in request(). + * * At present, the clock API solely supports device-tree. If this * changes, other xxx_xlate() functions may be added to support those * other mechanisms. * - * Return: 0 if OK, or a negative error code. + * Return: + * * 0 on success + * * -%EINVAL if @args does not have the correct format. For example, it could + * have too many/few arguments. + * * -%ENOENT if @args has the correct format but cannot be translated. This can + * happen if translation involves a table lookup and @args is not present. */ int of_xlate(struct clk *clock, struct ofnode_phandle_args *args); @@ -75,16 +84,36 @@ int of_xlate(struct clk *clock, struct ofnode_phandle_args *args); * xxx_xlate() call, or as the only step in implementing a client's * clk_request() call. * - * Return: 0 if OK, or a negative error code. + * This is the right place to do bounds checking (rejecting invalid or + * unimplemented clocks), allocate resources, or perform other setup not done + * during driver probe(). Most clock drivers should allocate resources in their + * probe() function, but it is possible to lazily initialize something here. + * + * Return: + * * 0 on success + * * -%ENOENT, if there is no clock corresponding to @clock->id and + * @clock->data. */ int request(struct clk *clock); /** * round_rate() - Adjust a rate to the exact rate a clock can provide. - * @clk: The clock to manipulate. - * @rate: Desidered clock rate in Hz. + * @clk: The clock to query. + * @rate: Desired clock rate in Hz. * - * Return: rounded rate in Hz, or -ve error code. + * This function returns a new rate which can be provided to set_rate(). This + * new rate should be the closest rate to @rate which can be set without + * rounding. The following pseudo-code should hold:: + * + * for all rate in range(ULONG_MAX): + * rounded = round_rate(clk, rate) + * new_rate = set_rate(clk, rate) + * assert(IS_ERR_VALUE(new_rate) || new_rate == rounded) + * + * Return: + * * The rounded rate in Hz on success + * * A negative error value from another API (such as clk_get_rate()). This + * function must not return an error for any other reason. */ ulong round_rate(struct clk *clk, ulong rate); @@ -92,7 +121,22 @@ ulong round_rate(struct clk *clk, ulong rate); * get_rate() - Get current clock rate. * @clk: The clock to query. * - * Return: clock rate in Hz, or -ve error code + * This returns the current rate of a clock. If the clock is disabled, it + * returns the rate at which the clock would run if it was enabled. The + * following pseudo-code should hold:: + * + * disable(clk) + * rate = get_rate(clk) + * enable(clk) + * assert(get_rate(clk) == rate) + * + * Return: + * * The rate of @clk + * * -%ENOSYS if
[PATCH 0/3] clk: Remove clk_free and better document clk_ops
This series contains two unrelated changes. They are included together because they touch the same files and would otherwise conflict. The first change is to completely remove clk_free. This is because the op it calls (rfree) is completely unused. I believe the original intent of this op was to allow clock drivers to free resources allocated in request. However, no clock drivers do this, so we can remove the function and all its callers. The second change is to document the expected behavior and return codes of all clock functions. At the moment, drivers are very inconsistent. I would like to have some easily-verified expected behavior we can use when reviewing clock drivers. Existing code is not fixed, but I hope to make some conversions in the future. Sean Anderson (3): clk: Remove rfree treewide: Remove clk_free clk: Document clk_ops return codes and behavior arch/arm/mach-rockchip/rk3288/rk3288.c| 2 - arch/arm/mach-socfpga/clock_manager_agilex.c | 2 - arch/arm/mach-socfpga/clock_manager_arria10.c | 7 +- arch/arm/mach-socfpga/clock_manager_n5x.c | 2 - arch/arm/mach-zynq/clk.c | 2 - arch/mips/mach-pic32/cpu.c| 7 +- arch/sandbox/include/asm/clk.h| 8 -- board/microchip/pic32mzda/pic32mzda.c | 2 - board/sipeed/maix/maix.c | 1 - board/synopsys/hsdk/clk-lib.c | 2 - drivers/clk/aspeed/clk_ast2600.c | 2 - drivers/clk/at91/compat.c | 14 +- drivers/clk/clk-uclass.c | 40 +- drivers/clk/clk-xlnx-clock-wizard.c | 1 - drivers/clk/clk_sandbox.c | 12 -- drivers/clk/clk_sandbox_test.c| 12 -- drivers/clk/clk_versaclock.c | 12 +- drivers/clk/clk_zynq.c| 2 - drivers/clk/clk_zynqmp.c | 2 - drivers/clk/imx/clk-imx8.c| 2 - drivers/clk/mvebu/armada-37xx-periph.c| 2 - drivers/cpu/riscv_cpu.c | 2 - drivers/dma/bcm6348-iudma.c | 2 - drivers/gpio/at91_gpio.c | 2 - drivers/gpio/atmel_pio4.c | 2 - drivers/gpio/gpio-rcar.c | 1 - drivers/hwspinlock/stm32_hwspinlock.c | 6 +- drivers/i2c/at91_i2c.c| 2 - drivers/i2c/designware_i2c.c | 2 - drivers/i2c/i2c-microchip.c | 2 - drivers/i2c/npcm_i2c.c| 1 - drivers/i2c/ocores_i2c.c | 2 - drivers/i2c/stm32f7_i2c.c | 4 +- drivers/mailbox/stm32-ipcc.c | 7 +- drivers/misc/ls2_sfp.c| 1 - drivers/mmc/arm_pl180_mmci.c | 1 - drivers/mmc/aspeed_sdhci.c| 4 +- drivers/mmc/atmel_sdhci.c | 2 - drivers/mmc/gen_atmel_mci.c | 19 +-- drivers/mmc/msm_sdhci.c | 1 - drivers/mmc/pic32_sdhci.c | 1 - drivers/mmc/renesas-sdhi.c| 21 +-- drivers/mmc/snps_dw_mmc.c | 8 +- drivers/mmc/socfpga_dw_mmc.c | 1 - drivers/mmc/stm32_sdmmc2.c| 4 +- drivers/mmc/uniphier-sd.c | 1 - drivers/mtd/nand/raw/atmel/nand-controller.c | 4 +- drivers/mtd/renesas_rpc_hf.c | 1 - drivers/net/bcm6348-eth.c | 2 - drivers/net/bcm6368-eth.c | 2 - drivers/net/designware.c | 1 - drivers/net/dwc_eth_qos.c | 43 +- drivers/net/dwc_eth_qos_imx.c | 21 +-- drivers/net/dwc_eth_qos_qcom.c| 1 - drivers/net/dwc_eth_qos_rockchip.c| 6 +- drivers/net/sni_ave.c | 5 +- drivers/net/ti/am65-cpsw-nuss.c | 1 - drivers/phy/bcm6318-usbh-phy.c| 2 - drivers/phy/bcm6348-usbh-phy.c| 2 - drivers/phy/bcm6368-usbh-phy.c| 4 - drivers/phy/meson-axg-mipi-dphy.c | 1 - drivers/phy/meson-g12a-usb3-pcie.c| 1 - drivers/phy/meson-gxl-usb2.c | 1 - drivers/phy/phy-rcar-gen2.c | 1 - drivers/phy/phy-rcar-gen3.c | 1 - drivers/pinctrl/pinctrl-k210.c| 20 +-- drivers/power/domain/imx8mp-hsiomix.c | 4 +- drivers/rtc/stm32_rtc.c | 16 +-- drivers/serial/atmel_usart.c | 2 - drivers/serial/serial_bcm6345.c | 1 - drivers/serial/serial_msm.c | 1 - drivers/serial/serial_pic32.c | 1 - drivers/spi/atcspi200_spi.c | 1 - drivers/spi/atmel-quadspi.c | 14 +-
Re: [PATCH v2 8/9] configs: qemu: add config fragment for ACPI
On 12/16/23 19:46, Simon Glass wrote: On Fri, 15 Dec 2023 at 06:33, Heinrich Schuchardt wrote: Provide a configuration fragment to enable ACPI on QEMU. Signed-off-by: Heinrich Schuchardt Acked-by: Ilias Apalodimas --- v2: no change --- MAINTAINERS | 1 + board/emulation/configs/acpi.config | 3 +++ doc/board/emulation/acpi.rst| 23 +++ doc/board/emulation/index.rst | 1 + 4 files changed, 28 insertions(+) create mode 100644 board/emulation/configs/acpi.config create mode 100644 doc/board/emulation/acpi.rst Reviewed-by: Simon Glass Are you planning to enable binman to build configurations with config fragments? This would be needed for testing in CI. Best regards Heinrich
Re: [PATCH v2 3/9] arrm: add ACPI fields to global data
On 12/16/23 19:46, Simon Glass wrote: Hi Heinrich, On Fri, 15 Dec 2023 at 06:32, Heinrich Schuchardt wrote: Add fields for the location of ACPI tables to the global data. Signed-off-by: Heinrich Schuchardt --- v2: new patch --- arch/arm/include/asm/global_data.h | 4 1 file changed, 4 insertions(+) These need to be #ifdefed as they affect global_data size, particularly in SPL. In fact, we have a problem there. Why would it be problematic to have more unused fields in global data? Best regards Heinrich diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h index 75bd9d56f8..9505399bb1 100644 --- a/arch/arm/include/asm/global_data.h +++ b/arch/arm/include/asm/global_data.h @@ -19,6 +19,10 @@ struct arch_global_data { #if defined(CONFIG_FSL_ESDHC) || defined(CONFIG_FSL_ESDHC_IMX) u32 sdhc_clk; #endif + ulong table_start; /* Start address of ACPI tables */ + ulong table_end;/* End address of ACPI tables */ + ulong table_start_high; /* Start address of high ACPI tables */ + ulong table_end_high; /* End address of high ACPI tables */ #if defined(CONFIG_FSL_ESDHC) u32 sdhc_per_clk; -- 2.40.1 Regards, Simon
Re: [PATCH v3 4/4] acpi: support 64bit in acpi_find_table for DSDT and FACS
On 12/16/23 19:45, Simon Glass wrote: On Sat, 16 Dec 2023 at 01:12, Heinrich Schuchardt wrote: Use X_DSDT and X_FIRMWARE_CTRL if available. Signed-off-by: Heinrich Schuchardt --- lib/acpi/acpi.c | 29 - 1 file changed, 24 insertions(+), 5 deletions(-) Reviewed-by: Simon Glass This could really use a test I guess we want a test on library level separate from test/dm/acpi.c as this test will not be driver model related? Best regards Heinrich
Re: Proposal: U-Boot memory management
On 12/16/23 19:01, Simon Glass wrote: Hi, This records my thoughts after a discussion with Ilias & Heinrich re memory allocation in U-Boot. 1. malloc() malloc() is used for programmatic memory allocation. It allows memory to be freed. It is not designed for very large allocations (e.g. a 10MB kernel or 100MB ramdisk). 2. lmb lmb is used for large blocks of memory, such as those needed for a kernel or ramdisk. Allocation is only transitory, for the purposes of loading some images and booting. If the boot fails, then all lmb allocations go away. lmb is set up by getting all available memory and then removing what is used by U-Boot (code, data, malloc() space, etc.) lmb reservations have a few flags so that areas of memory can be provided with attributes There are some corner cases...e.g. loading a file does an lmb allocation but only for the purpose of avoiding a file being loaded over U-Boot code/data. The allocation is dropped immediately after the file is loaded. Within the bootm command, or when using standard boot, this would be fairly easy to solve. Linux has renamed lmb to memblock. We should consider doing the same. 3. EFI EFI has its own memory-allocation tables. Like lmb, EFI is able to deal with large allocations. But via a 'pool' function it can also do smaller allocations similar to malloc(), although each one uses at least 4KB at present. EFI allocations do not go away when a boot fails. With EFI it is possible to add allocations post facto, in which case they are added to the allocation table just as if the memory was allocated with EFI to begin with. The EFI allocations and the lmb allocations use the same memory, so in principle could conflict. EFI allocations are sometimes used to allocate internal U-Boot data as well, if needed by the EFI app. For example, while efi_image_parse() uses malloc(), efi_var_mem.c uses EFI allocations since the code runs in the app context and may need to access the memory after U-Boot has exited. Also efi_smbios.c uses allocate_pages() and then adds a new mapping as well. EFI memory has attributes, including what the memory is used for (to some degree of granularity). See enum efi_memory_type and struct efi_mem_desc. In the latter there are also attribute flags - whether memory is cacheable, etc. EFI also has the x86 idea of 'conventional' memory, meaning (I believe) that below 4GB that isn't reserved for the hardware/system. This is meaningless, or at least confusing, on ARM systems. 4. reservations It is perhaps worth mentioning a fourth method of memory management, where U-Boot reserves chunks of memory before relocation (in board_init_f.c), e.g. for the framebuffer, U-Boot code, the malloc() region, etc. Problems —--- There are no urgent problems, but here are some things that could be improved: 1. EFI should attach most of its data structures to driver model. This work has started, with the partition support, but more effort would help. This would make it easier to see which memory is related to devices and which is separate. 2. Some drivers do EFI reservations today, whether EFI is used for booting or not (e.g. rockchip video rk_vop_probe()). Hello Simon, thank you for summarizing our discussion. Some U-Boot drivers including rockchip video inform the EFI sub-system that memory is reserved. Furthermore drivers like arch/arm/mach-bcm283x/reset.c exist that are still used after ExitBootServices. mmio addresses have to be updated when Linux creates its virtual memory map. Currently this is done via efi_add_runtime_mmio(). A more UEFI style method would be to register an event handler for ExitBootServices() and use ConvertPointer() in the event handler. 3. U-Boot doesn't really map arch-specific memory attributes (e.g. armv8's struct mm_region) to EFI ones. U-Boot fails to set up RWX properties. E.g. the region where a FIT image is loaded should not be executable. 4. EFI duplicates some code from bootm, some of which relates to memory allocation (e.g. FDT fixup). Fixup code is not duplicated but invoked via image_setup_libfdt(). 5. EFI code is used even if EFI is never used to boot * Only a minimum initialization of the EFI sub-system happens in efi_init_early(). * Some EFI code is called when probing block devices because we wanted the EFI and the dm part to be integrated. * The rest of the initialization in efi_init_obj_list() is only invoked if an EFI command is invoked. 6. EFI allocations can result in the same memory being used as has already been allocated by lmb. Users may load files which overwrite memory allocated by EFI. The most worrisome issue is that EFI may allocate memory where U-Boot has loaded files like initrd as the EFI sub-system is never informed which memory is used for files. Loading files should not be possible without creating a memory reservation that becomes visible to the EFI sub-system. Lifetime We have three different memory allocators with different
Re: [PATCH v2 4/4] aci: fix acpi_find_table() for DSDT and FACS
On Fri, 15 Dec 2023 at 11:27, Heinrich Schuchardt wrote: > > Use X_DSDT and X_FIRMWARE_CTRL if available. > > Signed-off-by: Heinrich Schuchardt > --- > v2: > new patch > --- > lib/acpi/acpi.c | 29 - > 1 file changed, 24 insertions(+), 5 deletions(-) Reviewed-by: Simon Glass Could use a test.
Re: [PATCH v2 3/4] cmd: acpi: check HW reduced flag in acpi list
On Fri, 15 Dec 2023 at 11:27, Heinrich Schuchardt wrote: > > On non x86 platforms the hardware reduce flag must be set in the FADT > table. Write an error message if the flag is missing. > > Signed-off-by: Heinrich Schuchardt > --- > v2: > no change > --- > cmd/acpi.c | 4 > 1 file changed, 4 insertions(+) Reviewed-by: Simon Glass This was when the mistake (of using ACPI on ARM) was made...
Re: [v2 10/16] global: Rework architecture global_data.h to include
On Thu, 14 Dec 2023 at 11:18, Tom Rini wrote: > > In most cases, the architecture global data currently makes use of > assorted linux types, but does not include to provide > them. Add instead of relying on indirect inclusion. > > Signed-off-by: Tom Rini > --- > Changes in v2: > - Update include/system-constraints.h and board/cssi/cmpc885/sdram.c > --- > arch/mips/include/asm/global_data.h| 2 +- > arch/nios2/include/asm/global_data.h | 2 ++ > arch/powerpc/include/asm/global_data.h | 1 - > arch/riscv/include/asm/global_data.h | 1 + > arch/x86/include/asm/global_data.h | 1 + > board/cssi/cmpc885/sdram.c | 1 + > include/system-constants.h | 2 ++ > 7 files changed, 8 insertions(+), 2 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH v2 1/2] tools: kwbimage: Allow disabling build on non-mvebu platforms
On Thu, 14 Dec 2023 at 05:11, Alexander Dahl wrote: > > Some users want to build with CONFIG_TOOLS_LIBCRYPTO disabled, which in > general is possible for at least some boards. 32-bit mvebu however > requires kwbimage for building SPL, and kwbimage has a hard dependency > to host OpenSSL. > > The new symbol CONFIG_TOOLS_KWBIMAGE allows disabling kwbimage build on > non-mvebu platforms, and thus building without host libcrypto from > OpenSSL. > > Based on previous work and discussions, see links below. > > Link: https://lore.kernel.org/u-boot/20211021093304.25399-1-p...@kernel.org/ > Link: https://lore.kernel.org/u-boot/2022053120.1276641-1-ma...@denx.de/ > Link: > https://lore.kernel.org/u-boot/20230121154743.667253-2-paulerwan@gmail.com/ > Cc: Marek Vasut > Cc: Paul-Erwan Rio > Signed-off-by: Alexander Dahl > --- > > Notes: > This is more or less a mashup of the patches of Pali and Marek, but > considering the feedback given by Samuel on Pali's patch and considering > what I thought was the preferred style in other parts of the Makefile. > > Link: > https://lore.kernel.org/u-boot/f4660467-9d25-dc46-9e60-b2f7f0923...@sholland.org/ > > arch/arm/mach-mvebu/Kconfig | 1 + > tools/Kconfig | 5 + > tools/Makefile | 5 - > 3 files changed, 10 insertions(+), 1 deletion(-) Reviewed-by: Simon Glass But please drop the whitespace changes > > diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig > index c80d8587b14..2058c95ca2d 100644 > --- a/arch/arm/mach-mvebu/Kconfig > +++ b/arch/arm/mach-mvebu/Kconfig > @@ -15,6 +15,7 @@ config ARMADA_32BIT > select SUPPORT_SPL > select SYS_L2_PL310 if !SYS_L2CACHE_OFF > select TRANSLATION_OFFSET > + select TOOLS_KWBIMAGE if SPL > select SPL_SYS_NO_VECTOR_TABLE if SPL > select ARCH_VERY_EARLY_INIT > > diff --git a/tools/Kconfig b/tools/Kconfig > index 6e23f44d550..f8632cd59d0 100644 > --- a/tools/Kconfig > +++ b/tools/Kconfig > @@ -25,6 +25,11 @@ config TOOLS_LIBCRYPTO > This selection does not affect target features, such as runtime FIT > signature verification. > > +config TOOLS_KWBIMAGE > + bool "Enable kwbimage support in host tools" > + default y > + select TOOLS_LIBCRYPTO > + > config TOOLS_FIT > def_bool y > help > diff --git a/tools/Makefile b/tools/Makefile > index 1aa1e36137b..fd3b207eb96 100644 > --- a/tools/Makefile > +++ b/tools/Makefile > @@ -94,8 +94,11 @@ LIBCRYPTO_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := \ > generated/lib/fdt-libcrypto.o \ > sunxi_toc0.o > > +KWB_IMAGE_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := kwbimage.o > + > ROCKCHIP_OBS = generated/lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o > > + > # common objs for dumpimage and mkimage > dumpimage-mkimage-objs := aisimage.o \ > atmelimage.o \ > @@ -114,7 +117,7 @@ dumpimage-mkimage-objs := aisimage.o \ > imximage.o \ > imx8image.o \ > imx8mimage.o \ > - kwbimage.o \ > + $(KWB_IMAGE_OBJS-y) \ > generated/lib/md5.o \ > lpc32xximage.o \ > mxsimage.o \ > -- > 2.39.2 >
Re: [PATCH 1/1] binman: used-before-assignment in ftest.py
On Fri, 15 Dec 2023 at 16:26, Heinrich Schuchardt wrote: > > Pytest 7.4.3 complains if a variable is used in a finally clause without > having been initialized before the try clause. > > Signed-off-by: Heinrich Schuchardt > --- > tools/binman/ftest.py | 20 +++- > 1 file changed, 15 insertions(+), 5 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH v4] arm: dts: rockpro64: Add RockPro64 smbios
Hi Shantur, On Thu, 14 Dec 2023 at 07:14, Shantur Rathore wrote: > > Hi, > > On Wed, Dec 13, 2023 at 8:41 PM Simon Glass wrote: > > > > Hi Tom, > > > > On Wed, 13 Dec 2023 at 13:29, Tom Rini wrote: > > > > > > On Wed, Dec 13, 2023 at 12:50:06PM -0700, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Mon, 11 Dec 2023 at 13:53, Tom Rini wrote: > > > > > > > > > > On Mon, Dec 11, 2023 at 08:42:19PM +, Shantur Rathore wrote: > > > > > > Hi, > > > > > > > > > > > > On Mon, Nov 13, 2023 at 11:24 AM Shantur Rathore > > > > > > wrote: > > > > > > > > > > > > > > Add smbios information for Pine64 RockPro64 board and enable in > > > > > > > config > > > > > > > > > > > > > > Signed-off-by: Shantur Rathore > > > > > > > --- > > > > > > > Changes > > > > > > > v4: Change PINE64 to Pine64 > > > > > > > v3: Enable SYSINFO and SYSINFO_SMBIOS in defconfig > > > > > > > > > > > > > > arch/arm/dts/rk3399-rockpro64-u-boot.dtsi | 22 > > > > > > > ++ > > > > > > > configs/rockpro64-rk3399_defconfig| 2 ++ > > > > > > > 2 files changed, 24 insertions(+) > > > > > > > > > > > > > > diff --git a/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi > > > > > > > b/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi > > > > > > > index 732727d9b0..089732524a 100644 > > > > > > > --- a/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi > > > > > > > +++ b/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi > > > > > > > @@ -9,6 +9,28 @@ > > > > > > > chosen { > > > > > > > u-boot,spl-boot-order = "same-as-spl", > > > > > > > _flash, , > > > > > > > }; > > > > > > > + > > > > > > > +smbios { > > > > > > > +compatible = "u-boot,sysinfo-smbios"; > > > > > > > +smbios { > > > > > > > +system { > > > > > > > +manufacturer = "Pine64"; > > > > > > > +product = "RockPro64"; > > > > > > > +}; > > > [snip] > > > > > Yes, we should just defer this and pickup the SMBIOS series that Ilias > > > > > has posted. > > > > > > > > I don't think it is a suitable substitute, it is just a fallback. > > > > > > > > So I believe this patch should be applied. > > > > > > Please note that this patch is adding _less_ details than the top-level > > > model field contains today for the platform. The only difference is > > > "Pine64" vs "pine64". > > > > The top-level model is "Pine64 RockPro64 v2.1", I believe. But: > > > > - the first part of that is the manufacturer, not the product > > - the second part is what we have in this patch > > - the third part is the version > > > > SMBIOS tries to split things up into separate fields. > > > > So, perhaps a new version of this patch could add: > > > >system { > > version = "2.1"; > >}; > > > > I can add the above or any more details needed to a new patch > Will that be enough to make it merge-able >From my POV, yes. Having dug into this a bit, patches like this are the only way we can get authoritative SMBIOS data. There are various user-space workarounds, but these should not cause any problems. Regards, Simon
Re: [PATCH v2 9/9] arm: enable support for QEMU firmware tables
On Fri, 15 Dec 2023 at 06:33, Heinrich Schuchardt wrote: > > Enable the QEMU firmware interface if ACPI tables are to be supported on > the QEMU platform. > > Signed-off-by: Heinrich Schuchardt > --- > v2: > new patch > --- > board/emulation/qemu-arm/Kconfig | 1 + > 1 file changed, 1 insertion(+) > Reviewed-by: Simon Glass
Re: [PATCH v2 8/9] configs: qemu: add config fragment for ACPI
On Fri, 15 Dec 2023 at 06:33, Heinrich Schuchardt wrote: > > Provide a configuration fragment to enable ACPI on QEMU. > > Signed-off-by: Heinrich Schuchardt > Acked-by: Ilias Apalodimas > --- > v2: > no change > --- > MAINTAINERS | 1 + > board/emulation/configs/acpi.config | 3 +++ > doc/board/emulation/acpi.rst| 23 +++ > doc/board/emulation/index.rst | 1 + > 4 files changed, 28 insertions(+) > create mode 100644 board/emulation/configs/acpi.config > create mode 100644 doc/board/emulation/acpi.rst > Reviewed-by: Simon Glass
Re: [PATCH 1/1] boot: CONFIG_CEDIT must depend on CONFIG_EXPO
On Sat, 16 Dec 2023 at 08:38, Heinrich Schuchardt wrote: > > Building sandbox_defconfig with > > CONFIG_CMD_CEDIT=y > CONFIG_EXPO=n > > fails with > > cmd/cedit.c:258:(.text.do_cedit_run+0x4c): > undefined reference to `expo_apply_theme > > Fix the dependencies. > > Fixes: a0874dc4ac71 ("expo: Add a configuration editor") > Signed-off-by: Heinrich Schuchardt > --- > boot/Kconfig | 2 +- > boot/Makefile | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH v3 22/22] bootm: Create a new boot_run() function to handle booting
Hi Tom, On Sat, 16 Dec 2023 at 10:12, Tom Rini wrote: > > On Fri, Dec 15, 2023 at 08:14:26PM -0700, Simon Glass wrote: > > Create a common function used by the three existing bootz/i/m_run() > > functions, to reduce duplicated code. > > > > Signed-off-by: Simon Glass > > Suggested-by: Tom Rini > > --- > > > > Changes in v3: > > - Add new boot_run() function > > > > boot/bootm.c| 40 ++-- > > include/bootm.h | 18 ++ > > 2 files changed, 32 insertions(+), 26 deletions(-) > > > > diff --git a/boot/bootm.c b/boot/bootm.c > > index 53236136f489..6a4cebcf7a08 100644 > > --- a/boot/bootm.c > > +++ b/boot/bootm.c > > @@ -1123,47 +1123,35 @@ err: > > return ret; > > } > > > > -int bootm_run(struct bootm_info *bmi) > > +int boot_run(struct bootm_info *bmi, const char *cmd, int extra_states) > > { > > int states; > > > > - bmi->cmd_name = "bootm"; > > - states = BOOTM_STATE_START | BOOTM_STATE_FINDOS | > > BOOTM_STATE_PRE_LOAD | > > - BOOTM_STATE_FINDOTHER | BOOTM_STATE_LOADOS | > > - BOOTM_STATE_OS_PREP | BOOTM_STATE_OS_FAKE_GO | > > - BOOTM_STATE_OS_GO | BOOTM_STATE_MEASURE; > > + bmi->cmd_name = cmd; > > + states = BOOTM_STATE_MEASURE | BOOTM_STATE_OS_PREP | > > + BOOTM_STATE_OS_FAKE_GO | BOOTM_STATE_OS_GO; > > if (IS_ENABLED(CONFIG_SYS_BOOT_RAMDISK_HIGH)) > > states |= BOOTM_STATE_RAMDISK; > > - if (IS_ENABLED(CONFIG_PPC) || IS_ENABLED(CONFIG_MIPS)) > > - states |= BOOTM_STATE_OS_CMDLINE; > > + states |= extra_states; > > > > return bootm_run_states(bmi, states); > > } > > > > -int bootz_run(struct bootm_info *bmi) > > +int bootm_run(struct bootm_info *bmi) > > { > > - int states; > > - > > - bmi->cmd_name = "bootz"; > > - states = BOOTM_STATE_MEASURE | BOOTM_STATE_OS_PREP | > > - BOOTM_STATE_OS_FAKE_GO | BOOTM_STATE_OS_GO; > > - if (IS_ENABLED(CONFIG_SYS_BOOT_RAMDISK_HIGH)) > > - states |= BOOTM_STATE_RAMDISK; > > + return boot_run(bmi, "bootm", BOOTM_STATE_START | BOOTM_STATE_FINDOS | > > + BOOTM_STATE_PRE_LOAD | BOOTM_STATE_FINDOTHER | > > + BOOTM_STATE_LOADOS); > > +} > > > > - return bootm_run_states(bmi, states); > > +int bootz_run(struct bootm_info *bmi) > > +{ > > + return boot_run(bmi, "bootz", 0); > > } > > > > int booti_run(struct bootm_info *bmi) > > { > > - int states; > > - > > - bmi->cmd_name = "booti"; > > - states = BOOTM_STATE_MEASURE | BOOTM_STATE_OS_PREP | > > - BOOTM_STATE_OS_FAKE_GO | BOOTM_STATE_OS_GO; > > - if (IS_ENABLED(CONFIG_SYS_BOOT_RAMDISK_HIGH)) > > - states |= BOOTM_STATE_RAMDISK; > > - > > - return bootm_run_states(bmi, states); > > + return boot_run(bmi, "booti", 0); > > } > > > > int bootm_boot_start(ulong addr, const char *cmdline) > > diff --git a/include/bootm.h b/include/bootm.h > > index eba35b33b4e5..9e0f8d60de0a 100644 > > --- a/include/bootm.h > > +++ b/include/bootm.h > > @@ -150,6 +150,24 @@ int bootm_measure(struct bootm_headers *images); > > */ > > int bootm_run_states(struct bootm_info *bmi, int states); > > > > +/** > > + * boot_run() - Run the entire bootm/booti/bootz process > > + * > > + * This runs through the boot process from start to finish, with a base > > set of > > + * states, along with the extra ones supplied. > > + * > > + * This uses bootm_run_states(). > > + * > > + * Note that it is normally easier to use bootm_run(), etc. since they > > handle > > + * the extra states correctly. > > + * > > + * @bmi: bootm information > > + * @cmd: command being run, NULL if none > > + * @extra_states: Mask of extra states to use for the boot > > + * Return: 0 if ok, something else on error > > + */ > > +int boot_run(struct bootm_info *bmi, const char *cmd, int extra_states); > > + > > /** > > * bootm_run() - Run the entire bootm process > > * > > Overall good, thanks. My question is, should we mark the others as > static, or is this new helper static and not to be called externally? The others are used externally. For bootm_run() I toyed with the idea of it being static, but then wondered if we might use that programmatically instead of the bootz/i/m(_run) versions. It is quite nice that the differences are now pretty minor between them. I don't mind either way, though. Regards, Simon
Re: [PATCH v2 3/9] arrm: add ACPI fields to global data
Hi Heinrich, On Fri, 15 Dec 2023 at 06:32, Heinrich Schuchardt wrote: > > Add fields for the location of ACPI tables to the global data. > > Signed-off-by: Heinrich Schuchardt > --- > v2: > new patch > --- > arch/arm/include/asm/global_data.h | 4 > 1 file changed, 4 insertions(+) These need to be #ifdefed as they affect global_data size, particularly in SPL. In fact, we have a problem there. > > diff --git a/arch/arm/include/asm/global_data.h > b/arch/arm/include/asm/global_data.h > index 75bd9d56f8..9505399bb1 100644 > --- a/arch/arm/include/asm/global_data.h > +++ b/arch/arm/include/asm/global_data.h > @@ -19,6 +19,10 @@ struct arch_global_data { > #if defined(CONFIG_FSL_ESDHC) || defined(CONFIG_FSL_ESDHC_IMX) > u32 sdhc_clk; > #endif > + ulong table_start; /* Start address of ACPI tables */ > + ulong table_end;/* End address of ACPI tables */ > + ulong table_start_high; /* Start address of high ACPI tables > */ > + ulong table_end_high; /* End address of high ACPI tables */ > > #if defined(CONFIG_FSL_ESDHC) > u32 sdhc_per_clk; > -- > 2.40.1 > Regards, Simon
Re: [PATCH] get_maintainer.pl: Add --git to look up CCed in git history
On Sat, 16 Dec 2023 at 11:08, Marek Vasut wrote: > > Add the --git parameter, else recent contributors are left out of the CC list. > > Signed-off-by: Marek Vasut > --- > Cc: Simon Glass > Cc: Tom Rini > --- > .get_maintainer.conf | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Simon Glass
Re: Where is the u-boot for BeagleV-Fire?
Hi, On Fri, 15 Dec 2023 at 06:15, Pei Jia wrote: > > > Where can I find the u-boot defconfig file for BeagleV-Fire? > > I've got an issue posted at > https://forum.beagleboard.org/t/error-illegal-operands-li-t1/36823 . You could check here, or ask the vendor? https://github.com/orgs/beagleboard/repositories Regards, Simon
Re: [PATCH] global: Restrict use of '#include '
On Thu, 14 Dec 2023 at 05:16, Tom Rini wrote: > > In general terms, we -include include/linux/kconfig.h and so normal > U-Boot code does not need to also #include it. However, for code which > is shared with userspace we may need to add it so that either our full > config is available or so that macros such as CONFIG_IS_ENABLED() can be > evaluated. In this case make sure that we guard these includes with a > test for USE_HOSTCC so that it clear as to why we're doing this. > > Signed-off-by: Tom Rini > --- > Cc: Simon Glass > --- > arch/arm/include/asm/arch-fsl-layerscape/config.h | 1 - > arch/arm/mach-rockchip/tpl.c | 1 - > arch/arm/mach-sunxi/dram_sun50i_h6.c | 1 - > arch/arm/mach-sunxi/dram_sun50i_h616.c| 1 - > arch/arm/mach-sunxi/dram_sunxi_dw.c | 1 - > boot/image-fit.c | 2 +- > boot/image.c | 2 +- > drivers/timer/dw-apb-timer.c | 1 - > env/embedded.c| 2 ++ > include/bootstage.h | 2 ++ > include/configs/at91-sama5_common.h | 2 -- > include/configs/tqma6.h | 1 - > include/env_internal.h| 1 - > include/u-boot/ecdsa.h| 1 - > lib/rsa/rsa-verify.c | 2 +- > test/dm/scmi.c| 1 - > tools/mkeficapsule.c | 1 - > 17 files changed, 7 insertions(+), 16 deletions(-) > Reviewed-by: Simon Glass
Re: [v2 13/16] efi_loader: Remove
On Thu, 14 Dec 2023 at 11:19, Tom Rini wrote: > > We largely do not need in these files, so drop it. The only > exception here is that efi_freestanding.c needs and had > been getting that via . > > Signed-off-by: Tom Rini > --- > lib/efi_loader/dtbdump.c | 1 - > lib/efi_loader/efi_acpi.c | 1 - > lib/efi_loader/efi_bootmgr.c | 1 - > lib/efi_loader/efi_boottime.c | 1 - > lib/efi_loader/efi_capsule.c | 1 - > lib/efi_loader/efi_conformance.c | 1 - > lib/efi_loader/efi_console.c | 1 - > lib/efi_loader/efi_device_path.c | 1 - > lib/efi_loader/efi_device_path_to_text.c | 1 - > lib/efi_loader/efi_device_path_utilities.c | 1 - > lib/efi_loader/efi_disk.c | 1 - > lib/efi_loader/efi_dt_fixup.c | 1 - > lib/efi_loader/efi_esrt.c | 1 - > lib/efi_loader/efi_file.c | 1 - > lib/efi_loader/efi_firmware.c | 1 - > lib/efi_loader/efi_freestanding.c | 2 +- > lib/efi_loader/efi_gop.c | 1 - > lib/efi_loader/efi_helper.c| 1 - > lib/efi_loader/efi_hii.c | 1 - > lib/efi_loader/efi_hii_config.c| 1 - > lib/efi_loader/efi_image_loader.c | 1 - > lib/efi_loader/efi_load_initrd.c | 1 - > lib/efi_loader/efi_load_options.c | 1 - > lib/efi_loader/efi_memory.c| 1 - > lib/efi_loader/efi_net.c | 1 - > lib/efi_loader/efi_riscv.c | 1 - > lib/efi_loader/efi_rng.c | 1 - > lib/efi_loader/efi_root_node.c | 1 - > lib/efi_loader/efi_runtime.c | 1 - > lib/efi_loader/efi_setup.c | 1 - > lib/efi_loader/efi_signature.c | 1 - > lib/efi_loader/efi_smbios.c| 1 - > lib/efi_loader/efi_string.c| 1 - > lib/efi_loader/efi_tcg2.c | 1 - > lib/efi_loader/efi_unicode_collation.c | 1 - > lib/efi_loader/efi_var_common.c| 1 - > lib/efi_loader/efi_var_file.c | 1 - > lib/efi_loader/efi_var_mem.c | 1 - > lib/efi_loader/efi_variable.c | 1 - > lib/efi_loader/efi_variable_tee.c | 1 - > lib/efi_loader/efi_watchdog.c | 1 - > lib/efi_loader/initrddump.c| 1 - > 42 files changed, 1 insertion(+), 42 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH v2 5/9] acpi: enable writing ACPI tables on QEMU
Hi Heinrich, On Fri, 15 Dec 2023 at 06:33, Heinrich Schuchardt wrote: > > Invoke write_acpi_tables() via EVT_LAST_STAGE_INIT on QEMU except on X86. > X86 calls write_acpi_tables() in write_tables(). > > Signed-off-by: Heinrich Schuchardt > --- > v2: > new patch > --- > drivers/misc/qfw_acpi.c | 25 + > 1 file changed, 25 insertions(+) > This is fine for now. Reviewed-by: Simon Glass I would like to collect all tables generation into one piece, using a bloblist to hold them, so they are contiguous. This is implemented on x86, but is optional. Still, this patch gets things running and makes it possible to make such a change later. > diff --git a/drivers/misc/qfw_acpi.c b/drivers/misc/qfw_acpi.c > index 6e14b2a504..7ffed1e8c0 100644 > --- a/drivers/misc/qfw_acpi.c > +++ b/drivers/misc/qfw_acpi.c > @@ -9,9 +9,11 @@ > #include > #include > #include > +#include > #include > #include > #include > +#include > #include > #include > > @@ -254,3 +256,26 @@ ulong acpi_get_rsdp_addr(void) > file = qfw_find_file(dev, "etc/acpi/rsdp"); > return file->addr; > } > + > +#ifndef CONFIG_X86 > +static int evt_write_acpi_tables(void) > +{ > + ulong addr, end; > + void *ptr; > + > + /* Reserve 64K for ACPI tables, aligned to a 4K boundary */ > + ptr = memalign(SZ_4K, SZ_64K); > + if (!ptr) > + return -ENOMEM; > + addr = map_to_sysmem(ptr); > + > + /* Generate ACPI tables */ > + end = write_acpi_tables(addr); > + gd->arch.table_start = addr; > + gd->arch.table_end = addr; > + > + return 0; > +} > + > +EVENT_SPY_SIMPLE(EVT_LAST_STAGE_INIT, evt_write_acpi_tables); > +#endif > -- > 2.40.1 > Regards, Simon
Re: [PATCH 2/2 v3] smbios: Fallback to the default DT if sysinfo nodes are missing
Hi Tom, On Thu, 14 Dec 2023 at 06:11, Tom Rini wrote: > > On Wed, Dec 13, 2023 at 08:19:11PM -0700, Simon Glass wrote: > > [snip] > > The new DT nodes / SMBIOS binding [1] allows for the correct > > information to be provided, though. > [snip] > > [1] > > https://github.com/u-boot/u-boot/blob/master/doc/device-tree-bindings/sysinfo/smbios.txt > > I think the only feedback I can give on your message here is to please > go upstream that binding, and then we can see what to do afterwards. I am still tearing my hair out waiting for the reserved-memory and binman patches to be accepted. Every few weeks there is another comment, but no action. Rob seems to be the only one engaged. Perhaps I should do a conference talk about what is wrong with DT? This is my experience so far: - there is no urgency to apply anything - no one wins acclaim for applying a patch - everyone complains later if a patch is applied that they didn't agree with - people chime in wanting to understand the use case, but don't/can't/won't - any sort of expressed doubt results in delay - maintainers hope that the submitter will lose interest and go away - not enough people add review tags, even if they look at it ... because they are worried about looking bad to others on the ML I would be happy to discuss how to improve matters, but that is what I see. Anyway, the lowest-hanging fruit at present is the U-Boot /options stuff. I was hoping someone else would step up to clean it up. There should be no impediment. Regards, Simon
Re: [v2 11/16] lib/sha*.c: Update header list
On Thu, 14 Dec 2023 at 11:18, Tom Rini wrote: > > Cleanup the list of headers we include here. For the tools build we only > need to exclude as that's used by the target build for the > prototype for schedule(), and we don't need to get that via > . We can also make use of our intentionally > existing as a redirection to to reduce ifdef'd lines. > > Signed-off-by: Tom Rini > --- > lib/sha1.c | 7 ++- > lib/sha256.c | 7 ++- > lib/sha512.c | 6 +- > 3 files changed, 5 insertions(+), 15 deletions(-) Reviewed-by: Simon Glass
Re: Booting fails on the sandbox
Hi Heinrich, On Sat, 16 Dec 2023 at 07:50, Heinrich Schuchardt wrote: > > Hello Simon, > > I have built sandbox_defconfig with PREBOOT='host bind 0 ../sct.img' > > On the image I have an ESP with file EFI/BOOT/BOOTX64.EFI. > > I would expect the sandbox to boot from the ESP. > > But when starting the sandbox is does not boot. Instead it shows a menu > with some Fedora related entry: > > Fedora-Workstation-armhfp-31-1.9 Boot Options. > 1: Fedora-Workstation-armhfp-31-1.9 (5.3.7-301.fc31.armv7hl) > > This makes absolutely no sense on an x86_64 system. Could you, please, > fix sandbox_defconfig to boot according to distroboot. What sort of 'fix' are you thinking of? You should be able to discover all the options with 'bootflow scan' and then use 'bootflow select' or 'bootflow menu' to choose which one you want. The Fedora thing is used for testing and development. Regards. Simon
Re: Where to find those definition for particular boards? CFG_SYS_INIT_RAM_ADDR, CFG_SYS_INIT_RAM_SIZE, GENERATED_GBL_DATA_SIZE? etc.
Hi, On Fri, 15 Dec 2023 at 06:15, Pei Jia wrote: > > > Hi, all: > > I'm using a *BeagleV-Fire*, However, I failed to build *u-boot*, and now > traced to this step. Where to find those *macro values* specifically for > this board *BeagleV-Fire*? I assume that beaglebone.org has a U-Boot for this board. Perhaps it is somewhere at [1] ? If you are wanting to do the port yourself, then the SoC docs should provide the memory map. CUSTOM_SYS_INIT_SP_ADDR could be set to somewhere near the top of RAM, perhaps? It is just used to start up U-Boot and prepare for relocation. > > > #ifdef CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR > #define SYS_INIT_SP_ADDRCONFIG_CUSTOM_SYS_INIT_SP_ADDR > #else > #ifdef CONFIG_MIPS > #define SYS_INIT_SP_ADDR(CFG_SYS_SDRAM_BASE + CFG_SYS_INIT_SP_OFFSET) > #else > #define SYS_INIT_SP_ADDR\ > (CFG_SYS_INIT_RAM_ADDR + CFG_SYS_INIT_RAM_SIZE - > GENERATED_GBL_DATA_SIZE) > #endif > #endif > > > Cheers > > > -- > Dr. Pei Jia > Email: jia...@longervision.com > Cell: +1 778-863-5816 > Website: https://www.longervision.ca Regards, Simon [1] https://github.com/orgs/beagleboard/repositories
Re: [PATCH] arm: Add OVERLAY command to BSS section
On Sat, 16 Dec 2023 at 11:08, Marek Vasut wrote: > > Avoid allocating and loading the BSS section. > > $ aarch64-linux-gnu-objdump -Sh u-boot > > Before: > 10 .bss_start 000f21d8 000f21d8 001021d8 > 2**0 >CONTENTS, ALLOC, LOAD, DATA > 11 .bss 68f8 000f2200 000f2200 001021d8 > 2**6 >ALLOC > 12 .bss_end 000f8af8 000f8af8 00108af8 > 2**0 >CONTENTS, ALLOC, LOAD, DATA > > After: > 10 .bss_start 000f21d8 000f21d8 001021d8 > 2**0 >CONTENTS > 11 .bss 68f8 000f2200 000f2200 00102200 > 2**6 >CONTENTS > 12 .bss_end 000f21d8 000f21d8 00108af8 > 2**0 >CONTENTS This one seems wrong to me. I would expect to see it after the end of .bss > > Signed-off-by: Marek Vasut > --- > Cc: Simon Glass > Cc: Tom Rini > --- > arch/arm/cpu/armv8/u-boot.lds | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Regards, Simon
Re: [PATCH 1/1] test: build test/boot for CONFIG_UT_BOOTSTD=n
On Sat, 16 Dec 2023 at 08:20, Heinrich Schuchardt wrote: > > Building sandbox_defconfig with > > CONFIG_UT_BOOTSTD=n > CONFIG_MEASURMENT=y > > results in an error: > > /usr/bin/ld: test/cmd_ut.o:(.data.rel.cmd_ut_sub+0x408): > undefined reference to `do_ut_measurement' > > Fixes: 5999ea20fa42 ("test: Add sandbox TPM boot measurement") > Signed-off-by: Heinrich Schuchardt > --- > test/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Simon Glass This looks like the best fix, to me. > > diff --git a/test/Makefile b/test/Makefile > index 6b8a1506f5..feba097543 100644 > --- a/test/Makefile > +++ b/test/Makefile > @@ -23,7 +23,7 @@ obj-$(CONFIG_UT_TIME) += time_ut.o > obj-y += ut.o > > ifeq ($(CONFIG_SPL_BUILD),) > -obj-$(CONFIG_$(SPL_)UT_BOOTSTD) += boot/ > +obj-y += boot/ > obj-$(CONFIG_UNIT_TEST) += common/ > obj-y += log/ > obj-$(CONFIG_$(SPL_)UT_UNICODE) += unicode_ut.o > -- > 2.40.1 >
Re: [v2 03/16] arm: Partial cleanup and audit usage of
On Thu, 14 Dec 2023 at 11:17, Tom Rini wrote: > > We need to include directly when a file needs to have > something such as CFG_SYS_SDRAM_SIZE referenced as this file is not > automatically globally included and is most commonly indirectly included > via common.h. Remove most cases of arm including config.h directly, but > add it where needed. This includes a few board-specific fixes. > > Signed-off-by: Tom Rini > --- > Changes in v2: > - Update arch/arm/mach-keystone/cmd_mon.c > --- > arch/arm/include/asm/arch-fsl-layerscape/fsl_serdes.h | 2 -- > arch/arm/include/asm/arch-ls102xa/fsl_serdes.h| 2 -- > arch/arm/include/asm/assembler.h | 1 - > arch/arm/include/asm/secure.h | 1 - > arch/arm/include/asm/string.h | 2 -- > arch/arm/mach-davinci/include/mach/pinmux_defs.h | 1 - > arch/arm/mach-exynos/exynos4_setup.h | 1 - > arch/arm/mach-exynos/exynos5_setup.h | 1 - > arch/arm/mach-k3/common.c | 1 + > arch/arm/mach-k3/include/mach/clock.h | 2 -- > arch/arm/mach-k3/include/mach/j721e_hardware.h| 1 - > arch/arm/mach-k3/include/mach/j721s2_hardware.h | 1 - > arch/arm/mach-keystone/cmd_mon.c | 1 + > board/freescale/ls1088a/eth_ls1088aqds.c | 1 + > board/freescale/ls2080aqds/eth.c | 1 + > board/toradex/verdin-am62/verdin-am62.c | 1 + > drivers/ddr/marvell/axp/ddr3_axp.h| 2 ++ > 17 files changed, 7 insertions(+), 15 deletions(-) Reviewed-by: Simon Glass
Re: [v2 04/16] sandbox: Audit config.h and common.h usage
On Thu, 14 Dec 2023 at 11:17, Tom Rini wrote: > > Remove and replace common.h and config.h in sandbox when it's not needed > and add some explicit includes where needed. > > Signed-off-by: Tom Rini > --- > Changes in v2: > - Drop from arch/sandbox/cpu/state.c as it's also not > needed and leads to unrelated build failures with CMDLINE=n > --- > arch/sandbox/cpu/cache.c | 1 - > arch/sandbox/cpu/cpu.c | 1 - > arch/sandbox/cpu/sdl.c | 2 +- > arch/sandbox/cpu/spl.c | 1 - > arch/sandbox/cpu/start.c | 2 +- > arch/sandbox/cpu/state.c | 3 +-- > arch/sandbox/include/asm/io.h| 2 ++ > arch/sandbox/include/asm/state.h | 1 - > arch/sandbox/lib/bootm.c | 1 - > arch/sandbox/lib/fdt_fixup.c | 1 - > arch/sandbox/lib/interrupts.c| 1 - > arch/sandbox/lib/pci_io.c| 1 - > board/sandbox/sandbox.c | 2 +- > 13 files changed, 6 insertions(+), 13 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH 1/1] test: CONFIG_UT_BOOTSTD must depend on CONFIG_BOOTSTD
On Sat, 16 Dec 2023 at 08:46, Heinrich Schuchardt wrote: > > Building sandbox_defconfig with > > CONFIG_BOOTSTD=n > CONFIG_UT_BOOTSTD=y > > leads to an error > > /usr/bin/ld: test/cmd_ut.o:(.data.rel.cmd_ut_sub+0xc0): > undefined reference to `do_ut_bootstd' > > Add the missing dependency. > > Signed-off-by: Heinrich Schuchardt > --- > test/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Simon Glass
Re: [PATCH v3 1/4] acpi: use 64-bit addresses in FADT table
On Sat, 16 Dec 2023 at 01:12, Heinrich Schuchardt wrote: > > Fields X_FIRMWAE_CTRL and X_DSDT must be 64bit wide. Convert pointers to > to uintptr_t to fill these. > > If field X_FIRMWARE_CTRL is filled, field FIRMWARE must be ignored. If > field X_DSDT is filled, field DSDT must be ignored. We should not fill > unused fields. > > See the field definitions in chapter "5.2.9 Fixed ACPI Description Table > (FADT)" of the ACPI Specification 6.5. > > Signed-off-by: Heinrich Schuchardt > --- > arch/x86/cpu/baytrail/acpi.c | 9 +++-- > arch/x86/cpu/quark/acpi.c| 9 +++-- > arch/x86/cpu/tangier/acpi.c | 9 +++-- > arch/x86/lib/acpi_table.c| 9 ++--- > include/acpi/acpi_table.h| 6 ++ > 5 files changed, 13 insertions(+), 29 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v3 4/4] acpi: support 64bit in acpi_find_table for DSDT and FACS
On Sat, 16 Dec 2023 at 01:12, Heinrich Schuchardt wrote: > > Use X_DSDT and X_FIRMWARE_CTRL if available. > > Signed-off-by: Heinrich Schuchardt > --- > lib/acpi/acpi.c | 29 - > 1 file changed, 24 insertions(+), 5 deletions(-) Reviewed-by: Simon Glass This could really use a test
Re: [PATCH v3 2/4] cmd: acpi: fix listing DSDT and FACS
On Sat, 16 Dec 2023 at 01:12, Heinrich Schuchardt wrote: > > If field X_FIRMWARE_CTRL is filled, field FIRMWARE must be ignored. If > field X_DSDT is filled, field DSDT must be ignored. > > Signed-off-by: Heinrich Schuchardt > --- > cmd/acpi.c | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v3 3/4] cmd: acpi: check HW reduced flag in acpi list
On Sat, 16 Dec 2023 at 01:12, Heinrich Schuchardt wrote: > > On non x86 platforms the hardware reduce flag must be set in the FADT > table. Write an error message if the flag is missing. > > Signed-off-by: Heinrich Schuchardt > --- > cmd/acpi.c | 4 > 1 file changed, 4 insertions(+) Reviewed-by: Simon Glass
Re: [PATCH] get_maintainer.pl: Add --git to look up CCed in git history
On Sat, Dec 16, 2023 at 07:07:52PM +0100, Marek Vasut wrote: > Add the --git parameter, else recent contributors are left out of the CC list. > > Signed-off-by: Marek Vasut Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH] arm: Add OVERLAY command to BSS section
On Sat, Dec 16, 2023 at 07:08:22PM +0100, Marek Vasut wrote: > Avoid allocating and loading the BSS section. > > $ aarch64-linux-gnu-objdump -Sh u-boot > > Before: > 10 .bss_start 000f21d8 000f21d8 001021d8 > 2**0 >CONTENTS, ALLOC, LOAD, DATA > 11 .bss 68f8 000f2200 000f2200 001021d8 > 2**6 >ALLOC > 12 .bss_end 000f8af8 000f8af8 00108af8 > 2**0 >CONTENTS, ALLOC, LOAD, DATA > > After: > 10 .bss_start 000f21d8 000f21d8 001021d8 > 2**0 >CONTENTS > 11 .bss 68f8 000f2200 000f2200 00102200 > 2**6 >CONTENTS > 12 .bss_end 000f21d8 000f21d8 00108af8 > 2**0 >CONTENTS > > Signed-off-by: Marek Vasut > --- > Cc: Simon Glass > Cc: Tom Rini > --- > arch/arm/cpu/armv8/u-boot.lds | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds > index fb6a30c922f..25838b9892c 100644 > --- a/arch/arm/cpu/armv8/u-boot.lds > +++ b/arch/arm/cpu/armv8/u-boot.lds > @@ -151,16 +151,16 @@ SECTIONS > > . = ALIGN(8); > > - .bss_start : { > + .bss_start (OVERLAY) : { > KEEP(*(.__bss_start)); > } > > - .bss : { > + .bss (OVERLAY) : { > *(.bss*) >. = ALIGN(8); > } > > - .bss_end : { > + .bss_end (OVERLAY) : { > KEEP(*(.__bss_end)); > } This is, I think not sufficient. Looking at arch/arm/cpu/u-boot.lds, seeing the comment about overlay ordering (and that it needs to point to arch/arm/lib/sections.c not bss.c now) and looking harder at arch/arm/cpu/armv8/u-boot.lds I don't know what to think at first about what's going on here. My guess is that what v7 has to / had to do for relocation / etc stuff back at the time is just done differently with the v8 infrastructure (and possibly v7m). -- Tom signature.asc Description: PGP signature
[PATCH] arm: Add OVERLAY command to BSS section
Avoid allocating and loading the BSS section. $ aarch64-linux-gnu-objdump -Sh u-boot Before: 10 .bss_start 000f21d8 000f21d8 001021d8 2**0 CONTENTS, ALLOC, LOAD, DATA 11 .bss 68f8 000f2200 000f2200 001021d8 2**6 ALLOC 12 .bss_end 000f8af8 000f8af8 00108af8 2**0 CONTENTS, ALLOC, LOAD, DATA After: 10 .bss_start 000f21d8 000f21d8 001021d8 2**0 CONTENTS 11 .bss 68f8 000f2200 000f2200 00102200 2**6 CONTENTS 12 .bss_end 000f21d8 000f21d8 00108af8 2**0 CONTENTS Signed-off-by: Marek Vasut --- Cc: Simon Glass Cc: Tom Rini --- arch/arm/cpu/armv8/u-boot.lds | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds index fb6a30c922f..25838b9892c 100644 --- a/arch/arm/cpu/armv8/u-boot.lds +++ b/arch/arm/cpu/armv8/u-boot.lds @@ -151,16 +151,16 @@ SECTIONS . = ALIGN(8); - .bss_start : { + .bss_start (OVERLAY) : { KEEP(*(.__bss_start)); } - .bss : { + .bss (OVERLAY) : { *(.bss*) . = ALIGN(8); } - .bss_end : { + .bss_end (OVERLAY) : { KEEP(*(.__bss_end)); } -- 2.43.0
[PATCH] get_maintainer.pl: Add --git to look up CCed in git history
Add the --git parameter, else recent contributors are left out of the CC list. Signed-off-by: Marek Vasut --- Cc: Simon Glass Cc: Tom Rini --- .get_maintainer.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.get_maintainer.conf b/.get_maintainer.conf index df595f5420d..f916cfbe480 100644 --- a/.get_maintainer.conf +++ b/.get_maintainer.conf @@ -1 +1 @@ ---find-maintainer-files --maintainer-path=. +--find-maintainer-files --git --maintainer-path=. -- 2.43.0
Proposal: U-Boot memory management
Hi, This records my thoughts after a discussion with Ilias & Heinrich re memory allocation in U-Boot. 1. malloc() malloc() is used for programmatic memory allocation. It allows memory to be freed. It is not designed for very large allocations (e.g. a 10MB kernel or 100MB ramdisk). 2. lmb lmb is used for large blocks of memory, such as those needed for a kernel or ramdisk. Allocation is only transitory, for the purposes of loading some images and booting. If the boot fails, then all lmb allocations go away. lmb is set up by getting all available memory and then removing what is used by U-Boot (code, data, malloc() space, etc.) lmb reservations have a few flags so that areas of memory can be provided with attributes There are some corner cases...e.g. loading a file does an lmb allocation but only for the purpose of avoiding a file being loaded over U-Boot code/data. The allocation is dropped immediately after the file is loaded. Within the bootm command, or when using standard boot, this would be fairly easy to solve. Linux has renamed lmb to memblock. We should consider doing the same. 3. EFI EFI has its own memory-allocation tables. Like lmb, EFI is able to deal with large allocations. But via a 'pool' function it can also do smaller allocations similar to malloc(), although each one uses at least 4KB at present. EFI allocations do not go away when a boot fails. With EFI it is possible to add allocations post facto, in which case they are added to the allocation table just as if the memory was allocated with EFI to begin with. The EFI allocations and the lmb allocations use the same memory, so in principle could conflict. EFI allocations are sometimes used to allocate internal U-Boot data as well, if needed by the EFI app. For example, while efi_image_parse() uses malloc(), efi_var_mem.c uses EFI allocations since the code runs in the app context and may need to access the memory after U-Boot has exited. Also efi_smbios.c uses allocate_pages() and then adds a new mapping as well. EFI memory has attributes, including what the memory is used for (to some degree of granularity). See enum efi_memory_type and struct efi_mem_desc. In the latter there are also attribute flags - whether memory is cacheable, etc. EFI also has the x86 idea of 'conventional' memory, meaning (I believe) that below 4GB that isn't reserved for the hardware/system. This is meaningless, or at least confusing, on ARM systems. 4. reservations It is perhaps worth mentioning a fourth method of memory management, where U-Boot reserves chunks of memory before relocation (in board_init_f.c), e.g. for the framebuffer, U-Boot code, the malloc() region, etc. Problems —--- There are no urgent problems, but here are some things that could be improved: 1. EFI should attach most of its data structures to driver model. This work has started, with the partition support, but more effort would help. This would make it easier to see which memory is related to devices and which is separate. 2. Some drivers do EFI reservations today, whether EFI is used for booting or not (e.g. rockchip video rk_vop_probe()). 3. U-Boot doesn't really map arch-specific memory attributes (e.g. armv8's struct mm_region) to EFI ones. 4. EFI duplicates some code from bootm, some of which relates to memory allocation (e.g. FDT fixup). 5. EFI code is used even if EFI is never used to boot 6. EFI allocations can result in the same memory being used as has already been allocated by lmb. Users may load files which overwrite memory allocated by EFI. Lifetime We have three different memory allocators with different purposes. Can we unify them a little? Within U-Boot: - malloc() space lives forever - lmb lives while setting out images for booting - EFI (mostly) lives while booting an EFI app In practice, EFI is set up early in U-Boot. Some of this is necessary, some not. EFI allocations stay around forever. This works OK since large allocations are normally not done in EFI, so memory isn't really consumed to any great degree by the boot process. What happens to EFI allocations if the app returns? They are still present, in case another app is run. This seems fine. API –-- Can we unify some APIs? It should be possible to use lmb for large EFI memory allocations, so long as they are only needed for booting. We effectively do this today, since EFI does not manage the arrangement of loaded images in memory. for the most part. It would not make sense to use EFI allocation to replace lmb and malloc(), of course. Could we use a common (lower-level) API for allocation, used by both lmb and EFI? They do have some similarities. However they have different lifetime constraints (EFI allocations are never dropped, unlikely lmb). ** Overall, it seems that the existence of memory allocation in boot-time services has created confusion. Memory allocation is muddled, with both U-Boot code and boot-time services calling the same memory allocator. This
Re: [PATCH v1 0/8] Convert Tegra pinctrl to DM
сб, 9 груд. 2023 р. о 16:14 Tom Rini пише: > > On Mon, Dec 04, 2023 at 04:45:38PM +0200, Svyatoslav Ryhel wrote: > > пн, 4 груд. 2023 р. о 12:26 Thierry Reding пише: > > > > > > On Mon, Dec 04, 2023 at 10:20:46AM +0200, Svyatoslav Ryhel wrote: > > > > Create a DM supported wrapper arround existing Tegra pinmux logic. > > > > This implementation is backwards compatible with all existing board > > > > setups and early stages of setup. All new boards must be device tree > > > > based. Linux dts pinmux nodes are fully compatible with this driver. > > > > > > > > As an example I have converted recently merged T30 boards to this > > > > layout (I have those devices and was able to test them properly) and > > > > Paz00 T20 board which was tested as well by owner. > > > > > > > > Svyatoslav Ryhel (8): > > > > ARM: mach-tegra: rearrange SPL configs > > > > drivers: pinctrl: create Tegra DM pinctrl driver > > > > drivers: pinctrl: tegra: incorporate existing code > > > > board: asus: grouper: switch to DM pinmux > > > > board: lg: x3-t30: switch to DM pinmux > > > > board: asus: transformer: switch to DM pinmux > > > > board: htc: endeavoru: switch to DM pinmux > > > > board: compal: paz00: clean up the board > > > > > > > > arch/arm/dts/tegra20-paz00.dts|4 +- > > > > arch/arm/dts/tegra30-asus-grouper-common.dtsi | 712 ++ > > > > .../dts/tegra30-asus-nexus7-grouper-E1565.dts | 113 ++ > > > > .../dts/tegra30-asus-nexus7-grouper-PM269.dts | 113 ++ > > > > .../dts/tegra30-asus-nexus7-tilapia-E1565.dts | 149 +++ > > > > arch/arm/dts/tegra30-asus-p1801-t.dts | 982 ++ > > > > arch/arm/dts/tegra30-asus-tf201.dts | 45 + > > > > arch/arm/dts/tegra30-asus-tf300t.dts | 45 + > > > > arch/arm/dts/tegra30-asus-tf300tg.dts | 128 ++ > > > > arch/arm/dts/tegra30-asus-tf300tl.dts | 163 +++ > > > > arch/arm/dts/tegra30-asus-tf600t.dts | 889 + > > > > arch/arm/dts/tegra30-asus-tf700t.dts | 53 + > > > > arch/arm/dts/tegra30-asus-transformer.dtsi| 984 ++ > > > > arch/arm/dts/tegra30-htc-endeavoru.dts| 1147 + > > > > arch/arm/dts/tegra30-lg-p880.dts | 90 ++ > > > > arch/arm/dts/tegra30-lg-p895.dts | 93 ++ > > > > arch/arm/dts/tegra30-lg-x3.dtsi | 845 > > > > arch/arm/include/asm/arch-tegra114/pinmux.h | 303 + > > > > arch/arm/include/asm/arch-tegra124/pinmux.h | 327 + > > > > arch/arm/include/asm/arch-tegra20/pinmux.h| 291 + > > > > arch/arm/include/asm/arch-tegra210/pinmux.h | 394 ++ > > > > arch/arm/include/asm/arch-tegra30/pinmux.h| 381 ++ > > > > arch/arm/mach-tegra/Kconfig | 16 +- > > > > arch/arm/mach-tegra/Makefile |1 - > > > > arch/arm/mach-tegra/board.c |6 +- > > > > arch/arm/mach-tegra/board2.c |2 +- > > > > arch/arm/mach-tegra/tegra114/Makefile |2 +- > > > > arch/arm/mach-tegra/tegra124/Makefile |2 - > > > > arch/arm/mach-tegra/tegra20/Makefile |2 +- > > > > arch/arm/mach-tegra/tegra210/Makefile |1 - > > > > arch/arm/mach-tegra/tegra30/Makefile |2 +- > > > > board/asus/grouper/grouper.c | 18 - > > > > board/asus/grouper/pinmux-config-grouper.h| 362 -- > > > > .../pinmux-config-transformer.h | 365 -- > > > > board/asus/transformer-t30/transformer-t30.c | 23 - > > > > board/compal/paz00/Makefile |8 - > > > > board/compal/paz00/paz00.c| 53 - > > > > board/htc/endeavoru/endeavoru.c | 14 - > > > > board/htc/endeavoru/pinmux-config-endeavoru.h | 362 -- > > > > board/lg/x3-t30/Kconfig | 12 - > > > > board/lg/x3-t30/configs/p880.config |1 - > > > > board/lg/x3-t30/configs/p895.config |1 - > > > > board/lg/x3-t30/pinmux-config-x3.h| 449 --- > > > > board/lg/x3-t30/x3-t30.c | 23 - > > > > configs/paz00_defconfig |3 + > > > > drivers/pinctrl/Kconfig |1 + > > > > drivers/pinctrl/Makefile |1 + > > > > drivers/pinctrl/tegra/Kconfig | 18 + > > > > drivers/pinctrl/tegra/Makefile| 16 + > > > > .../pinctrl/tegra/funcmux-tegra114.c |0 > > > > .../pinctrl/tegra/funcmux-tegra124.c |0 > > > > .../pinctrl/tegra/funcmux-tegra20.c |0 > > > > .../pinctrl/tegra/funcmux-tegra210.c |0 > > > > .../pinctrl/tegra/funcmux-tegra30.c |0 > > > > drivers/pinctrl/tegra/pinctrl-tegra.c | 248 > > > > drivers/pinctrl/tegra/pinctrl-tegra20.c | 177 +++ > > > >
Re: [PATCH v3 21/22] bootm: Create a function to run through the booti states
On Fri, Dec 15, 2023 at 08:14:25PM -0700, Simon Glass wrote: > In a few places, the booti command is used to handle a boot. We want > these to be done without needing CONFIG_CMDLINE, so add a new > booti_run() function to handle this. > > So far this is not used. > > Signed-off-by: Simon Glass Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH v3 19/22] bootm: Create a function to run through the bootz states
On Fri, Dec 15, 2023 at 08:14:23PM -0700, Simon Glass wrote: > In a few places, the bootz command is used to handle a boot. We want > these to be done without needing CONFIG_CMDLINE, so add a new > bootz_run() function to handle this. > > Signed-off-by: Simon Glass Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH v3 17/22] bootm: Create a function to run through the bootm states
On Fri, Dec 15, 2023 at 08:14:21PM -0700, Simon Glass wrote: > In quite a few places, the bootm command is used to handle a boot. We > want these to be done without needing CONFIG_CMDLINE, so add a new > bootm_run() function to handle this. > > Signed-off-by: Simon Glass Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH v3 22/22] bootm: Create a new boot_run() function to handle booting
On Fri, Dec 15, 2023 at 08:14:26PM -0700, Simon Glass wrote: > Create a common function used by the three existing bootz/i/m_run() > functions, to reduce duplicated code. > > Signed-off-by: Simon Glass > Suggested-by: Tom Rini > --- > > Changes in v3: > - Add new boot_run() function > > boot/bootm.c| 40 ++-- > include/bootm.h | 18 ++ > 2 files changed, 32 insertions(+), 26 deletions(-) > > diff --git a/boot/bootm.c b/boot/bootm.c > index 53236136f489..6a4cebcf7a08 100644 > --- a/boot/bootm.c > +++ b/boot/bootm.c > @@ -1123,47 +1123,35 @@ err: > return ret; > } > > -int bootm_run(struct bootm_info *bmi) > +int boot_run(struct bootm_info *bmi, const char *cmd, int extra_states) > { > int states; > > - bmi->cmd_name = "bootm"; > - states = BOOTM_STATE_START | BOOTM_STATE_FINDOS | BOOTM_STATE_PRE_LOAD | > - BOOTM_STATE_FINDOTHER | BOOTM_STATE_LOADOS | > - BOOTM_STATE_OS_PREP | BOOTM_STATE_OS_FAKE_GO | > - BOOTM_STATE_OS_GO | BOOTM_STATE_MEASURE; > + bmi->cmd_name = cmd; > + states = BOOTM_STATE_MEASURE | BOOTM_STATE_OS_PREP | > + BOOTM_STATE_OS_FAKE_GO | BOOTM_STATE_OS_GO; > if (IS_ENABLED(CONFIG_SYS_BOOT_RAMDISK_HIGH)) > states |= BOOTM_STATE_RAMDISK; > - if (IS_ENABLED(CONFIG_PPC) || IS_ENABLED(CONFIG_MIPS)) > - states |= BOOTM_STATE_OS_CMDLINE; > + states |= extra_states; > > return bootm_run_states(bmi, states); > } > > -int bootz_run(struct bootm_info *bmi) > +int bootm_run(struct bootm_info *bmi) > { > - int states; > - > - bmi->cmd_name = "bootz"; > - states = BOOTM_STATE_MEASURE | BOOTM_STATE_OS_PREP | > - BOOTM_STATE_OS_FAKE_GO | BOOTM_STATE_OS_GO; > - if (IS_ENABLED(CONFIG_SYS_BOOT_RAMDISK_HIGH)) > - states |= BOOTM_STATE_RAMDISK; > + return boot_run(bmi, "bootm", BOOTM_STATE_START | BOOTM_STATE_FINDOS | > + BOOTM_STATE_PRE_LOAD | BOOTM_STATE_FINDOTHER | > + BOOTM_STATE_LOADOS); > +} > > - return bootm_run_states(bmi, states); > +int bootz_run(struct bootm_info *bmi) > +{ > + return boot_run(bmi, "bootz", 0); > } > > int booti_run(struct bootm_info *bmi) > { > - int states; > - > - bmi->cmd_name = "booti"; > - states = BOOTM_STATE_MEASURE | BOOTM_STATE_OS_PREP | > - BOOTM_STATE_OS_FAKE_GO | BOOTM_STATE_OS_GO; > - if (IS_ENABLED(CONFIG_SYS_BOOT_RAMDISK_HIGH)) > - states |= BOOTM_STATE_RAMDISK; > - > - return bootm_run_states(bmi, states); > + return boot_run(bmi, "booti", 0); > } > > int bootm_boot_start(ulong addr, const char *cmdline) > diff --git a/include/bootm.h b/include/bootm.h > index eba35b33b4e5..9e0f8d60de0a 100644 > --- a/include/bootm.h > +++ b/include/bootm.h > @@ -150,6 +150,24 @@ int bootm_measure(struct bootm_headers *images); > */ > int bootm_run_states(struct bootm_info *bmi, int states); > > +/** > + * boot_run() - Run the entire bootm/booti/bootz process > + * > + * This runs through the boot process from start to finish, with a base set > of > + * states, along with the extra ones supplied. > + * > + * This uses bootm_run_states(). > + * > + * Note that it is normally easier to use bootm_run(), etc. since they handle > + * the extra states correctly. > + * > + * @bmi: bootm information > + * @cmd: command being run, NULL if none > + * @extra_states: Mask of extra states to use for the boot > + * Return: 0 if ok, something else on error > + */ > +int boot_run(struct bootm_info *bmi, const char *cmd, int extra_states); > + > /** > * bootm_run() - Run the entire bootm process > * Overall good, thanks. My question is, should we mark the others as static, or is this new helper static and not to be called externally? -- Tom signature.asc Description: PGP signature
Re: [PATCH v3 10/22] bootm: Add more fields to bootm_info
On Fri, Dec 15, 2023 at 08:14:14PM -0700, Simon Glass wrote: > Add fields for the three bootm parameters and other things needed for > booting. Also add a helper to set up the struct correctly. > > Signed-off-by: Simon Glass Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH v6 0/5] Populate kaslr seed with RNG
On Thu, Nov 30, 2023 at 04:54:39PM -0800, Sean Edmond wrote: > This patch series creates a common API (fdt_fixup_kaslr_seed()) for > populating the kaslr seed in the DTB. Existing users (kaslrseed, > and ARMv8 sec firmware) have been updated to use this common API. > > New functionality has been introduced to populate the kaslr using > the RNG. This can be enabled with CONFIG_RNG_TPM_SEED. This series introduces a fail to build on ls1012afrdm_tfa and others. Also one of the patches has a double Signed-off-by, please fix that too. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 1/1] clk: clk-gpio: add actual gated clock
On 12/16/23 11:52, Marek Vasut wrote: On 12/16/23 16:37, Sean Anderson wrote: On 12/16/23 03:48, Svyatoslav Ryhel wrote: Existing gpio-gate-clock driver acts like a simple GPIO switch without any effect on gated clock. Add actual clock actions into enable/disable ops and implement get_rate op by passing gated clock if it is enabled. Signed-off-by: Svyatoslav Ryhel --- drivers/clk/clk-gpio.c | 44 ++ 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/drivers/clk/clk-gpio.c b/drivers/clk/clk-gpio.c index 26d795b978..72d9747a47 100644 --- a/drivers/clk/clk-gpio.c +++ b/drivers/clk/clk-gpio.c @@ -3,19 +3,23 @@ * Copyright (C) 2023 Marek Vasut */ -#include -#include -#include +#include #include +#include +#include + +#include struct clk_gpio_priv { - struct gpio_desc enable; + struct gpio_desc enable; /* GPIO, controlling the gate */ + struct clk *clk; /* Gated clock */ }; static int clk_gpio_enable(struct clk *clk) { struct clk_gpio_priv *priv = dev_get_priv(clk->dev); + clk_enable(priv->clk); dm_gpio_set_value(>enable, 1); return 0; @@ -26,21 +30,45 @@ static int clk_gpio_disable(struct clk *clk) struct clk_gpio_priv *priv = dev_get_priv(clk->dev); dm_gpio_set_value(>enable, 0); + clk_disable(priv->clk); return 0; } +static ulong clk_gpio_get_rate(struct clk *clk) +{ + struct clk_gpio_priv *priv = dev_get_priv(clk->dev); + + return clk_get_rate(priv->clk); +} + const struct clk_ops clk_gpio_ops = { .enable = clk_gpio_enable, .disable = clk_gpio_disable, + .get_rate = clk_gpio_get_rate, }; -static int clk_gpio_probe(struct udevice *dev) +static int clk_gpio_of_to_plat(struct udevice *dev) { struct clk_gpio_priv *priv = dev_get_priv(dev); + int ret; - return gpio_request_by_name(dev, "enable-gpios", 0, - >enable, GPIOD_IS_OUT); + priv->clk = devm_clk_get(dev, NULL); + if (IS_ERR(priv->clk)) { + log_debug("%s: Could not get gated clock: %ld\n", + __func__, PTR_ERR(priv->clk)); + return PTR_ERR(priv->clk); + } + + ret = gpio_request_by_name(dev, "enable-gpios", 0, + >enable, GPIOD_IS_OUT); + if (ret) { + log_debug("%s: Could not decode enable-gpios (%d)\n", + __func__, ret); + return ret; + } + + return 0; All this forwarding of clock enable/disable/get_rate for parent clock should already be done in clock core. If it is not, then it should be fixed in clock core. Only CCF forwards things. It's up to the driver to do it in the non-CCF case. --Sean
Re: [PATCH v2 1/1] clk: clk-gpio: add actual gated clock
сб, 16 груд. 2023 р. о 18:52 Marek Vasut пише: > > On 12/16/23 16:37, Sean Anderson wrote: > > On 12/16/23 03:48, Svyatoslav Ryhel wrote: > >> Existing gpio-gate-clock driver acts like a simple GPIO switch without > >> any > >> effect on gated clock. Add actual clock actions into enable/disable > >> ops and > >> implement get_rate op by passing gated clock if it is enabled. > >> > >> Signed-off-by: Svyatoslav Ryhel > >> --- > >> drivers/clk/clk-gpio.c | 44 ++ > >> 1 file changed, 36 insertions(+), 8 deletions(-) > >> > >> diff --git a/drivers/clk/clk-gpio.c b/drivers/clk/clk-gpio.c > >> index 26d795b978..72d9747a47 100644 > >> --- a/drivers/clk/clk-gpio.c > >> +++ b/drivers/clk/clk-gpio.c > >> @@ -3,19 +3,23 @@ > >>* Copyright (C) 2023 Marek Vasut > >>*/ > >> -#include > >> -#include > >> -#include > >> +#include > >> #include > >> +#include > >> +#include > >> + > >> +#include > >> struct clk_gpio_priv { > >> -struct gpio_descenable; > >> +struct gpio_descenable;/* GPIO, controlling the gate */ > >> +struct clk*clk;/* Gated clock */ > >> }; > >> static int clk_gpio_enable(struct clk *clk) > >> { > >> struct clk_gpio_priv *priv = dev_get_priv(clk->dev); > >> +clk_enable(priv->clk); > >> dm_gpio_set_value(>enable, 1); > >> return 0; > >> @@ -26,21 +30,45 @@ static int clk_gpio_disable(struct clk *clk) > >> struct clk_gpio_priv *priv = dev_get_priv(clk->dev); > >> dm_gpio_set_value(>enable, 0); > >> +clk_disable(priv->clk); > >> return 0; > >> } > >> +static ulong clk_gpio_get_rate(struct clk *clk) > >> +{ > >> +struct clk_gpio_priv *priv = dev_get_priv(clk->dev); > >> + > >> +return clk_get_rate(priv->clk); > >> +} > >> + > >> const struct clk_ops clk_gpio_ops = { > >> .enable= clk_gpio_enable, > >> .disable= clk_gpio_disable, > >> +.get_rate= clk_gpio_get_rate, > >> }; > >> -static int clk_gpio_probe(struct udevice *dev) > >> +static int clk_gpio_of_to_plat(struct udevice *dev) > >> { > >> struct clk_gpio_priv *priv = dev_get_priv(dev); > >> +int ret; > >> -return gpio_request_by_name(dev, "enable-gpios", 0, > >> ->enable, GPIOD_IS_OUT); > >> +priv->clk = devm_clk_get(dev, NULL); > >> +if (IS_ERR(priv->clk)) { > >> +log_debug("%s: Could not get gated clock: %ld\n", > >> + __func__, PTR_ERR(priv->clk)); > >> +return PTR_ERR(priv->clk); > >> +} > >> + > >> +ret = gpio_request_by_name(dev, "enable-gpios", 0, > >> + >enable, GPIOD_IS_OUT); > >> +if (ret) { > >> +log_debug("%s: Could not decode enable-gpios (%d)\n", > >> + __func__, ret); > >> +return ret; > >> +} > >> + > >> +return 0; > > All this forwarding of clock enable/disable/get_rate for parent clock > should already be done in clock core. If it is not, then it should be > fixed in clock core. It is not and this driver, as is, did not work on tegra since you messed up headers.
Re: [PATCH v2 1/1] clk: clk-gpio: add actual gated clock
On 12/16/23 16:37, Sean Anderson wrote: On 12/16/23 03:48, Svyatoslav Ryhel wrote: Existing gpio-gate-clock driver acts like a simple GPIO switch without any effect on gated clock. Add actual clock actions into enable/disable ops and implement get_rate op by passing gated clock if it is enabled. Signed-off-by: Svyatoslav Ryhel --- drivers/clk/clk-gpio.c | 44 ++ 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/drivers/clk/clk-gpio.c b/drivers/clk/clk-gpio.c index 26d795b978..72d9747a47 100644 --- a/drivers/clk/clk-gpio.c +++ b/drivers/clk/clk-gpio.c @@ -3,19 +3,23 @@ * Copyright (C) 2023 Marek Vasut */ -#include -#include -#include +#include #include +#include +#include + +#include struct clk_gpio_priv { - struct gpio_desc enable; + struct gpio_desc enable; /* GPIO, controlling the gate */ + struct clk *clk; /* Gated clock */ }; static int clk_gpio_enable(struct clk *clk) { struct clk_gpio_priv *priv = dev_get_priv(clk->dev); + clk_enable(priv->clk); dm_gpio_set_value(>enable, 1); return 0; @@ -26,21 +30,45 @@ static int clk_gpio_disable(struct clk *clk) struct clk_gpio_priv *priv = dev_get_priv(clk->dev); dm_gpio_set_value(>enable, 0); + clk_disable(priv->clk); return 0; } +static ulong clk_gpio_get_rate(struct clk *clk) +{ + struct clk_gpio_priv *priv = dev_get_priv(clk->dev); + + return clk_get_rate(priv->clk); +} + const struct clk_ops clk_gpio_ops = { .enable = clk_gpio_enable, .disable = clk_gpio_disable, + .get_rate = clk_gpio_get_rate, }; -static int clk_gpio_probe(struct udevice *dev) +static int clk_gpio_of_to_plat(struct udevice *dev) { struct clk_gpio_priv *priv = dev_get_priv(dev); + int ret; - return gpio_request_by_name(dev, "enable-gpios", 0, - >enable, GPIOD_IS_OUT); + priv->clk = devm_clk_get(dev, NULL); + if (IS_ERR(priv->clk)) { + log_debug("%s: Could not get gated clock: %ld\n", + __func__, PTR_ERR(priv->clk)); + return PTR_ERR(priv->clk); + } + + ret = gpio_request_by_name(dev, "enable-gpios", 0, + >enable, GPIOD_IS_OUT); + if (ret) { + log_debug("%s: Could not decode enable-gpios (%d)\n", + __func__, ret); + return ret; + } + + return 0; All this forwarding of clock enable/disable/get_rate for parent clock should already be done in clock core. If it is not, then it should be fixed in clock core.
Re: [GIT PULL] clock changes for u-boot/next
On Fri, Dec 15, 2023 at 03:52:26PM -0500, Sean Anderson wrote: > The following changes since commit fa3f19aa56c519d6345cc774187b7a8fdc053d71: > > Merge tag 'xilinx-for-v2024.04-rc1' of > https://source.denx.de/u-boot/custodians/u-boot-microblaze into next > (2023-12-14 13:27:11 -0500) > > are available in the Git repository at: > > https://source.denx.de/u-boot/custodians/u-boot-clk.git > tags/clk-2024.01-next > > for you to fetch changes up to 652d8d4561a34fc71d9bcb6ad2b0035d2c4189c1: > > clk: nuvoton: add read only feature for clk driver (2023-12-15 13:05:55 > -0500) > Applied to u-boot/next, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v3 08/16] fastboot: Remove dependencies on CMDLINE
On Sat, Dec 16, 2023 at 06:16:38AM -0700, Simon Glass wrote: > It is possible to boot a kernel without CMDLINE being enabled. Update > the implementation to handle this, and drop the condition from the > FASTBOOT config. > > Reviewed-by: Mattijs Korpershoek > > Signed-off-by: Simon Glass Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [GIT PULL] clock changes for u-boot/master
On Fri, Dec 15, 2023 at 04:06:23PM -0500, Sean Anderson wrote: > The following changes since commit 3ac22891cfc0dc6d8eec25d2b0fbdd2eb8f3d3ed: > > Merge tag 'u-boot-imx-20231214' of > https://gitlab.denx.de/u-boot/custodians/u-boot-imx (2023-12-15 08:22:31 > -0500) > > are available in the Git repository at: > > https://source.denx.de/u-boot/custodians/u-boot-clk.git tags/clk-2024.01-rc5 > > for you to fetch changes up to 97d65b32d76cb3b8297cd8aa2c1f0caab5ab6c57: > > test: dm: clk_ccf: fix building error (2023-12-15 15:30:12 -0500) > Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
[PATCH 1/1] test: CONFIG_UT_BOOTSTD must depend on CONFIG_BOOTSTD
Building sandbox_defconfig with CONFIG_BOOTSTD=n CONFIG_UT_BOOTSTD=y leads to an error /usr/bin/ld: test/cmd_ut.o:(.data.rel.cmd_ut_sub+0xc0): undefined reference to `do_ut_bootstd' Add the missing dependency. Signed-off-by: Heinrich Schuchardt --- test/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Kconfig b/test/Kconfig index e842c01308..e2ec0994a2 100644 --- a/test/Kconfig +++ b/test/Kconfig @@ -67,7 +67,7 @@ endif config UT_BOOTSTD bool "Unit tests for standard boot" - depends on UNIT_TEST && SANDBOX + depends on UNIT_TEST && BOOTSTD && SANDBOX default y config UT_COMPRESSION -- 2.40.1
[PATCH 1/1] boot: CONFIG_CEDIT must depend on CONFIG_EXPO
Building sandbox_defconfig with CONFIG_CMD_CEDIT=y CONFIG_EXPO=n fails with cmd/cedit.c:258:(.text.do_cedit_run+0x4c): undefined reference to `expo_apply_theme Fix the dependencies. Fixes: a0874dc4ac71 ("expo: Add a configuration editor") Signed-off-by: Heinrich Schuchardt --- boot/Kconfig | 2 +- boot/Makefile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/boot/Kconfig b/boot/Kconfig index b438002059..98273b2d3b 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -1678,7 +1678,7 @@ menu "Configuration editor" config CEDIT bool "Configuration editor" - depends on BOOTSTD + depends on EXPO help Provides a way to deal with board configuration and present it to the user for adjustment. diff --git a/boot/Makefile b/boot/Makefile index de0eafed14..40931218ee 100644 --- a/boot/Makefile +++ b/boot/Makefile @@ -33,11 +33,11 @@ obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_EFILOADER) += bootmeth_efi.o obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_CROS) += bootm.o bootm_os.o bootmeth_cros.o obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SANDBOX) += bootmeth_sandbox.o obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SCRIPT) += bootmeth_script.o +obj-$(CONFIG_$(SPL_TPL_)CEDIT) += cedit.o ifdef CONFIG_$(SPL_TPL_)BOOTSTD_FULL obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += bootmeth_efi_mgr.o obj-$(CONFIG_$(SPL_TPL_)EXPO) += bootflow_menu.o obj-$(CONFIG_$(SPL_TPL_)BOOTSTD) += bootflow_menu.o -obj-$(CONFIG_$(SPL_TPL_)CEDIT) += cedit.o endif obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += fdt_support.o -- 2.40.1
Re: [PATCH v2 1/1] clk: clk-gpio: add actual gated clock
On 12/16/23 03:48, Svyatoslav Ryhel wrote: Existing gpio-gate-clock driver acts like a simple GPIO switch without any effect on gated clock. Add actual clock actions into enable/disable ops and implement get_rate op by passing gated clock if it is enabled. Signed-off-by: Svyatoslav Ryhel --- drivers/clk/clk-gpio.c | 44 ++ 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/drivers/clk/clk-gpio.c b/drivers/clk/clk-gpio.c index 26d795b978..72d9747a47 100644 --- a/drivers/clk/clk-gpio.c +++ b/drivers/clk/clk-gpio.c @@ -3,19 +3,23 @@ * Copyright (C) 2023 Marek Vasut */ -#include -#include -#include +#include #include +#include +#include + +#include struct clk_gpio_priv { - struct gpio_descenable; + struct gpio_descenable; /* GPIO, controlling the gate */ + struct clk *clk; /* Gated clock */ }; static int clk_gpio_enable(struct clk *clk) { struct clk_gpio_priv *priv = dev_get_priv(clk->dev); + clk_enable(priv->clk); dm_gpio_set_value(>enable, 1); return 0; @@ -26,21 +30,45 @@ static int clk_gpio_disable(struct clk *clk) struct clk_gpio_priv *priv = dev_get_priv(clk->dev); dm_gpio_set_value(>enable, 0); + clk_disable(priv->clk); return 0; } +static ulong clk_gpio_get_rate(struct clk *clk) +{ + struct clk_gpio_priv *priv = dev_get_priv(clk->dev); + + return clk_get_rate(priv->clk); +} + const struct clk_ops clk_gpio_ops = { .enable = clk_gpio_enable, .disable= clk_gpio_disable, + .get_rate = clk_gpio_get_rate, }; -static int clk_gpio_probe(struct udevice *dev) +static int clk_gpio_of_to_plat(struct udevice *dev) { struct clk_gpio_priv *priv = dev_get_priv(dev); + int ret; - return gpio_request_by_name(dev, "enable-gpios", 0, - >enable, GPIOD_IS_OUT); + priv->clk = devm_clk_get(dev, NULL); + if (IS_ERR(priv->clk)) { + log_debug("%s: Could not get gated clock: %ld\n", + __func__, PTR_ERR(priv->clk)); + return PTR_ERR(priv->clk); + } + + ret = gpio_request_by_name(dev, "enable-gpios", 0, + >enable, GPIOD_IS_OUT); + if (ret) { + log_debug("%s: Could not decode enable-gpios (%d)\n", + __func__, ret); + return ret; + } + + return 0; } /* @@ -59,7 +87,7 @@ U_BOOT_DRIVER(gpio_gate_clock) = { .name = "gpio_clock", .id = UCLASS_CLK, .of_match = clk_gpio_match, - .probe = clk_gpio_probe, + .of_to_plat = clk_gpio_of_to_plat, .priv_auto = sizeof(struct clk_gpio_priv), .ops= _gpio_ops, .flags = DM_FLAG_PRE_RELOC, +CC Marek
[PATCH 1/1] test: build test/boot for CONFIG_UT_BOOTSTD=n
Building sandbox_defconfig with CONFIG_UT_BOOTSTD=n CONFIG_MEASURMENT=y results in an error: /usr/bin/ld: test/cmd_ut.o:(.data.rel.cmd_ut_sub+0x408): undefined reference to `do_ut_measurement' Fixes: 5999ea20fa42 ("test: Add sandbox TPM boot measurement") Signed-off-by: Heinrich Schuchardt --- test/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Makefile b/test/Makefile index 6b8a1506f5..feba097543 100644 --- a/test/Makefile +++ b/test/Makefile @@ -23,7 +23,7 @@ obj-$(CONFIG_UT_TIME) += time_ut.o obj-y += ut.o ifeq ($(CONFIG_SPL_BUILD),) -obj-$(CONFIG_$(SPL_)UT_BOOTSTD) += boot/ +obj-y += boot/ obj-$(CONFIG_UNIT_TEST) += common/ obj-y += log/ obj-$(CONFIG_$(SPL_)UT_UNICODE) += unicode_ut.o -- 2.40.1
Booting fails on the sandbox
Hello Simon, I have built sandbox_defconfig with PREBOOT='host bind 0 ../sct.img' On the image I have an ESP with file EFI/BOOT/BOOTX64.EFI. I would expect the sandbox to boot from the ESP. But when starting the sandbox is does not boot. Instead it shows a menu with some Fedora related entry: Fedora-Workstation-armhfp-31-1.9 Boot Options. 1: Fedora-Workstation-armhfp-31-1.9 (5.3.7-301.fc31.armv7hl) This makes absolutely no sense on an x86_64 system. Could you, please, fix sandbox_defconfig to boot according to distroboot. Best regards Heinrich
[PATCH v3 08/16] fastboot: Remove dependencies on CMDLINE
It is possible to boot a kernel without CMDLINE being enabled. Update the implementation to handle this, and drop the condition from the FASTBOOT config. Reviewed-by: Mattijs Korpershoek Signed-off-by: Simon Glass --- Changes in v3: - Expand the fastboot help Changes in v2: - Avoid changing the logic when a command is not set drivers/fastboot/Kconfig | 5 - drivers/fastboot/fb_common.c | 30 ++ 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig index 11fc0fe1c800..449244ab6a8e 100644 --- a/drivers/fastboot/Kconfig +++ b/drivers/fastboot/Kconfig @@ -1,5 +1,4 @@ menu "Fastboot support" - depends on CMDLINE config FASTBOOT bool @@ -13,6 +12,10 @@ config FASTBOOT More information about the protocol and usecases: https://android.googlesource.com/platform/system/core/+/refs/heads/master/fastboot/ + Note that enabling CMDLINE is recommended since fastboot allows U-Boot + commands to be executed on request. The CMDLINE option is required + for anything other than simply booting the OS. + config USB_FUNCTION_FASTBOOT bool "Enable USB fastboot gadget" depends on USB_GADGET diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c index 07f5946d9ed1..595954542a6e 100644 --- a/drivers/fastboot/fb_common.c +++ b/drivers/fastboot/fb_common.c @@ -11,6 +11,7 @@ */ #include +#include #include #include #include @@ -142,22 +143,19 @@ void (*fastboot_get_progress_callback(void))(const char *) */ void fastboot_boot(void) { - char *s; - - s = env_get("fastboot_bootcmd"); - if (s) { - run_command(s, CMD_FLAG_ENV); - } else if (IS_ENABLED(CONFIG_CMD_BOOTM)) { - static char boot_addr_start[20]; - static char *const bootm_args[] = { - "bootm", boot_addr_start, NULL - }; - - snprintf(boot_addr_start, sizeof(boot_addr_start) - 1, -"%lx", fastboot_buf_addr); - printf("Booting kernel at %s...\n\n\n", boot_addr_start); - - do_bootm(NULL, 0, 2, bootm_args); + char *s = NULL; + + if (IS_ENABLED(CONFIG_CMDLINE)) { + s = env_get("fastboot_bootcmd"); + if (s) + run_command(s, CMD_FLAG_ENV); + } + + if (!s && IS_ENABLED(CONFIG_BOOTM)) { + int ret; + + printf("Booting kernel at %lx...\n\n\n", fastboot_buf_addr); + ret = bootm_boot_start(fastboot_buf_addr, NULL); /* * This only happens if image is somehow faulty so we start -- 2.43.0.472.g3155946c3a-goog
Re: [PATCH 1/2] ARM: imx: Force DRAM regulators into FPWM mode on DH i.MX8MP DHCOM SoM
On Sat, Dec 16, 2023 at 2:43 AM Marek Vasut wrote: > > In case the Buck5 and Buck6 regulators which supply DRAM Vdd1 and Vdd2/Vddq > respectively operate in automatic PWM/PFM mode, the DRAM EDAC detects more > correctable errors than if the regulators operate in forced PWM only mode. > Force DRAM regulators to forced PWM mode only to stop tempting the DRAM. > > Signed-off-by: Marek Vasut Applied both to u-boot-imx master, thanks.
Re: [PATCH 1/2] configs: imx8m{m, n, p}_venice_defconfig: add arch_misc_init
On Thu, Dec 14, 2023 at 1:25 PM Tim Harvey wrote: > > Enable call to arch_misc_init in order to probe the CAAM driver. > > Signed-off-by: Tim Harvey Applied only this one to u-boot-imx next, thanks.
Re: [PATCH] arm: dts: imx8mp-venice-gw72xx: add TPM device
On Mon, Nov 27, 2023 at 4:37 PM Tim Harvey wrote: > > Add the TPM device found on the GW72xx revision F PCB. > > This hangs off of SPI2, uses gpio1_10 as a CS and gpio1_11 as RST#. > > Signed-off-by: Tim Harvey Applied to u-boot-imx next, thanks.
Re: [PATCH] arm: dts: imx8mm-venice-gw72xx: add TPM device
On Mon, Nov 27, 2023 at 4:37 PM Tim Harvey wrote: > > Add the TPM device found on the GW72xx revision F PCB. > > This hangs off of SPI2, uses gpio1_10 as a CS and gpio1_11 as RST#. > > Signed-off-by: Tim Harvey Applied to u-boot-imx next, thanks.
Re: [PATCH] ARM: dts: imx7d-pico: Fix alias node indent
Hi Marek, On Sat, Dec 16, 2023 at 2:36 AM Marek Vasut wrote: > > Switch from indent with spaces to indent with tabs. No functional change. > > Fixes: f8548ce0e093 ("imx7d-pico: Fix the name of the u-boot.dtsi file") > Signed-off-by: Marek Vasut Thanks for the patch, but this is already fixed in the U-Boot next branch: https://source.denx.de/u-boot/u-boot/-/commit/4b5c1175a2ba8493ecb223d99b31d5f55eb4c3f4
[PATCH v2 1/1] clk: clk-gpio: add actual gated clock
Existing gpio-gate-clock driver acts like a simple GPIO switch without any effect on gated clock. Add actual clock actions into enable/disable ops and implement get_rate op by passing gated clock if it is enabled. Signed-off-by: Svyatoslav Ryhel --- drivers/clk/clk-gpio.c | 44 ++ 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/drivers/clk/clk-gpio.c b/drivers/clk/clk-gpio.c index 26d795b978..72d9747a47 100644 --- a/drivers/clk/clk-gpio.c +++ b/drivers/clk/clk-gpio.c @@ -3,19 +3,23 @@ * Copyright (C) 2023 Marek Vasut */ -#include -#include -#include +#include #include +#include +#include + +#include struct clk_gpio_priv { - struct gpio_descenable; + struct gpio_descenable; /* GPIO, controlling the gate */ + struct clk *clk; /* Gated clock */ }; static int clk_gpio_enable(struct clk *clk) { struct clk_gpio_priv *priv = dev_get_priv(clk->dev); + clk_enable(priv->clk); dm_gpio_set_value(>enable, 1); return 0; @@ -26,21 +30,45 @@ static int clk_gpio_disable(struct clk *clk) struct clk_gpio_priv *priv = dev_get_priv(clk->dev); dm_gpio_set_value(>enable, 0); + clk_disable(priv->clk); return 0; } +static ulong clk_gpio_get_rate(struct clk *clk) +{ + struct clk_gpio_priv *priv = dev_get_priv(clk->dev); + + return clk_get_rate(priv->clk); +} + const struct clk_ops clk_gpio_ops = { .enable = clk_gpio_enable, .disable= clk_gpio_disable, + .get_rate = clk_gpio_get_rate, }; -static int clk_gpio_probe(struct udevice *dev) +static int clk_gpio_of_to_plat(struct udevice *dev) { struct clk_gpio_priv *priv = dev_get_priv(dev); + int ret; - return gpio_request_by_name(dev, "enable-gpios", 0, - >enable, GPIOD_IS_OUT); + priv->clk = devm_clk_get(dev, NULL); + if (IS_ERR(priv->clk)) { + log_debug("%s: Could not get gated clock: %ld\n", + __func__, PTR_ERR(priv->clk)); + return PTR_ERR(priv->clk); + } + + ret = gpio_request_by_name(dev, "enable-gpios", 0, + >enable, GPIOD_IS_OUT); + if (ret) { + log_debug("%s: Could not decode enable-gpios (%d)\n", + __func__, ret); + return ret; + } + + return 0; } /* @@ -59,7 +87,7 @@ U_BOOT_DRIVER(gpio_gate_clock) = { .name = "gpio_clock", .id = UCLASS_CLK, .of_match = clk_gpio_match, - .probe = clk_gpio_probe, + .of_to_plat = clk_gpio_of_to_plat, .priv_auto = sizeof(struct clk_gpio_priv), .ops= _gpio_ops, .flags = DM_FLAG_PRE_RELOC, -- 2.40.1
[PATCH v2 0/1] Fix clk-gpio driver
Existing gpio-gate-clock driver acts like a simple GPIO switch without any effect on gated clock. Add actual clock actions into enable/disable ops and implement get_rate op by passing gated clock if it is enabled. Testing current driver implementation shows that it is not fully capable and lacks gated clock manipulations. --- Changes from v1 - added comments for priv struct members - return gatet clock rate unconditionally - switch log_err > log_debug on of_to_plat --- Svyatoslav Ryhel (1): clk: clk-gpio: add actual gated clock drivers/clk/clk-gpio.c | 44 ++ 1 file changed, 36 insertions(+), 8 deletions(-) -- 2.40.1
[PATCH v3 4/4] acpi: support 64bit in acpi_find_table for DSDT and FACS
Use X_DSDT and X_FIRMWARE_CTRL if available. Signed-off-by: Heinrich Schuchardt --- lib/acpi/acpi.c | 29 - 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/lib/acpi/acpi.c b/lib/acpi/acpi.c index f21e509461..f80b2176e1 100644 --- a/lib/acpi/acpi.c +++ b/lib/acpi/acpi.c @@ -45,11 +45,30 @@ struct acpi_table_header *acpi_find_table(const char *sig) if (!memcmp(hdr->signature, "FACP", ACPI_NAME_LEN)) { struct acpi_fadt *fadt = (struct acpi_fadt *)hdr; - if (!memcmp(sig, "DSDT", ACPI_NAME_LEN) && fadt->dsdt) - return map_sysmem(fadt->dsdt, 0); - if (!memcmp(sig, "FACS", ACPI_NAME_LEN) && - fadt->firmware_ctrl) - return map_sysmem(fadt->firmware_ctrl, 0); + if (!memcmp(sig, "DSDT", ACPI_NAME_LEN)) { + void *dsdt; + + if (fadt->header.revision >= 3 && fadt->x_dsdt) + dsdt = map_sysmem(fadt->x_dsdt, 0); + else if (fadt->dsdt) + dsdt = map_sysmem(fadt->dsdt, 0); + else + dsdt = NULL; + return dsdt; + } + + if (!memcmp(sig, "FACS", ACPI_NAME_LEN)) { + void *facs; + + if (fadt->header.revision >= 3 && + fadt->x_firmware_ctrl) + facs = map_sysmem(fadt->x_firmware_ctrl, 0); + else if (fadt->firmware_ctrl) + facs = map_sysmem(fadt->firmware_ctrl, 0); + else + facs = NULL; + return facs; + } } } -- 2.40.1
[PATCH v3 3/4] cmd: acpi: check HW reduced flag in acpi list
On non x86 platforms the hardware reduce flag must be set in the FADT table. Write an error message if the flag is missing. Signed-off-by: Heinrich Schuchardt --- cmd/acpi.c | 4 1 file changed, 4 insertions(+) diff --git a/cmd/acpi.c b/cmd/acpi.c index 8d9eaf36c9..f6e02ea7a5 100644 --- a/cmd/acpi.c +++ b/cmd/acpi.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -57,6 +58,9 @@ static void list_fadt(struct acpi_fadt *fadt) dump_hdr(map_sysmem(fadt->x_dsdt, 0)); else if (fadt->dsdt) dump_hdr(map_sysmem(fadt->dsdt, 0)); + if (!IS_ENABLED(CONFIG_X86) && + !(fadt->flags & ACPI_FADT_HW_REDUCED_ACPI)) + log_err("FADT not ACPI hardware reduced compliant\n"); if (fadt->header.revision >= 3 && fadt->x_firmware_ctrl) dump_hdr(map_sysmem(fadt->x_firmware_ctrl, 0)); else if (fadt->firmware_ctrl) -- 2.40.1
[PATCH v3 2/4] cmd: acpi: fix listing DSDT and FACS
If field X_FIRMWARE_CTRL is filled, field FIRMWARE must be ignored. If field X_DSDT is filled, field DSDT must be ignored. Signed-off-by: Heinrich Schuchardt --- cmd/acpi.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cmd/acpi.c b/cmd/acpi.c index 0c14409242..8d9eaf36c9 100644 --- a/cmd/acpi.c +++ b/cmd/acpi.c @@ -53,9 +53,13 @@ static int dump_table_name(const char *sig) static void list_fadt(struct acpi_fadt *fadt) { - if (fadt->dsdt) + if (fadt->header.revision >= 3 && fadt->x_dsdt) + dump_hdr(map_sysmem(fadt->x_dsdt, 0)); + else if (fadt->dsdt) dump_hdr(map_sysmem(fadt->dsdt, 0)); - if (fadt->firmware_ctrl) + if (fadt->header.revision >= 3 && fadt->x_firmware_ctrl) + dump_hdr(map_sysmem(fadt->x_firmware_ctrl, 0)); + else if (fadt->firmware_ctrl) dump_hdr(map_sysmem(fadt->firmware_ctrl, 0)); } -- 2.40.1
[PATCH v3 1/4] acpi: use 64-bit addresses in FADT table
Fields X_FIRMWAE_CTRL and X_DSDT must be 64bit wide. Convert pointers to to uintptr_t to fill these. If field X_FIRMWARE_CTRL is filled, field FIRMWARE must be ignored. If field X_DSDT is filled, field DSDT must be ignored. We should not fill unused fields. See the field definitions in chapter "5.2.9 Fixed ACPI Description Table (FADT)" of the ACPI Specification 6.5. Signed-off-by: Heinrich Schuchardt --- arch/x86/cpu/baytrail/acpi.c | 9 +++-- arch/x86/cpu/quark/acpi.c| 9 +++-- arch/x86/cpu/tangier/acpi.c | 9 +++-- arch/x86/lib/acpi_table.c| 9 ++--- include/acpi/acpi_table.h| 6 ++ 5 files changed, 13 insertions(+), 29 deletions(-) diff --git a/arch/x86/cpu/baytrail/acpi.c b/arch/x86/cpu/baytrail/acpi.c index 4378846f8b..ccc4851b18 100644 --- a/arch/x86/cpu/baytrail/acpi.c +++ b/arch/x86/cpu/baytrail/acpi.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -31,8 +32,6 @@ static int baytrail_write_fadt(struct acpi_ctx *ctx, header->length = sizeof(struct acpi_fadt); header->revision = 4; - fadt->firmware_ctrl = (u32)ctx->facs; - fadt->dsdt = (u32)ctx->dsdt; fadt->preferred_pm_profile = ACPI_PM_MOBILE; fadt->sci_int = 9; fadt->smi_cmd = 0; @@ -79,10 +78,8 @@ static int baytrail_write_fadt(struct acpi_ctx *ctx, fadt->reset_reg.addrh = 0; fadt->reset_value = SYS_RST | RST_CPU | FULL_RST; - fadt->x_firmware_ctl_l = (u32)ctx->facs; - fadt->x_firmware_ctl_h = 0; - fadt->x_dsdt_l = (u32)ctx->dsdt; - fadt->x_dsdt_h = 0; + fadt->x_firmware_ctrl = map_to_sysmem(ctx->facs); + fadt->x_dsdt = map_to_sysmem(ctx->dsdt); fadt->x_pm1a_evt_blk.space_id = ACPI_ADDRESS_SPACE_IO; fadt->x_pm1a_evt_blk.bit_width = fadt->pm1_evt_len * 8; diff --git a/arch/x86/cpu/quark/acpi.c b/arch/x86/cpu/quark/acpi.c index 9a2d682451..0e18ceab68 100644 --- a/arch/x86/cpu/quark/acpi.c +++ b/arch/x86/cpu/quark/acpi.c @@ -4,6 +4,7 @@ */ #include +#include #include #include #include @@ -26,8 +27,6 @@ static int quark_write_fadt(struct acpi_ctx *ctx, header->length = sizeof(struct acpi_fadt); header->revision = 4; - fadt->firmware_ctrl = (u32)ctx->facs; - fadt->dsdt = (u32)ctx->dsdt; fadt->preferred_pm_profile = ACPI_PM_UNSPECIFIED; fadt->sci_int = 9; fadt->smi_cmd = 0; @@ -74,10 +73,8 @@ static int quark_write_fadt(struct acpi_ctx *ctx, fadt->reset_reg.addrh = 0; fadt->reset_value = SYS_RST | RST_CPU | FULL_RST; - fadt->x_firmware_ctl_l = (u32)ctx->facs; - fadt->x_firmware_ctl_h = 0; - fadt->x_dsdt_l = (u32)ctx->dsdt; - fadt->x_dsdt_h = 0; + fadt->x_firmware_ctrl = map_to_sysmem(ctx->facs); + fadt->x_dsdt = map_to_sysmem(ctx->dsdt); fadt->x_pm1a_evt_blk.space_id = ACPI_ADDRESS_SPACE_IO; fadt->x_pm1a_evt_blk.bit_width = fadt->pm1_evt_len * 8; diff --git a/arch/x86/cpu/tangier/acpi.c b/arch/x86/cpu/tangier/acpi.c index 1c667c7d56..1d37cc9e2b 100644 --- a/arch/x86/cpu/tangier/acpi.c +++ b/arch/x86/cpu/tangier/acpi.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -31,8 +32,6 @@ static int tangier_write_fadt(struct acpi_ctx *ctx, header->length = sizeof(struct acpi_fadt); header->revision = 6; - fadt->firmware_ctrl = (u32)ctx->facs; - fadt->dsdt = (u32)ctx->dsdt; fadt->preferred_pm_profile = ACPI_PM_UNSPECIFIED; fadt->iapc_boot_arch = ACPI_FADT_VGA_NOT_PRESENT | @@ -45,10 +44,8 @@ static int tangier_write_fadt(struct acpi_ctx *ctx, fadt->minor_revision = 2; - fadt->x_firmware_ctl_l = (u32)ctx->facs; - fadt->x_firmware_ctl_h = 0; - fadt->x_dsdt_l = (u32)ctx->dsdt; - fadt->x_dsdt_h = 0; + fadt->x_firmware_ctrl = map_to_sysmem(ctx->facs); + fadt->x_dsdt = map_to_sysmem(ctx->dsdt); header->checksum = table_compute_checksum(fadt, header->length); diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c index c5b33dc65d..ef738c02a6 100644 --- a/arch/x86/lib/acpi_table.c +++ b/arch/x86/lib/acpi_table.c @@ -572,13 +572,8 @@ void acpi_fadt_common(struct acpi_fadt *fadt, struct acpi_facs *facs, memcpy(header->aslc_id, ASLC_ID, 4); header->aslc_revision = 1; - fadt->firmware_ctrl = (unsigned long)facs; - fadt->dsdt = (unsigned long)dsdt; - - fadt->x_firmware_ctl_l = (unsigned long)facs; - fadt->x_firmware_ctl_h = 0; - fadt->x_dsdt_l = (unsigned long)dsdt; - fadt->x_dsdt_h = 0; + fadt->x_firmware_ctrl = map_to_sysmem(facs); + fadt->x_dsdt = map_to_sysmem(dsdt); fadt->preferred_pm_profile = ACPI_PM_MOBILE; diff --git a/include/acpi/acpi_table.h b/include/acpi/acpi_table.h index 20ac3b51ba..e67562ef65 100644 --- a/include/acpi/acpi_table.h +++ b/include/acpi/acpi_table.h @@
[PATCH v3 0/4] cmd: acpi: correct handling of DSDT and FACS
Fields X_FIRMWARE and X_DSDT in the FADT table must be 64bit. Fix the definition in our include. The 64bit fields X_FIRMWARE and X_DSDT take precedence over the respective 32bit fields. Consider this in the 'acpi list' and 'acpi dump' commands. The fields only exist for FADT table revision 3 and above. Don't fill unused fields FIRMWAE and DSDT. Write an error if the hardware reduce flag is not set for non-x86 systems. v3: fix subject lines of patches 1 and 4 use map_to_sysmem for pointer to address conversion v2: check FADT table revision correct dumping DSDT and FACS Heinrich Schuchardt (4): acpi: use 64-bit addresses in FADT table cmd: acpi: fix listing DSDT and FACS cmd: acpi: check HW reduced flag in acpi list acpi: support 64bit in acpi_find_table for DSDT and FACS arch/x86/cpu/baytrail/acpi.c | 9 +++-- arch/x86/cpu/quark/acpi.c| 9 +++-- arch/x86/cpu/tangier/acpi.c | 9 +++-- arch/x86/lib/acpi_table.c| 9 ++--- cmd/acpi.c | 12 ++-- include/acpi/acpi_table.h| 6 ++ lib/acpi/acpi.c | 29 - 7 files changed, 47 insertions(+), 36 deletions(-) -- 2.40.1