Re: [GIT PULL] ARM: tegra: Changes for v2023.10-rc1

2023-08-21 Thread Svyatoslav Ryhel



18 серпня 2023 р. 20:04:47 GMT+03:00, Tom Rini  
написав(-ла):
>On Fri, Aug 18, 2023 at 08:02:30PM +0300, Svyatoslav Ryhel wrote:
>> 
>> 
>> 18 серпня 2023 р. 19:49:43 GMT+03:00, Tom Rini  
>> написав(-ла):
>> >On Fri, Aug 18, 2023 at 03:39:22PM +0200, Thierry Reding wrote:
>> >
>> >> From: Thierry Reding 
>> >> 
>> >> Hi Tom,
>> >> 
>> >> The following changes since commit 
>> >> 68c07fc5fdf34f0926cf06fc0c4ebd6f2f3afe19:
>> >> 
>> >>   Merge https://source.denx.de/u-boot/custodians/u-boot-usb (2023-06-21 
>> >> 14:42:50 -0400)
>> >> 
>> >> are available in the Git repository at:
>> >> 
>> >>   g...@source.denx.de:u-boot/custodians/u-boot-tegra.git 
>> >> tags/tegra-for-2023.10-rc1
>> >
>> >FYI, in your .git/config you can do:
>> >url = https://source.denx.de/u-boot/custodians/u-boot-tegra.git
>> >pushurl = g...@source.denx.de:u-boot/custodians/u-boot-tegra.git
>> >
>> >And this part looks "nicer".  Not a big deal, just an FYI.
>> >
>> >> for you to fetch changes up to bdf9dead86f06c7d6980c399a4a6339430b531ec:
>> >> 
>> >>   board: htc: endeavoru: add One X support (2023-06-30 15:20:37 +0200)
>> >> 
>> >> This passes CI successfully:
>> >> 
>> >>   https://source.denx.de/u-boot/custodians/u-boot-tegra/-/pipelines/17411
>> >> 
>> >> Thanks,
>> >> Thierry
>> >> 
>> >> 
>> >> ARM: tegra: Changes for v2023.10-rc1
>> >> 
>> >> This adds support for various new Tegra30 boards (ASUS, LG and HTC) and
>> >> has some other minor enhancements, such as enabling the poweroff command
>> >> on several Tegra210 and Tegra186 boards.
>> >> 
>> >> 
>> >> Jonas Schwöbel (1):
>> >>   configs: tegra-common-post: make PXE and DHCP boot targets optional
>> >> 
>> >> Svyatoslav Ryhel (6):
>> >>   configs: tegra-common-post: add GPIO keyboard as STDIN device
>> >>   ARM: tegra: add SoC UID calculation function
>> >>   board: asus: transformer: add ASUS Transformer T30 family support
>> >>   board: asus: grouper: add Google Nexus 7 (2012) support
>> >>   board: lg: x3: add Optimus 4X HD and Optimus Vu support
>> >>   board: htc: endeavoru: add One X support
>> >> 
>> >> Thierry Reding (2):
>> >>   ARM: tegra: Enable poweroff command on Jetson TX1 and Jetson Nano
>> >>   ARM: tegra: Enable poweroff command on Jetson TX2
>> >> 
>> >> Tomasz Maciej Nowak (1):
>> >>   ARM: dts: trimslice: sync SPI node with Linux dts
>> >
>> >Since I'm not sure if this is v8 or v9, given when v9 was posted, I've
>> >applied this PR now, as I had said I wanted this in.  I do have two
>> >requests for you Svyatoslav, however.  First, if there's changes that
>> >are missing in master, please post those ASAP and we'll get them in, for
>> >v2023.10. 
>> 
>> Patches Thierry applied are v8, while v9 has significant improvements.
>
>OK, I've updated patchwork to try and reflect this too.
>

Merged patches are not the most recent I have sent. How to fix this?

>> Second, on Monday after I release -rc3 and merge that in to
>> >next, in the next branch we allow for (and encourage!) config fragments
>> 
>> I will apply fragments move in the next patchset for t20/t30 devices
>> which will be send once this pathes are applied. This is acceptable
>> for you?
>
>Yes, I think this will be fine, thanks!
>


Re: [PATCH v2 7/7] ufs: Implement cache management

2023-08-21 Thread Bhupesh Sharma

On 8/16/23 8:35 PM, Marek Vasut wrote:

Add function to flush and invalidate cache over request and response
queue entries, and perform flush and optional invalidate over block
layer data that are passed into the UFS layer. This makes it possible
to use UFS with caches enabled.

Signed-off-by: Marek Vasut 
---
Cc: Bhupesh Sharma 
Cc: Faiz Abbas 
---
V2: No change
---
  drivers/ufs/ufs.c | 42 +-
  1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/ufs.c b/drivers/ufs/ufs.c
index 041caee714f..7c48d57f99d 100644
--- a/drivers/ufs/ufs.c
+++ b/drivers/ufs/ufs.c
@@ -692,6 +692,21 @@ static inline u8 ufshcd_get_upmcrs(struct ufs_hba *hba)
return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) >> 8) & 0x7;
  }
  
+/**

+ * ufshcd_cache_flush_and_invalidate - Flush and invalidate cache
+ *
+ * Flush and invalidate cache in aligned address..address+size range.
+ * The invalidation is in place to avoid stale data in cache.
+ */
+static void ufshcd_cache_flush_and_invalidate(void *addr, unsigned long size)
+{
+   uintptr_t aaddr = (uintptr_t)addr & ~(ARCH_DMA_MINALIGN - 1);
+   unsigned long asize = ALIGN(size, ARCH_DMA_MINALIGN);
+
+   flush_dcache_range(aaddr, aaddr + asize);
+   invalidate_dcache_range(aaddr, aaddr + asize);
+}
+
  /**
   * ufshcd_prepare_req_desc_hdr() - Fills the requests header
   * descriptor according to request
@@ -735,6 +750,8 @@ static void ufshcd_prepare_req_desc_hdr(struct ufs_hba *hba,
req_desc->header.dword_3 = 0;
  
  	req_desc->prd_table_length = 0;

+
+   ufshcd_cache_flush_and_invalidate(req_desc, sizeof(*req_desc));
  }
  
  static void ufshcd_prepare_utp_query_req_upiu(struct ufs_hba *hba,

@@ -763,10 +780,15 @@ static void ufshcd_prepare_utp_query_req_upiu(struct 
ufs_hba *hba,
memcpy(_req_ptr->qr, >request.upiu_req, QUERY_OSF_SIZE);
  
  	/* Copy the Descriptor */

-   if (query->request.upiu_req.opcode == UPIU_QUERY_OPCODE_WRITE_DESC)
+   if (query->request.upiu_req.opcode == UPIU_QUERY_OPCODE_WRITE_DESC) {
memcpy(ucd_req_ptr + 1, query->descriptor, len);
+   ufshcd_cache_flush_and_invalidate(ucd_req_ptr, 2 * 
sizeof(*ucd_req_ptr));
+   } else {
+   ufshcd_cache_flush_and_invalidate(ucd_req_ptr, 
sizeof(*ucd_req_ptr));
+   }
  
  	memset(hba->ucd_rsp_ptr, 0, sizeof(struct utp_upiu_rsp));

+   ufshcd_cache_flush_and_invalidate(hba->ucd_rsp_ptr, 
sizeof(*hba->ucd_rsp_ptr));
  }
  
  static inline void ufshcd_prepare_utp_nop_upiu(struct ufs_hba *hba)

@@ -783,6 +805,9 @@ static inline void ufshcd_prepare_utp_nop_upiu(struct 
ufs_hba *hba)
ucd_req_ptr->header.dword_2 = 0;
  
  	memset(hba->ucd_rsp_ptr, 0, sizeof(struct utp_upiu_rsp));

+
+   ufshcd_cache_flush_and_invalidate(ucd_req_ptr, sizeof(*ucd_req_ptr));
+   ufshcd_cache_flush_and_invalidate(hba->ucd_rsp_ptr, 
sizeof(*hba->ucd_rsp_ptr));
  }
  
  /**

@@ -1409,6 +1434,8 @@ void ufshcd_prepare_utp_scsi_cmd_upiu(struct ufs_hba *hba,
memcpy(ucd_req_ptr->sc.cdb, pccb->cmd, cdb_len);
  
  	memset(hba->ucd_rsp_ptr, 0, sizeof(struct utp_upiu_rsp));

+   ufshcd_cache_flush_and_invalidate(ucd_req_ptr, sizeof(*ucd_req_ptr));
+   ufshcd_cache_flush_and_invalidate(hba->ucd_rsp_ptr, 
sizeof(*hba->ucd_rsp_ptr));
  }
  
  static inline void prepare_prdt_desc(struct ufshcd_sg_entry *entry,

@@ -1423,6 +1450,7 @@ static void prepare_prdt_table(struct ufs_hba *hba, 
struct scsi_cmd *pccb)
  {
struct utp_transfer_req_desc *req_desc = hba->utrdl;
struct ufshcd_sg_entry *prd_table = hba->ucd_prdt_ptr;
+   uintptr_t aaddr = (uintptr_t)(pccb->pdata) & ~(ARCH_DMA_MINALIGN - 1);
ulong datalen = pccb->datalen;
int table_length;
u8 *buf;
@@ -1430,9 +1458,19 @@ static void prepare_prdt_table(struct ufs_hba *hba, 
struct scsi_cmd *pccb)
  
  	if (!datalen) {

req_desc->prd_table_length = 0;
+   ufshcd_cache_flush_and_invalidate(req_desc, sizeof(*req_desc));
return;
}
  
+	if (pccb->dma_dir == DMA_TO_DEVICE) {	/* Write to device */

+   flush_dcache_range(aaddr, aaddr +
+  ALIGN(datalen, ARCH_DMA_MINALIGN));
+   }
+
+   /* In any case, invalidate cache to avoid stale data in it. */
+   invalidate_dcache_range(aaddr, aaddr +
+   ALIGN(datalen, ARCH_DMA_MINALIGN));
+
table_length = DIV_ROUND_UP(pccb->datalen, MAX_PRDT_ENTRY);
buf = pccb->pdata;
i = table_length;
@@ -1446,6 +1484,8 @@ static void prepare_prdt_table(struct ufs_hba *hba, 
struct scsi_cmd *pccb)
prepare_prdt_desc(_table[table_length - i - 1], buf, datalen - 1);
  
  	req_desc->prd_table_length = table_length;

+   ufshcd_cache_flush_and_invalidate(prd_table, sizeof(*prd_table) * 
table_length);
+   ufshcd_cache_flush_and_invalidate(req_desc, sizeof(*req_desc));
  }
  
  static int 

Re: [PATCH v2 6/7] ufs: Use utp_transfer_req_desc pointer in ufshcd_get_tr_ocs

2023-08-21 Thread Bhupesh Sharma




On 8/16/23 8:35 PM, Marek Vasut wrote:

Use utp_transfer_req_desc pointer to reference to utrdl queue
instead of referencing the queue directly. This makes the code
more consistent. No functional change.

Signed-off-by: Marek Vasut 
---
Cc: Bhupesh Sharma 
Cc: Faiz Abbas 
---
V2: No change
---
  drivers/ufs/ufs.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/ufs.c b/drivers/ufs/ufs.c
index da1009e2c14..041caee714f 100644
--- a/drivers/ufs/ufs.c
+++ b/drivers/ufs/ufs.c
@@ -858,7 +858,9 @@ static inline int ufshcd_get_req_rsp(struct utp_upiu_rsp 
*ucd_rsp_ptr)
   */
  static inline int ufshcd_get_tr_ocs(struct ufs_hba *hba)
  {
-   return le32_to_cpu(hba->utrdl->header.dword_2) & MASK_OCS;
+   struct utp_transfer_req_desc *req_desc = hba->utrdl;
+
+   return le32_to_cpu(req_desc->header.dword_2) & MASK_OCS;
  }
  
  static inline int ufshcd_get_rsp_upiu_result(struct utp_upiu_rsp *ucd_rsp_ptr)


Reviewed-by: Bhupesh Sharma 
Tested-by: Bhupesh Sharma 


Re: [PATCH v2 5/7] ufs: Pass hba pointer to ufshcd_prepare_req_desc_hdr()

2023-08-21 Thread Bhupesh Sharma




On 8/16/23 8:35 PM, Marek Vasut wrote:

Pass the hba pointer itself to ufshcd_prepare_req_desc_hdr()
instead of duplicating utp_transfer_req_desc access at each
call site. No functional change.

Signed-off-by: Marek Vasut 
---
Cc: Bhupesh Sharma 
Cc: Faiz Abbas 
---
V2: No change
---
  drivers/ufs/ufs.c | 9 -
  1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/ufs/ufs.c b/drivers/ufs/ufs.c
index 58830c8ddca..da1009e2c14 100644
--- a/drivers/ufs/ufs.c
+++ b/drivers/ufs/ufs.c
@@ -696,10 +696,11 @@ static inline u8 ufshcd_get_upmcrs(struct ufs_hba *hba)
   * ufshcd_prepare_req_desc_hdr() - Fills the requests header
   * descriptor according to request
   */
