Re: [PATCH v1 2/8] riscv: dts: Add device tree for Microchip Icicle Kit

2020-10-17 Thread Sean Anderson
On 10/16/20 10:23 AM, padmarao.beg...@microchip.com wrote:
> From: Padmarao Begari 
> 
> Add device tree for Microchip PolarFire SoC Icicle Kit.
> 
> Signed-off-by: Padmarao Begari 
> ---
>  arch/riscv/dts/Makefile  |   1 +
>  arch/riscv/dts/microchip-icicle-kit-a000.dts | 419 +++
>  2 files changed, 420 insertions(+)
>  create mode 100644 arch/riscv/dts/microchip-icicle-kit-a000.dts
> 
> diff --git a/arch/riscv/dts/Makefile b/arch/riscv/dts/Makefile
> index 3a6f96c67d..48c43bd122 100644
> --- a/arch/riscv/dts/Makefile
> +++ b/arch/riscv/dts/Makefile
> @@ -3,6 +3,7 @@
>  dtb-$(CONFIG_TARGET_AX25_AE350) += ae350_32.dtb ae350_64.dtb
>  dtb-$(CONFIG_TARGET_SIFIVE_FU540) += hifive-unleashed-a00.dtb
>  dtb-$(CONFIG_TARGET_SIPEED_MAIX) += k210-maix-bit.dtb
> +dtb-$(CONFIG_TARGET_MICROCHIP_ICICLE) += microchip-icicle-kit-a000.dtb
>  
>  targets += $(dtb-y)
>  
> diff --git a/arch/riscv/dts/microchip-icicle-kit-a000.dts 
> b/arch/riscv/dts/microchip-icicle-kit-a000.dts
> new file mode 100644
> index 00..e7f0ec6926
> --- /dev/null
> +++ b/arch/riscv/dts/microchip-icicle-kit-a000.dts
> @@ -0,0 +1,419 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/* Copyright (c) 2020 Microchip Technology Inc */
> +
> +/dts-v1/;
> +#include "dt-bindings/clock/microchip,pfsoc-clock.h"
> +
> +/ {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + model = "Microchip PolarFire-SoC";
> + compatible = "microchip,polarfire-soc";
> +
> + aliases {
> + serial0 = 
> + ethernet0 = 
> + };
> +
> + chosen {
> + stdout-path = "serial0";
> + };
> +
> + cpucomplex: cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + timebase-frequency = <100>;
> + cpu0: cpu@0 {
> + clocks = < CLK_CPU>;
> + compatible = "sifive,e51", "sifive,rocket0", "riscv";
> + device_type = "cpu";
> + i-cache-block-size = <64>;
> + i-cache-sets = <128>;
> + i-cache-size = <16384>;
> + reg = <0>;
> + riscv,isa = "rv64imac";
> + status = "disabled";
> + operating-points = <
> + /* kHz  uV */
> + 60  110
> + 30   95
> + 15   75
> + >;
> + cpu0intc: interrupt-controller {
> + #interrupt-cells = <1>;
> + compatible = "riscv,cpu-intc";
> + interrupt-controller;
> + };
> + };
> + cpu1: cpu@1 {
> + clocks = < CLK_CPU>;
> + compatible = "sifive,u54-mc", "sifive,rocket0", "riscv";
> + d-cache-block-size = <64>;
> + d-cache-sets = <64>;
> + d-cache-size = <32768>;
> + d-tlb-sets = <1>;
> + d-tlb-size = <32>;
> + device_type = "cpu";
> + i-cache-block-size = <64>;
> + i-cache-sets = <64>;
> + i-cache-size = <32768>;
> + i-tlb-sets = <1>;
> + i-tlb-size = <32>;
> + mmu-type = "riscv,sv39";
> + reg = <1>;
> + riscv,isa = "rv64imafdc";
> + tlb-split;
> + status = "okay";
> + operating-points = <
> + /* kHz  uV */
> + 60  110
> + 30   95
> + 15   75
> + >;
> + cpu1intc: interrupt-controller {
> + #interrupt-cells = <1>;
> + compatible = "riscv,cpu-intc";
> + interrupt-controller;
> + };
> + };
> + cpu2: cpu@2 {
> + clocks = < CLK_CPU>;
> + compatible = "sifive,u54-mc", "sifive,rocket0", "riscv";
> + d-cache-block-size = <64>;
> + d-cache-sets = <64>;
> + d-cache-size = <32768>;
> + d-tlb-sets = <1>;
> + d-tlb-size = <32>;
> + device_type = "cpu";
> + i-cache-block-size = <64>;
> + i-cache-sets = <64>;
> + i-cache-size = <32768>;
> + i-tlb-sets = <1>;
> + i-tlb-size = <32>;
> + mmu-type = "riscv,sv39";
> + reg = <2>;
> + riscv,isa = 

Re: [PATCH v1 1/8] riscv: Add Microchip MPFS Icicle Kit support

2020-10-17 Thread Sean Anderson
On 10/16/20 10:23 AM, padmarao.beg...@microchip.com wrote:
> From: Padmarao Begari 
> 
> This patch adds Microchip MPFS Icicle Kit support. For now, only
> NS16550 Serial, Microchip clock, Cadence eMMC and MACB drivers are
> only enabled. The Microchip MPFS Icicle defconfig by default builds
> U-Boot for S-Mode because U-Boot on Microchip PolarFire SoC will run
> in S-Mode as payload of HSS + OpenSBI.
> 
> Signed-off-by: Padmarao Begari 
> ---
>  board/microchip/mpfs_icicle/Kconfig   | 26 ++
>  board/microchip/mpfs_icicle/mpfs_icicle.c | 96 ++-
>  configs/microchip_mpfs_icicle_defconfig   |  9 ++-
>  include/configs/microchip_mpfs_icicle.h   | 60 +-
>  4 files changed, 146 insertions(+), 45 deletions(-)
> 
> diff --git a/board/microchip/mpfs_icicle/Kconfig 
> b/board/microchip/mpfs_icicle/Kconfig
> index bf8e1a13ec..4406d1a13f 100644
> --- a/board/microchip/mpfs_icicle/Kconfig
> +++ b/board/microchip/mpfs_icicle/Kconfig
> @@ -20,7 +20,33 @@ config BOARD_SPECIFIC_OPTIONS # dummy
>   def_bool y
>   select GENERIC_RISCV
>   select BOARD_EARLY_INIT_F
> + select BOARD_LATE_INIT
>   imply SMP
> + imply CLK
> + imply CLK_CCF
> + imply CLK_MPFS
>   imply SYS_NS16550
> + imply CMD_DHCP
> + imply CMD_EXT2
> + imply CMD_EXT4
> + imply CMD_FAT
> + imply CMD_FS_GENERIC
> + imply CMD_NET
> + imply CMD_PING

Perhaps just imply DISTRO_DEFAULTS?

> + imply CMD_MMC
> + imply DOS_PARTITION
> + imply EFI_PARTITION
> + imply IP_DYN
> + imply ISO_PARTITION
> + imply MACB
> + imply MII
> + imply NET_RANDOM_ETHADDR
> + imply PHY_LIB
> + imply PHY_VITESSE
> + imply DMA_ADDR_T_64BIT
> + imply MMC
> + imply MMC_WRITE
> + imply MMC_SDHCI
> + imply MMC_SDHCI_CADENCE
>  
>  endif
> diff --git a/board/microchip/mpfs_icicle/mpfs_icicle.c 
> b/board/microchip/mpfs_icicle/mpfs_icicle.c
> index 8381361ec3..64133aee59 100644
> --- a/board/microchip/mpfs_icicle/mpfs_icicle.c
> +++ b/board/microchip/mpfs_icicle/mpfs_icicle.c
> @@ -5,11 +5,47 @@
>   */
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
>  
> -#define MPFS_SYSREG_SOFT_RESET   ((unsigned int *)0x20002088)
> +#define MPFS_SYSREG_SOFT_RESET   ((unsigned int *)0x20002088)
> +#define MPFS_SYS_SERVICE_CR  ((unsigned int *)0x37020050)
> +#define MPFS_SYS_SERVICE_SR  ((unsigned int *)0x37020054)
> +#define MPFS_SYS_SERVICE_MAILBOX ((unsigned char *)0x37020800)
> +
> +#define PERIPH_RESET_VALUE   0x1e8u
> +#define SERVICE_CR_REQ   0x1u
> +#define SERVICE_SR_BUSY  0x2u
> +
> +static void read_device_serial_number(u8 *response, u8 response_size)
> +{
> + u8 idx;
> + u8 *response_buf;
> + unsigned int val;
> +
> + response_buf = (u8 *)response;
> +
> + writel(SERVICE_CR_REQ, MPFS_SYS_SERVICE_CR);
> +
> + /* REQ bit will remain set till the system controller starts
> +  * processing.
> +  */
> + do {
> + val = readl(MPFS_SYS_SERVICE_CR);
> + } while (SERVICE_CR_REQ == (val & SERVICE_CR_REQ));
> +
> + /* Once system controller starts processing the busy bit will
> +  * go high and service is completed when busy bit is gone low
> +  */
> + do {
> + val = readl(MPFS_SYS_SERVICE_SR);
> + } while (SERVICE_SR_BUSY == (val & SERVICE_SR_BUSY));
> +
> + for (idx = 0; idx < response_size; idx++)
> + response_buf[idx] = readb(MPFS_SYS_SERVICE_MAILBOX + idx);
> +}
>  
>  int board_init(void)
>  {
> @@ -22,10 +58,64 @@ int board_early_init_f(void)
>  {
>   unsigned int val;
>  
> - /* Reset uart peripheral */
> + /* Reset uart, mmc peripheral */
>   val = readl(MPFS_SYSREG_SOFT_RESET);
> - val = (val & ~(1u << 5u));
> + val = (val & ~(PERIPH_RESET_VALUE));
>   writel(val, MPFS_SYSREG_SOFT_RESET);
>  
>   return 0;
>  }
> +
> +int board_late_init(void)
> +{
> + u32 ret;
> + u32 node;
> + u8 idx;
> + u8 device_serial_number[16] = { 0 };
> + unsigned char mac_addr[6];
> + char icicle_mac_addr[20];
> + void *blob = (void *)gd->fdt_blob;
> +
> + node = fdt_path_offset(blob, "ethernet0");
> + if (node < 0) {
> + printf("No ethernet0 path offset\n");
> + return -ENODEV;
> + }
> +
> + ret = fdtdec_get_byte_array(blob, node, "mac-address", mac_addr, 6);
> + if (ret) {
> + printf("No mac-address property\n");
> + return -EINVAL;
> + }
> +
> + read_device_serial_number(device_serial_number, 16);
> +
> + /* Update MAC address with device serial number */
> + mac_addr[0] = 0x00;
> + mac_addr[1] = 0x04;
> + mac_addr[2] = 0xA3;
> + mac_addr[3] = device_serial_number[2];
> + mac_addr[4] = device_serial_number[1];
> + mac_addr[5] = device_serial_number[0];
> +
> + ret = fdt_setprop(blob, node, 

Re: [PATCH v4 13/13] riscv: Add support for SPI on Kendryte K210

2020-10-17 Thread Sean Anderson
On 10/16/20 6:57 PM, Sean Anderson wrote:
> This enables configs necessary for using SPI. The environment is saved to
> the very end of SPI flash. This is unlikely to be overwritten unless the
> entire flash is reprogrammed.
> 
> This also supplies a default bootcommand. It loads an image and device tree
> from the first partition of the MMC. This is a minimal/least effort
> bootcmd, so suggestions (especially in the form of patches) are welcome. I
> didn't set up distro boot because I think it is unlikely that any
> general-purpose linux distros will ever be ported to this board.
> 
> Signed-off-by: Sean Anderson 
> ---
> 
> Changes in v4:
> - Enable booting from MMC
> - Place env in spi flash
> - Update documentation
> 
> Changes in v3:
> - Rebase onto U-Boot master
> - Remove env and bootcmd configuration. I'm going to punt on those for now,
>   since I haven't worked out the best way to boot with SPI yet. Those
>   settings may be added back in a follow-up patch.
> 
> Changes in v2:
> - Add Gigadevice SPI chips to dependencies
> 
>  board/sipeed/maix/Kconfig  |  16 ++
>  configs/sipeed_maix_bitm_defconfig |  10 +
>  doc/board/sipeed/maix.rst  | 319 -
>  include/configs/sipeed-maix.h  |   7 +-
>  4 files changed, 300 insertions(+), 52 deletions(-)
> 
> diff --git a/board/sipeed/maix/Kconfig b/board/sipeed/maix/Kconfig
> index 4c42dd2087..2cdea8ea81 100644
> --- a/board/sipeed/maix/Kconfig
> +++ b/board/sipeed/maix/Kconfig
> @@ -53,4 +53,20 @@ config BOARD_SPECIFIC_OPTIONS
>   imply CMD_GPIO
>   imply LED
>   imply LED_GPIO
> + imply SPI
> + imply DESIGNWARE_SPI
> + imply SPI_FLASH_GIGADEVICE
> + imply SPI_FLASH_WINBOND
> + imply DM_MTD
> + imply SPI_FLASH_MTD
> + imply CMD_MTD
> + imply ENV_IS_IN_SPI_FLASH
> + imply MMC
> + imply MMC_BROKEN_CD
> + imply MMC_SPI
> + imply CMD_MMC
> + imply DOS_PARTITION
> + imply EFI_PARTITION
> + imply CMD_PART
> + imply CMD_FS_GENERIC
>  endif
> diff --git a/configs/sipeed_maix_bitm_defconfig 
> b/configs/sipeed_maix_bitm_defconfig
> index 459bf0d530..cb9824e84e 100644
> --- a/configs/sipeed_maix_bitm_defconfig
> +++ b/configs/sipeed_maix_bitm_defconfig
> @@ -1,8 +1,18 @@
>  CONFIG_RISCV=y
> +CONFIG_ENV_SIZE=0x1000
> +CONFIG_ENV_OFFSET=0xfff000
> +CONFIG_ENV_SECT_SIZE=0x1000
>  CONFIG_TARGET_SIPEED_MAIX=y
>  CONFIG_ARCH_RV64I=y
>  CONFIG_STACK_SIZE=0x10
> +CONFIG_USE_BOOTCOMMAND=y

This is missing CONFIG_HUSH_PARSER to run the bootcmd.

--Sean

> +CONFIG_BOOTCOMMAND="run k210_bootcmd"
> +CONFIG_MTDIDS_DEFAULT="nor0=spi3:0"
> +CONFIG_MTDPARTS_DEFAULT="nor0:1M(u-boot),0x1000@0xfff000(env)"
>  # CONFIG_NET is not set
>  # CONFIG_INPUT is not set
> +CONFIG_SF_DEFAULT_BUS=3
>  # CONFIG_DM_ETH is not set
> +CONFIG_FS_EXT4=y
> +CONFIG_FS_FAT=y
>  # CONFIG_EFI_LOADER is not set
> diff --git a/doc/board/sipeed/maix.rst b/doc/board/sipeed/maix.rst
> index 92f2d112a9..bf945b3458 100644
> --- a/doc/board/sipeed/maix.rst
> +++ b/doc/board/sipeed/maix.rst
> @@ -70,6 +70,7 @@ console shall be opened immediately. Boot output should 
> look like the following:
>  U-Boot 2020.04-rc2-00087-g2221cc09c1-dirty (Feb 28 2020 - 13:53:09 -0500)
>  
>  DRAM:  8 MiB
> +MMC:   spi@5300:slot@0: 0
>  In:serial@3800
>  Out:   serial@3800
>  Err:   serial@3800
> @@ -118,14 +119,115 @@ The value of FW_PAYLOAD_OFFSET must match 
> CONFIG_SYS_TEXT_BASE - 0x8000.
>  
>  The file to flash is build/platform/kendryte/k210/firmware/fw_payload.bin.
>  
> -Loading Images
> -^^
> +Booting
> +^^^
>  
> -To load a kernel, transfer it over serial.
> +The default boot process is to load and boot the files ``/uImage`` and
> +``/k210.dtb`` off of the first partition of the MMC. For Linux, this will 
> result
> +in an output like
>  
>  .. code-block:: none
>  
> -=> loady 8000 150
> +U-Boot 2020.10-00691-gd1d651d988-dirty (Oct 16 2020 - 17:05:24 -0400)
> +
> +DRAM:  8 MiB
> +MMC:   spi@5300:slot@0: 0
> +Loading Environment from SPIFlash... SF: Detected w25q128fw with page 
> size 256 Bytes, erase size 4 KiB, total 16 MiB
> +OK
> +In:serial@3800
> +Out:   serial@3800
> +Err:   serial@3800
> +Hit any key to stop autoboot:  0
> +1827380 bytes read in 1044 ms (1.7 MiB/s)
> +13428 bytes read in 10 ms (1.3 MiB/s)
> +## Booting kernel from Legacy Image at 8006 ...
> +   Image Name:   linux
> +   Image Type:   RISC-V Linux Kernel Image (uncompressed)
> +   Data Size:1827316 Bytes = 1.7 MiB
> +   Load Address: 8000
> +   Entry Point:  8000
> +   Verifying Checksum ... OK
> +## Flattened Device Tree blob at 8040
> +   Booting using the fdt blob at 0x8040
> +   Loading Kernel Image
> +   Loading Device Tree to 803f9000, end 803ff473 ... OK
> +
> +Starting kernel ...
> +
> +[ 

[PATCH v3 21/23] test: Add a test for log filter-*

2020-10-17 Thread Sean Anderson
This exercises a few success and failure modes of the log filter-*
commands. log filter-list is not tested because it's purely informational.
I don't think there's a good way to test it except by testing if the output
of the command exactly matches a sample run.

Signed-off-by: Sean Anderson 
Reviewed-by: Simon Glass 
---

Changes in v3:
- Update copyright for log_filter.c

Changes in v2:
- Converted log filter-* tests to C from python

 include/test/log.h|   1 +
 test/log/Makefile |   1 +
 test/log/log_filter.c | 108 ++
 3 files changed, 110 insertions(+)
 create mode 100644 test/log/log_filter.c

diff --git a/include/test/log.h b/include/test/log.h
index 772e197806..e902891450 100644
--- a/include/test/log.h
+++ b/include/test/log.h
@@ -14,5 +14,6 @@
 
 /* Declare a new logging test */
 #define LOG_TEST(_name) UNIT_TEST(_name, 0, log_test)
+#define LOG_TEST_FLAGS(_name, _flags) UNIT_TEST(_name, _flags, log_test)
 
 #endif /* __TEST_LOG_H__ */
diff --git a/test/log/Makefile b/test/log/Makefile
index 52e2f7b41c..9a6cdbe643 100644
--- a/test/log/Makefile
+++ b/test/log/Makefile
@@ -3,6 +3,7 @@
 # Copyright (c) 2017 Google, Inc
 
 obj-$(CONFIG_LOG_TEST) += log_test.o
+obj-$(CONFIG_CMD_LOG) += log_filter.o
 
 ifdef CONFIG_UT_LOG
 
diff --git a/test/log/log_filter.c b/test/log/log_filter.c
new file mode 100644
index 00..e27229dbd9
--- /dev/null
+++ b/test/log/log_filter.c
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2020 Sean Anderson 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/* Test invalid options */
+static int log_test_filter_invalid(struct unit_test_state *uts)
+{
+   ut_asserteq(1, run_command("log filter-add -AD", 0));
+   ut_asserteq(1, run_command("log filter-add -l1 -L1", 0));
+   ut_asserteq(1, run_command("log filter-add -l1 -L1", 0));
+   ut_asserteq(1, run_command("log filter-add -lfoo", 0));
+   ut_asserteq(1, run_command("log filter-add -cfoo", 0));
+   ut_asserteq(1, run_command("log filter-add -ccore -ccore -ccore -ccore "
+  "-ccore -ccore", 0));
+
+   return 0;
+}
+LOG_TEST_FLAGS(log_test_filter_invalid, UT_TESTF_CONSOLE_REC);
+
+/* Test adding and removing filters */
+static int log_test_filter(struct unit_test_state *uts)
+{
+   bool any_found = false;
+   bool filt1_found = false;
+   bool filt2_found = false;
+   char cmd[32];
+   struct log_filter *filt;
+   struct log_device *ldev;
+   ulong filt1, filt2;
+
+#define create_filter(args, filter_num) do {\
+   ut_assertok(console_record_reset_enable()); \
+   ut_assertok(run_command("log filter-add -p " args, 0)); \
+   ut_assert_skipline(); \
+   ut_assertok(strict_strtoul(uts->actual_str, 10, &(filter_num))); \
+   ut_assert_console_end(); \
+} while (0)
+
+   create_filter("", filt1);
+   create_filter("-DL warning -cmmc -cspi -ffile", filt2);
+
+   ldev = log_device_find_by_name("console");
+   ut_assertnonnull(ldev);
+   list_for_each_entry(filt, >filter_head, sibling_node) {
+   if (filt->filter_num == filt1) {
+   filt1_found = true;
+   ut_asserteq(0, filt->flags);
+   ut_asserteq(LOGL_MAX, filt->level);
+   ut_assertnull(filt->file_list);
+   } else if (filt->filter_num == filt2) {
+   filt2_found = true;
+   ut_asserteq(LOGFF_HAS_CAT | LOGFF_DENY |
+   LOGFF_LEVEL_MIN, filt->flags);
+   ut_asserteq(true, log_has_cat(filt->cat_list,
+ UCLASS_MMC));
+   ut_asserteq(true, log_has_cat(filt->cat_list,
+ UCLASS_SPI));
+   ut_asserteq(LOGL_WARNING, filt->level);
+   ut_asserteq_str("file", filt->file_list);
+   }
+   }
+   ut_asserteq(true, filt1_found);
+   ut_asserteq(true, filt2_found);
+
+#define remove_filter(filter_num) do { \
+   ut_assertok(console_record_reset_enable()); \
+   snprintf(cmd, sizeof(cmd), "log filter-remove %lu", filter_num); \
+   ut_assertok(run_command(cmd, 0)); \
+   ut_assert_console_end(); \
+} while (0)
+
+   remove_filter(filt1);
+   remove_filter(filt2);
+
+   filt1_found = false;
+   filt2_found = false;
+   list_for_each_entry(filt, >filter_head, sibling_node) {
+   if (filt->filter_num == filt1)
+   filt1_found = true;
+   else if (filt->filter_num == filt2)
+   filt2_found = true;
+   }
+   ut_asserteq(false, filt1_found);
+   ut_asserteq(false, filt2_found);
+
+   create_filter("", filt1);
+   create_filter("", filt2);
+
+   

[PATCH v3 20/23] cmd: log: Add commands to manipulate filters

2020-10-17 Thread Sean Anderson
This adds several commands to add, list, and remove log filters. Due to the
complexity of adding a filter, `log filter-list` uses options instead of
positional arguments.

These commands have been added as subcommands to log by using a dash to
join the subcommand and subsubcommand. This is stylistic, and they could be
converted to proper subsubcommands if it is wished.

Signed-off-by: Sean Anderson 
Reviewed-by: Simon Glass 
---

(no changes since v2)

Changes in v2:
- Add option to remove all filters to filter-remove
- Clarify filter-* help text

 cmd/Kconfig |   1 +
 cmd/log.c   | 240 
 2 files changed, 241 insertions(+)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index a3166e4f31..768ac541b2 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2218,6 +2218,7 @@ config CMD_KGDB
 config CMD_LOG
bool "log - Generation, control and access to logging"
select LOG
+   select GETOPT
help
  This provides access to logging features. It allows the output of
  log data to be controlled to a limited extent (setting up the default
diff --git a/cmd/log.c b/cmd/log.c
index 596bc73f47..6190a274e4 100644
--- a/cmd/log.c
+++ b/cmd/log.c
@@ -7,7 +7,9 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+#include 
 
 static char log_fmt_chars[LOGF_COUNT] = "clFLfm";
 
@@ -84,6 +86,220 @@ static int do_log_drivers(struct cmd_tbl *cmdtp, int flag, 
int argc,
return CMD_RET_SUCCESS;
 }
 
+static int do_log_filter_list(struct cmd_tbl *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+   int opt;
+   const char *drv_name = "console";
+   struct getopt_state gs;
+   struct log_filter *filt;
+   struct log_device *ldev;
+
+   getopt_init_state();
+   while ((opt = getopt(, argc, argv, "d:")) > 0) {
+   switch (opt) {
+   case 'd':
+   drv_name = gs.arg;
+   break;
+   default:
+   return CMD_RET_USAGE;
+   }
+   }
+
+   if (gs.index != argc)
+   return CMD_RET_USAGE;
+
+   ldev = log_device_find_by_name(drv_name);
+   if (!ldev) {
+   printf("Could not find log device for \"%s\"\n", drv_name);
+   return CMD_RET_FAILURE;
+   }
+
+   printf("num policy level   categories files\n");
+   list_for_each_entry(filt, >filter_head, sibling_node) {
+   printf("%3d %6.6s %s%-7.7s ", filt->filter_num,
+  filt->flags & LOGFF_DENY ? "deny" : "allow",
+  filt->flags & LOGFF_LEVEL_MIN ? ">=" : "<=",
+  log_get_level_name(filt->level));
+
+   if (filt->flags & LOGFF_HAS_CAT) {
+   int i;
+
+   if (filt->cat_list[0] != LOGC_END)
+   printf("%16.16s %s\n",
+  log_get_cat_name(filt->cat_list[0]),
+  filt->file_list ? filt->file_list : "");
+
+   for (i = 1; i < LOGF_MAX_CATEGORIES &&
+   filt->cat_list[i] != LOGC_END; i++)
+   printf("%20c %16.16s\n", ' ',
+  log_get_cat_name(filt->cat_list[i]));
+   } else {
+   printf("%16c %s\n", ' ',
+  filt->file_list ? filt->file_list : "");
+   }
+   }
+
+   return CMD_RET_SUCCESS;
+}
+
+static int do_log_filter_add(struct cmd_tbl *cmdtp, int flag, int argc,
+char *const argv[])
+{
+   bool level_set = false;
+   bool print_num = false;
+   bool type_set = false;
+   char *file_list = NULL;
+   const char *drv_name = "console";
+   int opt, err;
+   int cat_count = 0;
+   int flags = 0;
+   enum log_category_t cat_list[LOGF_MAX_CATEGORIES + 1];
+   enum log_level_t level = LOGL_MAX;
+   struct getopt_state gs;
+
+   getopt_init_state();
+   while ((opt = getopt(, argc, argv, "Ac:d:Df:l:L:p")) > 0) {
+   switch (opt) {
+   case 'A':
+#define do_type() do { \
+   if (type_set) { \
+   printf("Allow or deny set twice\n"); \
+   return CMD_RET_USAGE; \
+   } \
+   type_set = true; \
+} while (0)
+   do_type();
+   break;
+   case 'c': {
+   enum log_category_t cat;
+
+   if (cat_count >= LOGF_MAX_CATEGORIES) {
+   printf("Too many categories\n");
+   return CMD_RET_FAILURE;
+   }
+
+   cat = log_get_cat_by_name(gs.arg);
+   if (cat == LOGC_NONE) {
+ 

[PATCH v3 18/23] lib: Add getopt

2020-10-17 Thread Sean Anderson
Some commands can get very unweildy if they have too many positional
arguments. Adding options makes them easier to read, remember, and
understand.

This implementation of getopt has been taken from barebox, which has had
option support for quite a while. I have made a few modifications to their
version, such as the removal of opterr in favor of a separate getopt_silent
function. In addition, I have moved all global variables into struct
getopt_context.

The getopt from barebox also re-orders the arguments passed to it so that
non-options are placed last. This allows users to specify options anywhere.
For example, `ls -l foo/ -R` would be re-ordered to `ls -l -R foo/` as
getopt parsed the options. However, this feature conflicts with the const
argv in cmd_tbl->cmd. This was originally added in 54841ab50c ("Make sure
that argv[] argument pointers are not modified."). The reason stated in
that commit is that hush requires argv to stay unmodified. Has this
situation changed? Barebox also uses hush, and does not have this problem.
Perhaps we could use their fix?

I have assigned maintenance of getopt to Simon Glass, as it is currently
only used by the log command. I would also be fine maintaining it.

Signed-off-by: Sean Anderson 
---

(no changes since v2)

Changes in v2:
- Expand documentation of getopt() to include examples
- Remove opt prefix from getopt_state members

 MAINTAINERS|   1 +
 doc/api/getopt.rst |   8 +++
 doc/api/index.rst  |   1 +
 include/getopt.h   | 130 +
 lib/Kconfig|   5 ++
 lib/Makefile   |   1 +
 lib/getopt.c   | 125 +++
 7 files changed, 271 insertions(+)
 create mode 100644 doc/api/getopt.rst
 create mode 100644 include/getopt.h
 create mode 100644 lib/getopt.c

diff --git a/MAINTAINERS b/MAINTAINERS
index fb9ba37984..0f8e1f6d4c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -747,6 +747,7 @@ T:  git https://gitlab.denx.de/u-boot/u-boot.git
 F: common/log*
 F: cmd/log.c
 F: doc/develop/logging.rst
+F: lib/getopt.c
 F: test/log/
 F: test/py/tests/test_log.py
 
diff --git a/doc/api/getopt.rst b/doc/api/getopt.rst
new file mode 100644
index 00..773f79aeb6
--- /dev/null
+++ b/doc/api/getopt.rst
@@ -0,0 +1,8 @@
+.. SPDX-License-Identifier: GPL-2.0+
+.. Copyright (C) 2020 Sean Anderson 
+
+Option Parsing
+==
+
+.. kernel-doc:: include/getopt.h
+   :internal:
diff --git a/doc/api/index.rst b/doc/api/index.rst
index 1c261bcb73..d90e70e16a 100644
--- a/doc/api/index.rst
+++ b/doc/api/index.rst
@@ -8,6 +8,7 @@ U-Boot API documentation
 
dfu
efi
+   getopt
linker_lists
pinctrl
rng
diff --git a/include/getopt.h b/include/getopt.h
new file mode 100644
index 00..6f5811e64b
--- /dev/null
+++ b/include/getopt.h
@@ -0,0 +1,130 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * getopt.h - a simple getopt(3) implementation.
+ *
+ * Copyright (C) 2020 Sean Anderson 
+ * Copyright (c) 2007 Sascha Hauer , Pengutronix
+ */
+
+#ifndef __GETOPT_H
+#define __GETOPT_H
+
+/**
+ * struct getopt_state - Saved state across getopt() calls
+ */
+struct getopt_state {
+   /**
+* @index: Index of the next unparsed argument of @argv. If getopt() has
+* parsed all of @argv, then @index will equal @argc.
+*/
+   int index;
+   /* private: */
+   /** @arg_index: Index within the current argument */
+   int arg_index;
+   union {
+   /* public: */
+   /**
+* @opt: Option being parsed when an error occurs. @opt is only
+* valid when getopt() returns ``?`` or ``:``.
+*/
+   int opt;
+   /**
+* @arg: The argument to an option, NULL if there is none. @arg
+* is only valid when getopt() returns an option character.
+*/
+   char *arg;
+   /* private: */
+   };
+};
+
+/**
+ * getopt_init_state() - Initialize a  getopt_state
+ * @gs: The state to initialize
+ *
+ * This must be called before using @gs with getopt().
+ */
+void getopt_init_state(struct getopt_state *gs);
+
+int __getopt(struct getopt_state *gs, int argc, char *const argv[],
+const char *optstring, bool silent);
+
+/**
+ * getopt() - Parse short command-line options
+ * @gs: Internal state and out-of-band return arguments. This must be
+ *  initialized with getopt_init_context() beforehand.
+ * @argc: Number of arguments, not including the %NULL terminator
+ * @argv: Argument list, terminated by %NULL
+ * @optstring: Option specification, as described below
+ *
+ * getopt() parses short options. Short options are single characters. They may
+ * be followed by a required argument or an optional argument. Arguments to
+ * options may occur in the same argument as an option (like ``-larg``), or
+ * in the following argument (like ``-l arg``). An argument 

[PATCH v3 23/23] doc: Update logging documentation

2020-10-17 Thread Sean Anderson
This updates logging documentation with some examples of the new commands
added in the previous commits. It also removes some items from the to-do
list which have been implemented.

Signed-off-by: Sean Anderson 
Reviewed-by: Simon Glass 
---

Changes in v3:
- Fix heading level of Filters section
- Remove a few more already-implemented features from the TODO list

Changes in v2:
- Add a few informational commands
- Clarify wording of filter documentation
- Include enum definitions instead of re-documenting them
- Reorganize log documentation; related sections should now be more proximate

 doc/develop/logging.rst | 242 +++-
 1 file changed, 115 insertions(+), 127 deletions(-)

diff --git a/doc/develop/logging.rst b/doc/develop/logging.rst
index 6a22062073..2b87bc2b4f 100644
--- a/doc/develop/logging.rst
+++ b/doc/develop/logging.rst
@@ -21,23 +21,13 @@ is visible from the basic console output.
 U-Boot's logging feature aims to satisfy this goal for both users and
 developers.
 
-
 Logging levels
 --
 
-There are a number logging levels available, in increasing order of verbosity:
-
-* LOGL_EMERG - Printed before U-Boot halts
-* LOGL_ALERT - Indicates action must be taken immediate or U-Boot will crash
-* LOGL_CRIT - Indicates a critical error that will cause boot failure
-* LOGL_ERR - Indicates an error that may cause boot failure
-* LOGL_WARNING - Warning about an unexpected condition
-* LOGL_NOTE - Important information about progress
-* LOGL_INFO - Information about normal boot progress
-* LOGL_DEBUG - Debug information (useful for debugging a driver or subsystem)
-* LOGL_DEBUG_CONTENT - Debug message showing full message content
-* LOGL_DEBUG_IO - Debug message showing hardware I/O access
+There are a number logging levels available.
 
+.. kernel-doc:: include/log.h
+   :identifiers: log_level_t
 
 Logging category
 
@@ -46,16 +36,8 @@ Logging can come from a wide variety of places within 
U-Boot. Each log message
 has a category which is intended to allow messages to be filtered according to
 their source.
 
-The following main categories are defined:
-
-* LOGC_NONE - Unknown category (e.g. a debug() statement)
-* UCLASS\_... - Related to a particular uclass (e.g. UCLASS_USB)
-* LOGC_ARCH - Related to architecture-specific code
-* LOGC_BOARD - Related to board-specific code
-* LOGC_CORE - Related to core driver-model support
-* LOGC_DT - Related to device tree control
-* LOGC_EFI - Related to EFI implementation
-
+.. kernel-doc:: include/log.h
+   :identifiers: log_category_t
 
 Enabling logging
 
@@ -72,7 +54,6 @@ If CONFIG_LOG is not set, then no logging will be available.
 The above have SPL and TPL versions also, e.g. CONFIG_SPL_LOG_MAX_LEVEL and
 CONFIG_TPL_LOG_MAX_LEVEL.
 
-
 Temporary logging within a single file
 --
 
@@ -86,46 +67,8 @@ to enable building in of all logging statements in a single 
file. Put it at
 the top of the file, before any #includes.
 
 To actually get U-Boot to output this you need to also set the default logging
-level - e.g. set CONFIG_LOG_DEFAULT_LEVEL to 7 (LOGL_DEBUG) or more. Otherwise
-debug output is suppressed and will not be generated.
-
-
-Convenience functions
--
-
-A number of convenience functions are available to shorten the code needed
-for logging:
-
-* log_err(_fmt...)
-* log_warning(_fmt...)
-* log_notice(_fmt...)
-* log_info(_fmt...)
-* log_debug(_fmt...)
-* log_content(_fmt...)
-* log_io(_fmt...)
-
-With these the log level is implicit in the name. The category is set by
-LOG_CATEGORY, which you can only define once per file, above all #includes, 
e.g.
-
-.. code-block:: c
-
-   #define LOG_CATEGORY LOGC_ALLOC
-
-Remember that all uclasses IDs are log categories too.
-
-
-Log command

-
-The 'log' command provides access to several features:
-
-* level - access the default log level
-* format - access the console log format
-* rec - output a log record
-* test - run tests
-
-Type 'help log' for details.
-
+level - e.g. set CONFIG_LOG_DEFAULT_LEVEL to 7 (:c:type:`LOGL_DEBUG`) or more.
+Otherwise debug output is suppressed and will not be generated.
 
 Using DEBUG
 ---
@@ -142,51 +85,6 @@ at the top of a file, then it takes precedence. This means 
that debug()
 statements will result in output to the console and this output will not be
 logged.
 
-
-Logging destinations
-
-
-If logging information goes nowhere then it serves no purpose. U-Boot provides
-several possible determinations for logging information, all of which can be
-enabled or disabled independently:
-
-* console - goes to stdout
-* syslog - broadcast RFC 3164 messages to syslog servers on UDP port 514
-
-The syslog driver sends the value of environmental variable 'log_hostname' as
-HOSTNAME if available.
-
-
-Log format
---
-
-You can control the log format using the 'log format' command. The basic

[PATCH v3 22/23] doc: Add log kerneldocs to documentation

2020-10-17 Thread Sean Anderson
The functions in log.h are already mostly documented, so add them to the
generated documentation.

Signed-off-by: Sean Anderson 
Reviewed-by: Simon Glass 
---

(no changes since v2)

Changes in v2:
- Add % before constants in kerneldocs
- Document log_level_t and log_category_t members

 doc/develop/logging.rst |   5 ++
 include/log.h   | 142 +---
 2 files changed, 95 insertions(+), 52 deletions(-)

diff --git a/doc/develop/logging.rst b/doc/develop/logging.rst
index 7ce8482ab6..6a22062073 100644
--- a/doc/develop/logging.rst
+++ b/doc/develop/logging.rst
@@ -288,3 +288,8 @@ information
 Add a command to add new log records and delete existing records.
 
 Provide additional log() functions - e.g. logc() to specify the category
+
+Logging API
+---
+.. kernel-doc:: include/log.h
+   :internal:
diff --git a/include/log.h b/include/log.h
index 9f57957aae..639f9e1199 100644
--- a/include/log.h
+++ b/include/log.h
@@ -17,52 +17,86 @@
 
 struct cmd_tbl;
 
-/** Log levels supported, ranging from most to least important */
+/**
+ * enum log_level_t - Log levels supported, ranging from most to least 
important
+ */
 enum log_level_t {
-   LOGL_EMERG = 0, /* U-Boot is unstable */
-   LOGL_ALERT, /* Action must be taken immediately */
-   LOGL_CRIT,  /* Critical conditions */
-   LOGL_ERR,   /* Error that prevents something from working */
-   LOGL_WARNING,   /* Warning may prevent optimial operation */
-   LOGL_NOTICE,/* Normal but significant condition, printf() */
-   LOGL_INFO,  /* General information message */
-   LOGL_DEBUG, /* Basic debug-level message */
-   LOGL_DEBUG_CONTENT, /* Debug message showing full message content */
-   LOGL_DEBUG_IO,  /* Debug message showing hardware I/O access */
+   /** @LOGL_EMERG: U-Boot is unstable */
+   LOGL_EMERG = 0,
+   /** @LOGL_ALERT: Action must be taken immediately */
+   LOGL_ALERT,
+   /** @LOGL_CRIT: Critical conditions */
+   LOGL_CRIT,
+   /** @LOGL_ERR: Error that prevents something from working */
+   LOGL_ERR,
+   /** @LOGL_WARNING: Warning may prevent optimial operation */
+   LOGL_WARNING,
+   /** @LOGL_NOTICE: Normal but significant condition, printf() */
+   LOGL_NOTICE,
+   /** @LOGL_INFO: General information message */
+   LOGL_INFO,
+   /** @LOGL_DEBUG: Basic debug-level message */
+   LOGL_DEBUG,
+   /** @LOGL_DEBUG_CONTENT: Debug message showing full message content */
+   LOGL_DEBUG_CONTENT,
+   /** @LOGL_DEBUG_IO: Debug message showing hardware I/O access */
+   LOGL_DEBUG_IO,
 
+   /** @LOGL_COUNT: Total number of valid log levels */
LOGL_COUNT,
+   /** @LOGL_NONE: Used to indicate that there is no valid log level */
LOGL_NONE,
 
-   LOGL_LEVEL_MASK = 0xf,  /* Mask for valid log levels */
-   LOGL_FORCE_DEBUG = 0x10, /* Mask to force output due to LOG_DEBUG */
+   /** @LOGL_LEVEL_MASK: Mask for valid log levels */
+   LOGL_LEVEL_MASK = 0xf,
+   /** @LOGL_FORCE_DEBUG: Mask to force output due to LOG_DEBUG */
+   LOGL_FORCE_DEBUG = 0x10,
 
+   /** @LOGL_FIRST: The first, most-important log level */
LOGL_FIRST = LOGL_EMERG,
+   /** @LOGL_MAX: The last, least-important log level */
LOGL_MAX = LOGL_DEBUG_IO,
 };
 
 /**
- * Log categories supported. Most of these correspond to uclasses (i.e.
- * enum uclass_id) but there are also some more generic categories
+ * enum log_category_t - Log categories supported.
+ *
+ * Log categories between %LOGC_FIRST and %LOGC_NONE correspond to uclasses
+ * (i.e.  uclass_id), but there are also some more generic categories.
  */
 enum log_category_t {
+   /** @LOGC_FIRST: First log category */
LOGC_FIRST = 0, /* First part mirrors UCLASS_... */
 
+   /** @LOGC_NONE: Default log category */
LOGC_NONE = UCLASS_COUNT,   /* First number is after all uclasses */
-   LOGC_ARCH,  /* Related to arch-specific code */
-   LOGC_BOARD, /* Related to board-specific code */
-   LOGC_CORE,  /* Related to core features (non-driver-model) */
-   LOGC_DM,/* Core driver-model */
-   LOGC_DT,/* Device-tree */
-   LOGC_EFI,   /* EFI implementation */
-   LOGC_ALLOC, /* Memory allocation */
-   LOGC_SANDBOX,   /* Related to the sandbox board */
-   LOGC_BLOBLIST,  /* Bloblist */
-   LOGC_DEVRES,/* Device resources (devres_... functions) */
-   /* Advanced Configuration and Power Interface (ACPI) */
+   /** @LOGC_ARCH: Related to arch-specific code */
+   LOGC_ARCH,
+   /** @LOGC_BOARD: Related to board-specific code */
+   LOGC_BOARD,
+   /** @LOGC_CORE: Related to core features (non-driver-model) */
+   LOGC_CORE,
+   /** @LOGC_DM: Core driver-model */
+   

[PATCH v3 13/23] test: Add test for LOGFF_MIN

2020-10-17 Thread Sean Anderson
This tests log filters matching on a minimum level.

Signed-off-by: Sean Anderson 
Reviewed-by: Simon Glass 
---

Changes in v3:
- Modified to be completely written in C

 test/log/log_test.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/test/log/log_test.c b/test/log/log_test.c
index e4ab999a7d..ea4fc6bc30 100644
--- a/test/log/log_test.c
+++ b/test/log/log_test.c
@@ -359,3 +359,26 @@ int log_test_level_deny(struct unit_test_state *uts)
return 0;
 }
 LOG_TEST_FLAGS(log_test_level_deny, UT_TESTF_CONSOLE_REC);
+
+/* Check matching based on minimum level */
+int log_test_min(struct unit_test_state *uts)
+{
+   int filt1, filt2;
+
+   filt1 = log_add_filter_flags("console", NULL, LOGL_WARNING, NULL,
+LOGFF_LEVEL_MIN);
+   ut_assert(filt1 >= 0);
+   filt2 = log_add_filter_flags("console", NULL, LOGL_INFO, NULL,
+LOGFF_DENY | LOGFF_LEVEL_MIN);
+   ut_assert(filt2 >= 0);
+
+   ut_assertok(console_record_reset_enable());
+   log_run();
+   check_log_entries_flags_levels(EXPECT_LOG | EXPECT_DIRECT,
+  LOGL_WARNING, LOGL_INFO - 1);
+
+   ut_assertok(log_remove_filter("console", filt1));
+   ut_assertok(log_remove_filter("console", filt2));
+   return 0;
+}
+LOG_TEST_FLAGS(log_test_min, UT_TESTF_CONSOLE_REC);
-- 
2.28.0



[PATCH v3 15/23] cmd: log: Split off log level parsing

2020-10-17 Thread Sean Anderson
Move parsing of log level into its own function so it can be re-used. This
also adds support for using log level names instead of just the integer
equivalent.

Signed-off-by: Sean Anderson 
Reviewed-by: Simon Glass 
---

(no changes since v2)

Changes in v2:
- Print an error message if the log level is invalid.

 cmd/log.c | 29 +++--
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/cmd/log.c b/cmd/log.c
index 82e3a7b62f..651e50358c 100644
--- a/cmd/log.c
+++ b/cmd/log.c
@@ -11,23 +11,40 @@
 
 static char log_fmt_chars[LOGF_COUNT] = "clFLfm";
 
+static enum log_level_t parse_log_level(char *const arg)
+{
+   enum log_level_t ret;
+   ulong level;
+
+   if (!strict_strtoul(arg, 10, )) {
+   if (level > _LOG_MAX_LEVEL) {
+   printf("Only log levels <= %d are supported\n",
+  _LOG_MAX_LEVEL);
+   return LOGL_NONE;
+   }
+   return level;
+   }
+
+   ret = log_get_level_by_name(arg);
+   if (ret == LOGL_NONE)
+   printf("Unknown log level \"%s\"\n", arg);
+   return ret;
+}
+
 static int do_log_level(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
 {
if (argc > 1) {
-   long log_level = simple_strtol(argv[1], NULL, 10);
+   enum log_level_t log_level = parse_log_level(argv[1]);
 
-   if (log_level < 0 || log_level > _LOG_MAX_LEVEL) {
-   printf("Only log levels <= %d are supported\n",
-  _LOG_MAX_LEVEL);
+   if (log_level == LOGL_NONE)
return CMD_RET_FAILURE;
-   }
gd->default_log_level = log_level;
} else {
printf("Default log level: %d\n", gd->default_log_level);
}
 
-   return 0;
+   return CMD_RET_SUCCESS;
 }
 
 static int do_log_format(struct cmd_tbl *cmdtp, int flag, int argc,
-- 
2.28.0



[PATCH v3 19/23] test: Add a test for getopt

2020-10-17 Thread Sean Anderson
A few of these tests were inspired by those in glibc. The syntax for
invoking test_getopt is a bit funky, but it's necessary so that the CPP can
parse the arguments correctly.

Signed-off-by: Sean Anderson 
---

(no changes since v1)

 test/lib/Makefile |   1 +
 test/lib/getopt.c | 123 ++
 2 files changed, 124 insertions(+)
 create mode 100644 test/lib/getopt.c

diff --git a/test/lib/Makefile b/test/lib/Makefile
index 22236f8587..3140c2dad7 100644
--- a/test/lib/Makefile
+++ b/test/lib/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_ERRNO_STR) += test_errno_str.o
 obj-$(CONFIG_UT_LIB_ASN1) += asn1.o
 obj-$(CONFIG_UT_LIB_RSA) += rsa.o
 obj-$(CONFIG_AES) += test_aes.o
+obj-$(CONFIG_GETOPT) += getopt.o
diff --git a/test/lib/getopt.c b/test/lib/getopt.c
new file mode 100644
index 00..3c68b93c8a
--- /dev/null
+++ b/test/lib/getopt.c
@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2020 Sean Anderson 
+ *
+ * Portions of these tests were inspired by glibc's posix/bug-getopt1.c and
+ * posix/tst-getopt-cancel.c
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static int do_test_getopt(struct unit_test_state *uts, int line,
+ struct getopt_state *gs, const char *optstring,
+ int args, char *argv[], int expected_count,
+ int expected[])
+{
+   int opt;
+
+   getopt_init_state(gs);
+   for (int i = 0; i < expected_count; i++) {
+   opt = getopt_silent(gs, args, argv, optstring);
+   if (expected[i] != opt) {
+   /*
+* Fudge the line number so we can tell which test
+* failed
+*/
+   ut_failf(uts, __FILE__, line, __func__,
+"expected[i] == getopt()",
+"Expected '%c' (%d) with i=%d, got '%c' (%d)",
+expected[i], expected[i], i, opt, opt);
+   return CMD_RET_FAILURE;
+   }
+   }
+
+   opt = getopt_silent(gs, args, argv, optstring);
+   if (opt != -1) {
+   ut_failf(uts, __FILE__, line, __func__,
+"getopt() != -1",
+"Expected -1, got '%c' (%d)", opt, opt);
+   return CMD_RET_FAILURE;
+   }
+
+   return 0;
+}
+
+#define test_getopt(optstring, argv, expected) do { \
+   int ret = do_test_getopt(uts, __LINE__, , optstring, \
+ARRAY_SIZE(argv) - 1, argv, \
+ARRAY_SIZE(expected), expected); \
+   if (ret) \
+   return ret; \
+} while (0)
+
+static int lib_test_getopt(struct unit_test_state *uts)
+{
+   struct getopt_state gs;
+
+   /* Happy path */
+   test_getopt("ab:c",
+   ((char *[]){ "program", "-cb", "x", "-a", "foo", 0 }),
+   ((int []){ 'c', 'b', 'a' }));
+   ut_asserteq(4, gs.index);
+
+   /* Make sure we pick up the optional argument */
+   test_getopt("a::b:c",
+   ((char *[]){ "program", "-cbx", "-a", "foo", 0 }),
+   ((int []){ 'c', 'b', 'a' }));
+   ut_asserteq(4, gs.index);
+
+   /* Test required arguments */
+   test_getopt("a:b", ((char *[]){ "program", "-a", 0 }),
+   ((int []){ ':' }));
+   ut_asserteq('a', gs.opt);
+   test_getopt("a:b", ((char *[]){ "program", "-b", "-a", 0 }),
+   ((int []){ 'b', ':' }));
+   ut_asserteq('a', gs.opt);
+
+   /* Test invalid arguments */
+   test_getopt("ab:c", ((char *[]){ "program", "-d", 0 }),
+   ((int []){ '?' }));
+   ut_asserteq('d', gs.opt);
+
+   /* Test arg */
+   test_getopt("a::b:c",
+   ((char *[]){ "program", "-a", 0 }),
+   ((int []){ 'a' }));
+   ut_asserteq(2, gs.index);
+   ut_assertnull(gs.arg);
+
+   test_getopt("a::b:c",
+   ((char *[]){ "program", "-afoo", 0 }),
+   ((int []){ 'a' }));
+   ut_asserteq(2, gs.index);
+   ut_assertnonnull(gs.arg);
+   ut_asserteq_str("foo", gs.arg);
+
+   test_getopt("a::b:c",
+   ((char *[]){ "program", "-a", "foo", 0 }),
+   ((int []){ 'a' }));
+   ut_asserteq(3, gs.index);
+   ut_assertnonnull(gs.arg);
+   ut_asserteq_str("foo", gs.arg);
+
+   test_getopt("a::b:c",
+   ((char *[]){ "program", "-bfoo", 0 }),
+   ((int []){ 'b' }));
+   ut_asserteq(2, gs.index);
+   ut_assertnonnull(gs.arg);
+   ut_asserteq_str("foo", gs.arg);
+
+   test_getopt("a::b:c",
+   ((char *[]){ "program", "-b", "foo", 0 }),
+   ((int []){ 'b' }));
+   ut_asserteq(3, gs.index);
+   ut_assertnonnull(gs.arg);
+   ut_asserteq_str("foo", 

[PATCH v3 16/23] cmd: log: Add commands to list categories and drivers

2020-10-17 Thread Sean Anderson
This allows users to query which categories and drivers are available on
their system. This allows them to construct filter-add commands without
(e.g.) adjusting the log format to show categories and drivers.

Signed-off-by: Sean Anderson 
Reviewed-by: Simon Glass 
---

Changes in v3:
- Document assumption that erroneous results from log_get_cat_name begin with
  '<'

Changes in v2:
- New

 cmd/log.c | 35 +++
 common/log.c  |  1 +
 include/log.h |  5 +++--
 3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/cmd/log.c b/cmd/log.c
index 651e50358c..8d8d8a8172 100644
--- a/cmd/log.c
+++ b/cmd/log.c
@@ -47,6 +47,37 @@ static int do_log_level(struct cmd_tbl *cmdtp, int flag, int 
argc,
return CMD_RET_SUCCESS;
 }
 
+static int do_log_categories(struct cmd_tbl *cmdtp, int flag, int argc,
+char *const argv[])
+{
+   enum log_category_t cat;
+   const char *name;
+
+   for (cat = LOGC_FIRST; cat < LOGC_COUNT; cat++) {
+   name = log_get_cat_name(cat);
+   /*
+* Invalid category names (e.g.  or ) begin
+* with '<'.
+*/
+   if (name[0] == '<')
+   continue;
+   printf("%s\n", name);
+   }
+
+   return CMD_RET_SUCCESS;
+}
+
+static int do_log_drivers(struct cmd_tbl *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+   struct log_device *ldev;
+
+   list_for_each_entry(ldev, >log_head, sibling_node)
+   printf("%s\n", ldev->drv->name);
+
+   return CMD_RET_SUCCESS;
+}
+
 static int do_log_format(struct cmd_tbl *cmdtp, int flag, int argc,
 char *const argv[])
 {
@@ -123,6 +154,8 @@ static int do_log_rec(struct cmd_tbl *cmdtp, int flag, int 
argc,
 #ifdef CONFIG_SYS_LONGHELP
 static char log_help_text[] =
"level - get/set log level\n"
+   "categories - list log categories\n"
+   "drivers - list log drivers\n"
"log format  - set log output format.  is a string where\n"
"\teach letter indicates something that should be displayed:\n"
"\tc=category, l=level, F=file, L=line number, f=function, m=msg\n"
@@ -134,6 +167,8 @@ static char log_help_text[] =
 
 U_BOOT_CMD_WITH_SUBCMDS(log, "log system", log_help_text,
U_BOOT_SUBCMD_MKENT(level, 2, 1, do_log_level),
+   U_BOOT_SUBCMD_MKENT(categories, 1, 1, do_log_categories),
+   U_BOOT_SUBCMD_MKENT(drivers, 1, 1, do_log_drivers),
U_BOOT_SUBCMD_MKENT(format, 2, 1, do_log_format),
U_BOOT_SUBCMD_MKENT(rec, 7, 1, do_log_rec),
 );
diff --git a/common/log.c b/common/log.c
index 259d922987..c4a14e191a 100644
--- a/common/log.c
+++ b/common/log.c
@@ -41,6 +41,7 @@ static const char *const log_level_name[LOGL_COUNT] = {
"IO",
 };
 
+/* All error responses MUST begin with '<' */
 const char *log_get_cat_name(enum log_category_t cat)
 {
const char *name;
diff --git a/include/log.h b/include/log.h
index d5e09a838f..9f57957aae 100644
--- a/include/log.h
+++ b/include/log.h
@@ -407,8 +407,9 @@ struct log_filter {
  * log_get_cat_name() - Get the name of a category
  *
  * @cat: Category to look up
- * @return category name (which may be a uclass driver name) if found, or
- *  "" if invalid, or "" if not found
+ * @return: category name (which may be a uclass driver name) if found, or
+ *"" if invalid, or "" if not found. All error
+ *responses begin with '<'.
  */
 const char *log_get_cat_name(enum log_category_t cat);
 
-- 
2.28.0



[PATCH v3 17/23] cmd: log: Make "log level" print all log levels

2020-10-17 Thread Sean Anderson
This makes the log level command print all valid log levels. The default
log level is annotated. This provides an easy way to see which log levels
are compiled-in.

Signed-off-by: Sean Anderson 
Reviewed-by: Simon Glass 
---

(no changes since v2)

Changes in v2:
- New

 cmd/log.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/cmd/log.c b/cmd/log.c
index 8d8d8a8172..596bc73f47 100644
--- a/cmd/log.c
+++ b/cmd/log.c
@@ -34,14 +34,20 @@ static enum log_level_t parse_log_level(char *const arg)
 static int do_log_level(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
 {
+   enum log_level_t log_level;
+
if (argc > 1) {
-   enum log_level_t log_level = parse_log_level(argv[1]);
+   log_level = parse_log_level(argv[1]);
 
if (log_level == LOGL_NONE)
return CMD_RET_FAILURE;
gd->default_log_level = log_level;
} else {
-   printf("Default log level: %d\n", gd->default_log_level);
+   for (log_level = LOGL_FIRST; log_level <= _LOG_MAX_LEVEL;
+log_level++)
+   printf("%s%s\n", log_get_level_name(log_level),
+  log_level == gd->default_log_level ?
+  " (default)" : "");
}
 
return CMD_RET_SUCCESS;
@@ -153,7 +159,7 @@ static int do_log_rec(struct cmd_tbl *cmdtp, int flag, int 
argc,
 
 #ifdef CONFIG_SYS_LONGHELP
 static char log_help_text[] =
-   "level - get/set log level\n"
+   "level [] - get/set log level\n"
"categories - list log categories\n"
"drivers - list log drivers\n"
"log format  - set log output format.  is a string where\n"
-- 
2.28.0



[PATCH v3 12/23] log: Add filter flag to match greater than a log level

2020-10-17 Thread Sean Anderson
This is the complement of the existing behavior to match only messages with
a log level less than a threshold. This is primarily useful in conjunction
with LOGFF_DENY.

Signed-off-by: Sean Anderson 
Reviewed-by: Simon Glass 
---

(no changes since v1)

 common/log.c  | 12 +---
 include/log.h | 10 ++
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/common/log.c b/common/log.c
index 878800b3bb..259d922987 100644
--- a/common/log.c
+++ b/common/log.c
@@ -159,11 +159,17 @@ static bool log_passes_filters(struct log_device *ldev, 
struct log_rec *rec)
}
 
list_for_each_entry(filt, >filter_head, sibling_node) {
-   if (rec->level > filt->max_level)
+   if (filt->flags & LOGFF_LEVEL_MIN) {
+   if (rec->level < filt->level)
+   continue;
+   } else if (rec->level > filt->level) {
continue;
+   }
+
if ((filt->flags & LOGFF_HAS_CAT) &&
!log_has_cat(filt->cat_list, rec->cat))
continue;
+
if (filt->file_list &&
!log_has_file(filt->file_list, rec->file))
continue;
@@ -238,7 +244,7 @@ int _log(enum log_category_t cat, enum log_level_t level, 
const char *file,
 }
 
 int log_add_filter_flags(const char *drv_name, enum log_category_t cat_list[],
-enum log_level_t max_level, const char *file_list,
+enum log_level_t level, const char *file_list,
 int flags)
 {
struct log_filter *filt;
@@ -266,7 +272,7 @@ int log_add_filter_flags(const char *drv_name, enum 
log_category_t cat_list[],
break;
}
}
-   filt->max_level = max_level;
+   filt->level = level;
if (file_list) {
filt->file_list = strdup(file_list);
if (!filt->file_list) {
diff --git a/include/log.h b/include/log.h
index 8cabe82eb5..d5e09a838f 100644
--- a/include/log.h
+++ b/include/log.h
@@ -365,6 +365,8 @@ enum log_filter_flags {
LOGFF_HAS_CAT   = 1 << 0,
/** @LOGFF_DENY: Filter denies matching messages */
LOGFF_DENY  = 1 << 1,
+   /** @LOGFF_LEVEL_MIN: Filter's level is a minimum, not a maximum */
+   LOGFF_LEVEL_MIN = 1 << 2,
 };
 
 /**
@@ -380,7 +382,7 @@ enum log_filter_flags {
  * @cat_list: List of categories to allow (terminated by %LOGC_END). If empty
  * then all categories are permitted. Up to LOGF_MAX_CATEGORIES entries
  * can be provided
- * @max_level: Maximum log level to allow
+ * @level: Maximum (or minimum, if LOGFF_MIN_LEVEL) log level to allow
  * @file_list: List of files to allow, separated by comma. If NULL then all
  * files are permitted
  * @sibling_node: Next filter in the list of filters for this log device
@@ -389,7 +391,7 @@ struct log_filter {
int filter_num;
int flags;
enum log_category_t cat_list[LOGF_MAX_CATEGORIES];
-   enum log_level_t max_level;
+   enum log_level_t level;
const char *file_list;
struct list_head sibling_node;
 };
@@ -490,14 +492,14 @@ int do_log_test(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[]);
  * @cat_list: List of categories to allow (terminated by %LOGC_END). If empty
  * then all categories are permitted. Up to LOGF_MAX_CATEGORIES entries
  * can be provided
- * @max_level: Maximum log level to allow
+ * @level: Maximum (or minimum, if LOGFF_LEVEL_MIN) log level to allow
  * @file_list: List of files to allow, separated by comma. If NULL then all
  * files are permitted
  * @return the sequence number of the new filter (>=0) if the filter was added,
  * or a -ve value on error
  */
 int log_add_filter_flags(const char *drv_name, enum log_category_t cat_list[],
-enum log_level_t max_level, const char *file_list,
+enum log_level_t level, const char *file_list,
 int flags);
 
 /**
-- 
2.28.0



[PATCH v3 14/23] cmd: log: Use sub-commands for log

2020-10-17 Thread Sean Anderson
This reduces duplicate code, and makes adding new sub-commands easier.

Signed-off-by: Sean Anderson 
Reviewed-by: Simon Glass 
---

(no changes since v1)

 cmd/log.c | 31 ---
 1 file changed, 4 insertions(+), 27 deletions(-)

diff --git a/cmd/log.c b/cmd/log.c
index d20bfdf744..82e3a7b62f 100644
--- a/cmd/log.c
+++ b/cmd/log.c
@@ -103,30 +103,6 @@ static int do_log_rec(struct cmd_tbl *cmdtp, int flag, int 
argc,
return 0;
 }
 
-static struct cmd_tbl log_sub[] = {
-   U_BOOT_CMD_MKENT(level, CONFIG_SYS_MAXARGS, 1, do_log_level, "", ""),
-   U_BOOT_CMD_MKENT(format, CONFIG_SYS_MAXARGS, 1, do_log_format, "", ""),
-   U_BOOT_CMD_MKENT(rec, CONFIG_SYS_MAXARGS, 1, do_log_rec, "", ""),
-};
-
-static int do_log(struct cmd_tbl *cmdtp, int flag, int argc, char *const 
argv[])
-{
-   struct cmd_tbl *cp;
-
-   if (argc < 2)
-   return CMD_RET_USAGE;
-
-   /* drop initial "log" arg */
-   argc--;
-   argv++;
-
-   cp = find_cmd_tbl(argv[0], log_sub, ARRAY_SIZE(log_sub));
-   if (cp)
-   return cp->cmd(cmdtp, flag, argc, argv);
-
-   return CMD_RET_USAGE;
-}
-
 #ifdef CONFIG_SYS_LONGHELP
 static char log_help_text[] =
"level - get/set log level\n"
@@ -139,7 +115,8 @@ static char log_help_text[] =
;
 #endif
 
-U_BOOT_CMD(
-   log, CONFIG_SYS_MAXARGS, 1, do_log,
-   "log system", log_help_text
+U_BOOT_CMD_WITH_SUBCMDS(log, "log system", log_help_text,
+   U_BOOT_SUBCMD_MKENT(level, 2, 1, do_log_level),
+   U_BOOT_SUBCMD_MKENT(format, 2, 1, do_log_format),
+   U_BOOT_SUBCMD_MKENT(rec, 7, 1, do_log_rec),
 );
-- 
2.28.0



[PATCH v3 10/23] test: log: Give tests names instead of numbers

2020-10-17 Thread Sean Anderson
Now that the log test command is no more, we can give the log tests proper
names.

Signed-off-by: Sean Anderson 
---

Changes in v3:
- New

 test/log/log_test.c | 48 ++---
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/test/log/log_test.c b/test/log/log_test.c
index c5ca6dcb7a..4ac378fdad 100644
--- a/test/log/log_test.c
+++ b/test/log/log_test.c
@@ -72,7 +72,7 @@ static int do_check_log_entries(struct unit_test_state *uts, 
int flags, int min,
 #define check_log_entries_none() check_log_entries_flags(0)
 
 /* Check a category filter using the first category */
-int log_test_00(struct unit_test_state *uts)
+int log_test_cat_allow(struct unit_test_state *uts)
 {
enum log_category_t cat_list[] = {
log_uc_cat(UCLASS_MMC), log_uc_cat(UCLASS_SPI),
@@ -94,10 +94,10 @@ int log_test_00(struct unit_test_state *uts)
ut_assertok(log_remove_filter("console", filt));
return 0;
 }
-LOG_TEST_FLAGS(log_test_00, UT_TESTF_CONSOLE_REC);
+LOG_TEST_FLAGS(log_test_cat_allow, UT_TESTF_CONSOLE_REC);
 
 /* Check a category filter that should block log entries */
-int log_test_02(struct unit_test_state *uts)
+int log_test_cat_deny_implicit(struct unit_test_state *uts)
 {
enum log_category_t cat_list[] = {
log_uc_cat(UCLASS_MMC),  LOGC_NONE, LOGC_END
@@ -114,10 +114,10 @@ int log_test_02(struct unit_test_state *uts)
ut_assertok(log_remove_filter("console", filt));
return 0;
 }
-LOG_TEST_FLAGS(log_test_02, UT_TESTF_CONSOLE_REC);
+LOG_TEST_FLAGS(log_test_cat_deny_implicit, UT_TESTF_CONSOLE_REC);
 
 /* Check passing and failing file filters */
-int log_test_03(struct unit_test_state *uts)
+int log_test_file(struct unit_test_state *uts)
 {
int filt;
 
@@ -135,10 +135,10 @@ int log_test_03(struct unit_test_state *uts)
ut_assertok(log_remove_filter("console", filt));
return 0;
 }
-LOG_TEST_FLAGS(log_test_03, UT_TESTF_CONSOLE_REC);
+LOG_TEST_FLAGS(log_test_file, UT_TESTF_CONSOLE_REC);
 
 /* Check a passing file filter (second in list) */
-int log_test_05(struct unit_test_state *uts)
+int log_test_file_second(struct unit_test_state *uts)
 {
int filt;
 
@@ -152,10 +152,10 @@ int log_test_05(struct unit_test_state *uts)
ut_assertok(log_remove_filter("console", filt));
return 0;
 }
-LOG_TEST_FLAGS(log_test_05, UT_TESTF_CONSOLE_REC);
+LOG_TEST_FLAGS(log_test_file_second, UT_TESTF_CONSOLE_REC);
 
 /* Check a passing file filter (middle of list) */
-int log_test_06(struct unit_test_state *uts)
+int log_test_file_mid(struct unit_test_state *uts)
 {
int filt;
 
@@ -170,10 +170,10 @@ int log_test_06(struct unit_test_state *uts)
ut_assertok(log_remove_filter("console", filt));
return 0;
 }
-LOG_TEST_FLAGS(log_test_06, UT_TESTF_CONSOLE_REC);
+LOG_TEST_FLAGS(log_test_file_mid, UT_TESTF_CONSOLE_REC);
 
 /* Check a log level filter */
-int log_test_07(struct unit_test_state *uts)
+int log_test_level(struct unit_test_state *uts)
 {
int filt;
 
@@ -188,10 +188,10 @@ int log_test_07(struct unit_test_state *uts)
ut_assertok(log_remove_filter("console", filt));
return 0;
 }
-LOG_TEST_FLAGS(log_test_07, UT_TESTF_CONSOLE_REC);
+LOG_TEST_FLAGS(log_test_level, UT_TESTF_CONSOLE_REC);
 
 /* Check two filters, one of which passes everything */
-int log_test_08(struct unit_test_state *uts)
+int log_test_double(struct unit_test_state *uts)
 {
int filt1, filt2;
 
@@ -208,10 +208,10 @@ int log_test_08(struct unit_test_state *uts)
ut_assertok(log_remove_filter("console", filt2));
return 0;
 }
-LOG_TEST_FLAGS(log_test_08, UT_TESTF_CONSOLE_REC);
+LOG_TEST_FLAGS(log_test_double, UT_TESTF_CONSOLE_REC);
 
 /* Check three filters, which together pass everything */
-int log_test_09(struct unit_test_state *uts)
+int log_test_triple(struct unit_test_state *uts)
 {
int filt1, filt2, filt3;
 
@@ -231,9 +231,9 @@ int log_test_09(struct unit_test_state *uts)
ut_assertok(log_remove_filter("console", filt3));
return 0;
 }
-LOG_TEST_FLAGS(log_test_09, UT_TESTF_CONSOLE_REC);
+LOG_TEST_FLAGS(log_test_triple, UT_TESTF_CONSOLE_REC);
 
-int do_log_test_10(struct unit_test_state *uts)
+int do_log_test_helpers(struct unit_test_state *uts)
 {
int i;
 
@@ -255,18 +255,18 @@ int do_log_test_10(struct unit_test_state *uts)
return 0;
 }
 
-int log_test_10(struct unit_test_state *uts)
+int log_test_helpers(struct unit_test_state *uts)
 {
int ret;
 
gd->log_fmt = LOGF_TEST;
-   ret = do_log_test_10(uts);
+   ret = do_log_test_helpers(uts);
gd->log_fmt = log_get_default_format();
return ret;
 }
-LOG_TEST_FLAGS(log_test_10, UT_TESTF_CONSOLE_REC);
+LOG_TEST_FLAGS(log_test_helpers, UT_TESTF_CONSOLE_REC);
 
-int do_log_test_11(struct unit_test_state *uts)
+int do_log_test_disable(struct unit_test_state *uts)
 {
ut_assertok(console_record_reset_enable());

[PATCH v3 11/23] test: Add tests for LOGFF_DENY

2020-10-17 Thread Sean Anderson
This adds some tests for log filters which deny if they match.

Signed-off-by: Sean Anderson 
Reviewed-by: Simon Glass 
---

Changes in v3:
- Modified to be completely written in C

 test/log/log_test.c | 67 +
 1 file changed, 67 insertions(+)

diff --git a/test/log/log_test.c b/test/log/log_test.c
index 4ac378fdad..e4ab999a7d 100644
--- a/test/log/log_test.c
+++ b/test/log/log_test.c
@@ -292,3 +292,70 @@ int log_test_disable(struct unit_test_state *uts)
return ret;
 }
 LOG_TEST_FLAGS(log_test_disable, UT_TESTF_CONSOLE_REC);
+
+/* Check denying based on category */
+int log_test_cat_deny(struct unit_test_state *uts)
+{
+   int filt1, filt2;
+   enum log_category_t cat_list[] = {
+   log_uc_cat(UCLASS_SPI), LOGC_END
+   };
+
+   filt1 = log_add_filter("console", cat_list, LOGL_MAX, NULL);
+   ut_assert(filt1 >= 0);
+   filt2 = log_add_filter_flags("console", cat_list, LOGL_MAX, NULL,
+LOGFF_DENY);
+   ut_assert(filt2 >= 0);
+
+   ut_assertok(console_record_reset_enable());
+   log_run_cat(UCLASS_SPI);
+   check_log_entries_none();
+
+   ut_assertok(log_remove_filter("console", filt1));
+   ut_assertok(log_remove_filter("console", filt2));
+   return 0;
+}
+LOG_TEST_FLAGS(log_test_cat_deny, UT_TESTF_CONSOLE_REC);
+
+/* Check denying based on file */
+int log_test_file_deny(struct unit_test_state *uts)
+{
+   int filt1, filt2;
+
+   filt1 = log_add_filter("console", NULL, LOGL_MAX, "file");
+   ut_assert(filt1 >= 0);
+   filt2 = log_add_filter_flags("console", NULL, LOGL_MAX, "file",
+LOGFF_DENY);
+   ut_assert(filt2 >= 0);
+
+   ut_assertok(console_record_reset_enable());
+   log_run_file("file");
+   check_log_entries_none();
+
+   ut_assertok(log_remove_filter("console", filt1));
+   ut_assertok(log_remove_filter("console", filt2));
+   return 0;
+}
+LOG_TEST_FLAGS(log_test_file_deny, UT_TESTF_CONSOLE_REC);
+
+/* Check denying based on level */
+int log_test_level_deny(struct unit_test_state *uts)
+{
+   int filt1, filt2;
+
+   filt1 = log_add_filter("console", NULL, LOGL_INFO, NULL);
+   ut_assert(filt1 >= 0);
+   filt2 = log_add_filter_flags("console", NULL, LOGL_WARNING, NULL,
+LOGFF_DENY);
+   ut_assert(filt2 >= 0);
+
+   ut_assertok(console_record_reset_enable());
+   log_run();
+   check_log_entries_flags_levels(EXPECT_LOG | EXPECT_DIRECT,
+  LOGL_WARNING + 1, _LOG_MAX_LEVEL);
+
+   ut_assertok(log_remove_filter("console", filt1));
+   ut_assertok(log_remove_filter("console", filt2));
+   return 0;
+}
+LOG_TEST_FLAGS(log_test_level_deny, UT_TESTF_CONSOLE_REC);
-- 
2.28.0



[PATCH v3 09/23] test: log: Convert log_test from python to C

2020-10-17 Thread Sean Anderson
When rebasing this series I had to renumber all my log tests because
someone made another log test in the meantime. This involved updaing a
number in several places (C and python), and it wasn't checked by the
compiler. So I though "how hard could it be to just rewrite in C?" And
though it wasn't hard, it *was* tedious. Tests are numbered the same as
before to allow for easier review.

A note that if a test fails, everything after it will probably also fail.
This is because that test won't clean up its filters.  There's no easy way
to do the cleanup, except perhaps removing all filters in a wrapper
function.

Signed-off-by: Sean Anderson 
---

Changes in v3:
- New

 cmd/log.c |   6 -
 include/test/log.h|   2 +
 test/log/log_test.c   | 446 ++
 test/log/syslog_test.h|   2 -
 test/py/tests/test_log.py | 104 -
 5 files changed, 260 insertions(+), 300 deletions(-)

diff --git a/cmd/log.c b/cmd/log.c
index 16a6ef7539..d20bfdf744 100644
--- a/cmd/log.c
+++ b/cmd/log.c
@@ -105,9 +105,6 @@ static int do_log_rec(struct cmd_tbl *cmdtp, int flag, int 
argc,
 
 static struct cmd_tbl log_sub[] = {
U_BOOT_CMD_MKENT(level, CONFIG_SYS_MAXARGS, 1, do_log_level, "", ""),
-#if CONFIG_IS_ENABLED(LOG_TEST)
-   U_BOOT_CMD_MKENT(test, 2, 1, do_log_test, "", ""),
-#endif
U_BOOT_CMD_MKENT(format, CONFIG_SYS_MAXARGS, 1, do_log_format, "", ""),
U_BOOT_CMD_MKENT(rec, CONFIG_SYS_MAXARGS, 1, do_log_rec, "", ""),
 };
@@ -133,9 +130,6 @@ static int do_log(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
 #ifdef CONFIG_SYS_LONGHELP
 static char log_help_text[] =
"level - get/set log level\n"
-#if CONFIG_IS_ENABLED(LOG_TEST)
-   "log test - run log tests\n"
-#endif
"log format  - set log output format.  is a string where\n"
"\teach letter indicates something that should be displayed:\n"
"\tc=category, l=level, F=file, L=line number, f=function, m=msg\n"
diff --git a/include/test/log.h b/include/test/log.h
index c661cde75a..772e197806 100644
--- a/include/test/log.h
+++ b/include/test/log.h
@@ -10,6 +10,8 @@
 
 #include 
 
+#define LOGF_TEST (BIT(LOGF_FUNC) | BIT(LOGF_MSG))
+
 /* Declare a new logging test */
 #define LOG_TEST(_name) UNIT_TEST(_name, 0, log_test)
 
diff --git a/test/log/log_test.c b/test/log/log_test.c
index 6a60ff6be3..c5ca6dcb7a 100644
--- a/test/log/log_test.c
+++ b/test/log/log_test.c
@@ -9,12 +9,17 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+
+DECLARE_GLOBAL_DATA_PTR;
 
 /* emit some sample log records in different ways, for testing */
-static int log_run(enum uclass_id cat, const char *file)
+static int do_log_run(int cat, const char *file)
 {
int i;
 
+   gd->log_fmt = LOGF_TEST;
debug("debug\n");
for (i = LOGL_FIRST; i < LOGL_COUNT; i++) {
log(cat, i, "log %d\n", i);
@@ -22,203 +27,268 @@ static int log_run(enum uclass_id cat, const char *file)
 i);
}
 
+   gd->log_fmt = log_get_default_format();
return 0;
 }
 
-static int log_test(int testnum)
+#define log_run_cat(cat) do_log_run(cat, "file")
+#define log_run_file(file) do_log_run(UCLASS_SPI, file)
+#define log_run() do_log_run(UCLASS_SPI, "file")
+
+#define EXPECT_LOG BIT(0)
+#define EXPECT_DIRECT BIT(1)
+#define EXPECT_EXTRA BIT(2)
+
+static int do_check_log_entries(struct unit_test_state *uts, int flags, int 
min,
+   int max)
 {
-   int ret;
+   int i;
 
-   printf("test %d\n", testnum);
-   switch (testnum) {
-   case 0: {
-   /* Check a category filter using the first category */
-   enum log_category_t cat_list[] = {
-   log_uc_cat(UCLASS_MMC), log_uc_cat(UCLASS_SPI),
-   LOGC_NONE, LOGC_END
-   };
-
-   ret = log_add_filter("console", cat_list, LOGL_MAX, NULL);
-   if (ret < 0)
-   return ret;
-   log_run(UCLASS_MMC, "file");
-   ret = log_remove_filter("console", ret);
-   if (ret < 0)
-   return ret;
-   break;
-   }
-   case 1: {
-   /* Check a category filter using the second category */
-   enum log_category_t cat_list[] = {
-   log_uc_cat(UCLASS_MMC), log_uc_cat(UCLASS_SPI), LOGC_END
-   };
-
-   ret = log_add_filter("console", cat_list, LOGL_MAX, NULL);
-   if (ret < 0)
-   return ret;
-   log_run(UCLASS_SPI, "file");
-   ret = log_remove_filter("console", ret);
-   if (ret < 0)
-   return ret;
-   break;
-   }
-   case 2: {
-   /* Check a category filter that should block log entries */
-   enum log_category_t cat_list[] = {
-   

[PATCH v3 07/23] log: Add function to create a filter with flags

2020-10-17 Thread Sean Anderson
This function exposes a way to specify flags when creating a filter.

Signed-off-by: Sean Anderson 
Reviewed-by: Simon Glass 
---

(no changes since v1)

 common/log.c  |  6 --
 include/log.h | 29 +++--
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/common/log.c b/common/log.c
index 63e8df820e..d43c9eb15d 100644
--- a/common/log.c
+++ b/common/log.c
@@ -233,8 +233,9 @@ int _log(enum log_category_t cat, enum log_level_t level, 
const char *file,
return 0;
 }
 
-int log_add_filter(const char *drv_name, enum log_category_t cat_list[],
-  enum log_level_t max_level, const char *file_list)
+int log_add_filter_flags(const char *drv_name, enum log_category_t cat_list[],
+enum log_level_t max_level, const char *file_list,
+int flags)
 {
struct log_filter *filt;
struct log_device *ldev;
@@ -248,6 +249,7 @@ int log_add_filter(const char *drv_name, enum 
log_category_t cat_list[],
if (!filt)
return -ENOMEM;
 
+   filt->flags = flags;
if (cat_list) {
filt->flags |= LOGFF_HAS_CAT;
for (i = 0; ; i++) {
diff --git a/include/log.h b/include/log.h
index 8d0227a384..4b33815f01 100644
--- a/include/log.h
+++ b/include/log.h
@@ -472,6 +472,25 @@ enum log_fmt {
 /* Handle the 'log test' command */
 int do_log_test(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
 
+/**
+ * log_add_filter_flags() - Add a new filter to a log device, specifying flags
+ *
+ * @drv_name: Driver name to add the filter to (since each driver only has a
+ * single device)
+ * @flags: Flags for this filter (LOGFF_...)
+ * @cat_list: List of categories to allow (terminated by %LOGC_END). If empty
+ * then all categories are permitted. Up to LOGF_MAX_CATEGORIES entries
+ * can be provided
+ * @max_level: Maximum log level to allow
+ * @file_list: List of files to allow, separated by comma. If NULL then all
+ * files are permitted
+ * @return the sequence number of the new filter (>=0) if the filter was added,
+ * or a -ve value on error
+ */
+int log_add_filter_flags(const char *drv_name, enum log_category_t cat_list[],
+enum log_level_t max_level, const char *file_list,
+int flags);
+
 /**
  * log_add_filter() - Add a new filter to a log device
  *
@@ -486,8 +505,14 @@ int do_log_test(struct cmd_tbl *cmdtp, int flag, int argc, 
char *const argv[]);
  * @return the sequence number of the new filter (>=0) if the filter was added,
  * or a -ve value on error
  */
-int log_add_filter(const char *drv_name, enum log_category_t cat_list[],
-  enum log_level_t max_level, const char *file_list);
+static inline int log_add_filter(const char *drv_name,
+enum log_category_t cat_list[],
+enum log_level_t max_level,
+const char *file_list)
+{
+   return log_add_filter_flags(drv_name, cat_list, max_level, file_list,
+   0);
+}
 
 /**
  * log_remove_filter() - Remove a filter from a log device
-- 
2.28.0



[PATCH v3 08/23] log: Add filter flag to deny on match

2020-10-17 Thread Sean Anderson
Without this flag, log filters can only explicitly accept messages.
Allowing denial makes it easier to filter certain subsystems. Unlike
allow-ing filters, deny-ing filters are added to the beginning of the
filter list. This should do the Right Thing most of the time, but it's
less-universal than allowing filters to be inserted anywhere. If this
becomes a problem, then perhaps log_filter_add* should take a filter number
to insert before/after.

Signed-off-by: Sean Anderson 
Reviewed-by: Simon Glass 
---

(no changes since v1)

 common/log.c  | 12 ++--
 include/log.h | 11 ++-
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/common/log.c b/common/log.c
index d43c9eb15d..878800b3bb 100644
--- a/common/log.c
+++ b/common/log.c
@@ -167,7 +167,11 @@ static bool log_passes_filters(struct log_device *ldev, 
struct log_rec *rec)
if (filt->file_list &&
!log_has_file(filt->file_list, rec->file))
continue;
-   return true;
+
+   if (filt->flags & LOGFF_DENY)
+   return false;
+   else
+   return true;
}
 
return false;
@@ -271,7 +275,11 @@ int log_add_filter_flags(const char *drv_name, enum 
log_category_t cat_list[],
}
}
filt->filter_num = ldev->next_filter_num++;
-   list_add_tail(>sibling_node, >filter_head);
+   /* Add deny filters to the beginning of the list */
+   if (flags & LOGFF_DENY)
+   list_add(>sibling_node, >filter_head);
+   else
+   list_add_tail(>sibling_node, >filter_head);
 
return filt->filter_num;
 
diff --git a/include/log.h b/include/log.h
index 4b33815f01..8cabe82eb5 100644
--- a/include/log.h
+++ b/include/log.h
@@ -357,13 +357,22 @@ enum {
LOGF_MAX_CATEGORIES = 5,/* maximum categories per filter */
 };
 
+/**
+ * enum log_filter_flags - Flags which modify a filter
+ */
 enum log_filter_flags {
-   LOGFF_HAS_CAT   = 1 << 0,   /* Filter has a category list */
+   /** @LOGFF_HAS_CAT: Filter has a category list */
+   LOGFF_HAS_CAT   = 1 << 0,
+   /** @LOGFF_DENY: Filter denies matching messages */
+   LOGFF_DENY  = 1 << 1,
 };
 
 /**
  * struct log_filter - criterial to filter out log messages
  *
+ * If a message matches all criteria, then it is allowed. If LOGFF_DENY is set,
+ * then it is denied instead.
+ *
  * @filter_num: Sequence number of this filter.  This is returned when adding a
  * new filter, and must be provided when removing a previously added
  * filter.
-- 
2.28.0



[PATCH v3 06/23] log: Expose some helper functions

2020-10-17 Thread Sean Anderson
These functions are required by "cmd: log: Add commands to manipulate
filters" and "test: Add a test for log filter-*".

Signed-off-by: Sean Anderson 
Reviewed-by: Simon Glass 
---

(no changes since v2)

Changes in v2:
- Expose log_has_cat and log_has_file for filter tests

 common/log.c  | 23 +++
 include/log.h | 31 +++
 2 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/common/log.c b/common/log.c
index 5a588c4152..63e8df820e 100644
--- a/common/log.c
+++ b/common/log.c
@@ -96,7 +96,7 @@ enum log_level_t log_get_level_by_name(const char *name)
return LOGL_NONE;
 }
 
-static struct log_device *log_device_find_by_name(const char *drv_name)
+struct log_device *log_device_find_by_name(const char *drv_name)
 {
struct log_device *ldev;
 
@@ -108,15 +108,7 @@ static struct log_device *log_device_find_by_name(const 
char *drv_name)
return NULL;
 }
 
-/**
- * log_has_cat() - check if a log category exists within a list
- *
- * @cat_list: List of categories to check, at most LOGF_MAX_CATEGORIES entries
- * long, terminated by LC_END if fewer
- * @cat: Category to search for
- * @return true if @cat is in @cat_list, else false
- */
-static bool log_has_cat(enum log_category_t cat_list[], enum log_category_t 
cat)
+bool log_has_cat(enum log_category_t cat_list[], enum log_category_t cat)
 {
int i;
 
@@ -128,16 +120,7 @@ static bool log_has_cat(enum log_category_t cat_list[], 
enum log_category_t cat)
return false;
 }
 
-/**
- * log_has_file() - check if a file is with a list
- *
- * @file_list: List of files to check, separated by comma
- * @file: File to check for. This string is matched against the end of each
- * file in the list, i.e. ignoring any preceding path. The list is
- * intended to consist of relative pathnames, e.g. common/main.c,cmd/log.c
- * @return true if @file is in @file_list, else false
- */
-static bool log_has_file(const char *file_list, const char *file)
+bool log_has_file(const char *file_list, const char *file)
 {
int file_len = strlen(file);
const char *s, *p;
diff --git a/include/log.h b/include/log.h
index 3487c936c9..8d0227a384 100644
--- a/include/log.h
+++ b/include/log.h
@@ -425,6 +425,37 @@ const char *log_get_level_name(enum log_level_t level);
  */
 enum log_level_t log_get_level_by_name(const char *name);
 
+/**
+ * log_device_find_by_name() - Look up a log device by its driver's name
+ *
+ * @drv_name: Name of the driver
+ * @return the log device, or NULL if not found
+ */
+struct log_device *log_device_find_by_name(const char *drv_name);
+
+/**
+ * log_has_cat() - check if a log category exists within a list
+ *
+ * @cat_list: List of categories to check, at most %LOGF_MAX_CATEGORIES entries
+ * long, terminated by %LC_END if fewer
+ * @cat: Category to search for
+ *
+ * Return: ``true`` if @cat is in @cat_list, else ``false``
+ */
+bool log_has_cat(enum log_category_t cat_list[], enum log_category_t cat);
+
+/**
+ * log_has_file() - check if a file is with a list
+ *
+ * @file_list: List of files to check, separated by comma
+ * @file: File to check for. This string is matched against the end of each
+ * file in the list, i.e. ignoring any preceding path. The list is
+ * intended to consist of relative pathnames, e.g. common/main.c,cmd/log.c
+ *
+ * Return: ``true`` if @file is in @file_list, else ``false``
+ */
+bool log_has_file(const char *file_list, const char *file);
+
 /* Log format flags (bit numbers) for gd->log_fmt. See log_fmt_chars */
 enum log_fmt {
LOGF_CAT= 0,
-- 
2.28.0



[PATCH v3 05/23] log: Use CONFIG_IS_ENABLED() for LOG_TEST

2020-10-17 Thread Sean Anderson
Checkpatch complains about using #ifdef for CONFIG variables.

Signed-off-by: Sean Anderson 
Reviewed-by: Simon Glass 
---

(no changes since v1)

 cmd/log.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cmd/log.c b/cmd/log.c
index 6afe6ead25..16a6ef7539 100644
--- a/cmd/log.c
+++ b/cmd/log.c
@@ -105,7 +105,7 @@ static int do_log_rec(struct cmd_tbl *cmdtp, int flag, int 
argc,
 
 static struct cmd_tbl log_sub[] = {
U_BOOT_CMD_MKENT(level, CONFIG_SYS_MAXARGS, 1, do_log_level, "", ""),
-#ifdef CONFIG_LOG_TEST
+#if CONFIG_IS_ENABLED(LOG_TEST)
U_BOOT_CMD_MKENT(test, 2, 1, do_log_test, "", ""),
 #endif
U_BOOT_CMD_MKENT(format, CONFIG_SYS_MAXARGS, 1, do_log_format, "", ""),
@@ -133,7 +133,7 @@ static int do_log(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
 #ifdef CONFIG_SYS_LONGHELP
 static char log_help_text[] =
"level - get/set log level\n"
-#ifdef CONFIG_LOG_TEST
+#if CONFIG_IS_ENABLED(LOG_TEST)
"log test - run log tests\n"
 #endif
"log format  - set log output format.  is a string where\n"
-- 
2.28.0



[PATCH v3 04/23] log: Add new category names to log_cat_name

2020-10-17 Thread Sean Anderson
Without every category between LOGC_NONE and LOGC_COUNT present in
log_cat_name, log_get_cat_by_name will dereference NULL pointers if it
doesn't find a name early enough.

Fixes: c3aed5db59 ("sandbox: spi: Add more logging")
Fixes: a5c13b68e7 ("sandbox: log: Add a category for sandbox")
Fixes: 9f407d4ef0 ("Add core support for a bloblist to convey data from SPL")
Fixes: cce61fc428 ("dm: devres: Convert to use logging")
Fixes: 7ca2850cbc ("dm: core: Add basic ACPI support")

Signed-off-by: Sean Anderson 
Reviewed-by: Simon Glass 
---

(no changes since v2)

Changes in v2:
- Add compiletime assert on size of log_cat_name

 common/log.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/common/log.c b/common/log.c
index f3d9f4a728..5a588c4152 100644
--- a/common/log.c
+++ b/common/log.c
@@ -13,7 +13,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-static const char *const log_cat_name[LOGC_COUNT - LOGC_NONE] = {
+static const char *const log_cat_name[] = {
"none",
"arch",
"board",
@@ -21,6 +21,11 @@ static const char *const log_cat_name[LOGC_COUNT - 
LOGC_NONE] = {
"driver-model",
"device-tree",
"efi",
+   "alloc",
+   "sandbox",
+   "bloblist",
+   "devres",
+   "acpi",
 };
 
 static const char *const log_level_name[LOGL_COUNT] = {
@@ -40,6 +45,9 @@ const char *log_get_cat_name(enum log_category_t cat)
 {
const char *name;
 
+   compiletime_assert(ARRAY_SIZE(log_cat_name) == LOGC_COUNT - LOGC_NONE,
+  "missing logging category name");
+
if (cat < 0 || cat >= LOGC_COUNT)
return "";
if (cat >= LOGC_NONE)
-- 
2.28.0



[PATCH v3 03/23] log: Add additional const qualifier to arrays

2020-10-17 Thread Sean Anderson
Both these arrays and their members are const. Fixes checkpatch complaint.

Signed-off-by: Sean Anderson 
Reviewed-by: Simon Glass 
---

(no changes since v2)

Changes in v2:
- New

 common/log.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/log.c b/common/log.c
index 807babbb6e..f3d9f4a728 100644
--- a/common/log.c
+++ b/common/log.c
@@ -13,7 +13,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-static const char *log_cat_name[LOGC_COUNT - LOGC_NONE] = {
+static const char *const log_cat_name[LOGC_COUNT - LOGC_NONE] = {
"none",
"arch",
"board",
@@ -23,7 +23,7 @@ static const char *log_cat_name[LOGC_COUNT - LOGC_NONE] = {
"efi",
 };
 
-static const char *log_level_name[LOGL_COUNT] = {
+static const char *const log_level_name[LOGL_COUNT] = {
"EMERG",
"ALERT",
"CRIT",
-- 
2.28.0



[PATCH v3 01/23] log: Fix missing negation of ENOMEM

2020-10-17 Thread Sean Anderson
Errors returned should be negative.

Fixes: 45fac9fc18 ("log: Correct missing free() on error in log_add_filter()")

Signed-off-by: Sean Anderson 
Reviewed-by: Heinrich Schuchardt 
---

(no changes since v1)

 common/log.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/log.c b/common/log.c
index 1b10f6f180..807babbb6e 100644
--- a/common/log.c
+++ b/common/log.c
@@ -273,7 +273,7 @@ int log_add_filter(const char *drv_name, enum 
log_category_t cat_list[],
if (file_list) {
filt->file_list = strdup(file_list);
if (!filt->file_list) {
-   ret = ENOMEM;
+   ret = -ENOMEM;
goto err;
}
}
-- 
2.28.0



[PATCH v3 02/23] log: Fix incorrect documentation of log_filter.cat_list

2020-10-17 Thread Sean Anderson
Logging category lists are terminated by LOGC_END, not LOGC_NONE.

Fixes: e9c8d49d54 ("log: Add an implementation of logging")

Signed-off-by: Sean Anderson 
Reviewed-by: Heinrich Schuchardt 
---

(no changes since v2)

Changes in v2:
- Also fix misdocumentation of for log_add_filter()

 include/log.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/log.h b/include/log.h
index 4acc087b2e..3487c936c9 100644
--- a/include/log.h
+++ b/include/log.h
@@ -368,7 +368,7 @@ enum log_filter_flags {
  * new filter, and must be provided when removing a previously added
  * filter.
  * @flags: Flags for this filter (LOGFF_...)
- * @cat_list: List of categories to allow (terminated by LOGC_none). If empty
+ * @cat_list: List of categories to allow (terminated by %LOGC_END). If empty
  * then all categories are permitted. Up to LOGF_MAX_CATEGORIES entries
  * can be provided
  * @max_level: Maximum log level to allow
@@ -446,7 +446,7 @@ int do_log_test(struct cmd_tbl *cmdtp, int flag, int argc, 
char *const argv[]);
  *
  * @drv_name: Driver name to add the filter to (since each driver only has a
  * single device)
- * @cat_list: List of categories to allow (terminated by LOGC_none). If empty
+ * @cat_list: List of categories to allow (terminated by %LOGC_END). If empty
  * then all categories are permitted. Up to LOGF_MAX_CATEGORIES entries
  * can be provided
  * @max_level: Maximum log level to allow
-- 
2.28.0



[PATCH v3 00/23] log: Add commands for manipulating filters

2020-10-17 Thread Sean Anderson
This series adds several commands for adding, listing, and removing log filters.
It also adds getopt, since the filter-add command needs to have several
optional arguments to be complete, and positional specification of those
arguments would have been difficult.

Changes in v3:
- Convert log_test from python to C
- Document assumption that erroneous results from log_get_cat_name begin with
  '<'
- Fix heading level of Filters section
- Remove a few more already-implemented features from the TODO list
- Update copyright for log_filter.c

Changes in v2:
- Add % before constants in kerneldocs
- Add a few informational commands
- Add compiletime assert on size of log_cat_name
- Add const qualifier to log_*_name
- Add option to remove all filters to filter-remove
- Clarify filter-* help text
- Clarify wording of filter documentation
- Converted log filter-* tests to C from python
- Document log_level_t and log_category_t members
- Expand documentation of getopt() to include examples
- Expose log_has_cat and log_has_file for filter tests
- Include enum definitions instead of re-documenting them
- Print an error message if the log level is invalid.
- Remove opt prefix from getopt_state members
- Reorganize log documentation; related sections should now be more proximate

Sean Anderson (23):
  log: Fix missing negation of ENOMEM
  log: Fix incorrect documentation of log_filter.cat_list
  log: Add additional const qualifier to arrays
  log: Add new category names to log_cat_name
  log: Use CONFIG_IS_ENABLED() for LOG_TEST
  log: Expose some helper functions
  log: Add function to create a filter with flags
  log: Add filter flag to deny on match
  test: log: Convert log_test from python to C
  test: log: Give tests names instead of numbers
  test: Add tests for LOGFF_DENY
  log: Add filter flag to match greater than a log level
  test: Add test for LOGFF_MIN
  cmd: log: Use sub-commands for log
  cmd: log: Split off log level parsing
  cmd: log: Add commands to list categories and drivers
  cmd: log: Make "log level" print all log levels
  lib: Add getopt
  test: Add a test for getopt
  cmd: log: Add commands to manipulate filters
  test: Add a test for log filter-*
  doc: Add log kerneldocs to documentation
  doc: Update logging documentation

 MAINTAINERS   |   1 +
 cmd/Kconfig   |   1 +
 cmd/log.c | 353 ++---
 common/log.c  |  66 ++---
 doc/api/getopt.rst|   8 +
 doc/api/index.rst |   1 +
 doc/develop/logging.rst   | 245 +
 include/getopt.h  | 130 +
 include/log.h | 212 +++
 include/test/log.h|   3 +
 lib/Kconfig   |   5 +
 lib/Makefile  |   1 +
 lib/getopt.c  | 125 +
 test/lib/Makefile |   1 +
 test/lib/getopt.c | 123 +
 test/log/Makefile |   1 +
 test/log/log_filter.c | 108 
 test/log/log_test.c   | 536 +-
 test/log/syslog_test.h|   2 -
 test/py/tests/test_log.py | 104 
 20 files changed, 1482 insertions(+), 544 deletions(-)
 create mode 100644 doc/api/getopt.rst
 create mode 100644 include/getopt.h
 create mode 100644 lib/getopt.c
 create mode 100644 test/lib/getopt.c
 create mode 100644 test/log/log_filter.c

-- 
2.28.0



Re: [PATCH 1/1] Makefile: consistent include path for out of tree build

2020-10-17 Thread Tom Rini
On Sat, Oct 17, 2020 at 07:57:31PM +0200, Heinrich Schuchardt wrote:
> On 10/17/20 6:49 PM, Tom Rini wrote:
> > On Sat, Oct 17, 2020 at 06:42:53PM +0200, Heinrich Schuchardt wrote:
> >
> >> When compiling path/foo.c, we should not add -I$(scr_tree)/path to the gcc
> >> flags. Otherwise we get different build results for in tree and out of tree
> >> builds.
> >>
> >> Signed-off-by: Heinrich Schuchardt 
> >> ---
> >>  scripts/Makefile.lib | 6 ++
> >>  1 file changed, 2 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> >> index 56e9d54242..98817084f4 100644
> >> --- a/scripts/Makefile.lib
> >> +++ b/scripts/Makefile.lib
> >> @@ -136,10 +136,8 @@ __cpp_flags = $(_cpp_flags)
> >>  else
> >>
> >>  # -I$(obj) locates generated .h files
> >> -# $(call addtree,-I$(obj)) locates .h files in srctree, from generated .c 
> >> files
> >> -#   and locates generated .h files
> >> -# FIXME: Replace both with specific CFLAGS* statements in the makefiles
> >> -__c_flags = $(if $(obj),$(call addtree,-I$(src)) -I$(obj)) \
> >> +# FIXME: Replace with specific CFLAGS* statements in the makefiles
> >> +__c_flags = $(if $(obj), -I$(obj)) \
> >>  $(call flags,_c_flags)
> >>  __a_flags = $(call flags,_a_flags)
> >>  __cpp_flags = $(call flags,_cpp_flags)
> >
> > As this will make future re-syncs with the Kbuild system harder, NAK.
> >
> > I would suggest making this problem show up in the Linux kernel, get it
> > addressed there, and then backport.  Or, if it can't be and it's just
> > another problem related to our last sync being on v4.19, help syncing up
> > with v4.20 and so forth would be greatly appreciated.
> >
> 
> v4.20 does not even exist as maintained release. Current Linux stable is
> v5.9.

Exactly.  We move to v4.20 then v5.0 and so on.

> addtree() does not exist in scripts/Makefile.lib of Linux next-20201016.
> So I would not expect the kernel people to fix our outdated script.

Exactly.  I'm saying you need to reproduce the problem in the Linux
kernel.  If it's not a problem there, we just need to continue
re-syncing until the problem is fixed.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 1/1] Makefile: consistent include path for out of tree build

2020-10-17 Thread Heinrich Schuchardt
On 10/17/20 6:49 PM, Tom Rini wrote:
> On Sat, Oct 17, 2020 at 06:42:53PM +0200, Heinrich Schuchardt wrote:
>
>> When compiling path/foo.c, we should not add -I$(scr_tree)/path to the gcc
>> flags. Otherwise we get different build results for in tree and out of tree
>> builds.
>>
>> Signed-off-by: Heinrich Schuchardt 
>> ---
>>  scripts/Makefile.lib | 6 ++
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>> index 56e9d54242..98817084f4 100644
>> --- a/scripts/Makefile.lib
>> +++ b/scripts/Makefile.lib
>> @@ -136,10 +136,8 @@ __cpp_flags = $(_cpp_flags)
>>  else
>>
>>  # -I$(obj) locates generated .h files
>> -# $(call addtree,-I$(obj)) locates .h files in srctree, from generated .c 
>> files
>> -#   and locates generated .h files
>> -# FIXME: Replace both with specific CFLAGS* statements in the makefiles
>> -__c_flags   = $(if $(obj),$(call addtree,-I$(src)) -I$(obj)) \
>> +# FIXME: Replace with specific CFLAGS* statements in the makefiles
>> +__c_flags   = $(if $(obj), -I$(obj)) \
>>$(call flags,_c_flags)
>>  __a_flags   = $(call flags,_a_flags)
>>  __cpp_flags = $(call flags,_cpp_flags)
>
> As this will make future re-syncs with the Kbuild system harder, NAK.
>
> I would suggest making this problem show up in the Linux kernel, get it
> addressed there, and then backport.  Or, if it can't be and it's just
> another problem related to our last sync being on v4.19, help syncing up
> with v4.20 and so forth would be greatly appreciated.
>

v4.20 does not even exist as maintained release. Current Linux stable is
v5.9.

addtree() does not exist in scripts/Makefile.lib of Linux next-20201016.
So I would not expect the kernel people to fix our outdated script.

Best regards

Heinrich





Re: [PATCH 1/1] Makefile: consistent include path for out of tree build

2020-10-17 Thread Tom Rini
On Sat, Oct 17, 2020 at 06:42:53PM +0200, Heinrich Schuchardt wrote:

> When compiling path/foo.c, we should not add -I$(scr_tree)/path to the gcc
> flags. Otherwise we get different build results for in tree and out of tree
> builds.
> 
> Signed-off-by: Heinrich Schuchardt 
> ---
>  scripts/Makefile.lib | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 56e9d54242..98817084f4 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -136,10 +136,8 @@ __cpp_flags = $(_cpp_flags)
>  else
> 
>  # -I$(obj) locates generated .h files
> -# $(call addtree,-I$(obj)) locates .h files in srctree, from generated .c 
> files
> -#   and locates generated .h files
> -# FIXME: Replace both with specific CFLAGS* statements in the makefiles
> -__c_flags= $(if $(obj),$(call addtree,-I$(src)) -I$(obj)) \
> +# FIXME: Replace with specific CFLAGS* statements in the makefiles
> +__c_flags= $(if $(obj), -I$(obj)) \
> $(call flags,_c_flags)
>  __a_flags= $(call flags,_a_flags)
>  __cpp_flags = $(call flags,_cpp_flags)

As this will make future re-syncs with the Kbuild system harder, NAK.

I would suggest making this problem show up in the Linux kernel, get it
addressed there, and then backport.  Or, if it can't be and it's just
another problem related to our last sync being on v4.19, help syncing up
with v4.20 and so forth would be greatly appreciated.

-- 
Tom


signature.asc
Description: PGP signature


Re: [BUG] sandbox_defconfig fails to build with origin/master dadc1e38306b1c3

2020-10-17 Thread Heinrich Schuchardt
On 10/17/20 2:57 PM, Tom Rini wrote:
> On Sat, Oct 17, 2020 at 02:46:27PM +0200, Heinrich Schuchardt wrote:
>> On 10/17/20 2:15 PM, Tom Rini wrote:
>>> On Sat, Oct 17, 2020 at 01:43:25PM +0200, Heinrich Schuchardt wrote:
 Hello Tom, hello Simon,

 make sandbox_defconfig
 make V=1

 yields:

 make -f ./scripts/Makefile.build obj=test/log
   gcc -Wp,-MD,test/log/.syslog_test.o.d -nostdinc -isystem
 /usr/lib/gcc/x86_64-linux-gnu/10/include -Iinclude
 -I./arch/sandbox/include -include ./include/linux/kconfig.h -D__KERNEL__
 -D__UBOOT__ -Wall -Wstrict-prototypes -Wno-format-security -fno-builtin
 -ffreestanding -std=gnu11 -fshort-wchar -fno-strict-aliasing -fno-PIE
 -Os -fno-stack-protector -fno-delete-null-pointer-checks
 -Wno-stringop-truncation -Wno-maybe-uninitialized -fmacro-prefix-map=./=
 -g -fstack-usage -Wno-format-nonliteral -Wno-address-of-packed-member
 -Wno-unused-but-set-variable -Werror=date-time -Werror -D__SANDBOX__
 -U_FORTIFY_SOURCE -DCONFIG_ARCH_MAP_SYSMEM -fPIC -I/usr/include/SDL2
 -D_REENTRANT -pipe-DKBUILD_BASENAME='"syslog_test"'
 -DKBUILD_MODNAME='"syslog_test"' -c -o test/log/syslog_test.o
 test/log/syslog_test.c
 test/log/syslog_test.c:21:10: fatal error: syslog_test.h: No such file
 or directory
21 | #include 
   |  ^~~
 compilation terminated.

 git bisect:

 52d3df7fefe30b05677db9055e68c666a071d89a is the first bad commit
 commit 52d3df7fefe30b05677db9055e68c666a071d89a
 Author: Simon Glass 
 Date:   Sat Sep 12 11:13:34 2020 -0600

 log: Allow LOG_DEBUG to always enable log output

 @Tom:
 Why are such build failures not detected before merging?
>>>
>>> I think because we have no tests for building in the source directory.
>>
>> You are right
>>
>>make -j8 O=build
>>
>> works while
>>
>> make -j8
>>
>> fails.
>>
>> Why do we generate
>>
>> -I../test/log -Itest/log
>>
>> as gcc parameters when building out of source? If we avoid this, we will
>> get consistent test results.
>>
>>   gcc -Wp,-MD,test/log/.syslog_test.o.d -nostdinc -isystem
>> /usr/lib/gcc/x86_64-linux-gnu/10/include -Iinclude  -I../include
>> -I../arch/sandbox/include -include ../include/linux/kconfig.h
>> -I../test/log -Itest/log -D__KERNEL__ -D__UBOOT__ -Wall
>> -Wstrict-prototypes -Wno-format-security -fno-builtin -ffreestanding
>> -std=gnu11 -fshort-wchar -fno-strict-aliasing -fno-PIE -Os
>> -fno-stack-protector -fno-delete-null-pointer-checks
>> -Wno-stringop-truncation -Wno-maybe-uninitialized
>> -fmacro-prefix-map=../= -g -fstack-usage -Wno-format-nonliteral
>> -Wno-address-of-packed-member -Wno-unused-but-set-variable
>> -Werror=date-time -Werror -D__SANDBOX__ -U_FORTIFY_SOURCE
>> -DCONFIG_ARCH_MAP_SYSMEM -fPIC  -I/usr/include/SDL2 -D_REENTRANT -pipe
>>   -DKBUILD_BASENAME='"syslog_test"'  -DKBUILD_MODNAME='"syslog_test"' -c
>> -o test/log/syslog_test.o ../test/log/syslog_test.c
>
> A quick poke around and I don't see us doing anything special for CFLAGS
> there, so that's how kbuild as of our last sync with the Linux kernel
> works is I suppose the answer.
>

Cf.
[PATCH 1/1] Makefile: consistent include path for out of tree build
https://lists.denx.de/pipermail/u-boot/2020-October/429780.html

Best regards

Heinrich


[PATCH 1/1] Makefile: consistent include path for out of tree build

2020-10-17 Thread Heinrich Schuchardt
When compiling path/foo.c, we should not add -I$(scr_tree)/path to the gcc
flags. Otherwise we get different build results for in tree and out of tree
builds.

Signed-off-by: Heinrich Schuchardt 
---
 scripts/Makefile.lib | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 56e9d54242..98817084f4 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -136,10 +136,8 @@ __cpp_flags = $(_cpp_flags)
 else

 # -I$(obj) locates generated .h files
-# $(call addtree,-I$(obj)) locates .h files in srctree, from generated .c files
-#   and locates generated .h files
-# FIXME: Replace both with specific CFLAGS* statements in the makefiles
-__c_flags  = $(if $(obj),$(call addtree,-I$(src)) -I$(obj)) \
+# FIXME: Replace with specific CFLAGS* statements in the makefiles
+__c_flags  = $(if $(obj), -I$(obj)) \
  $(call flags,_c_flags)
 __a_flags  = $(call flags,_a_flags)
 __cpp_flags = $(call flags,_cpp_flags)
--
2.28.0



[PATCH] test: log: Actually test enable/disable output

2020-10-17 Thread Sean Anderson
Strings are truth-y in python, so ``assert 'anything'`` will always
evaluate to True. Fix the test so that it actually checks the output.

Fixes: 3d03ab6361 ("log: Add a way to enable/disable a log device")
Signed-off-by: Sean Anderson 
---

 test/py/tests/test_log.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/py/tests/test_log.py b/test/py/tests/test_log.py
index 275f9382d2..e8b2b17145 100644
--- a/test/py/tests/test_log.py
+++ b/test/py/tests/test_log.py
@@ -95,9 +95,9 @@ def test_log(u_boot_console):
 def test11():
 """Test use of log_device_set_enable()"""
 lines = run_test(11)
-assert 'log_test() default'
+assert 'log_test() default' == next(lines)
 # disabled should not be displayed
-assert 'log_test() enabled'
+assert 'log_test() enabled' == next(lines)
 
 # TODO(s...@chromium.org): Consider structuring this as separate tests
 cons = u_boot_console
-- 
2.28.0



RE: [PATCH v2 3/5] pinctrl: renesas: r8a77965: Add R8A774B1 PFC support

2020-10-17 Thread Biju Das
Hi Marek,

> Subject: Re: [PATCH v2 3/5] pinctrl: renesas: r8a77965: Add R8A774B1 PFC
> support
> 
> On 10/14/20 1:29 PM, Biju Das wrote:
> 
> Hi,
> 
> [...]
> 
>  Check with the linux maintainers please, surely there should be
>  some way to separate the extras in a way that's not too hard to
>  maintain, and thus reduce the resulting binary size. For U-Boot,
>  that is quite important already, I think the TFA can only load 1 MiB
> binary in total.
> >>>
> >>> I agree for bootloader size is important. So I will add macros as
> >>> per your
> >> suggestion (we don't need to look into linux for this).
> >>
> >> The PFC tables and clock tables are the same between U-Boot and
> >> Linux, so if you only change them in U-Boot, it will make it hard to
> >> synchronize the tables later with Linux again. Please fix this in Linux and
> synchronize to U-Boot.
> >
> > I have posted a patch for optimizing pin control size for RZ/G2N in
> > Linux [1]
> >
> > [1]
> > https://patchwork.kernel.org/project/linux-renesas-soc/patch/202010141
> > 10238.9600-1-biju.das...@bp.renesas.com/
> >
> > This approach will save ~ 6KB=(3x 2KB/SoC) of memory on RZ/G2[HMN] u-
> boot with multi dtb support.
> >
> > 1) By compiling out Automotive parts
> > $ size drivers/pinctrl/renesas/pfc-r8a77965.o
> >textdata bss dec hex filename
> >   46141   0   0   46141b43d drivers/pinctrl/renesas/pfc-
> r8a77965.o
> >
> > 2) without patch
> > $ size drivers/pinctrl/renesas/pfc-r8a77965.o
> >textdata bss dec hex filename
> >   48191   0   0   48191bc3f drivers/pinctrl/renesas/pfc-
> r8a77965.o
> 
> Have a look at the size of the image of rcar3_salvator-x_defconfig , it is 
> just a
> few kiB short of 1MiB , which is the hard limit. Any size reduction helps.

Yes I agree, we need to look into size reduction for Salvator-XS. But currently 
there is no issue. See the details below.

Case 1) R Car Salvator-XS u-boot size (u-boot-sh/master):

$ ls -al u-boot.bin
-rw-r--r-- 1 biju biju 1025067 Oct 17 12:47 u-boot.bin --> 0xFA42B (23 Kbytes 
to reach 1MB)

$ size u-boot
   textdata bss dec hex filename
 942180   36208   71632 1050020  1005a4 u-boot

$ size drivers/pinctrl/renesas/*.o
   textdata bss dec hex filename
 151332 288   1  151621   25045 drivers/pinctrl/renesas/built-in.o
   3811 288   141001004 drivers/pinctrl/renesas/pfc.o
  48123   0   0   48123bbfb drivers/pinctrl/renesas/pfc-r8a7795.o
  47939   0   0   47939bb43 drivers/pinctrl/renesas/pfc-r8a77965.o
  47751   0   0   47751ba87 drivers/pinctrl/renesas/pfc-r8a7796.o

Case 2) R Car Salvator-X u-boot size after adding RZ/G2[HMN] pin control support

$ ls -al u-boot.bin
-rw-r--r-- 1 biju biju 1027107 Oct 17 12:59 u-boot.bin --> 0xFAC23 (21 Kbytes 
to reach 1 MB)

$ size u-boot
   textdata bss dec hex filename
 944224   36208   71632 1052064  100da0 u-boot

$ size drivers/pinctrl/renesas/*.o
   textdata bss dec hex filename
 151868 288   1  152157   2525d drivers/pinctrl/renesas/built-in.o
   3811 288   141001004 drivers/pinctrl/renesas/pfc.o
  48375   0   0   48375bcf7 drivers/pinctrl/renesas/pfc-r8a7795.o
  48191   0   0   48191bc3f drivers/pinctrl/renesas/pfc-r8a77965.o
  47751   0   0   47751ba87 drivers/pinctrl/renesas/pfc-r8a7796.o


Regards,
Biju


Re: [PATCH] test: Fix sandbox tests failing to build

2020-10-17 Thread Tom Rini
On Tue, Oct 13, 2020 at 03:20:52PM -0400, Sean Anderson wrote:

> syslog_test.h is in test/log/, not include/
> 
> Fixes: 52d3df7fef ("log: Allow LOG_DEBUG to always enable log output")
> Signed-off-by: Sean Anderson 
> Reviewed-by: Simon Glass 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [BUG] sandbox_defconfig fails to build with origin/master dadc1e38306b1c3

2020-10-17 Thread Tom Rini
On Sat, Oct 17, 2020 at 02:46:27PM +0200, Heinrich Schuchardt wrote:
> On 10/17/20 2:15 PM, Tom Rini wrote:
> > On Sat, Oct 17, 2020 at 01:43:25PM +0200, Heinrich Schuchardt wrote:
> >> Hello Tom, hello Simon,
> >>
> >> make sandbox_defconfig
> >> make V=1
> >>
> >> yields:
> >>
> >> make -f ./scripts/Makefile.build obj=test/log
> >>   gcc -Wp,-MD,test/log/.syslog_test.o.d -nostdinc -isystem
> >> /usr/lib/gcc/x86_64-linux-gnu/10/include -Iinclude
> >> -I./arch/sandbox/include -include ./include/linux/kconfig.h -D__KERNEL__
> >> -D__UBOOT__ -Wall -Wstrict-prototypes -Wno-format-security -fno-builtin
> >> -ffreestanding -std=gnu11 -fshort-wchar -fno-strict-aliasing -fno-PIE
> >> -Os -fno-stack-protector -fno-delete-null-pointer-checks
> >> -Wno-stringop-truncation -Wno-maybe-uninitialized -fmacro-prefix-map=./=
> >> -g -fstack-usage -Wno-format-nonliteral -Wno-address-of-packed-member
> >> -Wno-unused-but-set-variable -Werror=date-time -Werror -D__SANDBOX__
> >> -U_FORTIFY_SOURCE -DCONFIG_ARCH_MAP_SYSMEM -fPIC -I/usr/include/SDL2
> >> -D_REENTRANT -pipe-DKBUILD_BASENAME='"syslog_test"'
> >> -DKBUILD_MODNAME='"syslog_test"' -c -o test/log/syslog_test.o
> >> test/log/syslog_test.c
> >> test/log/syslog_test.c:21:10: fatal error: syslog_test.h: No such file
> >> or directory
> >>21 | #include 
> >>   |  ^~~
> >> compilation terminated.
> >>
> >> git bisect:
> >>
> >> 52d3df7fefe30b05677db9055e68c666a071d89a is the first bad commit
> >> commit 52d3df7fefe30b05677db9055e68c666a071d89a
> >> Author: Simon Glass 
> >> Date:   Sat Sep 12 11:13:34 2020 -0600
> >>
> >> log: Allow LOG_DEBUG to always enable log output
> >>
> >> @Tom:
> >> Why are such build failures not detected before merging?
> >
> > I think because we have no tests for building in the source directory.
> 
> You are right
> 
>make -j8 O=build
> 
> works while
> 
> make -j8
> 
> fails.
> 
> Why do we generate
> 
> -I../test/log -Itest/log
> 
> as gcc parameters when building out of source? If we avoid this, we will
> get consistent test results.
> 
>   gcc -Wp,-MD,test/log/.syslog_test.o.d -nostdinc -isystem
> /usr/lib/gcc/x86_64-linux-gnu/10/include -Iinclude  -I../include
> -I../arch/sandbox/include -include ../include/linux/kconfig.h
> -I../test/log -Itest/log -D__KERNEL__ -D__UBOOT__ -Wall
> -Wstrict-prototypes -Wno-format-security -fno-builtin -ffreestanding
> -std=gnu11 -fshort-wchar -fno-strict-aliasing -fno-PIE -Os
> -fno-stack-protector -fno-delete-null-pointer-checks
> -Wno-stringop-truncation -Wno-maybe-uninitialized
> -fmacro-prefix-map=../= -g -fstack-usage -Wno-format-nonliteral
> -Wno-address-of-packed-member -Wno-unused-but-set-variable
> -Werror=date-time -Werror -D__SANDBOX__ -U_FORTIFY_SOURCE
> -DCONFIG_ARCH_MAP_SYSMEM -fPIC  -I/usr/include/SDL2 -D_REENTRANT -pipe
>   -DKBUILD_BASENAME='"syslog_test"'  -DKBUILD_MODNAME='"syslog_test"' -c
> -o test/log/syslog_test.o ../test/log/syslog_test.c

A quick poke around and I don't see us doing anything special for CFLAGS
there, so that's how kbuild as of our last sync with the Linux kernel
works is I suppose the answer.

-- 
Tom


signature.asc
Description: PGP signature


Re: i.MX RT1050 toolchain

2020-10-17 Thread Andy Pont

Giulio wrote...

Is there a preferred / recommended toolchain for building U-Boot for the
i.MX RT10xx platforms or should any recent ARM cross compiler work?


It should compile fine with any recent ARM cross-compiler.

Please let me know if it builds correctly
I used the arm-linux-gnueabi-gcc v10.1.0 compiler available from 
www.kernel.org and it has built fine. Now I have a working toolchain I 
can start adding the new board.


-Andy.




Re: [BUG] sandbox_defconfig fails to build with origin/master dadc1e38306b1c3

2020-10-17 Thread Heinrich Schuchardt
On 10/17/20 2:15 PM, Tom Rini wrote:
> On Sat, Oct 17, 2020 at 01:43:25PM +0200, Heinrich Schuchardt wrote:
>> Hello Tom, hello Simon,
>>
>> make sandbox_defconfig
>> make V=1
>>
>> yields:
>>
>> make -f ./scripts/Makefile.build obj=test/log
>>   gcc -Wp,-MD,test/log/.syslog_test.o.d -nostdinc -isystem
>> /usr/lib/gcc/x86_64-linux-gnu/10/include -Iinclude
>> -I./arch/sandbox/include -include ./include/linux/kconfig.h -D__KERNEL__
>> -D__UBOOT__ -Wall -Wstrict-prototypes -Wno-format-security -fno-builtin
>> -ffreestanding -std=gnu11 -fshort-wchar -fno-strict-aliasing -fno-PIE
>> -Os -fno-stack-protector -fno-delete-null-pointer-checks
>> -Wno-stringop-truncation -Wno-maybe-uninitialized -fmacro-prefix-map=./=
>> -g -fstack-usage -Wno-format-nonliteral -Wno-address-of-packed-member
>> -Wno-unused-but-set-variable -Werror=date-time -Werror -D__SANDBOX__
>> -U_FORTIFY_SOURCE -DCONFIG_ARCH_MAP_SYSMEM -fPIC -I/usr/include/SDL2
>> -D_REENTRANT -pipe-DKBUILD_BASENAME='"syslog_test"'
>> -DKBUILD_MODNAME='"syslog_test"' -c -o test/log/syslog_test.o
>> test/log/syslog_test.c
>> test/log/syslog_test.c:21:10: fatal error: syslog_test.h: No such file
>> or directory
>>21 | #include 
>>   |  ^~~
>> compilation terminated.
>>
>> git bisect:
>>
>> 52d3df7fefe30b05677db9055e68c666a071d89a is the first bad commit
>> commit 52d3df7fefe30b05677db9055e68c666a071d89a
>> Author: Simon Glass 
>> Date:   Sat Sep 12 11:13:34 2020 -0600
>>
>> log: Allow LOG_DEBUG to always enable log output
>>
>> @Tom:
>> Why are such build failures not detected before merging?
>
> I think because we have no tests for building in the source directory.

You are right

   make -j8 O=build

works while

make -j8

fails.

Why do we generate

-I../test/log -Itest/log

as gcc parameters when building out of source? If we avoid this, we will
get consistent test results.

  gcc -Wp,-MD,test/log/.syslog_test.o.d -nostdinc -isystem
/usr/lib/gcc/x86_64-linux-gnu/10/include -Iinclude  -I../include
-I../arch/sandbox/include -include ../include/linux/kconfig.h
-I../test/log -Itest/log -D__KERNEL__ -D__UBOOT__ -Wall
-Wstrict-prototypes -Wno-format-security -fno-builtin -ffreestanding
-std=gnu11 -fshort-wchar -fno-strict-aliasing -fno-PIE -Os
-fno-stack-protector -fno-delete-null-pointer-checks
-Wno-stringop-truncation -Wno-maybe-uninitialized
-fmacro-prefix-map=../= -g -fstack-usage -Wno-format-nonliteral
-Wno-address-of-packed-member -Wno-unused-but-set-variable
-Werror=date-time -Werror -D__SANDBOX__ -U_FORTIFY_SOURCE
-DCONFIG_ARCH_MAP_SYSMEM -fPIC  -I/usr/include/SDL2 -D_REENTRANT -pipe
  -DKBUILD_BASENAME='"syslog_test"'  -DKBUILD_MODNAME='"syslog_test"' -c
-o test/log/syslog_test.o ../test/log/syslog_test.c

> I'll pick up the fix for this today.

Thanks.

Best regards

Heinrich


[PATCH] mmc: Add some helper functions for retrying on error

2020-10-17 Thread Sean Anderson
All of the existing quirks add retries to various calls of mmc_send_cmd.
mmc_send_cmd_quirks is a helper function to do this retrying behavior. It
checks if quirks mode is enabled, and if a specific quirk is activated it
retries on error.

This also adds mmc_send_cmd_retry, which retries on error every time
(instead of if a quirk is activated).

Signed-off-by: Sean Anderson 
---

 drivers/mmc/mmc.c | 141 +++---
 1 file changed, 58 insertions(+), 83 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index d79cdef62e..83c3c23757 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -207,26 +207,65 @@ int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, 
struct mmc_data *data)
 }
 #endif
 
+/**
+ * mmc_send_cmd_retry() - send a command to the mmc device, retrying on error
+ *
+ * @dev:   device to receive the command
+ * @cmd:   command to send
+ * @data:  additional data to send/receive
+ * @retries:   how many times to retry; mmc_send_cmd is always called at least
+ *  once
+ * @return 0 if ok, -ve on error
+ */
+static int mmc_send_cmd_retry(struct mmc *mmc, struct mmc_cmd *cmd,
+ struct mmc_data *data, uint retries)
+{
+   int ret;
+
+   do {
+   ret = mmc_send_cmd(mmc, cmd, data);
+   } while (ret && retries--);
+
+   return ret;
+}
+
+/**
+ * mmc_send_cmd_quirks() - send a command to the mmc device, retrying if a
+ * specific quirk is enabled
+ *
+ * @dev:   device to receive the command
+ * @cmd:   command to send
+ * @data:  additional data to send/receive
+ * @quirk: retry only if this quirk is enabled
+ * @retries:   how many times to retry; mmc_send_cmd is always called at least
+ *  once
+ * @return 0 if ok, -ve on error
+ */
+static int mmc_send_cmd_quirks(struct mmc *mmc, struct mmc_cmd *cmd,
+  struct mmc_data *data, u32 quirk, uint retries)
+{
+   if (CONFIG_IS_ENABLED(MMC_QUIRKS) && mmc->quirks & quirk)
+   return mmc_send_cmd_retry(mmc, cmd, data, retries);
+   else
+   return mmc_send_cmd(mmc, cmd, data);
+}
+
 int mmc_send_status(struct mmc *mmc, unsigned int *status)
 {
struct mmc_cmd cmd;
-   int err, retries = 5;
+   int ret;
 
cmd.cmdidx = MMC_CMD_SEND_STATUS;
cmd.resp_type = MMC_RSP_R1;
if (!mmc_host_is_spi(mmc))
cmd.cmdarg = mmc->rca << 16;
 
-   while (retries--) {
-   err = mmc_send_cmd(mmc, , NULL);
-   if (!err) {
-   mmc_trace_state(mmc, );
-   *status = cmd.response[0];
-   return 0;
-   }
-   }
+   ret = mmc_send_cmd_retry(mmc, , NULL, 4);
mmc_trace_state(mmc, );
-   return -ECOMM;
+   if (!ret)
+   *status = cmd.response[0];
+
+   return ret;
 }
 
 int mmc_poll_for_busy(struct mmc *mmc, int timeout_ms)
@@ -274,7 +313,6 @@ int mmc_poll_for_busy(struct mmc *mmc, int timeout_ms)
 int mmc_set_blocklen(struct mmc *mmc, int len)
 {
struct mmc_cmd cmd;
-   int err;
 
if (mmc->ddr_mode)
return 0;
@@ -283,24 +321,8 @@ int mmc_set_blocklen(struct mmc *mmc, int len)
cmd.resp_type = MMC_RSP_R1;
cmd.cmdarg = len;
 
-   err = mmc_send_cmd(mmc, , NULL);
-
-#ifdef CONFIG_MMC_QUIRKS
-   if (err && (mmc->quirks & MMC_QUIRK_RETRY_SET_BLOCKLEN)) {
-   int retries = 4;
-   /*
-* It has been seen that SET_BLOCKLEN may fail on the first
-* attempt, let's try a few more time
-*/
-   do {
-   err = mmc_send_cmd(mmc, , NULL);
-   if (!err)
-   break;
-   } while (retries--);
-   }
-#endif
-
-   return err;
+   return mmc_send_cmd_quirks(mmc, , NULL,
+  MMC_QUIRK_RETRY_SET_BLOCKLEN, 4);
 }
 
 #ifdef MMC_SUPPORTS_TUNING
@@ -771,7 +793,6 @@ static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, 
u8 value,
int timeout_ms = DEFAULT_CMD6_TIMEOUT_MS;
bool is_part_switch = (set == EXT_CSD_CMD_SET_NORMAL) &&
  (index == EXT_CSD_PART_CONF);
-   int retries = 3;
int ret;
 
if (mmc->gen_cmd6_time)
@@ -786,10 +807,7 @@ static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, 
u8 value,
 (index << 16) |
 (value << 8);
 
-   do {
-   ret = mmc_send_cmd(mmc, , NULL);
-   } while (ret && retries-- > 0);
-
+   ret = mmc_send_cmd_retry(mmc, , NULL, 3);
if (ret)
return ret;
 
@@ -1285,22 +1303,15 @@ static int sd_get_capabilities(struct mmc *mmc)
cmd.resp_type = MMC_RSP_R1;
cmd.cmdarg = 0;
 
-   timeout = 3;
-

[PATCH v2 3/3] test: log: test message continuation

2020-10-17 Thread Heinrich Schuchardt
Provide a unit test checking that a continuation message will use the same
log level and log category as the previous message.

Signed-off-by: Heinrich Schuchardt 
Reviewed-by: Simon Glass 
---
v2:
rebased
---
 test/log/Makefile|  4 +++-
 test/log/cont_test.c | 52 
 2 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100644 test/log/cont_test.c

diff --git a/test/log/Makefile b/test/log/Makefile
index 52e2f7b41c..fdf529582d 100644
--- a/test/log/Makefile
+++ b/test/log/Makefile
@@ -13,7 +13,9 @@ obj-$(CONFIG_LOG_SYSLOG) += syslog_test.o
 obj-$(CONFIG_LOG_SYSLOG) += syslog_test_ndebug.o
 endif

-ifndef CONFIG_LOG
+ifdef CONFIG_LOG
+obj-$(CONFIG_CONSOLE_RECORD) += cont_test.o
+else
 obj-$(CONFIG_CONSOLE_RECORD) += nolog_test.o
 endif

diff --git a/test/log/cont_test.c b/test/log/cont_test.c
new file mode 100644
index 00..68ca1d262c
--- /dev/null
+++ b/test/log/cont_test.c
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020, Heinrich Schuchardt 
+ *
+ * Test continuation of log messages.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#define BUFFSIZE 64
+
+static int log_test_cont(struct unit_test_state *uts)
+{
+   int log_fmt;
+   int log_level;
+
+   log_fmt = gd->log_fmt;
+   log_level = gd->default_log_level;
+
+   /* Write two messages, the second continuing the first */
+   gd->log_fmt = (1 << LOGF_CAT) | (1 << LOGF_LEVEL) | (1 << LOGF_MSG);
+   gd->default_log_level = LOGL_INFO;
+   console_record_reset_enable();
+   log(LOGC_ARCH, LOGL_ERR, "ea%d ", 1);
+   log(LOGC_CONT, LOGL_CONT, "cc%d\n", 2);
+   gd->default_log_level = log_level;
+   gd->log_fmt = log_fmt;
+   gd->flags &= ~GD_FLG_RECORD;
+   ut_assertok(ut_check_console_line(uts, "ERR.arch, ea1 ERR.arch, cc2"));
+   ut_assertok(ut_check_console_end(uts));
+
+   /* Write a third message which is not a continuation */
+   gd->log_fmt = (1 << LOGF_CAT) | (1 << LOGF_LEVEL) | (1 << LOGF_MSG);
+   gd->default_log_level = LOGL_INFO;
+   console_record_reset_enable();
+   log(LOGC_EFI, LOGL_INFO, "ie%d\n", 3);
+   gd->default_log_level = log_level;
+   gd->log_fmt = log_fmt;
+   gd->flags &= ~GD_FLG_RECORD;
+   ut_assertok(ut_check_console_line(uts, "INFO.efi, ie3"));
+   ut_assertok(ut_check_console_end(uts));
+
+   return 0;
+}
+LOG_TEST(log_test_cont);
--
2.28.0



[PATCH v2 0/3] log: allow for message continuation

2020-10-17 Thread Heinrich Schuchardt
The first patch move a static variable to the global data.

With the second patch it becomes possible to continue a log message with
the same log level and category as the previous messages.

We need this facility to convert pr_cont() to use our logging drivers.

The third patch provides a unit test.

v2:
move static variables to global data

Heinrich Schuchardt (3):
  log: move processing_msg to global data
  log: allow for message continuation
  test: log: test message continuation

 common/log.c  | 30 --
 doc/develop/logging.rst   |  6 
 include/asm-generic/global_data.h | 20 
 include/log.h |  2 ++
 test/log/Makefile |  4 ++-
 test/log/cont_test.c  | 52 +++
 6 files changed, 104 insertions(+), 10 deletions(-)
 create mode 100644 test/log/cont_test.c

--
2.28.0



[PATCH v2 2/3] log: allow for message continuation

2020-10-17 Thread Heinrich Schuchardt
Some drivers use macro pr_cont() for continuing a message sent via printk.
Hence if we want to convert printk messaging to using the logging system,
we must support continuation of log messages too.

As pr_cont() does not provide a message level we need a means of
remembering the last log level.

With the patch a pseudo log level LOGL_CONT as well as a pseudo log
category LOGC_CONT are introduced. Using these results in the application
of the same log level and category as in the previous log message.

Signed-off-by: Heinrich Schuchardt 
---
v2:
replace static variables by global data
---
 common/log.c  | 23 ++-
 doc/develop/logging.rst   |  6 ++
 include/asm-generic/global_data.h | 12 
 include/log.h |  2 ++
 4 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/common/log.c b/common/log.c
index f95f09c1b6..58be9f7440 100644
--- a/common/log.c
+++ b/common/log.c
@@ -186,10 +186,12 @@ static bool log_passes_filters(struct log_device *ldev, 
struct log_rec *rec)
  * log_dispatch() - Send a log record to all log devices for processing
  *
  * The log record is sent to each log device in turn, skipping those which have
- * filters which block the record
+ * filters which block the record.
  *
- * @rec: Log record to dispatch
- * @return 0 (meaning success)
+ * All log messages created while processing log record @rec are ignored.
+ *
+ * @rec:   log record to dispatch
+ * Return: 0 msg sent, 1 msg not sent while already dispatching another msg
  */
 static int log_dispatch(struct log_rec *rec)
 {
@@ -201,7 +203,7 @@ static int log_dispatch(struct log_rec *rec)
 * as this might result in infinite recursion.
 */
if (gd->processing_msg)
-   return 0;
+   return 1;

/* Emit message */
gd->processing_msg = true;
@@ -221,6 +223,12 @@ int _log(enum log_category_t cat, enum log_level_t level, 
const char *file,
struct log_rec rec;
va_list args;

+   /* Check for message continuation */
+   if (cat == LOGC_CONT)
+   cat = gd->logc_prev;
+   if (level == LOGL_CONT)
+   level = gd->logl_prev;
+
rec.cat = cat;
rec.level = level & LOGL_LEVEL_MASK;
rec.force_debug = level & LOGL_FORCE_DEBUG;
@@ -236,7 +244,10 @@ int _log(enum log_category_t cat, enum log_level_t level, 
const char *file,
gd->log_drop_count++;
return -ENOSYS;
}
-   log_dispatch();
+   if (!log_dispatch()) {
+   gd->logc_prev = cat;
+   gd->logl_prev = level;
+   }

return 0;
 }
@@ -376,6 +387,8 @@ int log_init(void)
if (!gd->default_log_level)
gd->default_log_level = CONFIG_LOG_DEFAULT_LEVEL;
gd->log_fmt = log_get_default_format();
+   gd->logc_prev = LOGC_NONE;
+   gd->logl_prev = LOGL_INFO;

return 0;
 }
diff --git a/doc/develop/logging.rst b/doc/develop/logging.rst
index 7ce8482ab6..c36f6bbbe4 100644
--- a/doc/develop/logging.rst
+++ b/doc/develop/logging.rst
@@ -38,6 +38,9 @@ There are a number logging levels available, in increasing 
order of verbosity:
 * LOGL_DEBUG_CONTENT - Debug message showing full message content
 * LOGL_DEBUG_IO - Debug message showing hardware I/O access

+To continue a log message in a separate call of function log() use
+
+* LOGL_CONT - Use same log level as in previous call

 Logging category
 
@@ -56,6 +59,9 @@ The following main categories are defined:
 * LOGC_DT - Related to device tree control
 * LOGC_EFI - Related to EFI implementation

+To continue a log message in a separate call of function log() use
+
+* LOGC_CONT - Use same category as in previous call

 Enabling logging
 
diff --git a/include/asm-generic/global_data.h 
b/include/asm-generic/global_data.h
index db83f59ceb..0157af1aa4 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -371,6 +371,18 @@ struct global_data {
 * while another message is being processed.
 */
bool processing_msg;
+   /**
+* @logc_prev: logging category of previous message
+*
+* This value is used as logging category for continuation messages.
+*/
+   int logc_prev;
+   /**
+* @logl_pref: logging level of the previous message
+*
+* This value is used as logging level for continuation messages.
+*/
+   int logl_prev;
 #endif
 #if CONFIG_IS_ENABLED(BLOBLIST)
/**
diff --git a/include/log.h b/include/log.h
index 4acc087b2e..73cfb6153c 100644
--- a/include/log.h
+++ b/include/log.h
@@ -38,6 +38,7 @@ enum log_level_t {

LOGL_FIRST = LOGL_EMERG,
LOGL_MAX = LOGL_DEBUG_IO,
+   LOGL_CONT = -1, /* Use same log level as in previous call */
 };

 /**
@@ -63,6 +64,7 @@ enum log_category_t {

LOGC_COUNT,  

[PATCH v2 1/3] log: move processing_msg to global data

2020-10-17 Thread Heinrich Schuchardt
Replace the static variable processing_msg by a field in the global data.
Make the field bool at it can only be true or false.

Signed-off-by: Heinrich Schuchardt 
---
v2:
new patch
---
 common/log.c  | 7 +++
 include/asm-generic/global_data.h | 8 
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/common/log.c b/common/log.c
index 1b10f6f180..f95f09c1b6 100644
--- a/common/log.c
+++ b/common/log.c
@@ -194,24 +194,23 @@ static bool log_passes_filters(struct log_device *ldev, 
struct log_rec *rec)
 static int log_dispatch(struct log_rec *rec)
 {
struct log_device *ldev;
-   static int processing_msg;

/*
 * When a log driver writes messages (e.g. via the network stack) this
 * may result in further generated messages. We cannot process them here
 * as this might result in infinite recursion.
 */
-   if (processing_msg)
+   if (gd->processing_msg)
return 0;

/* Emit message */
-   processing_msg = 1;
+   gd->processing_msg = true;
list_for_each_entry(ldev, >log_head, sibling_node) {
if ((ldev->flags & LOGDF_ENABLE) &&
log_passes_filters(ldev, rec))
ldev->drv->emit(ldev, rec);
}
-   processing_msg = 0;
+   gd->processing_msg = false;
return 0;
 }

diff --git a/include/asm-generic/global_data.h 
b/include/asm-generic/global_data.h
index ebb740d34f..db83f59ceb 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -363,6 +363,14 @@ struct global_data {
 *  log_fmt defines the bits of the bit mask.
 */
int log_fmt;
+
+   /**
+* @processing_msg: a log message is being processed
+*
+* This flag is used to suppress the creation of additional messages
+* while another message is being processed.
+*/
+   bool processing_msg;
 #endif
 #if CONFIG_IS_ENABLED(BLOBLIST)
/**
--
2.28.0



Re: [BUG] sandbox_defconfig fails to build with origin/master dadc1e38306b1c3

2020-10-17 Thread Tom Rini
On Sat, Oct 17, 2020 at 01:43:25PM +0200, Heinrich Schuchardt wrote:
> Hello Tom, hello Simon,
> 
> make sandbox_defconfig
> make V=1
> 
> yields:
> 
> make -f ./scripts/Makefile.build obj=test/log
>   gcc -Wp,-MD,test/log/.syslog_test.o.d -nostdinc -isystem
> /usr/lib/gcc/x86_64-linux-gnu/10/include -Iinclude
> -I./arch/sandbox/include -include ./include/linux/kconfig.h -D__KERNEL__
> -D__UBOOT__ -Wall -Wstrict-prototypes -Wno-format-security -fno-builtin
> -ffreestanding -std=gnu11 -fshort-wchar -fno-strict-aliasing -fno-PIE
> -Os -fno-stack-protector -fno-delete-null-pointer-checks
> -Wno-stringop-truncation -Wno-maybe-uninitialized -fmacro-prefix-map=./=
> -g -fstack-usage -Wno-format-nonliteral -Wno-address-of-packed-member
> -Wno-unused-but-set-variable -Werror=date-time -Werror -D__SANDBOX__
> -U_FORTIFY_SOURCE -DCONFIG_ARCH_MAP_SYSMEM -fPIC -I/usr/include/SDL2
> -D_REENTRANT -pipe-DKBUILD_BASENAME='"syslog_test"'
> -DKBUILD_MODNAME='"syslog_test"' -c -o test/log/syslog_test.o
> test/log/syslog_test.c
> test/log/syslog_test.c:21:10: fatal error: syslog_test.h: No such file
> or directory
>21 | #include 
>   |  ^~~
> compilation terminated.
> 
> git bisect:
> 
> 52d3df7fefe30b05677db9055e68c666a071d89a is the first bad commit
> commit 52d3df7fefe30b05677db9055e68c666a071d89a
> Author: Simon Glass 
> Date:   Sat Sep 12 11:13:34 2020 -0600
> 
> log: Allow LOG_DEBUG to always enable log output
> 
> @Tom:
> Why are such build failures not detected before merging?

I think because we have no tests for building in the source directory.
I'll pick up the fix for this today.

-- 
Tom


signature.asc
Description: PGP signature


Re: [BUG] sandbox_defconfig fails to build with origin/master dadc1e38306b1c3

2020-10-17 Thread Heinrich Schuchardt
On 10/17/20 1:43 PM, Heinrich Schuchardt wrote:
> Hello Tom, hello Simon,
>
> make sandbox_defconfig
> make V=1
>
> yields:
>
> make -f ./scripts/Makefile.build obj=test/log
>   gcc -Wp,-MD,test/log/.syslog_test.o.d -nostdinc -isystem
> /usr/lib/gcc/x86_64-linux-gnu/10/include -Iinclude
> -I./arch/sandbox/include -include ./include/linux/kconfig.h -D__KERNEL__
> -D__UBOOT__ -Wall -Wstrict-prototypes -Wno-format-security -fno-builtin
> -ffreestanding -std=gnu11 -fshort-wchar -fno-strict-aliasing -fno-PIE
> -Os -fno-stack-protector -fno-delete-null-pointer-checks
> -Wno-stringop-truncation -Wno-maybe-uninitialized -fmacro-prefix-map=./=
> -g -fstack-usage -Wno-format-nonliteral -Wno-address-of-packed-member
> -Wno-unused-but-set-variable -Werror=date-time -Werror -D__SANDBOX__
> -U_FORTIFY_SOURCE -DCONFIG_ARCH_MAP_SYSMEM -fPIC -I/usr/include/SDL2
> -D_REENTRANT -pipe-DKBUILD_BASENAME='"syslog_test"'
> -DKBUILD_MODNAME='"syslog_test"' -c -o test/log/syslog_test.o
> test/log/syslog_test.c
> test/log/syslog_test.c:21:10: fatal error: syslog_test.h: No such file
> or directory
>21 | #include 
>   |  ^~~
> compilation terminated.
>
> git bisect:
>
> 52d3df7fefe30b05677db9055e68c666a071d89a is the first bad commit
> commit 52d3df7fefe30b05677db9055e68c666a071d89a
> Author: Simon Glass 
> Date:   Sat Sep 12 11:13:34 2020 -0600
>
> log: Allow LOG_DEBUG to always enable log output
>
> @Tom:
> Why are such build failures not detected before merging?
>
> Best regards
>
> Heinrich
>

Cf.
[PATCH 1/1] test: fix build failure for syslog_test
https://lists.denx.de/pipermail/u-boot/2020-October/429766.html



[PATCH 1/1] test: fix build failure for syslog_test

2020-10-17 Thread Heinrich Schuchardt
Avoid failures like:

test/log/syslog_test_ndebug.c:18:10:
fatal error: syslog_test.h: No such file or directory
   18 | #include 
  |  ^~~

syslog_test.h is a local include. Use "" and not <>.

Signed-off-by: Heinrich Schuchardt 
---
 test/log/syslog_test.c| 2 +-
 test/log/syslog_test_ndebug.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/log/syslog_test.c b/test/log/syslog_test.c
index febaca68e8..a058d8f569 100644
--- a/test/log/syslog_test.c
+++ b/test/log/syslog_test.c
@@ -18,7 +18,7 @@
 #include 
 #include 
 #include 
-#include 
+#include "syslog_test.h"

 DECLARE_GLOBAL_DATA_PTR;

diff --git a/test/log/syslog_test_ndebug.c b/test/log/syslog_test_ndebug.c
index c7f5a60861..84844a3944 100644
--- a/test/log/syslog_test_ndebug.c
+++ b/test/log/syslog_test_ndebug.c
@@ -15,7 +15,7 @@
 #include 
 #include 
 #include 
-#include 
+#include "syslog_test.h"

 DECLARE_GLOBAL_DATA_PTR;

--
2.28.0



[BUG] sandbox_defconfig fails to build with origin/master dadc1e38306b1c3

2020-10-17 Thread Heinrich Schuchardt
Hello Tom, hello Simon,

make sandbox_defconfig
make V=1

yields:

make -f ./scripts/Makefile.build obj=test/log
  gcc -Wp,-MD,test/log/.syslog_test.o.d -nostdinc -isystem
/usr/lib/gcc/x86_64-linux-gnu/10/include -Iinclude
-I./arch/sandbox/include -include ./include/linux/kconfig.h -D__KERNEL__
-D__UBOOT__ -Wall -Wstrict-prototypes -Wno-format-security -fno-builtin
-ffreestanding -std=gnu11 -fshort-wchar -fno-strict-aliasing -fno-PIE
-Os -fno-stack-protector -fno-delete-null-pointer-checks
-Wno-stringop-truncation -Wno-maybe-uninitialized -fmacro-prefix-map=./=
-g -fstack-usage -Wno-format-nonliteral -Wno-address-of-packed-member
-Wno-unused-but-set-variable -Werror=date-time -Werror -D__SANDBOX__
-U_FORTIFY_SOURCE -DCONFIG_ARCH_MAP_SYSMEM -fPIC -I/usr/include/SDL2
-D_REENTRANT -pipe-DKBUILD_BASENAME='"syslog_test"'
-DKBUILD_MODNAME='"syslog_test"' -c -o test/log/syslog_test.o
test/log/syslog_test.c
test/log/syslog_test.c:21:10: fatal error: syslog_test.h: No such file
or directory
   21 | #include 
  |  ^~~
compilation terminated.

git bisect:

52d3df7fefe30b05677db9055e68c666a071d89a is the first bad commit
commit 52d3df7fefe30b05677db9055e68c666a071d89a
Author: Simon Glass 
Date:   Sat Sep 12 11:13:34 2020 -0600

log: Allow LOG_DEBUG to always enable log output

@Tom:
Why are such build failures not detected before merging?

Best regards

Heinrich


Re: am335x uart boot

2020-10-17 Thread Matwey V. Kornilov
12.10.2020 11:39, Matwey V. Kornilov пишет:
> Hello,
> 
> I am trying to provide u-boot via uart to am335x bootrom (I use
> BeagleBone Black board). Has anyone succeeded on it?
> 
> The CPU technical reference says that I have to use X-Modem 1K to load
> the firmware. I am trying to reach this with screen and sx command.
> 
> In the serial console I see transmission invitation from the hardware
> side: 'C'.
> When I use sx --1k -X to transmit the binary there are no data accepted.
> When I use sx -X (with 128 byte frame) the transmission is running until
> hardware sends "Cancel" command in the middle.
> 
> I've also tried minicom terminal. It behaves like X-Modem 128 byte
> transmission with no success.
> 
> 

Now I believe this was due to transmission errors. I've taken another
RS232 converter and now everything boots both with sx -X and sx --1k -X



[PATCH] clk: mediatek: Add MT8183 clock driver

2020-10-17 Thread Fabien Parent
Add the topckgen, apmixedsys and infracfg clock driver for the MT8183
SoC.

Signed-off-by: Fabien Parent 
---
 drivers/clk/mediatek/Makefile  |   1 +
 drivers/clk/mediatek/clk-mt8183.c  | 823 +
 include/dt-bindings/clock/mt8183-clk.h | 329 ++
 3 files changed, 1153 insertions(+)
 create mode 100644 drivers/clk/mediatek/clk-mt8183.c
 create mode 100644 include/dt-bindings/clock/mt8183-clk.h

diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile
index 237fd17f1670..522e72422144 100644
--- a/drivers/clk/mediatek/Makefile
+++ b/drivers/clk/mediatek/Makefile
@@ -7,5 +7,6 @@ obj-$(CONFIG_MT8512) += clk-mt8512.o
 obj-$(CONFIG_TARGET_MT7623) += clk-mt7623.o
 obj-$(CONFIG_TARGET_MT7622) += clk-mt7622.o
 obj-$(CONFIG_TARGET_MT7629) += clk-mt7629.o
+obj-$(CONFIG_TARGET_MT8183) += clk-mt8183.o
 obj-$(CONFIG_TARGET_MT8516) += clk-mt8516.o
 obj-$(CONFIG_TARGET_MT8518) += clk-mt8518.o
diff --git a/drivers/clk/mediatek/clk-mt8183.c 
b/drivers/clk/mediatek/clk-mt8183.c
new file mode 100644
index ..f25f5ee740c9
--- /dev/null
+++ b/drivers/clk/mediatek/clk-mt8183.c
@@ -0,0 +1,823 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * MediaTek clock driver for MT8183 SoC
+ *
+ * Copyright (C) 2020 BayLibre, SAS
+ * Copyright (c) 2020 MediaTek Inc.
+ * Author: Fabien Parent 
+ * Author: Weiyi Lu 
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include "clk-mtk.h"
+
+#define MT8183_PLL_FMAX(3800UL * MHZ)
+#define MT8183_PLL_FMIN(1500UL * MHZ)
+
+/* apmixedsys */
+#define PLL(_id, _reg, _pwr_reg, _en_mask, _flags, _rst_bar_mask, _pcwbits, \
+   _pcwibits, _pd_reg, _pd_shift, _pcw_reg, _pcw_shift) {  \
+   .id = _id,  \
+   .reg = _reg,\
+   .pwr_reg = _pwr_reg,\
+   .en_mask = _en_mask,\
+   .rst_bar_mask = _rst_bar_mask,  \
+   .fmax = MT8183_PLL_FMAX,\
+   .fmin = MT8183_PLL_FMIN,\
+   .flags = _flags,\
+   .pcwbits = _pcwbits,\
+   .pcwibits = _pcwibits,  \
+   .pd_reg = _pd_reg,  \
+   .pd_shift = _pd_shift,  \
+   .pcw_reg = _pcw_reg,\
+   .pcw_shift = _pcw_shift,\
+   }
+
+static const struct mtk_pll_data apmixed_plls[] = {
+   PLL(CLK_APMIXED_ARMPLL_LL, 0x0200, 0x020C, 0x0001,
+   HAVE_RST_BAR, BIT(24), 22, 8, 0x0204, 24,
+   0x0204, 0),
+   PLL(CLK_APMIXED_ARMPLL_L, 0x0210, 0x021C, 0x0001,
+   HAVE_RST_BAR, BIT(24), 22, 8, 0x0214, 24,
+   0x0214, 0),
+   PLL(CLK_APMIXED_CCIPLL, 0x0290, 0x029C, 0x0001,
+   HAVE_RST_BAR, BIT(24), 22, 8, 0x0294, 24,
+   0x0294, 0),
+   PLL(CLK_APMIXED_MAINPLL, 0x0220, 0x022C, 0x0001,
+   HAVE_RST_BAR, BIT(24), 22, 8, 0x0224, 24,
+   0x0224, 0),
+   PLL(CLK_APMIXED_UNIV2PLL, 0x0230, 0x023C, 0x0001,
+   HAVE_RST_BAR, BIT(24), 22, 8, 0x0234, 24,
+   0x0234, 0),
+   PLL(CLK_APMIXED_MSDCPLL, 0x0250, 0x025C, 0x0001,
+   0, 0, 22, 8, 0x0254, 24, 0x0254, 0),
+   PLL(CLK_APMIXED_MMPLL, 0x0270, 0x027C, 0x0001,
+   HAVE_RST_BAR, BIT(23), 22, 8, 0x0274, 24,
+   0x0274, 0),
+   PLL(CLK_APMIXED_MFGPLL, 0x0240, 0x024C, 0x0001,
+   0, 0, 22, 8, 0x0244, 24, 0x0244, 0),
+   PLL(CLK_APMIXED_TVDPLL, 0x0260, 0x026C, 0x0001,
+   0, 0, 22, 8, 0x0264, 24, 0x0264, 0),
+   PLL(CLK_APMIXED_APLL1, 0x02A0, 0x02B0, 0x0001,
+   0, 0, 32, 8, 0x02A0, 1, 0x02A4, 0),
+   PLL(CLK_APMIXED_APLL2, 0x02b4, 0x02c4, 0x0001,
+   0, 0, 32, 8, 0x02B4, 1, 0x02B8, 0),
+};
+
+static const struct mtk_fixed_clk top_fixed_clks[] = {
+   FIXED_CLK(CLK_TOP_CLK26M, CLK_XTAL, 2600),
+   FIXED_CLK(CLK_TOP_ULPOSC, CLK_XTAL, 25),
+   FIXED_CLK(CLK_TOP_UNIVP_192M, CLK_TOP_UNIVPLL, 19200),
+};
+
+static const struct mtk_fixed_factor top_fixed_divs[] = {
+   FACTOR(CLK_TOP_CLK13M, CLK_TOP_CLK26M, 1, 2, CLK_PARENT_TOPCKGEN),
+   FACTOR(CLK_TOP_F26M_CK_D2, CLK_TOP_CLK26M, 1, 2, CLK_PARENT_TOPCKGEN),
+   FACTOR(CLK_TOP_SYSPLL_CK, CLK_APMIXED_MAINPLL, 1,
+  1, CLK_PARENT_APMIXED),
+   FACTOR(CLK_TOP_SYSPLL_D2, CLK_TOP_SYSPLL_CK, 1,
+  2, CLK_PARENT_TOPCKGEN),
+   FACTOR(CLK_TOP_SYSPLL_D3, CLK_APMIXED_MAINPLL, 1,
+  3, CLK_PARENT_APMIXED),
+   FACTOR(CLK_TOP_SYSPLL_D5, 

Re: [PATCH v1 8/8] doc: board: Add Microchip MPFS Icicle Kit doc

2020-10-17 Thread Anup Patel
On Fri, Oct 16, 2020 at 7:55 PM  wrote:
>
> From: Padmarao Begari 
>
> This doc describes the procedure to build, flash and
> boot Linux using U-boot on Microchip MPFS Icicle Kit.
>
> Signed-off-by: Padmarao Begari 
> ---
>  doc/board/index.rst |   1 +
>  doc/board/microchip/index.rst   |   9 +
>  doc/board/microchip/mpfs_icicle.rst | 605 
>  3 files changed, 615 insertions(+)
>  create mode 100644 doc/board/microchip/index.rst
>  create mode 100644 doc/board/microchip/mpfs_icicle.rst
>
> diff --git a/doc/board/index.rst b/doc/board/index.rst
> index 63935abcd7..e50a78d752 100644
> --- a/doc/board/index.rst
> +++ b/doc/board/index.rst
> @@ -15,6 +15,7 @@ Board-specific doc
> freescale/index
> google/index
> intel/index
> +   microchip/index
> renesas/index
> rockchip/index
> sifive/index
> diff --git a/doc/board/microchip/index.rst b/doc/board/microchip/index.rst
> new file mode 100644
> index 00..b09e6788af
> --- /dev/null
> +++ b/doc/board/microchip/index.rst
> @@ -0,0 +1,9 @@
> +.. SPDX-License-Identifier: GPL-2.0+
> +
> +Microchip
> +==
> +
> +.. toctree::
> +   :maxdepth: 2
> +
> +   mpfs_icicle
> diff --git a/doc/board/microchip/mpfs_icicle.rst 
> b/doc/board/microchip/mpfs_icicle.rst
> new file mode 100644
> index 00..d1e6bd2077
> --- /dev/null
> +++ b/doc/board/microchip/mpfs_icicle.rst
> @@ -0,0 +1,605 @@
> +.. SPDX-License-Identifier: GPL-2.0+
> +
> +Microchip PolarFire SoC Icicle Kit
> +==
> +
> +RISC-V PolarFire SoC
> +-
> +The PolarFire SoC is the 4+1 64-bit RISC-V SoC from Microchip.
> +
> +The Icicle Kit development platform is based on PolarFire SoC and capable
> +of running Linux.
> +
> +Mainline support
> +
> +The support for following drivers are already enabled:
> +
> +1. NS16550 UART Driver.
> +2. Microchip Clock Driver.
> +3. Cadence MACB ethernet driver for networking support.
> +4. Cadence MMC Driver for eMMC/SD support.
> +
> +Booting from eMMC using HSS
> +---
> +
> +Building
> +
> +
> +1. Add the RISC-V toolchain to your PATH.
> +2. Setup ARCH & cross compilation environment variable:
> +
> +.. code-block:: none
> +
> +   export CROSS_COMPILE=
> +
> +3. make microchip_mpfs_icicle_defconfig
> +4. make
> +
> +Flashing
> +
> +
> +The current U-Boot port is supported in S-mode only and loaded from DRAM.
> +
> +A prior stage M-mode firmware/bootloader (e.g HSS with OpenSBI) is required 
> to
> +boot the u-boot.bin in S-mode.
> +
> +Currently, the u-boot.bin is used as a payload of the HSS firmware.
> +
> +You will be creating a payload from `u-boot-dtb.bin`.
> +Copy this file to the toplevel HSS (Hart Software Services) directory.
> +
> +Creating the HSS payload
> +
> +
> +Please refer to HSS documenation to build the HSS firmware.
> +(Note: HSS git repo is at
> +https://github.com/polarfire-soc/hart-software-services/blob/master
> +/tools/hss-payload-generator/README.md)
> +
> +Once the payload binary is generated, it should be copied to the eMMC.
> +
> +FPGA design with HSS programming file
> +-
> +https://github.com/polarfire-soc/polarfire-soc-documentation/blob/master/boards
> +/mpfs-icicle-kit-es/updating-icicle-kit/updating-icicle-kit-design-and-linux.md
> +
> +The HSS firmware runs from the PolarFire SoC eNVM on reset.
> +
> +eMMC
> +
> +Program eMMC with payload binary is explained in the PolarFire SoC 
> documentation.
> +
> +(Note: PolarFire SoC Documentation git repo is at
> +https://github.com/polarfire-soc/polarfire-soc-documentation/blob/master/boards
> +/mpfs-icicle-kit-es/updating-icicle-kit/updating-icicle-kit-design-and-linux.md#eMMC
> +
> +Once the payload image is copied to the eMMC, press CTRL+C in the HSS command
> +line interface, then type 'boot' and enter to boot the newly copied image.
> +
> +.. code-block:: none
> +
> +sudo dd if= of=/dev/sdX bs=512
> +
> +Booting
> +---
> +you should see the U-Boot prompt on UART1.

Like I mentioned in another PATCH, all the HSS boot logs are on
UART0 and U-Boot appears on UART1. This is very inconvenient.

Please use UART0 for U-Boot and users can easily use UART1
for Linux (if they want) using bootargs passed to Linux kernel.

This will be consistent with other boards as well.

Regards,
Anup

> +
> +Sample boot log from MPFS Icicle Kit
> +---
> +
> +.. code-block:: none
> +
> +   U-Boot 2020.10-00544-g4f642dd804-dirty (Oct 16 2020 - 11:37:31 +0530)
> +
> +   CPU:   rv64imafdc
> +   Model: Microchip PolarFire-SoC
> +   DRAM:  1 GiB
> +   MMC:   sdhc@20008000: 0
> +   In:serial@2010
> +   Out:   serial@2010
> +   Err:   serial@2010
> +   Net:   eth0: ethernet@20112000
> +   Hit any key to stop autoboot:  0
> +
> +Now you can configure your networking, tftp server and use tftp boot method 
> to
> +load 

Re: [PATCH v1 3/8] dt-bindings: clock: Add indexes for reset signals

2020-10-17 Thread Anup Patel
On Fri, Oct 16, 2020 at 7:54 PM  wrote:
>
> From: Padmarao Begari 
>
> Add indexes for reset and clock control signals within the system register
> module of the Microchip PolarFire SoC.

This patch should be squashed into your PATCH7.

>
> Signed-off-by: Padmarao Begari 
> ---
>  .../dt-bindings/clock/microchip,pfsoc-clock.h | 45 +++
>  1 file changed, 45 insertions(+)
>  create mode 100644 include/dt-bindings/clock/microchip,pfsoc-clock.h
>
> diff --git a/include/dt-bindings/clock/microchip,pfsoc-clock.h 
> b/include/dt-bindings/clock/microchip,pfsoc-clock.h
> new file mode 100644
> index 00..527cff1a28
> --- /dev/null
> +++ b/include/dt-bindings/clock/microchip,pfsoc-clock.h
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2020 Microchip Technology Inc.
> + * Padmarao Begari 
> + */
> +
> +#ifndef _DT_BINDINGS_CLK_MICROCHIP_PFSOC_H_
> +#define _DT_BINDINGS_CLK_MICROCHIP_PFSOC_H_
> +
> +#define CLK_CPU0
> +#define CLK_AXI1
> +#define CLK_AHB2
> +
> +#define CLK_ENVM   3
> +#define CLK_MAC0   4
> +#define CLK_MAC1   5
> +#define CLK_MMC6
> +#define CLK_TIMER  7
> +#define CLK_MMUART08
> +#define CLK_MMUART19
> +#define CLK_MMUART210
> +#define CLK_MMUART311
> +#define CLK_MMUART412
> +#define CLK_SPI0   13
> +#define CLK_SPI1   14
> +#define CLK_I2C0   15
> +#define CLK_I2C1   16
> +#define CLK_CAN0   17
> +#define CLK_CAN1   18
> +#define CLK_USB19
> +#define CLK_RESERVED   20
> +#define CLK_RTC21
> +#define CLK_QSPI   22
> +#define CLK_GPIO0  23
> +#define CLK_GPIO1  24
> +#define CLK_GPIO2  25
> +#define CLK_DDRC   26
> +#define CLK_FIC0   27
> +#define CLK_FIC1   28
> +#define CLK_FIC2   29
> +#define CLK_FIC3   30
> +#define CLK_ATHENA 31
> +#define CLK_CFM32
> +
> +#endif /* _DT_BINDINGS_CLK_MICROCHIP_PFSOC_H_ */
> --
> 2.17.1
>

Regards,
Anup


Re: [PATCH v1 2/8] riscv: dts: Add device tree for Microchip Icicle Kit

2020-10-17 Thread Anup Patel
On Fri, Oct 16, 2020 at 7:54 PM  wrote:
>
> From: Padmarao Begari 
>
> Add device tree for Microchip PolarFire SoC Icicle Kit.
>
> Signed-off-by: Padmarao Begari 
> ---
>  arch/riscv/dts/Makefile  |   1 +
>  arch/riscv/dts/microchip-icicle-kit-a000.dts | 419 +++
>  2 files changed, 420 insertions(+)
>  create mode 100644 arch/riscv/dts/microchip-icicle-kit-a000.dts
>
> diff --git a/arch/riscv/dts/Makefile b/arch/riscv/dts/Makefile
> index 3a6f96c67d..48c43bd122 100644
> --- a/arch/riscv/dts/Makefile
> +++ b/arch/riscv/dts/Makefile
> @@ -3,6 +3,7 @@
>  dtb-$(CONFIG_TARGET_AX25_AE350) += ae350_32.dtb ae350_64.dtb
>  dtb-$(CONFIG_TARGET_SIFIVE_FU540) += hifive-unleashed-a00.dtb
>  dtb-$(CONFIG_TARGET_SIPEED_MAIX) += k210-maix-bit.dtb
> +dtb-$(CONFIG_TARGET_MICROCHIP_ICICLE) += microchip-icicle-kit-a000.dtb
>
>  targets += $(dtb-y)
>
> diff --git a/arch/riscv/dts/microchip-icicle-kit-a000.dts 
> b/arch/riscv/dts/microchip-icicle-kit-a000.dts
> new file mode 100644
> index 00..e7f0ec6926
> --- /dev/null
> +++ b/arch/riscv/dts/microchip-icicle-kit-a000.dts
> @@ -0,0 +1,419 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/* Copyright (c) 2020 Microchip Technology Inc */
> +
> +/dts-v1/;
> +#include "dt-bindings/clock/microchip,pfsoc-clock.h"
> +
> +/ {
> +   #address-cells = <2>;
> +   #size-cells = <2>;
> +   model = "Microchip PolarFire-SoC";
> +   compatible = "microchip,polarfire-soc";
> +
> +   aliases {
> +   serial0 = 
> +   ethernet0 = 

I see that HSS uses uart0 and over here we are assigning uart1. This
forces users to always use two UARTs on the ICICLE board. This is very
inconvenient and not at all consistent with SiFive Unleashed  and other
ARM boards.

It should be user's choice to use whether or not to use separate uart for
S-mode software. Please don't force multiple uarts by default.

By default, I suggest to use uart0 for U-Boot S-mode. If users want to
use separate uart for Linux then they can do it using "bootargs" passed
to Linux kernel from U-Boot.

U-Boot being transient in the boot flow should use same uart as
previous booting stage (in this case HSS).

> +   };
> +
> +   chosen {
> +   stdout-path = "serial0";
> +   };
> +
> +   cpucomplex: cpus {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   timebase-frequency = <100>;
> +   cpu0: cpu@0 {
> +   clocks = < CLK_CPU>;
> +   compatible = "sifive,e51", "sifive,rocket0", "riscv";
> +   device_type = "cpu";
> +   i-cache-block-size = <64>;
> +   i-cache-sets = <128>;
> +   i-cache-size = <16384>;
> +   reg = <0>;
> +   riscv,isa = "rv64imac";
> +   status = "disabled";
> +   operating-points = <
> +   /* kHz  uV */
> +   60  110
> +   30   95
> +   15   75
> +   >;
> +   cpu0intc: interrupt-controller {
> +   #interrupt-cells = <1>;
> +   compatible = "riscv,cpu-intc";
> +   interrupt-controller;
> +   };
> +   };
> +   cpu1: cpu@1 {
> +   clocks = < CLK_CPU>;
> +   compatible = "sifive,u54-mc", "sifive,rocket0", 
> "riscv";
> +   d-cache-block-size = <64>;
> +   d-cache-sets = <64>;
> +   d-cache-size = <32768>;
> +   d-tlb-sets = <1>;
> +   d-tlb-size = <32>;
> +   device_type = "cpu";
> +   i-cache-block-size = <64>;
> +   i-cache-sets = <64>;
> +   i-cache-size = <32768>;
> +   i-tlb-sets = <1>;
> +   i-tlb-size = <32>;
> +   mmu-type = "riscv,sv39";
> +   reg = <1>;
> +   riscv,isa = "rv64imafdc";
> +   tlb-split;
> +   status = "okay";
> +   operating-points = <
> +   /* kHz  uV */
> +   60  110
> +   30   95
> +   15   75
> +   >;
> +   cpu1intc: interrupt-controller {
> +   #interrupt-cells = <1>;
> +   compatible = "riscv,cpu-intc";
> +   interrupt-controller;
> +   };
> +   };
> +   cpu2: 

Re: [PATCH v1 0/8] Microchip PolarFire SoC support

2020-10-17 Thread Anup Patel
On Fri, Oct 16, 2020 at 7:54 PM  wrote:
>
> From: Padmarao Begari 
>
> This patch set adds Microchip PolarFire SoC Icicle Kit support
> to RISC-V U-Boot.
>
> The patches are based upon latest U-Boot tree
> (https://gitlab.denx.de/u-boot/u-boot.git) at commit id
> 9dc6aef8c963ae17e1263b89c692792fce0c7198
>
> All drivers namely: NS16550 Serial, Microchip clock,
> Cadence eMMC and Cadence MACB Ethernet work fine on actual
> Microchip PolarFire SoC Icicle Kit.
>
> Padmarao Begari (8):
>   riscv: Add Microchip MPFS Icicle Kit support
>   riscv: dts: Add device tree for Microchip Icicle Kit
>   dt-bindings: clock: Add indexes for reset signals
>   riscv: Add DMA 64-bit address support
>   net: macb: Add DMA 64-bit address support for macb
>   net: macb: Add phy address to read it from device tree
>   clk: Add Microchip PolarFire SoC clock driver
>   doc: board: Add Microchip MPFS Icicle Kit doc

The way patches are organized, it breaks git bisect-ability
because PATCH2 depends on PATCH3. Also, PATCH3 should
be squashed into PATCH7.

Based on dependency, here's better ordering of patches:
PATCH1) riscv: Add DMA 64-bit address support
PATCH2) net: macb: Add DMA 64-bit address support for macb
PATCH3) net: macb: Add phy address to read it from device tree
PATCH4) clk: Add Microchip PolarFire SoC clock driver
(Note: PATCH4 also include "dt-bindings: clock: Add indexes for reset signals")
PATCH5) riscv: dts: Add device tree for Microchip Icicle Kit
PATCH6) riscv: Add Microchip MPFS Icicle Kit support
PATCH7) doc: board: Add Microchip MPFS Icicle Kit doc

Regards,
Anup

>
>  arch/riscv/Kconfig|   5 +
>  arch/riscv/dts/Makefile   |   1 +
>  arch/riscv/dts/microchip-icicle-kit-a000.dts  | 419 
>  arch/riscv/include/asm/types.h|   4 +
>  board/microchip/mpfs_icicle/Kconfig   |  26 +
>  board/microchip/mpfs_icicle/mpfs_icicle.c |  96 ++-
>  configs/microchip_mpfs_icicle_defconfig   |   9 +-
>  doc/board/index.rst   |   1 +
>  doc/board/microchip/index.rst |   9 +
>  doc/board/microchip/mpfs_icicle.rst   | 605 ++
>  drivers/clk/Kconfig   |   1 +
>  drivers/clk/Makefile  |   1 +
>  drivers/clk/microchip/Kconfig |   5 +
>  drivers/clk/microchip/Makefile|   1 +
>  drivers/clk/microchip/clk_pfsoc.c | 120 
>  drivers/clk/microchip/clk_pfsoc.h |  19 +
>  drivers/clk/microchip/clk_pfsoc_cfg.c | 135 
>  drivers/clk/microchip/clk_pfsoc_periph.c  | 171 +
>  drivers/net/macb.c|  57 +-
>  drivers/net/macb.h|   6 +
>  include/configs/microchip_mpfs_icicle.h   |  60 +-
>  .../dt-bindings/clock/microchip,pfsoc-clock.h |  45 ++
>  22 files changed, 1744 insertions(+), 52 deletions(-)
>  create mode 100644 arch/riscv/dts/microchip-icicle-kit-a000.dts
>  create mode 100644 doc/board/microchip/index.rst
>  create mode 100644 doc/board/microchip/mpfs_icicle.rst
>  create mode 100644 drivers/clk/microchip/Kconfig
>  create mode 100644 drivers/clk/microchip/Makefile
>  create mode 100644 drivers/clk/microchip/clk_pfsoc.c
>  create mode 100644 drivers/clk/microchip/clk_pfsoc.h
>  create mode 100644 drivers/clk/microchip/clk_pfsoc_cfg.c
>  create mode 100644 drivers/clk/microchip/clk_pfsoc_periph.c
>  create mode 100644 include/dt-bindings/clock/microchip,pfsoc-clock.h
>
> --
> 2.17.1
>


[PATCH 1/1] trace: conserve gd register on RISC-V

2020-10-17 Thread Heinrich Schuchardt
An UEFI application may change the value of the register that gd lives in.
But some of our functions like get_ticks() access this register. So we
have to set the gd register to the U-Boot value when entering a trace
point and set it back to the application value when exiting the trace
point.

Signed-off-by: Heinrich Schuchardt 
---
 lib/trace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/trace.c b/lib/trace.c
index 831283c283..defc9716d8 100644
--- a/lib/trace.c
+++ b/lib/trace.c
@@ -57,7 +57,7 @@ static inline uintptr_t 
__attribute__((no_instrument_function))
return offset / FUNC_SITE_SIZE;
 }

-#if defined(CONFIG_EFI_LOADER) && defined(CONFIG_ARM)
+#if defined(CONFIG_EFI_LOADER) && (defined(CONFIG_ARM) || 
defined(CONFIG_RISCV))

 /**
  * trace_gd - the value of the gd register
--
2.28.0