Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V5 10/10] dmaengine: qcom_hidma: add MSI support for interrupts
From: Sinan Kaya @ 2016-10-07  5:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1475817915-11976-1-git-send-email-okaya@codeaurora.org>

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    | 143 ++++++++++++++++++++++++++++++++++++++++++--
 drivers/dma/qcom/hidma.h    |   2 +
 drivers/dma/qcom/hidma_ll.c |   8 +++
 3 files changed, 147 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/qcom/hidma.c b/drivers/dma/qcom/hidma.c
index 10a9e3a..7b13213 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)
 {
@@ -584,6 +595,104 @@ static int hidma_sysfs_init(struct hidma_dev *dev)
 	return device_create_file(dev->ddev.dev, dev->chid_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);
+	}
+}
+#endif
+
+static void hidma_free_msis(struct hidma_dev *dmadev)
+{
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+	struct device *dev = dmadev->ddev.dev;
+	struct msi_desc *desc;
+
+	/* free allocated MSI interrupts above */
+	for_each_msi_entry(desc, dev)
+		devm_free_irq(dev, desc->irq, &dmadev->lldev);
+
+	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 */
+		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 = -EINVAL;
+
+	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 {
+#ifdef CONFIG_ACPI
+		ret = strcmp(acpi_device_hid(adev), "QCOM8062");
+#endif
+	}
+	return ret == 0;
+}
+
 static int hidma_probe(struct platform_device *pdev)
 {
 	struct hidma_dev *dmadev;
@@ -593,6 +702,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);
@@ -654,6 +764,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);
 
@@ -682,10 +798,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);
@@ -701,12 +824,14 @@ static int hidma_probe(struct platform_device *pdev)
 	hidma_debug_init(dmadev);
 	hidma_sysfs_init(dmadev);
 	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;
 
 uninit:
+	if (msi)
+		hidma_free_msis(dmadev);
+
 	hidma_debug_uninit(dmadev);
 	hidma_ll_uninit(dmadev->lldev);
 dmafree:
@@ -724,7 +849,11 @@ static int hidma_remove(struct platform_device *pdev)
 
 	pm_runtime_get_sync(dmadev->ddev.dev);
 	dma_async_device_unregister(&dmadev->ddev);
-	devm_free_irq(dmadev->ddev.dev, dmadev->irq, dmadev->lldev);
+	if (!dmadev->lldev->msi_support)
+		devm_free_irq(dmadev->ddev.dev, dmadev->irq, dmadev->lldev);
+	else
+		hidma_free_msis(dmadev);
+
 	tasklet_kill(&dmadev->task);
 	hidma_sysfs_uninit(dmadev);
 	hidma_debug_uninit(dmadev);
@@ -741,12 +870,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 b74a56e..b6a9028 100644
--- a/drivers/dma/qcom/hidma.h
+++ b/drivers/dma/qcom/hidma.h
@@ -115,6 +115,7 @@ struct hidma_dev {
 	int				irq;
 	int				chidx;
 	u32				nr_descriptors;
+	int				msi_virqbase;
 
 	struct hidma_lldev		*lldev;
 	void				__iomem *dev_trca;
@@ -156,6 +157,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 aa76ec1..40bcf31 100644
--- a/drivers/dma/qcom/hidma_ll.c
+++ b/drivers/dma/qcom/hidma_ll.c
@@ -478,6 +478,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.9.1

^ permalink raw reply related

* [PATCH V5 09/10] dmaengine: qcom_hidma: break completion processing on error
From: Sinan Kaya @ 2016-10-07  5:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1475817915-11976-1-git-send-email-okaya@codeaurora.org>

We try to consume as much successful transfers as possible. Now that we
support MSI interrupts, an error interrupt might be observed by another
processor while we are finishing the successful ones.

Try to abort successful processing if this is the case.

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

diff --git a/drivers/dma/qcom/hidma_ll.c b/drivers/dma/qcom/hidma_ll.c
index c4e8b64..aa76ec1 100644
--- a/drivers/dma/qcom/hidma_ll.c
+++ b/drivers/dma/qcom/hidma_ll.c
@@ -291,6 +291,13 @@ static int hidma_handle_tre_completion(struct hidma_lldev *lldev)
 		evre_write_off =
 		    readl_relaxed(lldev->evca + HIDMA_EVCA_WRITE_PTR_REG);
 		num_completed++;
+
+		/*
+		 * An error interrupt might have arrived while we are processing
+		 * the completed interrupt.
+		 */
+		if (!hidma_ll_isenabled(lldev))
+			break;
 	}
 
 	if (num_completed) {
-- 
1.9.1

^ permalink raw reply related

* [PATCH V5 08/10] dmaengine: qcom_hidma: protect common data structures
From: Sinan Kaya @ 2016-10-07  5:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1475817915-11976-1-git-send-email-okaya@codeaurora.org>

When MSI interrupts are supported, error and the transfer interrupt can
come from multiple processor contexts.

Each error interrupt is an MSI interrupt. If the channel is disabled by
the first error interrupt, the remaining error interrupts will gracefully
return in the interrupt handler.

If an error is observed while servicing the completions in success case,
the posting of the completions will be aborted as soon as channel disabled
state is observed. The error interrupt handler will take it from there and
finish the remaining completions. We don't want to create multiple success
and error messages to be delivered to the client in mixed order.

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

diff --git a/drivers/dma/qcom/hidma_ll.c b/drivers/dma/qcom/hidma_ll.c
index 9d78c86..c4e8b64 100644
--- a/drivers/dma/qcom/hidma_ll.c
+++ b/drivers/dma/qcom/hidma_ll.c
@@ -198,13 +198,16 @@ static void hidma_ll_tre_complete(unsigned long arg)
 	}
 }
 