-static void ufshcd_prepare_req_desc_hdr(struct utp_transfer_req_desc *req_desc,
+static void ufshcd_prepare_req_desc_hdr(struct ufs_hba *hba,
u32 *upiu_flags,
enum dma_data_direction cmd_dir)
  {
+   struct utp_transfer_req_desc *req_desc = hba->utrdl;
u32 data_direction;
u32 dword_0;
  
@@ -793,11 +794,10 @@ static int ufshcd_comp_devman_upiu(struct ufs_hba *hba,

  {
u32 upiu_flags;
int ret = 0;
-   struct utp_transfer_req_desc *req_desc = hba->utrdl;
  
  	hba->dev_cmd.type = cmd_type;
  
-	ufshcd_prepare_req_desc_hdr(req_desc, _flags, DMA_NONE);

+   ufshcd_prepare_req_desc_hdr(hba, _flags, DMA_NONE);
switch (cmd_type) {
case DEV_CMD_TYPE_QUERY:
ufshcd_prepare_utp_query_req_upiu(hba, upiu_flags);
@@ -1449,12 +1449,11 @@ static void prepare_prdt_table(struct ufs_hba *hba, 
struct scsi_cmd *pccb)
  static int ufs_scsi_exec(struct udevice *scsi_dev, struct scsi_cmd *pccb)
  {
struct ufs_hba *hba = dev_get_uclass_priv(scsi_dev->parent);
-   struct utp_transfer_req_desc *req_desc = hba->utrdl;
u32 upiu_flags;
int ocs, result = 0;
u8 scsi_status;
  
-	ufshcd_prepare_req_desc_hdr(req_desc, _flags, pccb->dma_dir);

+   ufshcd_prepare_req_desc_hdr(hba, _flags, pccb->dma_dir);
ufshcd_prepare_utp_scsi_cmd_upiu(hba, pccb, upiu_flags);
prepare_prdt_table(hba, pccb);


Reviewed-by: Bhupesh Sharma 
Tested-by: Bhupesh Sharma 


Re: [PATCH v2 4/7] ufs: Handle UFS 3.0 controllers

2023-08-21 Thread Bhupesh Sharma




On 8/16/23 8:35 PM, Marek Vasut wrote:

Extend the version check to handle UFS 3.0 controllers as well.
Tested on R-Car S4 UFS 3.0 controller.

Signed-off-by: Marek Vasut 
---
Cc: Bhupesh Sharma 
Cc: Faiz Abbas 
---
V2: No change
---
  drivers/ufs/ufs.c | 3 ++-
  drivers/ufs/ufs.h | 1 +
  2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/ufs.c b/drivers/ufs/ufs.c
index 261ae2843c2..58830c8ddca 100644
--- a/drivers/ufs/ufs.c
+++ b/drivers/ufs/ufs.c
@@ -1903,7 +1903,8 @@ int ufshcd_probe(struct udevice *ufs_dev, struct 
ufs_hba_ops *hba_ops)
if (hba->version != UFSHCI_VERSION_10 &&
hba->version != UFSHCI_VERSION_11 &&
hba->version != UFSHCI_VERSION_20 &&
-   hba->version != UFSHCI_VERSION_21)
+   hba->version != UFSHCI_VERSION_21 &&
+   hba->version != UFSHCI_VERSION_30)
dev_err(hba->dev, "invalid UFS version 0x%x\n",
hba->version);
  
diff --git a/drivers/ufs/ufs.h b/drivers/ufs/ufs.h

index 638c10b5503..9daaf03d222 100644
--- a/drivers/ufs/ufs.h
+++ b/drivers/ufs/ufs.h
@@ -781,6 +781,7 @@ enum {
UFSHCI_VERSION_11 = 0x00010100, /* 1.1 */
UFSHCI_VERSION_20 = 0x0200, /* 2.0 */
UFSHCI_VERSION_21 = 0x0210, /* 2.1 */
+   UFSHCI_VERSION_30 = 0x0300, /* 3.0 */
  };
  
  /* Interrupt disable masks */


Reviewed-by: Bhupesh Sharma 
Tested-by: Bhupesh Sharma 


Re: [PATCH v2 3/7] ufs: Add UFSHCD_QUIRK_HIBERN_FASTAUTO

2023-08-21 Thread Bhupesh Sharma




On 8/16/23 8:35 PM, Marek Vasut wrote:

Add UFSHCD_QUIRK_HIBERN_FASTAUTO quirk for host controllers which supports
auto-hibernate the capability but only FASTAUTO mode.

Ported from Linux kernel commit
2f11bbc2c7f3 ("scsi: ufs: core: Add UFSHCD_QUIRK_HIBERN_FASTAUTO")

Signed-off-by: Marek Vasut 
---
Cc: Bhupesh Sharma 
Cc: Faiz Abbas 
---
V2: Use BIT() macro
---
  drivers/ufs/ufs.c | 9 +++--
  drivers/ufs/ufs.h | 6 ++
  2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/ufs/ufs.c b/drivers/ufs/ufs.c
index da0550d98c6..261ae2843c2 100644
--- a/drivers/ufs/ufs.c
+++ b/drivers/ufs/ufs.c
@@ -1631,8 +1631,13 @@ static int ufshcd_get_max_pwr_mode(struct ufs_hba *hba)
if (hba->max_pwr_info.is_valid)
return 0;
  
-	pwr_info->pwr_tx = FAST_MODE;

-   pwr_info->pwr_rx = FAST_MODE;
+   if (hba->quirks & UFSHCD_QUIRK_HIBERN_FASTAUTO) {
+   pwr_info->pwr_tx = FASTAUTO_MODE;
+   pwr_info->pwr_rx = FASTAUTO_MODE;
+   } else {
+   pwr_info->pwr_tx = FAST_MODE;
+   pwr_info->pwr_rx = FAST_MODE;
+   }
pwr_info->hs_rate = PA_HS_MODE_B;
  
  	/* Get the connected lane count */

diff --git a/drivers/ufs/ufs.h b/drivers/ufs/ufs.h
index e5ddb6f64a9..638c10b5503 100644
--- a/drivers/ufs/ufs.h
+++ b/drivers/ufs/ufs.h
@@ -725,6 +725,12 @@ struct ufs_hba {
   */
  #define UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS BIT(1)
  
+/*

+ * This quirk needs to be enabled if the host controller has
+ * auto-hibernate capability but it's FASTAUTO only.
+ */
+#define UFSHCD_QUIRK_HIBERN_FASTAUTO   BIT(2)
+
/* Virtual memory reference */
struct utp_transfer_cmd_desc *ucdl;
struct utp_transfer_req_desc *utrdl;


Reviewed-by: Bhupesh Sharma 
Tested-by: Bhupesh Sharma 


Re: [PATCH v2 2/7] ufs: Add UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS

2023-08-21 Thread Bhupesh Sharma




On 8/16/23 8:35 PM, Marek Vasut wrote:

Add UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS for host controllers which do not
support 64-bit addressing.

Ported from Linux kernel commit
6554400d6f66 ("scsi: ufs: core: Add UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS")
with ufs_scsi_buffer_aligned() based on U-Boot generic bounce buffer.

Signed-off-by: Marek Vasut 
---
Cc: Bhupesh Sharma 
Cc: Faiz Abbas 
---
V2: Use BIT() macro
---
  drivers/ufs/ufs.c | 26 ++
  drivers/ufs/ufs.h |  6 ++
  2 files changed, 32 insertions(+)

diff --git a/drivers/ufs/ufs.c b/drivers/ufs/ufs.c
index 3bf1a95e7f2..da0550d98c6 100644
--- a/drivers/ufs/ufs.c
+++ b/drivers/ufs/ufs.c
@@ -8,6 +8,7 @@
   * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com
   */
  
+#include 

  #include 
  #include 
  #include 
@@ -1889,6 +1890,8 @@ int ufshcd_probe(struct udevice *ufs_dev, struct 
ufs_hba_ops *hba_ops)
  
  	/* Read capabilties registers */

hba->capabilities = ufshcd_readl(hba, REG_CONTROLLER_CAPABILITIES);
+   if (hba->quirks & UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS)
+   hba->capabilities &= ~MASK_64_ADDRESSING_SUPPORT;
  
  	/* Get UFS version supported by the controller */

hba->version = ufshcd_get_ufs_version(hba);
@@ -1942,8 +1945,31 @@ int ufs_scsi_bind(struct udevice *ufs_dev, struct 
udevice **scsi_devp)
return ret;
  }
  
+#if IS_ENABLED(CONFIG_BOUNCE_BUFFER)

+static int ufs_scsi_buffer_aligned(struct udevice *scsi_dev, struct 
bounce_buffer *state)
+{
+#ifdef CONFIG_PHYS_64BIT
+   struct ufs_hba *hba = dev_get_uclass_priv(scsi_dev->parent);
+   uintptr_t ubuf = (uintptr_t)state->user_buffer;
+   size_t len = state->len_aligned;
+
+   /* Check if below 32bit boundary */
+   if ((hba->quirks & UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS) &&
+   ((ubuf >> 32) || (ubuf + len) >> 32)) {
+   dev_dbg(scsi_dev, "Buffer above 32bit boundary %lx-%lx\n",
+   ubuf, ubuf + len);
+   return 0;
+   }
+#endif
+   return 1;
+}
+#endif /* CONFIG_BOUNCE_BUFFER */
+
  static struct scsi_ops ufs_ops = {
.exec   = ufs_scsi_exec,
+#if IS_ENABLED(CONFIG_BOUNCE_BUFFER)
+   .buffer_aligned = ufs_scsi_buffer_aligned,
+#endif /* CONFIG_BOUNCE_BUFFER */
  };
  
  int ufs_probe_dev(int index)

diff --git a/drivers/ufs/ufs.h b/drivers/ufs/ufs.h
index 5a5c13aefdf..e5ddb6f64a9 100644
--- a/drivers/ufs/ufs.h
+++ b/drivers/ufs/ufs.h
@@ -719,6 +719,12 @@ struct ufs_hba {
   */
  #define UFSHCD_QUIRK_BROKEN_LCC   BIT(0)
  
+/*

+ * This quirk needs to be enabled if the host controller has
+ * 64-bit addressing supported capability but it doesn't work.
+ */
+#define UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS  BIT(1)
+
/* Virtual memory reference */
struct utp_transfer_cmd_desc *ucdl;
struct utp_transfer_req_desc *utrdl;


Reviewed-by: Bhupesh Sharma 
Tested-by: Bhupesh Sharma 


Re: [PATCH v2 1/7] ufs: Convert quirks to BIT() macro

2023-08-21 Thread Bhupesh Sharma

On 8/16/23 8:35 PM, Marek Vasut wrote:

Use BIT() macro for quirks, no functional change.

Signed-off-by: Marek Vasut 
---
Cc: Bhupesh Sharma 
Cc: Faiz Abbas 
---
V2: New patch
---
  drivers/ufs/ufs.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ufs/ufs.h b/drivers/ufs/ufs.h
index 8a38832b05f..5a5c13aefdf 100644
--- a/drivers/ufs/ufs.h
+++ b/drivers/ufs/ufs.h
@@ -717,7 +717,7 @@ struct ufs_hba {
   * the LCC transmission on UFS device (by clearing TX_LCC_ENABLE
   * attribute of device to 0).
   */
-#define UFSHCD_QUIRK_BROKEN_LCC0x1
+#define UFSHCD_QUIRK_BROKEN_LCCBIT(0)
  
  	/* Virtual memory reference */

struct utp_transfer_cmd_desc *ucdl;


Reviewed-by: Bhupesh Sharma 


Re: [PATCH v2 2/2] riscv: cpu: make riscv_cpu_probe to EVT_DM_POST_INIT_R callback

2023-08-21 Thread Roland Ruckerbauer

Tested on visionfive2, it works.

On 18.08.23 07:11, Chanho Park wrote:

Since the Patch 55171aedda88, VisionFive2 booting has been broken [1].
VisionFive2 board requires to enable CONFIG_TIMER_EARLY but booting went
to panic from initr_dm_devices due to lack of a timer device.

- Error logs
initcall sequence fffd8d38 failed at call 402185e4
(err=-19)

Thus, we need to move riscv_cpu_probe function in order to register
the timer earlier than initr_dm_devices.

Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
Cc: Simon Glass 
Cc: Bin Meng 
Signed-off-by: Chanho Park 

Tested-by: Roland Ruckerbauer 

---
  arch/riscv/cpu/cpu.c | 11 +++
  1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
index ecfb1fb08c4b..0b4208e72199 100644
--- a/arch/riscv/cpu/cpu.c
+++ b/arch/riscv/cpu/cpu.c
@@ -66,7 +66,7 @@ static inline bool supports_extension(char ext)
  #endif /* CONFIG_CPU */
  }
  
-static int riscv_cpu_probe(void)

+static int riscv_cpu_probe(void *ctx, struct event *event)
  {
  #ifdef CONFIG_CPU
int ret;
@@ -79,6 +79,7 @@ static int riscv_cpu_probe(void)
  
  	return 0;

  }
+EVENT_SPY(EVT_DM_POST_INIT_R, riscv_cpu_probe);
  
  /*

   * This is called on secondary harts just after the IPI is init'd. Currently
@@ -95,7 +96,7 @@ int riscv_cpu_setup(void *ctx, struct event *event)
  {
int ret;
  
-	ret = riscv_cpu_probe();

+   ret = riscv_cpu_probe(ctx, event);
if (ret)
return ret;
  
@@ -149,12 +150,6 @@ EVENT_SPY(EVT_DM_POST_INIT_F, riscv_cpu_setup);
  
  int arch_early_init_r(void)

  {
-   int ret;
-
-   ret = riscv_cpu_probe();
-   if (ret)
-   return ret;
-
if (IS_ENABLED(CONFIG_SYSRESET_SBI))
device_bind_driver(gd->dm_root, "sbi-sysreset",
   "sbi-sysreset", NULL);


Re: [PATCH v2 1/2] dm: event: add EVT_DM_POST_INIT_R event type

2023-08-21 Thread Roland Ruckerbauer

Tested on visionfive2 board, it works. Thanks for taking a look into this.

On 18.08.23 07:11, Chanho Park wrote:

This patch introduces EVT_DM_POST_INIT_R event type for handling hooks
after relocation.

Fixes: 55171aedda88 ("dm: Emit the arch_cpu_init_dm() even only before 
relocation")
Suggested-by: Simon Glass 
Cc: Bin Meng 
Signed-off-by: Chanho Park 


Tested-by: Roland Ruckerbauer 


---
  drivers/core/root.c | 6 --
  include/event.h | 1 +
  2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/core/root.c b/drivers/core/root.c
index 6775fb0b6575..79d871ab291a 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -436,8 +436,10 @@ int dm_init_and_scan(bool pre_reloc_only)
return ret;
}
}
-   if (CONFIG_IS_ENABLED(DM_EVENT) && !(gd->flags & GD_FLG_RELOC)) {
-   ret = event_notify_null(EVT_DM_POST_INIT_F);
+   if (CONFIG_IS_ENABLED(DM_EVENT)) {
+   ret = event_notify_null(gd->flags & GD_FLG_RELOC ?
+   EVT_DM_POST_INIT_R :
+   EVT_DM_POST_INIT_F);
if (ret)
return log_msg_ret("ev", ret);
}
diff --git a/include/event.h b/include/event.h
index daf44bf8a83b..bb38ba98e73b 100644
--- a/include/event.h
+++ b/include/event.h
@@ -24,6 +24,7 @@ enum event_t {
  
  	/* Events related to driver model */

EVT_DM_POST_INIT_F,
+   EVT_DM_POST_INIT_R,
EVT_DM_PRE_PROBE,
EVT_DM_POST_PROBE,
EVT_DM_PRE_REMOVE,


Re: [PATCH] cmd: dm: allow for selecting uclass and device

2023-08-21 Thread Simon Glass
Hi AKASHI,

On Mon, 21 Aug 2023 at 20:46, AKASHI Takahiro
 wrote:
>
> The output from "dm tree" or "dm uclass" is a bit annoying
> if the number of devices available on the system is huge.
> (This is especially true on sandbox when I debug some DM code.)
>
> With this patch, we can specify the uclass or the device
> that we are interested in in order to limit the output.
>
> For instance,
>
> => dm uclass usb
> uclass 121: usb
> 0 usb@1 @ 0bd008b0, seq 1
>
> => dm tree usb:usb@1

Perhaps this should just provide a substring to search for and it can
find everything with a uclass name or device name that contains the
string?

Otherwise, can you drop the usb. part ? Is it needed?

>  Class Index  Probed  DriverName
> ---
>  usb   0  [   ]   usb_sandbox   usb@1
>  usb_hub   0  [   ]   usb_hub   `-- hub
>  usb_emul  0  [   ]   usb_sandbox_hub   `-- hub-emul
>  usb_emul  1  [   ]   usb_sandbox_flash |-- flash-stick@0
>  usb_emul  2  [   ]   usb_sandbox_flash |-- flash-stick@1
>  usb_emul  3  [   ]   usb_sandbox_flash |-- flash-stick@2
>  usb_emul  4  [   ]   usb_sandbox_keyb  `-- keyb@3
>
> Please note that, at "dm tree", the uclass name (usb) as well as
> the device name (usb@1) is required.
>
> Signed-off-by: AKASHI Takahiro 
> ---
>  cmd/dm.c| 30 
>  drivers/core/dump.c | 48 +++--
>  include/dm/util.h   | 13 
>  3 files changed, 73 insertions(+), 18 deletions(-)

Thanks, I've been wanting this for ages.

Can you please add doc/ as well?

>
> diff --git a/cmd/dm.c b/cmd/dm.c
> index 3263547cbec6..99268df2f87a 100644
> --- a/cmd/dm.c
> +++ b/cmd/dm.c
> @@ -59,11 +59,20 @@ static int do_dm_dump_static_driver_info(struct cmd_tbl 
> *cmdtp, int flag,
>  static int do_dm_dump_tree(struct cmd_tbl *cmdtp, int flag, int argc,
>char *const argv[])
>  {
> -   bool sort;
> +   bool sort = false;
> +   char *device = NULL;
>
> -   sort = argc > 1 && !strcmp(argv[1], "-s");
> +   if (argc > 1) {
> +   if (!strcmp(argv[1], "-s")) {
> +   sort = true;
> +   argc--;
> +   argv++;
> +   }
> +   if (argc > 1)
> +   device = argv[1];
> +   }
>
> -   dm_dump_tree(sort);
> +   dm_dump_tree(device, sort);
>
> return 0;
>  }
> @@ -71,7 +80,12 @@ static int do_dm_dump_tree(struct cmd_tbl *cmdtp, int 
> flag, int argc,
>  static int do_dm_dump_uclass(struct cmd_tbl *cmdtp, int flag, int argc,
>  char *const argv[])
>  {
> -   dm_dump_uclass();
> +   char *uclass = NULL;
> +
> +   if (argc > 1)
> +   uclass = argv[1];
> +
> +   dm_dump_uclass(uclass);
>
> return 0;
>  }
> @@ -91,8 +105,8 @@ static char dm_help_text[] =
> "dm drivers   Dump list of drivers with uclass and instances\n"
> DM_MEM_HELP
> "dm staticDump list of drivers with static platform data\n"
> -   "dm tree [-s] Dump tree of driver model devices (-s=sort)\n"
> -   "dm uclassDump list of instances for each uclass"
> +   "dm tree [-s][name] Dump tree of driver model devices (-s=sort)\n"
> +   "dm uclass [name] Dump list of instances for each uclass"
> ;
>  #endif
>
> @@ -102,5 +116,5 @@ U_BOOT_CMD_WITH_SUBCMDS(dm, "Driver model low level 
> access", dm_help_text,
> U_BOOT_SUBCMD_MKENT(drivers, 1, 1, do_dm_dump_drivers),
> DM_MEM
> U_BOOT_SUBCMD_MKENT(static, 1, 1, do_dm_dump_static_driver_info),
> -   U_BOOT_SUBCMD_MKENT(tree, 2, 1, do_dm_dump_tree),
> -   U_BOOT_SUBCMD_MKENT(uclass, 1, 1, do_dm_dump_uclass));
> +   U_BOOT_SUBCMD_MKENT(tree, 3, 1, do_dm_dump_tree),
> +   U_BOOT_SUBCMD_MKENT(uclass, 2, 1, do_dm_dump_uclass));
> diff --git a/drivers/core/dump.c b/drivers/core/dump.c
> index 3e77832a3a00..855d6ca002b7 100644
> --- a/drivers/core/dump.c
> +++ b/drivers/core/dump.c
> @@ -85,11 +85,35 @@ static void show_devices(struct udevice *dev, int depth, 
> int last_flag,
> }
>  }
>
> -void dm_dump_tree(bool sort)
> +void dm_dump_tree(char *uclass, bool sort)
>  {
> struct udevice *root;
>
> -   root = dm_root();
> +   if (uclass) {
> +   char *device;
> +   enum uclass_id id;
> +
> +   device = strchr(uclass, ':');
> +   if (!device) {
> +   printf("name %s: invalid format\n", uclass);
> +   return;
> +   }

Can the parsing needs to happen in the caller? This might be called
from SPL, for example. We don't want it returning an error

> +   *device = '\0';
> +   device++;
> +
> +   id = 

[PATCH 14/14] event: Use an event to replace last_stage_init()

2023-08-21 Thread Simon Glass
Add a new event which handles this function. Convert existing use of
the function to use the new event instead.

Make sure that EVENT is enabled by affected boards, by selecting it from
the LAST_STAGE_INIT option. For x86, enable it by default since all boards
need it.

For controlcenterdc, inline the get_tpm() function and make sure the event
is not built in SPL.

Signed-off-by: Simon Glass 
---

 arch/Kconfig  |  1 +
 arch/mips/mach-mtmips/cpu.c   |  6 +++-
 arch/mips/mach-pic32/cpu.c|  4 ++-
 arch/x86/cpu/coreboot/coreboot.c  |  7 -
 arch/x86/cpu/cpu.c| 10 --
 arch/x86/cpu/efi/payload.c|  4 ++-
 arch/x86/cpu/quark/quark.c|  4 ++-
 board/CZ.NIC/turris_mox/turris_mox.c  |  4 ++-
 board/Marvell/mvebu_armada-37xx/board.c   |  5 ++-
 board/Marvell/octeon_nic23/board.c|  4 ++-
 board/Marvell/octeontx2/board.c   |  4 ++-
 board/cortina/presidio-asic/presidio.c|  4 ++-
 board/emulation/qemu-ppce500/qemu-ppce500.c   |  4 ++-
 board/gdsys/a38x/controlcenterdc.c| 31 +--
 board/gdsys/mpc8308/gazerbeam.c   |  4 ++-
 board/ge/bx50v3/bx50v3.c  |  4 ++-
 board/keymile/km83xx/km83xx.c |  4 ++-
 board/keymile/kmcent2/kmcent2.c   |  3 +-
 .../keymile/pg-wcom-ls102xa/pg-wcom-ls102xa.c |  6 +---
 board/phytium/durian/durian.c |  4 ++-
 board/phytium/pomelo/pomelo.c |  4 ++-
 common/Kconfig|  1 +
 common/board_r.c  |  9 +-
 common/event.c|  1 +
 configs/bayleybay_defconfig   |  1 -
 configs/cherryhill_defconfig  |  1 -
 configs/chromebook_coral_defconfig|  1 -
 configs/chromebook_link64_defconfig   |  1 -
 configs/chromebook_link_defconfig |  1 -
 configs/chromebook_samus_defconfig|  1 -
 configs/chromebook_samus_tpl_defconfig|  1 -
 configs/chromebox_panther_defconfig   |  1 -
 ...-qeval20-qa3-e3845-internal-uart_defconfig |  1 -
 configs/conga-qeval20-qa3-e3845_defconfig |  1 -
 configs/coreboot64_defconfig  |  1 -
 configs/coreboot_defconfig|  1 -
 configs/cougarcanyon2_defconfig   |  1 -
 configs/crownbay_defconfig|  1 -
 configs/dfi-bt700-q7x-151_defconfig   |  1 -
 configs/edison_defconfig  |  1 -
 configs/efi-x86_app32_defconfig   |  1 -
 configs/efi-x86_app64_defconfig   |  1 -
 configs/efi-x86_payload32_defconfig   |  1 -
 configs/efi-x86_payload64_defconfig   |  1 -
 configs/galileo_defconfig |  1 -
 configs/minnowmax_defconfig   |  1 -
 configs/qemu-x86_64_defconfig |  3 +-
 configs/qemu-x86_defconfig|  1 -
 configs/som-db5800-som-6867_defconfig |  1 -
 ...able-x86-conga-qa3-e3845-pcie-x4_defconfig |  1 -
 .../theadorable-x86-conga-qa3-e3845_defconfig |  1 -
 configs/theadorable-x86-dfi-bt700_defconfig   |  1 -
 include/event.h   | 12 +++
 include/init.h|  1 -
 54 files changed, 95 insertions(+), 80 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index c9a335922528..90345cbee0d8 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -252,6 +252,7 @@ config X86
imply DM_SPI
imply DM_SPI_FLASH
imply DM_USB
+   imply LAST_STAGE_INIT
imply VIDEO
imply SYSRESET
imply SPL_SYSRESET
diff --git a/arch/mips/mach-mtmips/cpu.c b/arch/mips/mach-mtmips/cpu.c
index f1e902273863..e88dab10c76e 100644
--- a/arch/mips/mach-mtmips/cpu.c
+++ b/arch/mips/mach-mtmips/cpu.c
@@ -4,6 +4,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -21,7 +22,8 @@ int dram_init(void)
return 0;
 }
 
-int last_stage_init(void)
+#ifndef CONFIG_SPL_BUILD
+static int last_stage_init(void)
 {
void *src, *dst;
 
@@ -46,3 +48,5 @@ int last_stage_init(void)
 
return 0;
 }
+EVENT_SPY_SIMPLE(EVT_LAST_STAGE_INIT, last_stage_init);
+#endif
diff --git a/arch/mips/mach-pic32/cpu.c b/arch/mips/mach-pic32/cpu.c
index 785a87b618b6..7ed306e045ea 100644
--- a/arch/mips/mach-pic32/cpu.c
+++ b/arch/mips/mach-pic32/cpu.c
@@ -57,7 +57,7 @@ static ulong clk_get_cpu_rate(void)
 }
 
 /* initialize prefetch module related to cpu_clk */
-static void prefetch_init(void)
+static int prefetch_init(void)
 {
struct pic32_reg_atomic *regs;
const void __iomem *base;
@@ -93,6 +93,8 @@ static void prefetch_init(void)
/* Enable prefetch for all */
writel(0x30, >set);
iounmap(regs);
+
+   return 0;
 }
 
 /* arch-specific CPU init after DM: flash prefetch */
diff --git 

[PATCH 13/14] freescale: Drop call to init_func_vid() in the init sequence

2023-08-21 Thread Simon Glass
Use the misc_init_f event instead, which is designed for this purpose.

All boards with CONFIG_VID already enable CONFIG_EVENT.

Signed-off-by: Simon Glass 
---

 arch/arm/cpu/armv8/fsl-layerscape/spl.c |  5 +
 board/freescale/ls1088a/ls1088a.c   |  3 ++-
 board/freescale/lx2160a/lx2160a.c   |  2 ++
 common/board_f.c| 10 --
 include/init.h  |  3 ---
 5 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/spl.c 
b/arch/arm/cpu/armv8/fsl-layerscape/spl.c
index 61fced451eb5..033f48d04b90 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/spl.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/spl.c
@@ -78,6 +78,11 @@ void tzpc_init(void)
 #endif
 }
 
+__weak int init_func_vid(void)
+{
+   return 0;
+}
+
 void board_init_f(ulong dummy)
 {
int ret;
diff --git a/board/freescale/ls1088a/ls1088a.c 
b/board/freescale/ls1088a/ls1088a.c
index 7a1047a77f73..f2b8bec03729 100644
--- a/board/freescale/ls1088a/ls1088a.c
+++ b/board/freescale/ls1088a/ls1088a.c
@@ -181,13 +181,14 @@ unsigned long long get_qixis_addr(void)
 #endif
 
 #if defined(CONFIG_VID)
-int init_func_vid(void)
+static int setup_core_voltage(void)
 {
if (adjust_vdd(0) < 0)
printf("core voltage not adjusted\n");
 
return 0;
 }
+EVENT_SPY_SIMPLE(EVT_MISC_INIT_F, setup_core_voltage);
 
 u16 soc_get_fuse_vid(int vid_index)
 {
diff --git a/board/freescale/lx2160a/lx2160a.c 
b/board/freescale/lx2160a/lx2160a.c
index d631a11ff667..2883848550af 100644
--- a/board/freescale/lx2160a/lx2160a.c
+++ b/board/freescale/lx2160a/lx2160a.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -242,6 +243,7 @@ int init_func_vid(void)
return 0;
 }
 #endif
+EVENT_SPY_SIMPLE(EVT_MISC_INIT_F, init_func_vid);
 
 int checkboard(void)
 {
diff --git a/common/board_f.c b/common/board_f.c
index 46008bac6595..aef395b1354e 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -280,13 +280,6 @@ static int init_func_i2c(void)
 }
 #endif
 
-#if defined(CONFIG_VID)
-__weak int init_func_vid(void)
-{
-   return 0;
-}
-#endif
-
 static int setup_mon_len(void)
 {
 #if defined(__ARM__) || defined(__MICROBLAZE__)
@@ -896,9 +889,6 @@ static const init_fnc_t init_sequence_f[] = {
INIT_FUNC_WATCHDOG_RESET
 #if CONFIG_IS_ENABLED(SYS_I2C_LEGACY)
init_func_i2c,
-#endif
-#if defined(CONFIG_VID) && !defined(CONFIG_SPL)
-   init_func_vid,
 #endif
announce_dram_init,
dram_init,  /* configure available RAM banks */
diff --git a/include/init.h b/include/init.h
index 1bf76e4eff72..13579db75900 100644
--- a/include/init.h
+++ b/include/init.h
@@ -276,9 +276,6 @@ int set_cpu_clk_info(void);
 int update_flash_size(int flash_size);
 int arch_early_init_r(void);
 int misc_init_r(void);
-#if defined(CONFIG_VID)
-int init_func_vid(void);
-#endif
 
 /* common/board_info.c */
 int checkboard(void);
-- 
2.42.0.rc1.204.g551eb34607-goog



[PATCH 12/14] x86: Convert arch_fsp_init() to use events

2023-08-21 Thread Simon Glass
Convert this to use events instead of calling a function directly in the
init sequence.

Rename it to arch_fsp_init_f() to distinguish it from the one that happens
after relocation.

For FSPv2 nothing needs to be done here, so drop the empty function.

Signed-off-by: Simon Glass 
---

 arch/x86/lib/fsp1/fsp_common.c |  1 +
 arch/x86/lib/fsp2/fsp_common.c |  5 -
 common/board_f.c   |  4 +---
 common/event.c |  1 +
 include/event.h|  9 +
 include/init.h | 11 ---
 6 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/arch/x86/lib/fsp1/fsp_common.c b/arch/x86/lib/fsp1/fsp_common.c
index 20926171822d..df18f4767562 100644
--- a/arch/x86/lib/fsp1/fsp_common.c
+++ b/arch/x86/lib/fsp1/fsp_common.c
@@ -101,3 +101,4 @@ int arch_fsp_init(void)
 
return 0;
 }
+EVENT_SPY_SIMPLE(EVT_FSP_INIT_F, arch_fsp_init);
diff --git a/arch/x86/lib/fsp2/fsp_common.c b/arch/x86/lib/fsp2/fsp_common.c
index 20c3f6406adf..d802a86967d5 100644
--- a/arch/x86/lib/fsp2/fsp_common.c
+++ b/arch/x86/lib/fsp2/fsp_common.c
@@ -8,11 +8,6 @@
 #include 
 #include 
 
-int arch_fsp_init(void)
-{
-   return 0;
-}
-
 void board_final_cleanup(void)
 {
u32 status;
diff --git a/common/board_f.c b/common/board_f.c
index a485ba62fa17..46008bac6595 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -855,9 +855,7 @@ static const init_fnc_t init_sequence_f[] = {
 #if defined(CONFIG_CONSOLE_RECORD_INIT_F)
console_record_init,
 #endif
-#if defined(CONFIG_HAVE_FSP)
-   arch_fsp_init,
-#endif
+   INITCALL_EVENT(EVT_FSP_INIT_F),
arch_cpu_init,  /* basic arch cpu dependent setup */
mach_cpu_init,  /* SoC/machine dependent CPU setup */
initf_dm,
diff --git a/common/event.c b/common/event.c
index 55f6932ef62c..8a619045 100644
--- a/common/event.c
+++ b/common/event.c
@@ -35,6 +35,7 @@ const char *const type_name[] = {
 
/* init hooks */
"misc_init_f",
+   "fsp_init_r",
 
/* Fpga load hook */
"fpga_load",
diff --git a/include/event.h b/include/event.h
index b2cfd65c9f8b..85269aa317aa 100644
--- a/include/event.h
+++ b/include/event.h
@@ -32,6 +32,15 @@ enum event_t {
/* Init hooks */
EVT_MISC_INIT_F,
 
+   /*
+* Emitted before relocation to set up Firmware Support Package
+*
+* Where U-Boot relies on binary blobs to handle part of the system
+* init, this event can be used to set up the blobs. This is used on
+* some Intel platforms
+*/
+   EVT_FSP_INIT_F,
+
/* Fpga load hook */
EVT_FPGA_LOAD,
 
diff --git a/include/init.h b/include/init.h
index 3bf30476a2e0..1bf76e4eff72 100644
--- a/include/init.h
+++ b/include/init.h
@@ -57,17 +57,6 @@ int arch_cpu_init(void);
  */
 int mach_cpu_init(void);
 
-/**
- * arch_fsp_init() - perform firmware support package init
- *
- * Where U-Boot relies on binary blobs to handle part of the system init, this
- * function can be used to set up the blobs. This is used on some Intel
- * platforms.
- *
- * Return: 0
- */
-int arch_fsp_init(void);
-
 /**
  * arch_fsp_init() - perform post-relocation firmware support package init
  *
-- 
2.42.0.rc1.204.g551eb34607-goog



[PATCH 11/14] event: Update documentation for simple spy

2023-08-21 Thread Simon Glass
Now that we have two types of spy, mention this in the documentation. Put
the simple spy first, since it seems to be the common case.

Signed-off-by: Simon Glass 
---

 doc/develop/event.rst | 23 +--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/doc/develop/event.rst b/doc/develop/event.rst
index cb09e9c85a9a..d5043ec4f4ca 100644
--- a/doc/develop/event.rst
+++ b/doc/develop/event.rst
@@ -21,16 +21,31 @@ Declaring a spy
 
 To declare a spy, use something like this::
 
-static int snow_setup_cpus(void *ctx, struct event *event)
+static int snow_check_temperature(void)
 {
 /* do something */
 return 0;
 }
-EVENT_SPY(EVT_DM_POST_INIT_F, snow_setup_cpus);
+EVENT_SPY_SIMPLE(EVT_DM_POST_INIT_F, snow_check_temperature);
 
 This function is called when EVT_DM_POST_INIT_F is emitted, i.e. after the
 driver model is initialized (in U-Boot proper before and after relocation).
 
+If you need access to the event data, use `EVENT_SPY_FULL`, like this::
+
+static int snow_setup_cpus(void *ctx, struct event *event)
+{
+/* do something that uses event->data*/
+return 0;
+}
+EVENT_SPY_FULL(EVT_DM_POST_INIT_F, snow_setup_cpus);
+
+Note that the context is always NULL for a static spy. See below for 
information
+about how to use a dynamic spy.
+
+The return value is handled by the event emitter. If non-zero, then the error
+is returned to the function which emitted the event, i.e. the one that called
+`event_notify()`.
 
 Debugging
 -
@@ -80,6 +95,10 @@ to be notified when a particular device is probed or removed.
 This can be handled by enabling `CONFIG_EVENT_DYNAMIC`. It is then possible to
 call `event_register()` to register a new handler for a particular event.
 
+If some context is need for the spy, you can pass a pointer to
+`event_register()` to provide that. Note that the context is only passed to
+a spy registered with `EVENT_SPY_FULL`.
+
 Dynamic event handlers are called after all the static event spy handlers have
 been processed. Of course, since dynamic event handlers are created at runtime
 it is not possible to use the `event_dump.py` to see them.
-- 
2.42.0.rc1.204.g551eb34607-goog



[PATCH 10/14] event: Rename EVENT_SPY to EVENT_SPY_FULL

2023-08-21 Thread Simon Glass
The new name makes it clearer that this is for a full spy, with access to
the context and the event data.

Signed-off-by: Simon Glass 
---

 boot/vbe_request.c   | 2 +-
 boot/vbe_simple_os.c | 2 +-
 include/event.h  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/boot/vbe_request.c b/boot/vbe_request.c
index 312edfa2bdb5..2f218d4bf97c 100644
--- a/boot/vbe_request.c
+++ b/boot/vbe_request.c
@@ -230,4 +230,4 @@ static int bootmeth_vbe_ft_fixup(void *ctx, struct event 
*event)
 
return 0;
 }
-EVENT_SPY(EVT_FT_FIXUP, bootmeth_vbe_ft_fixup);
+EVENT_SPY_FULL(EVT_FT_FIXUP, bootmeth_vbe_ft_fixup);
diff --git a/boot/vbe_simple_os.c b/boot/vbe_simple_os.c
index 8c641ec07e2d..3285e438a568 100644
--- a/boot/vbe_simple_os.c
+++ b/boot/vbe_simple_os.c
@@ -109,4 +109,4 @@ static int bootmeth_vbe_simple_ft_fixup(void *ctx, struct 
event *event)
 
return 0;
 }
-EVENT_SPY(EVT_FT_FIXUP, bootmeth_vbe_simple_ft_fixup);
+EVENT_SPY_FULL(EVT_FT_FIXUP, bootmeth_vbe_simple_ft_fixup);
diff --git a/include/event.h b/include/event.h
index 062b5847897e..b2cfd65c9f8b 100644
--- a/include/event.h
+++ b/include/event.h
@@ -195,7 +195,7 @@ static inline const char *event_spy_id(struct evspy_info 
*spy)
  * away the linker-list entry sometimes, e.g. with the EVT_FT_FIXUP entry in
  * vbe_simple.c - so for now, make it global.
  */
-#define EVENT_SPY(_type, _func) \
+#define EVENT_SPY_FULL(_type, _func) \
__used ll_entry_declare(struct evspy_info, _type ## _3_ ## _func, \
evspy_info) = _ESPY_REC(_type, _func)
 
-- 
2.42.0.rc1.204.g551eb34607-goog



[PATCH 09/14] event: Convert existing spy records to simple

2023-08-21 Thread Simon Glass
Very few of the existing event-spy records use the arguments they are
passed. Update them to use a simple spy instead, to simplify the code.

Where an adaptor function is currently used, remove it where possible.

Signed-off-by: Simon Glass 
---

 arch/arm/mach-imx/imx8/cpu.c  |  4 +--
 arch/arm/mach-imx/imx8m/soc.c |  4 +--
 arch/arm/mach-imx/imx8ulp/soc.c   |  7 +
 arch/arm/mach-imx/imx9/soc.c  |  4 +--
 arch/arm/mach-omap2/am33xx/board.c|  4 +--
 arch/arm/mach-omap2/hwinit-common.c   | 12 +++--
 arch/mips/mach-pic32/cpu.c| 10 ++-
 arch/nios2/cpu/cpu.c  |  4 +--
 arch/riscv/cpu/cpu.c  |  4 +--
 arch/riscv/include/asm/system.h   |  2 +-
 arch/riscv/lib/spl.c  |  2 +-
 arch/sandbox/cpu/start.c  |  7 +
 arch/x86/cpu/baytrail/cpu.c   |  4 +--
 arch/x86/cpu/broadwell/cpu.c  |  4 +--
 arch/x86/cpu/ivybridge/cpu.c  |  4 +--
 arch/x86/cpu/quark/quark.c| 26 +++
 arch/x86/lib/fsp2/fsp_init.c  |  4 +--
 board/google/chromebook_coral/coral.c |  4 +--
 board/keymile/kmcent2/kmcent2.c   |  4 +--
 .../keymile/pg-wcom-ls102xa/pg-wcom-ls102xa.c |  4 +--
 board/starfive/visionfive2/spl.c  |  2 +-
 drivers/cpu/microblaze_cpu.c  |  4 +--
 lib/fwu_updates/fwu.c |  4 +--
 test/py/tests/test_event_dump.py  |  2 +-
 24 files changed, 52 insertions(+), 78 deletions(-)

diff --git a/arch/arm/mach-imx/imx8/cpu.c b/arch/arm/mach-imx/imx8/cpu.c
index c62357044e01..39ac0bc4140b 100644
--- a/arch/arm/mach-imx/imx8/cpu.c
+++ b/arch/arm/mach-imx/imx8/cpu.c
@@ -69,7 +69,7 @@ int arch_cpu_init(void)
return 0;
 }
 
-static int imx8_init_mu(void *ctx, struct event *event)
+static int imx8_init_mu(void)
 {
struct udevice *devp;
int node, ret;
@@ -91,7 +91,7 @@ static int imx8_init_mu(void *ctx, struct event *event)
 
return 0;
 }
-EVENT_SPY(EVT_DM_POST_INIT_F, imx8_init_mu);
+EVENT_SPY_SIMPLE(EVT_DM_POST_INIT_F, imx8_init_mu);
 
 #if defined(CONFIG_ARCH_MISC_INIT)
 int arch_misc_init(void)
diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
index 78b775f449d9..431ad959f533 100644
--- a/arch/arm/mach-imx/imx8m/soc.c
+++ b/arch/arm/mach-imx/imx8m/soc.c
@@ -532,7 +532,7 @@ static void imx_set_wdog_powerdown(bool enable)
writew(enable, >wmcr);
 }
 
-static int imx8m_check_clock(void *ctx, struct event *event)
+static int imx8m_check_clock(void)
 {
struct udevice *dev;
int ret;
@@ -549,7 +549,7 @@ static int imx8m_check_clock(void *ctx, struct event *event)
 
return 0;
 }
-EVENT_SPY(EVT_DM_POST_INIT_F, imx8m_check_clock);
+EVENT_SPY_SIMPLE(EVT_DM_POST_INIT_F, imx8m_check_clock);
 
 static void imx8m_setup_snvs(void)
 {
diff --git a/arch/arm/mach-imx/imx8ulp/soc.c b/arch/arm/mach-imx/imx8ulp/soc.c
index e23cf60d126f..fd436dd88514 100644
--- a/arch/arm/mach-imx/imx8ulp/soc.c
+++ b/arch/arm/mach-imx/imx8ulp/soc.c
@@ -803,12 +803,7 @@ int imx8ulp_dm_post_init(void)
 
return 0;
 }
-
-static int imx8ulp_evt_dm_post_init(void *ctx, struct event *event)
-{
-   return imx8ulp_dm_post_init();
-}
-EVENT_SPY(EVT_DM_POST_INIT_F, imx8ulp_evt_dm_post_init);
+EVENT_SPY_SIMPLE(EVT_DM_POST_INIT_F, imx8ulp_dm_post_init);
 
 #if defined(CONFIG_SPL_BUILD)
 __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
diff --git a/arch/arm/mach-imx/imx9/soc.c b/arch/arm/mach-imx/imx9/soc.c
index f43b73a6c219..5d8687b6f438 100644
--- a/arch/arm/mach-imx/imx9/soc.c
+++ b/arch/arm/mach-imx/imx9/soc.c
@@ -552,7 +552,7 @@ int arch_cpu_init(void)
return 0;
 }
 
-int imx9_probe_mu(void *ctx, struct event *event)
+int imx9_probe_mu(void)
 {
struct udevice *devp;
int node, ret;
@@ -576,7 +576,7 @@ int imx9_probe_mu(void *ctx, struct event *event)
 
return 0;
 }
-EVENT_SPY(EVT_DM_POST_INIT_F, imx9_probe_mu);
+EVENT_SPY_SIMPLE(EVT_DM_POST_INIT_F, imx9_probe_mu);
 
 int timer_init(void)
 {
diff --git a/arch/arm/mach-omap2/am33xx/board.c 
b/arch/arm/mach-omap2/am33xx/board.c
index ecc0a592e993..a6307251c1fc 100644
--- a/arch/arm/mach-omap2/am33xx/board.c
+++ b/arch/arm/mach-omap2/am33xx/board.c
@@ -527,7 +527,7 @@ void board_init_f(ulong dummy)
 
 #endif
 
-static int am33xx_dm_post_init(void *ctx, struct event *event)
+static int am33xx_dm_post_init(void)
 {
hw_data_init();
 #if !CONFIG_IS_ENABLED(SKIP_LOWLEVEL_INIT)
@@ -535,4 +535,4 @@ static int am33xx_dm_post_init(void *ctx, struct event 
*event)
 #endif
return 0;
 }
-EVENT_SPY(EVT_DM_POST_INIT_F, am33xx_dm_post_init);
+EVENT_SPY_SIMPLE(EVT_DM_POST_INIT_F, am33xx_dm_post_init);
diff --git a/arch/arm/mach-omap2/hwinit-common.c 
b/arch/arm/mach-omap2/hwinit-common.c

[PATCH 08/14] initcall: Support manual relocation

2023-08-21 Thread Simon Glass
Move the manual-relocation code to the initcall file. Make sure to avoid
manually relocating event types. Only true function pointers should be
relocated.

Signed-off-by: Simon Glass 
---

 common/board_r.c   |  6 ++
 include/initcall.h |  7 +++
 lib/initcall.c | 10 ++
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/common/board_r.c b/common/board_r.c
index 85860861f0ac..7c1fbc69ed6d 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -813,10 +813,8 @@ void board_init_r(gd_t *new_gd, ulong dest_addr)
 #endif
gd->flags &= ~GD_FLG_LOG_READY;
 
-   if (IS_ENABLED(CONFIG_NEEDS_MANUAL_RELOC)) {
-   for (int i = 0; i < ARRAY_SIZE(init_sequence_r); i++)
-   MANUAL_RELOC(init_sequence_r[i]);
-   }
+   if (IS_ENABLED(CONFIG_NEEDS_MANUAL_RELOC))
+   initcall_manual_reloc(init_sequence_r);
 
if (initcall_run_list(init_sequence_r))
hang();
diff --git a/include/initcall.h b/include/initcall.h
index 62d3bb67f089..208effd8d13e 100644
--- a/include/initcall.h
+++ b/include/initcall.h
@@ -35,4 +35,11 @@ typedef int (*init_fnc_t)(void);
  */
 int initcall_run_list(const init_fnc_t init_sequence[]);
 
+/**
+ * initcall_manual_reloc() - Do manual relocation on an initcall sequence
+ *
+ * @init_sequence: NULL-terminated init sequence to relocate
+ */
+void initcall_manual_reloc(init_fnc_t init_sequence[]);
+
 #endif
diff --git a/lib/initcall.c b/lib/initcall.c
index 33b7d761dc7e..480490ea239d 100644
--- a/lib/initcall.c
+++ b/lib/initcall.c
@@ -97,3 +97,13 @@ int initcall_run_list(const init_fnc_t init_sequence[])
 
return 0;
 }
+
+void initcall_manual_reloc(init_fnc_t init_sequence[])
+{
+   init_fnc_t *ptr;
+
+   for (ptr = init_sequence; *ptr; ptr++) {
+   if (!initcall_is_event(*ptr))
+   MANUAL_RELOC(*ptr);
+   }
+}
-- 
2.42.0.rc1.204.g551eb34607-goog



[PATCH 07/14] initcall: Support emitting events

2023-08-21 Thread Simon Glass
At present the initcall list consists of a list of function pointers. Over
time the initcall lists will likely change to mostly emitting events,
since most of the calls are board- or arch-specific.

As a first step, allow an initcall to be an event type instead of a
function pointer. Add the required macro and update initcall_run_list() to
emit an event in that case, or ignore it if events are not enabled.

The bottom 8 bits of the function pointer are used to hold the event type,
with the rest being all ones. This should avoid any collision, since
initcalls should not be above 0xff00 in memory.

Convert misc_init_f over to use this mechanism.

Add comments to the initcall header file while we are here. Also fix up
the trace test to handle the change.

Signed-off-by: Simon Glass 
---

 common/board_f.c|  7 +-
 include/initcall.h  | 25 +++
 lib/initcall.c  | 50 +
 test/py/tests/test_trace.py | 11 
 4 files changed, 77 insertions(+), 16 deletions(-)

diff --git a/common/board_f.c b/common/board_f.c
index 2f986d9b2890..a485ba62fa17 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -836,11 +836,6 @@ __weak int clear_bss(void)
return 0;
 }
 
-static int misc_init_f(void)
-{
-   return event_notify_null(EVT_MISC_INIT_F);
-}
-
 static const init_fnc_t init_sequence_f[] = {
setup_mon_len,
 #ifdef CONFIG_OF_CONTROL
@@ -899,7 +894,7 @@ static const init_fnc_t init_sequence_f[] = {
show_board_info,
 #endif
INIT_FUNC_WATCHDOG_INIT
-   misc_init_f,
+   INITCALL_EVENT(EVT_MISC_INIT_F),
INIT_FUNC_WATCHDOG_RESET
 #if CONFIG_IS_ENABLED(SYS_I2C_LEGACY)
init_func_i2c,
diff --git a/include/initcall.h b/include/initcall.h
index 01f3f2833f10..62d3bb67f089 100644
--- a/include/initcall.h
+++ b/include/initcall.h
@@ -6,8 +6,33 @@
 #ifndef __INITCALL_H
 #define __INITCALL_H
 
+#include 
+#include 
+
+_Static_assert(EVT_COUNT < 256, "Can only support 256 event types with 8 
bits");
+
+/**
+ * init_fnc_t - Init function
+ *
+ * Return: 0 if OK -ve on error
+ */
 typedef int (*init_fnc_t)(void);
 
+/* Top bit indicates that the initcall is an event */
+#define INITCALL_IS_EVENT  GENMASK(BITS_PER_LONG - 1, 8)
+#define INITCALL_EVENT_TYPEGENMASK(7, 0)
+
+#define INITCALL_EVENT(_type)  (void *)((_type) | INITCALL_IS_EVENT)
+
+/**
+ * initcall_run_list() - Run through a list of function calls
+ *
+ * This calls functions one after the other, stopping at the first error, or
+ * when NULL is obtained.
+ *
+ * @init_sequence: NULL-terminated init sequence to run
+ * Return: 0 if OK, or -ve error code from the first failure
+ */
 int initcall_run_list(const init_fnc_t init_sequence[]);
 
 #endif
diff --git a/lib/initcall.c b/lib/initcall.c
index 0f74cef32f85..33b7d761dc7e 100644
--- a/lib/initcall.c
+++ b/lib/initcall.c
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -25,6 +26,23 @@ static ulong calc_reloc_ofs(void)
 
return 0;
 }
+
+/**
+ * initcall_is_event() - Get the event number for an initcall
+ *
+ * func: Function pointer to check
+ * Return: Event number, if this is an event, else 0
+ */
+static int initcall_is_event(init_fnc_t func)
+{
+   ulong val = (ulong)func;
+
+   if ((val & INITCALL_IS_EVENT) == INITCALL_IS_EVENT)
+   return val & INITCALL_EVENT_TYPE;
+
+   return 0;
+}
+
 /*
  * To enable debugging. add #define DEBUG at the top of the including file.
  *
@@ -34,23 +52,45 @@ int initcall_run_list(const init_fnc_t init_sequence[])
 {
ulong reloc_ofs = calc_reloc_ofs();
const init_fnc_t *ptr;
+   enum event_t type;
init_fnc_t func;
int ret = 0;
 
for (ptr = init_sequence; func = *ptr, !ret && func; ptr++) {
-   if (reloc_ofs) {
+   type = initcall_is_event(func);
+
+   if (type) {
+   if (!CONFIG_IS_ENABLED(EVENT))
+   continue;
+   debug("initcall: event %d/%s\n", type,
+ event_type_name(type));
+   } else if (reloc_ofs) {
debug("initcall: %p (relocated to %p)\n",
- (char *)func - reloc_ofs, func);
+ (char *)func - reloc_ofs, (char *)func);
} else {
debug("initcall: %p\n", (char *)func - reloc_ofs);
}
 
-   ret = func();
+   ret = type ? event_notify_null(type) : func();
}
 
if (ret) {
-   printf("initcall failed at call %p (err=%dE)\n",
-  (char *)func - reloc_ofs, ret);
+   if (CONFIG_IS_ENABLED(EVENT)) {
+   char buf[60];
+
+   /* don't worry about buf size as we are dying here */
+   if (type) {
+   

[PATCH 06/14] event: Export event_type_name()

2023-08-21 Thread Simon Glass
Export this function so it can be used with initcall debugging.

Signed-off-by: Simon Glass 
---

 common/event.c  | 2 +-
 include/event.h | 8 
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/common/event.c b/common/event.c
index 7e2590eb0400..55f6932ef62c 100644
--- a/common/event.c
+++ b/common/event.c
@@ -49,7 +49,7 @@ const char *const type_name[] = {
 _Static_assert(ARRAY_SIZE(type_name) == EVT_COUNT, "event type_name size");
 #endif
 
-static const char *event_type_name(enum event_t type)
+const char *event_type_name(enum event_t type)
 {
 #if CONFIG_IS_ENABLED(EVENT_DEBUG)
return type_name[type];
diff --git a/include/event.h b/include/event.h
index 0e3222c2e243..062b5847897e 100644
--- a/include/event.h
+++ b/include/event.h
@@ -230,6 +230,14 @@ void event_show_spy_list(void);
  */
 int event_manual_reloc(void);
 
+/**
+ * event_type_name() - Get the name of an event type
+ *
+ * @type: Type to check
+ * Return: Name of event, or "(unknown)" if not known
+ */
+const char *event_type_name(enum event_t type);
+
 /**
  * event_notify() - notify spies about an event
  *
-- 
2.42.0.rc1.204.g551eb34607-goog



[PATCH 05/14] initcall: Adjust the failure message and return value

2023-08-21 Thread Simon Glass
Move the failure message outside the loop, so it is easier to follow the
code. Avoid swallowing the error code - just pass it along.

Drop the initcall-list address from the output. This is confusing since
we show two addresses. Really it is only the function address which is
useful, since it can be looked up in the map, e.g. with:

   grep -A1 -B1 serial_init u-boot.map

Signed-off-by: Simon Glass 
---

 lib/initcall.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/lib/initcall.c b/lib/initcall.c
index 81c5d2450735..0f74cef32f85 100644
--- a/lib/initcall.c
+++ b/lib/initcall.c
@@ -46,11 +46,13 @@ int initcall_run_list(const init_fnc_t init_sequence[])
}
 
ret = func();
-   if (ret) {
-   printf("initcall sequence %p failed at call %p 
(err=%d)\n",
-  init_sequence, (char *)func - reloc_ofs, ret);
-   return -1;
-   }
+   }
+
+   if (ret) {
+   printf("initcall failed at call %p (err=%dE)\n",
+  (char *)func - reloc_ofs, ret);
+
+   return ret;
}
 
return 0;
-- 
2.42.0.rc1.204.g551eb34607-goog



[PATCH 04/14] initcall: Adjust the loop logic

2023-08-21 Thread Simon Glass
Use a variable to hold the function, so we don't need to repeat the
pointer access each time. Rename the init pointer to 'ptr' since we only
refer to it in the for() statement now.

Signed-off-by: Simon Glass 
---

 lib/initcall.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/lib/initcall.c b/lib/initcall.c
index 89a68b2444fe..81c5d2450735 100644
--- a/lib/initcall.c
+++ b/lib/initcall.c
@@ -33,25 +33,25 @@ static ulong calc_reloc_ofs(void)
 int initcall_run_list(const init_fnc_t init_sequence[])
 {
ulong reloc_ofs = calc_reloc_ofs();
-   const init_fnc_t *init_fnc_ptr;
+   const init_fnc_t *ptr;
+   init_fnc_t func;
+   int ret = 0;
 
-   for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
-   int ret;
-
-   if (reloc_ofs)
+   for (ptr = init_sequence; func = *ptr, !ret && func; ptr++) {
+   if (reloc_ofs) {
debug("initcall: %p (relocated to %p)\n",
- (char *)*init_fnc_ptr - reloc_ofs,
- (char *)*init_fnc_ptr);
-   else
-   debug("initcall: %p\n", (char *)*init_fnc_ptr - 
reloc_ofs);
+ (char *)func - reloc_ofs, func);
+   } else {
+   debug("initcall: %p\n", (char *)func - reloc_ofs);
+   }
 
-   ret = (*init_fnc_ptr)();
+   ret = func();
if (ret) {
printf("initcall sequence %p failed at call %p 
(err=%d)\n",
-  init_sequence,
-  (char *)*init_fnc_ptr - reloc_ofs, ret);
+  init_sequence, (char *)func - reloc_ofs, ret);
return -1;
}
}
+
return 0;
 }
-- 
2.42.0.rc1.204.g551eb34607-goog



[PATCH 01/14] event: Support a simple spy record

2023-08-21 Thread Simon Glass
The current event spy is always passed the event context and the event.
The context is always NULL for a static spy. The event is not often used.

Introduce a 'simple' spy which takes no arguments. This allows us to drop
the adaptation code that many of these spy records use.

Update the event script to find these in the image.

Signed-off-by: Simon Glass 
---

 common/event.c   |  9 ++-
 include/event.h  | 41 ++--
 scripts/event_dump.py| 12 ++
 test/common/event.c  | 22 +
 test/py/tests/test_event_dump.py |  3 ++-
 5 files changed, 79 insertions(+), 8 deletions(-)

diff --git a/common/event.c b/common/event.c
index 3224e2812224..7e2590eb0400 100644
--- a/common/event.c
+++ b/common/event.c
@@ -71,7 +71,14 @@ static int notify_static(struct event *ev)
 
log_debug("Sending event %x/%s to spy '%s'\n", ev->type,
  event_type_name(ev->type), event_spy_id(spy));
-   ret = spy->func(NULL, ev);
+   if (spy->flags & EVSPYF_SIMPLE) {
+   const struct evspy_info_simple *simple;
+
+   simple = (struct evspy_info_simple *)spy;
+   ret = simple->func();
+   } else {
+   ret = spy->func(NULL, ev);
+   }
 
/*
 * TODO: Handle various return codes to
diff --git a/include/event.h b/include/event.h
index daf44bf8a83b..0e3222c2e243 100644
--- a/include/event.h
+++ b/include/event.h
@@ -99,19 +99,48 @@ struct event {
union event_data data;
 };
 
+/* Flags for event spy */
+enum evspy_flags {
+   EVSPYF_SIMPLE   = 1 << 0,
+};
+
 /** Function type for event handlers */
 typedef int (*event_handler_t)(void *ctx, struct event *event);
 
+/** Function type for simple event handlers */
+typedef int (*event_handler_simple_t)(void);
+
 /**
  * struct evspy_info - information about an event spy
  *
  * @func: Function to call when the event is activated (must be first)
  * @type: Event type
+ * @flag: Flags for this spy
  * @id: Event id string
  */
 struct evspy_info {
event_handler_t func;
-   enum event_t type;
+   u8 type;
+   u8 flags;
+#if CONFIG_IS_ENABLED(EVENT_DEBUG)
+   const char *id;
+#endif
+};
+
+/**
+ * struct evspy_info_simple - information about an event spy
+ *
+ * THis is the 'simple' record, the only difference being the handler function
+ *
+ * @func: Function to call when the event is activated (must be first)
+ * @type: Event type
+ * @flag: Flags for this spy
+ * @id: Event id string
+ */
+struct evspy_info_simple {
+   event_handler_simple_t func;
+   u8 type;
+   u8 flags;
 #if CONFIG_IS_ENABLED(EVENT_DEBUG)
const char *id;
 #endif
@@ -119,9 +148,11 @@ struct evspy_info {
 
 /* Declare a new event spy */
 #if CONFIG_IS_ENABLED(EVENT_DEBUG)
-#define _ESPY_REC(_type, _func)   { _func, _type, #_func, }
+#define _ESPY_REC(_type, _func)   { _func, _type, 0, #_func, }
+#define _ESPY_REC_SIMPLE(_type, _func)  { _func, _type, EVSPYF_SIMPLE, #_func, 
}
 #else
 #define _ESPY_REC(_type, _func)   { _func, _type, }
+#define _ESPY_REC_SIMPLE(_type, _func)  { _func, _type, EVSPYF_SIMPLE }
 #endif
 
 static inline const char *event_spy_id(struct evspy_info *spy)
@@ -168,6 +199,12 @@ static inline const char *event_spy_id(struct evspy_info 
*spy)
__used ll_entry_declare(struct evspy_info, _type ## _3_ ## _func, \
evspy_info) = _ESPY_REC(_type, _func)
 
+/* Simple spy with no function arguemnts */
+#define EVENT_SPY_SIMPLE(_type, _func) \
+   __used ll_entry_declare(struct evspy_info_simple, \
+   _type ## _3_ ## _func, \
+   evspy_info) = _ESPY_REC_SIMPLE(_type, _func)
+
 /**
  * event_register - register a new spy
  *
diff --git a/scripts/event_dump.py b/scripts/event_dump.py
index 0117457526ef..24dfe2bda91f 100755
--- a/scripts/event_dump.py
+++ b/scripts/event_dump.py
@@ -19,8 +19,10 @@ from u_boot_pylib import tools
 
 # A typical symbol looks like this:
 #   _u_boot_list_2_evspy_info_2_EVT_MISC_INIT_F_3_sandbox_misc_init_f
-PREFIX = '_u_boot_list_2_evspy_info_2_'
-RE_EVTYPE = re.compile('%s(.*)_3_.*' % PREFIX)
+PREFIX_FULL = '_u_boot_list_2_evspy_info_2_'
+PREFIX_SIMPLE = '_u_boot_list_2_evspy_info_simple_2_'
+RE_EVTYPE_FULL = re.compile('%s(.*)_3_.*' % PREFIX_FULL)
+RE_EVTYPE_SIMPLE = re.compile('%s(.*)_3_.*' % PREFIX_SIMPLE)
 
 def show_sym(fname, data, endian, evtype, sym):
 """Show information about an evspy entry
@@ -88,12 +90,14 @@ def show_event_spy_list(fname, endian):
 fname (str): Filename of ELF file
 endian (str): Endianness to use ('little', 'big', 'auto')
 """
-syms = elf.GetSymbolFileOffset(fname, [PREFIX])
+syms = elf.GetSymbolFileOffset(fname, [PREFIX_FULL, 

[PATCH 02/14] Revert "initcall: Move to inline function"

2023-08-21 Thread Simon Glass
Somehow I do not see any inlining with initcalls now. I was sure I saw
it when this commit went in, but now it seems to make things worse.

This reverts commit 47870afab92fca6e672c03d0dea802a55e200675.

Signed-off-by: Simon Glass 
---

 common/board_r.c   |  5 -
 include/initcall.h | 46 +---
 lib/Makefile   |  1 +
 lib/initcall.c | 52 ++
 4 files changed, 58 insertions(+), 46 deletions(-)
 create mode 100644 lib/initcall.c

diff --git a/common/board_r.c b/common/board_r.c
index 598155c7753d..85860861f0ac 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -583,7 +583,10 @@ static int run_main_loop(void)
 }
 
 /*
- * We hope to remove most of the driver-related init and do it if/when
+ * Over time we hope to remove these functions with code fragments and
+ * stub functions, and instead call the relevant function directly.
+ *
+ * We also hope to remove most of the driver-related init and do it if/when
  * the driver is later used.
  *
  * TODO: perhaps reset the watchdog in the initcall function after each call?
diff --git a/include/initcall.h b/include/initcall.h
index 69ce26807051..01f3f2833f10 100644
--- a/include/initcall.h
+++ b/include/initcall.h
@@ -8,50 +8,6 @@
 
 typedef int (*init_fnc_t)(void);
 
-#include 
-#ifdef CONFIG_EFI_APP
-#include 
-#endif
-#include 
-
-/*
- * To enable debugging. add #define DEBUG at the top of the including file.
- *
- * To find a symbol, use grep on u-boot.map
- */
-static inline int initcall_run_list(const init_fnc_t init_sequence[])
-{
-   const init_fnc_t *init_fnc_ptr;
-
-   for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
-   unsigned long reloc_ofs = 0;
-   int ret;
-
-   /*
-* Sandbox is relocated by the OS, so symbols always appear at
-* the relocated address.
-*/
-   if (IS_ENABLED(CONFIG_SANDBOX) || (gd->flags & GD_FLG_RELOC))
-   reloc_ofs = gd->reloc_off;
-#ifdef CONFIG_EFI_APP
-   reloc_ofs = (unsigned long)image_base;
-#endif
-   if (reloc_ofs)
-   debug("initcall: %p (relocated to %p)\n",
-   (char *)*init_fnc_ptr - reloc_ofs,
-   (char *)*init_fnc_ptr);
-   else
-   debug("initcall: %p\n", (char *)*init_fnc_ptr - 
reloc_ofs);
-
-   ret = (*init_fnc_ptr)();
-   if (ret) {
-   printf("initcall sequence %p failed at call %p 
(err=%d)\n",
-  init_sequence,
-  (char *)*init_fnc_ptr - reloc_ofs, ret);
-   return -1;
-   }
-   }
-   return 0;
-}
+int initcall_run_list(const init_fnc_t init_sequence[]);
 
 #endif
diff --git a/lib/Makefile b/lib/Makefile
index 8d8ccc8bbc39..839872d804b5 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_GZIP_COMPRESSED) += gzip.o
 obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += smbios.o
 obj-$(CONFIG_SMBIOS_PARSER) += smbios-parser.o
 obj-$(CONFIG_IMAGE_SPARSE) += image-sparse.o
+obj-y += initcall.o
 obj-y += ldiv.o
 obj-$(CONFIG_XXHASH) += xxhash.o
 obj-y += net_utils.o
diff --git a/lib/initcall.c b/lib/initcall.c
new file mode 100644
index ..eedb0fbf1d5c
--- /dev/null
+++ b/lib/initcall.c
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2013 The Chromium OS Authors.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/*
+ * To enable debugging. add #define DEBUG at the top of the including file.
+ *
+ * To find a symbol, use grep on u-boot.map
+ */
+int initcall_run_list(const init_fnc_t init_sequence[])
+{
+   const init_fnc_t *init_fnc_ptr;
+
+   for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
+   unsigned long reloc_ofs = 0;
+   int ret;
+
+   /*
+* Sandbox is relocated by the OS, so symbols always appear at
+* the relocated address.
+*/
+   if (IS_ENABLED(CONFIG_SANDBOX) || (gd->flags & GD_FLG_RELOC))
+   reloc_ofs = gd->reloc_off;
+#ifdef CONFIG_EFI_APP
+   reloc_ofs = (unsigned long)image_base;
+#endif
+   if (reloc_ofs)
+   debug("initcall: %p (relocated to %p)\n",
+ (char *)*init_fnc_ptr - reloc_ofs,
+ (char *)*init_fnc_ptr);
+   else
+   debug("initcall: %p\n", (char *)*init_fnc_ptr - 
reloc_ofs);
+
+   ret = (*init_fnc_ptr)();
+   if (ret) {
+   printf("initcall sequence %p failed at call %p 
(err=%d)\n",
+  init_sequence,
+  (char 

[PATCH 03/14] initcall: Factor out reloc_off calculation

2023-08-21 Thread Simon Glass
Move this into a function and do it once, not each time around the loop.

Signed-off-by: Simon Glass 
---

 lib/initcall.c | 25 +++--
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/lib/initcall.c b/lib/initcall.c
index eedb0fbf1d5c..89a68b2444fe 100644
--- a/lib/initcall.c
+++ b/lib/initcall.c
@@ -11,6 +11,20 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+static ulong calc_reloc_ofs(void)
+{
+#ifdef CONFIG_EFI_APP
+   return (ulong)image_base;
+#endif
+   /*
+* Sandbox is relocated by the OS, so symbols always appear at
+* the relocated address.
+*/
+   if (IS_ENABLED(CONFIG_SANDBOX) || (gd->flags & GD_FLG_RELOC))
+   return gd->reloc_off;
+
+   return 0;
+}
 /*
  * To enable debugging. add #define DEBUG at the top of the including file.
  *
@@ -18,21 +32,12 @@ DECLARE_GLOBAL_DATA_PTR;
  */
 int initcall_run_list(const init_fnc_t init_sequence[])
 {
+   ulong reloc_ofs = calc_reloc_ofs();
const init_fnc_t *init_fnc_ptr;
 
for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
-   unsigned long reloc_ofs = 0;
int ret;
 
-   /*
-* Sandbox is relocated by the OS, so symbols always appear at
-* the relocated address.
-*/
-   if (IS_ENABLED(CONFIG_SANDBOX) || (gd->flags & GD_FLG_RELOC))
-   reloc_ofs = gd->reloc_off;
-#ifdef CONFIG_EFI_APP
-   reloc_ofs = (unsigned long)image_base;
-#endif
if (reloc_ofs)
debug("initcall: %p (relocated to %p)\n",
  (char *)*init_fnc_ptr - reloc_ofs,
-- 
2.42.0.rc1.204.g551eb34607-goog



[PATCH 00/14] event: Replace some more init hooks

2023-08-21 Thread Simon Glass


This series replaces some more of the init hooks in board_f.c and
board_r.c with events. Notably it converts last_state_init() over.

It also provides a 'simple' event spy, which takes no arguments. It turns
out that this is quite a common case, so it is worth optimising for this,
to reduce code size, before events become too commonly used.

Finally, it introduces a way of emitting an event in an initcall, instead
of calling a function. This is likely to be used at least as often as the
functions, as we convert more of these initcalls.

As part of this, the initcall code is brought back into a C file. Somehow
the compiler has changed or something else, so that this does not confer
any benefits now.

For boards with EVENT enabled, this unfortunately results in small growth,
e.g. for firefly:

   aarch64: (for 1/1 boards) all +114.0 data +16.0 rodata +22.0 text +76.0
   arm: (for 1/1 boards) all +82.0 rodata +18.0 text +64.0

For boards without EVENT enabled the growth is smaller, e.g. nokia_rx51:

   arm: (for 1/1 boards) all +32.0 data +8.0 rodata -8.0 text +32.0

I cannot find a good way to avoid the latter, other than macro magic with
an embedded comma (to completely remove an event entry), which seems
nasty.


Simon Glass (14):
  event: Support a simple spy record
  Revert "initcall: Move to inline function"
  initcall: Factor out reloc_off calculation
  initcall: Adjust the loop logic
  initcall: Adjust the failure message and return value
  event: Export event_type_name()
  initcall: Support emitting events
  initcall: Support manual relocation
  event: Convert existing spy records to simple
  event: Rename EVENT_SPY to EVENT_SPY_FULL
  event: Update documentation for simple spy
  x86: Convert arch_fsp_init() to use events
  freescale: Drop call to init_func_vid() in the init sequence
  event: Use an event to replace last_stage_init()

 arch/Kconfig  |   1 +
 arch/arm/cpu/armv8/fsl-layerscape/spl.c   |   5 +
 arch/arm/mach-imx/imx8/cpu.c  |   4 +-
 arch/arm/mach-imx/imx8m/soc.c |   4 +-
 arch/arm/mach-imx/imx8ulp/soc.c   |   7 +-
 arch/arm/mach-imx/imx9/soc.c  |   4 +-
 arch/arm/mach-omap2/am33xx/board.c|   4 +-
 arch/arm/mach-omap2/hwinit-common.c   |  12 +-
 arch/mips/mach-mtmips/cpu.c   |   6 +-
 arch/mips/mach-pic32/cpu.c|  12 +-
 arch/nios2/cpu/cpu.c  |   4 +-
 arch/riscv/cpu/cpu.c  |   4 +-
 arch/riscv/include/asm/system.h   |   2 +-
 arch/riscv/lib/spl.c  |   2 +-
 arch/sandbox/cpu/start.c  |   7 +-
 arch/x86/cpu/baytrail/cpu.c   |   4 +-
 arch/x86/cpu/broadwell/cpu.c  |   4 +-
 arch/x86/cpu/coreboot/coreboot.c  |   7 +-
 arch/x86/cpu/cpu.c|  10 +-
 arch/x86/cpu/efi/payload.c|   4 +-
 arch/x86/cpu/ivybridge/cpu.c  |   4 +-
 arch/x86/cpu/quark/quark.c|  30 +++--
 arch/x86/lib/fsp1/fsp_common.c|   1 +
 arch/x86/lib/fsp2/fsp_common.c|   5 -
 arch/x86/lib/fsp2/fsp_init.c  |   4 +-
 board/CZ.NIC/turris_mox/turris_mox.c  |   4 +-
 board/Marvell/mvebu_armada-37xx/board.c   |   5 +-
 board/Marvell/octeon_nic23/board.c|   4 +-
 board/Marvell/octeontx2/board.c   |   4 +-
 board/cortina/presidio-asic/presidio.c|   4 +-
 board/emulation/qemu-ppce500/qemu-ppce500.c   |   4 +-
 board/freescale/ls1088a/ls1088a.c |   3 +-
 board/freescale/lx2160a/lx2160a.c |   2 +
 board/gdsys/a38x/controlcenterdc.c|  31 +++--
 board/gdsys/mpc8308/gazerbeam.c   |   4 +-
 board/ge/bx50v3/bx50v3.c  |   4 +-
 board/google/chromebook_coral/coral.c |   4 +-
 board/keymile/km83xx/km83xx.c |   4 +-
 board/keymile/kmcent2/kmcent2.c   |   7 +-
 .../keymile/pg-wcom-ls102xa/pg-wcom-ls102xa.c |  10 +-
 board/phytium/durian/durian.c |   4 +-
 board/phytium/pomelo/pomelo.c |   4 +-
 board/starfive/visionfive2/spl.c  |   2 +-
 boot/vbe_request.c|   2 +-
 boot/vbe_simple_os.c  |   2 +-
 common/Kconfig|   1 +
 common/board_f.c  |  21 +---
 common/board_r.c  |  20 ++--
 common/event.c|  13 ++-
 configs/bayleybay_defconfig   |   1 -
 configs/cherryhill_defconfig  |   1 -
 configs/chromebook_coral_defconfig|   1 -
 configs/chromebook_link64_defconfig   |   1 -
 configs/chromebook_link_defconfig |   1 -
 configs/chromebook_samus_defconfig|   1 -
 

[PATCH] cmd: dm: allow for selecting uclass and device

2023-08-21 Thread AKASHI Takahiro
The output from "dm tree" or "dm uclass" is a bit annoying
if the number of devices available on the system is huge.
(This is especially true on sandbox when I debug some DM code.)

With this patch, we can specify the uclass or the device
that we are interested in in order to limit the output.

For instance,

=> dm uclass usb
uclass 121: usb
0 usb@1 @ 0bd008b0, seq 1

=> dm tree usb:usb@1
 Class Index  Probed  DriverName
---
 usb   0  [   ]   usb_sandbox   usb@1
 usb_hub   0  [   ]   usb_hub   `-- hub
 usb_emul  0  [   ]   usb_sandbox_hub   `-- hub-emul
 usb_emul  1  [   ]   usb_sandbox_flash |-- flash-stick@0
 usb_emul  2  [   ]   usb_sandbox_flash |-- flash-stick@1
 usb_emul  3  [   ]   usb_sandbox_flash |-- flash-stick@2
 usb_emul  4  [   ]   usb_sandbox_keyb  `-- keyb@3

Please note that, at "dm tree", the uclass name (usb) as well as
the device name (usb@1) is required.

Signed-off-by: AKASHI Takahiro 
---
 cmd/dm.c| 30 
 drivers/core/dump.c | 48 +++--
 include/dm/util.h   | 13 
 3 files changed, 73 insertions(+), 18 deletions(-)

diff --git a/cmd/dm.c b/cmd/dm.c
index 3263547cbec6..99268df2f87a 100644
--- a/cmd/dm.c
+++ b/cmd/dm.c
@@ -59,11 +59,20 @@ static int do_dm_dump_static_driver_info(struct cmd_tbl 
*cmdtp, int flag,
 static int do_dm_dump_tree(struct cmd_tbl *cmdtp, int flag, int argc,
   char *const argv[])
 {
-   bool sort;
+   bool sort = false;
+   char *device = NULL;
 
-   sort = argc > 1 && !strcmp(argv[1], "-s");
+   if (argc > 1) {
+   if (!strcmp(argv[1], "-s")) {
+   sort = true;
+   argc--;
+   argv++;
+   }
+   if (argc > 1)
+   device = argv[1];
+   }
 
-   dm_dump_tree(sort);
+   dm_dump_tree(device, sort);
 
return 0;
 }
@@ -71,7 +80,12 @@ static int do_dm_dump_tree(struct cmd_tbl *cmdtp, int flag, 
int argc,
 static int do_dm_dump_uclass(struct cmd_tbl *cmdtp, int flag, int argc,
 char *const argv[])
 {
-   dm_dump_uclass();
+   char *uclass = NULL;
+
+   if (argc > 1)
+   uclass = argv[1];
+
+   dm_dump_uclass(uclass);
 
return 0;
 }
@@ -91,8 +105,8 @@ static char dm_help_text[] =
"dm drivers   Dump list of drivers with uclass and instances\n"
DM_MEM_HELP
"dm staticDump list of drivers with static platform data\n"
-   "dm tree [-s] Dump tree of driver model devices (-s=sort)\n"
-   "dm uclassDump list of instances for each uclass"
+   "dm tree [-s][name] Dump tree of driver model devices (-s=sort)\n"
+   "dm uclass [name] Dump list of instances for each uclass"
;
 #endif
 
@@ -102,5 +116,5 @@ U_BOOT_CMD_WITH_SUBCMDS(dm, "Driver model low level 
access", dm_help_text,
U_BOOT_SUBCMD_MKENT(drivers, 1, 1, do_dm_dump_drivers),
DM_MEM
U_BOOT_SUBCMD_MKENT(static, 1, 1, do_dm_dump_static_driver_info),
-   U_BOOT_SUBCMD_MKENT(tree, 2, 1, do_dm_dump_tree),
-   U_BOOT_SUBCMD_MKENT(uclass, 1, 1, do_dm_dump_uclass));
+   U_BOOT_SUBCMD_MKENT(tree, 3, 1, do_dm_dump_tree),
+   U_BOOT_SUBCMD_MKENT(uclass, 2, 1, do_dm_dump_uclass));
diff --git a/drivers/core/dump.c b/drivers/core/dump.c
index 3e77832a3a00..855d6ca002b7 100644
--- a/drivers/core/dump.c
+++ b/drivers/core/dump.c
@@ -85,11 +85,35 @@ static void show_devices(struct udevice *dev, int depth, 
int last_flag,
}
 }
 
-void dm_dump_tree(bool sort)
+void dm_dump_tree(char *uclass, bool sort)
 {
struct udevice *root;
 
-   root = dm_root();
+   if (uclass) {
+   char *device;
+   enum uclass_id id;
+
+   device = strchr(uclass, ':');
+   if (!device) {
+   printf("name %s: invalid format\n", uclass);
+   return;
+   }
+   *device = '\0';
+   device++;
+
+   id = uclass_get_by_name(uclass);
+   if (id == UCLASS_INVALID) {
+   printf("uclass %s: not found\n", uclass);
+   return;
+   }
+   if (uclass_find_device_by_name(id, device, )) {
+   printf("device %s:%s: not found\n", uclass, device);
+   return;
+   }
+   } else {
+   root = dm_root();
+   }
+
if (root) {
int dev_count, uclasses;
struct udevice **devs = NULL;
@@ -127,13 +151,25 @@ static void dm_display_line(struct udevice *dev, int 
index)
puts("\n");
 }
 
-void dm_dump_uclass(void)
+void 

Re: Trying to boot custom kernel on Wink Hub (i.MX28)

2023-08-21 Thread Fabio Estevam
Hi Rogan,

On Fri, Aug 18, 2023 at 10:45 AM Rogan Dawes  wrote:
>
> Circling back to this, I wanted to get a modern u-boot running on the
> Wink Hub v1 as well. Since it is possible to invoke SDP over USB
> (having soldered on a suitable microUSB connector), I have no concern
> about bricking it now. It's also a LOT easier than JTAG!
>
> I tried to build and flash the mx28evk_defconfig, but failed to get
> any output on the serial console. This is likely to be unsurprising,
> due to the already established alternative configuration of the UART
> identified by Fabio.
>
> I then patched imx28.dtsi to add duart_pins_c as per Fabio's previous
> patch for the Linux kernel:
>
> + duart_pins_c: duart@2 {
> + reg = <2>;
> + fsl,pinmux-ids = <
> + MX28_PAD_I2C0_SCL__DUART_RX
> + MX28_PAD_I2C0_SDA__DUART_TX
> + >;
> + fsl,drive-strength = ;
> + fsl,voltage = ;
> + fsl,pull-up = ;
> + };
> +
>
> And updated mx28evk_defconfig to use duart_pins_c instead of
> duart_pins_a. However, I still got no output after using:
>
> sudo $(which snagrecover) --soc imx28 -f
> src/snagrecover/templates/imx28-evk.yaml

I am not familiar with this tool. I was only aware of the one below:

https://source.denx.de/denx/mxsldr

Are you able to load the original vendor U-Boot with it?

Also, is the original vendor U-Boot source code available?

It is important to re-use the same DDR initialization from the original U-Boot.

> with u-boot.sb in the current directory.
>
> I also tried using the complete imx28-wink-hub-v1.dts as created by
> Fabio, and updated the .config to reference that instead of the
> imx28-evk, but I suspect that I may not have made the updates
> sufficiently well, if the name of the board needs to correspond with
> any details of the device tree? Neither of these resulted in any
> output.
>
> Any other suggestions?

Please check board/freescale/mx28evk/iomux.c:

const iomux_cfg_t iomux_setup[] = {
/* DUART */
MX28_PAD_PWM0__DUART_RX,
MX28_PAD_PWM1__DUART_TX,

The DUART pins need to be changed here as well.


[PATCH] ARM: renesas: Enable UFS on R8A779F0 S4 Spider

2023-08-21 Thread Marek Vasut
Enable UFS controller driver and matching UFS and SCSI commands. The
former is used to initialize the device, the later is used to perform
low level access to the SCSI interface of the UFS device.

Enable R8A779F0 S4 Spider specific dependencies, the PCA953x driver
and GPIO clock gate driver. This setup is used to toggle 38.4 MHz
clock for the UFS controller.

Enable support for 48bit LBA in block layer to address disks larger
than 512*2^32 ~= 144 PiB. Enable use of 64bit LBA variables in the
rest of U-Boot, instead of the default 32bit ones.

Increase FAT cluster size to 128k as that is what is used by the
filesystem that is populated on the UFS device.

Use 'ufs init && scsi scan' to start the UFS device from U-Boot prompt.

Signed-off-by: Marek Vasut 
---
Cc: Faiz Abbas 
---
 configs/r8a779f0_spider_defconfig | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/configs/r8a779f0_spider_defconfig 
b/configs/r8a779f0_spider_defconfig
index 0edacc4d569..f250d1a13c1 100644
--- a/configs/r8a779f0_spider_defconfig
+++ b/configs/r8a779f0_spider_defconfig
@@ -31,7 +31,9 @@ CONFIG_CMD_BOOTZ=y
 CONFIG_CMD_GPIO=y
 CONFIG_CMD_I2C=y
 CONFIG_CMD_MMC=y
+CONFIG_CMD_PART=y
 CONFIG_CMD_SPI=y
+CONFIG_CMD_UFS=y
 CONFIG_CMD_DHCP=y
 CONFIG_CMD_MII=y
 CONFIG_CMD_PING=y
@@ -46,9 +48,13 @@ CONFIG_SYS_RELOC_GD_ENV_ADDR=y
 CONFIG_VERSION_VARIABLE=y
 CONFIG_REGMAP=y
 CONFIG_SYSCON=y
+CONFIG_LBA48=y
+CONFIG_SYS_64BIT_LBA=y
 CONFIG_CLK=y
+CONFIG_CLK_GPIO=y
 CONFIG_CLK_RENESAS=y
 CONFIG_RCAR_GPIO=y
+CONFIG_DM_PCA953X=y
 CONFIG_DM_I2C=y
 CONFIG_SYS_I2C_RCAR_I2C=y
 CONFIG_MMC_IO_VOLTAGE=y
@@ -67,8 +73,13 @@ CONFIG_PHY_R8A779F0_ETHERNET_SERDES=y
 CONFIG_DM_REGULATOR=y
 CONFIG_DM_REGULATOR_FIXED=y
 CONFIG_DM_REGULATOR_GPIO=y
+CONFIG_SCSI=y
+CONFIG_DM_SCSI=y
 CONFIG_BAUDRATE=1843200
 CONFIG_SCIF_CONSOLE=y
 CONFIG_SPI=y
 CONFIG_DM_SPI=y
 CONFIG_RENESAS_RPC_SPI=y
+CONFIG_UFS=y
+CONFIG_UFS_RENESAS=y
+CONFIG_FS_FAT_MAX_CLUSTSIZE=131072
-- 
2.40.1



[PATCH] ufs: ufs-renesas: Add support for Renesas R-Car UFS controller

2023-08-21 Thread Marek Vasut
Add support for Renesas R-Car UFS controller which needs vendor-specific
initialization.

Ported from Linux kernel as of commit
c2ab666072bc ("scsi: ufs: Explicitly include correct DT includes")

Signed-off-by: Marek Vasut 
---
Cc: Faiz Abbas 
---
 drivers/ufs/Kconfig   |   9 +
 drivers/ufs/Makefile  |   1 +
 drivers/ufs/ufs-renesas.c | 413 ++
 3 files changed, 423 insertions(+)
 create mode 100644 drivers/ufs/ufs-renesas.c

diff --git a/drivers/ufs/Kconfig b/drivers/ufs/Kconfig
index 69ea18edf8d..0e0cc58e3d6 100644
--- a/drivers/ufs/Kconfig
+++ b/drivers/ufs/Kconfig
@@ -21,4 +21,13 @@ config TI_J721E_UFS
  This selects the glue layer driver for Cadence controller
  present on TI's J721E devices.
 
+config UFS_RENESAS
+   bool "Renesas specific hooks to UFS controller platform driver"
+   depends on UFS
+   select BOUNCE_BUFFER
+   help
+ This selects the Renesas specific additions to UFSHCD platform driver.
+ UFS host on Renesas needs some vendor specific configuration before
+ accessing the hardware.
+
 endmenu
diff --git a/drivers/ufs/Makefile b/drivers/ufs/Makefile
index 62ed0166084..4f3344fd4e4 100644
--- a/drivers/ufs/Makefile
+++ b/drivers/ufs/Makefile
@@ -6,3 +6,4 @@
 obj-$(CONFIG_UFS) += ufs.o ufs-uclass.o
 obj-$(CONFIG_CADENCE_UFS) += cdns-platform.o
 obj-$(CONFIG_TI_J721E_UFS) += ti-j721e-ufs.o
+obj-$(CONFIG_UFS_RENESAS) += ufs-renesas.o
diff --git a/drivers/ufs/ufs-renesas.c b/drivers/ufs/ufs-renesas.c
new file mode 100644
index 000..f6086050cde
--- /dev/null
+++ b/drivers/ufs/ufs-renesas.c
@@ -0,0 +1,413 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Renesas UFS host controller driver
+ *
+ * Copyright (C) 2022 Renesas Electronics Corporation
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ufs.h"
+
+struct ufs_renesas_priv {
+   struct clk_bulk clks;
+   bool initialized;   /* The hardware needs initialization once */
+};
+
+enum {
+   SET_PHY_INDEX_LO = 0,
+   SET_PHY_INDEX_HI,
+   TIMER_INDEX,
+   MAX_INDEX
+};
+
+enum ufs_renesas_init_param_mode {
+   MODE_RESTORE,
+   MODE_SET,
+   MODE_SAVE,
+   MODE_POLL,
+   MODE_WAIT,
+   MODE_WRITE,
+};
+
+#define PARAM_RESTORE(_reg, _index) \
+   { .mode = MODE_RESTORE, .reg = _reg, .index = _index }
+#define PARAM_SET(_index, _set) \
+   { .mode = MODE_SET, .index = _index, .u.set = _set }
+#define PARAM_SAVE(_reg, _mask, _index) \
+   { .mode = MODE_SAVE, .reg = _reg, .mask = (u32)(_mask), \
+ .index = _index }
+#define PARAM_POLL(_reg, _expected, _mask) \
+   { .mode = MODE_POLL, .reg = _reg, .u.expected = _expected, \
+ .mask = (u32)(_mask) }
+#define PARAM_WAIT(_delay_us) \
+   { .mode = MODE_WAIT, .u.delay_us = _delay_us }
+
+#define PARAM_WRITE(_reg, _val) \
+   { .mode = MODE_WRITE, .reg = _reg, .u.val = _val }
+
+#define PARAM_WRITE_D0_D4(_d0, _d4) \
+   PARAM_WRITE(0xd0, _d0), PARAM_WRITE(0xd4, _d4)
+
+#define PARAM_WRITE_800_80C_POLL(_addr, _data_800) \
+   PARAM_WRITE_D0_D4(0x080c, 0x0100),  \
+   PARAM_WRITE_D0_D4(0x0800, ((_data_800) << 16) | BIT(8) | 
(_addr)), \
+   PARAM_WRITE(0xd0, 0x080c),  \
+   PARAM_POLL(0xd4, BIT(8), BIT(8))
+
+#define PARAM_RESTORE_800_80C_POLL(_index) \
+   PARAM_WRITE_D0_D4(0x080c, 0x0100),  \
+   PARAM_WRITE(0xd0, 0x0800),  \
+   PARAM_RESTORE(0xd4, (_index)),  \
+   PARAM_WRITE(0xd0, 0x080c),  \
+   PARAM_POLL(0xd4, BIT(8), BIT(8))
+
+#define PARAM_WRITE_804_80C_POLL(_addr, _data_804) \
+   PARAM_WRITE_D0_D4(0x080c, 0x0100),  \
+   PARAM_WRITE_D0_D4(0x0804, ((_data_804) << 16) | BIT(8) | 
(_addr)), \
+   PARAM_WRITE(0xd0, 0x080c),  \
+   PARAM_POLL(0xd4, BIT(8), BIT(8))
+
+#define PARAM_WRITE_828_82C_POLL(_data_828)\
+   PARAM_WRITE_D0_D4(0x082c, 0x0f00),  \
+   PARAM_WRITE_D0_D4(0x0828, _data_828),   \
+   PARAM_WRITE(0xd0, 0x082c),  \
+   PARAM_POLL(0xd4, _data_828, _data_828)
+
+#define PARAM_WRITE_PHY(_addr16, _data16)  \
+   PARAM_WRITE(0xf0, 1),   \
+   PARAM_WRITE_800_80C_POLL(0x16, (_addr16) & 0xff), \
+   PARAM_WRITE_800_80C_POLL(0x17, ((_addr16) >> 8) & 0xff), \
+   PARAM_WRITE_800_80C_POLL(0x18, (_data16) & 0xff), \
+   PARAM_WRITE_800_80C_POLL(0x19, ((_data16) >> 8) & 0xff), \
+ 

Re: [PATCH v2 1/1] usb: host: tegra: implement dts based xcvr setup

2023-08-21 Thread Marek Vasut

On 8/21/23 16:06, Svyatoslav Ryhel wrote:

21 серпня 2023 р. 16:41:51 GMT+03:00, Marek Vasut  написав(-ла):

On 8/21/23 13:19, Svyatoslav Ryhel wrote:

Some devices (like ASUS TF201) may not use fuse based xcvr setup
which will result in not working USB line. To fix this situation
allow xcvr setup to be performed from device tree values if
nvidia,xcvr-setup-use-fuses is not defined.

Signed-off-by: Svyatoslav Ryhel 
---
   drivers/usb/host/ehci-tegra.c | 48 +++
   1 file changed, 43 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index 2cf1625670..a9aee908f3 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -81,6 +81,8 @@ struct fdt_usb {
enum periph_id periph_id;/* peripheral id */
struct gpio_desc vbus_gpio; /* GPIO for vbus enable */
struct gpio_desc phy_reset_gpio; /* GPIO to reset ULPI phy */
+   bool xcvr_setup_use_fuses; /* Indicates that the value is read from the 
on-chip fuses */
+   u32 xcvr_setup; /* Input of XCVR cell, HS driver output control */
   };
 /*
@@ -464,10 +466,16 @@ static int init_utmi_usb_controller(struct fdt_usb 
*config,
/* Recommended PHY settings for EYE diagram */
val = readl(>utmip_xcvr_cfg0);
-   clrsetbits_le32(, UTMIP_XCVR_SETUP_MASK,
-   0x4 << UTMIP_XCVR_SETUP_SHIFT);
-   clrsetbits_le32(, UTMIP_XCVR_SETUP_MSB_MASK,
-   0x3 << UTMIP_XCVR_SETUP_MSB_SHIFT);
+
+   if (!config->xcvr_setup_use_fuses) {
+   clrsetbits_le32(, UTMIP_XCVR_SETUP_MASK,
+   config->xcvr_setup <<
+   UTMIP_XCVR_SETUP_SHIFT);
+   clrsetbits_le32(, UTMIP_XCVR_SETUP_MSB_MASK,
+   config->xcvr_setup <<
+   UTMIP_XCVR_SETUP_MSB_SHIFT);
+   }
+
clrsetbits_le32(, UTMIP_XCVR_HSSLEW_MSB_MASK,
0x8 << UTMIP_XCVR_HSSLEW_MSB_SHIFT);
writel(val, >utmip_xcvr_cfg0);
@@ -522,7 +530,9 @@ static int init_utmi_usb_controller(struct fdt_usb *config,
setbits_le32(>utmip_bat_chrg_cfg0, UTMIP_PD_CHRG);
clrbits_le32(>utmip_xcvr_cfg0, UTMIP_XCVR_LSBIAS_SE);
-   setbits_le32(>utmip_spare_cfg0, FUSE_SETUP_SEL);
+
+   if (config->xcvr_setup_use_fuses)
+   setbits_le32(>utmip_spare_cfg0, FUSE_SETUP_SEL);
/*
 * Configure the UTMIP_IDLE_WAIT and UTMIP_ELASTIC_LIMIT
@@ -843,6 +853,8 @@ static const struct ehci_ops tegra_ehci_ops = {
   static int ehci_usb_of_to_plat(struct udevice *dev)
   {
struct fdt_usb *priv = dev_get_priv(dev);
+   u32 usb_phy_phandle;
+   ofnode usb_phy_node;
int ret;
ret = fdt_decode_usb(dev, priv);
@@ -851,6 +863,32 @@ static int ehci_usb_of_to_plat(struct udevice *dev)
priv->type = dev_get_driver_data(dev);
   +ret = ofnode_read_u32(dev_ofnode(dev), "nvidia,phy", _phy_phandle);
+   if (ret) {
+   log_debug("%s: required usb phy node isn't provided\n", 
__func__);
+   priv->xcvr_setup_use_fuses = true;
+   return 0;
+   }
+
+   usb_phy_node = ofnode_get_by_phandle(usb_phy_phandle);
+   if (!ofnode_valid(usb_phy_node)) {
+   log_err("%s: failed to find usb phy node\n", __func__);
+   return -EINVAL;
+   }


Can we get a simple PHY driver instead of this hackage please ?

See the generic PHY stuff in other ehci drivers for example of how to interact 
with PHY drivers, and drivers/phy/ for PHY driver examples.


No we can't. As I said, I will not start a split of tegra-echi to chipidea 
controller and phy because of this small tweak. It seems that you don't 
understand what your are requesting. Sorry for bothering.


Since the i.MX EHCI controller is also ChipIdea HDRC and already uses a 
PHY driver, I think the split is perfectly doable.


Re: [PATCH v5 00/13] Add video damage tracking

2023-08-21 Thread Simon Glass
Hi Alex,

On Mon, 21 Aug 2023 at 16:40, Alexander Graf  wrote:
>
>
> On 22.08.23 00:10, Simon Glass wrote:
> > Hi Alex,
> >
> > On Mon, 21 Aug 2023 at 14:20, Alexander Graf  wrote:
> >>
> >> On 21.08.23 21:57, Simon Glass wrote:
> >>> Hi Alex,
> >>>
> >>> On Mon, 21 Aug 2023 at 13:33, Alexander Graf  wrote:
>  On 21.08.23 21:11, Simon Glass wrote:
> > Hi Alper,
> >
> > On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak 
> >  wrote:
> >> This is a rebase of Alexander Graf's video damage tracking series, with
> >> some tests and other changes. The original cover letter is as follows:
> >>
> >>> This patch set speeds up graphics output on ARM by a factor of 60x.
> >>>
> >>> On most ARM SBCs, we keep the frame buffer in DRAM and map it as 
> >>> cached,
> >>> but need it accessible by the display controller which reads directly
> >>> from a later point of consistency. Hence, we flush the frame buffer to
> >>> DRAM on every change. The full frame buffer.
> > It should not, see below.
> >
> >>> Unfortunately, with the advent of 4k displays, we are seeing frame 
> >>> buffers
> >>> that can take a while to flush out. This was reported by Da Xue with 
> >>> grub,
> >>> which happily print 1000s of spaces on the screen to draw a menu. 
> >>> Every
> >>> printed space triggers a cache flush.
> > That is a bug somewhere in EFI.
>  Unfortunately not :). You may call it a bug in grub: It literally prints
>  over space characters for every character in its menu that it wants
>  cleared. On every text screen draw.
> 
>  This wouldn't be a big issue if we only flush the reactangle that gets
>  modified. But without this patch set, we're flushing the full DRAM
>  buffer on every u-boot text console character write, which means for
>  every character (as that's the only API UEFI has).
> 
>  As a nice side effect, we speed up the normal U-Boot text console as
>  well with this patch set, because even "normal" text prints that write
>  for example a single line of text on the screen today flush the full
>  frame buffer to DRAM.
> >>> No, I mean that it is a bug that U-Boot (apparently) flushes the cache
> >>> after every character. It doesn't do that for normal character output
> >>> and I don't think it makes sense to do it for EFI either.
> >>
> >> I see. Let's trace the calls:
> >>
> >> efi_cout_output_string()
> >> -> fputs()
> >> -> vidconsole_puts()
> >> -> video_sync()
> >> -> flush_dcache_range()
> >>
> >> Unfortunately grub abstracts character backends down to the "print
> >> character" level, so it calls UEFI's sopisticated "output_string"
> >> callback with single characters at a time, which means we do a full
> >> dcache flush for every character that we print:
> >>
> >> https://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/term/efi/console.c#n165
> >>
> >>
> >>> This patch set implements the easiest mitigation against this problem:
> >>> Damage tracking. We remember the lowest common denominator region 
> >>> that was
> >>> touched since the last video_sync() call and only flush that. The most
> >>> typical writer to the frame buffer is the video console, which always
> >>> writes rectangles of characters on the screen and syncs afterwards.
> >>>
> >>> With this patch set applied, we reduce drawing a large grub menu (with
> >>> serial console attached for size information) on an RK3399-ROC system
> >>> at 1440p from 55 seconds to less than 1 second.
> >>>
> >>> Version 2 also implements VIDEO_COPY using this mechanism, reducing 
> >>> its
> >>> overhead compared to before as well. So even x86 systems should be 
> >>> faster
> >>> with this now :).
> >>>
> >>>
> >>> Alternatives considered:
> >>>
> >>>  1) Lazy sync - Sandbox does this. It only calls video_sync(true) 
> >>> ever
> >>> so often. We are missing timers to do this generically.
> >>>
> >>>  2) Double buffering - We could try to identify whether anything 
> >>> changed
> >>> at all and only draw to the FB if it did. That would require
> >>> maintaining a second buffer that we need to scan.
> >>>
> >>>  3) Text buffer - Maintain a buffer of all text printed on the 
> >>> screen with
> >>> respective location. Don't write if the old and new character 
> >>> are
> >>> identical. This would limit applicability to text only and is 
> >>> an
> >>> optimization on top of this patch set.
> >>>
> >>>  4) Hash screen lines - Create a hash (sha256?) over every line 
> >>> when it
> >>> changes. Only flush when it does. I'm not sure if this would 
> >>> waste
> >>> more time, memory and cache than the current approach. It 
> >>> would make
> >>> full 

Re: [PATCH v5 10/13] video: Only dcache flush damaged lines

2023-08-21 Thread Simon Glass
Hi Alex,

On Mon, 21 Aug 2023 at 16:44, Alexander Graf  wrote:
>
>
> On 22.08.23 00:10, Simon Glass wrote:
> > Hi Alex,
> >
> > On Mon, 21 Aug 2023 at 13:59, Alexander Graf  wrote:
> >>
> >> On 21.08.23 21:11, Simon Glass wrote:
> >>> Hi Alper,
> >>>
> >>> On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  
> >>> wrote:
>  From: Alexander Graf 
> 
>  Now that we have a damage area tells us which parts of the frame buffer
>  actually need updating, let's only dcache flush those on video_sync()
>  calls. With this optimization in place, frame buffer updates - especially
>  on large screen such as 4k displays - speed up significantly.
> 
>  Signed-off-by: Alexander Graf 
>  Reported-by: Da Xue 
>  [Alper: Use damage.xstart/yend, IS_ENABLED()]
>  Co-developed-by: Alper Nebi Yasak 
>  Signed-off-by: Alper Nebi Yasak 
>  ---
> 
>  Changes in v5:
>  - Use xstart, ystart, xend, yend as names for damage region
>  - Use IS_ENABLED() instead of CONFIG_IS_ENABLED()
> 
>  Changes in v2:
>  - Fix dcache range; we were flushing too much before
>  - Remove ifdefs
> 
> drivers/video/video-uclass.c | 41 +++-
> 1 file changed, 36 insertions(+), 5 deletions(-)
> >>> This is a little strange, since flushing the whole cache will only
> >>> actually write out data that was actually written (to the display). Is
> >>> there a benefit to this patch, in terms of performance?
> >>
> >> I'm happy to see you go through the same thought process I went through
> >> when writing these: "This surely can't be the problem, can it?". The
> >> answer is "simple" in hindsight:
> >>
> >> Have a look at the ARMv8 cache flush function. It does the only "safe"
> >> thing you can expect it to do: Clean+Invalidate to POC because we use it
> >> for multiple things, clearing modified code among others:
> >>
> >> ENTRY(__asm_flush_dcache_range)
> >>   mrs x3, ctr_el0
> >>   ubfxx3, x3, #16, #4
> >>   mov x2, #4
> >>   lsl x2, x2, x3  /* cache line size */
> >>
> >>   /* x2 <- minimal cache line size in cache system */
> >>   sub x3, x2, #1
> >>   bic x0, x0, x3
> >> 1:  dc  civac, x0   /* clean & invalidate data or unified
> >> cache */
> >>   add x0, x0, x2
> >>   cmp x0, x1
> >>   b.lo1b
> >>   dsb sy
> >>   ret
> >> ENDPROC(__asm_flush_dcache_range)
> >>
> >>
> >> Looking at the "dc civac" call, we find this documentation page from
> >> ARM:
> >> https://developer.arm.com/documentation/ddi0601/2022-03/AArch64-Instructions/DC-CIVAC--Data-or-unified-Cache-line-Clean-and-Invalidate-by-VA-to-PoC
> >>
> >> This says we're writing any dirtyness of this cache line up to the POC
> >> and then invalidate (remove the cache line) also up to POC. That means
> >> when you look at a typical SBC, this will either be L2 or system level
> >> cache. Every read afterwards needs to go and pull it all the way back to
> >> L1 to modify it (or not) on the next character write and then flush it
> >> again.
> >>
> >> Even worse: Because of the invalidate, we may even evict it from caches
> >> that the display controller uses to read the frame buffer. So depending
> >> on the SoC's cache topology and implementation, we may force the display
> >> controller to refetch the full FB content on its next screen refresh cycle.
> >>
> >> I faintly remember that I tried to experiment with CVAC instead to only
> >> flush without invalidating. I don't fully remember the results anymore
> >> though. I believe CVAC just behaved identical to CIVAC on the A53
> >> platform I was working on. And then I looked at Cortex-A53 errata like
> >> [1] and just accepted that doing anything but restricting the flushing
> >> range is a waste of time :)
> > Yuck I didn't know it was invalidating too. That is horrible. Is there
> > no way to fix it?
>
>
> Before building all of this damage logic, I tried, but failed. I'd
> welcome anyone else to try again :). I'm not even convinced yet that it
> is actually fixable: Depending on the SoC's internal cache logic, it may
> opt to always invalidate I think.

Wow, that is crazy! How is anyone supposed to make the system run well
with logic like that??!

>
> That said, this patch set really also makes sense outside of the
> particular invalidate problem. It creates a generic abstraction between
> the copy and non-copy code path and allows us to reduce the amount of
> work spent for both, generically for any video sync operation.

Sure...my question was really why it helps so much, given what I
understood the caches to be doing. If they are invalidating, then it
is amazing anything gets done...

Regards,
SImon


Re: [PATCH v5 10/13] video: Only dcache flush damaged lines

2023-08-21 Thread Alexander Graf



On 22.08.23 00:10, Simon Glass wrote:

Hi Alex,

On Mon, 21 Aug 2023 at 13:59, Alexander Graf  wrote:


On 21.08.23 21:11, Simon Glass wrote:

Hi Alper,

On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  wrote:

From: Alexander Graf 

Now that we have a damage area tells us which parts of the frame buffer
actually need updating, let's only dcache flush those on video_sync()
calls. With this optimization in place, frame buffer updates - especially
on large screen such as 4k displays - speed up significantly.

Signed-off-by: Alexander Graf 
Reported-by: Da Xue 
[Alper: Use damage.xstart/yend, IS_ENABLED()]
Co-developed-by: Alper Nebi Yasak 
Signed-off-by: Alper Nebi Yasak 
---

Changes in v5:
- Use xstart, ystart, xend, yend as names for damage region
- Use IS_ENABLED() instead of CONFIG_IS_ENABLED()

Changes in v2:
- Fix dcache range; we were flushing too much before
- Remove ifdefs

   drivers/video/video-uclass.c | 41 +++-
   1 file changed, 36 insertions(+), 5 deletions(-)

This is a little strange, since flushing the whole cache will only
actually write out data that was actually written (to the display). Is
there a benefit to this patch, in terms of performance?


I'm happy to see you go through the same thought process I went through
when writing these: "This surely can't be the problem, can it?". The
answer is "simple" in hindsight:

Have a look at the ARMv8 cache flush function. It does the only "safe"
thing you can expect it to do: Clean+Invalidate to POC because we use it
for multiple things, clearing modified code among others:

ENTRY(__asm_flush_dcache_range)
  mrs x3, ctr_el0
  ubfxx3, x3, #16, #4
  mov x2, #4
  lsl x2, x2, x3  /* cache line size */

  /* x2 <- minimal cache line size in cache system */
  sub x3, x2, #1
  bic x0, x0, x3
1:  dc  civac, x0   /* clean & invalidate data or unified
cache */
  add x0, x0, x2
  cmp x0, x1
  b.lo1b
  dsb sy
  ret
ENDPROC(__asm_flush_dcache_range)


Looking at the "dc civac" call, we find this documentation page from
ARM:
https://developer.arm.com/documentation/ddi0601/2022-03/AArch64-Instructions/DC-CIVAC--Data-or-unified-Cache-line-Clean-and-Invalidate-by-VA-to-PoC

This says we're writing any dirtyness of this cache line up to the POC
and then invalidate (remove the cache line) also up to POC. That means
when you look at a typical SBC, this will either be L2 or system level
cache. Every read afterwards needs to go and pull it all the way back to
L1 to modify it (or not) on the next character write and then flush it
again.

Even worse: Because of the invalidate, we may even evict it from caches
that the display controller uses to read the frame buffer. So depending
on the SoC's cache topology and implementation, we may force the display
controller to refetch the full FB content on its next screen refresh cycle.

I faintly remember that I tried to experiment with CVAC instead to only
flush without invalidating. I don't fully remember the results anymore
though. I believe CVAC just behaved identical to CIVAC on the A53
platform I was working on. And then I looked at Cortex-A53 errata like
[1] and just accepted that doing anything but restricting the flushing
range is a waste of time :)

Yuck I didn't know it was invalidating too. That is horrible. Is there
no way to fix it?



Before building all of this damage logic, I tried, but failed. I'd 
welcome anyone else to try again :). I'm not even convinced yet that it 
is actually fixable: Depending on the SoC's internal cache logic, it may 
opt to always invalidate I think.


That said, this patch set really also makes sense outside of the 
particular invalidate problem. It creates a generic abstraction between 
the copy and non-copy code path and allows us to reduce the amount of 
work spent for both, generically for any video sync operation.



Alex




Re: [PATCH v5 00/13] Add video damage tracking

2023-08-21 Thread Alexander Graf



On 22.08.23 00:10, Simon Glass wrote:

Hi Alex,

On Mon, 21 Aug 2023 at 14:20, Alexander Graf  wrote:


On 21.08.23 21:57, Simon Glass wrote:

Hi Alex,

On Mon, 21 Aug 2023 at 13:33, Alexander Graf  wrote:

On 21.08.23 21:11, Simon Glass wrote:

Hi Alper,

On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  wrote:

This is a rebase of Alexander Graf's video damage tracking series, with
some tests and other changes. The original cover letter is as follows:


This patch set speeds up graphics output on ARM by a factor of 60x.

On most ARM SBCs, we keep the frame buffer in DRAM and map it as cached,
but need it accessible by the display controller which reads directly
from a later point of consistency. Hence, we flush the frame buffer to
DRAM on every change. The full frame buffer.

It should not, see below.


Unfortunately, with the advent of 4k displays, we are seeing frame buffers
that can take a while to flush out. This was reported by Da Xue with grub,
which happily print 1000s of spaces on the screen to draw a menu. Every
printed space triggers a cache flush.

That is a bug somewhere in EFI.

Unfortunately not :). You may call it a bug in grub: It literally prints
over space characters for every character in its menu that it wants
cleared. On every text screen draw.

This wouldn't be a big issue if we only flush the reactangle that gets
modified. But without this patch set, we're flushing the full DRAM
buffer on every u-boot text console character write, which means for
every character (as that's the only API UEFI has).

As a nice side effect, we speed up the normal U-Boot text console as
well with this patch set, because even "normal" text prints that write
for example a single line of text on the screen today flush the full
frame buffer to DRAM.

No, I mean that it is a bug that U-Boot (apparently) flushes the cache
after every character. It doesn't do that for normal character output
and I don't think it makes sense to do it for EFI either.


I see. Let's trace the calls:

efi_cout_output_string()
-> fputs()
-> vidconsole_puts()
-> video_sync()
-> flush_dcache_range()

Unfortunately grub abstracts character backends down to the "print
character" level, so it calls UEFI's sopisticated "output_string"
callback with single characters at a time, which means we do a full
dcache flush for every character that we print:

https://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/term/efi/console.c#n165



This patch set implements the easiest mitigation against this problem:
Damage tracking. We remember the lowest common denominator region that was
touched since the last video_sync() call and only flush that. The most
typical writer to the frame buffer is the video console, which always
writes rectangles of characters on the screen and syncs afterwards.

With this patch set applied, we reduce drawing a large grub menu (with
serial console attached for size information) on an RK3399-ROC system
at 1440p from 55 seconds to less than 1 second.

Version 2 also implements VIDEO_COPY using this mechanism, reducing its
overhead compared to before as well. So even x86 systems should be faster
with this now :).


Alternatives considered:

 1) Lazy sync - Sandbox does this. It only calls video_sync(true) ever
so often. We are missing timers to do this generically.

 2) Double buffering - We could try to identify whether anything changed
at all and only draw to the FB if it did. That would require
maintaining a second buffer that we need to scan.

 3) Text buffer - Maintain a buffer of all text printed on the screen with
respective location. Don't write if the old and new character are
identical. This would limit applicability to text only and is an
optimization on top of this patch set.

 4) Hash screen lines - Create a hash (sha256?) over every line when it
