diff mbox series

[1/2] dmaengine: qcom: Drop hidma DT support

Message ID 20240423161413.481670-1-robh@kernel.org (mailing list archive)
State New, archived
Headers show
Series [1/2] dmaengine: qcom: Drop hidma DT support | expand

Commit Message

Rob Herring April 23, 2024, 4:14 p.m. UTC
The DT support in hidma has been broken since commit 37fa4905d22a
("dmaengine: qcom_hidma: simplify DT resource parsing") in 2018. The
issue is the of_address_to_resource() calls bail out on success rather
than failure. This driver is for a defunct QCom server platform where
DT use was limited to start with. As it seems no one has noticed the
breakage, just remove the DT support altogether.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
 drivers/dma/qcom/hidma.c      |  11 ----
 drivers/dma/qcom/hidma_mgmt.c | 109 +---------------------------------
 2 files changed, 1 insertion(+), 119 deletions(-)

Comments

Konrad Dybcio April 23, 2024, 4:23 p.m. UTC | #1
On 4/23/24 18:14, Rob Herring (Arm) wrote:
> The DT support in hidma has been broken since commit 37fa4905d22a
> ("dmaengine: qcom_hidma: simplify DT resource parsing") in 2018. The
> issue is the of_address_to_resource() calls bail out on success rather
> than failure. This driver is for a defunct QCom server platform where
> DT use was limited to start with. As it seems no one has noticed the
> breakage, just remove the DT support altogether.
> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
Jeffrey Hugo April 23, 2024, 4:24 p.m. UTC | #2
On 4/23/2024 10:14 AM, Rob Herring (Arm) wrote:
> The DT support in hidma has been broken since commit 37fa4905d22a
> ("dmaengine: qcom_hidma: simplify DT resource parsing") in 2018. The
> issue is the of_address_to_resource() calls bail out on success rather
> than failure. This driver is for a defunct QCom server platform where
> DT use was limited to start with. As it seems no one has noticed the
> breakage, just remove the DT support altogether.
> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>

Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Dan Carpenter April 24, 2024, 6:22 a.m. UTC | #3
On Tue, Apr 23, 2024 at 08:51:36PM -0400, Sinan Kaya wrote:
> On 4/23/2024 12:14 PM, Rob Herring (Arm) wrote:
> > The DT support in hidma has been broken since commit 37fa4905d22a
> > ("dmaengine: qcom_hidma: simplify DT resource parsing") in 2018. The
> > issue is the of_address_to_resource() calls bail out on success rather
> > than failure. This driver is for a defunct QCom server platform where
> > DT use was limited to start with. As it seems no one has noticed the
> > breakage, just remove the DT support altogether.
> 
> I disagree here. This seems to have been broken your patch.
> 
> dmaengine: qcom_hidma: simplify DT resource parsing ยท torvalds/linux@37fa490
> (github.com) <https://github.com/torvalds/linux/commit/37fa4905d22a903f9fe120016fe7d6a2ece8d736>

That's the same commit that was mentioned in the first sentence of the
commit message.  The commit is from Jan 2018 but the oldest supported
kernel (v4.19) is from Oct 2018.  If someone really cares about this
code then they should be testing supported kernels...

regards,
dan carpenter
Vinod Koul April 25, 2024, 9:17 a.m. UTC | #4
On Tue, 23 Apr 2024 11:14:11 -0500, Rob Herring (Arm) wrote:
> The DT support in hidma has been broken since commit 37fa4905d22a
> ("dmaengine: qcom_hidma: simplify DT resource parsing") in 2018. The
> issue is the of_address_to_resource() calls bail out on success rather
> than failure. This driver is for a defunct QCom server platform where
> DT use was limited to start with. As it seems no one has noticed the
> breakage, just remove the DT support altogether.
> 
> [...]

Applied, thanks!

[1/2] dmaengine: qcom: Drop hidma DT support
      commit: d100ffe5048ef10065a2dac426d27dc458d9a94a
[2/2] dt-bindings: dma: Drop unused QCom hidma binding
      commit: e83cd59df0959bd9fbec76b7cff0b717ff8bc16f

Best regards,
diff mbox series

Patch

diff --git a/drivers/dma/qcom/hidma.c b/drivers/dma/qcom/hidma.c
index 202ac95227cb..721b4ac0857a 100644
--- a/drivers/dma/qcom/hidma.c
+++ b/drivers/dma/qcom/hidma.c
@@ -50,7 +50,6 @@ 
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
-#include <linux/of_dma.h>
 #include <linux/property.h>
 #include <linux/delay.h>
 #include <linux/acpi.h>
