[PATCH 0/4] firewire: ohci: add helper functions for data format specific to 1394 OHCI AT/IT DMA

[PATCH 0/4] firewire: ohci: add helper functions for data format specific to 1394 OHCI AT/IT DMA

[Takashi Sakamoto]

Hi,

In 1394 OHCI specification, the data formats of AT/IT DMA are specific
and different from packert format of IEEE 1394 specification. Current
implementation includes some ad-hoc lines to construct the data, while
it is not so easy to read and understand.

This patchset includes some helper functions to serialize and
deserialize the data. These helper functions are tested by KUnit, and
replace the ad-hoc implementations.

Takashi Sakamoto (4):
  firewire: ohci: add static inline functions to serialize/deserialize
    data of AT DMA
  firewire: ohci: use static inline functions to serialize data of AT
    DMA
  firewire: ohci: add static inline functions to serialize/deserialize
    data of IT DMA
  firewire: ohci: use static inline functions to serialize data of IT
    DMA

 drivers/firewire/ohci-serdes-test.c |  66 +++++++++
 drivers/firewire/ohci.c             |  66 ++++-----
 drivers/firewire/ohci.h             | 199 ++++++++++++++++++++++++++++
 3 files changed, 299 insertions(+), 32 deletions(-)

-- 
2.43.0

[PATCH 1/4] firewire: ohci: add static inline functions to serialize/deserialize data of AT DMA

[Takashi Sakamoto]

In 1394 OHCI specification, the format of data for AT DMA is different from
the format of asynchronous packet in IEEE 1394 specification, in its spd
and srcBusID fields.

This commit adds some static inline functions to serialize/deserialize the
data of AT DMA.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/ohci-serdes-test.c |  34 ++++++++
 drivers/firewire/ohci.h             | 115 ++++++++++++++++++++++++++++
 2 files changed, 149 insertions(+)

diff --git a/drivers/firewire/ohci-serdes-test.c b/drivers/firewire/ohci-serdes-test.c
index 304a09ff528e..c6820f4f7ec1 100644
--- a/drivers/firewire/ohci-serdes-test.c
+++ b/drivers/firewire/ohci-serdes-test.c
@@ -40,9 +40,43 @@ static void test_self_id_receive_buffer_deserialization(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, 0xf38b, timestamp);
 }
 
+static void test_at_data_serdes(struct kunit *test)
+{
+	static const __le32 expected[] = {
+		cpu_to_le32(0x00020e80),
+		cpu_to_le32(0xffc2ffff),
+		cpu_to_le32(0xe0000000),
+	};
+	__le32 quadlets[] = {0, 0, 0};
+	bool has_src_bus_id = ohci1394_at_data_get_src_bus_id(expected);
+	unsigned int speed = ohci1394_at_data_get_speed(expected);
+	unsigned int tlabel = ohci1394_at_data_get_tlabel(expected);
+	unsigned int retry = ohci1394_at_data_get_retry(expected);
+	unsigned int tcode = ohci1394_at_data_get_tcode(expected);
+	unsigned int destination_id = ohci1394_at_data_get_destination_id(expected);
+	u64 destination_offset = ohci1394_at_data_get_destination_offset(expected);
+
+	KUNIT_EXPECT_FALSE(test, has_src_bus_id);
+	KUNIT_EXPECT_EQ(test, 0x02, speed);
+	KUNIT_EXPECT_EQ(test, 0x03, tlabel);
+	KUNIT_EXPECT_EQ(test, 0x02, retry);
+	KUNIT_EXPECT_EQ(test, 0x08, tcode);
+
+	ohci1394_at_data_set_src_bus_id(quadlets, has_src_bus_id);
+	ohci1394_at_data_set_speed(quadlets, speed);
+	ohci1394_at_data_set_tlabel(quadlets, tlabel);
+	ohci1394_at_data_set_retry(quadlets, retry);
+	ohci1394_at_data_set_tcode(quadlets, tcode);
+	ohci1394_at_data_set_destination_id(quadlets, destination_id);
+	ohci1394_at_data_set_destination_offset(quadlets, destination_offset);
+
+	KUNIT_EXPECT_MEMEQ(test, quadlets, expected, sizeof(expected));
+}
+
 static struct kunit_case ohci_serdes_test_cases[] = {
 	KUNIT_CASE(test_self_id_count_register_deserialization),
 	KUNIT_CASE(test_self_id_receive_buffer_deserialization),
+	KUNIT_CASE(test_at_data_serdes),
 	{}
 };
 