changes. Only flush when it does. I'm not sure if this would waste
more time, memory and cache than the current approach. It would make
full screen updates much more expensive.

5) Fix the bug mentioned above?


Changes in v5:
- Add patch "video: test: Split copy frame buffer check into a function"
- Add patch "video: test: Support checking copy frame buffer contents"
- Add patch "video: test: Test partial updates of hardware frame buffer"
- Use xstart, ystart, xend, yend as names for damage region
- Document damage struct and fields in struct video_priv comment
- Return void from video_damage()
- Fix undeclared priv error in video_sync()
- Drop unused headers from video-uclass.c
- Use IS_ENABLED() instead of CONFIG_IS_ENABLED()
- Call video_damage() also in video_fill_part()
- Use met->baseline instead of priv->baseline
- Use fontdata->height/width instead of VIDEO_FONT_HEIGHT/WIDTH
- Update console_rotate.c video_damage() calls to pass video tests
- Remove mention about not having minimal damage for console_rotate.c
- Add patch "video: test: Test video damage tracking via 

[PATCH v2 5/5] rockchip: board: Add minimal generic RK3566/RK3568 board

2023-08-21 Thread Jonas Karlman
Add a minimal generic RK3566/RK3568 board that only have eMMC and SDMMC
enabled. This defconfig can be used to boot from eMMC or SD-card on most
RK3566/RK3568 boards that follow reference board design.