@@ -947,22 +946,12 @@  static const struct acpi_device_id hidma_acpi_ids[] = {
 MODULE_DEVICE_TABLE(acpi, hidma_acpi_ids);
 #endif
 
-static const struct of_device_id hidma_match[] = {
-	{.compatible = "qcom,hidma-1.0",},
-	{.compatible = "qcom,hidma-1.1", .data = (void *)(HIDMA_MSI_CAP),},
-	{.compatible = "qcom,hidma-1.2",
-	 .data = (void *)(HIDMA_MSI_CAP | HIDMA_IDENTITY_CAP),},
-	{},
-};
-MODULE_DEVICE_TABLE(of, hidma_match);
-
 static struct platform_driver hidma_driver = {
 	.probe = hidma_probe,
 	.remove_new = hidma_remove,
 	.shutdown = hidma_shutdown,
 	.driver = {
 		   .name = "hidma",
-		   .of_match_table = hidma_match,
 		   .acpi_match_table = ACPI_PTR(hidma_acpi_ids),
 	},
 };
diff --git a/drivers/dma/qcom/hidma_mgmt.c b/drivers/dma/qcom/hidma_mgmt.c
index 1d675f31252b..bb883e138ebf 100644
--- a/drivers/dma/qcom/hidma_mgmt.c
+++ b/drivers/dma/qcom/hidma_mgmt.c
@@ -7,12 +7,7 @@ 
 
 #include <linux/dmaengine.h>
 #include <linux/acpi.h>
-#include <linux/of.h>
 #include <linux/property.h>
-#include <linux/of_address.h>
-#include <linux/of_irq.h>
-#include <linux/of_platform.h>
-#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/module.h>
 #include <linux/uaccess.h>
@@ -327,115 +322,13 @@  static const struct acpi_device_id hidma_mgmt_acpi_ids[] = {
 MODULE_DEVICE_TABLE(acpi, hidma_mgmt_acpi_ids);
 #endif
 
-static const struct of_device_id hidma_mgmt_match[] = {
-	{.compatible = "qcom,hidma-mgmt-1.0",},
-	{},
-};
-MODULE_DEVICE_TABLE(of, hidma_mgmt_match);
-
 static struct platform_driver hidma_mgmt_driver = {
 	.probe = hidma_mgmt_probe,
 	.driver = {
 		   .name = "hidma-mgmt",
-		   .of_match_table = hidma_mgmt_match,
 		   .acpi_match_table = ACPI_PTR(hidma_mgmt_acpi_ids),
 	},
 };
 
-#if defined(CONFIG_OF) && defined(CONFIG_OF_IRQ)
-static int object_counter;
-
-static int __init hidma_mgmt_of_populate_channels(struct device_node *np)
-{
-	struct platform_device *pdev_parent = of_find_device_by_node(np);
-	struct platform_device_info pdevinfo;
-	struct device_node *child;
-	struct resource *res;
-	int ret = 0;
-
-	/* allocate a resource array */
-	res = kcalloc(3, sizeof(*res), GFP_KERNEL);
-	if (!res)
-		return -ENOMEM;
-
-	for_each_available_child_of_node(np, child) {
-		struct platform_device *new_pdev;
-
-		ret = of_address_to_resource(child, 0, &res[0]);
-		if (!ret)
-			goto out;
-
-		ret = of_address_to_resource(child, 1, &res[1]);
-		if (!ret)
-			goto out;
-
-		ret = of_irq_to_resource(child, 0, &res[2]);
-		if (ret <= 0)
-			goto out;
-
-		memset(&pdevinfo, 0, sizeof(pdevinfo));
-		pdevinfo.fwnode = &child->fwnode;
-		pdevinfo.parent = pdev_parent ? &pdev_parent->dev : NULL;
-		pdevinfo.name = child->name;
-		pdevinfo.id = object_counter++;
-		pdevinfo.res = res;
-		pdevinfo.num_res = 3;
-		pdevinfo.data = NULL;
-		pdevinfo.size_data = 0;
-		pdevinfo.dma_mask = DMA_BIT_MASK(64);
-		new_pdev = platform_device_register_full(&pdevinfo);
-		if (IS_ERR(new_pdev)) {
-			ret = PTR_ERR(new_pdev);
-			goto out;
-		}
-		new_pdev->dev.of_node = child;
-		of_dma_configure(&new_pdev->dev, child, true);
-		/*
-		 * It is assumed that calling of_msi_configure is safe on
-		 * platforms with or without MSI support.
-		 */
-		of_msi_configure(&new_pdev->dev, child);
-	}
-
-	kfree(res);
-
-	return ret;
-
-out:
-	of_node_put(child);
-	kfree(res);
-
-	return ret;
-}
-#endif
-
-static int __init hidma_mgmt_init(void)
-{
-#if defined(CONFIG_OF) && defined(CONFIG_OF_IRQ)
-	struct device_node *child;
-
-	for_each_matching_node(child, hidma_mgmt_match) {
-		/* device tree based firmware here */
-		hidma_mgmt_of_populate_channels(child);
-	}
-#endif
-	/*
-	 * We do not check for return value here, as it is assumed that
-	 * platform_driver_register must not fail. The reason for this is that
-	 * the (potential) hidma_mgmt_of_populate_channels calls above are not
-	 * cleaned up if it does fail, and to do this work is quite
-	 * complicated. In particular, various calls of of_address_to_resource,
-	 * of_irq_to_resource, platform_device_register_full, of_dma_configure,
-	 * and of_msi_configure which then call other functions and so on, must
-	 * be cleaned up - this is not a trivial exercise.
-	 *
-	 * Currently, this module is not intended to be unloaded, and there is
-	 * no module_exit function defined which does the needed cleanup. For
-	 * this reason, we have to assume success here.
-	 */
-	platform_driver_register(&hidma_mgmt_driver);
-
-	return 0;
-}
-module_init(hidma_mgmt_init);
+module_platform_driver(hidma_mgmt_driver);
 MODULE_LICENSE("GPL v2");