[PATCH 01/10] Documentation: DT: qcom_hidma: update binding for MSI

[PATCH 01/10] Documentation: DT: qcom_hidma: update binding for MSI

[Sinan Kaya]

Adding a new binding for qcom,hidma-1.1 to distinguish HW supporting
MSI interrupts from the older revision.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
index fd5618b..45ce75b 100644
--- a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
+++ b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
@@ -47,7 +47,8 @@ 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"
+- compatible: must contain "qcom,hidma-1.0" for initial HW or "qcom,hidma-1.1"
+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
-- 
1.8.2.1

Re: [PATCH 01/10] Documentation: DT: qcom_hidma: update binding for MSI

[Mark Rutland]

On Mon, Jul 18, 2016 at 02:39:28PM -0400, Sinan Kaya wrote:
> Adding a new binding for qcom,hidma-1.1 to distinguish HW supporting
> MSI interrupts from the older revision.
> 
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
> index fd5618b..45ce75b 100644
> --- a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
> +++ b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
> @@ -47,7 +47,8 @@ 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"
> +- compatible: must contain "qcom,hidma-1.0" for initial HW or "qcom,hidma-1.1"
> +for MSI capable HW.

Don't we need properties to describe the MSI details (e.g. sideband data
like DeviceID)?

Thanks,
Mark.

>  - reg: Addresses for the transfer and event channel
>  - interrupts: Should contain the event interrupt
>  - desc-count: Number of asynchronous requests this channel can handle
> -- 
> 1.8.2.1
> 

Re: [PATCH 01/10] Documentation: DT: qcom_hidma: update binding for MSI

[Sinan Kaya]

On 7/19/2016 6:37 AM, Mark Rutland wrote:
>>  Required properties:
>> > -- compatible: must contain "qcom,hidma-1.0"
>> > +- compatible: must contain "qcom,hidma-1.0" for initial HW or "qcom,hidma-1.1"
>> > +for MSI capable HW.
> Don't we need properties to describe the MSI details (e.g. sideband data
> like DeviceID)?
> 
> Thanks,
> Mark.
> 

You are right. I forgot to copy those.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 02/10] Documentation: DT: qcom_hidma: correct spelling mistakes

[Sinan Kaya]

Fix the spelling mistakes and extra and statements in the sentences.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
index 45ce75b..314562b 100644
--- a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
+++ b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
@@ -5,13 +5,13 @@ 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 parititioned
+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 and number of bytes to
+instance can use like maximum read/write request and number of bytes to
 read/write in a single burst.
 
 Main node required properties:
-- 
1.8.2.1

Re: [PATCH 02/10] Documentation: DT: qcom_hidma: correct spelling mistakes

[Rob Herring]

On Mon, Jul 18, 2016 at 02:39:29PM -0400, Sinan Kaya wrote:
> Fix the spelling mistakes and extra and statements in the sentences.
> 
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>

[PATCH 03/10] of: irq: make of_msi_configure accessible from modules

[Sinan Kaya]

The of_msi_configure routine is only accessible by the built-in
kernel drivers. Export this function so that modules can use it
too.

This function is useful for configuring MSI on child device tree
nodes on hierarchical objects.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/of/irq.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 6ec743f..d84b323 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -762,3 +762,4 @@ void of_msi_configure(struct device *dev, struct device_node *np)
 	dev_set_msi_domain(dev,
 			   of_msi_get_domain(dev, np, DOMAIN_BUS_PLATFORM_MSI));
 }
+EXPORT_SYMBOL_GPL(of_msi_configure);
-- 
1.8.2.1

Re: [PATCH 03/10] of: irq: make of_msi_configure accessible from modules

[Rob Herring]

On Mon, Jul 18, 2016 at 1:39 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
> The of_msi_configure routine is only accessible by the built-in
> kernel drivers. Export this function so that modules can use it
> too.
>
> This function is useful for configuring MSI on child device tree
> nodes on hierarchical objects.
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
>  drivers/of/irq.c | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 03/10] of: irq: make of_msi_configure accessible from modules

[Sinan Kaya]

On 7/18/2016 5:24 PM, Rob Herring wrote:
> On Mon, Jul 18, 2016 at 1:39 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
>> The of_msi_configure routine is only accessible by the built-in
>> kernel drivers. Export this function so that modules can use it
>> too.
>>
>> This function is useful for configuring MSI on child device tree
>> nodes on hierarchical objects.
>>
>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
>> ---
>>  drivers/of/irq.c | 1 +
>>  1 file changed, 1 insertion(+)
> 
> Acked-by: Rob Herring <robh@kernel.org>
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Thanks Rob

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 04/10] dmaending: qcom_hidma: configure DMA and MSI for OF

[Sinan Kaya]

Configure the DMA bindings for the device tree based firmware.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/dma/qcom/hidma_mgmt.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/qcom/hidma_mgmt.c b/drivers/dma/qcom/hidma_mgmt.c
index c0e3653..fb30730 100644
--- a/drivers/dma/qcom/hidma_mgmt.c
+++ b/drivers/dma/qcom/hidma_mgmt.c
@@ -375,8 +375,10 @@ static int __init hidma_mgmt_of_populate_channels(struct device_node *np)
 			ret = -ENODEV;
 			goto out;
 		}
+		of_node_get(child);
+		new_pdev->dev.of_node = child;
 		of_dma_configure(&new_pdev->dev, child);
-
+		of_msi_configure(&new_pdev->dev, child);
 		kfree(res);
 		res = NULL;
 	}
-- 
1.8.2.1

Re: [PATCH 04/10] dmaending: qcom_hidma: configure DMA and MSI for OF

[Vinod Koul]

On Tue, Jul 19, 2016 at 12:09:31AM +0530, Sinan Kaya wrote:
> Configure the DMA bindings for the device tree based firmware.
> 
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
>  drivers/dma/qcom/hidma_mgmt.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/qcom/hidma_mgmt.c b/drivers/dma/qcom/hidma_mgmt.c
> index c0e3653..fb30730 100644
> --- a/drivers/dma/qcom/hidma_mgmt.c
> +++ b/drivers/dma/qcom/hidma_mgmt.c
> @@ -375,8 +375,10 @@ static int __init hidma_mgmt_of_populate_channels(struct device_node *np)
>                         ret = -ENODEV;
>                         goto out;
>                 }
> +               of_node_get(child);
> +               new_pdev->dev.of_node = child;
>                 of_dma_configure(&new_pdev->dev, child);
> -
> +               of_msi_configure(&new_pdev->dev, child);