-static int hidma_post_completed(struct hidma_lldev *lldev, int tre_iterator,
-				u8 err_info, u8 err_code)
+static int hidma_post_completed(struct hidma_lldev *lldev, u8 err_info,
+				u8 err_code)
 {
 	struct hidma_tre *tre;
 	unsigned long flags;
+	u32 tre_iterator;
 
 	spin_lock_irqsave(&lldev->lock, flags);
+
+	tre_iterator = lldev->tre_processed_off;
 	tre = lldev->pending_tre_list[tre_iterator / HIDMA_TRE_SIZE];
 	if (!tre) {
 		spin_unlock_irqrestore(&lldev->lock, flags);
@@ -223,6 +226,9 @@ static int hidma_post_completed(struct hidma_lldev *lldev, int tre_iterator,
 		atomic_set(&lldev->pending_tre_count, 0);
 	}
 
+	HIDMA_INCREMENT_ITERATOR(tre_iterator, HIDMA_TRE_SIZE,
+				 lldev->tre_ring_size);
+	lldev->tre_processed_off = tre_iterator;
 	spin_unlock_irqrestore(&lldev->lock, flags);
 
 	tre->err_info = err_info;
@@ -244,13 +250,11 @@ static int hidma_post_completed(struct hidma_lldev *lldev, int tre_iterator,
 static int hidma_handle_tre_completion(struct hidma_lldev *lldev)
 {
 	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 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;
 
 	if ((evre_write_off > evre_ring_size) ||
@@ -273,12 +277,9 @@ static int hidma_handle_tre_completion(struct hidma_lldev *lldev)
 		err_code =
 		    (cfg >> HIDMA_EVRE_CODE_BIT_POS) & HIDMA_EVRE_CODE_MASK;
 
-		if (hidma_post_completed(lldev, tre_iterator, err_info,
-					 err_code))
+		if (hidma_post_completed(lldev, 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);
 
@@ -295,16 +296,10 @@ static int hidma_handle_tre_completion(struct hidma_lldev *lldev)
 	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;
 	}
 
@@ -314,27 +309,10 @@ 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))
+		if (hidma_post_completed(lldev, 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;
 }
 
 static int hidma_ll_reset(struct hidma_lldev *lldev)
-- 
1.9.1

^ permalink raw reply related

* [PATCH V5 07/10] dmaengine: qcom_hidma: add a common API to setup the interrupt
From: Sinan Kaya @ 2016-10-07  5:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1475817915-11976-1-git-send-email-okaya@codeaurora.org>

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 afaeb9a..b74a56e 100644
--- a/drivers/dma/qcom/hidma.h
+++ b/drivers/dma/qcom/hidma.h
@@ -46,6 +46,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	  */
@@ -148,6 +149,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 015df4b..9d78c86 100644
--- a/drivers/dma/qcom/hidma_ll.c
+++ b/drivers/dma/qcom/hidma_ll.c
@@ -715,17 +715,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.9.1

^ permalink raw reply related

* [PATCH V5 06/10] dmaengine: qcom_hidma: bring out interrupt cause
From: Sinan Kaya @ 2016-10-07  5:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1475817915-11976-1-git-send-email-okaya@codeaurora.org>

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 | 57 ++++++++++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 24 deletions(-)

diff --git a/drivers/dma/qcom/hidma_ll.c b/drivers/dma/qcom/hidma_ll.c
index a4fc941..015df4b 100644
--- a/drivers/dma/qcom/hidma_ll.c
+++ b/drivers/dma/qcom/hidma_ll.c
@@ -432,12 +432,24 @@ 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, disabling...\n",
+				cause);
+
+		/* Clear out pending interrupts */
+		writel(cause, lldev->evca + HIDMA_EVCA_IRQ_CLR_REG);
+
+		/* No further submissions. */
+		hidma_ll_disable(lldev);
+
+		/* Driver completes the txn and intimates the client.*/
+		hidma_cleanup_pending_tre(lldev, 0xFF,
+					  HIDMA_EVRE_STATUS_ERROR);
+
+		return;
+	}
 
 	/*
 	 * Fine tuned for this HW...
@@ -446,30 +458,28 @@ irqreturn_t hidma_ll_inthandler(int chirq, void *arg)
 	 * read and write accessors are used for performance reasons due to
 	 * interrupt delivery guarantees. Do not copy this code blindly and
 	 * expect that to work.
+	 *
+	 * Try to consume as many EVREs as possible.
 	 */
+	hidma_handle_tre_completion(lldev);
+
+	/* We consumed TREs or there are pending TREs or EVREs. */
+	writel_relaxed(cause, lldev->evca + HIDMA_EVCA_IRQ_CLR_REG);
+}
+
+irqreturn_t hidma_ll_inthandler(int chirq, void *arg)
+{
+	struct hidma_lldev *lldev = arg;
+	u32 status;
+	u32 enable;
+	u32 cause;
+
 	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;
-		}
-
-		/*
-		 * Try to consume as many EVREs as possible.
-		 */
-		hidma_handle_tre_completion(lldev);
-
-		/* We consumed TREs or there are pending TREs or EVREs. */
-		writel_relaxed(cause, lldev->evca + HIDMA_EVCA_IRQ_CLR_REG);
+		hidma_ll_int_handler_internal(lldev, cause);
 
 		/*
 		 * Another interrupt might have arrived while we are
@@ -480,7 +490,6 @@ irqreturn_t hidma_ll_inthandler(int chirq, void *arg)
 		cause = status & enable;
 	}
 
-out:
 	return IRQ_HANDLED;
 }
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH V5 05/10] dmaengine: qcom_hidma: make pending_tre_count atomic
From: Sinan Kaya @ 2016-10-07  5:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1475817915-11976-1-git-send-email-okaya@codeaurora.org>

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

The driver will allocate 11 MSI interrupts. Each MSI interrupt can be
assigned to a different CPU. Then, we have a race condition for common
variables as they share the same interrupt handler with a different
cause bit and they can potentially be executed in parallel. Making this
variable atomic so that it can be updated from multiple processor
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 b209942..afaeb9a 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 3d83b99..3bdcb80 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 ad20dfb..a4fc941 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;
@@ -834,7 +833,7 @@ int hidma_ll_uninit(struct hidma_lldev *lldev)
 	tasklet_kill(&lldev->rst_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.9.1

^ permalink raw reply related

* [PATCH V5 04/10] dmaengine: qcom_hidma: configure DMA and MSI for OF
From: Sinan Kaya @ 2016-10-07  5:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1475817915-11976-1-git-send-email-okaya@codeaurora.org>

Configure the DMA bindings for the device tree based firmware.

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

diff --git a/drivers/dma/qcom/hidma_mgmt.c b/drivers/dma/qcom/hidma_mgmt.c
index 82f36e4..185d29c 100644
--- a/drivers/dma/qcom/hidma_mgmt.c
+++ b/drivers/dma/qcom/hidma_mgmt.c
@@ -375,8 +375,15 @@ static int __init hidma_mgmt_of_populate_channels(struct device_node *np)
 			ret = PTR_ERR(new_pdev);
 			goto out;
 		}
+		of_node_get(child);
+		new_pdev->dev.of_node = child;
 		of_dma_configure(&new_pdev->dev, child);
-
+		/*
+		 * It is assumed that calling of_msi_configure is safe on
+		 * platforms with or without MSI support.
+		 */
+		of_msi_configure(&new_pdev->dev, child);
+		of_node_put(child);
 		kfree(res);
 		res = NULL;
 	}
-- 
1.9.1

^ permalink raw reply related

* [PATCH V5 03/10] of: irq: make of_msi_configure accessible from modules
From: Sinan Kaya @ 2016-10-07  5:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1475817915-11976-1-git-send-email-okaya@codeaurora.org>

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.

Acked-by: Rob Herring <robh@kernel.org>
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 a2e68f7..20c09e0 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -767,3 +767,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.9.1

^ permalink raw reply related

* [PATCH V5 02/10] Documentation: DT: qcom_hidma: correct spelling mistakes
From: Sinan Kaya @ 2016-10-07  5:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1475817915-11976-1-git-send-email-okaya@codeaurora.org>

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

Acked-by: Rob Herring <robh@kernel.org>
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 2c5e4b8..55492c2 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.9.1

^ permalink raw reply related

* [PATCH V5 01/10] Documentation: DT: qcom_hidma: update binding for MSI
From: Sinan Kaya @ 2016-10-07  5:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1475817915-11976-1-git-send-email-okaya@codeaurora.org>

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 | 8 +++++++-
 1 file changed, 7 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..2c5e4b8 100644
--- a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
+++ b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
@@ -47,12 +47,18 @@ 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
 - 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:
-- 
1.9.1

^ permalink raw reply related

* [PATCH V5 00/10] dmaengine: qcom_hidma: add MSI interrupt support
From: Sinan Kaya @ 2016-10-07  5:25 UTC (permalink / raw)
  To: linux-arm-kernel

The new version of the HW supports MSI interrupts instead of wired
interrupts. The MSI interrupts are especially useful for the guest machine
execution. The wired interrupts usually trap to the hypervisor and then are
relayed to the actual interrupt.

The MSI interrupts can be directly fed into the interrupt controller.

Adding a new OF compat string (qcom,hidma-1.1) and ACPI string (QCOM8062)
to distinguish newer HW from the older ones.

v5:
* dmaengine: qcom_hidma: add MSI support for interrupts
** Return MSI interrupts before calling platform_msi_domain_free_irqs.
Also cleanup MSI interrupts on the error path.
** Free the legacy IRQ only if MSI is disabled
* add dmaengine: qcom_hidma: break completion processing on error
in order to break the completions if an error is observed while servicing
completed work.
* drop dmaengine: qcom_hidma: make error and success path common
as the success path assumes that we'll get the number of notifications for
the
jobs queued. This is not true under error conditions.
* simplify dmaengine: qcom_hidma: protect common data structures. We just
need to protect the TRE processed offset. It is the variable that keeps
track
of outstanding requests.
* document assumptions about of_msi_configure routine call.

v4:
http://www.spinics.net/lists/devicetree/msg144563.html
* device tree binding update to refer to msi.txt

v3:
* day 0 fix for when ACPI is not compiled in
* https://www.spinics.net/lists/arm-kernel/msg532179.html

v2:
https://patchwork.kernel.org/patch/9326399/
* Documentation update for DT bindings
* Rebased to slave-next
* Dropped dmaengine: qcom_hidma: eliminate processed variables. Replaced it
  with dmaengine: qcom_hidma: protect common data structures

v1:
http://lists.infradead.org/pipermail/linux-arm-kernel/2016-July/444167.html
* initial implementation