Signed-off-by: Jonas Karlman 
---
v2:
- New patch

 arch/arm/dts/rk3568-generic-u-boot.dtsi | 14 ++
 arch/arm/dts/rk3568-generic.dts | 38 +++
 board/rockchip/evb_rk3568/MAINTAINERS   |  7 +++
 configs/generic-rk3568_defconfig| 64 +
 doc/board/rockchip/rockchip.rst |  1 +
 5 files changed, 124 insertions(+)
 create mode 100644 arch/arm/dts/rk3568-generic-u-boot.dtsi
 create mode 100644 arch/arm/dts/rk3568-generic.dts
 create mode 100644 configs/generic-rk3568_defconfig

diff --git a/arch/arm/dts/rk3568-generic-u-boot.dtsi 
b/arch/arm/dts/rk3568-generic-u-boot.dtsi
new file mode 100644
index ..90022580a13b
--- /dev/null
+++ b/arch/arm/dts/rk3568-generic-u-boot.dtsi
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+
+#include "rk356x-u-boot.dtsi"
+
+/ {
+   chosen {
+   stdout-path = 
+   };
+};
+
+ {
+   bootph-pre-ram;
+   clock-frequency = <2400>;
+};
diff --git a/arch/arm/dts/rk3568-generic.dts b/arch/arm/dts/rk3568-generic.dts
new file mode 100644
index ..1006ea55bb98
--- /dev/null
+++ b/arch/arm/dts/rk3568-generic.dts
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Minimal generic DT for RK3566/RK3568 with eMMC and SD-card enabled
+ */
+
+/dts-v1/;
+#include "rk356x.dtsi"
+
+/ {
+   model = "Generic RK3566/RK3568";
+   compatible = "rockchip,rk3568";
+
+   chosen: chosen {
+   stdout-path = "serial2:150n8";
+   };
+};
+
+ {
+   bus-width = <8>;
+   cap-mmc-highspeed;
+   non-removable;
+   pinctrl-names = "default";
+   pinctrl-0 = <_bus8 _clk _cmd>;
+   status = "okay";
+};
+
+ {
+   bus-width = <4>;
+   cap-sd-highspeed;
+   disable-wp;
+   pinctrl-names = "default";
+   pinctrl-0 = <_bus4 _clk _cmd>;
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
diff --git a/board/rockchip/evb_rk3568/MAINTAINERS 
b/board/rockchip/evb_rk3568/MAINTAINERS
index cc9eb432a8b5..fa8d1190931d 100644
--- a/board/rockchip/evb_rk3568/MAINTAINERS
+++ b/board/rockchip/evb_rk3568/MAINTAINERS
@@ -7,6 +7,13 @@ F: configs/evb-rk3568_defconfig
 F: arch/arm/dts/rk3568-evb-u-boot.dtsi
 F: arch/arm/dts/rk3568-evb.dts
 
+GENERIC-RK3568
+M: Jonas Karlman 
+S: Maintained
+F: configs/generic-rk3568_defconfig
+F: arch/arm/dts/rk3568-generic.dts
+F: arch/arm/dts/rk3568-generic-u-boot.dtsi
+
 LUBANCAT-2
 M: Andy Yan 
 S: Maintained
diff --git a/configs/generic-rk3568_defconfig b/configs/generic-rk3568_defconfig
new file mode 100644
index ..8f0a9c8c449f
--- /dev/null
+++ b/configs/generic-rk3568_defconfig
@@ -0,0 +1,64 @@
+CONFIG_ARM=y
+CONFIG_SKIP_LOWLEVEL_INIT=y
+CONFIG_COUNTER_FREQUENCY=2400
+CONFIG_ARCH_ROCKCHIP=y
+CONFIG_TEXT_BASE=0x00a0
+CONFIG_SPL_LIBCOMMON_SUPPORT=y
+CONFIG_SPL_LIBGENERIC_SUPPORT=y
+CONFIG_NR_DRAM_BANKS=2
+CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xc0
+CONFIG_DEFAULT_DEVICE_TREE="rk3568-generic"
+CONFIG_ROCKCHIP_RK3568=y
+CONFIG_SPL_ROCKCHIP_COMMON_BOARD=y
+CONFIG_SPL_SERIAL=y
+CONFIG_SPL_STACK_R_ADDR=0x60
+CONFIG_SPL_STACK=0x40
+CONFIG_DEBUG_UART_BASE=0xFE66
+CONFIG_DEBUG_UART_CLOCK=2400
+CONFIG_SYS_LOAD_ADDR=0xc00800
+CONFIG_DEBUG_UART=y
+CONFIG_FIT=y
+CONFIG_FIT_VERBOSE=y
+CONFIG_SPL_FIT_SIGNATURE=y
+CONFIG_SPL_LOAD_FIT=y
+CONFIG_LEGACY_IMAGE_FORMAT=y
+CONFIG_DEFAULT_FDT_FILE="rockchip/rk3568-generic.dtb"
+# CONFIG_DISPLAY_CPUINFO is not set
+CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_SPL_MAX_SIZE=0x4
+CONFIG_SPL_PAD_TO=0x7f8000
+CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
+CONFIG_SPL_BSS_START_ADDR=0x400
+CONFIG_SPL_BSS_MAX_SIZE=0x4000
+# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
+# CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
+CONFIG_SPL_STACK_R=y
+CONFIG_SPL_ATF=y
+CONFIG_CMD_GPIO=y
+CONFIG_CMD_GPT=y
+CONFIG_CMD_MMC=y
+# CONFIG_CMD_SETEXPR is not set
+# CONFIG_SPL_DOS_PARTITION is not set
+CONFIG_SPL_OF_CONTROL=y
+CONFIG_OF_LIVE=y
+CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clock-names 
interrupt-parent assigned-clocks assigned-clock-rates assigned-clock-parents"
+CONFIG_SPL_DM_SEQ_ALIAS=y
+CONFIG_SPL_REGMAP=y
+CONFIG_SPL_SYSCON=y
+CONFIG_SPL_CLK=y
+CONFIG_ROCKCHIP_GPIO=y
+CONFIG_MISC=y
+# CONFIG_ROCKCHIP_IODOMAIN is not set
+CONFIG_SUPPORT_EMMC_RPMB=y
+CONFIG_MMC_DW=y
+CONFIG_MMC_DW_ROCKCHIP=y
+CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_SDMA=y
+CONFIG_MMC_SDHCI_ROCKCHIP=y
+CONFIG_SPL_RAM=y
+CONFIG_BAUDRATE=150
+CONFIG_DEBUG_UART_SHIFT=2
+CONFIG_SYS_NS16550_MEM32=y
+CONFIG_SYSRESET=y
+CONFIG_ERRNO_STR=y
diff --git a/doc/board/rockchip/rockchip.rst b/doc/board/rockchip/rockchip.rst
index de9fe8e642b1..183a9e4aa699 100644
--- a/doc/board/rockchip/rockchip.rst
+++ 

[PATCH v2 4/5] rockchip: Port IO-domain driver for RK3568 from linux

2023-08-21 Thread Jonas Karlman
Port the Rockchip IO-domain driver for RK3568 from linux.

The driver auto probe after bind to configure IO-domain based on the
regulator voltage. Compared to the linux driver this driver is not
notified about regulator voltage changes and only configure IO-domain
based on the initial voltage autoset by the regulator.

It is not recommended to enable MMC_IO_VOLTAGE or the mmc signal voltage
and IO-domain may end up out of sync.

Based on the linux commit 28b05a64e47c ("soc: rockchip: io-domain: add
rk3568 support").

Signed-off-by: Jonas Karlman 
Reviewed-by: Simon Glass 
---
Cc: Jianqun Xu 
Cc: Heiko Stuebner 
Cc: Doug Anderson 
---
v2:
- Add probe after bind comment
- Drop parenthesis
- Collect r-b tag

 drivers/misc/Kconfig  |   9 ++
 drivers/misc/Makefile |   1 +
 drivers/misc/rockchip-io-domain.c | 167 ++
 3 files changed, 177 insertions(+)
 create mode 100644 drivers/misc/rockchip-io-domain.c

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index b9f5c7a37aed..d160ce693939 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -101,6 +101,15 @@ config ROCKCHIP_OTP
  addressing and a length or through child-nodes that are generated
  based on the e-fuse map retrieved from the DTS.
 
+config ROCKCHIP_IODOMAIN
+   bool "Rockchip IO-domain driver support"
+   depends on DM_REGULATOR && ARCH_ROCKCHIP
+   default y if ROCKCHIP_RK3568
+   help
+ Enable support for IO-domains in Rockchip SoCs. It is necessary
+ for the IO-domain setting of the SoC to match the voltage supplied
+ by the regulators.
+
 config SIFIVE_OTP
bool "SiFive eMemory OTP driver"
depends on MISC
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index fd8805f34bd9..b67b82358a6c 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -69,6 +69,7 @@ obj-$(CONFIG_SANDBOX) += qfw_sandbox.o
 endif
 obj-$(CONFIG_ROCKCHIP_EFUSE) += rockchip-efuse.o
 obj-$(CONFIG_ROCKCHIP_OTP) += rockchip-otp.o
+obj-$(CONFIG_$(SPL_TPL_)ROCKCHIP_IODOMAIN) += rockchip-io-domain.o
 obj-$(CONFIG_SANDBOX) += syscon_sandbox.o misc_sandbox.o
 obj-$(CONFIG_SIFIVE_OTP) += sifive-otp.o
 obj-$(CONFIG_SMSC_LPC47M) += smsc_lpc47m.o
diff --git a/drivers/misc/rockchip-io-domain.c 
b/drivers/misc/rockchip-io-domain.c
new file mode 100644
index ..3f6227f993f9
--- /dev/null
+++ b/drivers/misc/rockchip-io-domain.c
@@ -0,0 +1,167 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Rockchip IO Voltage Domain driver
+ *
+ * Ported from linux drivers/soc/rockchip/io-domain.c
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MAX_SUPPLIES   16
+
+/*
+ * The max voltage for 1.8V and 3.3V come from the Rockchip datasheet under
+ * "Recommended Operating Conditions" for "Digital GPIO".   When the typical
+ * is 3.3V the max is 3.6V.  When the typical is 1.8V the max is 1.98V.
+ *
+ * They are used like this:
+ * - If the voltage on a rail is above the "1.8" voltage (1.98V) we'll tell the
+ *   SoC we're at 3.3.
+ * - If the voltage on a rail is above the "3.3" voltage (3.6V) we'll consider
+ *   that to be an error.
+ */
+#define MAX_VOLTAGE_1_8198
+#define MAX_VOLTAGE_3_3360
+
+#define RK3568_PMU_GRF_IO_VSEL00x0140
+#define RK3568_PMU_GRF_IO_VSEL10x0144
+#define RK3568_PMU_GRF_IO_VSEL20x0148
+
+struct rockchip_iodomain_soc_data {
+   int grf_offset;
+   const char *supply_names[MAX_SUPPLIES];
+   int (*write)(struct regmap *grf, int idx, int uV);
+};
+
+static int rk3568_iodomain_write(struct regmap *grf, int idx, int uV)
+{
+   u32 is_3v3 = uV > MAX_VOLTAGE_1_8;
+   u32 val0, val1;
+   int b;
+
+   switch (idx) {
+   case 0: /* pmuio1 */
+   break;
+   case 1: /* pmuio2 */
+   b = idx;
+   val0 = BIT(16 + b) | (is_3v3 ? 0 : BIT(b));
+   b = idx + 4;
+   val1 = BIT(16 + b) | (is_3v3 ? BIT(b) : 0);
+
+   regmap_write(grf, RK3568_PMU_GRF_IO_VSEL2, val0);
+   regmap_write(grf, RK3568_PMU_GRF_IO_VSEL2, val1);
+   break;
+   case 3: /* vccio2 */
+   break;
+   case 2: /* vccio1 */
+   case 4: /* vccio3 */
+   case 5: /* vccio4 */
+   case 6: /* vccio5 */
+   case 7: /* vccio6 */
+   case 8: /* vccio7 */
+   b = idx - 1;
+   val0 = BIT(16 + b) | (is_3v3 ? 0 : BIT(b));
+   val1 = BIT(16 + b) | (is_3v3 ? BIT(b) : 0);
+
+   regmap_write(grf, RK3568_PMU_GRF_IO_VSEL0, val0);
+   regmap_write(grf, RK3568_PMU_GRF_IO_VSEL1, val1);
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+static const struct rockchip_iodomain_soc_data soc_data_rk3568_pmu = {
+   .grf_offset = 0x140,
+   .supply_names = {
+   NULL,

[PATCH v2 2/5] regulator: rk8xx: Return correct voltage for buck converters

2023-08-21 Thread Jonas Karlman
From: Joseph Chen 

Information from the first range group is always used to calculate the
voltage returned for buck converters. This may result in wrong voltage
reported back to the regulator_get_value caller.

Traverse all the possible BUCK ranges to fix this issue.

Fixes: addd062beacc ("power: pmic: rk816: support rk816 pmic")
Fixes: b62280745e55 ("power: pmic: rk805: support rk805 pmic")
Fixes: b4a35574b38d ("power: pmic: rk817: support rk817 pmic")
Fixes: ee30068fa574 ("power: pmic: rk809: support rk809 pmic")
Signed-off-by: Joseph Chen 
[jo...@kwiboo.se: fix checkpatch error, simplify buck get_value, update commit 
message]
Signed-off-by: Jonas Karlman 
Reviewed-by: Simon Glass 
---
v2:
- Simplify locating correct rk8xx_reg_info
- Update max_sel to include range up to and including vsel_mask
- Drop range_num
- Collect r-b tag

 drivers/power/regulator/rk8xx.c | 76 +
 1 file changed, 40 insertions(+), 36 deletions(-)

diff --git a/drivers/power/regulator/rk8xx.c b/drivers/power/regulator/rk8xx.c
index e95640a39b0a..9444daa85c19 100644
--- a/drivers/power/regulator/rk8xx.c
+++ b/drivers/power/regulator/rk8xx.c
@@ -88,62 +88,63 @@ struct rk8xx_reg_info {
u8 config_reg;
u8 vsel_mask;
u8 min_sel;
+   u8 max_sel;
 };
 
 static const struct rk8xx_reg_info rk808_buck[] = {
-   { 712500,   12500, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, 
REG_BUCK1_CONFIG, RK808_BUCK_VSEL_MASK, },
-   { 712500,   12500, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, 
REG_BUCK2_CONFIG, RK808_BUCK_VSEL_MASK, },
-   { 712500,   12500, NA,NA, 
REG_BUCK3_CONFIG, RK808_BUCK_VSEL_MASK, },
-   { 180, 10, REG_BUCK4_ON_VSEL, REG_BUCK4_SLP_VSEL, 
REG_BUCK4_CONFIG, RK808_BUCK4_VSEL_MASK, },
+   {  712500,  12500, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, 
REG_BUCK1_CONFIG,  RK808_BUCK_VSEL_MASK, 0x00, 0x3f },
+   {  712500,  12500, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, 
REG_BUCK2_CONFIG,  RK808_BUCK_VSEL_MASK, 0x00, 0x3f },
+   {  NA, NA,NA, NA, 
REG_BUCK3_CONFIG,NA,   NA,   NA },
+   { 180, 10, REG_BUCK4_ON_VSEL, REG_BUCK4_SLP_VSEL, 
REG_BUCK4_CONFIG, RK808_BUCK4_VSEL_MASK, 0x00, 0x0f },
 };
 
 static const struct rk8xx_reg_info rk816_buck[] = {
/* buck 1 */
-   {  712500,  12500, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, 
REG_BUCK1_CONFIG, RK818_BUCK_VSEL_MASK, 0x00, },
-   { 180, 20, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, 
REG_BUCK1_CONFIG, RK818_BUCK_VSEL_MASK, 0x3c, },
-   { 230,  0, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, 
REG_BUCK1_CONFIG, RK818_BUCK_VSEL_MASK, 0x3f, },
+   {  712500,  12500, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, 
REG_BUCK1_CONFIG,  RK818_BUCK_VSEL_MASK, 0x00, 0x3b },
+   { 180, 20, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, 
REG_BUCK1_CONFIG,  RK818_BUCK_VSEL_MASK, 0x3c, 0x3e },
+   { 230,  0, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, 
REG_BUCK1_CONFIG,  RK818_BUCK_VSEL_MASK, 0x3f, 0x3f },
/* buck 2 */
-   {  712500,  12500, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, 
REG_BUCK2_CONFIG, RK818_BUCK_VSEL_MASK, 0x00, },
-   { 180, 20, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, 
REG_BUCK2_CONFIG, RK818_BUCK_VSEL_MASK, 0x3c, },
-   { 230,  0, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, 
REG_BUCK2_CONFIG, RK818_BUCK_VSEL_MASK, 0x3f, },
+   {  712500,  12500, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, 
REG_BUCK2_CONFIG,  RK818_BUCK_VSEL_MASK, 0x00, 0x3b },
+   { 180, 20, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, 
REG_BUCK2_CONFIG,  RK818_BUCK_VSEL_MASK, 0x3c, 0x3e },
+   { 230,  0, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, 
REG_BUCK2_CONFIG,  RK818_BUCK_VSEL_MASK, 0x3f, 0x3f },
/* buck 3 */
-   { 712500,   12500, NA,NA, 
REG_BUCK3_CONFIG, RK818_BUCK_VSEL_MASK, },
+   {  NA, NA,NA, NA, 
REG_BUCK3_CONFIG,NA,   NA,   NA },
/* buck 4 */
-   {  80, 10, REG_BUCK4_ON_VSEL, REG_BUCK4_SLP_VSEL, 
REG_BUCK4_CONFIG, RK818_BUCK4_VSEL_MASK, },
+   {  80, 10, REG_BUCK4_ON_VSEL, REG_BUCK4_SLP_VSEL, 
REG_BUCK4_CONFIG, RK818_BUCK4_VSEL_MASK, 0x00, 0x1f },
 };
 
 static const struct rk8xx_reg_info rk809_buck5[] = {
/* buck 5 */
-   { 150,  0, RK809_BUCK5_CONFIG(0), RK809_BUCK5_CONFIG(1), NA, 
RK809_BUCK5_VSEL_MASK, 0x00, },
-   { 180, 20, RK809_BUCK5_CONFIG(0), RK809_BUCK5_CONFIG(1), NA, 
RK809_BUCK5_VSEL_MASK, 0x01, },
-   { 280, 20, RK809_BUCK5_CONFIG(0), RK809_BUCK5_CONFIG(1), NA, 
RK809_BUCK5_VSEL_MASK, 0x04, },
-   { 330, 30, RK809_BUCK5_CONFIG(0), RK809_BUCK5_CONFIG(1), NA, 
RK809_BUCK5_VSEL_MASK, 0x06, },
+   { 150,  0, RK809_BUCK5_CONFIG(0), RK809_BUCK5_CONFIG(1), NA, 
RK809_BUCK5_VSEL_MASK, 0x00, 0x00 },
+   { 180, 20, 

[PATCH v2 3/5] regulator: rk8xx: Return correct voltage for switchout converters

2023-08-21 Thread Jonas Karlman
From: shengfei Xu 

The voltage value for switchout converters is always reported as 0 uV.
When the switch is enabled, it's voltage is same as input supply.

Fix this by implementing get_value for switchout converters.

Fixes: ee30068fa574 ("power: pmic: rk809: support rk809 pmic")
Signed-off-by: shengfei Xu 
[jo...@kwiboo.se: fix checkpatch error, update commit message]
Signed-off-by: Jonas Karlman 
---
v2:
- New patch

 drivers/power/regulator/rk8xx.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/drivers/power/regulator/rk8xx.c b/drivers/power/regulator/rk8xx.c
index 9444daa85c19..e80bd6c37230 100644
--- a/drivers/power/regulator/rk8xx.c
+++ b/drivers/power/regulator/rk8xx.c
@@ -1032,6 +1032,25 @@ static int switch_get_suspend_enable(struct udevice *dev)
  */
 static int switch_get_value(struct udevice *dev)
 {
+   static const char * const supply_name_rk809[] = {
+   "vcc9-supply",
+   "vcc8-supply",
+   };
+   struct rk8xx_priv *priv = dev_get_priv(dev->parent);
+   struct udevice *supply;
+   int id = dev->driver_data - 1;
+
+   if (!switch_get_enable(dev))
+   return 0;
+
+   if (priv->variant == RK809_ID) {
+   if (!uclass_get_device_by_phandle(UCLASS_REGULATOR,
+ dev->parent,
+ supply_name_rk809[id],
+ ))
+   return regulator_get_value(supply);
+   }
+
return 0;
 }
 
-- 
2.41.0



[PATCH v2 1/5] power: regulator: Only run autoset once for each regulator

2023-08-21 Thread Jonas Karlman
With the commit 4fcba5d556b4 ("regulator: implement basic reference
counter"), keeping regulator enablement in balance become more important.
Calling regulator_autoset multiple times on a fixed regulator increase
the enable count for each call, resulting in an unbalanced enable count.

Introduce a AUTOSET_DONE flag and use it to mark that autoset has run
for the regulator. Return -EALREADY on any subsequent call to autoset.

This fixes so that the enable count is only ever increased by one per
regulator for autoset.

Fixes: 4fcba5d556b4 ("regulator: implement basic reference counter")
Signed-off-by: Jonas Karlman 
---
Cc: Svyatoslav Ryhel 
---
v2:
- No change

 drivers/power/regulator/regulator-uclass.c | 18 ++
 include/power/regulator.h  |  1 +
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/power/regulator/regulator-uclass.c 
b/drivers/power/regulator/regulator-uclass.c
index 3a6ba69f6d5f..77d101f262e2 100644
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -293,6 +293,9 @@ int regulator_autoset(struct udevice *dev)
 
uc_pdata = dev_get_uclass_plat(dev);
 
+   if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_DONE)
+   return -EALREADY;
+
ret = regulator_set_suspend_enable(dev, uc_pdata->suspend_on);
if (ret == -ENOSYS)
ret = 0;
@@ -306,11 +309,15 @@ int regulator_autoset(struct udevice *dev)
return ret;
}
 
-   if (!uc_pdata->always_on && !uc_pdata->boot_on)
-   return -EMEDIUMTYPE;
+   if (!uc_pdata->always_on && !uc_pdata->boot_on) {
+   ret = -EMEDIUMTYPE;
+   goto out;
+   }
 
-   if (uc_pdata->type == REGULATOR_TYPE_FIXED)
-   return regulator_set_enable(dev, true);
+   if (uc_pdata->type == REGULATOR_TYPE_FIXED) {
+   ret = regulator_set_enable(dev, true);
+   goto out;
+   }
 
if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UV)
ret = regulator_set_value(dev, uc_pdata->min_uV);
@@ -322,6 +329,9 @@ int regulator_autoset(struct udevice *dev)
if (!ret)
ret = regulator_set_enable(dev, true);
 
+out:
+   uc_pdata->flags |= REGULATOR_FLAG_AUTOSET_DONE;
+
return ret;
 }
 
diff --git a/include/power/regulator.h b/include/power/regulator.h
index ff1bfc2435ae..200652cb3d7a 100644
--- a/include/power/regulator.h
+++ b/include/power/regulator.h
@@ -134,6 +134,7 @@ struct dm_regulator_mode {
 enum regulator_flag {
REGULATOR_FLAG_AUTOSET_UV   = 1 << 0,
REGULATOR_FLAG_AUTOSET_UA   = 1 << 1,
+   REGULATOR_FLAG_AUTOSET_DONE = 1 << 2,
 };
 
 /**
-- 
2.41.0



[PATCH v2 0/5] rockchip: Port IO-domain driver for RK3568 from linux

2023-08-21 Thread Jonas Karlman
This series port the IO-domain driver for RK3568 from linux. It is
necessary for the IO-domain setting of the SoC to match the voltage
supplied by the regulators.

The driver auto probe after bind and configures IO-domain based on the
voltage reported by the regulators. This fixes issues observed while
working on the GMAC driver for RK3568 devices. The IO-domain setting
default to 3v3, however some vccio domains may be supplied by 1v8.

This series also add a minimal generic RK356x board that only enable
eMMC and SD-card node and keep the IO-domain driver disabled. The
generic board can be used to boot from eMMC/SD-card on boards that
follow reference board design.

Changes in v2:
- Include patch to return correct voltage for switchout converters
- Simplify code that return correct voltage for buck converters
- Add a minimal generic RK356x board with IO-domain driver disabled.
- Collect r-b tags

Patch 1 adds protection so that regulator autoset only happens one time.
Patch 2-3 fixes rk8xx buck/switchout regulator driver to report correct
voltage.
Patch 4 adds the IO-domain driver.
Patch 5 adds the minimal generic RK3566/RK3568 board.

This series can also be found at [1]

[1] https://github.com/Kwiboo/u-boot-rockchip/commits/rk3568-io-domain-v2

Jonas Karlman (3):
  power: regulator: Only run autoset once for each regulator
  rockchip: Port IO-domain driver for RK3568 from linux
  rockchip: board: Add minimal generic RK3566/RK3568 board

Joseph Chen (1):
  regulator: rk8xx: Return correct voltage for buck converters

shengfei Xu (1):
  regulator: rk8xx: Return correct voltage for switchout converters

 arch/arm/dts/rk3568-generic-u-boot.dtsi|  14 ++
 arch/arm/dts/rk3568-generic.dts|  38 +
 board/rockchip/evb_rk3568/MAINTAINERS  |   7 +
 configs/generic-rk3568_defconfig   |  64 
 doc/board/rockchip/rockchip.rst|   1 +
 drivers/misc/Kconfig   |   9 ++
 drivers/misc/Makefile  |   1 +
 drivers/misc/rockchip-io-domain.c  | 167 +
 drivers/power/regulator/regulator-uclass.c |  18 ++-
 drivers/power/regulator/rk8xx.c|  95 +++-
 include/power/regulator.h  |   1 +
 11 files changed, 375 insertions(+), 40 deletions(-)
 create mode 100644 arch/arm/dts/rk3568-generic-u-boot.dtsi
 create mode 100644 arch/arm/dts/rk3568-generic.dts
 create mode 100644 configs/generic-rk3568_defconfig
 create mode 100644 drivers/misc/rockchip-io-domain.c

-- 
2.41.0



Re: [PATCH v5 10/13] video: Only dcache flush damaged lines

2023-08-21 Thread Simon Glass
Hi Alex,

On Mon, 21 Aug 2023 at 13:59, Alexander Graf  wrote:
>
>
> On 21.08.23 21:11, Simon Glass wrote:
> > Hi Alper,
> >
> > On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  
> > wrote:
> >> From: Alexander Graf 
> >>
> >> Now that we have a damage area tells us which parts of the frame buffer
> >> actually need updating, let's only dcache flush those on video_sync()
> >> calls. With this optimization in place, frame buffer updates - especially
> >> on large screen such as 4k displays - speed up significantly.
> >>
> >> Signed-off-by: Alexander Graf 
> >> Reported-by: Da Xue 
> >> [Alper: Use damage.xstart/yend, IS_ENABLED()]
> >> Co-developed-by: Alper Nebi Yasak 
> >> Signed-off-by: Alper Nebi Yasak 
> >> ---
> >>
> >> Changes in v5:
> >> - Use xstart, ystart, xend, yend as names for damage region
> >> - Use IS_ENABLED() instead of CONFIG_IS_ENABLED()
> >>
> >> Changes in v2:
> >> - Fix dcache range; we were flushing too much before
> >> - Remove ifdefs
> >>
> >>   drivers/video/video-uclass.c | 41 +++-
> >>   1 file changed, 36 insertions(+), 5 deletions(-)
> > This is a little strange, since flushing the whole cache will only
> > actually write out data that was actually written (to the display). Is
> > there a benefit to this patch, in terms of performance?
>
>
> I'm happy to see you go through the same thought process I went through
> when writing these: "This surely can't be the problem, can it?". The
> answer is "simple" in hindsight:
>
> Have a look at the ARMv8 cache flush function. It does the only "safe"
> thing you can expect it to do: Clean+Invalidate to POC because we use it
> for multiple things, clearing modified code among others:
>
> ENTRY(__asm_flush_dcache_range)
>  mrs x3, ctr_el0
>  ubfxx3, x3, #16, #4
>  mov x2, #4
>  lsl x2, x2, x3  /* cache line size */
>
>  /* x2 <- minimal cache line size in cache system */
>  sub x3, x2, #1
>  bic x0, x0, x3
> 1:  dc  civac, x0   /* clean & invalidate data or unified
> cache */
>  add x0, x0, x2
>  cmp x0, x1
>  b.lo1b
>  dsb sy
>  ret
> ENDPROC(__asm_flush_dcache_range)
>
>
> Looking at the "dc civac" call, we find this documentation page from
> ARM:
> https://developer.arm.com/documentation/ddi0601/2022-03/AArch64-Instructions/DC-CIVAC--Data-or-unified-Cache-line-Clean-and-Invalidate-by-VA-to-PoC
>
> This says we're writing any dirtyness of this cache line up to the POC
> and then invalidate (remove the cache line) also up to POC. That means
> when you look at a typical SBC, this will either be L2 or system level
> cache. Every read afterwards needs to go and pull it all the way back to
> L1 to modify it (or not) on the next character write and then flush it
> again.
>
> Even worse: Because of the invalidate, we may even evict it from caches
> that the display controller uses to read the frame buffer. So depending
> on the SoC's cache topology and implementation, we may force the display
> controller to refetch the full FB content on its next screen refresh cycle.
>
> I faintly remember that I tried to experiment with CVAC instead to only
> flush without invalidating. I don't fully remember the results anymore
> though. I believe CVAC just behaved identical to CIVAC on the A53
> platform I was working on. And then I looked at Cortex-A53 errata like
> [1] and just accepted that doing anything but restricting the flushing
> range is a waste of time :)

Yuck I didn't know it was invalidating too. That is horrible. Is there
no way to fix it?

Regards,
Simon

>
>
> Alex
>
>
> [1]
> https://patchwork.kernel.org/project/xen-devel/patch/1462466065-30212-14-git-send-email-julien.gr...@arm.com/
>
>


Re: [RFC PATCH 5/5] sandbox: video: Use partial updates for SDL display

2023-08-21 Thread Simon Glass
On Mon, 21 Aug 2023 at 12:13, Alper Nebi Yasak  wrote:
>
> Now that we have video damage tracking, try to reduce the SDL display
> work by copying only the updated regions onto the SDL texture instead of
> the entire framebuffer. We still have to do RenderClear and RenderCopy
> the whole texture onto the renderer, but that allegedly happens in the
> GPU.
>
> Signed-off-by: Alper Nebi Yasak 
> ---
> The second half of copy_to_texture is untested.
>
>  arch/sandbox/cpu/sdl.c | 25 +
>  arch/sandbox/include/asm/sdl.h |  9 +++--
>  drivers/video/sandbox_sdl.c| 16 +++-
>  3 files changed, 39 insertions(+), 11 deletions(-)
>

Reviewed-by: Simon Glass 


Re: [PATCH 4/5] sandbox: video: Move sandbox video sync to a driver operation

2023-08-21 Thread Simon Glass
On Mon, 21 Aug 2023 at 12:13, Alper Nebi Yasak  wrote:
>
> The sandbox SDL video sync is handled in the uclass because there has
> been a sync rate limiter and a way to bypass that. Previous patches
> move the rate limit code into SDL-specific files, and provide a generic
> way to defer and force video syncs.
>
> Sandbox code shouldn't be in the uclasses if possible. Move the
> remaining sandbox sync call into the driver ops. Now that sandbox video
> sync attempts can defer without resetting video damage, force a video
> sync before checking that video syncs reset video damage.
>
> Signed-off-by: Alper Nebi Yasak 
> ---
>
>  drivers/video/sandbox_sdl.c  | 16 
>  drivers/video/video-uclass.c | 14 --
>  test/dm/video.c  |  2 +-
>  3 files changed, 17 insertions(+), 15 deletions(-)

Reviewed-by: Simon Glass 


Re: [PATCH 2/5] video: Allow deferring and retrying driver-specific video sync

2023-08-21 Thread Simon Glass
Hi Alper,

On Mon, 21 Aug 2023 at 12:13, Alper Nebi Yasak  wrote:
>
> The sandbox SDL driver has some code in the video uclass to rate limit
> video syncs by postponing them, and forcing a sync nonetheless with a
> "force" argument.
>
> Add infrastructure for doing this through driver ops, where the driver
> can request to defer a sync with -EAGAIN, and the uclass can force it by
> calling the op again it until it does something.
>
> Signed-off-by: Alper Nebi Yasak 
> ---
> Alternatively, add "force" argument into the driver ops->video_sync().
> But I think it should go away instead. The problem is video_sync() being
> called irregularly and too frequently, maybe we can call it only from
> cyclic at the hardware refresh rate?

Yes I like that idea better. Calling it in a tight loop until it does
something seems wrong to me.

>
>  drivers/video/video-uclass.c | 2 ++
>  include/video.h  | 3 ++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
> index 7cec6362570f..accf486509cb 100644
> --- a/drivers/video/video-uclass.c
> +++ b/drivers/video/video-uclass.c
> @@ -446,6 +446,8 @@ int video_sync(struct udevice *vid, bool force)
>
> if (ops && ops->video_sync) {
> ret = ops->video_sync(vid);
> +   while (force && ret == -EAGAIN)
> +   ret = ops->video_sync(vid);
> if (ret)
> return ret;
> }
> diff --git a/include/video.h b/include/video.h
> index 42e57b44188d..5c4327d4e455 100644
> --- a/include/video.h
> +++ b/include/video.h
> @@ -137,7 +137,8 @@ struct video_priv {
>   * displays needs synchronization when data in an FB is 
> available.
>   * For these devices implement video_sync hook to call a sync
>   * function. vid is pointer to video device udevice. Function
> - * should return 0 on success video_sync and error code otherwise
> + * should return 0 on success video_sync, -EAGAIN if it was
> + * deferred and should be tried again, and error code otherwise
>   */
>  struct video_ops {
> int (*video_sync)(struct udevice *vid);
> --
> 2.40.1
>

Regards,
Simon


Re: [PATCH 1/5] sandbox: video: Display copy framebuffer if enabled

2023-08-21 Thread Simon Glass
On Mon, 21 Aug 2023 at 12:13, Alper Nebi Yasak  wrote:
>
> When VIDEO_COPY is enabled, the "main" framebuffer is a cached work area
> in U-Boot allocated memory and the "copy" framebuffer is the hardware
> frame buffer displayed on the screen. The sandbox SDL video driver sets
> copy_base to indicate support for this, but it displays the work area
> instead. Change it to display the copy buffer if enabled.
>
> Signed-off-by: Alper Nebi Yasak 
> ---
>
>  drivers/video/video-uclass.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>

Reviewed-by: Simon Glass 


Re: [PATCH v5 00/13] Add video damage tracking

2023-08-21 Thread Simon Glass
Hi Alex,

On Mon, 21 Aug 2023 at 14:20, Alexander Graf  wrote:
>
>
> On 21.08.23 21:57, Simon Glass wrote:
> > Hi Alex,
> >
> > On Mon, 21 Aug 2023 at 13:33, Alexander Graf  wrote:
> >>
> >> On 21.08.23 21:11, Simon Glass wrote:
> >>> Hi Alper,
> >>>
> >>> On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  
> >>> wrote:
>  This is a rebase of Alexander Graf's video damage tracking series, with
>  some tests and other changes. The original cover letter is as follows:
> 
> > This patch set speeds up graphics output on ARM by a factor of 60x.
> >
> > On most ARM SBCs, we keep the frame buffer in DRAM and map it as cached,
> > but need it accessible by the display controller which reads directly
> > from a later point of consistency. Hence, we flush the frame buffer to
> > DRAM on every change. The full frame buffer.
> >>> It should not, see below.
> >>>
> > Unfortunately, with the advent of 4k displays, we are seeing frame 
> > buffers
> > that can take a while to flush out. This was reported by Da Xue with 
> > grub,
> > which happily print 1000s of spaces on the screen to draw a menu. Every
> > printed space triggers a cache flush.
> >>> That is a bug somewhere in EFI.
> >>
> >> Unfortunately not :). You may call it a bug in grub: It literally prints
> >> over space characters for every character in its menu that it wants
> >> cleared. On every text screen draw.
> >>
> >> This wouldn't be a big issue if we only flush the reactangle that gets
> >> modified. But without this patch set, we're flushing the full DRAM
> >> buffer on every u-boot text console character write, which means for
> >> every character (as that's the only API UEFI has).
> >>
> >> As a nice side effect, we speed up the normal U-Boot text console as
> >> well with this patch set, because even "normal" text prints that write
> >> for example a single line of text on the screen today flush the full
> >> frame buffer to DRAM.
> > No, I mean that it is a bug that U-Boot (apparently) flushes the cache
> > after every character. It doesn't do that for normal character output
> > and I don't think it makes sense to do it for EFI either.
>
>
> I see. Let's trace the calls:
>
> efi_cout_output_string()
> -> fputs()
> -> vidconsole_puts()
> -> video_sync()
> -> flush_dcache_range()
>
> Unfortunately grub abstracts character backends down to the "print
> character" level, so it calls UEFI's sopisticated "output_string"
> callback with single characters at a time, which means we do a full
> dcache flush for every character that we print:
>
> https://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/term/efi/console.c#n165
>
>
> >
> >>
> > This patch set implements the easiest mitigation against this problem:
> > Damage tracking. We remember the lowest common denominator region that 
> > was
> > touched since the last video_sync() call and only flush that. The most
> > typical writer to the frame buffer is the video console, which always
> > writes rectangles of characters on the screen and syncs afterwards.
> >
> > With this patch set applied, we reduce drawing a large grub menu (with
> > serial console attached for size information) on an RK3399-ROC system
> > at 1440p from 55 seconds to less than 1 second.
> >
> > Version 2 also implements VIDEO_COPY using this mechanism, reducing its
> > overhead compared to before as well. So even x86 systems should be 
> > faster
> > with this now :).
> >
> >
> > Alternatives considered:
> >
> > 1) Lazy sync - Sandbox does this. It only calls video_sync(true) 
> > ever
> >so often. We are missing timers to do this generically.
> >
> > 2) Double buffering - We could try to identify whether anything 
> > changed
> >at all and only draw to the FB if it did. That would require
> >maintaining a second buffer that we need to scan.
> >
> > 3) Text buffer - Maintain a buffer of all text printed on the 
> > screen with
> >respective location. Don't write if the old and new character are
> >identical. This would limit applicability to text only and is an
> >optimization on top of this patch set.
> >
> > 4) Hash screen lines - Create a hash (sha256?) over every line when 
> > it
> >changes. Only flush when it does. I'm not sure if this would 
> > waste
> >more time, memory and cache than the current approach. It would 
> > make
> >full screen updates much more expensive.
> >>> 5) Fix the bug mentioned above?
> >>>
>  Changes in v5:
>  - Add patch "video: test: Split copy frame buffer check into a function"
>  - Add patch "video: test: Support checking copy frame buffer contents"
>  - Add patch "video: test: Test partial updates of hardware frame buffer"
>  - Use xstart, ystart, xend, yend as names 

Fwd: New Defects reported by Coverity Scan for Das U-Boot

2023-08-21 Thread Tom Rini
Here's the latest report

-- Forwarded message -
From: 
Date: Mon, Aug 21, 2023 at 4:30 PM
Subject: New Defects reported by Coverity Scan for Das U-Boot
To: 


Hi,

Please find the latest report on new defect(s) introduced to Das
U-Boot found with Coverity Scan.

4 new defect(s) introduced to Das U-Boot found with Coverity Scan.


New defect(s) Reported-by: Coverity Scan
Showing 4 of 4 defect(s)


** CID 464362:  Integer handling issues  (OVERFLOW_BEFORE_WIDEN)
/drivers/video/pwm_backlight.c: 68 in set_pwm()



*** CID 464362:  Integer handling issues  (OVERFLOW_BEFORE_WIDEN)
/drivers/video/pwm_backlight.c: 68 in set_pwm()
62 {
63  u64 width;
64  uint duty_cycle;
65  int ret;
66
67  if (priv->period_ns) {
>>> CID 464362:  Integer handling issues  (OVERFLOW_BEFORE_WIDEN)
>>> Potentially overflowing expression "priv->period_ns * (priv->cur_level 
>>> - priv->min_level)" with type "unsigned int" (32 bits, unsigned) is 
>>> evaluated using 32-bit arithmetic, and then used in a context that expects 
>>> an expression of type "u64" (64 bits, unsigned).
68  width = priv->period_ns * (priv->cur_level - priv->min_level);
69  duty_cycle = div_u64(width,
70   (priv->max_level - priv->min_level));
71  ret = pwm_set_config(priv->pwm, priv->channel, priv->period_ns,
72   duty_cycle);
73  } else {

** CID 464361:  Control flow issues  (DEADCODE)
/drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log()



*** CID 464361:  Control flow issues  (DEADCODE)
/drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log()
142
143 if (ffa_id < FFA_FIRST_ID || ffa_id > FFA_LAST_ID)
144 return -EINVAL;
145
146 abi_idx = FFA_ID_TO_ERRMAP_ID(ffa_id);
147 if (abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)
>>> CID 464361:  Control flow issues  (DEADCODE)
>>> Execution cannot reach this statement: "return -22;".
148 return -EINVAL;
149
150 if (!err_msg_map[abi_idx].err_str[err_idx])
151 return -EINVAL;
152
153 log_err("%s\n", err_msg_map[abi_idx].err_str[err_idx]);

** CID 464360:  Control flow issues  (NO_EFFECT)
/drivers/firmware/arm-ffa/arm-ffa-uclass.c: 207 in ffa_get_version_hdlr()



*** CID 464360:  Control flow issues  (NO_EFFECT)
/drivers/firmware/arm-ffa/arm-ffa-uclass.c: 207 in ffa_get_version_hdlr()
201 major = GET_FFA_MAJOR_VERSION(res.a0);
202 minor = GET_FFA_MINOR_VERSION(res.a0);
203
204 log_debug("FF-A driver %d.%d\nFF-A framework %d.%d\n",
205  FFA_MAJOR_VERSION, FFA_MINOR_VERSION, major, minor);
206
>>> CID 464360:  Control flow issues  (NO_EFFECT)
>>> This greater-than-or-equal-to-zero comparison of an unsigned value is 
>>> always true. "minor >= 0".
207 if (major == FFA_MAJOR_VERSION && minor >= FFA_MINOR_VERSION) {
208 log_debug("FF-A versions are compatible\n");
209
210 if (dev) {
211 uc_priv = dev_get_uclass_priv(dev);
212 if (uc_priv)

** CID 464359:(PASS_BY_VALUE)
/drivers/firmware/arm-ffa/arm-ffa-uclass.c: 168 in invoke_ffa_fn()
/drivers/firmware/arm-ffa/ffa-emul-uclass.c: 673 in invoke_ffa_fn()



*** CID 464359:(PASS_BY_VALUE)
/drivers/firmware/arm-ffa/arm-ffa-uclass.c: 168 in invoke_ffa_fn()
162  * @args: FF-A ABI arguments to be copied to Xn registers
163  * @res: FF-A ABI return data to be copied from Xn registers
164  *
165  * Calls low level SMC implementation.
166  * This function should be implemented by the user driver.
167  */
>>> CID 464359:(PASS_BY_VALUE)
>>> Passing parameter args of type "ffa_value_t" (size 144 bytes) by value, 
>>> which exceeds the low threshold of 128 bytes.
168 void __weak invoke_ffa_fn(ffa_value_t args, ffa_value_t *res)
169 {
170 }
171
172 /**
173  * ffa_get_version_hdlr() - FFA_VERSION handler function
/drivers/firmware/arm-ffa/ffa-emul-uclass.c: 673 in invoke_ffa_fn()
667  * invoke_ffa_fn() - SMC wrapper
668  * @args: FF-A ABI arguments to be copied to Xn registers
669  * @res: FF-A ABI return data to be copied from Xn registers
670  *
671  * Calls the emulated SMC call.
672  */
>>> CID 464359:(PASS_BY_VALUE)
>>> Passing parameter args of type "ffa_value_t" (size 144 bytes) by value, 
>>> which 

[ANN] U-Boot v2023.10-rc3 released

2023-08-21 Thread Tom Rini
Hey all,

It's release day and I've tagged and pushed -rc3, and will merge that to
next as well shortly. Things should really stay stable now until
release, so I'd really like to start seeing PRs for -next at this point.

In terms of a changelog, 
git log --merges v2023.10-rc2..v2023.10-rc3
contains what I've pulled but as always, better PR messages and tags
will provide better results here.

I'm going to try and stay on schedule now and that means the rest of the
rcs every other Monday, and with final release on October 2nd, 2023.
Thanks all!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v5 00/13] Add video damage tracking

2023-08-21 Thread Alexander Graf



On 21.08.23 21:57, Simon Glass wrote:

Hi Alex,

On Mon, 21 Aug 2023 at 13:33, Alexander Graf  wrote:


On 21.08.23 21:11, Simon Glass wrote:

Hi Alper,

On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  wrote:

This is a rebase of Alexander Graf's video damage tracking series, with
some tests and other changes. The original cover letter is as follows:


This patch set speeds up graphics output on ARM by a factor of 60x.

On most ARM SBCs, we keep the frame buffer in DRAM and map it as cached,
but need it accessible by the display controller which reads directly
from a later point of consistency. Hence, we flush the frame buffer to
DRAM on every change. The full frame buffer.

It should not, see below.


Unfortunately, with the advent of 4k displays, we are seeing frame buffers
that can take a while to flush out. This was reported by Da Xue with grub,
which happily print 1000s of spaces on the screen to draw a menu. Every
printed space triggers a cache flush.

That is a bug somewhere in EFI.


Unfortunately not :). You may call it a bug in grub: It literally prints
over space characters for every character in its menu that it wants
cleared. On every text screen draw.

This wouldn't be a big issue if we only flush the reactangle that gets
modified. But without this patch set, we're flushing the full DRAM
buffer on every u-boot text console character write, which means for
every character (as that's the only API UEFI has).

As a nice side effect, we speed up the normal U-Boot text console as
well with this patch set, because even "normal" text prints that write
for example a single line of text on the screen today flush the full
frame buffer to DRAM.

No, I mean that it is a bug that U-Boot (apparently) flushes the cache
after every character. It doesn't do that for normal character output
and I don't think it makes sense to do it for EFI either.



I see. Let's trace the calls:

efi_cout_output_string()
-> fputs()
-> vidconsole_puts()
-> video_sync()
-> flush_dcache_range()

Unfortunately grub abstracts character backends down to the "print 
character" level, so it calls UEFI's sopisticated "output_string" 
callback with single characters at a time, which means we do a full 
dcache flush for every character that we print:


https://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/term/efi/console.c#n165







This patch set implements the easiest mitigation against this problem:
Damage tracking. We remember the lowest common denominator region that was
touched since the last video_sync() call and only flush that. The most
typical writer to the frame buffer is the video console, which always
writes rectangles of characters on the screen and syncs afterwards.

With this patch set applied, we reduce drawing a large grub menu (with
serial console attached for size information) on an RK3399-ROC system
at 1440p from 55 seconds to less than 1 second.

Version 2 also implements VIDEO_COPY using this mechanism, reducing its
overhead compared to before as well. So even x86 systems should be faster
with this now :).


Alternatives considered:

1) Lazy sync - Sandbox does this. It only calls video_sync(true) ever
   so often. We are missing timers to do this generically.

2) Double buffering - We could try to identify whether anything changed
   at all and only draw to the FB if it did. That would require
   maintaining a second buffer that we need to scan.

