Re: [PATCH v2 0/2] Fix 'no USB device found' error.
On 06/07/2023 06:53, Tony Lindgren wrote: > * Tom Rini [230703 16:22]: >> On Mon, Jul 03, 2023 at 07:12:47PM +0300, Roger Quadros wrote: >>> Linux DT files are correct. USB0 is a dual-role port so it sets it to 'otg'. >>> u-boot doesn't support 'otg' so we need to override it to 'peripheral' in >>> -u-boot.dtsi >> >> Ah, thanks, that was the missing bit of background information. > > It would be best to parse dual-role feature in the driver to handle it as > peripheral only and keep the dts the same. +1 -- cheers, -roger
Re: [PATCH 2/2] board: rockchip: Add Hardkernel ODROID-M1
On 2023-07-06 01:27, Stefan Agner wrote: > On 2023-07-02 22:47, Jonas Karlman wrote: >> Hardkernel ODROID-M1 is a single board computer with a RK3568B2 SoC, >> a slightly modified version of the RK3568 SoC. >> >> Features tested on a ODROID-M1 8GB v1.0 2022-06-13: >> - SD-card boot >> - eMMC boot >> - SPI Flash boot >> - PCIe/NVMe/AHCI >> - SATA port >> - USB host >> >> Device tree is imported from linux v6.4. >> >> Signed-off-by: Jonas Karlman > > Thanks for you patch! I've compared it to my version, and did not notice > any downside/anything missing. If anything, your version is more feature > complete. I've also tested it on an ODORID-M1 with 8GB of memory, it > boots fine from SD card. > > Reviewed-by: Stefan Agner > Tested-by: Stefan Agner Thanks for review and testing! > > One thing I've noticed is that USB isn't working when I use the stock > SPL (2017.09) running from the SPI RAM and upstream U-Boot (by writing > u-boot.itb to a raw GPT partition named "uboot" to the SD-card). That is > the same in my patchset. If the upstream SPL is used, things work. It > seems that something is not (re)initialized in U-Boot. Not sure if we > typically rely on the state the SPL leaves the HW at, but it would be > nice if we are able to make that combination work. This allows to boot > an upstream U-Boot from an SD-card without having to reflash the onboard > SPI. Trying to use a combo of vendor u-boot SPL and mainline U-Boot proper is not something I have tested or something I would recommend anyone to use. To make use of all features one should press the "SPI recovery switch" during boot or erase/replace U-Boot in SPI flash. Does this USB issue only affect U-Boot proper or also leave USB unusable in linux? Booting OS from USB3 have been a little bit random when I have tested, some of my USB3 devices work if plugged in from cold start others needs to be removed and plugged in again for U-Boot to recognize them. USB2 ports have been much more stable during my testing. > > On a different note: Do you know if PCIe/NVMe support in SPL is > something which is in the cards for this board? As mentioned in the cover-letter this series has some dependencies for all features enabled in defconfig to work, most notably PCIe/NVMe. You can use my rk3568-2023.07-rc6 branch at [1] that have all dependencies included or test with an artifact from my github actions test build workflow at [2]. Or do you mean SPL to load FIT (U-Boot proper and TF-A) from NVMe? [1] https://github.com/Kwiboo/u-boot-rockchip/commits/rk3568-2023.07-rc6 [2] https://github.com/Kwiboo/u-boot-build/actions/runs/5448465108 Regards, Jonas > > -- > Stefan > >> --- >> arch/arm/dts/Makefile | 1 + >> arch/arm/dts/rk3568-odroid-m1-u-boot.dtsi | 46 ++ >> arch/arm/dts/rk3568-odroid-m1.dts | 744 ++ >> board/rockchip/evb_rk3568/MAINTAINERS | 7 + >> configs/odroid-m1-rk3568_defconfig| 103 +++ >> doc/board/rockchip/rockchip.rst | 1 + >> 6 files changed, 902 insertions(+) >> create mode 100644 arch/arm/dts/rk3568-odroid-m1-u-boot.dtsi >> create mode 100644 arch/arm/dts/rk3568-odroid-m1.dts >> create mode 100644 configs/odroid-m1-rk3568_defconfig >>[...]
Re: [PATCH v2 0/2] Fix 'no USB device found' error.
* Tom Rini [230703 16:22]: > On Mon, Jul 03, 2023 at 07:12:47PM +0300, Roger Quadros wrote: > > Linux DT files are correct. USB0 is a dual-role port so it sets it to 'otg'. > > u-boot doesn't support 'otg' so we need to override it to 'peripheral' in > > -u-boot.dtsi > > Ah, thanks, that was the missing bit of background information. It would be best to parse dual-role feature in the driver to handle it as peripheral only and keep the dts the same. Regards, Tony
RE: [PATCH 1/2] dm: adc: add iMX93 ADC support
> -Original Message-
> From: Luca Ellero
> Sent: 2023年7月5日 20:56
> To: u-boot@lists.denx.de; sba...@denx.de; feste...@gmail.com; dl-uboot-imx
> ; luca.ell...@brickedbrain.com; Ye Li ;
> Peng Fan ; Bough Chen
> Cc: Luca Ellero
> Subject: [PATCH 1/2] dm: adc: add iMX93 ADC support
>
> This commit adds driver for iMX93 ADC.
>
> The driver is implemented using driver model and provides ADC uclass's methods
> for ADC single channel operations:
> - adc_start_channel()
> - adc_channel_data()
> - adc_stop()
>
> ADC features:
> - channels: 4
> - resolution: 12-bit
>
> Signed-off-by: Luca Ellero
Reviewed-by: Haibo Chen
Best Regards
Haibo Chen
> ---
> drivers/adc/Kconfig | 8 ++
> drivers/adc/Makefile| 1 +
> drivers/adc/imx93-adc.c | 290
>
> 3 files changed, 299 insertions(+)
> create mode 100644 drivers/adc/imx93-adc.c
>
> diff --git a/drivers/adc/Kconfig b/drivers/adc/Kconfig index
> e719c38bb3..4336732dee 100644
> --- a/drivers/adc/Kconfig
> +++ b/drivers/adc/Kconfig
> @@ -63,3 +63,11 @@ config STM32_ADC
> - core driver to deal with common resources
> - child driver to deal with individual ADC resources (declare ADC
> device and associated channels, start/stop conversions)
> +
> +config ADC_IMX93
> + bool "Enable NXP IMX93 ADC driver"
> + help
> + This enables basic driver for NXP IMX93 ADC.
> + It provides:
> + - 4 analog input channels
> + - 12-bit resolution
> diff --git a/drivers/adc/Makefile b/drivers/adc/Makefile index
> c1387f3a34..5336c82097 100644
> --- a/drivers/adc/Makefile
> +++ b/drivers/adc/Makefile
> @@ -10,3 +10,4 @@ obj-$(CONFIG_ADC_SANDBOX) += sandbox.o
> obj-$(CONFIG_SARADC_ROCKCHIP) += rockchip-saradc.o
> obj-$(CONFIG_SARADC_MESON) += meson-saradc.o
> obj-$(CONFIG_STM32_ADC) += stm32-adc.o stm32-adc-core.o
> +obj-$(CONFIG_ADC_IMX93) += imx93-adc.o
> diff --git a/drivers/adc/imx93-adc.c b/drivers/adc/imx93-adc.c new file mode
> 100644 index 00..41d04e0426
> --- /dev/null
> +++ b/drivers/adc/imx93-adc.c
> @@ -0,0 +1,290 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2023 ASEM Srl
> + * Author: Luca Ellero
> + *
> + * Originally based on NXP linux-imx kernel v5.15
> +drivers/iio/adc/imx93_adc.c */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define IMX93_ADC_MCR0x00
> +#define IMX93_ADC_MSR0x04
> +#define IMX93_ADC_ISR0x10
> +#define IMX93_ADC_IMR0x20
> +#define IMX93_ADC_CIMR0 0x24
> +#define IMX93_ADC_CTR0 0x94
> +#define IMX93_ADC_NCMR0 0xA4
> +#define IMX93_ADC_PCDR0 0x100
> +#define IMX93_ADC_PCDR1 0x104
> +#define IMX93_ADC_PCDR2 0x108
> +#define IMX93_ADC_PCDR3 0x10c
> +#define IMX93_ADC_PCDR4 0x110
> +#define IMX93_ADC_PCDR5 0x114
> +#define IMX93_ADC_PCDR6 0x118
> +#define IMX93_ADC_PCDR7 0x11c
> +#define IMX93_ADC_CALSTAT0x39C
> +
> +#define IMX93_ADC_MCR_MODE_MASK BIT(29)
> +#define IMX93_ADC_MCR_NSTART_MASKBIT(24)
> +#define IMX93_ADC_MCR_CALSTART_MASK BIT(14)
> +#define IMX93_ADC_MCR_ADCLKSE_MASK BIT(8)
> +#define IMX93_ADC_MCR_PWDN_MASK BIT(0)
> +
> +#define IMX93_ADC_MSR_CALFAIL_MASK BIT(30)
> +#define IMX93_ADC_MSR_CALBUSY_MASK BIT(29)
> +#define IMX93_ADC_MSR_ADCSTATUS_MASK GENMASK(2, 0)
> +
> +#define IMX93_ADC_ISR_EOC_MASK BIT(1)
> +
> +#define IMX93_ADC_IMR_EOC_MASK BIT(1)
> +#define IMX93_ADC_IMR_ECH_MASK BIT(0)
> +
> +#define IMX93_ADC_PCDR_CDATA_MASKGENMASK(11, 0)
> +
> +#define IDLE 0
> +#define POWER_DOWN 1
> +#define WAIT_STATE 2
> +#define BUSY_IN_CALIBRATION 3
> +#define SAMPLE 4
> +#define CONVERSION 6
> +
> +#define IMX93_ADC_MAX_CHANNEL3
> +#define IMX93_ADC_DAT_MASK 0xfff
> +#define IMX93_ADC_TIMEOUT10
> +
> +struct imx93_adc_priv {
> + int active_channel;
> + void __iomem *regs;
> + struct clk ipg_clk;
> +};
> +
> +static void imx93_adc_power_down(struct imx93_adc_priv *adc) {
> + u32 mcr, msr;
> + int ret;
> +
> + mcr = readl(adc->regs + IMX93_ADC_MCR);
> + mcr |= FIELD_PREP(IMX93_ADC_MCR_PWDN_MASK, 1);
> + writel(mcr, adc->regs + IMX93_ADC_MCR);
> +
> + ret = readl_poll_timeout(adc->regs + IMX93_ADC_MSR, msr,
> + ((msr & IMX93_ADC_MSR_ADCSTATUS_MASK) == POWER_DOWN),
> 50);
> + if (ret == -ETIMEDOUT)
> + pr_warn("ADC not in power down mode, current MSR: %x\n", msr); }
> +
> +static void im
Re: EFI Secure boot default keys
Hi,
On Wed, Jul 05, 2023 at 01:24:32PM +, Neil Jones wrote:
> >> Please can someone describe the format of the file needed for the default
> >> / built-in EFI secure boot keys (ubootefi.var)
> >>
> >> The only docs I have found suggest its best to enroll the keys from within
> >> u-boot onto some removable media, then copy this off and use this as the
> >> default, this is not very helpful and doesn't work for me:
> >>
> >> => fatload mmc 0:1 ${loadaddr} PK.aut
> >> 2053 bytes read in 18 ms (111.3 KiB/s)
> >> => setenv -e -nv -bs -rt -at -i ${loadaddr}:$filesize PK
> >> setenv - set environment variables
> >>
> >> Usage:
> >> setenv setenv [-f] name value ...
> >> - [forcibly] set environment variable 'name' to 'value ...'
> >> setenv [-f] name
> >> - [forcibly] delete environment variable 'name'
> >>
> >> my setenv doesn't support all the extra switches ? This is with 2022.04,
> >> all other EFI options seem to be in this release and I can boot unsigned
> >> EFI images ok.
> >
> >Please turn on CONFIG_CMD_NVEDIT_EFI when building your U-Boot.
> >
> >This option was disabled by the commit:
> >commit 3b728f8728fa (tag: efi-2020-01-rc1)
> >Author: Heinrich Schuchardt
> >Date: Sun Oct 6 15:44:22 2019 +0200
> >
> >cmd: disable CMD_NVEDIT_EFI by default
> >
> >The binary size of efi has grown much since in the past, though.
> >
> >-Takahiro Akashi
>
> Thanks, I have secure boot working now. A tool to generate the ubootefi.var
> offline or even just a description of the file format would be very useful.
Thank you for the suggestion. While I'd like to defer to Heinrich,
the C definition of the file format can be found as struct efi_var_file
in include/efi_variable.h
> I have noticed one issue when using ubootefi.var on mmc, when I switch boot
> order it wipes out the keys and I have to re-enrol them:
>
> => fatls mmc 0:1
> 3040 ubootefi.var
>
> 1 file(s), 0 dir(s)
I'm not sure that secure boot related variables have been loaded
at this point.
Anyhow, please try to enable CONFIG_EFI_VARIABLES_PRESEED with
EFI_VAR_FILE_NAME set. Otherwise, those variables will never be
restored.
(This is another topic that are not described in doc/develop/uefi.)
Thanks,
-Takahiro Akashi
> => efidebug boot order 2 1
> => fatls mmc 0:1
> 440 ubootefi.var
>
> (Size drops from 3040 to 440 bytes and keys have gone)
>
>
>
>
>
>
> From: AKASHI Takahiro
> Sent: 29 June 2023 02:01
> To: Neil Jones
> Cc: u-boot@lists.denx.de
> Subject: Re: EFI Secure boot default keys
>
> On Wed, Jun 28, 2023 at 04:26:58PM +, Neil Jones wrote:
> > Please can someone describe the format of the file needed for the default /
> > built-in EFI secure boot keys (ubootefi.var)
> >
> > The only docs I have found suggest its best to enroll the keys from within
> > u-boot onto some removable media, then copy this off and use this as the
> > default, this is not very helpful and doesn't work for me:
> >
> > => fatload mmc 0:1 ${loadaddr} PK.aut
> > 2053 bytes read in 18 ms (111.3 KiB/s)
> > => setenv -e -nv -bs -rt -at -i ${loadaddr}:$filesize PK
> > setenv - set environment variables
> >
> > Usage:
> > setenv setenv [-f] name value ...
> > - [forcibly] set environment variable 'name' to 'value ...'
> > setenv [-f] name
> > - [forcibly] delete environment variable 'name'
> >
> > my setenv doesn't support all the extra switches ? This is with 2022.04,
> > all other EFI options seem to be in this release and I can boot unsigned
> > EFI images ok.
>
> Please turn on CONFIG_CMD_NVEDIT_EFI when building your U-Boot.
>
> This option was disabled by the commit:
> commit 3b728f8728fa (tag: efi-2020-01-rc1)
> Author: Heinrich Schuchardt
> Date: Sun Oct 6 15:44:22 2019 +0200
>
> cmd: disable CMD_NVEDIT_EFI by default
>
> The binary size of efi has grown much since in the past, though.
>
> -Takahiro Akashi
>
> > Cheers,
> >
> > Neil
> >
> >
> >
Re: [PATCH v5] dt-bindings: riscv: deprecate riscv,isa
On Sun, 02 Jul 2023 00:10:01 +0100, Conor Dooley wrote: > intro > = > > When the RISC-V dt-bindings were accepted upstream in Linux, the base > ISA etc had yet to be ratified. By the ratification of the base ISA, > incompatible changes had snuck into the specifications - for example the > Zicsr and Zifencei extensions were spun out of the base ISA. > > [...] Applied, thanks! [1/1] dt-bindings: riscv: deprecate riscv,isa https://git.kernel.org/palmer/c/aeb71e42caae Best regards, -- Palmer Dabbelt
Re: [PATCH 2/2] board: rockchip: Add Hardkernel ODROID-M1
On 2023-07-02 22:47, Jonas Karlman wrote:
> Hardkernel ODROID-M1 is a single board computer with a RK3568B2 SoC,
> a slightly modified version of the RK3568 SoC.
>
> Features tested on a ODROID-M1 8GB v1.0 2022-06-13:
> - SD-card boot
> - eMMC boot
> - SPI Flash boot
> - PCIe/NVMe/AHCI
> - SATA port
> - USB host
>
> Device tree is imported from linux v6.4.
>
> Signed-off-by: Jonas Karlman
Thanks for you patch! I've compared it to my version, and did not notice
any downside/anything missing. If anything, your version is more feature
complete. I've also tested it on an ODORID-M1 with 8GB of memory, it
boots fine from SD card.
Reviewed-by: Stefan Agner
Tested-by: Stefan Agner
One thing I've noticed is that USB isn't working when I use the stock
SPL (2017.09) running from the SPI RAM and upstream U-Boot (by writing
u-boot.itb to a raw GPT partition named "uboot" to the SD-card). That is
the same in my patchset. If the upstream SPL is used, things work. It
seems that something is not (re)initialized in U-Boot. Not sure if we
typically rely on the state the SPL leaves the HW at, but it would be
nice if we are able to make that combination work. This allows to boot
an upstream U-Boot from an SD-card without having to reflash the onboard
SPI.
On a different note: Do you know if PCIe/NVMe support in SPL is
something which is in the cards for this board?
--
Stefan
> ---
> arch/arm/dts/Makefile | 1 +
> arch/arm/dts/rk3568-odroid-m1-u-boot.dtsi | 46 ++
> arch/arm/dts/rk3568-odroid-m1.dts | 744 ++
> board/rockchip/evb_rk3568/MAINTAINERS | 7 +
> configs/odroid-m1-rk3568_defconfig| 103 +++
> doc/board/rockchip/rockchip.rst | 1 +
> 6 files changed, 902 insertions(+)
> create mode 100644 arch/arm/dts/rk3568-odroid-m1-u-boot.dtsi
> create mode 100644 arch/arm/dts/rk3568-odroid-m1.dts
> create mode 100644 configs/odroid-m1-rk3568_defconfig
>
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index 480269fa6065..334c1bafda17 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -169,6 +169,7 @@ dtb-$(CONFIG_ROCKCHIP_RK3568) += \
> rk3566-anbernic-rgxx3.dtb \
> rk3566-radxa-cm3-io.dtb \
> rk3568-evb.dtb \
> + rk3568-odroid-m1.dtb \
> rk3568-rock-3a.dtb
>
> dtb-$(CONFIG_ROCKCHIP_RK3588) += \
> diff --git a/arch/arm/dts/rk3568-odroid-m1-u-boot.dtsi
> b/arch/arm/dts/rk3568-odroid-m1-u-boot.dtsi
> new file mode 100644
> index ..dc8ad98715ce
> --- /dev/null
> +++ b/arch/arm/dts/rk3568-odroid-m1-u-boot.dtsi
> @@ -0,0 +1,46 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include "rk356x-u-boot.dtsi"
> +
> +/ {
> + aliases {
> + spi0 = &sfc;
> + };
> +
> + chosen {
> + stdout-path = &uart2;
> + };
> +};
> +
> +&fspi_dual_io_pins {
> + bootph-all;
> +};
> +
> +&sdhci {
> + cap-mmc-highspeed;
> + mmc-ddr-1_8v;
> + mmc-hs200-1_8v;
> + mmc-hs400-1_8v;
> + mmc-hs400-enhanced-strobe;
> + pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_datastrobe>;
> +};
> +
> +&sfc {
> + bootph-pre-ram;
> + u-boot,spl-sfc-no-dma;
> +
> + flash@0 {
> + bootph-pre-ram;
> + };
> +};
> +
> +&uart2 {
> + bootph-all;
> + clock-frequency = <2400>;
> + status = "okay";
> +};
> +
> +&vcc5v0_usb_host {
> + /* Workaround until regulator implement basic reference counter */
> + regulator-always-on;
> +};
> diff --git a/arch/arm/dts/rk3568-odroid-m1.dts
> b/arch/arm/dts/rk3568-odroid-m1.dts
> new file mode 100644
> index ..59ecf868dbd0
> --- /dev/null
> +++ b/arch/arm/dts/rk3568-odroid-m1.dts
> @@ -0,0 +1,744 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2022 Hardkernel Co., Ltd.
> + *
> + */
> +
> +/dts-v1/;
> +#include
> +#include
> +#include
> +#include
> +#include "rk3568.dtsi"
> +
> +/ {
> + model = "Hardkernel ODROID-M1";
> + compatible = "rockchip,rk3568-odroid-m1", "rockchip,rk3568";
> +
> + aliases {
> + ethernet0 = &gmac0;
> + i2c0 = &i2c3;
> + i2c3 = &i2c0;
> + mmc0 = &sdhci;
> + mmc1 = &sdmmc0;
> + serial0 = &uart1;
> + serial1 = &uart0;
> + };
> +
> + chosen {
> + stdout-path = "serial2:150n8";
> + };
> +
> + dc_12v: dc-12v-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "dc_12v";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <1200>;
> + regulator-max-microvolt = <1200>;
> + };
> +
> + hdmi-con {
> + compatible = "hdmi-connector";
> + type = "a";
> +
> + port {
> + hdmi_con_in: endpoint {
> + remote-endpoint = <&hdmi_out_con>;
> + };
> + };
[PATCH] ARM: dts: imx: Fix eMMC boot on Data Modul i.MX8M Plus eDM SBC
In case the i.MX8M Plus starts from eMMC BOOT1/BOOT2 HW partitions, the
flash.bin container is stored at offset 0 from the start, that means the
fitImage itb is at offset 0x2c0 instead of 0x300 sectors from the start.
Handle this difference in custom spl_mmc_get_uboot_raw_sector() .
Signed-off-by: Marek Vasut
---
Cc: Fabio Estevam
Cc: Peng Fan
Cc: Stefano Babic
---
board/data_modul/imx8mp_edm_sbc/spl.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/board/data_modul/imx8mp_edm_sbc/spl.c
b/board/data_modul/imx8mp_edm_sbc/spl.c
index c30185e48d4..2fdd95a730c 100644
--- a/board/data_modul/imx8mp_edm_sbc/spl.c
+++ b/board/data_modul/imx8mp_edm_sbc/spl.c
@@ -107,6 +107,20 @@ void board_boot_order(u32 *spl_boot_list)
spl_boot_list[4] = BOOT_DEVICE_NONE;
}
+unsigned long spl_mmc_get_uboot_raw_sector(struct mmc *mmc, unsigned long sect)
+{
+ const u32 boot_dev = spl_boot_device();
+ int part;
+
+ if (boot_dev == BOOT_DEVICE_MMC2) { /* eMMC */
+ part = spl_mmc_emmc_boot_partition(mmc);
+ if (part == 1 || part == 2) /* eMMC BOOT1/BOOT2 HW
partitions */
+ return sect - 0x40;
+ }
+
+ return sect;
+}
+
static struct dram_timing_info *dram_timing_info[8] = {
&dmo_imx8mp_sbc_dram_timing_32_32, /* 32 Gbit x32 */
NULL, /* 32 Gbit x16 */
--
2.40.1
Re: [PATCH] arm64: dts: rockchip: rk3568: Add ODROID-M1 board support
On 2023-07-05 18:24, Jonas Karlman wrote: > Hi Stefan, > On 2023-07-05 18:16, Stefan Agner wrote: >> Add ODROID-M1 board support. Board device tree rk3568-odroid-m1.dts >> from v6.4. > > I sent out a series that add support for ODROID-M1 a few days ago. > Please see https://patchwork.ozlabs.org/project/uboot/list/?series=362045 Cool, thanks for the hint! I've checked master branch but not the ML :) -- Stefan > > Regards, > Jonas > >> >> Signed-off-by: Stefan Agner >> --- >> arch/arm/dts/Makefile | 1 + >> arch/arm/dts/rk3568-odroid-m1-u-boot.dtsi | 29 + >> arch/arm/dts/rk3568-odroid-m1.dts | 744 ++ >> configs/odroid-m1_defconfig | 84 +++ >> 4 files changed, 858 insertions(+) >> create mode 100644 arch/arm/dts/rk3568-odroid-m1-u-boot.dtsi >> create mode 100644 arch/arm/dts/rk3568-odroid-m1.dts >> create mode 100644 configs/odroid-m1_defconfig >> > > [...]
Re: [PATCH] phy: phy-uclass: Add a missing error handling path
Hi Bhupesh,
On 2023-07-05 06:48, Bhupesh Sharma wrote:
> Hi Jonas,
>
> On Mon, 3 Jul 2023 at 01:18, Jonas Karlman wrote:
>>
>> On 2023-07-02 20:47, Bhupesh Sharma wrote:
>>> Since function 'phy_get_counts()' can return NULL,
>>> add handling for that error path inside callers of
>>> this function.
>>
>> Do you have any example where this can/has happened?
>
> Yes, it happened while I was writing a UFS Host Controller driver for
> u-boot and tried to initialize the UFS PHY via the generic u-boot PHY
> CLASS utility functions, namely:
>
> /* get phy */
> ret = generic_phy_get_by_name(hba->dev, "ufsphy", &phy);
> ...
>
> /* phy initialization */
> ret = generic_phy_init(&phy);
> ...
> /* power on phy */
> ret = generic_phy_power_on(&phy);
>
This looks like a proper call order that should not cause counts ending
up as NULL. If this call order still cause issue for your driver there
must be some other underlying issue that also needs to be fixed.
>> Counts should never be NULL in a normal working call flow. I am a bit
>> worried that this patch may hide some other bug or use of an
>> uninitialized phy struct.
>
> I agree, but I found that if the UFS Host Controller driver would mess
> up the phy call sequences while it was in a 'power_up_sequence',
> instead of using the standard UCLASS_PHY
> 'generic_phy_get_by_index_nodev' flow, it might get a counts value
> NULL and then the PHY driver would silently crash while setting /
> accessing members of 'counts'.
>
> int generic_phy_init(struct phy *phy)
> {
> counts = phy_get_counts(phy);
> ...
> if (counts->init_count > 0) {
> // crash
> ..
I fully understand that this will cause a crash here, the issue is that
this should never happen in the first place as long as the documentation
for the phy struct is followed:
Clients provide storage for phy handles. The content of the structure is
managed solely by the PHY API and PHY drivers. A phy struct is
initialized by "get"ing the phy struct. The phy struct is passed to all
other phy APIs to identify which PHY port to operate upon.
The possible reasons I can see for counts ending up as NULL is:
- Use of a phy struct not initialized by generic_phy_get_by_index_nodev.
- phy->id or phy->dev is somehow changed from "get"ing the phy struct
and when it is passed to generic_phy_ functions.
- Some other issue or bad behavior in phy driver.
- of_xlate ops, device_get_supply_regulator or phy_alloc_counts fails in
generic_phy_get_by_index_nodev and phy struct is later passed to
generic_phy_ functions.
The last one seem like a bug, phy->dev is left assigned when
generic_phy_get_by_index_nodev return an error.
This could be fixed with the following diff:
--- a/drivers/phy/phy-uclass.c
+++ b/drivers/phy/phy-uclass.c
@@ -195,6 +195,7 @@ int generic_phy_get_by_index_nodev(ofnode node, int index,
struct phy *phy)
return 0;
err:
+ phy->dev = NULL;
return ret;
}
If we also need a null check for counts, I think a return value of
-EINVAL is more appropriate, the phy struct passed to the function is in
an invalid state, ENODEV should have been returned when "get"ing the phy
struct.
>
>> The phy struct is initialized in generic_phy_get_by_index_nodev. That
>> function should fail when counts cannot be allocated.
>>
>> Maybe generic_phy_get_by_index_nodev should unset phy->dev if it fails,
>> or generic_phy_valid should be extended to also check phy->counts.
>> That way generic_phy_valid would return false and these functions
>> should return earlier before trying to use counts.
>
> I agree, this error handling should be here for counts being
> uninitialized, whether we do it per-function or at the top level.. As
> I can see several users of the flow I described in the u-boot code
> itself (for setting up a PHY), for e.g.:
>
> board/sunxi/board.c
> drivers/usb/cdns3/core.c
These drivers seem to follow a proper call order and use appropriate
return value checks. They may possible be affected by the issue
mentioned above.
Regards,
Jonas
>
> etc..
>
> Thanks,
> Bhupesh
[PATCH 0/5] ARM: dts: at91: Improve sam9x60-curiosity
Hello everyone, currently I have the Microchip SAM9X60-Curiosity board on my desk and for evaluation purposes I'm trying to get it booting from NAND flash without SD card. This series contains a collection of patches I made on that journey. It's probably not the last set of patches, but I send it out now before my holidays so anyone who's interested has some time to look into it before I return to my desk in roughly two weeks. ;-) Note: while I could access the I²C EEPROM populated on that board with Linux v6.4 through nvmem and the 100 bytes content seem valid including a MAC address attributed to Microchip, I could not do so in U-Boot. That also means out of the box access to Ethernet is currently broken without valid MAC address. Note: I prepared the patches at the office, but sending them from home now. This is why I use two different mail addresses. Greets Alex Alexander Dahl (5): ARM: dts: at91: sam9x60: Better align with upstream dtsi ARM: dts: at91: sam9x60-curiosity: Fix EEPROM type ARM: dts: at91: sam9x60: Change i2c compatible ARM: dts: at91: sam9x60-curiosity: Improve alignment with upstream ARM: dts: at91: sam9x60-curiosity: Add raw NAND flash .../dts/at91-sam9x60_curiosity-u-boot.dtsi| 8 +- arch/arm/dts/at91-sam9x60_curiosity.dts | 203 +- arch/arm/dts/sam9x60.dtsi | 66 +++--- arch/arm/dts/sam9x60ek.dts| 2 +- 4 files changed, 190 insertions(+), 89 deletions(-) base-commit: e1bebc16e1d9aa0ddd56c53c0b781f7186dce557 -- 2.30.2
[PATCH 4/5] ARM: dts: at91: sam9x60-curiosity: Improve alignment with upstream
From: Alexander Dahl
- nodes moved
- using node references by label instead of dulicating the node tree
Makes it easier to compare with the dts file from Linux kernel.
Signed-off-by: Alexander Dahl
---
.../dts/at91-sam9x60_curiosity-u-boot.dtsi| 8 +-
arch/arm/dts/at91-sam9x60_curiosity.dts | 100 +-
2 files changed, 53 insertions(+), 55 deletions(-)
diff --git a/arch/arm/dts/at91-sam9x60_curiosity-u-boot.dtsi
b/arch/arm/dts/at91-sam9x60_curiosity-u-boot.dtsi
index 0c3c0406b4..a1b76e94d1 100644
--- a/arch/arm/dts/at91-sam9x60_curiosity-u-boot.dtsi
+++ b/arch/arm/dts/at91-sam9x60_curiosity-u-boot.dtsi
@@ -14,10 +14,6 @@
apb {
bootph-all;
-
- pinctrl {
- bootph-all;
- };
};
};
@@ -42,6 +38,10 @@
bootph-all;
};
+&pinctrl {
+ bootph-all;
+};
+
&pinctrl_dbgu {
bootph-all;
};
diff --git a/arch/arm/dts/at91-sam9x60_curiosity.dts
b/arch/arm/dts/at91-sam9x60_curiosity.dts
index ae707dd64b..fb59405b24 100644
--- a/arch/arm/dts/at91-sam9x60_curiosity.dts
+++ b/arch/arm/dts/at91-sam9x60_curiosity.dts
@@ -11,60 +11,18 @@
#include "sam9x60.dtsi"
/ {
- model = "Microchip SAM9X60 CURIOSITY";
+ model = "Microchip SAM9X60 Curiosity";
compatible = "microchip,sam9x60-curiosity", "microchip,sam9x60",
"atmel,at91sam9";
- ahb {
- apb {
- flx0: flexcom@f801c600 {
- atmel,flexcom-mode = ;
- status = "okay";
-
- i2c@600 {
- compatible = "microchip,sam9x60-i2c";
- reg = <0x600 0x200>;
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_flx0>;
- #address-cells = <1>;
- #size-cells = <0>;
- clocks = <&pmc PMC_TYPE_PERIPHERAL 5>;
- status = "okay";
-
- eeprom@53 {
- compatible = "atmel,24c02";
- reg = <0x53>;
- pagesize = <16>;
- };
- };
- };
-
- pinctrl {
- pinctrl_flx0: flx0_default {
- atmel,pins =
- ;
- };
-
- pinctrl_onewire_tm_default:
onewire_tm_default {
- atmel,pins =
- ;
- };
-
- usb1 {
- pinctrl_usb_default:
usb_default {
- atmel,pins = ;
- };
- };
- };
- };
- };
-
chosen {
stdout-path = &dbgu;
i2c0 = &flx0;
};
+ memory {
+ reg = <0x2000 0x800>;
+ };
+
clocks {
slow_xtal: slow_xtal {
clock-frequency = <32768>;
@@ -75,10 +33,6 @@
};
};
- memory {
- reg = <0x2000 0x800>;
- };
-
onewire_tm: onewire {
gpios = <&pioD 14 GPIO_ACTIVE_HIGH>;
pinctrl-names = "default";
@@ -92,11 +46,55 @@
};
};
+&flx0 {
+ atmel,flexcom-mode = ;
+ status = "okay";
+
+ i2c@600 {
+ compatible = "microchip,sam9x60-i2c";
+ reg = <0x600 0x200>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_flx0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ clocks = <&pmc PMC_TYPE_PERIPHERAL 5>;
+ status = "okay";
+
+ eeprom@53 {
+ compatible = "atmel,24c02";
+ reg = <0x53>;
+ pagesize = <16>;
+ };
+ };
+};
+
&macb0 {
phy-mode = "rmii";
status = "okay";
};
+&pinctrl {
+ flexcom {
+ pinctrl_flx0: flx0_default {
+ atmel,pins =
+ ;
+ };
+ };
+
+ pinctrl_onewire_tm_default: onewire_tm_default {
+ atmel,pins =
+
[PATCH 5/5] ARM: dts: at91: sam9x60-curiosity: Add raw NAND flash
From: Alexander Dahl
Basically the same as on sam9x60-ek. Same as in Linux. NAND flash is
correctly detected when booting into U-Boot:
U-Boot 2023.07-rc6-5-g12719f75dc-dirty (Jul 05 2023 - 13:06:35 +)
CPU: SAM9X60 128MiB DDR2 SiP
Crystal frequency: 24 MHz
CPU clock: 600 MHz
Master clock : 200 MHz
Model: Microchip SAM9X60 Curiosity
DRAM: 128 MiB
Core: 145 devices, 22 uclasses, devicetree: separate
NAND: 512 MiB
MMC: sdhci-host@8000: 0, sdhci-host@9000: 1
Loading Environment from FAT... Unable to read "uboot.env" from mmc0:1...
In:serial
Out: serial
Err: serial
Net: eth0: ethernet@f802c000
Hit any key to stop autoboot: 0
Signed-off-by: Alexander Dahl
---
arch/arm/dts/at91-sam9x60_curiosity.dts | 103
1 file changed, 103 insertions(+)
diff --git a/arch/arm/dts/at91-sam9x60_curiosity.dts
b/arch/arm/dts/at91-sam9x60_curiosity.dts
index fb59405b24..2547b4527c 100644
--- a/arch/arm/dts/at91-sam9x60_curiosity.dts
+++ b/arch/arm/dts/at91-sam9x60_curiosity.dts
@@ -46,6 +46,71 @@
};
};
+&ebi {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_ebi_addr_nand &pinctrl_ebi_data_0_7>;
+ status = "okay";
+
+ nand_controller: nand-controller {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_nand_oe_we &pinctrl_nand_cs
&pinctrl_nand_rb>;
+ status = "okay";
+
+ nand@3 {
+ reg = <0x3 0x0 0x80>;
+ rb-gpios = <&pioD 5 GPIO_ACTIVE_HIGH>;
+ cs-gpios = <&pioD 4 GPIO_ACTIVE_HIGH>;
+ nand-bus-width = <8>;
+ nand-ecc-mode = "hw";
+ nand-ecc-strength = <8>;
+ nand-ecc-step-size = <512>;
+ nand-on-flash-bbt;
+ label = "atmel_nand";
+
+ partitions {
+ compatible = "fixed-partitions";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ at91bootstrap@0 {
+ label = "at91bootstrap";
+ reg = <0x0 0x4>;
+ };
+
+ uboot@4 {
+ label = "u-boot";
+ reg = <0x4 0xc>;
+ };
+
+ ubootenvred@10 {
+ label = "U-Boot Env Redundant";
+ reg = <0x10 0x4>;
+ };
+
+ ubootenv@14 {
+ label = "U-Boot Env";
+ reg = <0x14 0x4>;
+ };
+
+ dtb@18 {
+ label = "device tree";
+ reg = <0x18 0x8>;
+ };
+
+ kernel@20 {
+ label = "kernel";
+ reg = <0x20 0x60>;
+ };
+
+ rootfs@80 {
+ label = "rootfs";
+ reg = <0x80 0x1f80>;
+ };
+ };
+ };
+ };
+};
+
&flx0 {
atmel,flexcom-mode = ;
status = "okay";
@@ -74,6 +139,26 @@
};
&pinctrl {
+ ebi {
+ pinctrl_ebi_data_0_7: ebi-data-lsb-0 {
+ atmel,pins =
+ ;
+ };
+
+ pinctrl_ebi_addr_nand: ebi-addr-0 {
+ atmel,pins =
+ ;
+ };
+ };
+
flexcom {
pinctrl_flx0: flx0_default {
atmel,pins =
@@ -82,6 +167,24 @@
};
};
+ nand {
+ pinctrl_nand_oe_we: nand-oe-we-0 {
+ atmel,pins =
+ ;
+ };
+
+ pinctrl_nand_rb: nand-rb-0 {
+ atmel,pins =
+ ;
+ };
+
+ pinctrl_nand_cs: nand-cs-0 {
+ atmel,pins =
+ ;
+ };
+ };
+
pinctrl_onewire_tm_default: onewire_tm_default {
atmel,pins =
;
--
2.30.2
[PATCH 2/5] ARM: dts: at91: sam9x60-curiosity: Fix EEPROM type
From: Alexander Dahl
The user guide says it's a Microchip 24AA025E48 serial EEPROM, which is
a 2-Kbit I2C Serial EEPROM with EUI-48™ Identity. This is the chip
actually populated on board EV40E67A rev 4.
Signed-off-by: Alexander Dahl
---
Notes:
Sadly this did not fix the problem, that I could not access that
eeprom through I²C.
arch/arm/dts/at91-sam9x60_curiosity.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/dts/at91-sam9x60_curiosity.dts
b/arch/arm/dts/at91-sam9x60_curiosity.dts
index d6ae3d648d..da5e19b66b 100644
--- a/arch/arm/dts/at91-sam9x60_curiosity.dts
+++ b/arch/arm/dts/at91-sam9x60_curiosity.dts
@@ -31,7 +31,7 @@
status = "okay";
eeprom@53 {
- compatible = "atmel,24c32";
+ compatible = "atmel,24c02";
reg = <0x53>;
pagesize = <16>;
};
--
2.30.2
[PATCH 1/5] ARM: dts: at91: sam9x60: Better align with upstream dtsi
From: Alexander Dahl
No functional changes, but this:
- reorder nodes (ordered by memory offset as in Linux)
- add label to pinctrl node name for easier reference in board files
- fix whitespace
Diff to sam9x60.dtsi in Linux is much better readable now.
Signed-off-by: Alexander Dahl
---
arch/arm/dts/sam9x60.dtsi | 66 +++
1 file changed, 33 insertions(+), 33 deletions(-)
diff --git a/arch/arm/dts/sam9x60.dtsi b/arch/arm/dts/sam9x60.dtsi
index 2b93d08938..3b684fc63d 100644
--- a/arch/arm/dts/sam9x60.dtsi
+++ b/arch/arm/dts/sam9x60.dtsi
@@ -27,6 +27,18 @@
spi0 = &qspi;
};
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ARM9260_0: cpu@0 {
+ device_type = "cpu";
+ compatible = "arm,arm926ej-s";
+ clocks = <&pmc PMC_TYPE_CORE 19>, <&pmc PMC_TYPE_CORE
11>, <&main_xtal>;
+ clock-names = "cpu", "master", "xtal";
+ };
+ };
+
clocks {
slow_rc_osc: slow_rc_osc {
compatible = "fixed-clock";
@@ -51,18 +63,6 @@
};
};
- cpus {
- #address-cells = <1>;
- #size-cells = <0>;
-
- ARM9260_0: cpu@0 {
- device_type = "cpu";
- compatible = "arm,arm926ej-s";
- clocks = <&pmc PMC_TYPE_CORE 19>, <&pmc PMC_TYPE_CORE
11>, <&main_xtal>;
- clock-names = "cpu", "master", "xtal";
- };
- };
-
ahb {
compatible = "simple-bus";
#address-cells = <1>;
@@ -149,13 +149,20 @@
compatible = "microchip,sam9x60-qspi";
reg = <0xf0014000 0x100>, <0x7000
0x1000>;
reg-names = "qspi_base", "qspi_mmap";
- clocks = <&pmc PMC_TYPE_PERIPHERAL 35>, <&pmc
PMC_TYPE_SYSTEM 18>; /* ID_QSPI */
+ clocks = <&pmc PMC_TYPE_PERIPHERAL 35>, <&pmc
PMC_TYPE_SYSTEM 18>; /* ID_QSPI */
clock-names = "pclk", "qspick";
#address-cells = <1>;
#size-cells = <0>;
status = "disabled";
};
+ pit64b0: timer@f0028000 {
+ compatible = "microchip,sam9x60-pit64b";
+ reg = <0xf0028000 0xec>;
+ clocks = <&pmc PMC_TYPE_PERIPHERAL 37>, <&pmc
PMC_TYPE_GCK 37>;
+ clock-names = "pclk", "gclk";
+ };
+
flx0: flexcom@f801c600 {
compatible = "atmel,sama5d2-flexcom";
reg = <0xf801c000 0x200>;
@@ -181,6 +188,17 @@
reg = <0xf805 0x100>;
};
+ pmecc: ecc-engine@e000 {
+ compatible = "microchip,sam9x60-pmecc",
"atmel,at91sam9g45-pmecc";
+ reg = <0xe000 0x300>,
+ <0xe600 0x100>;
+ };
+
+ smc: smc@ea00 {
+ compatible = "microchip,sam9x60-smc",
"atmel,at91sam9260-smc", "syscon";
+ reg = <0xea00 0x100>;
+ };
+
dbgu: serial@f200 {
compatible = "atmel,at91sam9260-dbgu",
"atmel,at91sam9260-usart";
reg = <0xf200 0x200>;
@@ -190,7 +208,7 @@
clock-names = "usart";
};
- pinctrl {
+ pinctrl: pinctrl@f400 {
#address-cells = <1>;
#size-cells = <1>;
compatible = "microchip,sam9x60-pinctrl",
"simple-bus";
@@ -205,7 +223,7 @@
pinctrl_dbgu: dbgu-0 {
atmel,pins =
;
+AT91_PIOA 10
AT91_PERIPH_A AT91_PINCTRL_NONE>;
};
};
@@ -256,17 +274,6 @@
};
};
- pmecc: ecc-engine@e000 {
- compatible = "microchip,sam9x60-pmecc",
"atmel,at91sam9g45-pmecc";
- reg = <0xe000 0x300>,
- <0xe600 0x100>;
-
[PATCH 3/5] ARM: dts: at91: sam9x60: Change i2c compatible
From: Alexander Dahl
There's a more specific compatible string for the i2c interface, use it.
Signed-off-by: Alexander Dahl
---
Notes:
I²C access to the eeprom did not work though, neither before nor after
this change.
arch/arm/dts/at91-sam9x60_curiosity.dts | 2 +-
arch/arm/dts/sam9x60ek.dts | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/dts/at91-sam9x60_curiosity.dts
b/arch/arm/dts/at91-sam9x60_curiosity.dts
index da5e19b66b..ae707dd64b 100644
--- a/arch/arm/dts/at91-sam9x60_curiosity.dts
+++ b/arch/arm/dts/at91-sam9x60_curiosity.dts
@@ -21,7 +21,7 @@
status = "okay";
i2c@600 {
- compatible = "atmel,sama5d2-i2c";
+ compatible = "microchip,sam9x60-i2c";
reg = <0x600 0x200>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_flx0>;
diff --git a/arch/arm/dts/sam9x60ek.dts b/arch/arm/dts/sam9x60ek.dts
index 45e2f4cc40..74016f5e28 100644
--- a/arch/arm/dts/sam9x60ek.dts
+++ b/arch/arm/dts/sam9x60ek.dts
@@ -62,7 +62,7 @@
status = "okay";
i2c@600 {
- compatible = "atmel,sama5d2-i2c";
+ compatible = "microchip,sam9x60-i2c";
reg = <0x600 0x200>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_flx0>;
--
2.30.2
Re: [PATCH 1/1] tools: spkgimage: correct printf specifier
On Tue, 04 Jul 2023 22:18:09 +0200, Heinrich Schuchardt wrote:
> Compiling on armv7 results in:
>
> tools/renesas_spkgimage.c: In function ‘spkgimage_parse_config_line’:
> tools/renesas_spkgimage.c:76:66: warning: format ‘%ld’ expects
> argument of type ‘long int’, but argument 3 has type ‘size_t’
> {aka ‘unsigned int’} [-Wformat=]
>76 | "config error: unknown keyword on line %ld\n",
> |~~^
> | |
> | long int
> |%d
>77 | line_num);
> |
> | |
> | size_t {aka unsigned int}
>
> [...]
Applied to u-boot/master, thanks!
--
Tom
[PATCH] rockchip: rk3399: nanopc-t4: use 1600MHz sdram config
Current 1866MHz sdram config is too high for NanoPC-T4. On this frequency, its lpddr3 sdram becomes unstable, causing memtest failures and random kernel crashes. Signed-off-by: Lu jicong --- arch/arm/dts/rk3399-nanopc-t4-u-boot.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/dts/rk3399-nanopc-t4-u-boot.dtsi b/arch/arm/dts/rk3399-nanopc-t4-u-boot.dtsi index 17201bcf41..8b6c9059ab 100644 --- a/arch/arm/dts/rk3399-nanopc-t4-u-boot.dtsi +++ b/arch/arm/dts/rk3399-nanopc-t4-u-boot.dtsi @@ -4,4 +4,4 @@ */ #include "rk3399-nanopi4-u-boot.dtsi" -#include "rk3399-sdram-lpddr3-samsung-4GB-1866.dtsi" +#include "rk3399-sdram-lpddr3-4GB-1600.dtsi" -- 2.39.2
Re: [bug report] sunxi: H6: no ethernet on Orange Pi One Plus
On Wed, Jul 5, 2023 at 1:31 PM Anne Macedo wrote: > I think I'm on the right path :) > > 1. Included SUNXI_SETUP_REGULATORS=0 to the bl31 make > 2. Changed the phy mode on arch/arm/dts/sun50i-h6-orangepi-one-plus.dts >From rgmii-id to rgmii This does not look like the correct fix. Please see this commit from Linux: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=v6.4.1&id=544cc3f8573bf9a82e8f348741f2f68d2a8376fb >More info on [1][2][3] > 3. Added this configs to configs/orangepi_one_plus_defconfig: >CONFIG_SPL_SPI_SUNXI=y >CONFIG_SUNXI_NO_PMIC=y >CONFIG_SUN8I_EMAC=y > > Result: > > U-Boot 2023.04-gfd4ed6b (Apr 03 2023 - 20:38:50 +) Allwinner > Technology > > CPU: Allwinner H6 (SUN50I) > Model: OrangePi One Plus > DRAM: 1 GiB > Core: 55 devices, 17 uclasses, devicetree: separate > WDT: Not starting watchdog@7020400 > MMC: mmc@402: 0 > Loading Environment from FAT... Unable to read "uboot.env" from > mmc0:1... > In:serial@500 > Out: serial@500 > Err: serial@500 > Net: eth0: ethernet@502 > > => dhcp > sun8i_emac_eth_start: Timeout > => mdio list > ethernet@502: > 1 - Generic PHY <--> ethernet@502 What about using the Realtek PHY driver instead of the Generic one? --- a/configs/orangepi_one_plus_defconfig +++ b/configs/orangepi_one_plus_defconfig @@ -8,3 +8,6 @@ CONFIG_SUNXI_DRAM_H6_LPDDR3=y # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set CONFIG_USB_EHCI_HCD=y CONFIG_USB_OHCI_HCD=y +CONFIG_PHY_REALTEK=y +CONFIG_RGMII=y +CONFIG_MII=y Does this help?
Re: [bug report] sunxi: H6: no ethernet on Orange Pi One Plus
On Wed, Jul 05, 2023 at 01:31:28PM -0300, Anne Macedo wrote: > On Wed, Jul 05, 2023 at 03:24:23PM +, Anne Macedo wrote: > > On Wed, Jul 05, 2023 at 10:46:25AM -0300, Fabio Estevam wrote: > > > Hi Anne, > > > > > > On Tue, Jul 4, 2023 at 8:52 PM Anne Macedo wrote: > > > > > > > > Hey! > > > > > > > > I'm trying to bake Linux images for the Orange Pi One Plus using Yocto. > > > > Everything works fine, except for Ethernet. > > > > > > > > On the u-boot prompt: > > > > > > > > => dhcp > > > > No ethernet found. > > > > > > > > After adding: > > > > > > > > CONFIG_SPL_SPI_SUNXI=y > > > > CONFIG_SUN8I_EMAC=y > > > > > > > > to configs/orangepi_one_plus_defconfig, I started seeing this error: > > > > > > > > => dhcp > > > > sun8i_emac_eth_start: Timeout > > > > > > > > I saw this other bug report but I couldn't really understand what has > > > > been made to fix this issue [1]. > > > > > > > > More context here [2]. > > > > > > > > [1] https://lists.denx.de/pipermail/u-boot/2021-June/451357.html > > > > > > Does it help if you revert 4f0278dac56a658ef1e0967fec0bb95372a875bd ? > > > > > > > Hey! After reverting the commit, but with PMIC disabled: > > > > => dhcp > > mdio_register: non unique device name 'ethernet@502' > > Could not get PHY for ethernet@502: addr 1 > > > > > I added on CC the folks involved in the previous report. > > > > Regards, > > Anne > > I think I'm on the right path :) > > 1. Included SUNXI_SETUP_REGULATORS=0 to the bl31 make > 2. Changed the phy mode on arch/arm/dts/sun50i-h6-orangepi-one-plus.dts >From rgmii-id to rgmii >More info on [1][2][3] > 3. Added this configs to configs/orangepi_one_plus_defconfig: >CONFIG_SPL_SPI_SUNXI=y >CONFIG_SUNXI_NO_PMIC=y >CONFIG_SUN8I_EMAC=y > > Result: > > U-Boot 2023.04-gfd4ed6b (Apr 03 2023 - 20:38:50 +) Allwinner > Technology > > CPU: Allwinner H6 (SUN50I) > Model: OrangePi One Plus > DRAM: 1 GiB > Core: 55 devices, 17 uclasses, devicetree: separate > WDT: Not starting watchdog@7020400 > MMC: mmc@402: 0 > Loading Environment from FAT... Unable to read "uboot.env" from > mmc0:1... > In:serial@500 > Out: serial@500 > Err: serial@500 > Net: eth0: ethernet@502 > > => dhcp > sun8i_emac_eth_start: Timeout > => mdio list > ethernet@502: > 1 - Generic PHY <--> ethernet@502 > > Still no interface on Linux, but at least eth0 is detected on u-boot? > > [1] > https://forum.armbian.com/topic/7108-orangepi-zero-plus-ethernet-in-u-boot/ > [2] https://forum.openwrt.org/t/sunxi-target-broken-in-master/110643/12 > [3] > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/net/phy/realtek.c?h=v5.10.79&id=bbc4d71d63549bcd003a430de18a72a742d8c91e > Actually had commented out SUNXI_SETUP_REGULATORS=0, with it I see: Net: PHY reset timed out eth0: ethernet@502 > Regards, > Anne
Re: [bug report] sunxi: H6: no ethernet on Orange Pi One Plus
On Wed, Jul 05, 2023 at 03:24:23PM +, Anne Macedo wrote: > On Wed, Jul 05, 2023 at 10:46:25AM -0300, Fabio Estevam wrote: > > Hi Anne, > > > > On Tue, Jul 4, 2023 at 8:52 PM Anne Macedo wrote: > > > > > > Hey! > > > > > > I'm trying to bake Linux images for the Orange Pi One Plus using Yocto. > > > Everything works fine, except for Ethernet. > > > > > > On the u-boot prompt: > > > > > > => dhcp > > > No ethernet found. > > > > > > After adding: > > > > > > CONFIG_SPL_SPI_SUNXI=y > > > CONFIG_SUN8I_EMAC=y > > > > > > to configs/orangepi_one_plus_defconfig, I started seeing this error: > > > > > > => dhcp > > > sun8i_emac_eth_start: Timeout > > > > > > I saw this other bug report but I couldn't really understand what has > > > been made to fix this issue [1]. > > > > > > More context here [2]. > > > > > > [1] https://lists.denx.de/pipermail/u-boot/2021-June/451357.html > > > > Does it help if you revert 4f0278dac56a658ef1e0967fec0bb95372a875bd ? > > > > Hey! After reverting the commit, but with PMIC disabled: > > => dhcp > mdio_register: non unique device name 'ethernet@502' > Could not get PHY for ethernet@502: addr 1 > > > I added on CC the folks involved in the previous report. > > Regards, > Anne I think I'm on the right path :) 1. Included SUNXI_SETUP_REGULATORS=0 to the bl31 make 2. Changed the phy mode on arch/arm/dts/sun50i-h6-orangepi-one-plus.dts From rgmii-id to rgmii More info on [1][2][3] 3. Added this configs to configs/orangepi_one_plus_defconfig: CONFIG_SPL_SPI_SUNXI=y CONFIG_SUNXI_NO_PMIC=y CONFIG_SUN8I_EMAC=y Result: U-Boot 2023.04-gfd4ed6b (Apr 03 2023 - 20:38:50 +) Allwinner Technology CPU: Allwinner H6 (SUN50I) Model: OrangePi One Plus DRAM: 1 GiB Core: 55 devices, 17 uclasses, devicetree: separate WDT: Not starting watchdog@7020400 MMC: mmc@402: 0 Loading Environment from FAT... Unable to read "uboot.env" from mmc0:1... In:serial@500 Out: serial@500 Err: serial@500 Net: eth0: ethernet@502 => dhcp sun8i_emac_eth_start: Timeout => mdio list ethernet@502: 1 - Generic PHY <--> ethernet@502 Still no interface on Linux, but at least eth0 is detected on u-boot? [1] https://forum.armbian.com/topic/7108-orangepi-zero-plus-ethernet-in-u-boot/ [2] https://forum.openwrt.org/t/sunxi-target-broken-in-master/110643/12 [3] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/net/phy/realtek.c?h=v5.10.79&id=bbc4d71d63549bcd003a430de18a72a742d8c91e Regards, Anne
Re: [PATCH] arm64: dts: rockchip: rk3568: Add ODROID-M1 board support
Hi Stefan, On 2023-07-05 18:16, Stefan Agner wrote: > Add ODROID-M1 board support. Board device tree rk3568-odroid-m1.dts > from v6.4. I sent out a series that add support for ODROID-M1 a few days ago. Please see https://patchwork.ozlabs.org/project/uboot/list/?series=362045 Regards, Jonas > > Signed-off-by: Stefan Agner > --- > arch/arm/dts/Makefile | 1 + > arch/arm/dts/rk3568-odroid-m1-u-boot.dtsi | 29 + > arch/arm/dts/rk3568-odroid-m1.dts | 744 ++ > configs/odroid-m1_defconfig | 84 +++ > 4 files changed, 858 insertions(+) > create mode 100644 arch/arm/dts/rk3568-odroid-m1-u-boot.dtsi > create mode 100644 arch/arm/dts/rk3568-odroid-m1.dts > create mode 100644 configs/odroid-m1_defconfig > [...]
[PATCH] arm64: dts: rockchip: rk3568: Add ODROID-M1 board support
Add ODROID-M1 board support. Board device tree rk3568-odroid-m1.dts
from v6.4.
Signed-off-by: Stefan Agner
---
arch/arm/dts/Makefile | 1 +
arch/arm/dts/rk3568-odroid-m1-u-boot.dtsi | 29 +
arch/arm/dts/rk3568-odroid-m1.dts | 744 ++
configs/odroid-m1_defconfig | 84 +++
4 files changed, 858 insertions(+)
create mode 100644 arch/arm/dts/rk3568-odroid-m1-u-boot.dtsi
create mode 100644 arch/arm/dts/rk3568-odroid-m1.dts
create mode 100644 configs/odroid-m1_defconfig
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 480269fa60..11028c3a5e 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -169,6 +169,7 @@ dtb-$(CONFIG_ROCKCHIP_RK3568) += \
rk3566-anbernic-rgxx3.dtb \
rk3566-radxa-cm3-io.dtb \
rk3568-evb.dtb \
+ rk3568-odroid-m1.dts \
rk3568-rock-3a.dtb
dtb-$(CONFIG_ROCKCHIP_RK3588) += \
diff --git a/arch/arm/dts/rk3568-odroid-m1-u-boot.dtsi
b/arch/arm/dts/rk3568-odroid-m1-u-boot.dtsi
new file mode 100644
index 00..3d75dd403b
--- /dev/null
+++ b/arch/arm/dts/rk3568-odroid-m1-u-boot.dtsi
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2021 Rockchip Electronics Co., Ltd
+ * (C) Copyright 2023 Akash Gajjar
+ * (C) Copyright 2023 Stefan Agner
+ */
+
+#include "rk356x-u-boot.dtsi"
+
+/ {
+ chosen {
+ stdout-path = &uart2;
+ u-boot,spl-boot-order = "same-as-spl", &sdhci, &sdmmc0;
+ };
+};
+
+&sdmmc0 {
+ status = "okay";
+};
+
+&sdhci {
+ status = "okay";
+};
+
+&uart2 {
+ clock-frequency = <2400>;
+ bootph-all;
+ u-boot,dm-pre-reloc;
+};
diff --git a/arch/arm/dts/rk3568-odroid-m1.dts
b/arch/arm/dts/rk3568-odroid-m1.dts
new file mode 100644
index 00..59ecf868db
--- /dev/null
+++ b/arch/arm/dts/rk3568-odroid-m1.dts
@@ -0,0 +1,744 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2022 Hardkernel Co., Ltd.
+ *
+ */
+
+/dts-v1/;
+#include
+#include
+#include
+#include
+#include "rk3568.dtsi"
+
+/ {
+ model = "Hardkernel ODROID-M1";
+ compatible = "rockchip,rk3568-odroid-m1", "rockchip,rk3568";
+
+ aliases {
+ ethernet0 = &gmac0;
+ i2c0 = &i2c3;
+ i2c3 = &i2c0;
+ mmc0 = &sdhci;
+ mmc1 = &sdmmc0;
+ serial0 = &uart1;
+ serial1 = &uart0;
+ };
+
+ chosen {
+ stdout-path = "serial2:150n8";
+ };
+
+ dc_12v: dc-12v-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "dc_12v";
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-min-microvolt = <1200>;
+ regulator-max-microvolt = <1200>;
+ };
+
+ hdmi-con {
+ compatible = "hdmi-connector";
+ type = "a";
+
+ port {
+ hdmi_con_in: endpoint {
+ remote-endpoint = <&hdmi_out_con>;
+ };
+ };
+ };
+
+ ir-receiver {
+ compatible = "gpio-ir-receiver";
+ gpios = <&gpio0 RK_PC2 GPIO_ACTIVE_LOW>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&ir_receiver_pin>;
+ };
+
+ leds {
+ compatible = "gpio-leds";
+
+ led_power: led-0 {
+ gpios = <&gpio0 RK_PC6 GPIO_ACTIVE_HIGH>;
+ function = LED_FUNCTION_POWER;
+ color = ;
+ default-state = "keep";
+ linux,default-trigger = "default-on";
+ pinctrl-names = "default";
+ pinctrl-0 = <&led_power_pin>;
+ };
+ led_work: led-1 {
+ gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
+ function = LED_FUNCTION_HEARTBEAT;
+ color = ;
+ linux,default-trigger = "heartbeat";
+ pinctrl-names = "default";
+ pinctrl-0 = <&led_work_pin>;
+ };
+ };
+
+ rk809-sound {
+ compatible = "simple-audio-card";
+ pinctrl-names = "default";
+ pinctrl-0 = <&hp_det_pin>;
+ simple-audio-card,name = "Analog RK817";
+ simple-audio-card,format = "i2s";
+ simple-audio-card,hp-det-gpio = <&gpio0 RK_PB0
GPIO_ACTIVE_HIGH>;
+ simple-audio-card,mclk-fs = <256>;
+ simple-audio-card,widgets =
+ "Headphone", "Headphones",
+ "Speaker", "Speaker";
+ simple-audio-card,routing =
+ "Headphones", "HPOL",
+ "Headphones", "HPOR",
+ "Speaker", "SPKO";
+
+
Re: v2023.07-rc5 regression: Image overlaps SPL
On Tue, Jul 04, 2023 at 02:13:01PM -0300, Fabio Estevam wrote: > On 04/07/2023 14:04, Francesco Dolcini wrote: > > > The boards that do not check the return value might start to behave > > wrongly without an obvious error to help the debugging. > > Yes, the current implementation of fdt_status_disabled() is fragile, but > there's not so much we can do for the upcoming 2023.07. > > I can try to prepare a patch to improve it after 2023.07 is out next week. It's also been fragile-as-designed since inception. The trigger here was that renaming a ton of properties _reduced_ the overall dtb size itself and so some edge cases got pushed over the edge. I'm honestly not sure if it's better to: - Give everyone a small padding by default - Make the platforms which may call one of these functions a small padding by default - Audit the callers and make them handle -FDT_ERR_NOSPACE like the other cases where we grow the dtb size do. Even if yes, many of those other cases are for non-trivial growth rather than just adding 4 more letters. - Make fdt_set_node_status() handle the disabled case when out of space and grow/retry. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2] colibri-imx7: Call fdt_increase_size()
On Tue, 04 Jul 2023 14:09:45 -0300, Fabio Estevam wrote: > For changing the USB OTG node status from "okay" to "disabled" more > space is needed, so call fdt_increase_size() to avoid the following > error: > > ``` > U-Boot 2023.07-rc5-0.0.0-devel+git.580eb31199be (Jun 27 2023 - 13:39:58 > +) > CPU: Freescale i.MX7S rev1.2 800 MHz (running at 792 MHz) > CPU: Extended Commercial temperature grade (-20C to 105C) at 30C > Reset cause: POR > DRAM: initcall sequence 8786eafc failed at call 8781b351 (err=-3) > ### ERROR ### Please RESET the board ### > ``` > > [...] Applied to u-boot/master, thanks! -- Tom
Re: [PATCH] git-mailrc: add alias for Eugen Hristev
On Tue, 04 Jul 2023 14:25:36 +0300, Eugen Hristev wrote: > add my patchwork alias > > Applied to u-boot/master, thanks! -- Tom
Re: [bug report] sunxi: H6: no ethernet on Orange Pi One Plus
On Wed, Jul 05, 2023 at 10:46:25AM -0300, Fabio Estevam wrote: > Hi Anne, > > On Tue, Jul 4, 2023 at 8:52 PM Anne Macedo wrote: > > > > Hey! > > > > I'm trying to bake Linux images for the Orange Pi One Plus using Yocto. > > Everything works fine, except for Ethernet. > > > > On the u-boot prompt: > > > > => dhcp > > No ethernet found. > > > > After adding: > > > > CONFIG_SPL_SPI_SUNXI=y > > CONFIG_SUN8I_EMAC=y > > > > to configs/orangepi_one_plus_defconfig, I started seeing this error: > > > > => dhcp > > sun8i_emac_eth_start: Timeout > > > > I saw this other bug report but I couldn't really understand what has > > been made to fix this issue [1]. > > > > More context here [2]. > > > > [1] https://lists.denx.de/pipermail/u-boot/2021-June/451357.html > > Does it help if you revert 4f0278dac56a658ef1e0967fec0bb95372a875bd ? > Hey! After reverting the commit, but with PMIC disabled: => dhcp mdio_register: non unique device name 'ethernet@502' Could not get PHY for ethernet@502: addr 1 > I added on CC the folks involved in the previous report. Regards, Anne
[PATCHv3 3/3] net/lwip: add doc/develop/net_lwip.rst
Just add inital doc. Signed-off-by: Maxim Uvarov --- doc/develop/index.rst| 1 + doc/develop/net_lwip.rst | 59 2 files changed, 60 insertions(+) create mode 100644 doc/develop/net_lwip.rst diff --git a/doc/develop/index.rst b/doc/develop/index.rst index 97c526e997..a092c33df0 100644 --- a/doc/develop/index.rst +++ b/doc/develop/index.rst @@ -43,6 +43,7 @@ Implementation smbios spl uefi/index + net_lwip vbe version diff --git a/doc/develop/net_lwip.rst b/doc/develop/net_lwip.rst new file mode 100644 index 00..2520fe11b1 --- /dev/null +++ b/doc/develop/net_lwip.rst @@ -0,0 +1,59 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +LWIP IP stack intergation for U-boot + + +Intro +- + +LWIP is a library for implementation network protocols, which is commonly used +on embedded devices. + +https://savannah.nongnu.org/projects/lwip/ + +LwIP license: +LwIP is licensed under a BSD-style license: http://lwip.wikia.com/wiki/License. + +Main features include: + +* Protocols: IP, IPv6, ICMP, ND, MLD, UDP, TCP, IGMP, ARP, PPPoS, PPPoE + +* DHCP client, DNS client (incl. mDNS hostname resolver), AutoIP/APIPA (Zeroconf), SNMP agent (v1, v2c, v3, private MIB support & MIB compiler) + +* APIs: specialized APIs for enhanced performance, optional Berkeley-alike socket API + +* Extended features: IP forwarding over multiple network interfaces, TCP congestion control, RTT estimation and fast recovery/fast retransmit + +* Addon applications: HTTP(S) server, SNTP client, SMTP(S) client, ping, NetBIOS nameserver, mDNS responder, MQTT client, TFTP server + +U-boot implementation details +- + +1. In general we can build lwIP as .a library and link it against u-boot or compile it in +the U-boot tree in the same way as other U-boot files. There are few reasons why I selected +the second variant: LwIP is very customizable with defines for features, memory size, types of +allocation, some internal types and platform specific code. And it was more easy to enable/disable +debug which is also done with defines, and is needed periodically. + +2. lwIP has 2 APIs - raw mode and sequential (as lwIP names it, or socket API as we name it in Linux). +For now only raw API is supported. + +Raw IP means that the call back function for RX path is registered and will be called when packet +data passes the IP stack and is ready for the application. + +Example is unmodified working ping example from lwip sources which registeres the callback: + +.. code-block:: c + +ping_pcb = raw_new(IP_PROTO_ICMP); +raw_recv(ping_pcb, ping_recv, NULL); <- ping_recv is app callback. +raw_bind(ping_pcb, IP_ADDR_ANY) + +Socket API also gives nice advantages due it will be easy to port linux socket applications to u-boot. +I.e. LwIP sockets compatible with the linux ones. But that will require RX thread running in the background. +So that means we need some kind of scheduler, locking and threading support or find some other solution. + +3. Input and output + +RX packet path is injected to U-boot eth_rx() polling loop and TX patch is in eth_send() accordingly. +So we do not touch any drivers code and just eat packets when they are ready. -- 2.30.2
[PATCHv3 2/3] net/lwip: add lwip library for the network stack
This commit adds lwip library for the U-boot network
stack. Supported commands: ping, tftp, dhcp and wget.
Signed-off-by: Maxim Uvarov
---
.gitignore| 9 +
boot/bootmeth_pxe.c | 2 +-
cmd/net.c | 48 +
cmd/pxe.c | 2 +-
include/net.h | 8 +-
lib/Kconfig | 2 +
lib/Makefile | 2 +
lib/lwip/Kconfig | 63 ++
lib/lwip/Makefile | 98 ++
lib/lwip/apps/dhcp/lwip-dhcp.c| 52 +
lib/lwip/apps/http/lwip-wget.c| 71 +++
lib/lwip/apps/ping/lwip_ping.c| 37
lib/lwip/apps/ping/lwip_ping.h| 24 +++
lib/lwip/apps/ping/ping.h | 35
lib/lwip/apps/tftp/lwip-tftp.c| 121
lib/lwip/cmd-lwip.c | 269 ++
lib/lwip/lwipopts.h | 203 +++
lib/lwip/port/if.c| 260 +
lib/lwip/port/include/arch/cc.h | 46 +
lib/lwip/port/include/arch/sys_arch.h | 59 ++
lib/lwip/port/include/limits.h| 0
lib/lwip/port/sys-arch.c | 20 ++
lib/lwip/ulwip.h | 9 +
net/Kconfig | 1 +
net/net.c | 24 +++
25 files changed, 1421 insertions(+), 44 deletions(-)
create mode 100644 lib/lwip/Kconfig
create mode 100644 lib/lwip/Makefile
create mode 100644 lib/lwip/apps/dhcp/lwip-dhcp.c
create mode 100644 lib/lwip/apps/http/lwip-wget.c
create mode 100644 lib/lwip/apps/ping/lwip_ping.c
create mode 100644 lib/lwip/apps/ping/lwip_ping.h
create mode 100644 lib/lwip/apps/ping/ping.h
create mode 100644 lib/lwip/apps/tftp/lwip-tftp.c
create mode 100644 lib/lwip/cmd-lwip.c
create mode 100644 lib/lwip/lwipopts.h
create mode 100644 lib/lwip/port/if.c
create mode 100644 lib/lwip/port/include/arch/cc.h
create mode 100644 lib/lwip/port/include/arch/sys_arch.h
create mode 100644 lib/lwip/port/include/limits.h
create mode 100644 lib/lwip/port/sys-arch.c
create mode 100644 lib/lwip/ulwip.h
diff --git a/.gitignore b/.gitignore
index eb769f144c..be3676c59e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -104,3 +104,12 @@ __pycache__
# pylint files
/pylint.cur
/pylint.out/
+
+lib/lwip/lwip-external
+lib/lwip/apps/ping/ping.c
+lib/lwip/apps/http/http_client.c
+lib/lwip/apps/http/http_client.h
+lib/lwip/apps/tftp/tftp.c
+lib/lwip/apps/tftp/tftp_client.h
+lib/lwip/apps/tftp/tftp_common.h
+lib/lwip/apps/tftp/tftp_example.h
diff --git a/boot/bootmeth_pxe.c b/boot/bootmeth_pxe.c
index e6992168c0..30331a9806 100644
--- a/boot/bootmeth_pxe.c
+++ b/boot/bootmeth_pxe.c
@@ -118,7 +118,7 @@ static int distro_pxe_read_file(struct udevice *dev, struct
bootflow *bflow,
tftp_argv[1] = file_addr;
tftp_argv[2] = (void *)file_path;
- if (do_tftpb(ctx->cmdtp, 0, 3, tftp_argv))
+ if (do_lwip_tftp(ctx->cmdtp, 0, 3, tftp_argv))
return -ENOENT;
ret = pxe_get_file_size(&size);
if (ret)
diff --git a/cmd/net.c b/cmd/net.c
index 0e9f200ca9..6d704fba86 100644
--- a/cmd/net.c
+++ b/cmd/net.c
@@ -36,19 +36,9 @@ U_BOOT_CMD(
#endif
#ifdef CONFIG_CMD_TFTPBOOT
-int do_tftpb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
-{
- int ret;
-
- bootstage_mark_name(BOOTSTAGE_KERNELREAD_START, "tftp_start");
- ret = netboot_common(TFTPGET, cmdtp, argc, argv);
- bootstage_mark_name(BOOTSTAGE_KERNELREAD_STOP, "tftp_done");
- return ret;
-}
-
#if IS_ENABLED(CONFIG_IPV6)
U_BOOT_CMD(
- tftpboot, 4, 1, do_tftpb,
+ tftpboot, 4, 1, do_lwip_tftp,
"boot image via network using TFTP protocol\n"
"To use IPv6 add -ipv6 parameter or use IPv6 hostIPaddr framed "
"with [] brackets",
@@ -56,7 +46,7 @@ U_BOOT_CMD(
);
#else
U_BOOT_CMD(
- tftpboot, 3, 1, do_tftpb,
+ tftpboot, 3, 1, do_lwip_tftp,
"load file via network using TFTP protocol",
"[loadAddress] [[hostIPaddr:]bootfilename]"
);
@@ -112,7 +102,7 @@ U_BOOT_CMD(
static int do_dhcp(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
{
- return netboot_common(DHCP, cmdtp, argc, argv);
+ return do_lwip_dhcp();
}
U_BOOT_CMD(
@@ -137,13 +127,11 @@ U_BOOT_CMD(
#endif
#if defined(CONFIG_CMD_WGET)
-static int do_wget(struct cmd_tbl *cmdtp, int flag, int argc, char * const
argv[])
-{
- return netboot_common(WGET, cmdtp, argc, argv);
-}
+int do_lwip_wget(struct cmd_tbl *cmdtp, int flag, int argc,
+char *const argv[]);
U_BOOT_CMD(
- wget, 3, 1, do_wget,
+ wget, 3, 1, do_lwip_wget,
"boot image via network using HTTP protocol",
"[loadAddress] [[hostIPaddr:]path and i
[PATCHv3 1/3] net/lwip: add lwip-external submodule
This commit adds the lwip library as a git submodule. I think there has to be advantages to compile lwip inside U-boot, i.e. use the same compiler and flags as the main code. One of them is LTO and the other is to enable additional debug options for network protocol during development. Also we can copy lwip library code inside U-boot, but for now I don't want to send all lwip code to the mailing list. So it's git submodule. Signed-off-by: Maxim Uvarov --- .gitmodules| 3 +++ lib/lwip/lwip-external | 1 + 2 files changed, 4 insertions(+) create mode 100644 .gitmodules create mode 16 lib/lwip/lwip-external diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 00..afc709af10 --- /dev/null +++ b/.gitmodules @@ -0,0 +1,3 @@ +[submodule "lib/lwip/lwip-external"] + path = lib/lwip/lwip-external + url = https://git.savannah.nongnu.org/git/lwip.git diff --git a/lib/lwip/lwip-external b/lib/lwip/lwip-external new file mode 16 index 00..3fe8d2fc43 --- /dev/null +++ b/lib/lwip/lwip-external @@ -0,0 +1 @@ +Subproject commit 3fe8d2fc43a9b69f7ed28c63d44a7744f9c0def9 -- 2.30.2
[PATCHv3 0/3] net/lwip: add lwip library for the network stack
changelog: v3: - use lwip commands for ping,tftp,wget,dhcp if this patch applied. Drop CONFIG_LIB_LWIP_REPLACE_ option. - docs: use rst variant and drop references to RFC. build: git submodule init git submodule update make I tested with qemu and ubuntu host for the server. If we have more advance CI testing for networking it will be good to get results. Maxim Uvarov (3): net/lwip: add lwip-external submodule net/lwip: add lwip library for the network stack net/lwip: add doc/develop/net_lwip.rst .gitignore| 9 + .gitmodules | 3 + boot/bootmeth_pxe.c | 2 +- cmd/net.c | 48 + cmd/pxe.c | 2 +- doc/develop/index.rst | 1 + doc/develop/net_lwip.rst | 59 ++ include/net.h | 8 +- lib/Kconfig | 2 + lib/Makefile | 2 + lib/lwip/Kconfig | 63 ++ lib/lwip/Makefile | 98 ++ lib/lwip/apps/dhcp/lwip-dhcp.c| 52 + lib/lwip/apps/http/lwip-wget.c| 71 +++ lib/lwip/apps/ping/lwip_ping.c| 37 lib/lwip/apps/ping/lwip_ping.h| 24 +++ lib/lwip/apps/ping/ping.h | 35 lib/lwip/apps/tftp/lwip-tftp.c| 121 lib/lwip/cmd-lwip.c | 269 ++ lib/lwip/lwip-external| 1 + lib/lwip/lwipopts.h | 203 +++ lib/lwip/port/if.c| 260 + lib/lwip/port/include/arch/cc.h | 46 + lib/lwip/port/include/arch/sys_arch.h | 59 ++ lib/lwip/port/include/limits.h| 0 lib/lwip/port/sys-arch.c | 20 ++ lib/lwip/ulwip.h | 9 + net/Kconfig | 1 + net/net.c | 24 +++ 29 files changed, 1485 insertions(+), 44 deletions(-) create mode 100644 .gitmodules create mode 100644 doc/develop/net_lwip.rst create mode 100644 lib/lwip/Kconfig create mode 100644 lib/lwip/Makefile create mode 100644 lib/lwip/apps/dhcp/lwip-dhcp.c create mode 100644 lib/lwip/apps/http/lwip-wget.c create mode 100644 lib/lwip/apps/ping/lwip_ping.c create mode 100644 lib/lwip/apps/ping/lwip_ping.h create mode 100644 lib/lwip/apps/ping/ping.h create mode 100644 lib/lwip/apps/tftp/lwip-tftp.c create mode 100644 lib/lwip/cmd-lwip.c create mode 16 lib/lwip/lwip-external create mode 100644 lib/lwip/lwipopts.h create mode 100644 lib/lwip/port/if.c create mode 100644 lib/lwip/port/include/arch/cc.h create mode 100644 lib/lwip/port/include/arch/sys_arch.h create mode 100644 lib/lwip/port/include/limits.h create mode 100644 lib/lwip/port/sys-arch.c create mode 100644 lib/lwip/ulwip.h -- 2.30.2
[PATCH] powerpc: Fix flush_cache() speed regression
Flushing kernel image after decompression was taking 113 milliseconds
with U-boot 2022.10. With U-boot 2023.01 and 2023.04, flushing
the same amount of memory takes approx 1.5 seconds. With
U-boot 2023.07-rc6, it takes 6.5 seconds.
powerpc flush_cache() function used to call WATCHDOG_RESET() after
flushing every cacheline. At that time WATCHDOG_RESET() was light
so the operation was almost seamless.
But commit 29caf9305b6 ("cyclic: Use schedule() instead of
WATCHDOG_RESET()") replaced WATCHDOG_RESET() by schedule() and that
started to hurt with U-boot 2022.10.
And in U-boot 2023.07-rc6 that's even worse after
commit 26e8ebcd7cb ("watchdog: mpc8xxx: Make it generic").
In the meantime commit 729c1fe656 ("powerpc: introduce
CONFIG_CACHE_FLUSH_WATCHDOG_THRESHOLD") gives us the opportinity to
only call schedule() every given chunk of data instead of every
cacheline. As explained in that commit there is no point in pinging
the watchdog after every cacheline flush, so lets define a sensible
default chunk size of 4k which matches to size of a page on most
powerpc platforms.
With that new default threshold, the culprit flushing performed after
kernel image decompression now takes 85 milliseconds on a powerpc 8xx.
Fixes: 29caf9305b6 ("cyclic: Use schedule() instead of WATCHDOG_RESET()")
Signed-off-by: Christophe Leroy
---
arch/powerpc/lib/Kconfig | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/lib/Kconfig b/arch/powerpc/lib/Kconfig
index b30b5edf7c..d38ba45a99 100644
--- a/arch/powerpc/lib/Kconfig
+++ b/arch/powerpc/lib/Kconfig
@@ -1,9 +1,9 @@
config CACHE_FLUSH_WATCHDOG_THRESHOLD
- int "Bytes to flush between WATCHDOG_RESET calls"
- default 0
+ int "Bytes to flush between schedule() calls"
+ default 4096
help
The flush_cache() function periodically, and by default for
- every cache line, calls WATCHDOG_RESET(). When flushing a
- large area, that may add a significant amount of
+ every 4k block, calls schedule() to reset watchdog. When
+ flushing a large area, that may add a significant amount of
overhead. This option allows you to set a threshold for how
- many bytes to flush between each WATCHDOG_RESET call.
+ many bytes to flush between each schedule() call.
--
2.41.0
Re: [bug report] sunxi: H6: no ethernet on Orange Pi One Plus
Hi Anne, On Tue, Jul 4, 2023 at 8:52 PM Anne Macedo wrote: > > Hey! > > I'm trying to bake Linux images for the Orange Pi One Plus using Yocto. > Everything works fine, except for Ethernet. > > On the u-boot prompt: > > => dhcp > No ethernet found. > > After adding: > > CONFIG_SPL_SPI_SUNXI=y > CONFIG_SUN8I_EMAC=y > > to configs/orangepi_one_plus_defconfig, I started seeing this error: > > => dhcp > sun8i_emac_eth_start: Timeout > > I saw this other bug report but I couldn't really understand what has > been made to fix this issue [1]. > > More context here [2]. > > [1] https://lists.denx.de/pipermail/u-boot/2021-June/451357.html Does it help if you revert 4f0278dac56a658ef1e0967fec0bb95372a875bd ? I added on CC the folks involved in the previous report.
Re: EFI Secure boot default keys
>> Please can someone describe the format of the file needed for the default /
>> built-in EFI secure boot keys (ubootefi.var)
>>
>> The only docs I have found suggest its best to enroll the keys from within
>> u-boot onto some removable media, then copy this off and use this as the
>> default, this is not very helpful and doesn't work for me:
>>
>> => fatload mmc 0:1 ${loadaddr} PK.aut
>> 2053 bytes read in 18 ms (111.3 KiB/s)
>> => setenv -e -nv -bs -rt -at -i ${loadaddr}:$filesize PK
>> setenv - set environment variables
>>
>> Usage:
>> setenv setenv [-f] name value ...
>> - [forcibly] set environment variable 'name' to 'value ...'
>> setenv [-f] name
>> - [forcibly] delete environment variable 'name'
>>
>> my setenv doesn't support all the extra switches ? This is with 2022.04, all
>> other EFI options seem to be in this release and I can boot unsigned EFI
>> images ok.
>
>Please turn on CONFIG_CMD_NVEDIT_EFI when building your U-Boot.
>
>This option was disabled by the commit:
>commit 3b728f8728fa (tag: efi-2020-01-rc1)
>Author: Heinrich Schuchardt
>Date: Sun Oct 6 15:44:22 2019 +0200
>
>cmd: disable CMD_NVEDIT_EFI by default
>
>The binary size of efi has grown much since in the past, though.
>
>-Takahiro Akashi
Thanks, I have secure boot working now. A tool to generate the ubootefi.var
offline or even just a description of the file format would be very useful. I
have noticed one issue when using ubootefi.var on mmc, when I switch boot order
it wipes out the keys and I have to re-enrol them:
=> fatls mmc 0:1
3040 ubootefi.var
1 file(s), 0 dir(s)
=> efidebug boot order 2 1
=> fatls mmc 0:1
440 ubootefi.var
(Size drops from 3040 to 440 bytes and keys have gone)
From: AKASHI Takahiro
Sent: 29 June 2023 02:01
To: Neil Jones
Cc: u-boot@lists.denx.de
Subject: Re: EFI Secure boot default keys
On Wed, Jun 28, 2023 at 04:26:58PM +, Neil Jones wrote:
> Please can someone describe the format of the file needed for the default /
> built-in EFI secure boot keys (ubootefi.var)
>
> The only docs I have found suggest its best to enroll the keys from within
> u-boot onto some removable media, then copy this off and use this as the
> default, this is not very helpful and doesn't work for me:
>
> => fatload mmc 0:1 ${loadaddr} PK.aut
> 2053 bytes read in 18 ms (111.3 KiB/s)
> => setenv -e -nv -bs -rt -at -i ${loadaddr}:$filesize PK
> setenv - set environment variables
>
> Usage:
> setenv setenv [-f] name value ...
> - [forcibly] set environment variable 'name' to 'value ...'
> setenv [-f] name
> - [forcibly] delete environment variable 'name'
>
> my setenv doesn't support all the extra switches ? This is with 2022.04, all
> other EFI options seem to be in this release and I can boot unsigned EFI
> images ok.
Please turn on CONFIG_CMD_NVEDIT_EFI when building your U-Boot.
This option was disabled by the commit:
commit 3b728f8728fa (tag: efi-2020-01-rc1)
Author: Heinrich Schuchardt
Date: Sun Oct 6 15:44:22 2019 +0200
cmd: disable CMD_NVEDIT_EFI by default
The binary size of efi has grown much since in the past, though.
-Takahiro Akashi
> Cheers,
>
> Neil
>
>
>
Re: [GIT PULL] Please pull u-boot-pmic next
On Tue, Jul 04, 2023 at 04:25:55PM +0900, Jaehoon Chung wrote: > Dear Tom, > > Please pull u-boot-pmic next into u-boot next branch. > If there is a problem, let me know, plz. > > Best Regards, > Jaehoon Chung > > CI: https://source.denx.de/u-boot/custodians/u-boot-pmic/-/pipelines/16767 > > The following changes since commit 67d8b46e6efa306403e45f4c76f24b86a5e63b75: > > Merge tag 'u-boot-amlogic-next-20230628' of > https://source.denx.de/u-boot/custodians/u-boot-amlogic into next (2023-06-28 > 10:10:03 -0400) > > are available in the Git repository at: > > g...@source.denx.de:u-boot/custodians/u-boot-pmic.git next > > for you to fetch changes up to 8202bc29454cfdd5839058e4b79c36a3fbb221cf: > > regulator: handle different error codes in regulator_set_enable_if_allowed > (2023-07-04 11:21:12 +0900) > Applied to u-boot/next, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH V5 0/9] Update SPL splashscreen framework for AM62x
On Wed, Jul 05, 2023 at 06:33:43PM +0530, Nikhil M Jain wrote: > Hi Tom, > > Gentle reminder > > On 21/06/23 15:51, Nikhil M Jain wrote: > > This patch series aims at updating SPL splashscreen framework for AM62x. > > > > This patch series depends on > > https://lore.kernel.org/u-boot/20230504225829.2537050-1-...@chromium.org/ > > > > This series: > > - Fixes compilation issues in case splash related configs are not > >defined in SPL. > > - Does page table setup, dram initialisation and dcache enabling in > >one function call spl_enable_dcache. > > - Allows passing of framebuffer from spl to u-boot, eliminating flicker. > > > > V5: > > - Change A53 SPL DDR layout from ASCII table to tabular format. > > > > V4: > > - Fix commit message. > > - Introduce patch defining DDR layout in A53 SPL. > > - Add Reviewed-by tags. > > > > V3: > > - Fix spacing issues. > > - Add Reviewed-by tag. > > - Replace #if with if in patch > >common: spl: spl: Remove video driver > > - Add link to updated memory map. > > > > V2: > > - Update cover letter. > > - Fix commit message. > > > > Nikhil M Jain (9): > >common: spl: spl: Update stack pointer address > >arch: arm: mach-k3: common: Return a pointer after setting page table > >board: ti: am62x: evm: Update function calls for splash screen > >include: video: Reserve video using blob > >common: board_f: Pass frame buffer info from SPL to u-boot > >drivers: video: Kconfig: Add config remove video > >common: spl: spl: Remove video driver > >configs: am62x_evm_a53: Add bloblist address > >doc: board: ti: am62x_sk: Add A53 SPL DDR layout > > > > arch/arm/mach-k3/am625_init.c | 1 + > > arch/arm/mach-k3/common.c | 2 ++ > > board/ti/am62x/evm.c| 41 +--- > > common/board_f.c| 11 ++- > > common/spl/spl.c| 23 ++--- > > configs/am62x_evm_a53_defconfig | 1 + > > doc/board/ti/am62x_sk.rst | 57 + > > drivers/video/Kconfig | 12 +++ > > drivers/video/video-uclass.c| 23 + > > include/video.h | 9 ++ > > 10 files changed, 142 insertions(+), 38 deletions(-) Yes, as this is based on another series it's still blocked until that gets merged, right? Thanks. -- Tom signature.asc Description: PGP signature
[PATCH v2] fpga: add inline stub for fpga_load
In case CC_OPTIMIZE_FOR_DEBUG is set, unused code will not be optimized out,
hence the reference to fpga_load will be compiled.
if DM_FPGA and SPL_FPGA are not set, the build will fail with :
aarch64-none-linux-gnu-ld.bfd: common/spl/spl_fit.o: in function
`spl_fit_upload_fpga':
u-boot/common/spl/spl_fit.c:595: undefined reference to `fpga_load'
To solve this, added an inline empty stub in the header if
CC_OPTIMIZE_FOR_DEBUG is set such that the build will succeed.
Signed-off-by: Eugen Hristev
---
Changes in v2:
- this is a rework as suggested by Simon of this previous patch :
https://patchwork.ozlabs.org/project/uboot/patch/20230619102839.277902-1-eugen.hris...@collabora.com/
include/fpga.h | 9 +
1 file changed, 9 insertions(+)
diff --git a/include/fpga.h b/include/fpga.h
index ed688cc0fa3b..50ab03af06bb 100644
--- a/include/fpga.h
+++ b/include/fpga.h
@@ -60,8 +60,17 @@ int fpga_add(fpga_type devtype, void *desc);
int fpga_count(void);
const fpga_desc *const fpga_get_desc(int devnum);
int fpga_is_partial_data(int devnum, size_t img_len);
+#if defined(CONFIG_CC_OPTIMIZE_FOR_DEBUG)
+static inline int fpga_load(int devnum, const void *buf, size_t bsize,
+ bitstream_type bstype, int flags)
+{
+ return 0;
+}
+#else
int fpga_load(int devnum, const void *buf, size_t bsize,
bitstream_type bstype, int flags);
+#endif
+
int fpga_fsload(int devnum, const void *buf, size_t size,
fpga_fs_info *fpga_fsinfo);
int fpga_loads(int devnum, const void *buf, size_t size,
--
2.34.1
Re: [PATCH V5 0/9] Update SPL splashscreen framework for AM62x
Hi Tom, Gentle reminder On 21/06/23 15:51, Nikhil M Jain wrote: This patch series aims at updating SPL splashscreen framework for AM62x. This patch series depends on https://lore.kernel.org/u-boot/20230504225829.2537050-1-...@chromium.org/ This series: - Fixes compilation issues in case splash related configs are not defined in SPL. - Does page table setup, dram initialisation and dcache enabling in one function call spl_enable_dcache. - Allows passing of framebuffer from spl to u-boot, eliminating flicker. V5: - Change A53 SPL DDR layout from ASCII table to tabular format. V4: - Fix commit message. - Introduce patch defining DDR layout in A53 SPL. - Add Reviewed-by tags. V3: - Fix spacing issues. - Add Reviewed-by tag. - Replace #if with if in patch common: spl: spl: Remove video driver - Add link to updated memory map. V2: - Update cover letter. - Fix commit message. Nikhil M Jain (9): common: spl: spl: Update stack pointer address arch: arm: mach-k3: common: Return a pointer after setting page table board: ti: am62x: evm: Update function calls for splash screen include: video: Reserve video using blob common: board_f: Pass frame buffer info from SPL to u-boot drivers: video: Kconfig: Add config remove video common: spl: spl: Remove video driver configs: am62x_evm_a53: Add bloblist address doc: board: ti: am62x_sk: Add A53 SPL DDR layout arch/arm/mach-k3/am625_init.c | 1 + arch/arm/mach-k3/common.c | 2 ++ board/ti/am62x/evm.c| 41 +--- common/board_f.c| 11 ++- common/spl/spl.c| 23 ++--- configs/am62x_evm_a53_defconfig | 1 + doc/board/ti/am62x_sk.rst | 57 + drivers/video/Kconfig | 12 +++ drivers/video/video-uclass.c| 23 + include/video.h | 9 ++ 10 files changed, 142 insertions(+), 38 deletions(-) Thanks Nikhil
[PATCH 2/2] imx93_evk: defconfig: add adc support
iMX93 ADC features: - 4 channels - 12 bit resolution Signed-off-by: Luca Ellero --- configs/imx93_11x11_evk_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/configs/imx93_11x11_evk_defconfig b/configs/imx93_11x11_evk_defconfig index 89edebc4c6..30ef460c80 100644 --- a/configs/imx93_11x11_evk_defconfig +++ b/configs/imx93_11x11_evk_defconfig @@ -81,6 +81,7 @@ CONFIG_NET_RANDOM_ETHADDR=y CONFIG_SPL_DM=y CONFIG_REGMAP=y CONFIG_SYSCON=y +CONFIG_ADC_IMX93=y CONFIG_CPU=y CONFIG_CPU_IMX=y CONFIG_IMX_RGPIO2P=y -- 2.25.1
[PATCH 1/2] dm: adc: add iMX93 ADC support
This commit adds driver for iMX93 ADC.
The driver is implemented using driver model and provides
ADC uclass's methods for ADC single channel operations:
- adc_start_channel()
- adc_channel_data()
- adc_stop()
ADC features:
- channels: 4
- resolution: 12-bit
Signed-off-by: Luca Ellero
---
drivers/adc/Kconfig | 8 ++
drivers/adc/Makefile| 1 +
drivers/adc/imx93-adc.c | 290
3 files changed, 299 insertions(+)
create mode 100644 drivers/adc/imx93-adc.c
diff --git a/drivers/adc/Kconfig b/drivers/adc/Kconfig
index e719c38bb3..4336732dee 100644
--- a/drivers/adc/Kconfig
+++ b/drivers/adc/Kconfig
@@ -63,3 +63,11 @@ config STM32_ADC
- core driver to deal with common resources
- child driver to deal with individual ADC resources (declare ADC
device and associated channels, start/stop conversions)
+
+config ADC_IMX93
+ bool "Enable NXP IMX93 ADC driver"
+ help
+ This enables basic driver for NXP IMX93 ADC.
+ It provides:
+ - 4 analog input channels
+ - 12-bit resolution
diff --git a/drivers/adc/Makefile b/drivers/adc/Makefile
index c1387f3a34..5336c82097 100644
--- a/drivers/adc/Makefile
+++ b/drivers/adc/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_ADC_SANDBOX) += sandbox.o
obj-$(CONFIG_SARADC_ROCKCHIP) += rockchip-saradc.o
obj-$(CONFIG_SARADC_MESON) += meson-saradc.o
obj-$(CONFIG_STM32_ADC) += stm32-adc.o stm32-adc-core.o
+obj-$(CONFIG_ADC_IMX93) += imx93-adc.o
diff --git a/drivers/adc/imx93-adc.c b/drivers/adc/imx93-adc.c
new file mode 100644
index 00..41d04e0426
--- /dev/null
+++ b/drivers/adc/imx93-adc.c
@@ -0,0 +1,290 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2023 ASEM Srl
+ * Author: Luca Ellero
+ *
+ * Originally based on NXP linux-imx kernel v5.15 drivers/iio/adc/imx93_adc.c
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define IMX93_ADC_MCR 0x00
+#define IMX93_ADC_MSR 0x04
+#define IMX93_ADC_ISR 0x10
+#define IMX93_ADC_IMR 0x20
+#define IMX93_ADC_CIMR00x24
+#define IMX93_ADC_CTR0 0x94
+#define IMX93_ADC_NCMR00xA4
+#define IMX93_ADC_PCDR00x100
+#define IMX93_ADC_PCDR10x104
+#define IMX93_ADC_PCDR20x108
+#define IMX93_ADC_PCDR30x10c
+#define IMX93_ADC_PCDR40x110
+#define IMX93_ADC_PCDR50x114
+#define IMX93_ADC_PCDR60x118
+#define IMX93_ADC_PCDR70x11c
+#define IMX93_ADC_CALSTAT 0x39C
+
+#define IMX93_ADC_MCR_MODE_MASKBIT(29)
+#define IMX93_ADC_MCR_NSTART_MASK BIT(24)
+#define IMX93_ADC_MCR_CALSTART_MASKBIT(14)
+#define IMX93_ADC_MCR_ADCLKSE_MASK BIT(8)
+#define IMX93_ADC_MCR_PWDN_MASKBIT(0)
+
+#define IMX93_ADC_MSR_CALFAIL_MASK BIT(30)
+#define IMX93_ADC_MSR_CALBUSY_MASK BIT(29)
+#define IMX93_ADC_MSR_ADCSTATUS_MASK GENMASK(2, 0)
+
+#define IMX93_ADC_ISR_EOC_MASK BIT(1)
+
+#define IMX93_ADC_IMR_EOC_MASK BIT(1)
+#define IMX93_ADC_IMR_ECH_MASK BIT(0)
+
+#define IMX93_ADC_PCDR_CDATA_MASK GENMASK(11, 0)
+
+#define IDLE 0
+#define POWER_DOWN 1
+#define WAIT_STATE 2
+#define BUSY_IN_CALIBRATION3
+#define SAMPLE 4
+#define CONVERSION 6
+
+#define IMX93_ADC_MAX_CHANNEL 3
+#define IMX93_ADC_DAT_MASK 0xfff
+#define IMX93_ADC_TIMEOUT 10
+
+struct imx93_adc_priv {
+ int active_channel;
+ void __iomem *regs;
+ struct clk ipg_clk;
+};
+
+static void imx93_adc_power_down(struct imx93_adc_priv *adc)
+{
+ u32 mcr, msr;
+ int ret;
+
+ mcr = readl(adc->regs + IMX93_ADC_MCR);
+ mcr |= FIELD_PREP(IMX93_ADC_MCR_PWDN_MASK, 1);
+ writel(mcr, adc->regs + IMX93_ADC_MCR);
+
+ ret = readl_poll_timeout(adc->regs + IMX93_ADC_MSR, msr,
+ ((msr & IMX93_ADC_MSR_ADCSTATUS_MASK) == POWER_DOWN), 50);
+ if (ret == -ETIMEDOUT)
+ pr_warn("ADC not in power down mode, current MSR: %x\n", msr);
+}
+
+static void imx93_adc_power_up(struct imx93_adc_priv *adc)
+{
+ u32 mcr;
+
+ /* bring ADC out of power down state, in idle state */
+ mcr = readl(adc->regs + IMX93_ADC_MCR);
+ mcr &= ~FIELD_PREP(IMX93_ADC_MCR_PWDN_MASK, 1);
+ writel(mcr, adc->regs + IMX93_ADC_MCR);
+}
+
+static void imx93_adc_config_ad_clk(struct imx93_adc_priv *adc)
+{
+ u32 mcr;
+
+ /* put adc in power down mode */
+ imx93_adc_power_down(adc);
+
+ /* config the AD_CLK equal to bus clock */
+ mcr = readl(adc->regs + IMX93_ADC_MCR);
+ mcr |= FIELD_P
[PATCH v5 0/2 RESEND] imx93: add ADC support
Add ADC support for NXP iMX93 Changes for v2: - add "static" to functions - enable ADC in iMX93 EVK Changes for v3: - split in 3 commits - keep dts file in sync with Linux devicetree - add comments to commits Changes for v4: - add imx93_adc_power_down() in imx93_adc_stop() Changes for v5: - simplify code - remove redundant code - add clock handling Luca Ellero (2): dm: adc: add iMX93 ADC support imx93_evk: defconfig: add adc support configs/imx93_11x11_evk_defconfig | 1 + drivers/adc/Kconfig | 8 + drivers/adc/Makefile | 1 + drivers/adc/imx93-adc.c | 290 ++ 4 files changed, 300 insertions(+) create mode 100644 drivers/adc/imx93-adc.c -- 2.25.1
Re: [bug report] sunxi: H6: no ethernet on Orange Pi One Plus
On Wed, Jul 05, 2023 at 12:08:30PM +, Anne Macedo wrote: > On Wed, Jul 05, 2023 at 11:26:45AM +0100, Christopher Obbard wrote: > > Hi Anne, > > > > [please don't forget to CC the list, in case someone else > > has the same issue in future they don't miss the context ;-)] > > > > On Wed, 2023-07-05 at 10:12 +, Anne Macedo wrote: > > > On Wed, Jul 05, 2023 at 09:23:35AM +0100, Christopher Obbard wrote: > > > > Hi Anne, > > > > > > > > On Tue, 2023-07-04 at 23:22 +, Anne Macedo wrote: > > > > > Hey! > > > > > > > > > > I'm trying to bake Linux images for the Orange Pi One Plus using > > > > > Yocto. > > > > > Everything works fine, except for Ethernet. > > > > > > > > > > On the u-boot prompt: > > > > > > > > > > => dhcp > > > > > No ethernet found. > > > > > > > > > > After adding: > > > > > > > > > > CONFIG_SPL_SPI_SUNXI=y > > > > > CONFIG_SUN8I_EMAC=y > > > > > > > > > > to configs/orangepi_one_plus_defconfig, I started seeing this error: > > > > > > > > > > => dhcp > > > > > sun8i_emac_eth_start: Timeout > > > > > > > > Can you attempt to change the TF-A target from sun50i_h6 to > > > > sun50i_h6_no_pmic? > > > > > > Hello Christopher! I tried this, but it failed. It doesn't seem that > > > there's any PLAT for sun50i_h6_no_pmic on TF-A: > > > > > > https://github.com/ARM-software/arm-trusted-firmware/tree/master/plat/allwinner > > > > > > This is the error I get: > > > > > > "Error: Invalid platform. The following platforms are available: > > > a3700|a5ds|a70x0|a70x0_amc|a70x0_mochabin|a80x0|a80x0_mcbin|a80x0_puzzle|agilex|arm_fpga|axg|corstone1000|corstone700|fvp|fvp_r|fvp_ve|g12a|gxbb|gxl|hikey|hikey960|imx8mm|imx8mn|imx8mp|imx8mq|imx8qm > > > |imx8qx|juno|k3|ls1028ardb|ls1043ardb|ls1046afrwy|ls1046aqds|ls1046ardb|ls1088aqds|ls1088ardb|lx2160aqds|lx2160ardb|lx2162aqds|morello|msm8916|mt8173|mt8183|mt8186|mt8188|mt8192|mt8195|n1sdp|n5x|pic > > > opi|poplar|px30|qemu|qemu_sbsa|rcar|rde1edge|rdn1edge|rdn2|rdv1|rdv1mc|rk3288|rk3328|rk3368|rk3399|rpi3|rpi4|rzg|sc7180|sc7280|sgi575|stingray|stm32mp1|stratix10|sun50i_a64|sun50i_h6|sun50i_h616|sun > > > 50i_r329|synquacer|t9130|t9130_cex7_eval|tc|tegra|uniphier|versal|versal_net|warp7|zynqmp" > > > > > > > > > > I also tried adding CONFIG_SUNXI_NO_PMIC=y but it also didn't work :( > > > > > > I didn't find the sun50i_h6_no_pmic platform being defined anywhere... > > > https://packages.debian.org/bookworm/arm-trusted-firmware > > > > > > Wow, this is very recent! > > > https://www.mail-archive.com/debian-bugs-dist@lists.debian.org/msg1911472.html. > > > > > > The change was merged a few days ago. > > > > Right, looking at the packaging once more, it seems like this is a custom > > target > > just for Debian which sets SUNXI_SETUP_REGULATORS=0 > > (Forgot to reply all on the previous message, thanks for replying :) > Okay, so that seems to be mentioned on the docs and configs: > > https://github.com/ARM-software/arm-trusted-firmware/blob/master/docs/plat/allwinner.rst > https://linux-sunxi.org/Xunlong_Orange_Pi_3#Tips.2C_Tricks.2C_Caveats > > I tried it using Yocto, and it generated this (redacted) make: > > make -j 12 LD=aarch64-poky-linux-ld CC=aarch64-poky-linux-gcc V=1 E=0 > PLAT=sun50i_h6 SUNXI_SETUP_REGULATORS=0 bl31 > > But still: > > => dhcp > sun8i_emac_eth_start: Timeout > Okay, I verified that I successfully disabled PMIC, but I still get the error. (I will send a patch afterwards to meta-sunxi adding this information btw). make -j 12 LD=aarch64-poky-linux-ld CC=aarch64-poky-linux-gcc V=1 E=0 PLAT=sun50i_h6 LOG_LEVEL=40 bl31 U-Boot SPL 2023.04-gfd4ed6b (Apr 03 2023 - 20:38:50 +) DRAM: 1024 MiB Trying to boot from MMC1 NOTICE: BL31: lts-v2.8.6(release):lts-v2.8.6-dirty NOTICE: BL31: Built : 17:57:15, Apr 21 2023 NOTICE: BL31: Detected Allwinner H6 SoC (1728) NOTICE: BL31: Found U-Boot DTB at 0xa095670, model: OrangePi One Plus INFO:ARM GICv2 driver initialized INFO:Configuring SPC Controller INFO:PMIC: Probing AXP805 on RSB INFO:PMIC: aldo1 voltage: 3.300V INFO:PMIC: aldo2 voltage: 3.300V INFO:PMIC: aldo3 voltage: 3.300V INFO:PMIC: bldo1 voltage: 1.800V INFO:PMIC: bldo2 voltage: 1.800V INFO:PMIC: bldo3 voltage: 1.800V INFO:PMIC: cldo1 voltage: 3.300V INFO:PMIC: cldo2 voltage: 3.300V INFO:PMIC: cldo3 voltage: 3.300V INFO:PMIC: dcdcd voltage: 0.960V INFO:PMIC: dcdce voltage: 1.200V INFO:BL31: Platform setup done make -j 12 LD=aarch64-poky-linux-ld CC=aarch64-poky-linux-gcc V=1 E=0 PLAT=sun50i_h6 SUNXI_SETUP_REGULATORS=0 LOG_LEVEL=40 bl31 U-Boot SPL 2023.04-gfd4ed6b (Apr 03 2023 - 20:38:50 +) DRAM: 1024 MiB Trying to boot from MMC1 NOTICE: BL31: lts-v2.8.6(release):lts-v2.8.6-dirty NOTICE: BL31: Built : 17:57:15, Apr 21 2023 NOTICE: BL31: Detected Allwinner H6 SoC (1728) NOTICE: BL31: Found U-Boot DTB at 0xa095670, model: OrangePi One Plus INFO:ARM GICv2 driver initialized INFO:Configuring SPC Controller INFO:PMI
[PATCH v2 60/60] buildman: Enable test coverage
Enable measuring test coverage for buildman so we can see the gaps. It is
currently at 68%.
Signed-off-by: Simon Glass
---
Changes in v2:
- Drop patch to move -A logic up a little (since it breaks it)
tools/buildman/cmdline.py | 2 ++
tools/buildman/main.py | 15 ++-
tools/u_boot_pylib/test_util.py | 10 +++---
3 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index 41384840427a..983165546ea2 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -136,6 +136,8 @@ def add_after_m(parser):
help='Skip tests which need the network')
parser.add_argument('-t', '--test', action='store_true', dest='test',
default=False, help='run tests')
+parser.add_argument('--coverage', action='store_true',
+help='Calculated test coverage')
parser.add_argument('-T', '--threads', type=int,
default=None,
help='Number of builder threads to use (0=single-thread)')
diff --git a/tools/buildman/main.py b/tools/buildman/main.py
index 593911353101..cb3d0b4fc663 100755
--- a/tools/buildman/main.py
+++ b/tools/buildman/main.py
@@ -47,12 +47,22 @@ def run_tests(skip_net_tests, verbose, args):
# Run the entry tests first ,since these need to be the first to import the
# 'entry' module.
result = test_util.run_test_suites(
-'buildman', False, verbose, False, None, test_name, [],
+'buildman', False, verbose, False, args.threads, test_name, [],
[test.TestBuild, func_test.TestFunctional,
'buildman.toolchain', 'patman.gitutil'])
return (0 if result.wasSuccessful() else 1)
+def run_test_coverage():
+"""Run the tests and check that we get 100% coverage"""
+test_util.run_test_coverage(
+'tools/buildman/buildman', None,
+['tools/patman/*.py', 'tools/u_boot_pylib/*', '*test_fdt.py',
+ 'tools/buildman/kconfiglib.py', 'tools/buildman/*test*.py',
+ 'tools/buildman/main.py'],
+'/tmp/b', single_thread='-T1')
+
+
def run_buildman():
"""Run bulidman
@@ -68,6 +78,9 @@ def run_buildman():
if cmdline.HAS_TESTS and args.test:
run_tests(args.skip_net_tests, args.verbose, args)
+elif cmdline.HAS_TESTS and args.coverage:
+run_test_coverage()
+
elif args.full_help:
tools.print_full_help(str(files('buildman').joinpath('README.rst')))
diff --git a/tools/u_boot_pylib/test_util.py b/tools/u_boot_pylib/test_util.py
index e7564e10c997..f18d385d9951 100644
--- a/tools/u_boot_pylib/test_util.py
+++ b/tools/u_boot_pylib/test_util.py
@@ -24,7 +24,7 @@ except:
def run_test_coverage(prog, filter_fname, exclude_list, build_dir,
required=None,
-extra_args=None):
+extra_args=None, single_thread='-P1'):
"""Run tests and check that we get 100% coverage
Args:
@@ -39,6 +39,9 @@ def run_test_coverage(prog, filter_fname, exclude_list,
build_dir, required=None
required: List of modules which must be in the coverage report
extra_args (str): Extra arguments to pass to the tool before the
-t/test
arg
+single_thread (str): Argument string to make the tests run
+single-threaded. This is necessary to get proper coverage results.
+The default is '-P0'
Raises:
ValueError if the code coverage is not 100%
@@ -58,8 +61,9 @@ def run_test_coverage(prog, filter_fname, exclude_list,
build_dir, required=None
if build_dir:
prefix = 'PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools ' % build_dir
cmd = ('%spython3-coverage run '
- '--omit "%s" %s %s %s -P1' % (prefix, ','.join(glob_list),
- prog, extra_args or '', test_cmd))
+ '--omit "%s" %s %s %s %s' % (prefix, ','.join(glob_list),
+prog, extra_args or '', test_cmd,
+single_thread or '-P1'))
os.system(cmd)
stdout = command.output('python3-coverage', 'report')
lines = stdout.splitlines()
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 59/60] buildman: Add a way to print the architecture for a board
This is useful for some tools and is easily available for buildman. Add
a new --print-arch option.
Signed-off-by: Simon Glass
---
Changes in v2:
- Add new patch to print the architecture for a board
tools/buildman/cmdline.py | 2 ++
tools/buildman/control.py | 24
tools/buildman/func_test.py | 7 +++
3 files changed, 33 insertions(+)
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index e295c7aef1a0..41384840427a 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -119,6 +119,8 @@ def add_after_m(parser):
default=False, help="Use full toolchain path in CROSS_COMPILE")
parser.add_argument('-P', '--per-board-out-dir', action='store_true',
default=False, help="Use an O= (output) directory per board rather
than per thread")
+parser.add_argument('--print-arch', action='store_true',
+ default=False, help="Print the architecture for a board (ARCH=)")
parser.add_argument('-r', '--reproducible-builds', action='store_true',
help='Set SOURCE_DATE_EPOCH=0 to suuport a reproducible build')
parser.add_argument('-R', '--regen-board-list', type=str,
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 4306d4eb2a7c..8a29534d602f 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -140,6 +140,26 @@ def show_toolchain_prefix(brds, toolchains):
tchain = tc_set.pop()
print(tchain.GetEnvArgs(toolchain.VAR_CROSS_COMPILE))
+def show_arch(brds):
+"""Show information about a the architecture used by one or more boards
+
+The function checks that all boards use the same architecture, then prints
+the correct value for ARCH.
+
+Args:
+boards: Boards object containing selected boards
+
+Return:
+None on success, string error message otherwise
+"""
+board_selected = brds.get_selected_dict()
+arch_set = set()
+for brd in board_selected.values():
+arch_set.add(brd.arch)
+if len(arch_set) != 1:
+sys.exit('Supplied boards must share one arch')
+print(arch_set.pop())
+
def get_allow_missing(opt_allow, opt_no_allow, num_selected, has_branch):
"""Figure out whether to allow external blobs
@@ -597,6 +617,10 @@ def do_buildman(args, toolchains=None, make_func=None,
brds=None,
show_toolchain_prefix(brds, toolchains)
return 0
+if args.print_arch:
+show_arch(brds)
+return 0
+
series = determine_series(selected, col, git_dir, args.count,
args.branch, args.work_in_output)
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index aa0838538d23..d89bde1d95e2 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -812,3 +812,10 @@ CONFIG_LOCALVERSION=y
self._RunControl('--boards', 'board1,board2', '--boards', 'board4')
self.assertEqual(3, self._builder.count)
+
+def test_print_arch(self):
+"""Test that we can print the board architecture"""
+with test_util.capture_sys_output() as (stdout, stderr):
+result = self._RunControl('--print-arch', 'board0')
+self.assertEqual('arm\n', stdout.getvalue())
+self.assertEqual('', stderr.getvalue())
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 58/60] buildman: Move copy_files() out ot BuilderThread class
This does not need to be in the class. Move it out to avoid a pylint
warning.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/builderthread.py | 47 +
1 file changed, 24 insertions(+), 23 deletions(-)
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index ed84a0f6baa3..25f460c207db 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -64,6 +64,27 @@ def _remove_old_outputs(out_dir):
os.remove(fname)
+def copy_files(out_dir, build_dir, dirname, patterns):
+"""Copy files from the build directory to the output.
+
+Args:
+out_dir (str): Path to output directory containing the files
+build_dir (str): Place to copy the files
+dirname (str): Source directory, '' for normal U-Boot, 'spl' for SPL
+patterns (list of str): A list of filenames to copy, each relative
+ to the build directory
+"""
+for pattern in patterns:
+file_list = glob.glob(os.path.join(out_dir, dirname, pattern))
+for fname in file_list:
+target = os.path.basename(fname)
+if dirname:
+base, ext = os.path.splitext(target)
+if ext:
+target = f'{base}-{dirname}{ext}'
+shutil.copy(fname, os.path.join(build_dir, target))
+
+
# pylint: disable=R0903
class BuilderJob:
"""Holds information about a job to be performed by a thread
@@ -592,7 +613,7 @@ class BuilderThread(threading.Thread):
capture_stderr=True, cwd=result.out_dir,
raise_on_error=False, env=env)
if not work_in_output:
-self.copy_files(result.out_dir, build_dir, '', ['uboot.env'])
+copy_files(result.out_dir, build_dir, '', ['uboot.env'])
# Write out the image sizes file. This is similar to the output
# of binutil's 'size' utility, but it omits the header line and
@@ -607,7 +628,7 @@ class BuilderThread(threading.Thread):
if not work_in_output:
# Write out the configuration files, with a special case for SPL
for dirname in ['', 'spl', 'tpl']:
-self.copy_files(
+copy_files(
result.out_dir, build_dir, dirname,
['u-boot.cfg', 'spl/u-boot-spl.cfg', 'tpl/u-boot-tpl.cfg',
'.config', 'include/autoconf.mk',
@@ -615,31 +636,11 @@ class BuilderThread(threading.Thread):
# Now write the actual build output
if keep_outputs:
-self.copy_files(
+copy_files(
result.out_dir, build_dir, '',
['u-boot*', '*.bin', '*.map', '*.img', 'MLO', 'SPL',
'include/autoconf.mk', 'spl/u-boot-spl*'])
-def copy_files(self, out_dir, build_dir, dirname, patterns):
-"""Copy files from the build directory to the output.
-
-Args:
-out_dir (str): Path to output directory containing the files
-build_dir (str): Place to copy the files
-dirname (str): Source directory, '' for normal U-Boot, 'spl' for
SPL
-patterns (list of str): A list of filenames to copy, each relative
- to the build directory
-"""
-for pattern in patterns:
-file_list = glob.glob(os.path.join(out_dir, dirname, pattern))
-for fname in file_list:
-target = os.path.basename(fname)
-if dirname:
-base, ext = os.path.splitext(target)
-if ext:
-target = f'{base}-{dirname}{ext}'
-shutil.copy(fname, os.path.join(build_dir, target))
-
def _send_result(self, result):
"""Send a result to the builder for processing
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 57/60] buildman: Tidy up some comments in builderthread
Make sure all functions have full argument documentation. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/builderthread.py | 66 ++--- 1 file changed, 36 insertions(+), 30 deletions(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index 043e92b6d92d..ed84a0f6baa3 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -23,11 +23,15 @@ from u_boot_pylib import command RETURN_CODE_RETRY = -1 BASE_ELF_FILENAMES = ['u-boot', 'spl/u-boot-spl', 'tpl/u-boot-tpl'] -def mkdir(dirname, parents = False): +def mkdir(dirname, parents=False): """Make a directory if it doesn't already exist. Args: -dirname: Directory to create +dirname (str): Directory to create +parents (bool): True to also make parent directories + +Raises: +OSError: File already exists """ try: if parents: @@ -141,14 +145,16 @@ class BuilderThread(threading.Thread): argument is only for information. Args: -commit: Commit object that is being built -brd: Board object that is being built -stage: Stage of the build. Valid stages are: +commit (Commit): Commit that is being built +brd (Board): Board that is being built +stage (str): Stage of the build. Valid stages are: mrproper - can be called to clean source config - called to configure for a board build - the main make invocation - it does the build -args: A list of arguments to pass to 'make' -kwargs: A list of keyword arguments to pass to command.run_pipe() +cwd (str): Working directory to set, or None to leave it alone +*args (list of str): Arguments to pass to 'make' +**kwargs (dict): A list of keyword arguments to pass to +command.run_pipe() Returns: CommandResult object @@ -418,16 +424,16 @@ class BuilderThread(threading.Thread): the build and just return the previously-saved results. Args: -commit_upto: Commit number to build (0...n-1) -brd: Board object to build -work_dir: Directory to which the source will be checked out -do_config: True to run a make _defconfig on the source -config_only: Only configure the source, do not build it -force_build: Force a build even if one was previously done -force_build_failures: Force a bulid if the previous result showed -failure -work_in_output: Use the output directory as the work directory and -don't write to a separate output directory. +commit_upto (int): Commit number to build (0...n-1) +brd (Board): Board to build +work_dir (str): Directory to which the source will be checked out +do_config (bool): True to run a make _defconfig on the source +config_only (bool): Only configure the source, do not build it +force_build (bool): Force a build even if one was previously done +force_build_failures (bool): Force a bulid if the previous result +showed failure +work_in_output (bool) : Use the output directory as the work +directory and don't write to a separate output directory. adjust_cfg (list of str): List of changes to make to .config file before building. Each is one of (where C is either CONFIG_xxx or just xxx): @@ -476,11 +482,11 @@ class BuilderThread(threading.Thread): """Write a built result to the output directory. Args: -result: CommandResult object containing result to write -keep_outputs: True to store the output binaries, False +result (CommandResult): result to write +keep_outputs (bool): True to store the output binaries, False to delete them -work_in_output: Use the output directory as the work directory and -don't write to a separate output directory. +work_in_output (bool): Use the output directory as the work +directory and don't write to a separate output directory. """ # If we think this might have been aborted with Ctrl-C, record the # failure but not that we are 'done' with this board. A retry may fix @@ -618,10 +624,10 @@ class BuilderThread(threading.Thread): """Copy files from the build directory to the output. Args: -out_dir: Path to output directory containing the files -build_dir: Place to copy the files -dirname: Source directory, '' for normal U-Boot, 'spl' for SPL -patterns: A list of filenames (strings) to copy, each relative +
[PATCH v2 56/60] buildman: Tidy up reporting of a toolchain error
Provide the text of the exception when something goes wrong.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/builderthread.py | 7 +--
tools/buildman/func_test.py | 6 --
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index d3912390bc4f..043e92b6d92d 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -457,18 +457,13 @@ class BuilderThread(threading.Thread):
except ValueError as err:
result.return_code = 10
result.stdout = ''
-result.stderr = str(err)
-# TODO(s...@chromium.org): This gets swallowed, but needs
-# to be reported.
+result.stderr = f'Tool chain error for {brd.arch}:
{str(err)}'
if self.toolchain:
commit = self._checkout(commit_upto, work_dir)
result, do_config = self._config_and_build(
commit_upto, brd, work_dir, do_config, config_only,
adjust_cfg, commit, out_dir, out_rel_dir, result)
-else:
-result.return_code = 1
-result.stderr = f'No tool chain for {brd.arch}\n'
result.already_done = False
result.toolchain = self.toolchain
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index f5058559b4fa..aa0838538d23 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -503,8 +503,10 @@ Some images are invalid'''
if brd.arch != 'sandbox':
errfile = self._builder.get_err_file(commit, brd.target)
fd = open(errfile)
- self.assertEqual(fd.readlines(),
- ['No tool chain for %s\n' % brd.arch])
+ self.assertEqual(
+ fd.readlines(),
+ [f'Tool chain error for {brd.arch}: '
+ f"No tool chain found for arch '{brd.arch}'"])
fd.close()
def testBranch(self):
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 55/60] buildman: Avoid passing result into _read_done_file()
Move the creating of the result object into the function which sets it up, to simplify the code. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/builderthread.py | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index aa4a9d9919c6..d3912390bc4f 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -262,21 +262,26 @@ class BuilderThread(threading.Thread): result.return_code = 0 return result -def _read_done_file(self, commit_upto, brd, result, force_build, +def _read_done_file(self, commit_upto, brd, force_build, force_build_failures): """Check the 'done' file and see if this commit should be built Args: commit (Commit): Commit only being built brd (Board): Board being built -result (CommandResult): result object to update force_build (bool): Force a build even if one was previously done force_build_failures (bool): Force a bulid if the previous result showed failure Returns: -bool: True if build should be built +tuple: +bool: True if build should be built +CommandResult: if there was a previous run: +- already_done set to True +- return_code set to return code +- result.stderr set to 'bad' if stderr output was recorded """ +result = command.CommandResult() done_file = self.builder.get_done_file(commit_upto, brd.target) result.already_done = os.path.exists(done_file) will_build = (force_build or force_build_failures or @@ -300,7 +305,7 @@ class BuilderThread(threading.Thread): elif not force_build: # The build passed, so no need to build it again will_build = False -return will_build +return will_build, result def _decide_dirs(self, brd, work_dir, work_in_output): """Decide the output directory to use @@ -438,13 +443,11 @@ class BuilderThread(threading.Thread): """ # Create a default result - it will be overwritte by the call to # self.make() below, in the event that we do a build. -result = command.CommandResult() -result.return_code = 0 out_dir, out_rel_dir = self._decide_dirs(brd, work_dir, work_in_output) # Check if the job was already completed last time -will_build = self._read_done_file(commit_upto, brd, result, force_build, - force_build_failures) +will_build, result = self._read_done_file(commit_upto, brd, force_build, + force_build_failures) if will_build: # We are going to have to build it. First, get a toolchain -- 2.41.0.255.g8b1d071c50-goog
[PATCH v2 54/60] buildman: Create a function to handle config and build
Move this code into a _config_and_build() function, so reduce the size of
run_commit().
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/builderthread.py | 97 +
1 file changed, 61 insertions(+), 36 deletions(-)
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 0c73b8646b1c..aa4a9d9919c6 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -345,6 +345,64 @@ class BuilderThread(threading.Thread):
commit = 'current'
return commit
+def _config_and_build(self, commit_upto, brd, work_dir, do_config,
+ config_only, adjust_cfg, commit, out_dir,
out_rel_dir,
+ result):
+"""Do the build, configuring first if necessary
+
+Args:
+commit_upto (int): Commit number to build (0...n-1)
+brd (Board): Board to create arguments for
+work_dir (str): Directory to which the source will be checked out
+do_config (bool): True to run a make _defconfig on the
source
+config_only (bool): Only configure the source, do not build it
+adjust_cfg (list of str): See the cfgutil module and run_commit()
+commit (Commit): Commit only being built
+out_dir (str): Output directory for the build
+out_rel_dir (str): Output directory relatie to the current dir
+result (CommandResult): Previous result
+
+Returns:
+tuple:
+result (CommandResult): Result of the build
+do_config (bool): indicates whether 'make config' is needed on
+the next incremental build
+"""
+# Set up the environment and command line
+env = self.toolchain.MakeEnvironment(self.builder.full_path)
+mkdir(out_dir)
+
+args, cwd, src_dir = self._build_args(brd, out_dir, out_rel_dir,
+ work_dir, commit_upto)
+config_args = [f'{brd.target}_defconfig']
+config_out = io.StringIO()
+
+_remove_old_outputs(out_dir)
+
+# If we need to reconfigure, do that now
+cfg_file = os.path.join(out_dir, '.config')
+cmd_list = []
+if do_config or adjust_cfg:
+result = self._reconfigure(
+commit, brd, cwd, args, env, config_args, config_out, cmd_list)
+do_config = False # No need to configure next time
+if adjust_cfg:
+cfgutil.adjust_cfg_file(cfg_file, adjust_cfg)
+
+# Now do the build, if everything looks OK
+if result.return_code == 0:
+result = self._build(commit, brd, cwd, args, env, cmd_list,
+ config_only)
+if adjust_cfg:
+errs = cfgutil.check_cfg_file(cfg_file, adjust_cfg)
+if errs:
+result.stderr += errs
+result.return_code = 1
+result.stderr = result.stderr.replace(src_dir + '/', '')
+if self.builder.verbose_build:
+result.stdout = config_out.getvalue() + result.stdout
+result.cmd_list = cmd_list
+return result, do_config
def run_commit(self, commit_upto, brd, work_dir, do_config, config_only,
force_build, force_build_failures, work_in_output,
@@ -402,42 +460,9 @@ class BuilderThread(threading.Thread):
if self.toolchain:
commit = self._checkout(commit_upto, work_dir)
-
-# Set up the environment and command line
-env = self.toolchain.MakeEnvironment(self.builder.full_path)
-mkdir(out_dir)
-
-args, cwd, src_dir = self._build_args(brd, out_dir,
out_rel_dir,
- work_dir, commit_upto)
-config_args = [f'{brd.target}_defconfig']
-config_out = io.StringIO()
-
-_remove_old_outputs(out_dir)
-
-# If we need to reconfigure, do that now
-cfg_file = os.path.join(out_dir, '.config')
-cmd_list = []
-if do_config or adjust_cfg:
-result = self._reconfigure(
-commit, brd, cwd, args, env, config_args, config_out,
-cmd_list)
-do_config = False # No need to configure next time
-if adjust_cfg:
-cfgutil.adjust_cfg_file(cfg_file, adjust_cfg)
-
-# Now do the build, if everything looks OK
-if result.return_code == 0:
-result = self._build(commit, brd, cwd, args, env, cmd_list,
- config_only)
-if adjust_cfg:
-errs = cfgutil.check_cfg_file(cfg_file, adjust_cfg)
-
[PATCH v2 53/60] buildman: Move checkout code to a separate function
Put this in its own function to reduce the size of the run_commit() function Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/builderthread.py | 30 +- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index 78405956ef51..0c73b8646b1c 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -326,6 +326,26 @@ class BuilderThread(threading.Thread): out_dir = os.path.join(work_dir, out_rel_dir) return out_dir, out_rel_dir +def _checkout(self, commit_upto, work_dir): +"""Checkout the right commit + +Args: +commit_upto (int): Commit number to build (0...n-1) +work_dir (str): Directory to which the source will be checked out + +Returns: +Commit: Commit being built, or 'current' for current source +""" +if self.builder.commits: +commit = self.builder.commits[commit_upto] +if self.builder.checkout: +git_dir = os.path.join(work_dir, '.git') +gitutil.checkout(commit.hash, git_dir, work_dir, force=True) +else: +commit = 'current' +return commit + + def run_commit(self, commit_upto, brd, work_dir, do_config, config_only, force_build, force_build_failures, work_in_output, adjust_cfg): @@ -381,15 +401,7 @@ class BuilderThread(threading.Thread): # to be reported. if self.toolchain: -# Checkout the right commit -if self.builder.commits: -commit = self.builder.commits[commit_upto] -if self.builder.checkout: -git_dir = os.path.join(work_dir, '.git') -gitutil.checkout(commit.hash, git_dir, work_dir, - force=True) -else: -commit = 'current' +commit = self._checkout(commit_upto, work_dir) # Set up the environment and command line env = self.toolchain.MakeEnvironment(self.builder.full_path) -- 2.41.0.255.g8b1d071c50-goog
[PATCH v2 52/60] buildman: Move code to decide output dirs
Put this in its own function to reduce the size of the run_commit()
function.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/builderthread.py | 34 -
1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 4abad86ebc7b..78405956ef51 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -302,6 +302,30 @@ class BuilderThread(threading.Thread):
will_build = False
return will_build
+def _decide_dirs(self, brd, work_dir, work_in_output):
+"""Decide the output directory to use
+
+Args:
+work_dir (str): Directory to which the source will be checked out
+work_in_output (bool): Use the output directory as the work
+directory and don't write to a separate output directory.
+
+Returns:
+tuple:
+out_dir (str): Output directory for the build
+out_rel_dir (str): Output directory relatie to the current dir
+"""
+if work_in_output or self.builder.in_tree:
+out_rel_dir = None
+out_dir = work_dir
+else:
+if self.per_board_out_dir:
+out_rel_dir = os.path.join('..', brd.target)
+else:
+out_rel_dir = 'build'
+out_dir = os.path.join(work_dir, out_rel_dir)
+return out_dir, out_rel_dir
+
def run_commit(self, commit_upto, brd, work_dir, do_config, config_only,
force_build, force_build_failures, work_in_output,
adjust_cfg):
@@ -338,15 +362,7 @@ class BuilderThread(threading.Thread):
# self.make() below, in the event that we do a build.
result = command.CommandResult()
result.return_code = 0
-if work_in_output or self.builder.in_tree:
-out_rel_dir = None
-out_dir = work_dir
-else:
-if self.per_board_out_dir:
-out_rel_dir = os.path.join('..', brd.target)
-else:
-out_rel_dir = 'build'
-out_dir = os.path.join(work_dir, out_rel_dir)
+out_dir, out_rel_dir = self._decide_dirs(brd, work_dir, work_in_output)
# Check if the job was already completed last time
will_build = self._read_done_file(commit_upto, brd, result,
force_build,
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 51/60] buildman: Move code to remove old outputs
Put this in its own function to reduce the size of the run_commit()
function.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/builderthread.py | 28 +++-
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index b4891059b6db..4abad86ebc7b 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -42,6 +42,24 @@ def mkdir(dirname, parents = False):
else:
raise
+
+def _remove_old_outputs(out_dir):
+"""Remove any old output-target files
+
+Args:
+out_dir (str): Output directory for the build
+
+Since we use a build directory that was previously used by another
+board, it may have produced an SPL image. If we don't remove it (i.e.
+see do_config and self.mrproper below) then it will appear to be the
+output of this build, even if it does not produce SPL images.
+"""
+for elf in BASE_ELF_FILENAMES:
+fname = os.path.join(out_dir, elf)
+if os.path.exists(fname):
+os.remove(fname)
+
+
# pylint: disable=R0903
class BuilderJob:
"""Holds information about a job to be performed by a thread
@@ -366,15 +384,7 @@ class BuilderThread(threading.Thread):
config_args = [f'{brd.target}_defconfig']
config_out = io.StringIO()
-# Remove any output targets. Since we use a build directory
that
-# was previously used by another board, it may have produced an
-# SPL image. If we don't remove it (i.e. see do_config and
-# self.mrproper below) then it will appear to be the output of
-# this build, even if it does not produce SPL images.
-for elf in BASE_ELF_FILENAMES:
-fname = os.path.join(out_dir, elf)
-if os.path.exists(fname):
-os.remove(fname)
+_remove_old_outputs(out_dir)
# If we need to reconfigure, do that now
cfg_file = os.path.join(out_dir, '.config')
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 50/60] buildman: Move reading of the done file into a function
Move this logic into its own function to reduce the size of the run_commt() function. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/builderthread.py | 66 + 1 file changed, 42 insertions(+), 24 deletions(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index 2d54e6283023..b4891059b6db 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -244,6 +244,46 @@ class BuilderThread(threading.Thread): result.return_code = 0 return result +def _read_done_file(self, commit_upto, brd, result, force_build, +force_build_failures): +"""Check the 'done' file and see if this commit should be built + +Args: +commit (Commit): Commit only being built +brd (Board): Board being built +result (CommandResult): result object to update +force_build (bool): Force a build even if one was previously done +force_build_failures (bool): Force a bulid if the previous result +showed failure + +Returns: +bool: True if build should be built +""" +done_file = self.builder.get_done_file(commit_upto, brd.target) +result.already_done = os.path.exists(done_file) +will_build = (force_build or force_build_failures or +not result.already_done) +if result.already_done: +with open(done_file, 'r', encoding='utf-8') as outf: +try: +result.return_code = int(outf.readline()) +except ValueError: +# The file may be empty due to running out of disk space. +# Try a rebuild +result.return_code = RETURN_CODE_RETRY + +# Check the signal that the build needs to be retried +if result.return_code == RETURN_CODE_RETRY: +will_build = True +elif will_build: +err_file = self.builder.get_err_file(commit_upto, brd.target) +if os.path.exists(err_file) and os.stat(err_file).st_size: +result.stderr = 'bad' +elif not force_build: +# The build passed, so no need to build it again +will_build = False +return will_build + def run_commit(self, commit_upto, brd, work_dir, do_config, config_only, force_build, force_build_failures, work_in_output, adjust_cfg): @@ -291,30 +331,8 @@ class BuilderThread(threading.Thread): out_dir = os.path.join(work_dir, out_rel_dir) # Check if the job was already completed last time -done_file = self.builder.get_done_file(commit_upto, brd.target) -result.already_done = os.path.exists(done_file) -will_build = (force_build or force_build_failures or -not result.already_done) -if result.already_done: -# Get the return code from that build and use it -with open(done_file, 'r', encoding='utf-8') as outf: -try: -result.return_code = int(outf.readline()) -except ValueError: -# The file may be empty due to running out of disk space. -# Try a rebuild -result.return_code = RETURN_CODE_RETRY - -# Check the signal that the build needs to be retried -if result.return_code == RETURN_CODE_RETRY: -will_build = True -elif will_build: -err_file = self.builder.get_err_file(commit_upto, brd.target) -if os.path.exists(err_file) and os.stat(err_file).st_size: -result.stderr = 'bad' -elif not force_build: -# The build passed, so no need to build it again -will_build = False +will_build = self._read_done_file(commit_upto, brd, result, force_build, + force_build_failures) if will_build: # We are going to have to build it. First, get a toolchain -- 2.41.0.255.g8b1d071c50-goog
[PATCH v2 49/60] buildman: Move bulid code into its own function
Split this into its own function so reduce the size of run_commit().
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/builderthread.py | 40 -
1 file changed, 30 insertions(+), 10 deletions(-)
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 80fca2a61bb3..2d54e6283023 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -218,6 +218,32 @@ class BuilderThread(threading.Thread):
config_out.write(result.combined)
return result
+def _build(self, commit, brd, cwd, args, env, cmd_list, config_only):
+"""Perform the build
+
+Args:
+commit (Commit): Commit only being built
+brd (Board): Board being built
+cwd (str): Current working directory
+args (list of str): Arguments to pass to make
+env (dict): Environment strings
+cmd_list (list of str): List to add the commands to, for logging
+config_only (bool): True if this is a config-only build (using the
+'make cfg' target)
+
+Returns:
+CommandResult object
+"""
+if config_only:
+args.append('cfg')
+result = self.make(commit, brd, 'build', cwd, *args, env=env)
+cmd_list.append([self.builder.gnu_make] + args)
+if (result.return_code == 2 and
+('Some images are invalid' in result.stderr)):
+# This is handled later by the check for output in stderr
+result.return_code = 0
+return result
+
def run_commit(self, commit_upto, brd, work_dir, do_config, config_only,
force_build, force_build_failures, work_in_output,
adjust_cfg):
@@ -342,17 +368,11 @@ class BuilderThread(threading.Thread):
do_config = False # No need to configure next time
if adjust_cfg:
cfgutil.adjust_cfg_file(cfg_file, adjust_cfg)
+
+# Now do the build, if everything looks OK
if result.return_code == 0:
-if config_only:
-args.append('cfg')
-result = self.make(commit, brd, 'build', cwd, *args,
-env=env)
-cmd_list.append([self.builder.gnu_make] + args)
-if (result.return_code == 2 and
-('Some images are invalid' in result.stderr)):
-# This is handled later by the check for output in
-# stderr
-result.return_code = 0
+result = self._build(commit, brd, cwd, args, env, cmd_list,
+ config_only)
if adjust_cfg:
errs = cfgutil.check_cfg_file(cfg_file, adjust_cfg)
if errs:
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 48/60] buildman: Move reconfigure code into its own function
Split this into its own function so reduce the size of run_commit(). Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/builderthread.py | 41 - 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index 9c2938d1b071..80fca2a61bb3 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -191,6 +191,33 @@ class BuilderThread(threading.Thread): args.extend(self.toolchain.MakeArgs()) return args, cwd, src_dir +def _reconfigure(self, commit, brd, cwd, args, env, config_args, config_out, + cmd_list): +"""Reconfigure the build + +Args: +commit (Commit): Commit only being built +brd (Board): Board being built +cwd (str): Current working directory +args (list of str): Arguments to pass to make +env (dict): Environment strings +config_args (list of str): defconfig arg for this board +cmd_list (list of str): List to add the commands to, for logging + +Returns: +CommandResult object +""" +if self.mrproper: +result = self.make(commit, brd, 'mrproper', cwd, 'mrproper', *args, + env=env) +config_out.write(result.combined) +cmd_list.append([self.builder.gnu_make, 'mrproper', *args]) +result = self.make(commit, brd, 'config', cwd, *(args + config_args), + env=env) +cmd_list.append([self.builder.gnu_make] + args + config_args) +config_out.write(result.combined) +return result + def run_commit(self, commit_upto, brd, work_dir, do_config, config_only, force_build, force_build_failures, work_in_output, adjust_cfg): @@ -309,17 +336,9 @@ class BuilderThread(threading.Thread): cfg_file = os.path.join(out_dir, '.config') cmd_list = [] if do_config or adjust_cfg: -if self.mrproper: -result = self.make(commit, brd, 'mrproper', cwd, -'mrproper', *args, env=env) -config_out.write(result.combined) -cmd_list.append([self.builder.gnu_make, 'mrproper', - *args]) -result = self.make(commit, brd, 'config', cwd, -*(args + config_args), env=env) -cmd_list.append([self.builder.gnu_make] + args + -config_args) -config_out.write(result.combined) +result = self._reconfigure( +commit, brd, cwd, args, env, config_args, config_out, +cmd_list) do_config = False # No need to configure next time if adjust_cfg: cfgutil.adjust_cfg_file(cfg_file, adjust_cfg) -- 2.41.0.255.g8b1d071c50-goog
[PATCH v2 47/60] buildman: Convert config_out to string IO
This is probably a little more efficient and it allows passing the object
to another function to write data. Convert config_out to use a string I/O
device.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/builderthread.py | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 42af5197dace..9c2938d1b071 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -10,6 +10,7 @@ based on the jobs provided.
import errno
import glob
+import io
import os
import shutil
import sys
@@ -292,7 +293,7 @@ class BuilderThread(threading.Thread):
args, cwd, src_dir = self._build_args(brd, out_dir,
out_rel_dir,
work_dir, commit_upto)
config_args = [f'{brd.target}_defconfig']
-config_out = ''
+config_out = io.StringIO()
# Remove any output targets. Since we use a build directory
that
# was previously used by another board, it may have produced an
@@ -311,14 +312,14 @@ class BuilderThread(threading.Thread):
if self.mrproper:
result = self.make(commit, brd, 'mrproper', cwd,
'mrproper', *args, env=env)
-config_out += result.combined
+config_out.write(result.combined)
cmd_list.append([self.builder.gnu_make, 'mrproper',
*args])
result = self.make(commit, brd, 'config', cwd,
*(args + config_args), env=env)
cmd_list.append([self.builder.gnu_make] + args +
config_args)
-config_out += result.combined
+config_out.write(result.combined)
do_config = False # No need to configure next time
if adjust_cfg:
cfgutil.adjust_cfg_file(cfg_file, adjust_cfg)
@@ -340,7 +341,7 @@ class BuilderThread(threading.Thread):
result.return_code = 1
result.stderr = result.stderr.replace(src_dir + '/', '')
if self.builder.verbose_build:
-result.stdout = config_out + result.stdout
+result.stdout = config_out.getvalue() + result.stdout
result.cmd_list = cmd_list
else:
result.return_code = 1
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 46/60] buildman: Move more things into _build_args()
Move more of the argument-building code into this function. Fix a missing
assignment for out_rel_dir too.
Rename the function since it now builds all the arguments.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/builderthread.py | 55 -
1 file changed, 34 insertions(+), 21 deletions(-)
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index e7d9a29d03ef..42af5197dace 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -137,13 +137,40 @@ class BuilderThread(threading.Thread):
return self.builder.do_make(commit, brd, stage, cwd, *args,
**kwargs)
-def _build_args(self, args, brd):
+def _build_args(self, brd, out_dir, out_rel_dir, work_dir, commit_upto):
"""Set up arguments to the args list based on the settings
Args:
-args (list of str): List of string arguments to add things to
brd (Board): Board to create arguments for
+out_dir (str): Path to output directory containing the files
+out_rel_dir (str): Output directory relative to the current dir
+work_dir (str): Directory to which the source will be checked out
+commit_upto (int): Commit number to build (0...n-1)
+
+Returns:
+tuple:
+list of str: Arguments to pass to make
+str: Current working directory, or None if no commit
+str: Source directory (typically the work directory)
"""
+args = []
+cwd = work_dir
+src_dir = os.path.realpath(work_dir)
+if not self.builder.in_tree:
+if commit_upto is None:
+# In this case we are building in the original source directory
+# (i.e. the current directory where buildman is invoked. The
+# output directory is set to this thread's selected work
+# directory.
+#
+# Symlinks can confuse U-Boot's Makefile since we may use '..'
+# in our path, so remove them.
+real_dir = os.path.realpath(out_dir)
+args.append(f'O={real_dir}')
+cwd = None
+src_dir = os.getcwd()
+else:
+args.append(f'O={out_rel_dir}')
if self.builder.verbose_build:
args.append('V=1')
else:
@@ -161,6 +188,7 @@ class BuilderThread(threading.Thread):
args.append('SOURCE_DATE_EPOCH=0')
args.extend(self.builder.toolchains.GetMakeArguments(brd))
args.extend(self.toolchain.MakeArgs())
+return args, cwd, src_dir
def run_commit(self, commit_upto, brd, work_dir, do_config, config_only,
force_build, force_build_failures, work_in_output,
@@ -199,6 +227,7 @@ class BuilderThread(threading.Thread):
result = command.CommandResult()
result.return_code = 0
if work_in_output or self.builder.in_tree:
+out_rel_dir = None
out_dir = work_dir
else:
if self.per_board_out_dir:
@@ -259,25 +288,9 @@ class BuilderThread(threading.Thread):
# Set up the environment and command line
env = self.toolchain.MakeEnvironment(self.builder.full_path)
mkdir(out_dir)
-args = []
-cwd = work_dir
-src_dir = os.path.realpath(work_dir)
-if not self.builder.in_tree:
-if commit_upto is None:
-# In this case we are building in the original source
-# directory (i.e. the current directory where buildman
-# is invoked. The output directory is set to this
-# thread's selected work directory.
-#
-# Symlinks can confuse U-Boot's Makefile since
-# we may use '..' in our path, so remove them.
-real_dir = os.path.realpath(out_dir)
-args.append(f'O={real_dir}')
-cwd = None
-src_dir = os.getcwd()
-else:
-args.append(f'O={out_rel_dir}')
-self._build_args(args, brd)
+
+args, cwd, src_dir = self._build_args(brd, out_dir,
out_rel_dir,
+ work_dir, commit_upto)
config_args = [f'{brd.target}_defconfig']
config_out = ''
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 45/60] buildman: Move setting of toolchain arguments to _build_args()
Move a few more pieces to this new function.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/builderthread.py | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 47ebf4dcdd90..e7d9a29d03ef 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -137,11 +137,12 @@ class BuilderThread(threading.Thread):
return self.builder.do_make(commit, brd, stage, cwd, *args,
**kwargs)
-def _build_args(self, args):
+def _build_args(self, args, brd):
"""Set up arguments to the args list based on the settings
Args:
args (list of str): List of string arguments to add things to
+brd (Board): Board to create arguments for
"""
if self.builder.verbose_build:
args.append('V=1')
@@ -158,6 +159,8 @@ class BuilderThread(threading.Thread):
args.append('NO_LTO=1')
if self.builder.reproducible_builds:
args.append('SOURCE_DATE_EPOCH=0')
+args.extend(self.builder.toolchains.GetMakeArguments(brd))
+args.extend(self.toolchain.MakeArgs())
def run_commit(self, commit_upto, brd, work_dir, do_config, config_only,
force_build, force_build_failures, work_in_output,
@@ -274,11 +277,9 @@ class BuilderThread(threading.Thread):
src_dir = os.getcwd()
else:
args.append(f'O={out_rel_dir}')
-self._build_args(args)
+self._build_args(args, brd)
config_args = [f'{brd.target}_defconfig']
config_out = ''
-args.extend(self.builder.toolchains.GetMakeArguments(brd))
-args.extend(self.toolchain.MakeArgs())
# Remove any output targets. Since we use a build directory
that
# was previously used by another board, it may have produced an
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 44/60] buildman: Start a function to set up the make arguments
Move some of this code into a new funciion, to help reduce the size of the
run_commits() function.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/builderthread.py | 38 -
1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index ad12e9ede7f4..47ebf4dcdd90 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -137,6 +137,28 @@ class BuilderThread(threading.Thread):
return self.builder.do_make(commit, brd, stage, cwd, *args,
**kwargs)
+def _build_args(self, args):
+"""Set up arguments to the args list based on the settings
+
+Args:
+args (list of str): List of string arguments to add things to
+"""
+if self.builder.verbose_build:
+args.append('V=1')
+else:
+args.append('-s')
+if self.builder.num_jobs is not None:
+args.extend(['-j', str(self.builder.num_jobs)])
+if self.builder.warnings_as_errors:
+args.append('KCFLAGS=-Werror')
+args.append('HOSTCFLAGS=-Werror')
+if self.builder.allow_missing:
+args.append('BINMAN_ALLOW_MISSING=1')
+if self.builder.no_lto:
+args.append('NO_LTO=1')
+if self.builder.reproducible_builds:
+args.append('SOURCE_DATE_EPOCH=0')
+
def run_commit(self, commit_upto, brd, work_dir, do_config, config_only,
force_build, force_build_failures, work_in_output,
adjust_cfg):
@@ -252,21 +274,7 @@ class BuilderThread(threading.Thread):
src_dir = os.getcwd()
else:
args.append(f'O={out_rel_dir}')
-if self.builder.verbose_build:
-args.append('V=1')
-else:
-args.append('-s')
-if self.builder.num_jobs is not None:
-args.extend(['-j', str(self.builder.num_jobs)])
-if self.builder.warnings_as_errors:
-args.append('KCFLAGS=-Werror')
-args.append('HOSTCFLAGS=-Werror')
-if self.builder.allow_missing:
-args.append('BINMAN_ALLOW_MISSING=1')
-if self.builder.no_lto:
-args.append('NO_LTO=1')
-if self.builder.reproducible_builds:
-args.append('SOURCE_DATE_EPOCH=0')
+self._build_args(args)
config_args = [f'{brd.target}_defconfig']
config_out = ''
args.extend(self.builder.toolchains.GetMakeArguments(brd))
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 43/60] buildman: Drop unnecessary assignment of config_out
This is already set up earlier in the function, so drop the extra assignment. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/builderthread.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index f110137e8f6e..ad12e9ede7f4 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -286,7 +286,6 @@ class BuilderThread(threading.Thread): cfg_file = os.path.join(out_dir, '.config') cmd_list = [] if do_config or adjust_cfg: -config_out = '' if self.mrproper: result = self.make(commit, brd, 'mrproper', cwd, 'mrproper', *args, env=env) -- 2.41.0.255.g8b1d071c50-goog
[PATCH v2 42/60] buildman: Correct invalid use of out_dir variable
This variable has a different meaning in the outer scrop. Use a different
name to avoid confusion, or bugs.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/builderthread.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 45ae6edf9f48..f110137e8f6e 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -246,8 +246,8 @@ class BuilderThread(threading.Thread):
#
# Symlinks can confuse U-Boot's Makefile since
# we may use '..' in our path, so remove them.
-out_dir = os.path.realpath(out_dir)
-args.append(f'O={out_dir}')
+real_dir = os.path.realpath(out_dir)
+args.append(f'O={real_dir}')
cwd = None
src_dir = os.getcwd()
else:
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 41/60] buildman: Export _get_output_dir() to avoid warnings
Make this a public memory since it is used outside the class.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/builder.py | 8
tools/buildman/builderthread.py | 2 +-
tools/buildman/test.py | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index 4cc266ff3049..ecbd368c47a5 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -565,7 +565,7 @@ class Builder:
terminal.print_clear()
tprint(line, newline=False, limit_to_line=True)
-def _get_output_dir(self, commit_upto):
+def get_output_dir(self, commit_upto):
"""Get the name of the output directory for a commit number
The output directory is typically ...//.
@@ -598,7 +598,7 @@ class Builder:
commit_upto: Commit number to use (0..self.count-1)
target: Target name
"""
-output_dir = self._get_output_dir(commit_upto)
+output_dir = self.get_output_dir(commit_upto)
if self.work_in_output:
return output_dir
return os.path.join(output_dir, target)
@@ -1717,7 +1717,7 @@ class Builder:
Figure out what needs to be deleted in the output directory before it
can be used. We only delete old buildman directories which have the
-expected name pattern. See _get_output_dir().
+expected name pattern. See get_output_dir().
Returns:
List of full paths of directories to remove
@@ -1726,7 +1726,7 @@ class Builder:
return
dir_list = []
for commit_upto in range(self.commit_count):
-dir_list.append(self._get_output_dir(commit_upto))
+dir_list.append(self.get_output_dir(commit_upto))
to_remove = []
for dirname in glob.glob(os.path.join(self.base_dir, '*')):
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 179fc477c008..45ae6edf9f48 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -351,7 +351,7 @@ class BuilderThread(threading.Thread):
return
# Write the output and stderr
-output_dir = self.builder._get_output_dir(result.commit_upto)
+output_dir = self.builder.get_output_dir(result.commit_upto)
mkdir(output_dir)
build_dir = self.builder.get_build_dir(result.commit_upto,
result.brd.target)
diff --git a/tools/buildman/test.py b/tools/buildman/test.py
index 7ac67260c337..bdd3d84158a6 100644
--- a/tools/buildman/test.py
+++ b/tools/buildman/test.py
@@ -528,7 +528,7 @@ class TestBuild(unittest.TestCase):
'sandbox']),
({'all': ['board4'], 'sandbox': ['board4']}, []))
def CheckDirs(self, build, dirname):
-self.assertEqual('base%s' % dirname, build._get_output_dir(1))
+self.assertEqual('base%s' % dirname, build.get_output_dir(1))
self.assertEqual('base%s/fred' % dirname,
build.get_build_dir(1, 'fred'))
self.assertEqual('base%s/fred/done' % dirname,
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 40/60] buildman: Correct most pylint warnings in builderthread
Fix the easy warnings in this file.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/builderthread.py | 95 ++---
1 file changed, 51 insertions(+), 44 deletions(-)
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 3f52d95fed9f..179fc477c008 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -2,6 +2,12 @@
# Copyright (c) 2014 Google, Inc
#
+"""Implementation the bulider threads
+
+This module provides the BuilderThread class, which handles calling the builder
+based on the jobs provided.
+"""
+
import errno
import glob
import os
@@ -30,12 +36,12 @@ def mkdir(dirname, parents = False):
except OSError as err:
if err.errno == errno.EEXIST:
if os.path.realpath('.') == os.path.realpath(dirname):
-print("Cannot create the current working directory '%s'!" %
dirname)
+print(f"Cannot create the current working directory
'{dirname}'!")
sys.exit(1)
-pass
else:
raise
+# pylint: disable=R0903
class BuilderJob:
"""Holds information about a job to be performed by a thread
@@ -107,6 +113,7 @@ class BuilderThread(threading.Thread):
self.mrproper = mrproper
self.per_board_out_dir = per_board_out_dir
self.test_exception = test_exception
+self.toolchain = None
def make(self, commit, brd, stage, cwd, *args, **kwargs):
"""Run 'make' on a particular commit and board.
@@ -182,9 +189,9 @@ class BuilderThread(threading.Thread):
not result.already_done)
if result.already_done:
# Get the return code from that build and use it
-with open(done_file, 'r') as fd:
+with open(done_file, 'r', encoding='utf-8') as outf:
try:
-result.return_code = int(fd.readline())
+result.return_code = int(outf.readline())
except ValueError:
# The file may be empty due to running out of disk space.
# Try a rebuild
@@ -240,11 +247,11 @@ class BuilderThread(threading.Thread):
# Symlinks can confuse U-Boot's Makefile since
# we may use '..' in our path, so remove them.
out_dir = os.path.realpath(out_dir)
-args.append('O=%s' % out_dir)
+args.append(f'O={out_dir}')
cwd = None
src_dir = os.getcwd()
else:
-args.append('O=%s' % out_rel_dir)
+args.append(f'O={out_rel_dir}')
if self.builder.verbose_build:
args.append('V=1')
else:
@@ -260,7 +267,7 @@ class BuilderThread(threading.Thread):
args.append('NO_LTO=1')
if self.builder.reproducible_builds:
args.append('SOURCE_DATE_EPOCH=0')
-config_args = ['%s_defconfig' % brd.target]
+config_args = [f'{brd.target}_defconfig']
config_out = ''
args.extend(self.builder.toolchains.GetMakeArguments(brd))
args.extend(self.toolchain.MakeArgs())
@@ -270,7 +277,6 @@ class BuilderThread(threading.Thread):
# SPL image. If we don't remove it (i.e. see do_config and
# self.mrproper below) then it will appear to be the output of
# this build, even if it does not produce SPL images.
-build_dir = self.builder.get_build_dir(commit_upto, brd.target)
for elf in BASE_ELF_FILENAMES:
fname = os.path.join(out_dir, elf)
if os.path.exists(fname):
@@ -317,7 +323,7 @@ class BuilderThread(threading.Thread):
result.cmd_list = cmd_list
else:
result.return_code = 1
-result.stderr = 'No tool chain for %s\n' % brd.arch
+result.stderr = f'No tool chain for {brd.arch}\n'
result.already_done = False
result.toolchain = self.toolchain
@@ -352,15 +358,15 @@ class BuilderThread(threading.Thread):
mkdir(build_dir)
outfile = os.path.join(build_dir, 'log')
-with open(outfile, 'w') as fd:
+with open(outfile, 'w', encoding='utf-8') as outf:
if result.stdout:
-fd.write(result.stdout)
+outf.write(result.stdout)
errfile = self.builder.get_err_file(result.commit_upto,
result.brd.target)
if result.stderr:
-with open(errfile, 'w') as fd:
-fd.write(result.stderr)
+with open(errfile, 'w', encoding='utf-8') as outf:
+outf.write(result.stderr)
elif os.path.exists(errfile):
[PATCH v2 37/60] buildman: Convert camel case in builder.py
Convert this file to snake case and update all files which use it.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/builder.py | 228
tools/buildman/builderthread.py | 26 ++--
tools/buildman/control.py | 13 +-
tools/buildman/func_test.py | 2 +-
tools/buildman/test.py | 22 +--
5 files changed, 145 insertions(+), 146 deletions(-)
diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index 620b7b8c31a8..80c05fbd1e73 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -134,7 +134,7 @@ class Config:
for fname in config_filename:
self.config[fname] = {}
-def Add(self, fname, key, value):
+def add(self, fname, key, value):
self.config[fname][key] = value
def __hash__(self):
@@ -151,7 +151,7 @@ class Environment:
self.target = target
self.environment = {}
-def Add(self, key, value):
+def add(self, key, value):
self.environment[key] = value
class Builder:
@@ -315,7 +315,7 @@ class Builder:
else:
self._working_dir = os.path.join(base_dir, '.bm-work')
self.threads = []
-self.do_make = make_func or self.Make
+self.do_make = make_func or self.make
self.gnu_make = gnu_make
self.checkout = checkout
self.num_threads = num_threads
@@ -401,7 +401,7 @@ class Builder:
def signal_handler(self, signal, frame):
sys.exit(1)
-def SetDisplayOptions(self, show_errors=False, show_sizes=False,
+def set_display_options(self, show_errors=False, show_sizes=False,
show_detail=False, show_bloat=False,
list_error_boards=False, show_config=False,
show_environment=False, filter_dtb_warnings=False,
@@ -434,7 +434,7 @@ class Builder:
self._filter_migration_warnings = filter_migration_warnings
self._ide = ide
-def _AddTimestamp(self):
+def _add_timestamp(self):
"""Add a new timestamp to the list and record the build period.
The build period is the length of time taken to perform a single
@@ -463,14 +463,14 @@ class Builder:
self._timestamps.popleft()
count -= 1
-def SelectCommit(self, commit, checkout=True):
+def select_commit(self, commit, checkout=True):
"""Checkout the selected commit for this build
"""
self.commit = commit
if checkout and self.checkout:
gitutil.checkout(commit.hash)
-def Make(self, commit, brd, stage, cwd, *args, **kwargs):
+def make(self, commit, brd, stage, cwd, *args, **kwargs):
"""Run make
Args:
@@ -515,7 +515,7 @@ class Builder:
result.combined = '%s\n' % (' '.join(cmd)) + result.combined
return result
-def ProcessResult(self, result):
+def process_result(self, result):
"""Process the result of a build, showing progress information
Args:
@@ -536,8 +536,8 @@ class Builder:
if self._verbose:
terminal.print_clear()
boards_selected = {target : result.brd}
-self.ResetResultSummary(boards_selected)
-self.ProduceResultSummary(result.commit_upto, self.commits,
+self.reset_result_summary(boards_selected)
+self.produce_result_summary(result.commit_upto, self.commits,
boards_selected)
else:
target = '(starting)'
@@ -556,7 +556,7 @@ class Builder:
line += ' ' * 8
# Add our current completion time estimate
-self._AddTimestamp()
+self._add_timestamp()
if self._complete_delay:
line += '%s : ' % self._complete_delay
@@ -565,7 +565,7 @@ class Builder:
terminal.print_clear()
tprint(line, newline=False, limit_to_line=True)
-def _GetOutputDir(self, commit_upto):
+def _get_output_dir(self, commit_upto):
"""Get the name of the output directory for a commit number
The output directory is typically ...//.
@@ -580,7 +580,7 @@ class Builder:
if self.commits:
commit = self.commits[commit_upto]
subject = commit.subject.translate(trans_valid_chars)
-# See _GetOutputSpaceRemovals() which parses this name
+# See _get_output_space_removals() which parses this name
commit_dir = ('%02d_g%s_%s' % (commit_upto + 1,
commit.hash, subject[:20]))
elif not self.no_subdirs:
@@ -589,7 +589,7 @@ class Builder:
return self.base_dir
return os.path.join(self.base_dir, commit_dir)
-def GetBuildDir(self, commit_upto, target):
+def get_build_dir(self, commit_upto, target):
"""Get the name of the build directory for a commit number
[PATCH v2 39/60] buildman: Convert camel case in builderthread.py
Convert this file to snake case and update all files which use it. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/builder.py | 8 +++--- tools/buildman/builderthread.py | 50 - 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 80c05fbd1e73..4cc266ff3049 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -1648,7 +1648,7 @@ class Builder: 'worktree' to set up a git worktree """ thread_dir = self.get_thread_dir(thread_num) -builderthread.Mkdir(thread_dir) +builderthread.mkdir(thread_dir) git_dir = os.path.join(thread_dir, '.git') # Create a worktree or a git repo clone for this thread if it @@ -1696,7 +1696,7 @@ class Builder: work setup_git: True to set up a git worktree or a git clone """ -builderthread.Mkdir(self._working_dir) +builderthread.mkdir(self._working_dir) if setup_git and self.git_dir: src_dir = os.path.abspath(self.git_dir) if gitutil.check_worktree_is_available(src_dir): @@ -1772,7 +1772,7 @@ class Builder: self._verbose = verbose self.reset_result_summary(board_selected) -builderthread.Mkdir(self.base_dir, parents = True) +builderthread.mkdir(self.base_dir, parents = True) self._prepare_working_space(min(self.num_threads, len(board_selected)), commits is not None) self._prepare_output_space() @@ -1793,7 +1793,7 @@ class Builder: if self.num_threads: self.queue.put(job) else: -self._single_builder.RunJob(job) +self._single_builder.run_job(job) if self.num_threads: term = threading.Thread(target=self.queue.join) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index 5f1200ae890f..3f52d95fed9f 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -16,7 +16,7 @@ from u_boot_pylib import command RETURN_CODE_RETRY = -1 BASE_ELF_FILENAMES = ['u-boot', 'spl/u-boot-spl', 'tpl/u-boot-tpl'] -def Mkdir(dirname, parents = False): +def mkdir(dirname, parents = False): """Make a directory if it doesn't already exist. Args: @@ -108,7 +108,7 @@ class BuilderThread(threading.Thread): self.per_board_out_dir = per_board_out_dir self.test_exception = test_exception -def Make(self, commit, brd, stage, cwd, *args, **kwargs): +def make(self, commit, brd, stage, cwd, *args, **kwargs): """Run 'make' on a particular commit and board. The source code will already be checked out, so the 'commit' @@ -130,7 +130,7 @@ class BuilderThread(threading.Thread): return self.builder.do_make(commit, brd, stage, cwd, *args, **kwargs) -def RunCommit(self, commit_upto, brd, work_dir, do_config, config_only, +def run_commit(self, commit_upto, brd, work_dir, do_config, config_only, force_build, force_build_failures, work_in_output, adjust_cfg): """Build a particular commit. @@ -163,7 +163,7 @@ class BuilderThread(threading.Thread): - boolean indicating whether 'make config' is still needed """ # Create a default result - it will be overwritte by the call to -# self.Make() below, in the event that we do a build. +# self.make() below, in the event that we do a build. result = command.CommandResult() result.return_code = 0 if work_in_output or self.builder.in_tree: @@ -226,7 +226,7 @@ class BuilderThread(threading.Thread): # Set up the environment and command line env = self.toolchain.MakeEnvironment(self.builder.full_path) -Mkdir(out_dir) +mkdir(out_dir) args = [] cwd = work_dir src_dir = os.path.realpath(work_dir) @@ -282,12 +282,12 @@ class BuilderThread(threading.Thread): if do_config or adjust_cfg: config_out = '' if self.mrproper: -result = self.Make(commit, brd, 'mrproper', cwd, +result = self.make(commit, brd, 'mrproper', cwd, 'mrproper', *args, env=env) config_out += result.combined cmd_list.append([self.builder.gnu_make, 'mrproper', *args]) -result = self.Make(commit, brd, 'config', cwd, +result = self.make(commit, brd, 'config', cwd, *(args + config_args), env=env) cmd_list.append([self.builder.gnu_make] + args +
[PATCH v2 38/60] buildman: Split parser creation in two
Split this into two functions to avoid a warning about too many
statements.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/cmdline.py | 44 +--
1 file changed, 33 insertions(+), 11 deletions(-)
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index 6b9c1db14af7..e295c7aef1a0 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -14,19 +14,14 @@ import pathlib
BUILDMAN_DIR = pathlib.Path(__file__).parent
HAS_TESTS = os.path.exists(BUILDMAN_DIR / "test.py")
-def parse_args():
-"""Parse command line arguments from sys.argv[]
-
-Returns:
-tuple containing:
-options: command line options
-args: command lin arguments
-"""
-epilog = """ [list of target/arch/cpu/board/vendor/soc to build]
+def add_upto_m(parser):
+"""Add arguments up to 'M'
-Build U-Boot for all commits in a branch. Use -n to do a dry run"""
+Args:
+parser (ArgumentParser): Parse to add to
-parser = argparse.ArgumentParser(epilog=epilog)
+This is split out to avoid having too many statements in one function
+"""
parser.add_argument('-a', '--adjust-cfg', type=str, action='append',
help='Adjust the Kconfig settings in .config before building')
parser.add_argument('-A', '--print-prefix', action='store_true',
@@ -104,6 +99,16 @@ def parse_args():
parser.add_argument('-N', '--no-subdirs', action='store_true',
dest='no_subdirs',
default=False,
help="Don't create subdirectories when building current source for a
single board")
+
+
+def add_after_m(parser):
+"""Add arguments after 'M'
+
+Args:
+parser (ArgumentParser): Parse to add to
+
+This is split out to avoid having too many statements in one function
+"""
parser.add_argument('-o', '--output-dir', type=str, dest='output_dir',
help='Directory where all builds happen and buildman has its
workspace (default is ../)')
parser.add_argument('-O', '--override-toolchain', type=str,
@@ -153,6 +158,23 @@ def parse_args():
parser.add_argument('-Y', '--filter-migration-warnings',
action='store_true',
default=False,
help='Filter out migration warnings from output')
+
+
+def parse_args():
+"""Parse command line arguments from sys.argv[]
+
+Returns:
+tuple containing:
+options: command line options
+args: command lin arguments
+"""
+epilog = """ [list of target/arch/cpu/board/vendor/soc to build]
+
+Build U-Boot for all commits in a branch. Use -n to do a dry run"""
+
+parser = argparse.ArgumentParser(epilog=epilog)
+add_upto_m(parser)
+add_after_m(parser)
parser.add_argument('terms', type=str, nargs='*',
help='Board / SoC names to build')
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 35/60] buildman: Convert to argparse
Use argparse to parse the arguments, since OptionParser is deprecated now.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/cmdline.py | 125
tools/buildman/control.py | 140 ++--
tools/buildman/func_test.py | 6 +-
tools/buildman/main.py | 16 ++---
4 files changed, 145 insertions(+), 142 deletions(-)
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index 91571a5adb1d..6b9c1db14af7 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -7,7 +7,7 @@
This creates the argument parser and uses it to parse the arguments passed in
"""
-from optparse import OptionParser
+import argparse
import os
import pathlib
@@ -22,135 +22,138 @@ def parse_args():
options: command line options
args: command lin arguments
"""
-parser = OptionParser()
-parser.add_option('-a', '--adjust-cfg', type=str, action='append',
+epilog = """ [list of target/arch/cpu/board/vendor/soc to build]
+
+Build U-Boot for all commits in a branch. Use -n to do a dry run"""
+
+parser = argparse.ArgumentParser(epilog=epilog)
+parser.add_argument('-a', '--adjust-cfg', type=str, action='append',
help='Adjust the Kconfig settings in .config before building')
-parser.add_option('-A', '--print-prefix', action='store_true',
+parser.add_argument('-A', '--print-prefix', action='store_true',
help='Print the tool-chain prefix for a board (CROSS_COMPILE=)')
-parser.add_option('-b', '--branch', type='string',
+parser.add_argument('-b', '--branch', type=str,
help='Branch name to build, or range of commits to build')
-parser.add_option('-B', '--bloat', dest='show_bloat',
+parser.add_argument('-B', '--bloat', dest='show_bloat',
action='store_true', default=False,
help='Show changes in function code size for each board')
-parser.add_option('--boards', type='string', action='append',
+parser.add_argument('--boards', type=str, action='append',
help='List of board names to build separated by comma')
-parser.add_option('-c', '--count', dest='count', type='int',
+parser.add_argument('-c', '--count', dest='count', type=int,
default=-1, help='Run build on the top n commits')
-parser.add_option('-C', '--force-reconfig', dest='force_reconfig',
+parser.add_argument('-C', '--force-reconfig', dest='force_reconfig',
action='store_true', default=False,
help='Reconfigure for every commit (disable incremental build)')
-parser.add_option('-d', '--detail', dest='show_detail',
+parser.add_argument('-d', '--detail', dest='show_detail',
action='store_true', default=False,
help='Show detailed size delta for each board in the -S summary')
-parser.add_option('-D', '--config-only', action='store_true',
default=False,
+parser.add_argument('-D', '--config-only', action='store_true',
+default=False,
help="Don't build, just configure each commit")
-parser.add_option('--debug', action='store_true',
+parser.add_argument('--debug', action='store_true',
help='Enabling debugging (provides a full traceback on error)')
-parser.add_option('-e', '--show_errors', action='store_true',
+parser.add_argument('-e', '--show_errors', action='store_true',
default=False, help='Show errors and warnings')
-parser.add_option('-E', '--warnings-as-errors', action='store_true',
+parser.add_argument('-E', '--warnings-as-errors', action='store_true',
default=False, help='Treat all compiler warnings as errors')
-parser.add_option('-f', '--force-build', dest='force_build',
+parser.add_argument('-f', '--force-build', dest='force_build',
action='store_true', default=False,
help='Force build of boards even if already built')
-parser.add_option('-F', '--force-build-failures',
dest='force_build_failures',
+parser.add_argument('-F', '--force-build-failures',
dest='force_build_failures',
action='store_true', default=False,
help='Force build of previously-failed build')
-parser.add_option('--fetch-arch', type='string',
+parser.add_argument('--fetch-arch', type=str,
help="Fetch a toolchain for architecture FETCH_ARCH ('list' to
list)."
' You can also fetch several toolchains separate by comma, or'
" 'all' to download all")
-parser.add_option('-g', '--git', type='string',
+parser.add_argument('-g', '--git', type=str,
help='Git repo containing branch to build', default='.')
-parser.add_option('-G', '--config-file', type='string',
+parser.add_argument('-G', '--config-file', type=str,
help='Path to buildman config file', default='')
-parser.add_option('-H', '--full-help', action='store_true',
dest='full_help',
+
[PATCH v2 36/60] buildman: Convert camel case in bsettings.py
Convert this file to snake case and update all files which use it.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/bsettings.py | 14 +++---
tools/buildman/control.py | 2 +-
tools/buildman/func_test.py | 12 ++--
tools/buildman/main.py | 2 +-
tools/buildman/test.py | 4 ++--
tools/buildman/toolchain.py | 14 +++---
tools/moveconfig.py | 2 +-
7 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/tools/buildman/bsettings.py b/tools/buildman/bsettings.py
index 0eb894a558c1..612ec0c28464 100644
--- a/tools/buildman/bsettings.py
+++ b/tools/buildman/bsettings.py
@@ -7,7 +7,7 @@ import io
config_fname = None
-def Setup(fname=''):
+def setup(fname=''):
"""Set up the buildman settings module by reading config files
Args:
@@ -23,15 +23,15 @@ def Setup(fname=''):
config_fname = '%s/.buildman' % os.getenv('HOME')
if not os.path.exists(config_fname):
print('No config file found ~/.buildman\nCreating one...\n')
-CreateBuildmanConfigFile(config_fname)
+create_buildman_config_file(config_fname)
print('To install tool chains, please use the --fetch-arch option')
if config_fname:
settings.read(config_fname)
-def AddFile(data):
+def add_file(data):
settings.readfp(io.StringIO(data))
-def GetItems(section):
+def get_items(section):
"""Get the items from a section of the config.
Args:
@@ -47,7 +47,7 @@ def GetItems(section):
except:
raise
-def GetGlobalItemValue(name):
+def get_global_item_value(name):
"""Get an item from the 'global' section of the config.
Args:
@@ -58,7 +58,7 @@ def GetGlobalItemValue(name):
"""
return settings.get('global', name, fallback=None)
-def SetItem(section, tag, value):
+def set_item(section, tag, value):
"""Set an item and write it back to the settings file"""
global settings
global config_fname
@@ -68,7 +68,7 @@ def SetItem(section, tag, value):
with open(config_fname, 'w') as fd:
settings.write(fd)
-def CreateBuildmanConfigFile(config_fname):
+def create_buildman_config_file(config_fname):
"""Creates a new config file with no tool chain information.
Args:
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index e67d410b6295..2b25fb2abf8b 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -157,7 +157,7 @@ def get_allow_missing(opt_allow, opt_no_allow,
num_selected, has_branch):
external blobs are used
"""
allow_missing = False
-am_setting = bsettings.GetGlobalItemValue('allow-missing')
+am_setting = bsettings.get_global_item_value('allow-missing')
if am_setting:
if am_setting == 'always':
allow_missing = True
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index 8d15e3a9cce2..7162d8c2fcc0 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -184,8 +184,8 @@ class TestFunctional(unittest.TestCase):
self._buildman_pathname = sys.argv[0]
self._buildman_dir = os.path.dirname(os.path.realpath(sys.argv[0]))
command.test_result = self._HandleCommand
-bsettings.Setup(None)
-bsettings.AddFile(settings_data)
+bsettings.setup(None)
+bsettings.add_file(settings_data)
self.setupToolchains()
self._toolchains.Add('arm-gcc', test=False)
self._toolchains.Add('powerpc-gcc', test=False)
@@ -691,7 +691,7 @@ Some images are invalid'''
def testBlobSettingsAlways(self):
"""Test the 'always' policy"""
-bsettings.SetItem('global', 'allow-missing', 'always')
+bsettings.set_item('global', 'allow-missing', 'always')
self.assertEqual(True,
control.get_allow_missing(False, False, 1, False))
self.assertEqual(False,
@@ -699,7 +699,7 @@ Some images are invalid'''
def testBlobSettingsBranch(self):
"""Test the 'branch' policy"""
-bsettings.SetItem('global', 'allow-missing', 'branch')
+bsettings.set_item('global', 'allow-missing', 'branch')
self.assertEqual(False,
control.get_allow_missing(False, False, 1, False))
self.assertEqual(True,
@@ -709,7 +709,7 @@ Some images are invalid'''
def testBlobSettingsMultiple(self):
"""Test the 'multiple' policy"""
-bsettings.SetItem('global', 'allow-missing', 'multiple')
+bsettings.set_item('global', 'allow-missing', 'multiple')
self.assertEqual(False,
control.get_allow_missing(False, False, 1, False))
self.assertEqual(True,
@@ -719,7 +719,7 @@ Some images are invalid'''
def testBlobSettingsBranchMultiple(self):
"""Test the 'branch multiple' policy"""
-bsettings.SetItem('global', 'allow-missing', 'branch multiple')
[PATCH v2 34/60] buildman: Add a test for --boards
Add a simple functional test for the --boards option. Fix the example in
the docs while we are here. Also improve the docs for Builder.count so it
is clearer what it contains.
Signed-off-by: Simon Glass
---
Changes in v2:
- Use snake case for tests
tools/buildman/builder.py | 3 ++-
tools/buildman/buildman.rst | 2 +-
tools/buildman/func_test.py | 19 +++
3 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index cb3628a8a085..620b7b8c31a8 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -163,7 +163,8 @@ class Builder:
checkout: True to check out source, False to skip that step.
This is used for testing.
col: terminal.Color() object
-count: Number of commits to build
+count: Total number of commits to build, which is the number of commits
+multiplied by the number of boards
do_make: Method to call to invoke Make
fail: Number of builds that failed due to error
force_build: Force building even if a build already exists
diff --git a/tools/buildman/buildman.rst b/tools/buildman/buildman.rst
index b28aea477dfe..d8c3957097e8 100644
--- a/tools/buildman/buildman.rst
+++ b/tools/buildman/buildman.rst
@@ -159,7 +159,7 @@ on the command line:
.. code-block:: bash
- buildman --boards sandbox,snow --boards
+ buildman --boards sandbox,snow --boards firefly-rk3399
It is convenient to use the -n option to see what will be built based on
the subset given. Use -v as well to get an actual list of boards.
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index 412718cc3db9..c0c35408c839 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -791,3 +791,22 @@ CONFIG_LOCALVERSION=y
result = self._RunControl('-A', 'board0')
self.assertEqual('arm-\n', stdout.getvalue())
self.assertEqual('', stderr.getvalue())
+
+def test_regen_boards(self):
+"""Test that we can regenerate the boards.cfg file"""
+outfile = os.path.join(self._output_dir, 'test-boards.cfg')
+if os.path.exists(outfile):
+os.remove(outfile)
+result = self._RunControl('-R', outfile, brds=None, get_builder=False)
+self.assertTrue(os.path.exists(outfile))
+
+def test_single_boards(self):
+"""Test building single boards"""
+self._RunControl('--boards', 'board1')
+self.assertEqual(1, self._builder.count)
+
+self._RunControl('--boards', 'board1', '--boards', 'board2')
+self.assertEqual(2, self._builder.count)
+
+self._RunControl('--boards', 'board1,board2', '--boards', 'board4')
+self.assertEqual(3, self._builder.count)
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 33/60] buildman: Correct most pylint warnings in cmdline
Tidu up warnings in this file.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/cmdline.py | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index 3066a0b38bbf..91571a5adb1d 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -2,6 +2,11 @@
# Copyright (c) 2014 Google, Inc
#
+"""Handles parsing of buildman arguments
+
+This creates the argument parser and uses it to parse the arguments passed in
+"""
+
from optparse import OptionParser
import os
import pathlib
@@ -71,7 +76,8 @@ def parse_args():
parser.add_option('-k', '--keep-outputs', action='store_true',
default=False, help='Keep all build output files (e.g. binaries)')
parser.add_option('-K', '--show-config', action='store_true',
- default=False, help='Show configuration changes in summary (both
board config files and Kconfig)')
+ default=False,
+ help='Show configuration changes in summary (both board config files
and Kconfig)')
parser.add_option('--preserve-config-y', action='store_true',
default=False, help="Don't convert y to 1 in configs")
parser.add_option('-l', '--list-error-boards', action='store_true',
@@ -84,14 +90,15 @@ def parse_args():
default=False, help="Run 'make mrproper before reconfiguring")
parser.add_option(
'-M', '--allow-missing', action='store_true', default=False,
- help='Tell binman to allow missing blobs and generate fake ones as
needed'),
+ help='Tell binman to allow missing blobs and generate fake ones as
needed')
parser.add_option(
'--no-allow-missing', action='store_true', default=False,
- help='Disable telling binman to allow missing blobs'),
+ help='Disable telling binman to allow missing blobs')
parser.add_option('-n', '--dry-run', action='store_true', dest='dry_run',
default=False, help="Do a dry run (describe actions, but do
nothing)")
parser.add_option('-N', '--no-subdirs', action='store_true',
dest='no_subdirs',
- default=False, help="Don't create subdirectories when building
current source for a single board")
+ default=False,
+ help="Don't create subdirectories when building current source for a
single board")
parser.add_option('-o', '--output-dir', type='string', dest='output_dir',
help='Directory where all builds happen and buildman has its
workspace (default is ../)')
parser.add_option('-O', '--override-toolchain', type='string',
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 32/60] buildman: Convert camel case in cmdline.py
Convert this file to snake case and update all files which use it. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/cmdline.py | 2 +- tools/buildman/func_test.py | 2 +- tools/buildman/main.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py index ea43685e394d..3066a0b38bbf 100644 --- a/tools/buildman/cmdline.py +++ b/tools/buildman/cmdline.py @@ -9,7 +9,7 @@ import pathlib BUILDMAN_DIR = pathlib.Path(__file__).parent HAS_TESTS = os.path.exists(BUILDMAN_DIR / "test.py") -def ParseArgs(): +def parse_args(): """Parse command line arguments from sys.argv[] Returns: diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py index 5dbbb53b6572..412718cc3db9 100644 --- a/tools/buildman/func_test.py +++ b/tools/buildman/func_test.py @@ -244,7 +244,7 @@ class TestFunctional(unittest.TestCase): result code from buildman """ sys.argv = [sys.argv[0]] + list(args) -options, args = cmdline.ParseArgs() +options, args = cmdline.parse_args() if brds == False: brds = self._boards result = control.do_buildman( diff --git a/tools/buildman/main.py b/tools/buildman/main.py index 2253401709a8..0f711c8653b3 100755 --- a/tools/buildman/main.py +++ b/tools/buildman/main.py @@ -59,7 +59,7 @@ def run_buildman(): This is the main program. It collects arguments and runs either the tests or the control module. """ -options, args = cmdline.ParseArgs() +options, args = cmdline.parse_args() if not options.debug: sys.tracebacklimit = 0 -- 2.41.0.255.g8b1d071c50-goog
[PATCH v2 31/60] buildman: Create a function to get number of built commits
Move this code into a function. This removes the last pylint error in
the control module.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/control.py | 34 +-
1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index c2ac56e43cc9..bf4843c4170d 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -29,7 +29,24 @@ def get_plural(count):
"""Returns a plural 's' if count is not 1"""
return 's' if count != 1 else ''
-def get_action_summary(is_summary, commits, selected, step, threads, jobs):
+
+def count_build_commits(commits, step):
+"""Calculate the number of commits to be built
+
+Args:
+commits (list of Commit): Commits to build or None
+step (int): Step value for commits, typically 1
+
+Returns:
+Number of commits that will be built
+"""
+if commits:
+count = len(commits)
+return (count + step - 1) // step
+return 0
+
+
+def get_action_summary(is_summary, commit_count, selected, threads, jobs):
"""Return a string summarising the intended action.
Args:
@@ -43,10 +60,8 @@ def get_action_summary(is_summary, commits, selected, step,
threads, jobs):
Returns:
Summary string.
"""
-if commits:
-count = len(commits)
-count = (count + step - 1) // step
-commit_str = f'{count} commit{get_plural(count)}'
+if commit_count:
+commit_str = f'{commit_count} commit{get_plural(commit_count)}'
else:
commit_str = 'current source'
msg = (f"{'Summary of' if is_summary else 'Building'} "
@@ -83,8 +98,8 @@ def show_actions(series, why_selected, boards_selected,
output_dir,
commits = series.commits
else:
commits = None
-print(get_action_summary(False, commits, boards_selected, step, threads,
- jobs))
+print(get_action_summary(False, count_build_commits(commits, step),
+ boards_selected, threads, jobs))
print(f'Build directory: {output_dir}')
if commits:
for upto in range(0, len(series.commits), step):
@@ -486,8 +501,9 @@ def run_builder(builder, commits, board_selected, options):
builder.gnu_make = gnu_make
if not options.ide:
-tprint(get_action_summary(options.summary, commits, board_selected,
- options.step, options.threads, options.jobs))
+commit_count = count_build_commits(commits, options.step)
+tprint(get_action_summary(options.summary, commit_count,
board_selected,
+ options.threads, options.jobs))
builder.SetDisplayOptions(
options.show_errors, options.show_sizes, options.show_detail,
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 30/60] buildman: Use get_alow_missing() directly to avoid var
Avoid an unnecessary local variable by moving this code to a function. This fixes the pylint warning about too many local variables. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/control.py | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 873b5aadfca8..c2ac56e43cc9 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -594,10 +594,6 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None, options.verbose) return 0 -allow_missing = get_allow_missing(options.allow_missing, - options.no_allow_missing, len(selected), - options.branch) - # Create a new builder with the selected options builder = Builder(toolchains, output_dir, git_dir, options.threads, options.jobs, checkout=True, @@ -613,7 +609,10 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None, test_thread_exceptions=test_thread_exceptions, adjust_cfg=calc_adjust_cfg(options.adjust_cfg, options.reproducible_builds), -allow_missing=allow_missing, no_lto=options.no_lto, +allow_missing=get_allow_missing(options.allow_missing, +options.no_allow_missing, +len(selected), options.branch), +no_lto=options.no_lto, reproducible_builds=options.reproducible_builds, force_build = options.force_build, force_build_failures = options.force_build_failures, -- 2.41.0.255.g8b1d071c50-goog
[PATCH v2 29/60] buildman: Move getting the adjust_cfg into run_builder()
Move this into its own function to reduce the size of do_buildman().
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/control.py | 38 +++---
1 file changed, 27 insertions(+), 11 deletions(-)
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 06d8e7b9527b..873b5aadfca8 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -507,6 +507,31 @@ def run_builder(builder, commits, board_selected, options):
return 101
return 0
+
+def calc_adjust_cfg(adjust_cfg, reproducible_builds):
+"""Calculate the value to use for adjust_cfg
+
+Args:
+adjust_cfg (list of str): List of configuration changes. See cfgutil
for
+details
+reproducible_builds (bool): True to adjust the configuration to get
+reproduceable builds
+
+Returns:
+adjust_cfg (list of str): List of configuration changes
+"""
+adjust_cfg = cfgutil.convert_list_to_dict(adjust_cfg)
+
+# Drop LOCALVERSION_AUTO since it changes the version string on every
commit
+if reproducible_builds:
+# If these are mentioned, leave the local version alone
+if 'LOCALVERSION' in adjust_cfg or 'LOCALVERSION_AUTO' in adjust_cfg:
+print('Not dropping LOCALVERSION_AUTO for reproducible build')
+else:
+adjust_cfg['LOCALVERSION_AUTO'] = '~'
+return adjust_cfg
+
+
def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
clean_dir=False, test_thread_exceptions=False):
"""The main control code for buildman
@@ -573,16 +598,6 @@ def do_buildman(options, args, toolchains=None,
make_func=None, brds=None,
options.no_allow_missing, len(selected),
options.branch)
-adjust_cfg = cfgutil.convert_list_to_dict(options.adjust_cfg)
-
-# Drop LOCALVERSION_AUTO since it changes the version string on every
commit
-if options.reproducible_builds:
-# If these are mentioned, leave the local version alone
-if 'LOCALVERSION' in adjust_cfg or 'LOCALVERSION_AUTO' in adjust_cfg:
-print('Not dropping LOCALVERSION_AUTO for reproducible build')
-else:
-adjust_cfg['LOCALVERSION_AUTO'] = '~'
-
# Create a new builder with the selected options
builder = Builder(toolchains, output_dir, git_dir,
options.threads, options.jobs, checkout=True,
@@ -596,7 +611,8 @@ def do_buildman(options, args, toolchains=None,
make_func=None, brds=None,
warnings_as_errors=options.warnings_as_errors,
work_in_output=options.work_in_output,
test_thread_exceptions=test_thread_exceptions,
-adjust_cfg=adjust_cfg,
+adjust_cfg=calc_adjust_cfg(options.adjust_cfg,
+ options.reproducible_builds),
allow_missing=allow_missing, no_lto=options.no_lto,
reproducible_builds=options.reproducible_builds,
force_build = options.force_build,
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 28/60] buildman: Move checking for make into run_builder()
This is not needed until the builder is run. Move it there to reduce the
size of the do_buildman() function.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/control.py | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 66fd04d90e44..06d8e7b9527b 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -465,6 +465,7 @@ def setup_output_dir(output_dir, work_in_output, branch,
no_subdirs, col,
shutil.rmtree(output_dir)
return output_dir
+
def run_builder(builder, commits, board_selected, options):
"""Run the builder or show the summary
@@ -478,6 +479,12 @@ def run_builder(builder, commits, board_selected, options):
Returns:
int: Return code for buildman
"""
+gnu_make = command.output(os.path.join(options.git,
+'scripts/show-gnu-make'), raise_on_error=False).rstrip()
+if not gnu_make:
+sys.exit('GNU Make not found')
+builder.gnu_make = gnu_make
+
if not options.ide:
tprint(get_action_summary(options.summary, commits, board_selected,
options.step, options.threads, options.jobs))
@@ -562,11 +569,6 @@ def do_buildman(options, args, toolchains=None,
make_func=None, brds=None,
options.verbose)
return 0
-gnu_make = command.output(os.path.join(options.git,
-'scripts/show-gnu-make'), raise_on_error=False).rstrip()
-if not gnu_make:
-sys.exit('GNU Make not found')
-
allow_missing = get_allow_missing(options.allow_missing,
options.no_allow_missing, len(selected),
options.branch)
@@ -583,7 +585,7 @@ def do_buildman(options, args, toolchains=None,
make_func=None, brds=None,
# Create a new builder with the selected options
builder = Builder(toolchains, output_dir, git_dir,
-options.threads, options.jobs, gnu_make=gnu_make, checkout=True,
+options.threads, options.jobs, checkout=True,
show_unknown=options.show_unknown, step=options.step,
no_subdirs=options.no_subdirs, full_path=options.full_path,
verbose_build=options.verbose_build,
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 27/60] buildman: Adjust show_toolchain_prefix() to not return
This function does not need to return. Simplify the code by exiting
immediately.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/control.py | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index b5181304d8f7..66fd04d90e44 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -121,10 +121,9 @@ def show_toolchain_prefix(brds, toolchains):
for brd in board_selected.values():
tc_set.add(toolchains.Select(brd.arch))
if len(tc_set) != 1:
-return 'Supplied boards must share one toolchain'
+sys.exit('Supplied boards must share one toolchain')
tchain = tc_set.pop()
print(tchain.GetEnvArgs(toolchain.VAR_CROSS_COMPILE))
-return None
def get_allow_missing(opt_allow, opt_no_allow, num_selected, has_branch):
"""Figure out whether to allow external blobs
@@ -548,9 +547,7 @@ def do_buildman(options, args, toolchains=None,
make_func=None, brds=None,
brds, args, col, options.boards, options.exclude)
if options.print_prefix:
-err = show_toolchain_prefix(brds, toolchains)
-if err:
-sys.exit(col.build(col.RED, err))
+show_toolchain_prefix(brds, toolchains)
return 0
series = determine_series(selected, col, git_dir, options.count,
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 24/60] buildman: Tweak commits and show_bloat
Move setting of show_bloat to adjust_options() and adjust how the commits variable is set. Together these remove the pylint warning about too many statements. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/control.py | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 83ce8141eae4..4f493006474e 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -433,6 +433,10 @@ def adjust_options(options, series, selected): if not options.step: options.step = len(series.commits) - 1 +# We can't show function sizes without board details at present +if options.show_bloat: +options.show_detail = True + def setup_output_dir(output_dir, work_in_output, branch, no_subdirs, col, clean_dir): @@ -572,18 +576,11 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None, # Work out which boards to build board_selected = brds.get_selected_dict() -if series: -commits = series.commits -else: -commits = None - +commits = series.commits if series else None if not options.ide: tprint(get_action_summary(options.summary, commits, board_selected, options.step, options.threads, options.jobs)) -# We can't show function sizes without board details at present -if options.show_bloat: -options.show_detail = True builder.SetDisplayOptions( options.show_errors, options.show_sizes, options.show_detail, options.show_bloat, options.list_error_boards, options.show_config, -- 2.41.0.255.g8b1d071c50-goog
[PATCH v2 26/60] buildman: Drop some unnecessary variables
Drop some variables at the end of the do_bulidman() function. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/control.py | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 84d672619d80..b5181304d8f7 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -607,9 +607,5 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None, TEST_BUILDER = builder -# Work out which boards to build -board_selected = brds.get_selected_dict() - -commits = series.commits if series else None -retval = run_builder(builder, commits, board_selected, options) -return retval +return run_builder(builder, series.commits if series else None, + brds.get_selected_dict(), options) -- 2.41.0.255.g8b1d071c50-goog
[PATCH v2 25/60] buildman: Moving running of the builder into a function
Move this code into a new function. This removes the pylint warning about too many branches. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/control.py | 56 --- 1 file changed, 35 insertions(+), 21 deletions(-) diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 4f493006474e..84d672619d80 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -466,6 +466,40 @@ def setup_output_dir(output_dir, work_in_output, branch, no_subdirs, col, shutil.rmtree(output_dir) return output_dir +def run_builder(builder, commits, board_selected, options): +"""Run the builder or show the summary + +Args: +commits (list of Commit): List of commits being built, None if no branch +boards_selected (dict): Dict of selected boards: +key: target name +value: Board object +options (Options): Options to use + +Returns: +int: Return code for buildman +""" +if not options.ide: +tprint(get_action_summary(options.summary, commits, board_selected, + options.step, options.threads, options.jobs)) + +builder.SetDisplayOptions( +options.show_errors, options.show_sizes, options.show_detail, +options.show_bloat, options.list_error_boards, options.show_config, +options.show_environment, options.filter_dtb_warnings, +options.filter_migration_warnings, options.ide) +if options.summary: +builder.ShowSummary(commits, board_selected) +else: +fail, warned, excs = builder.BuildBoards( +commits, board_selected, options.keep_outputs, options.verbose) +if excs: +return 102 +if fail: +return 100 +if warned and not options.ignore_warnings: +return 101 +return 0 def do_buildman(options, args, toolchains=None, make_func=None, brds=None, clean_dir=False, test_thread_exceptions=False): @@ -577,25 +611,5 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None, board_selected = brds.get_selected_dict() commits = series.commits if series else None -if not options.ide: -tprint(get_action_summary(options.summary, commits, board_selected, - options.step, options.threads, options.jobs)) - -builder.SetDisplayOptions( -options.show_errors, options.show_sizes, options.show_detail, -options.show_bloat, options.list_error_boards, options.show_config, -options.show_environment, options.filter_dtb_warnings, -options.filter_migration_warnings, options.ide) -retval = 0 -if options.summary: -builder.ShowSummary(commits, board_selected) -else: -fail, warned, excs = builder.BuildBoards( -commits, board_selected, options.keep_outputs, options.verbose) -if excs: -retval = 102 -if fail: -retval = 100 -if warned and not options.ignore_warnings: -retval = 101 +retval = run_builder(builder, commits, board_selected, options) return retval -- 2.41.0.255.g8b1d071c50-goog
[PATCH v2 23/60] buildman: Move remaining builder properties to constructor
Do these all in the constructor, so it is consistent. Move the stray builder comment into the correct place. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/builder.py | 25 ++--- tools/buildman/control.py | 18 +++--- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index d81752e99438..cb3628a8a085 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -255,7 +255,10 @@ class Builder: config_only=False, squash_config_y=False, warnings_as_errors=False, work_in_output=False, test_thread_exceptions=False, adjust_cfg=None, - allow_missing=False, no_lto=False, reproducible_builds=False): + allow_missing=False, no_lto=False, reproducible_builds=False, + force_build=False, force_build_failures=False, + force_reconfig=False, in_tree=False, + force_config_on_failure=False, make_func=None): """Create a new Builder object Args: @@ -295,7 +298,14 @@ class Builder: a string Kconfig allow_missing: Run build with BINMAN_ALLOW_MISSING=1 no_lto (bool): True to set the NO_LTO flag when building - +force_build (bool): Rebuild even commits that are already built +force_build_failures (bool): Rebuild commits that have not been +built, or failed to build +force_reconfig (bool): Reconfigure on each commit +in_tree (bool): Bulid in tree instead of out-of-tree +force_config_on_failure (bool): Reconfigure the build before +retrying a failed build +make_func (function): Function to call to run 'make' """ self.toolchains = toolchains self.base_dir = base_dir @@ -304,7 +314,7 @@ class Builder: else: self._working_dir = os.path.join(base_dir, '.bm-work') self.threads = [] -self.do_make = self.Make +self.do_make = make_func or self.Make self.gnu_make = gnu_make self.checkout = checkout self.num_threads = num_threads @@ -318,11 +328,7 @@ class Builder: self._complete_delay = None self._next_delay_update = datetime.now() self._start_time = datetime.now() -self.force_config_on_failure = True -self.force_build_failures = False -self.force_reconfig = False self._step = step -self.in_tree = False self._error_lines = 0 self.no_subdirs = no_subdirs self.full_path = full_path @@ -336,6 +342,11 @@ class Builder: self._ide = False self.no_lto = no_lto self.reproducible_builds = reproducible_builds +self.force_build = force_build +self.force_build_failures = force_build_failures +self.force_reconfig = force_reconfig +self.in_tree = in_tree +self.force_config_on_failure = force_config_on_failure if not self.squash_config_y: self.config_filenames += EXTRA_CONFIG_FILENAMES diff --git a/tools/buildman/control.py b/tools/buildman/control.py index b0d3cf85c52c..83ce8141eae4 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -536,8 +536,6 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None, options.no_allow_missing, len(selected), options.branch) -# Create a new builder with the selected options. - adjust_cfg = cfgutil.convert_list_to_dict(options.adjust_cfg) # Drop LOCALVERSION_AUTO since it changes the version string on every commit @@ -548,6 +546,7 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None, else: adjust_cfg['LOCALVERSION_AUTO'] = '~' +# Create a new builder with the selected options builder = Builder(toolchains, output_dir, git_dir, options.threads, options.jobs, gnu_make=gnu_make, checkout=True, show_unknown=options.show_unknown, step=options.step, @@ -562,16 +561,13 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None, test_thread_exceptions=test_thread_exceptions, adjust_cfg=adjust_cfg, allow_missing=allow_missing, no_lto=options.no_lto, -reproducible_builds=options.reproducible_builds) +reproducible_builds=options.reproducible_builds, +force_build = options.force_build, +force_build_failures = options.force_build_failures, +force_reconfig = options.force_reconfig, in_tree = options.in_tree, +force_config_on_failure=not options.quick, make_func=make_func) + TEST_BUILDER = builder -builder.force_config_on_failure = not options.quick -if m
[PATCH v2 20/60] buildman: Move setting up the output dir into a function
Move this code into a separate function to reduce the size of the main
do_buildman() directory.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/control.py | 45 ---
1 file changed, 33 insertions(+), 12 deletions(-)
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 25142a46066b..9ff9f20aa128 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -426,6 +426,36 @@ def adjust_options(options, series, selected):
if not options.step:
options.step = len(series.commits) - 1
+
+def setup_output_dir(output_dir, work_in_output, branch, no_subdirs, col,
+ clean_dir):
+"""Set up the output directory
+
+Args:
+output_dir (str): Output directory provided by the user, or None if
none
+work_in_output (bool): True to work in the output directory
+branch (str): Name of branch to build, or None if none
+no_subdirs (bool): True to put the output in the top-level output dir
+clean_dir: Used for tests only, indicates that the existing output_dir
+should be removed before starting the build
+
+Returns:
+str: Updated output directory pathname
+"""
+if not output_dir:
+if work_in_output:
+sys.exit(col.build(col.RED, '-w requires that you specify -o'))
+output_dir = '..'
+if branch and not no_subdirs:
+# As a special case allow the board directory to be placed in the
+# output directory itself rather than any subdirectory.
+dirname = branch.replace('/', '_')
+output_dir = os.path.join(output_dir, dirname)
+if clean_dir and os.path.exists(output_dir):
+shutil.rmtree(output_dir)
+return output_dir
+
+
def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
clean_dir=False, test_thread_exceptions=False):
"""The main control code for buildman
@@ -458,18 +488,9 @@ def do_buildman(options, args, toolchains=None,
make_func=None, brds=None,
toolchains = get_toolchains(toolchains, col, options.override_toolchain,
options.fetch_arch, options.list_tool_chains,
options.verbose)
-output_dir = options.output_dir
-if not output_dir:
-if options.work_in_output:
-sys.exit(col.build(col.RED, '-w requires that you specify -o'))
-output_dir = '..'
-if options.branch and not options.no_subdirs:
-# As a special case allow the board directory to be placed in the
-# output directory itself rather than any subdirectory.
-dirname = options.branch.replace('/', '_')
-output_dir = os.path.join(output_dir, dirname)
-if clean_dir and os.path.exists(output_dir):
-shutil.rmtree(output_dir)
+output_dir = setup_output_dir(
+options.output_dir, options.work_in_output, options.branch,
+options.no_subdirs, col, clean_dir)
# Work out what subset of the boards we are building
if not brds:
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 22/60] buildman: Avoid too many returns in do_buildman()
Fix the pylint warning by using a variable instead of lots of 'return' statements. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/control.py | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tools/buildman/control.py b/tools/buildman/control.py index a38798af2138..b0d3cf85c52c 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -593,15 +593,16 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None, options.show_bloat, options.list_error_boards, options.show_config, options.show_environment, options.filter_dtb_warnings, options.filter_migration_warnings, options.ide) +retval = 0 if options.summary: builder.ShowSummary(commits, board_selected) else: fail, warned, excs = builder.BuildBoards( commits, board_selected, options.keep_outputs, options.verbose) if excs: -return 102 +retval = 102 if fail: -return 100 +retval = 100 if warned and not options.ignore_warnings: -return 101 -return 0 +retval = 101 +return retval -- 2.41.0.255.g8b1d071c50-goog
[PATCH v2 21/60] buildman: Move commit numbering into determine_series()
Commits are numbered for use in tests. Do this in determine_series() since it is already dealing with the series. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/control.py | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 9ff9f20aa128..a38798af2138 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -202,6 +202,9 @@ def count_commits(branch, count, col, git_dir): def determine_series(selected, col, git_dir, count, branch, work_in_output): """Determine the series which is to be built, if any +If there is a series, the commits in that series are numbered by setting +their sequence value (starting from 0). This is used by tests. + Args: selected (list of Board): List of Board objects that are marked selected @@ -254,6 +257,10 @@ def determine_series(selected, col, git_dir, count, branch, work_in_output): # Honour the count series = patchstream.get_metadata_for_list(branch, git_dir, count, series=None, allow_overwrite=True) + +# Number the commits for test purposes +for i, commit in enumerate(series.commits): +commit.sequence = i else: series = None return series @@ -571,9 +578,6 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None, if series: commits = series.commits -# Number the commits for test purposes -for i, commit in enumerate(commits): -commit.sequence = i else: commits = None -- 2.41.0.255.g8b1d071c50-goog
[PATCH v2 19/60] buildman: Move counting of commits into a function
Move this code into a separate function to avoid a pylint warning in
determine_series().
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/control.py | 63 +--
1 file changed, 41 insertions(+), 22 deletions(-)
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 0479dfd17d0c..25142a46066b 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -159,6 +159,46 @@ def get_allow_missing(opt_allow, opt_no_allow,
num_selected, has_branch):
return allow_missing
+def count_commits(branch, count, col, git_dir):
+"""Could the number of commits in the branch/ranch being built
+
+Args:
+branch (str): Name of branch to build, or None if none
+count (int): Number of commits to build, or -1 for all
+col (Terminal.Color): Color object to use
+git_dir (str): Git directory to use, e.g. './.git'
+
+Returns:
+tuple:
+Number of commits being built
+True if the 'branch' string contains a range rather than a simple
+name
+"""
+has_range = branch and '..' in branch
+if count == -1:
+if not branch:
+count = 1
+else:
+if has_range:
+count, msg = gitutil.count_commits_in_range(git_dir, branch)
+else:
+count, msg = gitutil.count_commits_in_branch(git_dir, branch)
+if count is None:
+sys.exit(col.build(col.RED, msg))
+elif count == 0:
+sys.exit(col.build(col.RED,
+ f"Range '{branch}' has no commits"))
+if msg:
+print(col.build(col.YELLOW, msg))
+count += 1 # Build upstream commit also
+
+if not count:
+msg = (f"No commits found to process in branch '{branch}': "
+ "set branch's upstream or use -c flag")
+sys.exit(col.build(col.RED, msg))
+return count, has_range
+
+
def determine_series(selected, col, git_dir, count, branch, work_in_output):
"""Determine the series which is to be built, if any
@@ -189,28 +229,7 @@ def determine_series(selected, col, git_dir, count,
branch, work_in_output):
# Work out how many commits to build. We want to build everything on the
# branch. We also build the upstream commit as a control so we can see
# problems introduced by the first commit on the branch.
-has_range = branch and '..' in branch
-if count == -1:
-if not branch:
-count = 1
-else:
-if has_range:
-count, msg = gitutil.count_commits_in_range(git_dir, branch)
-else:
-count, msg = gitutil.count_commits_in_branch(git_dir, branch)
-if count is None:
-sys.exit(col.build(col.RED, msg))
-elif count == 0:
-sys.exit(col.build(col.RED,
- f"Range '{branch}' has no commits"))
-if msg:
-print(col.build(col.YELLOW, msg))
-count += 1 # Build upstream commit also
-
-if not count:
-msg = (f"No commits found to process in branch '{branch}': "
- "set branch's upstream or use -c flag")
-sys.exit(col.build(col.RED, msg))
+count, has_range = count_commits(branch, count, col, git_dir)
if work_in_output:
if len(selected) != 1:
sys.exit(col.build(col.RED,
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 18/60] buildman: Build option-adjusting into a function
Create a separate function to adjust options. Also move show_actions() up as far as we can in the function. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/control.py | 53 --- 1 file changed, 33 insertions(+), 20 deletions(-) diff --git a/tools/buildman/control.py b/tools/buildman/control.py index af3f2a6a0c42..0479dfd17d0c 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -381,6 +381,32 @@ def determine_boards(brds, args, col, opt_boards, exclude): return selected, why_selected, board_warnings +def adjust_options(options, series, selected): +"""Adjust options according to various constraints + +Updates verbose, show_errors, threads, jobs and step + +Args: +options (Options): Options object to adjust +series (Series): Series being built / summarised +selected (list of Board): List of Board objects that are marked +""" +if not series and not options.dry_run: +options.verbose = True +if not options.summary: +options.show_errors = True + +# By default we have one thread per CPU. But if there are not enough jobs +# we can have fewer threads and use a high '-j' value for make. +if options.threads is None: +options.threads = min(multiprocessing.cpu_count(), len(selected)) +if not options.jobs: +options.jobs = max(1, (multiprocessing.cpu_count() + +len(selected) - 1) // len(selected)) + +if not options.step: +options.step = len(series.commits) - 1 + def do_buildman(options, args, toolchains=None, make_func=None, brds=None, clean_dir=False, test_thread_exceptions=False): """The main control code for buildman @@ -444,21 +470,15 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None, series = determine_series(selected, col, git_dir, options.count, options.branch, options.work_in_output) -if not series and not options.dry_run: -options.verbose = True -if not options.summary: -options.show_errors = True -# By default we have one thread per CPU. But if there are not enough jobs -# we can have fewer threads and use a high '-j' value for make. -if options.threads is None: -options.threads = min(multiprocessing.cpu_count(), len(selected)) -if not options.jobs: -options.jobs = max(1, (multiprocessing.cpu_count() + -len(selected) - 1) // len(selected)) +adjust_options(options, series, selected) -if not options.step: -options.step = len(series.commits) - 1 +# For a dry run, just show our actions as a sanity check +if options.dry_run: +show_actions(series, why_selected, selected, output_dir, board_warnings, + options.step, options.threads, options.jobs, + options.verbose) +return 0 gnu_make = command.output(os.path.join(options.git, 'scripts/show-gnu-make'), raise_on_error=False).rstrip() @@ -471,13 +491,6 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None, # Create a new builder with the selected options. -# For a dry run, just show our actions as a sanity check -if options.dry_run: -show_actions(series, why_selected, selected, output_dir, board_warnings, - options.step, options.threads, options.jobs, - options.verbose) -return 0 - adjust_cfg = cfgutil.convert_list_to_dict(options.adjust_cfg) # Drop LOCALVERSION_AUTO since it changes the version string on every commit -- 2.41.0.255.g8b1d071c50-goog
[PATCH v2 17/60] buildman: Pass option values to show_actions()
Pass in the individual values rather than the whole options object, so we
can see what is needed.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/control.py | 22 +-
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index e5e807b87390..af3f2a6a0c42 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -56,8 +56,8 @@ def get_action_summary(is_summary, commits, selected, step,
threads, jobs):
return msg
# pylint: disable=R0913
-def show_actions(series, why_selected, boards_selected, output_dir, options,
- board_warnings):
+def show_actions(series, why_selected, boards_selected, output_dir,
+ board_warnings, step, threads, jobs, verbose):
"""Display a list of actions that we would take, if not a dry run.
Args:
@@ -70,8 +70,11 @@ def show_actions(series, why_selected, boards_selected,
output_dir, options,
boards_selected: Dict of selected boards, key is target name,
value is Board object
output_dir (str): Output directory for builder
-options: Command line options object
board_warnings: List of warnings obtained from board selected
+step (int): Step increment through commits
+threads (int): Number of processor threads being used
+jobs (int): Number of jobs to build at once
+verbose (bool): True to indicate why each board was selected
"""
col = terminal.Color()
print('Dry run, so not doing much. But I would do this:')
@@ -80,11 +83,11 @@ def show_actions(series, why_selected, boards_selected,
output_dir, options,
commits = series.commits
else:
commits = None
-print(get_action_summary(False, commits, boards_selected,
- options.step, options.threads, options.jobs))
+print(get_action_summary(False, commits, boards_selected, step, threads,
+ jobs))
print(f'Build directory: {output_dir}')
if commits:
-for upto in range(0, len(series.commits), options.step):
+for upto in range(0, len(series.commits), step):
commit = series.commits[upto]
print(' ', col.build(col.YELLOW, commit.hash[:8], bright=False),
end=' ')
print(commit.subject)
@@ -92,7 +95,7 @@ def show_actions(series, why_selected, boards_selected,
output_dir, options,
for arg in why_selected:
if arg != 'all':
print(arg, f': {len(why_selected[arg])} boards')
-if options.verbose:
+if verbose:
print(f" {' '.join(why_selected[arg])}")
print('Total boards to build for each '
f"commit: {len(why_selected['all'])}\n")
@@ -470,8 +473,9 @@ def do_buildman(options, args, toolchains=None,
make_func=None, brds=None,
# For a dry run, just show our actions as a sanity check
if options.dry_run:
-show_actions(series, why_selected, selected, output_dir, options,
-board_warnings)
+show_actions(series, why_selected, selected, output_dir,
board_warnings,
+ options.step, options.threads, options.jobs,
+ options.verbose)
return 0
adjust_cfg = cfgutil.convert_list_to_dict(options.adjust_cfg)
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 16/60] buildman: Pass option values to get_action_summary()
Pass in the individual values rather than the whole options object, so we
can see what is needed.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/control.py | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 7759784b22a9..e5e807b87390 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -29,22 +29,30 @@ def get_plural(count):
"""Returns a plural 's' if count is not 1"""
return 's' if count != 1 else ''
-def get_action_summary(is_summary, commits, selected, options):
+def get_action_summary(is_summary, commits, selected, step, threads, jobs):
"""Return a string summarising the intended action.
+Args:
+is_summary (bool): True if this is a summary (otherwise it is building)
+commits (list): List of commits being built
+selected (list of Board): List of Board objects that are marked
+step (int): Step increment through commits
+threads (int): Number of processor threads being used
+jobs (int): Number of jobs to build at once
+
Returns:
Summary string.
"""
if commits:
count = len(commits)
-count = (count + options.step - 1) // options.step
+count = (count + step - 1) // step
commit_str = f'{count} commit{get_plural(count)}'
else:
commit_str = 'current source'
msg = (f"{'Summary of' if is_summary else 'Building'} "
f'{commit_str} for {len(selected)} boards')
-msg += (f' ({options.threads} thread{get_plural(options.threads)}, '
-f'{options.jobs} job{get_plural(options.jobs)} per thread)')
+msg += (f' ({threads} thread{get_plural(threads)}, '
+f'{jobs} job{get_plural(jobs)} per thread)')
return msg
# pylint: disable=R0913
@@ -73,7 +81,7 @@ def show_actions(series, why_selected, boards_selected,
output_dir, options,
else:
commits = None
print(get_action_summary(False, commits, boards_selected,
-options))
+ options.step, options.threads, options.jobs))
print(f'Build directory: {output_dir}')
if commits:
for upto in range(0, len(series.commits), options.step):
@@ -152,7 +160,7 @@ def determine_series(selected, col, git_dir, count, branch,
work_in_output):
"""Determine the series which is to be built, if any
Args:
-selected (list of Board(: List of Board objects that are marked
+selected (list of Board): List of Board objects that are marked
selected
col (Terminal.Color): Color object to use
git_dir (str): Git directory to use, e.g. './.git'
@@ -514,7 +522,7 @@ def do_buildman(options, args, toolchains=None,
make_func=None, brds=None,
if not options.ide:
tprint(get_action_summary(options.summary, commits, board_selected,
-options))
+ options.step, options.threads, options.jobs))
# We can't show function sizes without board details at present
if options.show_bloat:
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 15/60] buildman: Move output-file setup into one place
Collect the two parts of the output-file handling into single place.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/control.py | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 7c356ec5020e..7759784b22a9 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -407,6 +407,13 @@ def do_buildman(options, args, toolchains=None,
make_func=None, brds=None,
if options.work_in_output:
sys.exit(col.build(col.RED, '-w requires that you specify -o'))
output_dir = '..'
+if options.branch and not options.no_subdirs:
+# As a special case allow the board directory to be placed in the
+# output directory itself rather than any subdirectory.
+dirname = options.branch.replace('/', '_')
+output_dir = os.path.join(output_dir, dirname)
+if clean_dir and os.path.exists(output_dir):
+shutil.rmtree(output_dir)
# Work out what subset of the boards we are building
if not brds:
@@ -452,14 +459,6 @@ def do_buildman(options, args, toolchains=None,
make_func=None, brds=None,
options.branch)
# Create a new builder with the selected options.
-if options.branch:
-dirname = options.branch.replace('/', '_')
-# As a special case allow the board directory to be placed in the
-# output directory itself rather than any subdirectory.
-if not options.no_subdirs:
-output_dir = os.path.join(output_dir, dirname)
-if clean_dir and os.path.exists(output_dir):
-shutil.rmtree(output_dir)
# For a dry run, just show our actions as a sanity check
if options.dry_run:
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 14/60] bulldman: Set up output_dir earlier
Set up output_dir at the start of the main function, instead of updating
the options.output_dir option.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/control.py | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index f319dc8724e2..7c356ec5020e 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -402,14 +402,15 @@ def do_buildman(options, args, toolchains=None,
make_func=None, brds=None,
toolchains = get_toolchains(toolchains, col, options.override_toolchain,
options.fetch_arch, options.list_tool_chains,
options.verbose)
-if not options.output_dir:
+output_dir = options.output_dir
+if not output_dir:
if options.work_in_output:
sys.exit(col.build(col.RED, '-w requires that you specify -o'))
-options.output_dir = '..'
+output_dir = '..'
# Work out what subset of the boards we are building
if not brds:
-brds = get_boards_obj(options.output_dir, options.regen_board_list,
+brds = get_boards_obj(output_dir, options.regen_board_list,
options.threads, options.verbose)
if isinstance(brds, int):
return brds
@@ -451,13 +452,12 @@ def do_buildman(options, args, toolchains=None,
make_func=None, brds=None,
options.branch)
# Create a new builder with the selected options.
-output_dir = options.output_dir
if options.branch:
dirname = options.branch.replace('/', '_')
# As a special case allow the board directory to be placed in the
# output directory itself rather than any subdirectory.
if not options.no_subdirs:
-output_dir = os.path.join(options.output_dir, dirname)
+output_dir = os.path.join(output_dir, dirname)
if clean_dir and os.path.exists(output_dir):
shutil.rmtree(output_dir)
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 13/60] bulidman: Move toolchain handling to a function
Move the code for dealing with toolchains out into its own function, to reduce the size of the main function. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/control.py | 53 --- 1 file changed, 38 insertions(+), 15 deletions(-) diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 5c01ea8e1743..f319dc8724e2 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -259,6 +259,41 @@ def do_fetch_arch(toolchains, col, fetch_arch): return 0 +def get_toolchains(toolchains, col, override_toolchain, fetch_arch, + list_tool_chains, verbose): +"""Get toolchains object to use + +Args: +toolchains (Toolchains or None): Toolchains to use. If None, then a +Toolchains object will be created and scanned +col (Terminal.Color): Color object +override_toolchain (str or None): Override value for toolchain, or None +fetch_arch (bool): True to fetch the toolchain for the architectures +list_tool_chains (bool): True to list all tool chains +verbose (bool): True for verbose output when listing toolchains + +Returns: +Either: +int: Operation completed and buildman should exit with exit code +Toolchains: Toolchains object to use +""" +no_toolchains = toolchains is None +if no_toolchains: +toolchains = toolchain.Toolchains(override_toolchain) + +if fetch_arch: +return do_fetch_arch(toolchains, col, fetch_arch) + +if no_toolchains: +toolchains.GetSettings() +toolchains.Scan(list_tool_chains and verbose) +if list_tool_chains: +toolchains.List() +print() +return 0 +return toolchains + + def get_boards_obj(output_dir, regen_board_list, threads, verbose): """Object the Boards object to use @@ -364,21 +399,9 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None, git_dir = os.path.join(options.git, '.git') -no_toolchains = toolchains is None -if no_toolchains: -toolchains = toolchain.Toolchains(options.override_toolchain) - -if options.fetch_arch: -return do_fetch_arch(toolchains, col, options.fetch_arch) - -if no_toolchains: -toolchains.GetSettings() -toolchains.Scan(options.list_tool_chains and options.verbose) -if options.list_tool_chains: -toolchains.List() -print() -return 0 - +toolchains = get_toolchains(toolchains, col, options.override_toolchain, +options.fetch_arch, options.list_tool_chains, +options.verbose) if not options.output_dir: if options.work_in_output: sys.exit(col.build(col.RED, '-w requires that you specify -o')) -- 2.41.0.255.g8b1d071c50-goog
[PATCH v2 12/60] buildman: Move Boards-object code into a function
Move the code which obtains a Boards object into its own function, to reduce the size of the main function. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/control.py | 54 --- 1 file changed, 39 insertions(+), 15 deletions(-) diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 5bc2ab9063f2..5c01ea8e1743 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -259,6 +259,41 @@ def do_fetch_arch(toolchains, col, fetch_arch): return 0 +def get_boards_obj(output_dir, regen_board_list, threads, verbose): +"""Object the Boards object to use + +Creates the output directory and ensures there is a boards.cfg file, then +read it in. + +Args: +output_dir (str): Output directory to use +regen_board_list (bool): True to just regenerate the board list +threads (int or None): Number of threads to use to create boards file +verbose (bool): False to suppress output from boards-file generation + +Returns: +Either: +int: Operation completed and buildman should exit with exit code +Boards: Boards object to use +""" +if not os.path.exists(output_dir): +os.makedirs(output_dir) +board_file = os.path.join(output_dir, 'boards.cfg') +if regen_board_list and regen_board_list != '-': +board_file = regen_board_list + +brds = boards.Boards() +okay = brds.ensure_board_list( +board_file, +threads or multiprocessing.cpu_count(), +force=regen_board_list, +quiet=not verbose) +if regen_board_list: +return 0 if okay else 2 +brds.read_boards(board_file) +return brds + + def determine_boards(brds, args, col, opt_boards, exclude): """Determine which boards to build @@ -351,21 +386,10 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None, # Work out what subset of the boards we are building if not brds: -if not os.path.exists(options.output_dir): -os.makedirs(options.output_dir) -board_file = os.path.join(options.output_dir, 'boards.cfg') -if options.regen_board_list and options.regen_board_list != '-': -board_file = options.regen_board_list - -brds = boards.Boards() -okay = brds.ensure_board_list( -board_file, -options.threads or multiprocessing.cpu_count(), -force=options.regen_board_list, -quiet=not options.verbose) -if options.regen_board_list: -return 0 if okay else 2 -brds.read_boards(board_file) +brds = get_boards_obj(options.output_dir, options.regen_board_list, + options.threads, options.verbose) +if isinstance(brds, int): +return brds selected, why_selected, board_warnings = determine_boards( brds, args, col, options.boards, options.exclude) -- 2.41.0.255.g8b1d071c50-goog
[PATCH v2 11/60] bulidman: Move more code to determine_series()
Move some more series-related code here, to reduce the size of the main
function.
Signed-off-by: Simon Glass
---
Changes in v2:
- Correct operation of -A
tools/buildman/control.py | 82 ---
1 file changed, 42 insertions(+), 40 deletions(-)
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index b94e066d1bdf..5bc2ab9063f2 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -148,14 +148,17 @@ def get_allow_missing(opt_allow, opt_no_allow,
num_selected, has_branch):
return allow_missing
-def determine_series(count, has_range, branch, git_dir):
+def determine_series(selected, col, git_dir, count, branch, work_in_output):
"""Determine the series which is to be built, if any
Args:
+selected (list of Board(: List of Board objects that are marked
+selected
+col (Terminal.Color): Color object to use
+git_dir (str): Git directory to use, e.g. './.git'
count (int): Number of commits in branch
-has_range (bool): True if a range of commits ('xx..yy') is being built
branch (str): Name of branch to build, or None if none
-git_dir (str): Git directory to use, e.g. './.git'
+work_in_output (bool): True to work in the output directory
Returns:
Series: Series to build, or None for none
@@ -171,6 +174,40 @@ def determine_series(count, has_range, branch, git_dir):
other hand conflicting tags will cause an error. So allow later tags
to overwrite earlier ones by setting allow_overwrite=True
"""
+
+# Work out how many commits to build. We want to build everything on the
+# branch. We also build the upstream commit as a control so we can see
+# problems introduced by the first commit on the branch.
+has_range = branch and '..' in branch
+if count == -1:
+if not branch:
+count = 1
+else:
+if has_range:
+count, msg = gitutil.count_commits_in_range(git_dir, branch)
+else:
+count, msg = gitutil.count_commits_in_branch(git_dir, branch)
+if count is None:
+sys.exit(col.build(col.RED, msg))
+elif count == 0:
+sys.exit(col.build(col.RED,
+ f"Range '{branch}' has no commits"))
+if msg:
+print(col.build(col.YELLOW, msg))
+count += 1 # Build upstream commit also
+
+if not count:
+msg = (f"No commits found to process in branch '{branch}': "
+ "set branch's upstream or use -c flag")
+sys.exit(col.build(col.RED, msg))
+if work_in_output:
+if len(selected) != 1:
+sys.exit(col.build(col.RED,
+ '-w can only be used with a single board'))
+if count != 1:
+sys.exit(col.build(col.RED,
+ '-w can only be used with a single commit'))
+
if branch:
if count == -1:
if has_range:
@@ -339,43 +376,8 @@ def do_buildman(options, args, toolchains=None,
make_func=None, brds=None,
sys.exit(col.build(col.RED, err))
return 0
-# Work out how many commits to build. We want to build everything on the
-# branch. We also build the upstream commit as a control so we can see
-# problems introduced by the first commit on the branch.
-count = options.count
-has_range = options.branch and '..' in options.branch
-if count == -1:
-if not options.branch:
-count = 1
-else:
-if has_range:
-count, msg = gitutil.count_commits_in_range(git_dir,
-options.branch)
-else:
-count, msg = gitutil.count_commits_in_branch(git_dir,
- options.branch)
-if count is None:
-sys.exit(col.build(col.RED, msg))
-elif count == 0:
-sys.exit(col.build(col.RED,
- f"Range '{options.branch}' has no commits"))
-if msg:
-print(col.build(col.YELLOW, msg))
-count += 1 # Build upstream commit also
-
-if not count:
-msg = (f"No commits found to process in branch '{options.branch}': "
- "set branch's upstream or use -c flag")
-sys.exit(col.build(col.RED, msg))
-if options.work_in_output:
-if len(selected) != 1:
-sys.exit(col.build(col.RED,
- '-w can only be used with a single board'))
-if count != 1:
-sys.exit(col.build(col.RED,
- '-w can only be used with a single commit'))
-
-series = determine_series(count, has_range, options.branch, git_dir)
+series = determine_series(selected, col, git_di
[PATCH v2 10/60] buildman: Move board-selection code into a function
Create a new determine_boards() function to hold the code which selects
which boards to build.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/control.py | 59 ---
1 file changed, 43 insertions(+), 16 deletions(-)
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index c45a28cce3c4..b94e066d1bdf 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -222,6 +222,47 @@ def do_fetch_arch(toolchains, col, fetch_arch):
return 0
+def determine_boards(brds, args, col, opt_boards, exclude):
+"""Determine which boards to build
+
+Each element of args and exclude can refer to a board name, arch or SoC
+
+Args:
+brds (Boards): Boards object
+args (list of str): Arguments describing boards to build
+col (Terminal.Color): Color object
+opt_boards (list of str): Specific boards to build, or None for all
+exclude (list of str): Arguments describing boards to exclude
+
+Returns:
+tuple:
+list of Board: List of Board objects that are marked selected
+why_selected: Dictionary where each key is a buildman argument
+provided by the user, and the value is the list of boards
+brought in by that argument. For example, 'arm' might bring
+in 400 boards, so in this case the key would be 'arm' and
+the value would be a list of board names.
+board_warnings: List of warnings obtained from board selected
+"""
+exclude = []
+if exclude:
+for arg in exclude:
+exclude += arg.split(',')
+
+if opt_boards:
+requested_boards = []
+for brd in opt_boards:
+requested_boards += brd.split(',')
+else:
+requested_boards = None
+why_selected, board_warnings = brds.select_boards(args, exclude,
+ requested_boards)
+selected = brds.get_selected()
+if not selected:
+sys.exit(col.build(col.RED, 'No matching boards found'))
+return selected, why_selected, board_warnings
+
+
def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
clean_dir=False, test_thread_exceptions=False):
"""The main control code for buildman
@@ -289,22 +330,8 @@ def do_buildman(options, args, toolchains=None,
make_func=None, brds=None,
return 0 if okay else 2
brds.read_boards(board_file)
-exclude = []
-if options.exclude:
-for arg in options.exclude:
-exclude += arg.split(',')
-
-if options.boards:
-requested_boards = []
-for brd in options.boards:
-requested_boards += brd.split(',')
-else:
-requested_boards = None
-why_selected, board_warnings = brds.select_boards(args, exclude,
- requested_boards)
-selected = brds.get_selected()
-if not selected:
-sys.exit(col.build(col.RED, 'No matching boards found'))
+selected, why_selected, board_warnings = determine_boards(
+brds, args, col, options.boards, options.exclude)
if options.print_prefix:
err = show_toolchain_prefix(brds, toolchains)
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 09/60] buildman: Move dry-run handling higher in do_buildman()
Move this up above where the builder is created, since it no-longer makes use of the builder. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/control.py | 88 --- 1 file changed, 45 insertions(+), 43 deletions(-) diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 7beb5bce6444..c45a28cce3c4 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -384,6 +384,13 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None, output_dir = os.path.join(options.output_dir, dirname) if clean_dir and os.path.exists(output_dir): shutil.rmtree(output_dir) + +# For a dry run, just show our actions as a sanity check +if options.dry_run: +show_actions(series, why_selected, selected, output_dir, options, +board_warnings) +return 0 + adjust_cfg = cfgutil.convert_list_to_dict(options.adjust_cfg) # Drop LOCALVERSION_AUTO since it changes the version string on every commit @@ -414,48 +421,43 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None, if make_func: builder.do_make = make_func -# For a dry run, just show our actions as a sanity check -if options.dry_run: -show_actions(series, why_selected, selected, output_dir, options, -board_warnings) +builder.force_build = options.force_build +builder.force_build_failures = options.force_build_failures +builder.force_reconfig = options.force_reconfig +builder.in_tree = options.in_tree + +# Work out which boards to build +board_selected = brds.get_selected_dict() + +if series: +commits = series.commits +# Number the commits for test purposes +for i, commit in enumerate(commits): +commit.sequence = i else: -builder.force_build = options.force_build -builder.force_build_failures = options.force_build_failures -builder.force_reconfig = options.force_reconfig -builder.in_tree = options.in_tree - -# Work out which boards to build -board_selected = brds.get_selected_dict() - -if series: -commits = series.commits -# Number the commits for test purposes -for i, commit in enumerate(commits): -commit.sequence = i -else: -commits = None - -if not options.ide: -tprint(get_action_summary(options.summary, commits, board_selected, -options)) - -# We can't show function sizes without board details at present -if options.show_bloat: -options.show_detail = True -builder.SetDisplayOptions( -options.show_errors, options.show_sizes, options.show_detail, -options.show_bloat, options.list_error_boards, options.show_config, -options.show_environment, options.filter_dtb_warnings, -options.filter_migration_warnings, options.ide) -if options.summary: -builder.ShowSummary(commits, board_selected) -else: -fail, warned, excs = builder.BuildBoards( -commits, board_selected, options.keep_outputs, options.verbose) -if excs: -return 102 -if fail: -return 100 -if warned and not options.ignore_warnings: -return 101 +commits = None + +if not options.ide: +tprint(get_action_summary(options.summary, commits, board_selected, +options)) + +# We can't show function sizes without board details at present +if options.show_bloat: +options.show_detail = True +builder.SetDisplayOptions( +options.show_errors, options.show_sizes, options.show_detail, +options.show_bloat, options.list_error_boards, options.show_config, +options.show_environment, options.filter_dtb_warnings, +options.filter_migration_warnings, options.ide) +if options.summary: +builder.ShowSummary(commits, board_selected) +else: +fail, warned, excs = builder.BuildBoards( +commits, board_selected, options.keep_outputs, options.verbose) +if excs: +return 102 +if fail: +return 100 +if warned and not options.ignore_warnings: +return 101 return 0 -- 2.41.0.255.g8b1d071c50-goog
[PATCH v2 08/60] buildman: Drop use of builder in show_actions()
This function only needs the output directory from the builder. This is
passed into the builder, so just pass the same value to show_actions().
The avoids needing a builder to call show_actions().
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/control.py | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index fd136e54baf9..7beb5bce6444 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -48,7 +48,7 @@ def get_action_summary(is_summary, commits, selected,
options):
return msg
# pylint: disable=R0913
-def show_actions(series, why_selected, boards_selected, bldr, options,
+def show_actions(series, why_selected, boards_selected, output_dir, options,
board_warnings):
"""Display a list of actions that we would take, if not a dry run.
@@ -61,7 +61,7 @@ def show_actions(series, why_selected, boards_selected, bldr,
options,
the value would be a list of board names.
boards_selected: Dict of selected boards, key is target name,
value is Board object
-bldr: The builder that will be used to build the commits
+output_dir (str): Output directory for builder
options: Command line options object
board_warnings: List of warnings obtained from board selected
"""
@@ -74,7 +74,7 @@ def show_actions(series, why_selected, boards_selected, bldr,
options,
commits = None
print(get_action_summary(False, commits, boards_selected,
options))
-print(f'Build directory: {bldr.base_dir}')
+print(f'Build directory: {output_dir}')
if commits:
for upto in range(0, len(series.commits), options.step):
commit = series.commits[upto]
@@ -416,7 +416,7 @@ def do_buildman(options, args, toolchains=None,
make_func=None, brds=None,
# For a dry run, just show our actions as a sanity check
if options.dry_run:
-show_actions(series, why_selected, selected, builder, options,
+show_actions(series, why_selected, selected, output_dir, options,
board_warnings)
else:
builder.force_build = options.force_build
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 07/60] buildman: Add a test for the -A option
This lacks a test at present. Add one.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/func_test.py | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index d9b4c41ec68c..5dbbb53b6572 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -785,10 +785,9 @@ CONFIG_LOCALVERSION=y
''', cfg_data)
self.assertIn('Not dropping LOCALVERSION_AUTO', stdout.getvalue())
-def testRegenBoards(self):
-"""Test that we can regenerate the boards.cfg file"""
-outfile = os.path.join(self._output_dir, 'test-boards.cfg')
-if os.path.exists(outfile):
-os.remove(outfile)
-result = self._RunControl('-R', outfile, brds=None, get_builder=False)
-self.assertTrue(os.path.exists(outfile))
+def test_print_prefix(self):
+"""Test that we can print the toolchain prefix"""
+with test_util.capture_sys_output() as (stdout, stderr):
+result = self._RunControl('-A', 'board0')
+self.assertEqual('arm-\n', stdout.getvalue())
+self.assertEqual('', stderr.getvalue())
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 06/60] buildman: Move fetch-arch code into a separate function
Reduce the size of the do_buildman() function a little by moving the code
that handles --fetch-arch into a separate function.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/control.py | 49 +--
1 file changed, 31 insertions(+), 18 deletions(-)
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 7a81c2ced8b3..fd136e54baf9 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -192,6 +192,36 @@ def determine_series(count, has_range, branch, git_dir):
return series
+def do_fetch_arch(toolchains, col, fetch_arch):
+"""Handle the --fetch-arch option
+
+Args:
+toolchains (Toolchains): Tool chains to use
+col (terminal.Color): Color object to build
+fetch_arch (str): Argument passed to the --fetch-arch option
+
+Returns:
+int: Return code for buildman
+"""
+if fetch_arch == 'list':
+sorted_list = toolchains.ListArchs()
+print(col.build(
+col.BLUE,
+f"Available architectures: {' '.join(sorted_list)}\n"))
+return 0
+
+if fetch_arch == 'all':
+fetch_arch = ','.join(toolchains.ListArchs())
+print(col.build(col.CYAN,
+f'\nDownloading toolchains: {fetch_arch}'))
+for arch in fetch_arch.split(','):
+print()
+ret = toolchains.FetchAndInstall(arch)
+if ret:
+return ret
+return 0
+
+
def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
clean_dir=False, test_thread_exceptions=False):
"""The main control code for buildman
@@ -226,24 +256,7 @@ def do_buildman(options, args, toolchains=None,
make_func=None, brds=None,
toolchains = toolchain.Toolchains(options.override_toolchain)
if options.fetch_arch:
-if options.fetch_arch == 'list':
-sorted_list = toolchains.ListArchs()
-print(col.build(
-col.BLUE,
-f"Available architectures: {' '.join(sorted_list)}\n"))
-return 0
-
-fetch_arch = options.fetch_arch
-if fetch_arch == 'all':
-fetch_arch = ','.join(toolchains.ListArchs())
-print(col.build(col.CYAN,
-f'\nDownloading toolchains: {fetch_arch}'))
-for arch in fetch_arch.split(','):
-print()
-ret = toolchains.FetchAndInstall(arch)
-if ret:
-return ret
-return 0
+return do_fetch_arch(toolchains, col, options.fetch_arch)
if no_toolchains:
toolchains.GetSettings()
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 04/60] buildman: Move full-help processing to main
This does not need any of the control features. Move it out of main to
reduce the size of the do_buildman() function.
For Python 3.6 the -H feature will not work, but this does not seem to be
a huge problem, as it dates from 2016.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/control.py | 11 ---
tools/buildman/main.py| 9 +
2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index c890f778f501..b27a6b08477f 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -8,11 +8,6 @@ This holds the main control logic for buildman, when not
running tests.
"""
import multiprocessing
-try:
-import importlib.resources
-except ImportError:
-# for Python 3.6
-import importlib_resources
import os
import shutil
import sys
@@ -26,7 +21,6 @@ from patman import gitutil
from patman import patchstream
from u_boot_pylib import command
from u_boot_pylib import terminal
-from u_boot_pylib import tools
from u_boot_pylib.terminal import tprint
TEST_BUILDER = None
@@ -177,11 +171,6 @@ def do_buildman(options, args, toolchains=None,
make_func=None, brds=None,
# Used so testing can obtain the builder: pylint: disable=W0603
global TEST_BUILDER
-if options.full_help:
-with importlib.resources.path('buildman', 'README.rst') as readme:
-tools.print_full_help(str(readme))
-return 0
-
gitutil.setup()
col = terminal.Color()
diff --git a/tools/buildman/main.py b/tools/buildman/main.py
index 9c84e16e7df0..2253401709a8 100755
--- a/tools/buildman/main.py
+++ b/tools/buildman/main.py
@@ -6,6 +6,11 @@
"""See README for more information"""
+try:
+from importlib.resources import files
+except ImportError:
+# for Python 3.6
+import importlib_resources
import os
import sys
@@ -19,6 +24,7 @@ from buildman import bsettings
from buildman import cmdline
from buildman import control
from u_boot_pylib import test_util
+from u_boot_pylib import tools
def run_tests(skip_net_tests, verbose, args):
"""Run the buildman tests
@@ -62,6 +68,9 @@ def run_buildman():
if cmdline.HAS_TESTS and options.test:
run_tests(options.skip_net_tests, options.verbose, args)
+elif options.full_help:
+tools.print_full_help(str(files('buildman').joinpath('README.rst')))
+
# Build selected commits for selected boards
else:
bsettings.Setup(options.config_file)
--
2.41.0.255.g8b1d071c50-goog
[PATCH v2 05/60] buildman: Move series calculations into a separate function
Reduce the size of the do_buildman() function a little by moving the code
that figures out the series into a separate function.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/control.py | 95 +++
1 file changed, 56 insertions(+), 39 deletions(-)
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index b27a6b08477f..7a81c2ced8b3 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -147,6 +147,51 @@ def get_allow_missing(opt_allow, opt_no_allow,
num_selected, has_branch):
allow_missing = False
return allow_missing
+
+def determine_series(count, has_range, branch, git_dir):
+"""Determine the series which is to be built, if any
+
+Args:
+count (int): Number of commits in branch
+has_range (bool): True if a range of commits ('xx..yy') is being built
+branch (str): Name of branch to build, or None if none
+git_dir (str): Git directory to use, e.g. './.git'
+
+Returns:
+Series: Series to build, or None for none
+
+Read the metadata from the commits. First look at the upstream commit,
+then the ones in the branch. We would like to do something like
+upstream/master~..branch but that isn't possible if upstream/master is
+a merge commit (it will list all the commits that form part of the
+merge)
+
+Conflicting tags are not a problem for buildman, since it does not use
+them. For example, Series-version is not useful for buildman. On the
+other hand conflicting tags will cause an error. So allow later tags
+to overwrite earlier ones by setting allow_overwrite=True
+"""
+if branch:
+if count == -1:
+if has_range:
+range_expr = branch
+else:
+range_expr = gitutil.get_range_in_branch(git_dir, branch)
+upstream_commit = gitutil.get_upstream(git_dir, branch)
+series = patchstream.get_metadata_for_list(upstream_commit,
+git_dir, 1, series=None, allow_overwrite=True)
+
+series = patchstream.get_metadata_for_list(range_expr,
+git_dir, None, series, allow_overwrite=True)
+else:
+# Honour the count
+series = patchstream.get_metadata_for_list(branch,
+git_dir, count, series=None, allow_overwrite=True)
+else:
+series = None
+return series
+
+
def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
clean_dir=False, test_thread_exceptions=False):
"""The main control code for buildman
@@ -174,7 +219,7 @@ def do_buildman(options, args, toolchains=None,
make_func=None, brds=None,
gitutil.setup()
col = terminal.Color()
-options.git_dir = os.path.join(options.git, '.git')
+git_dir = os.path.join(options.git, '.git')
no_toolchains = toolchains is None
if no_toolchains:
@@ -264,11 +309,11 @@ def do_buildman(options, args, toolchains=None,
make_func=None, brds=None,
count = 1
else:
if has_range:
-count, msg = gitutil.count_commits_in_range(options.git_dir,
- options.branch)
+count, msg = gitutil.count_commits_in_range(git_dir,
+options.branch)
else:
-count, msg = gitutil.count_commits_in_branch(options.git_dir,
- options.branch)
+count, msg = gitutil.count_commits_in_branch(git_dir,
+ options.branch)
if count is None:
sys.exit(col.build(col.RED, msg))
elif count == 0:
@@ -290,39 +335,11 @@ def do_buildman(options, args, toolchains=None,
make_func=None, brds=None,
sys.exit(col.build(col.RED,
'-w can only be used with a single commit'))
-# Read the metadata from the commits. First look at the upstream commit,
-# then the ones in the branch. We would like to do something like
-# upstream/master~..branch but that isn't possible if upstream/master is
-# a merge commit (it will list all the commits that form part of the
-# merge)
-# Conflicting tags are not a problem for buildman, since it does not use
-# them. For example, Series-version is not useful for buildman. On the
-# other hand conflicting tags will cause an error. So allow later tags
-# to overwrite earlier ones by setting allow_overwrite=True
-if options.branch:
-if count == -1:
-if has_range:
-range_expr = options.branch
-else:
-range_expr = gitutil.get_range_in_branch(options.git_dir,
- options.branch)
-u
[PATCH v2 03/60] buildman: Fix most pylint warnings in control
Tidy up the easier-to-fix pylint warnings in module 'control'.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/control.py | 119 +---
tools/buildman/func_test.py | 2 +-
2 files changed, 70 insertions(+), 51 deletions(-)
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index bfb02834e8fb..c890f778f501 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -15,7 +15,6 @@ except ImportError:
import importlib_resources
import os
import shutil
-import subprocess
import sys
from buildman import boards
@@ -30,6 +29,8 @@ from u_boot_pylib import terminal
from u_boot_pylib import tools
from u_boot_pylib.terminal import tprint
+TEST_BUILDER = None
+
def get_plural(count):
"""Returns a plural 's' if count is not 1"""
return 's' if count != 1 else ''
@@ -43,17 +44,17 @@ def get_action_summary(is_summary, commits, selected,
options):
if commits:
count = len(commits)
count = (count + options.step - 1) // options.step
-commit_str = '%d commit%s' % (count, get_plural(count))
+commit_str = f'{count} commit{get_plural(count)}'
else:
commit_str = 'current source'
-str = '%s %s for %d boards' % (
-'Summary of' if is_summary else 'Building', commit_str,
-len(selected))
-str += ' (%d thread%s, %d job%s per thread)' % (options.threads,
-get_plural(options.threads), options.jobs,
get_plural(options.jobs))
-return str
-
-def show_actions(series, why_selected, boards_selected, builder, options,
+msg = (f"{'Summary of' if is_summary else 'Building'} "
+ f'{commit_str} for {len(selected)} boards')
+msg += (f' ({options.threads} thread{get_plural(options.threads)}, '
+f'{options.jobs} job{get_plural(options.jobs)} per thread)')
+return msg
+
+# pylint: disable=R0913
+def show_actions(series, why_selected, boards_selected, bldr, options,
board_warnings):
"""Display a list of actions that we would take, if not a dry run.
@@ -66,7 +67,7 @@ def show_actions(series, why_selected, boards_selected,
builder, options,
the value would be a list of board names.
boards_selected: Dict of selected boards, key is target name,
value is Board object
-builder: The builder that will be used to build the commits
+bldr: The builder that will be used to build the commits
options: Command line options object
board_warnings: List of warnings obtained from board selected
"""
@@ -79,7 +80,7 @@ def show_actions(series, why_selected, boards_selected,
builder, options,
commits = None
print(get_action_summary(False, commits, boards_selected,
options))
-print('Build directory: %s' % builder.base_dir)
+print(f'Build directory: {bldr.base_dir}')
if commits:
for upto in range(0, len(series.commits), options.step):
commit = series.commits[upto]
@@ -88,11 +89,11 @@ def show_actions(series, why_selected, boards_selected,
builder, options,
print()
for arg in why_selected:
if arg != 'all':
-print(arg, ': %d boards' % len(why_selected[arg]))
+print(arg, f': {len(why_selected[arg])} boards')
if options.verbose:
-print(' %s' % ' '.join(why_selected[arg]))
-print(('Total boards to build for each commit: %d\n' %
-len(why_selected['all'])))
+print(f" {' '.join(why_selected[arg])}")
+print('Total boards to build for each '
+ f"commit: {len(why_selected['all'])}\n")
if board_warnings:
for warning in board_warnings:
print(col.build(col.YELLOW, warning))
@@ -116,12 +117,26 @@ def show_toolchain_prefix(brds, toolchains):
tc_set.add(toolchains.Select(brd.arch))
if len(tc_set) != 1:
return 'Supplied boards must share one toolchain'
-return False
-tc = tc_set.pop()
-print(tc.GetEnvArgs(toolchain.VAR_CROSS_COMPILE))
+tchain = tc_set.pop()
+print(tchain.GetEnvArgs(toolchain.VAR_CROSS_COMPILE))
return None
def get_allow_missing(opt_allow, opt_no_allow, num_selected, has_branch):
+"""Figure out whether to allow external blobs
+
+Uses the allow-missing setting and the provided arguments to decide whether
+missing external blobs should be allowed
+
+Args:
+opt_allow (bool): True if --allow-missing flag is set
+opt_no_allow (bool): True if --no-allow-missing flag is set
+num_selected (int): Number of selected board
+has_branch (bool): True if a git branch (to build) has been provided
+
+Returns:
+bool: True to allow missing external blobs, False to produce an error
if
+external blobs are used
+"""
allow_missing = False
am_setting = bsettings.GetGlobalItemValue('allow-missing')
if am_se