diff --git a/drivers/firewire/ohci.h b/drivers/firewire/ohci.h
index 9ed36cfc6cae..a5501996137c 100644
--- a/drivers/firewire/ohci.h
+++ b/drivers/firewire/ohci.h
@@ -154,6 +154,121 @@
 #define OHCI1394_evt_flushed		0xf
 
 
+// Asynchronous Transmit DMA.
+//
+// The content of first two quadlets of data for AT DMA is different from the header for IEEE 1394
+// asynchronous packet.
+
+#define OHCI1394_AT_DATA_Q0_srcBusID_MASK		0x00800000
+#define OHCI1394_AT_DATA_Q0_srcBusID_SHIFT		23
+#define OHCI1394_AT_DATA_Q0_spd_MASK			0x00070000
+#define OHCI1394_AT_DATA_Q0_spd_SHIFT			16
+#define OHCI1394_AT_DATA_Q0_tLabel_MASK			0x0000fc00
+#define OHCI1394_AT_DATA_Q0_tLabel_SHIFT		10
+#define OHCI1394_AT_DATA_Q0_rt_MASK			0x00000300
+#define OHCI1394_AT_DATA_Q0_rt_SHIFT			8
+#define OHCI1394_AT_DATA_Q0_tCode_MASK			0x000000f0
+#define OHCI1394_AT_DATA_Q0_tCode_SHIFT			4
+#define OHCI1394_AT_DATA_Q1_destinationId_MASK		0xffff0000
+#define OHCI1394_AT_DATA_Q1_destinationId_SHIFT		16
+#define OHCI1394_AT_DATA_Q1_destinationOffsetHigh_MASK	0x0000ffff
+#define OHCI1394_AT_DATA_Q1_destinationOffsetHigh_SHIFT	0
+#define OHCI1394_AT_DATA_Q1_rCode_MASK			0x0000f000
+#define OHCI1394_AT_DATA_Q1_rCode_SHIFT			12
+
+static inline bool ohci1394_at_data_get_src_bus_id(const __le32 *data)
+{
+	return !!((data[0] & OHCI1394_AT_DATA_Q0_srcBusID_MASK) >> OHCI1394_AT_DATA_Q0_srcBusID_SHIFT);
+}
+
+static inline void ohci1394_at_data_set_src_bus_id(__le32 *data, bool src_bus_id)
+{
+	data[0] &= cpu_to_le32(~OHCI1394_AT_DATA_Q0_srcBusID_MASK);
+	data[0] |= cpu_to_le32((src_bus_id << OHCI1394_AT_DATA_Q0_srcBusID_SHIFT) & OHCI1394_AT_DATA_Q0_srcBusID_MASK);
+}
+
+static inline unsigned int ohci1394_at_data_get_speed(const __le32 *data)
+{
+	return (le32_to_cpu(data[0]) & OHCI1394_AT_DATA_Q0_spd_MASK) >> OHCI1394_AT_DATA_Q0_spd_SHIFT;
+}
+
+static inline void ohci1394_at_data_set_speed(__le32 *data, unsigned int scode)
+{
+	data[0] &= cpu_to_le32(~OHCI1394_AT_DATA_Q0_spd_MASK);
+	data[0] |= cpu_to_le32((scode << OHCI1394_AT_DATA_Q0_spd_SHIFT) & OHCI1394_AT_DATA_Q0_spd_MASK);
+}
+
+static inline unsigned int ohci1394_at_data_get_tlabel(const __le32 *data)
+{
+	return (le32_to_cpu(data[0]) & OHCI1394_AT_DATA_Q0_tLabel_MASK) >> OHCI1394_AT_DATA_Q0_tLabel_SHIFT;
+}
+
+static inline void ohci1394_at_data_set_tlabel(__le32 *data, unsigned int tlabel)
+{
+	data[0] &= cpu_to_le32(~OHCI1394_AT_DATA_Q0_tLabel_MASK);
+	data[0] |= cpu_to_le32((tlabel << OHCI1394_AT_DATA_Q0_tLabel_SHIFT) & OHCI1394_AT_DATA_Q0_tLabel_MASK);
+}
+
+static inline unsigned int ohci1394_at_data_get_retry(const __le32 *data)
+{
+	return (le32_to_cpu(data[0]) & OHCI1394_AT_DATA_Q0_rt_MASK) >> OHCI1394_AT_DATA_Q0_rt_SHIFT;
+}
+
+static inline void ohci1394_at_data_set_retry(__le32 *data, unsigned int retry)
+{
+	data[0] &= cpu_to_le32(~OHCI1394_AT_DATA_Q0_rt_MASK);
+	data[0] |= cpu_to_le32((retry << OHCI1394_AT_DATA_Q0_rt_SHIFT) & OHCI1394_AT_DATA_Q0_rt_MASK);
+}
+
+static inline unsigned int ohci1394_at_data_get_tcode(const __le32 *data)
+{
+	return (le32_to_cpu(data[0]) & OHCI1394_AT_DATA_Q0_tCode_MASK) >> OHCI1394_AT_DATA_Q0_tCode_SHIFT;
+}
+
+static inline void ohci1394_at_data_set_tcode(__le32 *data, unsigned int tcode)
+{
+	data[0] &= cpu_to_le32(~OHCI1394_AT_DATA_Q0_tCode_MASK);
+	data[0] |= cpu_to_le32((tcode << OHCI1394_AT_DATA_Q0_tCode_SHIFT) & OHCI1394_AT_DATA_Q0_tCode_MASK);
+}
+
+static inline unsigned int ohci1394_at_data_get_destination_id(const __le32 *data)
+{
+	return (le32_to_cpu(data[1]) & OHCI1394_AT_DATA_Q1_destinationId_MASK) >> OHCI1394_AT_DATA_Q1_destinationId_SHIFT;
+}
+
+static inline void ohci1394_at_data_set_destination_id(__le32 *data, unsigned int destination_id)
+{
+	data[1] &= cpu_to_le32(~OHCI1394_AT_DATA_Q1_destinationId_MASK);
+	data[1] |= cpu_to_le32((destination_id << OHCI1394_AT_DATA_Q1_destinationId_SHIFT) & OHCI1394_AT_DATA_Q1_destinationId_MASK);
+}
+
+static inline u64 ohci1394_at_data_get_destination_offset(const __le32 *data)
+{
+	u64 hi = (u64)((le32_to_cpu(data[1]) & OHCI1394_AT_DATA_Q1_destinationOffsetHigh_MASK) >> OHCI1394_AT_DATA_Q1_destinationOffsetHigh_SHIFT);
+	u64 lo = (u64)le32_to_cpu(data[2]);
+	return (hi << 32) | lo;
+}
+
+static inline void ohci1394_at_data_set_destination_offset(__le32 *data, u64 offset)
+{
+	u32 hi = (u32)(offset >> 32);
+	u32 lo = (u32)(offset & 0x00000000ffffffff);
+	data[1] &= cpu_to_le32(~OHCI1394_AT_DATA_Q1_destinationOffsetHigh_MASK);
+	data[1] |= cpu_to_le32((hi << OHCI1394_AT_DATA_Q1_destinationOffsetHigh_SHIFT) & OHCI1394_AT_DATA_Q1_destinationOffsetHigh_MASK);
+	data[2] = cpu_to_le32(lo);
+}
+
+static inline unsigned int ohci1394_at_data_get_rcode(const __le32 *data)
+{
+	return (le32_to_cpu(data[1]) & OHCI1394_AT_DATA_Q1_rCode_MASK) >> OHCI1394_AT_DATA_Q1_rCode_SHIFT;
+}
+
+static inline void ohci1394_at_data_set_rcode(__le32 *data, unsigned int rcode)
+{
+	data[1] &= cpu_to_le32(~OHCI1394_AT_DATA_Q1_rCode_MASK);
+	data[1] |= cpu_to_le32((rcode << OHCI1394_AT_DATA_Q1_rCode_SHIFT) & OHCI1394_AT_DATA_Q1_rCode_MASK);
+}
+
 // Self-ID DMA.
 
 #define OHCI1394_SelfIDCount_selfIDError_MASK		0x80000000
