All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 00/13] firewire: add tracepoints events for asynchronous communication
@ 2024-04-18  9:22 Takashi Sakamoto
  2024-04-18  9:22 ` [RFC PATCH 01/13] firewire: core: add common inline functions to serialize/deserialize asynchronous packet header Takashi Sakamoto
  0 siblings, 1 reply; 14+ messages in thread
From: Takashi Sakamoto @ 2024-04-18  9:22 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

Hi,

In a view of IEEE 1394 bus, the main function of kernel core is to
provide transaction service to the bus. It is helpful to have some
mechanisms to trace any action of the service.

This series of changes adds some tracepoints events for the purpose.
It adds the following tracepoints events via firewire subsystem:

* For outbound transactions (e.g. initiated by user process)
    * async_request_outbound_initiate
    * async_request_outbound_complete
    * async_response_inbound
* For inbound transactions (e.g. initiated by the other nodes in the bus)
    * async_request_inbound
    * async_response_outbound_initiate
    * async_response_outbound_complete

When probing these tracepoints events, the content of 'struct fw_packet'
passed between the core function and 1394 OHCI driver is recorded with
the fields of header and packet data. For example of the outbound
transaction:

async_request_outbound_initiate: \
    transaction=0xffffb7e382373718 scode=2 generation=6 dst_id=0xffc0 \
    tlabel=59 retry=1 tcode=1 priority=0 src_id=0xffc1 \
    offset=0xecc000000000 \
    data={0x6000000,0x1000000,0x40000100,0x3000000,0x1000000,0x0}
async_request_outbound_complete: \
    transaction=0xffffb7e382373718 scode=2 generation=6 ack=2 \
    timestamp=0x2296
async_response_inbound: \
    transaction=0xffffb7e382373718 scode=2 timestamp=0x2297 dst_id=0xffc1 \
    tlabel=59 retry=1 tcode=2 priority=0 src_id=0xffc0 rcode=0 data={}

To provide the parsed fields of header, the series adds some helper
incline functions for this purpose, then refactors the existent code in
both core and 1394 OHCI driver with sufficient tests.

Takashi Sakamoto (13):
  firewire: core: add common inline functions to serialize/deserialize
    asynchronous packet header
  firewire: core: replace local macros with common inline functions for
    asynchronous packet header
  firewire: ohci: replace local macros with common inline functions for
    asynchronous packet header
  firewire: ohci: replace hard-coded values with inline functions for
    asynchronous packet header
  firewire: ohci: replace hard-coded values with common macros
  firewire: core: obsolete tcode check macros with inline functions
  firewire: core: add common macro to serialize/deserialize isochronous
    packet header
  firewire: core: replace local macros with common inline functions for
    isochronous packet header
  firewire: core: add support for Linux kernel tracepoints
  firewire: core: add tracepoints events for asynchronous outbound
    request
  firewire: core: add tracepoints event for asynchronous inbound
    response
  firewire: core: add tracepoint event for asynchronous inbound request
  firewire: core: add tracepoints events for asynchronous outbound
    response

 drivers/firewire/.kunitconfig                |   1 +
 drivers/firewire/Kconfig                     |  16 +
 drivers/firewire/Makefile                    |   8 +-
 drivers/firewire/core-transaction.c          | 239 ++++----
 drivers/firewire/core.h                      |  21 +-
 drivers/firewire/ohci.c                      |  78 +--
 drivers/firewire/packet-header-definitions.h | 234 ++++++++
 drivers/firewire/packet-serdes-test.c        | 582 +++++++++++++++++++
 drivers/firewire/trace.c                     |   5 +
 drivers/firewire/trace.h                     | 265 +++++++++
 10 files changed, 1280 insertions(+), 169 deletions(-)
 create mode 100644 drivers/firewire/packet-header-definitions.h
 create mode 100644 drivers/firewire/packet-serdes-test.c
 create mode 100644 drivers/firewire/trace.c
 create mode 100644 drivers/firewire/trace.h

-- 
2.43.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 01/13] firewire: core: add common inline functions to serialize/deserialize asynchronous packet header
  2024-04-18  9:22 [RFC PATCH 00/13] firewire: add tracepoints events for asynchronous communication Takashi Sakamoto
@ 2024-04-18  9:22 ` Takashi Sakamoto
  2024-04-18  9:22   ` [RFC PATCH 02/13] firewire: core: replace local macros with common inline functions for " Takashi Sakamoto
  0 siblings, 1 reply; 14+ messages in thread
From: Takashi Sakamoto @ 2024-04-18  9:22 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

In both core and 1394 OHCI driver, some hard-coded values and macros are
used to serialize/deserialize the header for asynchronous packets. It is
inconvenient to reuse them.

This commit adds some helper inline functions with their tests for the
purpose.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/.kunitconfig                |   1 +
 drivers/firewire/Kconfig                     |  16 +
 drivers/firewire/Makefile                    |   3 +
 drivers/firewire/packet-header-definitions.h | 168 ++++++
 drivers/firewire/packet-serdes-test.c        | 538 +++++++++++++++++++
 5 files changed, 726 insertions(+)
 create mode 100644 drivers/firewire/packet-header-definitions.h
 create mode 100644 drivers/firewire/packet-serdes-test.c

diff --git a/drivers/firewire/.kunitconfig b/drivers/firewire/.kunitconfig
index 76444a2d5e12..60d9e7c35417 100644
--- a/drivers/firewire/.kunitconfig
+++ b/drivers/firewire/.kunitconfig
@@ -3,3 +3,4 @@ CONFIG_PCI=y
 CONFIG_FIREWIRE=y
 CONFIG_FIREWIRE_KUNIT_UAPI_TEST=y
 CONFIG_FIREWIRE_KUNIT_DEVICE_ATTRIBUTE_TEST=y
+CONFIG_FIREWIRE_KUNIT_PACKET_SERDES_TEST=y
diff --git a/drivers/firewire/Kconfig b/drivers/firewire/Kconfig
index 552a39df8cbd..869598b20e3a 100644
--- a/drivers/firewire/Kconfig
+++ b/drivers/firewire/Kconfig
@@ -50,6 +50,22 @@ config FIREWIRE_KUNIT_DEVICE_ATTRIBUTE_TEST
 	  For more information on KUnit and unit tests in general, refer
 	  to the KUnit documentation in Documentation/dev-tools/kunit/.
 
+config FIREWIRE_KUNIT_PACKET_SERDES_TEST
+	tristate "KUnit tests for packet serialization/deserialization" if !KUNIT_ALL_TESTS
+	depends on FIREWIRE && KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  This builds the KUnit tests for packet serialization and
+	  deserialization.
+
+	  KUnit tests run during boot and output the results to the debug
+	  log in TAP format (https://testanything.org/). Only useful for
+	  kernel devs running KUnit test harness and are not for inclusion
+	  into a production build.
+
+	  For more information on KUnit and unit tests in general, refer
+	  to the KUnit documentation in Documentation/dev-tools/kunit/.
+
 config FIREWIRE_OHCI
 	tristate "OHCI-1394 controllers"
 	depends on PCI && FIREWIRE && MMU
diff --git a/drivers/firewire/Makefile b/drivers/firewire/Makefile
index b24b2879ac34..bbde29a0fba6 100644
--- a/drivers/firewire/Makefile
+++ b/drivers/firewire/Makefile
@@ -17,4 +17,7 @@ obj-$(CONFIG_FIREWIRE_NOSY) += nosy.o
 obj-$(CONFIG_PROVIDE_OHCI1394_DMA_INIT) += init_ohci1394_dma.o
 
 firewire-uapi-test-objs += uapi-test.o
+firewire-packet-serdes-test-objs += packet-serdes-test.o
+
 obj-$(CONFIG_FIREWIRE_KUNIT_UAPI_TEST) += firewire-uapi-test.o
+obj-$(CONFIG_FIREWIRE_KUNIT_PACKET_SERDES_TEST) += firewire-packet-serdes-test.o
diff --git a/drivers/firewire/packet-header-definitions.h b/drivers/firewire/packet-header-definitions.h
new file mode 100644
index 000000000000..83e550427706
--- /dev/null
+++ b/drivers/firewire/packet-header-definitions.h
@@ -0,0 +1,168 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+//
+// packet-header-definitions.h - The definitions of header fields for IEEE 1394 packet.
+//
+// Copyright (c) 2024 Takashi Sakamoto
+
+#ifndef _FIREWIRE_PACKET_HEADER_DEFINITIONS_H
+#define _FIREWIRE_PACKET_HEADER_DEFINITIONS_H
+
+#define ASYNC_HEADER_QUADLET_COUNT		4
+
+#define ASYNC_HEADER_Q0_DESTINATION_SHIFT	16
+#define ASYNC_HEADER_Q0_DESTINATION_MASK	0xffff0000
+#define ASYNC_HEADER_Q0_TLABEL_SHIFT		10
+#define ASYNC_HEADER_Q0_TLABEL_MASK		0x0000fc00
+#define ASYNC_HEADER_Q0_RETRY_SHIFT		8
+#define ASYNC_HEADER_Q0_RETRY_MASK		0x00000300
+#define ASYNC_HEADER_Q0_TCODE_SHIFT		4
+#define ASYNC_HEADER_Q0_TCODE_MASK		0x000000f0
+#define ASYNC_HEADER_Q0_PRIORITY_SHIFT		0
+#define ASYNC_HEADER_Q0_PRIORITY_MASK		0x0000000f
+#define ASYNC_HEADER_Q1_SOURCE_SHIFT		16
+#define ASYNC_HEADER_Q1_SOURCE_MASK		0xffff0000
+#define ASYNC_HEADER_Q1_RCODE_SHIFT		12
+#define ASYNC_HEADER_Q1_RCODE_MASK		0x0000f000
+#define ASYNC_HEADER_Q1_RCODE_SHIFT		12
+#define ASYNC_HEADER_Q1_RCODE_MASK		0x0000f000
+#define ASYNC_HEADER_Q1_OFFSET_HIGH_SHIFT	0
+#define ASYNC_HEADER_Q1_OFFSET_HIGH_MASK	0x0000ffff
+#define ASYNC_HEADER_Q3_DATA_LENGTH_SHIFT	16
+#define ASYNC_HEADER_Q3_DATA_LENGTH_MASK	0xffff0000
+#define ASYNC_HEADER_Q3_EXTENDED_TCODE_SHIFT	0
+#define ASYNC_HEADER_Q3_EXTENDED_TCODE_MASK	0x0000ffff
+
+static inline unsigned int async_header_get_destination(const u32 header[ASYNC_HEADER_QUADLET_COUNT])
+{
+	return (header[0] & ASYNC_HEADER_Q0_DESTINATION_MASK) >> ASYNC_HEADER_Q0_DESTINATION_SHIFT;
+}
+
+static inline unsigned int async_header_get_tlabel(const u32 header[ASYNC_HEADER_QUADLET_COUNT])
+{
+	return (header[0] & ASYNC_HEADER_Q0_TLABEL_MASK) >> ASYNC_HEADER_Q0_TLABEL_SHIFT;
+}
+
+static inline unsigned int async_header_get_retry(const u32 header[ASYNC_HEADER_QUADLET_COUNT])
+{
+	return (header[0] & ASYNC_HEADER_Q0_RETRY_MASK) >> ASYNC_HEADER_Q0_RETRY_SHIFT;
+}
+
+static inline unsigned int async_header_get_tcode(const u32 header[ASYNC_HEADER_QUADLET_COUNT])
+{
+	return (header[0] & ASYNC_HEADER_Q0_TCODE_MASK) >> ASYNC_HEADER_Q0_TCODE_SHIFT;
+}
+
+static inline unsigned int async_header_get_priority(const u32 header[ASYNC_HEADER_QUADLET_COUNT])
+{
+	return (header[0] & ASYNC_HEADER_Q0_PRIORITY_MASK) >> ASYNC_HEADER_Q0_PRIORITY_SHIFT;
+}
+
+static inline unsigned int async_header_get_source(const u32 header[ASYNC_HEADER_QUADLET_COUNT])
+{
+	return (header[1] & ASYNC_HEADER_Q1_SOURCE_MASK) >> ASYNC_HEADER_Q1_SOURCE_SHIFT;
+}
+
+static inline unsigned int async_header_get_rcode(const u32 header[ASYNC_HEADER_QUADLET_COUNT])
+{
+	return (header[1] & ASYNC_HEADER_Q1_RCODE_MASK) >> ASYNC_HEADER_Q1_RCODE_SHIFT;
+}
+
+static inline u64 async_header_get_offset(const u32 header[ASYNC_HEADER_QUADLET_COUNT])
+{
+	u32 hi = (header[1] & ASYNC_HEADER_Q1_OFFSET_HIGH_MASK) >> ASYNC_HEADER_Q1_OFFSET_HIGH_SHIFT;
+	return (((u64)hi) << 32) | ((u64)header[2]);
+}
+
+static inline u32 async_header_get_quadlet_data(const u32 header[ASYNC_HEADER_QUADLET_COUNT])
+{
+	return header[3];
+}
+
+static inline unsigned int async_header_get_data_length(const u32 header[ASYNC_HEADER_QUADLET_COUNT])
+{
+	return (header[3] & ASYNC_HEADER_Q3_DATA_LENGTH_MASK) >> ASYNC_HEADER_Q3_DATA_LENGTH_SHIFT;
+}
+
+static inline unsigned int async_header_get_extended_tcode(const u32 header[ASYNC_HEADER_QUADLET_COUNT])
+{
+	return (header[3] & ASYNC_HEADER_Q3_EXTENDED_TCODE_MASK) >> ASYNC_HEADER_Q3_EXTENDED_TCODE_SHIFT;
+}
+
+static inline void async_header_set_destination(u32 header[ASYNC_HEADER_QUADLET_COUNT],
+						unsigned int destination)
+{
+	header[0] &= ~ASYNC_HEADER_Q0_DESTINATION_MASK;
+	header[0] |= (((u32)destination) << ASYNC_HEADER_Q0_DESTINATION_SHIFT) & ASYNC_HEADER_Q0_DESTINATION_MASK;
+}
+
+static inline void async_header_set_tlabel(u32 header[ASYNC_HEADER_QUADLET_COUNT],
+					   unsigned int tlabel)
+{
+	header[0] &= ~ASYNC_HEADER_Q0_TLABEL_MASK;
+	header[0] |= (((u32)tlabel) << ASYNC_HEADER_Q0_TLABEL_SHIFT) & ASYNC_HEADER_Q0_TLABEL_MASK;
+}
+
+static inline void async_header_set_retry(u32 header[ASYNC_HEADER_QUADLET_COUNT],
+					  unsigned int retry)
+{
+	header[0] &= ~ASYNC_HEADER_Q0_RETRY_MASK;
+	header[0] |= (((u32)retry) << ASYNC_HEADER_Q0_RETRY_SHIFT) & ASYNC_HEADER_Q0_RETRY_MASK;
+}
+
+static inline void async_header_set_tcode(u32 header[ASYNC_HEADER_QUADLET_COUNT],
+					  unsigned int tcode)
+{
+	header[0] &= ~ASYNC_HEADER_Q0_TCODE_MASK;
+	header[0] |= (((u32)tcode) << ASYNC_HEADER_Q0_TCODE_SHIFT) & ASYNC_HEADER_Q0_TCODE_MASK;
+}
+
+static inline void async_header_set_priority(u32 header[ASYNC_HEADER_QUADLET_COUNT],
+					     unsigned int priority)
+{
+	header[0] &= ~ASYNC_HEADER_Q0_PRIORITY_MASK;
+	header[0] |= (((u32)priority) << ASYNC_HEADER_Q0_PRIORITY_SHIFT) & ASYNC_HEADER_Q0_PRIORITY_MASK;
+}
+
+
+static inline void async_header_set_source(u32 header[ASYNC_HEADER_QUADLET_COUNT],
+					   unsigned int source)
+{
+	header[1] &= ~ASYNC_HEADER_Q1_SOURCE_MASK;
+	header[1] |= (((u32)source) << ASYNC_HEADER_Q1_SOURCE_SHIFT) & ASYNC_HEADER_Q1_SOURCE_MASK;
+}
+
+static inline void async_header_set_rcode(u32 header[ASYNC_HEADER_QUADLET_COUNT],
+					  unsigned int rcode)
+{
+	header[1] &= ~ASYNC_HEADER_Q1_RCODE_MASK;
+	header[1] |= (((u32)rcode) << ASYNC_HEADER_Q1_RCODE_SHIFT) & ASYNC_HEADER_Q1_RCODE_MASK;
+}
+
+static inline void async_header_set_offset(u32 header[ASYNC_HEADER_QUADLET_COUNT], u64 offset)
+{
+	u32 hi = (u32)(offset >> 32);
+	header[1] &= ~ASYNC_HEADER_Q1_OFFSET_HIGH_MASK;
+	header[1] |= (hi << ASYNC_HEADER_Q1_OFFSET_HIGH_SHIFT) & ASYNC_HEADER_Q1_OFFSET_HIGH_MASK;
+	header[2] = (u32)(offset & 0x00000000ffffffff);
+}
+
+static inline void async_header_set_quadlet_data(u32 header[ASYNC_HEADER_QUADLET_COUNT], u32 quadlet_data)
+{
+	header[3] = quadlet_data;
+}
+
+static inline void async_header_set_data_length(u32 header[ASYNC_HEADER_QUADLET_COUNT],
+						unsigned int data_length)
+{
+	header[3] &= ~ASYNC_HEADER_Q3_DATA_LENGTH_MASK;
+	header[3] |= (((u32)data_length) << ASYNC_HEADER_Q3_DATA_LENGTH_SHIFT) & ASYNC_HEADER_Q3_DATA_LENGTH_MASK;
+}
+
+static inline void async_header_set_extended_tcode(u32 header[ASYNC_HEADER_QUADLET_COUNT],
+						   unsigned int extended_tcode)
+{
+	header[3] &= ~ASYNC_HEADER_Q3_EXTENDED_TCODE_MASK;
+	header[3] |= (((u32)extended_tcode) << ASYNC_HEADER_Q3_EXTENDED_TCODE_SHIFT) & ASYNC_HEADER_Q3_EXTENDED_TCODE_MASK;
+}
+
+#endif // _FIREWIRE_PACKET_HEADER_DEFINITIONS_H
diff --git a/drivers/firewire/packet-serdes-test.c b/drivers/firewire/packet-serdes-test.c
new file mode 100644
index 000000000000..299e9f908463
--- /dev/null
+++ b/drivers/firewire/packet-serdes-test.c
@@ -0,0 +1,538 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+//
+// packet-serdes-test.c - An application of Kunit to check serialization/deserialization of packets
+//			  defined by IEEE 1394.
+//
+// Copyright (c) 2024 Takashi Sakamoto
+
+#include <kunit/test.h>
+
+#include <linux/firewire-constants.h>
+
+#include "packet-header-definitions.h"
+
+static void serialize_async_header_common(u32 header[ASYNC_HEADER_QUADLET_COUNT],
+					  unsigned int dst_id, unsigned int tlabel,
+					  unsigned int retry, unsigned int tcode,
+					  unsigned int priority, unsigned int src_id)
+{
+	async_header_set_destination(header, dst_id);
+	async_header_set_tlabel(header, tlabel);
+	async_header_set_retry(header, retry);
+	async_header_set_tcode(header, tcode);
+	async_header_set_priority(header, priority);
+	async_header_set_source(header, src_id);
+}
+
+static void serialize_async_header_request(u32 header[ASYNC_HEADER_QUADLET_COUNT],
+					   unsigned int dst_id, unsigned int tlabel,
+					   unsigned int retry, unsigned int tcode,
+					   unsigned int priority, unsigned int src_id, u64 offset)
+{
+	serialize_async_header_common(header, dst_id, tlabel, retry, tcode, priority, src_id);
+	async_header_set_offset(header, offset);
+}
+
+static void serialize_async_header_quadlet_request(u32 header[ASYNC_HEADER_QUADLET_COUNT],
+						   unsigned int dst_id, unsigned int tlabel,
+						   unsigned int retry, unsigned int tcode,
+						   unsigned int priority, unsigned int src_id,
+						   u64 offset)
+{
+	serialize_async_header_request(header, dst_id, tlabel, retry, tcode, priority, src_id,
+				       offset);
+}
+
+static void serialize_async_header_block_request(u32 header[ASYNC_HEADER_QUADLET_COUNT],
+						 unsigned int dst_id, unsigned int tlabel,
+						 unsigned int retry, unsigned int tcode,
+						 unsigned int priority, unsigned int src_id,
+						 u64 offset, unsigned int data_length,
+						 unsigned int extended_tcode)
+{
+	serialize_async_header_request(header, dst_id, tlabel, retry, tcode, priority, src_id,
+				       offset);
+	async_header_set_data_length(header, data_length);
+	async_header_set_extended_tcode(header, extended_tcode);
+}
+
+static void serialize_async_header_response(u32 header[ASYNC_HEADER_QUADLET_COUNT],
+					    unsigned int dst_id, unsigned int tlabel,
+					    unsigned int retry, unsigned int tcode,
+					    unsigned int priority, unsigned int src_id,
+					    unsigned int rcode)
+{
+	serialize_async_header_common(header, dst_id, tlabel, retry, tcode, priority, src_id);
+	async_header_set_rcode(header, rcode);
+}
+
+static void serialize_async_header_quadlet_response(u32 header[ASYNC_HEADER_QUADLET_COUNT],
+						    unsigned int dst_id, unsigned int tlabel,
+						    unsigned int retry, unsigned int tcode,
+						    unsigned int priority, unsigned int src_id,
+						    unsigned int rcode)
+{
+	serialize_async_header_response(header, dst_id, tlabel, retry, tcode, priority, src_id,
+					rcode);
+}
+
+static void serialize_async_header_block_response(u32 header[ASYNC_HEADER_QUADLET_COUNT],
+						  unsigned int dst_id, unsigned int tlabel,
+						  unsigned int retry, unsigned int tcode,
+						  unsigned int priority, unsigned int src_id,
+						  unsigned int rcode, unsigned int data_length,
+						  unsigned int extended_tcode)
+{
+	serialize_async_header_response(header, dst_id, tlabel, retry, tcode, priority, src_id,
+					rcode);
+	async_header_set_data_length(header, data_length);
+	async_header_set_extended_tcode(header, extended_tcode);
+}
+
+static void deserialize_async_header_common(const u32 header[ASYNC_HEADER_QUADLET_COUNT],
+					    unsigned int *dst_id, unsigned int *tlabel,
+					    unsigned int *retry, unsigned int *tcode,
+					    unsigned int *priority, unsigned int *src_id)
+{
+	*dst_id = async_header_get_destination(header);
+	*tlabel = async_header_get_tlabel(header);
+	*retry = async_header_get_retry(header);
+	*tcode = async_header_get_tcode(header);
+	*priority = async_header_get_priority(header);
+	*src_id = async_header_get_source(header);
+}
+
+static void deserialize_async_header_request(const u32 header[ASYNC_HEADER_QUADLET_COUNT],
+					     unsigned int *dst_id, unsigned int *tlabel,
+					     unsigned int *retry, unsigned int *tcode,
+					     unsigned int *priority, unsigned int *src_id,
+					     u64 *offset)
+{
+	deserialize_async_header_common(header, dst_id, tlabel, retry, tcode, priority, src_id);
+	*offset = async_header_get_offset(header);
+}
+
+static void deserialize_async_header_quadlet_request(const u32 header[ASYNC_HEADER_QUADLET_COUNT],
+						     unsigned int *dst_id, unsigned int *tlabel,
+						     unsigned int *retry, unsigned int *tcode,
+						     unsigned int *priority, unsigned int *src_id,
+						     u64 *offset)
+{
+	deserialize_async_header_request(header, dst_id, tlabel, retry, tcode, priority, src_id,
+					 offset);
+}
+
+static void deserialize_async_header_block_request(const u32 header[ASYNC_HEADER_QUADLET_COUNT],
+						   unsigned int *dst_id, unsigned int *tlabel,
+						   unsigned int *retry, unsigned int *tcode,
+						   unsigned int *priority, unsigned int *src_id,
+						   u64 *offset,
+						   unsigned int *data_length,
+						   unsigned int *extended_tcode)
+{
+	deserialize_async_header_request(header, dst_id, tlabel, retry, tcode, priority, src_id,
+					 offset);
+	*data_length = async_header_get_data_length(header);
+	*extended_tcode = async_header_get_extended_tcode(header);
+}
+
+static void deserialize_async_header_response(const u32 header[ASYNC_HEADER_QUADLET_COUNT],
+					      unsigned int *dst_id, unsigned int *tlabel,
+					      unsigned int *retry, unsigned int *tcode,
+					      unsigned int *priority, unsigned int *src_id,
+					      unsigned int *rcode)
+{
+	deserialize_async_header_common(header, dst_id, tlabel, retry, tcode, priority, src_id);
+	*rcode = async_header_get_rcode(header);
+}
+
+static void deserialize_async_header_quadlet_response(const u32 header[ASYNC_HEADER_QUADLET_COUNT],
+						      unsigned int *dst_id, unsigned int *tlabel,
+						      unsigned int *retry, unsigned int *tcode,
+						      unsigned int *priority, unsigned int *src_id,
+						      unsigned int *rcode)
+{
+	deserialize_async_header_response(header, dst_id, tlabel, retry, tcode, priority, src_id, rcode);
+}
+
+static void deserialize_async_header_block_response(const u32 header[ASYNC_HEADER_QUADLET_COUNT],
+						    unsigned int *dst_id, unsigned int *tlabel,
+						    unsigned int *retry, unsigned int *tcode,
+						    unsigned int *priority, unsigned int *src_id,
+						    unsigned int *rcode, unsigned int *data_length,
+						    unsigned int *extended_tcode)
+{
+	deserialize_async_header_response(header, dst_id, tlabel, retry, tcode, priority, src_id, rcode);
+	*data_length = async_header_get_data_length(header);
+	*extended_tcode = async_header_get_extended_tcode(header);
+}
+
+static void test_async_header_write_quadlet_request(struct kunit *test)
+{
+	static const u32 expected[ASYNC_HEADER_QUADLET_COUNT] = {
+		0xffc05100,
+		0xffc1ffff,
+		0xf0000234,
+		0x1f0000c0,
+	};
+	u32 header[ASYNC_HEADER_QUADLET_COUNT] = {0, 0, 0, 0};
+
+	unsigned int dst_id;
+	unsigned int tlabel;
+	unsigned int retry;
+	unsigned int tcode;
+	unsigned int priority;
+	unsigned int src_id;
+	u64 offset;
+	u32 quadlet_data;
+
+	deserialize_async_header_quadlet_request(expected, &dst_id, &tlabel, &retry, &tcode,
+						 &priority, &src_id, &offset);
+	quadlet_data = async_header_get_quadlet_data(expected);
+
+	KUNIT_EXPECT_EQ(test, 0xffc0, dst_id);
+	KUNIT_EXPECT_EQ(test, 0x14, tlabel);
+	KUNIT_EXPECT_EQ(test, 0x01, retry);
+	KUNIT_EXPECT_EQ(test, TCODE_WRITE_QUADLET_REQUEST, tcode);
+	KUNIT_EXPECT_EQ(test, 0x00, priority);
+	KUNIT_EXPECT_EQ(test, 0xffc1, src_id);
+	KUNIT_EXPECT_EQ(test, 0xfffff0000234, offset);
+	KUNIT_EXPECT_EQ(test, 0x1f0000c0, quadlet_data);
+
+	serialize_async_header_quadlet_request(header, dst_id, tlabel, retry, tcode, priority,
+					       src_id, offset);
+	async_header_set_quadlet_data(header, quadlet_data);
+
+	KUNIT_EXPECT_MEMEQ(test, header, expected, sizeof(expected));
+}
+
+static void test_async_header_write_block_request(struct kunit *test)
+{
+	static const u32 expected[ASYNC_HEADER_QUADLET_COUNT] = {
+		0xffc06510,
+		0xffc1ecc0,
+		0x00000000,
+		0x00180000,
+	};
+	u32 header[ASYNC_HEADER_QUADLET_COUNT] = {0, 0, 0, 0};
+
+	unsigned int dst_id;
+	unsigned int tlabel;
+	unsigned int retry;
+	unsigned int tcode;
+	unsigned int priority;
+	unsigned int src_id;
+	u64 offset;
+	unsigned int data_length;
+	unsigned int extended_tcode;
+
+	deserialize_async_header_block_request(expected, &dst_id, &tlabel, &retry, &tcode,
+					       &priority, &src_id, &offset, &data_length,
+					       &extended_tcode);
+
+	KUNIT_EXPECT_EQ(test, 0xffc0, dst_id);
+	KUNIT_EXPECT_EQ(test, 0x19, tlabel);
+	KUNIT_EXPECT_EQ(test, 0x01, retry);
+	KUNIT_EXPECT_EQ(test, TCODE_WRITE_BLOCK_REQUEST, tcode);
+	KUNIT_EXPECT_EQ(test, 0x00, priority);
+	KUNIT_EXPECT_EQ(test, 0xffc1, src_id);
+	KUNIT_EXPECT_EQ(test, 0xecc000000000, offset);
+	KUNIT_EXPECT_EQ(test, 0x0018, data_length);
+	KUNIT_EXPECT_EQ(test, 0x0000, extended_tcode);
+
+	serialize_async_header_block_request(header, dst_id, tlabel, retry, tcode, priority, src_id,
+					     offset, data_length, extended_tcode);
+
+	KUNIT_EXPECT_MEMEQ(test, header, expected, sizeof(expected));
+}
+
+static void test_async_header_write_response(struct kunit *test)
+{
+	static const u32 expected[ASYNC_HEADER_QUADLET_COUNT] = {
+		0xffc15120,
+		0xffc00000,
+		0x00000000,
+		0x00000000,
+	};
+	u32 header[ASYNC_HEADER_QUADLET_COUNT] = {0, 0, 0, 0};
+
+	unsigned int dst_id;
+	unsigned int tlabel;
+	unsigned int retry;
+	unsigned int tcode;
+	unsigned int priority;
+	unsigned int src_id;
+	unsigned int rcode;
+
+	deserialize_async_header_quadlet_response(expected, &dst_id, &tlabel, &retry, &tcode,
+						  &priority, &src_id, &rcode);
+
+	KUNIT_EXPECT_EQ(test, 0xffc1, dst_id);
+	KUNIT_EXPECT_EQ(test, 0x14, tlabel);
+	KUNIT_EXPECT_EQ(test, 0x01, retry);
+	KUNIT_EXPECT_EQ(test, TCODE_WRITE_RESPONSE, tcode);
+	KUNIT_EXPECT_EQ(test, 0x00, priority);
+	KUNIT_EXPECT_EQ(test, 0xffc0, src_id);
+	KUNIT_EXPECT_EQ(test, RCODE_COMPLETE, rcode);
+
+	serialize_async_header_quadlet_response(header, dst_id, tlabel, retry, tcode, priority,
+						src_id, rcode);
+
+	KUNIT_EXPECT_MEMEQ(test, header, expected, sizeof(expected) - sizeof(expected[0]));
+}
+
+static void test_async_header_read_quadlet_request(struct kunit *test)
+{
+	static const u32 expected[ASYNC_HEADER_QUADLET_COUNT] = {
+		0xffc0f140,
+		0xffc1ffff,
+		0xf0000984,
+		0x00000000,
+	};
+	u32 header[ASYNC_HEADER_QUADLET_COUNT] = {0, 0, 0, 0};
+
+	unsigned int dst_id;
+	unsigned int tlabel;
+	unsigned int retry;
+	unsigned int tcode;
+	unsigned int priority;
+	unsigned int src_id;
+	u64 offset;
+
+	deserialize_async_header_quadlet_request(expected, &dst_id, &tlabel, &retry, &tcode,
+						 &priority, &src_id, &offset);
+
+	KUNIT_EXPECT_EQ(test, 0xffc0, dst_id);
+	KUNIT_EXPECT_EQ(test, 0x3c, tlabel);
+	KUNIT_EXPECT_EQ(test, 0x01, retry);
+	KUNIT_EXPECT_EQ(test, TCODE_READ_QUADLET_REQUEST, tcode);
+	KUNIT_EXPECT_EQ(test, 0x00, priority);
+	KUNIT_EXPECT_EQ(test, 0xffc1, src_id);
+	KUNIT_EXPECT_EQ(test, 0xfffff0000984, offset);
+
+	serialize_async_header_quadlet_request(header, dst_id, tlabel, retry, tcode, priority,
+					       src_id, offset);
+
+	KUNIT_EXPECT_MEMEQ(test, header, expected, sizeof(expected));
+}
+
+static void test_async_header_read_quadlet_response(struct kunit *test)
+{
+	static const u32 expected[ASYNC_HEADER_QUADLET_COUNT] = {
+		0xffc1f160,
+		0xffc00000,
+		0x00000000,
+		0x00000180,
+	};
+	u32 header[ASYNC_HEADER_QUADLET_COUNT] = {0, 0, 0, 0};
+
+	unsigned int dst_id;
+	unsigned int tlabel;
+	unsigned int retry;
+	unsigned int tcode;
+	unsigned int priority;
+	unsigned int src_id;
+	unsigned int rcode;
+	u32 quadlet_data;
+
+	deserialize_async_header_quadlet_response(expected, &dst_id, &tlabel, &retry, &tcode,
+						  &priority, &src_id, &rcode);
+	quadlet_data = async_header_get_quadlet_data(expected);
+
+	KUNIT_EXPECT_EQ(test, 0xffc1, dst_id);
+	KUNIT_EXPECT_EQ(test, 0x3c, tlabel);
+	KUNIT_EXPECT_EQ(test, 0x01, retry);
+	KUNIT_EXPECT_EQ(test, TCODE_READ_QUADLET_RESPONSE, tcode);
+	KUNIT_EXPECT_EQ(test, 0x00, priority);
+	KUNIT_EXPECT_EQ(test, 0xffc0, src_id);
+	KUNIT_EXPECT_EQ(test, RCODE_COMPLETE, rcode);
+	KUNIT_EXPECT_EQ(test, 0x00000180, quadlet_data);
+
+	serialize_async_header_quadlet_response(header, dst_id, tlabel, retry, tcode, priority,
+						src_id, rcode);
+	async_header_set_quadlet_data(header, quadlet_data);
+
+	KUNIT_EXPECT_MEMEQ(test, header, expected, sizeof(expected));
+}
+
+static void test_async_header_read_block_request(struct kunit *test)
+{
+	static const u32 expected[ASYNC_HEADER_QUADLET_COUNT] = {
+		0xffc0e150,
+		0xffc1ffff,
+		0xf0000400,
+		0x00200000,
+	};
+	u32 header[ASYNC_HEADER_QUADLET_COUNT] = {0, 0, 0, 0};
+
+	unsigned int dst_id;
+	unsigned int tlabel;
+	unsigned int retry;
+	unsigned int tcode;
+	unsigned int priority;
+	unsigned int src_id;
+	u64 offset;
+	unsigned int data_length;
+	unsigned int extended_tcode;
+
+	deserialize_async_header_block_request(expected, &dst_id, &tlabel, &retry, &tcode,
+					       &priority, &src_id, &offset, &data_length,
+					       &extended_tcode);
+
+	KUNIT_EXPECT_EQ(test, 0xffc0, dst_id);
+	KUNIT_EXPECT_EQ(test, 0x38, tlabel);
+	KUNIT_EXPECT_EQ(test, 0x01, retry);
+	KUNIT_EXPECT_EQ(test, TCODE_READ_BLOCK_REQUEST, tcode);
+	KUNIT_EXPECT_EQ(test, 0x00, priority);
+	KUNIT_EXPECT_EQ(test, 0xffc1, src_id);
+	KUNIT_EXPECT_EQ(test, 0xfffff0000400, offset);
+	KUNIT_EXPECT_EQ(test, 0x0020, data_length);
+	KUNIT_EXPECT_EQ(test, 0x0000, extended_tcode);
+
+	serialize_async_header_block_request(header, dst_id, tlabel, retry, tcode, priority, src_id,
+					     offset, data_length, extended_tcode);
+
+	KUNIT_EXPECT_MEMEQ(test, header, expected, sizeof(expected));
+}
+
+static void test_async_header_read_block_response(struct kunit *test)
+{
+	static const u32 expected[ASYNC_HEADER_QUADLET_COUNT] = {
+		0xffc1e170,
+		0xffc00000,
+		0x00000000,
+		0x00200000,
+	};
+	u32 header[ASYNC_HEADER_QUADLET_COUNT] = {0, 0, 0, 0};
+
+	unsigned int dst_id;
+	unsigned int tlabel;
+	unsigned int retry;
+	unsigned int tcode;
+	unsigned int priority;
+	unsigned int src_id;
+	unsigned int rcode;
+	unsigned int data_length;
+	unsigned int extended_tcode;
+
+	deserialize_async_header_block_response(expected, &dst_id, &tlabel, &retry, &tcode,
+						&priority, &src_id, &rcode, &data_length,
+						&extended_tcode);
+
+	KUNIT_EXPECT_EQ(test, 0xffc1, dst_id);
+	KUNIT_EXPECT_EQ(test, 0x38, tlabel);
+	KUNIT_EXPECT_EQ(test, 0x01, retry);
+	KUNIT_EXPECT_EQ(test, TCODE_READ_BLOCK_RESPONSE, tcode);
+	KUNIT_EXPECT_EQ(test, 0x00, priority);
+	KUNIT_EXPECT_EQ(test, 0xffc0, src_id);
+	KUNIT_EXPECT_EQ(test, RCODE_COMPLETE, rcode);
+	KUNIT_EXPECT_EQ(test, 0x0020, data_length);
+	KUNIT_EXPECT_EQ(test, 0x0000, extended_tcode);
+
+	serialize_async_header_block_response(header, dst_id, tlabel, retry, tcode, priority,
+					      src_id, rcode, data_length, extended_tcode);
+
+	KUNIT_EXPECT_MEMEQ(test, header, expected, sizeof(expected));
+}
+
+static void test_async_header_lock_request(struct kunit *test)
+{
+	static const u32 expected[ASYNC_HEADER_QUADLET_COUNT] = {
+		0xffc02d90,
+		0xffc1ffff,
+		0xf0000984,
+		0x00080002,
+	};
+	u32 header[ASYNC_HEADER_QUADLET_COUNT] = {0, 0, 0, 0};
+
+	unsigned int dst_id;
+	unsigned int tlabel;
+	unsigned int retry;
+	unsigned int tcode;
+	unsigned int priority;
+	unsigned int src_id;
+	u64 offset;
+	unsigned int data_length;
+	unsigned int extended_tcode;
+
+	deserialize_async_header_block_request(expected, &dst_id, &tlabel, &retry, &tcode,
+					       &priority, &src_id, &offset, &data_length,
+					       &extended_tcode);
+
+	KUNIT_EXPECT_EQ(test, 0xffc0, dst_id);
+	KUNIT_EXPECT_EQ(test, 0x0b, tlabel);
+	KUNIT_EXPECT_EQ(test, 0x01, retry);
+	KUNIT_EXPECT_EQ(test, TCODE_LOCK_REQUEST, tcode);
+	KUNIT_EXPECT_EQ(test, 0x00, priority);
+	KUNIT_EXPECT_EQ(test, 0xffc1, src_id);
+	KUNIT_EXPECT_EQ(test, 0xfffff0000984, offset);
+	KUNIT_EXPECT_EQ(test, 0x0008, data_length);
+	KUNIT_EXPECT_EQ(test, EXTCODE_COMPARE_SWAP, extended_tcode);
+
+	serialize_async_header_block_request(header, dst_id, tlabel, retry, tcode, priority, src_id,
+					     offset, data_length, extended_tcode);
+
+	KUNIT_EXPECT_MEMEQ(test, header, expected, sizeof(expected));
+}
+
+static void test_async_header_lock_response(struct kunit *test)
+{
+	static const u32 expected[ASYNC_HEADER_QUADLET_COUNT] = {
+		0xffc12db0,
+		0xffc00000,
+		0x00000000,
+		0x00040002,
+	};
+	u32 header[ASYNC_HEADER_QUADLET_COUNT] = {0, 0, 0, 0};
+
+	unsigned int dst_id;
+	unsigned int tlabel;
+	unsigned int retry;
+	unsigned int tcode;
+	unsigned int priority;
+	unsigned int src_id;
+	unsigned int rcode;
+	unsigned int data_length;
+	unsigned int extended_tcode;
+
+	deserialize_async_header_block_response(expected, &dst_id, &tlabel, &retry, &tcode,
+						&priority, &src_id, &rcode, &data_length,
+						&extended_tcode);
+
+	KUNIT_EXPECT_EQ(test, 0xffc1, dst_id);
+	KUNIT_EXPECT_EQ(test, 0x0b, tlabel);
+	KUNIT_EXPECT_EQ(test, 0x01, retry);
+	KUNIT_EXPECT_EQ(test, TCODE_LOCK_RESPONSE, tcode);
+	KUNIT_EXPECT_EQ(test, 0x00, priority);
+	KUNIT_EXPECT_EQ(test, 0xffc0, src_id);
+	KUNIT_EXPECT_EQ(test, RCODE_COMPLETE, rcode);
+	KUNIT_EXPECT_EQ(test, 0x0004, data_length);
+	KUNIT_EXPECT_EQ(test, EXTCODE_COMPARE_SWAP, extended_tcode);
+
+	serialize_async_header_block_response(header, dst_id, tlabel, retry, tcode, priority,
+					      src_id, rcode, data_length, extended_tcode);
+
+	KUNIT_EXPECT_MEMEQ(test, header, expected, sizeof(expected));
+}
+
+
+static struct kunit_case packet_serdes_test_cases[] = {
+	KUNIT_CASE(test_async_header_write_quadlet_request),
+	KUNIT_CASE(test_async_header_write_block_request),
+	KUNIT_CASE(test_async_header_write_response),
+	KUNIT_CASE(test_async_header_read_quadlet_request),
+	KUNIT_CASE(test_async_header_read_quadlet_response),
+	KUNIT_CASE(test_async_header_read_block_request),
+	KUNIT_CASE(test_async_header_read_block_response),
+	KUNIT_CASE(test_async_header_lock_request),
+	KUNIT_CASE(test_async_header_lock_response),
+	{}
+};
+
+static struct kunit_suite packet_serdes_test_suite = {
+	.name = "firewire-packet-serdes",
+	.test_cases = packet_serdes_test_cases,
+};
+kunit_test_suite(packet_serdes_test_suite);
+
+MODULE_LICENSE("GPL");
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 02/13] firewire: core: replace local macros with common inline functions for asynchronous packet header
  2024-04-18  9:22 ` [RFC PATCH 01/13] firewire: core: add common inline functions to serialize/deserialize asynchronous packet header Takashi Sakamoto