How would this work on non MSI platforms?

-- 
~Vinod

Re: [PATCH 04/10] dmaending: qcom_hidma: configure DMA and MSI for OF

[Sinan Kaya]

On 7/24/2016 2:33 AM, Vinod Koul wrote:
> On Tue, Jul 19, 2016 at 12:09:31AM +0530, Sinan Kaya wrote:
>> Configure the DMA bindings for the device tree based firmware.
>>
>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
>> ---
>>  drivers/dma/qcom/hidma_mgmt.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma/qcom/hidma_mgmt.c b/drivers/dma/qcom/hidma_mgmt.c
>> index c0e3653..fb30730 100644
>> --- a/drivers/dma/qcom/hidma_mgmt.c
>> +++ b/drivers/dma/qcom/hidma_mgmt.c
>> @@ -375,8 +375,10 @@ static int __init hidma_mgmt_of_populate_channels(struct device_node *np)
>>                         ret = -ENODEV;
>>                         goto out;
>>                 }
>> +               of_node_get(child);
>> +               new_pdev->dev.of_node = child;
>>                 of_dma_configure(&new_pdev->dev, child);
>> -
>> +               of_msi_configure(&new_pdev->dev, child);
> 
> How would this work on non MSI platforms?
> 

The way it works is MSI on OF platforms relies on an optional msi-parent and msi-cells attribute
present in the device tree firmware. 

If these attributes do not exist, the function bails out. The function would create the MSI bindings
only if the correct attributes are present. 

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 05/10] dmaengine: qcom_hidma: make pending_tre_count atomic

[Sinan Kaya]

Getting ready for the MSI interrupts. The pending_tre_count is used
in the interrupt handler to make sure all outstanding requests are
serviced.

