[PATCH 0/2] Add trace events for Qualcomm GENI SPI drivers

[PATCH 0/2] Add trace events for Qualcomm GENI SPI drivers

[Praveen Talari]

Add tracepoints to the Qualcomm GENI (Generic Interface) SPI driver.
These trace events enable runtime debugging and performance analysis
of SPI operations.

The trace events capture SPI clock configuration, FIFO parameters,
transfer details, interrupt status, and actual transmitted/received
data in hexadecimal format.

Usage examples:

Enable all SPI traces:
  echo 1 > /sys/kernel/debug/tracing/events/qcom_geni_spi/enable
  cat /sys/kernel/debug/tracing/trace_pipe

Example trace output:

71.364028: geni_spi_fifo_params: 888000.spi: cs=0 mode=0x00000020
   mode_changed=0x00000020 cs_changed=0
71.364054: geni_spi_clk_cfg: 888000.spi: req_hz=10000000
   sclk_hz=100000000 clk_idx=5 clk_div=10 bpw=8
71.364095: geni_spi_transfer: 888000.spi: len=16 m_cmd=0x00000003
71.364096: geni_spi_tx_data: 888000.spi: tx_len=16 tx_rem=0 data=56 f1
   0d 95 c1 09 33 d2 27 e7 ec 9d 9c e2 11 ff
71.364121: geni_spi_irq: 888000.spi: m_irq=0x08000081 dma_tx=0x00000000
   dma_rx=0x00000000
71.364126: geni_spi_rx_data: 888000.spi: rx_len=16 rx_rem=0 data=56 f1
   0d 95 c1 09 33 d2 27 e7 ec 9d 9c e2 11 ff

Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
Praveen Talari (2):
      spi: qcom-geni: trace: Add trace events for Qualcomm GENI SPI
      spi: qcom-geni: Add trace events for Qualcomm GENI SPI driver

 drivers/spi/spi-geni-qcom.c          |  17 ++++
 include/trace/events/qcom_geni_spi.h | 147 +++++++++++++++++++++++++++++++++++
 2 files changed, 164 insertions(+)
---
base-commit: 1f5ffc672165ff851063a5fd044b727ab2517ae3
change-id: 20260506-add-tracepoints-for-qcom-geni-spi-e31457c2267c

Best regards,
-- 
Praveen Talari <praveen.talari@oss.qualcomm.com>

[PATCH v1 1/2] spi: qcom-geni: trace: Add trace events for Qualcomm GENI SPI

[Praveen Talari]

Add tracepoint support to the Qualcomm GENI SPI driver to provide
runtime visibility into driver behavior without requiring invasive debug
patches.

The trace events cover clock and FIFO parameter configuration,
transfer metadata, interrupt status, and optional TX/RX payload dumps
to be making it easier to diagnose communication issues in the field..

Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
 include/trace/events/qcom_geni_spi.h | 147 +++++++++++++++++++++++++++++++++++
 1 file changed, 147 insertions(+)