@ 2024-04-18  9:22   ` Takashi Sakamoto
  2024-04-18  9:22     ` [RFC PATCH 03/13] firewire: ohci: " Takashi Sakamoto
  0 siblings, 1 reply; 14+ messages in thread
From: Takashi Sakamoto @ 2024-04-18  9:22 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

This commit uses common inline functions to serialize and deserialize
header for asynchronous packet.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-transaction.c | 138 +++++++++++-----------------
 1 file changed, 56 insertions(+), 82 deletions(-)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index 130b95aca629..01ce07f87452 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -29,29 +29,13 @@
 #include <asm/byteorder.h>
 
 #include "core.h"
+#include "packet-header-definitions.h"
 
-#define HEADER_PRI(pri)			((pri) << 0)
 #define HEADER_TCODE(tcode)		((tcode) << 4)
-#define HEADER_RETRY(retry)		((retry) << 8)
-#define HEADER_TLABEL(tlabel)		((tlabel) << 10)
-#define HEADER_DESTINATION(destination)	((destination) << 16)
-#define HEADER_SOURCE(source)		((source) << 16)
-#define HEADER_RCODE(rcode)		((rcode) << 12)
-#define HEADER_OFFSET_HIGH(offset_high)	((offset_high) << 0)
 #define HEADER_DATA_LENGTH(length)	((length) << 16)
