linux1394-devel Mailing List for IEEE 1394 for Linux (Page 15)
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
(34) |
Oct
|
Nov
|
Dec
|
From: Takashi S. <o-t...@sa...> - 2023-10-04 04:04:05
|
Hi, Thanks for your sending the patch. On Tue, Oct 03, 2023 at 09:16:50PM +0000, Kai Bosch wrote: > --- > drivers/firewire/net.c | 48 +++++++++++++++++++++--------------------- > 1 file changed, 24 insertions(+), 24 deletions(-) As long as I know, the coding style of Linux kernel[1] does not forbidden brief expression of 'unsigned int' (= just 'unsigned'). Furthermore, in C language specification, 'unsigned' and 'unsigned int' are the same[2]. I have no functional change over the patch. If the intension of patch is in the point of human readability, please note it in the commit comment. It is the most important for the kind of change. I'm waiting for your next post. [1] Linux kernel coding style https://www.kernel.org/doc/html/v4.10/process/coding-style.html [2] See '6.7.2 Type specifiers' in ISO/IEC 9899:1999, so called 'C99'. Regards Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2023-09-24 12:55:11
|
Hi, On Fri, Sep 22, 2023 at 10:53:35AM -0700, Kees Cook wrote: > Prepare for the coming implementation by GCC and Clang of the __counted_by > attribute. Flexible array members annotated with __counted_by can have > their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS > (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family > functions). > > As found with Coccinelle[1], add __counted_by for struct fw_node. > > [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci > > Cc: Takashi Sakamoto <o-t...@sa...> > Cc: lin...@li... > Signed-off-by: Kees Cook <kee...@ch...> > --- > drivers/firewire/core.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied to for-next branch, thanks. Regards Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2023-09-15 10:21:01
|
Hi, On Wed, Sep 13, 2023 at 11:54:39AM +0206, John Ogness wrote: > On 2023-06-04, Takashi Sakamoto <o-t...@sa...> wrote: > > The flag of GFP_ATOMIC is given to the call of kmalloc when building node > > tree, but the call is not atomic context. The call of > > fw_core_handle_bus_reset() and fw_core_remove_card() builds the tree, > > while they are done in specific workqueue or pci remove callback. > > The comment is incorrect about not being in atomic > context. fw_core_handle_bus_reset() calls build_tree() with the > card->lock spinlock held. With 6.6-rc1 I see: Thanks for your report. Just now I sent fixes to Linus for 6.6-rc1. It is expected to apply it to stable 6.5 kernel as well. > [ 12.555784] BUG: sleeping function called from invalid context at include/linux/sched/mm.h:306 > [ 12.555808] in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 70, name: kworker/1:2 > [ 12.555814] preempt_count: 1, expected: 0 > [ 12.555820] INFO: lockdep is turned off. > [ 12.555824] irq event stamp: 208 > [ 12.555828] hardirqs last enabled at (207): [<c00000000111e414>] ._raw_spin_unlock_irq+0x44/0x80 > [ 12.555850] hardirqs last disabled at (208): [<c00000000110ff94>] .__schedule+0x854/0xfe0 > [ 12.555859] softirqs last enabled at (188): [<c000000000f73504>] .addrconf_verify_rtnl+0x2c4/0xb70 > [ 12.555872] softirqs last disabled at (182): [<c000000000f732b0>] .addrconf_verify_rtnl+0x70/0xb70 > [ 12.555884] CPU: 1 PID: 70 Comm: kworker/1:2 Tainted: G S 6.6.0-rc1 #1 > [ 12.555893] Hardware name: PowerMac7,2 PPC970 0x390202 PowerMac > [ 12.555898] Workqueue: firewire_ohci .bus_reset_work [firewire_ohci] > [ 12.555939] Call Trace: > [ 12.555944] [c000000009677830] [c0000000010d83c0] .dump_stack_lvl+0x8c/0xd0 (unreliable) > [ 12.555963] [c0000000096778b0] [c000000000140270] .__might_resched+0x320/0x340 > [ 12.555978] [c000000009677940] [c000000000497600] .__kmem_cache_alloc_node+0x390/0x460 > [ 12.555993] [c000000009677a10] [c0000000003fe620] .__kmalloc+0x70/0x310 > [ 12.556007] [c000000009677ac0] [c0003d00004e2268] .fw_core_handle_bus_reset+0x2c8/0xba0 [firewire_core] > [ 12.556060] [c000000009677c20] [c0003d0000491190] .bus_reset_work+0x330/0x9b0 [firewire_ohci] > [ 12.556079] [c000000009677d10] [c00000000011d0d0] .process_one_work+0x280/0x6f0 > [ 12.556094] [c000000009677e10] [c00000000011d8a0] .worker_thread+0x360/0x500 > [ 12.556107] [c000000009677ef0] [c00000000012e3b4] .kthread+0x154/0x160 > [ 12.556120] [c000000009677f90] [c00000000000bfa8] .start_kernel_thread+0x10/0x14 > > John Ogness Thanks Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2023-09-07 10:36:03
|
Hi, On Wed, Sep 06, 2023 at 10:12:16AM +0200, Jan Engelhardt wrote: > Presently, one needs to enter "FireWire (IEEE 1394) subsystem --->", > then deselect all options individually. Speed up that process by > making the "--->" line be togglable in its own right. > > Signed-off-by: Jan Engelhardt <je...@in...> > --- > drivers/firewire/Kconfig | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) We are at the second half of merge window for v6.6 kernel, while the patch looks good to me. I applied it to for-next branch and will send it to Linus within a few days, after it passes check by for-next integration. > diff --git a/drivers/firewire/Kconfig b/drivers/firewire/Kconfig > index 0a6596b027db..be1a9e685782 100644 > --- a/drivers/firewire/Kconfig > +++ b/drivers/firewire/Kconfig > @@ -1,8 +1,18 @@ > # SPDX-License-Identifier: GPL-2.0-only > -menu "IEEE 1394 (FireWire) support" > +menuconfig FIREWIRE_SUPPORT > + bool "IEEE 1394 (FireWire) support" > + default y > depends on PCI || COMPILE_TEST > # firewire-core does not depend on PCI but is > # not useful without PCI controller driver > + help > + Support for FireWire. > + > + The answer to this question will not directly affect the > + kernel: saying N will just cause the configurator to skip all > + the questions about FireWire. > + > +if FIREWIRE_SUPPORT > > config FIREWIRE > tristate "FireWire driver stack" > @@ -96,4 +106,4 @@ config FIREWIRE_NOSY > > If unsure, say N. > > -endmenu > +endif # FIREWIRE_SUPPORT > -- > 2.42.0 Thanks Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2023-06-24 05:06:42
|
Hi, Thanks for the fix. On Fri, Jun 23, 2023 at 01:39:35PM +0800, Zhang Shurong wrote: > The netif_rx() function frees the skb so we can't dereference it to > save the skb->len. > > Signed-off-by: Zhang Shurong <zha...@fo...> > --- > drivers/firewire/net.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c > index 538bd677c254..7a4d1a478e33 100644 > --- a/drivers/firewire/net.c > +++ b/drivers/firewire/net.c > @@ -479,7 +479,7 @@ static int fwnet_finish_incoming_packet(struct net_device *net, > struct sk_buff *skb, u16 source_node_id, > bool is_broadcast, u16 ether_type) > { > - int status; > + int status, len; > > switch (ether_type) { > case ETH_P_ARP: > @@ -533,13 +533,15 @@ static int fwnet_finish_incoming_packet(struct net_device *net, > } > skb->protocol = protocol; > } > + > + len = skb->len; > status = netif_rx(skb); > if (status == NET_RX_DROP) { > net->stats.rx_errors++; > net->stats.rx_dropped++; > } else { > net->stats.rx_packets++; > - net->stats.rx_bytes += skb->len; > + net->stats.rx_bytes += len; > } > > return 0; I'm not good at network subsystem, but as long as reading network core, I think it better not to access members of sk_buff structure after the call of netif_rx() since the ownership is already delegated from firewire-net to the network core to dispatch network protocol. The patch looks good to me. Reviewed-by: Takashi Sakamoto <o-t...@sa...> The most of code in firewire-net comes from old driver (drivers/ieee1394/eth1394.c), thus the most of linux kernel which supports ieee1394/firewire subsystem affects the issue, however the potential issue has never appeared since the skb is released enough later in the different context. I applied the patch to linux-next branch for v6.5-rc1. Thanks Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2023-06-05 22:57:13
|
On Sun, Jun 04, 2023 at 04:02:55PM +0900, Takashi Sakamoto wrote: > The flag of GFP_ATOMIC is given to the call of kmalloc when building node > tree, but the call is not atomic context. The call of > fw_core_handle_bus_reset() and fw_core_remove_card() builds the tree, > while they are done in specific workqueue or pci remove callback. > > This commit obsolete the usage of GFP_ATOMIC. > > Signed-off-by: Takashi Sakamoto <o-t...@sa...> > --- > drivers/firewire/core-device.c | 2 +- > drivers/firewire/core-topology.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) Applied to for-next branch. Regards Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2023-06-05 22:56:42
|
On Sun, Jun 04, 2023 at 02:44:42PM +0900, Takashi Sakamoto wrote: > Hi, > > Linux FireWire subsystem includes a driver (firewire-ohci) for 1394 OHCI > controller. The code of driver is mostly written at the time when device > managed resource (devres) was not widely used. Nowadays the usage of > devres is standard when writing drivers. The series is an adoption of > devres for firewire-ohci. > > I note that MSI-related operation is left as is. The hardware vendors > forms their products of extension card with 1394 OHCI controller > connected to PCIe bus by several ways. If chip of 1394 OHCI controller has > PCIe interface (e.g. VIA VT6315, LSI FW643), it is just connected to PCIe > bus. If the chip has PCI interface only, it is connected to PCIe bus via > PCI/PCIe bridge chip (e.g. VIA VT6307 + asmedia ASM1083). There is some > chip of 1394 OHCI controller integrated with the bus bridge (e.g. TI > XIO2213, XIO2221). The MSI-related operation should cover the above > forms as well as module option, while it is still unclear that the > operation from pci device driver to the bus bridge. > > Takashi Sakamoto (9): > firewire: ohci: use devres for memory object of ohci structure > firewire: ohci: use devres for PCI-related resources > firewire: ohci: use devres for MMIO region mapping > firewire: ohci: use devres for misc DMA buffer > firewire: ohci: use devres for requested IRQ > firewire: ohci: use devres for list of isochronous contexts > firewire: ohci: use devres for IT, IR, AT/receive, and AT/request > contexts > firewire: ohci: use devres for content of configuration ROM > firewire: ohci: release buffer for AR req/resp contexts when managed > resource is released > > drivers/firewire/ohci.c | 174 +++++++++++++++------------------------- > 1 file changed, 63 insertions(+), 111 deletions(-) Applied to for-next branch. Regards Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2023-06-05 22:56:13
|
On Thu, Jun 01, 2023 at 11:49:37PM +0900, Takashi Sakamoto wrote: > Any target to generate UAPI documentation reports warnings to missing > annotation for padding member in structures added recently. > > This commit suppresses the warnings. > > Reported-by: Stephen Rothwell <sf...@ca...> > Closes: https://lore.kernel.org/lkml/202...@ca.../ > Fixes: 7c22d4a92bb2 ("firewire: cdev: add new event to notify request subaction with time stamp") > Fixes: fc2b52cf2e0e ("firewire: cdev: add new event to notify response subaction with time stamp") > Signed-off-by: Takashi Sakamoto <o-t...@sa...> > --- > include/uapi/linux/firewire-cdev.h | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) Applied to for-next branch. Regards Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2023-06-04 07:03:12
|
The flag of GFP_ATOMIC is given to the call of kmalloc when building node tree, but the call is not atomic context. The call of fw_core_handle_bus_reset() and fw_core_remove_card() builds the tree, while they are done in specific workqueue or pci remove callback. This commit obsolete the usage of GFP_ATOMIC. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-device.c | 2 +- drivers/firewire/core-topology.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index aa597cda0d88..a3104e35412c 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -1211,7 +1211,7 @@ void fw_node_event(struct fw_card *card, struct fw_node *node, int event) * without actually having a link. */ create: - device = kzalloc(sizeof(*device), GFP_ATOMIC); + device = kzalloc(sizeof(*device), GFP_KERNEL); if (device == NULL) break; diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c index f40c81534381..88466b663482 100644 --- a/drivers/firewire/core-topology.c +++ b/drivers/firewire/core-topology.c @@ -101,7 +101,7 @@ static struct fw_node *fw_node_create(u32 sid, int port_count, int color) { struct fw_node *node; - node = kzalloc(struct_size(node, ports, port_count), GFP_ATOMIC); + node = kzalloc(struct_size(node, ports, port_count), GFP_KERNEL); if (node == NULL) return NULL; -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-06-04 05:45:18
|
The 1394 OHCI driver allocates the list of isochronous contexts as much as the hardware supports. This commit utilizes managed device resource to maintain the lifetime of list. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/ohci.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index cb6b43e3f67e..086505bd1729 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -3666,7 +3666,11 @@ static int pci_probe(struct pci_dev *dev, ohci->ir_context_mask = ohci->ir_context_support; ohci->n_ir = hweight32(ohci->ir_context_mask); size = sizeof(struct iso_context) * ohci->n_ir; - ohci->ir_context_list = kzalloc(size, GFP_KERNEL); + ohci->ir_context_list = devm_kzalloc(&dev->dev, size, GFP_KERNEL); + if (!ohci->ir_context_list) { + err = -ENOMEM; + goto fail_atresp_ctx; + } reg_write(ohci, OHCI1394_IsoXmitIntMaskSet, ~0); ohci->it_context_support = reg_read(ohci, OHCI1394_IsoXmitIntMaskSet); @@ -3679,11 +3683,10 @@ static int pci_probe(struct pci_dev *dev, ohci->it_context_mask = ohci->it_context_support; ohci->n_it = hweight32(ohci->it_context_mask); size = sizeof(struct iso_context) * ohci->n_it; - ohci->it_context_list = kzalloc(size, GFP_KERNEL); - - if (ohci->it_context_list == NULL || ohci->ir_context_list == NULL) { + ohci->it_context_list = devm_kzalloc(&dev->dev, size, GFP_KERNEL); + if (!ohci->it_context_list) { err = -ENOMEM; - goto fail_contexts; + goto fail_atresp_ctx; } ohci->self_id = ohci->misc_buffer + PAGE_SIZE/2; @@ -3721,9 +3724,7 @@ static int pci_probe(struct pci_dev *dev, fail_msi: pci_disable_msi(dev); - fail_contexts: - kfree(ohci->ir_context_list); - kfree(ohci->it_context_list); + fail_atresp_ctx: context_release(&ohci->at_response_ctx); fail_atreq_ctx: context_release(&ohci->at_request_ctx); @@ -3767,8 +3768,6 @@ static void pci_remove(struct pci_dev *dev) ar_context_release(&ohci->ar_response_ctx); context_release(&ohci->at_request_ctx); context_release(&ohci->at_response_ctx); - kfree(ohci->it_context_list); - kfree(ohci->ir_context_list); pci_disable_msi(dev); dev_notice(&dev->dev, "removing fw-ohci device\n"); -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-06-04 05:45:18
|
The 1394 OHCI driver allocates DMA coherent buffer for descriptors of IT, IR, AT receive, and AT request contexts by the same way. This commit utilizes managed device resource to maintain the lifetime of buffers. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/ohci.c | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 086505bd1729..21eb13ea4dad 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -1105,8 +1105,7 @@ static int context_add_buffer(struct context *ctx) if (ctx->total_allocation >= 16*1024*1024) return -ENOMEM; - desc = dma_alloc_coherent(ctx->ohci->card.device, PAGE_SIZE, - &bus_addr, GFP_ATOMIC); + desc = dmam_alloc_coherent(ctx->ohci->card.device, PAGE_SIZE, &bus_addr, GFP_ATOMIC); if (!desc) return -ENOMEM; @@ -1165,10 +1164,10 @@ static void context_release(struct context *ctx) struct fw_card *card = &ctx->ohci->card; struct descriptor_buffer *desc, *tmp; - list_for_each_entry_safe(desc, tmp, &ctx->buffer_list, list) - dma_free_coherent(card->device, PAGE_SIZE, desc, - desc->buffer_bus - - ((void *)&desc->buffer - (void *)desc)); + list_for_each_entry_safe(desc, tmp, &ctx->buffer_list, list) { + dmam_free_coherent(card->device, PAGE_SIZE, desc, + desc->buffer_bus - ((void *)&desc->buffer - (void *)desc)); + } } /* Must be called with ohci->lock held */ @@ -3657,7 +3656,7 @@ static int pci_probe(struct pci_dev *dev, err = context_init(&ohci->at_response_ctx, ohci, OHCI1394_AsRspTrContextControlSet, handle_at_packet); if (err < 0) - goto fail_atreq_ctx; + goto fail_arrsp_ctx; reg_write(ohci, OHCI1394_IsoRecvIntMaskSet, ~0); ohci->ir_context_channels = ~0ULL; @@ -3669,7 +3668,7 @@ static int pci_probe(struct pci_dev *dev, ohci->ir_context_list = devm_kzalloc(&dev->dev, size, GFP_KERNEL); if (!ohci->ir_context_list) { err = -ENOMEM; - goto fail_atresp_ctx; + goto fail_arrsp_ctx; } reg_write(ohci, OHCI1394_IsoXmitIntMaskSet, ~0); @@ -3686,7 +3685,7 @@ static int pci_probe(struct pci_dev *dev, ohci->it_context_list = devm_kzalloc(&dev->dev, size, GFP_KERNEL); if (!ohci->it_context_list) { err = -ENOMEM; - goto fail_atresp_ctx; + goto fail_arrsp_ctx; } ohci->self_id = ohci->misc_buffer + PAGE_SIZE/2; @@ -3724,10 +3723,6 @@ static int pci_probe(struct pci_dev *dev, fail_msi: pci_disable_msi(dev); - fail_atresp_ctx: - context_release(&ohci->at_response_ctx); - fail_atreq_ctx: - context_release(&ohci->at_request_ctx); fail_arrsp_ctx: ar_context_release(&ohci->ar_response_ctx); fail_arreq_ctx: @@ -3766,8 +3761,6 @@ static void pci_remove(struct pci_dev *dev) ohci->config_rom, ohci->config_rom_bus); ar_context_release(&ohci->ar_request_ctx); ar_context_release(&ohci->ar_response_ctx); - context_release(&ohci->at_request_ctx); - context_release(&ohci->at_response_ctx); pci_disable_msi(dev); dev_notice(&dev->dev, "removing fw-ohci device\n"); -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-06-04 05:45:18
|
The 1394 OHCI controller register handler to single interrupt number. This commit uses managed device resource to maintain the lifetime of requested IRQ. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/ohci.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 3b31d90781fe..cb6b43e3f67e 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -3697,17 +3697,16 @@ static int pci_probe(struct pci_dev *dev, if (!(ohci->quirks & QUIRK_NO_MSI)) pci_enable_msi(dev); - if (request_irq(dev->irq, irq_handler, - pci_dev_msi_enabled(dev) ? 0 : IRQF_SHARED, - ohci_driver_name, ohci)) { + err = devm_request_irq(&dev->dev, dev->irq, irq_handler, + pci_dev_msi_enabled(dev) ? 0 : IRQF_SHARED, ohci_driver_name, ohci); + if (err < 0) { ohci_err(ohci, "failed to allocate interrupt %d\n", dev->irq); - err = -EIO; goto fail_msi; } err = fw_card_add(&ohci->card, max_receive, link_speed, guid); if (err) - goto fail_irq; + goto fail_msi; version = reg_read(ohci, OHCI1394_Version) & 0x00ff00ff; ohci_notice(ohci, @@ -3720,8 +3719,6 @@ static int pci_probe(struct pci_dev *dev, return 0; - fail_irq: - free_irq(dev->irq, ohci); fail_msi: pci_disable_msi(dev); fail_contexts: @@ -3759,7 +3756,6 @@ static void pci_remove(struct pci_dev *dev) */ software_reset(ohci); - free_irq(dev->irq, ohci); if (ohci->next_config_rom && ohci->next_config_rom != ohci->config_rom) dma_free_coherent(ohci->card.device, CONFIG_ROM_SIZE, -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-06-04 05:45:18
|
The 1394 OHCI driver allocates DMA coherent buffer to transfer content of configuration ROM. This commit utilizes managed device resource to maintain the lifetime of buffer. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/ohci.c | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 21eb13ea4dad..298667963538 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -2052,8 +2052,7 @@ static void bus_reset_work(struct work_struct *work) spin_unlock_irq(&ohci->lock); if (free_rom) - dma_free_coherent(ohci->card.device, CONFIG_ROM_SIZE, - free_rom, free_rom_bus); + dmam_free_coherent(ohci->card.device, CONFIG_ROM_SIZE, free_rom, free_rom_bus); log_selfids(ohci, generation, self_id_count); @@ -2385,10 +2384,8 @@ static int ohci_enable(struct fw_card *card, */ if (config_rom) { - ohci->next_config_rom = - dma_alloc_coherent(ohci->card.device, CONFIG_ROM_SIZE, - &ohci->next_config_rom_bus, - GFP_KERNEL); + ohci->next_config_rom = dmam_alloc_coherent(ohci->card.device, CONFIG_ROM_SIZE, + &ohci->next_config_rom_bus, GFP_KERNEL); if (ohci->next_config_rom == NULL) return -ENOMEM; @@ -2480,9 +2477,8 @@ static int ohci_set_config_rom(struct fw_card *card, * ohci->next_config_rom to NULL (see bus_reset_work). */ - next_config_rom = - dma_alloc_coherent(ohci->card.device, CONFIG_ROM_SIZE, - &next_config_rom_bus, GFP_KERNEL); + next_config_rom = dmam_alloc_coherent(ohci->card.device, CONFIG_ROM_SIZE, + &next_config_rom_bus, GFP_KERNEL); if (next_config_rom == NULL) return -ENOMEM; @@ -2515,9 +2511,10 @@ static int ohci_set_config_rom(struct fw_card *card, spin_unlock_irq(&ohci->lock); /* If we didn't use the DMA allocation, delete it. */ - if (next_config_rom != NULL) - dma_free_coherent(ohci->card.device, CONFIG_ROM_SIZE, - next_config_rom, next_config_rom_bus); + if (next_config_rom != NULL) { + dmam_free_coherent(ohci->card.device, CONFIG_ROM_SIZE, next_config_rom, + next_config_rom_bus); + } /* * Now initiate a bus reset to have the changes take @@ -3753,12 +3750,6 @@ static void pci_remove(struct pci_dev *dev) software_reset(ohci); - if (ohci->next_config_rom && ohci->next_config_rom != ohci->config_rom) - dma_free_coherent(ohci->card.device, CONFIG_ROM_SIZE, - ohci->next_config_rom, ohci->next_config_rom_bus); - if (ohci->config_rom) - dma_free_coherent(ohci->card.device, CONFIG_ROM_SIZE, - ohci->config_rom, ohci->config_rom_bus); ar_context_release(&ohci->ar_request_ctx); ar_context_release(&ohci->ar_response_ctx); pci_disable_msi(dev); -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-06-04 05:45:18
|
The 1394 OHCI driver allocates several non-coherent DMA buffers for AR request and response contexts. The buffers are mapped to kernel virtual address (VMA) so that the first page locates after the last page. Even when large payload of packet is handled crossing the boundary of buffers, the driver operates continuously on VMA. No kernel API is provided for this kind of mapping, while it is possible to release the buffer when PCI device is going to be released. This commit moves the call of release helper function to the callback function of release resources. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/ohci.c | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 298667963538..7e88fd489741 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -677,6 +677,9 @@ static void ar_context_release(struct ar_context *ctx) struct device *dev = ctx->ohci->card.device; unsigned int i; + if (!ctx->buffer) + return; + vunmap(ctx->buffer); for (i = 0; i < AR_BUFFERS; i++) { @@ -3556,9 +3559,13 @@ static inline void pmac_ohci_off(struct pci_dev *dev) {} static void release_ohci(struct device *dev, void *data) { struct pci_dev *pdev = to_pci_dev(dev); + struct fw_ohci *ohci = pci_get_drvdata(pdev); pmac_ohci_off(pdev); + ar_context_release(&ohci->ar_response_ctx); + ar_context_release(&ohci->ar_request_ctx); + dev_notice(dev, "removed fw-ohci device\n"); } @@ -3643,17 +3650,17 @@ static int pci_probe(struct pci_dev *dev, err = ar_context_init(&ohci->ar_response_ctx, ohci, PAGE_SIZE/4, OHCI1394_AsRspRcvContextControlSet); if (err < 0) - goto fail_arreq_ctx; + return err; err = context_init(&ohci->at_request_ctx, ohci, OHCI1394_AsReqTrContextControlSet, handle_at_packet); if (err < 0) - goto fail_arrsp_ctx; + return err; err = context_init(&ohci->at_response_ctx, ohci, OHCI1394_AsRspTrContextControlSet, handle_at_packet); if (err < 0) - goto fail_arrsp_ctx; + return err; reg_write(ohci, OHCI1394_IsoRecvIntMaskSet, ~0); ohci->ir_context_channels = ~0ULL; @@ -3663,10 +3670,8 @@ static int pci_probe(struct pci_dev *dev, ohci->n_ir = hweight32(ohci->ir_context_mask); size = sizeof(struct iso_context) * ohci->n_ir; ohci->ir_context_list = devm_kzalloc(&dev->dev, size, GFP_KERNEL); - if (!ohci->ir_context_list) { - err = -ENOMEM; - goto fail_arrsp_ctx; - } + if (!ohci->ir_context_list) + return -ENOMEM; reg_write(ohci, OHCI1394_IsoXmitIntMaskSet, ~0); ohci->it_context_support = reg_read(ohci, OHCI1394_IsoXmitIntMaskSet); @@ -3680,10 +3685,8 @@ static int pci_probe(struct pci_dev *dev, ohci->n_it = hweight32(ohci->it_context_mask); size = sizeof(struct iso_context) * ohci->n_it; ohci->it_context_list = devm_kzalloc(&dev->dev, size, GFP_KERNEL); - if (!ohci->it_context_list) { - err = -ENOMEM; - goto fail_arrsp_ctx; - } + if (!ohci->it_context_list) + return -ENOMEM; ohci->self_id = ohci->misc_buffer + PAGE_SIZE/2; ohci->self_id_bus = ohci->misc_buffer_bus + PAGE_SIZE/2; @@ -3720,10 +3723,6 @@ static int pci_probe(struct pci_dev *dev, fail_msi: pci_disable_msi(dev); - fail_arrsp_ctx: - ar_context_release(&ohci->ar_response_ctx); - fail_arreq_ctx: - ar_context_release(&ohci->ar_request_ctx); return err; } @@ -3750,8 +3749,6 @@ static void pci_remove(struct pci_dev *dev) software_reset(ohci); - ar_context_release(&ohci->ar_request_ctx); - ar_context_release(&ohci->ar_response_ctx); pci_disable_msi(dev); dev_notice(&dev->dev, "removing fw-ohci device\n"); -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-06-04 05:45:14
|
Hi, Linux FireWire subsystem includes a driver (firewire-ohci) for 1394 OHCI controller. The code of driver is mostly written at the time when device managed resource (devres) was not widely used. Nowadays the usage of devres is standard when writing drivers. The series is an adoption of devres for firewire-ohci. I note that MSI-related operation is left as is. The hardware vendors forms their products of extension card with 1394 OHCI controller connected to PCIe bus by several ways. If chip of 1394 OHCI controller has PCIe interface (e.g. VIA VT6315, LSI FW643), it is just connected to PCIe bus. If the chip has PCI interface only, it is connected to PCIe bus via PCI/PCIe bridge chip (e.g. VIA VT6307 + asmedia ASM1083). There is some chip of 1394 OHCI controller integrated with the bus bridge (e.g. TI XIO2213, XIO2221). The MSI-related operation should cover the above forms as well as module option, while it is still unclear that the operation from pci device driver to the bus bridge. Takashi Sakamoto (9): firewire: ohci: use devres for memory object of ohci structure firewire: ohci: use devres for PCI-related resources firewire: ohci: use devres for MMIO region mapping firewire: ohci: use devres for misc DMA buffer firewire: ohci: use devres for requested IRQ firewire: ohci: use devres for list of isochronous contexts firewire: ohci: use devres for IT, IR, AT/receive, and AT/request contexts firewire: ohci: use devres for content of configuration ROM firewire: ohci: release buffer for AR req/resp contexts when managed resource is released drivers/firewire/ohci.c | 174 +++++++++++++++------------------------- 1 file changed, 63 insertions(+), 111 deletions(-) -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-06-04 05:45:13
|
The PCI framework has the convenient helper function to check and map MMIO region with managed device resource. This commit elaborates 1394 OHCI driver to use the function. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/ohci.c | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index f3d0882a876c..26c64b60144d 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -3608,18 +3608,12 @@ static int pci_probe(struct pci_dev *dev, return -ENXIO; } - err = pci_request_region(dev, 0, ohci_driver_name); + err = pcim_iomap_regions(dev, 1 << 0, ohci_driver_name); if (err) { - ohci_err(ohci, "MMIO resource unavailable\n"); - return err; - } - - ohci->registers = pci_iomap(dev, 0, OHCI1394_REGISTER_SIZE); - if (ohci->registers == NULL) { - ohci_err(ohci, "failed to remap registers\n"); - err = -ENXIO; - goto fail_iomem; + ohci_err(ohci, "request and map MMIO resource unavailable\n"); + return -ENXIO; } + ohci->registers = pcim_iomap_table(dev)[0]; for (i = 0; i < ARRAY_SIZE(ohci_quirks); i++) if ((ohci_quirks[i].vendor == dev->vendor) && @@ -3644,10 +3638,8 @@ static int pci_probe(struct pci_dev *dev, PAGE_SIZE, &ohci->misc_buffer_bus, GFP_KERNEL); - if (!ohci->misc_buffer) { - err = -ENOMEM; - goto fail_iounmap; - } + if (!ohci->misc_buffer) + return -ENOMEM; err = ar_context_init(&ohci->ar_request_ctx, ohci, 0, OHCI1394_AsReqRcvContextControlSet); @@ -3747,10 +3739,6 @@ static int pci_probe(struct pci_dev *dev, fail_misc_buf: dma_free_coherent(ohci->card.device, PAGE_SIZE, ohci->misc_buffer, ohci->misc_buffer_bus); - fail_iounmap: - pci_iounmap(dev, ohci->registers); - fail_iomem: - pci_release_region(dev, 0); return err; } @@ -3793,8 +3781,6 @@ static void pci_remove(struct pci_dev *dev) kfree(ohci->it_context_list); kfree(ohci->ir_context_list); pci_disable_msi(dev); - pci_iounmap(dev, ohci->registers); - pci_release_region(dev, 0); dev_notice(&dev->dev, "removing fw-ohci device\n"); } -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-06-04 05:45:12
|
The 1394 OHCI driver allocates a DMA coherent buffer for multi-purposes. The buffer is split into three region for specific purposes; i.e. 1/4 for context descriptors of AR request and response as well as 1/2 for self ID handling. This commit uses managed device resource to maintain the lifetime of buffer. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/ohci.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 26c64b60144d..3b31d90781fe 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -3634,17 +3634,15 @@ static int pci_probe(struct pci_dev *dev, */ BUILD_BUG_ON(AR_BUFFERS * sizeof(struct descriptor) > PAGE_SIZE/4); BUILD_BUG_ON(SELF_ID_BUF_SIZE > PAGE_SIZE/2); - ohci->misc_buffer = dma_alloc_coherent(ohci->card.device, - PAGE_SIZE, - &ohci->misc_buffer_bus, - GFP_KERNEL); + ohci->misc_buffer = dmam_alloc_coherent(&dev->dev, PAGE_SIZE, &ohci->misc_buffer_bus, + GFP_KERNEL); if (!ohci->misc_buffer) return -ENOMEM; err = ar_context_init(&ohci->ar_request_ctx, ohci, 0, OHCI1394_AsReqRcvContextControlSet); if (err < 0) - goto fail_misc_buf; + return err; err = ar_context_init(&ohci->ar_response_ctx, ohci, PAGE_SIZE/4, OHCI1394_AsRspRcvContextControlSet); @@ -3736,9 +3734,6 @@ static int pci_probe(struct pci_dev *dev, ar_context_release(&ohci->ar_response_ctx); fail_arreq_ctx: ar_context_release(&ohci->ar_request_ctx); - fail_misc_buf: - dma_free_coherent(ohci->card.device, PAGE_SIZE, - ohci->misc_buffer, ohci->misc_buffer_bus); return err; } @@ -3774,8 +3769,6 @@ static void pci_remove(struct pci_dev *dev) ohci->config_rom, ohci->config_rom_bus); ar_context_release(&ohci->ar_request_ctx); ar_context_release(&ohci->ar_response_ctx); - dma_free_coherent(ohci->card.device, PAGE_SIZE, - ohci->misc_buffer, ohci->misc_buffer_bus); context_release(&ohci->at_request_ctx); context_release(&ohci->at_response_ctx); kfree(ohci->it_context_list); -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-06-04 05:45:11
|
The PCI framework supports managed device resource to maintain the lifetime of PCI specific resources. This commit allows 1394 OHCI driver to utilize it. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/ohci.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 2b02cebcb0ae..f3d0882a876c 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -3588,7 +3588,7 @@ static int pci_probe(struct pci_dev *dev, pmac_ohci_on(dev); devres_add(&dev->dev, ohci); - err = pci_enable_device(dev); + err = pcim_enable_device(dev); if (err) { dev_err(&dev->dev, "failed to enable OHCI hardware\n"); return err; @@ -3605,14 +3605,13 @@ static int pci_probe(struct pci_dev *dev, if (!(pci_resource_flags(dev, 0) & IORESOURCE_MEM) || pci_resource_len(dev, 0) < OHCI1394_REGISTER_SIZE) { ohci_err(ohci, "invalid MMIO resource\n"); - err = -ENXIO; - goto fail_disable; + return -ENXIO; } err = pci_request_region(dev, 0, ohci_driver_name); if (err) { ohci_err(ohci, "MMIO resource unavailable\n"); - goto fail_disable; + return err; } ohci->registers = pci_iomap(dev, 0, OHCI1394_REGISTER_SIZE); @@ -3752,8 +3751,6 @@ static int pci_probe(struct pci_dev *dev, pci_iounmap(dev, ohci->registers); fail_iomem: pci_release_region(dev, 0); - fail_disable: - pci_disable_device(dev); return err; } @@ -3798,7 +3795,6 @@ static void pci_remove(struct pci_dev *dev) pci_disable_msi(dev); pci_iounmap(dev, ohci->registers); pci_release_region(dev, 0); - pci_disable_device(dev); dev_notice(&dev->dev, "removing fw-ohci device\n"); } -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-06-04 05:45:11
|
The managed device resource (devres) framework is convenient to maintain lifetime of allocated memory object for device. This commit utilizes the framework for the object of ohci structure. The extra operation for power management is required in Apple PowerMac based machines, thus release callback is assigned to the object to call the operation. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/ohci.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 06386c3b7f03..2b02cebcb0ae 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -3557,6 +3557,15 @@ static inline void pmac_ohci_on(struct pci_dev *dev) {} static inline void pmac_ohci_off(struct pci_dev *dev) {} #endif /* CONFIG_PPC_PMAC */ +static void release_ohci(struct device *dev, void *data) +{ + struct pci_dev *pdev = to_pci_dev(dev); + + pmac_ohci_off(pdev); + + dev_notice(dev, "removed fw-ohci device\n"); +} + static int pci_probe(struct pci_dev *dev, const struct pci_device_id *ent) { @@ -3571,25 +3580,22 @@ static int pci_probe(struct pci_dev *dev, return -ENOSYS; } - ohci = kzalloc(sizeof(*ohci), GFP_KERNEL); - if (ohci == NULL) { - err = -ENOMEM; - goto fail; - } - + ohci = devres_alloc(release_ohci, sizeof(*ohci), GFP_KERNEL); + if (ohci == NULL) + return -ENOMEM; fw_card_initialize(&ohci->card, &ohci_driver, &dev->dev); - + pci_set_drvdata(dev, ohci); pmac_ohci_on(dev); + devres_add(&dev->dev, ohci); err = pci_enable_device(dev); if (err) { dev_err(&dev->dev, "failed to enable OHCI hardware\n"); - goto fail_free; + return err; } pci_set_master(dev); pci_write_config_dword(dev, OHCI1394_PCI_HCI_Control, 0); - pci_set_drvdata(dev, ohci); spin_lock_init(&ohci->lock); mutex_init(&ohci->phy_reg_mutex); @@ -3748,10 +3754,7 @@ static int pci_probe(struct pci_dev *dev, pci_release_region(dev, 0); fail_disable: pci_disable_device(dev); - fail_free: - kfree(ohci); - pmac_ohci_off(dev); - fail: + return err; } @@ -3796,10 +3799,8 @@ static void pci_remove(struct pci_dev *dev) pci_iounmap(dev, ohci->registers); pci_release_region(dev, 0); pci_disable_device(dev); - kfree(ohci); - pmac_ohci_off(dev); - dev_notice(&dev->dev, "removed fw-ohci device\n"); + dev_notice(&dev->dev, "removing fw-ohci device\n"); } #ifdef CONFIG_PM -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-06-03 04:25:29
|
Hi Randy, On Thu, Jun 01, 2023 at 01:23:27PM -0700, Randy Dunlap wrote: > Hi, > > On 6/1/23 07:49, Takashi Sakamoto wrote: > > Any target to generate UAPI documentation reports warnings to missing > > annotation for padding member in structures added recently. > > > > This commit suppresses the warnings. > > > > Reported-by: Stephen Rothwell <sf...@ca...> > > Closes: https://lore.kernel.org/lkml/202...@ca.../ > > Fixes: 7c22d4a92bb2 ("firewire: cdev: add new event to notify request subaction with time stamp") > > Fixes: fc2b52cf2e0e ("firewire: cdev: add new event to notify response subaction with time stamp") > > Signed-off-by: Takashi Sakamoto <o-t...@sa...> > > --- > > include/uapi/linux/firewire-cdev.h | 14 ++++++-------- > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > You can do it as this patch shows, or you can hide those padding fields as > described in Documentation/doc-guide/kernel-doc.rst: > > Inside a struct or union description, you can use the ``private:`` and > ``public:`` comment tags. Structure fields that are inside a ``private:`` > area are not listed in the generated output documentation. > > The ``private:`` and ``public:`` tags must begin immediately following a > ``/*`` comment marker. They may optionally include comments between the > ``:`` and the ending ``*/`` marker. > > See below. > (snip) Thanks for your review and suggestion. Indeed, the private/public annotation is one of our options and I have realized it. I think it my preference to reduce load when reading structure layout. The usage of private/public requires readers' brain to switch context of access modifier. I guess that I would like to avoid such load if I were the reader. If the structure definition includes many annotations, it also increases such load. In my experience, it is likely that annotation in structure definition does not necessarily include enough information about the member itself, especially when writing applications. I think it my preference as well that member annotation of structure is in document, (but it would be case-by-case.) It seems to be explicit way, vaguely. Anyway thanks for your comment. It is fun to me;) Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2023-06-01 14:49:51
|
Any target to generate UAPI documentation reports warnings to missing annotation for padding member in structures added recently. This commit suppresses the warnings. Reported-by: Stephen Rothwell <sf...@ca...> Closes: https://lore.kernel.org/lkml/202...@ca.../ Fixes: 7c22d4a92bb2 ("firewire: cdev: add new event to notify request subaction with time stamp") Fixes: fc2b52cf2e0e ("firewire: cdev: add new event to notify response subaction with time stamp") Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- include/uapi/linux/firewire-cdev.h | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/include/uapi/linux/firewire-cdev.h b/include/uapi/linux/firewire-cdev.h index 99e823935427..1f2c9469f921 100644 --- a/include/uapi/linux/firewire-cdev.h +++ b/include/uapi/linux/firewire-cdev.h @@ -130,6 +130,9 @@ struct fw_cdev_event_response { * @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. * @response_tstamp: The time stamp of isochronous cycle at which the response was sent. + * @padding: 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. * @data: Payload data, if any * * This event is sent when the stack receives a response to an outgoing request @@ -155,10 +158,6 @@ struct fw_cdev_event_response2 { __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[]; }; @@ -231,6 +230,9 @@ struct fw_cdev_event_request2 { * @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. + * @padding: 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. * @data: Incoming data, if any * * This event is sent when the stack receives an incoming request to an address @@ -284,10 +286,6 @@ struct fw_cdev_event_request3 { __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[]; }; -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-05-30 10:25:49
|
The added KUnit test has no MODULE_LICENSE even if built for tristate. It brings build failure in linux-next integration. This commit releases the test under GPL and fixes the bug. Reported-by: Stephen Rothwell <sf...@ca...> Closes: https://lore.kernel.org/lkml/202...@ca.../ Fixes: dc7c51638f46 ("firewire: add KUnit test to check layout of UAPI structures") Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/uapi-test.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/firewire/uapi-test.c b/drivers/firewire/uapi-test.c index c6ebf02e3d45..2fcbede4fab1 100644 --- a/drivers/firewire/uapi-test.c +++ b/drivers/firewire/uapi-test.c @@ -85,3 +85,5 @@ static struct kunit_suite structure_layout_test_suite = { .test_cases = structure_layout_test_cases, }; kunit_test_suite(structure_layout_test_suite); + +MODULE_LICENSE("GPL"); -- 2.39.2 |
From: Takashi S. <o-t...@sa...> - 2023-05-29 23:15:14
|
On Mon, May 29, 2023 at 08:33:54PM +0900, Takashi Sakamoto wrote: > Cc: kun...@go... > > Hi, > > This patchset is revised version of the previous one[1], I realized that > it included wrong version number of Linux kernel. The new series > includes changes just for it. > > > Copied from the initial 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. > > [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 Applied to for-next branch. * https://git.kernel.org/ieee1394/linux1394/c/fe971f9163b6 Thanks Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2023-05-29 22:45:58
|
Hi, On Mon, May 29, 2023 at 12:52:07PM -0600, Gustavo A. R. Silva wrote: > Zero-length and one-element arrays are deprecated, and we are moving > towards adopting C99 flexible-array members, instead. > > Address the following warnings found with GCC-13 and > -fstrict-flex-arrays=3 enabled: > sound/firewire/amdtp-stream.c: In function ‘build_it_pkt_header’: > sound/firewire/amdtp-stream.c:694:17: warning: ‘generate_cip_header’ accessing 8 bytes in a region of size 0 [-Wstringop-overflow=] > 694 | generate_cip_header(s, cip_header, data_block_counter, syt); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > sound/firewire/amdtp-stream.c:694:17: note: referencing argument 2 of type ‘__be32[2]’ {aka ‘unsigned int[2]’} > sound/firewire/amdtp-stream.c:667:13: note: in a call to function ‘generate_cip_header’ > 667 | static void generate_cip_header(struct amdtp_stream *s, __be32 cip_header[2], > | ^~~~~~~~~~~~~~~~~~~ > > This helps with the ongoing efforts to tighten the FORTIFY_SOURCE > routines on memcpy() and help us make progress towards globally > enabling -fstrict-flex-arrays=3 [1]. > > Link: https://github.com/KSPP/linux/issues/21 > Link: https://github.com/KSPP/linux/issues/303 > Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [1] > Signed-off-by: Gustavo A. R. Silva <gus...@ke...> > --- > include/linux/firewire.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Thanks for the patch. I applied it to for-linus branch as a fix for v6.4-rc4, instead of for-next branch. * https://git.kernel.org/ieee1394/linux1394/c/41dd10a086c5 Thanks Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2023-05-29 11:34:43
|
In 1394 OHCI, the OUTPUT_LAST descriptor of Asynchronous Transmit (AT) context has timeStamp field, in which 1394 OHCI controller record the isochronous cycle when the packet was sent for the request subaction. Additionally, the trailing quadlet of Asynchronous Receive (AR) context has timeStamp field as well in which 1394 OHCI controller record the isochronous cycle when the packet arrived. The time stamps are also available for the cases to send and receive phy packet. This commit implements new events with time stamp field for user space. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-cdev.c | 90 +++++++++++++++++++++++++++--------- 1 file changed, 68 insertions(+), 22 deletions(-) diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index 2220de3c945e..6274b86eb943 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -206,6 +206,7 @@ struct outbound_phy_packet_event { struct fw_packet p; union { struct fw_cdev_event_phy_packet without_tstamp; + struct fw_cdev_event_phy_packet2 with_tstamp; } phy_packet; }; @@ -213,6 +214,7 @@ struct inbound_phy_packet_event { struct event event; union { struct fw_cdev_event_phy_packet without_tstamp; + struct fw_cdev_event_phy_packet2 with_tstamp; } phy_packet; }; @@ -1555,7 +1557,6 @@ static void outbound_phy_packet_callback(struct fw_packet *packet, container_of(packet, struct outbound_phy_packet_event, p); struct client *e_client = e->client; u32 rcode; - struct fw_cdev_event_phy_packet *pp; switch (status) { // expected: @@ -1583,10 +1584,31 @@ static void outbound_phy_packet_callback(struct fw_packet *packet, break; } - pp = &e->phy_packet.without_tstamp; - pp->rcode = rcode; - pp->data[0] = packet->timestamp; - queue_event(e->client, &e->event, &e->phy_packet, sizeof(*pp) + pp->length, NULL, 0); + switch (e->phy_packet.without_tstamp.type) { + case FW_CDEV_EVENT_PHY_PACKET_SENT: + { + struct fw_cdev_event_phy_packet *pp = &e->phy_packet.without_tstamp; + + pp->rcode = rcode; + pp->data[0] = packet->timestamp; + queue_event(e->client, &e->event, &e->phy_packet, sizeof(*pp) + pp->length, + NULL, 0); + break; + } + case FW_CDEV_EVENT_PHY_PACKET_SENT2: + { + struct fw_cdev_event_phy_packet2 *pp = &e->phy_packet.with_tstamp; + + pp->rcode = rcode; + pp->tstamp = packet->timestamp; + queue_event(e->client, &e->event, &e->phy_packet, sizeof(*pp) + pp->length, + NULL, 0); + break; + } + default: + WARN_ON(1); + break; + } client_put(e_client); } @@ -1596,13 +1618,12 @@ static int ioctl_send_phy_packet(struct client *client, union ioctl_arg *arg) struct fw_cdev_send_phy_packet *a = &arg->send_phy_packet; struct fw_card *card = client->device->card; struct outbound_phy_packet_event *e; - struct fw_cdev_event_phy_packet *pp; /* Access policy: Allow this ioctl only on local nodes' device files. */ if (!client->device->is_local) return -ENOSYS; - e = kzalloc(sizeof(*e) + 4, GFP_KERNEL); + e = kzalloc(sizeof(*e) + sizeof(a->data), GFP_KERNEL); if (e == NULL) return -ENOMEM; @@ -1616,11 +1637,23 @@ static int ioctl_send_phy_packet(struct client *client, union ioctl_arg *arg) e->p.header_length = 12; e->p.callback = outbound_phy_packet_callback; - pp = &e->phy_packet.without_tstamp; - pp->closure = a->closure; - pp->type = FW_CDEV_EVENT_PHY_PACKET_SENT; - if (is_ping_packet(a->data)) - pp->length = 4; + if (client->version < FW_CDEV_VERSION_EVENT_ASYNC_TSTAMP) { + struct fw_cdev_event_phy_packet *pp = &e->phy_packet.without_tstamp; + + pp->closure = a->closure; + pp->type = FW_CDEV_EVENT_PHY_PACKET_SENT; + if (is_ping_packet(a->data)) + pp->length = 4; + } else { + struct fw_cdev_event_phy_packet2 *pp = &e->phy_packet.with_tstamp; + + pp->closure = a->closure; + pp->type = FW_CDEV_EVENT_PHY_PACKET_SENT2; + // Keep the data field so that application can match the response event to the + // request. + pp->length = sizeof(a->data); + memcpy(pp->data, a->data, sizeof(a->data)); + } card->driver->send_request(card, &e->p); @@ -1655,20 +1688,33 @@ void fw_cdev_handle_phy_packet(struct fw_card *card, struct fw_packet *p) spin_lock_irqsave(&card->lock, flags); list_for_each_entry(client, &card->phy_receiver_list, phy_receiver_link) { - struct fw_cdev_event_phy_packet *pp; - e = kmalloc(sizeof(*e) + 8, GFP_ATOMIC); if (e == NULL) break; - pp = &e->phy_packet.without_tstamp; - pp->closure = client->phy_receiver_closure; - pp->type = FW_CDEV_EVENT_PHY_PACKET_RECEIVED; - pp->rcode = RCODE_COMPLETE; - pp->length = 8; - pp->data[0] = p->header[1]; - pp->data[1] = p->header[2]; - queue_event(client, &e->event, &e->phy_packet, sizeof(*pp) + 8, NULL, 0); + if (client->version < FW_CDEV_VERSION_EVENT_ASYNC_TSTAMP) { + struct fw_cdev_event_phy_packet *pp = &e->phy_packet.without_tstamp; + + pp->closure = client->phy_receiver_closure; + pp->type = FW_CDEV_EVENT_PHY_PACKET_RECEIVED; + pp->rcode = RCODE_COMPLETE; + pp->length = 8; + pp->data[0] = p->header[1]; + pp->data[1] = p->header[2]; + queue_event(client, &e->event, &e->phy_packet, sizeof(*pp) + 8, NULL, 0); + } else { + struct fw_cdev_event_phy_packet2 *pp = &e->phy_packet.with_tstamp; + + pp = &e->phy_packet.with_tstamp; + pp->closure = client->phy_receiver_closure; + pp->type = FW_CDEV_EVENT_PHY_PACKET_RECEIVED2; + pp->rcode = RCODE_COMPLETE; + pp->length = 8; + pp->tstamp = p->timestamp; + pp->data[0] = p->header[1]; + pp->data[1] = p->header[2]; + queue_event(client, &e->event, &e->phy_packet, sizeof(*pp) + 8, NULL, 0); + } } spin_unlock_irqrestore(&card->lock, flags); -- 2.39.2 |