Re: Pass user defines for Barebox build

2023-11-28 Thread Ahmad Fatoum
Hello Lior,

On 27.11.23 08:02, Lior Weintraub wrote:
> Hi guys,
> 
> Is there a formal way to pass user compilation flags into Barebox build?

There isn't. Both barebox and Linux have been broken in the past by distros
setting CFLAGS that force hardening options that require kernel/libc 
cooperation,
which didn't apply to barebox. For that reason, the variables were prefixed
with KBUILD_ and a way to inject variables into the build of barebox itself
(i.e. not host tools) is intentionally not provided.

What options do you want to inject?

> I couldn't find one so I just patched the main Makefile 
> diff --git a/Makefile b/Makefile
> index 471bbc2679..febc94b7f3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -443,7 +443,7 @@ KBUILD_CPPFLAGS:= -D__KERNEL__ -D__BAREBOX__ 
> $(LINUXINCLUDE) -fno-builti
>  KBUILD_CFLAGS   := -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs \
>-fno-strict-aliasing -fno-common -fshort-wchar \
> -Werror=implicit-function-declaration 
> -Werror=implicit-int \
> -   -Os -pipe -Wmissing-prototypes -std=gnu89
> +   -Os -pipe -Wmissing-prototypes -std=gnu89 
> $(BAREBOX_USER_CFLAGS)

USER is an unfortunate name, because there's already KBUILD_USERCFLAGS
and that user is short for userspace and not the user, who is building
barebox.

Cheers,
Ahmad

>  KBUILD_AFLAGS  := -D__ASSEMBLY__
>  KBUILD_AFLAGS_KERNEL :=
>  KBUILD_CFLAGS_KERNEL :=
> 
> This patch allowed me to set BAREBOX_CFLAGS environment when calling make.
> 
> Thanks, 
> Lior.
> 
> 
> 

-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |




Re: [PATCH 1/2] net: macb: fix dma_alloc for rx_buffer

2023-11-28 Thread Steffen Trumtrar



On 2023-11-28 at 17:56 +01, Lucas Stach  wrote:


Am Dienstag, dem 28.11.2023 um 17:29 +0100 schrieb Steffen Trumtrar:

rx_buffer gets dma_alloc'ed but is never dma_map'ed and therefor not
flushed before it is initially used.

Map the rx_buffer when the macb is initialized and unmap it on ether_halt.

While at it, cleanup the dma_alloc_coherent rx_ring/tx_ring, too.

Signed-off-by: Steffen Trumtrar 
---
 drivers/net/macb.c | 37 -
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index 260c1e806a..92f78f7253 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -63,10 +63,13 @@ struct macb_device {
unsigned inttx_head;

void*rx_buffer;
+   dma_addr_t  rx_buffer_phys;
void*tx_buffer;
void*rx_packet_buf;
struct macb_dma_desc*rx_ring;
+   dma_addr_t  rx_ring_phys;
struct macb_dma_desc*tx_ring;
+   dma_addr_t  tx_ring_phys;
struct macb_dma_desc*gem_q1_descs;

int rx_buffer_size;
@@ -181,7 +184,7 @@ static int gem_recv(struct eth_device *edev)
barrier();
status = macb->rx_ring[macb->rx_tail].ctrl;
length = MACB_BFEXT(RX_FRMLEN, status);
-   buffer = macb->rx_buffer + macb->rx_buffer_size * macb->rx_tail;
+   buffer = (void *)macb->rx_buffer_phys + macb->rx_buffer_size * 
macb->rx_tail;
dma_sync_single_for_cpu(macb->dev, (unsigned long)buffer, 
length,
DMA_FROM_DEVICE);
net_receive(edev, buffer, length);
@@ -221,7 +224,7 @@ static int macb_recv(struct eth_device *edev)
}