diff --git a/include/trace/events/qcom_geni_spi.h b/include/trace/events/qcom_geni_spi.h
new file mode 100644
index 000000000000..a2e6ff9df520
--- /dev/null
+++ b/include/trace/events/qcom_geni_spi.h
@@ -0,0 +1,147 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM qcom_geni_spi
+
+#if !defined(_TRACE_QCOM_GENI_SPI_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_QCOM_GENI_SPI_H
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(geni_spi_fifo_params,
+	    TP_PROTO(struct device *dev, u8 cs, u32 mode,
+		     u32 mode_changed, bool cs_changed),
+	    TP_ARGS(dev, cs, mode, mode_changed, cs_changed),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(u8, cs)
+			     __field(u32, mode)
+			     __field(u32, mode_changed)
+			     __field(bool, cs_changed)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->cs = cs;
+			   __entry->mode = mode;
+			   __entry->mode_changed = mode_changed;
+			   __entry->cs_changed = cs_changed;
+	    ),
+
+	    TP_printk("%s: cs=%u mode=0x%08x mode_changed=0x%08x cs_changed=%d",
+		      __get_str(name), __entry->cs, __entry->mode,
+		      __entry->mode_changed, __entry->cs_changed)
+);
+
+TRACE_EVENT(geni_spi_clk_cfg,
+	    TP_PROTO(struct device *dev, unsigned long req_hz,
+		     unsigned long sclk_hz, unsigned int clk_idx,
+		     unsigned int clk_div, unsigned int bpw),
+	    TP_ARGS(dev, req_hz, sclk_hz, clk_idx, clk_div, bpw),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(unsigned long, req_hz)
+			     __field(unsigned long, sclk_hz)
+			     __field(unsigned int, clk_idx)
+			     __field(unsigned int, clk_div)
+			     __field(unsigned int, bpw)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->req_hz = req_hz;
+			   __entry->sclk_hz = sclk_hz;
+			   __entry->clk_idx = clk_idx;
+			   __entry->clk_div = clk_div;
+			   __entry->bpw = bpw;
+	    ),
+
+	    TP_printk("%s: req_hz=%lu sclk_hz=%lu clk_idx=%u clk_div=%u bpw=%u",
+		      __get_str(name), __entry->req_hz, __entry->sclk_hz,
+		      __entry->clk_idx, __entry->clk_div, __entry->bpw)
+);
+
+TRACE_EVENT(geni_spi_transfer,
+	    TP_PROTO(struct device *dev, unsigned int len, u32 m_cmd),
+	    TP_ARGS(dev, len, m_cmd),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(unsigned int, len)
+			     __field(u32, m_cmd)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->len = len;
+			   __entry->m_cmd = m_cmd;
+	    ),
+
+	    TP_printk("%s: len=%u m_cmd=0x%08x",
+		      __get_str(name), __entry->len, __entry->m_cmd)
+);
+
+TRACE_EVENT(geni_spi_irq,
+	    TP_PROTO(struct device *dev, u32 m_irq, u32 dma_tx, u32 dma_rx),
+	    TP_ARGS(dev, m_irq, dma_tx, dma_rx),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(u32, m_irq)
+			     __field(u32, dma_tx)
+			     __field(u32, dma_rx)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->m_irq = m_irq;
+			   __entry->dma_tx = dma_tx;
+			   __entry->dma_rx = dma_rx;
+	    ),
+
+	    TP_printk("%s: m_irq=0x%08x dma_tx=0x%08x dma_rx=0x%08x",
+		      __get_str(name), __entry->m_irq, __entry->dma_tx,
+		      __entry->dma_rx)
+);
+
+TRACE_EVENT(geni_spi_tx_data,
+	    TP_PROTO(struct device *dev, const u8 *buf, unsigned int len,
+		     unsigned int tx_rem),
+	    TP_ARGS(dev, buf, len, tx_rem),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(unsigned int, len)
+			     __field(unsigned int, tx_rem)
+			     __dynamic_array(u8, data, len)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->len = len;
+			   __entry->tx_rem = tx_rem;
+			   memcpy(__get_dynamic_array(data), buf, len);
+	    ),
+
+	    TP_printk("%s: tx_len=%u tx_rem=%u data=%s",
+		      __get_str(name), __entry->len, __entry->tx_rem,
+		      __print_hex(__get_dynamic_array(data), __entry->len))
+);
+
+TRACE_EVENT(geni_spi_rx_data,
+	    TP_PROTO(struct device *dev, const u8 *buf, unsigned int len,
+		     unsigned int rx_rem),
+	    TP_ARGS(dev, buf, len, rx_rem),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(unsigned int, len)
+			     __field(unsigned int, rx_rem)
+			     __dynamic_array(u8, data, len)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->len = len;
+			   __entry->rx_rem = rx_rem;
+			   memcpy(__get_dynamic_array(data), buf, len);
+	    ),
+
+	    TP_printk("%s: rx_len=%u rx_rem=%u data=%s",
+		      __get_str(name), __entry->len, __entry->rx_rem,
+		      __print_hex(__get_dynamic_array(data), __entry->len))
+);
+
+#endif /* _TRACE_QCOM_GENI_SPI_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>