-#define HEADER_EXTENDED_TCODE(tcode)	((tcode) << 0)
 
-#define HEADER_GET_TCODE(q)		(((q) >> 4) & 0x0f)
-#define HEADER_GET_TLABEL(q)		(((q) >> 10) & 0x3f)
-#define HEADER_GET_RCODE(q)		(((q) >> 12) & 0x0f)
-#define HEADER_GET_DESTINATION(q)	(((q) >> 16) & 0xffff)
-#define HEADER_GET_SOURCE(q)		(((q) >> 16) & 0xffff)
-#define HEADER_GET_OFFSET_HIGH(q)	(((q) >> 0) & 0xffff)
-#define HEADER_GET_DATA_LENGTH(q)	(((q) >> 16) & 0xffff)
-#define HEADER_GET_EXTENDED_TCODE(q)	(((q) >> 0) & 0xffff)
-
-#define HEADER_DESTINATION_IS_BROADCAST(q) \
-	(((q) & HEADER_DESTINATION(0x3f)) == HEADER_DESTINATION(0x3f))
+#define HEADER_DESTINATION_IS_BROADCAST(header) \
+	((async_header_get_destination(header) & 0x3f) == 0x3f)
 
 #define PHY_PACKET_CONFIG	0x0
 #define PHY_PACKET_LINK_ON	0x1
