2024-04-18 09:23:45

by Takashi Sakamoto

[permalink] [raw]
Subject: [RFC PATCH 04/13] firewire: ohci: replace hard-coded values with inline functions for asynchronous packet header

This commit replaces the hard-coded values with the common inline functions
to serialize and deserialize the header of asynchronous packet.

Signed-off-by: Takashi Sakamoto <[email protected]>
---
drivers/firewire/ohci.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 5254cf5c2e58..4666d941a2ae 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -517,14 +517,14 @@ static const char *tcodes[] = {
static void log_ar_at_event(struct fw_ohci *ohci,
char dir, int speed, u32 *header, int evt)
{
- int tcode = header[0] >> 4 & 0xf;
+ int tcode = async_header_get_tcode(header);
char specific[12];

if (likely(!(param_debug & OHCI_PARAM_DEBUG_AT_AR)))
return;

if (unlikely(evt >= ARRAY_SIZE(evts)))
- evt = 0x1f;
+ evt = 0x1f;

if (evt == OHCI1394_evt_bus_reset) {
ohci_notice(ohci, "A%c evt_bus_reset, generation %d\n",
@@ -539,7 +539,8 @@ static void log_ar_at_event(struct fw_ohci *ohci,
break;
case 0x1: case 0x5: case 0x7: case 0x9: case 0xb:
snprintf(specific, sizeof(specific), " %x,%x",
- header[3] >> 16, header[3] & 0xffff);
+ async_header_get_data_length(header),
+ async_header_get_extended_tcode(header));
break;
default:
specific[0] = '\0';
@@ -556,17 +557,17 @@ static void log_ar_at_event(struct fw_ohci *ohci,
break;
case 0x0: case 0x1: case 0x4: case 0x5: case 0x9:
ohci_notice(ohci,
- "A%c spd %x tl %02x, %04x -> %04x, %s, %s, %04x%08x%s\n",
- dir, speed, header[0] >> 10 & 0x3f,
- header[1] >> 16, header[0] >> 16, evts[evt],
- tcodes[tcode], header[1] & 0xffff, header[2], specific);
+ "A%c spd %x tl %02x, %04x -> %04x, %s, %s, %012llx%s\n",
+ dir, speed, async_header_get_tlabel(header),
+ async_header_get_source(header), async_header_get_destination(header),
+ evts[evt], tcodes[tcode], async_header_get_offset(header), specific);
break;
default:
ohci_notice(ohci,
"A%c spd %x tl %02x, %04x -> %04x, %s, %s%s\n",
- dir, speed, header[0] >> 10 & 0x3f,
- header[1] >> 16, header[0] >> 16, evts[evt],
- tcodes[tcode], specific);
+ dir, speed, async_header_get_tlabel(header),
+ async_header_get_source(header), async_header_get_destination(header),
+ evts[evt], tcodes[tcode], specific);
}
}

@@ -854,7 +855,7 @@ static __le32 *handle_ar_packet(struct ar_context *ctx, __le32 *buffer)
p.header[1] = cond_le32_to_cpu(buffer[1]);
p.header[2] = cond_le32_to_cpu(buffer[2]);

- tcode = (p.header[0] >> 4) & 0x0f;
+ tcode = async_header_get_tcode(p.header);
switch (tcode) {
case TCODE_WRITE_QUADLET_REQUEST:
case TCODE_READ_QUADLET_RESPONSE:
@@ -875,7 +876,7 @@ static __le32 *handle_ar_packet(struct ar_context *ctx, __le32 *buffer)
case TCODE_LOCK_RESPONSE:
p.header[3] = cond_le32_to_cpu(buffer[3]);
p.header_length = 16;
- p.payload_length = p.header[3] >> 16;
+ p.payload_length = async_header_get_data_length(p.header);
if (p.payload_length > MAX_ASYNC_PAYLOAD) {
ar_context_abort(ctx, "invalid packet length");
return NULL;
@@ -912,8 +913,7 @@ static __le32 *handle_ar_packet(struct ar_context *ctx, __le32 *buffer)
* Several controllers, notably from NEC and VIA, forget to
* write ack_complete status at PHY packet reception.
*/
- if (evt == OHCI1394_evt_no_status &&
- (p.header[0] & 0xff) == (OHCI1394_phy_tcode << 4))
+ if (evt == OHCI1394_evt_no_status && async_header_get_tcode(p.header) == OHCI1394_phy_tcode)
p.ack = ACK_COMPLETE;

/*
@@ -1354,7 +1354,7 @@ static int at_context_queue_packet(struct context *ctx,
* accordingly.
*/

- tcode = (packet->header[0] >> 4) & 0x0f;
+ tcode = async_header_get_tcode(packet->header);
header = (__le32 *) &d[1];
switch (tcode) {
case TCODE_WRITE_QUADLET_REQUEST:
--
2.43.0



2024-04-18 09:23:57

by Takashi Sakamoto

[permalink] [raw]
Subject: [RFC PATCH 05/13] firewire: ohci: replace hard-coded values with common macros

In the helper function for logging in 1394 ohci driver includes the
hard-coded variables for transaction code. They can be replaced with
the enumerations in UAPI header.

Signed-off-by: Takashi Sakamoto <[email protected]>
---
drivers/firewire/ohci.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 4666d941a2ae..85223a1c90a1 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -533,11 +533,17 @@ static void log_ar_at_event(struct fw_ohci *ohci,
}

switch (tcode) {
- case 0x0: case 0x6: case 0x8:
+ case TCODE_WRITE_QUADLET_REQUEST:
+ case TCODE_READ_QUADLET_RESPONSE:
+ case TCODE_CYCLE_START:
snprintf(specific, sizeof(specific), " = %08x",
be32_to_cpu((__force __be32)header[3]));
break;
- case 0x1: case 0x5: case 0x7: case 0x9: case 0xb:
+ case TCODE_WRITE_BLOCK_REQUEST:
+ case TCODE_READ_BLOCK_REQUEST:
+ case TCODE_READ_BLOCK_RESPONSE:
+ case TCODE_LOCK_REQUEST:
+ case TCODE_LOCK_RESPONSE:
snprintf(specific, sizeof(specific), " %x,%x",
async_header_get_data_length(header),
async_header_get_extended_tcode(header));
@@ -547,7 +553,7 @@ static void log_ar_at_event(struct fw_ohci *ohci,
}

switch (tcode) {
- case 0xa:
+ case TCODE_STREAM_DATA:
ohci_notice(ohci, "A%c %s, %s\n",
dir, evts[evt], tcodes[tcode]);
break;
@@ -555,7 +561,11 @@ static void log_ar_at_event(struct fw_ohci *ohci,
ohci_notice(ohci, "A%c %s, PHY %08x %08x\n",
dir, evts[evt], header[1], header[2]);
break;
- case 0x0: case 0x1: case 0x4: case 0x5: case 0x9:
+ case TCODE_WRITE_QUADLET_REQUEST:
+ case TCODE_WRITE_BLOCK_REQUEST:
+ case TCODE_READ_QUADLET_REQUEST:
+ case TCODE_READ_BLOCK_REQUEST:
+ case TCODE_LOCK_REQUEST:
ohci_notice(ohci,
"A%c spd %x tl %02x, %04x -> %04x, %s, %s, %012llx%s\n",
dir, speed, async_header_get_tlabel(header),
--
2.43.0


2024-04-18 09:24:18

by Takashi Sakamoto

[permalink] [raw]
Subject: [RFC PATCH 06/13] firewire: core: obsolete tcode check macros with inline functions

This commit declares the helper functions to check tcode to obsolete
the functional macros.

Signed-off-by: Takashi Sakamoto <[email protected]>
---
drivers/firewire/core-transaction.c | 4 ++--
drivers/firewire/core.h | 21 ++++++++++++++-------
drivers/firewire/ohci.c | 6 +++---
3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index 01ce07f87452..24febc23c0c4 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -972,7 +972,7 @@ void fw_core_handle_request(struct fw_card *card, struct fw_packet *p)
if (p->ack != ACK_PENDING && p->ack != ACK_COMPLETE)
return;

- if (TCODE_IS_LINK_INTERNAL(async_header_get_tcode(p->header))) {
+ if (tcode_is_link_internal(async_header_get_tcode(p->header))) {
fw_cdev_handle_phy_packet(card, p);
return;
}
@@ -1109,7 +1109,7 @@ static void handle_topology_map(struct fw_card *card, struct fw_request *request
{
int start;

- if (!TCODE_IS_READ_REQUEST(tcode)) {
+ if (!tcode_is_read_request(tcode)) {
fw_send_response(card, request, RCODE_TYPE_ERROR);
return;
}
diff --git a/drivers/firewire/core.h b/drivers/firewire/core.h
index 95c10f3d2282..7c36d2628e37 100644
--- a/drivers/firewire/core.h
+++ b/drivers/firewire/core.h
@@ -225,13 +225,20 @@ static inline bool is_next_generation(int new_generation, int old_generation)

#define TCODE_LINK_INTERNAL 0xe

-#define TCODE_IS_READ_REQUEST(tcode) (((tcode) & ~1) == 4)
-#define TCODE_IS_BLOCK_PACKET(tcode) (((tcode) & 1) != 0)
-#define TCODE_IS_LINK_INTERNAL(tcode) ((tcode) == TCODE_LINK_INTERNAL)
-#define TCODE_IS_REQUEST(tcode) (((tcode) & 2) == 0)
-#define TCODE_IS_RESPONSE(tcode) (((tcode) & 2) != 0)
-#define TCODE_HAS_REQUEST_DATA(tcode) (((tcode) & 12) != 4)
-#define TCODE_HAS_RESPONSE_DATA(tcode) (((tcode) & 12) != 0)
+static inline bool tcode_is_read_request(unsigned int tcode)
+{
+ return (tcode & ~1u) == 4u;
+}
+
+static inline bool tcode_is_block_packet(unsigned int tcode)
+{
+ return (tcode & 1u) != 0u;
+}
+
+static inline bool tcode_is_link_internal(unsigned int tcode)
+{
+ return (tcode == TCODE_LINK_INTERNAL);
+}

#define LOCAL_BUS 0xffc0

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 85223a1c90a1..77f098fb9484 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -1382,7 +1382,7 @@ static int at_context_queue_packet(struct context *ctx,
(packet->header[0] & 0xffff0000));
header[2] = cpu_to_le32(packet->header[2]);

- if (TCODE_IS_BLOCK_PACKET(tcode))
+ if (tcode_is_block_packet(tcode))
header[3] = cpu_to_le32(packet->header[3]);
else
header[3] = (__force __le32) packet->header[3];
@@ -1568,7 +1568,7 @@ static void handle_local_rom(struct fw_ohci *ohci,
int tcode, length, i;

tcode = async_header_get_tcode(packet->header);
- if (TCODE_IS_BLOCK_PACKET(tcode))
+ if (tcode_is_block_packet(tcode))
length = async_header_get_data_length(packet->header);
else
length = 4;
@@ -1577,7 +1577,7 @@ static void handle_local_rom(struct fw_ohci *ohci,
if (i + length > CONFIG_ROM_SIZE) {
fw_fill_response(&response, packet->header,
RCODE_ADDRESS_ERROR, NULL, 0);
- } else if (!TCODE_IS_READ_REQUEST(tcode)) {
+ } else if (!tcode_is_read_request(tcode)) {
fw_fill_response(&response, packet->header,
RCODE_TYPE_ERROR, NULL, 0);
} else {
--
2.43.0


2024-04-18 09:24:33

by Takashi Sakamoto

[permalink] [raw]
Subject: [RFC PATCH 07/13] firewire: core: add common macro to serialize/deserialize isochronous packet header

The packet for Asynchronous Streaming Packet includes the same header
fields as the isochronous packet has. It is helpful to have some helper
functions to serialize/deserialize them.

This commit adds such helper functions with their test.

Signed-off-by: Takashi Sakamoto <[email protected]>
---
drivers/firewire/packet-header-definitions.h | 66 ++++++++++++++++++++
drivers/firewire/packet-serdes-test.c | 44 +++++++++++++
2 files changed, 110 insertions(+)

diff --git a/drivers/firewire/packet-header-definitions.h b/drivers/firewire/packet-header-definitions.h
index 83e550427706..ab9d0fa790d4 100644
--- a/drivers/firewire/packet-header-definitions.h
+++ b/drivers/firewire/packet-header-definitions.h
@@ -165,4 +165,70 @@ static inline void async_header_set_extended_tcode(u32 header[ASYNC_HEADER_QUADL
header[3] |= (((u32)extended_tcode) << ASYNC_HEADER_Q3_EXTENDED_TCODE_SHIFT) & ASYNC_HEADER_Q3_EXTENDED_TCODE_MASK;
}

+#define ISOC_HEADER_DATA_LENGTH_SHIFT 16
+#define ISOC_HEADER_DATA_LENGTH_MASK 0xffff0000
+#define ISOC_HEADER_TAG_SHIFT 14
+#define ISOC_HEADER_TAG_MASK 0x0000c000
+#define ISOC_HEADER_CHANNEL_SHIFT 8
+#define ISOC_HEADER_CHANNEL_MASK 0x00003f00
+#define ISOC_HEADER_TCODE_SHIFT 4
+#define ISOC_HEADER_TCODE_MASK 0x000000f0
+#define ISOC_HEADER_SY_SHIFT 0
+#define ISOC_HEADER_SY_MASK 0x0000000f
+
+static inline unsigned int isoc_header_get_data_length(u32 header)
+{
+ return (header & ISOC_HEADER_DATA_LENGTH_MASK) >> ISOC_HEADER_DATA_LENGTH_SHIFT;
+}
+
+static inline unsigned int isoc_header_get_tag(u32 header)
+{
+ return (header & ISOC_HEADER_TAG_MASK) >> ISOC_HEADER_TAG_SHIFT;
+}
+
+static inline unsigned int isoc_header_get_channel(u32 header)
+{
+ return (header & ISOC_HEADER_CHANNEL_MASK) >> ISOC_HEADER_CHANNEL_SHIFT;
+}
+
+static inline unsigned int isoc_header_get_tcode(u32 header)
+{
+ return (header & ISOC_HEADER_TCODE_MASK) >> ISOC_HEADER_TCODE_SHIFT;
+}
+
+static inline unsigned int isoc_header_get_sy(u32 header)
+{
+ return (header & ISOC_HEADER_SY_MASK) >> ISOC_HEADER_SY_SHIFT;
+}
+
+static inline void isoc_header_set_data_length(u32 *header, unsigned int data_length)
+{
+ *header &= ~ISOC_HEADER_DATA_LENGTH_MASK;
+ *header |= (((u32)data_length) << ISOC_HEADER_DATA_LENGTH_SHIFT) & ISOC_HEADER_DATA_LENGTH_MASK;
+}
+
+static inline void isoc_header_set_tag(u32 *header, unsigned int tag)
+{
+ *header &= ~ISOC_HEADER_TAG_MASK;
+ *header |= (((u32)tag) << ISOC_HEADER_TAG_SHIFT) & ISOC_HEADER_TAG_MASK;
+}
+
+static inline void isoc_header_set_channel(u32 *header, unsigned int channel)
+{
+ *header &= ~ISOC_HEADER_CHANNEL_MASK;
+ *header |= (((u32)channel) << ISOC_HEADER_CHANNEL_SHIFT) & ISOC_HEADER_CHANNEL_MASK;
+}
+
+static inline void isoc_header_set_tcode(u32 *header, unsigned int tcode)
+{
+ *header &= ~ISOC_HEADER_TCODE_MASK;
+ *header |= (((u32)tcode) << ISOC_HEADER_TCODE_SHIFT) & ISOC_HEADER_TCODE_MASK;
+}
+
+static inline void isoc_header_set_sy(u32 *header, unsigned int sy)
+{
+ *header &= ~ISOC_HEADER_SY_MASK;
+ *header |= (((u32)sy) << ISOC_HEADER_SY_SHIFT) & ISOC_HEADER_SY_MASK;
+}
+
#endif // _FIREWIRE_PACKET_HEADER_DEFINITIONS_H
diff --git a/drivers/firewire/packet-serdes-test.c b/drivers/firewire/packet-serdes-test.c
index 299e9f908463..f93c966e794d 100644
--- a/drivers/firewire/packet-serdes-test.c
+++ b/drivers/firewire/packet-serdes-test.c
@@ -167,6 +167,26 @@ static void deserialize_async_header_block_response(const u32 header[ASYNC_HEADE
*extended_tcode = async_header_get_extended_tcode(header);
}

+static void serialize_isoc_header(u32 *header, unsigned int data_length, unsigned int tag,
+ unsigned int channel, unsigned int tcode, unsigned int sy)
+{
+ isoc_header_set_data_length(header, data_length);
+ isoc_header_set_tag(header, tag);
+ isoc_header_set_channel(header, channel);
+ isoc_header_set_tcode(header, tcode);
+ isoc_header_set_sy(header, sy);
+}
+
+static void deserialize_isoc_header(u32 header, unsigned int *data_length, unsigned int *tag,
+ unsigned int *channel, unsigned int *tcode, unsigned int *sy)
+{
+ *data_length = isoc_header_get_data_length(header);
+ *tag = isoc_header_get_tag(header);
+ *channel = isoc_header_get_channel(header);
+ *tcode = isoc_header_get_tcode(header);
+ *sy = isoc_header_get_sy(header);
+}
+
static void test_async_header_write_quadlet_request(struct kunit *test)
{
static const u32 expected[ASYNC_HEADER_QUADLET_COUNT] = {
@@ -515,6 +535,29 @@ static void test_async_header_lock_response(struct kunit *test)
KUNIT_EXPECT_MEMEQ(test, header, expected, sizeof(expected));
}

+static void test_isoc_header(struct kunit *test)
+{
+ const u32 expected = 0x00d08dec;
+ u32 header = 0;
+
+ unsigned int data_length;
+ unsigned int tag;
+ unsigned int channel;
+ unsigned int tcode;
+ unsigned int sy;
+
+ deserialize_isoc_header(expected, &data_length, &tag, &channel, &tcode, &sy);
+
+ KUNIT_EXPECT_EQ(test, 0xd0, data_length);
+ KUNIT_EXPECT_EQ(test, 0x02, tag);
+ KUNIT_EXPECT_EQ(test, 0x0d, channel);
+ KUNIT_EXPECT_EQ(test, 0x0e, tcode);
+ KUNIT_EXPECT_EQ(test, 0x0c, sy);
+
+ serialize_isoc_header(&header, data_length, tag, channel, tcode, sy);
+
+ KUNIT_EXPECT_EQ(test, header, expected);
+}

static struct kunit_case packet_serdes_test_cases[] = {
KUNIT_CASE(test_async_header_write_quadlet_request),
@@ -526,6 +569,7 @@ static struct kunit_case packet_serdes_test_cases[] = {
KUNIT_CASE(test_async_header_read_block_response),
KUNIT_CASE(test_async_header_lock_request),
KUNIT_CASE(test_async_header_lock_response),
+ KUNIT_CASE(test_isoc_header),
{}
};

--
2.43.0


2024-04-18 09:24:42

by Takashi Sakamoto

[permalink] [raw]
Subject: [RFC PATCH 08/13] firewire: core: replace local macros with common inline functions for isochronous packet header

This commit replaces the local macros with the common inline functions
to serialize the packer header for Asynchronous Streaming Packet.

Signed-off-by: Takashi Sakamoto <[email protected]>
---
drivers/firewire/core-transaction.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index 24febc23c0c4..52d8d483c178 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -31,9 +31,6 @@
#include "core.h"
#include "packet-header-definitions.h"

-#define HEADER_TCODE(tcode) ((tcode) << 4)
-#define HEADER_DATA_LENGTH(length) ((length) << 16)
-
#define HEADER_DESTINATION_IS_BROADCAST(header) \
((async_header_get_destination(header) & 0x3f) == 0x3f)

@@ -215,10 +212,11 @@ static void fw_fill_request(struct fw_packet *packet, int tcode, int tlabel,
int ext_tcode;

if (tcode == TCODE_STREAM_DATA) {
- packet->header[0] =
- HEADER_DATA_LENGTH(length) |
- destination_id |
- HEADER_TCODE(TCODE_STREAM_DATA);
+ // The value of destination_id argument should include tag, channel, and sy fields
+ // as isochronous packet header has.
+ packet->header[0] = destination_id;
+ isoc_header_set_data_length(packet->header, length);
+ isoc_header_set_tcode(packet->header, TCODE_STREAM_DATA);
packet->header_length = 4;
packet->payload = payload;
packet->payload_length = length;
--
2.43.0


2024-04-18 09:24:57

by Takashi Sakamoto

[permalink] [raw]
Subject: [RFC PATCH 09/13] firewire: core: add support for Linux kernel tracepoints

The Linux Kernel Tracepoints framework is enough useful to trace
packet data inbound to and outbound from core.

This commit adds firewire subsystem to use the framework.

Signed-off-by: Takashi Sakamoto <[email protected]>
---
drivers/firewire/Makefile | 5 ++++-
drivers/firewire/trace.c | 5 +++++
drivers/firewire/trace.h | 17 +++++++++++++++++
3 files changed, 26 insertions(+), 1 deletion(-)
create mode 100644 drivers/firewire/trace.c
create mode 100644 drivers/firewire/trace.h

diff --git a/drivers/firewire/Makefile b/drivers/firewire/Makefile
index bbde29a0fba6..92376e045805 100644
--- a/drivers/firewire/Makefile
+++ b/drivers/firewire/Makefile
@@ -3,12 +3,15 @@
# Makefile for the Linux IEEE 1394 implementation
#

-firewire-core-y += core-card.o core-cdev.o core-device.o \
+firewire-core-y += trace.o core-card.o core-cdev.o core-device.o \
core-iso.o core-topology.o core-transaction.o
firewire-ohci-y += ohci.o
firewire-sbp2-y += sbp2.o
firewire-net-y += net.o

+# Let "include/trace/define_trace.h" find the header.
+CFLAGS_trace.o := -I$(src)
+
obj-$(CONFIG_FIREWIRE) += firewire-core.o
obj-$(CONFIG_FIREWIRE_OHCI) += firewire-ohci.o
obj-$(CONFIG_FIREWIRE_SBP2) += firewire-sbp2.o
diff --git a/drivers/firewire/trace.c b/drivers/firewire/trace.c
new file mode 100644
index 000000000000..ffe427764a90
--- /dev/null
+++ b/drivers/firewire/trace.c
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright (c) 2024 Takashi Sakamoto
+
+#define CREATE_TRACE_POINTS
+#include "trace.h"
diff --git a/drivers/firewire/trace.h b/drivers/firewire/trace.h
new file mode 100644
index 000000000000..d36a10460301
--- /dev/null
+++ b/drivers/firewire/trace.h
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright (c) 2024 Takashi Sakamoto
+
+#define TRACE_SYSTEM firewire
+
+#if !defined(_FIREWIRE_TRACE_EVENT_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _FIREWIRE_TRACE_EVENT_H
+
+#include <linux/tracepoint.h>
+
+// Placeholder for future use.
+
+#endif // _FIREWIRE_TRACE_EVENT_H
+
+#define TRACE_INCLUDE_PATH .
+#define TRACE_INCLUDE_FILE trace
+#include <trace/define_trace.h>
--
2.43.0


2024-04-18 09:25:14

by Takashi Sakamoto

[permalink] [raw]
Subject: [RFC PATCH 10/13] firewire: core: add tracepoints events for asynchronous outbound request

In a view of core transaction service, the asynchronous outbound request
consists of two stages; initiation and completion.

This commit adds a pair of event for them.

Signed-off-by: Takashi Sakamoto <[email protected]>
---
drivers/firewire/core-transaction.c | 6 +++
drivers/firewire/trace.h | 80 ++++++++++++++++++++++++++++-
2 files changed, 85 insertions(+), 1 deletion(-)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index 52d8d483c178..11a60094182a 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -29,6 +29,7 @@
#include <asm/byteorder.h>

#include "core.h"
+#include "trace.h"
#include "packet-header-definitions.h"

#define HEADER_DESTINATION_IS_BROADCAST(header) \
@@ -173,6 +174,8 @@ static void transmit_complete_callback(struct fw_packet *packet,
struct fw_transaction *t =
container_of(packet, struct fw_transaction, packet);

+ trace_async_request_outbound_complete(card, t, packet);
+
switch (status) {
case ACK_COMPLETE:
close_transaction(t, card, RCODE_COMPLETE, packet->timestamp);
@@ -394,6 +397,9 @@ void __fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode

spin_unlock_irqrestore(&card->lock, flags);

+ trace_async_request_outbound_initiate(card, t, &t->packet, payload,
+ tcode_is_read_request(tcode) ? 0 : length / 4);
+
card->driver->send_request(card, &t->packet);
}
EXPORT_SYMBOL_GPL(__fw_send_request);
diff --git a/drivers/firewire/trace.h b/drivers/firewire/trace.h
index d36a10460301..0f7d176ba647 100644
--- a/drivers/firewire/trace.h
+++ b/drivers/firewire/trace.h
@@ -7,8 +7,86 @@
#define _FIREWIRE_TRACE_EVENT_H

#include <linux/tracepoint.h>
+#include <linux/firewire.h>

-// Placeholder for future use.
+#include <linux/firewire-constants.h>
+
+#include "packet-header-definitions.h"
+
+TRACE_EVENT(async_request_outbound_initiate,
+ TP_PROTO(const struct fw_card *card, const struct fw_transaction *transaction,
+ const struct fw_packet *packet, const u32 *data, size_t data_count),
+ TP_ARGS(card, transaction, packet, data, data_count),
+ TP_STRUCT__entry(
+ __field(u64, transaction)
+ __field(u8, scode)
+ __field(u8, generation)
+ __field(u16, destination)
+ __field(u8, tlabel)
+ __field(u8, retry)
+ __field(u8, tcode)
+ __field(u8, priority)
+ __field(u16, source)
+ __field(u64, offset)
+ __dynamic_array(u32, data, data_count)
+ ),
+ TP_fast_assign(
+ __entry->transaction = (u64)transaction;
+ __entry->scode = packet->speed;
+ __entry->generation = packet->generation;
+ __entry->destination = async_header_get_destination(packet->header);
+ __entry->tlabel = async_header_get_tlabel(packet->header);
+ __entry->retry = async_header_get_retry(packet->header);
+ __entry->tcode = async_header_get_tcode(packet->header);
+ __entry->priority = async_header_get_priority(packet->header);
+ __entry->source = async_header_get_source(packet->header);
+ __entry->offset = async_header_get_offset(packet->header);
+ memcpy(__get_dynamic_array(data), data, __get_dynamic_array_len(data));
+ ),
+ TP_printk(
+ "transaction=0x%llx scode=%u generation=%u dst_id=0x%04x tlabel=%u retry=%u tcode=%u priority=%u src_id=0x%04x offset=0x%012llx data=%s",
+ __entry->transaction,
+ __entry->scode,
+ __entry->generation,
+ __entry->destination,
+ __entry->tlabel,
+ __entry->retry,
+ __entry->tcode,
+ __entry->priority,
+ __entry->source,
+ __entry->offset,
+ __print_array(__get_dynamic_array(data),
+ __get_dynamic_array_len(data) / sizeof(u32), sizeof(u32))
+ )
+)
+
+TRACE_EVENT(async_request_outbound_complete,
+ TP_PROTO(const struct fw_card *card, const struct fw_transaction *transaction,
+ const struct fw_packet *packet),
+ TP_ARGS(card, transaction, packet),
+ TP_STRUCT__entry(
+ __field(u64, transaction)
+ __field(u8, scode)
+ __field(u8, generation)
+ __field(u8, ack)
+ __field(u16, timestamp)
+ ),
+ TP_fast_assign(
+ __entry->transaction = (u64)transaction;
+ __entry->scode = packet->speed;
+ __entry->generation = packet->generation;
+ __entry->ack = packet->ack;
+ __entry->timestamp = packet->timestamp;
+ ),
+ TP_printk(
+ "transaction=0x%llx scode=%u generation=%u ack=%u timestamp=0x%04x",
+ __entry->transaction,
+ __entry->scode,
+ __entry->generation,
+ __entry->ack,
+ __entry->timestamp
+ )
+)

#endif // _FIREWIRE_TRACE_EVENT_H

--
2.43.0


2024-04-18 09:25:27

by Takashi Sakamoto

[permalink] [raw]
Subject: [RFC PATCH 11/13] firewire: core: add tracepoints event for asynchronous inbound response

In the transaction of IEEE 1394, the node to receive the asynchronous
request transfers response packet to the requester.

This commit adds an event for the incoming packet. Note that the code to
decode the packet header is moved, against the note about the sanity
check.

Signed-off-by: Takashi Sakamoto <[email protected]>
---
drivers/firewire/core-transaction.c | 54 +++++++++++++++--------------
drivers/firewire/trace.h | 48 +++++++++++++++++++++++++
2 files changed, 76 insertions(+), 26 deletions(-)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index 11a60094182a..977d8a36f969 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -1010,32 +1010,10 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p)
source = async_header_get_source(p->header);
rcode = async_header_get_rcode(p->header);

- spin_lock_irqsave(&card->lock, flags);
- list_for_each_entry(iter, &card->transaction_list, link) {
- if (iter->node_id == source && iter->tlabel == tlabel) {
- if (!try_cancel_split_timeout(iter)) {
- spin_unlock_irqrestore(&card->lock, flags);
- goto timed_out;
- }
- list_del_init(&iter->link);
- card->tlabel_mask &= ~(1ULL << iter->tlabel);
- t = iter;
- break;
- }
- }
- spin_unlock_irqrestore(&card->lock, flags);
-
- if (!t) {
- timed_out:
- fw_notice(card, "unsolicited response (source %x, tlabel %x)\n",
- source, tlabel);
- return;
- }
-
- /*
- * FIXME: sanity check packet, is length correct, does tcodes
- * and addresses match.
- */
+ // FIXME: sanity check packet, is length correct, does tcodes
+ // and addresses match to the transaction request queried later.
+ //
+ // For the tracepoints event, let us decode the header here against the concern.

switch (tcode) {
case TCODE_READ_QUADLET_RESPONSE:
@@ -1061,6 +1039,30 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p)
break;
}

+ spin_lock_irqsave(&card->lock, flags);
+ list_for_each_entry(iter, &card->transaction_list, link) {
+ if (iter->node_id == source && iter->tlabel == tlabel) {
+ if (!try_cancel_split_timeout(iter)) {
+ spin_unlock_irqrestore(&card->lock, flags);
+ goto timed_out;
+ }
+ list_del_init(&iter->link);
+ card->tlabel_mask &= ~(1ULL << iter->tlabel);
+ t = iter;
+ break;
+ }
+ }
+ spin_unlock_irqrestore(&card->lock, flags);
+
+ trace_async_response_inbound(card, t, p, data, data_length / 4);
+
+ if (!t) {
+ timed_out:
+ fw_notice(card, "unsolicited response (source %x, tlabel %x)\n",
+ source, tlabel);
+ return;
+ }
+
/*
* The response handler may be executed while the request handler
* is still pending. Cancel the request handler.
diff --git a/drivers/firewire/trace.h b/drivers/firewire/trace.h
index 0f7d176ba647..5187f5f2b140 100644
--- a/drivers/firewire/trace.h
+++ b/drivers/firewire/trace.h
@@ -88,6 +88,54 @@ TRACE_EVENT(async_request_outbound_complete,
)
)

+TRACE_EVENT(async_response_inbound,
+ TP_PROTO(const struct fw_card *card, const struct fw_transaction *transaction,
+ const struct fw_packet *packet, u32 *data, unsigned int data_count),
+ TP_ARGS(card, transaction, packet, data, data_count),
+ TP_STRUCT__entry(
+ __field(u64, transaction)
+ __field(u8, scode)
+ __field(u8, generation)
+ __field(u16, timestamp)
+ __field(u16, destination)
+ __field(u8, tlabel)
+ __field(u8, retry)
+ __field(u8, tcode)
+ __field(u8, priority)
+ __field(u16, source)
+ __field(u8, rcode)
+ __dynamic_array(u32, data, data_count)
+ ),
+ TP_fast_assign(
+ __entry->transaction = (u64)transaction;
+ __entry->scode = packet->speed;
+ __entry->timestamp = packet->timestamp;
+ __entry->destination = async_header_get_destination(packet->header);
+ __entry->tlabel = async_header_get_tlabel(packet->header);
+ __entry->retry = async_header_get_retry(packet->header);
+ __entry->tcode = async_header_get_tcode(packet->header);
+ __entry->priority = async_header_get_priority(packet->header);
+ __entry->source = async_header_get_source(packet->header);
+ __entry->rcode = async_header_get_rcode(packet->header);
+ memcpy(__get_dynamic_array(data), data, __get_dynamic_array_len(data));
+ ),
+ TP_printk(
+ "transaction=0x%llx scode=%u timestamp=0x%04x dst_id=0x%04x tlabel=%u retry=%u tcode=%u priority=%u src_id=0x%04x rcode=%u data=%s",
+ __entry->transaction,
+ __entry->scode,
+ __entry->timestamp,
+ __entry->destination,
+ __entry->tlabel,
+ __entry->retry,
+ __entry->tcode,
+ __entry->priority,
+ __entry->source,
+ __entry->rcode,
+ __print_array(__get_dynamic_array(data),
+ __get_dynamic_array_len(data) / sizeof(u32), sizeof(u32))
+ )
+)
+
#endif // _FIREWIRE_TRACE_EVENT_H

#define TRACE_INCLUDE_PATH .
--
2.43.0


2024-04-18 09:25:54

by Takashi Sakamoto

[permalink] [raw]
Subject: [RFC PATCH 12/13] firewire: core: add tracepoint event for asynchronous inbound request

This commit adds an event for asynchronous inbound request.

Signed-off-by: Takashi Sakamoto <[email protected]>
---
drivers/firewire/core-transaction.c | 8 ++++-
drivers/firewire/trace.h | 54 +++++++++++++++++++++++++++++
2 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index 977d8a36f969..1b972e95fe36 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -972,11 +972,13 @@ void fw_core_handle_request(struct fw_card *card, struct fw_packet *p)
{
struct fw_request *request;
unsigned long long offset;
+ unsigned int tcode;

if (p->ack != ACK_PENDING && p->ack != ACK_COMPLETE)
return;

- if (tcode_is_link_internal(async_header_get_tcode(p->header))) {
+ tcode = async_header_get_tcode(p->header);
+ if (tcode_is_link_internal(tcode)) {
fw_cdev_handle_phy_packet(card, p);
return;
}
@@ -987,6 +989,10 @@ void fw_core_handle_request(struct fw_card *card, struct fw_packet *p)
return;
}

+ trace_async_request_inbound(card, request, p->ack, p->speed, p->timestamp, p->generation,
+ p->header, request->data,
+ tcode_is_read_request(tcode) ? 0 : request->length / 4);
+
offset = async_header_get_offset(p->header);

if (!is_in_fcp_region(offset, request->length))
diff --git a/drivers/firewire/trace.h b/drivers/firewire/trace.h
index 5187f5f2b140..ba09eb720933 100644
--- a/drivers/firewire/trace.h
+++ b/drivers/firewire/trace.h
@@ -136,6 +136,60 @@ TRACE_EVENT(async_response_inbound,
)
)

+TRACE_EVENT(async_request_inbound,
+ TP_PROTO(const struct fw_card *card, const struct fw_request *request, unsigned int ack,
+ unsigned int scode, unsigned int timestamp, unsigned int generation,
+ const u32 *header, const u32 *data, unsigned int data_count),
+ TP_ARGS(card, request, ack, scode, timestamp, generation, header, data, data_count),
+ TP_STRUCT__entry(
+ __field(u64, transaction)
+ __field(u8, scode)
+ __field(u8, ack)
+ __field(u8, generation)
+ __field(u16, timestamp)
+ __field(u16, destination)
+ __field(u8, tlabel)
+ __field(u8, retry)
+ __field(u8, tcode)
+ __field(u8, priority)
+ __field(u16, source)
+ __field(u64, offset)
+ __dynamic_array(u32, data, data_count)
+ ),
+ TP_fast_assign(
+ __entry->transaction = (u64)request;
+ __entry->scode = scode;
+ __entry->ack = ack;
+ __entry->generation = generation;
+ __entry->timestamp = timestamp;
+ __entry->destination = async_header_get_destination(header);
+ __entry->tlabel = async_header_get_tlabel(header);
+ __entry->retry = async_header_get_retry(header);
+ __entry->tcode = async_header_get_tcode(header);
+ __entry->priority = async_header_get_priority(header);
+ __entry->source = async_header_get_source(header);
+ __entry->offset = async_header_get_offset(header);
+ memcpy(__get_dynamic_array(data), data, __get_dynamic_array_len(data));
+ ),
+ TP_printk(
+ "transaction=0x%llx scode=%u ack=%u generation=%u timestamp=0x%04x dst_id=0x%04x tlabel=%u retry=%u tcode=%u priority=%u src_id=0x%04x offset=0x%012llx data=%s",
+ __entry->transaction,
+ __entry->scode,
+ __entry->ack,
+ __entry->generation,
+ __entry->timestamp,
+ __entry->destination,
+ __entry->tlabel,
+ __entry->retry,
+ __entry->tcode,
+ __entry->priority,
+ __entry->source,
+ __entry->offset,
+ __print_array(__get_dynamic_array(data),
+ __get_dynamic_array_len(data) / sizeof(u32), sizeof(u32))
+ )
+)
+
#endif // _FIREWIRE_TRACE_EVENT_H

#define TRACE_INCLUDE_PATH .
--
2.43.0