Sinan Kaya (10):
  Documentation: DT: qcom_hidma: update binding for MSI
  Documentation: DT: qcom_hidma: correct spelling mistakes
  of: irq: make of_msi_configure accessible from modules
  dmaengine: qcom_hidma: configure DMA and MSI for OF
  dmaengine: qcom_hidma: make pending_tre_count atomic
  dmaengine: qcom_hidma: bring out interrupt cause
  dmaengine: qcom_hidma: add a common API to setup the interrupt
  dmaengine: qcom_hidma: protect common data structures
  dmaengine: qcom_hidma: break completion processing on error
  dmaengine: qcom_hidma: add MSI support for interrupts

 .../devicetree/bindings/dma/qcom_hidma_mgmt.txt    |  12 +-
 drivers/dma/qcom/hidma.c                           | 143 ++++++++++++++++++-
 drivers/dma/qcom/hidma.h                           |   6 +-
 drivers/dma/qcom/hidma_dbg.c                       |   3 +-
 drivers/dma/qcom/hidma_ll.c                        | 156 ++++++++++++---------
 drivers/dma/qcom/hidma_mgmt.c                      |   9 +-
 drivers/of/irq.c                                   |   1 +
 7 files changed, 250 insertions(+), 80 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support
From: Archit Taneja @ 2016-10-07  4:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1757833.hOePAQC2sg@avalon>



On 10/07/2016 02:34 AM, Laurent Pinchart wrote:
> Hi Sean,
>
> On Thursday 06 Oct 2016 15:53:28 Sean Paul wrote:
>> On Thu, Oct 6, 2016 at 1:27 PM, Laurent Pinchart wrote:
>>> On Thursday 06 Oct 2016 17:09:57 Archit Taneja wrote:
>>>> On 10/06/2016 12:51 PM, Maxime Ripard wrote:
>>>>> On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote:
>>>>>> On 09/30/2016 08:07 PM, Maxime Ripard wrote:
>>>>>>> Some boards have an entirely passive RGB to VGA bridge, based on
>>>>>>> either DACs or resistor ladders.
>>>>>>>
>>>>>>> Those might or might not have an i2c bus routed to the VGA connector
>>>>>>> in order to access the screen EDIDs.
>>>>>>>
>>>>>>> Add a bridge that doesn't do anything but expose the modes available
>>>>>>> on the screen, either based on the EDIDs if available, or based on
>>>>>>> the XGA standards.
>>>>>>>
>>>>>>> Acked-by: Rob Herring <robh@kernel.org>
>>>>>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>>>>>>> ---
>>>>>>> .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
>>>>>>> drivers/gpu/drm/bridge/Kconfig                     |   7 +
>>>>>>> drivers/gpu/drm/bridge/Makefile                    |   1 +
>>>>>>> drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++
>>>>>>> 4 files changed, 285 insertions(+)
>>>>>>> create mode 100644
>>>>>>> Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
>>>>>>> t
>>>>>>> create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
>>>>>>>
>>>>>>> diff --git
>>>>>>> a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
>>>>>>> txt
>>>>>>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
>>>>>>> txt
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..a8375bc1f9cb
>>>>>>> --- /dev/null
>>>>>>> +++
>>>>>>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
>>>>>>> tx
>>>>>>> t @@ -0,0 +1,48 @@
>>>>>>> +Dumb RGB to VGA bridge
>>>>>>> +----------------------
>>>>>>> +
>>>>>>> +This binding is aimed for dumb RGB to VGA bridges that do not
>>>>>>> require
>>>>>>> +any configuration.
>>>>>>> +
>>>>>>> +Required properties:
>>>>>>> +
>>>>>>> +- compatible: Must be "rgb-to-vga-bridge"
>>>>>>
>>>>>> I'd talked to Laurent on IRC if he's okay with this. And I guess you
>>>>>> to had discussed it during XDC too. He's suggested that it'd be better
>>>>>> to have the compatible string as "simple-vga-dac".
>>>>>
>>>>> I just wished this bikeshedding had taken place publicly and be
>>>>> actually part of that discussion, but yeah, ok.
>>>>
>>>> Sorry about that. I'd pinged him for an Ack, the discussion went
>>>> more than that :)
>>>>
>>>>>> Some of the reasons behind having this:
>>>>>>
>>>>>> - We don't need to specify "rgb" in the compatible string since most
>>>>>> simple VGA DACs can only work with an RGB input.
>>>>>
>>>>> Ok.
>>>>>
>>>>>> - Also, with "dac" specified in the string, we don't need to
>>>>>> specifically mention "bridge" in the string. Also, bridge is a drm
>>>>>> specific term.
>>>>>>
>>>>>> - "simple" is considered because it's an unconfigurable bridge, and it
>>>>>> might be misleading for other VGA DACs to not use "vga-dac".
>>>>>
>>>>> All those "simple" bindings are just the biggest lie we ever
>>>>> told. It's simple when you introduce it, and then grows into something
>>>>> much more complicated than a non-simple implementation.
>>>>
>>>> "simple" here is supposed to mean that it's an unconfigurable RGB to
>>>> VGA DAC. This isn't supposed to follow the simple-panel model, where
>>>> you add the "simple-panel" string in the compatible node, along with
>>>> you chip specific compatible string.
>>>
>>> I agree with Maxime, I don't like the word "simple". My preference would
>>> be "vga-dac" for a lack of a better qualifier than "simple" to describe
>>> the fact that the device requires no configuration. My only concern with
>>> "vga-dac" is that we would restrict usage of that compatible string for a
>>> subset of VGA DACs, with more complex devices not being compatible with
>>> "vga-dac" even though they are VGA DACs. That's a problem I can live with
>>> though.
>>
>> While we're bikeshedding (feel free to ignore my input on this), I
>> think Maxime's initial "dumb" qualifier was better than "simple".
>
> I think I agree.
>
>> I think "passive" also gets the point across better than "simple", which
>> we've already established as something else in drm.
>
> To my electrical engineer's ear, passive refers to a component or combination
> of components that is not capable of power gain. The resistors ladder VGA DAC
> that Maxime is trying to support is a passive system, but the ADV7123 VGA DAC
> that equally requires no configuration is an active component.

If no one has any more objections within the next day, I'll pull in
Maxime's v5 RGB to VGA bridge driver, and change the compatible to
"dumb-vga-dac".

Thanks,
Archit


>
>> Now that I've gotten that out of the way, this patch looks good to me
>> regardless of the name.
>>
>> Reviewed-by: Sean Paul <seanpaul@chromium.org>
>>
>> Sean
>>
>>>> In other words, this driver shouldn't be touched again in the future :)
>>>> If someone wants to write a RGB to VGA driver which is even
>>>> slightly configurable, they'll need to write a new bridge driver.
>>>
>>> I'm sure that won't be true. I can certainly foresee the addition of
>>> regulators support for instance. It's unfortunately never black and white.
>>>
>>>>>> What do you think about this? If you think it's good, would it be
>>>>>> possible for you to change this? I guess it's okay for the rest of
>>>>>> the patch to stay the same.
>>>>>
>>>>> I'll update and respin the serie.
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* [PATCH v2 5/6] arm: dts: imx7-colibri: Use pwm polarity control
From: Bhuvanchandra DV @ 2016-10-07  4:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161006084007.44c5cbae@jawa>

On 10/06/16 12:10, Lukasz Majewski wrote:

> On Sat, 1 Oct 2016 15:42:34 +0530
> Bhuvanchandra DV <bhuvanchandra.dv@toradex.com> wrote:
>
>> Configure PWM polarity control.
>>
>> Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
>> ---
>>   arch/arm/boot/dts/imx7-colibri.dtsi | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/imx7-colibri.dtsi
>> b/arch/arm/boot/dts/imx7-colibri.dtsi index a9cc657..2af5e3e 100644
>> --- a/arch/arm/boot/dts/imx7-colibri.dtsi
>> +++ b/arch/arm/boot/dts/imx7-colibri.dtsi
>> @@ -43,7 +43,7 @@
>>   / {
>>   	bl: backlight {
>>   		compatible = "pwm-backlight";
>> -		pwms = <&pwm1 0 5000000>;
>> +		pwms = <&pwm1 0 5000000 0>;
> My recommendation would be to add:
> #include <dt-bindings/pwm/pwm.h>
>
> and then define pwms as:
>
> pwms = <&pwm1 0 5000000 PWM_POLARITY_NORMAL>;
>
> It would be more readable

Ok, will add that.

--
Bhuvan

>
> Best regards,
> ?ukasz Majewski
>
>>   	};
>>   
>>   	reg_module_3v3: regulator-module-3v3 {

^ permalink raw reply

* [PATCH v2 2/6] pwm: core: make the PWM_POLARITY flag in DTB optional
From: Bhuvanchandra DV @ 2016-10-07  4:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161006083642.6a28a629@jawa>

Hi Lukasz,

On 10/06/16 12:06, Lukasz Majewski wrote:

> Hi Bhuvanchandra,
>
>> From: Lothar Wassmann <LW@KARO-electronics.de>
>>
>> Change the pwm chip driver registration, so that a chip driver that
>> supports polarity inversion can still be used with DTBs that don't
>> provide the 'PWM_POLARITY' flag.
>>
>> This is done to provide polarity inversion support for the pwm-imx
>> driver without having to modify all existing DTS files.
>>
>> Signed-off-by: Lothar Wassmann <LW@KARO-electronics.de>
>> Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
>> Suggested-by: Thierry Reding <thierry.reding@gmail.com>
>> ---
>>   drivers/pwm/core.c | 27 ++++++++++++++++-----------
>>   1 file changed, 16 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
>> index 195373e..aae8db3 100644
>> --- a/drivers/pwm/core.c
>> +++ b/drivers/pwm/core.c
>> @@ -137,9 +137,14 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc,
>> const struct of_phandle_args *args) {
>>   	struct pwm_device *pwm;
>>   
>> +	/* check, whether the driver supports a third cell for flags
>> */ if (pc->of_pwm_n_cells < 3)
>>   		return ERR_PTR(-EINVAL);
>>   
>> +	/* flags in the third cell are optional */
>> +	if (args->args_count < 2)
>> +		return ERR_PTR(-EINVAL);
>> +
>>   	if (args->args[0] >= pc->npwm)
>>   		return ERR_PTR(-EINVAL);
>>   
>> @@ -149,10 +154,12 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc,
>> const struct of_phandle_args *args)
>>   	pwm->args.period = args->args[1];
>>   
>> -	if (args->args[2] & PWM_POLARITY_INVERTED)
>> -		pwm->args.polarity = PWM_POLARITY_INVERSED;
>> -	else
>> -		pwm->args.polarity = PWM_POLARITY_NORMAL;
>> +	if (args->args_count > 2) {
>> +		if (args->args[2] & PWM_POLARITY_INVERTED)
>> +			pwm_set_polarity(pwm, PWM_POLARITY_INVERSED);
> 			^^^^^^^^^^^^^^^^
> 			here we should set pwm->args.polarity, since
> 			the pwm_set_polarity() calls pwm_apply_state()
> 			which requires duty_cycle and period to be set.
>
> 			In this particular moment it is not yet set and
> 			polarity is not properly configured.

Agreed. Will do a clean v3 patchset along with the patch you sent
(pwm: core: Use pwm->args.polarity to setup PWM_POLARITY_INVERSED).

--
Bhuvan

>
> Patch fixing this will be sent as a reply to this e-mail. Please just
> squash it and test on your platform.
>
> Best regards,
> ?ukasz Majewski
>
>> +		else
>> +			pwm_set_polarity(pwm, PWM_POLARITY_NORMAL);
>> +	}
>>   
>>   	return pwm;
>>   }
>> @@ -163,9 +170,14 @@ of_pwm_simple_xlate(struct pwm_chip *pc, const
>> struct of_phandle_args *args) {
>>   	struct pwm_device *pwm;
>>   
>> +	/* sanity check driver support */
>>   	if (pc->of_pwm_n_cells < 2)
>>   		return ERR_PTR(-EINVAL);
>>   
>> +	/* all cells are required */
>> +	if (args->args_count != pc->of_pwm_n_cells)
>> +		return ERR_PTR(-EINVAL);
>> +
>>   	if (args->args[0] >= pc->npwm)
>>   		return ERR_PTR(-EINVAL);
>>   
>> @@ -672,13 +684,6 @@ struct pwm_device *of_pwm_get(struct device_node
>> *np, const char *con_id) goto put;
>>   	}
>>   
>> -	if (args.args_count != pc->of_pwm_n_cells) {
>> -		pr_debug("%s: wrong #pwm-cells for %s\n",
>> np->full_name,
>> -			 args.np->full_name);
>> -		pwm = ERR_PTR(-EINVAL);
>> -		goto put;
>> -	}
>> -
>>   	pwm = pc->of_xlate(pc, &args);
>>   	if (IS_ERR(pwm))
>>   		goto put;

^ permalink raw reply

* [PATCH] ARM: dts: r8a7794: Fix W=1 dtc warnings
From: Simon Horman @ 2016-10-07  3:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1475587908-22151-1-git-send-email-geert+renesas@glider.be>

On Tue, Oct 04, 2016 at 03:31:48PM +0200, Geert Uytterhoeven wrote:
> Warning (unit_address_vs_reg): Node /sound at ec500000/rcar_sound,dvc/dvc at 0 has a unit name, but no reg property

...

> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  arch/arm/boot/dts/r8a7794.dtsi | 58 +++++++++++++++++++++---------------------
>  1 file changed, 29 insertions(+), 29 deletions(-)

Thanks, I have queued this up.

^ permalink raw reply

* [PATCH 0/2] ARM: shmobile: alt/gose: Add board part number to DT bindings
From: Simon Horman @ 2016-10-07  3:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1475587248-13670-1-git-send-email-geert+renesas@glider.be>

On Tue, Oct 04, 2016 at 03:20:46PM +0200, Geert Uytterhoeven wrote:
> 	Hi Simon, Magnus,
> 
> This series at the missing board part numbers for r8a7794/alt and
> r8a7793/gose to the DT binding documentation, like is done for the other
> boards.
> 
> Thanks for applying!
> 
> Geert Uytterhoeven (2):
>   ARM: shmobile: r8a7794/alt: Add board part number to DT bindings
>   ARM: shmobile: r8a7793/gose: Add board part number to DT bindings

Thanks, I have queued these up.

^ permalink raw reply

* [PATCH v3 03/11] ARM: shmobile: r8a7743: basic SoC support
From: Simon Horman @ 2016-10-07  3:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <15557770.lH5aPepOeZ@wasted.cogentembedded.com>

On Thu, Oct 06, 2016 at 12:37:08AM +0300, Sergei Shtylyov wrote:
> Add minimal support for the RZ/G1M (R8A7743) SoC.
> 
> Based on the original (and large) patch by Dmitry Shifrin
> <dmitry.shifrin@cogentembedded.com>.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks, I have queued this up.

^ permalink raw reply

* [PATCH v3 04/11] ARM: dts: r8a7743: initial SoC device tree
From: Simon Horman @ 2016-10-07  3:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2883940.Oo8sg10L7m@wasted.cogentembedded.com>

Hi Sergei,

On Thu, Oct 06, 2016 at 12:38:18AM +0300, Sergei Shtylyov wrote:
> The  initial R8A7743 SoC device tree including CPU cores, GIC, timer, SYSC,
> CPG, and the required clock descriptions.
> 
> Based on the original (and large) patch by Dmitry Shifrin
> <dmitry.shifrin@cogentembedded.com>.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

I notice that this patch enables two CPUs. Have you tested SMP and:
- CPU hotplug
- Suspend to RAM

^ permalink raw reply

* [PATCH 2/3] doc: dt: add cyclone-spi binding document
From: Moritz Fischer @ 2016-10-07  2:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <8d89e70475e8e1c3ce5117a0367c8444c11c61e3.1475783742.git.stillcompiling@gmail.com>

Hi Joshua,

On Thu, Oct 6, 2016 at 1:34 PM, Joshua Clayton <stillcompiling@gmail.com> wrote:
> Describe a cyclonespi devicetree entry, required features
>
> Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
> ---
>  .../bindings/fpga/cyclone-spi-fpga-mgr.txt         | 23 ++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/fpga/cyclone-spi-fpga-mgr.txt
>
> diff --git a/Documentation/devicetree/bindings/fpga/cyclone-spi-fpga-mgr.txt b/Documentation/devicetree/bindings/fpga/cyclone-spi-fpga-mgr.txt
> new file mode 100644
> index 0000000..8de34db
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fpga/cyclone-spi-fpga-mgr.txt
> @@ -0,0 +1,23 @@
> +Altera SOCFPGA FPGA Manager

Copy & Paste? :)

> +Altera cyclone FPGAs support a method of loading the bitstream over what is

cyclone->Cyclone