3) Text buffer - Maintain a buffer of all text printed on the screen with
   respective location. Don't write if the old and new character are
   identical. This would limit applicability to text only and is an
   optimization on top of this patch set.

4) Hash screen lines - Create a hash (sha256?) over every line when it
   changes. Only flush when it does. I'm not sure if this would waste
   more time, memory and cache than the current approach. It would make
   full screen updates much more expensive.

5) Fix the bug mentioned above?


Changes in v5:
- Add patch "video: test: Split copy frame buffer check into a function"
- Add patch "video: test: Support checking copy frame buffer contents"
- Add patch "video: test: Test partial updates of hardware frame buffer"
- Use xstart, ystart, xend, yend as names for damage region
- Document damage struct and fields in struct video_priv comment
- Return void from video_damage()
- Fix undeclared priv error in video_sync()
- Drop unused headers from video-uclass.c
- Use IS_ENABLED() instead of CONFIG_IS_ENABLED()
- Call video_damage() also in video_fill_part()
- Use met->baseline instead of priv->baseline
- Use fontdata->height/width instead of VIDEO_FONT_HEIGHT/WIDTH
- Update console_rotate.c video_damage() calls to pass video tests
- Remove mention about not having minimal damage for console_rotate.c
- Add patch "video: test: Test video damage tracking via vidconsole"
- Document new vdev field in struct efi_gop_obj comment
- Remove video_sync_copy() also from 

Re: Please pull u-boot-dm

2023-08-21 Thread Tom Rini
On Mon, Aug 21, 2023 at 01:11:38PM -0600, Simon Glass wrote:

> Hi Tom,
> 
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/pipelines/17449
> https://dev.azure.com/simon0972/u-boot/_build/results?buildId=47=results
> 
> 
> The following changes since commit 17aad803551500e1a3d87339a6559e99b7fad479:
> 
>   Merge branch 'master' of
> https://source.denx.de/u-boot/custodians/u-boot-sh (2023-08-20
> 11:09:11 -0400)
> 
> are available in the Git repository at:
> 
>   git://git.denx.de/u-boot-dm.git tags/dm-pull-20aug23
> 
> for you to fetch changes up to 25a9be71ec1ca779aac8bcb1d8a363725ff0ac7f:
> 
>   test: cpu: Handle both 32bit and 64bit CPUs (2023-08-20 15:55:27 -0600)
> 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v5 11/13] video: Use VIDEO_DAMAGE for VIDEO_COPY

2023-08-21 Thread Alexander Graf



On 21.08.23 21:11, Simon Glass wrote:

Hi Alper,

On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  wrote:

From: Alexander Graf 

CONFIG_VIDEO_COPY implemented a range-based copying mechanism: If we
print a single character, it will always copy the full range of bytes
from the top left corner of the character to the lower right onto the
uncached frame buffer. This includes pretty much the full line contents
of the printed character.

Since we now have proper damage tracking, let's make use of that to reduce
the amount of data we need to copy. With this patch applied, we will only
copy the tiny rectangle surrounding characters when we print them,
speeding up the video console.

I suppose for rotated consoles it copies whole lines, but otherwise it
does a lot of small copies?



I tried to keep the code as simple as possible and only track an "upper 
left" and "lower right" corner of modifications. So sync will always 
copy/flush a single rectangle.






After this, changes to the main frame buffer are not immediately copied
to the copy frame buffer, but postponed until the next video device
sync. So issue an explicit sync before inspecting the copy frame buffer
contents for the video tests.

So how does the sync get done in this case?



It gets called as part of video_sync():

+static void video_flush_copy(struct udevice *vid)
+{
+   struct video_priv *priv = dev_get_uclass_priv(vid);
+
+   if (!priv->copy_fb)
+   return;
+
+   if (priv->damage.xend && priv->damage.yend) {
+   int lstart = priv->damage.xstart * VNBYTES(priv->bpix);
+   int lend = priv->damage.xend * VNBYTES(priv->bpix);
+   int y;
+
+   for (y = priv->damage.ystart; y < priv->damage.yend; y++) {
+   ulong offset = (y * priv->line_length) + lstart;
+   ulong len = lend - lstart;
+
+   memcpy(priv->copy_fb + offset, priv->fb + offset, len);
+   }
+   }
+}





Signed-off-by: Alexander Graf 
[Alper: Rebase for fontdata->height/w, fill_part(), fix memmove(dev),
 drop from defconfig, use damage.xstart/yend, use IS_ENABLED(),
 call video_sync() before copy_fb check, update video_copy test]
Co-developed-by: Alper Nebi Yasak 
Signed-off-by: Alper Nebi Yasak 
---

Changes in v5:
- Remove video_sync_copy() also from video_fill(), video_fill_part()
- Fix memmove() calls by removing the extra dev argument
- Call video_sync() before checking copy_fb in video tests
- Use xstart, ystart, xend, yend as names for damage region
- Use met->baseline instead of priv->baseline
- Use fontdata->height/width instead of VIDEO_FONT_HEIGHT/WIDTH
- Use xstart, ystart, xend, yend as names for damage region
- Use IS_ENABLED() instead of CONFIG_IS_ENABLED()
- Drop VIDEO_DAMAGE from sandbox defconfig added in a new patch
- Update dm_test_video_copy test added in a new patch

Changes in v3:
- Make VIDEO_COPY always select VIDEO_DAMAGE

Changes in v2:
- Add patch "video: Use VIDEO_DAMAGE for VIDEO_COPY"

  configs/sandbox_defconfig |  1 -
  drivers/video/Kconfig |  5 ++
  drivers/video/console_normal.c| 13 +
  drivers/video/console_rotate.c| 44 +++---
  drivers/video/console_truetype.c  | 16 +
  drivers/video/vidconsole-uclass.c | 16 -
  drivers/video/video-uclass.c  | 97 ---
  drivers/video/video_bmp.c |  7 ---
  include/video.h   | 37 
  include/video_console.h   | 52 -
  test/dm/video.c   |  3 +-
  11 files changed, 43 insertions(+), 248 deletions(-)

diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 51b820f13121..259f31f26cee 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -307,7 +307,6 @@ CONFIG_USB_ETH_CDC=y
  CONFIG_VIDEO=y
  CONFIG_VIDEO_FONT_SUN12X22=y
  CONFIG_VIDEO_COPY=y
-CONFIG_VIDEO_DAMAGE=y
  CONFIG_CONSOLE_ROTATION=y
  CONFIG_CONSOLE_TRUETYPE=y
  CONFIG_CONSOLE_TRUETYPE_CANTORAONE=y
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 97f494a1340b..b3fbd9d7d9ca 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -83,11 +83,14 @@ config VIDEO_PCI_DEFAULT_FB_SIZE

  config VIDEO_COPY
 bool "Enable copying the frame buffer to a hardware copy"
