linux1394-devel Mailing List for IEEE 1394 for Linux (Page 17)
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
(42) |
Oct
(1) |
Nov
|
Dec
|
From: Takashi S. <o-t...@sa...> - 2023-05-25 10:16:50
|
The callback function now receives an argument for time stamps relevant to asynchronous transaction. This commit implements a new event to notify response subaction with the time stamps for user space. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-cdev.c | 96 ++++++++++++++++++++++++------------ 1 file changed, 65 insertions(+), 31 deletions(-) diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index 315ebc8c545d..8b24abdd51b8 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -172,6 +172,7 @@ struct outbound_transaction_event { struct outbound_transaction_resource r; union { struct fw_cdev_event_response without_tstamp; + struct fw_cdev_event_response2 with_tstamp; } rsp; }; @@ -538,41 +539,64 @@ static void release_transaction(struct client *client, { } -static void complete_transaction(struct fw_card *card, int rcode, - void *payload, size_t length, void *data) +static void complete_transaction(struct fw_card *card, int rcode, u32 request_tstamp, + u32 response_tstamp, void *payload, size_t length, void *data) { struct outbound_transaction_event *e = data; - struct fw_cdev_event_response *rsp = &e->rsp.without_tstamp; struct client *client = e->client; unsigned long flags; - if (length < rsp->length) - rsp->length = length; - if (rcode == RCODE_COMPLETE) - memcpy(rsp->data, payload, rsp->length); - spin_lock_irqsave(&client->lock, flags); idr_remove(&client->resource_idr, e->r.resource.handle); if (client->in_shutdown) wake_up(&client->tx_flush_wait); spin_unlock_irqrestore(&client->lock, flags); - rsp->type = FW_CDEV_EVENT_RESPONSE; - rsp->rcode = rcode; + switch (e->rsp.without_tstamp.type) { + case FW_CDEV_EVENT_RESPONSE: + { + struct fw_cdev_event_response *rsp = &e->rsp.without_tstamp; + + if (length < rsp->length) + rsp->length = length; + if (rcode == RCODE_COMPLETE) + memcpy(rsp->data, payload, rsp->length); + + rsp->rcode = rcode; + + // In the case that sizeof(*rsp) doesn't align with the position of the + // data, and the read is short, preserve an extra copy of the data + // to stay compatible with a pre-2.6.27 bug. Since the bug is harmless + // for short reads and some apps depended on it, this is both safe + // and prudent for compatibility. + if (rsp->length <= sizeof(*rsp) - offsetof(typeof(*rsp), data)) + queue_event(client, &e->event, rsp, sizeof(*rsp), rsp->data, rsp->length); + else + queue_event(client, &e->event, rsp, sizeof(*rsp) + rsp->length, NULL, 0); - /* - * In the case that sizeof(*rsp) doesn't align with the position of the - * data, and the read is short, preserve an extra copy of the data - * to stay compatible with a pre-2.6.27 bug. Since the bug is harmless - * for short reads and some apps depended on it, this is both safe - * and prudent for compatibility. - */ - if (rsp->length <= sizeof(*rsp) - offsetof(typeof(*rsp), data)) - queue_event(client, &e->event, rsp, sizeof(*rsp), - rsp->data, rsp->length); - else - queue_event(client, &e->event, rsp, sizeof(*rsp) + rsp->length, - NULL, 0); + break; + } + case FW_CDEV_EVENT_RESPONSE2: + { + struct fw_cdev_event_response2 *rsp = &e->rsp.with_tstamp; + + if (length < rsp->length) + rsp->length = length; + if (rcode == RCODE_COMPLETE) + memcpy(rsp->data, payload, rsp->length); + + rsp->rcode = rcode; + rsp->request_tstamp = request_tstamp; + rsp->response_tstamp = response_tstamp; + + queue_event(client, &e->event, rsp, sizeof(*rsp) + rsp->length, NULL, 0); + + break; + default: + WARN_ON(1); + break; + } + } /* Drop the idr's reference */ client_put(client); @@ -583,7 +607,6 @@ static int init_request(struct client *client, int destination_id, int speed) { struct outbound_transaction_event *e; - struct fw_cdev_event_response *rsp; void *payload; int ret; @@ -600,10 +623,21 @@ static int init_request(struct client *client, return -ENOMEM; e->client = client; - rsp = &e->rsp.without_tstamp; - rsp->length = request->length; - rsp->closure = request->closure; - payload = rsp->data; + if (client->version < FW_CDEV_VERSION_EVENT_ASYNC_TSTAMP) { + struct fw_cdev_event_response *rsp = &e->rsp.without_tstamp; + + rsp->type = FW_CDEV_EVENT_RESPONSE; + rsp->length = request->length; + rsp->closure = request->closure; + payload = rsp->data; + } else { + struct fw_cdev_event_response2 *rsp = &e->rsp.with_tstamp; + + rsp->type = FW_CDEV_EVENT_RESPONSE2; + rsp->length = request->length; + rsp->closure = request->closure; + payload = rsp->data; + } if (request->data && copy_from_user(payload, u64_to_uptr(request->data), request->length)) { ret = -EFAULT; @@ -615,9 +649,9 @@ static int init_request(struct client *client, if (ret < 0) goto failed; - fw_send_request(client->device->card, &e->r.transaction, request->tcode, destination_id, - request->generation, speed, request->offset, payload, request->length, - complete_transaction, e); + fw_send_request_with_tstamp(client->device->card, &e->r.transaction, request->tcode, + destination_id, request->generation, speed, request->offset, + payload, request->length, complete_transaction, e); return 0; failed: -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-05-25 10:16:49
|
In the previous commit, the core function of Linux FireWire subsystem was changed for two cases to operate asynchronous transaction with or without time stamp. This commit changes kernel API for the two cases. Current kernel API, fw_send_request(), is changed to be static inline function to call __fw_send_request(), which receives two argument for union and flag of callback function. The new kernel API, fw_send_request_with_tstamp() is also added as static inline function, too. When calling, the two arguments are copied to internal structure, then used in softIRQ context. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-transaction.c | 41 +++++++++++------ include/linux/firewire.h | 69 +++++++++++++++++++++++++++-- 2 files changed, 92 insertions(+), 18 deletions(-) diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c index a20f97fdd06c..130b95aca629 100644 --- a/drivers/firewire/core-transaction.c +++ b/drivers/firewire/core-transaction.c @@ -316,7 +316,8 @@ static int allocate_tlabel(struct fw_card *card) } /** - * fw_send_request() - submit a request packet for transmission + * __fw_send_request() - submit a request packet for transmission to generate callback for response + * subaction with or without time stamp. * @card: interface to send the request at * @t: transaction instance to which the request belongs * @tcode: transaction code @@ -326,7 +327,9 @@ static int allocate_tlabel(struct fw_card *card) * @offset: 48bit wide offset into destination's address space * @payload: data payload for the request subaction * @length: length of the payload, in bytes - * @callback: function to be called when the transaction is completed + * @callback: union of two functions whether to receive time stamp or not for response + * subaction. + * @with_tstamp: Whether to receive time stamp or not for response subaction. * @callback_data: data to be passed to the transaction completion callback * * Submit a request packet into the asynchronous request transmission queue. @@ -363,10 +366,10 @@ static int allocate_tlabel(struct fw_card *card) * transaction completion and hence execution of @callback may happen even * before fw_send_request() returns. */ -void fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode, - int destination_id, int generation, int speed, - unsigned long long offset, void *payload, size_t length, - fw_transaction_callback_t callback, void *callback_data) +void __fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode, + int destination_id, int generation, int speed, unsigned long long offset, + void *payload, size_t length, union fw_transaction_callback callback, + bool with_tstamp, void *callback_data) { unsigned long flags; int tlabel; @@ -381,7 +384,19 @@ void fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode, tlabel = allocate_tlabel(card); if (tlabel < 0) { spin_unlock_irqrestore(&card->lock, flags); - callback(card, RCODE_SEND_ERROR, NULL, 0, callback_data); + if (!with_tstamp) { + callback.without_tstamp(card, RCODE_SEND_ERROR, NULL, 0, callback_data); + } else { + // Timestamping on behalf of hardware. + u32 curr_cycle_time = 0; + u32 tstamp; + + (void)fw_card_read_cycle_time(card, &curr_cycle_time); + tstamp = cycle_time_to_ohci_tstamp(curr_cycle_time); + + callback.with_tstamp(card, RCODE_SEND_ERROR, tstamp, tstamp, NULL, 0, + callback_data); + } return; } @@ -389,14 +404,12 @@ void fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode, t->tlabel = tlabel; t->card = card; t->is_split_transaction = false; - timer_setup(&t->split_timeout_timer, - split_transaction_timeout_callback, 0); - t->callback.without_tstamp = callback; - t->with_tstamp = false; + timer_setup(&t->split_timeout_timer, split_transaction_timeout_callback, 0); + t->callback = callback; + t->with_tstamp = with_tstamp; t->callback_data = callback_data; - fw_fill_request(&t->packet, tcode, t->tlabel, - destination_id, card->node_id, generation, + fw_fill_request(&t->packet, tcode, t->tlabel, destination_id, card->node_id, generation, speed, offset, payload, length); t->packet.callback = transmit_complete_callback; @@ -406,7 +419,7 @@ void fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode, card->driver->send_request(card, &t->packet); } -EXPORT_SYMBOL(fw_send_request); +EXPORT_SYMBOL_GPL(__fw_send_request); struct transaction_callback_data { struct completion done; diff --git a/include/linux/firewire.h b/include/linux/firewire.h index d61693341da1..a7fd23d0010d 100644 --- a/include/linux/firewire.h +++ b/include/linux/firewire.h @@ -356,10 +356,71 @@ void fw_send_response(struct fw_card *card, struct fw_request *request, int rcode); int fw_get_request_speed(struct fw_request *request); u32 fw_request_get_timestamp(const struct fw_request *request); -void fw_send_request(struct fw_card *card, struct fw_transaction *t, - int tcode, int destination_id, int generation, int speed, - unsigned long long offset, void *payload, size_t length, - fw_transaction_callback_t callback, void *callback_data); + +void __fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode, + int destination_id, int generation, int speed, unsigned long long offset, + void *payload, size_t length, union fw_transaction_callback callback, + bool with_tstamp, void *callback_data); + +/** + * fw_send_request() - submit a request packet for transmission to generate callback for response + * subaction without time stamp. + * @card: interface to send the request at + * @t: transaction instance to which the request belongs + * @tcode: transaction code + * @destination_id: destination node ID, consisting of bus_ID and phy_ID + * @generation: bus generation in which request and response are valid + * @speed: transmission speed + * @offset: 48bit wide offset into destination's address space + * @payload: data payload for the request subaction + * @length: length of the payload, in bytes + * @callback: function to be called when the transaction is completed + * @callback_data: data to be passed to the transaction completion callback + * + * A variation of __fw_send_request() to generate callback for response subaction without time + * stamp. + */ +static inline void fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode, + int destination_id, int generation, int speed, + unsigned long long offset, void *payload, size_t length, + fw_transaction_callback_t callback, void *callback_data) +{ + union fw_transaction_callback cb = { + .without_tstamp = callback, + }; + __fw_send_request(card, t, tcode, destination_id, generation, speed, offset, payload, + length, cb, false, callback_data); +} + +/** + * fw_send_request_with_tstamp() - submit a request packet for transmission to generate callback for + * response with time stamp. + * @card: interface to send the request at + * @t: transaction instance to which the request belongs + * @tcode: transaction code + * @destination_id: destination node ID, consisting of bus_ID and phy_ID + * @generation: bus generation in which request and response are valid + * @speed: transmission speed + * @offset: 48bit wide offset into destination's address space + * @payload: data payload for the request subaction + * @length: length of the payload, in bytes + * @callback: function to be called when the transaction is completed + * @callback_data: data to be passed to the transaction completion callback + * + * A variation of __fw_send_request() to generate callback for response subaction with time stamp. + */ +static inline void fw_send_request_with_tstamp(struct fw_card *card, struct fw_transaction *t, + int tcode, int destination_id, int generation, int speed, unsigned long long offset, + void *payload, size_t length, fw_transaction_callback_with_tstamp_t callback, + void *callback_data) +{ + union fw_transaction_callback cb = { + .with_tstamp = callback, + }; + __fw_send_request(card, t, tcode, destination_id, generation, speed, offset, payload, + length, cb, true, callback_data); +} + int fw_cancel_transaction(struct fw_card *card, struct fw_transaction *transaction); int fw_run_transaction(struct fw_card *card, int tcode, int destination_id, -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-05-25 10:16:49
|
This commit adds new event to notify event of response subaction with time stamp field. Current compiler implementation of System V ABI selects one of structure members which has the maximum alignment size in the structure to decide the size of structure. In the case of fw_cdev_event_request3 structure, it is closure member which has 8 byte storage. The size of alignment for the type of 8 byte storage differs depending on architectures; 4 byte for i386 architecture and 8 byte for the others including x32 architecture. It is inconvenient to device driver developer to use structure layout which varies between architectures since the developer takes care of ioctl compat layer. This commit adds 32 bit member for padding to keep the size of structure as multiples of 8. Cc: kun...@go... Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/uapi-test.c | 15 ++++++++ include/uapi/linux/firewire-cdev.h | 59 +++++++++++++++++++++++++----- 2 files changed, 64 insertions(+), 10 deletions(-) diff --git a/drivers/firewire/uapi-test.c b/drivers/firewire/uapi-test.c index c7c713babaa0..c342ba474ee6 100644 --- a/drivers/firewire/uapi-test.c +++ b/drivers/firewire/uapi-test.c @@ -45,9 +45,24 @@ static void structure_layout_event_request3(struct kunit *test) KUNIT_EXPECT_EQ(test, 56, offsetof(struct fw_cdev_event_request3, data)); } +// Added at v6.4. +static void structure_layout_event_response2(struct kunit *test) +{ + KUNIT_EXPECT_EQ(test, 32, sizeof(struct fw_cdev_event_response2)); + + KUNIT_EXPECT_EQ(test, 0, offsetof(struct fw_cdev_event_response2, closure)); + KUNIT_EXPECT_EQ(test, 8, offsetof(struct fw_cdev_event_response2, type)); + KUNIT_EXPECT_EQ(test, 12, offsetof(struct fw_cdev_event_response2, rcode)); + KUNIT_EXPECT_EQ(test, 16, offsetof(struct fw_cdev_event_response2, length)); + KUNIT_EXPECT_EQ(test, 20, offsetof(struct fw_cdev_event_response2, request_tstamp)); + KUNIT_EXPECT_EQ(test, 24, offsetof(struct fw_cdev_event_response2, response_tstamp)); + KUNIT_EXPECT_EQ(test, 32, offsetof(struct fw_cdev_event_response2, data)); +} + static struct kunit_case structure_layout_test_cases[] = { KUNIT_CASE(structure_layout_event_response), KUNIT_CASE(structure_layout_event_request3), + KUNIT_CASE(structure_layout_event_response2), {} }; diff --git a/include/uapi/linux/firewire-cdev.h b/include/uapi/linux/firewire-cdev.h index cc9b03244a62..ae8ccf7d7d2a 100644 --- a/include/uapi/linux/firewire-cdev.h +++ b/include/uapi/linux/firewire-cdev.h @@ -48,6 +48,7 @@ /* available since kernel version 6.3 */ #define FW_CDEV_EVENT_REQUEST3 0x0a +#define FW_CDEV_EVENT_RESPONSE2 0x0b /** * struct fw_cdev_event_common - Common part of all fw_cdev_event_* types @@ -106,6 +107,29 @@ struct fw_cdev_event_bus_reset { * @length: Data length, i.e. the response's payload size in bytes * @data: Payload data, if any * + * This event is sent instead of &fw_cdev_event_response if the kernel or the client implements + * ABI version <= 5. It has the lack of time stamp field comparing to &fw_cdev_event_response2. + */ +struct fw_cdev_event_response { + __u64 closure; + __u32 type; + __u32 rcode; + __u32 length; + __u32 data[]; +}; + +/** + * struct fw_cdev_event_response2 - Sent when a response packet was received + * @closure: See &fw_cdev_event_common; set by %FW_CDEV_IOC_SEND_REQUEST + * or %FW_CDEV_IOC_SEND_BROADCAST_REQUEST + * or %FW_CDEV_IOC_SEND_STREAM_PACKET ioctl + * @type: See &fw_cdev_event_common; always %FW_CDEV_EVENT_RESPONSE + * @rcode: Response code returned by the remote node + * @length: Data length, i.e. the response's payload size in bytes + * @request_tstamp: The time stamp of isochronous cycle at which the request was sent. + * @request_tstamp: The time stamp of isochronous cycle at which the response was sent. + * @data: Payload data, if any + * * This event is sent when the stack receives a response to an outgoing request * sent by %FW_CDEV_IOC_SEND_REQUEST ioctl. The payload data for responses * carrying data (read and lock responses) follows immediately and can be @@ -115,12 +139,25 @@ struct fw_cdev_event_bus_reset { * involve response packets. This includes unified write transactions, * broadcast write transactions, and transmission of asynchronous stream * packets. @rcode indicates success or failure of such transmissions. + * + * The value of @request_tstamp expresses the isochronous cycle at which the request was sent to + * initiate the transaction. The value of @response_tstamp expresses the isochronous cycle at which + * the response arrived to complete the transaction. Each value is unsigned 16 bit integer + * containing three low order bits of second field and all 13 bits of cycle field in format of + * CYCLE_TIMER register. */ -struct fw_cdev_event_response { +struct fw_cdev_event_response2 { __u64 closure; __u32 type; __u32 rcode; __u32 length; + __u32 request_tstamp; + __u32 response_tstamp; + /* + * Padding to keep the size of structure as multiples of 8 in various architectures since + * 4 byte alignment is used for 8 byte of object type in System V ABI for i386 architecture. + */ + __u32 padding; __u32 data[]; }; @@ -421,6 +458,7 @@ struct fw_cdev_event_phy_packet { * %FW_CDEV_EVENT_PHY_PACKET_RECEIVED * * @request3: Valid if @common.type == %FW_CDEV_EVENT_REQUEST3 + * @response2: Valid if @common.type == %FW_CDEV_EVENT_RESPONSE2 * * Convenience union for userspace use. Events could be read(2) into an * appropriately aligned char buffer and then cast to this union for further @@ -441,6 +479,7 @@ union fw_cdev_event { struct fw_cdev_event_iso_resource iso_resource; /* added in 2.6.30 */ struct fw_cdev_event_phy_packet phy_packet; /* added in 2.6.36 */ struct fw_cdev_event_request3 request3; /* added in 6.3 */ + struct fw_cdev_event_response2 response2; /* added in 6.3 */ }; /* available since kernel version 2.6.22 */ @@ -507,6 +546,7 @@ union fw_cdev_event { * - added %FW_CDEV_IOC_FLUSH_ISO * 6 (6.4) - added some event for subactions of asynchronous transaction with time stamp * - %FW_CDEV_EVENT_REQUEST3 + * - %FW_CDEV_EVENT_RESPONSE2 */ /** @@ -552,11 +592,11 @@ struct fw_cdev_get_info { * @data: Userspace pointer to payload * @generation: The bus generation where packet is valid * - * Send a request to the device. This ioctl implements all outgoing requests. - * Both quadlet and block request specify the payload as a pointer to the data - * in the @data field. Once the transaction completes, the kernel writes an - * &fw_cdev_event_response event back. The @closure field is passed back to - * user space in the response event. + * Send a request to the device. This ioctl implements all outgoing requests. Both quadlet and + * block request specify the payload as a pointer to the data in the @data field. Once the + * transaction completes, the kernel writes either &fw_cdev_event_response event or + * &fw_cdev_event_response event back. The @closure field is passed back to user space in the + * response event. */ struct fw_cdev_send_request { __u32 tcode; @@ -1039,10 +1079,9 @@ struct fw_cdev_allocate_iso_resource { * @generation: The bus generation where packet is valid * @speed: Speed to transmit at * - * The %FW_CDEV_IOC_SEND_STREAM_PACKET ioctl sends an asynchronous stream packet - * to every device which is listening to the specified channel. The kernel - * writes an &fw_cdev_event_response event which indicates success or failure of - * the transmission. + * The %FW_CDEV_IOC_SEND_STREAM_PACKET ioctl sends an asynchronous stream packet to every device + * which is listening to the specified channel. The kernel writes either &fw_cdev_event_response + * event or &fw_cdev_event_response2 event which indicates success or failure of the transmission. */ struct fw_cdev_send_stream_packet { __u32 length; -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-05-25 10:16:49
|
In 1394 OHCI, the OUTPUT_LAST descriptor of Asynchronous Transmit (AT) request context has timeStamp field, in which 1394 OHCI controller record the isochronous cycle when the packet was sent for the request subaction. Additionally, for the case of split transaction in IEEE 1394, Asynchronous Receive (AT) request context is used for response subaction to finish the transaction. The trailer quadlet of descriptor in the context has timeStamp field, in which 1394 OHCI controller records the isochronous cycle when the packet arrived. Current implementation of 1394 OHCI controller driver stores values of both fields to internal structure as time stamp, while Linux FireWire subsystem provides no way to access to it. When using asynchronous transaction service provided by the subsystem, callback function is passed to kernel API. The prototype of callback function has the lack of argument for the values. This commit adds a new callback function for the purpose. It has an additional argument to point to the constant array with two elements. For backward compatibility to kernel space, a new union is also adds to wrap two different prototype of callback function. The fw_transaction structure has the union as a member and a boolean flag to express which function callback is available. The core function is changed to handle the two cases; with or without time stamp. For the error path to process transaction, the isochronous cycle is computed by current value of CYCLE_TIMER register in 1394 OHCI controller. Especially for the case of timeout of split transaction, the expected isochronous cycle is computed. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-transaction.c | 58 +++++++++++++++++++++++------ drivers/firewire/core.h | 7 ++++ drivers/firewire/ohci.c | 17 ++++++++- include/linux/firewire.h | 13 ++++++- 4 files changed, 80 insertions(+), 15 deletions(-) diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c index a9f70c96323e..a20f97fdd06c 100644 --- a/drivers/firewire/core-transaction.c +++ b/drivers/firewire/core-transaction.c @@ -70,8 +70,8 @@ static int try_cancel_split_timeout(struct fw_transaction *t) return 1; } -static int close_transaction(struct fw_transaction *transaction, - struct fw_card *card, int rcode) +static int close_transaction(struct fw_transaction *transaction, struct fw_card *card, int rcode, + u32 response_tstamp) { struct fw_transaction *t = NULL, *iter; unsigned long flags; @@ -92,7 +92,12 @@ static int close_transaction(struct fw_transaction *transaction, spin_unlock_irqrestore(&card->lock, flags); if (t) { - t->callback(card, rcode, NULL, 0, t->callback_data); + if (!t->with_tstamp) { + t->callback.without_tstamp(card, rcode, NULL, 0, t->callback_data); + } else { + t->callback.with_tstamp(card, rcode, t->packet.timestamp, response_tstamp, + NULL, 0, t->callback_data); + } return 0; } @@ -107,6 +112,8 @@ static int close_transaction(struct fw_transaction *transaction, int fw_cancel_transaction(struct fw_card *card, struct fw_transaction *transaction) { + u32 tstamp; + /* * Cancel the packet transmission if it's still queued. That * will call the packet transmission callback which cancels @@ -121,7 +128,17 @@ int fw_cancel_transaction(struct fw_card *card, * if the transaction is still pending and remove it in that case. */ - return close_transaction(transaction, card, RCODE_CANCELLED); + if (transaction->packet.ack == 0) { + // The timestamp is reused since it was just read now. + tstamp = transaction->packet.timestamp; + } else { + u32 curr_cycle_time = 0; + + (void)fw_card_read_cycle_time(card, &curr_cycle_time); + tstamp = cycle_time_to_ohci_tstamp(curr_cycle_time); + } + + return close_transaction(transaction, card, RCODE_CANCELLED, tstamp); } EXPORT_SYMBOL(fw_cancel_transaction); @@ -140,7 +157,12 @@ static void split_transaction_timeout_callback(struct timer_list *timer) card->tlabel_mask &= ~(1ULL << t->tlabel); spin_unlock_irqrestore(&card->lock, flags); - t->callback(card, RCODE_CANCELLED, NULL, 0, t->callback_data); + if (!t->with_tstamp) { + t->callback.without_tstamp(card, RCODE_CANCELLED, NULL, 0, t->callback_data); + } else { + t->callback.with_tstamp(card, RCODE_CANCELLED, t->packet.timestamp, + t->split_timeout_cycle, NULL, 0, t->callback_data); + } } static void start_split_transaction_timeout(struct fw_transaction *t, @@ -162,6 +184,8 @@ static void start_split_transaction_timeout(struct fw_transaction *t, spin_unlock_irqrestore(&card->lock, flags); } +static u32 compute_split_timeout_timestamp(struct fw_card *card, u32 request_timestamp); + static void transmit_complete_callback(struct fw_packet *packet, struct fw_card *card, int status) { @@ -170,28 +194,32 @@ static void transmit_complete_callback(struct fw_packet *packet, switch (status) { case ACK_COMPLETE: - close_transaction(t, card, RCODE_COMPLETE); + close_transaction(t, card, RCODE_COMPLETE, packet->timestamp); break; case ACK_PENDING: + { + t->split_timeout_cycle = + compute_split_timeout_timestamp(card, packet->timestamp) & 0xffff; start_split_transaction_timeout(t, card); break; + } case ACK_BUSY_X: case ACK_BUSY_A: case ACK_BUSY_B: - close_transaction(t, card, RCODE_BUSY); + close_transaction(t, card, RCODE_BUSY, packet->timestamp); break; case ACK_DATA_ERROR: - close_transaction(t, card, RCODE_DATA_ERROR); + close_transaction(t, card, RCODE_DATA_ERROR, packet->timestamp); break; case ACK_TYPE_ERROR: - close_transaction(t, card, RCODE_TYPE_ERROR); + close_transaction(t, card, RCODE_TYPE_ERROR, packet->timestamp); break; default: /* * In this case the ack is really a juju specific * rcode, so just forward that to the callback. */ - close_transaction(t, card, status); + close_transaction(t, card, status, packet->timestamp); break; } } @@ -363,7 +391,8 @@ void fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode, t->is_split_transaction = false; timer_setup(&t->split_timeout_timer, split_transaction_timeout_callback, 0); - t->callback = callback; + t->callback.without_tstamp = callback; + t->with_tstamp = false; t->callback_data = callback_data; fw_fill_request(&t->packet, tcode, t->tlabel, @@ -1047,7 +1076,12 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p) */ card->driver->cancel_packet(card, &t->packet); - t->callback(card, rcode, data, data_length, t->callback_data); + if (!t->with_tstamp) { + t->callback.without_tstamp(card, rcode, data, data_length, t->callback_data); + } else { + t->callback.with_tstamp(card, rcode, t->packet.timestamp, p->timestamp, data, + data_length, t->callback_data); + } } EXPORT_SYMBOL(fw_core_handle_response); diff --git a/drivers/firewire/core.h b/drivers/firewire/core.h index eafa4eaae737..2a05f411328f 100644 --- a/drivers/firewire/core.h +++ b/drivers/firewire/core.h @@ -247,6 +247,13 @@ void fw_fill_response(struct fw_packet *response, u32 *request_header, void fw_request_get(struct fw_request *request); void fw_request_put(struct fw_request *request); +// Convert the value of IEEE 1394 CYCLE_TIME register to the format of timeStamp field in +// descriptors of 1394 OHCI. +static inline u32 cycle_time_to_ohci_tstamp(u32 tstamp) +{ + return (tstamp & 0x0ffff000) >> 12; +} + #define FW_PHY_CONFIG_NO_NODE_ID -1 #define FW_PHY_CONFIG_CURRENT_GAP_COUNT -1 void fw_send_phy_config(struct fw_card *card, diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 17c9d825188b..06386c3b7f03 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -1623,6 +1623,8 @@ static void handle_local_request(struct context *ctx, struct fw_packet *packet) } } +static u32 get_cycle_time(struct fw_ohci *ohci); + static void at_context_transmit(struct context *ctx, struct fw_packet *packet) { unsigned long flags; @@ -1633,6 +1635,10 @@ static void at_context_transmit(struct context *ctx, struct fw_packet *packet) if (HEADER_GET_DESTINATION(packet->header[0]) == ctx->ohci->node_id && ctx->ohci->generation == packet->generation) { spin_unlock_irqrestore(&ctx->ohci->lock, flags); + + // Timestamping on behalf of the hardware. + packet->timestamp = cycle_time_to_ohci_tstamp(get_cycle_time(ctx->ohci)); + handle_local_request(ctx, packet); return; } @@ -1640,9 +1646,12 @@ static void at_context_transmit(struct context *ctx, struct fw_packet *packet) ret = at_context_queue_packet(ctx, packet); spin_unlock_irqrestore(&ctx->ohci->lock, flags); - if (ret < 0) - packet->callback(packet, &ctx->ohci->card, packet->ack); + if (ret < 0) { + // Timestamping on behalf of the hardware. + packet->timestamp = cycle_time_to_ohci_tstamp(get_cycle_time(ctx->ohci)); + packet->callback(packet, &ctx->ohci->card, packet->ack); + } } static void detect_dead_context(struct fw_ohci *ohci, @@ -2557,6 +2566,10 @@ static int ohci_cancel_packet(struct fw_card *card, struct fw_packet *packet) log_ar_at_event(ohci, 'T', packet->speed, packet->header, 0x20); driver_data->packet = NULL; packet->ack = RCODE_CANCELLED; + + // Timestamping on behalf of the hardware. + packet->timestamp = cycle_time_to_ohci_tstamp(get_cycle_time(ohci)); + packet->callback(packet, &ohci->card, packet->ack); ret = 0; out: diff --git a/include/linux/firewire.h b/include/linux/firewire.h index 1716c01c4e54..d61693341da1 100644 --- a/include/linux/firewire.h +++ b/include/linux/firewire.h @@ -261,6 +261,15 @@ typedef void (*fw_packet_callback_t)(struct fw_packet *packet, typedef void (*fw_transaction_callback_t)(struct fw_card *card, int rcode, void *data, size_t length, void *callback_data); +typedef void (*fw_transaction_callback_with_tstamp_t)(struct fw_card *card, int rcode, + u32 request_tstamp, u32 response_tstamp, void *data, + size_t length, void *callback_data); + +union fw_transaction_callback { + fw_transaction_callback_t without_tstamp; + fw_transaction_callback_with_tstamp_t with_tstamp; +}; + /* * This callback handles an inbound request subaction. It is called in * RCU read-side context, therefore must not sleep. @@ -312,6 +321,7 @@ struct fw_transaction { struct fw_card *card; bool is_split_transaction; struct timer_list split_timeout_timer; + u32 split_timeout_cycle; struct fw_packet packet; @@ -319,7 +329,8 @@ struct fw_transaction { * The data passed to the callback is valid only during the * callback. */ - fw_transaction_callback_t callback; + union fw_transaction_callback callback; + bool with_tstamp; void *callback_data; }; -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-05-25 10:16:44
|
This commit adds new event to notify event of request subaction with time stamp field. Current compiler implementation of System V ABI selects one of structure members which has the maximum alignment size in the structure to decide the size of structure. In the case of fw_cdev_event_request3 structure, it is closure member which has 8 byte storage. The size of alignment for the type of 8 byte storage differs depending on architectures; 4 byte for i386 architecture and 8 byte for the others including x32 architecture. It is inconvenient to device driver developer to use structure layout which varies between architectures since the developer takes care of ioctl compat layer. This commit adds 32 bit member for padding to keep the size of structure as multiples of 8. Cc: kun...@go... Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/uapi-test.c | 20 +++++++++++ include/uapi/linux/firewire-cdev.h | 53 ++++++++++++++++++++++++++++-- 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/drivers/firewire/uapi-test.c b/drivers/firewire/uapi-test.c index 4dc633b91336..c7c713babaa0 100644 --- a/drivers/firewire/uapi-test.c +++ b/drivers/firewire/uapi-test.c @@ -26,8 +26,28 @@ static void structure_layout_event_response(struct kunit *test) KUNIT_EXPECT_EQ(test, 20, offsetof(struct fw_cdev_event_response, data)); } +// Added at v6.4. +static void structure_layout_event_request3(struct kunit *test) +{ + KUNIT_EXPECT_EQ(test, 56, sizeof(struct fw_cdev_event_request3)); + + KUNIT_EXPECT_EQ(test, 0, offsetof(struct fw_cdev_event_request3, closure)); + KUNIT_EXPECT_EQ(test, 8, offsetof(struct fw_cdev_event_request3, type)); + KUNIT_EXPECT_EQ(test, 12, offsetof(struct fw_cdev_event_request3, tcode)); + KUNIT_EXPECT_EQ(test, 16, offsetof(struct fw_cdev_event_request3, offset)); + KUNIT_EXPECT_EQ(test, 24, offsetof(struct fw_cdev_event_request3, source_node_id)); + KUNIT_EXPECT_EQ(test, 28, offsetof(struct fw_cdev_event_request3, destination_node_id)); + KUNIT_EXPECT_EQ(test, 32, offsetof(struct fw_cdev_event_request3, card)); + KUNIT_EXPECT_EQ(test, 36, offsetof(struct fw_cdev_event_request3, generation)); + KUNIT_EXPECT_EQ(test, 40, offsetof(struct fw_cdev_event_request3, handle)); + KUNIT_EXPECT_EQ(test, 44, offsetof(struct fw_cdev_event_request3, length)); + KUNIT_EXPECT_EQ(test, 48, offsetof(struct fw_cdev_event_request3, tstamp)); + KUNIT_EXPECT_EQ(test, 56, offsetof(struct fw_cdev_event_request3, data)); +} + static struct kunit_case structure_layout_test_cases[] = { KUNIT_CASE(structure_layout_event_response), + KUNIT_CASE(structure_layout_event_request3), {} }; diff --git a/include/uapi/linux/firewire-cdev.h b/include/uapi/linux/firewire-cdev.h index e3d463b2c288..cc9b03244a62 100644 --- a/include/uapi/linux/firewire-cdev.h +++ b/include/uapi/linux/firewire-cdev.h @@ -46,6 +46,9 @@ #define FW_CDEV_EVENT_PHY_PACKET_RECEIVED 0x08 #define FW_CDEV_EVENT_ISO_INTERRUPT_MULTICHANNEL 0x09 +/* available since kernel version 6.3 */ +#define FW_CDEV_EVENT_REQUEST3 0x0a + /** * struct fw_cdev_event_common - Common part of all fw_cdev_event_* types * @closure: For arbitrary use by userspace @@ -159,6 +162,38 @@ struct fw_cdev_event_request { * @length: Data length, i.e. the request's payload size in bytes * @data: Incoming data, if any * + * This event is sent instead of &fw_cdev_event_request3 if the kernel or the client implements + * ABI version <= 5. It has the lack of time stamp field comparing to &fw_cdev_event_request3. + */ +struct fw_cdev_event_request2 { + __u64 closure; + __u32 type; + __u32 tcode; + __u64 offset; + __u32 source_node_id; + __u32 destination_node_id; + __u32 card; + __u32 generation; + __u32 handle; + __u32 length; + __u32 data[]; +}; + +/** + * struct fw_cdev_event_request3 - Sent on incoming request to an address region + * @closure: See &fw_cdev_event_common; set by %FW_CDEV_IOC_ALLOCATE ioctl + * @type: See &fw_cdev_event_common; always %FW_CDEV_EVENT_REQUEST2 + * @tcode: Transaction code of the incoming request + * @offset: The offset into the 48-bit per-node address space + * @source_node_id: Sender node ID + * @destination_node_id: Destination node ID + * @card: The index of the card from which the request came + * @generation: Bus generation in which the request is valid + * @handle: Reference to the kernel-side pending request + * @length: Data length, i.e. the request's payload size in bytes + * @tstamp: The time stamp of isochronous cycle at which the request arrived. + * @data: Incoming data, if any + * * This event is sent when the stack receives an incoming request to an address * region registered using the %FW_CDEV_IOC_ALLOCATE ioctl. The request is * guaranteed to be completely contained in the specified region. Userspace is @@ -191,10 +226,14 @@ struct fw_cdev_event_request { * sent. * * If the client subsequently needs to initiate requests to the sender node of - * an &fw_cdev_event_request2, it needs to use a device file with matching + * an &fw_cdev_event_request3, it needs to use a device file with matching * card index, node ID, and generation for outbound requests. + * + * @tstamp is isochronous cycle at which the request arrived. It is 16 bit integer value and the + * higher 3 bits expresses three low order bits of second field in the format of CYCLE_TIME + * register and the rest 13 bits expresses cycle field. */ -struct fw_cdev_event_request2 { +struct fw_cdev_event_request3 { __u64 closure; __u32 type; __u32 tcode; @@ -205,6 +244,12 @@ struct fw_cdev_event_request2 { __u32 generation; __u32 handle; __u32 length; + __u32 tstamp; + /* + * Padding to keep the size of structure as multiples of 8 in various architectures since + * 4 byte alignment is used for 8 byte of object type in System V ABI for i386 architecture. + */ + __u32 padding; __u32 data[]; }; @@ -375,6 +420,8 @@ struct fw_cdev_event_phy_packet { * %FW_CDEV_EVENT_PHY_PACKET_SENT or * %FW_CDEV_EVENT_PHY_PACKET_RECEIVED * + * @request3: Valid if @common.type == %FW_CDEV_EVENT_REQUEST3 + * * Convenience union for userspace use. Events could be read(2) into an * appropriately aligned char buffer and then cast to this union for further * processing. Note that for a request, response or iso_interrupt event, @@ -393,6 +440,7 @@ union fw_cdev_event { struct fw_cdev_event_iso_interrupt_mc iso_interrupt_mc; /* added in 2.6.36 */ struct fw_cdev_event_iso_resource iso_resource; /* added in 2.6.30 */ struct fw_cdev_event_phy_packet phy_packet; /* added in 2.6.36 */ + struct fw_cdev_event_request3 request3; /* added in 6.3 */ }; /* available since kernel version 2.6.22 */ @@ -458,6 +506,7 @@ union fw_cdev_event { * avoid dropping data * - added %FW_CDEV_IOC_FLUSH_ISO * 6 (6.4) - added some event for subactions of asynchronous transaction with time stamp + * - %FW_CDEV_EVENT_REQUEST3 */ /** -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-05-25 10:16:44
|
In 1394 OHCI, the trailer quadlet of descriptor in Asynchronous Receive (AR) request context has timeStamp field, in which the 1394 OHCI controller record the isochronous cycle when the packet arrived. Current implementation of 1394 OHCI controller driver stores the value of field to internal structure as time stamp, while the implementation of FireWire character device doesn't have a field for the time stamp, thus it is not available in user space. The time stamp is convenient to some kind of application in which data from several sources are compared in isochronous cycle unit. This commit implement the new event, fw_cdev_event_request3, with an additional field, tstamp. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-cdev.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index 88c8b5fac5e5..5a9446d30447 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -178,6 +178,7 @@ struct inbound_transaction_event { union { struct fw_cdev_event_request request; struct fw_cdev_event_request2 request2; + struct fw_cdev_event_request3 with_tstamp; } req; }; @@ -709,7 +710,7 @@ static void handle_request(struct fw_card *card, struct fw_request *request, req->handle = r->resource.handle; req->closure = handler->closure; event_size0 = sizeof(*req); - } else { + } else if (handler->client->version < FW_CDEV_VERSION_EVENT_ASYNC_TSTAMP) { struct fw_cdev_event_request2 *req = &e->req.request2; req->type = FW_CDEV_EVENT_REQUEST2; @@ -723,6 +724,21 @@ static void handle_request(struct fw_card *card, struct fw_request *request, req->handle = r->resource.handle; req->closure = handler->closure; event_size0 = sizeof(*req); + } else { + struct fw_cdev_event_request3 *req = &e->req.with_tstamp; + + req->type = FW_CDEV_EVENT_REQUEST3; + req->tcode = tcode; + req->offset = offset; + req->source_node_id = source; + req->destination_node_id = destination; + req->card = card->index; + req->generation = generation; + req->length = length; + req->handle = r->resource.handle; + req->closure = handler->closure; + req->tstamp = fw_request_get_timestamp(request); + event_size0 = sizeof(*req); } queue_event(handler->client, &e->event, -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-05-25 10:16:43
|
Hi, This patchset is revised version of the previous one[1]. Copied from the cover letter: 1394 OHCI hardware supports hardware time stamp for asynchronous communication at 8,000 Hz resolution (= isochronous cycle), while current implementation of FireWire subsystem does not deliver the time stamp to both unit driver and user space application when operating the asynchronous communication. It is inconvenient to a kind of application which attempts to synchronize data from multiple sources by the (coarse) time stamp. This patchset changes the subsystem so that the unit driver and the user space application to receive the time stamp, therefore it affects kernel service for asynchronous transaction, kernel API for unit driver, and UAPI for user space application. The first patch is newly added for KUnit test to check layout of structure exposed to user space. I'm pleased if getting any review by KUnit developers since it is my first time to write any KUnit test. The new feature will be used for my future work to replace tasklet with workqueue. The hardware time stamp could enable us to compute the processing delay so precise. [1] https://lore.kernel.org/lkml/202...@sa.../ Takashi Sakamoto (12): firewire: add KUnit test to check layout of UAPI structures firewire: cdev: add new version of ABI to notify time stamp at request/response subaction of transaction firewire: cdev: add new event to notify request subaction with time stamp firewire: cdev: implement new event to notify request subaction with time stamp firewire: core: use union for callback of transaction completion firewire: core: implement variations to send request and wait for response with time stamp firewire: cdev: code refactoring to operate event of response firewire: cdev: add new event to notify response subaction with time stamp firewire: cdev: implement new event to notify response subaction with time stamp firewire: cdev: code refactoring to dispatch event for phy packet firewire: cdev: add new event to notify phy packet with time stamp firewire: cdev: implement new event relevant to phy packet with time stamp drivers/firewire/.kunitconfig | 4 + drivers/firewire/Kconfig | 16 ++ drivers/firewire/Makefile | 3 + drivers/firewire/core-cdev.c | 252 +++++++++++++++++++++------- drivers/firewire/core-transaction.c | 93 +++++++--- drivers/firewire/core.h | 7 + drivers/firewire/ohci.c | 17 +- drivers/firewire/uapi-test.c | 87 ++++++++++ include/linux/firewire.h | 82 ++++++++- include/uapi/linux/firewire-cdev.h | 180 +++++++++++++++++--- 10 files changed, 625 insertions(+), 116 deletions(-) create mode 100644 drivers/firewire/.kunitconfig create mode 100644 drivers/firewire/uapi-test.c -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-05-25 10:16:42
|
This commit adds new version of ABI for future new events with time stamp for request/response subaction of asynchronous transaction to user space. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-cdev.c | 1 + include/uapi/linux/firewire-cdev.h | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index 2c16ee8fd842..88c8b5fac5e5 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -43,6 +43,7 @@ #define FW_CDEV_VERSION_EVENT_REQUEST2 4 #define FW_CDEV_VERSION_ALLOCATE_REGION_END 4 #define FW_CDEV_VERSION_AUTO_FLUSH_ISO_OVERFLOW 5 +#define FW_CDEV_VERSION_EVENT_ASYNC_TSTAMP 6 struct client { u32 version; diff --git a/include/uapi/linux/firewire-cdev.h b/include/uapi/linux/firewire-cdev.h index 92be3ea3c6e0..e3d463b2c288 100644 --- a/include/uapi/linux/firewire-cdev.h +++ b/include/uapi/linux/firewire-cdev.h @@ -457,6 +457,7 @@ union fw_cdev_event { * 5 (3.4) - send %FW_CDEV_EVENT_ISO_INTERRUPT events when needed to * avoid dropping data * - added %FW_CDEV_IOC_FLUSH_ISO + * 6 (6.4) - added some event for subactions of asynchronous transaction with time stamp */ /** -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-05-25 10:16:42
|
In future commits, some new structure will be added to express new type of event. They are exposed to user space as the part of UAPI. It is likely to get trouble in ioctl compatibility layer for 32 bit binaries in 64 bit host machine since the layout of structure could differ depending on System V ABI for these architectures. Actually the subsystem already got such trouble at v2.6.27. It is preferable to decide the layout of structure carefully so that the layer is free from such trouble. This commit utilizes KUnit framework to check the layout of structure for the purpose. A test is added for the existent issue. Cc: kun...@go... Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/.kunitconfig | 4 ++++ drivers/firewire/Kconfig | 16 +++++++++++++++ drivers/firewire/Makefile | 3 +++ drivers/firewire/uapi-test.c | 38 +++++++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+) create mode 100644 drivers/firewire/.kunitconfig create mode 100644 drivers/firewire/uapi-test.c diff --git a/drivers/firewire/.kunitconfig b/drivers/firewire/.kunitconfig new file mode 100644 index 000000000000..1599e069395f --- /dev/null +++ b/drivers/firewire/.kunitconfig @@ -0,0 +1,4 @@ +CONFIG_KUNIT=y +CONFIG_PCI=y +CONFIG_FIREWIRE=y +CONFIG_FIREWIRE_KUNIT_UAPI_TEST=y diff --git a/drivers/firewire/Kconfig b/drivers/firewire/Kconfig index ec00a6f70da8..0a6596b027db 100644 --- a/drivers/firewire/Kconfig +++ b/drivers/firewire/Kconfig @@ -18,6 +18,22 @@ config FIREWIRE To compile this driver as a module, say M here: the module will be called firewire-core. +config FIREWIRE_KUNIT_UAPI_TEST + tristate "KUnit tests for layout of structure in UAPI" if !KUNIT_ALL_TESTS + depends on FIREWIRE && KUNIT + default KUNIT_ALL_TESTS + help + This builds the KUnit tests whether structures exposed to user + space have expected layout. + + 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 e58c8c794778..b24b2879ac34 100644 --- a/drivers/firewire/Makefile +++ b/drivers/firewire/Makefile @@ -15,3 +15,6 @@ obj-$(CONFIG_FIREWIRE_SBP2) += firewire-sbp2.o obj-$(CONFIG_FIREWIRE_NET) += firewire-net.o obj-$(CONFIG_FIREWIRE_NOSY) += nosy.o obj-$(CONFIG_PROVIDE_OHCI1394_DMA_INIT) += init_ohci1394_dma.o + +firewire-uapi-test-objs += uapi-test.o +obj-$(CONFIG_FIREWIRE_KUNIT_UAPI_TEST) += firewire-uapi-test.o diff --git a/drivers/firewire/uapi-test.c b/drivers/firewire/uapi-test.c new file mode 100644 index 000000000000..4dc633b91336 --- /dev/null +++ b/drivers/firewire/uapi-test.c @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: GPL-2.0-only +// +// uapi_test.c - An application of Kunit to check layout of structures exposed to user space for +// FireWire subsystem. +// +// Copyright (c) 2023 Takashi Sakamoto + +#include <kunit/test.h> +#include <linux/firewire-cdev.h> + +// Known issue added at v2.6.27 kernel. +static void structure_layout_event_response(struct kunit *test) +{ +#if defined(CONFIG_X86_32) + // 4 bytes alignment for aggregate type including 8 bytes storage types. + KUNIT_EXPECT_EQ(test, 20, sizeof(struct fw_cdev_event_response)); +#else + // 8 bytes alignment for aggregate type including 8 bytes storage types. + KUNIT_EXPECT_EQ(test, 24, sizeof(struct fw_cdev_event_response)); +#endif + + KUNIT_EXPECT_EQ(test, 0, offsetof(struct fw_cdev_event_response, closure)); + KUNIT_EXPECT_EQ(test, 8, offsetof(struct fw_cdev_event_response, type)); + KUNIT_EXPECT_EQ(test, 12, offsetof(struct fw_cdev_event_response, rcode)); + KUNIT_EXPECT_EQ(test, 16, offsetof(struct fw_cdev_event_response, length)); + KUNIT_EXPECT_EQ(test, 20, offsetof(struct fw_cdev_event_response, data)); +} + +static struct kunit_case structure_layout_test_cases[] = { + KUNIT_CASE(structure_layout_event_response), + {} +}; + +static struct kunit_suite structure_layout_test_suite = { + .name = "firewire-uapi-structure-layout", + .test_cases = structure_layout_test_cases, +}; +kunit_test_suite(structure_layout_test_suite); -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-05-21 05:07:19
|
Hi, Today I released version 0.5.0 of linux-firewire-utils. You can download the new release from the following URL: https://git.kernel.org/pub/scm/utils/ieee1394/linux-firewire-utils.git/tag/?h=v0.5.0 This marks the first release since the project was hosted at git.kernel.org[1]. In this release, crpp tool has been removed due to Python 2 and license issues. Instead, a new tool called config-rom-pretty-printer has been added as an alternate. This tool is implemented in C language and closely resembles the output of the original tool, capturing approximately 80% of its functionality. However some legacy functions, such as the firecontrol parser, have been dropped. The original tool is maintained by Clemens Ladisch and hosted on github.com[2]. However there has been no activity on the repository in recent years[3]. As a result, the project was forked to git.kernel.org and is now maintained by Takashi Sakamoto. I would like to take this opportunity to express my gratitude for Clemens Ladisch's work on the original tool. I am delighted to be able to release an updated version after 8 years of silence. [1] https://git.kernel.org/pub/scm/utils/ieee1394/linux-firewire-utils.git/ [2] https://github.com/cladisch/linux-firewire-utils/ [3] https://github.com/cladisch/linux-firewire-utils/pull/1 Regards Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2023-05-12 09:36:24
|
Hi, Still no response, while I forked it to git.kernel.org for further integration: * https://git.kernel.org/pub/scm/utils/ieee1394/linux-firewire-utils.git/ The crpp script is written by Python 2 language. Python 2 is outdated. I rewrite it by C language and push to the repository. On Sat, Apr 22, 2023 at 10:00:27AM +0900, Takashi Sakamoto wrote: > Hi Clemens, > > It takes a bit long time since I see you in open list last time. I wish > you are still of good cheer. > > I decide to take over maintenance of Linux FireWire subsystem[1] and I'm > preparing stuffs for the work with help of kernel.org administrators. If > thing goes well, I'll start my task in next merge window for Linux kernel > v6.4. > > Well, if you don't mind, let us move the upstream of your > linux-firewire-utils? I already prepared repository directories for > utilities of Linux 1394[2]. Your software would locates under the directory. > > In the case, I can maintain your software on behalf of you if you don't > mind. Especially, I need new version of the software including crpp > written by Python 3 for my work. > > I'm glad if receiving any of your reply. > > > [1] https://lore.kernel.org/lkml/202...@sa.../ > [2] https://git.kernel.org/pub/scm/utils/ieee1394/ Thanks Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2023-05-11 00:08:47
|
On Wed, May 10, 2023 at 12:12:05PM +0900, Takashi Sakamoto wrote: > The lifetime of object for asynchronous request packet is now maintained > by reference counting, while current implementation of firewire-net > releases the passed object in the handler. > > This commit fixes the bug. > > Reported-by: Dan Carpenter <er...@gm...> > Link: https://lore.kernel.org/lkml/Y%2Fymx6WZIAlrtjLc@workstation/ > Fixes: 13a55d6bb15f ("firewire: core: use kref structure to maintain lifetime of data for fw_request structure") > Signed-off-by: Takashi Sakamoto <o-t...@sa...> > --- > drivers/firewire/net.c | 21 +++++++++++---------- > 1 file changed, 11 insertions(+), 10 deletions(-) The patch is applied to for-linus branch and will be sent to mainline within a few days. Regards Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2023-05-10 03:12:19
|
The lifetime of object for asynchronous request packet is now maintained by reference counting, while current implementation of firewire-net releases the passed object in the handler. This commit fixes the bug. Reported-by: Dan Carpenter <er...@gm...> Link: https://lore.kernel.org/lkml/Y%2Fymx6WZIAlrtjLc@workstation/ Fixes: 13a55d6bb15f ("firewire: core: use kref structure to maintain lifetime of data for fw_request structure") Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/net.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c index af22be84034b..538bd677c254 100644 --- a/drivers/firewire/net.c +++ b/drivers/firewire/net.c @@ -706,21 +706,22 @@ static void fwnet_receive_packet(struct fw_card *card, struct fw_request *r, int rcode; if (destination == IEEE1394_ALL_NODES) { - kfree(r); - - return; - } - - if (offset != dev->handler.offset) + // Although the response to the broadcast packet is not necessarily required, the + // fw_send_response() function should still be called to maintain the reference + // counting of the object. In the case, the call of function just releases the + // object as a result to decrease the reference counting. + rcode = RCODE_COMPLETE; + } else if (offset != dev->handler.offset) { rcode = RCODE_ADDRESS_ERROR; - else if (tcode != TCODE_WRITE_BLOCK_REQUEST) + } else if (tcode != TCODE_WRITE_BLOCK_REQUEST) { rcode = RCODE_TYPE_ERROR; - else if (fwnet_incoming_packet(dev, payload, length, - source, generation, false) != 0) { + } else if (fwnet_incoming_packet(dev, payload, length, + source, generation, false) != 0) { dev_err(&dev->netdev->dev, "incoming packet failure\n"); rcode = RCODE_CONFLICT_ERROR; - } else + } else { rcode = RCODE_COMPLETE; + } fw_send_response(card, r, rcode); } -- 2.37.2 |
From: Takashi S. <o-t...@sa...> - 2023-04-22 01:00:58
|
Hi Clemens, It takes a bit long time since I see you in open list last time. I wish you are still of good cheer. I decide to take over maintenance of Linux FireWire subsystem[1] and I'm preparing stuffs for the work with help of kernel.org administrators. If thing goes well, I'll start my task in next merge window for Linux kernel v6.4. Well, if you don't mind, let us move the upstream of your linux-firewire-utils? I already prepared repository directories for utilities of Linux 1394[2]. Your software would locates under the directory. In the case, I can maintain your software on behalf of you if you don't mind. Especially, I need new version of the software including crpp written by Python 3 for my work. I'm glad if receiving any of your reply. [1] https://lore.kernel.org/lkml/202...@sa.../ [2] https://git.kernel.org/pub/scm/utils/ieee1394/ Regards Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2023-04-20 16:04:45
|
On Sun, Apr 09, 2023 at 06:13:06PM -0700, Randy Dunlap wrote: > Prevent kernel-doc complaints by using the correct function names in > kernel-doc comments: > > drivers/firewire/init_ohci1394_dma.c:258: warning: expecting prototype for debug_init_ohci1394_dma(). Prototype was for init_ohci1394_dma_on_all_controllers() instead > drivers/firewire/init_ohci1394_dma.c:289: warning: expecting prototype for setup_init_ohci1394_early(). Prototype was for setup_ohci1394_dma() instead > > Signed-off-by: Randy Dunlap <rd...@in...> > Cc: Stefan Richter <st...@s5...> > Cc: lin...@li... > Cc: Andrew Morton <ak...@li...> > Cc: Takashi Sakamoto <o-t...@sa...> > --- > v2: rebase/resend, add note to Andrew > v3: add Takashi-san > > drivers/firewire/init_ohci1394_dma.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) The patch is applied to for-next branch for v6.4 kernel: * https://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git/ You can see the patch in linux-next integration as well. Thanks for your patience Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2023-04-20 16:02:51
|
On Mon, Mar 06, 2023 at 12:58:14PM +0900, Takashi Sakamoto wrote: > In the last few years, I have reviewed patches for FireWire subsystem and > requested sound subsystem maintainer to sent them to mainline, since > FireWire subsystem maintainer has been long absent. This situation is not > preferable since we have some user of sound hardware in IEEE 1394 bus. > > I will stand for the maintainer, and work for FireWire core functions and > 1394 OHCI driver, as well as sound drivers. This commit replaces the > corresponding entry. > > As you know, IEEE 1394 is enough legacy. I would like to schedule the end > of my work in the subsystem. My effort will last next 6 years. In 2026, I > will start strong announcement for users to migrate their work load from > IEEE 1394 bus (e.g. by purchasing alternative devices in USB and hardening > system for them), then in 2029 let me resign the maintainer and close > Linux 1394 project. > > My current work focuses on real time data (sampling data) transmission > protocol in packet-oriented communication, thus I would provide less help > to implementations for the other type of protocol; i.e. IPv4/IPv6 over > IEEE 1394 bus (firewire-net), SCSI transport protocol over IEEE 1394 bus > (firewire-sbp2) and iSCSI target (sbp-target). > > If receiving few objections from developers, I will start my work to send > fixes for v6.3 prepatch, and PR for future v6.4 or later. I'm pleased if > getting any help until the end. > > Reference: commit b32744751e75 ("firewire: add to MAINTAINERS") > Signed-off-by: Takashi Sakamoto <o-t...@sa...> > --- > MAINTAINERS | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) The patch is applied to for-next branch for v6.4 kernel: * https://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git/ Thanks Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2023-04-17 14:36:08
|
Hi Randy, I'm sorry that I didn't notice your reply, but I'm off from the receivers list of the message. > Hi-- > >On 3/11/23 23:07, Takashi Sakamoto wrote: >> Hi, >> >>> >>> It's good to see you being active in the kernel and related userland >>> development/ maintainership, and that you have a plan for the next years. >> >> At the moment, I have a problem about the list archive. > > If there is still a problem about the mailing list archives, > Hank Leininger at marc.info has been pretty good in the past about providing > archives fore lore.kernel.org. > See the bottom of https://marc.info/?q=about for his email address. Thanks for the information. As a quick glimpse, linux1394-devel has been archived since 2003. It looks to be a good source to push into lore.kernel.org, while I know that the list archive should include many spams since any one is allowed to post to the list without subscribing and being moderated at one time. It is one of my reason not to use the list so actively. I need to make filter to remove them before pushing. Furthermore, the ownership of list is still unclear to me. I have a small hesitation to use it further... > Are you waiting for the kernel.org account before merging the update to the > MAINTAINER's file? I've already got the account and enough permission to linux1394 repository, but The repository is inactive so long, and abandoned. Looking ahead to my future work, I'm preparing them at present. Thanks Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2023-04-11 00:06:01
|
On Sun, Apr 09, 2023 at 06:13:06PM -0700, Randy Dunlap wrote: > Prevent kernel-doc complaints by using the correct function names in > kernel-doc comments: > > drivers/firewire/init_ohci1394_dma.c:258: warning: expecting prototype for debug_init_ohci1394_dma(). Prototype was for init_ohci1394_dma_on_all_controllers() instead > drivers/firewire/init_ohci1394_dma.c:289: warning: expecting prototype for setup_init_ohci1394_early(). Prototype was for setup_ohci1394_dma() instead > > Signed-off-by: Randy Dunlap <rd...@in...> > Cc: Stefan Richter <st...@s5...> > Cc: lin...@li... > Cc: Andrew Morton <ak...@li...> > Cc: Takashi Sakamoto <o-t...@sa...> > --- > v2: rebase/resend, add note to Andrew > v3: add Takashi-san > > drivers/firewire/init_ohci1394_dma.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Thank you to remind it to me. Acked-by: Takashi Sakamoto <o-t...@sa...> By the way, I got enough access permission to linux1394.git repository and I'm preparing it for maintenance work (not done yet). * https://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git/ After finishing the preparation (e.g. joining to linux-next integration), I'll apply your patch and tell it to you, within the week. > diff -- a/drivers/firewire/init_ohci1394_dma.c b/drivers/firewire/init_ohci1394_dma.c > --- a/drivers/firewire/init_ohci1394_dma.c > +++ b/drivers/firewire/init_ohci1394_dma.c > @@ -251,7 +251,7 @@ static inline void __init init_ohci1394_ > } > > /** > - * debug_init_ohci1394_dma - scan for OHCI1394 controllers and init DMA on them > + * init_ohci1394_dma_on_all_controllers - scan for OHCI1394 controllers and init DMA on them > * Scans the whole PCI space for OHCI1394 controllers and inits DMA on them > */ > void __init init_ohci1394_dma_on_all_controllers(void) > @@ -283,7 +283,7 @@ void __init init_ohci1394_dma_on_all_con > } > > /** > - * setup_init_ohci1394_early - enables early OHCI1394 DMA initialization > + * setup_ohci1394_dma - enables early OHCI1394 DMA initialization > */ > static int __init setup_ohci1394_dma(char *opt) > { Kind regards Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2023-03-12 07:07:43
|
Hi, On Sat, Mar 11, 2023 at 10:15:54AM +0100, Stefan Richter wrote: > On Mar 11 Takashi Sakamoto wrote: > > I'm glad to see you again in the list ;) > > > > I really appreciate your long effort for the subsystem. I guess that your > > life became swamped recently against your work in the subsystem, while I > > feel that soft-landing of the project is still preferable for users. > > Thank you. I need to apologize that I neglected to (at least) drop my > maintainership title myself. I should have done so when I started to > realize that I am not able to fulfill this role anymore. > > > Below items are in my plan for the subsystem until being closed. I'm > > pleased if getting your help in any time. > > > > * 2023, 2024, 2025, 2026 > > * take over the subsystem maintainer > > * set up repositories in `<https://git.kernel.org/>`_ > > Do you plan to re-use the existing (but neglected, by me) repos at > kernel.org? > https://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git > https://git.kernel.org/pub/scm/libs/ieee1394/libraw1394.git > https://git.kernel.org/pub/scm/libs/ieee1394/libiec61883.git > > If so, is there something which I can do to transfer access to you? > (Although if yes, before that I need to enable myself to send encrypted > e-mails again, after both of the mail user agents which I use dropped gpg > support. Maybe the kernel.org admins can assist you quicker with repo > access than I might.) In the last week after posting the patch, I sent a request to system administrator of kernel.org for the purpose. I think it will be done within a few days so nothing left to us. > > * refresh web site and update information > > * take over the administration of communication channels > > * adding the list archive of linux1394-devel to > > `lore.kernel.org <https://korg.docs.kernel.org/lore.html>`_ > > * fix issues of subsystem > > * modernize 1394 OHCI driver > > * Pull requests to Linus > > * get help from Linux Foundation to place documents for specification > > defined by 1394 Trade Association > > * If no problems, upload the documents to the new web site > > * invite repositories of external librararies (``libavc1394``, ``libdc1394``) > > * Announcement to distribution package maintainers about the upstream shift > > * 2027, 2028 > > * Close announcement to below applications > > * `FFMPEG <https://ffmpeg.org/>`_ > > * `GStreamer <https://gstreamer.freedesktop.org/>`_ > > * `VLC <https://www.videolan.org/vlc/index.ja.html>`_ > > * `MythTV <https://www.mythtv.org/>`_ > > * `FFADO <http://ffado.org/>`_ > > * 2029 > > * Close the project > > * Close the communication channels > > * Archive repositories > > * Resign the subsystem maintainer > > It's good to see you being active in the kernel and related userland > development/ maintainership, and that you have a plan for the next years. At the moment, I have a problem about the list archive. As you know, kernel development process heavily relies on mail. Recently many developers use lore.kernel.org to store blasted message as persistent storage. I would like to follow to it. According to the documentation[1], the list archive should be prepared including sent messages as much as possible. However I subscribed it Feb 2011 and sometimes lost messages. I think you have the stock of enough amount of messages in your mail box. If you have enough free time, I would ask you to create list archive for the purpose. [1] https://korg.docs.kernel.org/lore.html Regards Takashi Sakamoto |
From: Stefan R. <st...@s5...> - 2023-03-11 09:16:12
|
On Mar 11 Takashi Sakamoto wrote: > I'm glad to see you again in the list ;) > > I really appreciate your long effort for the subsystem. I guess that your > life became swamped recently against your work in the subsystem, while I > feel that soft-landing of the project is still preferable for users. Thank you. I need to apologize that I neglected to (at least) drop my maintainership title myself. I should have done so when I started to realize that I am not able to fulfill this role anymore. > Below items are in my plan for the subsystem until being closed. I'm > pleased if getting your help in any time. > > * 2023, 2024, 2025, 2026 > * take over the subsystem maintainer > * set up repositories in `<https://git.kernel.org/>`_ Do you plan to re-use the existing (but neglected, by me) repos at kernel.org? https://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git https://git.kernel.org/pub/scm/libs/ieee1394/libraw1394.git https://git.kernel.org/pub/scm/libs/ieee1394/libiec61883.git If so, is there something which I can do to transfer access to you? (Although if yes, before that I need to enable myself to send encrypted e-mails again, after both of the mail user agents which I use dropped gpg support. Maybe the kernel.org admins can assist you quicker with repo access than I might.) > * refresh web site and update information > * take over the administration of communication channels > * adding the list archive of linux1394-devel to > `lore.kernel.org <https://korg.docs.kernel.org/lore.html>`_ > * fix issues of subsystem > * modernize 1394 OHCI driver > * Pull requests to Linus > * get help from Linux Foundation to place documents for specification > defined by 1394 Trade Association > * If no problems, upload the documents to the new web site > * invite repositories of external librararies (``libavc1394``, ``libdc1394``) > * Announcement to distribution package maintainers about the upstream shift > * 2027, 2028 > * Close announcement to below applications > * `FFMPEG <https://ffmpeg.org/>`_ > * `GStreamer <https://gstreamer.freedesktop.org/>`_ > * `VLC <https://www.videolan.org/vlc/index.ja.html>`_ > * `MythTV <https://www.mythtv.org/>`_ > * `FFADO <http://ffado.org/>`_ > * 2029 > * Close the project > * Close the communication channels > * Archive repositories > * Resign the subsystem maintainer It's good to see you being active in the kernel and related userland development/ maintainership, and that you have a plan for the next years. -- Stefan Richter -======--=== --== -=-== http://arcgraph.de/sr/ |
From: Takashi S. <o-t...@sa...> - 2023-03-11 08:21:05
|
Hi Stefan, On Fri, Mar 10, 2023 at 09:03:56PM +0100, Stefan Richter wrote: > On Mar 06 Takashi Sakamoto wrote: > > In the last few years, I have reviewed patches for FireWire subsystem and > > requested sound subsystem maintainer to sent them to mainline, since > > FireWire subsystem maintainer has been long absent. This situation is not > > preferable since we have some user of sound hardware in IEEE 1394 bus. > > > > I will stand for the maintainer, and work for FireWire core functions and > > 1394 OHCI driver, as well as sound drivers. This commit replaces the > > corresponding entry. > > > > As you know, IEEE 1394 is enough legacy. I would like to schedule the end > > of my work in the subsystem. My effort will last next 6 years. In 2026, I > > will start strong announcement for users to migrate their work load from > > IEEE 1394 bus (e.g. by purchasing alternative devices in USB and hardening > > system for them), then in 2029 let me resign the maintainer and close > > Linux 1394 project. > > > > My current work focuses on real time data (sampling data) transmission > > protocol in packet-oriented communication, thus I would provide less help > > to implementations for the other type of protocol; i.e. IPv4/IPv6 over > > IEEE 1394 bus (firewire-net), SCSI transport protocol over IEEE 1394 bus > > (firewire-sbp2) and iSCSI target (sbp-target). > > > > If receiving few objections from developers, I will start my work to send > > fixes for v6.3 prepatch, and PR for future v6.4 or later. I'm pleased if > > getting any help until the end. > > > > Reference: commit b32744751e75 ("firewire: add to MAINTAINERS") > > Signed-off-by: Takashi Sakamoto <o-t...@sa...> > > Acked-by: Stefan Richter <st...@s5...> I'm glad to see you again in the list ;) I really appreciate your long effort for the subsystem. I guess that your life became swamped recently against your work in the subsystem, while I feel that soft-landing of the project is still preferable for users. Below items are in my plan for the subsystem until being closed. I'm pleased if getting your help in any time. * 2023, 2024, 2025, 2026 * take over the subsystem maintainer * set up repositories in `<https://git.kernel.org/>`_ * refresh web site and update information * take over the administration of communication channels * adding the list archive of linux1394-devel to `lore.kernel.org <https://korg.docs.kernel.org/lore.html>`_ * fix issues of subsystem * modernize 1394 OHCI driver * Pull requests to Linus * get help from Linux Foundation to place documents for specification defined by 1394 Trade Association * If no problems, upload the documents to the new web site * invite repositories of external librararies (``libavc1394``, ``libdc1394``) * Announcement to distribution package maintainers about the upstream shift * 2027, 2028 * Close announcement to below applications * `FFMPEG <https://ffmpeg.org/>`_ * `GStreamer <https://gstreamer.freedesktop.org/>`_ * `VLC <https://www.videolan.org/vlc/index.ja.html>`_ * `MythTV <https://www.mythtv.org/>`_ * `FFADO <http://ffado.org/>`_ * 2029 * Close the project * Close the communication channels * Archive repositories * Resign the subsystem maintainer > > --- > > MAINTAINERS | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 8d5bc223f..e137c1b2f 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -7954,10 +7954,11 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/nab/lio-core-2.6.git master > > F: drivers/target/sbp/ > > > > FIREWIRE SUBSYSTEM > > -M: Stefan Richter <st...@s5...> > > +M: Takashi Sakamoto <o-t...@sa...> > > +M: Takashi Sakamoto <tak...@ke...> > > L: lin...@li... > > S: Maintained > > -W: http://ieee1394.wiki.kernel.org/ > > +W: http://ieee1394.docs.kernel.org/ > > T: git git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git > > F: drivers/firewire/ > > F: include/linux/firewire.h > > > > -- > Stefan Richter > -======--=== --== -=-=- > http://arcgraph.de/sr/ Regards Takashi Sakamoto |
From: Stefan R. <st...@s5...> - 2023-03-10 20:18:47
|
On Mar 06 Takashi Sakamoto wrote: > In the last few years, I have reviewed patches for FireWire subsystem and > requested sound subsystem maintainer to sent them to mainline, since > FireWire subsystem maintainer has been long absent. This situation is not > preferable since we have some user of sound hardware in IEEE 1394 bus. > > I will stand for the maintainer, and work for FireWire core functions and > 1394 OHCI driver, as well as sound drivers. This commit replaces the > corresponding entry. > > As you know, IEEE 1394 is enough legacy. I would like to schedule the end > of my work in the subsystem. My effort will last next 6 years. In 2026, I > will start strong announcement for users to migrate their work load from > IEEE 1394 bus (e.g. by purchasing alternative devices in USB and hardening > system for them), then in 2029 let me resign the maintainer and close > Linux 1394 project. > > My current work focuses on real time data (sampling data) transmission > protocol in packet-oriented communication, thus I would provide less help > to implementations for the other type of protocol; i.e. IPv4/IPv6 over > IEEE 1394 bus (firewire-net), SCSI transport protocol over IEEE 1394 bus > (firewire-sbp2) and iSCSI target (sbp-target). > > If receiving few objections from developers, I will start my work to send > fixes for v6.3 prepatch, and PR for future v6.4 or later. I'm pleased if > getting any help until the end. > > Reference: commit b32744751e75 ("firewire: add to MAINTAINERS") > Signed-off-by: Takashi Sakamoto <o-t...@sa...> Acked-by: Stefan Richter <st...@s5...> > --- > MAINTAINERS | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 8d5bc223f..e137c1b2f 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -7954,10 +7954,11 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/nab/lio-core-2.6.git master > F: drivers/target/sbp/ > > FIREWIRE SUBSYSTEM > -M: Stefan Richter <st...@s5...> > +M: Takashi Sakamoto <o-t...@sa...> > +M: Takashi Sakamoto <tak...@ke...> > L: lin...@li... > S: Maintained > -W: http://ieee1394.wiki.kernel.org/ > +W: http://ieee1394.docs.kernel.org/ > T: git git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git > F: drivers/firewire/ > F: include/linux/firewire.h -- Stefan Richter -======--=== --== -=-=- http://arcgraph.de/sr/ |
From: Takashi S. <o-t...@sa...> - 2023-03-06 03:58:35
|
In the last few years, I have reviewed patches for FireWire subsystem and requested sound subsystem maintainer to sent them to mainline, since FireWire subsystem maintainer has been long absent. This situation is not preferable since we have some user of sound hardware in IEEE 1394 bus. I will stand for the maintainer, and work for FireWire core functions and 1394 OHCI driver, as well as sound drivers. This commit replaces the corresponding entry. As you know, IEEE 1394 is enough legacy. I would like to schedule the end of my work in the subsystem. My effort will last next 6 years. In 2026, I will start strong announcement for users to migrate their work load from IEEE 1394 bus (e.g. by purchasing alternative devices in USB and hardening system for them), then in 2029 let me resign the maintainer and close Linux 1394 project. My current work focuses on real time data (sampling data) transmission protocol in packet-oriented communication, thus I would provide less help to implementations for the other type of protocol; i.e. IPv4/IPv6 over IEEE 1394 bus (firewire-net), SCSI transport protocol over IEEE 1394 bus (firewire-sbp2) and iSCSI target (sbp-target). If receiving few objections from developers, I will start my work to send fixes for v6.3 prepatch, and PR for future v6.4 or later. I'm pleased if getting any help until the end. Reference: commit b32744751e75 ("firewire: add to MAINTAINERS") Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- MAINTAINERS | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 8d5bc223f..e137c1b2f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7954,10 +7954,11 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/nab/lio-core-2.6.git master F: drivers/target/sbp/ FIREWIRE SUBSYSTEM -M: Stefan Richter <st...@s5...> +M: Takashi Sakamoto <o-t...@sa...> +M: Takashi Sakamoto <tak...@ke...> L: lin...@li... S: Maintained -W: http://ieee1394.wiki.kernel.org/ +W: http://ieee1394.docs.kernel.org/ T: git git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git F: drivers/firewire/ F: include/linux/firewire.h -- 2.37.2 |
From: Takashi S. <o-t...@sa...> - 2023-02-27 12:49:30
|
Hi, (C.C.ed to LKML and alsa-devel) On Mon, Feb 27, 2023 at 02:06:51PM +0300, Dan Carpenter wrote: > Hello Takashi Sakamoto, > > The patch e699600232e0: "firewire: cdev: obsolete NULL check to > detect IEC 61883-1 FCP region" from Jan 20, 2023, leads to the > following Smatch static checker warning: > > drivers/firewire/core-transaction.c:947 handle_fcp_region_request() > warn: passing freed memory 'request' > > drivers/firewire/core-transaction.c > 930 fw_send_response(card, request, RCODE_TYPE_ERROR); > 931 > 932 return; > 933 } > 934 > 935 rcu_read_lock(); > 936 list_for_each_entry_rcu(handler, &address_handler_list, link) { > 937 if (is_enclosing_handler(handler, offset, request->length)) > 938 handler->address_callback(card, request, tcode, > ^^^^^^^ > This warning is because fwnet_receive_packet() has a kfree(r) on the > first return path. > > 939 destination, source, > 940 p->generation, offset, > 941 request->data, > 942 request->length, > 943 handler->callback_data); > 944 } > 945 rcu_read_unlock(); > 946 > --> 947 fw_send_response(card, request, RCODE_COMPLETE); > 948 } Thanks for your report. Fortunately, We can not see the access to the released memory since the fwnet's address handler is registered to high memory region (0x'0001'0000'0000 to 0x'ffff'e000'0000). The region does not overlap IEC 61883-1 FCP region (0x'ffff'f000'0b00 to 0x'ffff'f000'0f00). The handler is called from handle_exclusive_region_request() instead of handle_fcp_region_request(). However, the code in fwnet is against the design of address handler apparently. The callee never release the memory for the request structure directly. It should be done by the call of fw_send_response(). I'll correct it for next merge window; i.e. for v6.4. Thanks Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2023-01-25 12:08:07
|
Hi, On Mon, Jan 23, 2023 at 09:22:51AM +0100, Takashi Iwai wrote: > On Fri, 20 Jan 2023 10:03:41 +0100, > Takashi Sakamoto wrote: > > > > Hi, > > > > This patch solves long standing issue mentioned by code comment[1] and a > > commit 281e20323ab7 ("firewire: core: fix use-after-free regression in FCP > > handler")[2]. This patchset is based on the kernel tree to which another > > fix is applied[3]. > > > > To Iwai-san, I would like to ask you picking them to your local > > tree, then send them to mainline tree as well as sound patches when > > the merge window is open for v6.3 kernel, unless any question and > > objection is posted. (Additionally, I have prepared the other patchset for > > the subsystem.) > > As those are spontaneous small fixes, now I merged all three patches > on topic/firewire branch (on top of the for-linus including your > previous FireWire core fix), merged back to for-next branch for 6.3. Thanks for your applying. > But, I have no will to keep doing this in a long term. I suppose the > best would be that you'd step up as a maintainer for FireWire > stack... Indeed. The next patchset is beyond your courtesy. I posted it to LKML with my concern. I'm pleased if you follow to it. * https://lore.kernel.org/lkml/202...@sa.../ > thanks, > > Takashi Thanks Takashi Sakamoto |