> +referred to as "passive serial".
> +The passive serial link is not technically spi, and might require extra
> +circuits in order to play nicely with other spi slaves on the same bus.
> +
> +See https://www.altera.com/literature/hb/cyc/cyc_c51013.pdf
> +
> +Required properties:
> +- compatible  : should contain "altr,cyclonespi-fpga-mgr"

Alan, do you guys have any input on the compat string?

I think generally the bindings should go before the actual usage in
your patch series. Meaning you wanna document the binding
before you use it. I think this patch should be [1/3].

Cheers,

Moritz

^ permalink raw reply

* [PATCH 1/3] fpga manager: Add cyclonespi driver for Altera fpgas
From: Moritz Fischer @ 2016-10-07  2:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4b4432c04b4ea92a2af814e3d7866c33f2eb12ea.1475783742.git.stillcompiling@gmail.com>

Hi Joshua,

couple of nits inline below:

On Thu, Oct 6, 2016 at 1:34 PM, Joshua Clayton <stillcompiling@gmail.com> wrote:

> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index cd84934..ccad5b1 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -13,6 +13,12 @@ config FPGA
>
>  if FPGA
>
> +config FPGA_MGR_CYCLONE_SPI
> +       tristate "Altera Cyclone V SPI"
> +       depends on SPI
> +       help
> +         FPGA manager driver support for Altera Cyclone V over SPI
> +
>  config FPGA_MGR_SOCFPGA
>         tristate "Altera SOCFPGA FPGA Manager"
>         depends on ARCH_SOCFPGA
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 8d83fc6..c03f40de 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -6,5 +6,6 @@
>  obj-$(CONFIG_FPGA)                     += fpga-mgr.o
>
>  # FPGA Manager Drivers
> +obj-$(CONFIG_FPGA_MGR_CYCLONE_SPI)     += cyclonespi.o
>  obj-$(CONFIG_FPGA_MGR_SOCFPGA)         += socfpga.o
>  obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)       += zynq-fpga.o
> diff --git a/drivers/fpga/cyclonespi.c b/drivers/fpga/cyclonespi.c
> new file mode 100644
> index 0000000..1ffa67c
> --- /dev/null
> +++ b/drivers/fpga/cyclonespi.c
> @@ -0,0 +1,173 @@
> +/**
> + * Copyright (c) 2015 United Western Technologies, Corporation
> + *
> + * Joshua Clayton <stillcompiling@gmail.com>
> + *
> + * Manage Altera fpga firmware that is loaded over spi.
> + * Firmware must be in binary "rbf" format.
> + * Works on Cyclone V. Should work on cyclone series.
> + * May work on other Altera fpgas.

I think at one point we decided it's gonna be 'FPGA' to be consistent.

> +
> +static const struct of_device_id of_ef_match[] = {
> +       { .compatible = "altr,cyclonespi-fpga-mgr", },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(of, of_ef_match);
> +
> +static enum fpga_mgr_states cyclonespi_state(struct fpga_manager *mgr)
> +{
> +       return mgr->state;
> +}
> +
> +static inline u32 revbit8x4(u32 n)
> +{
> +       n = ((n & 0xF0F0F0F0UL) >> 4) | ((n & 0x0F0F0F0FUL) << 4);
> +       n = ((n & 0xCCCCCCCCUL) >> 2) | ((n & 0x33333333UL) << 2);
> +       n = ((n & 0xAAAAAAAAUL) >> 1) | ((n & 0x55555555UL) << 1);
> +       return n;
> +}

During the Zynq FPGA manager reviews we decided that manipulating the bitstream
to be consumable by the driver is userland's job.

> +
> +static int cyclonespi_write_init(struct fpga_manager *mgr, u32 flags,
> +                               const char *buf, size_t count)
> +{
> +       struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
> +       u32 *fw32 = (u32 *)buf;
> +       const u32 *fw_end = (u32 *)(buf + count);
> +
> +       if (flags & FPGA_MGR_PARTIAL_RECONFIG) {
> +               dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");
> +               return -EINVAL;

-ENOTSUPP?
> +       }
> +
> +       gpiod_set_value(conf->reset, 0);
> +       udelay(50);

Should that value either be configurable, or a named constant?
> +       msleep(1);

See above.

> +       /* set buffer to lsb first */
> +       while (fw32 < fw_end) {
> +               *fw32 = revbit8x4(*fw32);
> +               fw32++;
> +       }

See above.
> +
> +       return 0;
> +}
> +
> +static int cyclonespi_write(struct fpga_manager *mgr, const char *buf,
> +                          size_t count)
> +{
> +       struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
> +       const char *fw_data = buf;
> +       const char *fw_data_end = fw_data + count;
> +
> +       while (fw_data < fw_data_end) {
> +               int ret;
> +               int stride = fw_data_end - fw_data;
> +
> +               if (stride > SZ_4K)
> +                       stride = SZ_4K;
> +
> +               ret = spi_write(conf->spi, fw_data, stride);
> +               if (ret) {
> +                       dev_err(&mgr->dev, "spi error in firmware write: %d\n",
> +                                       ret);
> +                       return ret;
> +               }
> +               fw_data += stride;
> +       }
> +
> +       return 0;
> +}
> +
> +static int cyclonespi_write_complete(struct fpga_manager *mgr, u32 flags)
> +{
> +       struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
> +
> +       if (gpiod_get_value(conf->status) == 0) {
> +               dev_err(&mgr->dev, "Error during configuration.\n");
> +               return -EIO;
> +       }
> +
> +       return 0;
> +}
> +
> +static const struct fpga_manager_ops cyclonespi_ops = {
> +       .state = cyclonespi_state,
> +       .write_init = cyclonespi_write_init,
> +       .write = cyclonespi_write,
> +       .write_complete = cyclonespi_write_complete,
> +};
> +
> +static int cyclonespi_probe(struct spi_device *spi)
> +{
> +       struct cyclonespi_conf *conf = devm_kzalloc(&spi->dev, sizeof(*conf),
> +                                               GFP_KERNEL);
> +
> +       if (!conf)
> +               return -ENOMEM;
> +
> +       conf->spi = spi;
> +       conf->reset = devm_gpiod_get(&spi->dev, "reset", GPIOD_OUT_LOW);
> +       if (IS_ERR(conf->reset)) {
> +               dev_err(&spi->dev, "Failed to get reset gpio: %ld\n",
> +                       PTR_ERR(conf->reset));
> +               return PTR_ERR(conf->reset);
> +       }
> +
> +       conf->status = devm_gpiod_get(&spi->dev, "status", GPIOD_IN);
> +       if (IS_ERR(conf->status)) {
> +               dev_err(&spi->dev, "Failed to get status gpio: %ld\n",
> +                               PTR_ERR(conf->status));
> +               return PTR_ERR(conf->status);
> +       }
> +
> +       return fpga_mgr_register(&spi->dev, "Altera SPI FPGA Manager",
> +                                &cyclonespi_ops, conf);

Nit: Altera >Cyclone< SPI FPGA Manager

Thanks ... reminds me I wanted to submit my patch for the icoboard ;-)

Moritz

^ permalink raw reply

* [PATCH v2 2/2] ARM: dts: rockchip: Add rk3066 MK808 board
From: Shawn Lin @ 2016-10-07  2:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <b63a0ad452a487beb1d0c07addb9208cc3b3eeef.1475774981.git.paweljarosz3691@gmail.com>

Hi Pawe? ,

