[PATCH 0/9] firewire: ohci: adoption of device managed resource

[PATCH 0/9] firewire: ohci: adoption of device managed resource

[Takashi Sakamoto]

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

[PATCH 1/9] firewire: ohci: use devres for memory object of ohci structure

[Takashi Sakamoto]

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-takashi@sakamocchi.jp>
---
 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

[PATCH 2/9] firewire: ohci: use devres for PCI-related resources

[Takashi Sakamoto]

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-takashi@sakamocchi.jp>
---
 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

[PATCH 3/9] firewire: ohci: use devres for MMIO region mapping

[Takashi Sakamoto]

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-takashi@sakamocchi.jp>
---
 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

[PATCH 4/9] firewire: ohci: use devres for misc DMA buffer

[Takashi Sakamoto]

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-takashi@sakamocchi.jp>
---
 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

[PATCH 5/9] firewire: ohci: use devres for requested IRQ

[Takashi Sakamoto]

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-takashi@sakamocchi.jp>
---
 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

[PATCH 6/9] firewire: ohci: use devres for list of isochronous contexts

[Takashi Sakamoto]

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-takashi@sakamocchi.jp>
---
 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

[PATCH 7/9] firewire: ohci: use devres for IT, IR, AT/receive, and AT/request contexts

[Takashi Sakamoto]

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-takashi@sakamocchi.jp>
---
 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

[PATCH 8/9] firewire: ohci: use devres for content of configuration ROM

[Takashi Sakamoto]

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-takashi@sakamocchi.jp>
---
 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

[PATCH 9/9] firewire: ohci: release buffer for AR req/resp contexts when managed resource is released

[Takashi Sakamoto]

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-takashi@sakamocchi.jp>
---
 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

Re: [PATCH 0/9] firewire: ohci: adoption of device managed resource

[Takashi Sakamoto]

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