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

[Praveen Talari <praveen.talari@oss.qualcomm.com>]

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