On 2016/10/7 1:38, Pawe? Jarosz wrote:
> MK808 is a tv stick which has rockchip rk3066 CPU inside, two usb ports
> - host and otg, micro sd card slot and onboard wifi RK901.
>
> Signed-off-by: Pawe? Jarosz <paweljarosz3691@gmail.com>
> ---
>
> Changes in v2:
> - included Heiko sugestion.
>
>  Documentation/devicetree/bindings/arm/rockchip.txt |   4 +
>  arch/arm/boot/dts/Makefile                         |   1 +
>  arch/arm/boot/dts/rk3066a-mk808.dts                | 184 +++++++++++++++++++++
>  3 files changed, 189 insertions(+)
>  create mode 100644 arch/arm/boot/dts/rk3066a-mk808.dts
>
> diff --git a/Documentation/devicetree/bindings/arm/rockchip.txt b/Documentation/devicetree/bindings/arm/rockchip.txt
> index 55f388f..c09595b 100644
> --- a/Documentation/devicetree/bindings/arm/rockchip.txt
> +++ b/Documentation/devicetree/bindings/arm/rockchip.txt
> @@ -17,6 +17,10 @@ Rockchip platforms device tree bindings
>      Required root node properties:
>        - compatible = "chipspark,rayeager-px2", "rockchip,rk3066a";
>
> +- Rikomagic MK808 v1 board:
> +    Required root node properties:
> +      - compatible = "rikomagic,mk808", "rockchip,rk3066a";
> +
>  - Radxa Rock board:
>      Required root node properties:
>        - compatible = "radxa,rock", "rockchip,rk3188";
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index befcd26..f19cc1d 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -639,6 +639,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += \
>  	rk3036-kylin.dtb \
>  	rk3066a-bqcurie2.dtb \
>  	rk3066a-marsboard.dtb \
> +	rk3066a-mk808.dtb \
>  	rk3066a-rayeager.dtb \
>  	rk3188-radxarock.dtb \
>  	rk3228-evb.dtb \
> diff --git a/arch/arm/boot/dts/rk3066a-mk808.dts b/arch/arm/boot/dts/rk3066a-mk808.dts
> new file mode 100644
> index 0000000..2878562
> --- /dev/null
> +++ b/arch/arm/boot/dts/rk3066a-mk808.dts
> @@ -0,0 +1,184 @@
> +/*
> + * Copyright (c) 2016 Pawe? Jarosz <paweljarosz3691@gmail.com>
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + *     modify it under the terms of the GNU General Public License as
> + *     published by the Free Software Foundation; either version 2 of the
> + *     License, or (at your option) any later version.
> + *
> + *     This file is distributed in the hope that it will be useful,
> + *     but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *     GNU General Public License for more details.
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + *     obtaining a copy of this software and associated documentation
> + *     files (the "Software"), to deal in the Software without
> + *     restriction, including without limitation the rights to use,
> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> + *     sell copies of the Software, and to permit persons to whom the
> + *     Software is furnished to do so, subject to the following
> + *     conditions:
> + *
> + *     The above copyright notice and this permission notice shall be
> + *     included in all copies or substantial portions of the Software.
> + *
> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + *     OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +/dts-v1/;
> +#include "rk3066a.dtsi"
> +
> +/ {
> +	model = "Rikomagic MK808";
> +	compatible = "rikomagic,mk808", "rockchip,rk3066a";
> +
> +	chosen {
> +		stdout-path = "serial2:115200n8";
> +	};
> +
> +	memory at 60000000 {
> +		device_type = "memory";
> +		reg = <0x60000000 0x40000000>;
> +	};
> +
> +	gpio-leds {
> +		compatible = "gpio-leds";
> +
> +		blue {
> +			label = "mk808:blue:power";
> +			gpios = <&gpio0 3 GPIO_ACTIVE_HIGH>;
> +			default-state = "off";
> +			linux,default-trigger = "default-on";
> +		};
> +	};
> +
> +	mmc_pwrseq: mmc-pwrseq {
> +		compatible = "mmc-pwrseq-simple";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&sdmmc_pwr>;

sd slot does not contain a reset pin. So the power pin should be
enough. just add sdmmc_pwr to mmc0's pinctrl-0 and let mmc driver
control it should be enough. Typically pwrseq is for emmc and sdio.
We don't need a pwerseq to control power for sd slot..

But it seems sdmmc_pwr is a GPIO, but not functional port.
So I am interesting that why MK808 board doesn't use the mmc
controller's default power pin but chosing another gpio, so finally
we have to add thses code for your DT.

> +		reset-gpios = <&gpio3 7 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	sdio_pwrseq: sdio-pwrseq {
> +		compatible = "mmc-pwrseq-simple";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&wifi_pwr>;
> +		reset-gpios = <&gpio3 24 GPIO_ACTIVE_LOW>;
> +	};
> +
> +	vcc_io: vcc-io {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc_io";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;

Are you sure it should be fixed to 5V? I ask this since I see
the mmc use this as vmmc which should be 3V~3V3 in principle.

> +		regulator-always-on;

I'm not sure whether you need this. The regulator would maintain
the user_count there. So from this patch, I think it should be turned
off since we possible don't enable usb and mmc?

Moreover, the always-on power supply is depreciated by vmmc since it
cause some problem to a certain degree.

> +	};
> +
> +	vcc_host: usb-host-regulator {
> +		compatible = "regulator-fixed";
> +		gpio = <&gpio0 6 GPIO_ACTIVE_HIGH>;
> +		pinctrl-0 = <&host_drv>;
> +		pinctrl-names = "default";
> +		regulator-always-on;
> +		regulator-name = "host-pwr";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		startup-delay-us = <100000>;
> +		vin-supply = <&vcc_io>;
> +	};
> +
> +	vcc_otg: usb-otg-regulator {
> +		compatible = "regulator-fixed";
> +		gpio = <&gpio0 5 GPIO_ACTIVE_HIGH>;
> +		pinctrl-0 = <&otg_drv>;
> +		pinctrl-names = "default";
> +		regulator-always-on;
> +		regulator-name = "vcc_otg";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		startup-delay-us = <100000>;
> +		vin-supply = <&vcc_io>;
> +	};
> +};

Same concern for these above.

> +
> +&mmc0 {
> +	bus-width = <4>;
> +	cap-mmc-highspeed;
> +	cap-sd-highspeed;
> +	mmc-pwrseq = <&mmc_pwrseq>;

Removed this could make SD works?

Also do you need disable-wp for sd slot?

> +	num-slots = <1>;
> +	status = "okay";
> +	vmmc-supply = <&vcc_io>;
> +};
> +
> +&mmc1 {
> +	bus-width = <4>;
> +	disable-wp;
> +	mmc-pwrseq = <&sdio_pwrseq>;
> +	non-removable;
> +	num-slots = <1>;
> +	status = "okay";
> +	vmmc-supply = <&vcc_io>;
> +};
> +
> +&pinctrl {
> +	usb-host {
> +		host_drv: host-drv {
> +			rockchip,pins = <RK_GPIO0 6 RK_FUNC_GPIO &pcfg_pull_default>;
> +		};
> +	};
> +
> +	usb-otg {
> +		otg_drv: otg-drv {
> +			rockchip,pins = <RK_GPIO0 5 RK_FUNC_GPIO &pcfg_pull_default>;
> +		};
> +	};
> +
> +	sdmmc {
> +		sdmmc_pwr: sdmmc-pwr {
> +			rockchip,pins = <RK_GPIO3 7 RK_FUNC_GPIO &pcfg_pull_default>;
> +		};
> +	};
> +
> +	sdio {
> +		wifi_pwr: wifi-pwr {
> +			rockchip,pins = <RK_GPIO3 24 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +	};
> +};
> +
> +&uart2 {
> +	status = "okay";
> +};
> +
> +&usb_host {
> +	status = "okay";
> +};
> +
> +&usb_otg {
> +	status = "okay";
> +};
> +
> +&usbphy {
> +	status = "okay";
> +};
> +
> +&wdt {
> +	status = "okay";
> +};
> +
>


-- 
Best Regards
Shawn Lin

^ permalink raw reply

* [PATCH 3/3] ARM: dts: lpc18xx: add boot rom node
From: Vladimir Zapolskiy @ 2016-10-07  1:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160913195117.2887-4-manabian@gmail.com>

Hi Joachim,

On 13.09.2016 22:51, Joachim Eastwood wrote:
> Add node for the boot ROM found on all NXP LPC18xx/43xx devices.
> 
> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
> ---
>  arch/arm/boot/dts/lpc18xx.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/lpc18xx.dtsi b/arch/arm/boot/dts/lpc18xx.dtsi
> index 631e6f6c..9f94f3e 100644
> --- a/arch/arm/boot/dts/lpc18xx.dtsi
> +++ b/arch/arm/boot/dts/lpc18xx.dtsi
> @@ -81,6 +81,14 @@
>  			status = "disabled";
>  		};
>  
> +		boot_rom: firmware at 10400000 {
> +			compatible = "nxp,lpc1850-boot-rom";
> +			reg = <0x10400000 0x10000>;
> +			syscon = <&creg>;
> +			nvmem-cells = <&part_id>;

nitpicking, this change has a compile time dependency on yours
"nvmem: add lpc18xx OTP memory driver" series, I haven't noticed
this info stated, but it may be an overlook on my part.

> +			nvmem-cell-names = "PartID";
> +		};
> +
>  		dmac: dma-controller at 40002000 {
>  			compatible = "arm,pl080", "arm,primecell";
>  			arm,primecell-periphid = <0x00041080>;
> 

Tested-by: Vladimir Zapolskiy <vz@mleia.com>
Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>

--
With best wishes,
Vladimir

^ permalink raw reply

* [PATCH 1/3] nvmem: add NXP LPC18xx OTP driver
From: Vladimir Zapolskiy @ 2016-10-07  1:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160913161241.22492-2-manabian@gmail.com>

Hi Joachim, Srinivas,

On 13.09.2016 19:12, Joachim Eastwood wrote:
> Add simple read only driver for the internal OTP (One Time Programmable)
> memory found on all NXP LPC18xx and LPC43xx devices.
> 
> The OTP memory is split into 4 banks each with 4 32-bits word. Some of
> the banks contain predefined data while others are for general purpose
> and user programmable via the OTP API in ROM. Note that writing to the
> OTP memory is not yet supported.
> 
> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
> ---

please feel free to add my

  Tested-by: Vladimir Zapolskiy <vz@mleia.com>

--
With best wishes,
Vladimir

^ permalink raw reply

* [PATCH 1/3] firmware: add lpc18xx boot rom driver
From: Vladimir Zapolskiy @ 2016-10-07  1:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160913195117.2887-2-manabian@gmail.com>

Hi Joachim,

On 13.09.2016 22:51, Joachim Eastwood wrote:
> Firmware driver for the boot ROM found on all NXP LPC18xx/43xx
> devices. This driver makes it possible to retrieve device specific
> information from either the ROM via API calls or from OTP memory.
> 
> The boot ROM contains several APIs for on-chip devices. Note that not
> all APIs are available on all devices. The API to retrieve device
> information and internal Flash programming (IAP) is only available on
> devices with Flash. Flashless devices retrieve device information from
> OTP memory. The CHIPID register in CREG (syscon) is used to check if
> IAP is available.
> 
> For now this driver is only used to expose device information via a
> 'SoC device'. Linux API for the IAP and OTP will be added later. These
> two APIs will be used by a Flash MTD driver and a OTP NVMEM driver to
> program the memory.
> 
> Signed-off-by: Joachim Eastwood <manabian@gmail.com>

please feel free to add to the series my

  Tested-by: Vladimir Zapolskiy <vz@mleia.com>
  Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>

I tested the change on a board powered by LPC4357.

Nevertheless I have some nitpicks for your consideration.

And the first one is that I'm not totally convinced that drivers/firmware
is the proper place for the driver, I tend to think that it might be
a good time to create drivers/soc/nxp/

> ---
>  drivers/firmware/Kconfig            |  12 ++
>  drivers/firmware/Makefile           |   1 +
>  drivers/firmware/nxp_lpc_boot_rom.c | 411 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 424 insertions(+)
>  create mode 100644 drivers/firmware/nxp_lpc_boot_rom.c
> 

[snip]

> +/* LPC18xx/43xx CREG (syscon) defines */
> +#define LPC18XX_CREG_CHIPID		0x200
> +#define LPC18XX_FLASH_CHIPID0		0x4284e02b
> +#define LPC18XX_FLASH_CHIPID1		0x7284e02b
> +#define LPC18XX_FLASHLESS_CHIPID0	0x5284e02b
> +#define LPC18XX_FLASHLESS_CHIPID1	0x6284e02b
> +#define LPC43XX_FLASH_CHIPID0		0x4906002b
> +#define LPC43XX_FLASH_CHIPID1		0x7906002b
> +#define LPC43XX_FLASHLESS_CHIPID0	0x5906002b
> +#define LPC43XX_FLASHLESS_CHIPID1	0x6906002b
> +
> +#define NXP_PART_LPC(_num, _id0, _id1, _sz0, _sz1)	\
> +	{						\
> +		.name = "LPC"#_num,			\
> +		.id[0] = _id0, .id[1] = _id1,		\
> +		.flash_size[0] = _sz0 * 1024,		\
> +		.flash_size[1] = _sz1 * 1024,		\
> +	}
> +
> +