Making it atomic so that it can be updated from multiple contexts.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/dma/qcom/hidma.h     |  2 +-
 drivers/dma/qcom/hidma_dbg.c |  3 ++-
 drivers/dma/qcom/hidma_ll.c  | 13 ++++++-------
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/qcom/hidma.h b/drivers/dma/qcom/hidma.h
index db413a5..158bf1f 100644
--- a/drivers/dma/qcom/hidma.h
+++ b/drivers/dma/qcom/hidma.h
@@ -58,7 +58,7 @@ struct hidma_lldev {
 	void __iomem *evca;		/* Event Channel address          */
 	struct hidma_tre
 		**pending_tre_list;	/* Pointers to pending TREs	  */
-	s32 pending_tre_count;		/* Number of TREs pending	  */
+	atomic_t pending_tre_count;	/* Number of TREs pending	  */
 
 	void *tre_ring;			/* TRE ring			  */
 	dma_addr_t tre_dma;		/* TRE ring to be shared with HW  */
diff --git a/drivers/dma/qcom/hidma_dbg.c b/drivers/dma/qcom/hidma_dbg.c
index fa827e5..87db285 100644
--- a/drivers/dma/qcom/hidma_dbg.c
+++ b/drivers/dma/qcom/hidma_dbg.c
@@ -74,7 +74,8 @@ static void hidma_ll_devstats(struct seq_file *s, void *llhndl)
 	seq_printf(s, "tre_ring_handle=%pap\n", &lldev->tre_dma);
 	seq_printf(s, "tre_ring_size = 0x%x\n", lldev->tre_ring_size);
 	seq_printf(s, "tre_processed_off = 0x%x\n", lldev->tre_processed_off);
-	seq_printf(s, "pending_tre_count=%d\n", lldev->pending_tre_count);
+	seq_printf(s, "pending_tre_count=%d\n",
+			atomic_read(&lldev->pending_tre_count));
 	seq_printf(s, "evca=%p\n", lldev->evca);
 	seq_printf(s, "evre_ring=%p\n", lldev->evre_ring);
 	seq_printf(s, "evre_ring_handle=%pap\n", &lldev->evre_dma);
diff --git a/drivers/dma/qcom/hidma_ll.c b/drivers/dma/qcom/hidma_ll.c
index f392900..f564c92 100644
--- a/drivers/dma/qcom/hidma_ll.c
+++ b/drivers/dma/qcom/hidma_ll.c
@@ -218,10 +218,9 @@ static int hidma_post_completed(struct hidma_lldev *lldev, int tre_iterator,
 	 * Keep track of pending TREs that SW is expecting to receive
 	 * from HW. We got one now. Decrement our counter.
 	 */
-	lldev->pending_tre_count--;
-	if (lldev->pending_tre_count < 0) {
+	if (atomic_dec_return(&lldev->pending_tre_count) < 0) {
 		dev_warn(lldev->dev, "tre count mismatch on completion");
-		lldev->pending_tre_count = 0;
+		atomic_set(&lldev->pending_tre_count, 0);
 	}
 
 	spin_unlock_irqrestore(&lldev->lock, flags);
@@ -321,7 +320,7 @@ void hidma_cleanup_pending_tre(struct hidma_lldev *lldev, u8 err_info,
 	u32 tre_read_off;
 
 	tre_iterator = lldev->tre_processed_off;
-	while (lldev->pending_tre_count) {
+	while (atomic_read(&lldev->pending_tre_count)) {
 		if (hidma_post_completed(lldev, tre_iterator, err_info,
 					 err_code))
 			break;
@@ -564,7 +563,7 @@ void hidma_ll_queue_request(struct hidma_lldev *lldev, u32 tre_ch)
 	tre->err_code = 0;
 	tre->err_info = 0;
 	tre->queued = 1;
-	lldev->pending_tre_count++;
+	atomic_inc(&lldev->pending_tre_count);
 	lldev->tre_write_offset = (lldev->tre_write_offset + HIDMA_TRE_SIZE)
 					% lldev->tre_ring_size;
 	spin_unlock_irqrestore(&lldev->lock, flags);
@@ -670,7 +669,7 @@ int hidma_ll_setup(struct hidma_lldev *lldev)
 	u32 val;
 	u32 nr_tres = lldev->nr_tres;
 
-	lldev->pending_tre_count = 0;
+	atomic_set(&lldev->pending_tre_count, 0);
 	lldev->tre_processed_off = 0;
 	lldev->evre_processed_off = 0;
 	lldev->tre_write_offset = 0;
@@ -833,7 +832,7 @@ int hidma_ll_uninit(struct hidma_lldev *lldev)
 	tasklet_kill(&lldev->task);
 	memset(lldev->trepool, 0, required_bytes);
 	lldev->trepool = NULL;
-	lldev->pending_tre_count = 0;
+	atomic_set(&lldev->pending_tre_count, 0);
 	lldev->tre_write_offset = 0;
 
 	rc = hidma_ll_reset(lldev);
-- 
1.8.2.1

[PATCH 06/10] dmaengine: qcom_hidma: make error and success path common

[Sinan Kaya]

Remove code duplication by feeding the error code from outside
into successful data transfer handler.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/dma/qcom/hidma_ll.c | 38 +++++++++++---------------------------
 1 file changed, 11 insertions(+), 27 deletions(-)

diff --git a/drivers/dma/qcom/hidma_ll.c b/drivers/dma/qcom/hidma_ll.c
index f564c92..a71f0c7 100644
--- a/drivers/dma/qcom/hidma_ll.c
+++ b/drivers/dma/qcom/hidma_ll.c
@@ -241,11 +241,11 @@ static int hidma_post_completed(struct hidma_lldev *lldev, int tre_iterator,
  * Return a positive number if there are pending TREs or EVREs.
  * Return 0 if there is nothing to consume or no pending TREs/EVREs found.
  */
-static int hidma_handle_tre_completion(struct hidma_lldev *lldev)
+static int hidma_handle_tre_completion(struct hidma_lldev *lldev, u8 err_info,
+				       u8 err_code)
 {
 	u32 evre_ring_size = lldev->evre_ring_size;
 	u32 tre_ring_size = lldev->tre_ring_size;
-	u32 err_info, err_code, evre_write_off;
 	u32 tre_iterator, evre_iterator;
 	u32 num_completed = 0;
 
@@ -268,10 +268,13 @@ static int hidma_handle_tre_completion(struct hidma_lldev *lldev)
 		u32 cfg;
 
 		cfg = current_evre[HIDMA_EVRE_CFG_IDX];
-		err_info = cfg >> HIDMA_EVRE_ERRINFO_BIT_POS;
-		err_info &= HIDMA_EVRE_ERRINFO_MASK;
-		err_code =
-		    (cfg >> HIDMA_EVRE_CODE_BIT_POS) & HIDMA_EVRE_CODE_MASK;
+		if (!err_info) {
+			err_info = cfg >> HIDMA_EVRE_ERRINFO_BIT_POS;
+			err_info &= HIDMA_EVRE_ERRINFO_MASK;
+		}
+		if (!err_code)
+			err_code = (cfg >> HIDMA_EVRE_CODE_BIT_POS) &
+					HIDMA_EVRE_CODE_MASK;
 
 		if (hidma_post_completed(lldev, tre_iterator, err_info,
 					 err_code))
@@ -314,27 +317,8 @@ static int hidma_handle_tre_completion(struct hidma_lldev *lldev)
 void hidma_cleanup_pending_tre(struct hidma_lldev *lldev, u8 err_info,
 			       u8 err_code)
 {
-	u32 tre_iterator;
-	u32 tre_ring_size = lldev->tre_ring_size;
-	int num_completed = 0;
-	u32 tre_read_off;
-
-	tre_iterator = lldev->tre_processed_off;
-	while (atomic_read(&lldev->pending_tre_count)) {
-		if (hidma_post_completed(lldev, tre_iterator, err_info,
-					 err_code))
-			break;
-		HIDMA_INCREMENT_ITERATOR(tre_iterator, HIDMA_TRE_SIZE,
-					 tre_ring_size);
-		num_completed++;
-	}
-	tre_read_off = (lldev->tre_processed_off +
-			HIDMA_TRE_SIZE * num_completed);
-
-	tre_read_off = tre_read_off % tre_ring_size;
-
-	/* record the last processed tre offset */
-	lldev->tre_processed_off = tre_read_off;
+	while (atomic_read(&lldev->pending_tre_count))
+		hidma_handle_tre_completion(lldev, err_info, err_code);
 }
 
 static int hidma_ll_reset(struct hidma_lldev *lldev)
-- 
1.8.2.1

[PATCH 07/10] dmaengine: qcom_hidma: eliminate processed variables

[Sinan Kaya]

Instead of tracking the read/write pointers in SW, track the
current state by reading from HW directly. This is an attempt
to eliminate potential race conditions.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/dma/qcom/hidma.h     |  5 +++--
 drivers/dma/qcom/hidma_dbg.c |  6 ++++--
 drivers/dma/qcom/hidma_ll.c  | 29 +++++++----------------------
 3 files changed, 14 insertions(+), 26 deletions(-)

diff --git a/drivers/dma/qcom/hidma.h b/drivers/dma/qcom/hidma.h
index 158bf1f..03c20f6 100644
--- a/drivers/dma/qcom/hidma.h
+++ b/drivers/dma/qcom/hidma.h
@@ -28,6 +28,9 @@
 #define HIDMA_TRE_DEST_LOW_IDX		4
 #define HIDMA_TRE_DEST_HI_IDX		5
 
+#define HIDMA_EVCA_READ_PTR_REG                0x018
+#define HIDMA_TRCA_READ_PTR_REG                0x018
+
 struct hidma_tre {
 	atomic_t allocated;		/* if this channel is allocated	    */
 	bool queued;			/* flag whether this is pending     */
@@ -63,12 +66,10 @@ struct hidma_lldev {
 	void *tre_ring;			/* TRE ring			  */
 	dma_addr_t tre_dma;		/* TRE ring to be shared with HW  */
 	u32 tre_ring_size;		/* Byte size of the ring	  */
-	u32 tre_processed_off;		/* last processed TRE		  */
 
 	void *evre_ring;		/* EVRE ring			   */
 	dma_addr_t evre_dma;		/* EVRE ring to be shared with HW  */
 	u32 evre_ring_size;		/* Byte size of the ring	   */
-	u32 evre_processed_off;		/* last processed EVRE		   */
 
 	u32 tre_write_offset;           /* TRE write location              */
 	struct tasklet_struct task;	/* task delivering notifications   */
diff --git a/drivers/dma/qcom/hidma_dbg.c b/drivers/dma/qcom/hidma_dbg.c
index 87db285..d741b9d 100644
--- a/drivers/dma/qcom/hidma_dbg.c
+++ b/drivers/dma/qcom/hidma_dbg.c
@@ -73,14 +73,16 @@ static void hidma_ll_devstats(struct seq_file *s, void *llhndl)
 	seq_printf(s, "tre_ring=%p\n", lldev->tre_ring);
 	seq_printf(s, "tre_ring_handle=%pap\n", &lldev->tre_dma);
 	seq_printf(s, "tre_ring_size = 0x%x\n", lldev->tre_ring_size);
-	seq_printf(s, "tre_processed_off = 0x%x\n", lldev->tre_processed_off);
+	seq_printf(s, "tre_processed_off = 0x%x\n",
+			readl(lldev->trca + HIDMA_TRCA_READ_PTR_REG));
 	seq_printf(s, "pending_tre_count=%d\n",
 			atomic_read(&lldev->pending_tre_count));
 	seq_printf(s, "evca=%p\n", lldev->evca);
 	seq_printf(s, "evre_ring=%p\n", lldev->evre_ring);
 	seq_printf(s, "evre_ring_handle=%pap\n", &lldev->evre_dma);
 	seq_printf(s, "evre_ring_size = 0x%x\n", lldev->evre_ring_size);
-	seq_printf(s, "evre_processed_off = 0x%x\n", lldev->evre_processed_off);
+	seq_printf(s, "evre_processed_off = 0x%x\n",
+			readl(lldev->evca + HIDMA_EVCA_READ_PTR_REG));
 	seq_printf(s, "tre_write_offset = 0x%x\n", lldev->tre_write_offset);
 }
 
diff --git a/drivers/dma/qcom/hidma_ll.c b/drivers/dma/qcom/hidma_ll.c
index a71f0c7..2753210 100644
--- a/drivers/dma/qcom/hidma_ll.c
+++ b/drivers/dma/qcom/hidma_ll.c
@@ -245,13 +245,13 @@ static int hidma_handle_tre_completion(struct hidma_lldev *lldev, u8 err_info,
 				       u8 err_code)
 {
 	u32 evre_ring_size = lldev->evre_ring_size;
-	u32 tre_ring_size = lldev->tre_ring_size;
+	u32 evre_write_off;
 	u32 tre_iterator, evre_iterator;
 	u32 num_completed = 0;
 
 	evre_write_off = readl_relaxed(lldev->evca + HIDMA_EVCA_WRITE_PTR_REG);
-	tre_iterator = lldev->tre_processed_off;
-	evre_iterator = lldev->evre_processed_off;
+	evre_iterator = readl_relaxed(lldev->evca + HIDMA_EVCA_READ_PTR_REG);
+	tre_iterator = (evre_iterator / HIDMA_EVRE_SIZE) * HIDMA_TRE_SIZE;
 
 	if ((evre_write_off > evre_ring_size) ||
 	    (evre_write_off % HIDMA_EVRE_SIZE)) {
@@ -280,10 +280,10 @@ static int hidma_handle_tre_completion(struct hidma_lldev *lldev, u8 err_info,
 					 err_code))
 			break;
 
-		HIDMA_INCREMENT_ITERATOR(tre_iterator, HIDMA_TRE_SIZE,
-					 tre_ring_size);
 		HIDMA_INCREMENT_ITERATOR(evre_iterator, HIDMA_EVRE_SIZE,
 					 evre_ring_size);
+		tre_iterator = (evre_iterator / HIDMA_EVRE_SIZE) *
+					HIDMA_TRE_SIZE;
 
 		/*
 		 * Read the new event descriptor written by the HW.
@@ -295,21 +295,8 @@ static int hidma_handle_tre_completion(struct hidma_lldev *lldev, u8 err_info,
 		num_completed++;
 	}
 
-	if (num_completed) {
-		u32 evre_read_off = (lldev->evre_processed_off +
-				     HIDMA_EVRE_SIZE * num_completed);
-		u32 tre_read_off = (lldev->tre_processed_off +
-				    HIDMA_TRE_SIZE * num_completed);
-
-		evre_read_off = evre_read_off % evre_ring_size;
-		tre_read_off = tre_read_off % tre_ring_size;
-
-		writel(evre_read_off, lldev->evca + HIDMA_EVCA_DOORBELL_REG);
-
-		/* record the last processed tre offset */
-		lldev->tre_processed_off = tre_read_off;
-		lldev->evre_processed_off = evre_read_off;
-	}
+	if (num_completed)
+		writel(evre_iterator, lldev->evca + HIDMA_EVCA_DOORBELL_REG);
 
 	return num_completed;
 }
@@ -654,8 +641,6 @@ int hidma_ll_setup(struct hidma_lldev *lldev)
 	u32 nr_tres = lldev->nr_tres;
 
 	atomic_set(&lldev->pending_tre_count, 0);
-	lldev->tre_processed_off = 0;
-	lldev->evre_processed_off = 0;
 	lldev->tre_write_offset = 0;
 
 	/* disable interrupts */
-- 
1.8.2.1

[PATCH 08/10] dmaengine: qcom_hidma: bring out interrupt cause

[Sinan Kaya]

Bring out the interrupt cause to the top level so that MSI interrupts
can be hooked at a later stage.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/dma/qcom/hidma_ll.c | 60 ++++++++++++++++++++-------------------------
 1 file changed, 27 insertions(+), 33 deletions(-)

diff --git a/drivers/dma/qcom/hidma_ll.c b/drivers/dma/qcom/hidma_ll.c
index 2753210..eb78952 100644
--- a/drivers/dma/qcom/hidma_ll.c
+++ b/drivers/dma/qcom/hidma_ll.c
@@ -403,12 +403,18 @@ static void hidma_ll_abort(unsigned long arg)
  * requests traditionally to the destination, this concept does not apply
  * here for this HW.
  */
-irqreturn_t hidma_ll_inthandler(int chirq, void *arg)
+static void hidma_ll_int_handler_internal(struct hidma_lldev *lldev, int cause)
 {
-	struct hidma_lldev *lldev = arg;
-	u32 status;
-	u32 enable;
-	u32 cause;
+	if (cause & HIDMA_ERR_INT_MASK) {
+		dev_err(lldev->dev, "error 0x%x, resetting...\n",
+				cause);
+
+		/* Clear out pending interrupts */
+		writel(cause, lldev->evca + HIDMA_EVCA_IRQ_CLR_REG);
+
+		tasklet_schedule(&lldev->rst_task);
+		return;
+	}
 
 	/*
 	 * Fine tuned for this HW...
@@ -418,40 +424,28 @@ irqreturn_t hidma_ll_inthandler(int chirq, void *arg)
 	 * interrupt delivery guarantees. Do not copy this code blindly and
 	 * expect that to work.
 	 */
-	status = readl_relaxed(lldev->evca + HIDMA_EVCA_IRQ_STAT_REG);
-	enable = readl_relaxed(lldev->evca + HIDMA_EVCA_IRQ_EN_REG);
-	cause = status & enable;
-
-	while (cause) {
-		if (cause & HIDMA_ERR_INT_MASK) {
-			dev_err(lldev->dev, "error 0x%x, resetting...\n",
-					cause);
-
-			/* Clear out pending interrupts */
-			writel(cause, lldev->evca + HIDMA_EVCA_IRQ_CLR_REG);
-
-			tasklet_schedule(&lldev->rst_task);
-			goto out;
-		}
-
+	while (atomic_read(&lldev->pending_tre_count)) {
 		/*
 		 * Try to consume as many EVREs as possible.
 		 */
-		hidma_handle_tre_completion(lldev);
+		hidma_handle_tre_completion(lldev, 0, 0);
+	}
 
-		/* We consumed TREs or there are pending TREs or EVREs. */
-		writel_relaxed(cause, lldev->evca + HIDMA_EVCA_IRQ_CLR_REG);
+	/* We consumed TREs or there are pending TREs or EVREs. */
+	writel_relaxed(cause, lldev->evca + HIDMA_EVCA_IRQ_CLR_REG);
+}
 
-		/*
-		 * Another interrupt might have arrived while we are
-		 * processing this one. Read the new cause.
-		 */
-		status = readl_relaxed(lldev->evca + HIDMA_EVCA_IRQ_STAT_REG);
-		enable = readl_relaxed(lldev->evca + HIDMA_EVCA_IRQ_EN_REG);
-		cause = status & enable;
-	}
+irqreturn_t hidma_ll_inthandler(int chirq, void *arg)
+{
+	struct hidma_lldev *lldev = arg;
+	u32 status;
+	u32 enable;
+	u32 cause;
 
-out:
+	status = readl_relaxed(lldev->evca + HIDMA_EVCA_IRQ_STAT_REG);
+	enable = readl_relaxed(lldev->evca + HIDMA_EVCA_IRQ_EN_REG);
+	cause = status & enable;
+	hidma_ll_int_handler_internal(lldev, cause);
 	return IRQ_HANDLED;
 }
 
-- 
1.8.2.1

[PATCH 09/10] dmaengine: qcom_hidma: add a common API to setup the interrupt

[Sinan Kaya]

Introducing the hidma_ll_setup_irq function to set up the interrupt
type externally from the OS interface.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/dma/qcom/hidma.h    |  2 ++
 drivers/dma/qcom/hidma_ll.c | 27 +++++++++++++++++++++++----
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/qcom/hidma.h b/drivers/dma/qcom/hidma.h
index 03c20f6..3f28f39 100644
--- a/drivers/dma/qcom/hidma.h
+++ b/drivers/dma/qcom/hidma.h
@@ -49,6 +49,7 @@ struct hidma_tre {
 };
 
 struct hidma_lldev {
+	bool msi_support;		/* flag indicating MSI support    */
 	bool initialized;		/* initialized flag               */
 	u8 trch_state;			/* trch_state of the device	  */
 	u8 evch_state;			/* evch_state of the device	  */
@@ -146,6 +147,7 @@ int hidma_ll_disable(struct hidma_lldev *lldev);
 int hidma_ll_enable(struct hidma_lldev *llhndl);
 void hidma_ll_set_transfer_params(struct hidma_lldev *llhndl, u32 tre_ch,
 	dma_addr_t src, dma_addr_t dest, u32 len, u32 flags);
+void hidma_ll_setup_irq(struct hidma_lldev *lldev, bool msi);
 int hidma_ll_setup(struct hidma_lldev *lldev);
 struct hidma_lldev *hidma_ll_init(struct device *dev, u32 max_channels,
 			void __iomem *trca, void __iomem *evca,
diff --git a/drivers/dma/qcom/hidma_ll.c b/drivers/dma/qcom/hidma_ll.c
index eb78952..c3d3252 100644
--- a/drivers/dma/qcom/hidma_ll.c
+++ b/drivers/dma/qcom/hidma_ll.c
@@ -669,17 +669,36 @@ int hidma_ll_setup(struct hidma_lldev *lldev)
 	writel(HIDMA_EVRE_SIZE * nr_tres,
 			lldev->evca + HIDMA_EVCA_RING_LEN_REG);
 
-	/* support IRQ only for now */
+	/* configure interrupts */
+	hidma_ll_setup_irq(lldev, lldev->msi_support);
+
+	rc = hidma_ll_enable(lldev);
+	if (rc)
+		return rc;
+
+	return rc;
+}
+
+void hidma_ll_setup_irq(struct hidma_lldev *lldev, bool msi)
+{
+	u32 val;
+
+	lldev->msi_support = msi;
+
+	/* disable interrupts again after reset */
+	writel(0, lldev->evca + HIDMA_EVCA_IRQ_CLR_REG);
+	writel(0, lldev->evca + HIDMA_EVCA_IRQ_EN_REG);
+
+	/* support IRQ by default */
 	val = readl(lldev->evca + HIDMA_EVCA_INTCTRL_REG);
 	val &= ~0xF;
-	val |= 0x1;
+	if (!lldev->msi_support)
+		val = val | 0x1;
 	writel(val, lldev->evca + HIDMA_EVCA_INTCTRL_REG);
 
 	/* clear all pending interrupts and enable them */
 	writel(ENABLE_IRQS, lldev->evca + HIDMA_EVCA_IRQ_CLR_REG);
 	writel(ENABLE_IRQS, lldev->evca + HIDMA_EVCA_IRQ_EN_REG);
-
-	return hidma_ll_enable(lldev);
 }
 
 struct hidma_lldev *hidma_ll_init(struct device *dev, u32 nr_tres,
-- 
1.8.2.1

[PATCH 10/10] dmaengine: qcom_hidma: add MSI support for interrupts

[Sinan Kaya]

The interrupts can now be delivered as platform MSI interrupts
on newer platforms. The code looks for a new OF and ACPI strings
in order to enable the functionality.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/dma/qcom/hidma.c    | 128 ++++++++++++++++++++++++++++++++++++++++++--
 drivers/dma/qcom/hidma.h    |   2 +
 drivers/dma/qcom/hidma_ll.c |   8 +++
 3 files changed, 133 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/qcom/hidma.c b/drivers/dma/qcom/hidma.c
index 41b5c6d..6a0eef2 100644
--- a/drivers/dma/qcom/hidma.c
+++ b/drivers/dma/qcom/hidma.c
@@ -56,6 +56,7 @@
 #include <linux/irq.h>
 #include <linux/atomic.h>
 #include <linux/pm_runtime.h>
+#include <linux/msi.h>
 
 #include "../dmaengine.h"
 #include "hidma.h"
@@ -70,6 +71,7 @@
 #define HIDMA_ERR_INFO_SW			0xFF
 #define HIDMA_ERR_CODE_UNEXPECTED_TERMINATE	0x0
 #define HIDMA_NR_DEFAULT_DESC			10
+#define HIDMA_MSI_INTS				11
 
 static inline struct hidma_dev *to_hidma_dev(struct dma_device *dmadev)
 {
@@ -530,6 +532,15 @@ static irqreturn_t hidma_chirq_handler(int chirq, void *arg)
 	return hidma_ll_inthandler(chirq, lldev);
 }
 
+static irqreturn_t hidma_chirq_handler_msi(int chirq, void *arg)
+{
+	struct hidma_lldev **lldevp = arg;
+	struct hidma_dev *dmadev = to_hidma_dev_from_lldev(lldevp);
+
+	return hidma_ll_inthandler_msi(chirq, *lldevp,
+				       1 << (chirq - dmadev->msi_virqbase));
+}
+
 static ssize_t hidma_show_values(struct device *dev,
 				 struct device_attribute *attr, char *buf)
 {
@@ -567,6 +578,98 @@ static int hidma_create_sysfs_entry(struct hidma_dev *dev, char *name,
 	return device_create_file(dev->ddev.dev, attrs);
 }
 
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+static void hidma_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg)
+{
+	struct device *dev = msi_desc_to_dev(desc);
+	struct hidma_dev *dmadev = dev_get_drvdata(dev);
+
+	if (!desc->platform.msi_index) {
+		writel(msg->address_lo, dmadev->dev_evca + 0x118);
+		writel(msg->address_hi, dmadev->dev_evca + 0x11C);
+		writel(msg->data, dmadev->dev_evca + 0x120);
+	}
+}
+
+static void hidma_free_msis(void *data)
+{
+	struct device *dev = data;
+
+	platform_msi_domain_free_irqs(dev);
+}
+#endif
+
+static int hidma_request_msi(struct hidma_dev *dmadev,
+			     struct platform_device *pdev)
+{
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+	int rc;
+	struct msi_desc *desc;
+	struct msi_desc *failed_desc = NULL;
+
+	rc = platform_msi_domain_alloc_irqs(&pdev->dev, HIDMA_MSI_INTS,
+					    hidma_write_msi_msg);
+	if (rc)
+		return rc;
+
+	for_each_msi_entry(desc, &pdev->dev) {
+		if (!desc->platform.msi_index)
+			dmadev->msi_virqbase = desc->irq;
+
+		rc = devm_request_irq(&pdev->dev, desc->irq,
+				       hidma_chirq_handler_msi,
+				       0, "qcom-hidma-msi",
+				       &dmadev->lldev);
+		if (rc) {
+			failed_desc = desc;
+			break;
+		}
+	}
+
+	if (rc) {
+		/* free allocated MSI interrupts above */
+		for_each_msi_entry(desc, &pdev->dev) {
+			if (desc == failed_desc)
+				break;
+			devm_free_irq(&pdev->dev, desc->irq,
+				      &dmadev->lldev);
+		}
+	} else {
+		/* Add callback to free MSIs on teardown */
+		devm_add_action(&pdev->dev, hidma_free_msis,
+				&pdev->dev);
+		hidma_ll_setup_irq(dmadev->lldev, true);
+
+	}
+	if (rc)
+		dev_warn(&pdev->dev,
+			 "failed to request MSI irq, falling back to wired IRQ\n");
+	return rc;
+#else
+	return -EINVAL;
+#endif
+}
+
+static bool hidma_msi_capable(struct device *dev)
+{
+	struct acpi_device *adev = ACPI_COMPANION(dev);
+	const char *of_compat;
+	int ret;
+
+	if (!adev || acpi_disabled) {
+		ret = device_property_read_string(dev, "compatible",
+						  &of_compat);
+		if (ret)
+			return false;
+
+		ret = strcmp(of_compat, "qcom,hidma-1.1");
+	} else {
+		ret = strcmp(acpi_device_hid(adev), "QCOM8062");
+	}
+
+	return ret == 0;
+}
+
 static int hidma_probe(struct platform_device *pdev)
 {
 	struct hidma_dev *dmadev;
@@ -576,6 +679,7 @@ static int hidma_probe(struct platform_device *pdev)
 	void __iomem *evca;
 	void __iomem *trca;
 	int rc;
+	bool msi;
 
 	pm_runtime_set_autosuspend_delay(&pdev->dev, HIDMA_AUTOSUSPEND_TIMEOUT);
 	pm_runtime_use_autosuspend(&pdev->dev);
@@ -637,6 +741,12 @@ static int hidma_probe(struct platform_device *pdev)
 	dmadev->ddev.device_terminate_all = hidma_terminate_all;
 	dmadev->ddev.copy_align = 8;
 
+	/*
+	 * Determine the MSI capability of the platform. Old HW doesn't
+	 * support MSI.
+	 */
+	msi = hidma_msi_capable(&pdev->dev);
+
 	device_property_read_u32(&pdev->dev, "desc-count",
 				 &dmadev->nr_descriptors);
 
@@ -665,10 +775,17 @@ static int hidma_probe(struct platform_device *pdev)
 		goto dmafree;
 	}
 
-	rc = devm_request_irq(&pdev->dev, chirq, hidma_chirq_handler, 0,
-			      "qcom-hidma", dmadev->lldev);
-	if (rc)
-		goto uninit;
+	platform_set_drvdata(pdev, dmadev);
+	if (msi)
+		rc = hidma_request_msi(dmadev, pdev);
+
+	if (!msi || rc) {
+		hidma_ll_setup_irq(dmadev->lldev, false);
+		rc = devm_request_irq(&pdev->dev, chirq, hidma_chirq_handler,
+				      0, "qcom-hidma", dmadev->lldev);
+		if (rc)
+			goto uninit;
+	}
 
 	INIT_LIST_HEAD(&dmadev->ddev.channels);
 	rc = hidma_chan_init(dmadev, 0);
@@ -684,7 +801,6 @@ static int hidma_probe(struct platform_device *pdev)
 	hidma_debug_init(dmadev);
 	hidma_create_sysfs_entry(dmadev, "chid", S_IRUGO);
 	dev_info(&pdev->dev, "HI-DMA engine driver registration complete\n");
-	platform_set_drvdata(pdev, dmadev);
 	pm_runtime_mark_last_busy(dmadev->ddev.dev);
 	pm_runtime_put_autosuspend(dmadev->ddev.dev);
 	return 0;
@@ -722,12 +838,14 @@ static int hidma_remove(struct platform_device *pdev)
 #if IS_ENABLED(CONFIG_ACPI)
 static const struct acpi_device_id hidma_acpi_ids[] = {
 	{"QCOM8061"},
+	{"QCOM8062"},
 	{},
 };
 #endif
 
 static const struct of_device_id hidma_match[] = {
 	{.compatible = "qcom,hidma-1.0",},
+	{.compatible = "qcom,hidma-1.1",},
 	{},
 };
 MODULE_DEVICE_TABLE(of, hidma_match);
diff --git a/drivers/dma/qcom/hidma.h b/drivers/dma/qcom/hidma.h
index 3f28f39..1c3df47 100644
--- a/drivers/dma/qcom/hidma.h
+++ b/drivers/dma/qcom/hidma.h
@@ -116,6 +116,7 @@ struct hidma_dev {
 	int				irq;
 	int				chidx;
 	u32				nr_descriptors;
+	int				msi_virqbase;
 
 	struct hidma_lldev		*lldev;
 	void				__iomem *dev_trca;
@@ -154,6 +155,7 @@ struct hidma_lldev *hidma_ll_init(struct device *dev, u32 max_channels,
 			u8 chidx);
 int hidma_ll_uninit(struct hidma_lldev *llhndl);
 irqreturn_t hidma_ll_inthandler(int irq, void *arg);
+irqreturn_t hidma_ll_inthandler_msi(int irq, void *arg, int cause);
 void hidma_cleanup_pending_tre(struct hidma_lldev *llhndl, u8 err_info,
 				u8 err_code);
 int hidma_debug_init(struct hidma_dev *dmadev);
diff --git a/drivers/dma/qcom/hidma_ll.c b/drivers/dma/qcom/hidma_ll.c
index c3d3252..0cf01bc 100644
--- a/drivers/dma/qcom/hidma_ll.c
+++ b/drivers/dma/qcom/hidma_ll.c
@@ -449,6 +449,14 @@ irqreturn_t hidma_ll_inthandler(int chirq, void *arg)
 	return IRQ_HANDLED;
 }
 
+irqreturn_t hidma_ll_inthandler_msi(int chirq, void *arg, int cause)
+{
+	struct hidma_lldev *lldev = arg;
+
+	hidma_ll_int_handler_internal(lldev, cause);
+	return IRQ_HANDLED;
+}
+
 int hidma_ll_enable(struct hidma_lldev *lldev)
 {
 	u32 val;
-- 
1.8.2.1

Re: [PATCH 10/10] dmaengine: qcom_hidma: add MSI support for interrupts

[Vinod Koul]

On Tue, Jul 19, 2016 at 12:09:37AM +0530, Sinan Kaya wrote:

> @@ -567,6 +578,98 @@ static int hidma_create_sysfs_entry(struct hidma_dev *dev, char *name,
>         return device_create_file(dev->ddev.dev, attrs);
>  }
> 
> +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN

This can be true on machine with your older HW, so I would be bit more
careful here..

> +       if (rc)
> +               dev_warn(&pdev->dev,
> +                        "failed to request MSI irq, falling back to wired IRQ\n");
> +       return rc;
> +#else
> +       return -EINVAL;

-EINVAL doesnt sound apt here..

> +       struct acpi_device *adev = ACPI_COMPANION(dev);
> +       const char *of_compat;
> +       int ret;
> +
> +       if (!adev || acpi_disabled) {
> +               ret = device_property_read_string(dev, "compatible",
> +                                                 &of_compat);
> +               if (ret)
> +                       return false;
> +
> +               ret = strcmp(of_compat, "qcom,hidma-1.1");
> +       } else {
> +               ret = strcmp(acpi_device_hid(adev), "QCOM8062");

Okay if you ahve a device ID then why do we need new binding? This device as
you said implies the support for MSI interrupts.

Thanks
-- 
~Vinod

Re: [PATCH 10/10] dmaengine: qcom_hidma: add MSI support for interrupts

[Sinan Kaya]

Hi Vinod,

On 7/24/2016 2:42 AM, Vinod Koul wrote:
> On Tue, Jul 19, 2016 at 12:09:37AM +0530, Sinan Kaya wrote:
> 
>> @@ -567,6 +578,98 @@ static int hidma_create_sysfs_entry(struct hidma_dev *dev, char *name,
>>         return device_create_file(dev->ddev.dev, attrs);
>>  }
>>
>> +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
> 
> This can be true on machine with your older HW, so I would be bit more
> careful here..

Correct, I tested the patch with new and old HW. The old HW won't attempt calling this function
since msi flag will be false.

> 
>> +       if (rc)
>> +               dev_warn(&pdev->dev,
>> +                        "failed to request MSI irq, falling back to wired IRQ\n");
>> +       return rc;
>> +#else
>> +       return -EINVAL;
> 
> -EINVAL doesnt sound apt here..

What should I use? -ENOENT ?

> 
>> +       struct acpi_device *adev = ACPI_COMPANION(dev);
>> +       const char *of_compat;
>> +       int ret;
>> +
>> +       if (!adev || acpi_disabled) {
>> +               ret = device_property_read_string(dev, "compatible",
>> +                                                 &of_compat);
>> +               if (ret)
>> +                       return false;
>> +
>> +               ret = strcmp(of_compat, "qcom,hidma-1.1");
>> +       } else {
>> +               ret = strcmp(acpi_device_hid(adev), "QCOM8062");
> 
> Okay if you ahve a device ID then why do we need new binding? This device as
> you said implies the support for MSI interrupts.


Yes, I do have a new device ID for platforms with MSI capability. 

Which new binding are you referring to?

> 
> Thanks
> 

Sinan

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH 10/10] dmaengine: qcom_hidma: add MSI support for interrupts

[Vinod Koul]

On Sun, Jul 24, 2016 at 10:38:11AM -0400, Sinan Kaya wrote:
> >> +       if (rc)
> >> +               dev_warn(&pdev->dev,
> >> +                        "failed to request MSI irq, falling back to wired IRQ\n");
> >> +       return rc;
> >> +#else
> >> +       return -EINVAL;
> > 
> > -EINVAL doesnt sound apt here..
> 
> What should I use? -ENOENT ?

that sounds okay or EIO

> >> +                       return false;
> >> +
> >> +               ret = strcmp(of_compat, "qcom,hidma-1.1");
> >> +       } else {
> >> +               ret = strcmp(acpi_device_hid(adev), "QCOM8062");
> > 
> > Okay if you ahve a device ID then why do we need new binding? This device as
> > you said implies the support for MSI interrupts.
> 
> 
> Yes, I do have a new device ID for platforms with MSI capability. 
> 
> Which new binding are you referring to?

If you have "QCOM8062" why do you need DT to tell hidma-1.1 ?

-- 
~Vinod

Re: [PATCH 10/10] dmaengine: qcom_hidma: add MSI support for interrupts

[Sinan Kaya]

On 8/4/2016 8:46 AM, Vinod Koul wrote:
>> > Yes, I do have a new device ID for platforms with MSI capability. 
>> > 
>> > Which new binding are you referring to?
> If you have "QCOM8062" why do you need DT to tell hidma-1.1 ?

Unfortunately, DT cannot do a binding with the ACPI names. Similarly, ACPI
cannot do a binding with the DT name. 

The structure of binding name is also subject to different kind of rules
for DT and ACPI.

This driver supports both device tree and ACPI. That's why, two different
names are required.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH 10/10] dmaengine: qcom_hidma: add MSI support for interrupts

[Vinod Koul]

On Thu, Aug 04, 2016 at 09:59:43AM -0400, Sinan Kaya wrote:
> On 8/4/2016 8:46 AM, Vinod Koul wrote:
> >> > Yes, I do have a new device ID for platforms with MSI capability. 
> >> > 
> >> > Which new binding are you referring to?
> > If you have "QCOM8062" why do you need DT to tell hidma-1.1 ?
> 
> Unfortunately, DT cannot do a binding with the ACPI names. Similarly, ACPI
> cannot do a binding with the DT name. 
> 
> The structure of binding name is also subject to different kind of rules
> for DT and ACPI.
> 
> This driver supports both device tree and ACPI. That's why, two different
> names are required.

Hmmm, wasn't the who get_property stuff supposed to make properties work on
both ACPi & DT. I am not sure though about the current state of affairs on
that.

-- 
~Vinod

Re: [PATCH 10/10] dmaengine: qcom_hidma: add MSI support for interrupts

[okaya]

On 2016-08-08 04:14, Vinod Koul wrote:
> On Thu, Aug 04, 2016 at 09:59:43AM -0400, Sinan Kaya wrote:
>> On 8/4/2016 8:46 AM, Vinod Koul wrote:
>> >> > Yes, I do have a new device ID for platforms with MSI capability.
>> >> >
>> >> > Which new binding are you referring to?
>> > If you have "QCOM8062" why do you need DT to tell hidma-1.1 ?
>> 
>> Unfortunately, DT cannot do a binding with the ACPI names. Similarly, 
>> ACPI
>> cannot do a binding with the DT name.
>> 
>> The structure of binding name is also subject to different kind of 
>> rules
>> for DT and ACPI.
>> 
>> This driver supports both device tree and ACPI. That's why, two 
>> different
>> names are required.
> 
> Hmmm, wasn't the who get_property stuff supposed to make properties 
> work on
> both ACPi & DT. I am not sure though about the current state of affairs 
> on
> that.


Get property works. It is able to abstract device driver properties. A 
driver doesn't need to know whether it is coming from acpi dsd or of.

However, no such mechanism exists for driver names due to nature of 
different naming requirements.

Of has its own match table and acpi has its own. Acpi also has 
ridiculous 8 character name limitation.