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

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

[Rob Herring (Arm)]

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(-)

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");
-- 
2.43.0

[PATCH 2/2] dt-bindings: dma: Drop unused QCom hidma binding

[Rob Herring (Arm)]

The QCom hidma binding was used on a defunct QCom server platform which
mainly used ACPI. DT support in the Linux driver has been broken since
2018, so it seems this binding is unused and can be dropped.

Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
 .../bindings/dma/qcom_hidma_mgmt.txt          | 95 -------------------
 1 file changed, 95 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt

diff --git a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
deleted file mode 100644
index 1ae4748730a8..000000000000
--- a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
+++ /dev/null
@@ -1,95 +0,0 @@
-Qualcomm Technologies HIDMA Management interface
-
-Qualcomm Technologies HIDMA is a high speed DMA device. It only supports
-memcpy and memset capabilities. It has been designed for virtualized
-environments.
-
-Each HIDMA HW instance consists of multiple DMA channels. These channels
-share the same bandwidth. The bandwidth utilization can be partitioned
-among channels based on the priority and weight assignments.
-
-There are only two priority levels and 15 weigh assignments possible.
-
-Other parameters here determine how much of the system bus this HIDMA
-instance can use like maximum read/write request and number of bytes to
-read/write in a single burst.
-
-Main node required properties:
-- compatible: "qcom,hidma-mgmt-1.0";
-- reg: Address range for DMA device
-- dma-channels: Number of channels supported by this DMA controller.
-- max-write-burst-bytes: Maximum write burst in bytes that HIDMA can
-  occupy the bus for in a single transaction. A memcpy requested is
-  fragmented to multiples of this amount. This parameter is used while
-  writing into destination memory. Setting this value incorrectly can
-  starve other peripherals in the system.
-- max-read-burst-bytes: Maximum read burst in bytes that HIDMA can
-  occupy the bus for in a single transaction. A memcpy request is
-  fragmented to multiples of this amount. This parameter is used while
-  reading the source memory. Setting this value incorrectly can starve
-  other peripherals in the system.
-- max-write-transactions: This value is how many times a write burst is
-  applied back to back while writing to the destination before yielding
-  the bus.
-- max-read-transactions: This value is how many times a read burst is
-  applied back to back while reading the source before yielding the bus.
-- channel-reset-timeout-cycles: Channel reset timeout in cycles for this SOC.
-  Once a reset is applied to the HW, HW starts a timer for reset operation
-  to confirm. If reset is not completed within this time, HW reports reset
-  failure.
-
-Sub-nodes:
-
-HIDMA has one or more DMA channels that are used to move data from one
-memory location to another.
-
-When the OS is not in control of the management interface (i.e. it's a guest),
-the channel nodes appear on their own, not under a management node.
-
-Required properties:
-- compatible: must contain "qcom,hidma-1.0" for initial HW or
-  "qcom,hidma-1.1"/"qcom,hidma-1.2" for MSI capable HW.
-- reg: Addresses for the transfer and event channel
-- interrupts: Should contain the event interrupt
-- desc-count: Number of asynchronous requests this channel can handle
-- iommus: required a iommu node
-
-Optional properties for MSI:
-- msi-parent : See the generic MSI binding described in
- devicetree/bindings/interrupt-controller/msi.txt for a description of the
- msi-parent property.
-
-Example:
-
-Hypervisor OS configuration:
-
-	hidma-mgmt@f9984000 = {
-		compatible = "qcom,hidma-mgmt-1.0";
-		reg = <0xf9984000 0x15000>;
-		dma-channels = <6>;
-		max-write-burst-bytes = <1024>;
-		max-read-burst-bytes = <1024>;
-		max-write-transactions = <31>;
-		max-read-transactions = <31>;
-		channel-reset-timeout-cycles = <0x500>;
-
-		hidma_24: dma-controller@5c050000 {
-			compatible = "qcom,hidma-1.0";
-			reg = <0 0x5c050000 0x0 0x1000>,
-			      <0 0x5c0b0000 0x0 0x1000>;
-			interrupts = <0 389 0>;
-			desc-count = <10>;
-			iommus = <&system_mmu>;
-		};
-	};
-
-Guest OS configuration:
-
-	hidma_24: dma-controller@5c050000 {
-		compatible = "qcom,hidma-1.0";
-		reg = <0 0x5c050000 0x0 0x1000>,
-		      <0 0x5c0b0000 0x0 0x1000>;
-		interrupts = <0 389 0>;
-		desc-count = <10>;
-		iommus = <&system_mmu>;
-	};
-- 
2.43.0

Re: [PATCH 2/2] dt-bindings: dma: Drop unused QCom hidma binding

[Konrad Dybcio]

On 4/23/24 18:14, Rob Herring (Arm) wrote:
> The QCom hidma binding was used on a defunct QCom server platform which
> mainly used ACPI. DT support in the Linux driver has been broken since
> 2018, so it seems this binding is unused and can be dropped.
> 
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---

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

Konrad

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

[Konrad Dybcio]

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

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

[Jeffrey Hugo]

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>

Re: [PATCH 2/2] dt-bindings: dma: Drop unused QCom hidma binding

[Jeffrey Hugo]

On 4/23/2024 10:14 AM, Rob Herring (Arm) wrote:
> The QCom hidma binding was used on a defunct QCom server platform which
> mainly used ACPI. DT support in the Linux driver has been broken since
> 2018, so it seems this binding is unused and can be dropped.
> 
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>

Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>

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

[Dan Carpenter]

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

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

[Vinod Koul]

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,
-- 
~Vinod