From 7bd10e0e3a74f84eba4122bfc63a5896268752b8 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Thu, 6 Jun 2024 08:51:45 +0900 Subject: [PATCH 01/32] firewire: core: add enumerator of self ID sequences and its KUnit test When the state of bus reset finishes, 1394 OHCI driver constructs self ID sequences, then it calls fw_core_handle_bus_reset() in core function. The core function enumerates the self ID sequences to build bus topology. This commit adds a structure and some helper functions for the enumeration, and adds a KUnit test suite to ensure its expected behaviour. Link: https://lore.kernel.org/r/20240605235155.116468-2-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/.kunitconfig | 1 + drivers/firewire/Kconfig | 15 ++++ drivers/firewire/Makefile | 1 + drivers/firewire/phy-packet-definitions.h | 78 ++++++++++++++++++ .../firewire/self-id-sequence-helper-test.c | 79 +++++++++++++++++++ 5 files changed, 174 insertions(+) create mode 100644 drivers/firewire/phy-packet-definitions.h create mode 100644 drivers/firewire/self-id-sequence-helper-test.c diff --git a/drivers/firewire/.kunitconfig b/drivers/firewire/.kunitconfig index 60d9e7c35417..74259204fcdd 100644 --- a/drivers/firewire/.kunitconfig +++ b/drivers/firewire/.kunitconfig @@ -4,3 +4,4 @@ CONFIG_FIREWIRE=y CONFIG_FIREWIRE_KUNIT_UAPI_TEST=y CONFIG_FIREWIRE_KUNIT_DEVICE_ATTRIBUTE_TEST=y CONFIG_FIREWIRE_KUNIT_PACKET_SERDES_TEST=y +CONFIG_FIREWIRE_KUNIT_SELF_ID_SEQUENCE_HELPER_TEST=y diff --git a/drivers/firewire/Kconfig b/drivers/firewire/Kconfig index 5268b3f0a25a..95e72e0b592b 100644 --- a/drivers/firewire/Kconfig +++ b/drivers/firewire/Kconfig @@ -66,6 +66,21 @@ config FIREWIRE_KUNIT_PACKET_SERDES_TEST For more information on KUnit and unit tests in general, refer to the KUnit documentation in Documentation/dev-tools/kunit/. +config FIREWIRE_KUNIT_SELF_ID_SEQUENCE_HELPER_TEST + tristate "KUnit tests for helpers of self ID sequence" if !KUNIT_ALL_TESTS + depends on FIREWIRE && KUNIT + default KUNIT_ALL_TESTS + help + This builds the KUnit tests for helpers of self ID sequence. + + 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 75c47d046925..21b975e0a387 100644 --- a/drivers/firewire/Makefile +++ b/drivers/firewire/Makefile @@ -18,3 +18,4 @@ obj-$(CONFIG_PROVIDE_OHCI1394_DMA_INIT) += init_ohci1394_dma.o obj-$(CONFIG_FIREWIRE_KUNIT_UAPI_TEST) += uapi-test.o obj-$(CONFIG_FIREWIRE_KUNIT_PACKET_SERDES_TEST) += packet-serdes-test.o +obj-$(CONFIG_FIREWIRE_KUNIT_SELF_ID_SEQUENCE_HELPER_TEST) += self-id-sequence-helper-test.o diff --git a/drivers/firewire/phy-packet-definitions.h b/drivers/firewire/phy-packet-definitions.h new file mode 100644 index 000000000000..479bb3431afb --- /dev/null +++ b/drivers/firewire/phy-packet-definitions.h @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +// +// phy-packet-definitions.h - The definitions of phy packet for IEEE 1394. +// +// Copyright (c) 2024 Takashi Sakamoto + +#ifndef _FIREWIRE_PHY_PACKET_DEFINITIONS_H +#define _FIREWIRE_PHY_PACKET_DEFINITIONS_H + +#define SELF_ID_EXTENDED_MASK 0x00800000 +#define SELF_ID_EXTENDED_SHIFT 23 +#define SELF_ID_MORE_PACKETS_MASK 0x00000001 +#define SELF_ID_MORE_PACKETS_SHIFT 0 + +#define SELF_ID_EXTENDED_SEQUENCE_MASK 0x00700000 +#define SELF_ID_EXTENDED_SEQUENCE_SHIFT 20 + +#define SELF_ID_SEQUENCE_MAXIMUM_QUADLET_COUNT 4 + +static inline bool phy_packet_self_id_get_extended(u32 quadlet) +{ + return (quadlet & SELF_ID_EXTENDED_MASK) >> SELF_ID_EXTENDED_SHIFT; +} + +static inline bool phy_packet_self_id_get_more_packets(u32 quadlet) +{ + return (quadlet & SELF_ID_MORE_PACKETS_MASK) >> SELF_ID_MORE_PACKETS_SHIFT; +} + +static inline unsigned int phy_packet_self_id_extended_get_sequence(u32 quadlet) +{ + return (quadlet & SELF_ID_EXTENDED_SEQUENCE_MASK) >> SELF_ID_EXTENDED_SEQUENCE_SHIFT; +} + +struct self_id_sequence_enumerator { + const u32 *cursor; + unsigned int quadlet_count; +}; + +static inline const u32 *self_id_sequence_enumerator_next( + struct self_id_sequence_enumerator *enumerator, unsigned int *quadlet_count) +{ + const u32 *self_id_sequence, *cursor; + u32 quadlet; + unsigned int count; + unsigned int sequence; + + if (enumerator->cursor == NULL || enumerator->quadlet_count == 0) + return ERR_PTR(-ENODATA); + cursor = enumerator->cursor; + count = 1; + + quadlet = *cursor; + sequence = 0; + while (phy_packet_self_id_get_more_packets(quadlet)) { + if (count >= enumerator->quadlet_count || + count >= SELF_ID_SEQUENCE_MAXIMUM_QUADLET_COUNT) + return ERR_PTR(-EPROTO); + ++cursor; + ++count; + quadlet = *cursor; + + if (!phy_packet_self_id_get_extended(quadlet) || + sequence != phy_packet_self_id_extended_get_sequence(quadlet)) + return ERR_PTR(-EPROTO); + ++sequence; + } + + *quadlet_count = count; + self_id_sequence = enumerator->cursor; + + enumerator->cursor += count; + enumerator->quadlet_count -= count; + + return self_id_sequence; +} + +#endif // _FIREWIRE_PHY_PACKET_DEFINITIONS_H diff --git a/drivers/firewire/self-id-sequence-helper-test.c b/drivers/firewire/self-id-sequence-helper-test.c new file mode 100644 index 000000000000..e8a435e20241 --- /dev/null +++ b/drivers/firewire/self-id-sequence-helper-test.c @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +// +// self-id-sequence-helper-test.c - An application of Kunit to test helpers of self ID sequence. +// +// Copyright (c) 2024 Takashi Sakamoto + +#include + +#include "phy-packet-definitions.h" + +static void test_self_id_sequence_enumerator_valid(struct kunit *test) +{ + static const u32 valid_sequences[] = { + 0x00000000, + 0x00000001, 0x00800000, + 0x00000001, 0x00800001, 0x00900000, + 0x00000000, + }; + struct self_id_sequence_enumerator enumerator; + const u32 *entry; + unsigned int quadlet_count; + + enumerator.cursor = valid_sequences; + enumerator.quadlet_count = ARRAY_SIZE(valid_sequences); + + entry = self_id_sequence_enumerator_next(&enumerator, &quadlet_count); + KUNIT_EXPECT_PTR_EQ(test, entry, &valid_sequences[0]); + KUNIT_EXPECT_EQ(test, quadlet_count, 1); + KUNIT_EXPECT_EQ(test, enumerator.quadlet_count, 6); + + entry = self_id_sequence_enumerator_next(&enumerator, &quadlet_count); + KUNIT_EXPECT_PTR_EQ(test, entry, &valid_sequences[1]); + KUNIT_EXPECT_EQ(test, quadlet_count, 2); + KUNIT_EXPECT_EQ(test, enumerator.quadlet_count, 4); + + entry = self_id_sequence_enumerator_next(&enumerator, &quadlet_count); + KUNIT_EXPECT_PTR_EQ(test, entry, &valid_sequences[3]); + KUNIT_EXPECT_EQ(test, quadlet_count, 3); + KUNIT_EXPECT_EQ(test, enumerator.quadlet_count, 1); + + entry = self_id_sequence_enumerator_next(&enumerator, &quadlet_count); + KUNIT_EXPECT_PTR_EQ(test, entry, &valid_sequences[6]); + KUNIT_EXPECT_EQ(test, quadlet_count, 1); + KUNIT_EXPECT_EQ(test, enumerator.quadlet_count, 0); + + entry = self_id_sequence_enumerator_next(&enumerator, &quadlet_count); + KUNIT_EXPECT_EQ(test, PTR_ERR(entry), -ENODATA); +} + +static void test_self_id_sequence_enumerator_invalid(struct kunit *test) +{ + static const u32 invalid_sequences[] = { + 0x00000001, + }; + struct self_id_sequence_enumerator enumerator; + const u32 *entry; + unsigned int count; + + enumerator.cursor = invalid_sequences; + enumerator.quadlet_count = ARRAY_SIZE(invalid_sequences); + + entry = self_id_sequence_enumerator_next(&enumerator, &count); + KUNIT_EXPECT_EQ(test, PTR_ERR(entry), -EPROTO); +} + +static struct kunit_case self_id_sequence_helper_test_cases[] = { + KUNIT_CASE(test_self_id_sequence_enumerator_valid), + KUNIT_CASE(test_self_id_sequence_enumerator_invalid), + {} +}; + +static struct kunit_suite self_id_sequence_helper_test_suite = { + .name = "self-id-sequence-helper", + .test_cases = self_id_sequence_helper_test_cases, +}; +kunit_test_suite(self_id_sequence_helper_test_suite); + +MODULE_DESCRIPTION("Unit test suite for helpers of self ID sequence"); +MODULE_LICENSE("GPL"); From 93389767da62142f4fa16733ddbc258d315180e0 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Thu, 6 Jun 2024 08:51:46 +0900 Subject: [PATCH 02/32] firewire: core: add helper function to handle port status from self ID sequence and its KUnit test The self ID sequence delivers the information about the state of port. This commit adds some enumerations to express the state of port, and some helper functions to handle the state. It adds a KUnit test for them, too. Link: https://lore.kernel.org/r/20240605235155.116468-3-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/phy-packet-definitions.h | 43 +++++++++++ .../firewire/self-id-sequence-helper-test.c | 73 +++++++++++++++++++ 2 files changed, 116 insertions(+) diff --git a/drivers/firewire/phy-packet-definitions.h b/drivers/firewire/phy-packet-definitions.h index 479bb3431afb..4ba8b18aa993 100644 --- a/drivers/firewire/phy-packet-definitions.h +++ b/drivers/firewire/phy-packet-definitions.h @@ -15,6 +15,8 @@ #define SELF_ID_EXTENDED_SEQUENCE_MASK 0x00700000 #define SELF_ID_EXTENDED_SEQUENCE_SHIFT 20 +#define SELF_ID_PORT_STATUS_MASK 0x3 + #define SELF_ID_SEQUENCE_MAXIMUM_QUADLET_COUNT 4 static inline bool phy_packet_self_id_get_extended(u32 quadlet) @@ -75,4 +77,45 @@ static inline const u32 *self_id_sequence_enumerator_next( return self_id_sequence; } +enum phy_packet_self_id_port_status { + PHY_PACKET_SELF_ID_PORT_STATUS_NONE = 0, + PHY_PACKET_SELF_ID_PORT_STATUS_NCONN = 1, + PHY_PACKET_SELF_ID_PORT_STATUS_PARENT = 2, + PHY_PACKET_SELF_ID_PORT_STATUS_CHILD = 3, +}; + +static inline unsigned int self_id_sequence_get_port_capacity(unsigned int quadlet_count) +{ + return quadlet_count * 8 - 5; +} + +static inline enum phy_packet_self_id_port_status self_id_sequence_get_port_status( + const u32 *self_id_sequence, unsigned int quadlet_count, unsigned int port_index) +{ + unsigned int index, shift; + + index = (port_index + 5) / 8; + shift = 16 - ((port_index + 5) % 8) * 2; + + if (index < quadlet_count && index < SELF_ID_SEQUENCE_MAXIMUM_QUADLET_COUNT) + return (self_id_sequence[index] >> shift) & SELF_ID_PORT_STATUS_MASK; + + return PHY_PACKET_SELF_ID_PORT_STATUS_NONE; +} + +static inline void self_id_sequence_set_port_status(u32 *self_id_sequence, unsigned int quadlet_count, + unsigned int port_index, + enum phy_packet_self_id_port_status status) +{ + unsigned int index, shift; + + index = (port_index + 5) / 8; + shift = 16 - ((port_index + 5) % 8) * 2; + + if (index < quadlet_count) { + self_id_sequence[index] &= ~(SELF_ID_PORT_STATUS_MASK << shift); + self_id_sequence[index] |= status << shift; + } +} + #endif // _FIREWIRE_PHY_PACKET_DEFINITIONS_H diff --git a/drivers/firewire/self-id-sequence-helper-test.c b/drivers/firewire/self-id-sequence-helper-test.c index e8a435e20241..eed7a2294e64 100644 --- a/drivers/firewire/self-id-sequence-helper-test.c +++ b/drivers/firewire/self-id-sequence-helper-test.c @@ -63,9 +63,82 @@ static void test_self_id_sequence_enumerator_invalid(struct kunit *test) KUNIT_EXPECT_EQ(test, PTR_ERR(entry), -EPROTO); } +static void test_self_id_sequence_get_port_status(struct kunit *test) +{ + static const u32 expected[] = { + 0x000000e5, + 0x00839e79, + 0x0091e79d, + 0x00a279e4, + }; + u32 quadlets [] = { + 0x00000001, + 0x00800001, + 0x00900001, + 0x00a00000, + }; + enum phy_packet_self_id_port_status port_status[28]; + unsigned int port_capacity; + unsigned int port_index; + + KUNIT_ASSERT_EQ(test, ARRAY_SIZE(expected), ARRAY_SIZE(quadlets)); + + // With an extra port. + port_capacity = self_id_sequence_get_port_capacity(ARRAY_SIZE(expected)) + 1; + KUNIT_ASSERT_EQ(test, port_capacity, ARRAY_SIZE(port_status)); + + for (port_index = 0; port_index < port_capacity; ++port_index) { + port_status[port_index] = + self_id_sequence_get_port_status(expected, ARRAY_SIZE(expected), port_index); + self_id_sequence_set_port_status(quadlets, ARRAY_SIZE(quadlets), port_index, + port_status[port_index]); + } + + // Self ID zero. + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_CHILD, port_status[0]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_PARENT, port_status[1]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NCONN, port_status[2]); + + // Self ID one. + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_CHILD, port_status[3]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_PARENT, port_status[4]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NCONN, port_status[5]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_CHILD, port_status[6]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_PARENT, port_status[7]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NCONN, port_status[8]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_CHILD, port_status[9]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_PARENT, port_status[10]); + + // Self ID two. + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NCONN, port_status[11]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_CHILD, port_status[12]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_PARENT, port_status[13]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NCONN, port_status[14]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_CHILD, port_status[15]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_PARENT, port_status[16]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NCONN, port_status[17]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_CHILD, port_status[18]); + + // Self ID three. + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_PARENT, port_status[19]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NCONN, port_status[20]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_CHILD, port_status[21]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_PARENT, port_status[22]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NCONN, port_status[23]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_CHILD, port_status[24]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_PARENT, port_status[25]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NCONN, port_status[26]); + + // Our of order. + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NONE, port_status[27]); + + KUNIT_EXPECT_MEMEQ(test, quadlets, expected, sizeof(expected)); +} + static struct kunit_case self_id_sequence_helper_test_cases[] = { KUNIT_CASE(test_self_id_sequence_enumerator_valid), KUNIT_CASE(test_self_id_sequence_enumerator_invalid), + KUNIT_CASE(test_self_id_sequence_get_port_status), {} }; From 10f5aee03787571057895762679c0bc3e1574692 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Thu, 6 Jun 2024 08:51:47 +0900 Subject: [PATCH 03/32] firewire: core: minor code refactoring for topology builder Current implementation to build tree according to self ID sequences has the rest to be refactored; e.g. putting local variables into block. This commit is for the purpose. Link: https://lore.kernel.org/r/20240605235155.116468-4-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-topology.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c index 8107eebd4296..6edba604f5cf 100644 --- a/drivers/firewire/core-topology.c +++ b/drivers/firewire/core-topology.c @@ -38,14 +38,11 @@ #define SELFID_PORT_NCONN 0x1 #define SELFID_PORT_NONE 0x0 -static u32 *count_ports(u32 *sid, int *total_port_count, int *child_port_count) +static const u32 *count_ports(const u32 *sid, int *total_port_count, int *child_port_count) { u32 q; int port_type, shift, seq; - *total_port_count = 0; - *child_port_count = 0; - shift = 6; q = *sid; seq = 0; @@ -89,7 +86,7 @@ static u32 *count_ports(u32 *sid, int *total_port_count, int *child_port_count) } } -static int get_port_type(u32 *sid, int port_index) +static int get_port_type(const u32 *sid, int port_index) { int index, shift; @@ -169,13 +166,12 @@ static inline struct fw_node *fw_node(struct list_head *l) * internally consistent. On success this function returns the * fw_node corresponding to the local card otherwise NULL. */ -static struct fw_node *build_tree(struct fw_card *card, - u32 *sid, int self_id_count) +static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self_id_count) { struct fw_node *node, *child, *local_node, *irm_node; - struct list_head stack, *h; - u32 *next_sid, *end, q; - int i, port_count, child_port_count, phy_id, parent_count, stack_depth; + struct list_head stack; + const u32 *end; + int phy_id, stack_depth; int gap_count; bool beta_repeaters_present; @@ -190,8 +186,15 @@ static struct fw_node *build_tree(struct fw_card *card, beta_repeaters_present = false; while (sid < end) { - next_sid = count_ports(sid, &port_count, &child_port_count); + int port_count = 0; + int child_port_count = 0; + int parent_count = 0; + const u32 *next_sid; + u32 q; + struct list_head *h; + int i; + next_sid = count_ports(sid, &port_count, &child_port_count); if (next_sid == NULL) { fw_err(card, "inconsistent extended self IDs\n"); return NULL; From e404cacfc5ed7273d4d2f7b4d2a70c3debc07adf Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Thu, 6 Jun 2024 08:51:48 +0900 Subject: [PATCH 04/32] firewire: ohci: minor code refactoring for self ID logging Current implementation to log self ID sequence has the rest to be refactored; e.g. moving translation-unit level variables to the dependent block. This commit is for the purpose. Link: https://lore.kernel.org/r/20240605235155.116468-5-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/ohci.c | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index f6de0b3a9a55..0ef76cf7b328 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -437,22 +437,21 @@ static void log_irqs(struct fw_ohci *ohci, u32 evt) ? " ?" : ""); } -static const char *speed[] = { - [0] = "S100", [1] = "S200", [2] = "S400", [3] = "beta", -}; -static const char *power[] = { - [0] = "+0W", [1] = "+15W", [2] = "+30W", [3] = "+45W", - [4] = "-3W", [5] = " ?W", [6] = "-3..-6W", [7] = "-3..-10W", -}; -static const char port[] = { '.', '-', 'p', 'c', }; - -static char _p(u32 *s, int shift) +static unsigned int _p(u32 *s, int shift) { - return port[*s >> shift & 3]; + return *s >> shift & 3; } static void log_selfids(struct fw_ohci *ohci, int generation, int self_id_count) { + static const char *const speed[] = { + [0] = "S100", [1] = "S200", [2] = "S400", [3] = "beta", + }; + static const char *const power[] = { + [0] = "+0W", [1] = "+15W", [2] = "+30W", [3] = "+45W", + [4] = "-3W", [5] = " ?W", [6] = "-3..-6W", [7] = "-3..-10W", + }; + static const char port[] = { '.', '-', 'p', 'c', }; u32 *s; if (likely(!(param_debug & OHCI_PARAM_DEBUG_SELFIDS))) @@ -465,7 +464,10 @@ static void log_selfids(struct fw_ohci *ohci, int generation, int self_id_count) if ((*s & 1 << 23) == 0) ohci_notice(ohci, "selfID 0: %08x, phy %d [%c%c%c] %s gc=%d %s %s%s%s\n", - *s, *s >> 24 & 63, _p(s, 6), _p(s, 4), _p(s, 2), + *s, *s >> 24 & 63, + port[_p(s, 6)], + port[_p(s, 4)], + port[_p(s, 2)], speed[*s >> 14 & 3], *s >> 16 & 63, power[*s >> 8 & 7], *s >> 22 & 1 ? "L" : "", *s >> 11 & 1 ? "c" : "", *s & 2 ? "i" : ""); @@ -473,8 +475,14 @@ static void log_selfids(struct fw_ohci *ohci, int generation, int self_id_count) ohci_notice(ohci, "selfID n: %08x, phy %d [%c%c%c%c%c%c%c%c]\n", *s, *s >> 24 & 63, - _p(s, 16), _p(s, 14), _p(s, 12), _p(s, 10), - _p(s, 8), _p(s, 6), _p(s, 4), _p(s, 2)); + port[_p(s, 16)], + port[_p(s, 14)], + port[_p(s, 12)], + port[_p(s, 10)], + port[_p(s, 8)], + port[_p(s, 6)], + port[_p(s, 4)], + port[_p(s, 2)]); } static const char *evts[] = { From 24b7f8e5cd656196a13077e160aec45ad89b58d9 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Thu, 6 Jun 2024 08:51:49 +0900 Subject: [PATCH 05/32] firewire: core: use helper functions for self ID sequence This commit replaces the existing implementation with the helper functions for self ID sequence. Link: https://lore.kernel.org/r/20240605235155.116468-6-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-topology.c | 189 +++++++++++-------------------- 1 file changed, 69 insertions(+), 120 deletions(-) diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c index 6edba604f5cf..8c10f47cc8fc 100644 --- a/drivers/firewire/core-topology.c +++ b/drivers/firewire/core-topology.c @@ -20,80 +20,15 @@ #include #include "core.h" +#include "phy-packet-definitions.h" #include #define SELF_ID_PHY_ID(q) (((q) >> 24) & 0x3f) -#define SELF_ID_EXTENDED(q) (((q) >> 23) & 0x01) #define SELF_ID_LINK_ON(q) (((q) >> 22) & 0x01) #define SELF_ID_GAP_COUNT(q) (((q) >> 16) & 0x3f) #define SELF_ID_PHY_SPEED(q) (((q) >> 14) & 0x03) #define SELF_ID_CONTENDER(q) (((q) >> 11) & 0x01) #define SELF_ID_PHY_INITIATOR(q) (((q) >> 1) & 0x01) -#define SELF_ID_MORE_PACKETS(q) (((q) >> 0) & 0x01) - -#define SELF_ID_EXT_SEQUENCE(q) (((q) >> 20) & 0x07) - -#define SELFID_PORT_CHILD 0x3 -#define SELFID_PORT_PARENT 0x2 -#define SELFID_PORT_NCONN 0x1 -#define SELFID_PORT_NONE 0x0 - -static const u32 *count_ports(const u32 *sid, int *total_port_count, int *child_port_count) -{ - u32 q; - int port_type, shift, seq; - - shift = 6; - q = *sid; - seq = 0; - - while (1) { - port_type = (q >> shift) & 0x03; - switch (port_type) { - case SELFID_PORT_CHILD: - (*child_port_count)++; - fallthrough; - case SELFID_PORT_PARENT: - case SELFID_PORT_NCONN: - (*total_port_count)++; - fallthrough; - case SELFID_PORT_NONE: - break; - } - - shift -= 2; - if (shift == 0) { - if (!SELF_ID_MORE_PACKETS(q)) - return sid + 1; - - shift = 16; - sid++; - q = *sid; - - /* - * Check that the extra packets actually are - * extended self ID packets and that the - * sequence numbers in the extended self ID - * packets increase as expected. - */ - - if (!SELF_ID_EXTENDED(q) || - seq != SELF_ID_EXT_SEQUENCE(q)) - return NULL; - - seq++; - } - } -} - -static int get_port_type(const u32 *sid, int port_index) -{ - int index, shift; - - index = (port_index + 5) / 8; - shift = 16 - ((port_index + 5) & 7) * 2; - return (sid[index] >> shift) & 0x03; -} static struct fw_node *fw_node_create(u32 sid, int port_count, int color) { @@ -168,9 +103,12 @@ static inline struct fw_node *fw_node(struct list_head *l) */ static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self_id_count) { + struct self_id_sequence_enumerator enumerator = { + .cursor = sid, + .quadlet_count = self_id_count, + }; struct fw_node *node, *child, *local_node, *irm_node; struct list_head stack; - const u32 *end; int phy_id, stack_depth; int gap_count; bool beta_repeaters_present; @@ -179,31 +117,54 @@ static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self node = NULL; INIT_LIST_HEAD(&stack); stack_depth = 0; - end = sid + self_id_count; phy_id = 0; irm_node = NULL; gap_count = SELF_ID_GAP_COUNT(*sid); beta_repeaters_present = false; - while (sid < end) { - int port_count = 0; - int child_port_count = 0; - int parent_count = 0; - const u32 *next_sid; - u32 q; + while (enumerator.quadlet_count > 0) { + unsigned int child_port_count = 0; + unsigned int total_port_count = 0; + unsigned int parent_count = 0; + unsigned int quadlet_count; + const u32 *self_id_sequence; + unsigned int port_capacity; + enum phy_packet_self_id_port_status port_status; + unsigned int port_index; struct list_head *h; int i; - next_sid = count_ports(sid, &port_count, &child_port_count); - if (next_sid == NULL) { - fw_err(card, "inconsistent extended self IDs\n"); - return NULL; + self_id_sequence = self_id_sequence_enumerator_next(&enumerator, &quadlet_count); + if (IS_ERR(self_id_sequence)) { + if (PTR_ERR(self_id_sequence) != -ENODATA) { + fw_err(card, "inconsistent extended self IDs: %ld\n", + PTR_ERR(self_id_sequence)); + return NULL; + } + break; } - q = *sid; - if (phy_id != SELF_ID_PHY_ID(q)) { + port_capacity = self_id_sequence_get_port_capacity(quadlet_count); + for (port_index = 0; port_index < port_capacity; ++port_index) { + port_status = self_id_sequence_get_port_status(self_id_sequence, quadlet_count, + port_index); + switch (port_status) { + case PHY_PACKET_SELF_ID_PORT_STATUS_CHILD: + ++child_port_count; + fallthrough; + case PHY_PACKET_SELF_ID_PORT_STATUS_PARENT: + case PHY_PACKET_SELF_ID_PORT_STATUS_NCONN: + ++total_port_count; + fallthrough; + case PHY_PACKET_SELF_ID_PORT_STATUS_NONE: + default: + break; + } + } + + if (phy_id != SELF_ID_PHY_ID(self_id_sequence[0])) { fw_err(card, "PHY ID mismatch in self ID: %d != %d\n", - phy_id, SELF_ID_PHY_ID(q)); + phy_id, SELF_ID_PHY_ID(self_id_sequence[0])); return NULL; } @@ -224,7 +185,7 @@ static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self */ child = fw_node(h); - node = fw_node_create(q, port_count, card->color); + node = fw_node_create(self_id_sequence[0], total_port_count, card->color); if (node == NULL) { fw_err(card, "out of memory while building topology\n"); return NULL; @@ -233,48 +194,40 @@ static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self if (phy_id == (card->node_id & 0x3f)) local_node = node; - if (SELF_ID_CONTENDER(q)) + if (SELF_ID_CONTENDER(self_id_sequence[0])) irm_node = node; - parent_count = 0; - - for (i = 0; i < port_count; i++) { - switch (get_port_type(sid, i)) { - case SELFID_PORT_PARENT: - /* - * Who's your daddy? We dont know the - * parent node at this time, so we - * temporarily abuse node->color for - * remembering the entry in the - * node->ports array where the parent - * node should be. Later, when we - * handle the parent node, we fix up - * the reference. - */ - parent_count++; + for (port_index = 0; port_index < total_port_count; ++port_index) { + port_status = self_id_sequence_get_port_status(self_id_sequence, quadlet_count, + port_index); + switch (port_status) { + case PHY_PACKET_SELF_ID_PORT_STATUS_PARENT: + // Who's your daddy? We dont know the parent node at this time, so + // we temporarily abuse node->color for remembering the entry in + // the node->ports array where the parent node should be. Later, + // when we handle the parent node, we fix up the reference. + ++parent_count; node->color = i; break; - case SELFID_PORT_CHILD: - node->ports[i] = child; - /* - * Fix up parent reference for this - * child node. - */ + case PHY_PACKET_SELF_ID_PORT_STATUS_CHILD: + node->ports[port_index] = child; + // Fix up parent reference for this child node. child->ports[child->color] = node; child->color = card->color; child = fw_node(child->link.next); break; + case PHY_PACKET_SELF_ID_PORT_STATUS_NCONN: + case PHY_PACKET_SELF_ID_PORT_STATUS_NONE: + default: + break; } } - /* - * Check that the node reports exactly one parent - * port, except for the root, which of course should - * have no parents. - */ - if ((next_sid == end && parent_count != 0) || - (next_sid < end && parent_count != 1)) { + // Check that the node reports exactly one parent port, except for the root, which + // of course should have no parents. + if ((enumerator.quadlet_count == 0 && parent_count != 0) || + (enumerator.quadlet_count > 0 && parent_count != 1)) { fw_err(card, "parent port inconsistency for node %d: " "parent_count=%d\n", phy_id, parent_count); return NULL; @@ -285,20 +238,16 @@ static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self list_add_tail(&node->link, &stack); stack_depth += 1 - child_port_count; - if (node->phy_speed == SCODE_BETA && - parent_count + child_port_count > 1) + if (node->phy_speed == SCODE_BETA && parent_count + child_port_count > 1) beta_repeaters_present = true; - /* - * If PHYs report different gap counts, set an invalid count - * which will force a gap count reconfiguration and a reset. - */ - if (SELF_ID_GAP_COUNT(q) != gap_count) + // If PHYs report different gap counts, set an invalid count which will force a gap + // count reconfiguration and a reset. + if (SELF_ID_GAP_COUNT(self_id_sequence[0]) != gap_count) gap_count = 0; update_hop_count(node); - sid = next_sid; phy_id++; } From a16931ac6d8e0eb3152f361fb1e24f3fb15e0cf0 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Thu, 6 Jun 2024 08:51:50 +0900 Subject: [PATCH 06/32] firewire: ohci: use helper functions for self ID sequence This commit replaces the existing implementation with the helper functions for self ID sequence. Link: https://lore.kernel.org/r/20240605235155.116468-7-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/ohci.c | 77 ++++++++++++++++++++++++++--------------- 1 file changed, 49 insertions(+), 28 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 0ef76cf7b328..342407d8bc9b 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -41,6 +41,7 @@ #include "core.h" #include "ohci.h" #include "packet-header-definitions.h" +#include "phy-packet-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) @@ -437,11 +438,6 @@ static void log_irqs(struct fw_ohci *ohci, u32 evt) ? " ?" : ""); } -static unsigned int _p(u32 *s, int shift) -{ - return *s >> shift & 3; -} - static void log_selfids(struct fw_ohci *ohci, int generation, int self_id_count) { static const char *const speed[] = { @@ -451,8 +447,16 @@ static void log_selfids(struct fw_ohci *ohci, int generation, int self_id_count) [0] = "+0W", [1] = "+15W", [2] = "+30W", [3] = "+45W", [4] = "-3W", [5] = " ?W", [6] = "-3..-6W", [7] = "-3..-10W", }; - static const char port[] = { '.', '-', 'p', 'c', }; - u32 *s; + static const char port[] = { + [PHY_PACKET_SELF_ID_PORT_STATUS_NONE] = '.', + [PHY_PACKET_SELF_ID_PORT_STATUS_NCONN] = '-', + [PHY_PACKET_SELF_ID_PORT_STATUS_PARENT] = 'p', + [PHY_PACKET_SELF_ID_PORT_STATUS_CHILD] = 'c', + }; + struct self_id_sequence_enumerator enumerator = { + .cursor = ohci->self_id_buffer, + .quadlet_count = self_id_count, + }; if (likely(!(param_debug & OHCI_PARAM_DEBUG_SELFIDS))) return; @@ -460,29 +464,46 @@ static void log_selfids(struct fw_ohci *ohci, int generation, int self_id_count) ohci_notice(ohci, "%d selfIDs, generation %d, local node ID %04x\n", self_id_count, generation, ohci->node_id); - for (s = ohci->self_id_buffer; self_id_count--; ++s) - if ((*s & 1 << 23) == 0) - ohci_notice(ohci, - "selfID 0: %08x, phy %d [%c%c%c] %s gc=%d %s %s%s%s\n", - *s, *s >> 24 & 63, - port[_p(s, 6)], - port[_p(s, 4)], - port[_p(s, 2)], - speed[*s >> 14 & 3], *s >> 16 & 63, - power[*s >> 8 & 7], *s >> 22 & 1 ? "L" : "", - *s >> 11 & 1 ? "c" : "", *s & 2 ? "i" : ""); - else + while (enumerator.quadlet_count > 0) { + unsigned int quadlet_count; + unsigned int port_index; + const u32 *s; + int i; + + s = self_id_sequence_enumerator_next(&enumerator, &quadlet_count); + if (IS_ERR(s)) + break; + + ohci_notice(ohci, + "selfID 0: %08x, phy %d [%c%c%c] %s gc=%d %s %s%s%s\n", + *s, + *s >> 24 & 63, + port[self_id_sequence_get_port_status(s, quadlet_count, 0)], + port[self_id_sequence_get_port_status(s, quadlet_count, 1)], + port[self_id_sequence_get_port_status(s, quadlet_count, 2)], + speed[*s >> 14 & 3], *s >> 16 & 63, + power[*s >> 8 & 7], *s >> 22 & 1 ? "L" : "", + *s >> 11 & 1 ? "c" : "", *s & 2 ? "i" : ""); + + port_index = 3; + for (i = 1; i < quadlet_count; ++i) { ohci_notice(ohci, "selfID n: %08x, phy %d [%c%c%c%c%c%c%c%c]\n", - *s, *s >> 24 & 63, - port[_p(s, 16)], - port[_p(s, 14)], - port[_p(s, 12)], - port[_p(s, 10)], - port[_p(s, 8)], - port[_p(s, 6)], - port[_p(s, 4)], - port[_p(s, 2)]); + s[i], + s[i] >> 24 & 63, + port[self_id_sequence_get_port_status(s, quadlet_count, port_index)], + port[self_id_sequence_get_port_status(s, quadlet_count, port_index + 1)], + port[self_id_sequence_get_port_status(s, quadlet_count, port_index + 2)], + port[self_id_sequence_get_port_status(s, quadlet_count, port_index + 3)], + port[self_id_sequence_get_port_status(s, quadlet_count, port_index + 4)], + port[self_id_sequence_get_port_status(s, quadlet_count, port_index + 5)], + port[self_id_sequence_get_port_status(s, quadlet_count, port_index + 6)], + port[self_id_sequence_get_port_status(s, quadlet_count, port_index + 7)] + ); + + port_index += 8; + } + } } static const char *evts[] = { From baf9d20b99ab2f79a501d8d1838cc5cf1ae731c3 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Thu, 6 Jun 2024 08:51:51 +0900 Subject: [PATCH 07/32] firewire: core: add common inline functions to serialize/deserialize self ID packet Within FireWire subsystem, the serializations and deserializations of phy packet are implemented in several parts. They includes some redundancies. This commit adds a series of helper functions for the serializations and deserializations of self ID packet with a Kunit test suite. Link: https://lore.kernel.org/r/20240605235155.116468-8-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/packet-serdes-test.c | 255 ++++++++++++++++++++++ drivers/firewire/phy-packet-definitions.h | 126 +++++++++++ 2 files changed, 381 insertions(+) diff --git a/drivers/firewire/packet-serdes-test.c b/drivers/firewire/packet-serdes-test.c index e83b1fece780..c56199e84f91 100644 --- a/drivers/firewire/packet-serdes-test.c +++ b/drivers/firewire/packet-serdes-test.c @@ -10,6 +10,7 @@ #include #include "packet-header-definitions.h" +#include "phy-packet-definitions.h" static void serialize_async_header_common(u32 header[ASYNC_HEADER_QUADLET_COUNT], unsigned int dst_id, unsigned int tlabel, @@ -187,6 +188,66 @@ static void deserialize_isoc_header(u32 header, unsigned int *data_length, unsig *sy = isoc_header_get_sy(header); } +static void serialize_phy_packet_self_id_zero(u32 *quadlet, unsigned int packet_identifier, + unsigned int phy_id, bool extended, + bool link_is_active, unsigned int gap_count, + unsigned int scode, bool is_contender, + unsigned int power_class, bool is_initiated_reset, + bool has_more_packets) +{ + phy_packet_set_packet_identifier(quadlet, packet_identifier); + phy_packet_self_id_set_phy_id(quadlet, phy_id); + phy_packet_self_id_set_extended(quadlet, extended); + phy_packet_self_id_zero_set_link_active(quadlet, link_is_active); + phy_packet_self_id_zero_set_gap_count(quadlet, gap_count); + phy_packet_self_id_zero_set_scode(quadlet, scode); + phy_packet_self_id_zero_set_contender(quadlet, is_contender); + phy_packet_self_id_zero_set_power_class(quadlet, power_class); + phy_packet_self_id_zero_set_initiated_reset(quadlet, is_initiated_reset); + phy_packet_self_id_set_more_packets(quadlet, has_more_packets); +} + +static void deserialize_phy_packet_self_id_zero(u32 quadlet, unsigned int *packet_identifier, + unsigned int *phy_id, bool *extended, + bool *link_is_active, unsigned int *gap_count, + unsigned int *scode, bool *is_contender, + unsigned int *power_class, + bool *is_initiated_reset, bool *has_more_packets) +{ + *packet_identifier = phy_packet_get_packet_identifier(quadlet); + *phy_id = phy_packet_self_id_get_phy_id(quadlet); + *extended = phy_packet_self_id_get_extended(quadlet); + *link_is_active = phy_packet_self_id_zero_get_link_active(quadlet); + *gap_count = phy_packet_self_id_zero_get_gap_count(quadlet); + *scode = phy_packet_self_id_zero_get_scode(quadlet); + *is_contender = phy_packet_self_id_zero_get_contender(quadlet); + *power_class = phy_packet_self_id_zero_get_power_class(quadlet); + *is_initiated_reset = phy_packet_self_id_zero_get_initiated_reset(quadlet); + *has_more_packets = phy_packet_self_id_get_more_packets(quadlet); +} + +static void serialize_phy_packet_self_id_extended(u32 *quadlet, unsigned int packet_identifier, + unsigned int phy_id, bool extended, + unsigned int sequence, bool has_more_packets) +{ + phy_packet_set_packet_identifier(quadlet, packet_identifier); + phy_packet_self_id_set_phy_id(quadlet, phy_id); + phy_packet_self_id_set_extended(quadlet, extended); + phy_packet_self_id_extended_set_sequence(quadlet, sequence); + phy_packet_self_id_set_more_packets(quadlet, has_more_packets); +} + +static void deserialize_phy_packet_self_id_extended(u32 quadlet, unsigned int *packet_identifier, + unsigned int *phy_id, bool *extended, + unsigned int *sequence, bool *has_more_packets) +{ + *packet_identifier = phy_packet_get_packet_identifier(quadlet); + *phy_id = phy_packet_self_id_get_phy_id(quadlet); + *extended = phy_packet_self_id_get_extended(quadlet); + *sequence = phy_packet_self_id_extended_get_sequence(quadlet); + *has_more_packets = phy_packet_self_id_get_more_packets(quadlet); +} + static void test_async_header_write_quadlet_request(struct kunit *test) { static const u32 expected[ASYNC_HEADER_QUADLET_COUNT] = { @@ -559,6 +620,197 @@ static void test_isoc_header(struct kunit *test) KUNIT_EXPECT_EQ(test, header, expected); } +static void test_phy_packet_self_id_zero_case0(struct kunit *test) +{ + // TSB41AB1/2 with 1 port. + const u32 expected[] = {0x80458c80}; + u32 quadlets[] = {0}; + + unsigned int packet_identifier; + unsigned int phy_id; + bool extended; + bool link_is_active; + unsigned int gap_count; + unsigned int scode; + bool is_contender; + unsigned int power_class; + enum phy_packet_self_id_port_status port_status[3]; + bool is_initiated_reset; + bool has_more_packets; + unsigned int port_index; + + deserialize_phy_packet_self_id_zero(expected[0], &packet_identifier, &phy_id, &extended, + &link_is_active, &gap_count, &scode, &is_contender, + &power_class, &is_initiated_reset, &has_more_packets); + + KUNIT_EXPECT_EQ(test, PHY_PACKET_PACKET_IDENTIFIER_SELF_ID, packet_identifier); + KUNIT_EXPECT_EQ(test, 0, phy_id); + KUNIT_EXPECT_FALSE(test, extended); + KUNIT_EXPECT_TRUE(test, link_is_active); + KUNIT_EXPECT_EQ(test, 0x05, gap_count); + KUNIT_EXPECT_EQ(test, SCODE_400, scode); + KUNIT_EXPECT_TRUE(test, is_contender); + KUNIT_EXPECT_EQ(test, 0x4, power_class); + KUNIT_EXPECT_FALSE(test, is_initiated_reset); + KUNIT_EXPECT_FALSE(test, has_more_packets); + + serialize_phy_packet_self_id_zero(quadlets, packet_identifier, phy_id, extended, + link_is_active, gap_count, scode, is_contender, + power_class, is_initiated_reset, has_more_packets); + + for (port_index = 0; port_index < ARRAY_SIZE(port_status); ++port_index) { + port_status[port_index] = + self_id_sequence_get_port_status(expected, ARRAY_SIZE(expected), port_index); + } + + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_PARENT, port_status[0]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NONE, port_status[1]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NONE, port_status[2]); + + for (port_index = 0; port_index < ARRAY_SIZE(port_status); ++port_index) { + self_id_sequence_set_port_status(quadlets, ARRAY_SIZE(quadlets), port_index, + port_status[port_index]); + } + + KUNIT_EXPECT_MEMEQ(test, quadlets, expected, sizeof(expected)); +} + +static void test_phy_packet_self_id_zero_case1(struct kunit *test) +{ + // XIO2213 and TSB81BA3E with 3 ports. + const u32 expected[] = {0x817fcc5e}; + u32 quadlets[] = {0}; + + unsigned int packet_identifier; + unsigned int phy_id; + bool extended; + bool link_is_active; + unsigned int gap_count; + unsigned int scode; + bool is_contender; + unsigned int power_class; + enum phy_packet_self_id_port_status port_status[3]; + bool is_initiated_reset; + bool has_more_packets; + unsigned int port_index; + + deserialize_phy_packet_self_id_zero(expected[0], &packet_identifier, &phy_id, &extended, + &link_is_active, &gap_count, &scode, &is_contender, + &power_class, &is_initiated_reset, &has_more_packets); + + KUNIT_EXPECT_EQ(test, PHY_PACKET_PACKET_IDENTIFIER_SELF_ID, packet_identifier); + KUNIT_EXPECT_EQ(test, 1, phy_id); + KUNIT_EXPECT_FALSE(test, extended); + KUNIT_EXPECT_TRUE(test, link_is_active); + KUNIT_EXPECT_EQ(test, 0x3f, gap_count); + KUNIT_EXPECT_EQ(test, SCODE_800, scode); + KUNIT_EXPECT_TRUE(test, is_contender); + KUNIT_EXPECT_EQ(test, 0x4, power_class); + KUNIT_EXPECT_TRUE(test, is_initiated_reset); + KUNIT_EXPECT_FALSE(test, has_more_packets); + + serialize_phy_packet_self_id_zero(quadlets, packet_identifier, phy_id, extended, + link_is_active, gap_count, scode, is_contender, + power_class, is_initiated_reset, has_more_packets); + + for (port_index = 0; port_index < ARRAY_SIZE(port_status); ++port_index) { + port_status[port_index] = + self_id_sequence_get_port_status(expected, ARRAY_SIZE(expected), port_index); + } + + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NCONN, port_status[0]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NCONN, port_status[1]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_CHILD, port_status[2]); + + for (port_index = 0; port_index < ARRAY_SIZE(port_status); ++port_index) { + self_id_sequence_set_port_status(quadlets, ARRAY_SIZE(quadlets), port_index, + port_status[port_index]); + } + + KUNIT_EXPECT_MEMEQ(test, quadlets, expected, sizeof(expected)); +} + +static void test_phy_packet_self_id_zero_and_one(struct kunit *test) +{ + // TSB41LV06A with 6 ports. + const u32 expected[] = { + 0x803f8459, + 0x80815000, + }; + u32 quadlets[] = {0, 0}; + + unsigned int packet_identifier; + unsigned int phy_id; + bool extended; + bool link_is_active; + unsigned int gap_count; + unsigned int scode; + bool is_contender; + unsigned int power_class; + enum phy_packet_self_id_port_status port_status[11]; + bool is_initiated_reset; + bool has_more_packets; + + unsigned int sequence; + unsigned int port_index; + + deserialize_phy_packet_self_id_zero(expected[0], &packet_identifier, &phy_id, &extended, + &link_is_active, &gap_count, &scode, &is_contender, + &power_class, &is_initiated_reset, &has_more_packets); + + KUNIT_EXPECT_EQ(test, PHY_PACKET_PACKET_IDENTIFIER_SELF_ID, packet_identifier); + KUNIT_EXPECT_EQ(test, 0, phy_id); + KUNIT_EXPECT_FALSE(test, extended); + KUNIT_EXPECT_FALSE(test, link_is_active); + KUNIT_EXPECT_EQ(test, 0x3f, gap_count); + KUNIT_EXPECT_EQ(test, SCODE_400, scode); + KUNIT_EXPECT_FALSE(test, is_contender); + KUNIT_EXPECT_EQ(test, 0x4, power_class); + KUNIT_EXPECT_FALSE(test, is_initiated_reset); + KUNIT_EXPECT_TRUE(test, has_more_packets); + + serialize_phy_packet_self_id_zero(quadlets, packet_identifier, phy_id, extended, + link_is_active, gap_count, scode, is_contender, + power_class, is_initiated_reset, has_more_packets); + + deserialize_phy_packet_self_id_extended(expected[1], &packet_identifier, &phy_id, &extended, + &sequence, &has_more_packets); + + KUNIT_EXPECT_EQ(test, PHY_PACKET_PACKET_IDENTIFIER_SELF_ID, packet_identifier); + KUNIT_EXPECT_EQ(test, 0, phy_id); + KUNIT_EXPECT_TRUE(test, extended); + KUNIT_EXPECT_EQ(test, 0, sequence); + KUNIT_EXPECT_FALSE(test, has_more_packets); + + serialize_phy_packet_self_id_extended(&quadlets[1], packet_identifier, phy_id, extended, + sequence, has_more_packets); + + + for (port_index = 0; port_index < ARRAY_SIZE(port_status); ++port_index) { + port_status[port_index] = + self_id_sequence_get_port_status(expected, ARRAY_SIZE(expected), port_index); + } + + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NCONN, port_status[0]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NCONN, port_status[1]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_PARENT, port_status[2]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NCONN, port_status[3]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NCONN, port_status[4]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NCONN, port_status[5]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NONE, port_status[6]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NONE, port_status[7]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NONE, port_status[8]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NONE, port_status[9]); + KUNIT_EXPECT_EQ(test, PHY_PACKET_SELF_ID_PORT_STATUS_NONE, port_status[10]); + + for (port_index = 0; port_index < ARRAY_SIZE(port_status); ++port_index) { + self_id_sequence_set_port_status(quadlets, ARRAY_SIZE(quadlets), port_index, + port_status[port_index]); + } + + KUNIT_EXPECT_MEMEQ(test, quadlets, 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), @@ -570,6 +822,9 @@ static struct kunit_case packet_serdes_test_cases[] = { KUNIT_CASE(test_async_header_lock_request), KUNIT_CASE(test_async_header_lock_response), KUNIT_CASE(test_isoc_header), + KUNIT_CASE(test_phy_packet_self_id_zero_case0), + KUNIT_CASE(test_phy_packet_self_id_zero_case1), + KUNIT_CASE(test_phy_packet_self_id_zero_and_one), {} }; diff --git a/drivers/firewire/phy-packet-definitions.h b/drivers/firewire/phy-packet-definitions.h index 4ba8b18aa993..8f78494ad371 100644 --- a/drivers/firewire/phy-packet-definitions.h +++ b/drivers/firewire/phy-packet-definitions.h @@ -7,11 +7,42 @@ #ifndef _FIREWIRE_PHY_PACKET_DEFINITIONS_H #define _FIREWIRE_PHY_PACKET_DEFINITIONS_H +#define PACKET_IDENTIFIER_MASK 0xc0000000 +#define PACKET_IDENTIFIER_SHIFT 30 + +static inline unsigned int phy_packet_get_packet_identifier(u32 quadlet) +{ + return (quadlet & PACKET_IDENTIFIER_MASK) >> PACKET_IDENTIFIER_SHIFT; +} + +static inline void phy_packet_set_packet_identifier(u32 *quadlet, unsigned int packet_identifier) +{ + *quadlet &= ~PACKET_IDENTIFIER_MASK; + *quadlet |= (packet_identifier << PACKET_IDENTIFIER_SHIFT) & PACKET_IDENTIFIER_MASK; +} + +#define PHY_PACKET_PACKET_IDENTIFIER_SELF_ID 2 + +#define SELF_ID_PHY_ID_MASK 0x3f000000 +#define SELF_ID_PHY_ID_SHIFT 24 #define SELF_ID_EXTENDED_MASK 0x00800000 #define SELF_ID_EXTENDED_SHIFT 23 #define SELF_ID_MORE_PACKETS_MASK 0x00000001 #define SELF_ID_MORE_PACKETS_SHIFT 0 +#define SELF_ID_ZERO_LINK_ACTIVE_MASK 0x00400000 +#define SELF_ID_ZERO_LINK_ACTIVE_SHIFT 22 +#define SELF_ID_ZERO_GAP_COUNT_MASK 0x003f0000 +#define SELF_ID_ZERO_GAP_COUNT_SHIFT 16 +#define SELF_ID_ZERO_SCODE_MASK 0x0000c000 +#define SELF_ID_ZERO_SCODE_SHIFT 14 +#define SELF_ID_ZERO_CONTENDER_MASK 0x00000800 +#define SELF_ID_ZERO_CONTENDER_SHIFT 11 +#define SELF_ID_ZERO_POWER_CLASS_MASK 0x00000700 +#define SELF_ID_ZERO_POWER_CLASS_SHIFT 8 +#define SELF_ID_ZERO_INITIATED_RESET_MASK 0x00000002 +#define SELF_ID_ZERO_INITIATED_RESET_SHIFT 1 + #define SELF_ID_EXTENDED_SEQUENCE_MASK 0x00700000 #define SELF_ID_EXTENDED_SEQUENCE_SHIFT 20 @@ -19,21 +50,116 @@ #define SELF_ID_SEQUENCE_MAXIMUM_QUADLET_COUNT 4 +static inline unsigned int phy_packet_self_id_get_phy_id(u32 quadlet) +{ + return (quadlet & SELF_ID_PHY_ID_MASK) >> SELF_ID_PHY_ID_SHIFT; +} + +static inline void phy_packet_self_id_set_phy_id(u32 *quadlet, unsigned int phy_id) +{ + *quadlet &= ~SELF_ID_PHY_ID_MASK; + *quadlet |= (phy_id << SELF_ID_PHY_ID_SHIFT) & SELF_ID_PHY_ID_MASK; +} + static inline bool phy_packet_self_id_get_extended(u32 quadlet) { return (quadlet & SELF_ID_EXTENDED_MASK) >> SELF_ID_EXTENDED_SHIFT; } +static inline void phy_packet_self_id_set_extended(u32 *quadlet, bool extended) +{ + *quadlet &= ~SELF_ID_EXTENDED_MASK; + *quadlet |= (extended << SELF_ID_EXTENDED_SHIFT) & SELF_ID_EXTENDED_MASK; +} + +static inline bool phy_packet_self_id_zero_get_link_active(u32 quadlet) +{ + return (quadlet & SELF_ID_ZERO_LINK_ACTIVE_MASK) >> SELF_ID_ZERO_LINK_ACTIVE_SHIFT; +} + +static inline void phy_packet_self_id_zero_set_link_active(u32 *quadlet, bool is_active) +{ + *quadlet &= ~SELF_ID_ZERO_LINK_ACTIVE_MASK; + *quadlet |= (is_active << SELF_ID_ZERO_LINK_ACTIVE_SHIFT) & SELF_ID_ZERO_LINK_ACTIVE_MASK; +} + +static inline unsigned int phy_packet_self_id_zero_get_gap_count(u32 quadlet) +{ + return (quadlet & SELF_ID_ZERO_GAP_COUNT_MASK) >> SELF_ID_ZERO_GAP_COUNT_SHIFT; +} + +static inline void phy_packet_self_id_zero_set_gap_count(u32 *quadlet, unsigned int gap_count) +{ + *quadlet &= ~SELF_ID_ZERO_GAP_COUNT_MASK; + *quadlet |= (gap_count << SELF_ID_ZERO_GAP_COUNT_SHIFT) & SELF_ID_ZERO_GAP_COUNT_MASK; +} + +static inline unsigned int phy_packet_self_id_zero_get_scode(u32 quadlet) +{ + return (quadlet & SELF_ID_ZERO_SCODE_MASK) >> SELF_ID_ZERO_SCODE_SHIFT; +} + +static inline void phy_packet_self_id_zero_set_scode(u32 *quadlet, unsigned int speed) +{ + *quadlet &= ~SELF_ID_ZERO_SCODE_MASK; + *quadlet |= (speed << SELF_ID_ZERO_SCODE_SHIFT) & SELF_ID_ZERO_SCODE_MASK; +} + +static inline bool phy_packet_self_id_zero_get_contender(u32 quadlet) +{ + return (quadlet & SELF_ID_ZERO_CONTENDER_MASK) >> SELF_ID_ZERO_CONTENDER_SHIFT; +} + +static inline void phy_packet_self_id_zero_set_contender(u32 *quadlet, bool is_contender) +{ + *quadlet &= ~SELF_ID_ZERO_CONTENDER_MASK; + *quadlet |= (is_contender << SELF_ID_ZERO_CONTENDER_SHIFT) & SELF_ID_ZERO_CONTENDER_MASK; +} + +static inline unsigned int phy_packet_self_id_zero_get_power_class(u32 quadlet) +{ + return (quadlet & SELF_ID_ZERO_POWER_CLASS_MASK) >> SELF_ID_ZERO_POWER_CLASS_SHIFT; +} + +static inline void phy_packet_self_id_zero_set_power_class(u32 *quadlet, unsigned int power_class) +{ + *quadlet &= ~SELF_ID_ZERO_POWER_CLASS_MASK; + *quadlet |= (power_class << SELF_ID_ZERO_POWER_CLASS_SHIFT) & SELF_ID_ZERO_POWER_CLASS_MASK; +} + +static inline bool phy_packet_self_id_zero_get_initiated_reset(u32 quadlet) +{ + return (quadlet & SELF_ID_ZERO_INITIATED_RESET_MASK) >> SELF_ID_ZERO_INITIATED_RESET_SHIFT; +} + +static inline void phy_packet_self_id_zero_set_initiated_reset(u32 *quadlet, bool is_initiated_reset) +{ + *quadlet &= ~SELF_ID_ZERO_INITIATED_RESET_MASK; + *quadlet |= (is_initiated_reset << SELF_ID_ZERO_INITIATED_RESET_SHIFT) & SELF_ID_ZERO_INITIATED_RESET_MASK; +} + static inline bool phy_packet_self_id_get_more_packets(u32 quadlet) { return (quadlet & SELF_ID_MORE_PACKETS_MASK) >> SELF_ID_MORE_PACKETS_SHIFT; } +static inline void phy_packet_self_id_set_more_packets(u32 *quadlet, bool is_more_packets) +{ + *quadlet &= ~SELF_ID_MORE_PACKETS_MASK; + *quadlet |= (is_more_packets << SELF_ID_MORE_PACKETS_SHIFT) & SELF_ID_MORE_PACKETS_MASK; +} + static inline unsigned int phy_packet_self_id_extended_get_sequence(u32 quadlet) { return (quadlet & SELF_ID_EXTENDED_SEQUENCE_MASK) >> SELF_ID_EXTENDED_SEQUENCE_SHIFT; } +static inline void phy_packet_self_id_extended_set_sequence(u32 *quadlet, unsigned int sequence) +{ + *quadlet &= ~SELF_ID_EXTENDED_SEQUENCE_MASK; + *quadlet |= (sequence << SELF_ID_EXTENDED_SHIFT) & SELF_ID_EXTENDED_SEQUENCE_MASK; +} + struct self_id_sequence_enumerator { const u32 *cursor; unsigned int quadlet_count; From 8f035147f095cc4b8e79dc4b83cf564ea1a61de1 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Thu, 6 Jun 2024 08:51:52 +0900 Subject: [PATCH 08/32] firewire: core: use helper inline functions to deserialize self ID packet This commit replaces the existing implementation with the helper functions for self ID packet. Link: https://lore.kernel.org/r/20240605235155.116468-9-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-topology.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c index 8c10f47cc8fc..e17a01345857 100644 --- a/drivers/firewire/core-topology.c +++ b/drivers/firewire/core-topology.c @@ -23,13 +23,6 @@ #include "phy-packet-definitions.h" #include -#define SELF_ID_PHY_ID(q) (((q) >> 24) & 0x3f) -#define SELF_ID_LINK_ON(q) (((q) >> 22) & 0x01) -#define SELF_ID_GAP_COUNT(q) (((q) >> 16) & 0x3f) -#define SELF_ID_PHY_SPEED(q) (((q) >> 14) & 0x03) -#define SELF_ID_CONTENDER(q) (((q) >> 11) & 0x01) -#define SELF_ID_PHY_INITIATOR(q) (((q) >> 1) & 0x01) - static struct fw_node *fw_node_create(u32 sid, int port_count, int color) { struct fw_node *node; @@ -39,10 +32,11 @@ static struct fw_node *fw_node_create(u32 sid, int port_count, int color) return NULL; node->color = color; - node->node_id = LOCAL_BUS | SELF_ID_PHY_ID(sid); - node->link_on = SELF_ID_LINK_ON(sid); - node->phy_speed = SELF_ID_PHY_SPEED(sid); - node->initiated_reset = SELF_ID_PHY_INITIATOR(sid); + node->node_id = LOCAL_BUS | phy_packet_self_id_get_phy_id(sid); + node->link_on = phy_packet_self_id_zero_get_link_active(sid); + // NOTE: Only two bits, thus only for SCODE_100, SCODE_200, SCODE_400, and SCODE_BETA. + node->phy_speed = phy_packet_self_id_zero_get_scode(sid); + node->initiated_reset = phy_packet_self_id_zero_get_initiated_reset(sid); node->port_count = port_count; refcount_set(&node->ref_count, 1); @@ -119,7 +113,7 @@ static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self stack_depth = 0; phy_id = 0; irm_node = NULL; - gap_count = SELF_ID_GAP_COUNT(*sid); + gap_count = phy_packet_self_id_zero_get_gap_count(*sid); beta_repeaters_present = false; while (enumerator.quadlet_count > 0) { @@ -162,9 +156,9 @@ static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self } } - if (phy_id != SELF_ID_PHY_ID(self_id_sequence[0])) { + if (phy_id != phy_packet_self_id_get_phy_id(self_id_sequence[0])) { fw_err(card, "PHY ID mismatch in self ID: %d != %d\n", - phy_id, SELF_ID_PHY_ID(self_id_sequence[0])); + phy_id, phy_packet_self_id_get_phy_id(self_id_sequence[0])); return NULL; } @@ -194,7 +188,7 @@ static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self if (phy_id == (card->node_id & 0x3f)) local_node = node; - if (SELF_ID_CONTENDER(self_id_sequence[0])) + if (phy_packet_self_id_zero_get_contender(self_id_sequence[0])) irm_node = node; for (port_index = 0; port_index < total_port_count; ++port_index) { @@ -243,7 +237,7 @@ static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self // If PHYs report different gap counts, set an invalid count which will force a gap // count reconfiguration and a reset. - if (SELF_ID_GAP_COUNT(self_id_sequence[0]) != gap_count) + if (phy_packet_self_id_zero_get_gap_count(self_id_sequence[0]) != gap_count) gap_count = 0; update_hop_count(node); From f9a228364e5e2e80ea964214806d7b65b8ec93fb Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Thu, 6 Jun 2024 08:51:53 +0900 Subject: [PATCH 09/32] firewire: ohci: use helper inline functions to serialize/deserialize self ID packet This commit replaces the existing implementation with the helper functions for self ID packet. Link: https://lore.kernel.org/r/20240605235155.116468-10-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/ohci.c | 69 +++++++++++++++++++++++++++-------------- 1 file changed, 45 insertions(+), 24 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 342407d8bc9b..1f6097a6366c 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -477,7 +477,7 @@ static void log_selfids(struct fw_ohci *ohci, int generation, int self_id_count) ohci_notice(ohci, "selfID 0: %08x, phy %d [%c%c%c] %s gc=%d %s %s%s%s\n", *s, - *s >> 24 & 63, + phy_packet_self_id_get_phy_id(*s), port[self_id_sequence_get_port_status(s, quadlet_count, 0)], port[self_id_sequence_get_port_status(s, quadlet_count, 1)], port[self_id_sequence_get_port_status(s, quadlet_count, 2)], @@ -490,7 +490,7 @@ static void log_selfids(struct fw_ohci *ohci, int generation, int self_id_count) ohci_notice(ohci, "selfID n: %08x, phy %d [%c%c%c%c%c%c%c%c]\n", s[i], - s[i] >> 24 & 63, + phy_packet_self_id_get_phy_id(s[i]), port[self_id_sequence_get_port_status(s, quadlet_count, port_index)], port[self_id_sequence_get_port_status(s, quadlet_count, port_index + 1)], port[self_id_sequence_get_port_status(s, quadlet_count, port_index + 2)], @@ -1846,7 +1846,8 @@ static u32 update_bus_time(struct fw_ohci *ohci) return ohci->bus_time | cycle_time_seconds; } -static int get_status_for_port(struct fw_ohci *ohci, int port_index) +static int get_status_for_port(struct fw_ohci *ohci, int port_index, + enum phy_packet_self_id_port_status *status) { int reg; @@ -1860,33 +1861,44 @@ static int get_status_for_port(struct fw_ohci *ohci, int port_index) switch (reg & 0x0f) { case 0x06: - return 2; /* is child node (connected to parent node) */ + // is child node (connected to parent node) + *status = PHY_PACKET_SELF_ID_PORT_STATUS_PARENT; + break; case 0x0e: - return 3; /* is parent node (connected to child node) */ + // is parent node (connected to child node) + *status = PHY_PACKET_SELF_ID_PORT_STATUS_CHILD; + break; + default: + // not connected + *status = PHY_PACKET_SELF_ID_PORT_STATUS_NCONN; + break; } - return 1; /* not connected */ + + return 0; } static int get_self_id_pos(struct fw_ohci *ohci, u32 self_id, int self_id_count) { + unsigned int left_phy_id = phy_packet_self_id_get_phy_id(self_id); int i; - u32 entry; for (i = 0; i < self_id_count; i++) { - entry = ohci->self_id_buffer[i]; - if ((self_id & 0xff000000) == (entry & 0xff000000)) + u32 entry = ohci->self_id_buffer[i]; + unsigned int right_phy_id = phy_packet_self_id_get_phy_id(entry); + + if (left_phy_id == right_phy_id) return -1; - if ((self_id & 0xff000000) < (entry & 0xff000000)) + if (left_phy_id < right_phy_id) return i; } return i; } -static int initiated_reset(struct fw_ohci *ohci) +static bool initiated_reset(struct fw_ohci *ohci) { int reg; - int ret = 0; + int ret = false; mutex_lock(&ohci->phy_reg_mutex); reg = write_phy_reg(ohci, 7, 0xe0); /* Select page 7 */ @@ -1899,7 +1911,7 @@ static int initiated_reset(struct fw_ohci *ohci) if (reg >= 0) { if ((reg & 0x08) == 0x08) { /* bit 3 indicates "initiated reset" */ - ret = 0x2; + ret = true; } } } @@ -1915,9 +1927,14 @@ static int initiated_reset(struct fw_ohci *ohci) */ static int find_and_insert_self_id(struct fw_ohci *ohci, int self_id_count) { - int reg, i, pos, status; - /* link active 1, speed 3, bridge 0, contender 1, more packets 0 */ - u32 self_id = 0x8040c800; + int reg, i, pos; + u32 self_id = 0; + + // link active 1, speed 3, bridge 0, contender 1, more packets 0. + phy_packet_set_packet_identifier(&self_id, PHY_PACKET_PACKET_IDENTIFIER_SELF_ID); + phy_packet_self_id_zero_set_link_active(&self_id, true); + phy_packet_self_id_zero_set_scode(&self_id, SCODE_800); + phy_packet_self_id_zero_set_contender(&self_id, true); reg = reg_read(ohci, OHCI1394_NodeID); if (!(reg & OHCI1394_NodeID_idValid)) { @@ -1925,26 +1942,30 @@ static int find_and_insert_self_id(struct fw_ohci *ohci, int self_id_count) "node ID not valid, new bus reset in progress\n"); return -EBUSY; } - self_id |= ((reg & 0x3f) << 24); /* phy ID */ + phy_packet_self_id_set_phy_id(&self_id, reg & 0x3f); reg = ohci_read_phy_reg(&ohci->card, 4); if (reg < 0) return reg; - self_id |= ((reg & 0x07) << 8); /* power class */ + phy_packet_self_id_zero_set_power_class(&self_id, reg & 0x07); reg = ohci_read_phy_reg(&ohci->card, 1); if (reg < 0) return reg; - self_id |= ((reg & 0x3f) << 16); /* gap count */ + phy_packet_self_id_zero_set_gap_count(&self_id, reg & 0x3f); for (i = 0; i < 3; i++) { - status = get_status_for_port(ohci, i); - if (status < 0) - return status; - self_id |= ((status & 0x3) << (6 - (i * 2))); + enum phy_packet_self_id_port_status status; + int err; + + err = get_status_for_port(ohci, i, &status); + if (err < 0) + return err; + + self_id_sequence_set_port_status(&self_id, 1, i, status); } - self_id |= initiated_reset(ohci); + phy_packet_self_id_zero_set_initiated_reset(&self_id, initiated_reset(ohci)); pos = get_self_id_pos(ohci, self_id, self_id_count); if (pos >= 0) { From 1ccfd1a4c809d99891142c1bb9851daa42e40f0d Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Thu, 6 Jun 2024 08:51:54 +0900 Subject: [PATCH 10/32] firewire: core: arrangement header inclusion for tracepoints events It is a bit inconvenient to put the relative path to local header from tree-wide header. This commit delegates the selection to include headers into users. Link: https://lore.kernel.org/r/20240605235155.116468-11-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-trace.c | 3 +++ drivers/firewire/core-transaction.c | 2 +- drivers/firewire/packet-header-definitions.h | 2 ++ include/trace/events/firewire.h | 2 +- 4 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/firewire/core-trace.c b/drivers/firewire/core-trace.c index 96cbd9d384dc..7cbf850f3719 100644 --- a/drivers/firewire/core-trace.c +++ b/drivers/firewire/core-trace.c @@ -1,5 +1,8 @@ // SPDX-License-Identifier: GPL-2.0-or-later // Copyright (c) 2024 Takashi Sakamoto +#include +#include "packet-header-definitions.h" + #define CREATE_TRACE_POINTS #include diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c index 76ab6a209768..8410e7b4ab60 100644 --- a/drivers/firewire/core-transaction.c +++ b/drivers/firewire/core-transaction.c @@ -29,8 +29,8 @@ #include #include "core.h" -#include #include "packet-header-definitions.h" +#include #define HEADER_DESTINATION_IS_BROADCAST(header) \ ((async_header_get_destination(header) & 0x3f) == 0x3f) diff --git a/drivers/firewire/packet-header-definitions.h b/drivers/firewire/packet-header-definitions.h index ab9d0fa790d4..87a5a31845c3 100644 --- a/drivers/firewire/packet-header-definitions.h +++ b/drivers/firewire/packet-header-definitions.h @@ -7,6 +7,8 @@ #ifndef _FIREWIRE_PACKET_HEADER_DEFINITIONS_H #define _FIREWIRE_PACKET_HEADER_DEFINITIONS_H +#include + #define ASYNC_HEADER_QUADLET_COUNT 4 #define ASYNC_HEADER_Q0_DESTINATION_SHIFT 16 diff --git a/include/trace/events/firewire.h b/include/trace/events/firewire.h index 5ccc0d91b220..1a18533778f1 100644 --- a/include/trace/events/firewire.h +++ b/include/trace/events/firewire.h @@ -11,7 +11,7 @@ #include -#include "../../../drivers/firewire/packet-header-definitions.h" +// Some macros are defined in 'drivers/firewire/packet-header-definitions.h'. // The content of TP_printk field is preprocessed, then put to the module binary. #define ASYNC_HEADER_GET_DESTINATION(header) \ From 677ceae190732ee844f940d1e4860af4022d2be3 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Thu, 6 Jun 2024 08:51:55 +0900 Subject: [PATCH 11/32] firewire: core: add tracepoints event for self_id_sequence It is helpful to trace the content of self ID sequence when the core function building bus topology. This commit adds a tracepoints event fot the purpose. It seems not to achieve printing variable length of array in print time without any storage, thus the structure of event includes a superfluous array to store the state of port. Additionally, there is no helper function to print symbol array, thus the state of port is printed as raw value. Link: https://lore.kernel.org/r/20240605235155.116468-12-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-topology.c | 7 ++-- drivers/firewire/core-trace.c | 15 ++++++++ include/trace/events/firewire.h | 59 ++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 2 deletions(-) diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c index e17a01345857..04e24d14e031 100644 --- a/drivers/firewire/core-topology.c +++ b/drivers/firewire/core-topology.c @@ -95,7 +95,8 @@ static inline struct fw_node *fw_node(struct list_head *l) * internally consistent. On success this function returns the * fw_node corresponding to the local card otherwise NULL. */ -static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self_id_count) +static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self_id_count, + unsigned int generation) { struct self_id_sequence_enumerator enumerator = { .cursor = sid, @@ -139,6 +140,8 @@ static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self } port_capacity = self_id_sequence_get_port_capacity(quadlet_count); + trace_self_id_sequence(self_id_sequence, quadlet_count, generation); + for (port_index = 0; port_index < port_capacity; ++port_index) { port_status = self_id_sequence_get_port_status(self_id_sequence, quadlet_count, port_index); @@ -482,7 +485,7 @@ void fw_core_handle_bus_reset(struct fw_card *card, int node_id, int generation, card->bm_abdicate = bm_abdicate; fw_schedule_bm_work(card, 0); - local_node = build_tree(card, self_ids, self_id_count); + local_node = build_tree(card, self_ids, self_id_count, generation); update_topology_map(card, self_ids, self_id_count); diff --git a/drivers/firewire/core-trace.c b/drivers/firewire/core-trace.c index 7cbf850f3719..c9bc4990d66e 100644 --- a/drivers/firewire/core-trace.c +++ b/drivers/firewire/core-trace.c @@ -2,7 +2,22 @@ // Copyright (c) 2024 Takashi Sakamoto #include +#include #include "packet-header-definitions.h" +#include "phy-packet-definitions.h" #define CREATE_TRACE_POINTS #include + +#ifdef TRACEPOINTS_ENABLED +void copy_port_status(u8 *port_status, unsigned int port_capacity, + const u32 *self_id_sequence, unsigned int quadlet_count) +{ + unsigned int port_index; + + for (port_index = 0; port_index < port_capacity; ++port_index) { + port_status[port_index] = + self_id_sequence_get_port_status(self_id_sequence, quadlet_count, port_index); + } +} +#endif diff --git a/include/trace/events/firewire.h b/include/trace/events/firewire.h index 1a18533778f1..1d0d63b0f8e7 100644 --- a/include/trace/events/firewire.h +++ b/include/trace/events/firewire.h @@ -366,6 +366,65 @@ TRACE_EVENT(bus_reset_handle, ) ); +// Some macros are defined in 'drivers/firewire/phy-packet-definitions.h'. + +// The content of TP_printk field is preprocessed, then put to the module binary. + +#define PHY_PACKET_SELF_ID_GET_PHY_ID(quads) \ + ((((const u32 *)quads)[0] & SELF_ID_PHY_ID_MASK) >> SELF_ID_PHY_ID_SHIFT) + +#define PHY_PACKET_SELF_ID_GET_LINK_ACTIVE(quads) \ + ((((const u32 *)quads)[0] & SELF_ID_ZERO_LINK_ACTIVE_MASK) >> SELF_ID_ZERO_LINK_ACTIVE_SHIFT) + +#define PHY_PACKET_SELF_ID_GET_GAP_COUNT(quads) \ + ((((const u32 *)quads)[0] & SELF_ID_ZERO_GAP_COUNT_MASK) >> SELF_ID_ZERO_GAP_COUNT_SHIFT) + +#define PHY_PACKET_SELF_ID_GET_SCODE(quads) \ + ((((const u32 *)quads)[0] & SELF_ID_ZERO_SCODE_MASK) >> SELF_ID_ZERO_SCODE_SHIFT) + +#define PHY_PACKET_SELF_ID_GET_CONTENDER(quads) \ + ((((const u32 *)quads)[0] & SELF_ID_ZERO_CONTENDER_MASK) >> SELF_ID_ZERO_CONTENDER_SHIFT) + +#define PHY_PACKET_SELF_ID_GET_POWER_CLASS(quads) \ + ((((const u32 *)quads)[0] & SELF_ID_ZERO_POWER_CLASS_MASK) >> SELF_ID_ZERO_POWER_CLASS_SHIFT) + +#define PHY_PACKET_SELF_ID_GET_INITIATED_RESET(quads) \ + ((((const u32 *)quads)[0] & SELF_ID_ZERO_INITIATED_RESET_MASK) >> SELF_ID_ZERO_INITIATED_RESET_SHIFT) + +void copy_port_status(u8 *port_status, unsigned int port_capacity, const u32 *self_id_sequence, + unsigned int quadlet_count); + +TRACE_EVENT(self_id_sequence, + TP_PROTO(const u32 *self_id_sequence, unsigned int quadlet_count, unsigned int generation), + TP_ARGS(self_id_sequence, quadlet_count, generation), + TP_STRUCT__entry( + __field(u8, generation) + __dynamic_array(u8, port_status, self_id_sequence_get_port_capacity(quadlet_count)) + __dynamic_array(u32, self_id_sequence, quadlet_count) + ), + TP_fast_assign( + __entry->generation = generation; + copy_port_status(__get_dynamic_array(port_status), __get_dynamic_array_len(port_status), + self_id_sequence, quadlet_count); + memcpy(__get_dynamic_array(self_id_sequence), self_id_sequence, + __get_dynamic_array_len(self_id_sequence)); + ), + TP_printk( + "generation=%u phy_id=0x%02x link_active=%s gap_count=%u scode=%u contender=%s power_class=%u initiated_reset=%s port_status=%s self_id_sequence=%s", + __entry->generation, + PHY_PACKET_SELF_ID_GET_PHY_ID(__get_dynamic_array(self_id_sequence)), + PHY_PACKET_SELF_ID_GET_LINK_ACTIVE(__get_dynamic_array(self_id_sequence)) ? "true" : "false", + PHY_PACKET_SELF_ID_GET_GAP_COUNT(__get_dynamic_array(self_id_sequence)), + PHY_PACKET_SELF_ID_GET_SCODE(__get_dynamic_array(self_id_sequence)), + PHY_PACKET_SELF_ID_GET_CONTENDER(__get_dynamic_array(self_id_sequence)) ? "true" : "false", + PHY_PACKET_SELF_ID_GET_POWER_CLASS(__get_dynamic_array(self_id_sequence)), + PHY_PACKET_SELF_ID_GET_INITIATED_RESET(__get_dynamic_array(self_id_sequence)) ? "true" : "false", + __print_array(__get_dynamic_array(port_status), __get_dynamic_array_len(port_status), 1), + __print_array(__get_dynamic_array(self_id_sequence), + __get_dynamic_array_len(self_id_sequence) / QUADLET_SIZE, QUADLET_SIZE) + ) +); + #undef QUADLET_SIZE #endif // _FIREWIRE_TRACE_EVENT_H From 75d47101fa37e60570bd97debd18e5519272d233 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 7 Jun 2024 08:51:32 +0900 Subject: [PATCH 12/32] firewire: core: add tests for serialization/deserialization of phy config packet In the protocol of IEEE 1394, phy configuration packet is broadcasted to the bus to configure all PHYs residing on the bus. It includes two purposes; selecting root node and optimizing gap count. This commit adds some helper function to serialize/deserialize the content of phy configuration packet, as well as some KUnit tests for it. Link: https://lore.kernel.org/r/20240606235133.231543-2-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/packet-serdes-test.c | 79 +++++++++++++++++++++++ drivers/firewire/phy-packet-definitions.h | 55 ++++++++++++++++ 2 files changed, 134 insertions(+) diff --git a/drivers/firewire/packet-serdes-test.c b/drivers/firewire/packet-serdes-test.c index c56199e84f91..62ba433756ae 100644 --- a/drivers/firewire/packet-serdes-test.c +++ b/drivers/firewire/packet-serdes-test.c @@ -248,6 +248,29 @@ static void deserialize_phy_packet_self_id_extended(u32 quadlet, unsigned int *p *has_more_packets = phy_packet_self_id_get_more_packets(quadlet); } +static void serialize_phy_packet_phy_config(u32 *quadlet, unsigned int packet_identifier, + unsigned int root_id, bool has_force_root_node, + bool has_gap_count_optimization, unsigned int gap_count) +{ + phy_packet_set_packet_identifier(quadlet, packet_identifier); + phy_packet_phy_config_set_root_id(quadlet, root_id); + phy_packet_phy_config_set_force_root_node(quadlet, has_force_root_node); + phy_packet_phy_config_set_gap_count_optimization(quadlet, has_gap_count_optimization); + phy_packet_phy_config_set_gap_count(quadlet, gap_count); +} + +static void deserialize_phy_packet_phy_config(u32 quadlet, unsigned int *packet_identifier, + unsigned int *root_id, bool *has_force_root_node, + bool *has_gap_count_optimization, + unsigned int *gap_count) +{ + *packet_identifier = phy_packet_get_packet_identifier(quadlet); + *root_id = phy_packet_phy_config_get_root_id(quadlet); + *has_force_root_node = phy_packet_phy_config_get_force_root_node(quadlet); + *has_gap_count_optimization = phy_packet_phy_config_get_gap_count_optimization(quadlet); + *gap_count = phy_packet_phy_config_get_gap_count(quadlet); +} + static void test_async_header_write_quadlet_request(struct kunit *test) { static const u32 expected[ASYNC_HEADER_QUADLET_COUNT] = { @@ -811,6 +834,60 @@ static void test_phy_packet_self_id_zero_and_one(struct kunit *test) KUNIT_EXPECT_MEMEQ(test, quadlets, expected, sizeof(expected)); } +static void test_phy_packet_phy_config_force_root_node(struct kunit *test) +{ + const u32 expected = 0x02800000; + u32 quadlet = 0; + + unsigned int packet_identifier; + unsigned int root_id; + bool has_force_root_node; + bool has_gap_count_optimization; + unsigned int gap_count; + + deserialize_phy_packet_phy_config(expected, &packet_identifier, &root_id, + &has_force_root_node, &has_gap_count_optimization, + &gap_count); + + KUNIT_EXPECT_EQ(test, PHY_PACKET_PACKET_IDENTIFIER_PHY_CONFIG, packet_identifier); + KUNIT_EXPECT_EQ(test, 0x02, root_id); + KUNIT_EXPECT_TRUE(test, has_force_root_node); + KUNIT_EXPECT_FALSE(test, has_gap_count_optimization); + KUNIT_EXPECT_EQ(test, 0, gap_count); + + serialize_phy_packet_phy_config(&quadlet, packet_identifier, root_id, has_force_root_node, + has_gap_count_optimization, gap_count); + + KUNIT_EXPECT_EQ(test, quadlet, expected); +} + +static void test_phy_packet_phy_config_gap_count_optimization(struct kunit *test) +{ + const u32 expected = 0x034f0000; + u32 quadlet = 0; + + unsigned int packet_identifier; + unsigned int root_id; + bool has_force_root_node; + bool has_gap_count_optimization; + unsigned int gap_count; + + deserialize_phy_packet_phy_config(expected, &packet_identifier, &root_id, + &has_force_root_node, &has_gap_count_optimization, + &gap_count); + + KUNIT_EXPECT_EQ(test, PHY_PACKET_PACKET_IDENTIFIER_PHY_CONFIG, packet_identifier); + KUNIT_EXPECT_EQ(test, 0x03, root_id); + KUNIT_EXPECT_FALSE(test, has_force_root_node); + KUNIT_EXPECT_TRUE(test, has_gap_count_optimization); + KUNIT_EXPECT_EQ(test, 0x0f, gap_count); + + serialize_phy_packet_phy_config(&quadlet, packet_identifier, root_id, has_force_root_node, + has_gap_count_optimization, gap_count); + + KUNIT_EXPECT_EQ(test, quadlet, 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), @@ -825,6 +902,8 @@ static struct kunit_case packet_serdes_test_cases[] = { KUNIT_CASE(test_phy_packet_self_id_zero_case0), KUNIT_CASE(test_phy_packet_self_id_zero_case1), KUNIT_CASE(test_phy_packet_self_id_zero_and_one), + KUNIT_CASE(test_phy_packet_phy_config_force_root_node), + KUNIT_CASE(test_phy_packet_phy_config_gap_count_optimization), {} }; diff --git a/drivers/firewire/phy-packet-definitions.h b/drivers/firewire/phy-packet-definitions.h index 8f78494ad371..03c7c606759f 100644 --- a/drivers/firewire/phy-packet-definitions.h +++ b/drivers/firewire/phy-packet-definitions.h @@ -21,6 +21,61 @@ static inline void phy_packet_set_packet_identifier(u32 *quadlet, unsigned int p *quadlet |= (packet_identifier << PACKET_IDENTIFIER_SHIFT) & PACKET_IDENTIFIER_MASK; } +#define PHY_PACKET_PACKET_IDENTIFIER_PHY_CONFIG 0 + +#define PHY_CONFIG_ROOT_ID_MASK 0x3f000000 +#define PHY_CONFIG_ROOT_ID_SHIFT 24 +#define PHY_CONFIG_FORCE_ROOT_NODE_MASK 0x00800000 +#define PHY_CONFIG_FORCE_ROOT_NODE_SHIFT 23 +#define PHY_CONFIG_GAP_COUNT_OPTIMIZATION_MASK 0x00400000 +#define PHY_CONFIG_GAP_COUNT_OPTIMIZATION_SHIFT 22 +#define PHY_CONFIG_GAP_COUNT_MASK 0x003f0000 +#define PHY_CONFIG_GAP_COUNT_SHIFT 16 + +static inline unsigned int phy_packet_phy_config_get_root_id(u32 quadlet) +{ + return (quadlet & PHY_CONFIG_ROOT_ID_MASK) >> PHY_CONFIG_ROOT_ID_SHIFT; +} + +static inline void phy_packet_phy_config_set_root_id(u32 *quadlet, unsigned int root_id) +{ + *quadlet &= ~PHY_CONFIG_ROOT_ID_MASK; + *quadlet |= (root_id << PHY_CONFIG_ROOT_ID_SHIFT) & PHY_CONFIG_ROOT_ID_MASK; +} + +static inline bool phy_packet_phy_config_get_force_root_node(u32 quadlet) +{ + return (quadlet & PHY_CONFIG_FORCE_ROOT_NODE_MASK) >> PHY_CONFIG_FORCE_ROOT_NODE_SHIFT; +} + +static inline void phy_packet_phy_config_set_force_root_node(u32 *quadlet, bool has_force_root_node) +{ + *quadlet &= ~PHY_CONFIG_FORCE_ROOT_NODE_MASK; + *quadlet |= (has_force_root_node << PHY_CONFIG_FORCE_ROOT_NODE_SHIFT) & PHY_CONFIG_FORCE_ROOT_NODE_MASK; +} + +static inline bool phy_packet_phy_config_get_gap_count_optimization(u32 quadlet) +{ + return (quadlet & PHY_CONFIG_GAP_COUNT_OPTIMIZATION_MASK) >> PHY_CONFIG_GAP_COUNT_OPTIMIZATION_SHIFT; +} + +static inline void phy_packet_phy_config_set_gap_count_optimization(u32 *quadlet, bool has_gap_count_optimization) +{ + *quadlet &= ~PHY_CONFIG_GAP_COUNT_OPTIMIZATION_MASK; + *quadlet |= (has_gap_count_optimization << PHY_CONFIG_GAP_COUNT_OPTIMIZATION_SHIFT) & PHY_CONFIG_GAP_COUNT_OPTIMIZATION_MASK; +} + +static inline unsigned int phy_packet_phy_config_get_gap_count(u32 quadlet) +{ + return (quadlet & PHY_CONFIG_GAP_COUNT_MASK) >> PHY_CONFIG_GAP_COUNT_SHIFT; +} + +static inline void phy_packet_phy_config_set_gap_count(u32 *quadlet, unsigned int gap_count) +{ + *quadlet &= ~PHY_CONFIG_GAP_COUNT_MASK; + *quadlet |= (gap_count << PHY_CONFIG_GAP_COUNT_SHIFT) & PHY_CONFIG_GAP_COUNT_MASK; +} + #define PHY_PACKET_PACKET_IDENTIFIER_SELF_ID 2 #define SELF_ID_PHY_ID_MASK 0x3f000000 From 67e51784c1974e0db8457ef5bcc179b9511bce22 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 7 Jun 2024 08:51:33 +0900 Subject: [PATCH 13/32] firewire: core: use inline helper functions to serialize phy config packet This commit uses the added helper functions to obsolete the existing implementation for phy configuration packet. Link: https://lore.kernel.org/r/20240606235133.231543-3-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-transaction.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c index 8410e7b4ab60..4d2fc1f31fec 100644 --- a/drivers/firewire/core-transaction.c +++ b/drivers/firewire/core-transaction.c @@ -30,19 +30,12 @@ #include "core.h" #include "packet-header-definitions.h" +#include "phy-packet-definitions.h" #include #define HEADER_DESTINATION_IS_BROADCAST(header) \ ((async_header_get_destination(header) & 0x3f) == 0x3f) -#define PHY_PACKET_CONFIG 0x0 -#define PHY_PACKET_LINK_ON 0x1 -#define PHY_PACKET_SELF_ID 0x2 - -#define PHY_CONFIG_GAP_COUNT(gap_count) (((gap_count) << 16) | (1 << 22)) -#define PHY_CONFIG_ROOT_ID(node_id) ((((node_id) & 0x3f) << 24) | (1 << 23)) -#define PHY_IDENTIFIER(id) ((id) << 30) - /* returns 0 if the split timeout handler is already running */ static int try_cancel_split_timeout(struct fw_transaction *t) { @@ -481,10 +474,14 @@ void fw_send_phy_config(struct fw_card *card, int node_id, int generation, int gap_count) { long timeout = DIV_ROUND_UP(HZ, 10); - u32 data = PHY_IDENTIFIER(PHY_PACKET_CONFIG); + u32 data = 0; - if (node_id != FW_PHY_CONFIG_NO_NODE_ID) - data |= PHY_CONFIG_ROOT_ID(node_id); + phy_packet_set_packet_identifier(&data, PHY_PACKET_PACKET_IDENTIFIER_PHY_CONFIG); + + if (node_id != FW_PHY_CONFIG_NO_NODE_ID) { + phy_packet_phy_config_set_root_id(&data, node_id); + phy_packet_phy_config_set_force_root_node(&data, true); + } if (gap_count == FW_PHY_CONFIG_CURRENT_GAP_COUNT) { gap_count = card->driver->read_phy_reg(card, 1); @@ -495,7 +492,8 @@ void fw_send_phy_config(struct fw_card *card, if (gap_count == 63) return; } - data |= PHY_CONFIG_GAP_COUNT(gap_count); + phy_packet_phy_config_set_gap_count(&data, gap_count); + phy_packet_phy_config_set_gap_count_optimization(&data, true); mutex_lock(&phy_config_mutex); From 2fd22faf0e9f120c39e6f47e5622b0039bb10817 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 14 Jun 2024 09:42:51 +0900 Subject: [PATCH 14/32] firewire: core: record card index in tracepoints event for self ID sequence This patch is for for-next branch. The selfIDComplete event occurs in the bus managed by one of 1394 OHCI controller in Linux system, while the existing tracepoints events has the lack of data about it to distinguish the issued hardware from the others. This commit adds card_index member into event structure to store the index of host controller in use, and prints it. Link: https://lore.kernel.org/r/20240614004251.460649-1-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-topology.c | 2 +- include/trace/events/firewire.h | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c index 04e24d14e031..b4e637aa6932 100644 --- a/drivers/firewire/core-topology.c +++ b/drivers/firewire/core-topology.c @@ -140,7 +140,7 @@ static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self } port_capacity = self_id_sequence_get_port_capacity(quadlet_count); - trace_self_id_sequence(self_id_sequence, quadlet_count, generation); + trace_self_id_sequence(card->index, self_id_sequence, quadlet_count, generation); for (port_index = 0; port_index < port_capacity; ++port_index) { port_status = self_id_sequence_get_port_status(self_id_sequence, quadlet_count, diff --git a/include/trace/events/firewire.h b/include/trace/events/firewire.h index 1d0d63b0f8e7..d237a30d197b 100644 --- a/include/trace/events/firewire.h +++ b/include/trace/events/firewire.h @@ -395,14 +395,16 @@ void copy_port_status(u8 *port_status, unsigned int port_capacity, const u32 *se unsigned int quadlet_count); TRACE_EVENT(self_id_sequence, - TP_PROTO(const u32 *self_id_sequence, unsigned int quadlet_count, unsigned int generation), - TP_ARGS(self_id_sequence, quadlet_count, generation), + TP_PROTO(unsigned int card_index, const u32 *self_id_sequence, unsigned int quadlet_count, unsigned int generation), + TP_ARGS(card_index, self_id_sequence, quadlet_count, generation), TP_STRUCT__entry( + __field(u8, card_index) __field(u8, generation) __dynamic_array(u8, port_status, self_id_sequence_get_port_capacity(quadlet_count)) __dynamic_array(u32, self_id_sequence, quadlet_count) ), TP_fast_assign( + __entry->card_index = card_index; __entry->generation = generation; copy_port_status(__get_dynamic_array(port_status), __get_dynamic_array_len(port_status), self_id_sequence, quadlet_count); @@ -410,7 +412,8 @@ TRACE_EVENT(self_id_sequence, __get_dynamic_array_len(self_id_sequence)); ), TP_printk( - "generation=%u phy_id=0x%02x link_active=%s gap_count=%u scode=%u contender=%s power_class=%u initiated_reset=%s port_status=%s self_id_sequence=%s", + "card_index=%u generation=%u phy_id=0x%02x link_active=%s gap_count=%u scode=%u contender=%s power_class=%u initiated_reset=%s port_status=%s self_id_sequence=%s", + __entry->card_index, __entry->generation, PHY_PACKET_SELF_ID_GET_PHY_ID(__get_dynamic_array(self_id_sequence)), PHY_PACKET_SELF_ID_GET_LINK_ACTIVE(__get_dynamic_array(self_id_sequence)) ? "true" : "false", From ae24ba769b5c108b00cdf49ed39b189eb012fbb8 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Sun, 23 Jun 2024 17:39:00 +0900 Subject: [PATCH 15/32] firewire: core: undefine macros after use in tracepoints events Some macros are defined in tracepoints events. They should be back to undefined state after use. Link: https://lore.kernel.org/r/20240623083900.777897-1-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- include/trace/events/firewire.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/include/trace/events/firewire.h b/include/trace/events/firewire.h index d237a30d197b..25289a063deb 100644 --- a/include/trace/events/firewire.h +++ b/include/trace/events/firewire.h @@ -428,6 +428,14 @@ TRACE_EVENT(self_id_sequence, ) ); +#undef PHY_PACKET_SELF_ID_GET_PHY_ID +#undef PHY_PACKET_SELF_ID_GET_LINK_ACTIVE +#undef PHY_PACKET_SELF_ID_GET_GAP_COUNT +#undef PHY_PACKET_SELF_ID_GET_SCODE +#undef PHY_PACKET_SELF_ID_GET_CONTENDER +#undef PHY_PACKET_SELF_ID_GET_POWER_CLASS +#undef PHY_PACKET_SELF_ID_GET_INITIATED_RESET + #undef QUADLET_SIZE #endif // _FIREWIRE_TRACE_EVENT_H From 25e6e00d3f78d73c30ee78fee8060cf68908be50 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Mon, 24 Jun 2024 07:08:52 +0900 Subject: [PATCH 16/32] firewire: core: add tracepoints events for allocation/deallocation of isochronous context It is helpful to trace the allocation and dealocation of isochronous when the core function is requested them by both in-kernel unit drivers and userspace applications. This commit adds some tracepoints events for the aim. Link: https://lore.kernel.org/r/20240623220859.851685-2-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-iso.c | 10 +++ include/trace/events/firewire.h | 105 ++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+) diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c index af70e74f9a7e..ddb999297c72 100644 --- a/drivers/firewire/core-iso.c +++ b/drivers/firewire/core-iso.c @@ -22,6 +22,8 @@ #include "core.h" +#include + /* * Isochronous DMA context management */ @@ -148,12 +150,20 @@ struct fw_iso_context *fw_iso_context_create(struct fw_card *card, ctx->callback.sc = callback; ctx->callback_data = callback_data; + trace_isoc_outbound_allocate(ctx, channel, speed); + trace_isoc_inbound_single_allocate(ctx, channel, header_size); + trace_isoc_inbound_multiple_allocate(ctx); + return ctx; } EXPORT_SYMBOL(fw_iso_context_create); void fw_iso_context_destroy(struct fw_iso_context *ctx) { + trace_isoc_outbound_destroy(ctx); + trace_isoc_inbound_single_destroy(ctx); + trace_isoc_inbound_multiple_destroy(ctx); + ctx->card->driver->free_iso_context(ctx); } EXPORT_SYMBOL(fw_iso_context_destroy); diff --git a/include/trace/events/firewire.h b/include/trace/events/firewire.h index 25289a063deb..0cca1ef841f3 100644 --- a/include/trace/events/firewire.h +++ b/include/trace/events/firewire.h @@ -436,6 +436,111 @@ TRACE_EVENT(self_id_sequence, #undef PHY_PACKET_SELF_ID_GET_POWER_CLASS #undef PHY_PACKET_SELF_ID_GET_INITIATED_RESET +TRACE_EVENT_CONDITION(isoc_outbound_allocate, + TP_PROTO(const struct fw_iso_context *ctx, unsigned int channel, unsigned int scode), + TP_ARGS(ctx, channel, scode), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_TRANSMIT), + TP_STRUCT__entry( + __field(u64, context) + __field(u8, card_index) + __field(u8, channel) + __field(u8, scode) + ), + TP_fast_assign( + __entry->context = (uintptr_t)ctx; + __entry->card_index = ctx->card->index; + __entry->channel = channel; + __entry->scode = scode; + ), + TP_printk( + "context=0x%llx card_index=%u channel=%u scode=%u", + __entry->context, + __entry->card_index, + __entry->channel, + __entry->scode + ) +); + +TRACE_EVENT_CONDITION(isoc_inbound_single_allocate, + TP_PROTO(const struct fw_iso_context *ctx, unsigned int channel, unsigned int header_size), + TP_ARGS(ctx, channel, header_size), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_RECEIVE), + TP_STRUCT__entry( + __field(u64, context) + __field(u8, card_index) + __field(u8, channel) + __field(u8, header_size) + ), + TP_fast_assign( + __entry->context = (uintptr_t)ctx; + __entry->card_index = ctx->card->index; + __entry->channel = channel; + __entry->header_size = header_size; + ), + TP_printk( + "context=0x%llx card_index=%u channel=%u header_size=%u", + __entry->context, + __entry->card_index, + __entry->channel, + __entry->header_size + ) +); + +TRACE_EVENT_CONDITION(isoc_inbound_multiple_allocate, + TP_PROTO(const struct fw_iso_context *ctx), + TP_ARGS(ctx), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL), + TP_STRUCT__entry( + __field(u64, context) + __field(u8, card_index) + ), + TP_fast_assign( + __entry->context = (uintptr_t)ctx; + __entry->card_index = ctx->card->index; + ), + TP_printk( + "context=0x%llx card_index=%u", + __entry->context, + __entry->card_index + ) +); + +DECLARE_EVENT_CLASS(isoc_destroy_template, + TP_PROTO(const struct fw_iso_context *ctx), + TP_ARGS(ctx), + TP_STRUCT__entry( + __field(u64, context) + __field(u8, card_index) + ), + TP_fast_assign( + __entry->context = (uintptr_t)ctx; + __entry->card_index = ctx->card->index; + ), + TP_printk( + "context=0x%llx card_index=%u", + __entry->context, + __entry->card_index + ) +) + +DEFINE_EVENT_CONDITION(isoc_destroy_template, isoc_outbound_destroy, + TP_PROTO(const struct fw_iso_context *ctx), + TP_ARGS(ctx), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_TRANSMIT) +); + +DEFINE_EVENT_CONDITION(isoc_destroy_template, isoc_inbound_single_destroy, + TP_PROTO(const struct fw_iso_context *ctx), + TP_ARGS(ctx), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_RECEIVE) +); + +DEFINE_EVENT_CONDITION(isoc_destroy_template, isoc_inbound_multiple_destroy, + TP_PROTO(const struct fw_iso_context *ctx), + TP_ARGS(ctx), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL) +); + #undef QUADLET_SIZE #endif // _FIREWIRE_TRACE_EVENT_H From 9f16ac725b233c53e2a043aa8d1b6f0453e94556 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Mon, 24 Jun 2024 07:08:53 +0900 Subject: [PATCH 17/32] firewire: core: add tracepoints events for setting channels of multichannel context It is helpful to trace the channel setting for the multichannel isochronous context when the core function is requested it by both in-kernel unit drivers and userspace applications. This commit adds some tracepoints events for the aim. Link: https://lore.kernel.org/r/20240623220859.851685-3-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-iso.c | 2 ++ include/trace/events/firewire.h | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c index ddb999297c72..7ec84de846e6 100644 --- a/drivers/firewire/core-iso.c +++ b/drivers/firewire/core-iso.c @@ -177,6 +177,8 @@ EXPORT_SYMBOL(fw_iso_context_start); int fw_iso_context_set_channels(struct fw_iso_context *ctx, u64 *channels) { + trace_isoc_inbound_multiple_channels(ctx, *channels); + return ctx->card->driver->set_iso_channels(ctx, channels); } diff --git a/include/trace/events/firewire.h b/include/trace/events/firewire.h index 0cca1ef841f3..0334e02f9d94 100644 --- a/include/trace/events/firewire.h +++ b/include/trace/events/firewire.h @@ -541,6 +541,27 @@ DEFINE_EVENT_CONDITION(isoc_destroy_template, isoc_inbound_multiple_destroy, TP_CONDITION(ctx->type == FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL) ); +TRACE_EVENT(isoc_inbound_multiple_channels, + TP_PROTO(const struct fw_iso_context *ctx, u64 channels), + TP_ARGS(ctx, channels), + TP_STRUCT__entry( + __field(u64, context) + __field(u8, card_index) + __field(u64, channels) + ), + TP_fast_assign( + __entry->context = (uintptr_t)ctx; + __entry->card_index = ctx->card->index; + __entry->channels = channels; + ), + TP_printk( + "context=0x%llx card_index=%u channels=0x%016llx", + __entry->context, + __entry->card_index, + __entry->channels + ) +); + #undef QUADLET_SIZE #endif // _FIREWIRE_TRACE_EVENT_H From 4e64210f67126e77d54ffcd3297a87539b858322 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Mon, 24 Jun 2024 07:08:54 +0900 Subject: [PATCH 18/32] firewire: core: add tracepoints events for starting/stopping of isochronous context It is helpful to trace the starting and stopping of isochronous context when the core function is requested them by both in-kernel unit drivers and userspace applications. This commit adds some tracepoints events for the aim. Link: https://lore.kernel.org/r/20240623220859.851685-4-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-iso.c | 8 +++ include/trace/events/firewire.h | 108 ++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+) diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c index 7ec84de846e6..49feb4c28224 100644 --- a/drivers/firewire/core-iso.c +++ b/drivers/firewire/core-iso.c @@ -171,6 +171,10 @@ EXPORT_SYMBOL(fw_iso_context_destroy); int fw_iso_context_start(struct fw_iso_context *ctx, int cycle, int sync, int tags) { + trace_isoc_outbound_start(ctx, cycle); + trace_isoc_inbound_single_start(ctx, cycle, sync, tags); + trace_isoc_inbound_multiple_start(ctx, cycle, sync, tags); + return ctx->card->driver->start_iso(ctx, cycle, sync, tags); } EXPORT_SYMBOL(fw_iso_context_start); @@ -205,6 +209,10 @@ EXPORT_SYMBOL(fw_iso_context_flush_completions); int fw_iso_context_stop(struct fw_iso_context *ctx) { + trace_isoc_outbound_stop(ctx); + trace_isoc_inbound_single_stop(ctx); + trace_isoc_inbound_multiple_stop(ctx); + return ctx->card->driver->stop_iso(ctx); } EXPORT_SYMBOL(fw_iso_context_stop); diff --git a/include/trace/events/firewire.h b/include/trace/events/firewire.h index 0334e02f9d94..c984b7d6380f 100644 --- a/include/trace/events/firewire.h +++ b/include/trace/events/firewire.h @@ -562,6 +562,114 @@ TRACE_EVENT(isoc_inbound_multiple_channels, ) ); +TRACE_EVENT_CONDITION(isoc_outbound_start, + TP_PROTO(const struct fw_iso_context *ctx, int cycle_match), + TP_ARGS(ctx, cycle_match), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_TRANSMIT), + TP_STRUCT__entry( + __field(u64, context) + __field(u8, card_index) + __field(bool, cycle_match) + __field(u16, cycle) + ), + TP_fast_assign( + __entry->context = (uintptr_t)ctx; + __entry->card_index = ctx->card->index; + __entry->cycle_match = cycle_match < 0 ? false : true; + __entry->cycle = __entry->cycle_match ? (u16)cycle_match : 0; + ), + TP_printk( + "context=0x%llx card_index=%u cycle_match=%s cycle=0x%04x", + __entry->context, + __entry->card_index, + __entry->cycle_match ? "true" : "false", + __entry->cycle + ) +); + +DECLARE_EVENT_CLASS(isoc_inbound_start_template, + TP_PROTO(const struct fw_iso_context *ctx, int cycle_match, unsigned int sync, unsigned int tags), + TP_ARGS(ctx, cycle_match, sync, tags), + TP_STRUCT__entry( + __field(u64, context) + __field(u8, card_index) + __field(bool, cycle_match) + __field(u16, cycle) + __field(u8, sync) + __field(u8, tags) + ), + TP_fast_assign( + __entry->context = (uintptr_t)ctx; + __entry->card_index = ctx->card->index; + __entry->cycle_match = cycle_match < 0 ? false : true; + __entry->cycle = __entry->cycle_match ? (u16)cycle_match : 0; + __entry->sync = sync; + __entry->tags = tags; + ), + TP_printk( + "context=0x%llx card_index=%u cycle_match=%s cycle=0x%04x sync=%u tags=%s", + __entry->context, + __entry->card_index, + __entry->cycle_match ? "true" : "false", + __entry->cycle, + __entry->sync, + __print_flags(__entry->tags, "|", + { FW_ISO_CONTEXT_MATCH_TAG0, "0" }, + { FW_ISO_CONTEXT_MATCH_TAG1, "1" }, + { FW_ISO_CONTEXT_MATCH_TAG2, "2" }, + { FW_ISO_CONTEXT_MATCH_TAG3, "3" } + ) + ) +); + +DEFINE_EVENT_CONDITION(isoc_inbound_start_template, isoc_inbound_single_start, + TP_PROTO(const struct fw_iso_context *ctx, int cycle_match, unsigned int sync, unsigned int tags), + TP_ARGS(ctx, cycle_match, sync, tags), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_RECEIVE) +); + +DEFINE_EVENT_CONDITION(isoc_inbound_start_template, isoc_inbound_multiple_start, + TP_PROTO(const struct fw_iso_context *ctx, int cycle_match, unsigned int sync, unsigned int tags), + TP_ARGS(ctx, cycle_match, sync, tags), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL) +); + +DECLARE_EVENT_CLASS(isoc_stop_template, + TP_PROTO(const struct fw_iso_context *ctx), + TP_ARGS(ctx), + TP_STRUCT__entry( + __field(u64, context) + __field(u8, card_index) + ), + TP_fast_assign( + __entry->context = (uintptr_t)ctx; + __entry->card_index = ctx->card->index; + ), + TP_printk( + "context=0x%llx card_index=%u", + __entry->context, + __entry->card_index + ) +) + +DEFINE_EVENT_CONDITION(isoc_stop_template, isoc_outbound_stop, + TP_PROTO(const struct fw_iso_context *ctx), + TP_ARGS(ctx), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_TRANSMIT) +); + +DEFINE_EVENT_CONDITION(isoc_stop_template, isoc_inbound_single_stop, + TP_PROTO(const struct fw_iso_context *ctx), + TP_ARGS(ctx), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_RECEIVE) +); + +DEFINE_EVENT_CONDITION(isoc_stop_template, isoc_inbound_multiple_stop, + TP_PROTO(const struct fw_iso_context *ctx), + TP_ARGS(ctx), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL) +); + #undef QUADLET_SIZE #endif // _FIREWIRE_TRACE_EVENT_H From c0b0ce6c47903ed2b9028933793801b3b4e72ca1 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Mon, 24 Jun 2024 07:08:55 +0900 Subject: [PATCH 19/32] firewire: core: add tracepoints events for flushing of isochronous context It is helpful to trace the flushing of isochronous context when the core function is requested them by both in-kernel unit drivers and userspace applications. This commit adds some tracepoints events for the aim. Link: https://lore.kernel.org/r/20240623220859.851685-5-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-iso.c | 4 ++++ include/trace/events/firewire.h | 36 +++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c index 49feb4c28224..5bad23fcce0c 100644 --- a/drivers/firewire/core-iso.c +++ b/drivers/firewire/core-iso.c @@ -197,6 +197,10 @@ EXPORT_SYMBOL(fw_iso_context_queue); void fw_iso_context_queue_flush(struct fw_iso_context *ctx) { + trace_isoc_outbound_flush(ctx); + trace_isoc_inbound_single_flush(ctx); + trace_isoc_inbound_multiple_flush(ctx); + ctx->card->driver->flush_queue_iso(ctx); } EXPORT_SYMBOL(fw_iso_context_queue_flush); diff --git a/include/trace/events/firewire.h b/include/trace/events/firewire.h index c984b7d6380f..b404e6324671 100644 --- a/include/trace/events/firewire.h +++ b/include/trace/events/firewire.h @@ -670,6 +670,42 @@ DEFINE_EVENT_CONDITION(isoc_stop_template, isoc_inbound_multiple_stop, TP_CONDITION(ctx->type == FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL) ); +DECLARE_EVENT_CLASS(isoc_flush_template, + TP_PROTO(const struct fw_iso_context *ctx), + TP_ARGS(ctx), + TP_STRUCT__entry( + __field(u64, context) + __field(u8, card_index) + ), + TP_fast_assign( + __entry->context = (uintptr_t)ctx; + __entry->card_index = ctx->card->index; + ), + TP_printk( + "context=0x%llx card_index=%u", + __entry->context, + __entry->card_index + ) +); + +DEFINE_EVENT_CONDITION(isoc_flush_template, isoc_outbound_flush, + TP_PROTO(const struct fw_iso_context *ctx), + TP_ARGS(ctx), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_TRANSMIT) +); + +DEFINE_EVENT_CONDITION(isoc_flush_template, isoc_inbound_single_flush, + TP_PROTO(const struct fw_iso_context *ctx), + TP_ARGS(ctx), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_RECEIVE) +); + +DEFINE_EVENT_CONDITION(isoc_flush_template, isoc_inbound_multiple_flush, + TP_PROTO(const struct fw_iso_context *ctx), + TP_ARGS(ctx), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL) +); + #undef QUADLET_SIZE #endif // _FIREWIRE_TRACE_EVENT_H From 8320b63e028f62542942941b51951006403d8b4f Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Mon, 24 Jun 2024 07:08:56 +0900 Subject: [PATCH 20/32] firewire: core: add tracepoints events for flushing completions of isochronous context It is helpful to trace the flushing completions of isochronous context when the core function is requested them by both in-kernel unit drivers and userspace applications. This commit adds some tracepoints events for the aim. Link: https://lore.kernel.org/r/20240623220859.851685-6-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-iso.c | 4 ++++ drivers/firewire/ohci.c | 2 ++ include/trace/events/firewire.h | 36 +++++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+) diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c index 5bad23fcce0c..93ca0dec1805 100644 --- a/drivers/firewire/core-iso.c +++ b/drivers/firewire/core-iso.c @@ -207,6 +207,10 @@ EXPORT_SYMBOL(fw_iso_context_queue_flush); int fw_iso_context_flush_completions(struct fw_iso_context *ctx) { + trace_isoc_outbound_flush_completions(ctx); + trace_isoc_inbound_single_flush_completions(ctx); + trace_isoc_inbound_multiple_flush_completions(ctx); + return ctx->card->driver->flush_iso_completions(ctx); } EXPORT_SYMBOL(fw_iso_context_flush_completions); diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 1f6097a6366c..f35d6e193bcb 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -43,6 +43,8 @@ #include "packet-header-definitions.h" #include "phy-packet-definitions.h" +#include + #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) #define ohci_err(ohci, f, args...) dev_err(ohci->card.device, f, ##args) diff --git a/include/trace/events/firewire.h b/include/trace/events/firewire.h index b404e6324671..ad1546120aa3 100644 --- a/include/trace/events/firewire.h +++ b/include/trace/events/firewire.h @@ -706,6 +706,42 @@ DEFINE_EVENT_CONDITION(isoc_flush_template, isoc_inbound_multiple_flush, TP_CONDITION(ctx->type == FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL) ); +DECLARE_EVENT_CLASS(isoc_flush_completions_template, + TP_PROTO(const struct fw_iso_context *ctx), + TP_ARGS(ctx), + TP_STRUCT__entry( + __field(u64, context) + __field(u8, card_index) + ), + TP_fast_assign( + __entry->context = (uintptr_t)ctx; + __entry->card_index = ctx->card->index; + ), + TP_printk( + "context=0x%llx card_index=%u", + __entry->context, + __entry->card_index + ) +); + +DEFINE_EVENT_CONDITION(isoc_flush_completions_template, isoc_outbound_flush_completions, + TP_PROTO(const struct fw_iso_context *ctx), + TP_ARGS(ctx), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_TRANSMIT) +); + +DEFINE_EVENT_CONDITION(isoc_flush_completions_template, isoc_inbound_single_flush_completions, + TP_PROTO(const struct fw_iso_context *ctx), + TP_ARGS(ctx), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_RECEIVE) +); + +DEFINE_EVENT_CONDITION(isoc_flush_completions_template, isoc_inbound_multiple_flush_completions, + TP_PROTO(const struct fw_iso_context *ctx), + TP_ARGS(ctx), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL) +); + #undef QUADLET_SIZE #endif // _FIREWIRE_TRACE_EVENT_H From 1f3c0d794df6d85834a0664db3658c422ec5c8d8 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Mon, 24 Jun 2024 07:08:57 +0900 Subject: [PATCH 21/32] firewire: core: add tracepoints events for queueing packets of isochronous context It is helpful to trace the queueing packets of isochronous context when the core function is requested them by both in-kernel unit drivers and userspace applications. This commit adds some tracepoints events for the aim. Link: https://lore.kernel.org/r/20240623220859.851685-7-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-iso.c | 4 ++ include/trace/events/firewire.h | 79 +++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c index 93ca0dec1805..b3eda38a36f3 100644 --- a/drivers/firewire/core-iso.c +++ b/drivers/firewire/core-iso.c @@ -191,6 +191,10 @@ int fw_iso_context_queue(struct fw_iso_context *ctx, struct fw_iso_buffer *buffer, unsigned long payload) { + trace_isoc_outbound_queue(ctx, payload, packet); + trace_isoc_inbound_single_queue(ctx, payload, packet); + trace_isoc_inbound_multiple_queue(ctx, payload, packet); + return ctx->card->driver->queue_iso(ctx, packet, buffer, payload); } EXPORT_SYMBOL(fw_iso_context_queue); diff --git a/include/trace/events/firewire.h b/include/trace/events/firewire.h index ad1546120aa3..0381b3ca4d0e 100644 --- a/include/trace/events/firewire.h +++ b/include/trace/events/firewire.h @@ -742,6 +742,85 @@ DEFINE_EVENT_CONDITION(isoc_flush_completions_template, isoc_inbound_multiple_fl TP_CONDITION(ctx->type == FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL) ); +#define TP_STRUCT__entry_iso_packet(ctx, buffer_offset, packet) \ + TP_STRUCT__entry( \ + __field(u64, context) \ + __field(u8, card_index) \ + __field(u32, buffer_offset) \ + __field(bool, interrupt) \ + __field(bool, skip) \ + __field(u8, sy) \ + __field(u8, tag) \ + __dynamic_array(u32, header, packet->header_length / QUADLET_SIZE) \ + ) + +#define TP_fast_assign_iso_packet(ctx, buffer_offset, packet) \ + TP_fast_assign( \ + __entry->context = (uintptr_t)ctx; \ + __entry->card_index = ctx->card->index; \ + __entry->buffer_offset = buffer_offset; \ + __entry->interrupt = packet->interrupt; \ + __entry->skip = packet->skip; \ + __entry->sy = packet->sy; \ + __entry->tag = packet->tag; \ + memcpy(__get_dynamic_array(header), packet->header, \ + __get_dynamic_array_len(header)); \ + ) + +TRACE_EVENT_CONDITION(isoc_outbound_queue, + TP_PROTO(const struct fw_iso_context *ctx, unsigned long buffer_offset, const struct fw_iso_packet *packet), + TP_ARGS(ctx, buffer_offset, packet), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_TRANSMIT), + TP_STRUCT__entry_iso_packet(ctx, buffer_offset, packet), + TP_fast_assign_iso_packet(ctx, buffer_offset, packet), + TP_printk( + "context=0x%llx card_index=%u buffer_offset=0x%x interrupt=%s skip=%s sy=%d tag=%u header=%s", + __entry->context, + __entry->card_index, + __entry->buffer_offset, + __entry->interrupt ? "true" : "false", + __entry->skip ? "true" : "false", + __entry->sy, + __entry->tag, + __print_array(__get_dynamic_array(header), + __get_dynamic_array_len(header) / QUADLET_SIZE, QUADLET_SIZE) + ) +); + +TRACE_EVENT_CONDITION(isoc_inbound_single_queue, + TP_PROTO(const struct fw_iso_context *ctx, unsigned long buffer_offset, const struct fw_iso_packet *packet), + TP_ARGS(ctx, buffer_offset, packet), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_RECEIVE), + TP_STRUCT__entry_iso_packet(ctx, buffer_offset, packet), + TP_fast_assign_iso_packet(ctx, buffer_offset, packet), + TP_printk( + "context=0x%llx card_index=%u buffer_offset=0x%x interrupt=%s skip=%s", + __entry->context, + __entry->card_index, + __entry->buffer_offset, + __entry->interrupt ? "true" : "false", + __entry->skip ? "true" : "false" + ) +); + +TRACE_EVENT_CONDITION(isoc_inbound_multiple_queue, + TP_PROTO(const struct fw_iso_context *ctx, unsigned long buffer_offset, const struct fw_iso_packet *packet), + TP_ARGS(ctx, buffer_offset, packet), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL), + TP_STRUCT__entry_iso_packet(ctx, buffer_offset, packet), + TP_fast_assign_iso_packet(ctx, buffer_offset, packet), + TP_printk( + "context=0x%llx card_index=%u buffer_offset=0x%x interrupt=%s", + __entry->context, + __entry->card_index, + __entry->buffer_offset, + __entry->interrupt ? "true" : "false" + ) +); + +#undef TP_STRUCT__entry_iso_packet +#undef TP_fast_assign_iso_packet + #undef QUADLET_SIZE #endif // _FIREWIRE_TRACE_EVENT_H From daf763c2d6d1dc50586e351aca868c7786d3c84e Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Mon, 24 Jun 2024 07:08:58 +0900 Subject: [PATCH 22/32] firewire: core: add tracepoints events for completions of packets in isochronous context It is helpful to trace completion of packets in isochronous context when the core function is requested them by both in-kernel units driver and userspace applications. This commit adds some tracepoints events for the aim. Link: https://lore.kernel.org/r/20240623220859.851685-8-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-trace.c | 4 ++ drivers/firewire/ohci.c | 23 +++++++--- include/trace/events/firewire.h | 78 +++++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 6 deletions(-) diff --git a/drivers/firewire/core-trace.c b/drivers/firewire/core-trace.c index c9bc4990d66e..5acb02c18a47 100644 --- a/drivers/firewire/core-trace.c +++ b/drivers/firewire/core-trace.c @@ -20,4 +20,8 @@ void copy_port_status(u8 *port_status, unsigned int port_capacity, self_id_sequence_get_port_status(self_id_sequence, quadlet_count, port_index); } } + +EXPORT_TRACEPOINT_SYMBOL_GPL(isoc_inbound_single_completions); +EXPORT_TRACEPOINT_SYMBOL_GPL(isoc_inbound_multiple_completions); +EXPORT_TRACEPOINT_SYMBOL_GPL(isoc_outbound_completions); #endif diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index f35d6e193bcb..bdb206157118 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -2833,8 +2833,13 @@ static void ohci_write_csr(struct fw_card *card, int csr_offset, u32 value) } } -static void flush_iso_completions(struct iso_context *ctx) +static void flush_iso_completions(struct iso_context *ctx, enum fw_iso_context_completions_cause cause) { + trace_isoc_inbound_single_completions(&ctx->base, ctx->last_timestamp, cause, ctx->header, + ctx->header_length); + trace_isoc_outbound_completions(&ctx->base, ctx->last_timestamp, cause, ctx->header, + ctx->header_length); + ctx->base.callback.sc(&ctx->base, ctx->last_timestamp, ctx->header_length, ctx->header, ctx->base.callback_data); @@ -2848,7 +2853,7 @@ static void copy_iso_headers(struct iso_context *ctx, const u32 *dma_hdr) if (ctx->header_length + ctx->base.header_size > PAGE_SIZE) { if (ctx->base.drop_overflow_headers) return; - flush_iso_completions(ctx); + flush_iso_completions(ctx, FW_ISO_CONTEXT_COMPLETIONS_CAUSE_HEADER_OVERFLOW); } ctx_hdr = ctx->header + ctx->header_length; @@ -2897,7 +2902,7 @@ static int handle_ir_packet_per_buffer(struct context *context, copy_iso_headers(ctx, (u32 *) (last + 1)); if (last->control & cpu_to_le16(DESCRIPTOR_IRQ_ALWAYS)) - flush_iso_completions(ctx); + flush_iso_completions(ctx, FW_ISO_CONTEXT_COMPLETIONS_CAUSE_IRQ); return 1; } @@ -2932,6 +2937,9 @@ static int handle_ir_buffer_fill(struct context *context, completed, DMA_FROM_DEVICE); if (last->control & cpu_to_le16(DESCRIPTOR_IRQ_ALWAYS)) { + trace_isoc_inbound_multiple_completions(&ctx->base, completed, + FW_ISO_CONTEXT_COMPLETIONS_CAUSE_IRQ); + ctx->base.callback.mc(&ctx->base, buffer_dma + completed, ctx->base.callback_data); @@ -2948,6 +2956,9 @@ static void flush_ir_buffer_fill(struct iso_context *ctx) ctx->mc_buffer_bus & ~PAGE_MASK, ctx->mc_completed, DMA_FROM_DEVICE); + trace_isoc_inbound_multiple_completions(&ctx->base, ctx->mc_completed, + FW_ISO_CONTEXT_COMPLETIONS_CAUSE_FLUSH); + ctx->base.callback.mc(&ctx->base, ctx->mc_buffer_bus + ctx->mc_completed, ctx->base.callback_data); @@ -3012,7 +3023,7 @@ static int handle_it_packet(struct context *context, if (ctx->header_length + 4 > PAGE_SIZE) { if (ctx->base.drop_overflow_headers) return 1; - flush_iso_completions(ctx); + flush_iso_completions(ctx, FW_ISO_CONTEXT_COMPLETIONS_CAUSE_HEADER_OVERFLOW); } ctx_hdr = ctx->header + ctx->header_length; @@ -3023,7 +3034,7 @@ static int handle_it_packet(struct context *context, ctx->header_length += 4; if (last->control & cpu_to_le16(DESCRIPTOR_IRQ_ALWAYS)) - flush_iso_completions(ctx); + flush_iso_completions(ctx, FW_ISO_CONTEXT_COMPLETIONS_CAUSE_IRQ); return 1; } @@ -3588,7 +3599,7 @@ static int ohci_flush_iso_completions(struct fw_iso_context *base) case FW_ISO_CONTEXT_TRANSMIT: case FW_ISO_CONTEXT_RECEIVE: if (ctx->header_length != 0) - flush_iso_completions(ctx); + flush_iso_completions(ctx, FW_ISO_CONTEXT_COMPLETIONS_CAUSE_FLUSH); break; case FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL: if (ctx->mc_completed != 0) diff --git a/include/trace/events/firewire.h b/include/trace/events/firewire.h index 0381b3ca4d0e..d9158a134beb 100644 --- a/include/trace/events/firewire.h +++ b/include/trace/events/firewire.h @@ -821,6 +821,84 @@ TRACE_EVENT_CONDITION(isoc_inbound_multiple_queue, #undef TP_STRUCT__entry_iso_packet #undef TP_fast_assign_iso_packet +#ifndef show_cause +enum fw_iso_context_completions_cause { + FW_ISO_CONTEXT_COMPLETIONS_CAUSE_FLUSH = 0, + FW_ISO_CONTEXT_COMPLETIONS_CAUSE_IRQ, + FW_ISO_CONTEXT_COMPLETIONS_CAUSE_HEADER_OVERFLOW, +}; +#define show_cause(cause) \ + __print_symbolic(cause, \ + { FW_ISO_CONTEXT_COMPLETIONS_CAUSE_FLUSH, "FLUSH" }, \ + { FW_ISO_CONTEXT_COMPLETIONS_CAUSE_IRQ, "IRQ" }, \ + { FW_ISO_CONTEXT_COMPLETIONS_CAUSE_HEADER_OVERFLOW, "HEADER_OVERFLOW" } \ + ) +#endif + +DECLARE_EVENT_CLASS(isoc_single_completions_template, + TP_PROTO(const struct fw_iso_context *ctx, u16 timestamp, enum fw_iso_context_completions_cause cause, const u32 *header, unsigned int header_length), + TP_ARGS(ctx, timestamp, cause, header, header_length), + TP_STRUCT__entry( + __field(u64, context) + __field(u8, card_index) + __field(u16, timestamp) + __field(u8, cause) + __dynamic_array(u32, header, header_length / QUADLET_SIZE) + ), + TP_fast_assign( + __entry->context = (uintptr_t)ctx; + __entry->card_index = ctx->card->index; + __entry->timestamp = timestamp; + __entry->cause = cause; + memcpy(__get_dynamic_array(header), header, __get_dynamic_array_len(header)); + ), + TP_printk( + "context=0x%llx card_index=%u timestap=0x%04x cause=%s header=%s", + __entry->context, + __entry->card_index, + __entry->timestamp, + show_cause(__entry->cause), + __print_array(__get_dynamic_array(header), + __get_dynamic_array_len(header) / QUADLET_SIZE, QUADLET_SIZE) + ) +) + +DEFINE_EVENT_CONDITION(isoc_single_completions_template, isoc_outbound_completions, + TP_PROTO(const struct fw_iso_context *ctx, u16 timestamp, enum fw_iso_context_completions_cause cause, const u32 *header, unsigned int header_length), + TP_ARGS(ctx, timestamp, cause, header, header_length), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_TRANSMIT) +); + +DEFINE_EVENT_CONDITION(isoc_single_completions_template, isoc_inbound_single_completions, + TP_PROTO(const struct fw_iso_context *ctx, u16 timestamp, enum fw_iso_context_completions_cause cause, const u32 *header, unsigned int header_length), + TP_ARGS(ctx, timestamp, cause, header, header_length), + TP_CONDITION(ctx->type == FW_ISO_CONTEXT_RECEIVE) +); + +TRACE_EVENT(isoc_inbound_multiple_completions, + TP_PROTO(const struct fw_iso_context *ctx, unsigned int completed, enum fw_iso_context_completions_cause cause), + TP_ARGS(ctx, completed, cause), + TP_STRUCT__entry( + __field(u64, context) + __field(u8, card_index) + __field(u16, completed) + __field(u8, cause) + ), + TP_fast_assign( + __entry->context = (uintptr_t)ctx; + __entry->card_index = ctx->card->index; + __entry->completed = completed; + __entry->cause = cause; + ), + TP_printk( + "context=0x%llx card_index=%u comleted=%u cause=%s", + __entry->context, + __entry->card_index, + __entry->completed, + show_cause(__entry->cause) + ) +); + #undef QUADLET_SIZE #endif // _FIREWIRE_TRACE_EVENT_H From 001c1ff5dc397bc26a3d1a0da8733c179f39c946 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Tue, 25 Jun 2024 12:18:05 +0900 Subject: [PATCH 23/32] firewire: ohci: add support for Linux kernel tracepoints The Linux Kernel Tracepoints framework is enough useful to trace the interaction between 1394 OHCI hardware and its driver. This commit adds firewire_ohci subsystem to use the framework. It is defined as the different subsystem from the existing firewire subsystem. The definition file for the existing subsystem is slightly changed so that both subsystems are available in 1394 OHCI driver. Link: https://lore.kernel.org/r/20240625031806.956650-2-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/ohci.c | 3 +++ include/trace/events/firewire.h | 1 + include/trace/events/firewire_ohci.h | 16 ++++++++++++++++ 3 files changed, 20 insertions(+) create mode 100644 include/trace/events/firewire_ohci.h diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index bdb206157118..07adb4ddd444 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -45,6 +45,9 @@ #include +#define CREATE_TRACE_POINTS +#include + #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) #define ohci_err(ohci, f, args...) dev_err(ohci->card.device, f, ##args) diff --git a/include/trace/events/firewire.h b/include/trace/events/firewire.h index d9158a134beb..aa00c9f33551 100644 --- a/include/trace/events/firewire.h +++ b/include/trace/events/firewire.h @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-or-later // Copyright (c) 2024 Takashi Sakamoto +#undef TRACE_SYSTEM #define TRACE_SYSTEM firewire #if !defined(_FIREWIRE_TRACE_EVENT_H) || defined(TRACE_HEADER_MULTI_READ) diff --git a/include/trace/events/firewire_ohci.h b/include/trace/events/firewire_ohci.h new file mode 100644 index 000000000000..67fa3c1c8f6d --- /dev/null +++ b/include/trace/events/firewire_ohci.h @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +// Copyright (c) 2024 Takashi Sakamoto + +#undef TRACE_SYSTEM +#define TRACE_SYSTEM firewire_ohci + +#if !defined(_FIREWIRE_OHCI_TRACE_EVENT_H) || defined(TRACE_HEADER_MULTI_READ) +#define _FIREWIRE_OHCI_TRACE_EVENT_H + +#include + +// Placeholder for future use. + +#endif // _FIREWIRE_OHCI_TRACE_EVENT_H + +#include From 0d8914165dd17b0fd330538871968efabb3227c0 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Tue, 25 Jun 2024 12:18:06 +0900 Subject: [PATCH 24/32] firewire: ohci: add tracepoints event for hardIRQ event 1394 OHCI hardware triggers PCI interrupts to notify any events to software. Current driver for the hardware is programmed by the typical way to utilize top- and bottom- halves, thus it has a timing gap to handle the notification in softIRQ (tasklet). This commit adds a tracepoint event for the hardIRQ event. The comparison of the tracepoint event to tracepoints events in firewire subsystem is helpful to diagnose the timing gap. Link: https://lore.kernel.org/r/20240625031806.956650-3-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/ohci.c | 1 + include/trace/events/firewire_ohci.h | 33 +++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 07adb4ddd444..df16a8f4ee7f 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -2185,6 +2185,7 @@ static irqreturn_t irq_handler(int irq, void *data) */ reg_write(ohci, OHCI1394_IntEventClear, event & ~(OHCI1394_busReset | OHCI1394_postedWriteErr)); + trace_irqs(ohci->card.index, event); log_irqs(ohci, event); // The flag is masked again at bus_reset_work() scheduled by selfID event. if (event & OHCI1394_busReset) diff --git a/include/trace/events/firewire_ohci.h b/include/trace/events/firewire_ohci.h index 67fa3c1c8f6d..483aeeb033af 100644 --- a/include/trace/events/firewire_ohci.h +++ b/include/trace/events/firewire_ohci.h @@ -9,7 +9,38 @@ #include -// Placeholder for future use. +TRACE_EVENT(irqs, + TP_PROTO(unsigned int card_index, u32 events), + TP_ARGS(card_index, events), + TP_STRUCT__entry( + __field(u8, card_index) + __field(u32, events) + ), + TP_fast_assign( + __entry->card_index = card_index; + __entry->events = events; + ), + TP_printk( + "card_index=%u events=%s", + __entry->card_index, + __print_flags(__entry->events, "|", + { OHCI1394_selfIDComplete, "selfIDComplete" }, + { OHCI1394_RQPkt, "RQPkt" }, + { OHCI1394_RSPkt, "RSPkt" }, + { OHCI1394_reqTxComplete, "reqTxComplete" }, + { OHCI1394_respTxComplete, "respTxComplete" }, + { OHCI1394_isochRx, "isochRx" }, + { OHCI1394_isochTx, "isochTx" }, + { OHCI1394_postedWriteErr, "postedWriteErr" }, + { OHCI1394_cycleTooLong, "cycleTooLong" }, + { OHCI1394_cycle64Seconds, "cycle64Seconds" }, + { OHCI1394_cycleInconsistent, "cycleInconsistent" }, + { OHCI1394_regAccessFail, "regAccessFail" }, + { OHCI1394_unrecoverableError, "unrecoverableError" }, + { OHCI1394_busReset, "busReset" } + ) + ) +); #endif // _FIREWIRE_OHCI_TRACE_EVENT_H From 502099acb8cbf08acb6b43ff2b8da43d2bf85166 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Thu, 27 Jun 2024 18:08:47 +0100 Subject: [PATCH 25/32] firewire: core: Fix spelling mistakes in tracepoint messages There are two spelling mistakes in the tracepoint message text. Fix them. Signed-off-by: Colin Ian King Link: https://lore.kernel.org/r/20240627170847.125531-1-colin.i.king@gmail.com Signed-off-by: Takashi Sakamoto --- include/trace/events/firewire.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/trace/events/firewire.h b/include/trace/events/firewire.h index aa00c9f33551..bac9d98e88e5 100644 --- a/include/trace/events/firewire.h +++ b/include/trace/events/firewire.h @@ -854,7 +854,7 @@ DECLARE_EVENT_CLASS(isoc_single_completions_template, memcpy(__get_dynamic_array(header), header, __get_dynamic_array_len(header)); ), TP_printk( - "context=0x%llx card_index=%u timestap=0x%04x cause=%s header=%s", + "context=0x%llx card_index=%u timestamp=0x%04x cause=%s header=%s", __entry->context, __entry->card_index, __entry->timestamp, @@ -892,7 +892,7 @@ TRACE_EVENT(isoc_inbound_multiple_completions, __entry->cause = cause; ), TP_printk( - "context=0x%llx card_index=%u comleted=%u cause=%s", + "context=0x%llx card_index=%u completed=%u cause=%s", __entry->context, __entry->card_index, __entry->completed, From f26a38e61c03fdfacb6b596e1daf665cf4526a60 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Wed, 3 Jul 2024 07:20:30 +0900 Subject: [PATCH 26/32] firewire: ohci: use common macro to interpret be32 data in le32 buffer The 1394 OHCI driver configures the hardware to transfer the data quadlets of packet via DMA after converting it to little endian, therefore the data is typed as __le32. Nevertheless some actual hardware ignores the configuration. In the case, the data in DMA buffer is aligned to big endian (__be32). For the case in big-endian machine, the driver includes the following interpretation from __le32 to u32 (host-endian = __be32): * (__force __u32)(v) In include/linux/byteorder/generic.h, be32_to_cpu() is available. It is expanded to the following expression in 'include/uapi/linux/byteorder/big_endian.h': * (__force __u32)(__be32)(x) This commit replace the ad-hoc endian interpretation with the above. Link: https://lore.kernel.org/r/20240702222034.1378764-2-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/ohci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index df16a8f4ee7f..a2d6d1d1ec2b 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -876,7 +876,7 @@ static void ar_sync_buffers_for_cpu(struct ar_context *ctx, #if defined(CONFIG_PPC_PMAC) && defined(CONFIG_PPC32) #define cond_le32_to_cpu(v) \ - (ohci->quirks & QUIRK_BE_HEADERS ? (__force __u32)(v) : le32_to_cpu(v)) + (ohci->quirks & QUIRK_BE_HEADERS ? be32_to_cpu(v) : le32_to_cpu(v)) #else #define cond_le32_to_cpu(v) le32_to_cpu(v) #endif From c538b06de6ce2966f72a7cf3b63e46e2f604a49e Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Wed, 3 Jul 2024 07:20:31 +0900 Subject: [PATCH 27/32] firewire: ohci: use static function to handle endian issue on PowerPC platform It is preferable to use static function instead of functional macro in some points. It checks type of argument, but would be optimized to embedded code instead of function calls. This commit obsoletes the functional macro with the static function. Additionally this commit refactors quirk detection to ease the later work. Link: https://lore.kernel.org/r/20240702222034.1378764-3-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/ohci.c | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index a2d6d1d1ec2b..5b90ca72fa64 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -875,10 +875,25 @@ static void ar_sync_buffers_for_cpu(struct ar_context *ctx, } #if defined(CONFIG_PPC_PMAC) && defined(CONFIG_PPC32) -#define cond_le32_to_cpu(v) \ - (ohci->quirks & QUIRK_BE_HEADERS ? be32_to_cpu(v) : le32_to_cpu(v)) +static u32 cond_le32_to_cpu(__le32 value, bool has_be_header_quirk) +{ + return has_be_header_quirk ? be32_to_cpu(value) : le32_to_cpu(value); +} + +static bool has_be_header_quirk(const struct fw_ohci *ohci) +{ + return !!(ohci->quirks & QUIRK_BE_HEADERS); +} #else -#define cond_le32_to_cpu(v) le32_to_cpu(v) +static u32 cond_le32_to_cpu(__le32 value, bool has_be_header_quirk __maybe_unused) +{ + return le32_to_cpu(value); +} + +static bool has_be_header_quirk(const struct fw_ohci *ohci) +{ + return false; +} #endif static __le32 *handle_ar_packet(struct ar_context *ctx, __le32 *buffer) @@ -888,9 +903,9 @@ static __le32 *handle_ar_packet(struct ar_context *ctx, __le32 *buffer) u32 status, length, tcode; int evt; - p.header[0] = cond_le32_to_cpu(buffer[0]); - p.header[1] = cond_le32_to_cpu(buffer[1]); - p.header[2] = cond_le32_to_cpu(buffer[2]); + p.header[0] = cond_le32_to_cpu(buffer[0], has_be_header_quirk(ohci)); + p.header[1] = cond_le32_to_cpu(buffer[1], has_be_header_quirk(ohci)); + p.header[2] = cond_le32_to_cpu(buffer[2], has_be_header_quirk(ohci)); tcode = async_header_get_tcode(p.header); switch (tcode) { @@ -902,7 +917,7 @@ static __le32 *handle_ar_packet(struct ar_context *ctx, __le32 *buffer) break; case TCODE_READ_BLOCK_REQUEST : - p.header[3] = cond_le32_to_cpu(buffer[3]); + p.header[3] = cond_le32_to_cpu(buffer[3], has_be_header_quirk(ohci)); p.header_length = 16; p.payload_length = 0; break; @@ -911,7 +926,7 @@ static __le32 *handle_ar_packet(struct ar_context *ctx, __le32 *buffer) case TCODE_READ_BLOCK_RESPONSE: case TCODE_LOCK_REQUEST: case TCODE_LOCK_RESPONSE: - p.header[3] = cond_le32_to_cpu(buffer[3]); + p.header[3] = cond_le32_to_cpu(buffer[3], has_be_header_quirk(ohci)); p.header_length = 16; p.payload_length = async_header_get_data_length(p.header); if (p.payload_length > MAX_ASYNC_PAYLOAD) { @@ -936,7 +951,7 @@ static __le32 *handle_ar_packet(struct ar_context *ctx, __le32 *buffer) /* FIXME: What to do about evt_* errors? */ length = (p.header_length + p.payload_length + 3) / 4; - status = cond_le32_to_cpu(buffer[length]); + status = cond_le32_to_cpu(buffer[length], has_be_header_quirk(ohci)); evt = (status >> 16) & 0x1f; p.ack = evt - 16; @@ -2030,12 +2045,12 @@ static void bus_reset_work(struct work_struct *work) return; } - generation = (cond_le32_to_cpu(ohci->self_id[0]) >> 16) & 0xff; + generation = (cond_le32_to_cpu(ohci->self_id[0], has_be_header_quirk(ohci)) >> 16) & 0xff; rmb(); for (i = 1, j = 0; j < self_id_count; i += 2, j++) { - u32 id = cond_le32_to_cpu(ohci->self_id[i]); - u32 id2 = cond_le32_to_cpu(ohci->self_id[i + 1]); + u32 id = cond_le32_to_cpu(ohci->self_id[i], has_be_header_quirk(ohci)); + u32 id2 = cond_le32_to_cpu(ohci->self_id[i + 1], has_be_header_quirk(ohci)); if (id != ~id2) { /* From 7a14f78d70dd9fde77275a3cc9427de862969c4c Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Wed, 3 Jul 2024 07:20:32 +0900 Subject: [PATCH 28/32] firewire: ohci: add static inline functions to deserialize for Self-ID DMA operation The SelfI-ID is one type of DMAs defined in 1394 OHCI specification. It is operated by two registers, one interrupt, and has one format of buffer. This commit adds some static inline functions to deserialize the data in the buffer and registers. Some KUnit tests are also added to check their reliability. Link: https://lore.kernel.org/r/20240702222034.1378764-4-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/.kunitconfig | 1 + drivers/firewire/Kconfig | 16 +++++++++ drivers/firewire/Makefile | 1 + drivers/firewire/ohci-serdes-test.c | 56 +++++++++++++++++++++++++++++ drivers/firewire/ohci.h | 42 ++++++++++++++++++++++ 5 files changed, 116 insertions(+) create mode 100644 drivers/firewire/ohci-serdes-test.c diff --git a/drivers/firewire/.kunitconfig b/drivers/firewire/.kunitconfig index 74259204fcdd..21b7e9eef63d 100644 --- a/drivers/firewire/.kunitconfig +++ b/drivers/firewire/.kunitconfig @@ -5,3 +5,4 @@ CONFIG_FIREWIRE_KUNIT_UAPI_TEST=y CONFIG_FIREWIRE_KUNIT_DEVICE_ATTRIBUTE_TEST=y CONFIG_FIREWIRE_KUNIT_PACKET_SERDES_TEST=y CONFIG_FIREWIRE_KUNIT_SELF_ID_SEQUENCE_HELPER_TEST=y +CONFIG_FIREWIRE_KUNIT_OHCI_SERDES_TEST=y diff --git a/drivers/firewire/Kconfig b/drivers/firewire/Kconfig index 95e72e0b592b..905c82e26ce7 100644 --- a/drivers/firewire/Kconfig +++ b/drivers/firewire/Kconfig @@ -92,6 +92,22 @@ config FIREWIRE_OHCI To compile this driver as a module, say M here: The module will be called firewire-ohci. +config FIREWIRE_KUNIT_OHCI_SERDES_TEST + tristate "KUnit tests for serialization/deserialization of data in buffers/registers" if !KUNIT_ALL_TESTS + depends on FIREWIRE && KUNIT + default KUNIT_ALL_TESTS + help + This builds the KUnit tests to check serialization and deserialization + of data in buffers and registers defined in 1394 OHCI specification. + + 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_SBP2 tristate "Storage devices (SBP-2 protocol)" depends on FIREWIRE && SCSI diff --git a/drivers/firewire/Makefile b/drivers/firewire/Makefile index 21b975e0a387..1ff550e93a8c 100644 --- a/drivers/firewire/Makefile +++ b/drivers/firewire/Makefile @@ -19,3 +19,4 @@ obj-$(CONFIG_PROVIDE_OHCI1394_DMA_INIT) += init_ohci1394_dma.o obj-$(CONFIG_FIREWIRE_KUNIT_UAPI_TEST) += uapi-test.o obj-$(CONFIG_FIREWIRE_KUNIT_PACKET_SERDES_TEST) += packet-serdes-test.o obj-$(CONFIG_FIREWIRE_KUNIT_SELF_ID_SEQUENCE_HELPER_TEST) += self-id-sequence-helper-test.o +obj-$(CONFIG_FIREWIRE_KUNIT_OHCI_SERDES_TEST) += ohci-serdes-test.o diff --git a/drivers/firewire/ohci-serdes-test.c b/drivers/firewire/ohci-serdes-test.c new file mode 100644 index 000000000000..304a09ff528e --- /dev/null +++ b/drivers/firewire/ohci-serdes-test.c @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +// +// ohci-serdes-test.c - An application of Kunit to check serialization/deserialization of data in +// buffers and registers defined in 1394 OHCI specification. +// +// Copyright (c) 2024 Takashi Sakamoto + +#include + +#include "ohci.h" + + +static void test_self_id_count_register_deserialization(struct kunit *test) +{ + const u32 expected = 0x803d0594; + + bool is_error = ohci1394_self_id_count_is_error(expected); + u8 generation = ohci1394_self_id_count_get_generation(expected); + u32 size = ohci1394_self_id_count_get_size(expected); + + KUNIT_EXPECT_TRUE(test, is_error); + KUNIT_EXPECT_EQ(test, 0x3d, generation); + KUNIT_EXPECT_EQ(test, 0x165, size); +} + +static void test_self_id_receive_buffer_deserialization(struct kunit *test) +{ + const u32 buffer[] = { + 0x0006f38b, + 0x807fcc56, + 0x7f8033a9, + 0x8145cc5e, + 0x7eba33a1, + }; + + u8 generation = ohci1394_self_id_receive_q0_get_generation(buffer[0]); + u16 timestamp = ohci1394_self_id_receive_q0_get_timestamp(buffer[0]); + + KUNIT_EXPECT_EQ(test, 0x6, generation); + KUNIT_EXPECT_EQ(test, 0xf38b, timestamp); +} + +static struct kunit_case ohci_serdes_test_cases[] = { + KUNIT_CASE(test_self_id_count_register_deserialization), + KUNIT_CASE(test_self_id_receive_buffer_deserialization), + {} +}; + +static struct kunit_suite ohci_serdes_test_suite = { + .name = "firewire-ohci-serdes", + .test_cases = ohci_serdes_test_cases, +}; +kunit_test_suite(ohci_serdes_test_suite); + +MODULE_DESCRIPTION("FireWire buffers and registers serialization/deserialization unit test suite"); +MODULE_LICENSE("GPL"); diff --git a/drivers/firewire/ohci.h b/drivers/firewire/ohci.h index c4d005a9901a..d83fd4731d56 100644 --- a/drivers/firewire/ohci.h +++ b/drivers/firewire/ohci.h @@ -156,4 +156,46 @@ #define OHCI1394_phy_tcode 0xe +// Self-ID DMA. + +#define OHCI1394_SelfIDCount_selfIDError_MASK 0x80000000 +#define OHCI1394_SelfIDCount_selfIDError_SHIFT 31 +#define OHCI1394_SelfIDCount_selfIDGeneration_MASK 0x00ff0000 +#define OHCI1394_SelfIDCount_selfIDGeneration_SHIFT 16 +#define OHCI1394_SelfIDCount_selfIDSize_MASK 0x000007fc +#define OHCI1394_SelfIDCount_selfIDSize_SHIFT 2 + +static inline bool ohci1394_self_id_count_is_error(u32 value) +{ + return !!((value & OHCI1394_SelfIDCount_selfIDError_MASK) >> OHCI1394_SelfIDCount_selfIDError_SHIFT); +} + +static inline u8 ohci1394_self_id_count_get_generation(u32 value) +{ + return (value & OHCI1394_SelfIDCount_selfIDGeneration_MASK) >> OHCI1394_SelfIDCount_selfIDGeneration_SHIFT; +} + +// In 1394 OHCI specification, the maximum size of self ID stream is 504 quadlets +// (= 63 devices * 4 self ID packets * 2 quadlets). The selfIDSize field accommodates it and its +// additional first quadlet, since the field is 9 bits (0x1ff = 511). +static inline u32 ohci1394_self_id_count_get_size(u32 value) +{ + return (value & OHCI1394_SelfIDCount_selfIDSize_MASK) >> OHCI1394_SelfIDCount_selfIDSize_SHIFT; +} + +#define OHCI1394_SELF_ID_RECEIVE_Q0_GENERATION_MASK 0x00ff0000 +#define OHCI1394_SELF_ID_RECEIVE_Q0_GENERATION_SHIFT 16 +#define OHCI1394_SELF_ID_RECEIVE_Q0_TIMESTAMP_MASK 0x0000ffff +#define OHCI1394_SELF_ID_RECEIVE_Q0_TIMESTAMP_SHIFT 0 + +static inline u8 ohci1394_self_id_receive_q0_get_generation(u32 quadlet0) +{ + return (quadlet0 & OHCI1394_SELF_ID_RECEIVE_Q0_GENERATION_MASK) >> OHCI1394_SELF_ID_RECEIVE_Q0_GENERATION_SHIFT; +} + +static inline u16 ohci1394_self_id_receive_q0_get_timestamp(u32 quadlet0) +{ + return (quadlet0 & OHCI1394_SELF_ID_RECEIVE_Q0_TIMESTAMP_MASK) >> OHCI1394_SELF_ID_RECEIVE_Q0_TIMESTAMP_SHIFT; +} + #endif /* _FIREWIRE_OHCI_H */ From 4a13617ef311cb01160bdecfa338e2148940059c Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Wed, 3 Jul 2024 07:20:33 +0900 Subject: [PATCH 29/32] firewire: ohci: use inline functions to operate data of self-ID DMA The code of 1394 OHCI driver includes hard-coded magic number to operate data of Self-ID DMA. This commit replaces them with the inline functions added/tested in the former commit. Link: https://lore.kernel.org/r/20240702222034.1378764-5-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/ohci.c | 12 +++++++----- drivers/firewire/ohci.h | 1 - 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 5b90ca72fa64..c95f26c74e2b 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -2003,7 +2003,7 @@ static void bus_reset_work(struct work_struct *work) struct fw_ohci *ohci = container_of(work, struct fw_ohci, bus_reset_work); int self_id_count, generation, new_generation, i, j; - u32 reg; + u32 reg, quadlet; void *free_rom = NULL; dma_addr_t free_rom_bus = 0; bool is_new_root; @@ -2028,7 +2028,7 @@ static void bus_reset_work(struct work_struct *work) ohci->is_root = is_new_root; reg = reg_read(ohci, OHCI1394_SelfIDCount); - if (reg & OHCI1394_SelfIDCount_selfIDError) { + if (ohci1394_self_id_count_is_error(reg)) { ohci_notice(ohci, "self ID receive error\n"); return; } @@ -2038,14 +2038,15 @@ static void bus_reset_work(struct work_struct *work) * the inverted quadlets and a header quadlet, we shift one * bit extra to get the actual number of self IDs. */ - self_id_count = (reg >> 3) & 0xff; + self_id_count = ohci1394_self_id_count_get_size(reg) >> 1; if (self_id_count > 252) { ohci_notice(ohci, "bad selfIDSize (%08x)\n", reg); return; } - generation = (cond_le32_to_cpu(ohci->self_id[0], has_be_header_quirk(ohci)) >> 16) & 0xff; + quadlet = cond_le32_to_cpu(ohci->self_id[0], has_be_header_quirk(ohci)); + generation = ohci1394_self_id_receive_q0_get_generation(quadlet); rmb(); for (i = 1, j = 0; j < self_id_count; i += 2, j++) { @@ -2102,7 +2103,8 @@ static void bus_reset_work(struct work_struct *work) * of self IDs. */ - new_generation = (reg_read(ohci, OHCI1394_SelfIDCount) >> 16) & 0xff; + reg = reg_read(ohci, OHCI1394_SelfIDCount); + new_generation = ohci1394_self_id_count_get_generation(reg); if (new_generation != generation) { ohci_notice(ohci, "new bus reset, discarding self ids\n"); return; diff --git a/drivers/firewire/ohci.h b/drivers/firewire/ohci.h index d83fd4731d56..71c2ed84cafb 100644 --- a/drivers/firewire/ohci.h +++ b/drivers/firewire/ohci.h @@ -31,7 +31,6 @@ #define OHCI1394_HCControl_softReset 0x00010000 #define OHCI1394_SelfIDBuffer 0x064 #define OHCI1394_SelfIDCount 0x068 -#define OHCI1394_SelfIDCount_selfIDError 0x80000000 #define OHCI1394_IRMultiChanMaskHiSet 0x070 #define OHCI1394_IRMultiChanMaskHiClear 0x074 #define OHCI1394_IRMultiChanMaskLoSet 0x078 From 526e21a2aa6fa7ac3309a8c87f4d6e5f7e407cb6 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Wed, 3 Jul 2024 07:20:34 +0900 Subject: [PATCH 30/32] firewire: ohci: add tracepoints event for data of Self-ID DMA In 1394 OHCI, the SelfIDComplete event occurs when the hardware has finished transmitting all of the self ID packets received during the bus initialization process to the host memory by DMA. This commit adds a tracepoints event for this event to trace the timing and packet data of Self-ID DMA. It is the part of following tracepoints events helpful to debug some events at bus reset; e.g. the issue addressed at a commit d0b06dc48fb1 ("firewire: core: use long bus reset on gap count error")[1]: * firewire_ohci:irqs * firewire_ohci:self_id_complete * firewire:bus_reset_handle * firewire:self_id_sequence They would be also helpful in the problem about invocation timing of hardIRQ and process (workqueue) contexts. We can often see this kind of problem with -rt kernel[2]. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d0b06dc48fb1 [2] https://lore.kernel.org/linux-rt-users/YAwPoaUZ1gTD5y+k@hmbx/ Link: https://lore.kernel.org/r/20240702222034.1378764-6-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/ohci.c | 11 +++++- include/trace/events/firewire_ohci.h | 54 ++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index c95f26c74e2b..f8d880574c19 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -45,6 +45,8 @@ #include +static u32 cond_le32_to_cpu(__le32 value, bool has_be_header_quirk); + #define CREATE_TRACE_POINTS #include @@ -2208,8 +2210,15 @@ static irqreturn_t irq_handler(int irq, void *data) if (event & OHCI1394_busReset) reg_write(ohci, OHCI1394_IntMaskClear, OHCI1394_busReset); - if (event & OHCI1394_selfIDComplete) + if (event & OHCI1394_selfIDComplete) { + if (trace_self_id_complete_enabled()) { + u32 reg = reg_read(ohci, OHCI1394_SelfIDCount); + + trace_self_id_complete(ohci->card.index, reg, ohci->self_id, + has_be_header_quirk(ohci)); + } queue_work(selfid_workqueue, &ohci->bus_reset_work); + } if (event & OHCI1394_RQPkt) tasklet_schedule(&ohci->ar_request_ctx.tasklet); diff --git a/include/trace/events/firewire_ohci.h b/include/trace/events/firewire_ohci.h index 483aeeb033af..4f9a7f2577f3 100644 --- a/include/trace/events/firewire_ohci.h +++ b/include/trace/events/firewire_ohci.h @@ -9,6 +9,8 @@ #include +// Some macros and helper functions are defined in 'drivers/firewire/ohci.c'. + TRACE_EVENT(irqs, TP_PROTO(unsigned int card_index, u32 events), TP_ARGS(card_index, events), @@ -42,6 +44,58 @@ TRACE_EVENT(irqs, ) ); +#define QUADLET_SIZE 4 + +#define SELF_ID_COUNT_IS_ERROR(reg) \ + (!!(((reg) & OHCI1394_SelfIDCount_selfIDError_MASK) >> OHCI1394_SelfIDCount_selfIDError_SHIFT)) + +#define SELF_ID_COUNT_GET_GENERATION(reg) \ + (((reg) & OHCI1394_SelfIDCount_selfIDGeneration_MASK) >> OHCI1394_SelfIDCount_selfIDGeneration_SHIFT) + +#define SELF_ID_RECEIVE_Q0_GET_GENERATION(quadlet) \ + (((quadlet) & OHCI1394_SELF_ID_RECEIVE_Q0_GENERATION_MASK) >> OHCI1394_SELF_ID_RECEIVE_Q0_GENERATION_SHIFT) + +#define SELF_ID_RECEIVE_Q0_GET_TIMESTAMP(quadlet) \ + (((quadlet) & OHCI1394_SELF_ID_RECEIVE_Q0_TIMESTAMP_MASK) >> OHCI1394_SELF_ID_RECEIVE_Q0_TIMESTAMP_SHIFT) + +TRACE_EVENT(self_id_complete, + TP_PROTO(unsigned int card_index, u32 reg, const __le32 *self_id_receive, bool has_be_header_quirk), + TP_ARGS(card_index, reg, self_id_receive, has_be_header_quirk), + TP_STRUCT__entry( + __field(u8, card_index) + __field(u32, reg) + __dynamic_array(u32, self_id_receive, ohci1394_self_id_count_get_size(reg)) + ), + TP_fast_assign( + __entry->card_index = card_index; + __entry->reg = reg; + { + u32 *ptr = __get_dynamic_array(self_id_receive); + int i; + + for (i = 0; i < __get_dynamic_array_len(self_id_receive) / QUADLET_SIZE; ++i) + ptr[i] = cond_le32_to_cpu(self_id_receive[i], has_be_header_quirk); + } + ), + TP_printk( + "card_index=%u is_error=%s generation_at_bus_reset=%u generation_at_completion=%u timestamp=0x%04x packet_data=%s", + __entry->card_index, + SELF_ID_COUNT_IS_ERROR(__entry->reg) ? "true" : "false", + SELF_ID_COUNT_GET_GENERATION(__entry->reg), + SELF_ID_RECEIVE_Q0_GET_GENERATION(((const u32 *)__get_dynamic_array(self_id_receive))[0]), + SELF_ID_RECEIVE_Q0_GET_TIMESTAMP(((const u32 *)__get_dynamic_array(self_id_receive))[0]), + __print_array(((const u32 *)__get_dynamic_array(self_id_receive)) + 1, + (__get_dynamic_array_len(self_id_receive) / QUADLET_SIZE) - 1, QUADLET_SIZE) + ) +); + +#undef SELF_ID_COUNT_IS_ERROR +#undef SELF_ID_COUNT_GET_GENERATION +#undef SELF_ID_RECEIVE_Q0_GET_GENERATION +#undef SELF_ID_RECEIVE_Q0_GET_TIMESTAMP + +#undef QUADLET_SIZE + #endif // _FIREWIRE_OHCI_TRACE_EVENT_H #include From 9f349e8e532d7e50b57a3e10367a8eb64d89e104 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Sun, 7 Jul 2024 22:45:23 +0900 Subject: [PATCH 31/32] Revert "firewire: ohci: use common macro to interpret be32 data in le32 buffer" This reverts commit f26a38e61c03fdfacb6b596e1daf665cf4526a60, since it causes the following sparse warnings: sparse warnings: (new ones prefixed by >>) >> drivers/firewire/ohci.c:891:23: sparse: sparse: cast to restricted __be32 >> drivers/firewire/ohci.c:891:23: sparse: sparse: cast from restricted __le32 drivers/firewire/ohci.c:892:23: sparse: sparse: cast to restricted __be32 drivers/firewire/ohci.c:892:23: sparse: sparse: cast from restricted __le32 drivers/firewire/ohci.c:893:23: sparse: sparse: cast to restricted __be32 drivers/firewire/ohci.c:893:23: sparse: sparse: cast from restricted __le32 drivers/firewire/ohci.c:905:31: sparse: sparse: cast to restricted __be32 drivers/firewire/ohci.c:905:31: sparse: sparse: cast from restricted __le32 drivers/firewire/ohci.c:914:31: sparse: sparse: cast to restricted __be32 drivers/firewire/ohci.c:914:31: sparse: sparse: cast from restricted __le32 drivers/firewire/ohci.c:939:18: sparse: sparse: cast to restricted __be32 drivers/firewire/ohci.c:939:18: sparse: sparse: cast from restricted __le32 drivers/firewire/ohci.c:2033:23: sparse: sparse: cast to restricted __be32 drivers/firewire/ohci.c:2033:23: sparse: sparse: cast from restricted __le32 drivers/firewire/ohci.c:2037:27: sparse: sparse: cast to restricted __be32 drivers/firewire/ohci.c:2037:27: sparse: sparse: cast from restricted __le32 drivers/firewire/ohci.c:2038:27: sparse: sparse: cast to restricted __be32 drivers/firewire/ohci.c:2038:27: sparse: sparse: cast from restricted __le32 Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202407050656.03bw1YXA-lkp@intel.com/ Link: https://lore.kernel.org/r/20240707134523.11784-1-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/ohci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index f8d880574c19..314a29c0fd3e 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -879,7 +879,7 @@ static void ar_sync_buffers_for_cpu(struct ar_context *ctx, #if defined(CONFIG_PPC_PMAC) && defined(CONFIG_PPC32) static u32 cond_le32_to_cpu(__le32 value, bool has_be_header_quirk) { - return has_be_header_quirk ? be32_to_cpu(value) : le32_to_cpu(value); + return has_be_header_quirk ? (__force __u32)value : le32_to_cpu(value); } static bool has_be_header_quirk(const struct fw_ohci *ohci) From 06dcc4c9baa9e92896f00d02ffa760c0988b4371 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 12 Jul 2024 09:30:10 +0900 Subject: [PATCH 32/32] firewire: core: move copy_port_status() helper function to TP_fast_assign() block It would be possible to put any statement in TP_fast_assign(). This commit obsoletes the helper function and put its statements to TP_fast_assign() for the code simplicity. Link: https://lore.kernel.org/r/20240712003010.87341-1-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-trace.c | 11 ----------- include/trace/events/firewire.h | 15 ++++++++++----- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/drivers/firewire/core-trace.c b/drivers/firewire/core-trace.c index 5acb02c18a47..b70947fc7b8d 100644 --- a/drivers/firewire/core-trace.c +++ b/drivers/firewire/core-trace.c @@ -10,17 +10,6 @@ #include #ifdef TRACEPOINTS_ENABLED -void copy_port_status(u8 *port_status, unsigned int port_capacity, - const u32 *self_id_sequence, unsigned int quadlet_count) -{ - unsigned int port_index; - - for (port_index = 0; port_index < port_capacity; ++port_index) { - port_status[port_index] = - self_id_sequence_get_port_status(self_id_sequence, quadlet_count, port_index); - } -} - EXPORT_TRACEPOINT_SYMBOL_GPL(isoc_inbound_single_completions); EXPORT_TRACEPOINT_SYMBOL_GPL(isoc_inbound_multiple_completions); EXPORT_TRACEPOINT_SYMBOL_GPL(isoc_outbound_completions); diff --git a/include/trace/events/firewire.h b/include/trace/events/firewire.h index bac9d98e88e5..b108176deb22 100644 --- a/include/trace/events/firewire.h +++ b/include/trace/events/firewire.h @@ -392,9 +392,6 @@ TRACE_EVENT(bus_reset_handle, #define PHY_PACKET_SELF_ID_GET_INITIATED_RESET(quads) \ ((((const u32 *)quads)[0] & SELF_ID_ZERO_INITIATED_RESET_MASK) >> SELF_ID_ZERO_INITIATED_RESET_SHIFT) -void copy_port_status(u8 *port_status, unsigned int port_capacity, const u32 *self_id_sequence, - unsigned int quadlet_count); - TRACE_EVENT(self_id_sequence, TP_PROTO(unsigned int card_index, const u32 *self_id_sequence, unsigned int quadlet_count, unsigned int generation), TP_ARGS(card_index, self_id_sequence, quadlet_count, generation), @@ -407,8 +404,16 @@ TRACE_EVENT(self_id_sequence, TP_fast_assign( __entry->card_index = card_index; __entry->generation = generation; - copy_port_status(__get_dynamic_array(port_status), __get_dynamic_array_len(port_status), - self_id_sequence, quadlet_count); + { + u8 *port_status = __get_dynamic_array(port_status); + unsigned int port_index; + + for (port_index = 0; port_index < __get_dynamic_array_len(port_status); ++port_index) { + port_status[port_index] = + self_id_sequence_get_port_status(self_id_sequence, + quadlet_count, port_index); + } + } memcpy(__get_dynamic_array(self_id_sequence), self_id_sequence, __get_dynamic_array_len(self_id_sequence)); ),