-- 
2.43.0

[PATCH 2/4] firewire: ohci: use static inline functions to serialize data of AT DMA

[Takashi Sakamoto]

This commit replaces current implementation with the helper functions added
in the former commit.

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

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index a0bb0e87e18a..e8429dbbc60d 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -1409,12 +1409,6 @@ static int at_context_queue_packet(struct context *ctx,
 	d[0].control   = cpu_to_le16(DESCRIPTOR_KEY_IMMEDIATE);
 	d[0].res_count = cpu_to_le16(packet->timestamp);
 
-	/*
-	 * The DMA format for asynchronous link packets is different
-	 * from the IEEE1394 layout, so shift the fields around
-	 * accordingly.
-	 */
-
 	tcode = async_header_get_tcode(packet->header);
 	header = (__le32 *) &d[1];
 	switch (tcode) {
@@ -1427,11 +1421,21 @@ static int at_context_queue_packet(struct context *ctx,
 	case TCODE_READ_BLOCK_RESPONSE:
 	case TCODE_LOCK_REQUEST:
 	case TCODE_LOCK_RESPONSE:
-		header[0] = cpu_to_le32((packet->header[0] & 0xffff) |
-					(packet->speed << 16));
-		header[1] = cpu_to_le32((packet->header[1] & 0xffff) |
-					(packet->header[0] & 0xffff0000));
-		header[2] = cpu_to_le32(packet->header[2]);
+		ohci1394_at_data_set_src_bus_id(header, false);
+		ohci1394_at_data_set_speed(header, packet->speed);
+		ohci1394_at_data_set_tlabel(header, async_header_get_tlabel(packet->header));
+		ohci1394_at_data_set_retry(header, async_header_get_retry(packet->header));
+		ohci1394_at_data_set_tcode(header, tcode);
+
+		ohci1394_at_data_set_destination_id(header,
+						    async_header_get_destination(packet->header));
+
+		if (ctx == &ctx->ohci->at_response_ctx) {
+			ohci1394_at_data_set_rcode(header, async_header_get_rcode(packet->header));
+		} else {
+			ohci1394_at_data_set_destination_offset(header,
+							async_header_get_offset(packet->header));
+		}
 
 		if (tcode_is_block_packet(tcode))
 			header[3] = cpu_to_le32(packet->header[3]);
@@ -1440,10 +1444,10 @@ static int at_context_queue_packet(struct context *ctx,
 
 		d[0].req_count = cpu_to_le16(packet->header_length);
 		break;
-
 	case TCODE_LINK_INTERNAL:
-		header[0] = cpu_to_le32((TCODE_LINK_INTERNAL << 4) |
-					(packet->speed << 16));
+		ohci1394_at_data_set_speed(header, packet->speed);
+		ohci1394_at_data_set_tcode(header, TCODE_LINK_INTERNAL);
+
 		header[1] = cpu_to_le32(packet->header[1]);
 		header[2] = cpu_to_le32(packet->header[2]);
 		d[0].req_count = cpu_to_le16(12);
-- 
2.43.0

[PATCH 3/4] firewire: ohci: add static inline functions to serialize/deserialize data of IT DMA

[Takashi Sakamoto]

In 1394 OHCI specification, the format of data for IT DMA is different from
the format of isochronous packet in IEEE 1394 specification, in its spd and
srcBusID fields.

This commit adds some static inline functions to serialize/deserialize the
data of IT DMA.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/ohci-serdes-test.c | 32 +++++++++++
 drivers/firewire/ohci.h             | 84 +++++++++++++++++++++++++++++
 2 files changed, 116 insertions(+)

diff --git a/drivers/firewire/ohci-serdes-test.c b/drivers/firewire/ohci-serdes-test.c
index c6820f4f7ec1..ab25ec28aeb5 100644
--- a/drivers/firewire/ohci-serdes-test.c
+++ b/drivers/firewire/ohci-serdes-test.c
@@ -73,10 +73,42 @@ static void test_at_data_serdes(struct kunit *test)
 	KUNIT_EXPECT_MEMEQ(test, quadlets, expected, sizeof(expected));
 }
 
+static void test_it_data_serdes(struct kunit *test)
+{
+	static const __le32 expected[] = {
+		cpu_to_le32(0x000349a7),
+		cpu_to_le32(0x02300000),
+	};
+	__le32 quadlets[] = {0, 0};
+	unsigned int scode = ohci1394_it_data_get_speed(expected);
+	unsigned int tag = ohci1394_it_data_get_tag(expected);
+	unsigned int channel = ohci1394_it_data_get_channel(expected);
+	unsigned int tcode = ohci1394_it_data_get_tcode(expected);
+	unsigned int sync = ohci1394_it_data_get_sync(expected);
+	unsigned int data_length = ohci1394_it_data_get_data_length(expected);
+
+	KUNIT_EXPECT_EQ(test, 0x03, scode);
+	KUNIT_EXPECT_EQ(test, 0x01, tag);
+	KUNIT_EXPECT_EQ(test, 0x09, channel);
+	KUNIT_EXPECT_EQ(test, 0x0a, tcode);
+	KUNIT_EXPECT_EQ(test, 0x7, sync);
+	KUNIT_EXPECT_EQ(test, 0x0230, data_length);
+
+	ohci1394_it_data_set_speed(quadlets, scode);
+	ohci1394_it_data_set_tag(quadlets, tag);
+	ohci1394_it_data_set_channel(quadlets, channel);
+	ohci1394_it_data_set_tcode(quadlets, tcode);
+	ohci1394_it_data_set_sync(quadlets, sync);
+	ohci1394_it_data_set_data_length(quadlets, data_length);
+
+	KUNIT_EXPECT_MEMEQ(test, quadlets, expected, sizeof(expected));
+}
+
 static struct kunit_case ohci_serdes_test_cases[] = {
 	KUNIT_CASE(test_self_id_count_register_deserialization),
 	KUNIT_CASE(test_self_id_receive_buffer_deserialization),
 	KUNIT_CASE(test_at_data_serdes),
+	KUNIT_CASE(test_it_data_serdes),
 	{}
 };
 
diff --git a/drivers/firewire/ohci.h b/drivers/firewire/ohci.h
index a5501996137c..218666cfe14a 100644
--- a/drivers/firewire/ohci.h
+++ b/drivers/firewire/ohci.h
@@ -269,6 +269,90 @@ static inline void ohci1394_at_data_set_rcode(__le32 *data, unsigned int rcode)
 	data[1] |= cpu_to_le32((rcode << OHCI1394_AT_DATA_Q1_rCode_SHIFT) & OHCI1394_AT_DATA_Q1_rCode_MASK);
 }
 
+// Isochronous Transmit DMA.
+//
+// The content of first two quadlets of data for IT DMA is different from the header for IEEE 1394
+// isochronous packet.
+
+#define OHCI1394_IT_DATA_Q0_spd_MASK		0x00070000
+#define OHCI1394_IT_DATA_Q0_spd_SHIFT		16
+#define OHCI1394_IT_DATA_Q0_tag_MASK		0x0000c000
+#define OHCI1394_IT_DATA_Q0_tag_SHIFT		14
+#define OHCI1394_IT_DATA_Q0_chanNum_MASK	0x00003f00
+#define OHCI1394_IT_DATA_Q0_chanNum_SHIFT	8
+#define OHCI1394_IT_DATA_Q0_tcode_MASK		0x000000f0
+#define OHCI1394_IT_DATA_Q0_tcode_SHIFT		4
+#define OHCI1394_IT_DATA_Q0_sy_MASK		0x0000000f
+#define OHCI1394_IT_DATA_Q0_sy_SHIFT		0
+#define OHCI1394_IT_DATA_Q1_dataLength_MASK	0xffff0000
+#define OHCI1394_IT_DATA_Q1_dataLength_SHIFT	16
+
+static inline unsigned int ohci1394_it_data_get_speed(const __le32 *data)
+{
+	return (le32_to_cpu(data[0]) & OHCI1394_IT_DATA_Q0_spd_MASK) >> OHCI1394_IT_DATA_Q0_spd_SHIFT;
+}
+
+static inline void ohci1394_it_data_set_speed(__le32 *data, unsigned int scode)
+{
+	data[0] &= cpu_to_le32(~OHCI1394_IT_DATA_Q0_spd_MASK);
+	data[0] |= cpu_to_le32((scode << OHCI1394_IT_DATA_Q0_spd_SHIFT) & OHCI1394_IT_DATA_Q0_spd_MASK);
+}
+
+static inline unsigned int ohci1394_it_data_get_tag(const __le32 *data)
+{
+	return (le32_to_cpu(data[0]) & OHCI1394_IT_DATA_Q0_tag_MASK) >> OHCI1394_IT_DATA_Q0_tag_SHIFT;
+}
+
+static inline void ohci1394_it_data_set_tag(__le32 *data, unsigned int tag)
+{
+	data[0] &= cpu_to_le32(~OHCI1394_IT_DATA_Q0_tag_MASK);
+	data[0] |= cpu_to_le32((tag << OHCI1394_IT_DATA_Q0_tag_SHIFT) & OHCI1394_IT_DATA_Q0_tag_MASK);
+}
+
+static inline unsigned int ohci1394_it_data_get_channel(const __le32 *data)
+{
+	return (le32_to_cpu(data[0]) & OHCI1394_IT_DATA_Q0_chanNum_MASK) >> OHCI1394_IT_DATA_Q0_chanNum_SHIFT;
+}
+
+static inline void ohci1394_it_data_set_channel(__le32 *data, unsigned int channel)
+{
+	data[0] &= cpu_to_le32(~OHCI1394_IT_DATA_Q0_chanNum_MASK);
+	data[0] |= cpu_to_le32((channel << OHCI1394_IT_DATA_Q0_chanNum_SHIFT) & OHCI1394_IT_DATA_Q0_chanNum_MASK);
+}
+
+static inline unsigned int ohci1394_it_data_get_tcode(const __le32 *data)
+{
+	return (le32_to_cpu(data[0]) & OHCI1394_IT_DATA_Q0_tcode_MASK) >> OHCI1394_IT_DATA_Q0_tcode_SHIFT;
+}
+
+static inline void ohci1394_it_data_set_tcode(__le32 *data, unsigned int tcode)
+{
+	data[0] &= cpu_to_le32(~OHCI1394_IT_DATA_Q0_tcode_MASK);
+	data[0] |= cpu_to_le32((tcode << OHCI1394_IT_DATA_Q0_tcode_SHIFT) & OHCI1394_IT_DATA_Q0_tcode_MASK);
+}
+
+static inline unsigned int ohci1394_it_data_get_sync(const __le32 *data)
+{
+	return (le32_to_cpu(data[0]) & OHCI1394_IT_DATA_Q0_sy_MASK) >> OHCI1394_IT_DATA_Q0_sy_SHIFT;
+}
+
+static inline void ohci1394_it_data_set_sync(__le32 *data, unsigned int sync)
+{
+	data[0] &= cpu_to_le32(~OHCI1394_IT_DATA_Q0_sy_MASK);
+	data[0] |= cpu_to_le32((sync << OHCI1394_IT_DATA_Q0_sy_SHIFT) & OHCI1394_IT_DATA_Q0_sy_MASK);
+}
+
+static inline unsigned int ohci1394_it_data_get_data_length(const __le32 *data)
+{
+	return (le32_to_cpu(data[1]) & OHCI1394_IT_DATA_Q1_dataLength_MASK) >> OHCI1394_IT_DATA_Q1_dataLength_SHIFT;
+}
+
+static inline void ohci1394_it_data_set_data_length(__le32 *data, unsigned int data_length)
+{
+	data[1] &= cpu_to_le32(~OHCI1394_IT_DATA_Q1_dataLength_MASK);
+	data[1] |= cpu_to_le32((data_length << OHCI1394_IT_DATA_Q1_dataLength_SHIFT) & OHCI1394_IT_DATA_Q1_dataLength_MASK);
+}
+
 // Self-ID DMA.
 
 #define OHCI1394_SelfIDCount_selfIDError_MASK		0x80000000
-- 
2.43.0

[PATCH 4/4] firewire: ohci: use static inline functions to serialize data of IT DMA

[Takashi Sakamoto]

THis commit replaces current implementation with the helper functions added
in the former commit.

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

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index e8429dbbc60d..8f2bbd0569fb 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -162,13 +162,6 @@ struct context {
 	struct tasklet_struct tasklet;
 };
 
-#define IT_HEADER_SY(v)          ((v) <<  0)
-#define IT_HEADER_TCODE(v)       ((v) <<  4)
-#define IT_HEADER_CHANNEL(v)     ((v) <<  8)
-#define IT_HEADER_TAG(v)         ((v) << 14)
-#define IT_HEADER_SPEED(v)       ((v) << 16)
-#define IT_HEADER_DATA_LENGTH(v) ((v) << 16)
-
 struct iso_context {
 	struct fw_iso_context base;
 	struct context context;
@@ -1457,9 +1450,14 @@ static int at_context_queue_packet(struct context *ctx,
 		break;
 
 	case TCODE_STREAM_DATA:
-		header[0] = cpu_to_le32((packet->header[0] & 0xffff) |
-					(packet->speed << 16));
-		header[1] = cpu_to_le32(packet->header[0] & 0xffff0000);
+		ohci1394_it_data_set_speed(header, packet->speed);
+		ohci1394_it_data_set_tag(header, isoc_header_get_tag(packet->header[0]));
+		ohci1394_it_data_set_channel(header, isoc_header_get_channel(packet->header[0]));
+		ohci1394_it_data_set_tcode(header, TCODE_STREAM_DATA);
+		ohci1394_it_data_set_sync(header, isoc_header_get_sy(packet->header[0]));
+
+		ohci1394_it_data_set_data_length(header, isoc_header_get_data_length(packet->header[0]));
+
 		d[0].req_count = cpu_to_le16(8);
 		break;
 
@@ -3403,14 +3401,14 @@ static int queue_iso_transmit(struct iso_context *ctx,
 		d[0].branch_address = cpu_to_le32(d_bus | z);
 
 		header = (__le32 *) &d[1];
-		header[0] = cpu_to_le32(IT_HEADER_SY(p->sy) |
-					IT_HEADER_TAG(p->tag) |
-					IT_HEADER_TCODE(TCODE_STREAM_DATA) |
-					IT_HEADER_CHANNEL(ctx->base.channel) |
-					IT_HEADER_SPEED(ctx->base.speed));
-		header[1] =
-			cpu_to_le32(IT_HEADER_DATA_LENGTH(p->header_length +
-							  p->payload_length));
+
+		ohci1394_it_data_set_speed(header, ctx->base.speed);
+		ohci1394_it_data_set_tag(header, p->tag);
+		ohci1394_it_data_set_channel(header, ctx->base.channel);
+		ohci1394_it_data_set_tcode(header, TCODE_STREAM_DATA);
+		ohci1394_it_data_set_sync(header, p->sy);
+
+		ohci1394_it_data_set_data_length(header, p->header_length + p->payload_length);
 	}
 
 	if (p->header_length > 0) {
-- 
2.43.0

Re: [PATCH 0/4] firewire: ohci: add helper functions for data format specific to 1394 OHCI AT/IT DMA

[Takashi Sakamoto]

On Fri, Aug 02, 2024 at 09:36:02AM +0900, Takashi Sakamoto wrote:
> Hi,
> 
> In 1394 OHCI specification, the data formats of AT/IT DMA are specific
> and different from packert format of IEEE 1394 specification. Current
> implementation includes some ad-hoc lines to construct the data, while
> it is not so easy to read and understand.
> 
> This patchset includes some helper functions to serialize and
> deserialize the data. These helper functions are tested by KUnit, and
> replace the ad-hoc implementations.
> 
> Takashi Sakamoto (4):
>   firewire: ohci: add static inline functions to serialize/deserialize
>     data of AT DMA
>   firewire: ohci: use static inline functions to serialize data of AT
>     DMA
>   firewire: ohci: add static inline functions to serialize/deserialize
>     data of IT DMA
>   firewire: ohci: use static inline functions to serialize data of IT
>     DMA
> 
>  drivers/firewire/ohci-serdes-test.c |  66 +++++++++
>  drivers/firewire/ohci.c             |  66 ++++-----
>  drivers/firewire/ohci.h             | 199 ++++++++++++++++++++++++++++
>  3 files changed, 299 insertions(+), 32 deletions(-)

Applied to for-next branch.


Regards

Takashi Sakamoto