+   select VIDEO_DAMAGE
 help
   On some machines (e.g. x86), reading from the frame buffer is very
   slow because it is uncached. To improve performance, this feature
   allows the frame buffer to be kept in cached memory (allocated by
   U-Boot) and then copied to the hardware frame-buffer as needed.
+ It uses the VIDEO_DAMAGE feature to keep track of regions to copy
+ and will only copy actually touched regions.

   To use this, your video driver must set @copy_base in
   struct video_uc_plat.
@@ -105,6 +108,8 @@ config VIDEO_DAMAGE
   

Re: [PATCH v5 10/13] video: Only dcache flush damaged lines

2023-08-21 Thread Alexander Graf



On 21.08.23 21:11, Simon Glass wrote:

Hi Alper,

On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  wrote:

From: Alexander Graf 

Now that we have a damage area tells us which parts of the frame buffer
actually need updating, let's only dcache flush those on video_sync()
calls. With this optimization in place, frame buffer updates - especially
on large screen such as 4k displays - speed up significantly.

Signed-off-by: Alexander Graf 
Reported-by: Da Xue 
[Alper: Use damage.xstart/yend, IS_ENABLED()]
Co-developed-by: Alper Nebi Yasak 
Signed-off-by: Alper Nebi Yasak 
---

Changes in v5:
- Use xstart, ystart, xend, yend as names for damage region
- Use IS_ENABLED() instead of CONFIG_IS_ENABLED()

Changes in v2:
- Fix dcache range; we were flushing too much before
- Remove ifdefs

  drivers/video/video-uclass.c | 41 +++-
  1 file changed, 36 insertions(+), 5 deletions(-)

This is a little strange, since flushing the whole cache will only
actually write out data that was actually written (to the display). Is
there a benefit to this patch, in terms of performance?



I'm happy to see you go through the same thought process I went through 
when writing these: "This surely can't be the problem, can it?". The 
answer is "simple" in hindsight:


Have a look at the ARMv8 cache flush function. It does the only "safe" 
thing you can expect it to do: Clean+Invalidate to POC because we use it 
for multiple things, clearing modified code among others:


ENTRY(__asm_flush_dcache_range)
    mrs x3, ctr_el0
    ubfx    x3, x3, #16, #4
    mov x2, #4
    lsl x2, x2, x3  /* cache line size */

    /* x2 <- minimal cache line size in cache system */
    sub x3, x2, #1
    bic x0, x0, x3
1:  dc  civac, x0   /* clean & invalidate data or unified 
cache */

    add x0, x0, x2
    cmp x0, x1
    b.lo    1b
    dsb sy
    ret
ENDPROC(__asm_flush_dcache_range)


Looking at the "dc civac" call, we find this documentation page from 
ARM: 
https://developer.arm.com/documentation/ddi0601/2022-03/AArch64-Instructions/DC-CIVAC--Data-or-unified-Cache-line-Clean-and-Invalidate-by-VA-to-PoC


This says we're writing any dirtyness of this cache line up to the POC 
and then invalidate (remove the cache line) also up to POC. That means 
when you look at a typical SBC, this will either be L2 or system level 
cache. Every read afterwards needs to go and pull it all the way back to 
L1 to modify it (or not) on the next character write and then flush it 
again.


Even worse: Because of the invalidate, we may even evict it from caches 
that the display controller uses to read the frame buffer. So depending 
on the SoC's cache topology and implementation, we may force the display 
controller to refetch the full FB content on its next screen refresh cycle.


I faintly remember that I tried to experiment with CVAC instead to only 
flush without invalidating. I don't fully remember the results anymore 
though. I believe CVAC just behaved identical to CIVAC on the A53 
platform I was working on. And then I looked at Cortex-A53 errata like 
[1] and just accepted that doing anything but restricting the flushing 
range is a waste of time :)



Alex


[1] 
https://patchwork.kernel.org/project/xen-devel/patch/1462466065-30212-14-git-send-email-julien.gr...@arm.com/





Re: [PATCH v5 00/13] Add video damage tracking

2023-08-21 Thread Simon Glass
Hi Alex,

On Mon, 21 Aug 2023 at 13:33, Alexander Graf  wrote:
>
>
> On 21.08.23 21:11, Simon Glass wrote:
> > Hi Alper,
> >
> > On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  
> > wrote:
> >> This is a rebase of Alexander Graf's video damage tracking series, with
> >> some tests and other changes. The original cover letter is as follows:
> >>
> >>> This patch set speeds up graphics output on ARM by a factor of 60x.
> >>>
> >>> On most ARM SBCs, we keep the frame buffer in DRAM and map it as cached,
> >>> but need it accessible by the display controller which reads directly
> >>> from a later point of consistency. Hence, we flush the frame buffer to
> >>> DRAM on every change. The full frame buffer.
> > It should not, see below.
> >
> >>> Unfortunately, with the advent of 4k displays, we are seeing frame buffers
> >>> that can take a while to flush out. This was reported by Da Xue with grub,
> >>> which happily print 1000s of spaces on the screen to draw a menu. Every
> >>> printed space triggers a cache flush.
> > That is a bug somewhere in EFI.
>
>
> Unfortunately not :). You may call it a bug in grub: It literally prints
> over space characters for every character in its menu that it wants
> cleared. On every text screen draw.
>
> This wouldn't be a big issue if we only flush the reactangle that gets
> modified. But without this patch set, we're flushing the full DRAM
> buffer on every u-boot text console character write, which means for
> every character (as that's the only API UEFI has).
>
> As a nice side effect, we speed up the normal U-Boot text console as
> well with this patch set, because even "normal" text prints that write
> for example a single line of text on the screen today flush the full
> frame buffer to DRAM.

No, I mean that it is a bug that U-Boot (apparently) flushes the cache
after every character. It doesn't do that for normal character output
and I don't think it makes sense to do it for EFI either.

>
>
> >
> >>> This patch set implements the easiest mitigation against this problem:
> >>> Damage tracking. We remember the lowest common denominator region that was
> >>> touched since the last video_sync() call and only flush that. The most
> >>> typical writer to the frame buffer is the video console, which always
> >>> writes rectangles of characters on the screen and syncs afterwards.
> >>>
> >>> With this patch set applied, we reduce drawing a large grub menu (with
> >>> serial console attached for size information) on an RK3399-ROC system
> >>> at 1440p from 55 seconds to less than 1 second.
> >>>
> >>> Version 2 also implements VIDEO_COPY using this mechanism, reducing its
> >>> overhead compared to before as well. So even x86 systems should be faster
> >>> with this now :).
> >>>
> >>>
> >>> Alternatives considered:
> >>>
> >>>1) Lazy sync - Sandbox does this. It only calls video_sync(true) ever
> >>>   so often. We are missing timers to do this generically.
> >>>
> >>>2) Double buffering - We could try to identify whether anything changed
> >>>   at all and only draw to the FB if it did. That would require
> >>>   maintaining a second buffer that we need to scan.
> >>>
> >>>3) Text buffer - Maintain a buffer of all text printed on the screen 
> >>> with
> >>>   respective location. Don't write if the old and new character are
> >>>   identical. This would limit applicability to text only and is an
> >>>   optimization on top of this patch set.
> >>>
> >>>4) Hash screen lines - Create a hash (sha256?) over every line when it
> >>>   changes. Only flush when it does. I'm not sure if this would waste
> >>>   more time, memory and cache than the current approach. It would make
> >>>   full screen updates much more expensive.
> > 5) Fix the bug mentioned above?
> >
> >> Changes in v5:
> >> - Add patch "video: test: Split copy frame buffer check into a function"
> >> - Add patch "video: test: Support checking copy frame buffer contents"
> >> - Add patch "video: test: Test partial updates of hardware frame buffer"
> >> - Use xstart, ystart, xend, yend as names for damage region
> >> - Document damage struct and fields in struct video_priv comment
> >> - Return void from video_damage()
> >> - Fix undeclared priv error in video_sync()
> >> - Drop unused headers from video-uclass.c
> >> - Use IS_ENABLED() instead of CONFIG_IS_ENABLED()
> >> - Call video_damage() also in video_fill_part()
> >> - Use met->baseline instead of priv->baseline
> >> - Use fontdata->height/width instead of VIDEO_FONT_HEIGHT/WIDTH
> >> - Update console_rotate.c video_damage() calls to pass video tests
> >> - Remove mention about not having minimal damage for console_rotate.c
> >> - Add patch "video: test: Test video damage tracking via vidconsole"
> >> - Document new vdev field in struct efi_gop_obj comment
> >> - Remove video_sync_copy() also from video_fill(), video_fill_part()
> >> - Fix memmove() calls by removing the extra dev argument
> >> - Call 

[PATCH v2] schemas: Add a schema for memory map

2023-08-21 Thread Simon Glass
The Devicespec specification skips over handling of a logical view of
the memory map, pointing users to the UEFI specification.

It is common to split firmware into 'Platform Init', which does the
initial hardware setup and a "Payload" which selects the OS to be booted.
Thus an handover interface is required between these two pieces.

Where UEFI boot-time services are not available, but UEFI firmware is
present on either side of this interface, information about memory usage
and attributes must be presented to the "Payload" in some form.

This aims to provide an initial schema for this mapping.

Note that this is separate from the existing /memory and /reserved-memory
nodes, since it is mostly concerned with what the memory is used for. It
may cover only a small fraction of available memory, although it could be
used to signal which area of memory has ECC.

For now, no attempt is made to create an exhaustive binding, so there are
some example types lists. This can be completed once this has passed
initial review.

Signed-off-by: Simon Glass 
---

Changes in v2:
- Reword commit message

 dtschema/schemas/memory-map.yaml | 51 
 1 file changed, 51 insertions(+)
 create mode 100644 dtschema/schemas/memory-map.yaml

diff --git a/dtschema/schemas/memory-map.yaml b/dtschema/schemas/memory-map.yaml
new file mode 100644
index 000..97e531e
--- /dev/null
+++ b/dtschema/schemas/memory-map.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: BSD-2-Clause
+# Copyright 2023 Google LLC
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/memory-map.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: /memory-map nodes
+description: |
+  Common properties always required in /memory-map nodes. These nodes are
+  intended to resolve the nonchalant clause 3.4.1 ("/memory node and UEFI")
+  in the Devicetree Specification.
+
+maintainers:
+  - Simon Glass 
+
+properties:
+  $nodename:
+const: '/'
+  usage:
+$ref: /schemas/types.yaml#/definitions/string
+description: |
+  Describes the usage of the memory region, e.g.:
+
+"acpi-reclaim", "acpi-nvs", "bootcode", "bootdata", "bootdata",
+"runtime-code", "runtime-data"
+  attr:
+$ref: /schemas/types.yaml#/definitions/string-array
+description: |
+  Attributes possessed by this memory region:
+
+"single-bit-ecc" - supports single-bit ECC
+"multi-bit-ecc" - supports multiple-bit ECC
+"no-ecc" - non-ECC memory
+
+patternProperties:
+  "^([a-z][a-z0-9\\-]+@[0-9a-f]+)?$":
+type: object
+additionalProperties: false
+
+properties:
+  reg:
+minItems: 1
+maxItems: 1024
+
+required:
+  - reg
+
+additionalProperties: true
+
+...
-- 
2.42.0.rc1.204.g551eb34607-goog



Re: Doc style for method functions

2023-08-21 Thread Heinrich Schuchardt

On 18.08.23 16:59, Simon Glass wrote:

Hi Heinrich,

On Thu, 17 Aug 2023 at 10:36, Heinrich Schuchardt  wrote:


On 16.08.23 19:47, Simon Glass wrote:

Hi Jonathan,

On Wed, 16 Aug 2023 at 11:15, Jonathan Corbet  wrote:


Simon Glass  writes:


Hi Jonathan,

I would like to do something like this:

struct part_driver {
 /**
  * get_info() - Get information about a partition

^ causes error

  *
  * @desc: Block device descriptor
  * @part: Partition number (1 = first)
  * @info: Returns partition information
  */
 int (*get_info)(struct blk_desc *desc, int part, struct
disk_partition *info);
...
};

But this gives:

scripts/kernel-doc:292:
 print STDERR "Incorrect use of kernel-doc format: $_";

Without the brackets on get_info() it works OK. What is the purpose of
that check, please?


That's how the kerneldoc syntax was defined, well before my time as the
maintainer.  This could be relaxed, I guess, but one would have to look
at the parsing code to be sure that the right thing happens all the way
through the process.  I'm not entirely sure it's worth it...


I see. It is inconsistent, since we use () after normal functions.

I think I can fix it just by removing that check.

Regards,
Simon


If the structure element in not a function pointer, we probably still
want to forbid adding parentheses. Just dropping the check might not be
the solution.


Is that the purpose of this check? Is it likely that someone would add
a bracket immediately after a variable?


We don't want anything but a colon ':' after a structure member name.
This excludes white space, parentheses (), brackets[], and braces {} and
a lot more.

A structure member name relating to a function pointer should be the
only exception. Here we expect the parentheses and the colon to follow
immediately without white space.

Best regards

Heinrich




Regards,
Simon




[PATCH] configs: set CONFIG_LMB_MAX_REGIONS=64 for MT7988 boards

2023-08-21 Thread Daniel Golle
Similar to MT7981 and MT7986 also MT7988 can have a high number of
reserved-memory regions used by the various hardware offloading
subsystems.

Raise CONFIG_LMB_MAX_REGIONS to 64 to avoid errors when trying to boot
Linux with more then 6 reserved regions:

ERROR: reserving fdt memory region failed (addr=4f70 size=24 flags=4)
ERROR: reserving fdt memory region failed (addr=15194000 size=1000 flags=4)
ERROR: reserving fdt memory region failed (addr=15294000 size=1000 flags=4)
ERROR: reserving fdt memory region failed (addr=15394000 size=1000 flags=4)
ERROR: Failed to allocate 0xb161 bytes below 0x8000.
device tree - allocation error

Fixes: bc4adc97cfb ("board: mediatek: add MT7988 reference boards")
Reported-by: Lorenzo Bianconi 
Signed-off-by: Daniel Golle 
---
 configs/mt7988_rfb_defconfig| 1 +
 configs/mt7988_sd_rfb_defconfig | 1 +
 2 files changed, 2 insertions(+)

diff --git a/configs/mt7988_rfb_defconfig b/configs/mt7988_rfb_defconfig
index dc97bb36ea..ced52edecf 100644
--- a/configs/mt7988_rfb_defconfig
+++ b/configs/mt7988_rfb_defconfig
@@ -81,3 +81,4 @@ CONFIG_MTK_SPIM=y
 CONFIG_LZO=y
 CONFIG_HEXDUMP=y
 # CONFIG_EFI_LOADER is not set
+CONFIG_LMB_MAX_REGIONS=64
diff --git a/configs/mt7988_sd_rfb_defconfig b/configs/mt7988_sd_rfb_defconfig
index 421999da86..670f5eae18 100644
--- a/configs/mt7988_sd_rfb_defconfig
+++ b/configs/mt7988_sd_rfb_defconfig
@@ -69,3 +69,4 @@ CONFIG_MTK_SPIM=y
 CONFIG_LZO=y
 CONFIG_HEXDUMP=y
 # CONFIG_EFI_LOADER is not set
+CONFIG_LMB_MAX_REGIONS=64
-- 
2.41.0


[PATCH] imx: imx8: ahab: refactor do_ahab_close command

2023-08-21 Thread Oleksandr Suvorov
From: Igor Opaniuk 

Move an OEM closing logic to ahab_close() function to be able to use
it directly without calling a u-boot command.

Signed-off-by: Igor Opaniuk 
Co-developed-by: Oleksandr Suvorov 
Signed-off-by: Oleksandr Suvorov 
---

 arch/arm/include/asm/arch-imx8/sys_proto.h |  1 +
 arch/arm/mach-imx/imx8/ahab.c  | 45 ++
 2 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/arch/arm/include/asm/arch-imx8/sys_proto.h 
b/arch/arm/include/asm/arch-imx8/sys_proto.h
index e7625c42985..405e9bd3d81 100644
--- a/arch/arm/include/asm/arch-imx8/sys_proto.h
+++ b/arch/arm/include/asm/arch-imx8/sys_proto.h
@@ -23,6 +23,7 @@ struct pass_over_info_t {
 
 extern unsigned long boot_pointer[];
 void build_info(void);
+int ahab_close(void);
 int print_bootinfo(void);
 int sc_pm_setup_uart(sc_rsrc_t uart_rsrc, sc_pm_clock_rate_t clk_rate);
 int imx8_power_domain_lookup_name(const char *name,
diff --git a/arch/arm/mach-imx/imx8/ahab.c b/arch/arm/mach-imx/imx8/ahab.c
index b58b14ca9b4..a7891bf8d98 100644
--- a/arch/arm/mach-imx/imx8/ahab.c
+++ b/arch/arm/mach-imx/imx8/ahab.c
@@ -340,6 +340,32 @@ static int do_ahab_status(struct cmd_tbl *cmdtp, int flag, 
int argc,
return 0;
 }
 
+int ahab_close(void)
+{
+   int err;
+   u16 lc;
+
+   err = sc_seco_chip_info(-1, , NULL, NULL, NULL);
+   if (err != SC_ERR_NONE) {
+   printf("Error in get lifecycle\n");
+   return -EIO;
+   }
+
+   if (lc != 0x20) {
+   puts("Current lifecycle is NOT NXP closed, can't move to OEM 
closed\n");
+   display_life_cycle(lc);
+   return -EPERM;
+   }
+
+   err = sc_seco_forward_lifecycle(-1, 16);
+   if (err != SC_ERR_NONE) {
+   printf("Error in forward lifecycle to OEM closed\n");
+   return -EIO;
+   }
+
+   return 0;
+}
+
 static int confirm_close(void)
 {
puts("Warning: Please ensure your sample is in NXP closed state, "
@@ -361,27 +387,14 @@ static int do_ahab_close(struct cmd_tbl *cmdtp, int flag, 
int argc,
 {
int confirmed = argc >= 2 && !strcmp(argv[1], "-y");
int err;
-   u16 lc;
 
if (!confirmed && !confirm_close())
return -EACCES;
 
-   err = sc_seco_chip_info(-1, , NULL, NULL, NULL);
+   err = ahab_close();
if (err) {
-   printf("Error in get lifecycle\n");
-   return -EIO;
-   }
-
-   if (lc != 0x20) {
-   puts("Current lifecycle is NOT NXP closed, can't move to OEM 
closed\n");
-   display_life_cycle(lc);
-   return -EPERM;
-   }
-
-   err = sc_seco_forward_lifecycle(-1, 16);
-   if (err) {
-   printf("Error in forward lifecycle to OEM closed\n");
-   return -EIO;
+   printf("Change to OEM closed failed\n");
+   return err;
}
 
printf("Change to OEM closed successfully\n");
-- 
2.41.0



Re: [PATCH v5 00/13] Add video damage tracking

2023-08-21 Thread Alexander Graf



On 21.08.23 21:11, Simon Glass wrote:

Hi Alper,

On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  wrote:

This is a rebase of Alexander Graf's video damage tracking series, with
some tests and other changes. The original cover letter is as follows:


This patch set speeds up graphics output on ARM by a factor of 60x.

On most ARM SBCs, we keep the frame buffer in DRAM and map it as cached,
but need it accessible by the display controller which reads directly
from a later point of consistency. Hence, we flush the frame buffer to
DRAM on every change. The full frame buffer.

It should not, see below.


Unfortunately, with the advent of 4k displays, we are seeing frame buffers
that can take a while to flush out. This was reported by Da Xue with grub,
which happily print 1000s of spaces on the screen to draw a menu. Every
printed space triggers a cache flush.

That is a bug somewhere in EFI.



Unfortunately not :). You may call it a bug in grub: It literally prints 
over space characters for every character in its menu that it wants 
cleared. On every text screen draw.


This wouldn't be a big issue if we only flush the reactangle that gets 
modified. But without this patch set, we're flushing the full DRAM 
buffer on every u-boot text console character write, which means for 
every character (as that's the only API UEFI has).


As a nice side effect, we speed up the normal U-Boot text console as 
well with this patch set, because even "normal" text prints that write 
for example a single line of text on the screen today flush the full 
frame buffer to DRAM.






This patch set implements the easiest mitigation against this problem:
Damage tracking. We remember the lowest common denominator region that was
touched since the last video_sync() call and only flush that. The most
typical writer to the frame buffer is the video console, which always
writes rectangles of characters on the screen and syncs afterwards.

With this patch set applied, we reduce drawing a large grub menu (with
serial console attached for size information) on an RK3399-ROC system
at 1440p from 55 seconds to less than 1 second.

Version 2 also implements VIDEO_COPY using this mechanism, reducing its
overhead compared to before as well. So even x86 systems should be faster
with this now :).


Alternatives considered:

   1) Lazy sync - Sandbox does this. It only calls video_sync(true) ever
  so often. We are missing timers to do this generically.

   2) Double buffering - We could try to identify whether anything changed
  at all and only draw to the FB if it did. That would require
  maintaining a second buffer that we need to scan.

   3) Text buffer - Maintain a buffer of all text printed on the screen with
  respective location. Don't write if the old and new character are
  identical. This would limit applicability to text only and is an
  optimization on top of this patch set.

   4) Hash screen lines - Create a hash (sha256?) over every line when it
  changes. Only flush when it does. I'm not sure if this would waste
  more time, memory and cache than the current approach. It would make
  full screen updates much more expensive.

5) Fix the bug mentioned above?


Changes in v5:
- Add patch "video: test: Split copy frame buffer check into a function"
- Add patch "video: test: Support checking copy frame buffer contents"
- Add patch "video: test: Test partial updates of hardware frame buffer"
- Use xstart, ystart, xend, yend as names for damage region
- Document damage struct and fields in struct video_priv comment
- Return void from video_damage()
- Fix undeclared priv error in video_sync()
- Drop unused headers from video-uclass.c
- Use IS_ENABLED() instead of CONFIG_IS_ENABLED()
- Call video_damage() also in video_fill_part()
- Use met->baseline instead of priv->baseline
- Use fontdata->height/width instead of VIDEO_FONT_HEIGHT/WIDTH
- Update console_rotate.c video_damage() calls to pass video tests
- Remove mention about not having minimal damage for console_rotate.c
- Add patch "video: test: Test video damage tracking via vidconsole"
- Document new vdev field in struct efi_gop_obj comment
- Remove video_sync_copy() also from video_fill(), video_fill_part()
- Fix memmove() calls by removing the extra dev argument
- Call video_sync() before checking copy_fb in video tests
- Imply VIDEO_DAMAGE for video drivers instead of selecting it
- Imply VIDEO_DAMAGE also for VIDEO_TIDSS

v4: https://lore.kernel.org/all/20230103215004.22646-1-ag...@csgraf.de/

Changes in v4:
- Move damage clear to patch "dm: video: Add damage tracking API"
- Simplify first damage logic
- Remove VIDEO_DAMAGE default for ARM
- Skip damage on EfiBltVideoToBltBuffer
- Add patch "video: Always compile cache flushing code"
- Add patch "video: Enable VIDEO_DAMAGE for drivers that need it"

v3: https://lore.kernel.org/all/20221230195828.88134-1-ag...@csgraf.de/

Changes in v3:
- Adapt to always assume DM is used
- Adapt to always 

Re: [PATCH 3/4] CI: Combine tools-only and envtools jobs

2023-08-21 Thread Simon Glass
On Sun, 20 Aug 2023 at 11:31, Tom Rini  wrote:
>
> These jobs are to confirm specific build targets, on a Linux host.  We
> can safely combine these two build tests, with a make mrproper in
> between.
>
> Signed-off-by: Tom Rini 
> ---
>  .azure-pipelines.yml | 13 ++---
>  .gitlab-ci.yml   | 12 
>  2 files changed, 6 insertions(+), 19 deletions(-)


Re: [PATCH v5 09/13] efi_loader: GOP: Add damage notification on BLT

2023-08-21 Thread Simon Glass
On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  wrote:
>
> From: Alexander Graf 
>
> Now that we have a damage tracking API, let's populate damage done by
> UEFI payloads when they BLT data onto the screen.
>
> Signed-off-by: Alexander Graf 
> Reported-by: Da Xue 
> Reviewed-by: Heinrich Schuchardt 
> [Alper: Add struct comment for new member]
> Signed-off-by: Alper Nebi Yasak 
> ---
>
> Changes in v5:
> - Document new vdev field in struct efi_gop_obj comment
>
> Changes in v4:
> - Skip damage on EfiBltVideoToBltBuffer
>
> Changes in v3:
> - Adapt to always assume DM is used
>
> Changes in v2:
> - Remove ifdefs from gop
>
>  lib/efi_loader/efi_gop.c | 7 +++
>  1 file changed, 7 insertions(+)
>

Reviewed-by: Simon Glass 


Re: [PATCH v5 00/13] Add video damage tracking

2023-08-21 Thread Simon Glass
Hi Alper,

On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  wrote:
>
> This is a rebase of Alexander Graf's video damage tracking series, with
> some tests and other changes. The original cover letter is as follows:
>
> > This patch set speeds up graphics output on ARM by a factor of 60x.
> >
> > On most ARM SBCs, we keep the frame buffer in DRAM and map it as cached,
> > but need it accessible by the display controller which reads directly
> > from a later point of consistency. Hence, we flush the frame buffer to
> > DRAM on every change. The full frame buffer.

It should not, see below.

> >
> > Unfortunately, with the advent of 4k displays, we are seeing frame buffers
> > that can take a while to flush out. This was reported by Da Xue with grub,
> > which happily print 1000s of spaces on the screen to draw a menu. Every
> > printed space triggers a cache flush.

That is a bug somewhere in EFI.

> >
> > This patch set implements the easiest mitigation against this problem:
> > Damage tracking. We remember the lowest common denominator region that was
> > touched since the last video_sync() call and only flush that. The most
> > typical writer to the frame buffer is the video console, which always
> > writes rectangles of characters on the screen and syncs afterwards.
> >
> > With this patch set applied, we reduce drawing a large grub menu (with
> > serial console attached for size information) on an RK3399-ROC system
> > at 1440p from 55 seconds to less than 1 second.
> >
> > Version 2 also implements VIDEO_COPY using this mechanism, reducing its
> > overhead compared to before as well. So even x86 systems should be faster
> > with this now :).
> >
> >
> > Alternatives considered:
> >
> >   1) Lazy sync - Sandbox does this. It only calls video_sync(true) ever
> >  so often. We are missing timers to do this generically.
> >
> >   2) Double buffering - We could try to identify whether anything changed
> >  at all and only draw to the FB if it did. That would require
> >  maintaining a second buffer that we need to scan.
> >
> >   3) Text buffer - Maintain a buffer of all text printed on the screen with
> >  respective location. Don't write if the old and new character are
> >  identical. This would limit applicability to text only and is an
> >  optimization on top of this patch set.
> >
> >   4) Hash screen lines - Create a hash (sha256?) over every line when it
> >  changes. Only flush when it does. I'm not sure if this would waste
> >  more time, memory and cache than the current approach. It would make
> >  full screen updates much more expensive.

5) Fix the bug mentioned above?

>
> Changes in v5:
> - Add patch "video: test: Split copy frame buffer check into a function"
> - Add patch "video: test: Support checking copy frame buffer contents"
> - Add patch "video: test: Test partial updates of hardware frame buffer"
> - Use xstart, ystart, xend, yend as names for damage region
> - Document damage struct and fields in struct video_priv comment
> - Return void from video_damage()
> - Fix undeclared priv error in video_sync()
> - Drop unused headers from video-uclass.c
> - Use IS_ENABLED() instead of CONFIG_IS_ENABLED()
> - Call video_damage() also in video_fill_part()
> - Use met->baseline instead of priv->baseline
> - Use fontdata->height/width instead of VIDEO_FONT_HEIGHT/WIDTH
> - Update console_rotate.c video_damage() calls to pass video tests
> - Remove mention about not having minimal damage for console_rotate.c
> - Add patch "video: test: Test video damage tracking via vidconsole"
> - Document new vdev field in struct efi_gop_obj comment
> - Remove video_sync_copy() also from video_fill(), video_fill_part()
> - Fix memmove() calls by removing the extra dev argument
> - Call video_sync() before checking copy_fb in video tests
> - Imply VIDEO_DAMAGE for video drivers instead of selecting it
> - Imply VIDEO_DAMAGE also for VIDEO_TIDSS
>
> v4: https://lore.kernel.org/all/20230103215004.22646-1-ag...@csgraf.de/
>
> Changes in v4:
> - Move damage clear to patch "dm: video: Add damage tracking API"
> - Simplify first damage logic
> - Remove VIDEO_DAMAGE default for ARM
> - Skip damage on EfiBltVideoToBltBuffer
> - Add patch "video: Always compile cache flushing code"
> - Add patch "video: Enable VIDEO_DAMAGE for drivers that need it"
>
> v3: https://lore.kernel.org/all/20221230195828.88134-1-ag...@csgraf.de/
>
> Changes in v3:
> - Adapt to always assume DM is used
> - Adapt to always assume DM is used
> - Make VIDEO_COPY always select VIDEO_DAMAGE
>
> v2: https://lore.kernel.org/all/20220609225921.62462-1-ag...@csgraf.de/
>
> Changes in v2:
> - Remove ifdefs
> - Fix ranges in truetype target
> - Limit rotate to necessary damage
> - Remove ifdefs from gop
> - Fix dcache range; we were flushing too much before
> - Add patch "video: Use VIDEO_DAMAGE for VIDEO_COPY"
>
> v1: https://lore.kernel.org/all/20220606234336.5021-1-ag...@csgraf.de/
>
> Alexander Graf (9):
>   dm: 

Re: [PATCH v1 1/2] drivers: firmware: introduce Meson Secure Monitor driver

2023-08-21 Thread Simon Glass
Hi Neil,

On Mon, 21 Aug 2023 at 03:16, neil.armstr...@linaro.org
 wrote:
>
> Hi,
>
> On 16/07/2023 01:40, Simon Glass wrote:
> > Hi,
> >
> > On Thu, 13 Jul 2023 at 23:30, AKASHI Takahiro
> >  wrote:
> >>
> >> On Tue, Jul 11, 2023 at 01:13:29PM -0600, Simon Glass wrote:
> >>> +AKASHI Takahiro
> >>
> >> Me?
> >
> > Yes, I'm asking for your help to try to clean this stuff up.
>
> The thread is long and hard to answer directly, but as AKASHI
> said there's no point to add a SMC class since it's only the message
> passing instruction, and there's no point using remoteproc since the
> firmware runs on a separate secure state of the same CPU.
>
> And I don't see how we can actually define a finite set of ops because
> none of the secure firmware interfaces has even similar functions.
>
> So a new UCLASS for each firmware interface should be added, not sure
> this is scalable or required since those firmwares are mainly SoC or
> vendor specific, except the PSCI or other ARM specific interfaces of course.
>
> So I think UCLASS_FIRMWARE is good enough since it avoids using UCLASS_MISC,
> but it should be probably documented somewhere that the ops are implementation
> defined.

Yes it needs docs...but what exactly is the 'firmware' uclass? I
assumed it was for loading firmware into a device, but it seems that
it is something else?

Perhaps we should have a UCLASS_SVC (supervisor call) or something
like that, rather than continuing with firmware?

[..]

Regards,
Simon


Re: [PATCH v5 04/13] dm: video: Add damage tracking API

2023-08-21 Thread Simon Glass
On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  wrote:
>
> From: Alexander Graf 
>
> We are going to introduce image damage tracking to fasten up screen
> refresh on large displays. This patch adds damage tracking for up to
> one rectangle of the screen which is typically enough to hold blt or
> text print updates. Callers into this API and a reduced dcache flush
> code path will follow in later patches.
>
> Signed-off-by: Alexander Graf 
> Reported-by: Da Xue 
> [Alper: Use xstart/yend, document new fields, return void from
> video_damage(), declare priv, drop headers, use IS_ENABLED()]
> Co-developed-by: Alper Nebi Yasak 
> Signed-off-by: Alper Nebi Yasak 
> ---
>
> Changes in v5:
> - Use xstart, ystart, xend, yend as names for damage region
> - Document damage struct and fields in struct video_priv comment
> - Return void from video_damage()
> - Fix undeclared priv error in video_sync()
> - Drop unused headers from video-uclass.c
> - Use IS_ENABLED() instead of CONFIG_IS_ENABLED()
>
> Changes in v4:
> - Move damage clear to patch "dm: video: Add damage tracking API"
> - Simplify first damage logic
> - Remove VIDEO_DAMAGE default for ARM
>
> Changes in v3:
> - Adapt to always assume DM is used
>
> Changes in v2:
> - Remove ifdefs
>
>  drivers/video/Kconfig| 13 
>  drivers/video/video-uclass.c | 41 +---
>  include/video.h  | 32 ++--
>  3 files changed, 81 insertions(+), 5 deletions(-)
>

Reviewed-by: Simon Glass 

But I suggest an empty static inline in the case where the feature is disabled?


Re: [PATCH v5 11/13] video: Use VIDEO_DAMAGE for VIDEO_COPY

2023-08-21 Thread Simon Glass
Hi Alper,

On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  wrote:
>
> From: Alexander Graf 
>
> CONFIG_VIDEO_COPY implemented a range-based copying mechanism: If we
> print a single character, it will always copy the full range of bytes
> from the top left corner of the character to the lower right onto the
> uncached frame buffer. This includes pretty much the full line contents
> of the printed character.
>
> Since we now have proper damage tracking, let's make use of that to reduce
> the amount of data we need to copy. With this patch applied, we will only
> copy the tiny rectangle surrounding characters when we print them,
> speeding up the video console.

I suppose for rotated consoles it copies whole lines, but otherwise it
does a lot of small copies?

>
> After this, changes to the main frame buffer are not immediately copied
> to the copy frame buffer, but postponed until the next video device
> sync. So issue an explicit sync before inspecting the copy frame buffer
> contents for the video tests.

So how does the sync get done in this case?

