linux1394-devel Mailing List for IEEE 1394 for Linux (Page 5)
Brought to you by:
aeb,
bencollins
You can subscribe to this list here.
2000 |
Jan
|
Feb
|
Mar
(39) |
Apr
(154) |
May
(172) |
Jun
(237) |
Jul
(127) |
Aug
(135) |
Sep
(193) |
Oct
(175) |
Nov
(173) |
Dec
(148) |
---|---|---|---|---|---|---|---|---|---|---|---|---|
2001 |
Jan
(161) |
Feb
(225) |
Mar
(193) |
Apr
(158) |
May
(179) |
Jun
(292) |
Jul
(146) |
Aug
(134) |
Sep
(185) |
Oct
(190) |
Nov
(149) |
Dec
(161) |
2002 |
Jan
(186) |
Feb
(236) |
Mar
(254) |
Apr
(207) |
May
(189) |
Jun
(182) |
Jul
(202) |
Aug
(155) |
Sep
(149) |
Oct
(449) |
Nov
(191) |
Dec
(108) |
2003 |
Jan
(174) |
Feb
(242) |
Mar
(243) |
Apr
(255) |
May
(202) |
Jun
(290) |
Jul
(237) |
Aug
(178) |
Sep
(101) |
Oct
(153) |
Nov
(144) |
Dec
(95) |
2004 |
Jan
(162) |
Feb
(278) |
Mar
(282) |
Apr
(152) |
May
(127) |
Jun
(138) |
Jul
(94) |
Aug
(63) |
Sep
(64) |
Oct
(150) |
Nov
(102) |
Dec
(197) |
2005 |
Jan
(102) |
Feb
(172) |
Mar
(89) |
Apr
(158) |
May
(139) |
Jun
(160) |
Jul
(288) |
Aug
(89) |
Sep
(201) |
Oct
(92) |
Nov
(190) |
Dec
(139) |
2006 |
Jan
(121) |
Feb
(204) |
Mar
(133) |
Apr
(134) |
May
(91) |
Jun
(226) |
Jul
(122) |
Aug
(101) |
Sep
(144) |
Oct
(141) |
Nov
|
Dec
|
2023 |
Jan
(19) |
Feb
(1) |
Mar
(5) |
Apr
(5) |
May
(33) |
Jun
(17) |
Jul
|
Aug
|
Sep
(3) |
Oct
(1) |
Nov
(5) |
Dec
(40) |
2024 |
Jan
(26) |
Feb
(14) |
Mar
(26) |
Apr
(46) |
May
(17) |
Jun
(47) |
Jul
(23) |
Aug
(72) |
Sep
(33) |
Oct
|
Nov
|
Dec
|
From: Takashi S. <o-t...@sa...> - 2024-08-02 00:36:24
|
This commit replaces current implementation with the helper functions added in the former commit. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/ohci.c | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index a0bb0e87e18a..e8429dbbc60d 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -1409,12 +1409,6 @@ static int at_context_queue_packet(struct context *ctx, d[0].control = cpu_to_le16(DESCRIPTOR_KEY_IMMEDIATE); d[0].res_count = cpu_to_le16(packet->timestamp); - /* - * The DMA format for asynchronous link packets is different - * from the IEEE1394 layout, so shift the fields around - * accordingly. - */ - tcode = async_header_get_tcode(packet->header); header = (__le32 *) &d[1]; switch (tcode) { @@ -1427,11 +1421,21 @@ static int at_context_queue_packet(struct context *ctx, case TCODE_READ_BLOCK_RESPONSE: case TCODE_LOCK_REQUEST: case TCODE_LOCK_RESPONSE: - header[0] = cpu_to_le32((packet->header[0] & 0xffff) | - (packet->speed << 16)); - header[1] = cpu_to_le32((packet->header[1] & 0xffff) | - (packet->header[0] & 0xffff0000)); - header[2] = cpu_to_le32(packet->header[2]); + ohci1394_at_data_set_src_bus_id(header, false); + ohci1394_at_data_set_speed(header, packet->speed); + ohci1394_at_data_set_tlabel(header, async_header_get_tlabel(packet->header)); + ohci1394_at_data_set_retry(header, async_header_get_retry(packet->header)); + ohci1394_at_data_set_tcode(header, tcode); + + ohci1394_at_data_set_destination_id(header, + async_header_get_destination(packet->header)); + + if (ctx == &ctx->ohci->at_response_ctx) { + ohci1394_at_data_set_rcode(header, async_header_get_rcode(packet->header)); + } else { + ohci1394_at_data_set_destination_offset(header, + async_header_get_offset(packet->header)); + } if (tcode_is_block_packet(tcode)) header[3] = cpu_to_le32(packet->header[3]); @@ -1440,10 +1444,10 @@ static int at_context_queue_packet(struct context *ctx, d[0].req_count = cpu_to_le16(packet->header_length); break; - case TCODE_LINK_INTERNAL: - header[0] = cpu_to_le32((TCODE_LINK_INTERNAL << 4) | - (packet->speed << 16)); + ohci1394_at_data_set_speed(header, packet->speed); + ohci1394_at_data_set_tcode(header, TCODE_LINK_INTERNAL); + header[1] = cpu_to_le32(packet->header[1]); header[2] = cpu_to_le32(packet->header[2]); d[0].req_count = cpu_to_le16(12); -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-02 00:36:23
|
In 1394 OHCI specification, the format of data for AT DMA is different from the format of asynchronous packet in IEEE 1394 specification, in its spd and srcBusID fields. This commit adds some static inline functions to serialize/deserialize the data of AT DMA. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/ohci-serdes-test.c | 34 ++++++++ drivers/firewire/ohci.h | 115 ++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+) diff --git a/drivers/firewire/ohci-serdes-test.c b/drivers/firewire/ohci-serdes-test.c index 304a09ff528e..c6820f4f7ec1 100644 --- a/drivers/firewire/ohci-serdes-test.c +++ b/drivers/firewire/ohci-serdes-test.c @@ -40,9 +40,43 @@ static void test_self_id_receive_buffer_deserialization(struct kunit *test) KUNIT_EXPECT_EQ(test, 0xf38b, timestamp); } +static void test_at_data_serdes(struct kunit *test) +{ + static const __le32 expected[] = { + cpu_to_le32(0x00020e80), + cpu_to_le32(0xffc2ffff), + cpu_to_le32(0xe0000000), + }; + __le32 quadlets[] = {0, 0, 0}; + bool has_src_bus_id = ohci1394_at_data_get_src_bus_id(expected); + unsigned int speed = ohci1394_at_data_get_speed(expected); + unsigned int tlabel = ohci1394_at_data_get_tlabel(expected); + unsigned int retry = ohci1394_at_data_get_retry(expected); + unsigned int tcode = ohci1394_at_data_get_tcode(expected); + unsigned int destination_id = ohci1394_at_data_get_destination_id(expected); + u64 destination_offset = ohci1394_at_data_get_destination_offset(expected); + + KUNIT_EXPECT_FALSE(test, has_src_bus_id); + KUNIT_EXPECT_EQ(test, 0x02, speed); + KUNIT_EXPECT_EQ(test, 0x03, tlabel); + KUNIT_EXPECT_EQ(test, 0x02, retry); + KUNIT_EXPECT_EQ(test, 0x08, tcode); + + ohci1394_at_data_set_src_bus_id(quadlets, has_src_bus_id); + ohci1394_at_data_set_speed(quadlets, speed); + ohci1394_at_data_set_tlabel(quadlets, tlabel); + ohci1394_at_data_set_retry(quadlets, retry); + ohci1394_at_data_set_tcode(quadlets, tcode); + ohci1394_at_data_set_destination_id(quadlets, destination_id); + ohci1394_at_data_set_destination_offset(quadlets, destination_offset); + + KUNIT_EXPECT_MEMEQ(test, quadlets, expected, sizeof(expected)); +} + static struct kunit_case ohci_serdes_test_cases[] = { KUNIT_CASE(test_self_id_count_register_deserialization), KUNIT_CASE(test_self_id_receive_buffer_deserialization), + KUNIT_CASE(test_at_data_serdes), {} }; diff --git a/drivers/firewire/ohci.h b/drivers/firewire/ohci.h index 9ed36cfc6cae..a5501996137c 100644 --- a/drivers/firewire/ohci.h +++ b/drivers/firewire/ohci.h @@ -154,6 +154,121 @@ #define OHCI1394_evt_flushed 0xf +// Asynchronous Transmit DMA. +// +// The content of first two quadlets of data for AT DMA is different from the header for IEEE 1394 +// asynchronous packet. + +#define OHCI1394_AT_DATA_Q0_srcBusID_MASK 0x00800000 +#define OHCI1394_AT_DATA_Q0_srcBusID_SHIFT 23 +#define OHCI1394_AT_DATA_Q0_spd_MASK 0x00070000 +#define OHCI1394_AT_DATA_Q0_spd_SHIFT 16 +#define OHCI1394_AT_DATA_Q0_tLabel_MASK 0x0000fc00 +#define OHCI1394_AT_DATA_Q0_tLabel_SHIFT 10 +#define OHCI1394_AT_DATA_Q0_rt_MASK 0x00000300 +#define OHCI1394_AT_DATA_Q0_rt_SHIFT 8 +#define OHCI1394_AT_DATA_Q0_tCode_MASK 0x000000f0 +#define OHCI1394_AT_DATA_Q0_tCode_SHIFT 4 +#define OHCI1394_AT_DATA_Q1_destinationId_MASK 0xffff0000 +#define OHCI1394_AT_DATA_Q1_destinationId_SHIFT 16 +#define OHCI1394_AT_DATA_Q1_destinationOffsetHigh_MASK 0x0000ffff +#define OHCI1394_AT_DATA_Q1_destinationOffsetHigh_SHIFT 0 +#define OHCI1394_AT_DATA_Q1_rCode_MASK 0x0000f000 +#define OHCI1394_AT_DATA_Q1_rCode_SHIFT 12 + +static inline bool ohci1394_at_data_get_src_bus_id(const __le32 *data) +{ + return !!((data[0] & OHCI1394_AT_DATA_Q0_srcBusID_MASK) >> OHCI1394_AT_DATA_Q0_srcBusID_SHIFT); +} + +static inline void ohci1394_at_data_set_src_bus_id(__le32 *data, bool src_bus_id) +{ + data[0] &= cpu_to_le32(~OHCI1394_AT_DATA_Q0_srcBusID_MASK); + data[0] |= cpu_to_le32((src_bus_id << OHCI1394_AT_DATA_Q0_srcBusID_SHIFT) & OHCI1394_AT_DATA_Q0_srcBusID_MASK); +} + +static inline unsigned int ohci1394_at_data_get_speed(const __le32 *data) +{ + return (le32_to_cpu(data[0]) & OHCI1394_AT_DATA_Q0_spd_MASK) >> OHCI1394_AT_DATA_Q0_spd_SHIFT; +} + +static inline void ohci1394_at_data_set_speed(__le32 *data, unsigned int scode) +{ + data[0] &= cpu_to_le32(~OHCI1394_AT_DATA_Q0_spd_MASK); + data[0] |= cpu_to_le32((scode << OHCI1394_AT_DATA_Q0_spd_SHIFT) & OHCI1394_AT_DATA_Q0_spd_MASK); +} + +static inline unsigned int ohci1394_at_data_get_tlabel(const __le32 *data) +{ + return (le32_to_cpu(data[0]) & OHCI1394_AT_DATA_Q0_tLabel_MASK) >> OHCI1394_AT_DATA_Q0_tLabel_SHIFT; +} + +static inline void ohci1394_at_data_set_tlabel(__le32 *data, unsigned int tlabel) +{ + data[0] &= cpu_to_le32(~OHCI1394_AT_DATA_Q0_tLabel_MASK); + data[0] |= cpu_to_le32((tlabel << OHCI1394_AT_DATA_Q0_tLabel_SHIFT) & OHCI1394_AT_DATA_Q0_tLabel_MASK); +} + +static inline unsigned int ohci1394_at_data_get_retry(const __le32 *data) +{ + return (le32_to_cpu(data[0]) & OHCI1394_AT_DATA_Q0_rt_MASK) >> OHCI1394_AT_DATA_Q0_rt_SHIFT; +} + +static inline void ohci1394_at_data_set_retry(__le32 *data, unsigned int retry) +{ + data[0] &= cpu_to_le32(~OHCI1394_AT_DATA_Q0_rt_MASK); + data[0] |= cpu_to_le32((retry << OHCI1394_AT_DATA_Q0_rt_SHIFT) & OHCI1394_AT_DATA_Q0_rt_MASK); +} + +static inline unsigned int ohci1394_at_data_get_tcode(const __le32 *data) +{ + return (le32_to_cpu(data[0]) & OHCI1394_AT_DATA_Q0_tCode_MASK) >> OHCI1394_AT_DATA_Q0_tCode_SHIFT; +} + +static inline void ohci1394_at_data_set_tcode(__le32 *data, unsigned int tcode) +{ + data[0] &= cpu_to_le32(~OHCI1394_AT_DATA_Q0_tCode_MASK); + data[0] |= cpu_to_le32((tcode << OHCI1394_AT_DATA_Q0_tCode_SHIFT) & OHCI1394_AT_DATA_Q0_tCode_MASK); +} + +static inline unsigned int ohci1394_at_data_get_destination_id(const __le32 *data) +{ + return (le32_to_cpu(data[1]) & OHCI1394_AT_DATA_Q1_destinationId_MASK) >> OHCI1394_AT_DATA_Q1_destinationId_SHIFT; +} + +static inline void ohci1394_at_data_set_destination_id(__le32 *data, unsigned int destination_id) +{ + data[1] &= cpu_to_le32(~OHCI1394_AT_DATA_Q1_destinationId_MASK); + data[1] |= cpu_to_le32((destination_id << OHCI1394_AT_DATA_Q1_destinationId_SHIFT) & OHCI1394_AT_DATA_Q1_destinationId_MASK); +} + +static inline u64 ohci1394_at_data_get_destination_offset(const __le32 *data) +{ + u64 hi = (u64)((le32_to_cpu(data[1]) & OHCI1394_AT_DATA_Q1_destinationOffsetHigh_MASK) >> OHCI1394_AT_DATA_Q1_destinationOffsetHigh_SHIFT); + u64 lo = (u64)le32_to_cpu(data[2]); + return (hi << 32) | lo; +} + +static inline void ohci1394_at_data_set_destination_offset(__le32 *data, u64 offset) +{ + u32 hi = (u32)(offset >> 32); + u32 lo = (u32)(offset & 0x00000000ffffffff); + data[1] &= cpu_to_le32(~OHCI1394_AT_DATA_Q1_destinationOffsetHigh_MASK); + data[1] |= cpu_to_le32((hi << OHCI1394_AT_DATA_Q1_destinationOffsetHigh_SHIFT) & OHCI1394_AT_DATA_Q1_destinationOffsetHigh_MASK); + data[2] = cpu_to_le32(lo); +} + +static inline unsigned int ohci1394_at_data_get_rcode(const __le32 *data) +{ + return (le32_to_cpu(data[1]) & OHCI1394_AT_DATA_Q1_rCode_MASK) >> OHCI1394_AT_DATA_Q1_rCode_SHIFT; +} + +static inline void ohci1394_at_data_set_rcode(__le32 *data, unsigned int rcode) +{ + data[1] &= cpu_to_le32(~OHCI1394_AT_DATA_Q1_rCode_MASK); + data[1] |= cpu_to_le32((rcode << OHCI1394_AT_DATA_Q1_rCode_SHIFT) & OHCI1394_AT_DATA_Q1_rCode_MASK); +} + // Self-ID DMA. #define OHCI1394_SelfIDCount_selfIDError_MASK 0x80000000 -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-02 00:36:17
|
Hi, In 1394 OHCI specification, the data formats of AT/IT DMA are specific and different from packert format of IEEE 1394 specification. Current implementation includes some ad-hoc lines to construct the data, while it is not so easy to read and understand. This patchset includes some helper functions to serialize and deserialize the data. These helper functions are tested by KUnit, and replace the ad-hoc implementations. Takashi Sakamoto (4): firewire: ohci: add static inline functions to serialize/deserialize data of AT DMA firewire: ohci: use static inline functions to serialize data of AT DMA firewire: ohci: add static inline functions to serialize/deserialize data of IT DMA firewire: ohci: use static inline functions to serialize data of IT DMA drivers/firewire/ohci-serdes-test.c | 66 +++++++++ drivers/firewire/ohci.c | 66 ++++----- drivers/firewire/ohci.h | 199 ++++++++++++++++++++++++++++ 3 files changed, 299 insertions(+), 32 deletions(-) -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-01 23:29:53
|
On Thu, Aug 01, 2024 at 11:26:29AM +0900, Takashi Sakamoto wrote: > Current implementation directly uses refcount_t to maintain the life time > of fw_node, while kref is available for the same purpose. > > This commit replaces the implementation with kref. > > Signed-off-by: Takashi Sakamoto <o-t...@sa...> > --- > drivers/firewire/core-topology.c | 2 +- > drivers/firewire/core.h | 15 +++++++++++---- > 2 files changed, 12 insertions(+), 5 deletions(-) > > diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c > index b4e637aa6932..46e6eb287d24 100644 > --- a/drivers/firewire/core-topology.c > +++ b/drivers/firewire/core-topology.c > @@ -39,7 +39,7 @@ static struct fw_node *fw_node_create(u32 sid, int port_count, int color) > node->initiated_reset = phy_packet_self_id_zero_get_initiated_reset(sid); > node->port_count = port_count; > > - refcount_set(&node->ref_count, 1); > + kref_init(&node->kref); > INIT_LIST_HEAD(&node->link); > > return node; > diff --git a/drivers/firewire/core.h b/drivers/firewire/core.h > index 7c36d2628e37..189e15e6ba82 100644 > --- a/drivers/firewire/core.h > +++ b/drivers/firewire/core.h > @@ -183,7 +183,8 @@ struct fw_node { > * local node to this node. */ > u8 max_depth:4; /* Maximum depth to any leaf node */ > u8 max_hops:4; /* Max hops in this sub tree */ > - refcount_t ref_count; > + > + struct kref kref; > > /* For serializing node topology into a list. */ > struct list_head link; > @@ -196,15 +197,21 @@ struct fw_node { > > static inline struct fw_node *fw_node_get(struct fw_node *node) > { > - refcount_inc(&node->ref_count); > + kref_get(&node->kref); > > return node; > } > > +static void release_node(struct kref *kref) > +{ > + struct fw_node *node = container_of(kref, struct fw_node, kref); > + > + kfree(node); > +} > + > static inline void fw_node_put(struct fw_node *node) > { > - if (refcount_dec_and_test(&node->ref_count)) > - kfree(node); > + kref_put(&node->kref, release_node); > } > > void fw_core_handle_bus_reset(struct fw_card *card, int node_id, Applied to for-next branch. Regards Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2024-08-01 02:26:40
|
Current implementation directly uses refcount_t to maintain the life time of fw_node, while kref is available for the same purpose. This commit replaces the implementation with kref. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-topology.c | 2 +- drivers/firewire/core.h | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c index b4e637aa6932..46e6eb287d24 100644 --- a/drivers/firewire/core-topology.c +++ b/drivers/firewire/core-topology.c @@ -39,7 +39,7 @@ static struct fw_node *fw_node_create(u32 sid, int port_count, int color) node->initiated_reset = phy_packet_self_id_zero_get_initiated_reset(sid); node->port_count = port_count; - refcount_set(&node->ref_count, 1); + kref_init(&node->kref); INIT_LIST_HEAD(&node->link); return node; diff --git a/drivers/firewire/core.h b/drivers/firewire/core.h index 7c36d2628e37..189e15e6ba82 100644 --- a/drivers/firewire/core.h +++ b/drivers/firewire/core.h @@ -183,7 +183,8 @@ struct fw_node { * local node to this node. */ u8 max_depth:4; /* Maximum depth to any leaf node */ u8 max_hops:4; /* Max hops in this sub tree */ - refcount_t ref_count; + + struct kref kref; /* For serializing node topology into a list. */ struct list_head link; @@ -196,15 +197,21 @@ struct fw_node { static inline struct fw_node *fw_node_get(struct fw_node *node) { - refcount_inc(&node->ref_count); + kref_get(&node->kref); return node; } +static void release_node(struct kref *kref) +{ + struct fw_node *node = container_of(kref, struct fw_node, kref); + + kfree(node); +} + static inline void fw_node_put(struct fw_node *node) { - if (refcount_dec_and_test(&node->ref_count)) - kfree(node); + kref_put(&node->kref, release_node); } void fw_core_handle_bus_reset(struct fw_card *card, int node_id, -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-07-30 12:36:23
|
On Mon, Jul 29, 2024 at 10:46:28PM +0900, Takashi Sakamoto wrote: > Hi, > > The current implementation of both core functions and 1394 OHCI driver has > some points that need to be refactored regarding the handling of link- > internal data. > > This patchset is for the purpose. > > Takashi Sakamoto (3): > firewire: ohci: use TCODE_LINK_INTERNAL consistently > firewire: ohci: minor code refactoring to localize text table > firewire: core: use common helper function to serialize phy > configuration packet > > drivers/firewire/core-cdev.c | 4 +++- > drivers/firewire/core-transaction.c | 2 +- > drivers/firewire/ohci.c | 37 +++++++++++++++++------------ > drivers/firewire/ohci.h | 1 - > 4 files changed, 26 insertions(+), 18 deletions(-) Applied to for-next branch. Regards Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2024-07-29 13:46:53
|
A common helper function is available to serialize the first quadlet of phy configuration packet. This commit is for the purpose. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-cdev.c | 4 +++- drivers/firewire/core-transaction.c | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index 9a7dc90330a3..619048dcfd72 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -37,6 +37,8 @@ #include "core.h" #include <trace/events/firewire.h> +#include "packet-header-definitions.h" + /* * ABI version history is documented in linux/firewire-cdev.h. */ @@ -1635,7 +1637,7 @@ static int ioctl_send_phy_packet(struct client *client, union ioctl_arg *arg) e->client = client; e->p.speed = SCODE_100; e->p.generation = a->generation; - e->p.header[0] = TCODE_LINK_INTERNAL << 4; + async_header_set_tcode(e->p.header, TCODE_LINK_INTERNAL); e->p.header[1] = a->data[0]; e->p.header[2] = a->data[1]; e->p.header_length = 12; diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c index 4d2fc1f31fec..a89c841a7dbe 100644 --- a/drivers/firewire/core-transaction.c +++ b/drivers/firewire/core-transaction.c @@ -464,7 +464,6 @@ static void transmit_phy_packet_callback(struct fw_packet *packet, static struct fw_packet phy_config_packet = { .header_length = 12, - .header[0] = TCODE_LINK_INTERNAL << 4, .payload_length = 0, .speed = SCODE_100, .callback = transmit_phy_packet_callback, @@ -497,6 +496,7 @@ void fw_send_phy_config(struct fw_card *card, mutex_lock(&phy_config_mutex); + async_header_set_tcode(phy_config_packet.header, TCODE_LINK_INTERNAL); phy_config_packet.header[1] = data; phy_config_packet.header[2] = ~data; phy_config_packet.generation = generation; -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-07-29 13:46:51
|
The string table for tcode is just used by log_ar_at_event(). In the case, it is suitable to move the table inner the function definition. This commit is for the purpose. Additionally, the hard-coded value for tcode is replaced with defined macros as many as possible. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/ohci.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index c3fff94b13e5..a0bb0e87e18a 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -531,20 +531,28 @@ static const char *evts[] = { [0x1e] = "ack_type_error", [0x1f] = "-reserved-", [0x20] = "pending/cancelled", }; -static const char *tcodes[] = { - [0x0] = "QW req", [0x1] = "BW req", - [0x2] = "W resp", [0x3] = "-reserved-", - [0x4] = "QR req", [0x5] = "BR req", - [0x6] = "QR resp", [0x7] = "BR resp", - [0x8] = "cycle start", [0x9] = "Lk req", - [0xa] = "async stream packet", [0xb] = "Lk resp", - [0xc] = "-reserved-", [0xd] = "-reserved-", - [0xe] = "link internal", [0xf] = "-reserved-", -}; static void log_ar_at_event(struct fw_ohci *ohci, char dir, int speed, u32 *header, int evt) { + static const char *const tcodes[] = { + [TCODE_WRITE_QUADLET_REQUEST] = "QW req", + [TCODE_WRITE_BLOCK_REQUEST] = "BW req", + [TCODE_WRITE_RESPONSE] = "W resp", + [0x3] = "-reserved-", + [TCODE_READ_QUADLET_REQUEST] = "QR req", + [TCODE_READ_BLOCK_REQUEST] = "BR req", + [TCODE_READ_QUADLET_RESPONSE] = "QR resp", + [TCODE_READ_BLOCK_RESPONSE] = "BR resp", + [TCODE_CYCLE_START] = "cycle start", + [TCODE_LOCK_REQUEST] = "Lk req", + [TCODE_STREAM_DATA] = "async stream packet", + [TCODE_LOCK_RESPONSE] = "Lk resp", + [0xc] = "-reserved-", + [0xd] = "-reserved-", + [TCODE_LINK_INTERNAL] = "link internal", + [0xf] = "-reserved-", + }; int tcode = async_header_get_tcode(header); char specific[12]; -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-07-29 13:46:47
|
Hi, The current implementation of both core functions and 1394 OHCI driver has some points that need to be refactored regarding the handling of link- internal data. This patchset is for the purpose. Takashi Sakamoto (3): firewire: ohci: use TCODE_LINK_INTERNAL consistently firewire: ohci: minor code refactoring to localize text table firewire: core: use common helper function to serialize phy configuration packet drivers/firewire/core-cdev.c | 4 +++- drivers/firewire/core-transaction.c | 2 +- drivers/firewire/ohci.c | 37 +++++++++++++++++------------ drivers/firewire/ohci.h | 1 - 4 files changed, 26 insertions(+), 18 deletions(-) -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-07-29 13:46:45
|
In IEEE 1394 specification, 0x0e in tcode field is reserved for internal purpose depending on link layer. In 1394 OHCI specification, it is used to express phy packet in AT/AR contexts. Current implementation of 1394 OHCI driver has several macros for the code. They can be simply replaced with a macro in core code. This commit obsoletes the macros. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/ohci.c | 9 ++++----- drivers/firewire/ohci.h | 1 - 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 314a29c0fd3e..c3fff94b13e5 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -264,7 +264,6 @@ static inline struct fw_ohci *fw_ohci(struct fw_card *card) #define OHCI1394_REGISTER_SIZE 0x800 #define OHCI1394_PCI_HCI_Control 0x40 #define SELF_ID_BUF_SIZE 0x800 -#define OHCI_TCODE_PHY_PACKET 0x0e #define OHCI_VERSION_1_1 0x010010 static char ohci_driver_name[] = KBUILD_MODNAME; @@ -586,7 +585,7 @@ static void log_ar_at_event(struct fw_ohci *ohci, ohci_notice(ohci, "A%c %s, %s\n", dir, evts[evt], tcodes[tcode]); break; - case 0xe: + case TCODE_LINK_INTERNAL: ohci_notice(ohci, "A%c %s, PHY %08x %08x\n", dir, evts[evt], header[1], header[2]); break; @@ -939,7 +938,7 @@ static __le32 *handle_ar_packet(struct ar_context *ctx, __le32 *buffer) case TCODE_WRITE_RESPONSE: case TCODE_READ_QUADLET_REQUEST: - case OHCI_TCODE_PHY_PACKET: + case TCODE_LINK_INTERNAL: p.header_length = 12; p.payload_length = 0; break; @@ -967,7 +966,7 @@ static __le32 *handle_ar_packet(struct ar_context *ctx, __le32 *buffer) * Several controllers, notably from NEC and VIA, forget to * write ack_complete status at PHY packet reception. */ - if (evt == OHCI1394_evt_no_status && tcode == OHCI1394_phy_tcode) + if (evt == OHCI1394_evt_no_status && tcode == TCODE_LINK_INTERNAL) p.ack = ACK_COMPLETE; /* @@ -1435,7 +1434,7 @@ static int at_context_queue_packet(struct context *ctx, break; case TCODE_LINK_INTERNAL: - header[0] = cpu_to_le32((OHCI1394_phy_tcode << 4) | + header[0] = cpu_to_le32((TCODE_LINK_INTERNAL << 4) | (packet->speed << 16)); header[1] = cpu_to_le32(packet->header[1]); header[2] = cpu_to_le32(packet->header[2]); diff --git a/drivers/firewire/ohci.h b/drivers/firewire/ohci.h index 71c2ed84cafb..9ed36cfc6cae 100644 --- a/drivers/firewire/ohci.h +++ b/drivers/firewire/ohci.h @@ -153,7 +153,6 @@ #define OHCI1394_evt_unknown 0xe #define OHCI1394_evt_flushed 0xf -#define OHCI1394_phy_tcode 0xe // Self-ID DMA. -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-07-27 01:51:59
|
Hi Linus, Please apply the following changes for v6.11 kernel to fix some regressions appended to v6.10 kernel. They include the change of sound subsystem and I have got an ack of sound subsystem maintainer. The following changes since commit 06dcc4c9baa9e92896f00d02ffa760c0988b4371: firewire: core: move copy_port_status() helper function to TP_fast_assign() block (2024-07-12 14:34:12 +0900) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git tags/firewire-fixes-6.11-rc1 for you to fetch changes up to c1839501fe3e67d98635f159dba8b170d08f6521: ALSA: firewire-lib: fix wrong value as length of header for CIP_NO_HEADER case (2024-07-27 10:28:36 +0900) ---------------------------------------------------------------- firewire fixes for 6.11-rc1 Two commits are included to fix some regressions. These commits should also be applied to the v6.10 kernel as well. The recent integration of compiler collections introduced the technology to check flexible array length at runtime by providing proper annotations. In v6.10 kernel, a patch was merged into firewire subsystem to utilize it, however the annotation was inadequate. There is also the related change for the flexible array in sound subsystem, but it causes a regression where the data in the payload of isochronous packet is incorrect for some devices. These bugs are now fixed. ---------------------------------------------------------------- Takashi Sakamoto (2): Revert "firewire: Annotate struct fw_iso_packet with __counted_by()" ALSA: firewire-lib: fix wrong value as length of header for CIP_NO_HEADER case include/linux/firewire.h | 5 ++--- sound/firewire/amdtp-stream.c | 3 +-- 2 files changed, 3 insertions(+), 5 deletions(-) Thanks Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2024-07-27 01:37:13
|
On Fri, Jul 26, 2024 at 01:16:48AM +0900, Takashi Sakamoto wrote: > This reverts commit d3155742db89df3b3c96da383c400e6ff4d23c25. > > The header_length field is byte unit, thus it can not express the number of > elements in header field. > > Signed-off-by: Takashi Sakamoto <o-t...@sa...> > --- > include/linux/firewire.h | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/include/linux/firewire.h b/include/linux/firewire.h > index 00abe0e5d602..1cca14cf5652 100644 > --- a/include/linux/firewire.h > +++ b/include/linux/firewire.h > @@ -462,9 +462,8 @@ struct fw_iso_packet { > /* rx: Sync bit, wait for matching sy */ > u32 tag:2; /* tx: Tag in packet header */ > u32 sy:4; /* tx: Sy in packet header */ > - u32 header_length:8; /* Length of immediate header */ > - /* tx: Top of 1394 isoch. data_block */ > - u32 header[] __counted_by(header_length); > + u32 header_length:8; /* Size of immediate header */ > + u32 header[]; /* tx: Top of 1394 isoch. data_block */ > }; > > #define FW_ISO_CONTEXT_TRANSMIT 0 Applied to for-linus branch in firewire subsystem tree[1], with slight change of comment. It would be sent to mainline today with another patch[2]. [1] https://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git/log/?h=for-linus [2] https://lore.kernel.org/stable/202...@sa.../ Regards Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2024-07-25 16:17:01
|
This reverts commit d3155742db89df3b3c96da383c400e6ff4d23c25. The header_length field is byte unit, thus it can not express the number of elements in header field. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- include/linux/firewire.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/include/linux/firewire.h b/include/linux/firewire.h index 00abe0e5d602..1cca14cf5652 100644 --- a/include/linux/firewire.h +++ b/include/linux/firewire.h @@ -462,9 +462,8 @@ struct fw_iso_packet { /* rx: Sync bit, wait for matching sy */ u32 tag:2; /* tx: Tag in packet header */ u32 sy:4; /* tx: Sy in packet header */ - u32 header_length:8; /* Length of immediate header */ - /* tx: Top of 1394 isoch. data_block */ - u32 header[] __counted_by(header_length); + u32 header_length:8; /* Size of immediate header */ + u32 header[]; /* tx: Top of 1394 isoch. data_block */ }; #define FW_ISO_CONTEXT_TRANSMIT 0 -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-07-20 08:47:48
|
Hi Linus, Please pull FireWire subsystem updates for v6.11. The following changes since commit 6ba59ff4227927d3a8530fc2973b80e94b54d58f: Linux 6.10-rc4 (2024-06-16 13:40:16 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git tags/firewire-updates-6.11 for you to fetch changes up to 06dcc4c9baa9e92896f00d02ffa760c0988b4371: firewire: core: move copy_port_status() helper function to TP_fast_assign() block (2024-07-12 14:34:12 +0900) ---------------------------------------------------------------- firewire updates for v6.11 There are many line changes for FireWire subsystem, while there is no functional change for practical use. The most of changes are for code refactoring, some KUnit tests to added helper functions, and new tracepoints events for both the core functions and 1394 OHCI driver. The tracepoints events now cover the verbose logging enabled by debug parameter of firewire-ohci kernel module. The parameter would be removed in any future timing, thus it is now deprecated. ---------------------------------------------------------------- Colin Ian King (1): firewire: core: Fix spelling mistakes in tracepoint messages Takashi Sakamoto (31): 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 firewire: core: add tests for serialization/deserialization of phy config packet firewire: core: use inline helper functions to serialize phy config packet firewire: core: record card index in tracepoints event for self ID sequence firewire: core: undefine macros after use in tracepoints events firewire: core: add tracepoints events for allocation/deallocation of isochronous context firewire: core: add tracepoints events for setting channels of multichannel context firewire: core: add tracepoints events for starting/stopping of isochronous context firewire: core: add tracepoints events for flushing of isochronous context firewire: core: add tracepoints events for flushing completions of isochronous context firewire: core: add tracepoints events for queueing packets of isochronous context firewire: core: add tracepoints events for completions of packets in isochronous context firewire: ohci: add support for Linux kernel tracepoints firewire: ohci: add tracepoints event for hardIRQ event firewire: ohci: use common macro to interpret be32 data in le32 buffer firewire: ohci: use static function to handle endian issue on PowerPC platform firewire: ohci: add static inline functions to deserialize for Self-ID DMA operation firewire: ohci: use inline functions to operate data of self-ID DMA firewire: ohci: add tracepoints event for data of Self-ID DMA Revert "firewire: ohci: use common macro to interpret be32 data in le32 buffer" firewire: core: move copy_port_status() helper function to TP_fast_assign() block drivers/firewire/.kunitconfig | 2 + drivers/firewire/Kconfig | 31 +++++++++++ drivers/firewire/Makefile | 2 + drivers/firewire/core-iso.c | 32 +++++++++++ drivers/firewire/core-topology.c | 219 +++++++++++++++++++++++++++++---------------------------------------------- drivers/firewire/core-trace.c | 11 ++++ drivers/firewire/core-transaction.c | 24 ++++----- drivers/firewire/ohci-serdes-test.c | 56 ++++++++++++++++++++ drivers/firewire/ohci.c | 237 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------- drivers/firewire/ohci.h | 43 ++++++++++++++- drivers/firewire/packet-header-definitions.h | 2 + drivers/firewire/packet-serdes-test.c | 334 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ drivers/firewire/phy-packet-definitions.h | 302 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ drivers/firewire/self-id-sequence-helper-test.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++++++++ include/trace/events/firewire.h | 541 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- include/trace/events/firewire_ohci.h | 101 +++++++++++++++++++++++++++++++++++ 16 files changed, 1867 insertions(+), 222 deletions(-) create mode 100644 drivers/firewire/ohci-serdes-test.c create mode 100644 drivers/firewire/phy-packet-definitions.h create mode 100644 drivers/firewire/self-id-sequence-helper-test.c create mode 100644 include/trace/events/firewire_ohci.h Regards Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2024-07-13 00:38:54
|
On Fri, Jul 12, 2024 at 09:30:10AM +0900, Takashi Sakamoto wrote: > 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. > > Signed-off-by: Takashi Sakamoto <o-t...@sa...> > --- > drivers/firewire/core-trace.c | 11 ----------- > include/trace/events/firewire.h | 15 ++++++++++----- > 2 files changed, 10 insertions(+), 16 deletions(-) Applied to for-next branch. Regards Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2024-07-12 00:30:26
|
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. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- 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 <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); - } -} - 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)); ), -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-07-12 00:25:20
|
Hi, On Fri, Jul 12, 2024 at 09:04:24AM +0900, Takashi Sakamoto wrote: > 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. > > Signed-off-by: Takashi Sakamoto <o-t...@sa...> > --- > drivers/firewire/core-trace.c | 11 ----------- > include/trace/events/firewire.h | 12 ++++++++++-- > 2 files changed, 10 insertions(+), 13 deletions(-) I realized that the prototype of helper function still exists in 'include/trace/events/firewire.h'. Let me abandon this patch and resend the revised version. Regards Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2024-07-12 00:04:36
|
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. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-trace.c | 11 ----------- include/trace/events/firewire.h | 12 ++++++++++-- 2 files changed, 10 insertions(+), 13 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 <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); - } -} - 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..57a2be01d31a 100644 --- a/include/trace/events/firewire.h +++ b/include/trace/events/firewire.h @@ -407,8 +407,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)); ), -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-07-07 21:43:43
|
On Sun, Jul 07, 2024 at 10:45:23PM +0900, Takashi Sakamoto wrote: > 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 <lk...@in...> > Closes: https://lore.kernel.org/oe-kbuild-all/202...@in.../ > Signed-off-by: Takashi Sakamoto <o-t...@sa...> > --- > drivers/firewire/ohci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied to for-next branch. Regards Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2024-07-07 13:45:41
|
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 <lk...@in...> Closes: https://lore.kernel.org/oe-kbuild-all/202...@in.../ Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- 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) -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-07-04 00:07:17
|
Hi, On Wed, Jul 03, 2024 at 07:20:34AM +0900, Takashi Sakamoto wrote: > 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/softIRQ contexts. We can often see this kind of problem with -rt > kernel[2]. Oops, I misunderstood. The handling of Self-ID DMA buffer is in hardIRQ and process (workqueue) contexts, thus the above issue is not the invocation timing of hardIRQ/softIRQ contexts. Let me correct it at the applied commit comment. > [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/ > > Signed-off-by: Takashi Sakamoto <o-t...@sa...> > --- > drivers/firewire/ohci.c | 11 +++++- > include/trace/events/firewire_ohci.h | 54 ++++++++++++++++++++++++++++ > 2 files changed, 64 insertions(+), 1 deletion(-) Regards Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2024-07-03 23:30:59
|
On Wed, Jul 03, 2024 at 07:20:29AM +0900, Takashi Sakamoto wrote: > Hi, > > In IEEE 1394, Self identification process is the last step at > bus configuration process initiated by reset. In this step, all nodes at > the bus broadcast self-ID packet according to the new topology. In 1394 > OHCI, the contents of the self-ID packet stream are transmitted to host > system by DMA. This is Self-ID DMA, and configured by two registers, > signaled by an interrupt, has a format of buffer. > > Software handles the interrupt, then parse the contents of self-ID packet > stream to detect all nodes in the bus. Finally it starts any type of > operation in the bus. Both the timing to handle the interrupt and the > data of DMA buffer are important as a first step to use the bus. > > This series of patches is to add a tracepoints event to trace the > interrupt and the data of DMA buffer. It would be useful to diagnose > some kind of issue related to the self-ID process; e.g. hardware quirks. > I also expect it useful to investigate the timing issue between hardIRQ > and softIRQ contexts to process the contents of self-ID packets. > > Takashi Sakamoto (5): > firewire: ohci: use common macro to interpret be32 data in le32 buffer > firewire: ohci: use static function to handle endian issue on PowerPC > platform > firewire: ohci: add static inline functions to deserialize for Self-ID > DMA operation > firewire: ohci: use inline functions to operate data of self-ID DMA > firewire: ohci: add tracepoints event for data of Self-ID DMA > > drivers/firewire/.kunitconfig | 1 + > drivers/firewire/Kconfig | 16 ++++++++ > drivers/firewire/Makefile | 1 + > drivers/firewire/ohci-serdes-test.c | 56 ++++++++++++++++++++++++++ > drivers/firewire/ohci.c | 60 ++++++++++++++++++++-------- > drivers/firewire/ohci.h | 43 +++++++++++++++++++- > include/trace/events/firewire_ohci.h | 54 +++++++++++++++++++++++++ > 7 files changed, 213 insertions(+), 18 deletions(-) > create mode 100644 drivers/firewire/ohci-serdes-test.c Applied to for-next branch. Regards Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2024-07-02 22:20:56
|
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. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- 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 -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-07-02 22:20:55
|
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. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- 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 <kunit/test.h> + +#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 */ -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-07-02 22:20:53
|
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. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- 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) { /* -- 2.43.0 |