checkpatch complains:

CHECK: Please don't use multiple blank lines
#145: FILE: drivers/firmware/nxp_lpc_boot_rom.c:75:
+
+

> +struct nxp_lpc_part {
> +	const char *name;
> +	u16 flash_size[2];

flash_size[2] is set by NXP_PART_LPC() macro but unused, however
in my build environment (W=1 and sparse checks) I get tons of
legitimate compile time warnings:

drivers/firmware/nxp_lpc_boot_rom.c:93:9: warning: cast truncates bits from constant value (80000 becomes 0)
drivers/firmware/nxp_lpc_boot_rom.c:93:9: warning: cast truncates bits from constant value (80000 becomes 0)
drivers/firmware/nxp_lpc_boot_rom.c:94:9: warning: cast truncates bits from constant value (80000 becomes 0)
drivers/firmware/nxp_lpc_boot_rom.c:94:9: warning: cast truncates bits from constant value (80000 becomes 0)
....

drivers/firmware/nxp_lpc_boot_rom.c:93:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
   NXP_PART_LPC(1857,  0xf001d830, 0x00, 512, 512),
   ^
drivers/firmware/nxp_lpc_boot_rom.c:93:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
drivers/firmware/nxp_lpc_boot_rom.c:94:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
   NXP_PART_LPC(18S57, 0xf001d860, 0x00, 512, 512),
   ^
....

and so on.

Please consider either to store flash size in kB or change storage type to u32.

> +	u32 id[2];
> +};
> +
> +static const struct nxp_lpc_part nxp_lpc_parts[] = {
> +	/* LPC18xx Flashless parts */
> +	NXP_PART_LPC(1850,  0xf000d830, 0x00,   0,   0),
> +	NXP_PART_LPC(18S50, 0xf000d860, 0x00,   0,   0),
> +	NXP_PART_LPC(1830,  0xf000da30, 0x00,   0,   0),
> +	NXP_PART_LPC(18S30, 0xf000da60, 0x00,   0,   0),
> +	NXP_PART_LPC(1820,  0xf00adb3c, 0x00,   0,   0),
> +	NXP_PART_LPC(18S20, 0xf00adb6c, 0x00,   0,   0),
> +	NXP_PART_LPC(1810,  0xf00b5b3f, 0x00,   0,   0),
> +	NXP_PART_LPC(18S10, 0xf00b5b6f, 0x00,   0,   0),
> +	/* LPC18xx Flash parts */
> +	NXP_PART_LPC(1857,  0xf001d830, 0x00, 512, 512),
> +	NXP_PART_LPC(18S57, 0xf001d860, 0x00, 512, 512),
> +	NXP_PART_LPC(1853,  0xf001d830, 0x44, 256, 256),
> +	NXP_PART_LPC(1837,  0xf001da30, 0x00, 512, 512),
> +	NXP_PART_LPC(18S37, 0xf001d860, 0x00, 512, 512),
> +	NXP_PART_LPC(1833,  0xf001da30, 0x44, 256, 256),
> +	NXP_PART_LPC(1827,  0xf001db3c, 0x00, 512, 512),
> +	NXP_PART_LPC(1825,  0xf001db3c, 0x22, 384, 384),
> +	NXP_PART_LPC(1823,  0xf00bdb3c, 0x44, 256, 256),
> +	NXP_PART_LPC(1822,  0xf00bdb3c, 0x80, 512,   0),
> +	NXP_PART_LPC(1817,  0xf001db3f, 0x00, 512, 512),
> +	NXP_PART_LPC(1815,  0xf001db3f, 0x22, 384, 384),
> +	NXP_PART_LPC(1813,  0xf00bdb3f, 0x44, 256, 256),
> +	NXP_PART_LPC(1812,  0xf00bdb3f, 0x80, 512,   0),
> +	/* LPC43xx Flashless parts */
> +	NXP_PART_LPC(4370,  0x00000030, 0x00,   0,   0), /* LBGA256 */
> +	NXP_PART_LPC(4370,  0x00000230, 0x00,   0,   0), /* TFBGA100 */
> +	NXP_PART_LPC(43S70, 0x00000060, 0x00,   0,   0),
> +	NXP_PART_LPC(4350,  0xa0000830, 0x00,   0,   0),
> +	NXP_PART_LPC(43S50, 0xa0000860, 0x00,   0,   0),
> +	NXP_PART_LPC(4330,  0xa0000a30, 0x00,   0,   0),
> +	NXP_PART_LPC(43S30, 0xa0000a60, 0x00,   0,   0),
> +	NXP_PART_LPC(4320,  0xa000cb3c, 0x00,   0,   0),
> +	NXP_PART_LPC(43S20, 0xa000cb6c, 0x00,   0,   0),
> +	NXP_PART_LPC(4310,  0xa00acb3f, 0x00,   0,   0),
> +	/* LPC43xx parts with Flash */
> +	NXP_PART_LPC(4367,  0x8001c030, 0x00, 512, 512),
> +	NXP_PART_LPC(43S67, 0x8001c060, 0x00, 512, 512),
> +	NXP_PART_LPC(4357,  0xa001c830, 0x00, 512, 512),
> +	NXP_PART_LPC(43S57, 0xa001c860, 0x00, 512, 512), /* LBGA256 */
> +	NXP_PART_LPC(43S57, 0xa001ca60, 0x00, 512, 512), /* LQFP208 */
> +	NXP_PART_LPC(4353,  0xa001c830, 0x44, 256, 256),
> +	NXP_PART_LPC(4337,  0xa001ca30, 0x00, 512, 512),
> +	NXP_PART_LPC(43S37, 0xa001ca60, 0x00, 512, 512),
> +	NXP_PART_LPC(4333,  0xa001ca30, 0x44, 256, 256),
> +	NXP_PART_LPC(4327,  0xa001cb3c, 0x00, 512, 512),
> +	NXP_PART_LPC(4325,  0xa001cb3c, 0x22, 384, 384),
> +	NXP_PART_LPC(4323,  0xa00bcb3c, 0x44, 256, 256),
> +	NXP_PART_LPC(4322,  0xa00bcb3c, 0x80, 512,   0),
> +	NXP_PART_LPC(4317,  0xa001cb3f, 0x00, 512, 512),
> +	NXP_PART_LPC(4315,  0xa001cb3f, 0x22, 384, 384),
> +	NXP_PART_LPC(4313,  0xa00bcb3f, 0x44, 256, 256),
> +	NXP_PART_LPC(4312,  0xa00bcb3f, 0x80, 512,   0),
> +};
> +
> +struct iap_rom {
> +	void (*entry)(u32 *, u32 *);
> +};