>
> Signed-off-by: Alexander Graf 
> [Alper: Rebase for fontdata->height/w, fill_part(), fix memmove(dev),
> drop from defconfig, use damage.xstart/yend, use IS_ENABLED(),
> call video_sync() before copy_fb check, update video_copy test]
> Co-developed-by: Alper Nebi Yasak 
> Signed-off-by: Alper Nebi Yasak 
> ---
>
> Changes in v5:
> - Remove video_sync_copy() also from video_fill(), video_fill_part()
> - Fix memmove() calls by removing the extra dev argument
> - Call video_sync() before checking copy_fb in video tests
> - Use xstart, ystart, xend, yend as names for damage region
> - Use met->baseline instead of priv->baseline
> - Use fontdata->height/width instead of VIDEO_FONT_HEIGHT/WIDTH
> - Use xstart, ystart, xend, yend as names for damage region
> - Use IS_ENABLED() instead of CONFIG_IS_ENABLED()
> - Drop VIDEO_DAMAGE from sandbox defconfig added in a new patch
> - Update dm_test_video_copy test added in a new patch
>
> Changes in v3:
> - Make VIDEO_COPY always select VIDEO_DAMAGE
>
> Changes in v2:
> - Add patch "video: Use VIDEO_DAMAGE for VIDEO_COPY"
>
>  configs/sandbox_defconfig |  1 -
>  drivers/video/Kconfig |  5 ++
>  drivers/video/console_normal.c| 13 +
>  drivers/video/console_rotate.c| 44 +++---
>  drivers/video/console_truetype.c  | 16 +
>  drivers/video/vidconsole-uclass.c | 16 -
>  drivers/video/video-uclass.c  | 97 ---
>  drivers/video/video_bmp.c |  7 ---
>  include/video.h   | 37 
>  include/video_console.h   | 52 -
>  test/dm/video.c   |  3 +-
>  11 files changed, 43 insertions(+), 248 deletions(-)
>
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index 51b820f13121..259f31f26cee 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -307,7 +307,6 @@ CONFIG_USB_ETH_CDC=y
>  CONFIG_VIDEO=y
>  CONFIG_VIDEO_FONT_SUN12X22=y
>  CONFIG_VIDEO_COPY=y
> -CONFIG_VIDEO_DAMAGE=y
>  CONFIG_CONSOLE_ROTATION=y
>  CONFIG_CONSOLE_TRUETYPE=y
>  CONFIG_CONSOLE_TRUETYPE_CANTORAONE=y
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 97f494a1340b..b3fbd9d7d9ca 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -83,11 +83,14 @@ config VIDEO_PCI_DEFAULT_FB_SIZE
>
>  config VIDEO_COPY
> bool "Enable copying the frame buffer to a hardware copy"
> +   select VIDEO_DAMAGE
> help
>   On some machines (e.g. x86), reading from the frame buffer is very
>   slow because it is uncached. To improve performance, this feature
>   allows the frame buffer to be kept in cached memory (allocated by
>   U-Boot) and then copied to the hardware frame-buffer as needed.
> + It uses the VIDEO_DAMAGE feature to keep track of regions to copy
> + and will only copy actually touched regions.
>
>   To use this, your video driver must set @copy_base in
>   struct video_uc_plat.
> @@ -105,6 +108,8 @@ config VIDEO_DAMAGE
>   regions of the frame buffer that were modified before, speeding up
>   screen refreshes significantly.
>
> + It is also used by VIDEO_COPY to identify which regions changed.
> +
>  config BACKLIGHT_PWM
> bool "Generic PWM based Backlight Driver"
> depends on BACKLIGHT && DM_PWM
> diff --git a/drivers/video/console_normal.c b/drivers/video/console_normal.c
> index a19ce6a2bc11..c44aa09473a3 100644
> --- a/drivers/video/console_normal.c
> +++ b/drivers/video/console_normal.c
> @@ -35,10 +35,6 @@ static int console_set_row(struct udevice *dev, uint row, 
> int clr)
> fill_pixel_and_goto_next(, clr, pbytes, pbytes);
> end = dst;
>
> -   ret = vidconsole_sync_copy(dev, line, end);
> -   if (ret)
> -   return ret;
> -
> video_damage(dev->parent,

Re: [PATCH v5 07/13] video: test: Test video damage tracking via vidconsole

2023-08-21 Thread Simon Glass
On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  wrote:
>
> With VIDEO_DAMAGE, the video uclass tracks updated regions of the frame
> buffer in order to avoid unnecessary work during a video sync. Enable
> the config in sandbox and add a test for it, by printing strings at a
> few locations and checking the tracked region.
>
> Signed-off-by: Alper Nebi Yasak 
> ---
> This is hard to test because most things issue video syncs that process
> and reset the damaged region.
>
> Changes in v5:
> - Add patch "video: test: Test video damage tracking via vidconsole"
>
>  configs/sandbox_defconfig |  1 +
>  test/dm/video.c   | 56 +++
>  2 files changed, 57 insertions(+)

Reviewed-by: Simon Glass 


Re: [PATCH v5 06/13] vidconsole: Add damage notifications to all vidconsole drivers

2023-08-21 Thread Simon Glass
On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  wrote:
>
> From: Alexander Graf 
>
> Now that we have a damage tracking API, let's populate damage done by
> vidconsole drivers. We try to declare as little memory as damaged as
> possible.
>
> Signed-off-by: Alexander Graf 
> Reported-by: Da Xue 
> [Alper: Rebase for met->baseline, fontdata->height/width, make rotated
> console_putc_xy() damages pass tests, edit patch message]
> Co-developed-by: Alper Nebi Yasak 
> Signed-off-by: Alper Nebi Yasak 
> ---
>
> Changes in v5:
> - Use met->baseline instead of priv->baseline
> - Use fontdata->height/width instead of VIDEO_FONT_HEIGHT/WIDTH
> - Update console_rotate.c video_damage() calls to pass video tests
> - Remove mention about not having minimal damage for console_rotate.c
>
> Changes in v2:
> - Fix ranges in truetype target
> - Limit rotate to necessary damage
>
>  drivers/video/console_normal.c   | 18 +++
>  drivers/video/console_rotate.c   | 54 
>  drivers/video/console_truetype.c | 21 +
>  drivers/video/video-uclass.c |  1 +
>  4 files changed, 94 insertions(+)
>

Reviewed-by: Simon Glass 

Suggest dropping the change to the final file


Re: [PATCH v5 13/13] video: Enable VIDEO_DAMAGE for drivers that need it

2023-08-21 Thread Simon Glass
On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  wrote:
>
> From: Alexander Graf 
>
> Some drivers call video_set_flush_dcache() to indicate that they want to
> have the dcache flushed for the frame buffer. These drivers benefit from
> our new video damage control, because we can reduce the amount of memory
> that gets flushed significantly.
>
> This patch enables video damage control for all device drivers that call
> video_set_flush_dcache() to make sure they benefit from it.
>
> Signed-off-by: Alexander Graf 
> [Alper: Add to VIDEO_TIDSS, imply instead of select]
> Co-developed-by: Alper Nebi Yasak 
> Signed-off-by: Alper Nebi Yasak 
> ---
>
> Changes in v5:
> - Imply VIDEO_DAMAGE for video drivers instead of selecting it
> - Imply VIDEO_DAMAGE also for VIDEO_TIDSS
>
> Changes in v4:
> - Add patch "video: Enable VIDEO_DAMAGE for drivers that need it"
>
>  arch/arm/mach-omap2/omap3/Kconfig | 1 +
>  arch/arm/mach-sunxi/Kconfig   | 1 +
>  drivers/video/Kconfig | 8 
>  drivers/video/exynos/Kconfig  | 1 +
>  drivers/video/imx/Kconfig | 1 +
>  drivers/video/meson/Kconfig   | 1 +
>  drivers/video/rockchip/Kconfig| 1 +
>  drivers/video/stm32/Kconfig   | 1 +
>  drivers/video/tegra20/Kconfig | 1 +
>  drivers/video/tidss/Kconfig   | 1 +
>  10 files changed, 17 insertions(+)

Reviewed-by: Simon Glass 


Re: [PATCH v5 10/13] video: Only dcache flush damaged lines

2023-08-21 Thread Simon Glass
Hi Alper,

On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  wrote:
>
> From: Alexander Graf 
>
> Now that we have a damage area tells us which parts of the frame buffer
> actually need updating, let's only dcache flush those on video_sync()
> calls. With this optimization in place, frame buffer updates - especially
> on large screen such as 4k displays - speed up significantly.
>
> Signed-off-by: Alexander Graf 
> Reported-by: Da Xue 
> [Alper: Use damage.xstart/yend, IS_ENABLED()]
> Co-developed-by: Alper Nebi Yasak 
> Signed-off-by: Alper Nebi Yasak 
> ---
>
> Changes in v5:
> - Use xstart, ystart, xend, yend as names for damage region
> - Use IS_ENABLED() instead of CONFIG_IS_ENABLED()
>
> Changes in v2:
> - Fix dcache range; we were flushing too much before
> - Remove ifdefs
>
>  drivers/video/video-uclass.c | 41 +++-
>  1 file changed, 36 insertions(+), 5 deletions(-)

This is a little strange, since flushing the whole cache will only
actually write out data that was actually written (to the display). Is
there a benefit to this patch, in terms of performance?

Regards,
Simon


Re: [PATCH v5 05/13] dm: video: Add damage notification on display fills

2023-08-21 Thread Simon Glass
On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  wrote:
>
> From: Alexander Graf 
>
> Let's report the video damage when we fill parts of the screen. This
> way we can later lazily flush only relevant regions to hardware.
>
> Signed-off-by: Alexander Graf 
> Reported-by: Da Xue 
> [Alper: Call video_damage() in video_fill_part(), edit commit message]
> Signed-off-by: Alper Nebi Yasak 
> ---
> Does video_fill_part() need a video_sync(dev, false) here?
>
> Changes in v5:
> - Call video_damage() also in video_fill_part()
>
>  drivers/video/video-uclass.c | 4 
>  1 file changed, 4 insertions(+)
>

Reviewed-by: Simon Glass 


Re: [PATCH v5 03/13] video: test: Test partial updates of hardware frame buffer

2023-08-21 Thread Simon Glass
On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  wrote:
>
> With VIDEO_COPY enabled, only the modified parts of the frame buffer are
> intended to be copied to the hardware. Add a test that checks this, by
> overwriting contents we prepared without telling the video uclass and
> then checking if the overwritten contents have been redrawn on the next
> sync.
>
> Signed-off-by: Alper Nebi Yasak 
> ---
>
> Changes in v5:
> - Add patch "video: test: Test partial updates of hardware frame buffer"
>
>  test/dm/video.c | 54 +
>  1 file changed, 54 insertions(+)

Reviewed-by: Simon Glass 


Re: [PATCH v5 01/13] video: test: Split copy frame buffer check into a function

2023-08-21 Thread Simon Glass
On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  wrote:
>
> While checking frame buffer contents, the video tests also check if the
> copy frame buffer contents match the main frame buffer. To test if only
> the modified regions are updated after a sync, we will need to create
> situations where the two are mismatched. Split this check into another
> function that we can skip calling, since we won't want it to error on
> those mismatched cases.
>
> Signed-off-by: Alper Nebi Yasak 
> ---
>
> Changes in v5:
> - Add patch "video: test: Split copy frame buffer check into a function"
>
>  test/dm/video.c | 69 +
>  1 file changed, 58 insertions(+), 11 deletions(-)
>

Reviewed-by: Simon Glass 


Re: [RESEND PATCH v3 0/3] rpi: Convert to standard boot

2023-08-21 Thread Simon Glass
Hi Peter,

On Sat, 19 Aug 2023 at 07:53, Peter Robinson  wrote:
>
> On Wed, Aug 16, 2023 at 4:08 PM Simon Glass  wrote:
> >
> > Hi Matthias, Peter,
> >
> > On Thu, 27 Jul 2023 at 15:54, Simon Glass  wrote:
> > >
> > > This series moves Raspberry Pi boards over to use standard boot.
> > >
> > > It also moves rpi over to use a text-based environment. Unfortunately it
> > > is not possible to empty the header file due to several CFG options.
> > >
> > > Fix the repeated "and and" while we are here.
> > >
> > > Note that this reduces rodata size by about 4.5KB. We could get another
> > >
> > > for a total image-size saving of about 15KB. This is mostly because
> > > HUSH_PARSER is not enabled anymore and the environment shrinks down by
> > > about 3.5K. Hush is not actually needed anymore, since standard boot does
> > > not use it. Also CMD_SYSBOOT is dropped since standard boot calls the
> > > pxe_utils code directly in that case.
> >
> > It has now been about 6 months since I sent the v1 series of this.
> > Even this v3 series is now out of date (in the cover letter).
> >
> > Is rpi still maintained?
>
> Yes, but we've had conversations about this in the past and snide
> comments and regressions don't enamor me to your patches...

I'm sorry for the snide comments.

>
> That said without seeing this email I actually started tested them yesterday.
>
> U-Boot isn't part of my $dayjob and I've had limited time of late,
> that's slowly getting better.

OK, good. We have seen quite a few regressions and bugs. I expect that
to continue for at least a few more releases, as more boards pick this
up.

Regards,
Simon


Re: [PATCH v5 02/13] video: test: Support checking copy frame buffer contents

2023-08-21 Thread Simon Glass
On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  wrote:
>
> The video tests have a helper function to generate a pseudo-digest of
> frame buffer contents, but it only does so for the main one. There is
> another check that the copy frame buffer is the same as that. But
> neither is enough to test if only the modified regions are copied to the
> copy frame buffer, since we will want the two to be different in very
> specific ways.
>
> Add a boolean argument to the existing helper function to indicate which
> frame buffer we want to inspect, and update the existing callers.
>
> Signed-off-by: Alper Nebi Yasak 
> ---
>
> Changes in v5:
> - Add patch "video: test: Support checking copy frame buffer contents"
>
>  test/dm/video.c | 76 ++---
>  1 file changed, 41 insertions(+), 35 deletions(-)
>

Reviewed-by: Simon Glass 


Re: [PATCH v1 0/4] Port gen_compile_commands.py from Linux to U-Boot

2023-08-21 Thread Simon Glass
Hi Joao,

On Sun, 20 Aug 2023 at 13:04, Joao Marcos Costa  wrote:
>
> Hello U-Boot community,
>
> I'm submitting a patch series that ports the gen_compile_commands.py
> script from the Linux kernel's sources to U-Boot. This script, originally
> located in scripts/clang-tools/gen_compile_commands.py, enables the
> generation of compile_commands.json files for improved code navigation
> and analysis. The series consists of four patches: the initial script
> import and the necessary modifications for U-Boot compatibility.
>
> Your feedback on these contributions would be greatly appreciated.
>
> Best regards,
>
> Joao Marcos Costa (4):
>   scripts: Port Linux's gen_compile_commands.py to U-Boot
>   scripts/gen_compile_commands.py: adapt _LINE_PATTERN
>   scripts/gen_compile_commands.py: fix docstring
>   scripts/gen_compile_commands.py: add acknowledgments
>
>  scripts/gen_compile_commands.py | 229 
>  1 file changed, 229 insertions(+)
>  create mode 100755 scripts/gen_compile_commands.py

Can you also please bring over the documentation for this feature?

Regards,
Simon


Re: [PATCH] configs: Enable CONFIG_DM_SCSI in am57xx_hs_evm_usb

2023-08-21 Thread Simon Glass
On Mon, 21 Aug 2023 at 08:53, Tom Rini  wrote:
>
> On Mon, Aug 21, 2023 at 08:59:10AM -0500, Andrew Davis wrote:
>
> > This should have already been enabled but was missed when converting the
> > base platform defconfig, fix this here.
> >
> > Fixes: 3c5aa6caccab ("configs: Enable CONFIG_BLK in am57xx_evm and 
> > am57xx_hs_evm")
> > Signed-off-by: Andrew Davis 
>
> Reviewed-by: Tom Rini 

Reviewed-by: Simon Glass 

Thank you


Re: [PATCH 4/4] CI: Drop some jobs we didn't really utilize

2023-08-21 Thread Simon Glass
On Sun, 20 Aug 2023 at 11:32, Tom Rini  wrote:
>
> - We have added more TODO/etc comments since this task was created and
>   never focused on removing them.
> - The output of sloccount isn't preserved or looked at, and if desired
>   should be in the release stats pages instead somehow.
> - The results of cppcheck aren't investigated and require modeling work
>   to be useful to start with.
>
> Signed-off-by: Tom Rini 
> ---
>  .azure-pipelines.yml| 32 
>  .gitlab-ci.yml  | 22 --
>  tools/docker/Dockerfile |  2 --
>  3 files changed, 56 deletions(-)

Reviewed-by: Simon Glass 


Re: [PATCH 2/4] Azure: Rework build the world jobs

2023-08-21 Thread Simon Glass
On Sun, 20 Aug 2023 at 11:31, Tom Rini  wrote:
>
> Now that we have 3600 minutes per build job, condense and rework things
> such that our overall time largely doesn't change, but we can also
> largely avoid having to re-tweak this job to avoid timeouts.  Given that
> we have 10 threads, we also move a few of the specific sandbox test
> builds to a prior stage.
>
> Note that while sandbox builds with address sanitization enabled (ASAN)
> not all tests pass, so we limit ourselves to just checking that the
> version test passes for now.
>
> Link: 
> https://learn.microsoft.com/en-us/azure/devops/pipelines/process/phases?view=azure-devops=yaml#timeouts
> Signed-off-by: Tom Rini 
> ---
> Cc: Simon Glass 
>
> I believe we merged ASAN with the intention of fixing some of the found
> issues later, so this isn't really a functional change.
> ---
>  .azure-pipelines.yml | 100 +++
>  1 file changed, 25 insertions(+), 75 deletions(-)

Reviewed-by: Simon Glass 


Re: [PATCH 1/4] Azure: Set the timeout for jobs to the maximum

2023-08-21 Thread Simon Glass
On Sun, 20 Aug 2023 at 11:31, Tom Rini  wrote:
>
> As per current Azure Pipelines documentation we qualify for 3600 minutes
> per job, if specified, as the timeout. The default unspecified timeout
> is 60 minutes. Rework things to specify 0 as the timeout (and so maximum
> allowed) so that we don't have failures due to running slightly past 60
> minutes total.
>
> Link: 
> https://learn.microsoft.com/en-us/azure/devops/pipelines/process/phases?view=azure-devops=yaml#timeouts
> Signed-off-by: Tom Rini 
> ---
>  .azure-pipelines.yml | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Simon Glass 


Please pull u-boot-dm

2023-08-21 Thread Simon Glass
Hi Tom,

https://source.denx.de/u-boot/custodians/u-boot-dm/-/pipelines/17449
https://dev.azure.com/simon0972/u-boot/_build/results?buildId=47=results


The following changes since commit 17aad803551500e1a3d87339a6559e99b7fad479:

  Merge branch 'master' of
https://source.denx.de/u-boot/custodians/u-boot-sh (2023-08-20
11:09:11 -0400)

are available in the Git repository at:

  git://git.denx.de/u-boot-dm.git tags/dm-pull-20aug23

for you to fetch changes up to 25a9be71ec1ca779aac8bcb1d8a363725ff0ac7f:

  test: cpu: Handle both 32bit and 64bit CPUs (2023-08-20 15:55:27 -0600)


sandbox64 fixes


Marek Vasut (11):
  configs: sandbox64: Enable SF bootdev
  configs: sandbox64: Increase console record size to 0x6000
  configs: sandbox64: Enable MC34708 driver
  configs: sandbox64: Enable BUTTON_ADC driver
  test: dm: pinmux: Handle %pa in pinctrl-single mux output
  configs: sandbox64: Enable PINCTRL_SINGLE driver
  configs: sandbox64: Enable video 16bpp and 24bpp support
  configs: sandbox64: Enable video 12x22 font support
  configs: sandbox64: Enable clock CCF driver
  configs: sandbox64: Enable PCI register multi-entry support
  test: cpu: Handle both 32bit and 64bit CPUs

 configs/sandbox64_defconfig | 12 ++-
 test/dm/cpu.c   |  2 +-
 test/dm/pinmux.c| 92
++
 3 files changed, 63 insertions(+), 43 deletions(-)

Regards,
Simon


Re: [PATCH v2 0/8] Add DFU, emmc and usb boot for TI am62x

2023-08-21 Thread Tom Rini
On Mon, Aug 21, 2023 at 04:13:32PM +, Marcel Ziswiler wrote:
> Hi Sjoerd
> 
> On Thu, 2023-06-01 at 08:37 +0200, Sjoerd Simons wrote:
> > On Wed, 2023-05-31 at 17:14 -0400, Tom Rini wrote:
> > > On Thu, Apr 06, 2023 at 08:55:34PM +0200, Sjoerd Simons wrote:
> > > 
> > > > This series adds more boot sources for the TI am62x. For that the
> > > > dts'
> > > > are synced from the upstream ti-next git tree (to add usb nodes),
> > > > some
> > > > dwc3 glue is and finally the default configuration is tuned to add
> > > > support for DFU and USB (host and gadget)
> > > 
> > > This seems, conceptually, fine.  But as we're getting the TI dts
> > > files
> > > in sync with the kernel, I'm deferring this version and you'll want
> > > to
> > > rebase and re-post once everything has settled.
> > 
> > Thanks for the update/hint ;) I also got a few review comments so the
> > plan is to include those and repost.. Just my may has been stupidly
> > busy causing me to not get around it in the first place, so maybe that
> > turned into good timing in the end.
> 
> Any progress on this?
> 
> I still carry your re-based series on top of latest master [1] and USB DFU is 
> working very well on Verdin AM62.
> 
> Thanks!
> 
> [1] https://github.com/ziswiler/u-boot/tree/verdin-am62-usb-support

As the am62 files have been re-synced, re-basing this and re-posting for
next would be appropriate at this point.

-- 
Tom


signature.asc
Description: PGP signature


[RFC PATCH 5/5] sandbox: video: Use partial updates for SDL display

2023-08-21 Thread Alper Nebi Yasak
Now that we have video damage tracking, try to reduce the SDL display
work by copying only the updated regions onto the SDL texture instead of
the entire framebuffer. We still have to do RenderClear and RenderCopy
the whole texture onto the renderer, but that allegedly happens in the
GPU.

Signed-off-by: Alper Nebi Yasak 
---
The second half of copy_to_texture is untested.

 arch/sandbox/cpu/sdl.c | 25 +
 arch/sandbox/include/asm/sdl.h |  9 +++--
 drivers/video/sandbox_sdl.c| 16 +++-
 3 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/arch/sandbox/cpu/sdl.c b/arch/sandbox/cpu/sdl.c
index 48fae20b4c2d..3a3221d89066 100644
--- a/arch/sandbox/cpu/sdl.c
+++ b/arch/sandbox/cpu/sdl.c
@@ -192,8 +192,10 @@ int sandbox_sdl_init_display(int width, int height, int 
log2_bpp,
return 0;
 }
 
-static int copy_to_texture(void *lcd_base)
+static int copy_to_texture(void *lcd_base, int xstart, int ystart,
+  int xend, int yend)
 {
+   struct SDL_Rect rect;
char *dest;
int pitch, x, y;
int src_pitch;
@@ -201,8 +203,15 @@ static int copy_to_texture(void *lcd_base)
char *src;
int ret;
 
+   rect.x = xstart;
+   rect.y = ystart;
+   rect.w = xend - xstart + 1;
+   rect.h = yend - ystart + 1;
+
if (sdl.src_depth == sdl.depth) {
-   SDL_UpdateTexture(sdl.texture, NULL, lcd_base, sdl.pitch);
+   src_pitch = sdl.width * sdl.src_depth / 8;
+   src = lcd_base + src_pitch * rect.y + rect.x * sdl.src_depth / 
8;
+   SDL_UpdateTexture(sdl.texture, , src, src_pitch);
return 0;
}
 
@@ -215,7 +224,7 @@ static int copy_to_texture(void *lcd_base)
return -EINVAL;
}
 