if (status & MACB_BIT(RX_EOF)) {
-   buffer = macb->rx_buffer + macb->rx_buffer_size * 
macb->rx_tail;
+   buffer = (void *)macb->rx_buffer_phys + 
macb->rx_buffer_size * macb->rx_tail;
length = MACB_BFEXT(RX_FRMLEN, status);
if (wrapped) {
unsigned int headlen, taillen;
@@ -232,12 +235,12 @@ static int macb_recv(struct eth_device *edev)
dma_sync_single_for_cpu(macb->dev, (unsigned 
long)buffer,
headlen, 
DMA_FROM_DEVICE);
memcpy(macb->rx_packet_buf, buffer, headlen);
-   dma_sync_single_for_cpu(macb->dev, (unsigned 
long)macb->rx_buffer,
+   dma_sync_single_for_cpu(macb->dev, (unsigned 
long)macb->rx_buffer_phys,


You can drop all those (unsigned long) casts in calls to
dma_sync_single, now that you are passing a argument of the proper
dma_addr_t type.



Thanks, will drop.


taillen, 
DMA_FROM_DEVICE);
memcpy(macb->rx_packet_buf + headlen, 
macb->rx_buffer, taillen);
dma_sync_single_for_device(macb->dev, (unsigned 
long)buffer,
headlen, 
DMA_FROM_DEVICE);
-   dma_sync_single_for_device(macb->dev, (unsigned 
long)macb->rx_buffer,
+   dma_sync_single_for_device(macb->dev, (unsigned 
long)macb->rx_buffer_phys,
taillen, 
DMA_FROM_DEVICE);
net_receive(edev, macb->rx_packet_buf, length);
} else {
@@ -377,7 +380,7 @@ static int gmac_init_dummy_tx_queues(struct macb_device 
*macb)
return 0;
 }

-static void macb_init(struct macb_device *macb)
+static int macb_init(struct macb_device *macb)
 {
unsigned long paddr, val = 0;
int i;
@@ -386,6 +389,11 @@ static void macb_init(struct macb_device *macb)
 * macb_halt should have been called at some point before now,
 * so we'll assume the controller is idle.
 */
+   macb->rx_buffer_phys = dma_map_single(macb->dev, macb->rx_buffer,
+ macb->rx_buffer_size * 
macb->rx_ring_size,
+ DMA_TO_DEVICE);


The RX buffer is used to hold data written by the device, so it must be
mapped with DMA_FROM_DEVICE.



Argh, of course :(


Thanks,
Steffen

--
Pengutronix e.K.| Dipl.-Inform. Steffen Trumtrar |
Steuerwalder Str. 21| https://www.pengutronix.de/|
31137 Hildesheim, Germany   | Phone: +49-5121-206917-0   |
Amtsgericht Hildesheim, HRA 2686| Fax:   +49-5121-206917-|



Re: [PATCH 2/2] net: macb: convert to volatile accesses

2023-11-28 Thread Ahmad Fatoum
Hello Steffen,

On 28.11.23 17:29, Steffen Trumtrar wrote:
> + writel(ctrl, >tx_ring[tx_head].ctrl);
> + writel((ulong)packet, >tx_ring[tx_head].addr);
>   dma_sync_single_for_device(macb->dev, (unsigned long)packet, length, 
> DMA_TO_DEVICE);

For this buffer dma_map_single is missing. I just sent out a series to implement
CONFIG_DMA_API_DEBUG by the way that should catch this (as well as the original
issue in the Rx path).

> - macb->gem_q1_descs[0].addr = 0;
> - macb->gem_q1_descs[0].ctrl = MACB_BIT(TX_WRAP) |
> - MACB_BIT(TX_LAST) | MACB_BIT(TX_USED);
> + writel(0, >gem_q1_descs[0].addr);
> + setbits_le32(>gem_q1_descs[0].ctrl,
> +  MACB_BIT(TX_WRAP) | MACB_BIT(TX_LAST) | MACB_BIT(TX_USED));

Should be writel to maintain previous semantics.

> - macb->tx_ring[TX_RING_SIZE - 1].addr |= MACB_BIT(TX_WRAP);
> + writel(MACB_BIT(TX_WRAP), >tx_ring[TX_RING_SIZE - 1].addr);

Should be a setbits_le32 to maintain previous semantics.

>   /* Disable the second priority rx queue */
> - macb->gem_q1_descs[1].addr = MACB_BIT(RX_USED) |
> - MACB_BIT(RX_WRAP);
> - macb->gem_q1_descs[1].ctrl = 0;
> + setbits_le32(>gem_q1_descs[1].addr,
> +  MACB_BIT(RX_USED) | MACB_BIT(RX_WRAP));

Should be a writel to maintain previous semantics.

> + writel(0, >gem_q1_descs[1].ctrl);
>  
>   gem_writel(macb, RQ1, (ulong)>gem_q1_descs[1]);
>   }
> 

Cheers,
Ahmad

-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |




Re: [PATCH 1/2] net: macb: fix dma_alloc for rx_buffer

2023-11-28 Thread Ahmad Fatoum
Hello Steffen,

On 28.11.23 17:29, Steffen Trumtrar wrote:
> rx_buffer gets dma_alloc'ed but is never dma_map'ed and therefor not
> flushed before it is initially used.
> 
> Map the rx_buffer when the macb is initialized and unmap it on ether_halt.
> 
> While at it, cleanup the dma_alloc_coherent rx_ring/tx_ring, too.
> 
> Signed-off-by: Steffen Trumtrar 
> ---
>  drivers/net/macb.c | 37 -
>  1 file changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> index 260c1e806a..92f78f7253 100644
> --- a/drivers/net/macb.c
> +++ b/drivers/net/macb.c
> @@ -63,10 +63,13 @@ struct macb_device {
>   unsigned inttx_head;
>  
>   void*rx_buffer;
> + dma_addr_t  rx_buffer_phys;
>   void*tx_buffer;
>   void*rx_packet_buf;
>   struct macb_dma_desc*rx_ring;
> + dma_addr_t  rx_ring_phys;
>   struct macb_dma_desc*tx_ring;
> + dma_addr_t  tx_ring_phys;
>   struct macb_dma_desc*gem_q1_descs;
>  
>   int rx_buffer_size;
> @@ -181,7 +184,7 @@ static int gem_recv(struct eth_device *edev)
>   barrier();
>   status = macb->rx_ring[macb->rx_tail].ctrl;
>   length = MACB_BFEXT(RX_FRMLEN, status);
> - buffer = macb->rx_buffer + macb->rx_buffer_size * macb->rx_tail;
> + buffer = (void *)macb->rx_buffer_phys + macb->rx_buffer_size * 
> macb->rx_tail;

For GEM-type NICs, rx_buffer_size is PKTSIZE (1518 bytes currently), which is 
not a multiple
of the cache line size of the 64 bytes cache line on the ZynqMP's Cortex-A53 
the driver
is supposed to support.

>   dma_sync_single_for_cpu(macb->dev, (unsigned long)buffer, 
> length,
>   DMA_FROM_DEVICE);

This means this could potentially invalidate adjacent buffer contents.


>   }
> @@ -891,8 +908,8 @@ static int macb_probe(struct device *dev)
>  
>   macb_init_rx_buffer_size(macb, PKTSIZE);

^ Here's where PKTSIZE comes from. I'd be in favor of changing the global 
PKTSIZE
definition to be a multiple of 64 bytes (or use DMA_ALIGNMENT, but this isn't
correctly set for ARM yet.  I just sent out a patch for that).

Cheers,
Ahmad

-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |




[PATCH 2/4] dma: add DMA API debugging support

2023-11-28 Thread Ahmad Fatoum
For DMA_FROM_DEVICE, calling dma_sync_single_for_cpu
before arch_sync_dma_for_device has been called is wrong:

  - Memory region is dirty in CPU cache
  - Device writes packet into region
  - CPU cache lines are written back
  - Buffer memory is corrupted

In order to spot such issues, let's add a new CONFIG_DMA_API_DEBUG
that will warn about mismatch in order.

Signed-off-by: Ahmad Fatoum 
---
 common/Kconfig   |  14 
 drivers/dma/Makefile |   1 +
 drivers/dma/debug.c  | 183 +++
 drivers/dma/debug.h  |  56 +
 drivers/dma/map.c|  13 ++-
 5 files changed, 266 insertions(+), 1 deletion(-)
 create mode 100644 drivers/dma/debug.c
 create mode 100644 drivers/dma/debug.h

diff --git a/common/Kconfig b/common/Kconfig
index 8bd8fa8df655..c8c23a8e03a2 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -1690,6 +1690,20 @@ config DEBUG_PROBES
  Most consoles do not implement a remove callback to remain operable 
until
  the very end. Consoles using DMA, however, must be removed.
 
+config DMA_API_DEBUG
+   bool "Enable debugging of DMA-API usage"
+   depends on HAS_DMA
+   help
+ Enable this option to debug the use of the DMA API by device drivers.
+ With this option you will be able to detect common bugs in device
+ drivers like double-freeing of DMA mappings or freeing mappings that
+ were never allocated.
+
+ This option causes a performance degradation.  Use only if you want to
+ debug device drivers and dma interactions.
+
+ If unsure, say N.
+
 config PBL_BREAK
bool "Execute software break on pbl start"
depends on ARM && (!CPU_32v4T && !ARCH_TEGRA)
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index e45476c23f14..b55c16e768d5 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_HAS_DMA)  += map.o
+obj-$(CONFIG_DMA_API_DEBUG)+= debug.o
 obj-$(CONFIG_MXS_APBH_DMA) += apbh_dma.o
diff --git a/drivers/dma/debug.c b/drivers/dma/debug.c
new file mode 100644
index ..b3bfbff9b2f5
--- /dev/null
+++ b/drivers/dma/debug.c
@@ -0,0 +1,183 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include 
+#include 
+#include "debug.h"
+
+static LIST_HEAD(dma_mappings);
+
+struct dma_debug_entry {
+   struct list_head list;
+   struct device*dev;
+   dma_addr_t   dev_addr;
+   size_t   size;
+   int  direction;
+};
+
+static const char *dir2name[] = {
+   [DMA_BIDIRECTIONAL] = "bidirectional",
+   [DMA_TO_DEVICE] = "to-device",
+   [DMA_FROM_DEVICE] = "from-device",
+   [DMA_NONE] = "none",
+};
+
+#define dma_dev_printf(level, args...) do {\
+   if (level > LOGLEVEL)   \
+   break;  \
+   dev_printf((level), args);  \
+   if ((level) <= MSG_WARNING) \
+   dump_stack();   \
+} while (0)
+
+#define dma_dev_warn(args...)  dma_dev_printf(MSG_WARNING, args)
+
+static void dma_printf(int level, struct dma_debug_entry *entry,
+  const char *fmt, ...)
+{
+   struct va_format vaf;
+   va_list va;
+
+   va_start(va, fmt);
+
+   vaf.fmt = fmt;
+   vaf.va = 
+
+   dma_dev_printf(level, entry->dev, "%s mapping 0x%llx+0x%zx: %pV\n",
+  dir2name[(entry)->direction], (u64)(entry)->dev_addr,
+  (entry)->size, );
+
+   va_end(va);
+}
+
+#define dma_warn(args...)  dma_printf(MSG_WARNING, args)
+#define dma_debug(args...) dma_printf(MSG_DEBUG, args)
+
+static inline int region_contains(struct dma_debug_entry *entry,
+ dma_addr_t buf_start, size_t buf_size)
+{
+   dma_addr_t dev_addr_end = entry->dev_addr + entry->size - 1;
+   dma_addr_t buf_end = buf_start + buf_size - 1;
+
+   /* Is the buffer completely within the mapping? */
+   if (entry->dev_addr <= buf_start && dev_addr_end >= buf_end)
+   return 1;
+
+   /* Does the buffer partially overlap the mapping? */
+   if (entry->dev_addr <= buf_end   && dev_addr_end >= buf_start)
+   return -1;
+
+   return 0;
+}
+
+static struct dma_debug_entry *
+dma_debug_entry_find(struct device *dev, dma_addr_t dev_addr, size_t size)
+{
+   struct dma_debug_entry *entry;
+
+   /*
+* DMA functions should be called with a device argument to support
+* non-1:1 device mappings.
+*/
+   if (!dev)
+   dma_dev_warn(NULL, "unportable NULL device passed with buffer 
0x%llx+0x%zx!\n",
+(u64)dev_addr, size);
+
+   list_for_each_entry(entry, _mappings, list) {
+   if (dev != entry->dev)
+   continue;
+
+   switch (region_contains(entry, dev_addr, size)) {
+   

[PATCH 4/4] mci: stm32_sdmmc2: correct usage of DMA API

2023-11-28 Thread Ahmad Fatoum
The new CONFIG_DMA_API_DEBUG option correctly detects that
dma_sync_single_for_device is called without dma_map_single.

In the particular case of the STM32 SDMMC2 driver, this shouldn't lead
to errors as dma_sync_single_for_cpu is only called after successful
DMA, not before. In other cases though, dirty cache lines may be evicted
and written back to cache, just before dma_sync_single_for_cpu,
resulting in memory corruption.

Switch to using dma_map_single to fix this.

Signed-off-by: Ahmad Fatoum 
---
 drivers/mci/stm32_sdmmc2.c | 41 --
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/drivers/mci/stm32_sdmmc2.c b/drivers/mci/stm32_sdmmc2.c
index 1bfef1ccf0eb..84969a29d0f4 100644
--- a/drivers/mci/stm32_sdmmc2.c
+++ b/drivers/mci/stm32_sdmmc2.c
@@ -257,11 +257,12 @@ static void stm32_sdmmc2_pwron(struct stm32_sdmmc2_priv 
*priv)
udelay(DIV_ROUND_UP(74 * USEC_PER_SEC, priv->mci.clock));
 }
 
-static void stm32_sdmmc2_start_data(struct stm32_sdmmc2_priv *priv,
+static int stm32_sdmmc2_start_data(struct stm32_sdmmc2_priv *priv,
struct mci_data *data, u32 data_length)
 {
unsigned int num_bytes = data->blocks * data->blocksize;
-   u32 data_ctrl, idmabase0;
+   dma_addr_t idmabase0;
+   u32 data_ctrl;
 
/* Configure the SDMMC DPSM (Data Path State Machine) */
data_ctrl = (__ilog2_u32(data->blocksize) <<
@@ -270,27 +271,27 @@ static void stm32_sdmmc2_start_data(struct 
stm32_sdmmc2_priv *priv,
 
if (data->flags & MMC_DATA_READ) {
data_ctrl |= SDMMC_DCTRL_DTDIR;
-   idmabase0 = (u32)data->dest;
+   idmabase0 = dma_map_single(priv->dev, (void *)data->src, 
num_bytes,
+  DMA_FROM_DEVICE);
} else {
-   idmabase0 = (u32)data->src;
+   idmabase0 = dma_map_single(priv->dev, (void *)data->src, 
num_bytes,
+  DMA_TO_DEVICE);
}
 
+   if (dma_mapping_error(priv->dev, idmabase0))
+   return DMA_ERROR_CODE;
+
/* Set the SDMMC DataLength value */
writel(data_length, priv->base + SDMMC_DLEN);
 
/* Write to SDMMC DCTRL */
writel(data_ctrl, priv->base + SDMMC_DCTRL);
 
-   if (data->flags & MMC_DATA_WRITE)
-   dma_sync_single_for_device(priv->dev, (unsigned long)idmabase0,
-  num_bytes, DMA_TO_DEVICE);
-   else
-   dma_sync_single_for_device(priv->dev, (unsigned long)idmabase0,
-  num_bytes, DMA_FROM_DEVICE);
-
/* Enable internal DMA */
writel(idmabase0, priv->base + SDMMC_IDMABASE0);
writel(SDMMC_IDMACTRL_IDMAEN, priv->base + SDMMC_IDMACTRL);
+
+   return idmabase0;
 }
 
 static void stm32_sdmmc2_start_cmd(struct stm32_sdmmc2_priv *priv,
@@ -415,7 +416,8 @@ static int stm32_sdmmc2_end_cmd(struct stm32_sdmmc2_priv 
*priv,
 
 static int stm32_sdmmc2_end_data(struct stm32_sdmmc2_priv *priv,
 struct mci_cmd *cmd,
-struct mci_data *data)
+struct mci_data *data,
+dma_addr_t dma_addr)
 {
u32 mask = SDMMC_STA_DCRCFAIL | SDMMC_STA_DTIMEOUT |
   SDMMC_STA_IDMATE | SDMMC_STA_DATAEND;
@@ -436,12 +438,10 @@ static int stm32_sdmmc2_end_data(struct stm32_sdmmc2_priv 
*priv,
return ret;
}
 
-   if (data->flags & MMC_DATA_WRITE)
-   dma_sync_single_for_cpu(priv->dev, (unsigned long)data->src,
-   num_bytes, DMA_TO_DEVICE);
+   if (data->flags & MMC_DATA_READ)
+   dma_unmap_single(priv->dev, dma_addr, num_bytes, 
DMA_FROM_DEVICE);
else
-   dma_sync_single_for_cpu(priv->dev, (unsigned long)data->dest,
-   num_bytes, DMA_FROM_DEVICE);
+   dma_unmap_single(priv->dev, dma_addr, num_bytes, DMA_TO_DEVICE);
 
if (status & SDMMC_STA_DCRCFAIL) {
dev_err(priv->dev, "error SDMMC_STA_DCRCFAIL (0x%x) for cmd 
%d\n",
@@ -481,12 +481,15 @@ static int stm32_sdmmc2_send_cmd(struct mci_host *mci, 
struct mci_cmd *cmd,
 {
struct stm32_sdmmc2_priv *priv = to_mci_host(mci);
u32 cmdat = data ? SDMMC_CMD_CMDTRANS : 0;
+   dma_addr_t dma_addr = DMA_ERROR_CODE;
u32 data_length = 0;
int ret;
 
if (data) {
data_length = data->blocks * data->blocksize;
-   stm32_sdmmc2_start_data(priv, data, data_length);
+   dma_addr = stm32_sdmmc2_start_data(priv, data, data_length);
+   if (dma_addr == DMA_ERROR_CODE)
+   return -EFAULT;
}
 
stm32_sdmmc2_start_cmd(priv, cmd, cmdat, data_length);
@@ -497,7 +500,7 @@ static int 

[PATCH 1/4] dma: factor out dma map generic implementations into file

2023-11-28 Thread Ahmad Fatoum
In preparation for adding optional debugging code for the DMA mapping
API, move the definition out of the header file into a source file.

Signed-off-by: Ahmad Fatoum 
---
 drivers/dma/Makefile |  1 +
 drivers/dma/map.c| 32 +++
 include/dma.h| 61 ++--
 3 files changed, 52 insertions(+), 42 deletions(-)
 create mode 100644 drivers/dma/map.c

diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index 8e1aac9f6f67..e45476c23f14 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
+obj-$(CONFIG_HAS_DMA)  += map.o
 obj-$(CONFIG_MXS_APBH_DMA) += apbh_dma.o
diff --git a/drivers/dma/map.c b/drivers/dma/map.c
new file mode 100644
index ..270a4899fd05
--- /dev/null
+++ b/drivers/dma/map.c
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#include 
+
+void dma_sync_single_for_cpu(struct device *dev, dma_addr_t address,
+size_t size, enum dma_data_direction dir)
+{
+   void *ptr = dma_to_cpu(dev, address);
+
+   arch_sync_dma_for_cpu(ptr, size, dir);
+}
+
+void dma_sync_single_for_device(struct device *dev, dma_addr_t address,
+ size_t size, enum 
dma_data_direction dir)
+{
+   void *ptr = dma_to_cpu(dev, address);
+
+   arch_sync_dma_for_device(ptr, size, dir);
+}
+
+dma_addr_t dma_map_single(struct device *dev, void *ptr,
+   size_t size, enum dma_data_direction 
dir)
+{
+   arch_sync_dma_for_device(ptr, size, dir);
+
+   return cpu_to_dma(dev, ptr);
+}
+
+void dma_unmap_single(struct device *dev, dma_addr_t dma_addr,
+   size_t size, enum dma_data_direction dir)
+{
+   dma_sync_single_for_cpu(dev, dma_addr, size, dir);
+}
diff --git a/include/dma.h b/include/dma.h
index 2a09b747d1e2..6eef55a7325d 100644
--- a/include/dma.h
+++ b/include/dma.h
@@ -68,8 +68,6 @@ static inline void *dma_to_cpu(struct device *dev, dma_addr_t 
addr)
return phys_to_virt(addr);
 }
 
-#ifndef __PBL__
-/* streaming DMA - implement the below calls to support HAS_DMA */
 #ifndef arch_sync_dma_for_cpu
 void arch_sync_dma_for_cpu(void *vaddr, size_t size,
   enum dma_data_direction dir);
@@ -79,57 +77,36 @@ void arch_sync_dma_for_cpu(void *vaddr, size_t size,
 void arch_sync_dma_for_device(void *vaddr, size_t size,
  enum dma_data_direction dir);
 #endif
+
+#ifndef __PBL__
+void dma_sync_single_for_cpu(struct device *dev, dma_addr_t address,
+size_t size, enum dma_data_direction dir);
+
+void dma_sync_single_for_device(struct device *dev, dma_addr_t address,
+   size_t size, enum dma_data_direction dir);
 #else
-#ifndef arch_sync_dma_for_cpu
 /*
  * assumes buffers are in coherent/uncached memory, e.g. because
  * MMU is only enabled in barebox_arm_entry which hasn't run yet.
  */
-static inline void arch_sync_dma_for_cpu(void *vaddr, size_t size,
-enum dma_data_direction dir)
+static inline void dma_sync_single_for_cpu(void *vaddr, size_t size,
+  enum dma_data_direction dir)
+{
+   barrier_data(vaddr);
+}
+
+static inline void dma_sync_single_for_device(void *vaddr, size_t size,
+ enum dma_data_direction dir)
 {
barrier_data(vaddr);
 }
 #endif
 
-#ifndef arch_sync_dma_for_device
-static inline void arch_sync_dma_for_device(void *vaddr, size_t size,
-   enum dma_data_direction dir)
-{
-   barrier_data(vaddr);
-}
-#endif
-#endif
+dma_addr_t dma_map_single(struct device *dev, void *ptr,
+ size_t size, enum dma_data_direction dir);
 
-static inline void dma_sync_single_for_cpu(struct device *dev, dma_addr_t 
address,
-  size_t size, enum dma_data_direction 
dir)
-{
-   void *ptr = dma_to_cpu(dev, address);
-
-   arch_sync_dma_for_cpu(ptr, size, dir);
-}
-
-static inline void dma_sync_single_for_device(struct device *dev, dma_addr_t 
address,
- size_t size, enum 
dma_data_direction dir)
-{
-   void *ptr = dma_to_cpu(dev, address);
-
-   arch_sync_dma_for_device(ptr, size, dir);
-}
-
-static inline dma_addr_t dma_map_single(struct device *dev, void *ptr,
-   size_t size, enum dma_data_direction 
dir)
-{
-   arch_sync_dma_for_device(ptr, size, dir);
-
-   return cpu_to_dma(dev, ptr);
-}
-
-static inline void dma_unmap_single(struct device *dev, dma_addr_t dma_addr,
-   size_t size, enum dma_data_direction dir)
-{
-   dma_sync_single_for_cpu(dev, dma_addr, size, dir);
-}
+void dma_unmap_single(struct device *dev, 

[PATCH 3/4] mci: core: remove broken, unneeded write bounce buffer

2023-11-28 Thread Ahmad Fatoum
mci_block_write uses a 512-byte long bounce buffer if the src argument
is not 4-byte aligned. This can never happen as src is the address of a
block cache chunk, which is always aligned for DMA, which is always a
multiple of 4 bytes. Furthermore, the bounce buffer is just a single
sector and the function may write multiple blocks resulting in
out-of-bounds read if that code hadn't been dead.

Signed-off-by: Ahmad Fatoum 
---
 drivers/mci/mci-core.c | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index 07eca96a9d61..280d08eb6253 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -218,7 +218,6 @@ static int mci_block_write(struct mci *mci, const void 
*src, int blocknum,
 {
struct mci_cmd cmd;
struct mci_data data;
-   const void *buf;
unsigned mmccmd;
int ret;
 
@@ -238,19 +237,12 @@ static int mci_block_write(struct mci *mci, const void 
*src, int blocknum,
else
mmccmd = MMC_CMD_WRITE_SINGLE_BLOCK;
 
-   if ((unsigned long)src & 0x3) {
-   memcpy(sector_buf, src, SECTOR_SIZE);
-   buf = sector_buf;
-   } else {
-   buf = src;
-   }
-
mci_setup_cmd(,
mmccmd,
mci->high_capacity != 0 ? blocknum : blocknum * 
mci->write_bl_len,
MMC_RSP_R1);
 
-   data.src = buf;
+   data.src = src;
data.blocks = blocks;
data.blocksize = mci->write_bl_len;
data.flags = MMC_DATA_WRITE;
-- 
2.39.2




[PATCH 0/4] dma: catch mistakes with CONFIG_DMA_API_DEBUG

2023-11-28 Thread Ahmad Fatoum
Cache invalidation issues around DMA accesses can be difficult to debug.
Motivated by recent fixes to the macb driver[1], let's add some optional
sanity checking to the DMA API inspired by the Linux CONFIG_DMA_API_DEBUG
option.

This would have caught the issue fixed by [1] in the macb driver and it
already caught a misuse of the API on the STM32MP system I tested it on.

Usage is simple: just enable it and ensure no warnings are printed.
All warnings are printed alongside the extents of the DMA buffer in
question and a stack trace at the time the check failed.

[1]: 
https://lore.barebox.org/barebox/20231128-v2023-08-0-topic-macb-v1-0-9faff73bc...@pengutronix.de/T/#t

Ahmad Fatoum (4):
  dma: factor out dma map generic implementations into file
  dma: add DMA API debugging support
  mci: core: remove broken, unneeded write bounce buffer
  mci: stm32_sdmmc2: correct usage of DMA API

 common/Kconfig |  14 +++
 drivers/dma/Makefile   |   2 +
 drivers/dma/debug.c| 183 +
 drivers/dma/debug.h|  56 
 drivers/dma/map.c  |  43 +
 drivers/mci/mci-core.c |  10 +-
 drivers/mci/stm32_sdmmc2.c |  41 +
 include/dma.h  |  61 -
 8 files changed, 340 insertions(+), 70 deletions(-)
 create mode 100644 drivers/dma/debug.c
 create mode 100644 drivers/dma/debug.h
 create mode 100644 drivers/dma/map.c

-- 
2.39.2




[PATCH] ARM: dma: define DMA_ALIGNMENT instead of defining dma_alloc

2023-11-28 Thread Ahmad Fatoum
There's a suitable fallback dma_alloc implementation already in dma.h,
which we can use as soon as we define DMA_ALIGNMENT.

Reviewed-by: Marco Felsch 
Signed-off-by: Ahmad Fatoum 
---
 arch/arm/include/asm/dma.h | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/dma.h b/arch/arm/include/asm/dma.h
index 53953a486393..0774a11c5a30 100644
--- a/arch/arm/include/asm/dma.h
+++ b/arch/arm/include/asm/dma.h
@@ -3,11 +3,7 @@
 
 #include 
 
-#define dma_alloc dma_alloc
-static inline void *dma_alloc(size_t size)
-{
-   return xmemalign(64, ALIGN(size, 64));
-}
+#define DMA_ALIGNMENT  64
 
 #ifndef CONFIG_MMU
 #define dma_alloc_coherent dma_alloc_coherent
-- 
2.39.2




Re: [PATCH 1/2] net: macb: fix dma_alloc for rx_buffer

2023-11-28 Thread Lucas Stach
Am Dienstag, dem 28.11.2023 um 17:29 +0100 schrieb Steffen Trumtrar:
> rx_buffer gets dma_alloc'ed but is never dma_map'ed and therefor not
> flushed before it is initially used.
> 
> Map the rx_buffer when the macb is initialized and unmap it on ether_halt.
> 
> While at it, cleanup the dma_alloc_coherent rx_ring/tx_ring, too.
> 
> Signed-off-by: Steffen Trumtrar 
> ---
>  drivers/net/macb.c | 37 -
>  1 file changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> index 260c1e806a..92f78f7253 100644
> --- a/drivers/net/macb.c
> +++ b/drivers/net/macb.c
> @@ -63,10 +63,13 @@ struct macb_device {
>   unsigned inttx_head;
>  
>   void*rx_buffer;
> + dma_addr_t  rx_buffer_phys;
>   void*tx_buffer;
>   void*rx_packet_buf;
>   struct macb_dma_desc*rx_ring;
> + dma_addr_t  rx_ring_phys;
>   struct macb_dma_desc*tx_ring;
> + dma_addr_t  tx_ring_phys;
>   struct macb_dma_desc*gem_q1_descs;
>  
>   int rx_buffer_size;
> @@ -181,7 +184,7 @@ static int gem_recv(struct eth_device *edev)
>   barrier();
>   status = macb->rx_ring[macb->rx_tail].ctrl;
>   length = MACB_BFEXT(RX_FRMLEN, status);
> - buffer = macb->rx_buffer + macb->rx_buffer_size * macb->rx_tail;
> + buffer = (void *)macb->rx_buffer_phys + macb->rx_buffer_size * 
> macb->rx_tail;
>   dma_sync_single_for_cpu(macb->dev, (unsigned long)buffer, 
> length,
>   DMA_FROM_DEVICE);
>   net_receive(edev, buffer, length);
> @@ -221,7 +224,7 @@ static int macb_recv(struct eth_device *edev)
>   }
>  
>   if (status & MACB_BIT(RX_EOF)) {
> - buffer = macb->rx_buffer + macb->rx_buffer_size * 
> macb->rx_tail;
> + buffer = (void *)macb->rx_buffer_phys + 
> macb->rx_buffer_size * macb->rx_tail;
>   length = MACB_BFEXT(RX_FRMLEN, status);
>   if (wrapped) {
>   unsigned int headlen, taillen;
> @@ -232,12 +235,12 @@ static int macb_recv(struct eth_device *edev)
>   dma_sync_single_for_cpu(macb->dev, (unsigned 
> long)buffer,
>   headlen, 
> DMA_FROM_DEVICE);
>   memcpy(macb->rx_packet_buf, buffer, headlen);
> - dma_sync_single_for_cpu(macb->dev, (unsigned 
> long)macb->rx_buffer,
> + dma_sync_single_for_cpu(macb->dev, (unsigned 
> long)macb->rx_buffer_phys,

You can drop all those (unsigned long) casts in calls to
dma_sync_single, now that you are passing a argument of the proper
dma_addr_t type.

>   taillen, 
> DMA_FROM_DEVICE);
>   memcpy(macb->rx_packet_buf + headlen, 
> macb->rx_buffer, taillen);
>   dma_sync_single_for_device(macb->dev, (unsigned 
> long)buffer,
>   headlen, 
> DMA_FROM_DEVICE);
> - dma_sync_single_for_device(macb->dev, (unsigned 
> long)macb->rx_buffer,
> + dma_sync_single_for_device(macb->dev, (unsigned 
> long)macb->rx_buffer_phys,
>   taillen, 
> DMA_FROM_DEVICE);
>   net_receive(edev, macb->rx_packet_buf, length);
>   } else {
> @@ -377,7 +380,7 @@ static int gmac_init_dummy_tx_queues(struct macb_device 
> *macb)
>   return 0;
>  }
>  
> -static void macb_init(struct macb_device *macb)
> +static int macb_init(struct macb_device *macb)
>  {
>   unsigned long paddr, val = 0;
>   int i;
> @@ -386,6 +389,11 @@ static void macb_init(struct macb_device *macb)
>* macb_halt should have been called at some point before now,
>* so we'll assume the controller is idle.
>*/
> + macb->rx_buffer_phys = dma_map_single(macb->dev, macb->rx_buffer,
> +   macb->rx_buffer_size * 
> macb->rx_ring_size,
> +   DMA_TO_DEVICE);

The RX buffer is used to hold data written by the device, so it must be
mapped with DMA_FROM_DEVICE.

Regards,
Lucas

> + if (dma_mapping_error(macb->dev, macb->rx_buffer_phys))
> + return -EFAULT;
>  
>   /* initialize DMA descriptors */
>   paddr = (ulong)macb->rx_buffer;
> @@ -442,6 +450,7 @@ static void macb_init(struct macb_device *macb)
>  
>   macb_or_gem_writel(macb, USRIO, val);
>  
> + return 0;
>  }
>  
>  static void macb_halt(struct eth_device *edev)
> @@ -460,6 +469,13 @@ static void macb_halt(struct 

[PATCH 1/2] net: macb: fix dma_alloc for rx_buffer

2023-11-28 Thread Steffen Trumtrar
rx_buffer gets dma_alloc'ed but is never dma_map'ed and therefor not
flushed before it is initially used.

Map the rx_buffer when the macb is initialized and unmap it on ether_halt.

While at it, cleanup the dma_alloc_coherent rx_ring/tx_ring, too.

Signed-off-by: Steffen Trumtrar 
---
 drivers/net/macb.c | 37 -
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index 260c1e806a..92f78f7253 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -63,10 +63,13 @@ struct macb_device {
unsigned inttx_head;
 
void*rx_buffer;
+   dma_addr_t  rx_buffer_phys;
void*tx_buffer;
void*rx_packet_buf;
struct macb_dma_desc*rx_ring;
+   dma_addr_t  rx_ring_phys;
struct macb_dma_desc*tx_ring;
+   dma_addr_t  tx_ring_phys;
struct macb_dma_desc*gem_q1_descs;
 
int rx_buffer_size;
@@ -181,7 +184,7 @@ static int gem_recv(struct eth_device *edev)
barrier();
status = macb->rx_ring[macb->rx_tail].ctrl;
length = MACB_BFEXT(RX_FRMLEN, status);
-   buffer = macb->rx_buffer + macb->rx_buffer_size * macb->rx_tail;
+   buffer = (void *)macb->rx_buffer_phys + macb->rx_buffer_size * 
macb->rx_tail;
dma_sync_single_for_cpu(macb->dev, (unsigned long)buffer, 
length,
DMA_FROM_DEVICE);
net_receive(edev, buffer, length);
@@ -221,7 +224,7 @@ static int macb_recv(struct eth_device *edev)
}
 
if (status & MACB_BIT(RX_EOF)) {
-   buffer = macb->rx_buffer + macb->rx_buffer_size * 
macb->rx_tail;
+   buffer = (void *)macb->rx_buffer_phys + 
macb->rx_buffer_size * macb->rx_tail;
length = MACB_BFEXT(RX_FRMLEN, status);
if (wrapped) {
unsigned int headlen, taillen;
@@ -232,12 +235,12 @@ static int macb_recv(struct eth_device *edev)
dma_sync_single_for_cpu(macb->dev, (unsigned 
long)buffer,
headlen, 
DMA_FROM_DEVICE);
memcpy(macb->rx_packet_buf, buffer, headlen);
-   dma_sync_single_for_cpu(macb->dev, (unsigned 
long)macb->rx_buffer,
+   dma_sync_single_for_cpu(macb->dev, (unsigned 
long)macb->rx_buffer_phys,
taillen, 
DMA_FROM_DEVICE);
memcpy(macb->rx_packet_buf + headlen, 
macb->rx_buffer, taillen);
dma_sync_single_for_device(macb->dev, (unsigned 
long)buffer,
headlen, 
DMA_FROM_DEVICE);
-   dma_sync_single_for_device(macb->dev, (unsigned 
long)macb->rx_buffer,
+   dma_sync_single_for_device(macb->dev, (unsigned 
long)macb->rx_buffer_phys,
taillen, 
DMA_FROM_DEVICE);
net_receive(edev, macb->rx_packet_buf, length);
} else {
@@ -377,7 +380,7 @@ static int gmac_init_dummy_tx_queues(struct macb_device 
*macb)
return 0;
 }
 
-static void macb_init(struct macb_device *macb)
+static int macb_init(struct macb_device *macb)
 {
unsigned long paddr, val = 0;
int i;
@@ -386,6 +389,11 @@ static void macb_init(struct macb_device *macb)
 * macb_halt should have been called at some point before now,
 * so we'll assume the controller is idle.
 */
+   macb->rx_buffer_phys = dma_map_single(macb->dev, macb->rx_buffer,
+ macb->rx_buffer_size * 
macb->rx_ring_size,
+ DMA_TO_DEVICE);
+   if (dma_mapping_error(macb->dev, macb->rx_buffer_phys))
+   return -EFAULT;
 
/* initialize DMA descriptors */
paddr = (ulong)macb->rx_buffer;
@@ -442,6 +450,7 @@ static void macb_init(struct macb_device *macb)
 
macb_or_gem_writel(macb, USRIO, val);
 
+   return 0;
 }
 
 static void macb_halt(struct eth_device *edev)
@@ -460,6 +469,13 @@ static void macb_halt(struct eth_device *edev)
 
/* Disable TX and RX, and clear statistics */
macb_writel(macb, NCR, MACB_BIT(CLRSTAT));
+
+   dma_unmap_single(macb->dev, macb->rx_buffer_phys,
+macb->rx_buffer_size * macb->rx_ring_size,
+DMA_TO_DEVICE);
+   free(macb->rx_buffer);
+   dma_free_coherent((void *)macb->rx_ring, macb->rx_ring_phys, 
RX_RING_BYTES(macb));
+   

[PATCH 0/2] net: macb: fix dma usage

2023-11-28 Thread Steffen Trumtrar
The rx_buffer is only dma_alloc'ed but never properly flushed.
Fix that.

While at it, also use proper volatile access instead of sw barriers.

Signed-off-by: Steffen Trumtrar 
---
Steffen Trumtrar (2):
  net: macb: fix dma_alloc for rx_buffer
  net: macb: convert to volatile accesses

 drivers/net/macb.c | 90 ++
 1 file changed, 50 insertions(+), 40 deletions(-)
---
base-commit: 5f200dd534c848dfa5d948334b6373f0310b8f73
change-id: 20231128-v2023-08-0-topic-macb-0c13ed91179d

Best regards,
-- 
Steffen Trumtrar 




[PATCH 2/2] net: macb: convert to volatile accesses

2023-11-28 Thread Steffen Trumtrar
Instead of directly reading from memory addresses and inserting
sw barriers to be sure that the compiler will not move loads/stores
behind this point, just use proper volatile writel/readl accesses.

Signed-off-by: Steffen Trumtrar 
---
 drivers/net/macb.c | 53 ++---
 1 file changed, 22 insertions(+), 31 deletions(-)

diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index 92f78f7253..c9a7e395d6 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -119,17 +119,15 @@ static int macb_send(struct eth_device *edev, void 
*packet,
macb->tx_head++;
}
 
-   macb->tx_ring[tx_head].ctrl = ctrl;
-   macb->tx_ring[tx_head].addr = (ulong)packet;
-   barrier();
+   writel(ctrl, >tx_ring[tx_head].ctrl);
+   writel((ulong)packet, >tx_ring[tx_head].addr);
dma_sync_single_for_device(macb->dev, (unsigned long)packet, length, 
DMA_TO_DEVICE);
macb_writel(macb, NCR, MACB_BIT(TE) | MACB_BIT(RE) | MACB_BIT(TSTART));
 
start = get_time_ns();
ret = -ETIMEDOUT;
do {
-   barrier();
-   ctrl = macb->tx_ring[0].ctrl;
+   ctrl = readl(>tx_ring[0].ctrl);
if (ctrl & MACB_BIT(TX_USED)) {
ret = 0;
break;
@@ -154,18 +152,17 @@ static void reclaim_rx_buffers(struct macb_device *macb,
 
i = macb->rx_tail;
while (i > new_tail) {
-   macb->rx_ring[i].addr &= ~MACB_BIT(RX_USED);
+   clrbits_le32(>rx_ring[i].addr, MACB_BIT(RX_USED));
i++;
if (i > macb->rx_ring_size)
i = 0;
}
 
while (i < new_tail) {
-   macb->rx_ring[i].addr &= ~MACB_BIT(RX_USED);
+   clrbits_le32(>rx_ring[i].addr, MACB_BIT(RX_USED));
i++;
}
 
-   barrier();
macb->rx_tail = new_tail;
 }
 
@@ -177,12 +174,10 @@ static int gem_recv(struct eth_device *edev)
u32 status;
 
for (;;) {
-   barrier();
-   if (!(macb->rx_ring[macb->rx_tail].addr & MACB_BIT(RX_USED)))
+   if (!(readl(>rx_ring[macb->rx_tail].addr) & 
MACB_BIT(RX_USED)))
return -1;
 
-   barrier();
-   status = macb->rx_ring[macb->rx_tail].ctrl;
+   status = readl(>rx_ring[macb->rx_tail].ctrl);
length = MACB_BFEXT(RX_FRMLEN, status);
buffer = (void *)macb->rx_buffer_phys + macb->rx_buffer_size * 
macb->rx_tail;
dma_sync_single_for_cpu(macb->dev, (unsigned long)buffer, 
length,
@@ -190,8 +185,7 @@ static int gem_recv(struct eth_device *edev)
net_receive(edev, buffer, length);
dma_sync_single_for_device(macb->dev, (unsigned long)buffer, 
length,
   DMA_FROM_DEVICE);
-   macb->rx_ring[macb->rx_tail].addr &= ~MACB_BIT(RX_USED);
-   barrier();
+   clrbits_le32(>rx_ring[macb->rx_tail].addr, 
MACB_BIT(RX_USED));
 
macb->rx_tail++;
if (macb->rx_tail >= macb->rx_ring_size)
@@ -211,12 +205,10 @@ static int macb_recv(struct eth_device *edev)
u32 status;
 
for (;;) {
-   barrier();
-   if (!(macb->rx_ring[rx_tail].addr & MACB_BIT(RX_USED)))
+   if (!(readl(>rx_ring[rx_tail].addr) & MACB_BIT(RX_USED)))
return -1;
 
-   barrier();
-   status = macb->rx_ring[rx_tail].ctrl;
+   status = readl(>rx_ring[rx_tail].ctrl);
if (status & MACB_BIT(RX_SOF)) {
if (rx_tail != macb->rx_tail)
reclaim_rx_buffers(macb, rx_tail);
@@ -250,7 +242,6 @@ static int macb_recv(struct eth_device *edev)
dma_sync_single_for_device(macb->dev, (unsigned 
long)buffer, length,
DMA_FROM_DEVICE);
}
-   barrier();
if (++rx_tail >= macb->rx_ring_size)
rx_tail = 0;
reclaim_rx_buffers(macb, rx_tail);
@@ -370,9 +361,9 @@ static int gmac_init_dummy_tx_queues(struct macb_device 
*macb)
if (queue_mask & (1 << i))
num_queues++;
 
-   macb->gem_q1_descs[0].addr = 0;
-   macb->gem_q1_descs[0].ctrl = MACB_BIT(TX_WRAP) |
-   MACB_BIT(TX_LAST) | MACB_BIT(TX_USED);
+   writel(0, >gem_q1_descs[0].addr);
+   setbits_le32(>gem_q1_descs[0].ctrl,
+MACB_BIT(TX_WRAP) | MACB_BIT(TX_LAST) | MACB_BIT(TX_USED));
 
for (i = 1; i < num_queues; i++)
gem_writel_queue_TBQP(macb, (ulong)macb->gem_q1_descs, i - 1);
@@ -398,17 +389,17 @@ static int macb_init(struct macb_device *macb)
/* initialize 

[PATCH] commands: regulator: add option to list provider devices

2023-11-28 Thread Ahmad Fatoum
Especially on SCMI boards, it isn't immediately clear if e.g. reg11 is
supplied by a directly accessed regulator or by the secure world.

Give the regulator command a -D option to list devices that the
regulator is associated with as well.

Signed-off-by: Ahmad Fatoum 
---
 commands/regulator.c | 11 ---
 drivers/regulator/core.c | 27 ---
 include/regulator.h  |  5 -
 3 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/commands/regulator.c b/commands/regulator.c
index e6b2f4852db4..3e38b616469f 100644
--- a/commands/regulator.c
+++ b/commands/regulator.c
@@ -11,9 +11,10 @@
 static int do_regulator(int argc, char *argv[])
 {
struct regulator *chosen;
+   unsigned flags = 0;
int opt, ret;
 
-   while ((opt = getopt(argc, argv, "e:d:")) > 0) {
+   while ((opt = getopt(argc, argv, "e:d:D")) > 0) {
switch (opt) {
case 'e':
case 'd':
@@ -27,12 +28,15 @@ static int do_regulator(int argc, char *argv[])
 : regulator_disable(chosen);
regulator_put(chosen);
return ret;
+   case 'D':
+   flags |= REGULATOR_PRINT_DEVS;
+   break;
default:
return COMMAND_ERROR_USAGE;
}
}
 
-   regulators_print();
+   regulators_print(flags);
return 0;
 }
 
@@ -42,12 +46,13 @@ BAREBOX_CMD_HELP_START(regulator)
BAREBOX_CMD_HELP_TEXT("Options:")
BAREBOX_CMD_HELP_OPT("-e REGULATOR\t", "enable REGULATOR")
BAREBOX_CMD_HELP_OPT("-d REGULATOR\t", "disable REGULATOR")
+   BAREBOX_CMD_HELP_OPT("-D\t", "list provider devices of regulators")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(regulator)
.cmd= do_regulator,
BAREBOX_CMD_DESC("list and control regulators")
-   BAREBOX_CMD_OPTS("[-ed] [REGULATOR]")
+   BAREBOX_CMD_OPTS("[-edD] [REGULATOR]")
BAREBOX_CMD_GROUP(CMD_GRP_HWMANIP)
BAREBOX_CMD_HELP(cmd_regulator_help)
 BAREBOX_CMD_END
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index b20a48e0f63c..bbba3b0b5712 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -729,20 +729,32 @@ int regulator_get_voltage(struct regulator *regulator)
 }
 EXPORT_SYMBOL_GPL(regulator_get_voltage);
 
-static void regulator_print_one(struct regulator_dev *rdev, int level)
+static int regulator_name_indent(unsigned flags)
 {
+   return 30 + (flags & REGULATOR_PRINT_DEVS ? 50 : 0);
+}
+
+static void regulator_print_one(struct regulator_dev *rdev, int level, 
unsigned flags)
+{
+   const char *name = rdev->name;
struct regulator *r;
+   char buf[256];
 
if (!rdev)
return;
 
+   if (flags & REGULATOR_PRINT_DEVS) {
+   snprintf(buf, sizeof(buf), "%s  %s", dev_name(rdev->dev), 
rdev->name);
+   name = buf;
+   }
+
printf("%*s%-*s %6d %10d %10d\n", level * 3, "",
-  30 - level * 3,
-  rdev->name, rdev->enable_count, rdev->min_uv, rdev->max_uv);
+  regulator_name_indent(flags) - level * 3,
+  name, rdev->enable_count, rdev->min_uv, rdev->max_uv);
 
list_for_each_entry(r, >consumer_list, list) {
if (r->rdev_consumer)
-   regulator_print_one(r->rdev_consumer, level + 1);
+   regulator_print_one(r->rdev_consumer, level + 1, flags);
else
printf("%*s%s\n", (level + 1) * 3, "", r->dev ? 
dev_name(r->dev) : "none");
}
@@ -751,13 +763,14 @@ static void regulator_print_one(struct regulator_dev 
*rdev, int level)
 /*
  * regulators_print - print informations about all regulators
  */
-void regulators_print(void)
+void regulators_print(unsigned flags)
 {
struct regulator_dev *rdev;
 
-   printf("%-30s %6s %10s %10s\n", "name", "enable", "min_uv", "max_uv");
+   printf("%-*s %6s %10s %10s\n", regulator_name_indent(flags),
+  "name", "enable", "min_uv", "max_uv");
list_for_each_entry(rdev, _list, list) {
if (!rdev->supply)
-   regulator_print_one(rdev, 0);
+   regulator_print_one(rdev, 0, flags);
}
 }
diff --git a/include/regulator.h b/include/regulator.h
index 240dbce5e7da..135fe6d91fd3 100644
--- a/include/regulator.h
+++ b/include/regulator.h
@@ -2,6 +2,8 @@
 #ifndef __REGULATOR_H
 #define __REGULATOR_H
 
+#include 
+
 struct device;
 
 /* struct regulator is an opaque object for consumers */
@@ -155,7 +157,8 @@ static inline int of_regulator_register(struct 
regulator_dev *rd,
 #endif
 int dev_regulator_register(struct regulator_dev *rd, const char *name);
 
-void regulators_print(void);
+#define REGULATOR_PRINT_DEVS   BIT(0)
+void regulators_print(unsigned flags);
 
 const char