-- 
2.34.1

[PATCH v1 2/2] spi: qcom-geni: Add trace events for Qualcomm GENI SPI driver

[Praveen Talari]

Add tracepoints to the Qualcomm GENI (Generic Interface) SPI driver.
These trace events enable runtime debugging and performance analysis
of SPI operations.

The trace events capture SPI clock configuration, FIFO parameters,
transfer details, interrupt status, and actual transmitted/received
data in hexadecimal format.

Usage examples:

Enable all SPI traces:
  echo 1 > /sys/kernel/debug/tracing/events/qcom_geni_spi/enable
  cat /sys/kernel/debug/tracing/trace_pipe

Example trace output:

71.364028: geni_spi_fifo_params: 888000.spi: cs=0 mode=0x00000020
   mode_changed=0x00000020 cs_changed=0
71.364054: geni_spi_clk_cfg: 888000.spi: req_hz=10000000
   sclk_hz=100000000 clk_idx=5 clk_div=10 bpw=8
71.364095: geni_spi_transfer: 888000.spi: len=16 m_cmd=0x00000003
71.364096: geni_spi_tx_data: 888000.spi: tx_len=16 tx_rem=0 data=56 f1
   0d 95 c1 09 33 d2 27 e7 ec 9d 9c e2 11 ff
71.364121: geni_spi_irq: 888000.spi: m_irq=0x08000081 dma_tx=0x00000000
   dma_rx=0x00000000
71.364126: geni_spi_rx_data: 888000.spi: rx_len=16 rx_rem=0 data=56 f1
   0d 95 c1 09 33 d2 27 e7 ec 9d 9c e2 11 ff

Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
 drivers/spi/spi-geni-qcom.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index d5fb0edc8e0c..4da888359cfc 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -1,6 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
 // Copyright (c) 2017-2018, The Linux foundation. All rights reserved.
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/qcom_geni_spi.h>
+
 #include <linux/clk.h>
 #include <linux/dmaengine.h>
 #include <linux/dma-mapping.h>