-   ret = SDL_LockTexture(sdl.texture, NULL, , );
+   ret = SDL_LockTexture(sdl.texture, , , );
if (ret) {
printf("SDL lock %d: %s\n", ret, SDL_GetError());
return ret;
@@ -223,12 +232,12 @@ static int copy_to_texture(void *lcd_base)
 
/* Copy the pixels one by one */
src_pitch = sdl.width * sdl.src_depth / 8;
-   for (y = 0; y < sdl.height; y++) {
+   for (y = 0; y < rect.h; y++) {
char val;
 
dest = pixels + y * pitch;
-   src = lcd_base + src_pitch * y;
-   for (x = 0; x < sdl.width; x++, dest += 4) {
+   src = lcd_base + src_pitch * (ystart + y) + xstart;
+   for (x = 0; x < rect.w; x++, dest += 4) {
val = *src++;
dest[0] = val;
dest[1] = val;
@@ -241,7 +250,7 @@ static int copy_to_texture(void *lcd_base)
return 0;
 }
 
-int sandbox_sdl_sync(void *lcd_base)
+int sandbox_sdl_sync(void *lcd_base, int xstart, int ystart, int xend, int 
yend)
 {
struct SDL_Rect rect;
int ret;
@@ -253,7 +262,7 @@ int sandbox_sdl_sync(void *lcd_base)
return -EAGAIN;
 
SDL_RenderClear(sdl.renderer);
-   ret = copy_to_texture(lcd_base);
+   ret = copy_to_texture(lcd_base, xstart, ystart, xend, yend);
if (ret) {
printf("copy_to_texture: %d: %s\n", ret, SDL_GetError());
return -EIO;
diff --git a/arch/sandbox/include/asm/sdl.h b/arch/sandbox/include/asm/sdl.h
index 1ace7d1a1217..c7c73ef3a3e6 100644
--- a/arch/sandbox/include/asm/sdl.h
+++ b/arch/sandbox/include/asm/sdl.h
@@ -41,10 +41,14 @@ int sandbox_sdl_remove_display(void);
  * system resources too much.
  *
  * @lcd_base: Base of frame buffer
+ * @xstart:   X start position of updated region in pixels from the left
+ * @ystart:   Y start position of updated region in pixels from the top
+ * @xend: X end position of updated region in pixels from the left
+ * @yend: Y end position of updated region in pixels from the top
  * Return: 0 if screen was updated, -ENODEV is there is no screen.
  *-EAGAIN if screen was not updated due to sync rate limit.
  */
-int sandbox_sdl_sync(void *lcd_base);
+int sandbox_sdl_sync(void *lcd_base, int xstart, int ystart, int xend, int 
yend);
 
 /**
  * sandbox_sdl_scan_keys() - scan for pressed keys
@@ -118,7 +122,8 @@ static inline int sandbox_sdl_remove_display(void)
return -ENODEV;
 }
 
-static inline int sandbox_sdl_sync(void *lcd_base)
+static inline int sandbox_sdl_sync(void *lcd_base, int xstart, int ystart,
+  int xend, int yend)
 {
return -ENODEV;
 }
diff --git a/drivers/video/sandbox_sdl.c b/drivers/video/sandbox_sdl.c
index 7dc2787a5d25..eb424072b6fe 100644
--- a/drivers/video/sandbox_sdl.c
+++ b/drivers/video/sandbox_sdl.c
@@ -103,11 +103,25 @@ static int sandbox_sdl_video_sync(struct udevice *dev)
 {
struct video_priv *priv = dev_get_uclass_priv(dev);
void *fb = priv->fb;
+   int xstart = 0;
+   int ystart = 0;
+   int xend = priv->xsize;
+

[PATCH 4/5] sandbox: video: Move sandbox video sync to a driver operation

2023-08-21 Thread Alper Nebi Yasak
The sandbox SDL video sync is handled in the uclass because there has
been a sync rate limiter and a way to bypass that. Previous patches
move the rate limit code into SDL-specific files, and provide a generic
way to defer and force video syncs.

Sandbox code shouldn't be in the uclasses if possible. Move the
remaining sandbox sync call into the driver ops. Now that sandbox video
sync attempts can defer without resetting video damage, force a video
sync before checking that video syncs reset video damage.

Signed-off-by: Alper Nebi Yasak 
---

 drivers/video/sandbox_sdl.c  | 16 
 drivers/video/video-uclass.c | 14 --
 test/dm/video.c  |  2 +-
 3 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/video/sandbox_sdl.c b/drivers/video/sandbox_sdl.c
index 9081c7da62e4..7dc2787a5d25 100644
--- a/drivers/video/sandbox_sdl.c
+++ b/drivers/video/sandbox_sdl.c
@@ -99,6 +99,17 @@ int sandbox_sdl_set_bpp(struct udevice *dev, enum 
video_log2_bpp l2bpp)
return 0;
 }
 
+static int sandbox_sdl_video_sync(struct udevice *dev)
+{
+   struct video_priv *priv = dev_get_uclass_priv(dev);
+   void *fb = priv->fb;
+
+   if (IS_ENABLED(CONFIG_VIDEO_COPY))
+   fb = priv->copy_fb;
+
+   return sandbox_sdl_sync(fb);
+}
+
 static int sandbox_sdl_remove(struct udevice *dev)
 {
/*
@@ -133,6 +144,10 @@ static const struct udevice_id sandbox_sdl_ids[] = {
{ }
 };
 
+static struct video_ops sandbox_sdl_ops = {
+   .video_sync = sandbox_sdl_video_sync,
+};
+
 U_BOOT_DRIVER(sandbox_lcd_sdl) = {
.name   = "sandbox_lcd_sdl",
.id = UCLASS_VIDEO,
@@ -141,4 +156,5 @@ U_BOOT_DRIVER(sandbox_lcd_sdl) = {
.probe  = sandbox_sdl_probe,
.remove = sandbox_sdl_remove,
.plat_auto  = sizeof(struct sandbox_sdl_plat),
+   .ops= _sdl_ops,
 };
diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
index d867185da539..2632216c05ae 100644
--- a/drivers/video/video-uclass.c
+++ b/drivers/video/video-uclass.c
@@ -23,9 +23,6 @@
 #include 
 #include 
 #include 
-#ifdef CONFIG_SANDBOX
-#include 
-#endif
 
 /*
  * Theory of operation:
@@ -454,17 +451,6 @@ int video_sync(struct udevice *vid, bool force)
 
video_flush_dcache(vid);
 
-#if defined(CONFIG_VIDEO_SANDBOX_SDL)
-   void *fb = priv->fb;
-
-   if (IS_ENABLED(CONFIG_VIDEO_COPY))
-   fb = priv->copy_fb;
-
-   ret = sandbox_sdl_sync(fb);
-   while (force && ret == -EAGAIN)
-   ret = sandbox_sdl_sync(fb);
-#endif
-
if (IS_ENABLED(CONFIG_VIDEO_DAMAGE)) {
priv->damage.xstart = priv->xsize;
priv->damage.ystart = priv->ysize;
diff --git a/test/dm/video.c b/test/dm/video.c
index 4c3bcd26e94f..487e4d4a73a2 100644
--- a/test/dm/video.c
+++ b/test/dm/video.c
@@ -756,7 +756,7 @@ static int dm_test_video_damage(struct unit_test_state *uts)
ut_asserteq(1280, priv->damage.xend);
ut_asserteq(510, priv->damage.yend);
 
-   video_sync(dev, false);
+   video_sync(dev, true);
ut_asserteq(priv->xsize, priv->damage.xstart);
ut_asserteq(priv->ysize, priv->damage.ystart);
ut_asserteq(0, priv->damage.xend);
-- 
2.40.1



[PATCH 3/5] sandbox: video: Move sync rate limit into SDL code

2023-08-21 Thread Alper Nebi Yasak
As a first step of removing sandbox-specific code from the video uclass,
move the sync rate limiter into the SDL code. We lose the ability to
immediately force a screen refresh, but can retry until it does.

Signed-off-by: Alper Nebi Yasak 
---

 arch/sandbox/cpu/sdl.c | 9 +
 arch/sandbox/include/asm/sdl.h | 4 +++-
 drivers/video/video-uclass.c   | 8 +++-
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/arch/sandbox/cpu/sdl.c b/arch/sandbox/cpu/sdl.c
index 590e406517bf..48fae20b4c2d 100644
--- a/arch/sandbox/cpu/sdl.c
+++ b/arch/sandbox/cpu/sdl.c
@@ -48,6 +48,7 @@ struct buf_info {
  * @screen: SDL window to use
  * @src_depth: Number of bits per pixel in the source frame buffer (that we 
read
  * from and render to SDL)
+ * @last_sync: Timestamp of last display update in milliseconds
  */
 static struct sdl_info {
int width;
@@ -67,6 +68,7 @@ static struct sdl_info {
SDL_Renderer *renderer;
SDL_Window *screen;
int src_depth;
+   int last_sync;
 } sdl;
 
 static void sandbox_sdl_poll_events(void)
@@ -148,6 +150,7 @@ int sandbox_sdl_init_display(int width, int height, int 
log2_bpp,
sdl.vis_width = sdl.width;
sdl.vis_height = sdl.height;
}
+   sdl.last_sync = 0;
 
if (!SDL_SetHint(SDL_HINT_RENDER_SCALE_QUALITY, "1"))
printf("Unable to init hinting: %s", SDL_GetError());
@@ -245,6 +248,10 @@ int sandbox_sdl_sync(void *lcd_base)
 
if (!sdl.texture)
return 0;
+
+   if (SDL_GetTicks() - sdl.last_sync < 100)
+   return -EAGAIN;
+
SDL_RenderClear(sdl.renderer);
ret = copy_to_texture(lcd_base);
if (ret) {
@@ -268,6 +275,8 @@ int sandbox_sdl_sync(void *lcd_base)
SDL_RenderDrawRect(sdl.renderer, );
 
SDL_RenderPresent(sdl.renderer);
+   sdl.last_sync = SDL_GetTicks();
+
sandbox_sdl_poll_events();
 
return 0;
diff --git a/arch/sandbox/include/asm/sdl.h b/arch/sandbox/include/asm/sdl.h
index ee4991f7c24a..1ace7d1a1217 100644
--- a/arch/sandbox/include/asm/sdl.h
+++ b/arch/sandbox/include/asm/sdl.h
@@ -37,10 +37,12 @@ int sandbox_sdl_remove_display(void);
  * sandbox_sdl_sync() - Sync current U-Boot LCD frame buffer to SDL
  *
  * This must be called periodically to update the screen for SDL so that the
- * user can see it.
+ * user can see it. It is internally rate limited to 10 Hz to avoid using
+ * system resources too much.
  *
  * @lcd_base: Base of frame buffer
  * Return: 0 if screen was updated, -ENODEV is there is no screen.
+ *-EAGAIN if screen was not updated due to sync rate limit.
  */
 int sandbox_sdl_sync(void *lcd_base);
 
diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
index accf486509cb..d867185da539 100644
--- a/drivers/video/video-uclass.c
+++ b/drivers/video/video-uclass.c
@@ -455,16 +455,14 @@ int video_sync(struct udevice *vid, bool force)
video_flush_dcache(vid);
 
 #if defined(CONFIG_VIDEO_SANDBOX_SDL)
-   static ulong last_sync;
void *fb = priv->fb;
 
if (IS_ENABLED(CONFIG_VIDEO_COPY))
fb = priv->copy_fb;
 
-   if (force || get_timer(last_sync) > 100) {
-   sandbox_sdl_sync(fb);
-   last_sync = get_timer(0);
-   }
+   ret = sandbox_sdl_sync(fb);
+   while (force && ret == -EAGAIN)
+   ret = sandbox_sdl_sync(fb);
 #endif
 
if (IS_ENABLED(CONFIG_VIDEO_DAMAGE)) {
-- 
2.40.1



[PATCH 2/5] video: Allow deferring and retrying driver-specific video sync

2023-08-21 Thread Alper Nebi Yasak
The sandbox SDL driver has some code in the video uclass to rate limit
video syncs by postponing them, and forcing a sync nonetheless with a
"force" argument.

Add infrastructure for doing this through driver ops, where the driver
can request to defer a sync with -EAGAIN, and the uclass can force it by
calling the op again it until it does something.

Signed-off-by: Alper Nebi Yasak 
---
Alternatively, add "force" argument into the driver ops->video_sync().
But I think it should go away instead. The problem is video_sync() being
called irregularly and too frequently, maybe we can call it only from
cyclic at the hardware refresh rate?

 drivers/video/video-uclass.c | 2 ++
 include/video.h  | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
index 7cec6362570f..accf486509cb 100644
--- a/drivers/video/video-uclass.c
+++ b/drivers/video/video-uclass.c
@@ -446,6 +446,8 @@ int video_sync(struct udevice *vid, bool force)
 
if (ops && ops->video_sync) {
ret = ops->video_sync(vid);
+   while (force && ret == -EAGAIN)
+   ret = ops->video_sync(vid);
if (ret)
return ret;
}
diff --git a/include/video.h b/include/video.h
index 42e57b44188d..5c4327d4e455 100644
--- a/include/video.h
+++ b/include/video.h
@@ -137,7 +137,8 @@ struct video_priv {
  * displays needs synchronization when data in an FB is available.
  * For these devices implement video_sync hook to call a sync
  * function. vid is pointer to video device udevice. Function
- * should return 0 on success video_sync and error code otherwise
+ * should return 0 on success video_sync, -EAGAIN if it was
+ * deferred and should be tried again, and error code otherwise
  */
 struct video_ops {
int (*video_sync)(struct udevice *vid);
-- 
2.40.1



[PATCH 1/5] sandbox: video: Display copy framebuffer if enabled

2023-08-21 Thread Alper Nebi Yasak
When VIDEO_COPY is enabled, the "main" framebuffer is a cached work area
in U-Boot allocated memory and the "copy" framebuffer is the hardware
frame buffer displayed on the screen. The sandbox SDL video driver sets
copy_base to indicate support for this, but it displays the work area
instead. Change it to display the copy buffer if enabled.

Signed-off-by: Alper Nebi Yasak 
---

 drivers/video/video-uclass.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
index 3f9ddaadd15d..7cec6362570f 100644
--- a/drivers/video/video-uclass.c
+++ b/drivers/video/video-uclass.c
@@ -454,9 +454,13 @@ int video_sync(struct udevice *vid, bool force)
 
 #if defined(CONFIG_VIDEO_SANDBOX_SDL)
static ulong last_sync;
+   void *fb = priv->fb;
+
+   if (IS_ENABLED(CONFIG_VIDEO_COPY))
+   fb = priv->copy_fb;
 
if (force || get_timer(last_sync) > 100) {
-   sandbox_sdl_sync(priv->fb);
+   sandbox_sdl_sync(fb);
last_sync = get_timer(0);
}
 #endif
-- 
2.40.1



[PATCH 0/5] sandbox: video: Refactor out of uclass, try partial screen updates

2023-08-21 Thread Alper Nebi Yasak
While working on the video damage tracking series [1], I noticed the
10Hz limitation on the sandbox screen refresh rate, and thought maybe
applying the damage tracking idea to SDL would make it reasonable to
increase the rate or even remove the limit completely.

Experimenting on that idea resulted in a bit of a refactoring to pull
sandbox out of the video uclass, so I'm sending it as a series in case
some of it could be useful.

As for the partial update idea itself, I found that having the delay
around 1/refresh-rate appears to be the most visually smooth (about
7ms for 144Hz, 16ms for 60Hz etc.), but couldn't exactly evaluate CPU
usage for which the original 10Hz rate limit was set. In any case, the
visual appeal of sandbox doesn't really matter, so I don't exactly care.

Of course, this applies onto the video damage tracking series that I
recently updated and sent [1], but only the "partial updates" patch
is functionally dependent on it.

[1] 
https://lore.kernel.org/u-boot/20230821135111.3558478-1-alpernebiya...@gmail.com/


Alper Nebi Yasak (5):
  sandbox: video: Display copy framebuffer if enabled
  video: Allow deferring and retrying driver-specific video sync
  sandbox: video: Move sync rate limit into SDL code
  sandbox: video: Move sandbox video sync to a driver operation
  RFC: sandbox: video: Use partial updates for SDL display

 arch/sandbox/cpu/sdl.c | 34 ++
 arch/sandbox/include/asm/sdl.h | 13 ++---
 drivers/video/sandbox_sdl.c| 30 ++
 drivers/video/video-uclass.c   | 14 ++
 include/video.h|  3 ++-
 test/dm/video.c|  2 +-
 6 files changed, 71 insertions(+), 25 deletions(-)


base-commit: 3881c9fbb7fdd98f6eae5cd33f7e9abe9455a585
prerequisite-patch-id: aad206b8942d0e8654ba1b11f28104950baf1518
prerequisite-patch-id: ee3d21bb91062ebbcf0c3a75342d7fa37d5630ce
prerequisite-patch-id: 7fcaec0bb8b5bbb5b3813bda896034c9b98b67b9
prerequisite-patch-id: aa4173ebedc3f6d0f04f72bf49bed642614f42a7
prerequisite-patch-id: 06676907c4d262880f7904fed33de11057e886f0
prerequisite-patch-id: 63bbfd7808bddea8b39120d50bb61ac1c4b3eeb8
prerequisite-patch-id: 8941f56b57f7ece319eed67ce8584e7769a36a3b
prerequisite-patch-id: b89b375a3221ac1f89f3eb15fa20a2140ea5687c
prerequisite-patch-id: 950508eaecbbc01560a2f24778786dd50e7c20e8
prerequisite-patch-id: c30b2a60881bc813e4d8a7e1a99fd99c368957d2
prerequisite-patch-id: 963770b5a43c750bac620788562ce549ac48dacd
prerequisite-patch-id: 500af25e869687516da147ce7c819c6b32a9bc92
prerequisite-patch-id: b064cd6b7db9cd122f10c2ae2573328ac945b3be
-- 
2.40.1



Re: [PATCH v2 2/2] riscv: cpu: make riscv_cpu_probe to EVT_DM_POST_INIT_R callback (resend)

2023-08-21 Thread Roland Ruckerbauer

Tested on visionfive2, it works.

On 18.08.23 07:11, Chanho Park wrote:

Since the Patch 55171aedda88, VisionFive2 booting has been broken [1].
VisionFive2 board requires to enable CONFIG_TIMER_EARLY but booting went
to panic from initr_dm_devices due to lack of a timer device.

- Error logs
initcall sequence fffd8d38 failed at call 402185e4
(err=-19)

Thus, we need to move riscv_cpu_probe function in order to register
the timer earlier than initr_dm_devices.

Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
Cc: Simon Glass 
Cc: Bin Meng 
Signed-off-by: Chanho Park 

Tested-by: Roland Ruckerbauer 

---
arch/riscv/cpu/cpu.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
index ecfb1fb08c4b..0b4208e72199 100644
--- a/arch/riscv/cpu/cpu.c
+++ b/arch/riscv/cpu/cpu.c
@@ -66,7 +66,7 @@ static inline bool supports_extension(char ext)
#endif /* CONFIG_CPU */
}
-static int riscv_cpu_probe(void)
+static int riscv_cpu_probe(void *ctx, struct event *event)
{
#ifdef CONFIG_CPU
int ret;
@@ -79,6 +79,7 @@ static int riscv_cpu_probe(void)
return 0;
}
+EVENT_SPY(EVT_DM_POST_INIT_R, riscv_cpu_probe);
/*
* This is called on secondary harts just after the IPI is init'd. 
Currently

@@ -95,7 +96,7 @@ int riscv_cpu_setup(void *ctx, struct event *event)
{
int ret;
- ret = riscv_cpu_probe();
+ ret = riscv_cpu_probe(ctx, event);
if (ret)
return ret;
@@ -149,12 +150,6 @@ EVENT_SPY(EVT_DM_POST_INIT_F, riscv_cpu_setup);
int arch_early_init_r(void)
{
- int ret;
-
- ret = riscv_cpu_probe();
- if (ret)
- return ret;
-
if (IS_ENABLED(CONFIG_SYSRESET_SBI))
device_bind_driver(gd->dm_root, "sbi-sysreset",
"sbi-sysreset", NULL);


Re: [PATCH v2 1/2] dm: event: add EVT_DM_POST_INIT_R event type (resend)

2023-08-21 Thread Roland Ruckerbauer

Tested on visionfive2 board, it works. Thanks for taking a look into this.

On 18.08.23 07:11, Chanho Park wrote:

This patch introduces EVT_DM_POST_INIT_R event type for handling hooks
after relocation.

Fixes: 55171aedda88 ("dm: Emit the arch_cpu_init_dm() even only before 
relocation")

Suggested-by: Simon Glass 
Cc: Bin Meng 
Signed-off-by: Chanho Park 


Tested-by: Roland Ruckerbauer 


---
drivers/core/root.c | 6 --
include/event.h | 1 +
2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/core/root.c b/drivers/core/root.c
index 6775fb0b6575..79d871ab291a 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -436,8 +436,10 @@ int dm_init_and_scan(bool pre_reloc_only)
return ret;
}
}
- if (CONFIG_IS_ENABLED(DM_EVENT) && !(gd->flags & GD_FLG_RELOC)) {
- ret = event_notify_null(EVT_DM_POST_INIT_F);
+ if (CONFIG_IS_ENABLED(DM_EVENT)) {
+ ret = event_notify_null(gd->flags & GD_FLG_RELOC ?
+ EVT_DM_POST_INIT_R :
+ EVT_DM_POST_INIT_F);
if (ret)
return log_msg_ret("ev", ret);
}
diff --git a/include/event.h b/include/event.h
index daf44bf8a83b..bb38ba98e73b 100644
--- a/include/event.h
+++ b/include/event.h
@@ -24,6 +24,7 @@ enum event_t {
/* Events related to driver model */
EVT_DM_POST_INIT_F,
+ EVT_DM_POST_INIT_R,
EVT_DM_PRE_PROBE,
EVT_DM_POST_PROBE,
EVT_DM_PRE_REMOVE,


[PATCH v3 3/3] schemas: Add a schema for binman

2023-08-21 Thread Simon Glass
With this version I have done with a generic name, in this case 'data',
as suggested by Alper Nebi Yasak. This may be controversial, but we may
as well have the dicussion now. I assume that there are no other
ongoing attempts to define the layout of firmware in devicetree.

Signed-off-by: Simon Glass 
---

Changes in v3:
Use data,layout for the layout property

Changes in v2:
- Reworked significantly based on Alper's comments

 dtschema/schemas/firmware/image.yaml| 77 +
 dtschema/schemas/firmware/layout/entry.yaml | 77 +
 2 files changed, 154 insertions(+)
 create mode 100644 dtschema/schemas/firmware/image.yaml
 create mode 100644 dtschema/schemas/firmware/layout/entry.yaml

diff --git a/dtschema/schemas/firmware/image.yaml 
b/dtschema/schemas/firmware/image.yaml
new file mode 100644
index 000..cc052e8
--- /dev/null
+++ b/dtschema/schemas/firmware/image.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2023 Google LLC
+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/image.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Binman firmware layout
+
+maintainers:
+  - Simon Glass 
+
+description: |
+  The image node provides a layout for firmware, used when packaging firmware
+  from multiple projects. For now it just supports a very simple set of
+  features, as a starting point for discussion.
+
+  The Binman tool processes this node to produce a final image which can be
+  loaded into suitable storage device. Documentation is at:
+
+  https://u-boot.readthedocs.io/en/latest/develop/package/binman.html
+
+  The current image-description format is here:
+
+  
https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#image-description-format
+
+  It is desirable to reference the image from the storage-device node, perhaps
+  using an image-desc property:
+
+spiflash@0 {
+  compatible = "spidev", "jedec,spi-nor";
+  data,layout = <>;
+};
+
+  Note that the intention is to change Binman to use whatever schema is agreed
+  here.
+
+properties:
+  $nodename:
+pattern: "^[-a-z]+(-[0-9]+)?$"
+
+  compatible:
+const: data,image
+
+  "#address-cells":
+const: 1
+
+  "#size-cells":
+const: 1
+
+required:
+  - compatible
+  - "#address-cell"
+  - "#size-cells"
+
+additionalProperties: true
+
+examples:
+  - |
+firmware {
+  image: image {
+compatible = "data,image";
+#address-cells = <1>;
+$size-cells = <1>;
+
+u-boot@0 {
+  compatible = "data,u-boot";
+  reg = <0 0xa>;
+};
+
+atf-bl31@0x10 {
+  compatible = "data,atf-bl31";
+  reg = <0x10 0x2>;
+};
+  };
+};
diff --git a/dtschema/schemas/firmware/layout/entry.yaml 
b/dtschema/schemas/firmware/layout/entry.yaml
new file mode 100644
index 000..29f0dfc
--- /dev/null
+++ b/dtschema/schemas/firmware/layout/entry.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2023 Google LLC
+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/layout/entry.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Image entry
+
+maintainers:
+  - Simon Glass 
+
+description: |
+  The entry node specifies a single entry in the firmware image.
+
+  Entries have a specific type, such as "u-boot" or "atf-bl31". This is 
provided
+  using compatible = "data,".
+
+  Note: This definition is intended to be hierarchical, so that entries can
+  appear in other entries. Schema for that is TBD.
+
+properties:
+  compatible:
+$ref: /schemas/types.yaml#/definitions/string
+
+  offset:
+$ref: /schemas/types.yaml#/definitions/uint32
+description: |
+  Provides the offset of this entry from the start of its parent section.
+
+  This may be omitted in the description provided by Binman, in which case
+  the value is calculated as part of image packing.
+
+  size:
+$ref: /schemas/types.yaml#/definitions/uint32
+description: |
+  Provides the size of this entry in bytes.
+
+  This may be omitted in the description provided by Binman, in which case
+  the value is calculated as part of image packing.
+
+  reg:
+description: |
+  Defines the offset and size of this entry, with reference to its parent
+  image / section.
+
+  Note This is typically omitted in the description provided to Binman,
+  since the value is calculated as part of image packing. Separate
+  properties are provided for the size and offset of an entry, so that it 
is
+  easy to specify none, one or both. The `reg` property is the only one 
that
+  needs to be looked at once the image has been built.
+
+required:
+  - compatible
+
+additionalProperties: true
+
+examples:
+  - |
+firmware {
+  image: image {
+compatible = "data,image";
+#address-cells = <1>;
+$size-cells = <1>;
+
+

[PATCH v3 2/3] schemas: Add firmware node schema

2023-08-21 Thread Simon Glass
Add a motivation and purpose for this new proposed node.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 dtschema/schemas/firmware.yaml | 83 ++
 1 file changed, 83 insertions(+)
 create mode 100644 dtschema/schemas/firmware.yaml

diff --git a/dtschema/schemas/firmware.yaml b/dtschema/schemas/firmware.yaml
new file mode 100644
index 000..4439a70
--- /dev/null
+++ b/dtschema/schemas/firmware.yaml
@@ -0,0 +1,83 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-clause
+# Copyright 2023 Google LLC
+#
+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: /firmware Node
+
+maintainers:
+  - Simon Glass 
+
+description: |
+  The '/firmware' node does not represent a real device, but serves as a place
+  for recording information about the main firmware used on the device, such as
+  a map of its contents. This is used by the Operating System (OS), user space
+  programs and possibly other firmware components. Data in the '/firmware' node
+  does not itself represent the hardware.
+
+  Properties in this node should be common to (and used by) at least two
+  firmware projects, such as U-Boot and TF-A. Project-specific subnodes can be
+  used for properties which are specific to a single project.
+
+  Purpose of '/firmware' node
+  ---
+
+  Firmware has traditionally been fairly opaque to the OS, with the OS taking
+  no interest in its contents, version, layout or how it might be updated. This
+  is less than ideal, since firmware is an important part of the system and
+  visibility into its operation is every bit as important as visbility into the
+  OS and user-space programs within the system.
+
+  The traditional approach has been to let firmware deal with firmware, and the
+  OS deal with everything else. Updating firmware has been handled by firmware.
+  For example, the UEFI spec defines a way for the OS to post a 'capsule' which
+  is discovered next time the system boots, permitting firmware updates. But
+  firmware updates in firmware are highly problematic. They require a reboot
+  and a sometimes-lengthy wait with a strange-looking interface unfamiliar
+  to most users. It seems better to make the update as transparent as possible
+  to the user. As an example of that, ChromeOS has full knowledge of the
+  firmware version and layout, updates it in the background from user space and
+  instantly selects the new firmware when the user reboots or logs out.
+
+  A common objection to considering the system holistically is that some parts
+  of the system are inaccessible to the OS, such as a secure enclave. However
+  this does not preclude providing visibility into what is present in that
+  enclave. Firmware-version information is still useful. Firmware updates are
+  still needed and can still be initiated from user space.
+
+  Another objection is that firmware should provide an interface to the OS,
+  while keeping its structure private. This thinking is largely driven by
+  extrapolating from how firmware has been handled in the 'BIOS' days.
+  It should be considered a degenerate case rather than the norm. As complexity
+  increases, it creates an artificial boundary between two pieces of the whole.
+  Mechanisms then need to be invented to cross this unnecessary chasm. An
+  example of this is Intel's Dynamic Platform and Thermal Framework (DPTF),
+  which consists of user-space, OS and firmware components all working towards
+  a shared goal. We need a standard description of these cross-system pieces.
+
+  In order to 'teach the OS about firmware', we need a place to put this
+  information. That is the purpose of this node.
+
+  In an Open Source world the entire model of firmware needs to adjust to be
+  more open, more visible and managed just like any other part of the system.
+  The major goal is to standardise how firmware is presented to the OS and user
+  space, so that common utilities can be used to manage the entire system,
+  including the firmware. For example, fwupd can look in this node for
+  information on how to update the firmware, similar to how VBE works. [1]
+  It is likely that other purposes will come to light over time.
+
+  [1] https://github.com/fwupd/fwupd/tree/main/plugins/vbe
+
+properties:
+  $nodename:
+const: firmware
+
+  "#address-cells": true
+  "#size-cells": true
+
+additionalProperties:
+  type: object
-- 
2.42.0.rc1.204.g551eb34607-goog



[PATCH v3 1/3] Add validation instructions

2023-08-21 Thread Simon Glass
Add simple instructions for people wanting to validate their schema.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 README.md | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/README.md b/README.md
index 9d2f6e8..d8767fa 100644
--- a/README.md
+++ b/README.md
@@ -118,6 +118,25 @@ Example:
 dt-check-compatible -s processed-schema.json vendor,a-compatible
 ```
 
+## Testing your changes
+
+Once you add or change schema, you should test it locally. This assumes that
+your system is set up so that 'python' runs Python 3.
+
+First make sure you don't have any existing dt-schema in your system as this
+may interfere:
+
+```
+sudo apt-get remove dt-schema
+pip remove dt-schema
+```
+
+Then, to validate, use:
+
+```
+PYTHONPATH=. test/test-dt-validate.py
+```
+
 ## Installing
 The project and its dependencies can be installed with pip:
 
-- 
2.42.0.rc1.204.g551eb34607-goog



Re: [PATCH v2 0/8] Add DFU, emmc and usb boot for TI am62x

2023-08-21 Thread Marcel Ziswiler
Hi Sjoerd

On Thu, 2023-06-01 at 08:37 +0200, Sjoerd Simons wrote:
> On Wed, 2023-05-31 at 17:14 -0400, Tom Rini wrote:
> > On Thu, Apr 06, 2023 at 08:55:34PM +0200, Sjoerd Simons wrote:
> > 
> > > This series adds more boot sources for the TI am62x. For that the
> > > dts'
> > > are synced from the upstream ti-next git tree (to add usb nodes),
> > > some
> > > dwc3 glue is and finally the default configuration is tuned to add
> > > support for DFU and USB (host and gadget)
> > 
> > This seems, conceptually, fine.  But as we're getting the TI dts
> > files
> > in sync with the kernel, I'm deferring this version and you'll want
> > to
> > rebase and re-post once everything has settled.
> 
> Thanks for the update/hint ;) I also got a few review comments so the
> plan is to include those and repost.. Just my may has been stupidly
> busy causing me to not get around it in the first place, so maybe that
> turned into good timing in the end.

Any progress on this?

I still carry your re-based series on top of latest master [1] and USB DFU is 
working very well on Verdin AM62.

Thanks!

[1] https://github.com/ziswiler/u-boot/tree/verdin-am62-usb-support

Cheers

Marcel


Re: [PATCH] configs: Enable CONFIG_DM_SCSI in am57xx_hs_evm_usb

2023-08-21 Thread Tom Rini
On Mon, Aug 21, 2023 at 08:59:10AM -0500, Andrew Davis wrote:

> This should have already been enabled but was missed when converting the
> base platform defconfig, fix this here.
> 
> Fixes: 3c5aa6caccab ("configs: Enable CONFIG_BLK in am57xx_evm and 
> am57xx_hs_evm")
> Signed-off-by: Andrew Davis 

Reviewed-by: Tom Rini 

-- 
Tom


signature.asc
Description: PGP signature


Re: [GIT PULL] ARM: tegra: Changes for v2023.10-rc1

2023-08-21 Thread Svyatoslav Ryhel



21 серпня 2023 р. 16:59:58 GMT+03:00, Thierry Reding  
написав(-ла):
>On Fri, Aug 18, 2023 at 08:02:30PM +0300, Svyatoslav Ryhel wrote:
>> 
>> 
>> 18 серпня 2023 р. 19:49:43 GMT+03:00, Tom Rini  
>> написав(-ла):
>> >On Fri, Aug 18, 2023 at 03:39:22PM +0200, Thierry Reding wrote:
>> >
>> >> From: Thierry Reding 
>> >> 
>> >> Hi Tom,
>> >> 
>> >> The following changes since commit 
>> >> 68c07fc5fdf34f0926cf06fc0c4ebd6f2f3afe19:
>> >> 
>> >>   Merge https://source.denx.de/u-boot/custodians/u-boot-usb (2023-06-21 
>> >> 14:42:50 -0400)
>> >> 
>> >> are available in the Git repository at:
>> >> 
>> >>   g...@source.denx.de:u-boot/custodians/u-boot-tegra.git 
>> >> tags/tegra-for-2023.10-rc1
>> >
>> >FYI, in your .git/config you can do:
>> >url = https://source.denx.de/u-boot/custodians/u-boot-tegra.git
>> >pushurl = g...@source.denx.de:u-boot/custodians/u-boot-tegra.git
>> >
>> >And this part looks "nicer".  Not a big deal, just an FYI.
>> >
>> >> for you to fetch changes up to bdf9dead86f06c7d6980c399a4a6339430b531ec:
>> >> 
>> >>   board: htc: endeavoru: add One X support (2023-06-30 15:20:37 +0200)
>> >> 
>> >> This passes CI successfully:
>> >> 
>> >>   https://source.denx.de/u-boot/custodians/u-boot-tegra/-/pipelines/17411
>> >> 
>> >> Thanks,
>> >> Thierry
>> >> 
>> >> 
>> >> ARM: tegra: Changes for v2023.10-rc1
>> >> 
>> >> This adds support for various new Tegra30 boards (ASUS, LG and HTC) and
>> >> has some other minor enhancements, such as enabling the poweroff command
>> >> on several Tegra210 and Tegra186 boards.
>> >> 
>> >> 
>> >> Jonas Schwöbel (1):
>> >>   configs: tegra-common-post: make PXE and DHCP boot targets optional
>> >> 
>> >> Svyatoslav Ryhel (6):
>> >>   configs: tegra-common-post: add GPIO keyboard as STDIN device
>> >>   ARM: tegra: add SoC UID calculation function
>> >>   board: asus: transformer: add ASUS Transformer T30 family support
>> >>   board: asus: grouper: add Google Nexus 7 (2012) support
>> >>   board: lg: x3: add Optimus 4X HD and Optimus Vu support
>> >>   board: htc: endeavoru: add One X support
>> >> 
>> >> Thierry Reding (2):
>> >>   ARM: tegra: Enable poweroff command on Jetson TX1 and Jetson Nano
>> >>   ARM: tegra: Enable poweroff command on Jetson TX2
>> >> 
>> >> Tomasz Maciej Nowak (1):
>> >>   ARM: dts: trimslice: sync SPI node with Linux dts
>> >
>> >Since I'm not sure if this is v8 or v9, given when v9 was posted, I've
>> >applied this PR now, as I had said I wanted this in.  I do have two
>> >requests for you Svyatoslav, however.  First, if there's changes that
>> >are missing in master, please post those ASAP and we'll get them in, for
>> >v2023.10. 
>> 
>> Patches Thierry applied are v8, while v9 has significant improvements.
>
>I'm a little surprised by how this series developed. You've been urging
>me since v6 to apply this, so I assumed this was mostly done. Having
>another 3 versions with significant improvements come in after that
>point indicates the opposite was true.

View changes, same as code. I am adapting and changing this to my needs and if 
you have hold it more, your would get even more iterations. Only way to stop is 
to upstream.

>Thierry


Re: [PATCH v2 1/1] usb: host: tegra: implement dts based xcvr setup

2023-08-21 Thread Svyatoslav Ryhel
21 серпня 2023 р. 16:41:51 GMT+03:00, Marek Vasut  написав(-ла):
>On 8/21/23 13:19, Svyatoslav Ryhel wrote:
>> Some devices (like ASUS TF201) may not use fuse based xcvr setup
>> which will result in not working USB line. To fix this situation
>> allow xcvr setup to be performed from device tree values if
>> nvidia,xcvr-setup-use-fuses is not defined.
>> 
>> Signed-off-by: Svyatoslav Ryhel 
>> ---
>>   drivers/usb/host/ehci-tegra.c | 48 +++
>>   1 file changed, 43 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
>> index 2cf1625670..a9aee908f3 100644
>> --- a/drivers/usb/host/ehci-tegra.c
>> +++ b/drivers/usb/host/ehci-tegra.c
>> @@ -81,6 +81,8 @@ struct fdt_usb {
>>  enum periph_id periph_id;/* peripheral id */
>>  struct gpio_desc vbus_gpio; /* GPIO for vbus enable */
>>  struct gpio_desc phy_reset_gpio; /* GPIO to reset ULPI phy */
>> +bool xcvr_setup_use_fuses; /* Indicates that the value is read from the 
>> on-chip fuses */
>> +u32 xcvr_setup; /* Input of XCVR cell, HS driver output control */
>>   };
>> /*
>> @@ -464,10 +466,16 @@ static int init_utmi_usb_controller(struct fdt_usb 
>> *config,
>>  /* Recommended PHY settings for EYE diagram */
>>  val = readl(>utmip_xcvr_cfg0);
>> -clrsetbits_le32(, UTMIP_XCVR_SETUP_MASK,
>> -0x4 << UTMIP_XCVR_SETUP_SHIFT);
>> -clrsetbits_le32(, UTMIP_XCVR_SETUP_MSB_MASK,
>> -0x3 << UTMIP_XCVR_SETUP_MSB_SHIFT);
>> +
>> +if (!config->xcvr_setup_use_fuses) {
>> +clrsetbits_le32(, UTMIP_XCVR_SETUP_MASK,
>> +config->xcvr_setup <<
>> +UTMIP_XCVR_SETUP_SHIFT);
>> +clrsetbits_le32(, UTMIP_XCVR_SETUP_MSB_MASK,
>> +config->xcvr_setup <<
>> +UTMIP_XCVR_SETUP_MSB_SHIFT);
>> +}
>> +
>>  clrsetbits_le32(, UTMIP_XCVR_HSSLEW_MSB_MASK,
>>  0x8 << UTMIP_XCVR_HSSLEW_MSB_SHIFT);
>>  writel(val, >utmip_xcvr_cfg0);
>> @@ -522,7 +530,9 @@ static int init_utmi_usb_controller(struct fdt_usb 
>> *config,
>>  setbits_le32(>utmip_bat_chrg_cfg0, UTMIP_PD_CHRG);
>>  clrbits_le32(>utmip_xcvr_cfg0, UTMIP_XCVR_LSBIAS_SE);
>> -setbits_le32(>utmip_spare_cfg0, FUSE_SETUP_SEL);
>> +
>> +if (config->xcvr_setup_use_fuses)
>> +setbits_le32(>utmip_spare_cfg0, FUSE_SETUP_SEL);
>>  /*
>>   * Configure the UTMIP_IDLE_WAIT and UTMIP_ELASTIC_LIMIT
>> @@ -843,6 +853,8 @@ static const struct ehci_ops tegra_ehci_ops = {
>>   static int ehci_usb_of_to_plat(struct udevice *dev)
>>   {
>>  struct fdt_usb *priv = dev_get_priv(dev);
>> +u32 usb_phy_phandle;
>> +ofnode usb_phy_node;
>>  int ret;
>>  ret = fdt_decode_usb(dev, priv);
>> @@ -851,6 +863,32 @@ static int ehci_usb_of_to_plat(struct udevice *dev)
>>  priv->type = dev_get_driver_data(dev);
>>   +  ret = ofnode_read_u32(dev_ofnode(dev), "nvidia,phy", _phy_phandle);
>> +if (ret) {
>> +log_debug("%s: required usb phy node isn't provided\n", 
>> __func__);
>> +priv->xcvr_setup_use_fuses = true;
>> +return 0;
>> +}
>> +
>> +usb_phy_node = ofnode_get_by_phandle(usb_phy_phandle);
>> +if (!ofnode_valid(usb_phy_node)) {
>> +log_err("%s: failed to find usb phy node\n", __func__);
>> +return -EINVAL;
>> +}
>
>Can we get a simple PHY driver instead of this hackage please ?
>
>See the generic PHY stuff in other ehci drivers for example of how to interact 
>with PHY drivers, and drivers/phy/ for PHY driver examples.

No we can't. As I said, I will not start a split of tegra-echi to chipidea 
controller and phy because of this small tweak. It seems that you don't 
understand what your are requesting. Sorry for bothering.


Re: [GIT PULL] ARM: tegra: Changes for v2023.10-rc1

2023-08-21 Thread Thierry Reding
On Fri, Aug 18, 2023 at 08:02:30PM +0300, Svyatoslav Ryhel wrote:
> 
> 
> 18 серпня 2023 р. 19:49:43 GMT+03:00, Tom Rini  
> написав(-ла):
> >On Fri, Aug 18, 2023 at 03:39:22PM +0200, Thierry Reding wrote:
> >
> >> From: Thierry Reding 
> >> 
> >> Hi Tom,
> >> 
> >> The following changes since commit 
> >> 68c07fc5fdf34f0926cf06fc0c4ebd6f2f3afe19:
> >> 
> >>   Merge https://source.denx.de/u-boot/custodians/u-boot-usb (2023-06-21 
> >> 14:42:50 -0400)
> >> 
> >> are available in the Git repository at:
> >> 
> >>   g...@source.denx.de:u-boot/custodians/u-boot-tegra.git 
> >> tags/tegra-for-2023.10-rc1
> >
> >FYI, in your .git/config you can do:
> > url = https://source.denx.de/u-boot/custodians/u-boot-tegra.git
> > pushurl = g...@source.denx.de:u-boot/custodians/u-boot-tegra.git
> >
> >And this part looks "nicer".  Not a big deal, just an FYI.
> >
> >> for you to fetch changes up to bdf9dead86f06c7d6980c399a4a6339430b531ec:
> >> 
> >>   board: htc: endeavoru: add One X support (2023-06-30 15:20:37 +0200)
> >> 
> >> This passes CI successfully:
> >> 
> >>   https://source.denx.de/u-boot/custodians/u-boot-tegra/-/pipelines/17411
> >> 
> >> Thanks,
> >> Thierry
> >> 
> >> 
> >> ARM: tegra: Changes for v2023.10-rc1
> >> 
> >> This adds support for various new Tegra30 boards (ASUS, LG and HTC) and
> >> has some other minor enhancements, such as enabling the poweroff command
> >> on several Tegra210 and Tegra186 boards.
> >> 
> >> 
> >> Jonas Schwöbel (1):
> >>   configs: tegra-common-post: make PXE and DHCP boot targets optional
> >> 
> >> Svyatoslav Ryhel (6):
> >>   configs: tegra-common-post: add GPIO keyboard as STDIN device
> >>   ARM: tegra: add SoC UID calculation function
> >>   board: asus: transformer: add ASUS Transformer T30 family support
> >>   board: asus: grouper: add Google Nexus 7 (2012) support
> >>   board: lg: x3: add Optimus 4X HD and Optimus Vu support
> >>   board: htc: endeavoru: add One X support
> >> 
> >> Thierry Reding (2):
> >>   ARM: tegra: Enable poweroff command on Jetson TX1 and Jetson Nano
> >>   ARM: tegra: Enable poweroff command on Jetson TX2
> >> 
> >> Tomasz Maciej Nowak (1):
> >>   ARM: dts: trimslice: sync SPI node with Linux dts
> >
> >Since I'm not sure if this is v8 or v9, given when v9 was posted, I've
> >applied this PR now, as I had said I wanted this in.  I do have two
> >requests for you Svyatoslav, however.  First, if there's changes that
> >are missing in master, please post those ASAP and we'll get them in, for
> >v2023.10. 
> 
> Patches Thierry applied are v8, while v9 has significant improvements.

I'm a little surprised by how this series developed. You've been urging
me since v6 to apply this, so I assumed this was mostly done. Having
another 3 versions with significant improvements come in after that
point indicates the opposite was true.

Thierry


signature.asc
Description: PGP signature


[PATCH] configs: Enable CONFIG_DM_SCSI in am57xx_hs_evm_usb

2023-08-21 Thread Andrew Davis
This should have already been enabled but was missed when converting the
base platform defconfig, fix this here.

Fixes: 3c5aa6caccab ("configs: Enable CONFIG_BLK in am57xx_evm and 
am57xx_hs_evm")
Signed-off-by: Andrew Davis 
---
 configs/am57xx_hs_evm_usb_defconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configs/am57xx_hs_evm_usb_defconfig 
b/configs/am57xx_hs_evm_usb_defconfig
index 404e714d4f0..9ed59ac9aec 100644
--- a/configs/am57xx_hs_evm_usb_defconfig
+++ b/configs/am57xx_hs_evm_usb_defconfig
@@ -70,7 +70,7 @@ CONFIG_NET_RETRY_COUNT=10
 CONFIG_BOOTP_SEND_HOSTNAME=y
 CONFIG_SPL_DM=y
 CONFIG_SPL_DM_SEQ_ALIAS=y
-CONFIG_SCSI_AHCI=y
+CONFIG_DWC_AHCI=y
 CONFIG_DFU_MMC=y
 CONFIG_DFU_RAM=y
 CONFIG_USB_FUNCTION_FASTBOOT=y
@@ -102,7 +102,7 @@ CONFIG_PMIC_PALMAS=y
 CONFIG_DM_REGULATOR=y
 CONFIG_DM_REGULATOR_PALMAS=y
 CONFIG_PALMAS_POWER=y
-CONFIG_SCSI_AHCI_PLAT=y
+CONFIG_DM_SCSI=y
 CONFIG_DM_SERIAL=y
 CONFIG_SPI=y
 CONFIG_DM_SPI=y
-- 
2.39.2



Re: [PATCH v4 1/2] musb-new: ti-musb: Handle usb dual-role feature as peripheral

2023-08-21 Thread Tom Rini
On Mon, Aug 21, 2023 at 03:40:42PM +0200, Marek Vasut wrote:
> On 8/21/23 14:16, Roger Quadros wrote:
> > Hi Julien,
> > 
> > On 21/08/2023 11:28, Julien Panis wrote:
> > > On 8/14/23 19:18, Tom Rini wrote:
> > > > On Thu, Jul 06, 2023 at 12:40:22PM +0200, Julien Panis wrote:
> > > > 
> > > > > This prevents from getting some 'No USB device found' error,
> > > > > in usb_ether_init() function for instance.
> > > > > 
> > > > > Signed-off-by: Julien Panis 
> > > > > Reviewed-by: Tony Lindgren 
> > > > > Reviewed-by: Nishanth Menon 
> > > > > ---
> > > > >    drivers/usb/musb-new/ti-musb.c | 1 +
> > > > >    1 file changed, 1 insertion(+)
> > > > > 
> > > > > diff --git a/drivers/usb/musb-new/ti-musb.c 
> > > > > b/drivers/usb/musb-new/ti-musb.c
> > > > > index 3be3f93dd85d..44697dc31387 100644
> > > > > --- a/drivers/usb/musb-new/ti-musb.c
> > > > > +++ b/drivers/usb/musb-new/ti-musb.c
> > > > > @@ -302,6 +302,7 @@ static int ti_musb_wrapper_bind(struct udevice 
> > > > > *parent)
> > > > >    dr_mode = usb_get_dr_mode(node);
> > > > >    switch (dr_mode) {
> > > > >    case USB_DR_MODE_PERIPHERAL:
> > > > > +    case USB_DR_MODE_OTG:
> > > > >    /* Bind MUSB device */
> > > > >    ret = device_bind_driver_to_node(parent,
> > > > >     "ti-musb-peripheral",
> > > > Julien, why don't we support OTG mode here instead?
> > > > 
> > > 
> > > Roger, can this be achieved with this controller ?
> > 
> > The Controller supports OTG mode but I'm not sure of the u-boot driver.
> > 
> > Bin / Stephan might know.
> 
> Surely the controller supports both Host and Peripheral mode.

And perhaps the other half of the disconnect here is that DWC2/DWC3 at
least behave in U-Boot as Marek is asking.

-- 
Tom


signature.asc
Description: PGP signature


Re: Dropping CONFIG_SCSI

2023-08-21 Thread Andrew Davis

On 8/20/23 12:59 PM, Tom Rini wrote:

On Sun, Aug 20, 2023 at 11:49:21AM -0600, Simon Glass wrote:

Hi,

The deadline has passed for the non-driver model SCSI code. The boards
below still have this enabled:

$ ./tools/moveconfig.py -f SCSI ~DM_SCSI
controlcenterdc am57xx_hs_evm_usb ls1088ardb_sdcard_qspi_SECURE_BOOT
ls1046ardb_sdcard_SECURE_BOOT ls1021atsn_qspi ls1021atsn_sdcard
highbank ls1021atwr_sdcard_ifc_SECURE_BOOT

Please can you take a look at migration?


Looking at am57xx_hs_evm_usb this is just a case where one of the
configs is out of sync with the rest as am57xx_evm and am57xx_hs_evm set
DM_SCSI.  Andrew, it might be worth looking at dropping
am57xx_hs_evm_usb specifically and adding a config fragment under
board/ti/am57xx/ for just the (I assume) SPL via USB case. And
dra7xx_evm can mirror that as well.



Yes, looks like several of the TI _hs_ configs have gotten out of sync again.

With this new config fragment stuff might be time to drop *all* the HS
defconfigs and add HS fragments..

I'll look into that, in the meantime sending a patch now to sync this one
missing config option.

Andrew


  1   2   >