Thread: [PATCH 00/11] firewire: add tracepoints events for self ID sequence
Brought to you by:
aeb,
bencollins
From: Takashi S. <o-t...@sa...> - 2024-06-05 23:52:16
|
Hi, In core function, the enumeration of self ID sequences is the first step to build bus topology for the current generation. Currently, 1394 OHCI driver has a module option to dump the content of self ID sequence, while it is implemented by printk. My recent work is going to replace such logging with tracepoints events, and this series of changes is for the self ID sequence. The content of self ID sequence is delivered by a kind of phy packet, and its serialization and deserialization codes exist in both core function and 1394 OHCI driver. They include some redundancies, and the series of changes includes some inline helper functions to replace them. In the series of changes, some KUnit tests are added to check behaviour of the enumeration and the helper functions. Takashi Sakamoto (11): firewire: core: add enumerator of self ID sequences and its KUnit test firewire: core: add helper function to handle port status from self ID sequence and its KUnit test firewire: core: minor code refactoring for topology builder firewire: ohci: minor code refactoring for self ID logging firewire: core: use helper functions for self ID sequence firewire: ohci: use helper functions for self ID sequence firewire: core: add common inline functions to serialize/deserialize self ID packet firewire: core: use helper inline functions to deserialize self ID packet firewire: ohci: use helper inline functions to serialize/deserialize self ID packet firewire: core: arrangement header inclusion for tracepoints events firewire: core: add tracepoints event for self_id_sequence drivers/firewire/.kunitconfig | 1 + drivers/firewire/Kconfig | 15 ++ drivers/firewire/Makefile | 1 + drivers/firewire/core-topology.c | 219 ++++++--------- drivers/firewire/core-trace.c | 18 ++ drivers/firewire/core-transaction.c | 2 +- drivers/firewire/ohci.c | 148 ++++++---- drivers/firewire/packet-header-definitions.h | 2 + drivers/firewire/packet-serdes-test.c | 255 ++++++++++++++++++ drivers/firewire/phy-packet-definitions.h | 247 +++++++++++++++++ .../firewire/self-id-sequence-helper-test.c | 152 +++++++++++ include/trace/events/firewire.h | 61 ++++- 12 files changed, 935 insertions(+), 186 deletions(-) create mode 100644 drivers/firewire/phy-packet-definitions.h create mode 100644 drivers/firewire/self-id-sequence-helper-test.c -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-06-05 23:52:16
|
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. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- 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 869598b20e3a..49ba692d17d8 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 <kunit/test.h> + +#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"); -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-06-05 23:52:16
|
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. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- 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), {} }; -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-06-05 23:52:25
|
This commit replaces the existing implementation with the helper functions for self ID sequence. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- 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 6a039293ee63..999ba2b121cd 100644 --- a/drivers/firewire/core-topology.c +++ b/drivers/firewire/core-topology.c @@ -20,80 +20,15 @@ #include <asm/byteorder.h> #include "core.h" +#include "phy-packet-definitions.h" #include <trace/events/firewire.h> #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++; } -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-06-05 23:52:24
|
This commit replaces the existing implementation with the helper functions for self ID sequence. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- 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[] = { -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-06-05 23:52:27
|
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. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- 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 <linux/types.h> +#include "packet-header-definitions.h" + #define CREATE_TRACE_POINTS #include <trace/events/firewire.h> diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c index 571fdff65c2b..6868ff17dc10 100644 --- a/drivers/firewire/core-transaction.c +++ b/drivers/firewire/core-transaction.c @@ -29,8 +29,8 @@ #include <asm/byteorder.h> #include "core.h" -#include <trace/events/firewire.h> #include "packet-header-definitions.h" +#include <trace/events/firewire.h> #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 <linux/types.h> + #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 d695a560673f..1f4ef0ed65bc 100644 --- a/include/trace/events/firewire.h +++ b/include/trace/events/firewire.h @@ -11,7 +11,7 @@ #include <linux/firewire-constants.h> -#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) \ -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-06-05 23:52:25
|
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. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- 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 837cc44d8d9f..6a039293ee63 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; -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-06-05 23:52:16
|
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. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- 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[] = { -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-06-05 23:52:25
|
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. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- 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 <linux/firewire-constants.h> #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; -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-06-05 23:52:28
|
This commit replaces the existing implementation with the helper functions for self ID packet. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- 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 999ba2b121cd..6ec100e17500 100644 --- a/drivers/firewire/core-topology.c +++ b/drivers/firewire/core-topology.c @@ -23,13 +23,6 @@ #include "phy-packet-definitions.h" #include <trace/events/firewire.h> -#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); -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-06-05 23:52:35
|
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. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- 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 6ec100e17500..4a0b273392ab 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 <linux/types.h> +#include <linux/err.h> #include "packet-header-definitions.h" +#include "phy-packet-definitions.h" #define CREATE_TRACE_POINTS #include <trace/events/firewire.h> + +#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 1f4ef0ed65bc..4761b700ff84 100644 --- a/include/trace/events/firewire.h +++ b/include/trace/events/firewire.h @@ -341,6 +341,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 -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-06-05 23:52:30
|
This commit replaces the existing implementation with the helper functions for self ID packet. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- 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) { -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-06-06 22:56:11
|
On Thu, Jun 06, 2024 at 08:51:44AM +0900, Takashi Sakamoto wrote: > Hi, > > In core function, the enumeration of self ID sequences is the first step > to build bus topology for the current generation. Currently, 1394 OHCI > driver has a module option to dump the content of self ID sequence, while > it is implemented by printk. My recent work is going to replace such > logging with tracepoints events, and this series of changes is for the > self ID sequence. > > The content of self ID sequence is delivered by a kind of phy packet, > and its serialization and deserialization codes exist in both core function > and 1394 OHCI driver. They include some redundancies, and the series of > changes includes some inline helper functions to replace them. > > In the series of changes, some KUnit tests are added to check behaviour > of the enumeration and the helper functions. > > Takashi Sakamoto (11): > firewire: core: add enumerator of self ID sequences and its KUnit test > firewire: core: add helper function to handle port status from self ID > sequence and its KUnit test > firewire: core: minor code refactoring for topology builder > firewire: ohci: minor code refactoring for self ID logging > firewire: core: use helper functions for self ID sequence > firewire: ohci: use helper functions for self ID sequence > firewire: core: add common inline functions to serialize/deserialize > self ID packet > firewire: core: use helper inline functions to deserialize self ID > packet > firewire: ohci: use helper inline functions to serialize/deserialize > self ID packet > firewire: core: arrangement header inclusion for tracepoints events > firewire: core: add tracepoints event for self_id_sequence > > drivers/firewire/.kunitconfig | 1 + > drivers/firewire/Kconfig | 15 ++ > drivers/firewire/Makefile | 1 + > drivers/firewire/core-topology.c | 219 ++++++--------- > drivers/firewire/core-trace.c | 18 ++ > drivers/firewire/core-transaction.c | 2 +- > drivers/firewire/ohci.c | 148 ++++++---- > drivers/firewire/packet-header-definitions.h | 2 + > drivers/firewire/packet-serdes-test.c | 255 ++++++++++++++++++ > drivers/firewire/phy-packet-definitions.h | 247 +++++++++++++++++ > .../firewire/self-id-sequence-helper-test.c | 152 +++++++++++ > include/trace/events/firewire.h | 61 ++++- > 12 files changed, 935 insertions(+), 186 deletions(-) > create mode 100644 drivers/firewire/phy-packet-definitions.h > create mode 100644 drivers/firewire/self-id-sequence-helper-test.c Applied to for-next branch. Regards Takashi Sakamoto |