@@ -332,6 +335,9 @@ static int geni_spi_set_clock_and_bw(struct spi_geni_master *mas,
 	writel(clk_sel, se->base + SE_GENI_CLK_SEL);
 	writel(m_clk_cfg, se->base + GENI_SER_M_CLK_CFG);
 
+	trace_geni_spi_clk_cfg(mas->dev, clk_hz, mas->cur_sclk_hz, idx, div,
+			       mas->cur_bits_per_word);
+
 	/* Set BW quota for CPU as driver supports FIFO mode only. */
 	se->icc_paths[CPU_TO_GENI].avg_bw = Bps_to_icc(mas->cur_speed_hz);
 	ret = geni_icc_set_bw(se);
@@ -366,6 +372,9 @@ static int setup_fifo_params(struct spi_device *spi_slv,
 	if ((mode_changed & SPI_CS_HIGH) || (cs_changed && (spi_slv->mode & SPI_CS_HIGH)))
 		writel((spi_slv->mode & SPI_CS_HIGH) ? BIT(chipselect) : 0, se->base + SE_SPI_DEMUX_OUTPUT_INV);
 
+	trace_geni_spi_fifo_params(mas->dev, chipselect, spi_slv->mode,
+				   mode_changed, cs_changed);
+
 	return 0;
 }
 
@@ -717,6 +726,7 @@ static bool geni_spi_handle_tx(struct spi_geni_master *mas)
 		max_bytes = mas->tx_rem_bytes;
 
 	tx_buf = mas->cur_xfer->tx_buf + mas->cur_xfer->len - mas->tx_rem_bytes;
+
 	while (i < max_bytes) {
 		unsigned int j;
 		unsigned int bytes_to_write;
@@ -729,6 +739,7 @@ static bool geni_spi_handle_tx(struct spi_geni_master *mas)
 		iowrite32_rep(se->base + SE_GENI_TX_FIFOn, &fifo_word, 1);
 	}
 	mas->tx_rem_bytes -= max_bytes;
+	trace_geni_spi_tx_data(mas->dev, tx_buf, max_bytes, mas->tx_rem_bytes);
 	if (!mas->tx_rem_bytes) {
 		writel(0, se->base + SE_GENI_TX_WATERMARK_REG);
 		return false;
@@ -778,6 +789,8 @@ static void geni_spi_handle_rx(struct spi_geni_master *mas)
 			rx_buf[i++] = fifo_byte[j];
 	}
 	mas->rx_rem_bytes -= rx_bytes;
+
+	trace_geni_spi_rx_data(mas->dev, rx_buf, rx_bytes, mas->rx_rem_bytes);
 }
 
 static int setup_se_xfer(struct spi_transfer *xfer,
@@ -861,6 +874,8 @@ static int setup_se_xfer(struct spi_transfer *xfer,
 	spin_lock_irq(&mas->lock);
 	geni_se_setup_m_cmd(se, m_cmd, m_params);
 
+	trace_geni_spi_transfer(mas->dev, len, m_cmd);
+
 	if (mas->cur_xfer_mode == GENI_SE_DMA) {
 		if (m_cmd & SPI_RX_ONLY)
 			geni_se_rx_init_dma(se, sg_dma_address(xfer->rx_sg.sgl),
@@ -915,6 +930,8 @@ static irqreturn_t geni_spi_isr(int irq, void *data)
 	if (!m_irq && !dma_tx_status && !dma_rx_status)
 		return IRQ_NONE;
 
+	trace_geni_spi_irq(mas->dev, m_irq, dma_tx_status, dma_rx_status);
+
 	if (m_irq & (M_CMD_OVERRUN_EN | M_ILLEGAL_CMD_EN | M_CMD_FAILURE_EN |
 		     M_RX_FIFO_RD_ERR_EN | M_RX_FIFO_WR_ERR_EN |
 		     M_TX_FIFO_RD_ERR_EN | M_TX_FIFO_WR_ERR_EN))

-- 
2.34.1

Re: [PATCH v1 1/2] spi: qcom-geni: trace: Add trace events for Qualcomm GENI SPI

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 408 bytes --]

On Wed, May 06, 2026 at 10:59:42PM +0530, Praveen Talari wrote:
> Add tracepoint support to the Qualcomm GENI SPI driver to provide
> runtime visibility into driver behavior without requiring invasive debug
> patches.

> +TRACE_EVENT(geni_spi_tx_data,
> +TRACE_EVENT(geni_spi_rx_data,

At least these feel like they really should be generic events, there
hopefully isn't anything driver specific about them.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 2/2] spi: qcom-geni: Add trace events for Qualcomm GENI SPI driver

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 393 bytes --]

On Wed, May 06, 2026 at 10:59:43PM +0530, Praveen Talari wrote:

> @@ -717,6 +726,7 @@ static bool geni_spi_handle_tx(struct spi_geni_master *mas)
>  		max_bytes = mas->tx_rem_bytes;
>  
>  	tx_buf = mas->cur_xfer->tx_buf + mas->cur_xfer->len - mas->tx_rem_bytes;
> +
>  	while (i < max_bytes) {
>  		unsigned int j;
>  		unsigned int bytes_to_write;

Unrelated whitespace change.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 1/2] spi: qcom-geni: trace: Add trace events for Qualcomm GENI SPI

[Praveen Talari]

Hi Mark,

On 07-05-2026 06:32, Mark Brown wrote:
> On Wed, May 06, 2026 at 10:59:42PM +0530, Praveen Talari wrote:
>> Add tracepoint support to the Qualcomm GENI SPI driver to provide
>> runtime visibility into driver behavior without requiring invasive debug
>> patches.
>> +TRACE_EVENT(geni_spi_tx_data,
>> +TRACE_EVENT(geni_spi_rx_data,
> At least these feel like they really should be generic events, there
> hopefully isn't anything driver specific about them.

Initially implemented as a generic event; however, splitting into 
separate TX and RX events may be more appropriate.

Which approach would you prefer?


Thanks,

Praveen Talari

Re: [PATCH v1 2/2] spi: qcom-geni: Add trace events for Qualcomm GENI SPI driver

[Praveen Talari]

Hi Mark,

On 07-05-2026 06:37, Mark Brown wrote:
> On Wed, May 06, 2026 at 10:59:43PM +0530, Praveen Talari wrote:
>
>> @@ -717,6 +726,7 @@ static bool geni_spi_handle_tx(struct spi_geni_master *mas)
>>   		max_bytes = mas->tx_rem_bytes;
>>   
>>   	tx_buf = mas->cur_xfer->tx_buf + mas->cur_xfer->len - mas->tx_rem_bytes;
>> +
>>   	while (i < max_bytes) {
>>   		unsigned int j;
>>   		unsigned int bytes_to_write;
> Unrelated whitespace change.

oh its my bad. will fix in next patch set.


Thanks,

Praveen Talari

Re: [PATCH v1 1/2] spi: qcom-geni: trace: Add trace events for Qualcomm GENI SPI

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 456 bytes --]

On Thu, May 07, 2026 at 08:58:02AM +0530, Praveen Talari wrote:
> On 07-05-2026 06:32, Mark Brown wrote:

> > At least these feel like they really should be generic events, there
> > hopefully isn't anything driver specific about them.

> Initially implemented as a generic event; however, splitting into separate
> TX and RX events may be more appropriate.

> Which approach would you prefer?

By generic I mean this should not be driver specific at all.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 1/2] spi: qcom-geni: trace: Add trace events for Qualcomm GENI SPI

[Praveen Talari]

Hi Mark,

On 07-05-2026 13:43, Mark Brown wrote:
> On Thu, May 07, 2026 at 08:58:02AM +0530, Praveen Talari wrote:
>> On 07-05-2026 06:32, Mark Brown wrote:
>>> At least these feel like they really should be generic events, there
>>> hopefully isn't anything driver specific about them.
>> Initially implemented as a generic event; however, splitting into separate
>> TX and RX events may be more appropriate.
>> Which approach would you prefer?
> By generic I mean this should not be driver specific at all.
I hope these changes are fine. Please let me know if you have any 
concerns or feedback.

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 4da888359cfc..9abb5f4f719b 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -1,6 +1,8 @@
  // SPDX-License-Identifier: GPL-2.0
  // Copyright (c) 2017-2018, The Linux foundation. All rights reserved.

+#include <trace/events/spi.h>
+
  #define CREATE_TRACE_POINTS
  #include <trace/events/qcom_geni_spi.h>

@@ -709,6 +711,7 @@ static unsigned int geni_byte_per_fifo_word(struct 
spi_geni_master *mas)

  static bool geni_spi_handle_tx(struct spi_geni_master *mas)
  {
+       struct spi_controller *spi = dev_get_drvdata(mas->dev);
         struct geni_se *se = &mas->se;
         unsigned int max_bytes;
         const u8 *tx_buf;
@@ -739,7 +742,7 @@ static bool geni_spi_handle_tx(struct 
spi_geni_master *mas)
                 iowrite32_rep(se->base + SE_GENI_TX_FIFOn, &fifo_word, 1);
         }
         mas->tx_rem_bytes -= max_bytes;
-       trace_geni_spi_tx_data(mas->dev, tx_buf, max_bytes, 
mas->tx_rem_bytes);
+       trace_spi_tx_data(spi->cur_msg->spi, tx_buf, max_bytes, 
mas->tx_rem_bytes);
         if (!mas->tx_rem_bytes) {
                 writel(0, se->base + SE_GENI_TX_WATERMARK_REG);
                 return false;
@@ -749,6 +752,7 @@ static bool geni_spi_handle_tx(struct 
spi_geni_master *mas)

  static void geni_spi_handle_rx(struct spi_geni_master *mas)
  {
+       struct spi_controller *spi = dev_get_drvdata(mas->dev);
         struct geni_se *se = &mas->se;
         u32 rx_fifo_status;
         unsigned int rx_bytes;
@@ -790,7 +794,7 @@ static void geni_spi_handle_rx(struct 
spi_geni_master *mas)
         }
         mas->rx_rem_bytes -= rx_bytes;

-       trace_geni_spi_rx_data(mas->dev, rx_buf, rx_bytes, 
mas->rx_rem_bytes);
+       trace_spi_rx_data(spi->cur_msg->spi, rx_buf, rx_bytes, 
mas->rx_rem_bytes);
  }

  static int setup_se_xfer(struct spi_transfer *xfer,
diff --git a/include/trace/events/spi.h b/include/trace/events/spi.h
index e63d4a24d879..4907625e019d 100644
--- a/include/trace/events/spi.h
+++ b/include/trace/events/spi.h
@@ -233,6 +233,53 @@ DEFINE_EVENT(spi_transfer, spi_transfer_stop,

  );

+DECLARE_EVENT_CLASS(spi_data,
+
+       TP_PROTO(struct spi_device *spi, const u8 *buf, unsigned int len,
+                unsigned int rem),
+
+       TP_ARGS(spi, buf, len, rem),
+
+       TP_STRUCT__entry(
+               __field(        int,            bus_num         )
+               __field(        int,            chip_select     )
+               __field(        unsigned int,   len             )
+               __field(        unsigned int,   rem             )
+               __dynamic_array(u8,     data,           len     )
+       ),
+
+       TP_fast_assign(
+               __entry->bus_num = spi->controller->bus_num;
+               __entry->chip_select = spi_get_chipselect(spi, 0);
+               __entry->len = len;
+               __entry->rem = rem;
+               memcpy(__get_dynamic_array(data), buf, len);
+       ),
+
+       TP_printk("spi%d.%d len=%u rem=%u data=%s",
+                 __entry->bus_num, __entry->chip_select,
+                 __entry->len, __entry->rem,
+                 __print_hex(__get_dynamic_array(data), __entry->len))
+);
+
+DEFINE_EVENT(spi_data, spi_tx_data,
+
+       TP_PROTO(struct spi_device *spi, const u8 *buf, unsigned int len,
+                unsigned int rem),
+
+       TP_ARGS(spi, buf, len, rem)
+
+);
+
+DEFINE_EVENT(spi_data, spi_rx_data,
+
+       TP_PROTO(struct spi_device *spi, const u8 *buf, unsigned int len,
+                unsigned int rem),
+
+       TP_ARGS(spi, buf, len, rem)
+
+);
+
  #endif /* _TRACE_POWER_H */



Thanks,

Praveen Talari

Re: [PATCH v1 1/2] spi: qcom-geni: trace: Add trace events for Qualcomm GENI SPI

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 429 bytes --]

On Thu, May 07, 2026 at 11:03:39PM +0530, Praveen Talari wrote:
> On 07-05-2026 13:43, Mark Brown wrote:

> > By generic I mean this should not be driver specific at all.

> I hope these changes are fine. Please let me know if you have any concerns
> or feedback.

The data tracepoints look plausible but I would expect them to be
generated by the core, they'll be there for everything so I'd expect
them to work for everything.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 1/2] spi: qcom-geni: trace: Add trace events for Qualcomm GENI SPI

[Trilok Soni]

On 5/8/2026 7:01 AM, Mark Brown wrote:
> On Thu, May 07, 2026 at 11:03:39PM +0530, Praveen Talari wrote:
>> On 07-05-2026 13:43, Mark Brown wrote:
> 
>>> By generic I mean this should not be driver specific at all.
> 
>> I hope these changes are fine. Please let me know if you have any concerns
>> or feedback.
> 
> The data tracepoints look plausible but I would expect them to be
> generated by the core, they'll be there for everything so I'd expect
> them to work for everything.

I agree here. Praveen - this is similar to suggestion I had for the i2c
internally. 

---Trilok Soni

Re: [PATCH v1 1/2] spi: qcom-geni: trace: Add trace events for Qualcomm GENI SPI

[Praveen Talari]

On 08-05-2026 19:31, Mark Brown wrote:
> On Thu, May 07, 2026 at 11:03:39PM +0530, Praveen Talari wrote:
>> On 07-05-2026 13:43, Mark Brown wrote:
>>> By generic I mean this should not be driver specific at all.
>> I hope these changes are fine. Please let me know if you have any concerns
>> or feedback.
> The data tracepoints look plausible but I would expect them to be
> generated by the core, they'll be there for everything so I'd expect

Thank you for the clarification. I now understand your point clearly.

Could you also please review the changes made in spi.c ?
I would appreciate any feedback or suggestions you may have.


diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 91dd831d2d3b..f0d3665412fe 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1658,6 +1658,11 @@ static int spi_transfer_one_message(struct 
spi_controller *ctlr,

                 trace_spi_transfer_stop(msg, xfer);

+               if (spi_valid_txbuf(msg, xfer))
+                       trace_spi_tx_data(msg->spi, xfer->tx_buf, 
xfer->len);
+               if (spi_valid_rxbuf(msg, xfer))
+                       trace_spi_rx_data(msg->spi, xfer->rx_buf, 
xfer->len);
+
                 if (msg->status != -EINPROGRESS)

                         goto out;


Thanks,

Praveen Talari

> them to work for everything.

Re: [PATCH v1 1/2] spi: qcom-geni: trace: Add trace events for Qualcomm GENI SPI

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 844 bytes --]

On Sat, May 09, 2026 at 07:37:26AM +0530, Praveen Talari wrote:

> Could you also please review the changes made in spi.c ?
> I would appreciate any feedback or suggestions you may have.

Please just sumbmit normal patches instead of sending partial patches in
reply to another thread unless something is really unclear.

> @@ -1658,6 +1658,11 @@ static int spi_transfer_one_message(struct
> spi_controller *ctlr,
> 
>                 trace_spi_transfer_stop(msg, xfer);
> 
> +               if (spi_valid_txbuf(msg, xfer))
> +                       trace_spi_tx_data(msg->spi, xfer->tx_buf,
> xfer->len);
> +               if (spi_valid_rxbuf(msg, xfer))
> +                       trace_spi_rx_data(msg->spi, xfer->rx_buf,
> xfer->len);

It feels like it'd be more helpful to log the transmit data before we do
the send.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 1/2] spi: qcom-geni: trace: Add trace events for Qualcomm GENI SPI

[Praveen Talari]

HI Mark,

On 10-05-2026 18:07, Mark Brown wrote:
> On Sat, May 09, 2026 at 07:37:26AM +0530, Praveen Talari wrote:
>
>> Could you also please review the changes made in spi.c ?
>> I would appreciate any feedback or suggestions you may have.
> Please just sumbmit normal patches instead of sending partial patches in
> reply to another thread unless something is really unclear.


Sure, I will send the patches next. I just wanted to confirm a few points

>
>> @@ -1658,6 +1658,11 @@ static int spi_transfer_one_message(struct
>> spi_controller *ctlr,
>>
>>                  trace_spi_transfer_stop(msg, xfer);
>>
>> +               if (spi_valid_txbuf(msg, xfer))
>> +                       trace_spi_tx_data(msg->spi, xfer->tx_buf,
>> xfer->len);
>> +               if (spi_valid_rxbuf(msg, xfer))
>> +                       trace_spi_rx_data(msg->spi, xfer->rx_buf,
>> xfer->len);
> It feels like it'd be more helpful to log the transmit data before we do
> the send.

Sure. will take care in next patch-set.


Thanks,

Praveen Talari