@@ -248,28 +232,24 @@ static void fw_fill_request(struct fw_packet *packet, int tcode, int tlabel,
 	} else
 		ext_tcode = 0;
 
-	packet->header[0] =
-		HEADER_RETRY(RETRY_X) |
-		HEADER_TLABEL(tlabel) |
-		HEADER_TCODE(tcode) |
-		HEADER_DESTINATION(destination_id);
-	packet->header[1] =
-		HEADER_OFFSET_HIGH(offset >> 32) | HEADER_SOURCE(source_id);
-	packet->header[2] =
-		offset;
+	async_header_set_retry(packet->header, RETRY_X);
+	async_header_set_tlabel(packet->header, tlabel);
+	async_header_set_tcode(packet->header, tcode);
+	async_header_set_destination(packet->header, destination_id);
+	async_header_set_source(packet->header, source_id);
+	async_header_set_offset(packet->header, offset);
 
 	switch (tcode) {
 	case TCODE_WRITE_QUADLET_REQUEST:
-		packet->header[3] = *(u32 *)payload;
+		async_header_set_quadlet_data(packet->header, *(u32 *)payload);
 		packet->header_length = 16;
 		packet->payload_length = 0;
 		break;
 
 	case TCODE_LOCK_REQUEST:
 	case TCODE_WRITE_BLOCK_REQUEST:
-		packet->header[3] =
-			HEADER_DATA_LENGTH(length) |
-			HEADER_EXTENDED_TCODE(ext_tcode);
+		async_header_set_data_length(packet->header, length);
+		async_header_set_extended_tcode(packet->header, ext_tcode);
 		packet->header_length = 16;
 		packet->payload = payload;
 		packet->payload_length = length;
@@ -281,9 +261,8 @@ static void fw_fill_request(struct fw_packet *packet, int tcode, int tlabel,
 		break;
 
 	case TCODE_READ_BLOCK_REQUEST:
-		packet->header[3] =
-			HEADER_DATA_LENGTH(length) |
-			HEADER_EXTENDED_TCODE(ext_tcode);
+		async_header_set_data_length(packet->header, length);
+		async_header_set_extended_tcode(packet->header, ext_tcode);
 		packet->header_length = 16;
 		packet->payload_length = 0;
 		break;
@@ -655,7 +634,7 @@ EXPORT_SYMBOL(fw_core_remove_address_handler);
 struct fw_request {
 	struct kref kref;
 	struct fw_packet response;
-	u32 request_header[4];
+	u32 request_header[ASYNC_HEADER_QUADLET_COUNT];
 	int ack;
 	u32 timestamp;
 	u32 length;
@@ -695,7 +674,7 @@ int fw_get_response_length(struct fw_request *r)
 {
 	int tcode, ext_tcode, data_length;
 
-	tcode = HEADER_GET_TCODE(r->request_header[0]);
+	tcode = async_header_get_tcode(r->request_header);
 
 	switch (tcode) {
 	case TCODE_WRITE_QUADLET_REQUEST:
@@ -706,12 +685,12 @@ int fw_get_response_length(struct fw_request *r)
 		return 4;
 
 	case TCODE_READ_BLOCK_REQUEST:
-		data_length = HEADER_GET_DATA_LENGTH(r->request_header[3]);
+		data_length = async_header_get_data_length(r->request_header);
 		return data_length;
 
 	case TCODE_LOCK_REQUEST:
-		ext_tcode = HEADER_GET_EXTENDED_TCODE(r->request_header[3]);
-		data_length = HEADER_GET_DATA_LENGTH(r->request_header[3]);
+		ext_tcode = async_header_get_extended_tcode(r->request_header);
+		data_length = async_header_get_data_length(r->request_header);
 		switch (ext_tcode) {
 		case EXTCODE_FETCH_ADD:
 		case EXTCODE_LITTLE_ADD:
@@ -731,46 +710,42 @@ void fw_fill_response(struct fw_packet *response, u32 *request_header,
 {
 	int tcode, tlabel, extended_tcode, source, destination;
 
-	tcode          = HEADER_GET_TCODE(request_header[0]);
-	tlabel         = HEADER_GET_TLABEL(request_header[0]);
-	source         = HEADER_GET_DESTINATION(request_header[0]);
-	destination    = HEADER_GET_SOURCE(request_header[1]);
-	extended_tcode = HEADER_GET_EXTENDED_TCODE(request_header[3]);
-
-	response->header[0] =
-		HEADER_RETRY(RETRY_1) |
-		HEADER_TLABEL(tlabel) |
-		HEADER_DESTINATION(destination);
-	response->header[1] =
-		HEADER_SOURCE(source) |
-		HEADER_RCODE(rcode);
-	response->header[2] = 0;
+	tcode = async_header_get_tcode(request_header);
+	tlabel = async_header_get_tlabel(request_header);
+	source = async_header_get_source(request_header);
+	destination = async_header_get_destination(request_header);
+	extended_tcode = async_header_get_extended_tcode(request_header);
+
+	async_header_set_retry(response->header, RETRY_1);
+	async_header_set_tlabel(response->header, tlabel);
+	async_header_set_destination(response->header, destination);
+	async_header_set_source(response->header, source);
+	async_header_set_rcode(response->header, rcode);
+	response->header[2] = 0;	// The field is reserved.
 
 	switch (tcode) {
 	case TCODE_WRITE_QUADLET_REQUEST:
 	case TCODE_WRITE_BLOCK_REQUEST:
-		response->header[0] |= HEADER_TCODE(TCODE_WRITE_RESPONSE);
+		async_header_set_tcode(response->header, TCODE_WRITE_RESPONSE);
 		response->header_length = 12;
 		response->payload_length = 0;
 		break;
 
 	case TCODE_READ_QUADLET_REQUEST:
-		response->header[0] |=
-			HEADER_TCODE(TCODE_READ_QUADLET_RESPONSE);
+		async_header_set_tcode(response->header, TCODE_READ_QUADLET_RESPONSE);
 		if (payload != NULL)
-			response->header[3] = *(u32 *)payload;
+			async_header_set_quadlet_data(response->header, *(u32 *)payload);
 		else
-			response->header[3] = 0;
+			async_header_set_quadlet_data(response->header, 0);
 		response->header_length = 16;
 		response->payload_length = 0;
 		break;
 
 	case TCODE_READ_BLOCK_REQUEST:
 	case TCODE_LOCK_REQUEST:
-		response->header[0] |= HEADER_TCODE(tcode + 2);
-		response->header[3] =
-			HEADER_DATA_LENGTH(length) |
-			HEADER_EXTENDED_TCODE(extended_tcode);
+		async_header_set_tcode(response->header, tcode + 2);
+		async_header_set_data_length(response->header, length);
+		async_header_set_extended_tcode(response->header, extended_tcode);
 		response->header_length = 16;
 		response->payload = payload;
 		response->payload_length = length;
@@ -807,7 +782,7 @@ static struct fw_request *allocate_request(struct fw_card *card,
 	u32 *data, length;
 	int request_tcode;
 
-	request_tcode = HEADER_GET_TCODE(p->header[0]);
+	request_tcode = async_header_get_tcode(p->header);
 	switch (request_tcode) {
 	case TCODE_WRITE_QUADLET_REQUEST:
 		data = &p->header[3];
@@ -817,7 +792,7 @@ static struct fw_request *allocate_request(struct fw_card *card,
 	case TCODE_WRITE_BLOCK_REQUEST:
 	case TCODE_LOCK_REQUEST:
 		data = p->payload;
-		length = HEADER_GET_DATA_LENGTH(p->header[3]);
+		length = async_header_get_data_length(p->header);
 		break;
 
 	case TCODE_READ_QUADLET_REQUEST:
@@ -827,7 +802,7 @@ static struct fw_request *allocate_request(struct fw_card *card,
 
 	case TCODE_READ_BLOCK_REQUEST:
 		data = NULL;
-		length = HEADER_GET_DATA_LENGTH(p->header[3]);
+		length = async_header_get_data_length(p->header);
 		break;
 
 	default:
@@ -872,7 +847,7 @@ void fw_send_response(struct fw_card *card,
 {
 	/* unified transaction or broadcast transaction: don't respond */
 	if (request->ack != ACK_PENDING ||
-	    HEADER_DESTINATION_IS_BROADCAST(request->request_header[0])) {
+	    HEADER_DESTINATION_IS_BROADCAST(request->request_header)) {
 		fw_request_put(request);
 		return;
 	}
@@ -926,11 +901,11 @@ static void handle_exclusive_region_request(struct fw_card *card,
 	struct fw_address_handler *handler;
 	int tcode, destination, source;
 
-	destination = HEADER_GET_DESTINATION(p->header[0]);
-	source      = HEADER_GET_SOURCE(p->header[1]);
-	tcode       = HEADER_GET_TCODE(p->header[0]);
+	destination = async_header_get_destination(p->header);
+	source = async_header_get_source(p->header);
+	tcode = async_header_get_tcode(p->header);
 	if (tcode == TCODE_LOCK_REQUEST)
-		tcode = 0x10 + HEADER_GET_EXTENDED_TCODE(p->header[3]);
+		tcode = 0x10 + async_header_get_extended_tcode(p->header);
 
 	rcu_read_lock();
 	handler = lookup_enclosing_address_handler(&address_handler_list,
@@ -963,9 +938,9 @@ static void handle_fcp_region_request(struct fw_card *card,
 		return;
 	}
 
-	tcode       = HEADER_GET_TCODE(p->header[0]);
-	destination = HEADER_GET_DESTINATION(p->header[0]);
-	source      = HEADER_GET_SOURCE(p->header[1]);
+	tcode = async_header_get_tcode(p->header);
+	destination = async_header_get_destination(p->header);
+	source = async_header_get_source(p->header);
 
 	if (tcode != TCODE_WRITE_QUADLET_REQUEST &&
 	    tcode != TCODE_WRITE_BLOCK_REQUEST) {
@@ -997,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(HEADER_GET_TCODE(p->header[0]))) {
+	if (TCODE_IS_LINK_INTERNAL(async_header_get_tcode(p->header))) {
 		fw_cdev_handle_phy_packet(card, p);
 		return;
 	}
@@ -1008,8 +983,7 @@ void fw_core_handle_request(struct fw_card *card, struct fw_packet *p)
 		return;
 	}
 
-	offset = ((u64)HEADER_GET_OFFSET_HIGH(p->header[1]) << 32) |
-		p->header[2];
+	offset = async_header_get_offset(p->header);
 
 	if (!is_in_fcp_region(offset, request->length))
 		handle_exclusive_region_request(card, p, request, offset);
@@ -1027,10 +1001,10 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p)
 	size_t data_length;
 	int tcode, tlabel, source, rcode;
 
-	tcode	= HEADER_GET_TCODE(p->header[0]);
-	tlabel	= HEADER_GET_TLABEL(p->header[0]);
-	source	= HEADER_GET_SOURCE(p->header[1]);
-	rcode	= HEADER_GET_RCODE(p->header[1]);
+	tcode = async_header_get_tcode(p->header);
+	tlabel = async_header_get_tlabel(p->header);
+	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) {
@@ -1073,7 +1047,7 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p)
 	case TCODE_READ_BLOCK_RESPONSE:
 	case TCODE_LOCK_RESPONSE:
 		data = p->payload;
-		data_length = HEADER_GET_DATA_LENGTH(p->header[3]);
+		data_length = async_header_get_data_length(p->header);
 		break;
 
 	default:
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 03/13] firewire: ohci: replace local macros with common inline functions for asynchronous packet header
  2024-04-18  9:22   ` [RFC PATCH 02/13] firewire: core: replace local macros with common inline functions for " Takashi Sakamoto
@ 2024-04-18  9:22     ` Takashi Sakamoto
  2024-04-18  9:22       ` [RFC PATCH 04/13] firewire: ohci: replace hard-coded values with " Takashi Sakamoto
  0 siblings, 1 reply; 14+ messages in thread
From: Takashi Sakamoto @ 2024-04-18  9:22 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

This commit uses the common inline functions to serialize and deserialize
header for asynchronous packet.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/ohci.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 38d19410a2be..5254cf5c2e58 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -40,6 +40,7 @@
 
 #include "core.h"
 #include "ohci.h"
+#include "packet-header-definitions.h"
 
 #define ohci_info(ohci, f, args...)	dev_info(ohci->card.device, f, ##args)
 #define ohci_notice(ohci, f, args...)	dev_notice(ohci->card.device, f, ##args)
@@ -1550,21 +1551,15 @@ static int handle_at_packet(struct context *context,
 	return 1;
 }
 
-#define HEADER_GET_DESTINATION(q)	(((q) >> 16) & 0xffff)
-#define HEADER_GET_TCODE(q)		(((q) >> 4) & 0x0f)
-#define HEADER_GET_OFFSET_HIGH(q)	(((q) >> 0) & 0xffff)
-#define HEADER_GET_DATA_LENGTH(q)	(((q) >> 16) & 0xffff)
-#define HEADER_GET_EXTENDED_TCODE(q)	(((q) >> 0) & 0xffff)
-
 static void handle_local_rom(struct fw_ohci *ohci,
 			     struct fw_packet *packet, u32 csr)
 {
 	struct fw_packet response;
 	int tcode, length, i;
 
-	tcode = HEADER_GET_TCODE(packet->header[0]);
+	tcode = async_header_get_tcode(packet->header);
 	if (TCODE_IS_BLOCK_PACKET(tcode))
-		length = HEADER_GET_DATA_LENGTH(packet->header[3]);
+		length = async_header_get_data_length(packet->header);
 	else
 		length = 4;
 
@@ -1591,10 +1586,10 @@ static void handle_local_lock(struct fw_ohci *ohci,
 	__be32 *payload, lock_old;
 	u32 lock_arg, lock_data;
 
-	tcode = HEADER_GET_TCODE(packet->header[0]);
-	length = HEADER_GET_DATA_LENGTH(packet->header[3]);
+	tcode = async_header_get_tcode(packet->header);
+	length = async_header_get_data_length(packet->header);
 	payload = packet->payload;
-	ext_tcode = HEADER_GET_EXTENDED_TCODE(packet->header[3]);
+	ext_tcode = async_header_get_extended_tcode(packet->header);
 
 	if (tcode == TCODE_LOCK_REQUEST &&
 	    ext_tcode == EXTCODE_COMPARE_SWAP && length == 8) {
@@ -1640,10 +1635,7 @@ static void handle_local_request(struct context *ctx, struct fw_packet *packet)
 		packet->callback(packet, &ctx->ohci->card, packet->ack);
 	}
 
-	offset =
-		((unsigned long long)
-		 HEADER_GET_OFFSET_HIGH(packet->header[1]) << 32) |
-		packet->header[2];
+	offset = async_header_get_offset(packet->header);
 	csr = offset - CSR_REGISTER_BASE;
 
 	/* Handle config rom reads. */
@@ -1679,7 +1671,7 @@ static void at_context_transmit(struct context *ctx, struct fw_packet *packet)
 
 	spin_lock_irqsave(&ctx->ohci->lock, flags);
 
-	if (HEADER_GET_DESTINATION(packet->header[0]) == ctx->ohci->node_id &&
+	if (async_header_get_destination(packet->header) == ctx->ohci->node_id &&
 	    ctx->ohci->generation == packet->generation) {
 		spin_unlock_irqrestore(&ctx->ohci->lock, flags);
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 04/13] firewire: ohci: replace hard-coded values with inline functions for asynchronous packet header
  2024-04-18  9:22     ` [RFC PATCH 03/13] firewire: ohci: " Takashi Sakamoto
@ 2024-04-18  9:22       ` Takashi Sakamoto
  2024-04-18  9:22         ` [RFC PATCH 05/13] firewire: ohci: replace hard-coded values with common macros Takashi Sakamoto
  0 siblings, 1 reply; 14+ messages in thread
From: Takashi Sakamoto @ 2024-04-18  9:22 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

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 <o-takashi@sakamocchi.jp>
---
 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 05/13] firewire: ohci: replace hard-coded values with common macros
  2024-04-18  9:22       ` [RFC PATCH 04/13] firewire: ohci: replace hard-coded values with " Takashi Sakamoto
@ 2024-04-18  9:22         ` Takashi Sakamoto
  2024-04-18  9:22           ` [RFC PATCH 06/13] firewire: core: obsolete tcode check macros with inline functions Takashi Sakamoto
  0 siblings, 1 reply; 14+ messages in thread
From: Takashi Sakamoto @ 2024-04-18  9:22 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

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 <o-takashi@sakamocchi.jp>
---
 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 06/13] firewire: core: obsolete tcode check macros with inline functions
  2024-04-18  9:22         ` [RFC PATCH 05/13] firewire: ohci: replace hard-coded values with common macros Takashi Sakamoto
@ 2024-04-18  9:22           ` Takashi Sakamoto
  2024-04-18  9:22             ` [RFC PATCH 07/13] firewire: core: add common macro to serialize/deserialize isochronous packet header Takashi Sakamoto
  0 siblings, 1 reply; 14+ messages in thread
From: Takashi Sakamoto @ 2024-04-18  9:22 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

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

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 07/13] firewire: core: add common macro to serialize/deserialize isochronous packet header
  2024-04-18  9:22           ` [RFC PATCH 06/13] firewire: core: obsolete tcode check macros with inline functions Takashi Sakamoto
@ 2024-04-18  9:22             ` Takashi Sakamoto
  2024-04-18  9:22               ` [RFC PATCH 08/13] firewire: core: replace local macros with common inline functions for " Takashi Sakamoto
  0 siblings, 1 reply; 14+ messages in thread
From: Takashi Sakamoto @ 2024-04-18  9:22 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

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 <o-takashi@sakamocchi.jp>
---
 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 08/13] firewire: core: replace local macros with common inline functions for isochronous packet header
  2024-04-18  9:22             ` [RFC PATCH 07/13] firewire: core: add common macro to serialize/deserialize isochronous packet header Takashi Sakamoto
@ 2024-04-18  9:22               ` Takashi Sakamoto
  2024-04-18  9:22                 ` [RFC PATCH 09/13] firewire: core: add support for Linux kernel tracepoints Takashi Sakamoto
  0 siblings, 1 reply; 14+ messages in thread
From: Takashi Sakamoto @ 2024-04-18  9:22 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

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 <o-takashi@sakamocchi.jp>
---
 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 09/13] firewire: core: add support for Linux kernel tracepoints
  2024-04-18  9:22               ` [RFC PATCH 08/13] firewire: core: replace local macros with common inline functions for " Takashi Sakamoto
@ 2024-04-18  9:22                 ` Takashi Sakamoto
  2024-04-18  9:23                   ` [RFC PATCH 10/13] firewire: core: add tracepoints events for asynchronous outbound request Takashi Sakamoto
  0 siblings, 1 reply; 14+ messages in thread
From: Takashi Sakamoto @ 2024-04-18  9:22 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

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 <o-takashi@sakamocchi.jp>
---
 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 10/13] firewire: core: add tracepoints events for asynchronous outbound request
  2024-04-18  9:22                 ` [RFC PATCH 09/13] firewire: core: add support for Linux kernel tracepoints Takashi Sakamoto
@ 2024-04-18  9:23                   ` Takashi Sakamoto
  2024-04-18  9:23                     ` [RFC PATCH 11/13] firewire: core: add tracepoints event for asynchronous inbound response Takashi Sakamoto
  0 siblings, 1 reply; 14+ messages in thread
From: Takashi Sakamoto @ 2024-04-18  9:23 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

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 <o-takashi@sakamocchi.jp>
---
 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 11/13] firewire: core: add tracepoints event for asynchronous inbound response
  2024-04-18  9:23                   ` [RFC PATCH 10/13] firewire: core: add tracepoints events for asynchronous outbound request Takashi Sakamoto
@ 2024-04-18  9:23                     ` Takashi Sakamoto
  2024-04-18  9:23                       ` [RFC PATCH 12/13] firewire: core: add tracepoint event for asynchronous inbound request Takashi Sakamoto
  0 siblings, 1 reply; 14+ messages in thread
From: Takashi Sakamoto @ 2024-04-18  9:23 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

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 <o-takashi@sakamocchi.jp>
---
 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 12/13] firewire: core: add tracepoint event for asynchronous inbound request
  2024-04-18  9:23                     ` [RFC PATCH 11/13] firewire: core: add tracepoints event for asynchronous inbound response Takashi Sakamoto
@ 2024-04-18  9:23                       ` Takashi Sakamoto
  2024-04-18  9:23                         ` [RFC PATCH 13/13] firewire: core: add tracepoints events for asynchronous outbound response Takashi Sakamoto
  0 siblings, 1 reply; 14+ messages in thread
From: Takashi Sakamoto @ 2024-04-18  9:23 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

This commit adds an event for asynchronous inbound request.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 13/13] firewire: core: add tracepoints events for asynchronous outbound response
  2024-04-18  9:23                       ` [RFC PATCH 12/13] firewire: core: add tracepoint event for asynchronous inbound request Takashi Sakamoto
@ 2024-04-18  9:23                         ` Takashi Sakamoto
  0 siblings, 0 replies; 14+ messages in thread
From: Takashi Sakamoto @ 2024-04-18  9:23 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

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

This commit adds a pair of events for the asynchronous outbound response.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-transaction.c | 21 ++++++---
 drivers/firewire/trace.h            | 68 +++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 7 deletions(-)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index 1b972e95fe36..c963832d9824 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -667,6 +667,8 @@ static void free_response_callback(struct fw_packet *packet,
 {
 	struct fw_request *request = container_of(packet, struct fw_request, response);
 
+	trace_async_response_outbound_complete(card, request, packet);
+
 	// Decrease the reference count since not at in-flight.
 	fw_request_put(request);
 
@@ -849,6 +851,9 @@ static struct fw_request *allocate_request(struct fw_card *card,
 void fw_send_response(struct fw_card *card,
 		      struct fw_request *request, int rcode)
 {
+	u32 *data = NULL;
+	unsigned int data_length = 0;
+
 	/* unified transaction or broadcast transaction: don't respond */
 	if (request->ack != ACK_PENDING ||
 	    HEADER_DESTINATION_IS_BROADCAST(request->request_header)) {
@@ -856,17 +861,19 @@ void fw_send_response(struct fw_card *card,
 		return;
 	}
 
-	if (rcode == RCODE_COMPLETE)
-		fw_fill_response(&request->response, request->request_header,
-				 rcode, request->data,
-				 fw_get_response_length(request));
-	else
-		fw_fill_response(&request->response, request->request_header,
-				 rcode, NULL, 0);
+	if (rcode == RCODE_COMPLETE) {
+		data = request->data;
+		data_length = fw_get_response_length(request);
+	}
+
+	fw_fill_response(&request->response, request->request_header, rcode, data, data_length);
 
 	// Increase the reference count so that the object is kept during in-flight.
 	fw_request_get(request);
 
+	trace_async_response_outbound_initiate(card, request, &request->response, data,
+					       data ? data_length / 4 : 0);
+
 	card->driver->send_response(card, &request->response);
 }
 EXPORT_SYMBOL(fw_send_response);
diff --git a/drivers/firewire/trace.h b/drivers/firewire/trace.h
index ba09eb720933..0109a70d3b02 100644
--- a/drivers/firewire/trace.h
+++ b/drivers/firewire/trace.h
@@ -190,6 +190,74 @@ TRACE_EVENT(async_request_inbound,
 	)
 )
 
+TRACE_EVENT(async_response_outbound_initiate,
+	TP_PROTO(const struct fw_card *card, const struct fw_request *request,
+		 const struct fw_packet *packet, const u32 *data, unsigned int data_count),
+	TP_ARGS(card, request, 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(u8, rcode)
+		__dynamic_array(u32, data, data_count)
+	),
+	TP_fast_assign(
+		__entry->transaction = (u64)request;
+		__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->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 generation=%u 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->generation,
+		__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))
+	)
+)
+
+TRACE_EVENT(async_response_outbound_complete,
+	TP_PROTO(const struct fw_card *card, const struct fw_request *request,
+		 const struct fw_packet *packet),
+	TP_ARGS(card, request, packet),
+	TP_STRUCT__entry(
+		__field(u64, transaction)
+		__field(u8, ack)
+		__field(u16, timestamp)
+	),
+	TP_fast_assign(
+		__entry->transaction = (u64)request;
+		__entry->ack = packet->ack;
+		__entry->timestamp = packet->timestamp;
+	),
+	TP_printk(
+		"transaction=0x%llx ack=%u timestamp=0x%04x",
+		__entry->transaction,
+		__entry->ack,
+		__entry->timestamp
+	)
+)
+
 #endif // _FIREWIRE_TRACE_EVENT_H
 
 #define TRACE_INCLUDE_PATH	.
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2024-04-18  9:23 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-18  9:22 [RFC PATCH 00/13] firewire: add tracepoints events for asynchronous communication Takashi Sakamoto
2024-04-18  9:22 ` [RFC PATCH 01/13] firewire: core: add common inline functions to serialize/deserialize asynchronous packet header Takashi Sakamoto
2024-04-18  9:22   ` [RFC PATCH 02/13] firewire: core: replace local macros with common inline functions for " Takashi Sakamoto
2024-04-18  9:22     ` [RFC PATCH 03/13] firewire: ohci: " Takashi Sakamoto
2024-04-18  9:22       ` [RFC PATCH 04/13] firewire: ohci: replace hard-coded values with " Takashi Sakamoto
2024-04-18  9:22         ` [RFC PATCH 05/13] firewire: ohci: replace hard-coded values with common macros Takashi Sakamoto
2024-04-18  9:22           ` [RFC PATCH 06/13] firewire: core: obsolete tcode check macros with inline functions Takashi Sakamoto
2024-04-18  9:22             ` [RFC PATCH 07/13] firewire: core: add common macro to serialize/deserialize isochronous packet header Takashi Sakamoto
2024-04-18  9:22               ` [RFC PATCH 08/13] firewire: core: replace local macros with common inline functions for " Takashi Sakamoto
2024-04-18  9:22                 ` [RFC PATCH 09/13] firewire: core: add support for Linux kernel tracepoints Takashi Sakamoto
2024-04-18  9:23                   ` [RFC PATCH 10/13] firewire: core: add tracepoints events for asynchronous outbound request Takashi Sakamoto
2024-04-18  9:23                     ` [RFC PATCH 11/13] firewire: core: add tracepoints event for asynchronous inbound response Takashi Sakamoto
2024-04-18  9:23                       ` [RFC PATCH 12/13] firewire: core: add tracepoint event for asynchronous inbound request Takashi Sakamoto
2024-04-18  9:23                         ` [RFC PATCH 13/13] firewire: core: add tracepoints events for asynchronous outbound response Takashi Sakamoto

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.