Here it might be simpler to declare a typedef instead of a struct.

> +struct nxp_rom_api {
> +	struct device *dev;
> +	void __iomem *rom;
> +
> +	bool has_iap;
> +	struct iap_rom iap;
> +	spinlock_t lock;
> +
> +	const struct nxp_lpc_part *part;
> +	const char *partname;
> +	u32 boot_version;
> +
> +	struct soc_device *soc_dev;
> +	struct soc_device_attribute soc_dev_attr;
> +};
> +

--
With best wishes,
Vladimir

^ permalink raw reply

* [PATCH] clk: lpc32xx: add a quirk for PWM and MS clock dividers
From: Vladimir Zapolskiy @ 2016-10-07  1:16 UTC (permalink / raw)
  To: linux-arm-kernel

In common clock framework CLK_DIVIDER_ONE_BASED or'ed with
CLK_DIVIDER_ALLOW_ZERO flags indicates that
1) a divider clock may be set to zero value,
2) divider's zero value is interpreted as a non-divided clock.

On the LPC32xx platform clock dividers of PWM and memory card clocks
comply with the first condition, but zero value means a gated clock,
thus it may happen that the divider value is not updated when
the clock is enabled and the clock remains gated.

The change adds one-shot quirks, which check for zero value of divider
on initialization and set it to a non-zero value, therefore in runtime
a gate clock will work as expected.

Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
 drivers/clk/nxp/clk-lpc32xx.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/nxp/clk-lpc32xx.c b/drivers/clk/nxp/clk-lpc32xx.c
index 34c9735..5b98ff9 100644
--- a/drivers/clk/nxp/clk-lpc32xx.c
+++ b/drivers/clk/nxp/clk-lpc32xx.c
@@ -1282,13 +1282,13 @@ static struct clk_hw_proto clk_hw_proto[LPC32XX_CLK_HW_MAX] = {
 
 	LPC32XX_DEFINE_MUX(PWM1_MUX, PWMCLK_CTRL, 1, 0x1, NULL, 0),
 	LPC32XX_DEFINE_DIV(PWM1_DIV, PWMCLK_CTRL, 4, 4, NULL,
-			   CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO),
+			   CLK_DIVIDER_ONE_BASED),
 	LPC32XX_DEFINE_GATE(PWM1_GATE, PWMCLK_CTRL, 0, 0),
 	LPC32XX_DEFINE_COMPOSITE(PWM1, PWM1_MUX, PWM1_DIV, PWM1_GATE),
 
 	LPC32XX_DEFINE_MUX(PWM2_MUX, PWMCLK_CTRL, 3, 0x1, NULL, 0),
 	LPC32XX_DEFINE_DIV(PWM2_DIV, PWMCLK_CTRL, 8, 4, NULL,
-			   CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO),
+			   CLK_DIVIDER_ONE_BASED),
 	LPC32XX_DEFINE_GATE(PWM2_GATE, PWMCLK_CTRL, 2, 0),
 	LPC32XX_DEFINE_COMPOSITE(PWM2, PWM2_MUX, PWM2_DIV, PWM2_GATE),
 
@@ -1335,8 +1335,7 @@ static struct clk_hw_proto clk_hw_proto[LPC32XX_CLK_HW_MAX] = {
 	LPC32XX_DEFINE_GATE(USB_DIV_GATE, USB_CTRL, 17, 0),
 	LPC32XX_DEFINE_COMPOSITE(USB_DIV, _NULL, USB_DIV_DIV, USB_DIV_GATE),
 
-	LPC32XX_DEFINE_DIV(SD_DIV, MS_CTRL, 0, 4, NULL,
-			   CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO),
+	LPC32XX_DEFINE_DIV(SD_DIV, MS_CTRL, 0, 4, NULL, CLK_DIVIDER_ONE_BASED),
 	LPC32XX_DEFINE_CLK(SD_GATE, MS_CTRL, BIT(5) | BIT(9), BIT(5) | BIT(9),
 			   0x0, BIT(5) | BIT(9), 0x0, 0x0, clk_mask_ops),
 	LPC32XX_DEFINE_COMPOSITE(SD, _NULL, SD_DIV, SD_GATE),
@@ -1478,6 +1477,20 @@ static struct clk * __init lpc32xx_clk_register(u32 id)
 	return clk;
 }
 
+static void __init lpc32xx_clk_div_quirk(u32 reg, u32 div_mask, u32 gate)
+{
+	u32 val;
+
+	regmap_read(clk_regmap, reg, &val);
+
+	if (!(val & div_mask)) {
+		val &= ~gate;
+		val |= BIT(__ffs(div_mask));
+	}
+
+	regmap_update_bits(clk_regmap, reg, gate | div_mask, val);
+}
+
 static void __init lpc32xx_clk_init(struct device_node *np)
 {
 	unsigned int i;
@@ -1517,6 +1530,17 @@ static void __init lpc32xx_clk_init(struct device_node *np)
 		return;
 	}
 
+	/*
+	 * Divider part of PWM and MS clocks requires a quirk to avoid
+	 * a misinterpretation of formally valid zero value in register
+	 * bitfield, which indicates another clock gate. Instead of
+	 * adding complexity to a gate clock ensure that zero value in
+	 * divider clock is never met in runtime.
+	 */
+	lpc32xx_clk_div_quirk(LPC32XX_CLKPWR_PWMCLK_CTRL, 0xf0, BIT(0));
+	lpc32xx_clk_div_quirk(LPC32XX_CLKPWR_PWMCLK_CTRL, 0xf00, BIT(2));
+	lpc32xx_clk_div_quirk(LPC32XX_CLKPWR_MS_CTRL, 0xf, BIT(5) | BIT(9));
+
 	for (i = 1; i < LPC32XX_CLK_MAX; i++) {
 		clk[i] = lpc32xx_clk_register(i);
 		if (IS_ERR(clk[i])) {
-- 
2.8.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox